On Sep 6, 2012, at 1:57 PM, John Coomes wrote: > Kelly O'Hair (kelly.oh...@oracle.com) wrote: >> Yes, but I feel like I need to get some kind of approval from a higher level >> before I try and declare that >> use of THIS_FILE is a 'jdk convention', and that will trigger long debates >> :^( >> >> I also need to deal with hotspot, which is a bit trickier because of the >> macros being expanded in include >> files (inline method bodies inside *.hpp files). > > There is also the fact that the basenames are not unique. Maybe it > won't be an issue, but someone should really check.
They aren't necessarily. But even if they were relative paths, you can't guarantee they would be unique then. The __FILE__ value is basically not well defined, anyone depending on it is probably in trouble. > > <wear_kevlar_suit> > Have you looked at normalizing to the path relative to the repo root? > </wear_kevlar_suit> If I could come up with a simple formula that did not involve a repeated exec of sed or a shell for every compilation, I could change THIS_FILE to a 'relative to the repo root' path. Just haven't come up with a good solution yet. But that could be an improvement later, e.g. ROOT_DIR:=$(shell hg root) # Or set it in top Makefile somehow? -DTHIS_FILE='"$(subst $(ROOT_DIR)/,./,$(@D))$(@F)"' might work, but it's a bit more complicated than that. -kto > > -John > >> On Sep 5, 2012, at 11:59 PM, Fredrik Öhrström wrote: >> >>> Looks good. Perhaps we can even remove the "#ifndef THIS_FILE" test in the >>> source files? At some time in the future…. >>> >>> //Fredrik >>> >>> 6 sep 2012 kl. 06:08 skrev Kelly O'Hair: >>> >>>> >>>> Need a reviewer for this change. >>>> >>>> http://cr.openjdk.java.net/~ohair/openjdk8/jdk8-this-file/webrev/ >>>> >>>> It does change source, but it's effectively a build change. >>>> >>>> The goal here is to try and create more predictable binary files and >>>> remove the possibility that >>>> a full source path (via __FILE__) gets baked into the shared libraries or >>>> executables shipped. >>>> >>>> So two changes: >>>> * sort the .o files during links so they are always provided to the linker >>>> in the same order. >>>> * replace use of __FILE__ to the macro THIS_FILE with just the basename of >>>> the source file being compiled >>>> >>>> The __FILE__ issue is that it has an implementation defined string literal >>>> value, depending on the compiler >>>> and the filename supplied on the compile line. By changing to the new >>>> THIS_FILE macro, the object files >>>> will always have a consistent string literal in them, making it easier to >>>> compare binaries between builds. >>>> Something we have never been able to do in the past. Granted it's just the >>>> basename for the file, but should be enough. >>>> The tricky part is that __FILE__ only gets evaluated when it is used. In >>>> normal C code, it will appear in >>>> macros but it only will get evaluated in the source file being compiled. >>>> It is rare that macros using >>>> __FILE__ will get expanded in include files and I detected this not >>>> happening in the jdk source at all. >>>> >>>> -kto >>> >>
