Skip to content

Commit d14a85e

Browse files
Merge pull request #80458 from AnthonyLatsis/hildoceras
DiagnosticEngine: Always favor the category of a wrapped diagnostic
2 parents 28c4930 + 148c177 commit d14a85e

File tree

3 files changed

+85
-13
lines changed

3 files changed

+85
-13
lines changed

include/swift/AST/DiagnosticEngine.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -599,6 +599,9 @@ namespace swift {
599599
void setDecl(const class Decl *decl) { Decl = decl; }
600600
void setBehaviorLimit(DiagnosticBehavior limit){ BehaviorLimit = limit; }
601601

602+
/// Returns the wrapped diagnostic, if any.
603+
std::optional<const DiagnosticInfo *> getWrappedDiagnostic() const;
604+
602605
/// Returns true if this object represents a particular diagnostic.
603606
///
604607
/// \code

lib/AST/DiagnosticEngine.cpp

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,16 @@ DiagnosticState::DiagnosticState() {
166166
Diagnostic::Diagnostic(DiagID ID)
167167
: Diagnostic(ID, storedDiagnosticInfos[(unsigned)ID].groupID) {}
168168

169+
std::optional<const DiagnosticInfo *> Diagnostic::getWrappedDiagnostic() const {
170+
for (const auto &arg : getArgs()) {
171+
if (arg.getKind() == DiagnosticArgumentKind::Diagnostic) {
172+
return arg.getAsDiagnostic();
173+
}
174+
}
175+
176+
return std::nullopt;
177+
}
178+
169179
static CharSourceRange toCharSourceRange(SourceManager &SM, SourceRange SR) {
170180
return CharSourceRange(SM, SR.Start, Lexer::getLocForEndOfToken(SM, SR.End));
171181
}
@@ -498,13 +508,14 @@ InFlightDiagnostic::wrapIn(const Diagnostic &wrapper) {
498508
Engine->WrappedDiagnosticArgs.emplace_back(wrapped.FormatArgs);
499509
wrapped.FormatArgs = Engine->WrappedDiagnosticArgs.back();
500510

501-
// Overwrite the ID and argument with those from the wrapper.
511+
// Overwrite the ID and arguments with those from the wrapper.
502512
Engine->getActiveDiagnostic().ID = wrapper.ID;
503513
Engine->getActiveDiagnostic().Args = wrapper.Args;
504514
// Intentionally keeping the original GroupID here
505515

506516
// Set the argument to the diagnostic being wrapped.
507-
assert(wrapper.getArgs().front().getKind() == DiagnosticArgumentKind::Diagnostic);
517+
ASSERT(wrapper.getArgs().front().getKind() ==
518+
DiagnosticArgumentKind::Diagnostic);
508519
Engine->getActiveDiagnostic().Args.front() = &wrapped;
509520

510521
return *this;
@@ -1377,7 +1388,9 @@ DiagnosticEngine::diagnosticInfoForDiagnostic(const Diagnostic &diagnostic,
13771388

13781389
auto groupID = diagnostic.getGroupID();
13791390
StringRef Category;
1380-
if (isAPIDigesterBreakageDiagnostic(diagnostic.getID()))
1391+
if (auto wrapped = diagnostic.getWrappedDiagnostic())
1392+
Category = wrapped.value()->Category;
1393+
else if (isAPIDigesterBreakageDiagnostic(diagnostic.getID()))
13811394
Category = "api-digester-breaking-change";
13821395
else if (isNoUsageDiagnostic(diagnostic.getID()))
13831396
Category = "no-usage";
@@ -1621,16 +1634,15 @@ DiagnosticEngine::getFormatStringForDiagnostic(const Diagnostic &diagnostic,
16211634
case PrintDiagnosticNamesMode::Group:
16221635
break;
16231636
case PrintDiagnosticNamesMode::Identifier: {
1624-
// If this diagnostic is a wrapper for another diagnostic, use the ID of
1625-
// the wrapped diagnostic.
1626-
for (const auto &arg : diagnostic.getArgs()) {
1627-
if (arg.getKind() == DiagnosticArgumentKind::Diagnostic) {
1628-
diagID = arg.getAsDiagnostic()->ID;
1629-
break;
1630-
}
1637+
auto userFacingID = diagID;
1638+
// If this diagnostic wraps another one, use the ID of the wrapped
1639+
// diagnostic.
1640+
if (auto wrapped = diagnostic.getWrappedDiagnostic()) {
1641+
userFacingID = wrapped.value()->ID;
16311642
}
16321643

1633-
message = formatMessageWithName(message, diagnosticIDStringFor(diagID));
1644+
message =
1645+
formatMessageWithName(message, diagnosticIDStringFor(userFacingID));
16341646
break;
16351647
}
16361648
}

unittests/AST/DiagnosticInfoTests.cpp

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
#include "swift/AST/DiagnosticEngine.h"
1414
#include "swift/AST/DiagnosticGroups.h"
1515
#include "swift/AST/DiagnosticsFrontend.h"
16+
#include "swift/AST/DiagnosticsModuleDiffer.h"
17+
#include "swift/AST/DiagnosticsSema.h"
1618
#include "swift/Basic/SourceManager.h"
1719
#include "gtest/gtest.h"
1820

@@ -142,7 +144,7 @@ TEST(DiagnosticInfo, PrintDiagnosticNamesMode_Group) {
142144
},
143145
[](DiagnosticEngine &, const DiagnosticInfo &info) {
144146
EXPECT_FALSE(info.FormatString.ends_with(" [DeprecatedDeclaration]"));
145-
EXPECT_TRUE(info.Category == "DeprecatedDeclaration");
147+
EXPECT_EQ(info.Category, "DeprecatedDeclaration");
146148
},
147149
/*expectedNumCallbackCalls=*/1);
148150
}
@@ -162,9 +164,64 @@ TEST(DiagnosticInfo, PrintDiagnosticNamesMode_Group_WrappedDiag) {
162164
[](DiagnosticEngine &, const DiagnosticInfo &info) {
163165
EXPECT_EQ(info.ID, diag::error_in_a_future_swift_lang_mode.ID);
164166
EXPECT_FALSE(info.FormatString.ends_with(" [DeprecatedDeclaration]"));
165-
EXPECT_TRUE(info.Category == "DeprecatedDeclaration");
167+
EXPECT_EQ(info.Category, "DeprecatedDeclaration");
166168
},
167169
/*expectedNumCallbackCalls=*/1);
168170
}
169171

172+
// Test that the category is appropriately set in these cases, and that the
173+
// category of a wrapped diagnostic is favored.
174+
TEST(DiagnosticInfo, CategoryDeprecation) {
175+
testCase(
176+
[](DiagnosticEngine &diags) {
177+
diags.setLanguageVersion(version::Version({5}));
178+
179+
const auto diag = diag::iuo_deprecated_here;
180+
EXPECT_TRUE(diags.isDeprecationDiagnostic(diag.ID));
181+
182+
diags.diagnose(SourceLoc(), diag);
183+
diags.diagnose(SourceLoc(), diag).warnUntilSwiftVersion(6);
184+
diags.diagnose(SourceLoc(), diag).warnUntilSwiftVersion(99);
185+
},
186+
[](DiagnosticEngine &, const DiagnosticInfo &info) {
187+
EXPECT_EQ(info.Category, "deprecation");
188+
},
189+
/*expectedNumCallbackCalls=*/3);
190+
}
191+
TEST(DiagnosticInfo, CategoryNoUsage) {
192+
testCase(
193+
[](DiagnosticEngine &diags) {
194+
diags.setLanguageVersion(version::Version({5}));
195+
196+
const auto diag = diag::expression_unused_function;
197+
EXPECT_TRUE(diags.isNoUsageDiagnostic(diag.ID));
198+
199+
diags.diagnose(SourceLoc(), diag);
200+
diags.diagnose(SourceLoc(), diag).warnUntilSwiftVersion(6);
201+
diags.diagnose(SourceLoc(), diag).warnUntilSwiftVersion(99);
202+
},
203+
[](DiagnosticEngine &, const DiagnosticInfo &info) {
204+
EXPECT_EQ(info.Category, "no-usage");
205+
},
206+
/*expectedNumCallbackCalls=*/3);
207+
}
208+
TEST(DiagnosticInfo, CategoryAPIDigesterBreakage) {
209+
testCase(
210+
[](DiagnosticEngine &diags) {
211+
diags.setLanguageVersion(version::Version({5}));
212+
213+
const auto diag = diag::enum_case_added;
214+
EXPECT_TRUE(diags.isAPIDigesterBreakageDiagnostic(diag.ID));
215+
216+
diags.diagnose(SourceLoc(), diag, StringRef());
217+
diags.diagnose(SourceLoc(), diag, StringRef()).warnUntilSwiftVersion(6);
218+
diags.diagnose(SourceLoc(), diag, StringRef())
219+
.warnUntilSwiftVersion(99);
220+
},
221+
[](DiagnosticEngine &, const DiagnosticInfo &info) {
222+
EXPECT_EQ(info.Category, "api-digester-breaking-change");
223+
},
224+
/*expectedNumCallbackCalls=*/3);
225+
}
226+
170227
} // end anonymous namespace

0 commit comments

Comments
 (0)