Subject: bug#59468: 29.0.50; c-ts-mode cannot fontify after macros are encountered

From: Po Lu <[email-masked]>
Date: Thu, 24 Nov 2022 08:36:13 +0800

Yuan Fu <[email-masked]> writes:

> Po Lu <[email-masked]> writes:
>
> Ok, I had a look at each of these cases, comments:
>
>> Insert the following code in a buffer, and enable c-ts-mode.
>>
>> #define CheckExtension(name)				\
>>   if (!name)						\
>>     {							\
>>       if (display)					\
>> 	eglTerminate (display);				\
>>       fprintf (stderr, "Missing: egl%s\n", #name + 1);	\
>>       return False;					\
>>     }
>>
>> #define CheckExtensionGl(name)				\
>>   if (!name)						\
>>     {							\
>>       /* If the context remains current, then nothing	\
>> 	 will get released upon eglTerminate.  */	\
>>       eglMakeCurrent (display, EGL_NO_SURFACE,		\
>> 		      EGL_NO_SURFACE, EGL_NO_CONTEXT);	\
>>       eglTerminate (display);				\
>>       fprintf (stderr, "Missing: gl%s\n", #name + 1);	\
>>       return False;					\
>>     }
>>
>> #define LoadProc(name, ext, extname)			\
>>   if (HaveEglExtension (extname))			\
>>     I##name						\
>>       = (void *) eglGetProcAddress ("egl" #name ext)
>>
>> #define LoadProcGl(name, ext, extname)			\
>>   if (HaveGlExtension (extname))			\
>>     I##name						\
>>       = (void *) eglGetProcAddress ("gl" #name ext)
>>
>> #define CheckGlExtension(name)			\
>>   if (!HaveGlExtension (name))			\
>>     {						\
>>       fprintf (stderr, "Missing %s\n", name);	\
>> 						\
>>       eglMakeCurrent (display, EGL_NO_SURFACE,	\
>> 		      EGL_NO_SURFACE,		\
>> 		      EGL_NO_CONTEXT);		\
>>       eglTerminate (display);			\
>>       return False;				\
>>     }
>>
>> There will be no fontification of types or string constants inside the
>> macro bodies.  CC Mode works fine.
>
> Tree-sitter right now doesn’t parse the content of macro bodies. I think
> it should be reasonable for tree-sitter-c to parse them. In the meantime
> I’ll see if I can trick tree-sitter to parse it as normal C source.
>
>
>> Now, insert the following code below the macro definition:
>>
>> static Visual *
>> FindVisual (VisualID visual, int *depth)
>> {
>>   XVisualInfo vinfo, *visuals;
>>   Visual *value;
>>   int nvisuals;
>>   const char *override;
>> }
>>
>> visual, depth, visuals, value and override are not fontified as
>> identifiers.  Line feed characters below the macros are also fontfied as
>> "errors".
>
> That should fixed by a recent change.
>
>> Within the following code:
>>
>> static BufferActivityRecord *
>> FindBufferActivityRecord (PictureBuffer *buffer, PictureTarget *target)
>> {
>>   BufferActivityRecord *record;
>>
>>   /* Look through the global activity list for a record matching
>>      the given values.  */
>>   record = all_activity.global_next;
>>   while (record != &all_activity)
>>     {
>>       if (record->buffer == buffer
>> 	  && record->target == target)
>> 	return record;
>>
>>       record = record->global_next;
>>     }
>>
>>   return NULL;
>> }
>>
>> "record" is fontified as an identifier, but not within the condition in
>> the while loop.
>
> That’s expected, since the font-lock rule is to fontify LHS of
> assignments (the first record), but not all appearances of identifiers.
> Now we have font-lock rules that fontifies all occurrences of functions
> and variables. I personally don’t find them useful, but if someone wants
> it it’s there.
>
>>
>> Within the following code:
>>
>> static void
>> compare_single_row_8bpc4 (unsigned char *data, unsigned char *xdata,
>> 			  size_t size,
>> 			  struct image_difference_statistics *statistics)
>> {
>>   unsigned char channel_a, channel_b;
>>   int diff;
>>   size_t i;
>>
>>   for (i = 0; i < size; ++i)
>>     {
>>       channel_a = data[i];
>>       channel_b = xdata[i];
>>
>>       diff = (int) channel_b - (int) channel_a;
>>       statistics->min_diff = MIN (statistics->min_diff,
>> 				  diff);
>>       statistics->max_diff = MAX (statistics->max_diff,
>> 				  diff);
>>     }
>> }
>>
>> the "i" in "size_t i" is fontified as an identifier the first time, but
>> not in future appearances in the function.
>
> In the first appearance, i is a declaration, so it’s fontified. Like the
> previous case, not all i are declaration or assignment, so they are not
> all fontified. And now you can enable the "variable" feture to fontify
> all of them.
>
>>
>> Within src/xterm.c, every appearance of ATOM_REFS_INIT is fontified in
>> the error face.  Also, inside `xm_setup_dnd_targets',
>>
>>   xm_targets_table_header header;
>>   xm_targets_table_rec **recs UNINIT;
>>   xm_byte_order byteorder;
>
> I think tree-sitter isn’t expecting another identifier (UNINIT) after
> recs. Here if we don’t enable highlighting errors recs would be
> highlighted fine, as an identifier.
>
>>
>> "recs" is fontified in the error face.
>>
>> 	      recs = xmalloc (sizeof *recs);
>> 	      recs[0] = xmalloc (FLEXSIZEOF (struct xm_targets_table_rec,
>> 					     targets, ntargets * 4));
>>
>> 	      recs[0]->n_targets = ntargets;
>>
>> "struct" is fontified in the error face.  Within
>> x_sync_note_frame_times:
>
> Tree-sitter thinks FLEXSIZEOF is a function call and struct
> xm_targets_table_rec is an argument. We can add a heuristic here:
> (ERROR (identifier)) is either type or struct/enum keyword. I should be
> able to fix that.
>
>> #ifdef FRAME_DEBUG
>>   uint_fast64_t last_frame_ms = output->last_frame_time / 1000;
>>   fprintf (stderr,
>> 	   "Drawing the last frame took: %"PRIuFAST64" ms (%"PRIuFAST64")\n",
>> 	   last_frame_ms, time);
>> #endif
>>
>> "PRIuFAST64" is fontified in the error face.

> Is this valid C? If it it, we should report to tree-sitter-c. If not, we
> can add a heuristic.

This is valid C.  PRIuFAST64 is a standard macro containing the format
specifier for a uint_fast64_t.

> It fontifies fine here. I guess it’s fixes by some changes I made,
> although I don’t recall making changes related to if statements.

Maybe you scrolled up the file instead of down?  That sometimes matters.

> The "else" is separated from the "if" in the parse tree by the
> preprocessor directive, I’ll report this to tree-sitter-c and see if
> they can do anything.
>
>>
>> static int
>> handle_one_xevent (struct x_display_info *dpyinfo,
>> #ifndef HAVE_XINPUT2
>> 		   const XEvent *event,
>> #else
>> 		   XEvent *event,
>> #endif
>> 		   int *finish, struct input_event *hold_quit)
>>
>> the "const XEvent *" is fontified in the error face.
>
> Similar to the previous case. There could be better ways to handle
> ifdef’s in tree-sitter-c, I don’t know if they are easy to implement.
>
>
>> Here:
>>
>>     case GraphicsExpose:	/* This occurs when an XCopyArea's
>>                                    source area was obscured or not
>>                                    available.  */
>>       f = x_window_to_frame (dpyinfo, event->xgraphicsexpose.drawable);
>>       if (f)
>>         {
>>           expose_frame (f, event->xgraphicsexpose.x,
>>                         event->xgraphicsexpose.y,
>>                         event->xgraphicsexpose.width,
>>                         event->xgraphicsexpose.height);
>> #ifndef USE_TOOLKIT_SCROLL_BARS
>> 	  x_scroll_bar_handle_exposure (f, (XEvent *) event);
>> #endif
>> #ifdef USE_GTK
>> 	  x_clear_under_internal_border (f);
>> #endif
>> #ifdef HAVE_XDBE
>> 	  show_back_buffer (f);
>> #endif
>>         }
>> #ifdef USE_X_TOOLKIT
>>       else
>>         goto OTHER;
>> #endif /* USE_X_TOOLKIT */
>>       break;
>>
>> "goto OTHER" is fontified in the error face, and "else" as a type.
>
> Same as above.
>
>> Here:
>>
>> #ifdef HAVE_XINPUT2
>> 	      if (event->xkey.time == pending_keystroke_time)
>> 		{
>> 		  source = xi_device_from_id (dpyinfo,
>> 					      dpyinfo->pending_keystroke_source);
>>
>> 		  if (source)
>> 		    inev.ie.device = source->name;
>> 		}
>> #endif
>>
>> "inev" is fontified in the error face.
>
> Fontifies fine here, should be fixed.
>
>> Here:
>>
>> #ifdef USE_GTK
>>       /* See comment in EnterNotify above */
>>       else if (dpyinfo->last_mouse_glyph_frame)
>>         x_note_mouse_movement (dpyinfo->last_mouse_glyph_frame,
>> 			       &event->xmotion, Qnil);
>> #endif
>>
>> "else" and "dpyinfo" are fontified as types.
>
> Same.
>
>> #ifdef USE_MOTIF
>> 	    Widget widget;
>>
>> 	    widget = XtWindowToWidget (dpyinfo->display,
>> 				       event->xbutton.window);
>>
>> 	    if (widget && XmIsCascadeButton (widget)
>> 		&& XtIsSensitive (widget))
>> 	      {
>> #endif
>> 		if (!f->output_data.x->saved_menu_event)
>> 		  f->output_data.x->saved_menu_event = xmalloc (sizeof *event);
>> 		*f->output_data.x->saved_menu_event = *event;
>> 		inev.ie.kind = MENU_BAR_ACTIVATE_EVENT;
>> 		XSETFRAME (inev.ie.frame_or_window, f);
>> 		*finish = X_EVENT_DROP;
>> #ifdef USE_MOTIF
>> 	      }
>> #endif
>>
>> here, the last closing brace is fontified in the error face.
>
> Still, blame #ifdef. I don’t have good ideas for these.

Can't #ifdef simply not be passed to tree-sitter, if it is so bad at
understanding them?