[cxx-interop] Do not import template type arguments #84866
Conversation
Prior to this patch, we eagerly imported template type arguments. A consequence of doing so is that we over-instantiate (potentially unused) type arguments, which causes unnecessary errors. The only apparent use we have for these type arguments are to check whether they are unsafe, so that we can mark the instantiated type as unsafe as well... even if the instantiated type does not use its unsafe template type argument at all. Note that using un-instantiatable types in template type arguments is supported in C++. The test case included in this patch validates this behavior, for both missing member and incomplete type errors. Note, also, that as of this patch, dumping the module interface of CxxModule actually causes swift-ide-test to emit compiler errors, since it tries to instantiate the invalid types MissingMember<Empty> and IncompleteField<Incomplete>. However, these errors are simply swallowed by swift-ide-test, so they should be harmless, though we should probably get rid of them entirely in future work. rdar://145238539
j-hui
requested review from
Xazax-hun,
beccadax,
egorzhdan,
fahadnayyar,
hnrklssn,
ian-twilightcoder,
susmonteiro and
zoecarver
as code owners
|
@swift-ci please test |
|
Do I understand correctly that when the template is instantiated, if the template argument is used inside the class, it is still imported at that point? Do we want to also remove the logic for marking templated types as unsafe if the type parameter is unsafe? Does it risk being inconsistent, depending on if full importing is triggered or not? |
I think we don't want to do that. |
Doing so preserves ClangImporter's behavior first introduced in f12b48a, but do so without relying on importing the type argument (since that can lead to template over-instantiation). This patch also promotes the logic of hasUnsafeType() to a standalone SimpleRequest, to provide a QualType-typed entry point for evaluating the safety of a Clang entity.
I didn't realize this was the safety model in Swift! I added back the behavior, and checked in some test code to ensure this is the case. |
|
@swift-ci please test |
…ptor This will hopefully improve the cache hit rate.
importAsUnsafe was removed in c859557
|
@swift-ci Please test |
3 participants
Prior to this patch, we eagerly imported template type arguments. A consequence of doing so is that we over-instantiate (potentially unused) type arguments, which causes unnecessary errors.
The only apparent use we have for these type arguments are to check whether they are unsafe, so that we can mark the instantiated type as unsafe as well... even if the instantiated type does not use its unsafe template type argument at all.
Note that using un-instantiatable types in template type arguments is supported in C++. The test case included in this patch validates this behavior, for both missing member and incomplete type errors.
Note, also, that as of this patch, dumping the module interface of CxxModule actually causes swift-ide-test to emit compiler errors, since it tries to instantiate the invalid types MissingMember and IncompleteField. However, these errors are simply swallowed by swift-ide-test, so they should be harmless, though we should probably get rid of them entirely in future work.
rdar://145238539