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

Rebase const-generics branch #1

Closed
varkor opened this issue Feb 19, 2018 · 4 comments
Closed

Rebase const-generics branch #1

varkor opened this issue Feb 19, 2018 · 4 comments

Comments

@varkor
Copy link
Owner

varkor commented Feb 19, 2018

@yodaldevoid: I'd like to rebase the const-generics branch on to master to fix up the few merge conflicts that have arisen, and also avoid having to rebuild the new version of LLVM after switching between const-generics and other branches each time. I was originally going to wait until rust-lang#48149 had been merged, but as that'll probably take a little while longer, I'd rather do it first now.

Can I force push a rebased version of the branch, or will that cause issues for you at the moment?

@yodaldevoid
Copy link
Collaborator

Feel free to go ahead. I probably won't be able to work on this for a few weeks as I am currently swamped by school work.

@varkor
Copy link
Owner Author

varkor commented Feb 27, 2018

I've been holding off on making more changes to const-generics until the two other refactoring PRs get merged. After that, though, there are only a few TODOs left — though I ran into some compilation errors when trying to compile the examples, so there are definitely still some rough patches that need to be looked into before we have something that works.

varkor pushed a commit that referenced this issue Apr 12, 2018
Building for x86_64-unknown-linux-musl currently results in an executable lacking debug information for musl libc itself. If you request a backtrace in GDB while control flow is within musl – including sycalls made by musl – the result looks like:

#0  0x0000000000434b46 in __cp_end ()
#1  0x0000000000432dbd in __syscall_cp_c ()
#2  0x0000000000000000 in ?? ()

i.e. not very helpful. Adding --enable-debug resolves this, and --enable-optimize re-enables optimisations which default to off given the previous flag.
varkor pushed a commit that referenced this issue Apr 12, 2018
Add --enable-debug flag to musl CI build script

Building for x86_64-unknown-linux-musl currently results in an executable lacking debug information for musl libc itself. If you request a backtrace in GDB while control flow is within musl – including sycalls made by musl – the result looks like:

```
#0  0x0000000000434b46 in __cp_end ()
#1  0x0000000000432dbd in __syscall_cp_c ()
#2  0x0000000000000000 in ?? ()
```

i.e. not very helpful. Adding --enable-debug resolves this, and --enable-optimize re-enables optimisations which default to off given the previous flag.
varkor pushed a commit that referenced this issue Apr 18, 2018
@varkor
Copy link
Owner Author

varkor commented Apr 21, 2018

@yodaldevoid: just thought it'd be good to update you on where I am with this (though you've probably noticed the lack of movement) — I've been busy and I'm not going to have time for the next month and a half to work on const generics, other than continuing to push the refactoring PRs through. In about 6 weeks, though, I should have a lot more time and plan to start working hard!

Just from some of the rebasing I did after miri: there's quite a lot that's changed, both for better (some of our // TODOs are irrelevant now) and for the worse (fixing everything) — and this will be even more true after the two generics PRs get merged, so it might even be more convenient overall this way.

I'm not sure when the lazy normalisation problems will hit (it seems there are at least some uses of const generics that should be fine), but integration with Chalk is already beginning to start, so it might even line up well with our branch. I'm optimistic we could be starting to merge things in by the end of summer...

@yodaldevoid
Copy link
Collaborator

I've been keeping an eye on the refactoring PRs. I should finally have time to work on this for long enough to move it forward around the same time that the first one goes through.

varkor pushed a commit that referenced this issue May 25, 2018
There is a hot path through `opt_normalize_projection_type`:
- `try_start` does a cache lookup (#1).
- The result is a `NormalizedTy`.
- There are no unresolved type vars, so we call `complete`.
- `complete` does *another* cache lookup (#2), then calls
  `SnapshotMap::insert`.
- `insert` does *another* cache lookup (rust-lang#3), inserting the same value
  that's already in the cache.

This patch optimizes this hot path by introducing `complete_normalized`,
for use when the value is known in advance to be a `NormalizedTy`. It
always avoids lookup #2. Furthermore, if the `NormalizedTy`'s
obligations are empty (the common case), we know that lookup rust-lang#3 would be
a no-op, so we avoid it, while inserting a Noop into the `SnapshotMap`'s
undo log.
varkor pushed a commit that referenced this issue Jun 3, 2018
When encountering an unexisting method for a given trait where an
associated function has the same name, suggest using the appropriate
syntax, instead of using `help` text.

When only one candidate is found, do not call it "candidate #1", just
call it "the candidate".
varkor pushed a commit that referenced this issue Jun 3, 2018
Tweak output on E0599 for assoc fn used as method

 - Use suggestion instead of `help` when possible
 - Add primary span label
 - Remove incorrect `help` suggestion using incorrect syntax
 - Do not refer to only one possible candidate as `candidate #1`, refer to it as `the candidate`
varkor pushed a commit that referenced this issue Jun 29, 2018
Suggestion for 'static impl Trait return

When encountering a named or anonymous sup requirement (for example,
`&'a self`) and a `'static` impl Trait return type, suggest adding the
`'_` lifetime constraing to the return type.

Fix rust-lang#43719, rust-lang#51282.

```
error: cannot infer an appropriate lifetime
  --> $DIR/static-return-lifetime-infered.rs:17:16
   |
LL |     fn iter_values_anon(&self) -> impl Iterator<Item=u32> {
   |                                   ----------------------- this return type evaluates to the `'static` lifetime...
LL |         self.x.iter().map(|a| a.0)
   |         ------ ^^^^
   |         |
   |         ...but this borrow...
   |
note: ...can't outlive the anonymous lifetime #1 defined on the method body at 16:5
  --> $DIR/static-return-lifetime-infered.rs:16:5
   |
LL | /     fn iter_values_anon(&self) -> impl Iterator<Item=u32> {
LL | |         self.x.iter().map(|a| a.0)
LL | |     }
   | |_____^
help: you can add a constraint to the return type to make it last less than `'static` and match the anonymous lifetime #1 defined on the method body at 16:5
   |
LL |     fn iter_values_anon(&self) -> impl Iterator<Item=u32> + '_ {
   |                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```
varkor pushed a commit that referenced this issue Aug 3, 2018
…-box, r=eddyb

[NLL] Dangly paths for box

Special-case `Box` in `rustc_mir::borrow_check`.

Since we know dropping a box will not access any `&mut` or `&` references, it is safe to model its destructor as only touching the contents *owned* by the box.

----

There are three main things going on here:

1. The first main thing, this PR is fixing a bug in NLL where `rustc` previously would issue a diagnostic error in a case like this:
```rust
fn foo(x: Box<&mut i32>) -> &mut i32 { &mut **x }
```

such code was accepted by the AST-borrowck in the past, but NLL was rejecting it with the following message ([playground](https://play.rust-lang.org/?gist=13c5560f73bfb16d6dab3ceaad44c0f8&version=nightly&mode=release&edition=2015))
```
error[E0597]: `**x` does not live long enough
 --> src/main.rs:3:40
  |
3 | fn foo(x: Box<&mut i32>) -> &mut i32 { &mut **x }
  |                                        ^^^^^^^^ - `**x` dropped here while still borrowed
  |                                        |
  |                                        borrowed value does not live long enough
  |
note: borrowed value must be valid for the anonymous lifetime #1 defined on the function body at 3:1...
 --> src/main.rs:3:1
  |
3 | fn foo(x: Box<&mut i32>) -> &mut i32 { &mut **x }
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error
```

2. The second main thing: The reason such code was previously rejected was because NLL (MIR-borrowck) incorporates a fix for issue rust-lang#31567, where it models a destructor's execution as potentially accessing any borrows held by the thing being destructed. The tests with `Scribble` model this, showing that the compiler now catches such unsoundness.

However, that fix for issue rust-lang#31567 is too strong, in that NLL (MIR-borrowck) includes `Box` as one of the types with a destructor that potentially accesses any borrows held by the box. This thus was the cause of the main remaining discrepancy between AST-borrowck and MIR-borrowck, as documented in issue rust-lang#45696, specifically in [the last example of this comment](rust-lang#45696 (comment)), which I have adapted into the `fn foo` shown above.

We did close issue rust-lang#45696 back in December of 2017, but AFAICT that example was not fixed by PR rust-lang#46268. (And we did not include a test, etc etc.)

This PR fixes that case, by trying to model the so-called `DerefPure` semantics of `Box<T>` when we traverse the type of the input to `visit_terminator_drop`.

3. The third main thing is that during a review of the first draft of this PR, @matthewjasper pointed out that the new traversal of `Box<T>` could cause the compiler to infinite loop. I have adjusted the PR to avoid this (by tracking what types we have previously seen), and added a much needed test of this somewhat odd scenario. (Its an odd scenario because the particular case only arises for things like `struct A(Box<A>);`, something which cannot be constructed in practice.)

Fix rust-lang#45696.
varkor pushed a commit that referenced this issue Sep 25, 2018
varkor pushed a commit that referenced this issue Nov 3, 2018
sync fork with upstream (master)
varkor pushed a commit that referenced this issue Jan 13, 2019
@varkor varkor closed this as completed Jan 13, 2019
varkor pushed a commit that referenced this issue Apr 30, 2019
varkor pushed a commit that referenced this issue Jul 22, 2019
varkor pushed a commit that referenced this issue Sep 6, 2019
varkor pushed a commit that referenced this issue Sep 6, 2019
varkor pushed a commit that referenced this issue Sep 15, 2020
* Fix `const-display.rs` XPATH queries

* Add `issue_76501.rs` test file

* Rename issue_76501.rs to issue-76501.rs
varkor pushed a commit that referenced this issue Nov 22, 2020
Before:

```
2:rustc INFO rustc_interface::passes Pre-codegen
2:rustcTy interner             total           ty lt ct all
2:rustc    Adt               :   1078 81.3%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Array             :      1  0.1%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Slice             :      1  0.1%,  0.0%   0.0%  0.0%  0.0%
2:rustc    RawPtr            :      2  0.2%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Ref               :      4  0.3%,  0.1%   0.1%  0.0%  0.0%
2:rustc    FnDef             :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc    FnPtr             :     76  5.7%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Placeholder       :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Generator         :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc    GeneratorWitness  :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Dynamic           :      3  0.2%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Closure           :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Tuple             :     13  1.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Bound             :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Param             :    146 11.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Infer             :      2  0.2%,  0.1%   0.0%  0.0%  0.0%
2:rustc    Projection        :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Opaque            :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Foreign           :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc                  total   1326         0.2%   0.1%  0.0%  0.0%
2:rustcInternalSubsts interner: rust-lang#437
2:rustcRegion interner: rust-lang#355
2:rustcStability interner: #1
2:rustcConst Stability interner: #0
2:rustcAllocation interner: #0
2:rustcLayout interner: #0
```

After:

```
 INFO rustc_interface::passes Post-codegen
Ty interner             total           ty lt ct all
    Adt               :   1078 81.3%,  0.0%   0.0%  0.0%  0.0%
    Array             :      1  0.1%,  0.0%   0.0%  0.0%  0.0%
    Slice             :      1  0.1%,  0.0%   0.0%  0.0%  0.0%
    RawPtr            :      2  0.2%,  0.0%   0.0%  0.0%  0.0%
    Ref               :      4  0.3%,  0.1%   0.1%  0.0%  0.0%
    FnDef             :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
    FnPtr             :     76  5.7%,  0.0%   0.0%  0.0%  0.0%
    Placeholder       :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
    Generator         :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
    GeneratorWitness  :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
    Dynamic           :      3  0.2%,  0.0%   0.0%  0.0%  0.0%
    Closure           :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
    Tuple             :     13  1.0%,  0.0%   0.0%  0.0%  0.0%
    Bound             :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
    Param             :    146 11.0%,  0.0%   0.0%  0.0%  0.0%
    Infer             :      2  0.2%,  0.1%   0.0%  0.0%  0.0%
    Projection        :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
    Opaque            :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
    Foreign           :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
                  total   1326         0.2%   0.1%  0.0%  0.0%
InternalSubsts interner: rust-lang#437
Region interner: rust-lang#355
Stability interner: #1
Const Stability interner: #0
Allocation interner: #0
Layout interner: #0
```
varkor pushed a commit that referenced this issue Nov 22, 2020
Don't print thread ids and names in `tracing` logs

Before:

```
2:rustc INFO rustc_interface::passes Pre-codegen
2:rustcTy interner             total           ty lt ct all
2:rustc    Adt               :   1078 81.3%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Array             :      1  0.1%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Slice             :      1  0.1%,  0.0%   0.0%  0.0%  0.0%
2:rustc    RawPtr            :      2  0.2%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Ref               :      4  0.3%,  0.1%   0.1%  0.0%  0.0%
2:rustc    FnDef             :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc    FnPtr             :     76  5.7%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Placeholder       :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Generator         :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc    GeneratorWitness  :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Dynamic           :      3  0.2%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Closure           :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Tuple             :     13  1.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Bound             :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Param             :    146 11.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Infer             :      2  0.2%,  0.1%   0.0%  0.0%  0.0%
2:rustc    Projection        :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Opaque            :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc    Foreign           :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
2:rustc                  total   1326         0.2%   0.1%  0.0%  0.0%
2:rustcInternalSubsts interner: rust-lang#437
2:rustcRegion interner: rust-lang#355
2:rustcStability interner: #1
2:rustcConst Stability interner: #0
2:rustcAllocation interner: #0
2:rustcLayout interner: #0
```

After:

```
 INFO rustc_interface::passes Post-codegen
Ty interner             total           ty lt ct all
    Adt               :   1078 81.3%,  0.0%   0.0%  0.0%  0.0%
    Array             :      1  0.1%,  0.0%   0.0%  0.0%  0.0%
    Slice             :      1  0.1%,  0.0%   0.0%  0.0%  0.0%
    RawPtr            :      2  0.2%,  0.0%   0.0%  0.0%  0.0%
    Ref               :      4  0.3%,  0.1%   0.1%  0.0%  0.0%
    FnDef             :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
    FnPtr             :     76  5.7%,  0.0%   0.0%  0.0%  0.0%
    Placeholder       :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
    Generator         :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
    GeneratorWitness  :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
    Dynamic           :      3  0.2%,  0.0%   0.0%  0.0%  0.0%
    Closure           :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
    Tuple             :     13  1.0%,  0.0%   0.0%  0.0%  0.0%
    Bound             :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
    Param             :    146 11.0%,  0.0%   0.0%  0.0%  0.0%
    Infer             :      2  0.2%,  0.1%   0.0%  0.0%  0.0%
    Projection        :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
    Opaque            :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
    Foreign           :      0  0.0%,  0.0%   0.0%  0.0%  0.0%
                  total   1326         0.2%   0.1%  0.0%  0.0%
InternalSubsts interner: rust-lang#437
Region interner: rust-lang#355
Stability interner: #1
Const Stability interner: #0
Allocation interner: #0
Layout interner: #0
```

Closes rust-lang#78931
r? ``@oli-obk``
varkor pushed a commit that referenced this issue Dec 30, 2020
```
Benchmark #1: ./raytracer_cg_clif_pre
  Time (mean ± σ):      9.553 s ±  0.129 s    [User: 9.543 s, System: 0.008 s]
  Range (min … max):    9.438 s …  9.837 s    10 runs

Benchmark #2: ./raytracer_cg_clif_post
  Time (mean ± σ):      9.463 s ±  0.055 s    [User: 9.452 s, System: 0.008 s]
  Range (min … max):    9.387 s …  9.518 s    10 runs

Summary
  './raytracer_cg_clif_post' ran
    1.01 ± 0.01 times faster than './raytracer_cg_clif_pre'
```
varkor pushed a commit that referenced this issue Dec 30, 2020
Don't run `resolve_vars_if_possible` in `normalize_erasing_regions`

Neither `@eddyb` nor I could figure out what this was for. I changed it to `assert_eq!(normalized_value, infcx.resolve_vars_if_possible(&normalized_value));` and it passed the UI test suite.

<details><summary>

Outdated, I figured out the issue - `needs_infer()` needs to come _after_ erasing the lifetimes

</summary>

Strangely, if I change it to `assert!(!normalized_value.needs_infer())` it panics almost immediately:

```
query stack during panic:
#0 [normalize_generic_arg_after_erasing_regions] normalizing `<str::IsWhitespace as str::pattern::Pattern>::Searcher`
#1 [needs_drop_raw] computing whether `str::iter::Split<str::IsWhitespace>` needs drop
#2 [mir_built] building MIR for `str::<impl str>::split_whitespace`
rust-lang#3 [unsafety_check_result] unsafety-checking `str::<impl str>::split_whitespace`
rust-lang#4 [mir_const] processing MIR for `str::<impl str>::split_whitespace`
rust-lang#5 [mir_promoted] processing `str::<impl str>::split_whitespace`
rust-lang#6 [mir_borrowck] borrow-checking `str::<impl str>::split_whitespace`
rust-lang#7 [analysis] running analysis passes on this crate
end of query stack
```

I'm not entirely sure what's going on - maybe the two disagree?

</details>

For context, this came up while reviewing rust-lang#77467 (cc `@lcnr).`

Possibly this needs a crater run?

r? `@nikomatsakis`
cc `@matthewjasper`
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

No branches or pull requests

2 participants