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
