Should we recommend explicit constructors as part of WebKit style?
This came up before: https://lists.webkit.org/pipermail/webkit-dev/2010-May/012873.html but I'd like to understand it a bit better. It feels there were two points of view: 1. Use explicit only when necessary to prevent an undesirable implicit conversion like int to vector. 2. Use explicit except when it is desirable to allow an implicit conversion that makes the code simpler. For example, the String <-> AtomicString makes the bindings generator code simpler since it doesn't need to know which the underlying method takes. Are there any reasons beyond personal preference to select either of these? Starting list: Pro's for #1 It is a pain to remember to put explicit every time you have a constructor with one argument. Pro's for #2 It would prevent accidental mistakes that happen with implicit constructors. dave
On Tue, Sep 28, 2010 at 4:26 PM, David Levin <levin@google.com> wrote:
This came up before: https://lists.webkit.org/pipermail/webkit-dev/2010-May/012873.html but I'd like to understand it a bit better.
It feels there were two points of view:
1. Use explicit only when necessary to prevent an undesirable implicit conversion like int to vector. 2. Use explicit except when it is desirable to allow an implicit conversion that makes the code simpler. For example, the String <-> AtomicString makes the bindings generator code simpler since it doesn't need to know which the underlying method takes.
Are there any reasons beyond personal preference to select either of these?
Starting list:
Pro's for #1 It is a pain to remember to put explicit every time you have a constructor with one argument.
Pro's for #2 It would prevent accidental mistakes that happen with implicit constructors.
We're there some recent mishaps with implicit constructors that motivate this thread? -Darin
On Tue, Sep 28, 2010 at 4:30 PM, Oliver Hunt <oliver@apple.com> wrote:
Pro's for #1 It is a pain to remember to put explicit every time you have a constructor with one argument.
Could check-webkit-style be beaten into forcing this for us?
Yep, check-webkit-style could check this automatically, but it wouldn't be subtle, so it would flag cases where we want an implicit constructor. It would just have to point to the guideline. (The check is currently turned off because this isn't WebKit style.) On Tue, Sep 28, 2010 at 4:29 PM, Darin Fisher <darin@chromium.org> wrote:
We're there some recent mishaps with implicit constructors that motivate this thread?
I've noticed Adam Barth putting comments in reviews about it (and check-webkit-style could do it automatically for him). I tend to think it is a good thing (and have been bitten in the past by not doing it), but I don't usually notice it in reviews. dave
On Tue, Sep 28, 2010 at 4:45 PM, David Levin <levin@google.com> wrote:
On Tue, Sep 28, 2010 at 4:29 PM, Darin Fisher <darin@chromium.org> wrote:
We're there some recent mishaps with implicit constructors that motivate this thread?
I've noticed Adam Barth putting comments in reviews about it (and check-webkit-style could do it automatically for him). I tend to think it is a good thing (and have been bitten in the past by not doing it), but I don't usually notice it in reviews.
I've been recently sensitized to this issue because we had a non-explicit one-argument constructor in the HTML5 parser that was eating performance (due to ref churn). Luckily it was bad enough that it showed up on profiles. If we were less diligent about understanding where all the perf was going, we could have easily missed it. Adam
On Sep 28, 2010, at 4:26 PM, David Levin wrote:
This came up before: https://lists.webkit.org/pipermail/webkit-dev/2010-May/012873.html but I'd like to understand it a bit better.
It feels there were two points of view: Use explicit only when necessary to prevent an undesirable implicit conversion like int to vector. Use explicit except when it is desirable to allow an implicit conversion that makes the code simpler. For example, the String <-> AtomicString makes the bindings generator code simpler since it doesn't need to know which the underlying method takes. Are there any reasons beyond personal preference to select either of these?
Starting list:
Pro's for #1 It is a pain to remember to put explicit every time you have a constructor with one argument.
Could check-webkit-style be beaten into forcing this for us?
Pro's for #2 It would prevent accidental mistakes that happen with implicit constructors.
dave
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
On Sep 28, 2010, at 4:26 PM, David Levin wrote:
This came up before: https://lists.webkit.org/pipermail/webkit-dev/2010-May/012873.html but I'd like to understand it a bit better.
It feels there were two points of view: Use explicit only when necessary to prevent an undesirable implicit conversion like int to vector. Use explicit except when it is desirable to allow an implicit conversion that makes the code simpler. For example, the String <-> AtomicString makes the bindings generator code simpler since it doesn't need to know which the underlying method takes. Are there any reasons beyond personal preference to select either of these?
Starting list:
Pro's for #1 It is a pain to remember to put explicit every time you have a constructor with one argument.
Pro's for #2 It would prevent accidental mistakes that happen with implicit constructors.
I think the rule should be something like: 3. Do not explicit when the single-argument constructor can be thought of as a type conversion - the class will be in some sense an alternate form of its sole parameter. Do use explicit when the single-argument constructor is *not* reasonably thought of as a type conversion - the single argument just happens to be the sole initialization parameter. Or to put it another way - can you imagine having a type conversion operator overload that does the same thing as this constroctor? Applying this rule to your two examples, Vector(int) should be explicit, because it doesn't convert the int to a vector, it uses the int as a size instead of the default one. But String(AtomicString) or AtomicString(String) need not be explicit, since they convert from one string type to another, carrying largely the same data. I realize this rule requires some judgment, so it's worse than a purely mechanical rule, but I think it accurately captures the proper use of explicit. Regards, Maciej
On Sep 28, 2010, at 4:31 PM, Maciej Stachowiak wrote:
I think the rule should be something like:
3. Do not explicit when the single-argument constructor can be thought of as a type conversion - the class will be in some sense an alternate form of its sole parameter. Do use explicit when the single-argument constructor is *not* reasonably thought of as a type conversion - the single argument just happens to be the sole initialization parameter. Or to put it another way - can you imagine having a type conversion operator overload that does the same thing as this constroctor?
Applying this rule to your two examples, Vector(int) should be explicit, because it doesn't convert the int to a vector, it uses the int as a size instead of the default one. But String(AtomicString) or AtomicString(String) need not be explicit, since they convert from one string type to another, carrying largely the same data.
I realize this rule requires some judgment, so it's worse than a purely mechanical rule, but I think it accurately captures the proper use of explicit.
This seems like a good rule. I agree completely that it’s the right principle. When the conversion between types is costly enough, there may be some cases where we don’t want to offer an implicit constructor. Specifically, the implicit conversion from String to AtomicString may be a mistake; we might be better off if we had to utter some explicit function name every time we wanted a string looked up in the atomic string table. I don’t think of this, though, as a rule for when to use “explicit”. It’s a rule for when to use a constructor that can be used implicitly for type conversion. If a constructor can’t be used that way, an explicit constructor might be OK, or you might not want the other constructor to exist at all. For example, if we want to be more explicit about the cost of looking up an AtomicString, the syntax for making an AtomicString from a String should probably be a named function, not AtomicString(string). -- Darin
It sounds like there is consensus. Here's a patch for adding this to the style guide: https://bugs.webkit.org/show_bug.cgi?id=47646 dave On Wed, Sep 29, 2010 at 9:41 AM, Darin Adler <darin@apple.com> wrote:
On Sep 28, 2010, at 4:31 PM, Maciej Stachowiak wrote:
I think the rule should be something like:
3. Do not explicit when the single-argument constructor can be thought of as a type conversion - the class will be in some sense an alternate form of its sole parameter. Do use explicit when the single-argument constructor is *not* reasonably thought of as a type conversion - the single argument just happens to be the sole initialization parameter. Or to put it another way - can you imagine having a type conversion operator overload that does the same thing as this constroctor?
Applying this rule to your two examples, Vector(int) should be explicit, because it doesn't convert the int to a vector, it uses the int as a size instead of the default one. But String(AtomicString) or AtomicString(String) need not be explicit, since they convert from one string type to another, carrying largely the same data.
I realize this rule requires some judgment, so it's worse than a purely mechanical rule, but I think it accurately captures the proper use of explicit.
This seems like a good rule. I agree completely that it’s the right principle.
When the conversion between types is costly enough, there may be some cases where we don’t want to offer an implicit constructor. Specifically, the implicit conversion from String to AtomicString may be a mistake; we might be better off if we had to utter some explicit function name every time we wanted a string looked up in the atomic string table.
I don’t think of this, though, as a rule for when to use “explicit”. It’s a rule for when to use a constructor that can be used implicitly for type conversion. If a constructor can’t be used that way, an explicit constructor might be OK, or you might not want the other constructor to exist at all. For example, if we want to be more explicit about the cost of looking up an AtomicString, the syntax for making an AtomicString from a String should probably be a named function, not AtomicString(string).
-- Darin
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
participants (6)
-
Adam Barth
-
Darin Adler
-
Darin Fisher
-
David Levin
-
Maciej Stachowiak
-
Oliver Hunt