Good afternoon, Oscar. I attached updated version of patches.
Checked on CentOS5/2.8.2, mingw, msys, msvs8 and msvs10. 2010/11/9 Óscar Fuentes <[email protected]>: > Unless I'm missing something on how cmake processes options, there is an > use-before-first-assignment problem here. > > LLVM_LIT_ARGS is defined in test/CMakeLists.txt, but it is used in > tools/clang/test/CMakeLists.txt, which is processed before > test/CMakeLists.txt. If the user sets LLVM_LIT_ARGS from the cmake > command line there is no problem, but if he doesn't, the default value > you set in test/CMakeLists.txt is not used for clang. > > You can test this by starting from a new build directory without using > LLVM_LIT_ARGS on the command line, and checkig if the -sv argument is > passed to lit on the clang tests. Sure. I reconfirmed what I was wrong. I have not realized that because I prefer "make edit_cache" :p > The variable is defined on a LLVM/Clang CMakeLists.txt file, so it > belongs to the LLVM namespace. The LIT namespace would make sense iif > LIT had its own CMakeLists.txt files. IMO. I see. > I'm wary by the use of separate_arguments. Why do you need it and why > UNIX_COMMAND was chosen? My intention is simply to separate simple options to list, not to separate complex arguments. It seems UNIX_COMMAND works fine. I may choose whatever works. What would be better? Without separation, we will see lit.py "-sv --no-progress-bar". > On the clang part, the variable CLANG_TEST_EXTRA_ARGS is referenced by > > set(CLANG_TEST_EXTRA_ARGS) > > (which is equivalent to unsetting it) and then you > > + list(APPEND CLANG_TEST_EXTRA_ARGS ${LIT_ARGS}) > > It would be clearer if the set(CLANG_TEST_EXTRA_ARGS) were replaced by > > + set(CLANG_TEST_EXTRA_ARGS ${LIT_ARGS}) > > or replacing the occurrences of CLANG_TEST_EXTRA_ARGS by LIT_ARGS > altogether, as it no longer holds the "extra args" but all the > configurable lit args for clang. I intended to minimize changes and append LLVM_LIT_ARGS to tail of EXTRA_ARGS. It makes sense not to use CLANG_TEST_EXTRA_ARGS. > Also, it would be a good thing to add documentation for the > LLVM_LIT_ARGS variable in docs/CMake.html. I added an entry to docs/CMake.html. Feel free to add something or rewrite whole. ...Takumi
0001-CMake-Add-the-new-option-LLVM_LIT_ARGS.patch
Description: Binary data
0001-test-CMakeLists.txt-Use-LLVM_LIT_ARGS-and-remove-red.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
