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.

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

Ruby: rename max_recursion_depth to recursion_limit #9486

Merged
merged 1 commit into from Feb 9, 2022

Conversation

acozzette
Copy link
Member

This will help keep the terminology consistent with the other language
implementations.

This will help keep the terminology consistent with the other language
implementations.
@haberman
Copy link
Member

haberman commented Feb 9, 2022

Thanks Adam!

One related thing that would be helpful, if you have the time, is to verify that the recursion limit here has the same semantics as in C++. In other words, it would be nice if there weren't off-by-one errors, where using the same recursion limit in two languages yielded slightly different results.

@acozzette
Copy link
Member Author

Let me see about adding a conformance test, since that seems like it would be the most reliable way to make sure languages are enforcing the same recursion limit.

@acozzette acozzette merged commit d5ef16c into protocolbuffers:master Feb 9, 2022
@acozzette acozzette deleted the recursion-limit branch February 9, 2022 18:21
@aleksandarabas
Copy link

NO NO NO, some people will mistake it as a rushed abbreviation for recursion limit case.
But the stack depth limit which is imposed on the recursion process must be clearly distinguished from whatever custom property that doesn't require privilege escalation. This will help avoid ww3. thank you

@aleksandarabas
Copy link

Let me see about adding a conformance test, since that seems like it would be the most reliable way to make sure languages are enforcing the same recursion limit.

This is very plainly why you struggle with the phenomena of small government republicans. :D

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

Successfully merging this pull request may close these issues.

None yet

4 participants