Desugar parenthesized generic arguments in HIR #43532
Conversation
| @@ -16,6 +16,7 @@ struct Bar<A> { | |||
|
|
|||
| fn foo(b: Box<Bar()>) { | |||
| //~^ ERROR parenthesized parameters may only be used with a trait | |||
| //~| ERROR the type placeholder `_` is not allowed within types on item signatures | |||
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... |
carols10cents
added
the
S-waiting-on-review
|
@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 ! |
| /// Were parameters written in parenthesized form `Fn(T) -> U`? | ||
| /// This is required mostly for pretty-printing and diagnostics, | ||
| /// but also for changing lifetime elision rules to be "function-like". | ||
| pub parenthesized: bool, |
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! |
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.
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 ?
| self.prohibit_projection(binding.span); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| 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?
| @@ -2359,21 +2320,6 @@ impl Foo { | |||
| "##, | |||
| */ | |||
|
|
|||
| 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