Skip to content

Commit ee819c8

Browse files
committed
Build support for ~Copyable atop @_moveOnly
We parse `~Copyable` in an inheritance clause of enum and struct decls as a synonym for the `@_moveOnly` attribute being added to that decl. This completely side-steps the additional infrastructure for generalized suppressed conformances in favor of a minimal solution. One benefit of this minimal solution is that it doesn't risk introducing any back-compat issues with older compilers or stdlibs. The trade-off is that we're more committed to supporting `@_moveOnly` in compiled modules in the future. In fact, this change does not deprecate `@_moveOnly` in any way. resolves rdar://106775103
1 parent 404b925 commit ee819c8

File tree

8 files changed

+168
-6
lines changed

8 files changed

+168
-6
lines changed

include/swift/AST/DiagnosticsParse.def

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -944,6 +944,19 @@ ERROR(expected_rparen_layout_constraint,none,
944944
ERROR(layout_constraints_only_inside_specialize_attr,none,
945945
"layout constraints are only allowed inside '_specialize' attributes", ())
946946

947+
//------------------------------------------------------------------------------
948+
// MARK: Conformance suppression diagnostics
949+
//------------------------------------------------------------------------------
950+
951+
ERROR(cannot_suppress_here,none,
952+
"cannot suppress conformances here", ())
953+
954+
ERROR(only_suppress_copyable,none,
955+
"can only suppress 'Copyable'", ())
956+
957+
ERROR(already_suppressed,none,
958+
"duplicate suppression of %0", (Identifier))
959+
947960
//------------------------------------------------------------------------------
948961
// MARK: Pattern parsing diagnostics
949962
//------------------------------------------------------------------------------

include/swift/AST/KnownIdentifiers.def

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ IDENTIFIER(Context)
5656
IDENTIFIER(CoreGraphics)
5757
IDENTIFIER(CoreMedia)
5858
IDENTIFIER(CGFloat)
59+
IDENTIFIER(Copyable)
5960
IDENTIFIER(CoreFoundation)
6061
IDENTIFIER(count)
6162
IDENTIFIER(CVarArg)

include/swift/Parse/Parser.h

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1195,9 +1195,18 @@ class Parser {
11951195

11961196
ParserResult<ImportDecl> parseDeclImport(ParseDeclOptions Flags,
11971197
DeclAttributes &Attributes);
1198+
1199+
/// Parse an inheritance clause into a vector of InheritedEntry's.
1200+
///
1201+
/// \param allowClassRequirement whether to permit parsing of 'class'
1202+
/// \param allowAnyObject whether to permit parsing of 'AnyObject'
1203+
/// \param parseTildeCopyable if non-null, permits parsing of `~Copyable`
1204+
/// and writes out a valid source location if it was parsed. If null, then a
1205+
/// parsing error will be emitted upon the appearance of `~` in the clause.
11981206
ParserStatus parseInheritance(SmallVectorImpl<InheritedEntry> &Inherited,
11991207
bool allowClassRequirement,
1200-
bool allowAnyObject);
1208+
bool allowAnyObject,
1209+
SourceLoc *parseTildeCopyable = nullptr);
12011210
ParserStatus parseDeclItem(bool &PreviousHadSemi,
12021211
ParseDeclOptions Options,
12031212
llvm::function_ref<void(Decl*)> handler);

include/swift/Parse/Token.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,10 @@ class Token {
124124
return !isEllipsis();
125125
}
126126

127+
bool isTilde() const {
128+
return isAnyOperator() && Text == "~";
129+
}
130+
127131
/// Determine whether this token occurred at the start of a line.
128132
bool isAtStartOfLine() const { return AtStartOfLine; }
129133

lib/Parse/ParseDecl.cpp

Lines changed: 73 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5666,6 +5666,29 @@ ParserResult<ImportDecl> Parser::parseDeclImport(ParseDeclOptions Flags,
56665666
return DCC.fixupParserResult(ID);
56675667
}
56685668

5669+
static void addMoveOnlyAttrIf(SourceLoc const &parsedTildeCopyable,
5670+
ASTContext &Context,
5671+
Decl *decl) {
5672+
if (parsedTildeCopyable.isInvalid())
5673+
return;
5674+
5675+
auto &attrs = decl->getAttrs();
5676+
5677+
// Don't add if it's already explicitly written on the decl, but error about
5678+
// the duplication and point to the `~Copyable`.
5679+
if (auto attr = attrs.getAttribute<MoveOnlyAttr>()) {
5680+
const bool sayModifier = false;
5681+
Context.Diags.diagnose(attr->getLocation(), diag::duplicate_attribute,
5682+
sayModifier)
5683+
.fixItRemove(attr->getRange());
5684+
Context.Diags.diagnose(parsedTildeCopyable, diag::previous_attribute,
5685+
sayModifier);
5686+
return;
5687+
}
5688+
5689+
attrs.add(new(Context) MoveOnlyAttr(/*IsImplicit=*/true));
5690+
}
5691+
56695692
/// Parse an inheritance clause.
56705693
///
56715694
/// \verbatim
@@ -5675,15 +5698,19 @@ ParserResult<ImportDecl> Parser::parseDeclImport(ParseDeclOptions Flags,
56755698
/// inherited:
56765699
/// 'class'
56775700
/// type-identifier
5701+
/// '~' 'Copyable'
56785702
/// \endverbatim
56795703
ParserStatus Parser::parseInheritance(
56805704
SmallVectorImpl<InheritedEntry> &Inherited,
5681-
bool allowClassRequirement, bool allowAnyObject) {
5705+
bool allowClassRequirement,
5706+
bool allowAnyObject,
5707+
SourceLoc *parseTildeCopyable) {
56825708
consumeToken(tok::colon);
56835709

56845710
SourceLoc classRequirementLoc;
56855711

56865712
ParserStatus Status;
5713+
SourceLoc TildeCopyableLoc;
56875714
SourceLoc prevComma;
56885715
bool HasNextType;
56895716
do {
@@ -5737,6 +5764,38 @@ ParserStatus Parser::parseInheritance(
57375764
continue;
57385765
}
57395766

5767+
// Is suppression permitted?
5768+
if (parseTildeCopyable) {
5769+
// Try to find '~' 'Copyable'
5770+
//
5771+
// We do this knowing that Copyable is not a real type as of now, so we
5772+
// can't rely on parseType.
5773+
if (Tok.isTilde()) {
5774+
const auto &nextTok = peekToken(); // lookahead
5775+
if (isIdentifier(nextTok, Context.Id_Copyable.str())) {
5776+
auto tildeLoc = consumeToken();
5777+
consumeToken(); // the 'Copyable' token
5778+
5779+
if (TildeCopyableLoc)
5780+
diagnose(tildeLoc, diag::already_suppressed, Context.Id_Copyable);
5781+
else
5782+
TildeCopyableLoc = tildeLoc;
5783+
5784+
continue;
5785+
}
5786+
5787+
// can't suppress whatever is between '~' and ',' or '{'.
5788+
diagnose(Tok, diag::only_suppress_copyable);
5789+
consumeToken();
5790+
}
5791+
5792+
} else if (Tok.isTilde()) {
5793+
// a suppression isn't allowed here, so emit an error eat the token to
5794+
// prevent further parsing errors.
5795+
diagnose(Tok, diag::cannot_suppress_here);
5796+
consumeToken();
5797+
}
5798+
57405799
auto ParsedTypeResult = parseType();
57415800
Status |= ParsedTypeResult;
57425801

@@ -5745,6 +5804,9 @@ ParserStatus Parser::parseInheritance(
57455804
Inherited.push_back(InheritedEntry(ParsedTypeResult.get()));
57465805
} while (HasNextType);
57475806

5807+
if (parseTildeCopyable)
5808+
*parseTildeCopyable = TildeCopyableLoc;
5809+
57485810
return Status;
57495811
}
57505812

@@ -8207,10 +8269,14 @@ ParserResult<EnumDecl> Parser::parseDeclEnum(ParseDeclOptions Flags,
82078269
// Parse optional inheritance clause within the context of the enum.
82088270
if (Tok.is(tok::colon)) {
82098271
SmallVector<InheritedEntry, 2> Inherited;
8272+
SourceLoc parsedTildeCopyable;
82108273
Status |= parseInheritance(Inherited,
82118274
/*allowClassRequirement=*/false,
8212-
/*allowAnyObject=*/false);
8275+
/*allowAnyObject=*/false,
8276+
&parsedTildeCopyable);
82138277
ED->setInherited(Context.AllocateCopy(Inherited));
8278+
8279+
addMoveOnlyAttrIf(parsedTildeCopyable, Context, ED);
82148280
}
82158281

82168282
diagnoseWhereClauseInGenericParamList(GenericParams);
@@ -8470,10 +8536,14 @@ ParserResult<StructDecl> Parser::parseDeclStruct(ParseDeclOptions Flags,
84708536
// Parse optional inheritance clause within the context of the struct.
84718537
if (Tok.is(tok::colon)) {
84728538
SmallVector<InheritedEntry, 2> Inherited;
8539+
SourceLoc parsedTildeCopyable;
84738540
Status |= parseInheritance(Inherited,
84748541
/*allowClassRequirement=*/false,
8475-
/*allowAnyObject=*/false);
8542+
/*allowAnyObject=*/false,
8543+
&parsedTildeCopyable);
84768544
SD->setInherited(Context.AllocateCopy(Inherited));
8545+
8546+
addMoveOnlyAttrIf(parsedTildeCopyable, Context, SD);
84778547
}
84788548

84798549
diagnoseWhereClauseInGenericParamList(GenericParams);

lib/Sema/TypeCheckDeclPrimary.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2754,11 +2754,15 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
27542754
/// check to see if a move-only type can ever conform to the given type.
27552755
/// \returns true iff a diagnostic was emitted because it was not compatible
27562756
static bool diagnoseIncompatibleWithMoveOnlyType(SourceLoc loc,
2757-
NominalTypeDecl *moveonlyType,
2758-
Type type) {
2757+
NominalTypeDecl *moveonlyType,
2758+
Type type) {
27592759
assert(type && "got an empty type?");
27602760
assert(moveonlyType->isMoveOnly());
27612761

2762+
// no need to emit a diagnostic if the type itself is already problematic.
2763+
if (type->hasError())
2764+
return false;
2765+
27622766
auto canType = type->getCanonicalType();
27632767
if (auto prot = canType->getAs<ProtocolType>()) {
27642768
// Permit conformance to marker protocol Sendable.

test/ModuleInterface/moveonly_interface_flag.swift

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,15 @@
88
// CHECK: #if compiler(>=5.3) && $MoveOnly
99
// CHECK-NEXT: @_moveOnly public struct MoveOnlyStruct {
1010

11+
// CHECK: #if compiler(>=5.3) && $MoveOnly
12+
// CHECK-NEXT: @_moveOnly public struct MoveOnlyStructSupp {
13+
1114
// CHECK: #if compiler(>=5.3) && $MoveOnly
1215
// CHECK-NEXT: @_moveOnly public enum MoveOnlyEnum {
1316

17+
// CHECK: #if compiler(>=5.3) && $MoveOnly
18+
// CHECK-NEXT: @_moveOnly public enum MoveOnlyEnumSupp {
19+
1420
// CHECK: #if compiler(>=5.3) && $MoveOnly
1521
// CHECK-NEXT: public func someFn() -> Library.MoveOnlyEnum
1622

@@ -25,10 +31,18 @@
2531
let x = 0
2632
}
2733

34+
public struct MoveOnlyStructSupp: ~Copyable {
35+
let x = 0
36+
}
37+
2838
@_moveOnly public enum MoveOnlyEnum {
2939
case depth
3040
}
3141

42+
public enum MoveOnlyEnumSupp: ~Copyable {
43+
case depth
44+
}
45+
3246
public func someFn() -> MoveOnlyEnum { return .depth }
3347

3448
public class What {

test/Parse/without_copyable.swift

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// RUN: %target-typecheck-verify-swift
2+
3+
protocol Sando { func make() } // expected-note 2{{protocol requires function 'make()'}}
4+
5+
struct S: ~U, // expected-error {{can only suppress 'Copyable'}}
6+
// expected-error@-1 {{inheritance from non-protocol type 'U'}}
7+
~Copyable {}
8+
9+
struct U: // expected-error {{move-only struct 'U' cannot conform to 'Sando'}}
10+
// expected-error@-1 {{type 'U' does not conform to protocol 'Sando'}}
11+
~Copyable,
12+
Sando,
13+
~Copyable // expected-error {{duplicate suppression of 'Copyable'}}
14+
{}
15+
16+
17+
// The expected behavior for '~' in the inheritance clause of a decl not supporting
18+
// suppression is to emit an error and then to treat it as if it's inheriting from
19+
// the type, rather than suppressing. That is, it treats it like the '~' wasn't there
20+
// after emitting an error.
21+
22+
class C: // expected-error {{type 'C' does not conform to protocol 'Sando'}}
23+
~Copyable, // expected-error {{cannot suppress conformances here}}
24+
// expected-error@-1 {{cannot find type 'Copyable' in scope}}
25+
~Sando // expected-error {{cannot suppress conformances here}}
26+
{}
27+
28+
protocol Rope<Element>: ~Copyable { // expected-error {{cannot suppress conformances here}}
29+
// expected-error@-1 {{cannot find type 'Copyable' in scope}}
30+
31+
associatedtype Element: ~Copyable // expected-error {{cannot suppress conformances here}}
32+
// expected-error@-1 {{cannot find type 'Copyable' in scope}}
33+
}
34+
35+
extension S: ~Copyable {} // expected-error {{cannot suppress conformances here}}
36+
// expected-error@-1 {{cannot find type 'Copyable' in scope}}
37+
38+
func takeNoncopyableGeneric<T: ~Copyable>(_ t: T) {} // expected-error {{expected a class type or protocol-constrained type restricting 'T'}}
39+
40+
@_moveOnly struct ExtraNonCopyable: // expected-error {{duplicate attribute}}{{1-12=}}
41+
~Copyable // expected-note {{attribute already specified here}}
42+
{}
43+
44+
// basic test to ensure it's viewed as a noncopyable struct:
45+
struct HasADeinit: ~Copyable {
46+
deinit {}
47+
}

0 commit comments

Comments
 (0)