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

Evaluation order of cases #858

Closed
XmiliaH opened this issue May 9, 2024 · 7 comments
Closed

Evaluation order of cases #858

XmiliaH opened this issue May 9, 2024 · 7 comments
Labels
question Further information is requested

Comments

@XmiliaH
Copy link
Contributor

XmiliaH commented May 9, 2024

Currently the evaluation of cases is from top to bottom with the exception that cases that map to default will not be evaluated at all.

This could be somewhat unexpected in cases such as

switch 1 do
    case 2: break
    case 1: default: print("default") break
    case 1: print("one") break
end

where I would have expected that the default case would be used.

I would like if the evaluation order is either defined to be from top to bottom with only the default case being an exception or explicitly undefined, which could allow to test all constant cases first and then the dynamic cases.

Futhermore, the optiomisation to not run cases mapping to default seem to me to contradict the statement

The default case is executed if none of the other cases are true.

from the documentation https://pluto-lang.org/docs/New%20Features/Switch%20Blocks, but these cases are not even executed which can be observable.

@Sainan Sainan added the question Further information is requested label May 9, 2024
@Sainan
Copy link
Collaborator

Sainan commented May 9, 2024

Mind you that the website documentation is not like a language standards document which has to "define behaviour" or have "undefined behaviour."

I think your given example code is nonsensical due to having multiple cases with the same condition, and it's reasonable to assume that this is undefined behaviour and the compiler has the right to optimise this in some ways. In this case, the compiler does optimise it — maybe for a different reason than why the code is nonsensical — but it has the right to do so either way.

Pluto should maybe raise a warning for the duplicate switch case so you could fix your code (because, again, it's clearly nonsensical) as was already mentioned in #649, although it was kind of forgotten about.

@XmiliaH
Copy link
Contributor Author

XmiliaH commented May 9, 2024

The example is obviously nonsensical, it should just show the point. Maybe it would have been better to replace the last case with a call to some random function which could or could not return 1.

@Sainan
Copy link
Collaborator

Sainan commented May 9, 2024

I get that casecond allows function calls, and we made sure to make the switch blocks flexible like that, but when it comes to optimisations, it has to be based on real-world considerations, and people simply don't write this kind of code. And if they did, I would say the compiler is right to have done the optimisation because your code made certain implications.

I would document this as a "pitfall", but I don't feel like such theoretical code is enough to justify that.

@XmiliaH
Copy link
Contributor Author

XmiliaH commented May 9, 2024

I would be happy about some form of notice that the order is up to the compiler and that cases can be optimised fully out.

This would actually very nice for an optimisation for the switch statement I was writing. It would allow to move all the constants up and test them first with a table lookup to map each case to an integer and do a binary search to make switches O(log n) and then test in linear time the dynamic cases.

@Sainan
Copy link
Collaborator

Sainan commented May 9, 2024

I will reiterate that the website documentation is not a standards document, but I will also point out that at no point is it guaranteed that the conditions are evaluated in the order they are written, and I think the general expectation kind of is that the compiler can and will reorder them if it helps it execute the code faster; the optimisations just have to work as expected in all the "real-world" cases.

@XmiliaH
Copy link
Contributor Author

XmiliaH commented May 9, 2024

There we disagree. I have the expectation (especially for dynamic languages) that things are executed from top to bottom, left to right if not specified otherwise. Maybe this is caused by the languages I have seen.

Furthermore, I would not expect a compiler to just optimise away expressions with side effects, as done here with casevalues that map to default.

Note that this is just my view and others might differ, but there might be other developers out there wo have the same view and for them it would be nice to have a notice that the order is not defined and the default case is executed if none of the other cases that map to non-default cases are true.

@Sainan
Copy link
Collaborator

Sainan commented May 9, 2024

Well, what I have been doing professionally for the last couple of years is C++ and there switches are just notoriously well-optimised and result in pretty unpredictable code. Ideally, we would like to have the same happen with Pluto generating Lua bytecode. And with this as a guideline, we can see that in this case, it's actually an error:

switch (1) {
case 1: default: printf("1 alpha\n"); break;
case 1: printf("1 beta\n"); break; //  error : duplicate case value '1'
}

and forget about calling functions inside of the case conditions...

So, I think Pluto is in a bit of a weird situation, where the kind of optimisations that happen to the switch blocks are actually observable due to how flexible they were designed. Maybe we will have to compromise on either the flexibility or the optimisations, but right now I don't see an issue with having both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants