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

[Erik Joelsson]

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.

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
>>>