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

[Diederik de Groot]

> On Feb. 20, 2015, 6:03 p.m., Corey Farrell wrote:
> > /branches/11/include/asterisk/utils.h, lines 946-948
> > <https://reviewboard.asterisk.org/r/4370/diff/2/?file=71539#file71539line946>
> >
> >     I feel that configure should create a #define for the type of nested procedure supported.  This should use results of configure (#ifdef HAVE_CLANG_BLOCKS), instead of implementing it's own direct tests.
> 
> Matt Jordan wrote:
>     Hm. I'm not sure how much better that makes this, since I find parsing out a configure.ac file to be much harder than the #defines in a header.
>     
>     What specifically are you envisioning? Right now, I'm having a hard time determining how this would look in the configure script and how it would be reflected here.
> 
> Corey Farrell wrote:
>     The idea is that configure is responsible for testing features and reporting what was found, so we shouldn't attempt to duplicate the logic of those tests.  The tests performed in configure are better anyways.
>     
>     In configure.ac where it finds working CLANG support, we want to add:
>     AC_DEFINE([HAVE_CLANG_BLOCKS], 1, [Define to 1 if your compiler supports CLANG blocks.])
>     And when it finds GCC nested functions:
>     AC_DEFINE([HAVE_GCC_NESTED_FUNCTIONS], 1, [Define to 1 if your compiler supports GCC nested functions.])
>     
>     Then when you run bootstrap / configure, you will get the appropriate #define / #undef in asterisk/autoconfig.h.
>     
>     Then in asterisk/utils.h:
>     #if defined(HAVE_CLANG_BLOCKS)
>     /* CLANG support code */
>     #elif defined(HAVE_GCC_NESTED_FUNCTIONS)
>     /* GCC support code */
>     #else
>     #error
>     #endif

This almost sounds like replacing one define for another. This is not being done for other (gcc) builtin defines/attributes in other parts of the code.
It might also make crosscompiling a lot more difficult, clang might take the destination into account when evaluating the __has_feature() (crosscompilation should be tested/checked, to check what happens). 


- Diederik


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


On Feb. 21, 2015, 3:35 a.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4370/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2015, 3:35 a.m.)
> 
> 
> Review request for Asterisk Developers and Diederik de Groot.
> 
> 
> Bugs: ASTERISK-20850
>     https://issues.asterisk.org/jira/browse/ASTERISK-20850
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This is a continuation of the patch put up for review on r3488. It addresses the issues found on that review.
> 
> This patch *should* make Asterisk compile under clang. Note that compiling with --enable-dev-mode will cause Asterisk to fail to compile under clang, as it detects a number of warnings that aren't fixed under this patch.
> 
> 
> Diffs
> -----
> 
>   /branches/11/makeopts.in 432053 
>   /branches/11/main/Makefile 432053 
>   /branches/11/include/asterisk/utils.h 432053 
>   /branches/11/include/asterisk/inline_api.h 432053 
>   /branches/11/configure.ac 432053 
>   /branches/11/configure UNKNOWN 
>   /branches/11/Makefile 432053 
> 
> Diff: https://reviewboard.asterisk.org/r/4370/diff/
> 
> 
> Testing
> -------
> 
> * Compiled Asterisk with and without --enable-dev-mode using gcc. Asterisk compiles correctly.
> * Compiled Asterisk without --enable-dev-mode using clang. Asterisk compiles, links, and executes.
> 
> Note that you will need the BlocksRuntime to run Asterisk when it is compiled with clang.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

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