RFR: 8080775: Better argument formatting for assert() and friends

Kim Barrett kim.barrett at oracle.com
Mon Sep 28 14:48:04 UTC 2015


This is the last of my comments on:
http://cr.openjdk.java.net/~david/JDK-8080775/webrev.00/

A few more minor formatting nits, and a couple of things that should
probably be left for later.

------------------------------------------------------------------------------  
src/share/vm/oops/klassVtable.cpp  
1464     fatal("klass %s: klass object too short (vtable extends beyond "
1465           "end)", _klass->internal_name());

Remove line break in format string.

------------------------------------------------------------------------------  
src/share/vm/oops/method.cpp
 249   { ResourceMark rm;
 250   assert(is_native() && bcp == code_base() || contains(bcp) || is_error_reported(),
 251          "bcp doesn't belong to this method: bcp: " INTPTR_FORMAT ", method: %s", bcp, name_and_sig_as_C_string());
 252   }

[Pre-existing]  
The assert isn't properly indented within the nested block. Note that
I think the ResourceMark is required, due to
name_and_sig_as_C_String().  [Recall that failed assert can still
return, though I think only in non-PRODUCT mode.]

------------------------------------------------------------------------------  
src/share/vm/oops/method.cpp 
 282   assert((is_native() && bci == 0)  || (!is_native() && 0 <= bci && bci < code_size()), "illegal bci: %d", bci);

[Pre-existing]   
Extra space before "||".

------------------------------------------------------------------------------  
src/share/vm/oops/method.cpp 
 575   if (class_access_flags.is_interface()) {
 576       assert(is_nonv == is_static(), "is_nonv=%s", name_and_sig_as_C_string());
 577   }

[Pre-existing]    
Indentation of assert statement should be 2, not 4.

------------------------------------------------------------------------------  
src/share/vm/oops/oop.cpp
 126   guarantee(obj->is_oop_or_null(), "invalid oop: " INTPTR_FORMAT, p2i((oopDesc*) obj));

[Pre-existing]    
I don't think the cast is needed here.

[Maybe this sort of thing should be left to separate cleanup of p2i
usage.  See below.]

------------------------------------------------------------------------------   
src/share/vm/oops/oop.inline.hpp
 192   assert(check_obj_alignment(result), "address not aligned: " INTPTR_FORMAT, p2i((void*) result));

[Pre-existing]    
I don't think the cast is needed here.

[Maybe this sort of thing should be left to a separate cleanup of p2i
usage.  With two in a row now, I'm inclined to suggest that.]

------------------------------------------------------------------------------  
src/share/vm/opto/castnode.cpp
 132               fatal("unexpected comparison %s", ss.as_string());

[Pre-existing. Any change related to this should be separate from the
assert formatting arguments change.]

[Note that this change removed err_msg_res.]

There's no obvious associated ResourceMark.  But maybe there should
be, since ss.as_string() performs resource allocation.

Recall that fatal() and other related macros can return under some
circumstances. (Though I think only non-PRODUCT.)

------------------------------------------------------------------------------  
src/share/vm/opto/graphKit.hpp
 356     assert(value->bottom_type()->basic_type() == T_INT,
 357         "wrong type: %s", type2name(value->bottom_type()->basic_type()));
...
 361     assert(value->bottom_type()->basic_type() == T_LONG,
 362         "wrong type: %s", type2name(value->bottom_type()->basic_type()));

Indentation of message.

------------------------------------------------------------------------------ 



More information about the hotspot-dev mailing list