Opened 21 months ago

Last modified 19 months ago

#50913 new defect

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 Cc: RJVB (René Bertin)
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) 21 months ago.
src_misc_picture.c.diff (514 bytes) - added by majoc-at-astro (majoc-at-astro) 19 months ago.

Download all attachments as: .zip

Change History (22)

Changed 21 months ago by majoc-at-astro (majoc-at-astro)

comment:1 follow-up: Changed 21 months 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 21 months ago by ryandesign (Ryan Schmidt) (previous) (diff)

comment:2 in reply to: ↑ 1 Changed 21 months 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 21 months ago by ryandesign (Ryan Schmidt) (previous) (diff)

comment:3 Changed 21 months 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 follow-up: Changed 21 months 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 21 months ago by ryandesign (Ryan Schmidt)

  • Description modified (diff)

comment:6 follow-up: Changed 21 months ago by ryandesign (Ryan Schmidt)

  • Keywords yosemite added
  • Port VLC added; vlc removed
  • Summary changed from Build problem with VLC 2.2.2 under Mavericks to VLC @2.2.2: error: expected identifier or '('

You said Mavericks, but OS X 10.10 is Yosemite.

comment:7 in reply to: ↑ 6 Changed 21 months 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 21 months 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 21 months 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 follow-ups: Changed 21 months 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 ; follow-up: Changed 21 months 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 ; follow-up: Changed 21 months 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 21 months 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 ; follow-up: Changed 21 months 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 ; follow-up: Changed 21 months 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 ; follow-ups: Changed 21 months 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 21 months 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 follow-up: Changed 21 months 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 21 months 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 19 months ago by majoc-at-astro (majoc-at-astro)

comment:20 in reply to: ↑ 18 Changed 19 months 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.

Note: See TracTickets for help on using tickets.