Community
Participate
Working Groups
Build Identifier: 20110916-0149 See the code in Steps to Reproduce. Errors should flag s_atomic1, s_atomic2, and s_atomic3. s_atomic1's fieldName doesn't point to any member variable in Enhancement. s_atomic2's fieldName points to a member variable but it is of the wrong type. s_atomic3's fieldName points to a member variable but it isn't volatile. Reproducible: Always Steps to Reproduce: 1. Enter the following code 2. Save and see that no errors are flagged private static final AtomicIntegerFieldUpdater<Bug> s_atomic1 = AtomicIntegerFieldUpdater.newUpdater(Bug.class, "noSuchField"); private static final AtomicIntegerFieldUpdater<Bug> s_atomic2 = AtomicIntegerFieldUpdater.newUpdater(Bug.class, "incorrectFieldType"); private static final AtomicIntegerFieldUpdater<Bug> s_atomic3 = AtomicIntegerFieldUpdater.newUpdater(Bug.class, "fieldNotVolatile"); private volatile long incorrectFieldType; private int fieldNotVolatile;
You're passing strings in a method call (and not fields) and the strings are: noSuchField incorrectFieldType fieldNotVolatile In the method call, these strings do not have any connection whatsoever to any fields/members or a java type. Marking as INVALID.
Switching to an enhancement request since the compiler (?) has to be enhanced to detect the bug. At runtime, if the strings or fields aren't declared correctly, then the JVM will throw an Error. This means thorough testing is required to make sure that each atomic updater is working properly. If Eclipse could flag an error, then the user wouldn't have to wait for testing (or production) to figure out that the code has a bug in it.
(In reply to comment #2) >If Eclipse could flag an error, then What error are you talking about? There's no error in your code. It is syntactically correct and valid java code. >if the strings or fields aren't declared correctly which fields aren't declared correctly? I don't see any such fields in your code. In a call such as AtomicIntegerFieldUpdater.newUpdater(Bug.class, "noSuchField"); how can you expect the compiler to interpret what the passed string actually means? The compiler only knows that you passed a string whose value is "noSuchField", not the fact that you want it to point to some existing field, ok? This is legal code and the compiler cannot do anything else. I will close as INVALID once again unless you actually report a bug. I see no bug until now.
This is NOT a bug. This is an *enhancement* request. Yes, the compiler doesn't report an error. Yes, this is syntactically correct Java code. However, the code won't load at runtime. The JVM will throw a RuntimeException for s_atomic1 and an IllegalArgumentException for s_atomic2 and s_atomic3. This enhancement request is to add an option to Window | Preferences | Java | Compiler | Errors/Warnings | Potential programming problems. The feature will flag this kind of code as a problem.
(In reply to comment #4) > The feature will > flag this kind of code as a problem. This is not possible, if my understanding of your problem is correct. All I have here is a few lines of code which doesn't even define AtomicIntegerFieldUpdater.newUpdater(..). You probably intend to pass the name of a field as the second parameter into this method. You want the compiler to match a string value to some existing field and complain if the field doesn't exist or is not declared properly. If this is your enhancement request, please note that no compiler can make such a prediction.Are we on the same page?
I think we are on the same page. Although, I am not sure why a compiler or bytecode checker tool couldn't do this. In the byte code, we would see the following in the static initializer... 0: ldc #1 // class Bug 2: ldc #29 // String noSuchField 4: invokestatic #31 // Method java/util/concurrent/atomic/AtomicIntegerFieldUpdater.newUpdater:(Ljava/lang/Class;Ljava/lang/String;)Ljava/util/concurrent/atomic/AtomicIntegerFieldUpdater; 7: putstatic #37 // Field s_atomic1:Ljava/util/concurrent/atomic/AtomicIntegerFieldUpdater; Here's how I envision the feature working. It will... 1) See the invokestatic instruction on Atomic___FieldUpdater.newUpdater. 2) Look at what class and string are being passed as a parameters. 3) Examine the fields of the referred class. 4) Find the field that has the same name as the string parameter. a) If no field with that name exists, flag a problem. 5) Check the field to ensure it is the right type. 6) Check the field to ensure it is a volatile.
Reading the generated bytecode to do some static checking after compliation is done is very wrong from a fundamental point of view. Even if there can be better ways of doing this, we currently do not have enough time to work on such a feature for reflection. Please feel free to provide a patch.
There is more than one way to solve this. One could write a regular expression to parse the Java code. Then figure out the class and field name. Here's a regular expression which puts into group #1 the class parameter and group #2 the field name. All that's left to do is decode the class parameter and check the class's field. Atomic(?:Integer|Long|Reference)FieldUpdater\s*\.\s*newUpdater\s*\(\s*([^,]+?)\s*,\s*"([^"]*)"\s*\)\s*;
I think the fundamental difference is that the Eclipse Java Compiler is defined for compiling Java as specified in the JLS. This means that the compiler has no idea about the semantics of specific methods in specific libraries (even from the JRE). What Nathan would like to see OTOH is a tool that knows that some methods in certain classes Atomic*FieldUpdater require a string that should correlate to a Java field. When Ayush says that the request is basically impossible, this doesn't mean we're out of ideas to find an algorithm, but that such algorithm would need to use information (about the intended semantics of specific methods) which are none of the compiler's business. Yet another way to look at this: the underlying flaw is in the design of Java's reflection. Classes can be safely passed around using MyClass.class syntax, which the compiler can check. Unfortunately, there is no such syntax for fields, hence the clumsy workaround to use strings, which are not checkable by the compiler, not by any rules defined in the JLS. Please also note, that the constructors of the field updaters directly perform lots of checks, which means that such errors are found by the simplest smoke tests. Hence the advantage of checking this at compile time wouldn't really be huge.
This enhancement is nice to have since it can flag the mistakes as the engineer is writing the code. This cuts down on the amount of wasted time of running a test to only find out it is broken.
When the variable is renamed via refactoring, the corresponding String parameter in Atomic___FieldUpdater.newUpdater should be changed.
(In reply to comment #10) > This enhancement is nice to have since it can flag the mistakes as the engineer > is writing the code. This cuts down on the amount of wasted time of running a > test to only find out it is broken. That's what tests are for: to find out if anything is broken. Even with the best of compilers you'll need tests. Currently our point is: it's not the business of any compiler to interpret strings in a Java program and you'd need very good arguments to convince us otherwise. (In reply to comment #11) > When the variable is renamed via refactoring, the corresponding String parameter > in Atomic___FieldUpdater.newUpdater should be changed. Please try the dialog based refactoring and check "Update textual occurrences in comments and strings ..." I see no need for an answer unless you have really new and convincing arguments why among all tools a Java compiler should spend the time on detecting this kind of error. I'll leave it to Ayushman to close this bug again.
Help me understand. I believe this feature would be a perfect fit for something like FindBugs. But, this feature would not be something for an IDE to do?
(In reply to comment #13) > Help me understand. I believe this feature would be a perfect fit for > something like FindBugs. But, this feature would not be something for an IDE > to do? If the IDE starts doing everything people want it to do, the same people will start complaining, "Hey, Eclipse is just too slow". :) Plus, you missed the point about "Eclipse is an open source, extensible platform". An extension to JDT which does this specific analysis will be preferred solution here. We will be happy to help with APIs, if relevant. Anyway, I will keep this bug open in case you want to submit a patch which does not go against the principles of the compiler. Both proposed solutions above go in the wrong direction in this sense.
Verified for 3.8 M5
