-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[PIR] Unify dyn_cast interface with pir::cast #57463
[PIR] Unify dyn_cast interface with pir::cast #57463
Conversation
…n_cast_interface
你的PR提交成功,感谢你对开源项目的贡献! |
@@ -37,6 +37,14 @@ class InferMetaInterface : public pir::OpInterfaceBase<InferMetaInterface> { | |||
}; | |||
|
|||
/// Constructor | |||
InferMetaInterface(std::nullptr_t) // NOLINT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这两个构造函数的增加感觉不是必要的,而且会给IR库的扩展着造成额外的负担。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK,这里是想把 interface 抽出来,做一个 interfaceBase 的,在没有做 interfaceBase 前先和 op 的保持一致
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interface新增的两个构造函数不是必须的。会给Interface的开发者造成额外的负担,不建议增加。
… dyn_cast_interface
82bae84
to
afaac44
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* PR comment * unify dyn_cast_interface * rm detail
* PR comment * unify dyn_cast_interface * rm detail
* PR comment * unify dyn_cast_interface * rm detail
PR types
Others
PR changes
Others
Description
Other
Pcard-67164