https://github.com/legrosbuffle updated
https://github.com/llvm/llvm-project/pull/82617
>From 9b93c8bf0614e64352de8e210adf6ff316c0133e Mon Sep 17 00:00:00 2001
From: Clement Courbet <cour...@google.com>
Date: Mon, 19 Feb 2024 14:58:27 +0000
Subject: [PATCH 1/5] [llvm-exegesis][NFC] Refactor all `ValidationEvent` info
in a single table.
All data is derived from a single table rather than being spread out
over an enum, a table and the main entry point.
---
llvm/include/llvm/Target/TargetPfmCounters.td | 1 +
.../llvm-exegesis/lib/BenchmarkResult.cpp | 49 +--------------
.../tools/llvm-exegesis/lib/BenchmarkResult.h | 15 +----
llvm/tools/llvm-exegesis/lib/CMakeLists.txt | 1 +
.../lib/LatencyBenchmarkRunner.cpp | 4 +-
llvm/tools/llvm-exegesis/lib/Target.h | 1 +
.../llvm-exegesis/lib/ValidationEvent.cpp | 53 ++++++++++++++++
.../tools/llvm-exegesis/lib/ValidationEvent.h | 60 +++++++++++++++++++
llvm/tools/llvm-exegesis/llvm-exegesis.cpp | 20 +------
9 files changed, 124 insertions(+), 80 deletions(-)
create mode 100644 llvm/tools/llvm-exegesis/lib/ValidationEvent.cpp
create mode 100644 llvm/tools/llvm-exegesis/lib/ValidationEvent.h
diff --git a/llvm/include/llvm/Target/TargetPfmCounters.td
b/llvm/include/llvm/Target/TargetPfmCounters.td
index 8c4d5f50c63a24..cfe432a992b71f 100644
--- a/llvm/include/llvm/Target/TargetPfmCounters.td
+++ b/llvm/include/llvm/Target/TargetPfmCounters.td
@@ -35,6 +35,7 @@ class ValidationEvent <int event_number> {
int EventNumber = event_number;
}
+// TableGen names for events defined in `llvm::exegesis::ValidationEvent`.
def InstructionRetired : ValidationEvent<0>;
def L1DCacheLoadMiss : ValidationEvent<1>;
def L1DCacheStoreMiss : ValidationEvent<2>;
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp
b/llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp
index 189add6464173f..f84ebd2a4e68ef 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkResult.cpp
@@ -9,6 +9,7 @@
#include "BenchmarkResult.h"
#include "BenchmarkRunner.h"
#include "Error.h"
+#include "ValidationEvent.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/StringRef.h"
@@ -198,7 +199,7 @@ struct
CustomMappingTraits<std::map<exegesis::ValidationEvent, int64_t>> {
static void inputOne(IO &Io, StringRef KeyStr,
std::map<exegesis::ValidationEvent, int64_t> &VI) {
Expected<exegesis::ValidationEvent> Key =
- exegesis::stringToValidationEvent(KeyStr);
+ exegesis::getValidationEventByName(KeyStr);
if (!Key) {
Io.setError("Key is not a valid validation event");
return;
@@ -208,7 +209,7 @@ struct
CustomMappingTraits<std::map<exegesis::ValidationEvent, int64_t>> {
static void output(IO &Io, std::map<exegesis::ValidationEvent, int64_t> &VI)
{
for (auto &IndividualVI : VI) {
- Io.mapRequired(exegesis::validationEventToString(IndividualVI.first),
+ Io.mapRequired(exegesis::getValidationEventName(IndividualVI.first),
IndividualVI.second);
}
}
@@ -441,49 +442,5 @@ bool operator==(const BenchmarkMeasure &A, const
BenchmarkMeasure &B) {
std::tie(B.Key, B.PerInstructionValue, B.PerSnippetValue);
}
-const char *validationEventToString(ValidationEvent VE) {
- switch (VE) {
- case exegesis::ValidationEvent::InstructionRetired:
- return "instructions-retired";
- case exegesis::ValidationEvent::L1DCacheLoadMiss:
- return "l1d-cache-load-misses";
- case exegesis::ValidationEvent::L1DCacheStoreMiss:
- return "l1d-cache-store-misses";
- case exegesis::ValidationEvent::L1ICacheLoadMiss:
- return "l1i-cache-load-misses";
- case exegesis::ValidationEvent::DataTLBLoadMiss:
- return "data-tlb-load-misses";
- case exegesis::ValidationEvent::DataTLBStoreMiss:
- return "data-tlb-store-misses";
- case exegesis::ValidationEvent::InstructionTLBLoadMiss:
- return "instruction-tlb-load-misses";
- case exegesis::ValidationEvent::BranchPredictionMiss:
- return "branch-prediction-misses";
- }
- llvm_unreachable("Unhandled exegesis::ValidationEvent enum");
-}
-
-Expected<ValidationEvent> stringToValidationEvent(StringRef Input) {
- if (Input == "instructions-retired")
- return exegesis::ValidationEvent::InstructionRetired;
- else if (Input == "l1d-cache-load-misses")
- return exegesis::ValidationEvent::L1DCacheLoadMiss;
- else if (Input == "l1d-cache-store-misses")
- return exegesis::ValidationEvent::L1DCacheStoreMiss;
- else if (Input == "l1i-cache-load-misses")
- return exegesis::ValidationEvent::L1ICacheLoadMiss;
- else if (Input == "data-tlb-load-misses")
- return exegesis::ValidationEvent::DataTLBLoadMiss;
- else if (Input == "data-tlb-store-misses")
- return exegesis::ValidationEvent::DataTLBStoreMiss;
- else if (Input == "instruction-tlb-load-misses")
- return exegesis::ValidationEvent::InstructionTLBLoadMiss;
- else if (Input == "branch-prediction-misses")
- return exegesis::ValidationEvent::BranchPredictionMiss;
- else
- return make_error<StringError>("Invalid validation event string",
- errc::invalid_argument);
-}
-
} // namespace exegesis
} // namespace llvm
diff --git a/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h
b/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h
index 60115c51bba321..0aecaaeea4b2e7 100644
--- a/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h
+++ b/llvm/tools/llvm-exegesis/lib/BenchmarkResult.h
@@ -17,6 +17,7 @@
#include "LlvmState.h"
#include "RegisterValue.h"
+#include "ValidationEvent.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/MC/MCInst.h"
#include "llvm/MC/MCInstBuilder.h"
@@ -32,20 +33,6 @@ class Error;
namespace exegesis {
-enum ValidationEvent {
- InstructionRetired,
- L1DCacheLoadMiss,
- L1DCacheStoreMiss,
- L1ICacheLoadMiss,
- DataTLBLoadMiss,
- DataTLBStoreMiss,
- InstructionTLBLoadMiss,
- BranchPredictionMiss
-};
-
-const char *validationEventToString(exegesis::ValidationEvent VE);
-Expected<ValidationEvent> stringToValidationEvent(StringRef Input);
-
enum class BenchmarkPhaseSelectorE {
PrepareSnippet,
PrepareAndAssembleSnippet,
diff --git a/llvm/tools/llvm-exegesis/lib/CMakeLists.txt
b/llvm/tools/llvm-exegesis/lib/CMakeLists.txt
index 6ae441d31f07fe..414b49e5e021c2 100644
--- a/llvm/tools/llvm-exegesis/lib/CMakeLists.txt
+++ b/llvm/tools/llvm-exegesis/lib/CMakeLists.txt
@@ -73,6 +73,7 @@ add_llvm_library(LLVMExegesis
SubprocessMemory.cpp
Target.cpp
UopsBenchmarkRunner.cpp
+ ValidationEvent.cpp
LINK_LIBS ${libs}
diff --git a/llvm/tools/llvm-exegesis/lib/LatencyBenchmarkRunner.cpp
b/llvm/tools/llvm-exegesis/lib/LatencyBenchmarkRunner.cpp
index a9917a29cce24d..de61fff6432944 100644
--- a/llvm/tools/llvm-exegesis/lib/LatencyBenchmarkRunner.cpp
+++ b/llvm/tools/llvm-exegesis/lib/LatencyBenchmarkRunner.cpp
@@ -107,8 +107,8 @@ Expected<std::vector<BenchmarkMeasure>>
LatencyBenchmarkRunner::runMeasurements(
}
for (size_t I = 0; I < ValCounterValues.size(); ++I) {
- LLVM_DEBUG(dbgs() << validationEventToString(ValidationCounters[I])
- << ": " << IterationValCounterValues[I] << "\n");
+ LLVM_DEBUG(dbgs() << getValidationEventName(ValidationCounters[I]) << ":
"
+ << IterationValCounterValues[I] << "\n");
ValCounterValues[I] += IterationValCounterValues[I];
}
}
diff --git a/llvm/tools/llvm-exegesis/lib/Target.h
b/llvm/tools/llvm-exegesis/lib/Target.h
index 3d6169c9650213..7bbd946b03331f 100644
--- a/llvm/tools/llvm-exegesis/lib/Target.h
+++ b/llvm/tools/llvm-exegesis/lib/Target.h
@@ -22,6 +22,7 @@
#include "LlvmState.h"
#include "PerfHelper.h"
#include "SnippetGenerator.h"
+#include "ValidationEvent.h"
#include "llvm/CodeGen/TargetPassConfig.h"
#include "llvm/IR/CallingConv.h"
#include "llvm/IR/LegacyPassManager.h"
diff --git a/llvm/tools/llvm-exegesis/lib/ValidationEvent.cpp
b/llvm/tools/llvm-exegesis/lib/ValidationEvent.cpp
new file mode 100644
index 00000000000000..a212e1c4fdf7c8
--- /dev/null
+++ b/llvm/tools/llvm-exegesis/lib/ValidationEvent.cpp
@@ -0,0 +1,53 @@
+
+#include "ValidationEvent.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Errc.h"
+#include "llvm/Support/Error.h"
+
+namespace llvm {
+namespace exegesis {
+
+namespace {
+
+struct ValidationEventInfo {
+ const char *const Name;
+ const char *const Description;
+};
+
+// Information about validation events, indexed by `ValidationEvent` enum
+// value.
+static constexpr ValidationEventInfo ValidationEventInfos[NumValidationEvents]
=
+ {
+ {"instructions-retired", "Count retired instructions"},
+ {"l1d-cache-load-misses", "Count L1D load cache misses"},
+ {"l1d-cache-store-misses", "Count L1D store cache misses"},
+ {"l1i-cache-load-misses", "Count L1I load cache misses"},
+ {"data-tlb-load-misses", "Count DTLB load misses"},
+ {"data-tlb-store-misses", "Count DTLB store misses"},
+ {"instruction-tlb-load-misses", "Count ITLB load misses"},
+ {"branch-prediction-misses", "Branch prediction misses"},
+};
+
+} // namespace
+
+const char *getValidationEventName(ValidationEvent VE) {
+ return ValidationEventInfos[VE].Name;
+}
+const char *getValidationEventDescription(ValidationEvent VE) {
+ return ValidationEventInfos[VE].Description;
+}
+
+Expected<ValidationEvent> getValidationEventByName(StringRef Name) {
+ int VE = 0;
+ for (const ValidationEventInfo &Info : ValidationEventInfos) {
+ if (Name == Info.Name)
+ return static_cast<ValidationEvent>(VE);
+ ++VE;
+ }
+
+ return make_error<StringError>("Invalid validation event string",
+ errc::invalid_argument);
+}
+
+} // namespace exegesis
+} // namespace llvm
diff --git a/llvm/tools/llvm-exegesis/lib/ValidationEvent.h
b/llvm/tools/llvm-exegesis/lib/ValidationEvent.h
new file mode 100644
index 00000000000000..8a9f3af57dca97
--- /dev/null
+++ b/llvm/tools/llvm-exegesis/lib/ValidationEvent.h
@@ -0,0 +1,60 @@
+//===-- ValidationEvent.h ---------------------------------------*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// Definitions and utilities for Validation Events.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TOOLS_LLVM_EXEGESIS_VALIDATIONEVENT_H
+#define LLVM_TOOLS_LLVM_EXEGESIS_VALIDATIONEVENT_H
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+
+namespace llvm {
+
+namespace exegesis {
+
+// The main list of supported validation events. The mapping between validation
+// events and pfm counters is defined in TableDef files for each target.
+enum ValidationEvent {
+ InstructionRetired,
+ L1DCacheLoadMiss,
+ L1DCacheStoreMiss,
+ L1ICacheLoadMiss,
+ DataTLBLoadMiss,
+ DataTLBStoreMiss,
+ InstructionTLBLoadMiss,
+ BranchPredictionMiss,
+ // Number of events.
+ NumValidationEvents,
+};
+
+// Returns the name/description of the given event.
+const char *getValidationEventName(ValidationEvent VE);
+const char *getValidationEventDescription(ValidationEvent VE);
+
+// Returns the ValidationEvent with the given name.
+Expected<ValidationEvent> getValidationEventByName(StringRef Name);
+
+// Command-line options for validation events.
+struct ValidationEventOptions {
+ template <class Opt> void apply(Opt &O) const {
+ for (int I = 0; I < NumValidationEvents; ++I) {
+ const auto VE = static_cast<ValidationEvent>(I);
+ O.getParser().addLiteralOption(getValidationEventName(VE), VE,
+ getValidationEventDescription(VE));
+ }
+ }
+};
+
+} // namespace exegesis
+} // namespace llvm
+
+#endif
diff --git a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
index ac279029e6b004..66387bdec5a5a6 100644
--- a/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
+++ b/llvm/tools/llvm-exegesis/llvm-exegesis.cpp
@@ -25,6 +25,7 @@
#include "lib/SnippetRepetitor.h"
#include "lib/Target.h"
#include "lib/TargetSelect.h"
+#include "lib/ValidationEvent.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/Twine.h"
#include "llvm/MC/MCInstBuilder.h"
@@ -278,24 +279,7 @@ static cl::list<ValidationEvent> ValidationCounters(
cl::desc(
"The name of a validation counter to run concurrently with the main "
"counter to validate benchmarking assumptions"),
- cl::CommaSeparated, cl::cat(BenchmarkOptions),
- cl::values(
- clEnumValN(ValidationEvent::InstructionRetired, "instructions-retired",
- "Count retired instructions"),
- clEnumValN(ValidationEvent::L1DCacheLoadMiss, "l1d-cache-load-misses",
- "Count L1D load cache misses"),
- clEnumValN(ValidationEvent::L1DCacheStoreMiss,
"l1d-cache-store-misses",
- "Count L1D store cache misses"),
- clEnumValN(ValidationEvent::L1ICacheLoadMiss, "l1i-cache-load-misses",
- "Count L1I load cache misses"),
- clEnumValN(ValidationEvent::DataTLBLoadMiss, "data-tlb-load-misses",
- "Count DTLB load misses"),
- clEnumValN(ValidationEvent::DataTLBStoreMiss, "data-tlb-store-misses",
- "Count DTLB store misses"),
- clEnumValN(ValidationEvent::InstructionTLBLoadMiss,
- "instruction-tlb-load-misses", "Count ITLB load misses"),
- clEnumValN(ValidationEvent::BranchPredictionMiss,
- "branch-prediction-misses", "Branch prediction misses")));
+ cl::CommaSeparated, cl::cat(BenchmarkOptions), ValidationEventOptions());
static ExitOnError ExitOnErr("llvm-exegesis error: ");
>From a4e177fd8fe0289742be8a4a90ae010b5fbd5630 Mon Sep 17 00:00:00 2001
From: Clement Courbet <cour...@google.com>
Date: Tue, 20 Feb 2024 08:43:23 +0000
Subject: [PATCH 2/5] `static_assert` that the size of `ValidationEventInfos`
is consistent with that of the enum.
---
.../llvm-exegesis/lib/ValidationEvent.cpp | 23 +++++++++++--------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/llvm/tools/llvm-exegesis/lib/ValidationEvent.cpp
b/llvm/tools/llvm-exegesis/lib/ValidationEvent.cpp
index a212e1c4fdf7c8..c965b7ae55e10d 100644
--- a/llvm/tools/llvm-exegesis/lib/ValidationEvent.cpp
+++ b/llvm/tools/llvm-exegesis/lib/ValidationEvent.cpp
@@ -16,18 +16,21 @@ struct ValidationEventInfo {
// Information about validation events, indexed by `ValidationEvent` enum
// value.
-static constexpr ValidationEventInfo ValidationEventInfos[NumValidationEvents]
=
- {
- {"instructions-retired", "Count retired instructions"},
- {"l1d-cache-load-misses", "Count L1D load cache misses"},
- {"l1d-cache-store-misses", "Count L1D store cache misses"},
- {"l1i-cache-load-misses", "Count L1I load cache misses"},
- {"data-tlb-load-misses", "Count DTLB load misses"},
- {"data-tlb-store-misses", "Count DTLB store misses"},
- {"instruction-tlb-load-misses", "Count ITLB load misses"},
- {"branch-prediction-misses", "Branch prediction misses"},
+static constexpr ValidationEventInfo ValidationEventInfos[] = {
+ {"instructions-retired", "Count retired instructions"},
+ {"l1d-cache-load-misses", "Count L1D load cache misses"},
+ {"l1d-cache-store-misses", "Count L1D store cache misses"},
+ {"l1i-cache-load-misses", "Count L1I load cache misses"},
+ {"data-tlb-load-misses", "Count DTLB load misses"},
+ {"data-tlb-store-misses", "Count DTLB store misses"},
+ {"instruction-tlb-load-misses", "Count ITLB load misses"},
+ {"branch-prediction-misses", "Branch prediction misses"},
};
+static_assert(sizeof(ValidationEventInfos) ==
+ NumValidationEvents * sizeof(ValidationEventInfo),
+ "please update ValidationEventInfos");
+
} // namespace
const char *getValidationEventName(ValidationEvent VE) {
>From 881946112a2273e76c0daa3ee45bd4dce41e6f68 Mon Sep 17 00:00:00 2001
From: Clement Courbet <cour...@google.com>
Date: Wed, 21 Feb 2024 09:15:22 +0000
Subject: [PATCH 3/5] [clang-tidy] Add support for determining constness of
more expressions.
This uses a more systematic approach for determining whcich `DeclRefExpr`s
mutate
the underlying object: Instead of using a few matchers, we walk up the
AST until we find a parent that we can prove cannot change the
underlying object.
This allows us to handle most address taking and dereference, bindings
to value and const& variables, and track constness of pointee
(see changes in DeclRefExprUtilsTest.cpp).
This allows supporting more patterns in
`performance-unnecessary-copy-initialization`.
Those two patterns are relatively common:
```
const auto e = (*vector_ptr)[i]
```
and
```
const auto e = vector_ptr->at(i);
```
In our codebase, we have around 25% additional findings from
`performance-unnecessary-copy-initialization` with this change.
I did not see any additional false positives.
---
.../UnnecessaryCopyInitialization.cpp | 27 +-
.../clang-tidy/utils/DeclRefExprUtils.cpp | 212 +++++++++++---
.../clang-tidy/utils/DeclRefExprUtils.h | 33 ++-
clang-tools-extra/docs/ReleaseNotes.rst | 6 +
.../unnecessary-copy-initialization.cpp | 27 +-
.../clang-tidy/DeclRefExprUtilsTest.cpp | 274 +++++++++++-------
6 files changed, 396 insertions(+), 183 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
index dfe12c5b6007da..9beb185cba929d 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -86,16 +86,22 @@ AST_MATCHER_FUNCTION_P(StatementMatcher,
isConstRefReturningMethodCall,
const auto MethodDecl =
cxxMethodDecl(returns(hasCanonicalType(matchers::isReferenceToConst())))
.bind(MethodDeclId);
- const auto ReceiverExpr = declRefExpr(to(varDecl().bind(ObjectArgId)));
+ const auto ReceiverExpr =
+ ignoringParenImpCasts(declRefExpr(to(varDecl().bind(ObjectArgId))));
+ const auto OnExpr = anyOf(
+ // Direct reference to `*this`: `a.f()` or `a->f()`.
+ ReceiverExpr,
+ // Access through dereference, typically used for `operator[]`:
`(*a)[3]`.
+ unaryOperator(hasOperatorName("*"), hasUnaryOperand(ReceiverExpr)));
const auto ReceiverType =
hasCanonicalType(recordType(hasDeclaration(namedDecl(
unless(matchers::matchesAnyListedName(ExcludedContainerTypes))))));
- return expr(anyOf(
- cxxMemberCallExpr(callee(MethodDecl), on(ReceiverExpr),
- thisPointerType(ReceiverType)),
- cxxOperatorCallExpr(callee(MethodDecl), hasArgument(0, ReceiverExpr),
- hasArgument(0, hasType(ReceiverType)))));
+ return expr(
+ anyOf(cxxMemberCallExpr(callee(MethodDecl), on(OnExpr),
+ thisPointerType(ReceiverType)),
+ cxxOperatorCallExpr(callee(MethodDecl), hasArgument(0, OnExpr),
+ hasArgument(0, hasType(ReceiverType)))));
}
AST_MATCHER_FUNCTION(StatementMatcher, isConstRefReturningFunctionCall) {
@@ -136,10 +142,11 @@ AST_MATCHER_FUNCTION_P(StatementMatcher,
initializerReturnsReferenceToConst,
static bool isInitializingVariableImmutable(
const VarDecl &InitializingVar, const Stmt &BlockStmt, ASTContext &Context,
const std::vector<StringRef> &ExcludedContainerTypes) {
- if (!isOnlyUsedAsConst(InitializingVar, BlockStmt, Context))
+ QualType T = InitializingVar.getType().getCanonicalType();
+ if (!isOnlyUsedAsConst(InitializingVar, BlockStmt, Context,
+ T->isPointerType() ? 1 : 0))
return false;
- QualType T = InitializingVar.getType().getCanonicalType();
// The variable is a value type and we know it is only used as const. Safe
// to reference it and avoid the copy.
if (!isa<ReferenceType, PointerType>(T))
@@ -273,7 +280,9 @@ void UnnecessaryCopyInitialization::check(
VarDeclStmt.isSingleDecl() && !NewVar.getLocation().isMacroID();
const bool IsVarUnused = isVariableUnused(NewVar, BlockStmt,
*Result.Context);
const bool IsVarOnlyUsedAsConst =
- isOnlyUsedAsConst(NewVar, BlockStmt, *Result.Context);
+ isOnlyUsedAsConst(NewVar, BlockStmt, *Result.Context,
+ // `NewVar` is always of non-pointer type.
+ 0);
const CheckContext Context{
NewVar, BlockStmt, VarDeclStmt, *Result.Context,
IssueFix, IsVarUnused, IsVarOnlyUsedAsConst};
diff --git a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
index 2d73179150e8b8..663691c519b8e9 100644
--- a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
@@ -10,7 +10,9 @@
#include "Matchers.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/DeclCXX.h"
+#include "clang/AST/ExprCXX.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include <cassert>
namespace clang::tidy::utils::decl_ref_expr {
@@ -34,69 +36,185 @@ void extractNodesByIdTo(ArrayRef<BoundNodes> Matches,
StringRef ID,
Nodes.insert(Match.getNodeAs<Node>(ID));
}
+// A matcher that matches DeclRefExprs that are used in ways such that the
+// underlying declaration is not modified.
+// If the declaration is of pointer type, `Indirections` specifies the level
+// of indirection of the object whose mutations we are tracking.
+//
+// For example, given:
+// ```
+// int i;
+// int* p;
+// p = &i; // (A)
+// *p = 3; // (B)
+// ```
+//
+// `declRefExpr(to(varDecl(hasName("p"))), doesNotMutateObject(0))` matches
+// (B), but `declRefExpr(to(varDecl(hasName("p"))), doesNotMutateObject(1))`
+// matches (A).
+//
+AST_MATCHER_P(DeclRefExpr, doesNotMutateObject, int, Indirections) {
+ // We walk up the parents of the DeclRefExpr recursively until we end up on a
+ // parent that cannot modify the underlying object. There are a few kinds of
+ // expressions:
+ // - Those that cannot be used to mutate the underlying object. We can stop
+ // recursion there.
+ // - Those that can be used to mutate the underlying object in analyzable
+ // ways (such as taking the address or accessing a subobject). We have to
+ // examine the parents.
+ // - Those that we don't know how to analyze. In that case we stop there and
+ // we assume that they can mutate the underlying expression.
+
+ struct StackEntry {
+ StackEntry(const Expr *E, int Indirections)
+ : E(E), Indirections(Indirections) {}
+ // The expression to analyze.
+ const Expr *E;
+ // The number of pointer indirections of the object being tracked (how
+ // many times an address was taken).
+ int Indirections;
+ };
+
+ llvm::SmallVector<StackEntry, 4> Stack;
+ Stack.emplace_back(&Node, Indirections);
+ auto &Ctx = Finder->getASTContext();
+
+ while (!Stack.empty()) {
+ const StackEntry Entry = Stack.back();
+ Stack.pop_back();
+
+ // If the expression type is const-qualified at the appropriate indirection
+ // level then we can not mutate the object.
+ QualType Ty = Entry.E->getType().getCanonicalType();
+ for (int I = 0; I < Entry.Indirections; ++I) {
+ assert(Ty->isPointerType());
+ Ty = Ty->getPointeeType().getCanonicalType();
+ }
+ if (Ty.isConstQualified()) {
+ continue;
+ }
+
+ // Otherwise we have to look at the parents to see how the expression is
+ // used.
+ const auto Parents = Ctx.getParents(*Entry.E);
+ // Note: most nodes have a single parents, but there exist nodes that have
+ // several parents, such as `InitListExpr` that have semantic and syntactic
+ // forms.
+ for (const auto &Parent : Parents) {
+ if (Parent.get<CompoundStmt>()) {
+ // Unused block-scope statement.
+ continue;
+ }
+ const Expr *const P = Parent.get<Expr>();
+ if (P == nullptr) {
+ // `Parent` is not an expr (e.g. a `VarDecl`).
+ // The case of binding to a `const&` or `const*` variable is handled by
+ // the fact that there is going to be a `NoOp` cast to const below the
+ // `VarDecl`, so we're not even going to get there.
+ // The case of copying into a value-typed variable is handled by the
+ // rvalue cast.
+ // This triggers only when binding to a mutable reference/ptr variable.
+ // FIXME: When we take a mutable reference we could keep checking the
+ // new variable for const usage only.
+ return false;
+ }
+ // Cosmetic nodes.
+ if (isa<ParenExpr>(P) || isa<MaterializeTemporaryExpr>(P)) {
+ Stack.emplace_back(P, Entry.Indirections);
+ continue;
+ }
+ if (const auto *const Cast = dyn_cast<CastExpr>(P)) {
+ switch (Cast->getCastKind()) {
+ // NoOp casts are used to add `const`. We'll check whether adding that
+ // const prevents modification when we process the cast.
+ case CK_NoOp:
+ // These do nothing w.r.t. to mutability.
+ case CK_BaseToDerived:
+ case CK_DerivedToBase:
+ case CK_UncheckedDerivedToBase:
+ case CK_Dynamic:
+ case CK_BaseToDerivedMemberPointer:
+ case CK_DerivedToBaseMemberPointer:
+ Stack.emplace_back(Cast, Entry.Indirections);
+ continue;
+ case CK_ToVoid:
+ case CK_PointerToBoolean:
+ // These do not mutate the underlying variable.
+ continue;
+ case CK_LValueToRValue: {
+ // An rvalue is immutable.
+ if (Entry.Indirections == 0) {
+ continue;
+ }
+ Stack.emplace_back(Cast, Entry.Indirections);
+ continue;
+ }
+ default:
+ // Bail out on casts that we cannot analyze.
+ return false;
+ }
+ }
+ if (const auto *const Member = dyn_cast<MemberExpr>(P)) {
+ if (const auto *const Method =
+ dyn_cast<CXXMethodDecl>(Member->getMemberDecl())) {
+ if (!Method->isConst()) {
+ // The method can mutate our variable.
+ return false;
+ }
+ continue;
+ }
+ Stack.emplace_back(Member, 0);
+ continue;
+ }
+ if (const auto *const Op = dyn_cast<UnaryOperator>(P)) {
+ switch (Op->getOpcode()) {
+ case UO_AddrOf:
+ Stack.emplace_back(Op, Entry.Indirections + 1);
+ continue;
+ case UO_Deref:
+ assert(Entry.Indirections > 0);
+ Stack.emplace_back(Op, Entry.Indirections - 1);
+ continue;
+ default:
+ // Bail out on unary operators that we cannot analyze.
+ return false;
+ }
+ }
+
+ // Assume any other expression can modify the underlying variable.
+ return false;
+ }
+ }
+
+ // No parent can modify the variable.
+ return true;
+}
+
} // namespace
-// Finds all DeclRefExprs where a const method is called on VarDecl or VarDecl
-// is the a const reference or value argument to a CallExpr or
CXXConstructExpr.
SmallPtrSet<const DeclRefExpr *, 16>
constReferenceDeclRefExprs(const VarDecl &VarDecl, const Stmt &Stmt,
- ASTContext &Context) {
- auto DeclRefToVar =
- declRefExpr(to(varDecl(equalsNode(&VarDecl)))).bind("declRef");
- auto MemberExprOfVar = memberExpr(hasObjectExpression(DeclRefToVar));
- auto DeclRefToVarOrMemberExprOfVar =
- stmt(anyOf(DeclRefToVar, MemberExprOfVar));
- auto ConstMethodCallee = callee(cxxMethodDecl(isConst()));
- // Match method call expressions where the variable is referenced as the this
- // implicit object argument and operator call expression for member operators
- // where the variable is the 0-th argument.
- auto Matches = match(
- findAll(expr(anyOf(
- cxxMemberCallExpr(ConstMethodCallee,
- on(DeclRefToVarOrMemberExprOfVar)),
- cxxOperatorCallExpr(ConstMethodCallee,
- hasArgument(0,
DeclRefToVarOrMemberExprOfVar))))),
- Stmt, Context);
+ ASTContext &Context, int Indirections) {
+ auto Matches = match(findAll(declRefExpr(to(varDecl(equalsNode(&VarDecl))),
+ doesNotMutateObject(Indirections))
+ .bind("declRef")),
+ Stmt, Context);
SmallPtrSet<const DeclRefExpr *, 16> DeclRefs;
extractNodesByIdTo(Matches, "declRef", DeclRefs);
- auto ConstReferenceOrValue =
- qualType(anyOf(matchers::isReferenceToConst(),
- unless(anyOf(referenceType(), pointerType(),
- substTemplateTypeParmType()))));
- auto ConstReferenceOrValueOrReplaced = qualType(anyOf(
- ConstReferenceOrValue,
- substTemplateTypeParmType(hasReplacementType(ConstReferenceOrValue))));
- auto UsedAsConstRefOrValueArg = forEachArgumentWithParam(
- DeclRefToVarOrMemberExprOfVar,
- parmVarDecl(hasType(ConstReferenceOrValueOrReplaced)));
- Matches = match(findAll(invocation(UsedAsConstRefOrValueArg)), Stmt,
Context);
- extractNodesByIdTo(Matches, "declRef", DeclRefs);
- // References and pointers to const assignments.
- Matches = match(
- findAll(declStmt(has(varDecl(
- hasType(qualType(matchers::isReferenceToConst())),
- hasInitializer(ignoringImpCasts(DeclRefToVarOrMemberExprOfVar)))))),
- Stmt, Context);
- extractNodesByIdTo(Matches, "declRef", DeclRefs);
- Matches = match(findAll(declStmt(has(varDecl(
- hasType(qualType(matchers::isPointerToConst())),
- hasInitializer(ignoringImpCasts(unaryOperator(
- hasOperatorName("&"),
-
hasUnaryOperand(DeclRefToVarOrMemberExprOfVar)))))))),
- Stmt, Context);
- extractNodesByIdTo(Matches, "declRef", DeclRefs);
+
return DeclRefs;
}
bool isOnlyUsedAsConst(const VarDecl &Var, const Stmt &Stmt,
- ASTContext &Context) {
+ ASTContext &Context, int Indirections) {
// Collect all DeclRefExprs to the loop variable and all CallExprs and
// CXXConstructExprs where the loop variable is used as argument to a const
// reference parameter.
// If the difference is empty it is safe for the loop variable to be a const
// reference.
auto AllDeclRefs = allDeclRefExprs(Var, Stmt, Context);
- auto ConstReferenceDeclRefs = constReferenceDeclRefExprs(Var, Stmt, Context);
+ auto ConstReferenceDeclRefs =
+ constReferenceDeclRefExprs(Var, Stmt, Context, Indirections);
return isSetDifferenceEmpty(AllDeclRefs, ConstReferenceDeclRefs);
}
diff --git a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.h
b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.h
index 2c16d95408cf68..8361b9d89ed268 100644
--- a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.h
+++ b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.h
@@ -15,15 +15,6 @@
namespace clang::tidy::utils::decl_ref_expr {
-/// Returns true if all ``DeclRefExpr`` to the variable within ``Stmt``
-/// do not modify it.
-///
-/// Returns ``true`` if only const methods or operators are called on the
-/// variable or the variable is a const reference or value argument to a
-/// ``callExpr()``.
-bool isOnlyUsedAsConst(const VarDecl &Var, const Stmt &Stmt,
- ASTContext &Context);
-
/// Returns set of all ``DeclRefExprs`` to ``VarDecl`` within ``Stmt``.
llvm::SmallPtrSet<const DeclRefExpr *, 16>
allDeclRefExprs(const VarDecl &VarDecl, const Stmt &Stmt, ASTContext &Context);
@@ -34,9 +25,31 @@ allDeclRefExprs(const VarDecl &VarDecl, const Decl &Decl,
ASTContext &Context);
/// Returns set of all ``DeclRefExprs`` to ``VarDecl`` within ``Stmt`` where
/// ``VarDecl`` is guaranteed to be accessed in a const fashion.
+///
+/// If ``VarDecl`` is of pointer type, ``Indirections`` specifies the level
+/// of indirection of the object whose mutations we are tracking.
+///
+/// For example, given:
+/// ```
+/// int i;
+/// int* p;
+/// p = &i; // (A)
+/// *p = 3; // (B)
+/// ```
+///
+/// - `constReferenceDeclRefExprs(P, Stmt, Context, 0)` returns the reference
+// to `p` in (B): the pointee is modified, but the pointer is not;
+/// - `constReferenceDeclRefExprs(P, Stmt, Context, 1)` returns the reference
+// to `p` in (A): the pointee is modified, but the pointer is not;
llvm::SmallPtrSet<const DeclRefExpr *, 16>
constReferenceDeclRefExprs(const VarDecl &VarDecl, const Stmt &Stmt,
- ASTContext &Context);
+ ASTContext &Context, int Indirections);
+
+/// Returns true if all ``DeclRefExpr`` to the variable within ``Stmt``
+/// do not modify it.
+/// See `constReferenceDeclRefExprs` for the meaning of ``Indirections``.
+bool isOnlyUsedAsConst(const VarDecl &Var, const Stmt &Stmt,
+ ASTContext &Context, int Indirections);
/// Returns ``true`` if ``DeclRefExpr`` is the argument of a copy-constructor
/// call expression within ``Decl``.
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index a0b9fcfe0d7774..79eb777afee575 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -179,6 +179,12 @@ Changes in existing checks
<clang-tidy/checks/modernize/use-override>` check to also remove any trailing
whitespace when deleting the ``virtual`` keyword.
+- Improved :doc:`performance-unnecessary-copy-initialization
+ <clang-tidy/checks/performance/unnecessary-copy-initialization>` check by
+ detecting more cases of constant access. In particular, pointers can be
+ analyzed, se the check now handles the common patterns
+ `const auto e = (*vector_ptr)[i]` and `const auto e = vector_ptr->at(i);`.
+
- Improved :doc:`readability-implicit-bool-conversion
<clang-tidy/checks/readability/implicit-bool-conversion>` check to provide
valid fix suggestions for ``static_cast`` without a preceding space and
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization.cpp
b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization.cpp
index 049ba64d6aedeb..92625cc1332e28 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization.cpp
@@ -41,6 +41,10 @@ struct Container {
ConstIterator<T> end() const;
void nonConstMethod();
bool constMethod() const;
+
+ const T& at(int) const;
+ T& at(int);
+
};
using ExpensiveToCopyContainerAlias = Container<ExpensiveToCopyType>;
@@ -225,25 +229,25 @@ void PositiveOperatorCallConstValueParamAlias(const
ExpensiveToCopyContainerAlia
VarCopyConstructed.constMethod();
}
-void PositiveOperatorCallConstValueParam(const Container<ExpensiveToCopyType>*
C) {
+void PositiveOperatorCallConstPtrParam(const Container<ExpensiveToCopyType>*
C) {
const auto AutoAssigned = (*C)[42];
- // TODO-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable
'AutoAssigned'
- // TODO-FIXES: const auto& AutoAssigned = (*C)[42];
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable
'AutoAssigned'
+ // CHECK-FIXES: const auto& AutoAssigned = (*C)[42];
AutoAssigned.constMethod();
const auto AutoCopyConstructed((*C)[42]);
- // TODO-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable
'AutoCopyConstructed'
- // TODO-FIXES: const auto& AutoCopyConstructed((*C)[42]);
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable
'AutoCopyConstructed'
+ // CHECK-FIXES: const auto& AutoCopyConstructed((*C)[42]);
AutoCopyConstructed.constMethod();
- const ExpensiveToCopyType VarAssigned = C->operator[](42);
- // TODO-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable
'VarAssigned'
- // TODO-FIXES: const ExpensiveToCopyType& VarAssigned = C->operator[](42);
+ const ExpensiveToCopyType VarAssigned = C->at(42);
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable
'VarAssigned'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = C->at(42);
VarAssigned.constMethod();
- const ExpensiveToCopyType VarCopyConstructed(C->operator[](42));
- // TODO-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable
'VarCopyConstructed'
- // TODO-FIXES: const ExpensiveToCopyType&
VarCopyConstructed(C->operator[](42));
+ const ExpensiveToCopyType VarCopyConstructed(C->at(42));
+ // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable
'VarCopyConstructed'
+ // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(C->at(42));
VarCopyConstructed.constMethod();
}
@@ -876,3 +880,4 @@ void negativeNonConstMemberExpr() {
mutate(&Copy.Member);
}
}
+
diff --git a/clang-tools-extra/unittests/clang-tidy/DeclRefExprUtilsTest.cpp
b/clang-tools-extra/unittests/clang-tidy/DeclRefExprUtilsTest.cpp
index a653b11faad282..4c9e81ea0f61ac 100644
--- a/clang-tools-extra/unittests/clang-tidy/DeclRefExprUtilsTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/DeclRefExprUtilsTest.cpp
@@ -12,6 +12,7 @@ namespace tidy {
namespace {
using namespace clang::ast_matchers;
+template <int Indirections>
class ConstReferenceDeclRefExprsTransform : public ClangTidyCheck {
public:
ConstReferenceDeclRefExprsTransform(StringRef CheckName,
@@ -27,7 +28,7 @@ class ConstReferenceDeclRefExprsTransform : public
ClangTidyCheck {
using utils::decl_ref_expr::constReferenceDeclRefExprs;
const auto const_decrefexprs = constReferenceDeclRefExprs(
*D, *cast<FunctionDecl>(D->getDeclContext())->getBody(),
- *Result.Context);
+ *Result.Context, Indirections);
for (const DeclRefExpr *const Expr : const_decrefexprs) {
assert(Expr);
@@ -40,7 +41,7 @@ class ConstReferenceDeclRefExprsTransform : public
ClangTidyCheck {
namespace test {
-void RunTest(StringRef Snippet) {
+template <int Indirections> void RunTest(StringRef Snippet) {
StringRef CommonCode = R"(
struct ConstTag{};
@@ -59,6 +60,9 @@ void RunTest(StringRef Snippet) {
bool operator==(const S&) const;
int int_member;
+ // We consider a mutation of the `*ptr_member` to be a const use of
+ // `*this`. This is consistent with the semantics of `const`-qualified
+ // methods, which prevent modifying `ptr_member` but not `*ptr_member`.
int* ptr_member;
};
@@ -92,69 +96,88 @@ void RunTest(StringRef Snippet) {
llvm::SmallVector<StringRef, 1> Parts;
StringRef(Code).split(Parts, "/*const*/");
- EXPECT_EQ(Code, runCheckOnCode<ConstReferenceDeclRefExprsTransform>(
- join(Parts, "")));
+ EXPECT_EQ(Code,
+ runCheckOnCode<ConstReferenceDeclRefExprsTransform<Indirections>>(
+ join(Parts, "")));
}
TEST(ConstReferenceDeclRefExprsTest, ConstValueVar) {
- RunTest(R"(
+ RunTest<0>(R"(
void f(const S target) {
useVal(/*const*/target);
useConstRef(/*const*/target);
- useConstPtr(&target);
- useConstPtrConstRef(&target);
+ useConstPtr(&/*const*/target);
+ useConstPtrConstRef(&/*const*/target);
/*const*/target.constMethod();
/*const*/target(ConstTag{});
/*const*/target[42];
useConstRef((/*const*/target));
(/*const*/target).constMethod();
(void)(/*const*/target == /*const*/target);
- (void)target;
- (void)⌖
- (void)*⌖
+ (void)/*const*/target;
+ (void)&/*const*/target;
+ (void)*&/*const*/target;
+ /*const*/target;
S copy1 = /*const*/target;
S copy2(/*const*/target);
+ /*const*/target.int_member;
useInt(/*const*/target.int_member);
useIntConstRef(/*const*/target.int_member);
- useIntPtr(target.ptr_member);
- useIntConstPtr(&target.int_member);
+ useIntPtr(/*const*/target.ptr_member);
+ useIntConstPtr(&/*const*/target.int_member);
+
+ const S& const_target_ref = /*const*/target;
+ const S* const_target_ptr = &/*const*/target;
}
)");
}
TEST(ConstReferenceDeclRefExprsTest, ConstRefVar) {
- RunTest(R"(
+ RunTest<0>(R"(
void f(const S& target) {
useVal(/*const*/target);
useConstRef(/*const*/target);
- useConstPtr(&target);
- useConstPtrConstRef(&target);
+ useConstPtr(&/*const*/target);
+ useConstPtrConstRef(&/*const*/target);
/*const*/target.constMethod();
/*const*/target(ConstTag{});
/*const*/target[42];
useConstRef((/*const*/target));
(/*const*/target).constMethod();
(void)(/*const*/target == /*const*/target);
- (void)target;
- (void)⌖
- (void)*⌖
+ (void)/*const*/target;
+ (void)&/*const*/target;
+ (void)*&/*const*/target;
+ /*const*/target;
S copy1 = /*const*/target;
S copy2(/*const*/target);
+ /*const*/target.int_member;
useInt(/*const*/target.int_member);
useIntConstRef(/*const*/target.int_member);
- useIntPtr(target.ptr_member);
- useIntConstPtr(&target.int_member);
+ useIntPtr(/*const*/target.ptr_member);
+ useIntConstPtr(&/*const*/target.int_member);
+
+ const S& const_target_ref = /*const*/target;
+ const S* const_target_ptr = &/*const*/target;
+ }
+)");
+}
+
+TEST(ConstReferenceDeclRefExprsTest, DEBUGREMOVEME) {
+ RunTest<0>(R"(
+ void f(S target, const S& other) {
+ S* target_ptr = ⌖
}
)");
}
TEST(ConstReferenceDeclRefExprsTest, ValueVar) {
- RunTest(R"(
+ RunTest<0>(R"(
void f(S target, const S& other) {
useConstRef(/*const*/target);
useVal(/*const*/target);
- useConstPtr(&target);
- useConstPtrConstRef(&target);
+ useConstPtr(&/*const*/target);
+ useConstPtrConstRef(&/*const*/target);
/*const*/target.constMethod();
target.nonConstMethod();
/*const*/target(ConstTag{});
@@ -167,26 +190,33 @@ TEST(ConstReferenceDeclRefExprsTest, ValueVar) {
(/*const*/target).constMethod();
(void)(/*const*/target == /*const*/target);
(void)(/*const*/target == other);
- (void)target;
- (void)⌖
- (void)*⌖
+ (void)/*const*/target;
+ (void)&/*const*/target;
+ (void)*&/*const*/target;
+ /*const*/target;
S copy1 = /*const*/target;
S copy2(/*const*/target);
+ /*const*/target.int_member;
useInt(/*const*/target.int_member);
useIntConstRef(/*const*/target.int_member);
- useIntPtr(target.ptr_member);
- useIntConstPtr(&target.int_member);
+ useIntPtr(/*const*/target.ptr_member);
+ useIntConstPtr(&/*const*/target.int_member);
+
+ const S& const_target_ref = /*const*/target;
+ const S* const_target_ptr = &/*const*/target;
+ S* target_ptr = ⌖
}
)");
}
TEST(ConstReferenceDeclRefExprsTest, RefVar) {
- RunTest(R"(
+ RunTest<0>(R"(
void f(S& target) {
useVal(/*const*/target);
+ usePtr(&target);
useConstRef(/*const*/target);
- useConstPtr(&target);
- useConstPtrConstRef(&target);
+ useConstPtr(&/*const*/target);
+ useConstPtrConstRef(&/*const*/target);
/*const*/target.constMethod();
target.nonConstMethod();
/*const*/target(ConstTag{});
@@ -194,118 +224,150 @@ TEST(ConstReferenceDeclRefExprsTest, RefVar) {
useConstRef((/*const*/target));
(/*const*/target).constMethod();
(void)(/*const*/target == /*const*/target);
- (void)target;
- (void)⌖
- (void)*⌖
+ (void)/*const*/target;
+ (void)&/*const*/target;
+ (void)*&/*const*/target;
+ /*const*/target;
S copy1 = /*const*/target;
S copy2(/*const*/target);
+ /*const*/target.int_member;
useInt(/*const*/target.int_member);
useIntConstRef(/*const*/target.int_member);
- useIntPtr(target.ptr_member);
- useIntConstPtr(&target.int_member);
+ useIntPtr(/*const*/target.ptr_member);
+ useIntConstPtr(&/*const*/target.int_member);
+
+ (void)(&/*const*/target)->int_member;
+ useIntRef((&target)->int_member);
+
+ const S& const_target_ref = /*const*/target;
+ const S* const_target_ptr = &/*const*/target;
+ S* target_ptr = ⌖
}
)");
}
TEST(ConstReferenceDeclRefExprsTest, PtrVar) {
- RunTest(R"(
+ RunTest<1>(R"(
void f(S* target) {
- useVal(*target);
- useConstRef(*target);
- useConstPtr(target);
+ useVal(*/*const*/target);
+ usePtr(target);
+ useConstRef(*/*const*/target);
+ useConstPtr(/*const*/target);
useConstPtrConstRef(/*const*/target);
+ usePtrConstPtr(&target);
/*const*/target->constMethod();
target->nonConstMethod();
- (*target)(ConstTag{});
+ (*/*const*/target)(ConstTag{});
(*target)[42];
target->operator[](42);
- useConstRef((*target));
+ useConstRef((*/*const*/target));
(/*const*/target)->constMethod();
- (void)(*target == *target);
- (void)*target;
- (void)target;
- S copy1 = *target;
- S copy2(*target);
- useInt(target->int_member);
- useIntConstRef(target->int_member);
- useIntPtr(target->ptr_member);
- useIntConstPtr(&target->int_member);
+ (void)(*/*const*/target == */*const*/target);
+ (void)*/*const*/target;
+ (void)/*const*/target;
+ /*const*/target;
+ S copy1 = */*const*/target;
+ S copy2(*/*const*/target);
+ /*const*/target->int_member;
+ useInt(/*const*/target->int_member);
+ useIntConstRef(/*const*/target->int_member);
+ useIntPtr(/*const*/target->ptr_member);
+ useIntConstPtr(&/*const*/target->int_member);
+
+ const S& const_target_ref = */*const*/target;
+ const S* const_target_ptr = /*const*/target;
+ S* target_ptr = target; // FIXME: we could chect const usage of
`target_ptr`.
}
)");
}
TEST(ConstReferenceDeclRefExprsTest, ConstPtrVar) {
- RunTest(R"(
+ RunTest<1>(R"(
void f(const S* target) {
- useVal(*target);
- useConstRef(*target);
- useConstPtr(target);
- useConstPtrRef(target);
- useConstPtrPtr(&target);
- useConstPtrConstPtr(&target);
+ useVal(*/*const*/target);
+ useConstRef(*/*const*/target);
+ useConstPtr(/*const*/target);
+ useConstPtrRef(/*const*/target);
+ useConstPtrPtr(&/*const*/target);
+ useConstPtrConstPtr(&/*const*/target);
useConstPtrConstRef(/*const*/target);
/*const*/target->constMethod();
- (*target)(ConstTag{});
- (*target)[42];
+ (*/*const*/target)(ConstTag{});
+ (*/*const*/target)[42];
/*const*/target->operator[](42);
- (void)(*target == *target);
- (void)target;
- (void)*target;
- if(target) {}
- S copy1 = *target;
- S copy2(*target);
- useInt(target->int_member);
- useIntConstRef(target->int_member);
- useIntPtr(target->ptr_member);
- useIntConstPtr(&target->int_member);
+ (void)(*/*const*/target == */*const*/target);
+ (void)/*const*/target;
+ (void)*/*const*/target;
+ /*const*/target;
+ if(/*const*/target) {}
+ S copy1 = */*const*/target;
+ S copy2(*/*const*/target);
+ /*const*/target->int_member;
+ useInt(/*const*/target->int_member);
+ useIntConstRef(/*const*/target->int_member);
+ useIntPtr(/*const*/target->ptr_member);
+ useIntConstPtr(&/*const*/target->int_member);
+
+ const S& const_target_ref = */*const*/target;
+ const S* const_target_ptr = /*const*/target;
}
)");
}
TEST(ConstReferenceDeclRefExprsTest, ConstPtrPtrVar) {
- RunTest(R"(
+ RunTest<2>(R"(
void f(const S** target) {
- useVal(**target);
- useConstRef(**target);
- useConstPtr(*target);
- useConstPtrRef(*target);
- useConstPtrPtr(target);
- useConstPtrConstPtr(target);
- useConstPtrConstRef(*target);
- (void)target;
- (void)*target;
- (void)**target;
- if(target) {}
- if(*target) {}
- S copy1 = **target;
- S copy2(**target);
- useInt((*target)->int_member);
- useIntConstRef((*target)->int_member);
- useIntPtr((*target)->ptr_member);
- useIntConstPtr(&(*target)->int_member);
+ useVal(**/*const*/target);
+ useConstRef(**/*const*/target);
+ useConstPtr(*/*const*/target);
+ useConstPtrRef(*/*const*/target);
+ useConstPtrPtr(/*const*/target);
+ useConstPtrConstPtr(/*const*/target);
+ useConstPtrConstRef(*/*const*/target);
+ (void)/*const*/target;
+ (void)*/*const*/target;
+ (void)**/*const*/target;
+ /*const*/target;
+ if(/*const*/target) {}
+ if(*/*const*/target) {}
+ S copy1 = **/*const*/target;
+ S copy2(**/*const*/target);
+ (*/*const*/target)->int_member;
+ useInt((*/*const*/target)->int_member);
+ useIntConstRef((*/*const*/target)->int_member);
+ useIntPtr((*/*const*/target)->ptr_member);
+ useIntConstPtr(&(*/*const*/target)->int_member);
+
+ const S& const_target_ref = **/*const*/target;
+ const S* const_target_ptr = */*const*/target;
}
)");
}
TEST(ConstReferenceDeclRefExprsTest, ConstPtrConstPtrVar) {
- RunTest(R"(
+ RunTest<2>(R"(
void f(const S* const* target) {
- useVal(**target);
- useConstRef(**target);
- useConstPtr(*target);
- useConstPtrConstPtr(target);
- useConstPtrConstRef(*target);
- (void)target;
- (void)target;
- (void)**target;
- if(target) {}
- if(*target) {}
- S copy1 = **target;
- S copy2(**target);
- useInt((*target)->int_member);
- useIntConstRef((*target)->int_member);
- useIntPtr((*target)->ptr_member);
- useIntConstPtr(&(*target)->int_member);
+ useVal(**/*const*/target);
+ useConstRef(**/*const*/target);
+ useConstPtr(*/*const*/target);
+ useConstPtrConstPtr(/*const*/target);
+ useConstPtrConstRef(*/*const*/target);
+ (void)/*const*/target;
+ (void)*/*const*/target;
+ (void)**/*const*/target;
+ /*const*/target;
+ if(/*const*/target) {}
+ if(*/*const*/target) {}
+ S copy1 = **/*const*/target;
+ S copy2(**/*const*/target);
+ (*/*const*/target)->int_member;
+ useInt((*/*const*/target)->int_member);
+ useIntConstRef((*/*const*/target)->int_member);
+ useIntPtr((*/*const*/target)->ptr_member);
+ useIntConstPtr(&(*/*const*/target)->int_member);
+
+ const S& const_target_ref = **/*const*/target;
+ const S* const_target_ptr = */*const*/target;
}
)");
}
>From 09f6a7bd538abf6575dc933f59f80a19fabe0e25 Mon Sep 17 00:00:00 2001
From: Clement Courbet <cour...@google.com>
Date: Mon, 26 Feb 2024 08:36:21 +0000
Subject: [PATCH 4/5] Spell types explicitly (no auto).
---
clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
index 663691c519b8e9..c6e01d2ecd8284 100644
--- a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
@@ -77,7 +77,7 @@ AST_MATCHER_P(DeclRefExpr, doesNotMutateObject, int,
Indirections) {
llvm::SmallVector<StackEntry, 4> Stack;
Stack.emplace_back(&Node, Indirections);
- auto &Ctx = Finder->getASTContext();
+ ASTContext &Ctx = Finder->getASTContext();
while (!Stack.empty()) {
const StackEntry Entry = Stack.back();
@@ -96,7 +96,7 @@ AST_MATCHER_P(DeclRefExpr, doesNotMutateObject, int,
Indirections) {
// Otherwise we have to look at the parents to see how the expression is
// used.
- const auto Parents = Ctx.getParents(*Entry.E);
+ const DynTypedNodeList Parents = Ctx.getParents(*Entry.E);
// Note: most nodes have a single parents, but there exist nodes that have
// several parents, such as `InitListExpr` that have semantic and syntactic
// forms.
>From 4a6aaf65d58b37e10553ca0421d4a9385602329e Mon Sep 17 00:00:00 2001
From: Clement Courbet <cour...@google.com>
Date: Mon, 26 Feb 2024 08:37:58 +0000
Subject: [PATCH 5/5] remove braces in single-line conditions
---
clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
index c6e01d2ecd8284..f0ffa517047b27 100644
--- a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
@@ -90,9 +90,8 @@ AST_MATCHER_P(DeclRefExpr, doesNotMutateObject, int,
Indirections) {
assert(Ty->isPointerType());
Ty = Ty->getPointeeType().getCanonicalType();
}
- if (Ty.isConstQualified()) {
+ if (Ty.isConstQualified())
continue;
- }
// Otherwise we have to look at the parents to see how the expression is
// used.
@@ -143,9 +142,8 @@ AST_MATCHER_P(DeclRefExpr, doesNotMutateObject, int,
Indirections) {
continue;
case CK_LValueToRValue: {
// An rvalue is immutable.
- if (Entry.Indirections == 0) {
+ if (Entry.Indirections == 0)
continue;
- }
Stack.emplace_back(Cast, Entry.Indirections);
continue;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
