Note that the two existing catch blocks
243 } catch (Exception t) {
244 t.printStackTrace(System.err);
245 failed.add(s);
246 } catch (NoClassDefFoundError e) {
247 e.printStackTrace(System.err);
248 failed.add(s);
249 }
could be combined into one using multi-catch:
243 } catch (Exception | NoClassDefFoundError t) {
244 t.printStackTrace(System.err);
245 failed.add(s);
246 }
Cheers,
-Joe
On 5/17/2016 6:26 AM, Sundararajan Athijegannathan wrote:
Please review the updated webrev:
http://cr.openjdk.java.net/~sundar/8157146/webrev.01/
Thanks,
-Sundar
On 5/17/2016 6:42 PM, Alan Bateman wrote:
On 17/05/2016 14:08, Aleksey Shipilev wrote:
On 05/17/2016 03:54 PM, Sundararajan Athijegannathan wrote:
Please review http://cr.openjdk.java.net/~sundar/8157146/webrev.00/ for
https://bugs.openjdk.java.net/browse/JDK-8157146
Shouldn't it follow the same pattern other catch blocks? Surely you want
to run all test cases, and not abort on the first VerifyError?
239 } catch (VerifyError ve) {
240 System.err.println("VerifyError for " + clsName);
241 throw ve;
Should be:
239 } catch (VerifyError ve) {
240 ve.printStackTrace(System.err);
241 failed.add(s);
242 } catch (Exception t) {
243 t.printStackTrace(System.err);
244 failed.add(s);
245 } catch (NoClassDefFoundError e) {
246 e.printStackTrace(System.err);
247 failed.add(s);
Also I think this should be before the attempt to load the class, to
capture even the failing attempt:
237 System.out.println("Loading " + clsName);
Ditto for VerifyJimage.java.
I agree, best to keep the test consistent. That said, something is
very broken if we have .class files in the jimage that are failing
verification so I am curious what this is.
-Alan
