Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Refactor Binding and Prefix to use Rc for pointers #57

Open
wants to merge 80 commits into
base: master
Choose a base branch
from
Open

Refactor Binding and Prefix to use Rc for pointers #57

wants to merge 80 commits into from

Conversation

ahomescu
Copy link
Contributor

No description provided.

@ahomescu
Replace m_dtd type with Rc<RefCell<DTD>>
8d1a2e1
@ahomescu
Simplify some code in xmlparse.rs
cb96243
@ahomescu
Move a let statement lower in the function
9a5caeb
@ahomescu
Merge branch 'master' into feature/cow_dtd
f5f9bc0
@ahomescu
Update lifetimes
7c27067
@ahomescu
Replace pointer to DTD in XML_ParserStruct with a Cow<DTD>
aa8cb89
@ahomescu
Convert dtd{Create,Reset,Copy} into methods of DTD
f67515b
@ahomescu
Merge both lifetimes for DTD::try_clone into a single one
8953fc4
@ahomescu
Implement Default for prefix
02d35d4
@ahomescu
Implement panicking Clone for Scaffold
8246c5c
@ahomescu
Rename Scaffold to DTDScaffold
844a66e
@ahomescu
Store the parent parser in XML_ParserStruct as an Option-of-reference…
c750c17 
… instead of a raw pointer, and give it the same lifetime as the DTD and scaffold
@ahomescu
Simplify XML_ExternalEntityParserCreate
8fdf975
@ahomescu
Replace 'p lifetime with 'parser
998fe9b
@ahomescu
Implement our own fallible Rc<T> and use it instead of Cow
387f445
@ahomescu
Add assertion that old_rc isn't the maximum value before increment
548a22b
@ahomescu
Replace m_parentParser reference with a boolean is_child_parser flag …
77f43be 
…in XML_ParserStruct and get rid of the 'parser lifetime
@ahomescu
Merge branch 'master' into feature/cow_dtd
44e3ddc
@ahomescu
Un-implement Send and Sync for our Rc
52aff93
@ahomescu
Refactor Binding and Prefix to use Option<Rc<T>> for pointers
6d667a0 
Simplify Rc<T> by using the standard implementation for most operations,
and just implementing Rc::try_new
@ahomescu
Implement Drop for Binding
cb68bbd
@ahomescu
Remove a redundant Cell set to None
3d5f2d5
@ahomescu ahomescu added the enhancement New feature or request label Mar 25, 2020
@ahomescu ahomescu self-assigned this Mar 25, 2020
@ahomescu
Pass Rc<Prefix> to addBinding by reference
6673af6
@ahomescu
Simplify some control flow in setContext()
86f134f
@ahomescu
Reorder two locals for consistency
d51e92a
@ahomescu
Remove unnecessary reference
d348d5c
@ahomescu
Pass Rc arguments by reference to defineAttribute()
82ea3a4
@ahomescu
Replace Rc argument with reference to inner value in setElementTypePr…
e509c66 
…efix()
@ahomescu
Use as_deref() to access self.m_declEntity's inner entity directly
dd40dc3
@ahomescu
Replace one call to unwrap() with if let Some(...)
a00ea91
@ahomescu
Raise the number of allowed allocations to make the test_misc_alloc_c…
ceb3aba 
…reate_parser test pass
@ahomescu
Restore the reallocation count checks in context_realloc_test and tes…
a85fd18 
…t_nsalloc_realloc_long_ge_name
@ahomescu
Merge branch 'feature/cow_dtd' into feature/rc_bindings
0849dc4
@ahomescu
Remove derives on Binding left over from previous merge
d979d55
@ahomescu
Remove a couple of calls to Rc::clone
1014300
@ahomescu
Remove unnecessary re-borrows of self.m_encoding
1cf7d12
@ahomescu
Replace some as_ref() calls with as_deref()
56c0ac1
@ahomescu
Remove DTDPools and the surrounding RefCell since most StringPool met…
de0eebe 
…hods take an immutable reference
@ahomescu
Merge branch 'feature/cow_dtd' into feature/rc_bindings
703f0c3
@ahomescu
Convert Rc::get_mut() to an associated function
cba0d53
@ahomescu
Merge branch 'feature/cow_dtd' into feature/rc_bindings
ea353fc
@ahomescu
Backport the simplified Rc implementation from the feature/rc_binding…
cb0e7a8 
…s branch
@ahomescu
Merge branch 'feature/cow_dtd' into feature/rc_bindings
f3abb4a
@ahomescu
Add comment on DTD tables field
bee70a9
@ahomescu ahomescu changed the base branch from feature/cow_dtd to master May 1, 2020 00:07
TheDan64
TheDan64 reviewed May 11, 2020
View reviewed changes
Copy link
Contributor

@TheDan64 TheDan64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall, just have a few thoughts

src/lib/xmlparse.rs
use std::rc::{Rc, Weak};
use std::slice;

type Ptr<T> = Option<Rc<T>>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I hadn't seen this alias, it wouldn't have been obvious what Ptr<T> is. OptRc<T> would be more obvious, but at that point it's not far off from Option<Rc<T>>. Is the full type that verbose that it warrants an alias?

src/lib/xmlparse.rs Outdated Show resolved Hide resolved
src/lib/xmlparse.rs
pub notation: Cell<*const XML_Char>,
pub open: Cell<XML_Bool>,
pub is_param: Cell<XML_Bool>,
pub is_internal: Cell<XML_Bool>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that this simplifies a lot, but I keep wondering if it's becoming an anti-pattern if we have to use it absolutely everywhere

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TheDan64 TheDan64 TheDan64 left review comments

@rinon rinon Awaiting requested review from rinon

@thedataking thedataking Awaiting requested review from thedataking

Assignees

@ahomescu ahomescu

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ahomescu @TheDan64