Skip to content

Commit 6d94900

Browse files
ahatanakaahatanaka
committed
Address review comments
- Create a fake virtual source file instead of asking users to create one and pass it to the command line. - Remove the "translation-units" output from test case modules-full-by-mod-name.cpp. The information isn't useful as the source file name isn't passed on the command line in this case. - Inherit from `PreprocessOnlyAction`.
1 parent 0759a5b commit 6d94900

File tree

13 files changed

+152
-159
lines changed

13 files changed

+152
-159
lines changed

‎clang/include/clang-c/Dependencies.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,8 +230,8 @@ clang_experimental_DependencyScannerWorker_getFileDependencies_v1(
230230
CINDEX_LINKAGE CXFileDependencies *
231231
clang_experimental_DependencyScannerWorkerByModName_getFileDependencies_v0(
232232
CXDependencyScannerWorker Worker, int argc, const char *const *argv,
233-
const char *WorkingDirectory, CXModuleDiscoveredCallback *MDC,
234-
void *Context, CXString *error, const char *LookedUpModuleName);
233+
const char *ModuleName, const char *WorkingDirectory,
234+
CXModuleDiscoveredCallback *MDC, void *Context, CXString *error);
235235

236236
/**
237237
* @}

‎clang/include/clang/Frontend/FrontendActions.h

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,6 @@ class ReadPCHAndPreprocessAction : public FrontendAction {
4545
bool usesPreprocessorOnly() const override { return false; }
4646
};
4747

48-
class ReadPCHAndPreprocessByModNameAction : public ReadPCHAndPreprocessAction {
49-
const char *LookedUpModuleName;
50-
void ExecuteAction() override;
51-
52-
public:
53-
ReadPCHAndPreprocessByModNameAction(const char *ModName) : LookedUpModuleName(ModName) {}
54-
};
55-
5648
class DumpCompilerOptionsAction : public FrontendAction {
5749
std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
5850
StringRef InFile) override {
@@ -307,6 +299,14 @@ class PrintPreprocessedAction : public PreprocessorFrontendAction {
307299
bool hasPCHSupport() const override { return true; }
308300
};
309301

302+
class GetDependiciesByModuleNameAction : public PreprocessOnlyAction {
303+
const char *ModuleName;
304+
void ExecuteAction() override;
305+
306+
public:
307+
GetDependiciesByModuleNameAction(const char *ModName) : ModuleName(ModName) {}
308+
};
309+
310310
} // end namespace clang
311311

312312
#endif

‎clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ class DependencyScanningTool {
7474
public:
7575
/// Construct a dependency scanning tool.
7676
DependencyScanningTool(DependencyScanningService &Service,
77-
const char *LookedUpModuleName = nullptr);
77+
const char *ModuleName = nullptr);
7878

7979
/// Print out the dependency information into a string using the dependency
8080
/// file format that is specified in the options (-MD is the default) and
@@ -103,7 +103,7 @@ class DependencyScanningTool {
103103

104104
private:
105105
DependencyScanningWorker Worker;
106-
const char *LookedUpModuleName;
106+
const char *ModuleName;
107107
};
108108

109109
} // end namespace dependencies

‎clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,10 @@ class DependencyScanningWorker {
7474

7575
ScanningOutputFormat getFormat() const { return Format; }
7676

77-
void setLookedUpModuleName(const char *Name) { LookedUpModuleName = Name; }
77+
const char *getModuleName() const { return ModuleName; }
78+
79+
/// Set module name and create a memory buffer for the fake source file.
80+
void setModuleName(const char *Name);
7881

7982
llvm::StringSet<> AlreadySeen;
8083

@@ -93,7 +96,8 @@ class DependencyScanningWorker {
9396
llvm::IntrusiveRefCntPtr<FileManager> Files;
9497
ScanningOutputFormat Format;
9598

96-
const char *LookedUpModuleName = nullptr;
99+
const char *ModuleName = nullptr;
100+
std::unique_ptr<llvm::MemoryBuffer> FakeMemBuffer;
97101
};
98102

99103
} // end namespace dependencies

‎clang/lib/Frontend/FrontendActions.cpp

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -77,20 +77,6 @@ void ReadPCHAndPreprocessAction::ExecuteAction() {
7777
} while (Tok.isNot(tok::eof));
7878
}
7979

80-
void ReadPCHAndPreprocessByModNameAction::ExecuteAction() {
81-
CompilerInstance &CI = getCompilerInstance();
82-
Preprocessor &PP = CI.getPreprocessor();
83-
SourceManager &SM = PP.getSourceManager();
84-
FileID MainFileID = SM.getMainFileID();
85-
SourceLocation FileStart = SM.getLocForStartOfFile(MainFileID);
86-
SmallVector<std::pair<IdentifierInfo *, SourceLocation>, 2> Path;
87-
IdentifierInfo *ModuleID = PP.getIdentifierInfo(LookedUpModuleName);
88-
Path.push_back(std::make_pair(ModuleID, FileStart));
89-
auto ModResult = CI.loadModule(FileStart, Path, Module::Hidden, false);
90-
PPCallbacks *CB = PP.getPPCallbacks();
91-
CB->moduleImport(SourceLocation(), Path, ModResult);
92-
}
93-
9480
std::unique_ptr<ASTConsumer>
9581
ReadPCHAndPreprocessAction::CreateASTConsumer(CompilerInstance &CI,
9682
StringRef InFile) {
@@ -1007,3 +993,17 @@ void PrintDependencyDirectivesSourceMinimizerAction::ExecuteAction() {
1007993
}
1008994
llvm::outs() << Output;
1009995
}
996+
997+
void GetDependiciesByModuleNameAction::ExecuteAction() {
998+
CompilerInstance &CI = getCompilerInstance();
999+
Preprocessor &PP = CI.getPreprocessor();
1000+
SourceManager &SM = PP.getSourceManager();
1001+
FileID MainFileID = SM.getMainFileID();
1002+
SourceLocation FileStart = SM.getLocForStartOfFile(MainFileID);
1003+
SmallVector<std::pair<IdentifierInfo *, SourceLocation>, 2> Path;
1004+
IdentifierInfo *ModuleID = PP.getIdentifierInfo(ModuleName);
1005+
Path.push_back(std::make_pair(ModuleID, FileStart));
1006+
auto ModResult = CI.loadModule(FileStart, Path, Module::Hidden, false);
1007+
PPCallbacks *CB = PP.getPPCallbacks();
1008+
CB->moduleImport(SourceLocation(), Path, ModResult);
1009+
}

‎clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ FullDependencies::getAdditionalArgsWithoutModulePaths() const {
4646
}
4747

4848
DependencyScanningTool::DependencyScanningTool(
49-
DependencyScanningService &Service, const char *LookedUpModuleName)
50-
: Worker(Service), LookedUpModuleName(LookedUpModuleName) {}
49+
DependencyScanningService &Service, const char *ModuleName)
50+
: Worker(Service), ModuleName(ModuleName) {}
5151

5252
llvm::Expected<std::string> DependencyScanningTool::getDependencyFile(
5353
const tooling::CompilationDatabase &Compilations, StringRef CWD) {
@@ -196,7 +196,7 @@ DependencyScanningTool::getFullDependencies(
196196
std::string Input = Compilations.getAllCompileCommands().front().Filename;
197197

198198
FullDependencyPrinterConsumer Consumer(AlreadySeen);
199-
Worker.setLookedUpModuleName(LookedUpModuleName);
199+
Worker.setModuleName(ModuleName);
200200
llvm::Error Result =
201201
Worker.computeDependencies(Input, CWD, Compilations, Consumer);
202202
if (Result)

‎clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,11 @@ class DependencyScanningAction : public tooling::ToolAction {
141141
StringRef WorkingDirectory, DependencyConsumer &Consumer,
142142
llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS,
143143
ExcludedPreprocessorDirectiveSkipMapping *PPSkipMappings,
144-
ScanningOutputFormat Format, const char *LookedUpModuleName)
144+
ScanningOutputFormat Format, const char *ModuleName,
145+
const llvm::MemoryBuffer *FakeMemBuffer)
145146
: WorkingDirectory(WorkingDirectory), Consumer(Consumer),
146147
DepFS(std::move(DepFS)), PPSkipMappings(PPSkipMappings), Format(Format),
147-
LookedUpModuleName(LookedUpModuleName) {}
148+
ModuleName(ModuleName), FakeMemBuffer(FakeMemBuffer) {}
148149

149150
bool runInvocation(std::shared_ptr<CompilerInvocation> Invocation,
150151
FileManager *FileMgr,
@@ -214,6 +215,16 @@ class DependencyScanningAction : public tooling::ToolAction {
214215
.ExcludedConditionalDirectiveSkipMappings = PPSkipMappings;
215216
}
216217

218+
if (ModuleName) {
219+
SmallString<128> FullPath(ModuleName);
220+
llvm::sys::fs::make_absolute(WorkingDirectory, FullPath);
221+
SourceManager &SrcMgr = Compiler.getSourceManager();
222+
FileMgr->getVirtualFile(FullPath.c_str(), FakeMemBuffer->getBufferSize(),
223+
0);
224+
FileID MainFileID = SrcMgr.createFileID(*FakeMemBuffer);
225+
SrcMgr.setMainFileID(MainFileID);
226+
}
227+
217228
// Create the dependency collector that will collect the produced
218229
// dependencies.
219230
//
@@ -249,11 +260,13 @@ class DependencyScanningAction : public tooling::ToolAction {
249260
// the impact of strict context hashing.
250261
Compiler.getHeaderSearchOpts().ModulesStrictContextHash = true;
251262

252-
std::unique_ptr<ReadPCHAndPreprocessAction> Action =
253-
LookedUpModuleName
254-
? std::make_unique<ReadPCHAndPreprocessByModNameAction>(
255-
LookedUpModuleName)
256-
: std::make_unique<ReadPCHAndPreprocessAction>();
263+
std::unique_ptr<FrontendAction> Action;
264+
265+
if (ModuleName)
266+
Action = std::make_unique<GetDependiciesByModuleNameAction>(ModuleName);
267+
else
268+
Action = std::make_unique<ReadPCHAndPreprocessAction>();
269+
257270
const bool Result = Compiler.ExecuteAction(*Action);
258271
if (!DepFS)
259272
FileMgr->clearStatCache();
@@ -266,7 +279,8 @@ class DependencyScanningAction : public tooling::ToolAction {
266279
llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS;
267280
ExcludedPreprocessorDirectiveSkipMapping *PPSkipMappings;
268281
ScanningOutputFormat Format;
269-
const char *LookedUpModuleName;
282+
const char *ModuleName;
283+
const llvm::MemoryBuffer *FakeMemBuffer;
270284
};
271285

272286
} // end anonymous namespace
@@ -318,14 +332,30 @@ llvm::Error DependencyScanningWorker::computeDependencies(
318332
/// Create the tool that uses the underlying file system to ensure that any
319333
/// file system requests that are made by the driver do not go through the
320334
/// dependency scanning filesystem.
321-
tooling::ClangTool Tool(CDB, Input, PCHContainerOps, RealFS, Files);
335+
assert(!!ModuleName == Input.empty());
336+
SmallString<128> FullPath;
337+
tooling::ClangTool Tool(CDB, ModuleName ? ModuleName : Input,
338+
PCHContainerOps, RealFS, Files);
322339
Tool.clearArgumentsAdjusters();
323340
Tool.setRestoreWorkingDir(false);
324341
Tool.setPrintErrorMessage(false);
325342
Tool.setDiagnosticConsumer(&DC);
326343
DependencyScanningAction Action(WorkingDirectory, Consumer, DepFS,
327-
PPSkipMappings.get(), Format,
328-
LookedUpModuleName);
344+
PPSkipMappings.get(), Format, ModuleName,
345+
FakeMemBuffer.get());
346+
347+
if (ModuleName) {
348+
Tool.mapVirtualFile(ModuleName, FakeMemBuffer->getBuffer());
349+
FullPath = ModuleName;
350+
llvm::sys::fs::make_absolute(WorkingDirectory, FullPath);
351+
Tool.appendArgumentsAdjuster(
352+
[&](const tooling::CommandLineArguments &Args, StringRef FileName) {
353+
tooling::CommandLineArguments AdjustedArgs(Args);
354+
AdjustedArgs.push_back(std::string(FullPath));
355+
return AdjustedArgs;
356+
});
357+
}
358+
329359
return !Tool.run(&Action);
330360
});
331361
}
@@ -342,12 +372,13 @@ llvm::Error DependencyScanningWorker::computeDependenciesForClangInvocation(
342372
llvm::opt::ArgStringList CC1Args;
343373
for (const auto &Arg : Arguments)
344374
CC1Args.push_back(Arg.c_str());
375+
345376
std::unique_ptr<CompilerInvocation> Invocation(
346377
newInvocation(&Diags, CC1Args, /*BinaryName=*/nullptr));
347378

