Summary:	ASTERISK-20850: [patch]Nested functions aren't portable. Adapting RAII_VAR to use clang/llvm blocks to get the same/similar functionality.
Reporter:	Diederik de Groot (dkdegroot)	Labels:
Date Opened:	2012-12-31 20:24:29.000-0600	Date Closed:	2015-03-12 07:29:21
Priority:	Major	Regression?	Yes
Status:	Closed/Complete	Components:	Core/BuildSystem
Versions:	11.1.0	Frequency of Occurrence	Constant
Related Issues:	is a clone of ASTERISK-23666 CLONE - nested functions aren't portable duplicates ASTERISK-24133 [patch]Please support Clang; Allow no-exec stacks is related to ASTERISK-20399 Compilation on some systems requires the -fnested-functions flag
Environment:	CLANG	Attachments:	( 0) RAII_CLANG.patch
Description:	Nested functions are a GCC extension that is not sanctioned by C standards, thus the use of them is non-portable.  Specifically, CLANG does not support them.  CLANG is used by OSX, current versions of FreeBSD, and NetBSD is experimenting with it.  The RAII_VAR macro in include/asterisk/utils.h creates nested functions.  This prevents Asterisk 11.* from compiling with CLANG.
Comments:	By: Matt Jordan (mjordan) 2013-01-02 08:05:45.935-0600 We are well aware that RAII_VAR uses gcc extensions. The purpose of the macro is to provide scoped cleanup of local variables; this has been immensely useful throughout the codebase. This was an intentional decision during the development of Asterisk 11, as many portions of the codebase benefited from the use of the RAII_VAR macro. By: John Nemeth (jnemeth) 2013-07-20 02:53:29.058-0500 I am certainly not disputing the usefulness of RAII_VAR, just that the implementation is going to cause problems.  There are multiple OSes (NetBSD, FreeBSD, OSX, possibly others) that are currently evaluating and are likely to move to clang, which doesn't support nested functions.  At that point Asterisk would no longer be compilable on those systems. If I were to provide a tested alternative implementation that doesn't use nested functions would you consider it?  I'm not asking if you will accept it, just if you would look at it? By: Matt Jordan (mjordan) 2013-07-20 15:00:10.416-0500 Yes! If there was a mechanism such that the macro could be #ifdef'd to provide an alternative, we would absolutely include it. By: Diederik de Groot (dkdegroot) 2013-10-09 10:50:06.966-0500 Example of a possible clang solution (using blocks & cleanup): [Edit by Rusty - Removed inline patch as per Issue tracker guidelines] Can someone please look into this, and see if this is a viable solution. If it is, it would make clang/scan-build possible compilers for the asterisk project again. scan-build might actually be very helpfull. [Edit by Rusty - Updating this comment based on the comment on the clone ASTERISK-23666 that was erroneously created. Plus, re-opening this issue.] By: Rusty Newton (rnewton) 2014-04-25 17:54:41.873-0500 [~dkdegroot] Take a look through the [Code Revew process|https://wiki.asterisk.org/wiki/display/AST/Code+Review] and post the patch on reviewboard. That system is designed to facilitate peer review. Before that step you'll need to sign a license agreement and *attach* your patch to this issue as a code contribution for it to be considered. Thanks! By: Diederik de Groot (dkdegroot) 2014-04-26 04:36:05.554-0500 Thank you Rusty for Reopening this issue and sending me on the right track ! Accepted the Agreement, waiting for confirmation. Will post on the reviewboard next week. By: Diederik de Groot (dkdegroot) 2014-04-28 08:07:10.853-0500 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 can be announced. By: Diederik de Groot (dkdegroot) 2014-04-28 08:27:47.316-0500 Posted review on Review-Board: 3488 By: Matt Jordan (mjordan) 2015-01-25 16:27:44.771-0600 Sorry I didn't have a chance to test this patch further until now. I've commented on your review, fixed a few more findings, cleaned up the {{AST_INLINE_API}} issues, and put up a patch for Asterisk 11 here: https://reviewboard.asterisk.org/r/4370/