[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

aaron.ballman closed this revision. aaron.ballman added a comment. In D70094#2742760 , @ffrankies wrote: > @aaron.ballman Thank you! As far as I'm aware, this is the last check that we > are planning to submit, so if I do get commit access now it's

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies marked 2 inline comments as done. ffrankies added a comment. @aaron.ballman Thank you! As far as I'm aware, this is the last check that we are planning to submit, so if I do get commit access now it's likely to be unused. However, if that does change, then yes I would be interested in

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Btw, given that you've had several of these approved, would you be interested in getting commit access

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 342467. ffrankies added a comment. Addressed comment by @aaron.ballman and the Pre-update checks linter - Removed calls to `std::move` for `llvm::Twine::str()` object in `IdDependencyRecord` constructors CHANGES SINCE LAST ACTION

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h:32 +: VariableDeclaration(Declaration), Location(Location), + Message(Message) {} +IdDependencyRecord(const FieldDecl *Declaration,

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 342280. ffrankies marked 12 inline comments as done. ffrankies added a comment. Removed unused import statements from IdDependentBackwardBranch.cpp CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70094/new/ https://reviews.llvm.org/D70094 Files:

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 342278. ffrankies added a comment. Addressed comments by @aaron.ballman - Changed capitalization in enum - Used `std::move` in `IdDependencyRecord` constructors - Initialized `VariableDeclaration` and `FieldDeclaration` to `nullptr` - Used

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp:28-31 + hasOperatorName("="), hasOperatorName("*="), hasOperatorName("/="), + hasOperatorName("%="), hasOperatorName("+="), hasOperatorName("-="), +

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies marked an inline comment as done. ffrankies added a comment. @Eugene.Zelenko @aaron.ballman Are there any more changes that need to be made to this check or comments that need to be addressed? Comment at:

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 338160. ffrankies marked 3 inline comments as done. ffrankies added a comment. Removed `*- C++ -*` from IdDependentBackwardBranchCheck.cpp file-level comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70094/new/

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp:1 +//===--- IdDependentBackwardBranchCheck.cpp - clang-tidy *- C++ -*-===// +// `*- C++ -*` is not necessary for `.cpp` files.

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 336512. ffrankies added a comment. Addressed comments by @Eugene.Zelenko and the automated - Fixed header comments and include guard style - Removed unnecessary comments in `getLoopType()` - changed `IDDependencyRecord() {}` to `IDDependencyRecord() =

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp:200 + if (isa(Loop)) +return DO_LOOP; // loop_type = 0; + else if (isa(Loop)) Is loop ID is not enough? Why does comment with

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 335195. ffrankies added a comment. - Addressed comments from the automated pre-merge checks - Moved link to external documentation to the end of `clang-tools-extra/docs/clang-tidy/checks/altera-id-dependent-backward-branch.rst` as requested by

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/altera-id-dependent-backward-branch.rst:9 + +Based on the `Altera SDK for OpenCL: Best Practices Guide

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 334979. ffrankies added a comment. - Rebased on top of latest changes in main branch - The diagnostic that identifies the code location where an ID-dependent variable/field is assigned has been changed from a warning to a note - Changes addressing code

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 254305. ffrankies marked 5 inline comments as done. ffrankies added a comment. - Updated underlying repo to https://github.com/llvm/llvm-project - Removed braces from one-line if-statements CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70094/new/

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 232724. ffrankies marked 2 inline comments as done. ffrankies added a comment. Addressed @Eugene.Zelenko's comments on use of `auto` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70094/new/ https://reviews.llvm.org/D70094 Files:

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

Eugene.Zelenko added inline comments. Comment at: clang-tidy/altera/IdDependentBackwardBranchCheck.cpp:115 +IdDependentBackwardBranchCheck::hasIdDepField(const Expr *Expression) { + if (const MemberExpr *MemberExpression = dyn_cast(Expression)) { +const FieldDecl

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 231309. ffrankies added a comment. Implemented changes requested by @Eugene.Zelenko Also changed for loop in `hasIdDepVar` and `hasIdDepField` to range-based for loops, and updated the license information in IdDependentBackwardBranchCheck.cpp CHANGES

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

Eugene.Zelenko added inline comments. Comment at: clang-tidy/altera/IdDependentBackwardBranchCheck.cpp:106 +if (auto *ChildExpression = dyn_cast(*i)) { + auto Result = hasIdDepVar(ChildExpression); + if (Result) { Please don't use auto when type is

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies updated this revision to Diff 228743. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70094/new/ https://reviews.llvm.org/D70094 Files: clang-tidy/CMakeLists.txt clang-tidy/ClangTidyForceLinker.h clang-tidy/altera/AlteraTidyModule.cpp clang-tidy/altera/CMakeLists.txt

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

ffrankies created this revision. ffrankies added reviewers: alexfh, jdoerfert, hokein, aaron.ballman. ffrankies added projects: clang-tools-extra, clang, LLVM. Herald added subscribers: mgehre, arphaman, xazax.hun, Anastasia, mgorny. This lint check is a part of the FLOCL (FPGA Linters for