You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by se...@apache.org on 2013/10/31 16:13:47 UTC
svn commit: r1537525 -
/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java
Author: sebb
Date: Thu Oct 31 15:13:47 2013
New Revision: 1537525
URL: http://svn.apache.org/r1537525
Log:
Document that CCE is handled OK; there is no "unchecked" warning happening here
Modified:
commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java
Modified: commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java
URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java?rev=1537525&r1=1537524&r2=1537525&view=diff
==============================================================================
--- commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java (original)
+++ commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java Thu Oct 31 15:13:47 2013
@@ -1743,23 +1743,18 @@ public class BasicDataSource implements
try {
try {
if (driverClassLoader == null) {
- @SuppressWarnings("unchecked")
- Class<Driver> c =
- (Class<Driver>) Class.forName(driverClassName);
- driverFromCCL = c;
+ driverFromCCL = (Class<Driver>) Class.forName(driverClassName);
} else {
- @SuppressWarnings("unchecked")
- Class<Driver> c = (Class<Driver>) Class.forName(
+ driverFromCCL = (Class<Driver>) Class.forName(
driverClassName, true, driverClassLoader);
- driverFromCCL = c;
}
} catch (ClassNotFoundException cnfe) {
- @SuppressWarnings("unchecked")
- Class<Driver> c = (Class<Driver>) Thread.currentThread(
+ driverFromCCL = (Class<Driver>) Thread.currentThread(
).getContextClassLoader().loadClass(
driverClassName);
- driverFromCCL = c;
}
+ // N.B. the casts above may cause ClassCastException if classname is not correct
+ // This is caught below
} catch (Exception t) {
String message = "Cannot load JDBC driver class '" +
driverClassName + "'";
Re: svn commit: r1537525 - /commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java
Posted by sebb <se...@gmail.com>.
On 31 October 2013 15:51, Mark Thomas <ma...@apache.org> wrote:
> On 31/10/2013 15:36, sebb wrote:
>> On 31 October 2013 15:26, Mark Thomas <ma...@apache.org> wrote:
>>> On 31/10/2013 15:13, sebb@apache.org wrote:
>>>> Author: sebb
>>>> Date: Thu Oct 31 15:13:47 2013
>>>> New Revision: 1537525
>>>>
>>>> URL: http://svn.apache.org/r1537525
>>>> Log:
>>>> Document that CCE is handled OK; there is no "unchecked" warning happening here
>>>
>>> Why did you make this change? It undoes a recent commit that fixed a
>>> handful of type safety warnings.
>>
>> I don't see any warnings when building with Java 7 or Eclipse.
>
> I suspect you have "Ignore unavoidable generic type problems" enabled.
No, but I did have other generics warnings disabled by mistake.
I had to reinstall Eclipse completely when it would not start up, and
had overlooked updating the settings.
Sorry about that.
However, I did not get any warnings when building with Maven and Java 7 either.
Not sure if that can/should be fixed?
> The issue with this is that it is an Eclipse specific feature. Users of
> $OTHER_IDE can't enabled it and therefore can't work with a clean (i.e.
> warning free) code base.
>
> I do use Eclipse and could enable that option but having gone through a
> enable it, remove the warnings, have $OTHER_IDE users complain, disable
> it, add the warnings back cycle with Tomcat, I'd rather not repeat the
> exercise here.
>
> @SuppressWarnings("unchecked") // Unavoidable
>
> is probably the way to go here.
That's part of it, but in the case in point, a CCE can be generated.
This needs to be documented as well.
As it happens, I found a slighlty different way to do it which I have
now committed (and documented).
Hope that's OK.
> Mark
>
>>
>> AFAICT these are now just class casts that can fail with CCE.
>>
>> If necessary it can be reverted.
>> In which case need to document why the annotatiions are needed and why
>> they are OK to use (i.e. in this case the CCE is expected and
>> handled).
>>
>>> Mark
>>>
>>>
>>>> Modified:
>>>> commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java
>>>>
>>>> Modified: commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java
>>>> URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java?rev=1537525&r1=1537524&r2=1537525&view=diff
>>>> ==============================================================================
>>>> --- commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java (original)
>>>> +++ commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java Thu Oct 31 15:13:47 2013
>>>> @@ -1743,23 +1743,18 @@ public class BasicDataSource implements
>>>> try {
>>>> try {
>>>> if (driverClassLoader == null) {
>>>> - @SuppressWarnings("unchecked")
>>>> - Class<Driver> c =
>>>> - (Class<Driver>) Class.forName(driverClassName);
>>>> - driverFromCCL = c;
>>>> + driverFromCCL = (Class<Driver>) Class.forName(driverClassName);
>>>> } else {
>>>> - @SuppressWarnings("unchecked")
>>>> - Class<Driver> c = (Class<Driver>) Class.forName(
>>>> + driverFromCCL = (Class<Driver>) Class.forName(
>>>> driverClassName, true, driverClassLoader);
>>>> - driverFromCCL = c;
>>>> }
>>>> } catch (ClassNotFoundException cnfe) {
>>>> - @SuppressWarnings("unchecked")
>>>> - Class<Driver> c = (Class<Driver>) Thread.currentThread(
>>>> + driverFromCCL = (Class<Driver>) Thread.currentThread(
>>>> ).getContextClassLoader().loadClass(
>>>> driverClassName);
>>>> - driverFromCCL = c;
>>>> }
>>>> + // N.B. the casts above may cause ClassCastException if classname is not correct
>>>> + // This is caught below
>>>> } catch (Exception t) {
>>>> String message = "Cannot load JDBC driver class '" +
>>>> driverClassName + "'";
>>>>
>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1537525 - /commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java
Posted by Mark Thomas <ma...@apache.org>.
On 31/10/2013 15:36, sebb wrote:
> On 31 October 2013 15:26, Mark Thomas <ma...@apache.org> wrote:
>> On 31/10/2013 15:13, sebb@apache.org wrote:
>>> Author: sebb
>>> Date: Thu Oct 31 15:13:47 2013
>>> New Revision: 1537525
>>>
>>> URL: http://svn.apache.org/r1537525
>>> Log:
>>> Document that CCE is handled OK; there is no "unchecked" warning happening here
>>
>> Why did you make this change? It undoes a recent commit that fixed a
>> handful of type safety warnings.
>
> I don't see any warnings when building with Java 7 or Eclipse.
I suspect you have "Ignore unavoidable generic type problems" enabled.
The issue with this is that it is an Eclipse specific feature. Users of
$OTHER_IDE can't enabled it and therefore can't work with a clean (i.e.
warning free) code base.
I do use Eclipse and could enable that option but having gone through a
enable it, remove the warnings, have $OTHER_IDE users complain, disable
it, add the warnings back cycle with Tomcat, I'd rather not repeat the
exercise here.
@SuppressWarnings("unchecked") // Unavoidable
is probably the way to go here.
Mark
>
> AFAICT these are now just class casts that can fail with CCE.
>
> If necessary it can be reverted.
> In which case need to document why the annotatiions are needed and why
> they are OK to use (i.e. in this case the CCE is expected and
> handled).
>
>> Mark
>>
>>
>>> Modified:
>>> commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java
>>>
>>> Modified: commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java
>>> URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java?rev=1537525&r1=1537524&r2=1537525&view=diff
>>> ==============================================================================
>>> --- commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java (original)
>>> +++ commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java Thu Oct 31 15:13:47 2013
>>> @@ -1743,23 +1743,18 @@ public class BasicDataSource implements
>>> try {
>>> try {
>>> if (driverClassLoader == null) {
>>> - @SuppressWarnings("unchecked")
>>> - Class<Driver> c =
>>> - (Class<Driver>) Class.forName(driverClassName);
>>> - driverFromCCL = c;
>>> + driverFromCCL = (Class<Driver>) Class.forName(driverClassName);
>>> } else {
>>> - @SuppressWarnings("unchecked")
>>> - Class<Driver> c = (Class<Driver>) Class.forName(
>>> + driverFromCCL = (Class<Driver>) Class.forName(
>>> driverClassName, true, driverClassLoader);
>>> - driverFromCCL = c;
>>> }
>>> } catch (ClassNotFoundException cnfe) {
>>> - @SuppressWarnings("unchecked")
>>> - Class<Driver> c = (Class<Driver>) Thread.currentThread(
>>> + driverFromCCL = (Class<Driver>) Thread.currentThread(
>>> ).getContextClassLoader().loadClass(
>>> driverClassName);
>>> - driverFromCCL = c;
>>> }
>>> + // N.B. the casts above may cause ClassCastException if classname is not correct
>>> + // This is caught below
>>> } catch (Exception t) {
>>> String message = "Cannot load JDBC driver class '" +
>>> driverClassName + "'";
>>>
>>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1537525 - /commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java
Posted by sebb <se...@gmail.com>.
On 31 October 2013 15:26, Mark Thomas <ma...@apache.org> wrote:
> On 31/10/2013 15:13, sebb@apache.org wrote:
>> Author: sebb
>> Date: Thu Oct 31 15:13:47 2013
>> New Revision: 1537525
>>
>> URL: http://svn.apache.org/r1537525
>> Log:
>> Document that CCE is handled OK; there is no "unchecked" warning happening here
>
> Why did you make this change? It undoes a recent commit that fixed a
> handful of type safety warnings.
I don't see any warnings when building with Java 7 or Eclipse.
AFAICT these are now just class casts that can fail with CCE.
If necessary it can be reverted.
In which case need to document why the annotatiions are needed and why
they are OK to use (i.e. in this case the CCE is expected and
handled).
> Mark
>
>
>> Modified:
>> commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java
>>
>> Modified: commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java
>> URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java?rev=1537525&r1=1537524&r2=1537525&view=diff
>> ==============================================================================
>> --- commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java (original)
>> +++ commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java Thu Oct 31 15:13:47 2013
>> @@ -1743,23 +1743,18 @@ public class BasicDataSource implements
>> try {
>> try {
>> if (driverClassLoader == null) {
>> - @SuppressWarnings("unchecked")
>> - Class<Driver> c =
>> - (Class<Driver>) Class.forName(driverClassName);
>> - driverFromCCL = c;
>> + driverFromCCL = (Class<Driver>) Class.forName(driverClassName);
>> } else {
>> - @SuppressWarnings("unchecked")
>> - Class<Driver> c = (Class<Driver>) Class.forName(
>> + driverFromCCL = (Class<Driver>) Class.forName(
>> driverClassName, true, driverClassLoader);
>> - driverFromCCL = c;
>> }
>> } catch (ClassNotFoundException cnfe) {
>> - @SuppressWarnings("unchecked")
>> - Class<Driver> c = (Class<Driver>) Thread.currentThread(
>> + driverFromCCL = (Class<Driver>) Thread.currentThread(
>> ).getContextClassLoader().loadClass(
>> driverClassName);
>> - driverFromCCL = c;
>> }
>> + // N.B. the casts above may cause ClassCastException if classname is not correct
>> + // This is caught below
>> } catch (Exception t) {
>> String message = "Cannot load JDBC driver class '" +
>> driverClassName + "'";
>>
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1537525 - /commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java
Posted by Mark Thomas <ma...@apache.org>.
On 31/10/2013 15:13, sebb@apache.org wrote:
> Author: sebb
> Date: Thu Oct 31 15:13:47 2013
> New Revision: 1537525
>
> URL: http://svn.apache.org/r1537525
> Log:
> Document that CCE is handled OK; there is no "unchecked" warning happening here
Why did you make this change? It undoes a recent commit that fixed a
handful of type safety warnings.
Mark
> Modified:
> commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java
>
> Modified: commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java
> URL: http://svn.apache.org/viewvc/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java?rev=1537525&r1=1537524&r2=1537525&view=diff
> ==============================================================================
> --- commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java (original)
> +++ commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp2/BasicDataSource.java Thu Oct 31 15:13:47 2013
> @@ -1743,23 +1743,18 @@ public class BasicDataSource implements
> try {
> try {
> if (driverClassLoader == null) {
> - @SuppressWarnings("unchecked")
> - Class<Driver> c =
> - (Class<Driver>) Class.forName(driverClassName);
> - driverFromCCL = c;
> + driverFromCCL = (Class<Driver>) Class.forName(driverClassName);
> } else {
> - @SuppressWarnings("unchecked")
> - Class<Driver> c = (Class<Driver>) Class.forName(
> + driverFromCCL = (Class<Driver>) Class.forName(
> driverClassName, true, driverClassLoader);
> - driverFromCCL = c;
> }
> } catch (ClassNotFoundException cnfe) {
> - @SuppressWarnings("unchecked")
> - Class<Driver> c = (Class<Driver>) Thread.currentThread(
> + driverFromCCL = (Class<Driver>) Thread.currentThread(
> ).getContextClassLoader().loadClass(
> driverClassName);
> - driverFromCCL = c;
> }
> + // N.B. the casts above may cause ClassCastException if classname is not correct
> + // This is caught below
> } catch (Exception t) {
> String message = "Cannot load JDBC driver class '" +
> driverClassName + "'";
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org