RFR(S) 7172226: HotSpot fails to build with GCC 4.7 because of stricter c++ argument dependent lookup

[Mikael Gerdin]

Bengt,

On 2012-06-01 08:18, Bengt Rutisson wrote:
>
> Mikael,
>
> I like this! Looks much better.
>
> One question. Line 248 in binaryTreeDictionary.cpp looks like this:
>
> 248 assert(!FreeList<Chunk>::verify_chunk_in_free_list(chunk), "Double
> entry");

Good catch! I missed that one.

>
> Would it be possible to fix this with a "using" fix as well?

Yes, fixed in
http://cr.openjdk.java.net/~mgerdin/7172226/webrev.2/

I've also built the changes (webrev.1) in JPRT on all platforms except 
Mac. The gcc we use on Mac is fairly recent, so I don't foresee any with 
that.

/mg

>
> Bengt
>
>
> On 2012-05-31 15:15, Mikael Gerdin wrote:
>> Hi all,
>> New version of the webrev, I changed the wrapper functions existing in
>> binaryTreeDictionary.hpp to use "using" as well and changed the comment.
>> I also removed some calls that already used fully qualified function
>> names to call to the parent class to also use "using".
>>
>> http://cr.openjdk.java.net/~mgerdin/7172226/webrev.1/
>>
>> /Mikael
>>
>> On 2012-05-30 20:39, Igor Veresov wrote:
>>> I think "using" in nicer, since you have to write it only once.
>>>
>>> igor
>>>
>>> On May 30, 2012, at 8:28 AM, Jon Masamitsu wrote:
>>>
>>>> Mikael,
>>>>
>>>> Thanks for doing these changes. It's fine as it is but did you consider
>>>> the "using" directive? I'm asking because I'm trying to decide in
>>>> a similar situation if I should use "using". Asking for advice more
>>>> than making a suggestion.
>>>>
>>>> Jon
>>>>
>>>> On 5/29/2012 8:07 AM, Mikael Gerdin wrote:
>>>>> (changed the subject to clarify that there's a code review hidden
>>>>> in here)
>>>>>
>>>>> As stated earlier in this thread GCC 4.7 has become more strict
>>>>> when doing name lookup in template classes.
>>>>> I have a patch that will fix the build
>>>>> http://cr.openjdk.java.net/~mgerdin/7172226/webrev.0/
>>>>>
>>>>> but I'm not sure about the style, the code already contains some
>>>>> wrapper functions which look like they tried to solve the same
>>>>> problem.
>>>>>
>>>>> Does anyone have any suggestions on how to handle this more cleanly?
>>>>>
>>>>> Thanks
>>>>> /Mikael
>>>>>
>>>>> On 2012-05-29 15:11, Andrew Hughes wrote:
>>>>>>
>>>>>>
>>>>>> ----- Original Message -----
>>>>>>> Andrew,
>>>>>>>
>>>>>>> I filed 7172226
>>>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7172226 (not
>>>>>>> available yet)
>>>>>>> See below for the patch.
>>>>>>>
>>>>>>> On 2012-05-28 15:34, Andrew Hughes wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> ----- Original Message -----
>>>>>>>>> Hi Andrew,
>>>>>>>>>
>>>>>>>>> On 2012-05-28 14:57, Andrew Hughes wrote:
>>>>> (...)
>>>>>>>>>
>>>>>>>>> I've seen this when I tried building with GCC 4.7 as well.
>>>>>>>>> This problem comes from 9f059abe8cf2, "Generalize the CMS free
>>>>>>>>> list
>>>>>>>>> code" which introduced some templates.
>>>>>>>>> It appears that GCC 4.7 is more strict when doing argument
>>>>>>>>> dependent
>>>>>>>>> lookup than 4.6.
>>>>>>>>> The fix is to do just what gcc suggests and use this->* or use the
>>>>>>>>> qualified name of the function, for example
>>>>>>>>> TreeList<Chunk>::link_tail
>>>>>>>>> in the first failure. As you can see there are already some places
>>>>>>>>> in
>>>>>>>>> that code which use the fully qualified name.
>>>>>>>>>
>>>>>>>>> I just discovered that I had a patch that fixes most of these
>>>>>>>>> issues
>>>>>>>>> in
>>>>>>>>> binaryTreeDictionary so I'll go ahead and file a CR for this.
>>>>>>>>>
>>>>>>>>> Regards
>>>>>>>>> /mg
>>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks Mikael. If you could post the patch, it would be much
>>>>>>>> appreciated.
>>>>>>>> I can't build OpenJDK8 at present.
>>>>>>>
>>>>>>> I've uploaded my fix (only build tested) to
>>>>>>> http://cr.openjdk.java.net/~mgerdin/7172226/webrev.0/
>>>>>>>
>>>>>>> /mg
>>>>>>>
>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Mikael Gerdin
>>>>>>>>> Java SE VM SQE Stockholm
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Mikael Gerdin
>>>>>>> Java SE VM SQE Stockholm
>>>>>>>
>>>>>>
>>>>>> Thanks. This patch worked and I've committed it to the IcedTea
>>>>>> OpenJDK8 tree:
>>>>>>
>>>>>> http://hg.openjdk.java.net/icedtea/jdk8/hotspot/rev/f1947ffdccd9
>>>>>>
>>>>>> Can someone with JPRT access please push this through?
>>>>>
>>>
>>
>

-- 
Mikael Gerdin
Java SE VM SQE Stockholm