Changing the name of OMR::AutomaticSymbol::getKind() to getOpCodeKind() #3241
Conversation
|
@genie-omr build all |
There was a problem hiding this comment.
Changes LGTM (though it would be nice to squash the commits). @vijaysun-omr should review this.
One question, would it make more sense to use the name getOpCode() instead, since that is what is being returned, not a "kind"?
|
Thanks for the contribution @samasri ! Just another minor thing: your commits are not following the commit format specified in Contributing.md. In the first commit, for example, you are going over 70 characters in the first line and into the body of the commit message. The second commit is meant to amend what you missed in the first commit, and could perhaps be squashed. |
samasri
force-pushed
the
Removing-Ambiguous-getKind
branch
4 times, most recently
from
c9062ce to
9f6835a
Compare
* OMR::Symbol::getKind() returns an int32_t * OMR::AutomaticSymbol::getKind() returns an TR::ILOpCodes object * OMR::AutomaticSymbol indirectly inherits from OMR::Symbol A derived class cannot virtually override a method that returns a different type than the parent class method getKind() is renamed to remove ambiguity and allow the virtualization of that function Signed-off-by: Samer AL Masri <[email protected]>
samasri
force-pushed
the
Removing-Ambiguous-getKind
branch
2 times, most recently
from
6a528c1 to
fceb94b
Compare
|
More commits seem to have been pushed to this branch that are unrelated to this PR. Was this intentional? |
samasri
force-pushed
the
Removing-Ambiguous-getKind
branch
from
6a528c1 to
fceb94b
Compare
|
Sorry, that was by mistake; I fixed it now. |
|
@genie-omr build all |
Changed the name of
getKindinOMR::AutomaticSymbolto have less ambiguity in function names.getKind()function is being defined in two classes of the same hierarchy, but with different return types:OMR::Symbol::getKind()returns anint32_tOMR::AutomaticSymbol::getKind()returns anTR::ILOpCodesobjectOMR::AutomaticSymbolindirectly inherits fromOMR::SymbolAs part of our work to virtualize the class hierarchies of Eclipse OMR (see motivation of #3235 for more info), we stumbled upon this case. Virtualizing
getKind()here is not possible due to the different return type. In order to move on with our process and help OMR have a clearer design (less ambiguity in function names), we suggest changing thegetKind()function inOMR::AutomaticSymboltogetOpCodeKind(), which describes the functionality of this method more clearly.Signed-off-by: Samer AL Masri [email protected]