Make Base.get_extension private to prevent user mistakes
#59708
Conversation
Added in #58108 without valid justification: something simply being documented in the manual is never any allowable argument for making something public. Being public is confusing users, since all of the other related infrastructure is also declared private (project files, `loaded_modules_array`, `PkgId`, `require`, etc.). Some of those do have legit uses in reflection testing, but we don't currently have a representation for this sort of private-except-reflection in most places (other than `Core.IR`). However is not allowed for packages to call `Base.get_extension`, since it requires accessing private implementation details of the target package to construct the arguments, and can run afoul of various implementation rules (world ages, module identity, precompile file consistency) that can lead to buggy execution. The correct way to use this functionality is with dispatch. Loosely: ```julia module Main function get_extension end end module MainPkgExt # extension module in Main using Other Main.get_extension(args...) = Other.call(args...) end ```
is this referring to the name of the extension? (surely the package's module itself is not private?) |
giordano
added
the
needs pkgeval
|
There are hundreds of packages using this already: https://juliahub.com/ui/Search?type=code&q=Base.get_extension |
KristofferC
removed
the
backport 1.12
|
Docstring can be updated to avoid user mistakes. |
|
That's fine. I am not proposing that we delete it, simply to communicate formally (to lint tools) that this is not an approved way to do things. There is no pkgeval required, since this annotation does not affect runtime. |
vtjnash
removed
the
needs pkgeval
Yes, I didn't not write reland or revert since this is the requested doc change. |
| Return the module for `extension` of `parent` or return `nothing` if the extension is not loaded. | ||
| Return the module for `extension` relative to `parent` or return `nothing` if the extension is not loaded. | ||
| This function is private, since the arguments to it are private implementation details. |
There was a problem hiding this comment.
In how far are the arguments private?
There was a problem hiding this comment.
The ext name is private, and the parent argument is used to extract PkgId which is internally also private
A module is private, other than its own name & the things it declares public internally: a module reference does not give you the public permission to do any arbitrary reflection with it (otherwise the whole public/private distinction is moot). This is a reflection function, and thus simply having the module reference does not imply you can call this function. |
|
It's up to package authors to decide whether they consider extension module names public or not. For packages that do, Also, this function can totally legitimately be used in the package defining the extension itself. |
|
They cannot, since this is private to Base. Particular not in the package itself, since that means the package loads its own extension, which will trigger at lot of issues |
Added in #58108 without valid justification: something simply being documented in the manual is never any allowable argument for making something public. Being public is confusing users, since all of the other related infrastructure is also declared private (project files,
loaded_modules_array,PkgId,require, etc.). Some of those do have legit uses in reflection testing, but we don't currently have a representation for this sort of private-except-reflection in most places (other thanCore.IR).However is not allowed for packages to call
Base.get_extension, since it requires accessing private implementation details of the target package to construct the arguments, and can run afoul of various implementation rules (world ages, module identity, precompile file consistency) that can lead to buggy execution.The correct way to use this functionality is with dispatch. Loosely: