Page MenuHomePhabricator

aaron.ballman (Aaron Ballman)
User

Projects

User does not belong to any projects.

User Details

User Since
Mar 14 2013, 3:16 PM (480 w, 1 d)

Recent Activity

Yesterday

aaron.ballman added a comment to D120244: [clang][sema] Enable first-class bool support for C2x.

I'll roll back the #warning use and report back when that's done.

Thu, May 26, 11:58 AM · Restricted Project, Restricted Project
aaron.ballman committed rG6273b5cbcdd3: Roll back use of #warning for header deprecations (authored by aaron.ballman).
Roll back use of #warning for header deprecations
Thu, May 26, 11:52 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D120244: [clang][sema] Enable first-class bool support for C2x.

The system header including a header that's explicitly deprecated is dubious practice, but that does still require some amount of timing coordination.

I agree, and I'll be filing bug reports against system headers within my control that use <stdbool.h>. That is unfortunately not the majority of headers, though.

Thu, May 26, 11:36 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D120244: [clang][sema] Enable first-class bool support for C2x.

We could maybe extend #pragma clang deprecate to deprecate inclusion of the current file so that it acts sort of like a [[deprecated]] attribute that triggers on inclusion using typical diagnostics instead of #warning.

Thu, May 26, 11:12 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D120244: [clang][sema] Enable first-class bool support for C2x.

We've started having several internal user complaints because their system headers are suddenly triggering a warning for using <stdbool.h>. This seems to be caused by the fact that #warning is surfaced to users even when the #warning is present in a system header (which makes sense, because otherwise there would be no way to issue a #warning from a system header). This ends up causing problems because users have no way to suppress the warning in the system headers they use without also disabling deprecation warnings in their own code. Is this intended? Instead, it seems to me like what we'd want is some way to mark the header as deprecated such that Clang will not flag uses of <stdbool.h> from within system headers, but will flag them from user code. This would be consistent with how the deprecated attributes work for classes and functions.

Thoughts?

This is what _CLANG_DISABLE_CRT_DEPRECATION_WARNINGS is for. We documented it here: https://clang.llvm.org/docs/UsersManual.html#controlling-deprecation-diagnostics-in-clang-provided-c-runtime-headers but the basic idea is, if that's defined before including the header, we don't issue those warnings. Does that suffice?

Well, the problem is that a user that is using some system header like Foundation.h (which includes <stdbool.h>) will be forced to define _CLANG_DISABLE_CRT_DEPRECATION_WARNINGS, which has two drawbacks:

  1. It's annoying to them because they never used <stdbool.h> in their own code
  2. It turns off deprecation warnings in their own code as well
Thu, May 26, 10:27 AM · Restricted Project, Restricted Project
aaron.ballman added inline comments to D126061: [clang] [WIP] Reject non-declaration C++11 attributes on declarations.
Thu, May 26, 7:50 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D102122: Support warn_unused_result on typedefs.

Sorry, with all the layers in the previous messages I'm not sure what the summary is. Sounds like the summary is "this is OK to continue work in the direction of supporting [[clang::warn_unused_result]] (only that spelling) for typedefs in C and C++ (for compatibility with C/so that C headers, like LLVM C's APIs can still be parsed as C++), while considering some of the complications raised in the WG21 discussion"?

Thu, May 26, 6:07 AM · Restricted Project, Restricted Project
aaron.ballman committed rG605651135b4c: Fix failing test case with strict prototype changes (authored by aaron.ballman).
Fix failing test case with strict prototype changes
Thu, May 26, 5:47 AM · Restricted Project, Restricted Project
aaron.ballman committed rG681c50c62e93: Improve the strict prototype diagnostic behavior (authored by aaron.ballman).
Improve the strict prototype diagnostic behavior
Thu, May 26, 5:36 AM · Restricted Project, Restricted Project
aaron.ballman closed D125814: Improve the strict prototype diagnostic behavior.
Thu, May 26, 5:36 AM · Restricted Project, Restricted Project
aaron.ballman added inline comments to D125814: Improve the strict prototype diagnostic behavior.
Thu, May 26, 5:36 AM · Restricted Project, Restricted Project
aaron.ballman added inline comments to D126084: [Sema] Reject implicit conversions between different scoped enum types in list initialization.
Thu, May 26, 5:11 AM · Restricted Project, Restricted Project
aaron.ballman committed rG85750de685f5: Use the canonical type when matching a generic selection association (authored by aaron.ballman).
Use the canonical type when matching a generic selection association
Thu, May 26, 4:42 AM · Restricted Project, Restricted Project

