Is it time to remove INTERNAL_FIELD?

Is it time to remove INTERNAL_FIELD?

[Oleh Krehel]

Hi all,

In an effort to make the mark and mark-active be window-local (like the
point) and not buffer-local, I've started learning the way that the
Emacs C code is organized.

This macro looks to me as more confusing than useful:

    #define INTERNAL_FIELD(field) field ## _

It's got <200 uses, and this comment, dated 2012:

    /* Deprecated and will be removed soon.  */

Can I remove it? I suppose the way to do it is just to expand all
occurences?

Oleh

Re: Is it time to remove INTERNAL_FIELD?

[Paul Eggert]

Oleh Krehel wrote:
> Can I remove it? I suppose the way to do it is just to expand all
> occurences?

I think it's time to remove it, yes.  Simply replace 'INTERNAL_FIELD (foo)' with 
'foo' everywhere, and remove the #define.

Re: Is it time to remove INTERNAL_FIELD?

[Oleh Krehel]

Paul Eggert <eggert@cs.ucla.edu> writes:

> Oleh Krehel wrote:
>> Can I remove it? I suppose the way to do it is just to expand all
>> occurences?
>
> I think it's time to remove it, yes.  Simply replace 'INTERNAL_FIELD
> (foo)' with 'foo' everywhere, and remove the #define.

Should it be 'foo' or 'foo_'?

Oleh

Re: Is it time to remove INTERNAL_FIELD?

[Paul Eggert]

Oleh Krehel wrote:
> Should it be 'foo' or 'foo_'?

The former, surely.  There should be no need for the trailing underscore.

Re: Is it time to remove INTERNAL_FIELD?

[Eli Zaretskii]

> Date: Thu, 23 Apr 2015 03:17:36 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> Cc: emacs-devel@gnu.org
> 
> Oleh Krehel wrote:
> > Should it be 'foo' or 'foo_'?
> 
> The former, surely.  There should be no need for the trailing underscore.

Please don't forget to change src/.gdbinit to follow suit.  There's
nothing more annoying than have .gdbinit commands broken by such
changes, since you discover this in the middle of a debugging session,
and they manifest themselves by cryptic error messages.

Re: Is it time to remove INTERNAL_FIELD?

[Oleh Krehel]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Thu, 23 Apr 2015 03:17:36 -0700
>> From: Paul Eggert <eggert@cs.ucla.edu>
>> Cc: emacs-devel@gnu.org
>> 
>> Oleh Krehel wrote:
>> > Should it be 'foo' or 'foo_'?
>> 
>> The former, surely.  There should be no need for the trailing underscore.
>
> Please don't forget to change src/.gdbinit to follow suit.  There's
> nothing more annoying than have .gdbinit commands broken by such
> changes, since you discover this in the middle of a debugging session,
> and they manifest themselves by cryptic error messages.

The change is in scratch/remove-internal-field. I've compiled and it
works properly. I don't know what to do with src/.gdbinit though.

Oleh

Re: Is it time to remove INTERNAL_FIELD?

[Eli Zaretskii]

> From: Oleh Krehel <ohwoeowho@gmail.com>
> Cc: Paul Eggert <eggert@cs.ucla.edu>,  emacs-devel@gnu.org
> Date: Thu, 23 Apr 2015 12:56:38 +0200
> 
> I don't know what to do with src/.gdbinit though.

Any reference to SOMETHING_ there should be replaced with SOMETHING
now.  Otherwise GDB will say there's no field named SOMETHING_ in
such-and-such struct.  E.g., try this:

 $ cd src
 $ gdb ./emacs
 (gdb) break set_cursor_from_row
 (gdb) r -Q
 Breakpoint 3, set_cursor_from_row (w=0x17ceae8 <dumped_data+2395368>,
    row=0x52740b8, matrix=0x5273be8, delta=0, delta_bytes=0, dy=0, dvpos=0)
    at xdisp.c:14185
 14185     struct glyph *glyph = row->glyphs[TEXT_AREA];
 (gdb) p w->contents
 $2 = 16446781
 (gdb) xbuffer
 $3 = (struct buffer *) 0xfaf538
 (unsigned char *) 0xfb8a80 " *Echo Area 1*"
                            ^^^^^^^^^^^^^^^^
The highlighted part access the buffer name via the name_ field.
After your change, you will see something like this instead:

  (gdb) xbuffer
  $2 = (struct buffer *) 0xfaf538
  There is no member named name_.

More generally, src/.gdbinit defines additional commands for GDB that
are useful for debugging Emacs, and in particular there are commands
there to display Lisp objects in human-readable format, like the
'xbuffer' command I used above.  So any changes in struct field names
should be reflected there, or else those commands will become broken.

Re: Is it time to remove INTERNAL_FIELD?

[Oleh Krehel]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Oleh Krehel <ohwoeowho@gmail.com>
>> Cc: Paul Eggert <eggert@cs.ucla.edu>,  emacs-devel@gnu.org
>> Date: Thu, 23 Apr 2015 12:56:38 +0200
>> 
>> I don't know what to do with src/.gdbinit though.
>
> Any reference to SOMETHING_ there should be replaced with SOMETHING
> now.  Otherwise GDB will say there's no field named SOMETHING_ in
> such-and-such struct.  E.g., try this:
>
>  $ cd src
>  $ gdb ./emacs
>  (gdb) break set_cursor_from_row
>  (gdb) r -Q
>  Breakpoint 3, set_cursor_from_row (w=0x17ceae8 <dumped_data+2395368>,
>     row=0x52740b8, matrix=0x5273be8, delta=0, delta_bytes=0, dy=0, dvpos=0)
>     at xdisp.c:14185
>  14185     struct glyph *glyph = row->glyphs[TEXT_AREA];
>  (gdb) p w->contents
>  $2 = 16446781
>  (gdb) xbuffer

What does xbuffer do? It shows up as unrecognized command for me.
I see it defined in .gdbinit, but it's automatically sourced. How can I
source it?

>  $3 = (struct buffer *) 0xfaf538
>  (unsigned char *) 0xfb8a80 " *Echo Area 1*"
>                             ^^^^^^^^^^^^^^^^
> The highlighted part access the buffer name via the name_ field.
> After your change, you will see something like this instead:
>
>   (gdb) xbuffer
>   $2 = (struct buffer *) 0xfaf538
>   There is no member named name_.

I saw that there was a change 3 years ago adding "_" to ends of some
symbols. This change was subsequently reverted, and it supposedly worked
fine for 3 years. So I'm guessing that there is a mechanism that makes
it work without modifying with symbols with "_".

Oleh

Re: Is it time to remove INTERNAL_FIELD?

[Eli Zaretskii]

> From: Oleh Krehel <ohwoeowho@gmail.com>
> Cc: eggert@cs.ucla.edu,  emacs-devel@gnu.org
> Date: Thu, 23 Apr 2015 13:32:32 +0200
> 
> >  (gdb) p w->contents
> >  $2 = 16446781
> >  (gdb) xbuffer
> 
> What does xbuffer do?

See its doc string in .gdbinit: it shows a Lisp buffer object in
human-readable format.  Like other xSOMETHING commands do for other
objects.

> It shows up as unrecognized command for me.  I see it defined in
> .gdbinit, but it's automatically sourced. How can I source it?

You can source it manually like this:

 (gdb) source .gdbinit

If you start GDB from the src directory, it should read that file
automatically.  However, latest versions of GDB refuse to do that, for
security reasons, unless you customize GDB to countermand that (the
message GDB displays when it refuses to load the file hints on these
customizations; read the GDB manual to know more).

See etc/DEBUG for more about debugging Emacs on the C level.

> I saw that there was a change 3 years ago adding "_" to ends of some
> symbols. This change was subsequently reverted, and it supposedly worked
> fine for 3 years. So I'm guessing that there is a mechanism that makes
> it work without modifying with symbols with "_".

No, there is no such mechanism.  You will find "name_" in .gdbinit.
Those changes you mention are probably about changes in the sources
that were reverted later.

Re: Is it time to remove INTERNAL_FIELD?

[Oleh Krehel]

Eli Zaretskii <eliz@gnu.org> writes:

>> It shows up as unrecognized command for me.  I see it defined in
>> .gdbinit, but it's automatically sourced. How can I source it?
>
> You can source it manually like this:
>
>  (gdb) source .gdbinit
>
> If you start GDB from the src directory, it should read that file
> automatically.  However, latest versions of GDB refuse to do that, for
> security reasons, unless you customize GDB to countermand that (the
> message GDB displays when it refuses to load the file hints on these
> customizations; read the GDB manual to know more).
>
> See etc/DEBUG for more about debugging Emacs on the C level.

Thanks.

>> I saw that there was a change 3 years ago adding "_" to ends of some
>> symbols. This change was subsequently reverted, and it supposedly worked
>> fine for 3 years. So I'm guessing that there is a mechanism that makes
>> it work without modifying with symbols with "_".
>
> No, there is no such mechanism.  You will find "name_" in .gdbinit.
> Those changes you mention are probably about changes in the sources
> that were reverted later.

OK, I see it now. The only two occurences that I found for
"_\([^a-z]\|$\)" were "name_". I've pushed the change.
Is there anything else that needs to be done before merging?

Oleh

Re: Is it time to remove INTERNAL_FIELD?

[Eli Zaretskii]

> From: Oleh Krehel <ohwoeowho@gmail.com>
> Cc: eggert@cs.ucla.edu,  emacs-devel@gnu.org
> Date: Thu, 23 Apr 2015 14:05:28 +0200
> 
> Is there anything else that needs to be done before merging?

Please wait for the discussion of this issue to settle down:

> Wasn't that underscore introduced for the concurrency branch?  E.g.,
> see KVAR and BVAR macros.  I think the idea was to prevent people from
> writing code that directly accesses these fields, which is bad for
> concurrency.

It's possible that we will want the underscore to be left alone, at
least in some structures.

Re: Is it time to remove INTERNAL_FIELD?

[Oleh Krehel]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Oleh Krehel <ohwoeowho@gmail.com>
>> Cc: eggert@cs.ucla.edu,  emacs-devel@gnu.org
>> Date: Thu, 23 Apr 2015 14:05:28 +0200
>> 
>> Is there anything else that needs to be done before merging?
>
> Please wait for the discussion of this issue to settle down:

Is there a decision?

Oleh

Re: Is it time to remove INTERNAL_FIELD?

[Eli Zaretskii]

> From: Oleh Krehel <ohwoeowho@gmail.com>
> Cc: eggert@cs.ucla.edu,  emacs-devel@gnu.org
> Date: Sat, 25 Apr 2015 12:57:42 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Oleh Krehel <ohwoeowho@gmail.com>
> >> Cc: eggert@cs.ucla.edu,  emacs-devel@gnu.org
> >> Date: Thu, 23 Apr 2015 14:05:28 +0200
> >> 
> >> Is there anything else that needs to be done before merging?
> >
> > Please wait for the discussion of this issue to settle down:
> 
> Is there a decision?

I'd like to know as well.  So far no one objected to leaving the
trailing underscores for the sake of the concurrency branch.  So
unless someone does come up with objections soon, I'd interpret that
as meaning that BVAR and KVAR should still append the underscore, and
the fields marked INTERNAL till today should have that underscore.

Re: Is it time to remove INTERNAL_FIELD?

[Stefan Monnier]

> I'd like to know as well.  So far no one objected to leaving the
> trailing underscores for the sake of the concurrency branch.  So
> unless someone does come up with objections soon, I'd interpret that
> as meaning that BVAR and KVAR should still append the underscore, and
> the fields marked INTERNAL till today should have that underscore.

I'm not opposed to removing INTERNAL_FIELD and wouldn't oppose removing
the underscore either.  But if Eli wants to keep either of them that's
fine by me as well.
The only problem I have with this is the fact that we're just going
back&forth on this.


        Stefan

Re: Is it time to remove INTERNAL_FIELD?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: Oleh Krehel <ohwoeowho@gmail.com>, eggert@cs.ucla.edu, emacs-devel@gnu.org
> Date: Sat, 25 Apr 2015 10:28:02 -0400
> 
> > I'd like to know as well.  So far no one objected to leaving the
> > trailing underscores for the sake of the concurrency branch.  So
> > unless someone does come up with objections soon, I'd interpret that
> > as meaning that BVAR and KVAR should still append the underscore, and
> > the fields marked INTERNAL till today should have that underscore.
> 
> I'm not opposed to removing INTERNAL_FIELD and wouldn't oppose removing
> the underscore either.  But if Eli wants to keep either of them that's
> fine by me as well.

Unless the concurrency branch is officially and finally dead, I'd like
to keep the underscores, to make it easier to revive that branch, if
and when some volunteer emerges.

> The only problem I have with this is the fact that we're just going
> back&forth on this.

We didn't yet get back on this, except in the
scratch/remove-internal-field branch.

Re: Is it time to remove INTERNAL_FIELD?

[Oleh Krehel]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
>> Cc: Oleh Krehel <ohwoeowho@gmail.com>, eggert@cs.ucla.edu, emacs-devel@gnu.org
>> Date: Sat, 25 Apr 2015 10:28:02 -0400
>> 
>> > I'd like to know as well.  So far no one objected to leaving the
>> > trailing underscores for the sake of the concurrency branch.  So
>> > unless someone does come up with objections soon, I'd interpret that
>> > as meaning that BVAR and KVAR should still append the underscore, and
>> > the fields marked INTERNAL till today should have that underscore.
>> 
>> I'm not opposed to removing INTERNAL_FIELD and wouldn't oppose removing
>> the underscore either.  But if Eli wants to keep either of them that's
>> fine by me as well.
>
> Unless the concurrency branch is officially and finally dead, I'd like
> to keep the underscores, to make it easier to revive that branch, if
> and when some volunteer emerges.

I think that a good start towards concurrency is making the code as
simple as possible. Removing INTERNAL_FIELD is a tiny step towards that
goal.

Perhaps, removing some other abstraction mechanisms that don't actually
do anything yet (but potentially could be useful in the future) would
also be good. Concurrency would come with its own abstractions, I think
the old ones would just get in the way.

>> The only problem I have with this is the fact that we're just going
>> back&forth on this.
>
> We didn't yet get back on this, except in the
> scratch/remove-internal-field branch.

Should I update the patch to include the underscores everywhere?

Oleh

Re: Is it time to remove INTERNAL_FIELD?

[Stefan Monnier]

> I think that a good start towards concurrency is making the code as
> simple as possible.

The `concurrency' branch is a good bit further than "a start", IIRC.


        Stefan

Re: Is it time to remove INTERNAL_FIELD?

[Eli Zaretskii]

> From: Oleh Krehel <ohwoeowho@gmail.com>
> Cc: Stefan Monnier <monnier@IRO.UMontreal.CA>,  eggert@cs.ucla.edu,  emacs-devel@gnu.org
> Date: Tue, 28 Apr 2015 09:39:10 +0200
> 
> > Unless the concurrency branch is officially and finally dead, I'd like
> > to keep the underscores, to make it easier to revive that branch, if
> > and when some volunteer emerges.
> 
> I think that a good start towards concurrency is making the code as
> simple as possible. Removing INTERNAL_FIELD is a tiny step towards that
> goal.

I'm not against removing INTERNAL_FIELD.  I was only talking about
keeping the code that appends the underscores to fields in buffer and
keyboard structures where we were doing that with INTERNAL_FIELD.

> Perhaps, removing some other abstraction mechanisms that don't actually
> do anything yet (but potentially could be useful in the future) would
> also be good. Concurrency would come with its own abstractions, I think
> the old ones would just get in the way.

We append the underscores not as some abstraction, but as an aid to
catch early code that will interfere with merging the concurrency
branch.

> Should I update the patch to include the underscores everywhere?

Not everywhere, only where the fields are used in BVAR and KVAR.

Thanks.

Re: Is it time to remove INTERNAL_FIELD?

[Oleh Krehel]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Oleh Krehel <ohwoeowho@gmail.com>
>> Cc: Stefan Monnier <monnier@IRO.UMontreal.CA>,  eggert@cs.ucla.edu,  emacs-devel@gnu.org
>> Date: Tue, 28 Apr 2015 09:39:10 +0200
>> 
>> > Unless the concurrency branch is officially and finally dead, I'd like
>> > to keep the underscores, to make it easier to revive that branch, if
>> > and when some volunteer emerges.
>> 
>> I think that a good start towards concurrency is making the code as
>> simple as possible. Removing INTERNAL_FIELD is a tiny step towards that
>> goal.
>
> I'm not against removing INTERNAL_FIELD.  I was only talking about
> keeping the code that appends the underscores to fields in buffer and
> keyboard structures where we were doing that with INTERNAL_FIELD.
>
>> Perhaps, removing some other abstraction mechanisms that don't actually
>> do anything yet (but potentially could be useful in the future) would
>> also be good. Concurrency would come with its own abstractions, I think
>> the old ones would just get in the way.
>
> We append the underscores not as some abstraction, but as an aid to
> catch early code that will interfere with merging the concurrency
> branch.
>
>> Should I update the patch to include the underscores everywhere?
>
> Not everywhere, only where the fields are used in BVAR and KVAR.

So that means everywhere, except allow to use BVAR and KVAR macros
without an underscore. Did I understand correctly?

Oleh

Re: Is it time to remove INTERNAL_FIELD?

[Eli Zaretskii]

> From: Oleh Krehel <ohwoeowho@gmail.com>
> Cc: monnier@IRO.UMontreal.CA,  eggert@cs.ucla.edu,  emacs-devel@gnu.org
> Date: Tue, 28 Apr 2015 17:11:58 +0200
> 
> >> Should I update the patch to include the underscores everywhere?
> >
> > Not everywhere, only where the fields are used in BVAR and KVAR.
> 
> So that means everywhere

If you say so, I didn't double-check that.

> except allow to use BVAR and KVAR macros without an underscore. Did
> I understand correctly?

Yes, thanks.

Re: Is it time to remove INTERNAL_FIELD?

[Oleh Krehel]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Oleh Krehel <ohwoeowho@gmail.com>
>> Cc: monnier@IRO.UMontreal.CA,  eggert@cs.ucla.edu,  emacs-devel@gnu.org
>> Date: Tue, 28 Apr 2015 17:11:58 +0200
>> 
>> >> Should I update the patch to include the underscores everywhere?
>> >
>> > Not everywhere, only where the fields are used in BVAR and KVAR.
>> 
>> So that means everywhere
>
> If you say so, I didn't double-check that.
>
>> except allow to use BVAR and KVAR macros without an underscore. Did
>> I understand correctly?
>
> Yes, thanks.

I attach the flattened patch. It compiles and runs fine, but please
check it again.

Oleh

===File
/home/oleh/git/gnu-emacs/0001-Remove-the-deprecated-INTERNAL_FIELD-macro-by-expand.patch===
From 8e906175c418b68c07df9f382713a34761092f51 Mon Sep 17 00:00:00 2001
From: Oleh Krehel <ohwoeowho@gmail.com>
Date: Thu, 23 Apr 2015 12:36:22 +0200
Subject: [PATCH] Remove the deprecated INTERNAL_FIELD macro by expanding it

* src/lisp.h (INTERNAL_FIELD): Remove.
(DEFVAR_KBOARD): Modify accordingly.

* alloc.c, buffer.c, buffer.h, category.c, keyboard.c, keyboard.h:
* syntax.c: Adjust users.
* src/buffer.c (compact_buffer): Use BVAR.
---
 src/alloc.c    |   2 +-
 src/buffer.c   |  92 ++++++++++++++--------------
 src/buffer.h   | 188 ++++++++++++++++++++++++++++-----------------------------
 src/category.c |   2 +-
 src/keyboard.c |  18 +++---
 src/keyboard.h |  52 ++++++++--------
 src/lisp.h     |   4 --
 src/syntax.c   |   2 +-
 8 files changed, 178 insertions(+), 182 deletions(-)

diff --git a/src/alloc.c b/src/alloc.c
index 1f4b1a4..688363d 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -4496,7 +4496,7 @@ live_buffer_p (struct mem_node *m, void *p)
      must not have been killed.  */
   return (m->type == MEM_TYPE_BUFFER
 	  && p == m->start
-	  && !NILP (((struct buffer *) p)->INTERNAL_FIELD (name)));
+	  && !NILP (((struct buffer *) p)->name_));
 }
 
 #endif /* GC_MARK_STACK || defined GC_MALLOC_CHECK */
diff --git a/src/buffer.c b/src/buffer.c
index 332d6d5..b09676c 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -134,227 +134,227 @@ CHECK_OVERLAY (Lisp_Object x)
 static void
 bset_abbrev_mode (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (abbrev_mode) = val;
+  b->abbrev_mode_ = val;
 }
 static void
 bset_abbrev_table (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (abbrev_table) = val;
+  b->abbrev_table_ = val;
 }
 static void
 bset_auto_fill_function (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (auto_fill_function) = val;
+  b->auto_fill_function_ = val;
 }
 static void
 bset_auto_save_file_format (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (auto_save_file_format) = val;
+  b->auto_save_file_format_ = val;
 }
 static void
 bset_auto_save_file_name (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (auto_save_file_name) = val;
+  b->auto_save_file_name_ = val;
 }
 static void
 bset_backed_up (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (backed_up) = val;
+  b->backed_up_ = val;
 }
 static void
 bset_begv_marker (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (begv_marker) = val;
+  b->begv_marker_ = val;
 }
 static void
 bset_bidi_display_reordering (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (bidi_display_reordering) = val;
+  b->bidi_display_reordering_ = val;
 }
 static void
 bset_buffer_file_coding_system (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (buffer_file_coding_system) = val;
+  b->buffer_file_coding_system_ = val;
 }
 static void
 bset_case_fold_search (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (case_fold_search) = val;
+  b->case_fold_search_ = val;
 }
 static void
 bset_ctl_arrow (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (ctl_arrow) = val;
+  b->ctl_arrow_ = val;
 }
 static void
 bset_cursor_in_non_selected_windows (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (cursor_in_non_selected_windows) = val;
+  b->cursor_in_non_selected_windows_ = val;
 }
 static void
 bset_cursor_type (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (cursor_type) = val;
+  b->cursor_type_ = val;
 }
 static void
 bset_display_table (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (display_table) = val;
+  b->display_table_ = val;
 }
 static void
 bset_extra_line_spacing (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (extra_line_spacing) = val;
+  b->extra_line_spacing_ = val;
 }
 static void
 bset_file_format (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (file_format) = val;
+  b->file_format_ = val;
 }
 static void
 bset_file_truename (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (file_truename) = val;
+  b->file_truename_ = val;
 }
 static void
 bset_fringe_cursor_alist (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (fringe_cursor_alist) = val;
+  b->fringe_cursor_alist_ = val;
 }
 static void
 bset_fringe_indicator_alist (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (fringe_indicator_alist) = val;
+  b->fringe_indicator_alist_ = val;
 }
 static void
 bset_fringes_outside_margins (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (fringes_outside_margins) = val;
+  b->fringes_outside_margins_ = val;
 }
 static void
 bset_header_line_format (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (header_line_format) = val;
+  b->header_line_format_ = val;
 }
 static void
 bset_indicate_buffer_boundaries (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (indicate_buffer_boundaries) = val;
+  b->indicate_buffer_boundaries_ = val;
 }
 static void
 bset_indicate_empty_lines (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (indicate_empty_lines) = val;
+  b->indicate_empty_lines_ = val;
 }
 static void
 bset_invisibility_spec (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (invisibility_spec) = val;
+  b->invisibility_spec_ = val;
 }
 static void
 bset_left_fringe_width (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (left_fringe_width) = val;
+  b->left_fringe_width_ = val;
 }
 static void
 bset_major_mode (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (major_mode) = val;
+  b->major_mode_ = val;
 }
 static void
 bset_mark (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (mark) = val;
+  b->mark_ = val;
 }
 static void
 bset_minor_modes (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (minor_modes) = val;
+  b->minor_modes_ = val;
 }
 static void
 bset_mode_line_format (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (mode_line_format) = val;
+  b->mode_line_format_ = val;
 }
 static void
 bset_mode_name (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (mode_name) = val;
+  b->mode_name_ = val;
 }
 static void
 bset_name (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (name) = val;
+  b->name_ = val;
 }
 static void
 bset_overwrite_mode (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (overwrite_mode) = val;
+  b->overwrite_mode_ = val;
 }
 static void
 bset_pt_marker (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (pt_marker) = val;
+  b->pt_marker_ = val;
 }
 static void
 bset_right_fringe_width (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (right_fringe_width) = val;
+  b->right_fringe_width_ = val;
 }
 static void
 bset_save_length (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (save_length) = val;
+  b->save_length_ = val;
 }
 static void
 bset_scroll_bar_width (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (scroll_bar_width) = val;
+  b->scroll_bar_width_ = val;
 }
 static void
 bset_scroll_bar_height (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (scroll_bar_height) = val;
+  b->scroll_bar_height_ = val;
 }
 static void
 bset_scroll_down_aggressively (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (scroll_down_aggressively) = val;
+  b->scroll_down_aggressively_ = val;
 }
 static void
 bset_scroll_up_aggressively (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (scroll_up_aggressively) = val;
+  b->scroll_up_aggressively_ = val;
 }
 static void
 bset_selective_display (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (selective_display) = val;
+  b->selective_display_ = val;
 }
 static void
 bset_selective_display_ellipses (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (selective_display_ellipses) = val;
+  b->selective_display_ellipses_ = val;
 }
 static void
 bset_vertical_scroll_bar_type (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (vertical_scroll_bar_type) = val;
+  b->vertical_scroll_bar_type_ = val;
 }
 static void
 bset_horizontal_scroll_bar_type (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (horizontal_scroll_bar_type) = val;
+  b->horizontal_scroll_bar_type_ = val;
 }
 static void
 bset_word_wrap (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (word_wrap) = val;
+  b->word_wrap_ = val;
 }
 static void
 bset_zv_marker (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (zv_marker) = val;
+  b->zv_marker_ = val;
 }
 
 void
@@ -1591,7 +1591,7 @@ compact_buffer (struct buffer *buffer)
 	 turned off in that buffer.  Calling truncate_undo_list on
 	 Qt tends to return NULL, which effectively turns undo back on.
 	 So don't call truncate_undo_list if undo_list is Qt.  */
-      if (!EQ (buffer->INTERNAL_FIELD (undo_list), Qt))
+      if (!EQ (BVAR(buffer, undo_list), Qt))
 	truncate_undo_list (buffer);
 
       /* Shrink buffer gaps.  */
diff --git a/src/buffer.h b/src/buffer.h
index 81852ca..a0410d4 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -483,7 +483,7 @@ struct buffer_text
 
 /* Most code should use this macro to access Lisp fields in struct buffer.  */
 
-#define BVAR(buf, field) ((buf)->INTERNAL_FIELD (field))
+#define BVAR(buf, field) ((buf)->field ## _)
 
 /* This is the structure that the buffer Lisp object points to.  */
 
@@ -492,17 +492,17 @@ struct buffer
   struct vectorlike_header header;
 
   /* The name of this buffer.  */
-  Lisp_Object INTERNAL_FIELD (name);
+  Lisp_Object name_;
 
   /* The name of the file visited in this buffer, or nil.  */
-  Lisp_Object INTERNAL_FIELD (filename);
+  Lisp_Object filename_;
 
   /* Directory for expanding relative file names.  */
-  Lisp_Object INTERNAL_FIELD (directory);
+  Lisp_Object directory_;
 
   /* True if this buffer has been backed up (if you write to the visited
      file and it hasn't been backed up, then a backup will be made).  */
-  Lisp_Object INTERNAL_FIELD (backed_up);
+  Lisp_Object backed_up_;
 
   /* Length of file when last read or saved.
      -1 means auto saving turned off because buffer shrank a lot.
@@ -510,132 +510,132 @@ struct buffer
        (That value is used with buffer-swap-text.)
      This is not in the  struct buffer_text
      because it's not used in indirect buffers at all.  */
-  Lisp_Object INTERNAL_FIELD (save_length);
+  Lisp_Object save_length_;
 
   /* File name used for auto-saving this buffer.
      This is not in the  struct buffer_text
      because it's not used in indirect buffers at all.  */
-  Lisp_Object INTERNAL_FIELD (auto_save_file_name);
+  Lisp_Object auto_save_file_name_;
 
   /* Non-nil if buffer read-only.  */
-  Lisp_Object INTERNAL_FIELD (read_only);
+  Lisp_Object read_only_;
 
   /* "The mark".  This is a marker which may
      point into this buffer or may point nowhere.  */
-  Lisp_Object INTERNAL_FIELD (mark);
+  Lisp_Object mark_;
 
   /* Alist of elements (SYMBOL . VALUE-IN-THIS-BUFFER) for all
      per-buffer variables of this buffer.  For locally unbound
      symbols, just the symbol appears as the element.  */
-  Lisp_Object INTERNAL_FIELD (local_var_alist);
+  Lisp_Object local_var_alist_;
 
   /* Symbol naming major mode (e.g., lisp-mode).  */
-  Lisp_Object INTERNAL_FIELD (major_mode);
+  Lisp_Object major_mode_;
 
   /* Pretty name of major mode (e.g., "Lisp"). */
-  Lisp_Object INTERNAL_FIELD (mode_name);
+  Lisp_Object mode_name_;
 
   /* Mode line element that controls format of mode line.  */
-  Lisp_Object INTERNAL_FIELD (mode_line_format);
+  Lisp_Object mode_line_format_;
 
   /* Analogous to mode_line_format for the line displayed at the top
      of windows.  Nil means don't display that line.  */
-  Lisp_Object INTERNAL_FIELD (header_line_format);
+  Lisp_Object header_line_format_;
 
   /* Keys that are bound local to this buffer.  */
-  Lisp_Object INTERNAL_FIELD (keymap);
+  Lisp_Object keymap_;
 
   /* This buffer's local abbrev table.  */
-  Lisp_Object INTERNAL_FIELD (abbrev_table);
+  Lisp_Object abbrev_table_;
 
   /* This buffer's syntax table.  */
-  Lisp_Object INTERNAL_FIELD (syntax_table);
+  Lisp_Object syntax_table_;
 
   /* This buffer's category table.  */
-  Lisp_Object INTERNAL_FIELD (category_table);
+  Lisp_Object category_table_;
 
   /* Values of several buffer-local variables.  */
   /* tab-width is buffer-local so that redisplay can find it
      in buffers that are not current.  */
-  Lisp_Object INTERNAL_FIELD (case_fold_search);
-  Lisp_Object INTERNAL_FIELD (tab_width);
-  Lisp_Object INTERNAL_FIELD (fill_column);
-  Lisp_Object INTERNAL_FIELD (left_margin);
+  Lisp_Object case_fold_search_;
+  Lisp_Object tab_width_;
+  Lisp_Object fill_column_;
+  Lisp_Object left_margin_;
 
   /* Function to call when insert space past fill column.  */
-  Lisp_Object INTERNAL_FIELD (auto_fill_function);
+  Lisp_Object auto_fill_function_;
 
   /* Case table for case-conversion in this buffer.
      This char-table maps each char into its lower-case version.  */
-  Lisp_Object INTERNAL_FIELD (downcase_table);
+  Lisp_Object downcase_table_;
 
   /* Char-table mapping each char to its upper-case version.  */
-  Lisp_Object INTERNAL_FIELD (upcase_table);
+  Lisp_Object upcase_table_;
 
   /* Char-table for conversion for case-folding search.  */
-  Lisp_Object INTERNAL_FIELD (case_canon_table);
+  Lisp_Object case_canon_table_;
 
   /* Char-table of equivalences for case-folding search.  */
-  Lisp_Object INTERNAL_FIELD (case_eqv_table);
+  Lisp_Object case_eqv_table_;
 
   /* Non-nil means do not display continuation lines.  */
-  Lisp_Object INTERNAL_FIELD (truncate_lines);
+  Lisp_Object truncate_lines_;
 
   /* Non-nil means to use word wrapping when displaying continuation lines.  */
-  Lisp_Object INTERNAL_FIELD (word_wrap);
+  Lisp_Object word_wrap_;
 
   /* Non-nil means display ctl chars with uparrow.  */
-  Lisp_Object INTERNAL_FIELD (ctl_arrow);
+  Lisp_Object ctl_arrow_;
 
   /* Non-nil means reorder bidirectional text for display in the
      visual order.  */
-  Lisp_Object INTERNAL_FIELD (bidi_display_reordering);
+  Lisp_Object bidi_display_reordering_;
 
   /* If non-nil, specifies which direction of text to force in all the
      paragraphs of the buffer.  Nil means determine paragraph
      direction dynamically for each paragraph.  */
-  Lisp_Object INTERNAL_FIELD (bidi_paragraph_direction);
+  Lisp_Object bidi_paragraph_direction_;
 
   /* Non-nil means do selective display;
      see doc string in syms_of_buffer (buffer.c) for details.  */
-  Lisp_Object INTERNAL_FIELD (selective_display);
+  Lisp_Object selective_display_;
 
   /* Non-nil means show ... at end of line followed by invisible lines.  */
-  Lisp_Object INTERNAL_FIELD (selective_display_ellipses);
+  Lisp_Object selective_display_ellipses_;
 
   /* Alist of (FUNCTION . STRING) for each minor mode enabled in buffer.  */
-  Lisp_Object INTERNAL_FIELD (minor_modes);
+  Lisp_Object minor_modes_;
 
   /* t if "self-insertion" should overwrite; `binary' if it should also
      overwrite newlines and tabs - for editing executables and the like.  */
