1716247 - Visual Studio 2017 fails to compile SM headers

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect, P3)

Description

[Ted Campbell [:tcampbell]]

We apparently didn't actually fix the issue in Bug 1679736. At the root of the problem is are some reinterpret_casts which I do not like so I have a few patches to remove the RootListEntry type in favour of an appropriate untemplated base type. This has the side effect of preventing MSVC from freaking out too.

Comment 1

[Ted Campbell [:tcampbell]]

Attachment: Bug 1716247 - Remove dead code from GC rooting API. r?sfink!

Comment 2

[Ted Campbell [:tcampbell]]

Attachment: Bug 1716247 - Add js::PersistentRootedBase type. r?sfink!

Add a common base type for all PersistentRooted instead of relying on undefined
reinterpret_casts. This avoids PersistentRooted<RootListEntry*> which causes
potential issues when SpiderMonkey is used in a project compiled under MSVC. The
static_cast is still unchecked so the same care needs to be taken as before.

Depends on D117603

Comment 3

[Ted Campbell [:tcampbell]]

Attachment: Bug 1716247 - Add js::RootedUntypedBase type. r?sfink!

Add a common base type with the template parameter erased. This lets us remove
the RootListEntry type and the undefined reinterpret_casts it is using.

Depends on D117604

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c52f0e7918ec
Remove dead code from GC rooting API. r=sfink
https://hg.mozilla.org/integration/autoland/rev/0ea7f627a717
Add js::PersistentRootedBase type. r=sfink
https://hg.mozilla.org/integration/autoland/rev/58137e59d8fc
Add js::RootedUntypedBase type. r=sfink

Comment 6

[Sandor Molnar[:smolnar]]

Backed out for causing jsreftest failures

Backout link: https://hg.mozilla.org/integration/autoland/rev/2c895de877c039bafccadccea349be9337ededf8

Push with failures

Failure log

LE
Also causing xpc failures

Failure log xpc

Comment 7

[Ted Campbell [:tcampbell]]

Attachment: Bug 1716247 - Fix RootListEntry on MSVC. r?sfink!

When compiling spidermonkey public APIs under MSVC, the dummy type RootListEntry
needs a GCPolicy even though it is not instantiated.

Comment 8

[Ted Campbell [:tcampbell]]

In order to get something into ESR-91, I'll just use the minimal hack. The refactoring patches ran into issues with RootedTraceable and more design work is ndeeded there.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2e52e5e4b021
Fix RootListEntry on MSVC. r=sfink

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/416ef98b0533
Remove dead code from GC rooting API. r=sfink

Comment 11

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/2e52e5e4b021
https://hg.mozilla.org/mozilla-central/rev/416ef98b0533