diff --git a/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h b/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h --- a/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h +++ b/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h @@ -56,14 +56,14 @@ 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); + void saveIdDepVarFromPotentialReference(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); + 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/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp b/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp --- a/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp +++ b/clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp @@ -127,7 +127,7 @@ 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. @@ -140,20 +140,26 @@ if (RefExpr) { const auto *RefVar = dyn_cast(RefExpr->getDecl()); // If variable isn't ID-dependent, but RefVar is. - if (IdDepVarsMap.find(RefVar) != IdDepVarsMap.end()) + if (IdDepVarsMap.find(RefVar) != IdDepVarsMap.end()) { StringStream << "variable " << RefVar->getNameAsString(); + IdDepVarsMap[PotentialVar] = IdDependencyRecord( + PotentialVar, PotentialVar->getBeginLoc(), Message); + return; // Optional, as we only match only one of `RefExpr` or `MemExpr` + } } if (MemExpr) { const auto *RefField = dyn_cast(MemExpr->getMemberDecl()); // If variable isn't ID-dependent, but RefField is. - if (IdDepFieldsMap.find(RefField) != IdDepFieldsMap.end()) + if (IdDepFieldsMap.find(RefField) != IdDepFieldsMap.end()) { StringStream << "member " << RefField->getNameAsString(); + IdDepVarsMap[PotentialVar] = IdDependencyRecord( + PotentialVar, PotentialVar->getBeginLoc(), Message); + return; + } } - 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. @@ -166,16 +172,22 @@ if (RefExpr) { const auto *RefVar = dyn_cast(RefExpr->getDecl()); // If field isn't ID-dependent, but RefVar is. - if (IdDepVarsMap.find(RefVar) != IdDepVarsMap.end()) + if (IdDepVarsMap.find(RefVar) != IdDepVarsMap.end()) { StringStream << "variable " << RefVar->getNameAsString(); + IdDepFieldsMap[PotentialField] = IdDependencyRecord( + PotentialField, PotentialField->getBeginLoc(), Message); + return; // Optional, as we only match only one of `RefExpr` or `MemExpr` + } } if (MemExpr) { const auto *RefField = dyn_cast(MemExpr->getMemberDecl()); - if (IdDepFieldsMap.find(RefField) != IdDepFieldsMap.end()) + if (IdDepFieldsMap.find(RefField) != IdDepFieldsMap.end()) { StringStream << "member " << RefField->getNameAsString(); + IdDepFieldsMap[PotentialField] = IdDependencyRecord( + PotentialField, PotentialField->getBeginLoc(), Message); + return; + } } - IdDepFieldsMap[PotentialField] = IdDependencyRecord( - PotentialField, PotentialField->getBeginLoc(), Message); } IdDependentBackwardBranchCheck::LoopType @@ -215,11 +227,11 @@ // 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/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp b/clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp --- 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 @@ -71,6 +71,8 @@ UnusedStruct.IDDepField = ThreadID * 2; // OK: not used in any loops } +int foo(int); + void success() { int accumulator = 0; @@ -79,4 +81,57 @@ accumulator++; } } + + // ==== Conditional Expressions ==== + for (int i = 0; i < foo(0); i++) { + accumulator++; + } + + int j = 0; + while (j < foo(0)) { + accumulator++; + } + + do { + accumulator++; + } while (j < foo(0)); + + // ==== Assignments ==== + int NotThreadID = foo(0); + + while (j < NotThreadID) { + accumulator++; + } + + do { + accumulator++; + } while (j < NotThreadID); + + struct { int NotIDDepField; } Example; + Example.NotIDDepField = foo(0); + + for (int i = 0; i < Example.NotIDDepField; i++) { + accumulator++; + } + + while (j < Example.NotIDDepField) { + accumulator++; + } + + do { + accumulator++; + } while (j < Example.NotIDDepField); + + // ==== Inferred Assignments ==== + int NotThreadID2 = NotThreadID * 2; + + for (int i = 0; i < NotThreadID2; i++) { + accumulator++; + } + + // ==== Unused Inferred Assignments ==== + int UnusedNotThreadID = Example.NotIDDepField; + + struct { int NotIDDepField; } UnusedStruct; + UnusedStruct.NotIDDepField = NotThreadID * 2; }