style guide update for an accessor returning a pointer to non const object. It should not be const

[oshima]

Hi,

I've been using (and also asked) the rule of "an accessor returning a pointer to non const object

should not be const", and I believe this is now "required" rather than "recommended" at least

in ui related code. However, this looks like not clearly documented in the style guide and somewhat

contradicts with the statement "Accessors should almost always be const"

in the google c++ style guide.

Having it the chromium style guide makes the review process easy, so

I wonder if we can add this to the style guide.  Does it worth making

already-too-long-chromium-style-guide longer? Any objection and/or thoughts?

Thanks,

- oshima

[Peter Kasting]

I'm OK with adding this to the style guide, although we may want to add rationale when we do so, as it's not immediately obvious to everyone why this can be a bad pattern.

PK

[Steven Bennetts]

+1 to putting it in the style guide with a brief but clear explanation.

(I'm generally on the side of keeping the Chromium style guide as short/simple as possible, but this is a common enough exception to what the Google guide suggests that I think it is worth adding).

On Tue, Jun 5, 2012 at 1:11 PM, Peter Kasting <pkas...@chromium.org> wrote:

I'm OK with adding this to the style guide, although we may want to add rationale when we do so, as it's not immediately obvious to everyone why this can be a bad pattern.

PK

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Paweł Hajdan, Jr.]

If possible, this should also be made part of our clang style checker. Automatic tools always are better at finding those issues. :)

You can just file a bug for now, so we remember this (and post the link here ideally). Thanks!

--

[Gregg Tavares (社用)]

I was hoping someone was going to post an enlightening example here before I make a fool of myself but, ... let me be the noob here and ask why this is bad?

class HTMLElement {

 public:

   HTMLElement* parent() const {...};

   Document* owner() const {..};

};

Why would you want to force me to have to have a non-const reference/pointer to HTMLElement just so I can access and possibly manipulate its parent or document?

I've always looked at the point of const is to keep me safe from myself. The more places I'm forced to not use const the more likely I'm going to accidentally do something I shouldn't.

[oshima]

It is my fault if it sounded like you shouldn't use const accessor. It's actually opposite and should read as "a const accessor should return const pointer".

Here is an example that illustrates the issue this style will address:

void doSomething(const HTMLElement& dont_modify_me) {

   HTMLElement* parent = dont_modify_me.parent();

   for (HTMLElement* child : parent->children()) {

      child->removeAllChild();

  }

}

Of cause, this is not perfect and there are other back-doors which allow you to modify const object,

but this style can prevent such mistake.

- oshima

 

[oshima]

I received a concern about making this strict rule because there are some cases that returned pointer is not considered

a part of the state of the object, so I'll make it just a strong recommendation (like const accessor in google style guide) with

clear explanation.

On Tue, Jun 5, 2012 at 11:33 PM, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:

If possible, this should also be made part of our clang style checker. Automatic tools always are better at finding those issues. :)

You can just file a bug for now, so we remember this (and post the link here ideally). Thanks!

crbug.com/131432. Note that we need some way to exclude particular code because there will be some exception.

I assume it's possible?

- oshima

[Rachel Blum]

Why are we not just saying that you must return a const pointer if the object pointed to is part of the state? That would definitely address Gregg's objection, for example. (Also: Any kind of factory object)

[Peter Kasting]

On Wed, Jun 6, 2012 at 1:27 PM, oshima <osh...@chromium.org> wrote:

I received a concern about making this strict rule because there are some cases that returned pointer is not considered

a part of the state of the object, so I'll make it just a strong recommendation (like const accessor in google style guide) with

clear explanation.

This confusion is actually part of why we have this rule.  The problem is that in general, _all_ returned pointers are "part of the state" because the issue is not whether they're members of the const object, but whether there is anything in the transitive closure of everything accessible from the returned pointer that allows you to affect something that the const object sees.  The answer to this question is practically always "yes" (indeed, if not, why would you even have an accessor for this pointer?) and thus this non-const returned pointer can basically always be used to violate logical constness.

To put this another way, consider oshima's example code, but imagine that the implementation of HTMLElement does not in fact include a parent HTMLElement*, but simply has some pointer to the top-level document.  parent() is then implemented by walking the tree to find the parent.  In this case, the returned parent pointer is not part of the child's state, so one might assume that the access pattern Gregg describes is OK.  But the usage in oshima's example shows that logical constness is still violated once the pointer is returned.  Generalize this, and you will understand why after some discussion we eventually concluded that returning a non-const pointer from a const accessor is basically never OK.

