Closed Bug 1432312 Opened 2 years ago Closed 2 years ago

Move a few trivial -inl.h definitions into the .h to eliminate used-but-not-defined compiler warnings/errors

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: Waldo, Assigned: Waldo)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

These definitions are appearing in textual inclusion order *after* the uses of them in some of the SharedICHelpers-*.h files for the different platforms.  Specifically, SharedICHelpers-arm64.h calls |masm.adjustFrame| despite being file-named as a *.h and therefore it being impermissible for it to depend on a *-inl.h where the definition previously resided.

This doesn't show up as a build error there in normal builds, and (bizarrely) it doesn't show up as an error in the nu build on treeherder.  But it *does* show up as an error if I set |FILES_PER_UNIFIED_FILE = 1| in js/src/moz.build.  The reason for that difference, I'm not sure.  (th basically does s/UNIFIED_SOURCES/SOURCES/g on the moz.build, and it's not obvious why that would be at all different from the FPUF change.)

check_macroassembler_style.py seemingly has support specifically designed to allow multiple delimited blocks of decls/defs to check, so it's 100% kosher to do what I've done in MacroAssembler.h.  The other option would be to move those declarations outside the delimited section in the class.  You know better than I would which is aesthetically more pleasing.
Attached patch PatchSplinter Review
Attachment #8944553 - Flags: review?(nicolas.b.pierron)
Comment on attachment 8944553 [details] [diff] [review]
Patch

Review of attachment 8944553 [details] [diff] [review]:
-----------------------------------------------------------------

As documented at the beginning of MacroAssembler.h, being inline this function belongs to the -inl.h file.
Thus, this solution is not acceptable, as we want to migrate all methods to be checked by the check_macroassembler_style.py script. (to enforce sanity)

The problem seems to be only caused by EmitUnstowICValues functions, which seems to be unused based on searchfox.
Would removing these functions works?  Otherwise, moving them to the proper .cpp file, or to the proper -inl.h files.
Attachment #8944553 - Flags: review?(nicolas.b.pierron)
Blocks: 1432600
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ebc47d5a71dc
Move a few trivial -inl.h definitions into the .h to eliminate used-but-not-defined compiler warnings (errors, with -Werror).  r=nbp
https://hg.mozilla.org/integration/mozilla-inbound/rev/c450d89fd86e
Remove the now-unused EmitUnstowICValues function from SharedICHelpers.h.  rs=nbp
(The first patch-landing is as discussed/approved on IRC.  Second patch was a trivial removal suggested there as well.)
https://hg.mozilla.org/mozilla-central/rev/ebc47d5a71dc
https://hg.mozilla.org/mozilla-central/rev/c450d89fd86e
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.