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

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