(FWIW, WebKit's existing code violates logical constness a lot; I started a thread about this once, and got an OK to at least make these sorts of accessors non-const, which I checked in a patch or two for, but there are still many problems and fixing them all would take a lot of time and effort.)

PK

[Antoine Labour]

On Wed, Jun 6, 2012 at 2:36 PM, Peter Kasting <pkas...@chromium.org> wrote:

On Wed, Jun 6, 2012 at 1:27 PM, oshima <osh...@chromium.org> wrote:

I received a concern about making this strict rule because there are some cases that returned pointer is not considered

a part of the state of the object, so I'll make it just a strong recommendation (like const accessor in google style guide) with

clear explanation.

This confusion is actually part of why we have this rule.  The problem is that in general, _all_ returned pointers are "part of the state" because the issue is not whether they're members of the const object, but whether there is anything in the transitive closure of everything accessible from the returned pointer that allows you to affect something that the const object sees.  The answer to this question is practically always "yes" (indeed, if not, why would you even have an accessor for this pointer?) and thus this non-const returned pointer can basically always be used to violate logical constness.

To put this another way, consider oshima's example code, but imagine that the implementation of HTMLElement does not in fact include a parent HTMLElement*, but simply has some pointer to the top-level document.  parent() is then implemented by walking the tree to find the parent.  In this case, the returned parent pointer is not part of the child's state, so one might assume that the access pattern Gregg describes is OK.  But the usage in oshima's example shows that logical constness is still violated once the pointer is returned.  Generalize this, and you will understand why after some discussion we eventually concluded that returning a non-const pointer from a const accessor is basically never OK.

I realize my message only went to oshima. Anyway, I disagree strongly with this rule being applied always.

There are legitimate cases where an object and its owned state are const, but a parent/owning object can change its state without modifying the original object. It should be able to do so without requiring a const_cast.

For example, an object A can have a parent B that keeps data associated with A in a map keyed with object A. It's perfectly valid to have A::parent() be const, but return a non-const B*, and do a->parent()->set_data(a, 3).

const is for the compiler to do optimizations. You can use it to do some safety checks in code where it makes sense, but don't overdo it.

Antoine

  

(FWIW, WebKit's existing code violates logical constness a lot; I started a thread about this once, and got an OK to at least make these sorts of accessors non-const, which I checked in a patch or two for, but there are still many problems and fixing them all would take a lot of time and effort.)

PK

--

[Brett Wilson]

On Wed, Jun 6, 2012 at 2:53 PM, Antoine Labour <pi...@google.com> wrote:

On Wed, Jun 6, 2012 at 2:36 PM, Peter Kasting <pkas...@chromium.org> wrote:

On Wed, Jun 6, 2012 at 1:27 PM, oshima <osh...@chromium.org> wrote:

I received a concern about making this strict rule because there are some cases that returned pointer is not considered

a part of the state of the object, so I'll make it just a strong recommendation (like const accessor in google style guide) with

clear explanation.

This confusion is actually part of why we have this rule.  The problem is that in general, _all_ returned pointers are "part of the state" because the issue is not whether they're members of the const object, but whether there is anything in the transitive closure of everything accessible from the returned pointer that allows you to affect something that the const object sees.  The answer to this question is practically always "yes" (indeed, if not, why would you even have an accessor for this pointer?) and thus this non-const returned pointer can basically always be used to violate logical constness.

To put this another way, consider oshima's example code, but imagine that the implementation of HTMLElement does not in fact include a parent HTMLElement*, but simply has some pointer to the top-level document.  parent() is then implemented by walking the tree to find the parent.  In this case, the returned parent pointer is not part of the child's state, so one might assume that the access pattern Gregg describes is OK.  But the usage in oshima's example shows that logical constness is still violated once the pointer is returned.  Generalize this, and you will understand why after some discussion we eventually concluded that returning a non-const pointer from a const accessor is basically never OK.

I realize my message only went to oshima. Anyway, I disagree strongly with this rule being applied always.

There are legitimate cases where an object and its owned state are const, but a parent/owning object can change its state without modifying the original object. It should be able to do so without requiring a const_cast.

For example, an object A can have a parent B that keeps data associated with A in a map keyed with object A. It's perfectly valid to have A::parent() be const, but return a non-const B*, and do a->parent()->set_data(a, 3).

const is for the compiler to do optimizations. You can use it to do some safety checks in code where it makes sense, but don't overdo it.

As I've said in other threads, I don't think we need to add all of these kinds of things to the style guide.

Use const correctly. If your function is logically const, add a const. If it's not, leave it off. It doesn't seem like a big deal to think about whether const means the right thing for your case. I don't think we need to have a big style thread about every case like this.

Brett 

[Peter Kasting]

On Wed, Jun 6, 2012 at 2:53 PM, Antoine Labour <pi...@google.com> wrote:

I realize my message only went to oshima. Anyway, I disagree strongly with this rule being applied always.

There are legitimate cases where an object and its owned state are const, but a parent/owning object can change its state without modifying the original object. It should be able to do so without requiring a const_cast.

For example, an object A can have a parent B that keeps data associated with A in a map keyed with object A. It's perfectly valid to have A::parent() be const, but return a non-const B*, and do a->parent()->set_data(a, 3).

I would argue this violates logical constness, and therefore is not acceptable.

If some code is logically const for object X, that means nothing about X, anything X can see, or anything related to or touched by X changes.  This is a much stricter guarantee than physical constness.

const is for the compiler to do optimizations.

const cannot be used by compilers to do optimizations.  const_cast means that the compiler can't ever rely on "const" meaning anything.  const is solely for humans.

You can use it to do some safety checks in code where it makes sense, but don't overdo it.

In general, we follow the advice of this sentence by making things non-const.  There is nothing wrong with a non-const accessor that returns a non-const pointer.

To put it another way: if in doubt, just don't add consts.

PK

[William Chan (陈智昌)]

+1

 

Brett 

[Jeffrey Yasskin]

On Wed, Jun 6, 2012 at 2:53 PM, Antoine Labour <pi...@google.com> wrote:

> const is for the compiler to do optimizations. You can use it to do some
> safety checks in code where it makes sense, but don't overdo it.

I don't have much opinion about the rest of the thread, but I have to
jump in for misunderstandings of C++.

const only very rarely helps the compiler optimize. When you define a
whole object 'const', then the compiler can assume it never changes.
That is, in:

const MyObject obj;
int i = obj.non_mutable_field;
DoStuff(&obj);
if (i == obj.non_mutable_field) {
...
}

The compiler is free to optimize away the equality check, even if
DoStuff() uses const_cast to modify its argument.

*However*, this almost never comes up in practice. Usually instead the
compiler sees:

void DoStuff(const MyObject& obj) {
int i = obj.non_mutable_field;
DoMoreStuff(&obj);
if (i == obj.non_mutable_field) {
...
}
}

Now the compiler is *not* allowed to remove the == without analyzing
DoMoreStuff, since DoMoreStuff could have its own, non-const,
reference to 'obj', and could modify non_mutable_field through that
reference. Or DoMoreStuff could somehow know that obj was originally
defined a non-const, and only had the const added through an implicit
conversion, in which case modifying through the const_cast would have
defined results (despite being bad style).

So, no, const is not there primarily for the benefit of compilers.
It's there to help programmers understand our code.

Jeffrey

[Peter Kasting]

On Wed, Jun 6, 2012 at 3:05 PM, Jeffrey Yasskin <jyas...@chromium.org> wrote:

When you define a

whole object 'const', then the compiler can assume it never changes.
That is, in:

const MyObject obj;
int i = obj.non_mutable_field;
DoStuff(&obj);
if (i == obj.non_mutable_field) {
 ...
}

The compiler is free to optimize away the equality check, even if
DoStuff() uses const_cast to modify its argument.

Thanks for this technical clarity -- I didn't realize the compiler could do this :)

PK 

[Peter Kasting]

On Wed, Jun 6, 2012 at 3:00 PM, Brett Wilson <bre...@chromium.org> wrote:

Use const correctly. If your function is logically const, add a const. If it's not, leave it off. It doesn't seem like a big deal to think about whether const means the right thing for your case. I don't think we need to have a big style thread about every case like this.

I would agree, except that there's been way too much misuse of const I've had to personally clean up in Chrome for me to believe people really understand the issue :(.  (The fact that codesearch finds 954 instances of "const_cast" kind of worries me, for example.)

PK

[Jeffrey Yasskin]

That's [dcl.type.cv]p4 in C++03: "Except that any class member
declared mutable (7.1.1) can be modified, any attempt to modify a
const object during its lifetime (3.8) results in undefined behavior."
The part that compilers usually can't do is determine that an object
is a "const object" (except when it breaks your code, of course).

Jeffrey

[oshima]

(to correct destination this time)

On Wed, Jun 6, 2012 at 3:04 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

On Wed, Jun 6, 2012 at 3:00 PM, Brett Wilson <bre...@chromium.org> wrote:

On Wed, Jun 6, 2012 at 2:53 PM, Antoine Labour <pi...@google.com> wrote:

On Wed, Jun 6, 2012 at 2:36 PM, Peter Kasting <pkas...@chromium.org> wrote:

On Wed, Jun 6, 2012 at 1:27 PM, oshima <osh...@chromium.org> wrote:

I received a concern about making this strict rule because there are some cases that returned pointer is not considered

a part of the state of the object, so I'll make it just a strong recommendation (like const accessor in google style guide) with

clear explanation.

This confusion is actually part of why we have this rule.  The problem is that in general, _all_ returned pointers are "part of the state" because the issue is not whether they're members of the const object, but whether there is anything in the transitive closure of everything accessible from the returned pointer that allows you to affect something that the const object sees.  The answer to this question is practically always "yes" (indeed, if not, why would you even have an accessor for this pointer?) and thus this non-const returned pointer can basically always be used to violate logical constness.

To put this another way, consider oshima's example code, but imagine that the implementation of HTMLElement does not in fact include a parent HTMLElement*, but simply has some pointer to the top-level document.  parent() is then implemented by walking the tree to find the parent.  In this case, the returned parent pointer is not part of the child's state, so one might assume that the access pattern Gregg describes is OK.  But the usage in oshima's example shows that logical constness is still violated once the pointer is returned.  Generalize this, and you will understand why after some discussion we eventually concluded that returning a non-const pointer from a const accessor is basically never OK.

I realize my message only went to oshima. Anyway, I disagree strongly with this rule being applied always.

There are legitimate cases where an object and its owned state are const, but a parent/owning object can change its state without modifying the original object. It should be able to do so without requiring a const_cast.

For example, an object A can have a parent B that keeps data associated with A in a map keyed with object A. It's perfectly valid to have A::parent() be const, but return a non-const B*, and do a->parent()->set_data(a, 3).

const is for the compiler to do optimizations. You can use it to do some safety checks in code where it makes sense, but don't overdo it.

As I've said in other threads, I don't think we need to add all of these kinds of things to the style guide.

Use const correctly. If your function is logically const, add a const. If it's not, leave it off.

This thread is about the return type of const accessor, where the function itself is indeed const, but can cause the side effect that can modify the const object.

And we sometimes disagree what's correct, which makes code review sometime painful because you have to explain it each time.

 

It doesn't seem like a big deal to think about whether const means the right thing for your case. I don't think we need to have a big style thread about every case like this.

+1

 

-1

I disagree not to add a style that is treated as standard to the style guide, just because it makes it longer. I believe adding 2~3 line to style

guide is much more efficient than spending engineering time to explain it over and over again (if there is a consensus, of cause).

If you're really concerned about about the size, I'd suggest to clean up existing style guide and remove obvious ones like

"Don't use else after return:", or dups (Unnamed namepsace is already recommended in google style guide, for example).

- oshima

[William Chan (陈智昌)]

I think that the style guide is large enough that the vast majority of people don't know 90% of it. I often am pointing out parts of the style guide that people don't know. If your hope is to prevent problems from occurring in the first place, I think it's naive to assume that the addition of a new rule in the style guide is going to actually change anything. Do people actually debate the issue of logical constness with you? If you're using the style guide to settle debates, then yes, that's useful. I'd be disappointed if people actually debate this very much, mod some corner exceptional cases discussed earlier in the thread. Are all these instances corner cases like that?

[Peter Kasting]

On Wed, Jun 6, 2012 at 5:07 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

Do people actually debate the issue of logical constness with you?

Yes.

If you're using the style guide to settle debates, then yes, that's useful.

That is my goal here.  I don't know about oshima's.

Although I'm probably not as pessimistic as the group that always argues for shorter style guides, either.  I do think some people actually try to read and learn the style guide and adding things there can have a nonzero effect :)

 

I'd be disappointed if people actually debate this very much, mod some corner exceptional cases discussed earlier in the thread. Are all these instances corner cases like that?

It's not clear to me what you consider a corner case.  But regardless, I thought a key point of a style guide was to have a consistent policy on the corner cases.  If there weren't corner cases, wouldn't "obviousness" mean we could do away with lots of things?

PK

[Ilya Sherman]

Actual debate might be rare, but it's somewhat common for someone to ask "Why is that better?".  It's helpful to have a canonical resource to link to.

[Steven Bennetts]

If you're using the style guide to settle debates, then yes, that's useful.

That is my goal here.  I don't know about oshima's.

I specifically suggested to oshima that we add a few lines to the style guide because of the differing opinions on the specific issue of whether or not an accessor returning a non const pointer should be const. 

Being able to look up what the Chromium preference is in the style guide will reduce (ideally eliminate) the need for future discussion/speculation on the topic in a code review.

[Daniel Cheng]

I don't really care what the outcome of this debate is. However, it seems odd to add a style rule that things like scoped_ptr don't even follow.

Daniel

[Peter Kasting]

On Wed, Jun 6, 2012 at 5:32 PM, Daniel Cheng <dch...@chromium.org> wrote:

I don't really care what the outcome of this debate is. However, it seems odd to add a style rule that things like scoped_ptr don't even follow.

Yeah, those should probably be fixed (presumably by splitting into const and non-const versions that return const and non-const objects).  I suspect usage of "const scoped_ptr" and "const scoped_ptr&" in our codebase is quite low, though.  Codesearch gives 46 entries for "const\ scoped_ptr".  File http://crbug.com/131513 against myself.

PK

[William Chan (陈智昌)]

I point them to Effective C++ Item 3 where Scott Meyers educates people on logical constness.

[Peter Kasting]

On Wed, Jun 6, 2012 at 5:41 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

Actual debate might be rare, but it's somewhat common for someone to ask "Why is that better?".  It's helpful to have a canonical resource to link to.

I point them to Effective C++ Item 3 where Scott Meyers educates people on logical constness.

Me too!  But even reading that still leaves one a bit vague on whether this particular case can ever be OK.

PK

[Ilya Sherman]

Is Effective C++ published to be freely accessible on the Internet somewhere?  I agree that it's a great resource, but not every Chromium developer owns a copy ;)

[William Chan (陈智昌)]

Get a copy. Seriously.

[William Chan (陈智昌)]

I disagree here. I'll respond on the bug, but basically I agree with vtl.

[oshima]

On Wed, Jun 6, 2012 at 5:32 PM, Daniel Cheng <dch...@chromium.org> wrote:

I don't really care what the outcome of this debate is. However, it seems odd to add a style rule that things like scoped_ptr don't even follow.

This will be strong recommendation but not strict rule and there will be some exceptions.

For example, bind helper is using multiple inheritance, but it doesn't mean multiple inheritance should be allowed in general.

- oshima

[John Abd-El-Malek]

On Wed, Jun 6, 2012 at 10:23 PM, oshima <osh...@chromium.org> wrote:

On Wed, Jun 6, 2012 at 5:32 PM, Daniel Cheng <dch...@chromium.org> wrote:

I don't really care what the outcome of this debate is. However, it seems odd to add a style rule that things like scoped_ptr don't even follow.

This will be strong recommendation but not strict rule and there will be some exceptions.

This makes it not very useful, since the style guide should have (few) rules that are easy to follow. If it becomes a judgement call, then it might as well not be there.

It's also not clear to me why, if this would be helpful, it's not in the Google style guide. Sticking as closely as possible to it is very valuable, and the bar to diverge should be very high. IMO it should be to stop serious bugs, which I don't see this helping against.

[Peter Kasting]

On Wed, Jun 6, 2012 at 10:23 PM, oshima <osh...@chromium.org> wrote:

On Wed, Jun 6, 2012 at 5:32 PM, Daniel Cheng <dch...@chromium.org> wrote:

I don't really care what the outcome of this debate is. However, it seems odd to add a style rule that things like scoped_ptr don't even follow.

Will has convinced me that scoped_ptr<> should be different because it's meant to match the C++11 unique_ptr<>, which in turn uses this convention because it's trying to pretend to be a pointer rather than an object.

This will be strong recommendation but not strict rule and there will be some exceptions.

For example, bind helper is using multiple inheritance, but it doesn't mean multiple inheritance should be allowed in general.

Doesn't the Google style guide outright ban multiple inheritance (except when all but one of the bases is pure virtual)?

I'm not suggesting by that that we need to go rewrite this code, but it seems like having an exceptional case where we explicitly decide to violate the style guide doesn't imply that the style guide has to be written to explicitly allow exceptions.  Just say not to do it.  Don't tell people probably not to do it.

PK

[oshima]

I believe you're joking. I'm sure most people implicitly knows more than 50% of it, just by following the existing code.

 

I often am pointing out parts of the style guide that people don't know. If your hope is to prevent problems from occurring in the first place, I think it's naive to assume that the addition of a new rule in the style guide is going to actually change anything. Do people actually debate the issue of logical constness with you? If you're using the style guide to settle debates, then yes, that's useful. I'd be disappointed if people actually debate this very much, mod some corner exceptional cases discussed earlier in the thread. Are all these instances corner cases like that?

Actual debate might be rare, but it's somewhat common for someone to ask "Why is that better?".  It's helpful to have a canonical resource to link to.

I point them to Effective C++ Item 3 where Scott Meyers educates people on logical constness.

Sorry, but I don't get the point. You concerned that the style guide is loo large, yet you recommend to read a book that is way larger?

I disagree that "the style guide is too large" is valid reason not to extended it. It value is in being clear, rather than being short guide line.

If I have to pick, "long, but clear style guide" vs "ambiguous, but short style guide", I'd pick former.

If you think the proposal itself doesn't make sense, that's another story tho.

- oshima 

[Brett Wilson]

Not everything can be dictated in the style guide. It's everybody's responsibility to write good code and make the proper technical decisions based on their particular case. There is too much code and there are too many people on the team to make people do every little thing the way you want them to. Arguing either way about these cases won't make any significant changes in the code base.

Brett

[William Chan (陈智昌)]

Sorry, I wasn't joking, but I don't know why I wrote 90%. That's clearly wrong. But they don't know large chunks of it.

 

 

I often am pointing out parts of the style guide that people don't know. If your hope is to prevent problems from occurring in the first place, I think it's naive to assume that the addition of a new rule in the style guide is going to actually change anything. Do people actually debate the issue of logical constness with you? If you're using the style guide to settle debates, then yes, that's useful. I'd be disappointed if people actually debate this very much, mod some corner exceptional cases discussed earlier in the thread. Are all these instances corner cases like that?

Actual debate might be rare, but it's somewhat common for someone to ask "Why is that better?".  It's helpful to have a canonical resource to link to.

I point them to Effective C++ Item 3 where Scott Meyers educates people on logical constness.

Sorry, but I don't get the point. You concerned that the style guide is loo large, yet you recommend to read a book that is way larger?

I disagree that "the style guide is too large" is valid reason not to extended it. It value is in being clear, rather than being short guide line.

If I have to pick, "long, but clear style guide" vs "ambiguous, but short style guide", I'd pick former.

Effective C++ teaches people how to use C++ effectively. The style guide primarily establishes consistent coding conventions. I think understanding logical constness is not a coding convention issue, but an issue about understanding how to use C++ effectively.

[oshima]

On Wed, Jun 6, 2012 at 10:41 PM, John Abd-El-Malek <j...@chromium.org> wrote:

On Wed, Jun 6, 2012 at 10:23 PM, oshima <osh...@chromium.org> wrote:

On Wed, Jun 6, 2012 at 5:32 PM, Daniel Cheng <dch...@chromium.org> wrote:

I don't really care what the outcome of this debate is. However, it seems odd to add a style rule that things like scoped_ptr don't even follow.

This will be strong recommendation but not strict rule and there will be some exceptions.

This makes it not very useful, since the style guide should have (few) rules that are easy to follow. If it becomes a judgement call, then it might as well not be there.

Strong recommendation means "follow this unless there is strong reason not to do so", or "Prefer X over Y". it's pretty common in the style guide

and still useful.

 

It's also not clear to me why, if this would be helpful, it's not in the Google style guide. Sticking as closely as possible to it is very valuable, and the bar to diverge should be very high. IMO it should be to stop serious bugs, which I don't see this helping against.

Sounds like it's hard to reach the consensus that this proposal itself is useful. I'm fine to leave the style guide as is if that's the case.

- oshima 

[Ian Fette]

Will, I largely agree with you, except that "const" has a tendency to creep ("Oh, this function is const? Well, now I need const versions of the following N things...") and so having agreement on the level of const correctness you're going for can be helpful... 

[oshima]

Style guide is about both, coding convention and how to use C++ effectively. Examples are

multiple inheritance, RTTI, overloading, copy constructor, preincrement, and more. 

In any case, it sounds difficult to reach agreement and to change the style guide now.

I'll withdraw my proposal and move on.

- oshima