Question on ADLC's RegClass destructor

[Krystal Mok]

Hi Zoltan,

Thanks for your reply. When I was looking at the call chain that leads to
RegClass' constructor, get_ident() is what's passed into RegClass as
classid.

The comment on get_ident_common() says:
http://hg.openjdk.java.net/jdk9/jdk9/hotspot/file/ac291bc3ece2/src/share/vm/adlc/adlparse.cpp#l4560
//------------------------------get_ident_common-------------------------------
// Looks for an identifier in the buffer, and turns it into a null
terminated
// string(still inside the file buffer).  Returns a pointer to the string or
// NULL if some other token is found instead.
char *ADLParser::get_ident_common(bool do_preproc) {

So normally, the string returned should be still inside the file buffer (if
no preprocessing is needed...), and shouldn't need to be free'd afterwards;
but if preprocessing is needed, then yes, there's dynamically allocated
memory for the string returned.
get_ident() is get_ident_common(true), so it's possible that preprocessing
is needed; but from the RegClass constructor, it'd be hard to tell whether
the string passed in is from the file buffer or from a dynamically
allocated piece of memory.

It (the original code before your changes) was an odd piece of code to
begin with...

- Kris


On Tue, May 26, 2015 at 12:06 AM, Zoltán Majó <zoltan.majo at oracle.com>
wrote:

> Hi Kris,
>
>
> On 05/23/2015 12:22 AM, Krystal Mok wrote:
>
>> Hi compiler team,
>>
>> I came across this piece of code in ADLC, and I'm not sure if it's right
>> or not:
>>
>>
>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/file/ac291bc3ece2/src/share/vm/adlc/formsopt.cpp#l236
>>
>> RegClass::~RegClass() {
>>   delete _classid;
>> }
>>
>> Why do we need to delete the _classid field here? If this is needed, then
>> a bunch of other classes in formsopt.hpp would need the same treatment.
>>
>
> If I remember correctly, I added the destructor because the caller of the
> RegClass constructor allocates memory for _classid, but does not free it.
> But I'd have to check to be sure.
>
> Best regards,
>
>
> Zoltan
>
>
>
>> (I know this virtual destructor was added mainly for the two new
>> subclasses, but I'm just wondering about this one...)
>>
>> Thanks,
>> Kris
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20150526/1e83b7e3/attachment.html>