3

When one assigns to a volatile int from a non-volatile int, the compiler. When one assigns from a volatile struct from a non-volatile struct of the same type, the compiler appears to be extremely unhappy.

Consider the following simple program.

struct Bar {
    int a;
};

volatile int foo;
int foo2;

volatile Bar bar;
Bar bar2;

int main(){
    foo = foo2;
    bar = bar2;
}

When I try to compile this code, I get an error on the second line of main but not the first.

g++     Main.cc   -o Main
Main.cc: In function ‘int main()’:
Main.cc:13:9: error: passing ‘volatile Bar’ as ‘this’ argument discards qualifiers [-fpermissive]
     bar = bar2;
         ^
Main.cc:1:8: note:   in call to ‘Bar& Bar::operator=(const Bar&)’
 struct Bar {

It seems that the problem occurs because a volatile Bar is being passed to the left side of the assignment operator, although I am not sure why this is not a problem for int.

I looked at this answer which suggested the following fix.

struct Bar {
    int a;
    volatile Bar& operator= (const Bar& other) volatile {
       *this = other; 
    }
};

Unfortunately, this resulted in the following two warnings.

g++     Main.cc   -o Main
Main.cc: In member function ‘volatile Bar& Bar::operator=(const Bar&) volatile’:
Main.cc:4:21: warning: implicit dereference will not access object of type ‘volatile Bar’ in statement
        *this = other; 
                     ^
Main.cc: In function ‘int main()’:
Main.cc:16:15: warning: implicit dereference will not access object of type ‘volatile Bar’ in statement
     bar = bar2;

I then looked at this answer, which mentions that I should cast the reference to an rvalue, but I am not sure which reference to cast, and which cast syntax to use in this case.

What is the correct incantation to make the assignment on the line 2 of main behave exactly as line 1 of main does, without warnings or errors?

Community
  • 1
  • 1
merlin2011
  • 71,677
  • 44
  • 195
  • 329

2 Answers2

3

Your initial problem is because the implicit assignment operator has signature

Bar& operator=(const Bar& rhs);

... and that isn't callable for a volatile object. The warnings are because your updated function returns a volatile reference, but that reference is never used. GCC thinks that this might be a problem. The simplest way to fix this is to change the return type to void!

There is another problem: Your function will call itself in an infinite recursion. I suggest the following:

struct Bar {
    int a;
    Bar& operator=(const Bar&rhs) = default;
    void operator=(const volatile Bar& rhs) volatile // Note void return.
    {
         // Caution: This const_cast removes the volatile from
         // the reference.  This may lose the point of the original
         // volatile qualification.
         //
         // If this is a problem, use "a = rhs.a;" instead - but this
         // obviously doesn't generalize so well.
         const_cast<Bar&>(*this) = const_cast<const Bar&>(rhs);
    }
};

volatile Bar vbar;
Bar bar;

int main(){
    vbar = bar;  // All four combinations work.
    bar = vbar;
    vbar = vbar;
    bar = bar;
    return 0;
}

This means you won't be able to chain assignment operators when using volatile structs. I assert this is not a huge loss.

Final aside: Why are you using volatile - it's not very useful for multi-threaded code (it is useful for memory mapped IO).

0

@martin provides a great description of why the problem occurs, but then does not provide a general solution that doesn't cast away the volatile.

If casting away the volatile is okay, then it should probably be done outside the function before the assignment. Hiding it within the copy constructor leaves more potential of unforseen bugs creeping in.

Adjusting the solution a bit, gives the following more general solution:

struct Bar {
    int a;
    Bar& operator=(const Bar&rhs) = default;
    auto & operator=(const volatile Bar& rhs) volatile
    {
         // Solution 1: depends on struct definition and can't be copied to many
         static_assert(sizeof(*this) == sizeof(a), "struct elements added or changed. update below");
         a = rhs.a;

         // Solution 2: very general solution that could be used if copying to many structs
         // Do default byte-by-byte copy operation, but can't use memcpy for volatile
         static_assert(is_trivially_copy_assignable_v<remove_cvref_t<decltype(*this)>>>, "Byte-by-byte copy may not be possible with this class");
         std::copy_n(reinterpret_cast<const volatile std::byte*>(&rhs), sizeof(*this), reinterpret_cast<volatile std::byte*>(this));

         // Support chaining of assignments
         //return *this;  // Okay, but returning a volatile will generate spurious warnings if that value is not read afterword
         return rhs; // assumes that you want to just chain assignments and not do anything weird
    }
};

This version of copy constructor works for all 4 combos of volatile/non-volatile source and destination, however, you could also split into different combinations in case you want to allow optimizations for each case.

mattgately
  • 932
  • 8
  • 22