Skip to content

Fixes #20348; only respect the recursion limit if the symbol's generic type has been generated by the compiler #20377

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
Merged
merged 1 commit into from
Sep 22, 2022
Merged

Fixes #20348; only respect the recursion limit if the symbol's generic type has been generated by the compiler #20377

merged 1 commit into from
Sep 22, 2022

Conversation

@deech
Copy link
Contributor

@deech deech commented Sep 17, 2022

No description provided.

@deech
Copy link
Contributor Author

deech commented Sep 17, 2022

It looks like this change is causing timeouts on CI. Can I get some help diagnosing which test is taking too long? I don't know where to look for test timing logs.

@deech deech changed the title Fixes #20348; reset the type checker recursion limit for each object field Fixes #20348; only respect the recursion limit if the symbol's generic type has been generated by the compiler Sep 18, 2022
@deech
Copy link
Contributor Author

deech commented Sep 18, 2022

Now that CI passes unless people have an issue with the new flag this an incremental improvement that can be merged. In the long term it would be good to talk about

  • a re-design that avoids recursionLimit
  • setting the recursion limit to 2, I'm not sure what the difference is between bailing out at 2 vs. 100.

@Varriount Varriount requested a review from Araq September 19, 2022 00:44
@Varriount Varriount added the Requires Araq To Merge PR should only be merged by Araq label Sep 19, 2022
@deech
Copy link
Contributor Author

deech commented Sep 22, 2022

This PR can be merged. We talked about it on Discord and Araq was good with this change.

@Varriount Varriount merged commit be4bd8a into nim-lang:devel Sep 22, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Sep 22, 2022

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from be4bd8a

Hint: mm: orc; threads: on; opt: speed; options: -d:release
164064 lines; 14.425s; 842.332MiB peakmem

@deech deech deleted the devel branch October 30, 2022 18:36
capocasa pushed a commit to capocasa/Nim that referenced this pull request Mar 31, 2023
@deech @capocasa
Fixes nim-lang#20348; only respect the recursion limit if the symbol…
508066a 
…'s generic type has been generated by the compiler (nim-lang#20377)

Fixes nim-lang#20348
narimiran pushed a commit that referenced this pull request May 12, 2023
@deech @narimiran
Fixes #20348; only respect the recursion limit if the symbol's gener…
cb36bfd 
…ic type has been generated by the compiler (#20377)

Fixes #20348

(cherry picked from commit be4bd8a)
narimiran pushed a commit that referenced this pull request May 15, 2023
@deech @narimiran
Fixes #20348; only respect the recursion limit if the symbol's gener…
e3275b2 
…ic type has been generated by the compiler (#20377)

Fixes #20348

(cherry picked from commit be4bd8a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Araq Araq Awaiting requested review from Araq

Assignees

No one assigned

Labels

Requires Araq To Merge PR should only be merged by Araq

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@deech @Varriount