Opened 3 years ago

Closed 3 months ago

Last modified 3 months ago

#50913 closed defect (fixed)

VLC @2.2.2: error: expected identifier or '('

Reported by: majoc-at-astro (majoc-at-astro) Owned by: macports-tickets@…
Priority: Normal Milestone:
Component: ports Version: 2.3.4
Keywords: yosemite haspatch Cc: RJVB (René Bertin), mojca (Mojca Miklavec)
Port: VLC

Description (last modified by ryandesign (Ryan Schmidt))

VLC fails to build for me on 10.10, with a somewhat bizarre failure msg (please see enclosed build log):

:info:build misc/picture.c:43:1: error: expected identifier or '('

This is on a completely clean and fully-patched Mavericks system (10.10.5, 14F1605) with a freshly-installed XCode toolset (7.2.1, 7C1002), as well as on our production MacPorts 10.10 build server.

The break appears to coincide with the transition from VLC v2.1.5 to v2.2.2 (XCode had already been updated to v7.2, and built 2.1.5 successfully). Curiously, our 10.11 server didn't turn a hair, and builds v2.2.2 happily.

Do please let me know if there's any other information you need, or if there's anything spectacularly stupid I'm doing wrong. Apologies if I'm not filling in the boxes correctly here.

Attachments (2)

vlc.main.log (680.8 KB) - added by majoc-at-astro (majoc-at-astro) 3 years ago.
src_misc_picture.c.diff (514 bytes) - added by majoc-at-astro (majoc-at-astro) 3 years ago.

Download all attachments as: .zip

Change History (26)

Changed 3 years ago by majoc-at-astro (majoc-at-astro)

Attachment: vlc.main.log added

comment:1 Changed 3 years ago by RJVB (René Bertin)

