Re: [cdt-dev] False positives with Codan using Avr-gcc toolchain

[Chris Marr <cdabone@xxxxxxxxx>]

I've submitted the patch to gerrit - this is the first time I've actually used eclipse and git together, this has been something of a learning curve, I appreciate the time you've taken to point me in the right direction, now I only hope I've done it right.

On Fri, Apr 25, 2014 at 7:19 PM, Nathan Ridge <zeratul976@xxxxxxxxxxx> wrote:

> I've reconsidered and modified my patch - please see attached, I'll
> look at testing and submitting via gerrit ASAP, in the meantime, can
> someone look at and check my code - I'm not familiar with the API and
> it's very possible I've made a silly mistake or assumed too much.

Thanks! A couple of comments:

Instead of modifying all the places where hasNoEffect() returns true,
we can just change the call site in visit() from

    if (hasNoEffect(_expression_))

to

    if (hasNoEffect(_expression_) && !isFinalStatementExpression(_expression_))

.

I would call 'isFinalStatementExpression' 'isFinalStatementInStatementExpression'
instead - we are not checking whether it's a "final statement-_expression_"
(whatever that means), we are checking whether it is the final statement
in a statement-_expression_.

IASTCompoundStatement has a getStatements() method. It will return
the same nodes as getChildren(), just as an IASTStatement[] rather than
an IASTNode[], but I think it's more conceptually correct to use
getStatements() in this case.

Your assumptions

    // NOTE: Presumes that the array returned is in the same order as presented in the source

and

    // NOTE: Presumes that the _expression_ is the actual _expression_, and not a copy.

hold. It is good to ask these questions when writing the patch, but there is no
need to have these comments in the final patch.

Regards,
Nate

_______________________________________________
cdt-dev mailing list
cdt-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/cdt-dev