Skip to content
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
10 changes: 0 additions & 10 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -398,22 +398,12 @@ NOTE(generic_parameter_inferred_from_result_context,none,
ERROR(cannot_pass_type_to_non_ephemeral,none,
"cannot pass %0 to parameter; argument %1 must be a pointer that "
"outlives the call%select{| to %3}2", (Type, StringRef, bool, DeclName))
Comment on lines 398 to 400
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should tweak the wording of these diagnostics so they read correctly whether they're warnings or errors. For this one, I'm thinking:

Suggested change
ERROR(cannot_pass_type_to_non_ephemeral,none,
"cannot pass %0 to parameter; argument %1 must be a pointer that "
"outlives the call%select{| to %3}2", (Type, StringRef, bool, DeclName))
ERROR(cannot_pass_type_to_non_ephemeral,none,
"argument %1 passes temporary %0 to a parameter that must outlive "
"the call%select{| to %3}2", (Type, StringRef, bool, DeclName))

Copy link
Member Author

Choose a reason for hiding this comment

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

I begrudgingly admit that your wording is a little better, and fear only for the hundreds of lines of test cases I need to update to make it better for this minor feature.

WARNING(cannot_pass_type_to_non_ephemeral_warning,none,
"passing %0 to parameter, but argument %1 should be a pointer that "
"outlives the call%select{| to %3}2", (Type, StringRef, bool, DeclName))
ERROR(cannot_use_inout_non_ephemeral,none,
"cannot use inout expression here; argument %0 must be a pointer that "
"outlives the call%select{| to %2}1", (StringRef, bool, DeclName))
Comment on lines 401 to 403
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ERROR(cannot_use_inout_non_ephemeral,none,
"cannot use inout expression here; argument %0 must be a pointer that "
"outlives the call%select{| to %2}1", (StringRef, bool, DeclName))
ERROR(cannot_use_inout_non_ephemeral,none,
"inout expression on argument %0 passes temporary pointer to a "
"parameter that outlives the call%select{| to %2}1",
(StringRef, bool, DeclName))

WARNING(cannot_use_inout_non_ephemeral_warning,none,
"inout expression creates a temporary pointer, but argument %0 should "
"be a pointer that outlives the call%select{| to %2}1",
(StringRef, bool, DeclName))
ERROR(cannot_construct_dangling_pointer,none,
"initialization of %0 results in a dangling %select{|buffer }1pointer",
(Type, unsigned))
WARNING(cannot_construct_dangling_pointer_warning,none,
"initialization of %0 results in a dangling %select{|buffer }1pointer",
(Type, unsigned))
NOTE(ephemeral_pointer_argument_conversion_note,none,
"implicit argument conversion from %0 to %1 produces a pointer valid only "
"for the duration of the call%select{| to %3}2",
Expand Down
8 changes: 0 additions & 8 deletions include/swift/AST/EducationalNotes.def
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,9 @@ EDUCATIONAL_NOTES(could_not_use_member_on_existential,
"existential-member-access-limitations.md")

EDUCATIONAL_NOTES(cannot_pass_type_to_non_ephemeral, "temporary-pointers.md")
EDUCATIONAL_NOTES(cannot_pass_type_to_non_ephemeral_warning,
"temporary-pointers.md")
EDUCATIONAL_NOTES(cannot_use_inout_non_ephemeral,
"temporary-pointers.md")
EDUCATIONAL_NOTES(cannot_use_inout_non_ephemeral_warning,
"temporary-pointers.md")
EDUCATIONAL_NOTES(cannot_construct_dangling_pointer, "temporary-pointers.md")
EDUCATIONAL_NOTES(cannot_construct_dangling_pointer_warning,
"temporary-pointers.md")



EDUCATIONAL_NOTES(non_nominal_no_initializers, "nominal-types.md")
EDUCATIONAL_NOTES(non_nominal_extension, "nominal-types.md")
Expand Down
21 changes: 12 additions & 9 deletions include/swift/Sema/CSFix.h
Original file line number Diff line number Diff line change
Expand Up @@ -650,9 +650,9 @@ class ContextualMismatch : public ConstraintFix {
Type LHS, RHS;

ContextualMismatch(ConstraintSystem &cs, Type lhs, Type rhs,
ConstraintLocator *locator)
: ConstraintFix(cs, FixKind::ContextualMismatch, locator), LHS(lhs),
RHS(rhs) {}
ConstraintLocator *locator, bool warning)
: ConstraintFix(cs, FixKind::ContextualMismatch, locator, warning),
LHS(lhs), RHS(rhs) {}

protected:
ContextualMismatch(ConstraintSystem &cs, FixKind kind, Type lhs, Type rhs,
Expand Down Expand Up @@ -741,9 +741,9 @@ class MarkExplicitlyEscaping final : public ContextualMismatch {
/// Mark function type as being part of a global actor.
class MarkGlobalActorFunction final : public ContextualMismatch {
MarkGlobalActorFunction(ConstraintSystem &cs, Type lhs, Type rhs,
ConstraintLocator *locator)
ConstraintLocator *locator, bool warning)
: ContextualMismatch(cs, FixKind::MarkGlobalActorFunction, lhs, rhs,
locator) {
locator, warning) {
}

public:
Expand All @@ -752,7 +752,8 @@ class MarkGlobalActorFunction final : public ContextualMismatch {
bool diagnose(const Solution &solution, bool asNote = false) const override;

static MarkGlobalActorFunction *create(ConstraintSystem &cs, Type lhs,
Type rhs, ConstraintLocator *locator);
Type rhs, ConstraintLocator *locator,
bool warning);

static bool classof(ConstraintFix *fix) {
return fix->getKind() == FixKind::MarkGlobalActorFunction;
Expand Down Expand Up @@ -787,9 +788,10 @@ class ForceOptional final : public ContextualMismatch {
/// function types, repair it by adding @Sendable attribute.
class AddSendableAttribute final : public ContextualMismatch {
AddSendableAttribute(ConstraintSystem &cs, FunctionType *fromType,
FunctionType *toType, ConstraintLocator *locator)
FunctionType *toType, ConstraintLocator *locator,
bool warning)
: ContextualMismatch(cs, FixKind::AddSendableAttribute, fromType, toType,
locator) {
locator, warning) {
assert(fromType->isSendable() != toType->isSendable());
}

Expand All @@ -801,7 +803,8 @@ class AddSendableAttribute final : public ContextualMismatch {
static AddSendableAttribute *create(ConstraintSystem &cs,
FunctionType *fromType,
FunctionType *toType,
ConstraintLocator *locator);
ConstraintLocator *locator,
bool warning);

static bool classof(ConstraintFix *fix) {
return fix->getKind() == FixKind::AddSendableAttribute;
Expand Down
28 changes: 10 additions & 18 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,10 @@ template <typename... ArgTypes>
InFlightDiagnostic
FailureDiagnostic::emitDiagnosticAt(ArgTypes &&... Args) const {
auto &DE = getASTContext().Diags;
return DE.diagnose(std::forward<ArgTypes>(Args)...);
auto behavior = isWarning ? DiagnosticBehavior::Warning
: DiagnosticBehavior::Unspecified;
return std::move(DE.diagnose(std::forward<ArgTypes>(Args)...)
.limitBehavior(behavior));
}

Expr *FailureDiagnostic::findParentExpr(const Expr *subExpr) const {
Expand Down Expand Up @@ -6779,12 +6782,9 @@ bool NonEphemeralConversionFailure::diagnosePointerInit() const {
return false;
}

auto diagID = DowngradeToWarning
? diag::cannot_construct_dangling_pointer_warning
: diag::cannot_construct_dangling_pointer;

auto anchor = getRawAnchor();
emitDiagnosticAt(::getLoc(anchor), diagID, constructedTy, constructorKind)
emitDiagnosticAt(::getLoc(anchor), diag::cannot_construct_dangling_pointer,
constructedTy, constructorKind)
.highlight(::getSourceRange(anchor));

emitSuggestionNotes();
Expand Down Expand Up @@ -6814,20 +6814,12 @@ bool NonEphemeralConversionFailure::diagnoseAsError() {

auto *argExpr = getArgExpr();
if (isa<InOutExpr>(argExpr)) {
auto diagID = DowngradeToWarning
? diag::cannot_use_inout_non_ephemeral_warning
: diag::cannot_use_inout_non_ephemeral;

emitDiagnosticAt(argExpr->getLoc(), diagID, argDesc, getCallee(),
getCalleeFullName())
emitDiagnosticAt(argExpr->getLoc(), diag::cannot_use_inout_non_ephemeral,
argDesc, getCallee(), getCalleeFullName())
.highlight(argExpr->getSourceRange());
} else {
auto diagID = DowngradeToWarning
? diag::cannot_pass_type_to_non_ephemeral_warning
: diag::cannot_pass_type_to_non_ephemeral;

emitDiagnosticAt(argExpr->getLoc(), diagID, getArgType(), argDesc,
getCallee(), getCalleeFullName())
emitDiagnosticAt(argExpr->getLoc(), diag::cannot_pass_type_to_non_ephemeral,
getArgType(), argDesc, getCallee(), getCalleeFullName())
.highlight(argExpr->getSourceRange());
}
emitSuggestionNotes();
Expand Down
45 changes: 26 additions & 19 deletions lib/Sema/CSDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,17 @@ class FunctionArgApplyInfo;
class FailureDiagnostic {
const Solution &S;
ConstraintLocator *Locator;
bool isWarning;

public:
FailureDiagnostic(const Solution &solution, ConstraintLocator *locator)
: S(solution), Locator(locator) {}
FailureDiagnostic(const Solution &solution, ConstraintLocator *locator,
bool isWarning = false)
: S(solution), Locator(locator), isWarning(isWarning) {}

FailureDiagnostic(const Solution &solution, ASTNode anchor)
: FailureDiagnostic(solution, solution.getConstraintLocator(anchor)) {}
FailureDiagnostic(const Solution &solution, ASTNode anchor,
bool isWarning = false)
: FailureDiagnostic(solution, solution.getConstraintLocator(anchor),
isWarning) { }

virtual ~FailureDiagnostic();

Expand Down Expand Up @@ -601,20 +605,21 @@ class ContextualFailure : public FailureDiagnostic {

public:
ContextualFailure(const Solution &solution, Type lhs, Type rhs,
ConstraintLocator *locator)
ConstraintLocator *locator, bool isWarning = false)
: ContextualFailure(
solution,
locator->isForContextualType()
? locator->castLastElementTo<LocatorPathElt::ContextualType>()
.getPurpose()
: solution.getConstraintSystem().getContextualTypePurpose(
locator->getAnchor()),
lhs, rhs, locator) {}
lhs, rhs, locator, isWarning) {}

ContextualFailure(const Solution &solution, ContextualTypePurpose purpose,
Type lhs, Type rhs, ConstraintLocator *locator)
: FailureDiagnostic(solution, locator), CTP(purpose), RawFromType(lhs),
RawToType(rhs) {
Type lhs, Type rhs, ConstraintLocator *locator,
bool isWarning = false)
: FailureDiagnostic(solution, locator, isWarning), CTP(purpose),
RawFromType(lhs), RawToType(rhs) {
assert(lhs && "Expected a valid 'from' type");
assert(rhs && "Expected a valid 'to' type");
}
Expand Down Expand Up @@ -753,8 +758,9 @@ class AttributedFuncToTypeConversionFailure final : public ContextualFailure {

AttributedFuncToTypeConversionFailure(const Solution &solution, Type fromType,
Type toType, ConstraintLocator *locator,
AttributeKind attributeKind)
: ContextualFailure(solution, fromType, toType, locator),
AttributeKind attributeKind,
bool isWarning = false)
: ContextualFailure(solution, fromType, toType, locator, isWarning),
attributeKind(attributeKind) {}

bool diagnoseAsError() override;
Expand All @@ -777,8 +783,9 @@ class AttributedFuncToTypeConversionFailure final : public ContextualFailure {
class DroppedGlobalActorFunctionAttr final : public ContextualFailure {
public:
DroppedGlobalActorFunctionAttr(const Solution &solution, Type fromType,
Type toType, ConstraintLocator *locator)
: ContextualFailure(solution, fromType, toType, locator) {}
Type toType, ConstraintLocator *locator,
bool isWarning)
: ContextualFailure(solution, fromType, toType, locator, isWarning) { }

bool diagnoseAsError() override;
};
Expand Down Expand Up @@ -1942,8 +1949,9 @@ class ArgumentMismatchFailure : public ContextualFailure {

public:
ArgumentMismatchFailure(const Solution &solution, Type argType,
Type paramType, ConstraintLocator *locator)
: ContextualFailure(solution, argType, paramType, locator),
Type paramType, ConstraintLocator *locator,
bool warning = false)
: ContextualFailure(solution, argType, paramType, locator, warning),
Info(*getFunctionArgApplyInfo(getLocator())) {}

bool diagnoseAsError() override;
Expand Down Expand Up @@ -2112,16 +2120,15 @@ class InvalidUseOfTrailingClosure final : public ArgumentMismatchFailure {
/// ```
class NonEphemeralConversionFailure final : public ArgumentMismatchFailure {
ConversionRestrictionKind ConversionKind;
bool DowngradeToWarning;

public:
NonEphemeralConversionFailure(const Solution &solution,
ConstraintLocator *locator, Type fromType,
Type toType,
ConversionRestrictionKind conversionKind,
bool downgradeToWarning)
: ArgumentMismatchFailure(solution, fromType, toType, locator),
ConversionKind(conversionKind), DowngradeToWarning(downgradeToWarning) {
bool warning)
: ArgumentMismatchFailure(solution, fromType, toType, locator, warning),
ConversionKind(conversionKind) {
}

bool diagnoseAsError() override;
Expand Down
21 changes: 12 additions & 9 deletions lib/Sema/CSFix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -201,39 +201,41 @@ MarkExplicitlyEscaping::create(ConstraintSystem &cs, Type lhs, Type rhs,
bool MarkGlobalActorFunction::diagnose(const Solution &solution,
bool asNote) const {
DroppedGlobalActorFunctionAttr failure(
solution, getFromType(), getToType(), getLocator());
solution, getFromType(), getToType(), getLocator(), isWarning());
return failure.diagnose(asNote);
}

MarkGlobalActorFunction *
MarkGlobalActorFunction::create(ConstraintSystem &cs, Type lhs, Type rhs,
ConstraintLocator *locator) {
ConstraintLocator *locator, bool warning) {
if (locator->isLastElement<LocatorPathElt::ApplyArgToParam>())
locator = cs.getConstraintLocator(
locator, LocatorPathElt::ArgumentAttribute::forGlobalActor());

return new (cs.getAllocator()) MarkGlobalActorFunction(cs, lhs, rhs, locator);
return new (cs.getAllocator()) MarkGlobalActorFunction(
cs, lhs, rhs, locator, warning);
}

bool AddSendableAttribute::diagnose(const Solution &solution,
bool asNote) const {
AttributedFuncToTypeConversionFailure failure(
solution, getFromType(), getToType(), getLocator(),
AttributedFuncToTypeConversionFailure::Concurrent);
AttributedFuncToTypeConversionFailure::Concurrent, isWarning());
return failure.diagnose(asNote);
}

AddSendableAttribute *
AddSendableAttribute::create(ConstraintSystem &cs,
FunctionType *fromType,
FunctionType *toType,
ConstraintLocator *locator) {
FunctionType *fromType,
FunctionType *toType,
ConstraintLocator *locator,
bool warning) {
if (locator->isLastElement<LocatorPathElt::ApplyArgToParam>())
locator = cs.getConstraintLocator(
locator, LocatorPathElt::ArgumentAttribute::forConcurrent());

return new (cs.getAllocator()) AddSendableAttribute(
cs, fromType, toType, locator);
cs, fromType, toType, locator, warning);
}
bool RelabelArguments::diagnose(const Solution &solution, bool asNote) const {
LabelingFailure failure(solution, getLocator(), getLabels());
Expand Down Expand Up @@ -369,7 +371,8 @@ bool ContextualMismatch::diagnoseForAmbiguity(
ContextualMismatch *ContextualMismatch::create(ConstraintSystem &cs, Type lhs,
Type rhs,
ConstraintLocator *locator) {
return new (cs.getAllocator()) ContextualMismatch(cs, lhs, rhs, locator);
return new (cs.getAllocator()) ContextualMismatch(
cs, lhs, rhs, locator, /*warning=*/false);
}

bool AllowWrappedValueMismatch::diagnose(const Solution &solution, bool asError) const {
Expand Down
23 changes: 21 additions & 2 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
//===----------------------------------------------------------------------===//

#include "CSDiagnostics.h"
#include "TypeCheckConcurrency.h"
#include "swift/AST/ASTPrinter.h"
#include "swift/AST/Decl.h"
#include "swift/AST/ExistentialLayout.h"
Expand Down Expand Up @@ -2170,6 +2171,22 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
increaseScore(SK_SyncInAsync);
}

/// Whether to downgrade to a concurrency warning.
auto isConcurrencyWarning = [&] {
if (contextUsesConcurrencyFeatures(DC) ||
Context.LangOpts.isSwiftVersionAtLeast(6))
return false;

switch (kind) {
case ConstraintKind::Conversion:
case ConstraintKind::ArgumentConversion:
return true;

default:
return false;
}
};

// A @Sendable function can be a subtype of a non-@Sendable function.
if (func1->isSendable() != func2->isSendable()) {
// Cannot add '@Sendable'.
Expand All @@ -2178,7 +2195,8 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
return getTypeMatchFailure(locator);

auto *fix = AddSendableAttribute::create(
*this, func1, func2, getConstraintLocator(locator));
*this, func1, func2, getConstraintLocator(locator),
isConcurrencyWarning());
if (recordFix(fix))
return getTypeMatchFailure(locator);
}
Expand Down Expand Up @@ -2212,7 +2230,8 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2,
return getTypeMatchFailure(locator);

auto *fix = MarkGlobalActorFunction::create(
*this, func1, func2, getConstraintLocator(locator));
*this, func1, func2, getConstraintLocator(locator),
isConcurrencyWarning());

if (recordFix(fix))
return getTypeMatchFailure(locator);
Expand Down
2 changes: 1 addition & 1 deletion test/Concurrency/actor_isolation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ func testGlobalActorClosures() {
return 17
}

acceptConcurrentClosure { @SomeGlobalActor in 5 } // expected-error{{converting function value of type '@SomeGlobalActor @Sendable () -> Int' to '@Sendable () -> Int' loses global actor 'SomeGlobalActor'}}
acceptConcurrentClosure { @SomeGlobalActor in 5 } // expected-warning{{converting function value of type '@SomeGlobalActor @Sendable () -> Int' to '@Sendable () -> Int' loses global actor 'SomeGlobalActor'}}
}

@available(SwiftStdlib 5.1, *)
Expand Down
4 changes: 2 additions & 2 deletions test/Concurrency/global_actor_function_types.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ struct OtherGlobalActor {
func testConversions(f: @escaping @SomeGlobalActor (Int) -> Void, g: @escaping (Int) -> Void) {
let _: Int = f // expected-error{{cannot convert value of type '@SomeGlobalActor (Int) -> Void' to specified type 'Int'}}

let _: (Int) -> Void = f // expected-error{{converting function value of type '@SomeGlobalActor (Int) -> Void' to '(Int) -> Void' loses global actor 'SomeGlobalActor'}}
let _: (Int) -> Void = f // expected-warning{{converting function value of type '@SomeGlobalActor (Int) -> Void' to '(Int) -> Void' loses global actor 'SomeGlobalActor'}}
let _: @SomeGlobalActor (Int) -> Void = g // okay

// FIXME: this could be better.
Expand Down Expand Up @@ -119,7 +119,7 @@ func testTypesNonConcurrencyContext() { // expected-note{{add '@SomeGlobalActor'
let f1 = onSomeGlobalActor // expected-note{{calls to let 'f1' from outside of its actor context are implicitly asynchronous}}
let f2 = onSomeGlobalActorUnsafe

let _: () -> Int = f1 // expected-error{{converting function value of type '@SomeGlobalActor () -> Int' to '() -> Int' loses global actor 'SomeGlobalActor'}}
let _: () -> Int = f1 // expected-warning{{converting function value of type '@SomeGlobalActor () -> Int' to '() -> Int' loses global actor 'SomeGlobalActor'}}
let _: () -> Int = f2

_ = f1() // expected-error{{call to global actor 'SomeGlobalActor'-isolated let 'f1' in a synchronous nonisolated context}}
Expand Down
Loading