Add argument checking for plot-area.

[samth]

Fixes racket/racket#3753.

[sorawee]

Can't we just replace unsafe-provide with provide?

[samth]

We could but the unsafe-provide was added because the generated contracts are extremely expensive, especially in compile time.

[rfindler]

Is this something where it might be possible to make a program that depends on racket/contract that is slower to compile than expected? Robby

…

On Fri, Apr 2, 2021 at 9:30 AM Sam Tobin-Hochstadt ***@***.***> wrote: We could but the unsafe-provide was added because the generated contracts are extremely expensive, especially in compile time. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#91 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADBNMEYNKCKHK6QGAPCNOLTGXIILANCNFSM42I5HDIQ> .

[samth]

I don't think this is a situation where racket/contract takes a surprising amount of time. This is just the problem we already know about where GUI types generate very large contracts.

[alex-hhh]

HI @samth , thanks for the pull request, I will test it out later today. I will also need to fix set-overlay-renderers to correctly handle the #f argument.

@sorawee , with regards to the slowdown of the generated contracts, the slowdown was severe: at runtime, a plot in a GUI application would simply consume 100% CPU and be unresponsive, making the feature effectively useless.

[alex-hhh]

Hi @samth , this unfortunately does not fix the issue.

Here is a simple program to reproduce the issue. You need to run it in DrRacket and move the mouse over the plot. Note that this will crash DrRacket.

#lang racket
(require plot)
(define snip (plot-snip (function sin -5 5)))
(send snip set-mouse-event-callback
      (lambda (snip x y event)
        (send snip set-overlay-renderers #f)))
snip

Unfortunately, adding argument checks to functions defined in TypedRacket and exported using unsafe-provide does not work: the TR optimizer will remove these checks, since they are redundant given the type information. This does create some surprising behaviors. To work around that, #:no-optimize needs to be added to the TR module, but in this case this might be too high price. I explained this in more detail here:

plot/plot-gui-lib/plot/private/gui/plot2d.rkt

Line 3 in afd0a67

;;;; About `#:no-optimize` and `unsafe-provide`

---

In this case, however, the problem is with the set-overlay-renderers call. This method is protected by a contract (which is correct), but internally it contains a mistake by creating a list out of #f, as @mflatt pointed out.

I will push a fix for this bug today.

[sorawee]

What exactly has "too high price"? Couldn't we do something similar to plot2d.rkt. That is:

1. create a new #:no-optimize module
2. define a wrapper for plot-area with explicit checks.
3. provide the wrapper unsafely.
4. change the original plot-area to provide safely.

That way, plot2d-utils.rkt is still optimized.

[alex-hhh]

Hi @sorawee , the steps you provided are indeed one way to deal with this problem, however:

• The fix would still not address the specific problem, which was a bug in set-overlay-renderers, this is already fixed in 9189c8f
• The fix would still not address the general problem that argument checks are removed from TR functions even when they are provided unsafely, creating the situation where the programmer adds some checks to prevent a crash, these checks are silently removed by TR and the crash still happens. This would be an interesting problem to solve, but it is above my skill level.

Basically, doing what you suggest will address a minor issue, and I already have too many Racket related things to do and too little time to do them...

[sorawee]

The specific problem is indeed an issue, and 9189c8f is the right fix for it.

The general problem does get fixed by my proposal above (submitted as #92). Without 9189c8f, running your code causes a contract violation error, as opposed to a crash.

[alex-hhh]

Hi @sorawee , the general problem I was referring to is that Typed Racket incorrectly removes user code when "unsafe-provide" is used:

#lang racket

(module a typed/racket
  (require typed/racket/unsafe)
  (unsafe-provide foo)
  
  (: foo (-> Integer Integer))
  (define (foo arg)
    (when (integer? arg)
      (printf "we just checked and arg is definitely an integer: ~a~%" arg)
      arg)))

(require 'a)
(foo "I guess this is an integer")

Which prints:

we just checked and arg is definitely an integer: I guess this is an integer
"I guess this is an integer"

plot-area is just one instance of this problem, and you fixed that particular instance. Solutions to the general problem would be one of the following:

• TR not optimizing away the argument check in these cases
• TR automatically disabling optimizations when unsafe-provide is used
• TR printing a warning if unsafe-provide is used with optimizations enabled, (suggesting the #:no-optimize option).

[alex-hhh]

The actual problem was fixed in this commit 9189c8f