-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[test] Add some extra variants in capture_order.swift
#85257
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
Conversation
test/SILGen/capture_order.swift
Outdated
| } | ||
| } | ||
|
|
||
| // FIXME: should/could these be diagnosed instead of accepted? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tangential: after thinking about these cases a bit more, i'm not sure the commentary is warranted (i.e. recording my past confusion), and perhaps they should be removed. given how the implementation of lazy variables works, i don't think these have the same issues as the non-lazy cases. the first one will infinitely recurse, but that's 'fine', and the second one is confusing, but there's a level of indirection where the actual storage is the implicitly-synthesized lazy storage variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think these cases are at least consistent with the rule where local functions are allowed to forward reference vars, and their behavior should be well-defined AFAIK. Interestingly the local function version crashes in SILGen though:
do {
func foo() -> Any { x }
lazy var x = foo()
}Filed #85266
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer it if local functions couldn't forward-reference vars. The only decl you really need to be able to forward reference is another local function. We can't change that without breaking source compatibility though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I would love if we could ban forward referencing local vars across the board in Sema
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would the benefit primarily be to eliminate implementation complexity, or are there other reasons this would be desirable? in theory something like this could be language mode gated, but if the existing handling has to exist 'forever' to support older language modes, then perhaps there would not be much point in trying to do that?
The existing tests we have here I'm hoping to reject in Sema, add equivalent variants that use local functions which will continue to be caught in SILGen.
These cases are odd, but well defined.
8582209 to
828b10a
Compare
|
@swift-ci please smoke test |
The existing tests we have here I'm hoping to reject in Sema, add equivalent variants that use local functions which will continue to be caught in SILGen.