Subject: bug#37415: Asserting failure setting frame parameters to non-fixnum values in early-init.el

From: Eli Zaretskii <[email-masked]>
Date: Mon, 23 Sep 2019 19:35:35 +0300

> Cc: [email-masked], [email-masked]
> From: martin rudalics <[email-masked]>
> Date: Mon, 23 Sep 2019 09:32:23 +0200
> 
>  > If it's fragile, then we must take a look at gui_figure_window_size, I
>  > think.  It should handle all those cases which you are afraid of.
> 
> What bothers me more is that we base the Windows code on a concept
> that it can neither understand nor control.

Which concept is that?

>  > I prefer using the hint flags as the indicator because that explicitly
>  > tells us we can use f->top and f->left.
> 
> But we do not use them in my_create_window with your patch.

my_create_window just prepares the coordinates for w32_createwindow,
and the latter does use f->top_pos and f->left_pos when appropriate.

> We just use left and top which are zero when a notation like (- 100)
> is used.

When such a notation is used, gui_figure_window_size will have already
computed the coordinates in f->top_pos and f->left_pos, and set the
hint flags, before my_create_window is called.  Which is why we don't
need to do anything in my_create_window when the hint flags are set.

But if you will feel safer with the alternative patch below, I don't
mind:

diff --git a/src/w32fns.c b/src/w32fns.c
index 34abd02..4581015 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -5421,20 +5421,33 @@ my_create_window (struct frame * f)
   Lisp_Object left, top;
   struct w32_display_info *dpyinfo = &one_w32_display_info;
 
-  /* When called with RES_TYPE_NUMBER, gui_display_get_arg will return
-     zero for anything that is not a number and is not Qunbound.  */
-  left = gui_display_get_arg (dpyinfo, Qnil, Qleft, "left", "Left",
-                              RES_TYPE_NUMBER);
-  top = gui_display_get_arg (dpyinfo, Qnil, Qtop, "top", "Top",
-                             RES_TYPE_NUMBER);
-  if (EQ (left, Qunbound))
-    coords[0] = CW_USEDEFAULT;
-  else
-    coords[0] = XFIXNUM (left);
-  if (EQ (top, Qunbound))
-    coords[1] = CW_USEDEFAULT;
+  if (!(f->size_hint_flags & USPosition || f->size_hint_flags & PPosition))
+    {
+      /* When called with RES_TYPE_NUMBER, and there's no 'top' or
+	 'left' parameters in the frame's parameter alist,
+	 gui_display_get_arg will return zero for anything that is
+	 neither a number nor Qunbound.  If frame parameter alist does
+	 have 'left' or 'top', they are interpreted by
+	 gui_figure_window_size, which was already called, and which
+	 sets f->size_hint_flags.  */
+      left = gui_display_get_arg (dpyinfo, Qnil, Qleft, "left", "Left",
+				  RES_TYPE_NUMBER);
+      top = gui_display_get_arg (dpyinfo, Qnil, Qtop, "top", "Top",
+				 RES_TYPE_NUMBER);
+      if (EQ (left, Qunbound))
+	coords[0] = CW_USEDEFAULT;
+      else
+	coords[0] = XFIXNUM (left);
+      if (EQ (top, Qunbound))
+	coords[1] = CW_USEDEFAULT;
+      else
+	coords[1] = XFIXNUM (top);
+    }
   else
-    coords[1] = XFIXNUM (top);
+    {
+      coords[0] = f->left_pos;
+      coords[1] = f->top_pos;
+    }
 
   if (!PostThreadMessage (dwWindowsThreadId, WM_EMACS_CREATEWINDOW,
 			  (WPARAM)f, (LPARAM)coords))

The 'else' block is redundant, because when the hint flags are set,
w32_createwindow will disregard coords[].  But it does no harm, so if
you are more comfortable with it, fine.

Thanks.