Generate warning on use of copy constructor and assignment

Viewed 285

Is there a way to get a compiler (MSVC 2017 in particular here, but others may be interesting as well) to emit a warning at the locations where a particular class's copy constructor and copy-assignment operator are used (and in a way that can be explicitly suppressed at each call site, even when indirect)?

This question asks about producing compile errors, which is easy now with C++11 deleted methods, but I want the code to still compile, merely output warnings.

The reason is that I have a class which is currently being copied a great deal throughout the codebase. I don't want to prevent it being copied (some of these are necessary) but I do want to review each location to determine if it should be changed to a move or to pass-by-reference instead.

Letting the compiler temporarily flag usage of the constructor as a warning seemed like a great way to do this.

I tried adding something like this:

__declspec(deprecated) MyType(MyType const&) = default;

But this doesn't work; apparently = default wins over any other modifiers.

I did the same thing but fully implementing the method instead, and this almost works -- it produces C4996 at each call site, and I can review these and either change them or tack on a:

#pragma warning(suppress:4996)

if I'm happy that this is a required copy. (I eventually plan to remove these, along with the deprecation on the constructor -- this is just housekeeping to track which ones I haven't dealt with yet.)

Unfortunately there are some instances which I can't suppress this way, for example:

std::vector<MyType> list;
list.push_back(type);
list.emplace_back(MyType{ type });

Each of these lines raises warnings (the first because it's a field declaration inside a class with a regular copy constructor), but only the third can be directly suppressed here. The first two raise the warnings inside of <vector> and don't seem to be affected by warning suppress (or disable) at this line of code.

Is there some way to solve this, or some other way to do what I want?

2 Answers

It doesn't seem entirely elegant, but I was able to suppress the most prolifically noisy case (the vector member of a class in a header file) using the following tactic. The remaining cases stood out fairly well after that and didn't need to be individually suppressed; they could just be fixed or ignored.

  1. Add deprecated copy and non-deprecated move constructors and assignment:

    __declspec(deprecated) MyType(MyType const& o) { /* actual impl */ }
    __declspec(deprecated) MyType& operator=(MyType const& o) { /* impl */ return *this; }
    MyType(MyType&&) = default;
    MyType& operator=(MyType&&) = default;
    
  2. Add a placeholder non-deprecated forwarding type as well:

    struct SuppressMyType : MyType
    {
        using MyType::MyType;
    #pragma warning(disable:4996)
        SuppressMyType(SuppressMyType const& o) : MyType(o) {}
        SuppressMyType& operator=(SuppressMyType const& o)
            { MyType::operator=(static_cast<MyType const&>(o)); return *this; }
        SuppressMyType(MyType const& o) : MyType(o) {}
        operator MyType() const { return *this; }
    #pragma warning(default:4996)
    };
    
  3. Change places where the copy was intended to use SuppressMyType instead of MyType.

  4. Fix other places until the warnings go away.

  5. Replace all SuppressMyType back to MyType, then remove the code added in #1.

A bit convoluted, but it did the trick.

Thinking about it some more after doing it the other way, it might have been better (and easier) to do this the other way around:

  1. Mass-replace all MyType to WarnMyType (except the actual definition of MyType, of course).
  2. Add WarnMyType with deprecated constructors:

    struct WarnMyType : MyType
    {
        using MyType::MyType;
        __declspec(deprecated) WarnMyType(WarnMyType const& o) : MyType(o) {}
        __declspec(deprecated) WarnMyType& operator=(WarnMyType const& o)
            { MyType::operator=(static_cast<MyType const&>(o)); return *this; }
        WarnMyType(WarnMyType&&) = default;
        WarnMyType& operator=(WarnMyType&&) = default;
    };
    
  3. Gradually change uses of WarnMyType back to MyType as they're checked.

  4. Remove WarnMyType.
  5. Mass-replace any remaining uses of WarnMyType back to MyType (since #3 would only find those where a copy was performed).

It'd be nice if the tools made it as easy to find constructor/operator usage as they do for named method use...

Related