[clang-tools-extra] 4f265ce - [clang-tidy] Fix `altera-id-dependent-backward-branch` false positives (#200660)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Jun 6 12:26:11 PDT 2026
Author: Daniil Dudkin
Date: 2026-06-06T19:26:06Z
New Revision: 4f265ce6730f6b109783a56f4c9251ed537c08c4
URL: https://github.com/llvm/llvm-project/commit/4f265ce6730f6b109783a56f4c9251ed537c08c4
DIFF: https://github.com/llvm/llvm-project/commit/4f265ce6730f6b109783a56f4c9251ed537c08c4.diff
LOG: [clang-tidy] Fix `altera-id-dependent-backward-branch` false positives (#200660)
The check unconditionally marked the LHS of any assignment from a
variable or field as ID-dependent. As a result, ordinary assignments
like `x = y` or `i += chunk_size` could trigger false positives in later
loop conditions.
Only propagate ID-dependency when the RHS variable or field is already
known ID-dependent.
Based on the fix by @yeputons, with helper renaming and extra tests for
ordinary assignments, fields, macros, aliases, cv/ref cases, lambdas,
templates, concepts, and the #53777 range-for case.
Fix #52790
Fix #53777
Assisted by Codex.
Added:
Modified:
clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp b/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
index 58db9c9ecb62a..51f02b417bca6 100644
--- a/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
+++ b/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
@@ -138,7 +138,7 @@ void IdDependentBackwardBranchCheck::saveIdDepField(const Stmt *Statement,
Twine("assignment of ID-dependent field ") + Field->getNameAsString());
}
-void IdDependentBackwardBranchCheck::saveIdDepVarFromReference(
+void IdDependentBackwardBranchCheck::saveIdDepVarFromPotentialReference(
const DeclRefExpr *RefExpr, const MemberExpr *MemExpr,
const VarDecl *PotentialVar) {
// If the variable is already in IdDepVarsMap, ignore it.
@@ -151,20 +151,25 @@ void IdDependentBackwardBranchCheck::saveIdDepVarFromReference(
if (RefExpr) {
const auto *RefVar = dyn_cast<VarDecl>(RefExpr->getDecl());
// If variable isn't ID-dependent, but RefVar is.
- if (IdDepVarsMap.contains(RefVar))
+ if (IdDepVarsMap.contains(RefVar)) {
StringStream << "variable " << RefVar->getNameAsString();
+ IdDepVarsMap[PotentialVar] = IdDependencyRecord(
+ PotentialVar, PotentialVar->getBeginLoc(), Message);
+ return;
+ }
}
if (MemExpr) {
const auto *RefField = dyn_cast<FieldDecl>(MemExpr->getMemberDecl());
// If variable isn't ID-dependent, but RefField is.
- if (IdDepFieldsMap.contains(RefField))
+ if (IdDepFieldsMap.contains(RefField)) {
StringStream << "member " << RefField->getNameAsString();
+ IdDepVarsMap[PotentialVar] = IdDependencyRecord(
+ PotentialVar, PotentialVar->getBeginLoc(), Message);
+ }
}
- IdDepVarsMap[PotentialVar] =
- IdDependencyRecord(PotentialVar, PotentialVar->getBeginLoc(), Message);
}
-void IdDependentBackwardBranchCheck::saveIdDepFieldFromReference(
+void IdDependentBackwardBranchCheck::saveIdDepFieldFromPotentialReference(
const DeclRefExpr *RefExpr, const MemberExpr *MemExpr,
const FieldDecl *PotentialField) {
// If the field is already in IdDepFieldsMap, ignore it.
@@ -177,16 +182,21 @@ void IdDependentBackwardBranchCheck::saveIdDepFieldFromReference(
if (RefExpr) {
const auto *RefVar = dyn_cast<VarDecl>(RefExpr->getDecl());
// If field isn't ID-dependent, but RefVar is.
- if (IdDepVarsMap.contains(RefVar))
+ if (IdDepVarsMap.contains(RefVar)) {
StringStream << "variable " << RefVar->getNameAsString();
+ IdDepFieldsMap[PotentialField] = IdDependencyRecord(
+ PotentialField, PotentialField->getBeginLoc(), Message);
+ return;
+ }
}
if (MemExpr) {
const auto *RefField = dyn_cast<FieldDecl>(MemExpr->getMemberDecl());
- if (IdDepFieldsMap.contains(RefField))
+ if (IdDepFieldsMap.contains(RefField)) {
StringStream << "member " << RefField->getNameAsString();
+ IdDepFieldsMap[PotentialField] = IdDependencyRecord(
+ PotentialField, PotentialField->getBeginLoc(), Message);
+ }
}
- IdDepFieldsMap[PotentialField] = IdDependencyRecord(
- PotentialField, PotentialField->getBeginLoc(), Message);
}
IdDependentBackwardBranchCheck::LoopType
@@ -226,11 +236,11 @@ void IdDependentBackwardBranchCheck::check(
// Save variables assigned to values of Id-dependent variables and fields.
if ((RefExpr || MemExpr) && PotentialVar)
- saveIdDepVarFromReference(RefExpr, MemExpr, PotentialVar);
+ saveIdDepVarFromPotentialReference(RefExpr, MemExpr, PotentialVar);
// Save fields assigned to values of ID-dependent variables and fields.
if ((RefExpr || MemExpr) && PotentialField)
- saveIdDepFieldFromReference(RefExpr, MemExpr, PotentialField);
+ saveIdDepFieldFromPotentialReference(RefExpr, MemExpr, PotentialField);
// The second part of the callback deals with checking if a branch inside a
// loop is thread dependent.
diff --git a/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h b/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h
index 01a4ccdf1e717..e8e22cf8c992a 100644
--- a/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h
+++ b/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h
@@ -54,16 +54,16 @@ class IdDependentBackwardBranchCheck : public ClangTidyCheck {
/// Stores the location an ID-dependent field is created from a call to an ID
/// function in IdDepFieldsMap.
void saveIdDepField(const Stmt *Statement, const FieldDecl *Field);
- /// Stores the location an ID-dependent variable is created from a reference
- /// to another ID-dependent variable or field in IdDepVarsMap.
- void saveIdDepVarFromReference(const DeclRefExpr *RefExpr,
- const MemberExpr *MemExpr,
- const VarDecl *PotentialVar);
- /// Stores the location an ID-dependent field is created from a reference to
- /// another ID-dependent variable or field in IdDepFieldsMap.
- void saveIdDepFieldFromReference(const DeclRefExpr *RefExpr,
- const MemberExpr *MemExpr,
- const FieldDecl *PotentialField);
+ /// Stores the location an ID-dependent variable is created from a potential
+ /// reference to another ID-dependent variable or field in IdDepVarsMap.
+ void saveIdDepVarFromPotentialReference(const DeclRefExpr *RefExpr,
+ const MemberExpr *MemExpr,
+ const VarDecl *PotentialVar);
+ /// Stores the location an ID-dependent field is created from a potential
+ /// reference to another ID-dependent variable or field in IdDepFieldsMap.
+ void saveIdDepFieldFromPotentialReference(const DeclRefExpr *RefExpr,
+ const MemberExpr *MemExpr,
+ const FieldDecl *PotentialField);
/// Returns the loop type.
LoopType getLoopType(const Stmt *Loop);
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index cfa9e4ed18b4b..fca6c21f22768 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -338,6 +338,11 @@ New check aliases
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Improved :doc:`altera-id-dependent-backward-branch
+ <clang-tidy/checks/altera/id-dependent-backward-branch>` check by fixing false
+ positives when ordinary variable or field assignments are used in loop
+ conditions.
+
- Improved :doc:`bugprone-argument-comment
<clang-tidy/checks/bugprone/argument-comment>`:
diff --git a/clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp b/clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
index 6137e6f929dc0..c3eb6b6ea541a 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp
@@ -1,4 +1,35 @@
// RUN: %check_clang_tidy %s altera-id-dependent-backward-branch %t -- -header-filter=.* "--" -cl-std=CLC++1.0 -c
+// RUN: %check_clang_tidy -std=c++20-or-later %s altera-id-dependent-backward-branch %t -- -header-filter=.* --
+
+unsigned long get_local_id(unsigned);
+int foo(int);
+
+#ifndef __OPENCL_CPP_VERSION__
+namespace std {
+typedef decltype(sizeof(0)) size_t;
+
+template <class E>
+class initializer_list {
+public:
+ typedef const E *iterator;
+ typedef const E *const_iterator;
+ typedef size_t size_type;
+
+private:
+ iterator Begin;
+ size_type Size;
+
+ constexpr initializer_list(const_iterator Begin, size_type Size)
+ : Begin(Begin), Size(Size) {}
+
+public:
+ constexpr initializer_list() : Begin(nullptr), Size(0) {}
+ constexpr size_type size() const { return Size; }
+ constexpr const_iterator begin() const { return Begin; }
+ constexpr const_iterator end() const { return Begin + Size; }
+};
+} // namespace std
+#endif
void error() {
// ==== Conditional Expressions ====
@@ -64,6 +95,32 @@ void error() {
accumulator++;
}
+ struct {
+ int FieldFromVar;
+ int FieldFromField;
+ } InferredField;
+
+ InferredField.FieldFromVar = ThreadID * 2;
+ while (j < InferredField.FieldFromVar) {
+ // CHECK-NOTES: :[[@LINE-1]]:10: warning: backward branch (while loop) is ID-dependent due to member reference to 'FieldFromVar' and may cause performance degradation [altera-id-dependent-backward-branch]
+ // CHECK-NOTES: :[[@LINE-7]]:5: note: inferred assignment of ID-dependent member from ID-dependent variable ThreadID
+ accumulator++;
+ }
+
+ InferredField.FieldFromField = Example.IDDepField;
+ while (j < InferredField.FieldFromField) {
+ // CHECK-NOTES: :[[@LINE-1]]:10: warning: backward branch (while loop) is ID-dependent due to member reference to 'FieldFromField' and may cause performance degradation [altera-id-dependent-backward-branch]
+ // CHECK-NOTES: :[[@LINE-13]]:5: note: inferred assignment of ID-dependent member from ID-dependent member IDDepField
+ accumulator++;
+ }
+
+ int ThreadIDFromField = Example.IDDepField;
+ while (j < ThreadIDFromField) {
+ // CHECK-NOTES: :[[@LINE-1]]:10: warning: backward branch (while loop) is ID-dependent due to variable reference to 'ThreadIDFromField' and may cause performance degradation [altera-id-dependent-backward-branch]
+ // CHECK-NOTES: :[[@LINE-3]]:3: note: inferred assignment of ID-dependent value from ID-dependent member IDDepField
+ accumulator++;
+ }
+
// ==== Unused Inferred Assignments ====
int UnusedThreadID = Example.IDDepField; // OK: not used in any loops
@@ -79,6 +136,83 @@ void success() {
accumulator++;
}
}
+
+ // ==== Regression tests: ordinary values are not ID-dependent ====
+ int Value = 0;
+ int OtherValue = 1;
+ Value = OtherValue;
+ while (Value < OtherValue) {
+ ++Value;
+ }
+
+ int ComputedValue = foo(0);
+ while (Value < ComputedValue) {
+ ++Value;
+ }
+
+ int InferredValue = OtherValue * 2;
+ for (int i = 0; i < InferredValue; ++i) {
+ accumulator += i;
+ }
+
+ struct {
+ int OrdinaryField;
+ int OtherOrdinaryField;
+ } OrdinaryStruct;
+ OrdinaryStruct.OrdinaryField = foo(0);
+ while (Value < OrdinaryStruct.OrdinaryField) {
+ ++Value;
+ }
+
+ int FromOrdinaryField = OrdinaryStruct.OrdinaryField;
+ while (Value < FromOrdinaryField) {
+ ++Value;
+ }
+
+ OrdinaryStruct.OtherOrdinaryField = OtherValue;
+ while (Value < OrdinaryStruct.OtherOrdinaryField) {
+ ++Value;
+ }
+
+#define ASSIGN_ORDINARY_ID_DEPENDENT_TEST(lhs, rhs) lhs = rhs
+ int MacroAssigned = 0;
+ ASSIGN_ORDINARY_ID_DEPENDENT_TEST(MacroAssigned, OtherValue);
+ while (MacroAssigned < OtherValue) {
+ ++MacroAssigned;
+ }
+#undef ASSIGN_ORDINARY_ID_DEPENDENT_TEST
+
+ typedef int IntTypedef;
+ using IntAlias = int;
+ IntTypedef TypedefValue = OtherValue;
+ IntAlias AliasValue = TypedefValue;
+ while (AliasValue < OtherValue) {
+ ++AliasValue;
+ }
+
+ const int ConstValue = OtherValue;
+ volatile int VolatileValue = OtherValue;
+ int &ValueRef = Value;
+ ValueRef = ConstValue;
+ while (ValueRef < VolatileValue) {
+ ++ValueRef;
+ }
+
+ auto OrdinaryLambda = [&Value, OtherValue]() {
+ int LambdaValue = OtherValue;
+ while (Value < LambdaValue) {
+ ++Value;
+ }
+ };
+ OrdinaryLambda();
+
+#ifndef __OPENCL_CPP_VERSION__
+ for (int ChunkSize : {1, 2, 3}) {
+ for (int i = 0; i < 500; i += ChunkSize) {
+ static_cast<void>(i);
+ }
+ }
+#endif
}
template<char... STOP>
@@ -86,3 +220,34 @@ void gh55408(char const input[], int pos) {
while (((input[pos] != STOP) && ...));
}
+template <typename T>
+void ordinary_template(T Value) {
+ T OtherValue = Value;
+ while (OtherValue < Value) {
+ ++OtherValue;
+ }
+}
+
+template <typename T>
+void dependent_template() {
+ T Value = T();
+ T OtherValue = Value;
+ while (OtherValue < Value) {
+ ++OtherValue;
+ }
+}
+
+#ifndef __OPENCL_CPP_VERSION__
+template <typename T>
+concept HasValue = requires(T Value) {
+ Value + 1;
+};
+
+template <HasValue T>
+void ordinary_constrained_template(T Value) {
+ T OtherValue = Value;
+ while (OtherValue < Value) {
+ ++OtherValue;
+ }
+}
+#endif
More information about the cfe-commits
mailing list