oontvoo updated this revision to Diff 269627.
oontvoo marked an inline comment as done.
oontvoo added a comment.
- Less brittle way to find the loc of const.
- Add more tests
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81444/new/
https://reviews.llvm.org/D81444
Files:
clang/include/clang/Lex/Lexer.h
clang/lib/Lex/Lexer.cpp
clang/lib/Sema/SemaDecl.cpp
clang/test/Sema/warn-missing-prototypes.c
Index: clang/test/Sema/warn-missing-prototypes.c
===================================================================
--- clang/test/Sema/warn-missing-prototypes.c
+++ clang/test/Sema/warn-missing-prototypes.c
@@ -49,3 +49,57 @@
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 "
+ int *ret;
+ return ret;
+}
+
+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;
+}
+
+// Random comment before const. We should put the `static` between the comment block and `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 "
+ struct MyStruct ret;
+ return ret;
+}
+
+// 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 "
+ struct MyStruct ret;
+ return ret;
+}
+// clang-format on
+
+#define MY_CONST const
+
+// This is not supported. We can't expand MY_CONST (efficiently) so we just give
+// up and put the 'static' before 'struct'
+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 "
+ struct MyStruct ret;
+ return ret;
+}
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14243,11 +14243,24 @@
: FixItHint{});
}
} else {
+ // If the return type has `const` qualifier, we want to insert `static`
+ // before `const` (and not before the typename).
+ auto findBeginLoc = [&]() {
+ if ((FD->getReturnType()->isAnyPointerType() &&
+ FD->getReturnType()->getPointeeType().isConstQualified()) ||
+ FD->getReturnType().isConstQualified())
+ // But only do this if we can determine where the `const` is.
+ if (clang::Lexer::isConst(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{});
}
Index: clang/lib/Lex/Lexer.cpp
===================================================================
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -1299,6 +1299,28 @@
return TokenLoc.getLocWithOffset(Tok->getLength() + NumWhitespaceChars);
}
+bool Lexer::isConst(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("//"));
+}
+
/// getCharAndSizeSlow - Peek a single 'character' from the specified buffer,
/// get its size, and return it. This is tricky in several cases:
/// 1. If currently at the start of a trigraph, we warn about the trigraph,
Index: clang/include/clang/Lex/Lexer.h
===================================================================
--- clang/include/clang/Lex/Lexer.h
+++ clang/include/clang/Lex/Lexer.h
@@ -531,6 +531,10 @@
const LangOptions &LangOpts,
bool SkipTrailingWhitespaceAndNewLine);
+ /// Returns true if the token beginning at this Loc is `const`.
+ static bool isConst(SourceLocation Loc, const SourceManager &SM,
+ const LangOptions &LangOpts);
+
/// Returns true if the given character could appear in an identifier.
static bool isIdentifierBodyChar(char c, const LangOptions &LangOpts);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
