Make DOMString a newtype around String, rather than a typedef. #8312

Ms2ger · 2015-11-03T15:14:04Z

highfive · 2015-11-03T15:14:08Z

Warning

These commits modify unsafe code. Please review it carefully!
These commits modify layout code, but no reftests are modified. Please consider adding a reftest!

asajeffrey · 2015-11-03T15:23:42Z

Yay, this will make experimenting with alternative DOMString representations a lot easier! A couple of comments...

It would be nice to have a function for converting DOMString to String: x.flatten() or x.to_utf8() or something. At the moment there's lots of x.0 in the code, which would all need rewritten if DOMString changed internals.

It might be nice to use a macro domstring!("foo") or something, so that we might be able to do compile-time magic to optimize constant DOMStrings. I can imagine the macro using a different representation for short strings or strings which are in the list of blessed atoms.

Ms2ger · 2015-11-03T15:25:47Z

Agreed, but I'd prefer to push that to a followup.

Manishearth · 2015-11-03T15:41:38Z

Reviewed 24 of 24 files at r1, 79 of 79 files at r2, 1 of 1 files at r3.
Review status: all files reviewed at latest revision, 3 unresolved discussions.

^{components/script/dom/bindings/trace.rs, line 287 [r2] (raw file):}
I prefer derive(trace) on DOMstring, the macro is only for stuff we importn

^{components/util/mem.rs, line 104 [r2] (raw file):}
use derivexz

^{components/util/str.rs, line 31 [r2] (raw file):}
I'd prefer this to deref to &str if it doesn't cause too much churnxzx

Comments from the review on Reviewable.io

Ms2ger · 2015-11-03T16:03:44Z

Review status: all files reviewed at latest revision, 3 unresolved discussions.

^{components/script/dom/bindings/trace.rs, line 287 [r2] (raw file):}
Me too, but DOMString is in util, so it can't see script's JSTraceable.

^{components/util/mem.rs, line 104 [r2] (raw file):}
It doesn't work in util itself.

Comments from the review on Reviewable.io

Ms2ger · 2015-11-03T16:16:25Z

@bors-servo r=jdm

bors-servo · 2015-11-03T16:16:25Z

📌 Commit 98c8437 has been approved by jdm

Ms2ger · 2015-11-03T16:23:53Z

@bors-servo p=9

bors-servo · 2015-11-03T16:41:24Z

⌛ Testing commit 98c8437 with merge aadfcd0...

Make DOMString a newtype around String, rather than a typedef.  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8312)

bors-servo · 2015-11-03T17:15:20Z

💔 Test failed - mac-dev-ref-unit

bors-servo · 2015-11-03T18:04:43Z

☔ The latest upstream changes (presumably #8232) made this pull request unmergeable. Please resolve the merge conflicts.

Manishearth · 2015-11-03T18:05:13Z

nooooo

jdm · 2015-11-03T19:17:10Z

/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:21:55: 21:75 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:21     let mut textinput = TextInput::new(Lines::Single, "abcdefg".to_owned(), DummyClipboardContext::new(""));
                                                                                                                                           ^~~~~~~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:21:55: 21:75 help: run `rustc --explain E0308` to see a detailed explanation
<std macros>:9:3: 9:13 error: the trait `core::fmt::Debug` is not implemented for the type `util::str::DOMString` [E0277]
<std macros>:9 , * left_val , * right_val ) } } } } )
                 ^~~~~~~~~~
<std macros>:9:1: 9:40 note: in this expansion of format_args!
<std macros>:6:1: 9:29 note: in this expansion of panic! (defined in <std macros>)
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:24:5: 24:51 note: in this expansion of assert_eq! (defined in <std macros>)
<std macros>:9:3: 9:13 help: run `rustc --explain E0277` to see a detailed explanation
<std macros>:9:3: 9:13 note: `util::str::DOMString` cannot be formatted using `:?`; if it is defined in your crate, add `#[derive(Debug)]` or manually implement it
<std macros>:9:3: 9:13 note: required by `core::fmt::Debug::fmt`
<std macros>:9:3: 9:13 error: the trait `core::fmt::Debug` is not implemented for the type `util::str::DOMString` [E0277]
<std macros>:9 , * left_val , * right_val ) } } } } )
                 ^~~~~~~~~~
<std macros>:9:1: 9:40 note: in this expansion of format_args!
<std macros>:6:1: 9:29 note: in this expansion of panic! (defined in <std macros>)
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:27:5: 27:50 note: in this expansion of assert_eq! (defined in <std macros>)
<std macros>:9:3: 9:13 help: run `rustc --explain E0277` to see a detailed explanation
<std macros>:9:3: 9:13 note: `util::str::DOMString` cannot be formatted using `:?`; if it is defined in your crate, add `#[derive(Debug)]` or manually implement it
<std macros>:9:3: 9:13 note: required by `core::fmt::Debug::fmt`
<std macros>:9:3: 9:13 error: the trait `core::fmt::Debug` is not implemented for the type `util::str::DOMString` [E0277]
<std macros>:9 , * left_val , * right_val ) } } } } )
                 ^~~~~~~~~~
<std macros>:9:1: 9:40 note: in this expansion of format_args!
<std macros>:6:1: 9:29 note: in this expansion of panic! (defined in <std macros>)
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:31:5: 31:48 note: in this expansion of assert_eq! (defined in <std macros>)
<std macros>:9:3: 9:13 help: run `rustc --explain E0277` to see a detailed explanation
<std macros>:9:3: 9:13 note: `util::str::DOMString` cannot be formatted using `:?`; if it is defined in your crate, add `#[derive(Debug)]` or manually implement it
<std macros>:9:3: 9:13 note: required by `core::fmt::Debug::fmt`
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:33:55: 33:71 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:33     let mut textinput = TextInput::new(Lines::Single, "a🌠b".to_owned(), DummyClipboardContext::new(""));
                                                                                                                                           ^~~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:33:55: 33:71 help: run `rustc --explain E0308` to see a detailed explanation
<std macros>:9:3: 9:13 error: the trait `core::fmt::Debug` is not implemented for the type `util::str::DOMString` [E0277]
<std macros>:9 , * left_val , * right_val ) } } } } )
                 ^~~~~~~~~~
<std macros>:9:1: 9:40 note: in this expansion of format_args!
<std macros>:6:1: 9:29 note: in this expansion of panic! (defined in <std macros>)
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:38:5: 38:47 note: in this expansion of assert_eq! (defined in <std macros>)
<std macros>:9:3: 9:13 help: run `rustc --explain E0277` to see a detailed explanation
<std macros>:9:3: 9:13 note: `util::str::DOMString` cannot be formatted using `:?`; if it is defined in your crate, add `#[derive(Debug)]` or manually implement it
<std macros>:9:3: 9:13 note: required by `core::fmt::Debug::fmt`
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:43:55: 43:75 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:43     let mut textinput = TextInput::new(Lines::Single, "abcdefg".to_owned(), DummyClipboardContext::new(""));
                                                                                                                                           ^~~~~~~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:43:55: 43:75 help: run `rustc --explain E0308` to see a detailed explanation
<std macros>:9:3: 9:13 error: the trait `core::fmt::Debug` is not implemented for the type `util::str::DOMString` [E0277]
<std macros>:9 , * left_val , * right_val ) } } } } )
                 ^~~~~~~~~~
