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.

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

Store Ident in DefPathData instead of Symbol #69906

Closed
wants to merge 8 commits into from
Closed

Store Ident in DefPathData instead of Symbol #69906

wants to merge 8 commits into from

Commits on Mar 17, 2020

  1. Store Ident in DefPathData instead of Symbol 

    This allows us to recover span information when emitting cross crate
errors. A number of test cases benefit from this change, since we were
previously not emitting messages due to invalid spans.

There are four main parts to this commit:

1. Adding `Ident` to `DefPathData`, and updating the affected code. This
mostly consists of mechanical changes.

2. Updating how we determine the disambiguator for a `DefPath`. Since
`DefPathData` now stores a `Span` (inside the `Ident`), we need to
make sure we ignore this span we determining if a path needs to be
disambiguated. This ensure that two paths with the same symbols but
different spans are considered equal (this can occur when a macro is
repeatedly expanded to a definition).

3. Ensuring that we are able to decode `Spans` when decoding the
`DefPathTable`. Since the `DefPathTable` is stored in `CrateMetadata`, we
must decode it before we have a `CrateMetadata` available. Since
decoding a `Span` requires access to several fields from
`CrateMetadata`, this commit adds a new struct `InitialCrateMetadata`,
which implements `Metadata` and stores all of the necessary fields.

4. The specialized metadata encoder/decoder impls for `Ident` are
removed. This causes us to fall back to the default encoder/decoder
implementations for `Ident`, which simply serializes and deserializes
the fields of `Ident`. This is strictly an improvement - we still don't
have any hygiene information, but we now have a non-dummy Span.

This should hopefully allow us to test PR rust-lang#68941, since we will now use
cross-crate spans in more places.
    @Aaron1011
    Aaron1011 committed Mar 17, 2020
    Configuration menu
    Copy the full SHA
    5e98189 View commit details
    Browse the repository at this point in the history

  2. Use underlying symbol whenever we string-ify DefPathData 

    Converting an `Ident` to a string will cause us to try to guess if the
identifier is 'raw', potentially inserting a 'r#' prefix. However, all
of the existing code that string-ifys `DefPathData` is expecting to be
working with a `Symbol`, which never has a `r#` suffix.

This was causing Rustdoc to generate links containing `r#` for
structs/traits with raw identifiers (e.g. `r#struct`), leading to broken
links when the actual html files were generated without the `r#` suffix.
    @Aaron1011
    Aaron1011 committed Mar 17, 2020
    Configuration menu
    Copy the full SHA
    fdd2523 View commit details
    Browse the repository at this point in the history

  3. Exhaustively match on DefPathData in with_dummy_span 

    This will cause an error if we ever add/change variants of
`DefPathData`, as we need to ensure we strip spans from any `Ident`s
    @Aaron1011
    Aaron1011 committed Mar 17, 2020
    Configuration menu
    Copy the full SHA
    050c8c8 View commit details
    Browse the repository at this point in the history

  4. Remove Hash, PartialEq, and Eq impls from DefPathData 

    These are a footgun, since they will compare/hash based on the `Span` of
any `Ident` in the `DefPathData`.

We now use a private `PlainDefPathData` enum as the key for the
`next_disambiguator` `HashMap`. This enum is the same as `DefPathData`,
but it stores `Symbol` instead of `Ident` (e.g. what `DefPathData` used
to store).

This create a small amount of boilerplate, but allows the rest of the
codebase to work exclusively with the normal `DefPathData` enum (which
uses `Ident`s).

All of the non-`HashMap` uses of `PartialEq` turned out to be
comparisons against a constant, so `matches!` can be used instead of
`==`.
    @Aaron1011
    Aaron1011 committed Mar 17, 2020
    Configuration menu
    Copy the full SHA
    2daf5c6 View commit details
    Browse the repository at this point in the history

  5. Switch baack to Spanned<Name> instead of Ident 

    While the fields are equivalent, we we constructing `Ident`s for things
that weren't really identifiers (e.g `kw::Invalid` + the span of one of
the types in a tuple struct).
    @Aaron1011
    Aaron1011 committed Mar 17, 2020
    Configuration menu
    Copy the full SHA
    145697c View commit details
    Browse the repository at this point in the history

  6. Undo switch to type alias 

    This was pointless churn
    @Aaron1011
    Aaron1011 committed Mar 17, 2020
    Configuration menu
    Copy the full SHA
    bb928a7 View commit details
    Browse the repository at this point in the history

  7. Revert trivial changes

    @Aaron1011
    Aaron1011 committed Mar 17, 2020
    Configuration menu
    Copy the full SHA
    f9a34e8 View commit details
    Browse the repository at this point in the history

  8. Fix rebase fallout

    @Aaron1011
    Aaron1011 committed Mar 17, 2020
    Configuration menu
    Copy the full SHA
    766a1d3 View commit details
    Browse the repository at this point in the history