Skip to content

Commit e364552

Browse files
jrose-applejrose-apple
committed
[ClangImporter] Pull buffer importing out of ClangDiagnosticConsumer
Emitting Swift diagnostics in Clang buffers requires making those buffers valid places to put Swift SourceLocs, which means making a mirror of those buffers in the Swift SourceManager. This isn't a copy; instead, any Clang SourceManagers that are involved are kept alive until the importer is torn down. (There might be more than one because of diagnostics emitted during module building.) For a long time we only emitted diagnostics in Clang buffers if the diagnostics came from Clang, but then we added another case for custom Swift names that fail to import. I'm about to add another such diagnostic, so let's formalize this buffer mapping first. No intended functionality change.
1 parent 4d9e5bf commit e364552

8 files changed

+186
-104
lines changed

‎lib/ClangImporter/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ add_swift_host_library(swiftClangImporter STATIC
99
ClangAdapter.cpp
1010
ClangDiagnosticConsumer.cpp
1111
ClangImporter.cpp
12+
ClangSourceBufferImporter.cpp
1213
DWARFImporter.cpp
1314
IAMInference.cpp
1415
ImportDecl.cpp

‎lib/ClangImporter/ClangDiagnosticConsumer.cpp

Lines changed: 14 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
//===----------------------------------------------------------------------===//
1212

1313
#include "ClangDiagnosticConsumer.h"
14+
#include "ClangSourceBufferImporter.h"
1415
#include "ImporterImpl.h"
1516
#include "swift/AST/ASTContext.h"
1617
#include "swift/AST/DiagnosticEngine.h"
@@ -22,6 +23,7 @@
2223
#include "llvm/ADT/STLExtras.h"
2324

2425
using namespace swift;
26+
using namespace swift::importer;
2527

2628
namespace {
2729
class ClangDiagRenderer final : public clang::DiagnosticNoteRenderer {
@@ -109,81 +111,6 @@ ClangDiagnosticConsumer::ClangDiagnosticConsumer(
109111
: TextDiagnosticPrinter(llvm::errs(), &clangDiagOptions),
110112
ImporterImpl(impl), DumpToStderr(dumpToStderr) {}
111113

112-
static SourceLoc findEndOfLine(SourceManager &SM, SourceLoc loc,
113-
unsigned bufferID) {
114-
CharSourceRange entireBuffer = SM.getRangeForBuffer(bufferID);
115-
CharSourceRange rangeFromLoc{SM, loc, entireBuffer.getEnd()};
116-
StringRef textFromLoc = SM.extractText(rangeFromLoc);
117-
size_t newlineOffset = textFromLoc.find_first_of({"\r\n\0", 3});
118-
if (newlineOffset == StringRef::npos)
119-
return entireBuffer.getEnd();
120-
return loc.getAdvancedLoc(newlineOffset);
121-
}
122-
123-
SourceLoc ClangDiagnosticConsumer::resolveSourceLocation(
124-
const clang::SourceManager &clangSrcMgr,
125-
clang::SourceLocation clangLoc) {
126-
SourceManager &swiftSrcMgr = ImporterImpl.SwiftContext.SourceMgr;
127-
SourceLoc loc;
128-
129-
clangLoc = clangSrcMgr.getFileLoc(clangLoc);
130-
auto decomposedLoc = clangSrcMgr.getDecomposedLoc(clangLoc);
131-
if (decomposedLoc.first.isInvalid())
132-
return loc;
133-
134-
auto buffer = clangSrcMgr.getBuffer(decomposedLoc.first);
135-
unsigned mirrorID;
136-
137-
auto mirrorIter = mirroredBuffers.find(buffer);
138-
if (mirrorIter != mirroredBuffers.end()) {
139-
mirrorID = mirrorIter->second;
140-
} else {
141-
std::unique_ptr<llvm::MemoryBuffer> mirrorBuffer{
142-
llvm::MemoryBuffer::getMemBuffer(buffer->getBuffer(),
143-
buffer->getBufferIdentifier(),
144-
/*RequiresNullTerminator=*/true)
145-
};
146-
mirrorID = swiftSrcMgr.addNewSourceBuffer(std::move(mirrorBuffer));
147-
mirroredBuffers[buffer] = mirrorID;
148-
}
149-
loc = swiftSrcMgr.getLocForOffset(mirrorID, decomposedLoc.second);
150-
151-
auto presumedLoc = clangSrcMgr.getPresumedLoc(clangLoc);
152-
if (!presumedLoc.getFilename())
153-
return loc;
154-
if (presumedLoc.getLine() == 0)
155-
return SourceLoc();
156-
157-
unsigned bufferLineNumber =
158-
clangSrcMgr.getLineNumber(decomposedLoc.first, decomposedLoc.second);
159-
160-
StringRef presumedFile = presumedLoc.getFilename();
161-
SourceLoc startOfLine = loc.getAdvancedLoc(-presumedLoc.getColumn() + 1);
162-
bool isNewVirtualFile =
163-
swiftSrcMgr.openVirtualFile(startOfLine, presumedFile,
164-
presumedLoc.getLine() - bufferLineNumber);
165-
if (isNewVirtualFile) {
166-
SourceLoc endOfLine = findEndOfLine(swiftSrcMgr, loc, mirrorID);
167-
swiftSrcMgr.closeVirtualFile(endOfLine);
168-
}
169-
170-
using SourceManagerRef = llvm::IntrusiveRefCntPtr<const clang::SourceManager>;
171-
auto iter = std::lower_bound(sourceManagersWithDiagnostics.begin(),
172-
sourceManagersWithDiagnostics.end(),
173-
&clangSrcMgr,
174-
[](const SourceManagerRef &inArray,
175-
const clang::SourceManager *toInsert) {
176-
return std::less<const clang::SourceManager *>()(inArray.get(), toInsert);
177-
});
178-
if (iter == sourceManagersWithDiagnostics.end() ||
179-
iter->get() != &clangSrcMgr) {
180-
sourceManagersWithDiagnostics.insert(iter, &clangSrcMgr);
181-
}
182-
183-
return loc;
184-
}
185-
186-
187114
void ClangDiagnosticConsumer::HandleDiagnostic(
188115
clang::DiagnosticsEngine::Level clangDiagLevel,
189116
const clang::Diagnostic &clangDiag) {
@@ -195,13 +122,16 @@ void ClangDiagnosticConsumer::HandleDiagnostic(
195122
}
196123

197124
const ASTContext &ctx = ImporterImpl.SwiftContext;
125+
ClangSourceBufferImporter &bufferImporter =
126+
ImporterImpl.getBufferImporterForDiagnostics();
198127

199128
if (clangDiag.getID() == clang::diag::err_module_not_built &&
200129
CurrentImport && clangDiag.getArgStdStr(0) == CurrentImport->getName()) {
201130
SourceLoc loc = DiagLoc;
202-
if (clangDiag.getLocation().isValid())
203-
loc = resolveSourceLocation(clangDiag.getSourceManager(),
204-
clangDiag.getLocation());
131+
if (clangDiag.getLocation().isValid()) {
132+
loc = bufferImporter.resolveSourceLocation(clangDiag.getSourceManager(),
133+
clangDiag.getLocation());
134+
}
205135

206136
ctx.Diags.diagnose(loc, diag::clang_cannot_build_module,
207137
ctx.LangOpts.EnableObjCInterop,
@@ -216,9 +146,10 @@ void ClangDiagnosticConsumer::HandleDiagnostic(
216146
DiagnosticConsumer::HandleDiagnostic(clangDiagLevel, clangDiag);
217147

218148
// FIXME: Map over source ranges in the diagnostic.
219-
auto emitDiag = [&ctx, this](clang::FullSourceLoc clangNoteLoc,
220-
clang::DiagnosticsEngine::Level clangDiagLevel,
221-
StringRef message) {
149+
auto emitDiag =
150+
[&ctx, &bufferImporter](clang::FullSourceLoc clangNoteLoc,
151+
clang::DiagnosticsEngine::Level clangDiagLevel,
152+
StringRef message) {
222153
decltype(diag::error_from_clang) diagKind;
223154
switch (clangDiagLevel) {
224155
case clang::DiagnosticsEngine::Ignored:
@@ -241,8 +172,8 @@ void ClangDiagnosticConsumer::HandleDiagnostic(
241172

242173
SourceLoc noteLoc;
243174
if (clangNoteLoc.isValid())
244-
noteLoc = resolveSourceLocation(clangNoteLoc.getManager(),
245-
clangNoteLoc);
175+
noteLoc = bufferImporter.resolveSourceLocation(clangNoteLoc.getManager(),
176+
clangNoteLoc);
246177
ctx.Diags.diagnose(noteLoc, diagKind, message);
247178
};
248179

‎lib/ClangImporter/ClangDiagnosticConsumer.h

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -55,18 +55,6 @@ class ClangDiagnosticConsumer : public clang::TextDiagnosticPrinter {
5555

5656
ClangImporter::Implementation &ImporterImpl;
5757

58-
/// Keeps alive the Clang source managers where diagnostics have been
59-
/// reported.
60-
///
61-
/// This is a bit of a hack, but LLVM's source manager (and by extension
62-
/// Swift's) does not support buffers going away.
63-
//
64-
// This is not using SmallPtrSet or similar because we need the
65-
// IntrusiveRefCntPtr to stay a ref-counting pointer.
66-
SmallVector<llvm::IntrusiveRefCntPtr<const clang::SourceManager>, 4>
67-
sourceManagersWithDiagnostics;
68-
llvm::DenseMap<const llvm::MemoryBuffer *, unsigned> mirroredBuffers;
69-
7058
const clang::IdentifierInfo *CurrentImport = nullptr;
7159
SourceLoc DiagLoc;
7260
const bool DumpToStderr;
@@ -82,12 +70,6 @@ class ClangDiagnosticConsumer : public clang::TextDiagnosticPrinter {
8270
return LoadModuleRAII(*this, name);
8371
}
8472

85-
/// Returns a Swift source location that points into a Clang buffer.
86-
///
87-
/// This will keep the Clang buffer alive as long as this diagnostic consumer.
88-
SourceLoc resolveSourceLocation(const clang::SourceManager &clangSrcMgr,
89-
clang::SourceLocation clangLoc);
90-
9173
void HandleDiagnostic(clang::DiagnosticsEngine::Level diagLevel,
9274
const clang::Diagnostic &info) override;
9375
};

‎lib/ClangImporter/ClangImporter.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1844,6 +1844,7 @@ ClangImporter::Implementation::Implementation(
18441844
ASTContext &ctx, const ClangImporterOptions &opts,
18451845
DWARFImporterDelegate *dwarfImporterDelegate)
18461846
: SwiftContext(ctx),
1847+
BuffersForDiagnostics(ctx.SourceMgr),
18471848
ImportForwardDeclarations(opts.ImportForwardDeclarations),
18481849
InferImportAsMember(opts.InferImportAsMember),
18491850
DisableSwiftBridgeAttr(opts.DisableSwiftBridgeAttr),

‎lib/ClangImporter/ClangSourceBufferImporter.cpp

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
//===--- ClangSourceBufferImporter.cpp - Map Clang buffers to Swift -------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
#import "ClangSourceBufferImporter.h"
14+
#import "swift/Basic/SourceManager.h"
15+
#import "clang/Basic/SourceManager.h"
16+
#import "llvm/Support/MemoryBuffer.h"
17+
18+
using namespace swift;
19+
using namespace swift::importer;
20+
21+
static SourceLoc findEndOfLine(SourceManager &SM, SourceLoc loc,
22+
unsigned bufferID) {
23+
CharSourceRange entireBuffer = SM.getRangeForBuffer(bufferID);
24+
CharSourceRange rangeFromLoc{SM, loc, entireBuffer.getEnd()};
25+
StringRef textFromLoc = SM.extractText(rangeFromLoc);
26+
size_t newlineOffset = textFromLoc.find_first_of({"\r\n\0", 3});
27+
if (newlineOffset == StringRef::npos)
28+
return entireBuffer.getEnd();
29+
return loc.getAdvancedLoc(newlineOffset);
30+
}
31+
32+
SourceLoc ClangSourceBufferImporter::resolveSourceLocation(
33+
const clang::SourceManager &clangSrcMgr,
34+
clang::SourceLocation clangLoc) {
35+
SourceLoc loc;
36+
37+
clangLoc = clangSrcMgr.getFileLoc(clangLoc);
38+
auto decomposedLoc = clangSrcMgr.getDecomposedLoc(clangLoc);
39+
if (decomposedLoc.first.isInvalid())
40+
return loc;
41+
42+
auto buffer = clangSrcMgr.getBuffer(decomposedLoc.first);
43+
unsigned mirrorID;
44+
45+
auto mirrorIter = mirroredBuffers.find(buffer);
46+
if (mirrorIter != mirroredBuffers.end()) {
47+
mirrorID = mirrorIter->second;
48+
} else {
49+
std::unique_ptr<llvm::MemoryBuffer> mirrorBuffer{
50+
llvm::MemoryBuffer::getMemBuffer(buffer->getBuffer(),
51+
buffer->getBufferIdentifier(),
52+
/*RequiresNullTerminator=*/true)
53+
};
54+
mirrorID = swiftSourceManager.addNewSourceBuffer(std::move(mirrorBuffer));
55+
mirroredBuffers[buffer] = mirrorID;
56+
}
57+
loc = swiftSourceManager.getLocForOffset(mirrorID, decomposedLoc.second);
58+
59+
auto presumedLoc = clangSrcMgr.getPresumedLoc(clangLoc);
60+
if (!presumedLoc.getFilename())
61+
return loc;
62+
if (presumedLoc.getLine() == 0)
63+
return SourceLoc();
64+
65+
unsigned bufferLineNumber =
66+
clangSrcMgr.getLineNumber(decomposedLoc.first, decomposedLoc.second);
67+
68+
StringRef presumedFile = presumedLoc.getFilename();
69+
SourceLoc startOfLine = loc.getAdvancedLoc(-presumedLoc.getColumn() + 1);
70+
bool isNewVirtualFile = swiftSourceManager.openVirtualFile(
71+
startOfLine, presumedFile, presumedLoc.getLine() - bufferLineNumber);
72+
if (isNewVirtualFile) {
73+
SourceLoc endOfLine = findEndOfLine(swiftSourceManager, loc, mirrorID);
74+
swiftSourceManager.closeVirtualFile(endOfLine);
75+
}
76+
77+
using SourceManagerRef = llvm::IntrusiveRefCntPtr<const clang::SourceManager>;
78+
auto iter = std::lower_bound(sourceManagersWithDiagnostics.begin(),
79+
sourceManagersWithDiagnostics.end(),
80+
&clangSrcMgr,
81+
[](const SourceManagerRef &inArray,
82+
const clang::SourceManager *toInsert) {
83+
return std::less<const clang::SourceManager *>()(inArray.get(), toInsert);
84+
});
85+
if (iter == sourceManagersWithDiagnostics.end() ||
86+
iter->get() != &clangSrcMgr) {
87+
sourceManagersWithDiagnostics.insert(iter, &clangSrcMgr);
88+
}
89+
90+
return loc;
91+
}

‎lib/ClangImporter/ClangSourceBufferImporter.h

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
//===--- ClangSourceBufferImporter.h - Map Clang buffers over ---*- C++ -*-===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
#ifndef SWIFT_CLANGIMPORTER_CLANGSOURCEBUFFERIMPORTER_H
14+
#define SWIFT_CLANGIMPORTER_CLANGSOURCEBUFFERIMPORTER_H
15+
16+
#include "swift/Basic/LLVM.h"
17+
#include "swift/Basic/SourceLoc.h"
18+
#include "clang/Basic/SourceLocation.h"
19+
#include "llvm/ADT/DenseMap.h"
20+
#include "llvm/ADT/IntrusiveRefCntPtr.h"
21+
#include "llvm/ADT/SmallVector.h"
22+
23+
namespace llvm {
24+
class MemoryBuffer;
25+
}
26+
27+
namespace clang {
28+
class SourceManager;
29+
}
30+
31+
namespace swift {
32+
class SourceManager;
33+
34+
namespace importer {
35+
36+
/// A helper class used to keep alive the Clang source managers where
37+
/// diagnostics have been reported.
38+
///
39+
/// This is a bit of a hack, but LLVM's source manager (and by extension
40+
/// Swift's) does not support buffers going away, so if we want to report
41+
/// diagnostics in them we have to do it this way.
42+
class ClangSourceBufferImporter {
43+
// This is not using SmallPtrSet or similar because we need the
44+
// IntrusiveRefCntPtr to stay a ref-counting pointer.
45+
SmallVector<llvm::IntrusiveRefCntPtr<const clang::SourceManager>, 4>
46+
sourceManagersWithDiagnostics;
47+
llvm::DenseMap<const llvm::MemoryBuffer *, unsigned> mirroredBuffers;
48+
SourceManager &swiftSourceManager;
49+
50+
public:
51+
explicit ClangSourceBufferImporter(SourceManager &sourceMgr)
52+
: swiftSourceManager(sourceMgr) {}
53+
54+
/// Returns a Swift source location that points into a Clang buffer.
55+
///
56+
/// This will keep the Clang buffer alive as long as this object.
57+
SourceLoc resolveSourceLocation(const clang::SourceManager &clangSrcMgr,
58+
clang::SourceLocation clangLoc);
59+
};
60+
61+
} // end namespace importer
62+
} // end namespace swift
63+
64+
#endif

‎lib/ClangImporter/ImportType.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2194,10 +2194,10 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
21942194
if (importedName.hasCustomName() && argNames.size() != swiftParams.size()) {
21952195
// Note carefully: we're emitting a warning in the /Clang/ buffer.
21962196
auto &srcMgr = getClangASTContext().getSourceManager();
2197-
auto &rawDiagClient = Instance->getDiagnosticClient();
2198-
auto &diagClient = static_cast<ClangDiagnosticConsumer &>(rawDiagClient);
2197+
ClangSourceBufferImporter &bufferImporter =
2198+
getBufferImporterForDiagnostics();
21992199
SourceLoc methodLoc =
2200-
diagClient.resolveSourceLocation(srcMgr, clangDecl->getLocation());
2200+
bufferImporter.resolveSourceLocation(srcMgr, clangDecl->getLocation());
22012201
if (methodLoc.isValid()) {
22022202
SwiftContext.Diags.diagnose(methodLoc, diag::invalid_swift_name_method,
22032203
swiftParams.size() < argNames.size(),

‎lib/ClangImporter/ImporterImpl.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#define SWIFT_CLANG_IMPORTER_IMPL_H
1919

2020
#include "ClangAdapter.h"
21+
#include "ClangSourceBufferImporter.h"
2122
#include "ImportEnumInfo.h"
2223
#include "ImportName.h"
2324
#include "SwiftLookupTable.h"
@@ -345,6 +346,13 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
345346
/// (through the Swift name lookup module file extension).
346347
LookupTableMap LookupTables;
347348

349+
/// A helper class used to bring Clang buffers into Swift's SourceManager
350+
/// for the purpose of emitting diagnostics.
351+
///
352+
/// Listed early so that it gets torn down after the underlying Clang
353+
/// instances that also use these buffers.
354+
importer::ClangSourceBufferImporter BuffersForDiagnostics;
355+
348356
/// The fake buffer used to import modules.
349357
///
350358
/// \see getNextIncludeLoc
@@ -651,6 +659,10 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
651659
return Instance->getCodeGenOpts();
652660
}
653661

662+
importer::ClangSourceBufferImporter &getBufferImporterForDiagnostics() {
663+
return BuffersForDiagnostics;
664+
}
665+
654666
/// Imports the given header contents into the Clang context.
655667
bool importHeader(ModuleDecl *adapter, StringRef headerName,
656668
SourceLoc diagLoc, bool trackParsedSymbols,

0 commit comments

Comments
 (0)