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

Compile error when a function branch always returns early #4565

Open
junetried opened this issue Dec 4, 2024 · 8 comments
Open

Compile error when a function branch always returns early #4565

junetried opened this issue Dec 4, 2024 · 8 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@junetried
Copy link

With this code, ponyc complains the first line of maybe_error isn't reachable. This modified version compiles as expected.

Match seems to give a different error. This code incorrectly believes the function body is None.

Reproduced in ponyc versions 0.58.0 and 0.58.7.

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Dec 4, 2024
@junetried
Copy link
Author

I also found this interesting one while trying to reproduce it. If you try to run this code, it gives two errors: the type of 42 in the return can't be inferred, and that the if is unreachable. But, if you simply add the return type, it compiles without the unreachable error. If you annotate the type, but wrongly (like String), the unreachable error also goes away and it only complains that it can't infer the type of 42.

@chalcolith
Copy link
Member

chalcolith commented Dec 4, 2024

Note that if a match isn't exhaustive and doesn't have an else part, an implicit else None will be added: https://playground.ponylang.io/?gist=d16a22ee34051e3a359e2e9d15ebb448

@junetried
Copy link
Author

Note that if a match isn't exhaustive and doesn't have an else part, an implicit else None will be added: https://playground.ponylang.io/?gist=d16a22ee34051e3a359e2e9d15ebb448

This match is exhaustive, no?

@chalcolith
Copy link
Member

Hmm, it should be. @jemc might know more.

@SeanTAllen
Copy link
Member

I believe that comes not from exhaustive but from "jumps away early" being treated as a None by the compiler as all expressions have to return something and error returns None. That None might never happen, but it's still a thing.

@SeanTAllen SeanTAllen added help wanted Extra attention is needed bug needs investigation This needs to be looked into before its "ready for work" good first issue Good for newcomers labels Dec 5, 2024
@jemc
Copy link
Member

jemc commented Dec 17, 2024

Sean opened a new ticket for the "exhaustive match with true and false" use case. Our exhaustive match checking code in the compiler doesn't handle that case yet, but it could be extended to do so.

The initial code shown in the ticket is still its own bug.

@SeanTAllen SeanTAllen removed the needs investigation This needs to be looked into before its "ready for work" label Dec 17, 2024
@jemc
Copy link
Member

jemc commented Dec 17, 2024

Issue ticket #2792 (which reports two separate bugs) mentions this same bug for loops in the 2nd report.

@jemc
Copy link
Member

jemc commented Dec 17, 2024

We discussed this in the sync call today. Here's a short summary.

For functions where the return type signature is None, as a convenience to the programmer we insert an invisible None token in the AST so they don't have to explicitly return "None" themselves.

Separately, if there's code after a control block (like this if statement) which jumps away in every branch, we show a compiler error for the next expression after that block, if there is one.

These two features combined, give us the error above, with the invisible None expression being marked as unreachable. The error message points to the first expression in the function body because the invisible None expression borrows the location information from that expression.

To fix this, someone would need to update our unreachable code compiler error to check for this special case (where the next expression is None and is also the last expression) and skip the unreachable code error in that case.

Sean asked if we can instead avoid inserting the magic None in this case, but that is not feasible due to the order in which this info becomes available during the compiler passes.

@SeanTAllen SeanTAllen removed the discuss during sync Should be discussed during an upcoming sync label Jan 7, 2025
@SeanTAllen SeanTAllen removed the bug label Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants