Skip to content

Commit

Permalink
Add a Fix-It to the warning about unnecessary @preconcurrency confo…
Browse files Browse the repository at this point in the history
…rmance

When we diagnose an unnecessary `@preconcurrency` on a conformance,
also provide a Fix-It to remove the `@preconcurrency`.
  • Loading branch information
DougGregor committed May 20, 2024
1 parent c7c244e commit e9220b4
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 11 deletions.
3 changes: 2 additions & 1 deletion include/swift/AST/ASTContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -1238,7 +1238,8 @@ class ASTContext final {
DeclContext *dc,
ProtocolConformanceState state,
bool isUnchecked,
bool isPreconcurrency);
bool isPreconcurrency,
SourceLoc preconcurrencyLoc = SourceLoc());

/// Produce a self-conformance for the given protocol.
SelfProtocolConformance *
Expand Down
15 changes: 12 additions & 3 deletions include/swift/AST/ProtocolConformance.h
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,9 @@ class NormalProtocolConformance : public RootProtocolConformance,
/// The location of this protocol conformance in the source.
SourceLoc Loc;

/// The location of the `@preconcurrency` attribute, if any.
SourceLoc PreconcurrencyLoc;

/// The declaration context containing the ExtensionDecl or
/// NominalTypeDecl that declared the conformance.
DeclContext *Context;
Expand Down Expand Up @@ -562,10 +565,12 @@ class NormalProtocolConformance : public RootProtocolConformance,
NormalProtocolConformance(Type conformingType, ProtocolDecl *protocol,
SourceLoc loc, DeclContext *dc,
ProtocolConformanceState state, bool isUnchecked,
bool isPreconcurrency)
bool isPreconcurrency,
SourceLoc preconcurrencyLoc)
: RootProtocolConformance(ProtocolConformanceKind::Normal,
conformingType),
Protocol(protocol), Loc(loc), Context(dc) {
Protocol(protocol), Loc(loc), PreconcurrencyLoc(preconcurrencyLoc),
Context(dc) {
assert(!conformingType->hasArchetype() &&
"ProtocolConformances should store interface types");
setState(state);
Expand All @@ -580,7 +585,7 @@ class NormalProtocolConformance : public RootProtocolConformance,
/// Get the protocol being conformed to.
ProtocolDecl *getProtocol() const { return Protocol; }

/// Retrieve the location of this
/// Retrieve the location of this conformance.
SourceLoc getLoc() const { return Loc; }

/// Get the declaration context that contains the conforming extension or
Expand Down Expand Up @@ -629,6 +634,10 @@ class NormalProtocolConformance : public RootProtocolConformance,
return Bits.NormalProtocolConformance.IsPreconcurrency;
}

/// Retrieve the location of `@preconcurrency`, if there is one and it is
/// known.
SourceLoc getPreconcurrencyLoc() const { return PreconcurrencyLoc; }

/// Determine whether we've lazily computed the associated conformance array
/// already.
bool hasComputedAssociatedConformances() const {
Expand Down
5 changes: 3 additions & 2 deletions lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2593,7 +2593,8 @@ ASTContext::getNormalConformance(Type conformingType,
DeclContext *dc,
ProtocolConformanceState state,
bool isUnchecked,
bool isPreconcurrency) {
bool isPreconcurrency,
SourceLoc preconcurrencyLoc) {
assert(dc->isTypeContext());

llvm::FoldingSetNodeID id;
Expand All @@ -2609,7 +2610,7 @@ ASTContext::getNormalConformance(Type conformingType,
// Build a new normal protocol conformance.
auto result = new (*this, AllocationArena::Permanent)
NormalProtocolConformance(conformingType, protocol, loc, dc, state,
isUnchecked, isPreconcurrency);
isUnchecked, isPreconcurrency, preconcurrencyLoc);
normalConformances.InsertNode(result, insertPos);

return result;
Expand Down
3 changes: 2 additions & 1 deletion lib/AST/ConformanceLookupTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -989,7 +989,8 @@ ConformanceLookupTable::getConformance(NominalTypeDecl *nominal,
conformingType, protocol, conformanceLoc, conformingDC,
ProtocolConformanceState::Incomplete,
entry->Source.getUncheckedLoc().isValid(),
entry->Source.getPreconcurrencyLoc().isValid());
entry->Source.getPreconcurrencyLoc().isValid(),
entry->Source.getPreconcurrencyLoc());
// Invalid code may cause the getConformance call below to loop, so break
// the infinite recursion by setting this eagerly to shortcircuit with the
// early return at the start of this function.
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/TypeRepr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ SourceLoc TypeRepr::findAttrLoc(TypeAttrKind kind) const {
for (auto attr : attrTypeRepr->getAttrs()) {
if (auto typeAttr = attr.dyn_cast<TypeAttribute*>())
if (typeAttr->getKind() == kind)
return typeAttr->getAttrLoc();
return typeAttr->getStartLoc();
}

typeRepr = attrTypeRepr->getTypeRepr();
Expand Down
9 changes: 8 additions & 1 deletion lib/Sema/TypeCheckProtocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5120,9 +5120,16 @@ void ConformanceChecker::resolveValueWitnesses() {
}

if (Conformance->isPreconcurrency() && !usesPreconcurrencyConformance) {
DC->getASTContext().Diags.diagnose(
auto diag = DC->getASTContext().Diags.diagnose(
Conformance->getLoc(), diag::preconcurrency_conformance_not_used,
Proto->getDeclaredInterfaceType());

SourceLoc preconcurrencyLoc = Conformance->getPreconcurrencyLoc();
if (preconcurrencyLoc.isValid()) {
SourceLoc endLoc =
preconcurrencyLoc.getAdvancedLoc(strlen("@preconcurrency "));
diag.fixItRemoveChars(preconcurrencyLoc, endLoc);
}
}

// Finally, check some ad-hoc protocol requirements.
Expand Down
4 changes: 2 additions & 2 deletions test/Concurrency/preconcurrency_conformances.swift
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ protocol WithNonIsolated {

do {
class TestExplicitOtherIsolation : @preconcurrency WithNonIsolated {
// expected-warning@-1 {{@preconcurrency attribute on conformance to 'WithNonIsolated' has no effect}}
// expected-warning@-1 {{@preconcurrency attribute on conformance to 'WithNonIsolated' has no effect}}{{38-54=}}

@GlobalActor var prop: Int = 42
// expected-warning@-1 {{global actor 'GlobalActor'-isolated property 'prop' cannot be used to satisfy main actor-isolated protocol requirement}}
Expand All @@ -164,7 +164,7 @@ do {

do {
class InferredGlobalActorAttrs : @preconcurrency WithNonIsolated {
// expected-warning@-1 {{@preconcurrency attribute on conformance to 'WithNonIsolated' has no effect}}
// expected-warning@-1 {{@preconcurrency attribute on conformance to 'WithNonIsolated' has no effect}}{{36-52=}}
var prop: Int = 42
func test() {}
}
Expand Down

0 comments on commit e9220b4

Please sign in to comment.