Change 'actor class' to 'actor' #35794

etcwilde · 2021-02-05T20:20:17Z

We've decided to go with calling actor classes actor instead of actor class.
We still want to be able to parse in actor class and recommend the correct fix-its to make it just actor. This is a little challenging since we parse other modifiers in any order, so actor public class Foo and public actor class Foo were both totally parsable. We still need to be able to do that, which makes this change less pleasant.

Furthermore, we want to make it a contextual keyword instead of an actual keyword so that we avoid breaking as much source. This also adds to the complexity.

I will update the diagnostics to refer to actor later.

Resolves: rdar://73728503

ktoso · 2021-02-05T20:42:44Z

test/decl/class/actor/basic.swift


 class NonActor { }

-actor class NonActorSubclass : NonActor { } // expected-error{{actor class cannot inherit from non-actor class 'NonActor'}}
+actor NonActorSubclass : NonActor { } // expected-error{{actor class cannot inherit from non-actor class 'NonActor'}}


should we instead be calling them "actor" rather than actor class in all diagnostics from here on?

Yeah, that seems like a good idea.

etcwilde · 2021-02-09T19:40:31Z

@swift-ci please test

DougGregor

This looks great; just two trivial comments that shouldn't block you from merging.

DougGregor · 2021-02-09T19:42:05Z

lib/Parse/ParseDecl.cpp

