Skip to content

Commit 217d179

Browse files
DougGregorDougGregor
authored
Merge pull request #67149 from DougGregor/accessor-macros-finishing-touches-5.9
[5.9] Accessor macros finishing touches
2 parents 3eba3da + 8dc52ec commit 217d179

File tree

7 files changed

+210
-59
lines changed

7 files changed

+210
-59
lines changed

‎include/swift/AST/Decl.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5238,6 +5238,8 @@ class AbstractStorageDecl : public ValueDecl {
52385238

52395239
void addOpaqueAccessor(AccessorDecl *accessor);
52405240

5241+
void removeAccessor(AccessorDecl *accessor);
5242+
52415243
private:
52425244
MutableArrayRef<AccessorDecl *> getAccessorsBuffer() {
52435245
return { getTrailingObjects<AccessorDecl*>(), NumAccessors };
@@ -5401,6 +5403,11 @@ class AbstractStorageDecl : public ValueDecl {
54015403
return {};
54025404
}
54035405

5406+
void removeAccessor(AccessorDecl *accessor) {
5407+
if (auto *info = Accessors.getPointer())
5408+
return info->removeAccessor(accessor);
5409+
}
5410+
54045411
/// This is the primary mechanism by which we can easily determine whether
54055412
/// this storage decl has any effects.
54065413
///
@@ -8912,6 +8919,9 @@ const ParamDecl *getParameterAt(const ValueDecl *source, unsigned index);
89128919
/// nullptr if the source does not have a parameter list.
89138920
const ParamDecl *getParameterAt(const DeclContext *source, unsigned index);
89148921

8922+
StringRef getAccessorNameForDiagnostic(AccessorDecl *accessor, bool article);
8923+
StringRef getAccessorNameForDiagnostic(AccessorKind accessorKind, bool article);
8924+
89158925
void simple_display(llvm::raw_ostream &out,
89168926
OptionSet<NominalTypeDecl::LookupDirectFlags> options);
89178927

‎lib/AST/Decl.cpp

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6411,6 +6411,38 @@ bool ProtocolDecl::hasCircularInheritedProtocols() const {
64116411
ctx.evaluator, HasCircularInheritedProtocolsRequest{mutableThis}, true);
64126412
}
64136413

6414+
/// Returns a descriptive name for the given accessor/addressor kind.
6415+
StringRef swift::getAccessorNameForDiagnostic(AccessorKind accessorKind,
6416+
bool article) {
6417+
switch (accessorKind) {
6418+
case AccessorKind::Get:
6419+
return article ? "a getter" : "getter";
6420+
case AccessorKind::Set:
6421+
return article ? "a setter" : "setter";
6422+
case AccessorKind::Address:
6423+
return article ? "an addressor" : "addressor";
6424+
case AccessorKind::MutableAddress:
6425+
return article ? "a mutable addressor" : "mutable addressor";
6426+
case AccessorKind::Read:
6427+
return article ? "a 'read' accessor" : "'read' accessor";
6428+
case AccessorKind::Modify:
6429+
return article ? "a 'modify' accessor" : "'modify' accessor";
6430+
case AccessorKind::WillSet:
6431+
return "'willSet'";
6432+
case AccessorKind::DidSet:
6433+
return "'didSet'";
6434+
case AccessorKind::Init:
6435+
return article ? "an init accessor" : "init accessor";
6436+
}
6437+
llvm_unreachable("bad accessor kind");
6438+
}
6439+
6440+
StringRef swift::getAccessorNameForDiagnostic(AccessorDecl *accessor,
6441+
bool article) {
6442+
return getAccessorNameForDiagnostic(accessor->getAccessorKind(),
6443+
article);
6444+
}
6445+
64146446
bool AbstractStorageDecl::hasStorage() const {
64156447
ASTContext &ctx = getASTContext();
64166448
return evaluateOrDefault(ctx.evaluator,
@@ -6478,7 +6510,8 @@ void AbstractStorageDecl::setAccessors(SourceLoc lbraceLoc,
64786510
auto record = Accessors.getPointer();
64796511
if (record) {
64806512
for (auto accessor : accessors) {
6481-
(void) record->addOpaqueAccessor(accessor);
6513+
if (!record->getAccessor(accessor->getAccessorKind()))
6514+
(void) record->addOpaqueAccessor(accessor);
64826515
}
64836516
} else {
64846517
record = AccessorRecord::create(getASTContext(),
@@ -6572,6 +6605,27 @@ void AbstractStorageDecl::AccessorRecord::addOpaqueAccessor(AccessorDecl *decl){
65726605
(void) isUnique;
65736606
}
65746607

6608+
void AbstractStorageDecl::AccessorRecord::removeAccessor(
6609+
AccessorDecl *accessor
6610+
) {
6611+
// Remove this accessor from the list of accessors.
6612+
assert(getAccessor(accessor->getAccessorKind()) == accessor);
6613+
std::remove(getAccessorsBuffer().begin(), getAccessorsBuffer().end(),
6614+
accessor);
6615+
6616+
// Clear out the accessor kind -> index mapping.
6617+
std::memset(AccessorIndices, 0, sizeof(AccessorIndices));
6618+
6619+
// Re-add all of the remaining accessors to build up the index mapping.
6620+
unsigned numAccessorsLeft = NumAccessors - 1;
6621+
NumAccessors = numAccessorsLeft;
6622+
auto buffer = getAccessorsBuffer();
6623+
NumAccessors = 0;
6624+
for (auto index : range(0, numAccessorsLeft)) {
6625+
addOpaqueAccessor(buffer[index]);
6626+
}
6627+
}
6628+
65756629
/// Register that we have an accessor of the given kind.
65766630
bool AbstractStorageDecl::AccessorRecord::registerAccessor(AccessorDecl *decl,
65776631
AccessorIndex index){

‎lib/Parse/ParseDecl.cpp

Lines changed: 8 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -6937,38 +6937,6 @@ void Parser::skipAnyAttribute() {
69376937
(void)canParseCustomAttribute();
69386938
}
69396939

6940-
/// Returns a descriptive name for the given accessor/addressor kind.
6941-
static StringRef getAccessorNameForDiagnostic(AccessorKind accessorKind,
6942-
bool article) {
6943-
switch (accessorKind) {
6944-
case AccessorKind::Get:
6945-
return article ? "a getter" : "getter";
6946-
case AccessorKind::Set:
6947-
return article ? "a setter" : "setter";
6948-
case AccessorKind::Address:
6949-
return article ? "an addressor" : "addressor";
6950-
case AccessorKind::MutableAddress:
6951-
return article ? "a mutable addressor" : "mutable addressor";
6952-
case AccessorKind::Read:
6953-
return article ? "a 'read' accessor" : "'read' accessor";
6954-
case AccessorKind::Modify:
6955-
return article ? "a 'modify' accessor" : "'modify' accessor";
6956-
case AccessorKind::WillSet:
6957-
return "'willSet'";
6958-
case AccessorKind::DidSet:
6959-
return "'didSet'";
6960-
case AccessorKind::Init:
6961-
return article ? "an init accessor" : "init accessor";
6962-
}
6963-
llvm_unreachable("bad accessor kind");
6964-
}
6965-
6966-
static StringRef getAccessorNameForDiagnostic(AccessorDecl *accessor,
6967-
bool article) {
6968-
return getAccessorNameForDiagnostic(accessor->getAccessorKind(),
6969-
article);
6970-
}
6971-
69726940
static void diagnoseRedundantAccessors(Parser &P, SourceLoc loc,
69736941
AccessorKind accessorKind,
69746942
bool isSubscript,
@@ -7023,15 +6991,6 @@ struct Parser::ParsedAccessors {
70236991
func(accessor);
70246992
}
70256993

7026-
/// Find the first accessor that's not an observing accessor.
7027-
AccessorDecl *findFirstNonObserver() {
7028-
for (auto accessor : Accessors) {
7029-
if (!accessor->isObservingAccessor())
7030-
return accessor;
7031-
}
7032-
return nullptr;
7033-
}
7034-
70356994
/// Find the first accessor that can be used to perform mutation.
70366995
AccessorDecl *findFirstMutator() const {
70376996
if (Set) return Set;
@@ -7448,8 +7407,14 @@ void Parser::parseTopLevelAccessors(
74487407

74497408
bool hadLBrace = consumeIf(tok::l_brace);
74507409

7451-
ParserStatus status;
7410+
// Prepopulate the field for any accessors that were already parsed parsed accessors
74527411
ParsedAccessors accessors;
7412+
#define ACCESSOR(ID) \
7413+
if (auto accessor = storage->getAccessor(AccessorKind::ID)) \
7414+
accessors.ID = accessor;
7415+
#include "swift/AST/AccessorKinds.def"
7416+
7417+
ParserStatus status;
74537418
bool hasEffectfulGet = false;
74547419
bool parsingLimitedSyntax = false;
74557420
while (!Tok.isAny(tok::r_brace, tok::eof)) {
@@ -7721,16 +7686,10 @@ void Parser::ParsedAccessors::classify(Parser &P, AbstractStorageDecl *storage,
77217686
// The observing accessors have very specific restrictions.
77227687
// Prefer to ignore them.
77237688
if (WillSet || DidSet) {
7724-
// For now, we don't support the observing accessors on subscripts.
7689+
// We don't support the observing accessors on subscripts.
77257690
if (isa<SubscriptDecl>(storage)) {
77267691
diagnoseAndIgnoreObservers(P, *this,
77277692
diag::observing_accessor_in_subscript);
7728-
7729-
// The observing accessors cannot be combined with other accessors.
7730-
} else if (auto nonObserver = findFirstNonObserver()) {
7731-
diagnoseAndIgnoreObservers(P, *this,
7732-
diag::observing_accessor_conflicts_with_accessor,
7733-
getAccessorNameForDiagnostic(nonObserver, /*article*/ true));
77347693
}
77357694
}
77367695

‎lib/Sema/TypeCheckMacros.cpp

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1346,23 +1346,24 @@ Optional<unsigned> swift::expandAccessors(
13461346
// side effect of registering those accessor declarations with the storage
13471347
// declaration, so there is nothing further to do.
13481348
bool foundNonObservingAccessor = false;
1349+
bool foundNonObservingAccessorInMacro = false;
13491350
bool foundInitAccessor = false;
1350-
for (auto decl : macroSourceFile->getTopLevelItems()) {
1351-
auto accessor = dyn_cast_or_null<AccessorDecl>(decl.dyn_cast<Decl *>());
1352-
if (!accessor)
1353-
continue;
1354-
1351+
for (auto accessor : storage->getAllAccessors()) {
13551352
if (accessor->isInitAccessor())
13561353
foundInitAccessor = true;
13571354

1358-
if (!accessor->isObservingAccessor())
1355+
if (!accessor->isObservingAccessor()) {
13591356
foundNonObservingAccessor = true;
1357+
1358+
if (accessor->isInMacroExpansionInContext())
1359+
foundNonObservingAccessorInMacro = true;
1360+
}
13601361
}
13611362

13621363
auto roleAttr = macro->getMacroRoleAttr(MacroRole::Accessor);
13631364
bool expectedNonObservingAccessor =
13641365
!accessorMacroOnlyIntroducesObservers(macro, roleAttr);
1365-
if (foundNonObservingAccessor) {
1366+
if (foundNonObservingAccessorInMacro) {
13661367
// If any non-observing accessor was added, mark the initializer as
13671368
// subsumed unless it has init accessor, because the initializer in
13681369
// such cases could be used for memberwise initialization.
@@ -1373,6 +1374,13 @@ Optional<unsigned> swift::expandAccessors(
13731374
binding->setInitializerSubsumed(index);
13741375
}
13751376
}
1377+
1378+
// Also remove didSet and willSet, because they are subsumed by a
1379+
// macro expansion that turns a stored property into a computed one.
1380+
if (auto accessor = storage->getParsedAccessor(AccessorKind::WillSet))
1381+
storage->removeAccessor(accessor);
1382+
if (auto accessor = storage->getParsedAccessor(AccessorKind::DidSet))
1383+
storage->removeAccessor(accessor);
13761384
}
13771385

13781386
// Make sure we got non-observing accessors exactly where we expected to.

‎lib/Sema/TypeCheckStorage.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3636,6 +3636,37 @@ StorageImplInfoRequest::evaluate(Evaluator &evaluator,
36363636

36373637
bool hasWillSet = storage->getParsedAccessor(AccessorKind::WillSet);
36383638
bool hasDidSet = storage->getParsedAccessor(AccessorKind::DidSet);
3639+
if ((hasWillSet || hasDidSet) && !isa<SubscriptDecl>(storage)) {
3640+
// Observers conflict with non-observers.
3641+
AccessorDecl *firstNonObserver = nullptr;
3642+
for (auto accessor : storage->getAllAccessors()) {
3643+
if (!accessor->isImplicit() && !accessor->isObservingAccessor()) {
3644+
firstNonObserver = accessor;
3645+
break;
3646+
}
3647+
}
3648+
3649+
if (firstNonObserver) {
3650+
if (auto willSet = storage->getParsedAccessor(AccessorKind::WillSet)) {
3651+
willSet->diagnose(
3652+
diag::observing_accessor_conflicts_with_accessor, 0,
3653+
getAccessorNameForDiagnostic(
3654+
firstNonObserver->getAccessorKind(), /*article=*/ true));
3655+
willSet->setInvalid();
3656+
hasWillSet = false;
3657+
}
3658+
3659+
if (auto didSet = storage->getParsedAccessor(AccessorKind::DidSet)) {
3660+
didSet->diagnose(
3661+
diag::observing_accessor_conflicts_with_accessor, 1,
3662+
getAccessorNameForDiagnostic(
3663+
firstNonObserver->getAccessorKind(), /*article=*/ true));
3664+
didSet->setInvalid();
3665+
hasDidSet = false;
3666+
}
3667+
}
3668+
}
3669+
36393670
bool hasSetter = storage->getParsedAccessor(AccessorKind::Set);
36403671
bool hasModify = storage->getParsedAccessor(AccessorKind::Modify);
36413672
bool hasMutableAddress = storage->getParsedAccessor(AccessorKind::MutableAddress);

‎test/Macros/Inputs/syntax_macro_definitions.swift

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -422,8 +422,7 @@ extension PropertyWrapperMacro: AccessorMacro, Macro {
422422
) throws -> [AccessorDeclSyntax] {
423423
guard let varDecl = declaration.as(VariableDeclSyntax.self),
424424
let binding = varDecl.bindings.first,
425-
let identifier = binding.pattern.as(IdentifierPatternSyntax.self)?.identifier,
426-
binding.accessor == nil
425+
let identifier = binding.pattern.as(IdentifierPatternSyntax.self)?.identifier
427426
else {
428427
return []
429428
}
@@ -468,6 +467,55 @@ extension PropertyWrapperMacro: PeerMacro {
468467
}
469468
}
470469

470+
extension PatternBindingSyntax.Accessor {
471+
var hasGetter: Bool {
472+
switch self {
473+
case .accessors(let accessors):
474+
for accessor in accessors.accessors {
475+
if accessor.accessorKind.text == "get" {
476+
return true
477+
}
478+
}
479+
480+
return false
481+
case .getter:
482+
return true
483+
}
484+
}
485+
}
486+
487+
public struct PropertyWrapperSkipsComputedMacro {}
488+
489+
extension PropertyWrapperSkipsComputedMacro: AccessorMacro, Macro {
490+
public static func expansion(
491+
of node: AttributeSyntax,
492+
providingAccessorsOf declaration: some DeclSyntaxProtocol,
493+
in context: some MacroExpansionContext
494+
) throws -> [AccessorDeclSyntax] {
495+
guard let varDecl = declaration.as(VariableDeclSyntax.self),
496+
let binding = varDecl.bindings.first,
497+
let identifier = binding.pattern.as(IdentifierPatternSyntax.self)?.identifier, !(binding.accessor?.hasGetter ?? false)
498+
else {
499+
return []
500+
}
501+
502+
return [
503+
"""
504+
505+
get {
506+
_\(identifier).wrappedValue
507+
}
508+
""",
509+
"""
510+
511+
set {
512+
_\(identifier).wrappedValue = newValue
513+
}
514+
""",
515+
]
516+
}
517+
}
518+
471519
public struct WrapAllProperties: MemberAttributeMacro {
472520
public static func expansion(
473521
of node: AttributeSyntax,

0 commit comments

Comments
 (0)