Re: Remaining doclint issues in java.net

2013-08-02 Thread Stuart Marks

On 8/2/13 2:37 AM, Chris Hegarty wrote:

On 01/08/2013 22:18, Stuart Marks wrote:

SocketAddress overloads. Two of them were within methods that declared
"throws Exception." The third was within a try/catch block that catches
IOException. None of the three cases would suffer a source incompatibility.


I cannot comment on the code you are talking about, but removing 'throws SE'
from these constructors will affect anyone that is catching IOE ( since SE is a
subclass of IOE ). Simple test:  [...]


You're correct, of course, that constructing a new DatagramPacket within a 
try/catch of IOException would suffer the source incompatibility.


I had neglected to mention, though, that the try/catch(IOException) code that I 
looked at also did a bunch of other IOException-throwing stuff within the try 
block. So in that particular case the code wouldn't have had a compatibility issue.



This isn't definitive, of course, but it does seem to supply some
evidence that making this change would result in a relatively minor
source incompatibility.


Given the above, I still agree with the sentiment here. I will file a bug and
proceed with the necessary changes to remove SE.


Great! Glad to hear it. Thanks.

s'marks



Re: Remaining doclint issues in java.net

2013-08-02 Thread Chris Hegarty

On 01/08/2013 23:08, Alan Bateman wrote:

On 01/08/2013 14:18, Stuart Marks wrote:

:

To my eye the InetAddress/port constructors are used quite a bit more
often than the SocketAddress ones. I did a web search for "java
DatagramPacket example" and looked at all the examples on the first
page of hits. All of them used the InetAddress+port constructor
overloads (including the Oracle Java tutorial). I didn't see any uses
of the SocketAddress overloads.

That's what I would expect as DatagramPacket is old and and pre-dates
the SocketAddress abstraction.

One other thing to check is whether this API bug has ever been reported.
If not then it suggests that not too many people have actually run into
it. If these constructors are widely used then I would expect that the
bug would be reported before now.


I can find not such bug(s) reporting this issue. But I will file one now ;-)

-Chris.



-Alan





Re: Remaining doclint issues in java.net

2013-08-02 Thread Chris Hegarty

comments inline...

On 01/08/2013 22:18, Stuart Marks wrote:

On 7/31/13 2:39 PM, Matthew Hall wrote:

On Wed, Jul 31, 2013 at 02:38:26PM -0700, Stuart Marks wrote:

The alternative is to add "@throws SocketException never" to the
javadoc, just to get rid of the doclint warning, but this has the
consequence of requiring people to keep dead code around
indefinitely, and furthermore it requires them to add new dead code
every time they create a DatagramPacket.


I have never understood in many years using Java why the compiler
generates
errors about attempting to catch supposedly-impossible exceptions,
instead of
warnings.

For me it only leads to difficulties when I'm trying to write
prototypes or
refactor and clean up some old brittle code, and I run into that rather
dubious sort of error.

This is a good example of where it causes more harm than good. Is
there still
a really good reason for this over-paranoid compiler error given that
checked
exceptions aren't as popular as they used to be anyways?


I don't have the definitive answer, but it was a very early Java design
decision to make unreachable code be a compilation error instead of a
warning. This is a matter of judgment, of course; reasonable people come
down on different sides of this, for different cases. For example, see
this Stackoverflow conversation:

http://stackoverflow.com/questions/3795585/why-does-java-have-an-unreachable-statement-compiler-error


* * *

Meanwhile, I did a little bit of studying about DatagramPacket. The
doclint warnings occur on two of the six DatagramPacket constructors:

DatagramPacket(byte[] buf, int len, SocketAddress sa)
DatagramPacket(byte[] buf, int off, int len, SocketAddress sa)

There are two other equivalent overloaded constructors that take
InetAddress and port arguments:

DatagramPacket(byte[] buf, int len, InetAddress ia, int port)
DatagramPacket(byte[] buf, int off, int len, InetAddress ia, int port)

To my eye the InetAddress/port constructors are used quite a bit more
often than the SocketAddress ones. I did a web search for "java
DatagramPacket example" and looked at all the examples on the first page
of hits. All of them used the InetAddress+port constructor overloads
(including the Oracle Java tutorial). I didn't see any uses of the
SocketAddress overloads.

I also did an ohloh search for "new DatagramPacket(":

http://code.ohloh.net/search?s=%22new%20DatagramPacket%28%22&p=2&pp=0&fl=Java&mp=1&ml=1&me=1&md=1&ff=1&filterChecked=true


Thank you Stuart, this is really helpful.


(HT: Dalibor)

I looked through around 100 examples, and the vast majority of them use
the InetAddress+port overloads. I saw three examples that use the


I would tend to agree with this, the InetAddress+port are probably most 
commonly used. The SocketAddress variants are useful when you have 
already constructed an InetSocketAddress elsewhere and pass it into the 
method that creates/sends the packet. I can only find this sort of code 
in tests.


Also worth noting is that typically datagram packets are created in 
close proximity to the code that sends them, and the sending of a packet 
requires handling IOE. This, IMO, further reduces the impact of removing 
'throw SE'.



SocketAddress overloads. Two of them were within methods that declared
"throws Exception." The third was within a try/catch block that catches
IOException. None of the three cases would suffer a source incompatibility.


I cannot comment on the code you are talking about, but removing 'throws 
SE' from these constructors will affect anyone that is catching IOE ( 
since SE is a subclass of IOE ). Simple test:


: cat ThrowingCtrs.java
import java.net.*;
import java.io.IOException;

public class ThrowingCtrs {

public static void main(String[] args) {
SocketAddress sa = new InetSocketAddress("127.0.0.1", 45678);

// DatagramPacket(byte[], int, SocketAddress)
try { DatagramPacket dp = new DatagramPacket(new byte[1], 1, sa); }
catch (SocketException x) { }

try { DatagramPacket dp = new DatagramPacket(new byte[1], 1, sa); }
catch (IOException x) { }

try { DatagramPacket dp = new DatagramPacket(new byte[1], 1, sa); }
catch (Exception x) { }

// DatagramPacket(byte[], int, int, SocketAddress)
try { DatagramPacket dp = new DatagramPacket(new byte[1], 0, 1, 
sa); }

catch (SocketException x) { }

try { DatagramPacket dp = new DatagramPacket(new byte[1], 0, 1, 
sa); }

catch (IOException x) { }

try { DatagramPacket dp = new DatagramPacket(new byte[1], 0, 1, 
sa); }

catch (Exception x) { }
}
}

Output of javac from a build with SE removed.
 build/solaris-x86_64-normal-server-release/jdk/bin/javac ThrowingCtrs.java
ThrowingCtrs.java:11: error: exception SocketException is never thrown 
in body of corresponding try statement

catch (SocketException x) { }
^
ThrowingCtrs.java:14: error: exception IOException is never thro

Re: Remaining doclint issues in java.net

2013-08-01 Thread Joe Darcy

On 08/01/2013 03:08 PM, Alan Bateman wrote:

On 01/08/2013 14:18, Stuart Marks wrote:

:

To my eye the InetAddress/port constructors are used quite a bit more 
often than the SocketAddress ones. I did a web search for "java 
DatagramPacket example" and looked at all the examples on the first 
page of hits. All of them used the InetAddress+port constructor 
overloads (including the Oracle Java tutorial). I didn't see any uses 
of the SocketAddress overloads.
That's what I would expect as DatagramPacket is old and and pre-dates 
the SocketAddress abstraction.


One other thing to check is whether this API bug has ever been 
reported. If not then it suggests that not too many people have 
actually run into it. If these constructors are widely used then I 
would expect that the bug would be reported before now.


-Alan 


Given the results of Stuart's initial investigation, I would favor 
removing the throws clauses and calling out the change in the release notes.


Cheers,

-Joe


Re: Remaining doclint issues in java.net

2013-08-01 Thread Alan Bateman

On 01/08/2013 14:18, Stuart Marks wrote:

:

To my eye the InetAddress/port constructors are used quite a bit more 
often than the SocketAddress ones. I did a web search for "java 
DatagramPacket example" and looked at all the examples on the first 
page of hits. All of them used the InetAddress+port constructor 
overloads (including the Oracle Java tutorial). I didn't see any uses 
of the SocketAddress overloads.
That's what I would expect as DatagramPacket is old and and pre-dates 
the SocketAddress abstraction.


