RFR: 8333211: NMT Reports: replace manual indentation handling with auto indent

David Holmes dholmes at openjdk.org
Tue Jun 4 07:49:08 UTC 2024


On Tue, 4 Jun 2024 07:31:47 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> src/hotspot/share/utilities/ostream.hpp line 83:
>> 
>>> 81:    void dec(int n) { _indentation -= n; };
>>> 82:    int  indentation() const    { return _indentation; }
>>> 83:    void set_indentation(int i) { _indentation = i;    }
>> 
>> Why have you deleted these?
>
> They were not needed, nobody uses them. Indentation level is only modified by outputStream itself and it accesses _indentation directly.

`indention()` is being used in this PR: https://github.com/openjdk/jdk/pull/19482

Though I am querying the correctness/appropriateness of the logic there.

>> src/hotspot/share/utilities/ostream.hpp line 106:
>> 
>>> 104:    void print_raw(const char* str, size_t len);
>>> 105:    void print_raw_cr(const char* str)             { print_raw(str); cr(); }
>>> 106:    void print_raw_cr(const char* str, size_t len) { print_raw(str, len); cr(); }
>> 
>> Can these be simplified if we have a default value for len? i.e.
>> 
>> void print_raw(const char* str, size_t len = strlen(str));
>
> Interesting question. I try to avoid non-const default arguments since I never can recall their restrictions. 
> 
> Looked it up, I think it is not possible to refer to other function arguments in default values:
> 
> 
>   A default argument is evaluated each time the function is called with no argument for the corresponding  
>   parameter. Function parameters are not allowed in default arguments except if they are [not evaluated
>   (https://en.cppreference.com/w/cpp/language/expressions#Potentially-evaluated_expressions). Note that
>   parameters that appear earlier in the parameter list are in [scope
>   (https://en.cppreference.com/w/cpp/language/scope):

Okay - thanks

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19461#discussion_r1625519566
PR Review Comment: https://git.openjdk.org/jdk/pull/19461#discussion_r1625520211


More information about the hotspot-runtime-dev mailing list