'Re: [webkit-dev] Using C++ constant local variables in WebKit'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       webkit-dev
Subject:    Re: [webkit-dev] Using C++ constant local variables in WebKit
From:       David Levin <levin () chromium ! org>
Date:       2011-11-30 21:35:18
Message-ID: CACmjMJSVcc9datCR7a1rkMT88qrcyJOsvaXP0uyWQV_dBcfGFw () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]

On Tue, Nov 29, 2011 at 6:19 PM, Darin Adler <darin@apple.com> wrote:

> On Nov 28, 2011, at 1:38 PM, David Kilzer wrote:
>
> In a discussion on Bug 71921<https://bugs.webkit.org/show_bug.cgi?id=71921>,
> Antti, Darin Adler and I started a discussion about using C++ constant
> pointers in WebKit.  Does the WebKit community have a consensus opinion on
> the matter?
>
>
> I thought we were discussing local variables in general, not pointer-typed
> ones specifically.
>
> * Pros
>   - Documents use of variable.
>
>
> I would say “documents the fact that the variable’s value is not changed”.
> I think it’s overstating things to say it “documents use”.
>
>   - Prevents misuse of variable in a later patch (by a different author)
> through enforcement of const-ness.
>
>
> Prevents one specific type of misuse: Setting the variable to another
> value. And that may not be misuse despite the fact that the original author
> didn’t plan on changing it.
>
>   - May help compiler optimize code.  (We weren't sure whether modern
> compilers do this on their own or not.)
>
>
> Doesn’t.
>
> * Cons
>   - Darin Adler doesn't ever recall fixing a bug in WebKit where a
> constant pointer would have helped.
>
>
> While true, not really a “con”; just weakens the “pro” argument above that
> this prevents misuse.
>
>   - Slightly more verbose syntax for constant pointers to a constant
> string (const char * const pointer;) or even a constant pointer to a
> mutable string (char * const pointer;).
>
>
> Not sure this is a con. Just stating what the C++ syntax.
>
> This is the con I am aware of:
>
>   - Less brief than omitting const.
>
> I’m not strongly opposed to using const more, but I am mildly opposed to
> it.
>

I can see your point of view. All of this seems to apply equally to
const_iterator as well. Are you mildly opposed to it as well? Or is
something different about it?

Minor plug (which is why I happened this think about this): An additional
current downside for const_iterator is that hashtable const_iterator has an
unfortunate issue where it can't be compared ==, != to an iterator which
isn't nice. https://bugs.webkit.org/show_bug.cgi?id=73370

dave

[Attachment #5 (text/html)]

<br><br><div class="gmail_quote">On Tue, Nov 29, 2011 at 6:19 PM, Darin Adler <span \
dir="ltr">&lt;<a href="mailto:darin@apple.com">darin@apple.com</a>&gt;</span> \
wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px \
#ccc solid;padding-left:1ex;">

<div style="word-wrap:break-word"><div><div>On Nov 28, 2011, at 1:38 PM, David Kilzer \
wrote:</div><br><blockquote type="cite"><div style="word-wrap:break-word">In a \
discussion on <a href="https://bugs.webkit.org/show_bug.cgi?id=71921" \
target="_blank">Bug 71921</a>, Antti, Darin Adler and I started a discussion about \
using C++ constant pointers in WebKit.  Does the WebKit community have a consensus \
opinion on the matter?</div>

</blockquote><div><br></div><div>I thought we were discussing local variables in \
general, not pointer-typed ones specifically.</div><br><blockquote type="cite"><div \
style="word-wrap:break-word"><div>* Pros</div><div>  - Documents use of \
variable.</div>

</div></blockquote><div><br></div><div>I would say “documents the fact that the \
variable’s value is not changed”. I think it’s overstating things to say it \
“documents use”.</div><br><blockquote type="cite"><div style="word-wrap:break-word">

<div>  - Prevents misuse of variable in a later patch (by a different author) through \
enforcement of const-ness.</div></div></blockquote><div><br></div><div>Prevents one \
specific type of misuse: Setting the variable to another value. And that may not be \
misuse despite the fact that the original author didn’t plan on changing it.</div>

<br><blockquote type="cite"><div style="word-wrap:break-word"><div>  - May help \
compiler optimize code.  (We weren&#39;t sure whether modern compilers do this on \
their own or not.)</div></div></blockquote><div><br></div> <div>
Doesn’t.</div><br><blockquote type="cite"><div style="word-wrap:break-word"><div>* \
Cons</div><div><div>  - Darin Adler doesn&#39;t ever recall fixing a bug in WebKit \
where a constant pointer would have helped.</div></div>

</div></blockquote><div><br></div><div>While true, not really a “con”; just weakens \
the “pro” argument above that this prevents misuse.</div><br><blockquote \
type="cite"><div style="word-wrap:break-word"><div>  - Slightly more verbose syntax \
for constant pointers to a constant string (const char * const pointer;) or even a \
constant pointer to a mutable string (char * const pointer;).</div>

</div></blockquote><div><br></div>Not sure this is a con. Just stating what the C++ \
syntax.<br><br></div><div><div>This is the con I am aware \
of:</div><div><br></div><div>  - Less brief than omitting const.</div><div><br>

</div><div>I’m not strongly opposed to using const more, but I am mildly opposed to \
it.</div></div></div></blockquote><div><br></div><div>I can see your point of view. \
All of this seems to apply equally to const_iterator as well. Are you mildly opposed \
to it as well? Or is something different about it?</div>

<div><br></div><div>Minor plug (which is why I happened this think about this): An \
additional current downside for const_iterator is that hashtable const_iterator has \
an unfortunate issue where it can&#39;t be compared ==, != to an iterator which \
isn&#39;t nice. <a href="https://bugs.webkit.org/show_bug.cgi?id=73370">https://bugs.webkit.org/show_bug.cgi?id=73370</a></div>

<div><br></div><div>dave</div></div>

_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[prev in list] [next in list] [prev in thread] [next in thread] 