Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: Support #![recursion_limit] attribute #11360

Merged
merged 3 commits into from
Jan 28, 2022
Merged

feat: Support #![recursion_limit] attribute #11360

merged 3 commits into from
Jan 28, 2022

Conversation

WaffleLapkin
Copy link
Member

Peek 2022-01-28 02-33

Resolves #8640

@matklad thanks, for the instructions, they were very helpful :)

@lnicola
Copy link
Member

lnicola commented Jan 28, 2022

😍 does this fix #4243 (comment) too?

@lnicola
Copy link
Member

lnicola commented Jan 28, 2022 •
edited
Loading

I think you need to rebase on top of master to fix the formatting error.

@WaffleLapkin
Add crate_limits query to SourceDatabase
c932ca5 
This allows fetching crate limits like `recursion_limit`. The
implementation is currently dummy and just returns the defaults.

Future work: Use this query instead of the hardcoded constant.

Future work: Actually implement this query by parsing
`#![recursion_limit = N]` attribute.
@WaffleLapkin
Use crate_limits query in macro expansion
81211f5
@WaffleLapkin
feat: Honor recursion limit configuration
6d18c5b 
This patch makes RA understand `#![recursion_limit = "N"]` annotations.

- `crate_limits` query is moved to `DefDatabase`
- `DefMap` now has `recursion_limit: Option<u32>` field
@WaffleLapkin
Copy link
Member Author

@lnicola async-std is still broken, it seems. Not sure what exactly happens to that cursed macro, but yeah, next is still unresolved by RA.

@lnicola
Copy link
Member

lnicola commented Jan 28, 2022 

// 5 <-- increment this if you've looked at `async_std::extension_trait!` without figuring out why it fails

@lnicola
Copy link
Member

lnicola commented Jan 28, 2022

It actually starts working for me if I increase the limit:

image

But it's sooooooooo slow 😢.

I didn't test your PR yet.

@lnicola
Copy link
Member

lnicola commented Jan 28, 2022

Yeah, your PR fixes #4243 for me, at least in the tcp-echo example.

@WaffleLapkin
Copy link
Member Author

@lnicola wait 🤔

If RA ignores the limit (without this PR anyway) how can increasing it help? And since cargo check works I'd assume that the limit is already high enough...

Yeah, your PR fixes #4243 for me, at least in the tcp-echo example.

The weird part is that it doesn't for me, lol. I wander what's up with that 🤔

@lnicola
Copy link
Member

lnicola commented Jan 28, 2022

RA used to bail out after 128 expansions, now your PR makes it run to the end.

@Veykril
Copy link
Member

Veykril commented Jan 28, 2022

bors r+

@bors
Copy link
Contributor

bors bot commented Jan 28, 2022

Build succeeded:

@bors bors bot merged commit cd6521e into rust-lang:master Jan 28, 2022
@lnicola lnicola mentioned this pull request Jan 28, 2022
Closed
@WaffleLapkin WaffleLapkin deleted the recursion_limit branch January 28, 2022 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lnicola lnicola lnicola approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Configure recursion limit for macro expansion
3 participants
@WaffleLapkin @lnicola @Veykril