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

[Bengt Rutisson]

Thumbs up! Ship it!
Bengt

On 2012-06-01 09:54, Mikael Gerdin wrote:
> 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?
>>>>>>
>>>>
>>>
>>
>