On 24/08/2013 5:27 a.m., Kinkie wrote:
On Fri, Aug 23, 2013 at 9:21 AM, Amos Jeffries <squ...@treenet.co.nz> wrote:
On 23/08/2013 4:49 p.m., Alex Rousskov wrote:
On 08/22/2013 11:25 AM, Kinkie wrote:
- clang is very strict on the number of parentheses in conditionals:
as a way to double-check that the programmer knows what she's doing it
will accept as valid
if (foo==bar) {} and if ((foo=bar)) {} , and emit a warning (which
then -Wall turns into an error) in case of if ((foo==bar)) {} and if
(foo=bar) {}.
Makes sense.
This leads to bad results when the condition is a cpp macro, even
worse when the macro is defined in an external library.
Ouch. That is a clang bug IMHO.
How about making COMPILE_STACK_EMPTY and friends into functions instead
of deleting them?
- if (!COMPILE_STACK_EMPTY)
+ if (!compile_stack.avail == 0)
That would automatically avoid weird code like the one quoted above and
make code more self-documenting.
Better: if (!compile_stack.avail == false)
for avoiding the integer magic conversion please.
Well, compile_stack.avail is actually an unsigned integer.
I've turned it into a clearer syntax, but that's it.
- if (BN_is_zero(serial))
+ if BN_is_zero(serial) // BN_is_zero has its own set of surrounding
parentheses
Please no. Let's not violate the visual syntax of an if statement. If we
have to accommodate clang bugs, let's write
if (BN_is_zero(serial) == true)
or something like that.
Done.
Agreed.
Also ...
* in acinclude/compiler-flags.m4 I am worried that you are leaving
-Qunused-arguments present in the variable labeled Werror.
- IMO this needs to be a separate flag only added if a compiler flag check
proves it necessary.
It is necessary, unless we completely overhaul our build system: the
"unused arguments" it refers to are unused _compiler_ arguments. Such
as for instance include directories added to the search path but which
contain no headers needed by the current translation unit.
Clang complaining about those?
Two thing to keep in mind:
* For automake 1.13 we will shortly have to be having to remove the
INCLUDES macro in the .am and spread its contents between LDFLAGS,
CFLAGS, CXXFLAGS, etc properly.
- why is the -Wno-error=unused-value being added there now too?
I'm removing it, it's apparently unneeded.
Although some clang versions _are_ complainign about unused return
values somewhere in the CBDATA_CLASS2 macro definition. I wasn't able
to track where exactly so far.
Okay. If those are older than the clang built into FreeBSD 9/10 by
default (IIRC they were) we don't really care, they also lack C++11
functionality we want to make use of.
We have a separate section for the compiler-specific default flags right? if
not we need to add it.
I'm not sure about what you mean..
I seem to remember separate sectiosn of switch case and if statements
early and late in the configure.ac the one you are editing here setting
up the details for --enable-strict-error-detection variables. And a
later one doing compiler-specific and OS-specific tuning for things like
-g / -stdc++11, march-native, etc etc. That later flags setup was still
quite messy and spread over a few different if statements with switches
inside. (it has been nagging at my OCD tendencies every time I see bits
of it).
Amos
