[clang-tools-extra] r216726 - [clang-tidy] Add a checker that suggests replacing short/long/long long with fixed-size types.

Richard Smith richard at metafoo.co.uk
Sat Aug 30 15:16:29 PDT 2014


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;

+  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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140830/2b703778/attachment.html>


More information about the cfe-commits mailing list