This revision was automatically updated to reflect the committed changes.
Closed by commit rL298059: [clang-tidy] readability-misleading-indentation: fix
chained if (authored by alexfh).
Changed prior to commit:
https://reviews.llvm.org/D30841?vs=92026&id=92116#toc
Repository:
rL LLVM
https
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
In https://reviews.llvm.org/D30841#702980, @fgross wrote:
> Now using `ASTContext::getParents` instead of `ChainedIfs` map.
>
> For some reason I thought of `getParents` as an expensive functio
fgross updated this revision to Diff 92026.
fgross added a comment.
Now using `ASTContext::getParents` instead of `ChainedIfs` map.
For some reason I thought of `getParents` as an expensive function to call...
https://reviews.llvm.org/D30841
Files:
clang-tidy/readability/MisleadingIndentatio
alexfh added inline comments.
Comment at: clang-tidy/readability/MisleadingIndentationCheck.cpp:40-42
+ for (auto Iter = ChainedIfs.find(If); Iter != ChainedIfs.end();
+ Iter = ChainedIfs.find(Iter->second))
+IfLoc = Iter->second->getIfLoc();
alexfh wr
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Thank you for the fix! One comment inline.
Comment at: clang-tidy/readability/MisleadingIndentationCheck.cpp:40-42
+ for (auto Iter = ChainedIfs.find(If); Iter !=
fgross added a comment.
I don't have commit access, so could someone please commit this for me? Thanks!
https://reviews.llvm.org/D30841
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
In https://reviews.llvm.org/D30841#698634, @fgross wrote:
> I just assumed it would traverse in the "right" way, is there any
> documentation about AST / matcher traversal?
I do not kn
fgross updated this revision to Diff 91473.
fgross added a comment.
Replaced std::map with llvm::DenseMap, added comment about traversal.
I just assumed it would traverse in the "right" way, is there any documentation
about AST / matcher traversal?
https://reviews.llvm.org/D30841
Files:
cla
xazax.hun added a comment.
Functionally LGTM!
Note that while the traversal of AST Matchers are not defined in general, in
this particular case of chained ifs, it is guaranteed that parent nodes will be
matched before the child nodes. For this reason I think it is ok to have a
state like this.
fgross created this revision.
Herald added a subscriber: JDevlieghere.
Fixed erroneously flagging of chained if statements when styled like this:
if (cond) {
}
else if (cond) {
}
else {
}
https://reviews.llvm.org/D30841
Files:
clang-tidy/readability/MisleadingIndentationCheck.cpp
10 matches
Mail list logo