dexonsmith added inline comments.
================
Comment at:
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h:78-80
+ /// The compiler invocation associated with the translation unit that imports
+ /// this module.
+ CompilerInvocation Invocation;
----------------
Looks like this will be a deep copy, but it doesn't look like it's being
modified. Can this just be a `const &`, taken in the `ModuleDeps` constructor?
Or is there a lifetime reason this needs to be as it is?
================
Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:25
+
+ // Remove options incompatible with explicit module build.
+ CI.getFrontendOpts().Inputs.clear();
----------------
Should this call any of the `resetNonModularOptions()` functions, or are those
intentionally omitted?
================
Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:26-27
+ // Remove options incompatible with explicit module build.
+ CI.getFrontendOpts().Inputs.clear();
+ CI.getFrontendOpts().OutputFile.clear();
+
----------------
Should `FrontendOpts` gain a `resetNonModularOptions()`?
================
Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:62
std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps) const {
- // TODO: Build full command line. That also means capturing the original
- // command line into NonPathCommandLine.
-
- std::vector<std::string> Ret{
- "-fno-implicit-modules",
- "-fno-implicit-module-maps",
- };
+ CompilerInvocation CI = getFullCommandLineCompilerInvocation(*this);
----------------
I think guaranteed copy elision means this won't be a deep copy of the return,
but it might be nice to add a move constructor for `CompilerInvocation` so it's
more obvious.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100534/new/
https://reviews.llvm.org/D100534
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