<std macros>:9:1: 9:40 note: in this expansion of format_args!
<std macros>:6:1: 9:29 note: in this expansion of panic! (defined in <std macros>)
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:46:5: 46:53 note: in this expansion of assert_eq! (defined in <std macros>)
<std macros>:9:3: 9:13 help: run `rustc --explain E0277` to see a detailed explanation
<std macros>:9:3: 9:13 note: `util::str::DOMString` cannot be formatted using `:?`; if it is defined in your crate, add `#[derive(Debug)]` or manually implement it
<std macros>:9:3: 9:13 note: required by `core::fmt::Debug::fmt`
<std macros>:9:3: 9:13 error: the trait `core::fmt::Debug` is not implemented for the type `util::str::DOMString` [E0277]
<std macros>:9 , * left_val , * right_val ) } } } } )
                 ^~~~~~~~~~
<std macros>:9:1: 9:40 note: in this expansion of format_args!
<std macros>:6:1: 9:29 note: in this expansion of panic! (defined in <std macros>)
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:50:5: 50:52 note: in this expansion of assert_eq! (defined in <std macros>)
<std macros>:9:3: 9:13 help: run `rustc --explain E0277` to see a detailed explanation
<std macros>:9:3: 9:13 note: `util::str::DOMString` cannot be formatted using `:?`; if it is defined in your crate, add `#[derive(Debug)]` or manually implement it
<std macros>:9:3: 9:13 note: required by `core::fmt::Debug::fmt`
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:52:55: 52:71 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:52     let mut textinput = TextInput::new(Lines::Single, "a🌠c".to_owned(), DummyClipboardContext::new(""));
                                                                                                                                           ^~~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:52:55: 52:71 help: run `rustc --explain E0308` to see a detailed explanation
<std macros>:9:3: 9:13 error: the trait `core::fmt::Debug` is not implemented for the type `util::str::DOMString` [E0277]
<std macros>:9 , * left_val , * right_val ) } } } } )
                 ^~~~~~~~~~
<std macros>:9:1: 9:40 note: in this expansion of format_args!
<std macros>:6:1: 9:29 note: in this expansion of panic! (defined in <std macros>)
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:58:5: 58:49 note: in this expansion of assert_eq! (defined in <std macros>)
<std macros>:9:3: 9:13 help: run `rustc --explain E0277` to see a detailed explanation
<std macros>:9:3: 9:13 note: `util::str::DOMString` cannot be formatted using `:?`; if it is defined in your crate, add `#[derive(Debug)]` or manually implement it
<std macros>:9:3: 9:13 note: required by `core::fmt::Debug::fmt`
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:63:55: 63:75 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:63     let mut textinput = TextInput::new(Lines::Single, "abcdefg".to_owned(), DummyClipboardContext::new(""));
                                                                                                                                           ^~~~~~~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:63:55: 63:75 help: run `rustc --explain E0308` to see a detailed explanation
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:80:55: 80:75 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:80     let mut textinput = TextInput::new(Lines::Single, "abcdefg".to_owned(), DummyClipboardContext::new(""));
                                                                                                                                           ^~~~~~~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:80:55: 80:75 help: run `rustc --explain E0308` to see a detailed explanation
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:84:33: 84:49 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:84     textinput.replace_selection("xyz".to_owned());
                                                                                                                     ^~~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:84:33: 84:49 help: run `rustc --explain E0308` to see a detailed explanation
<std macros>:9:3: 9:13 error: the trait `core::fmt::Debug` is not implemented for the type `util::str::DOMString` [E0277]
<std macros>:9 , * left_val , * right_val ) } } } } )
                 ^~~~~~~~~~
<std macros>:9:1: 9:40 note: in this expansion of format_args!
<std macros>:6:1: 9:29 note: in this expansion of panic! (defined in <std macros>)
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:85:5: 85:53 note: in this expansion of assert_eq! (defined in <std macros>)
<std macros>:9:3: 9:13 help: run `rustc --explain E0277` to see a detailed explanation
<std macros>:9:3: 9:13 note: `util::str::DOMString` cannot be formatted using `:?`; if it is defined in your crate, add `#[derive(Debug)]` or manually implement it
<std macros>:9:3: 9:13 note: required by `core::fmt::Debug::fmt`
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:90:57: 90:80 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:90     let mut textinput = TextInput::new(Lines::Multiple, "abc\nde\nf".to_owned(), DummyClipboardContext::new(""));
                                                                                                                                             ^~~~~~~~~~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:90:57: 90:80 help: run `rustc --explain E0308` to see a detailed explanation
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:102:57: 102:80 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:102     let mut textinput = TextInput::new(Lines::Multiple, "abc\nde\nf".to_owned(), DummyClipboardContext::new(""));
                                                                                                                                              ^~~~~~~~~~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:102:57: 102:80 help: run `rustc --explain E0308` to see a detailed explanation
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:119:57: 119:80 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:119     let mut textinput = TextInput::new(Lines::Multiple, "abc\nde\nf".to_owned(), DummyClipboardContext::new(""));
                                                                                                                                              ^~~~~~~~~~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:119:57: 119:80 help: run `rustc --explain E0308` to see a detailed explanation
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:140:24: 140:43 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:140         Lines::Single, "abcdef".to_owned(), DummyClipboardContext::new(""));
                                                                                                             ^~~~~~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:140:24: 140:43 help: run `rustc --explain E0308` to see a detailed explanation
<std macros>:9:3: 9:13 error: the trait `core::fmt::Debug` is not implemented for the type `util::str::DOMString` [E0277]
<std macros>:9 , * left_val , * right_val ) } } } } )
                 ^~~~~~~~~~
<std macros>:9:1: 9:40 note: in this expansion of format_args!
<std macros>:6:1: 9:29 note: in this expansion of panic! (defined in <std macros>)
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:143:5: 143:63 note: in this expansion of assert_eq! (defined in <std macros>)
<std macros>:9:3: 9:13 help: run `rustc --explain E0277` to see a detailed explanation
<std macros>:9:3: 9:13 note: `util::str::DOMString` cannot be formatted using `:?`; if it is defined in your crate, add `#[derive(Debug)]` or manually implement it
<std macros>:9:3: 9:13 note: required by `core::fmt::Debug::fmt`
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:146:26: 146:45 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:146         Lines::Multiple, "abcdef".to_owned(), DummyClipboardContext::new(""));
                                                                                                               ^~~~~~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:146:26: 146:45 help: run `rustc --explain E0308` to see a detailed explanation
<std macros>:9:3: 9:13 error: the trait `core::fmt::Debug` is not implemented for the type `util::str::DOMString` [E0277]
<std macros>:9 , * left_val , * right_val ) } } } } )
                 ^~~~~~~~~~
<std macros>:9:1: 9:40 note: in this expansion of format_args!
<std macros>:6:1: 9:29 note: in this expansion of panic! (defined in <std macros>)
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:149:5: 149:64 note: in this expansion of assert_eq! (defined in <std macros>)
<std macros>:9:3: 9:13 help: run `rustc --explain E0277` to see a detailed explanation
<std macros>:9:3: 9:13 note: `util::str::DOMString` cannot be formatted using `:?`; if it is defined in your crate, add `#[derive(Debug)]` or manually implement it
<std macros>:9:3: 9:13 note: required by `core::fmt::Debug::fmt`
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:155:26: 155:49 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:155         Lines::Multiple, "abc\nde\nf".to_owned(), DummyClipboardContext::new(""));
                                                                                                               ^~~~~~~~~~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:155:26: 155:49 help: run `rustc --explain E0308` to see a detailed explanation
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:166:63: 166:83 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:166     let single_line_textinput = TextInput::new(Lines::Single, "abcdefg".to_owned(), DummyClipboardContext::new(""));
                                                                                                                                                    ^~~~~~~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:166:63: 166:83 help: run `rustc --explain E0308` to see a detailed explanation
<std macros>:9:3: 9:13 error: the trait `core::fmt::Debug` is not implemented for the type `util::str::DOMString` [E0277]
<std macros>:9 , * left_val , * right_val ) } } } } )
                 ^~~~~~~~~~
<std macros>:9:1: 9:40 note: in this expansion of format_args!
<std macros>:6:1: 9:29 note: in this expansion of panic! (defined in <std macros>)
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:167:5: 167:64 note: in this expansion of assert_eq! (defined in <std macros>)
<std macros>:9:3: 9:13 help: run `rustc --explain E0277` to see a detailed explanation
<std macros>:9:3: 9:13 note: `util::str::DOMString` cannot be formatted using `:?`; if it is defined in your crate, add `#[derive(Debug)]` or manually implement it
<std macros>:9:3: 9:13 note: required by `core::fmt::Debug::fmt`
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:170:26: 170:49 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:170         Lines::Multiple, "abc\nde\nf".to_owned(), DummyClipboardContext::new(""));
                                                                                                               ^~~~~~~~~~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:170:26: 170:49 help: run `rustc --explain E0308` to see a detailed explanation
<std macros>:9:3: 9:13 error: the trait `core::fmt::Debug` is not implemented for the type `util::str::DOMString` [E0277]
<std macros>:9 , * left_val , * right_val ) } } } } )
                 ^~~~~~~~~~
<std macros>:9:1: 9:40 note: in this expansion of format_args!
<std macros>:6:1: 9:29 note: in this expansion of panic! (defined in <std macros>)
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:171:5: 171:66 note: in this expansion of assert_eq! (defined in <std macros>)
<std macros>:9:3: 9:13 help: run `rustc --explain E0277` to see a detailed explanation
<std macros>:9:3: 9:13 note: `util::str::DOMString` cannot be formatted using `:?`; if it is defined in your crate, add `#[derive(Debug)]` or manually implement it
<std macros>:9:3: 9:13 note: required by `core::fmt::Debug::fmt`
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:176:57: 176:80 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:176     let mut textinput = TextInput::new(Lines::Multiple, "abc\nde\nf".to_owned(), DummyClipboardContext::new(""));
                                                                                                                                              ^~~~~~~~~~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:176:57: 176:80 help: run `rustc --explain E0308` to see a detailed explanation
<std macros>:9:3: 9:13 error: the trait `core::fmt::Debug` is not implemented for the type `util::str::DOMString` [E0277]
<std macros>:9 , * left_val , * right_val ) } } } } )
                 ^~~~~~~~~~
<std macros>:9:1: 9:40 note: in this expansion of format_args!
<std macros>:6:1: 9:29 note: in this expansion of panic! (defined in <std macros>)
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:177:5: 177:55 note: in this expansion of assert_eq! (defined in <std macros>)
<std macros>:9:3: 9:13 help: run `rustc --explain E0277` to see a detailed explanation
<std macros>:9:3: 9:13 note: `util::str::DOMString` cannot be formatted using `:?`; if it is defined in your crate, add `#[derive(Debug)]` or manually implement it
<std macros>:9:3: 9:13 note: required by `core::fmt::Debug::fmt`
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:179:27: 179:46 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:179     textinput.set_content("abc\nf".to_owned());
                                                                                                                ^~~~~~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:179:27: 179:46 help: run `rustc --explain E0308` to see a detailed explanation
<std macros>:9:3: 9:13 error: the trait `core::fmt::Debug` is not implemented for the type `util::str::DOMString` [E0277]
<std macros>:9 , * left_val , * right_val ) } } } } )
                 ^~~~~~~~~~
<std macros>:9:1: 9:40 note: in this expansion of format_args!
<std macros>:6:1: 9:29 note: in this expansion of panic! (defined in <std macros>)
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:180:5: 180:51 note: in this expansion of assert_eq! (defined in <std macros>)
<std macros>:9:3: 9:13 help: run `rustc --explain E0277` to see a detailed explanation
<std macros>:9:3: 9:13 note: `util::str::DOMString` cannot be formatted using `:?`; if it is defined in your crate, add `#[derive(Debug)]` or manually implement it
<std macros>:9:3: 9:13 note: required by `core::fmt::Debug::fmt`
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:187:27: 187:42 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:187     textinput.set_content("de".to_owned());
                                                                                                                ^~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:187:27: 187:42 help: run `rustc --explain E0308` to see a detailed explanation
<std macros>:9:3: 9:13 error: the trait `core::fmt::Debug` is not implemented for the type `util::str::DOMString` [E0277]
<std macros>:9 , * left_val , * right_val ) } } } } )
                 ^~~~~~~~~~
<std macros>:9:1: 9:40 note: in this expansion of format_args!
<std macros>:6:1: 9:29 note: in this expansion of panic! (defined in <std macros>)
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:188:5: 188:47 note: in this expansion of assert_eq! (defined in <std macros>)
<std macros>:9:3: 9:13 help: run `rustc --explain E0277` to see a detailed explanation
<std macros>:9:3: 9:13 note: `util::str::DOMString` cannot be formatted using `:?`; if it is defined in your crate, add `#[derive(Debug)]` or manually implement it
<std macros>:9:3: 9:13 note: required by `core::fmt::Debug::fmt`
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:200:55: 200:72 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:200     let mut textinput = TextInput::new(Lines::Single, "defg".to_owned(), DummyClipboardContext::new("abc"));
                                                                                                                                            ^~~~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:200:55: 200:72 help: run `rustc --explain E0308` to see a detailed explanation
<std macros>:9:3: 9:13 error: the trait `core::fmt::Debug` is not implemented for the type `util::str::DOMString` [E0277]
<std macros>:9 , * left_val , * right_val ) } } } } )
                 ^~~~~~~~~~
<std macros>:9:1: 9:40 note: in this expansion of format_args!
<std macros>:6:1: 9:29 note: in this expansion of panic! (defined in <std macros>)
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:201:5: 201:49 note: in this expansion of assert_eq! (defined in <std macros>)
<std macros>:9:3: 9:13 help: run `rustc --explain E0277` to see a detailed explanation
<std macros>:9:3: 9:13 note: `util::str::DOMString` cannot be formatted using `:?`; if it is defined in your crate, add `#[derive(Debug)]` or manually implement it
<std macros>:9:3: 9:13 note: required by `core::fmt::Debug::fmt`
<std macros>:9:3: 9:13 error: the trait `core::fmt::Debug` is not implemented for the type `util::str::DOMString` [E0277]
<std macros>:9 , * left_val , * right_val ) } } } } )
                 ^~~~~~~~~~
<std macros>:9:1: 9:40 note: in this expansion of format_args!
<std macros>:6:1: 9:29 note: in this expansion of panic! (defined in <std macros>)
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:204:5: 204:52 note: in this expansion of assert_eq! (defined in <std macros>)
<std macros>:9:3: 9:13 help: run `rustc --explain E0277` to see a detailed explanation
<std macros>:9:3: 9:13 note: `util::str::DOMString` cannot be formatted using `:?`; if it is defined in your crate, add `#[derive(Debug)]` or manually implement it
<std macros>:9:3: 9:13 note: required by `core::fmt::Debug::fmt`
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:209:55: 209:74 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:209     let mut textinput = TextInput::new(Lines::Single, "abcdef".to_owned(), DummyClipboardContext::new(""));
                                                                                                                                            ^~~~~~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:209:55: 209:74 help: run `rustc --explain E0308` to see a detailed explanation
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:233:57: 233:80 error: mismatched types:
 expected `util::str::DOMString`,
    found `collections::string::String`
(expected struct `util::str::DOMString`,
    found struct `collections::string::String`) [E0308]
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:233     let mut textinput = TextInput::new(Lines::Multiple, "abc\nde\nf".to_owned(), DummyClipboardContext::new(""));
                                                                                                                                              ^~~~~~~~~~~~~~~~~~~~~~~
/Users/servo/buildbot/slave/mac-dev-ref-unit/build/tests/unit/script/textinput.rs:233:57: 233:80 help: run `rustc --explain E0308` to see a detailed explanation
error: aborting due to 38 previous errors

This should make it somewhat easier to experiment with alternative representations in the future. To reduce churn, this commit leaves the String field public, though. Also, this will allow us to use the default String type to represent the IDL USVString type, which explicitly forbids unpaired surrogates, ans as such is a better match to the Rust String type.

Ms2ger · 2015-11-04T11:11:19Z

@bors-servo r=jdm

bors-servo · 2015-11-04T11:11:20Z

📌 Commit 6b75078 has been approved by jdm

bors-servo · 2015-11-04T11:17:41Z

⌛ Testing commit 6b75078 with merge abfd1fb...

Make DOMString a newtype around String, rather than a typedef.  [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8312)

bors-servo · 2015-11-04T11:51:25Z

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Nov 3, 2015

Ms2ger force-pushed the DOMString branch from dac13e0 to 389541b Compare November 3, 2015 15:36

Ms2ger force-pushed the DOMString branch from f6b7e20 to 98c8437 Compare November 3, 2015 16:16

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Nov 3, 2015

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Nov 3, 2015

highfive added S-needs-rebase There are merge conflict errors. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Nov 3, 2015

eefriedman assigned jdm Nov 3, 2015

Ms2ger force-pushed the DOMString branch from 98c8437 to c961e0d Compare November 4, 2015 10:08

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Nov 4, 2015

Ms2ger added 3 commits November 4, 2015 12:09

Create a helper function to create TextInput structs in its unit test.

dab3926

Use DOMString::new() somewhat consistently.

e6aa976

Ms2ger force-pushed the DOMString branch from c961e0d to 6b75078 Compare November 4, 2015 11:11

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. S-needs-rebase There are merge conflict errors. labels Nov 4, 2015

bors-servo merged commit 6b75078 into servo:master Nov 4, 2015

This was referenced Nov 4, 2015

Cleanup some unneeded let bindings #8304

Merged

Perf timing dom props #7242

Merged

Ms2ger deleted the DOMString branch November 4, 2015 12:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make DOMString a newtype around String, rather than a typedef. #8312

Make DOMString a newtype around String, rather than a typedef. #8312

Ms2ger commented Nov 3, 2015

highfive commented Nov 3, 2015

asajeffrey commented Nov 3, 2015

Ms2ger commented Nov 3, 2015

Manishearth commented Nov 3, 2015

Ms2ger commented Nov 3, 2015

Ms2ger commented Nov 3, 2015

bors-servo commented Nov 3, 2015

Ms2ger commented Nov 3, 2015

bors-servo commented Nov 3, 2015

bors-servo commented Nov 3, 2015

bors-servo commented Nov 3, 2015

Manishearth commented Nov 3, 2015

jdm commented Nov 3, 2015

Ms2ger commented Nov 4, 2015

bors-servo commented Nov 4, 2015

bors-servo commented Nov 4, 2015

bors-servo commented Nov 4, 2015

Make DOMString a newtype around String, rather than a typedef. #8312

Make DOMString a newtype around String, rather than a typedef. #8312

Conversation

Ms2ger commented Nov 3, 2015

highfive commented Nov 3, 2015

asajeffrey commented Nov 3, 2015

Ms2ger commented Nov 3, 2015

Manishearth commented Nov 3, 2015

Ms2ger commented Nov 3, 2015

Ms2ger commented Nov 3, 2015

bors-servo commented Nov 3, 2015

Ms2ger commented Nov 3, 2015

bors-servo commented Nov 3, 2015

bors-servo commented Nov 3, 2015

bors-servo commented Nov 3, 2015

Manishearth commented Nov 3, 2015

jdm commented Nov 3, 2015

Ms2ger commented Nov 4, 2015

bors-servo commented Nov 4, 2015

bors-servo commented Nov 4, 2015

bors-servo commented Nov 4, 2015