Skip to content

[6.2] [Sema] Downgrade noasync diagnostic to warning for closure macro args #80910

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
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
11 changes: 11 additions & 0 deletions include/swift/AST/DiagnosticEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,17 @@ namespace swift {
return limitBehaviorUntilSwiftVersion(limit, languageMode);
}

/// Limit the diagnostic behavior to warning until the next future
/// language mode.
///
/// This should be preferred over passing the next major version to
/// `warnUntilSwiftVersion` to make it easier to find and update clients
/// when a new language mode is introduced.
///
/// This helps stage in fixes for stricter diagnostics as warnings
/// until the next major language version.
InFlightDiagnostic &warnUntilFutureSwiftVersion();

/// Limit the diagnostic behavior to warning until the specified version.
///
/// This helps stage in fixes for stricter diagnostics as warnings
Expand Down
21 changes: 19 additions & 2 deletions include/swift/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ class alignas(8) Expr : public ASTAllocated<Expr> {
Kind : 2
);

SWIFT_INLINE_BITFIELD(ClosureExpr, AbstractClosureExpr, 1+1+1+1+1+1+1,
SWIFT_INLINE_BITFIELD(ClosureExpr, AbstractClosureExpr, 1+1+1+1+1+1+1+1,
/// True if closure parameters were synthesized from anonymous closure
/// variables.
HasAnonymousClosureVars : 1,
Expand Down Expand Up @@ -295,7 +295,12 @@ class alignas(8) Expr : public ASTAllocated<Expr> {
/// isolation checks when it either specifies or inherits isolation
/// and was passed as an argument to a function that is not fully
/// concurrency checked.
RequiresDynamicIsolationChecking : 1
RequiresDynamicIsolationChecking : 1,

/// Whether this closure was type-checked as an argument to a macro. This
/// is only populated after type-checking, and only exists for diagnostic
/// logic. Do not add more uses of this.
IsMacroArgument : 1
);

SWIFT_INLINE_BITFIELD_FULL(BindOptionalExpr, Expr, 16,
Expand Down Expand Up @@ -4316,6 +4321,7 @@ class ClosureExpr : public AbstractClosureExpr {
Bits.ClosureExpr.IsPassedToSendingParameter = false;
Bits.ClosureExpr.NoGlobalActorAttribute = false;
Bits.ClosureExpr.RequiresDynamicIsolationChecking = false;
Bits.ClosureExpr.IsMacroArgument = false;
}

SourceRange getSourceRange() const;
Expand Down Expand Up @@ -4394,6 +4400,17 @@ class ClosureExpr : public AbstractClosureExpr {
Bits.ClosureExpr.RequiresDynamicIsolationChecking = value;
}

/// Whether this closure was type-checked as an argument to a macro. This
/// is only populated after type-checking, and only exists for diagnostic
/// logic. Do not add more uses of this.
bool isMacroArgument() const {
return Bits.ClosureExpr.IsMacroArgument;
}

void setIsMacroArgument(bool value = true) {
Bits.ClosureExpr.IsMacroArgument = value;
}

/// Determine whether this closure expression has an
/// explicitly-specified result type.
bool hasExplicitResultType() const { return ArrowLoc.isValid(); }
Expand Down
7 changes: 7 additions & 0 deletions include/swift/Basic/Version.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,13 @@ class Version {
/// SWIFT_VERSION_MINOR.
static Version getCurrentLanguageVersion();

/// Returns a major version to represent the next future language mode. This
/// exists to make it easier to find and update clients when a new language
/// mode is added.
static constexpr unsigned getFutureMajorLanguageVersion() {
return 7;
}

// List of backward-compatibility versions that we permit passing as
// -swift-version <vers>
static std::array<StringRef, 4> getValidEffectiveVersions() {
Expand Down
7 changes: 6 additions & 1 deletion lib/AST/DiagnosticEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ InFlightDiagnostic::limitBehaviorUntilSwiftVersion(
// version. We do this before limiting the behavior, because
// wrapIn will result in the behavior of the wrapping diagnostic.
if (limit >= DiagnosticBehavior::Warning) {
if (majorVersion > 6) {
if (majorVersion >= version::Version::getFutureMajorLanguageVersion()) {
wrapIn(diag::error_in_a_future_swift_lang_mode);
} else {
wrapIn(diag::error_in_swift_lang_mode, majorVersion);
Expand All @@ -472,6 +472,11 @@ InFlightDiagnostic::limitBehaviorUntilSwiftVersion(
return *this;
}

InFlightDiagnostic &InFlightDiagnostic::warnUntilFutureSwiftVersion() {
using namespace version;
return warnUntilSwiftVersion(Version::getFutureMajorLanguageVersion());
}

InFlightDiagnostic &
InFlightDiagnostic::warnUntilSwiftVersion(unsigned majorVersion) {
return limitBehaviorUntilSwiftVersion(DiagnosticBehavior::Warning,
Expand Down
17 changes: 10 additions & 7 deletions lib/Basic/Version.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -181,16 +181,19 @@ std::optional<Version> Version::getEffectiveLanguageVersion() const {
static_assert(SWIFT_VERSION_MAJOR == 6,
"getCurrentLanguageVersion is no longer correct here");
return Version::getCurrentLanguageVersion();
case 7:
// Allow version '7' in asserts compilers *only* so that we can start
// testing changes planned for after Swift 6. Note that it's still not
// listed in `Version::getValidEffectiveVersions()`.
// FIXME: When Swift 7 becomes real, remove 'REQUIRES: swift7' from tests
// using '-swift-version 7'.

// FIXME: When Swift 7 becomes real, remove 'REQUIRES: swift7' from tests
// using '-swift-version 7'.

case Version::getFutureMajorLanguageVersion():
// Allow the future language mode version in asserts compilers *only* so
// that we can start testing changes planned for after the current latest
// language mode. Note that it'll not be listed in
// `Version::getValidEffectiveVersions()`.
#ifdef NDEBUG
LLVM_FALLTHROUGH;
#else
return Version{7};
return Version{Version::getFutureMajorLanguageVersion()};
#endif
default:
return std::nullopt;
Expand Down
17 changes: 12 additions & 5 deletions lib/Sema/CSApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6053,28 +6053,32 @@ static bool hasCurriedSelf(ConstraintSystem &cs, ConcreteDeclRef callee,
static void applyContextualClosureFlags(Expr *expr, bool implicitSelfCapture,
bool inheritActorContext,
bool isPassedToSendingParameter,
bool requiresDynamicIsolationChecking) {
bool requiresDynamicIsolationChecking,
bool isMacroArg) {
if (auto closure = dyn_cast<ClosureExpr>(expr)) {
closure->setAllowsImplicitSelfCapture(implicitSelfCapture);
closure->setInheritsActorContext(inheritActorContext);
closure->setIsPassedToSendingParameter(isPassedToSendingParameter);
closure->setRequiresDynamicIsolationChecking(
requiresDynamicIsolationChecking);
closure->setIsMacroArgument(isMacroArg);
return;
}

if (auto captureList = dyn_cast<CaptureListExpr>(expr)) {
applyContextualClosureFlags(captureList->getClosureBody(),
implicitSelfCapture, inheritActorContext,
isPassedToSendingParameter,
requiresDynamicIsolationChecking);
requiresDynamicIsolationChecking,
isMacroArg);
}

if (auto identity = dyn_cast<IdentityExpr>(expr)) {
applyContextualClosureFlags(identity->getSubExpr(), implicitSelfCapture,
inheritActorContext,
isPassedToSendingParameter,
requiresDynamicIsolationChecking);
requiresDynamicIsolationChecking,
isMacroArg);
}
}

Expand Down Expand Up @@ -6199,19 +6203,22 @@ ArgumentList *ExprRewriter::coerceCallArguments(
}();

auto applyFlagsToArgument = [&paramInfo,
&closuresRequireDynamicIsolationChecking](
&closuresRequireDynamicIsolationChecking,
&locator](
unsigned paramIdx, Expr *argument) {
if (!isClosureLiteralExpr(argument))
return;

bool isImplicitSelfCapture = paramInfo.isImplicitSelfCapture(paramIdx);
bool inheritsActorContext = paramInfo.inheritsActorContext(paramIdx);
bool isPassedToSendingParameter = paramInfo.isSendingParameter(paramIdx);
bool isMacroArg = isExpr<MacroExpansionExpr>(locator.getAnchor());

applyContextualClosureFlags(argument, isImplicitSelfCapture,
inheritsActorContext,
isPassedToSendingParameter,
closuresRequireDynamicIsolationChecking);
closuresRequireDynamicIsolationChecking,
isMacroArg);
};

// Quickly test if any further fix-ups for the argument types are necessary.
Expand Down
11 changes: 6 additions & 5 deletions lib/Sema/MiscDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2243,11 +2243,12 @@ class ImplicitSelfUsageChecker : public BaseDiagnosticWalker {
invalidImplicitSelfShouldOnlyWarn510(base, closure)) {
warnUntilVersion.emplace(6);
}
// Prior to Swift 7, downgrade to a warning if we're in a macro to preserve
// compatibility with the Swift 6 diagnostic behavior where we previously
// skipped diagnosing.
if (!Ctx.isSwiftVersionAtLeast(7) && isInMacro())
warnUntilVersion.emplace(7);
// Prior to the next language mode, downgrade to a warning if we're in a
// macro to preserve compatibility with the Swift 6 diagnostic behavior
// where we previously skipped diagnosing.
auto futureVersion = version::Version::getFutureMajorLanguageVersion();
if (!Ctx.isSwiftVersionAtLeast(futureVersion) && isInMacro())
warnUntilVersion.emplace(futureVersion);

auto diag = Ctx.Diags.diagnose(loc, ID, std::move(Args)...);
if (warnUntilVersion)
Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/TypeCheckAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1249,7 +1249,7 @@ void AttributeChecker::visitAccessControlAttr(AccessControlAttr *attr) {
diagnose(attr->getLocation(),
diag::access_control_non_objc_open_member, VD)
.fixItReplace(attr->getRange(), "public")
.warnUntilSwiftVersion(7);
.warnUntilFutureSwiftVersion();
}
}
}
Expand Down
54 changes: 38 additions & 16 deletions lib/Sema/TypeCheckAvailability.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -276,16 +276,18 @@ static bool shouldAllowReferenceToUnavailableInSwiftDeclaration(
return false;
}

// Utility function to help determine if noasync diagnostics are still
// appropriate even if a `DeclContext` returns `false` from `isAsyncContext()`.
static bool shouldTreatDeclContextAsAsyncForDiagnostics(const DeclContext *DC) {
if (auto *D = DC->getAsDecl())
if (auto *FD = dyn_cast<FuncDecl>(D))
/// Retrieve the innermost DeclContext that should be consulted for noasync
/// checking.
static const DeclContext *
getInnermostDeclContextForNoAsync(const DeclContext *DC) {
if (auto *D = DC->getAsDecl()) {
if (auto *FD = dyn_cast<FuncDecl>(D)) {
if (FD->isDeferBody())
// If this is a defer body, we should delegate to its parent.
return shouldTreatDeclContextAsAsyncForDiagnostics(DC->getParent());

return DC->isAsyncContext();
return getInnermostDeclContextForNoAsync(DC->getParent());
}
}
return DC;
}

/// A class that walks the AST to find the innermost (i.e., deepest) node that
Expand Down Expand Up @@ -2758,7 +2760,8 @@ static bool
diagnoseDeclAsyncAvailability(const ValueDecl *D, SourceRange R,
const Expr *call, const ExportContext &Where) {
// If we are not in an (effective) async context, don't check it
if (!shouldTreatDeclContextAsAsyncForDiagnostics(Where.getDeclContext()))
auto *noAsyncDC = getInnermostDeclContextForNoAsync(Where.getDeclContext());
if (!noAsyncDC->isAsyncContext())
return false;

ASTContext &ctx = Where.getDeclContext()->getASTContext();
Expand All @@ -2774,14 +2777,27 @@ diagnoseDeclAsyncAvailability(const ValueDecl *D, SourceRange R,
}
}

// In Swift 6 we previously didn't coerce macro arguments to parameter types,
// so closure arguments may be treated as async in cases where they weren't in
// Swift 6. As such we need to warn if the use is within a closure macro
// argument until the next language mode.
auto shouldWarnUntilFutureVersion = [&]() {
auto *CE = dyn_cast<ClosureExpr>(noAsyncDC);
return CE && CE->isMacroArgument();
};

// @available(noasync) spelling
if (auto attr = D->getNoAsyncAttr()) {
SourceLoc diagLoc = call ? call->getLoc() : R.Start;
auto diag = ctx.Diags.diagnose(diagLoc, diag::async_unavailable_decl, D,
attr->getMessage());
diag.warnUntilSwiftVersion(6);
diag.limitBehaviorWithPreconcurrency(DiagnosticBehavior::Warning,
D->preconcurrency());
if (D->preconcurrency()) {
diag.limitBehavior(DiagnosticBehavior::Warning);
} else if (shouldWarnUntilFutureVersion()) {
diag.warnUntilFutureSwiftVersion();
} else {
diag.warnUntilSwiftVersion(6);
}

if (!attr->getRename().empty()) {
fixItAvailableAttrRename(diag, R, D, attr->getRename(), call);
Expand All @@ -2797,10 +2813,16 @@ diagnoseDeclAsyncAvailability(const ValueDecl *D, SourceRange R,
// @_unavailableFromAsync spelling
const UnavailableFromAsyncAttr *attr =
D->getAttrs().getAttribute<UnavailableFromAsyncAttr>();
SourceLoc diagLoc = call ? call->getLoc() : R.Start;
ctx.Diags
.diagnose(diagLoc, diag::async_unavailable_decl, D, attr->Message)
.warnUntilSwiftVersion(6);
{
SourceLoc diagLoc = call ? call->getLoc() : R.Start;
auto diag = ctx.Diags.diagnose(diagLoc, diag::async_unavailable_decl, D,
attr->Message);
if (shouldWarnUntilFutureVersion()) {
diag.warnUntilFutureSwiftVersion();
} else {
diag.warnUntilSwiftVersion(6);
}
}
D->diagnose(diag::decl_declared_here, D);
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/TypeCheckConcurrency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2676,7 +2676,7 @@ namespace {
fromType, toType);

if (downgradeToWarning)
diag.warnUntilSwiftVersion(7);
diag.warnUntilFutureSwiftVersion();
}

for (auto type : nonSendableTypes) {
Expand Down
3 changes: 2 additions & 1 deletion lib/Sema/TypeCheckProtocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5169,7 +5169,8 @@ static bool diagnoseTypeWitnessAvailability(
return false;

// In Swift 6 and earlier type witness availability diagnostics are warnings.
const unsigned warnBeforeVersion = 7;
using namespace version;
const unsigned warnBeforeVersion = Version::getFutureMajorLanguageVersion();
bool shouldError =
ctx.LangOpts.EffectiveLanguageVersion.isVersionAtLeast(warnBeforeVersion);

Expand Down
Loading