348379
DependencyScanningAction Action(WorkingDirectory, Consumer, DepFS,
349-
PPSkipMappings.get(), Format,
350-
LookedUpModuleName);
380+
PPSkipMappings.get(), Format, ModuleName,
381+
FakeMemBuffer.get());
351382

352383
llvm::IntrusiveRefCntPtr<FileManager> FM = Files;
353384
if (!FM)
@@ -356,3 +387,8 @@ llvm::Error DependencyScanningWorker::computeDependenciesForClangInvocation(
356387
PCHContainerOps, &DC);
357388
});
358389
}
390+
391+
void DependencyScanningWorker::setModuleName(const char *Name) {
392+
ModuleName = Name;
393+
FakeMemBuffer = llvm::MemoryBuffer::getMemBuffer(" ");
394+
}

‎clang/test/ClangScanDeps/Inputs/modules_cdb_by_mod_name.json

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
[
2+
{
3+
"directory": "DIR",
4+
"command": "clang -E -IInputs -D INCLUDE_HEADER2 -MD -MF DIR/modules_cdb2.d -fmodules -fcxx-modules -fmodules-cache-path=DIR/module-cache -fimplicit-modules -fimplicit-module-maps -gmodules -x c++",
5+
"file": ""
6+
},
7+
{
8+
"directory": "DIR",
9+
"command": "clang -E -IInputs -fmodules -fcxx-modules -fmodules-cache-path=DIR/module-cache -fimplicit-modules -fimplicit-module-maps -x c++",
10+
"file": ""
11+
},
12+
{
13+
"directory": "DIR",
14+
"command": "clang -E -IInputs -fmodules -fcxx-modules -fmodules-cache-path=DIR/module-cache -fimplicit-modules -fimplicit-module-maps -o a.o -x c++",
15+
"file": ""
16+
},
17+
{
18+
"directory": "DIR",
19+
"command": "clang -E -IInputs -fmodules -fcxx-modules -fmodules-cache-path=DIR/module-cache -fimplicit-modules -fimplicit-module-maps -o b.o -x c++",
20+
"file": ""
21+
}
22+
]

