[FIRRTL] Add containTypeAlias to type storage and define getAnonymousType
#5493
Conversation
uenoku
force-pushed
the
dev/uenoku/get-anonymous-type
branch
from
87598b5
to
448adb0
Compare
uenoku
changed the title
getAnonymousTypegetAnonymousType
This adds `anonymousType` to type storages and define `getAnonymousType`. Also add `containsTypeAlias` to recurisve type prop.
uenoku
force-pushed
the
dev/uenoku/get-anonymous-type
branch
from
448adb0
to
e67a8b7
Compare
| return impl->anonymousType; | ||
|
|
||
| // If this type is already anonymous, use it and remember for next time. | ||
| if (!impl->props.containsTypeAlias) { |
There was a problem hiding this comment.
Not sure if it makes a difference, but this could be the first check and just return *this without setting the anonymousType, maybe.
There was a problem hiding this comment.
Yeah, that makes sense and I agree. I implemented in a consistent way to getPassiveType implementation so I'd like to prioritize the consistency here. I think it's better to change the style at the same time. Thank you for the suggestion!
lib/Dialect/FIRRTL/FIRRTLTypes.cpp
Outdated
| return impl->anonymousType; | ||
|
|
||
| // If this type is already anonymous, return it and remember for next time. | ||
| if (impl->elementType.getRecursiveTypeProperties().containsTypeAlias) |
There was a problem hiding this comment.
The not is missing on the check ?
| if (impl->elementType.getRecursiveTypeProperties().containsTypeAlias) | |
| if (!impl->elementType.getRecursiveTypeProperties().containsTypeAlias) |
There was a problem hiding this comment.
Also same comment, maybe check this and early return, without setting the cached type.
There was a problem hiding this comment.
The not is missing on the check ?
Totally, great catch. Thanks!
lib/Dialect/FIRRTL/FIRRTLTypes.cpp
Outdated
| /// Return this type with any type aliases recursively removed from itself. | ||
| FIRRTLBaseType FEnumType::getAnonymousType() { | ||
| if (!this->containsTypeAlias()) | ||
| return *this; |
There was a problem hiding this comment.
Sorry, I changed to the other implementation for the consistency. Thank you for pointing out!
uenoku
force-pushed
the
dev/uenoku/get-anonymous-type
branch
from
c5fb0c5
to
ea63f98
Compare
…sType` (llvm#5493) This adds `anonymousType` to type storages of bundle/vector/enum/alias type and define `getAnonymousType` to FIRTRLBaseType. Also add `containsTypeAlias` to recursive type prop.
This adds
anonymousTypeto type storages and definegetAnonymousTypeto provide a convenient way to get structural information. Also addcontainsTypeAliasto recursive type prop.