From 15583ac269340952659d738b2ce9bc4a10676fe5 Mon Sep 17 00:00:00 2001 From: Meghana Gupta Date: Mon, 1 Jun 2020 23:29:13 -0700 Subject: [PATCH 01/33] Make some PassManager options to accept comma separated values --- lib/SILOptimizer/PassManager/PassManager.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/SILOptimizer/PassManager/PassManager.cpp b/lib/SILOptimizer/PassManager/PassManager.cpp index 369ad70e03d88..97e59d76fe648 100644 --- a/lib/SILOptimizer/PassManager/PassManager.cpp +++ b/lib/SILOptimizer/PassManager/PassManager.cpp @@ -69,42 +69,42 @@ llvm::cl::opt "whose name contains this substring")); llvm::cl::list - SILPrintBefore("sil-print-before", + SILPrintBefore("sil-print-before", llvm::cl::CommaSeparated, llvm::cl::desc("Print out the sil before passes which " "contain a string from this list.")); llvm::cl::list - SILPrintAfter("sil-print-after", + SILPrintAfter("sil-print-after", llvm::cl::CommaSeparated, llvm::cl::desc("Print out the sil after passes which contain " "a string from this list.")); llvm::cl::list - SILPrintAround("sil-print-around", + SILPrintAround("sil-print-around", llvm::cl::CommaSeparated, llvm::cl::desc("Print out the sil before and after passes " "which contain a string from this list")); llvm::cl::list - SILDisablePass("sil-disable-pass", + SILDisablePass("sil-disable-pass", llvm::cl::CommaSeparated, llvm::cl::desc("Disable passes " "which contain a string from this list")); llvm::cl::list SILVerifyBeforePass( - "sil-verify-before-pass", + "sil-verify-before-pass", llvm::cl::CommaSeparated, llvm::cl::desc("Verify the module/analyses before we run " "a pass from this list")); llvm::cl::list SILVerifyAroundPass( - "sil-verify-around-pass", + "sil-verify-around-pass", llvm::cl::CommaSeparated, llvm::cl::desc("Verify the module/analyses before/after we run " "a pass from this list")); llvm::cl::list - SILVerifyAfterPass("sil-verify-after-pass", + SILVerifyAfterPass("sil-verify-after-pass", llvm::cl::CommaSeparated, llvm::cl::desc("Verify the module/analyses after we run " "a pass from this list")); llvm::cl::list SILForceVerifyAroundPass( - "sil-verify-force-analysis-around-pass", + "sil-verify-force-analysis-around-pass", llvm::cl::CommaSeparated, llvm::cl::desc("For the given passes, precompute analyses before the pass " "and verify analyses after the pass")); From 5f162eb1fab0c03210a78973c36e6a6a6a902a48 Mon Sep 17 00:00:00 2001 From: David Zarzycki Date: Fri, 5 Jun 2020 07:09:03 -0400 Subject: [PATCH 02/33] [SIL] NFC: Shrink SILGlobalVariable by 16 bytes --- include/swift/SIL/SILGlobalVariable.h | 20 ++++++++++---------- include/swift/SIL/SILLocation.h | 2 ++ lib/SIL/IR/SILGlobalVariable.cpp | 3 ++- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/include/swift/SIL/SILGlobalVariable.h b/include/swift/SIL/SILGlobalVariable.h index c40ba2c5766a1..e6a08be1305f0 100644 --- a/include/swift/SIL/SILGlobalVariable.h +++ b/include/swift/SIL/SILGlobalVariable.h @@ -53,7 +53,7 @@ class SILGlobalVariable /// The SIL location of the variable, which provides a link back to the AST. /// The variable only gets a location after it's been emitted. - Optional Location; + const SILLocation Location; /// The linkage of the global variable. unsigned Linkage : NumSILLinkageBits; @@ -67,13 +67,16 @@ class SILGlobalVariable /// once (either in its declaration, or once later), making it immutable. unsigned IsLet : 1; + /// Whether or not this is a declaration. + unsigned IsDeclaration : 1; + + /// Whether or not there is a valid SILLocation. + unsigned HasLocation : 1; + /// The VarDecl associated with this SILGlobalVariable. Must by nonnull for /// language-level global variables. VarDecl *VDecl; - /// Whether or not this is a declaration. - bool IsDeclaration; - /// If this block is not empty, the global variable has a static initializer. /// /// The last instruction of this block is the top-level value of the static @@ -132,20 +135,17 @@ class SILGlobalVariable VarDecl *getDecl() const { return VDecl; } - /// Initialize the source location of the function. - void setLocation(SILLocation L) { Location = L; } - /// Check if the function has a location. /// FIXME: All functions should have locations, so this method should not be /// necessary. bool hasLocation() const { - return Location.hasValue(); + return HasLocation; } /// Get the source location of the function. SILLocation getLocation() const { - assert(Location.hasValue()); - return Location.getValue(); + assert(HasLocation); + return Location; } /// Returns the value of the static initializer or null if the global has no diff --git a/include/swift/SIL/SILLocation.h b/include/swift/SIL/SILLocation.h index 60bedd6f8ef31..3181eaf745a9d 100644 --- a/include/swift/SIL/SILLocation.h +++ b/include/swift/SIL/SILLocation.h @@ -273,6 +273,8 @@ class SILLocation { assert(isASTNode()); } + static SILLocation invalid() { return SILLocation(); } + /// Check if the location wraps an AST node or a valid SIL file /// location. /// diff --git a/lib/SIL/IR/SILGlobalVariable.cpp b/lib/SIL/IR/SILGlobalVariable.cpp index a7473bbd287e6..8f40e120dc696 100644 --- a/lib/SIL/IR/SILGlobalVariable.cpp +++ b/lib/SIL/IR/SILGlobalVariable.cpp @@ -48,8 +48,9 @@ SILGlobalVariable::SILGlobalVariable(SILModule &Module, SILLinkage Linkage, : Module(Module), Name(Name), LoweredType(LoweredType), - Location(Loc), + Location(Loc.getValueOr(SILLocation::invalid())), Linkage(unsigned(Linkage)), + HasLocation(Loc.hasValue()), VDecl(Decl) { setSerialized(isSerialized); IsDeclaration = isAvailableExternally(Linkage); From 67e88f4f26c20c2d7e4f339dbfe2a5891743d3e4 Mon Sep 17 00:00:00 2001 From: Rintaro Ishizaki Date: Tue, 9 Jun 2020 12:21:56 -0700 Subject: [PATCH 03/33] [SourceKit] Disable labeled trailing closure support except code completion Don't insert CodeCompletionExpr at the cursor position in "conforming method list" or "typecontext" mode. This increase the chance of successful type checking. rdar://problem/63781922 --- include/swift/Parse/CodeCompletionCallbacks.h | 4 +++ lib/IDE/CodeCompletion.cpp | 4 +++ lib/Parse/ParseExpr.cpp | 7 +++++ .../IDE/conforming-methods-afterclosure.swift | 27 +++++++++++++++++++ 4 files changed, 42 insertions(+) create mode 100644 test/IDE/conforming-methods-afterclosure.swift diff --git a/include/swift/Parse/CodeCompletionCallbacks.h b/include/swift/Parse/CodeCompletionCallbacks.h index b8c5fe151fe83..eeee748a4b904 100644 --- a/include/swift/Parse/CodeCompletionCallbacks.h +++ b/include/swift/Parse/CodeCompletionCallbacks.h @@ -198,6 +198,10 @@ class CodeCompletionCallbacks { virtual void completeCallArg(CodeCompletionExpr *E, bool isFirst) {}; + virtual bool canPerformCompleteLabeledTrailingClosure() const { + return false; + } + virtual void completeLabeledTrailingClosure(CodeCompletionExpr *E, bool isAtStartOfLine) {}; diff --git a/lib/IDE/CodeCompletion.cpp b/lib/IDE/CodeCompletion.cpp index f05c3c8ffe83a..99a42527e03ba 100644 --- a/lib/IDE/CodeCompletion.cpp +++ b/lib/IDE/CodeCompletion.cpp @@ -1687,6 +1687,10 @@ class CodeCompletionCallbacksImpl : public CodeCompletionCallbacks { void completeLabeledTrailingClosure(CodeCompletionExpr *E, bool isAtStartOfLine) override; + bool canPerformCompleteLabeledTrailingClosure() const override { + return true; + } + void completeReturnStmt(CodeCompletionExpr *E) override; void completeYieldStmt(CodeCompletionExpr *E, Optional yieldIndex) override; diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp index 3922c9e2d1569..d35321bbced2e 100644 --- a/lib/Parse/ParseExpr.cpp +++ b/lib/Parse/ParseExpr.cpp @@ -3213,6 +3213,13 @@ Parser::parseTrailingClosures(bool isExprBasic, SourceRange calleeRange, if (!Tok.is(tok::code_complete)) break; + // If the current completion mode doesn't support trailing closure + // completion, leave the token here and let "postfix completion" to + // handle it. + if (CodeCompletion && + !CodeCompletion->canPerformCompleteLabeledTrailingClosure()) + break; + // foo() {} auto CCExpr = new (Context) CodeCompletionExpr(Tok.getLoc()); if (CodeCompletion) diff --git a/test/IDE/conforming-methods-afterclosure.swift b/test/IDE/conforming-methods-afterclosure.swift new file mode 100644 index 0000000000000..faa168b12761d --- /dev/null +++ b/test/IDE/conforming-methods-afterclosure.swift @@ -0,0 +1,27 @@ +// RUN: %target-swift-ide-test -conforming-methods -source-filename %s -code-completion-token=AFTER_TRAILINGCLOSURE -module-name MyModule -conforming-methods-expected-types 's:8MyModule7TargetPP' | %FileCheck %s -check-prefix=AFTER_TRAILINGCLOSURE + +public protocol TargetP {} +struct ConcreteP: TargetP {} + +public struct MyStruct { + init(arg1: Int = 0, fn: () -> Int) {} + + public func returnSomeP -> some TargetP { ConcreteP() } + public func returnConcreteP -> ConcreteP { ConcreteP() } + public func reutrnInt -> Int { 1 } +} + +func test() { + MyStruct { + 1 + } #^AFTER_TRAILINGCLOSURE^# +} + +//AFTER_TRAILINGCLOSURE: -----BEGIN CONFORMING METHOD LIST----- +//AFTER_TRAILINGCLOSURE-NEXT: - TypeName: MyStruct +//AFTER_TRAILINGCLOSURE-NEXT: - Members: +//AFTER_TRAILINGCLOSURE-NEXT: - Name: returnSomeP() +//AFTER_TRAILINGCLOSURE-NEXT: TypeName: some TargetP +//AFTER_TRAILINGCLOSURE-NEXT: - Name: returnConcreteP() +//AFTER_TRAILINGCLOSURE-NEXT: TypeName: ConcreteP +//AFTER_TRAILINGCLOSURE-NEXT: -----END CONFORMING METHOD LIST----- From f60d17bf3d18c28ac28c8a4804f500c28259f16e Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Wed, 10 Jun 2020 16:58:03 -0700 Subject: [PATCH 04/33] [Function builders] Use one-way constraints for closure parameters. Introduce one-way constraints for the parameters of closures to which a function builder is being applied. This was an intended part of the model when one-way constraints were introduced, but somehow got missed. This should further break up large constraints systems for faster solving, and *most likely* won't break much source code in practice. Fixes rdar://problem/64231116. --- lib/Sema/CSSimplify.cpp | 38 ++++++++++++------- test/Constraints/function_builder_diags.swift | 11 ++++++ .../function_builder_one_way.swift | 21 +++++----- 3 files changed, 46 insertions(+), 24 deletions(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 37e2639ccf9a3..cadfea40c9518 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -7222,6 +7222,24 @@ bool ConstraintSystem::resolveClosure(TypeVariableType *typeVar, auto *closure = castToExpr(closureLocator->getAnchor()); auto *inferredClosureType = getClosureType(closure); + // Determine whether a function builder will be applied. + Type functionBuilderType; + ConstraintLocator *calleeLocator = nullptr; + if (auto last = locator.last()) { + if (auto argToParam = last->getAs()) { + calleeLocator = getCalleeLocator(getConstraintLocator(locator)); + functionBuilderType = getFunctionBuilderTypeFor( + *this, argToParam->getParamIdx(), calleeLocator); + } + } + + // Determine whether to introduce one-way constraints between the parameter's + // type as seen in the body of the closure and the external parameter + // type. + bool oneWayConstraints = + getASTContext().TypeCheckerOpts.EnableOneWayClosureParameters || + functionBuilderType; + auto *paramList = closure->getParameters(); SmallVector parameters; for (unsigned i = 0, n = paramList->size(); i != n; ++i) { @@ -7235,9 +7253,6 @@ bool ConstraintSystem::resolveClosure(TypeVariableType *typeVar, } Type internalType; - - bool oneWayConstraints = - getASTContext().TypeCheckerOpts.EnableOneWayClosureParameters; if (paramList->get(i)->getTypeRepr()) { // Internal type is the type used in the body of the closure, // so "external" type translates to it as follows: @@ -7288,17 +7303,12 @@ bool ConstraintSystem::resolveClosure(TypeVariableType *typeVar, inferredClosureType->getExtInfo()); assignFixedType(typeVar, closureType, closureLocator); - if (auto last = locator.last()) { - if (auto argToParam = last->getAs()) { - auto *calleeLocator = getCalleeLocator(getConstraintLocator(locator)); - if (auto functionBuilderType = getFunctionBuilderTypeFor( - *this, argToParam->getParamIdx(), calleeLocator)) { - if (auto result = matchFunctionBuilder( - closure, functionBuilderType, closureType->getResult(), - ConstraintKind::Conversion, calleeLocator, locator)) { - return result->isSuccess(); - } - } + // If there is a function builder to apply, do so now. + if (functionBuilderType) { + if (auto result = matchFunctionBuilder( + closure, functionBuilderType, closureType->getResult(), + ConstraintKind::Conversion, calleeLocator, locator)) { + return result->isSuccess(); } } diff --git a/test/Constraints/function_builder_diags.swift b/test/Constraints/function_builder_diags.swift index 389ddc5ba6bc1..1a04e05892926 100644 --- a/test/Constraints/function_builder_diags.swift +++ b/test/Constraints/function_builder_diags.swift @@ -558,3 +558,14 @@ func rdar61347993() { func test_closure(_: () -> Result) {} test_closure {} // expected-error {{cannot convert value of type '()' to closure result type 'Result'}} } + +// One-way constraints through parameters. +func wrapperifyInfer(_ cond: Bool, @WrapperBuilder body: (U) -> T) -> T { + fatalError("boom") +} + +let intValue = 17 +wrapperifyInfer(true) { x in // expected-error{{unable to infer type of a closure parameter 'x' in the current context}} + intValue + x +} + diff --git a/test/Constraints/function_builder_one_way.swift b/test/Constraints/function_builder_one_way.swift index d393025c4b01c..e3519800497f1 100644 --- a/test/Constraints/function_builder_one_way.swift +++ b/test/Constraints/function_builder_one_way.swift @@ -49,16 +49,17 @@ func tuplify(_ collection: C, @TupleBuilder body: (C.Element) } // CHECK: ---Connected components--- -// CHECK-NEXT: 0: $T1 $T2 $T3 $T5 $T6 $T7 $T8 $T10 $T77 $T78 depends on 2 -// CHECK-NEXT: 2: $T12 $T17 $T28 $T42 $T53 $T54 $T55 $T56 $T57 $T58 $T59 $T60 $T61 $T62 $T63 $T64 $T65 $T66 $T68 $T69 $T70 $T71 $T72 $T73 $T74 $T75 $T76 depends on 1, 3, 4, 6, 9 -// CHECK-NEXT: 9: $T48 $T49 $T50 $T51 $T52 depends on 8 -// CHECK-NEXT: 8: $T43 $T44 $T45 $T46 $T47 -// CHECK-NEXT: 6: $T31 $T35 $T36 $T37 $T38 $T39 $T40 $T41 depends on 5, 7 -// CHECK-NEXT: 7: $T32 $T33 $T34 -// CHECK-NEXT: 5: $T30 -// CHECK-NEXT: 4: $T18 $T19 $T20 $T21 $T22 $T23 $T24 $T25 $T26 $T27 -// CHECK-NEXT: 3: $T15 $T16 -// CHECK-NEXT: 1: $T11 +// CHECK-NEXT: 1: $T10 depends on 0 +// CHECK-NEXT: 0: $T1 $T2 $T3 $T5 $T6 $T7 $T8 $T77 $T78 depends on 3 +// CHECK-NEXT: 3: $T12 $T17 $T28 $T42 $T53 $T54 $T55 $T56 $T57 $T58 $T59 $T60 $T61 $T62 $T63 $T64 $T65 $T66 $T68 $T69 $T70 $T71 $T72 $T73 $T74 $T75 $T76 depends on 2, 4, 5, 7, 10 +// CHECK-NEXT: 10: $T48 $T49 $T50 $T51 $T52 depends on 9 +// CHECK-NEXT: 9: $T43 $T44 $T45 $T46 $T47 +// CHECK-NEXT: 7: $T31 $T35 $T36 $T37 $T38 $T39 $T40 $T41 depends on 6, 8 +// CHECK-NEXT: 8: $T32 $T33 $T34 +// CHECK-NEXT: 6: $T30 +// CHECK-NEXT: 5: $T18 $T19 $T20 $T21 $T22 $T23 $T24 $T25 $T26 $T27 +// CHECK-NEXT: 4: $T15 $T16 +// CHECK-NEXT: 2: $T11 let names = ["Alice", "Bob", "Charlie"] let b = true var number = 17 From f46e2c271d9f51fc230f226c56c21a96f9dcf164 Mon Sep 17 00:00:00 2001 From: David Zarzycki Date: Thu, 11 Jun 2020 07:40:44 -0400 Subject: [PATCH 05/33] [testing] Workaround unsorted output --- .../Dependencies/driver-show-incremental-mutual-fine.swift | 6 +++--- .../driver-show-incremental-mutual-fine.swift | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/Driver/Dependencies/driver-show-incremental-mutual-fine.swift b/test/Driver/Dependencies/driver-show-incremental-mutual-fine.swift index 3455fb2e484f4..29aeea5f37e74 100644 --- a/test/Driver/Dependencies/driver-show-incremental-mutual-fine.swift +++ b/test/Driver/Dependencies/driver-show-incremental-mutual-fine.swift @@ -5,9 +5,9 @@ // RUN: touch -t 201401240005 %t/* // RUN: cd %t && %swiftc_driver -c -driver-use-frontend-path "%{python};%S/Inputs/update-dependencies.py;%swift-dependency-tool" -output-file-map %t/output.json -incremental -disable-direct-intramodule-dependencies -driver-always-rebuild-dependents ./main.swift ./other.swift -module-name main -j1 -v -driver-show-incremental -disable-direct-intramodule-dependencies 2>&1 | %FileCheck -check-prefix=CHECK-FIRST %s -// CHECK-FIRST: Handled main.swift -// CHECK-FIRST: Handled other.swift -// CHECK-FIRST: Disabling incremental build: could not read build record +// CHECK-FIRST-DAG: Handled main.swift +// CHECK-FIRST-DAG: Handled other.swift +// CHECK-FIRST-DAG: Disabling incremental build: could not read build record // RUN: cd %t && %swiftc_driver -c -driver-use-frontend-path "%{python};%S/Inputs/update-dependencies.py;%swift-dependency-tool" -output-file-map %t/output.json -incremental -disable-direct-intramodule-dependencies -driver-always-rebuild-dependents ./main.swift ./other.swift -module-name main -j1 -v -driver-show-incremental -disable-direct-intramodule-dependencies 2>&1 | %FileCheck -check-prefix=CHECK-SECOND %s // CHECK-SECOND-NOT: Queuing diff --git a/test/Driver/PrivateDependencies/driver-show-incremental-mutual-fine.swift b/test/Driver/PrivateDependencies/driver-show-incremental-mutual-fine.swift index 5449fb09c7746..000a5d3a1ef7f 100644 --- a/test/Driver/PrivateDependencies/driver-show-incremental-mutual-fine.swift +++ b/test/Driver/PrivateDependencies/driver-show-incremental-mutual-fine.swift @@ -5,9 +5,9 @@ // RUN: touch -t 201401240005 %t/* // RUN: cd %t && %swiftc_driver -c -driver-use-frontend-path "%{python};%S/Inputs/update-dependencies.py;%swift-dependency-tool" -output-file-map %t/output.json -incremental -enable-direct-intramodule-dependencies -driver-always-rebuild-dependents ./main.swift ./other.swift -module-name main -j1 -v -driver-show-incremental 2>&1 | %FileCheck -check-prefix=CHECK-FIRST %s -// CHECK-FIRST: Handled main.swift -// CHECK-FIRST: Handled other.swift -// CHECK-FIRST: Disabling incremental build: could not read build record +// CHECK-FIRST-DAG: Handled main.swift +// CHECK-FIRST-DAG: Handled other.swift +// CHECK-FIRST-DAG: Disabling incremental build: could not read build record // RUN: cd %t && %swiftc_driver -c -driver-use-frontend-path "%{python};%S/Inputs/update-dependencies.py;%swift-dependency-tool" -output-file-map %t/output.json -incremental -enable-direct-intramodule-dependencies -driver-always-rebuild-dependents ./main.swift ./other.swift -module-name main -j1 -v -driver-show-incremental 2>&1 | %FileCheck -check-prefix=CHECK-SECOND %s // CHECK-SECOND-NOT: Queuing From acf923b9fe02ebd1dd25bf95136a15737885de93 Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Thu, 11 Jun 2020 18:56:44 +0200 Subject: [PATCH 06/33] MandatoryInlining: fix a memory lifetime bug related to partial_apply with in_guaranteed parameters. Because partial_apply consumes it's arguments we need to copy them. This was done for "direct" parameters but not for address parameters. rdar://problem/64035105 --- .../Mandatory/MandatoryInlining.cpp | 53 ++++++++++++++----- test/SILOptimizer/mandatory_inlining.sil | 35 ++++++++++++ 2 files changed, 75 insertions(+), 13 deletions(-) diff --git a/lib/SILOptimizer/Mandatory/MandatoryInlining.cpp b/lib/SILOptimizer/Mandatory/MandatoryInlining.cpp index ce7c2dcf951ed..6af5002074429 100644 --- a/lib/SILOptimizer/Mandatory/MandatoryInlining.cpp +++ b/lib/SILOptimizer/Mandatory/MandatoryInlining.cpp @@ -59,7 +59,10 @@ static SILValue stripCopiesAndBorrows(SILValue v) { /// It is important to note that, we can not assume that the partial apply, the /// apply site, or the callee value are control dependent in any way. This /// requires us to need to be very careful. See inline comments. -static void fixupReferenceCounts( +/// +/// Returns true if the stack nesting is invalidated and must be corrected +/// afterwards. +static bool fixupReferenceCounts( PartialApplyInst *pai, FullApplySite applySite, SILValue calleeValue, ArrayRef captureArgConventions, MutableArrayRef capturedArgs, bool isCalleeGuaranteed) { @@ -72,6 +75,7 @@ static void fixupReferenceCounts( // FIXME: Can we cache this in between inlining invocations? DeadEndBlocks deadEndBlocks(pai->getFunction()); SmallVector leakingBlocks; + bool invalidatedStackNesting = false; // Add a copy of each non-address type capture argument to lifetime extend the // captured argument over at least the inlined function and till the end of a @@ -83,14 +87,6 @@ static void fixupReferenceCounts( for (unsigned i : indices(captureArgConventions)) { auto convention = captureArgConventions[i]; SILValue &v = capturedArgs[i]; - if (v->getType().isAddress()) { - // FIXME: What about indirectly owned parameters? The invocation of the - // closure would perform an indirect copy which we should mimick here. - assert(convention != ParameterConvention::Indirect_In && - "Missing indirect copy"); - continue; - } - auto *f = applySite.getFunction(); // See if we have a trivial value. In such a case, just continue. We do not @@ -102,11 +98,40 @@ static void fixupReferenceCounts( switch (convention) { case ParameterConvention::Indirect_In: + llvm_unreachable("Missing indirect copy"); + case ParameterConvention::Indirect_In_Constant: case ParameterConvention::Indirect_Inout: case ParameterConvention::Indirect_InoutAliasable: - case ParameterConvention::Indirect_In_Guaranteed: - llvm_unreachable("Should be handled above"); + break; + + case ParameterConvention::Indirect_In_Guaranteed: { + // Do the same as for Direct_Guaranteed, just the address version. + // (See comment below). + SILBuilderWithScope builder(pai); + auto *stackLoc = builder.createAllocStack(loc, v->getType().getObjectType()); + builder.createCopyAddr(loc, v, stackLoc, IsNotTake, IsInitialization); + visitedBlocks.clear(); + + LinearLifetimeChecker checker(visitedBlocks, deadEndBlocks); + bool consumedInLoop = checker.completeConsumingUseSet( + pai, applySite.getCalleeOperand(), + [&](SILBasicBlock::iterator insertPt) { + SILBuilderWithScope builder(insertPt); + builder.createDestroyAddr(loc, stackLoc); + builder.createDeallocStack(loc, stackLoc); + }); + + if (!consumedInLoop) { + applySite.insertAfterInvocation([&](SILBasicBlock::iterator iter) { + SILBuilderWithScope(iter).createDestroyAddr(loc, stackLoc); + SILBuilderWithScope(iter).createDeallocStack(loc, stackLoc); + }); + } + v = stackLoc; + invalidatedStackNesting = true; + break; + } case ParameterConvention::Direct_Guaranteed: { // If we have a direct_guaranteed value, the value is being taken by the @@ -242,6 +267,7 @@ static void fixupReferenceCounts( SILBuilderWithScope(iter).emitDestroyValueOperation(loc, calleeValue); }); } + return invalidatedStackNesting; } static SILValue cleanupLoadedCalleeValue(SILValue calleeValue, LoadInst *li) { @@ -917,8 +943,9 @@ runOnFunctionRecursively(SILOptFunctionBuilder &FuncBuilder, // We need to insert the copies before the partial_apply since if we can // not remove the partial_apply the captured values will be dead by the // time we hit the call site. - fixupReferenceCounts(PAI, InnerAI, CalleeValue, CapturedArgConventions, - CapturedArgs, IsCalleeGuaranteed); + invalidatedStackNesting |= fixupReferenceCounts(PAI, InnerAI, + CalleeValue, CapturedArgConventions, + CapturedArgs, IsCalleeGuaranteed); } // Register a callback to record potentially unused function values after diff --git a/test/SILOptimizer/mandatory_inlining.sil b/test/SILOptimizer/mandatory_inlining.sil index 911343961cc62..ad225f0ae1683 100644 --- a/test/SILOptimizer/mandatory_inlining.sil +++ b/test/SILOptimizer/mandatory_inlining.sil @@ -1428,3 +1428,38 @@ bb0(%0 : $@callee_guaranteed () -> ()): %9999 = tuple() return %9999 : $() } + +struct Mystruct { + var s: String +} + +sil @use_string : $@convention(thin) (@guaranteed String) -> () + +sil private [transparent] [ossa] @callee_with_guaranteed : $@convention(thin) (Bool, @in_guaranteed Mystruct) -> () { +bb0(%0 : $Bool, %1 : $*Mystruct): + %4 = struct_element_addr %1 : $*Mystruct, #Mystruct.s + %5 = load [copy] %4 : $*String + %6 = function_ref @use_string : $@convention(thin) (@guaranteed String) -> () + %7 = apply %6(%5) : $@convention(thin) (@guaranteed String) -> () + destroy_value %5 : $String + %19 = tuple () + return %19 : $() +} + +// Make sure that this doesn't cause a memory lifetime failure. +// +// CHECK-LABEL: sil [ossa] @partial_apply_with_indirect_param +// CHECK: } // end sil function 'partial_apply_with_indirect_param' +sil [ossa] @partial_apply_with_indirect_param : $@convention(thin) (@in_guaranteed Mystruct, Bool) -> () { +bb0(%0 : $*Mystruct, %1 : $Bool): + %216 = function_ref @callee_with_guaranteed : $@convention(thin) (Bool, @in_guaranteed Mystruct) -> () + %217 = alloc_stack $Mystruct + copy_addr %0 to [initialization] %217 : $*Mystruct + %219 = partial_apply [callee_guaranteed] %216(%217) : $@convention(thin) (Bool, @in_guaranteed Mystruct) -> () + %220 = apply %219(%1) : $@callee_guaranteed (Bool) -> () + destroy_value %219 : $@callee_guaranteed (Bool) -> () + dealloc_stack %217 : $*Mystruct + %19 = tuple () + return %19 : $() +} + From d54bbc664a8086ae8b8a105607917edd8a8facb8 Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 10:54:37 -0700 Subject: [PATCH 07/33] [Gardening] Remove TypeChecker::validateType Entrypoint --- lib/Sema/TypeChecker.h | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index c3bd2d9384d70..c971d3bc34aa8 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -358,20 +358,6 @@ Type getUInt8Type(ASTContext &ctx); /// for the lookup. Expr *resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *Context); -/// Validate the given type. -/// -/// Type validation performs name lookup, checking of generic arguments, -/// and so on to determine whether the given type is well-formed and can -/// be used as a type. -/// -/// \param Loc The type (with source location information) to validate. -/// If the type has already been validated, returns immediately. -/// -/// \param resolution The type resolution being performed. -/// -/// \returns true if type validation failed, or false otherwise. -bool validateType(TypeLoc &Loc, TypeResolution resolution); - /// Check for unsupported protocol types in the given declaration. void checkUnsupportedProtocolType(Decl *decl); From 4c0c7daf3f8fac23bc0a48027203680d703de544 Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 10:58:17 -0700 Subject: [PATCH 08/33] [NFC] Fixup TypeChecker::getArraySliceType To Not Return Type() --- lib/Sema/CSGen.cpp | 2 +- lib/Sema/TypeCheckDecl.cpp | 2 +- lib/Sema/TypeCheckType.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 15b1d5ee7a85a..376baa96168a0 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -2938,7 +2938,7 @@ namespace { // Try to build the appropriate type for a variadic argument list of // the fresh element type. If that failed, just bail out. auto array = TypeChecker::getArraySliceType(expr->getLoc(), element); - if (!array) return element; + if (array->hasError()) return element; // Require the operand to be convertible to the array type. CS.addConstraint(ConstraintKind::Conversion, diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index 6a5f8c7ac3b56..333583258b1b6 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -2101,7 +2101,7 @@ static Type validateParameterType(ParamDecl *decl) { if (decl->isVariadic()) { Ty = TypeChecker::getArraySliceType(decl->getStartLoc(), Ty); - if (Ty.isNull()) { + if (Ty->hasError()) { decl->setInvalid(); return ErrorType::get(ctx); } diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index 3f368011f8aa1..7316d0d9adbbf 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -364,7 +364,7 @@ Type TypeChecker::getArraySliceType(SourceLoc loc, Type elementType) { ASTContext &ctx = elementType->getASTContext(); if (!ctx.getArrayDecl()) { ctx.Diags.diagnose(loc, diag::sugar_type_not_found, 0); - return Type(); + return ErrorType::get(ctx); } return ArraySliceType::get(elementType); From bd6724c6d83196b7105adcb82b76edee9eefc69f Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 11:17:21 -0700 Subject: [PATCH 09/33] [NFC] Collapse TypeChecker::getDictionaryType This method's only caller was using this to form a substituted DictionaryType, but also using TypeChecker::applyUnboundGenericArguments. We need to use the latter unconditionally so we actually check the requirements on DictionaryType and emit diagnostics. Also clean up resolveDictionaryType so it consistently returns ErrorTypes. --- lib/Sema/TypeCheckType.cpp | 51 ++++++++++++++++---------------------- lib/Sema/TypeChecker.h | 1 - 2 files changed, 22 insertions(+), 30 deletions(-) diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index 7316d0d9adbbf..09c6dd091721d 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -370,17 +370,6 @@ Type TypeChecker::getArraySliceType(SourceLoc loc, Type elementType) { return ArraySliceType::get(elementType); } -Type TypeChecker::getDictionaryType(SourceLoc loc, Type keyType, - Type valueType) { - ASTContext &ctx = keyType->getASTContext(); - if (!ctx.getDictionaryDecl()) { - ctx.Diags.diagnose(loc, diag::sugar_type_not_found, 3); - return Type(); - } - - return DictionaryType::get(keyType, valueType); -} - Type TypeChecker::getOptionalType(SourceLoc loc, Type elementType) { ASTContext &ctx = elementType->getASTContext(); if (!ctx.getOptionalDecl()) { @@ -3305,11 +3294,13 @@ Type TypeResolver::resolveSpecifierTypeRepr(SpecifierTypeRepr *repr, Type TypeResolver::resolveArrayType(ArrayTypeRepr *repr, TypeResolutionOptions options) { Type baseTy = resolveType(repr->getBase(), options.withoutContext()); - if (!baseTy || baseTy->hasError()) return baseTy; + if (!baseTy || baseTy->hasError()) { + return ErrorType::get(Context); + } auto sliceTy = TypeChecker::getArraySliceType(repr->getBrackets().Start, baseTy); - if (!sliceTy) + if (sliceTy->hasError()) return ErrorType::get(Context); return sliceTy; @@ -3320,28 +3311,30 @@ Type TypeResolver::resolveDictionaryType(DictionaryTypeRepr *repr, options = adjustOptionsForGenericArgs(options); Type keyTy = resolveType(repr->getKey(), options.withoutContext()); - if (!keyTy || keyTy->hasError()) return keyTy; + if (!keyTy || keyTy->hasError()) { + return ErrorType::get(Context); + } Type valueTy = resolveType(repr->getValue(), options.withoutContext()); - if (!valueTy || valueTy->hasError()) return valueTy; + if (!valueTy || valueTy->hasError()) { + return ErrorType::get(Context); + } auto dictDecl = Context.getDictionaryDecl(); - - if (auto dictTy = TypeChecker::getDictionaryType(repr->getBrackets().Start, - keyTy, valueTy)) { - auto unboundTy = dictDecl->getDeclaredType()->castTo(); - - Type args[] = {keyTy, valueTy}; - - if (!TypeChecker::applyUnboundGenericArguments( - unboundTy, dictDecl, repr->getStartLoc(), resolution, args)) { - return nullptr; - } - - return dictTy; + if (!dictDecl) { + Context.Diags.diagnose(repr->getBrackets().Start, + diag::sugar_type_not_found, 3); + return ErrorType::get(Context); } - return ErrorType::get(Context); + auto unboundTy = dictDecl->getDeclaredType()->castTo(); + Type args[] = {keyTy, valueTy}; + if (!TypeChecker::applyUnboundGenericArguments( + unboundTy, dictDecl, repr->getStartLoc(), resolution, args)) { + assert(Context.Diags.hadAnyError()); + return ErrorType::get(Context); + } + return DictionaryType::get(keyTy, valueTy); } Type TypeResolver::resolveOptionalType(OptionalTypeRepr *repr, diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index c971d3bc34aa8..2cb71edf98274 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -345,7 +345,6 @@ enum class CheckedCastContextKind { namespace TypeChecker { Type getArraySliceType(SourceLoc loc, Type elementType); -Type getDictionaryType(SourceLoc loc, Type keyType, Type valueType); Type getOptionalType(SourceLoc loc, Type elementType); Type getStringType(ASTContext &ctx); Type getSubstringType(ASTContext &ctx); From d81f1482c38ce2010ebaef9d9d1dc3085ccbdb56 Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 11:24:44 -0700 Subject: [PATCH 10/33] [NFC] Fixup TypeChecker::getOptionalType To Not Return Type() --- lib/Sema/CSGen.cpp | 3 ++- lib/Sema/CSSimplify.cpp | 1 + lib/Sema/TypeCheckType.cpp | 27 ++++++++++++++------------- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 376baa96168a0..77808245beacd 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -2404,6 +2404,7 @@ namespace { } varType = TypeChecker::getOptionalType(var->getLoc(), varType); + assert(!varType->hasError()); if (oneWayVarType) { oneWayVarType = @@ -3304,7 +3305,7 @@ namespace { /// worth QoI efforts. Type getOptionalType(SourceLoc optLoc, Type valueTy) { auto optTy = TypeChecker::getOptionalType(optLoc, valueTy); - if (!optTy || + if (optTy->hasError() || TypeChecker::requireOptionalIntrinsics(CS.getASTContext(), optLoc)) return Type(); diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 37e2639ccf9a3..179b270d9ee7f 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -6889,6 +6889,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint( TVO_CanBindToLValue | TVO_CanBindToNoEscape); Type optTy = TypeChecker::getOptionalType(SourceLoc(), innerTV); + assert(!optTy->hasError()); SmallVector optionalities; auto nonoptionalResult = Constraint::createFixed( *this, ConstraintKind::Bind, diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index 09c6dd091721d..b6112ed488088 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -374,7 +374,7 @@ Type TypeChecker::getOptionalType(SourceLoc loc, Type elementType) { ASTContext &ctx = elementType->getASTContext(); if (!ctx.getOptionalDecl()) { ctx.Diags.diagnose(loc, diag::sugar_type_not_found, 1); - return Type(); + return ErrorType::get(ctx); } return OptionalType::get(elementType); @@ -3342,14 +3342,16 @@ Type TypeResolver::resolveOptionalType(OptionalTypeRepr *repr, TypeResolutionOptions elementOptions = options.withoutContext(true); elementOptions.setContext(TypeResolverContext::ImmediateOptionalTypeArgument); - // The T in T? is a generic type argument and therefore always an AST type. - // FIXME: diagnose non-materializability of element type! Type baseTy = resolveType(repr->getBase(), elementOptions); - if (!baseTy || baseTy->hasError()) return baseTy; + if (!baseTy || baseTy->hasError()) { + return ErrorType::get(Context); + } auto optionalTy = TypeChecker::getOptionalType(repr->getQuestionLoc(), baseTy); - if (!optionalTy) return ErrorType::get(Context); + if (optionalTy->hasError()) { + return ErrorType::get(Context); + } return optionalTy; } @@ -3412,17 +3414,16 @@ Type TypeResolver::resolveImplicitlyUnwrappedOptionalType( TypeResolutionOptions elementOptions = options.withoutContext(true); elementOptions.setContext(TypeResolverContext::ImmediateOptionalTypeArgument); - // The T in T! is a generic type argument and therefore always an AST type. - // FIXME: diagnose non-materializability of element type! Type baseTy = resolveType(repr->getBase(), elementOptions); - if (!baseTy || baseTy->hasError()) return baseTy; - - Type uncheckedOptionalTy; - uncheckedOptionalTy = TypeChecker::getOptionalType(repr->getExclamationLoc(), - baseTy); + if (!baseTy || baseTy->hasError()) { + return ErrorType::get(Context); + } - if (!uncheckedOptionalTy) + Type uncheckedOptionalTy = + TypeChecker::getOptionalType(repr->getExclamationLoc(), baseTy); + if (uncheckedOptionalTy->hasError()) { return ErrorType::get(Context); + } return uncheckedOptionalTy; } From 743230e1693bff536612b96ee9c4851ae4cae7de Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 11:29:10 -0700 Subject: [PATCH 11/33] [NFC] applyGenericArguments Returns ErrorType Consistently --- lib/Sema/TypeCheckType.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index b6112ed488088..093404e7c66a8 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -120,7 +120,7 @@ GenericSignature TypeResolution::getGenericSignature() const { return dc->getGenericSignatureOfContext(); case TypeResolutionStage::Structural: - return nullptr; + return GenericSignature(); } llvm_unreachable("unhandled stage"); } @@ -781,8 +781,9 @@ static Type applyGenericArguments(Type type, TypeResolution resolution, if (nominal->isOptionalDecl()) { // Validate the generic argument. Type objectType = resolution.resolveType(genericArgs[0]); - if (!objectType || objectType->hasError()) - return nullptr; + if (!objectType || objectType->hasError()) { + return ErrorType::get(ctx); + } return BoundGenericType::get(nominal, /*parent*/ Type(), objectType); } @@ -3205,7 +3206,9 @@ Type TypeResolver::resolveIdentifierType(IdentTypeRepr *IdType, ComponentRange.end()); Type result = resolveIdentTypeComponent(resolution.withOptions(options), Components); - if (!result) return nullptr; + if (!result || result->hasError()) { + return ErrorType::get(Context); + } if (auto moduleTy = result->getAs()) { // Allow module types only if flag is specified. From 26ee5d534e0f9cac181583077d3081dfa5a200cf Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 11:30:45 -0700 Subject: [PATCH 12/33] [NFC] Fixup resolveSILFunctionType To Not Return Type() --- lib/Sema/TypeCheckType.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index 093404e7c66a8..73f5a0415bcbb 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -2857,9 +2857,8 @@ Type TypeResolver::resolveSILFunctionType(FunctionTypeRepr *repr, elementOptions.setContext(TypeResolverContext::FunctionInput); auto param = resolveSILParameter(elt.Type, elementOptions); params.push_back(param); - if (!param.getInterfaceType()) return nullptr; - if (param.getInterfaceType()->hasError()) + if (!param.getInterfaceType() || param.getInterfaceType()->hasError()) hasError = true; } From 69dc77f7fe924cb01716b96c4ffbaf33e77c18c5 Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 11:34:22 -0700 Subject: [PATCH 13/33] [NFC] Refactor resolveASTFunctionTypeParams Directly return the parameter vector, and handle missing types the same way we handle error types. --- lib/Sema/TypeCheckType.cpp | 39 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index 73f5a0415bcbb..0c08804d89810 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -1725,12 +1725,9 @@ namespace { = nullptr, DifferentiabilityKind diffKind = DifferentiabilityKind::NonDifferentiable); - bool - resolveASTFunctionTypeParams(TupleTypeRepr *inputRepr, - TypeResolutionOptions options, - bool requiresMappingOut, - DifferentiabilityKind diffKind, - SmallVectorImpl &ps); + SmallVector resolveASTFunctionTypeParams( + TupleTypeRepr *inputRepr, TypeResolutionOptions options, + bool requiresMappingOut, DifferentiabilityKind diffKind); Type resolveSILFunctionType(FunctionTypeRepr *repr, TypeResolutionOptions options, @@ -2453,10 +2450,12 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs, return ty; } -bool TypeResolver::resolveASTFunctionTypeParams( - TupleTypeRepr *inputRepr, TypeResolutionOptions options, - bool requiresMappingOut, DifferentiabilityKind diffKind, - SmallVectorImpl &elements) { +SmallVector +TypeResolver::resolveASTFunctionTypeParams(TupleTypeRepr *inputRepr, + TypeResolutionOptions options, + bool requiresMappingOut, + DifferentiabilityKind diffKind) { + SmallVector elements; elements.reserve(inputRepr->getNumElements()); auto elementOptions = options.withoutContext(true); @@ -2477,9 +2476,7 @@ bool TypeResolver::resolveASTFunctionTypeParams( } Type ty = resolveType(eltTypeRepr, thisElementOptions); - if (!ty) return true; - - if (ty->hasError()) { + if (!ty || ty->hasError()) { elements.emplace_back(ErrorType::get(Context)); continue; } @@ -2573,7 +2570,7 @@ bool TypeResolver::resolveASTFunctionTypeParams( } } - return false; + return elements; } Type TypeResolver::resolveOpaqueReturnType(TypeRepr *repr, @@ -2626,18 +2623,16 @@ Type TypeResolver::resolveASTFunctionType( TypeResolutionOptions options = None; options |= parentOptions.withoutContext().getFlags(); - - SmallVector params; - if (resolveASTFunctionTypeParams(repr->getArgsTypeRepr(), options, - repr->getGenericEnvironment() != nullptr, - diffKind, params)) { - return Type(); - } + auto params = resolveASTFunctionTypeParams( + repr->getArgsTypeRepr(), options, + repr->getGenericEnvironment() != nullptr, diffKind); auto resultOptions = options.withoutContext(); resultOptions.setContext(TypeResolverContext::FunctionResult); Type outputTy = resolveType(repr->getResultTypeRepr(), resultOptions); - if (!outputTy || outputTy->hasError()) return outputTy; + if (!outputTy || outputTy->hasError()) { + return ErrorType::get(Context); + } // If this is a function type without parens around the parameter list, // diagnose this and produce a fixit to add them. From b6143c62a8265b1ca0e335cf34c8055cfdcbdaaf Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 11:48:27 -0700 Subject: [PATCH 14/33] [NFC] Remove TypeChecker::getIntType Inline it into its only caller --- lib/Sema/CSApply.cpp | 2 +- lib/Sema/TypeCheckType.cpp | 7 ------- lib/Sema/TypeChecker.h | 1 - 3 files changed, 1 insertion(+), 9 deletions(-) diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index b7d81bf2caa5d..92648cb78aa6a 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -2488,7 +2488,7 @@ namespace { // Make the integer literals for the parameters. auto buildExprFromUnsigned = [&](unsigned value) { LiteralExpr *expr = IntegerLiteralExpr::createFromUnsigned(ctx, value); - cs.setType(expr, TypeChecker::getIntType(ctx)); + cs.setType(expr, ctx.getIntDecl()->getDeclaredInterfaceType()); return handleIntegerLiteralExpr(expr); }; diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index 0c08804d89810..5ebe17e0d3094 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -394,13 +394,6 @@ Type TypeChecker::getSubstringType(ASTContext &Context) { return Type(); } -Type TypeChecker::getIntType(ASTContext &Context) { - if (auto typeDecl = Context.getIntDecl()) - return typeDecl->getDeclaredInterfaceType(); - - return Type(); -} - Type TypeChecker::getInt8Type(ASTContext &Context) { if (auto typeDecl = Context.getInt8Decl()) return typeDecl->getDeclaredInterfaceType(); diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index 2cb71edf98274..1fecda02df67c 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -348,7 +348,6 @@ Type getArraySliceType(SourceLoc loc, Type elementType); Type getOptionalType(SourceLoc loc, Type elementType); Type getStringType(ASTContext &ctx); Type getSubstringType(ASTContext &ctx); -Type getIntType(ASTContext &ctx); Type getInt8Type(ASTContext &ctx); Type getUInt8Type(ASTContext &ctx); From f02a0f0439edaaeb2b321406f553bab0da5797be Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 11:49:32 -0700 Subject: [PATCH 15/33] [NFC] Remove TypeChecker::getSubstringType Inline it into its only caller --- lib/Sema/CSDiagnostics.cpp | 5 +---- lib/Sema/TypeCheckType.cpp | 4 ---- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index f83ffed73fc32..648308fb6e746 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -2571,10 +2571,7 @@ bool ContextualFailure::trySequenceSubsequenceFixIts( return false; auto String = TypeChecker::getStringType(getASTContext()); - auto Substring = TypeChecker::getSubstringType(getASTContext()); - - if (!String || !Substring) - return false; + auto Substring = getASTContext().getSubstringDecl()->getDeclaredInterfaceType(); // Substring -> String conversion // Wrap in String.init diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index 5ebe17e0d3094..2c0b8f8a3248d 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -387,10 +387,6 @@ Type TypeChecker::getStringType(ASTContext &Context) { return Type(); } -Type TypeChecker::getSubstringType(ASTContext &Context) { - if (auto typeDecl = Context.getSubstringDecl()) - return typeDecl->getDeclaredInterfaceType(); - return Type(); } From 09395ba345bbe06c7d0a4f4a13f905d4c1dcc1bf Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 11:52:17 -0700 Subject: [PATCH 16/33] [NFC] Inline TypeChecker::getInt8Type and TypeChecker::getUInt8Type --- lib/Sema/CSSimplify.cpp | 8 ++++---- lib/Sema/TypeCheckType.cpp | 16 +--------------- lib/Sema/TypeChecker.h | 2 -- 3 files changed, 5 insertions(+), 21 deletions(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 179b270d9ee7f..a534d8fed3166 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -2426,9 +2426,9 @@ ConstraintSystem::matchExistentialTypes(Type type1, Type type2, static bool isStringCompatiblePointerBaseType(ASTContext &ctx, Type baseType) { // Allow strings to be passed to pointer-to-byte or pointer-to-void types. - if (baseType->isEqual(TypeChecker::getInt8Type(ctx))) + if (baseType->isEqual(ctx.getInt8Decl()->getDeclaredInterfaceType())) return true; - if (baseType->isEqual(TypeChecker::getUInt8Type(ctx))) + if (baseType->isEqual(ctx.getUInt8Decl()->getDeclaredInterfaceType())) return true; if (baseType->isEqual(ctx.TheEmptyTupleType)) return true; @@ -9213,11 +9213,11 @@ ConstraintSystem::simplifyRestrictedConstraintImpl( auto &ctx = getASTContext(); auto int8Con = Constraint::create(*this, ConstraintKind::Bind, baseType2, - TypeChecker::getInt8Type(ctx), + ctx.getInt8Decl()->getDeclaredInterfaceType(), getConstraintLocator(locator)); auto uint8Con = Constraint::create(*this, ConstraintKind::Bind, baseType2, - TypeChecker::getUInt8Type(ctx), + ctx.getUInt8Decl()->getDeclaredInterfaceType(), getConstraintLocator(locator)); auto voidCon = Constraint::create(*this, ConstraintKind::Bind, baseType2, ctx.TheEmptyTupleType, diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index 2c0b8f8a3248d..f5a2c3d8c0d12 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -386,21 +386,7 @@ Type TypeChecker::getStringType(ASTContext &Context) { return Type(); } - - return Type(); -} - -Type TypeChecker::getInt8Type(ASTContext &Context) { - if (auto typeDecl = Context.getInt8Decl()) - return typeDecl->getDeclaredInterfaceType(); - - return Type(); -} - -Type TypeChecker::getUInt8Type(ASTContext &Context) { - if (auto typeDecl = Context.getUInt8Decl()) - return typeDecl->getDeclaredInterfaceType(); - + llvm::report_fatal_error("Broken Standard library: Cannot resolve String"); return Type(); } diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index 1fecda02df67c..68f83dd0b774d 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -348,8 +348,6 @@ Type getArraySliceType(SourceLoc loc, Type elementType); Type getOptionalType(SourceLoc loc, Type elementType); Type getStringType(ASTContext &ctx); Type getSubstringType(ASTContext &ctx); -Type getInt8Type(ASTContext &ctx); -Type getUInt8Type(ASTContext &ctx); /// Bind an UnresolvedDeclRefExpr by performing name lookup and /// returning the resultant expression. Context is the DeclContext used From 399a5510d21d7e9d272505e40ad3df714030cc93 Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 11:54:21 -0700 Subject: [PATCH 17/33] [NFC] Inline TypeChecker::getStringType --- lib/Sema/CSApply.cpp | 4 +++- lib/Sema/CSDiagnostics.cpp | 2 +- lib/Sema/CSSimplify.cpp | 3 ++- lib/Sema/TypeCheckType.cpp | 10 ---------- lib/Sema/TypeChecker.h | 2 -- 5 files changed, 6 insertions(+), 15 deletions(-) diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 92648cb78aa6a..766c57bc496f1 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -4698,7 +4698,9 @@ namespace { StringRef(stringCopy, compatStringBuf.size()), SourceRange(), /*implicit*/ true); - cs.setType(stringExpr, TypeChecker::getStringType(cs.getASTContext())); + cs.setType( + stringExpr, + cs.getASTContext().getStringDecl()->getDeclaredInterfaceType()); E->setObjCStringLiteralExpr(stringExpr); } } diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index 648308fb6e746..1e455b9a25dc6 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -2570,7 +2570,7 @@ bool ContextualFailure::trySequenceSubsequenceFixIts( if (!getASTContext().getStdlibModule()) return false; - auto String = TypeChecker::getStringType(getASTContext()); + auto String = getASTContext().getStringDecl()->getDeclaredInterfaceType(); auto Substring = getASTContext().getSubstringDecl()->getDeclaredInterfaceType(); // Substring -> String conversion diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index a534d8fed3166..50e3b0ef2ed21 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -4926,7 +4926,8 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind, // The pointer can be converted from a string, if the element // type is compatible. auto &ctx = getASTContext(); - if (type1->isEqual(TypeChecker::getStringType(ctx))) { + if (type1->isEqual( + ctx.getStringDecl()->getDeclaredInterfaceType())) { auto baseTy = getFixedTypeRecursive(pointeeTy, false); if (baseTy->isTypeVariableOrMember() || diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index f5a2c3d8c0d12..39701fafc2fc8 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -380,16 +380,6 @@ Type TypeChecker::getOptionalType(SourceLoc loc, Type elementType) { return OptionalType::get(elementType); } -Type TypeChecker::getStringType(ASTContext &Context) { - if (auto typeDecl = Context.getStringDecl()) - return typeDecl->getDeclaredInterfaceType(); - - return Type(); -} - llvm::report_fatal_error("Broken Standard library: Cannot resolve String"); - return Type(); -} - Type TypeChecker::getDynamicBridgedThroughObjCClass(DeclContext *dc, Type dynamicType, diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index 68f83dd0b774d..7315910400fff 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -346,8 +346,6 @@ enum class CheckedCastContextKind { namespace TypeChecker { Type getArraySliceType(SourceLoc loc, Type elementType); Type getOptionalType(SourceLoc loc, Type elementType); -Type getStringType(ASTContext &ctx); -Type getSubstringType(ASTContext &ctx); /// Bind an UnresolvedDeclRefExpr by performing name lookup and /// returning the resultant expression. Context is the DeclContext used From eacc130999f7163df5a7f3669bdfa0bc88629f6e Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 11:56:11 -0700 Subject: [PATCH 18/33] [NFC] Consistently return ErrorTypes when resolveType fails --- lib/Sema/TypeCheckType.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index 39701fafc2fc8..cbf73d6c55a9c 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -3530,7 +3530,9 @@ Type TypeResolver::resolveMetatypeType(MetatypeTypeRepr *repr, TypeResolutionOptions options) { // The instance type of a metatype is always abstract, not SIL-lowered. Type ty = resolveType(repr->getBase(), options.withoutContext()); - if (!ty || ty->hasError()) return ty; + if (!ty || ty->hasError()) { + return ErrorType::get(Context); + } Optional storedRepr; @@ -3561,7 +3563,9 @@ Type TypeResolver::resolveProtocolType(ProtocolTypeRepr *repr, TypeResolutionOptions options) { // The instance type of a metatype is always abstract, not SIL-lowered. Type ty = resolveType(repr->getBase(), options.withoutContext()); - if (!ty || ty->hasError()) return ty; + if (!ty || ty->hasError()) { + return ErrorType::get(Context); + } Optional storedRepr; From 22ead9f9f7a33d8c350f693d2697e853894c0ff3 Mon Sep 17 00:00:00 2001 From: Xi Ge Date: Thu, 11 Jun 2020 12:32:55 -0700 Subject: [PATCH 19/33] DepScanner: use emplace_back instead of push_back. rdar://64227623 --- lib/FrontendTool/ScanDependencies.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/FrontendTool/ScanDependencies.cpp b/lib/FrontendTool/ScanDependencies.cpp index 3487946ca3346..ab0c1f4160c59 100644 --- a/lib/FrontendTool/ScanDependencies.cpp +++ b/lib/FrontendTool/ScanDependencies.cpp @@ -164,7 +164,7 @@ static std::vector resolveDirectDependencies( /*onlyClangModule=*/false, cache, ASTDelegate)) { - result.push_back({overlayName.str(), found->getKind()}); + result.emplace_back(overlayName.str(), found->getKind()); } } }); From 6ab29cbe4e973779baebefdf0318c894764a72aa Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 13:00:21 -0700 Subject: [PATCH 20/33] Introduce NeverNullType to Assert resolveType Never Returns the null Type --- lib/Sema/TypeCheckType.cpp | 89 ++++++++++++------- .../0163-sr8033.swift | 2 +- 2 files changed, 60 insertions(+), 31 deletions(-) diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index cbf73d6c55a9c..92d38f57dc98e 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -1647,6 +1647,34 @@ namespace { const auto DefaultParameterConvention = ParameterConvention::Direct_Unowned; const auto DefaultResultConvention = ResultConvention::Unowned; + /// A wrapper that ensures that the returned type from + /// \c TypeResolver::resolveType is never the null \c Type. It otherwise + /// tries to behave like \c Type, so it provides the proper conversion and + /// arrow operators. + class NeverNullType final { + public: + /// Forbid default construction. + NeverNullType() = delete; + /// Forbid construction from \c nullptr. + NeverNullType(std::nullptr_t) = delete; + + public: + /// Construct a never-null Type. If \p Ty is null, a fatal error is thrown. + NeverNullType(Type Ty) : WrappedTy(Ty) { + if (Ty.isNull()) { + llvm::report_fatal_error("Resolved to null type!"); + } + } + + operator Type() const { return WrappedTy; } + Type get() const { return WrappedTy; } + + TypeBase *operator->() const { return WrappedTy.operator->(); } + + private: + Type WrappedTy; + }; + class TypeResolver { ASTContext &Context; TypeResolution resolution; @@ -1660,7 +1688,7 @@ namespace { { } - Type resolveType(TypeRepr *repr, TypeResolutionOptions options); + NeverNullType resolveType(TypeRepr *repr, TypeResolutionOptions options); private: template @@ -1795,7 +1823,8 @@ Type ResolveTypeRequest::evaluate(Evaluator &evaluator, return result; } -Type TypeResolver::resolveType(TypeRepr *repr, TypeResolutionOptions options) { +NeverNullType TypeResolver::resolveType(TypeRepr *repr, + TypeResolutionOptions options) { assert(repr && "Cannot validate null TypeReprs!"); // If we know the type representation is invalid, just return an @@ -1891,9 +1920,9 @@ Type TypeResolver::resolveType(TypeRepr *repr, TypeResolutionOptions options) { options |= TypeResolutionFlags::SilenceErrors; auto constraintType = resolveType(opaqueRepr->getConstraint(), options); - - return constraintType && !constraintType->hasError() - ? ErrorType::get(constraintType) : ErrorType::get(Context); + + return !constraintType->hasError() ? ErrorType::get(constraintType) + : ErrorType::get(Context); } case TypeReprKind::Fixed: @@ -1976,7 +2005,7 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs, instanceOptions -= TypeResolutionFlags::SILType; auto instanceTy = resolveType(base, instanceOptions); - if (!instanceTy || instanceTy->hasError()) + if (instanceTy->hasError()) return instanceTy; // Check for @thin. @@ -2440,8 +2469,8 @@ TypeResolver::resolveASTFunctionTypeParams(TupleTypeRepr *inputRepr, variadic = true; } - Type ty = resolveType(eltTypeRepr, thisElementOptions); - if (!ty || ty->hasError()) { + auto ty = resolveType(eltTypeRepr, thisElementOptions); + if (ty->hasError()) { elements.emplace_back(ErrorType::get(Context)); continue; } @@ -2550,7 +2579,7 @@ Type TypeResolver::resolveOpaqueReturnType(TypeRepr *repr, for (auto argRepr : generic->getGenericArgs()) { auto argTy = resolveType(argRepr, options); // If we cannot resolve the generic parameter, propagate the error out. - if (!argTy || argTy->hasError()) { + if (argTy->hasError()) { return ErrorType::get(Context); } TypeArgsBuf.push_back(argTy); @@ -2594,8 +2623,8 @@ Type TypeResolver::resolveASTFunctionType( auto resultOptions = options.withoutContext(); resultOptions.setContext(TypeResolverContext::FunctionResult); - Type outputTy = resolveType(repr->getResultTypeRepr(), resultOptions); - if (!outputTy || outputTy->hasError()) { + auto outputTy = resolveType(repr->getResultTypeRepr(), resultOptions); + if (outputTy->hasError()) { return ErrorType::get(Context); } @@ -3255,8 +3284,8 @@ Type TypeResolver::resolveSpecifierTypeRepr(SpecifierTypeRepr *repr, Type TypeResolver::resolveArrayType(ArrayTypeRepr *repr, TypeResolutionOptions options) { - Type baseTy = resolveType(repr->getBase(), options.withoutContext()); - if (!baseTy || baseTy->hasError()) { + auto baseTy = resolveType(repr->getBase(), options.withoutContext()); + if (baseTy->hasError()) { return ErrorType::get(Context); } @@ -3272,13 +3301,13 @@ Type TypeResolver::resolveDictionaryType(DictionaryTypeRepr *repr, TypeResolutionOptions options) { options = adjustOptionsForGenericArgs(options); - Type keyTy = resolveType(repr->getKey(), options.withoutContext()); - if (!keyTy || keyTy->hasError()) { + auto keyTy = resolveType(repr->getKey(), options.withoutContext()); + if (keyTy->hasError()) { return ErrorType::get(Context); } - Type valueTy = resolveType(repr->getValue(), options.withoutContext()); - if (!valueTy || valueTy->hasError()) { + auto valueTy = resolveType(repr->getValue(), options.withoutContext()); + if (valueTy->hasError()) { return ErrorType::get(Context); } @@ -3304,8 +3333,8 @@ Type TypeResolver::resolveOptionalType(OptionalTypeRepr *repr, TypeResolutionOptions elementOptions = options.withoutContext(true); elementOptions.setContext(TypeResolverContext::ImmediateOptionalTypeArgument); - Type baseTy = resolveType(repr->getBase(), elementOptions); - if (!baseTy || baseTy->hasError()) { + auto baseTy = resolveType(repr->getBase(), elementOptions); + if (baseTy->hasError()) { return ErrorType::get(Context); } @@ -3376,12 +3405,12 @@ Type TypeResolver::resolveImplicitlyUnwrappedOptionalType( TypeResolutionOptions elementOptions = options.withoutContext(true); elementOptions.setContext(TypeResolverContext::ImmediateOptionalTypeArgument); - Type baseTy = resolveType(repr->getBase(), elementOptions); - if (!baseTy || baseTy->hasError()) { + auto baseTy = resolveType(repr->getBase(), elementOptions); + if (baseTy->hasError()) { return ErrorType::get(Context); } - Type uncheckedOptionalTy = + auto uncheckedOptionalTy = TypeChecker::getOptionalType(repr->getExclamationLoc(), baseTy); if (uncheckedOptionalTy->hasError()) { return ErrorType::get(Context); @@ -3416,8 +3445,8 @@ Type TypeResolver::resolveTupleType(TupleTypeRepr *repr, for (unsigned i = 0, end = repr->getNumElements(); i != end; ++i) { auto *tyR = repr->getElementType(i); - Type ty = resolveType(tyR, elementOptions); - if (!ty || ty->hasError()) + auto ty = resolveType(tyR, elementOptions); + if (ty->hasError()) hadError = true; auto eltName = repr->getElementName(i); @@ -3485,8 +3514,8 @@ Type TypeResolver::resolveCompositionType(CompositionTypeRepr *repr, }; for (auto tyR : repr->getTypes()) { - Type ty = resolveType(tyR, options.withoutContext()); - if (!ty || ty->hasError()) return ty; + auto ty = resolveType(tyR, options.withoutContext()); + if (ty->hasError()) return ty; auto nominalDecl = ty->getAnyNominal(); if (nominalDecl && isa(nominalDecl)) { @@ -3529,8 +3558,8 @@ Type TypeResolver::resolveCompositionType(CompositionTypeRepr *repr, Type TypeResolver::resolveMetatypeType(MetatypeTypeRepr *repr, TypeResolutionOptions options) { // The instance type of a metatype is always abstract, not SIL-lowered. - Type ty = resolveType(repr->getBase(), options.withoutContext()); - if (!ty || ty->hasError()) { + auto ty = resolveType(repr->getBase(), options.withoutContext()); + if (ty->hasError()) { return ErrorType::get(Context); } @@ -3562,8 +3591,8 @@ Type TypeResolver::buildMetatypeType( Type TypeResolver::resolveProtocolType(ProtocolTypeRepr *repr, TypeResolutionOptions options) { // The instance type of a metatype is always abstract, not SIL-lowered. - Type ty = resolveType(repr->getBase(), options.withoutContext()); - if (!ty || ty->hasError()) { + auto ty = resolveType(repr->getBase(), options.withoutContext()); + if (ty->hasError()) { return ErrorType::get(Context); } diff --git a/validation-test/compiler_crashers_2_fixed/0163-sr8033.swift b/validation-test/compiler_crashers_2_fixed/0163-sr8033.swift index 3553340caf55c..cd51bdf472be7 100644 --- a/validation-test/compiler_crashers_2_fixed/0163-sr8033.swift +++ b/validation-test/compiler_crashers_2_fixed/0163-sr8033.swift @@ -7,4 +7,4 @@ protocol P1 { } extension Foo: P1 where A : P1 {} // expected-error {{unsupported recursion for reference to associated type 'A' of type 'Foo'}} // expected-error@-1 {{type 'Foo' does not conform to protocol 'P1'}} -// expected-error@-2 {{type 'Foo' in conformance requirement does not refer to a generic parameter or associated type}} +// expected-error@-2 {{type '<>' in conformance requirement does not refer to a generic parameter or associated type}} From 002369565b9ab3052c352046fe59c6e0155d5184 Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 13:04:53 -0700 Subject: [PATCH 21/33] [NFC] Update calls to assume resolveType never returns null --- lib/Sema/CSGen.cpp | 4 ++-- lib/Sema/TypeCheckAttr.cpp | 2 +- lib/Sema/TypeCheckConstraints.cpp | 2 +- lib/Sema/TypeCheckPattern.cpp | 4 ++-- lib/Sema/TypeCheckType.cpp | 2 +- lib/Sema/TypeCheckType.h | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 77808245beacd..356833955c0df 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -1499,7 +1499,7 @@ namespace { options |= TypeResolutionFlags::AllowUnboundGenerics; auto result = TypeResolution::forContextual(CS.DC, options) .resolveType(repr); - if (!result || result->hasError()) { + if (result->hasError()) { return Type(); } return result; @@ -2771,7 +2771,7 @@ namespace { Type castType = TypeResolution::forContextual( CS.DC, TypeResolverContext::InExpression) .resolveType(isp->getCastTypeRepr()); - if (!castType) { + if (castType->hasError()) { return false; } diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index 505232f6748ad..77bdb991f794d 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -2853,7 +2853,7 @@ void AttributeChecker::visitImplementsAttr(ImplementsAttr *attr) { } // Definite error-types were already diagnosed in resolveType. - if (!T || T->hasError()) + if (T->hasError()) return; attr->setProtocolType(T); diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index 7435775298de8..3aec9a838eacb 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -1397,7 +1397,7 @@ TypeExpr *PreCheckExpression::simplifyNestedTypeExpr(UnresolvedDotExpr *UDE) { auto resolution = TypeResolution::forContextual(DC, options); auto BaseTy = resolution.resolveType(InnerTypeRepr); - if (BaseTy && BaseTy->mayHaveMembers()) { + if (BaseTy->mayHaveMembers()) { auto lookupOptions = defaultMemberLookupOptions; if (isa(DC) || isa(DC)) diff --git a/lib/Sema/TypeCheckPattern.cpp b/lib/Sema/TypeCheckPattern.cpp index 3cee1b6d1ce01..f24f3df0c0c0e 100644 --- a/lib/Sema/TypeCheckPattern.cpp +++ b/lib/Sema/TypeCheckPattern.cpp @@ -705,7 +705,7 @@ static Type validateTypedPattern(TypedPattern *TP, TypeResolution resolution) { } auto ty = resolution.resolveType(Repr); - if (!ty || ty->hasError()) { + if (ty->hasError()) { return ErrorType::get(Context); } @@ -1233,7 +1233,7 @@ Pattern *TypeChecker::coercePatternToType(ContextualPattern pattern, TypeResolutionOptions paramOptions(TypeResolverContext::InExpression); auto castType = TypeResolution::forContextual(dc, paramOptions) .resolveType(IP->getCastTypeRepr()); - if (!castType || castType->hasError()) + if (castType->hasError()) return nullptr; IP->setCastType(castType); diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index 92d38f57dc98e..ca1c584062354 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -746,7 +746,7 @@ static Type applyGenericArguments(Type type, TypeResolution resolution, if (nominal->isOptionalDecl()) { // Validate the generic argument. Type objectType = resolution.resolveType(genericArgs[0]); - if (!objectType || objectType->hasError()) { + if (objectType->hasError()) { return ErrorType::get(ctx); } diff --git a/lib/Sema/TypeCheckType.h b/lib/Sema/TypeCheckType.h index b09e418464c2a..e32632d877330 100644 --- a/lib/Sema/TypeCheckType.h +++ b/lib/Sema/TypeCheckType.h @@ -366,7 +366,7 @@ class TypeResolution { /// /// \param TyR The type representation to check. /// - /// \returns a well-formed type or an ErrorType in case of an error. + /// \returns A well-formed type that is never null, or an \c ErrorType in case of an error. Type resolveType(TypeRepr *TyR); /// Whether this type resolution uses archetypes (vs. generic parameters). From 0c597a8b9221b416215f199a717c82f8fc4c3499 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 11 Jun 2020 15:25:11 -0400 Subject: [PATCH 22/33] Dependencies: Move YAML reader and writer code to swift-dependency-tool --- include/swift/AST/FineGrainedDependencies.h | 55 -------- lib/AST/FineGrainedDependencies.cpp | 86 ------------ .../swift-dependency-tool.cpp | 130 +++++++++++++++++- 3 files changed, 129 insertions(+), 142 deletions(-) diff --git a/include/swift/AST/FineGrainedDependencies.h b/include/swift/AST/FineGrainedDependencies.h index 381b4443c0cb3..e904cf4fbe703 100644 --- a/include/swift/AST/FineGrainedDependencies.h +++ b/include/swift/AST/FineGrainedDependencies.h @@ -804,15 +804,6 @@ class SourceFileDepGraph { forEachNode([&](SourceFileDepGraphNode *n) { delete n; }); } - /// Goes at the start of an emitted YAML file to help tools recognize it. - /// May vary in the future according to version, etc. - std::string yamlProlog(const bool hadCompilationError) const { - return std::string("# Fine-grained v0\n") + - (!hadCompilationError ? "" - : "# Dependencies are unknown because a " - "compilation error occurred.\n"); - } - SourceFileDepGraphNode *getNode(size_t sequenceNumber) const; InterfaceAndImplementationPair @@ -1016,50 +1007,4 @@ template class DotFileEmitter { } // end namespace fine_grained_dependencies } // end namespace swift -//============================================================================== -// MARK: Declarations for YAMLTraits for reading/writing of SourceFileDepGraph -//============================================================================== - -// This introduces a redefinition where ever std::is_same_t -// holds -#if !(defined(__linux__) || defined(_WIN64)) -LLVM_YAML_DECLARE_SCALAR_TRAITS(size_t, QuotingType::None) -#endif -LLVM_YAML_DECLARE_ENUM_TRAITS(swift::fine_grained_dependencies::NodeKind) -LLVM_YAML_DECLARE_ENUM_TRAITS(swift::fine_grained_dependencies::DeclAspect) -LLVM_YAML_DECLARE_MAPPING_TRAITS( - swift::fine_grained_dependencies::DependencyKey) -LLVM_YAML_DECLARE_MAPPING_TRAITS(swift::fine_grained_dependencies::DepGraphNode) - -namespace llvm { -namespace yaml { -template <> -struct MappingContextTraits< - swift::fine_grained_dependencies::SourceFileDepGraphNode, - swift::fine_grained_dependencies::SourceFileDepGraph> { - using SourceFileDepGraphNode = - swift::fine_grained_dependencies::SourceFileDepGraphNode; - using SourceFileDepGraph = - swift::fine_grained_dependencies::SourceFileDepGraph; - - static void mapping(IO &io, SourceFileDepGraphNode &node, - SourceFileDepGraph &g); -}; - -template <> -struct SequenceTraits< - std::vector> { - using SourceFileDepGraphNode = - swift::fine_grained_dependencies::SourceFileDepGraphNode; - using NodeVec = std::vector; - static size_t size(IO &, NodeVec &vec); - static SourceFileDepGraphNode &element(IO &, NodeVec &vec, size_t index); -}; - -} // namespace yaml -} // namespace llvm - -LLVM_YAML_DECLARE_MAPPING_TRAITS( - swift::fine_grained_dependencies::SourceFileDepGraph) - #endif // SWIFT_AST_FINE_GRAINED_DEPENDENCIES_H diff --git a/lib/AST/FineGrainedDependencies.cpp b/lib/AST/FineGrainedDependencies.cpp index 4a0c99bbfcd3c..5543d03c7f4a5 100644 --- a/lib/AST/FineGrainedDependencies.cpp +++ b/lib/AST/FineGrainedDependencies.cpp @@ -28,7 +28,6 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" -#include "llvm/Support/YAMLParser.h" // This file holds the definitions for the fine-grained dependency system @@ -371,88 +370,3 @@ void SourceFileDepGraph::emitDotFile(StringRef outputPath, return false; }); } - -//============================================================================== -// MARK: SourceFileDepGraph YAML reading & writing -//============================================================================== - -namespace llvm { -namespace yaml { -// This introduces a redefinition for Linux. -#if !(defined(__linux__) || defined(_WIN64)) -void ScalarTraits::output(const size_t &Val, void *, raw_ostream &out) { - out << Val; -} - -StringRef ScalarTraits::input(StringRef scalar, void *ctxt, - size_t &value) { - return scalar.getAsInteger(10, value) ? "could not parse size_t" : ""; -} -#endif - -void ScalarEnumerationTraits:: - enumeration(IO &io, swift::fine_grained_dependencies::NodeKind &value) { - using NodeKind = swift::fine_grained_dependencies::NodeKind; - io.enumCase(value, "topLevel", NodeKind::topLevel); - io.enumCase(value, "nominal", NodeKind::nominal); - io.enumCase(value, "potentialMember", NodeKind::potentialMember); - io.enumCase(value, "member", NodeKind::member); - io.enumCase(value, "dynamicLookup", NodeKind::dynamicLookup); - io.enumCase(value, "externalDepend", NodeKind::externalDepend); - io.enumCase(value, "sourceFileProvide", NodeKind::sourceFileProvide); -} - -void ScalarEnumerationTraits::enumeration( - IO &io, swift::fine_grained_dependencies::DeclAspect &value) { - using DeclAspect = swift::fine_grained_dependencies::DeclAspect; - io.enumCase(value, "interface", DeclAspect::interface); - io.enumCase(value, "implementation", DeclAspect::implementation); -} - -void MappingTraits::mapping( - IO &io, swift::fine_grained_dependencies::DependencyKey &key) { - io.mapRequired("kind", key.kind); - io.mapRequired("aspect", key.aspect); - io.mapRequired("context", key.context); - io.mapRequired("name", key.name); -} - -void MappingTraits::mapping( - IO &io, swift::fine_grained_dependencies::DepGraphNode &node) { - io.mapRequired("key", node.key); - io.mapOptional("fingerprint", node.fingerprint); -} - -void MappingContextTraits::mapping( - IO &io, SourceFileDepGraphNode &node, SourceFileDepGraph &g) { - MappingTraits::mapping(io, node); - io.mapRequired("sequenceNumber", node.sequenceNumber); - std::vector defsIDependUponVec(node.defsIDependUpon.begin(), - node.defsIDependUpon.end()); - io.mapRequired("defsIDependUpon", defsIDependUponVec); - io.mapRequired("isProvides", node.isProvides); - if (!io.outputting()) { - for (size_t u : defsIDependUponVec) - node.defsIDependUpon.insert(u); - } - assert(g.getNode(node.sequenceNumber) && "Bad sequence number"); -} - -size_t SequenceTraits>::size( - IO &, std::vector &vec) { - return vec.size(); -} - -SourceFileDepGraphNode & -SequenceTraits>::element( - IO &, std::vector &vec, size_t index) { - while (vec.size() <= index) - vec.push_back(new SourceFileDepGraphNode()); - return *vec[index]; -} - -void MappingTraits::mapping(IO &io, SourceFileDepGraph &g) { - io.mapRequired("allNodes", g.allNodes, g); -} -} // namespace yaml -} // namespace llvm diff --git a/tools/swift-dependency-tool/swift-dependency-tool.cpp b/tools/swift-dependency-tool/swift-dependency-tool.cpp index fd60d63c85070..cc6f50c0f8b8f 100644 --- a/tools/swift-dependency-tool/swift-dependency-tool.cpp +++ b/tools/swift-dependency-tool/swift-dependency-tool.cpp @@ -19,10 +19,138 @@ #include "llvm/Support/CommandLine.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/YAMLParser.h" +#include "llvm/Support/YAMLTraits.h" using namespace swift; using namespace fine_grained_dependencies; +//============================================================================== +// MARK: SourceFileDepGraph YAML reading & writing +//============================================================================== + +// This introduces a redefinition where ever std::is_same_t +// holds +#if !(defined(__linux__) || defined(_WIN64)) +LLVM_YAML_DECLARE_SCALAR_TRAITS(size_t, QuotingType::None) +#endif +LLVM_YAML_DECLARE_ENUM_TRAITS(swift::fine_grained_dependencies::NodeKind) +LLVM_YAML_DECLARE_ENUM_TRAITS(swift::fine_grained_dependencies::DeclAspect) +LLVM_YAML_DECLARE_MAPPING_TRAITS( + swift::fine_grained_dependencies::DependencyKey) +LLVM_YAML_DECLARE_MAPPING_TRAITS(swift::fine_grained_dependencies::DepGraphNode) + +namespace llvm { +namespace yaml { +template <> +struct MappingContextTraits< + swift::fine_grained_dependencies::SourceFileDepGraphNode, + swift::fine_grained_dependencies::SourceFileDepGraph> { + using SourceFileDepGraphNode = + swift::fine_grained_dependencies::SourceFileDepGraphNode; + using SourceFileDepGraph = + swift::fine_grained_dependencies::SourceFileDepGraph; + + static void mapping(IO &io, SourceFileDepGraphNode &node, + SourceFileDepGraph &g); +}; + +template <> +struct SequenceTraits< + std::vector> { + using SourceFileDepGraphNode = + swift::fine_grained_dependencies::SourceFileDepGraphNode; + using NodeVec = std::vector; + static size_t size(IO &, NodeVec &vec); + static SourceFileDepGraphNode &element(IO &, NodeVec &vec, size_t index); +}; + +} // namespace yaml +} // namespace llvm + +LLVM_YAML_DECLARE_MAPPING_TRAITS( + swift::fine_grained_dependencies::SourceFileDepGraph) + +namespace llvm { +namespace yaml { +// This introduces a redefinition for Linux. +#if !(defined(__linux__) || defined(_WIN64)) +void ScalarTraits::output(const size_t &Val, void *, raw_ostream &out) { + out << Val; +} + +StringRef ScalarTraits::input(StringRef scalar, void *ctxt, + size_t &value) { + return scalar.getAsInteger(10, value) ? "could not parse size_t" : ""; +} +#endif + +void ScalarEnumerationTraits:: + enumeration(IO &io, swift::fine_grained_dependencies::NodeKind &value) { + using NodeKind = swift::fine_grained_dependencies::NodeKind; + io.enumCase(value, "topLevel", NodeKind::topLevel); + io.enumCase(value, "nominal", NodeKind::nominal); + io.enumCase(value, "potentialMember", NodeKind::potentialMember); + io.enumCase(value, "member", NodeKind::member); + io.enumCase(value, "dynamicLookup", NodeKind::dynamicLookup); + io.enumCase(value, "externalDepend", NodeKind::externalDepend); + io.enumCase(value, "sourceFileProvide", NodeKind::sourceFileProvide); +} + +void ScalarEnumerationTraits::enumeration( + IO &io, swift::fine_grained_dependencies::DeclAspect &value) { + using DeclAspect = swift::fine_grained_dependencies::DeclAspect; + io.enumCase(value, "interface", DeclAspect::interface); + io.enumCase(value, "implementation", DeclAspect::implementation); +} + +void MappingTraits::mapping( + IO &io, swift::fine_grained_dependencies::DependencyKey &key) { + io.mapRequired("kind", key.kind); + io.mapRequired("aspect", key.aspect); + io.mapRequired("context", key.context); + io.mapRequired("name", key.name); +} + +void MappingTraits::mapping( + IO &io, swift::fine_grained_dependencies::DepGraphNode &node) { + io.mapRequired("key", node.key); + io.mapOptional("fingerprint", node.fingerprint); +} + +void MappingContextTraits::mapping( + IO &io, SourceFileDepGraphNode &node, SourceFileDepGraph &g) { + MappingTraits::mapping(io, node); + io.mapRequired("sequenceNumber", node.sequenceNumber); + std::vector defsIDependUponVec(node.defsIDependUpon.begin(), + node.defsIDependUpon.end()); + io.mapRequired("defsIDependUpon", defsIDependUponVec); + io.mapRequired("isProvides", node.isProvides); + if (!io.outputting()) { + for (size_t u : defsIDependUponVec) + node.defsIDependUpon.insert(u); + } + assert(g.getNode(node.sequenceNumber) && "Bad sequence number"); +} + +size_t SequenceTraits>::size( + IO &, std::vector &vec) { + return vec.size(); +} + +SourceFileDepGraphNode & +SequenceTraits>::element( + IO &, std::vector &vec, size_t index) { + while (vec.size() <= index) + vec.push_back(new SourceFileDepGraphNode()); + return *vec[index]; +} + +void MappingTraits::mapping(IO &io, SourceFileDepGraph &g) { + io.mapRequired("allNodes", g.allNodes, g); +} +} // namespace yaml +} // namespace llvm + enum class ActionType : unsigned { None, BinaryToYAML, @@ -81,7 +209,7 @@ int main(int argc, char *argv[]) { bool hadError = withOutputFile(diags, options::OutputFilename, [&](llvm::raw_pwrite_stream &out) { - out << fg->yamlProlog(/*hadError=*/false); + out << "# Fine-grained v0\n"; llvm::yaml::Output yamlWriter(out); yamlWriter << *fg; return false; From 86b7e989e7abd639d7a4015cd1b72563c52689e4 Mon Sep 17 00:00:00 2001 From: Anthony Latsis Date: Thu, 11 Jun 2020 23:58:01 +0300 Subject: [PATCH 23/33] [NFC] CS: Give the type-transforming openUnboundGenericType method a more accurate name --- lib/Sema/CSBindings.cpp | 2 +- lib/Sema/CSGen.cpp | 22 +++++++++++----------- lib/Sema/ConstraintSystem.cpp | 8 ++++---- lib/Sema/ConstraintSystem.h | 2 +- lib/Sema/TypeCheckPropertyWrapper.cpp | 2 +- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index a05f865768a32..b159a8494c566 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -1072,7 +1072,7 @@ bool TypeVariableBinding::attempt(ConstraintSystem &cs) const { auto *dstLocator = TypeVar->getImpl().getLocator(); if (Binding.hasDefaultedLiteralProtocol()) { - type = cs.openUnboundGenericType(type, dstLocator); + type = cs.openUnboundGenericTypes(type, dstLocator); type = type->reconstituteSugar(/*recursive=*/false); } diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 15b1d5ee7a85a..a384e5e54f6ed 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -1446,7 +1446,7 @@ namespace { if (auto *PD = dyn_cast(VD)) { if (!CS.hasType(PD)) { if (knownType && knownType->hasUnboundGenericType()) - knownType = CS.openUnboundGenericType(knownType, locator); + knownType = CS.openUnboundGenericTypes(knownType, locator); CS.setType( PD, knownType ? knownType @@ -1521,7 +1521,7 @@ namespace { if (!type || type->hasError()) return Type(); auto locator = CS.getConstraintLocator(E); - type = CS.openUnboundGenericType(type, locator); + type = CS.openUnboundGenericTypes(type, locator); return MetatypeType::get(type); } @@ -2205,7 +2205,7 @@ namespace { Type externalType; if (param->getTypeRepr()) { auto declaredTy = param->getType(); - externalType = CS.openUnboundGenericType(declaredTy, paramLoc); + externalType = CS.openUnboundGenericTypes(declaredTy, paramLoc); } else { // Let's allow parameters which haven't been explicitly typed // to become holes by default, this helps in situations like @@ -2439,7 +2439,7 @@ namespace { // Look through reference storage types. type = type->getReferenceStorageReferent(); - Type openedType = CS.openUnboundGenericType(type, locator); + Type openedType = CS.openUnboundGenericTypes(type, locator); assert(openedType); auto *subPattern = cast(pattern)->getSubPattern(); @@ -2541,7 +2541,7 @@ namespace { if (!castType) return Type(); - castType = CS.openUnboundGenericType( + castType = CS.openUnboundGenericTypes( castType, locator.withPathElement(LocatorPathElt::PatternMatch(pattern))); @@ -2607,7 +2607,7 @@ namespace { if (!parentType) return Type(); - parentType = CS.openUnboundGenericType( + parentType = CS.openUnboundGenericTypes( parentType, CS.getConstraintLocator( locator, {LocatorPathElt::PatternMatch(pattern), ConstraintLocator::ParentType})); @@ -3100,7 +3100,7 @@ namespace { // Open the type we're casting to. const auto toType = - CS.openUnboundGenericType(type, CS.getConstraintLocator(expr)); + CS.openUnboundGenericTypes(type, CS.getConstraintLocator(expr)); if (repr) CS.setType(repr, toType); auto fromType = CS.getType(fromExpr); @@ -3127,7 +3127,7 @@ namespace { // Open the type we're casting to. const auto toType = - CS.openUnboundGenericType(type, CS.getConstraintLocator(expr)); + CS.openUnboundGenericTypes(type, CS.getConstraintLocator(expr)); if (repr) CS.setType(repr, toType); auto fromType = CS.getType(expr->getSubExpr()); @@ -3160,7 +3160,7 @@ namespace { // Open the type we're casting to. const auto toType = - CS.openUnboundGenericType(type, CS.getConstraintLocator(expr)); + CS.openUnboundGenericTypes(type, CS.getConstraintLocator(expr)); if (repr) CS.setType(repr, toType); auto fromType = CS.getType(fromExpr); @@ -3189,7 +3189,7 @@ namespace { // Open up the type we're checking. // FIXME: Locator for the cast type? const auto toType = - CS.openUnboundGenericType(type, CS.getConstraintLocator(expr)); + CS.openUnboundGenericTypes(type, CS.getConstraintLocator(expr)); CS.setType(expr->getCastTypeRepr(), toType); // Add a checked cast constraint. @@ -3469,7 +3469,7 @@ namespace { rootRepr, TypeResolverContext::InExpression); if (!rootObjectTy || rootObjectTy->hasError()) return Type(); - rootObjectTy = CS.openUnboundGenericType(rootObjectTy, locator); + rootObjectTy = CS.openUnboundGenericTypes(rootObjectTy, locator); // Allow \Derived.property to be inferred as \Base.property to // simulate a sort of covariant conversion from // KeyPath to KeyPath. diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 52eb827460772..7b28796ae60b1 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -661,7 +661,7 @@ ConstraintSystem::openUnboundGenericType(UnboundGenericType *unbound, auto unboundDecl = unbound->getDecl(); auto parentTy = unbound->getParent(); if (parentTy) { - parentTy = openUnboundGenericType(parentTy, locator); + parentTy = openUnboundGenericTypes(parentTy, locator); unbound = UnboundGenericType::get(unboundDecl, parentTy, getASTContext()); } @@ -782,7 +782,7 @@ static void checkNestedTypeConstraints(ConstraintSystem &cs, Type type, checkNestedTypeConstraints(cs, parentTy, locator); } -Type ConstraintSystem::openUnboundGenericType( +Type ConstraintSystem::openUnboundGenericTypes( Type type, ConstraintLocatorBuilder locator) { assert(!type->getCanonicalType()->hasTypeParameter()); @@ -1235,7 +1235,7 @@ ConstraintSystem::getTypeOfReference(ValueDecl *value, checkNestedTypeConstraints(*this, type, locator); // Open the type. - type = openUnboundGenericType(type, locator); + type = openUnboundGenericTypes(type, locator); // Module types are not wrapped in metatypes. if (type->is()) @@ -1491,7 +1491,7 @@ ConstraintSystem::getTypeOfMemberReference( checkNestedTypeConstraints(*this, memberTy, locator); // Open the type if it was a reference to a generic type. - memberTy = openUnboundGenericType(memberTy, locator); + memberTy = openUnboundGenericTypes(memberTy, locator); // Wrap it in a metatype. memberTy = MetatypeType::get(memberTy); diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index d458367f0f40b..4c93b2ef74571 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -3420,7 +3420,7 @@ class ConstraintSystem { /// \param type The type to open. /// /// \returns The opened type. - Type openUnboundGenericType(Type type, ConstraintLocatorBuilder locator); + Type openUnboundGenericTypes(Type type, ConstraintLocatorBuilder locator); /// "Open" the given type by replacing any occurrences of generic /// parameter types and dependent member types with fresh type variables. diff --git a/lib/Sema/TypeCheckPropertyWrapper.cpp b/lib/Sema/TypeCheckPropertyWrapper.cpp index 5c1cdfc6f9987..6866e96a2100c 100644 --- a/lib/Sema/TypeCheckPropertyWrapper.cpp +++ b/lib/Sema/TypeCheckPropertyWrapper.cpp @@ -619,7 +619,7 @@ PropertyWrapperBackingPropertyTypeRequest::evaluate( // Open the type. Type openedWrapperType = - cs.openUnboundGenericType(rawWrapperType, emptyLocator); + cs.openUnboundGenericTypes(rawWrapperType, emptyLocator); if (!outermostOpenedWrapperType) outermostOpenedWrapperType = openedWrapperType; From 8c3f154258837606e3af658bac516bab0852d823 Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 16:02:17 -0700 Subject: [PATCH 24/33] [NFC] Wean SILParser off of TypeLocs It was just using them as a currency type because performTypeLocChecking accepted them as a parameter. Cleaning up performTypeLocChecking is something that needs to be done independently of this changeset, so for now just encapsulate the use of TypeLocs as much as possible and use TypeRepr instead. --- lib/SIL/Parser/ParseSIL.cpp | 124 +++++++++++++++++------------------- 1 file changed, 60 insertions(+), 64 deletions(-) diff --git a/lib/SIL/Parser/ParseSIL.cpp b/lib/SIL/Parser/ParseSIL.cpp index f08dbc88235fc..90603b27bfbf3 100644 --- a/lib/SIL/Parser/ParseSIL.cpp +++ b/lib/SIL/Parser/ParseSIL.cpp @@ -174,7 +174,7 @@ namespace { /// A callback to be invoked every time a type was deserialized. std::function ParsedTypeCallback; - bool performTypeLocChecking(TypeLoc &T, bool IsSILType, + Type performTypeLocChecking(TypeRepr *TyR, bool IsSILType, GenericEnvironment *GenericEnv = nullptr); void convertRequirements(SILFunction *F, ArrayRef From, @@ -868,17 +868,15 @@ void SILParser::convertRequirements(SILFunction *F, IdentTypeReprLookup PerformLookup(P); // Use parser lexical scopes to resolve references // to the generic parameters. - auto ResolveToInterfaceType = [&](TypeLoc Ty) -> Type { - Ty.getTypeRepr()->walk(PerformLookup); - performTypeLocChecking(Ty, /* IsSIL */ false); - assert(Ty.getType()); - return Ty.getType()->mapTypeOutOfContext(); + auto ResolveToInterfaceType = [&](TypeRepr *Ty) -> Type { + Ty->walk(PerformLookup); + return performTypeLocChecking(Ty, /* IsSIL */ false)->mapTypeOutOfContext(); }; for (auto &Req : From) { if (Req.getKind() == RequirementReprKind::SameType) { - auto FirstType = ResolveToInterfaceType(Req.getFirstTypeLoc()); - auto SecondType = ResolveToInterfaceType(Req.getSecondTypeLoc()); + auto FirstType = ResolveToInterfaceType(Req.getFirstTypeRepr()); + auto SecondType = ResolveToInterfaceType(Req.getSecondTypeRepr()); Requirement ConvertedRequirement(RequirementKind::SameType, FirstType, SecondType); To.push_back(ConvertedRequirement); @@ -886,8 +884,8 @@ void SILParser::convertRequirements(SILFunction *F, } if (Req.getKind() == RequirementReprKind::TypeConstraint) { - auto Subject = ResolveToInterfaceType(Req.getSubjectLoc()); - auto Constraint = ResolveToInterfaceType(Req.getConstraintLoc()); + auto Subject = ResolveToInterfaceType(Req.getSubjectRepr()); + auto Constraint = ResolveToInterfaceType(Req.getConstraintRepr()); Requirement ConvertedRequirement(RequirementKind::Conformance, Subject, Constraint); To.push_back(ConvertedRequirement); @@ -895,7 +893,7 @@ void SILParser::convertRequirements(SILFunction *F, } if (Req.getKind() == RequirementReprKind::LayoutConstraint) { - auto Subject = ResolveToInterfaceType(Req.getSubjectLoc()); + auto Subject = ResolveToInterfaceType(Req.getSubjectRepr()); Requirement ConvertedRequirement(RequirementKind::Layout, Subject, Req.getLayoutConstraint()); To.push_back(ConvertedRequirement); @@ -1094,14 +1092,16 @@ static bool parseDeclSILOptional(bool *isTransparent, return false; } -bool SILParser::performTypeLocChecking(TypeLoc &T, bool IsSILType, +Type SILParser::performTypeLocChecking(TypeRepr *T, bool IsSILType, GenericEnvironment *GenericEnv) { if (GenericEnv == nullptr) GenericEnv = ContextGenericEnv; - return swift::performTypeLocChecking(P.Context, T, + TypeLoc loc(T); + (void) swift::performTypeLocChecking(P.Context, loc, /*isSILMode=*/true, IsSILType, GenericEnv, &P.SF); + return loc.getType(); } /// Find the top-level ValueDecl or Module given a name. @@ -1155,17 +1155,17 @@ static ValueDecl *lookupMember(Parser &P, Type Ty, DeclBaseName Name, bool SILParser::parseASTType(CanType &result, GenericEnvironment *env) { ParserResult parsedType = P.parseType(); if (parsedType.isNull()) return true; - TypeLoc loc = parsedType.get(); - if (performTypeLocChecking(loc, /*IsSILType=*/ false, env)) + auto resolvedType = performTypeLocChecking(parsedType.get(), /*IsSILType=*/ false, env); + if (resolvedType->hasError()) return true; if (env) - result = loc.getType()->mapTypeOutOfContext()->getCanonicalType(); + result = resolvedType->mapTypeOutOfContext()->getCanonicalType(); else - result = loc.getType()->getCanonicalType(); + result = resolvedType->getCanonicalType(); // Invoke the callback on the parsed type. - ParsedTypeCallback(loc.getType()); + ParsedTypeCallback(resolvedType); return false; } @@ -1244,16 +1244,16 @@ bool SILParser::parseSILType(SILType &Result, ParsedGenericEnv = env; // Apply attributes to the type. - TypeLoc Ty = P.applyAttributeToType(TyR.get(), attrs, specifier, specifierLoc); - - if (performTypeLocChecking(Ty, /*IsSILType=*/true, OuterGenericEnv)) + auto *attrRepr = P.applyAttributeToType(TyR.get(), attrs, specifier, specifierLoc); + auto Ty = performTypeLocChecking(attrRepr, /*IsSILType=*/true, OuterGenericEnv); + if (Ty->hasError()) return true; - Result = SILType::getPrimitiveType(Ty.getType()->getCanonicalType(), + Result = SILType::getPrimitiveType(Ty->getCanonicalType(), category); // Invoke the callback on the parsed type. - ParsedTypeCallback(Ty.getType()); + ParsedTypeCallback(Ty); return false; } @@ -1644,34 +1644,34 @@ bool SILParser::parseSILBBArgsAtBranch(SmallVector &Args, /// /// FIXME: This is a hack to work around the lack of a DeclContext for /// witness tables. -static void bindProtocolSelfInTypeRepr(TypeLoc &TL, ProtocolDecl *proto) { - if (auto typeRepr = TL.getTypeRepr()) { - // AST walker to update 'Self' references. - class BindProtocolSelf : public ASTWalker { - ProtocolDecl *proto; - GenericTypeParamDecl *selfParam; - Identifier selfId; - - public: - BindProtocolSelf(ProtocolDecl *proto) - : proto(proto), - selfParam(proto->getProtocolSelfType()->getDecl()), - selfId(proto->getASTContext().Id_Self) { - } +static void bindProtocolSelfInTypeRepr(TypeRepr *typeRepr, ProtocolDecl *proto) { + assert(typeRepr); - virtual bool walkToTypeReprPre(TypeRepr *T) override { - if (auto ident = dyn_cast(T)) { - auto firstComponent = ident->getComponentRange().front(); - if (firstComponent->getNameRef().isSimpleName(selfId)) - firstComponent->setValue(selfParam, proto); - } + // AST walker to update 'Self' references. + class BindProtocolSelf : public ASTWalker { + ProtocolDecl *proto; + GenericTypeParamDecl *selfParam; + Identifier selfId; - return true; + public: + BindProtocolSelf(ProtocolDecl *proto) + : proto(proto), + selfParam(proto->getProtocolSelfType()->getDecl()), + selfId(proto->getASTContext().Id_Self) { + } + + virtual bool walkToTypeReprPre(TypeRepr *T) override { + if (auto ident = dyn_cast(T)) { + auto firstComponent = ident->getComponentRange().front(); + if (firstComponent->getNameRef().isSimpleName(selfId)) + firstComponent->setValue(selfParam, proto); } - }; - typeRepr->walk(BindProtocolSelf(proto)); - } + return true; + } + }; + + typeRepr->walk(BindProtocolSelf(proto)); } /// Parse the substitution list for an apply instruction or @@ -1693,12 +1693,13 @@ bool SILParser::parseSubstitutions(SmallVectorImpl &parsed, ParserResult TyR = P.parseType(); if (TyR.isNull()) return true; - TypeLoc Ty = TyR.get(); if (defaultForProto) - bindProtocolSelfInTypeRepr(Ty, defaultForProto); - if (performTypeLocChecking(Ty, /*IsSILType=*/ false, GenericEnv)) + bindProtocolSelfInTypeRepr(TyR.get(), defaultForProto); + + auto Ty = performTypeLocChecking(TyR.get(), /*IsSILType=*/ false, GenericEnv); + if (Ty->hasError()) return true; - parsed.push_back({Loc, Ty.getType()}); + parsed.push_back({Loc, Ty}); } while (P.consumeIf(tok::comma)); // Consume the closing '>'. @@ -2090,31 +2091,27 @@ bool SILParser::parseSILDeclRef(SILDeclRef &Member, bool FnTypeRequired) { GenericsScope.reset(); if (TyR.isNull()) return true; - TypeLoc Ty = TyR.get(); // The type can be polymorphic. GenericEnvironment *genericEnv = nullptr; if (auto fnType = dyn_cast(TyR.get())) { if (auto generics = fnType->getGenericParams()) { - assert(!Ty.wasValidated() && Ty.getType().isNull()); - genericEnv = handleSILGenericParams(generics, &P.SF); fnType->setGenericEnvironment(genericEnv); } if (auto generics = fnType->getPatternGenericParams()) { - assert(!Ty.wasValidated() && Ty.getType().isNull()); - genericEnv = handleSILGenericParams(generics, &P.SF); fnType->setPatternGenericEnvironment(genericEnv); } } - if (performTypeLocChecking(Ty, /*IsSILType=*/ false, genericEnv)) + auto Ty = performTypeLocChecking(TyR.get(), /*IsSILType=*/ false, genericEnv); + if (Ty->hasError()) return true; // Pick the ValueDecl that has the right type. ValueDecl *TheDecl = nullptr; - auto declTy = Ty.getType()->getCanonicalType(); + auto declTy = Ty->getCanonicalType(); for (unsigned I = 0, E = values.size(); I < E; ++I) { auto *decl = values[I]; @@ -6306,14 +6303,13 @@ ProtocolConformanceRef SILParser::parseProtocolConformanceHelper( ParserResult TyR = P.parseType(); if (TyR.isNull()) return ProtocolConformanceRef(); - TypeLoc Ty = TyR.get(); if (defaultForProto) { - bindProtocolSelfInTypeRepr(Ty, defaultForProto); + bindProtocolSelfInTypeRepr(TyR.get(), defaultForProto); } - if (performTypeLocChecking(Ty, /*IsSILType=*/ false, witnessEnv)) + auto ConformingTy = performTypeLocChecking(TyR.get(), /*IsSILType=*/ false, witnessEnv); + if (ConformingTy->hasError()) return ProtocolConformanceRef(); - auto ConformingTy = Ty.getType(); if (P.parseToken(tok::colon, diag::expected_sil_witness_colon)) return ProtocolConformanceRef(); @@ -6429,7 +6425,7 @@ static bool parseSILVTableEntry( return true; TypeLoc Ty = TyR.get(); if (isDefaultWitnessTable) - bindProtocolSelfInTypeRepr(Ty, proto); + bindProtocolSelfInTypeRepr(TyR.get(), proto); if (swift::performTypeLocChecking(P.Context, Ty, /*isSILMode=*/false, /*isSILType=*/false, @@ -6490,7 +6486,7 @@ static bool parseSILVTableEntry( return true; TypeLoc Ty = TyR.get(); if (isDefaultWitnessTable) - bindProtocolSelfInTypeRepr(Ty, proto); + bindProtocolSelfInTypeRepr(TyR.get(), proto); if (swift::performTypeLocChecking(P.Context, Ty, /*isSILMode=*/false, /*isSILType=*/false, From 8be08b16a8368680ed5781cfa3de869e629ee0f1 Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 16:03:43 -0700 Subject: [PATCH 25/33] [NFC] Remove Trivial Projections from TypeLocs in RequirementRepr --- lib/AST/ASTWalker.cpp | 6 +++--- lib/Sema/TypeCheckDeclPrimary.cpp | 2 +- lib/Sema/TypeCheckType.cpp | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/AST/ASTWalker.cpp b/lib/AST/ASTWalker.cpp index 57d4cda33ff27..e121f3528db75 100644 --- a/lib/AST/ASTWalker.cpp +++ b/lib/AST/ASTWalker.cpp @@ -1345,15 +1345,15 @@ class Traversal : public ASTVisitorisEqual(proto->getSelfInterfaceType())) { auto &diags = proto->getASTContext().Diags; - diags.diagnose(reqRepr->getSubjectLoc().getLoc(), + diags.diagnose(reqRepr->getSubjectRepr()->getLoc(), diag::protocol_where_clause_self_requirement); } diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index 3f368011f8aa1..9d142ebf69ad7 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -3804,9 +3804,9 @@ class UnsupportedProtocolVisitor void visitRequirements(ArrayRef reqts) { for (auto reqt : reqts) { if (reqt.getKind() == RequirementReprKind::SameType) { - if (auto *repr = reqt.getFirstTypeLoc().getTypeRepr()) + if (auto *repr = reqt.getFirstTypeRepr()) repr->walk(*this); - if (auto *repr = reqt.getSecondTypeLoc().getTypeRepr()) + if (auto *repr = reqt.getSecondTypeRepr()) repr->walk(*this); } } From a748d8ed7785d11ba16c15037533c2710e4de4fc Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 16:04:32 -0700 Subject: [PATCH 26/33] [NFC] Fixup RequirementRepr Printing To Use Only the Repr If a semantic representation of the RequirementRepr is needed, then it should be resolved to a Requirement and printed. --- include/swift/AST/Decl.h | 6 +++--- lib/AST/ASTDumper.cpp | 34 ++++++++++++++++++---------------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index 600042bf07563..2a7523cd69bc3 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -1044,16 +1044,16 @@ class RequirementRepr { StringRef AsWrittenString; RequirementRepr(SourceLoc SeparatorLoc, RequirementReprKind Kind, - TypeLoc FirstType, TypeLoc SecondType) + TypeRepr *FirstType, TypeRepr *SecondType) : SeparatorLoc(SeparatorLoc), Kind(Kind), Invalid(false), FirstType(FirstType), SecondType(SecondType) { } RequirementRepr(SourceLoc SeparatorLoc, RequirementReprKind Kind, - TypeLoc FirstType, LayoutConstraintLoc SecondLayout) + TypeRepr *FirstType, LayoutConstraintLoc SecondLayout) : SeparatorLoc(SeparatorLoc), Kind(Kind), Invalid(false), FirstType(FirstType), SecondLayout(SecondLayout) { } - void printImpl(ASTPrinter &OS, bool AsWritten) const; + void printImpl(ASTPrinter &OS) const; public: /// Construct a new type-constraint requirement. diff --git a/lib/AST/ASTDumper.cpp b/lib/AST/ASTDumper.cpp index 4aef9bc3905ca..a8c26677f43bc 100644 --- a/lib/AST/ASTDumper.cpp +++ b/lib/AST/ASTDumper.cpp @@ -121,15 +121,7 @@ void RequirementRepr::dump() const { llvm::errs() << "\n"; } -void RequirementRepr::printImpl(ASTPrinter &out, bool AsWritten) const { - auto printTy = [&](const TypeLoc &TyLoc) { - if (AsWritten && TyLoc.getTypeRepr()) { - TyLoc.getTypeRepr()->print(out, PrintOptions()); - } else { - TyLoc.getType().print(out, PrintOptions()); - } - }; - +void RequirementRepr::printImpl(ASTPrinter &out) const { auto printLayoutConstraint = [&](const LayoutConstraintLoc &LayoutConstraintLoc) { LayoutConstraintLoc.getLayoutConstraint()->print(out, PrintOptions()); @@ -137,31 +129,41 @@ void RequirementRepr::printImpl(ASTPrinter &out, bool AsWritten) const { switch (getKind()) { case RequirementReprKind::LayoutConstraint: - printTy(getSubjectLoc()); + if (auto *repr = getSubjectRepr()) { + repr->print(out, PrintOptions()); + } out << " : "; printLayoutConstraint(getLayoutConstraintLoc()); break; case RequirementReprKind::TypeConstraint: - printTy(getSubjectLoc()); + if (auto *repr = getSubjectRepr()) { + repr->print(out, PrintOptions()); + } out << " : "; - printTy(getConstraintLoc()); + if (auto *repr = getConstraintRepr()) { + repr->print(out, PrintOptions()); + } break; case RequirementReprKind::SameType: - printTy(getFirstTypeLoc()); + if (auto *repr = getFirstTypeRepr()) { + repr->print(out, PrintOptions()); + } out << " == "; - printTy(getSecondTypeLoc()); + if (auto *repr = getSecondTypeRepr()) { + repr->print(out, PrintOptions()); + } break; } } void RequirementRepr::print(raw_ostream &out) const { StreamPrinter printer(out); - printImpl(printer, /*AsWritten=*/true); + printImpl(printer); } void RequirementRepr::print(ASTPrinter &out) const { - printImpl(out, /*AsWritten=*/true); + printImpl(out); } static void printTrailingRequirements(ASTPrinter &Printer, From 10f53c07b5eb9277c494f8f42d6765bd204e26fd Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 16:06:02 -0700 Subject: [PATCH 27/33] [NFC] Refactor getAdopteeSelfSameTypeConstraint This method performs a linear-ish search over all the generic requirements and tries to pull out a matching written requirement so it can provide a richer diagnostic. Unpack this search, teach it to walk resolved requirements, and clean up the code here. --- lib/Sema/TypeCheckProtocol.cpp | 74 ++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 30 deletions(-) diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index b9cd51877744e..b674225c27244 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -3103,43 +3103,57 @@ diagnoseMissingWitnesses(MissingWitnessDiagnosisKind Kind) { /// /// \returns None if there is no such constraint; a non-empty optional that /// may have the \c RequirementRepr for the actual constraint. -static Optional +static Optional> getAdopteeSelfSameTypeConstraint(ClassDecl *selfClass, ValueDecl *witness) { auto genericSig = witness->getInnermostDeclContext()->getGenericSignatureOfContext(); if (!genericSig) return None; - for (const auto &req : genericSig->getRequirements()) { + // First, search for any bogus requirements. + auto it = llvm::find_if(genericSig->getRequirements(), + [&selfClass](const auto &req) { if (req.getKind() != RequirementKind::SameType) - continue; + return false; - if (req.getFirstType()->getAnyNominal() == selfClass || - req.getSecondType()->getAnyNominal() == selfClass) { - // Try to find the requirement-as-written. - GenericParamList *genericParams = nullptr; - - if (auto func = dyn_cast(witness)) - genericParams = func->getGenericParams(); - else if (auto subscript = dyn_cast(witness)) - genericParams = subscript->getGenericParams(); - if (genericParams) { - for (auto &req : genericParams->getRequirements()) { - if (req.getKind() != RequirementReprKind::SameType) - continue; + return req.getFirstType()->getAnyNominal() == selfClass + || req.getSecondType()->getAnyNominal() == selfClass; + }); + if (it == genericSig->getRequirements().end()) { + return None; + } - if (req.getFirstType()->getAnyNominal() == selfClass || - req.getSecondType()->getAnyNominal() == selfClass) - return &req; - } - } + // Got it! Now try to find the requirement-as-written. + GenericParamList *genericParams = nullptr; + if (auto func = dyn_cast(witness)) + genericParams = func->getGenericParams(); + else if (auto subscript = dyn_cast(witness)) + genericParams = subscript->getGenericParams(); - // Form an optional(nullptr) to indicate that we don't have the - // requirement itself. - return nullptr; - } + // A null repr indicates we don't have a valid location to diagnose. But + // at least we have a requirement we can signal is bogus. + Optional> target + = std::make_pair((RequirementRepr *)nullptr, Requirement(*it)); + if (!genericParams) { + return target; } - return None; + // Resolve and search for a written requirement to match our bogus one. + WhereClauseOwner(cast(witness), genericParams) + .visitRequirements(TypeResolutionStage::Structural, + [&](Requirement req, RequirementRepr *repr) { + if (req.getKind() != RequirementKind::SameType) { + return false; + } + + if (req.getFirstType()->getAnyNominal() != selfClass && + req.getSecondType()->getAnyNominal() != selfClass) { + return false; + } + + target.emplace(repr, req); + return true; + }); + return target; } void ConformanceChecker::checkNonFinalClassWitness(ValueDecl *requirement, @@ -3239,7 +3253,7 @@ void ConformanceChecker::checkNonFinalClassWitness(ValueDecl *requirement, }); } } else if (selfKind.requirement) { - if (auto constraint = getAdopteeSelfSameTypeConstraint(classDecl, + if (auto targetPair = getAdopteeSelfSameTypeConstraint(classDecl, witness)) { // A "Self ==" constraint works incorrectly with subclasses. Complain. auto proto = Conformance->getProtocol(); @@ -3254,11 +3268,11 @@ void ConformanceChecker::checkNonFinalClassWitness(ValueDecl *requirement, proto->getDeclaredType()); emitDeclaredHereIfNeeded(diags, diagLoc, witness); - if (auto requirementRepr = *constraint) { + if (auto requirementRepr = targetPair->first) { diags.diagnose(requirementRepr->getSeparatorLoc(), diag::witness_self_weaken_same_type, - requirementRepr->getFirstType(), - requirementRepr->getSecondType()) + targetPair->second.getFirstType(), + targetPair->second.getSecondType()) .fixItReplace(requirementRepr->getSeparatorLoc(), ":"); } } From 68d2d824b76b936cb488f4ea79d5f1c72031b8dc Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 16:07:27 -0700 Subject: [PATCH 28/33] [NFC] Teach BuiltinFunctionBuilder to Build Requirements There is no need to build a fake RequirementRepr if we can directly represent a the requested AnyObject conformance constraint in the generated generic signature. This removes the final user of the TypeLoc-bearing constructors of RequirementReprs. --- lib/AST/Builtins.cpp | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/lib/AST/Builtins.cpp b/lib/AST/Builtins.cpp index 32516e69f9a5b..8c3bf3e7ec5d2 100644 --- a/lib/AST/Builtins.cpp +++ b/lib/AST/Builtins.cpp @@ -431,26 +431,13 @@ createGenericParam(ASTContext &ctx, const char *name, unsigned index) { /// Create a generic parameter list with multiple generic parameters. static GenericParamList *getGenericParams(ASTContext &ctx, - unsigned numParameters, - bool isAnyObject) { + unsigned numParameters) { assert(numParameters <= llvm::array_lengthof(GenericParamNames)); - SmallVector genericParams; + SmallVector genericParams; for (unsigned i = 0; i != numParameters; ++i) genericParams.push_back(createGenericParam(ctx, GenericParamNames[i], i)); - - if (isAnyObject) { - CanType ao = ctx.getAnyObjectType(); - SmallVector req; - req.push_back(RequirementRepr::getTypeConstraint(TypeLoc::withoutLoc(genericParams[0]->getInterfaceType()), SourceLoc(), - TypeLoc::withoutLoc(ao))); - - auto paramList = GenericParamList::create(ctx, SourceLoc(), genericParams, - SourceLoc(), req, SourceLoc()); - return paramList; - } - auto paramList = GenericParamList::create(ctx, SourceLoc(), genericParams, SourceLoc()); return paramList; @@ -474,9 +461,15 @@ namespace { public: BuiltinFunctionBuilder(ASTContext &ctx, unsigned numGenericParams = 1, - bool isAnyObject = false) + bool wantsAdditionalAnyObjectRequirement = false) : Context(ctx) { - TheGenericParamList = getGenericParams(ctx, numGenericParams, isAnyObject); + TheGenericParamList = getGenericParams(ctx, numGenericParams); + if (wantsAdditionalAnyObjectRequirement) { + Requirement req(RequirementKind::Conformance, + TheGenericParamList->getParams()[0]->getInterfaceType(), + ctx.getAnyObjectType()); + addedRequirements.push_back(req); + } for (auto gp : TheGenericParamList->getParams()) { genericParamTypes.push_back( gp->getDeclaredInterfaceType()->castTo()); From b8d0cf342cf74213c4bbbe2eb89e22184ef26dad Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 16:08:34 -0700 Subject: [PATCH 29/33] [NFC] Only Resolve TypeReprs If Given a RequirementRepr Resolve the written type instead of the semantic type since that's the only data these requests will have access to once RequirementRepr is made completely syntactic. --- lib/AST/NameLookup.cpp | 28 +++++++++------------------- lib/Sema/TypeCheckGeneric.cpp | 20 +++++--------------- 2 files changed, 14 insertions(+), 34 deletions(-) diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp index 36311b7f85644..156c511cb590e 100644 --- a/lib/AST/NameLookup.cpp +++ b/lib/AST/NameLookup.cpp @@ -857,8 +857,6 @@ SelfBoundsFromWhereClauseRequest::evaluate( if (auto identTypeRepr = dyn_cast(typeRepr)) isSelfLHS = (identTypeRepr->getNameRef().getBaseIdentifier() == ctx.Id_Self); - } else if (Type type = req.getSubject()) { - isSelfLHS = type->isEqual(dc->getSelfInterfaceType()); } if (!isSelfLHS) continue; @@ -867,8 +865,6 @@ SelfBoundsFromWhereClauseRequest::evaluate( DirectlyReferencedTypeDecls rhsDecls; if (auto typeRepr = req.getConstraintRepr()) { rhsDecls = directReferencesForTypeRepr(evaluator, ctx, typeRepr, lookupDC); - } else if (Type type = req.getConstraint()) { - rhsDecls = directReferencesForType(type); } SmallVector modulesFound; @@ -899,30 +895,24 @@ TypeDeclsFromWhereClauseRequest::evaluate(Evaluator &evaluator, ASTContext &ctx = ext->getASTContext(); TinyPtrVector result; + auto resolve = [&](TypeRepr *typeRepr) { + auto decls = directReferencesForTypeRepr(evaluator, ctx, typeRepr, ext); + result.insert(result.end(), decls.begin(), decls.end()); + }; for (const auto &req : ext->getGenericParams()->getTrailingRequirements()) { - auto resolve = [&](TypeLoc loc) { - DirectlyReferencedTypeDecls decls; - if (auto *typeRepr = loc.getTypeRepr()) - decls = directReferencesForTypeRepr(evaluator, ctx, typeRepr, ext); - else if (Type type = loc.getType()) - decls = directReferencesForType(type); - - result.insert(result.end(), decls.begin(), decls.end()); - }; - switch (req.getKind()) { case RequirementReprKind::TypeConstraint: - resolve(req.getSubjectLoc()); - resolve(req.getConstraintLoc()); + resolve(req.getSubjectRepr()); + resolve(req.getConstraintRepr()); break; case RequirementReprKind::SameType: - resolve(req.getFirstTypeLoc()); - resolve(req.getSecondTypeLoc()); + resolve(req.getFirstTypeRepr()); + resolve(req.getSecondTypeRepr()); break; case RequirementReprKind::LayoutConstraint: - resolve(req.getSubjectLoc()); + resolve(req.getSubjectRepr()); break; } } diff --git a/lib/Sema/TypeCheckGeneric.cpp b/lib/Sema/TypeCheckGeneric.cpp index ee60881ea9634..c18cc16e3d965 100644 --- a/lib/Sema/TypeCheckGeneric.cpp +++ b/lib/Sema/TypeCheckGeneric.cpp @@ -931,21 +931,11 @@ RequirementRequest::evaluate(Evaluator &evaluator, llvm_unreachable("No clients care about this. Use mapTypeIntoContext()"); } - auto resolveType = [&](TypeLoc &typeLoc) -> Type { - Type result; - if (auto typeRepr = typeLoc.getTypeRepr()) - result = resolution->resolveType(typeRepr); - else - result = typeLoc.getType(); - - return result ? result : ErrorType::get(owner.dc->getASTContext()); - }; - auto &reqRepr = getRequirement(); switch (reqRepr.getKind()) { case RequirementReprKind::TypeConstraint: { - Type subject = resolveType(reqRepr.getSubjectLoc()); - Type constraint = resolveType(reqRepr.getConstraintLoc()); + Type subject = resolution->resolveType(reqRepr.getSubjectRepr()); + Type constraint = resolution->resolveType(reqRepr.getConstraintRepr()); return Requirement(constraint->getClassOrBoundGenericClass() ? RequirementKind::Superclass : RequirementKind::Conformance, @@ -954,12 +944,12 @@ RequirementRequest::evaluate(Evaluator &evaluator, case RequirementReprKind::SameType: return Requirement(RequirementKind::SameType, - resolveType(reqRepr.getFirstTypeLoc()), - resolveType(reqRepr.getSecondTypeLoc())); + resolution->resolveType(reqRepr.getFirstTypeRepr()), + resolution->resolveType(reqRepr.getSecondTypeRepr())); case RequirementReprKind::LayoutConstraint: return Requirement(RequirementKind::Layout, - resolveType(reqRepr.getSubjectLoc()), + resolution->resolveType(reqRepr.getSubjectRepr()), reqRepr.getLayoutConstraint()); } llvm_unreachable("unhandled kind"); From d197f9d1abf7f6ab88b157d36cc38fe806b86d44 Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 16:09:32 -0700 Subject: [PATCH 30/33] [NFC] Make RequirementRequest Cached In preparation for the removal of the TypeLocs here, cache down the resulting Requirement from a RequirementRepr. --- include/swift/AST/TypeCheckRequests.h | 6 +-- include/swift/AST/TypeCheckerTypeIDZone.def | 2 +- lib/AST/TypeCheckRequests.cpp | 54 --------------------- 3 files changed, 3 insertions(+), 59 deletions(-) diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h index 75392765ce473..46d6b5ae0810a 100644 --- a/include/swift/AST/TypeCheckRequests.h +++ b/include/swift/AST/TypeCheckRequests.h @@ -441,7 +441,7 @@ class RequirementRequest : public SimpleRequest { + RequestFlags::Cached> { public: using SimpleRequest::SimpleRequest; @@ -464,10 +464,8 @@ class RequirementRequest : // Cycle handling. void noteCycleStep(DiagnosticEngine &diags) const; - // Separate caching. + // Caching. bool isCached() const; - Optional getCachedResult() const; - void cacheResult(Requirement value) const; }; /// Generate the USR for the given declaration. diff --git a/include/swift/AST/TypeCheckerTypeIDZone.def b/include/swift/AST/TypeCheckerTypeIDZone.def index 4df761af7c2d2..2acdda4869312 100644 --- a/include/swift/AST/TypeCheckerTypeIDZone.def +++ b/include/swift/AST/TypeCheckerTypeIDZone.def @@ -176,7 +176,7 @@ SWIFT_REQUEST(TypeChecker, ProtocolRequiresClassRequest, bool(ProtocolDecl *), SeparatelyCached, NoLocationInfo) SWIFT_REQUEST(TypeChecker, RequirementRequest, Requirement(WhereClauseOwner, unsigned, TypeResolutionStage), - SeparatelyCached, HasNearestLocation) + Cached, HasNearestLocation) SWIFT_REQUEST(TypeChecker, RequirementSignatureRequest, ArrayRef(ProtocolDecl *), SeparatelyCached, NoLocationInfo) diff --git a/lib/AST/TypeCheckRequests.cpp b/lib/AST/TypeCheckRequests.cpp index 6ecd7c87e2b50..41276bd6d1956 100644 --- a/lib/AST/TypeCheckRequests.cpp +++ b/lib/AST/TypeCheckRequests.cpp @@ -459,60 +459,6 @@ bool RequirementRequest::isCached() const { return std::get<2>(getStorage()) == TypeResolutionStage::Interface; } -Optional RequirementRequest::getCachedResult() const { - auto &reqRepr = getRequirement(); - switch (reqRepr.getKind()) { - case RequirementReprKind::TypeConstraint: - if (!reqRepr.getSubjectLoc().wasValidated() || - !reqRepr.getConstraintLoc().wasValidated()) - return None; - - return Requirement(reqRepr.getConstraint()->getClassOrBoundGenericClass() - ? RequirementKind::Superclass - : RequirementKind::Conformance, - reqRepr.getSubject(), - reqRepr.getConstraint()); - - case RequirementReprKind::SameType: - if (!reqRepr.getFirstTypeLoc().wasValidated() || - !reqRepr.getSecondTypeLoc().wasValidated()) - return None; - - return Requirement(RequirementKind::SameType, reqRepr.getFirstType(), - reqRepr.getSecondType()); - - case RequirementReprKind::LayoutConstraint: - if (!reqRepr.getSubjectLoc().wasValidated()) - return None; - - return Requirement(RequirementKind::Layout, reqRepr.getSubject(), - reqRepr.getLayoutConstraint()); - } - llvm_unreachable("unhandled kind"); -} - -void RequirementRequest::cacheResult(Requirement value) const { - auto &reqRepr = getRequirement(); - switch (value.getKind()) { - case RequirementKind::Conformance: - case RequirementKind::Superclass: - reqRepr.getSubjectLoc().setType(value.getFirstType()); - reqRepr.getConstraintLoc().setType(value.getSecondType()); - break; - - case RequirementKind::SameType: - reqRepr.getFirstTypeLoc().setType(value.getFirstType()); - reqRepr.getSecondTypeLoc().setType(value.getSecondType()); - break; - - case RequirementKind::Layout: - reqRepr.getSubjectLoc().setType(value.getFirstType()); - reqRepr.getLayoutConstraintLoc() - .setLayoutConstraint(value.getLayoutConstraint()); - break; - } -} - //----------------------------------------------------------------------------// // DefaultTypeRequest. //----------------------------------------------------------------------------// From b9427b0a88c1a0a985c7020965be2e31ed153be7 Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Thu, 11 Jun 2020 16:17:30 -0700 Subject: [PATCH 31/33] [NFC] Strip RequirementRepr of its TypeLocs Now that the previous commits have removed the data dependencies on TypeLoc, remove the TypeLoc and replace it with a TypeRepr. This makes RequirementRepr a purely syntactic object once again. --- include/swift/AST/Decl.h | 89 +++++----------------------------------- lib/AST/Decl.cpp | 9 ++++ 2 files changed, 19 insertions(+), 79 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index 2a7523cd69bc3..8b372835d08b1 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -1030,12 +1030,12 @@ class RequirementRepr { SourceLoc SeparatorLoc; RequirementReprKind Kind : 2; bool Invalid : 1; - TypeLoc FirstType; + TypeRepr *FirstType; /// The second element represents the right-hand side of the constraint. /// It can be e.g. a type or a layout constraint. union { - TypeLoc SecondType; + TypeRepr *SecondType; LayoutConstraintLoc SecondLayout; }; @@ -1064,9 +1064,9 @@ class RequirementRepr { /// this requirement was implied. /// \param Constraint The protocol or protocol composition to which the /// subject must conform, or superclass from which the subject must inherit. - static RequirementRepr getTypeConstraint(TypeLoc Subject, + static RequirementRepr getTypeConstraint(TypeRepr *Subject, SourceLoc ColonLoc, - TypeLoc Constraint) { + TypeRepr *Constraint) { return { ColonLoc, RequirementReprKind::TypeConstraint, Subject, Constraint }; } @@ -1076,9 +1076,9 @@ class RequirementRepr { /// \param EqualLoc The location of the '==' in the same-type constraint, or /// an invalid location if this requirement was implied. /// \param SecondType The second type. - static RequirementRepr getSameType(TypeLoc FirstType, + static RequirementRepr getSameType(TypeRepr *FirstType, SourceLoc EqualLoc, - TypeLoc SecondType) { + TypeRepr *SecondType) { return { EqualLoc, RequirementReprKind::SameType, FirstType, SecondType }; } @@ -1090,7 +1090,7 @@ class RequirementRepr { /// this requirement was implied. /// \param Layout The layout requirement to which the /// subject must conform. - static RequirementRepr getLayoutConstraint(TypeLoc Subject, + static RequirementRepr getLayoutConstraint(TypeRepr *Subject, SourceLoc ColonLoc, LayoutConstraintLoc Layout) { return {ColonLoc, RequirementReprKind::LayoutConstraint, Subject, @@ -1108,25 +1108,7 @@ class RequirementRepr { /// For a type-bound requirement, return the subject of the /// conformance relationship. - Type getSubject() const { - assert(getKind() == RequirementReprKind::TypeConstraint || - getKind() == RequirementReprKind::LayoutConstraint); - return FirstType.getType(); - } - TypeRepr *getSubjectRepr() const { - assert(getKind() == RequirementReprKind::TypeConstraint || - getKind() == RequirementReprKind::LayoutConstraint); - return FirstType.getTypeRepr(); - } - - TypeLoc &getSubjectLoc() { - assert(getKind() == RequirementReprKind::TypeConstraint || - getKind() == RequirementReprKind::LayoutConstraint); - return FirstType; - } - - const TypeLoc &getSubjectLoc() const { assert(getKind() == RequirementReprKind::TypeConstraint || getKind() == RequirementReprKind::LayoutConstraint); return FirstType; @@ -1134,22 +1116,7 @@ class RequirementRepr { /// For a type-bound requirement, return the protocol or to which /// the subject conforms or superclass it inherits. - Type getConstraint() const { - assert(getKind() == RequirementReprKind::TypeConstraint); - return SecondType.getType(); - } - TypeRepr *getConstraintRepr() const { - assert(getKind() == RequirementReprKind::TypeConstraint); - return SecondType.getTypeRepr(); - } - - TypeLoc &getConstraintLoc() { - assert(getKind() == RequirementReprKind::TypeConstraint); - return SecondType; - } - - const TypeLoc &getConstraintLoc() const { assert(getKind() == RequirementReprKind::TypeConstraint); return SecondType; } @@ -1170,43 +1137,13 @@ class RequirementRepr { } /// Retrieve the first type of a same-type requirement. - Type getFirstType() const { - assert(getKind() == RequirementReprKind::SameType); - return FirstType.getType(); - } - TypeRepr *getFirstTypeRepr() const { - assert(getKind() == RequirementReprKind::SameType); - return FirstType.getTypeRepr(); - } - - TypeLoc &getFirstTypeLoc() { - assert(getKind() == RequirementReprKind::SameType); - return FirstType; - } - - const TypeLoc &getFirstTypeLoc() const { assert(getKind() == RequirementReprKind::SameType); return FirstType; } /// Retrieve the second type of a same-type requirement. - Type getSecondType() const { - assert(getKind() == RequirementReprKind::SameType); - return SecondType.getType(); - } - TypeRepr *getSecondTypeRepr() const { - assert(getKind() == RequirementReprKind::SameType); - return SecondType.getTypeRepr(); - } - - TypeLoc &getSecondTypeLoc() { - assert(getKind() == RequirementReprKind::SameType); - return SecondType; - } - - const TypeLoc &getSecondTypeLoc() const { assert(getKind() == RequirementReprKind::SameType); return SecondType; } @@ -1217,19 +1154,13 @@ class RequirementRepr { return SeparatorLoc; } - SourceRange getSourceRange() const { - if (getKind() == RequirementReprKind::LayoutConstraint) - return SourceRange(FirstType.getSourceRange().Start, - SecondLayout.getSourceRange().End); - return SourceRange(FirstType.getSourceRange().Start, - SecondType.getSourceRange().End); - } + SourceRange getSourceRange() const; /// Retrieve the first or subject type representation from the \c repr, /// or \c nullptr if \c repr is null. static TypeRepr *getFirstTypeRepr(const RequirementRepr *repr) { if (!repr) return nullptr; - return repr->FirstType.getTypeRepr(); + return repr->FirstType; } /// Retrieve the second or constraint type representation from the \c repr, @@ -1238,7 +1169,7 @@ class RequirementRepr { if (!repr) return nullptr; assert(repr->getKind() == RequirementReprKind::TypeConstraint || repr->getKind() == RequirementReprKind::SameType); - return repr->SecondType.getTypeRepr(); + return repr->SecondType; } SWIFT_DEBUG_DUMP; diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 81706cee2a48b..53bede44749f5 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -875,6 +875,15 @@ bool Decl::isWeakImported(ModuleDecl *fromModule) const { return !fromContext.isContainedIn(containingContext); } + +SourceRange RequirementRepr::getSourceRange() const { + if (getKind() == RequirementReprKind::LayoutConstraint) + return SourceRange(FirstType->getSourceRange().Start, + SecondLayout.getSourceRange().End); + return SourceRange(FirstType->getSourceRange().Start, + SecondType->getSourceRange().End); +} + GenericParamList::GenericParamList(SourceLoc LAngleLoc, ArrayRef Params, SourceLoc WhereLoc, From 9cc07c7a4a40acdc81809a5e0bfb765c61fbca88 Mon Sep 17 00:00:00 2001 From: 3405691582 Date: Sun, 10 May 2020 18:20:44 -0400 Subject: [PATCH 32/33] [test][stdlib] Define stdio stubs for OpenBSD. These are macros on OpenBSD, which don't get imported to an equivalent symbol in Swift. --- stdlib/public/Platform/Platform.swift | 4 ++++ stdlib/public/SwiftShims/LibcOverlayShims.h | 17 +++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/stdlib/public/Platform/Platform.swift b/stdlib/public/Platform/Platform.swift index 51ec96a046ef2..5243b516379f9 100644 --- a/stdlib/public/Platform/Platform.swift +++ b/stdlib/public/Platform/Platform.swift @@ -142,6 +142,10 @@ public func snprintf(ptr: UnsafeMutablePointer, _ len: Int, _ format: Unsa return vsnprintf(ptr, len, format, va_args) } } +#elseif os(OpenBSD) +public var stdin: UnsafeMutablePointer { return _swift_stdlib_stdin() } +public var stdout: UnsafeMutablePointer { return _swift_stdlib_stdout() } +public var stderr: UnsafeMutablePointer { return _swift_stdlib_stderr() } #elseif os(Windows) public var stdin: UnsafeMutablePointer { return __acrt_iob_func(0) } public var stdout: UnsafeMutablePointer { return __acrt_iob_func(1) } diff --git a/stdlib/public/SwiftShims/LibcOverlayShims.h b/stdlib/public/SwiftShims/LibcOverlayShims.h index a4ec14402a4be..b4c2ba1d0a002 100644 --- a/stdlib/public/SwiftShims/LibcOverlayShims.h +++ b/stdlib/public/SwiftShims/LibcOverlayShims.h @@ -20,6 +20,9 @@ #include "Visibility.h" +#if defined(__OpenBSD__) +#include +#endif #if defined(_WIN32) && !defined(__CYGWIN__) #include #include @@ -112,6 +115,20 @@ int static inline _swift_stdlib_openat(int fd, const char *path, int oflag, } #endif +#if defined(__OpenBSD__) +static inline FILE *_swift_stdlib_stdin(void) { + return stdin; +} + +static inline FILE *_swift_stdlib_stdout(void) { + return stdout; +} + +static inline FILE *_swift_stdlib_stderr(void) { + return stderr; +} +#endif + #if __has_feature(nullability) #pragma clang assume_nonnull end #endif From ee1d76ed44308bb02b52851a9320299ed283a332 Mon Sep 17 00:00:00 2001 From: Zoe Carver Date: Thu, 11 Jun 2020 23:10:19 -0700 Subject: [PATCH 33/33] [opt] Re-work broadenSingleElementStores to use projections. (#32318) * Fixes loadable edge case for address-only types. * Re-work, generalize, and simplify by using projections. * Support `-enable-cxx-interop` in sil-opt. --- include/swift/SIL/Projection.h | 2 +- lib/SIL/Utils/Projection.cpp | 7 +- .../Utils/CanonicalizeInstruction.cpp | 77 +++++++++---------- test/SILOptimizer/Inputs/foo.h | 18 +++++ test/SILOptimizer/Inputs/module.modulemap | 3 + .../dont_broaden_cxx_address_only.sil | 44 +++++++++++ tools/sil-opt/SILOpt.cpp | 7 ++ 7 files changed, 113 insertions(+), 45 deletions(-) create mode 100644 test/SILOptimizer/Inputs/foo.h create mode 100644 test/SILOptimizer/Inputs/module.modulemap create mode 100644 test/SILOptimizer/dont_broaden_cxx_address_only.sil diff --git a/include/swift/SIL/Projection.h b/include/swift/SIL/Projection.h index 16c9c192e0236..d02089ae9380b 100644 --- a/include/swift/SIL/Projection.h +++ b/include/swift/SIL/Projection.h @@ -462,7 +462,7 @@ class Projection { static NullablePtr createAggFromFirstLevelProjections(SILBuilder &B, SILLocation Loc, SILType BaseType, - llvm::SmallVectorImpl &Values); + ArrayRef Values); void print(raw_ostream &os, SILType baseType) const; private: diff --git a/lib/SIL/Utils/Projection.cpp b/lib/SIL/Utils/Projection.cpp index c54ab49ec5b0f..a190c17f31213 100644 --- a/lib/SIL/Utils/Projection.cpp +++ b/lib/SIL/Utils/Projection.cpp @@ -803,10 +803,9 @@ Projection::operator<(const Projection &Other) const { } NullablePtr -Projection:: -createAggFromFirstLevelProjections(SILBuilder &B, SILLocation Loc, - SILType BaseType, - llvm::SmallVectorImpl &Values) { +Projection::createAggFromFirstLevelProjections( + SILBuilder &B, SILLocation Loc, SILType BaseType, + ArrayRef Values) { if (BaseType.getStructOrBoundGenericStruct()) { return B.createStruct(Loc, BaseType, Values); } diff --git a/lib/SILOptimizer/Utils/CanonicalizeInstruction.cpp b/lib/SILOptimizer/Utils/CanonicalizeInstruction.cpp index e6122d2bd5de9..87d8b9c68cff0 100644 --- a/lib/SILOptimizer/Utils/CanonicalizeInstruction.cpp +++ b/lib/SILOptimizer/Utils/CanonicalizeInstruction.cpp @@ -315,6 +315,11 @@ splitAggregateLoad(LoadInst *loadInst, CanonicalizeInstruction &pass) { // (store (struct_element_addr %base) object) // -> // (store %base (struct object)) +// +// TODO: supporting enums here would be very easy. The main thing is adding +// support in `createAggFromFirstLevelProjections`. +// Note: we will not be able to support tuples because we cannot have a +// single-element tuple. static SILBasicBlock::iterator broadenSingleElementStores(StoreInst *storeInst, CanonicalizeInstruction &pass) { @@ -322,29 +327,14 @@ broadenSingleElementStores(StoreInst *storeInst, // instructions. This must be valid regardless of whether the pass immediately // deletes the instructions or simply records them for later deletion. auto nextII = std::next(storeInst->getIterator()); - - // Bail if the store's destination is not a struct_element_addr. - auto *sea = dyn_cast(storeInst->getDest()); - if (!sea) - return nextII; - auto *f = storeInst->getFunction(); - // Continue up the struct_element_addr chain, as long as each struct only has - // a single property, creating StoreInsts along the way. - SILBuilderWithScope builder(storeInst); - - SILValue result = storeInst->getSrc(); - SILValue baseAddr = sea->getOperand(); - SILValue storeAddr; - while (true) { + ProjectionPath projections(storeInst->getDest()->getType()); + SILValue op = storeInst->getDest(); + while (isa(op)) { + auto *inst = cast(op); + SILValue baseAddr = inst->getOperand(0); SILType baseAddrType = baseAddr->getType(); - - // If our aggregate has unreferenced storage then we can never prove if it - // actually has a single field. - if (baseAddrType.aggregateHasUnreferenceableStorage()) - break; - auto *decl = baseAddrType.getStructOrBoundGenericStruct(); assert( !decl->isResilient(f->getModule().getSwiftModule(), @@ -352,31 +342,38 @@ broadenSingleElementStores(StoreInst *storeInst, "This code assumes resilient structs can not have fragile fields. If " "this assert is hit, this has been changed. Please update this code."); - // NOTE: If this is ever changed to support enums, we must check for address - // only types here. For structs we do not have to check since a single - // element struct with a loadable element can never be address only. We - // additionally do not have to worry about our input value being address - // only since we are storing into it. - if (decl->getStoredProperties().size() != 1) + // Bail if the store's destination is not a struct_element_addr or if the + // store's destination (%base) is not a loadable type. If %base is not a + // loadable type, we can't create it as a struct later on. + // If our aggregate has unreferenced storage then we can never prove if it + // actually has a single field. + if (!baseAddrType.isLoadable(*f) || + baseAddrType.aggregateHasUnreferenceableStorage() || + decl->getStoredProperties().size() != 1) break; - // Update the store location now that we know it is safe. - storeAddr = baseAddr; + projections.push_back(Projection(inst)); + op = baseAddr; + } - // Otherwise, create the struct. - result = builder.createStruct(storeInst->getLoc(), - baseAddrType.getObjectType(), result); + // If we couldn't create a projection, bail. + if (projections.empty()) + return nextII; - // See if we have another struct_element_addr we can strip off. If we don't - // then this as much as we can promote. - sea = dyn_cast(sea->getOperand()); - if (!sea) - break; - baseAddr = sea->getOperand(); + // Now work our way back up. At this point we know all operations we are going + // to do succeed (cast, createAggFromFirstLevelProjections, + // etc.) so we can omit null checks. We should not bail at this point (we + // could create a double consume, or worse). + SILBuilderWithScope builder(storeInst); + SILValue result = storeInst->getSrc(); + SILValue storeAddr = storeInst->getDest(); + for (Projection proj : llvm::reverse(projections)) { + storeAddr = cast(storeAddr)->getOperand(0); + result = proj.createAggFromFirstLevelProjections( + builder, storeInst->getLoc(), + storeAddr->getType().getObjectType(), {result}) + .get(); } - // If we failed to create any structs, bail. - if (result == storeInst->getSrc()) - return nextII; // Store the new struct-wrapped value into the final base address. builder.createStore(storeInst->getLoc(), result, storeAddr, diff --git a/test/SILOptimizer/Inputs/foo.h b/test/SILOptimizer/Inputs/foo.h new file mode 100644 index 0000000000000..7faf6b5576515 --- /dev/null +++ b/test/SILOptimizer/Inputs/foo.h @@ -0,0 +1,18 @@ +#ifndef VALIDATION_TEST_SILOPTIMIZER_FOO_H +#define VALIDATION_TEST_SILOPTIMIZER_FOO_H + +struct Foo { + int x; + ~Foo() {} +}; + +struct Loadable { + int x; +}; + +struct Bar { + Loadable y; + ~Bar() {} +}; + +#endif // VALIDATION_TEST_SILOPTIMIZER_FOO_H diff --git a/test/SILOptimizer/Inputs/module.modulemap b/test/SILOptimizer/Inputs/module.modulemap new file mode 100644 index 0000000000000..6522d75170250 --- /dev/null +++ b/test/SILOptimizer/Inputs/module.modulemap @@ -0,0 +1,3 @@ +module Foo { + header "foo.h" +} diff --git a/test/SILOptimizer/dont_broaden_cxx_address_only.sil b/test/SILOptimizer/dont_broaden_cxx_address_only.sil new file mode 100644 index 0000000000000..369179ae38902 --- /dev/null +++ b/test/SILOptimizer/dont_broaden_cxx_address_only.sil @@ -0,0 +1,44 @@ +// RUN: %target-sil-opt -silgen-cleanup %s -I %S/Inputs -enable-sil-verify-all -enable-cxx-interop | %FileCheck %s + +sil_stage canonical + +import Builtin +import Swift +import SwiftShims +import Foo + +// Make sure we don't try to create a struct here. Foo is not loadable, even +// though it's only property is. +// CHECK-LABEL: @test_foo +// CHECK: bb0 +// CHECK-NEXT: [[E:%.*]] = struct_element_addr +// CHECK-NEXT: store %1 to [trivial] [[E]] +// CHECK-NEXT: tuple +// CHECK-NEXT: return +// CHECK-LABEL: end sil function 'test_foo' +sil shared [transparent] [serializable] [ossa] @test_foo : $@convention(method) (Int32, @thin Foo.Type) -> @out Foo { +bb0(%0 : $*Foo, %1 : $Int32, %2 : $@thin Foo.Type): + %3 = struct_element_addr %0 : $*Foo, #Foo.x + store %1 to [trivial] %3 : $*Int32 + %5 = tuple () + return %5 : $() +} + +// Make sure we create a struct for the first (loadable) type but not the second +// type (Bar). +// CHECK-LABEL: @test_bar +// CHECK: bb0 +// CHECK-NEXT: [[E:%.*]] = struct_element_addr +// CHECK-NEXT: [[AGG:%.*]] = struct $Loadable (%1 : $Int32) +// CHECK-NEXT: store [[AGG]] to [trivial] [[E]] +// CHECK-NEXT: tuple +// CHECK-NEXT: return +// CHECK-LABEL: end sil function 'test_bar' +sil shared [transparent] [serializable] [ossa] @test_bar : $@convention(method) (Int32, @thin Bar.Type) -> @out Bar { +bb0(%0 : $*Bar, %1 : $Int32, %2 : $@thin Bar.Type): + %3 = struct_element_addr %0 : $*Bar, #Bar.y + %3a = struct_element_addr %3 : $*Loadable, #Loadable.x + store %1 to [trivial] %3a : $*Int32 + %5 = tuple () + return %5 : $() +} diff --git a/tools/sil-opt/SILOpt.cpp b/tools/sil-opt/SILOpt.cpp index a47bf656b43b1..edee557560ee7 100644 --- a/tools/sil-opt/SILOpt.cpp +++ b/tools/sil-opt/SILOpt.cpp @@ -270,6 +270,11 @@ static cl::opt RemarksFormat( cl::desc("The format used for serializing remarks (default: YAML)"), cl::value_desc("format"), cl::init("yaml")); +static llvm::cl::opt + EnableCxxInterop("enable-cxx-interop", + llvm::cl::desc("Enable C++ interop."), + llvm::cl::init(false)); + static void runCommandLineSelectedPasses(SILModule *Module, irgen::IRGenModule *IRGenMod) { auto &opts = Module->getOptions(); @@ -348,6 +353,8 @@ int main(int argc, char **argv) { Invocation.getLangOptions().EnableExperimentalDifferentiableProgramming = EnableExperimentalDifferentiableProgramming; + Invocation.getLangOptions().EnableCXXInterop = EnableCxxInterop; + Invocation.getDiagnosticOptions().VerifyMode = VerifyMode ? DiagnosticOptions::Verify : DiagnosticOptions::NoVerify;