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@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@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits