Skip to content
This repository has been archived by the owner on Nov 12, 2022. It is now read-only.

Update SpiderMonkey #494

Merged
merged 1 commit into from
Mar 6, 2020
Merged

Update SpiderMonkey #494

merged 1 commit into from
Mar 6, 2020

Conversation

nox
Copy link
Contributor

@nox nox commented Jan 22, 2020

No description provided.

@nox nox force-pushed the smup branch 19 times, most recently from f8235de to 3f1feac Compare January 29, 2020 12:59
@nox nox force-pushed the smup branch 4 times, most recently from 011f38b to 2ec45c6 Compare February 4, 2020 11:59
@nox
Copy link
Contributor Author

nox commented Feb 4, 2020

The enumerate test seems to fail on x86_64 on Windows.

@nox
Copy link
Contributor Author

nox commented Feb 4, 2020

Even better, this does not build on Windows with --features debugmozjs.

@nox
Copy link
Contributor Author

nox commented Feb 4, 2020

Error seems to be rust-lang/libc#1433

appveyor.yml Outdated Show resolved Hide resolved
@nox nox force-pushed the smup branch 2 times, most recently from 95b07b4 to 9ba26ab Compare February 13, 2020 16:24
@nox nox force-pushed the smup branch 4 times, most recently from dd3bffe to cc0a7b1 Compare February 24, 2020 16:51
@nox
Copy link
Contributor Author

nox commented Feb 24, 2020

I fixed the failing enumerate test, this was indeed an ABI issue with MSVC (and clang-cl it seems).

@nox nox force-pushed the smup branch 2 times, most recently from 1b8107d to a44a2de Compare February 24, 2020 17:25
@nox

This comment has been minimized.

@nox nox force-pushed the smup branch 12 times, most recently from b27943c to 30ac54c Compare March 2, 2020 14:21
@nox nox changed the title Update SpiderMonkey (WIP) Update SpiderMonkey Mar 5, 2020
Copy link
Member

@asajeffrey asajeffrey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. You can r=me once the mozjs-sys changes land and are reflected here.


VCVARSALL_PATH: C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Auxiliary\Build

matrix:
- TARGET: i686-pc-windows-msvc
CROSS_COMPILE: 1
- TARGET: x86_64-pc-windows-msvc
- TARGET: x86_64-pc-windows-msvc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add some UWP targets too?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't without using something like xargo to also build the standard library, sadly.

src/consts.rs Show resolved Hide resolved
ownsUnits_: false,
_phantom_0: std::marker::PhantomData,
};
if !Evaluate2(self.cx(), options.ptr, &mut source, rval.into()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need the Utf marker any more?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

source is a root::JS::SourceText<root::mozilla::Utf8Unit>, Evaluate2 only understands UTF-8.

} else {
(script.as_bytes().as_ptr(), script.len() as c_uint)
};
assert!(!ptr.is_null());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It got over its dislike of null?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty slices have not been the null pointer for years.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And strings etc I mean, an &str with a null pointer is UB and has been for many years.

@nox
Copy link
Contributor Author

nox commented Mar 5, 2020

@bors-servo r=asajeffrey

@bors-servo
Copy link
Contributor

📌 Commit a09ebb1 has been approved by asajeffrey

@bors-servo
Copy link
Contributor

⌛ Testing commit a09ebb1 with merge 98ebda0...

@bors-servo
Copy link
Contributor

☀️ Test successful - checks-travis, status-appveyor
Approved by: asajeffrey
Pushing 98ebda0 to master...

@bors-servo bors-servo merged commit 98ebda0 into master Mar 6, 2020
@nox nox deleted the smup branch March 6, 2020 10:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants