`E0465` cannot logically occur

compiler
1

Ok, so I'm trying to help finish off writing long docs for compiler error codes (found here). Currently I'm working on E0465, which does not have associated UI tests. This means that I have to try and work my way through compiler code trying to figure out how to get the error to occur. The relevant code is in compiler/rustc_metadata/src/locator.rs (github link) in the function extract_one (github link).

The CrateError::MultipleCandidates, as far as I can tell, cannot be created:

  • Here requires slot to be set, however the function prematurely returns if that is the case here.
  • Here requires err_data to be set, which only occurs in the above (i.e. it doesn't occur).

Nowhere else in the compiler is this error created. Could someone else please verify this? And if so, can I quickly just make a PR removing this error code?

I'm sorry if this isn't the the correct place to ask this, especially with all the code links lol, but I'm not sure where else I can.

2

The place to ask should be fine. Alternatively, on Zulip would probably also have been an option. I didn't look into or try to understand any code. Looking through GitHub I found this issue though that seems relevant to your question.

The PR that closes the issue did apparently introduce a list of error codes that are explicitly not tested, because - as far as I understand the intention - they cannot happen anymore (though I'm not sure whether the criterion was that it's known for sure they can't happen, or maybe just that it isn't known how they could happen and further work is required.

Here's the list in the code today

Actually looking at that now, it seems like the latter is the case and it's a list that also contains errors where simply nobody created a test yet.

Would seem reasonable to me if this list was eventually eliminated by making sure that every error is either tested or - if actually impossible, removed, I guess? (My immediate personal thoughts, someone more knowledgeable than me about the compiler could probably say more.)

It might also make sense, for prior art, to look (through git blame) into the commits that changed this list, which might then - assuming my hypothesis above is correct - show previous contributions adding tests to error codes or maybe even removing error codes.

Perhaps, all this isn't news to you and you're just looking for someone to confirm your analysis of the code, in which case I'm not all that helpful :slight_smile:

3

I'm reasonably familiar with that list, I've been working on documenting various obscure error codes. There can be valid reasons why a error code might not have doc-tests, for example errors that occur when linking (including, in theory, E0465); doc-tests can't manage dependencies like this. Although I've found that most of those tests are just copied from the UI ones (which can do linking, etc) and so it's not a big loss.

Assuming that this error is unreachable (that issue seems to confirm this could be a possibility), is it ok to just create a PR removing the error entirely? I can't imagine that this is a breaking change or anything; the error is redundant and appears to be taken over by E0464.

lol, I only read this at the end. You have confirmed some other stuff so thanks for your help but I am still looking to see if that error is unreachable.

4

Sounds reasonable to me, though of course feel free to wait for someone else to also say something before you spend efforts based on my personal interpretation alone here. In case the check that produces the error shall be kept nonetheless (i. e. as a sanity check), it could be turned into an ICE case. That would also mean that in case it wasn't unreachable after all, or future changes make the case possible again, we can know by issues being opened.

In that case, assuming that such errors need to be listed in that list[1], it could - in my opinion - make sense to sort-of split the list into two parts; one part being documented to be known to be not testable with doc tests (and reasonably so), and the other half being documented, i. e. effective in a "we don't know for sure yet" category.

  1. I'm not exactly familiar with what exactly this particular tidy lint checks ↩︎

5

Umm, I'm not sure what you mean but this error code, among many, has no tests. It's just randomly placed in a file with no docs/ testing/ explanation whatsoever.

If I understand you correctly then that sounds good. However keep this in mind: it's typically not possible to provide long-form documentation about an error (with possible solutions, etc) without having a good reproduction of the error (even if not doc-tested), and - IMHO - it's hard for a user to understand docs without some example.

Thanks for your thoughts, this is very thought-provoking.

EDIT: Also I should mention that I've never looked closely at what those lists do. I just fumble around with them until something works.

6

Right, I meant that in case that tidy check can only recognize certain kinds of tests and thus there are legitimate cases of errors that (although they are tested at all) need to stay on the list of exceptions got that tidy check, then it might make sense to split up the list (until the number of "we don't know yet" cases has gone to zero).

7

Sorry for being vague. I haven't looked much at the code you linked and can't right now. I just assumed there is some code creating an error in a case that (you think) should be unreachable, and in that case, there might be different approaches of what it means to "remove the error". Either the unreachability of a condition being used to remove some kind of check, or the case being made a panic or ICE case.

8

  1. The tidy test just looks at the markdown files for the long error docs, and makes sure that they have, among other things, a doc-test that fails with the correct error code. The mysterious lists affect this process somehow. I absolutely agree that for now, and probably for a while, even though not ideal that there needs to be a list for "we don't know yet."

  2. I'm not entirely sure what you mean sorry, I'm quite new to developing on the compiler but I think that the condition has kind of been absorbed into another broader error (and is therefore physically unable to be created), and the error in question can just be purged from the codebase.

9

slot is set within the loop too. Line 595 is reached if there are multiple candidate libraries with different hashes.

1 Like
10

Here's a convoluted way to recreate E0465. There might be a simpler way.

mkdir dir1
mkdir dir2
echo "pub fn foo() {}" > a1.rs
echo "pub fn bar() {}" > a2.rs
rustc --crate-type=rlib --crate-name=a a1.rs --out-dir dir1
rustc --crate-type=rlib --crate-name=a a2.rs --out-dir dir2
echo "extern crate a; fn main() {}" > main.rs
rustc main.rs -L dir1 -L dir2
10 Likes
11

It's also worth noting there are other error codes listed as "no longer emitted by the compiler." It's generally better to keep the error number registered, so it doesn't get reused in the future and someone googling for the error number finds irrelevant information.

If there's literally no code to emit E0465 (rather than it just being logically unreadable) it could make sense to mark it as no longer emitted. If there is still a check which can emit the error, what explanation that is there should imho remain in case it is (or becomes) reachable, though perhaps with a note that it's only possible to hit when using rustc directly, not through cargo (presuming cargo does catch it before calling rustc).

7 Likes
12

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.