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


(Core :: JavaScript Engine: JIT, defect)

Not set



Tracking Status
firefox60 --- fixed


(Reporter: Waldo, Assigned: Waldo)


(Blocks 1 open bug)



(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/  The reason for that difference, I'm not sure.  (th basically does s/UNIFIED_SOURCES/SOURCES/g on the, and it's not obvious why that would be at all different from the FPUF change.) 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]

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 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
Move a few trivial -inl.h definitions into the .h to eliminate used-but-not-defined compiler warnings (errors, with -Werror).  r=nbp
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.)
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.