Wed, May 25

aaron.ballman added a comment to D120244: [clang][sema] Enable first-class bool support for C2x.

We've started having several internal user complaints because their system headers are suddenly triggering a warning for using <stdbool.h>. This seems to be caused by the fact that #warning is surfaced to users even when the #warning is present in a system header (which makes sense, because otherwise there would be no way to issue a #warning from a system header). This ends up causing problems because users have no way to suppress the warning in the system headers they use without also disabling deprecation warnings in their own code. Is this intended? Instead, it seems to me like what we'd want is some way to mark the header as deprecated such that Clang will not flag uses of <stdbool.h> from within system headers, but will flag them from user code. This would be consistent with how the deprecated attributes work for classes and functions.

Thoughts?

Wed, May 25, 2:25 PM · Restricted Project, Restricted Project
aaron.ballman added a comment to D123235: [OpenMP] atomic compare fail : Parser & AST support.

Please revert the changes while investigating how to resolve the crashes though.

I just noticed that you needed someone to commit on your behalf for this, so I went ahead and did the revert myself in 69da3b6aead2e7a18a2578aad661d6d36b8d30cf.

Wed, May 25, 10:47 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
aaron.ballman committed rG9368bf9023ee: Removing this as part of the revert done in… (authored by aaron.ballman).
Removing this as part of the revert done in…
Wed, May 25, 10:47 AM · Restricted Project, Restricted Project
aaron.ballman accepted D123924: [clang-apply-replacements] Added an option to ignore insert conflict..

I think this has sat for long enough and my LGTM will have to suffice. :-)

Wed, May 25, 10:39 AM · Restricted Project, Restricted Project, Restricted Project
aaron.ballman added a comment to D123235: [OpenMP] atomic compare fail : Parser & AST support.

Please revert the changes while investigating how to resolve the crashes though.

Wed, May 25, 10:36 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
aaron.ballman added a reverting change for rG232bf8189ef7: [OpenMP] atomic compare fail : Parser & AST support: rG69da3b6aead2: Revert "[OpenMP] atomic compare fail : Parser & AST support".
Wed, May 25, 10:35 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
aaron.ballman committed rG69da3b6aead2: Revert "[OpenMP] atomic compare fail : Parser & AST support" (authored by aaron.ballman).
Revert "[OpenMP] atomic compare fail : Parser & AST support"
Wed, May 25, 10:35 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
aaron.ballman added a reverting change for D123235: [OpenMP] atomic compare fail : Parser & AST support: rG69da3b6aead2: Revert "[OpenMP] atomic compare fail : Parser & AST support".
Wed, May 25, 10:35 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
aaron.ballman added a reviewer for D126084: [Sema] Reject implicit conversions between different scoped enum types in list initialization: Restricted Project.

Please add more details to the summary and remove the rdar link (nobody outside of Apple can access that anyway). Also, this should have a release note for the bug fix.

Wed, May 25, 10:28 AM · Restricted Project, Restricted Project
aaron.ballman accepted D126170: C++ DR2394: Const-default-constructible for members..

LGTM (minor nit with release notes, take it or leave it at your discretion).

Wed, May 25, 10:04 AM · Restricted Project, Restricted Project
aaron.ballman added a reviewer for D126341: Order implicitly instantiated global variable's initializer by the reverse instantiation order: Restricted Project.

Adding the language WG as a reviewer in case others have opinions.

Wed, May 25, 9:54 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D123235: [OpenMP] atomic compare fail : Parser & AST support.

I am not sure why it is indicating an uninitialized variable at that point. The code at " llvm-project/clang/lib/Sema/SemaOpenMP.cpp:12060 " is as below:

Wed, May 25, 9:42 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
aaron.ballman added a comment to D125814: Improve the strict prototype diagnostic behavior.

Good improvement, but an additional change to wording for just the zero-arg non-prototype function declaration case could help a lot. The fact that zero-arg it's only being warned about because of the "...because of" note isn't particularly clear -- especially when the "because of" isn't emitted.

So, suggestion for zero-arg-specific warning text:

Wed, May 25, 9:26 AM · Restricted Project, Restricted Project
aaron.ballman updated the diff for D125814: Improve the strict prototype diagnostic behavior.

Update based on review feedback.

Wed, May 25, 9:26 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D126324: [clang] Allow const variables with weak attribute to be overridden.

The changes so far look sensible, but I think we should add some more tests for a few situations. 1) Using a const weak symbol as a valid initializer should be diagnosed (with a warning? with an error?) so users are alerted to the behavioral quirks. 2) Using a const weak symbol in a constant expression context should probably be an error, right? e.g.,

I do think we have already diagnose/fail on all the relevant cases here. And have tests for them, like

extern const int weak_int __attribute__((weak));
const int weak_int = 42;
int weak_int_test = weak_int; // expected-error {{not a compile-time constant}}

in clang/test/Sema/const-eval.c

But I'll have another look to make sure we have proper coverage.

Wed, May 25, 9:25 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D126324: [clang] Allow const variables with weak attribute to be overridden.

The changes so far look sensible, but I think we should add some more tests for a few situations. 1) Using a const weak symbol as a valid initializer should be diagnosed (with a warning? with an error?) so users are alerted to the behavioral quirks.

I don't feel this is really necessary.

Wed, May 25, 9:20 AM · Restricted Project, Restricted Project
aaron.ballman committed rGf96aa834d7d7: Test C DR conformance (part three of many) (authored by aaron.ballman).
Test C DR conformance (part three of many)
Wed, May 25, 5:18 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D102122: Support warn_unused_result on typedefs.

Thank you for the ping, this fell entirely off my radar!

Wed, May 25, 5:12 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D126324: [clang] Allow const variables with weak attribute to be overridden.

The changes so far look sensible, but I think we should add some more tests for a few situations. 1) Using a const weak symbol as a valid initializer should be diagnosed (with a warning? with an error?) so users are alerted to the behavioral quirks. 2) Using a const weak symbol in a constant expression context should probably be an error, right? e.g.,

const int hmm __attribute__((weak)) = 12;
int erp = hmm; // Error in C, dynamic init in C++?
Wed, May 25, 4:31 AM · Restricted Project, Restricted Project

Tue, May 24

aaron.ballman added inline comments to D126061: [clang] [WIP] Reject non-declaration C++11 attributes on declarations.
Tue, May 24, 1:29 PM · Restricted Project, Restricted Project
aaron.ballman added inline comments to D125622: [clang-tidy] Reject invalid enum initializers in C files.
Tue, May 24, 12:03 PM · Restricted Project, Restricted Project
aaron.ballman added a comment to D126061: [clang] [WIP] Reject non-declaration C++11 attributes on declarations.

Thank you for your work on this, I generally like this new approach and think we're heading in the right direction.

Tue, May 24, 10:12 AM · Restricted Project, Restricted Project
aaron.ballman accepted D126062: [clang] Don't parse MS attributes in `ParseExportDeclaration()`..

LGTM -- I checked as well and these changes are NFC. Thanks!

Tue, May 24, 7:02 AM · Restricted Project, Restricted Project
aaron.ballman added inline comments to D116593: Fix `performance-unnecessary-value-param` for template specialization.
Tue, May 24, 6:51 AM · Restricted Project, Restricted Project
aaron.ballman added inline comments to D126258: [Clang] Avoid misleading 'conflicting types' diagnostic with no-prototype decls..
Tue, May 24, 6:17 AM · Restricted Project, Restricted Project
aaron.ballman accepted D126138: [clang-tidy] Fix #55134 (regression introduced by 5da7c04).

LGTM, though this should have a release note for the fix.

Tue, May 24, 5:03 AM · Restricted Project, Restricted Project
aaron.ballman accepted D126258: [Clang] Avoid misleading 'conflicting types' diagnostic with no-prototype decls..

LGTM, thank you for catching this!

Tue, May 24, 5:00 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D125814: Improve the strict prototype diagnostic behavior.

Ping

Tue, May 24, 5:00 AM · Restricted Project, Restricted Project
aaron.ballman accepted D126093: Sema: adjust assertion to account for deduced types.

