[asterisk-dev] [Code Review] 3488: RAII_VAR: nested functions aren't portable. Adapting RAII_VAR to use clang/llvm blocks to get the same/similar functionality.

[Diederik de Groot]

> On Jan. 25, 2015, 11:25 p.m., Matt Jordan wrote:
> > /trunk/configure.ac, lines 1094-1097
> > <https://reviewboard.asterisk.org/r/3488/diff/2/?file=58453#file58453line1094>
> >
> >     This will actually always fail, causing gcc to fail to compile Asterisk.
> >     
> >     The AC_LANG_PROGRAM macro will expand the second argument as the contents of int main(void). This causes the following program to be generated:
> >     
> >     int main(void)
> >     {
> >         #if defined (__clang__)
> >         choke
> >         #end if
> >         int main(void) { return 0 };
> >     
> >     }
> >     
> >     Since this is invalid C code even for gcc, this will cause the Clang macros to always be applied. As a result, the Makefile will always add "-fblocks" to the _ASTCFLAGS, which will cause gcc to fail to compile.
> >     
> >     Changing the AC_LANG_PROGRAM macro to:
> >     
> >     AC_LANG_PROGRAM([], [
> >         #if defined(__clang__)
> >         choke
> >         #endif
> >     ],
> >     ...
> >     
> >     Should resolve the issue for gcc and still allow for detection of the compiler options for Clang.

Ok super... Must have missed that.


> On Jan. 25, 2015, 11:25 p.m., Matt Jordan wrote:
> > /trunk/include/asterisk/utils.h, lines 987-1009
> > <https://reviewboard.asterisk.org/r/3488/diff/2/?file=58454#file58454line987>
> >
> >     I'd propose changing this slightly.
> >     
> >     Clang, unfortunately, does define the __GNUC__ macro, so we can't rely on it to know for sure that we have GCC as the compiler or Clang. On the other hand, as our configure script shows, we do know that Clang will define the __clang__ macro, which we can be pretty sure that GCC will not define. As such, we can rewrite this as:
> >     
> >     #if defined(__clang__) && defined(__has_feature)
> >     
> >     #elif defined(__GNUC__)
> >     
> >     #else
> >         #warning
> >     #endif
> >     
> >     I prefer this nomenclature to a #ifndef of __has_feature simply because we (a) don't define __has_feature, which may be used a test in other places; and (b) it is a bit clearer from reading the code that we are testing explicitly for Clang. The __has_feature is a bit ambiguous, unless you are intimately familiar with the Clang provided macros.

Got it !


- Diederik


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3488/#review14277
-----------------------------------------------------------


On May 15, 2014, 10:57 a.m., Diederik de Groot wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3488/
> -----------------------------------------------------------
> 
> (Updated May 15, 2014, 10:57 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-20850
>     https://issues.asterisk.org/jira/browse/ASTERISK-20850
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> RAII_VAR: nested functions aren't portable. Adapting RAII_VAR to use clang/llvm blocks to get the same/similar functionality.
> Making it possible again to use clang as a compiler, instead of depending on gcc completely.
> 
> Compile instructions:
> ================
> ./bootstrap.sh
> CC=clang CXX=clang++ ./configure --enable-dev-mode
> Needed to set DISABLE_INLINE to get passed the double declaration error in api-inline.h, i guess someone needs to figure out how to get this passed clang, correctly
> make menuselect.makeopts
> menuselect/menuselect --enable DISABLE_INLINE
> Needed to suppresse some of the warnings to get clang to compile (for now), clang can be a little panicky, but for a good reason.
> 
>     -Wno-unknown-warning-option. When gcc doesn't know a compiler option it returns NON-ZERO errorlevel, clang returns ZERO errorlevel, which is annoying. Even the linux kernel developers group complained about this. Will be fixed/changed (hopefully soon). For now, when checking clang compiler options, you would need to grep and parse the error output
>     -Wno-error needed to quite down clang being panicky (Standard asterisk -Werror is a good idea in general, but not when compiling against a 'new' compiler )
> 
> ASTCFLAGS="-Wno-unknown-warning-option -Wno-error" make
> make install
> RAII_VAR seems to work, but i guess there is still a bit of work before clang support for the rest of asterisk can be announced.
> 
> 
> Diffs
> -----
> 
>   /trunk/makeopts.in 413043 
>   /trunk/main/Makefile 413043 
>   /trunk/include/asterisk/utils.h 413043 
>   /trunk/configure.ac 413043 
>   /trunk/configure UNKNOWN 
>   /trunk/Makefile 413043 
> 
> Diff: https://reviewboard.asterisk.org/r/3488/diff/
> 
> 
> Testing
> -------
> 
> Just a proof of concept, to show how asterisk could be made clang compatible again.
> 
> 
> Thanks,
> 
> Diederik de Groot
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150126/dd73d867/attachment-0001.html>