Skip to content

[Diagnostics] Refactor DiagnosticConsumer interface to remove unnecessary params #27868

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 30, 2019
Merged

[Diagnostics] Refactor DiagnosticConsumer interface to remove unnecessary params #27868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 19 additions & 51 deletions include/swift/AST/DiagnosticConsumer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ struct DiagnosticInfo {
DiagnosticKind Kind;
StringRef FormatString;
ArrayRef<DiagnosticArgument> FormatArgs;

/// Only used when directing diagnostics to different outputs.
/// In batch mode a diagnostic may be
/// located in a non-primary file, but there will be no .dia file for a
/// non-primary. If valid, this argument contains a location within a buffer
/// that corresponds to a primary input. The .dia file for that primary can be
/// used for the diagnostic, as if it had occurred at this location.
SourceLoc BufferIndirectlyCausingDiagnostic;

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

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

/// DiagnosticConsumer that forwards diagnostics to the consumers of
Expand All @@ -162,11 +145,7 @@ class ForwardingDiagnosticConsumer : public DiagnosticConsumer {
DiagnosticEngine &TargetEngine;
public:
ForwardingDiagnosticConsumer(DiagnosticEngine &Target);
void handleDiagnostic(SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
StringRef FormatString,
ArrayRef<DiagnosticArgument> FormatArgs,
const DiagnosticInfo &Info,
SourceLoc bufferIndirectlyCausingDiagnostic) override;
void handleDiagnostic(SourceManager &SM, const DiagnosticInfo &Info) override;
};

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

void handleDiagnostic(SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
StringRef FormatString,
ArrayRef<DiagnosticArgument> FormatArgs,
const DiagnosticInfo &Info,
const SourceLoc bufferIndirectlyCausingDiagnostic) {
void handleDiagnostic(SourceManager &SM, const DiagnosticInfo &Info) {
if (!getConsumer())
return;
hasAnErrorBeenConsumed |= Kind == DiagnosticKind::Error;
getConsumer()->handleDiagnostic(SM, Loc, Kind, FormatString, FormatArgs,
Info, bufferIndirectlyCausingDiagnostic);
hasAnErrorBeenConsumed |= Info.Kind == DiagnosticKind::Error;
getConsumer()->handleDiagnostic(SM, Info);
}

void informDriverOfIncompleteBatchModeCompilation() {
if (!hasAnErrorBeenConsumed && getConsumer())
getConsumer()->informDriverOfIncompleteBatchModeCompilation();
Expand Down Expand Up @@ -324,11 +298,7 @@ class FileSpecificDiagnosticConsumer : public DiagnosticConsumer {
SmallVectorImpl<Subconsumer> &consumers);

public:
void handleDiagnostic(SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
StringRef FormatString,
ArrayRef<DiagnosticArgument> FormatArgs,
const DiagnosticInfo &Info,
SourceLoc bufferIndirectlyCausingDiagnostic) override;
void handleDiagnostic(SourceManager &SM, const DiagnosticInfo &Info) override;

bool finishProcessing() override;

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

Optional<FileSpecificDiagnosticConsumer::Subconsumer *>
findSubconsumer(SourceManager &SM, SourceLoc loc, DiagnosticKind Kind,
SourceLoc bufferIndirectlyCausingDiagnostic);
findSubconsumer(SourceManager &SM, const DiagnosticInfo &Info);

Optional<FileSpecificDiagnosticConsumer::Subconsumer *>
findSubconsumerForNonNote(SourceManager &SM, SourceLoc loc,
SourceLoc bufferIndirectlyCausingDiagnostic);
findSubconsumerForNonNote(SourceManager &SM, const DiagnosticInfo &Info);
};

} // end namespace swift
Expand Down
14 changes: 3 additions & 11 deletions include/swift/Frontend/PrintingDiagnosticConsumer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,8 @@ class PrintingDiagnosticConsumer : public DiagnosticConsumer {
PrintingDiagnosticConsumer(llvm::raw_ostream &stream = llvm::errs()) :
Stream(stream) { }

virtual void
handleDiagnostic(SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
StringRef FormatString,
ArrayRef<DiagnosticArgument> FormatArgs,
const DiagnosticInfo &Info,
SourceLoc bufferIndirectlyCausingDiagnostic) override;
virtual void handleDiagnostic(SourceManager &SM,
const DiagnosticInfo &Info) override;

void forceColors() {
ForceColors = true;
Expand All @@ -52,11 +48,7 @@ class PrintingDiagnosticConsumer : public DiagnosticConsumer {
}

private:
void printDiagnostic(SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
StringRef FormatString,
ArrayRef<DiagnosticArgument> FormatArgs,
const DiagnosticInfo &Info,
SourceLoc bufferIndirectlyCausingDiagnostic);
void printDiagnostic(SourceManager &SM, const DiagnosticInfo &Info);
};

}
Expand Down
6 changes: 1 addition & 5 deletions include/swift/Migrator/FixitApplyDiagnosticConsumer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,7 @@ class FixitApplyDiagnosticConsumer final
/// output stream.
void printResult(llvm::raw_ostream &OS) const;

void handleDiagnostic(SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
StringRef FormatString,
ArrayRef<DiagnosticArgument> FormatArgs,
const DiagnosticInfo &Info,
SourceLoc bufferIndirectlyCausingDiagnostic) override;
void handleDiagnostic(SourceManager &SM, const DiagnosticInfo &Info) override;

unsigned getNumFixitsApplied() const {
return NumFixitsApplied;
Expand Down
5 changes: 2 additions & 3 deletions include/swift/Migrator/FixitFilter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ namespace migrator {

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

if (Kind == DiagnosticKind::Error)
if (Info.Kind == DiagnosticKind::Error)
return true;

// Fixits from warnings/notes that should be applied.
Expand Down
58 changes: 21 additions & 37 deletions lib/AST/DiagnosticConsumer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,38 +179,29 @@ FileSpecificDiagnosticConsumer::subconsumerForLocation(SourceManager &SM,
}

void FileSpecificDiagnosticConsumer::handleDiagnostic(
SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
StringRef FormatString, ArrayRef<DiagnosticArgument> FormatArgs,
const DiagnosticInfo &Info,
const SourceLoc bufferIndirectlyCausingDiagnostic) {
SourceManager &SM, const DiagnosticInfo &Info) {

HasAnErrorBeenConsumed |= Kind == DiagnosticKind::Error;
HasAnErrorBeenConsumed |= Info.Kind == DiagnosticKind::Error;

auto subconsumer =
findSubconsumer(SM, Loc, Kind, bufferIndirectlyCausingDiagnostic);
auto subconsumer = findSubconsumer(SM, Info);
if (subconsumer) {
subconsumer.getValue()->handleDiagnostic(SM, Loc, Kind, FormatString,
FormatArgs, Info,
bufferIndirectlyCausingDiagnostic);
subconsumer.getValue()->handleDiagnostic(SM, Info);
return;
}
// Last resort: spray it everywhere
for (auto &subconsumer : Subconsumers)
subconsumer.handleDiagnostic(SM, Loc, Kind, FormatString, FormatArgs, Info,
bufferIndirectlyCausingDiagnostic);
subconsumer.handleDiagnostic(SM, Info);
}

Optional<FileSpecificDiagnosticConsumer::Subconsumer *>
FileSpecificDiagnosticConsumer::findSubconsumer(
SourceManager &SM, SourceLoc loc, DiagnosticKind Kind,
SourceLoc bufferIndirectlyCausingDiagnostic) {
FileSpecificDiagnosticConsumer::findSubconsumer(SourceManager &SM,
const DiagnosticInfo &Info) {
// Ensure that a note goes to the same place as the preceeding non-note.
switch (Kind) {
switch (Info.Kind) {
case DiagnosticKind::Error:
case DiagnosticKind::Warning:
case DiagnosticKind::Remark: {
auto subconsumer =
findSubconsumerForNonNote(SM, loc, bufferIndirectlyCausingDiagnostic);
auto subconsumer = findSubconsumerForNonNote(SM, Info);
SubconsumerForSubsequentNotes = subconsumer;
return subconsumer;
}
Expand All @@ -222,18 +213,17 @@ FileSpecificDiagnosticConsumer::findSubconsumer(

Optional<FileSpecificDiagnosticConsumer::Subconsumer *>
FileSpecificDiagnosticConsumer::findSubconsumerForNonNote(
SourceManager &SM, const SourceLoc loc,
const SourceLoc bufferIndirectlyCausingDiagnostic) {
const auto subconsumer = subconsumerForLocation(SM, loc);
SourceManager &SM, const DiagnosticInfo &Info) {
const auto subconsumer = subconsumerForLocation(SM, Info.Loc);
if (!subconsumer)
return None; // No place to put it; might be in an imported module
if ((*subconsumer)->getConsumer())
return subconsumer; // A primary file with a .dia file
// Try to put it in the responsible primary input
if (bufferIndirectlyCausingDiagnostic.isInvalid())
if (Info.BufferIndirectlyCausingDiagnostic.isInvalid())
return None;
const auto currentPrimarySubconsumer =
subconsumerForLocation(SM, bufferIndirectlyCausingDiagnostic);
subconsumerForLocation(SM, Info.BufferIndirectlyCausingDiagnostic);
assert(!currentPrimarySubconsumer ||
(*currentPrimarySubconsumer)->getConsumer() &&
"current primary must have a .dia file");
Expand Down Expand Up @@ -261,14 +251,12 @@ void FileSpecificDiagnosticConsumer::
(*this)[info].informDriverOfIncompleteBatchModeCompilation();
}

void NullDiagnosticConsumer::handleDiagnostic(
SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
StringRef FormatString, ArrayRef<DiagnosticArgument> FormatArgs,
const DiagnosticInfo &Info, const SourceLoc) {
void NullDiagnosticConsumer::handleDiagnostic(SourceManager &SM,
const DiagnosticInfo &Info) {
LLVM_DEBUG({
llvm::dbgs() << "NullDiagnosticConsumer received diagnostic: ";
DiagnosticEngine::formatDiagnosticText(llvm::dbgs(), FormatString,
FormatArgs);
DiagnosticEngine::formatDiagnosticText(llvm::dbgs(), Info.FormatString,
Info.FormatArgs);
llvm::dbgs() << "\n";
});
}
Expand All @@ -277,18 +265,14 @@ ForwardingDiagnosticConsumer::ForwardingDiagnosticConsumer(DiagnosticEngine &Tar
: TargetEngine(Target) {}

void ForwardingDiagnosticConsumer::handleDiagnostic(
SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
StringRef FormatString, ArrayRef<DiagnosticArgument> FormatArgs,
const DiagnosticInfo &Info,
const SourceLoc bufferIndirectlyCausingDiagnostic) {
SourceManager &SM, const DiagnosticInfo &Info) {
LLVM_DEBUG({
llvm::dbgs() << "ForwardingDiagnosticConsumer received diagnostic: ";
DiagnosticEngine::formatDiagnosticText(llvm::dbgs(), FormatString,
FormatArgs);
DiagnosticEngine::formatDiagnosticText(llvm::dbgs(), Info.FormatString,
Info.FormatArgs);
llvm::dbgs() << "\n";
});
for (auto *C : TargetEngine.getConsumers()) {
C->handleDiagnostic(SM, Loc, Kind, FormatString, FormatArgs, Info,
bufferIndirectlyCausingDiagnostic);
C->handleDiagnostic(SM, Info);
}
}
4 changes: 1 addition & 3 deletions lib/AST/DiagnosticEngine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -952,9 +952,7 @@ void DiagnosticEngine::emitDiagnostic(const Diagnostic &diagnostic) {
}
info->ChildDiagnosticInfo = childInfoPtrs;
for (auto &consumer : Consumers) {
consumer->handleDiagnostic(SourceMgr, info->Loc, info->Kind,
info->FormatString, info->FormatArgs, *info,
info->BufferIndirectlyCausingDiagnostic);
consumer->handleDiagnostic(SourceMgr, *info);
}
}

Expand Down
60 changes: 26 additions & 34 deletions lib/Frontend/PrintingDiagnosticConsumer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,53 +63,44 @@ namespace {
};
} // end anonymous namespace

void PrintingDiagnosticConsumer::handleDiagnostic(
SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
StringRef FormatString, ArrayRef<DiagnosticArgument> FormatArgs,
const DiagnosticInfo &Info,
const SourceLoc bufferIndirectlyCausingDiagnostic) {
void PrintingDiagnosticConsumer::handleDiagnostic(SourceManager &SM,
const DiagnosticInfo &Info) {
if (Info.IsChildNote)
return;

printDiagnostic(SM, Loc, Kind, FormatString, FormatArgs, Info,
bufferIndirectlyCausingDiagnostic);
printDiagnostic(SM, Info);

for (auto ChildInfo : Info.ChildDiagnosticInfo) {
printDiagnostic(SM, ChildInfo->Loc, ChildInfo->Kind,
ChildInfo->FormatString, ChildInfo->FormatArgs, *ChildInfo,
ChildInfo->BufferIndirectlyCausingDiagnostic);
printDiagnostic(SM, *ChildInfo);
}
}

void PrintingDiagnosticConsumer::printDiagnostic(
SourceManager &SM, SourceLoc Loc, DiagnosticKind Kind,
StringRef FormatString, ArrayRef<DiagnosticArgument> FormatArgs,
const DiagnosticInfo &Info,
const SourceLoc bufferIndirectlyCausingDiagnostic) {
void PrintingDiagnosticConsumer::printDiagnostic(SourceManager &SM,
const DiagnosticInfo &Info) {

// Determine what kind of diagnostic we're emitting.
llvm::SourceMgr::DiagKind SMKind;
switch (Kind) {
case DiagnosticKind::Error:
SMKind = llvm::SourceMgr::DK_Error;
break;
case DiagnosticKind::Warning:
SMKind = llvm::SourceMgr::DK_Warning;
break;

case DiagnosticKind::Note:
SMKind = llvm::SourceMgr::DK_Note;
break;

case DiagnosticKind::Remark:
SMKind = llvm::SourceMgr::DK_Remark;
break;
switch (Info.Kind) {
case DiagnosticKind::Error:
SMKind = llvm::SourceMgr::DK_Error;
break;
case DiagnosticKind::Warning:
SMKind = llvm::SourceMgr::DK_Warning;
break;

case DiagnosticKind::Note:
SMKind = llvm::SourceMgr::DK_Note;
break;

case DiagnosticKind::Remark:
SMKind = llvm::SourceMgr::DK_Remark;
break;
}

if (Kind == DiagnosticKind::Error) {
if (Info.Kind == DiagnosticKind::Error) {
DidErrorOccur = true;
}

// Translate ranges.
SmallVector<llvm::SMRange, 2> Ranges;
for (auto R : Info.Ranges)
Expand All @@ -129,10 +120,11 @@ void PrintingDiagnosticConsumer::printDiagnostic(
llvm::SmallString<256> Text;
{
llvm::raw_svector_ostream Out(Text);
DiagnosticEngine::formatDiagnosticText(Out, FormatString, FormatArgs);
DiagnosticEngine::formatDiagnosticText(Out, Info.FormatString,
Info.FormatArgs);
}

auto Msg = SM.GetMessage(Loc, SMKind, Text, Ranges, FixIts);
auto Msg = SM.GetMessage(Info.Loc, SMKind, Text, Ranges, FixIts);
rawSM.PrintMessage(out, Msg, ForceColors);
}

Expand Down
Loading