Skip to content

StringSwitchEcjFilter fails to recognize switch where default is first #741

Merged
marchof merged 1 commit intomasterfrom
ecj_switch_string
Aug 18, 2018
Merged

StringSwitchEcjFilter fails to recognize switch where default is first #741
marchof merged 1 commit intomasterfrom
ecj_switch_string

Conversation

@Godin
Copy link
Copy Markdown
Member

@Godin Godin commented Aug 18, 2018

	private static void example(Object s) {
		switch (String.valueOf(s)) { // assertFullyCovered(0, 2)
			default:
				nop("default");
				break;
			case "a":
				nop("case a");
				break;
		}
	}

in this case ECJ omits last goto expected by filter -

if (cursor.getNext().getOpcode() == Opcodes.GOTO) {
// end of comparisons for same hashCode
// jump to default
nextIs(Opcodes.GOTO);

Looks like that this is due to optimization in algorithm that generates goto instructions - https://github.com/eclipse/eclipse.jdt.core/blob/97b95fa05a64615fc7296744b69bd741cc4af2c7/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/codegen/CodeStream.java#L3544

which is used by algorithm that generates code for switch on String - https://github.com/eclipse/eclipse.jdt.core/blob/97b95fa05a64615fc7296744b69bd741cc4af2c7/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java#L245-L269

@Godin Godin self-assigned this Aug 18, 2018
@Godin Godin changed the title StringSwitchEcjFilter fails to recognize switch where default is first StringSwitchEcjFilter fails to recognize switch where default is first Aug 18, 2018
@Godin Godin added this to the 0.8.2 milestone Aug 18, 2018
@Godin Godin force-pushed the ecj_switch_string branch from 7e6e528 to 2f6341b Compare August 18, 2018 02:03
@Godin Godin force-pushed the ecj_switch_string branch from 2f6341b to fab9a65 Compare August 18, 2018 02:08
Copy link
Copy Markdown
Member

@marchof marchof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, how did you even discover this? I've never seen someone putting default first.

@marchof marchof merged commit e629bf0 into master Aug 18, 2018
@marchof marchof deleted the ecj_switch_string branch August 18, 2018 04:19
@Godin
Copy link
Copy Markdown
Member Author

Godin commented Aug 20, 2018

@marchof found this during check that #735 covers all cases from #496 , which mentions this case in comments 😉 don't remember how/why comment was originally added into #496 , however can admit that in C/C++ default as first case with early-return seems quite handy to me in some cases, especially in big switches

@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

Archived in project
Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants