[PATCH] D113148: Add new clang-tidy check for string_view(nullptr)

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel accepted this revision.
ymandel added a comment.
This revision is now accepted and ready to land.

To other reviewers: barring any additional comments, I will push this patch tomorrow morning (CJ doesn't have commit rights).



================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp:117-130
+    // (void)(const std::string_view(nullptr)) /* a6 */;
+    // CV qualifiers do not compile in this context
+
+    // (void)(const std::string_view((nullptr))) /* a7 */;
+    // CV qualifiers do not compile in this context
+
+    // (void)(const std::string_view({nullptr})) /* a8 */;
----------------
nit: personally, i'd just delete these -- anything that doens't compile isn't really of interest to clang tidy. While I see the educational value, these kinds of comments are prone to bitrot (since we're not actually testing anything) and therefore have questionable value even for education.


================
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 */;
----------------
CJ-Johnson wrote:
> 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 :)
ah, i see. maybe put in a comment to that respect?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113148/new/

https://reviews.llvm.org/D113148