diff mbox

RFC: C++0x ABI PATCH to decltype handling

Message ID 4D65E4E6.9080508@redhat.com
State New
Headers show

Commit Message

Jason Merrill Feb. 24, 2011, 4:56 a.m. UTC
On 02/22/2011 06:42 PM, Mark Mitchell wrote:
> Maybe we ought to decide that -fcxa-abi=latest (sp?) is the default, and
> that immediately after each release we bump the version, so that every
> compiler has a new version number, and users can always go backwards if
> they really care?  (I know that EDG has done something similar to that.)

Perhaps so; I'm not sure what the right answer is for the default ABI 
version.  For now I've just made these changes ABI v6, and I'm checking 
in this patch.

Tested x86_64-pc-linux-gnu, applied to trunk.
commit b4f6e5b5d9e45d35ce7cafbb843f90064e2d7eab
Author: Jason Merrill <jason@redhat.com>
Date:   Mon Feb 21 21:42:29 2011 -0500

    	* cp-tree.h (DECL_PARM_LEVEL): New.
    	(struct lang_decl_parm): Add level field.
    	* name-lookup.c (function_parm_depth): New fn.
    	* name-lookup.h: Declare it.
    	* parser.c (cp_parser_parameter_declaration_list): Use it.
    	* mangle.c (struct globals): Add parm_depth field.
    	(write_bare_function_type): Adjust it.
    	(write_expression): Include the level delta in PARM_DECL mangling
    	for abi >= 6.
    
    	* semantics.c (finish_decltype_type): Remove shortcut for decltype
    	of id-expression.
    	* mangle.c (write_type) [DECLTYPE_TYPE]: Strip it here for abi < 6.

Comments

Mark Mitchell Feb. 24, 2011, 5:14 a.m. UTC | #1
On 2/23/2011 8:56 PM, Jason Merrill wrote:

> Perhaps so; I'm not sure what the right answer is for the default ABI
> version.  For now I've just made these changes ABI v6, and I'm checking
> in this patch.

That's good future-proofing.

I know I'm being a bit pedantic, but I was just getting beat up about
GCC backwards-compatibility again today.  It's possible to fix 100 bugs,
add 100 features, and still have an unhappy user if their code stops
working.

Thank you,
Jack Howarth Feb. 26, 2011, 4:30 p.m. UTC | #2
On Wed, Feb 23, 2011 at 09:14:11PM -0800, Mark Mitchell wrote:
> On 2/23/2011 8:56 PM, Jason Merrill wrote:
> 
> > Perhaps so; I'm not sure what the right answer is for the default ABI
> > version.  For now I've just made these changes ABI v6, and I'm checking
> > in this patch.
> 
> That's good future-proofing.
> 
> I know I'm being a bit pedantic, but I was just getting beat up about
> GCC backwards-compatibility again today.  It's possible to fix 100 bugs,
> add 100 features, and still have an unhappy user if their code stops
> working.
> 
> Thank you,

Mark and Jason,
   This change regresses g++.dg/abi/mangle39.C on all darwin targets and introduces
new failures with the g++.dg/abi/mangle45.C testcase...

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47906

Isn't it rather late in stage 4 for a change like this anyway?
                Jack

> 
> -- 
> Mark Mitchell
> CodeSourcery
> mark@codesourcery.com
> (650) 331-3385 x713
Jakub Jelinek Feb. 26, 2011, 5:25 p.m. UTC | #3
On Sat, Feb 26, 2011 at 11:30:51AM -0500, Jack Howarth wrote:
> On Wed, Feb 23, 2011 at 09:14:11PM -0800, Mark Mitchell wrote:
> > On 2/23/2011 8:56 PM, Jason Merrill wrote:
> > 
> > > Perhaps so; I'm not sure what the right answer is for the default ABI
> > > version.  For now I've just made these changes ABI v6, and I'm checking
> > > in this patch.
> > 
> > That's good future-proofing.
> > 
> > I know I'm being a bit pedantic, but I was just getting beat up about
> > GCC backwards-compatibility again today.  It's possible to fix 100 bugs,
> > add 100 features, and still have an unhappy user if their code stops
> > working.
> > 
> > Thank you,
> 
>    This change regresses g++.dg/abi/mangle39.C on all darwin targets and introduces
> new failures with the g++.dg/abi/mangle45.C testcase...

