Add note when item accessed from module via m.i rather than m::i.

[pnkfelix]

Add note when item accessed from module via m.i rather than m::i.

(I tried to make this somewhat future-proofed, in that the UnresolvedNameContext could be expanded in the future with other cases besides paths that are known to be modules.)

This supersedes PR #30356 ; since I'm responsible for a bunch of new code here, someone else should review it. :)

[rust-highfive]

r? @nikomatsakis

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

[pnkfelix]

gaar I posted this before running make check locally ... so of course it has tidy issues...

(update: fixed that and the typo in the unit test; make check-stage-cfail passes locally now)

[GuillaumeGomez]

I'm still working on adding the two missing additional information.

[pnkfelix]

I'm still working on adding the two missing additional information.

(and I think we do not need to block this PR on that change.)

[pnkfelix]

@GuillaumeGomez Just to be clear: The reason that I turned #30356 into this PR was that I was concerned that it was giving "bad hints" in some contexts.

• I was concerned that Add note when a function is called from a module like it's an object #30356 was reporting an error mentioning a function call form like m.f(...) in source code contexts where there was no occurrence of .f(...).

I wrote up the examples you noted as ways to illustrate cases where #30356 would give bad hints.

But I did not intend to imply something like "we should wait until we have solved all of these cases before we land anything."

My intention in this PR was to continue to solve the same narrow problem that #30356 was trying to solve, but to do it in a more targeted fashion that ensures that we only provide in hint about m.i or m.f(..) in cases where there actually is an m.i or m.f(..) present.

---

I definitely encourage you to try to come up with good error messages for other cases that this PR does not cover. You should do that work and put it up as a separate PR.

[Manishearth]

Could you add a doc comment on this so it's not confusing for future hackers?

(Also explain what the nodeid is)

[Manishearth]

r=me with nit

[frewsxcv]

This should also solve #22692 ?

[pnkfelix]

@bors r=Manishearth 04c05c7 rollup

[nikomatsakis]

@pnkfelix

The reason that I turned #30356 into this PR was that I was concerned that it was giving "bad hints" in some contexts.

Can you add tests that check that we do NOT provide bad hints? Hmm, I guess that's hard to do, isn't it?

[nikomatsakis]

@frewsxcv

This should also solve #22692 ?

I suspect not. This seems targeted at modules, not items found in impls on types (but resolve works in mysterious ways and constructs some pseudo-modules, so maybe it works sometimes).

[pnkfelix]

Can you add tests that check that we do NOT provide bad hints? Hmm, I guess that's hard to do, isn't it?

I don't think I'm going to try to do that. :)

[bors]

⌛ Testing commit 04c05c7 with merge ea6e312...

[bors]

💔 Test failed - auto-win-gnu-32-opt

[pnkfelix]

What the heck, a bunch of errors in C:/bot/slave/auto-win-gnu-32-opt/build/src/llvm/lib/Target/Mips/MipsInstrInfo.h ...?

[alexcrichton]

@bors: retry

On Sun, Dec 20, 2015 at 4:38 AM, Felix S Klock II notifications@github.com
wrote:

What the heck, a bunch of errors in
C:/bot/slave/auto-win-gnu-32-opt/build/src/llvm/lib/Target/Mips/MipsInstrInfo.h
...?

—
Reply to this email directly or view it on GitHub
#30413 (comment).

[bors]

⌛ Testing commit 04c05c7 with merge e2834a2...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt