Skip to content

Commit 5c2185f

Browse files
jrose-applejrose-apple
authored
Merge pull request #27868 from owenv/diag-info-refactor
[Diagnostics] Refactor DiagnosticConsumer interface to remove unnecessary params
2 parents 4980bc7 + 8a67117 commit 5c2185f

22 files changed

+316
-416
lines changed

‎include/swift/AST/DiagnosticConsumer.h

Lines changed: 19 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,13 @@ struct DiagnosticInfo {
4545
DiagnosticKind Kind;
4646
StringRef FormatString;
4747
ArrayRef<DiagnosticArgument> FormatArgs;
48+
49+
/// Only used when directing diagnostics to different outputs.
50+
/// In batch mode a diagnostic may be
51+
/// located in a non-primary file, but there will be no .dia file for a
52+
/// non-primary. If valid, this argument contains a location within a buffer
53+
/// that corresponds to a primary input. The .dia file for that primary can be
54+
/// used for the diagnostic, as if it had occurred at this location.
4855
SourceLoc BufferIndirectlyCausingDiagnostic;
4956

5057
/// DiagnosticInfo of notes which are children of this diagnostic, if any
@@ -109,29 +116,9 @@ class DiagnosticConsumer {
109116
/// \param SM The source manager associated with the source locations in
110117
/// this diagnostic.
111118
///
112-
/// \param Loc The source location associated with this diagnostic. This
113-
/// location may be invalid, if the diagnostic is not directly related to
114-
/// the source (e.g., if it comes from command-line parsing).
115-
///
116-
/// \param Kind The severity of the diagnostic (error, warning, note).
117-
///
118-
/// \param FormatArgs The diagnostic format string arguments.
119-
///
120-
/// \param Info Extra information associated with the diagnostic.
121-
///
122-
/// \param bufferIndirectlyCausingDiagnostic Only used when directing
123-
/// diagnostics to different outputs.
124-
/// In batch mode a diagnostic may be
125-
/// located in a non-primary file, but there will be no .dia file for a
126-
/// non-primary. If valid, this argument contains a location within a buffer
127-
/// that corresponds to a primary input. The .dia file for that primary can be
128-
/// used for the diagnostic, as if it had occurred at this location.
129-
virtual void
130-
handleDiagnostic(SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
131-
StringRef FormatString,
132-
ArrayRef<DiagnosticArgument> FormatArgs,
133-
const DiagnosticInfo &Info,
134-
SourceLoc bufferIndirectlyCausingDiagnostic) = 0;
119+
/// \param Info Information describing the diagnostic.
120+
virtual void handleDiagnostic(SourceManager &SM,
121+
const DiagnosticInfo &Info) = 0;
135122

136123
/// \returns true if an error occurred while finishing-up.
137124
virtual bool finishProcessing() { return false; }
@@ -149,11 +136,7 @@ class DiagnosticConsumer {
149136
/// DiagnosticConsumer that discards all diagnostics.
150137
class NullDiagnosticConsumer : public DiagnosticConsumer {
151138
public:
152-
void handleDiagnostic(SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
153-
StringRef FormatString,
154-
ArrayRef<DiagnosticArgument> FormatArgs,
155-
const DiagnosticInfo &Info,
156-
SourceLoc bufferIndirectlyCausingDiagnostic) override;
139+
void handleDiagnostic(SourceManager &SM, const DiagnosticInfo &Info) override;
157140
};
158141

159142
/// DiagnosticConsumer that forwards diagnostics to the consumers of
@@ -162,11 +145,7 @@ class ForwardingDiagnosticConsumer : public DiagnosticConsumer {
162145
DiagnosticEngine &TargetEngine;
163146
public:
164147
ForwardingDiagnosticConsumer(DiagnosticEngine &Target);
165-
void handleDiagnostic(SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
166-
StringRef FormatString,
167-
ArrayRef<DiagnosticArgument> FormatArgs,
168-
const DiagnosticInfo &Info,
169-
SourceLoc bufferIndirectlyCausingDiagnostic) override;
148+
void handleDiagnostic(SourceManager &SM, const DiagnosticInfo &Info) override;
170149
};
171150

172151
/// DiagnosticConsumer that funnels diagnostics in certain files to
@@ -228,18 +207,13 @@ class FileSpecificDiagnosticConsumer : public DiagnosticConsumer {
228207
std::unique_ptr<DiagnosticConsumer> consumer)
229208
: inputFileName(inputFileName), consumer(std::move(consumer)) {}
230209

231-
void handleDiagnostic(SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
232-
StringRef FormatString,
233-
ArrayRef<DiagnosticArgument> FormatArgs,
234-
const DiagnosticInfo &Info,
235-
const SourceLoc bufferIndirectlyCausingDiagnostic) {
210+
void handleDiagnostic(SourceManager &SM, const DiagnosticInfo &Info) {
236211
if (!getConsumer())
237212
return;
238-
hasAnErrorBeenConsumed |= Kind == DiagnosticKind::Error;
239-
getConsumer()->handleDiagnostic(SM, Loc, Kind, FormatString, FormatArgs,
240-
Info, bufferIndirectlyCausingDiagnostic);
213+
hasAnErrorBeenConsumed |= Info.Kind == DiagnosticKind::Error;
214+
getConsumer()->handleDiagnostic(SM, Info);
241215
}
242-
216+
243217
void informDriverOfIncompleteBatchModeCompilation() {
244218
if (!hasAnErrorBeenConsumed && getConsumer())
245219
getConsumer()->informDriverOfIncompleteBatchModeCompilation();
@@ -324,11 +298,7 @@ class FileSpecificDiagnosticConsumer : public DiagnosticConsumer {
324298
SmallVectorImpl<Subconsumer> &consumers);
325299

326300
public:
327-
void handleDiagnostic(SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
328-
StringRef FormatString,
329-
ArrayRef<DiagnosticArgument> FormatArgs,
330-
const DiagnosticInfo &Info,
331-
SourceLoc bufferIndirectlyCausingDiagnostic) override;
301+
void handleDiagnostic(SourceManager &SM, const DiagnosticInfo &Info) override;
332302

333303
bool finishProcessing() override;
334304

@@ -348,12 +318,10 @@ class FileSpecificDiagnosticConsumer : public DiagnosticConsumer {
348318
subconsumerForLocation(SourceManager &SM, SourceLoc loc);
349319

350320
Optional<FileSpecificDiagnosticConsumer::Subconsumer *>
351-
findSubconsumer(SourceManager &SM, SourceLoc loc, DiagnosticKind Kind,
352-
SourceLoc bufferIndirectlyCausingDiagnostic);
321+
findSubconsumer(SourceManager &SM, const DiagnosticInfo &Info);
353322

354323
Optional<FileSpecificDiagnosticConsumer::Subconsumer *>
355-
findSubconsumerForNonNote(SourceManager &SM, SourceLoc loc,
356-
SourceLoc bufferIndirectlyCausingDiagnostic);
324+
findSubconsumerForNonNote(SourceManager &SM, const DiagnosticInfo &Info);
357325
};
358326

359327
} // end namespace swift

‎include/swift/Frontend/PrintingDiagnosticConsumer.h

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

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;
38+
virtual void handleDiagnostic(SourceManager &SM,
39+
const DiagnosticInfo &Info) override;
4440

4541
void forceColors() {
4642
ForceColors = true;
@@ -52,11 +48,7 @@ class PrintingDiagnosticConsumer : public DiagnosticConsumer {
5248
}
5349

5450
private:
55-
void printDiagnostic(SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
56-
StringRef FormatString,
57-
ArrayRef<DiagnosticArgument> FormatArgs,
58-
const DiagnosticInfo &Info,
59-
SourceLoc bufferIndirectlyCausingDiagnostic);
51+
void printDiagnostic(SourceManager &SM, const DiagnosticInfo &Info);
6052
};
6153

6254
}

‎include/swift/Migrator/FixitApplyDiagnosticConsumer.h

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

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

7167
unsigned getNumFixitsApplied() const {
7268
return NumFixitsApplied;

‎include/swift/Migrator/FixitFilter.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@ namespace migrator {
2525

2626
struct FixitFilter {
2727
/// Returns true if the fix-it should be applied.
28-
bool shouldTakeFixit(const DiagnosticKind Kind,
29-
const DiagnosticInfo &Info) const {
28+
bool shouldTakeFixit(const DiagnosticInfo &Info) const {
3029
// Do not add a semi or comma as it is wrong in most cases during migration
3130
if (Info.ID == diag::statement_same_line_without_semi.ID ||
3231
Info.ID == diag::declaration_same_line_without_semi.ID ||
@@ -114,7 +113,7 @@ struct FixitFilter {
114113
return false;
115114
}
116115

117-
if (Kind == DiagnosticKind::Error)
116+
if (Info.Kind == DiagnosticKind::Error)
118117
return true;
119118

120119
// Fixits from warnings/notes that should be applied.

‎lib/AST/DiagnosticConsumer.cpp

Lines changed: 21 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -179,38 +179,29 @@ FileSpecificDiagnosticConsumer::subconsumerForLocation(SourceManager &SM,
179179
}
180180

181181
void FileSpecificDiagnosticConsumer::handleDiagnostic(
182-
SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
183-
StringRef FormatString, ArrayRef<DiagnosticArgument> FormatArgs,
184-
const DiagnosticInfo &Info,
185-
const SourceLoc bufferIndirectlyCausingDiagnostic) {
182+
SourceManager &SM, const DiagnosticInfo &Info) {
186183

187-
HasAnErrorBeenConsumed |= Kind == DiagnosticKind::Error;
184+
HasAnErrorBeenConsumed |= Info.Kind == DiagnosticKind::Error;
188185

189-
auto subconsumer =
190-
findSubconsumer(SM, Loc, Kind, bufferIndirectlyCausingDiagnostic);
186+
auto subconsumer = findSubconsumer(SM, Info);
191187
if (subconsumer) {
192-
subconsumer.getValue()->handleDiagnostic(SM, Loc, Kind, FormatString,
193-
FormatArgs, Info,
194-
bufferIndirectlyCausingDiagnostic);
188+
subconsumer.getValue()->handleDiagnostic(SM, Info);
195189
return;
196190
}
197191
// Last resort: spray it everywhere
198192
for (auto &subconsumer : Subconsumers)
199-
subconsumer.handleDiagnostic(SM, Loc, Kind, FormatString, FormatArgs, Info,
200-
bufferIndirectlyCausingDiagnostic);
193+
subconsumer.handleDiagnostic(SM, Info);
201194
}
202195

203196
Optional<FileSpecificDiagnosticConsumer::Subconsumer *>
204-
FileSpecificDiagnosticConsumer::findSubconsumer(
205-
SourceManager &SM, SourceLoc loc, DiagnosticKind Kind,
206-
SourceLoc bufferIndirectlyCausingDiagnostic) {
197+
FileSpecificDiagnosticConsumer::findSubconsumer(SourceManager &SM,
198+
const DiagnosticInfo &Info) {
207199
// Ensure that a note goes to the same place as the preceeding non-note.
208-
switch (Kind) {
200+
switch (Info.Kind) {
209201
case DiagnosticKind::Error:
210202
case DiagnosticKind::Warning:
211203
case DiagnosticKind::Remark: {
212-
auto subconsumer =
213-
findSubconsumerForNonNote(SM, loc, bufferIndirectlyCausingDiagnostic);
204+
auto subconsumer = findSubconsumerForNonNote(SM, Info);
214205
SubconsumerForSubsequentNotes = subconsumer;
215206
return subconsumer;
216207
}
@@ -222,18 +213,17 @@ FileSpecificDiagnosticConsumer::findSubconsumer(
222213

223214
Optional<FileSpecificDiagnosticConsumer::Subconsumer *>
224215
FileSpecificDiagnosticConsumer::findSubconsumerForNonNote(
225-
SourceManager &SM, const SourceLoc loc,
226-
const SourceLoc bufferIndirectlyCausingDiagnostic) {
227-
const auto subconsumer = subconsumerForLocation(SM, loc);
216+
SourceManager &SM, const DiagnosticInfo &Info) {
217+
const auto subconsumer = subconsumerForLocation(SM, Info.Loc);
228218
if (!subconsumer)
229219
return None; // No place to put it; might be in an imported module
230220
if ((*subconsumer)->getConsumer())
231221
return subconsumer; // A primary file with a .dia file
232222
// Try to put it in the responsible primary input
233-
if (bufferIndirectlyCausingDiagnostic.isInvalid())
223+
if (Info.BufferIndirectlyCausingDiagnostic.isInvalid())
234224
return None;
235225
const auto currentPrimarySubconsumer =
236-
subconsumerForLocation(SM, bufferIndirectlyCausingDiagnostic);
226+
subconsumerForLocation(SM, Info.BufferIndirectlyCausingDiagnostic);
237227
assert(!currentPrimarySubconsumer ||
238228
(*currentPrimarySubconsumer)->getConsumer() &&
239229
"current primary must have a .dia file");
@@ -261,14 +251,12 @@ void FileSpecificDiagnosticConsumer::
261251
(*this)[info].informDriverOfIncompleteBatchModeCompilation();
262252
}
263253

264-
void NullDiagnosticConsumer::handleDiagnostic(
265-
SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
266-
StringRef FormatString, ArrayRef<DiagnosticArgument> FormatArgs,
267-
const DiagnosticInfo &Info, const SourceLoc) {
254+
void NullDiagnosticConsumer::handleDiagnostic(SourceManager &SM,
255+
const DiagnosticInfo &Info) {
268256
LLVM_DEBUG({
269257
llvm::dbgs() << "NullDiagnosticConsumer received diagnostic: ";
270-
DiagnosticEngine::formatDiagnosticText(llvm::dbgs(), FormatString,
271-
FormatArgs);
258+
DiagnosticEngine::formatDiagnosticText(llvm::dbgs(), Info.FormatString,
259+
Info.FormatArgs);
272260
llvm::dbgs() << "\n";
273261
});
274262
}
@@ -277,18 +265,14 @@ ForwardingDiagnosticConsumer::ForwardingDiagnosticConsumer(DiagnosticEngine &Tar
277265
: TargetEngine(Target) {}
278266

279267
void ForwardingDiagnosticConsumer::handleDiagnostic(
280-
SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
281-
StringRef FormatString, ArrayRef<DiagnosticArgument> FormatArgs,
282-
const DiagnosticInfo &Info,
283-
const SourceLoc bufferIndirectlyCausingDiagnostic) {
268+
SourceManager &SM, const DiagnosticInfo &Info) {
284269
LLVM_DEBUG({
285270
llvm::dbgs() << "ForwardingDiagnosticConsumer received diagnostic: ";
286-
DiagnosticEngine::formatDiagnosticText(llvm::dbgs(), FormatString,
287-
FormatArgs);
271+
DiagnosticEngine::formatDiagnosticText(llvm::dbgs(), Info.FormatString,
272+
Info.FormatArgs);
288273
llvm::dbgs() << "\n";
289274
});
290275
for (auto *C : TargetEngine.getConsumers()) {
291-
C->handleDiagnostic(SM, Loc, Kind, FormatString, FormatArgs, Info,
292-
bufferIndirectlyCausingDiagnostic);
276+
C->handleDiagnostic(SM, Info);
293277
}
294278
}

‎lib/AST/DiagnosticEngine.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -952,9 +952,7 @@ void DiagnosticEngine::emitDiagnostic(const Diagnostic &diagnostic) {
952952
}
953953
info->ChildDiagnosticInfo = childInfoPtrs;
954954
for (auto &consumer : Consumers) {
955-
consumer->handleDiagnostic(SourceMgr, info->Loc, info->Kind,
956-
info->FormatString, info->FormatArgs, *info,
957-
info->BufferIndirectlyCausingDiagnostic);
955+
consumer->handleDiagnostic(SourceMgr, *info);
958956
}
959957
}
960958

‎lib/Frontend/PrintingDiagnosticConsumer.cpp

Lines changed: 26 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -63,53 +63,44 @@ namespace {
6363
};
6464
} // end anonymous namespace
6565

66-
void PrintingDiagnosticConsumer::handleDiagnostic(
67-
SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
68-
StringRef FormatString, ArrayRef<DiagnosticArgument> FormatArgs,
69-
const DiagnosticInfo &Info,
70-
const SourceLoc bufferIndirectlyCausingDiagnostic) {
66+
void PrintingDiagnosticConsumer::handleDiagnostic(SourceManager &SM,
67+
const DiagnosticInfo &Info) {
7168
if (Info.IsChildNote)
7269
return;
7370

74-
printDiagnostic(SM, Loc, Kind, FormatString, FormatArgs, Info,
75-
bufferIndirectlyCausingDiagnostic);
71+
printDiagnostic(SM, Info);
7672

7773
for (auto ChildInfo : Info.ChildDiagnosticInfo) {
78-
printDiagnostic(SM, ChildInfo->Loc, ChildInfo->Kind,
79-
ChildInfo->FormatString, ChildInfo->FormatArgs, *ChildInfo,
80-
ChildInfo->BufferIndirectlyCausingDiagnostic);
74+
printDiagnostic(SM, *ChildInfo);
8175
}
8276
}
8377

84-
void PrintingDiagnosticConsumer::printDiagnostic(
85-
SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
86-
StringRef FormatString, ArrayRef<DiagnosticArgument> FormatArgs,
87-
const DiagnosticInfo &Info,
88-
const SourceLoc bufferIndirectlyCausingDiagnostic) {
78+
void PrintingDiagnosticConsumer::printDiagnostic(SourceManager &SM,
79+
const DiagnosticInfo &Info) {
8980

9081
// Determine what kind of diagnostic we're emitting.
9182
llvm::SourceMgr::DiagKind SMKind;
92-
switch (Kind) {
93-
case DiagnosticKind::Error:
94-
SMKind = llvm::SourceMgr::DK_Error;
95-
break;
96-
case DiagnosticKind::Warning:
97-
SMKind = llvm::SourceMgr::DK_Warning;
98-
break;
99-
100-
case DiagnosticKind::Note:
101-
SMKind = llvm::SourceMgr::DK_Note;
102-
break;
103-
104-
case DiagnosticKind::Remark:
105-
SMKind = llvm::SourceMgr::DK_Remark;
106-
break;
83+
switch (Info.Kind) {
84+
case DiagnosticKind::Error:
85+
SMKind = llvm::SourceMgr::DK_Error;
86+
break;
87+
case DiagnosticKind::Warning:
88+
SMKind = llvm::SourceMgr::DK_Warning;
89+
break;
90+
91+
case DiagnosticKind::Note:
92+
SMKind = llvm::SourceMgr::DK_Note;
93+
break;
94+
95+
case DiagnosticKind::Remark:
96+
SMKind = llvm::SourceMgr::DK_Remark;
97+
break;
10798
}
10899

109-
if (Kind == DiagnosticKind::Error) {
100+
if (Info.Kind == DiagnosticKind::Error) {
110101
DidErrorOccur = true;
111102
}
112-
103+
113104
// Translate ranges.
114105
SmallVector<llvm::SMRange, 2> Ranges;
115106
for (auto R : Info.Ranges)
@@ -129,10 +120,11 @@ void PrintingDiagnosticConsumer::printDiagnostic(
129120
llvm::SmallString<256> Text;
130121
{
131122
llvm::raw_svector_ostream Out(Text);
132-
DiagnosticEngine::formatDiagnosticText(Out, FormatString, FormatArgs);
123+
DiagnosticEngine::formatDiagnosticText(Out, Info.FormatString,
124+
Info.FormatArgs);
133125
}
134126

135-
auto Msg = SM.GetMessage(Loc, SMKind, Text, Ranges, FixIts);
127+
auto Msg = SM.GetMessage(Info.Loc, SMKind, Text, Ranges, FixIts);
136128
rawSM.PrintMessage(out, Msg, ForceColors);
137129
}
138130

0 commit comments

Comments
 (0)