8

If you turn up your warning level to -Wall, GCC 8 (at least g++ (Ubuntu 8.3.0-6ubuntu1~18.10.1) 8.3.0 with --std=c++17) gives -Wclass-memaccess on:

#ifdef __cplusplus
   #include <type_traits>
#endif
#include <string.h>

struct CantCopy {
    int i;

  #ifdef __cplusplus
    CantCopy () = default;
    CantCopy (const CantCopy &other) = delete;
    void operator= (const CantCopy &rhs) = delete;
  #endif
};

int main(int argc, char *argv[]) {

  #ifdef __cplusplus
    static_assert(std::is_pod<CantCopy>::value);
  #endif

    struct CantCopy src;
    src.i = 1020;

    struct CantCopy dest;
    memcpy(&dest, &src, sizeof(struct CantCopy));

    return 0;
}

The warning says:

warning: ‘void* memcpy(void*, const void*, size_t)’ writing to an object of type ‘struct CantCopy’ with no trivial copy-assignment [-Wclass-memaccess]

memcpy(&dest, &src, sizeof(CantCopy));

What I have is code that's intended to build as either C or C++. But the C++ build is supposed to be able to statically check when C programmers are going through direct assignments of the structure--when they should be using a function. Hence disabling copying.

But it's still POD, and all this does was delete the operators. I can see the scenario this warning is designed to help with, but this isn't that situation.

Is there any easy way to do something equivalent to this without needing to disable the warning?

  • 6
    Try typecasting the addresses to `char*`: `memcpy((char*)&dest, (char*)&src, sizeof(struct CantCopy)); ` – Remy Lebeau Aug 30 '19 at 05:53
  • Which version of C++ are you compiling with? Is it C++17? – L. F. Aug 30 '19 at 05:59
  • 1
    @RemyLebeau That seemed to do the trick! Assuming it's not shady and breaking some other hidden rule to do so, that seems an answer to me, if you want to migrate the comment to a post... – HostileFork says dont trust SE Aug 30 '19 at 06:01
  • @HostileFork Because I cannot imagine that this is a POD ... It seems to violate [class]/6.2 https://timsong-cpp.github.io/cppwp/n4659/class#6.2 – L. F. Aug 30 '19 at 06:03
  • @L.F. Er...well, I dunno. I was under the impression that having the constructor `= default;` made it one...? :-/ In any case, the only intent here is just to stop the type from being copied literally bytewise, outside of a few very specific and narrow safe points to do so (where memcpy is used). If there is a more-standards-compliant-recipe for this intent, I'm all ears...just trying to get past a new warning that showed up. (!) – HostileFork says dont trust SE Aug 30 '19 at 06:10
  • So if I understand correctly, you want to allow `memcpy(&dest, &src, sizeof(struct CantCopy))`, but prohibit `dest = src`? Because I don't see the difference between them – L. F. Aug 30 '19 at 06:14
  • @L.F. For the type in question, *most* of the time it is not safe to blit the bytes. The typical C client is supposed to call `Move_Value(&cantcopy_dest, &cantcopy_src)`, and there are some tweaks that happen during that movement (based on bit patterns at the destination location, which carries some formatting indicating what kind of destination it is, so the copy is sensitive to it). A very few core routines actually do want to blit the bytes because they know they can, and they are the ones using memcpy(). – HostileFork says dont trust SE Aug 30 '19 at 06:22
  • Well, if that is the case, is providing a separate copy function an option? – L. F. Aug 30 '19 at 06:25
  • It becomes a bit of a pain to go through a copy because of embedding; it's not just about this type, but where the type appears in other structures. :-/ I'd be happiest if I can avoid creating parallel `#ifdef __cplusplus` code that calls methods, which then degrades to plain assignments in the C build. – HostileFork says dont trust SE Aug 30 '19 at 06:27
  • @HostileFork "*The typical C client is supposed to call `Move_Value(&cantcopy_dest, &cantcopy_src)`*" - can't be helped in C, but in C++ you could add a move constructor and move assignment operator which both call `Move_Value()`. – Remy Lebeau Aug 30 '19 at 14:34

1 Answers1

1

It seems that you could achieve your aim to make assignments and copies in C++ outside of a designated function impossible better by making the assignment operator and copy constructor default and private and adding your Move_Value(&cantcopy_dest, &cantcopy_src) as a friend.

struct CantCopy;
#ifdef __cplusplus
extern "C" {
#endif
void Move_Value(CantCopy *cantcopy_dest, const CantCopy *cantcopy_src);
#ifdef __cplusplus
} // extern "C"
#endif


struct CantCopy {
    int i;

#ifdef __cplusplus
    CantCopy () = default;
  private:
    CantCopy (const CantCopy &other) = default;
    constexpr CantCopy &operator= (const CantCopy &rhs) = default;

    friend void Move_Value(CantCopy *cantcopy_dest, const CantCopy *cantcopy_src);
#endif
};

struct Other
{
  CantCopy c;
};

void test()
{
    CantCopy c1;
    CantCopy c2;
    // fails: c3{c1};
    // fails: c2 = c1;
    Move_Value(&c1, &c2);
    Other o;
    Other o2;
    // also fails: o = o2;
}

void Move_Value(CantCopy *cantcopy_dest, const CantCopy *cantcopy_src)
{
    // do special stuff
    *cantcopy_dest = *cantcopy_src;
}
PaulR
  • 3,587
  • 14
  • 24
  • Hey, that seems to work as well (e.g. still says std::is_pod, beats the memcpy warning...) From a sheer simplicity point of view, the char* cast seems less likely to scare the C programmers reading the code, but if there's any practical problems with that this looks like a good alternative! – HostileFork says dont trust SE Sep 02 '19 at 19:09