Merge pull request swiftlang#28697 from ravikandhadai/oslog-generic-operator-bug-fix

[Constant Evaluator] Fix a bug in the composition of substitution maps

Author: ravikandhadai

include/swift/SIL/SILConstants.h

@@ -19,6 +19,7 @@
 #define SWIFT_SIL_CONSTANTS_H
 
 #include "swift/AST/SubstitutionMap.h"
+#include "swift/SIL/SILInstruction.h"
 #include "swift/SIL/SILValue.h"
 #include "llvm/Support/CommandLine.h"
 
@@ -572,7 +573,7 @@ class SymbolicValue {
   static SymbolicValue makeClosure(
       SILFunction *target,
       ArrayRef<std::pair<SILValue, Optional<SymbolicValue>>> capturedArguments,
-      SubstitutionMap substMap, SILType closureType,
+      SubstitutionMap substMap, SingleValueInstruction *closureInst,
       SymbolicValueAllocator &allocator);
 
   SymbolicClosure *getClosure() const {
@@ -689,21 +690,25 @@ struct SymbolicClosure final
   // applied function to the generic arguments of passed to the call.
   SubstitutionMap substitutionMap;
 
-  SILType closureType;
+  // The closure instruction such as partial apply that resulted in this
+  // symbolic value. This is tracked to obtain SILType and other SIL-level
+  // information of the symbolic closure.
+  SingleValueInstruction *closureInst;
 
   SymbolicClosure() = delete;
   SymbolicClosure(const SymbolicClosure &) = delete;
   SymbolicClosure(SILFunction *callee, unsigned numArguments,
-                  SubstitutionMap substMap, SILType closureType,
+                  SubstitutionMap substMap, SingleValueInstruction *inst,
                   bool nonConstantCaptures)
       : target(callee), numCaptures(numArguments),
         hasNonConstantCaptures(nonConstantCaptures), substitutionMap(substMap),
-        closureType(closureType) {}
+        closureInst(inst) {}
 
 public:
   static SymbolicClosure *create(SILFunction *callee,
                                  ArrayRef<SymbolicClosureArgument> args,
-                                 SubstitutionMap substMap, SILType closureType,
+                                 SubstitutionMap substMap,
+                                 SingleValueInstruction *closureInst,
                                  SymbolicValueAllocator &allocator);
 
   ArrayRef<SymbolicClosureArgument> getCaptures() const {
@@ -719,7 +724,9 @@ struct SymbolicClosure final
     return target;
   }
 
-  SILType getClosureType() { return closureType; }
+  SingleValueInstruction *getClosureInst() { return closureInst; }
+
+  SILType getClosureType() { return closureInst->getType(); }
 
   SubstitutionMap getCallSubstitutionMap() { return substitutionMap; }
 };

lib/SIL/SILConstants.cpp

@@ -249,7 +249,7 @@ SymbolicValue::cloneInto(SymbolicValueAllocator &allocator) const {
     ArrayRef<SymbolicClosureArgument> closureArgs = clo->getCaptures();
     return SymbolicValue::makeClosure(clo->getTarget(), closureArgs,
                                       clo->getCallSubstitutionMap(),
-                                      clo->getClosureType(), allocator);
+                                      clo->getClosureInst(), allocator);
   }
   }
   llvm_unreachable("covered switch");
@@ -747,10 +747,10 @@ Type SymbolicValue::getArrayType() const {
 SymbolicValue SymbolicValue::makeClosure(SILFunction *target,
                                          ArrayRef<SymbolicClosureArgument> args,
                                          SubstitutionMap substMap,
-                                         SILType closureType,
+                                         SingleValueInstruction *closureInst,
                                          SymbolicValueAllocator &allocator) {
   auto clo =
-      SymbolicClosure::create(target, args, substMap, closureType, allocator);
+      SymbolicClosure::create(target, args, substMap, closureInst, allocator);
   SymbolicValue result;
   result.representationKind = RK_Closure;
   result.value.closure = clo;
@@ -760,7 +760,7 @@ SymbolicValue SymbolicValue::makeClosure(SILFunction *target,
 SymbolicClosure *SymbolicClosure::create(SILFunction *target,
                                          ArrayRef<SymbolicClosureArgument> args,
                                          SubstitutionMap substMap,
-                                         SILType closureType,
+                                         SingleValueInstruction *closureInst,
                                          SymbolicValueAllocator &allocator) {
   // Determine whether there are captured arguments without a symbolic value.
   bool hasNonConstantCapture = false;
@@ -776,7 +776,7 @@ SymbolicClosure *SymbolicClosure::create(SILFunction *target,
   auto rawMem = allocator.allocate(byteSizeOfArgs, alignof(SymbolicClosure));
   //  Placement initialize the object.
   auto closure = ::new (rawMem) SymbolicClosure(
-      target, args.size(), substMap, closureType, hasNonConstantCapture);
+      target, args.size(), substMap, closureInst, hasNonConstantCapture);
   std::uninitialized_copy(
       args.begin(), args.end(),
       closure->getTrailingObjects<SymbolicClosureArgument>());

lib/SILOptimizer/Utils/ConstExpr.cpp

@@ -1147,8 +1147,10 @@ ConstExprFunctionState::computeCallResult(ApplyInst *apply) {
           calleeFnType->getWitnessMethodConformanceOrInvalid().getRequirement();
       // Compute a mapping that maps the Self type of the protocol given by
       // 'requirement' to the concrete type available in the substitutionMap.
-      auto protoSelfToConcreteType =
-          apply->getSubstitutionMap().subst(substitutionMap);
+      SubstitutionMap applySubstMap = apply->getSubstitutionMap();
+      auto protoSelfToConcreteType = substitutionMap.empty()
+                                         ? applySubstMap
+                                         : applySubstMap.subst(substitutionMap);
       // Get a concrete protocol conformance by using the mapping for the
       // Self type of the requirement.
       auto conf = protoSelfToConcreteType.lookupConformance(
@@ -1174,7 +1176,8 @@ ConstExprFunctionState::computeCallResult(ApplyInst *apply) {
     // or conformance, with the mapping introduced by the call itself.  This
     // ensures that the callee's substitution map can map from its type
     // namespace back to concrete types and conformances.
-    calleeSubMap = callSubMap.subst(substitutionMap);
+    calleeSubMap = substitutionMap.empty() ? callSubMap
+                                           : callSubMap.subst(substitutionMap);
   }
 
   // Now that we have successfully folded all of the parameters, we can evaluate
@@ -1606,13 +1609,14 @@ llvm::Optional<SymbolicValue> ConstExprFunctionState::evaluateClosureCreation(
       }
       captures.push_back({capturedSILValue, capturedSymbolicValue});
     }
-    callSubstMap = papply->getSubstitutionMap().subst(this->substitutionMap);
+    SubstitutionMap applySubstMap = papply->getSubstitutionMap();
+    callSubstMap = substitutionMap.empty()
+                       ? applySubstMap
+                       : applySubstMap.subst(substitutionMap);
   }
 
-  SILType closureType = closureInst->getType();
-  assert(closureType.is<SILFunctionType>());
   auto closureVal = SymbolicValue::makeClosure(
-      target, captures, callSubstMap, closureType, evaluator.getAllocator());
+      target, captures, callSubstMap, closureInst, evaluator.getAllocator());
   setValue(closureInst, closureVal);
   return None;
 }

test/stdlib/OSLogPrototypeExecTest.swift

@@ -647,3 +647,23 @@ InterpolationTestSuite.test("NSObject") {
       })
   }
 }
+
+// A generic function.
+func toString<T>(_ subject: T?) -> String {
+  return ""
+}
+
+protocol TestProto {
+}
+
+InterpolationTestSuite.test("Interpolation of complex expressions") {
+  class TestClass<T: TestProto>: NSObject {
+    func testFunction() {
+      // The following call should no crash.
+      _checkFormatStringAndBuffer("A complex expression \(toString(self))") {
+        (formatString, _) in
+        expectEqual("A complex expression %s", formatString)
+      }
+    }
+  }
+}