SwitchPatternTests failing in I-builds #2986
Conversation
|
Have you looked at the failing test in the verification build? Hopefully, it's not caused by the change here. |
|
Btw, if we(committers) find it too much of an effort to fill the PR template maybe we should remove it? @eclipse-jdt/eclipse-jdt-committers wdyt? |
That failures is unrelated - so is the failure that has happened with the repeat run. I have ran those tests locally and are passing. Am planning to merge it |
I have filled the template now - thanks for pointing out. I think we can keep the template |
PR template is too much overhead to fill out on daily bases, but as long as i can delete that part i don't care. For non-committers it's a good explanation what we expect though. |
I had the same thought many times. For first time contributors the template may make sense as a guideline of what information we need. Do non-committer contributors use the template as intended? I don't have any figures. When committers leave the template untouched, this sets a poor example. I personally figure, I don't need to specify how to test, e.g., because I will already include unit tests that give that exact information. Being a committer also adhering to coding conventions and ECA can be assumed even if I don't specify this every time. But even we should keep in mind that github issues and PRs are an essential source of information for reviewers (now) and maintainers (later). As a consequence, I personally have the habit of replacing the template with information that is to the point for committer-to-committer communication. Does anyone object to that? |
| this.isPrimitiveSwitch | ||
| ? exprType.signature() | ||
| : "Ljava/lang/Object;".toCharArray(); //$NON-NLS-1$ |
There was a problem hiding this comment.
there are two more locations below this block which still use exprType.signature() unconditionally. Shouldn't those have the same guard, too?
There was a problem hiding this comment.
... or, simpler still, make an early return if !this.isPrimitiveSwitch.
There was a problem hiding this comment.
yep, will take a look - will wait for an I build first to confirm the fix
fixes issue #2982
What it does
fixes issue #2982
The return value of the CallSite [used by invokedynamic of typeSwitch] has changed in Java 23 - we need to check that if its a primitive enabled switch pattern (meaning that its 23 + preview then use the new signature else old]
How to test
run
org.eclipse.jdt.core.tests.compiler.regression.SwitchPatternTest testBug574525_03 - at level 21 and 22 with a 21 and 22 jre respectively.
-- | --
Author checklist