isocpp / CppCoreGuidelines

The C++ Core Guidelines are a set of tried-and-true guidelines, rules, and best practices about coding in C++
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines
Other
42.8k stars 5.44k forks source link

C.183 recommend bit_cast for type punning #1987

Open mark0n opened 2 years ago

mark0n commented 2 years ago

C.183 says that unions shouldn't be used for type punning but IMHO the guidance it provides on what to use instead leaves room for improvement. It mentions reinterpret_cast for casting to char*, unsigned char*, or std::byte. AFAIK we need to keep the following things in mind when punning types:

  1. reinterpret_cast<bar>(foo) is UB if bar has stricter alignment requirements than the type of foo.
  2. reinterpret_casting a pointer to a std::byte buffer to gadget* and then using this pointer to call member functions/access member variables is UB since no instance of gadget has been instantiated (object lifetime).
  3. In general reinterpret_cast might lead to UB due to aliasing ([example demonstrating this](https://godbolt.org/#g:!((g:!((g:!((h:codeEditor,i:(filename:'1',fontScale:14,fontUsePx:'0',j:1,lang:c%2B%2B,selection:(endColumn:14,endLineNumber:17,positionColumn:14,positionLineNumber:17,selectionStartColumn:14,selectionStartLineNumber:17,startColumn:14,startLineNumber:17),source:'void+foo(int+*+x,+float+*+y)%0A%7B%0A++++//+Depending+on+the+level+of+optimization,+GCC+might+or+might+not+choose+to+reorder%0A++++//+the+following+two+lines+and+squash+the+two+assignments+to+x+into+a+single+assignment.%0A++++//+This+is+in+line+with+the+C%2B%2B+standard+since+x+and+y+are+of+different+type+so%0A++++//+the+assumption+is+that+y+cannot+be+aliasing+x.%0A++++*x+%3D+1%3B%0A++++*y+%3D+2.f%3B%0A++++*x+%2B%3D+static_cast%3Cint%3E(*y)%3B%0A%7D%0A%0Aint+main()+%7B%0A++++int+a+%3D+0%3B%0A%0A++++foo(%26a,+reinterpret_cast%3Cfloat*%3E(%26a))%3B%0A%0A++++return+a%3B%0A%7D'),l:'5',n:'0',o:'C%2B%2B+source+%231',t:'0')),k:33.333333333333336,l:'4',m:100,n:'0',o:'',s:0,t:'0'),(g:!((h:compiler,i:(compiler:gsnapshot,filters:(b:'0',binary:'1',commentOnly:'0',demangle:'0',directives:'0',execute:'0',intel:'0',libraryCode:'0',trim:'1'),flagsViewOpen:'1',fontScale:14,fontUsePx:'0',j:1,lang:c%2B%2B,libs:!(),options:'-O3',selection:(endColumn:1,endLineNumber:1,positionColumn:1,positionLineNumber:1,selectionStartColumn:1,selectionStartLineNumber:1,startColumn:1,startLineNumber:1),source:1),l:'5',n:'0',o:'+x86-64+gcc+(trunk)+(Editor+%231)',t:'0')),k:33.333333333333336,l:'4',n:'0',o:'',s:0,t:'0'),(g:!((h:output,i:(editorid:1,fontScale:14,fontUsePx:'0',j:1,wrap:'1'),l:'5',n:'0',o:'Output+of+x86-64+gcc+(trunk)+(Compiler+%231)',t:'0')),k:33.33333333333333,l:'4',n:'0',o:'',s:0,t:'0')),l:'2',n:'0',o:'',t:'0')),version:4)).

I guess it would be better to suggest a way which is safe with respect to all three points. In particular I would like to suggest replacing the reinterpret_cast paragraph by something along the following lines:

Use std::bit_cast (introduced with C++20) for type punning. If your standard library doesn't support std::bit_cast, yet, use std::memcpy instead. This is preferred over using union or reinterpret_cast since bitcast and memcpy reliably prevent undefined behavior due to violated alignment/aliasing rules. In practice modern compilers are often able to eliminate the copy.

Example:
    // Assume sizeof(int) == sizeof(float) for these examples

    void if_you_must_pun(float& x)
    {
        auto i = std::bit_cast<int>(x);
        cout << i << '\n';
    }

    void if_you_must_pun_pre_cpp20(float& x)
    {
        int i;
        memcpy(&i, &x, sizeof(x));
        cout << i << '\n';
    }
BjarneStroustrup commented 1 year ago

I think this should be left to a C++20 sweep of the whole CG.