Skip to content

Add DiagGroupID to Diagnostic #77928

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
Dec 4, 2024
Merged

Add DiagGroupID to Diagnostic #77928

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
24 changes: 19 additions & 5 deletions include/swift/AST/DiagnosticEngine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#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 @@ -498,6 +499,7 @@ namespace swift {

private:
DiagID ID;
DiagGroupID GroupID;
SmallVector<DiagnosticArgument, 3> Args;
SmallVector<CharSourceRange, 2> Ranges;
SmallVector<FixIt, 2> FixIts;
Expand All @@ -510,7 +512,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.
Expand All @@ -535,6 +540,7 @@ namespace swift {

// Accessors.
DiagID getID() const { return ID; }
DiagGroupID getGroupID() const { return GroupID; }
ArrayRef<DiagnosticArgument> getArgs() const { return Args; }
ArrayRef<CharSourceRange> getRanges() const { return Ranges; }
ArrayRef<FixIt> getFixIts() const { return FixIts; }
Expand Down Expand Up @@ -1434,7 +1440,8 @@ namespace swift {

/// Generate DiagnosticInfo for a Diagnostic to be passed to consumers.
std::optional<DiagnosticInfo>
diagnosticInfoForDiagnostic(const Diagnostic &diagnostic);
diagnosticInfoForDiagnostic(const Diagnostic &diagnostic,
bool includeDiagnosticName);

/// Send \c diag to all diagnostic consumers.
void emitDiagnostic(const Diagnostic &diag);
Expand All @@ -1460,9 +1467,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);

Expand Down
1 change: 1 addition & 0 deletions include/swift/AST/DiagnosticGroups.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ GROUP(no_group, "")
GROUP(DeprecatedDeclaration, "DeprecatedDeclaration.md")
GROUP(Unsafe, "Unsafe.md")
GROUP(UnknownWarningGroup, "UnknownWarningGroup.md")
GROUP(DeclarationUnavailableFromAsynchronousContext, "DeclarationUnavailableFromAsynchronousContext.md")
Copy link
Collaborator

Choose a reason for hiding this comment

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

As far as I am concerned, we cannot afford to remove a group once it ships, which calls for very mindful additions. If this group is purely intended to support test coverage, we should establish and document mechanisms for defining and conveying its experimental or internal status. Otherwise, I don’t think it wise to modify the group tree without at least an approving review from the Language Steering Group, if not a proposal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was hoping to gather all the concurrency related groups and start a thread on the forum. Unfortunately, I didn't find enough time to do that. This commit was merged after the 6.1 release was branched. So we can still remove this group for now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have a few ideas about how to make this a unit test. I will tag you in the PR.


#define UNDEFINE_DIAGNOSTIC_GROUPS_MACROS
#include "swift/AST/DefineDiagnosticGroupsMacros.h"
4 changes: 4 additions & 0 deletions include/swift/AST/DiagnosticsCommon.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 *))
Expand Down
2 changes: 1 addition & 1 deletion include/swift/AST/DiagnosticsSema.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5950,7 +5950,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 *))
ERROR(async_unavailable_decl,none,
GROUPED_ERROR(async_unavailable_decl,DeclarationUnavailableFromAsynchronousContext,none,
"%kindbase0 is unavailable from asynchronous contexts%select{|; %1}1",
(const ValueDecl *, StringRef))

Expand Down
49 changes: 35 additions & 14 deletions lib/AST/DiagnosticEngine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down Expand Up @@ -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);

Expand All @@ -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);
Expand Down Expand Up @@ -1294,7 +1298,8 @@ void DiagnosticEngine::forwardTentativeDiagnosticsTo(
}

std::optional<DiagnosticInfo>
DiagnosticEngine::diagnosticInfoForDiagnostic(const Diagnostic &diagnostic) {
DiagnosticEngine::diagnosticInfoForDiagnostic(const Diagnostic &diagnostic,
bool includeDiagnosticName) {
auto behavior = state.determineBehavior(diagnostic);
if (behavior == DiagnosticBehavior::Ignore)
return std::nullopt;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1462,7 +1474,9 @@ void DiagnosticEngine::emitDiagnostic(const Diagnostic &diagnostic) {
ArrayRef<Diagnostic> childNotes = diagnostic.getChildNotes();
std::vector<Diagnostic> 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
Expand All @@ -1478,7 +1492,9 @@ void DiagnosticEngine::emitDiagnostic(const Diagnostic &diagnostic) {

SmallVector<DiagnosticInfo, 1> 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?!");
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion lib/IDE/CodeCompletionDiagnostics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ bool CodeCompletionDiagnostics::getDiagnostics(
typename swift::detail::PassArgument<ArgTypes>::type... VArgs) {
DiagID id = ID.ID;
std::vector<DiagnosticArgument> 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));

Expand Down
4 changes: 2 additions & 2 deletions lib/PrintAsClang/ModuleContentsWriter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -977,8 +977,8 @@ class ModuleWriter {
const_cast<ValueDecl *>(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";
Expand Down
26 changes: 13 additions & 13 deletions test/attr/attr_availability_noasync.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %target-typecheck-verify-swift
// RUN: %target-typecheck-verify-swift -print-diagnostic-groups

// 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}}
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}
basicNoAsync()

// expected-warning@+1{{global function 'messageNoAsync' is unavailable from asynchronous contexts; a message from the author}}
// 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]}}
messageNoAsync()

// expected-warning@+1{{global function 'renamedNoAsync' is unavailable from asynchronous contexts}}{{5-19=asyncReplacement}}
// 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}}
renamedNoAsync() { _ in }

// expected-warning@+1{{global function 'readStringFromIO' is unavailable from asynchronous contexts}}{{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 [DeclarationUnavailableFromAsynchronousContext]}}{{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}}
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}
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}}
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}
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}}
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}
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}}
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}
defer { basicNoAsync() }
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode}}
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}
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}}
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}
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}}
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}
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}}
// expected-warning@+1{{global function 'basicNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode [DeclarationUnavailableFromAsynchronousContext]}}
defer { basicNoAsync() }
return ()
}
Expand Down
18 changes: 18 additions & 0 deletions userdocs/diagnostic_groups/DeclarationUnavailableFromAsynchronousContext.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# `DeclarationUnavailableFromAsynchronousContext`

This diagnostic group includes errors and warnings produced by the compiler when attempting to call a function from an asynchronous context that is marked as unavailable in an asynchronous context.

```swift
@available(*, noasync)
func fNoAsync() {}

// Swift 6 language mode
func test() async {
fNoAsync() // error: global function 'fNoAsync' is unavailable from asynchronous contexts
}

// Swift 5 language mode
func test() async {
fNoAsync() // warning: global function 'fNoAsync' is unavailable from asynchronous contexts; this is an error in the Swift 6 language mode
}
```