| Message ID | CAEwic4azzVr9MVpoKp_P-bVSju5DLbE2mfNC4SBzOaYT6DDxYg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Hi Kai, > 2014-11-28 Kai Tietz <ktietz@redhat.com> > > * gcc.dg/anon-struct-1.c: > * gcc.dg/anon-struct-11.c: > * gcc.dg/anon-struct-2.c: > * gcc.dg/c11-anon-struct-2.c: > * gcc.dg/c11-anon-struct-3.c: those are not exactly useful ChangeLog entries ;-) Rainer
2014-11-28 12:14 GMT+01:00 Rainer Orth <ro@cebitec.uni-bielefeld.de>: > Hi Kai, > >> 2014-11-28 Kai Tietz <ktietz@redhat.com> >> >> * gcc.dg/anon-struct-1.c: >> * gcc.dg/anon-struct-11.c: >> * gcc.dg/anon-struct-2.c: >> * gcc.dg/c11-anon-struct-2.c: >> * gcc.dg/c11-anon-struct-3.c: > > those are not exactly useful ChangeLog entries ;-) > > Rainer Sorry, missed that ... I had them prior together with other patch ... Statement would be: Disable ms-extensions for mingw-targets. ... Kai
Kai Tietz <ktietz70@googlemail.com> writes: > 2014-11-28 12:14 GMT+01:00 Rainer Orth <ro@cebitec.uni-bielefeld.de>: >> Hi Kai, >> >>> 2014-11-28 Kai Tietz <ktietz@redhat.com> >>> >>> * gcc.dg/anon-struct-1.c: >>> * gcc.dg/anon-struct-11.c: >>> * gcc.dg/anon-struct-2.c: >>> * gcc.dg/c11-anon-struct-2.c: >>> * gcc.dg/c11-anon-struct-3.c: >> >> those are not exactly useful ChangeLog entries ;-) >> >> Rainer > > Sorry, missed that ... I had them prior together with other patch ... > Statement would be: Disable ms-extensions for mingw-targets. ... ... and again, here holds what I said elsewhere: better use Disable ms-extensions for *-*-mingw*. which is more accurate/descriptive. Rainer
On Fri, 28 Nov 2014, Kai Tietz wrote: > Hi, > > this patch turns off ms-extensions for mingw-targets to match > diagnostics checked in testcases. > > Ok for apply? For the tests using -std=<some ISO standard> -pedantic (or -pedantic-errors), are you saying the diagnostics are *different*, or that some constructs (that are invalid in the relevant ISO standard) are *not diagnosed at all*? If the latter, it's a bug in the MinGW port - when a conformance mode is selected, the constructs invalid in that mode must be diagnosed appropriately, independent of the target. You could for example disable ms-extensions if flag_iso, although it might be better to distinguish explicit and implicit ms-extensions and have the (pedantic, only OK because of implicit ms-extensions) case give a pedwarn and then continue with what it would have done with the extension enabled.
2014-11-28 18:53 GMT+01:00 Joseph Myers <joseph@codesourcery.com>: > On Fri, 28 Nov 2014, Kai Tietz wrote: > >> Hi, >> >> this patch turns off ms-extensions for mingw-targets to match >> diagnostics checked in testcases. >> >> Ok for apply? > > For the tests using -std=<some ISO standard> -pedantic (or > -pedantic-errors), are you saying the diagnostics are *different*, or that > some constructs (that are invalid in the relevant ISO standard) are *not > diagnosed at all*? If the latter, it's a bug in the MinGW port - when a > conformance mode is selected, the constructs invalid in that mode must be > diagnosed appropriately, independent of the target. You could for example > disable ms-extensions if flag_iso, although it might be better to > distinguish explicit and implicit ms-extensions and have the (pedantic, > only OK because of implicit ms-extensions) case give a pedwarn and then > continue with what it would have done with the extension enabled. > > -- > Joseph S. Myers > joseph@codesourcery.com Some diagnostics are different and some constructs getting allowed with enabled ms-extensions flag. Additionally is the pedantic-flag not automatically set for *-*-mingw* targets. So for enforcing ISO-C++ pedantic checks the *-*-mingw* targets need to have explicit set the -pedantic flag (which is on for some other targets by default). Kai
On Fri, 28 Nov 2014, Kai Tietz wrote: > Some diagnostics are different and some constructs getting allowed > with enabled ms-extensions flag. Additionally is the pedantic-flag > not automatically set for *-*-mingw* targets. So for enforcing > ISO-C++ pedantic checks the *-*-mingw* targets need to have explicit > set the -pedantic flag (which is on for some other targets by > default). It seems very odd for the default dg-options to vary depending on target - where is that target-dependent default set? But for my main point: -std=c11 -pedantic-errors must diagnose with an error anything where C11 requires a diagnostic. Not "anything where C11 requires a diagnostic, except for certain extensions on MinGW targets". It must not be necessary to add -fno-ms-extensions for such diagnostics; the options for conformance must not depend on the target. So for the cases where the test uses -std=<some ISO standard> -pedantic/-pedantic-errors, you need to fix the port: the test failures are showing up an actual bug in the port, and the tests are correct as-is. The same principle of course applies to C++ tests, though I didn't examine your changes to those to see if the same issue applies to any of those changes.
2014-11-28 19:10 GMT+01:00 Joseph Myers <joseph@codesourcery.com>: > On Fri, 28 Nov 2014, Kai Tietz wrote: > >> Some diagnostics are different and some constructs getting allowed >> with enabled ms-extensions flag. Additionally is the pedantic-flag >> not automatically set for *-*-mingw* targets. So for enforcing >> ISO-C++ pedantic checks the *-*-mingw* targets need to have explicit >> set the -pedantic flag (which is on for some other targets by >> default). > > It seems very odd for the default dg-options to vary depending on target - > where is that target-dependent default set? True, I was curious to see this difference too. > But for my main point: -std=c11 -pedantic-errors must diagnose with an > error anything where C11 requires a diagnostic. Not "anything where C11 > requires a diagnostic, except for certain extensions on MinGW targets". > It must not be necessary to add -fno-ms-extensions for such diagnostics; > the options for conformance must not depend on the target. So for the > cases where the test uses -std=<some ISO standard> > -pedantic/-pedantic-errors, you need to fix the port: the test failures > are showing up an actual bug in the port, and the tests are correct as-is. > The same principle of course applies to C++ tests, though I didn't examine > your changes to those to see if the same issue applies to any of those > changes. How so? That ms-extensions are enabled by default is all the port does. The rest is done in FEs, and indeed the diagnostic here about handling ms-extensions, or on standard-case seems to be buggy. To depend in testsuite on implicit set flags seems to be a bug too. So I can't follow you that it is a port issue. .. Kai Tietz ktietz@redhat.com > -- > Joseph S. Myers > joseph@codesourcery.com
On Fri, 28 Nov 2014, Kai Tietz wrote: > 2014-11-28 19:10 GMT+01:00 Joseph Myers <joseph@codesourcery.com>: > > On Fri, 28 Nov 2014, Kai Tietz wrote: > > > >> Some diagnostics are different and some constructs getting allowed > >> with enabled ms-extensions flag. Additionally is the pedantic-flag > >> not automatically set for *-*-mingw* targets. So for enforcing > >> ISO-C++ pedantic checks the *-*-mingw* targets need to have explicit > >> set the -pedantic flag (which is on for some other targets by > >> default). > > > > It seems very odd for the default dg-options to vary depending on target - > > where is that target-dependent default set? > > True, I was curious to see this difference too. Well, clearly you shouldn't be changing the tests without actually understanding where the problem comes from. As far as I can tell, the -ansi -pedantic-errors default (-pedantic-errors looping over C++ standards for C++) is hardcoded in the relevant .exp files, so there certainly shouldn't be any per-target variation. So if the motivation for any testcase patch was supposed variation in the default, you need to go back and work out what the *real* cause of the difference was, and then reassess the correct fix based on that. > > But for my main point: -std=c11 -pedantic-errors must diagnose with an > > error anything where C11 requires a diagnostic. Not "anything where C11 > > requires a diagnostic, except for certain extensions on MinGW targets". > > It must not be necessary to add -fno-ms-extensions for such diagnostics; > > the options for conformance must not depend on the target. So for the > > cases where the test uses -std=<some ISO standard> > > -pedantic/-pedantic-errors, you need to fix the port: the test failures > > are showing up an actual bug in the port, and the tests are correct as-is. > > The same principle of course applies to C++ tests, though I didn't examine > > your changes to those to see if the same issue applies to any of those > > changes. > > How so? That ms-extensions are enabled by default is all the port > does. The rest is done in FEs, and indeed the diagnostic here about > handling ms-extensions, or on standard-case seems to be buggy. To > depend in testsuite on implicit set flags seems to be a bug too. > So I can't follow you that it is a port issue. The semantics of -std=c11 -pedantic-errors don't depend on the port. It is documented, target-independent, that those options are sufficient to cause diagnosis of anything for which C11 requires a diagnostic. If you find you need -fno-ms-extensions as well, that's not a bug in the documentation or the testcase, it's a bug in GCC, failing to give a diagnostic for certain code for certain targets even though the standard requires it to do so. The testcase has shown up that GCC is behaving incorrectly - it's done what testcases are meant to do. Rather than making the testcase incorrect to match the bug, you should fix the bug. The very simple fix is to remove the -fms-extensions default as ill-conceived. If you don't want to do that, the next simplest fix is to remove that default if flag_iso, so it doesn't interfere with standards conformance. However, since the purpose of flag_iso is not to cause code to be rejected, a preferable fix would be to take the code in c-decl.c:grokfield that computes "ok" and compute it twice: once with the existing logic, once (computing std_ok, say) with flag_ms_extensions changed to (flag_ms_extensions && global_options_set.x.x_flag_ms_extensions). If ok && !std_ok (i.e. something would only be accepted because of a default -fms-extensions), then do a pedwarn (loc, OPT_pedantic, ...) to diagnose the use of an extension not permitted by ISO C. Something similar should be done in any C++ case where the standard requires a diagnostic but GCC for MinGW target is wrongly failing to give such a diagnostic with -pedantic.
On 11/28/2014 03:35 PM, Joseph Myers wrote: > If you find you need -fno-ms-extensions as well, that's not a bug in the > documentation or the testcase, it's a bug in GCC, failing to give a > diagnostic for certain code for certain targets even though the standard > requires it to do so. The testcase has shown up that GCC is behaving > incorrectly - it's done what testcases are meant to do. Rather than > making the testcase incorrect to match the bug, you should fix the bug. I agree, with -pedantic we should complain about use of MS extensions outside of system headers even if we then proceed to compile the program. Jason
Index: gcc.dg/anon-struct-1.c =================================================================== --- gcc.dg/anon-struct-1.c (Revision 218142) +++ gcc.dg/anon-struct-1.c (Arbeitskopie) @@ -1,4 +1,5 @@ /* { dg-options "-std=iso9899:1990 -pedantic" } */ +/* { dg-additional-options "-fno-ms-extensions" { target *-*-mingw* } } */ /* In strict ISO C mode, we don't recognize the anonymous struct/union extension or any Microsoft extensions. */ Index: gcc.dg/anon-struct-11.c =================================================================== --- gcc.dg/anon-struct-11.c (Revision 218142) +++ gcc.dg/anon-struct-11.c (Arbeitskopie) @@ -3,6 +3,7 @@ /* No special options--in particular, turn off the default -pedantic-errors option. */ /* { dg-options "" } */ +/* { dg-additional-options "-fno-ms-extensions" { target *-*-mingw* } } */ /* When not using -fplan9-extensions, we don't support automatic conversion of pointer types, and we don't support referring to a Index: gcc.dg/anon-struct-2.c =================================================================== --- gcc.dg/anon-struct-2.c (Revision 218142) +++ gcc.dg/anon-struct-2.c (Arbeitskopie) @@ -1,4 +1,5 @@ /* { dg-options "-std=gnu89" } */ +/* { dg-additional-options "-fno-ms-extensions" { target *-*-mingw* } } */ /* In GNU C mode, we recognize the anonymous struct/union extension, but not Microsoft extensions. */ Index: gcc.dg/c11-anon-struct-2.c =================================================================== --- gcc.dg/c11-anon-struct-2.c (Revision 218142) +++ gcc.dg/c11-anon-struct-2.c (Arbeitskopie) @@ -2,6 +2,7 @@ cases. */ /* { dg-do compile } */ /* { dg-options "-std=c11 -pedantic-errors" } */ +/* { dg-additional-options "-fno-ms-extensions" { target *-*-mingw* } } */ typedef struct s0 { Index: gcc.dg/c11-anon-struct-3.c =================================================================== --- gcc.dg/c11-anon-struct-3.c (Revision 218142) +++ gcc.dg/c11-anon-struct-3.c (Arbeitskopie) @@ -2,6 +2,7 @@ cases: typedefs disallowed by N1549. */ /* { dg-do compile } */ /* { dg-options "-std=c11 -pedantic-errors" } */ +/* { dg-additional-options "-fno-ms-extensions" { target *-*-mingw* } } */ typedef struct {
Hi, this patch turns off ms-extensions for mingw-targets to match diagnostics checked in testcases. Ok for apply? Kai ChangeLog 2014-11-28 Kai Tietz <ktietz@redhat.com> * gcc.dg/anon-struct-1.c: * gcc.dg/anon-struct-11.c: * gcc.dg/anon-struct-2.c: * gcc.dg/c11-anon-struct-2.c: * gcc.dg/c11-anon-struct-3.c: