Author: Ilya Mirsky Date: 2019-12-24T10:10:01-05:00 New Revision: f58f39137c6e5a324ef684b1d72bddae244aa94d
URL: https://github.com/llvm/llvm-project/commit/f58f39137c6e5a324ef684b1d72bddae244aa94d DIFF: https://github.com/llvm/llvm-project/commit/f58f39137c6e5a324ef684b1d72bddae244aa94d.diff LOG: Fix readability-const-return-type identifying the wrong `const` token Replace tidy::utils::lexer::getConstQualifyingToken with a corrected and also generalized to other qualifiers variant - getQualifyingToken. Fixes PR44326 Added: Modified: clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp clang-tools-extra/clang-tidy/utils/LexerUtils.cpp clang-tools-extra/clang-tidy/utils/LexerUtils.h clang-tools-extra/test/clang-tidy/checkers/readability-const-return-type.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp index 0f237ec8ee1d..d987886b62b5 100644 --- a/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp @@ -47,8 +47,8 @@ findConstToRemove(const FunctionDecl *Def, 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.cpp b/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp index e80fda6e318e..17838fe577fa 100644 --- a/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp +++ b/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp @@ -102,15 +102,20 @@ bool rangeContainsExpansionsOrDirectives(SourceRange Range, return false; } -llvm::Optional<Token> getConstQualifyingToken(CharSourceRange Range, - const ASTContext &Context, - const SourceManager &SM) { +llvm::Optional<Token> 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<FileID, unsigned> 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<Token> FirstConstTok; - Token LastTokInRange; + llvm::Optional<Token> LastMatchBeforeTemplate; + llvm::Optional<Token> LastMatchAfterTemplate; + bool SawTemplate = false; Token Tok; while (!RawLexer.LexFromRawLexer(Tok) && Range.getEnd() != Tok.getLocation() && @@ -121,13 +126,19 @@ llvm::Optional<Token> getConstQualifyingToken(CharSourceRange Range, 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/clang-tidy/utils/LexerUtils.h b/clang-tools-extra/clang-tidy/utils/LexerUtils.h index 2c4a2518259d..fcf9ada85ff7 100644 --- a/clang-tools-extra/clang-tidy/utils/LexerUtils.h +++ b/clang-tools-extra/clang-tidy/utils/LexerUtils.h @@ -92,13 +92,15 @@ bool rangeContainsExpansionsOrDirectives(SourceRange Range, 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<Token> getConstQualifyingToken(CharSourceRange Range, - const ASTContext &Context, - const SourceManager &SM); +/// \note: doesn't support member function qualifiers. +llvm::Optional<Token> getQualifyingToken(tok::TokenKind TK, + CharSourceRange Range, + const ASTContext &Context, + const SourceManager &SM); } // 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 index 78557c5e652e..f801b18a98b8 100644 --- 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 @@ const T p32(T t) { return t; } template <typename T> typename std::add_const<T>::type n15(T v) { return v; } +template <bool B> +struct MyStruct {}; + template <typename A> class Klazz { public: @@ -128,10 +131,46 @@ const Klazz<const int> p12() {} // CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz<const int>' // CHECK-FIXES: Klazz<const int> p12() {} +const Klazz<const Klazz<const int>> p33() {} +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz< +// CHECK-FIXES: Klazz<const Klazz<const int>> p33() {} + const Klazz<const int>* const p13() {} // CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz<const int> * // CHECK-FIXES: const Klazz<const int>* p13() {} +const Klazz<const int>* const volatile p14() {} +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz<const int> * +// CHECK-FIXES: const Klazz<const int>* 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<MyStruct<0 < 1> const> const p36() {} +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz<const MyStru +// CHECK-FIXES: Klazz<MyStruct<0 < 1> const> p36() {} + +const Klazz<MyStruct<0 < 1> const> *const p37() {} +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz<const MyStru +// CHECK-FIXES: const Klazz<MyStruct<0 < 1> const> *p37() {} + +Klazz<const MyStruct<0 < 1>> const p38() {} +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz<const MyStru +// CHECK-FIXES: Klazz<const MyStruct<0 < 1>> p38() {} + +const Klazz<const MyStruct<0 < 1>> p39() {} +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz< +// CHECK-FIXES: Klazz<const MyStruct<0 < 1>> p39() {} + +const Klazz<const MyStruct<(0 > 1)>> p40() {} +// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const Klazz<const MyStru +// CHECK-FIXES: Klazz<const MyStruct<(0 > 1)>> p40() {} + // re-declaration of p15. const int p15(); // CHECK-FIXES: int p15(); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
