Make sure error from Expr(:new) is not optimized away. (#23353)

* Fix effect_free for `Expr(:new)`

  Check if the type being constructed is leaftype and check if all fields are correctly typed.
  The expression could throw an error otherwise.

* Fix functions used by interpreter and codegen to check field types

* Fix (remove) assumption of `Expr(:new)` being effect free in lowering

Author: yuyichao

base/inference.jl

@@ -3925,12 +3925,19 @@ function effect_free(@nospecialize(e), src::CodeInfo, mod::Module, allow_volatil
                 return false
             end
         elseif head === :new
-            if !allow_volatile
-                a = ea[1]
-                typ = widenconst(exprtype(a, src, mod))
-                if !isType(typ) || !isa((typ::Type).parameters[1],DataType) || ((typ::Type).parameters[1]::DataType).mutable
-                    return false
-                end
+            a = ea[1]
+            typ = exprtype(a, src, mod)
+            # `Expr(:new)` of unknown type could raise arbitrary TypeError.
+            typ, isexact = instanceof_tfunc(typ)
+            isexact || return false
+            (isleaftype(typ) && !iskindtype(typ)) || return false
+            typ = typ::DataType
+            if !allow_volatile && typ.mutable
+                return false
+            end
+            fieldcount(typ) >= length(ea) - 1 || return false
+            for fld_idx in 1:(length(ea) - 1)
+                exprtype(ea[fld_idx + 1], src, mod) ⊑ fieldtype(typ, fld_idx) || return false
             end
             # fall-through
         elseif head === :return

src/datatype.c

@@ -717,7 +717,7 @@ JL_DLLEXPORT jl_value_t *jl_new_struct(jl_datatype_t *type, ...)
     size_t nf = jl_datatype_nfields(type);
     va_start(args, type);
     jl_value_t *jv = jl_gc_alloc(ptls, jl_datatype_size(type), type);
-    for(size_t i=0; i < nf; i++) {
+    for (size_t i = 0; i < nf; i++) {
         jl_set_nth_field(jv, i, va_arg(args, jl_value_t*));
     }
     va_end(args);
@@ -731,7 +731,10 @@ JL_DLLEXPORT jl_value_t *jl_new_structv(jl_datatype_t *type, jl_value_t **args,
     if (type->instance != NULL) return type->instance;
     size_t nf = jl_datatype_nfields(type);
     jl_value_t *jv = jl_gc_alloc(ptls, jl_datatype_size(type), type);
-    for(size_t i=0; i < na; i++) {
+    for (size_t i = 0; i < na; i++) {
+        jl_value_t *ft = jl_field_type(type, i);
+        if (!jl_isa(args[i], ft))
+            jl_type_error("new", ft, args[i]);
         jl_set_nth_field(jv, i, args[i]);
     }
     for(size_t i=na; i < nf; i++) {

src/interpreter.c

@@ -270,8 +270,12 @@ static jl_value_t *eval(jl_value_t *e, interpreter_state *s)
         JL_GC_PUSH2(&thetype, &v);
         assert(jl_is_structtype(thetype));
         v = jl_new_struct_uninit((jl_datatype_t*)thetype);
-        for(size_t i=1; i < nargs; i++) {
-            jl_set_nth_field(v, i-1, eval(args[i], s));
+        for (size_t i = 1; i < nargs; i++) {
+            jl_value_t *ft = jl_field_type(thetype, i - 1);
+            jl_value_t *fldv = eval(args[i], s);
+            if (!jl_isa(fldv, ft))
+                jl_type_error("new", ft, fldv);
+            jl_set_nth_field(v, i - 1, fldv);
         }
         JL_GC_POP();
         return v;

src/julia-syntax.scm

@@ -3507,7 +3507,6 @@ f(x) = yt(x)
                     (callex (cons (car e) args)))
                (cond (tail (emit-return callex))
                      (value callex)
-                     ((eq? (car e) 'new) #f)
                      (else (emit callex)))))
             ((=)
              (let* ((rhs (compile (caddr e) break-labels #t #f))

test/core.jl

@@ -3147,6 +3147,37 @@ end
 @test_throws TypeError MyType8010([3.0;4.0])
 @test_throws TypeError MyType8010_ghost([3.0;4.0])
 
+module TestNewTypeError
+using Base.Test
+
+struct A
+end
+struct B
+    a::A
+end
+@eval function f1()
+    # Emitting this direction is not recommended but it can come from `convert` that does not
+    # return the correct type.
+    $(Expr(:new, B, 1))
+end
+@eval function f2()
+    a = $(Expr(:new, B, 1))
+    a = a
+    return nothing
+end
+@generated function f3()
+    quote
+        $(Expr(:new, B, 1))
+        return nothing
+    end
+end
+@test_throws TypeError f1()
+@test_throws TypeError f2()
+@test_throws TypeError f3()
+@test_throws TypeError eval(Expr(:new, B, 1))
+
+end
+
 # don't allow redefining types if ninitialized changes
 struct NInitializedTestType
     a