bug#26597: 25.1; Compilation error on master with --enable-check-lisp-object-type

bug#26597: 25.1; Compilation error on master with --enable-check-lisp-object-type

[Philipp Stephani]

When compiling current master with --enable-check-lisp-object-type,
compilation fails:

/Applications/Xcode.app/Contents/Developer/usr/bin/make -C src VCSWITNESS='$(srcdir)/../.git/logs/HEAD' all
gcc -c  -Demacs  -I. -I. -I../lib -I../lib   -I/usr/X11/include   -D_REENTRANT -I/usr/local/Cellar/librsvg/2.40.16_1/include/librsvg-2.0 -I/usr/local/Cellar/gdk-pixbuf/2.36.2/include/gdk-pixbuf-2.0 -I/usr/local/Cellar/libpng/1.6.29/include/libpng16 -I/usr/local/Cellar/cairo/1.14.8/include/cairo -I/usr/local/Cellar/glib/2.52.0/include/glib-2.0 -I/usr/local/Cellar/glib/2.52.0/lib/glib-2.0/include -I/usr/local/opt/gettext/include -I/usr/local/Cellar/pcre/8.40/include -I/usr/local/Cellar/pixman/0.34.0/include/pixman-1 -I/usr/local/Cellar/fontconfig/2.12.1_2/include -I/usr/local/opt/freetype/include/freetype2 -I/usr/local/Cellar/libpng/1.6.29/include/libpng16    -I/usr/local/Cellar/dbus/1.10.16/include/dbus-1.0 -I/usr/local/Cellar/dbus/1.10.16/lib/dbus-1.0/include           -MMD -MF deps/nsterm.d -MP  -I/usr/local/Cellar/gnutls/3.5.10/include -I/usr/local/Cellar/nettle/3.3/include -I/usr/local/Cellar/libtasn1/4.10/include -I/usr/local/Cellar/p11-kit/0.23.5/include/p11-kit-1    -Wno-switch -Wno-tautological-constant-out-of-range-compare -Wno-pointer-sign -Wno-string-plus-int -Wno-unknown-attributes -ggdb3 -O0   nsterm.m
nsterm.m:6969:8: error: member reference base type 'enum z_group' is not a structure or union
  if (!NILP (FRAME_Z_GROUP (f)))
       ^~~~~~~~~~~~~~~~~~~~~~~~

FRAME_Z_GROUP is indeed an enum, and NILP shouldn't be applied to it.  I
guess this works by accident if --enable-check-lisp-object-type is not
set because then Lisp_Object is a plain integer.

(Tangentially, is there any reason not to define Lisp_Object as struct
unconditionally, to avoid such coding errors?)



In GNU Emacs 25.1.1 (x86_64-apple-darwin15.6.0, NS appkit-1404.47 Version 10.11.6 (Build 15G1004))
 of 2016-09-22 built on p
Windowing system distributor 'Apple', version 10.3.1504
Configured using:
 'configure --disable-dependency-tracking --disable-silent-rules
 --enable-locallisppath=/usr/local/share/emacs/site-lisp
 --infodir=/usr/local/Cellar/emacs/25.1/share/info/emacs
 --prefix=/usr/local/Cellar/emacs/25.1 --without-x --with-xml2
 --without-dbus --without-gnutls --with-rsvg --with-ns
 --disable-ns-self-contained'

Configured features:
JPEG RSVG NOTIFY ACL LIBXML2 ZLIB TOOLKIT_SCROLL_BARS NS

Important settings:
  value of $LANG: de_DE.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Fundamental

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Load-path shadows:
/usr/local/share/emacs/site-lisp/f/f hides /usr/local/share/emacs/site-lisp/f-emacs/f
/usr/local/share/emacs/site-lisp/let-alist/let-alist hides /usr/local/Cellar/emacs/25.1/share/emacs/25.1/lisp/emacs-lisp/let-alist

Features:
(shadow sort mail-extr emacsbug message dired format-spec rfc822 mml
mml-sec password-cache epg epg-config gnus-util mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util help-fns help-mode easymenu
cl-loaddefs pcase cl-lib mail-prsvr mail-utils time-date mule-util
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel ns-win ucs-normalize term/common-win tool-bar dnd fontset image
regexp-opt fringe tabulated-list newcomment elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cl-generic cham
georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese charscript case-table epa-hook
jka-cmpr-hook help simple abbrev minibuffer cl-preloaded nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote kqueue cocoa ns multi-tty
make-network-process emacs)

