Skip to content

Swift: Models for String methods involving closures. #14578

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
Nov 2, 2023

Conversation

geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Oct 24, 2023

Add flow models for String methods involving closures, such as String.withUTF8(_:). Most already had test coverage, but I did have to add tests for withMutableCharacters(_:) and an additional test for String(unsafeUninitializedCapacity:initializingUTF8With:) to show that the model is working despite other issues in the existing test. Unfortunately the diff of the test is also polluted by a renumbering.

@geoffw0 geoffw0 added the Swift label Oct 24, 2023
@geoffw0 geoffw0 requested a review from a team as a code owner October 24, 2023 15:20
@geoffw0 geoffw0 requested a review from rdmarsh2 October 24, 2023 15:29
@geoffw0
Copy link
Contributor Author

geoffw0 commented Oct 25, 2023

I fixed a merge conflict.

Comment on lines +71 to +72
";StringProtocol;true;withCString(_:);;;Argument[-1];Argument[0].Parameter[0].CollectionElement;taint",
";StringProtocol;true;withCString(_:);;;Argument[0].ReturnValue;ReturnValue;value",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should there be a step from the mutated parameter of the callback to the this parameter of the method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed on the other comment.

Comment on lines +136 to +137
";String;true;withPlatformString(_:);;;Argument[-1];Argument[0].Parameter[0].CollectionElement;taint",
";String;true;withPlatformString(_:);;;Argument[0].ReturnValue;ReturnValue;value",
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question here.

Copy link
Contributor Author

@geoffw0 geoffw0 Oct 25, 2023

Choose a reason for hiding this comment

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

I don't think you can mutate the string with these particular methods. I'll check...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for taking a while to get back to this. I think it's only possible to modify the object using the methods marked mutating, that is withMutableCharacters and withUTF8 in this PR (and I'm not sure it's practically possible with the latter, see the doc).

Some experimentation here - notice that the changes are never reflected back in s.

@rdmarsh2 rdmarsh2 merged commit 81d77bf into github:main Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants