[PATCH] D38954: [Sema] -Wzero-as-null-pointer-constant: don't warn for system macros other than NULL.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D38954#906900, @thakis wrote:

> Can you build some large-ish codebase (say, LLVM) with and without this patch and make sure that this doesn't measurably add to build perf? (With the warning turned on, obviously.)
>
> Other than that, this looks good to me.


Very contrived example:

  $ head -n 3 test.cpp 
  #include <cstddef>
  
  int main(int argc, char* argv[]) {
  $ perl -e 'print "if(argv == NULL) return 1;\n"x1000000; print "\n"' >> test.cpp
  $ tail -n 2 test.cpp 
  }

So this is a file with 1 million comparisons of pointer with `NULL`.

  $ time clang -fsyntax-only -Wzero-as-null-pointer-constant -std=c++11 test.cpp -w
  
  real    0m8.197s
  user    0m8.071s
  sys     0m0.124s
  $ time clang -fsyntax-only -Wzero-as-null-pointer-constant -std=c++11 test.cpp -w
  
  real    0m7.881s
  user    0m7.728s
  sys     0m0.152s
  $ time clang -fsyntax-only -Wzero-as-null-pointer-constant -std=c++11 test.cpp -w
  
  real    0m7.212s
  user    0m7.063s
  sys     0m0.148s
  $ time /build/llvm-build-Clang-release/bin/clang -fsyntax-only -Wzero-as-null-pointer-constant -std=c++11 test.cpp -w
  
  real    0m11.200s
  user    0m11.070s
  sys     0m0.128s
  $ time /build/llvm-build-Clang-release/bin/clang -fsyntax-only -Wzero-as-null-pointer-constant -std=c++11 test.cpp -w
  
  real    0m11.141s
  user    0m11.019s
  sys     0m0.121s
  $ time /build/llvm-build-Clang-release/bin/clang -fsyntax-only -Wzero-as-null-pointer-constant -std=c++11 test.cpp -w
  
  real    0m11.254s
  user    0m11.127s
  sys     0m0.126s

So there absolutely is some penalty, but it does not appear to be huge, at least on this contrived example.
I *could* try benchmark-building llvm, but i'm still sporadically experiencing a crash <https://bugs.llvm.org/show_bug.cgi?id=34480>, so the timings will not be comparable.



================
Comment at: test/SemaCXX/warn-zero-nullptr.cpp:68
+ public:
+// FIXME: this one should *NOT* warn.
+  explicit TemplateClass1(int a, T default_value = 0) {} // expected-warning{{zero as null pointer constant}} expected-warning{{zero as null pointer constant}}
----------------
thakis wrote:
> Did you mean to fix this before commit?
Eh, i'm conflicted about this one.
This is more of "for future consideration"


Repository:
  rL LLVM

https://reviews.llvm.org/D38954