Bug 46960 - Specify ALWAYS_INLINE at function declaration not function definition
Summary: Specify ALWAYS_INLINE at function declaration not function definition
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2010-09-30 22:48 PDT by Pratik Solanki
Modified: 2010-10-01 10:50 PDT (History)
4 users (show)

See Also:


Attachments
Patch (6.36 KB, patch)
2010-09-30 23:40 PDT, Pratik Solanki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pratik Solanki 2010-09-30 22:48:34 PDT
Take for example the following code in FastMalloc.cpp


template <bool crashOnFailure>
void* malloc(size_t);

void* fastMalloc(size_t size)
{
    return malloc<true>(size);
}


template <bool crashOnFailure>
ALWAYS_INLINE
void* malloc(size_t size) {
...
}

We need to have the ALWAYS_INLINE be on the declaration, otherwise the compiler may not inline the call to malloc<true>(size) in fastMalloc(). There are other instances of this in the code as well. Patch coming up.
Comment 1 Pratik Solanki 2010-09-30 22:50:03 PDT
<rdar://problem/8498709>
Comment 2 Pratik Solanki 2010-09-30 23:40:24 PDT
Created attachment 69426 [details]
Patch
Comment 3 Pratik Solanki 2010-09-30 23:42:10 PDT
From other files, I see that the practice seems to be to add ALWAYS_INLINE to both definition and declaration so thats what I've done in my patch.
Comment 4 WebKit Review Bot 2010-09-30 23:42:26 PDT
Attachment 69426 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
JavaScriptCore/wtf/FastMalloc.cpp:2188:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
JavaScriptCore/wtf/FastMalloc.cpp:2191:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
JavaScriptCore/wtf/FastMalloc.cpp:2292:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
JavaScriptCore/wtf/FastMalloc.cpp:2297:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
JavaScriptCore/wtf/FastMalloc.cpp:2310:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 5 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Geoffrey Garen 2010-10-01 10:18:44 PDT
Comment on attachment 69426 [details]
Patch

Consider these docs:

http://gcc.gnu.org/onlinedocs/gcc/Inline.html
http://gcc.gnu.org/onlinedocs/gcc-4.0.4/gcc/Attribute-Syntax.html#Attribute-Syntax
http://gcc.gnu.org/onlinedocs/gcc-4.0.4/gcc/Function-Attributes.html

I'm pretty sure that ALWAYS_INLINE is only needed at the declaration site, not the definition site.

Even so, this patch is good, so r=me.
Comment 6 WebKit Commit Bot 2010-10-01 10:50:47 PDT
Comment on attachment 69426 [details]
Patch

Clearing flags on attachment: 69426

Committed r68899: <http://trac.webkit.org/changeset/68899>
Comment 7 WebKit Commit Bot 2010-10-01 10:50:52 PDT
All reviewed patches have been landed.  Closing bug.