LGTM aside from a testing request and a nit. Thanks for the fix!

Tue, May 24, 4:20 AM · Restricted Project, Restricted Project

Mon, May 23

aaron.ballman added inline comments to D125555: [clang] Add __has_target_feature.
Mon, May 23, 10:55 AM · Restricted Project
aaron.ballman added a comment to D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang.

If it was adding a calling convention, sure - caution warranted. There's no llvm change here though, an existing CC is exposed to C++. No change to the type system either.

This is adding a user-facing calling convention to Clang and it changes the type system as a result. For example, lambda function pointer conversion operators sometimes are generated for each calling convention so that you can form a function pointer of the correct type (this might not be impacted by your change here); there's a specific number of bits for representing the enumeration of calling conventions and this uses one of those bits, etc.

It slightly changes the type system of C++ code in that the calling convention was previously only available in opencl / openmp etc. I was under the impression that the compiler data representation cost of calling conventions was in LLVM and thus pre-paid for the calling convention this gives access to. There's the enum CallingConv which has gained a field, I didn't realise that was input into something of limited bitwidth.

Mon, May 23, 10:40 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang.

If it was adding a calling convention, sure - caution warranted. There's no llvm change here though, an existing CC is exposed to C++. No change to the type system either.

Mon, May 23, 8:52 AM · Restricted Project, Restricted Project
aaron.ballman accepted D126134: [clang-tidy] Improve add_new_check.py to recognize more checks.

LGTM, thank you!

Mon, May 23, 8:22 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D126134: [clang-tidy] Improve add_new_check.py to recognize more checks.

Thanks for this! It generally LGTM (though my Python skills are not particularly awesome, so take it with a grain of salt). Just a few questions at this point.

Mon, May 23, 6:52 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D126186: [clang-tidy] Extend cert-oop57-cpp to check non-zero memset values.

As a side point I'm not sure this change really follows what the rule is trying to enforce. The rule is about not using std::memset to reinitialise objects that aren't trivial. Having said that limiting it to only 0 does seem a little restrictive,

Mon, May 23, 5:49 AM · Restricted Project, Restricted Project

Sun, May 22

aaron.ballman committed rG202a4fde2ba9: Test more C DR conformance (part two of many) (authored by aaron.ballman).
Test more C DR conformance (part two of many)
Sun, May 22, 10:38 AM · Restricted Project, Restricted Project

Sat, May 21

aaron.ballman added a comment to rGdf46fb40557a: Test C DR conformance (part one of many).
Sat, May 21, 1:12 PM · Restricted Project, Restricted Project
aaron.ballman committed rG36fde81f9360: Fix failing test bots from df46fb40557a14807dd508af32251ceb1cab8b86 (authored by aaron.ballman).
Fix failing test bots from df46fb40557a14807dd508af32251ceb1cab8b86
Sat, May 21, 1:11 PM · Restricted Project, Restricted Project
aaron.ballman committed rGdf46fb40557a: Test C DR conformance (part one of many) (authored by aaron.ballman).
Test C DR conformance (part one of many)
Sat, May 21, 12:03 PM · Restricted Project, Restricted Project

Fri, May 20

aaron.ballman committed rGade5b55af574: Add a page to track C defect report status (authored by aaron.ballman).
Add a page to track C defect report status
Fri, May 20, 1:08 PM · Restricted Project, Restricted Project
aaron.ballman added a comment to D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang.

The primary application is for more rapid debugging of the amdgpu backend by permuting a C or C++ test file instead of manually updating an IR file.

Given that this is adding a calling convention, which has significant impacts on our type system: is this use case important enough to steal a bit for this CC? This sounds *super* special case to me, but maybe it's a common need?

Fri, May 20, 6:51 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D125970: [amdgpu] Add amdgpu_kernel calling conv attribute to clang.

The primary application is for more rapid debugging of the amdgpu backend by permuting a C or C++ test file instead of manually updating an IR file.

Fri, May 20, 6:49 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D123667: [clang][AST] Fix crash getting name of a template decl.

This affects all released versions of llvm14, and potentially llvm13 too (I got a crash today on llvm13 hitting this exact code).
yet, this is not backported to llvm14 on the current 14.x github branch. Please backport so this is released on the next tarball.

