Skip to content

suggest doubling recursion limit in more situations - #39655

Merged
bors merged 2 commits into
rust-lang:masterfrom
durka:recursion-limit-suggestion
Mar 2, 2017
Merged

suggest doubling recursion limit in more situations#39655
bors merged 2 commits into
rust-lang:masterfrom
durka:recursion-limit-suggestion

Conversation

@durka

@durkadurka commented Feb 8, 2017

Copy link
Copy Markdown
Contributor

Fixes#38852.

r? @bluss

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This duplicate error shouldn't be here and yet it is. cc #38940@jseyfried

@durka
durkaforce-pushed the recursion-limit-suggestion branch 2 times, most recently from fbaef50 to 422b789CompareFebruary 9, 2017 06:35
@durka
durkaforce-pushed the recursion-limit-suggestion branch from 422b789 to b4993ecCompareFebruary 9, 2017 06:40
@durka

Copy link
Copy Markdown
ContributorAuthor

r?

ei.callee.name()));
err.note(&format!(
"consider adding a `#![recursion_limit=\"{}\"]` attribute to your crate",
suggested_limit));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be .help() too or is there a reason it can't?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think it can be.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need update to help I think

@bluss

bluss commented Feb 28, 2017

Copy link
Copy Markdown
Contributor

Thanks. I'll approve this after checking with rustc devs

@eddyb

Copy link
Copy Markdown
Contributor

r? @nikomatsakis

@durka
durkaforce-pushed the recursion-limit-suggestion branch from 80cd3cf to e1773cbCompareFebruary 28, 2017 18:57
Comment threadsrc/libsyntax/ext/base.rs Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should factor this code into some sort of helper?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like it'd also be nice to make this into an error code ("recursion limit reached") with a note giving the specific details. That way, we could give an extended error description.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where should I put the helper and diagnostic code? Note that the PR is split across libsyntax and librustc_typeck.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. Can we only put diagnostic codes into rustc crates? I'm not sure.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are diagnostic codes in libsyntax. Can I define it there and use the same one from librustc_typeck?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meh. I think we should improve this, but it's not that big a deal.

@nikomatsakis

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Mar 1, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit e1773cb has been approved by nikomatsakis

@bors

bors commented Mar 2, 2017

Copy link
Copy Markdown
Collaborator

⌛ Testing commit e1773cb with merge f46ed40...

@bors

bors commented Mar 2, 2017

Copy link
Copy Markdown
Collaborator

💔 Test failed - status-appveyor

@durka
durkaforce-pushed the recursion-limit-suggestion branch from e1773cb to 6e259dcCompareMarch 2, 2017 07:11
@durka

durka commented Mar 2, 2017

Copy link
Copy Markdown
ContributorAuthor

Should be fixed.

@bluss

bluss commented Mar 2, 2017

Copy link
Copy Markdown
Contributor

@bors r=nikomatsakis

@bors

bors commented Mar 2, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit 6e259dc has been approved by nikomatsakis

@bors

bors commented Mar 2, 2017

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 6e259dc with merge 907f26d...

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 2, 2017
…=nikomatsakis
suggest doubling recursion limit in more situations
Fixesrust-lang#38852.
r? @bluss
@frewsxcvfrewsxcv mentioned this pull request Mar 2, 2017
@frewsxcv

Copy link
Copy Markdown
Contributor

@bors retry

@bors

bors commented Mar 2, 2017

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 6e259dc with merge 5907ed6...

bors added a commit that referenced this pull request Mar 2, 2017
suggest doubling recursion limit in more situations
Fixes#38852.
r? @bluss
@bors

bors commented Mar 2, 2017

Copy link
Copy Markdown
Collaborator

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 5907ed6 to master...

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@durka@bluss@eddyb@nikomatsakis@bors@frewsxcv