Skip to content

Commit 746128c

Browse files
committed
Use StringMap in EditorDiagConsumer
As noted by the comment, storing a `StringRef` is indeed brittle. Update to use a StringMap. rdar://111589090
1 parent 2d37e5b commit 746128c

File tree

4 files changed

+11
-12
lines changed

4 files changed

+11
-12
lines changed

include/swift/AST/DiagnosticConsumer.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,9 @@ class DiagnosticConsumer {
122122
/// Invoked whenever the frontend emits a diagnostic.
123123
///
124124
/// \param SM The source manager associated with the source locations in
125-
/// this diagnostic.
125+
/// this diagnostic. NOTE: Do not persist either the SourceManager, or the
126+
/// buffer names from the SourceManager, since it may not outlive the
127+
/// DiagnosticConsumer (this is the case when building module interfaces).
126128
///
127129
/// \param Info Information describing the diagnostic.
128130
virtual void handleDiagnostic(SourceManager &SM,

lib/Frontend/SerializedDiagnosticConsumer.cpp

+6-5
Original file line numberDiff line numberDiff line change
@@ -228,11 +228,12 @@ namespace serialized_diagnostics {
228228
unsigned SerializedDiagnosticConsumer::getEmitFile(
229229
SourceManager &SM, StringRef Filename, unsigned bufferID
230230
) {
231-
// NOTE: Using Filename.data() here relies on SourceMgr using
232-
// const char* as buffer identifiers. This is fast, but may
233-
// be brittle. We can always switch over to using a StringMap.
234-
// Note that the logic in EditorDiagConsumer::getBufferInfo
235-
// will also need changing.
231+
// FIXME: Using Filename.data() here is wrong, since the provided
232+
// SourceManager may not live as long as this consumer (which is
233+
// the case if it's a diagnostic produced from building a module
234+
// interface). We ought to switch over to using a StringMap once
235+
// buffer names are unique (currently not the case for
236+
// pretty-printed decl buffers).
236237
unsigned &existingEntry = State->Files[Filename.data()];
237238
if (existingEntry)
238239
return existingEntry;

tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp

-4
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,6 @@ BufferInfoSharedPtr
9191
EditorDiagConsumer::getBufferInfo(StringRef FileName,
9292
llvm::Optional<unsigned> BufferID,
9393
swift::SourceManager &SM) {
94-
// NOTE: Using StringRef as a key here relies on SourceMgr using const char*
95-
// as buffer identifiers. This is fast, but may be brittle. We can always
96-
// switch over to using a StringMap. Note that the logic in
97-
// SerializedDiagnosticConsumer::getEmitFile will also need changing.
9894
auto Result = BufferInfos.find(FileName);
9995
if (Result != BufferInfos.end())
10096
return Result->second;

tools/SourceKit/lib/SwiftLang/SwiftEditorDiagConsumer.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
#include "SourceKit/Core/LangSupport.h"
1717
#include "swift/AST/DiagnosticConsumer.h"
1818
#include "llvm/ADT/DenseMap.h"
19-
#include "llvm/ADT/MapVector.h"
19+
#include "llvm/ADT/StringMap.h"
2020

2121
namespace SourceKit {
2222

@@ -27,7 +27,7 @@ class EditorDiagConsumer : public swift::DiagnosticConsumer {
2727
llvm::DenseMap<unsigned, DiagnosticsTy> BufferDiagnostics;
2828
DiagnosticsTy InvalidLocDiagnostics;
2929

30-
llvm::MapVector<StringRef, BufferInfoSharedPtr> BufferInfos;
30+
llvm::StringMap<BufferInfoSharedPtr> BufferInfos;
3131

3232
int LastDiagBufferID = -1;
3333
unsigned LastDiagIndex = 0;

0 commit comments

Comments
 (0)