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

suggestion-diagnostics: as_ref improve snippet #57451

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@dlrobertson
Copy link
Contributor

dlrobertson commented Jan 8, 2019

Improve the code snippet suggested in suggestion-diagnostics when
suggesting the use of as_ref.

Given:

fn test(x: &usize) {}
fn main() {
    Some(42).map(|x| test(x));
}

Suggest

  help: consider using `as_ref` instead: `as_ref().map`

Instead of

  help: consider using `as_ref` instead: `as_ref().`
@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 8, 2019

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 8, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:04ee9a5e:start=1546978089151273248,finish=1546978090321928223,duration=1170654975
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-6.0
---

[00:05:08] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:05:08] tidy error: /checkout/src/librustc_typeck/check/demand.rs:195: line longer than 100 chars
[00:05:10] some tidy checks failed
[00:05:10] 
[00:05:10] 
[00:05:10] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:05:10] 
[00:05:10] 
[00:05:10] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:05:10] Build completed unsuccessfully in 0:00:45
[00:05:10] Build completed unsuccessfully in 0:00:45
[00:05:10] Makefile:69: recipe for target 'tidy' failed
[00:05:10] make: *** [tidy] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:094e3067
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Tue Jan  8 20:13:40 UTC 2019
---
travis_time:end:0148d452:start=1546978421677496844,finish=1546978421682105515,duration=4608671
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:03c82578
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:2f41bc7b
travis_time:start:2f41bc7b
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:0247d91f
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

suggestion-diagnostics: as_ref improve snippet
Improve the code snippet suggested in suggestion-diagnostics when
suggesting the use of as_ref.

@dlrobertson dlrobertson force-pushed the dlrobertson:can_use_as_ref_nit branch from 94fe935 to b244e51 Jan 8, 2019

@@ -191,7 +192,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
/// ```
/// opt.map(|arg| { takes_ref(arg) });
/// ```
fn can_use_as_ref(&self, expr: &hir::Expr) -> Option<(Span, &'static str, String)> {
fn can_use_as_ref(&self, expr: &hir::Expr, cm: &SourceMap)
-> Option<(Span, &'static str, String)> {

This comment has been minimized.

@estebank

estebank Jan 10, 2019

Contributor

Formatting:

    fn can_use_as_ref(
        &self,
        expr: &hir::Expr,
        cm: &SourceMap,
    ) -> Option<(Span, &'static str, String)> {
match (is_as_ref_able, cm.span_to_snippet(*span)) {
(true, Ok(src)) => {
return Some((span.shrink_to_lo(), "consider using `as_ref` instead",
format!("as_ref().{}", src)));

This comment has been minimized.

@estebank

estebank Jan 10, 2019

Contributor

I understand why you are looking to change the suggestion to make it clearer, but this change would cause the code opt.and_then(|arg| Some(takes_ref(arg))); to be replaced with opt.map.and_thenand_then(|arg| Some(takes_ref(arg))); because the span only points at the point after the dot (containing nothing).

I see two alternatives: change the span to make the new suggestion work, or make the suggestion text longer than 10 words, which is the arbitrary length to inline suggestions instead of showing them on their own, which would make this suggestion easier to follow:

LL |   opt.map(|arg| takes_ref(arg));
   |                           ^^^ expected &Foo, found struct `Foo`
   |
help: consider using `as_ref` in order to hold a borrow of the inner element
   |
LL |   opt.as_ref().map(|arg| takes_ref(arg));
   |       ^^^^^^^^^
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment