[llvm-dev] [cfe-dev] [SemaCXX] Should we fix test failing due to reverse iteration?
Vedant Kumar via llvm-dev
llvm-dev at lists.llvm.org
Thu Jun 1 15:49:28 PDT 2017
> On Jun 1, 2017, at 1:49 PM, Craig Topper via cfe-dev <cfe-dev at lists.llvm.org> wrote:
> Adding cfe-dev
> On Thu, Jun 1, 2017 at 1:46 PM, Grang, Mandeep Singh via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
> I see that the following test fails if reverse iteration of SmallPtrSet is enabled:
I think I saw a bot complaining about this. Could you back out the change until these failures can be addressed? (Apologies if you already have.)
> This is because in SemaStmt.cpp we iterate SmallPtrSet and output warnings about the variables not used in the loop.
> Expected output: warning: variables 'i', 'j', and 'k' used in loop condition not modified
> Output with reverse iteration: warning: variables 'k', 'j', and 'i' used in loop condition not
> I would like the community's opinion on whether this is something worth fixing? In this case, should the output always be the same irrespective of the iteration order?
Yes, I think so. Running the compiler twice should produce identical diagnostics.
> If yes, then we have 2 alternatives:
> 1. Change SmallPtrSet to SmallVector for the container (VarDecls) being iterated - this may have a compile time impact (need to measure).
> 2. Sort the container (VarDecls) before iteration. We can sort based on decl source location and decl name. Not sure if these guaranteed to be unique?
CheckForLoopConditionalStatement is hot, relative to the code that actually emits the diagnostic . So I'd prefer the second option.
 http://lab.llvm.org:8080/coverage/coverage-reports/clang/coverage/Users/buildslave/jenkins/sharedspace/clang-stage2-coverage-R@2/llvm/tools/clang/lib/Sema/SemaStmt.cpp.html#L1454 <http://lab.llvm.org:8080/coverage/coverage-reports/clang/coverage/Users/buildslave/jenkins/sharedspace/clang-stage2-coverage-R@2/llvm/tools/clang/lib/Sema/SemaStmt.cpp.html#L1454>
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-dev