-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
not exporting symbols based on *package* breaks the hu.dwim codebase #12
Comments
I've thought about it a little. One way to fix it would be simply to replace That said, this means that defining a class can change the export of a symbol from a foreign package. This seems to be a broken design, but as I understand this is what |
that would still break the assumptions in our codebase. here's how i am (we were) thinking about packages and export/import: when we when i ask defclass* to export something, then i expect it to export the symbol in question from the package that the definition is in. one lib mutating the package of another is a very bad idea. it would mean that whether lib A is loaded or not (or when it is loaded -- even worse!) would influence the behavior of the rest of the system -- that's a recipe for undebuggable, potentially nondeterministic bugs. there was a time when we used IMO, there's nothing to be "fixed" in this is a fundamental design decision/assumption. "fixing" that would mean a very time consuming audit of the entire codebase... and as i remember, about half of our debugging time/effort in CL was spent on package issues. i remember i ended up even patching SBCL to emit warnings/errors with fully qualified symbol names... we even developed a joke/instinct that whenever something didn't make any sense, we concluded that it must be a package issue. does this make sense? |
I agree with your assessment, but then there is still a bug I believe:
when i ask defclass* to export something, then i expect it to export the symbol in question from the package that the *definition* is in. one lib mutating the package of another is a very
bad idea
Currently (without my patch), defclass-star tries to reexport symbols
(i.e. mutate) in foreign packages.
This is what my patch was meant to fix.
See the recipe in #7 for a concrete example.
The problem may lie in what you mean with "symbol from the package", because
"cl-user:foo" is not a symbol from "foo-package" and there is no way to
export it in "foo-package". (Indeed, "foo-package::foo" is a different symbol.)
Can you share a minimal example that exhibits how hu.dwim.walker fails
with the patch from #7 ?
|
i finally took the effort to understand your example, sorry for the lag. i've pushed a new (failing) test that formally captures your example: 2698bd9
not easily. but it's something like this: your change filters everything in
my gut reaction is to make sure that the symbol is present in package before attempting to export it by first importing it if needed. i'm not sure about its implications though. hrm, actually... what if all remains as it is now? and we just say that it's the users' responsibility to set up their packages so that this doesn't happen? i'm leaning towards this, because this is the solution with the least probability of surprises, and i don't think it requires much extra effort in a lot of use-cases. what do you think? |
i've pushed a commit that reverts it. closing it, but feel free to reopen if the current state is still not satisfying. |
my reasoning: in my experience package related bugs are part of the class of bugs that eat up most of my time as a CL programmer. my general strategy when it comes to packages is to try to keep surprises to the minimum. things should rather fail early and loud than introduce a subtle bug that takes ages to catch. |
commit 39d458f (from PR #7) breaks the hu.dwim codebase (i reverted it).
it fails to export the NAME-OF symbol from hu.dwim.walker, due to which an originally single generic method got split into two, the other one being a private/isolated NAME-OF internal to hu.dwim.meta-model.
or to put it another way: our codebase expects that symbols get exported, regardless of what is their home package [EDIT: i.e. the CL export semantics].
The text was updated successfully, but these errors were encountered: