Skip to content

Sema: Simplify adjustSelfTypeForMember() a little bit to avoid a cycle #34092

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 26, 2020

Conversation

slavapestov
Copy link
Contributor

We used to wrap the base expression in an InOutExpr when accessing a
computed property. This was a vestigial remnant of differences in the
SILGen code paths for stored vs computed property access.

These days SILGen doesn't care and is perfectly happy to call getters
and setters with an LValueType base as well.

This allows us to remove the call to getAccessSemantics(), which for
a 'didSet', had to kick off type checking of the body.

Fixes rdar://problem/69532933.

@slavapestov slavapestov requested review from xedin and CodaFi September 26, 2020 03:29
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

This check did not handle MemberRefExprs with an InOutExpr base,
giving it inconsistent behavior:

- With a class, we would diagnose self-assignment of computed
  properties

- With a struct, accesses to computed properties would build a
  MemberRefExpr with an InOutExpr base, so self-assignments
  were *not* diagnosed.

Let's tweak the check to consistently diagnose self-assignments
to stored properties only, instead of the emergent behavior
as described above.
We used to wrap the base expression in an InOutExpr when accessing a
computed property. This was a vestigial remnant of differences in the
SILGen code paths for stored vs computed property access.

These days SILGen doesn't care and is perfectly happy to call getters
and setters with an LValueType base as well.

This allows us to remove the call to getAccessSemantics(), which for
a 'didSet', had to kick off type checking of the body.

Fixes <rdar://problem/69532933>.
Also remove the unused AccessSemantics parameter.
@slavapestov slavapestov force-pushed the didset-access-semantics-cycle branch from 893036e to 71a281c Compare September 26, 2020 04:03
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

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

Awesome!

@DougGregor
Copy link
Member

@swift-ci please smoke test macOS

@slavapestov slavapestov merged commit 2fb3c7c into swiftlang:main Sep 26, 2020
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.

3 participants