Improve printing method name in VectorAPIExpansion #22348
Conversation
| TR::ResolvedMethodSymbol *resolvedMethodSymbol = (TR::ResolvedMethodSymbol *)methodSymbol; | ||
|
|
||
| const char *name = resolvedMethodSymbol->getResolvedMethod() ? resolvedMethodSymbol->getResolvedMethod()->nameChars() | ||
| : "unresolved"; |
There was a problem hiding this comment.
How do we know that the cast (TR::ResolvedMethodSymbol *)methodSymbol is safe? This code is explicitly trying to guard against the possibility of an unresolved method.
I looked at the calling context where this code was moved from, and I couldn't find an obvious reason why the symbol must necessarily be a ResolvedMethodSymbol.
It does have to satisfy isVectorAPIMethod() (at least at that location), which requires one of a relatively small set of recognized methods, and the recognized method will only be known for TR_ResolvedJ9Method, which I'm sure would generally be wrapped in ResolvedMethodSymbol, but I'm not sure there's a guarantee.
What do you think of using methodSymbol->getResolvedMethodSymbol()? That will check and return null if it's not the right type. Then we could return "unresolved" if there is a null result from either getResolvedMethodSymbol() or getResolvedMethod()
There was a problem hiding this comment.
getMethodName()is only called for methods for which isVectorAPIMethod() is true, essentially only to report the name of some VectorAPI intrinsic. Maybe I should add an ASSERT and/or rename the method into getIntrinsicName() ?
There was a problem hiding this comment.
It does have to satisfy
isVectorAPIMethod()(at least at that location), which requires one of a relatively small set of recognized methods, and the recognized method will only be known forTR_ResolvedJ9Method, which I'm sure would generally be wrapped inResolvedMethodSymbol, but I'm not sure there's a guarantee.
There was a problem hiding this comment.
Sure, I will add the check.
No description provided.