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

Meta issue for FIXMEs that reference closed issues #44366

Open
oli-obk opened this issue Sep 6, 2017 · 12 comments
Open

Meta issue for FIXMEs that reference closed issues #44366

oli-obk opened this issue Sep 6, 2017 · 12 comments
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-help-wanted Call for participation: Help is requested to fix this issue. metabug Issues about issues themselves ("bugs about bugs") T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Sep 6, 2017

@Eh2406 compiled this awesome list. I think it should be a meta issue. So here it is

https://gist.github.com/Eh2406/b92f034f8adc193894482e19c79d80f1

@Eh2406
Copy link
Contributor

Eh2406 commented Sep 6, 2017

The good part, this was made with an automated script. I can keep it up-to-date by rerunning as often as we want.

The bad part, it was made with an automated script. Some of these are false positives, like the first one, where the script was too dumb to realize said it's an issue on a different Repository.

In general fees can be fixed by:

  1. Doing what the fix me said to do and seeing if the pr passes
  2. Updating the fix me to refer to a more current issue number
  3. Removing the fix me has the issue was closed won't fix

In the past couple of days I have been reviewing some of these, and if I'm not sure what to do I'v leaving a comment on the clothes issue with a link to the fix me asking for advice.

@alexcrichton alexcrichton added the C-cleanup Category: PRs that clean code up or issues documenting cleanup. label Sep 7, 2017
@TimNN TimNN added the metabug Issues about issues themselves ("bugs about bugs") label Sep 17, 2017
bors added a commit that referenced this issue Oct 5, 2017
address some FIXME whose associated issues were marked as closed

part of #44366
@Eh2406
Copy link
Contributor

Eh2406 commented Oct 6, 2017

Thanks @nivkner! Let me know when I should rerun the script.

@nivkner
Copy link
Contributor

nivkner commented Oct 7, 2017

This kind of metabug seems like something that should be tackled continuously, as new FIXMEs are added over time, so if possible this would be better run routinely. Otherwise there really isn't much of a difference between running the script or marking FIXMEs as complete.

Since I wouldn't ask you to dedicate so much time to this one issue, I'd like to know what is your opinion on turning this script into a github bot like "rust-highfive" or "bors". Then whenever an issue is closed, this "FIXME bot" could update this issue.

zackmdavis added a commit to zackmdavis/rust that referenced this issue Oct 16, 2017
This comment made sense when it was introduced in fbef241. It does not
make sense in its current context, where the referred-to guard is no
longer present.

This being an item under the fabulous metabug rust-lang#44366.
zackmdavis added a commit to zackmdavis/rust that referenced this issue Oct 16, 2017
At reviewer's suggestion, we remove the function/static name from the
main lint message. While we're correspondingly adjusting the
expectations of a compile-fail test, we remove an obsolete FIXME
comment, another quantum of progress towards resolving the fabulous
metabug rust-lang#44366.
kennytm added a commit to kennytm/rust that referenced this issue Oct 17, 2017
address more FIXME whose associated issues were marked as closed

part of rust-lang#44366
@varkor
Copy link
Member

varkor commented Jan 20, 2018

It'd be helpful if the code links pointed to specific commit IDs, so that the line numbers remain correct.

@Eh2406
Copy link
Contributor

Eh2406 commented Jan 20, 2018

@varkor thank you for the suggestion. I changed the script to use the sha in the links, and updated the gist to master.

@nivkner It looks like my reply to you never posted, I am very sorry for not replying for so long. I don't have the experience to set up a bot like "rust-highfive" or "bors". So I posted the script I am using so someone with more experience can do that if they like. In the meantime it is no bother for me to run it as needed. I had written up a response like this but apparently it did not post.

@nivkner
Copy link
Contributor

nivkner commented Jan 20, 2018

Thanks for replying @Eh2406,
Could you please post a link here to the script? I can't find it in your github repositories.
If you don't mind, then could you also post the output of the script, just so we'll know where we are standing after all this time.

@Eh2406
Copy link
Contributor

Eh2406 commented Jan 20, 2018

The gist is at:
https://gist.github.com/Eh2406/b92f034f8adc193894482e19c79d80f1
it has the current (as of today) output and the code.
I posted the code on Oct 14, 2017, the same day I failed to reply hear.

kennytm added a commit to kennytm/rust that referenced this issue Mar 21, 2018
address some FIXME whose associated issues were marked as closed

part of rust-lang#44366
kennytm added a commit to kennytm/rust that referenced this issue Mar 22, 2018
address some FIXME whose associated issues were marked as closed

part of rust-lang#44366
@Eh2406
Copy link
Contributor

Eh2406 commented Jun 7, 2018

Just found one not picked up by my script at:

// Annotate exceedingly likely branches in `table::make_hash`
// and `search_hashed` to reduce instruction cache pressure
// and mispredictions once it becomes possible (blocked on issue #11092).

@jonas-schievink jonas-schievink added E-help-wanted Call for participation: Help is requested to fix this issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jan 7, 2020
@ljedrz
Copy link
Contributor

ljedrz commented Jan 7, 2020

Actually it appears that many of those have already been removed; we could use another pass of that script.

@Eh2406
Copy link
Contributor

Eh2406 commented Jan 7, 2020

The script is still available on the Gist https://gist.github.com/Eh2406/b92f034f8adc193894482e19c79d80f1#file-fixme-py

I stopped running it (when I updated computers) after see this comment https://internals.rust-lang.org/t/compiler-steering-meeting/8588/49 that @pnkfelix and @oli-obk had a more official project.

Maybe we can add this data to that project?

@varkor
Copy link
Member

varkor commented Jan 7, 2020

For reference, @oli-obk's project is here.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 3, 2023
…=Nilstrieb

Handle non-utf8 rpaths (fix FIXME)

Removes a FIXME for rust-lang#9639 which is closed since long ago.

Part of rust-lang#44366 which is E-help-wanted.

(Also see rust-lang#114377)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 3, 2023
compiletest: Handle non-utf8 paths (fix FIXME)

Removes the last FIXME in the code for rust-lang#9639  🎉 (which was closed 8 years ago)

Part of rust-lang#44366 which is E-help-wanted.

(The other two PRs that does this are rust-lang#114377 and rust-lang#114427)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 4, 2023
compiletest: Handle non-utf8 paths (fix FIXME)

Removes the last FIXME in the code for rust-lang#9639  🎉 (which was closed 8 years ago)

Part of rust-lang#44366 which is E-help-wanted.

(The other two PRs that does this are rust-lang#114377 and rust-lang#114427)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 4, 2023
…=Nilstrieb

Handle non-utf8 rpaths (fix FIXME)

Removes a FIXME for rust-lang#9639 which is closed since long ago.

Part of rust-lang#44366 which is E-help-wanted.

(Also see rust-lang#114377)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 6, 2023
linkchecker: Remove unneeded FIXME about intra-doc links

It was added by rust-lang#77971 but the adder [proposed](rust-lang#77971 (comment)) that the added code is a good fallback to have in case rustdoc gets buggy, and I agree. So remove the FIXME.

This PR is part of rust-lang#44366 which is E-help-wanted.

r? `@jyn514` since you added the FIXME

`@rustbot` label T-dev-tools
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 6, 2023
…compiler-errors

Issue numbers are enforced on active features; remove FIXME

Since rust-lang#51090 tidy enforces that active features have an issue number, so remove the FIXME.

This PR is part of rust-lang#44366 which is E-help-wanted.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 6, 2023
… r=cjgillot

Remove FIXME about NLL diagnostic that is already improved

The FIXME was added in rust-lang#46984 when the diagnostic message looked like this:

    // FIXME(rust-lang#46983): error message should be better
    &s.0 //~ ERROR free region `` does not outlive free region `'static`

The message was improved in rust-lang#90667 and now looks like this:

    &s.0 //~ ERROR lifetime may not live long enough

but the FIXME was not removed. The issue rust-lang#46983 about that diagnostics should be improved has been closed. We can remove the FIXME now.

(This PR was made for rust-lang#44366.)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 8, 2023
…iler-errors

tests: Uncomment now valid GAT code behind FIXME

The code fails to parse with `nightly-2021-02-05`:

    $ cargo +nightly-2021-02-05 build
    error: generic associated types in trait paths are currently not implemented
     --> src/main.rs:9:42
      |
    9 | fn _bar<T: for<'a> StreamingIterator<Item<'a> = &'a [i32]>>(_iter: T) { /* ... */
      |                                          ^^^^

but parses with `nightly-2021-02-06`:

    $ cargo +nightly-2021-02-06 build
    warning: the feature `generic_associated_types` is incomplete and may not be safe to use and/or cause compiler crashes
    warning: 1 warning emitted

because it was (with high probability) fixed by rust-lang#79554 which was merged within that nightly range.

This PR is part of rust-lang#44366 which is E-help-wanted.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 9, 2023
…tf-8, r=thomcc

test_get_dbpath_for_term(): handle non-utf8 paths (fix FIXME)

Removes a FIXME for rust-lang#9639

Part of rust-lang#44366 which is E-help-wanted

The remaining two FIXMEs for rust-lang#9639 are considerably more complicated, so I will create separate PRs for them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-help-wanted Call for participation: Help is requested to fix this issue. metabug Issues about issues themselves ("bugs about bugs") T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants