Skip to content

Add back in helper function semantic::Policy::_translate_pk #611

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

Conversation

tcharding
Copy link
Member

Recently while removing recursion I mistakenly removed the _translate_pk function forgetting that this was a trick to prevent compilation explosion caused by the generics.

@tcharding tcharding changed the title Add back in helper function _translate_pk Add back in helper function semantic::Policy::_translate_pk Oct 11, 2023
@tcharding tcharding force-pushed the 10-11-add-back-helper-function branch from 374324a to 8ac9baa Compare October 11, 2023 02:32
@tcharding
Copy link
Member Author

Needs #613

Recently while removing recursion I mistakenly removed the
`_translate_pk` function forgetting that this was a trick to prevent
compilation explosion caused by the generics.
@tcharding tcharding force-pushed the 10-11-add-back-helper-function branch from 8ac9baa to aeb1a7c Compare October 11, 2023 21:28
@apoelstra
Copy link
Member

I don't understand this PR.

@tcharding
Copy link
Member Author

tcharding commented Oct 12, 2023

There was a post sometime recently by @sanket1729 about how we were getting an explosion of code once all the generics were expanded [0]. And something about the helper function not needing to be re-compiled each time ...

Can you help me out Sanket, I clearly did not fully grok it when I read it.

[0] Is "expanded" the correct term?

@apoelstra
Copy link
Member

There are a couple things that I think you might mean:

  1. When we take generic parameters that can take a T or &mut T and have functions that call each other with a call tree a few layers deep, we wind up compiling functions for T, &mut T, &mut mut T, etc., which is wasteful. This happens with the rust-bitcoin encoding stuff but never here.
  2. When we call functions from themselves we get the same thing but infinitely many times and compilation fails. No longer an issue here.
  3. When a function takes a generic AsRef (for example) and all we do with the generic argument is call .as_ref(), then everything after that line uses concrete types, and it's wasteful to compile the whole function multiple times with different generic parameters. Better to have a helper function with concrete types, then the real function calls as_ref() before calling that.

I think you are trying to address issue (3). But the current code here is a no-op. You have two functions with identical signatures, with identical generics, and one is calling the other.

@tcharding
Copy link
Member Author

Thanks for the explanation, should have trusted myself more originally when I removed the helper because I couldn't see it was achieving anything. Closing.

@tcharding tcharding closed this Oct 12, 2023
@tcharding
Copy link
Member Author

I dug through the git index and the _translate_pk function was introduced when translate_pk had a different signature, when we changed it we didn't notice that the helper was no longer needed. The "mysterious" reason I vaguely thought existed must be some unrelated thing floating around in my brain.

For interest it was introduced in commit ef1d69c81e3acc34ed2ce71b6484d719cc631ab6

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

Successfully merging this pull request may close these issues.

2 participants