RFR: 8255416: Investigate err_msg to detect unnecessary uses [v2]

[Anton Kozlov]

On Wed, 28 Oct 2020 18:22:37 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Anton Kozlov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix codestyle
>
> src/hotspot/share/gc/shenandoah/mode/shenandoahMode.hpp line 44:
> 
>> 42:     if ((name)) {                                                           \
>> 43:       const char* msg = "GC mode needs -XX:-" #name " to work correctly";   \
>> 44:       vm_exit_during_initialization("Error", msg);                          \
> 
> Same as above, can be inlined into one call. No need for the temporary variable.

Please see my comment in the thread above.

> src/hotspot/share/utilities/formatBuffer.hpp line 124:
> 
>> 122:   // If compilation fails because of ambiguity between this and real constructor, you
>> 123:   // could drop err_msg use at all.
>> 124:   inline FormatErrBuffer(const char* msg) { ShouldNotReachHere(); }
> 
> I do not think this is a good idea (apart from it being too complex for a not that serious issue).
> 
> The asserts fire, of course, only at runtime. But this function is used usually in some error context, as part of of error reporting. I do not think our tests cover all those paths.
> 
> Either somehow make this a compile time error or just leave it as it is. We also could, since this buffer object is used usually as input for vm_exit_during_initialization(), give that function a var-arg overload.

Actually, this code prevents a single string argument in compile time. It relies on two constructors to introduce ambiguity in overload resolution that makes C++ compiler complain and abort compilation. Comment above the dummy constructor should clarify that for anyone stepping on compile error.
  inline FormatErrBuffer(const char* format, ...) ATTRIBUTE_PRINTF(2, 3);
  inline FormatErrBuffer(const char* msg) { ShouldNotReachHere(); }
For sanity check, I've used this sample code https://godbolt.org/z/szs6rE

And the cases that were fixed have been detected by the compiler.

I don't think that this problem is a serious issue as well. But as for me, it is a minor code complexity increase to ensure that the minor problem of extra string copy will never appear again.

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

PR: https://git.openjdk.java.net/jdk/pull/905