On 1/09/2012 5:42 AM, Mike Duigou wrote:
The changes look good to me.
I am starting to come into agreement with Remi though that unless a type
specific array can be created for situations like the E[] elements array in
ArrayDeque then it should be declared as Object[] array since that's what is
actually created.
The fact we can not create an E[] is a limitation of generics. The
elements should be an E[] though. Better one cast on array creation than
a cast on every element access.
David
Mike
On Aug 31 2012, at 12:06 , Dan Xu wrote:
On 08/30/2012 06:45 PM, Stuart Marks wrote:
On 8/30/12 7:14 AM, Ulf Zibis wrote:
Am 30.08.2012 01:20, schrieb Stuart Marks:
On 8/29/12 4:36 AM, Ulf Zibis wrote:
@SuppressWarnings("fallthrough") is put to suppress warnings generated by
another switch/case statements
Can't you move it from method scope to there?
while (i>= matchlen&& !seencomma) {
switch(a[i-matchlen]) {
case ',':
seencomma = true;
/*FALLTHROUGH*/
case ' ': case '\r': case '\n':
case '\f': case '\t':
break;
default:
throw new IllegalArgumentException(
"invalid permission: " + actions);
}
i--;
}
Unfortunately there is no suitable place to put the annotation. Annotations
can only be placed on declarations, and the narrowest enclosing declaration
around this switch statement is, unfortunately, the entire method. One might
consider refactoring the switch statement into its own method, but that kind
of refactoring is too large a change for warnings cleanup.
I can see that it is reasonable/possible to bind a
@SuppressWarnings("unchecked") to a declaration which contains the unchecked
assignment, but which declaration should contain a fallthrough case?
So shouldn't @SuppressWarnings("fallthrough") better be binded to a switch or
case statement?
In principle it would be nice to place the @SuppressWarnings right next to the
switch statement for which the warnings are suppressed. However, it is a
syntactic restriction of the Java programming language that annotations can
only be used as modifiers on declarations, specifically package, class, method,
method parameter, constructor, and local variable declarations. See JLS 9.7
[1]. It is not legal to place annotations on statements. So, the
SuppressWarnings annotation has to be placed on a declaration that encloses the
switch statement in question. In this case that's the declaration of the entire
method.
[1] http://docs.oracle.com/javase/specs/jls/se7/html/jls-9.html#jls-9.7
It turns out that this discussion is moot, as the switch fall-through is
actually unnecessary! The comma case falls through to the next case, but all
that does is break. The warning can be eliminated simply by replacing
/*FALLTHROUGH*/ with a break statement. This removes the need for the
SuppressWarnings annotation.
This permissions-parsing code occurs in several places around the JDK, copied
and slightly modified. Some of them use the string identity comparison
optimization, and some have this switch fall through. (We've discussed this
previously. See [2].) Accordingly, I've asked Dan to adjust a couple of these
other cases to have the same comment about string identity comparisons, and to
change the switch statement to avoid the fall-through entirely.
[2] http://mail.openjdk.java.net/pipermail/jdk8-dev/2011-December/000415.html
While I'm reluctant to expand the scope of this changeset, I think it's
important to keep these five occurrences consistent. Obviously it would be
nicer to unify the five copies into one, but this is a much bigger change that
I think is clearly out of scope.
Dan will issue another webrev with these changes shortly.
s'marks
I have adjusted my fix with the above suggestions.
The latest webrev is posted at,
http://cr.openjdk.java.net/~dxu/7193406/webrev.05/. Thanks for your comments!
-Dan