One other thing to check is whether this API bug has ever been reported. 
If not then it suggests that not too many people have actually run into 
it. If these constructors are widely used then I would expect that the 
bug would be reported before now.


-Alan





Re: Remaining doclint issues in java.net

2013-08-01 Thread Matthew Hall
On Thu, Aug 01, 2013 at 02:18:01PM -0700, Stuart Marks wrote:
> This isn't definitive, of course, but it does seem to supply some
> evidence that making this change would result in a relatively minor
> source incompatibility.

Good, maybe it will allow a true fix to be made in this case.

Matthew.


Re: Remaining doclint issues in java.net

2013-08-01 Thread Stuart Marks

On 7/31/13 2:39 PM, Matthew Hall wrote:

On Wed, Jul 31, 2013 at 02:38:26PM -0700, Stuart Marks wrote:

The alternative is to add "@throws SocketException never" to the
javadoc, just to get rid of the doclint warning, but this has the
consequence of requiring people to keep dead code around
indefinitely, and furthermore it requires them to add new dead code
every time they create a DatagramPacket.


I have never understood in many years using Java why the compiler generates
errors about attempting to catch supposedly-impossible exceptions, instead of
warnings.

For me it only leads to difficulties when I'm trying to write prototypes or
refactor and clean up some old brittle code, and I run into that rather
dubious sort of error.

This is a good example of where it causes more harm than good. Is there still
a really good reason for this over-paranoid compiler error given that checked
exceptions aren't as popular as they used to be anyways?


I don't have the definitive answer, but it was a very early Java design 
decision to make unreachable code be a compilation error instead of a warning. 
This is a matter of judgment, of course; reasonable people come down on 
different sides of this, for different cases. For example, see this 
Stackoverflow conversation:


http://stackoverflow.com/questions/3795585/why-does-java-have-an-unreachable-statement-compiler-error

* * *

Meanwhile, I did a little bit of studying about DatagramPacket. The doclint 
warnings occur on two of the six DatagramPacket constructors:


DatagramPacket(byte[] buf, int len, SocketAddress sa)
DatagramPacket(byte[] buf, int off, int len, SocketAddress sa)

There are two other equivalent overloaded constructors that take InetAddress 
and port arguments:


DatagramPacket(byte[] buf, int len, InetAddress ia, int port)
DatagramPacket(byte[] buf, int off, int len, InetAddress ia, int port)

To my eye the InetAddress/port constructors are used quite a bit more often 
than the SocketAddress ones. I did a web search for "java DatagramPacket 
example" and looked at all the examples on the first page of hits. All of them 
used the InetAddress+port constructor overloads (including the Oracle Java 
tutorial). I didn't see any uses of the SocketAddress overloads.


I also did an ohloh search for "new DatagramPacket(":

http://code.ohloh.net/search?s=%22new%20DatagramPacket%28%22&p=2&pp=0&fl=Java&mp=1&ml=1&me=1&md=1&ff=1&filterChecked=true

(HT: Dalibor)

I looked through around 100 examples, and the vast majority of them use the 
InetAddress+port overloads. I saw three examples that use the SocketAddress 
overloads. Two of them were within methods that declared "throws Exception." 
The third was within a try/catch block that catches IOException. None of the 
three cases would suffer a source incompatibility.


This isn't definitive, of course, but it does seem to supply some evidence that 
making this change would result in a relatively minor source incompatibility.


s'marks


Re: Remaining doclint issues in java.net

2013-07-31 Thread Matthew Hall
On Wed, Jul 31, 2013 at 02:38:26PM -0700, Stuart Marks wrote:
> The alternative is to add "@throws SocketException never" to the
> javadoc, just to get rid of the doclint warning, but this has the
> consequence of requiring people to keep dead code around
> indefinitely, and furthermore it requires them to add new dead code
> every time they create a DatagramPacket.

I have never understood in many years using Java why the compiler generates 
errors about attempting to catch supposedly-impossible exceptions, instead of 
warnings.

