840339 - jsscript.h:846:24: warning: inline function 'JSFunction* JSScript::getFunction(size_t)' used but never defined [enabled by default]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Daniel Holbert [:dholbert]]

When building mozilla-inbound, I'm getting a bunch of instances of this set of warnings:

{

 0:40.64 In file included from /scratch/work/builds/mozilla-inbound/mozilla/js/src/vm/SPSProfiler.h:18:0,
 0:40.64                  from /scratch/work/builds/mozilla-inbound/mozilla/js/src/jscntxt.h:37,
 0:40.64                  from /scratch/work/builds/mozilla-inbound/mozilla/js/src/ion/IonAllocPolicy.h:13,
 0:40.64                  from /scratch/work/builds/mozilla-inbound/mozilla/js/src/ion/BitSet.h:11,
 0:40.64                  from /scratch/work/builds/mozilla-inbound/mozilla/js/src/ion/BitSet.cpp:9:
 0:40.64 Warning: enabled by default in /scratch/work/builds/mozilla-inbound/mozilla/js/src/jsscript.h: inline function ‘js::RegExpObject* JSScript::getRegExp(size_t)’ used but never defined
 0:40.64 /scratch/work/builds/mozilla-inbound/mozilla/js/src/jsscript.h:849:30: warning: inline function ‘js::RegExpObject* JSScript::getRegExp(size_t)’ used but never defined [enabled by default]
 0:40.64 Warning: enabled by default in /scratch/work/builds/mozilla-inbound/mozilla/js/src/jsscript.h: inline function ‘JSFunction* JSScript::getFunction(size_t)’ used but never defined
 0:40.64 /scratch/work/builds/mozilla-inbound/mozilla/js/src/jsscript.h:846:24: warning: inline function ‘JSFunction* JSScript::getFunction(size_t)’ used but never defined [enabled by default]
 }


Pretty sure this is new today. I think all of them the warnings are for .cpp files in /js/src/ion.

Comment 1

[Daniel Holbert [:dholbert]]

CC'ing sstangl, since he was last to touch js/ion (in bug 839727, which doesn't look like it would've caused this, but I'm not sure -- and if it didn't cause this, maybe he knows what did and/or how to fix it. :))

Comment 2

[Sean Stangl [:sstangl]]

Attachment: Define getRegExp() for ion/CompileInfo.h.

Comment 3

[Daniel Holbert [:dholbert]]

Comment on attachment 712712 [details] [diff] [review]
Define getRegExp() for ion/CompileInfo.h.

>Subject: [PATCH] Define getRegExp() for ion/CompileInfo.h.

nit: we're (importantly) defining getFunction(), too. (that's one of the two missing definitions), so maybe mention that in the commit message, or just mention the header name.

Anyway -- this fixes it on my end, and seems sane, so r=me to that extent.

(I'm not a JS reviewer, but I'll trust you to make the call about whether this needs "official" JS review vs. "it works" review)

Comment 4

[Daniel Holbert [:dholbert]]

(I'm still a bit curious why this only started happening today -- looks like CompileInfo.h's been untouched for months, so it wasn't a recent change there.)

Comment 5

[Sean Stangl [:sstangl]]

I'm not sure why it only started happening today, but it's definitely correct for CompileInfo.h to include its dependencies.

https://hg.mozilla.org/integration/mozilla-inbound/rev/d2702388ca93

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d2702388ca93

Comment 7

[Daniel Holbert [:dholbert]]

hg bisect says this was introduced by:

changeset:   121193:80a21124ddbd
user:        Nicholas D. Matsakis <[e-mail]>
date:        Thu Feb 07 11:34:43 2013 -0800
summary:     Bug 807853 - Add (but do not yet use) parallel compilation mode to ion r=dvander,terrence

(So this appeared last week, earlier than I'd guessed at the end of comment 0)

That cset added an << #include "CompileInfo.h" >> statement to Ion.h -- I bet that's what triggered this, since lots of stuff includes Ion.h.

Anyway, fixed now -- woot!

Comment 8

[Daniel Holbert [:dholbert]]

Apparently bug 842723 removed this bug's #include, and I filed bug 845503 on the new instances of these warnings before discovering that this bug here had effectively just been backed out.

Reopening this bug, since its patch was backed out.

Quoting from bug 845503 comment #9:
> Here's the list of .cpp files that suffer from these exact build warnings,
> on my machine (w/ up-to-date m-i):
>  BitSet.cpp
>  IonSpewer.cpp
>  MoveResolver.cpp
>  IonFrames-x86-shared.cpp

Perhaps those .cpp files all just need < #include "jsscriptinlines.h" > ?

Comment 10

[Niko Matsakis [:nmatsakis]]

I think what is needed is for CompileInfo.h to have a CompileInfo-inl.h, if it needs content from jsscriptinlines.h.  If you include jsscriptinlines.h directly from CompileInfo.h, you wind up with circular includes.  I can prepare a patch to this effect in a few hours.

Comment 11

[Niko Matsakis [:nmatsakis]]

Attachment: Move inline functions that use contents of jsscriptinlines.h into CompileInfo-inl.h

Hopefully this is the final round of these lingering warnings.

Comment 12

[Niko Matsakis [:nmatsakis]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/569102ce294a

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/569102ce294a