[clang-tools-extra] r216726 - [clang-tidy] Add a checker that suggests replacing short/long/long long with fixed-size types.
Benjamin Kramer
benny.kra at gmail.com
Tue Sep 2 02:44:30 PDT 2014
On Sun, Aug 31, 2014 at 12:16 AM, Richard Smith <richard at metafoo.co.uk> wrote:
>
>
>
> On Fri, Aug 29, 2014 at 7:38 AM, Benjamin Kramer <benny.kra at googlemail.com>
> wrote:
>>
>> Author: d0k
>> Date: Fri Aug 29 09:38:46 2014
>> New Revision: 216726
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=216726&view=rev
>> Log:
>> [clang-tidy] Add a checker that suggests replacing short/long/long long
>> with fixed-size types.
>>
>> Differential Revision: http://reviews.llvm.org/D5119
>>
>> Added:
>> clang-tools-extra/trunk/clang-tidy/google/IntegerTypesCheck.cpp
>> clang-tools-extra/trunk/clang-tidy/google/IntegerTypesCheck.h
>> clang-tools-extra/trunk/test/clang-tidy/google-runtime-int.cpp
>> Modified:
>> clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt
>> clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp
>>
>> Modified: clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt?rev=216726&r1=216725&r2=216726&view=diff
>>
>> ==============================================================================
>> --- clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt (original)
>> +++ clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt Fri Aug 29
>> 09:38:46 2014
>> @@ -5,6 +5,7 @@ add_clang_library(clangTidyGoogleModule
>> ExplicitConstructorCheck.cpp
>> ExplicitMakePairCheck.cpp
>> GoogleTidyModule.cpp
>> + IntegerTypesCheck.cpp
>> MemsetZeroLengthCheck.cpp
>> NamedParameterCheck.cpp
>> OverloadedUnaryAndCheck.cpp
>>
>> Modified: clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp?rev=216726&r1=216725&r2=216726&view=diff
>>
>> ==============================================================================
>> --- clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp
>> (original)
>> +++ clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp Fri Aug
>> 29 09:38:46 2014
>> @@ -13,6 +13,7 @@
>> #include "AvoidCStyleCastsCheck.h"
>> #include "ExplicitConstructorCheck.h"
>> #include "ExplicitMakePairCheck.h"
>> +#include "IntegerTypesCheck.h"
>> #include "MemsetZeroLengthCheck.h"
>> #include "NamedParameterCheck.h"
>> #include "OverloadedUnaryAndCheck.h"
>> @@ -41,6 +42,9 @@ public:
>> "google-explicit-constructor",
>> new ClangTidyCheckFactory<ExplicitConstructorCheck>());
>> CheckFactories.addCheckFactory(
>> + "google-runtime-int",
>> + new ClangTidyCheckFactory<runtime::IntegerTypesCheck>());
>> + CheckFactories.addCheckFactory(
>> "google-runtime-operator",
>> new ClangTidyCheckFactory<runtime::OverloadedUnaryAndCheck>());
>> CheckFactories.addCheckFactory(
>>
>> Added: clang-tools-extra/trunk/clang-tidy/google/IntegerTypesCheck.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/IntegerTypesCheck.cpp?rev=216726&view=auto
>>
>> ==============================================================================
>> --- clang-tools-extra/trunk/clang-tidy/google/IntegerTypesCheck.cpp
>> (added)
>> +++ clang-tools-extra/trunk/clang-tidy/google/IntegerTypesCheck.cpp Fri
>> Aug 29 09:38:46 2014
>> @@ -0,0 +1,103 @@
>> +//===--- IntegerTypesCheck.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 "IntegerTypesCheck.h"
>> +#include "clang/AST/ASTContext.h"
>> +#include "clang/ASTMatchers/ASTMatchers.h"
>> +#include "clang/ASTMatchers/ASTMatchFinder.h"
>> +#include "clang/Basic/CharInfo.h"
>> +#include "clang/Basic/TargetInfo.h"
>> +
>> +namespace clang {
>> +
>> +namespace ast_matchers {
>> +const internal::VariadicDynCastAllOfMatcher<Decl, TypedefDecl>
>> typedefDecl;
>> +} // namespace ast_matchers
>> +
>> +namespace tidy {
>> +namespace runtime {
>> +
>> +using namespace ast_matchers;
>> +
>> +void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
>> + // Find all TypeLocs.
>> + Finder->addMatcher(typeLoc().bind("tl"), this);
>> +}
>> +
>> +void IntegerTypesCheck::check(const MatchFinder::MatchResult &Result) {
>> + auto TL = *Result.Nodes.getNodeAs<TypeLoc>("tl");
>> + SourceLocation Loc = TL.getLocStart();
>> +
>> + if (Loc.isMacroID())
>> + return;
>> +
>> + // Look through qualification.
>> + if (auto QualLoc = TL.getAs<QualifiedTypeLoc>())
>> + TL = QualLoc.getUnqualifiedLoc();
>> +
>> + auto BuiltinLoc = TL.getAs<BuiltinTypeLoc>();
>> + if (!BuiltinLoc)
>> + return;
>> +
>> + bool IsSigned;
>> + unsigned Width;
>> + const TargetInfo &TargetInfo = Result.Context->getTargetInfo();
>> +
>> + // Look for uses of short, long, long long and their unsigned versions.
>> + switch (BuiltinLoc.getTypePtr()->getKind()) {
>> + case BuiltinType::Short:
>> + Width = TargetInfo.getShortWidth();
>> + IsSigned = true;
>> + break;
>> + case BuiltinType::Long:
>> + Width = TargetInfo.getLongWidth();
>> + IsSigned = true;
>> + break;
>> + case BuiltinType::LongLong:
>> + Width = TargetInfo.getLongLongWidth();
>> + IsSigned = true;
>> + break;
>> + case BuiltinType::UShort:
>> + Width = TargetInfo.getShortWidth();
>> + IsSigned = false;
>> + break;
>> + case BuiltinType::ULong:
>> + Width = TargetInfo.getLongWidth();
>> + IsSigned = false;
>> + break;
>> + case BuiltinType::ULongLong:
>> + Width = TargetInfo.getLongLongWidth();
>> + IsSigned = false;
>> + break;
>> + default:
>> + return;
>> + }
>> +
>> + // We allow "unsigned short port" as that's reasonably common and
>> required by
>> + // the sockets API.
>> + const StringRef Port = "unsigned short port";
>
>
> Should we really be looking for an exact source-level string here? This
> seems surprisingly specific. What about cases like:
>
> unsigned short portion_size;
> unsigned short port_in, port_out;
> unsigned short port, something_else;
We get those right. What doesn't work is 'unsigned short foo, port;'.
Implementing this correctly is tricky with recursive visitation of
typelocs.
take for example.
void foo(unsigned short port);
We will visit 'unsigned short' twice, once coming from the function
type 'void (unsigned short)' and once from the ParmVarDecl. For the
second case we see the name, for the first one we don't.
I went with the easy route for now as this specific rule (whitelisting
port) seems like a hack anyways. Maybe we'll come up with something
better.
- Ben
>
>> + const char *Data = Result.SourceManager->getCharacterData(Loc);
>> + if (!std::strncmp(Data, Port.data(), Port.size()) &&
>> + !isIdentifierBody(Data[Port.size()]))
>> + return;
>> +
>> + std::string Replacement =
>> + ((IsSigned ? SignedTypePrefix : UnsignedTypePrefix) + Twine(Width)
>> +
>> + (AddUnderscoreT ? "_t" : "")).str();
>> +
>> + // We don't add a fix-it as changing the type can easily break code,
>> + // e.g. when a function requires a 'long' argument on all platforms.
>> + // QualTypes are printed with implicit quotes.
>> + diag(Loc, "consider replacing %0 with '%1'") << BuiltinLoc.getType()
>> + << Replacement;
>> +}
>> +
>> +} // namespace runtime
>> +} // namespace tidy
>> +} // namespace clang
>>
>> Added: clang-tools-extra/trunk/clang-tidy/google/IntegerTypesCheck.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/IntegerTypesCheck.h?rev=216726&view=auto
>>
>> ==============================================================================
>> --- clang-tools-extra/trunk/clang-tidy/google/IntegerTypesCheck.h (added)
>> +++ clang-tools-extra/trunk/clang-tidy/google/IntegerTypesCheck.h Fri Aug
>> 29 09:38:46 2014
>> @@ -0,0 +1,40 @@
>> +//===--- IntegerTypesCheck.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_GOOGLE_INTEGERTYPESCHECK_H
>> +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_INTEGERTYPESCHECK_H
>> +
>> +#include "../ClangTidy.h"
>> +
>> +namespace clang {
>> +namespace tidy {
>> +namespace runtime {
>> +
>> +/// \brief Finds uses of short, long and long long and suggest replacing
>> them
>> +/// with u?intXX(_t)?.
>> +/// Correspondig cpplint.py check: runtime/int.
>> +class IntegerTypesCheck : public ClangTidyCheck {
>> +public:
>> + IntegerTypesCheck()
>> + : UnsignedTypePrefix("uint"), SignedTypePrefix("int"),
>> + AddUnderscoreT(false) {}
>> + void registerMatchers(ast_matchers::MatchFinder *Finder) override;
>> + void check(const ast_matchers::MatchFinder::MatchResult &Result)
>> override;
>> +
>> +private:
>> + const StringRef UnsignedTypePrefix;
>> + const StringRef SignedTypePrefix;
>> + const bool AddUnderscoreT;
>> +};
>> +
>> +} // namespace runtime
>> +} // namespace tidy
>> +} // namespace clang
>> +
>> +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_INTEGERTYPESCHECK_H
>>
>> Added: clang-tools-extra/trunk/test/clang-tidy/google-runtime-int.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-runtime-int.cpp?rev=216726&view=auto
>>
>> ==============================================================================
>> --- clang-tools-extra/trunk/test/clang-tidy/google-runtime-int.cpp (added)
>> +++ clang-tools-extra/trunk/test/clang-tidy/google-runtime-int.cpp Fri Aug
>> 29 09:38:46 2014
>> @@ -0,0 +1,54 @@
>> +// RUN: clang-tidy -checks=-*,google-runtime-int %s -- -x c++ 2>&1 |
>> FileCheck %s -implicit-check-not='{{warning:|error:}}'
>> +
>> +long a();
>> +// CHECK: [[@LINE-1]]:1: warning: consider replacing 'long' with 'int64'
>> +
>> +typedef unsigned long long uint64; // NOLINT
>> +
>> +long b(long = 1);
>> +// CHECK: [[@LINE-1]]:1: warning: consider replacing 'long' with 'int64'
>> +// CHECK: [[@LINE-2]]:8: warning: consider replacing 'long' with 'int64'
>> +
>> +template <typename T>
>> +void tmpl() {
>> + T i;
>> +}
>> +
>> +short bar(const short, unsigned short) {
>> +// CHECK: [[@LINE-1]]:1: warning: consider replacing 'short' with 'int16'
>> +// CHECK: [[@LINE-2]]:17: warning: consider replacing 'short' with
>> 'int16'
>> +// CHECK: [[@LINE-3]]:24: warning: consider replacing 'unsigned short'
>> with 'uint16'
>> + long double foo = 42;
>> + uint64 qux = 42;
>> + unsigned short port;
>> +
>> + const unsigned short bar = 0;
>> +// CHECK: [[@LINE-1]]:9: warning: consider replacing 'unsigned short'
>> with 'uint16'
>> + long long *baar;
>> +// CHECK: [[@LINE-1]]:3: warning: consider replacing 'long long' with
>> 'int64'
>> + const unsigned short &bara = bar;
>> +// CHECK: [[@LINE-1]]:9: warning: consider replacing 'unsigned short'
>> with 'uint16'
>> + long const long moo = 1;
>> +// CHECK: [[@LINE-1]]:3: warning: consider replacing 'long long' with
>> 'int64'
>> + long volatile long wat = 42;
>> +// CHECK: [[@LINE-1]]:3: warning: consider replacing 'long long' with
>> 'int64'
>> + unsigned long y;
>> +// CHECK: [[@LINE-1]]:3: warning: consider replacing 'unsigned long' with
>> 'uint64'
>> + unsigned long long **const *tmp;
>> +// CHECK: [[@LINE-1]]:3: warning: consider replacing 'unsigned long long'
>> with 'uint64'
>> + unsigned long long **const *&z = tmp;
>> +// CHECK: [[@LINE-1]]:3: warning: consider replacing 'unsigned long long'
>> with 'uint64'
>> + unsigned short porthole;
>> +// CHECK: [[@LINE-1]]:3: warning: consider replacing 'unsigned short'
>> with 'uint16'
>> +
>> +#define l long
>> + l x;
>> +
>> + tmpl<short>();
>> +// CHECK: [[@LINE-1]]:8: warning: consider replacing 'short' with 'int16'
>> +}
>> +
>> +void qux() {
>> + short port;
>> +// CHECK: [[@LINE-1]]:3: warning: consider replacing 'short' with 'int16'
>> +}
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
More information about the cfe-commits
mailing list