I filed https://github.com/llvm/llvm-project/issues/55585 to see if we can get this into the 14.x branch, thanks for the request!

Fri, May 20, 4:32 AM · Restricted Project, Restricted Project

Thu, May 19

aaron.ballman committed rGd374b65f2da1: Drop qualifiers from return types in C (DR423) (authored by aaron.ballman).
Drop qualifiers from return types in C (DR423)
Thu, May 19, 10:07 AM · Restricted Project, Restricted Project
aaron.ballman closed D125919: Drop qualifiers from return types in C (DR423).
Thu, May 19, 10:07 AM · Restricted Project, Restricted Project
aaron.ballman accepted D125963: [Office Hours] add initial guidance for hosts.

LGTM, thank you!

Thu, May 19, 8:29 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D125919: Drop qualifiers from return types in C (DR423).

We definitely don't want ObjC to differ here; thanks for the CC.

Thu, May 19, 6:08 AM · Restricted Project, Restricted Project
aaron.ballman updated the diff for D125919: Drop qualifiers from return types in C (DR423).

Fixes failed AST matching unit test and speculatively fixes a CodeGen test, all found via precommit CI testing.

Thu, May 19, 6:08 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D125620: [Clang] Fix diagnostics formatting.

Thank you for this! Is there a way to add test coverage for this change? Also, this should come with a release note as it's fixing a bug someone reported.

Thu, May 19, 5:21 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D123667: [clang][AST] Fix crash getting name of a template decl.

This affects all released versions of llvm14, and potentially llvm13 too (I got a crash today on llvm13 hitting this exact code).
yet, this is not backported to llvm14 on the current 14.x github branch. Please backport so this is released on the next tarball.

Thu, May 19, 5:08 AM · Restricted Project, Restricted Project
aaron.ballman added inline comments to D125963: [Office Hours] add initial guidance for hosts.
Thu, May 19, 5:04 AM · Restricted Project, Restricted Project

Wed, May 18

aaron.ballman added reviewers for D125919: Drop qualifiers from return types in C (DR423): rjmccall, dexonsmith.

Adding some Apple reviewers because this touches ObjC behavior (I would be very surprised if ObjC wanted different rules here than C though).

Wed, May 18, 12:51 PM · Restricted Project, Restricted Project
aaron.ballman requested review of D125919: Drop qualifiers from return types in C (DR423).
Wed, May 18, 12:34 PM · Restricted Project, Restricted Project
aaron.ballman added a comment to D125259: [C11] Diagnose unreachable generic selection associations.

Oh wow, good catch! I'll correct this.

Oof, the plot thickens... the diagnostic is correct for some types, but is incorrect for others: https://godbolt.org/z/fahzx53W6. I also discovered a parsing issue in C++ where we get confused by elaborated type specifiers: https://godbolt.org/z/1ejxqd9ss.

https://reviews.llvm.org/D125882 addresses the diagnostic behavior for the unreachable warning.

Wed, May 18, 9:46 AM · Restricted Project, Restricted Project
aaron.ballman committed rG47b8424a533d: Correct the diagnostic behavior for unreachable _Generic associations in C++ (authored by aaron.ballman).
Correct the diagnostic behavior for unreachable _Generic associations in C++
Wed, May 18, 9:46 AM · Restricted Project, Restricted Project
aaron.ballman closed D125882: Correct the diagnostic behavior for unreachable _Generic associations in C++.
Wed, May 18, 9:46 AM · Restricted Project, Restricted Project
aaron.ballman added inline comments to D125882: Correct the diagnostic behavior for unreachable _Generic associations in C++.
Wed, May 18, 9:44 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D125259: [C11] Diagnose unreachable generic selection associations.

Oh wow, good catch! I'll correct this.

Oof, the plot thickens... the diagnostic is correct for some types, but is incorrect for others: https://godbolt.org/z/fahzx53W6. I also discovered a parsing issue in C++ where we get confused by elaborated type specifiers: https://godbolt.org/z/1ejxqd9ss.

