From 7934a16a6871d621da9b5f99bd2795d880d39985 Mon Sep 17 00:00:00 2001 From: Dmitrii Galimzianov Date: Tue, 3 Dec 2024 20:12:11 +0100 Subject: [PATCH 1/3] DiagnosticEngine: Add `DiagGroupID` to `Diagnostic` This change addresses the following issue: when an error is being wrapped in a warning, the diagnostic message will use the wrapper's `DiagGroupID` as the warning's name. However, we want to retain the original error's group for use. For example, in Swift 5, `async_unavailable_decl` is wrapped in `error_in_future_swift_version`. When we print a diagnostic of this kind, we want to keep the `DiagGroupID` of `async_unavailable_decl`, not that of `error_in_future_swift_version`. To achieve this, we add `DiagGroupID` to the `Diagnostic` class. When an active diagnostic is wrapped in `DiagnosticEngine`, we retain the original `DiagGroupID`. --- include/swift/AST/DiagnosticEngine.h | 25 +++++++++--- include/swift/AST/DiagnosticsCommon.def | 4 ++ lib/AST/DiagnosticEngine.cpp | 49 ++++++++++++++++------- lib/IDE/CodeCompletionDiagnostics.cpp | 2 +- lib/PrintAsClang/ModuleContentsWriter.cpp | 4 +- 5 files changed, 62 insertions(+), 22 deletions(-) diff --git a/include/swift/AST/DiagnosticEngine.h b/include/swift/AST/DiagnosticEngine.h index 3319614217df7..f0c1f1fff4751 100644 --- a/include/swift/AST/DiagnosticEngine.h +++ b/include/swift/AST/DiagnosticEngine.h @@ -67,6 +67,8 @@ namespace swift { /// this enumeration type that uniquely identifies it. enum class DiagID : uint32_t; + enum class DiagGroupID : uint16_t; + /// Describes a diagnostic along with its argument types. /// /// The diagnostics header introduces instances of this type for each @@ -498,6 +500,7 @@ namespace swift { private: DiagID ID; + DiagGroupID GroupID; SmallVector Args; SmallVector Ranges; SmallVector FixIts; @@ -510,7 +513,10 @@ namespace swift { friend DiagnosticEngine; friend class InFlightDiagnostic; - Diagnostic(DiagID ID) : ID(ID) {} + Diagnostic(DiagID ID, DiagGroupID GroupID) : ID(ID), GroupID(GroupID) {} + + /// Constructs a Diagnostic with DiagGroupID infered from DiagID. + Diagnostic(DiagID ID); public: // All constructors are intentionally implicit. @@ -535,6 +541,7 @@ namespace swift { // Accessors. DiagID getID() const { return ID; } + DiagGroupID getGroupID() const { return GroupID; } ArrayRef getArgs() const { return Args; } ArrayRef getRanges() const { return Ranges; } ArrayRef getFixIts() const { return FixIts; } @@ -1434,7 +1441,8 @@ namespace swift { /// Generate DiagnosticInfo for a Diagnostic to be passed to consumers. std::optional - diagnosticInfoForDiagnostic(const Diagnostic &diagnostic); + diagnosticInfoForDiagnostic(const Diagnostic &diagnostic, + bool includeDiagnosticName); /// Send \c diag to all diagnostic consumers. void emitDiagnostic(const Diagnostic &diag); @@ -1460,9 +1468,16 @@ namespace swift { public: DiagnosticKind declaredDiagnosticKindFor(const DiagID id); - llvm::StringRef - diagnosticStringFor(const DiagID id, - PrintDiagnosticNamesMode printDiagnosticNamesMode); + /// Get a localized format string for a given `DiagID`. If no localization + /// available returns the default string for that `DiagID`. + llvm::StringRef diagnosticStringFor(DiagID id); + + /// Get a localized format string with an optional diagnostic name appended + /// to it. The diagnostic name type is defined by + /// `PrintDiagnosticNamesMode`. + llvm::StringRef diagnosticStringWithNameFor( + DiagID id, DiagGroupID groupID, + PrintDiagnosticNamesMode printDiagnosticNamesMode); static llvm::StringRef diagnosticIDStringFor(const DiagID id); diff --git a/include/swift/AST/DiagnosticsCommon.def b/include/swift/AST/DiagnosticsCommon.def index 380608bd6c46a..f1ea5d7ca27fa 100644 --- a/include/swift/AST/DiagnosticsCommon.def +++ b/include/swift/AST/DiagnosticsCommon.def @@ -47,10 +47,14 @@ ERROR(error_no_group_info,none, NOTE(brace_stmt_suggest_do,none, "did you mean to use a 'do' statement?", ()) +// `error_in_future_swift_version` does not have a group because warnings of this kind +// inherit the group from the wrapped error. WARNING(error_in_future_swift_version,none, "%0; this is an error in the Swift %1 language mode", (DiagnosticInfo *, unsigned)) +// `error_in_a_future_swift_version` does not have a group because warnings of this kind +// inherit the group from the wrapped error. WARNING(error_in_a_future_swift_version,none, "%0; this will be an error in a future Swift language mode", (DiagnosticInfo *)) diff --git a/lib/AST/DiagnosticEngine.cpp b/lib/AST/DiagnosticEngine.cpp index f5bca58bd0c70..80d476856e8bf 100644 --- a/lib/AST/DiagnosticEngine.cpp +++ b/lib/AST/DiagnosticEngine.cpp @@ -179,6 +179,9 @@ DiagnosticState::DiagnosticState() { warningsAsErrors.resize(LocalDiagID::NumDiags); } +Diagnostic::Diagnostic(DiagID ID) + : Diagnostic(ID, storedDiagnosticInfos[(unsigned)ID].groupID) {} + static CharSourceRange toCharSourceRange(SourceManager &SM, SourceRange SR) { return CharSourceRange(SM, SR.Start, Lexer::getLocForEndOfToken(SM, SR.End)); } @@ -464,8 +467,8 @@ InFlightDiagnostic::wrapIn(const Diagnostic &wrapper) { limit(Engine->getActiveDiagnostic().BehaviorLimit, DiagnosticBehavior::Unspecified); - Engine->WrappedDiagnostics.push_back( - *Engine->diagnosticInfoForDiagnostic(Engine->getActiveDiagnostic())); + Engine->WrappedDiagnostics.push_back(*Engine->diagnosticInfoForDiagnostic( + Engine->getActiveDiagnostic(), /* includeDiagnosticName= */ false)); Engine->state.swap(tempState); @@ -478,6 +481,7 @@ InFlightDiagnostic::wrapIn(const Diagnostic &wrapper) { // Overwrite the ID and argument with those from the wrapper. Engine->getActiveDiagnostic().ID = wrapper.ID; Engine->getActiveDiagnostic().Args = wrapper.Args; + // Intentionally keeping the original GroupID here // Set the argument to the diagnostic being wrapped. assert(wrapper.getArgs().front().getKind() == DiagnosticArgumentKind::Diagnostic); @@ -1294,7 +1298,8 @@ void DiagnosticEngine::forwardTentativeDiagnosticsTo( } std::optional -DiagnosticEngine::diagnosticInfoForDiagnostic(const Diagnostic &diagnostic) { +DiagnosticEngine::diagnosticInfoForDiagnostic(const Diagnostic &diagnostic, + bool includeDiagnosticName) { auto behavior = state.determineBehavior(diagnostic); if (behavior == DiagnosticBehavior::Ignore) return std::nullopt; @@ -1347,12 +1352,19 @@ DiagnosticEngine::diagnosticInfoForDiagnostic(const Diagnostic &diagnostic) { } } - return DiagnosticInfo( - diagnostic.getID(), loc, toDiagnosticKind(behavior), - diagnosticStringFor(diagnostic.getID(), getPrintDiagnosticNamesMode()), - diagnostic.getArgs(), Category, getDefaultDiagnosticLoc(), - /*child note info*/ {}, diagnostic.getRanges(), fixIts, - diagnostic.isChildNote()); + llvm::StringRef format; + if (includeDiagnosticName) + format = + diagnosticStringWithNameFor(diagnostic.getID(), diagnostic.getGroupID(), + getPrintDiagnosticNamesMode()); + else + format = diagnosticStringFor(diagnostic.getID()); + + return DiagnosticInfo(diagnostic.getID(), loc, toDiagnosticKind(behavior), + format, diagnostic.getArgs(), Category, + getDefaultDiagnosticLoc(), + /*child note info*/ {}, diagnostic.getRanges(), fixIts, + diagnostic.isChildNote()); } static DeclName @@ -1462,7 +1474,9 @@ void DiagnosticEngine::emitDiagnostic(const Diagnostic &diagnostic) { ArrayRef childNotes = diagnostic.getChildNotes(); std::vector extendedChildNotes; - if (auto info = diagnosticInfoForDiagnostic(diagnostic)) { + if (auto info = + diagnosticInfoForDiagnostic(diagnostic, + /* includeDiagnosticName= */ true)) { // If the diagnostic location is within a buffer containing generated // source code, add child notes showing where the generation occurred. // We need to avoid doing this if this is itself a child note, as otherwise @@ -1478,7 +1492,9 @@ void DiagnosticEngine::emitDiagnostic(const Diagnostic &diagnostic) { SmallVector childInfo; for (unsigned i : indices(childNotes)) { - auto child = diagnosticInfoForDiagnostic(childNotes[i]); + auto child = + diagnosticInfoForDiagnostic(childNotes[i], + /* includeDiagnosticName= */ true); assert(child); assert(child->Kind == DiagnosticKind::Note && "Expected child diagnostics to all be notes?!"); @@ -1516,12 +1532,18 @@ DiagnosticKind DiagnosticEngine::declaredDiagnosticKindFor(const DiagID id) { return storedDiagnosticInfos[(unsigned)id].kind; } -llvm::StringRef DiagnosticEngine::diagnosticStringFor( - const DiagID id, PrintDiagnosticNamesMode printDiagnosticNamesMode) { +llvm::StringRef DiagnosticEngine::diagnosticStringFor(DiagID id) { llvm::StringRef message = diagnosticStrings[(unsigned)id]; if (auto localizationProducer = localization.get()) { message = localizationProducer->getMessageOr(id, message); } + return message; +} + +llvm::StringRef DiagnosticEngine::diagnosticStringWithNameFor( + DiagID id, DiagGroupID groupID, + PrintDiagnosticNamesMode printDiagnosticNamesMode) { + auto message = diagnosticStringFor(id); auto formatMessageWithName = [&](StringRef message, StringRef name) { const int additionalCharsLength = 3; // ' ', '[', ']' std::string messageWithName; @@ -1540,7 +1562,6 @@ llvm::StringRef DiagnosticEngine::diagnosticStringFor( message = formatMessageWithName(message, diagnosticIDStringFor(id)); break; case PrintDiagnosticNamesMode::Group: - auto groupID = storedDiagnosticInfos[(unsigned)id].groupID; if (groupID != DiagGroupID::no_group) { message = formatMessageWithName(message, getDiagGroupInfoByID(groupID).name); diff --git a/lib/IDE/CodeCompletionDiagnostics.cpp b/lib/IDE/CodeCompletionDiagnostics.cpp index a399109dc35aa..1928245d963c6 100644 --- a/lib/IDE/CodeCompletionDiagnostics.cpp +++ b/lib/IDE/CodeCompletionDiagnostics.cpp @@ -67,7 +67,7 @@ bool CodeCompletionDiagnostics::getDiagnostics( typename swift::detail::PassArgument::type... VArgs) { DiagID id = ID.ID; std::vector DiagArgs{std::move(VArgs)...}; - auto format = Engine.diagnosticStringFor(id, PrintDiagnosticNamesMode::None); + auto format = Engine.diagnosticStringFor(id); DiagnosticEngine::formatDiagnosticText(Out, format, DiagArgs); severity = getSeverity(Engine.declaredDiagnosticKindFor(id)); diff --git a/lib/PrintAsClang/ModuleContentsWriter.cpp b/lib/PrintAsClang/ModuleContentsWriter.cpp index ec09f2ca97bd5..5deec8caf3346 100644 --- a/lib/PrintAsClang/ModuleContentsWriter.cpp +++ b/lib/PrintAsClang/ModuleContentsWriter.cpp @@ -977,8 +977,8 @@ class ModuleWriter { const_cast(vd)); // Emit a specific unavailable message when we know why a decl can't be // exposed, or a generic message otherwise. - auto diagString = M.getASTContext().Diags.diagnosticStringFor( - diag.getID(), PrintDiagnosticNamesMode::None); + auto diagString = + M.getASTContext().Diags.diagnosticStringFor(diag.getID()); DiagnosticEngine::formatDiagnosticText(os, diagString, diag.getArgs(), DiagnosticFormatOptions()); os << "\");\n"; From d8435e46520ec5b95181f93a36bb7de14fa00403 Mon Sep 17 00:00:00 2001 From: Anthony Latsis Date: Thu, 2 Jan 2025 08:52:47 +0000 Subject: [PATCH 2/3] DiagnosticEngine: Fix escalation for wrapped diagnostics Wrapped diagnostics were not escalated to errors because the check was based on the diagnostic ID, which is that of the wrapper diagnostic in this case. Switch to tracking whether escalation was enabled for a given group instead. --- include/swift/AST/DiagnosticEngine.h | 29 +++- include/swift/AST/DiagnosticGroups.h | 2 +- lib/AST/DiagnosticEngine.cpp | 8 +- unittests/AST/CMakeLists.txt | 1 + unittests/AST/DiagnosticGroupsTests.cpp | 198 ++++++++++++++++++++++++ 5 files changed, 225 insertions(+), 13 deletions(-) create mode 100644 unittests/AST/DiagnosticGroupsTests.cpp diff --git a/include/swift/AST/DiagnosticEngine.h b/include/swift/AST/DiagnosticEngine.h index f0c1f1fff4751..7a46ceab41a57 100644 --- a/include/swift/AST/DiagnosticEngine.h +++ b/include/swift/AST/DiagnosticEngine.h @@ -513,11 +513,17 @@ namespace swift { friend DiagnosticEngine; friend class InFlightDiagnostic; - Diagnostic(DiagID ID, DiagGroupID GroupID) : ID(ID), GroupID(GroupID) {} - /// Constructs a Diagnostic with DiagGroupID infered from DiagID. Diagnostic(DiagID ID); + protected: + /// Only use this constructor privately in this class or in unit tests by + /// subclassing. + /// In unit tests, it is used as a means for associating diagnostics with + /// groups and, thus, circumventing the need to otherwise define mock + /// diagnostics, which is not accounted for in the current design. + Diagnostic(DiagID ID, DiagGroupID GroupID) : ID(ID), GroupID(GroupID) {} + public: // All constructors are intentionally implicit. template @@ -903,7 +909,9 @@ namespace swift { /// Don't emit any remarks bool suppressRemarks = false; - /// Treat these warnings as errors. Indices here correspond to DiagID enum + /// A mapping from `DiagGroupID` identifiers to Boolean values indicating + /// whether warnings belonging to the respective diagnostic groups should be + /// escalated to errors. llvm::BitVector warningsAsErrors; /// Whether a fatal error has occurred @@ -945,16 +953,23 @@ namespace swift { void setSuppressRemarks(bool val) { suppressRemarks = val; } bool getSuppressRemarks() const { return suppressRemarks; } - /// Whether a warning should be upgraded to an error or not - void setWarningAsErrorForDiagID(DiagID id, bool value) { + /// Sets whether warnings belonging to the diagnostic group identified by + /// `id` should be escalated to errors. + void setWarningsAsErrorsForDiagGroupID(DiagGroupID id, bool value) { warningsAsErrors[(unsigned)id] = value; } - bool getWarningAsErrorForDiagID(DiagID id) { + + /// Returns a Boolean value indicating whether warnings belonging to the + /// diagnostic group identified by `id` should be escalated to errors. + bool getWarningsAsErrorsForDiagGroupID(DiagGroupID id) { return warningsAsErrors[(unsigned)id]; } - /// Whether all warnings should be upgraded to errors or not + /// Whether all warnings should be upgraded to errors or not. void setAllWarningsAsErrors(bool value) { + // This works as intended because every diagnostic belongs to either a + // custom group or the top-level `DiagGroupID::no_group`, which is also + // a group. if (value) { warningsAsErrors.set(); } else { diff --git a/include/swift/AST/DiagnosticGroups.h b/include/swift/AST/DiagnosticGroups.h index 744d4911594b0..ff8908c638546 100644 --- a/include/swift/AST/DiagnosticGroups.h +++ b/include/swift/AST/DiagnosticGroups.h @@ -33,7 +33,7 @@ enum class DiagGroupID : uint16_t { constexpr const auto DiagGroupsCount = [] { size_t count = 0; -#define GROUP(Name, Version) count++; +#define GROUP(Name, Version) ++count; #include "DiagnosticGroups.def" return count; }(); diff --git a/lib/AST/DiagnosticEngine.cpp b/lib/AST/DiagnosticEngine.cpp index 80d476856e8bf..c840fec4ec337 100644 --- a/lib/AST/DiagnosticEngine.cpp +++ b/lib/AST/DiagnosticEngine.cpp @@ -176,7 +176,7 @@ DiagnosticState::DiagnosticState() { // Initialize our ignored diagnostics to default ignoredDiagnostics.resize(LocalDiagID::NumDiags); // Initialize warningsAsErrors to default - warningsAsErrors.resize(LocalDiagID::NumDiags); + warningsAsErrors.resize(DiagGroupsCount); } Diagnostic::Diagnostic(DiagID ID) @@ -551,9 +551,7 @@ void DiagnosticEngine::setWarningsAsErrorsRules( if (auto groupID = getDiagGroupIDByName(name); groupID && *groupID != DiagGroupID::no_group) { getDiagGroupInfoByID(*groupID).traverseDepthFirst([&](auto group) { - for (DiagID diagID : group.diagnostics) { - state.setWarningAsErrorForDiagID(diagID, isEnabled); - } + state.setWarningsAsErrorsForDiagGroupID(*groupID, isEnabled); }); } else { unknownGroups.push_back(std::string(name)); @@ -1232,7 +1230,7 @@ DiagnosticBehavior DiagnosticState::determineBehavior(const Diagnostic &diag) { // 4) If the user substituted a different behavior for this behavior, apply // that change if (lvl == DiagnosticBehavior::Warning) { - if (getWarningAsErrorForDiagID(diag.getID())) + if (getWarningsAsErrorsForDiagGroupID(diag.getGroupID())) lvl = DiagnosticBehavior::Error; if (suppressWarnings) lvl = DiagnosticBehavior::Ignore; diff --git a/unittests/AST/CMakeLists.txt b/unittests/AST/CMakeLists.txt index dcc91733edc6d..7e3b3305e8971 100644 --- a/unittests/AST/CMakeLists.txt +++ b/unittests/AST/CMakeLists.txt @@ -4,6 +4,7 @@ add_swift_unittest(SwiftASTTests ASTWalkerTests.cpp IndexSubsetTests.cpp DiagnosticConsumerTests.cpp + DiagnosticGroupsTests.cpp SourceLocTests.cpp TestContext.cpp TypeMatchTests.cpp diff --git a/unittests/AST/DiagnosticGroupsTests.cpp b/unittests/AST/DiagnosticGroupsTests.cpp new file mode 100644 index 0000000000000..395d9bb26797f --- /dev/null +++ b/unittests/AST/DiagnosticGroupsTests.cpp @@ -0,0 +1,198 @@ +//===--- DiagnosticGroupsTests.cpp ----------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2025 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// +// +// NB: The correctness of the diagnostic group graph is verified in lib/AST +// ('namespace validation'). +// +//===----------------------------------------------------------------------===// + +#include "swift/AST/DiagnosticGroups.h" +#include "swift/AST/DiagnosticEngine.h" +#include "swift/AST/DiagnosticsFrontend.h" +#include "swift/Basic/SourceManager.h" +#include "gtest/gtest.h" + +using namespace swift; + +namespace { +class TestDiagnosticConsumer : public DiagnosticConsumer { + llvm::function_ref callback; + +public: + TestDiagnosticConsumer(decltype(callback) callback) : callback(callback) {} + + void handleDiagnostic(SourceManager &SM, + const DiagnosticInfo &Info) override { + this->callback(Info); + } +}; + +struct TestDiagnostic : public Diagnostic { + TestDiagnostic(DiagID ID, DiagGroupID GroupID) : Diagnostic(ID, GroupID) {} +}; +} // end anonymous namespace + +static void diagnosticGroupsTestCase( + llvm::function_ref diagnose, + llvm::function_ref callback, + unsigned expectedNumCallbackCalls) { + SourceManager sourceMgr; + DiagnosticEngine diags(sourceMgr); + + unsigned count = 0; + + const auto countingCallback = [&](const DiagnosticInfo &info) { + ++count; + callback(info); + }; + + TestDiagnosticConsumer consumer(countingCallback); + diags.addConsumer(consumer); + diagnose(diags); + diags.removeConsumer(consumer); + + EXPECT_EQ(count, expectedNumCallbackCalls); +} + +TEST(DiagnosticGroups, PrintDiagnosticGroups) { + // Test that we append the correct group in the format string. + diagnosticGroupsTestCase( + [](DiagnosticEngine &diags) { + diags.setPrintDiagnosticNamesMode(PrintDiagnosticNamesMode::Group); + + TestDiagnostic diagnostic(diag::error_immediate_mode_missing_stdlib.ID, + DiagGroupID::DeprecatedDeclaration); + diags.diagnose(SourceLoc(), diagnostic); + }, + [](const DiagnosticInfo &info) { + EXPECT_TRUE(info.FormatString.ends_with(" [DeprecatedDeclaration]")); + }, + /*expectedNumCallbackCalls=*/1); + + diagnosticGroupsTestCase( + [](DiagnosticEngine &diags) { + diags.setPrintDiagnosticNamesMode(PrintDiagnosticNamesMode::Group); + + TestDiagnostic diagnostic(diag::error_immediate_mode_missing_stdlib.ID, + DiagGroupID::no_group); + diags.diagnose(SourceLoc(), diagnostic); + }, + [](const DiagnosticInfo &info) { + EXPECT_FALSE(info.FormatString.ends_with("]")); + }, + /*expectedNumCallbackCalls=*/1); +} + +TEST(DiagnosticGroups, DiagnosticsWrappersInheritGroups) { + // Test that we don't loose the group of a diagnostic when it gets wrapped in + // another one. + diagnosticGroupsTestCase( + [](DiagnosticEngine &diags) { + diags.setPrintDiagnosticNamesMode(PrintDiagnosticNamesMode::Group); + + TestDiagnostic diagnostic(diag::error_immediate_mode_missing_stdlib.ID, + DiagGroupID::DeprecatedDeclaration); + diags.diagnose(SourceLoc(), diagnostic) + .limitBehaviorUntilSwiftVersion(DiagnosticBehavior::Warning, 99); + }, + [](const DiagnosticInfo &info) { + EXPECT_EQ(info.ID, diag::error_in_a_future_swift_version.ID); + EXPECT_TRUE(info.FormatString.ends_with(" [DeprecatedDeclaration]")); + }, + /*expectedNumCallbackCalls=*/1); +} + +TEST(DiagnosticGroups, TargetAll) { + // Test that uncategorized diagnostics are escalated when escalating all + // warnings. + diagnosticGroupsTestCase( + [](DiagnosticEngine &diags) { + const std::vector rules = { + WarningAsErrorRule(WarningAsErrorRule::Action::Enable)}; + diags.setWarningsAsErrorsRules(rules); + + TestDiagnostic diagnostic( + diag::warn_unsupported_module_interface_library_evolution.ID, + DiagGroupID::no_group); + diags.diagnose(SourceLoc(), diagnostic); + }, + [](const DiagnosticInfo &info) { + EXPECT_EQ(info.Kind, DiagnosticKind::Error); + }, + /*expectedNumCallbackCalls=*/1); +} + +TEST(DiagnosticGroups, OverrideBehaviorLimitations) { + // Test that escalating warnings to errors for *errors* in a diagnostic group + // overrides emission site behavior limitations. + { + TestDiagnostic diagnostic(diag::error_immediate_mode_missing_stdlib.ID, + DiagGroupID::DeprecatedDeclaration); + + // Make sure ID actually is an error by default. + diagnosticGroupsTestCase( + [&diagnostic](DiagnosticEngine &diags) { + diags.diagnose(SourceLoc(), diagnostic); + }, + [](const DiagnosticInfo &info) { + EXPECT_TRUE(info.Kind == DiagnosticKind::Error); + }, + /*expectedNumCallbackCalls=*/1); + + diagnosticGroupsTestCase( + [&diagnostic](DiagnosticEngine &diags) { + const std::vector rules = {WarningAsErrorRule( + WarningAsErrorRule::Action::Enable, "DeprecatedDeclaration")}; + diags.setWarningsAsErrorsRules(rules); + + diags.diagnose(SourceLoc(), diagnostic); + diags.diagnose(SourceLoc(), diagnostic) + .limitBehaviorUntilSwiftVersion(DiagnosticBehavior::Warning, 99); + }, + [](const DiagnosticInfo &info) { + EXPECT_EQ(info.Kind, DiagnosticKind::Error); + }, + /*expectedNumCallbackCalls=*/2); + } + + // Test that escalating warnings to errors for *warnings* in a diagnostic + // group overrides emission site behavior limitations. + { + TestDiagnostic diagnostic( + diag::warn_unsupported_module_interface_library_evolution.ID, + DiagGroupID::DeprecatedDeclaration); + + // Make sure ID actually is a warning by default. + diagnosticGroupsTestCase( + [&diagnostic](DiagnosticEngine &diags) { + diags.diagnose(SourceLoc(), diagnostic); + }, + [](const DiagnosticInfo &info) { + EXPECT_EQ(info.Kind, DiagnosticKind::Warning); + }, + /*expectedNumCallbackCalls=*/1); + + diagnosticGroupsTestCase( + [&diagnostic](DiagnosticEngine &diags) { + const std::vector rules = {WarningAsErrorRule( + WarningAsErrorRule::Action::Enable, "DeprecatedDeclaration")}; + diags.setWarningsAsErrorsRules(rules); + + diags.diagnose(SourceLoc(), diagnostic) + .limitBehaviorUntilSwiftVersion(DiagnosticBehavior::Warning, 99); + }, + [](const DiagnosticInfo &info) { + EXPECT_EQ(info.Kind, DiagnosticKind::Error); + }, + /*expectedNumCallbackCalls=*/1); + } +} From 6517bced361f5259ebd57e9047888de03c7793f6 Mon Sep 17 00:00:00 2001 From: Anthony Latsis Date: Wed, 8 Jan 2025 20:30:09 +0000 Subject: [PATCH 3/3] DiagnosticEngine: Fix a `Diagnostic` ctor that skipped group computation --- include/swift/AST/DiagnosticEngine.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/swift/AST/DiagnosticEngine.h b/include/swift/AST/DiagnosticEngine.h index 7a46ceab41a57..50623747f267f 100644 --- a/include/swift/AST/DiagnosticEngine.h +++ b/include/swift/AST/DiagnosticEngine.h @@ -534,8 +534,9 @@ namespace swift { gatherArgs(VArgs...); } - /*implicit*/Diagnostic(DiagID ID, ArrayRef Args) - : ID(ID), Args(Args.begin(), Args.end()) {} + Diagnostic(DiagID ID, ArrayRef Args) : Diagnostic(ID) { + this->Args.append(Args.begin(), Args.end()); + } template static Diagnostic fromTuple(Diag id,