https://github.com/unterumarmung updated https://github.com/llvm/llvm-project/pull/200660
>From 52499242bb66f58abd1f664274f8ee042985f37c Mon Sep 17 00:00:00 2001 From: Daniil Dudkin <[email protected]> Date: Sun, 31 May 2026 16:01:00 +0300 Subject: [PATCH 1/4] [clang-tidy] Fix `altera-id-dependent-backward-branch` false positives 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 @yepytons, 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. --- .../altera/IdDependentBackwardBranchCheck.cpp | 34 ++-- .../altera/IdDependentBackwardBranchCheck.h | 20 +-- .../altera/id-dependent-backward-branch.cpp | 164 ++++++++++++++++++ 3 files changed, 196 insertions(+), 22 deletions(-) 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/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..1a587bf1929bd 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,31 @@ 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-6]]: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-12]]: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 +135,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 +219,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 >From 51bd3d999269ed04173ea01e6bf9442ae1c14d6f Mon Sep 17 00:00:00 2001 From: Daniil Dudkin <[email protected]> Date: Sun, 31 May 2026 16:11:28 +0300 Subject: [PATCH 2/4] Add release notes --- clang-tools-extra/docs/ReleaseNotes.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index e40150f3603da..a385c69ca0714 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -332,6 +332,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>`: >From 5c251975d8de8f5ea8e65011866fdac13fb5f644 Mon Sep 17 00:00:00 2001 From: Daniil Dudkin <[email protected]> Date: Fri, 5 Jun 2026 23:11:28 +0300 Subject: [PATCH 3/4] Add blank like --- .../clang-tidy/checkers/altera/id-dependent-backward-branch.cpp | 1 + 1 file changed, 1 insertion(+) 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 1a587bf1929bd..fa187bf025e1e 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 @@ -99,6 +99,7 @@ void error() { 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] >From 718bf8a2748507b94b26eca1431729e814298d9b Mon Sep 17 00:00:00 2001 From: Daniil Dudkin <[email protected]> Date: Sat, 6 Jun 2026 21:20:05 +0300 Subject: [PATCH 4/4] fix tests --- .../checkers/altera/id-dependent-backward-branch.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 fa187bf025e1e..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 @@ -103,14 +103,14 @@ void error() { 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-6]]:5: note: inferred assignment of ID-dependent member from ID-dependent variable ThreadID + // 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-12]]:5: note: inferred assignment of ID-dependent member from ID-dependent member IDDepField + // CHECK-NOTES: :[[@LINE-13]]:5: note: inferred assignment of ID-dependent member from ID-dependent member IDDepField accumulator++; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
