codegen: Do the same workaround we do for template parameters with typename on aliases for decltypes we can't resolve.
#384
Conversation
|
Actually, fitzgen was on vacation IIRC, want to review this @nox? |
| inner_canon_type.name().map_or(false, |name| { | ||
| name.contains("decltype ") || | ||
| name.contains("decltype(") | ||
| }) { |
There was a problem hiding this comment.
We should never have any spaces or parens for named types, regardless of decltype or typename.
What we want here is a generic check that this named type is a valid C++ identifier.
There was a problem hiding this comment.
I tend to agree with this. My thinking was that given this is the exception, and not the norm, I think we want to know when can we generate these incorrect bits before skipping them.
I'm fine tweaking this to be a generic identifier check though, I guess.
|
☔ The latest upstream changes (presumably #414) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@emilio were you going to circle back to this? |
We do the same for template parameters with `typename` on aliases. This is not great, but it's better than generating invalid code.
|
Whoops, forgot about this, there you go :) |
| } | ||
| _ => false, | ||
| } | ||
| } |
There was a problem hiding this comment.
Would be great to have a small #[test] unit test, or a doc test. Just for sanity.
Ok as a follow up, or as an issue marked E-Easy.
There was a problem hiding this comment.
r=me with your decision on what to do about ^
|
@bors-servo r=fitzgen Will open an |
|
📌 Commit f4d4994 has been approved by |
|
⚡ Test exempted - status |
codegen: Do the same workaround we do for template parameters with `typename` on aliases for `decltypes` we can't resolve. r? @fitzgen
r? @fitzgen