Skip to content

Commit cf5d81c

Browse files
author
David Ungar
authored
Merge pull request swiftlang#23735 from davidungar/tracking-primary
[Batch mode] Cope with bugs that cause error suppression.
2 parents 9a0d6e8 + 6d79bed commit cf5d81c

26 files changed

+352
-220
lines changed

‎include/swift/AST/DiagnosticConsumer.h‎

Lines changed: 42 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ class DiagnosticConsumer {
7979

8080
public:
8181
virtual ~DiagnosticConsumer();
82-
82+
8383
/// Invoked whenever the frontend emits a diagnostic.
8484
///
8585
/// \param SM The source manager associated with the source locations in
@@ -94,11 +94,20 @@ class DiagnosticConsumer {
9494
/// \param FormatArgs The diagnostic format string arguments.
9595
///
9696
/// \param Info Extra information associated with the diagnostic.
97-
virtual void handleDiagnostic(SourceManager &SM, SourceLoc Loc,
98-
DiagnosticKind Kind,
99-
StringRef FormatString,
100-
ArrayRef<DiagnosticArgument> FormatArgs,
101-
const DiagnosticInfo &Info) = 0;
97+
///
98+
/// \param bufferIndirectlyCausingDiagnostic Only used when directing
99+
/// diagnostics to different outputs.
100+
/// In batch mode a diagnostic may be
101+
/// located in a non-primary file, but there will be no .dia file for a
102+
/// non-primary. If valid, this argument contains a location within a buffer
103+
/// that corresponds to a primary input. The .dia file for that primary can be
104+
/// used for the diagnostic, as if it had occurred at this location.
105+
virtual void
106+
handleDiagnostic(SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
107+
StringRef FormatString,
108+
ArrayRef<DiagnosticArgument> FormatArgs,
109+
const DiagnosticInfo &Info,
110+
SourceLoc bufferIndirectlyCausingDiagnostic) = 0;
102111

103112
/// \returns true if an error occurred while finishing-up.
104113
virtual bool finishProcessing() { return false; }
@@ -116,11 +125,11 @@ class DiagnosticConsumer {
116125
/// DiagnosticConsumer that discards all diagnostics.
117126
class NullDiagnosticConsumer : public DiagnosticConsumer {
118127
public:
119-
void handleDiagnostic(SourceManager &SM, SourceLoc Loc,
120-
DiagnosticKind Kind,
128+
void handleDiagnostic(SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
121129
StringRef FormatString,
122130
ArrayRef<DiagnosticArgument> FormatArgs,
123-
const DiagnosticInfo &Info) override;
131+
const DiagnosticInfo &Info,
132+
SourceLoc bufferIndirectlyCausingDiagnostic) override;
124133
};
125134

126135
/// DiagnosticConsumer that forwards diagnostics to the consumers of
@@ -129,11 +138,11 @@ class ForwardingDiagnosticConsumer : public DiagnosticConsumer {
129138
DiagnosticEngine &TargetEngine;
130139
public:
131140
ForwardingDiagnosticConsumer(DiagnosticEngine &Target);
132-
void handleDiagnostic(SourceManager &SM, SourceLoc Loc,
133-
DiagnosticKind Kind,
141+
void handleDiagnostic(SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
134142
StringRef FormatString,
135143
ArrayRef<DiagnosticArgument> FormatArgs,
136-
const DiagnosticInfo &Info) override;
144+
const DiagnosticInfo &Info,
145+
SourceLoc bufferIndirectlyCausingDiagnostic) override;
137146
};
138147

139148
/// DiagnosticConsumer that funnels diagnostics in certain files to
@@ -175,8 +184,12 @@ class FileSpecificDiagnosticConsumer : public DiagnosticConsumer {
175184
std::string inputFileName;
176185

177186
/// The consumer (if any) for diagnostics associated with the inputFileName.
178-
/// A null pointer for the DiagnosticConsumer means that diagnostics for
179-
/// this file should not be emitted.
187+
/// A null pointer for the DiagnosticConsumer means that this file is a
188+
/// non-primary one in batch mode and we have no .dia file for it.
189+
/// If there is a responsible primary when the diagnostic is handled
190+
/// it will be shunted to that primary's .dia file.
191+
/// Otherwise it will be suppressed, assuming that the diagnostic will
192+
/// surface in another frontend job that compiles that file as a primary.
180193
std::unique_ptr<DiagnosticConsumer> consumer;
181194

182195
// Has this subconsumer ever handled a diagnostic that is an error?
@@ -191,16 +204,16 @@ class FileSpecificDiagnosticConsumer : public DiagnosticConsumer {
191204
std::unique_ptr<DiagnosticConsumer> consumer)
192205
: inputFileName(inputFileName), consumer(std::move(consumer)) {}
193206

194-
void handleDiagnostic(SourceManager &SM, SourceLoc Loc,
195-
DiagnosticKind Kind,
207+
void handleDiagnostic(SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
196208
StringRef FormatString,
197209
ArrayRef<DiagnosticArgument> FormatArgs,
198-
const DiagnosticInfo &Info) {
210+
const DiagnosticInfo &Info,
211+
const SourceLoc bufferIndirectlyCausingDiagnostic) {
199212
if (!getConsumer())
200213
return;
201214
hasAnErrorBeenConsumed |= Kind == DiagnosticKind::Error;
202215
getConsumer()->handleDiagnostic(SM, Loc, Kind, FormatString, FormatArgs,
203-
Info);
216+
Info, bufferIndirectlyCausingDiagnostic);
204217
}
205218

206219
void informDriverOfIncompleteBatchModeCompilation() {
@@ -287,11 +300,11 @@ class FileSpecificDiagnosticConsumer : public DiagnosticConsumer {
287300
SmallVectorImpl<Subconsumer> &consumers);
288301

289302
public:
290-
void handleDiagnostic(SourceManager &SM, SourceLoc Loc,
291-
DiagnosticKind Kind,
303+
void handleDiagnostic(SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
292304
StringRef FormatString,
293305
ArrayRef<DiagnosticArgument> FormatArgs,
294-
const DiagnosticInfo &Info) override;
306+
const DiagnosticInfo &Info,
307+
SourceLoc bufferIndirectlyCausingDiagnostic) override;
295308

296309
bool finishProcessing() override;
297310

@@ -309,6 +322,14 @@ class FileSpecificDiagnosticConsumer : public DiagnosticConsumer {
309322
/// a particular consumer if diagnostic goes there.
310323
Optional<FileSpecificDiagnosticConsumer::Subconsumer *>
311324
subconsumerForLocation(SourceManager &SM, SourceLoc loc);
325+
326+
Optional<FileSpecificDiagnosticConsumer::Subconsumer *>
327+
findSubconsumer(SourceManager &SM, SourceLoc loc, DiagnosticKind Kind,
328+
SourceLoc bufferIndirectlyCausingDiagnostic);
329+
330+
Optional<FileSpecificDiagnosticConsumer::Subconsumer *>
331+
findSubconsumerForNonNote(SourceManager &SM, SourceLoc loc,
332+
SourceLoc bufferIndirectlyCausingDiagnostic);
312333
};
313334

314335
} // end namespace swift

‎include/swift/AST/DiagnosticEngine.h‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ namespace swift {
2828
class DiagnosticEngine;
2929
class SourceManager;
3030
class ValueDecl;
31-
31+
class SourceFile;
32+
3233
enum class PatternKind : uint8_t;
3334
enum class SelfAccessKind : uint8_t;
3435
enum class ReferenceOwnership : uint8_t;
@@ -561,6 +562,12 @@ namespace swift {
561562
/// emitted once all transactions have closed.
562563
unsigned TransactionCount = 0;
563564

565+
/// For batch mode, use this to know where to output a diagnostic from a
566+
/// non-primary file. It's any location in the buffer of the current primary
567+
/// input being compiled.
568+
/// May be invalid.
569+
SourceLoc bufferIndirectlyCausingDiagnostic;
570+
564571
friend class InFlightDiagnostic;
565572
friend class DiagnosticTransaction;
566573

@@ -799,6 +806,27 @@ namespace swift {
799806

800807
public:
801808
static const char *diagnosticStringFor(const DiagID id);
809+
810+
/// If there is no clear .dia file for a diagnostic, put it in the one
811+
/// corresponding to the SourceLoc given here.
812+
/// In particular, in batch mode when a diagnostic is located in
813+
/// a non-primary file, use this affordance to place it in the .dia
814+
/// file for the primary that is currently being worked on.
815+
void setBufferIndirectlyCausingDiagnosticToInput(SourceLoc);
816+
void resetBufferIndirectlyCausingDiagnostic();
817+
SourceLoc getDefaultDiagnosticLoc() const {
818+
return bufferIndirectlyCausingDiagnostic;
819+
}
820+
};
821+
822+
class BufferIndirectlyCausingDiagnosticRAII {
823+
private:
824+
DiagnosticEngine &Diags;
825+
public:
826+
BufferIndirectlyCausingDiagnosticRAII(const SourceFile &SF);
827+
~BufferIndirectlyCausingDiagnosticRAII() {
828+
Diags.resetBufferIndirectlyCausingDiagnostic();
829+
}
802830
};
803831

804832
/// Represents a diagnostic transaction. While a transaction is

‎include/swift/Frontend/PrintingDiagnosticConsumer.h‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,12 @@ class PrintingDiagnosticConsumer : public DiagnosticConsumer {
3535
PrintingDiagnosticConsumer(llvm::raw_ostream &stream = llvm::errs()) :
3636
Stream(stream) { }
3737

38-
virtual void handleDiagnostic(SourceManager &SM, SourceLoc Loc,
39-
DiagnosticKind Kind,
40-
StringRef FormatString,
41-
ArrayRef<DiagnosticArgument> FormatArgs,
42-
const DiagnosticInfo &Info) override;
38+
virtual void
39+
handleDiagnostic(SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
40+
StringRef FormatString,
41+
ArrayRef<DiagnosticArgument> FormatArgs,
42+
const DiagnosticInfo &Info,
43+
SourceLoc bufferIndirectlyCausingDiagnostic) override;
4344

4445
void forceColors() {
4546
ForceColors = true;

‎include/swift/Migrator/FixitApplyDiagnosticConsumer.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,11 @@ class FixitApplyDiagnosticConsumer final
6262
/// output stream.
6363
void printResult(llvm::raw_ostream &OS) const;
6464

65-
void handleDiagnostic(SourceManager &SM, SourceLoc Loc,
66-
DiagnosticKind Kind,
65+
void handleDiagnostic(SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
6766
StringRef FormatString,
6867
ArrayRef<DiagnosticArgument> FormatArgs,
69-
const DiagnosticInfo &Info) override;
68+
const DiagnosticInfo &Info,
69+
SourceLoc bufferIndirectlyCausingDiagnostic) override;
7070

7171
unsigned getNumFixitsApplied() const {
7272
return NumFixitsApplied;

‎lib/AST/DiagnosticConsumer.cpp‎

Lines changed: 58 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,12 @@ FileSpecificDiagnosticConsumer::subconsumerForLocation(SourceManager &SM,
118118
if (loc.isInvalid())
119119
return None;
120120

121+
// What if a there's a FileSpecificDiagnosticConsumer but there are no
122+
// subconsumers in it? (This situation occurs for the fix-its
123+
// FileSpecificDiagnosticConsumer.) In such a case, bail out now.
124+
if (Subconsumers.empty())
125+
return None;
126+
121127
// This map is generated on first use and cached, to allow the
122128
// FileSpecificDiagnosticConsumer to be set up before the source files are
123129
// actually loaded.
@@ -165,29 +171,62 @@ FileSpecificDiagnosticConsumer::subconsumerForLocation(SourceManager &SM,
165171
void FileSpecificDiagnosticConsumer::handleDiagnostic(
166172
SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
167173
StringRef FormatString, ArrayRef<DiagnosticArgument> FormatArgs,
168-
const DiagnosticInfo &Info) {
174+
const DiagnosticInfo &Info,
175+
const SourceLoc bufferIndirectlyCausingDiagnostic) {
169176

170177
HasAnErrorBeenConsumed |= Kind == DiagnosticKind::Error;
171178

172-
Optional<FileSpecificDiagnosticConsumer::Subconsumer *> subconsumer;
179+
auto subconsumer =
180+
findSubconsumer(SM, Loc, Kind, bufferIndirectlyCausingDiagnostic);
181+
if (subconsumer) {
182+
subconsumer.getValue()->handleDiagnostic(SM, Loc, Kind, FormatString,
183+
FormatArgs, Info,
184+
bufferIndirectlyCausingDiagnostic);
185+
return;
186+
}
187+
// Last resort: spray it everywhere
188+
for (auto &subconsumer : Subconsumers)
189+
subconsumer.handleDiagnostic(SM, Loc, Kind, FormatString, FormatArgs, Info,
190+
bufferIndirectlyCausingDiagnostic);
191+
}
192+
193+
Optional<FileSpecificDiagnosticConsumer::Subconsumer *>
194+
FileSpecificDiagnosticConsumer::findSubconsumer(
195+
SourceManager &SM, SourceLoc loc, DiagnosticKind Kind,
196+
SourceLoc bufferIndirectlyCausingDiagnostic) {
197+
// Ensure that a note goes to the same place as the preceeding non-note.
173198
switch (Kind) {
174199
case DiagnosticKind::Error:
175200
case DiagnosticKind::Warning:
176-
case DiagnosticKind::Remark:
177-
subconsumer = subconsumerForLocation(SM, Loc);
201+
case DiagnosticKind::Remark: {
202+
auto subconsumer =
203+
findSubconsumerForNonNote(SM, loc, bufferIndirectlyCausingDiagnostic);
178204
SubconsumerForSubsequentNotes = subconsumer;
179-
break;
180-
case DiagnosticKind::Note:
181-
subconsumer = SubconsumerForSubsequentNotes;
182-
break;
205+
return subconsumer;
183206
}
184-
if (subconsumer.hasValue()) {
185-
subconsumer.getValue()->handleDiagnostic(SM, Loc, Kind, FormatString,
186-
FormatArgs, Info);
187-
return;
207+
case DiagnosticKind::Note:
208+
return SubconsumerForSubsequentNotes;
188209
}
189-
for (auto &subconsumer : Subconsumers)
190-
subconsumer.handleDiagnostic(SM, Loc, Kind, FormatString, FormatArgs, Info);
210+
}
211+
212+
Optional<FileSpecificDiagnosticConsumer::Subconsumer *>
213+
FileSpecificDiagnosticConsumer::findSubconsumerForNonNote(
214+
SourceManager &SM, const SourceLoc loc,
215+
const SourceLoc bufferIndirectlyCausingDiagnostic) {
216+
const auto subconsumer = subconsumerForLocation(SM, loc);
217+
if (!subconsumer)
218+
return None; // No place to put it; might be in an imported module
219+
if ((*subconsumer)->getConsumer())
220+
return subconsumer; // A primary file with a .dia file
221+
// Try to put it in the responsible primary input
222+
if (bufferIndirectlyCausingDiagnostic.isInvalid())
223+
return None;
224+
const auto currentPrimarySubconsumer =
225+
subconsumerForLocation(SM, bufferIndirectlyCausingDiagnostic);
226+
assert(!currentPrimarySubconsumer ||
227+
(*currentPrimarySubconsumer)->getConsumer() &&
228+
"current primary must have a .dia file");
229+
return currentPrimarySubconsumer;
191230
}
192231

193232
bool FileSpecificDiagnosticConsumer::finishProcessing() {
@@ -214,7 +253,7 @@ void FileSpecificDiagnosticConsumer::
214253
void NullDiagnosticConsumer::handleDiagnostic(
215254
SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
216255
StringRef FormatString, ArrayRef<DiagnosticArgument> FormatArgs,
217-
const DiagnosticInfo &Info) {
256+
const DiagnosticInfo &Info, const SourceLoc) {
218257
LLVM_DEBUG({
219258
llvm::dbgs() << "NullDiagnosticConsumer received diagnostic: ";
220259
DiagnosticEngine::formatDiagnosticText(llvm::dbgs(), FormatString,
@@ -229,14 +268,16 @@ ForwardingDiagnosticConsumer::ForwardingDiagnosticConsumer(DiagnosticEngine &Tar
229268
void ForwardingDiagnosticConsumer::handleDiagnostic(
230269
SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
231270
StringRef FormatString, ArrayRef<DiagnosticArgument> FormatArgs,
232-
const DiagnosticInfo &Info) {
271+
const DiagnosticInfo &Info,
272+
const SourceLoc bufferIndirectlyCausingDiagnostic) {
233273
LLVM_DEBUG({
234274
llvm::dbgs() << "ForwardingDiagnosticConsumer received diagnostic: ";
235275
DiagnosticEngine::formatDiagnosticText(llvm::dbgs(), FormatString,
236276
FormatArgs);
237277
llvm::dbgs() << "\n";
238278
});
239279
for (auto *C : TargetEngine.getConsumers()) {
240-
C->handleDiagnostic(SM, Loc, Kind, FormatString, FormatArgs, Info);
280+
C->handleDiagnostic(SM, Loc, Kind, FormatString, FormatArgs, Info,
281+
bufferIndirectlyCausingDiagnostic);
241282
}
242283
}

‎lib/AST/DiagnosticEngine.cpp‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -838,14 +838,31 @@ void DiagnosticEngine::emitDiagnostic(const Diagnostic &diagnostic) {
838838
for (auto &Consumer : Consumers) {
839839
Consumer->handleDiagnostic(SourceMgr, loc, toDiagnosticKind(behavior),
840840
diagnosticStringFor(Info.ID),
841-
diagnostic.getArgs(), Info);
841+
diagnostic.getArgs(), Info,
842+
getDefaultDiagnosticLoc());
842843
}
843844
}
844845

845846
const char *DiagnosticEngine::diagnosticStringFor(const DiagID id) {
846847
return diagnosticStrings[(unsigned)id];
847848
}
848849

850+
void DiagnosticEngine::setBufferIndirectlyCausingDiagnosticToInput(
851+
SourceLoc loc) {
852+
// If in the future, nested BufferIndirectlyCausingDiagnosticRAII need be
853+
// supported, the compiler will need a stack for
854+
// bufferIndirectlyCausingDiagnostic.
855+
assert(bufferIndirectlyCausingDiagnostic.isInvalid() &&
856+
"Buffer should not already be set.");
857+
bufferIndirectlyCausingDiagnostic = loc;
858+
assert(bufferIndirectlyCausingDiagnostic.isValid() &&
859+
"Buffer must be valid for previous assertion to work.");
860+
}
861+
862+
void DiagnosticEngine::resetBufferIndirectlyCausingDiagnostic() {
863+
bufferIndirectlyCausingDiagnostic = SourceLoc();
864+
}
865+
849866
DiagnosticSuppression::DiagnosticSuppression(DiagnosticEngine &diags)
850867
: diags(diags)
851868
{
@@ -856,3 +873,14 @@ DiagnosticSuppression::~DiagnosticSuppression() {
856873
for (auto consumer : consumers)
857874
diags.addConsumer(*consumer);
858875
}
876+
877+
BufferIndirectlyCausingDiagnosticRAII::BufferIndirectlyCausingDiagnosticRAII(
878+
const SourceFile &SF)
879+
: Diags(SF.getASTContext().Diags) {
880+
auto id = SF.getBufferID();
881+
if (!id)
882+
return;
883+
auto loc = SF.getASTContext().SourceMgr.getLocForBufferStart(*id);
884+
if (loc.isValid())
885+
Diags.setBufferIndirectlyCausingDiagnosticToInput(loc);
886+
}

‎lib/Frontend/PrintingDiagnosticConsumer.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ namespace {
6666
void PrintingDiagnosticConsumer::handleDiagnostic(
6767
SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
6868
StringRef FormatString, ArrayRef<DiagnosticArgument> FormatArgs,
69-
const DiagnosticInfo &Info) {
69+
const DiagnosticInfo &Info,
70+
const SourceLoc bufferIndirectlyCausingDiagnostic) {
7071
// Determine what kind of diagnostic we're emitting.
7172
llvm::SourceMgr::DiagKind SMKind;
7273
switch (Kind) {

0 commit comments

Comments
 (0)