Every. Single. Class.



  • Throughout a half-million LOC project: every single class with a pointer member has duplicated accessors to that member.

    class Bar;
    class Foo {
    public:
        // ...
        Bar* bar() { return bar; }
        const Bar* bar() const { return bar; }
    
    private:
        Bar* bar;
    };
    

    No one understands the difference between const Bar* and Bar* const. Bonus wtf; our custom smart pointer class (pre-STL) is written this way too, so that

    void foo(const SmartPt& pt) {
        pt->nonConstMethod();
    }
    

    doesn't compile.



  • Makes you want to head to the bar


  • FoxDev

    @Snuffles said:

    No one understands the difference between const Bar* and Bar* const.

    I'm not experienced with C++, but I believe the former means you can change where the pointer points, but not the thing it's pointing at. The latter, you can change what it's pointing at, but not where it points.

    Or something.



  • @RaceProUK said:

    I'm not experienced with C++, but I believe the former means you can change where the pointer points, but not the thing it's pointing at. The latter, you can change what it's pointing at, but not where it points.

    Exactly.



  • That... um... mostly seems quite reasonable.

    @Snuffles said:

    Bar* bar() { return bar; }
    const Bar* bar() const { return bar; }
    ...thus making sure that if you have a const Foo, you can't change the Bar it's holding onto. I don't see the problem, at least if you want Foo to have valueish semantics.

    The smart pointer part is wrong though.



  • @EvanED said:

    That... um... mostly seems quite reasonable.

    ...thus making sure that if you have a const Foo, you can't change the Bar it's holding onto. I don't see the problem, at least if you want Foo to have valueish semantics.

    Bar* bar() const { return bar; }
    

    is the right way to do it. Otherwise it's the same error as with the smart pointer. There is a difference between changing the pointer's referent and changing the pointer itself.

    Obviously this doesn't apply to all cases; if for example I've got a pointer because the class is forward declared and I really do want it to have value semantics. These guys use it for everything, including weak pointers.



  • Well trolled. The code as written considers that the data pointed to is a part of the object, so if the object or member is const then the data pointed to in return is const. Otherwise you get a pointer that you can use to change the object that is supposed to be const. I could quote Item 21 of the Second Edition of Meyer's Effective C++ if you wish (Item 3 in 3rd edition). Returning a const pointer to non-const data is rarely if ever useful, because you usually don't care what your caller does to his data. Your custom smart pointer class . . . well, even though EvanED thinks it's wrong, if you have half a million pre-STL LOC working on the premise that "const SmartPt" is a pointer to something const, I can work with that. You hiring?



  • For smart pointers, this would probably be a better choice:

    template< class T, bool PropagateConst > class SmartPtr //code goes next


  • @Medinoc said:

    For smart pointers, this would probably be a better choice:

    template&lt; class T, bool PropagateConst &gt; class SmartPtr //code goes next</blockquote>
    

    That's a great idea!



  • @Lawrence said:

    Well trolled. The code as written considers that the data pointed to is a part of the object, so if the object or member is const then the data pointed to in return is const. Otherwise you get a pointer that you can use to change the object that is supposed to be const. I could quote Item 21 of the Second Edition of Meyer's Effective C++ if you wish (Item 3 in 3rd edition). Returning a const pointer to non-const data is rarely if ever useful, because you usually don't care what your caller does to his data. Your custom smart pointer class . . . well, even though EvanED thinks it's wrong, if you have half a million pre-STL LOC working on the premise that "const SmartPt" is a pointer to something const, I can work with that. You hiring?

    You warmed my steely heart with a reference to Effective C++. Yes we're hiring.


  • Discourse touched me in a no-no place

    @Lawrence said:

    The code as written considers that the data pointed to is a part of the object, so if the object or member is const then the data pointed to in return is const.

    The only compiler I've had problems with that sort of thing with was MSVC (specifically with an argument that was declared as a const pointer to non-const data). All the others consider the constness of a pointer to something to be not something that propagates; who else cares whether you change the pointer?

    Everything was fine, except a downstream-developer reported that MSVC didn't like it at all.



  • I haven't seen any problem on MSVC about pointer constness, except for one nonstandard behavior in C:

    • In C++, a Thing** is implicitly convertible into a const Thing * const * (but not a const Thing**, because that would break const-correctness).
    • In standard C, that's not possible for reasons that elude me (an explicit cast is required).
    • MSVC accepts it in both languages.

  • Discourse touched me in a no-no place

    I was dealing with arguments that were char * const (i.e., can't change the variable itself, but what it points to is open season) and which only MSVC didn't like it that that was being used with type declarations that said char *. OK, might not've been char * but the point stands: whether a function modifies its own copy of a pointer is nothing to do with the caller or the type signature.

    I couldn't probe in detail: I didn't have a copy of MSVC (and never did, FWIW). I just fixed things so that everything worked anyway; removing a const from a place where it shouldn't have mattered fixed everything. And the function? It just continued to never modify that pointer variable…


  • FoxDev

    Yes, you fixed the issue, but I'd be wary of removing a const in that way. It's just asking for someone to make their own change which introduces a far more insidious bug. Wouldn't it have been better to add the const to the signature of the function being called, so the types match?


  • Discourse touched me in a no-no place

    @RaceProUK said:

    It's just asking for someone to make their own change which introduces a far more insidious bug.

    But then they'll just break that function. It's pretty thoroughly tested elsewhere (about 280 unit test cases go through this function) so fucking it up will be detected.

    @RaceProUK said:

    Wouldn't it have been better to add the const to the signature of the function being called, so the types match?

    But that would affect other code, less under my control.

    Hmm, revisiting the code shows that the const is back. Must've been resolved to be a bug in some older version of MSVC. I forget the details, just that it was one of the more bizarre episodes of programming (though not as odd as finding out that sizeof(char*)!=sizeof(void*) on some platforms…)



  • Ah, you may have been confronted to a problem I had forgotten about MSVC and constness: To Visual C++ version (unknown) to (unknown), a by-value argument's constness, instead of being an implementation detail, is part of the function's signature. And part of its mangled name. For example, if you declare a C++ function thusly:

    int Sum(const int* pointerPassedByValue, size_t length);
    

    And implement it thusly:

    int Sum(const int* const pointerPassedByValue, size_t const length)
    {
        int sum = 0;
        for(size_t i=0; i<length ; i++) { sum += pointerPassedByValue[i]; }
        return sum;
    }
    

    You will get a linker error when you try to call it from another .cpp file.

    Note that this behavior may have changed since, as C++ name mangling is compiler-specific.



  • @Medinoc said:

    Ah, you may have been confronted to a problem I had forgotten about MSVC and constness: To Visual C++ version (unknown) to (unknown), a by-value argument's constness, instead of being an implementation detail, is part of the function's signature. And part of its mangled name.

    Somehow, I smell a violation of the C++ standard here, namely dcl.fct paragraph 5:

    A single name can be used for several different functions in a single scope; this is function overloading (Clause 13). All declarations for a function shall agree exactly in both the return type and the parameter-type-list. The type of a function is determined using the following rules. The type of each parameter (including function parameter packs) is determined from its own *decl-specifier-seq* and *declarator*. After determining the type of each parameter, any parameter of type “array of `T`” or “function returning `T`” isadjusted to be “pointer to `T`” or “pointer to function returning `T`,” respectively. After producing the list of parameter types, any top-level *cv-qualifiers* modifying a parameter type are deleted when forming the function type. The resulting list of transformed parameter types and the presence or absence of the ellipsis or a function parameter pack is the function’s *parameter-type-list*. [Note: This transformation does not affect the types of the parameters. For example, `int(*)(const int p, decltype(p)*)` and `int(*)(int, const int*)` are identical types.—end note]


  • Even better, this changes on whether the function has been declared previously. Here is a sample that reproduces the problem in Visual 2005:
    Header file:

    //TestConstParam.hpp
    #include < cstddef>
    
    int Sum(int const *tab, std::size_t length);
    

    Source file 1:

    //TestConstParam1.cpp
    #include <cstddef>
    //#include "TestConstParam.hpp"
    
    int Sum(int const * const tab, std::size_t const length)
    {
        int sum = 0;
        for(std::size_t i=0; i<length ; i++) { sum += tab[i]; }
        return sum;
    }
    

    Source file 2:

    //TestConstParam2.cpp
    #include "TestConstParam.hpp"
    
    void TestConstParam(void)
    {
        int tab[] = {1, 2, 3};
        int sum1 = Sum(tab, 3);
        sum1++; //hides "unused" warning
    }
    

    This reproduces the problem in Visual 2005, but it disappears if you uncomment the #include line in TestConstParam1.cpp. When the function is pre-declared with non-const parameters, this actually changes the mangled name under which the function definition is exported (from ?Sum@@YAHQEBH_K@Z to ?Sum@@YAHPEBH_K@Z)


Log in to reply