Skip to content

Commit

Permalink
[clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CE…
Browse files Browse the repository at this point in the history
…RT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

Summary:
Detects when the integral literal or floating point (decimal or hexadecimal)
literal has non-uppercase suffix, and suggests to make the suffix uppercase,
with fix-it.

All valid combinations of suffixes are supported.

```
  auto x = 1;  // OK, no suffix.

  auto x = 1u; // warning: integer literal suffix 'u' is not upper-case

  auto x = 1U; // OK, suffix is uppercase.

  ...
```

References:
* [[ https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152241 | CERT DCL16-C ]]
* MISRA C:2012, 7.3 - The lowercase character "l" shall not be used in a literal suffix
* MISRA C++:2008, 2-13-4 - Literal suffixes shall be upper case

Reviewers: JonasToth, aaron.ballman, alexfh, hokein, xazax.hun

Reviewed By: aaron.ballman

Subscribers: Eugene.Zelenko, mgorny, rnkovacs, cfe-commits

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D52670

llvm-svn: 344755
  • Loading branch information
LebedevRI committed Oct 18, 2018
1 parent 87b6725 commit b2eec58
Show file tree
Hide file tree
Showing 24 changed files with 1,454 additions and 19 deletions.
10 changes: 10 additions & 0 deletions clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
Expand Up @@ -16,6 +16,7 @@
#include "../misc/StaticAssertCheck.h"
#include "../misc/ThrowByValueCatchByReferenceCheck.h"
#include "../performance/MoveConstructorInitCheck.h"
#include "../readability/UppercaseLiteralSuffixCheck.h"
#include "CommandProcessorCheck.h"
#include "DontModifyStdNamespaceCheck.h"
#include "FloatLoopCounter.h"
Expand Down Expand Up @@ -65,6 +66,8 @@ class CERTModule : public ClangTidyModule {
// C checkers
// DCL
CheckFactories.registerCheck<misc::StaticAssertCheck>("cert-dcl03-c");
CheckFactories.registerCheck<readability::UppercaseLiteralSuffixCheck>(
"cert-dcl16-c");
// ENV
CheckFactories.registerCheck<CommandProcessorCheck>("cert-env33-c");
// FLP
Expand All @@ -78,6 +81,13 @@ class CERTModule : public ClangTidyModule {
CheckFactories.registerCheck<ProperlySeededRandomGeneratorCheck>(
"cert-msc32-c");
}

ClangTidyOptions getModuleOptions() override {
ClangTidyOptions Options;
ClangTidyOptions::OptionMap &Opts = Options.CheckOptions;
Opts["cert-dcl16-c.NewSuffixes"] = "L;LL;LU;LLU";
return Options;
}
};

} // namespace cert
Expand Down
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/cert/CMakeLists.txt
Expand Up @@ -24,5 +24,6 @@ add_clang_library(clangTidyCERTModule
clangTidyGoogleModule
clangTidyMiscModule
clangTidyPerformanceModule
clangTidyReadabilityModule
clangTidyUtils
)
3 changes: 3 additions & 0 deletions clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp
Expand Up @@ -35,6 +35,7 @@
#include "../readability/BracesAroundStatementsCheck.h"
#include "../readability/FunctionSizeCheck.h"
#include "../readability/IdentifierNamingCheck.h"
#include "../readability/UppercaseLiteralSuffixCheck.h"
#include "ExceptionBaseclassCheck.h"
#include "MultiwayPathsCoveredCheck.h"
#include "NoAssemblerCheck.h"
Expand Down Expand Up @@ -100,6 +101,8 @@ class HICPPModule : public ClangTidyModule {
"hicpp-use-nullptr");
CheckFactories.registerCheck<modernize::UseOverrideCheck>(
"hicpp-use-override");
CheckFactories.registerCheck<readability::UppercaseLiteralSuffixCheck>(
"hicpp-uppercase-literal-suffix");
CheckFactories.registerCheck<cppcoreguidelines::ProTypeVarargCheck>(
"hicpp-vararg");
}
Expand Down
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/readability/CMakeLists.txt
Expand Up @@ -31,6 +31,7 @@ add_clang_library(clangTidyReadabilityModule
StaticDefinitionInAnonymousNamespaceCheck.cpp
StringCompareCheck.cpp
UniqueptrDeleteReleaseCheck.cpp
UppercaseLiteralSuffixCheck.cpp

LINK_LIBS
clangAST
Expand Down
21 changes: 2 additions & 19 deletions clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
Expand Up @@ -9,6 +9,7 @@

#include "IdentifierNamingCheck.h"

#include "../utils/ASTUtils.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Lex/PPCallbacks.h"
Expand Down Expand Up @@ -681,25 +682,7 @@ static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
if (!Failure.ShouldFix)
return;

// 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;
Failure.ShouldFix = utils::rangeCanBeFixed(Range, SourceMgr);
}

/// Convenience method when the usage to be added is a NamedDecl
Expand Down
Expand Up @@ -38,6 +38,7 @@
#include "StaticDefinitionInAnonymousNamespaceCheck.h"
#include "StringCompareCheck.h"
#include "UniqueptrDeleteReleaseCheck.h"
#include "UppercaseLiteralSuffixCheck.h"

namespace clang {
namespace tidy {
Expand Down Expand Up @@ -102,6 +103,8 @@ class ReadabilityModule : public ClangTidyModule {
"readability-simplify-boolean-expr");
CheckFactories.registerCheck<UniqueptrDeleteReleaseCheck>(
"readability-uniqueptr-delete-release");
CheckFactories.registerCheck<UppercaseLiteralSuffixCheck>(
"readability-uppercase-literal-suffix");
}
};

Expand Down
@@ -0,0 +1,236 @@
//===--- 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
@@ -0,0 +1,44 @@
//===--- 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

0 comments on commit b2eec58

Please sign in to comment.