awarzynski added a comment. I think that this is almost ready to be merged. Just a couple more comments.
================ Comment at: flang/examples/CMakeLists.txt:1 # This test is not run by default as it requires input. add_executable(external-hello-world ---------------- Could you add the following at the top of this file: ``` if(NOT FLANG_BUILD_EXAMPLES) set(EXCLUDE_FROM_ALL ON) endif() ``` This will make sure that the examples are not build with e.g. `ninja` when `FLANG_BUILD_EXAMPLES` is `Off`. In [[ https://github.com/llvm/llvm-project/blob/43e45f0ec920b45d6073c0aff47597c44948f52c/clang/examples/CMakeLists.txt#L1-L4 | Clang ]], CMake also contains ``` set_property(DIRECTORY PROPERTY EXCLUDE_FROM_ALL ON) ``` I think that `set(EXCLUDE_FROM_ALL ON)` should be sufficient though (see [[ https://cmake.org/cmake/help/latest/prop_tgt/EXCLUDE_FROM_ALL.html#prop_tgt:EXCLUDE_FROM_ALL | CMake ]] docs for `EXCLUDE_FROM_ALL`). Note that in LLVM, there is [[ https://github.com/llvm/llvm-project/blob/43e45f0ec920b45d6073c0aff47597c44948f52c/llvm/cmake/modules/AddLLVM.cmake#L1264-L1273 | add_llvm_example_library ]]. It would be nice to use that, be we'd need to make sure that `LLVM_BUILD_EXAMPLES` is set correctly. But that's not required just now and could be added later. ================ Comment at: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:147 + // If there were errors in processing arguments, don't do anything else. + if (flang->diagnostics().hasErrorOccurred()) + return false; ---------------- This will silence this warning: ``` if (flang->diagnostics().hasErrorOccurred()) { return false; } ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106137/new/ https://reviews.llvm.org/D106137 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits