Re: [PATCH] [GSOC] ref-filter: solve bugs caused by enumeration

ZheNing Hu [off-list ref] writes:

Hmph, I do no think that would be futureproof at all.  If a new atom
"pushe" gets added, it is not all that unlikely that it would add
its own member to the same union.  name here would be "pushe" and
starts with "push", and upon parsing "pushe", its own member in the
union, atom->u.X, would have been assigned to, but the code we see
here still accesses atom->u.remote_ref.*, so you still have the same
problem you started to solve, no?

The check we use in remote_ref_atom_parser() to see if the atom is
about pushing, i.e.

	if (!strcmp(atom->name, "push") || starts_with(atom->name, "push:"))

is unlikely to be invalidated in future changes, as this is very
much end-user facing and changing the condition will break existing
scripts, so that is what I was expecting you to use instead.

Hmph, I do no think that would be futureproof at all.  If a new atom
"pushe" gets added, it is not all that unlikely that it would add
its own member to the same union.  name here would be "pushe" and
starts with "push", and upon parsing "pushe", its own member in the
union, atom->u.X, would have been assigned to, but the code we see
here still accesses atom->u.remote_ref.*, so you still have the same
problem you started to solve, no?

Well, this "pushe" has `atom->u.X` which is different from "push" and
"push:". This is indeed worth worrying about.

The check we use in remote_ref_atom_parser() to see if the atom is
about pushing, i.e.

        if (!strcmp(atom->name, "push") || starts_with(atom->name, "push:"))

is unlikely to be invalidated in future changes, as this is very
much end-user facing and changing the condition will break existing
scripts, so that is what I was expecting you to use instead.

But I am afraid that the cost we paid for string matching here is too high,
Since the string matching here is to illustrate that we use one of the members
of the union, why can't we directly add a "union_type" member to used_atom?
if we have this, we can just switch the "union_type" flag and also make the
program correct. Perhaps this would be better than the large number of string
matches have done here.

Thanks.
--
ZheNing Hu