hackett icon indicating copy to clipboard operation
hackett copied to clipboard

refactor name mangling

Open AlexKnauth opened this issue 6 years ago • 2 comments

This PR makes way for other namespaces such as the module and signature namespaces in the module language @iitalics and I are working on.

It splits up the logic into these files:

  • hackett/private/mangle/mangle-string.rkt handles mangling/unmangling of strings
  • hackett/private/mangle/mangle-identifier.rkt handles identifiers and introducing scopes
  • hackett/private/mangle/mangle-import-export.rkt deals with the import and export objects used by require and provide transformers
  • hackett/private/mangle/mangle-reqprov.rkt defines the transformers, parsing the keywords #:no-introduce, #:prefix, and #:only to call different functions from the other files

AlexKnauth avatar Jun 04 '18 20:06 AlexKnauth

I’m sorry for not commenting on this sooner. I didn’t forget about it, but I should have gotten to it earlier. Hopefully two weeks late isn’t too late!

Anyway, to discuss the actual contents of this PR: bluntly, this seems like a lot of added complexity in the Hackett codebase for something that is used in precisely one place. That is to say, in the absence of multiple consumers within Hackett itself, it feels like strongly premature abstraction to me.

Now, I do understand that this is useful to you, since you’re adding more namespaces and actually do have more than one consumer, but I’m reluctant to take on the complexity overhead of abstracting this stuff out in the mainline repo until there is a need to. Does that make sense?

To put it another way: is there some reason these changes can’t stay on your branch for now? Or is there a reason you really need them merged into Hackett itself right now?

lexi-lambda avatar Jun 18 '18 16:06 lexi-lambda

What we currently have is a copy-pasted version of this code in hackett-module. Right now it "happens to work" because the mangling scheme "happens" to be compatible with hackett's mangling scheme. Having this be merged in to Hackett would change "happens to work because implementation details" into "works because interface".

But if "happens to work because implementation details" is good enough, this PR is not completely necessary.

AlexKnauth avatar Jun 18 '18 17:06 AlexKnauth