Skip to content

Make nil == null, js.Undefined == undefined a bijection. #240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 9, 2015
Merged

Make nil == null, js.Undefined == undefined a bijection. #240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a55786e
Circle CI: Update Go to latest stable version 1.4.2.
dmitshur Jun 8, 2015
8839012
Circle CI: Run gopherjs tests with Go as well.
dmitshur Jun 8, 2015
cb553f3
Fix interface equality for undefined (js.Undefined).
dmitshur Jun 8, 2015
bf81f38
Fix internalize case for undefined value.
dmitshur Jun 8, 2015
0bfcd10
Fix $interfaceIsEqual general handling of two *js.Object.
dmitshur Jun 9, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion circle.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ machine:

dependencies:
pre:
- cd /usr/local && sudo rm -rf go && curl https://storage.googleapis.com/golang/go1.4.1.linux-amd64.tar.gz | sudo tar -xz && sudo chmod a+w go/src/path/filepath
- cd /usr/local && sudo rm -rf go && curl https://storage.googleapis.com/golang/go1.4.2.linux-amd64.tar.gz | sudo tar -xz && sudo chmod a+w go/src/path/filepath
Copy link
Member

Choose a reason for hiding this comment

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

Separate commit pls.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in a55786e.

post:
- npm install --global node-gyp
- cd node-syscall && node-gyp rebuild && mkdir -p ~/.node_libraries/ && cp build/Release/syscall.node ~/.node_libraries/syscall.node

test:
override:
- ./gopherjs test --short --minify github.com/gopherjs/gopherjs/tests github.com/gopherjs/gopherjs/js archive/tar archive/zip bufio bytes compress/bzip2 compress/flate compress/gzip compress/lzw compress/zlib container/heap container/list container/ring crypto/aes crypto/cipher crypto/des crypto/dsa crypto/ecdsa crypto/elliptic crypto/hmac crypto/md5 crypto/rand crypto/rc4 crypto/rsa crypto/sha1 crypto/sha256 crypto/sha512 crypto/subtle crypto/x509 database/sql database/sql/driver debug/dwarf debug/elf debug/gosym debug/macho debug/pe encoding/ascii85 encoding/asn1 encoding/base32 encoding/base64 encoding/binary encoding/csv encoding/gob encoding/hex encoding/json encoding/pem encoding/xml errors expvar flag fmt go/ast go/doc go/format go/parser go/printer go/scanner go/token hash/adler32 hash/crc32 hash/crc64 hash/fnv html html/template image image/color image/draw image/gif image/jpeg image/png index/suffixarray io io/ioutil math math/big math/cmplx math/rand mime mime/multipart net/http/cookiejar net/http/fcgi net/mail net/rpc/jsonrpc net/textproto net/url path path/filepath reflect regexp regexp/syntax sort strconv strings sync sync/atomic testing/quick text/scanner text/tabwriter text/template text/template/parse time unicode unicode/utf16 unicode/utf8
- go test -v -race ./...
Copy link
Member Author

Choose a reason for hiding this comment

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

The beauty of doing this is that it verifies that my "expected test output" for GopherJS tests match what the actual Go compiler produces. So doing this is a great idea.

Note that the newly added TestNilUndefinedEquality test was actually passing using Go compiler without me making any changes (clearly, this PR only makes changes to the GopherJS compiler, not Go compiler), and I used this to verify.

3 changes: 3 additions & 0 deletions compiler/prelude/jsmapping.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,9 @@ var $internalize = function(v, t, recv) {
if (v === null) {
return $ifaceNil;
}
if (v === undefined) {
return new $jsObjectPtr(undefined);
}
Copy link
Member

Choose a reason for hiding this comment

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

This looks good. The JS undefined is stored in an interface by wrapping it into *js.Object.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the reason a === b doesn't hold below when comparing two instances of undefined may be because I'm calling new every time here. Maybe it should be done once outside, and this can return a single/the same instance of new $jsObjectPtr(undefined)" every time.

Edit: I tried this, maybe not very correctly, and it didn't get me anywhere, so I stayed with doing return new $jsObjectPtr(undefined); here. It's still possible there may be a future improvement here, but I'll leave it for now.

switch (v.constructor) {
case Int8Array:
return new ($sliceType($Int8))(v);
Expand Down
3 changes: 3 additions & 0 deletions compiler/prelude/prelude.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,9 @@ var $interfaceIsEqual = function(a, b) {
if (a.constructor !== b.constructor) {
return false;
}
if (a.constructor === $jsObjectPtr) {
return a.object === b.object;
}
if (!a.constructor.comparable) {
$throwRuntimeError("comparing uncomparable type " + a.constructor.string);
}
Expand Down
25 changes: 25 additions & 0 deletions js/js_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,31 @@ func TestEquality(t *testing.T) {
}
}

func TestUndefinedEquality(t *testing.T) {
var ui interface{} = js.Undefined
if ui != js.Undefined {
t.Fail()
}
}

func TestInterfaceEquality(t *testing.T) {
o := js.Global.Get("Object").New()
var i interface{} = o
if i != o {
t.Fail()
}
}

func TestUndefinedInternalization(t *testing.T) {
undefinedEqualsJsUndefined := func(i interface{}) bool {
return i == js.Undefined
}
js.Global.Set("undefinedEqualsJsUndefined", undefinedEqualsJsUndefined)
if !js.Global.Call("eval", "(undefinedEqualsJsUndefined(undefined))").Bool() {
t.Fail()
}
}

func TestSameFuncWrapper(t *testing.T) {
a := func(_ string) {} // string argument to force wrapping
b := func(_ string) {} // string argument to force wrapping
Expand Down