Skip to content

Macro recursion limit is still too low #38541

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
JoeyAcc opened this issue Dec 22, 2016 · 17 comments
Closed

Macro recursion limit is still too low #38541

JoeyAcc opened this issue Dec 22, 2016 · 17 comments

Comments

@JoeyAcc
Copy link

JoeyAcc commented Dec 22, 2016

I've seen this issue, and since it's closed I've decided to open a new one.

I have just hit this limit. The problem for me is that I hit it while using std::vec!, and AFAICT the only way to accommodate more elements is not just by specifying #[recursion_limit = "10000"] but also defining my own version of vec! since otherwise the default vec! macro will simply ignore the attribute as it's not defined in my crate.

I don't believe it's a good idea for Rust programmers to have to define their own versions of definitions in std, hence the issue.

@bluss
Copy link
Member

bluss commented Dec 22, 2016

You don't need to define your own vec, simply up the recursion limit. Put #![recursion_limit = "1024"] at the top of your crate.

That said (1) vec![] is not deeply recursive (but calls itself at most once), so something else must be consuming recursion steps, and (2) please let's raise the default macro recursion limit.

@JoeyAcc
Copy link
Author

JoeyAcc commented Dec 22, 2016

I put the attribute with some insanely high number (10k or something) in my own crate and it still hit the recursion limit.
My datastructure is huge and deeply nested (the definition of the instance alone is on the order of 400 SLOC and so deeply nested it rightward-drifted off screen 2x over), but it's not huge enough to hit 10k. Nowhere near it in fact.

Ultimately I decided to just keep the boilerplate as editing the datastructure is not exactly fast or pleasant, and it's easy to mess up to boot. That said, I would still like the recursion limit to either be raised to some really high number (e.g. 10k) or just to disappear at all.

The fact that it even exists is I think more of a necessary (for now) evil, as with traversers (e.g. macros) this isn't something I intuitively think of. In fact Rust is the first language I've used that actually limits how far the code can recurse, apparently with the goal of guaranteeing that compilation actually completes.

if I'm correct, I'd still choose letting go the recursion limit as even with the attribute it can still give rise to problems. On top of that, you'll likely notice if the code either 1. doesn't compile and errors out or 2. takes an extraordinary amount of time to complete e.g. 10 minutes or more for a "Hello world" style program.

@bluss
Copy link
Member

bluss commented Dec 22, 2016

Just to make sure, did you use the attribute form with the #! (exclamation mark included)?

@durka
Copy link
Contributor

durka commented Dec 22, 2016

