From 0e1af0be89ec592a66ff93ff8d39a21e668a868a Mon Sep 17 00:00:00 2001
From: Matthew Flatt <mflatt@racket-lang.org>
Date: Mon, 1 Jul 2019 05:28:48 -0600
Subject: [PATCH] schemify: recognize `define-values` split after inlining

Recognize `(define-values (id ...) (values rhs ...))` and split to
multiple `define`s after simplifying the right-hand side of
`define-values`. Also, don't split if a define variable is referenced
too early.
---
 .../racket-test-core/tests/racket/module.rktl | 10 ++++
 racket/src/schemify/schemify.rkt              | 51 +++++++++++--------
 racket/src/schemify/simple.rkt                |  5 ++
 3 files changed, 44 insertions(+), 22 deletions(-)

diff --git a/pkgs/racket-test-core/tests/racket/module.rktl b/pkgs/racket-test-core/tests/racket/module.rktl
index c2de6501f3..ccf6f4fbfe 100644
--- a/pkgs/racket-test-core/tests/racket/module.rktl
+++ b/pkgs/racket-test-core/tests/racket/module.rktl
@@ -2934,6 +2934,16 @@ case of module-leve bindings; it doesn't cover local bindings.
   (write re-m2 re-o2)
   (check-vm (get-output-bytes re-o2) (system-type 'vm)))
 
+;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+;; Make sure `(define-values (id ...) (values rhs ...))` is not
+;; split if one of the `id`s is referenced early
+
+(module uses-a-in-define-values-before-a-is-defined racket/base
+  (define-values (a b)
+    (values 'a a)))
+
+(err/rt-test/once (dynamic-require ''uses-a-in-define-values-before-a-is-defined #f))
+
 ;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 
 (report-errs)
diff --git a/racket/src/schemify/schemify.rkt b/racket/src/schemify/schemify.rkt
index 98baa3eb1c..e3bb243978 100644
--- a/racket/src/schemify/schemify.rkt
+++ b/racket/src/schemify/schemify.rkt
@@ -249,14 +249,18 @@
         ;; For the case that the right-hand side won't capture a
         ;; continuation or return multiple times, we can generate a
         ;; simple definition:
-        (define (finish-definition ids [accum-exprs accum-exprs] [accum-ids accum-ids])
+        (define (finish-definition ids [accum-exprs accum-exprs] [accum-ids accum-ids]
+                                   #:schemified [schemified schemified]
+                                   #:k [k #f])
           (append
            (make-expr-defns accum-exprs)
            (cons
             schemified
             (let id-loop ([ids ids] [accum-exprs null] [accum-ids accum-ids])
               (cond
-                [(wrap-null? ids) (loop (wrap-cdr l) mut-l accum-exprs accum-ids)]
+                [(wrap-null? ids) (if k
+                                      (k accum-exprs accum-ids)
+                                      (loop (wrap-cdr l) mut-l accum-exprs accum-ids))]
                 [(or (or for-jitify? for-cify?)
                      (via-variable-mutated-state? (hash-ref mutated (unwrap (wrap-car ids)) #f)))
                  (define id (unwrap (wrap-car ids)))
@@ -302,8 +306,7 @@
                (if for-jitify?
                    expr
                    (make-expr-defn expr))
-               (append defns
-                       (loop (wrap-cdr l) mut-l null null)))])))
+               (append defns (loop (wrap-cdr l) mut-l null null)))])))
         ;; Dispatch on the schemified form, distinguishing definitions
         ;; from expressions:
         (match schemified
@@ -316,11 +319,29 @@
           [`(define-values ,ids ,rhs)
            (cond
              [(simple? #:pure? #f rhs prim-knowns knowns imports mutated simples)
-              (finish-definition ids)]
+              (match rhs
+                [`(values ,rhss ...)
+                 ;; Flatten `(define-values (id ...) (values rhs ...))` to
+                 ;; a sequence `(define id rhs) ...`
+                 (if (and (= (length rhss) (length ids))
+                          ;; Must be pure, otherwise a variable might be referenced
+                          ;; too early:
+                          (for/and ([rhs (in-list rhss)])
+                            (simple? rhs prim-knowns knowns imports mutated simples)))
+                     (let values-loop ([ids ids] [rhss rhss] [accum-exprs accum-exprs] [accum-ids accum-ids])
+                       (cond
+                         [(null? ids) (loop (wrap-cdr l) mut-l accum-exprs accum-ids)]
+                         [else
+                          (define id (car ids))
+                          (define rhs (car rhss))
+                          (finish-definition (list id) accum-exprs accum-ids
+                                              #:schemified `(define ,id ,rhs)
+                                              #:k (lambda (accum-exprs accum-ids)
+                                                    (values-loop (cdr ids) (cdr rhss) accum-exprs accum-ids)))]))
+                     (finish-definition ids))]
+                [`,_ (finish-definition ids)])]
              [else
               (finish-wrapped-definition ids rhs)])]
-          [`(splice . ,ls)
-           (loop (append ls (wrap-cdr l)) in-mut-l accum-exprs accum-ids)]
           [`,_
            (match form
              [`(define-values ,ids ,_)
@@ -418,21 +439,7 @@
            [`(define-values (,id) ,rhs)
             `(define ,id ,(schemify rhs))]
            [`(define-values ,ids ,rhs)
-            (let loop ([rhs rhs])
-              (match rhs
-                [`(values ,rhss ...)
-                 (cond
-                   [(= (length rhss) (length ids))
-                    `(splice ; <- result goes back to schemify, so don't schemify rhss
-                      ,@(for/list ([id (in-list ids)]
-                                   [rhs (in-list rhss)])
-                          `(define-values (,id) ,rhs)))]
-                   [else
-                    `(define-values ,ids ,(schemify rhs))])]
-                [`(let-values () ,rhs)
-                 (loop rhs)]
-                [`,_
-                 `(define-values ,ids ,(schemify rhs))]))]
+            `(define-values ,ids ,(schemify rhs))]
            [`(quote ,_) v]
            [`(let-values () ,body)
             (schemify body)]
diff --git a/racket/src/schemify/simple.rkt b/racket/src/schemify/simple.rkt
index d1f553e89a..9426f8f255 100644
--- a/racket/src/schemify/simple.rkt
+++ b/racket/src/schemify/simple.rkt
@@ -52,6 +52,11 @@
        (cached
         (for/and ([e (in-list es)])
           (simple? e)))]
+      [`(values ,es ...)
+       #:guard (not pure?)
+       (cached
+        (for/and ([e (in-list es)])
+          (simple? e)))]
       [`(,proc . ,args)
        (cached
         (let ([proc (unwrap proc)])