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

Dan Liew via llvm-commits llvm-commits at lists.llvm.org
Thu May 26 16:03:05 PDT 2016


delcypher added inline comments.

================
Comment at: lib/Fuzzer/test/CMakeLists.txt:31
@@ +30,3 @@
+# add_libfuzzer_test(<name>
+#   [NO_MAIN]
+#   SOURCES source0.cpp [source1.cpp ...]
----------------
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?


http://reviews.llvm.org/D20706





More information about the llvm-commits mailing list