You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Improve error message for infinitely recursive impl Trait type #47659
After #47529, attempts to compile code which builds an infinitely recursive impl Trait type will error with "overflow evaluating the requirement impl Quux". This isn't super user-friendly, and should be improved.
Just encountered this issue when refactoring large parser using combine crate. It used to have a wrapper function parser(func) for creating reusable parsers. Now the recommended way is to rely on -> impl Parser return type instead (see second example).
Parsers inherently have many branches and are self-recursive. So after refactoring the codebase I encountered this error and had to guess which parser from 15 refactored functions has this issue.
It wasn't very hard when I understood the issue, but the stack overflow error wasn't very informative of what happened at all.
@ZerothLaw I'm not sure-- it's hard to know exactly what to report. You know that an overflow has occurred, but not that it is necessarily due to recursive use of impl Trait. This is where the current overflow reporting is triggered-- we could add a suggestion hinting that this may be the result of attempting to build an infinitely recursive impl Trait type, but we don't know for sure that that is the cause. You could record the DefId of each TyAnon type encountered while folding recursively and then report a recursion error if you see a repeat DefId.
@cramertj provides a good approach in his comment. To be specific, you might want to start by:
Add a new field to the AssociatedTypeNormalizer struct for storing already-seen types (values of ty parameter to fold_ty) that you're folding with. (I think @cramertj's suggestion of using the def-ids for the types would work in most cases, but may fail in some complex edge cases. Maybe he can chip in here.) A HashSet (FxHashSet?) may be the best type for this.
At the top of the fold_ty function, check for the newly-encountered type in the set of already-seen types. If you find it, raise the error; if not, add the type to the set and continue.
Model the error reporting on one of the examples @estebank gave in his above comment. Probably the second one is a better prototype. Here is the code for reporting it; I suggest creating a new method that's pretty similar to that one.
Create a test for this issue under src/tests/ui/issues/issue-47659 with an appropriate minimal reproducible case. Make sure you "bless" the test once everything is running.
The rustc development guide is pretty handy if you're curious about how the compiler fits together, or what purpose a particular type or function serves, in many cases. Still, if you have any difficulties with the above, post back here, or hop onto the #compiler channel on Discord. Good luck!
Activity
tailhook commentedon Aug 4, 2018
Just encountered this issue when refactoring large parser using
combinecrate. It used to have a wrapper functionparser(func)for creating reusable parsers. Now the recommended way is to rely on-> impl Parserreturn type instead (see second example).Parsers inherently have many branches and are self-recursive. So after refactoring the codebase I encountered this error and had to guess which parser from 15 refactored functions has this issue.
It wasn't very hard when I understood the issue, but the stack overflow error wasn't very informative of what happened at all.
ZerothLaw commentedon Aug 21, 2018
Would this be a good issue for a relative newcomer to tackle? As I understand it, its mostly about adding a more informative error message.
cramertj commentedon Aug 22, 2018
@ZerothLaw I'm not sure-- it's hard to know exactly what to report. You know that an overflow has occurred, but not that it is necessarily due to recursive use of
impl Trait. This is where the current overflow reporting is triggered-- we could add a suggestion hinting that this may be the result of attempting to build an infinitely recursiveimpl Traittype, but we don't know for sure that that is the cause. You could record theDefIdof eachTyAnontype encountered while folding recursively and then report a recursion error if you see a repeatDefId.ZerothLaw commentedon Aug 22, 2018
That's a good point. So before working on it, one should first figure out what the acceptable solution would be?
estebank commentedon Sep 12, 2018
You can take inspiration from https://github.com/estebank/rust/blob/master/src/test/ui/lint-unconditional-recursion.stderr and https://github.com/estebank/rust/blob/master/src/test/ui/span/recursive-type-field.stderr
alexreg commentedon Nov 12, 2018
@ZerothLaw Did you make any progress on this lately? I can maybe help give you more pointers, if needed.
ZerothLaw commentedon Nov 13, 2018
I haven't made any attempt on this. Any suggestions?
alexreg commentedon Nov 13, 2018
@ZerothLaw
@cramertj provides a good approach in his comment. To be specific, you might want to start by:
AssociatedTypeNormalizerstruct for storing already-seen types (values oftyparameter tofold_ty) that you're folding with. (I think @cramertj's suggestion of using the def-ids for the types would work in most cases, but may fail in some complex edge cases. Maybe he can chip in here.) AHashSet(FxHashSet?) may be the best type for this.fold_tyfunction, check for the newly-encountered type in the set of already-seen types. If you find it, raise the error; if not, add the type to the set and continue.src/tests/ui/issues/issue-47659with an appropriate minimal reproducible case. Make sure you "bless" the test once everything is running.The rustc development guide is pretty handy if you're curious about how the compiler fits together, or what purpose a particular type or function serves, in many cases. Still, if you have any difficulties with the above, post back here, or hop onto the #compiler channel on Discord. Good luck!
alexreg commentedon Dec 25, 2018
@ZerothLaw Had a chance to tackle the above yet, by chance?
cramertj commentedon Dec 26, 2018
@alexreg This is covered by #56074.