For me it only leads to difficulties when I'm trying to write prototypes or 
refactor and clean up some old brittle code, and I run into that rather 
dubious sort of error.

This is a good example of where it causes more harm than good. Is there still 
a really good reason for this over-paranoid compiler error given that checked 
exceptions aren't as popular as they used to be anyways?

Matthew.


Re: Remaining doclint issues in java.net

2013-07-31 Thread Stuart Marks

On 7/29/13 7:28 AM, Chris Hegarty wrote:


There are two remaining doclint warnings in the java.net package.

 >:javac -Xdoclint:all/protected src/share/classes/java/net/DatagramPacket.java
src/share/classes/java/net/DatagramPacket.java:142: warning: no @throws for
java.net.SocketException
 public DatagramPacket(byte buf[], int offset, int length,
^
src/share/classes/java/net/DatagramPacket.java:178: warning: no @throws for
java.net.SocketException
 public DatagramPacket(byte buf[], int length,
^

These are caused by no @throws SE declaration on the constructors.

As it happens 'throws SE' was incorrectly added to these constructors when
introduced in 1.4. The original API specified that SE was thrown when the given
SocketAddress was not supported. That was later changed to throw IAE, in 1.4.2.
These constructor now can never throw SE.

Removing 'throws SE' from the method declaration is a binary compatible change,
but not source compatible ( XXX  is never thrown in body of corresponding try
statement ). I don't think breaking source compatibility for this kind of
change is justified. If it is, then the solution is to simply remove 'throws 
SE'.

Back in the real world, I guess we need to come up with some wording for the
'@throws SE' declaration. Something vague like "If an I/O Exception occurs", or
can we put something stronger like "will never be thrown" ??


I'd like to make a case for removing 'throws SE' even though it's a source 
incompatible change.


It's not that source incompatibilities are strictly prohibited. They are 
allowed, and it depends on how often they occur and how difficult they are to 
fix. I seem to recall there was a similar issue when "more precise rethrow" was 
added in Java 7; this was indeed a source incompatibility but a survey was done 
and it was quite rare.


How often are these DatagramPacket constructors used? I'd have to say, probably 
more often than the fairly obscure cases that the "more precise rethrow" 
feature caused issues with.


On the other hand, fixing up code that constructs a DatagramPacket ought to be 
pretty simple: removing the catch of SocketException. This is already known to 
be dead code, so it can simply be removed.


The alternative is to add "@throws SocketException never" to the javadoc, just 
to get rid of the doclint warning, but this has the consequence of requiring 
people to keep dead code around indefinitely, and furthermore it requires them 
to add new dead code every time they create a DatagramPacket.


I'm not claiming that removing 'throws SE' is obviously the right answer, but 
I'd like to see it considered seriously.


s'marks


Remaining doclint issues in java.net

2013-07-29 Thread Chris Hegarty


There are two remaining doclint warnings in the java.net package.

>:javac -Xdoclint:all/protected 
src/share/classes/java/net/DatagramPacket.java
src/share/classes/java/net/DatagramPacket.java:142: warning: no @throws 
for java.net.SocketException

public DatagramPacket(byte buf[], int offset, int length,
   ^
src/share/classes/java/net/DatagramPacket.java:178: warning: no @throws 
for java.net.SocketException

public DatagramPacket(byte buf[], int length,
   ^

These are caused by no @throws SE declaration on the constructors.

As it happens 'throws SE' was incorrectly added to these constructors 
when introduced in 1.4. The original API specified that SE was thrown 
when the given SocketAddress was not supported. That was later changed 
to throw IAE, in 1.4.2. These constructor now can never throw SE.


Removing 'throws SE' from the method declaration is a binary compatible 
change, but not source compatible ( XXX  is never thrown in body of 
corresponding try statement ). I don't think breaking source 
compatibility for this kind of change is justified. If it is, then the 
solution is to simply remove 'throws SE'.


Back in the real world, I guess we need to come up with some wording for 
the '@throws SE' declaration. Something vague like "If an I/O Exception 
occurs", or can we put something stronger like "will never be thrown" ??


Thanks,
-Chris.