Request for member 'begin' in 'c', which is of non-class type 'TL(Led, Led, Led)'

[sterretje]

I'm trying to cleanup some code. While trying to do so step by step I encountered the error in the topic title.

The below code is the minimum example

class Led
{
private:
  uint8_t pin;
  uint8_t state;

public:
  Led(const uint8_t pin_)
  {
    pin = pin_;
    state = LOW;
  }

  void begin()
  {
    digitalWrite(pin, LOW);
    pinMode(pin, OUTPUT);
  }
};

class TL
{
private:
  Led ledR;
  Led ledG;
  Led ledY;

public:
  TL(const Led ledR_, const Led ledG_, const Led ledY_)
    : ledR(ledR_), ledG(ledG_), ledY(ledY_)
  {
  }

  void begin()
  {
    ledR.begin();
    ledG.begin();
    ledY.begin();
  }
};

class TL2
{
private:
  Led ledR;
  Led ledG;
  Led ledY;

public:
  TL2(const Led ledR_, const Led ledG_, const Led ledY_)
    : ledR(Led(ledR_)), ledG(Led(ledG_)), ledY(Led(ledY_))
  {
  }

  void begin()
  {
    ledR.begin();
    ledG.begin();
    ledY.begin();
  }
};


const uint8_t pinR = 13;
const uint8_t pinG = 12;
const uint8_t pinY = 11;

Led ledR(pinR);
Led ledG(pinG);
Led ledY(pinY);

TL a(ledR, ledG, ledY);
TL b(Led(13), Led(12), Led(11));
TL c(Led(pinR), Led(pinG), Led(pinY));
TL2 d(pinR, pinG, pinY);

void setup()
{
  // ok
  d.begin();
  // ok
  a.begin();
  // ok
  b.begin();
  // error
  c.begin();
}

void loop()
{
  // put your main code here, to run repeatedly:
}

1. a is the original.
2. c was the attempt to get rid of ledX variables.
3. Suspecting the pinX variables are the cause (I might be wrong) I did create b.
4. d1 (with class TL2) is what I eventually want to achieve.

So I have the solution but trying to the understand the error. I did try to search for the problem but most errors like this don't seem to match the above scenario so I'm at a loss.

Can somebody explain in layman's terms (!!) what goes wrong?

[J-M-L]

forget what I wrote (deleted it so no-one is confused)

