Bug 74990

Summary: Rename WTF_INLINE, JS_INLINE to HIDDEN_INLINE
Product: WebKit Reporter: Hajime Morrita <morrita>
Component: Web Template FrameworkAssignee: Hajime Morrita <morrita>
Status: RESOLVED FIXED    
Severity: Normal CC: kevino, mrowe, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 72855    
Attachments:
Description Flags
Patch none

Hajime Morrita
Reported 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.
Attachments
Patch (1.60 KB, patch)
2011-12-20 21:22 PST, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Hajime Morrita
Comment 1 2011-12-20 21:22:07 PST
Created attachment 120136 [details] Patch
Kevin Ollivier
Comment 2 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.
Darin Adler
Comment 3 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.
Kevin Ollivier
Comment 4 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.
WebKit Review Bot
Comment 5 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>
WebKit Review Bot
Comment 6 2011-12-26 08:41:07 PST
All reviewed patches have been landed. Closing bug.
Hajime Morrita
Comment 7 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.
Note You need to log in before you can comment on or make changes to this bug.