Alter parser to insert a use-statement for ChapelStandard rather than updating modUses and modUseList


On Jul 23, 2014, at 3:31 PM, Tom Hildebrandt <notifications@github.com<mailto:notifications@github.com>> wrote:


In compiler/AST/stmt.cpp:

> @@ -140,18 +141,24 @@ BlockStmt::copyInner(SymbolMap* map) {
>  }
>
>
> -void BlockStmt::replaceChild(Expr* old_ast, Expr* new_ast) {
> -  if (old_ast == blockInfo)
> -    blockInfo = toCallExpr(new_ast);
> +void BlockStmt::replaceChild(Expr* oldAst, Expr* newAst) {
> +  CallExpr* oldExpr = toCallExpr(oldAst);
> +  CallExpr* newExpr = toCallExpr(newAst);
> +
> +  if (oldExpr == NULL)


Why not also check that newExpr is non-NULL?
Even if this can be inferred from context, it ought to be either tested or listed in a header comment as the caller's responsibility.

—
Reply to this email directly or view it on GitHub<https://github.com/chapel-lang/chapel/pull/49/files#r15320416>.

I had thought about that but the original definition allowed new_ast to be NULL.  I can’t recall off the top
of my head how far I looked to see if there was every a case in which it is in fact NULL but one could
argue that it’s a valid value.

However I think it makes sense to make this concept a little clearer when newAst is being promoted to newExpr
rather than relying on toCallExpr to handle NULL gracefully.

Thanks!

Mike