Skip to content

Improve documentation and error messages for safe initialization #15659

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

Merged
merged 7 commits into from
Jul 13, 2022

Conversation

liufengyun
Copy link
Contributor

Improve documentation and error messages for safe initialization

@michelou
Copy link
Contributor

michelou commented Jul 13, 2022

@liufengyun Here are 3 more changes to Markdown file safe-initialization.md

Old New
The three principles above The four principles above
Another exception too this rule Another exception to this rule
essentially invovles close essentially involves close

Copy link
Contributor

@olhotak olhotak left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM.

- `v` is `Fun(e, V, C)` and calling the function encounters no errors and the
function return value is _effectively hot_.
- `ThisRef` is _effectively hot_.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
In the `ThisRef` case, local reasoning ensures that all fields assigned implies that all fields themselves contain effectively hot values.
In the `Warm` case, if the value `v` leaks outside the analysis scope, the unanalyzed code could instantiate inner classes of `v`, call methods on `v`, or read fields of `v`. The conditions of the `Warm` case ensure that these actions do not lead to accessing a cold object.
In the `Fun` case, ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we can add the justification later --- they are not immediately understandable to readers and can itself be an obstacle in reading the docs.

Copy link
Contributor Author

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

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

Apply suggested changes

Copy link
Contributor Author

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

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

Apply suggested changes

liufengyun and others added 3 commits July 13, 2022 18:58
Co-authored-by: Ondřej Lhoták <olhotak@uwaterloo.ca>
Co-authored-by: Ondřej Lhoták <olhotak@uwaterloo.ca>
@liufengyun
Copy link
Contributor Author

Thanks @michelou , I fixed two of them. Regarding

The three principles above

The usage of "three" is intended here and it's not a typo.

@liufengyun liufengyun marked this pull request as ready for review July 13, 2022 17:51
@liufengyun liufengyun enabled auto-merge July 13, 2022 18:53
@liufengyun liufengyun merged commit 7d43b44 into scala:main Jul 13, 2022
@liufengyun liufengyun deleted the init-docs branch July 13, 2022 19:37
@Kordyjan Kordyjan added this to the 3.2.1 milestone Aug 1, 2023
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.

4 participants