Skip to content

Auto-borrow trait types - #4178

Closed
catamorphism wants to merge 1 commit into
rust-lang:incomingfrom
catamorphism:issue-3794
Closed

Auto-borrow trait types#4178
catamorphism wants to merge 1 commit into
rust-lang:incomingfrom
catamorphism:issue-3794

Conversation

@catamorphism

Copy link
Copy Markdown
Contributor

r? @nikomatsakis - Allow @A to be used where &A is expected, if A is a trait. As per
issue #3794

Allow @A to be used where &A is expected, if A is a trait. As per
issue 3794
@catamorphism

Copy link
Copy Markdown
ContributorAuthor

Sorry, I think when I re-pushed it wiped @nikomatsakis 's comments. In answer to:

"this... doesn't seem right to me...maybe I am confused, but I thought that an object was represented as the pair of a vtable and data pointer. I'd expect that you want to preserve this vtable. But then your test does work, presumably, so maybe I'm confused.. or maybe the test is bogus. Can you modify the test somewhat to have multiple fields and assert that they have their expected values, rather than just printing them out?"

I'll try modifying the test.

"I think we should call this AutoBorrowObject. I've been trying to use the term "object" to refer to instances of a trait type."

I'll do this.

"this seems like a bug in our casting logic, because a ~Trait really ought to be unique. that said, I think that the plan was to move towards auto-coercing as the means of constructing objects and not casts, so maybe this is a moot point."

Ok, sounds like it's fine to leave this as it is for now?

@nikomatsakis

Copy link
Copy Markdown
Contributor

r- pending further tests.

In addition to the comments above, there was this one regarding borrow check:

I don't think this is quite correct. The problem is that, unlike AutoRef, the "AutoBorrowTrait" kind of combines a deref and a borrow into one operation. So, if you have a variable x: ~T that is "autoborrowed" into &T, we want to say that the data found at *x---that is, the data that x points at---will remain valid. But here we are only guaranteeing that x remains valid, not necessarily the data it points at.

Can you add a test like:

struct X { x: uint }
trait Foo {
fn foo(&self) -> &self/uint;
}
impl X: Foo {
fn foo(&self) -> &self/uint { &self.x }
}
fn main() {
let x0 = ~X { x: 3 };
let x1 = ~X { x: 3 };
let y = (move x0) as ~Foo;
let z = y.foo();
y = (move x1) as ~Foo; // Should result in an error because we are modifying `y`, which is still borrowed
io::println(*z);
}

Fixing this will take a bit of work, though, because *x isn't a legal rust expression. So we have to extend the categorization structure, I think, to be able to represent it... actually the structure is probably ok, it's just that we have to extend the code for handling derefs to recognize that there are derefable things that are not permitted by the * operator. We already have this problem to some extent, I think we have addressed it before by adding a special method for handling these sorts of special-case derefs.

@catamorphism

Copy link
Copy Markdown
ContributorAuthor

I need to put more work into this, so closing it for now.

calebcartwright pushed a commit to calebcartwright/rust that referenced this pull request Oct 20, 2021
RalfJung added a commit to RalfJung/rust that referenced this pull request Feb 24, 2025
U007D pushed a commit to U007D/rust-mos that referenced this pull request Aug 21, 2026
4178: Validate the location of `crate` in paths r=matklad a=djrenren
**This solution does not fully handle `use` statements. See below**
This pull requests implements simple validation of usages of the `crate` keyword in `Path`s. Specifically it validates that:
- If a `PathSegment` is starts with the `crate` keyword, it is also the first segment of the `Path`
- All other usages of `crate` in `Path`s are considered errors.
This aligns with `rustc`'s rules. Unlike rustc this implementation does not issue a special error message in the case of `::crate` but it does catch the error.
Furthermore, this change does not cover all error cases. Specifically the following is not caught:
```rust
use foo::{crate}
```
This is because this check is context sensitive. From an AST perspective, `crate` is the root of the `Path`. Only by inspecting the full `UseItem` do we see that it is not in fact the root. This problem becomes worse because `UseTree`s are allowed to be arbitrarily nested:
```rust
use {crate, {{crate, foo::{crate}}}
```
So this is a hard problem to solve without essentially a breadth-first search. In a traditional compiler, I'd say this error is most easily found during the AST -> HIR conversion pass but within rust-analyzer I'm not sure where it belongs. Under the implementation in this PR, such errors are ignored so we're *more correct* just not *entirely correct*. Co-authored-by: John Renner <john@jrenner.net>
Sign up for freeto 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.

2 participants

@catamorphism@nikomatsakis