Skip to content

DiagnosticEngine: Fix escalation for wrapped diagnostics #78398

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.

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 4 commits into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
37 changes: 27 additions & 10 deletions include/swift/AST/DiagnosticEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include "swift/AST/ActorIsolation.h"
#include "swift/AST/DeclNameLoc.h"
#include "swift/AST/DiagnosticConsumer.h"
#include "swift/AST/DiagnosticGroups.h"
#include "swift/AST/TypeLoc.h"
#include "swift/Basic/PrintDiagnosticNamesMode.h"
#include "swift/Basic/Statistic.h"
Expand Down Expand Up @@ -68,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
Expand Down Expand Up @@ -512,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<typename ...ArgTypes>
Expand All @@ -527,8 +534,9 @@ namespace swift {
gatherArgs(VArgs...);
}

/*implicit*/Diagnostic(DiagID ID, ArrayRef<DiagnosticArgument> Args)
: ID(ID), Args(Args.begin(), Args.end()) {}
Diagnostic(DiagID ID, ArrayRef<DiagnosticArgument> Args) : Diagnostic(ID) {
this->Args.append(Args.begin(), Args.end());
}

template <class... ArgTypes>
static Diagnostic fromTuple(Diag<ArgTypes...> id,
Expand Down Expand Up @@ -902,7 +910,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
Expand Down Expand Up @@ -944,16 +954,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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It took me a moment to understand why it's a valid change for the case when we apply WarningAsErrorRule with TargetAll. But it seems it's ok as all the uncategorized warnings have the no_group group which will be stored in this BitVector like the others.
I'd leave a comment about it though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Will add a test too.

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 {
Expand Down
1 change: 0 additions & 1 deletion include/swift/AST/DiagnosticGroups.def
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ GROUP(no_group, "")
GROUP(DeprecatedDeclaration, "DeprecatedDeclaration.md")
GROUP(Unsafe, "Unsafe.md")
GROUP(UnknownWarningGroup, "UnknownWarningGroup.md")
GROUP(DeclarationUnavailableFromAsynchronousContext, "DeclarationUnavailableFromAsynchronousContext.md")

#define UNDEFINE_DIAGNOSTIC_GROUPS_MACROS
#include "swift/AST/DefineDiagnosticGroupsMacros.h"
2 changes: 1 addition & 1 deletion include/swift/AST/DiagnosticGroups.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}();
Expand Down
2 changes: 1 addition & 1 deletion include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -5963,7 +5963,7 @@ ERROR(async_decl_must_be_available_from_async,none,
ERROR(async_named_decl_must_be_available_from_async,none,
"asynchronous %kind0 must be available from asynchronous contexts",
(const ValueDecl *))
GROUPED_ERROR(async_unavailable_decl,DeclarationUnavailableFromAsynchronousContext,none,
ERROR(async_unavailable_decl,none,
"%kindbase0 is unavailable from asynchronous contexts%select{|; %1}1",
(const ValueDecl *, StringRef))

Expand Down
8 changes: 3 additions & 5 deletions lib/AST/DiagnosticEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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;
Expand Down
26 changes: 13 additions & 13 deletions test/attr/attr_availability_noasync.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %target-typecheck-verify-swift -print-diagnostic-groups
// RUN: %target-typecheck-verify-swift

// REQUIRES: concurrency

Expand Down Expand Up @@ -27,16 +27,16 @@ actor IOActor {

@available(SwiftStdlib 5.5, *)
func asyncFunc() async {
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}
basicNoAsync()

// expected-warning@+1{{global function 'messageNoAsync' is unavailable from asynchronous contexts; a message from the author; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}
// expected-warning@+1{{global function 'messageNoAsync' is unavailable from asynchronous contexts; a message from the author; this is an error in the Swift 6 language mode}}
messageNoAsync()

// expected-warning@+1{{global function 'renamedNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}{{5-19=asyncReplacement}}
// expected-warning@+1{{global function 'renamedNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}{{5-19=asyncReplacement}}
renamedNoAsync() { _ in }

// expected-warning@+1{{global function 'readStringFromIO' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}{{13-29=IOActor.readString}}
// expected-warning@+1{{global function 'readStringFromIO' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}{{13-29=IOActor.readString}}
let _ = readStringFromIO()
}

Expand Down Expand Up @@ -76,7 +76,7 @@ func test_defers_sync() {
}

func local_async_func() async {
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}
defer { basicNoAsync() }
_ = ()
}
Expand All @@ -89,7 +89,7 @@ func test_defers_sync() {

// local async closure
let local_async_closure = { () async -> Void in
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}
defer { basicNoAsync() }
_ = ()
}
Expand All @@ -102,7 +102,7 @@ func test_defers_sync() {

var local_async_var: Void {
get async {
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}
defer { basicNoAsync() }
return ()
}
Expand All @@ -112,9 +112,9 @@ func test_defers_sync() {
@available(SwiftStdlib 5.5, *)
func test_defer_async() async {
defer {
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}
defer { basicNoAsync() }
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}
basicNoAsync()
}

Expand All @@ -124,7 +124,7 @@ func test_defer_async() async {
}

func local_async_func() async {
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}
defer { basicNoAsync() }
_ = ()
}
Expand All @@ -136,7 +136,7 @@ func test_defer_async() async {
_ = local_sync_closure

let local_async_closure = { () async -> Void in
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}
defer { basicNoAsync() }
_ = ()
}
Expand All @@ -149,7 +149,7 @@ func test_defer_async() async {

var local_async_var: Void {
get async {
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}
defer { basicNoAsync() }
return ()
}
Expand Down
1 change: 1 addition & 0 deletions unittests/AST/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ add_swift_unittest(SwiftASTTests
ASTWalkerTests.cpp
IndexSubsetTests.cpp
DiagnosticConsumerTests.cpp
DiagnosticGroupsTests.cpp
SourceLocTests.cpp
TestContext.cpp
TypeMatchTests.cpp
Expand Down
Loading