[Swiftify] Do not swiftify non-Swift-like counted_by exprs #81395
Conversation
j-hui
requested review from
zoecarver,
egorzhdan,
Xazax-hun,
fahadnayyar,
susmonteiro,
hnrklssn,
beccadax and
ian-twilightcoder
as code owners
|
@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
j-hui
force-pushed
the
dont-swiftify-invalid-expr
branch
from
f585cf3 to
e5b1f4a
Compare
|
@swift-ci please test |
lib/ClangImporter/ImportDecl.cpp
Outdated
| bool VisitBin ## BINOP(const clang::BinaryOperator *binop) { \ | ||
| return this->Visit(binop->getLHS()) && this->Visit(binop->getRHS()); \ | ||
| } | ||
| SUPPORTED_BINOP(Add) |
There was a problem hiding this comment.
How can we ensure that we have comprehensively considered all the valid cases involving operators?
There was a problem hiding this comment.
We can't really, other than extensive testing. (E.g., I didn't realize we needed to whitelist ParenExpr in my initial draft of this.)
However, I do think this whitelist should cover the "obvious" cases, like basic arithmetic expressions, and even slightly weirder cases like bitwise arithmetic. The macro is not equipped to deal with anything much more complicated than that anyway, at least at the moment.
I'm erring on the side not Swiftifying imports, because if the user can't call the safe overload, they can always work around with the uglier unsafe code (what the macro should expand to anyway). The alternative, which is to produce a syntax error when encountering un-Swiftifiable code (what is happening now anyway), leads to un-compilable code from a macro they can't easily opt out of at a per-function granularity.
(Renamed from CATExprValidator)
|
@swift-ci please test |
__counted_by (and __sized_by) expressions can have arbitrary C syntax in them, such as:
When @_SwififyImport tries to generate Swift code for this, the expression
*lenleads 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