> 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
> 
>

-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Reply via email to