Skip to content

Conversation

Manishearth and others added 6 commits March 27, 2015 22:37
…tring, r=alexcrichton

 closes rust-lang#23620

This PR patches the issue mentioned in rust-lang#23620, but there is also an ICE for invalid escape sequences in byte literals. This is due to the fact that the `scan_byte` function returns ` token::intern(\"??\") ` for invalid bytes, resulting in an ICE later on. Is there a reason for this behavior? Shouldn't `scan_byte` fail when it encounters an invalid byte?

And I noticed a small inconsistency in the documentation. According to the formal byte literal definition in http://doc.rust-lang.org/reference.html#byte-and-byte-string-literals , a byte string literal contains `string_body *`, but according to the text (and the behavior of the lexer) it should not accept unicode escape sequences. Hence it should be replaced by `byte_body *`. If this is valid, I can add this fix to this PR.
 This PR introduces a `Reflect` marker trait which is a supertrait of `Any`. The idea is that `Reflect` is defined for all concrete types, but is not defined for type parameters unless there is a `T:Reflect` bound. This is intended to preserve the parametricity property. This allows the `Any` interface to be stabilized without committing us to unbounded reflection that is not easily detectable by the caller.

The implementation of `Reflect` relies on an experimental variant of OIBIT. This variant behaves differently for objects, since it requires that all types exposed as part of the object's *interface* are `Reflect`, but isn't concerned about other types that may be closed over. In other words, you don't have to write `Foo+Reflect` in order for `Foo: Reflect` to hold (where `Foo` is a trait).

Given that `Any` is slated to stabilization and hence that we are committed to some form of reflection, the goal of this PR is to leave our options open with respect to parametricity. I see the options for full stabilization as follows (I think an RFC would be an appropriate way to confirm whichever of these three routes we take):

1. We make `Reflect` a lang-item.
2. We stabilize some version of the OIBIT variation I implemented as a general mechanism that may be appropriate for other use cases.
3. We give up on preserving parametricity here and just have `impl<T> Reflect for T` instead. In that case, `Reflect` is a harmless but not especially useful trait going forward.

cc @aturon
cc @alexcrichton
cc @glaebhoerl (this is more-or-less your proposal, as I understood it)
cc @reem (this is more-or-less what we discussed on IRC at some point)
cc @flaper87 (vaguely pertains to OIBIT)
…ing-syntax, r=aturon

 This syntax has been deprecated for quite some time, and there were only a few
remaining uses of it in the codebase anyway.
@rust-highfive
Copy link
Contributor

r? @huonw

(rust_highfive has picked a reviewer for you, use r? to override)

@Manishearth
Copy link
Member Author

@bors: r+ p=2

@bors
Copy link
Collaborator

bors commented Mar 30, 2015

📌 Commit 414d3e7 has been approved by Manishearth

@Manishearth
Copy link
Member Author

@bors: r+ p=2

@bors
Copy link
Collaborator

bors commented Mar 30, 2015

📌 Commit 3b45470 has been approved by Manishearth

bors added a commit that referenced this pull request Mar 30, 2015
@bors
Copy link
Collaborator

bors commented Mar 30, 2015

⌛ Testing commit 3b45470 with merge 6cf3b0b...

@bors
Copy link
Collaborator

bors commented Mar 30, 2015

@bors bors merged commit 3b45470 into rust-lang:master Mar 30, 2015
@Centril Centril added the rollup A PR which is a rollup label Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rollup A PR which is a rollup

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants