Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 545983

Summary: [12] EJC allows switch expression to be concluded without returning a value
Product: [Eclipse Project] JDT Reporter: Nir Lisker <nlisker>
Component: CoreAssignee: Manoj N Palat <manoj.palat>
Status: VERIFIED FIXED QA Contact:
Severity: major    
Priority: P3 CC: jarthana, manoj.palat
Version: 4.9   
Target Milestone: 4.12 M3   
Hardware: PC   
OS: Windows 10   
See Also: https://git.eclipse.org/r/141124
https://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=a8d59be0a1fe090c489c1eb9fda14a27bfb7fb74
https://git.eclipse.org/r/142563
https://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=3d670fe48cd3ec2fe4d3f7a2fca5830a4057291b
Whiteboard:

Description Nir Lisker CLA 2019-03-31 13:40:45 EDT 
The last code snippet from https://openjdk.java.net/jeps/325 does not function correctly in Eclipse:

	for (int i = 0; i < 20; ++i) {
		int k = switch (i) {
			case 0:
				break 1;
			case 1:
				break 2;
			default:
				continue;
			// ERROR! Illegal jump through a switch expression 
		};
		System.out.println(k);
	}

The compiler does not complain where there should be the error in the comments. Running this code prints the following to the error stream (the class name I ran this code in is GenericExpressionSwitch):

Error: Unable to initialize main class various.GenericExpressionSwitch
Caused by: java.lang.VerifyError: Operand stack overflow
Exception Details:
  Location:
    various/GenericExpressionSwitch.main([Ljava/lang/String;)V @43: iload_2
  Reason:
    Exceeded max stack size.
  Current Frame:
    bci: @43
    flags: { }
    locals: { '[Ljava/lang/String;', integer, integer }
    stack: { 'java/io/PrintStream' }
  Bytecode:
    0000000: 033c a700 301b aa00 0000 001e 0000 0000
    0000010: 0000 0001 0000 0016 0000 001a 04a7 000a
    0000020: 05a7 0006 a700 0b3d b200 101c b600 1684
    0000030: 0101 1b10 14a1 ffd0 b1                 
  Stackmap Table:
    append_frame(@5,Integer)
    same_frame(@28)
    same_frame(@32)
    same_frame(@36)
    same_locals_1_stack_item_frame(@39,Integer)
    same_frame(@47)
    same_frame(@50)
Comment 1 Eclipse Genie CLA 2019-04-25 06:10:58 EDT 
New Gerrit change created: https://git.eclipse.org/r/141124
Comment 2 Manoj N Palat CLA 2019-04-25 06:14:39 EDT 
(In reply to Eclipse Genie from comment #1)
> New Gerrit change created: https://git.eclipse.org/r/141124

Patch above flags this code as illegal.

[Side-note: With the change envisaged in https://mail.openjdk.java.net/pipermail/amber-spec-experts/2019-April/001232.html  we should be able to improve the error message in Java 13 time-frame]
Comment 4 Jay Arthanareeswaran CLA 2019-05-21 02:51:08 EDT 
I have verified that compiler now reports the expected problem, but the new error message could be better worded/formatted:

Illegal last statement in Switch expression case body - continue, return not allowed

I think we should use one of

"continue and return are not allowed"

 (or)

"Illegal last statement: continue"

but having both is redundant. If we are using the first, we should use ":" instead of "-" for consistency sake. And for the latter, it should be "and/or= instead of ",".

Probably not worth a new bug and we can take this up now or for RC1?
Comment 5 Manoj N Palat CLA 2019-05-21 05:37:52 EDT 
(In reply to Jay Arthanareeswaran from comment #4)
> I have verified that compiler now reports the expected problem, but the new
> error message could be better worded/formatted:
> 
See comment 2 - will change the error message in Java 13 time frame.
Comment 6 Manoj N Palat CLA 2019-05-21 12:10:21 EDT 
(In reply to Manoj Palat from comment #5)
> (In reply to Jay Arthanareeswaran from comment #4)
> > I have verified that compiler now reports the expected problem, but the new
> > error message could be better worded/formatted:
> > 
> See comment 2 - will change the error message in Java 13 time frame.

@Jay: Please note that the relevant part in the spec is undergoing a change, expected to be stabilized in a few weeks. Hence this message will be changed anyway once there is an agreement in the spec.
Comment 7 Jay Arthanareeswaran CLA 2019-05-21 23:37:18 EDT 
(In reply to Manoj Palat from comment #6)
> @Jay: Please note that the relevant part in the spec is undergoing a change,
> expected to be stabilized in a few weeks. Hence this message will be changed
> anyway once there is an agreement in the spec.

I am not sure if that will happen before 4.12? If not, this should go into 4.12. This message will be seen by users and probably translated as well. So, we should make it consistent with our standards.
Comment 8 Eclipse Genie CLA 2019-05-21 23:43:58 EDT 
New Gerrit change created: https://git.eclipse.org/r/142563
Comment 9 Manoj N Palat CLA 2019-05-22 00:45:13 EDT 
(In reply to Jay Arthanareeswaran from comment #7)

Updating the result of an offline discussion: Jay will go ahead with this patch for 4.12. Further changes once the spec freezes will be targeted for 4.13.
Comment 11 Jay Arthanareeswaran CLA 2019-05-22 06:18:44 EDT 
Verified for 4.12 M3 using build I20190521-1800