Bug 411312 - compile error in nested statement should not make whole method non-executable
Summary: compile error in nested statement should not make whole method non-executable
Status: NEW
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 4.3   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: JDT-Core-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-06-20 15:35 EDT by Markus Keller CLA
Modified: 2013-11-19 06:37 EST (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2013-06-20 15:35:51 EDT
A compile error in a nested statement should not make the whole method non-executable.

Example:

class Try {
	public static void main(String[] args) {
		System.out.println("Hello");
		
		if (true) {
			System.out.println(x);
		}
	}
}

The unresolved variable 'x' should not turn the whole method body into a
'throw new Error("Unresolved compilation problem: ...")'. It's enough to throw the error when the if-statement's "then" branch is entered. The compiler should not prevent the user from debugging code that doesn't enter the problematic block. Note that in this example, the user can just extract the 'System.out.println(x);' into a method, and then he can debug the first statement of the main method.


The reason for this request is obviously not the simple example as given here. I had to debug/fix a multi-screen-long monster method [1], and I couldn't easily fix all branches after changing the type of a variable. But I was sure that my test case wouldn't enter the branches that still had compile errors. Nevertheless, I had to comment out a lot of code just to be able to debug into the error-free branches.

[1] org.eclipse.jdt.core.dom.ASTConverter#convertType(TypeReference)
Comment 1 Jay Arthanareeswaran CLA 2013-07-16 01:21:32 EDT
Interesting proposal! This would also mean that the compiled files will be bigger when there are compilation errors. Of course,at this point I don't even know how complicated it's going to be to implement, though.
Comment 2 Stephan Herrmann CLA 2013-07-17 15:19:49 EDT
Mh, what would it take to implement?
Currently, generation of problem methods is controlled by
AbstractMethodDeclaration.ignoreFurtherInvestigation which is set
from many locations including the central call referenceContext.tagAsHavingErrors()
inside ProblemHandler#handle.

In order to record this in a more fine grained fashion we could use a flag in
all BlockScopes. Now, having only a ReferenceContext we don't have the affected scope,
i.e., inside ProblemHandler cannot record against the least affected scope, yet.

I could think of a new wrapper 
  class ErrorContext {
      BlockScope scope;
      ReferenceContext referenceContext;
  }
so then Scope.problemReporter() could assemble an instance of the above wrapper,
to sneak the scope into the ProblemHandler. The transitive fan-in of handle() is huge.
We can still hope that most call chains actually feed ProblemHandler.referenceContext
into this parameter, so most methods in ProblemReporter could be unaffected.

Next question: how much short-circuiting do we do when seeing ignoreFurtherInvestion?
My guess: with the current behavior we won't have sufficient information for actually
generating code inside a method with any error inside (e.g., no analyseCode at all).
OTOH, I wouldn't like to change anything of how ignoreFurtherInvestigation is used
(only add additional flags in scopes), because the impact would be huge, I'm afraid.

Due to this difficulty - unless I'm missing anything - I suggest closing as WONTFIX.
Comment 3 Jan Vrany CLA 2013-11-17 13:33:49 EST
I would be also very interested in this feature [1]. If I understand correctly,
the main problem is to keep track of errors at the statement (or parse node) level because the code is to designed to work this way. 

I'm new to the codebase so maybe following idea is completely stupid, but 
what about following: 

instead of marking individual statements as erroneous, replace 
them by corresponding `throw new java.lang.Error(...)` statement and then
restart the analysis.

Could this work? 

[1] http://permalink.gmane.org/gmane.comp.ide.eclipse.jdt.devel/330
Comment 4 Jay Arthanareeswaran CLA 2013-11-18 01:14:09 EST
(In reply to Jan Vrany from comment #3)
> 
> Could this work? 
> 
> [1] http://permalink.gmane.org/gmane.comp.ide.eclipse.jdt.devel/330

Theoretically, it would work. We can introduce this behavior with a UI option. But then I don't yet know what it takes to implement this.
Comment 5 Srikanth Sankaran CLA 2013-11-18 20:45:18 EST
All problems in computer science can be solved by another level of indirection ?
Comment 6 Srikanth Sankaran CLA 2013-11-18 20:59:07 EST
(In reply to Jan Vrany from comment #3)

> instead of marking individual statements as erroneous, replace 
> them by corresponding `throw new java.lang.Error(...)` statement and then
> restart the analysis.
> 
> Could this work? 

In general, you cannot make *any* visible changes to the parse tree. If you do,
the AST converter would convert the modified program and that would get 
serialized back into the original buffer.

Today, we have "Problem types", "Problem methods" and "Problem constructors".
(See ClassFile.addProblem* and ClassFile.createProb*)

If you want to reduce the granularity to the level of blocks, it should be
possible by introducing a level of indirection in org.eclipse.jdt.internal.compiler.ast.Block.generateCode(BlockScope, CodeStream).

Instead of emitting code for this.statements, you will have to generate code
for statements obtained by a method call which would return this.statements
if error free or return s suitable block for erroneous blocks.

You will also need the ability to map a problem to its closest block. This is
possible by having the reference context the current block. As blocks get
entered and existed in various recursive descent traversals, they need to
collaborate with the reference context to maintain a consistent picture.

It could be bit of a challenge to get the class file to pass verification,
if the stack map table captures one block while the emitted code is something
altogether different.

I am sure there are lots of other fun challenges all along.
Comment 7 Srikanth Sankaran CLA 2013-11-18 21:01:43 EST
(In reply to Srikanth Sankaran from comment #6)

> You will also need the ability to map a problem to its closest block. This is
> possible by having the reference context the current block. As blocks get

by having the reference context _track_ the current block.


> entered and existed in various recursive descent traversals, they need to

entered and exited
Comment 8 Jan Vrany CLA 2013-11-19 06:37:43 EST
Thanks for the comment. Is there any documentation/text on how the eclipse compiler internally works? So far I have only vague idea what all this 
mean. 

Indeed, it would be o lot of fun to fix all this.