Closed Bug 924642 Opened 6 years ago Closed 6 years ago

internal name linkage

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: sunfish, Assigned: sunfish)

Details

Attachments

(2 files)

No description provided.
The attached patch adds static keywords and anonymous namespaces to give more symbols internal name linkage.

See comment 8 of bug 910829 for some discussion of a similar patch. I'm aware that this patch isn't very important, and am ok dropping it if you don't want it.
Attachment #814582 - Flags: review?(n.nethercote)
Comment on attachment 814582 [details] [diff] [review]
internal-name-linkage.patch

Review of attachment 814582 [details] [diff] [review]:
-----------------------------------------------------------------

I'm very happy with all the |static| changes;  r=me for them.

As before, I'm not a fan of anonymous namespaces due to the extra lines of code and debugging difficulties, and I'd prefer to not have them.  Thanks.

(BTW, if you type something like "bug 910829 comment 8" then Bugzilla will linkify it to the exact comment.)

::: js/src/ctypes/CTypes.cpp
@@ +793,5 @@
>    JS_FN("libraryName", Library::Name, 1, CTYPESFN_FLAGS),
>    JS_FS_END
>  };
>  
> +JS_ALWAYS_INLINE static JSString*

Elsewhere you put |static| before |inline|...
Attachment #814582 - Flags: review?(n.nethercote) → review+
Ok. I checked in just the static ones, and put static before JS_ALWAYS_INLINE for consistency:

https://hg.mozilla.org/integration/mozilla-inbound/rev/37ca290c9256
Attached is the anonymous namespaces part of the patch, in case it's of interest to anyone.
https://hg.mozilla.org/mozilla-central/rev/37ca290c9256
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.