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

Jason Wilkins jason.a.wilkins at gmail.com
Mon Oct 15 01:37:12 CEST 2012


Well that's unfortunate because that would have been acceptable to me.

On Wed, Oct 10, 2012 at 11:03 PM, Campbell Barton <ideasman42 at gmail.com> wrote:
> Checked, and that would work in MSVC since its the same warning that
> checks a function matches its declaration in the header,
> so turning that off isn't an option (was thinking it may have been a
> different warning ID).
>
>
> On Thu, Oct 11, 2012 at 12:45 AM, Campbell Barton <ideasman42 at gmail.com> wrote:
>> @Jason, wouldn't it work to remove offending const from headers and
>> disable the warning in MSVC?
>>
>> On Wed, Oct 10, 2012 at 9:57 PM, Jason Wilkins
>> <jason.a.wilkins at gmail.com> wrote:
>>> C is ALWAYS call by value and C++ is call by value by default there is
>>> no need to mark the parameters const to know it will not change the
>>> caller's value in that case.  The const on a call by value function
>>> parameter is an internal detail of the function itself.  The function
>>> is saying that it won't change its copy.  To the caller this is like
>>> function saying what kind of magazines are hidden under its mattress.
>>>
>>> I do agree that C# is nice in that you have to mark a parameter as
>>> 'ref' both in the interface and at each call site (same with having to
>>> declare a shadowing variable as 'shadow').  However imagine if you had
>>> to prefix 'val' every time you called the function even though that is
>>> the default.  That would be very ugly and unneeded.  That sort of
>>> thing would be similar to what I'm arguing against here.
>>>
>>> I've made my case though, so if people disagree I'll just have to live
>>> with their code.  I won't belabor the point any more.
>>>
>>> On Tue, Oct 9, 2012 at 3:02 PM, Dahlia Trimble <dahliatrimble at gmail.com> wrote:
>>>> I don't poke around much in blender source but I've found in c++ in general
>>>> that I kind of like being able to declare a function parameter as const,
>>>> for the simple reason that I don't see well and it helps me easily
>>>> differentiate foo(const int bar) from foo(int& bar). Where in the latter I
>>>> might not notice it's a reference.
>>>>
>>>> I like how C# handles references: if you declare a function you need to use
>>>> the ref keyword both in the declaration and when calling the function. This
>>>> way you cannot call a function that may modify a passed parameter without
>>>> explicitly stating that you are aware it's passed by reference. I think c++
>>>> could use something similar.
>>>>
>>>>
>>>> On Sun, Oct 7, 2012 at 10:17 PM, Jason Wilkins <jason.a.wilkins at gmail.com>wrote:
>>>>
>>>>> 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
>>>>> _______________________________________________
>>>>> 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
>>> _______________________________________________
>>> Bf-committers mailing list
>>> Bf-committers at blender.org
>>> http://lists.blender.org/mailman/listinfo/bf-committers
>>
>>
>>
>> --
>> - Campbell
>
>
>
> --
> - Campbell
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> http://lists.blender.org/mailman/listinfo/bf-committers


More information about the Bf-committers mailing list