When trying to import two types with the same name, say which line the first one was imported on

[nstoddard]

Right now, you can get errors like "a type named Primitive has already been imported in this module", which isn't very helpful. The error message would be much more helpful if it would tell you which line the other Primitive type was imported on.

[nrc]

Happy to mentor this

[ddgond]

I would like to take a stab at it.

[ddgond]

How would one find the line number?

[nrc]

You don't explicitly need to find the line number, you should call note_span on the current session with the span from the first import. That contains the position info including the line number. note_span will format the message properly. The tricky bit of this is getting the span for the previous import.

[nrc]

If you haven't already, look at check_for_conflicting_import in librustc_resolve/lib.rs. You'll need to some how save a span when you insert a name into the Target

[ddgond]

Could you give me a quick explanation of how Target works? How does one store a value such as a span in it? Also, did you mean span_note?

[nrc]

Yeah, span_note, sorry. So, actually, ImportResolution is a more likely candidate for this. That represents a use and the resolved module it points to (in fact, potentially two modules, one in each namespace). The ids in ImportResolution are, I think, ids for the use expression. You should be able to look these ids up in th ast_map in the current Resolver, the ids should always point to use AST nodes and these should have spans associated which you can pass to span_note. The slight complication is that the functions that check for conflicts don't always take an ImportResolution (the caller looks up the Target of the resolution) - you'll have to change that.

[ddgond]

A few questions for clarification. Which of the two ids in ImportResolution, type_id or value_id, is the one we want? Also, how does one use the id to find the ast nodes? How does ast_map work, and how do you access the spans of the nodes?

[nrc]

Whether you want the type or value id depends on whether the new import is importing into the type or value namespace. It might be both, in which case you might need to check which id is already used (if both are set, then they should be set to the same value and you should probably assert! that).

The ast map is a mapping from node ids to ast nodes, so to find an ast node, you lookup the node id in the ast map. Check out libsyntax/ast_map/mod.rs to see the functions available. Since there are many kinds of nodes, you'll need to match the result to get the node you want, but there are convenience functions to do this in that file (expect_*) if there is not one for the node you need, add it. See libsyntax/ast.rs for all the node kinds. If you look in ast.rs you'll see how the span is stored, it is a bit different from node to node, some store it explicitly, some use the Spanned helper, but it should be clear which is which.

[ddgond]

How do I know which namespace the import is importing into, or whether it's importing into both? Also, how do I access the ast map, and how do I know which expect_* I want?

[nrc]

I realised that the ast_map won't in fact have the use imports in, but I needed this for something else so is fixed by #20963, you'll want to pull that in to your branch before it lands. You'll want expect_view_item (which is new in that PR).

You access the ast_map via Resolver::ast_map, I believe all the places you need to change are methods on Resolver, so self.ast_map.expect_view_item(id).

I'm not exactly sure where you're working, but there is usually a namespace arg to tell you which namespace you are dealing with - type is Namespace and its either ValueNS or TypeNS. If it's not obvious which namespace you're dealing with let me know which method you are in and I'll try to be more specific.

[bagedevimo]

@ddgond, are you still working on it? If not, I'd love to take a stab at it. :)

[ddgond]

Sure, go right ahead!

[tbelaire]

@bagedevimo How'd it go? Can I try my hand at it now?

[bagedevimo]

Go ahead. I totally ran out of time and didn't get to this one.
On 25/04/2015 4:02 pm, "Theo Belaire" notifications@github.com wrote:

@bagedevimo https://github.com/bagedevimo How'd it go? Can I try my
hand at it now?

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

[tbelaire]

@nrc How do you get the logging statements to show up?
I can getting some output from export RUST_LOG=debug,
but it's all lines like

INFO:rustc::metadata::loader: reading "libcore-4e7c5e5c.rlib" => 0ms

and none of the lines like

 230         debug!("(resolving imports for module subtree) resolving {}",           

[tbelaire]

I'm currently basing this code off of the code for rejecting duplicate function definitions, since that has a similar note to what we want. Line 217 of build_reduced_graph.rs seems to be promising,
and I'd like to put some of that at line 905 of resolve_imports.rs.

Why does self.session.span_note, and self.resolve_error get used, as opposed to span_error!?

[tbelaire]

Hmm, that approach does do something, but it's not exactly what I had in mind

mod sub1 {                                                                         
    fn foo() {} // implementation 1                                                
}                                                                                  

mod sub2 {                                                                         
    fn foo() {} // implementation 2                                                
}                                                                                  

use sub1::foo; //~ note first imported here                                        
use sub2::foo; //~ error a value named `foo` has already been imported in this module [e0252]

fn main() {}

My approach gives

double-import.rs:15:5: 15:16 note: first import of value `foo` here
double-import.rs:15     fn foo() {} // Implementation 1
                        ^~~~~~~~~~~

which isn't exactly what I want, but it is interesting.

[tbelaire]

@nrc
Has expect_view_item been removed since you added it? I don't see it anymore.
What would you recommend instead?

[tbelaire]

Well, I've got something that compiles, at least in stage0. I'm sure it's not exactly the best looking at this moment, but how'd you make this more idiomatic? #24818

[tbelaire]

Alright, looks like it's reasonable, though I had a type that needed fixing.

[nrc]

@tbelaire argh, sorry I missed your questions here, I've been remiss in checking my email the last few days. I'll have a look at your PR tomorrow.

[barosl]

This can be closed now, as the feature PR was merged.