[clang-tidy] altera-id-dependent-backward-branch: do not mark all variables

[yeputons-gh]

Summary

Mark variables/fields as ID-dependent when initializing from ID-dependent
expressions only, not any expressions.

Fixes https://github.com/llvm/llvm-project/issues/52790

Depends on D145304

[yeputons-gh]

Applied clang-format to the header as well

[yeputons-gh]

clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp

128

This line previously yielded a false positive. Now it does not. More tests are created in upcoming patches.

[yeputons-gh]

Rebased on top of the recent main.

clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h

private:
   /// Stores the location an ID-dependent variable is created from a call to
   /// an ID function in IdDepVarsMap.
   void saveIdDepVar(const Stmt *Statement, const VarDecl *Variable);
   /// 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,
+  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,
+  void saveIdDepFieldFromPotentialReference(const DeclRefExpr *RefExpr,
                                    const MemberExpr *MemExpr,
                                    const FieldDecl *PotentialField);
   /// Returns the loop type.
   LoopType getLoopType(const Stmt *Loop);
 
 public:
   IdDependentBackwardBranchCheck(StringRef Name, ClangTidyContext *Context)
       : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 };
 
 } // namespace clang::tidy::altera
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ALTERA_IDDEPENDENTBACKWARDBRANCHCHECK_H

clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp

 void IdDependentBackwardBranchCheck::saveIdDepField(const Stmt *Statement,
                                                     const FieldDecl *Field) {
   // Record that this field is thread-dependent.
   IdDepFieldsMap[Field] = IdDependencyRecord(
       Field, Statement->getBeginLoc(),
       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.
   if (IdDepVarsMap.find(PotentialVar) != IdDepVarsMap.end())
     return;
   std::string Message;
   llvm::raw_string_ostream StringStream(Message);
   StringStream << "inferred assignment of ID-dependent value from "
                   "ID-dependent ";
   if (RefExpr) {
     const auto *RefVar = dyn_cast<VarDecl>(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<FieldDecl>(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.
   if (IdDepFieldsMap.find(PotentialField) != IdDepFieldsMap.end())
     return;
   std::string Message;
   llvm::raw_string_ostream StringStream(Message);
   StringStream << "inferred assignment of ID-dependent member from "
                   "ID-dependent ";
   if (RefExpr) {
     const auto *RefVar = dyn_cast<VarDecl>(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<FieldDecl>(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;
+    }
+  }
 }
 
 IdDependentBackwardBranchCheck::LoopType
 IdDependentBackwardBranchCheck::getLoopType(const Stmt *Loop) {
   switch (Loop->getStmtClass()) {
   case Stmt::DoStmtClass:
     return DoLoop;
   case Stmt::WhileStmtClass:
if (Statement && (Variable || Field)) {
     if (Variable)
       saveIdDepVar(Statement, Variable);
     else if (Field)
       saveIdDepField(Statement, Field);
   }
 
   // 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.
   const auto *CondExpr = Result.Nodes.getNodeAs<Expr>("cond_expr");
   const auto *IDCall = Result.Nodes.getNodeAs<CallExpr>("id_call");
   const auto *Loop = Result.Nodes.getNodeAs<Stmt>("backward_branch");
   if (!Loop)
     return;

clang-tools-extra/test/clang-tidy/checkers/altera/id-dependent-backward-branch.cpp

void error() {
 
   // ==== Unused Inferred Assignments ====
   int UnusedThreadID = Example.IDDepField; // OK: not used in any loops
 
   struct { int IDDepField; } UnusedStruct;
   UnusedStruct.IDDepField = ThreadID * 2; // OK: not used in any loops
 }
 
+int foo(int);
+
 void success() {
   int accumulator = 0;
 
   for (int i = 0; i < 1000; i++) {
     if (i < get_local_id(0)) {
       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;
 }