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