Merge pull request #38053 from guitard0g/fix_closure_constantness_check

[Sema]  Fix constantness diagnostics for single expression closures

Author: Josh Learn

lib/Sema/ConstantnessSemaDiagnostics.cpp

@@ -335,25 +335,56 @@ static void diagnoseConstantArgumentRequirementOfCall(const CallExpr *callExpr,
 void swift::diagnoseConstantArgumentRequirement(
     const Expr *expr, const DeclContext *declContext) {
   class ConstantReqCallWalker : public ASTWalker {
-    const ASTContext &astContext;
+    DeclContext *DC;
 
   public:
-    ConstantReqCallWalker(ASTContext &ctx) : astContext(ctx) {}
+    ConstantReqCallWalker(DeclContext *DC) : DC(DC) {}
 
     // Descend until we find a call expressions. Note that the input expression
     // could be an assign expression or another expression that contains the
     // call.
     std::pair<bool, Expr *> walkToExprPre(Expr *expr) override {
-      if (!expr || isa<ErrorExpr>(expr) || !expr->getType())
+      // Handle closure expressions separately as we may need to
+      // manually descend into the body.
+      if (auto *closureExpr = dyn_cast<ClosureExpr>(expr)) {
+        return walkToClosureExprPre(closureExpr);
+      }
+      // Interpolated expressions' bodies will be type checked
+      // separately so exit early to avoid duplicate diagnostics.
+      if (!expr || isa<ErrorExpr>(expr) || !expr->getType() ||
+          isa<InterpolatedStringLiteralExpr>(expr))
         return {false, expr};
       if (auto *callExpr = dyn_cast<CallExpr>(expr)) {
-        diagnoseConstantArgumentRequirementOfCall(callExpr, astContext);
-        return {false, expr};
+        diagnoseConstantArgumentRequirementOfCall(callExpr, DC->getASTContext());
       }
       return {true, expr};
     }
+    
+    std::pair<bool, Expr *> walkToClosureExprPre(ClosureExpr *closure) {
+      if (closure->hasSingleExpressionBody()) {
+        // Single expression closure bodies are not visited directly
+        // by the ASTVisitor, so we must descend into the body manually
+        // and set the DeclContext to that of the closure.
+        DC = closure;
+        return {true, closure};
+      }
+      return {false, closure};
+    }
+    
+    Expr *walkToExprPost(Expr *expr) override {
+      if (auto *closureExpr = dyn_cast<ClosureExpr>(expr)) {
+        // Reset the DeclContext to the outer scope if we descended
+        // into a closure expr.
+        DC = closureExpr->getParent();
+      }
+      return expr;
+    }
+    
+    std::pair<bool, Stmt *> walkToStmtPre(Stmt *stmt) override {
+      return {true, stmt};
+    }
   };
 
-  ConstantReqCallWalker walker(declContext->getASTContext());
+  ConstantReqCallWalker walker(const_cast<DeclContext *>(declContext));
   const_cast<Expr *>(expr)->walk(walker);
 }

test/Sema/diag_constantness_check.swift

@@ -361,3 +361,66 @@ func testConstructorAnnotation(x: Int) {
   let _ = ConstructorTest(x)
     // expected-error@-1 {{argument must be an integer literal}}
 }
+
+// Test closure expressions
+
+func funcAcceptingClosure<T>(_ x: () -> T) -> T {
+  return x()
+}
+
+func normalFunction() {}
+
+@_semantics("oslog.requires_constant_arguments")
+func constantArgumentFunctionReturningIntCollection(_ constArg: Int) -> Array<Int> {
+  return [constArg, constArg, constArg]
+}
+
+@_semantics("oslog.requires_constant_arguments")
+func constantArgumentFunctionReturningInt(_ constArg: Int) -> Int {
+  return constArg
+}
+
+func testCallsWithinClosures(s: String, x: Int) {
+  funcAcceptingClosure {
+    constantArgumentFunction(s)
+    // expected-error@-1 {{argument must be a string literal}}
+  }
+  funcAcceptingClosure {
+    constantArgumentFunction(s)
+    // expected-error@-1 {{argument must be a string literal}}
+    constantArgumentFunction(s)
+    // expected-error@-1 {{argument must be a string literal}}
+  }
+  funcAcceptingClosure {
+    funcAcceptingClosure {
+      constantArgumentFunction(s)
+        // expected-error@-1 {{argument must be a string literal}}
+    }
+  }
+  funcAcceptingClosure {
+    normalFunction()
+    funcAcceptingClosure {
+      constantArgumentFunction(s)
+        // expected-error@-1 {{argument must be a string literal}}
+    }
+  }
+  let _ =
+    funcAcceptingClosure {
+      constantArgumentFunctionReturningIntCollection(x)
+        // expected-error@-1 {{argument must be an integer literal}}
+    }
+    .filter { $0 > 0 }
+    .map { $0 + 1 }
+  let _ =
+    funcAcceptingClosure {
+      constantArgumentFunctionReturningInt(x)
+        // expected-error@-1 {{argument must be an integer literal}}
+    } + 10 * x
+  let _ = { constantArgumentFunctionReturningIntCollection(x) }
+    // expected-error@-1 {{argument must be an integer literal}}
+  funcAcceptingClosure {
+    constantArgumentFunction(1)
+    constantArgumentFunction("string literal")
+    constantArgumentFunction("string with a single interpolation \(x)")
+  }
+}

test/Sema/diag_constantness_check_os_log.swift

@@ -185,3 +185,44 @@ func testLogMessageWrappingDiagnostics() {
   _osLogTestHelper(nonConstantFunction("key", fallback: "A literal message"))
     // expected-error@-1{{argument must be a string interpolation}}
 }
+
+// Test closure expressions
+
+func funcAcceptingClosure<T>(_ x: () -> T) -> T {
+  return x()
+}
+
+func normalFunction() {}
+
+func testCallsWithinClosures(formatOpt: OSLogIntegerFormatting) {
+  funcAcceptingClosure {
+    _osLogTestHelper("Minimum integer value: \(Int.min, format: formatOpt)")
+      // expected-error@-1 {{argument must be a static method or property of 'OSLogIntegerFormatting'}}
+  }
+  funcAcceptingClosure {
+    _osLogTestHelper("Minimum integer value: \(Int.min, format: formatOpt)")
+      // expected-error@-1 {{argument must be a static method or property of 'OSLogIntegerFormatting'}}
+    _osLogTestHelper("Maximum integer value: \(Int.max, format: formatOpt)")
+      // expected-error@-1 {{argument must be a static method or property of 'OSLogIntegerFormatting'}}
+  }
+  funcAcceptingClosure {
+    funcAcceptingClosure {
+      _osLogTestHelper("Minimum integer value: \(Int.min, format: formatOpt)")
+        // expected-error@-1 {{argument must be a static method or property of 'OSLogIntegerFormatting'}}
+    }
+  }
+  funcAcceptingClosure {
+    normalFunction()
+    funcAcceptingClosure {
+      _osLogTestHelper("Minimum integer value: \(Int.min, format: formatOpt)")
+        // expected-error@-1 {{argument must be a static method or property of 'OSLogIntegerFormatting'}}
+    }
+  }
+  funcAcceptingClosure {
+    _osLogTestHelper("Minimum integer value: \(Int.min, format: .hex)")
+  }
+  funcAcceptingClosure {
+    _osLogTestHelper("Minimum integer value: \(Int.min, format: .hex)")
+    _osLogTestHelper("Maximum integer value: \(Int.max, privacy: .public)")
+  }
+}