<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/4370/">https://reviewboard.asterisk.org/r/4370/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 21st, 2015, 7:24 p.m. CET, <b>Corey Farrell</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/4370/diff/3/?file=71557#file71557line1065" style="color: black; font-weight: bold; text-decoration: underline;">/branches/11/configure.ac</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1057</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span>AC_MSG_RESULT(no)</pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1065</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="tb"> </span>AC_MSG_RESULT(no)</pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I missed this before / thought Josh pointed it out. This one should be AC_MSG_ERROR as well.</pre>
</blockquote>
<p>On March 11th, 2015, 11:34 p.m. CET, <b>Matt Jordan</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Replacing this with:
AC_MSG_ERROR("nested-functions is required for gcc")
Caused configure to fail:
checking for -Wshadow... yes
checking for -march=native support... yes
checking for gcc -fnested-functions... configure: error: "nested-functions is required for gcc"
Even though I did have nested functions....
So that's weird.
</pre>
</blockquote>
<p>On March 11th, 2015, 11:46 p.m. CET, <b>Matt Jordan</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'm actually not sure what -fnested-functions would even do. My google-foo is failing me on its use with GCC (which appears to not understand it), and since we only set that on the "false" path (that is, we failed to compile a program with a nested function), I'm not sure how it would help.
As it is, I think I will put an AC_MSG_ERROR in the 'false' path of the compilation.</pre>
</blockquote>
<p>On March 11th, 2015, 11:47 p.m. CET, <b>Corey Farrell</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">So this finding can be dropped, I was wrong. This line (and the next line with [AST_NESTED_FUNCTIONS=]) is the block that is run if AC_COMPILE_IFELSE succeeds without the -fnested-functions gcc flag. So we're not checking for nested functions, we're checking if nested functions fail without the flag.
Now if AC_COMPILE_IFELSE does fail we assume that it would succeed with the GCC flag. In theory we should verify that it would work with -fnested-functions. But that is a separate issue.</pre>
</blockquote>
<p>On March 11th, 2015, 11:49 p.m. CET, <b>Matt Jordan</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'm not even sure '-fnested-functions' does anything...</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">See: https://gcc.gnu.org/onlinedocs/gcc/Nested-Functions.html
GCC Nested functions are actually used inside the gcc version of the RAII_VAR macro on the second line, to declare block local function which is called by the cleanup function to execute the desctuctor. This function gets embedded locally in the function where the RAII_VAR macro is called, creating a nested function in the process, ie:
void _dtor_ ## varname (vartype * v) {
dtor(*v);
}
called by:
vartype varname __attribute__((cleanup(_dtor_ ## varname))) = (initval)
Nested functions are somewhat similar to the clang blocks extension (though clang's version is much more powerfull in most, respects, except for using them by cleanup directly, which is were this whole patch is coming in).
In all this also means that if -fnested-functions is not supported in gcc, then RAII_VAR on gcc will fail and should be checked and cause a configure error, in my opinion. Gcc's nested-functions have been around since version 3.0.4 i think, by default enabled in gcc, but disabled (requiring -fnested-functions) when using llvm-gcc.</pre>
<br />
<p>- Diederik</p>
<br />
<p>On February 21st, 2015, 3:35 a.m. CET, Matt Jordan wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers and Diederik de Groot.</div>
<div>By Matt Jordan.</div>
<p style="color: grey;"><i>Updated Feb. 21, 2015, 3:35 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-20850">ASTERISK-20850</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">* 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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/11/makeopts.in <span style="color: grey">(432053)</span></li>
<li>/branches/11/main/Makefile <span style="color: grey">(432053)</span></li>
<li>/branches/11/include/asterisk/utils.h <span style="color: grey">(432053)</span></li>
<li>/branches/11/include/asterisk/inline_api.h <span style="color: grey">(432053)</span></li>
<li>/branches/11/configure.ac <span style="color: grey">(432053)</span></li>
<li>/branches/11/configure <span style="color: grey">(UNKNOWN)</span></li>
<li>/branches/11/Makefile <span style="color: grey">(432053)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/4370/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>