Skip to content

Prevent PrintOptions from being implicitly copied #82008

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 1 commit into from
Jun 6, 2025

Conversation

rjmccall
Copy link
Contributor

@rjmccall rjmccall commented Jun 5, 2025

NFC except that I noticed a bug by inspection where we suppress @escaping when print enum element types. Since this affects recursive positions, we end up suppressing @escaping in places we shouldn't. This is unlikely to affect much real code, but should still obviously be fixed.

The new design is a little sketchy in that we're using const to prevent direct use (and allow initialization of const & parameters) but still relying on modification of the actual object. Essentially, we are treating the const-ness of the reference as a promise to leave the original value in the object after computation rather than a guarantee of not modifying the object. This is okay --- a temporary bound to a const reference is still a non-const object formally and can be modified without invoking UB --- but makes me a little uncomfortable.

@rjmccall
Copy link
Contributor Author

rjmccall commented Jun 5, 2025

@swift-ci Please test

NFC *except* that I noticed a bug by inspection where we suppress
`@escaping` when print enum element types. Since this affects
recursive positions, we end up suppressing `@escaping` in places
we shouldn't. This is unlikely to affect much real code, but should
still obviously be fixed.

The new design is a little sketchy in that we're using `const` to
prevent direct use (and allow initialization of `const &` parameters)
but still relying on modification of the actual object.  Essentially,
we are treating the `const`-ness of the reference as a promise to leave
the original value in the object after computation rather than a
guarantee of not modifying the object. This is okay --- a temporary
bound to a `const` reference is still a non-`const` object formally
and can be modified without invoking UB --- but makes me a little
uncomfortable.
@rjmccall rjmccall force-pushed the dont-copy-print-options branch from e1bd406 to b3493bf Compare June 5, 2025 16:52
@rjmccall
Copy link
Contributor Author

rjmccall commented Jun 5, 2025

@swift-ci Please test

@rjmccall
Copy link
Contributor Author

rjmccall commented Jun 6, 2025

@swift-ci Please smoke test Linux

@rjmccall
Copy link
Contributor Author

rjmccall commented Jun 6, 2025

@swift-ci Please smoke test macOS

@rjmccall
Copy link
Contributor Author

rjmccall commented Jun 6, 2025

Verified that the test failures appear to be existing failures block full test runs

@rjmccall
Copy link
Contributor Author

rjmccall commented Jun 6, 2025

@swift-ci Please smoke test Linux

@rjmccall rjmccall merged commit 237e617 into swiftlang:main Jun 6, 2025
3 of 5 checks passed
@rjmccall rjmccall deleted the dont-copy-print-options branch June 6, 2025 21:06
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.

1 participant