Clone + Copy for Lexer and SpannedIter?
#263
Comments
|
Hello! I currently work around the limitations of ´SpannedIter´ by allowing access to the inner lexer #231, feel free to use my branch. Unfortunately it doesn't look like this lovely lexer will get any new changes anytime soon. |
Jakobeha
added a commit
to Jakobeha/logos
that referenced
this issue
May 2, 2024
`Lexer::clone` shouldn't clone the inner `ManuallyDrop`, because doing so clones the inner value, which is moved out in `Lexer::next`. This causes use-after-free if the lexer is cloned after the last-returned token is dropped, especially if the token contains an overridden implementation of `Clone` (such as `Rc`) that tries to read the dropped data. It causes a memory leak if the token contains a heap-allocated value, because cloning makes a new allocation. This allocation is in the `ManuallyDrop` and it's guaranteed to be overridden before the call to `ManuallyDrop::take`, so it's never freed. Idk if also `ManuallyDrop` needs to be replaced with `MaybeUninit`, that's another solution but I don't think it's necessary. Another thing I didn't add is maciejhirsz#263 (make `Lexer` `Copy`), although it probably should be added (referencing here because it looks like the issue has been forgotten).
Jakobeha
added a commit
to Jakobeha/logos
that referenced
this issue
May 2, 2024
`Lexer::clone` shouldn't clone the inner `ManuallyDrop`, because doing so clones the inner value, which is moved out in `Lexer::next`. This causes use-after-free if the lexer is cloned after the last-returned token is dropped, especially if the token contains an overridden implementation of `Clone` (such as `Rc`) that tries to read the dropped data. It causes a memory leak if the token contains a heap-allocated value, because cloning makes a new allocation. This allocation is in the `ManuallyDrop` and it's guaranteed to be overridden before the call to `ManuallyDrop::take`, so it's never freed. Idk if also `ManuallyDrop` needs to be replaced with `MaybeUninit`, that's another solution but I don't think it's necessary. Another thing I didn't add is maciejhirsz#263 (make `Lexer` `Copy`), although it probably should be added (referencing here because it looks like the issue has been forgotten).
Jakobeha
added a commit
to Jakobeha/logos
that referenced
this issue
May 2, 2024
`Lexer::clone` shouldn't clone the inner `ManuallyDrop`, because doing so clones the inner value, which is moved out in `Lexer::next`. This causes use-after-free if the lexer is cloned after the last-returned token is dropped, especially if the token contains an overridden implementation of `Clone` (such as `Rc`) that tries to read the dropped data. It causes a memory leak if the token contains a heap-allocated value, because cloning makes a new allocation. This allocation is in the `ManuallyDrop` and it's guaranteed to be overridden before the call to `ManuallyDrop::take`, so it's never freed. Another thing: maciejhirsz#263 (make `Lexer` implement `Copy`) probably should be added (referencing here because it looks like the issue has been forgotten).
Jakobeha
added a commit
to Jakobeha/logos
that referenced
this issue
May 3, 2024
`Lexer::clone` shouldn't clone the inner `ManuallyDrop`, because doing so clones the inner value, which is moved out in `Lexer::next`. This causes use-after-free if the lexer is cloned after the last-returned token is dropped, especially if the token contains an overridden implementation of `Clone` (such as `Rc`) that tries to read the dropped data. It causes a memory leak if the token contains a heap-allocated value, because cloning makes a new allocation. This allocation is in the `ManuallyDrop` and it's guaranteed to be overridden before the call to `ManuallyDrop::take`, so it's never freed. Another thing: maciejhirsz#263 (make `Lexer` implement `Copy`) probably should be added (referencing here because it looks like the issue has been forgotten).
jeertmans
pushed a commit
that referenced
this issue
May 3, 2024
`Lexer::clone` shouldn't clone the inner `ManuallyDrop`, because doing so clones the inner value, which is moved out in `Lexer::next`. This causes use-after-free if the lexer is cloned after the last-returned token is dropped, especially if the token contains an overridden implementation of `Clone` (such as `Rc`) that tries to read the dropped data. It causes a memory leak if the token contains a heap-allocated value, because cloning makes a new allocation. This allocation is in the `ManuallyDrop` and it's guaranteed to be overridden before the call to `ManuallyDrop::take`, so it's never freed. Another thing: #263 (make `Lexer` implement `Copy`) probably should be added (referencing here because it looks like the issue has been forgotten).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I have two questions regarding
CloneandCopyforLexerandSpannedIter:CopytoLexerif both the token and the extras areCopy?Clone + CopytoSpannedIter?I'm trying to use logos in conjunction with nom and this would help with branching. Specifically,
CloneonSpannedIterwould make it possible to use at all, andCopywould make my code look a bit cleaner.You've probably already thought about this so apologies if the answer is obvious but I can't figure out the reason behind it myself. 😊 Or maybe I'm just the first person that would benefit from that and then I could write a PR for this.
The text was updated successfully, but these errors were encountered: