loop / recur inference, warnings not suppressed on initial pass

Description

With CLJS-2873, we make an additional analyzer pass of loop / recur to properly infer types, but warnings are enabled during the first pass and can fire.

Repro:

cljs.user=> (loop [x "a"] (if (= "a" x) (recur true) (+ 3 x))) WARNING: cljs.core/+, all arguments must be numbers, got [number string] instead at line 4 <cljs repl> WARNING: cljs.core/+, all arguments must be numbers, got [number #{boolean string}] instead at line 4 <cljs repl> 4

Really, we want the last and only last warning in the example above to be emitted.

As another example, motivating this last claim, the following form should not cause any warnings to be emitted

(loop [x "a"] (if (= "a" x) (recur 1) (+ 3 x)))

Environment

None

Attachments

2
  • 06 Jan 2019, 09:45 PM
  • 04 Jan 2019, 04:53 AM

Activity

Show:

Mike Fikes January 6, 2019 at 9:45 PM

CLJS-3031-2.patch revises things so that warnings are only accumulated when we are doing the first analysis of a loop (and not for normal let s or second analysis of a loop).

CLJS-3031-2.patch passes CI
CLJS-3031-2.patch passes Canary

The revised patch passes Canary for cljs-oops because cljs-oops employs some macros to capture stderr during compilation, with the captured results being placed in an outermost let-bound atom. With the prevous patch, and with the outermost cljs-oops let, warns would be deferred until after the scope of cljs-oops's macros, thus defeating its strategy of capturing warns on stderr.

The revised previous falure of planck in Canary was sporadic and not related to the patch.

Mike Fikes January 4, 2019 at 11:27 AM

Mike Fikes January 4, 2019 at 4:54 AM

CLJS-3031.patch passes CI

Mike Fikes January 4, 2019 at 4:53 AM

CLJS-3031.patch solves the issue by suppressing all but the last set of warnings produced when analyzing let bodies.

Normally, you would just conditionally use the no-warn macro to suppress warnings, but in this situation, we actually need to analyze the let body in order to decide whether we need to re-analyze it. In other words, we don't know a priori whether to suppress warnings when analyzing the let body.

To solve this catch 22, the patch instead records any warnings, and only plays them back after we know we are not going to recur.

Completed

Details

Assignee

Reporter

Approval

Accepted

Patch

Code and Test

Priority

Created January 4, 2019 at 2:45 AM
Updated January 25, 2019 at 9:32 PM
Resolved January 25, 2019 at 9:32 PM