[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
Re: [cdt-dev] False positives with Codan using Avr-gcc toolchain
|
> 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