steakhal added inline comments.
================ Comment at: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp:10-13 +void foo(...); + +void * bar(void); +char * baz(void); ---------------- NoQ wrote: > ziqingluo-90 wrote: > > steakhal wrote: > > > I would expect this test file to grow quite a bit. > > > As such, I think we should have more self-descriptive names for these > > > functions. > > > > > > I'm also curious what's the purpose of `foo()`in the examples. > > Thanks for the comment. I agree that they should have better names or at > > least explaining comments. > > > > > I'm also curious what's the purpose of `foo()`in the examples. > > > > I make all testing expressions arguments of `foo` so that I do not have to > > create statements to use these expressions while avoiding irrelevant > > warnings. > That's pretty cool but please note that when `foo()` is declared this way, it > becomes a "C-style variadic function" - a very exotic construct that you > don't normally see in code (the only practical example is the > `printf`/`scanf` family of functions). So it may be good that we cover this > exotic case from the start, but it may also be really bad that we don't cover > the *basic* case. > > C++ offers a different way to declare variadic functions: //variadic > templates// (https://en.cppreference.com/w/cpp/language/parameter_pack). > These are less valuable to test because they expand to AST that's very > similar to the basic case, but that also allows you to cover the basic case > better. > > Or you can always make yourself happy with a few overloads that cover all > your needs, it's not like we're worried about code duplication in tests: > ```lang=c++ > void foo(int); > void foo(int, int); > void foo(int, int, int); > void foo(int, int, int, int); > void foo(int, int, int, int, int); > void foo(int, int, int, int, int, int); > ``` IMO its fine. I would probably call it `sink()` though. Ive used the same construct for the same reason in CSA tests with this name. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137379/new/ https://reviews.llvm.org/D137379 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
