RFR: JDK-8221851: Use of THIS_FILE in hotspot invalidates precompiled header on Linux/GCC
erik.joelsson at oracle.com
Tue Apr 9 13:55:15 UTC 2019
Here is a new webrev with an even simpler change:
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.
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":
> 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?
>> 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:
>>>>> 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__),
>>>>> 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.
More information about the build-dev