[ir] Make sure "StmtFieldManager" to be correct if we modify some fields after the ctor #1587
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1587 +/- ##
==========================================
- Coverage 86.81% 86.76% -0.05%
==========================================
Files 19 19
Lines 3685 3679 -6
Branches 653 653
==========================================
- Hits 3199 3192 -7
- Misses 353 354 +1
Partials 133 133
Continue to review full report at Codecov.
|
| } | ||
|
|
||
| bool equal(const StmtField *other_generic) const override { | ||
| if (auto other = dynamic_cast<const StmtFieldNumeric *>(other_generic)) { | ||
| if (other->value_ptr && value_ptr) { | ||
| return *(other->value_ptr) == *value_ptr; | ||
| } |
There was a problem hiding this comment.
What if only one of them is a pointer?
| } | |
| } else if (other->value_ptr || value_ptr) { | |
| return false; | |
| } |
taichi/ir/ir.h
Outdated
| T *value_ptr; | ||
| T value; |
There was a problem hiding this comment.
nit: Consider using std::variant?
There was a problem hiding this comment.
Wow, that's something I haven't think of, I'll definitely try that
There was a problem hiding this comment.
Thank you so much! Looks great.
If it makes sense, I would let the pointer-nonpointer comparison case return a loud error, since I don't think a normal Taichi program should run into that case. @xumingkuan WDYT?
I also prefer to report error on this. The way taichi construct StmtField would already ensure this case almost never happens. Updates: |
Co-authored-by: Yuanming Hu <yuanming-hu@users.noreply.github.com>
Sure, it makes sense to me. |
Related issue = #1286
[Click here for the format server]