* [patch c++]:PR/15774 - Conflicting function decls not diagnosed (this time really for 15774) [not found] <AANLkTinGynF-v_UmCCNJrH7GEOcFQDC-5RHc4XbK+Fr2@mail.gmail.com> @ 2010-12-22 23:48 ` Kai Tietz 2010-12-23 12:33 ` Kai Tietz 0 siblings, 1 reply; 10+ messages in thread From: Kai Tietz @ 2010-12-22 23:48 UTC (permalink / raw) To: GCC Patches; +Cc: Jason Merrill [-- Attachment #1: Type: text/plain, Size: 1095 bytes --] Hi, This time with the correct version of patch and changelog for the bug described at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=15774 in bugzilla. It takes care about the following problematic. If there was an old function prototype, and the new one has same or none attributes as the old one, C++ accepts. This behavior is unlikely to the C case, where this condition is warned or in some cases even errored out. If the new declaration specifies attributes, which aren't identical to the old declaration, an error has to be emitted. ChangeLog gcc/cp 2010-12-22 Kai Tietz PR c++/15774 * cp-tree.h (COMPARE_ALLOW_FIRST_NO_ATTRIBUTES): New define. * decl.c (decls_match): Compare parameter and function types by COMPARE_ALLOW_FIRST_NO_ATTRIBUTES. * typeck.c (structural_comptypes): Handle new flag. ChangeLog gcc/testsuite PR c++/15774 * g++.dg/pr15774-1.C: New test. * g++.dg/pr15774-2.C: New test. Tested for i686-pc-cygwin, i686-pc-mingw32, and x86_64-w64-mingw32. Ok for apply? Regards, Kai [-- Attachment #2: pr15774.txt --] [-- Type: text/plain, Size: 3901 bytes --] Index: gcc/gcc/cp/cp-tree.h =================================================================== --- gcc.orig/gcc/cp/cp-tree.h 2010-12-17 21:52:25.000000000 +0100 +++ gcc/gcc/cp/cp-tree.h 2010-12-22 20:31:54.963786500 +0100 @@ -4276,6 +4276,10 @@ enum overload_flags { NO_SPECIAL = 0, DT structural. The actual comparison will be identical to COMPARE_STRICT. */ +#define COMPARE_ALLOW_FIRST_NO_ATTRIBUTES 16 + /* Allow identity for two types, if difference + is in attributes only and first (the new) + has no attributes specified. */ /* Used with push_overloaded_decl. */ #define PUSH_GLOBAL 0 /* Push the DECL into namespace scope, Index: gcc/gcc/cp/decl.c =================================================================== --- gcc.orig/gcc/cp/decl.c 2010-12-17 21:52:25.000000000 +0100 +++ gcc/gcc/cp/decl.c 2010-12-22 20:31:29.549332800 +0100 @@ -1009,7 +1009,12 @@ decls_match (tree newdecl, tree olddecl) } #endif else - types_match = compparms (p1, p2); + types_match = compparms (p1, p2) + && comptypes (TREE_TYPE (newdecl), + TREE_TYPE (olddecl), + COMPARE_REDECLARATION + | COMPARE_ALLOW_FIRST_NO_ATTRIBUTES + ); } else types_match = 0; Index: gcc/gcc/cp/typeck.c =================================================================== --- gcc.orig/gcc/cp/typeck.c 2010-12-17 21:52:25.000000000 +0100 +++ gcc/gcc/cp/typeck.c 2010-12-22 20:31:46.535304400 +0100 @@ -1146,6 +1146,8 @@ comp_template_parms_position (tree t1, t static bool structural_comptypes (tree t1, tree t2, int strict) { + int attr_comp = 1; + if (t1 == t2) return true; @@ -1250,7 +1252,8 @@ structural_comptypes (tree t1, tree t2, case OFFSET_TYPE: if (!comptypes (TYPE_OFFSET_BASETYPE (t1), TYPE_OFFSET_BASETYPE (t2), - strict & ~COMPARE_REDECLARATION)) + strict & ~(COMPARE_REDECLARATION + | COMPARE_ALLOW_FIRST_NO_ATTRIBUTES))) return false; if (!same_type_p (TREE_TYPE (t1), TREE_TYPE (t2))) return false; @@ -1338,7 +1341,16 @@ structural_comptypes (tree t1, tree t2, /* If we get here, we know that from a target independent POV the types are the same. Make sure the target attributes are also the same. */ - return targetm.comp_type_attributes (t1, t2); + attr_comp = targetm.comp_type_attributes (t1, t2); + if (attr_comp || (TREE_CODE (t1) != FUNCTION_TYPE + && TREE_CODE (t1) != METHOD_TYPE)) + return attr_comp; + /* If new type T1 has no attributes, and therefore match fails, allow + check if COMPARE_ALLOW_FIRST_NO_ATTRIBUTES is defined. */ + if ((strict & COMPARE_ALLOW_FIRST_NO_ATTRIBUTES) !=0 + && TYPE_ATTRIBUTES (t1) == NULL) + attr_comp = 1; + return attr_comp; } /* Return true if T1 and T2 are related as allowed by STRICT. STRICT Index: gcc/gcc/testsuite/g++.dg/pr15774-1.C =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ gcc/gcc/testsuite/g++.dg/pr15774-1.C 2010-12-22 21:09:50.303928500 +0100 @@ -0,0 +1,15 @@ +// { dg-do compile { target i?86-*-* } } +// Test that an new declartion with different attributes then old one fail. +extern void foo (int); // { dg-error "ambiguates old declaration" } + +void +bar (void) +{ + foo (1); +} + +void __attribute__((stdcall)) foo (int i) // { dg-error "new declaration" } +{ +} + + Index: gcc/gcc/testsuite/g++.dg/pr15774-2.C =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ gcc/gcc/testsuite/g++.dg/pr15774-2.C 2010-12-22 21:08:46.691290000 +0100 @@ -0,0 +1,15 @@ +// { dg-do compile { target i?86-*-* } } +// Test that old declaration is used, if new one has no attributes. +extern void __attribute__((stdcall)) foo (int); + +void +bar (void) +{ + foo (1); +} + +void foo (int i) +{ +} + + ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch c++]:PR/15774 - Conflicting function decls not diagnosed (this time really for 15774) 2010-12-22 23:48 ` [patch c++]:PR/15774 - Conflicting function decls not diagnosed (this time really for 15774) Kai Tietz @ 2010-12-23 12:33 ` Kai Tietz 2010-12-25 6:27 ` Jason Merrill 0 siblings, 1 reply; 10+ messages in thread From: Kai Tietz @ 2010-12-23 12:33 UTC (permalink / raw) To: GCC Patches; +Cc: Jason Merrill [-- Attachment #1: Type: text/plain, Size: 531 bytes --] Hello, as we discussed yesterday on IRC, I simplified patch by doing checks for compatible attributes directly within decl, but take care the newdecl possibly can omit attributes. ChangeLog gcc/cp 2010-12-23 Kai Tietz * decl.c (decls_match): Check for FUNCTION_DECL also for identity of compatible attributes. ChangeLog gcc/testsuite 2010-12-23 Kai Tietz * g++.dg/pr15774-1.C: New test. * g++.dg/pr15774-2.C: New test. Tested for x86_64-w64-mingw32, i686-pc-cygwin, and i686-pc-mingw32. Ok for apply? Regards, Kai [-- Attachment #2: pr15774.txt --] [-- Type: text/plain, Size: 1647 bytes --] Index: gcc/gcc/cp/decl.c =================================================================== --- gcc.orig/gcc/cp/decl.c 2010-12-23 11:16:30.063178000 +0100 +++ gcc/gcc/cp/decl.c 2010-12-23 11:18:59.781928200 +0100 @@ -1009,7 +1009,11 @@ decls_match (tree newdecl, tree olddecl) } #endif else - types_match = compparms (p1, p2); + types_match = + compparms (p1, p2) + && (TYPE_ATTRIBUTES (TREE_TYPE (newdecl)) == NULL_TREE + || targetm.comp_type_attributes (TREE_TYPE (newdecl), + TREE_TYPE (olddecl)) != 0); } else types_match = 0; Index: gcc/gcc/testsuite/g++.dg/pr15774-1.C =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ gcc/gcc/testsuite/g++.dg/pr15774-1.C 2010-12-23 11:18:59.813178200 +0100 @@ -0,0 +1,15 @@ +// { dg-do compile { target i?86-*-* } } +// Test that an new declartion with different attributes then old one fail. +extern void foo (int); // { dg-error "ambiguates old declaration" } + +void +bar (void) +{ + foo (1); +} + +void __attribute__((stdcall)) foo (int i) // { dg-error "new declaration" } +{ +} + + Index: gcc/gcc/testsuite/g++.dg/pr15774-2.C =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ gcc/gcc/testsuite/g++.dg/pr15774-2.C 2010-12-23 11:18:59.813178200 +0100 @@ -0,0 +1,15 @@ +// { dg-do compile { target i?86-*-* } } +// Test that old declaration is used, if new one has no attributes. +extern void __attribute__((stdcall)) foo (int); + +void +bar (void) +{ + foo (1); +} + +void foo (int i) +{ +} + + ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch c++]:PR/15774 - Conflicting function decls not diagnosed (this time really for 15774) 2010-12-23 12:33 ` Kai Tietz @ 2010-12-25 6:27 ` Jason Merrill 2010-12-25 6:27 ` Kai Tietz 0 siblings, 1 reply; 10+ messages in thread From: Jason Merrill @ 2010-12-25 6:27 UTC (permalink / raw) To: Kai Tietz; +Cc: GCC Patches OK. Does the error message mention the attributes? Jason ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch c++]:PR/15774 - Conflicting function decls not diagnosed (this time really for 15774) 2010-12-25 6:27 ` Jason Merrill @ 2010-12-25 6:27 ` Kai Tietz 2010-12-25 16:00 ` Kai Tietz 2010-12-26 9:50 ` Jason Merrill 0 siblings, 2 replies; 10+ messages in thread From: Kai Tietz @ 2010-12-25 6:27 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches 2010/12/24 Jason Merrill <jason@redhat.com>: > OK. Does the error message mention the attributes? > > Jason > Ok, I'll apply it tomorrow. No the error message doesn't display the attribute as it displays a decl. This would be possibly another task to display attributes (at least a wished limited set) also for declarations. Regards, Kai PS: Merry Christmas ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch c++]:PR/15774 - Conflicting function decls not diagnosed (this time really for 15774) 2010-12-25 6:27 ` Kai Tietz @ 2010-12-25 16:00 ` Kai Tietz 2011-01-04 3:41 ` Dave Korn 2010-12-26 9:50 ` Jason Merrill 1 sibling, 1 reply; 10+ messages in thread From: Kai Tietz @ 2010-12-25 16:00 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches 2010/12/24 Kai Tietz <ktietz70@googlemail.com>: > 2010/12/24 Jason Merrill <jason@redhat.com>: >> OK. Does the error message mention the attributes? >> >> Jason >> > > Ok, I'll apply it tomorrow. No the error message doesn't display the > attribute as it displays a decl. This would be possibly another task > to display attributes (at least a wished limited set) also for > declarations. > > Regards, > Kai > > PS: Merry Christmas > Applied at revision 168241. Kai ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch c++]:PR/15774 - Conflicting function decls not diagnosed (this time really for 15774) 2010-12-25 16:00 ` Kai Tietz @ 2011-01-04 3:41 ` Dave Korn 2011-01-04 6:09 ` Dave Korn 0 siblings, 1 reply; 10+ messages in thread From: Dave Korn @ 2011-01-04 3:41 UTC (permalink / raw) To: Kai Tietz; +Cc: Jason Merrill, GCC Patches On 25/12/2010 10:41, Kai Tietz wrote: > 2010/12/24 Kai Tietz <ktietz70@googlemail.com>: >> 2010/12/24 Jason Merrill <jason@redhat.com>: >>> OK. Does the error message mention the attributes? >>> >>> Jason >>> >> Ok, I'll apply it tomorrow. No the error message doesn't display the >> attribute as it displays a decl. This would be possibly another task >> to display attributes (at least a wished limited set) also for >> declarations. >> >> Regards, >> Kai >> >> PS: Merry Christmas >> > > Applied at revision 168241. Unfortunately this broke the use of typedefs with dll attributes on them, breaking target-libjava build in the process. Testcase, reduced from libjava sources where the first build problem arises: -----------------------snip----------------------- #define JNICALL __stdcall typedef void *jobject; class _Jv_JNIEnv { public: int x; }; jobject _Jv_JNI_ToReflectedField (_Jv_JNIEnv*); typedef struct _Jv_JNIEnv JNIEnv; jobject JNICALL _Jv_JNI_ToReflectedField (JNIEnv *env) { return 0; } -----------------------snip----------------------- With your patch reverted: > admin@ubik /gnu/gcc/obj-mingw-pr43601/gcc/pr17136 > $ ../g++.exe -B.. -c tc.cc -Wall -W 2>&1 | tee pass.log > tc.cc:17:1: warning: unused parameter 'env' [-Wunused-parameter] > > admin@ubik /gnu/gcc/obj-mingw-pr43601/gcc/pr17136 > $ With your patch applied: > admin@ubik /gnu/gcc/obj-mingw-pr43601/gcc/pr17136 > $ ../g++.exe -B.. -c tc.cc -Wall -W > tc.cc: In function 'void* _Jv_JNI_ToReflectedField(JNIEnv*)': > tc.cc:17:38: error: new declaration 'void* _Jv_JNI_ToReflectedField(JNIEnv*)' > tc.cc:12:9: error: ambiguates old declaration 'void* _Jv_JNI_ToReflectedField(_Jv_JNIEnv*)' > tc.cc: At global scope: > tc.cc:17:1: warning: unused parameter 'env' [-Wunused-parameter] > > admin@ubik /gnu/gcc/obj-mingw-pr43601/gcc/pr17136 > $ Can you investigate please Kai? (And the moral of the story is: it's always a good idea to build and test java if you're doing something that affects C++.) cheers, DaveK ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch c++]:PR/15774 - Conflicting function decls not diagnosed (this time really for 15774) 2011-01-04 3:41 ` Dave Korn @ 2011-01-04 6:09 ` Dave Korn 0 siblings, 0 replies; 10+ messages in thread From: Dave Korn @ 2011-01-04 6:09 UTC (permalink / raw) To: Kai Tietz; +Cc: Jason Merrill, GCC Patches On 04/01/2011 03:46, Dave Korn wrote: > On 25/12/2010 10:41, Kai Tietz wrote: >> 2010/12/24 Kai Tietz <ktietz70@googlemail.com>: >>> 2010/12/24 Jason Merrill <jason@redhat.com>: >>>> OK. Does the error message mention the attributes? >>>> >>>> Jason >>>> >>> Ok, I'll apply it tomorrow. No the error message doesn't display the >>> attribute as it displays a decl. This would be possibly another task >>> to display attributes (at least a wished limited set) also for >>> declarations. >>> >>> Regards, >>> Kai >>> >>> PS: Merry Christmas >>> >> Applied at revision 168241. > > Unfortunately this broke the use of typedefs with dll attributes on them, Um, I'm not reading and thinking clearly. This is actually PR17136(*) come back to haunt us. Nothing to do with the typedef in the formal parameters, nor dll attributes; just the missing stdcall on the function prototype that is causing the problem. Reduced testcase: ------------------------------------- #define JNICALL __stdcall int _Jv_JNI_ToReflectedField (void); int JNICALL _Jv_JNI_ToReflectedField (void) { return 0; } ------------------------------------- As the explanation in the PR makes clear, this is a real bug in libjava, that we have not previously diagnosed. Danny suggested a fix in that PR; I'll take a look at updating it for current trunk. cheers, DaveK -- (*) - http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17136 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch c++]:PR/15774 - Conflicting function decls not diagnosed (this time really for 15774) 2010-12-25 6:27 ` Kai Tietz 2010-12-25 16:00 ` Kai Tietz @ 2010-12-26 9:50 ` Jason Merrill 2010-12-26 16:56 ` Kai Tietz 1 sibling, 1 reply; 10+ messages in thread From: Jason Merrill @ 2010-12-26 9:50 UTC (permalink / raw) To: Kai Tietz; +Cc: GCC Patches On 12/24/2010 05:24 PM, Kai Tietz wrote: > Ok, I'll apply it tomorrow. No the error message doesn't display the > attribute as it displays a decl. This would be possibly another task > to display attributes (at least a wished limited set) also for > declarations. I think we want to print whatever attributes cause the attributed entity to be considered distinct from the non-attributed version, rather than a redeclaration. Thinking about it that way, I think your patch actually isn't the right way to handle this: in pr15774-1.C, the two foos are the same foo, just the change in attributes is ill-formed. So decls_match should return 1, but we should complain about attribute mismatch in duplicate_decls, perhaps just before the call to merge_decl_attributes. Jason ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch c++]:PR/15774 - Conflicting function decls not diagnosed (this time really for 15774) 2010-12-26 9:50 ` Jason Merrill @ 2010-12-26 16:56 ` Kai Tietz 2010-12-26 18:19 ` Jason Merrill 0 siblings, 1 reply; 10+ messages in thread From: Kai Tietz @ 2010-12-26 16:56 UTC (permalink / raw) To: Jason Merrill; +Cc: GCC Patches 2010/12/25 Jason Merrill <jason@redhat.com>: > On 12/24/2010 05:24 PM, Kai Tietz wrote: >> >> Ok, I'll apply it tomorrow. No the error message doesn't display the >> attribute as it displays a decl. This would be possibly another task >> to display attributes (at least a wished limited set) also for >> declarations. > > I think we want to print whatever attributes cause the attributed entity to > be considered distinct from the non-attributed version, rather than a > redeclaration. > > Thinking about it that way, I think your patch actually isn't the right way > to handle this: in pr15774-1.C, the two foos are the same foo, just the > change in attributes is ill-formed. So decls_match should return 1, but we > should complain about attribute mismatch in duplicate_decls, perhaps just > before the call to merge_decl_attributes. > > Jason > Hmm, not sure. Case 2 is simply ill-formed, but doesn't cause wrong code. But case 1 in fact can produce wrong code. See as expample extern void foo (int); void doo(int a) { foo(a); } void __attribute__((stdcall)) foo (int i) { } As here in function "doo" the "foo" gets called with cdecl calling-convention, but its implementation gets "stdcall" one, which are incompatible and therefore lead to wrong-code. The -2 case doesn't have this issue Kai -- | (\_/) This is Bunny. Copy and paste | (='.'=) Bunny into your signature to help | (")_(") him gain world domination ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch c++]:PR/15774 - Conflicting function decls not diagnosed (this time really for 15774) 2010-12-26 16:56 ` Kai Tietz @ 2010-12-26 18:19 ` Jason Merrill 0 siblings, 0 replies; 10+ messages in thread From: Jason Merrill @ 2010-12-26 18:19 UTC (permalink / raw) To: Kai Tietz; +Cc: GCC Patches On 12/25/2010 11:00 AM, Kai Tietz wrote: > Hmm, not sure. Case 2 is simply ill-formed, but doesn't cause wrong > code. But case 1 in fact can produce wrong code. See as expample I agree that we should reject it, I'm just saying we should reject it in a different function. :) Jason ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-01-04 3:59 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <AANLkTinGynF-v_UmCCNJrH7GEOcFQDC-5RHc4XbK+Fr2@mail.gmail.com> 2010-12-22 23:48 ` [patch c++]:PR/15774 - Conflicting function decls not diagnosed (this time really for 15774) Kai Tietz 2010-12-23 12:33 ` Kai Tietz 2010-12-25 6:27 ` Jason Merrill 2010-12-25 6:27 ` Kai Tietz 2010-12-25 16:00 ` Kai Tietz 2011-01-04 3:41 ` Dave Korn 2011-01-04 6:09 ` Dave Korn 2010-12-26 9:50 ` Jason Merrill 2010-12-26 16:56 ` Kai Tietz 2010-12-26 18:19 ` Jason Merrill
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).