RFR(xs): 8224487: outputStream should not be copyable

Kim Barrett kim.barrett at oracle.com
Tue May 21 18:23:15 UTC 2019


> On May 21, 2019, at 12:20 PM, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
> src/hotspot/share/utilities/exceptions.hpp
>  189   static void log_exception(Handle exception, stringStream tempst);
> =>
>  189   static void log_exception(Handle exception, const char* message);
> 
> Why not change the stringStream parameter to be passed by const-ref
> rather than by value?  That would avoid changing all the callers.
> 
> 
> Because handing the whole stream object down to log_exception was overkill anyway. All it wants is to print a text message, so I just give it that and live with a slightly larger patch diff. 
> 
> Also, I am not a big fan of references but that is just me. I usually prefer to see at the call site if I hand in value or reference without looking up the prototype.

[Oops, I missed the bug introduced into log_exception by webrev.00;  I
was focused on the question of the parameter change.]

I disagree, for a number of reasons, but mostly because passing by
const-ref is completely normal for C++.  One doesn't need to look at
the prototype to determine value vs const-ref passing, because from
the caller's POV it doesn't matter.

> src/hotspot/share/utilities/events.hpp
> Removed from class FormatStringLogMessage
>  140  public:
>  141   // Wrap this buffer in a stringStream.
>  142   stringStream stream() {
>  143     return stringStream(this->_buf, this->size());
>  144   }
> 
> FormatStringLogMessage seems kind of pointless now.  Maybe it should
> be eliminated?  That could be a separate cleanup.
> 
> 
> Yes. As I wrote, I have a larger patch under review which changes all that code anyway.
> 
> If I just remove FormatStringMessage I must publish the ctor of the parent class, FormatMessage, and that was just that one step too far for this patch.

I suspected as much; thanks for confirming.



More information about the hotspot-dev mailing list