bug#41348: emacs compilation with clang10 fails in the file lib-src/etags.c

bug#41348: emacs compilation with clang10 fails in the file lib-src/etags.c

[Narayanan Nellayi]

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

Hi,

emacs compilation with clang10 on Ubuntu 20.04 (focal release) fails with
an error in the file lib-src/etags.c. I am able to fix the issue with a
one-line change. This compilation failure is seen on the emacs src from
top-of-the-master-branch.

Pls see if this fix is appropriate and consider accordingly.

I have given below the details on the issue and a patch description.

Regards
Narayanan

clang --version
> clang version 10.0.0-4ubuntu1
> Target: x86_64-pc-linux-gnu
> Thread model: posix
> InstalledDir: /usr/bin
>


> make -C lib-src all
> make[1]: Entering directory
> '/mnt/myvg_fs/home/anarayan/sources/emacs/lib-src'
>   CCLD     etags
> error: fallthrough annotation does not directly precede switch label
> 1 error generated.
> make[1]: *** [Makefile:366: etags] Error 1
> make[1]: Leaving directory
> '/mnt/myvg_fs/home/anarayan/sources/emacs/lib-src'
> make: *** [Makefile:411: lib-src] Error 2
>


> diff --git a/lib-src/etags.c b/lib-src/etags.c
> index eee2c59626..b5f077007b 100644
> --- a/lib-src/etags.c
> +++ b/lib-src/etags.c
> @@ -4196,7 +4196,7 @@ C_entries (int c_ext, FILE *inf)
>        objdef = omethodsign;
>        break;
>      }
> -  FALLTHROUGH;
> +  // FALLTHROUGH;
>   resetfvdef:
>   case '#': case '~': case '&': case '%': case '/':
>   case '|': case '^': case '!': case '.': case '?':
>

>

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

bug#41348: emacs compilation with clang10 fails in the file lib-src/etags.c

[Eli Zaretskii]

> From: Narayanan Nellayi <n.a.narayanan@gmail.com>
> Date: Sun, 17 May 2020 12:32:39 +0530
> Cc: "Narayanan N.A." <n.a.narayanan@gmail.com>
> 
> emacs compilation with clang10 on Ubuntu 20.04 (focal release) fails with an error in the file lib-src/etags.c.
> I am able to fix the issue with a one-line change. This compilation failure is seen on the emacs src from
> top-of-the-master-branch.
> 
> Pls see if this fix is appropriate and consider accordingly.

Does this mean Clang doesn't support __attribute__ ((__fallthrough__))?

bug#41348: emacs compilation with clang10 fails in the file lib-src/etags.c

[Benjamin Riefenstahl]

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

Eli Zaretskii writes:
> Does this mean Clang doesn't support __attribute__ ((__fallthrough__))?

It looks to like it does support it.  But it complains, that there is a
non-switch label "resetfvdef:" (for goto) between the annotation and the
next "case".  The correct fix is probably to move "resetfvdef:" after
the "case" labels.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-patch, Size: 453 bytes --]

diff --git a/lib-src/etags.c b/lib-src/etags.c
index 174c33a7a5..5eb7504e67 100644
--- a/lib-src/etags.c
+++ b/lib-src/etags.c
@@ -4196,9 +4196,9 @@ C_entries (int c_ext, FILE *inf)
 	      break;
 	    }
 	  FALLTHROUGH;
-	resetfvdef:
 	case '#': case '~': case '&': case '%': case '/':
 	case '|': case '^': case '!': case '.': case '?':
+	resetfvdef:
 	  if (definedef != dnone)
 	    break;
 	  /* These surely cannot follow a function tag in C. */

[-- Attachment #3: Type: text/plain, Size: 7 bytes --]


benny

bug#41348: emacs compilation with clang10 fails in the file lib-src/etags.c

[Narayanan Nellayi]

Hi,

The following patch also works; this is in case removal of a "break"
statement requires either a "FALLTHROUGH" or a"goto" statement. The
previous patch was to just comment out FALLTHROUGH and the other
alternative is to replace FALLTHROUGH with a goto, though using goto
to jump to the next statement looks odd.

> diff --git a/lib-src/etags.c b/lib-src/etags.c
> index eee2c59626..b3d4642505 100644
> --- a/lib-src/etags.c
> +++ b/lib-src/etags.c
> @@ -4196,7 +4196,8 @@ C_entries (int c_ext, FILE *inf)
>               objdef = omethodsign;
>               break;
>             }
> -         FALLTHROUGH;
> +         // FALLTHROUGH;
> +         goto resetfvdef;
>         resetfvdef:
>         case '#': case '~': case '&': case '%': case '/':
>         case '|': case '^': case '!': case '.': case '?':

Options I used to build emacs:

./configure 'CFLAGS=-Ofast -march=skylake -funroll-loops
-fno-finite-math-only' CC=clang   \
            --with-mailutils --with-sound=yes --with-x-toolkit=gtk3
                                               \
            --with-gconf --with-modules --with-file-notification=yes
                                               \
            --with-xwidgets --with-xaw3d=yes --with-libsystemd=yes
                                         \
            --with-imagemagick=yes

Regards
Narayanan

bug#41348: emacs compilation with clang10 fails in the file lib-src/etags.c

[Narayanan Nellayi]

Hi Benny,

Thanks, what you suggested works and is cleaner than what I had in
mind (which is commenting out FALLTHROUGH or using a goto).

diff --git a/lib-src/etags.c b/lib-src/etags.c
index eee2c59626..4672e3491d 100644
--- a/lib-src/etags.c
+++ b/lib-src/etags.c
@@ -4197,9 +4197,9 @@ C_entries (int c_ext, FILE *inf)
              break;
            }
          FALLTHROUGH;
-       resetfvdef:
        case '#': case '~': case '&': case '%': case '/':
        case '|': case '^': case '!': case '.': case '?':
+       resetfvdef:
          if (definedef != dnone)
            break;
          /* These surely cannot follow a function tag in C. */

Thanks
Narayanan

On Sun, May 17, 2020 at 9:06 PM Benjamin Riefenstahl
<b.riefenstahl@turtle-trading.net> wrote:
>
> Eli Zaretskii writes:
> > Does this mean Clang doesn't support __attribute__ ((__fallthrough__))?
>
> It looks to like it does support it.  But it complains, that there is a
> non-switch label "resetfvdef:" (for goto) between the annotation and the
> next "case".  The correct fix is probably to move "resetfvdef:" after
> the "case" labels.
>
>
> benny