#1518: Parameter parser and CCTK_ParameterSet interpret leading zeros in numbers differently ---------------------+------------------------------------------------------ Reporter: rhaas | Owner: sbrandt Type: defect | Status: review Priority: minor | Milestone: Component: Cactus | Version: development version Resolution: | Keywords: ---------------------+------------------------------------------------------
Comment (by hinder):
What "namespace" are the temporaries set in? Can codes create their own space to avoid conflicts with existing temporaries? Out of caution, I would comment out or otherwise disable the setting of temporaries using the flesh expression parser in case it causes problems. It could then be enabled if people need the feature and it is worth the risk of confusion.
This patch introduces a new type "sval", I assume for string valued expressions. Existing code will not check exhaustively for the type of the expression. I doubt this is a problem in practice, as the expression parser code is rarely used outside the flesh I think.
There is a switch statement at the top of the patch which does not have a default case. Please add a fatal error message here.
Do I understand correctly that you are replacing the Util_Expression* flesh API with a different API? Doesn't this break backward compatibility? Should there be a wrapper introduced? Then the changes to Groups.c and Parameters.c would not be needed.
I support replacing the expression parser with Piraha, but I don't think we should change/eliminate the Util_* expression API. Did I misunderstand what the patch was doing?