whisperity updated this revision to Diff 326440.
whisperity added a comment.
- **[NFC]** Reformat the documentation so option values are in single backticks.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95736/new/
https://reviews.llvm.org/D95736
Files:
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
@@ -106,20 +106,38 @@
// CHECK-MESSAGES: :[[@LINE-2]]:32: note: the first parameter in the range is 'I1'
// CHECK-MESSAGES: :[[@LINE-3]]:43: note: the last parameter in the range is 'I2'
-void throughTypedef(int I, MyInt1 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'throughTypedef' of similar type ('int')
-// CHECK-MESSAGES: :[[@LINE-2]]:25: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:35: note: the last parameter in the range is 'J'
+void typedefMultiple(MyInt1 I1, MyInt2 I2x, MyInt2 I2y) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 3 adjacent parameters of 'typedefMultiple' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I1'
+// CHECK-MESSAGES: :[[@LINE-3]]:52: note: the last parameter in the range is 'I2y'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+void throughTypedef1(int I, MyInt1 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'throughTypedef1' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:26: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:36: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+
+void betweenTypedef2(MyInt1 I, MyInt2 J) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 2 adjacent parameters of 'betweenTypedef2' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:29: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:39: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-4]]:22: note: after resolving type aliases, the common type of 'MyInt1' and 'MyInt2' is 'int'
+
+typedef MyInt2 MyInt2b;
-void betweenTypedef(MyInt1 I, MyInt2 J) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 2 adjacent parameters of 'betweenTypedef' of similar type ('MyInt1')
-// CHECK-MESSAGES: :[[@LINE-2]]:28: note: the first parameter in the range is 'I'
-// CHECK-MESSAGES: :[[@LINE-3]]:38: note: the last parameter in the range is 'J'
+void typedefChain(int I, MyInt1 MI1, MyInt2 MI2, MyInt2b MI2b) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: 4 adjacent parameters of 'typedefChain' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:23: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:58: note: the last parameter in the range is 'MI2b'
+// CHECK-MESSAGES: :[[@LINE-4]]:19: note: after resolving type aliases, 'int' and 'MyInt1' are the same
+// CHECK-MESSAGES: :[[@LINE-5]]:19: note: after resolving type aliases, 'int' and 'MyInt2' are the same
+// CHECK-MESSAGES: :[[@LINE-6]]:19: note: after resolving type aliases, 'int' and 'MyInt2b' are the same
typedef long MyLong1;
using MyLong2 = long;
-void throughTypedefToOtherType(MyInt1 I, MyLong1 J) {} // NO-WARN: Not the same type.
+void throughTypedefToOtherType(MyInt1 I, MyLong1 J) {} // NO-WARN: int and long.
void qualified1(int I, const int CI) {} // NO-WARN: Not the same type.
@@ -133,18 +151,73 @@
void qualifiedThroughTypedef1(int I, CInt CI) {} // NO-WARN: Not the same type.
-void qualifiedThroughTypedef2(CInt CI1, const int CI2) {} // NO-WARN: Not the same type.
-// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'qualifiedThroughTypedef2' of similar type ('CInt')
+void qualifiedThroughTypedef2(CInt CI1, const int CI2) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'qualifiedThroughTypedef2' of similar type are
// CHECK-MESSAGES: :[[@LINE-2]]:36: note: the first parameter in the range is 'CI1'
// CHECK-MESSAGES: :[[@LINE-3]]:51: note: the last parameter in the range is 'CI2'
-
-void reference1(int I, int &IR) {} // NO-WARN: Not the same type.
-
-void reference2(int I, const int &CIR) {} // NO-WARN: Not the same type.
-
-void reference3(int I, int &&IRR) {} // NO-WARN: Not the same type.
-
-void reference4(int I, const int &&CIRR) {} // NO-WARN: Not the same type.
+// CHECK-MESSAGES: :[[@LINE-4]]:31: note: after resolving type aliases, 'CInt' and 'const int' are the same
+
+void qualifiedThroughTypedef3(CInt CI1, const MyInt1 CI2, const int CI3) {} // NO-WARN: Not the same type.
+
+void qualifiedThroughTypedef4(CInt CI1, const MyInt1 CI2, const MyInt2 CI3) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:41: warning: 2 adjacent parameters of 'qualifiedThroughTypedef4' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:54: note: the first parameter in the range is 'CI2'
+// CHECK-MESSAGES: :[[@LINE-3]]:72: note: the last parameter in the range is 'CI3'
+// CHECK-MESSAGES: :[[@LINE-4]]:41: note: after resolving type aliases, the common type of 'const MyInt1' and 'const MyInt2' is 'int'
+
+void reference1(int I, int &IR) {} // NO-WARN: Distinct semantics when called.
+
+void reference2(int I, const int &CIR) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: 2 adjacent parameters of 'reference2' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:21: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:35: note: the last parameter in the range is 'CIR'
+// CHECK-MESSAGES: :[[@LINE-4]]:24: note: a call site binds an expression to 'int' and 'const int &'
+
+void reference3(int I, int &&IRR) {} // NO-WARN: Distinct semantics when called.
+
+void reference4(int I, const int &&CIRR) {} // NO-WARN: Distinct semantics when called.
+
+void reference5(const int CI, const int &CIR) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: 2 adjacent parameters of 'reference5' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:27: note: the first parameter in the range is 'CI'
+// CHECK-MESSAGES: :[[@LINE-3]]:42: note: the last parameter in the range is 'CIR'
+// CHECK-MESSAGES: :[[@LINE-4]]:31: note: a call site binds an expression to 'const int' and 'const int &'
+
+void reference6(int I, const int &CIR, int J, const int &CJR) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: 4 adjacent parameters of 'reference6' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:21: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:58: note: the last parameter in the range is 'CJR'
+// CHECK-MESSAGES: :[[@LINE-4]]:24: note: a call site binds an expression to 'int' and 'const int &'
+
+using ICRTy = const int &;
+using MyIntCRTy = const MyInt1 &;
+
+void referenceThroughTypedef(int I, ICRTy Builtin, MyIntCRTy MyInt) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: 3 adjacent parameters of 'referenceThroughTypedef' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:34: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:62: note: the last parameter in the range is 'MyInt'
+// CHECK-MESSAGES: :[[@LINE-4]]:30: note: after resolving type aliases, the common type of 'int' and 'ICRTy' is 'const int'
+// CHECK-MESSAGES: :[[@LINE-5]]:37: note: a call site binds an expression to 'int' and 'ICRTy'
+// CHECK-MESSAGES: :[[@LINE-6]]:30: note: after resolving type aliases, 'int' and 'MyIntCRTy' are the same
+// CHECK-MESSAGES: :[[@LINE-7]]:52: note: a call site binds an expression to 'int' and 'MyIntCRTy'
+// CHECK-MESSAGES: :[[@LINE-8]]:37: note: after resolving type aliases, the common type of 'ICRTy' and 'MyIntCRTy' is 'int'
+// CHECK-MESSAGES: :[[@LINE-9]]:52: note: a call site binds an expression to 'ICRTy' and 'MyIntCRTy'
+
+short const typedef int unsigned Eldritch;
+typedef const unsigned short Holy;
+
+void collapse(Eldritch Cursed, Holy Blessed) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 2 adjacent parameters of 'collapse' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:24: note: the first parameter in the range is 'Cursed'
+// CHECK-MESSAGES: :[[@LINE-3]]:37: note: the last parameter in the range is 'Blessed'
+// CHECK-MESSAGES: :[[@LINE-4]]:15: note: after resolving type aliases, the common type of 'Eldritch' and 'Holy' is 'const unsigned short'
+
+void collapseAndTypedef(Eldritch Cursed, const Holy &Blessed) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: 2 adjacent parameters of 'collapseAndTypedef' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:34: note: the first parameter in the range is 'Cursed'
+// CHECK-MESSAGES: :[[@LINE-3]]:54: note: the last parameter in the range is 'Blessed'
+// CHECK-MESSAGES: :[[@LINE-4]]:25: note: after resolving type aliases, the common type of 'Eldritch' and 'const Holy &' is 'const unsigned short'
+// CHECK-MESSAGES: :[[@LINE-5]]:42: note: a call site binds an expression to 'Eldritch' and 'const Holy &'
template <typename T1, typename T2>
struct Pair {};
@@ -177,3 +250,54 @@
// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 3 adjacent parameters of 'templateVariadic2<int, int, int>' of similar type ('int')
// CHECK-MESSAGES: :[[@LINE-2]]:28: note: the first parameter in the range is 'TVar'
// CHECK-MESSAGES: :[[@LINE-3]]:50: note: the last parameter in the range is 'UVars2'
+
+template <typename T>
+using TwoOf = Pair<T, T>;
+
+void templateAndAliasTemplate(Pair<int, int> P, TwoOf<int> I) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 2 adjacent parameters of 'templateAndAliasTemplate' of similar type ('Pair<int, int>')
+// CHECK-MESSAGES: :[[@LINE-2]]:46: note: the first parameter in the range is 'P'
+// CHECK-MESSAGES: :[[@LINE-3]]:60: note: the last parameter in the range is 'I'
+
+template <typename T>
+struct Vector {
+ typedef T element_type;
+ typedef T &reference_type;
+ typedef const T const_element_type;
+ typedef const T &const_reference_type;
+};
+
+void memberTypedef(int I, Vector<int>::element_type E) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 2 adjacent parameters of 'memberTypedef' of similar type are
+// CHECK-MESSAGES: :[[@LINE-2]]:24: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-3]]:53: note: the last parameter in the range is 'E'
+// CHECK-MESSAGES: :[[@LINE-4]]:20: note: after resolving type aliases, 'int' and 'Vector<int>::element_type' are the same
+
+template <typename T>
+void memberTypedefDependent1(T T1, typename Vector<T>::element_type T2) {} // NO-WARN: Dependent name is not instantiated and resolved against other type.
+
+template <typename T>
+void memberTypedefDependent2(typename Vector<T>::element_type E1,
+ typename Vector<T>::element_type E2) {}
+// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: 2 adjacent parameters of 'memberTypedefDependent2' of similar type ('typename Vector<T>::element_type')
+// CHECK-MESSAGES: :[[@LINE-3]]:63: note: the first parameter in the range is 'E1'
+// CHECK-MESSAGES: :[[@LINE-3]]:63: note: the last parameter in the range is 'E2'
+
+template <typename T>
+void memberTypedefDependentReference1(
+ typename Vector<T>::element_type E,
+ typename Vector<T>::const_element_type &R) {} // NO-WARN: Not instantiated.
+
+template <typename T>
+void memberTypedefDependentReference2(
+ typename Vector<T>::element_type E,
+ typename Vector<T>::const_reference_type R) {} // NO-WARN: Not instantiated.
+
+template <typename T>
+void memberTypedefDependentReference3(
+ typename Vector<T>::element_type E,
+ const typename Vector<T>::element_type &R) {}
+// CHECK-MESSAGES: :[[@LINE-2]]:5: warning: 2 adjacent parameters of 'memberTypedefDependentReference3' of similar type are
+// CHECK-MESSAGES: :[[@LINE-3]]:38: note: the first parameter in the range is 'E'
+// CHECK-MESSAGES: :[[@LINE-3]]:45: note: the last parameter in the range is 'R'
+// CHECK-MESSAGES: :[[@LINE-4]]:5: note: a call site binds an expression to 'typename Vector<T>::element_type' and 'const typename Vector<T>::element_type &'
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
@@ -111,3 +111,28 @@
add(1, 2); // Instantiates 'add<int, int>', but that's not a user-defined function.
}
+
+Due to the limitation above, parameters which type are further dependent upon
+template instantiations to *prove* that they mix with another parameter's is
+not diagnosed.
+
+.. code-block:: c++
+
+ template <typename T>
+ struct Vector {
+ typedef T element_type;
+ };
+
+ // Diagnosed: Explicit instantiation was done by the user, we can prove it
+ // is the same type.
+ void Explicit(int A, Vector<int>::element_type B) { /* ... */ }
+
+ // Diagnosed: The two parameter types are exactly the same.
+ template <typename T>
+ void Exact(typename Vector<T>::element_type A,
+ typename Vector<T>::element_type B) { /* ... */ }
+
+ // Skipped: The two parameters are both 'T' but we can not prove this
+ // without actually instantiating.
+ template <typename T>
+ void FalseNegative(T A, typename Vector<T>::element_type B) { /* ... */ }
Index: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
@@ -11,6 +11,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
+#include "llvm/ADT/SmallSet.h"
#define DEBUG_TYPE "EasilySwappableParametersCheck"
#include "llvm/Support/Debug.h"
@@ -103,27 +104,61 @@
#define FB(Name, K) MIX_##Name = (1ull << (K##ull - 1ull))
- FB(None, 1), //< Mix between the two parameters is not possible.
- FB(Trivial, 2), //< The two mix trivially, and are the exact same type.
- FB(Canonical, 3), //< The two mix because the types refer to the same
- // CanonicalType, but we do not elaborate as to how.
+ FB(None, 1), //< Mix between the two parameters is not possible.
+ FB(Trivial, 2), //< The two mix trivially, and are the exact same type.
+ FB(Canonical, 3), //< The two mix because the types refer to the same
+ // CanonicalType, but we do not elaborate as to how.
+ FB(TypeAlias, 4), //< The path from one type to the other involves
+ // desugaring type aliases.
+ FB(ReferenceBind, 5), //< The mix involves the binding power of "const &".
#undef FB
- LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue =*/MIX_Canonical)
+ LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue =*/MIX_ReferenceBind)
};
LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
/// Contains the metadata for the mixability result between two types,
/// independently of which parameters they were calculated from.
struct MixData {
+ /// The flag bits of the mix indicating what language features allow for it.
MixFlags Flags;
+ /// A potentially calculated common underlying type after desugaring, that
+ /// both sides of the mix can originate from.
+ QualType CommonType;
+
MixData(MixFlags Flags) : Flags(Flags) {}
+ MixData(MixFlags Flags, QualType CommonType)
+ : Flags(Flags), CommonType(CommonType) {}
void sanitize() {
assert(Flags != MIX_Invalid && "sanitize() called on invalid bitvec");
- // TODO: There will be statements here in further extensions of the check.
+
+ if (Flags & MIX_None) {
+ // If anywhere down the recursion a potential mix "path" is deemed
+ // impossible, throw away all the other bits because the mix is not
+ // possible.
+ Flags = MIX_None;
+ return;
+ }
+
+ if (Flags == MIX_Trivial)
+ return;
+
+ if (Flags ^ MIX_Trivial)
+ // If the mix involves somewhere trivial equivalence but down the
+ // recursion other bit(s) were set, remove the trivial bit, as it is not
+ // trivial.
+ Flags &= ~MIX_Trivial;
+ }
+
+ MixData operator|(MixFlags EnableFlags) const {
+ return {Flags | EnableFlags, CommonType};
+ }
+ MixData &operator|=(MixFlags EnableFlags) {
+ Flags |= EnableFlags;
+ return *this;
}
};
@@ -138,6 +173,7 @@
void sanitize() { Data.sanitize(); }
MixFlags flags() const { return Data.Flags; }
+ QualType commonUnderlyingType() const { return Data.CommonType; }
};
static_assert(std::is_trivially_copyable<Mix>::value &&
@@ -173,6 +209,11 @@
}
};
+static MixData isLRefEquallyBindingToType(const TheCheck &Check,
+ const LValueReferenceType *LRef,
+ QualType Ty, const ASTContext &Ctx,
+ bool IsRefRHS);
+
/// Approximate the way how LType and RType might refer to "essentially the
/// same" type, in a sense that at a particular call site, an expression of
/// type LType and RType might be successfully passed to a variable (in our
@@ -191,24 +232,94 @@
if (LType == RType) {
LLVM_DEBUG(llvm::dbgs() << "<<< calculateMixability. Trivial equality.\n");
- return {MIX_Trivial};
+ return {MIX_Trivial, LType};
}
- // TODO: Implement more elaborate logic, such as typedef, implicit
- // conversions, etc.
+ // Dissolve certain type sugars that do not affect the mixability of one type
+ // with the other, and also do not require any sort of elaboration for the
+ // user to understand.
+ if (isa<ParenType>(LType.getTypePtr())) {
+ LLVM_DEBUG(llvm::dbgs() << "--- calculateMixability. LHS is ParenType.\n");
+ return calculateMixability(Check, LType.getSingleStepDesugaredType(Ctx),
+ RType, Ctx);
+ }
+ if (isa<ParenType>(RType.getTypePtr())) {
+ LLVM_DEBUG(llvm::dbgs() << "--- calculateMixability. RHS is ParenType.\n");
+ return calculateMixability(Check, LType,
+ RType.getSingleStepDesugaredType(Ctx), Ctx);
+ }
+
+ // Dissolve typedefs.
+ if (const auto *LTypedef = LType->getAs<TypedefType>()) {
+ LLVM_DEBUG(llvm::dbgs() << "--- calculateMixability. LHS is typedef.\n");
+ return calculateMixability(Check, LTypedef->desugar(), RType, Ctx) |
+ MIX_TypeAlias;
+ }
+ if (const auto *RTypedef = RType->getAs<TypedefType>()) {
+ LLVM_DEBUG(llvm::dbgs() << "--- calculateMixability. RHS is typedef.\n");
+ return calculateMixability(Check, LType, RTypedef->desugar(), Ctx) |
+ MIX_TypeAlias;
+ }
+
+ // At a particular call site, what could be passed to a 'T' or 'const T' might
+ // also be passed to a 'const T &' without the call site putting a direct
+ // side effect on the passed expressions.
+ if (const auto *LRef = LType->getAs<LValueReferenceType>()) {
+ LLVM_DEBUG(llvm::dbgs() << "--- calculateMixability. LHS is &.\n");
+ return isLRefEquallyBindingToType(Check, LRef, RType, Ctx, false) |
+ MIX_ReferenceBind;
+ }
+ if (const auto *RRef = RType->getAs<LValueReferenceType>()) {
+ LLVM_DEBUG(llvm::dbgs() << "--- calculateMixability. RHS is &.\n");
+ return isLRefEquallyBindingToType(Check, RRef, LType, Ctx, true) |
+ MIX_ReferenceBind;
+ }
// If none of the previous logic found a match, try if Clang otherwise
// believes the types to be the same.
if (LType.getCanonicalType() == RType.getCanonicalType()) {
LLVM_DEBUG(llvm::dbgs()
<< "<<< calculateMixability. Same CanonicalType.\n");
- return {MIX_Canonical};
+ return {MIX_Canonical, LType.getCanonicalType()};
}
LLVM_DEBUG(llvm::dbgs() << "<<< calculateMixability. No match found.\n");
return {MIX_None};
}
+/// Calculates if the reference binds an expression of the given type. This is
+/// true iff 'LRef' is some 'const T &' type, and the 'Ty' is 'T' or 'const T'.
+static MixData isLRefEquallyBindingToType(const TheCheck &Check,
+ const LValueReferenceType *LRef,
+ QualType Ty, const ASTContext &Ctx,
+ bool IsRefRHS) {
+ LLVM_DEBUG(llvm::dbgs() << ">>> isLRefEquallyBindingToType for LRef:\n";
+ LRef->dump(llvm::dbgs(), Ctx); llvm::dbgs() << "\nand Type:\n";
+ Ty.dump(llvm::dbgs(), Ctx); llvm::dbgs() << '\n';);
+
+ QualType ReferredType = LRef->getPointeeType();
+ if (!ReferredType.isLocalConstQualified()) {
+ LLVM_DEBUG(llvm::dbgs()
+ << "<<< isLRefEquallyBindingToType. Not const ref.\n");
+ return {MIX_None};
+ };
+
+ QualType NonConstReferredType = ReferredType;
+ NonConstReferredType.removeLocalConst();
+ if (ReferredType == Ty || NonConstReferredType == Ty) {
+ LLVM_DEBUG(
+ llvm::dbgs()
+ << "<<< isLRefEquallyBindingToType. Type of referred matches.\n");
+ return {MIX_Trivial, ReferredType};
+ }
+
+ LLVM_DEBUG(
+ llvm::dbgs()
+ << "--- isLRefEquallyBindingToType. Checking mix for underlying type.\n");
+ return IsRefRHS ? calculateMixability(Check, Ty, NonConstReferredType, Ctx)
+ : calculateMixability(Check, NonConstReferredType, Ty, Ctx);
+}
+
static MixableParameterRange modelMixingRange(const TheCheck &Check,
const FunctionDecl *FD,
std::size_t StartIndex) {
@@ -329,6 +440,7 @@
return false;
}
+
} // namespace filter
/// Matches functions that have at least the specified amount of parameters.
@@ -375,6 +487,84 @@
return Name;
}
+/// Returns whether a particular Mix between two parameters should have the
+/// types involved diagnosed to the user. This is only a flag check.
+static inline bool needsToPrintTypeInDiagnostic(const model::Mix &M) {
+ return M.flags() & (model::MIX_TypeAlias | model::MIX_ReferenceBind);
+}
+
+namespace {
+
+/// Retains the elements called with and returns whether the call is done with
+/// a new element.
+template <typename E, std::size_t N> class InsertOnce {
+ llvm::SmallSet<E, N> CalledWith;
+
+public:
+ bool operator()(E El) { return CalledWith.insert(std::move(El)).second; }
+
+ bool calledWith(const E &El) const { return CalledWith.contains(El); }
+};
+
+struct SwappedEqualQualTypePair {
+ QualType LHSType, RHSType;
+
+ bool operator==(const SwappedEqualQualTypePair &Other) const {
+ return (LHSType == Other.LHSType && RHSType == Other.RHSType) ||
+ (LHSType == Other.RHSType && RHSType == Other.LHSType);
+ }
+
+ bool operator<(const SwappedEqualQualTypePair &Other) const {
+ return LHSType < Other.LHSType && RHSType < Other.RHSType;
+ }
+};
+
+struct TypeAliasDiagnosticTuple {
+ QualType LHSType, RHSType, CommonType;
+
+ bool operator==(const TypeAliasDiagnosticTuple &Other) const {
+ return CommonType == Other.CommonType &&
+ ((LHSType == Other.LHSType && RHSType == Other.RHSType) ||
+ (LHSType == Other.RHSType && RHSType == Other.LHSType));
+ }
+
+ bool operator<(const TypeAliasDiagnosticTuple &Other) const {
+ return CommonType < Other.CommonType && LHSType < Other.LHSType &&
+ RHSType < Other.RHSType;
+ }
+};
+
+/// Helper class to only emit a diagnostic related to MIX_TypeAlias once.
+class UniqueTypeAliasDiagnosticHelper
+ : public InsertOnce<TypeAliasDiagnosticTuple, 8> {
+ using Base = InsertOnce<TypeAliasDiagnosticTuple, 8>;
+
+public:
+ /// Returns whether the diagnostic for LHSType and RHSType which are both
+ /// referring to CommonType being the same has not been emitted already.
+ bool operator()(QualType LHSType, QualType RHSType, QualType CommonType) {
+ if (CommonType.isNull() || CommonType == LHSType || CommonType == RHSType)
+ return Base::operator()({LHSType, RHSType, {}});
+
+ TypeAliasDiagnosticTuple ThreeTuple{LHSType, RHSType, CommonType};
+ if (!Base::operator()(ThreeTuple))
+ return false;
+
+ bool AlreadySaidLHSAndCommonIsSame = calledWith({LHSType, CommonType, {}});
+ bool AlreadySaidRHSAndCommonIsSame = calledWith({RHSType, CommonType, {}});
+ if (AlreadySaidLHSAndCommonIsSame && AlreadySaidRHSAndCommonIsSame) {
+ // "SomeInt == int" && "SomeOtherInt == int" => "Common(SomeInt,
+ // SomeOtherInt) == int", no need to diagnose it. Save the 3-tuple only
+ // for shortcut if it ever appears again.
+ return false;
+ }
+
+ return true;
+ }
+};
+
+} // namespace
+
EasilySwappableParametersCheck::EasilySwappableParametersCheck(
StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
@@ -443,17 +633,23 @@
continue;
}
+ bool NeedsAnyTypeNote = llvm::any_of(R.Mixes, needsToPrintTypeInDiagnostic);
const ParmVarDecl *First = R.getFirstParam(), *Last = R.getLastParam();
std::string FirstParamTypeAsWritten = First->getType().getAsString(PP);
{
- StringRef DiagText = "%0 adjacent parameters of %1 of similar type "
- "('%2') are easily swapped by mistake";
- // TODO: This logic will get extended here with future flags.
+ StringRef DiagText;
+
+ if (NeedsAnyTypeNote)
+ DiagText = "%0 adjacent parameters of %1 of similar type are easily "
+ "swapped by mistake";
+ else
+ DiagText = "%0 adjacent parameters of %1 of similar type ('%2') are "
+ "easily swapped by mistake";
auto Diag = diag(First->getOuterLocStart(), DiagText)
<< static_cast<unsigned>(R.NumParamsChecked) << FD;
-
- Diag << FirstParamTypeAsWritten;
+ if (!NeedsAnyTypeNote)
+ Diag << FirstParamTypeAsWritten;
}
// Unfortunately, undersquiggly highlights are for some reason chewed up
@@ -464,6 +660,57 @@
diag(Last->getLocation(), "the last parameter in the range is '%0'",
DiagnosticIDs::Note)
<< getNameOrUnnamed(Last);
+
+ // Helper classes to silence elaborative diagnostic notes that would be
+ // too verbose.
+ UniqueTypeAliasDiagnosticHelper UniqueTypeAlias;
+ InsertOnce<SwappedEqualQualTypePair, 8> UniqueBindPower;
+
+ for (const model::Mix &M : R.Mixes) {
+ assert(M.flags() >= model::MIX_Trivial &&
+ "Sentinel or false mix in result.");
+
+ if (needsToPrintTypeInDiagnostic(M)) {
+ // Typedefs might result in the type of the variable needing to be
+ // emitted to a note diagnostic, so prepare it.
+ QualType LType = M.First->getType();
+ QualType RType = M.Second->getType();
+ QualType CommonType = M.commonUnderlyingType();
+ std::string LTypeAsWritten = LType.getAsString(PP);
+ std::string RTypeAsWritten = RType.getAsString(PP);
+ std::string CommonTypeStr = CommonType.getAsString(PP);
+
+ if (M.flags() & model::MIX_TypeAlias &&
+ UniqueTypeAlias(LType, RType, CommonType)) {
+ StringRef DiagText;
+ bool ExplicitlyPrintCommonType = false;
+ if (LTypeAsWritten == CommonTypeStr ||
+ RTypeAsWritten == CommonTypeStr)
+ DiagText =
+ "after resolving type aliases, '%0' and '%1' are the same";
+ else {
+ DiagText = "after resolving type aliases, the common type of '%0' "
+ "and '%1' is '%2'";
+ ExplicitlyPrintCommonType = true;
+ }
+
+ auto Diag =
+ diag(M.First->getOuterLocStart(), DiagText, DiagnosticIDs::Note)
+ << LTypeAsWritten << RTypeAsWritten;
+ if (ExplicitlyPrintCommonType)
+ Diag << CommonTypeStr;
+ }
+
+ if (M.flags() & model::MIX_ReferenceBind &&
+ UniqueBindPower({LType, RType})) {
+ // FIXME: Don't emit multiple combinations here either.
+ StringRef DiagText = "a call site binds an expression to '%0' and "
+ "'%1' with the same force";
+ diag(M.Second->getOuterLocStart(), DiagText, DiagnosticIDs::Note)
+ << LTypeAsWritten << RTypeAsWritten;
+ }
+ }
+ }
}
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
