Skip to content

Fix a couple of crash-on-invalid situations with local variables defined inside result builders #35812

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

Conversation

slavapestov
Copy link
Contributor

@slavapestov slavapestov commented Feb 7, 2021

Declarations inside result builders don't go via the usual typeCheckDecl() code path. Most are simply rejected, but local variables are allowed. We were missing some validation checks:

  • We need to reject non-stored local variables, and not just local variables without an initializer expression. We used to crash while trying to emit 'lazy', property wrappers and observers inside result builders.
  • We need to perform attribute validation, otherwise we allow bogus attributes, which were either ignored or could also cause crashes.

Fixes rdar://problem/73545981.

We would previously reject properties without an initializer expression,
but this is not quite sufficient, because of 'lazy', property wrappers,
and observers.

We could probably support all three in result builder contexts, but
for now, let's diagnose them like other computed properties, instead of
crashing.

Fixes part of <rdar://problem/73545981>.
@slavapestov slavapestov force-pushed the result-builder-invalid-var-crash branch from b560fed to 6714723 Compare February 7, 2021 04:38
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

This looks great, thanks!

}

// Also check for invalid attributes.
TypeChecker::checkDeclAttributes(var);
Copy link
Member

Choose a reason for hiding this comment

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

Whoops, this is a really important one. Thanks!

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