Both are new tests, so it can hardly regress.
The thing is just that Jason wants to match there the aliases created by:
#ifdef ASM_OUTPUT_DEF
      save_ver = flag_abi_version;
      flag_abi_version = 0;
      id2 = mangle_decl_string (decl);
      id2 = targetm.mangle_decl_assembler_name (decl, id2);
      flag_abi_version = save_ver;

      alias = make_alias_for (decl, id2);
      DECL_IGNORED_P (alias) = 1;
      TREE_PUBLIC (alias) = TREE_PUBLIC (decl);
      DECL_VISIBILITY (alias) = DECL_VISIBILITY (decl);
      if (vague_linkage_p (decl))
        DECL_WEAK (alias) = 1;
      if (TREE_CODE (decl) == FUNCTION_DECL)
        cgraph_same_body_alias (alias, decl);
      else
        varpool_extra_name_alias (alias, decl);
#endif
in mangle_decl, and likely Darwin doesn't emit them.
I guess either the tests could have -fabi-version=0 among dg-options, or
we should copy the tests, have one set of tests for the current
-fabi-version=2 and another one for -fabi-version=0.

This is just a testsuite issue.

	Jakub
Jack Howarth Feb. 26, 2011, 7:24 p.m. UTC | #4
On Sat, Feb 26, 2011 at 06:25:00PM +0100, Jakub Jelinek wrote:
> On Sat, Feb 26, 2011 at 11:30:51AM -0500, Jack Howarth wrote:
> > On Wed, Feb 23, 2011 at 09:14:11PM -0800, Mark Mitchell wrote:
> > > On 2/23/2011 8:56 PM, Jason Merrill wrote:
> > > 
> > > > Perhaps so; I'm not sure what the right answer is for the default ABI
> > > > version.  For now I've just made these changes ABI v6, and I'm checking
> > > > in this patch.
> > > 
> > > That's good future-proofing.
> > > 
> > > I know I'm being a bit pedantic, but I was just getting beat up about
> > > GCC backwards-compatibility again today.  It's possible to fix 100 bugs,
> > > add 100 features, and still have an unhappy user if their code stops
> > > working.
> > > 
> > > Thank you,
> > 
> >    This change regresses g++.dg/abi/mangle39.C on all darwin targets and introduces
> > new failures with the g++.dg/abi/mangle45.C testcase...
> 
> Both are new tests, so it can hardly regress.

Huh? The g++.dg/abi/mangle39.C testcase predates r170459 by over a year so it certainly
should be a regression.

Author: jason
Date: Thu Jan 21 01:58:53 2010
New Revision: 156103

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=156103
Log:
	PR c++/42338
	* mangle.c (write_expression): Handle tree codes that have extra
	arguments in the middle-end.
	* cp-demangle.c (d_print_comp): Fix array index printing.

Added:
    trunk/gcc/testsuite/g++.dg/abi/mangle39.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/mangle.c
    trunk/libiberty/ChangeLog
    trunk/libiberty/cp-demangle.c


> The thing is just that Jason wants to match there the aliases created by:
> #ifdef ASM_OUTPUT_DEF
>       save_ver = flag_abi_version;
>       flag_abi_version = 0;
>       id2 = mangle_decl_string (decl);
>       id2 = targetm.mangle_decl_assembler_name (decl, id2);
>       flag_abi_version = save_ver;
> 
>       alias = make_alias_for (decl, id2);
>       DECL_IGNORED_P (alias) = 1;
>       TREE_PUBLIC (alias) = TREE_PUBLIC (decl);
>       DECL_VISIBILITY (alias) = DECL_VISIBILITY (decl);
>       if (vague_linkage_p (decl))
>         DECL_WEAK (alias) = 1;
>       if (TREE_CODE (decl) == FUNCTION_DECL)
>         cgraph_same_body_alias (alias, decl);
>       else
>         varpool_extra_name_alias (alias, decl);
> #endif
> in mangle_decl, and likely Darwin doesn't emit them.
> I guess either the tests could have -fabi-version=0 among dg-options, or
> we should copy the tests, have one set of tests for the current
> -fabi-version=2 and another one for -fabi-version=0.
> 
> This is just a testsuite issue.
> 
> 	Jakub
Jakub Jelinek Feb. 26, 2011, 7:43 p.m. UTC | #5
On Sat, Feb 26, 2011 at 02:24:57PM -0500, Jack Howarth wrote:
> Huh? The g++.dg/abi/mangle39.C testcase predates r170459 by over a year so it certainly
> should be a regression.

You haven't looked carefully, 170459 changed the expected mangling from
the -fabi-version=2 to -fabi-version=6, so the part of the testcase which
actually fails on targets that are incapable of supplying both old and newly
mangled aliases is new too.

	Jakub
Gerald Pfeifer April 2, 2011, 11:45 p.m. UTC | #6
On Wed, 23 Feb 2011, Mark Mitchell wrote:
>> Perhaps so; I'm not sure what the right answer is for the default ABI
>> version.  For now I've just made these changes ABI v6, and I'm checking
>> in this patch.
> That's good future-proofing.
> 
> I know I'm being a bit pedantic, but I was just getting beat up about
> GCC backwards-compatibility again today.  It's possible to fix 100 bugs,
> add 100 features, and still have an unhappy user if their code stops
> working.

Well, we've just done it once again.  Quoting from the GCC 4.6 release
notes:

   Most libstdc++ standard headers have been changed to no longer include 
   the <code>cstddef</code> header as an implementation detail.  Code that 
   relied on that header being included as side-effect of including other 
   standard headers will need to include <code>cstddef</code> explicitly.

Gerald
diff mbox

Patch

diff --git a/gcc/common.opt b/gcc/common.opt
index f95805c..2717b11 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -741,6 +741,12 @@  Driver Undocumented
 ; 4: The version of the ABI that introduces unambiguous mangling of
 ;    vector types.
 ;
+; 5: The version of the ABI that ignores attribute const/noreturn
+;    in function pointer mangling.
+;
+; 6: The version of the ABI that corrects mangling of decltype and
+;    function parameters used in other parameters and the return type.
+;
 ; Additional positive integers will be assigned as new versions of
 ; the ABI become the default version of the ABI.
 fabi-version=
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 238d0cf..ee72322 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -1914,6 +1914,7 @@  struct GTY(()) lang_decl_ns {
 
 struct GTY(()) lang_decl_parm {
   struct lang_decl_base base;
+  int level;
   int index;
 };
 
@@ -2108,6 +2109,13 @@  struct GTY((variable_size)) lang_decl {
 #define DECL_PARM_INDEX(NODE) \
   (LANG_DECL_PARM_CHECK (NODE)->index)
 
+/* The level of a user-declared parameter in its function, starting at 1.
+   A parameter of the function will have level 1; a parameter of the first
+   nested function declarator (i.e. t in void f (void (*p)(T t))) will have
+   level 2.  */
+#define DECL_PARM_LEVEL(NODE) \
+  (LANG_DECL_PARM_CHECK (NODE)->level)
+
 /* Nonzero if the VTT parm has been added to NODE.  */
 #define DECL_HAS_VTT_PARM_P(NODE) \
   (LANG_DECL_FN_CHECK (NODE)->has_vtt_parm_p)
diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index 4d2ace6..dca8b60 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -96,6 +96,9 @@  typedef struct GTY(()) globals {
   /* The entity that is being mangled.  */
   tree GTY ((skip)) entity;
 
+  /* How many parameter scopes we are inside.  */
+  int parm_depth;
+
   /* True if the mangling will be different in a future version of the
      ABI.  */
   bool need_abi_warning;
@@ -1931,6 +1934,35 @@  write_type (tree type)
 	      gcc_assert (!DECLTYPE_FOR_LAMBDA_CAPTURE (type)
 			  && !DECLTYPE_FOR_LAMBDA_RETURN (type));
 
+	      /* In ABI <6, we stripped decltype of a plain decl.  */
+	      if (!abi_version_at_least (6)
+		  && DECLTYPE_TYPE_ID_EXPR_OR_MEMBER_ACCESS_P (type))
+		{
+		  tree expr = DECLTYPE_TYPE_EXPR (type);
+		  tree etype = NULL_TREE;
+		  switch (TREE_CODE (expr))
+		    {
+		    case VAR_DECL:
+		    case PARM_DECL:
+		    case RESULT_DECL:
+		    case FUNCTION_DECL:
+		    case CONST_DECL:
+		    case TEMPLATE_PARM_INDEX:
+		      etype = TREE_TYPE (expr);
+		      break;
+
+		    default:
+		      break;
+		    }
+
+		  if (etype && !type_uses_auto (etype))
+		    {
+		      G.need_abi_warning = 1;
+		      write_type (etype);
+		      return;
+		    }
+		}
+
               write_char ('D');
               if (DECLTYPE_TYPE_ID_EXPR_OR_MEMBER_ACCESS_P (type))
                 write_char ('t');
@@ -2270,9 +2302,11 @@  write_bare_function_type (const tree type, const int include_return_type_p,
     write_type (TREE_TYPE (type));
 
   /* Now mangle the types of the arguments.  */
+  ++G.parm_depth;
   write_method_parms (TYPE_ARG_TYPES (type),
 		      TREE_CODE (type) == METHOD_TYPE,
 		      decl);
+  --G.parm_depth;
 }
 
 /* Write the mangled representation of a method parameter list of
@@ -2458,8 +2492,28 @@  write_expression (tree expr)
     {
       /* A function parameter used in a late-specified return type.  */
       int index = DECL_PARM_INDEX (expr);
+      int level = DECL_PARM_LEVEL (expr);
+      int delta = G.parm_depth - level + 1;
       gcc_assert (index >= 1);
-      write_string ("fp");
+      write_char ('f');
+      if (delta != 0)
+	{
+	  if (abi_version_at_least (6))
+	    {
+	      /* Let L be the number of function prototype scopes from the
+		 innermost one (in which the parameter reference occurs) up
+		 to (and including) the one containing the declaration of
+		 the referenced parameter.  If the parameter declaration
+		 clause of the innermost function prototype scope has been
+		 completely seen, it is not counted (in that case -- which
+		 is perhaps the most common -- L can be zero).  */
+	      write_char ('L');
+	      write_unsigned_number (delta - 1);
+	    }
+	  else
+	    G.need_abi_warning = true;
+	}
+      write_char ('p');
       write_compact_number (index - 1);
     }
   else if (DECL_P (expr))
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index e2e5450..4117202 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -1635,6 +1635,22 @@  getdecls (void)
   return current_binding_level->names;
 }
 
+/* Return how many function prototypes we are currently nested inside.  */
+
+int
+function_parm_depth (void)
+{
+  int level = 0;
+  struct cp_binding_level *b;
+
+  for (b = current_binding_level;
+       b->kind == sk_function_parms;
+       b = b->level_chain)
+    ++level;
+
+  return level;
+}
+
 /* For debugging.  */
 static int no_print_functions = 0;
 static int no_print_builtins = 0;
diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
index f81a565..bfcac69 100644
--- a/gcc/cp/name-lookup.h
+++ b/gcc/cp/name-lookup.h
@@ -333,6 +333,7 @@  extern bool pushdecl_class_level (tree);
 extern tree pushdecl_namespace_level (tree, bool);
 extern bool push_class_level_binding (tree, tree);
 extern tree getdecls (void);
+extern int function_parm_depth (void);
 extern tree cp_namespace_decls (tree);
 extern void set_decl_namespace (tree, tree, bool);
 extern void push_decl_namespace (tree);
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 5b08389..1e8f03b 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -15942,6 +15942,7 @@  cp_parser_parameter_declaration_list (cp_parser* parser, bool *is_error)
 	{
 	  retrofit_lang_decl (decl);
 	  DECL_PARM_INDEX (decl) = ++index;
+	  DECL_PARM_LEVEL (decl) = function_parm_depth ();
 	}
 
       /* Add the new parameter to the list.  */
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 4af007d..8a7dd7d 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -4772,6 +4772,7 @@  finish_decltype_type (tree expr, bool id_expression_or_member_access_p)
       return error_mark_node;
     }
 
