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

Add fields for Scanners in DependentTypesHelper #4253

Merged
merged 3 commits into from
Feb 9, 2021

Conversation

smillst
Copy link
Member

@smillst smillst commented Feb 8, 2021

Scanners have HashMaps that can use a lot of memory, so it's better to use fields rather than creating a new object for each use. #3851 shows how a scanner caused problems else where. (#4252 is a new issue to document this problem and find more scanners to move to fields.)

Copy link
Member

@mernst mernst left a comment

Choose a reason for hiding this comment

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

Thanks for these efficiency improvements. I have just two questions about documentation.

/**
* A scanner that standardizes Java expression strings in dependent type annotations. Call
* {@link StandardizeTypeAnnotator#init(JavaExpressionContext, TreePath, boolean, boolean)}
* before using.
Copy link
Member

Choose a reason for hiding this comment

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

Please clarify whether this means to call init once, or to call init before each use.


/**
* Copies annotations that might have been viewpoint adapted from the visited type (the first
* formal parameter of the visit method) to the second formal parameter.
Copy link
Member

Choose a reason for hiding this comment

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

Is this the visit method of this class, or of the field?

@mernst mernst assigned smillst and unassigned mernst Feb 9, 2021
@smillst smillst merged commit 3daf370 into typetools:master Feb 9, 2021
@smillst smillst deleted the dth-scanners branch February 9, 2021 18:49
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