mbenfield added inline comments.
================ Comment at: clang/lib/Sema/SemaStmt.cpp:437 + CompoundStmt *CS = CompoundStmt::Create(Context, Elts, L, R); + DiagnoseUnusedButSetVariables(CS); + return CS; ---------------- mbenfield wrote: > mbenfield wrote: > > mbenfield wrote: > > > george.burgess.iv wrote: > > > > It seems like these two warnings are doing analyses with identical > > > > requirements (e.g., the function's body must be present), but are > > > > taking two different sets of variables into account while doing so. > > > > > > > > Is there a way we can rephrase this so we only need to walk the AST > > > > checking once for all of this? > > > I'll think about this. It might be tricky. > > I'll implement this. It will be a fairly different approach though. > I have this implemented. I am going to run a performance comparison on the > two approaches before making a new revision here. An informal comparison reveals no significant performance difference between these two approaches. I think it's a tradeoff: if you walk each block and/or each function separately, you do have to do more walks, but you're likely to be able to terminate your walk early (because you already ran into a use of each variable/parameter). OTOH, if you walk everything in one go, it is only one walk, but you definitely have to walk the whole thing (because you might run into another CompoundStmt with more declarations). I find the current approach conceptually simpler and so would prefer to keep it as-is. If you disagree let me know. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100581/new/ https://reviews.llvm.org/D100581 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
