Skip to content

Commit a9f9bd1

Browse files
committed
[SourceKit] Record module loading errors when generating interfaces
Record up to two errors emitted when we fail to load a module for interface generation, and include these errors in the message we pass back to the editor. This should help us better pin down the reason why interface generation failed. rdar://109511099
1 parent 85d59d2 commit a9f9bd1

File tree

4 files changed

+186
-15
lines changed

4 files changed

+186
-15
lines changed

include/swift/AST/DiagnosticEngine.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1449,6 +1449,22 @@ namespace swift {
14491449
}
14501450
};
14511451

1452+
/// A RAII object that adds and removes a diagnostic consumer from an engine.
1453+
class DiagnosticConsumerRAII final {
1454+
DiagnosticEngine &Diags;
1455+
DiagnosticConsumer &Consumer;
1456+
1457+
public:
1458+
DiagnosticConsumerRAII(DiagnosticEngine &diags,
1459+
DiagnosticConsumer &consumer)
1460+
: Diags(diags), Consumer(consumer) {
1461+
Diags.addConsumer(Consumer);
1462+
}
1463+
~DiagnosticConsumerRAII() {
1464+
Diags.removeConsumer(Consumer);
1465+
}
1466+
};
1467+
14521468
inline void
14531469
DiagnosticEngine::diagnoseWithNotes(InFlightDiagnostic parentDiag,
14541470
llvm::function_ref<void(void)> builder) {
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// RUN: %empty-directory(%t/Inputs)
2+
// RUN: %empty-directory(%t/Inputs/objcfail)
3+
// RUN: split-file %s %t/Inputs
4+
5+
//--- objcfail/objcfail.h
6+
7+
#ifdef FAIL
8+
#error some error from Clang module
9+
10+
// We only record the first error emitted, so we ignore this one.
11+
#error another error from Clang module
12+
#endif
13+
14+
void foo(void);
15+
16+
//--- objcfail/module.modulemap
17+
18+
module ObjCFail {
19+
header "objcfail.h"
20+
export *
21+
}
22+
23+
//--- Library.swift
24+
25+
import ObjCFail
26+
27+
// First try printing the interface of the Clang module directly.
28+
29+
// RUN: %sourcekitd-test -req=interface-gen -module ObjCFail -- -I %t/Inputs/objcfail -target %target-triple %s | %FileCheck --check-prefix DIRECT-SUCCESS %s
30+
// DIRECT-SUCCESS: public func foo()
31+
32+
// RUN: not %sourcekitd-test -req=interface-gen -module ObjCFail -- -Xcc -DFAIL -I %t/Inputs/objcfail -target %target-triple %s 2>&1 | %FileCheck --check-prefix DIRECT-FAIL %s
33+
// DIRECT-FAIL: Could not load module: ObjCFail (could not build {{Objective-C|C}} module 'ObjCFail', some error from Clang module)
34+
35+
// Now try doing it transitively
36+
37+
// RUN: %target-swift-frontend -emit-module %t/Inputs/Library.swift -I %t/Inputs/objcfail -module-name Library -o %t
38+
39+
// RUN: %sourcekitd-test -req=interface-gen -module Library -- -I %t -target %target-triple %s | %FileCheck --check-prefix TRANSITIVE-SUCCESS %s
40+
// TRANSITIVE-SUCCESS: import ObjCFail
41+
42+
// RUN: not %sourcekitd-test -req=interface-gen -module Library -- -Xcc -DFAIL -I %t -target %target-triple %s 2>&1 | %FileCheck --check-prefix TRANSITIVE-FAIL %s
43+
// TRANSITIVE-FAIL: Could not load module: Library (could not build {{Objective-C|C}} module 'ObjCFail', some error from Clang module)
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// RUN: %empty-directory(%t/Inputs)
2+
// RUN: split-file %s %t/Inputs
3+
4+
//--- Transitive.swift
5+
6+
public func foo() {}
7+
8+
//--- Library.swift
9+
10+
import Transitive
11+
12+
//--- LibraryWrong.swift
13+
14+
import WrongName
15+
16+
//--- LibraryNonExistant.swift
17+
18+
import NonExistant
19+
20+
// RUN: %target-swift-frontend -emit-module %t/Inputs/Transitive.swift -module-name Transitive -o %t/WrongName.swiftmodule
21+
// RUN: %target-swift-frontend -emit-module %t/Inputs/Transitive.swift -module-name Transitive -o %t/Transitive.swiftmodule
22+
23+
// First try printing the interface of the Transitive module directly.
24+
25+
// RUN: %sourcekitd-test -req=interface-gen -module Transitive -- -I %t -target %target-triple %s | %FileCheck --check-prefix DIRECT-SUCCESS %s
26+
// DIRECT-SUCCESS: public func foo()
27+
28+
// RUN: not %sourcekitd-test -req=interface-gen -module WrongName -- -I %t -target %target-triple %s 2>&1 | %FileCheck --check-prefix DIRECT-FAIL %s
29+
// DIRECT-FAIL: Could not load module: WrongName (cannot load module 'Transitive' as 'WrongName')
30+
31+
// Now try doing it transitively
32+
33+
// First undo the WrongName module
34+
// RUN: %target-swift-frontend -emit-module %t/Inputs/Transitive.swift -module-name WrongName -o %t/WrongName.swiftmodule
35+
36+
// RUN: %target-swift-frontend -emit-module %t/Inputs/Library.swift -I %t -module-name Library -o %t
37+
// RUN: %target-swift-frontend -emit-module %t/Inputs/LibraryWrong.swift -I %t -module-name LibraryWrong -o %t
38+
39+
// Then redo the WrongName module
40+
// RUN: %target-swift-frontend -emit-module %t/Inputs/Transitive.swift -module-name Transitive -o %t/WrongName.swiftmodule
41+
42+
// RUN: %sourcekitd-test -req=interface-gen -module Library -- -I %t -target %target-triple %s | %FileCheck --check-prefix TRANSITIVE-SUCCESS %s
43+
// TRANSITIVE-SUCCESS: import Transitive
44+
45+
// RUN: not %sourcekitd-test -req=interface-gen -module LibraryWrong -- -I %t -target %target-triple %s 2>&1 | %FileCheck --check-prefix TRANSITIVE-FAIL %s
46+
// TRANSITIVE-FAIL: Could not load module: LibraryWrong (cannot load module 'Transitive' as 'WrongName')
47+
48+
// Try a non-existant module
49+
50+
// RUN: not %sourcekitd-test -req=interface-gen -module NonExistant -- -I %t -target %target-triple %s 2>&1 | %FileCheck --check-prefix DIRECT-NONEXISTANT %s
51+
// DIRECT-NONEXISTANT: Could not load module: NonExistant
52+
53+
// RUN: %target-swift-frontend -emit-module %t/Inputs/Transitive.swift -module-name NonExistant -o %t
54+
// RUN: %target-swift-frontend -emit-module %t/Inputs/LibraryNonExistant.swift -module-name LibraryNonExistant -I %t -o %t
55+
// RUN: rm -rf %t/NonExistant.swiftmodule
56+
57+
// RUN: not %sourcekitd-test -req=interface-gen -module LibraryNonExistant -- -I %t -target %target-triple %s 2>&1 | %FileCheck --check-prefix TRANSITIVE-NONEXISTANT %s
58+
// TRANSITIVE-NONEXISTANT: Could not load module: LibraryNonExistant (missing required module 'NonExistant')

tools/SourceKit/lib/SwiftLang/SwiftEditorInterfaceGen.cpp

Lines changed: 69 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,46 @@ static void reportSemanticAnnotations(const SourceTextInfo &IFaceInfo,
265265
}
266266
}
267267

268+
namespace {
269+
/// A diagnostic consumer that picks up module loading errors.
270+
class ModuleLoadingErrorConsumer final : public DiagnosticConsumer {
271+
llvm::SmallVector<std::string, 2> DiagMessages;
272+
273+
void handleDiagnostic(SourceManager &SM,
274+
const DiagnosticInfo &Info) override {
275+
// Only record errors for now. In the future it might be useful to pick up
276+
// some notes, but some notes are just noise.
277+
if (Info.Kind != DiagnosticKind::Error)
278+
return;
279+
280+
std::string Message;
281+
{
282+
llvm::raw_string_ostream Out(Message);
283+
DiagnosticEngine::formatDiagnosticText(Out, Info.FormatString,
284+
Info.FormatArgs);
285+
}
286+
// We're only interested in the first and last errors. For a clang module
287+
// failure, the first error will be the reason why the module failed to
288+
// load, and the last error will be a generic "could not build Obj-C module"
289+
// error. For a Swift module, we'll typically only emit one error.
290+
//
291+
// NOTE: Currently when loading transitive dependencies for a Swift module,
292+
// we'll only diagnose the root failure, and not record the error for the
293+
// top-level module failure, as we stop emitting errors after a fatal error
294+
// has been recorded. This is currently fine for our use case though, as
295+
// we already include the top-level module name in the error we hand back.
296+
if (DiagMessages.size() < 2) {
297+
DiagMessages.emplace_back(std::move(Message));
298+
} else {
299+
DiagMessages.back() = std::move(Message);
300+
}
301+
}
302+
303+
public:
304+
ArrayRef<std::string> getDiagMessages() { return DiagMessages; }
305+
};
306+
} // end anonymous namespace
307+
268308
static bool getModuleInterfaceInfo(ASTContext &Ctx,
269309
StringRef ModuleName,
270310
Optional<StringRef> Group,
@@ -280,21 +320,35 @@ static bool getModuleInterfaceInfo(ASTContext &Ctx,
280320
return true;
281321
}
282322

283-
// Get the (sub)module to generate.
284-
Mod = Ctx.getModuleByName(ModuleName);
285-
if (!Mod) {
286-
ErrMsg = "Could not load module: ";
287-
ErrMsg += ModuleName;
288-
return true;
289-
}
290-
if (Mod->failedToLoad()) {
291-
// We might fail to load the underlying Clang module
292-
// for a Swift overlay module like 'CxxStdlib', or a mixed-language
293-
// framework. Make sure an error is reported in this case, so that we can
294-
// either retry to load with C++ interoperability enabled, and if that
295-
// fails, we can report this to the user.
296-
ErrMsg = "Could not load underlying module for: ";
297-
ErrMsg += ModuleName;
323+
// Get the (sub)module to generate, recording the errors emitted.
324+
ModuleLoadingErrorConsumer DiagConsumer;
325+
{
326+
DiagnosticConsumerRAII R(Ctx.Diags, DiagConsumer);
327+
Mod = Ctx.getModuleByName(ModuleName);
328+
}
329+
330+
// Check to see if we either couldn't find the module, or we failed to load
331+
// it, and report an error message back that includes the diagnostics we
332+
// collected, which should help pinpoint what the issue was. Note we do this
333+
// even if `Mod` is null, as the clang importer currently returns nullptr
334+
// when a module fails to load, and there may be interesting errors to
335+
// collect there.
336+
// Note that us failing here also means the caller may retry with e.g C++
337+
// interoperability enabled.
338+
if (!Mod || Mod->failedToLoad()) {
339+
llvm::raw_string_ostream OS(ErrMsg);
340+
341+
OS << "Could not load module: ";
342+
OS << ModuleName;
343+
auto ModuleErrs = DiagConsumer.getDiagMessages();
344+
if (!ModuleErrs.empty()) {
345+
// We print the errors in reverse, as they are typically emitted in
346+
// a bottom-up manner by module loading, and a top-down presentation
347+
// makes more sense.
348+
OS << " (";
349+
llvm::interleaveComma(llvm::reverse(ModuleErrs), OS);
350+
OS << ")";
351+
}
298352
return true;
299353
}
300354

0 commit comments

Comments
 (0)