Could you check if sizeof (uintptr_t) == sizeof (atomic_uintptr_t and alignof (uintptr_t) == alignof (atomic_uintptr_t) using the same compiler and options as used for building picture.c ?

This (almost) looks like a compiler bug to me, as I don't see how there could be a bracket balancing error if the preprocessor does its job correctly. Maybe it gets confused by the brackets in the string (you could try removing those).

Either way that would explain why you don't get issues on 10.11 (newer compiler) and I didn't either on 10.9 (__STDC_VERSION_ < 201112L or older compiler version without the bug).

Last edited 3 years ago by ryandesign (Ryan Schmidt) (previous) (diff)

comment:2 in reply to:  1 Changed 3 years ago by majoc-at-astro (majoc-at-astro)

Replying to rjvbertin@…:

Could you check if sizeof (uintptr_t) == sizeof (atomic_uintptr_t and alignof (uintptr_t) == alignof (atomic_uintptr_t) using the same compiler and options as used for building picture.c ?

I've been too long away from C, but I find so far that the build uses the system clang:

Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin14.5.0
Thread model: posix

Either way that would explain why you don't get issues on 10.11 (newer compiler) and I didn't either on 10.9 (__STDC_VERSION_ < 201112L or older compiler version without the bug).

For comparison, the 10.11 system's clang shows me:

Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.3.0
Thread model: posix

I'll dig into this further next week, inc a proper test build on 10.11 for better comparison. I'm short of minutes tonight, I'm afraid.

Last edited 3 years ago by ryandesign (Ryan Schmidt) (previous) (diff)

comment:3 Changed 3 years ago by RJVB (René Bertin)

If necessary I can write a simple C test file, but I cannot run it for you. However, it would appear that 10.10 and 10.11 use the exact same compiler version, so my idea of a bug seems unlikely.

The annoying thing here is that this is a test that clearly should be run during the configure step, because the asserts will cause the compilation and thus the build to abort.

Now, if we could be sure that the asserts will never trigger on a system running 10.10 I could simply remove the code through a patch...

comment:4 Changed 3 years ago by RJVB (René Bertin)

Something else to try:

(cd `port work vlc`/vlc-2.2.2/src ; /usr/bin/clang -DHAVE_CONFIG_H -I. -I..   -I/opt/local/include -DMODULE_STRING=\"core\" -DLOCALEDIR=\"/opt/local/share/locale\" -DPKGDATADIR=\"/opt/local/share/vlc\" -DPKGLIBDIR=\"/opt/local/lib/vlc\" -DHAVE_DYNAMIC_PLUGINS  -I../include -I../include -I/opt/local/include -D__unix__=1 -I/opt/local/lib/live/liveMedia/include -I/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_multimedia_VLC/VLC/work/vlc-2.2.2/contrib/include   -pipe -Os -arch x86_64 -D_INTL_REDIRECT_MACROS -I/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_multimedia_VLC/VLC/work/vlc-2.2.2/contrib/include -Wall -Wextra -Wsign-compare -Wundef -Wpointer-arith -Wbad-function-cast -Wwrite-strings -Wmissing-prototypes -Wvolatile-register-var -Werror-implicit-function-declaration -pipe -fvisibility=hidden -O3 -ffast-math -funroll-loops -fomit-frame-pointer  -CC -dD -E -o ~/Desktop/vlc_misc_picture.i misc/picture.c)

on both systems and compare the resulting vlc_misc_picture.i files?

Another thing to check is whether that compile command (from your attached vlc.main.log) is identical on 10.11 !

comment:5 Changed 3 years ago by ryandesign (Ryan Schmidt)

Description: modified (diff)

comment:6 Changed 3 years ago by ryandesign (Ryan Schmidt)

Keywords: yosemite added
Port: VLC added; vlc removed
Summary: Build problem with VLC 2.2.2 under MavericksVLC @2.2.2: error: expected identifier or '('

You said Mavericks, but OS X 10.10 is Yosemite.

comment:7 in reply to:  6 Changed 3 years ago by majoc-at-astro (majoc-at-astro)

Replying to ryandesign@…:

You said Mavericks, but OS X 10.10 is Yosemite.

s/Mavericks/Yosemite/g please. My brain fart.

comment:8 Changed 3 years ago by RJVB (René Bertin)

Bah, rather Apple's brain fart. There's no more logic at all in their OS code names.

Meanwhile here's something else you can try: install port:clang-3.8 and try to build VLC using that compiler, e.g.

> sudo port -vs install VLC configure.compiler=macports-clang-3.8

comment:9 in reply to:  4 Changed 3 years ago by majoc-at-astro (majoc-at-astro)

Replying to rjvbertin@…:

Something else to try:

(cd `port work vlc`/vlc-2.2.2/src ; /usr/bin/clang -DHAVE_CONFIG_H -I. -I..   -I/opt/local/include -DMODULE_STRING=\"core\" -DLOCALEDIR=\"/opt/local/share/locale\" -DPKGDATADIR=\"/opt/local/share/vlc\" -DPKGLIBDIR=\"/opt/local/lib/vlc\" -DHAVE_DYNAMIC_PLUGINS  -I../include -I../include -I/opt/local/include -D__unix__=1 -I/opt/local/lib/live/liveMedia/include -I/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_multimedia_VLC/VLC/work/vlc-2.2.2/contrib/include   -pipe -Os -arch x86_64 -D_INTL_REDIRECT_MACROS -I/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_multimedia_VLC/VLC/work/vlc-2.2.2/contrib/include -Wall -Wextra -Wsign-compare -Wundef -Wpointer-arith -Wbad-function-cast -Wwrite-strings -Wmissing-prototypes -Wvolatile-register-var -Werror-implicit-function-declaration -pipe -fvisibility=hidden -O3 -ffast-math -funroll-loops -fomit-frame-pointer  -CC -dD -E -o ~/Desktop/vlc_misc_picture.i misc/picture.c)

on both systems and compare the resulting vlc_misc_picture.i files?

Tentatively bingo. A diff of the respective picture.i files shows:

--- vlc_mist_picture.i.10-10    2016-03-21 11:35:28.000000000 +0000
+++ vlc_mist_picture.i.10-11    2016-03-21 11:35:18.000000000 +0000
[snipissimo]
 # 43 "misc/picture.c" 2
-((void) sizeof (struct { unsigned a[(sizeof (uintptr_t) == sizeof (atomic_uintptr_t)) ? 1 : -1];}));
-
-((void) sizeof (struct { unsigned a[(_Alignof (uintptr_t) == _Alignof (atomic_uintptr_t)) ? 1 : -1];}));
-
+_Static_assert(sizeof (uintptr_t) == sizeof (atomic_uintptr_t),
+              "Please compile in C99 mode (or update to LibVLC 3.0).");
+_Static_assert(_Alignof (uintptr_t) == _Alignof (atomic_uintptr_t),
+              "Please compile in C99 mode (or update to LibVLC 3.0).");
 
 
 /**

Looking further in, it would appear /usr/include/assert.h under 10.10 doesn't have a static_assert() definition; VLC then adds _Static_assert() (in ../include/vlc_fixups.h) as a macro which I don't pretend to understand, but which appears to at best be being misexpanded.

Another thing to check is whether that compile command (from your attached vlc.main.log) is identical on 10.11 !

They are indeed identical. Well called.

comment:10 Changed 3 years ago by RJVB (René Bertin)

Can you check in vlc_mist_picture.i.10-10 if you do have a definition for _Static_assert. Or rather, simply try to build after placing the 10.11 definition for static_assert (from /usr/include/assert.h) above its first use in picture.c ?

If that works, please post that definition as it'll make a useful workaround/patch.

VLC's macro asks the compiler to evaluate the sizeof of a struct containing an array; this array has 1 element when the assert is OK, but -1 elements when that is not the case. In the latter case the compiler is supposed to abort.

The macro expansions do look correct as far as bracket balancing is concerned!

@Ryan: can you check if the 10.10 build bot ran into the same issue please?

comment:11 in reply to:  10 ; Changed 3 years ago by ryandesign (Ryan Schmidt)

Replying to rjvbertin@…:

@Ryan: can you check if the 10.10 build bot ran into the same issue please?

I'm not sure what exactly you're asking me to check, but if you want to know if a build succeeded or failed, you can check on our buildbot web site.

comment:12 in reply to:  10 ; Changed 3 years ago by majoc-at-astro (majoc-at-astro)

Replying to rjvbertin@…:

Can you check in vlc_mist_picture.i.10-10 if you do have a definition for _Static_assert.

/* locale.h */
# 243 "../include/vlc_fixups.h"
#define _Static_assert(x,s) ((void) sizeof (struct { unsigned a[(x) ? 1 : -1];}))
#define static_assert _Static_assert

Or rather, simply try to build after placing the 10.11 definition for static_assert (from /usr/include/assert.h) above its first use in picture.c ?

I'll try it, but more in hope than expectation. The macro makes more sense now you explain it, though I can't see how it emits the message if the assertion fails.

Ay well: back to the day job.

comment:13 in reply to:  11 Changed 3 years ago by RJVB (René Bertin)

Replying to ryandesign@…:

I'm not sure what exactly you're asking me to check, but if you want to know if a build succeeded or failed, you can check on our buildbot web site.

I don't appear to have access to all parts of that site, and haven't managed to find even whether the VLC port was built successfully for 10.10 .

comment:14 in reply to:  12 ; Changed 3 years ago by RJVB (René Bertin)

Replying to majoc@…:

Can you check in vlc_mist_picture.i.10-10 if you do have a definition for _Static_assert.

So you only have the definition from vlc_fixups.h, apparently. Is there a definition for _Static_assert in vlc_mist_picture.i.10.11 ? I'm trying to figure out if that "function" is a built-in function (or part of the syntax) or not. If it is, i.e. if the 10.11 preprocessed file doesn't show an implementation, then it should be possible simply to invoke _Static_assert instead of static_assert, in picture.c .

And of course if 10.11 provides an implementation for _Static_assert, we could try to use that one.

I'll try it, but more in hope than expectation. The macro makes more sense now you explain it, though I can't see how it emits the message if the assertion fails.

I think that the compiler prints an error message showing the original code, not the preprocessed/expanded code (which is why it's often so difficult to figure out errors in macros). There's thus no need for the macro to do anything with the error message.

Ay well: back to the day job.

Good for you :)

comment:15 in reply to:  14 ; Changed 3 years ago by majoc-at-astro (majoc-at-astro)

Replying to rjvbertin@…:

Replying to majoc@…:

Can you check in vlc_mist_picture.i.10-10 if you do have a definition for _Static_assert.

So you only have the definition from vlc_fixups.h, apparently. Is there a definition for _Static_assert in vlc_mist_picture.i.10.11 ?

Nope, nor anywhere under /usr/include (otherwise ./configure would have found it, methinks).

I'll try it, but more in hope than expectation. The macro makes more sense now you explain it, though I can't see how it emits the message if the assertion fails.

I think that the compiler prints an error message showing the original code, not the preprocessed/expanded code (which is why it's often so difficult to figure out errors in macros). There's thus no need for the macro to do anything with the error message.

Unless, as in this case, the error message is on the next line .... which leads me to wonder whether what we're seeing is actually a static assertion failure which happens to be silent. I'll kludge up a test prog to show us the values being checked, and get back to you.

Ay well: back to the day job.

Good for you :)

As I filked long enough ago that I'm embarrassed: "I'm only what I'm paid to be/In my copious free time."

comment:16 in reply to:  15 ; Changed 3 years ago by RJVB (René Bertin)

Replying to majoc@…:

Nope, nor anywhere under /usr/include

So you could try to build the file after putting

#define static_assert _Static_assert

before including vlc_fixup.h

(otherwise ./configure would have found it, methinks).

Only if it checks, and I don't think that's the case (the check is being done by the compiler, after configure has been run!)

Unless, as in this case, the error message is on the next line .... which leads me to wonder whether what we're seeing is actually a static assertion failure which happens to be silent. I'll kludge up a test prog to show us the values being checked, and get back to you.

Yes, that's why I asked you to check the actual assertions in my first message.

I kind of expect those assertions to fail only on 32bit hardware (or when compiling in 32bit mode).

As I filked long enough ago that I'm embarrassed: "I'm only what I'm paid to be/In my copious free time."

Paid with copious free time, since long ago? Sounds like it could be worse ;)

comment:17 in reply to:  16 Changed 3 years ago by majoc-at-astro (majoc-at-astro)

Replying to rjvbertin@…:

Replying to majoc@…:

I'll kludge up a test prog to show us the values being checked, and get back to you.

Yes, that's why I asked you to check the actual assertions in my first message.

.... computer (eventually) says "8", twice. Apologies for delay.

comment:18 Changed 3 years ago by RJVB (René Bertin)

So, the asserts are not supposed to trigger.

Can you please try the define mentioned in my previous post as the next thing you test on 10.10?

When I look at later versions of picture.c , both static_asserts are gone and I cannot find anything in the git history that shows when and why they were removed.

@Ryan: what do you think, is there any chance that these asserts will ever fail when building this VLC port, e.g. because a pre-C99 flavour is used? If not, it may be easiest to remove the whole #ifdeffed block.

comment:19 in reply to:  16 Changed 3 years ago by majoc-at-astro (majoc-at-astro)

Replying to rjvbertin@…:

So you could try to build the file after putting

#define static_assert _Static_assert

before including vlc_fixup.h

--- ../config.h.orig	2016-03-22 15:34:33.000000000 +0000
+++ ../config.h	2016-03-22 15:34:57.000000000 +0000
@@ -793,6 +793,7 @@
 /* Define to `int' if <stddef.h> does not define. */
 /* #undef ssize_t */
 
+#define static_assert _Static_assert
 #include <vlc_fixups.h>
 
 

.... after which "clang -c" compiles an unchanged picture.c successfully. We're getting there :-) .

Changed 3 years ago by majoc-at-astro (majoc-at-astro)

Attachment: src_misc_picture.c.diff added

comment:20 in reply to:  18 Changed 3 years ago by majoc-at-astro (majoc-at-astro)

Replying to rjvbertin@…:

When I look at later versions of picture.c , both static_asserts are gone and I cannot find anything in the git history that shows when and why they were removed.

The file src_misc_picture.c.diff (attached) makes that so naiively; after applying it in port work vlc, VLC builds successfully on 10.10. Apologies if this isn't in the correct format, or is the wrong fix.

comment:21 Changed 3 months ago by mojca (Mojca Miklavec)

Keywords: haspatch added

What is the status of this ticket?

Is this still an issue (a few versions have been updated in the meantime)? If yes, it might be nice to ask upstream. Also, patches are nowadays much quicker acted upon when submitted as a PR.

comment:22 Changed 3 months ago by mojca (Mojca Miklavec)

Cc: mojca added

comment:23 Changed 3 months ago by ryandesign (Ryan Schmidt)

Resolution: fixed
Status: newclosed

Response received by email:

From: Dr M J Carter <Martin.Carter@physics...>
Subject: Re: [MacPorts] #50913: VLC @2.2.2: error: expected identifier or '('
Date: September 24, 2018 at 07:35:04 CDT
To: macports-dev@lists...
List-Id: MacPorts developer discussion <macports-dev.lists.macports.org>

On Sun, Sep 23, 2018 at 12:17:50AM +0000, MacPorts wrote:

> Is this still an issue (a few versions have been updated in the
> meantime)?  If yes, it might be nice to ask upstream.

We've long since snipped it from production.  Apologies for not having
updated the ticket to say so.

-- 
Dr Martin J Carter
Computer System Administrator
Astrophysics, University of Oxford

So I guess that means the reporter no longer uses VLC so we don't know if it's still an issue.

comment:24 Changed 3 months ago by RJVB (René Bertin)

We haven't seen any comparable bug reports about the current port:VLC version anyway, have we?

Note: See TracTickets for help on using tickets.