From ecc7dae50c41bc8a129a158ecf0ae0270126505c Mon Sep 17 00:00:00 2001 From: Alexander Lanin Date: Wed, 22 Jan 2020 08:45:20 -0500 Subject: [PATCH] Extend misc-misplaced-const to detect using declarations as well as typedef --- .../clang-tidy/misc/MisplacedConstCheck.cpp | 38 ++++++++++---- .../checks/misc-misplaced-const.rst | 21 ++++---- .../checkers/misc-misplaced-const.cpp | 52 +++++++++++++------ 3 files changed, 75 insertions(+), 36 deletions(-) diff --git a/clang-tools-extra/clang-tidy/misc/MisplacedConstCheck.cpp b/clang-tools-extra/clang-tidy/misc/MisplacedConstCheck.cpp index 1e1b2b0d303b..7a028df588ff 100644 --- a/clang-tools-extra/clang-tidy/misc/MisplacedConstCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/MisplacedConstCheck.cpp @@ -17,13 +17,16 @@ namespace tidy { namespace misc { void MisplacedConstCheck::registerMatchers(MatchFinder *Finder) { + auto NonConstAndNonFunctionPointerType = hasType(pointerType(unless( + pointee(anyOf(isConstQualified(), ignoringParens(functionType())))))); + Finder->addMatcher( - valueDecl(hasType(isConstQualified()), - hasType(typedefType(hasDeclaration( - typedefDecl(hasType(pointerType(unless(pointee( - anyOf(isConstQualified(), - ignoringParens(functionType()))))))) - .bind("typedef"))))) + valueDecl( + hasType(isConstQualified()), + hasType(typedefType(hasDeclaration(anyOf( + typedefDecl(NonConstAndNonFunctionPointerType).bind("typedef"), + typeAliasDecl(NonConstAndNonFunctionPointerType) + .bind("typeAlias")))))) .bind("decl"), this); } @@ -45,16 +48,29 @@ static QualType guessAlternateQualification(ASTContext &Context, QualType QT) { void MisplacedConstCheck::check(const MatchFinder::MatchResult &Result) { const auto *Var = Result.Nodes.getNodeAs("decl"); - const auto *Typedef = Result.Nodes.getNodeAs("typedef"); ASTContext &Ctx = *Result.Context; QualType CanQT = Var->getType().getCanonicalType(); - diag(Var->getLocation(), "%0 declared with a const-qualified typedef type; " - "results in the type being '%1' instead of '%2'") - << Var << CanQT.getAsString(Ctx.getPrintingPolicy()) + SourceLocation AliasLoc; + const char *AliasType; + if (const auto *Typedef = Result.Nodes.getNodeAs("typedef")) { + AliasLoc = Typedef->getLocation(); + AliasType = "typedef"; + } else if (const auto *TypeAlias = + Result.Nodes.getNodeAs("typeAlias")) { + AliasLoc = TypeAlias->getLocation(); + AliasType = "type alias"; + } else { + llvm_unreachable("registerMatchers has registered an unknown matcher," + " code out of sync"); + } + + diag(Var->getLocation(), "%0 declared with a const-qualified %1; " + "results in the type being '%2' instead of '%3'") + << Var << AliasType << CanQT.getAsString(Ctx.getPrintingPolicy()) << guessAlternateQualification(Ctx, CanQT) .getAsString(Ctx.getPrintingPolicy()); - diag(Typedef->getLocation(), "typedef declared here", DiagnosticIDs::Note); + diag(AliasLoc, "%0 declared here", DiagnosticIDs::Note) << AliasType; } } // namespace misc diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc-misplaced-const.rst b/clang-tools-extra/docs/clang-tidy/checks/misc-misplaced-const.rst index ee1549f11b2d..e583ecb54cac 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/misc-misplaced-const.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/misc-misplaced-const.rst @@ -3,10 +3,10 @@ misc-misplaced-const ==================== -This check diagnoses when a ``const`` qualifier is applied to a ``typedef`` to a -pointer type rather than to the pointee, because such constructs are often -misleading to developers because the ``const`` applies to the pointer rather -than the pointee. +This check diagnoses when a ``const`` qualifier is applied to a ``typedef``/ +``using`` to a pointer type rather than to the pointee, because such constructs +are often misleading to developers because the ``const`` applies to the pointer +rather than the pointee. For instance, in the following code, the resulting type is ``int *`` ``const`` rather than ``const int *``: @@ -14,9 +14,12 @@ rather than ``const int *``: .. code-block:: c++ typedef int *int_ptr; - void f(const int_ptr ptr); + void f(const int_ptr ptr) { + *ptr = 0; // potentially quite unexpectedly the int can be modified here + ptr = 0; // does not compile + } -The check does not diagnose when the underlying ``typedef`` type is a pointer to -a ``const`` type or a function pointer type. This is because the ``const`` -qualifier is less likely to be mistaken because it would be redundant (or -disallowed) on the underlying pointee type. +The check does not diagnose when the underlying ``typedef``/``using`` type is a +pointer to a ``const`` type or a function pointer type. This is because the +``const`` qualifier is less likely to be mistaken because it would be redundant +(or disallowed) on the underlying pointee type. diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc-misplaced-const.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc-misplaced-const.cpp index d7ea893c2156..4f602a6d86fa 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc-misplaced-const.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc-misplaced-const.cpp @@ -1,20 +1,40 @@ -// RUN: %check_clang_tidy %s misc-misplaced-const %t +// RUN: %check_clang_tidy %s misc-misplaced-const %t -- -- -DUSING +// RUN: %check_clang_tidy %s misc-misplaced-const %t -- -- -DTYPEDEF -typedef int plain_i; -typedef int *ip; -typedef const int *cip; +#ifdef TYPEDEF +typedef int int_; +typedef int *ptr_to_int; +typedef const int *ptr_to_const_int; +#endif +#ifdef USING +using int_ = int; +using ptr_to_int = int *; +using ptr_to_const_int = const int *; +#endif -void func() { - if (const int *i = 0) - ; - if (const plain_i *i = 0) - ; - if (const cip i = 0) - ; +void const_pointers() { + if (const int *i = 0) { + i = 0; + // *i = 0; + } - // CHECK-MESSAGES: :[[@LINE+1]]:16: warning: 'i' declared with a const-qualified typedef type; results in the type being 'int *const' instead of 'const int *' - if (const ip i = 0) - ; + if (const int_ *i = 0) { + i = 0; + // *i = 0; + } + + if (const ptr_to_const_int i = 0) { + // i = 0; + // *i = 0; + } + + // Potentially quite unexpectedly the int can be modified here + // CHECK-MESSAGES: :[[@LINE+1]]:23: warning: 'i' declared with a const-qualified {{.*}}; results in the type being 'int *const' instead of 'const int *' + if (const ptr_to_int i = 0) { + //i = 0; + + *i = 0; + } } template @@ -24,8 +44,8 @@ struct S { }; template struct S; -template struct S; // ok -template struct S; +template struct S; // ok +template struct S; template struct U {