https://github.com/mygitljf created https://github.com/llvm/llvm-project/pull/202105
I fixed the misleading note location for inferred ID-dependent fields. Now the note points to the assignment that introduces the dependency, not the field declaration. I also updated the existing clang-tidy test to cover the corrected behavior. Fixes #202077 >From d83d9b59472b58938b263aab3a63832fe150a8cd Mon Sep 17 00:00:00 2001 From: mygitljf <[email protected]> Date: Sun, 7 Jun 2026 06:09:51 +0000 Subject: [PATCH] [clang-tidy] Fix inferred field note location --- .../altera/IdDependentBackwardBranchCheck.cpp | 29 +++++++++++-------- .../altera/IdDependentBackwardBranchCheck.h | 3 +- .../altera/id-dependent-backward-branch.cpp | 4 +-- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp b/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp index 51f02b417bca6..82a02dd6e9344 100644 --- a/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp +++ b/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp @@ -50,11 +50,13 @@ void IdDependentBackwardBranchCheck::registerMatchers(MatchFinder *Finder) { // Bind all VarDecls that are assigned a value with a variable DeclRefExpr (in // case it is ID-dependent). Finder->addMatcher( - stmt(forEachDescendant(binaryOperator( - allOf(isAssignmentOperator(), hasRHS(RefVarOrField), - hasLHS(anyOf( - declRefExpr(to(varDecl().bind("pot_tid_var"))), - memberExpr(member(fieldDecl().bind("pot_tid_field"))))))))), + stmt(forEachDescendant( + binaryOperator( + allOf(isAssignmentOperator(), hasRHS(RefVarOrField), + hasLHS(anyOf(declRefExpr(to(varDecl().bind("pot_tid_var"))), + memberExpr(member( + fieldDecl().bind("pot_tid_field"))))))) + .bind("potential_assignment"))), this); // Second Matcher looks for branch statements inside of loops and bind on the @@ -170,8 +172,8 @@ void IdDependentBackwardBranchCheck::saveIdDepVarFromPotentialReference( } void IdDependentBackwardBranchCheck::saveIdDepFieldFromPotentialReference( - const DeclRefExpr *RefExpr, const MemberExpr *MemExpr, - const FieldDecl *PotentialField) { + const Stmt *Statement, const DeclRefExpr *RefExpr, + const MemberExpr *MemExpr, const FieldDecl *PotentialField) { // If the field is already in IdDepFieldsMap, ignore it. if (IdDepFieldsMap.contains(PotentialField)) return; @@ -184,8 +186,8 @@ void IdDependentBackwardBranchCheck::saveIdDepFieldFromPotentialReference( // If field isn't ID-dependent, but RefVar is. if (IdDepVarsMap.contains(RefVar)) { StringStream << "variable " << RefVar->getNameAsString(); - IdDepFieldsMap[PotentialField] = IdDependencyRecord( - PotentialField, PotentialField->getBeginLoc(), Message); + IdDepFieldsMap[PotentialField] = + IdDependencyRecord(PotentialField, Statement->getBeginLoc(), Message); return; } } @@ -193,8 +195,8 @@ void IdDependentBackwardBranchCheck::saveIdDepFieldFromPotentialReference( const auto *RefField = dyn_cast<FieldDecl>(MemExpr->getMemberDecl()); if (IdDepFieldsMap.contains(RefField)) { StringStream << "member " << RefField->getNameAsString(); - IdDepFieldsMap[PotentialField] = IdDependencyRecord( - PotentialField, PotentialField->getBeginLoc(), Message); + IdDepFieldsMap[PotentialField] = + IdDependencyRecord(PotentialField, Statement->getBeginLoc(), Message); } } } @@ -220,6 +222,8 @@ void IdDependentBackwardBranchCheck::check( const auto *Variable = Result.Nodes.getNodeAs<VarDecl>("tid_dep_var"); const auto *Field = Result.Nodes.getNodeAs<FieldDecl>("tid_dep_field"); const auto *Statement = Result.Nodes.getNodeAs<Stmt>("straight_assignment"); + const auto *PotentialAssignment = + Result.Nodes.getNodeAs<Stmt>("potential_assignment"); const auto *RefExpr = Result.Nodes.getNodeAs<DeclRefExpr>("assign_ref_var"); const auto *MemExpr = Result.Nodes.getNodeAs<MemberExpr>("assign_ref_field"); const auto *PotentialVar = Result.Nodes.getNodeAs<VarDecl>("pot_tid_var"); @@ -240,7 +244,8 @@ void IdDependentBackwardBranchCheck::check( // Save fields assigned to values of ID-dependent variables and fields. if ((RefExpr || MemExpr) && PotentialField) - saveIdDepFieldFromPotentialReference(RefExpr, MemExpr, PotentialField); + saveIdDepFieldFromPotentialReference(PotentialAssignment, 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 e8e22cf8c992a..6ce35e3b283c7 100644 --- a/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h +++ b/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h @@ -61,7 +61,8 @@ class IdDependentBackwardBranchCheck : public ClangTidyCheck { 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, + void saveIdDepFieldFromPotentialReference(const Stmt *Statement, + const DeclRefExpr *RefExpr, const MemberExpr *MemExpr, const FieldDecl *PotentialField); /// Returns the loop type. 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 c3eb6b6ea541a..9b52f0aac9704 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-7]]:5: note: inferred assignment of ID-dependent member from ID-dependent variable ThreadID + // CHECK-NOTES: :[[@LINE-3]]:3: 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 + // CHECK-NOTES: :[[@LINE-3]]:3: 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
