-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[Type] [refactor] Make TypeFactory Global #1963
[Type] [refactor] Make TypeFactory Global #1963
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1963 +/- ##
=======================================
Coverage 43.64% 43.64%
=======================================
Files 45 45
Lines 6225 6225
Branches 1106 1106
=======================================
Hits 2717 2717
Misses 3334 3334
Partials 174 174 Continue to review full report at Codecov.
|
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.
Cool! LGTM in general. Just one concern to address and it can be merged. Thanks!
taichi/ir/type_factory.cpp
Outdated
TypeFactory &TypeFactory::get_instance() { | ||
static TypeFactory type_factory; | ||
return type_factory; | ||
} |
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.
We probably want to stick to the old implementation in Program::get_type_factory
since if type_factory
get destroyed before other global variables that use it, the program won't finish smoothly.
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.
Btw, since #1962 is in, feel free to remove Program::get_type_factory()
in a different PR :-) Thanks!
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.
We probably want to stick to the old implementation in
Program::get_type_factory
since iftype_factory
get destroyed before other global variables that use it, the program won't finish smoothly.
Yes, you are right. let me change it to the old implementation. Thanks for pointing it out!
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, thanks!
Related issue = #1905
In this pr, we replace
Program::get_type_factory()
withTypeFactory::get_instance()
. We currently reserve the former function which may be removed in the future.[Click here for the format server]