Skip to content

Don't ICE when dealing with the count expr for fixed array types in various places. #15804

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

Don't ICE when dealing with the count expr for fixed array types in various places. #15804

wants to merge 3 commits into from

Conversation

luqmana
Copy link
Member

@luqmana luqmana commented Jul 19, 2014

The fact that TyFixedLengthVec contains an Expr seems to have been overlooked in a couple of places causes ICE's to pop up (especially obvious in pretty=typed)

Previously this would also ICE:

let _x: [u16, ..four!()];

where four!() just expanded to the literal 4 since we never consider that expression when expanded macros.

Fixes #15801 (in that it no longer ICE's and just outputs an error instead)
Fixes #4264

r? @jbclements re the macro expansion changes
r? @pcwalton re typeck changes

@lilyball
Copy link
Contributor

Yesterday I was trying to solve this exact problem (because of --pretty typed) but didn't get far enough to figure out the solution (I had never looked at this code before). So nice timing :) but anyway, I have a test case with a lot of different ways to use a fixed-length vec for testing --pretty typed. I'll give it to you tomorrow if you want to add it to this.

@lilyball
Copy link
Contributor

Turns out this does not in fact fix my --pretty typed issue. I thought it would, since you're folding over types and recording the expressions.

@lilyball
Copy link
Contributor

To clarify, it fixes some of my --pretty typed issues with vectors. But not all. Notably, it's not handling fixed-length vectors in the following locations:

  • Function argument type
  • format!("test") invocation
  • type declaration
  • fields of a struct declaration (both record-struct and tuple-struct)
  • enum variants

@lilyball
Copy link
Contributor

The test I'm using for this can be found at lilyball/rust@23007912e2a. I would love it if you could extend this PR to cover the missing cases so --pretty typed works again. The relevant issue is #4264.

@sfackler
Copy link
Member

Test failures: https://travis-ci.org/rust-lang/rust/jobs/30330452

@lilyball
Copy link
Contributor

Yeah this code is asserting that the type is uint, but astconv::ast_ty_to_ty() actually allows for it to be uint or int.

@pcwalton
Copy link
Contributor

Looks good.

@lilyball
Copy link
Contributor

@luqmana Any chance you can get --pretty typed working (see my previous comment)? If you don't, I'l try taking a stab at it again, but I don't know how this code works.

@luqmana
Copy link
Member Author

luqmana commented Jul 22, 2014

@kballard I did, i thought I had pinged you on irc. Maybe not :P

@lilyball
Copy link
Contributor

@luqmana You did? Awesome. Can you include the test case I linked to?

@luqmana
Copy link
Member Author

luqmana commented Jul 23, 2014

@kballard done.

@huonw huonw mentioned this pull request Sep 10, 2014
Closed
lnicola pushed a commit to lnicola/rust that referenced this pull request Jan 3, 2024
@bors
Auto merge of rust-lang#16112 - roife:rewrite-generate-delete-trait, …
34df296 
…r=Veykril

fix: rewrite code_action `generate_delegate_trait`

I've made substantial enhancements to the "generate delegate trait" code action in rust-analyzer. Here's a summary of the changes:

#### Resolved the "Can’t find [email protected] in AstIdMap" error

Fix rust-lang#15804, fix rust-lang#15968, fix rust-lang#15108

The issue stemmed from an incorrect application of PathTransform in the original code. Previously, a new 'impl' was generated first and then transformed, causing PathTransform to fail in locating the correct AST node, resulting in an error. I rectified this by performing the transformation before generating the new 'impl' (using make::impl_trait), ensuring a step-by-step transformation of associated items.

#### Rectified generation of `Self` type

`generate_delegate_trait` is unable to properly handle trait with `Self` type.

Let's take the following code as an example:

```rust
trait Trait {
    fn f() -> Self;
}

struct B {}
impl Trait for B {
    fn f() -> B { B{} }
}

struct S {
    b: B,
}
```

Here, if we implement `Trait` for `S`, the type of `f` should be `() -> Self`, i.e. `() -> S`. However we cannot automatically generate a function that constructs `S`.

To ensure that the code action doesn't generate delegate traits for traits with Self types, I add a function named `has_self_type` to handle it.

#### Extended support for generics in structs and fields within this code action

The former version of `generate_delegate_trait` cannot handle structs with generics properly. Here's an example:

```rust
struct B<T> {
    a: T
}

trait Trait<T> {
    fn f(a: T);
}

impl<T1, T2> Trait<T1> for B<T2> {
    fn f(a: T1) -> T2 { self.a }
}

struct A {}
struct S {
    b$0 : B<A>,
}
```

The former version  will generates improper code:

```rust
impl<T1, T2> Trait<T1, T2> for S {
    fn f(&self, a: T1) -> T1 {
        <B as Trait<T1, T2>>::f( &self.b , a)
    }
}
```

The rewritten version can handle generics properly:

```rust
impl<T1> Trait<T1> for S {
    fn f(&self, a: T1) -> T1 {
        <B<A> as Trait<T1>>::f(&self.b, a)
    }
}
```

See more examples in added unit tests.

I enabled support for generic structs in `generate_delegate_trait` through the following steps (using the code example provided):

1. Initially, to prevent conflicts between the generic parameters in struct `S` and the ones in the impl of `B`, I renamed the generic parameters of `S`.
2. Then, since `B`'s parameters are instantiated within `S`, the original generic parameters of `B` needed removal within `S` (to avoid errors from redundant parameters). An important consideration here arises when Trait and B share parameters in `B`'s impl. In such cases, these shared generic parameters cannot be removed.
3. Next, I addressed the matching of types between `B`'s type in `S` and its type in the impl. Given that some generic parameters in the impl are instantiated in `B`, I replaced these parameters with their instantiated results using PathTransform. For instance, in the example provided, matching `B<A>` and `B<T2>`, where `T2` is instantiated as `A`, I replaced all occurrences of `T2` in the impl with `A` (i.e. apply the instantiated generic arguments to the params).
4. Finally, I performed transformations on each assoc item (also to prevent the initial issue) and handled redundant where clauses.

For a more detailed explanation, please refer to the code and comments. I welcome suggestions and any further questions!
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 15, 2024
@bors
Auto merge of rust-lang#16277 - roife:fix-issue16276, r=Veykril
51ac6de 
Resolve panic in `generate_delegate_methods`

Fixes rust-lang#16276

This PR addresses two issues:

1. When using `PathTransform`, it searches for the node corresponding to the `path` in the `source_scope` during `make::fn_`. Therefore, we need to perform the transform before `make::fn_` (similar to the problem in issue rust-lang#15804). Otherwise, even though the tokens are the same, their offsets (i.e., `span`) differ, resulting in the error "Can't find CONST_ARG@xxx."

2. As mentioned in the first point, `PathTransform` searches for the node corresponding to the `path` in the `source_scope`. Thus, when transforming paths, we should update nodes from right to left (i.e., use **reverse of preorder** (right -> left -> root) instead of **postorder** (left -> right -> root)). Reasons are as follows:

    In the red-green tree (rowan), we do not store absolute ranges but instead store the length of each node and dynamically calculate offsets (spans). Therefore, when modifying the left-side node (such as nodes are inserted or deleted), it causes all right-side nodes' spans to change. This, in turn, leads to PathTransform being unable to find nodes with the same paths (due to different spans), resulting in errors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ICE when transmuting into a static 'rustc --pretty typed' fails for hello world
6 participants
@luqmana @lilyball @sfackler @pcwalton @alexcrichton @bors