[PATCH] 'size' call that could be replaced with 'empty' on STL containers

[Alexander Kornienko]

One comment seems to have slipped through the cracks. And one more nit.


================
Comment at: clang-tidy/readability/ContainerSizeEmpty.cpp:22
@@ +21,3 @@
+namespace {
+const llvm::StringSet<> ContainerNames = [] {
+  llvm::StringSet<> RetVal;
----------------
It looks like this variable would better be a static local variable inside isContainer.

That's a smart way to initialize a StringSet variable, btw ;)

================
Comment at: test/clang-tidy/readibility-container-size-empty.cpp:18
@@ +17,3 @@
+  // CHECK-MESSAGES: if (vect.size() == 0)
+  // CHECK-FIXES: vect.empty()
+  if (vect.size() != 0)
----------------
Not sure if Phabricator failed to display the previous comment on this line or you just missed it. Repeating it here for convenience:

---

I actually meant to make the CHECK-FIXES lines more specific, not CHECK-MESSAGES (they are specific enough due to line numbers). Each type of CHECK lines is checked independently, and the location of a CHECK line doesn't matter, only their order (unless the [[@LINE]] expression is used). So essentially, fixed output is verified against this set of checks:

  // CHECK-FIXES: vect.empty()
  // CHECK-FIXES: !vect.empty()
  // CHECK-FIXES: vect.empty()
  // CHECK-FIXES: !vect.empty()
  // CHECK-FIXES: !vect.empty()
  // CHECK-FIXES: !vect.empty()
  // CHECK-FIXES: vect.empty()
  // CHECK-FIXES: vect.empty()
  // CHECK-FIXES: !vect.empty()
  // CHECK-FIXES: !vect.empty()
  // CHECK-FIXES: vect.empty()
  // CHECK-FIXES: !vect.empty()
  // CHECK-FIXES: !vect2.empty()
  // CHECK-FIXES: vect3->empty()
  // CHECK-FIXES: vect4.empty()
  // CHECK-FIXES: !v.empty()

The only thing that is verified, is that there is an ordered subset of lines in the input that match the patterns above. As you can guess, this is not particularly useful, as any of the "vect.empty()" patterns would also match "if(!vect.empty());" line, for example.

So please specify full lines of the output together with the leading "{{^ }}" (the part in double curlies is treated as a regular expression).

Thanks!

---

One clarification: I think, CHECK-MESSAGES lines checking code snippets in warning messages don't add any value and just complicate the test. You could safely remove them and instead make CHECK-FIXES lines verify whole lines.

http://reviews.llvm.org/D6925

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/