[PATCH] D123955: [C2x] Disallow functions without prototypes/functions with identifier lists

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked an inline comment as done.
aaron.ballman added inline comments.


================
Comment at: clang/lib/AST/ASTContext.cpp:11243
 
   // We really shouldn't be making a no-proto type here.
+  if (ArgTypes.empty() && Variadic && !getLangOpts().StrictPrototypes)
----------------
erichkeane wrote:
> How tough would it be to change this to represent always as a C++-esque `foo(...)` prototype, just always?
C doesn't allow that (but we do if the user specifies the `overloadable` attribute), so it could be a bit tricky.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:8821
         (!R->getAsAdjusted<FunctionType>() && R->isFunctionProtoType());
+    assert(SemaRef.getLangOpts().StrictPrototypes
+               ? HasPrototype
----------------
erichkeane wrote:
> This assert is quite strange, particularly with the string part of it only in the 'else' of the conditional? It seems you're trying to ensure that:
> "if strictProtoTypes, assert hasProtoType"?
> 
> In that case, I'd likely suggest:
> 
> `assert((HasProtoType || !SemeaRef.getLangOpts().StrictPrototypes) && "Strict prototypes are required");`
> 
> 
You decoded what I was going for, but I like your approach better. Thanks!


================
Comment at: clang/test/Parser/c2x-attributes.c:64
+                                      c2x-warning 2 {{type specifier missing, defaults to 'int'}} \
+                                      c2x-error {{expected ';' after top level declarator}} \
+                                      c2x-error {{expected identifier or '('}}
----------------
erichkeane wrote:
> This is an unfortunate change in diagnostic quality.
Agreed, but thankfully this situation should be exceedingly rare (trying to put `[[]]` attributes after a function definition with an identifier list).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123955