Bug 74990 - Rename WTF_INLINE, JS_INLINE to HIDDEN_INLINE
Summary: Rename WTF_INLINE, JS_INLINE to HIDDEN_INLINE
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hajime Morrita
URL:
Keywords:
Depends on:
Blocks: 72855
  Show dependency treegraph
 
Reported: 2011-12-20 21:15 PST by Hajime Morrita
Modified: 2011-12-26 17:23 PST (History)
3 users (show)

See Also:


Attachments
Patch (1.60 KB, patch)
2011-12-20 21:22 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hajime Morrita 2011-12-20 21:15:49 PST
As bdash@ mentioned at Bug 72855, JS_INLINE is annoying and confusing name.
It's for marking functions to be hidden. So it should be renamed "HIDDEN_INLINE".

 * This is more intention revealing name than WTF_INLINE/JS_INLINE and its appearance is aligned to ALWAYS_INLINE.
 * We'll use the name even outside WTF so prefixing as WTF could be misleading.
   Its definition doesn't change depending on the build target.
Comment 1 Hajime Morrita 2011-12-20 21:22:07 PST
Created attachment 120136 [details]
Patch
Comment 2 Kevin Ollivier 2011-12-21 11:58:42 PST
Bug filed about the EWS bot traceback problem:

https://bugs.webkit.org/show_bug.cgi?id=75024

FWIW, last I checked it while it was working, all the bots except Win and Mac had run, and all passed, so we just need to know if this passes Mac and Win.
Comment 3 Darin Adler 2011-12-26 07:22:55 PST
Comment on attachment 120136 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=120136&action=review

> Source/JavaScriptCore/wtf/ExportMacros.h:88
> +#define HIDDEN_INLINE WTF_EXPORT_HIDDEN inline

Most of our WTF macros use a namespacing prefix such as WTF_ or JS_ to avoid conflicting with macros from other libraries. There are some exceptions like ALWAYS_INLINE, but I am concerned about future conflicts.
Comment 4 Kevin Ollivier 2011-12-26 08:40:31 PST
(In reply to comment #3)
> (From update of attachment 120136 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=120136&action=review
> 
> > Source/JavaScriptCore/wtf/ExportMacros.h:88
> > +#define HIDDEN_INLINE WTF_EXPORT_HIDDEN inline
> 
> Most of our WTF macros use a namespacing prefix such as WTF_ or JS_ to avoid conflicting with macros from other libraries. There are some exceptions like ALWAYS_INLINE, but I am concerned about future conflicts.

I thought about this as well, but I can see the argument that using WTF_ as a prefix here is problematic. Consider that WTF_EXPORT_PRIVATE and co. have different definitions based on if the WTF library is being built or linked. So I think that WTF_HIDDEN_INLINE would be naturally seen as a WTF-specific macro, in line with the other macros, rather than a project-wide one.

I'm not sure how to tackle this, except maybe introducing a project-wide macro prefix? I do think, though, that whatever approach we take should be consistent with ALWAYS_INLINE and similar macros, so that there's a clear policy for future macros of a project-wide nature to use.
Comment 5 WebKit Review Bot 2011-12-26 08:41:03 PST
Comment on attachment 120136 [details]
Patch

Clearing flags on attachment: 120136

Committed r103688: <http://trac.webkit.org/changeset/103688>
Comment 6 WebKit Review Bot 2011-12-26 08:41:07 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Hajime Morrita 2011-12-26 17:23:26 PST
> Most of our WTF macros use a namespacing prefix such as WTF_ or JS_ to avoid conflicting with macros from other libraries. There are some exceptions like ALWAYS_INLINE, but I am concerned about future conflicts.
Although I understand this concern, I think HIDDEN_INLINE is verbose enough to avoid future conflict.