RFR(XS): 8214534: Setting of THIS_FILE in the build is broken

[Volker Simonis]

Hi,

can I please have a review for the following trivial fix:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8214534/
https://bugs.openjdk.java.net/browse/JDK-8214534

DISCLAIMER: "XS" refers to the size of the fix, not the size of the
explanation :)

Currently the compilation of native files defines "THIS_FILE" to hold
the name of the current compilation unit. However, setting "THIS_FILE"
in NativeCompilation.gmk is broken and results in "THIS_FILE" always
being the empty string. I first thought that this is just a simple
quoting issue, but after I couldn't get it working, I found the
following explanation in the GNU Make manual [1]:

"A common mistake is attempting to use $@ within the prerequisites
list; this will not work. However, there is a special feature of GNU
make, secondary expansion (see Secondary Expansion), which will allow
automatic variable values to be used in prerequisite lists."

I'm not a Make expert, but I think this quote doesn't apply to "$@"
only, but to all automatic variables. The proposed solution (i.e.
"Secondary Expansion" [2]) seems overly complex for this problem. I
think the solution in the patch is much simpler and works "just as
well" :)

The other question is of course why do we need "THIS_FILE" at all? It
is used for various native logs in the class library (not in HotSpot)
which use the value of "THIS_FILE" to decorate their output with the
current file name. On the other hand, we already have the standard,
predefined "__FILE__" macro which could be used instead (and indeed,
if "THIS_FILE" isn't defined, the various logging routines fall back
to using "__FILE__").

The only explanation I could come up for having "THIS_FILE" until now
is that "__FILE__" may contain the full path name of the compilation
unit while we only want the simple file name (without path) in the
log. However, in order to solve this "path" problem, we can use
simpler solutions.

Some call sites (e.g.
"src/jdk.jdwp.agent/share/native/libjdwp/log_messages.h") already use
helper functions like "file_basename()" to cut off a potential path
component from "THIS_FILE". Other call sites (e.g.
"src/java.instrument/share/native/libinstrument/JPLISAssert.h" or
"src/jdk.jdwp.agent/share/native/libjdwp/error_messages.h") currently
simply use the value of "THIS_FILE" directly. But they could be easily
fixed by either using "file_basename()" there as well or even simpler,
wrapping "__FILE__" into another macro which calls "strrchr()" to do
the same work.

So as a follow up to this change, I'd like to propose another change
which completely removes "THIS_FILE" and changes all users of
"THIS_FILE" to use "__FILE__" instead. This would also shorten our
compile command lines (which doesn't happen too often :) What do you
think?

Thank you and best regards,
Volker

[1] https://www.gnu.org/software/make/manual/html_node/Automatic-Variables.html
[2] https://www.gnu.org/software/make/manual/html_node/Secondary-Expansion.html#Secondary-Expansion