@@ -4148,6 +4206,13 @@ Parser::parseDecl(ParseDeclOptions Flags,
  // Obvious nonsense.
  default:

+    if (shouldParseExperimentalConcurrency() && Tok.is(tok::identifier) &&


You don't need the Tok.is(too::identifier) here, because it's part of Tok.isContextualKeyword.

DougGregor · 2021-02-09T19:43:12Z

include/swift/AST/Decl.h

@@ -3587,7 +3590,8 @@ class ClassDecl final : public NominalTypeDecl {
 public:
  ClassDecl(SourceLoc ClassLoc, Identifier Name, SourceLoc NameLoc,
            ArrayRef<TypeLoc> Inherited,
-            GenericParamList *GenericParams, DeclContext *DC);
+            GenericParamList *GenericParams, DeclContext *DC,
+            bool isActor=false);


It might be worth dropping the default argument so we don't forget to pass it somewhere in the future.

Dropping it got a little nastier than intended. The unit tests have a makeNominal that will construct something based on a template argument. That thing is either a ClassDecl or a StructDecl.

Ended up with this little specialization of makeNominal. Not great, but not terrible.

template <typename Nominal> Nominal *makeNominal(StringRef name, GenericParamList *genericParams = nullptr) { auto result = new (Ctx) Nominal(SourceLoc(), Ctx.getIdentifier(name), SourceLoc(), /*inherited*/{}, genericParams, FileForLookups); result->setAccess(AccessLevel::Internal); return result; } template <> swift::ClassDecl *makeNominal(StringRef name, GenericParamList *genericParams) { auto result = new (Ctx) ClassDecl(SourceLoc(), Ctx.getIdentifier(name), SourceLoc(), /*inherited*/{}, genericParams, FileForLookups, /*isActor*/false); result->setAccess(AccessLevel::Internal); return result; }

etcwilde · 2021-02-09T20:03:13Z

@swift-ci please test

swift-ci · 2021-02-09T20:22:12Z

Build failed
Swift Test OS X Platform
Git Sha - 67cd3ad32151259ee2562d3d39ecd4e66bff53be

swift-ci · 2021-02-09T20:31:48Z

Build failed
Swift Test Linux Platform
Git Sha - 67cd3ad32151259ee2562d3d39ecd4e66bff53be

etcwilde · 2021-02-09T21:15:29Z

Heh, there were more places where constructed ClassDecls. :) I think I have all of them fixed, just testing locally.

etcwilde · 2021-02-09T22:08:36Z

@swift-ci please test

etcwilde · 2021-02-09T22:10:51Z

@swift-ci please test

swift-ci · 2021-02-09T22:48:17Z

Build failed
Swift Test Linux Platform
Git Sha - ecf7b81c31610a42911766da39b19edf865ddd5d

etcwilde · 2021-02-09T23:35:19Z

@swift-ci please test

swift-ci · 2021-02-10T00:55:58Z

Build failed
Swift Test OS X Platform
Git Sha - 1fcc7065f3bdeef8729c8e9fa5e58ebf95b8ad5d

swift-ci · 2021-02-10T01:05:32Z

Build failed
Swift Test Linux Platform
Git Sha - 1fcc7065f3bdeef8729c8e9fa5e58ebf95b8ad5d

etcwilde · 2021-02-10T01:12:52Z

@swift-ci please test

etcwilde · 2021-02-10T04:21:50Z

@swift-ci please smoke test

swift-ci · 2021-02-10T04:25:38Z

Build failed
Swift Test OS X Platform
Git Sha - 15ca7a4cd6e476db08379131ed7f697234ec7633

This patch softly updates the spelling of actors from `actor class` to `actor`. We still accept using `actor` as a modifying attribute of class, but emit a warning and fix-it to make the change. One of the challenges that makes this messier is that the modifier list can be in any order. e.g, `public actor class Foo {}` is the same as `actor public class Foo {}`. Classes have been updated to include whether they were explicitly declared as an actor. This change updates the swiftmodule serialization version number to 0.591. The additional bit only gets set of the class declaration was declared as an actor, not if the actor was applied as an attribute. This allows us to correctly emit `actor class` vs `actor` emitting the code back out.

This patch updates the once instance of actor class in the concurrency library to actor. This is the actor for the MainActor global actor.

This patch updates the `actor class` spelling to `actor` in almost all of the tests. There are places where I verify that we sanely handle `actor` as an attribute though. These include: - test/decl/class/actor/basic.swift - test/decl/protocol/special/Actor.swift - test/SourceKit/CursorInfo/cursor_info_concurrency.swift - test/attr/attr_objc_async.swift - test/ModuleInterface/actor_protocol.swift

etcwilde · 2021-02-10T16:09:51Z

@swift-ci please smoke test

etcwilde requested a review from rintaro February 5, 2021 20:20

ktoso reviewed Feb 5, 2021

View reviewed changes

etcwilde force-pushed the ewilde/all-the-world-is-a-stage-with-actors branch 2 times, most recently from ad4664d to 4487f9c Compare February 9, 2021 19:24

etcwilde marked this pull request as ready for review February 9, 2021 19:35

DougGregor approved these changes Feb 9, 2021

View reviewed changes

etcwilde force-pushed the ewilde/all-the-world-is-a-stage-with-actors branch from c611913 to 67cd3ad Compare February 9, 2021 20:02

etcwilde force-pushed the ewilde/all-the-world-is-a-stage-with-actors branch from 67cd3ad to d34e546 Compare February 9, 2021 21:59

etcwilde force-pushed the ewilde/all-the-world-is-a-stage-with-actors branch from d34e546 to ecf7b81 Compare February 9, 2021 22:10

etcwilde force-pushed the ewilde/all-the-world-is-a-stage-with-actors branch from ecf7b81 to 1fcc706 Compare February 9, 2021 23:13

etcwilde force-pushed the ewilde/all-the-world-is-a-stage-with-actors branch from 1fcc706 to 15ca7a4 Compare February 10, 2021 01:12

etcwilde force-pushed the ewilde/all-the-world-is-a-stage-with-actors branch from 15ca7a4 to a9ab561 Compare February 10, 2021 04:21

etcwilde added 3 commits February 10, 2021 08:05

Update standard library actor class

748f15e

This patch updates the once instance of actor class in the concurrency library to actor. This is the actor for the MainActor global actor.

etcwilde force-pushed the ewilde/all-the-world-is-a-stage-with-actors branch from a9ab561 to 8b80331 Compare February 10, 2021 16:09

etcwilde merged commit 1f3879b into apple:main Feb 10, 2021

etcwilde deleted the ewilde/all-the-world-is-a-stage-with-actors branch February 10, 2021 18:46

etcwilde mentioned this pull request Feb 11, 2021

Updating ClassDecl to also accept actor apple/swift-syntax#259

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change 'actor class' to 'actor' #35794

Change 'actor class' to 'actor' #35794

etcwilde commented Feb 5, 2021 •

edited

ktoso Feb 5, 2021

etcwilde Feb 5, 2021

etcwilde commented Feb 9, 2021

DougGregor left a comment

DougGregor Feb 9, 2021

DougGregor Feb 9, 2021

etcwilde Feb 9, 2021

etcwilde commented Feb 9, 2021

swift-ci commented Feb 9, 2021

swift-ci commented Feb 9, 2021

etcwilde commented Feb 9, 2021

etcwilde commented Feb 9, 2021

etcwilde commented Feb 9, 2021

swift-ci commented Feb 9, 2021

etcwilde commented Feb 9, 2021

swift-ci commented Feb 10, 2021

swift-ci commented Feb 10, 2021

etcwilde commented Feb 10, 2021

etcwilde commented Feb 10, 2021

swift-ci commented Feb 10, 2021

etcwilde commented Feb 10, 2021

Change 'actor class' to 'actor' #35794

Change 'actor class' to 'actor' #35794

Conversation

etcwilde commented Feb 5, 2021 • edited

ktoso Feb 5, 2021

Choose a reason for hiding this comment

etcwilde Feb 5, 2021

Choose a reason for hiding this comment

etcwilde commented Feb 9, 2021

DougGregor left a comment

Choose a reason for hiding this comment

DougGregor Feb 9, 2021

Choose a reason for hiding this comment

DougGregor Feb 9, 2021

Choose a reason for hiding this comment

etcwilde Feb 9, 2021

Choose a reason for hiding this comment

etcwilde commented Feb 9, 2021

swift-ci commented Feb 9, 2021

swift-ci commented Feb 9, 2021

etcwilde commented Feb 9, 2021

etcwilde commented Feb 9, 2021

etcwilde commented Feb 9, 2021

swift-ci commented Feb 9, 2021

etcwilde commented Feb 9, 2021

swift-ci commented Feb 10, 2021

swift-ci commented Feb 10, 2021

etcwilde commented Feb 10, 2021

etcwilde commented Feb 10, 2021

swift-ci commented Feb 10, 2021

etcwilde commented Feb 10, 2021

etcwilde commented Feb 5, 2021 •

edited