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

Unergonomic structured suggestions in rustc #47927

Open
oli-obk opened this issue Feb 1, 2018 · 2 comments
Open

Unergonomic structured suggestions in rustc #47927

oli-obk opened this issue Feb 1, 2018 · 2 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Feb 1, 2018

While checking span_helps that could be span_suggestion I noticed that rustc contains lots of code similar to

                    match fcx.tcx.sess.codemap().span_to_snippet(self.cast_span) {
                        Ok(s) => {
                            err.span_suggestion(self.cast_span,
                                                "try casting to a reference instead",
                                                format!("&{}{}", mtstr, s));
                        }
                        Err(_) => {
                            span_help!(err, self.cast_span, "did you mean `&{}{}`?", mtstr, tstr)
                        }
                    }

where we try to get a snippet and if that fails, since we can't produce a nice suggestion, we produce a help message that contains a message.

We should probably provide a helper for that. First I thought that we could add a helper that does essentially the above without all the duplication, but with the new approximate suggestions (#47540) we can always produce the suggestion, but mark it as approximate if we need to use the fallback value.

I'd assume the above example would look something like this:

err.span_possibly_approximate_suggestion(
    self.cast_span, // span to replace
    "try casting to a reference instead", // message
    fcx.tcx.sess.codemap().span_to_snippet(self.cast_span).ok(), // optional snippet
    tstr, // default if snippet is none
    |snip| format!("&{}{}", mtstr, snip), // closure taking snippet and producing the replacement code
);

cc @Manishearth @nrc

@zackmdavis
Copy link
Member

where we try to get a snippet [...] and if that fails

(This is probably a really dumb question, but this API had bugged me for a while, so just in case my intuition isn't going haywire here—)

If we can know that spans are valid, is there much reason span_to_snippet should fail? Are malformed codemaps and the like an expected contingency to return Err on, or a rare bug that we should .expect not to happen?

@Manishearth
Copy link
Member

macros and stuff, usually. not all spans come from a contiguous region of code

@kennytm kennytm added C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants