Forbid case-after-default, tweak error messages

[vlstill]

Fixes #4290. This forbids compilation of code which contains a switch label after a default label. This behaviour matches the P4 spec ($12.7.3):

It is optional to have a switch case with the default label, but if one is present, it must be the last one in the switch statement.

See #4290 (comment) for detains why I believe there is no way to allow this and be consistent.

I've also changed the error messages to be more readable. Note that neither SwitchStatement nor DefaultExpression have a reasonable string representation for the error messages, so there is no point in including anything more that the debug info in the message.

@fruffy, while this can't break "assumptions in back-end code" I guess it should still be marked as breaking change as it can actually break compilation of P4. Such P4 was not adhering to spec though and it should be simple to fix it.

[vlstill]

This issue apparently has nothing to do with case-after-default, and it just masks the "real" problem (the one the issue referred to).

[jafingerhut]

Would it perhaps be a good idea to add a modified version of default-switch.p4, that puts default last, in case that helps improve test coverage of whatever issue it was originally created for? That modified version would have no errors, and so belong in the testdata/p4_16_samples directory, where default-switch.p4 is before this PR.

[jafingerhut]

Regarding the comment about a breaking change, yes I agree this is a kind of breaking change.

Fortunately, it is what I think could be termed a "safe" breaking change: any program that breaks because of it, will have a clear error message and a straightforward path to update the code to work again.

As opposed to what I would call an "unsafe" breaking change, where some programs change their behavior with no warning or error messages at compile time. Those are the really nasty ones I would strongly suggest we bump up the major version of the language, if we make a change like that. I can't think of one that has been made in P4_16, or if there was, it was very early on, perhaps before version 1.0 was released.

[asl]

Fortunately, it is what I think could be termed a "safe" breaking change: any program that breaks because of it, will have a clear error message and a straightforward path to update the code to work again.

I think the semantics for "breaking change" was (as this is in description): "This change may break assumptions of compiler back ends", not about the input source. Yes, it may break malformed input, but I think it is ok, that next version of compiler is more strict wrt the spec.

This certainly does not affect backends

[kfcripps]

I think the semantics for "breaking change" was (as this is in description): "This change may break assumptions of compiler back ends", not about the input source.

Agree with Anton. Maybe we should add a different label for changes that cause existing input sources to no longer compile, or to have different semantics?

[fruffy]

Agree with Anton. Maybe we should add a different label for changes that cause existing input sources to no longer compile, or to have different semantics?

We could probably broaden the definition of the p4-spec label since this is a spec-related change.

[jafingerhut]

LGTM. As usual, I am reviewing the test programs and their expected outputs, and not paying careful attention to the C++ code changes (even as small as they are).

[vlstill]

Agree with Anton. Maybe we should add a different label for changes that cause existing input sources to no longer compile, or to have different semantics?

We could probably broaden the definition of the p4-spec label since this is a spec-related change.

Yep I think the p4-spec label matches it well (I'm not sure if you have already modified it :-D). I've added p4-spec and removed breaking-change.

[vlstill]

Would it perhaps be a good idea to add a modified version of default-switch.p4, that puts default last, in case that helps improve test coverage of whatever issue it was originally created for? That modified version would have no errors, and so belong in the testdata/p4_16_samples directory, where default-switch.p4 is before this PR.

Right, I should have done it that way originally. Added the positive version of the test now.