Improvements of librustc_resolve and add missing error codes #26898
Conversation
| @@ -948,7 +948,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { | |||
| span_err!(self.session, span, E0259, | |||
| "an external crate named `{}` has already \ | |||
| been imported into this module", | |||
| &token::get_name(name)); | |||
| &name); | |||
There was a problem hiding this comment.
The & is not necessary (same in a few other places below).
GuillaumeGomez
changed the title
|
I'm done here. Can you take a look @eddyb please ? |
| token::get_name(key), | ||
| "#", i + 1, "#")); | ||
| key, | ||
| "#", i + 1, "#"); |
There was a problem hiding this comment.
Any reason why # can't be part of the format string?
|
cc @pnkfelix |
| // NB: This module needs to be declared first so diagnostics are | ||
| // registered before they are used. | ||
| pub mod diagnostics; | ||
|
|
||
| macro_rules! resolve_err { |
There was a problem hiding this comment.
I continue to argue that this macro is not necessary.
There was a problem hiding this comment.
In particular, if all of the places where you current write resolve_err! instead called into one centralized error method in Resolver, then that central error method would then be the place that says, at the beginning:
if !resolver.emit_errors { return; }
|
Updated. |
| CannotCaptureDynamicEnvironmentInFnItem(&'b Resolver<'a, 'tcx>, syntax::codemap::Span), | ||
| /// error E0435: attempt to use a non-constant value in a constant | ||
| AttemptToUseNonConstantValueInConstant(&'b Resolver<'a, 'tcx>, syntax::codemap::Span), | ||
| } |
There was a problem hiding this comment.
I don't see why every variant has to repeat the same two fields.
|
Arguments are now passed directly to the function. |
| mod check_unused; | ||
| mod record_exports; | ||
| mod build_reduced_graph; | ||
| mod resolve_imports; | ||
|
|
||
| pub enum ResolutionError<'b> { |
| /// error E0427: cannot use `ref` binding mode with ... | ||
| CannotUseRefBindingModeWith(&'b str), | ||
| /// error E0428: duplicate definition | ||
| DuplicateDefinition(&'b str, &'b str), |
There was a problem hiding this comment.
Second value can be an ast::Name.
|
The macro has been removed and few other updates. |
| AttemptToUseNonConstantValueInConstant, | ||
| } | ||
|
|
||
| fn resolve_error<'b, 'a:'b, 'tcx:'a>(resolver: &'b Resolver<'a, 'tcx>, span: syntax::codemap::Span, |
There was a problem hiding this comment.
why does resolution_error need to be a reference?
There was a problem hiding this comment.
It doesn't, I totally forgot to remove it...
|
Updated. |
|
r+ or there is something else to be updated ? |
|
@bors r+ |
|
📌 Commit 60133aa has been approved by |
|
(This looks great, thanks for all the effort you put into the various revisions here, @GuillaumeGomez ! ) |
|
(You're very welcome @pnkfelix =D) |
r? @eddyb
First part of the improvement. I then intend to improve resolve_error as indicated by @eddyb. Do not merge for now (please !).