Replace gt## with As##

[franksinankaya]

@am11 @jkotas @janvorli @sandreenko

[mikedn]

gtOp. should be replaced with AsOp()->.

[franksinankaya]

Sure, let's see the consensus. The only agreement I heart so far was GetXYZ/SetXYZ and IsXYZ/SetXYZ function combinations.

[mikedn]

That was for actual getter/setter functions. This gtOp thing and others like it are basically casts - they cast the node pointer to another GenTree class, while asserting that the node has the proper operator. Hence the "as" name, which is used in many other places already.

[franksinankaya]

I see how about converting gtOp. to gtOp()./AsOp(). with a stub function as in this review?

I want to make the diff as close as possible for easy review.
I can go both ways. It doesn't matter for me.

[mikedn]

I'm not sure I understand what the stub is for. AsOp() already exists so it's simply a matter of replacing gtOp. with AsOp()->. Then, in another PR, you can do another gtX -> AsX replacement and once it's all done you can delete the __declspec.

[sandreenko]

Thanks for that change, LGTM, ignore the notes, I will fix them in another PR.

@dotnet/jit-contrib please pay attention that this PR deletes tree.gtOp property, use AsOp() instead from now.

It is very appealing to replace all AsOp()->gtOp1 with gtGetOp1(), but as I remember @mikedn wants to get rid of gtGetOp1(), am I correct?

[mikedn]

It is very appealing to replace all AsOp()->gtOp1 with gtGetOp1(), but as I remember @mikedn wants to get rid of gtGetOp1(), am I correct?

What I'd really like is to move gtGetOp1() and gtGetOp2() to GenTreeUnOp and GenTreeOp respectively.

I'm not sure if we'll ever get there though (it would require a very large PR) but I would recommend casting to the appropriate class before calling these 2 functions. Since you're going to call them you should already know what class you're dealing with and it makes the code more readable (though perhaps slightly longer, thanks to the unfortunate extensive use of GenTree* instead of the actual type) if that is made explicit.

This is even more important when the actual node class has getter/setters with alternative names, like GenTreeStoreInd has Addr() for gtOp1 and Data() for gtOp2. Using generic op1 and op2 names in such case makes the code even harder to follow and increases the chances of accidentally mixing up the 2 operands.

Given the above, I don't think that any kind of mechanical replacement around gtOp1, gtGetOp1 & co. is feasible. It will have to be done on a case by case basis.

[franksinankaya]

@sandreenko : I'm assuming that you are taking care of the rest per your notes above. Let me know otherwise.

[sandreenko]

@sandreenko : I'm assuming that you are taking care of the rest per your notes above. Let me know otherwise.

Yes, thanks for the last commit. I was planning to do it myself as well, was just waiting for clean testing to merge that before this PR gets merge conflicts.

[sandreenko]

Link https://github.com/dotnet/coreclr/issues/27155, merging on red, the failures are not related to the change.

[franksinankaya]

Just a note here that converting gtXYZ. to AsXYZ() is going to cost 36 more reviews.