New issue
Advanced search Search tips

Issue 905916 link

Starred by 1 user
Status: Available
Owner: ----
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

|visitor| should be a reference; GarbageCollectedMixin::Trace(Visitor* visitor)

Project Member Reported by hayato@chromium.org, Nov 16 
Is there any possibility that |visitor| is nullptr there?

If |visitor| is always non-null, it should be |Visitor& visitor|, as per Blink Style Guide.

https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/blink-c++.md#Use-references-for-all-non_null-pointer-arguments

Comment 1 by mlippautz@chromium.org, Jan 16 (6 days ago)

Labels: -Pri-2 Pri-3
Status: Available (was: Untriaged)
The |visitor| parameter can never be null, correct.

More and more code is being shared and interacts with other code bases (e.g. Chrome and V8) where mutable objects are declared using pointers.

Moreover, if we really wanted to do this, then we should do it across all Trace() methods to be consistent. This is a larger refactoring that affects all GCed classes though which is currently not a high priority.

(The method should probably also be const, as visitation methods should never alter state of the object.)
►

Sign in to add a comment