aboutsummaryrefslogtreecommitdiff
path: root/clang-tidy/readability
diff options
context:
space:
mode:
authorRoman Lebedev <lebedev.ri@gmail.com>2018-10-25 19:44:51 +0000
committerRoman Lebedev <lebedev.ri@gmail.com>2018-10-25 19:44:51 +0000
commit3065f23f1ea2ede750b15a2a8f58a4fab864119b (patch)
tree1db9113601262b4f206957a2f87fcba3e5e9687e /clang-tidy/readability
parent3a48eb2b57cb7ef7a04a8b19077cb41866b08e1f (diff)
[clang-tidy] Revert my readability-uppercase-literal-suffix check.
There are some lurking issues with the handling of the SourceManager. Somehow sometimes we end up extracting completely wrong portions of the source buffer. Reverts r344772, r44760, r344758, r344755. git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@345305 91177308-0d34-0410-b5e6-96231b3b80d8
Diffstat (limited to 'clang-tidy/readability')
-rw-r--r--clang-tidy/readability/CMakeLists.txt1
-rw-r--r--clang-tidy/readability/IdentifierNamingCheck.cpp21
-rw-r--r--clang-tidy/readability/ReadabilityTidyModule.cpp3
-rw-r--r--clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp236
-rw-r--r--clang-tidy/readability/UppercaseLiteralSuffixCheck.h44
5 files changed, 19 insertions, 286 deletions
diff --git a/clang-tidy/readability/CMakeLists.txt b/clang-tidy/readability/CMakeLists.txt
index bd010af9..da2320ea 100644
--- a/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tidy/readability/CMakeLists.txt
@@ -31,7 +31,6 @@ add_clang_library(clangTidyReadabilityModule
StaticDefinitionInAnonymousNamespaceCheck.cpp
StringCompareCheck.cpp
UniqueptrDeleteReleaseCheck.cpp
- UppercaseLiteralSuffixCheck.cpp
LINK_LIBS
clangAST
diff --git a/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tidy/readability/IdentifierNamingCheck.cpp
index fb3c02e1..1d116817 100644
--- a/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -9,7 +9,6 @@
#include "IdentifierNamingCheck.h"
-#include "../utils/ASTUtils.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Lex/PPCallbacks.h"
@@ -682,7 +681,25 @@ static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
if (!Failure.ShouldFix)
return;
- Failure.ShouldFix = utils::rangeCanBeFixed(Range, SourceMgr);
+ // Check if the range is entirely contained within a macro argument.
+ SourceLocation MacroArgExpansionStartForRangeBegin;
+ SourceLocation MacroArgExpansionStartForRangeEnd;
+ bool RangeIsEntirelyWithinMacroArgument =
+ SourceMgr &&
+ SourceMgr->isMacroArgExpansion(Range.getBegin(),
+ &MacroArgExpansionStartForRangeBegin) &&
+ SourceMgr->isMacroArgExpansion(Range.getEnd(),
+ &MacroArgExpansionStartForRangeEnd) &&
+ MacroArgExpansionStartForRangeBegin == MacroArgExpansionStartForRangeEnd;
+
+ // Check if the range contains any locations from a macro expansion.
+ bool RangeContainsMacroExpansion = RangeIsEntirelyWithinMacroArgument ||
+ Range.getBegin().isMacroID() ||
+ Range.getEnd().isMacroID();
+
+ bool RangeCanBeFixed =
+ RangeIsEntirelyWithinMacroArgument || !RangeContainsMacroExpansion;
+ Failure.ShouldFix = RangeCanBeFixed;
}
/// Convenience method when the usage to be added is a NamedDecl
diff --git a/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tidy/readability/ReadabilityTidyModule.cpp
index ca49f6f1..a5b91544 100644
--- a/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -38,7 +38,6 @@
#include "StaticDefinitionInAnonymousNamespaceCheck.h"
#include "StringCompareCheck.h"
#include "UniqueptrDeleteReleaseCheck.h"
-#include "UppercaseLiteralSuffixCheck.h"
namespace clang {
namespace tidy {
@@ -103,8 +102,6 @@ public:
"readability-simplify-boolean-expr");
CheckFactories.registerCheck<UniqueptrDeleteReleaseCheck>(
"readability-uniqueptr-delete-release");
- CheckFactories.registerCheck<UppercaseLiteralSuffixCheck>(
- "readability-uppercase-literal-suffix");
}
};
diff --git a/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp b/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
deleted file mode 100644
index 3feeaf99..00000000
--- a/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
+++ /dev/null
@@ -1,236 +0,0 @@
-//===--- UppercaseLiteralSuffixCheck.cpp - clang-tidy ---------------------===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#include "UppercaseLiteralSuffixCheck.h"
-#include "../utils/ASTUtils.h"
-#include "../utils/OptionsUtils.h"
-#include "clang/AST/ASTContext.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "llvm/ADT/Optional.h"
-#include "llvm/ADT/SmallString.h"
-
-using namespace clang::ast_matchers;
-
-namespace clang {
-namespace tidy {
-namespace readability {
-
-namespace {
-
-struct IntegerLiteralCheck {
- using type = clang::IntegerLiteral;
- static constexpr llvm::StringLiteral Name = llvm::StringLiteral("integer");
- // What should be skipped before looking for the Suffixes? (Nothing here.)
- static constexpr llvm::StringLiteral SkipFirst = llvm::StringLiteral("");
- // Suffix can only consist of 'u' and 'l' chars, and can be a complex number
- // ('i', 'j'). In MS compatibility mode, suffixes like i32 are supported.
- static constexpr llvm::StringLiteral Suffixes =
- llvm::StringLiteral("uUlLiIjJ");
-};
-constexpr llvm::StringLiteral IntegerLiteralCheck::Name;
-constexpr llvm::StringLiteral IntegerLiteralCheck::SkipFirst;
-constexpr llvm::StringLiteral IntegerLiteralCheck::Suffixes;
-
-struct FloatingLiteralCheck {
- using type = clang::FloatingLiteral;
- static constexpr llvm::StringLiteral Name =
- llvm::StringLiteral("floating point");
- // C++17 introduced hexadecimal floating-point literals, and 'f' is both a
- // valid hexadecimal digit in a hex float literal and a valid floating-point
- // literal suffix.
- // So we can't just "skip to the chars that can be in the suffix".
- // Since the exponent ('p'/'P') is mandatory for hexadecimal floating-point
- // literals, we first skip everything before the exponent.
- static constexpr llvm::StringLiteral SkipFirst = llvm::StringLiteral("pP");
- // Suffix can only consist of 'f', 'l', "f16", 'h', 'q' chars,
- // and can be a complex number ('i', 'j').
- static constexpr llvm::StringLiteral Suffixes =
- llvm::StringLiteral("fFlLhHqQiIjJ");
-};
-constexpr llvm::StringLiteral FloatingLiteralCheck::Name;
-constexpr llvm::StringLiteral FloatingLiteralCheck::SkipFirst;
-constexpr llvm::StringLiteral FloatingLiteralCheck::Suffixes;
-
-struct NewSuffix {
- SourceLocation LiteralLocation;
- StringRef OldSuffix;
- llvm::Optional<FixItHint> FixIt;
-};
-
-llvm::Optional<SourceLocation> GetMacroAwareLocation(SourceLocation Loc,
- const SourceManager &SM) {
- // Do nothing if the provided location is invalid.
- if (Loc.isInvalid())
- return llvm::None;
- // Look where the location was *actually* written.
- SourceLocation SpellingLoc = SM.getSpellingLoc(Loc);
- if (SpellingLoc.isInvalid())
- return llvm::None;
- return SpellingLoc;
-}
-
-llvm::Optional<SourceRange> GetMacroAwareSourceRange(SourceRange Loc,
- const SourceManager &SM) {
- llvm::Optional<SourceLocation> Begin =
- GetMacroAwareLocation(Loc.getBegin(), SM);
- llvm::Optional<SourceLocation> End = GetMacroAwareLocation(Loc.getEnd(), SM);
- if (!Begin || !End)
- return llvm::None;
- return SourceRange(*Begin, *End);
-}
-
-llvm::Optional<std::string>
-getNewSuffix(llvm::StringRef OldSuffix,
- const std::vector<std::string> &NewSuffixes) {
- // If there is no config, just uppercase the entirety of the suffix.
- if (NewSuffixes.empty())
- return OldSuffix.upper();
- // Else, find matching suffix, case-*insensitive*ly.
- auto NewSuffix = llvm::find_if(
- NewSuffixes, [OldSuffix](const std::string &PotentialNewSuffix) {
- return OldSuffix.equals_lower(PotentialNewSuffix);
- });
- // Have a match, return it.
- if (NewSuffix != NewSuffixes.end())
- return *NewSuffix;
- // Nope, I guess we have to keep it as-is.
- return llvm::None;
-}
-
-template <typename LiteralType>
-llvm::Optional<NewSuffix>
-shouldReplaceLiteralSuffix(const Expr &Literal,
- const std::vector<std::string> &NewSuffixes,
- const SourceManager &SM, const LangOptions &LO) {
- NewSuffix ReplacementDsc;
-
- const auto &L = cast<typename LiteralType::type>(Literal);
-
- // The naive location of the literal. Is always valid.
- ReplacementDsc.LiteralLocation = L.getLocation();
-
- // Was this literal fully spelled or is it a product of macro expansion?
- bool RangeCanBeFixed =
- utils::rangeCanBeFixed(ReplacementDsc.LiteralLocation, &SM);
-
- // The literal may have macro expansion, we need the final expanded src range.
- llvm::Optional<SourceRange> Range =
- GetMacroAwareSourceRange(ReplacementDsc.LiteralLocation, SM);
- if (!Range)
- return llvm::None;
-
- if (RangeCanBeFixed)
- ReplacementDsc.LiteralLocation = Range->getBegin();
- // Else keep the naive literal location!
-
- // Get the whole literal from the source buffer.
- bool Invalid;
- const StringRef LiteralSourceText = Lexer::getSourceText(
- CharSourceRange::getTokenRange(*Range), SM, LO, &Invalid);
- assert(!Invalid && "Failed to retrieve the source text.");
-
- size_t Skip = 0;
-
- // Do we need to ignore something before actually looking for the suffix?
- if (!LiteralType::SkipFirst.empty()) {
- // E.g. we can't look for 'f' suffix in hexadecimal floating-point literals
- // until after we skip to the exponent (which is mandatory there),
- // because hex-digit-sequence may contain 'f'.
- Skip = LiteralSourceText.find_first_of(LiteralType::SkipFirst);
- // We could be in non-hexadecimal floating-point literal, with no exponent.
- if (Skip == StringRef::npos)
- Skip = 0;
- }
-
- // Find the beginning of the suffix by looking for the first char that is
- // one of these chars that can be in the suffix, potentially starting looking
- // in the exponent, if we are skipping hex-digit-sequence.
- Skip = LiteralSourceText.find_first_of(LiteralType::Suffixes, /*From=*/Skip);
-
- // We can't check whether the *Literal has any suffix or not without actually
- // looking for the suffix. So it is totally possible that there is no suffix.
- if (Skip == StringRef::npos)
- return llvm::None;
-
- // Move the cursor in the source range to the beginning of the suffix.
- Range->setBegin(Range->getBegin().getLocWithOffset(Skip));
- // And in our textual representation too.
- ReplacementDsc.OldSuffix = LiteralSourceText.drop_front(Skip);
- assert(!ReplacementDsc.OldSuffix.empty() &&
- "We still should have some chars left.");
-
- // And get the replacement suffix.
- llvm::Optional<std::string> NewSuffix =
- getNewSuffix(ReplacementDsc.OldSuffix, NewSuffixes);
- if (!NewSuffix || ReplacementDsc.OldSuffix == *NewSuffix)
- return llvm::None; // The suffix was already the way it should be.
-
- if (RangeCanBeFixed)
- ReplacementDsc.FixIt = FixItHint::CreateReplacement(*Range, *NewSuffix);
-
- return ReplacementDsc;
-}
-
-} // namespace
-
-UppercaseLiteralSuffixCheck::UppercaseLiteralSuffixCheck(
- StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context),
- NewSuffixes(
- utils::options::parseStringList(Options.get("NewSuffixes", ""))) {}
-
-void UppercaseLiteralSuffixCheck::storeOptions(
- ClangTidyOptions::OptionMap &Opts) {
- Options.store(Opts, "NewSuffixes",
- utils::options::serializeStringList(NewSuffixes));
-}
-
-void UppercaseLiteralSuffixCheck::registerMatchers(MatchFinder *Finder) {
- // Sadly, we can't check whether the literal has sufix or not.
- // E.g. i32 suffix still results in 'BuiltinType::Kind::Int'.
- // And such an info is not stored in the *Literal itself.
- Finder->addMatcher(
- stmt(allOf(eachOf(integerLiteral().bind(IntegerLiteralCheck::Name),
- floatLiteral().bind(FloatingLiteralCheck::Name)),
- unless(hasParent(userDefinedLiteral())))),
- this);
-}
-
-template <typename LiteralType>
-bool UppercaseLiteralSuffixCheck::checkBoundMatch(
- const MatchFinder::MatchResult &Result) {
- const auto *Literal =
- Result.Nodes.getNodeAs<typename LiteralType::type>(LiteralType::Name);
- if (!Literal)
- return false;
-
- // We won't *always* want to diagnose.
- // We might have a suffix that is already uppercase.
- if (auto Details = shouldReplaceLiteralSuffix<LiteralType>(
- *Literal, NewSuffixes, *Result.SourceManager, getLangOpts())) {
- auto Complaint = diag(Details->LiteralLocation,
- "%0 literal has suffix '%1', which is not uppercase")
- << LiteralType::Name << Details->OldSuffix;
- if (Details->FixIt) // Similarly, a fix-it is not always possible.
- Complaint << *(Details->FixIt);
- }
-
- return true;
-}
-
-void UppercaseLiteralSuffixCheck::check(
- const MatchFinder::MatchResult &Result) {
- if (checkBoundMatch<IntegerLiteralCheck>(Result))
- return; // If it *was* IntegerLiteral, don't check for FloatingLiteral.
- checkBoundMatch<FloatingLiteralCheck>(Result);
-}
-
-} // namespace readability
-} // namespace tidy
-} // namespace clang
diff --git a/clang-tidy/readability/UppercaseLiteralSuffixCheck.h b/clang-tidy/readability/UppercaseLiteralSuffixCheck.h
deleted file mode 100644
index 8d26e6c2..00000000
--- a/clang-tidy/readability/UppercaseLiteralSuffixCheck.h
+++ /dev/null
@@ -1,44 +0,0 @@
-//===--- UppercaseLiteralSuffixCheck.h - clang-tidy -------------*- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_UPPERCASELITERALSUFFIXCHECK_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_UPPERCASELITERALSUFFIXCHECK_H
-
-#include "../ClangTidy.h"
-#include "../utils/OptionsUtils.h"
-
-namespace clang {
-namespace tidy {
-namespace readability {
-
-/// Detects when the integral literal or floating point literal has
-/// non-uppercase suffix, and suggests to make the suffix uppercase.
-/// Alternatively, a list of destination suffixes can be provided.
-///
-/// For the user-facing documentation see:
-/// http://clang.llvm.org/extra/clang-tidy/checks/readability-uppercase-literal-suffix.html
-class UppercaseLiteralSuffixCheck : public ClangTidyCheck {
-public:
- UppercaseLiteralSuffixCheck(StringRef Name, ClangTidyContext *Context);
- void registerMatchers(ast_matchers::MatchFinder *Finder) override;
- void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
- void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
-
-private:
- template <typename LiteralType>
- bool checkBoundMatch(const ast_matchers::MatchFinder::MatchResult &Result);
-
- const std::vector<std::string> NewSuffixes;
-};
-
-} // namespace readability
-} // namespace tidy
-} // namespace clang
-
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_UPPERCASELITERALSUFFIXCHECK_H