From b40b1be41dc84809cbcd2e855f491121f7e25580 Mon Sep 17 00:00:00 2001 From: Arnold Schwaighofer Date: Mon, 11 May 2020 07:56:01 -0700 Subject: [PATCH 01/15] Existentials don't always conform to a protocol via an abstract conformance So it is not always correct to use the current conformance when substituting. rdar://62894495 --- include/swift/AST/GenericSignatureBuilder.h | 6 +-- include/swift/AST/Type.h | 5 +- lib/AST/GenericSignatureBuilder.cpp | 13 +++++ lib/AST/ProtocolConformance.cpp | 6 --- lib/AST/Type.cpp | 16 +++++++ .../SILCombiner/SILCombinerApplyVisitors.cpp | 3 +- test/SILOptimizer/inline_generics.sil | 48 ++++++++++++++++++- 7 files changed, 79 insertions(+), 18 deletions(-) diff --git a/include/swift/AST/GenericSignatureBuilder.h b/include/swift/AST/GenericSignatureBuilder.h index 6243b843959c2..21b9c60d65a04 100644 --- a/include/swift/AST/GenericSignatureBuilder.h +++ b/include/swift/AST/GenericSignatureBuilder.h @@ -519,11 +519,7 @@ class GenericSignatureBuilder { ProtocolConformanceRef operator()(CanType dependentType, Type conformingReplacementType, - ProtocolDecl *conformedProtocol) const { - return builder->lookupConformance(dependentType, - conformingReplacementType, - conformedProtocol); - } + ProtocolDecl *conformedProtocol) const; }; /// Retrieve a function that can perform conformance lookup for this diff --git a/include/swift/AST/Type.h b/include/swift/AST/Type.h index 2691d18d461c4..eafb4ee206890 100644 --- a/include/swift/AST/Type.h +++ b/include/swift/AST/Type.h @@ -147,10 +147,7 @@ enum class SubstFlags { /// Map member types to their desugared witness type. DesugarMemberTypes = 0x02, /// Substitute types involving opaque type archetypes. - SubstituteOpaqueArchetypes = 0x04, - /// Force substitution of opened archetypes. Normally -- without these flag -- - /// opened archetype conformances are not substituted. - ForceSubstituteOpenedExistentials = 0x08, + SubstituteOpaqueArchetypes = 0x04 }; /// Options for performing substitutions into a type. diff --git a/lib/AST/GenericSignatureBuilder.cpp b/lib/AST/GenericSignatureBuilder.cpp index dae212db78f5b..e8657e2096a74 100644 --- a/lib/AST/GenericSignatureBuilder.cpp +++ b/lib/AST/GenericSignatureBuilder.cpp @@ -3501,6 +3501,19 @@ GenericSignatureBuilder::getLookupConformanceFn() return LookUpConformanceInBuilder(this); } +ProtocolConformanceRef +GenericSignatureBuilder::LookUpConformanceInBuilder::operator()( + CanType dependentType, Type conformingReplacementType, + ProtocolDecl *conformedProtocol) const { + // Lookup conformances for opened existential. + if (conformingReplacementType->isOpenedExistential()) { + return conformedProtocol->getModuleContext()->lookupConformance( + conformingReplacementType, conformedProtocol); + } + return builder->lookupConformance(dependentType, conformingReplacementType, + conformedProtocol); +} + ProtocolConformanceRef GenericSignatureBuilder::lookupConformance(CanType dependentType, Type conformingReplacementType, diff --git a/lib/AST/ProtocolConformance.cpp b/lib/AST/ProtocolConformance.cpp index 30db092e0ee38..3b519714fb9af 100644 --- a/lib/AST/ProtocolConformance.cpp +++ b/lib/AST/ProtocolConformance.cpp @@ -120,12 +120,6 @@ ProtocolConformanceRef::subst(Type origType, // Otherwise, compute the substituted type. auto substType = origType.subst(subs, conformances, options); - // Opened existentials trivially conform and do not need to go through - // substitution map lookup. - if (substType->isOpenedExistential() && - !options.contains(SubstFlags::ForceSubstituteOpenedExistentials)) - return *this; - auto *proto = getRequirement(); // If the type is an existential, it must be self-conforming. diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index 11fda2fd3a1c4..c43f850e15a8e 100644 --- a/lib/AST/Type.cpp +++ b/lib/AST/Type.cpp @@ -3547,6 +3547,11 @@ operator()(CanType dependentType, Type conformingReplacementType, ProtocolConformanceRef LookUpConformanceInSubstitutionMap:: operator()(CanType dependentType, Type conformingReplacementType, ProtocolDecl *conformedProtocol) const { + // Lookup conformances for opened existential. + if (conformingReplacementType->isOpenedExistential()) { + return conformedProtocol->getModuleContext()->lookupConformance( + conformingReplacementType, conformedProtocol); + } return Subs.lookupConformance(dependentType, conformedProtocol); } @@ -3558,12 +3563,23 @@ operator()(CanType dependentType, Type conformingReplacementType, || conformingReplacementType->is() || conformingReplacementType->is()) && "replacement requires looking up a concrete conformance"); + // Lookup conformances for opened existential. + if (conformingReplacementType->isOpenedExistential()) { + return conformedProtocol->getModuleContext()->lookupConformance( + conformingReplacementType, conformedProtocol); + } return ProtocolConformanceRef(conformedProtocol); } ProtocolConformanceRef LookUpConformanceInSignature:: operator()(CanType dependentType, Type conformingReplacementType, ProtocolDecl *conformedProtocol) const { + // Lookup conformances for opened existential. + if (conformingReplacementType->isOpenedExistential()) { + return conformedProtocol->getModuleContext()->lookupConformance( + conformingReplacementType, conformedProtocol); + } + // FIXME: Should pass dependentType instead, once // GenericSignature::lookupConformance() does the right thing return Sig->lookupConformance(conformingReplacementType->getCanonicalType(), diff --git a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp index d18d165842e7d..d092ce4fc5252 100644 --- a/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp +++ b/lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp @@ -952,8 +952,7 @@ SILInstruction *SILCombiner::createApplyWithConcreteType( return CEI.lookupExistentialConformance(proto); } return ProtocolConformanceRef(proto); - }, - SubstFlags::ForceSubstituteOpenedExistentials); + }); } // We need to make sure that we can a) update Apply to use the new args and b) diff --git a/test/SILOptimizer/inline_generics.sil b/test/SILOptimizer/inline_generics.sil index 9b103e17afb2b..5ba0af36fdfbb 100644 --- a/test/SILOptimizer/inline_generics.sil +++ b/test/SILOptimizer/inline_generics.sil @@ -1,5 +1,6 @@ -// RUN: %target-sil-opt -enable-sil-verify-all %s -inline -sil-inline-generics=true | %FileCheck %s +// RUN: %target-sil-opt -enable-sil-verify-all %s -inline -sil-inline-generics=true -sil-print-generic-specialization-info | %FileCheck %s // RUN: %target-sil-opt -enable-sil-verify-all %s -inline -sil-inline-generics=false | %FileCheck --check-prefix=DISABLED-GENERIC-INLINING-CHECK %s +// RUN: %target-swift-frontend -O -emit-ir %s sil_stage canonical @@ -178,3 +179,48 @@ bb0(%0 : $*T, %1 : $*T): sil_default_witness_table P { no_default } + +protocol SomeProto : class {} +protocol OtherProto {} + +class MyObject {} +extension MyObject : OtherProto {} + +class MyNumber : MyObject {} +extension MyNumber : SomeProto {} + +sil_vtable MyObject {} +sil_vtable MyNumber {} + +// Make sure we subsitute the correct conformance when inlining. When +// substituting we need to pick up the normal_conformance. + +// CHECK-LABEL: sil @test_inlining : $@convention(objc_method) (@owned MyNumber) -> () { +// CHECK-NOT: Generic specialization information for call-site dootherstuff conformances <(abstract_conformance protocol=OtherProto)> +// CHECK: Generic specialization information +// CHECK: (normal_conformance type=MyObject protocol=OtherProto) +// CHECK: end sil function 'test_inlining' + +sil @test_inlining : $@convention(objc_method) (@owned MyNumber) -> () { +bb0(%0 : $MyNumber): + %12 = init_existential_ref %0 : $MyNumber : $MyNumber, $MyNumber & SomeProto + %14 = open_existential_ref %12 : $MyNumber & SomeProto to $@opened("B9711AE4-946E-11EA-A871-ACDE48001122") MyNumber & SomeProto + %15 = alloc_stack $OtherProto + %16 = init_existential_addr %15 : $*OtherProto, $@opened("B9711AE4-946E-11EA-A871-ACDE48001122") MyNumber & SomeProto + store %14 to %16 : $*@opened("B9711AE4-946E-11EA-A871-ACDE48001122") MyNumber & SomeProto + %18 = function_ref @doStuff : $@convention(thin) <τ_0_0 where τ_0_0 : OtherProto> (@in_guaranteed τ_0_0) -> () + %20 = apply %18<@opened("B9711AE4-946E-11EA-A871-ACDE48001122") MyNumber & SomeProto>(%16) : $@convention(thin) <τ_0_0 where τ_0_0 : OtherProto> (@in_guaranteed τ_0_0) -> () + destroy_addr %15 : $*OtherProto + dealloc_stack %15 : $*OtherProto + %27 = tuple () + return %27 : $() +} + +sil @dootherstuff : $@convention(thin) <τ_0_0 where τ_0_0 : OtherProto> (@in_guaranteed τ_0_0) -> () + +sil shared [signature_optimized_thunk] [always_inline] @doStuff : $@convention(thin) <τ_0_0 where τ_0_0 : OtherProto> (@in_guaranteed τ_0_0) -> () { +bb0(%0 : $*τ_0_0): + %2 = function_ref @dootherstuff : $@convention(thin) <τ_0_0 where τ_0_0 : OtherProto> (@in_guaranteed τ_0_0) -> () + %3 = apply %2<τ_0_0>(%0) : $@convention(thin) <τ_0_0 where τ_0_0 : OtherProto> (@in_guaranteed τ_0_0) -> () + return %3 : $() +} From 489b6e0860520a1157f738486f4c9aedfc515e01 Mon Sep 17 00:00:00 2001 From: Dan Zheng Date: Tue, 12 May 2020 13:39:12 -0700 Subject: [PATCH 02/15] [AutoDiff] Clean up derivative type calculation. Remove all assertions from `AnyFunctionType::getAutoDiffDerivativeFunctionLinearMapType`. All error cases are represented by `DerivativeFunctionTypeError` now. Fix `DerivativeFunctionTypeError` error payloads and improve error case naming. --- include/swift/AST/AutoDiff.h | 46 ++++++++++++++++++------------------ lib/AST/AutoDiff.cpp | 18 ++++++++++---- lib/AST/Type.cpp | 24 ++++++++++++------- lib/Sema/TypeCheckAttr.cpp | 43 +++++++++++++-------------------- 4 files changed, 68 insertions(+), 63 deletions(-) diff --git a/include/swift/AST/AutoDiff.h b/include/swift/AST/AutoDiff.h index 75cbe0b645f19..ffd3919bdd6bd 100644 --- a/include/swift/AST/AutoDiff.h +++ b/include/swift/AST/AutoDiff.h @@ -394,9 +394,16 @@ class DerivativeFunctionTypeError : public llvm::ErrorInfo { public: enum class Kind { + /// Original function type has no semantic results. NoSemanticResults, + /// Original function type has multiple semantic results. + // TODO(TF-1250): Support function types with multiple semantic results. MultipleSemanticResults, - NonDifferentiableParameters, + /// Differentiability parmeter indices are empty. + NoDifferentiabilityParameters, + /// A differentiability parameter does not conform to `Differentiable`. + NonDifferentiableDifferentiabilityParameter, + /// The original result type does not conform to `Differentiable`. NonDifferentiableResult }; @@ -406,12 +413,13 @@ class DerivativeFunctionTypeError /// The error kind. Kind kind; + /// The type and index of a differentiability parameter or result. + using TypeAndIndex = std::pair; + private: union Value { - IndexSubset *indices; - Type type; - Value(IndexSubset *indices) : indices(indices) {} - Value(Type type) : type(type) {} + TypeAndIndex typeAndIndex; + Value(TypeAndIndex typeAndIndex) : typeAndIndex(typeAndIndex) {} Value() {} } value; @@ -419,29 +427,21 @@ class DerivativeFunctionTypeError explicit DerivativeFunctionTypeError(AnyFunctionType *functionType, Kind kind) : functionType(functionType), kind(kind), value(Value()) { assert(kind == Kind::NoSemanticResults || - kind == Kind::MultipleSemanticResults); - }; - - explicit DerivativeFunctionTypeError(AnyFunctionType *functionType, Kind kind, - IndexSubset *nonDiffParameterIndices) - : functionType(functionType), kind(kind), value(nonDiffParameterIndices) { - assert(kind == Kind::NonDifferentiableParameters); + kind == Kind::MultipleSemanticResults || + kind == Kind::NoDifferentiabilityParameters); }; explicit DerivativeFunctionTypeError(AnyFunctionType *functionType, Kind kind, - Type nonDiffResultType) - : functionType(functionType), kind(kind), value(nonDiffResultType) { - assert(kind == Kind::NonDifferentiableResult); + TypeAndIndex nonDiffTypeAndIndex) + : functionType(functionType), kind(kind), value(nonDiffTypeAndIndex) { + assert(kind == Kind::NonDifferentiableDifferentiabilityParameter || + kind == Kind::NonDifferentiableResult); }; - IndexSubset *getNonDifferentiableParameterIndices() const { - assert(kind == Kind::NonDifferentiableParameters); - return value.indices; - } - - Type getNonDifferentiableResultType() const { - assert(kind == Kind::NonDifferentiableResult); - return value.type; + TypeAndIndex getNonDifferentiableTypeAndIndex() const { + assert(kind == Kind::NonDifferentiableDifferentiabilityParameter || + kind == Kind::NonDifferentiableResult); + return value.typeAndIndex; } void log(raw_ostream &OS) const override; diff --git a/lib/AST/AutoDiff.cpp b/lib/AST/AutoDiff.cpp index 2d61601b5ff77..c1b65191b73bb 100644 --- a/lib/AST/AutoDiff.cpp +++ b/lib/AST/AutoDiff.cpp @@ -402,12 +402,20 @@ void DerivativeFunctionTypeError::log(raw_ostream &OS) const { case Kind::MultipleSemanticResults: OS << "has multiple semantic results"; break; - case Kind::NonDifferentiableParameters: - OS << "has non-differentiable parameters: "; - value.indices->print(OS); + case Kind::NoDifferentiabilityParameters: + OS << "has no differentiability parameters"; break; - case Kind::NonDifferentiableResult: - OS << "has non-differentiable result: " << value.type; + case Kind::NonDifferentiableDifferentiabilityParameter: { + auto nonDiffParam = getNonDifferentiableTypeAndIndex(); + OS << "has non-differentiable differentiability parameter " + << nonDiffParam.second << ": " << nonDiffParam.first; break; } + case Kind::NonDifferentiableResult: { + auto nonDiffResult = getNonDifferentiableTypeAndIndex(); + OS << "has non-differentiable result " << nonDiffResult.second << ": " + << nonDiffResult.first; + break; + } + } } diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index 11fda2fd3a1c4..e4a7f74139db1 100644 --- a/lib/AST/Type.cpp +++ b/lib/AST/Type.cpp @@ -5171,9 +5171,11 @@ llvm::Expected AnyFunctionType::getAutoDiffDerivativeFunctionLinearMapType( IndexSubset *parameterIndices, AutoDiffLinearMapKind kind, LookupConformanceFn lookupConformance, bool makeSelfParamFirst) { - assert(!parameterIndices->isEmpty() && - "Expected at least one differentiability parameter"); auto &ctx = getASTContext(); + // Error if differentiability parameter indices are empty. + if (parameterIndices->isEmpty()) + return llvm::make_error( + this, DerivativeFunctionTypeError::Kind::NoDifferentiabilityParameters); // Get differentiability parameters. SmallVector diffParams; @@ -5202,7 +5204,7 @@ AnyFunctionType::getAutoDiffDerivativeFunctionLinearMapType( if (!resultTan) { return llvm::make_error( this, DerivativeFunctionTypeError::Kind::NonDifferentiableResult, - originalResultType); + std::make_pair(originalResultType, /*index*/ 0)); } auto resultTanType = resultTan->getType(); @@ -5225,15 +5227,17 @@ AnyFunctionType::getAutoDiffDerivativeFunctionLinearMapType( // - Differential: `(T0.Tan, inout T1.Tan, ...) -> Void` SmallVector differentialParams; bool hasInoutDiffParameter = false; - for (auto diffParam : diffParams) { + for (auto i : range(diffParams.size())) { + auto diffParam = diffParams[i]; auto paramType = diffParam.getPlainType(); auto paramTan = paramType->getAutoDiffTangentSpace(lookupConformance); // Error if paraneter has no tangent space. if (!paramTan) { return llvm::make_error( this, - DerivativeFunctionTypeError::Kind::NonDifferentiableParameters, - parameterIndices); + DerivativeFunctionTypeError::Kind:: + NonDifferentiableDifferentiabilityParameter, + std::make_pair(paramType, i)); } differentialParams.push_back(AnyFunctionType::Param( paramTan->getType(), Identifier(), diffParam.getParameterFlags())); @@ -5261,15 +5265,17 @@ AnyFunctionType::getAutoDiffDerivativeFunctionLinearMapType( // - Pullback: `(inout T1.Tan) -> (T0.Tan, ...)` SmallVector pullbackResults; bool hasInoutDiffParameter = false; - for (auto diffParam : diffParams) { + for (auto i : range(diffParams.size())) { + auto diffParam = diffParams[i]; auto paramType = diffParam.getPlainType(); auto paramTan = paramType->getAutoDiffTangentSpace(lookupConformance); // Error if paraneter has no tangent space. if (!paramTan) { return llvm::make_error( this, - DerivativeFunctionTypeError::Kind::NonDifferentiableParameters, - parameterIndices); + DerivativeFunctionTypeError::Kind:: + NonDifferentiableDifferentiabilityParameter, + std::make_pair(paramType, i)); } if (diffParam.isInOut()) { hasInoutDiffParameter = true; diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index 2f81a25914ba6..532de0f628d95 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -4570,14 +4570,9 @@ static bool typeCheckDerivativeAttr(ASTContext &Ctx, Decl *D, if (!resolvedDiffParamIndices) return true; - // Check if the differentiability parameter indices are valid. - if (checkDifferentiabilityParameters( - originalAFD, resolvedDiffParamIndices, originalFnType, - derivative->getGenericEnvironment(), derivative->getModuleContext(), - parsedDiffParams, attr->getLocation())) - return true; - // Set the resolved differentiability parameter indices in the attribute. + // Differentiability parameter indices verification is done by + // `AnyFunctionType::getAutoDiffDerivativeFunctionLinearMapType` below. attr->setParameterIndices(resolvedDiffParamIndices); // Compute the expected differential/pullback type. @@ -4588,43 +4583,39 @@ static bool typeCheckDerivativeAttr(ASTContext &Ctx, Decl *D, // Helper for diagnosing derivative function type errors. auto errorHandler = [&](const DerivativeFunctionTypeError &error) { + attr->setInvalid(); switch (error.kind) { case DerivativeFunctionTypeError::Kind::NoSemanticResults: diags .diagnose(attr->getLocation(), diag::autodiff_attr_original_multiple_semantic_results) .highlight(attr->getOriginalFunctionName().Loc.getSourceRange()); - attr->setInvalid(); return; case DerivativeFunctionTypeError::Kind::MultipleSemanticResults: diags .diagnose(attr->getLocation(), diag::autodiff_attr_original_multiple_semantic_results) .highlight(attr->getOriginalFunctionName().Loc.getSourceRange()); - attr->setInvalid(); return; - case DerivativeFunctionTypeError::Kind::NonDifferentiableParameters: { - auto *nonDiffParamIndices = error.getNonDifferentiableParameterIndices(); - SmallVector diffParams; - error.functionType->getSubsetParameters(resolvedDiffParamIndices, - diffParams); - for (unsigned i : range(diffParams.size())) { - if (!nonDiffParamIndices->contains(i)) - continue; - SourceLoc loc = parsedDiffParams.empty() ? attr->getLocation() - : parsedDiffParams[i].getLoc(); - auto diffParamType = diffParams[i].getPlainType(); - diags.diagnose(loc, diag::diff_params_clause_param_not_differentiable, - diffParamType); - } + case DerivativeFunctionTypeError::Kind::NoDifferentiabilityParameters: + diags.diagnose(attr->getLocation(), + diag::diff_params_clause_no_inferred_parameters); + return; + case DerivativeFunctionTypeError::Kind:: + NonDifferentiableDifferentiabilityParameter: { + auto nonDiffParam = error.getNonDifferentiableTypeAndIndex(); + SourceLoc loc = parsedDiffParams.empty() + ? attr->getLocation() + : parsedDiffParams[nonDiffParam.second].getLoc(); + diags.diagnose(loc, diag::diff_params_clause_param_not_differentiable, + nonDiffParam.first); return; } case DerivativeFunctionTypeError::Kind::NonDifferentiableResult: - auto originalResultType = error.getNonDifferentiableResultType(); + auto nonDiffResult = error.getNonDifferentiableTypeAndIndex(); diags.diagnose(attr->getLocation(), diag::differentiable_attr_result_not_differentiable, - originalResultType); - attr->setInvalid(); + nonDiffResult.first); return; } }; From ec0c9484abbe6a561011f3bce8f16a270e09df88 Mon Sep 17 00:00:00 2001 From: Rintaro Ishizaki Date: Tue, 12 May 2020 15:38:08 -0700 Subject: [PATCH 03/15] [CodeCompletion] Inherit options when parsing new buffer for fast completions. Options may affect the parsing result. Also, don't collect interface hash tokens inside inactive blocks. --- lib/IDE/CompletionInstance.cpp | 17 ++++++--- lib/Parse/ParseIfConfig.cpp | 4 ++ .../complete_sequence_ifconfig.swift | 37 +++++++++++++++++++ 3 files changed, 53 insertions(+), 5 deletions(-) create mode 100644 test/SourceKit/CodeComplete/complete_sequence_ifconfig.swift diff --git a/lib/IDE/CompletionInstance.cpp b/lib/IDE/CompletionInstance.cpp index 09d0d6708c0c6..3dcbf77883faf 100644 --- a/lib/IDE/CompletionInstance.cpp +++ b/lib/IDE/CompletionInstance.cpp @@ -315,10 +315,10 @@ bool CompletionInstance::performCachedOperationIfPossible( auto tmpBufferID = tmpSM.addMemBufferCopy(completionBuffer); tmpSM.setCodeCompletionPoint(tmpBufferID, Offset); - LangOptions langOpts; + LangOptions langOpts = CI.getASTContext().LangOpts; langOpts.DisableParserLookup = true; - TypeCheckerOptions typeckOpts; - SearchPathOptions searchPathOpts; + TypeCheckerOptions typeckOpts = CI.getASTContext().TypeCheckerOpts; + SearchPathOptions searchPathOpts = CI.getASTContext().SearchPathOpts; DiagnosticEngine tmpDiags(tmpSM); std::unique_ptr tmpCtx( ASTContext::get(langOpts, typeckOpts, searchPathOpts, tmpSM, tmpDiags)); @@ -327,13 +327,20 @@ bool CompletionInstance::performCachedOperationIfPossible( registerTypeCheckerRequestFunctions(tmpCtx->evaluator); registerSILGenRequestFunctions(tmpCtx->evaluator); ModuleDecl *tmpM = ModuleDecl::create(Identifier(), *tmpCtx); - SourceFile *tmpSF = new (*tmpCtx) SourceFile(*tmpM, oldSF->Kind, tmpBufferID); + SourceFile *tmpSF = new (*tmpCtx) + SourceFile(*tmpM, oldSF->Kind, tmpBufferID, /*KeepParsedTokens=*/false, + /*BuildSyntaxTree=*/false, oldSF->getParsingOptions()); tmpSF->enableInterfaceHash(); // Ensure all non-function-body tokens are hashed into the interface hash tmpCtx->LangOpts.EnableTypeFingerprints = false; - // Couldn't find any completion token? + // FIXME: Since we don't setup module loaders on the temporary AST context, + // 'canImport()' conditional compilation directive always fails. That causes + // interface hash change and prevents fast-completion. + + // Parse and get the completion context. auto *newState = tmpSF->getDelayedParserState(); + // Couldn't find any completion token? if (!newState->hasCodeCompletionDelayedDeclState()) return false; diff --git a/lib/Parse/ParseIfConfig.cpp b/lib/Parse/ParseIfConfig.cpp index f8799b2ec9c43..ac68043a60193 100644 --- a/lib/Parse/ParseIfConfig.cpp +++ b/lib/Parse/ParseIfConfig.cpp @@ -676,6 +676,10 @@ ParserResult Parser::parseIfConfig( SmallVector Elements; llvm::SaveAndRestore S(InInactiveClauseEnvironment, InInactiveClauseEnvironment || !isActive); + // Disable updating the interface hash inside inactive blocks. + llvm::SaveAndRestore> T( + CurrentTokenHash, isActive ? CurrentTokenHash : nullptr); + if (isActive || !isVersionCondition) { parseElements(Elements, isActive); } else if (SyntaxContext->isEnabled()) { diff --git a/test/SourceKit/CodeComplete/complete_sequence_ifconfig.swift b/test/SourceKit/CodeComplete/complete_sequence_ifconfig.swift new file mode 100644 index 0000000000000..6ea0a98876e5e --- /dev/null +++ b/test/SourceKit/CodeComplete/complete_sequence_ifconfig.swift @@ -0,0 +1,37 @@ +struct MyStruct { func foo() {} } + +#if DEBUG +import Swift +#endif + +#if arch(x86_64) +operator ++++++ +#endif + +#if !targetEnvironment(simulator) +precedencegroup MyPrecedence +#endif + +func foo(value: MyStruct) { + value. +} + +// Ensure that compilation directives are equally evaluated and doesn't affect the fast completions. + +// RUN: %sourcekitd-test \ +// RUN: -req=complete -pos=16:9 -repeat-request=2 %s -- %s -target %target-triple \ +// RUN: | %FileCheck %s + +// CHECK-LABEL: key.results: [ +// CHECK-NOT: ] +// CHECK-DAG: key.description: "foo()", +// CHECK-DAG: key.description: "self", +// CHECK: ] +// CHECK-NOT: key.reusingastcontext + +// CHECK-LABEL: key.results: [ +// CHECK-NOT: ] +// CHECK-DAG: key.description: "foo()", +// CHECK-DAG: key.description: "self", +// CHECK: ], +// CHECK: key.reusingastcontext: 1 From 97e32abc3c6c6a6ea8d8b8f92c37b34a336cf46d Mon Sep 17 00:00:00 2001 From: Artem Chikin Date: Mon, 11 May 2020 14:49:30 -0700 Subject: [PATCH 04/15] Improve diagnostic for keypath used without 'keyPath:' label --- lib/Sema/CSSimplify.cpp | 38 +++++++++++++++++++---- test/Sema/keypath_subscript_nolabel.swift | 35 +++++++++++++++++++++ 2 files changed, 67 insertions(+), 6 deletions(-) create mode 100644 test/Sema/keypath_subscript_nolabel.swift diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 0f1279a2a578a..35bc522c25b7a 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -5674,6 +5674,19 @@ static bool isForKeyPathSubscript(ConstraintSystem &cs, return false; } +static bool isForKeyPathSubscriptWithoutLabel(ConstraintSystem &cs, + ConstraintLocator *locator) { + if (!locator || !locator->getAnchor()) + return false; + + if (auto *SE = getAsExpr(locator->getAnchor())) { + auto *indexExpr = SE->getIndex(); + return isa(indexExpr) && + isa(indexExpr->getSemanticsProvidingExpr()); + } + return false; +} + /// Determine whether all of the given candidate overloads /// found through conditional conformances of a given base type. /// This is useful to figure out whether it makes sense to @@ -5801,7 +5814,13 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName, MemberLookupResult result; result.OverallResult = MemberLookupResult::HasResults; - if (isForKeyPathSubscript(*this, memberLocator)) { + // Add key path result. + // If we are including inaccessible members, check for the use of a keypath + // subscript without a `keyPath:` label. Add it to the result so that it + // can be caught by the missing argument label checking later. + if (isForKeyPathSubscript(*this, memberLocator) || + (isForKeyPathSubscriptWithoutLabel(*this, memberLocator) + && includeInaccessibleMembers)) { if (baseTy->isAnyObject()) { result.addUnviable( OverloadChoice(baseTy, OverloadChoiceKind::KeyPathApplication), @@ -9693,14 +9712,21 @@ ConstraintSystem::addKeyPathApplicationRootConstraint(Type root, ConstraintLocat path[0].getKind() == ConstraintLocator::SubscriptMember) || (path.size() == 2 && path[1].getKind() == ConstraintLocator::KeyPathDynamicMember)); + auto indexTuple = dyn_cast(subscript->getIndex()); - if (!indexTuple || indexTuple->getNumElements() != 1) - return; - - auto keyPathExpr = dyn_cast(indexTuple->getElement(0)); + auto indexParen = dyn_cast(subscript->getIndex()); + // If a keypath subscript is used without the expected `keyPath:` label, + // continue with type-checking when attempting fixes so that it gets caught + // by the argument label checking. In such cases, the KeyPathExpr is contained + // in a ParenExpr, instead of a TupleExpr. + assert(((indexTuple && indexTuple->getNumElements() == 1) || indexParen) && + "Expected KeyPathExpr to be in either TupleExpr or ParenExpr"); + + auto keyPathExpr = dyn_cast( + indexTuple ? indexTuple->getElement(0) : indexParen->getSubExpr()); if (!keyPathExpr) return; - + auto typeVar = getType(keyPathExpr)->getAs(); if (!typeVar) return; diff --git a/test/Sema/keypath_subscript_nolabel.swift b/test/Sema/keypath_subscript_nolabel.swift new file mode 100644 index 0000000000000..ef1a135538b3b --- /dev/null +++ b/test/Sema/keypath_subscript_nolabel.swift @@ -0,0 +1,35 @@ +// RUN: %target-swift-frontend -typecheck -verify -primary-file %s +// [SR-12745] +// rdar://problem/62957095 +struct S1 { + var x : Int = 0 +} +var s1 = S1() +s1[\.x] = 10 // expected-error {{missing argument label 'keyPath:' in subscript}} {{4-4=keyPath: }} + +struct S2 { + var x : Int = 0 + subscript(_ v: Int) -> Int { 0 } +} +var s2 = S2() +s2[\.x] = 10 // expected-error {{missing argument label 'keyPath:' in subscript}} {{4-4=keyPath: }} + +struct S3 { + var x : Int = 0 + subscript(v v: KeyPath) -> Int { get { 0 } set(newValue) {} } +} +var s3 = S3() +// TODO(diagnostics): This should actually be a diagnostic that correctly identifies that in the presence +// of a missing label, there are two options for resolution: 'keyPath' and 'v:' and to offer the user +// a choice. +// Today, the ExprTypeChecker identifies the disjunction with two of these possibilities, but +// filters out some of the terms based on label mismatch (but not implicit keypath terms, for example). +// It should probably not do that. +s3[\.x] = 10 // expected-error {{missing argument label 'keyPath:' in subscript}} {{4-4=keyPath: }} + +struct S4 { + var x : Int = 0 + subscript(v: KeyPath) -> Int { get { 0 } set(newValue) {} } +} +var s4 = S4() +s4[\.x] = 10 // expected-error {{key path value type 'Int' cannot be converted to contextual type 'String'}} From cfd1fdb9a6bb854f92608f95974b3d27b1fe4868 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20Laferrie=CC=80re?= Date: Fri, 8 May 2020 17:05:40 -0700 Subject: [PATCH 05/15] [Sema] Clean up remaining uses of FromSPI --- lib/Sema/TypeCheckAccess.cpp | 29 ++++++----------------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/lib/Sema/TypeCheckAccess.cpp b/lib/Sema/TypeCheckAccess.cpp index 3d7e372d05d61..ce75ab27e9065 100644 --- a/lib/Sema/TypeCheckAccess.cpp +++ b/lib/Sema/TypeCheckAccess.cpp @@ -42,15 +42,6 @@ enum class DowngradeToWarning: bool { Yes }; -/// A uniquely-typed boolean to reduce the chances of accidentally inverting -/// a check. -/// -/// \see checkTypeAccessImpl -enum class FromSPI: bool { - No, - Yes -}; - /// Calls \p callback for each type in each requirement provided by /// \p source. static void forAllRequirementTypes( @@ -87,7 +78,7 @@ class AccessControlCheckerBase { void checkTypeAccessImpl( Type type, TypeRepr *typeRepr, AccessScope contextAccessScope, - const DeclContext *useDC, bool mayBeInferred, FromSPI fromSPI, + const DeclContext *useDC, bool mayBeInferred, llvm::function_ref diagnose); void checkTypeAccess( @@ -109,7 +100,7 @@ class AccessControlCheckerBase { llvm::function_ref diagnose) { forAllRequirementTypes(std::move(source), [&](Type type, TypeRepr *typeRepr) { checkTypeAccessImpl(type, typeRepr, accessScope, useDC, - /*mayBeInferred*/false, FromSPI::No, diagnose); + /*mayBeInferred*/false, diagnose); }); } @@ -196,12 +187,9 @@ class TypeAccessScopeDiagnoser : private ASTWalker { /// using `Array` to mean `Array` in an extension of Array.) If /// \p typeRepr is known to be absent, it's okay to pass \c false for /// \p mayBeInferred. -/// -/// If searching from an SPI context, pass \c FromSPI::YES for \p fromSPI. -/// In this mode, all types must be public and diagnostic messages are adapted. void AccessControlCheckerBase::checkTypeAccessImpl( Type type, TypeRepr *typeRepr, AccessScope contextAccessScope, - const DeclContext *useDC, bool mayBeInferred, FromSPI fromSPI, + const DeclContext *useDC, bool mayBeInferred, llvm::function_ref diagnose) { auto &Context = useDC->getASTContext(); @@ -310,9 +298,8 @@ void AccessControlCheckerBase::checkTypeAccess( context->getFormalAccessScope( context->getDeclContext(), checkUsableFromInline); - auto fromSPI = static_cast(context->isSPI()); checkTypeAccessImpl(type, typeRepr, contextAccessScope, DC, mayBeInferred, - fromSPI, diagnose); + diagnose); } /// Highlights the given TypeRepr, and adds a note pointing to the type's @@ -372,10 +359,6 @@ void AccessControlCheckerBase::checkGenericParamAccess( }; auto *DC = ownerDecl->getDeclContext(); - auto fromSPI = FromSPI::No; - if (auto ownerValueDecl = dyn_cast(ownerDecl)) { - fromSPI = static_cast(ownerValueDecl->isSPI()); - } for (auto param : *params) { if (param->getInherited().empty()) @@ -384,7 +367,7 @@ void AccessControlCheckerBase::checkGenericParamAccess( checkTypeAccessImpl(param->getInherited().front().getType(), param->getInherited().front().getTypeRepr(), accessScope, DC, /*mayBeInferred*/false, - fromSPI, callback); + callback); } callbackACEK = ACEK::Requirement; @@ -1496,7 +1479,7 @@ class ExportabilityChecker : public DeclVisitor { bool foundAnyIssues = false; // Check the TypeRepr first (if present), because that will give us a - // better diagonstic. + // better diagnostic. if (typeRepr) { const_cast(typeRepr)->walk(TypeReprIdentFinder( [&](const ComponentIdentTypeRepr *component) { From 08abc0dbec764dba789ebf7920017a59f1739641 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexis=20Laferrie=CC=80re?= Date: Fri, 8 May 2020 17:05:58 -0700 Subject: [PATCH 06/15] [Sema] Fix leak of implementation-only imported types in SPI signatures --- lib/Sema/TypeCheckAccess.cpp | 42 +++++++++-------- ...ntation_only_spi_import_exposability.swift | 46 +++++++++++++++++++ 2 files changed, 69 insertions(+), 19 deletions(-) create mode 100644 test/SPI/implementation_only_spi_import_exposability.swift diff --git a/lib/Sema/TypeCheckAccess.cpp b/lib/Sema/TypeCheckAccess.cpp index ce75ab27e9065..c67093cd5c843 100644 --- a/lib/Sema/TypeCheckAccess.cpp +++ b/lib/Sema/TypeCheckAccess.cpp @@ -1466,11 +1466,14 @@ class UsableFromInlineChecker : public AccessControlCheckerBase, } }; +// Diagnose public APIs exposing types that are either imported as +// implementation-only or declared as SPI. class ExportabilityChecker : public DeclVisitor { class Diagnoser; void checkTypeImpl( Type type, const TypeRepr *typeRepr, const SourceFile &SF, + const Decl *context, const Diagnoser &diagnoser) { // Don't bother checking errors. if (type && type->hasError()) @@ -1483,14 +1486,15 @@ class ExportabilityChecker : public DeclVisitor { if (typeRepr) { const_cast(typeRepr)->walk(TypeReprIdentFinder( [&](const ComponentIdentTypeRepr *component) { - ModuleDecl *M = component->getBoundDecl()->getModuleContext(); - if (!SF.isImportedImplementationOnly(M) && - !SF.isImportedAsSPI(component->getBoundDecl())) - return true; - - diagnoser.diagnoseType(component->getBoundDecl(), component, - SF.isImportedImplementationOnly(M)); - foundAnyIssues = true; + TypeDecl *typeDecl = component->getBoundDecl(); + ModuleDecl *M = typeDecl->getModuleContext(); + bool isImplementationOnly = SF.isImportedImplementationOnly(M); + if (isImplementationOnly || + (SF.isImportedAsSPI(typeDecl) && !context->isSPI())) { + diagnoser.diagnoseType(typeDecl, component, isImplementationOnly); + foundAnyIssues = true; + } + // We still continue even in the diagnostic case to report multiple // violations. return true; @@ -1508,19 +1512,19 @@ class ExportabilityChecker : public DeclVisitor { class ProblematicTypeFinder : public TypeDeclFinder { const SourceFile &SF; + const Decl *context; const Diagnoser &diagnoser; public: - ProblematicTypeFinder(const SourceFile &SF, const Diagnoser &diagnoser) - : SF(SF), diagnoser(diagnoser) {} + ProblematicTypeFinder(const SourceFile &SF, const Decl *context, const Diagnoser &diagnoser) + : SF(SF), context(context), diagnoser(diagnoser) {} void visitTypeDecl(const TypeDecl *typeDecl) { ModuleDecl *M = typeDecl->getModuleContext(); - if (!SF.isImportedImplementationOnly(M) && - !SF.isImportedAsSPI(typeDecl)) - return; - - diagnoser.diagnoseType(typeDecl, /*typeRepr*/nullptr, - SF.isImportedImplementationOnly(M)); + bool isImplementationOnly = SF.isImportedImplementationOnly(M); + if (isImplementationOnly || + (SF.isImportedAsSPI(typeDecl) && !context->isSPI())) + diagnoser.diagnoseType(typeDecl, /*typeRepr*/nullptr, + isImplementationOnly); } void visitSubstitutionMap(SubstitutionMap subs) { @@ -1580,7 +1584,7 @@ class ExportabilityChecker : public DeclVisitor { } }; - type.walk(ProblematicTypeFinder(SF, diagnoser)); + type.walk(ProblematicTypeFinder(SF, context, diagnoser)); } void checkType( @@ -1588,7 +1592,7 @@ class ExportabilityChecker : public DeclVisitor { const Diagnoser &diagnoser) { auto *SF = context->getDeclContext()->getParentSourceFile(); assert(SF && "checking a non-source declaration?"); - return checkTypeImpl(type, typeRepr, *SF, diagnoser); + return checkTypeImpl(type, typeRepr, *SF, context, diagnoser); } void checkType( @@ -1685,7 +1689,7 @@ class ExportabilityChecker : public DeclVisitor { AccessScope accessScope = VD->getFormalAccessScope(nullptr, /*treatUsableFromInlineAsPublic*/true); - if (accessScope.isPublic() && !accessScope.isSPI()) + if (accessScope.isPublic()) return false; // Is this a stored property in a non-resilient struct or class? diff --git a/test/SPI/implementation_only_spi_import_exposability.swift b/test/SPI/implementation_only_spi_import_exposability.swift new file mode 100644 index 0000000000000..aa227d7704351 --- /dev/null +++ b/test/SPI/implementation_only_spi_import_exposability.swift @@ -0,0 +1,46 @@ +/// @_implementationOnly imported decls (SPI or not) should not be exposed in SPI. + +// RUN: %empty-directory(%t) +// RUN: %target-swift-frontend -emit-module -DLIB %s -module-name Lib -emit-module-path %t/Lib.swiftmodule +// RUN: %target-typecheck-verify-swift -DCLIENT -I %t + +#if LIB + +@_spi(A) public func spiFunc() {} + +@_spi(A) public struct SPIStruct { + public init() {} +} + +@_spi(A) public protocol SPIProtocol {} + +public func ioiFunc() {} + +public struct IOIStruct { + public init() {} +} + +public protocol IOIProtocol {} + +#elseif CLIENT + +@_spi(A) @_implementationOnly import Lib + +@_spi(B) public func leakSPIStruct(_ a: SPIStruct) -> SPIStruct { fatalError() } // expected-error 2 {{cannot use struct 'SPIStruct' here; 'Lib' has been imported as implementation-only}} +@_spi(B) public func leakIOIStruct(_ a: IOIStruct) -> IOIStruct { fatalError() } // expected-error 2 {{cannot use struct 'IOIStruct' here; 'Lib' has been imported as implementation-only}} + +public struct PublicStruct : IOIProtocol, SPIProtocol { // expected-error {{cannot use protocol 'IOIProtocol' here; 'Lib' has been imported as implementation-only}} +// expected-error @-1 {{cannot use protocol 'SPIProtocol' here; 'Lib' has been imported as implementation-only}} + public var spiStruct = SPIStruct() // expected-error {{cannot use struct 'SPIStruct' here; 'Lib' has been imported as implementation-only}} + public var ioiStruct = IOIStruct() // expected-error {{cannot use struct 'IOIStruct' here; 'Lib' has been imported as implementation-only}} + + @inlinable + public func publicInlinable() { + spiFunc() // expected-error {{global function 'spiFunc()' is '@_spi' and cannot be referenced from an '@inlinable' function}} + ioiFunc() // expected-error {{global function 'ioiFunc()' cannot be used in an '@inlinable' function because 'Lib' was imported implementation-only}} + let s = SPIStruct() // expected-error {{struct 'SPIStruct' is '@_spi' and cannot be referenced from an '@inlinable' function}} + let i = IOIStruct() // expected-error {{struct 'IOIStruct' cannot be used in an '@inlinable' function because 'Lib' was imported implementation-only}} + } +} + +#endif From 12b9426312e21c3368d1fd0db9605e9bcfaea01d Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Tue, 12 May 2020 21:23:33 -0700 Subject: [PATCH 07/15] [Sema] Don't diagnose DynamicSubscriptExpr's arg as tuple Register DynamicSubscriptExpr's index expression as a call argument in MiscDiagnostics to avoid it being diagnosed as a single element labelled tuple. Resolves SR-12799. --- lib/Sema/MiscDiagnostics.cpp | 3 +++ test/Constraints/dynamic_lookup.swift | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp index f6bd269c6d072..a0ea012d0a2fb 100644 --- a/lib/Sema/MiscDiagnostics.cpp +++ b/lib/Sema/MiscDiagnostics.cpp @@ -137,6 +137,9 @@ static void diagSyntacticUseRestrictions(const Expr *E, const DeclContext *DC, if (auto *SE = dyn_cast(E)) CallArgs.insert(SE->getIndex()); + if (auto *DSE = dyn_cast(E)) + CallArgs.insert(DSE->getIndex()); + if (auto *KPE = dyn_cast(E)) { for (auto Comp : KPE->getComponents()) { if (auto *Arg = Comp.getIndexExpr()) diff --git a/test/Constraints/dynamic_lookup.swift b/test/Constraints/dynamic_lookup.swift index 6297f1475b76d..27e284842bf3e 100644 --- a/test/Constraints/dynamic_lookup.swift +++ b/test/Constraints/dynamic_lookup.swift @@ -371,6 +371,10 @@ class C1 { @objc subscript(unambiguousSubscript _: String) -> Int { return 0 } // expected-note {{found this candidate}} } +class C2 { + @objc subscript(singleCandidate _: Int) -> Int { return 0 } +} + func testAnyObjectAmbiguity(_ x: AnyObject) { _ = x.ambiguousProperty // expected-error {{ambiguous use of 'ambiguousProperty'}} _ = x.unambiguousProperty @@ -384,6 +388,9 @@ func testAnyObjectAmbiguity(_ x: AnyObject) { _ = x.ambiguousMethodParam // expected-error {{ambiguous use of 'ambiguousMethodParam'}} _ = x.unambiguousMethodParam + // SR-12799: Don't emit a "single-element" tuple error. + _ = x[singleCandidate: 0] + _ = x[ambiguousSubscript: 0] // expected-error {{ambiguous use of 'subscript(ambiguousSubscript:)'}} // FIX-ME(SR-8611): This is currently ambiguous but shouldn't be. From 4595f73760ba78b0775a5ad18d25fa389e04d050 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Tue, 12 May 2020 21:23:34 -0700 Subject: [PATCH 08/15] [Sema] Reject an @objc generic subscript Previously we could allow such a declaration to be marked @objc, despite being incompatible. This caused crashes later down the pipeline as the subscript's accessors were correctly checked for generic params and were not marked @objc. Resolves SR-12801. --- include/swift/AST/DiagnosticsSema.def | 4 ++-- lib/Sema/TypeCheckDeclObjC.cpp | 17 ++++++++++------- test/attr/attr_objc.swift | 6 ++++++ 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 362036d56da9d..43b5250de16b7 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -4262,8 +4262,8 @@ ERROR(objc_invalid_on_subscript,none, ERROR(objc_invalid_on_static_subscript,none, "%0 cannot be %" OBJC_ATTR_SELECT "1", (DescriptiveDeclKind, unsigned)) ERROR(objc_invalid_with_generic_params,none, - "method cannot be %" OBJC_ATTR_SELECT "0 because it has generic " - "parameters", (unsigned)) + "%0 cannot be %" OBJC_ATTR_SELECT "1 because it has generic parameters", + (DescriptiveDeclKind, unsigned)) ERROR(objc_convention_invalid,none, "%0 is not representable in Objective-C, so it cannot be used" " with '@convention(%1)'", (Type, StringRef)) diff --git a/lib/Sema/TypeCheckDeclObjC.cpp b/lib/Sema/TypeCheckDeclObjC.cpp index efffa42cb726e..21002610485e9 100644 --- a/lib/Sema/TypeCheckDeclObjC.cpp +++ b/lib/Sema/TypeCheckDeclObjC.cpp @@ -304,16 +304,17 @@ static bool isParamListRepresentableInObjC(const AbstractFunctionDecl *AFD, /// Check whether the given declaration contains its own generic parameters, /// and therefore is not representable in Objective-C. -static bool checkObjCWithGenericParams(const AbstractFunctionDecl *AFD, - ObjCReason Reason) { - bool Diagnose = shouldDiagnoseObjCReason(Reason, AFD->getASTContext()); +static bool checkObjCWithGenericParams(const ValueDecl *VD, ObjCReason Reason) { + bool Diagnose = shouldDiagnoseObjCReason(Reason, VD->getASTContext()); - if (AFD->getGenericParams()) { + auto *GC = VD->getAsGenericContext(); + assert(GC); + if (GC->getGenericParams()) { // Diagnose this problem, if asked to. if (Diagnose) { - AFD->diagnose(diag::objc_invalid_with_generic_params, - getObjCDiagnosticAttrKind(Reason)); - describeObjCReason(AFD, Reason); + VD->diagnose(diag::objc_invalid_with_generic_params, + VD->getDescriptiveKind(), getObjCDiagnosticAttrKind(Reason)); + describeObjCReason(VD, Reason); } return true; @@ -855,6 +856,8 @@ bool swift::isRepresentableInObjC(const SubscriptDecl *SD, ObjCReason Reason) { if (checkObjCInForeignClassContext(SD, Reason)) return false; + if (checkObjCWithGenericParams(SD, Reason)) + return false; // ObjC doesn't support class subscripts. if (!SD->isInstanceMember()) { diff --git a/test/attr/attr_objc.swift b/test/attr/attr_objc.swift index 739cd6a97a589..77848a0ab196a 100644 --- a/test/attr/attr_objc.swift +++ b/test/attr/attr_objc.swift @@ -2375,3 +2375,9 @@ class SR_9035_C {} func throwingMethod2() throws -> Unmanaged // expected-error {{method cannot be a member of an @objc protocol because its result type cannot be represented in Objective-C}} // expected-note@-1 {{inferring '@objc' because the declaration is a member of an '@objc' protocol}} } + +// SR-12801: Make sure we reject an @objc generic subscript. +class SR12801 { + @objc subscript(foo : [T]) -> Int { return 0 } + // expected-error@-1 {{subscript cannot be marked @objc because it has generic parameters}} +} From dad8f13f6ed27a673bae3a299ab87d28c2824c51 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Tue, 12 May 2020 21:23:34 -0700 Subject: [PATCH 09/15] [Sema] Eliminate a subscript AnyObject lookup ambiguity Previously we would always consider an AnyObject subscript lookup to be ambiguous if there was a candidate in both a class and protocol, even if the selectors and types matched. This was due to the protocol's generic signature preventing the signatures from being coalesced. Tweak the logic to strip generic signatures when comparing for AnyObject lookup, matching what we do for @objc methods. Resolves SR-8611. Resolves rdar://43645564 & rdar://62906344. --- lib/Sema/ConstraintSystem.cpp | 12 +++++++++++- test/Constraints/dynamic_lookup.swift | 22 ++++++++++++++++++---- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index db892b08f7668..f92a85935a67d 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -216,9 +216,19 @@ getDynamicResultSignature(ValueDecl *decl) { } if (auto asd = dyn_cast(decl)) { + auto ty = asd->getInterfaceType(); + + // Strip off a generic signature if we have one. This matches the logic + // for methods, and ensures that we don't take a protocol's generic + // signature into account for a subscript requirement. + if (auto *genericFn = ty->getAs()) { + ty = FunctionType::get(genericFn->getParams(), genericFn->getResult(), + genericFn->getExtInfo()); + } + // Handle properties and subscripts, anchored by the getter's selector. return std::make_tuple(asd->isStatic(), asd->getObjCGetterSelector(), - asd->getInterfaceType()->getCanonicalType()); + ty->getCanonicalType()); } llvm_unreachable("Not a valid @objc member"); diff --git a/test/Constraints/dynamic_lookup.swift b/test/Constraints/dynamic_lookup.swift index 27e284842bf3e..ce4398116c275 100644 --- a/test/Constraints/dynamic_lookup.swift +++ b/test/Constraints/dynamic_lookup.swift @@ -354,7 +354,11 @@ func dynamicInitCrash(ao: AnyObject.Type) { func unambiguousMethodParam(_ x: Int) subscript(ambiguousSubscript _: Int) -> String { get } // expected-note {{found this candidate}} - subscript(unambiguousSubscript _: String) -> Int { get } // expected-note {{found this candidate}} + subscript(unambiguousSubscript _: String) -> Int { get } + + subscript(differentSelectors _: Int) -> Int { // expected-note {{found this candidate}} + @objc(differentSelector1:) get + } } class C1 { @@ -368,7 +372,11 @@ class C1 { @objc func unambiguousMethodParam(_ x: Int) {} @objc subscript(ambiguousSubscript _: Int) -> Int { return 0 } // expected-note {{found this candidate}} - @objc subscript(unambiguousSubscript _: String) -> Int { return 0 } // expected-note {{found this candidate}} + @objc subscript(unambiguousSubscript _: String) -> Int { return 0 } + + @objc subscript(differentSelectors _: Int) -> Int { // expected-note {{found this candidate}} + @objc(differentSelector2:) get { return 0 } + } } class C2 { @@ -392,9 +400,15 @@ func testAnyObjectAmbiguity(_ x: AnyObject) { _ = x[singleCandidate: 0] _ = x[ambiguousSubscript: 0] // expected-error {{ambiguous use of 'subscript(ambiguousSubscript:)'}} + _ = x[ambiguousSubscript: 0] as Int + _ = x[ambiguousSubscript: 0] as String + + // SR-8611: Make sure we can coalesce subscripts with the same types and + // selectors through AnyObject lookup. + _ = x[unambiguousSubscript: ""] - // FIX-ME(SR-8611): This is currently ambiguous but shouldn't be. - _ = x[unambiguousSubscript: ""] // expected-error {{ambiguous use of 'subscript(unambiguousSubscript:)'}} + // But not if they have different selectors. + _ = x[differentSelectors: 0] // expected-error {{ambiguous use of 'subscript(differentSelectors:)}} } // SR-11648 From a3018081445f3f5511fbc5d6eb37e9b207bb7140 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Wed, 13 May 2020 00:27:52 -0400 Subject: [PATCH 10/15] Frontend: Go back to emitting index data after IRGen There's no longer a technical reason to do this, but it helps with crash analytics because AST crashes are less likely to end up happening during index emission. --- lib/FrontendTool/FrontendTool.cpp | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/lib/FrontendTool/FrontendTool.cpp b/lib/FrontendTool/FrontendTool.cpp index 316df9056c85b..32ab6ecae3faf 100644 --- a/lib/FrontendTool/FrontendTool.cpp +++ b/lib/FrontendTool/FrontendTool.cpp @@ -653,9 +653,6 @@ static void debugFailWithCrash() { LLVM_BUILTIN_TRAP; } -static void emitIndexDataIfNeeded(SourceFile *PrimarySourceFile, - const CompilerInstance &Instance); - static void countStatsOfSourceFile(UnifiedStatsReporter &Stats, const CompilerInstance &Instance, SourceFile *SF) { @@ -1115,13 +1112,16 @@ static bool performCompileStepsPostSema(CompilerInstance &Instance, return result; } +static void emitIndexDataForSourceFile(SourceFile *PrimarySourceFile, + const CompilerInstance &Instance); + /// Emits index data for all primary inputs, or the main module. static void emitIndexData(const CompilerInstance &Instance) { if (Instance.getPrimarySourceFiles().empty()) { - emitIndexDataIfNeeded(nullptr, Instance); + emitIndexDataForSourceFile(nullptr, Instance); } else { for (SourceFile *SF : Instance.getPrimarySourceFiles()) - emitIndexDataIfNeeded(SF, Instance); + emitIndexDataForSourceFile(SF, Instance); } } @@ -1288,12 +1288,13 @@ static bool performCompile(CompilerInstance &Instance, SWIFT_DEFER { // We might have freed the ASTContext already, but in that case we must have - // emitted the dependencies first. - if (Instance.hasASTContext()) + // emitted the dependencies and index first. + if (Instance.hasASTContext()) { emitReferenceDependenciesForAllPrimaryInputsIfNeeded(Instance); + emitIndexData(Instance); + } }; - emitIndexData(Instance); if (Context.hadError()) return true; @@ -1489,9 +1490,10 @@ static void freeASTContextIfPossible(CompilerInstance &Instance) { return; } - // Make sure we emit dependencies now, because we can't do it after the - // context is gone. + // Make sure we emit dependencies and index now, because we can't do it after + // the context is gone. emitReferenceDependenciesForAllPrimaryInputsIfNeeded(Instance); + emitIndexData(Instance); Instance.freeASTContext(); } @@ -1664,8 +1666,8 @@ static bool performCompileStepsPostSILGen(CompilerInstance &Instance, HadError; } -static void emitIndexDataIfNeeded(SourceFile *PrimarySourceFile, - const CompilerInstance &Instance) { +static void emitIndexDataForSourceFile(SourceFile *PrimarySourceFile, + const CompilerInstance &Instance) { const auto &Invocation = Instance.getInvocation(); const auto &opts = Invocation.getFrontendOptions(); From c9bbc14ed31a51572741db3106945c0fea4726a0 Mon Sep 17 00:00:00 2001 From: Dan Zheng Date: Tue, 12 May 2020 23:52:07 -0700 Subject: [PATCH 11/15] [AutoDiff] Simplify `@differentiable` attribute type-checking. Unify type-checking using `AnyFunctionType::getAutoDiffDerivativeFunctionLinearMapType`. Delete `checkDifferentiabilityParameters` helper, which is subsumed. Update tests with minor diagnostic changes. --- include/swift/AST/DiagnosticsSema.def | 8 +- lib/Sema/TypeCheckAttr.cpp | 156 +++++++----------- .../Sema/derivative_attr_type_checking.swift | 8 + .../differentiable_attr_type_checking.swift | 3 +- 4 files changed, 76 insertions(+), 99 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 362036d56da9d..3930e67949c22 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -2978,8 +2978,6 @@ ERROR(implements_attr_protocol_not_conformed_to,none, ERROR(differentiable_attr_no_vjp_or_jvp_when_linear,none, "cannot specify 'vjp:' or 'jvp:' for linear functions; use '@transpose' " "attribute for transpose registration instead", ()) -ERROR(differentiable_attr_void_result,none, - "cannot differentiate void function %0", (DeclName)) ERROR(differentiable_attr_overload_not_found,none, "%0 does not have expected type %1", (DeclNameRef, Type)) // TODO(TF-482): Change duplicate `@differentiable` attribute diagnostic to also @@ -2998,9 +2996,6 @@ ERROR(differentiable_attr_invalid_access,none, "derivative function %0 is required to either be public or " "'@usableFromInline' because the original function %1 is public or " "'@usableFromInline'", (DeclNameRef, DeclName)) -ERROR(differentiable_attr_result_not_differentiable,none, - "can only differentiate functions with results that conform to " - "'Differentiable', but %0 does not conform to 'Differentiable'", (Type)) ERROR(differentiable_attr_protocol_req_where_clause,none, "'@differentiable' attribute on protocol requirement cannot specify " "'where' clause", ()) @@ -3107,6 +3102,9 @@ ERROR(autodiff_attr_original_void_result,none, ERROR(autodiff_attr_original_multiple_semantic_results,none, "cannot differentiate functions with both an 'inout' parameter and a " "result", ()) +ERROR(autodiff_attr_result_not_differentiable,none, + "can only differentiate functions with results that conform to " + "'Differentiable', but %0 does not conform to 'Differentiable'", (Type)) // differentiation `wrt` parameters clause ERROR(diff_function_no_parameters,none, diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index 532de0f628d95..107f8d8c6d78c 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -3563,51 +3563,6 @@ static IndexSubset *computeDifferentiabilityParameters( return IndexSubset::get(ctx, parameterBits); } -// Checks if the given differentiability parameter indices are valid for the -// given original or derivative `AbstractFunctionDecl` and original function -// type in the given derivative generic environment and module context. Returns -// true on error. -// -// The parsed differentiability parameters and attribute location are used in -// diagnostics. -static bool checkDifferentiabilityParameters( - AbstractFunctionDecl *AFD, IndexSubset *diffParamIndices, - AnyFunctionType *functionType, GenericEnvironment *derivativeGenEnv, - ModuleDecl *module, ArrayRef parsedDiffParams, - SourceLoc attrLoc) { - auto &ctx = AFD->getASTContext(); - auto &diags = ctx.Diags; - - // Diagnose empty differentiability indices. No differentiability parameters - // were resolved or inferred. - if (diffParamIndices->isEmpty()) { - diags.diagnose(attrLoc, diag::diff_params_clause_no_inferred_parameters); - return true; - } - - // Check that differentiability parameters have allowed types. - SmallVector diffParams; - functionType->getSubsetParameters(diffParamIndices, diffParams); - for (unsigned i : range(diffParams.size())) { - SourceLoc loc = - parsedDiffParams.empty() ? attrLoc : parsedDiffParams[i].getLoc(); - auto diffParamType = diffParams[i].getPlainType(); - if (!diffParamType->hasTypeParameter()) - diffParamType = diffParamType->mapTypeOutOfContext(); - if (derivativeGenEnv) - diffParamType = derivativeGenEnv->mapTypeIntoContext(diffParamType); - else - diffParamType = AFD->mapTypeIntoContext(diffParamType); - // Parameter must conform to `Differentiable`. - if (!conformsToDifferentiable(diffParamType, AFD)) { - diags.diagnose(loc, diag::diff_params_clause_param_not_differentiable, - diffParamType); - return true; - } - } - return false; -} - // Returns the function declaration corresponding to the given function name and // lookup context. If the base type of the function is specified, member lookup // is performed. Otherwise, unqualified lookup is performed. @@ -4103,9 +4058,11 @@ bool resolveDifferentiableAttrDerivativeGenericSignature( /// `diffParamIndices`, and returns true. bool resolveDifferentiableAttrDifferentiabilityParameters( DifferentiableAttr *attr, AbstractFunctionDecl *original, - AnyFunctionType *derivativeFnTy, GenericEnvironment *derivativeGenEnv, + AnyFunctionType *originalFnRemappedTy, GenericEnvironment *derivativeGenEnv, IndexSubset *&diffParamIndices) { diffParamIndices = nullptr; + auto &ctx = original->getASTContext(); + auto &diags = ctx.Diags; // Get the parsed differentiability parameter indices, which have not yet been // resolved. Parsed differentiability parameter indices are defined only for @@ -4121,11 +4078,57 @@ bool resolveDifferentiableAttrDifferentiabilityParameters( } // Check if differentiability parameter indices are valid. - if (checkDifferentiabilityParameters(original, diffParamIndices, - derivativeFnTy, derivativeGenEnv, - original->getModuleContext(), - parsedDiffParams, attr->getLocation())) { + // Do this by compute the expected differential type and checking whether + // there is an error. + auto expectedLinearMapTypeOrError = + originalFnRemappedTy->getAutoDiffDerivativeFunctionLinearMapType( + diffParamIndices, AutoDiffLinearMapKind::Differential, + LookUpConformanceInModule(original->getModuleContext()), + /*makeSelfParamFirst*/ true); + + // Helper for diagnosing derivative function type errors. + auto errorHandler = [&](const DerivativeFunctionTypeError &error) { attr->setInvalid(); + switch (error.kind) { + case DerivativeFunctionTypeError::Kind::NoSemanticResults: + diags + .diagnose(attr->getLocation(), + diag::autodiff_attr_original_void_result, + original->getName()) + .highlight(original->getSourceRange()); + return; + case DerivativeFunctionTypeError::Kind::MultipleSemanticResults: + diags + .diagnose(attr->getLocation(), + diag::autodiff_attr_original_multiple_semantic_results) + .highlight(original->getSourceRange()); + return; + case DerivativeFunctionTypeError::Kind::NoDifferentiabilityParameters: + diags.diagnose(attr->getLocation(), + diag::diff_params_clause_no_inferred_parameters); + return; + case DerivativeFunctionTypeError::Kind:: + NonDifferentiableDifferentiabilityParameter: { + auto nonDiffParam = error.getNonDifferentiableTypeAndIndex(); + SourceLoc loc = parsedDiffParams.empty() + ? attr->getLocation() + : parsedDiffParams[nonDiffParam.second].getLoc(); + diags.diagnose(loc, diag::diff_params_clause_param_not_differentiable, + nonDiffParam.first); + return; + } + case DerivativeFunctionTypeError::Kind::NonDifferentiableResult: + auto nonDiffResult = error.getNonDifferentiableTypeAndIndex(); + diags.diagnose(attr->getLocation(), + diag::autodiff_attr_result_not_differentiable, + nonDiffResult.first); + return; + } + }; + // Diagnose any derivative function type errors. + if (!expectedLinearMapTypeOrError) { + auto error = expectedLinearMapTypeOrError.takeError(); + handleAllErrors(std::move(error), errorHandler); return true; } @@ -4222,52 +4225,19 @@ IndexSubset *DifferentiableAttributeTypeCheckRequest::evaluate( derivativeGenEnv = derivativeGenSig->getGenericEnvironment(); // Compute the derivative function type. - auto derivativeFnTy = originalFnTy; + auto originalFnRemappedTy = originalFnTy; if (derivativeGenEnv) - derivativeFnTy = derivativeGenEnv->mapTypeIntoContext(derivativeFnTy) - ->castTo(); + originalFnRemappedTy = + derivativeGenEnv->mapTypeIntoContext(originalFnRemappedTy) + ->castTo(); // Resolve and validate the differentiability parameters. IndexSubset *resolvedDiffParamIndices = nullptr; if (resolveDifferentiableAttrDifferentiabilityParameters( - attr, original, derivativeFnTy, derivativeGenEnv, + attr, original, originalFnRemappedTy, derivativeGenEnv, resolvedDiffParamIndices)) return nullptr; - // Get the original semantic result type. - llvm::SmallVector originalResults; - autodiff::getFunctionSemanticResultTypes(originalFnTy, originalResults, - derivativeGenEnv); - // Check that original function has at least one semantic result, i.e. - // that the original semantic result type is not `Void`. - if (originalResults.empty()) { - diags - .diagnose(attr->getLocation(), diag::autodiff_attr_original_void_result, - original->getName()) - .highlight(original->getSourceRange()); - attr->setInvalid(); - return nullptr; - } - // Check that original function does not have multiple semantic results. - if (originalResults.size() > 1) { - diags - .diagnose(attr->getLocation(), - diag::autodiff_attr_original_multiple_semantic_results) - .highlight(original->getSourceRange()); - attr->setInvalid(); - return nullptr; - } - auto originalResult = originalResults.front(); - auto originalResultTy = originalResult.type; - // Check that the original semantic result conforms to `Differentiable`. - if (!conformsToDifferentiable(originalResultTy, original)) { - diags.diagnose(attr->getLocation(), - diag::differentiable_attr_result_not_differentiable, - originalResultTy); - attr->setInvalid(); - return nullptr; - } - if (auto *asd = dyn_cast(D)) { // Remove `@differentiable` attribute from storage declaration to prevent // duplicate attribute registration during SILGen. @@ -4336,8 +4306,6 @@ static bool typeCheckDerivativeAttr(ASTContext &Ctx, Decl *D, if (checkIfDifferentiableProgrammingEnabled(Ctx, attr, D->getDeclContext())) return true; auto *derivative = cast(D); - auto lookupConformance = - LookUpConformanceInModule(D->getDeclContext()->getParentModule()); auto originalName = attr->getOriginalFunctionName(); auto *derivativeInterfaceType = @@ -4578,7 +4546,8 @@ static bool typeCheckDerivativeAttr(ASTContext &Ctx, Decl *D, // Compute the expected differential/pullback type. auto expectedLinearMapTypeOrError = originalFnType->getAutoDiffDerivativeFunctionLinearMapType( - resolvedDiffParamIndices, kind.getLinearMapKind(), lookupConformance, + resolvedDiffParamIndices, kind.getLinearMapKind(), + LookUpConformanceInModule(derivative->getModuleContext()), /*makeSelfParamFirst*/ true); // Helper for diagnosing derivative function type errors. @@ -4588,7 +4557,8 @@ static bool typeCheckDerivativeAttr(ASTContext &Ctx, Decl *D, case DerivativeFunctionTypeError::Kind::NoSemanticResults: diags .diagnose(attr->getLocation(), - diag::autodiff_attr_original_multiple_semantic_results) + diag::autodiff_attr_original_void_result, + originalAFD->getName()) .highlight(attr->getOriginalFunctionName().Loc.getSourceRange()); return; case DerivativeFunctionTypeError::Kind::MultipleSemanticResults: @@ -4614,7 +4584,7 @@ static bool typeCheckDerivativeAttr(ASTContext &Ctx, Decl *D, case DerivativeFunctionTypeError::Kind::NonDifferentiableResult: auto nonDiffResult = error.getNonDifferentiableTypeAndIndex(); diags.diagnose(attr->getLocation(), - diag::differentiable_attr_result_not_differentiable, + diag::autodiff_attr_result_not_differentiable, nonDiffResult.first); return; } diff --git a/test/AutoDiff/Sema/derivative_attr_type_checking.swift b/test/AutoDiff/Sema/derivative_attr_type_checking.swift index dd362a220d2a4..01d997d14560b 100644 --- a/test/AutoDiff/Sema/derivative_attr_type_checking.swift +++ b/test/AutoDiff/Sema/derivative_attr_type_checking.swift @@ -707,6 +707,14 @@ extension InoutParameters { ) { fatalError() } } +// Test no semantic results. + +func noSemanticResults(_ x: Float) {} + +// expected-error @+1 {{cannot differentiate void function 'noSemanticResults'}} +@derivative(of: noSemanticResults) +func vjpNoSemanticResults(_ x: Float) -> (value: Void, pullback: Void) {} + // Test multiple semantic results. extension InoutParameters { diff --git a/test/AutoDiff/Sema/differentiable_attr_type_checking.swift b/test/AutoDiff/Sema/differentiable_attr_type_checking.swift index 564e5e3f9b4a2..55cdfee43d9c6 100644 --- a/test/AutoDiff/Sema/differentiable_attr_type_checking.swift +++ b/test/AutoDiff/Sema/differentiable_attr_type_checking.swift @@ -92,7 +92,7 @@ func invalidDiffWrtClass(_ x: Class) -> Class { } protocol Proto {} -// expected-error @+1 {{can only differentiate with respect to parameters that conform to 'Differentiable', but 'Proto' does not conform to 'Differentiable'}} +// expected-error @+1 {{can only differentiate functions with results that conform to 'Differentiable', but 'Proto' does not conform to 'Differentiable'}} @differentiable(wrt: x) func invalidDiffWrtExistential(_ x: Proto) -> Proto { return x @@ -384,6 +384,7 @@ struct TF_521 { var real: T var imaginary: T + // expected-error @+1 {{can only differentiate functions with results that conform to 'Differentiable', but 'TF_521' does not conform to 'Differentiable'}} @differentiable(where T: Differentiable, T == T.TangentVector) init(real: T = 0, imaginary: T = 0) { self.real = real From 30b5fd52e8f9720cf62a5d15a4b50b24b68ce7ad Mon Sep 17 00:00:00 2001 From: Marcel Hlopko Date: Wed, 13 May 2020 13:34:28 +0200 Subject: [PATCH 12/15] [CxxInterop] Import C++ references. (#31702) --- include/swift/AST/Types.h | 10 +- lib/AST/Type.cpp | 3 +- lib/ClangImporter/ImportType.cpp | 66 +++++++++---- lib/ClangImporter/ImporterImpl.h | 7 +- lib/SIL/IR/AbstractionPattern.cpp | 2 + .../Cxx/reference/Inputs/module.modulemap | 3 + .../Cxx/reference/Inputs/reference.cpp | 19 ++++ test/Interop/Cxx/reference/Inputs/reference.h | 19 ++++ .../Cxx/reference/reference-irgen.swift | 71 ++++++++++++++ .../reference-module-interface.swift | 14 +++ .../Cxx/reference/reference-silgen.swift | 83 +++++++++++++++++ test/Interop/Cxx/reference/reference.swift | 92 +++++++++++++++++++ 12 files changed, 364 insertions(+), 25 deletions(-) create mode 100644 test/Interop/Cxx/reference/Inputs/module.modulemap create mode 100644 test/Interop/Cxx/reference/Inputs/reference.cpp create mode 100644 test/Interop/Cxx/reference/Inputs/reference.h create mode 100644 test/Interop/Cxx/reference/reference-irgen.swift create mode 100644 test/Interop/Cxx/reference/reference-module-interface.swift create mode 100644 test/Interop/Cxx/reference/reference-silgen.swift create mode 100644 test/Interop/Cxx/reference/reference.swift diff --git a/include/swift/AST/Types.h b/include/swift/AST/Types.h index 2ae23c4370b64..0bf41995878ab 100644 --- a/include/swift/AST/Types.h +++ b/include/swift/AST/Types.h @@ -2696,10 +2696,10 @@ enum class FunctionTypeRepresentation : uint8_t { /// A "thin" function that needs no context. Thin, - /// A C function pointer, which is thin and also uses the C calling - /// convention. + /// A C function pointer (or reference), which is thin and also uses the C + /// calling convention. CFunctionPointer, - + /// The value of the greatest AST function representation. Last = CFunctionPointer, }; @@ -2980,8 +2980,8 @@ class AnyFunctionType : public TypeBase { // We preserve a full clang::Type *, not a clang::FunctionType * as: // 1. We need to keep sugar in case we need to present an error to the user. // 2. The actual type being stored is [ignoring sugar] either a - // clang::PointerType or a clang::BlockPointerType which points to a - // clang::FunctionType. + // clang::PointerType, a clang::BlockPointerType, or a + // clang::ReferenceType which points to a clang::FunctionType. const clang::Type *ClangFunctionType; bool empty() const { return !ClangFunctionType; } diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index 67c2315c17ca8..37a9f4eb87341 100644 --- a/lib/AST/Type.cpp +++ b/lib/AST/Type.cpp @@ -3376,7 +3376,8 @@ void AnyFunctionType::ExtInfo::Uncommon::printClangFunctionType( void AnyFunctionType::ExtInfo::assertIsFunctionType(const clang::Type *type) { #ifndef NDEBUG - if (!(type->isFunctionPointerType() || type->isBlockPointerType())) { + if (!(type->isFunctionPointerType() || type->isBlockPointerType() || + type->isFunctionReferenceType())) { SmallString<256> buf; llvm::raw_svector_ostream os(buf); os << "Expected a Clang function type wrapped in a pointer type or " diff --git a/lib/ClangImporter/ImportType.cpp b/lib/ClangImporter/ImportType.cpp index 08238b491c2a5..0f4a2faf53086 100644 --- a/lib/ClangImporter/ImportType.cpp +++ b/lib/ClangImporter/ImportType.cpp @@ -159,6 +159,30 @@ namespace { explicit operator bool() const { return (bool) AbstractType; } }; + static ImportResult importFunctionPointerLikeType(const clang::Type &type, + const Type &pointeeType) { + auto funcTy = pointeeType->castTo(); + return {FunctionType::get( + funcTy->getParams(), funcTy->getResult(), + funcTy->getExtInfo() + .withRepresentation( + AnyFunctionType::Representation::CFunctionPointer) + .withClangFunctionType(&type)), + type.isReferenceType() ? ImportHint::None + : ImportHint::CFunctionPointer}; + } + + static ImportResult importOverAlignedFunctionPointerLikeType( + const clang::Type &type, ClangImporter::Implementation &Impl) { + auto opaquePointer = Impl.SwiftContext.getOpaquePointerDecl(); + if (!opaquePointer) { + return Type(); + } + return {opaquePointer->getDeclaredType(), + type.isReferenceType() ? ImportHint::None + : ImportHint::OtherPointer}; + } + class SwiftTypeConverter : public clang::TypeVisitor { @@ -406,23 +430,11 @@ namespace { // alignment is greater than the maximum Swift alignment, import as // OpaquePointer. if (!pointeeType || Impl.isOverAligned(pointeeQualType)) { - auto opaquePointer = Impl.SwiftContext.getOpaquePointerDecl(); - if (!opaquePointer) - return Type(); - return {opaquePointer->getDeclaredType(), - ImportHint::OtherPointer}; + return importOverAlignedFunctionPointerLikeType(*type, Impl); } - + if (pointeeQualType->isFunctionType()) { - auto funcTy = pointeeType->castTo(); - return { - FunctionType::get(funcTy->getParams(), funcTy->getResult(), - funcTy->getExtInfo() - .withRepresentation( - AnyFunctionType::Representation::CFunctionPointer) - .withClangFunctionType(type)), - ImportHint::CFunctionPointer - }; + return importFunctionPointerLikeType(*type, pointeeType); } PointerTypeKind pointerKind; @@ -472,7 +484,29 @@ namespace { } ImportResult VisitReferenceType(const clang::ReferenceType *type) { - return Type(); + auto pointeeQualType = type->getPointeeType(); + auto quals = pointeeQualType.getQualifiers(); + Type pointeeType = + Impl.importTypeIgnoreIUO(pointeeQualType, ImportTypeKind::Value, + AllowNSUIntegerAsInt, Bridgeability::None); + + if (pointeeQualType->isFunctionType()) { + return importFunctionPointerLikeType(*type, pointeeType); + } + + if (Impl.isOverAligned(pointeeQualType)) { + return importOverAlignedFunctionPointerLikeType(*type, Impl); + } + + PointerTypeKind pointerKind; + if (quals.hasConst()) { + pointerKind = PTK_UnsafePointer; + } else { + pointerKind = PTK_UnsafeMutablePointer; + } + + return {pointeeType->wrapInPointer(pointerKind), + ImportHint::None}; } ImportResult VisitMemberPointer(const clang::MemberPointerType *type) { diff --git a/lib/ClangImporter/ImporterImpl.h b/lib/ClangImporter/ImporterImpl.h index b974766d1a0e5..7a8d40f4bc3f1 100644 --- a/lib/ClangImporter/ImporterImpl.h +++ b/lib/ClangImporter/ImporterImpl.h @@ -135,9 +135,10 @@ enum class ImportTypeKind { /// Import the type of a function parameter. /// - /// This provides special treatment for C++ references (which become - /// [inout] parameters) and C pointers (which become magic [inout]-able types), - /// among other things, and enables the conversion of bridged types. + /// Special handling: + /// * C and C++ pointers become `UnsafePointer?` or `UnsafeMutablePointer?` + /// * C++ references become `UnsafePointer` or `UnsafeMutablePointer` + /// * Bridging that requires type conversions is allowed. /// Parameters are always considered CF-audited. Parameter, diff --git a/lib/SIL/IR/AbstractionPattern.cpp b/lib/SIL/IR/AbstractionPattern.cpp index ce5d24fb5ab37..fd89e9d9d8d54 100644 --- a/lib/SIL/IR/AbstractionPattern.cpp +++ b/lib/SIL/IR/AbstractionPattern.cpp @@ -319,6 +319,8 @@ getClangFunctionType(const clang::Type *clangType) { clangType = ptrTy->getPointeeType().getTypePtr(); } else if (auto blockTy = clangType->getAs()) { clangType = blockTy->getPointeeType().getTypePtr(); + } else if (auto refTy = clangType->getAs()) { + clangType = refTy->getPointeeType().getTypePtr(); } return clangType->castAs(); } diff --git a/test/Interop/Cxx/reference/Inputs/module.modulemap b/test/Interop/Cxx/reference/Inputs/module.modulemap new file mode 100644 index 0000000000000..06aa57364b223 --- /dev/null +++ b/test/Interop/Cxx/reference/Inputs/module.modulemap @@ -0,0 +1,3 @@ +module Reference { + header "reference.h" +} diff --git a/test/Interop/Cxx/reference/Inputs/reference.cpp b/test/Interop/Cxx/reference/Inputs/reference.cpp new file mode 100644 index 0000000000000..4e68fcfb42645 --- /dev/null +++ b/test/Interop/Cxx/reference/Inputs/reference.cpp @@ -0,0 +1,19 @@ +#include "reference.h" +#include + +static int staticInt = 42; + +int getStaticInt() { return staticInt; } +int &getStaticIntRef() { return staticInt; } +int &&getStaticIntRvalueRef() { return std::move(staticInt); } +const int &getConstStaticIntRef() { return staticInt; } +const int &&getConstStaticIntRvalueRef() { return std::move(staticInt); } + +void setStaticInt(int i) { staticInt = i; } +void setStaticIntRef(int &i) { staticInt = i; } +void setStaticIntRvalueRef(int &&i) { staticInt = i; } +void setConstStaticIntRef(const int &i) { staticInt = i; } +void setConstStaticIntRvalueRef(const int &&i) { staticInt = i; } + +auto getFuncRef() -> int (&)() { return getStaticInt; } +auto getFuncRvalueRef() -> int (&&)() { return getStaticInt; } diff --git a/test/Interop/Cxx/reference/Inputs/reference.h b/test/Interop/Cxx/reference/Inputs/reference.h new file mode 100644 index 0000000000000..a5beb2aba9dc3 --- /dev/null +++ b/test/Interop/Cxx/reference/Inputs/reference.h @@ -0,0 +1,19 @@ +#ifndef TEST_INTEROP_CXX_REFERENCE_INPUTS_REFERENCE_H +#define TEST_INTEROP_CXX_REFERENCE_INPUTS_REFERENCE_H + +int getStaticInt(); +int &getStaticIntRef(); +int &&getStaticIntRvalueRef(); +const int &getConstStaticIntRef(); +const int &&getConstStaticIntRvalueRef(); + +void setStaticInt(int); +void setStaticIntRef(int &); +void setStaticIntRvalueRef(int &&); +void setConstStaticIntRef(const int &); +void setConstStaticIntRvalueRef(const int &&); + +auto getFuncRef() -> int (&)(); +auto getFuncRvalueRef() -> int (&&)(); + +#endif // TEST_INTEROP_CXX_REFERENCE_INPUTS_REFERENCE_H diff --git a/test/Interop/Cxx/reference/reference-irgen.swift b/test/Interop/Cxx/reference/reference-irgen.swift new file mode 100644 index 0000000000000..cf5d26843849a --- /dev/null +++ b/test/Interop/Cxx/reference/reference-irgen.swift @@ -0,0 +1,71 @@ +// RUN: %target-swift-emit-ir -I %S/Inputs -enable-cxx-interop %s | %FileCheck %s + +import Reference + +public func getCxxRef() -> UnsafeMutablePointer { + return getStaticIntRef() +} + +// CHECK: define {{(protected |dllexport )?}}swiftcc i8* @"$s4main9getCxxRefSpys5Int32VGyF"() +// CHECK: call i32* @{{_Z15getStaticIntRefv|"\?getStaticIntRef@@YAAEAHXZ"}}() + +public func getConstCxxRef() -> UnsafePointer { + return getConstStaticIntRef() +} + +// CHECK: define {{(protected |dllexport )?}}swiftcc i8* @"$s4main14getConstCxxRefSPys5Int32VGyF"() +// CHECK: call i32* @{{_Z20getConstStaticIntRefv|"\?getConstStaticIntRef@@YAAEBHXZ"}}() + +public func getCxxRvalueRef() -> UnsafeMutablePointer { + return getStaticIntRvalueRef() +} + +// CHECK: define {{(protected |dllexport )?}}swiftcc i8* @"$s4main15getCxxRvalueRefSpys5Int32VGyF"() +// CHECK: call i32* @{{_Z21getStaticIntRvalueRefv|"\?getStaticIntRvalueRef@@YA\$\$QEAHXZ"}}() + +public func getConstCxxRvalueRef() -> UnsafePointer { + return getConstStaticIntRvalueRef() +} + +// CHECK: define {{(protected |dllexport )?}}swiftcc i8* @"$s4main20getConstCxxRvalueRefSPys5Int32VGyF"() +// CHECK: call i32* @{{_Z26getConstStaticIntRvalueRefv|"\?getConstStaticIntRvalueRef@@YA\$\$QEBHXZ"}}() + +public func setCxxRef() { + var val: CInt = 21 + withUnsafeMutablePointer(to: &val) { + setStaticIntRef($0) + } +} + +// CHECK: define {{(protected |dllexport )?}}swiftcc void @"$s4main9setCxxRefyyF"() +// CHECK: call void @{{_Z15setStaticIntRefRi|"\?setStaticIntRef@@YAXAEAH@Z"}}(i32* {{nonnull %val|%2}}) + +public func setCxxConstRef() { + var val: CInt = 21 + withUnsafePointer(to: &val) { + setConstStaticIntRef($0) + } +} + +// CHECK: define {{(protected |dllexport )?}}swiftcc void @"$s4main14setCxxConstRefyyF"() +// CHECK: call void @{{_Z20setConstStaticIntRefRKi|"\?setConstStaticIntRef@@YAXAEBH@Z"}}(i32* {{nonnull %val|%2}}) + +public func setCxxRvalueRef() { + var val: CInt = 21 + withUnsafeMutablePointer(to: &val) { + setStaticIntRvalueRef($0) + } +} + +// CHECK: define {{(protected |dllexport )?}}swiftcc void @"$s4main15setCxxRvalueRefyyF"() +// CHECK: call void @{{_Z21setStaticIntRvalueRefOi|"\?setStaticIntRvalueRef@@YAX\$\$QEAH@Z"}}(i32* {{nonnull %val|%2}}) + +public func setCxxConstRvalueRef() { + var val: CInt = 21 + withUnsafePointer(to: &val) { + setConstStaticIntRvalueRef($0) + } +} + +// CHECK: define {{(protected |dllexport )?}}swiftcc void @"$s4main20setCxxConstRvalueRefyyF"() +// CHECK: call void @{{_Z26setConstStaticIntRvalueRefOKi|"\?setConstStaticIntRvalueRef@@YAX\$\$QEBH@Z"}}(i32* {{nonnull %val|%2}}) diff --git a/test/Interop/Cxx/reference/reference-module-interface.swift b/test/Interop/Cxx/reference/reference-module-interface.swift new file mode 100644 index 0000000000000..82744a0d6581d --- /dev/null +++ b/test/Interop/Cxx/reference/reference-module-interface.swift @@ -0,0 +1,14 @@ +// RUN: %target-swift-ide-test -print-module -module-to-print=Reference -I %S/Inputs -source-filename=x -enable-cxx-interop | %FileCheck %s + +// CHECK: func getStaticInt() -> Int32 +// CHECK: func getStaticIntRef() -> UnsafeMutablePointer +// CHECK: func getStaticIntRvalueRef() -> UnsafeMutablePointer +// CHECK: func getConstStaticIntRef() -> UnsafePointer +// CHECK: func getConstStaticIntRvalueRef() -> UnsafePointer +// CHECK: func setStaticInt(_: Int32) +// CHECK: func setStaticIntRef(_: UnsafeMutablePointer) +// CHECK: func setStaticIntRvalueRef(_: UnsafeMutablePointer) +// CHECK: func setConstStaticIntRef(_: UnsafePointer) +// CHECK: func setConstStaticIntRvalueRef(_: UnsafePointer) +// CHECK: func getFuncRef() -> @convention(c) () -> Int32 +// CHECK: func getFuncRvalueRef() -> @convention(c) () -> Int32 diff --git a/test/Interop/Cxx/reference/reference-silgen.swift b/test/Interop/Cxx/reference/reference-silgen.swift new file mode 100644 index 0000000000000..9a58efb800319 --- /dev/null +++ b/test/Interop/Cxx/reference/reference-silgen.swift @@ -0,0 +1,83 @@ +// RUN: %target-swift-emit-sil -I %S/Inputs -enable-cxx-interop %s | %FileCheck %s + +import Reference + +func getCxxRef() -> UnsafeMutablePointer { + return getStaticIntRef() +} + +// CHECK: sil hidden @$s4main9getCxxRefSpys5Int32VGyF : $@convention(thin) () -> UnsafeMutablePointer +// CHECK: [[REF:%.*]] = function_ref @{{_Z15getStaticIntRefv|\?getStaticIntRef@@YAAEAHXZ}} : $@convention(c) () -> UnsafeMutablePointer +// CHECK: apply [[REF]]() : $@convention(c) () -> UnsafeMutablePointer + +func getConstCxxRef() -> UnsafePointer { + return getConstStaticIntRef() +} + +// CHECK: sil hidden @$s4main14getConstCxxRefSPys5Int32VGyF : $@convention(thin) () -> UnsafePointer +// CHECK: [[REF:%.*]] = function_ref @{{_Z20getConstStaticIntRefv|\?getConstStaticIntRef@@YAAEBHXZ}} : $@convention(c) () -> UnsafePointer +// CHECK: apply [[REF]]() : $@convention(c) () -> UnsafePointer + +func getCxxRvalueRef() -> UnsafeMutablePointer { + return getStaticIntRvalueRef() +} + +// CHECK: sil hidden @$s4main15getCxxRvalueRefSpys5Int32VGyF : $@convention(thin) () -> UnsafeMutablePointer +// CHECK: [[REF:%.*]] = function_ref @{{_Z21getStaticIntRvalueRefv|\?getStaticIntRvalueRef@@YA\$\$QEAHXZ}} : $@convention(c) () -> UnsafeMutablePointer +// CHECK: apply [[REF]]() : $@convention(c) () -> UnsafeMutablePointer + +func getConstCxxRvalueRef() -> UnsafePointer { + return getConstStaticIntRvalueRef() +} + +// CHECK: sil hidden @$s4main20getConstCxxRvalueRefSPys5Int32VGyF : $@convention(thin) () -> UnsafePointer +// CHECK: [[REF:%.*]] = function_ref @{{_Z26getConstStaticIntRvalueRefv|\?getConstStaticIntRvalueRef@@YA\$\$QEBHXZ}} : $@convention(c) () -> UnsafePointer +// CHECK: apply [[REF]]() : $@convention(c) () -> UnsafePointer + +func setCxxRef() { + var val: CInt = 21 + withUnsafeMutablePointer(to: &val) { + setStaticIntRef($0) + } +} + +// CHECK: // closure #1 in setCxxRef() +// CHECK: sil private @$s4main9setCxxRefyyFySpys5Int32VGXEfU_ : $@convention(thin) (UnsafeMutablePointer) -> () +// CHECK: [[REF:%.*]] = function_ref @{{_Z15setStaticIntRefRi|\?setStaticIntRef@@YAXAEAH@Z}} : $@convention(c) (UnsafeMutablePointer) -> () +// CHECK: apply [[REF]](%0) : $@convention(c) (UnsafeMutablePointer) -> () + +func setCxxConstRef() { + var val: CInt = 21 + withUnsafePointer(to: &val) { + setConstStaticIntRef($0) + } +} + +// CHECK: // closure #1 in setCxxConstRef() +// CHECK: sil private @$s4main14setCxxConstRefyyFySPys5Int32VGXEfU_ : $@convention(thin) (UnsafePointer) -> () +// CHECK: [[REF:%.*]] = function_ref @{{_Z20setConstStaticIntRefRKi|\?setConstStaticIntRef@@YAXAEBH@Z}} : $@convention(c) (UnsafePointer) -> () +// CHECK: apply [[REF]](%0) : $@convention(c) (UnsafePointer) -> () + +func setCxxRvalueRef() { + var val: CInt = 21 + withUnsafeMutablePointer(to: &val) { + setStaticIntRvalueRef($0) + } +} + +// CHECK: // closure #1 in setCxxRvalueRef() +// CHECK: sil private @$s4main15setCxxRvalueRefyyFySpys5Int32VGXEfU_ : $@convention(thin) (UnsafeMutablePointer) -> () +// CHECK: [[REF:%.*]] = function_ref @{{_Z21setStaticIntRvalueRefOi|\?setStaticIntRvalueRef@@YAX\$\$QEAH@Z}} : $@convention(c) (UnsafeMutablePointer) -> () +// CHECK: apply [[REF]](%0) : $@convention(c) (UnsafeMutablePointer) -> () + +func setCxxConstRvalueRef() { + var val: CInt = 21 + withUnsafePointer(to: &val) { + setConstStaticIntRvalueRef($0) + } +} + +// CHECK: // closure #1 in setCxxConstRvalueRef() +// CHECK: sil private @$s4main20setCxxConstRvalueRefyyFySPys5Int32VGXEfU_ : $@convention(thin) (UnsafePointer) -> () +// CHECK: [[REF:%.*]] = function_ref @{{_Z26setConstStaticIntRvalueRefOKi|\?setConstStaticIntRvalueRef@@YAX\$\$QEBH@Z}} : $@convention(c) (UnsafePointer) -> () +// CHECK: apply [[REF]](%0) : $@convention(c) (UnsafePointer) -> () diff --git a/test/Interop/Cxx/reference/reference.swift b/test/Interop/Cxx/reference/reference.swift new file mode 100644 index 0000000000000..71d211b04761c --- /dev/null +++ b/test/Interop/Cxx/reference/reference.swift @@ -0,0 +1,92 @@ +// RUN: %empty-directory(%t) +// RUN: %target-clang -c %S/Inputs/reference.cpp -I %S/Inputs -o %t/reference.o -std=c++17 +// RUN: %target-build-swift %s -I %S/Inputs -o %t/reference %t/reference.o -Xfrontend -enable-cxx-interop -Xcc -std=c++17 +// RUN: %target-codesign %t/reference +// RUN: %target-run %t/reference +// +// REQUIRES: executable_test + +import Reference +import StdlibUnittest + +var ReferenceTestSuite = TestSuite("Reference") + +ReferenceTestSuite.test("read-lvalue-reference") { + expectNotEqual(13, getStaticInt()) + setStaticInt(13) + expectEqual(13, getStaticIntRef().pointee) + expectEqual(13, getConstStaticIntRef().pointee) +} + +ReferenceTestSuite.test("read-rvalue-reference") { + expectNotEqual(32, getStaticInt()) + setStaticInt(32) + expectEqual(32, getStaticIntRvalueRef().pointee) + expectEqual(32, getConstStaticIntRvalueRef().pointee) +} + +ReferenceTestSuite.test("write-lvalue-reference") { + expectNotEqual(14, getStaticInt()) + getStaticIntRef().pointee = 14 + expectEqual(14, getStaticInt()) +} + +ReferenceTestSuite.test("write-rvalue-reference") { + expectNotEqual(41, getStaticInt()) + getStaticIntRvalueRef().pointee = 41 + expectEqual(41, getStaticInt()) +} + +ReferenceTestSuite.test("pass-lvalue-reference") { + expectNotEqual(21, getStaticInt()) + var val: CInt = 21 + withUnsafeMutablePointer(to: &val) { + setStaticIntRef($0) + } + expectEqual(21, getStaticInt()) +} + +ReferenceTestSuite.test("pass-const-lvalue-reference") { + expectNotEqual(22, getStaticInt()) + var val: CInt = 22 + withUnsafePointer(to: &val) { + setConstStaticIntRef($0) + } + expectEqual(22, getStaticInt()) +} + +ReferenceTestSuite.test("pass-rvalue-reference") { + expectNotEqual(52, getStaticInt()) + var val: CInt = 52 + withUnsafeMutablePointer(to: &val) { + setStaticIntRvalueRef($0) + } + expectEqual(52, getStaticInt()) +} + +ReferenceTestSuite.test("pass-const-rvalue-reference") { + expectNotEqual(53, getStaticInt()) + var val: CInt = 53 + withUnsafePointer(to: &val) { + setConstStaticIntRvalueRef($0) + } + expectEqual(53, getStaticInt()) +} + +ReferenceTestSuite.test("func-reference") { + let cxxF: @convention(c) () -> Int32 = getFuncRef() + + expectNotEqual(15, getStaticInt()) + setStaticInt(15) + expectEqual(15, cxxF()) +} + +ReferenceTestSuite.test("func-rvalue-reference") { + let cxxF: @convention(c) () -> Int32 = getFuncRvalueRef() + + expectNotEqual(61, getStaticInt()) + setStaticInt(61) + expectEqual(61, cxxF()) +} + +runAllTests() From e69abeba531bfbf5302ab13d70b7393551d4d04b Mon Sep 17 00:00:00 2001 From: Michael Forster Date: Wed, 13 May 2020 17:16:47 +0200 Subject: [PATCH 13/15] Classify C++ structs as loadable or address-only (#31707) * Classify C++ structs as loadable or address-only C++ structs are only loadable if they are trivially copyable. Resolves SR-12472. --- include/swift/AST/Decl.h | 14 +- lib/AST/Decl.cpp | 1 + lib/ClangImporter/ImportDecl.cpp | 20 ++ lib/SIL/IR/TypeLowering.cpp | 4 + .../Interop/Cxx/class/Inputs/loadable-types.h | 158 +++++++++++++++ .../Interop/Cxx/class/Inputs/module.modulemap | 4 + .../Cxx/class/loadable-types-silgen.swift | 180 ++++++++++++++++++ 7 files changed, 379 insertions(+), 2 deletions(-) create mode 100644 test/Interop/Cxx/class/Inputs/loadable-types.h create mode 100644 test/Interop/Cxx/class/loadable-types-silgen.swift diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index ce53e757dd562..288a6f805ad73 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -558,10 +558,12 @@ class alignas(1 << DeclAlignInBits) Decl { IsIncompatibleWithWeakReferences : 1 ); - SWIFT_INLINE_BITFIELD(StructDecl, NominalTypeDecl, 1, + SWIFT_INLINE_BITFIELD(StructDecl, NominalTypeDecl, 1+1, /// True if this struct has storage for fields that aren't accessible in /// Swift. - HasUnreferenceableStorage : 1 + HasUnreferenceableStorage : 1, + /// True if this struct is imported from C++ and not trivially copyable. + IsCxxNotTriviallyCopyable : 1 ); SWIFT_INLINE_BITFIELD(EnumDecl, NominalTypeDecl, 2+1, @@ -3822,6 +3824,14 @@ class StructDecl final : public NominalTypeDecl { void setHasUnreferenceableStorage(bool v) { Bits.StructDecl.HasUnreferenceableStorage = v; } + + bool isCxxNotTriviallyCopyable() const { + return Bits.StructDecl.IsCxxNotTriviallyCopyable; + } + + void setIsCxxNotTriviallyCopyable(bool v) { + Bits.StructDecl.IsCxxNotTriviallyCopyable = v; + } }; /// This is the base type for AncestryOptions. Each flag describes possible diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index b15bc60c1bb44..047b23bbe6234 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -4090,6 +4090,7 @@ StructDecl::StructDecl(SourceLoc StructLoc, Identifier Name, SourceLoc NameLoc, StructLoc(StructLoc) { Bits.StructDecl.HasUnreferenceableStorage = false; + Bits.StructDecl.IsCxxNotTriviallyCopyable = false; } bool NominalTypeDecl::hasMemberwiseInitializer() const { diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index ad1e40783062d..98dccac4e67fb 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -48,6 +48,7 @@ #include "clang/AST/DeclCXX.h" #include "clang/Basic/CharInfo.h" #include "swift/Basic/Statistic.h" +#include "clang/Basic/Specifiers.h" #include "clang/Basic/TargetInfo.h" #include "clang/Lex/Preprocessor.h" #include "clang/Sema/Lookup.h" @@ -3468,6 +3469,25 @@ namespace { result->setHasUnreferenceableStorage(hasUnreferenceableStorage); + if (auto cxxRecordDecl = dyn_cast(decl)) { + result->setIsCxxNotTriviallyCopyable( + !cxxRecordDecl->isTriviallyCopyable()); + + for (auto ctor : cxxRecordDecl->ctors()) { + if (ctor->isCopyConstructor() && + (ctor->isDeleted() || ctor->getAccess() != clang::AS_public)) { + result->setIsCxxNotTriviallyCopyable(true); + break; + } + } + + if (auto dtor = cxxRecordDecl->getDestructor()) { + if (dtor->isDeleted() || dtor->getAccess() != clang::AS_public) { + result->setIsCxxNotTriviallyCopyable(true); + } + } + } + return result; } diff --git a/lib/SIL/IR/TypeLowering.cpp b/lib/SIL/IR/TypeLowering.cpp index 60b8274e90b08..576d1c4f75109 100644 --- a/lib/SIL/IR/TypeLowering.cpp +++ b/lib/SIL/IR/TypeLowering.cpp @@ -1446,6 +1446,10 @@ namespace { if (handleResilience(structType, D, properties)) return handleAddressOnly(structType, properties); + if (D->isCxxNotTriviallyCopyable()) { + properties.setAddressOnly(); + } + auto subMap = structType->getContextSubstitutionMap(&TC.M, D); // Classify the type according to its stored properties. diff --git a/test/Interop/Cxx/class/Inputs/loadable-types.h b/test/Interop/Cxx/class/Inputs/loadable-types.h new file mode 100644 index 0000000000000..c2698961c4b41 --- /dev/null +++ b/test/Interop/Cxx/class/Inputs/loadable-types.h @@ -0,0 +1,158 @@ +#ifndef TEST_INTEROP_CXX_CLASS_INPUTS_LOADABLE_TYPES_H +#define TEST_INTEROP_CXX_CLASS_INPUTS_LOADABLE_TYPES_H + +struct EmptyStruct {}; + +// Tests for individual special members + +struct StructWithDefaultConstructor { + StructWithDefaultConstructor() {} +}; + +struct StructWithAdditionalConstructor { + StructWithAdditionalConstructor() {} + StructWithAdditionalConstructor(int parameter) {} +}; + +struct StructWithCopyConstructor { + StructWithCopyConstructor(const StructWithCopyConstructor &) {} +}; + +struct StructWithInheritedCopyConstructor : StructWithCopyConstructor {}; + +struct StructWithSubobjectCopyConstructor { + StructWithCopyConstructor subobject; +}; + +struct StructWithDefaultedCopyConstructor { + StructWithDefaultedCopyConstructor( + const StructWithDefaultedCopyConstructor &) = default; +}; + +struct StructWithInheritedDefaultedCopyConstructor + : StructWithDefaultedCopyConstructor {}; + +struct StructWithSubobjectDefaultedCopyConstructor { + StructWithDefaultedCopyConstructor subobject; +}; + +struct StructWithPrivateDefaultedCopyConstructor { +private: + StructWithPrivateDefaultedCopyConstructor( + const StructWithPrivateDefaultedCopyConstructor &) = default; +}; + +struct StructWithInheritedPrivateDefaultedCopyConstructor + : StructWithPrivateDefaultedCopyConstructor {}; + +struct StructWithSubobjectPrivateDefaultedCopyConstructor { + StructWithPrivateDefaultedCopyConstructor subobject; +}; + +struct StructWithMoveConstructor { + StructWithMoveConstructor(StructWithMoveConstructor &&) {} +}; + +struct StructWithInheritedMoveConstructor : StructWithMoveConstructor {}; + +struct StructWithSubobjectMoveConstructor { + StructWithMoveConstructor subobject; +}; + +struct StructWithCopyAssignment { + StructWithCopyAssignment &operator=(const StructWithCopyAssignment &) {} +}; + +struct StructWithInheritedCopyAssignment : StructWithCopyAssignment {}; + +struct StructWithSubobjectCopyAssignment { + StructWithCopyAssignment subobject; +}; + +struct StructWithMoveAssignment { + StructWithMoveAssignment &operator=(StructWithMoveAssignment &&) {} +}; + +struct StructWithInheritedMoveAssignment : StructWithMoveAssignment {}; + +struct StructWithSubobjectMoveAssignment { + StructWithMoveAssignment subobject; +}; + +struct StructWithDestructor { + ~StructWithDestructor(){} +}; + +struct StructWithInheritedDestructor : StructWithDestructor {}; + +struct StructWithSubobjectDestructor { + StructWithDestructor subobject; +}; + +struct StructWithDefaultedDestructor { + ~StructWithDefaultedDestructor() = default; +}; + +struct StructWithInheritedDefaultedDestructor : StructWithDefaultedDestructor { +}; + +struct StructWithSubobjectDefaultedDestructor { + StructWithDefaultedDestructor subobject; +}; + +struct StructWithPrivateDefaultedDestructor { +private: + ~StructWithPrivateDefaultedDestructor() = default; +}; + +struct StructWithInheritedPrivateDefaultedDestructor + : StructWithPrivateDefaultedDestructor {}; + +struct StructWithSubobjectPrivateDefaultedDestructor { + StructWithPrivateDefaultedDestructor subobject; +}; + +// Tests for common sets of special member functions. + +struct StructTriviallyCopyableMovable { + StructTriviallyCopyableMovable(const StructTriviallyCopyableMovable &) = + default; + StructTriviallyCopyableMovable(StructTriviallyCopyableMovable &&) = default; + StructTriviallyCopyableMovable & + operator=(const StructTriviallyCopyableMovable &) = default; + StructTriviallyCopyableMovable & + operator=(StructTriviallyCopyableMovable &&) = default; + ~StructTriviallyCopyableMovable() = default; +}; + +struct StructNonCopyableTriviallyMovable { + StructNonCopyableTriviallyMovable(const StructNonCopyableTriviallyMovable &) = + delete; + StructNonCopyableTriviallyMovable(StructNonCopyableTriviallyMovable &&) = + default; + StructNonCopyableTriviallyMovable & + operator=(const StructNonCopyableTriviallyMovable &) = delete; + StructNonCopyableTriviallyMovable & + operator=(StructNonCopyableTriviallyMovable &&) = default; + ~StructNonCopyableTriviallyMovable() = default; +}; + +struct StructNonCopyableNonMovable { + StructNonCopyableNonMovable(const StructNonCopyableNonMovable &) = delete; + StructNonCopyableNonMovable(StructNonCopyableNonMovable &&) = default; + StructNonCopyableNonMovable & + operator=(const StructNonCopyableNonMovable &) = delete; + StructNonCopyableNonMovable & + operator=(StructNonCopyableNonMovable &&) = default; + ~StructNonCopyableNonMovable() = default; +}; + +struct StructDeletedDestructor { + StructDeletedDestructor(const StructDeletedDestructor &) = default; + StructDeletedDestructor(StructDeletedDestructor &&) = default; + StructDeletedDestructor &operator=(const StructDeletedDestructor &) = default; + StructDeletedDestructor &operator=(StructDeletedDestructor &&) = default; + ~StructDeletedDestructor() = delete; +}; + +#endif diff --git a/test/Interop/Cxx/class/Inputs/module.modulemap b/test/Interop/Cxx/class/Inputs/module.modulemap index a7cb7a5feabef..cb61582d4b941 100644 --- a/test/Interop/Cxx/class/Inputs/module.modulemap +++ b/test/Interop/Cxx/class/Inputs/module.modulemap @@ -2,6 +2,10 @@ module AccessSpecifiers { header "access-specifiers.h" } +module LoadableTypes { + header "loadable-types.h" +} + module MemberwiseInitializer { header "memberwise-initializer.h" } diff --git a/test/Interop/Cxx/class/loadable-types-silgen.swift b/test/Interop/Cxx/class/loadable-types-silgen.swift new file mode 100644 index 0000000000000..7611f5d73d747 --- /dev/null +++ b/test/Interop/Cxx/class/loadable-types-silgen.swift @@ -0,0 +1,180 @@ +// RUN: %target-swift-emit-silgen -I %S/Inputs -enable-cxx-interop %s | %FileCheck %s + +// This test checks that we classify C++ types as loadable and address-only +// correctly. + +import LoadableTypes + +// Tests for individual special members + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}EmptyStruct) +func pass(s: EmptyStruct) { + // CHECK: bb0(%0 : $EmptyStruct): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithDefaultConstructor) +func pass(s: StructWithDefaultConstructor) { + // CHECK: bb0(%0 : $StructWithDefaultConstructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithAdditionalConstructor) +func pass(s: StructWithAdditionalConstructor) { + // CHECK: bb0(%0 : $StructWithAdditionalConstructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithCopyConstructor) +func pass(s: StructWithCopyConstructor) { + // CHECK: bb0(%0 : $*StructWithCopyConstructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithInheritedCopyConstructor) +func pass(s: StructWithInheritedCopyConstructor) { + // CHECK: bb0(%0 : $*StructWithInheritedCopyConstructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithSubobjectCopyConstructor) +func pass(s: StructWithSubobjectCopyConstructor) { + // CHECK: bb0(%0 : $*StructWithSubobjectCopyConstructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithDefaultedCopyConstructor) +func pass(s: StructWithDefaultedCopyConstructor) { + // CHECK: bb0(%0 : $StructWithDefaultedCopyConstructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithInheritedDefaultedCopyConstructor) +func pass(s: StructWithInheritedDefaultedCopyConstructor) { + // CHECK: bb0(%0 : $StructWithInheritedDefaultedCopyConstructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithSubobjectDefaultedCopyConstructor) +func pass(s: StructWithSubobjectDefaultedCopyConstructor) { + // CHECK: bb0(%0 : $StructWithSubobjectDefaultedCopyConstructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithPrivateDefaultedCopyConstructor) +func pass(s: StructWithPrivateDefaultedCopyConstructor) { + // CHECK: bb0(%0 : $*StructWithPrivateDefaultedCopyConstructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithInheritedPrivateDefaultedCopyConstructor) +func pass(s: StructWithInheritedPrivateDefaultedCopyConstructor) { + // CHECK: bb0(%0 : $*StructWithInheritedPrivateDefaultedCopyConstructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithSubobjectPrivateDefaultedCopyConstructor) +func pass(s: StructWithSubobjectPrivateDefaultedCopyConstructor) { + // CHECK: bb0(%0 : $*StructWithSubobjectPrivateDefaultedCopyConstructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithMoveConstructor) +func pass(s: StructWithMoveConstructor) { + // CHECK: bb0(%0 : $*StructWithMoveConstructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithInheritedMoveConstructor) +func pass(s: StructWithInheritedMoveConstructor) { + // CHECK: bb0(%0 : $*StructWithInheritedMoveConstructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithSubobjectMoveConstructor) +func pass(s: StructWithSubobjectMoveConstructor) { + // CHECK: bb0(%0 : $*StructWithSubobjectMoveConstructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithCopyAssignment) +func pass(s: StructWithCopyAssignment) { + // CHECK: bb0(%0 : $*StructWithCopyAssignment): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithInheritedCopyAssignment) +func pass(s: StructWithInheritedCopyAssignment) { + // CHECK: bb0(%0 : $*StructWithInheritedCopyAssignment): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithSubobjectCopyAssignment) +func pass(s: StructWithSubobjectCopyAssignment) { + // CHECK: bb0(%0 : $*StructWithSubobjectCopyAssignment): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithMoveAssignment) +func pass(s: StructWithMoveAssignment) { + // CHECK: bb0(%0 : $*StructWithMoveAssignment): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithInheritedMoveAssignment) +func pass(s: StructWithInheritedMoveAssignment) { + // CHECK: bb0(%0 : $*StructWithInheritedMoveAssignment): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithSubobjectMoveAssignment) +func pass(s: StructWithSubobjectMoveAssignment) { + // CHECK: bb0(%0 : $*StructWithSubobjectMoveAssignment): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithDestructor) +func pass(s: StructWithDestructor) { + // CHECK: bb0(%0 : $*StructWithDestructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithInheritedDestructor) +func pass(s: StructWithInheritedDestructor) { + // CHECK: bb0(%0 : $*StructWithInheritedDestructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithSubobjectDestructor) +func pass(s: StructWithSubobjectDestructor) { + // CHECK: bb0(%0 : $*StructWithSubobjectDestructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithDefaultedDestructor) +func pass(s: StructWithDefaultedDestructor) { + // CHECK: bb0(%0 : $StructWithDefaultedDestructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithInheritedDefaultedDestructor) +func pass(s: StructWithInheritedDefaultedDestructor) { + // CHECK: bb0(%0 : $StructWithInheritedDefaultedDestructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithSubobjectDefaultedDestructor) +func pass(s: StructWithSubobjectDefaultedDestructor) { + // CHECK: bb0(%0 : $StructWithSubobjectDefaultedDestructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithPrivateDefaultedDestructor) +func pass(s: StructWithPrivateDefaultedDestructor) { + // CHECK: bb0(%0 : $*StructWithPrivateDefaultedDestructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithInheritedPrivateDefaultedDestructor) +func pass(s: StructWithInheritedPrivateDefaultedDestructor) { + // CHECK: bb0(%0 : $*StructWithInheritedPrivateDefaultedDestructor): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructWithSubobjectPrivateDefaultedDestructor) +func pass(s: StructWithSubobjectPrivateDefaultedDestructor) { + // CHECK: bb0(%0 : $*StructWithSubobjectPrivateDefaultedDestructor): +} + +// Tests for common sets of special member functions. + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructTriviallyCopyableMovable) +func pass(s: StructTriviallyCopyableMovable) { + // CHECK: bb0(%0 : $StructTriviallyCopyableMovable): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructNonCopyableTriviallyMovable) +func pass(s: StructNonCopyableTriviallyMovable) { + // CHECK: bb0(%0 : $*StructNonCopyableTriviallyMovable): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructNonCopyableNonMovable) +func pass(s: StructNonCopyableNonMovable) { + // CHECK: bb0(%0 : $*StructNonCopyableNonMovable): +} + +// CHECK-LABEL: sil hidden [ossa] @$s4main4pass{{.*[ (]}}StructDeletedDestructor) +func pass(s: StructDeletedDestructor) { + // CHECK: bb0(%0 : $*StructDeletedDestructor): +} From 96313ce3e19eecd82c729565ffa8005d8f72395d Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Wed, 13 May 2020 00:05:16 +0000 Subject: [PATCH 14/15] runtime: explicitly namespace ArrayRef in shared headers There are a set of headers shared between the Swift compiler and the runtime. Ensure that we explicitly use `llvm::ArrayRef` rather than `ArrayRef` which is aliased to `::llvm::ArrayRef`. Doing so enables us to replace the `ArrayRef` with an inline namespaced version fixing ODR violations when the swift runtime is loaded into an address space with LLVM. --- include/swift/ABI/Metadata.h | 35 ++++++----- include/swift/Basic/LLVM.h | 6 +- include/swift/Demangling/TypeDecoder.h | 3 +- include/swift/Reflection/TypeRef.h | 38 ++++++------ include/swift/Reflection/TypeRefBuilder.h | 30 +++++---- include/swift/Runtime/Numeric.h | 6 +- stdlib/public/Reflection/TypeRef.cpp | 6 +- stdlib/public/runtime/Metadata.cpp | 2 +- stdlib/public/runtime/MetadataLookup.cpp | 75 ++++++++++++----------- stdlib/public/runtime/Private.h | 2 +- 10 files changed, 103 insertions(+), 100 deletions(-) diff --git a/include/swift/ABI/Metadata.h b/include/swift/ABI/Metadata.h index d57ec0db0315f..4c8e9660a31f2 100644 --- a/include/swift/ABI/Metadata.h +++ b/include/swift/ABI/Metadata.h @@ -2000,7 +2000,7 @@ struct TargetExistentialTypeMetadata } /// Retrieve the set of protocols required by the existential. - ArrayRef getProtocols() const { + llvm::ArrayRef getProtocols() const { return { this->template getTrailingObjects(), NumProtocols }; } @@ -2013,7 +2013,7 @@ struct TargetExistentialTypeMetadata } /// Retrieve the set of protocols required by the existential. - MutableArrayRef getMutableProtocols() { + llvm::MutableArrayRef getMutableProtocols() { return { this->template getTrailingObjects(), NumProtocols }; } @@ -2535,13 +2535,13 @@ struct TargetProtocolConformanceDescriptor final getWitnessTable(const TargetMetadata *type) const; /// Retrieve the resilient witnesses. - ArrayRef getResilientWitnesses() const{ + llvm::ArrayRef getResilientWitnesses() const { if (!Flags.hasResilientWitnesses()) return { }; - return ArrayRef( - this->template getTrailingObjects(), - numTrailingObjects(OverloadToken())); + return llvm::ArrayRef( + this->template getTrailingObjects(), + numTrailingObjects(OverloadToken())); } ConstTargetPointer @@ -2828,23 +2828,23 @@ class TargetGenericEnvironment public: /// Retrieve the cumulative generic parameter counts at each level of genericity. - ArrayRef getGenericParameterCounts() const { - return ArrayRef(this->template getTrailingObjects(), + llvm::ArrayRef getGenericParameterCounts() const { + return llvm::makeArrayRef(this->template getTrailingObjects(), Flags.getNumGenericParameterLevels()); } /// Retrieve the generic parameters descriptors. - ArrayRef getGenericParameters() const { - return ArrayRef( - this->template getTrailingObjects(), - getGenericParameterCounts().back()); + llvm::ArrayRef getGenericParameters() const { + return llvm::makeArrayRef( + this->template getTrailingObjects(), + getGenericParameterCounts().back()); } /// Retrieve the generic requirements. - ArrayRef getGenericRequirements() const { - return ArrayRef( - this->template getTrailingObjects(), - Flags.getNumGenericRequirements()); + llvm::ArrayRef getGenericRequirements() const { + return llvm::makeArrayRef( + this->template getTrailingObjects(), + Flags.getNumGenericRequirements()); } }; @@ -4604,7 +4604,8 @@ class DynamicReplacementScope DynamicReplacementDescriptor>; friend TrailingObjects; - ArrayRef getReplacementDescriptors() const { + llvm::ArrayRef + getReplacementDescriptors() const { return {this->template getTrailingObjects(), numReplacements}; } diff --git a/include/swift/Basic/LLVM.h b/include/swift/Basic/LLVM.h index e3876ae81d59c..7638de645138b 100644 --- a/include/swift/Basic/LLVM.h +++ b/include/swift/Basic/LLVM.h @@ -38,8 +38,10 @@ namespace llvm { template class SmallVector; template class SmallString; template class SmallSetVector; +#if !defined(swiftCore_EXPORTS) template class ArrayRef; template class MutableArrayRef; +#endif template class TinyPtrVector; template class Optional; template class PointerUnion; @@ -63,9 +65,11 @@ namespace swift { using llvm::cast_or_null; // Containers. +#if !defined(swiftCore_EXPORTS) using llvm::ArrayRef; - using llvm::iterator_range; using llvm::MutableArrayRef; +#endif + using llvm::iterator_range; using llvm::None; using llvm::Optional; using llvm::PointerUnion; diff --git a/include/swift/Demangling/TypeDecoder.h b/include/swift/Demangling/TypeDecoder.h index 4cc6c53d4b5d5..9e69717ac3b3f 100644 --- a/include/swift/Demangling/TypeDecoder.h +++ b/include/swift/Demangling/TypeDecoder.h @@ -23,6 +23,7 @@ #include "swift/Basic/LLVM.h" #include "swift/Runtime/Unreachable.h" #include "swift/Strings.h" +#include "llvm/ADT/ArrayRef.h" #include namespace swift { @@ -876,7 +877,7 @@ class TypeDecoder { } } genericArgsLevels.push_back(genericArgsBuf.size()); - std::vector> genericArgs; + std::vector> genericArgs; for (unsigned i = 0; i < genericArgsLevels.size() - 1; ++i) { auto start = genericArgsLevels[i], end = genericArgsLevels[i+1]; genericArgs.emplace_back(genericArgsBuf.data() + start, diff --git a/include/swift/Reflection/TypeRef.h b/include/swift/Reflection/TypeRef.h index aacc09f2bb00a..505a1acbc6aba 100644 --- a/include/swift/Reflection/TypeRef.h +++ b/include/swift/Reflection/TypeRef.h @@ -346,11 +346,11 @@ class OpaqueArchetypeTypeRef final : public TypeRef { // Each ArrayRef in ArgumentLists references into the buffer owned by this // vector, which must not be modified after construction. std::vector AllArgumentsBuf; - std::vector> ArgumentLists; - - static TypeRefID Profile(StringRef idString, - StringRef description, unsigned ordinal, - ArrayRef> argumentLists) { + std::vector> ArgumentLists; + + static TypeRefID + Profile(StringRef idString, StringRef description, unsigned ordinal, + llvm::ArrayRef> argumentLists) { TypeRefID ID; ID.addString(idString.str()); ID.addInteger(ordinal); @@ -362,14 +362,13 @@ class OpaqueArchetypeTypeRef final : public TypeRef { return ID; } - + public: - OpaqueArchetypeTypeRef(StringRef id, - StringRef description, unsigned ordinal, - ArrayRef> argumentLists) - : TypeRef(TypeRefKind::OpaqueArchetype), - ID(id), Description(description), Ordinal(ordinal) - { + OpaqueArchetypeTypeRef( + StringRef id, StringRef description, unsigned ordinal, + llvm::ArrayRef> argumentLists) + : TypeRef(TypeRefKind::OpaqueArchetype), ID(id), Description(description), + Ordinal(ordinal) { std::vector argumentListLengths; for (auto argList : argumentLists) { @@ -379,25 +378,24 @@ class OpaqueArchetypeTypeRef final : public TypeRef { } auto *data = AllArgumentsBuf.data(); for (auto length : argumentListLengths) { - ArgumentLists.push_back(ArrayRef(data, length)); + ArgumentLists.push_back(llvm::ArrayRef(data, length)); data += length; } assert(data == AllArgumentsBuf.data() + AllArgumentsBuf.size()); } - + template - static const OpaqueArchetypeTypeRef *create(Allocator &A, - StringRef id, StringRef description, - unsigned ordinal, - ArrayRef> arguments) { + static const OpaqueArchetypeTypeRef * + create(Allocator &A, StringRef id, StringRef description, unsigned ordinal, + llvm::ArrayRef> arguments) { FIND_OR_CREATE_TYPEREF(A, OpaqueArchetypeTypeRef, id, description, ordinal, arguments); } - ArrayRef> getArgumentLists() const { + llvm::ArrayRef> getArgumentLists() const { return ArgumentLists; } - + unsigned getOrdinal() const { return Ordinal; } diff --git a/include/swift/Reflection/TypeRefBuilder.h b/include/swift/Reflection/TypeRefBuilder.h index 1e098d937a107..42b617cf7e367 100644 --- a/include/swift/Reflection/TypeRefBuilder.h +++ b/include/swift/Reflection/TypeRefBuilder.h @@ -361,14 +361,14 @@ class TypeRefBuilder { const BoundGenericTypeRef * createBoundGenericType(const Optional &mangledName, - ArrayRef args, + llvm::ArrayRef args, const TypeRef *parent) { return BoundGenericTypeRef::create(*this, *mangledName, args, parent); } - + const TypeRef * resolveOpaqueType(NodePointer opaqueDescriptor, - ArrayRef> genericArgs, + llvm::ArrayRef> genericArgs, unsigned ordinal) { // TODO: Produce a type ref for the opaque type if the underlying type isn't // available. @@ -403,26 +403,25 @@ class TypeRefBuilder { genericArgs); } - const TupleTypeRef * - createTupleType(ArrayRef elements, - std::string &&labels, bool isVariadic) { + const TupleTypeRef *createTupleType(llvm::ArrayRef elements, + std::string &&labels, bool isVariadic) { // FIXME: Add uniqueness checks in TupleTypeRef::Profile and // unittests/Reflection/TypeRef.cpp if using labels for identity. return TupleTypeRef::create(*this, elements, isVariadic); } const FunctionTypeRef *createFunctionType( - ArrayRef> params, + llvm::ArrayRef> params, const TypeRef *result, FunctionTypeFlags flags) { return FunctionTypeRef::create(*this, params, result, flags); } const FunctionTypeRef *createImplFunctionType( - Demangle::ImplParameterConvention calleeConvention, - ArrayRef> params, - ArrayRef> results, - Optional> errorResult, - ImplFunctionTypeFlags flags) { + Demangle::ImplParameterConvention calleeConvention, + llvm::ArrayRef> params, + llvm::ArrayRef> results, + Optional> errorResult, + ImplFunctionTypeFlags flags) { // Minimal support for lowered function types. These come up in // reflection as capture types. For the reflection library's // purposes, the only part that matters is the convention. @@ -451,9 +450,8 @@ class TypeRefBuilder { } const ProtocolCompositionTypeRef * - createProtocolCompositionType(ArrayRef protocols, - BuiltType superclass, - bool isClassBound) { + createProtocolCompositionType(llvm::ArrayRef protocols, + BuiltType superclass, bool isClassBound) { std::vector protocolRefs; for (const auto &protocol : protocols) { if (!protocol) @@ -530,7 +528,7 @@ class TypeRefBuilder { const ObjCClassTypeRef * createBoundGenericObjCClassType(const std::string &name, - ArrayRef args) { + llvm::ArrayRef args) { // Remote reflection just ignores generic arguments for Objective-C // lightweight generic types, since they don't affect layout. return createObjCClassType(name); diff --git a/include/swift/Runtime/Numeric.h b/include/swift/Runtime/Numeric.h index 6d91e13040681..0d52e97d580fd 100644 --- a/include/swift/Runtime/Numeric.h +++ b/include/swift/Runtime/Numeric.h @@ -44,9 +44,9 @@ class IntegerLiteral { /// Return the chunks of data making up this value, arranged starting from /// the least-significant chunk. The value is sign-extended to fill the /// final chunk. - ArrayRef getData() const { - return ArrayRef(Data, - (Flags.getBitWidth() + BitsPerChunk - 1) / BitsPerChunk); + llvm::ArrayRef getData() const { + return llvm::makeArrayRef(Data, (Flags.getBitWidth() + BitsPerChunk - 1) / + BitsPerChunk); } /// The flags for this value. diff --git a/stdlib/public/Reflection/TypeRef.cpp b/stdlib/public/Reflection/TypeRef.cpp index de02475d36c3c..b27624338c839 100644 --- a/stdlib/public/Reflection/TypeRef.cpp +++ b/stdlib/public/Reflection/TypeRef.cpp @@ -1072,9 +1072,9 @@ class TypeRefSubstitution newArgsBuffer.push_back(visit(arg)); } } - - std::vector> newArgLists; - + + std::vector> newArgLists; + return OpaqueArchetypeTypeRef::create(Builder, O->getID(), O->getDescription(), O->getOrdinal(), newArgLists); diff --git a/stdlib/public/runtime/Metadata.cpp b/stdlib/public/runtime/Metadata.cpp index 33c875d07fd3f..bcedf1ee50aa7 100644 --- a/stdlib/public/runtime/Metadata.cpp +++ b/stdlib/public/runtime/Metadata.cpp @@ -5311,7 +5311,7 @@ checkTransitiveCompleteness(const Metadata *initialType) { /// Diagnose a metadata dependency cycle. SWIFT_NORETURN static void diagnoseMetadataDependencyCycle(const Metadata *start, - ArrayRef links) { + llvm::ArrayRef links) { assert(start == links.back().Value); std::string diagnostic = diff --git a/stdlib/public/runtime/MetadataLookup.cpp b/stdlib/public/runtime/MetadataLookup.cpp index 14aee3c589f8a..476cbfec2697e 100644 --- a/stdlib/public/runtime/MetadataLookup.cpp +++ b/stdlib/public/runtime/MetadataLookup.cpp @@ -892,9 +892,9 @@ struct FieldDescriptorCacheEntry { #pragma mark Metadata lookup via mangled name -Optional swift::_depthIndexToFlatIndex( - unsigned depth, unsigned index, - ArrayRef paramCounts) { +Optional +swift::_depthIndexToFlatIndex(unsigned depth, unsigned index, + llvm::ArrayRef paramCounts) { // Out-of-bounds depth. if (depth >= paramCounts.size()) return None; @@ -945,8 +945,8 @@ bool swift::_gatherGenericParameterCounts( } /// Retrieve the generic parameters introduced in this context. -static ArrayRef getLocalGenericParams( - const ContextDescriptor *context) { +static llvm::ArrayRef +getLocalGenericParams(const ContextDescriptor *context) { if (!context->isGeneric()) return { }; @@ -961,13 +961,13 @@ static ArrayRef getLocalGenericParams( return genericContext->getGenericParams().slice(startParamIndex); } -static bool _gatherGenericParameters( - const ContextDescriptor *context, - ArrayRef genericArgs, - const Metadata *parent, - SmallVectorImpl &genericParamCounts, - SmallVectorImpl &allGenericArgsVec, - Demangler &demangler) { +static bool +_gatherGenericParameters(const ContextDescriptor *context, + llvm::ArrayRef genericArgs, + const Metadata *parent, + llvm::SmallVectorImpl &genericParamCounts, + llvm::SmallVectorImpl &allGenericArgsVec, + Demangler &demangler) { // Figure out the various levels of generic parameters we have in // this type. (void)_gatherGenericParameterCounts(context, @@ -1176,9 +1176,10 @@ class DecodedMetadataBuilder { Demangle::NodeFactory &getNodeFactory() { return demangler; } - BuiltType resolveOpaqueType(NodePointer opaqueDecl, - ArrayRef> genericArgs, - unsigned ordinal) { + BuiltType + resolveOpaqueType(NodePointer opaqueDecl, + llvm::ArrayRef> genericArgs, + unsigned ordinal) { auto descriptor = _findOpaqueTypeDescriptor(opaqueDecl, demangler); if (!descriptor) return BuiltType(); @@ -1212,7 +1213,7 @@ class DecodedMetadataBuilder { return substitutions.getWitnessTable(type, index); }).getMetadata(); } - + BuiltTypeDecl createTypeDecl(NodePointer node, bool &typeAlias) const { // Look for a nominal type descriptor based on its mangled name. @@ -1255,8 +1256,9 @@ class DecodedMetadataBuilder { #endif } - BuiltType createBoundGenericObjCClassType(const std::string &mangledName, - ArrayRef args) const { + BuiltType + createBoundGenericObjCClassType(const std::string &mangledName, + llvm::ArrayRef args) const { // Generic arguments of lightweight Objective-C generic classes are not // reified in the metadata. return createObjCClassType(mangledName); @@ -1278,7 +1280,7 @@ class DecodedMetadataBuilder { } BuiltType createBoundGenericType(BuiltTypeDecl anyTypeDecl, - ArrayRef genericArgs, + llvm::ArrayRef genericArgs, BuiltType parent) const { auto typeDecl = dyn_cast(anyTypeDecl); if (!typeDecl) { @@ -1326,9 +1328,9 @@ class DecodedMetadataBuilder { return swift_getExistentialMetatypeMetadata(instance); } - BuiltType createProtocolCompositionType(ArrayRef protocols, - BuiltType superclass, - bool isClassBound) const { + BuiltType + createProtocolCompositionType(llvm::ArrayRef protocols, + BuiltType superclass, bool isClassBound) const { // Determine whether we have a class bound. ProtocolClassConstraint classConstraint = ProtocolClassConstraint::Any; if (isClassBound || superclass) { @@ -1360,9 +1362,9 @@ class DecodedMetadataBuilder { return BuiltType(); } - BuiltType createFunctionType( - ArrayRef> params, - BuiltType result, FunctionTypeFlags flags) const { + BuiltType + createFunctionType(llvm::ArrayRef> params, + BuiltType result, FunctionTypeFlags flags) const { SmallVector paramTypes; SmallVector paramFlags; @@ -1384,18 +1386,17 @@ class DecodedMetadataBuilder { } BuiltType createImplFunctionType( - Demangle::ImplParameterConvention calleeConvention, - ArrayRef> params, - ArrayRef> results, - Optional> errorResult, - ImplFunctionTypeFlags flags) { + Demangle::ImplParameterConvention calleeConvention, + llvm::ArrayRef> params, + llvm::ArrayRef> results, + Optional> errorResult, + ImplFunctionTypeFlags flags) { // We can't realize the metadata for a SILFunctionType. return BuiltType(); } - BuiltType createTupleType(ArrayRef elements, - std::string labels, - bool variadic) const { + BuiltType createTupleType(llvm::ArrayRef elements, + std::string labels, bool variadic) const { // TODO: 'variadic' should no longer exist auto flags = TupleTypeFlags().withNumElements(elements.size()); if (!labels.empty()) @@ -2195,8 +2196,8 @@ class AutomaticDynamicReplacements AutomaticDynamicReplacementEntry>; friend TrailingObjects; - - ArrayRef getReplacementEntries() const { + llvm::ArrayRef + getReplacementEntries() const { return { this->template getTrailingObjects(), numScopes}; @@ -2239,8 +2240,8 @@ class AutomaticDynamicReplacementsSome DynamicReplacementSomeDescriptor>; friend TrailingObjects; - - ArrayRef getReplacementEntries() const { + llvm::ArrayRef + getReplacementEntries() const { return { this->template getTrailingObjects(), numEntries}; diff --git a/stdlib/public/runtime/Private.h b/stdlib/public/runtime/Private.h index 56bd6c7953698..7e13941885eb8 100644 --- a/stdlib/public/runtime/Private.h +++ b/stdlib/public/runtime/Private.h @@ -286,7 +286,7 @@ class TypeInfo { /// An element in the descriptor path. struct PathElement { /// The generic parameters local to this element. - ArrayRef localGenericParams; + llvm::ArrayRef localGenericParams; /// The total number of generic parameters. unsigned numTotalGenericParams; From cc68375efdf273f5b53e51ab9326fa59c8600427 Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Mon, 11 May 2020 14:22:50 -0700 Subject: [PATCH 15/15] Adapt to Tuple-In-OptionalSome Patterns in Space Projection The space engine goes out of its way to rewrite OptionalSome patterns using the postfix-? sugar into .some(...). Unfortunately, it performed the following remapping: (x, y, z, ...)? -> .some(x, y, z, ...) This syntactic form used to behave correctly. However, we are no longer flattening nested tuples so the correct rewrite is: (x, y, z, ...)? -> .some((x, y, z)) Correct this space projection rule. rdar://62200966 --- lib/Sema/TypeCheckSwitchStmt.cpp | 8 +------- test/Sema/exhaustive_switch.swift | 12 ++++++++++++ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/Sema/TypeCheckSwitchStmt.cpp b/lib/Sema/TypeCheckSwitchStmt.cpp index 7e24182b48257..dd5e04c6a590d 100644 --- a/lib/Sema/TypeCheckSwitchStmt.cpp +++ b/lib/Sema/TypeCheckSwitchStmt.cpp @@ -212,12 +212,6 @@ namespace { } return Space(T, H, SP); } - static Space forConstructor(Type T, DeclName H, - std::forward_list SP) { - // No need to filter SP here; this is only used to copy other - // Constructor spaces. - return Space(T, H, SP); - } static Space forBool(bool C) { return Space(C); } @@ -1441,7 +1435,7 @@ namespace { if (subSpace.getKind() == SpaceKind::Constructor && subSpace.getHead().getBaseIdentifier().empty()) { return Space::forConstructor(item->getType(), name, - std::move(subSpace.getSpaces())); + {subSpace}); } return Space::forConstructor(item->getType(), name, subSpace); } diff --git a/test/Sema/exhaustive_switch.swift b/test/Sema/exhaustive_switch.swift index c2ed81f5eee95..a3933f3cfe4f6 100644 --- a/test/Sema/exhaustive_switch.swift +++ b/test/Sema/exhaustive_switch.swift @@ -1438,3 +1438,15 @@ enum SR11212Tests { } } // end SR11212Tests + +func sr12412() { + enum E { + case x + case y + } + switch (E.x, true) as Optional<(e: E, b: Bool)> { + case nil, (e: .x, b: _)?: break + case (e: .y, b: false)?: break + case (e: .y, b: true)?: break + } +}