Wed, May 18, 6:30 AM · Restricted Project, Restricted Project
aaron.ballman requested review of D125882: Correct the diagnostic behavior for unreachable _Generic associations in C++.
Wed, May 18, 6:30 AM · Restricted Project, Restricted Project
aaron.ballman added inline comments to D125802: Fix std::has_unique_object_representations for _BitInt types with padding bits.
Wed, May 18, 5:56 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D125259: [C11] Diagnose unreachable generic selection associations.

I'm questioning the benefit to supporting _Generic in C++ and starting to wonder if perhaps we should pull this extension back.

I imagine it may be used in shared system headers. E.g., implementations of tgmath.h that don't use the builtins.

Wed, May 18, 4:46 AM · Restricted Project, Restricted Project
aaron.ballman retitled D125814: Improve the strict prototype diagnostic behavior from Fix strict prototype diagnostic wording for definitions to Improve the strict prototype diagnostic behavior.
Wed, May 18, 4:38 AM · Restricted Project, Restricted Project
aaron.ballman updated the diff for D125814: Improve the strict prototype diagnostic behavior.

Went with a more simple implementation approach and the diagnostic results seem to be a mild improvement in terms of wording and behavior.

Wed, May 18, 4:33 AM · Restricted Project, Restricted Project

Tue, May 17

aaron.ballman added a comment to D125814: Improve the strict prototype diagnostic behavior.

This confuses me.

Looking at behavior with default flags:

We won't emit a -Wdeprecated-non-prototype warning for int foo();, until we subsequently find int foo(int arg) { return 5; }.

Tue, May 17, 12:35 PM · Restricted Project, Restricted Project
aaron.ballman updated subscribers of D125259: [C11] Diagnose unreachable generic selection associations.

Oh wow, good catch! I'll correct this.

Tue, May 17, 11:10 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D125259: [C11] Diagnose unreachable generic selection associations.
$ cat /tmp/a.cc
typedef struct Test {
} Test;

void f() {
  const Test constVal;
  _Generic(constVal, const Test : 0);
}
$ ./build/rel/bin/clang -fsyntax-only  -x c++ /tmp/a.cc -Wall
/tmp/a.cc:6:28: warning: due to lvalue conversion of the controlling expression, association of type 'const Test' will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
  _Generic(constVal, const Test : 0);
                           ^
/tmp/a.cc:6:35: warning: expression result unused [-Wunused-value]
  _Generic(constVal, const Test : 0);
                                  ^
2 warnings generated.
$ ./build/rel/bin/clang -fsyntax-only  -x c /tmp/a.cc -Wall
/tmp/a.cc:6:28: warning: due to lvalue conversion of the controlling expression, association of type 'const Test' (aka 'const struct Test') will never be selected because it is qualified [-Wunreachable-code-generic-assoc]
  _Generic(constVal, const Test : 0);
                           ^
/tmp/a.cc:6:12: error: controlling expression type 'Test' (aka 'struct Test') not compatible with any generic association type
  _Generic(constVal, const Test : 0);
           ^~~~~~~~
1 warning and 1 error generated.

the C++ case looks wrong, it's warning that const Test can't be selected then selects it

Tue, May 17, 10:27 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D122895: [C89/C2x] Improve diagnostics around strict prototypes in C.

The warnings for this case aren't great:

int foo();

int
foo(int arg) {
  return 5;
}

Yeah, that's not ideal, I'm looking into it to see if I can improve that scenario or not.

There's not much to be done about the strict prototype warning for the declaration; the issue is that we need to give the strict prototypes warning when forming the function *type* for the declaration, which is long before we have any idea there's a subsequent definition (also, because this is for forming the type, we don't know what declarations, if any, may be related, so there's no real way to improve the fix-it behavior). However, I think the second warning is just bad wording -- we know we have a function definition with a prototype for this particular diagnostic. However, there's the redeclaration case to consider at the same time. So here's the full test and the best behavior that I can come up with so far:

void foo();
void foo(int arg);

void bar();
void bar(int arg) {}

With -Wstrict-prototypes enabled:

F:\source\llvm-project>llvm\out\build\x64-Debug\bin\clang.exe -fsyntax-only -Wstrict-prototypes "C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c"
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:1:9: warning: a function declaration without a prototype
      is deprecated in all versions of C [-Wstrict-prototypes]
