Skip to content

Conversation

@AlexsanderDamaceno
Copy link

As stated in #83538, Isolated keyword should not be permitted in inheritance clause

NeverNullType
TypeResolver::resolveIsolatedTypeRepr(IsolatedTypeRepr *repr,
TypeResolutionOptions options) {
// isolated is only value for non-EnumCaseDecl parameters.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: maybe fix the typo in this comment while in here

Suggested change
// isolated is only value for non-EnumCaseDecl parameters.
// isolated is only valid for non-EnumCaseDecl parameters.

Copy link
Author

Choose a reason for hiding this comment

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

@jamieQ changed

(options.hasBase(TypeResolverContext::EnumElementDecl) &&
!options.is(TypeResolverContext::Inherited)) || options.is(TypeResolverContext::Inherited))) {
diagnoseInvalid(
repr, repr->getSpecifierLoc(), diag::attr_only_on_parameters,
Copy link
Contributor

Choose a reason for hiding this comment

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

aside: this diagnostic no longer seems accurate since isolated is now valid on deinits.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, will do a pr removing this

options.hasBase(TypeResolverContext::EnumElementDecl)) &&
!options.is(TypeResolverContext::Inherited)) {
(options.hasBase(TypeResolverContext::EnumElementDecl) &&
!options.is(TypeResolverContext::Inherited)) || options.is(TypeResolverContext::Inherited))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we actually need the inheritance check here? seems it was added here (or possibly here depending on your view of history), but wasn't present before. perhaps the other relevant changes from those PRs should be taken into account as well.

Copy link
Author

@AlexsanderDamaceno AlexsanderDamaceno Dec 12, 2025

Choose a reason for hiding this comment

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

it looks that this changes are related to related to use isolated in actors

}

protocol P {}
struct S: isolated P {} // expected-error {{'isolated' may only be used on parameters}}
Copy link
Contributor

@jamieQ jamieQ Dec 12, 2025

Choose a reason for hiding this comment

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

nit: the referenced 'enum decl' case should probably also have a test (i didn't see any when searching for this error message).

edit: when testing it out, seems this will just crash the compiler:

enum E {
  case iso(isolated any Actor)
}

but i guess that's a separate bug i will file filed: #86007

@AlexsanderDamaceno
Copy link
Author

@xedin hey, could take a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants