CJ-Johnson marked 5 inline comments as done.
CJ-Johnson added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp:75
+ compoundLiteralExpr(has(StringViewConstructingFromNullExpr)),
+ changeTo(node("null_argument_expr"), cat("")), construction_warning);
+
----------------
ymandel wrote:
> And more below...
Updated all 9 of them. Thanks!
================
Comment at: clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp:90
+ declStmt(
+ has(varDecl(has(StringViewConstructingFromNullExpr),
+ unless(has(cxxConstructExpr(isListInitialization()))))
----------------
ymandel wrote:
> Here and throughout the matchers: in general, `has` is a fallback when you
> don't have a more specific matcher. In this case, It think you mean to be
> testing the initialization expression, in which case you should query that
> directly: `hasInitializer`. I'd recommend you revisit the uses of `has` and
> check in each case if there's a more specific query. That's both cheaper
> (doesn't need to iterate through all children) and more readable (because it
> expresses the intent).
Thanks for the suggestion! I went back through all of the has calls and
replaced the ones I could with something more specific.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp:134-147
+ // (void)(const std::string_view{nullptr}) /* a16 */;
+ // CV qualifiers do not compile in this context
+
+ // (void)(const std::string_view{(nullptr)}) /* a17 */;
+ // CV qualifiers do not compile in this context
+
+ // (void)(const std::string_view{{nullptr}}) /* a18 */;
----------------
ymandel wrote:
> what are these lines doing?
These lines are not "tests" themselves but they clearly document that all of
the various permutations have been considered. If someone reads the test file
and thinks "what about this other case?", these demonstrate that such other
cases have been considered but are not valid :)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113148/new/
https://reviews.llvm.org/D113148
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
