patch 9.0.1053: default constructor arguments are not optional

Problem:    Default constructor arguments are not optional.
Solution:   Use "= v:none" to make constructor arguments optional.

runtime/doc/vim9class.txt

@@ -431,15 +431,15 @@ members, in the order they were specified.  Thus if your class looks like: >
 
 Then The default constructor will be: >
 
-	def new(this.name = void, this.age = void, this.gender = void)
+	def new(this.name = v:none, this.age = v:none, this.gender = v:none)
 	enddef
 
 All object members will be used, also private access ones.
 
-The "= void" default values make the arguments optional.  Thus you can also
+The "= v:none" default values make the arguments optional.  Thus you can also
-call `new()` without any arguments.  Since "void" isn't an actual value, no
+call `new()` without any arguments.  No assignment will happen and the default
-assignment will happen and the default value for the object members will be
+value for the object members will be used.  This is a more useful example,
-used.  This is a more useful example, with default values: >
+with default values: >
 
 	class TextPosition
 	   this.lnum: number = 1
@@ -450,8 +450,12 @@ If you want the constructor to have mandatory arguments, you need to write it
 yourself.  For example, if for the AutoNew class above you insist on getting
 the name, you can define the constructor like this: >
 
-	def new(this.name, this.age = void, this.gender = void)
+	def new(this.name, this.age = v:none, this.gender = v:none)
 	enddef
+<							*E1328*
+Note that you cannot use another default value than "v:none" here.  If you
+want to initialize the object members, do it where they are declared.  This
+way you only need to look in one place for the default values.
 
 
 Multiple constructors ~

src/errors.h

@@ -3372,4 +3372,6 @@ EXTERN char e_member_not_found_on_object_str_str[]
 	INIT(= N_("E1326: Member not found on object \"%s\": %s"));
 EXTERN char e_object_required_found_str[]
 	INIT(= N_("E1327: Object required, found %s"));
+EXTERN char e_constructor_default_value_must_be_vnone_str[]
+	INIT(= N_("E1328: Constructor default value must be v:none: %s"));
 #endif

src/proto/vim9instr.pro

@@ -47,7 +47,7 @@ int generate_NEWFUNC(cctx_T *cctx, char_u *lambda_name, char_u *func_name);
 int generate_DEF(cctx_T *cctx, char_u *name, size_t len);
 int generate_JUMP(cctx_T *cctx, jumpwhen_T when, int where);
 int generate_WHILE(cctx_T *cctx, int funcref_idx);
-int generate_JUMP_IF_ARG_SET(cctx_T *cctx, int arg_off);
+int generate_JUMP_IF_ARG(cctx_T *cctx, isntype_T isn_type, int arg_off);
 int generate_FOR(cctx_T *cctx, int loop_idx);
 int generate_ENDLOOP(cctx_T *cctx, loop_info_T *loop_info);
 int generate_TRYCONT(cctx_T *cctx, int levels, int where);

src/testdir/test_vim9_class.vim

@@ -182,5 +182,56 @@ def Test_class_member_initializer()
   v9.CheckScriptSuccess(lines)
 enddef
 
+def Test_class_default_new()
+  var lines =<< trim END
+      vim9script
+
+      class TextPosition
+        this.lnum: number = 1
+        this.col: number = 1
+      endclass
+
+      var pos = TextPosition.new()
+      assert_equal(1, pos.lnum)
+      assert_equal(1, pos.col)
+
+      pos = TextPosition.new(v:none, v:none)
+      assert_equal(1, pos.lnum)
+      assert_equal(1, pos.col)
+
+      pos = TextPosition.new(3, 22)
+      assert_equal(3, pos.lnum)
+      assert_equal(22, pos.col)
+
+      pos = TextPosition.new(v:none, 33)
+      assert_equal(1, pos.lnum)
+      assert_equal(33, pos.col)
+  END
+  v9.CheckScriptSuccess(lines)
+
+  lines =<< trim END
+      vim9script
+      class Person
+        this.name: string
+        this.age: number = 42
+        this.education: string = "unknown"
+
+        def new(this.name, this.age = v:none, this.education = v:none)
+        enddef
+      endclass
+
+      var piet = Person.new("Piet")
+      assert_equal("Piet", piet.name)
+      assert_equal(42, piet.age)
+      assert_equal("unknown", piet.education)
+
+      var chris = Person.new("Chris", 4, "none")
+      assert_equal("Chris", chris.name)
+      assert_equal(4, chris.age)
+      assert_equal("none", chris.education)
+  END
+  v9.CheckScriptSuccess(lines)
+enddef
+
 
 " vim: ts=8 sw=2 sts=2 expandtab tw=80 fdm=marker

src/userfunc.c

