[ClangImporter] Do not import DynamicRangePointerType and ValueTerminatedType #77910
Conversation
rapidsna
requested review from
zoecarver,
hyp,
egorzhdan,
beccadax,
ian-twilightcoder and
Xazax-hun
as code owners
|
@swift-ci test |
rapidsna
requested review from
DougGregor,
devincoughlin,
hnrklssn,
patrykstefanski,
danliew-apple and
bnbarham
…atedType DynamicRangePointerType and ValueTerminatedType are new Clang types for -fbounds-safety, annotated with the 'ended_by' and the 'terminated_by' attributes. This adds visitors for these types in ClangImporter so Swift still builds with Clang that has these new types.
rapidsna
force-pushed
the
handle_clang_bounds_safety_types
branch
from
6b5262c to
866a834
Compare
| // DynamicRangePointerType is a clang type representing a pointer with | ||
| // an "ended_by" type attribute for -fbounds-safety. For now, we don't | ||
| // import these into Swift. | ||
| return Type(); |
There was a problem hiding this comment.
Shouldn't we recurse on the wrapped type here, since it's okay to ignore the ended_by attribute? We'll be gathering this information separately.
There was a problem hiding this comment.
Right. Though this is to make them consistent with how CountAttributedType is handled in this codebase and downstream. @hnrklssn's WIP PR will fix the visitor for CountAttributedType later.
There was a problem hiding this comment.
If you prefer, I can make DynamicRangePointerType and ValueTerminatedType to be handled like other SUGAR_TYPE here. Please let me know.
There was a problem hiding this comment.
I would prefer that they be handled like other SUGAR_TYPE. The bounds-safety annotations shouldn't break interop, even when they aren't being used to improve it.
There was a problem hiding this comment.
@hnrklssn You will have to remove SUGAR_TYPE(CountAttributed) in your patch and reintroduce your Visit CountAttributedType. I fixed it anyway knowing that your patch won't be landing in the release branch, but this will be.
There was a problem hiding this comment.
@DougGregor actually, I couldn't land SUGAR_TYPE(DynamicRangePointer) because DynamicRangePointerType doesn't exist in Clang yet. I will revert back to return Type() and then open up a follow up patch to fix that right after I land swiftlang/llvm-project#9665.
swift/lib/ClangImporter/ImportType.cpp:943:5: error: member access into incomplete type 'const clang::DynamicRangePointerType'
943 | SUGAR_TYPE(DynamicRangePointer)
| ^
swift/lib/ClangImporter/ImportType.cpp:930:24: note: expanded from macro 'SUGAR_TYPE'
930 | return Visit(type->desugar()); \
| ^
|
@swift-ci test |
|
@DougGregor Thanks for the quick review! |
|
@swift-ci test |
rapidsna
changed the title
This reverts commit 95904eb.
rapidsna
changed the title
|
@swift-ci test |
|
Linux test failure seemed unrelated: |
|
@swift-ci test |
DynamicRangePointerType and ValueTerminatedType are new Clang types for -fbounds-safety, annotated with the 'ended_by' and the 'terminated_by' attributes. This adds visitors for these types in ClangImporter so Swift continues to build with the Clang version that will introduce these new types.
This should unblock swiftlang/llvm-project#9665