Skip to content
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

Fix #22604: Propagate info about terminating branches #22608

Closed
wants to merge 6 commits into from

Conversation

SirOlaf
Copy link
Contributor

@SirOlaf SirOlaf commented Aug 31, 2023

Close #22604

Propagates a new nfEndBlock flag for branches that terminate the block, improves type inference and unreachable code detection.
I don't know what sempass2 is for, so if that needs to handle this situation too please provide feedback.

Thinks the echo for "hey iterator" in #20362 is unreachable even though it prints, so either that case needs to be made consistent or the propagation needs to somehow work around this behavior.

Alternative to this pr would be making endsInNoReturn check the appropriate node types.

@SirOlaf SirOlaf changed the title Test CI Fix #22604: Propagate info about terminating branches Aug 31, 2023
@SirOlaf SirOlaf marked this pull request as ready for review August 31, 2023 20:21
@Araq
Copy link
Member

Araq commented Aug 31, 2023

Alternative to this pr would be making endsInNoReturn check the appropriate node types.

That's what you need to do. Node flags are unreliable and the wrong idea.

@SirOlaf
Copy link
Contributor Author

SirOlaf commented Aug 31, 2023

Doing that seemed more expensive to me because you gotta traverse the tree each time and keep track of the branches. Other problem is that the unreachable code check does not actually rely on it either (less problematic).

@Araq
Copy link
Member

Araq commented Sep 1, 2023

Actually, using some special internal type for this seems to be both easier and faster.

@SirOlaf
Copy link
Contributor Author

SirOlaf commented Sep 1, 2023

Yes like that never type brought up by @metagn, effectively the same as this pr but instead of a weird flag it's an actual part of the type system

@Araq
Copy link
Member

Araq commented Sep 1, 2023

Not quite, it would be an internal type for now that should not be exposed.

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.

Missing type inference in loop + case + if + continue
2 participants