zixuw created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
- Upstream methods for action wrappers in generating modules;
- Change the warning message for -Wincomplete-umbrella to report the location
of the umbrella header;
- Add a fix-it hint to include uncovered headers in the umbrella header.
rdar://57095551
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D82118
Files:
clang/include/clang/Frontend/CompilerInstance.h
clang/include/clang/Rewrite/Frontend/FrontendActions.h
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Frontend/Rewrite/FrontendActions.cpp
clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
clang/lib/Lex/PPLexerChange.cpp
clang/test/Modules/Inputs/incomplete-umbrella/normal-module/A1.h
clang/test/Modules/Inputs/incomplete-umbrella/normal-module/A2.h
clang/test/Modules/Inputs/incomplete-umbrella/normal-module/Umbrella.h
clang/test/Modules/Inputs/incomplete-umbrella/normal-module/module.modulemap
clang/test/Modules/incomplete-umbrella-fixit.m
clang/test/Modules/incomplete-umbrella.m
Index: clang/test/Modules/incomplete-umbrella.m
===================================================================
--- clang/test/Modules/incomplete-umbrella.m
+++ clang/test/Modules/incomplete-umbrella.m
@@ -6,8 +6,12 @@
#import <Foo/Baz.h>
@import Foo.Private;
-// CHECK: warning: umbrella header for module 'Foo' does not include header 'Bar.h'
-// CHECK: warning: umbrella header for module 'Foo.Private' does not include header 'Baz.h'
+// CHECK: While building module 'Foo' imported from {{.*[/\]}}incomplete-umbrella.m:4:
+// CHECK-NEXT: In file included from <module-includes>:1:
+// CHECK-NEXT: {{.*Foo[.]framework[/\]Headers[/\]}}FooPublic.h:2:1: warning: umbrella header for module 'Foo' does not include header 'Bar.h'
+// CHECK: While building module 'Foo' imported from {{.*[/\]}}incomplete-umbrella.m:4:
+// CHECK-NEXT: In file included from <module-includes>:2:
+// CHECK-NEXT: {{.*Foo[.]framework[/\]PrivateHeaders[/\]}}Foo.h:2:1: warning: umbrella header for module 'Foo.Private' does not include header 'Baz.h'
int foo() {
int a = BAR_PUBLIC;
int b = BAZ_PRIVATE;
Index: clang/test/Modules/incomplete-umbrella-fixit.m
===================================================================
--- /dev/null
+++ clang/test/Modules/incomplete-umbrella-fixit.m
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t
+// RUN: cp -R %S/Inputs/incomplete-umbrella/normal-module %t
+// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t -Wno-everything -Wincomplete-umbrella -fixit %s && FileCheck %t/Umbrella.h --input-file %t/Umbrella.h --match-full-lines
+// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t -Wno-everything -Wincomplete-umbrella -Werror %s
+
+@import A;
+
+int foo() {
+ return 0;
+}
Index: clang/test/Modules/Inputs/incomplete-umbrella/normal-module/module.modulemap
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/incomplete-umbrella/normal-module/module.modulemap
@@ -0,0 +1,4 @@
+module A {
+ umbrella header "Umbrella.h"
+ export *
+}
Index: clang/test/Modules/Inputs/incomplete-umbrella/normal-module/Umbrella.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/incomplete-umbrella/normal-module/Umbrella.h
@@ -0,0 +1,3 @@
+// Umbrella.h
+// CHECK: #import "A1.h"
+// CHECK: #import "A2.h"
Index: clang/test/Modules/Inputs/incomplete-umbrella/normal-module/A2.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/incomplete-umbrella/normal-module/A2.h
@@ -0,0 +1 @@
+// A2.h
Index: clang/test/Modules/Inputs/incomplete-umbrella/normal-module/A1.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/incomplete-umbrella/normal-module/A1.h
@@ -0,0 +1 @@
+// A1.h
Index: clang/lib/Lex/PPLexerChange.cpp
===================================================================
--- clang/lib/Lex/PPLexerChange.cpp
+++ clang/lib/Lex/PPLexerChange.cpp
@@ -263,10 +263,11 @@
}
void Preprocessor::diagnoseMissingHeaderInUmbrellaDir(const Module &Mod) {
- assert(Mod.getUmbrellaHeader() && "Module must use umbrella header");
- SourceLocation StartLoc =
- SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID());
- if (getDiagnostics().isIgnored(diag::warn_uncovered_module_header, StartLoc))
+ const auto &UmbrellaHeader = Mod.getUmbrellaHeader();
+ assert(UmbrellaHeader.Entry && "Module must use umbrella header");
+ const FileID &File = SourceMgr.translateFile(UmbrellaHeader.Entry);
+ SourceLocation EndLoc = SourceMgr.getLocForEndOfFile(File);
+ if (getDiagnostics().isIgnored(diag::warn_uncovered_module_header, EndLoc))
return;
ModuleMap &ModMap = getHeaderSearchInfo().getModuleMap();
@@ -291,8 +292,26 @@
// Find the relative path that would access this header.
SmallString<128> RelativePath;
computeRelativePath(FileMgr, Dir, *Header, RelativePath);
- Diag(StartLoc, diag::warn_uncovered_module_header)
- << Mod.getFullModuleName() << RelativePath;
+ // FIXME: match CRLF style newlines?
+ std::string IncludeStmt =
+ LangOpts.ObjC
+ ? Twine{"#import \"" + RelativePath + "\"\n"}.str()
+ : Twine{"#include \"" + RelativePath + "\"\n"}.str();
+ unsigned EndOffset = SourceMgr.getDecomposedLoc(EndLoc).second;
+ // If the umbrella header does not end with a newline,
+ // insert a newline before inserting FixIt code.
+ // FIXME: check CRLF style newlines
+ if (EndOffset > 0) {
+ SourceLocation LastCharLoc =
+ SourceMgr.getComposedLoc(File, EndOffset - 1);
+ if (*SourceMgr.getCharacterData(LastCharLoc) != '\n')
+ IncludeStmt = '\n' + IncludeStmt;
+ }
+ // FIXME: insert the missing include to a better location than EOF.
+ // For example right after an include block, or in lexical order.
+ Diag(EndLoc, diag::warn_uncovered_module_header)
+ << Mod.getFullModuleName() << RelativePath
+ << FixItHint::CreateInsertion(EndLoc, IncludeStmt);
}
}
}
Index: clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===================================================================
--- clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -30,6 +30,7 @@
#include "llvm/Support/BuryPointer.h"
#include "llvm/Support/DynamicLibrary.h"
#include "llvm/Support/ErrorHandling.h"
+#include <memory>
using namespace clang;
using namespace llvm::opt;
@@ -182,6 +183,18 @@
Act = std::make_unique<ASTMergeAction>(std::move(Act),
FEOpts.ASTMergeFiles);
+ // Forward the FixItAction as a wrapper when building a module, so that
+ // fix-it hints can be properly applied using the new SourceManager in
+ // the new CompilerInstance.
+ // FIXME: Is this the right way to do it?
+ if (FEOpts.ProgramAction == frontend::FixIt) {
+ CI.setGenModuleActionWrapper(
+ [](const FrontendOptions &FEOpts, std::unique_ptr<FrontendAction> Act)
+ -> std::unique_ptr<FrontendAction> {
+ return std::make_unique<WrappingFixItAction>(std::move(Act));
+ });
+ }
+
return Act;
}
Index: clang/lib/Frontend/Rewrite/FrontendActions.cpp
===================================================================
--- clang/lib/Frontend/Rewrite/FrontendActions.cpp
+++ clang/lib/Frontend/Rewrite/FrontendActions.cpp
@@ -94,7 +94,7 @@
} // end anonymous namespace
bool FixItAction::BeginSourceFileAction(CompilerInstance &CI) {
- const FrontendOptions &FEOpts = getCompilerInstance().getFrontendOpts();
+ const FrontendOptions &FEOpts = CI.getFrontendOpts();
if (!FEOpts.FixItSuffix.empty()) {
FixItOpts.reset(new FixItActionSuffixInserter(FEOpts.FixItSuffix,
FEOpts.FixWhatYouCan));
Index: clang/lib/Frontend/CompilerInstance.cpp
===================================================================
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -1136,6 +1136,10 @@
SourceMgr.pushModuleBuildStack(ModuleName,
FullSourceLoc(ImportLoc, ImportingInstance.getSourceManager()));
+ // Pass along the GenModuleActionWrapper callback
+ auto wrapGenModuleAction = ImportingInstance.getGenModuleActionWrapper();
+ Instance.setGenModuleActionWrapper(wrapGenModuleAction);
+
// If we're collecting module dependencies, we need to share a collector
// between all of the module CompilerInstances. Other than that, we don't
// want to produce any dependency output from the module build.
@@ -1153,8 +1157,12 @@
llvm::CrashRecoveryContext CRC;
CRC.RunSafelyOnThread(
[&]() {
- GenerateModuleFromModuleMapAction Action;
- Instance.ExecuteAction(Action);
+ std::unique_ptr<FrontendAction> Action(
+ new GenerateModuleFromModuleMapAction);
+ if (wrapGenModuleAction) {
+ Action = wrapGenModuleAction(FrontendOpts, std::move(Action));
+ }
+ Instance.ExecuteAction(*Action);
},
DesiredStackSize);
Index: clang/include/clang/Rewrite/Frontend/FrontendActions.h
===================================================================
--- clang/include/clang/Rewrite/Frontend/FrontendActions.h
+++ clang/include/clang/Rewrite/Frontend/FrontendActions.h
@@ -11,6 +11,7 @@
#include "clang/Frontend/FrontendAction.h"
#include "llvm/Support/raw_ostream.h"
+#include <memory>
namespace clang {
class FixItRewriter;
@@ -26,6 +27,8 @@
StringRef InFile) override;
};
+class WrappingFixItAction;
+
class FixItAction : public ASTFrontendAction {
protected:
std::unique_ptr<FixItRewriter> Rewriter;
@@ -43,6 +46,29 @@
public:
FixItAction();
~FixItAction() override;
+ friend WrappingFixItAction;
+};
+
+class WrappingFixItAction : public WrapperFrontendAction {
+ std::unique_ptr<FixItAction> fixItAction;
+
+public:
+ WrappingFixItAction(std::unique_ptr<FrontendAction> WrappedAction)
+ : WrapperFrontendAction(std::move(WrappedAction)),
+ fixItAction(std::make_unique<FixItAction>()) {}
+
+protected:
+ bool BeginSourceFileAction(CompilerInstance &CI) override {
+ // FIXME: Do we abort if the wrapping action returned failure?
+ bool Success = fixItAction->BeginSourceFileAction(CI);
+ Success &= WrapperFrontendAction::BeginSourceFileAction(CI);
+ return Success;
+ }
+
+ void EndSourceFileAction() override {
+ WrapperFrontendAction::EndSourceFileAction();
+ fixItAction->EndSourceFileAction();
+ }
};
/// Emits changes to temporary files and uses them for the original
Index: clang/include/clang/Frontend/CompilerInstance.h
===================================================================
--- clang/include/clang/Frontend/CompilerInstance.h
+++ clang/include/clang/Frontend/CompilerInstance.h
@@ -179,6 +179,12 @@
/// The list of active output files.
std::list<OutputFile> OutputFiles;
+ /// \brief An optional callback function used to wrap all FrontendActions
+ /// produced to generate imported modules before they are executed.
+ std::function<std::unique_ptr<FrontendAction>(
+ const FrontendOptions &opts, std::unique_ptr<FrontendAction> action)>
+ GenModuleActionWrapper;
+
/// Force an output buffer.
std::unique_ptr<llvm::raw_pwrite_stream> OutputStream;
@@ -819,6 +825,19 @@
bool lookupMissingImports(StringRef Name, SourceLocation TriggerLoc) override;
+ void setGenModuleActionWrapper(
+ std::function<std::unique_ptr<FrontendAction>(
+ const FrontendOptions &Opts, std::unique_ptr<FrontendAction> Action)>
+ Wrapper) {
+ GenModuleActionWrapper = Wrapper;
+ };
+
+ std::function<std::unique_ptr<FrontendAction>(
+ const FrontendOptions &Opts, std::unique_ptr<FrontendAction> Action)>
+ getGenModuleActionWrapper() const {
+ return GenModuleActionWrapper;
+ }
+
void addDependencyCollector(std::shared_ptr<DependencyCollector> Listener) {
DependencyCollectors.push_back(std::move(Listener));
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
