Agree to the motive and +1 for the concept.

At Wed, 25 Aug 2021 15:00:13 +0100, Dagfinn Ilmari Mannsåker 
<ilm...@ilmari.org> wrote in 
> However, the patch adds:
> 
> > +typedef struct Null
> > +{
> > +   NodeTag         type;
> > +   char       *val;
> > +} Null;
> 
> which doesn't seem to be used anywhere. Is that a leftoverf from an
> intermediate development stage?

+1 Looks like so, it can be simply removed.

0001 looks fine to me.

0002:
  there's an "integer Value node" in gram.y: 7776.

-                       n = makeFloatConst(v->val.str, location);
+                       n = (Node *) makeFloatConst(castNode(Float, v)->val, 
location);

makeFloatConst is Node* so the cast doesn't seem needed. The same can
be said for Int and String Consts.  This looks like a confustion with
makeInteger and friends.

+       else if (IsA(obj, Integer))
+               _outInteger(str, (Integer *) obj);
+       else if (IsA(obj, Float))
+               _outFloat(str, (Float *) obj);

I felt that the type enames are a bit confusing as they might be too
generic, or too close with the corresponding binary types.


-       Node       *arg;                        /* a (Value *) or a (TypeName 
*) */
+       Node       *arg;

Mmm. It's a bit pity that we lose the generic name for the value nodes.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center


Reply via email to