Update to 0.8 #17
Update to 0.8 #17
Conversation
| impl PartialEq for InternalStrRef { | ||
| fn eq(&self, other: &InternalStrRef) -> bool { | ||
| impl PartialEq for PinnedStr { | ||
| fn eq(&self, other: &Self) -> bool { | ||
| self.as_str() == other.as_str() |
There was a problem hiding this comment.
Why do you need to dereference the pointer here? I thought string-interner guarentees that two strings will always return the same Symbol, so shouldn't they have the same pointer as well?
There was a problem hiding this comment.
Good question! You can generally treat same symbols as being the same string. However, for the string interners internal hash map you need to compare incoming strings using their contents since they will probably not point to strings stored in the string interner and thus will have different pointers to potentially equal string contents.
Note that PinnedStr != Symbol.
PinnedStr is a pointer or identifier for internal access while Symbol is an identifier for external access.
There was a problem hiding this comment.
Ok, so a PinnedStr may not yet have been deduplicated? That makes sense.
|
Any progress on this PR? It would be great to no longer have undefined behavior due to the self-referencing structs ... |
From my understanding there is no UB in the current implementation of |
Codecov Report
@@ Coverage Diff @@
## master #17 +/- ##
==========================================
+ Coverage 94.44% 96.13% +1.69%
==========================================
Files 2 4 +2
Lines 414 414
==========================================
+ Hits 391 398 +7
+ Misses 23 16 -7
Continue to review full report at Codecov.
|
We still to coverage reporting via Travis CI because we haven't yet explored how to do this via GitHub Actions.
This mainly utilizes
Pinfor theStringInternerself-referencing and rebuilds performance tests.