void foo();
        ^
         void
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:2:6: warning: this function declaration with a prototype
      will change behavior in C2x because of a previous declaration with no prototype [-Wdeprecated-non-prototype]
void foo(int arg);
     ^
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:4:9: warning: a function declaration without a prototype
      is deprecated in all versions of C [-Wstrict-prototypes]
void bar();
        ^
         void
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:5:6: warning: this function definition with a prototype
      will change behavior in C2x because of a previous declaration with no prototype [-Wdeprecated-non-prototype]
void bar(int arg) {}
     ^
4 warnings generated.

With -Wstrict-prototypes disabled:

F:\source\llvm-project>llvm\out\build\x64-Debug\bin\clang.exe -fsyntax-only "C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c"
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:1:6: warning: a function declaration without a prototype
      is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
void foo();
     ^
         void
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:2:6: warning: this function declaration with a prototype
      will change behavior in C2x because of a previous declaration with no prototype [-Wdeprecated-non-prototype]
void foo(int arg);
     ^
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:4:6: warning: a function declaration without a prototype
      is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
void bar();
     ^
         void
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:5:6: warning: this function definition with a prototype
      will change behavior in C2x because of a previous declaration with no prototype [-Wdeprecated-non-prototype]
void bar(int arg) {}
     ^
4 warnings generated.

