Skip to content

/ servo

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

Jump to bottom

Report usages of &Vec<T> in function arguments #7640

Closed
jdm opened this issue Sep 15, 2015 · 7 comments
Closed

Report usages of &Vec<T> in function arguments #7640

jdm opened this issue Sep 15, 2015 · 7 comments
Labels
A-mach C-assigned E-easy L-python

Comments

@jdm
Copy link
Member

@jdm jdm commented Sep 15, 2015

There's no reason for a function to accept a &Vec<T> argument instead of &[T], since they allow the same operations and the first is an extra pointer dereference. We should check for the string : &Vec< in Rust source files and report any instances found.

Code: python/tidy.py
@jdramani
Copy link
Contributor

@jdramani jdramani commented Sep 15, 2015

I think I can do it.
@jdm
Copy link
Member Author

@jdm jdm commented Sep 15, 2015

Great!
@jdm jdm added the C-assigned label Sep 15, 2015
@jdramani jdramani mentioned this issue Sep 15, 2015
Merged
@wilfreddenton
Copy link

@wilfreddenton wilfreddenton commented Sep 20, 2015

I'm new to Rust so I just need some clarification. I thought that vectors allow for adding and removing elements whereas arrays do not. A function needing to manipulate a collection parameter's items would need the parameter to be a vector, no?
@jdm
Copy link
Member Author

@jdm jdm commented Sep 20, 2015

Those would need to be mutable, ie. &mut Vec. Immutable references provide no additional functionality over slices.
@wilfreddenton
Copy link

@wilfreddenton wilfreddenton commented Sep 20, 2015

Oh right. thanks for the help!
@sinistersnare
Copy link

@sinistersnare sinistersnare commented Sep 25, 2015

It seems clippy has this lint
jdramani added a commit to jdramani/servo that referenced this issue Sep 27, 2015
@jdramani
Check for Extra pointer dereferencing. Issue servo#7640.
2a99915
bors-servo pushed a commit that referenced this issue Sep 27, 2015
bors-servo
Auto merge of #7643 - jdramani:extra_ptr_dref, r=jdm
7413a95 
Check for Extra pointer dereferencing

Solves issue #7640

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7643)
<!-- Reviewable:end -->
bors-servo pushed a commit that referenced this issue Sep 27, 2015
bors-servo
Auto merge of #7643 - jdramani:extra_ptr_dref, r=jdm
Loading status checks…
9523283 
Check for Extra pointer dereferencing

Solves issue #7640

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7643)
<!-- Reviewable:end -->
josiahdaniels added a commit to josiahdaniels/servo that referenced this issue Sep 28, 2015
@jdramani @josiahdaniels
Check for Extra pointer dereferencing. Issue servo#7640.
7343087
@Manishearth
Copy link
Member

@Manishearth Manishearth commented Oct 13, 2015

#7643
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-mach C-assigned E-easy L-python
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
@jdm @wilfreddenton @Manishearth @sinistersnare @jdramani
You can’t perform that action at this time.