diff --git a/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp --- a/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp @@ -47,8 +47,8 @@ if (FileRange.isInvalid()) return None; - return utils::lexer::getConstQualifyingToken(FileRange, *Result.Context, - *Result.SourceManager); + return utils::lexer::getQualifyingToken( + tok::kw_const, FileRange, *Result.Context, *Result.SourceManager); } namespace { diff --git a/clang-tools-extra/clang-tidy/utils/LexerUtils.h b/clang-tools-extra/clang-tidy/utils/LexerUtils.h --- a/clang-tools-extra/clang-tidy/utils/LexerUtils.h +++ b/clang-tools-extra/clang-tidy/utils/LexerUtils.h @@ -92,13 +92,15 @@ const SourceManager &SM, const LangOptions &LangOpts); -/// Assuming that ``Range`` spans a const-qualified type, returns the ``const`` -/// token in ``Range`` that is responsible for const qualification. ``Range`` -/// must be valid with respect to ``SM``. Returns ``None`` if no ``const`` +/// Assuming that ``Range`` spans a CVR-qualified type, returns the +/// token in ``Range`` that is responsible for the qualification. ``Range`` +/// must be valid with respect to ``SM``. Returns ``None`` if no qualifying /// tokens are found. -llvm::Optional getConstQualifyingToken(CharSourceRange Range, - const ASTContext &Context, - const SourceManager &SM); +/// \note: doesn't support member function qualifiers. +llvm::Optional getQualifyingToken(tok::TokenKind TK, + CharSourceRange Range, + const ASTContext &Context, + const SourceManager &SM); } // namespace lexer } // namespace utils diff --git a/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp b/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp --- a/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp +++ b/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp @@ -102,15 +102,20 @@ return false; } -llvm::Optional getConstQualifyingToken(CharSourceRange Range, - const ASTContext &Context, - const SourceManager &SM) { +llvm::Optional getQualifyingToken(tok::TokenKind TK, + CharSourceRange Range, + const ASTContext &Context, + const SourceManager &SM) { + assert((TK == tok::kw_const || TK == tok::kw_volatile || + TK == tok::kw_restrict) && + "TK is not a qualifier keyword"); std::pair LocInfo = SM.getDecomposedLoc(Range.getBegin()); StringRef File = SM.getBufferData(LocInfo.first); Lexer RawLexer(SM.getLocForStartOfFile(LocInfo.first), Context.getLangOpts(), File.begin(), File.data() + LocInfo.second, File.end()); - llvm::Optional FirstConstTok; - Token LastTokInRange; + llvm::Optional LastMatchBeforeTemplate; + llvm::Optional LastMatchAfterTemplate; + bool SawTemplate = false; Token Tok; while (!RawLexer.LexFromRawLexer(Tok) && Range.getEnd() != Tok.getLocation() && @@ -121,13 +126,19 @@ Tok.setIdentifierInfo(&Info); Tok.setKind(Info.getTokenID()); } - if (Tok.is(tok::kw_const) && !FirstConstTok) - FirstConstTok = Tok; - LastTokInRange = Tok; + if (Tok.is(tok::less)) + SawTemplate = true; + else if (Tok.isOneOf(tok::greater, tok::greatergreater)) + LastMatchAfterTemplate = None; + else if (Tok.is(TK)) { + if (SawTemplate) + LastMatchAfterTemplate = Tok; + else + LastMatchBeforeTemplate = Tok; + } } - // If the last token in the range is a `const`, then it const qualifies the - // type. Otherwise, the first `const` token, if any, is the qualifier. - return LastTokInRange.is(tok::kw_const) ? LastTokInRange : FirstConstTok; + return LastMatchAfterTemplate != None ? LastMatchAfterTemplate + : LastMatchBeforeTemplate; } } // namespace lexer } // namespace utils diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability-const-return-type.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-const-return-type.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/readability-const-return-type.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability-const-return-type.cpp @@ -37,6 +37,9 @@ template typename std::add_const::type n15(T v) { return v; } +template +struct MyStruct {}; + template class Klazz { public: @@ -128,10 +131,46 @@ // CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz' // CHECK-FIXES: Klazz p12() {} +const Klazz> p33() {} +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz< +// CHECK-FIXES: Klazz> p33() {} + const Klazz* const p13() {} // CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz * // CHECK-FIXES: const Klazz* p13() {} +const Klazz* const volatile p14() {} +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz * +// CHECK-FIXES: const Klazz* volatile p14() {} + +const MyStruct<0 < 1> p34() {} +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const MyStruct<0 < 1>' +// CHECK-FIXES: MyStruct<0 < 1> p34() {} + +MyStruct<0 < 1> const p35() {} +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const MyStruct<0 < 1>' +// CHECK-FIXES: MyStruct<0 < 1> p35() {} + +Klazz const> const p36() {} +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz const> p36() {} + +const Klazz const> *const p37() {} +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz const> *p37() {} + +Klazz> const p38() {} +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz> p38() {} + +const Klazz> p39() {} +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz< +// CHECK-FIXES: Klazz> p39() {} + +const Klazz 1)>> p40() {} +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz 1)>> p40() {} + // re-declaration of p15. const int p15(); // CHECK-FIXES: int p15();