Re: RFR: 8325163: Enable -Wpedantic on clang
On Fri, 2 Feb 2024 15:22:03 GMT, Magnus Ihse Bursie wrote: > Inspired by (the later backed-out) > [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to > enable `-Wpedantic` for clang. This has already found some irregularities in > the code, like mistakenly using `#import` instead of `#include`. In this > patch, I disable warnings for these individual buggy or badly written files, > but I intend to post follow-up issues on the respective teams to have them > properly fixed. > > Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since > individual warnings in `-Wpedantic` cannot be disabled. This means that code > like this: > > > #define DEBUG_ONLY(code) code; > > DEBUG_ONLY(foo()); > > > will result in a `; ;`. This breaks the C standard, but is benign, and we use > it all over the place. On clang, we can ignore this by `-Wno-extra-semi`, but > this is not available on gcc. Marked as reviewed by erikj (Reviewer). Looks like GHA found some more issues. - PR Review: https://git.openjdk.org/jdk/pull/17687#pullrequestreview-1860130881 PR Review: https://git.openjdk.org/jdk/pull/17687#pullrequestreview-1860132459
Re: RFR: 8325163: Enable -Wpedantic on clang
On Fri, 2 Feb 2024 15:22:03 GMT, Magnus Ihse Bursie wrote: > Inspired by (the later backed-out) > [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to > enable `-Wpedantic` for clang. This has already found some irregularities in > the code, like mistakenly using `#import` instead of `#include`. In this > patch, I disable warnings for these individual buggy or badly written files, > but I intend to post follow-up issues on the respective teams to have them > properly fixed. > > Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since > individual warnings in `-Wpedantic` cannot be disabled. This means that code > like this: > > > #define DEBUG_ONLY(code) code; > > DEBUG_ONLY(foo()); > > > will result in a `; ;`. This breaks the C standard, but is benign, and we use > it all over the place. On clang, we can ignore this by `-Wno-extra-semi`, but > this is not available on gcc. Ah, dtrace triggers `dollar-in-identifier-extension`. Of course... *sigh* I actually had this disabled for hotspot on an earlier version of the patch (that had lied dormant in my personal fork for about a year), but I could not reproduce the problem it was supposed to solve now, so I removed it again. Now I know why I had it. - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1925848001
Re: RFR: 8325163: Enable -Wpedantic on clang
On Fri, 2 Feb 2024 15:59:56 GMT, Julian Waters wrote: > Guess I could work on the gcc counterpart and find a way around the inability > to disable -Wpedantic with it in tandem with this change... I don't think that is possible. The double semicolon rule can only be disabled by disabling pedantic completely. This is not the first time we've run into trouble because gcc do not have fine-grained enough warnings. :( (While clang has always excelled in this area.) - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1925954578
Re: RFR: 8325163: Enable -Wpedantic on clang
On Fri, 2 Feb 2024 15:22:03 GMT, Magnus Ihse Bursie wrote: > #define DEBUG_ONLY(code) code; > > DEBUG_ONLY(foo()); > ``` > > will result in a `; ;`. This breaks the C standard, but is benign, and we use > it all over the place. On clang, we can ignore this by `-Wno-extra-semi`, but > this is not available on gcc. All of the -Wpedantic warnings for extra ';' in C++ code I looked at look to me like a gcc bug (or bugs). C++11 added "empty-declaration", which is permitted anywhere a normal declaration is permitted (C++14 7), as well as for class member declarations (C++14 9.2). https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96068 seems to have fixed some, but not all, cases. It looks like it only removed the warnings for empty declarations at namespace scope. I couldn't find anything for other cases, including empty class member declarations. I consider the "format '%p' expects argument of type 'void*" warnings to be not at all helpful. Fortunately we don't use '%p' in HotSpot, but I don't know how other groups might feel having this inflicted on them. Because of the vast numbers of those, I ran out of patience trying to find and examine other warnings triggered by this option. Based on that, I'm not feeling very keen on this change, at least not for a future version applying to gcc builds. - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1926154325
Re: RFR: 8325163: Enable -Wpedantic on clang
On Fri, 2 Feb 2024 15:22:03 GMT, Magnus Ihse Bursie wrote: > Inspired by (the later backed-out) > [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to > enable `-Wpedantic` for clang. This has already found some irregularities in > the code, like mistakenly using `#import` instead of `#include`. In this > patch, I disable warnings for these individual buggy or badly written files, > but I intend to post follow-up issues on the respective teams to have them > properly fixed. Rather than first turning on pedantic warnings and then (maybe) going back and perhaps fixing things, I'd really prefer things be done in the other order. (That's how I handled the recent `-Wparentheses` changes, for example.) - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1926164327
Re: RFR: 8325163: Enable -Wpedantic on clang
On Mon, 5 Feb 2024 03:07:43 GMT, Kim Barrett wrote: > I consider the "format '%p' expects argument of type 'void*" warnings to be > not at all helpful. Fortunately we don't use '%p' in HotSpot, We do use it in hotspot. Not a huge amount as we have the legacy format specifiers for PTR_FORMAT etc. - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1926298655
Re: RFR: 8325163: Enable -Wpedantic on clang
On Sun, 4 Feb 2024 22:52:19 GMT, Magnus Ihse Bursie wrote: > > Guess I could work on the gcc counterpart and find a way around the > > inability to disable -Wpedantic with it in tandem with this change... > > I don't think that is possible. The double semicolon rule can only be > disabled by disabling pedantic completely. This is not the first time we've > run into trouble because gcc do not have fine-grained enough warnings. :( > (While clang has always excelled in this area.) I tried compiling this on both Linux and Windows, and it seems like there are only a few places where the double semicolons are actually a problem (The strangest case was in a NOT_LP64 macro in mallocHeader.hpp where gcc exploded when parsing a perfectly valid semicolon). This is more of a code style cleanliness check than anything, but as Kim said, it is probably a bug within gcc somewhere. I'll file a gcc bug for this in the meantime - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1926339574
Re: RFR: 8325163: Enable -Wpedantic on clang
On Fri, 2 Feb 2024 15:22:03 GMT, Magnus Ihse Bursie wrote: > Inspired by (the later backed-out) > [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to > enable `-Wpedantic` for clang. This has already found some irregularities in > the code, like mistakenly using `#import` instead of `#include`. In this > patch, I disable warnings for these individual buggy or badly written files, > but I intend to post follow-up issues on the respective teams to have them > properly fixed. > > Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since > individual warnings in `-Wpedantic` cannot be disabled. This means that code > like this: > > > #define DEBUG_ONLY(code) code; > > DEBUG_ONLY(foo()); > > > will result in a `; ;`. This breaks the C standard, but is benign, and we use > it all over the place. On clang, we can ignore this by `-Wno-extra-semi`, but > this is not available on gcc. Created https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113760 Interestingly a gcc maintainer cannot reproduce the issue at all - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1926361490
Re: RFR: 8325163: Enable -Wpedantic on clang
On Mon, 5 Feb 2024 03:21:35 GMT, Kim Barrett wrote:
>> Inspired by (the later backed-out)
>> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to
>> enable `-Wpedantic` for clang. This has already found some irregularities in
>> the code, like mistakenly using `#import` instead of `#include`. In this
>> patch, I disable warnings for these individual buggy or badly written files,
>> but I intend to post follow-up issues on the respective teams to have them
>> properly fixed.
>>
>> Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since
>> individual warnings in `-Wpedantic` cannot be disabled. This means that code
>> like this:
>>
>>
>> #define DEBUG_ONLY(code) code;
>>
>> DEBUG_ONLY(foo());
>>
>>
>> will result in a `; ;`. This breaks the C standard, but is benign, and we
>> use it all over the place. On clang, we can ignore this by
>> `-Wno-extra-semi`, but this is not available on gcc.
>
>> Inspired by (the later backed-out)
>> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to
>> enable `-Wpedantic` for clang. This has already found some irregularities in
>> the code, like mistakenly using `#import` instead of `#include`. In this
>> patch, I disable warnings for these individual buggy or badly written files,
>> but I intend to post follow-up issues on the respective teams to have them
>> properly fixed.
>
> Rather than first turning on pedantic warnings and then (maybe) going back
> and perhaps fixing things, I'd really prefer
> things be done in the other order. (That's how I handled the recent
> `-Wparentheses` changes, for example.)
@kimbarrett quoting the gcc maintainers
> Yes because the C++ defect report was only for `Spurious semicolons at
> namespace scope should be allowed`. See
> https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#569 .
>
>
> ```
> struct f
> {
> int t; ;
> };
> ```
>
> Is not allowed by the C++ standard currently and is a GCC extension, maybe it
> should have a seperate flag to control that but I am not 100% sure.
-
PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1926372614
Re: RFR: 8325163: Enable -Wpedantic on clang
On Mon, 5 Feb 2024 03:21:35 GMT, Kim Barrett wrote: >> Inspired by (the later backed-out) >> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to >> enable `-Wpedantic` for clang. This has already found some irregularities in >> the code, like mistakenly using `#import` instead of `#include`. In this >> patch, I disable warnings for these individual buggy or badly written files, >> but I intend to post follow-up issues on the respective teams to have them >> properly fixed. >> >> Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since >> individual warnings in `-Wpedantic` cannot be disabled. This means that code >> like this: >> >> >> #define DEBUG_ONLY(code) code; >> >> DEBUG_ONLY(foo()); >> >> >> will result in a `; ;`. This breaks the C standard, but is benign, and we >> use it all over the place. On clang, we can ignore this by >> `-Wno-extra-semi`, but this is not available on gcc. > >> Inspired by (the later backed-out) >> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to >> enable `-Wpedantic` for clang. This has already found some irregularities in >> the code, like mistakenly using `#import` instead of `#include`. In this >> patch, I disable warnings for these individual buggy or badly written files, >> but I intend to post follow-up issues on the respective teams to have them >> properly fixed. > > Rather than first turning on pedantic warnings and then (maybe) going back > and perhaps fixing things, I'd really prefer > things be done in the other order. (That's how I handled the recent > `-Wparentheses` changes, for example.) @kimbarrett > Rather than first turning on pedantic warnings and then (maybe) going back > and perhaps fixing things, I'd really prefer things be done in the other order. I hear what you are saying, but I respectfully disagree. If we do things in the order you suggest, the global flag cannot be turned on until all component teams have fixed their source code. That essentially makes the most overworked group putting an effective veto to this change (when your workload is too high, fixing warnings is not on top of your priority.) In contrast, if the global warning can be turned on now, it will have a positive effect on all new and modified code from now on. And the teams can work on their individual warnings, and remove them in their own time. This is the same method I used for turning on -Wall and -Wextra. Some teams are very eager to fix warnings, and others still have almost all their "DISABLED_WARNINGS" left several years later. (I will not mention any names; you both know who you are ;-)). If I had followed the route you suggest, we would not have -Wall -Wextra on all source code (sans a few, marked files) now. - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1926553386
Re: RFR: 8325163: Enable -Wpedantic on clang
On Mon, 5 Feb 2024 03:07:43 GMT, Kim Barrett wrote: > at least not for a future version applying to gcc builds. @kimbarrett @TheShermanTanker Please do not drag gcc into this PR. This is just about clang. Unless gcc makes a serious effort to shape up their inferior warning handling, I don't think we will ever be able to do something similar for gcc. But that is not a reason to avoid doing it on clang. In general, we have tried to utilize the strength of every compiler, regardless of if all the other compilers could detect the same problem or not. - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1926559951
Re: RFR: 8325163: Enable -Wpedantic on clang
On Fri, 2 Feb 2024 15:22:03 GMT, Magnus Ihse Bursie wrote: > Inspired by (the later backed-out) > [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to > enable `-Wpedantic` for clang. This has already found some irregularities in > the code, like mistakenly using `#import` instead of `#include`. In this > patch, I disable warnings for these individual buggy or badly written files, > but I intend to post follow-up issues on the respective teams to have them > properly fixed. > > Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since > individual warnings in `-Wpedantic` cannot be disabled. This means that code > like this: > > > #define DEBUG_ONLY(code) code; > > DEBUG_ONLY(foo()); > > > will result in a `; ;`. This breaks the C standard, but is benign, and we use > it all over the place. On clang, we can ignore this by `-Wno-extra-semi`, but > this is not available on gcc. Also, a general note about the parts of `pedantic` that is globally disabled in this PR (like `extra-semi` and `format-pedantic`). It might very well be that the offending code is restricted to a few places (if these places are in commonly included header files, I was forced to disable the warning globally), and that it can be rather easily fixed. All the better! But there is still no reason to do that *before* checking in this PR. Let's just get the minimum bar raised first. Then we can adress the issue of fixing the code to get rid of the exceptions for `extra-semi` and `format-pedantic`, if it is possible and if we want it. The latter is not at all clear; the rest of the list of globally disabled warnings (like `unused-parameter`) are warnings that we have agreed is useless, and only unfortunately dragged in with a catch-all flag like `-Wall` or `-Wextra`. From my PoV, the same could be said about `extra-semi`. - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1926569091
Re: RFR: 8325163: Enable -Wpedantic on clang
On Mon, 5 Feb 2024 09:32:09 GMT, Magnus Ihse Bursie wrote: > > at least not for a future version applying to gcc builds. > > @kimbarrett @TheShermanTanker Please do not drag gcc into this PR. This is > just about clang. Unless gcc makes a serious effort to shape up their > inferior warning handling, I don't think we will ever be able to do something > similar for gcc. > > But that is not a reason to avoid doing it on clang. In general, we have > tried to utilize the strength of every compiler, regardless of if all the > other compilers could detect the same problem or not. Hey, I never said anything about gcc compatibility being a blocker for this changeset :) (I do have however a couple a questions which are listed above) - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1926642510
Re: RFR: 8325163: Enable -Wpedantic on clang
On Fri, 2 Feb 2024 15:22:03 GMT, Magnus Ihse Bursie wrote: > Inspired by (the later backed-out) > [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to > enable `-Wpedantic` for clang. This has already found some irregularities in > the code, like mistakenly using `#import` instead of `#include`. In this > patch, I disable warnings for these individual buggy or badly written files, > but I intend to post follow-up issues on the respective teams to have them > properly fixed. > > Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since > individual warnings in `-Wpedantic` cannot be disabled. This means that code > like this: > > > #define DEBUG_ONLY(code) code; > > DEBUG_ONLY(foo()); > > > will result in a `; ;`. This breaks the C standard, but is benign, and we use > it all over the place. On clang, we can ignore this by `-Wno-extra-semi`, but > this is not available on gcc. I am sorry, but all I can see is: > Just a few questions... and then your comment ends. And I can't find any other comment with a list of questions. - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1926693945
Re: RFR: 8325163: Enable -Wpedantic on clang
On Mon, 5 Feb 2024 10:44:59 GMT, Magnus Ihse Bursie wrote: > I am sorry, but all I can see is: > > > Just a few questions... > > and then your comment ends. And I can't find any other comment with a list of > questions. Eh? Aren't they in the code review section? But in any case: > Shouldn't this be -pedantic -Wpedantic, and wouldn't this be better > positioned at where HotSpot currently sets -permissive- for Microsoft Visual > C++ (In other words, TOOLCHAIN_CFLAGS_JVM and TOOLCHAIN_CFLAGS_JDK)? The 2 > options are not the same, at least on gcc, and I'm unsure if the same is true > for clang The other concern I had was that there are a ton of disabled warnings added by this change, but I guess that's already been answered by that other reply - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1926701211
Re: RFR: 8325163: Enable -Wpedantic on clang
On Mon, 5 Feb 2024 10:49:07 GMT, Julian Waters wrote: > Shouldn't this be -pedantic -Wpedantic, and wouldn't this be better > positioned at where HotSpot currently sets -permissive- for Microsoft Visual > C++ (In other words, TOOLCHAIN_CFLAGS_JVM and TOOLCHAIN_CFLAGS_JDK)? The 2 > options are not the same, at least on gcc, and I'm unsure if the same is true > for clang (It is really weird, I cannot find that original comment anywhere in the Github PR :-() As far as I have been able to determine, -Wpedantic and -pedantic are aliases on gcc > 5, and the same is true for clang. -Wpedantic seems to be the preferred way nowadays. `TOOLCHAIN_CFLAGS_JVM` is arguably if not wrong so at least not the best place to put `-permissive-`. `-Wpermissive` belongs with `-Wall -Wextra`. - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1926861360
Re: RFR: 8325163: Enable -Wpedantic on clang
On Mon, 5 Feb 2024 10:49:07 GMT, Julian Waters wrote: > The other concern I had was that there are a ton of disabled warnings added > by this change, but I guess that's already been answered by that other reply Just to be clear: these warnings have never been turned on. They are implicitly turned on by `-Wpedantic`; and this works fine with most files, but this improved scrutiny catches issues in some files. My idea is to file bugs on these individual disabled warnings, to have the actual problem fixed, but as follow up to this PR. - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1926867260
Re: RFR: 8325163: Enable -Wpedantic on clang
On Mon, 5 Feb 2024 12:15:50 GMT, Magnus Ihse Bursie wrote: > > Shouldn't this be -pedantic -Wpedantic, and wouldn't this be better > > positioned at where HotSpot currently sets -permissive- for Microsoft > > Visual C++ (In other words, TOOLCHAIN_CFLAGS_JVM and TOOLCHAIN_CFLAGS_JDK)? > > The 2 options are not the same, at least on gcc, and I'm unsure if the same > > is true for clang > > (It is really weird, I cannot find that original comment anywhere in the > Github PR :-() > > As far as I have been able to determine, -Wpedantic and -pedantic are aliases > on gcc > 5, and the same is true for clang. -Wpedantic seems to be the > preferred way nowadays. > > `TOOLCHAIN_CFLAGS_JVM` is arguably if not wrong so at least not the best > place to put `-permissive-`. `-Wpermissive` belongs with `-Wall -Wextra`. TOOLCHAIN_CFLAGS_JVM and TOOLCHAIN_CFLAGS_JDK have, until now, been where the -Zc: conformance options are passed, which is why I recommended adding them there, to match what Visual C++ does at the moment. Maybe I have it the other way around, and it is Visual C++ that needs to be changed to match clang (Both for permissive- and -Zc: options) - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1926925644
Re: RFR: 8325163: Enable -Wpedantic on clang
On Mon, 5 Feb 2024 03:21:35 GMT, Kim Barrett wrote:
>> Inspired by (the later backed-out)
>> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to
>> enable `-Wpedantic` for clang. This has already found some irregularities in
>> the code, like mistakenly using `#import` instead of `#include`. In this
>> patch, I disable warnings for these individual buggy or badly written files,
>> but I intend to post follow-up issues on the respective teams to have them
>> properly fixed.
>>
>> Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since
>> individual warnings in `-Wpedantic` cannot be disabled. This means that code
>> like this:
>>
>>
>> #define DEBUG_ONLY(code) code;
>>
>> DEBUG_ONLY(foo());
>>
>>
>> will result in a `; ;`. This breaks the C standard, but is benign, and we
>> use it all over the place. On clang, we can ignore this by
>> `-Wno-extra-semi`, but this is not available on gcc.
>
>> Inspired by (the later backed-out)
>> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to
>> enable `-Wpedantic` for clang. This has already found some irregularities in
>> the code, like mistakenly using `#import` instead of `#include`. In this
>> patch, I disable warnings for these individual buggy or badly written files,
>> but I intend to post follow-up issues on the respective teams to have them
>> properly fixed.
>
> Rather than first turning on pedantic warnings and then (maybe) going back
> and perhaps fixing things, I'd really prefer
> things be done in the other order. (That's how I handled the recent
> `-Wparentheses` changes, for example.)
> @kimbarrett quoting the gcc maintainers
>
> > Yes because the C++ defect report was only for `Spurious semicolons at
> > namespace scope should be allowed`. See
> > https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#569 .
> > ```
> > struct f
> > {
> > int t; ;
> > };
> > ```
> >
> > Is not allowed by the C++ standard currently and is a GCC extension, maybe
> > it should have a seperate flag to control that but I am not 100% sure.
That's incorrect, and I've replied in the gcc bug. C++14 added
"empty-declaration" to "member-declaration" (C++ 9.2).
-
PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1927189316
Re: RFR: 8325163: Enable -Wpedantic on clang
> On Feb 5, 2024, at 4:31 AM, Magnus Ihse Bursie wrote: > > On Mon, 5 Feb 2024 03:21:35 GMT, Kim Barrett wrote: > >>> Inspired by (the later backed-out) >>> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to >>> enable `-Wpedantic` for clang. This has already found some irregularities >>> in the code, like mistakenly using `#import` instead of `#include`. In this >>> patch, I disable warnings for these individual buggy or badly written >>> files, but I intend to post follow-up issues on the respective teams to >>> have them properly fixed. >>> >>> Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since >>> individual warnings in `-Wpedantic` cannot be disabled. This means that >>> code like this: >>> >>> >>> #define DEBUG_ONLY(code) code; >>> >>> DEBUG_ONLY(foo()); >>> >>> >>> will result in a `; ;`. This breaks the C standard, but is benign, and we >>> use it all over the place. On clang, we can ignore this by >>> `-Wno-extra-semi`, but this is not available on gcc. >> >>> Inspired by (the later backed-out) >>> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to >>> enable `-Wpedantic` for clang. This has already found some irregularities >>> in the code, like mistakenly using `#import` instead of `#include`. In this >>> patch, I disable warnings for these individual buggy or badly written >>> files, but I intend to post follow-up issues on the respective teams to >>> have them properly fixed. >> >> Rather than first turning on pedantic warnings and then (maybe) going back >> and perhaps fixing things, I'd really prefer >> things be done in the other order. (That's how I handled the recent >> `-Wparentheses` changes, for example.) > > @kimbarrett >> Rather than first turning on pedantic warnings and then (maybe) going back >> and perhaps fixing things, I'd really prefer > things be done in the other order. > > I hear what you are saying, but I respectfully disagree. If we do things in > the order you suggest, the global flag cannot be turned on until all > component teams have fixed their source code. That essentially makes the most > overworked group putting an effective veto to this change (when your workload > is too high, fixing warnings is not on top of your priority.) In contrast, if > the global warning can be turned on now, it will have a positive effect on > all new and modified code from now on. And the teams can work on their > individual warnings, and remove them in their own time. Without knowing what the actual warnings are that are being triggered and then suppressed, I can't even begin to evaluate this change. Not all warnings are good and useful. Sometimes the avoidance effort is really not worth the benefit. Sometimes there is a future change to the Standard that is already supported as an extension. Sometimes there is a widely supported extension that has perhaps just not yet made it into the Standard. Sometimes platform-specific code uses platform-specific extensions. And so on. Enabling -Wpedantic requires an evaluation of the costs and benefits and a decision that there's a good tradeoff there. So far, this PR doesn't do that. Fixing the warnings first, or at least having the relevant bug reports, would provide that information. > This is the same method I used for turning on -Wall and -Wextra. Some teams > are very eager to fix warnings, and others still have almost all their > "DISABLED_WARNINGS" left several years later. (I will not mention any names; > you both know who you are ;-)). If I had followed the route you suggest, we > would not have -Wall -Wextra on all source code (sans a few, marked files) > now. For -Wparentheses I took the opposite approach and fixed all occurrences (finding and fixing a couple of bugs in the process) before enabling them. We've also been approaching -Wconversion from that direction. I think the enabled but then suppressed warnings has led to an out-of-sight out-of-mind situation for the suppressed warnings. I was not particularly happy with adding -Wextra, for example, because I think some of the warnings it triggers are questionable. You and I went through a very similar discussion for enabling that option for HotSpot. Right now I don't even have the information needed for such a discussion. signature.asc Description: Message signed with OpenPGP
Re: RFR: 8325163: Enable -Wpedantic on clang
On Mon, 5 Feb 2024 06:15:08 GMT, David Holmes wrote: > > I consider the "format '%p' expects argument of type 'void*" warnings to be > > not at all helpful. Fortunately we don't use '%p' in HotSpot, > > We do use it in hotspot. Not a huge amount as we have the legacy format > specifiers for PTR_FORMAT etc. You are correct, we do use "%p" a fair amount (107 lines today). I thought we didn't, because we were instead supposed to use INTPTR_FORMAT and the (currently?) equivalent PTR_FORMAT. Those macros aren't legacy, they are to provide consistent output across platforms. "%p" provides implementation defined output. For example, if I remember correctly, gcc "%p" prints "(null)" for nullptr, with no mechanism (such as a conversion flag) to control that. If you are printing a table of pointers, and you expect only numeric values because you are going to be processing the table or copying it into a spreadsheet or the like, that gcc-specific behavior isn't very helpful. Instead, we're "supposed" to use the `p2i` function and PTR_FORMAT for printing pointers, to get the same output on all platforms. That idiom also avoids the not very helpful -Wpedantic warnings. - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1927288508
Re: RFR: 8325163: Enable -Wpedantic on clang
On Mon, 5 Feb 2024 15:42:40 GMT, Kim Barrett wrote:
>>> I consider the "format '%p' expects argument of type 'void*" warnings to be
>>> not at all helpful. Fortunately we don't use '%p' in HotSpot,
>>
>> We do use it in hotspot. Not a huge amount as we have the legacy format
>> specifiers for PTR_FORMAT etc.
>
>> > I consider the "format '%p' expects argument of type 'void*" warnings to
>> > be not at all helpful. Fortunately we don't use '%p' in HotSpot,
>>
>> We do use it in hotspot. Not a huge amount as we have the legacy format
>> specifiers for PTR_FORMAT etc.
>
> You are correct, we do use "%p" a fair amount (107 lines today).
>
> I thought we didn't, because we were instead supposed to use INTPTR_FORMAT and
> the (currently?) equivalent PTR_FORMAT. Those macros aren't legacy, they are
> to provide consistent output across platforms. "%p" provides implementation
> defined output. For example, if I remember correctly, gcc "%p" prints "(null)"
> for nullptr, with no mechanism (such as a conversion flag) to control that. If
> you are printing a table of pointers, and you expect only numeric values
> because you are going to be processing the table or copying it into a
> spreadsheet or the like, that gcc-specific behavior isn't very helpful.
>
> Instead, we're "supposed" to use the `p2i` function and PTR_FORMAT for
> printing pointers, to get the same output on all platforms. That idiom also
> avoids the not very helpful -Wpedantic warnings.
> > @kimbarrett quoting the gcc maintainers
> > > Yes because the C++ defect report was only for `Spurious semicolons at
> > > namespace scope should be allowed`. See
> > > https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#569 .
> > > ```
> > > struct f
> > > {
> > > int t; ;
> > > };
> > > ```
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > Is not allowed by the C++ standard currently and is a GCC extension,
> > > maybe it should have a seperate flag to control that but I am not 100%
> > > sure.
>
> That's incorrect, and I've replied in the gcc bug. C++14 added
> "empty-declaration" to "member-declaration" (C++ 9.2).
With my additional information the gcc bug has now been confirmed.
-
PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1927299264
Re: RFR: 8325163: Enable -Wpedantic on clang
On Mon, 5 Feb 2024 15:42:40 GMT, Kim Barrett wrote: > I thought we didn't, because we were instead supposed to use INTPTR_FORMAT and the (currently?) equivalent PTR_FORMAT. Those macros aren't legacy, they are to provide consistent output across platforms. "%p" provides implementation defined output. My understanding/recollection was that those issues with `%p` had gone away and so we could start using it. - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1928814503
Re: RFR: 8325163: Enable -Wpedantic on clang
On Fri, 2 Feb 2024 15:22:03 GMT, Magnus Ihse Bursie wrote: > Inspired by (the later backed-out) > [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to > enable `-Wpedantic` for clang. This has already found some irregularities in > the code, like mistakenly using `#import` instead of `#include`. In this > patch, I disable warnings for these individual buggy or badly written files, > but I intend to post follow-up issues on the respective teams to have them > properly fixed. > > Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since > individual warnings in `-Wpedantic` cannot be disabled. This means that code > like this: > > > #define DEBUG_ONLY(code) code; > > DEBUG_ONLY(foo()); > > > will result in a `; ;`. This breaks the C standard, but is benign, and we use > it all over the place. On clang, we can ignore this by `-Wno-extra-semi`, but > this is not available on gcc. +1 for this, anything to help Standard conformance is great! Just a few questions... - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1924155329
Re: RFR: 8325163: Enable -Wpedantic on clang
On Fri, 2 Feb 2024 15:22:03 GMT, Magnus Ihse Bursie wrote: > Inspired by (the later backed-out) > [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to > enable `-Wpedantic` for clang. This has already found some irregularities in > the code, like mistakenly using `#import` instead of `#include`. In this > patch, I disable warnings for these individual buggy or badly written files, > but I intend to post follow-up issues on the respective teams to have them > properly fixed. > > Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since > individual warnings in `-Wpedantic` cannot be disabled. This means that code > like this: > > > #define DEBUG_ONLY(code) code; > > DEBUG_ONLY(foo()); > > > will result in a `; ;`. This breaks the C standard, but is benign, and we use > it all over the place. On clang, we can ignore this by `-Wno-extra-semi`, but > this is not available on gcc. Guess I could work on the gcc counterpart and find a way around the inability to disable -Wpedantic with it in tandem with this change... - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1924163226
Re: RFR: 8325163: Enable -Wpedantic on clang [v2]
> Inspired by (the later backed-out) > [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to > enable `-Wpedantic` for clang. This has already found some irregularities in > the code, like mistakenly using `#import` instead of `#include`. In this > patch, I disable warnings for these individual buggy or badly written files, > but I intend to post follow-up issues on the respective teams to have them > properly fixed. > > Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since > individual warnings in `-Wpedantic` cannot be disabled. This means that code > like this: > > > #define DEBUG_ONLY(code) code; > > DEBUG_ONLY(foo()); > > > will result in a `; ;`. This breaks the C standard, but is benign, and we use > it all over the place. On clang, we can ignore this by `-Wno-extra-semi`, but > this is not available on gcc. Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision: FIx dtrace build - Changes: - all: https://git.openjdk.org/jdk/pull/17687/files - new: https://git.openjdk.org/jdk/pull/17687/files/4de3c446..ffa70af6 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17687&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17687&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17687.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17687/head:pull/17687 PR: https://git.openjdk.org/jdk/pull/17687
Re: RFR: 8325163: Enable -Wpedantic on clang [v2]
On Mon, 5 Feb 2024 10:58:17 GMT, Magnus Ihse Bursie wrote: >> Inspired by (the later backed-out) >> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to >> enable `-Wpedantic` for clang. This has already found some irregularities in >> the code, like mistakenly using `#import` instead of `#include`. In this >> patch, I disable warnings for these individual buggy or badly written files, >> but I intend to post follow-up issues on the respective teams to have them >> properly fixed. >> >> Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since >> individual warnings in `-Wpedantic` cannot be disabled. This means that code >> like this: >> >> >> #define DEBUG_ONLY(code) code; >> >> DEBUG_ONLY(foo()); >> >> >> will result in a `; ;`. This breaks the C standard, but is benign, and we >> use it all over the place. On clang, we can ignore this by >> `-Wno-extra-semi`, but this is not available on gcc. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > FIx dtrace build The following is all that I was able to find, on the latest gcc docs. Nothing came up for clang for the search term "-pedantic vs -Wpedantic" > -pedantic-errors > Give an error whenever the base standard (see -Wpedantic) requires a > diagnostic, in some cases where there is undefined behavior at compile-time > and in some other cases that do not prevent compilation of programs that are > valid according to the standard. This is not equivalent to -Werror=pedantic: > the latter option is unlikely to be useful, as it only makes errors of the > diagnostics that are controlled by -Wpedantic, whereas this option also > affects required diagnostics that are always enabled or controlled by options > other than -Wpedantic. - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1926946873
Re: RFR: 8325163: Enable -Wpedantic on clang [v2]
On Mon, 5 Feb 2024 10:58:17 GMT, Magnus Ihse Bursie wrote: >> Inspired by (the later backed-out) >> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to >> enable `-Wpedantic` for clang. This has already found some irregularities in >> the code, like mistakenly using `#import` instead of `#include`. In this >> patch, I disable warnings for these individual buggy or badly written files, >> but I intend to post follow-up issues on the respective teams to have them >> properly fixed. >> >> Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since >> individual warnings in `-Wpedantic` cannot be disabled. This means that code >> like this: >> >> >> #define DEBUG_ONLY(code) code; >> >> DEBUG_ONLY(foo()); >> >> >> will result in a `; ;`. This breaks the C standard, but is benign, and we >> use it all over the place. On clang, we can ignore this by >> `-Wno-extra-semi`, but this is not available on gcc. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > FIx dtrace build The split in different categories of flags is perhaps not perfect, and more a heuristic to help us manage them. However, I think we are talking about two different categories here. The `/Z` flags change the behavior of the compiler to be more standards compliant (similar to `-std=c++1` on gcc/clang). The `-Wpedantic" flag enables a batch of additional warnings on top of `-Wall` and `-Wextra`. The only way this makes it more "standards compliant" is that the standard requires the compiler to produce a warning in these situations. But that is not really why we want to turn it on; we want it to be able to catch bad or suboptimal code. - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1927025804
Re: RFR: 8325163: Enable -Wpedantic on clang [v2]
On Mon, 5 Feb 2024 10:58:17 GMT, Magnus Ihse Bursie wrote: >> Inspired by (the later backed-out) >> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to >> enable `-Wpedantic` for clang. This has already found some irregularities in >> the code, like mistakenly using `#import` instead of `#include`. In this >> patch, I disable warnings for these individual buggy or badly written files, >> but I intend to post follow-up issues on the respective teams to have them >> properly fixed. >> >> Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since >> individual warnings in `-Wpedantic` cannot be disabled. This means that code >> like this: >> >> >> #define DEBUG_ONLY(code) code; >> >> DEBUG_ONLY(foo()); >> >> >> will result in a `; ;`. This breaks the C standard, but is benign, and we >> use it all over the place. On clang, we can ignore this by >> `-Wno-extra-semi`, but this is not available on gcc. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > FIx dtrace build I can't seem to find the post now but there was one mentioning about needing information on what warnings -Wpedantic enables, and well... It's quite a lot: https://clang.llvm.org/docs/DiagnosticsReference.html#wpedantic I also have Jonathan Wakely's confirmation that -pedantic and -Wpedantic aren't the same, but this may not be as relevant to the discussion at hand - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1927325037
Re: RFR: 8325163: Enable -Wpedantic on clang [v2]
On Mon, 5 Feb 2024 10:58:17 GMT, Magnus Ihse Bursie wrote: >> Inspired by (the later backed-out) >> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to >> enable `-Wpedantic` for clang. This has already found some irregularities in >> the code, like mistakenly using `#import` instead of `#include`. In this >> patch, I disable warnings for these individual buggy or badly written files, >> but I intend to post follow-up issues on the respective teams to have them >> properly fixed. >> >> Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since >> individual warnings in `-Wpedantic` cannot be disabled. This means that code >> like this: >> >> >> #define DEBUG_ONLY(code) code; >> >> DEBUG_ONLY(foo()); >> >> >> will result in a `; ;`. This breaks the C standard, but is benign, and we >> use it all over the place. On clang, we can ignore this by >> `-Wno-extra-semi`, but this is not available on gcc. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > FIx dtrace build This PR triggered a lot of discussion about C++ standards, gcc behavior, and possible breaches of coding standards in Hotspot -- but not a lot of discussion about the actual content of the PR. Is there anything in this proposed PR that you gentlemen disagree with or object to? Or is this fine to push as a step in our ongoing pursuit of increasing the code quality, that can (and will) be followed by many more? - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1927365995
Re: RFR: 8325163: Enable -Wpedantic on clang [v2]
On Mon, 5 Feb 2024 10:58:17 GMT, Magnus Ihse Bursie wrote: >> Inspired by (the later backed-out) >> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to >> enable `-Wpedantic` for clang. This has already found some irregularities in >> the code, like mistakenly using `#import` instead of `#include`. In this >> patch, I disable warnings for these individual buggy or badly written files, >> but I intend to post follow-up issues on the respective teams to have them >> properly fixed. >> >> Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since >> individual warnings in `-Wpedantic` cannot be disabled. This means that code >> like this: >> >> >> #define DEBUG_ONLY(code) code; >> >> DEBUG_ONLY(foo()); >> >> >> will result in a `; ;`. This breaks the C standard, but is benign, and we >> use it all over the place. On clang, we can ignore this by >> `-Wno-extra-semi`, but this is not available on gcc. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > FIx dtrace build No objections from me, though I still think -pedantic -Wpedantic should be placed where Visual Studio currently places -permissive- for HotSpot - Marked as reviewed by jwaters (Committer). PR Review: https://git.openjdk.org/jdk/pull/17687#pullrequestreview-1863251939
Re: RFR: 8325163: Enable -Wpedantic on clang [v2]
On Mon, 5 Feb 2024 10:58:17 GMT, Magnus Ihse Bursie wrote: >> Inspired by (the later backed-out) >> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to >> enable `-Wpedantic` for clang. This has already found some irregularities in >> the code, like mistakenly using `#import` instead of `#include`. In this >> patch, I disable warnings for these individual buggy or badly written files, >> but I intend to post follow-up issues on the respective teams to have them >> properly fixed. >> >> Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since >> individual warnings in `-Wpedantic` cannot be disabled. This means that code >> like this: >> >> >> #define DEBUG_ONLY(code) code; >> >> DEBUG_ONLY(foo()); >> >> >> will result in a `; ;`. This breaks the C standard, but is benign, and we >> use it all over the place. On clang, we can ignore this by >> `-Wno-extra-semi`, but this is not available on gcc. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > FIx dtrace build Here is the full list: https://clang.llvm.org/docs/DiagnosticsReference.html#wpedantic - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1928141600
Re: RFR: 8325163: Enable -Wpedantic on clang [v2]
On Mon, 5 Feb 2024 16:19:59 GMT, Magnus Ihse Bursie wrote: > Is there anything in this proposed PR that you gentlemen disagree with or > object to? Or is this fine to push as a step in our ongoing pursuit of > increasing the code quality, that can (and will) be followed by many more? Yes. As I said earlier: "Without knowing what the actual warnings are that are being triggered and then suppressed, I can't even begin to evaluate this change." - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1928006901
Re: RFR: 8325163: Enable -Wpedantic on clang [v2]
On Mon, 5 Feb 2024 10:58:17 GMT, Magnus Ihse Bursie wrote: >> Inspired by (the later backed-out) >> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to >> enable `-Wpedantic` for clang. This has already found some irregularities in >> the code, like mistakenly using `#import` instead of `#include`. In this >> patch, I disable warnings for these individual buggy or badly written files, >> but I intend to post follow-up issues on the respective teams to have them >> properly fixed. >> >> Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since >> individual warnings in `-Wpedantic` cannot be disabled. This means that code >> like this: >> >> >> #define DEBUG_ONLY(code) code; >> >> DEBUG_ONLY(foo()); >> >> >> will result in a `; ;`. This breaks the C standard, but is benign, and we >> use it all over the place. On clang, we can ignore this by >> `-Wno-extra-semi`, but this is not available on gcc. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > FIx dtrace build I think given the warnings have been enabled globally, but disabled locally where they cause build failures, and JBS issues will be filed for each of those special cases, then this is a reasonable change to make. It is then up to component teams to fix code where applicable, and enable disabled warnings in the future. - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1928824757
Re: RFR: 8325163: Enable -Wpedantic on clang [v2]
On Mon, 5 Feb 2024 21:39:06 GMT, Magnus Ihse Bursie wrote: > Here is the full list: > https://clang.llvm.org/docs/DiagnosticsReference.html#wpedantic I know about that list, and that's not what I was asking for. I want to understand the impact on *our* code. What warnings are arising from *our* code, how difficult are the fixes, and how beneficial are the fixes. Looking through the list of warnings it can generate, I don't see much that looks useful. I'm inclined to agree with Andrew Haley's comment in (draft) https://github.com/openjdk/jdk/pull/17727#issuecomment-1929178213. It might be that there are some specific warning options that are part of it that might be worth turning on (or working toward), but it's mostly uncompelling. The two that had bazillions of occurrences when I tried with gcc were (1) a gcc bug (incorrect warning about extra semis), and (2) a completely non-useful requirement that "%p" format spec only accepts exactly void*, with no implicit conversions from other pointer types. Clang presumably doesn't have the former, and I don't know about the latter. - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-1948881656
Re: RFR: 8325163: Enable -Wpedantic on clang [v2]
On Mon, 5 Feb 2024 10:58:17 GMT, Magnus Ihse Bursie wrote: >> Inspired by (the later backed-out) >> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to >> enable `-Wpedantic` for clang. This has already found some irregularities in >> the code, like mistakenly using `#import` instead of `#include`. In this >> patch, I disable warnings for these individual buggy or badly written files, >> but I intend to post follow-up issues on the respective teams to have them >> properly fixed. >> >> Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since >> individual warnings in `-Wpedantic` cannot be disabled. This means that code >> like this: >> >> >> #define DEBUG_ONLY(code) code; >> >> DEBUG_ONLY(foo()); >> >> >> will result in a `; ;`. This breaks the C standard, but is benign, and we >> use it all over the place. On clang, we can ignore this by >> `-Wno-extra-semi`, but this is not available on gcc. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > FIx dtrace build OK, although it would have been nice if you could have summarised what -pedantic and vla-extension are, so I didn't have to go and do research. - Marked as reviewed by prr (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17687#pullrequestreview-1886164910
Re: RFR: 8325163: Enable -Wpedantic on clang [v2]
On Mon, 5 Feb 2024 10:58:17 GMT, Magnus Ihse Bursie wrote: >> Inspired by (the later backed-out) >> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to >> enable `-Wpedantic` for clang. This has already found some irregularities in >> the code, like mistakenly using `#import` instead of `#include`. In this >> patch, I disable warnings for these individual buggy or badly written files, >> but I intend to post follow-up issues on the respective teams to have them >> properly fixed. >> >> Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since >> individual warnings in `-Wpedantic` cannot be disabled. This means that code >> like this: >> >> >> #define DEBUG_ONLY(code) code; >> >> DEBUG_ONLY(foo()); >> >> >> will result in a `; ;`. This breaks the C standard, but is benign, and we >> use it all over the place. On clang, we can ignore this by >> `-Wno-extra-semi`, but this is not available on gcc. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > FIx dtrace build Keep it for a bit more, bot. - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-2000843635
Re: RFR: 8325163: Enable -Wpedantic on clang [v2]
On Mon, 5 Feb 2024 10:58:17 GMT, Magnus Ihse Bursie wrote: >> Inspired by (the later backed-out) >> [JDK-8296115](https://bugs.openjdk.org/browse/JDK-8296115), I propose to >> enable `-Wpedantic` for clang. This has already found some irregularities in >> the code, like mistakenly using `#import` instead of `#include`. In this >> patch, I disable warnings for these individual buggy or badly written files, >> but I intend to post follow-up issues on the respective teams to have them >> properly fixed. >> >> Unfortunately, it is not possible to enable `-Wpedantic` on gcc, since >> individual warnings in `-Wpedantic` cannot be disabled. This means that code >> like this: >> >> >> #define DEBUG_ONLY(code) code; >> >> DEBUG_ONLY(foo()); >> >> >> will result in a `; ;`. This breaks the C standard, but is benign, and we >> use it all over the place. On clang, we can ignore this by >> `-Wno-extra-semi`, but this is not available on gcc. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > FIx dtrace build I'll close this now. I guess if we want to make progress on this, we will probably have to evaluate every individual warning separately, and add those which does make sense (e.g. `import-preprocessor-directive-pedantic`), and ignore the rest. I personally will not have time nor interest in driving this forward, at least not right now. - PR Comment: https://git.openjdk.org/jdk/pull/17687#issuecomment-2056659089
