Cppcoreguidelines: ES.50 advice leads to bloated binaries

Created on 12 Mar 2020  路  3Comments  路  Source: isocpp/CppCoreGuidelines

https://github.com/isocpp/CppCoreGuidelines/blob/2f0c2e5874db677122e7bbd3a8bcf3f33e5e824d/CppCoreGuidelines.md#Res-casts-const advices, to avoid code duplication, against return const_cast<Bar&>(static_cast<const Foo&>(*this).get_bar()); in favour of having the common code in a template.

While this does indeed result in eliminating the duplication in the source code, it doesn't remove it from the generated binary.

See https://godbolt.org/z/3csiVk

picture RedDwarf69
👎1

Most helpful comment

Don't do non-dependent work (like your printf+rand lines) inside a template, hoist it out into a separate function. The code is still going to be slightly larger than the horrible const_cast version, but not as bad as your example shows.

https://accu.org/content/conf2011/Jonathan-Wakely-diet-templates.pdf

picture jwakely on 13 Mar 2020
👍3 👎1

All 3 comments

Don't do non-dependent work (like your printf+rand lines) inside a template, hoist it out into a separate function. The code is still going to be slightly larger than the horrible const_cast version, but not as bad as your example shows.

https://accu.org/content/conf2011/Jonathan-Wakely-diet-templates.pdf

jwakely picture jwakely on 13 Mar 2020
👍3 👎1

Don't do non-dependent work (like your printf+rand lines) inside a template, hoist it out into a separate function. The code is still going to be slightly larger than the horrible const_cast version, but not as bad as your example shows.

The linked code is trying to simulate the /* the complex logic around getting a possibly-const reference to my_bar */ from the guideline while staying simple, I am not trying to say it's a good example of real code. And I agree the const_cast version is horrible. But if "The code is still going to be slightly larger than the horrible const_cast version, but not as bad as your example shows.", should the guidelines at least mention the caveat?

I actually though about this with a code more similar to the following. I'm using RapidJSON (which sure, it's not an example of modern C++), if you want to get data from:

{
    "data": {
        "value": 123
    }
}

You need to:
a) Call FindMember
b) Compare it to MemberEnd
c) Check it's an object

And that's the same every single time I want to "get_object_member". So I write a helper method for the task. I can either do:

#include <rapidjson/document.h>
#include <stdexcept>
#include <type_traits>

template<typename RapidjsonValue>
static auto
get_member(RapidjsonValue& object, const char* name) -> RapidjsonValue&
{
    static_assert(std::is_same_v<std::remove_const_t<RapidjsonValue>, rapidjson::Value>,
                  "get_member() can only be called with a rapidjson::Value reference");

    auto iter = object.FindMember(name);
    if (iter == object.MemberEnd()) {
        throw std::runtime_error{ "Value not found" };
    }

    return iter->value;
}

template<typename RapidjsonValue>
static auto
get_object_member_impl(RapidjsonValue& object, const char* name) -> RapidjsonValue&
{
    static_assert(std::is_same_v<std::remove_const_t<RapidjsonValue>, rapidjson::Value>,
                  "get_object_member_impl() can only be called with a rapidjson::Value reference");

    RapidjsonValue& value = get_member(object, name);
    if (!value.IsObject()) {
        throw std::runtime_error{ "Value is not object" };
    }

    return value;
}

auto
get_object_member(const rapidjson::Value& object, const char* name) -> const rapidjson::Value&
{
    return get_object_member_impl(object, name);
}

auto
get_object_member(rapidjson::Value& object, const char* name) -> rapidjson::Value&
{
    return get_object_member_impl(object, name);
}

or

#include <rapidjson/document.h>
#include <stdexcept>
#include <utility>

static auto
get_member(const rapidjson::Value& object, const char* name) -> const rapidjson::Value&
{
    auto iter = object.FindMember(name);
    if (iter == object.MemberEnd()) {
        throw std::runtime_error{ "Value not found" };
    }

    return iter->value;
}

static auto
get_member(rapidjson::Value& object, const char* name) -> rapidjson::Value&
{
    return const_cast<rapidjson::Value&>(get_member(std::as_const(object), name));
}

auto
get_object_member(const rapidjson::Value& object, const char* name) -> const rapidjson::Value&
{
    const rapidjson::Value& value = get_member(object, name);
    if (!value.IsObject()) {
        throw std::runtime_error{ "Value is not object" };
    }

    return value;
}

auto
get_object_member(rapidjson::Value& object, const char* name) -> rapidjson::Value&
{
    return const_cast<rapidjson::Value&>(get_object_member(std::as_const(object), name));
}

Using clang 9.0.1 with -Os the first version generates

0000000000000000 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)>:
   0:   41 56                   push   %r14
   2:   53                      push   %rbx
   3:   50                      push   %rax
   4:   48 89 f2                mov    %rsi,%rdx
   7:   49 89 fe                mov    %rdi,%r14
   a:   48 89 e3                mov    %rsp,%rbx
   d:   48 89 df                mov    %rbx,%rdi
  10:   4c 89 f6                mov    %r14,%rsi
  13:   e8 00 00 00 00          callq  18 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x18>
  18:   48 8b 1b                mov    (%rbx),%rbx
  1b:   4c 89 f7                mov    %r14,%rdi
  1e:   e8 00 00 00 00          callq  23 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x23>
  23:   48 39 c3                cmp    %rax,%rbx
  26:   74 16                   je     3e <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x3e>
  28:   66 83 7b 1e 03          cmpw   $0x3,0x1e(%rbx)
  2d:   75 2b                   jne    5a <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x5a>
  2f:   48 83 c3 10             add    $0x10,%rbx
  33:   48 89 d8                mov    %rbx,%rax
  36:   48 83 c4 08             add    $0x8,%rsp
  3a:   5b                      pop    %rbx
  3b:   41 5e                   pop    %r14
  3d:   c3                      retq   
  3e:   bf 10 00 00 00          mov    $0x10,%edi
  43:   e8 00 00 00 00          callq  48 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x48>
  48:   48 89 c3                mov    %rax,%rbx
  4b:   be 00 00 00 00          mov    $0x0,%esi
  50:   48 89 c7                mov    %rax,%rdi
  53:   e8 00 00 00 00          callq  58 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x58>
  58:   eb 1a                   jmp    74 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x74>
  5a:   bf 10 00 00 00          mov    $0x10,%edi
  5f:   e8 00 00 00 00          callq  64 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x64>
  64:   48 89 c3                mov    %rax,%rbx
  67:   be 00 00 00 00          mov    $0x0,%esi
  6c:   48 89 c7                mov    %rax,%rdi
  6f:   e8 00 00 00 00          callq  74 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x74>
  74:   be 00 00 00 00          mov    $0x0,%esi
  79:   ba 00 00 00 00          mov    $0x0,%edx
  7e:   48 89 df                mov    %rbx,%rdi
  81:   e8 00 00 00 00          callq  86 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x86>
  86:   eb 00                   jmp    88 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x88>
  88:   49 89 c6                mov    %rax,%r14
  8b:   48 89 df                mov    %rbx,%rdi
  8e:   e8 00 00 00 00          callq  93 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x93>
  93:   4c 89 f7                mov    %r14,%rdi
  96:   e8 00 00 00 00          callq  9b <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&, char const*)>

000000000000009b <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&, char const*)>:
  9b:   41 56                   push   %r14
  9d:   53                      push   %rbx
  9e:   50                      push   %rax
  9f:   48 89 f2                mov    %rsi,%rdx
  a2:   48 89 fb                mov    %rdi,%rbx
  a5:   48 89 e7                mov    %rsp,%rdi
  a8:   48 89 de                mov    %rbx,%rsi
  ab:   e8 00 00 00 00          callq  b0 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&, char const*)+0x15>
  b0:   66 83 7b 0e 03          cmpw   $0x3,0xe(%rbx)
  b5:   75 33                   jne    ea <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&, char const*)+0x4f>
  b7:   48 b8 ff ff ff ff ff    movabs $0xffffffffffff,%rax
  be:   ff 00 00 
  c1:   48 23 43 08             and    0x8(%rbx),%rax
  c5:   8b 0b                   mov    (%rbx),%ecx
  c7:   48 c1 e1 05             shl    $0x5,%rcx
  cb:   48 01 c1                add    %rax,%rcx
  ce:   48 8b 04 24             mov    (%rsp),%rax
  d2:   48 39 c8                cmp    %rcx,%rax
  d5:   74 2c                   je     103 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&, char const*)+0x68>
  d7:   66 83 78 1e 03          cmpw   $0x3,0x1e(%rax)
  dc:   75 41                   jne    11f <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&, char const*)+0x84>
  de:   48 83 c0 10             add    $0x10,%rax
  e2:   48 83 c4 08             add    $0x8,%rsp
  e6:   5b                      pop    %rbx
  e7:   41 5e                   pop    %r14
  e9:   c3                      retq   
  ea:   bf 00 00 00 00          mov    $0x0,%edi
  ef:   be 00 00 00 00          mov    $0x0,%esi
  f4:   b9 00 00 00 00          mov    $0x0,%ecx
  f9:   ba 3c 04 00 00          mov    $0x43c,%edx
  fe:   e8 00 00 00 00          callq  103 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&, char const*)+0x68>
 103:   bf 10 00 00 00          mov    $0x10,%edi
 108:   e8 00 00 00 00          callq  10d <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&, char const*)+0x72>
 10d:   48 89 c3                mov    %rax,%rbx
 110:   be 00 00 00 00          mov    $0x0,%esi
 115:   48 89 c7                mov    %rax,%rdi
 118:   e8 00 00 00 00          callq  11d <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&, char const*)+0x82>
 11d:   eb 1a                   jmp    139 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&, char const*)+0x9e>
 11f:   bf 10 00 00 00          mov    $0x10,%edi
 124:   e8 00 00 00 00          callq  129 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&, char const*)+0x8e>
 129:   48 89 c3                mov    %rax,%rbx
 12c:   be 00 00 00 00          mov    $0x0,%esi
 131:   48 89 c7                mov    %rax,%rdi
 134:   e8 00 00 00 00          callq  139 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&, char const*)+0x9e>
 139:   be 00 00 00 00          mov    $0x0,%esi
 13e:   ba 00 00 00 00          mov    $0x0,%edx
 143:   48 89 df                mov    %rbx,%rdi
 146:   e8 00 00 00 00          callq  14b <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&, char const*)+0xb0>
 14b:   eb 00                   jmp    14d <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&, char const*)+0xb2>
 14d:   49 89 c6                mov    %rax,%r14
 150:   48 89 df                mov    %rbx,%rdi
 153:   e8 00 00 00 00          callq  158 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&, char const*)+0xbd>
 158:   4c 89 f7                mov    %r14,%rdi
 15b:   e8 00 00 00 00          callq  160 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&, char const*)+0xc5>

and the second version

0000000000000000 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)>:
   0:   41 56                   push   %r14
   2:   53                      push   %rbx
   3:   50                      push   %rax
   4:   48 89 f2                mov    %rsi,%rdx
   7:   49 89 fe                mov    %rdi,%r14
   a:   48 89 e3                mov    %rsp,%rbx
   d:   48 89 df                mov    %rbx,%rdi
  10:   4c 89 f6                mov    %r14,%rsi
  13:   e8 00 00 00 00          callq  18 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x18>
  18:   48 8b 1b                mov    (%rbx),%rbx
  1b:   4c 89 f7                mov    %r14,%rdi
  1e:   e8 00 00 00 00          callq  23 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x23>
  23:   48 39 c3                cmp    %rax,%rbx
  26:   74 16                   je     3e <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x3e>
  28:   66 83 7b 1e 03          cmpw   $0x3,0x1e(%rbx)
  2d:   75 2b                   jne    5a <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x5a>
  2f:   48 83 c3 10             add    $0x10,%rbx
  33:   48 89 d8                mov    %rbx,%rax
  36:   48 83 c4 08             add    $0x8,%rsp
  3a:   5b                      pop    %rbx
  3b:   41 5e                   pop    %r14
  3d:   c3                      retq   
  3e:   bf 10 00 00 00          mov    $0x10,%edi
  43:   e8 00 00 00 00          callq  48 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x48>
  48:   48 89 c3                mov    %rax,%rbx
  4b:   be 00 00 00 00          mov    $0x0,%esi
  50:   48 89 c7                mov    %rax,%rdi
  53:   e8 00 00 00 00          callq  58 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x58>
  58:   eb 1a                   jmp    74 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x74>
  5a:   bf 10 00 00 00          mov    $0x10,%edi
  5f:   e8 00 00 00 00          callq  64 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x64>
  64:   48 89 c3                mov    %rax,%rbx
  67:   be 00 00 00 00          mov    $0x0,%esi
  6c:   48 89 c7                mov    %rax,%rdi
  6f:   e8 00 00 00 00          callq  74 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x74>
  74:   be 00 00 00 00          mov    $0x0,%esi
  79:   ba 00 00 00 00          mov    $0x0,%edx
  7e:   48 89 df                mov    %rbx,%rdi
  81:   e8 00 00 00 00          callq  86 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x86>
  86:   eb 00                   jmp    88 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x88>
  88:   49 89 c6                mov    %rax,%r14
  8b:   48 89 df                mov    %rbx,%rdi
  8e:   e8 00 00 00 00          callq  93 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)+0x93>
  93:   4c 89 f7                mov    %r14,%rdi
  96:   e8 00 00 00 00          callq  9b <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&, char const*)>

000000000000009b <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> >&, char const*)>:
  9b:   e9 60 ff ff ff          jmpq   0 <get_object_member(rapidjson::GenericValue<rapidjson::UTF8<char>, rapidjson::MemoryPoolAllocator<rapidjson::CrtAllocator> > const&, char const*)>

The text section of the version with const_cast is 160 bytes long, the version with templates 346 bytes long. So more than double, a 116.25% bigger.
Sure, there is a little bit of non-dependent work I could move away from the template... getting the thing a little bit closer to the messy level of the const_cast version.

And no, I don't care about the difference. Had I not opened this issue I would have never looked at it in so much detail. I am actually going to stay with the templated version. But reading ES.50 I get the impression they are equivalent. So:
a) There is any third alternative?
b) Should the guideline (which sure, it's not a "rule" and says "prefer" not "DO") mention the possible size increase?

RedDwarf69 picture RedDwarf69 on 13 Mar 2020
👎1

Editors call: We will add a note about the code bloat, near get_bar_impl.

hsutter picture hsutter on 30 Jul 2020
Was this page helpful?
0 / 5 - 0 ratings

Related issues

imre-palik picture imre-palik  路  6Comments
Artikash picture Artikash  路  5Comments
HowardHinnant picture HowardHinnant  路  3Comments
larsch picture larsch  路  4Comments
qalpaq picture qalpaq  路  5Comments