-  Lisp_Object INTERNAL_FIELD (overwrite_mode);
+  Lisp_Object overwrite_mode_;
 
   /* Non-nil means abbrev mode is on.  Expand abbrevs automatically.  */
-  Lisp_Object INTERNAL_FIELD (abbrev_mode);
+  Lisp_Object abbrev_mode_;
 
   /* Display table to use for text in this buffer.  */
-  Lisp_Object INTERNAL_FIELD (display_table);
+  Lisp_Object display_table_;
 
   /* t means the mark and region are currently active.  */
-  Lisp_Object INTERNAL_FIELD (mark_active);
+  Lisp_Object mark_active_;
 
   /* Non-nil means the buffer contents are regarded as multi-byte
      form of characters, not a binary code.  */
-  Lisp_Object INTERNAL_FIELD (enable_multibyte_characters);
+  Lisp_Object enable_multibyte_characters_;
 
   /* Coding system to be used for encoding the buffer contents on
      saving.  */
-  Lisp_Object INTERNAL_FIELD (buffer_file_coding_system);
+  Lisp_Object buffer_file_coding_system_;
 
   /* List of symbols naming the file format used for visited file.  */
-  Lisp_Object INTERNAL_FIELD (file_format);
+  Lisp_Object file_format_;
 
   /* List of symbols naming the file format used for auto-save file.  */
-  Lisp_Object INTERNAL_FIELD (auto_save_file_format);
+  Lisp_Object auto_save_file_format_;
 
   /* True if the newline position cache, width run cache and BIDI paragraph
      cache are enabled.  See search.c, indent.c and bidi.c for details.  */
-  Lisp_Object INTERNAL_FIELD (cache_long_scans);
+  Lisp_Object cache_long_scans_;
 
   /* If the width run cache is enabled, this table contains the
      character widths width_run_cache (see above) assumes.  When we
@@ -643,104 +643,104 @@ struct buffer
      current display table to see whether the display table has
      affected the widths of any characters.  If it has, we
      invalidate the width run cache, and re-initialize width_table.  */
-  Lisp_Object INTERNAL_FIELD (width_table);
+  Lisp_Object width_table_;
 
   /* In an indirect buffer, or a buffer that is the base of an
      indirect buffer, this holds a marker that records
      PT for this buffer when the buffer is not current.  */
-  Lisp_Object INTERNAL_FIELD (pt_marker);
+  Lisp_Object pt_marker_;
 
   /* In an indirect buffer, or a buffer that is the base of an
      indirect buffer, this holds a marker that records
      BEGV for this buffer when the buffer is not current.  */
-  Lisp_Object INTERNAL_FIELD (begv_marker);
+  Lisp_Object begv_marker_;
 
   /* In an indirect buffer, or a buffer that is the base of an
      indirect buffer, this holds a marker that records
      ZV for this buffer when the buffer is not current.  */
-  Lisp_Object INTERNAL_FIELD (zv_marker);
+  Lisp_Object zv_marker_;
 
   /* This holds the point value before the last scroll operation.
      Explicitly setting point sets this to nil.  */
-  Lisp_Object INTERNAL_FIELD (point_before_scroll);
+  Lisp_Object point_before_scroll_;
 
   /* Truename of the visited file, or nil.  */
-  Lisp_Object INTERNAL_FIELD (file_truename);
+  Lisp_Object file_truename_;
 
   /* Invisibility spec of this buffer.
      t => any non-nil `invisible' property means invisible.
      A list => `invisible' property means invisible
      if it is memq in that list.  */
-  Lisp_Object INTERNAL_FIELD (invisibility_spec);
+  Lisp_Object invisibility_spec_;
 
   /* This is the last window that was selected with this buffer in it,
      or nil if that window no longer displays this buffer.  */
-  Lisp_Object INTERNAL_FIELD (last_selected_window);
+  Lisp_Object last_selected_window_;
 
   /* Incremented each time the buffer is displayed in a window.  */
-  Lisp_Object INTERNAL_FIELD (display_count);
+  Lisp_Object display_count_;
 
   /* Widths of left and right marginal areas for windows displaying
      this buffer.  */
-  Lisp_Object INTERNAL_FIELD (left_margin_cols);
-  Lisp_Object INTERNAL_FIELD (right_margin_cols);
+  Lisp_Object left_margin_cols_;
+  Lisp_Object right_margin_cols_;
 
   /* Widths of left and right fringe areas for windows displaying
      this buffer.  */
-  Lisp_Object INTERNAL_FIELD (left_fringe_width);
-  Lisp_Object INTERNAL_FIELD (right_fringe_width);
+  Lisp_Object left_fringe_width_;
+  Lisp_Object right_fringe_width_;
 
   /* Non-nil means fringes are drawn outside display margins;
      othersize draw them between margin areas and text.  */
-  Lisp_Object INTERNAL_FIELD (fringes_outside_margins);
+  Lisp_Object fringes_outside_margins_;
 
   /* Width, height and types of scroll bar areas for windows displaying
      this buffer.  */
-  Lisp_Object INTERNAL_FIELD (scroll_bar_width);
-  Lisp_Object INTERNAL_FIELD (scroll_bar_height);
-  Lisp_Object INTERNAL_FIELD (vertical_scroll_bar_type);
-  Lisp_Object INTERNAL_FIELD (horizontal_scroll_bar_type);
+  Lisp_Object scroll_bar_width_;
+  Lisp_Object scroll_bar_height_;
+  Lisp_Object vertical_scroll_bar_type_;
+  Lisp_Object horizontal_scroll_bar_type_;
 
   /* Non-nil means indicate lines not displaying text (in a style
      like vi).  */
-  Lisp_Object INTERNAL_FIELD (indicate_empty_lines);
+  Lisp_Object indicate_empty_lines_;
 
   /* Non-nil means indicate buffer boundaries and scrolling.  */
-  Lisp_Object INTERNAL_FIELD (indicate_buffer_boundaries);
+  Lisp_Object indicate_buffer_boundaries_;
 
   /* Logical to physical fringe bitmap mappings.  */
-  Lisp_Object INTERNAL_FIELD (fringe_indicator_alist);
+  Lisp_Object fringe_indicator_alist_;
 
   /* Logical to physical cursor bitmap mappings.  */
-  Lisp_Object INTERNAL_FIELD (fringe_cursor_alist);
+  Lisp_Object fringe_cursor_alist_;
 
   /* Time stamp updated each time this buffer is displayed in a window.  */
-  Lisp_Object INTERNAL_FIELD (display_time);
+  Lisp_Object display_time_;
 
   /* If scrolling the display because point is below the bottom of a
      window showing this buffer, try to choose a window start so
      that point ends up this number of lines from the top of the
      window.  Nil means that scrolling method isn't used.  */
-  Lisp_Object INTERNAL_FIELD (scroll_up_aggressively);
+  Lisp_Object scroll_up_aggressively_;
 
   /* If scrolling the display because point is above the top of a
      window showing this buffer, try to choose a window start so
      that point ends up this number of lines from the bottom of the
      window.  Nil means that scrolling method isn't used.  */
-  Lisp_Object INTERNAL_FIELD (scroll_down_aggressively);
+  Lisp_Object scroll_down_aggressively_;
 
   /* Desired cursor type in this buffer.  See the doc string of
      per-buffer variable `cursor-type'.  */
-  Lisp_Object INTERNAL_FIELD (cursor_type);
+  Lisp_Object cursor_type_;
 
   /* An integer > 0 means put that number of pixels below text lines
      in the display of this buffer.  */
-  Lisp_Object INTERNAL_FIELD (extra_line_spacing);
+  Lisp_Object extra_line_spacing_;
 
   /* Cursor type to display in non-selected windows.
      t means to use hollow box cursor.
      See `cursor-type' for other values.  */
-  Lisp_Object INTERNAL_FIELD (cursor_in_non_selected_windows);
+  Lisp_Object cursor_in_non_selected_windows_;
 
   /* No more Lisp_Object beyond this point.  Except undo_list,
      which is handled specially in Fgarbage_collect.  */
@@ -872,7 +872,7 @@ struct buffer
      buffer of an indirect buffer.  But we can't store it in the
      struct buffer_text because local variables have to be right in
      the struct buffer. So we copy it around in set_buffer_internal.  */
-  Lisp_Object INTERNAL_FIELD (undo_list);
+  Lisp_Object undo_list_;
 };
 
 /* Most code should use these functions to set Lisp fields in struct
@@ -881,102 +881,102 @@ struct buffer
 INLINE void
 bset_bidi_paragraph_direction (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (bidi_paragraph_direction) = val;
+  b->bidi_paragraph_direction_ = val;
 }
 INLINE void
 bset_cache_long_scans (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (cache_long_scans) = val;
+  b->cache_long_scans_ = val;
 }
 INLINE void
 bset_case_canon_table (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (case_canon_table) = val;
+  b->case_canon_table_ = val;
 }
 INLINE void
 bset_case_eqv_table (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (case_eqv_table) = val;
+  b->case_eqv_table_ = val;
 }
 INLINE void
 bset_directory (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (directory) = val;
+  b->directory_ = val;
 }
 INLINE void
 bset_display_count (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (display_count) = val;
+  b->display_count_ = val;
 }
 INLINE void
 bset_display_time (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (display_time) = val;
+  b->display_time_ = val;
 }
 INLINE void
 bset_downcase_table (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (downcase_table) = val;
+  b->downcase_table_ = val;
 }
 INLINE void
 bset_enable_multibyte_characters (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (enable_multibyte_characters) = val;
+  b->enable_multibyte_characters_ = val;
 }
 INLINE void
 bset_filename (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (filename) = val;
+  b->filename_ = val;
 }
 INLINE void
 bset_keymap (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (keymap) = val;
+  b->keymap_ = val;
 }
 INLINE void
 bset_last_selected_window (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (last_selected_window) = val;
+  b->last_selected_window_ = val;
 }
 INLINE void
 bset_local_var_alist (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (local_var_alist) = val;
+  b->local_var_alist_ = val;
 }
 INLINE void
 bset_mark_active (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (mark_active) = val;
+  b->mark_active_ = val;
 }
 INLINE void
 bset_point_before_scroll (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (point_before_scroll) = val;
+  b->point_before_scroll_ = val;
 }
 INLINE void
 bset_read_only (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (read_only) = val;
+  b->read_only_ = val;
 }
 INLINE void
 bset_truncate_lines (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (truncate_lines) = val;
+  b->truncate_lines_ = val;
 }
 INLINE void
 bset_undo_list (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (undo_list) = val;
+  b->undo_list_ = val;
 }
 INLINE void
 bset_upcase_table (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (upcase_table) = val;
+  b->upcase_table_ = val;
 }
 INLINE void
 bset_width_table (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (width_table) = val;
+  b->width_table_ = val;
 }
 
 /* Number of Lisp_Objects at the beginning of struct buffer.
@@ -1253,7 +1253,7 @@ extern int last_per_buffer_idx;
    from the start of a buffer structure.  */
 
 #define PER_BUFFER_VAR_OFFSET(VAR) \
