Skip to content
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.

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

Add a type_id intrinsic #10182

Merged
merged 2 commits into from
Nov 5, 2013
Merged

Conversation

alexcrichton
Copy link
Member

This isn't quite as fancy as the struct in #9913, but I'm not sure we should be exposing crate names/hashes of the types. That being said, it'd be pretty easy to extend this (the deterministic hashing regardless of what crate you're in was the hard part).

}

impl TypeId {
/// Returns the `TypeId` of the type this generic function has been instantiated with
#[inline]
pub fn of<T>() -> TypeId {
TypeId{ t: unsafe { get_tydesc::<T>() } }
TypeId{ t: unsafe { type_id::<T>() } }
Copy link
Member

Choose a reason for hiding this comment

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

Won't this fail in stage0 since the intrinsic isn't snapshotted yet?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. As a responsible contributor I always compile my code before submitting it!

@brson
Copy link
Contributor

brson commented Oct 31, 2013

This looks really awesome. This hash routine should probably replace all our type hashing (which I understand is currently incorrectly hashing dids). Does anybody have a sense of whether a u64 is enough space to reasonably avoid collisions? It seems fairly small ...

@nikomatsakis Can you look at whether the region types are handled correctly here?

@nikomatsakis
Copy link
Contributor

This is an interesting idea. Can this replace all uses of type descriptors? Put another way: if all we care about is identity, I guess it can.

@nikomatsakis
Copy link
Contributor

It occurs to me that, since regions are erased from the hashed types, the methods like is() on Any are unsound unless the type T has the bound 'static. Also, we'll have to be awfully careful if we still do any monomorphic collapsing (i.e., the old type_use) -- was that removed?

@thestinger
Copy link
Contributor

@nikomatsakis: I think trans pretends all lifetime parameters are 'static.

@thestinger
Copy link
Contributor

This generates a single monomorphized instance:

fn foo<'a>(x: &'a int) -> &'a int {
    x
}

static FOO: int = 5;

fn main() {
    foo(&FOO);
    let x = 10;
    foo(&x);
}

@nikomatsakis
Copy link
Contributor

@thestinger this is my point.

@alexcrichton I realize I didn't go far enough. I think that the "type id" intrinsic should only work for T:'static

@alexcrichton
Copy link
Member Author

Hm, this could get interesting. So in theory the signature of the type_id intrinsic is fn type_id<T: 'static>() -> u64, but this means that callers need to have a 'static bound, which Any currently does not, and apparently you can't make trait Foo: 'static.

This also seems a little limiting to the Any trait to make it only applicable for types which don't have any borrowed pointers inside of them. It looks like the types that we're hashing aren't necessarily the "trans types" in that they've had all their regions substituted, so why can't we hash the regions as well? You could have some possibly equivalent types have different types, but I think that it's fine to have that happen (especially when lots of weird regions are involved).

@thestinger
Copy link
Contributor

The monomorphize pass isn't going to output new functions when the region parameters are different. It will generate incorrect code if you're giving runtime meaning to regions. If a function calls type_id, it's only going to output one version of it for any set of type parameters, regardless of the regions involved.

@glaebhoerl
Copy link
Contributor

@brson GHC uses [u64, ..2]

Now that the type_id intrinsic is working across crates, all of these
unnecessary messages can be removed to have the failure type for a task truly be
~Any and only ~Any
bors added a commit that referenced this pull request Nov 4, 2013
…akis

This isn't quite as fancy as the struct in #9913, but I'm not sure we should be exposing crate names/hashes of the types. That being said, it'd be pretty easy to extend this (the deterministic hashing regardless of what crate you're in was the hard part).
bors added a commit that referenced this pull request Nov 5, 2013
…akis

This isn't quite as fancy as the struct in #9913, but I'm not sure we should be exposing crate names/hashes of the types. That being said, it'd be pretty easy to extend this (the deterministic hashing regardless of what crate you're in was the hard part).
@bors bors merged commit b004493 into rust-lang:master Nov 5, 2013
@alexcrichton alexcrichton deleted the typeid-intrinsic branch November 5, 2013 05:08
@DaGenix
Copy link

DaGenix commented Nov 9, 2013

Due to SipHash having such a small output size, I think collisions are a concern. I believe I found a theoretical collision: https://gist.github.com/DaGenix/7390603. Collisions are fairly unlikely unless you are specifically looking for one, but, if collisions can be found they can also happen by accident. Is this an problem? I didn't want to open up an issue until someone that understands the impacts of such a collision better than I do took a look and I wasn't sure where else to post.

@thestinger
Copy link
Contributor

Yes, it's a problem. If there are collisions and the library assumes there aren't, Rust isn't memory safe.

@DaGenix DaGenix mentioned this pull request Nov 9, 2013
@DaGenix
Copy link

DaGenix commented Nov 9, 2013

I opened #10389.

flip1995 pushed a commit to flip1995/rust that referenced this pull request Jan 27, 2023
[needless_return]: Remove all semicolons on suggestion

Closes rust-lang#10182

Multiple semicolons currently breaks autofix for `needless_return` suggestions. Any semicolons left after removing return means that the return type will always be `()`, and thus fail to compile.

This PR allows `needless_return` to remove multiple semicolons.

The change won't cover the case where there is multiple line yet.

i.e.

```rust
fn needless_return() -> bool {
    return true;
;;
}
```

---

changelog: Sugg: [`needless_return`]: Now removes all semicolons on the same line
[rust-lang#10187](rust-lang/rust-clippy#10187)
<!-- changelog_checked -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants