It doesn't make much sense to return const UniquePtr<Foo>& for something that
can't be null, it's just confusing.
Also make more stuff actually const.
Details
Details
- Reviewers
TYLin bradwerth - Group Reviewers
Restricted Project - Commits
- Restricted Diffusion Commit
rMOZILLACENTRALd34c32dab9c9: Bug 1504078 - Use references in the shapes code. r=bradwerth,TYLin - Bugzilla Bug ID
- 1504078
Diff Detail
Diff Detail
- Repository
- rMOZILLACENTRAL mozilla-central
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Comment Actions
Good fixes, thanks!
| layout/style/nsStyleStruct.h | ||
|---|---|---|
| 348 | Should we really remove the parameter name here? Seems inconsistent with other methods. | |
| 1920–1921 | Likewise here -- no parameter name. | |
| layout/svg/nsCSSClipPathInstance.cpp | ||
| 112–113 | Here and in other calls to BasicShape(), it should work to have "const auto&", which might be clearer. | |
Comment Actions
Nice! r=me with Brad's comment addressed.
| layout/svg/nsCSSClipPathInstance.cpp | ||
|---|---|---|
| 175–176 | Nit: maybe take the chance to reformat this as const FillRule fillRule = basicShape.GetFillRule() == StyleFillRule::Nonzero
? FillRule::FILL_WINDING : FillRule::FILL_EVEN_ODD;or const FillRule fillRule = basicShape.GetFillRule() == StyleFillRule::Nonzero
? FillRule::FILL_WINDING
: FillRule::FILL_EVEN_ODD; | |
| 232–233 | Nit: reindent this line. | |
| layout/style/nsStyleStruct.h | ||
|---|---|---|
| 348 | Fair enough, will put it back, though in general I think having redundant parameter names is not useful. | |