-  offsetof (struct buffer, INTERNAL_FIELD (VAR))
+  offsetof (struct buffer, VAR ## _)
 
 /* Used to iterate over normal Lisp_Object fields of struct buffer (all
    Lisp_Objects except undo_list).  If you add, remove, or reorder
diff --git a/src/category.c b/src/category.c
index b20493e..ab90f5f 100644
--- a/src/category.c
+++ b/src/category.c
@@ -41,7 +41,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 static void
 bset_category_table (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (category_table) = val;
+  b->category_table_ = val;
 }
 
 /* The version number of the latest category table.  Each category
diff --git a/src/keyboard.c b/src/keyboard.c
index bd79f90..c2739df 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -391,47 +391,47 @@ static void store_user_signal_events (void);
 static void
 kset_echo_string (struct kboard *kb, Lisp_Object val)
 {
-  kb->INTERNAL_FIELD (echo_string) = val;
+  kb->echo_string_ = val;
 }
 static void
 kset_kbd_queue (struct kboard *kb, Lisp_Object val)
 {
-  kb->INTERNAL_FIELD (kbd_queue) = val;
+  kb->kbd_queue_ = val;
 }
 static void
 kset_keyboard_translate_table (struct kboard *kb, Lisp_Object val)
 {
-  kb->INTERNAL_FIELD (Vkeyboard_translate_table) = val;
+  kb->Vkeyboard_translate_table_ = val;
 }
 static void
 kset_last_prefix_arg (struct kboard *kb, Lisp_Object val)
 {
-  kb->INTERNAL_FIELD (Vlast_prefix_arg) = val;
+  kb->Vlast_prefix_arg_ = val;
 }
 static void
 kset_last_repeatable_command (struct kboard *kb, Lisp_Object val)
 {
-  kb->INTERNAL_FIELD (Vlast_repeatable_command) = val;
+  kb->Vlast_repeatable_command_ = val;
 }
 static void
 kset_local_function_key_map (struct kboard *kb, Lisp_Object val)
 {
-  kb->INTERNAL_FIELD (Vlocal_function_key_map) = val;
+  kb->Vlocal_function_key_map_ = val;
 }
 static void
 kset_overriding_terminal_local_map (struct kboard *kb, Lisp_Object val)
 {
-  kb->INTERNAL_FIELD (Voverriding_terminal_local_map) = val;
+  kb->Voverriding_terminal_local_map_ = val;
 }
 static void
 kset_real_last_command (struct kboard *kb, Lisp_Object val)
 {
-  kb->INTERNAL_FIELD (Vreal_last_command) = val;
+  kb->Vreal_last_command_ = val;
 }
 static void
 kset_system_key_syms (struct kboard *kb, Lisp_Object val)
 {
-  kb->INTERNAL_FIELD (system_key_syms) = val;
+  kb->system_key_syms_ = val;
 }
 
 \f
diff --git a/src/keyboard.h b/src/keyboard.h
index 0ce6d18..bcdeaf6 100644
--- a/src/keyboard.h
+++ b/src/keyboard.h
@@ -25,7 +25,7 @@ INLINE_HEADER_BEGIN
 
 /* Most code should use this macro to access Lisp fields in struct kboard.  */
 
-#define KVAR(kboard, field) ((kboard)->INTERNAL_FIELD (field))
+#define KVAR(kboard, field) ((kboard)->field ## _)
 
 /* Each KBOARD represents one logical input stream from which Emacs
    gets input.  If we are using ordinary terminals, it has one KBOARD
@@ -78,32 +78,32 @@ struct kboard
        can effectively wait for input in the any-kboard state, and hence
        avoid blocking out the other KBOARDs.  See universal-argument in
        lisp/simple.el for an example.  */
-    Lisp_Object INTERNAL_FIELD (Voverriding_terminal_local_map);
+    Lisp_Object Voverriding_terminal_local_map_;
 
     /* Last command executed by the editor command loop, not counting
        commands that set the prefix argument.  */
-    Lisp_Object INTERNAL_FIELD (Vlast_command);
+    Lisp_Object Vlast_command_;
 
     /* Normally same as last-command, but never modified by other commands.  */
-    Lisp_Object INTERNAL_FIELD (Vreal_last_command);
+    Lisp_Object Vreal_last_command_;
 
     /* User-supplied table to translate input characters through.  */
-    Lisp_Object INTERNAL_FIELD (Vkeyboard_translate_table);
+    Lisp_Object Vkeyboard_translate_table_;
 
     /* Last command that may be repeated by `repeat'.  */
-    Lisp_Object INTERNAL_FIELD (Vlast_repeatable_command);
+    Lisp_Object Vlast_repeatable_command_;
 
     /* The prefix argument for the next command, in raw form.  */
-    Lisp_Object INTERNAL_FIELD (Vprefix_arg);
+    Lisp_Object Vprefix_arg_;
 
     /* Saved prefix argument for the last command, in raw form.  */
-    Lisp_Object INTERNAL_FIELD (Vlast_prefix_arg);
+    Lisp_Object Vlast_prefix_arg_;
 
     /* Unread events specific to this kboard.  */
-    Lisp_Object INTERNAL_FIELD (kbd_queue);
+    Lisp_Object kbd_queue_;
 
     /* Non-nil while a kbd macro is being defined.  */
-    Lisp_Object INTERNAL_FIELD (defining_kbd_macro);
+    Lisp_Object defining_kbd_macro_;
 
     /* The start of storage for the current keyboard macro.  */
     Lisp_Object *kbd_macro_buffer;
@@ -125,28 +125,28 @@ struct kboard
     ptrdiff_t kbd_macro_bufsize;
 
     /* Last anonymous kbd macro defined.  */
-    Lisp_Object INTERNAL_FIELD (Vlast_kbd_macro);
+    Lisp_Object Vlast_kbd_macro_;
 
     /* Alist of system-specific X windows key symbols.  */
-    Lisp_Object INTERNAL_FIELD (Vsystem_key_alist);
+    Lisp_Object Vsystem_key_alist_;
 
     /* Cache for modify_event_symbol.  */
-    Lisp_Object INTERNAL_FIELD (system_key_syms);
+    Lisp_Object system_key_syms_;
 
     /* The kind of display: x, w32, ...  */
-    Lisp_Object INTERNAL_FIELD (Vwindow_system);
+    Lisp_Object Vwindow_system_;
 
     /* Keymap mapping keys to alternative preferred forms.
        See the DEFVAR for more documentation.  */
-    Lisp_Object INTERNAL_FIELD (Vlocal_function_key_map);
+    Lisp_Object Vlocal_function_key_map_;
 
     /* Keymap mapping ASCII function key sequences onto their preferred
        forms.  Initialized by the terminal-specific lisp files.  See the
        DEFVAR for more documentation.  */
-    Lisp_Object INTERNAL_FIELD (Vinput_decode_map);
+    Lisp_Object Vinput_decode_map_;
 
     /* Minibufferless frames on this display use this frame's minibuffer.  */
-    Lisp_Object INTERNAL_FIELD (Vdefault_minibuffer_frame);
+    Lisp_Object Vdefault_minibuffer_frame_;
 
     /* Number of displays using this KBOARD.  Normally 1, but can be
        larger when you have multiple screens on a single X display.  */
@@ -154,7 +154,7 @@ struct kboard
 
     /* The text we're echoing in the modeline - partial key sequences,
        usually.  This is nil when not echoing.  */
-    Lisp_Object INTERNAL_FIELD (echo_string);
+    Lisp_Object echo_string_;
 
     /* This flag indicates that events were put into kbd_queue
        while Emacs was running for some other KBOARD.
@@ -179,42 +179,42 @@ struct kboard
 INLINE void
 kset_default_minibuffer_frame (struct kboard *kb, Lisp_Object val)
 {
-  kb->INTERNAL_FIELD (Vdefault_minibuffer_frame) = val;
+  kb->Vdefault_minibuffer_frame_ = val;
 }
 INLINE void
 kset_defining_kbd_macro (struct kboard *kb, Lisp_Object val)
 {
-  kb->INTERNAL_FIELD (defining_kbd_macro) = val;
+  kb->defining_kbd_macro_ = val;
 }
 INLINE void
 kset_input_decode_map (struct kboard *kb, Lisp_Object val)
 {
-  kb->INTERNAL_FIELD (Vinput_decode_map) = val;
+  kb->Vinput_decode_map_ = val;
 }
 INLINE void
 kset_last_command (struct kboard *kb, Lisp_Object val)
 {
-  kb->INTERNAL_FIELD (Vlast_command) = val;
+  kb->Vlast_command_ = val;
 }
 INLINE void
 kset_last_kbd_macro (struct kboard *kb, Lisp_Object val)
 {
-  kb->INTERNAL_FIELD (Vlast_kbd_macro) = val;
+  kb->Vlast_kbd_macro_ = val;
 }
 INLINE void
 kset_prefix_arg (struct kboard *kb, Lisp_Object val)
 {
-  kb->INTERNAL_FIELD (Vprefix_arg) = val;
+  kb->Vprefix_arg_ = val;
 }
 INLINE void
 kset_system_key_alist (struct kboard *kb, Lisp_Object val)
 {
-  kb->INTERNAL_FIELD (Vsystem_key_alist) = val;
+  kb->Vsystem_key_alist_ = val;
 }
 INLINE void
 kset_window_system (struct kboard *kb, Lisp_Object val)
 {
-  kb->INTERNAL_FIELD (Vwindow_system) = val;
+  kb->Vwindow_system_ = val;
 }
 
 /* Temporarily used before a frame has been opened. */
diff --git a/src/lisp.h b/src/lisp.h
index 55c4c66..6d34ce3 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -1144,10 +1144,6 @@ LISP_MACRO_DEFUN_VOID (CHECK_TYPE,
 		       (int ok, Lisp_Object predicate, Lisp_Object x),
 		       (ok, predicate, x))
 
-/* Deprecated and will be removed soon.  */
-
-#define INTERNAL_FIELD(field) field ## _
-
 /* See the macros in intervals.h.  */
 
 typedef struct interval *INTERVAL;
diff --git a/src/syntax.c b/src/syntax.c
index 2f82156..1695815 100644
--- a/src/syntax.c
+++ b/src/syntax.c
@@ -186,7 +186,7 @@ static bool in_classes (int, Lisp_Object);
 static void
 bset_syntax_table (struct buffer *b, Lisp_Object val)
 {
-  b->INTERNAL_FIELD (syntax_table) = val;
+  b->syntax_table_ = val;
 }
 \f
 /* Whether the syntax of the character C has the prefix flag set.  */
-- 
1.8.4

============================================================

Re: Is it time to remove INTERNAL_FIELD?

[Eli Zaretskii]

> From: Oleh Krehel <ohwoeowho@gmail.com>
> Cc: eggert@cs.ucla.edu,  monnier@IRO.UMontreal.CA,  emacs-devel@gnu.org
> Date: Tue, 28 Apr 2015 20:58:38 +0200
> 
> I attach the flattened patch. It compiles and runs fine, but please
> check it again.

LGTM, thanks.

Re: Is it time to remove INTERNAL_FIELD?

[Eli Zaretskii]

> Date: Thu, 23 Apr 2015 03:17:36 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> Cc: emacs-devel@gnu.org
> 
> Oleh Krehel wrote:
> > Should it be 'foo' or 'foo_'?
> 
> The former, surely.  There should be no need for the trailing underscore.

Wasn't that underscore introduced for the concurrency branch?  E.g.,
see KVAR and BVAR macros.  I think the idea was to prevent people from
writing code that directly accesses these fields, which is bad for
concurrency.

Re: Is it time to remove INTERNAL_FIELD?

[Stefan Monnier]

> This macro looks to me as more confusing than useful:

It's there to make sure people don't write code which accesses the
field directly by accident.  It seems harmless and occasionally
marginally useful.
Why does it bother you, exactly?

> Can I remove it?

I guess it's OK to remove it, tho I fail to see the harm in keeping it.


        Stefan

Re: Is it time to remove INTERNAL_FIELD?

[Oleh Krehel]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> This macro looks to me as more confusing than useful:
>
> It's there to make sure people don't write code which accesses the
> field directly by accident.  It seems harmless and occasionally
> marginally useful.
> Why does it bother you, exactly?

I'm not very experienced in C, more in C++.  I really like to know the
types of the data structures that I'm accessing.

It's hard to see the structure past the macros, so I think if a macro
doesn't provide a clear advantage it should be removed.

>> Can I remove it?
>
> I guess it's OK to remove it, tho I fail to see the harm in keeping it.

As I'm new to the Emacs C code base, it's easier for me to look at it
without this macro. If you want to make C sources easier to read for new
people, we should remove the macros that don't do anything.

As for accidental access, I'm sure these rare errors will be caught by
the code review / test suite.

Oleh

Re: Is it time to remove INTERNAL_FIELD?

[Eli Zaretskii]

> From: Oleh Krehel <ohwoeowho@gmail.com>
> Date: Thu, 23 Apr 2015 15:30:26 +0200
> Cc: emacs-devel@gnu.org
> 
> we should remove the macros that don't do anything.

What this macro does is allow you to use field names like 'foo', when
the field is really called 'foo_'.

I think it's okay to remove INTERNAL_FIELD, but I think we should keep
the trailing underscore appended in BVAR and KVAR.  That's how all
this started: the fields were renamed to have a trailing underscore so
that code that used foo->bar instead of BVAR (foo, bar) would be
immediately flagged by the compiler.

> As for accidental access, I'm sure these rare errors will be caught by
> the code review / test suite.

We don't want to rely on code reviews, since they are very informal
and their coverage is too low to be efficient.

Based on bitter past experience with similar errors that lay low for
months, sometimes for years, I'd rather not give up those underscores
in BVAR and KVAR, which means the struct fields should retain them.

Re: Is it time to remove INTERNAL_FIELD?

[Oleh Krehel]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Oleh Krehel <ohwoeowho@gmail.com>
>> Date: Thu, 23 Apr 2015 15:30:26 +0200
>> Cc: emacs-devel@gnu.org
>> 
>> we should remove the macros that don't do anything.
>
> What this macro does is allow you to use field names like 'foo', when
> the field is really called 'foo_'.
>
> I think it's okay to remove INTERNAL_FIELD, but I think we should keep
> the trailing underscore appended in BVAR and KVAR.  That's how all
> this started: the fields were renamed to have a trailing underscore so
> that code that used foo->bar instead of BVAR (foo, bar) would be
> immediately flagged by the compiler.
>
>> As for accidental access, I'm sure these rare errors will be caught by
>> the code review / test suite.
>
> We don't want to rely on code reviews, since they are very informal
> and their coverage is too low to be efficient.
>
> Based on bitter past experience with similar errors that lay low for
> months, sometimes for years, I'd rather not give up those underscores
> in BVAR and KVAR, which means the struct fields should retain them.

I'm totally fine with this:

    INLINE void
    kset_last_kbd_macro (struct kboard *kb, Lisp_Object val)
    {
      kb->Vlast_kbd_macro_ = val;
    }

just as I'm fine with this:

    INLINE void
    kset_last_kbd_macro (struct kboard *kb, Lisp_Object val)
    {
      kb->Vlast_kbd_macro = val;
    }

Both are boilerplate that doesn't require additional thought. In the
first case, it's maybe more explicit that `Vlast_kbd_macro_' should not
be accessed outside the interface function `kset_last_kbd_macro'.

But this I don't like:

    INLINE void
    kset_last_kbd_macro (struct kboard *kb, Lisp_Object val)
    {
      kb->INTERNAL_FIELD (Vlast_kbd_macro) = val;
    }

It's not obvious how simple or intricate INTERNAL_FIELD is or what it
does. At the first glance, looks like C++ member function call.

Oleh

Re: Is it time to remove INTERNAL_FIELD?

[Nicolas Richard]

Oleh Krehel <ohwoeowho@gmail.com> writes:
> It's not obvious how simple or intricate INTERNAL_FIELD is or what it
> does. At the first glance, looks like C++ member function call.

From the perspective of someone who never touches C source, I think
INTERNAL_FIELD spelled out is more clear on the intent than an
underscore.

-- 
Nicolas

Re: Is it time to remove INTERNAL_FIELD?

[Eli Zaretskii]

> From: Nicolas Richard <theonewiththeevillook@yahoo.fr>
> Cc: Eli Zaretskii <eliz@gnu.org>,  monnier@IRO.UMontreal.CA,  emacs-devel@gnu.org
> Date: Thu, 23 Apr 2015 16:50:02 +0200
> 
> Oleh Krehel <ohwoeowho@gmail.com> writes:
> > It's not obvious how simple or intricate INTERNAL_FIELD is or what it
> > does. At the first glance, looks like C++ member function call.
> 
> >From the perspective of someone who never touches C source, I think
> INTERNAL_FIELD spelled out is more clear on the intent than an
> underscore.

I agree.  But if this underscore always hides inside the likes of BVAR
and KVAR, and is never explicitly seen anywhere else, then we don't
really need INTERNAL_FIELD, do we?

Re: Is it time to remove INTERNAL_FIELD?

[Nicolas Richard]

Eli Zaretskii <eliz@gnu.org> writes:
> I agree.  But if this underscore always hides inside the likes of BVAR
> and KVAR, and is never explicitly seen anywhere else, then we don't
> really need INTERNAL_FIELD, do we?

If you say we don't need it, I certainly believe you. I have zero
experience.

-- 
Nico

Re: Is it time to remove INTERNAL_FIELD?

[Eli Zaretskii]

> From: Oleh Krehel <ohwoeowho@gmail.com>
> Cc: monnier@IRO.UMontreal.CA,  emacs-devel@gnu.org
> Date: Thu, 23 Apr 2015 16:07:39 +0200
> 
> > What this macro does is allow you to use field names like 'foo', when
> > the field is really called 'foo_'.
> >
> > I think it's okay to remove INTERNAL_FIELD, but I think we should keep
> > the trailing underscore appended in BVAR and KVAR.
> [...]
> I'm totally fine with this:
> 
>     INLINE void
>     kset_last_kbd_macro (struct kboard *kb, Lisp_Object val)
>     {
>       kb->Vlast_kbd_macro_ = val;
>     }
> 
> just as I'm fine with this:
> 
>     INLINE void
>     kset_last_kbd_macro (struct kboard *kb, Lisp_Object val)
>     {
>       kb->Vlast_kbd_macro = val;
>     }

We are talking past each other.  I wasn't talking about
kset_last_kbd_macro etc, I was talking about expressions that
explicitly mention field names.  Like this one:

  foo->name = bar;

or this:

  BVAR (foo, name) = bar;

or this:

  buffer_name = BVAR (foo, name);

It's the "name" part that I care about.

If "BVAR (foo, name)" expands into "foo->name_", then no code can use
bar->name anywhere without triggering a compilation error.  But I, as
code write, can still call the field "name" and use it in my code, and
have the preprocessor append the underscore for me.

> It's not obvious how simple or intricate INTERNAL_FIELD is or what it
> does. At the first glance, looks like C++ member function call.

And what's wrong with that?  For someone who programs in C++, and
should therefore be ready to accept overloaded operators that can
compute the end of the world as part of their processing, how do you
know, in C++, that "->" is not overloaded to do just that?

Re: Is it time to remove INTERNAL_FIELD?

[Oleh Krehel]

Eli Zaretskii <eliz@gnu.org> writes:
>> [...]
>> I'm totally fine with this:
>>
>>     INLINE void
>>     kset_last_kbd_macro (struct kboard *kb, Lisp_Object val)
>>     {
>>       kb->Vlast_kbd_macro_ = val;
>>     }
>>
>> just as I'm fine with this:
>>
>>     INLINE void
>>     kset_last_kbd_macro (struct kboard *kb, Lisp_Object val)
>>     {
>>       kb->Vlast_kbd_macro = val;
>>     }
>
> We are talking past each other.  I wasn't talking about
> kset_last_kbd_macro etc, I was talking about expressions that
> explicitly mention field names.  Like this one:
>
>   foo->name = bar;
>
> or this:
>
>   BVAR (foo, name) = bar;
>
> or this:
>
>   buffer_name = BVAR (foo, name);
>
> It's the "name" part that I care about.
>
> If "BVAR (foo, name)" expands into "foo->name_", then no code can use
> bar->name anywhere without triggering a compilation error.  But I, as
> code write, can still call the field "name" and use it in my code, and
> have the preprocessor append the underscore for me.

Why is it preferred to type BVAR (foo, name) instead of foo->name?
This confuses me, because I can't use Semantic to assist me in what I'm
doing. For instance, starting with:

    kb->Vw

Semantic can tell me that the only possible completions are Vwindow_list
and Vwindow_system. This is great for someone who's new, because I see
what options are available to me. This is also great for someone who's
experienced, because it still acts as a spell checker and speeds up
coding. I can't get the same benefits for:

    kb->INTERNAL_FIELD (Vwindow_system) = val;

The first variant of the code feels like I'm in control of the code, and
I'm actually dealing with code.

The second variant feels like I'm doing incantations, keeping fingers
crossed that it works, and actually dealing with text and not with code.

>> It's not obvious how simple or intricate INTERNAL_FIELD is or what it
>> does. At the first glance, looks like C++ member function call.
>
> And what's wrong with that?  For someone who programs in C++, and
> should therefore be ready to accept overloaded operators that can
> compute the end of the world as part of their processing, how do you
> know, in C++, that "->" is not overloaded to do just that?

The kind of C++ libraries that I'm dealing with overload arithmetic
operators on vectors and matrices, actually simplifying the code.
INTERNAL_FIELD simply does string substitution. That's not immensely
useful and it hampers completion and introspection.

Oleh

Re: Is it time to remove INTERNAL_FIELD?

[Eli Zaretskii]

> From: Oleh Krehel <ohwoeowho@gmail.com>
> Cc: monnier@IRO.UMontreal.CA,  emacs-devel@gnu.org
> Date: Thu, 23 Apr 2015 18:32:17 +0200
> 
> Why is it preferred to type BVAR (foo, name) instead of foo->name?

Because it will then be easy to change the definition of BVAR into
something less trivial.  E.g., imagine this alternative:

 #define BVAR(buf, field) ((buf)->thread_storage (current_thread)->(field))

Or whatever, I hope you get the point.

> This confuses me, because I can't use Semantic to assist me in what I'm
> doing. For instance, starting with:
> 
>     kb->Vw
> 
> Semantic can tell me that the only possible completions are Vwindow_list
> and Vwindow_system. This is great for someone who's new, because I see
> what options are available to me. This is also great for someone who's
> experienced, because it still acts as a spell checker and speeds up
> coding. I can't get the same benefits for:
> 
>     kb->INTERNAL_FIELD (Vwindow_system) = val;
> 
> The first variant of the code feels like I'm in control of the code, and
> I'm actually dealing with code.

Using accessors has its downsides, yes.  It makes the object more
opaque.  Perhaps Semantic should become smarter about this.  But you
aren't saying that accessors should not be used, are you?

> >> It's not obvious how simple or intricate INTERNAL_FIELD is or what it
> >> does. At the first glance, looks like C++ member function call.
> >
> > And what's wrong with that?  For someone who programs in C++, and
> > should therefore be ready to accept overloaded operators that can
> > compute the end of the world as part of their processing, how do you
> > know, in C++, that "->" is not overloaded to do just that?
> 
> The kind of C++ libraries that I'm dealing with overload arithmetic
> operators on vectors and matrices, actually simplifying the code.

You have been lucky.  In general, when you work with C++ libraries to
which you have no sources, you can never know what the overloaded
operators do, or how expensive they are.

Re: Is it time to remove INTERNAL_FIELD?

[Oleh Krehel]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Oleh Krehel <ohwoeowho@gmail.com>
>> Cc: monnier@IRO.UMontreal.CA,  emacs-devel@gnu.org
>> Date: Thu, 23 Apr 2015 18:32:17 +0200
>> 
>> Why is it preferred to type BVAR (foo, name) instead of foo->name?
>
> Because it will then be easy to change the definition of BVAR into
> something less trivial.  E.g., imagine this alternative:
>
>  #define BVAR(buf, field) ((buf)->thread_storage (current_thread)->(field))
>
> Or whatever, I hope you get the point.

OK, fine. At least it's possible to cook up a viable heuristic for
completing BVAR with Semantic, because it has the semantics of accessing
the buffer object. INTERNAL_FIELD is just mental arithmetic, it doesn't
do anything. It's possible to make a heuristic for it as well, but I
like to see it removed instead.

>> This confuses me, because I can't use Semantic to assist me in what I'm
>> doing. For instance, starting with:
>> 
>>     kb->Vw
>> 
>> Semantic can tell me that the only possible completions are Vwindow_list
>> and Vwindow_system. This is great for someone who's new, because I see
>> what options are available to me. This is also great for someone who's
>> experienced, because it still acts as a spell checker and speeds up
>> coding. I can't get the same benefits for:
>> 
>>     kb->INTERNAL_FIELD (Vwindow_system) = val;
>> 
>> The first variant of the code feels like I'm in control of the code, and
>> I'm actually dealing with code.
>
> Using accessors has its downsides, yes.  It makes the object more
> opaque.  Perhaps Semantic should become smarter about this.  But you
> aren't saying that accessors should not be used, are you?

I like accessors for C++, but I'm not really familiar with large scale C
programming. Are macros really the state-of-the-art for making
accessors?

Oleh

Re: Is it time to remove INTERNAL_FIELD?

[Eli Zaretskii]

> From: Oleh Krehel <ohwoeowho@gmail.com>
> Cc: monnier@IRO.UMontreal.CA,  emacs-devel@gnu.org
> Date: Thu, 23 Apr 2015 19:09:24 +0200
> 
> Are macros really the state-of-the-art for making accessors?

It depends how deep in inner loops they are called.  If they are deep
enough, then either macros or inline functions are used.  We actually
use both, with some fine-tuning (see lisp.h), because we want Emacs to
be reasonably fast even in an unoptimized build, where inline
functions are not inlined.  (Most core developers run the development
version built without optimizations, since optimized code is hard to
debug.)

Accessor functions that are not speed-critical can be simple
functions.

Re: Is it time to remove INTERNAL_FIELD?

[Stefan Monnier]

> Why is it preferred to type BVAR (foo, name) instead of foo->name?

I can't remember the original motivation, but the basic idea is that
BVAR may (at some point) expand to more code, to handle the fact that
maybe this info can sometimes be stored elsewhere.


        Stefan

Re: Is it time to remove INTERNAL_FIELD?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Thu, 23 Apr 2015 13:14:09 -0400
> 
> > Why is it preferred to type BVAR (foo, name) instead of foo->name?
> 
> I can't remember the original motivation, but the basic idea is that
> BVAR may (at some point) expand to more code, to handle the fact that
> maybe this info can sometimes be stored elsewhere.

The question is do we still want to be able to detect direct
references to these fields?  If we do, we should keep the trailing
underscores, I think.

Re: Is it time to remove INTERNAL_FIELD?

[Stefan Monnier]

> In an effort to make the mark and mark-active be window-local (like the
> point) and not buffer-local, I've started learning the way that the

BTW, since our last discussion about it, I'm not sure it's going to be
a good change for everyone: when the mark is used to select a region on
which to operate, it'll usually be preferable for that mark to be
window-local, but for people who use the mark as a simple "remember this
location", they may occasionally by disappointed that it's not shared
among the various windows of a buffer.
Also, the mark-ring will definitely not want to become window-local, so
having a window-local mark but a buffer-local mark-ring could lead to
further surprises.
Not to say that it would be a bad change, but that it may require
a fair bit of twiddling until everyone's happy with it.


        Stefan

Re: Is it time to remove INTERNAL_FIELD?

[Oleh Krehel]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> In an effort to make the mark and mark-active be window-local (like the
>> point) and not buffer-local, I've started learning the way that the
>
> BTW, since our last discussion about it, I'm not sure it's going to be
> a good change for everyone: when the mark is used to select a region on
> which to operate, it'll usually be preferable for that mark to be
> window-local, but for people who use the mark as a simple "remember this
> location", they may occasionally by disappointed that it's not shared
> among the various windows of a buffer.

Surely it will be easy to customize this behavior if I manage to figure
out how to do it.

> Also, the mark-ring will definitely not want to become window-local, so
> having a window-local mark but a buffer-local mark-ring could lead to
> further surprises.
> Not to say that it would be a bad change, but that it may require
> a fair bit of twiddling until everyone's happy with it.

Fine, it's worth it for me to get rid of the annoyance.

Oleh