JDK 8 code review request for 6380161 (reflect) Exception from newInstance() not chained to cause.

Sebastian Sickelmann sebastian.sickelmann at gmx.de
Mon Aug 8 14:24:03 UTC 2011


Looks good to me.

But i have one question:

     Why there are two ways to plumb the causes of an exception?
     In many exceptions-classes you can use a constuctor-level argument 
to specify the cause, and in some classes you must use the initCause method.

     Is it: "When you often need a cause at creation-time than we got a 
specializes constructor." ?

     I found 500+ lines jdk/src/share/classes/**/*.java where i found 
line like

            XYZException iae = new XYZException();
             iae.initCause(e);
             throw iae;

     I am searching a minor project for my second Contribution to 
OpenJDK to learn more about the developement / review process.
     Is it worthwhile to look at this issue and maybe refactor some of 
the often used Exceptions to accept a cause at construction-time?

- Sebastian

Am 08.08.2011 07:24, schrieb Joe Darcy:
> Hello.
>
> Please review this small fix, developed after I went through my open 
> bug list; patch below, webrev at:
>
>    6380161 (reflect) Exception from newInstance() not chained to cause.
>    http://cr.openjdk.java.net/~darcy/6380161.0/
>
> Several call to initCause are added to plumb up caused-by exception 
> chains to exception types without constructors taking a cause.  In 
> addition, multi-catch is added in on location.
>
> I looked for similar issues in Constructor, Method, and Executable, 
> but there weren't any.
>
> Thanks,
>
> -Joe
>
> --- old/src/share/classes/java/lang/Class.java    2011-08-07 
> 22:21:58.000000000 -0700
> +++ new/src/share/classes/java/lang/Class.java    2011-08-07 
> 22:21:58.000000000 -0700
> @@ -349,7 +349,8 @@
>                         });
>                 cachedConstructor = c;
>             } catch (NoSuchMethodException e) {
> -                throw new InstantiationException(getName());
> +                throw (InstantiationException)
> +                    new InstantiationException(getName()).initCause(e);
>             }
>         }
>         Constructor<T> tmpConstructor = cachedConstructor;
> @@ -973,7 +974,8 @@
>                 descriptor      = (String)   enclosingInfo[2];
>                 assert((name != null && descriptor != null) || name == 
> descriptor);
>             } catch (ClassCastException cce) {
> -                throw new InternalError("Invalid type in enclosing 
> method information");
> +                throw (InternalError)
> +                    new InternalError("Invalid type in enclosing 
> method information").initCause(cce);
>             }
>         }
>
> @@ -1239,7 +1241,8 @@
>         try {
>             return 
> getName().substring(enclosingClass.getName().length());
>         } catch (IndexOutOfBoundsException ex) {
> -            throw new InternalError("Malformed class name");
> +            throw (InternalError)
> +                new InternalError("Malformed class name").initCause(ex);
>         }
>     }
>
> @@ -2954,9 +2957,8 @@
>             }
>             // These can happen when users concoct enum-like classes
>             // that don't comply with the enum spec.
> -            catch (InvocationTargetException ex) { return null; }
> -            catch (NoSuchMethodException ex) { return null; }
> -            catch (IllegalAccessException ex) { return null; }
> +            catch (InvocationTargetException | NoSuchMethodException |
> +                   IllegalAccessException ex) { return null; }
>         }
>         return enumConstants;
>     }
>




More information about the core-libs-dev mailing list