‎clang/test/ClangScanDeps/Inputs/modules_cdb_clangcl_by_mod_name.json

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
[
2+
{
3+
"directory": "DIR",
4+
"command": "clang-cl /E /IInputs /D INCLUDE_HEADER2 /clang:-MD /clang:-MF /clang:DIR/modules_cdb2_clangcl.d /clang:-fmodules /clang:-fcxx-modules /clang:-fmodules-cache-path=DIR/module-cache_clangcl /clang:-fimplicit-modules /clang:-fimplicit-module-maps /clang:-x /clang:c++ --",
5+
"file": ""
6+
},
7+
{
8+
"directory": "DIR",
9+
"command": "clang-cl /E /IInputs /clang:-fmodules /clang:-fcxx-modules /clang:-fmodules-cache-path=DIR/module-cache_clangcl /clang:-fimplicit-modules /clang:-fimplicit-module-maps /clang:-x /clang:c++ --",
10+
"file": ""
11+
},
12+
{
13+
"directory": "DIR",
14+
"command": "clang-cl /E /IInputs /clang:-fmodules /clang:-fcxx-modules /clang:-fmodules-cache-path=DIR/module-cache_clangcl /clang:-fimplicit-modules /clang:-fimplicit-module-maps -o a.o /clang:-x /clang:c++ --",
15+
"file": ""
16+
},
17+
{
18+
"directory": "DIR",
19+
"command": "clang-cl /E /IInputs /clang:-fmodules /clang:-fcxx-modules /clang:-fmodules-cache-path=DIR/module-cache_clangcl /clang:-fimplicit-modules /clang:-fimplicit-module-maps -o b.o /clang:-x /clang:c++ --",
20+
"file": ""
21+
}
22+
]

0 commit comments

Comments
 (0)