Desugar parenthesized generic arguments in HIR#43532
Conversation
There was a problem hiding this comment.
This is an annoying little regression, but from what I tried "infer everything" was the least evil (in terms of redundant diagnostics), given the constraints 1) usize() should be a warning and not an error and 2) lifetime resolution has some issues causing ICEs if () in non-traits is kept "parenthesized", I hope to fix this in another PR
|
cc @nikomatsakis I'll take a look later, but from what I remember is that there were some issues with doing it this way, so we didn't attempt. If only I wrote down the issues... |
|
@nikomatsakis is on vacation until about 2017-08-15, @eddyb. |
|
☔ The latest upstream changes (presumably #43522) made this pull request unmergeable. Please resolve the merge conflicts. |
|
ping @eddyb and/or @nikomatsakis ! |
src/librustc/hir/mod.rs
Outdated
There was a problem hiding this comment.
Ah, I see, with this here it's probably okay.
|
@carols10cents I still want to defer to @nikomatsakis here. |
|
Rebased. |
|
@petrochenkov sorry for delay! |
nikomatsakis
left a comment
There was a problem hiding this comment.
Sorry, I am going to have to revisit this PR tomorrow. I don't quite understand why it's ok to remove some of the checks that were removed (presumably they are either impossible or taking place earlier, but I have to take another stab at reading it to be sure).
| time(time_passes, | ||
| "early lint checks", | ||
| || lint::check_ast_crate(sess, &krate)); | ||
|
|
There was a problem hiding this comment.
wait what's this -- was this intentionally moved?
There was a problem hiding this comment.
Yes, after #43522 you can't report late lints without tcx, and you can't report early lints after lint::check_ast_crate is run. So you can't report lints during AST -> HIR lowering at all.
Thus, I had to move lint::check_ast_crate slightly further in the pipeline, to after lowering, to report PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES (compatibility lint).
There was a problem hiding this comment.
Makes sense, I'd move all the AST validation into HIR lowering itself.
nikomatsakis
left a comment
There was a problem hiding this comment.
Sorry for the delay. This time when I read through it all made sense to me. r=me presuming that my questions are answered in the affirmative -- the one thing is that I'd like to be sure there are tests for the various error cases... do you know @petrochenkov ?
| } | ||
| } | ||
|
|
||
| pub fn prohibit_parenthesized_params(&self, segment: &hir::PathSegment, emit_error: bool) { |
There was a problem hiding this comment.
these errors are now reported during lowering? do we have tests for all (or most?) of those cases?
| "##, | ||
| */ | ||
|
|
||
| E0214: r##" |
There was a problem hiding this comment.
this is also reported during lowering now?
|
@bors r=nikomatsakis |
|
📌 Commit 000f87a has been approved by |
|
☀️ Test successful - status-appveyor, status-travis |
Fixes ICE in #43431 and maybe some other similar issues.
r? @eddyb