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

Avoid unnecessary Docs.META initializations #48469

Merged
merged 1 commit into from
Feb 1, 2023

Conversation

topolarity
Copy link
Member

If the target module does not have a Docs.META dict (e.g. if --strip-metadata is used), Docs.meta() has the side effect of creating a new IdDict and initializing the Docs.META field of the target module. We need to avoid eval'ing into modules after they've been closed, so the init should be skipped in methods that do not mutate the new IdDict.

Resolves #48390.

Co-authored-by: @sjkelly

@KristofferC KristofferC added docsystem The documentation building system backport 1.9 Change should be backported to release-1.9 labels Jan 31, 2023
Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels a bit awkward to duplicate this everywhere. Could we add a boolean argument to meta that defines whether to call initmeta or to return some sort of empty object instead?

If the target module does not have a Docs.META dict (e.g. if
`--strip-metadata` is used), `Docs.meta()` has the side effect of
creating a new IdDict and initializing the Docs.META field of the target
module.

We need to avoid eval'ing into modules after they've been closed, so for
methods that do not mutate the new IdDict we should avoid the init.

Resolves JuliaLang#48390.

Co-authored-by: Steve Kelly <kd2cca@gmail.com>
@KristofferC KristofferC mentioned this pull request Feb 1, 2023
35 tasks
@vtjnash vtjnash merged commit 798b589 into JuliaLang:master Feb 1, 2023
@topolarity topolarity deleted the docs-meta-init branch February 1, 2023 19:29
KristofferC pushed a commit that referenced this pull request Feb 5, 2023
If the target module does not have a Docs.META dict (e.g. if
`--strip-metadata` is used), `Docs.meta()` has the side effect of
creating a new IdDict and initializing the Docs.META field of the target
module.

We need to avoid eval'ing into modules after they've been closed, so for
methods that do not mutate the new IdDict we should avoid the init.

Resolves #48390.

Co-authored-by: Steve Kelly <kd2cca@gmail.com>
(cherry picked from commit 798b589)
@KristofferC KristofferC added backport 1.8 Change should be backported to release-1.8 and removed backport 1.9 Change should be backported to release-1.9 labels Feb 7, 2023
KristofferC pushed a commit that referenced this pull request Feb 8, 2023
If the target module does not have a Docs.META dict (e.g. if
`--strip-metadata` is used), `Docs.meta()` has the side effect of
creating a new IdDict and initializing the Docs.META field of the target
module.

We need to avoid eval'ing into modules after they've been closed, so for
methods that do not mutate the new IdDict we should avoid the init.

Resolves #48390.

Co-authored-by: Steve Kelly <kd2cca@gmail.com>
(cherry picked from commit 798b589)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.8 Change should be backported to release-1.8 docsystem The documentation building system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LoadError during package precompilation when using @doc to copy docstrings
3 participants