Closed Bug 765159 Opened 8 years ago Closed 6 years ago

Remove SkippableDummy implementations in nsCycleCollectionParticipant.h

Categories

(Core :: XPCOM, defect)

All
Windows Server 2003
defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 881323

People

(Reporter: glandium, Unassigned)

References

Details

Somehow, Windows PGO builds fail because of lacking definitions for SkippableDummy member functions. Adding dummy implementations fixes the problem without impacting anything else, but it would be better to find why it is needed at all.
Blocks: 616262
This is the kind of stupid stuff MSVC generates when building with no optimization (-Od):
        xor     eax, eax
        je      SHORT $LN3@cycleColle
        mov     DWORD PTR tv65[ebp], OFFSET ?CanSkipImpl@SkippableDummy@@SG_NPAX_N@Z ; SkippableDummy::CanSkipImpl
        jmp     SHORT $LN4@cycleColle
$LN3@cycleColle:
        mov     DWORD PTR tv65[ebp], 0
$LN4@cycleColle:

This is for "isSkippable ? (template that ends up inheriting from SkippableDummy)::CanSkipImpl : NULL" (where isSkippable is a const and is false). I presume it keeps something about as stupid in track in the AST used in WPO/PGO builds (if someone knows a tool to "disassemble" these objects, I'd be glad to hear).

So it looks like we just can't get rid of them. Might be just as good to put a MOZ_NOT_REACHED in them.
(In reply to Mike Hommey [:glandium] from comment #1)
> This is the kind of stupid stuff MSVC generates when building with no
> optimization (-Od):

FTR, even gcc is not that dumb at -O0.
This corresponds to what we were doing before (including the NOT_REACHED sort of thing), so I don't think it is a big deal.  If we can put it in the .cpp file that's probably better, not that it really matters in the overall scheme of things.
I have nothing productive to add, but I would like to note for the record that my bug 616262 comment 30 was eerily prescient.  :-)
This was effectively fixed by bug 881323 essentially reverting bug 616262.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 881323
You need to log in before you can comment on or make changes to this bug.