kicad-developers team mailing list archive

[Ian McInerney <Ian.S.McInerney@xxxxxxxx>]

I thought that Wayne was agreeing with requiring them on all lengths of
statements (single line).

Although, now that I look at the code closer, I don't know if this is as
dire a problem. While it is a macro, it is defined like this:
    #define wxLogTrace
   \
        if ( !wxLog::IsLevelEnabled(wxLOG_Trace, wxLOG_COMPONENT) )
  \
        {}
   \
        else
   \
            wxMAKE_LOGGER(Trace).LogTrace

so it has an else statement already included in it. That means that any
else condition following it will be placed with the next higher level of if
statements outside the macro (and there is no worry about having an else-if
associated with this, since the else must come after all else-if
statements, meaning that this if condition is effectively ended).

The wxASSERT macro is an example of a case where they did introduce a guard
scope around the macro contents (by placing it inside a do-while loop that
only executes the first pass), since it is an if with only an else-if
statement and no else statement. If they hadn't given it that scoping, then
any else statements in user code that follows it would be associated with
the wxASSERT if statement instead.

It seems to me that on closer inspection, this may just be Coverity
throwing a false positive at us.

-Ian

On Sun, Jan 12, 2020 at 12:42 AM Jeff Young <jeff@xxxxxxxxx> wrote:

> We could also just require them on if/then/else statements….
>
> On 12 Jan 2020, at 00:35, Jeff Young <jeff@xxxxxxxxx> wrote:
>
> Sure, but unless we go with Seth’s option, then it’s just going to happen
> again….
>
> On 11 Jan 2020, at 23:28, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>
> I agree that adding the curly brackets would be the best option as well.
> It's less than ideal but it resolves the issue.
>
> On 1/11/20 6:21 PM, Ian McInerney wrote:
>
> That is probably the best option, since many things in wxWidgets are
> implemented as macros but masquerade as functions.
>
> -Ian
>
> On Sat, Jan 11, 2020 at 10:07 PM <seth@xxxxxxxxxxxxx
> <mailto:seth@xxxxxxxxxxxxx <seth@xxxxxxxxxxxxx>>> wrote:
>
>    I suppose that we could update our coding policy to require braces
>    even for single line statements.
>
>    -Seth
>
>    On Jan 11, 2020 1:28 PM, Jeff Young <jeff@xxxxxxxxx
>    <mailto:jeff@xxxxxxxxx <jeff@xxxxxxxxx>>> wrote:
>
>        This looks safe enough:
>
>        if( n_changed )
>            wxLogTrace( "CN", "Cluster %p : net : %d %s\n", cluster.get(),
>                    cluster->OriginNet(), (const char*)
> cluster->OriginNetName().c_str() );
>        else
>        wxLogTrace( "CN", "Cluster %p : nothing to propagate\n",
> cluster.get() );
>
>
>        Sadly, the macro wxLogTrace is not parenthesized, and starts
>        with an if statement.  So the else doesn’t go where you think it
>        does….
>
>        Any ideas on how to fix this that don’t include constantly
>        checking to see if new instances have been introduced?
>
>
>    _______________________________________________
>    Mailing list: https://launchpad.net/~kicad-developers
>    Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>    <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx
> <kicad-developers@xxxxxxxxxxxxxxxxxxx>>
>    Unsubscribe : https://launchpad.net/~kicad-developers
>    More help   : https://help.launchpad.net/ListHelp
>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>