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

[Nico Weber via cfe-commits]

Hm, that's a lot of overhead. Granted, it's a synthetic benchmark, but I
think it'd be good to try this on some real codebase to make sure it really
is negligible overhead in real-world scenarios.

On Wed, Oct 25, 2017 at 3:09 PM, Roman Lebedev via Phabricator via
cfe-commits <cfe-commits at lists.llvm.org> wrote:

> 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
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171025/d1c88d38/attachment-0001.html>