If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

144 lines of MSVC build warning output from "warning C4396: 'js::EmptyShape::ensureInitialCustomShape' : the inline specifier cannot be used when a friend declaration refers to a specialization of a function template"

RESOLVED FIXED in Firefox 39

Status

()

Core
JavaScript Engine
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: dholbert, Assigned: xidorn)

Tracking

(Blocks: 1 bug)

Trunk
mozilla39
All
Windows XP
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox39 fixed)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
Noticed a bunch of instances of this build warning when skimming through a recent Win32 build log:
{
Warning: C4396 in $SRC\js\src\vm\RegExpObject.h: 'js::EmptyShape::ensureInitialCustomShape' : the inline specifier cannot be used when a friend declaration refers to a specialization of a function template
$SRC\js\src\vm/RegExpObject.h(452) : warning C4396: 'js::EmptyShape::ensureInitialCustomShape' : the inline specifier cannot be used when a friend declaration refers to a specialization of a function template

Warning: C4396 in $SRC\js\src\vm\ErrorObject.h: 'js::EmptyShape::ensureInitialCustomShape' : the inline specifier cannot be used when a friend declaration refers to a specialization of a function template
$SRC\js\src\vm/ErrorObject.h(40) : warning C4396: 'js::EmptyShape::ensureInitialCustomShape' : the inline specifier cannot be used when a friend declaration refers to a specialization of a function template

Warning: C4396 in $SRC\js\src\vm\StringObject.h: 'js::EmptyShape::ensureInitialCustomShape' : the inline specifier cannot be used when a friend declaration refers to a specialization of a function template
$SRC\js\src\vm/StringObject.h(65) : warning C4396: 'js::EmptyShape::ensureInitialCustomShape' : the inline specifier cannot be used when a friend declaration refers to a specialization of a function template
}

Overall, this issue triggers 144 lines of warning-spam, in this latest m-c build win32 log:
http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32-debug/1422544309/mozilla-central-win32-debug-bm82-build1-build774.txt.gz

(based on grepping for "ensureInitialCustomShape' : the inline specifier ")

Looks like this function (ensureInitialCustomShape) was added in bug 724768.
(Reporter)

Updated

3 years ago
OS: Linux → Windows XP
Hardware: x86_64 → All
(Reporter)

Comment 1

3 years ago
This templated function is defined (as "inline", per the warning-message) here:
http://mxr.mozilla.org/mozilla-central/source/js/src/vm/Shape-inl.h?rev=7a7a248c492f#133

...and specializations of it are declared as "friend" functions in (at least) these three places:
http://mxr.mozilla.org/mozilla-central/source/js/src/vm/ErrorObject.h?rev=7a7a248c492f#38
http://mxr.mozilla.org/mozilla-central/source/js/src/vm/RegExpObject.h?rev=7a7a248c492f#450
http://mxr.mozilla.org/mozilla-central/source/js/src/vm/StringObject.h?rev=7a7a248c492f#63

And these headers are presumably #included all over the place, which is why this is so spammy.
(Reporter)

Comment 2

3 years ago
This build warning is documented here:
  https://msdn.microsoft.com/en-us/library/bb384968%28v=vs.90%29.aspx
That page says MSVC just ignores "inline" in this case.  So, should we drop the "inline" keyword, since it's not having any effect (at least on MSVC)?  (and perhaps we should then also move the function out of its "-inl" file)

Or is there something else we should do here?
Flags: needinfo?(jwalden+bmo)
Summary: 144 lines of MSVC build warning output for "js::EmptyShape::ensureInitialCustomShape' : the inline specifier cannot be used when a friend declaration refers to a specialization of a function template" → 144 lines of MSVC build warning output from "warning C4396: 'js::EmptyShape::ensureInitialCustomShape' : the inline specifier cannot be used when a friend declaration refers to a specialization of a function template"
We probably should not remove the "inline" here if it is added by design. I wrote some code to test the behavior of different compilers, and it seems that the "inline" here doesn't affect the asm code generated by GCC, but does affect the code by Clang.

But I wonder whether we can directly make |assignInitialShape| of the objects public, so that we can get rid of the friend declartions here. Alternatively, we can just move the |assignInitialShape|s out from there classes to an independent function template without adding any additional friend. AFAICS, all of those functions only call method NativeObject::addDataProperty() which is public. I see no reason why we cannot expose them to the public.
Created attachment 8567712 [details] [diff] [review]
patch
Attachment #8567712 - Flags: review?(wmccloskey)
Blocks: 1135535
Comment on attachment 8567712 [details] [diff] [review]
patch

Sorry, I've kinda lost touch with this code.
Attachment #8567712 - Flags: review?(wmccloskey) → review?(luke)

Updated

3 years ago
Attachment #8567712 - Flags: review?(luke) → review+
https://treeherder.mozilla.org/#/jobs?repo=try&revision=af0936a4d98f
https://hg.mozilla.org/integration/mozilla-inbound/rev/38c14390bd27
https://hg.mozilla.org/mozilla-central/rev/38c14390bd27
Assignee: nobody → quanxunzhen
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Flags: needinfo?(jwalden+bmo)
You need to log in before you can comment on or make changes to this bug.