+  /* FIXME instantiation-dependent  */
   if (type_dependent_expression_p (expr)
       /* In a template, a COMPONENT_REF has an IDENTIFIER_NODE for op1 even
 	 if it isn't dependent, so that we can check access control at
@@ -4780,27 +4781,6 @@  finish_decltype_type (tree expr, bool id_expression_or_member_access_p)
 	  && processing_template_decl
 	  && TREE_CODE (expr) == COMPONENT_REF))
     {
-      if (id_expression_or_member_access_p)
-	{
-	  switch (TREE_CODE (expr))
-	    {
-	    case VAR_DECL:
-	    case PARM_DECL:
-	    case RESULT_DECL:
-	    case FUNCTION_DECL:
-	    case CONST_DECL:
-	    case TEMPLATE_PARM_INDEX:
-	      type = TREE_TYPE (expr);
-	      break;
-
-	    default:
-	      break;
-	    }
-	}
-
-      if (type && !type_uses_auto (type))
-	return type;
-
     treat_as_dependent:
       type = cxx_make_type (DECLTYPE_TYPE);
       DECLTYPE_TYPE_EXPR (type) = expr;
diff --git a/gcc/testsuite/g++.dg/abi/mangle39.C b/gcc/testsuite/g++.dg/abi/mangle39.C
index 30a08b0..272385b 100644
--- a/gcc/testsuite/g++.dg/abi/mangle39.C
+++ b/gcc/testsuite/g++.dg/abi/mangle39.C
@@ -1,7 +1,7 @@ 
 // PR c++/42338
 // { dg-options "-std=c++0x" }
 // { dg-final { scan-assembler "_Z1fIPiEDTcmppfp_Li0EET_" } }
-// { dg-final { scan-assembler "_Z1gIiEvRK1AIT_EDTixfp_Li0EE" } }
+// { dg-final { scan-assembler "_Z1gIiEvRK1AIT_EDTixfL0p_Li0EE" } }
 
 template<typename T>
 auto f(T t) -> decltype(++t, 0)
diff --git a/gcc/testsuite/g++.dg/abi/mangle45.C b/gcc/testsuite/g++.dg/abi/mangle45.C
new file mode 100644
index 0000000..8e20831
--- /dev/null
+++ b/gcc/testsuite/g++.dg/abi/mangle45.C
@@ -0,0 +1,25 @@ 
+// Testcase for mangling of parameters used other than in a trailing return type
+// { dg-options -std=c++0x }
+
+template<class T> void f(T p, decltype(p)) { }                // L = 1
+template<class T> void g(T p, decltype(p) (*)()) { }          // L = 1
+// G++ incorrectly rejects these currently.
+// template<class T> void h(T p, auto (*)()->decltype(p));    // L = 1
+// template<class T> void i(T p, auto (*)(T q)->decltype(q)); // L = 0
+// template<class T> void j(T p, auto (*)(decltype(p))->T);   // L = 2
+template<class T> void k(T p, int (*(*)(T* p))[sizeof(p)]) {} // L = 1
+
+int garg();
+int (*karg (int*))[sizeof(int)];
+int main()
+{
+  // { dg-final { scan-assembler  "_Z1fIiEvT_DtfL0p_E" } }
+  f (1,0);
+  // { dg-final { scan-assembler  "_Z1gIiEvT_PFDtfL0p_EvE" } }
+  g (1,garg);
+  // h (1,0);
+  // i (1,0);
+  // j (1,0);
+  // { dg-final { scan-assembler  "_Z1kIiEvT_PFPAszfL0p__iPS0_E" } }
+  k (1,karg);
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/trailing1.C b/gcc/testsuite/g++.dg/cpp0x/trailing1.C
index 11e73d2..fcf65e3 100644
--- a/gcc/testsuite/g++.dg/cpp0x/trailing1.C
+++ b/gcc/testsuite/g++.dg/cpp0x/trailing1.C
@@ -78,7 +78,6 @@  auto k(T t, U u, V v) -> decltype (t.U::template B<V>::MEM)
   return t.U::template B<V>::MEM;
 }
 
-// For these two examples we can elide the 'decltype' and just mangle the type.
 template <class T>
 auto l(T t) -> decltype (t)
 {
@@ -111,8 +110,8 @@  int main()
   h(a,1.0);
   // { dg-final { scan-assembler "_Z1kI1C1AIiE1DEDtdtfp_srNT0_1BIT1_EE3MEMET_S4_S6_" } }
   k( C(), A<int>(), D() );
-  // { dg-final { scan-assembler "_Z1lIiET_S0_" } }
+  // { dg-final { scan-assembler "_Z1lIiEDtfp_ET_" } }
   l(1);
-  // { dg-final { scan-assembler "_Z1mIiLi1EET_S0_" } }
+  // { dg-final { scan-assembler "_Z1mIiLi1EEDtT0_ET_" } }
   m<int,1>(1);
 }
diff --git a/gcc/testsuite/g++.dg/template/canon-type-12.C b/gcc/testsuite/g++.dg/template/canon-type-12.C
index 694cc5e..08c86f0 100644
--- a/gcc/testsuite/g++.dg/template/canon-type-12.C
+++ b/gcc/testsuite/g++.dg/template/canon-type-12.C
@@ -9,7 +9,7 @@  struct S
 
 template<class T, T t>
 void
-S<T, t>::foo(T)
+S<T, t>::foo(decltype(t))
 {
 }
 
diff --git a/gcc/testsuite/g++.dg/template/canon-type-9.C b/gcc/testsuite/g++.dg/template/canon-type-9.C
index 8d1aa29..4fcd524 100644
--- a/gcc/testsuite/g++.dg/template/canon-type-9.C
+++ b/gcc/testsuite/g++.dg/template/canon-type-9.C
@@ -11,7 +11,7 @@  struct S
 };
 
 template<class T, T *u>
-T* S<T, u>::foo(T)
+decltype(u) S<T, u>::foo(T)
 {
   T t;
   return t;