on second though (after reading the actual compiler's message), what happens is that the compiler is confused when you write

@sterretje

TL c(Led(pinR), Led(pinG), Led(pinY));

and thinks it's a function forward definition.

use

TL a {ledR, ledG, ledY};
TL b {Led(13), Led(12), Led(11)};
TL c {Led(pinR), Led(pinG), Led(pinY)};
TL2 d {pinR, pinG, pinY};

and you should be fine.

Search for "the most vexing parse rule in C++" (Scott Meyers) for reference on what's happening.

[sterretje]

@J-M-L

forget what I wrote

I will.

@J-M-L

and thinks it's a function forward definition.

Your solution with the curly braces solved the issue. And I think that the research that I did pointed in the right direction, I just could not place it. What I found was referring to e.g. Led led() and that I had to get rid of the parenthesis.

Thanks for that; but I would expect another error message in the line of "pinR" does not name a type if the compiler did think that.

Now my question is if I always should use curly braces instead of parenthesis and if there is actually no place for parenthesis when calling a constructor?

Anyway, I think that that's the layman's answer that I was looking for

With regards to your "pass by value" comment in the deleted post I did try two codes to check for differences (in memory usage).

By value

class Led
{
private:
  uint8_t pin;
  uint8_t state;

public:
  Led(const uint8_t pin_)
    : pin(pin_)
  {
    //pin = pin_;
    state = LOW;
  }

  void begin()
  {
    digitalWrite(pin, LOW);
    pinMode(pin, OUTPUT);
  }

  void print()
  {
    Serial.print("pin = ");
    Serial.println(pin);
  }
};

class TL
{
private:
  Led ledR;
  Led ledG;
  Led ledY;

public:
  TL(const Led ledR_, const Led ledG_, const Led ledY_)
    : ledR(ledR_), ledG(ledG_), ledY(ledY_)
  {
  }

  void begin()
  {
    ledR.begin();
    ledG.begin();
    ledY.begin();
  }
  void print()
  {
    ledG.print();
    ledR.print();
    ledY.print();
  }
};


const uint8_t pinR = 13;
const uint8_t pinG = 12;
const uint8_t pinY = 11;

Led ledR(pinR);
Led ledG(pinG);
Led ledY(pinY);

TL a{ ledR, ledG, ledY };
TL b{ Led(13), Led(12), Led(11) };
TL c{ Led(pinR), Led(pinG), Led(pinY) };

void setup()
{
  // ok
  a.begin();
  // ok
  b.begin();
  // error
  c.begin();

  Serial.begin(115200);
  a.print();
}

void loop()
{
  // put your main code here, to run repeatedly:
}

Sketch uses 2124 bytes (6%) of program storage space. Maximum is 30720 bytes.
Global variables use 212 bytes (10%) of dynamic memory, leaving 1836 bytes for local variables. Maximum is 2048 bytes.

By reference

class Led
{
private:
  uint8_t pin;
  uint8_t state;

public:
  Led(const uint8_t &pin_)
    : pin(pin_)
  {
    //pin = pin_;
    state = LOW;
  }

  void begin()
  {
    digitalWrite(pin, LOW);
    pinMode(pin, OUTPUT);
  }

  void print()
  {
    Serial.print("pin = ");
    Serial.println(pin);
  }
};

class TL
{
private:
  Led ledR;
  Led ledG;
  Led ledY;

public:
  TL(const Led &ledR_, const Led &ledG_, const Led &ledY_)
    : ledR(ledR_), ledG(ledG_), ledY(ledY_)
  {
  }

  void begin()
  {
    ledR.begin();
    ledG.begin();
    ledY.begin();
  }
  void print()
  {
    ledG.print();
    ledR.print();
    ledY.print();
  }
};


const uint8_t pinR = 13;
const uint8_t pinG = 12;
const uint8_t pinY = 11;

Led ledR(pinR);
Led ledG(pinG);
Led ledY(pinY);

TL a{ ledR, ledG, ledY };
TL b{ Led(13), Led(12), Led(11) };
TL c{ Led(pinR), Led(pinG), Led(pinY) };

void setup()
{
  // ok
  a.begin();
  // ok
  b.begin();
  // error
  c.begin();

  Serial.begin(115200);
  a.print();
}

void loop()
{
  // put your main code here, to run repeatedly:
}

Sketch uses 2172 bytes (7%) of program storage space. Maximum is 30720 bytes.
Global variables use 218 bytes (10%) of dynamic memory, leaving 1830 bytes for local variables. Maximum is 2048 bytes.

I was actually surprised that "by reference" used more memory; but maybe the example is not concise enough to find improvements. Or I'm totally missing it which would be a question for another topic.

@J-M-L

Search for "the most vexing parse rule in C++" (Scott Meyers) for reference on what's happening.

I did, thanks

The expression was coined by Scott Meyers, who talks about it in details in Item 6 of Effective STL. It comes from a rule in C++ that says that anything that could be considered as a function declaration, the compiler should parse it as a function declaration. And even if such an expression could be interpreted as something else. And even if this something else would seems a lot more natural to a programmer.

[J-M-L]

@sterretje

Thanks for that; but I would expect another error message in the line of "pinR" does not name a type if the compiler did think that.

TL c(Led(pinR), Led(pinG), Led(pinY));
is legit because C++ grammar allows a statement of the form
Type Name(Type(...), Type(...), Type(...))
to be parsed as a function declaration, not an object construction.

Each Led(pinX) is interpreted as a parameter declarator where pinX is treated as the parameter name, not a variable or constructor argument.

The compiler does not evaluate the parentheses or check whether pinR exists; it only parses the syntax as a valid function signature returning TL with three Led parameters.

This is a classic example of the most vexing parse, where syntax that looks like object construction is actually a declaration.

That's where using braces, avoids the ambiguity and forces object construction.

@sterretje

Now my question is if I always should use curly braces instead of parenthesis and if there is actually no place for parenthesis when calling a constructor?

You could still use () to solve it using copy or direct initialization with = as it removes the most vexing parse ambiguity.

If you were to write

TL c = TL(Led(pinR), Led(pinG), Led(pinY));

the compiler clearly sees that you are defining a variable c of type TL and constructing it using the TL constructor. The parentheses are now interpreted as constructor arguments, not as a function parameter list, so pinR, pinG, and pinY are correctly used as values.

---

@sterretje

I was actually surprised that "by reference" used more memory; but maybe the example is not concise enough to find improvements. Or I'm totally missing it which would be a question for another topic.

In your example, the TL object still contains full copies of the Led objects as members.

Passing by reference avoids copying the argument when calling the constructor, but it does not avoid copying into the member variables the way you do it ( When you write : ledR(ledR_), ledG(ledG_), ledY(ledY_) in the initializer list, it makes a copy, each member ledR , ledG , ledY is constructed from the argument)

So you end up with both the temporary reference-handling overhead and the copied members.

You would need to make the member variables references themselves. For example, declare Led &ledR; Led &ledG; Led &ledY; in the class. Then the initializer list binds the constructor arguments to these references without making copies.

class TL {
  private:
    Led &ledR;
    Led &ledG;
    Led &ledY;

  public:
    TL(Led &ledR_, Led &ledG_, Led &ledY_) : ledR(ledR_), ledG(ledG_), ledY(ledY_) {}

};

With this, TL stores references to existing Led objects but you cannot pass temporaries like Led(pinR) because references cannot extend the lifetime of a temporary. You must pass actual objects like ledR, ledG, ledY.

[sterretje]

@J-M-L

Passing by reference avoids copying the argument when calling the constructor, but it does not avoid copying into the member variables the way you do it ( When you write : ledR(ledR_), ledG(ledG_), ledY(ledY_) in the initializer list, it makes a copy, each member ledR , ledG , ledY is constructed from the argument)

So you end up with both the temporary reference-handling overhead and the copied members.

Thanks, that explains that

@J-M-L

For example, declare Led &ledR; Led &ledG; Led &ledY; in the class.

Again thanks, for me that would make sense to do

@J-M-L

You must pass actual objects like ledR, ledG, ledY.

And that might be a disadvantage depending on what the expected outcome of the exercise is Less lines of code, memory usage, organisation of code etc.
It will be a fine balance.

Lots of stuff to learn.

[J-M-L]

yes, I try to have the instances only once in memory.

If you start having copies, then it's hard to keep track of what you do.

[van_der_decken]

I won't die today; I learned something new. I had never, ever (not hardly ever, but never, ever) run into that. Thank you for the "what the h..." moment followed by the "ahhh!!!!" moment!

[J-M-L]

you're welcome !

I don't think I had run into a "most vexing parse rule" example here ever.

[sterretje]

@van_der_decken

I won't die today

I'll appreciate that

@J-M-L

I don't think I had run into a "most vexing parse rule" example here ever.

There must be a first time for everything