From: Vy Nguyen Date: Tue, 9 Jun 2020 01:14:14 +0000 (-0400) Subject: Make the diagnostic-missing-prototypes put the suggested `static` in front of `const... X-Git-Tag: llvmorg-12-init~3541 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=a21a462051659d8e1281af7d11738b8abc0557dc;p=platform%2Fupstream%2Fllvm.git 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 --- diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 401aea3..e6d4222 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 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 5940a49..1830b3f 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@-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@-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@-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@-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@-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@-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@-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; +}