Bugzilla – Full Text Bug Listing
| Summary: | [inline] Inline Local Variable does not qualify accesses to obscured types | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Max Schaefer <mschaefer> | ||||||||||
| Component: | UI | Assignee: | Jeff Johnston <jjohnstn> | ||||||||||
| Status: | VERIFIED FIXED | QA Contact: | Jeff Johnston <jjohnstn> | ||||||||||
| Severity: | major | ||||||||||||
| Priority: | P3 | CC: | daniel_megert, deepakazad, jjohnstn, markus.kell.r, max.schaefer, nikolaymetchev, noopur_gupta, reprogrammer | ||||||||||
| Version: | 3.8 | ||||||||||||
| Target Milestone: | 4.24 M1 | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | All | ||||||||||||
| See Also: |
https://git.eclipse.org/r/71284 https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/71284 https://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=cf1c41cc8054c1c3f198e19fbc5eeb4d7204d20a |
||||||||||||
| Whiteboard: | stalebug | ||||||||||||
| Attachments: |
|
||||||||||||
*** Bug 367534 has been marked as a duplicate of this bug. *** Created attachment 236458 [details] unit tests + fix This contribution complies with http://www.eclipse.org/legal/CoO.php Comment on attachment 236458 [details] unit tests + fix Nikolay, see the duplicate bug 367534 and handle that also in the same patch. Also, the patch attached in comment #2 would fail in examples like: public class A1 { int f = 23; static class B { static int f = 42; } int foo() { int r = bar(B.f); A1 B = this; return r; } private int bar(int x) { return x; } public static void main(String[] args) { System.out.println(new A1().foo()); } } Created attachment 236707 [details] please review This contribution complies with http://www.eclipse.org/legal/CoO.php Hello Noopur, I have coded up a fix to all the cases you have come up with. I think there are more. I just wanted you to code review the changes so far before I change too much code. I would like to know if I have gone about this the right way. A case that I know exits is in the patch (commented out) is one where the inlining is actually not possible. It seems right now the inline refactoring doesn't have a case where it is impossible to inline. Just curious should we worry about this one or not? (In reply to Nikolay Metchev from comment #5) > Created attachment 236707 [details] [diff] > I have coded up a fix to all the cases you have come up with. I think there > are more. I just wanted you to code review the changes so far before I > change too much code. The patch looks complicated with lot of conditions for different test cases. It does not take care of org.eclipse.jdt.core.dom.NameQualifiedType also. It can be simplified and in future it would help if you could also outline the approach used by you for the fix. > A case that I know exits is in the patch (commented out) is one where the > inlining is actually not possible. It can be inlined as: "return p.A.B.f;" Created attachment 242720 [details] Unit Test + Patch + Code Review This contribution complies with http://www.eclipse.org/legal/CoO.php Hello Noopur, You were right. That code I wrote 6 months ago seems to be very bad. I gave it another go. I have a couple more unit tests from before to handle the case where you pointed out that I could add the package name to the front of the qualification. Hopefully my code is a lot clearer this time. The general gist of my code change is: for each reference that is to be inlined find all the variables that could potentially clash. If any of them clash then find all alternative re-writings of the QualifiedName or SimpleName that feature in the initializer and for each one see if it clashes. If one is found that doesn't clash then use it. P.S. I couldn't figure out a test case where NameQualifiedType would cause a problem. Can you please provide me with a test case? I have come up with a case which breaks my patch:
I will submit a new patch once I figure out how to fix it
package p.q.r;
public class A {
static class B {
static int f = 22;
static A B = new A();
}
static int f = 42;
static Runnable foo() {
Runnable r = new Runnable() {
A B = new A();
@Override
public void run() {
int r = B.f;
}
};
A B = new A();
return r;
}
}
Created attachment 242960 [details] Unit Tests + Patch + Code review This contribution complies with http://www.eclipse.org/legal/CoO.php I was able to clean up my previous patch with slightly cleaner code. It now handles a couple more corner cases. Please upload the rebased patch on Gerrit for review in 4.7. New Gerrit change created: https://git.eclipse.org/r/71284 Now that Neon is out can I expect this to be looked at? This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie. Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/71284 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=cf1c41cc8054c1c3f198e19fbc5eeb4d7204d20a Released for 4.24 M1 Verified for 4.24 M1 using I20220406 build |
Build Identifier: M20110909-1335 Consider the following example program: public class A { int f = 23; static class B { static int f = 42; } int foo() { int r = B.f; A B = this; return r; } public static void main(String[] args) { System.out.println(new A().foo()); } } This program prints 42. Inlining variable r yields the following program, which prints 23: public class A { int f = 23; static class B { static int f = 42; } int foo() { A B = this; return B.f; } public static void main(String[] args) { System.out.println(new A().foo()); } } The problem is that the local variable B obscures class A.B, hence B.f binds to the instance field f of variable B, and no longer to the static field f of class B. Reproducible: Always Steps to Reproduce: See details above.