ChuanqiXu added inline comments.
================ Comment at: clang/test/ClangScanDeps/Inputs/header-search-pruning/cdb.json:1 -[ - { - "directory": "DIR", - "command": "clang -E DIR/header-search-pruning.cpp -Ibegin -I1 -Ia -I3 -I4 -I5 -I6 -Ib -I8 -Iend DEFINES -fmodules -fcxx-modules -fmodules-cache-path=DIR/module-cache -fimplicit-modules -fmodule-map-file=DIR/module.modulemap", - "file": "DIR/header-search-pruning.cpp" - } -] +[{ + "directory" : "DIR", ---------------- jansvoboda11 wrote: > Please undo the whitespace changes in `ClangScanDeps` tests. It looks like that it's formatted by clang-format surprisingly. I would undo this manually. ================ Comment at: clang/test/ClangScanDeps/Inputs/header-search-pruning/cdb.json:3 + "directory" : "DIR", + "command" : "clang -E DIR/header-search-pruning.cpp -Ibegin -I1 -Ia -I3 -I4 -I5 -I6 -Ib -I8 -Iend DEFINES -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-modules -fmodule-map-file=DIR/module.modulemap", + "file" : "DIR/header-search-pruning.cpp" ---------------- jansvoboda11 wrote: > Can you explain why `-fcxx-modules` is removed? We want to explicitly enable > Clang modules for C++ inputs here. That's off by default in our downstream > repo and we'd like to keep this upstream to prevent conflicts. (it's benign > upstream) According to the discussion in the link, I think it is in consensus that we decide to not support clang modules and c++20 modules at the same time. (I know many people may not have visited it. If any one disagree with the higher idea, I think it would be better to reply in that thread). So the original test case would fail. > We want to explicitly enable Clang modules for C++ inputs here. I am not sure if I missed any thing. Personally, it looks like the test now would test clang modules for c++ inputs. Since it has `-fmodule` option so that clang module is enabled and the input is C++ (from its suffix). Do I misunderstand you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113391/new/ https://reviews.llvm.org/D113391 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits