[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-05-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3484189 , @rjmccall wrote:

> IIRC, the reason it works it that way is that "warnings which default to an 
> error" are really "errors which you can explicitly downgrade to a warning".  
> Maybe those ought to be different categories, or maybe we ought to just be 
> telling people to downgrade this specific diagnostic instead of generally 
> using `-Wno-error`.

Thanks for the background John! That seems defensible, but I think we're still 
in a confused state that we should do something about. A warning which default 
to an error is not sufficiently warning-like to be disabled via `-w` but isn't 
sufficiently error-like to be disabled via `-Wno-error`. IMO, one or the other 
of those options should work with a diagnostic like this.

However, for the time being, the advice is to use 
`-Wno-error=implicit-function-declaration`, which is what's documented in the 
release note. The commit message had a shorthand suggesting -Wno-error, but my 
intent was to suggest people narrowly disable the error instead of broadly 
disable them all.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-30 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

IIRC, the reason it works it that way is that "warnings which default to an 
error" are really "errors which you can explicitly downgrade to a warning".  
Maybe those ought to be different categories, or maybe we ought to just be 
telling people to downgrade this specific diagnostic instead of generally using 
`-Wno-error`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-30 Thread Manoj Gupta via Phabricator via cfe-commits
manojgupta added a comment.

Hmm, the commit message says that Wno-error should work but this is not really 
the case :(.

> (they can disable the warning or use -Wno-error to downgrade the
> error


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3484064 , @manojgupta 
wrote:

> We are finding a lot of failures in our ToT builds with this change. here is 
> an example for a configure script:
>
> $ cat tent.c
> int  main ()
> {
>  tgetent(0,0);
>  return 0;
> }
> $ bin/clang -c tent.c -Wno-error
> tent.c:3:2: error: call to undeclared function 'tgetent'; ISO C99 and later 
> do not support implicit function declarations 
> [-Wimplicit-function-declaration]
>  tgetent(0,0);
>  ^
> 1 error generated.
>
> It feels very surprising that Wno-error does not suppress this warning. Is 
> that expected?

Yes and no. Warnings which default to an error have very surprising behavior 
(at least to me) in terms of `-Werror` and `-w`. Specifying `-Wno-error` 
doesn't downgrade these diagnostics into a warning 
(https://godbolt.org/z/c43zTqTj1) and specifying `-w` doesn't disable the 
warning (https://godbolt.org/z/Y8YYYecTd). As you can see, that behavior is not 
specific to this patch but is just a general behavior with these kinds of 
diagnostics. I also find the behavior surprising (in both cases), but the last 
time I asked around about it, it sounded like this behavior was intentional for 
some reasons. More exploration is needed to know whether we should make any 
changes there.

In the meantime, `-Wno-error=implicit-function-declaration` should downgrade 
the error to a warning for you (but as you noticed, you can't later re-upgrade 
it into an error; so yes, these things behave a bit strangely IMO).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-30 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

In D122983#3484064 , @manojgupta 
wrote:

> We are finding a lot of failures in our ToT builds with this change. here is 
> an example for a configure script:
>
> $ cat tent.c
> int  main ()
> {
>  tgetent(0,0);
>  return 0;
> }
> $ bin/clang -c tent.c -Wno-error
> tent.c:3:2: error: call to undeclared function 'tgetent'; ISO C99 and later 
> do not support implicit function declarations 
> [-Wimplicit-function-declaration]
>  tgetent(0,0);
>  ^
> 1 error generated.
>
> It feels very surprising that Wno-error does not suppress this warning. Is 
> that expected?

Yéah, this is unfortunate if -Wno-error does nothing :/ other options from 
release notes work for you?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-30 Thread Manoj Gupta via Phabricator via cfe-commits
manojgupta added a comment.

We are finding a lot of failures in our ToT builds with this change. here is an 
example for a configure script:

$ cat tent.c
int  main ()
{
 tgetent(0,0);
 return 0;
}
$ bin/clang -c tent.c -Wno-error
tent.c:3:2: error: call to undeclared function 'tgetent'; ISO C99 and later do 
not support implicit function declarations [-Wimplicit-function-declaration]
 tgetent(0,0);
 ^
1 error generated.

It feels very surprising that Wno-error does not suppress this warning. Is that 
expected?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-30 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman marked an inline comment as done.
aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:15319-15321
   // OpenCL v2.0 s6.9.u - Implicit function declaration is not supported.
   else if (getLangOpts().OpenCL)
 diag_id = diag::err_opencl_implicit_function_decl;

aaron.ballman wrote:
> rsmith wrote:
> > Should we even be calling this function in OpenCL mode? It seems a bit 
> > inconsistent that we avoid calling this in C++ and C2x mode, and that we 
> > call it but error in OpenCL mode.
> > 
> > Maybe there should be a function on `LangOptions` to ask if implicit 
> > function declarations are permitted in the current language mode, to make 
> > it easy to opt out the right cases? (Happy for this to be a follow-on 
> > change if you agree.)
> I agree that it does seem inconsistent. I can look into making that change in 
> a follow-up.
I've fixed that up in a9d68a5524dea113cace5983697786599cbdce9a, thanks for the 
suggestion!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Thank you @nemanjai and @Jake-Egan for the help, I really appreciate it! I've 
disabled the diagnostics from another test (it looks like I caught the other 
ones earlier today), so hopefully the bot goes back to green shortly. If not, 
I'll keep on it. :-)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-21 Thread Jake Egan via Phabricator via cfe-commits
Jake-Egan added a comment.

@aaron.ballman Here are all the errors. Thanks!

  MultiSource/Applications/d/write_ctables.c:600:6: error: call to undeclared 
library function 'strncasecmp' with type 'int (const char *, const char *, 
unsigned long)'; ISO C99 and later do not support implicit function 
declarations [-Wimplicit-function-declaration]
  MultiSource/Benchmarks/Prolangs-C/bison/conflicts.c:266:7: error: call to 
undeclared function 'bcopy'; ISO C99 and later do not support implicit function 
declarations [-Wimplicit-function-declaration]
  MultiSource/Benchmarks/Prolangs-C/bison/symtab.c:65:3: error: call to 
undeclared library function 'strcpy' with type 'char *(char *, const char *)'; 
ISO C99 and later do not support implicit function declarations 
[-Wimplicit-function-declaration]
  MultiSource/Benchmarks/Prolangs-C/bison/symtab.c:91:11: error: call to 
undeclared library function 'strcmp' with type 'int (const char *, const char 
*)'; ISO C99 and later do not support implicit function declarations 
[-Wimplicit-function-declaration]
  MultiSource/Benchmarks/Ptrdist/anagram/anagram.c:317:5: error: call to 
undeclared library function 'bzero' with type 'void (void *, unsigned long)'; 
ISO C99 and later do not support implicit function declarations 
[-Wimplicit-function-declaration]
  MultiSource/Benchmarks/Ptrdist/yacr2/maze.c:316:5: error: call to undeclared 
library function 'bzero' with type 'void (void *, unsigned long)'; ISO C99 and 
later do not support implicit function declarations 
[-Wimplicit-function-declaration]
  MultiSource/Benchmarks/nbench/nbench1.c:2920:1: error: call to undeclared 
library function 'bzero' with type 'void (void *, unsigned long)'; ISO C99 and 
later do not support implicit function declarations 
[-Wimplicit-function-declaration]
  MultiSource/Benchmarks/MiBench/network-patricia/patricia.c:146:5: error: call 
to undeclared function 'bcopy'; ISO C99 and later do not support implicit 
function declarations [-Wimplicit-function-declaration]
  MultiSource/Benchmarks/MiBench/network-patricia/patricia.c:150:5: error: call 
to undeclared function 'bcopy'; ISO C99 and later do not support implicit 
function declarations [-Wimplicit-function-declaration]
  MultiSource/Benchmarks/MiBench/network-patricia/patricia.c:156:4: error: call 
to undeclared function 'bcopy'; ISO C99 and later do not support implicit 
function declarations [-Wimplicit-function-declaration]
  MultiSource/Benchmarks/MiBench/network-patricia/patricia.c:297:4: error: call 
to undeclared function 'bcopy'; ISO C99 and later do not support implicit 
function declarations [-Wimplicit-function-declaration]
  MultiSource/Benchmarks/MiBench/network-patricia/patricia_test.c:100:2: error: 
call to undeclared library function 'bzero' with type 'void (void *, unsigned 
long)'; ISO C99 and later do not support implicit function declarations 
[-Wimplicit-function-declaration]
  SingleSource/UnitTests/2005-05-11-Popcount-ffs-fls.c:110:69: error: call to 
undeclared function 'ffsl'; ISO C99 and later do not support implicit function 
declarations [-Wimplicit-function-declaration]


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-21 Thread Nemanja Ivanovic via Phabricator via cfe-commits
nemanjai added a comment.

In D122983#3465122 , @aaron.ballman 
wrote:

> In D122983#3465099 , @nemanjai 
> wrote:
>
>> @daltenty Can you please run this with the same config as the bot on one of 
>> our AIX machines but be sure to pass `-i` if it's `make` or `-k 0` if it's 
>> `ninja`?
>
> That would be *awesome*! I have to take off for a few hours, but I'm happy to 
> fix all of the issues you spot in one go when I get back in a bit. Thank you!

Actually, I had a build ready on AIX and ran it. Looks like just 2 more:

  test-suite :: 
MultiSource/Benchmarks/MiBench/network-patricia/network-patricia.test
  test-suite :: MultiSource/Benchmarks/nbench/nbench.test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3465099 , @nemanjai wrote:

> @daltenty Can you please run this with the same config as the bot on one of 
> our AIX machines but be sure to pass `-i` if it's `make` or `-k 0` if it's 
> `ninja`?

That would be *awesome*! I have to take off for a few hours, but I'm happy to 
fix all of the issues you spot in one go when I get back in a bit. Thank you!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-21 Thread Nemanja Ivanovic via Phabricator via cfe-commits
nemanjai added a comment.

@daltenty Can you please run this with the same config as the bot on one of our 
AIX machines but be sure to pass `-i` if it's `make` or `-k 0` if it's `ninja`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3465052 , @daltenty wrote:

> In D122983#3464759 , @aaron.ballman 
> wrote:
>
>> In D122983#3464724 , @Jake-Egan 
>> wrote:
>>
>>> Hi, we also have a failure on AIX with test-suite `call to undeclared 
>>> library function '%0' with type %1; ISO C99 and later`  
>>> https://lab.llvm.org/buildbot/#/builders/214/builds/825/steps/9/logs/stdio
>>
>> Thanks for letting me know, this should now be fixed.
>
> Looks like there are more test-suite case breaking beyond the one you fixed:
>
> https://lab.llvm.org/buildbot/#/builders/214/builds/842
>
> I suspect there will be more with the scope of this change unfortunately.

I agree there will be more -- it's incredibly frustrating that the AIX bot runs 
the test-suite but stops testing after the first failure. It makes it really 
hard to maintain. :-(

I've been keeping my eye on that bot and have been fixing up the tests as they 
get noticed, but if someone who has access to AIX wanted to tell me all of the 
test failures so I could hit them all at once (or disable the warnings in 
CMakeLists.txt and Makefile), that'd be greatly appreciated!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-21 Thread David Tenty via Phabricator via cfe-commits
daltenty added a comment.

In D122983#3464759 , @aaron.ballman 
wrote:

> In D122983#3464724 , @Jake-Egan 
> wrote:
>
>> Hi, we also have a failure on AIX with test-suite `call to undeclared 
>> library function '%0' with type %1; ISO C99 and later`  
>> https://lab.llvm.org/buildbot/#/builders/214/builds/825/steps/9/logs/stdio
>
> Thanks for letting me know, this should now be fixed.

Looks like there are more test-suite case breaking beyond the one you fixed:

https://lab.llvm.org/buildbot/#/builders/214/builds/842

I suspect there will be more with the scope of this change unfortunately.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3464724 , @Jake-Egan wrote:

> Hi, we also have a failure on AIX with test-suite `call to undeclared library 
> function '%0' with type %1; ISO C99 and later`  
> https://lab.llvm.org/buildbot/#/builders/214/builds/825/steps/9/logs/stdio

Thanks for letting me know, this should now be fixed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-21 Thread Jake Egan via Phabricator via cfe-commits
Jake-Egan added a comment.

Hi, we also have a failure on AIX with test-suite `call to undeclared library 
function '%0' with type %1; ISO C99 and later`  
https://lab.llvm.org/buildbot/#/builders/214/builds/825/steps/9/logs/stdio


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-21 Thread Florian Hahn via Phabricator via cfe-commits
fhahn added a comment.

In D122983#3464331 , @xbolva00 wrote:

> In D122983#3464315 , @aaron.ballman 
> wrote:
>
>> In D122983#3463561 , @MaskRay 
>> wrote:
>>
>>> Adding a global `-Wno-implicit-function-declaration` looks good. @Meinersbur
>>
>> +1 to this idea, but I'm worried it won't fix that particular bot.
>
> You probably need to fix Makefiles as well. I believe there were similar 
> mysterious errors with -ffp-contract changes. Try to look at history of llvm 
> test-suite.
>
> cc @fhahn if he can remember more

Depending on how many tests need updating in llvm-test-suite, I think it would 
be better to only disable the warning per-test, after verifying the warning 
works as expected.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3464347 , @aaron.ballman 
wrote:

> In D122983#3464342 , @xbolva00 
> wrote:
>
>> https://github.com/llvm/llvm-test-suite/commit/79f2b03c5111392d647fa542e120f70c8f39a991
>
> Awesome, parallel build systems to boot. I'll make that change at the same 
> time as setting CFLAGS and hopefully the combination of changes will appease 
> the bots.

Speculative fixes landed in 33a2cab37889f9599d71d27d48895d0992fe9735 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

In D122983#3464347 , @aaron.ballman 
wrote:

> In D122983#3464342 , @xbolva00 
> wrote:
>
>> https://github.com/llvm/llvm-test-suite/commit/79f2b03c5111392d647fa542e120f70c8f39a991
>
> Awesome, parallel build systems to boot. I'll make that change at the same 
> time as setting CFLAGS and hopefully the combination of changes will appease 
> the bots.

Anyway true pain to support both. Maybe time to open discussion to remove 
Makefile support as it is a maintenence hell.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3464342 , @xbolva00 wrote:

> https://github.com/llvm/llvm-test-suite/commit/79f2b03c5111392d647fa542e120f70c8f39a991

Awesome, parallel build systems to boot. I'll make that change at the same time 
as setting CFLAGS and hopefully the combination of changes will appease the 
bots.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

https://github.com/llvm/llvm-test-suite/commit/79f2b03c5111392d647fa542e120f70c8f39a991


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3464331 , @xbolva00 wrote:

> You probably need to fix Makefiles as well. I believe there were similar 
> mysterious errors with -ffp-contract changes. Try to look at history of llvm 
> test-suite.
>
> cc @fhahn if he can remember more

Good call on looking back through the commit history. It took a while to find 
anyone mucking about with warning flags, but I noticed: 
https://github.com/llvm/llvm-test-suite/commit/24550c3385e8e3703ed364e1ce20b06de97bbeee
 so it looks like CFLAGS is used. I'll push a speculative fix that tries that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-21 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a subscriber: fhahn.
xbolva00 added a comment.

In D122983#3464315 , @aaron.ballman 
wrote:

> In D122983#3463600 , @nemanjai 
> wrote:
>
>> In D122983#3463569 , @xbolva00 
>> wrote:
>>
>>> But your link shows failures in compiler-rt/, not in llvm test-suite
>>
>> It shows both. But the `compiler-rt` ones have subsequently been fixed while 
>> the `test-suite` ones are still failing as of 
>> https://lab.llvm.org/buildbot/#/builders/105/builds/24314.
>
> Thanks for pointing this out -- I'm baffled what's going on with that bot, 
> because 1) other bots running the test suite are fine, and 2) the test suite 
> was already fixed to disable the diagnostics (which is what fixed things for 
> #1):
>
> https://github.com/llvm/llvm-test-suite/blob/main/MultiSource/Benchmarks/Prolangs-C/archie-client/CMakeLists.txt#L2
> https://github.com/llvm/llvm-test-suite/blob/main/MultiSource/Benchmarks/Prolangs-C/bison/CMakeLists.txt#L1
>
> (and so on). It's almost as if something on that particular bot is overriding 
> the cmake flags (or perhaps using CFLAGS instead of CPPFLAGS)... Here's the 
> command line that's failing for bison's symtab.c:
>
> RunSafely.sh detected a failure with these command-line arguments:  
> --show-errors -t 
> /home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/test/sandbox/build/tools/timeit
>  500 /dev/null Output/symtab.llvm.o.compile 
> /home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/stage1.install/bin/clang
>  
> -I/home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/test/sandbox/build/MultiSource/Benchmarks/Prolangs-C/bison
>  
> -I/home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/test/test-suite/MultiSource/Benchmarks/Prolangs-C/bison
>  
> -I/home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/test/test-suite/include
>  -I../../../../include -D_GNU_SOURCE -D__STDC_LIMIT_MACROS -DNDEBUG -O3 
> -ffp-contract=off -fomit-frame-pointer -c 
> /home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/test/test-suite/MultiSource/Benchmarks/Prolangs-C/bison/symtab.c
>  -o Output/symtab.llvm.o
>
> Note how it's missing the warning flag from the CMakeLists.txt file.
>
> So I have no idea what's going on (I can't even really run the tests; I'm on 
> Windows and test-suite requires perl to run).
>
> In D122983#3463561 , @MaskRay wrote:
>
>> Adding a global `-Wno-implicit-function-declaration` looks good. @Meinersbur
>
> +1 to this idea, but I'm worried it won't fix that particular bot.

You probably need to fix Makefiles as well. I believe there were similar 
mysterious errors with -ffp-contract changes. Try to look at history of llvm 
test-suite.

cc @fhahn if he can remember more


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-21 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3463600 , @nemanjai wrote:

> In D122983#3463569 , @xbolva00 
> wrote:
>
>> But your link shows failures in compiler-rt/, not in llvm test-suite
>
> It shows both. But the `compiler-rt` ones have subsequently been fixed while 
> the `test-suite` ones are still failing as of 
> https://lab.llvm.org/buildbot/#/builders/105/builds/24314.

Thanks for pointing this out -- I'm baffled what's going on with that bot, 
because 1) other bots running the test suite are fine, and 2) the test suite 
was already fixed to disable the diagnostics (which is what fixed things for 
#1):

https://github.com/llvm/llvm-test-suite/blob/main/MultiSource/Benchmarks/Prolangs-C/archie-client/CMakeLists.txt#L2
https://github.com/llvm/llvm-test-suite/blob/main/MultiSource/Benchmarks/Prolangs-C/bison/CMakeLists.txt#L1

(and so on). It's almost as if something on that particular bot is overriding 
the cmake flags (or perhaps using CFLAGS instead of CPPFLAGS)... Here's the 
command line that's failing for bison's symtab.c:

RunSafely.sh detected a failure with these command-line arguments:  
--show-errors -t 
/home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/test/sandbox/build/tools/timeit
 500 /dev/null Output/symtab.llvm.o.compile 
/home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/stage1.install/bin/clang
 
-I/home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/test/sandbox/build/MultiSource/Benchmarks/Prolangs-C/bison
 
-I/home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/test/test-suite/MultiSource/Benchmarks/Prolangs-C/bison
 
-I/home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/test/test-suite/include
 -I../../../../include -D_GNU_SOURCE -D__STDC_LIMIT_MACROS -DNDEBUG -O3 
-ffp-contract=off -fomit-frame-pointer -c 
/home/buildbots/ppc64le-clang-lnt-test/clang-ppc64le-lnt/test/test-suite/MultiSource/Benchmarks/Prolangs-C/bison/symtab.c
 -o Output/symtab.llvm.o

Note how it's missing the warning flag from the CMakeLists.txt file.

So I have no idea what's going on (I can't even really run the tests; I'm on 
Windows and test-suite requires perl to run).

In D122983#3463561 , @MaskRay wrote:

> Adding a global `-Wno-implicit-function-declaration` looks good. @Meinersbur

+1 to this idea, but I'm worried it won't fix that particular bot.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-20 Thread Nemanja Ivanovic via Phabricator via cfe-commits
nemanjai added a comment.

In D122983#3463569 , @xbolva00 wrote:

> But your link shows failures in compiler-rt/, not in llvm test-suite

It shows both. But the `compiler-rt` ones have subsequently been fixed while 
the `test-suite` ones are still failing as of 
https://lab.llvm.org/buildbot/#/builders/105/builds/24314.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-20 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

In D122983#3463521 , @nemanjai wrote:

> This is still causing failures when building `test-suite`:
> https://lab.llvm.org/buildbot/#/builders/105/builds/24292
>
> Why don't we just turn off this warning for the entire `test-suite` since I 
> don't expect we want to modify the tests (as some of them have licenses that 
> probably don't allow us to modify them)?
> We could do something like this:
>
>   diff --git a/CMakeLists.txt b/CMakeLists.txt
>   index 8d76a45c7..032281ecf 100644
>   --- a/CMakeLists.txt
>   +++ b/CMakeLists.txt
>   @@ -47,6 +47,7 @@ set(LLVM_CODESIGNING_IDENTITY "" CACHE STRING
>  "Sign executables and dylibs with the given identity or skip if empty 
> (Darwin Only)")
>
>add_definitions(-DNDEBUG)
>   +add_definitions(-Wno-implicit-function-declaration)
>option(TEST_SUITE_SUPPRESS_WARNINGS "Suppress all warnings" ON)
>if(${TEST_SUITE_SUPPRESS_WARNINGS})
>  add_definitions(-w)

But your link shows failures in compiler-rt/, not in llvm test-suite


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-20 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added subscribers: Meinersbur, MaskRay.
MaskRay added a comment.

In D122983#3463521 , @nemanjai wrote:

> This is still causing failures when building `test-suite`:
> https://lab.llvm.org/buildbot/#/builders/105/builds/24292
>
> Why don't we just turn off this warning for the entire `test-suite` since I 
> don't expect we want to modify the tests (as some of them have licenses that 
> probably don't allow us to modify them)?
> We could do something like this:
>
>   diff --git a/CMakeLists.txt b/CMakeLists.txt
>   index 8d76a45c7..032281ecf 100644
>   --- a/CMakeLists.txt
>   +++ b/CMakeLists.txt
>   @@ -47,6 +47,7 @@ set(LLVM_CODESIGNING_IDENTITY "" CACHE STRING
>  "Sign executables and dylibs with the given identity or skip if empty 
> (Darwin Only)")
>
>add_definitions(-DNDEBUG)
>   +add_definitions(-Wno-implicit-function-declaration)
>option(TEST_SUITE_SUPPRESS_WARNINGS "Suppress all warnings" ON)
>if(${TEST_SUITE_SUPPRESS_WARNINGS})
>  add_definitions(-w)

Adding a global `-Wno-implicit-function-declaration` looks good. @Meinersbur


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-20 Thread Nemanja Ivanovic via Phabricator via cfe-commits
nemanjai added a comment.

This is still causing failures when building `test-suite`:
https://lab.llvm.org/buildbot/#/builders/105/builds/24292

Why don't we just turn off this warning for the entire `test-suite` since I 
don't expect we want to modify the tests (as some of them have licenses that 
probably don't allow us to modify them)?
We could do something like this:

  diff --git a/CMakeLists.txt b/CMakeLists.txt
  index 8d76a45c7..032281ecf 100644
  --- a/CMakeLists.txt
  +++ b/CMakeLists.txt
  @@ -47,6 +47,7 @@ set(LLVM_CODESIGNING_IDENTITY "" CACHE STRING
 "Sign executables and dylibs with the given identity or skip if empty 
(Darwin Only)")
   
   add_definitions(-DNDEBUG)
  +add_definitions(-Wno-implicit-function-declaration)
   option(TEST_SUITE_SUPPRESS_WARNINGS "Suppress all warnings" ON)
   if(${TEST_SUITE_SUPPRESS_WARNINGS})
 add_definitions(-w)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-20 Thread Aaron Ballman via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7d644e1215b3: [C11/C2x] Change the behavior of the implicit 
function declaration warning (authored by aaron.ballman).
Herald added a project: Sanitizers.
Herald added a subscriber: Sanitizers.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

Files:
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/ARCMT/objcmt-arc-cf-annotations.m
  clang/test/ARCMT/objcmt-arc-cf-annotations.m.result
  clang/test/Analysis/OSAtomic_mac.c
  clang/test/Analysis/ObjCProperties.m
  clang/test/Analysis/PR49642.c
  clang/test/Analysis/dead-stores.c
  clang/test/Analysis/diagnostics/no-store-func-path-notes.c
  clang/test/Analysis/exercise-ps.c
  clang/test/Analysis/malloc-three-arg.c
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/novoidtypecrash.c
  clang/test/Analysis/plist-macros-with-expansion.c
  clang/test/CodeGen/2002-07-14-MiscTests3.c
  clang/test/CodeGen/2002-07-31-SubregFailure.c
  clang/test/CodeGen/2003-08-18-SigSetJmp.c
  clang/test/CodeGen/2004-11-27-StaticFunctionRedeclare.c
  clang/test/CodeGen/2005-01-02-ConstantInits.c
  clang/test/CodeGen/2005-01-02-VAArgError-ICE.c
  clang/test/CodeGen/2005-02-20-AggregateSAVEEXPR.c
  clang/test/CodeGen/2006-01-13-StackSave.c
  clang/test/CodeGen/2006-03-03-MissingInitializer.c
  clang/test/CodeGen/2007-09-27-ComplexIntCompare.c
  clang/test/CodeGen/2008-05-12-TempUsedBeforeDef.c
  clang/test/CodeGen/2008-07-30-redef-of-bitcasted-decl.c
  clang/test/CodeGen/2008-08-19-cast-of-typedef.c
  clang/test/CodeGen/2008-10-13-FrontendCrash.c
  clang/test/CodeGen/2009-01-05-BlockInlining.c
  clang/test/CodeGen/PowerPC/builtins-ppc-p8vector.c
  clang/test/CodeGen/X86/bmi2-builtins.c
  clang/test/CodeGen/aarch64-mops.c
  clang/test/CodeGen/aarch64-neon-sm4-sm3.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_get2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_get3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_get4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ld1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ld1rq-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ldff1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ldnf1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ldnt1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_rev-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_set2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_set3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_set4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_st1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_stnt1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_trn1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_trn1-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_trn2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_trn2-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_undef-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_undef2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_undef3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_undef4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_uzp1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_uzp1-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_uzp2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_uzp2-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_zip1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_zip1-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_zip2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_zip2-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_aba.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abalb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abalt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abdlb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abdlt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_adalp.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_adclb.c
  

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:420-422
+def ext_implicit_function_decl_c99 : ExtWarn<
+  "implicit declaration of function %0 is invalid in C99 and later and "
+  "unsupported in C2x">, InGroup, DefaultError;

rsmith wrote:
> The context for someone reading this diagnostic is that they're building in 
> C99 / C11 / C17 mode, used an undeclared function, and (by default) saw this 
> error. In that context, I think it may not be especially relevant to them 
> that Clang would not even allow the error to be disabled if they built in C2x 
> mode, so I'd be inclined to remove the "and unsupported in C2x" part from 
> this diagnostic. Also, they probably didn't *intend* to implicitly declare a 
> function, so perhaps we could replace this diagnostic with something less 
> implementation-oriented and more user-oriented:
> 
> "call to undeclared function %0; ISO C99 and later do not support implicit 
> function declarations"
> 
> (The phrasing "ISO C99 and later do not support" is what we usually use when 
> we want to imply "but perhaps Clang does support" -- keeping in mind that 
> this diagnostic might appear as either an error or a warning, and we want 
> phrasing that works both ways.)
> 
> That said: this is orthogonal to the changes you're making here, so don't 
> consider this comment to be blocking.
> In that context, I think it may not be especially relevant to them that Clang 
> would not even allow the error to be disabled if they built in C2x mode, so 
> I'd be inclined to remove the "and unsupported in C2x" part from this 
> diagnostic.

The reason I had it worded that way was to justify why it's an error instead of 
a warning.

> "call to undeclared function %0; ISO C99 and later do not support implicit 
> function declarations"

I think this is shorter and equally as clear as what I had. I'll make the 
change and push it up again for precommit CI to run over (I would not be 
surprised if I missed a few places to update because of loose `-verify` 
checking behavior that will no longer match the new wording).



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:698-700
 def ext_implicit_lib_function_decl : ExtWarn<
   "implicitly declaring library function '%0' with type %1">,
   InGroup;

rsmith wrote:
> Hm, why is this one an `ExtWarn`? Are we really allowed to reject implicit 
> declarations of library functions (in `-std=c89 -pedantic-errors` mode)?
No, we're not. But `ext_implicit_lib_function_decl` was already `ExtWarn`; this 
new one is the diagnostic you get only in C99 and later.

If you'd like, I can make the C89 one into a `Warning` though (either now or in 
a follow-up). However, we should probably also think about whether we want C++ 
to be a default error or not. In this patch it does not default to an error.



Comment at: clang/lib/Sema/SemaDecl.cpp:15319-15321
   // OpenCL v2.0 s6.9.u - Implicit function declaration is not supported.
   else if (getLangOpts().OpenCL)
 diag_id = diag::err_opencl_implicit_function_decl;

rsmith wrote:
> Should we even be calling this function in OpenCL mode? It seems a bit 
> inconsistent that we avoid calling this in C++ and C2x mode, and that we call 
> it but error in OpenCL mode.
> 
> Maybe there should be a function on `LangOptions` to ask if implicit function 
> declarations are permitted in the current language mode, to make it easy to 
> opt out the right cases? (Happy for this to be a follow-on change if you 
> agree.)
I agree that it does seem inconsistent. I can look into making that change in a 
follow-up.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 423912.
aaron.ballman added a comment.

Fixes some failing tests found by precommit CI.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

Files:
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/ARCMT/objcmt-arc-cf-annotations.m
  clang/test/ARCMT/objcmt-arc-cf-annotations.m.result
  clang/test/Analysis/OSAtomic_mac.c
  clang/test/Analysis/ObjCProperties.m
  clang/test/Analysis/PR49642.c
  clang/test/Analysis/dead-stores.c
  clang/test/Analysis/diagnostics/no-store-func-path-notes.c
  clang/test/Analysis/exercise-ps.c
  clang/test/Analysis/malloc-three-arg.c
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/novoidtypecrash.c
  clang/test/Analysis/plist-macros-with-expansion.c
  clang/test/CodeGen/2002-07-14-MiscTests3.c
  clang/test/CodeGen/2002-07-31-SubregFailure.c
  clang/test/CodeGen/2003-08-18-SigSetJmp.c
  clang/test/CodeGen/2004-11-27-StaticFunctionRedeclare.c
  clang/test/CodeGen/2005-01-02-ConstantInits.c
  clang/test/CodeGen/2005-01-02-VAArgError-ICE.c
  clang/test/CodeGen/2005-02-20-AggregateSAVEEXPR.c
  clang/test/CodeGen/2006-01-13-StackSave.c
  clang/test/CodeGen/2006-03-03-MissingInitializer.c
  clang/test/CodeGen/2007-09-27-ComplexIntCompare.c
  clang/test/CodeGen/2008-05-12-TempUsedBeforeDef.c
  clang/test/CodeGen/2008-07-30-redef-of-bitcasted-decl.c
  clang/test/CodeGen/2008-08-19-cast-of-typedef.c
  clang/test/CodeGen/2008-10-13-FrontendCrash.c
  clang/test/CodeGen/2009-01-05-BlockInlining.c
  clang/test/CodeGen/PowerPC/builtins-ppc-p8vector.c
  clang/test/CodeGen/X86/bmi2-builtins.c
  clang/test/CodeGen/aarch64-mops.c
  clang/test/CodeGen/aarch64-neon-sm4-sm3.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_get2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_get3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_get4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ld1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ld1rq-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ldff1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ldnf1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ldnt1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_rev-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_set2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_set3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_set4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_st1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_stnt1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_trn1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_trn1-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_trn2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_trn2-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_undef-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_undef2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_undef3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_undef4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_uzp1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_uzp1-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_uzp2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_uzp2-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_zip1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_zip1-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_zip2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_zip2-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_aba.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abalb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abalt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abdlb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abdlt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_adalp.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_adclb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_adclt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addhnb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addhnt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addlb.c
  

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 423902.
aaron.ballman added a comment.

Rebased to hopefully get precommit CI to test it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

Files:
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/ARCMT/objcmt-arc-cf-annotations.m
  clang/test/ARCMT/objcmt-arc-cf-annotations.m.result
  clang/test/Analysis/OSAtomic_mac.c
  clang/test/Analysis/ObjCProperties.m
  clang/test/Analysis/PR49642.c
  clang/test/Analysis/dead-stores.c
  clang/test/Analysis/diagnostics/no-store-func-path-notes.c
  clang/test/Analysis/exercise-ps.c
  clang/test/Analysis/malloc-three-arg.c
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/novoidtypecrash.c
  clang/test/Analysis/plist-macros-with-expansion.c
  clang/test/CodeGen/2002-07-14-MiscTests3.c
  clang/test/CodeGen/2002-07-31-SubregFailure.c
  clang/test/CodeGen/2003-08-18-SigSetJmp.c
  clang/test/CodeGen/2004-11-27-StaticFunctionRedeclare.c
  clang/test/CodeGen/2005-01-02-ConstantInits.c
  clang/test/CodeGen/2005-01-02-VAArgError-ICE.c
  clang/test/CodeGen/2005-02-20-AggregateSAVEEXPR.c
  clang/test/CodeGen/2006-01-13-StackSave.c
  clang/test/CodeGen/2006-03-03-MissingInitializer.c
  clang/test/CodeGen/2007-09-27-ComplexIntCompare.c
  clang/test/CodeGen/2008-05-12-TempUsedBeforeDef.c
  clang/test/CodeGen/2008-07-30-redef-of-bitcasted-decl.c
  clang/test/CodeGen/2008-08-19-cast-of-typedef.c
  clang/test/CodeGen/2008-10-13-FrontendCrash.c
  clang/test/CodeGen/2009-01-05-BlockInlining.c
  clang/test/CodeGen/PowerPC/builtins-ppc-p8vector.c
  clang/test/CodeGen/X86/bmi2-builtins.c
  clang/test/CodeGen/aarch64-mops.c
  clang/test/CodeGen/aarch64-neon-sm4-sm3.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_get2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_get3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_get4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ld1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ld1rq-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ldff1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ldnf1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ldnt1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_rev-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_set2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_set3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_set4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_st1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_stnt1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_trn1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_trn1-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_trn2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_trn2-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_undef-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_undef2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_undef3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_undef4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_uzp1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_uzp1-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_uzp2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_uzp2-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_zip1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_zip1-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_zip2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_zip2-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_aba.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abalb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abalt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abdlb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abdlt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_adalp.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_adclb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_adclt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addhnb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addhnt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addlb.c
  

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 423895.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.

Updated the diagnostic wording, putting it through precommit CI again to see if 
I missed any tests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

Files:
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/test/ARCMT/objcmt-arc-cf-annotations.m
  clang/test/ARCMT/objcmt-arc-cf-annotations.m.result
  clang/test/Analysis/OSAtomic_mac.c
  clang/test/Analysis/ObjCProperties.m
  clang/test/Analysis/PR49642.c
  clang/test/Analysis/dead-stores.c
  clang/test/Analysis/diagnostics/no-store-func-path-notes.c
  clang/test/Analysis/exercise-ps.c
  clang/test/Analysis/malloc-three-arg.c
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/novoidtypecrash.c
  clang/test/Analysis/plist-macros-with-expansion.c
  clang/test/CodeGen/2002-07-14-MiscTests3.c
  clang/test/CodeGen/2002-07-31-SubregFailure.c
  clang/test/CodeGen/2003-08-18-SigSetJmp.c
  clang/test/CodeGen/2004-11-27-StaticFunctionRedeclare.c
  clang/test/CodeGen/2005-01-02-ConstantInits.c
  clang/test/CodeGen/2005-01-02-VAArgError-ICE.c
  clang/test/CodeGen/2005-02-20-AggregateSAVEEXPR.c
  clang/test/CodeGen/2006-01-13-StackSave.c
  clang/test/CodeGen/2006-03-03-MissingInitializer.c
  clang/test/CodeGen/2007-09-27-ComplexIntCompare.c
  clang/test/CodeGen/2008-05-12-TempUsedBeforeDef.c
  clang/test/CodeGen/2008-07-30-redef-of-bitcasted-decl.c
  clang/test/CodeGen/2008-08-19-cast-of-typedef.c
  clang/test/CodeGen/2008-10-13-FrontendCrash.c
  clang/test/CodeGen/2009-01-05-BlockInlining.c
  clang/test/CodeGen/PowerPC/builtins-ppc-p8vector.c
  clang/test/CodeGen/X86/bmi2-builtins.c
  clang/test/CodeGen/aarch64-mops.c
  clang/test/CodeGen/aarch64-neon-sm4-sm3.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_get2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_get3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_get4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ld1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ld1rq-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ldff1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ldnf1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ldnt1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_rev-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_set2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_set3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_set4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_st1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_stnt1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_trn1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_trn1-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_trn2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_trn2-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_undef-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_undef2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_undef3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_undef4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_uzp1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_uzp1-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_uzp2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_uzp2-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_zip1-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_zip1-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_zip2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_zip2-fp64-bfloat.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_aba.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abalb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abalt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abdlb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_abdlt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_adalp.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_adclb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_adclt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addhnb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_addhnt.c
  

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-19 Thread James Y Knight via Phabricator via cfe-commits
jyknight accepted this revision.
jyknight added a comment.

Looks good to me, whichever way you go regarding rsmith's comments.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-19 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

LGTM; two non-blocking comments and a question.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:420-422
+def ext_implicit_function_decl_c99 : ExtWarn<
+  "implicit declaration of function %0 is invalid in C99 and later and "
+  "unsupported in C2x">, InGroup, DefaultError;

The context for someone reading this diagnostic is that they're building in C99 
/ C11 / C17 mode, used an undeclared function, and (by default) saw this error. 
In that context, I think it may not be especially relevant to them that Clang 
would not even allow the error to be disabled if they built in C2x mode, so I'd 
be inclined to remove the "and unsupported in C2x" part from this diagnostic. 
Also, they probably didn't *intend* to implicitly declare a function, so 
perhaps we could replace this diagnostic with something less 
implementation-oriented and more user-oriented:

"call to undeclared function %0; ISO C99 and later do not support implicit 
function declarations"

(The phrasing "ISO C99 and later do not support" is what we usually use when we 
want to imply "but perhaps Clang does support" -- keeping in mind that this 
diagnostic might appear as either an error or a warning, and we want phrasing 
that works both ways.)

That said: this is orthogonal to the changes you're making here, so don't 
consider this comment to be blocking.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:698-700
 def ext_implicit_lib_function_decl : ExtWarn<
   "implicitly declaring library function '%0' with type %1">,
   InGroup;

Hm, why is this one an `ExtWarn`? Are we really allowed to reject implicit 
declarations of library functions (in `-std=c89 -pedantic-errors` mode)?



Comment at: clang/lib/Sema/SemaDecl.cpp:15319-15321
   // OpenCL v2.0 s6.9.u - Implicit function declaration is not supported.
   else if (getLangOpts().OpenCL)
 diag_id = diag::err_opencl_implicit_function_decl;

Should we even be calling this function in OpenCL mode? It seems a bit 
inconsistent that we avoid calling this in C++ and C2x mode, and that we call 
it but error in OpenCL mode.

Maybe there should be a function on `LangOptions` to ask if implicit function 
declarations are permitted in the current language mode, to make it easy to opt 
out the right cases? (Happy for this to be a follow-on change if you agree.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3454632 , @rsmith wrote:

> In D122983#3454508 , @aaron.ballman 
> wrote:
>
>> In D122983#3454494 , @jyknight 
>> wrote:
>>
>>> In D122983#3454406 , 
>>> @aaron.ballman wrote:
>>>
 Out of curiosity -- do you think we should remove the `DefaultIgnore` in 
 C89 mode so that we warn by default there (perhaps as a follow up)? Also, 
 I presume you expect `diag::ext_implicit_lib_function_decl` to behave the 
 same way (warn in C89, warn-as-err in C99 and up) as part of this patch?
>
> I think it would make sense to include the C89 warning in `-Wc99-compat`, but 
> I agree with James that it doesn't seem worthwhile to enable it by default.

Ah, that's a good idea for a follow-up, thanks!

>> I feel a bit more strongly that implicit builtins should be handled the same 
>> as other implicit functions (in terms of `DefaultError` behavior), unless 
>> there's some scenarios I'm not thinking of.
>
> I'm not sure which case you mean by "implicit builtins". Two cases seem 
> relevant:
>
> - `ext_implicit_lib_function_decl`, for things like using `strlen` without 
> including its header; I think that should behave the same as using any other 
> undeclared function name (error by default in C99 onwards, error with no 
> warning flag in C2x onwards even though we would synthesize the correct 
> prototype).

Great, this was the one I was talking about, and that's the approach I'd like 
to take with it. I'll have that in the next revision.

> - `warn_builtin_unknown`, for things like using 
> `__builtin_no_such_builtin_exists()` (a function name starting `__builtin_` 
> that is not a known builtin and hasn't been declared), which is currently a 
> warning-as-error in all language modes; I think that should stay an error by 
> default in all modes, and I'd be happy to see it turn into a harder error 
> (with no warning flag), whether that happens only in C2x onwards, in C99 
> onwards, or in all modes.

Okay, I hadn't touched that one because it was already default error. I'm happy 
to try to turn it into a hard error in a followup patch.

Thanks for the feedback!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In D122983#3454508 , @aaron.ballman 
wrote:

> In D122983#3454494 , @jyknight 
> wrote:
>
>> In D122983#3454406 , 
>> @aaron.ballman wrote:
>>
>>> Out of curiosity -- do you think we should remove the `DefaultIgnore` in 
>>> C89 mode so that we warn by default there (perhaps as a follow up)? Also, I 
>>> presume you expect `diag::ext_implicit_lib_function_decl` to behave the 
>>> same way (warn in C89, warn-as-err in C99 and up) as part of this patch?

I think it would make sense to include the C89 warning in `-Wc99-compat`, but I 
agree with James that it doesn't seem worthwhile to enable it by default.

> I feel a bit more strongly that implicit builtins should be handled the same 
> as other implicit functions (in terms of `DefaultError` behavior), unless 
> there's some scenarios I'm not thinking of.

I'm not sure which case you mean by "implicit builtins". Two cases seem 
relevant:

- `ext_implicit_lib_function_decl`, for things like using `strlen` without 
including its header; I think that should behave the same as using any other 
undeclared function name (error by default in C99 onwards, error with no 
warning flag in C2x onwards even though we would synthesize the correct 
prototype).
- `warn_builtin_unknown`, for things like using 
`__builtin_no_such_builtin_exists()` (a function name starting `__builtin_` 
that is not a known builtin and hasn't been declared), which is currently a 
warning-as-error in all language modes; I think that should stay an error by 
default in all modes, and I'd be happy to see it turn into a harder error (with 
no warning flag), whether that happens only in C2x onwards, in C99 onwards, or 
in all modes.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3454494 , @jyknight wrote:

> In D122983#3454406 , @aaron.ballman 
> wrote:
>
>> In D122983#3452994 , @rsmith wrote:
>>
>>> I think we should just make this an error by default in C99 onwards;
>>
>> Out of curiosity -- do you think we should remove the `DefaultIgnore` in C89 
>> mode so that we warn by default there (perhaps as a follow up)? Also, I 
>> presume you expect `diag::ext_implicit_lib_function_decl` to behave the same 
>> way (warn in C89, warn-as-err in C99 and up) as part of this patch?
>
> I'm not sure what purpose it'd serve to change -std=c89 to be more strict at 
> this point. It's not the default compilation mode, and the code is actually 
> valid under that standard. IMO, adding such a on-by-default warning there 
> would only serve to annoy folks explicitly trying to build super-old code 
> with a super-old standards version.

Yeah, I was waffling on that one -- my thinking was that it would at least warn 
users that they're using something dangerous and we're going to break them if 
they attempt to upgrade (aka, we treat it as almost-deprecated in C89). But I'm 
fine leaving that one as `DefaultIgnore` too; as you say, if someone is in that 
mode explicitly, this IS a "feature" of that language.

I feel a bit more strongly that implicit builtins should be handled the same as 
other implicit functions (in terms of `DefaultError` behavior), unless there's 
some scenarios I'm not thinking of.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-15 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment.

In D122983#3454406 , @aaron.ballman 
wrote:

> In D122983#3452994 , @rsmith wrote:
>
>> I think we should just make this an error by default in C99 onwards;
>
> Out of curiosity -- do you think we should remove the `DefaultIgnore` in C89 
> mode so that we warn by default there (perhaps as a follow up)? Also, I 
> presume you expect `diag::ext_implicit_lib_function_decl` to behave the same 
> way (warn in C89, warn-as-err in C99 and up) as part of this patch?

I'm not sure what purpose it'd serve to change -std=c89 to be more strict at 
this point. It's not the default compilation mode, and the code is actually 
valid under that standard. IMO, adding such a on-by-default warning there would 
only serve to annoy folks explicitly trying to build super-old code with a 
super-old standards version.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3452994 , @rsmith wrote:

> I think we should just make this an error by default in C99 onwards;

Out of curiosity -- do you think we should remove the `DefaultIgnore` in C89 
mode so that we warn by default there (perhaps as a follow up)? Also, I presume 
you expect `diag::ext_implicit_lib_function_decl` to behave the same way (warn 
in C89, warn-as-err in C99 and up) as part of this patch?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3452994 , @rsmith wrote:

> In D122983#3451920 , @erichkeane 
> wrote:
>
>> I think Aaron's approach provides a proper 'depreciation' period in our 
>> compiler, as best as we have the ability to do.
>
> We've been warning by default for a decade that this code is not valid in 
> C99; that was our deprecation period.

I thought about this perspective a bunch overnight and I think I agree that 
this is reasonable given that the warning explicitly calls the code out as 
being *invalid*.

> If the aim is to provide a deprecation period, the end goal should be that in 
> some future Clang version we complete the transition and change the C99 
> default to reject too. Otherwise, that's not a deprecation period, that's a 
> permanent language extension in our C99 mode -- and it seems capricious to 
> provide that extension by default in C99 mode but not C11 / C17 mode. Given 
> that this extension is, well, bad, I think we shouldn't be providing it by 
> default anywhere. It's not hard for people to turn the warning flag off, and 
> people intentionally using this in C99 onwards probably already have done so.

FWIW, I misunderstood your original suggestion:

> I think we should just make this an error by default in C99 onwards;

I thought you meant this should be an *error* (not a warning defaulting to an 
error) in C99 and later. That would be too aggressive of an approach for me to 
support because it gives no upgrade path to users to easily get onto Clang 15. 
Now that I understand you better, I like your approach and rationale.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In D122983#3451920 , @erichkeane 
wrote:

> I think Aaron's approach provides a proper 'depreciation' period in our 
> compiler, as best as we have the ability to do.

We've been warning by default for a decade that this code is not valid in C99; 
that was our deprecation period. If the aim is to provide a deprecation period, 
the end goal should be that in some future Clang version we complete the 
transition and change the C99 default to reject too. Otherwise, that's not a 
deprecation period, that's a permanent language extension in our C99 mode -- 
and it seems capricious to provide that extension by default in C99 mode but 
not C11 / C17 mode. Given that this extension is, well, bad, I think we 
shouldn't be providing it by default anywhere. It's not hard for people to turn 
the warning flag off, and people intentionally using this in C99 onwards 
probably already have done so.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-14 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

FWIW, Aaron and I discussed this at length while he was preparing this patch.  
I think the current behavior (warn in C99, warn-as-error in C11/C17, hard error 
in C20) is about as aggressive as I'd want us to be with erroring without 
causing extensive heartache on our users. WG14 did us no favors here in 1999, 
and our delay on making this an error for 20 years since then has created an 
environment where I wish for us to be somewhat understanding of our users.  If 
you'd asked me in 1998 of course my answer would have been different.

I think Aaron's approach provides a proper 'depreciation' period in our 
compiler, as best as we have the ability to do.  I consider MOST projects who 
use C99 to be 'legacy' code bases with little maintenance, so introducing new 
errors to them would be unfortunate (even default-W-as-E). I see value to still 
warning in the C99 case, as it _IS_ a good improvement for anyone going through 
the source code trying to do small improvements, but a default-W-as-E causes 
greater overhead in that case.

Comparatively, I see projects using C11/C17 to be 'modern' or 'maintained' code 
bases, so the disable-able error seems to me to be a proper 'kick' to let them 
know this is something they need to change ASAP.  I would be VERY against this 
being 'just a warning' though, as they are more simply/commonly ignored, and 
makes the hard-error in C20 that much more jarring.

While I agree this is quite a odd of a transition, I see it as the most-gentle 
we can be for our users.  Anything 'more aggressive' in the C99/C11/C17 case 
would be an unfortunate burden, and anything 'less aggressive' would make the 
C20 change a further jarring/unfortunate burden.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3450177 , @rsmith wrote:

> It seems surprising to me for the diagnostic to change from warn-by-default 
> to error-by-default when changing from C99 to C11, given that the language 
> rule did not change between C99 and C11 (as a Clang user, when changing my 
> `-std=` flag, I don't want other changes to come in that are unrelated to the 
> language mode change I requested).

FWIW, that's the approach I originally considered. I eventually discarded it as 
being likely too disruptive for users (with risk of pushing some people to not 
upgrade to Clang 15).

This situation is a somewhat unique one. C89 supported this feature and it was 
not obsolescent in that revision of the standard. C99 removed the feature 
outright. (The same thing happened to implicit int.) Because there was no 
deprecation period, people got caught off guard and so compilers made it a 
warning in C99 that also triggered in C89. As best I can tell, this is the 
first time the severity question has been revisited. The reason I took the 
approach I did is because of the lack of deprecation period and the somewhat 
common user practice of using an effectively-C89 code base with some C99 
features (most often: ability to mix decls + code, `//` comments, and `long 
long` support -- aka, what MSVC considered "C" to be for many, many years).

> I think we should just make this an error by default in C99 onwards; if we're 
> happy promoting this from warning-by-default to error-by-default for the 
> folks using `-std=c11` and later (and I think we are), then we should be 
> happy doing the same for the `-std=c99` folks too -- especially given that 
> C17 is the default everywhere other than on PS4.

I see where you're coming from, but I don't think it's practical. We've given 
users this as a warning in C99 and later for a *long* time (since Clang 2.6 
from 2009), so I anticipate an error breaking some amount of code for people 
passing `-std=c99` explicitly. We do still run the risk of breaking people who 
are on C11 or C17 (particularly, given it's the default when no `-std=` is 
provided), but that feels like a more manageable risk. Those folks are clearly 
interested in upgrading language modes that I don't think apply to folks 
specifying C99 specifically (now that it's 20+ years old).

tl;dr: I think the current approach strikes a good balance between pushing 
people to improve their code and giving them a reasonable upgrade path through 
the language standards.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-13 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

It seems surprising to me for the diagnostic to change from warn-by-default to 
error-by-default when changing from C99 to C11, given that the language rule 
did not change between C99 and C11 (as a Clang user, when changing my `-std=` 
flag, I don't want other changes to come in that are unrelated to the language 
mode change I requested). I think we should just make this an error by default 
in C99 onwards; if we're happy promoting this from warning-by-default to 
error-by-default for the folks using `-std=c11` and later (and I think we are), 
then we should be happy doing the same for the `-std=c99` folks too -- 
especially given that C17 is the default everywhere other than on PS4.




Comment at: clang/lib/Sema/SemaDecl.cpp:15269
   if (II.getName().startswith("__builtin_"))
 diag_id = diag::warn_builtin_unknown;
   // OpenCL v2.0 s6.9.u - Implicit function declaration is not supported.

For consistency, this case should be an `ExtWarn`, not just a `Warning`, in C99 
onwards (and an `Error` in C2x, which it will be because we don't get here in 
that case). But given that this is within the implementation namespace and it's 
currently a `DefaultError`, perhaps we should instead fully replace this 
`Warning` with an `Error` in all language modes?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 422643.
aaron.ballman marked 3 inline comments as done.
aaron.ballman added a comment.

Update based on review feedback.

- Removed a typo correction note so that we don't recommend a previous 
declaration which was implicitly declared
- Removed several -std=c99 options from various RUN lines; other updates to 
tests


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

Files:
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/ARCMT/objcmt-arc-cf-annotations.m
  clang/test/ARCMT/objcmt-arc-cf-annotations.m.result
  clang/test/Analysis/OSAtomic_mac.c
  clang/test/Analysis/ObjCProperties.m
  clang/test/Analysis/PR49642.c
  clang/test/Analysis/diagnostics/no-store-func-path-notes.c
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/novoidtypecrash.c
  clang/test/Analysis/plist-macros-with-expansion.c
  clang/test/CodeGen/2002-07-14-MiscTests3.c
  clang/test/CodeGen/2002-07-31-SubregFailure.c
  clang/test/CodeGen/2003-08-18-SigSetJmp.c
  clang/test/CodeGen/2004-11-27-StaticFunctionRedeclare.c
  clang/test/CodeGen/2005-01-02-ConstantInits.c
  clang/test/CodeGen/2005-01-02-VAArgError-ICE.c
  clang/test/CodeGen/2006-01-13-StackSave.c
  clang/test/CodeGen/2006-03-03-MissingInitializer.c
  clang/test/CodeGen/2008-05-12-TempUsedBeforeDef.c
  clang/test/CodeGen/2008-07-30-redef-of-bitcasted-decl.c
  clang/test/CodeGen/2008-08-19-cast-of-typedef.c
  clang/test/CodeGen/2008-10-13-FrontendCrash.c
  clang/test/CodeGen/PowerPC/builtins-ppc-p8vector.c
  clang/test/CodeGen/X86/bmi2-builtins.c
  clang/test/CodeGen/aarch64-mops.c
  clang/test/CodeGen/aarch64-neon-sm4-sm3.c
  clang/test/CodeGen/arm_acle.c
  clang/test/CodeGen/attribute_constructor.c
  clang/test/CodeGen/builtins-arm-microsoft.c
  clang/test/CodeGen/builtins-arm-msvc-compat-only.c
  clang/test/CodeGen/cast-emit.c
  clang/test/CodeGen/debug-info-block-vars.c
  clang/test/CodeGen/debug-info-crash.c
  clang/test/CodeGen/decl.c
  clang/test/CodeGen/init-with-member-expr.c
  clang/test/CodeGen/misaligned-param.c
  clang/test/CodeGen/neon-crypto.c
  clang/test/CodeGen/struct-comma.c
  clang/test/CodeGen/variable-array.c
  clang/test/Frontend/warning-mapping-2.c
  clang/test/Headers/arm-cmse-header-ns.c
  clang/test/Import/objc-arc/test-cleanup-object.m
  clang/test/Modules/config_macros.m
  clang/test/Modules/modulemap-locations.m
  clang/test/OpenMP/declare_mapper_messages.c
  clang/test/PCH/chain-macro-override.c
  clang/test/Rewriter/rewrite-foreach-2.m
  clang/test/Rewriter/rewrite-try-catch.m
  clang/test/Sema/__try.c
  clang/test/Sema/aarch64-tme-errors.c
  clang/test/Sema/arm-no-fp16.c
  clang/test/Sema/bitfield.c
  clang/test/Sema/builtin-setjmp.c
  clang/test/Sema/builtins.c
  clang/test/Sema/cxx-as-c.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/implicit-decl.c
  clang/test/Sema/implicit-ms-builtin-decl.c
  clang/test/Sema/typo-correction.c
  clang/test/Sema/vla.c
  clang/test/Sema/warn-strict-prototypes.c
  clang/test/SemaOpenCL/arm-integer-dot-product.cl
  clang/test/SemaOpenCL/clang-builtin-version.cl
  clang/test/SemaOpenCL/to_addr_builtin.cl
  clang/test/VFS/module_missing_vfs.m
  compiler-rt/test/safestack/pthread-cleanup.c

Index: compiler-rt/test/safestack/pthread-cleanup.c
===
--- compiler-rt/test/safestack/pthread-cleanup.c
+++ compiler-rt/test/safestack/pthread-cleanup.c
@@ -17,6 +17,8 @@
   return buffer;
 }
 
+extern unsigned sleep(unsigned seconds);
+
 int main(int argc, char **argv)
 {
   int arg = atoi(argv[1]);
Index: clang/test/VFS/module_missing_vfs.m
===
--- clang/test/VFS/module_missing_vfs.m
+++ clang/test/VFS/module_missing_vfs.m
@@ -1,12 +1,12 @@
 // RUN: rm -rf %t && mkdir -p %t
 // RUN: echo "void funcA(void);" >> %t/a.h
 
-// RUN: not %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
+// RUN: not %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
 // ERROR: virtual filesystem overlay file '{{.*}}' not found
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/MissingVFS/vfsoverlay.yaml > %t/vfs.yaml
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
+// RUN: %clang_cc1 -std=c99 

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman marked 10 inline comments as done.
aaron.ballman added a comment.

Thank you for the thorough double-check on the test changes, @jyknight! I was 
able to remove most of the ones you called out (I commented where I couldn't) 
plus a few more `-std=c99` from RUN lines, but there are quite a few left 
because it's entirely unclear what the test is actually testing (especially in 
CodeGen tests -- we have these RUN lines but they don't check anything, so I'm 
assuming these are "don't crash" tests reduced from user code, so I left the 
-std=c99 for them).




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c:76
 
 void pointeeConverison(int *IP, double *DP) { pointeeConversion(DP, IP); }
 // NO-WARN: Even though this is possible in C, a swap is diagnosed by the 
compiler.

jyknight wrote:
> This is just a typo, you won't need the -std=c99 on this test if you fix 
> pointeeConverison -> pointeeConversion
Good catch, I'll fix this up in an NFC change.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:423-426
+def ext_implicit_function_decl_c11 : ExtWarn<
+  "implicit declaration of function %0 is invalid in C99 and later and "
+  "unsupported in C2x">,
+  InGroup, DefaultError;

jyknight wrote:
> I think it'd be good to use the same message for both variants. (Even if 
> you're building in c99, might as well note it's gone in C2x).
Heh, I waffled on doing exactly that, so I'm glad for this feedback!



Comment at: clang/test/CodeGen/misaligned-param.c:1
-// RUN: %clang_cc1 %s -triple i386-apple-darwin -emit-llvm -o - | FileCheck %s
 // Misaligned parameter must be memcpy'd to correctly aligned temporary.

jyknight wrote:
> Declare `int bar(struct s*, struct s*);` instead
Good catch; I think I was worried this was intending to test the function call, 
which is why I added the `-std=c89`.



Comment at: clang/test/Sema/implicit-decl.c:30
+  __builtin_is_les(1, 3); // expected-error{{use of unknown builtin 
'__builtin_is_les'}} \
+ c2x-error {{unknown type name '__builtin_is_les'; 
did you mean '__builtin_va_list'?}} \
+ c2x-error {{expected identifier or '('}} \

jyknight wrote:
> Yikes! That's a particularly awful typo-fix suggestion. I assume from those 
> messages, it decided this should be a function declaration instead of a call, 
> and is parsing as a function declaration?
> 
> I'd say that's unrelated to this change, EXCEPT, that doesn't happen in C++ 
> mode. So I'm guessing there's some weird interaction here that triggers ONLY 
> in the C mode of the parser, which causes it to prefer to parse unknown 
> identifiers as a type name instead of a variable/function name (on the 
> assumption that implicit function decls exist, and would've triggered 
> already, if relevant, perhaps?).
> Yikes! That's a particularly awful typo-fix suggestion. I assume from those 
> messages, it decided this should be a function declaration instead of a call, 
> and is parsing as a function declaration?

That's correct.

> I'd say that's unrelated to this change, EXCEPT, that doesn't happen in C++ 
> mode. So I'm guessing there's some weird interaction here that triggers ONLY 
> in the C mode of the parser, which causes it to prefer to parse unknown 
> identifiers as a type name instead of a variable/function name (on the 
> assumption that implicit function decls exist, and would've triggered 
> already, if relevant, perhaps?).

That seems to be correct. It appears we get into this state because of implicit 
int. So I'd like to leave this alone for now and try to revisit later when I 
try to treat implicit int similar to what I've done in this patch for implicit 
function declarations.



Comment at: clang/test/VFS/module_missing_vfs.m:2
 // RUN: rm -rf %t && mkdir -p %t
 // RUN: echo "void funcA(void);" >> %t/a.h
 

jyknight wrote:
> I can't quite follow what this test is doing, but it looks like it does 
> intend to have a prototype available?
Yeah, I struggled mightily to figure out what this test was doing and 
eventually gave up. I *think* the behavior of the test is that it's putting the 
declaration in a.h and then overlaying with an a.h (in Inputs/MissingVFS) that 
does not have the declaration, and demonstrating that you can still call it? 
But I honestly don't know.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-12 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment.

Looks reasonable overall.

I've added a few specific comments to some tests, but in general, I think we 
should avoid adding -std=c99 to test-cases to workaround implicit decl issues, 
unless:

- the test is explicitly testing the behavior of implicit decls (potentially in 
interaction with some other feature).
- it's a regression test, with some particular reduced/artificial inputs.
- the ARM codegen tests, for now, on the assumption the arm maintainers will 
address it separately.




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c:76
 
 void pointeeConverison(int *IP, double *DP) { pointeeConversion(DP, IP); }
 // NO-WARN: Even though this is possible in C, a swap is diagnosed by the 
compiler.

This is just a typo, you won't need the -std=c99 on this test if you fix 
pointeeConverison -> pointeeConversion



Comment at: clang/docs/ReleaseNotes.rst:141-146
+- The ``-Wimplicit-function-declaration`` warning diagnostic now defaults to
+  emitting an error (which can be downgraded to a warning with
+  ``-Wno-error=implicit-function-declaration`` or disabled entirely with
+  ``-Wno-implicit-function-declaration``) in C11 and
+  C17 mode. This is because the feature was removed in C99 and cannot be
+  supported in C2x.

Some wording suggestions here, to remove parenthetical, and more explicitly 
note that the options have no effect in C2x.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:423-426
+def ext_implicit_function_decl_c11 : ExtWarn<
+  "implicit declaration of function %0 is invalid in C99 and later and "
+  "unsupported in C2x">,
+  InGroup, DefaultError;

I think it'd be good to use the same message for both variants. (Even if you're 
building in c99, might as well note it's gone in C2x).



Comment at: clang/test/CodeGen/PowerPC/builtins-ppc-p8vector.c:4
 // RUN: %clang_cc1 -target-feature +altivec -target-feature +power8-vector 
-triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s 
-check-prefix=CHECK-LE
-// RUN: not %clang_cc1 -target-feature +altivec -target-feature +vsx -triple 
powerpc64-unknown-unknown -emit-llvm %s -o - 2>&1 | FileCheck %s 
-check-prefix=CHECK-PPC
+// RUN: not %clang_cc1 -std=c99 -target-feature +altivec -target-feature +vsx 
-triple powerpc64-unknown-unknown -emit-llvm %s -o - 2>&1 | FileCheck %s 
-check-prefix=CHECK-PPC
 // Added -target-feature +vsx above to avoid errors about "vector double" and 
to

This test-run is already expected to fail, so the -std=c99 seems like it 
shouldn't be necessary. I think just change the CHECK-PPC lines looking for 
"warning: implicit declaration" to look for "error: implicit declaration" 
instead.



Comment at: clang/test/CodeGen/X86/bmi2-builtins.c:1
-// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin 
-target-feature +bmi2 -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin 
-target-feature +bmi2 -emit-llvm -std=c99 -o - | FileCheck %s
 // RUN: %clang_cc1 -ffreestanding %s -triple=i386-apple-darwin -target-feature 
+bmi2 -emit-llvm -o - | FileCheck %s --check-prefix=B32

That this is needed looks like a test bug here -- it should be testing 
_mulx_u64 on x86-64 and _mulx_u32 on i386.



Comment at: clang/test/CodeGen/misaligned-param.c:1
-// RUN: %clang_cc1 %s -triple i386-apple-darwin -emit-llvm -o - | FileCheck %s
 // Misaligned parameter must be memcpy'd to correctly aligned temporary.

Declare `int bar(struct s*, struct s*);` instead



Comment at: clang/test/Headers/arm-cmse-header-ns.c:1
-// RUN: %clang_cc1 -triple thumbv8m.base-eabi -fsyntax-only%s 2>&1 
| FileCheck --check-prefix=CHECK-c %s
+// RUN: %clang_cc1 -triple thumbv8m.base-eabi -fsyntax-only -std=c99 %s 
2>&1 | FileCheck --check-prefix=CHECK-c %s
 // RUN: not %clang_cc1 -triple thumbv8m.base-eabi -fsyntax-only -x c++ %s 2>&1 
| FileCheck --check-prefix=CHECK-cpp %s

Maybe better to edit the CHECK-c lines to expect an error?



Comment at: clang/test/Modules/modulemap-locations.m:2
 // RUN: rm -rf %t 
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I 
%S/Inputs/ModuleMapLocations/Module_ModuleMap -I 
%S/Inputs/ModuleMapLocations/Both -F %S/Inputs/ModuleMapLocations -I 
%S/Inputs/ModuleMapLocations -F %S/Inputs -x objective-c -fsyntax-only %s 
-verify -Wno-private-module
+// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps 
-fmodules-cache-path=%t -I %S/Inputs/ModuleMapLocations/Module_ModuleMap -I 
%S/Inputs/ModuleMapLocations/Both -F %S/Inputs/ModuleMapLocations -I 
%S/Inputs/ModuleMapLocations -F %S/Inputs -x objective-c -fsyntax-only %s 
-verify -Wno-private-module
 

Change 

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 421786.
aaron.ballman added a comment.

Rebased


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

Files:
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/ARCMT/objcmt-arc-cf-annotations.m
  clang/test/ARCMT/objcmt-arc-cf-annotations.m.result
  clang/test/Analysis/OSAtomic_mac.c
  clang/test/Analysis/ObjCProperties.m
  clang/test/Analysis/PR49642.c
  clang/test/Analysis/diagnostics/no-store-func-path-notes.c
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/novoidtypecrash.c
  clang/test/Analysis/plist-macros-with-expansion.c
  clang/test/CodeGen/2002-07-14-MiscTests3.c
  clang/test/CodeGen/2002-07-31-SubregFailure.c
  clang/test/CodeGen/2003-08-18-SigSetJmp.c
  clang/test/CodeGen/2004-11-27-StaticFunctionRedeclare.c
  clang/test/CodeGen/2005-01-02-ConstantInits.c
  clang/test/CodeGen/2005-01-02-VAArgError-ICE.c
  clang/test/CodeGen/2006-01-13-StackSave.c
  clang/test/CodeGen/2006-03-03-MissingInitializer.c
  clang/test/CodeGen/2008-05-12-TempUsedBeforeDef.c
  clang/test/CodeGen/2008-07-30-redef-of-bitcasted-decl.c
  clang/test/CodeGen/2008-08-19-cast-of-typedef.c
  clang/test/CodeGen/2008-10-13-FrontendCrash.c
  clang/test/CodeGen/PowerPC/builtins-ppc-p8vector.c
  clang/test/CodeGen/X86/bmi2-builtins.c
  clang/test/CodeGen/aarch64-mops.c
  clang/test/CodeGen/aarch64-neon-sm4-sm3.c
  clang/test/CodeGen/arm_acle.c
  clang/test/CodeGen/attribute_constructor.c
  clang/test/CodeGen/builtins-arm-microsoft.c
  clang/test/CodeGen/builtins-arm-msvc-compat-only.c
  clang/test/CodeGen/cast-emit.c
  clang/test/CodeGen/debug-info-block-vars.c
  clang/test/CodeGen/debug-info-crash.c
  clang/test/CodeGen/decl.c
  clang/test/CodeGen/init-with-member-expr.c
  clang/test/CodeGen/misaligned-param.c
  clang/test/CodeGen/neon-crypto.c
  clang/test/CodeGen/struct-comma.c
  clang/test/CodeGen/variable-array.c
  clang/test/Frontend/warning-mapping-2.c
  clang/test/Headers/arm-cmse-header-ns.c
  clang/test/Import/objc-arc/test-cleanup-object.m
  clang/test/Modules/config_macros.m
  clang/test/Modules/modulemap-locations.m
  clang/test/OpenMP/declare_mapper_messages.c
  clang/test/PCH/chain-macro-override.c
  clang/test/Rewriter/rewrite-foreach-2.m
  clang/test/Rewriter/rewrite-try-catch.m
  clang/test/Sema/__try.c
  clang/test/Sema/aarch64-tme-errors.c
  clang/test/Sema/arm-no-fp16.c
  clang/test/Sema/bitfield.c
  clang/test/Sema/builtin-setjmp.c
  clang/test/Sema/builtins.c
  clang/test/Sema/cxx-as-c.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/implicit-decl.c
  clang/test/Sema/implicit-ms-builtin-decl.c
  clang/test/Sema/typo-correction.c
  clang/test/Sema/vla.c
  clang/test/Sema/warn-strict-prototypes.c
  clang/test/VFS/module_missing_vfs.m
  compiler-rt/test/safestack/pthread-cleanup.c

Index: compiler-rt/test/safestack/pthread-cleanup.c
===
--- compiler-rt/test/safestack/pthread-cleanup.c
+++ compiler-rt/test/safestack/pthread-cleanup.c
@@ -17,6 +17,8 @@
   return buffer;
 }
 
+extern unsigned sleep(unsigned seconds);
+
 int main(int argc, char **argv)
 {
   int arg = atoi(argv[1]);
Index: clang/test/VFS/module_missing_vfs.m
===
--- clang/test/VFS/module_missing_vfs.m
+++ clang/test/VFS/module_missing_vfs.m
@@ -1,12 +1,12 @@
 // RUN: rm -rf %t && mkdir -p %t
 // RUN: echo "void funcA(void);" >> %t/a.h
 
-// RUN: not %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
+// RUN: not %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
 // ERROR: virtual filesystem overlay file '{{.*}}' not found
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/MissingVFS/vfsoverlay.yaml > %t/vfs.yaml
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
+// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 @import A;
Index: clang/test/Sema/warn-strict-prototypes.c
===
--- 

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-07 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3433919 , @erichkeane 
wrote:

> Up for review: https://reviews.llvm.org/D123241

Thank you @erichkeane! That landed in a87f4e4f9808a397dc4c575013142c9eac0b7eba 
, so it 
should no longer be blocking this patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-06 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

Up for review: https://reviews.llvm.org/D123241


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3433691 , @erichkeane 
wrote:

> In D122983#3433685 , @aaron.ballman 
> wrote:
>
>> The `-Wimplicit-function-declaration` already exists today, so I believe we 
>> can land the changes you have above to the test suite already today 
>> independent of this patch. Would you be interested in being nerd-sniped into 
>> doing that @erichkeane since you can actually run the tests? (I'm happy to 
>> support you in the work however I can, of course.)
>
> *GRUMBLE GRUMBLE GRUMBLE*.
> Sure, I can get on that in the next day or two.

:-D Thanks, I owe you Yet Another Beer!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-06 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

In D122983#3433685 , @aaron.ballman 
wrote:

> In D122983#3430706 , @erichkeane 
> wrote:
>
>> In D122983#3427518 , @xbolva00 
>> wrote:
>>
>>> Could you please check that https://github.com/llvm/llvm-test-suite is 
>>> buildable with your patch?
>>
>> Aaron wasn't able to get this working on his system, but I was.  I ended up 
>> collecting the following errors (though, if the build ended early and my 
>> workaround of adding the flag failed, i likely missed others):
>
> Thank you for the help with this, Erich!
>
>>   /iusers/ekeane1/workspaces/test-suite-build/tools/timeit --summary 
>> MultiSource/Applications/siod/CMakeFiles/siod.dir/slibu.c.o.time 
>> /iusers/ekeane1/workspaces/llvm-project2/build/bin/clang -DNDEBUG  -O3   -w 
>> -Werror=date-time -D__USE_MISC -D__USE_GNU -D__USE_SVID 
>> -D__USE_XOPEN_EXTENDED -D__USE_XOPEN -Dunix -MD -MT 
>> MultiSource/Applications/siod/CMakeFiles/siod.dir/slibu.c.o -MF 
>> MultiSource/Applications/siod/CMakeFiles/siod.dir/slibu.c.o.d -o 
>> MultiSource/Applications/siod/CMakeFiles/siod.dir/slibu.c.o   -c 
>> /iusers/ekeane1/workspaces/test-suite/MultiSource/Applications/siod/slibu.c
>>   
>> /iusers/ekeane1/workspaces/test-suite/MultiSource/Applications/siod/slibu.c:1720:6:
>>  error: implicit declaration of function 'strptime' is invalid in C99 and 
>> later and unsupported in C2x [-Wimplicit-function-declaration]
>>if (strptime(get_c_string(str),get_c_string(fmt),))
>>^
>>   1 error generated.
>>   
>>   
>>   
>>   
>>   /iusers/ekeane1/workspaces/test-suite-build/tools/timeit --summary 
>> MultiSource/Benchmarks/Prolangs-C/plot2fig/CMakeFiles/plot2fig.dir/fontname.c.o.time
>>  /iusers/ekeane1/workspaces/llvm-project2/build/bin/clang -DNDEBUG  -O3   -w 
>> -Werror=date-time -MD -MT 
>> MultiSource/Benchmarks/Prolangs-C/plot2fig/CMakeFiles/plot2fig.dir/fontname.c.o
>>  -MF 
>> MultiSource/Benchmarks/Prolangs-C/plot2fig/CMakeFiles/plot2fig.dir/fontname.c.o.d
>>  -o 
>> MultiSource/Benchmarks/Prolangs-C/plot2fig/CMakeFiles/plot2fig.dir/fontname.c.o
>>-c 
>> /iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/Prolangs-C/plot2fig/fontname.c
>>   
>> /iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/Prolangs-C/plot2fig/fontname.c:98:18:
>>  error: implicit declaration of function 're_comp' is invalid in C99 and 
>> later and unsupported in C2x [-Wimplicit-function-declaration]
>> res = (char *) re_comp (s);
>>^
>>   
>> /iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/Prolangs-C/plot2fig/fontname.c:106:7:
>>  error: implicit declaration of function 're_exec' is invalid in C99 and 
>> later and unsupported in C2x [-Wimplicit-function-declaration]
>> if (re_exec("typewriter")
>> ^
>>   2 errors generated.
>>   
>>   
>>   
>>   /iusers/ekeane1/workspaces/test-suite-build/tools/timeit --summary 
>> MultiSource/Benchmarks/Prolangs-C/archie-client/CMakeFiles/archie.dir/dirsend.c.o.time
>>  /iusers/ekeane1/workspaces/llvm-project2/build/bin/clang -DNDEBUG 
>> -I/iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/Prolangs-C/archie-client
>>  -O3   -w -Werror=date-time -MD -MT 
>> MultiSource/Benchmarks/Prolangs-C/archie-client/CMakeFiles/archie.dir/dirsend.c.o
>>  -MF 
>> MultiSource/Benchmarks/Prolangs-C/archie-client/CMakeFiles/archie.dir/dirsend.c.o.d
>>  -o 
>> MultiSource/Benchmarks/Prolangs-C/archie-client/CMakeFiles/archie.dir/dirsend.c.o
>>-c 
>> /iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/Prolangs-C/archie-client/dirsend.c
>>   
>> /iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/Prolangs-C/archie-client/dirsend.c:298:17:
>>  error: implicit declaration of function 'time' is invalid in C99 and later 
>> and unsupported in C2x [-Wimplicit-function-declaration]
>>   srand(getpid()+time(0)); /* XXX: arg ok, but not right type. */
>>  ^
>>   1 error generated.
>>   
>>   
>>   /iusers/ekeane1/workspaces/test-suite-build/tools/timeit --summary 
>> MultiSource/Benchmarks/MiBench/office-ispell/CMakeFiles/office-ispell.dir/term.c.o.time
>>  /iusers/ekeane1/workspaces/llvm-project2/build/bin/clang -DNDEBUG  -O3   -w 
>> -Werror=date-time -Dconst= -MD -MT 
>> MultiSource/Benchmarks/MiBench/office-ispell/CMakeFiles/office-ispell.dir/term.c.o
>>  -MF 
>> MultiSource/Benchmarks/MiBench/office-ispell/CMakeFiles/office-ispell.dir/term.c.o.d
>>  -o 
>> MultiSource/Benchmarks/MiBench/office-ispell/CMakeFiles/office-ispell.dir/term.c.o
>>-c 
>> /iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/MiBench/office-ispell/term.c
>>   
>> /iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/MiBench/office-ispell/term.c:519:9:
>>  error: implicit declaration of function 'wait' is invalid in C99 and later 
>> and unsupported in C2x 

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3430706 , @erichkeane 
wrote:

> In D122983#3427518 , @xbolva00 
> wrote:
>
>> Could you please check that https://github.com/llvm/llvm-test-suite is 
>> buildable with your patch?
>
> Aaron wasn't able to get this working on his system, but I was.  I ended up 
> collecting the following errors (though, if the build ended early and my 
> workaround of adding the flag failed, i likely missed others):

Thank you for the help with this, Erich!

>   /iusers/ekeane1/workspaces/test-suite-build/tools/timeit --summary 
> MultiSource/Applications/siod/CMakeFiles/siod.dir/slibu.c.o.time 
> /iusers/ekeane1/workspaces/llvm-project2/build/bin/clang -DNDEBUG  -O3   -w 
> -Werror=date-time -D__USE_MISC -D__USE_GNU -D__USE_SVID 
> -D__USE_XOPEN_EXTENDED -D__USE_XOPEN -Dunix -MD -MT 
> MultiSource/Applications/siod/CMakeFiles/siod.dir/slibu.c.o -MF 
> MultiSource/Applications/siod/CMakeFiles/siod.dir/slibu.c.o.d -o 
> MultiSource/Applications/siod/CMakeFiles/siod.dir/slibu.c.o   -c 
> /iusers/ekeane1/workspaces/test-suite/MultiSource/Applications/siod/slibu.c
>   
> /iusers/ekeane1/workspaces/test-suite/MultiSource/Applications/siod/slibu.c:1720:6:
>  error: implicit declaration of function 'strptime' is invalid in C99 and 
> later and unsupported in C2x [-Wimplicit-function-declaration]
>if (strptime(get_c_string(str),get_c_string(fmt),))
>^
>   1 error generated.
>   
>   
>   
>   
>   /iusers/ekeane1/workspaces/test-suite-build/tools/timeit --summary 
> MultiSource/Benchmarks/Prolangs-C/plot2fig/CMakeFiles/plot2fig.dir/fontname.c.o.time
>  /iusers/ekeane1/workspaces/llvm-project2/build/bin/clang -DNDEBUG  -O3   -w 
> -Werror=date-time -MD -MT 
> MultiSource/Benchmarks/Prolangs-C/plot2fig/CMakeFiles/plot2fig.dir/fontname.c.o
>  -MF 
> MultiSource/Benchmarks/Prolangs-C/plot2fig/CMakeFiles/plot2fig.dir/fontname.c.o.d
>  -o 
> MultiSource/Benchmarks/Prolangs-C/plot2fig/CMakeFiles/plot2fig.dir/fontname.c.o
>-c 
> /iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/Prolangs-C/plot2fig/fontname.c
>   
> /iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/Prolangs-C/plot2fig/fontname.c:98:18:
>  error: implicit declaration of function 're_comp' is invalid in C99 and 
> later and unsupported in C2x [-Wimplicit-function-declaration]
> res = (char *) re_comp (s);
>^
>   
> /iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/Prolangs-C/plot2fig/fontname.c:106:7:
>  error: implicit declaration of function 're_exec' is invalid in C99 and 
> later and unsupported in C2x [-Wimplicit-function-declaration]
> if (re_exec("typewriter")
> ^
>   2 errors generated.
>   
>   
>   
>   /iusers/ekeane1/workspaces/test-suite-build/tools/timeit --summary 
> MultiSource/Benchmarks/Prolangs-C/archie-client/CMakeFiles/archie.dir/dirsend.c.o.time
>  /iusers/ekeane1/workspaces/llvm-project2/build/bin/clang -DNDEBUG 
> -I/iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/Prolangs-C/archie-client
>  -O3   -w -Werror=date-time -MD -MT 
> MultiSource/Benchmarks/Prolangs-C/archie-client/CMakeFiles/archie.dir/dirsend.c.o
>  -MF 
> MultiSource/Benchmarks/Prolangs-C/archie-client/CMakeFiles/archie.dir/dirsend.c.o.d
>  -o 
> MultiSource/Benchmarks/Prolangs-C/archie-client/CMakeFiles/archie.dir/dirsend.c.o
>-c 
> /iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/Prolangs-C/archie-client/dirsend.c
>   
> /iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/Prolangs-C/archie-client/dirsend.c:298:17:
>  error: implicit declaration of function 'time' is invalid in C99 and later 
> and unsupported in C2x [-Wimplicit-function-declaration]
>   srand(getpid()+time(0)); /* XXX: arg ok, but not right type. */
>  ^
>   1 error generated.
>   
>   
>   /iusers/ekeane1/workspaces/test-suite-build/tools/timeit --summary 
> MultiSource/Benchmarks/MiBench/office-ispell/CMakeFiles/office-ispell.dir/term.c.o.time
>  /iusers/ekeane1/workspaces/llvm-project2/build/bin/clang -DNDEBUG  -O3   -w 
> -Werror=date-time -Dconst= -MD -MT 
> MultiSource/Benchmarks/MiBench/office-ispell/CMakeFiles/office-ispell.dir/term.c.o
>  -MF 
> MultiSource/Benchmarks/MiBench/office-ispell/CMakeFiles/office-ispell.dir/term.c.o.d
>  -o 
> MultiSource/Benchmarks/MiBench/office-ispell/CMakeFiles/office-ispell.dir/term.c.o
>-c 
> /iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/MiBench/office-ispell/term.c
>   
> /iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/MiBench/office-ispell/term.c:519:9:
>  error: implicit declaration of function 'wait' is invalid in C99 and later 
> and unsupported in C2x [-Wimplicit-function-declaration]
>   while (wait () != i)
>  ^
>   1 error generated.
>   
>   
>   /iusers/ekeane1/workspaces/test-suite-build/tools/timeit --summary 
> 

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-05 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

In D122983#3427518 , @xbolva00 wrote:

> Could you please check that https://github.com/llvm/llvm-test-suite is 
> buildable with your patch?

Aaron wasn't able to get this working on his system, but I was.  I ended up 
collecting the following errors (though, if the build ended early and my 
workaround of adding the flag failed, i likely missed others):

  /iusers/ekeane1/workspaces/test-suite-build/tools/timeit --summary 
MultiSource/Applications/siod/CMakeFiles/siod.dir/slibu.c.o.time 
/iusers/ekeane1/workspaces/llvm-project2/build/bin/clang -DNDEBUG  -O3   -w 
-Werror=date-time -D__USE_MISC -D__USE_GNU -D__USE_SVID -D__USE_XOPEN_EXTENDED 
-D__USE_XOPEN -Dunix -MD -MT 
MultiSource/Applications/siod/CMakeFiles/siod.dir/slibu.c.o -MF 
MultiSource/Applications/siod/CMakeFiles/siod.dir/slibu.c.o.d -o 
MultiSource/Applications/siod/CMakeFiles/siod.dir/slibu.c.o   -c 
/iusers/ekeane1/workspaces/test-suite/MultiSource/Applications/siod/slibu.c
  
/iusers/ekeane1/workspaces/test-suite/MultiSource/Applications/siod/slibu.c:1720:6:
 error: implicit declaration of function 'strptime' is invalid in C99 and later 
and unsupported in C2x [-Wimplicit-function-declaration]
   if (strptime(get_c_string(str),get_c_string(fmt),))
   ^
  1 error generated.
  
  
  
  
  /iusers/ekeane1/workspaces/test-suite-build/tools/timeit --summary 
MultiSource/Benchmarks/Prolangs-C/plot2fig/CMakeFiles/plot2fig.dir/fontname.c.o.time
 /iusers/ekeane1/workspaces/llvm-project2/build/bin/clang -DNDEBUG  -O3   -w 
-Werror=date-time -MD -MT 
MultiSource/Benchmarks/Prolangs-C/plot2fig/CMakeFiles/plot2fig.dir/fontname.c.o 
-MF 
MultiSource/Benchmarks/Prolangs-C/plot2fig/CMakeFiles/plot2fig.dir/fontname.c.o.d
 -o 
MultiSource/Benchmarks/Prolangs-C/plot2fig/CMakeFiles/plot2fig.dir/fontname.c.o 
  -c 
/iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/Prolangs-C/plot2fig/fontname.c
  
/iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/Prolangs-C/plot2fig/fontname.c:98:18:
 error: implicit declaration of function 're_comp' is invalid in C99 and later 
and unsupported in C2x [-Wimplicit-function-declaration]
res = (char *) re_comp (s);
   ^
  
/iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/Prolangs-C/plot2fig/fontname.c:106:7:
 error: implicit declaration of function 're_exec' is invalid in C99 and later 
and unsupported in C2x [-Wimplicit-function-declaration]
if (re_exec("typewriter")
^
  2 errors generated.
  
  
  
  /iusers/ekeane1/workspaces/test-suite-build/tools/timeit --summary 
MultiSource/Benchmarks/Prolangs-C/archie-client/CMakeFiles/archie.dir/dirsend.c.o.time
 /iusers/ekeane1/workspaces/llvm-project2/build/bin/clang -DNDEBUG 
-I/iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/Prolangs-C/archie-client
 -O3   -w -Werror=date-time -MD -MT 
MultiSource/Benchmarks/Prolangs-C/archie-client/CMakeFiles/archie.dir/dirsend.c.o
 -MF 
MultiSource/Benchmarks/Prolangs-C/archie-client/CMakeFiles/archie.dir/dirsend.c.o.d
 -o 
MultiSource/Benchmarks/Prolangs-C/archie-client/CMakeFiles/archie.dir/dirsend.c.o
   -c 
/iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/Prolangs-C/archie-client/dirsend.c
  
/iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/Prolangs-C/archie-client/dirsend.c:298:17:
 error: implicit declaration of function 'time' is invalid in C99 and later and 
unsupported in C2x [-Wimplicit-function-declaration]
  srand(getpid()+time(0)); /* XXX: arg ok, but not right type. */
 ^
  1 error generated.
  
  
  /iusers/ekeane1/workspaces/test-suite-build/tools/timeit --summary 
MultiSource/Benchmarks/MiBench/office-ispell/CMakeFiles/office-ispell.dir/term.c.o.time
 /iusers/ekeane1/workspaces/llvm-project2/build/bin/clang -DNDEBUG  -O3   -w 
-Werror=date-time -Dconst= -MD -MT 
MultiSource/Benchmarks/MiBench/office-ispell/CMakeFiles/office-ispell.dir/term.c.o
 -MF 
MultiSource/Benchmarks/MiBench/office-ispell/CMakeFiles/office-ispell.dir/term.c.o.d
 -o 
MultiSource/Benchmarks/MiBench/office-ispell/CMakeFiles/office-ispell.dir/term.c.o
   -c 
/iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/MiBench/office-ispell/term.c
  
/iusers/ekeane1/workspaces/test-suite/MultiSource/Benchmarks/MiBench/office-ispell/term.c:519:9:
 error: implicit declaration of function 'wait' is invalid in C99 and later and 
unsupported in C2x [-Wimplicit-function-declaration]
  while (wait () != i)
 ^
  1 error generated.
  
  
  /iusers/ekeane1/workspaces/test-suite-build/tools/timeit --summary 
SingleSource/Regression/C/gcc-c-torture/execute/CMakeFiles/GCC-C-execute-cmpsi-1.dir/cmpsi-1.c.o.time
 /iusers/ekeane1/workspaces/llvm-project2/build/bin/clang -DNDEBUG  -O3   -w 
-Werror=date-time -w -MD -MT 
SingleSource/Regression/C/gcc-c-torture/execute/CMakeFiles/GCC-C-execute-cmpsi-1.dir/cmpsi-1.c.o
 -MF 

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 420612.
aaron.ballman added a comment.

Rebase; NFC.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

Files:
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/ARCMT/objcmt-arc-cf-annotations.m
  clang/test/ARCMT/objcmt-arc-cf-annotations.m.result
  clang/test/Analysis/OSAtomic_mac.c
  clang/test/Analysis/ObjCProperties.m
  clang/test/Analysis/PR49642.c
  clang/test/Analysis/diagnostics/no-store-func-path-notes.c
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/novoidtypecrash.c
  clang/test/Analysis/plist-macros-with-expansion.c
  clang/test/CodeGen/2002-07-14-MiscTests3.c
  clang/test/CodeGen/2002-07-31-SubregFailure.c
  clang/test/CodeGen/2003-08-18-SigSetJmp.c
  clang/test/CodeGen/2004-11-27-StaticFunctionRedeclare.c
  clang/test/CodeGen/2005-01-02-ConstantInits.c
  clang/test/CodeGen/2005-01-02-VAArgError-ICE.c
  clang/test/CodeGen/2006-01-13-StackSave.c
  clang/test/CodeGen/2006-03-03-MissingInitializer.c
  clang/test/CodeGen/2008-05-12-TempUsedBeforeDef.c
  clang/test/CodeGen/2008-07-30-redef-of-bitcasted-decl.c
  clang/test/CodeGen/2008-08-19-cast-of-typedef.c
  clang/test/CodeGen/2008-10-13-FrontendCrash.c
  clang/test/CodeGen/PowerPC/builtins-ppc-p8vector.c
  clang/test/CodeGen/X86/bmi2-builtins.c
  clang/test/CodeGen/aarch64-mops.c
  clang/test/CodeGen/aarch64-neon-sm4-sm3.c
  clang/test/CodeGen/arm_acle.c
  clang/test/CodeGen/attribute_constructor.c
  clang/test/CodeGen/builtins-arm-microsoft.c
  clang/test/CodeGen/builtins-arm-msvc-compat-only.c
  clang/test/CodeGen/cast-emit.c
  clang/test/CodeGen/debug-info-block-vars.c
  clang/test/CodeGen/debug-info-crash.c
  clang/test/CodeGen/decl.c
  clang/test/CodeGen/init-with-member-expr.c
  clang/test/CodeGen/misaligned-param.c
  clang/test/CodeGen/neon-crypto.c
  clang/test/CodeGen/struct-comma.c
  clang/test/CodeGen/variable-array.c
  clang/test/Frontend/warning-mapping-2.c
  clang/test/Headers/arm-cmse-header-ns.c
  clang/test/Import/objc-arc/test-cleanup-object.m
  clang/test/Modules/config_macros.m
  clang/test/Modules/modulemap-locations.m
  clang/test/OpenMP/declare_mapper_messages.c
  clang/test/PCH/chain-macro-override.c
  clang/test/Rewriter/rewrite-foreach-2.m
  clang/test/Rewriter/rewrite-try-catch.m
  clang/test/Sema/__try.c
  clang/test/Sema/aarch64-tme-errors.c
  clang/test/Sema/arm-no-fp16.c
  clang/test/Sema/bitfield.c
  clang/test/Sema/builtin-setjmp.c
  clang/test/Sema/builtins.c
  clang/test/Sema/cxx-as-c.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/implicit-decl.c
  clang/test/Sema/implicit-ms-builtin-decl.c
  clang/test/Sema/typo-correction.c
  clang/test/Sema/vla.c
  clang/test/Sema/warn-strict-prototypes.c
  clang/test/VFS/module_missing_vfs.m
  compiler-rt/test/safestack/pthread-cleanup.c

Index: compiler-rt/test/safestack/pthread-cleanup.c
===
--- compiler-rt/test/safestack/pthread-cleanup.c
+++ compiler-rt/test/safestack/pthread-cleanup.c
@@ -17,6 +17,8 @@
   return buffer;
 }
 
+extern unsigned sleep(unsigned seconds);
+
 int main(int argc, char **argv)
 {
   int arg = atoi(argv[1]);
Index: clang/test/VFS/module_missing_vfs.m
===
--- clang/test/VFS/module_missing_vfs.m
+++ clang/test/VFS/module_missing_vfs.m
@@ -1,12 +1,12 @@
 // RUN: rm -rf %t && mkdir -p %t
 // RUN: echo "void funcA(void);" >> %t/a.h
 
-// RUN: not %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
+// RUN: not %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
 // ERROR: virtual filesystem overlay file '{{.*}}' not found
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/MissingVFS/vfsoverlay.yaml > %t/vfs.yaml
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
+// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 @import A;
Index: clang/test/Sema/warn-strict-prototypes.c
===
--- 

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-05 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment.

In D122983#3429060 , @aaron.ballman 
wrote:

> Morally, yes, that's reasonable in CodeGen because you're ensuring you get no 
> diagnostics. Practically, that's a convoluted, more expensive, less 
> maintainable way to spell `-Werror` for the test. When diagnostics are 
> introduced, this pattern encourages people to remove the `// 
> expected-no-diagnostics` comment and start adding `// expected-warning {{}}` 
> comments. Running the diagnostic verifier also slows down test execution 
> because of the extra verification step (which adds up over thousands of 
> tests).

Thanks for the tip. I never thought to use `-Werror` that way.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 420488.
aaron.ballman added a comment.

Fixing more precommit CI failures. The `ScudoWrappersCTest.MallocInfo` do not 
appear to be caused by this patch, however.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

Files:
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/ARCMT/objcmt-arc-cf-annotations.m
  clang/test/ARCMT/objcmt-arc-cf-annotations.m.result
  clang/test/Analysis/OSAtomic_mac.c
  clang/test/Analysis/ObjCProperties.m
  clang/test/Analysis/PR49642.c
  clang/test/Analysis/diagnostics/no-store-func-path-notes.c
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/novoidtypecrash.c
  clang/test/Analysis/plist-macros-with-expansion.c
  clang/test/CodeGen/2002-07-14-MiscTests3.c
  clang/test/CodeGen/2002-07-31-SubregFailure.c
  clang/test/CodeGen/2003-08-18-SigSetJmp.c
  clang/test/CodeGen/2004-11-27-StaticFunctionRedeclare.c
  clang/test/CodeGen/2005-01-02-ConstantInits.c
  clang/test/CodeGen/2005-01-02-VAArgError-ICE.c
  clang/test/CodeGen/2006-01-13-StackSave.c
  clang/test/CodeGen/2006-03-03-MissingInitializer.c
  clang/test/CodeGen/2008-05-12-TempUsedBeforeDef.c
  clang/test/CodeGen/2008-07-30-redef-of-bitcasted-decl.c
  clang/test/CodeGen/2008-08-19-cast-of-typedef.c
  clang/test/CodeGen/2008-10-13-FrontendCrash.c
  clang/test/CodeGen/PowerPC/builtins-ppc-p8vector.c
  clang/test/CodeGen/X86/bmi2-builtins.c
  clang/test/CodeGen/aarch64-mops.c
  clang/test/CodeGen/aarch64-neon-sm4-sm3.c
  clang/test/CodeGen/arm_acle.c
  clang/test/CodeGen/attribute_constructor.c
  clang/test/CodeGen/builtins-arm-microsoft.c
  clang/test/CodeGen/builtins-arm-msvc-compat-only.c
  clang/test/CodeGen/cast-emit.c
  clang/test/CodeGen/debug-info-block-vars.c
  clang/test/CodeGen/debug-info-crash.c
  clang/test/CodeGen/decl.c
  clang/test/CodeGen/init-with-member-expr.c
  clang/test/CodeGen/misaligned-param.c
  clang/test/CodeGen/neon-crypto.c
  clang/test/CodeGen/struct-comma.c
  clang/test/CodeGen/variable-array.c
  clang/test/Frontend/warning-mapping-2.c
  clang/test/Headers/arm-cmse-header-ns.c
  clang/test/Import/objc-arc/test-cleanup-object.m
  clang/test/Modules/config_macros.m
  clang/test/Modules/modulemap-locations.m
  clang/test/OpenMP/declare_mapper_messages.c
  clang/test/PCH/chain-macro-override.c
  clang/test/Rewriter/rewrite-foreach-2.m
  clang/test/Rewriter/rewrite-try-catch.m
  clang/test/Sema/__try.c
  clang/test/Sema/aarch64-tme-errors.c
  clang/test/Sema/arm-no-fp16.c
  clang/test/Sema/bitfield.c
  clang/test/Sema/builtin-setjmp.c
  clang/test/Sema/builtins.c
  clang/test/Sema/cxx-as-c.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/implicit-decl.c
  clang/test/Sema/implicit-ms-builtin-decl.c
  clang/test/Sema/typo-correction.c
  clang/test/Sema/vla.c
  clang/test/Sema/warn-strict-prototypes.c
  clang/test/VFS/module_missing_vfs.m
  compiler-rt/test/safestack/pthread-cleanup.c

Index: compiler-rt/test/safestack/pthread-cleanup.c
===
--- compiler-rt/test/safestack/pthread-cleanup.c
+++ compiler-rt/test/safestack/pthread-cleanup.c
@@ -17,6 +17,8 @@
   return buffer;
 }
 
+extern unsigned sleep(unsigned seconds);
+
 int main(int argc, char **argv)
 {
   int arg = atoi(argv[1]);
Index: clang/test/VFS/module_missing_vfs.m
===
--- clang/test/VFS/module_missing_vfs.m
+++ clang/test/VFS/module_missing_vfs.m
@@ -1,12 +1,12 @@
 // RUN: rm -rf %t && mkdir -p %t
 // RUN: echo "void funcA(void);" >> %t/a.h
 
-// RUN: not %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
+// RUN: not %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
 // ERROR: virtual filesystem overlay file '{{.*}}' not found
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/MissingVFS/vfsoverlay.yaml > %t/vfs.yaml
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
+// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 @import A;
Index: 

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3427518 , @xbolva00 wrote:

> Could you please check that https://github.com/llvm/llvm-test-suite is 
> buildable with your patch?

I gave it a shot just to see, but I'm unable to build it even without my patch:

  F:\source\test-suite-build>cmake --build . --clean-first
  Microsoft (R) Build Engine version 16.11.2+f32259642 for .NET Framework
  Copyright (C) Microsoft Corporation. All rights reserved.
  
  F:\source\test-suite-build\test-suite.sln : Solution file error MSB5004: The 
solution file has two projects named "pa
  thfinder".
  Microsoft (R) Build Engine version 16.11.2+f32259642 for .NET Framework
  Copyright (C) Microsoft Corporation. All rights reserved.
  
[TEST_SUITE_HOST_CC] Compiling host source fpcmp.c
'cc' is not recognized as an internal or external command,
operable program or batch file.
  C:\Program Files (x86)\Microsoft Visual 
Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.CppCommon.target
  s(241,5): error MSB8066: Custom build for 
'F:\source\test-suite-build\CMakeFiles\a30a18e8f3bb5de504f740ee3822d8a0\fpc
  
mp.c.o.rule;F:\source\test-suite-build\CMakeFiles\a30a18e8f3bb5de504f740ee3822d8a0\fpcmp.rule;F:\source\test-suite-bu
  ild\CMakeFiles\22099cfe59041de58024b8c82a571139\build-fpcmp.rule' exited with 
code 9009. [F:\source\test-suite-build\
  tools\build-fpcmp.vcxproj]
[TEST_SUITE_HOST_CC] Compiling host source timeit.c
'cc' is not recognized as an internal or external command,
operable program or batch file.
  C:\Program Files (x86)\Microsoft Visual 
Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.CppCommon.target
  s(241,5): error MSB8066: Custom build for 
'F:\source\test-suite-build\CMakeFiles\a30a18e8f3bb5de504f740ee3822d8a0\tim
  
eit.c.o.rule;F:\source\test-suite-build\CMakeFiles\a30a18e8f3bb5de504f740ee3822d8a0\timeit.rule;F:\source\test-suite-
  build\CMakeFiles\22099cfe59041de58024b8c82a571139\build-timeit.rule' exited 
with code 9009. [F:\source\test-suite-bui
  ld\tools\build-timeit.vcxproj]
  cl : command line warning D9025: overriding '/W4' with '/w' 
[F:\source\test-suite-build\MicroBenchmarks\libs\benchmar
  k\third_party\googletest\build\googlemock\gmock_main.vcxproj]
  cl : command line error D8021: invalid numeric argument '/Werror=date-time' 
[F:\source\test-suite-build\MicroBenchmar
  ks\libs\benchmark\third_party\googletest\build\googlemock\gmock_main.vcxproj]
  cl : command line warning D9025: overriding '/W3' with '/w' 
[F:\source\test-suite-build\tools\fpcmp-target.vcxproj]
  cl : command line error D8021: invalid numeric argument '/Werror=date-time' 
[F:\source\test-suite-build\tools\fpcmp-t
  arget.vcxproj]
Generating sqlite test inputs
'TCL_TCLSH-NOTFOUND' is not recognized as an internal or external command,
operable program or batch file.
  C:\Program Files (x86)\Microsoft Visual 
Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.CppCommon.target
  s(241,5): error MSB8066: Custom build for 
'F:\source\test-suite-build\CMakeFiles\47602658443eaf701f82747ba4e9979c\tes
  t15.sql.rule' exited with code 9009. 
[F:\source\test-suite-build\MultiSource\Applications\sqlite3\sqlite_input.vcxpro
  j]
  cl : command line warning D9025: overriding '/W3' with '/w' 
[F:\source\test-suite-build\tools\timeit-target.vcxproj]
  cl : command line error D8021: invalid numeric argument '/Werror=date-time' 
[F:\source\test-suite-build\tools\timeit-
  target.vcxproj]

FWIW, this was how I configured, and that step did not generate any errors for 
me:

  cmake 
-DCMAKE_C_COMPILER=F:\source\llvm-project\llvm\out\build\x64-Debug\bin\clang.exe
 
-DCMAKE_CXX_COMPILER="F:\source\llvm-project\llvm\out\build\x64-Debug\bin\clang++.exe"
 -C..\test-suite\cmake\caches\O3.cmake -DTEST_SUITE_COLLECT_CODE_SIZE=OFF 
..\test-suite

So I'm not certain what's going on there.

In D122983#3427677 , 
@hubert.reinterpretcast wrote:

> In D122983#3426716 , @erichkeane 
> wrote:
>
>> We typically avoid doing -verify in CodeGen (unless the diagnostic is 
>> ACTUALLY in CodeGen) as a matter of business.
>
> I hope that `-verify` and `// expected-no-diagnostics` in CodeGen tests is 
> compatible with the above. I believe it is valuable to confirm that the test 
> itself is not written problematically.

Morally, yes, that's reasonable in CodeGen because you're ensuring you get no 
diagnostics. Practically, that's a convoluted, more expensive, less 
maintainable way to spell `-Werror` for the test. When diagnostics are 
introduced, this pattern encourages people to remove the `// 
expected-no-diagnostics` comment and start adding `// expected-warning {{}}` 
comments. Running the diagnostic verifier also slows down test execution 
because of the extra verification step (which adds up over thousands of tests).


CHANGES SINCE LAST ACTION
  

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 420446.
aaron.ballman added a comment.

Fixing the clangd include fixer unit tests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

Files:
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/ARCMT/objcmt-arc-cf-annotations.m
  clang/test/ARCMT/objcmt-arc-cf-annotations.m.result
  clang/test/Analysis/OSAtomic_mac.c
  clang/test/Analysis/ObjCProperties.m
  clang/test/Analysis/PR49642.c
  clang/test/Analysis/diagnostics/no-store-func-path-notes.c
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/novoidtypecrash.c
  clang/test/Analysis/plist-macros-with-expansion.c
  clang/test/CodeGen/2002-07-14-MiscTests3.c
  clang/test/CodeGen/2002-07-31-SubregFailure.c
  clang/test/CodeGen/2003-08-18-SigSetJmp.c
  clang/test/CodeGen/2004-11-27-StaticFunctionRedeclare.c
  clang/test/CodeGen/2005-01-02-ConstantInits.c
  clang/test/CodeGen/2005-01-02-VAArgError-ICE.c
  clang/test/CodeGen/2006-01-13-StackSave.c
  clang/test/CodeGen/2006-03-03-MissingInitializer.c
  clang/test/CodeGen/2008-05-12-TempUsedBeforeDef.c
  clang/test/CodeGen/2008-07-30-redef-of-bitcasted-decl.c
  clang/test/CodeGen/2008-08-19-cast-of-typedef.c
  clang/test/CodeGen/2008-10-13-FrontendCrash.c
  clang/test/CodeGen/PowerPC/builtins-ppc-p8vector.c
  clang/test/CodeGen/X86/bmi2-builtins.c
  clang/test/CodeGen/aarch64-mops.c
  clang/test/CodeGen/aarch64-neon-sm4-sm3.c
  clang/test/CodeGen/arm_acle.c
  clang/test/CodeGen/attribute_constructor.c
  clang/test/CodeGen/builtins-arm-microsoft.c
  clang/test/CodeGen/builtins-arm-msvc-compat-only.c
  clang/test/CodeGen/cast-emit.c
  clang/test/CodeGen/debug-info-block-vars.c
  clang/test/CodeGen/debug-info-crash.c
  clang/test/CodeGen/decl.c
  clang/test/CodeGen/init-with-member-expr.c
  clang/test/CodeGen/misaligned-param.c
  clang/test/CodeGen/neon-crypto.c
  clang/test/CodeGen/struct-comma.c
  clang/test/CodeGen/variable-array.c
  clang/test/Frontend/warning-mapping-2.c
  clang/test/Headers/arm-cmse-header-ns.c
  clang/test/Import/objc-arc/test-cleanup-object.m
  clang/test/Modules/config_macros.m
  clang/test/Modules/modulemap-locations.m
  clang/test/OpenMP/declare_mapper_messages.c
  clang/test/PCH/chain-macro-override.c
  clang/test/Rewriter/rewrite-foreach-2.m
  clang/test/Rewriter/rewrite-try-catch.m
  clang/test/Sema/__try.c
  clang/test/Sema/aarch64-tme-errors.c
  clang/test/Sema/arm-no-fp16.c
  clang/test/Sema/bitfield.c
  clang/test/Sema/builtin-setjmp.c
  clang/test/Sema/builtins.c
  clang/test/Sema/cxx-as-c.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/implicit-decl.c
  clang/test/Sema/implicit-ms-builtin-decl.c
  clang/test/Sema/typo-correction.c
  clang/test/Sema/vla.c
  clang/test/Sema/warn-strict-prototypes.c
  clang/test/VFS/module_missing_vfs.m

Index: clang/test/VFS/module_missing_vfs.m
===
--- clang/test/VFS/module_missing_vfs.m
+++ clang/test/VFS/module_missing_vfs.m
@@ -1,12 +1,12 @@
 // RUN: rm -rf %t && mkdir -p %t
 // RUN: echo "void funcA(void);" >> %t/a.h
 
-// RUN: not %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
+// RUN: not %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
 // ERROR: virtual filesystem overlay file '{{.*}}' not found
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/MissingVFS/vfsoverlay.yaml > %t/vfs.yaml
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
+// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 @import A;
Index: clang/test/Sema/warn-strict-prototypes.c
===
--- clang/test/Sema/warn-strict-prototypes.c
+++ clang/test/Sema/warn-strict-prototypes.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -Wno-implicit-function-declaration -verify %s
-// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple 

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment.

In D122983#3426716 , @erichkeane 
wrote:

> We typically avoid doing -verify in CodeGen (unless the diagnostic is 
> ACTUALLY in CodeGen) as a matter of business.

I hope that `-verify` and `// expected-no-diagnostics` in CodeGen tests is 
compatible with the above. I believe it is valuable to confirm that the test 
itself is not written problematically.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

Could you please check that https://github.com/llvm/llvm-test-suite is 
buildable with your patch?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 420290.
aaron.ballman added a comment.

Fixed a failing test case caught by precommit CI.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

Files:
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/ARCMT/objcmt-arc-cf-annotations.m
  clang/test/ARCMT/objcmt-arc-cf-annotations.m.result
  clang/test/Analysis/OSAtomic_mac.c
  clang/test/Analysis/ObjCProperties.m
  clang/test/Analysis/PR49642.c
  clang/test/Analysis/diagnostics/no-store-func-path-notes.c
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/novoidtypecrash.c
  clang/test/Analysis/plist-macros-with-expansion.c
  clang/test/CodeGen/2002-07-14-MiscTests3.c
  clang/test/CodeGen/2002-07-31-SubregFailure.c
  clang/test/CodeGen/2003-08-18-SigSetJmp.c
  clang/test/CodeGen/2004-11-27-StaticFunctionRedeclare.c
  clang/test/CodeGen/2005-01-02-ConstantInits.c
  clang/test/CodeGen/2005-01-02-VAArgError-ICE.c
  clang/test/CodeGen/2006-01-13-StackSave.c
  clang/test/CodeGen/2006-03-03-MissingInitializer.c
  clang/test/CodeGen/2008-05-12-TempUsedBeforeDef.c
  clang/test/CodeGen/2008-07-30-redef-of-bitcasted-decl.c
  clang/test/CodeGen/2008-08-19-cast-of-typedef.c
  clang/test/CodeGen/2008-10-13-FrontendCrash.c
  clang/test/CodeGen/PowerPC/builtins-ppc-p8vector.c
  clang/test/CodeGen/X86/bmi2-builtins.c
  clang/test/CodeGen/aarch64-mops.c
  clang/test/CodeGen/aarch64-neon-sm4-sm3.c
  clang/test/CodeGen/arm_acle.c
  clang/test/CodeGen/attribute_constructor.c
  clang/test/CodeGen/builtins-arm-microsoft.c
  clang/test/CodeGen/builtins-arm-msvc-compat-only.c
  clang/test/CodeGen/cast-emit.c
  clang/test/CodeGen/debug-info-block-vars.c
  clang/test/CodeGen/debug-info-crash.c
  clang/test/CodeGen/decl.c
  clang/test/CodeGen/init-with-member-expr.c
  clang/test/CodeGen/misaligned-param.c
  clang/test/CodeGen/neon-crypto.c
  clang/test/CodeGen/struct-comma.c
  clang/test/CodeGen/variable-array.c
  clang/test/Frontend/warning-mapping-2.c
  clang/test/Headers/arm-cmse-header-ns.c
  clang/test/Import/objc-arc/test-cleanup-object.m
  clang/test/Modules/config_macros.m
  clang/test/Modules/modulemap-locations.m
  clang/test/OpenMP/declare_mapper_messages.c
  clang/test/PCH/chain-macro-override.c
  clang/test/Rewriter/rewrite-foreach-2.m
  clang/test/Rewriter/rewrite-try-catch.m
  clang/test/Sema/__try.c
  clang/test/Sema/aarch64-tme-errors.c
  clang/test/Sema/arm-no-fp16.c
  clang/test/Sema/bitfield.c
  clang/test/Sema/builtin-setjmp.c
  clang/test/Sema/builtins.c
  clang/test/Sema/cxx-as-c.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/implicit-decl.c
  clang/test/Sema/implicit-ms-builtin-decl.c
  clang/test/Sema/typo-correction.c
  clang/test/Sema/vla.c
  clang/test/Sema/warn-strict-prototypes.c
  clang/test/VFS/module_missing_vfs.m

Index: clang/test/VFS/module_missing_vfs.m
===
--- clang/test/VFS/module_missing_vfs.m
+++ clang/test/VFS/module_missing_vfs.m
@@ -1,12 +1,12 @@
 // RUN: rm -rf %t && mkdir -p %t
 // RUN: echo "void funcA(void);" >> %t/a.h
 
-// RUN: not %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
+// RUN: not %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
 // ERROR: virtual filesystem overlay file '{{.*}}' not found
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/MissingVFS/vfsoverlay.yaml > %t/vfs.yaml
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
+// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 @import A;
Index: clang/test/Sema/warn-strict-prototypes.c
===
--- clang/test/Sema/warn-strict-prototypes.c
+++ clang/test/Sema/warn-strict-prototypes.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -Wno-implicit-function-declaration -verify %s
-// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes 

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 420242.
aaron.ballman added a comment.
Herald added subscribers: kbarton, nemanjai.

Updated the release note and hopefully addressed the remaining test failures 
found by precommit CI.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

Files:
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/ARCMT/objcmt-arc-cf-annotations.m
  clang/test/ARCMT/objcmt-arc-cf-annotations.m.result
  clang/test/Analysis/OSAtomic_mac.c
  clang/test/Analysis/ObjCProperties.m
  clang/test/Analysis/PR49642.c
  clang/test/Analysis/diagnostics/no-store-func-path-notes.c
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/novoidtypecrash.c
  clang/test/Analysis/plist-macros-with-expansion.c
  clang/test/CodeGen/2002-07-14-MiscTests3.c
  clang/test/CodeGen/2002-07-31-SubregFailure.c
  clang/test/CodeGen/2003-08-18-SigSetJmp.c
  clang/test/CodeGen/2004-11-27-StaticFunctionRedeclare.c
  clang/test/CodeGen/2005-01-02-ConstantInits.c
  clang/test/CodeGen/2005-01-02-VAArgError-ICE.c
  clang/test/CodeGen/2006-01-13-StackSave.c
  clang/test/CodeGen/2006-03-03-MissingInitializer.c
  clang/test/CodeGen/2008-05-12-TempUsedBeforeDef.c
  clang/test/CodeGen/2008-07-30-redef-of-bitcasted-decl.c
  clang/test/CodeGen/2008-08-19-cast-of-typedef.c
  clang/test/CodeGen/2008-10-13-FrontendCrash.c
  clang/test/CodeGen/PowerPC/builtins-ppc-p8vector.c
  clang/test/CodeGen/X86/bmi2-builtins.c
  clang/test/CodeGen/aarch64-mops.c
  clang/test/CodeGen/aarch64-neon-sm4-sm3.c
  clang/test/CodeGen/arm_acle.c
  clang/test/CodeGen/attribute_constructor.c
  clang/test/CodeGen/builtins-arm-microsoft.c
  clang/test/CodeGen/builtins-arm-msvc-compat-only.c
  clang/test/CodeGen/cast-emit.c
  clang/test/CodeGen/debug-info-block-vars.c
  clang/test/CodeGen/debug-info-crash.c
  clang/test/CodeGen/decl.c
  clang/test/CodeGen/init-with-member-expr.c
  clang/test/CodeGen/misaligned-param.c
  clang/test/CodeGen/neon-crypto.c
  clang/test/CodeGen/struct-comma.c
  clang/test/CodeGen/variable-array.c
  clang/test/Frontend/warning-mapping-2.c
  clang/test/Headers/arm-cmse-header-ns.c
  clang/test/Import/objc-arc/test-cleanup-object.m
  clang/test/Modules/config_macros.m
  clang/test/Modules/modulemap-locations.m
  clang/test/OpenMP/declare_mapper_messages.c
  clang/test/PCH/chain-macro-override.c
  clang/test/Rewriter/rewrite-foreach-2.m
  clang/test/Rewriter/rewrite-try-catch.m
  clang/test/Sema/__try.c
  clang/test/Sema/aarch64-tme-errors.c
  clang/test/Sema/arm-no-fp16.c
  clang/test/Sema/bitfield.c
  clang/test/Sema/builtin-setjmp.c
  clang/test/Sema/builtins.c
  clang/test/Sema/cxx-as-c.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/implicit-decl.c
  clang/test/Sema/implicit-ms-builtin-decl.c
  clang/test/Sema/typo-correction.c
  clang/test/Sema/vla.c
  clang/test/Sema/warn-strict-prototypes.c
  clang/test/VFS/module_missing_vfs.m

Index: clang/test/VFS/module_missing_vfs.m
===
--- clang/test/VFS/module_missing_vfs.m
+++ clang/test/VFS/module_missing_vfs.m
@@ -1,12 +1,12 @@
 // RUN: rm -rf %t && mkdir -p %t
 // RUN: echo "void funcA(void);" >> %t/a.h
 
-// RUN: not %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
+// RUN: not %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
 // ERROR: virtual filesystem overlay file '{{.*}}' not found
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/MissingVFS/vfsoverlay.yaml > %t/vfs.yaml
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
+// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 @import A;
Index: clang/test/Sema/warn-strict-prototypes.c
===
--- clang/test/Sema/warn-strict-prototypes.c
+++ clang/test/Sema/warn-strict-prototypes.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -Wno-implicit-function-declaration -verify %s
-// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | 

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

I pushed 5d90004874c7b040cd43597826aabb34757d25ab 
 to 
hopefully address the lion's share of the currently failing precommit CI tests. 
I'll address the remaining few as an update to this patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3426739 , @paulwalker-arm 
wrote:

> Ok, message understood.  I'll try and get these cleaned up, I cannot say 
> precisely when but will push for before we branch for 15 if that sounds 
> sensible.

Oops, sorry, I didn't see this came in while I was also responding. :-) That 
would be fantastic and sounds like a very reasonable timeframe. I don't expect 
this to be a Real Issue until we go to change to C23 by default, which is still 
likely to be a few years away. Thank you for looking into it, and thanks again 
for speaking up about the potential loss of test coverage!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3426661 , @paulwalker-arm 
wrote:

> Thanks for this.

Any time, thank you for speaking up when I was about to drop test coverage by 
accident!

In D122983#3426716 , @erichkeane 
wrote:

> In D122983#3426661 , 
> @paulwalker-arm wrote:
>
>> Thanks for this.  I can see about cleaning up the tests but I'm still not 
>> sure what the advantage is.  The affected RUN lines are already 
>> `-fsyntax-only` tests.  Is it about where the test files live or is there 
>> something else I should be considering?  The benefit of the current tests is 
>> that it's easy to spot holes as a single function tests both requirements.  
>> My fear is that separating the tests based on Sema/CodeGen could mean we'll 
>> miss something and never know.
>
> There are two big problems with the way these are split up.  First, the 
> 'list' of these functions that you expect to not be declared under a flag 
> don't exist all in 1 place, and end up being difficult to follow/track down 
> as they are spread out as much as they are.  Second, they are in tests that 
> have a "REQUIRES" clause in it, which causes them to not be caught in many 
> configurations.  We typically avoid doing -verify in CodeGen (unless the 
> diagnostic is ACTUALLY in CodeGen) as a matter of business.  The diagnostic 
> you're using to validate these is likely a poor choice (as it is a 
> non-standard extension based warning) though, which is what makes this so 
> difficult on folks trying to modernize Clang to follow standards.  IMO, if 
> these had been C++ tests (which would have 'not found' failures), or 
> validated the existence of these in some other mechanism, we likely would 
> have never noticed other than via an audit.

+1 to this.

It's also about expectations -- the test directories are set up to be largely 
reflective of the libraries which comprise Clang and the contents of those 
directories are intended to test that particular component and not others. 
Mixing codegen and sema tests winds up being a surprise quite often when 
working on diagnostics because running the *whole* test suite is expensive 
while running just the Sema tests is quite quick. Then we can run the whole 
test suite once locally when we're finished changing diagnostics, just in case 
we missed stuff. So already, codegen tests with diagnostic checks come as a 
surprise from that workflow. But as Erich points out, these particular tests 
don't run everywhere to begin with, so it came as a rather unpleasant surprise 
that there's ~200 more tests impacted by changing the diagnostic. It would have 
been far less painful if these were in a single file in Sema -- we'd only have 
to fix it once instead of ~200 times and we'd catch the diagnostic regardless 
of what targets are registered.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Paul Walker via Phabricator via cfe-commits
paulwalker-arm added a comment.

Ok, message understood.  I'll try and get these cleaned up, I cannot say 
precisely when but will push for before we branch for 15 if that sounds 
sensible.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

In D122983#3426661 , @paulwalker-arm 
wrote:

> Thanks for this.  I can see about cleaning up the tests but I'm still not 
> sure what the advantage is.  The affected RUN lines are already 
> `-fsyntax-only` tests.  Is it about where the test files live or is there 
> something else I should be considering?  The benefit of the current tests is 
> that it's easy to spot holes as a single function tests both requirements.  
> My fear is that separating the tests based on Sema/CodeGen could mean we'll 
> miss something and never know.

There are two big problems with the way these are split up.  First, the 'list' 
of these functions that you expect to not be declared under a flag don't exist 
all in 1 place, and end up being difficult to follow/track down as they are 
spread out as much as they are.  Second, they are in tests that have a 
"REQUIRES" clause in it, which causes them to not be caught in many 
configurations.  We typically avoid doing -verify in CodeGen (unless the 
diagnostic is ACTUALLY in CodeGen) as a matter of business.  The diagnostic 
you're using to validate these is likely a poor choice (as it is a non-standard 
extension based warning) though, which is what makes this so difficult on folks 
trying to modernize Clang to follow standards.  IMO, if these had been C++ 
tests (which would have 'not found' failures), or validated the existence of 
these in some other mechanism, we likely would have never noticed other than 
via an audit.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Paul Walker via Phabricator via cfe-commits
paulwalker-arm added a comment.

Thanks for this.  I can see about cleaning up the tests but I'm still not sure 
what the advantage is.  The affected RUN lines are already `-fsyntax-only` 
tests.  Is it about where the test files live or is there something else I 
should be considering?  The benefit of the current tests is that it's easy to 
spot holes as a single function tests both requirements.  My fear is that 
separating the tests based on Sema/CodeGen could mean we'll miss something and 
never know.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D122983#3426541 , @paulwalker-arm 
wrote:

> The tests verify a set of builtins do not exist when the associated feature 
> flag is not present.  They sit within CodeGen because the tests were 
> plentiful and it did not seem worth duplicating them.

You don't need to duplicate hundreds of tests to have appropriate Sema tests 
for this functionality. You could have added a single test file to test/Sema 
that tests each of the builtins you care about the semantic diagnostic behavior 
of. Doing this from codegen on a hundreds of individual tests that don't run 
everywhrere is disruptive because it makes modifying diagnostic behavior 
considerably harder than it should be. We separate codegen and sema tests for a 
reason.

I'll leave the test coverage but add `-std=c99` to the verify lines because 
that will unblock this patch. However, I'd still appreciate if the AArch64 
maintainers would clean up these tests in a more appropriate fashion, because 
adding the -std= line also loses test coverage (especially when we switch the 
default language mode). This will need to happen sooner rather than later 
(relatively speaking; nothing is on fire requiring immediate attention) -- I 
expect we'll bump clang's default from C17 to C23 sometime in the next few 
years, at which point all of these tests will fail to compile because there 
will be no support for implicit function declarations whatsoever.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment.

In D122983#3426541 , @paulwalker-arm 
wrote:

> The tests verify a set of builtins do not exist when the associated feature 
> flag is not present.  They sit within CodeGen because the tests were 
> plentiful and it did not seem worth duplicating them.

This sounds like a situation where the test files should have more comments 
near the RUN lines (if the situation is not resolved via a different solution).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment.

In D122983#3426450 , @aaron.ballman 
wrote:

> Yeah, those tests seem to be overly-constraining. There's no reason to be 
> validating whether there's an implicit function declaration warning in a 
> *codegen* test. I will change all of those AAarch64 tests to require -std=c99 
> explicitly whenever possible, remove the `-verify` flag because there's no 
> reason for a codegen test to verify diagnostic behavior that isn't generated 
> by the CodeGen library, and remove the `// expected-warning` comments. I plan 
> to do that as an NFC change that I'll land outside of this patch, unless any 
> of the AArch64 folks speak up pretty quickly.

Maybe a smaller hammer can be used here, e.g., 
`-Wno-error=implicit-function-declaration`? The use of the `-verify` flag in a 
CodeGen test to validate that the test is written as "cleanly" as intended is 
something that I am sympathetic to.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Paul Walker via Phabricator via cfe-commits
paulwalker-arm added a comment.

The tests verify a set of builtins do not exist when the associated feature 
flag is not present.  They sit within CodeGen because the tests were plentiful 
and it did not seem worth duplicating them.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.

In D122983#3426534 , @paulwalker-arm 
wrote:

> Please consider this "AArch64 folks speaking up".  What are your plans here 
> exactly? I have no issue with adding `-std=c99` to the RUN lines, but "remove 
> the // expected-warning comments" sounds like a significant loss of test 
> coverage.

CodeGen tests don't typically add `-verify` unless the diagnostic is generated 
by the CodeGen library, so if removing the `expected-warning` lines loses test 
coverage, that speaks to a lack of test coverage in Sema. But what, exactly, 
are these tests trying to validate? We have Sema tests which test the behavior 
of "do we want to diagnose this as an implicit declaration or not", so I don't 
see these lines adding any value to the tests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Paul Walker via Phabricator via cfe-commits
paulwalker-arm added a comment.

Please consider this "AArch64 folks speaking up".  What are your plans here 
exactly? I have no issue with adding `-std=c99` to the RUN lines, but "remove 
the // expected-warning comments" sounds like a significant loss of test 
coverage.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman marked 2 inline comments as done.
aaron.ballman added a comment.

In D122983#3426261 , @erichkeane 
wrote:

> I don't have any real comments (the changes are quite trivial), and I really 
> like the approach here, I think it is more than generous.  I see the 
> transition of 
> C89/C99: Warn
> C11/C17: Warn-as-default-error
> C2x: Hard error
>
> as a really gentle and explainable behavior, its just a shame it took us this 
> long to do so.

Thanks!

> I DO see that there are a ton of AArch64 Intrin tests that seem to have the 
> same problems here (in Pre-commit-CI), that will need this change too, though 
> they are questionably written.  I might suggest just adding the -std=c99 flag 
> to all of those.  Attn: @paulwalker-arm @MattDevereau @david-arm @sdesmalen

Yeah, those tests seem to be overly-constraining. There's no reason to be 
validating whether there's an implicit function declaration warning in a 
*codegen* test. I will change all of those AAarch64 tests to require -std=c99 
explicitly whenever possible, remove the `-verify` flag because there's no 
reason for a codegen test to verify diagnostic behavior that isn't generated by 
the CodeGen library, and remove the `// expected-warning` comments. I plan to 
do that as an NFC change that I'll land outside of this patch, unless any of 
the AArch64 folks speak up pretty quickly.




Comment at: clang/docs/ReleaseNotes.rst:114
+- The ``-Wimplicit-function-declaration`` warning diagnostic now defaults to
+  emitting an error (which can be downgraded with ``-Wno-error``) in C11 and
+  C17 mode. This is because the feature was removed in C99 and cannot be

erichkeane wrote:
> xbolva00 wrote:
> > I would mention explicitly  -Wno-error= implicit-function-declaration as 
> > -Wno-error is a big hammer.
> I think this is a valuable change, as suggested by @xbolva00 
Agreed!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added subscribers: MattDevereau, david-arm, paulwalker-arm, 
sdesmalen.
erichkeane added a comment.

I don't have any real comments (the changes are quite trivial), and I really 
like the approach here, I think it is more than generous.  I see the transition 
of 
C89/C99: Warn
C11/C17: Warn-as-default-error
C2x: Hard error

as a really gentle and explainable behavior, its just a shame it took us this 
long to do so.

I DO see that there are a ton of AArch64 Intrin tests that seem to have the 
same problems here (in Pre-commit-CI), that will need this change too, though 
they are questionably written.  I might suggest just adding the -std=c99 flag 
to all of those.  Attn: @paulwalker-arm @MattDevereau @david-arm @sdesmalen




Comment at: clang/docs/ReleaseNotes.rst:114
+- The ``-Wimplicit-function-declaration`` warning diagnostic now defaults to
+  emitting an error (which can be downgraded with ``-Wno-error``) in C11 and
+  C17 mode. This is because the feature was removed in C99 and cannot be

xbolva00 wrote:
> I would mention explicitly  -Wno-error= implicit-function-declaration as 
> -Wno-error is a big hammer.
I think this is a valuable change, as suggested by @xbolva00 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 420138.
aaron.ballman added a comment.

Fix clang-tools-extra test caught by precommit CI.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

Files:
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/ARCMT/objcmt-arc-cf-annotations.m
  clang/test/ARCMT/objcmt-arc-cf-annotations.m.result
  clang/test/Analysis/OSAtomic_mac.c
  clang/test/Analysis/ObjCProperties.m
  clang/test/Analysis/PR49642.c
  clang/test/Analysis/diagnostics/no-store-func-path-notes.c
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/novoidtypecrash.c
  clang/test/Analysis/plist-macros-with-expansion.c
  clang/test/CodeGen/2002-07-14-MiscTests3.c
  clang/test/CodeGen/2002-07-31-SubregFailure.c
  clang/test/CodeGen/2003-08-18-SigSetJmp.c
  clang/test/CodeGen/2004-11-27-StaticFunctionRedeclare.c
  clang/test/CodeGen/2005-01-02-ConstantInits.c
  clang/test/CodeGen/2005-01-02-VAArgError-ICE.c
  clang/test/CodeGen/2006-01-13-StackSave.c
  clang/test/CodeGen/2006-03-03-MissingInitializer.c
  clang/test/CodeGen/2008-05-12-TempUsedBeforeDef.c
  clang/test/CodeGen/2008-07-30-redef-of-bitcasted-decl.c
  clang/test/CodeGen/2008-08-19-cast-of-typedef.c
  clang/test/CodeGen/2008-10-13-FrontendCrash.c
  clang/test/CodeGen/X86/bmi2-builtins.c
  clang/test/CodeGen/aarch64-mops.c
  clang/test/CodeGen/attribute_constructor.c
  clang/test/CodeGen/cast-emit.c
  clang/test/CodeGen/debug-info-block-vars.c
  clang/test/CodeGen/debug-info-crash.c
  clang/test/CodeGen/decl.c
  clang/test/CodeGen/init-with-member-expr.c
  clang/test/CodeGen/misaligned-param.c
  clang/test/CodeGen/struct-comma.c
  clang/test/CodeGen/variable-array.c
  clang/test/Frontend/warning-mapping-2.c
  clang/test/Headers/arm-cmse-header-ns.c
  clang/test/Import/objc-arc/test-cleanup-object.m
  clang/test/Modules/config_macros.m
  clang/test/Modules/modulemap-locations.m
  clang/test/OpenMP/declare_mapper_messages.c
  clang/test/PCH/chain-macro-override.c
  clang/test/Rewriter/rewrite-foreach-2.m
  clang/test/Rewriter/rewrite-try-catch.m
  clang/test/Sema/__try.c
  clang/test/Sema/aarch64-tme-errors.c
  clang/test/Sema/bitfield.c
  clang/test/Sema/builtin-setjmp.c
  clang/test/Sema/builtins.c
  clang/test/Sema/cxx-as-c.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/implicit-decl.c
  clang/test/Sema/implicit-ms-builtin-decl.c
  clang/test/Sema/typo-correction.c
  clang/test/Sema/vla.c
  clang/test/Sema/warn-strict-prototypes.c
  clang/test/VFS/module_missing_vfs.m

Index: clang/test/VFS/module_missing_vfs.m
===
--- clang/test/VFS/module_missing_vfs.m
+++ clang/test/VFS/module_missing_vfs.m
@@ -1,12 +1,12 @@
 // RUN: rm -rf %t && mkdir -p %t
 // RUN: echo "void funcA(void);" >> %t/a.h
 
-// RUN: not %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
+// RUN: not %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
 // ERROR: virtual filesystem overlay file '{{.*}}' not found
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/MissingVFS/vfsoverlay.yaml > %t/vfs.yaml
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
+// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 @import A;
Index: clang/test/Sema/warn-strict-prototypes.c
===
--- clang/test/Sema/warn-strict-prototypes.c
+++ clang/test/Sema/warn-strict-prototypes.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -Wno-implicit-function-declaration -verify %s
-// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -Wno-implicit-function-declaration -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 // function definition with 0 params, no prototype, no preceding declaration.
 void foo0() {} // expected-warning {{this old-style function definition is not preceded by a prototype}}
Index: 

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-02 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:114
+- The ``-Wimplicit-function-declaration`` warning diagnostic now defaults to
+  emitting an error (which can be downgraded with ``-Wno-error``) in C11 and
+  C17 mode. This is because the feature was removed in C99 and cannot be

I would mention explicitly  -Wno-error= implicit-function-declaration as 
-Wno-error is a big hammer.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122983/new/

https://reviews.llvm.org/D122983

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

2022-04-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision.
aaron.ballman added reviewers: jyknight, eli.friedman, rjmccall, erichkeane, 
clang-language-wg.
Herald added subscribers: usaxena95, pengfei, kadircet, arphaman.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added projects: clang, clang-tools-extra.

C89 had a questionable feature where the compiler would implicitly declare a 
function that the user called but was never previously declared. The resulting 
function would be globally declared as `extern int func();` -- a function 
without a prototype which accepts zero or more arguments.

C99 removed support for this questionable feature due to severe security 
concerns. However, there was no deprecation period; C89 had the feature, C99 
didn't. So Clang (and GCC) both supported the functionality as an extension in 
C99 and later modes.

C2x no longer supports that function signature as it now requires all functions 
to have a prototype, and given the known security issues with the feature, 
continuing to support it as an extension is not tenable.

This patch changes the diagnostic behavior for the 
`-Wimplicit-function-declaration` warning group depending on the language mode 
in effect. We continue to warn by default in C89 mode (due to the feature being 
dangerous to use), and we continue to warn by default as an extension in C99 
mode (due to the lack of a deprecation period). However, because this feature 
will not be supported in C2x mode, the security concerns with the feature, and 
the trivial workaround for users (declare the function), we now default the 
extension warning to an error in C11 and C17 mode. This still gives users an 
easy workaround if they are extensively using the extension in those modes 
(they can disable the warning or use `-Wno-error` to downgrade the error), but 
the new diagnostic makes it more clear that this feature is not supported in 
C2x and should be avoided. In C2x mode, we no longer allow an implicit function 
to be defined and treat the situation the same as any other lookup failure.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122983

Files:
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/ARCMT/objcmt-arc-cf-annotations.m
  clang/test/ARCMT/objcmt-arc-cf-annotations.m.result
  clang/test/Analysis/OSAtomic_mac.c
  clang/test/Analysis/ObjCProperties.m
  clang/test/Analysis/PR49642.c
  clang/test/Analysis/diagnostics/no-store-func-path-notes.c
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/novoidtypecrash.c
  clang/test/Analysis/plist-macros-with-expansion.c
  clang/test/CodeGen/2002-07-14-MiscTests3.c
  clang/test/CodeGen/2002-07-31-SubregFailure.c
  clang/test/CodeGen/2003-08-18-SigSetJmp.c
  clang/test/CodeGen/2004-11-27-StaticFunctionRedeclare.c
  clang/test/CodeGen/2005-01-02-ConstantInits.c
  clang/test/CodeGen/2005-01-02-VAArgError-ICE.c
  clang/test/CodeGen/2006-01-13-StackSave.c
  clang/test/CodeGen/2006-03-03-MissingInitializer.c
  clang/test/CodeGen/2008-05-12-TempUsedBeforeDef.c
  clang/test/CodeGen/2008-07-30-redef-of-bitcasted-decl.c
  clang/test/CodeGen/2008-08-19-cast-of-typedef.c
  clang/test/CodeGen/2008-10-13-FrontendCrash.c
  clang/test/CodeGen/X86/bmi2-builtins.c
  clang/test/CodeGen/aarch64-mops.c
  clang/test/CodeGen/attribute_constructor.c
  clang/test/CodeGen/cast-emit.c
  clang/test/CodeGen/debug-info-block-vars.c
  clang/test/CodeGen/debug-info-crash.c
  clang/test/CodeGen/decl.c
  clang/test/CodeGen/init-with-member-expr.c
  clang/test/CodeGen/misaligned-param.c
  clang/test/CodeGen/struct-comma.c
  clang/test/CodeGen/variable-array.c
  clang/test/Frontend/warning-mapping-2.c
  clang/test/Headers/arm-cmse-header-ns.c
  clang/test/Import/objc-arc/test-cleanup-object.m
  clang/test/Modules/config_macros.m
  clang/test/Modules/modulemap-locations.m
  clang/test/OpenMP/declare_mapper_messages.c
  clang/test/PCH/chain-macro-override.c
  clang/test/Rewriter/rewrite-foreach-2.m
  clang/test/Rewriter/rewrite-try-catch.m
  clang/test/Sema/__try.c
  clang/test/Sema/aarch64-tme-errors.c
  clang/test/Sema/bitfield.c
  clang/test/Sema/builtin-setjmp.c
  clang/test/Sema/builtins.c
  clang/test/Sema/cxx-as-c.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/implicit-decl.c
  clang/test/Sema/implicit-ms-builtin-decl.c
  clang/test/Sema/typo-correction.c
  clang/test/Sema/vla.c
  clang/test/Sema/warn-strict-prototypes.c
  clang/test/VFS/module_missing_vfs.m

Index: clang/test/VFS/module_missing_vfs.m
===
--- clang/test/VFS/module_missing_vfs.m
+++ clang/test/VFS/module_missing_vfs.m
@@ -1,12 +1,12 @@
 // RUN: rm -rf %t && mkdir -p %t
 // RUN: echo "void funcA(void);" >> %t/a.h
 
-// RUN: not %clang_cc1 -fmodules