This all sounds good to me. I just wanted to ask one question that's
nagged me from time to time. Maybe somebody knows the answer.
Are we actually allowed to use files with the same name in different
directories? Based on how stuff gets listed in moz.build, it seems like
it wouldn't work. Maybe there's some special way to qualify the
directory when needed though? I also wonder how one would specify that
file in gdb when setting breakpoints. Can you specify a directory there?
Finally, are we allowed to use filenames that overlap with filenames
elsewhere in gecko? It's hard for me to believe that we haven't already
done that. However, a quick search shows that we have the only copy of
Stack.cpp, so maybe it's really so?
Anyway, stupid question, but it's always made me wonder.
Bill
On 06/14/2013 02:55 PM, Nicholas Nethercote wrote:
Hi,
The standardization of our #ifndef wrappers came up in bug 881579.
Here are some examples:
- jsapi.h: jsapi_h___
- gc/Barrier.h: jsgc_barrier_h___
- vm/Stack.h: Stack_h__
- ion/Ion.h: jsion_ion_h__
Observations:
- Omitting the directory is dangerous if we ever have two files with
different directories but the same name.
- Decapitalizing filenames seems odd.
- Adding the |js| prefix for files that don't have a |js| prefix seems odd.
- The first two end with three underscores, and the last two end with
two underscores. (We have 149 headers that use 2, and 99 that use 3.)
I propose that we do the following: take the full path within js/src,
replace '/' and '.' with '_', and add two underscores to the end. For
example:
- jsapi.h: jsapi_h__
- gc/Barrier.h: gc_Barrier_h__
- vm/Stack.h: vm_Stack_h__
- ion/Ion.h: ion_Ion_h__
Thoughts?
----
Relatedly, we sometimes omit header directories. E.g. instead of
#include "ion/Ion.h", we do #include "Ion.h". This only works for
headers that are in the same directory, e.g. in ion/AsmJSLink.h we
have
#include "AsmJSModule.h" // |ion/| omitted because AsmJSLink.h is in ion/
#include "frontend/BytecodeCompiler.h" // |frontend/| present
(Also, we add -I($srcdir)/yarr and -I($srcdir)/assembler for some
reason. which allows those prefixes to be omitted.)
I propose that we require the directories, and get rid of the yarr and
assembler -I options. This increases consistency of the #include
lines, and makes putting them in sorted order easier. Thoughts?
----
I figure Luke has final say on these matters. I'll update the style
guide with whatever we decide. Thanks.
Nick
ps: if you're worried about enforcement of the style, I'm working
towards a #include-checking script in
https://bugzilla.mozilla.org/show_bug.cgi?id=880088.
_______________________________________________
dev-tech-js-engine-internals mailing list
dev-tech-js-engine-internals@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-tech-js-engine-internals
_______________________________________________
dev-tech-js-engine-internals mailing list
dev-tech-js-engine-internals@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-tech-js-engine-internals
