steveire marked 2 inline comments as done.
steveire added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp:80
auto Matches =
- match(expr(hasDescendant(typeLoc().bind("t"))), *E, *Result.Context);
+ match(traverse(TK_AsIs, expr(hasDescendant(typeLoc().bind("t")))), *E,
+ *Result.Context);
----------------
aaron.ballman wrote:
> While I believe the change is necessary here, it's not obvious to me what
> "hints" tell me this behavior is correct for the given matchers. None of the
> matchers look like they're going to care about implicit nodes, so how am I to
> know that AsIs is correct or not as a reviewer? As it stands, I sort of feel
> like I have to take it on faith that this change is correct and it looks a
> little suspicious because the code using the matcher is creating a fix-it at
> what now may be the location of an implicit node.
I don't know if I was wrong about it being required before, or if it was
required before, but the change to this file is not required now.
================
Comment at: clang/lib/AST/Expr.cpp:3001
Expr *A = C->getArg(0);
- if (A->getSourceRange() == SR || !isa<CXXTemporaryObjectExpr>(C))
+ if (A->getSourceRange() == SR || C->isElidable()) {
E = A;
----------------
ymandel wrote:
> aaron.ballman wrote:
> > Looks like the change introduced new curly braces for a single-line if.
> Why is it necessary to check isElidable? I think the logic here is subtle
> (since the AST doesn't explicitly tag implicit expressions), so please add an
> explanatory comment.
The `ignoringElidableConstructorCall` does it this way. I'm afraid I don't know
more than that.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82278/new/
https://reviews.llvm.org/D82278
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
