Now I'm second-guessing myself about the indexes used to access the operator and operands. All the C++ functions used to access the operands index them from 0, as expected. It seems foolish and confusing to index them from 1 in these bang operators.
One option is to index the operator as -1 and the operands from 0.
A second option is to remove the ability to index the operator and retain the !getop and !setop bangs for that purpose, possibly renamed to !getdagop and !setdagop.
Opinions, please.
I generally like the proposal.
Now I'm second-guessing myself about the indexes used to access the operator and operands. All the C++ functions used to access the operands index them from 0, as expected. It seems foolish and confusing to index them from 1 in these bang operators.
One option is to index the operator as -1 and the operands from 0.
A second option is to remove the ability to index the operator and retain the !getop and !setop bangs for that purpose, possibly renamed to !getdagop and !setdagop.
That's a good thought. It seems like a good idea to be consistent with
how things look from a backend's perspective in C++, so separate
!get(dag)op and !set(dag)op seems slightly better. Just my opinion 
Cheers,
Nicolai
I think I will do both. In order that dag(i) can work and fetch the operator, an integer index is needed for it. So -1 for the operator. Then that should be supported in the other bang operators.
Meanwhile, I will rename !getdagop and !setdagop just for consistency. The old !getop and !setop will remain, of course, but deprecated. !getop is used 0 times and !setop 4 times.
~~ Paul
I think I will do both. In order that dag(i) can work and fetch the operator, an integer index is needed for it. So -1 for the operator. Then that should be supported in the other bang operators.
Meanwhile, I will rename !getdagop and !setdagop just for consistency. The old !getop and !setop will remain, of course, but deprecated. !getop is used 0 times and !setop 4 times.
If it's really only used 4 times, it may be worth just removing those instances instead of keeping the deprecated versions around.
Cheers,
Nicolai
But aren't we concerned about users of TableGen outside of the LLVM project? I've come across at least one in my travels through the interwebs.
But aren't we concerned about users of TableGen outside of the LLVM project? I've come across at least one in my travels through the interwebs.
True, it's fair to give them a heads-up. Depending on the user, it may already be good enough to split the change up into two: first, a chancge to add !getdagop/!setdagop; second, a change to remove the old form. That provides at least one "moment in time" where both versions can be used, which can help with migrations.
If you leave a few weeks between committing the two changes, it's even nicer.
Cheers,
Nicolai
Yes, that is my plan with all the things I am deprecating.