It's not ideal, but it's about the most workable solution I can come up with that doesn't regress far more important cases. It'd be nice if we had a note instead of leaning on the previous warning like we're doing, but I still claim this is defensible given how rare the situation is that you'd declare without a prototype and later redeclare (perhaps through a definition) with a non-void prototype. Note (it's easy to miss with the wall of text), the difference between the two runs is that when strict prototypes are enabled, we issue the strict prototype warning on the first declaration and when strict prototypes are disabled, we issue the "is not supported in C2x" diagnostic on the first declaration -- but in either case the intention is to alert the user to which previous declaration has no prototype for the subsequent declaration/definition that caused the issue.

this function declaration|definition with a prototype will change behavior in C2x because of a previous declaration with no prototype is the new diagnostic wording I've got so far, and I'm not strongly tied to it, if you have suggestions to improve it. I would also be happy with this function declaration|definition with a prototype is not supported in C2x because of a previous declaration with no prototype or something along those lines.

Tue, May 17, 10:23 AM · Restricted Project, Restricted Project
aaron.ballman requested review of D125814: Improve the strict prototype diagnostic behavior.
Tue, May 17, 10:22 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D124446: [clang-tidy] Add the misc-discarded-return-value check.

Btw, I'm in C standards meetings all week this week, so my reviews are going to be delayed by a bit, just as an FYI.

Tue, May 17, 5:55 AM · Restricted Project, Restricted Project
aaron.ballman added inline comments to D124446: [clang-tidy] Add the misc-discarded-return-value check.
Tue, May 17, 5:54 AM · Restricted Project, Restricted Project
aaron.ballman accepted D124977: [NFC][Clang] Modify expect of fail test or XFAIL because CSKY align is different.

LGTM!

Tue, May 17, 4:14 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D125727: [clang] Avoid suggesting typoed directives in `.S` files.

I know it's retroactive, but this also LGTM. Thank you for the fix!

Tue, May 17, 4:12 AM · Restricted Project, Restricted Project

Mon, May 16

aaron.ballman added inline comments to D125622: [clang-tidy] Reject invalid enum initializers in C files.
Mon, May 16, 10:20 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D124726: Suggest typoed directives in preprocessor conditionals.

I see an instance of this warning in the Linux kernel due to the "Now, for unknown directives inside a skipped conditional block, we diagnose the unknown directive as a warning if it is sufficiently similar to a directive specific to preprocessor conditional blocks" part of this change:

arch/x86/crypto/aesni-intel_asm.S:532:8: warning: invalid preprocessing directive, did you mean '#if'? [-Wunknown-directives]
                                        # in in order to perform
                                          ^
arch/x86/crypto/aesni-intel_asm.S:547:8: warning: invalid preprocessing directive, did you mean '#if'? [-Wunknown-directives]
                                        # in in order to perform
                                          ^
2 warnings generated.
Mon, May 16, 10:17 AM · Restricted Project, Restricted Project, Restricted Project
aaron.ballman added inline comments to D124446: [clang-tidy] Add the misc-discarded-return-value check.
Mon, May 16, 6:17 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D125555: [clang] Add __has_target_feature.

It's not clear to me why existing facilities shouldn't be extended to cover this case rather than coming up with another feature testing macro. There's already plenty of confusion for users to decide between __has_feature and __has_extension, and now we're talking about adding a third choice to the mix.

Mon, May 16, 5:35 AM · Restricted Project
aaron.ballman accepted D125026: [clang-tidy][NFC] Reimplement SimplifyBooleanExpr with RecursiveASTVisitors.

LGTM, thank you for this!

Mon, May 16, 5:26 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D125622: [clang-tidy] Reject invalid enum initializers in C files.

Note, I'm in C standards committee meetings all week this week, so I expect I won't be doing many reviews this week and I'll be catching up as best I can starting next week.

Mon, May 16, 4:48 AM · Restricted Project, Restricted Project
aaron.ballman added inline comments to D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum.
Mon, May 16, 4:33 AM · Restricted Project, Restricted Project

Fri, May 13

aaron.ballman added a comment to D122895: [C89/C2x] Improve diagnostics around strict prototypes in C.

I think the current behavior today makes sense but we should see if we can improve it to make *more* sense. With -Wstrict-prototypes, we should complain about the block without a prototype, but the use at the call site is not declaring a conflicting declaration nor is it a call to a function without a prototype but passes arguments (both of those are -Wdeprecated-non-prototype warnings). Instead, it's a new kind of situation that we may want to consider adding additional coverage for under -Wdeprecated-non-prototype based on the same reasoning we used for diagnosing calls to a function without a prototype but pass arguments. This is not specific to blocks, consider:

void func(void (*fp)());
void do_it(int i);

int main(void) {
  func(do_it); // It'd be nice to diagnose this too, but we don't today
}
Fri, May 13, 12:09 PM · Restricted Project, Restricted Project
aaron.ballman added a comment to D122895: [C89/C2x] Improve diagnostics around strict prototypes in C.
void f1(void (^block)());

void f2(void) {
  f1(^(int x) { /* do something with x */ });
}

Your code example is interesting though as I would expect that to trigger -Wdeprecated-non-prototype as well as -Wstrict-prototypes. I'd expect the pedantic warning to complain about the block declaration because it specifies no prototype, and I'd expect the changes behavior warning because that code will break in C2x.

Fri, May 13, 11:37 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D122895: [C89/C2x] Improve diagnostics around strict prototypes in C.

The warnings for this case aren't great:

int foo();

int
foo(int arg) {
  return 5;
}

Yeah, that's not ideal, I'm looking into it to see if I can improve that scenario or not.

Fri, May 13, 11:24 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D122895: [C89/C2x] Improve diagnostics around strict prototypes in C.

Previously, -Wstrict-prototypes was useful for preventing actual bugs in code.

For example, it's important to have a warning that catches code like this:

void f1(void (^block)());

void f2(void) {
  f1(^(int x) { /* do something with x */ });
}

without triggering in cases that are pedantic.

(It also seems unfortunate to regress the false positive rate of this diagnostic before -fstrict-prototypes is available.)

Fri, May 13, 9:10 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D122895: [C89/C2x] Improve diagnostics around strict prototypes in C.

The warnings for this case aren't great:

int foo();

int
foo(int arg) {
  return 5;
}
Fri, May 13, 7:43 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D122895: [C89/C2x] Improve diagnostics around strict prototypes in C.

Sure, I'm all for adding a new warning for users that want a pedantic warning. Can it be put behind a separate flag, such as -Wstrict-prototypes-pedantic, which isn't triggered by -Wstrict-prototypes?

Previously, -Wstrict-prototypes was useful for preventing actual bugs in code.

Fri, May 13, 7:35 AM · Restricted Project, Restricted Project
aaron.ballman committed rGd364307542d1: Remove a stale FIXME comment; NFC (authored by aaron.ballman).
Remove a stale FIXME comment; NFC
Fri, May 13, 7:24 AM · Restricted Project, Restricted Project
aaron.ballman added inline comments to rG4be105c98a9c: Silence some false positive -Wstrict-prototype warnings.
Fri, May 13, 7:22 AM · Restricted Project, Restricted Project