[PATCH] D20706: [LibFuzzer] Refactor declaration of tests in CMake.

Kostya Serebryany via llvm-commits llvm-commits at lists.llvm.org
Thu May 26 16:07:38 PDT 2016


kcc added inline comments.

================
Comment at: lib/Fuzzer/test/CMakeLists.txt:31
@@ +30,3 @@
+# add_libfuzzer_test(<name>
+#   [NO_MAIN]
+#   SOURCES source0.cpp [source1.cpp ...]
----------------
delcypher wrote:
> kcc wrote:
> > Let's start from not having this "NO_MAIN" complexity. 
> > CustomMainTests is empty and could be safely removed. 
> > The gunit unit test is a separate beast, one of its kind. 
> @kcc:
> 
> I kept "NO_MAIN" because
> 
> * I presumed the intention was to add tests to `CustomMainTests` at some point in the future. Is this not  the intention?
> * Using the `NO_MAIN` variant of `add_libfuzzer_test()` for the unit test is very useful because in a subsequent patch (https://github.com/delcypher/llvm/commit/63ba587cd1f94e1b7c29877f1f7afdff5efc477c) I am going to add logic to fix weak linking issues on OSX. If I don't use `add_libfuzzer_test()` for the unit test then I will have duplicate the logic that fixes the weak linking issue for the unit test which isn't desirable because I am trying to avoid as much boiler plate CMake code as possible.
> 
> It is your call but I would prefer to use `add_libfuzzer_test()` for the unit test so that my subsequent patch for weak linking is cleaner. What would you like to do?
Yes, I hope to not need CustomMainTests in future you can safely delete that part. 

Sure, please have as much code reuse as possible for gnuint test, but I think you can do it w/o thie NO_MAIN



http://reviews.llvm.org/D20706





More information about the llvm-commits mailing list