@@ -224,7 +224,6 @@ get_function_args(
     char_u	*p;
     int		c;
     int		any_default = FALSE;
-    char_u	*expr;
     char_u	*whitep = *argp;
 
     if (newargs != NULL)
@@ -302,6 +301,34 @@ get_function_args(
 	    arg = p;
 	    while (ASCII_ISALNUM(*p) || *p == '_')
 		++p;
+	    char_u *argend = p;
+
+	    if (*skipwhite(p) == '=')
+	    {
+		char_u *defval = skipwhite(skipwhite(p) + 1);
+		if (STRNCMP(defval, "v:none", 6) != 0)
+		{
+		    semsg(_(e_constructor_default_value_must_be_vnone_str), p);
+		    goto err_ret;
+		}
+		any_default = TRUE;
+		p = defval + 6;
+
+		if (ga_grow(default_args, 1) == FAIL)
+		    goto err_ret;
+
+		char_u *expr = vim_strsave((char_u *)"v:none");
+		if (expr == NULL)
+		    goto err_ret;
+		((char_u **)(default_args->ga_data))
+						 [default_args->ga_len] = expr;
+		default_args->ga_len++;
+	    }
+	    else if (any_default)
+	    {
+		emsg(_(e_non_default_argument_follows_default_argument));
+		goto err_ret;
+	    }
 
 	    // TODO: check the argument is indeed a member
 	    if (newargs != NULL && ga_grow(newargs, 1) == FAIL)
@@ -309,7 +336,7 @@ get_function_args(
 	    if (newargs != NULL)
 	    {
 		((char_u **)(newargs->ga_data))[newargs->ga_len] =
-						    vim_strnsave(arg, p - arg);
+					       vim_strnsave(arg, argend - arg);
 		newargs->ga_len++;
 
 		if (argtypes != NULL && ga_grow(argtypes, 1) == OK)
@@ -322,15 +349,22 @@ get_function_args(
 		    if (ga_grow(newlines, 1) == OK)
 		    {
 			// "this.name = name"
-			int len = 5 + (p - arg) + 3 + (p - arg) + 1;
+			int len = 5 + (argend - arg) + 3 + (argend - arg) + 1;
+			if (any_default)
+			    len += 14 + 10;
 			char_u *assignment = alloc(len);
 			if (assignment != NULL)
 			{
-			    c = *p;
+			    c = *argend;
-			    *p = NUL;
+			    *argend = NUL;
-			    vim_snprintf((char *)assignment, len,
+			    if (any_default)
+				vim_snprintf((char *)assignment, len,
+						"ifargisset %d this.%s = %s",
+					   default_args->ga_len - 1, arg, arg);
+			    else
+				vim_snprintf((char *)assignment, len,
 						     "this.%s = %s", arg, arg);
-			    *p = c;
+			    *argend = c;
 			    ((char_u **)(newlines->ga_data))[
 					      newlines->ga_len++] = assignment;
 			}
@@ -361,7 +395,7 @@ get_function_args(
 		// find the end of the expression (doesn't evaluate it)
 		any_default = TRUE;
 		p = skipwhite(np + 1);
-		expr = p;
+		char_u *expr = p;
 		if (eval1(&p, &rettv, NULL) != FAIL)
 		{
 		    if (!skip)

src/version.c

@@ -695,6 +695,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1053,
 /**/
     1052,
 /**/

src/vim9.h

@@ -124,6 +124,8 @@ typedef enum {
     ISN_JUMP,	    // jump if condition is matched isn_arg.jump
     ISN_JUMP_IF_ARG_SET, // jump if argument is already set, uses
 			 // isn_arg.jumparg
+    ISN_JUMP_IF_ARG_NOT_SET, // jump if argument is not set, uses
+			 // isn_arg.jumparg
 
     // loop
     ISN_FOR,	    // get next item from a list, uses isn_arg.forloop
@@ -260,7 +262,7 @@ typedef struct {
     int		jump_where;	// position to jump to
 } jump_T;
 
-// arguments to ISN_JUMP_IF_ARG_SET
+// arguments to ISN_JUMP_IF_ARG_SET and ISN_JUMP_IF_ARG_NOT_SET
 typedef struct {
     int		jump_arg_off;	// argument index, negative
     int		jump_where;	// position to jump to

src/vim9class.c

@@ -269,6 +269,7 @@ ex_class(exarg_T *eap)
 		ga_concat(&fga, (char_u *)"this.");
 		objmember_T *m = cl->class_obj_members + i;
 		ga_concat(&fga, (char_u *)m->om_name);
+		ga_concat(&fga, (char_u *)" = v:none");
 	    }
 	    ga_concat(&fga, (char_u *)")\nenddef\n");
 	    ga_append(&fga, NUL);

src/vim9compile.c

@@ -2195,8 +2195,13 @@ push_default_value(
  * Return "arg" if it does not look like a variable list.
  */
     static char_u *
-compile_assignment(char_u *arg, exarg_T *eap, cmdidx_T cmdidx, cctx_T *cctx)
+compile_assignment(
+	char_u	    *arg_start,
+	exarg_T	    *eap,
+	cmdidx_T    cmdidx,
+	cctx_T	    *cctx)
 {
+    char_u	*arg = arg_start;
     char_u	*var_start;
     char_u	*p;
     char_u	*end = arg;
@@ -2206,6 +2211,7 @@ compile_assignment(char_u *arg, exarg_T *eap, cmdidx_T cmdidx, cctx_T *cctx)
     int		semicolon = 0;
     int		did_generate_slice = FALSE;
     garray_T	*instr = &cctx->ctx_instr;
+    int		jump_instr_idx = instr->ga_len;
     char_u	*op;
     int		oplen = 0;
     int		heredoc = FALSE;
@@ -2216,6 +2222,23 @@ compile_assignment(char_u *arg, exarg_T *eap, cmdidx_T cmdidx, cctx_T *cctx)
     lhs_T	lhs;
     long	start_lnum = SOURCING_LNUM;
 
+    int		has_arg_is_set_prefix = STRNCMP(arg, "ifargisset ", 11) == 0;
+    if (has_arg_is_set_prefix)
+    {
+	arg += 11;
+	int def_idx = getdigits(&arg);
+	arg = skipwhite(arg);
+
+	// Use a JUMP_IF_ARG_NOT_SET instruction to skip if the value was not
+	// given and the default value is "v:none".
+	int off = STACK_FRAME_SIZE + (cctx->ctx_ufunc->uf_va_name != NULL
+								      ? 1 : 0);
+	int count = cctx->ctx_ufunc->uf_def_args.ga_len;
+	if (generate_JUMP_IF_ARG(cctx, ISN_JUMP_IF_ARG_NOT_SET,
+						def_idx - count - off) == FAIL)
+	    goto theend;
+    }
+
     // Skip over the "varname" or "[varname, varname]" to get to any "=".
     p = skip_var_list(arg, TRUE, &var_count, &semicolon, TRUE);
     if (p == NULL)
@@ -2636,6 +2659,13 @@ compile_assignment(char_u *arg, exarg_T *eap, cmdidx_T cmdidx, cctx_T *cctx)
 
 	if (var_idx + 1 < var_count)
 	    var_start = skipwhite(lhs.lhs_end + 1);
+
+	if (has_arg_is_set_prefix)
+	{
+	    // set instruction index in JUMP_IF_ARG_SET to here
+	    isn_T *isn = ((isn_T *)instr->ga_data) + jump_instr_idx;
+	    isn->isn_arg.jumparg.jump_where = instr->ga_len;
+	}
     }
 
     // For "[var, var] = expr" drop the "expr" value.
@@ -2711,9 +2741,9 @@ may_compile_assignment(exarg_T *eap, char_u **line, cctx_T *cctx)
 	}
     }
 
-    if (*eap->cmd == '[')
+    // might be "[var, var] = expr" or "ifargisset this.member = expr"
+    if (*eap->cmd == '[' || STRNCMP(eap->cmd, "ifargisset ", 11) == 0)
     {
-	// might be "[var, var] = expr"
 	*line = compile_assignment(eap->cmd, eap, CMD_SIZE, cctx);
 	if (*line == NULL)
 	    return FAIL;
@@ -2994,7 +3024,6 @@ compile_def_function(
 	int	count = ufunc->uf_def_args.ga_len;
 	int	first_def_arg = ufunc->uf_args.ga_len - count;
 	int	i;
-	char_u	*arg;
 	int	off = STACK_FRAME_SIZE + (ufunc->uf_va_name != NULL ? 1 : 0);
 	int	did_set_arg_type = FALSE;
 
@@ -3002,23 +3031,27 @@ compile_def_function(
 	SOURCING_LNUM = 0;  // line number unknown
 	for (i = 0; i < count; ++i)
 	{
+	    char_u *arg = ((char_u **)(ufunc->uf_def_args.ga_data))[i];
+	    if (STRCMP(arg, "v:none") == 0)
+		// "arg = v:none" means the argument is optional without
+		// setting a value when the argument is missing.
+		continue;
+
 	    type_T	*val_type;
 	    int		arg_idx = first_def_arg + i;
 	    where_T	where = WHERE_INIT;
-	    int		r;
 	    int		jump_instr_idx = instr->ga_len;
 	    isn_T	*isn;
 
 	    // Use a JUMP_IF_ARG_SET instruction to skip if the value was given.
-	    if (generate_JUMP_IF_ARG_SET(&cctx, i - count - off) == FAIL)
+	    if (generate_JUMP_IF_ARG(&cctx, ISN_JUMP_IF_ARG_SET,
+						      i - count - off) == FAIL)
 		goto erret;
 
 	    // Make sure later arguments are not found.
 	    ufunc->uf_args_visible = arg_idx;
 
-	    arg = ((char_u **)(ufunc->uf_def_args.ga_data))[i];
+	    int r = compile_expr0(&arg, &cctx);
-	    r = compile_expr0(&arg, &cctx);
-
 	    if (r == FAIL)
 		goto erret;
 

src/vim9execute.c

@@ -2068,7 +2068,7 @@ handle_debug(isn_T *iptr, ectx_T *ectx)
 }
 
 /*
- * Store a value in a list, dict or blob variable.
+ * Store a value in a list, dict, blob or object variable.
  * Returns OK, FAIL or NOTDONE (uncatchable error).
  */
     static int
@@ -2177,9 +2177,9 @@ execute_storeindex(isn_T *iptr, ectx_T *ectx)
 	{
 	    long	    lidx = (long)tv_idx->vval.v_number;
 	    blob_T	    *blob = tv_dest->vval.v_blob;
-	    varnumber_T nr;
+	    varnumber_T	    nr;
-	    int	    error = FALSE;
+	    int		    error = FALSE;
-	    int	    len;
+	    int		    len;
 
 	    if (blob == NULL)
 	    {
@@ -2209,6 +2209,7 @@ execute_storeindex(isn_T *iptr, ectx_T *ectx)
 	    long	    idx = (long)tv_idx->vval.v_number;
 	    object_T	    *obj = tv_dest->vval.v_object;
 	    typval_T	    *otv = (typval_T *)(obj + 1);
+	    clear_tv(&otv[idx]);
 	    otv[idx] = *tv;
 	}
 	else
@@ -4293,10 +4294,12 @@ exec_instructions(ectx_T *ectx)
 	    // Jump if an argument with a default value was already set and not
 	    // v:none.
 	    case ISN_JUMP_IF_ARG_SET:
+	    case ISN_JUMP_IF_ARG_NOT_SET:
 		tv = STACK_TV_VAR(iptr->isn_arg.jumparg.jump_arg_off);
-		if (tv->v_type != VAR_UNKNOWN
+		int arg_set = tv->v_type != VAR_UNKNOWN
-			&& !(tv->v_type == VAR_SPECIAL
+				&& !(tv->v_type == VAR_SPECIAL
-					    && tv->vval.v_number == VVAL_NONE))
+					    && tv->vval.v_number == VVAL_NONE);
+		if (iptr->isn_type == ISN_JUMP_IF_ARG_SET ? arg_set : !arg_set)
 		    ectx->ec_iidx = iptr->isn_arg.jumparg.jump_where;
 		break;
 
@@ -6633,6 +6636,12 @@ list_instructions(char *pfx, isn_T *instr, int instr_count, ufunc_T *ufunc)
 						iptr->isn_arg.jump.jump_where);
 		break;
 
+	    case ISN_JUMP_IF_ARG_NOT_SET:
+		smsg("%s%4d JUMP_IF_ARG_NOT_SET arg[%d] -> %d", pfx, current,
+			 iptr->isn_arg.jumparg.jump_arg_off + STACK_FRAME_SIZE,
+						iptr->isn_arg.jump.jump_where);
+		break;
+
 	    case ISN_FOR:
 		{
 		    forloop_T *forloop = &iptr->isn_arg.forloop;

src/vim9instr.c

@@ -1408,15 +1408,15 @@ generate_WHILE(cctx_T *cctx, int funcref_idx)
 }
 
 /*
- * Generate an ISN_JUMP_IF_ARG_SET instruction.
+ * Generate an ISN_JUMP_IF_ARG_SET or ISN_JUMP_IF_ARG_NOT_SET instruction.
  */
     int
-generate_JUMP_IF_ARG_SET(cctx_T *cctx, int arg_off)
+generate_JUMP_IF_ARG(cctx_T *cctx, isntype_T isn_type, int arg_off)
 {
     isn_T	*isn;
 
     RETURN_OK_IF_SKIP(cctx);
-    if ((isn = generate_instr(cctx, ISN_JUMP_IF_ARG_SET)) == NULL)
+    if ((isn = generate_instr(cctx, isn_type)) == NULL)
 	return FAIL;
     isn->isn_arg.jumparg.jump_arg_off = arg_off;
     // jump_where is set later
@@ -2479,6 +2479,7 @@ delete_instr(isn_T *isn)
 	case ISN_GETITEM:
 	case ISN_GET_OBJ_MEMBER:
 	case ISN_JUMP:
+	case ISN_JUMP_IF_ARG_NOT_SET:
 	case ISN_JUMP_IF_ARG_SET:
 	case ISN_LISTAPPEND:
 	case ISN_LISTINDEX: