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

StringSwitchJavacFilter fails to recognize switch that has just a few cases #730

Merged
merged 1 commit into from
Aug 13, 2018

Conversation

Godin
Copy link
Member

@Godin Godin commented Aug 13, 2018

StringSwitchJavacFilter fails to recognize switch that has just a few cases

	private static void example(Object s) {
		switch (String.valueOf(s)) {
		case "a":
			nop("case a");
			break;
		case "b":
			nop("case b");
			break;
		}
	}

In this case javac generates lookupswitch, while filter expects tableswitch -

lookupswitch was noticed by @forax during my talk at Devoxx France and seems to be due to following algorithm that estimates space and time costs between lookupswitch and tableswitch - https://github.com/dmlloyd/openjdk/blob/84ba2b90d46ee4b06664d32594ef0cd2038373a6/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java#L1170

@Godin
Copy link
Member Author

Godin commented Aug 13, 2018

@marchof could you please review?

Also maybe you can advice how to recording this in changelog - wording? bug fix or new feature?

@Godin Godin requested a review from marchof August 13, 2018 18:54
@Godin Godin added this to the 0.8.2 milestone Aug 13, 2018
@Godin Godin added this to IN PROGRESS in Current work items Aug 13, 2018
@marchof marchof merged commit aa77868 into master Aug 13, 2018
@marchof marchof deleted the issue-730 branch August 13, 2018 20:23
@marchof
Copy link
Member

marchof commented Aug 13, 2018

@Godin Ups, sorry, I overlooked your question. For me is a bug fix, because we claimed that we can filter switch in string for javac. But probably no one ever noticed this corner case. Anyway, if we want to fix the latest commit here is my proposal:

Also filter part of bytecode generated for switch statements on java.lang.String values when only a few cases are present.

@Godin
Copy link
Member Author

Godin commented Aug 13, 2018

@marchof Changelog is also useful for us 😉 Fixed in f2c79fe

@Godin Godin added the type: bug 🐛 Something isn't working label Aug 13, 2018
@Godin Godin removed this from IN PROGRESS in Current work items Aug 13, 2018
@Godin Godin added this to DONE in Filtering Aug 13, 2018
@Godin Godin added this to In Review in Current work items via automation Aug 14, 2018
@Godin Godin moved this from In Review to Done in Current work items Aug 14, 2018
@jacoco jacoco locked as resolved and limited conversation to collaborators Oct 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Filtering
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants