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

comment out FIXMEs to not display them on UI #2186

Merged
merged 2 commits into from
Jan 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/method-lookup.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ imported to use an inherent method, they are associated with the type
itself (note that inherent impls can only be defined in the same
crate as the type itself).

<!--
FIXME: Inherent candidates are not always derived from impls. If you
have a trait object, such as a value of type `Box<ToString>`, then the
trait methods (`to_string()`, in this case) are inherently associated
Expand All @@ -76,7 +77,8 @@ to change: when DST's "impl Trait for Trait" is complete, trait object
dispatch could be subsumed into trait matching, and the type parameter
behavior should be reconsidered in light of where clauses.

TODO: Is this FIXME still accurate?
Is this still accurate?
-->

**Extension candidates** are derived from imported traits. If I have
the trait `ToString` imported, and I call `to_string()` as a method,
Expand Down
7 changes: 7 additions & 0 deletions src/solve/normalization.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# Normalization in the new solver

> **NOTE**: FIXME: The content of this chapter has some been changed quite
significantly since it was written.

With the new solver we've made some fairly significant changes to normalization when compared
to the existing implementation.

Expand Down Expand Up @@ -52,12 +55,14 @@ before assigning the resulting rigid type to an inference variable. This is used
This has to be used whenever we match on the value of some type, both inside
and outside of the trait solver.

<!--
FIXME: structure, maybe we should have an "alias handling" chapter instead as
talking about normalization without explaining that doesn't make too much
sense.

FIXME: it is likely that this will subtly change again by mostly moving structural
normalization into `NormalizesTo`.
-->
Comment on lines 63 to +65
Copy link
Member

Choose a reason for hiding this comment

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

normalization has changed pretty significantly since this chapter was first written iirc, imo we should have a big note at the top of the page saying that things have likely diverged from what is documented here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, done that.

image


[structural_norm]: https://github.com/rust-lang/rust/blob/2627e9f3012a97d3136b3e11bf6bd0853c38a534/compiler/rustc_trait_selection/src/solve/alias_relate.rs#L140-L175
[structural-relate]: https://github.com/rust-lang/rust/blob/a0569fa8f91b5271e92d2f73fd252de7d3d05b9c/compiler/rustc_trait_selection/src/solve/alias_relate.rs#L88-L107
Expand All @@ -72,11 +77,13 @@ possible. However, this only works for aliases referencing bound variables if th
not ambiguous as we're unable to replace the alias with a corresponding inference
variable without leaking universes.

<!--
FIXME: we previously had to also be careful about instantiating the new inference
variable with another normalizeable alias. Due to our recent changes to generalization,
this should not be the case anymore. Equating an inference variable with an alias
now always uses `AliasRelate` to fully normalize the alias before instantiating the
inference variable: [source][generalize-no-alias]
-->

[generalize-no-alias]: https://github.com/rust-lang/rust/blob/a0569fa8f91b5271e92d2f73fd252de7d3d05b9c/compiler/rustc_infer/src/infer/relate/generalize.rs#L353-L358

Expand Down
2 changes: 2 additions & 0 deletions src/solve/opaque-types.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ Finally, we check whether the item bounds of the opaque hold for the expected ty
[eq-prev]: https://github.com/rust-lang/rust/blob/384d26fc7e3bdd7687cc17b2662b091f6017ec2a/compiler/rustc_trait_selection/src/solve/normalizes_to/opaque_types.rs#L51-L59
[insert-storage]: https://github.com/rust-lang/rust/blob/384d26fc7e3bdd7687cc17b2662b091f6017ec2a/compiler/rustc_trait_selection/src/solve/normalizes_to/opaque_types.rs#L68
[item-bounds-ck]: https://github.com/rust-lang/rust/blob/384d26fc7e3bdd7687cc17b2662b091f6017ec2a/compiler/rustc_trait_selection/src/solve/normalizes_to/opaque_types.rs#L69-L74

[^1]: FIXME: this should ideally only result in a unique candidate given that we require the args to be placeholders and regions are always inference vars
[^2]: FIXME: why do we check whether the expected type is rigid for this.

Expand Down Expand Up @@ -101,6 +102,7 @@ The handling of member constraints does not change in the new solver. See the

FIXME: We need to continue to support calling methods on still unconstrained
opaque types in their defining scope. It's unclear how to best do this.

```rust
use std::future::Future;
use futures::FutureExt;
Expand Down
13 changes: 8 additions & 5 deletions src/tests/directives.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

<!-- toc -->

> **FIXME(jieyouxu)** completely revise this chapter.
<!--
FIXME(jieyouxu) completely revise this chapter.
Copy link
Member

Choose a reason for hiding this comment

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

FTR, this was intentional and I think it's more accurate to display this. But maybe elaborate with:

  • This chapter is quite outdated at various places.
  • The organization is confusing.
  • It's potentially better to have per-directive doc comments then backlink to them for details, instead of having them separately listed here.

Copy link
Member Author

Choose a reason for hiding this comment

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

It even includes a specific username to be resolved from. Why should it appear on the UI? Maybe I don't understand, what value does it bring to that page?

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense, I opened #2187 to track this instead.

-->

Directives are special comments that tell compiletest how to build and interpret
a test. They must appear before the Rust source in the test. They may also
Expand Down Expand Up @@ -248,10 +250,11 @@ Consider writing the test as a proper incremental test instead.
|-------------|--------------------------------------------------------------|------------------------------------------|---------------------------|
| `doc-flags` | Flags passed to `rustdoc` when building the test or aux file | `rustdoc`, `js-doc-test`, `rustdoc-json` | Any valid `rustdoc` flags |

> **FIXME(rustdoc)**: what does `check-test-line-numbers-match` do?
>
> Asked in
> <https://rust-lang.zulipchat.com/#narrow/stream/266220-t-rustdoc/topic/What.20is.20the.20.60check-test-line-numbers-match.60.20directive.3F>.
<!--
**FIXME(rustdoc)**: what does `check-test-line-numbers-match` do?
Asked in
<https://rust-lang.zulipchat.com/#narrow/stream/266220-t-rustdoc/topic/What.20is.20the.20.60check-test-line-numbers-match.60.20directive.3F>.
-->

### Pretty printing

Expand Down
Loading