5

Background

I'm trying to create two instances of a struct. One of them will not change and is therefore declared const, but the other may be changed asynchronously, therefore I'd like to make it volatile.

Problem

I'm trying to use the const instance of the structure to initialise the volatile one. However, if I use the volatile keyword the compiler throws this error:

passing 'volatile rect' as 'this' argument of 'rect& rect::operator=(rect&&)' discards qualifiers [-fpermissive]at line 15 col 8

Reproducible example

#include <Arduino.h>

struct rect {
    int x0;
    int y0;
    int width;
    int height;
};

const    rect outer    = {0, 0, 10, 5};
volatile rect inner;

void setup() {
    inner = {outer.x0 + 1, outer.y0 + 1,
             outer.width - 2, outer.height - 2};
}

void loop() {
    ;
}

Omitting volatile compiles fine:

rect inner = {outer.x0 + 1, outer.y0 + 1,
                            outer.width - 2, outer.height - 2};

Initialising one-by-one also works, but this is exactly what I'm trying to avoid:

inner.x0        = outer.x0 + 1;
inner.y0        = outer.y0 + 1;
inner.width     = 0;
inner.height    = outer.height - 2;

Question

What am I missing? ... It may be related to this.

Community
  • 1
  • 1
pfabri
  • 885
  • 1
  • 9
  • 25
  • The compiler output looks like you are compiling C++, why is it tagged as C ? – frostblue Dec 27 '16 at 15:05
  • I'm developing for the Arduino embedded platform, which is a mixture of C and C++. Unfortunately, I'm not always sure which side of the fence my code falls when there is an error. Would `C++` be more appropriate? – pfabri Dec 27 '16 at 15:07
  • Yes, `operator=` is not valid in C. – frostblue Dec 27 '16 at 15:08
  • This is clearly C++ but it is tagged C. – Clifford Dec 27 '16 at 15:08
  • Thanks, tags changed. `C` removed, `C++` and `Arduino` added. – pfabri Dec 27 '16 at 15:10
  • @pfabri : Arduino is C++ - even if you write code that is valid C code, if you use a C++ compiler to build it, it *is* C++. Moreover this code is not valid C code in any case. – Clifford Dec 27 '16 at 15:10
  • @Clifford Thanks, I thought it was some kind of a hybrid compiler. And I thought it was valid C code, as both `struct` and `volatile` exists in C. Could you please tell me why it is NOT valid C code? – pfabri Dec 27 '16 at 15:14
  • @pfabri you're using a C++ compiler, it doesn't matter whether the code coincidentally would be valid C code – M.M Dec 27 '16 at 15:15
  • I just put your code fragment into Arduino, and it raised no error. You need to include the code to which the error message refers (the line number is in the *complete* message). – Clifford Dec 27 '16 at 15:25
  • @pfabri : In C`struct` does not declare a type name. so `rect` on its own is not valid, it must be `struct rect` or use a `typedef`. Also C does not support operator overloading, so would not produce that error message. – Clifford Dec 27 '16 at 15:28
  • @M.M Updated, it is now fully reproducible. You're spot on, I am doing exactly that. Please see the exact context in the updated repex. (I was just trying to write a simple snippet, as the real code throwing the error is quite long.) – pfabri Dec 27 '16 at 15:29
  • @clifford Please see the updated repex. Thanks for your explanation! – pfabri Dec 27 '16 at 15:29
  • The reason there's no implicitly-generated `operator=` for updating a volatile struct, is because there might be a write from the ISR during that operation and that would mess up the operation. To answer this question depends on what the normal procedure is for Arduino to avoid that problem. In normal multithreaded programming the best solution would be not to make `inner` volatile, but have access to it gated by a memory fence that the compiler understands. – M.M Dec 27 '16 at 20:11
  • @M.M so the *"solution"* would be to implement `operator=` for a `volatile rect` which temporarily *disables/re-enables* interrupts? if so, please post an answer with the code example, the current one seems to be too generic. – Patrick Trentin Dec 28 '16 at 06:09
  • @PatrickTrentin I'm not familiar with Arduino interrupt coding, I can't be any more specific than what I've written already. As mentioned already, my preferred solution in general would be not to use a `volatile rect` at all, instead use a memory fence. – M.M Dec 28 '16 at 06:11
  • @M.M I'm not familiar with *memory fences* either. According to [this](http://blog.regehr.org/archives/28) one would use an instruction like `asm volatile ("" : : : "memory");`, which compiles on *Arduino* though I can't tell whether it is effective, as *memory fence* to *prevent the re-ordering of assignments to volatile* memory locations, but one would also need to use `noInterrupts(); ... interrupts();` if the problem is given by incoming *ISR* as you state. However, if i am not mistaken, unless @pfabri explicitly writes an *ISR* that uses `inner` that might not be the case. – Patrick Trentin Dec 28 '16 at 06:24
  • @PatrickTrentin typically you'd use a synchronization primitive rather than asm volatile but that is an option. There must be articles about this for Arduino somewhere. – M.M Dec 28 '16 at 06:25
  • @M.M please correct me if am mistaken, given that `rect` is an object, the instruction `inner = { ... }` is compiled into a function call to `operator =`, so there shouldn't be any re-ordering around `inner = { ... }` in the first place, because a function call is a *sequence point*. Thus it should be sufficient to *disable/re-enable* interrupts. – Patrick Trentin Dec 28 '16 at 06:39
  • @PatrickTrentin There can be reordering past sequence points for non-volatile variables. Sequence points are part of the language definition of ordering, but the compiler can emit anything so long as the end result matches the language definition's end result – M.M Dec 28 '16 at 07:29
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/131663/discussion-between-patrick-trentin-and-m-m). – Patrick Trentin Dec 28 '16 at 07:52

2 Answers2

2

You are missing what the error message actually says. The compiler is telling you that to assign the whole structure in one go needs a "copy constructor" which understands the volatile qualifier and there isn't one. Have a look at this answer for a discussion of what the error means.

But when you assign the individual elements of the structure one by one no copy constructor is needed and so the code works fine. Why are you "trying to avoid" this?

What you are expecting the volatile qualifier to do? In C/C++ it ONLY prevents the compiler from optimising away your variables or the code which uses them. Nothing more.

It wouldn't be useful to define a stock copy constructor for volatile structures, since your concurrency requirements will differ from everyone else's.

To guarantee that your structure elements are assigned consistently, you may need to disable interrupts, like this:

cli();
inner.x0        = outer.x0 + 1;
inner.y0        = outer.y0 + 1;
inner.width     = 0;
inner.height    = outer.height - 2;
sei();

But you'll have to analyse precisely what you need and that's heading off topic.

Community
  • 1
  • 1
  • `1.)` I was trying to avoid it purely to save space in my code. `2.)` I need `volatile` because the variable will me modified by an ISR, and therefore I must ensure that it won't be cached or optimised otherwise by the compiler. `3.)` In this specific case, the initialisation is done _before_ the interrupt is attached, but it's a good suggestion to keep in mind. Later on only `width` is accessed by the ISR. Many thanks! – pfabri Dec 29 '16 at 00:11
0

What am I missing? ... It may be related to this.

Yes, you're missing the proper copy c'tor for volatile

datafiddler
  • 1,755
  • 3
  • 17
  • 30