[Bf-committers] Using 'const' on primitive function arguments passed by value (Please don't do this)

[Jason Wilkins]

OK, when I get warnings I guess I will fix them by adding the const in
the appropriate spot.  At least this kind of const is not viral :)

On Sun, Oct 7, 2012 at 9:55 AM, Campbell Barton <ideasman42 at gmail.com> wrote:
> On Mon, Oct 8, 2012 at 1:26 AM, Jason Wilkins <jason.a.wilkins at gmail.com> wrote:
>> I probably would not notice except that it is done inconsistently and
>> I get lots of warnings.  I tend to fix things by removing the const
>> though.
>>
>> Another thing I noticed was returning a const pointer from a function
>> but then expecting to free it using 'free'.  Dynamically allocated
>> memory is not 'const' for the purposes of 'free'.  It would probably
>> be better to cast away the const inside a special function instead of
>> asking the user to use a raw 'free'.  (or just not use const).
> agree, const can be a bit of a pain like this.
>
>> I'm all for using const, but I really question if making primitive
>> arguments const is more trouble than it is worth. I mean, to follow
>> through on it would be a huge task.  It only generates a warning (no
>> error) when done inconsistently and if you change your mind now you
>> have to make a change in two places instead of one.
>
> You dont need to follow it though, if a dev wants to use it, they can,
> when done inconsistently it wont give warnings in GCC - but this stuff
> is really issue with multi-platform dev, it happens in other areas too
> - I often wake up to find blender wont compile because of an error in
> some commit from a dev with a different environment (-Werror helps
> here too :) ).
> you can correct warnings, or mail some other dev to fix, last I
> compiled on MSVC I didnt see any warnings like this though (a few
> weeks back).
>
>> To me this is a C programmers version of those people who have to turn
>> a light switch on and off a prime number of times.
>>
>> Reason #5 would be that it is just cluttered and ugly.  It decreases
>> readability instead of enhancing it.  It reminds me of when I had a
>> phase where I wanted to add 'struct' to everything so that people knew
>> that, yes, this is a struct.
>
> think this comes down to personal preference, if you think its not
> warranted, don't add it to your code.
>
>> I guess detecting stack corruption does not seem like a plus to me
>> because my environment does this very aggressively without help
>> (MSVC).
>
> Its no protection against stack corruption,
> It just means you know the var wont change under normal conditions, if
> it does change that something exceptional/wrong is happening.
>
>> On Sun, Oct 7, 2012 at 8:22 AM, Campbell Barton <ideasman42 at gmail.com> wrote:
>>> On Sun, Oct 7, 2012 at 8:25 PM, Jason Wilkins <jason.a.wilkins at gmail.com> wrote:
>>>> If I had a function with the prototype: foo(int bar)
>>>>
>>>> It may be tempting to declare the it as: foo(const int bar)
>>>>
>>>> The reason would be that bar is not modified inside of foo, so by
>>>> declaring it this way we prevent ourselves from accidentally modifying
>>>> it.
>>>>
>>>> This is not idiomatic C, and for good reasons.
>>>>
>>>> 1) We use 'const' on pointers to indicate that we are not going to
>>>> modify what is pointed at, when a programmer sees 'const int' it is
>>>> momentarily confusing because we expect 'const int*'
>>>>
>>>> 2) This exposes internal details of the function to the outside world.
>>>>  The fact that 'bar' is const in this case is not actually a part of
>>>> the interface of that function.
>>>>
>>>> 3) If we change our minds later and actually do want to modify the
>>>> copy of 'bar' inside the function then we have to change the interface
>>>> again, but as per #2 it actually has nothing to do with the user of
>>>> 'foo'
>>>>
>>>> 4) It is just not idiomatic.  Looking at it is like listening to a
>>>> foreigner speak your native language in "creative" ways.
>>>>
>>>> I have not figured out who is doing this, but please stop :)
>>>
>>> I've been doing this and Im not convinced its a bad thing, in some
>>> functions its a good hint that a var is `fixed` and shouldn't be
>>> changed.
>>> If a dev wants to change it they can just remove the `const` but it
>>> means they think twice before doing it (as in - maybe there is a good
>>> reason it shouldn't be changed).
>>>
>>> The main reason I like to have this sometimes is when debugging you
>>> know for sure a var wont change, if it does - its a buffer overflow or
>>> something exceptional.
>>> Often its not really an issue - but there are cases it can help verify
>>> whats going on when reading the function.
>>>
>>> That the `const` gets in the header is a little inconvenience if it
>>> changes often - but IMHO changing those is rare enough that its not an
>>> issue.
>>> _______________________________________________
>>> Bf-committers mailing list
>>> Bf-committers at blender.org
>>> http://lists.blender.org/mailman/listinfo/bf-committers
>> _______________________________________________
>> Bf-committers mailing list
>> Bf-committers at blender.org
>> http://lists.blender.org/mailman/listinfo/bf-committers
>
>
>
> --
> - Campbell
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers