Skip to main content

[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
 		 	   		  

Back to the top