RFR: JDK-8221851: Use of THIS_FILE in hotspot invalidates precompiled header on Linux/GCC

[David Holmes]

Hi Erik,

On 9/04/2019 11:55 pm, Erik Joelsson wrote:
> Hello,
> 
> Here is a new webrev with an even simpler change:
> 
> http://cr.openjdk.java.net/~erikj/8221851/webrev.06/
> 
> I decided to just remove the usage of THIS_FILE from exceptions.hpp 
> since it did not work well. It currently does not work at all on Windows 
> or Macosx builds. On Linux it invalidates the precompiled header, which 
> is the main issue I'm trying to fix.
> 
> As for the truncation issue, with the patch above, truncation will again 
> happen on Solaris and Linux if compiled with GCC <8. However, as Thomas 
> pointed out, there is already a review out for fixing the truncation 
> problems for real.

As JDK-8220762 is soon to be fixed I'm okay with this temporarily 
regressing JDK-8204551. I've cc'd Yasumasa so he knows.

Thanks,
David
-----

> 
> The reason I tried to pursue finding a solution with shortening of the 
> __FILE__ path was that I thought it would be beneficial in more 
> locations, but it now seems like more trouble than it's worth and not 
> actually a feature anyone seem to want. I do not want to spend more time 
> one this, I just want the precompiled header to start working again. 
> Yes, this regresses the truncation issue somewhat, but I do not think a 
> broken solution should be left in there.
> 
> /Erik
> 
> On 2019-04-08 15:27, David Holmes wrote:
>> Hi Erik,
>>
>> On 9/04/2019 8:08 am, Erik Joelsson wrote:
>>> New webrev with "_simple_basename": 
>>> http://cr.openjdk.java.net/~erikj/8221851/webrev.05/
>>
>> Given the usage is typically of the form:
>>
>>  Exceptions::_throw(THREAD_AND_LOCATION, e);
>>
>> which will expand to:
>>
>>  Exceptions::_throw(THREAD, _simple_basename(__FILE__), __LINE__, e);
>>
>> what does the compiler actually generate for this at the call sites? 
>> I'm struggling with the addition of an inline function to a .hpp which 
>> we generally frown upon and have been working to remove.
>>
>> What affect does this have on code size?
>>
>> Thanks,
>> David
>> -----
>>
>>> /Erik
>>>
>>> On 2019-04-08 12:20, Erik Joelsson wrote:
>>>> On 2019-04-08 11:40, Kim Barrett wrote:
>>>>>> On Apr 8, 2019, at 10:28 AM, Erik Joelsson 
>>>>>> <erik.joelsson at oracle.com> wrote:
>>>>>>
>>>>>> Hello,
>>>>>>
>>>>>> On 2019-04-05 15:46, Kim Barrett wrote:
>>>>>>> Assuming all that, consider instead putting this_file_helper in
>>>>>>> exceptions.hpp (perhaps with a better name?), don't bother with
>>>>>>> THIS_FILE, and define THREAD_AND_LOCATION as
>>>>>>>
>>>>>>> #define THREAD_AND_LOCATION THREAD, this_file_helper(__FILE__), 
>>>>>>> __LINE__
>>>>>>>
>>>>>> Moved to exceptions.hpp, renamed to "basename", and removed the 
>>>>>> THIS_FILE macro.
>>>>> “basename” might not count as a “better name”, as it conflicts with 
>>>>> a POSIX function,
>>>>> even though we don’t presently seem to be using that anywhere that 
>>>>> I could find.
>>>>>
>>>>>
>>>> How about "simple_basename" then? Or just prefix with an underscore? 
>>>> The idea is to keep it internal to the headerfile, but I'm not 
>>>> familiar with conventions in Hotspot to know how you usually 
>>>> prefix/namespace things as private.
>>>>
>>>> /Erik
>>>>