[PATCH] D81444: Make the diagnostic-missing-prototypes put the suggested `static` in front of `const` if exists.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.


================
Comment at: clang/lib/Lex/Lexer.cpp:1302
 
+bool Lexer::isConst(SourceLocation Loc, const SourceManager &SM,
+                    const LangOptions &LangOpts) {
----------------
I'm concerned about putting this API into Lexer, given this implementation. It is not a precise general-purpose implementation, it can have false negatives (for example, if the const token is formed through macros, or token pasting or whatnot), and it can have false positives (if the 'const' token is actually a macro that expands to something else).

I'd suggest to define this function as a local lambda right where it is used or inline it completely into findBeginLoc.


================
Comment at: clang/test/Sema/warn-missing-prototypes.c:57
+  int *ret;
+  return ret;
+}
----------------
Why not `return (void*)0;`?


================
Comment at: clang/test/Sema/warn-missing-prototypes.c:62
+
+const struct MyStruct get_struct() { // expected-warning{{no previous prototype for function 'get_struct'}}
+                                     // expected-note at -1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
----------------
Did you mean to return a pointer to MyStruct? The top-level const is not meaningful.


================
Comment at: clang/test/Sema/warn-missing-prototypes.c:63
+const struct MyStruct get_struct() { // expected-warning{{no previous prototype for function 'get_struct'}}
+                                     // expected-note at -1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
+                                     // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:1-[[@LINE-2]]:1}:"static "
----------------
Extra indentation? Please make it consistent with other tests.


================
Comment at: clang/test/Sema/warn-missing-prototypes.c:71
+// clang-format off
+// Two spaces between cost and struct
+const  struct MyStruct get_struct_2() {  // expected-warning{{no previous prototype for function 'get_struct_2'}}
----------------
cost => const


================
Comment at: clang/test/Sema/warn-missing-prototypes.c:98
+
+// This is not supported. We can't expand MY_CONST (efficiently) so we just give
+// up and put the 'static' before 'struct'
----------------
"Since we can't easily understand what MY_CONST means while preparing the diagnostic, the fix-it suggests to add 'static' in a non-idiomatic place."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81444