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

Reply via email to