Community
Participate
Working Groups
I20080330-1350 Released (inactive) test case org.eclipse.jdt.compiler.tool.tests.CompilerInvocationTests#007. Considering the boolean handleOption(String current, Iterator<String> remaining) method, the eclipse compiler: - does not support a remaining parameter backed by a non-modifiable iterator, while javac does; must be a different interpretation of the meaning for the consumption of remaining items: javac probably does next on the iterator, while ecj (according to sources) attempts a remove; - consumes one more item than javac (ecj considers that current is also the first item in remaining, while javac considers it is not); reading the javadoc for handleOption, I believe that remaining is only expected to start with the arguments for current, if any.
The javadoc is: Handles one option. If current is an option to this file manager it will consume any arguments to that option from remaining and return true, otherwise return false. I don't see where it is said that remaining is only expected to start with the arguments for current. The arguments are supposed to be consumed when arguments are expected.
At least the javadoc does not say it will consume anything but 'arguments to that option'. However, my second assertion is wrong (there is only one call to next() in the code - which means that the iterator is not 'nexted' more than needed - I was blindsided by the call to remove). Which would mean that, as far as shifting the iterator is concerned, the right behavior is implemented. It still remains true that passing an Iterator that does not support the remove operation throws an IllegalStateException, which I believe is not acceptable, especially since the default standard Java file manager does not make this tilt. Aligned the title accordingly.
Created attachment 96044 [details] Tentative fix + test case Would need more test coverage (only testing the -d option so far).
(In reply to comment #2) > It still remains true that passing an Iterator that does not support the remove > operation throws an IllegalStateException, which I believe is not acceptable, > especially since the default standard Java file manager does not make this > tilt. My claim is that this is not specified. It could be an implementation detail. The "spec" says "consume". From a spec point of view, this is weak as it doesn't say how to consume the elements in the iterator. I choose to remove the elements to make sure that we don't process them twice. The file manager "removes" the options that it knows how to handle them and leave the remaining ones for other to process.
The API only gives us an Iterator. Not all Iterator-s support the remove operation and, unless the APIs tells the client that only some Iterator-s are supported, he is free to use any Iterator he wishes. Hence I believe that the spec, while it does not insist so, makes clear that we should not tilt on any Iterator (as long as the options they carry are OK with us). Another angle from which we could consider the question is the following: if I use next() upon an Iterator, how could I go back and process the same element again? This would need that I go to something more sophisticated (a two-ways cursor or the underlying storage), which I wouldn't do inadvertently.
Created attachment 96204 [details] Fix + test cases Same fix, with improved test coverage (leverages the options lists of CompilerToolTests).
Released for 3.4 M7. Kent please yell if anything looks odd.
Verified for 3.4M7 using I20080427-2000
