[clang] a21a462 - Make the diagnostic-missing-prototypes put the suggested `static` in front of `const` if exists.

[Vy Nguyen via cfe-commits]

Author: Vy Nguyen
Date: 2020-06-09T19:34:57-04:00
New Revision: a21a462051659d8e1281af7d11738b8abc0557dc

URL: https://github.com/llvm/llvm-project/commit/a21a462051659d8e1281af7d11738b8abc0557dc
DIFF: https://github.com/llvm/llvm-project/commit/a21a462051659d8e1281af7d11738b8abc0557dc.diff

LOG: Make the diagnostic-missing-prototypes put the suggested `static` in front of `const` if exists.

Summary:
Consider: `const int* get_foo() {return nullptr;}`
The suggested fix should be `static const int* get_foo(){}`
and not `const static int* get_foo(){}`

Reviewers: gribozavr2

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D81444

Added: 
    

Modified: 
    clang/lib/Sema/SemaDecl.cpp
    clang/test/Sema/warn-missing-prototypes.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 401aea355c41..e6d4222d4b17 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -14243,11 +14243,48 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
                         : FixItHint{});
         }
       } else {
+        // Returns true if the token beginning at this Loc is `const`.
+        auto isLocAtConst = [&](SourceLocation Loc, const SourceManager &SM,
+                                const LangOptions &LangOpts) {
+          std::pair<FileID, unsigned> LocInfo = SM.getDecomposedLoc(Loc);
+          if (LocInfo.first.isInvalid())
+            return false;
+
+          bool Invalid = false;
+          StringRef Buffer = SM.getBufferData(LocInfo.first, &Invalid);
+          if (Invalid)
+            return false;
+
+          if (LocInfo.second > Buffer.size())
+            return false;
+
+          const char *LexStart = Buffer.data() + LocInfo.second;
+          StringRef StartTok(LexStart, Buffer.size() - LocInfo.second);
+
+          return StartTok.consume_front("const") &&
+                 (StartTok.empty() || isWhitespace(StartTok[0]) ||
+                  StartTok.startswith("/*") || StartTok.startswith("//"));
+        };
+
+        auto findBeginLoc = [&]() {
+          // If the return type has `const` qualifier, we want to insert
+          // `static` before `const` (and not before the typename).
+          if ((FD->getReturnType()->isAnyPointerType() &&
+               FD->getReturnType()->getPointeeType().isConstQualified()) ||
+              FD->getReturnType().isConstQualified()) {
+            // But only do this if we can determine where the `const` is.
+
+            if (isLocAtConst(FD->getBeginLoc(), getSourceManager(),
+                             getLangOpts()))
+
+              return FD->getBeginLoc();
+          }
+          return FD->getTypeSpecStartLoc();
+        };
         Diag(FD->getTypeSpecStartLoc(), diag::note_static_for_internal_linkage)
             << /* function */ 1
             << (FD->getStorageClass() == SC_None
-                    ? FixItHint::CreateInsertion(FD->getTypeSpecStartLoc(),
-                                                 "static ")
+                    ? FixItHint::CreateInsertion(findBeginLoc(), "static ")
                     : FixItHint{});
       }
 

diff  --git a/clang/test/Sema/warn-missing-prototypes.c b/clang/test/Sema/warn-missing-prototypes.c
index 5940a49572ba..1830b3ff9e73 100644
--- a/clang/test/Sema/warn-missing-prototypes.c
+++ b/clang/test/Sema/warn-missing-prototypes.c
@@ -49,3 +49,60 @@ int main(void) { return 0; }
 void not_a_prototype_test(); // expected-note{{this declaration is not a prototype; add 'void' to make it a prototype for a zero-parameter function}}
 // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:27-[[@LINE-1]]:27}:"void"
 void not_a_prototype_test() { } // expected-warning{{no previous prototype for function 'not_a_prototype_test'}}
+
+const int *get_const() { // expected-warning{{no previous prototype for function 'get_const'}}
+  // 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 "
+  return (void *)0;
+}
+
+struct MyStruct {};
+
+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 "
+  struct MyStruct ret;
+  return ret;
+}
+
+// Turn off clang-format for white-space dependent testing.
+// 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'}}
+  // 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 "
+  struct MyStruct ret;
+  return ret;
+}
+
+// Two spaces bewteen const and struct
+const  struct MyStruct* get_struct_ptr() {  // expected-warning{{no previous prototype for function 'get_struct_ptr'}}
+  // 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 "
+  return (void*)0;
+}
+
+// Random comment before const.
+/*some randome comment*/const  struct MyStruct* get_struct_3() {  // expected-warning{{no previous prototype for function 'get_struct_3'}}
+  // 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]]:25-[[@LINE-2]]:25}:"static "
+  return (void*)0;
+}
+
+// Random comment after const.
+const/*some randome comment*/ struct MyStruct* get_struct_4() {  // expected-warning{{no previous prototype for function 'get_struct_4'}}
+  // 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 "
+  return (void*)0;
+}
+// clang-format on
+
+#define MY_CONST const
+
+// 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.
+MY_CONST struct MyStruct *get_struct_nyi() { // expected-warning{{no previous prototype for function 'get_struct_nyi'}}
+  // 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]]:10-[[@LINE-2]]:10}:"static "
+  return (void *)0;
+}