mboehme marked 2 inline comments as done.
mboehme added inline comments.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp:1163
+namespace {
+
----------------
njames93 wrote:
> Whats with this namespace addition? looks unnecessary and should be removed
I'd like to avoid the definitions of the structs with short names spilling out
into the global namespace and possibly conflicting with other definitions.
The structs used to be defined within the function
`initializerListSequences()`, which is better, but I now need a class template
`S3`, and those can't be defined with a function, so I decided to add a
namespace. There are other places in this file that do the same, and I'm
following that example. It's probably a better idea to actually name the
namespace though, so I've done that now. WDYT?
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/bugprone/use-after-move.cpp:1206
+ {
+ // TODO: Note that this is a regression test.
+ A a;
----------------
njames93 wrote:
> Whats with the todo comment, surery a comment explaining that this shouldn't
> trigger a warning should suffice
Sorry, this was a note-to-self that I neglected to address before uploading the
patch. I've replaced this with a more meaningful comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148110/new/
https://reviews.llvm.org/D148110
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