Memory information:
((conses 16 196364 9526)
 (symbols 48 19634 0)
 (miscs 40 77 241)
 (strings 32 15041 5419)
 (string-bytes 1 436541)
 (vectors 16 32909)
 (vector-slots 8 654057 5600)
 (floats 8 158 96)
 (intervals 56 208 0)
 (buffers 976 19))

bug#26597: 25.1; Compilation error on master with --enable-check-lisp-object-type

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Fri, 21 Apr 2017 22:05:58 +0200
> 
> nsterm.m:6969:8: error: member reference base type 'enum z_group' is not a structure or union
>   if (!NILP (FRAME_Z_GROUP (f)))
>        ^~~~~~~~~~~~~~~~~~~~~~~~

It should compare against z_group_none instead.

> (Tangentially, is there any reason not to define Lisp_Object as struct
> unconditionally, to avoid such coding errors?)

Yes, it produces slower code.

bug#26597: 25.1; Compilation error on master with --enable-check-lisp-object-type

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 967 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am Sa., 22. Apr. 2017 um 09:42 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Fri, 21 Apr 2017 22:05:58 +0200
> >
> > nsterm.m:6969:8: error: member reference base type 'enum z_group' is not
> a structure or union
> >   if (!NILP (FRAME_Z_GROUP (f)))
> >        ^~~~~~~~~~~~~~~~~~~~~~~~
>
> It should compare against z_group_none instead.
>

Pushed eb52828a43 to fix.


>
> > (Tangentially, is there any reason not to define Lisp_Object as struct
> > unconditionally, to avoid such coding errors?)
>
> Yes, it produces slower code.
>

Are you sure that's still the case? I've just diffed the assembly output
for editfns.c with and without --enable-check-lisp-object-type, they seem
identical (apart from minor diffs due to different register allocation and
instruction ordering). Replacing a primitive value with a struct containing
such a value should never degrade performance; that would be a compiler
bug.

[-- Attachment #2: Type: text/html, Size: 1541 bytes --]

bug#26597: 25.1; Compilation error on master with --enable-check-lisp-object-type

[Andreas Schwab]

On Apr 22 2017, Philipp Stephani <p.stephani2@gmail.com> wrote:

> Are you sure that's still the case?

There are architectures where passing a struct is less efficient than
passing an integer, even if they are of the same size.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#26597: 25.1; Compilation error on master with --enable-check-lisp-object-type

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 22 Apr 2017 11:57:11 +0000
> 
>  > (Tangentially, is there any reason not to define Lisp_Object as struct
>  > unconditionally, to avoid such coding errors?)
> 
>  Yes, it produces slower code.
> 
> Are you sure that's still the case? I've just diffed the assembly output for editfns.c with and without
> --enable-check-lisp-object-type, they seem identical (apart from minor diffs due to different register allocation
> and instruction ordering). Replacing a primitive value with a struct containing such a value should never
> degrade performance; that would be a compiler bug. 

On what OS/architecture, and with what compiler options?

in any case, if you want to raise this issue, please post to
emacs-devel, not here.

bug#26597: 25.1; Compilation error on master with --enable-check-lisp-object-type

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 373 bytes --]

Andreas Schwab <schwab@linux-m68k.org> schrieb am Sa., 22. Apr. 2017 um
15:35 Uhr:

> On Apr 22 2017, Philipp Stephani <p.stephani2@gmail.com> wrote:
>
> > Are you sure that's still the case?
>
> There are architectures where passing a struct is less efficient than
> passing an integer, even if they are of the same size.
>
>
That might be the case, but not on x86/AMD64.

[-- Attachment #2: Type: text/html, Size: 742 bytes --]

bug#26597: 25.1; Compilation error on master with --enable-check-lisp-object-type

[Stefan Monnier]

> Replacing a primitive value with a struct containing such a value
> should never degrade performance; that would be a compiler bug.

FWIW, No, compilers do not guarantee that semantically-equivalent
programs will be compiled to code with equivalent performance, so it
wouldn't be a bug.  It's only a "quality of implementation" issue.


        Stefan

bug#26597: 25.1; Compilation error on master with --enable-check-lisp-object-type

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 497 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> schrieb am Mo., 1. Mai 2017 um
15:01 Uhr:

> > Replacing a primitive value with a struct containing such a value
> > should never degrade performance; that would be a compiler bug.
>
> FWIW, No, compilers do not guarantee that semantically-equivalent
> programs will be compiled to code with equivalent performance, so it
> wouldn't be a bug.  It's only a "quality of implementation" issue.
>

Yes, I mean "bug" in a wider sense that includes QoI issues.

[-- Attachment #2: Type: text/html, Size: 828 bytes --]