🍒 [6.2] [Swiftify] Do not swiftify non-Swift-like counted_by exprs #81397
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
@swift-ci please test |
__counted_by (and __sized_by) expressions can have arbitrary C syntax
in them, such as:
void foo(int * __counted_by(*len) p, int *len);
When @_SwififyImport tries to generate Swift code for this, the
expression `*len` leads to a syntax error, since it isn't valid Swift.
This patch adds a check to ensure we only attach the Swiftify macro to
__counted_by expressions that are also syntactically valid in Swift.
rdar://150956352
(cherry picked from commit e5b1f4a)
j-hui
force-pushed
the
cherry-pick-6.2/dont-swiftify-invalid-expr
branch
from
b290743 to
8be78da
Compare
Contributor
Author
|
@swift-ci please test |
DougGregor
approved these changes
May 9, 2025
Member
DougGregor
left a comment
DougGregor
left a comment
There was a problem hiding this comment.
The naming issue isn't a critical one, we can deal with it on main.
| }; | ||
| } // namespace | ||
|
|
||
| static bool SwiftifiableCAT(const clang::CountAttributedType *CAT) { |
Member
There was a problem hiding this comment.
Could we get some more descriptive names here? I see what "CAT" is an acronym for here, but it isn't quite discoverable. CountedByExpressionValidator, perhaps?
Contributor
Author
There was a problem hiding this comment.
Yeah that's a fair point! I'll address that in my PR on main.
j-hui
commented
May 9, 2025
| } // namespace | ||
|
|
||
| namespace { | ||
| /// Look for any side effects within a Stmt. |
Contributor
Author
There was a problem hiding this comment.
Oops, I just realized that I forgot to remove this stray comment that got copied over when I was setting up the StmtVisitor boilerplate 😮💨
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Explanation: This patch adds a check to ensure we only attach the Swiftify macro to
__counted_byand__sized_byexpressions that are also syntactically valid in Swift.Issue: rdar://150956352
Risk: low
Testing: added tests
Original PRs: #81395
Reviewer: @hnrklssn