If increasing the limit to a ridiculous number doesn't resolve the problem, that's a clue that you might actually have unbounded recursion (vec! won't be the culprit though). Try running cargo rustc -- -Zunstable-options -Ztrace-macros (might require nightly now) to see what macro expansions are actually happening.

@JoeyAcc
Copy link
Author

JoeyAcc commented Dec 28, 2016

@bluss Indeed I did, the attribute was #![...], not #[...].
@durka I don't have access to a nightly rustc atm, but it was decidedly vec! running into trouble, as it was the only macro left in the offending code.

@durka
Copy link
Contributor

durka commented Dec 28, 2016

I guess we'll need to see the code at this point. If a non-recursive macro can trigger the recursion limit, there is a problem somewhere. That said, I just checked and you don't need a nightly to use trace-macros as I suggested above, so you can try it.

@JoeyAcc
Copy link
Author

JoeyAcc commented Dec 29, 2016

Fair warning, it's pretty huge. But I can't and won't split it up since it's a parse tree resulting from a grammar I did not define (i.e. the xquery grammar).
Some info about the offending code snippet:

  1. Node is a struct
  2. The argument to Node::new() is of type NodeType, which is an enum
  3. Nodes have either children or data, but a valid parse tree node never has both.

Please let me know if you have any questions and I'll do my best to answer them.

        let expected = Node::new(ComparisonExpr).children(vec![
            Node::new(StringConcatExpr).children(vec![
                Node::new(RangeExpr).children(vec![
                    Node::new(AdditiveExpr).children(vec![
                        Node::new(MultiplicativeExpr).children(vec![
                            Node::new(UnionExpr).children(vec![
                                Node::new(IntersectExceptExpr).children(vec![
                                    Node::new(InstanceofExpr).children(vec![
                                        Node::new(TreatExpr).children(vec![
                                            Node::new(CastableExpr).children(vec![
                                                Node::new(CastExpr).children(vec![
                                                    Node::new(ArrowExpr).children(vec![
                                                        Node::new(UnaryExpr).children(vec![
                                                            Node::new(ValueExpr).children(vec![
                                                                Node::new(SimpleMapExpr).children(vec![
                                                                    Node::new(PathExpr).children(vec![
                                                                        Node::new(RelativePathExpr).children(vec![
                                                                            Node::new(StepExpr).children(vec![
                                                                                Node::new(PostfixExpr).children(vec![
                                                                                    Node::new(PrimaryExpr).children(vec![
                                                                                        Node::new(VarRef).children(vec![
                                                                                            Node::new(Operator).data("$"),
                                                                                            Node::new(VarName).data("CashFlowOperatingActivities"), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]),
            Node::new(Meta(Opt)).children(vec![
                Node::new(Meta(Seq)).children(vec![
                    Node::new(Whitespace).data(" "),
                    Node::new(Meta(Or)).children(vec![
                        Node::new(GeneralComp).children(vec![
                            Node::new(Operator).data("="), ]), ]),
                    Node::new(Whitespace).data(" \n"),
                    Node::new(StringConcatExpr).children(vec![
                        Node::new(RangeExpr).children(vec![
                            Node::new(AdditiveExpr).children(vec![
                                Node::new(MultiplicativeExpr).children(vec![
                                    Node::new(UnionExpr).children(vec![
                                        Node::new(IntersectExceptExpr).children(vec![
                                            Node::new(InstanceofExpr).children(vec![
                                                Node::new(TreatExpr).children(vec![
                                                    Node::new(CastableExpr).children(vec![
                                                        Node::new(CastExpr).children(vec![
                                                            Node::new(ArrowExpr).children(vec![
                                                                Node::new(UnaryExpr).children(vec![
                                                                    Node::new(Meta(MultStar)).children(vec![
                                                                        Node::new(Meta(Or)).children(vec![
                                                                            Node::new(Operator).data("-"), ]),
                                                                        Node::new(Meta(Opt)).children(vec![
                                                                            Node::new(Meta(Seq)).children(vec![
                                                                                Node::new(Whitespace).data(" "), ]), ]), ]),
                                                                    Node::new(ValueExpr).children(vec![
                                                                        Node::new(SimpleMapExpr).children(vec![
                                                                            Node::new(PathExpr).children(vec![
                                                                                Node::new(RelativePathExpr).children(vec![
                                                                                    Node::new(StepExpr).children(vec![
                                                                                        Node::new(PostfixExpr).children(vec![
                                                                                            Node::new(PrimaryExpr).children(vec![
                                                                                                Node::new(FunctionCall).children(vec![
                                                                                                    Node::new(EQName).children(vec![
                                                                                                        Node::new(QName).children(vec![
                                                                                                            Node::new(UnprefixedName).data("sum"), ]), ]),
                                                                                                    Node::new(ArgumentList).children(vec![
                                                                                                        Node::new(Delimiter).data("("),
                                                                                                        Node::new(Meta(Opt)).children(vec![
                                                                                                            Node::new(Meta(Seq)).children(vec![
                                                                                                                Node::new(Argument).children(vec![
                                                                                                                    Node::new(ExprSingle).children(vec![
                                                                                                                        Node::new(OrExpr).children(vec![
                                                                                                                            Node::new(AndExpr).children(vec![
                                                                                                                                Node::new(ComparisonExpr).children(vec![
                                                                                                                                    Node::new(StringConcatExpr).children(vec![
                                                                                                                                        Node::new(RangeExpr).children(vec![
                                                                                                                                            Node::new(AdditiveExpr).children(vec![
                                                                                                                                                Node::new(MultiplicativeExpr).children(vec![
                                                                                                                                                    Node::new(UnionExpr).children(vec![
                                                                                                                                                        Node::new(IntersectExceptExpr).children(vec![
                                                                                                                                                            Node::new(InstanceofExpr).children(vec![
                                                                                                                                                                Node::new(TreatExpr).children(vec![
                                                                                                                                                                    Node::new(CastableExpr).children(vec![
                                                                                                                                                                        Node::new(CastExpr).children(vec![
                                                                                                                                                                            Node::new(ArrowExpr).children(vec![
                                                                                                                                                                                Node::new(UnaryExpr).children(vec![
                                                                                                                                                                                    Node::new(ValueExpr).children(vec![
                                                                                                                                                                                        Node::new(SimpleMapExpr).children(vec![
                                                                                                                                                                                            Node::new(PathExpr).children(vec![
                                                                                                                                                                                                Node::new(RelativePathExpr).children(vec![
                                                                                                                                                                                                    Node::new(StepExpr).children(vec![
                                                                                                                                                                                                        Node::new(PostfixExpr).children(vec![
                                                                                                                                                                                                            Node::new(PrimaryExpr).children(vec![
                                                                                                                                                                                                                Node::new(VarRef).children(vec![
                                                                                                                                                                                                                    Node::new(Operator).data("$"),
                                                                                                                                                                                                                    Node::new(VarName).data("varArc_ConsolidatedCashFlowStatement_MsgConsolidatedSumOfChildrenParentDebit1_ChildrenOfCashFlowOperatingActivitiesCredit"), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]),
                                                                                                        Node::new(Delimiter).data(")"), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]),
                                Node::new(Meta(MultStar)).children(vec![
                                    Node::new(Meta(Opt)).children(vec![
                                        Node::new(Meta(Seq)).children(vec![
                                            Node::new(Whitespace).data(" \n"), ]), ]),
                                    Node::new(Meta(Or)).children(vec![
                                        Node::new(Operator).data("+"), ]),
                                    Node::new(Meta(Opt)).children(vec![
                                        Node::new(Meta(Seq)).children(vec![
                                            Node::new(Whitespace).data(" "), ]), ]),
                                    Node::new(MultiplicativeExpr).children(vec![
                                        Node::new(UnionExpr).children(vec![
                                            Node::new(IntersectExceptExpr).children(vec![
                                                Node::new(InstanceofExpr).children(vec![
                                                    Node::new(TreatExpr).children(vec![
                                                        Node::new(CastableExpr).children(vec![
                                                            Node::new(CastExpr).children(vec![
                                                                Node::new(ArrowExpr).children(vec![
                                                                    Node::new(UnaryExpr).children(vec![
                                                                        Node::new(ValueExpr).children(vec![
                                                                            Node::new(SimpleMapExpr).children(vec![
                                                                                Node::new(PathExpr).children(vec![
                                                                                    Node::new(RelativePathExpr).children(vec![
                                                                                        Node::new(StepExpr).children(vec![
                                                                                            Node::new(PostfixExpr).children(vec![
                                                                                                Node::new(PrimaryExpr).children(vec![
                                                                                                    Node::new(FunctionCall).children(vec![
                                                                                                        Node::new(EQName).children(vec![
                                                                                                            Node::new(QName).children(vec![
                                                                                                                Node::new(UnprefixedName).data("sum"), ]), ]),
                                                                                                        Node::new(ArgumentList).children(vec![
                                                                                                            Node::new(Delimiter).data("("),
                                                                                                            Node::new(Meta(Opt)).children(vec![
                                                                                                                Node::new(Meta(Seq)).children(vec![
                                                                                                                    Node::new(Argument).children(vec![
                                                                                                                        Node::new(ExprSingle).children(vec![
                                                                                                                            Node::new(OrExpr).children(vec![
                                                                                                                                Node::new(AndExpr).children(vec![
                                                                                                                                    Node::new(ComparisonExpr).children(vec![
                                                                                                                                        Node::new(StringConcatExpr).children(vec![
                                                                                                                                            Node::new(RangeExpr).children(vec![
                                                                                                                                                Node::new(AdditiveExpr).children(vec![
                                                                                                                                                    Node::new(MultiplicativeExpr).children(vec![
                                                                                                                                                        Node::new(UnionExpr).children(vec![
                                                                                                                                                            Node::new(IntersectExceptExpr).children(vec![
                                                                                                                                                                Node::new(InstanceofExpr).children(vec![
                                                                                                                                                                    Node::new(TreatExpr).children(vec![
                                                                                                                                                                        Node::new(CastableExpr).children(vec![
                                                                                                                                                                            Node::new(CastExpr).children(vec![
                                                                                                                                                                                Node::new(ArrowExpr).children(vec![
                                                                                                                                                                                    Node::new(UnaryExpr).children(vec![
                                                                                                                                                                                        Node::new(ValueExpr).children(vec![
                                                                                                                                                                                            Node::new(SimpleMapExpr).children(vec![
                                                                                            Node::new(PathExpr).children(vec![
                                                                                              Node::new(RelativePathExpr).children(vec![
                                                                                                Node::new(StepExpr).children(vec![
                                                                                                  Node::new(PostfixExpr).children(vec![
                                                                                                    Node::new(PrimaryExpr).children(vec![
                                                                                                      Node::new(VarRef).children(vec![
                                                                                                        Node::new(Operator).data("$"),
                                                                                                        Node::new(VarName).data("varArc_ConsolidatedCashFlowStatement_MsgConsolidatedSumOfChildrenParentDebit1_ChildrenOfCashFlowOperatingActivitiesDebit"), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]),
                                                  Node::new(Delimiter).data(")"), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]), ]);

@bluss
Copy link
Member

bluss commented Dec 29, 2016

It looks like you're using trailing commas. That's the case where the vec![] macro is recursive, it calls itself once. So you'd gain recursion levels by removing the trailing comma. Still, it's clear that it's not the vec macro that is to blame for using recursion levels, but the code itself is deeply nested.

@Mark-Simulacrum
Copy link
Member

Maybe we can make the recursion detection somehow detect nested macro invocations in the original source code. That case feels like something the author intended (and can't really be infinitely recursive, which AFAIK is what we're trying to prevent).

@bluss
Copy link
Member

bluss commented Dec 30, 2016

We need a way to reproduce this issue. I don't think recursion limit behaves in the way it has been described here.

@jjpe
Copy link

jjpe commented Jan 4, 2017

@bluss What form do you prefer?
A Gist? A cloneable Git project? I can have either of those done in very little time.

@bluss
Copy link
Member

bluss commented Jan 4, 2017

Gist is great!

@jjpe
Copy link

jjpe commented Jan 5, 2017

This gist demonstrates the issue nicely. I tested it on the playground and there I can reproduce the issue.

@bluss
Copy link
Member

bluss commented Jan 5, 2017

Thank you!

Raising the recursion limit removes the recursion limit error (playground link), so that seems to be as expected.

However, there is a regression here: It used to recommend raising the recursion limit, but it does not.

@durka
Copy link
Contributor

durka commented Jan 5, 2017 via email

@bluss
Copy link
Member

bluss commented Jan 5, 2017

The reporter seemed to say raising the recursion limit didn't help, but it does with the code in the gist. What I can see, the reported issue does not exist.

On raising the limit, I'm in favour.

@Mark-Simulacrum
Copy link
Member

Since we seem to believe that the author's problem was presumably fixable via recursion limit raising, but we do want to increase the macro recursion limit, I'm going to close this in favor of #22552, the original macro recursion limit raising issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants