Skip to content

Incorrect expansion of ,##__VA_ARGS__ in macro #4639

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wmacevoy opened this issue Apr 12, 2018 · 3 comments · Fixed by #6339
Closed

Incorrect expansion of ,##__VA_ARGS__ in macro #4639

wmacevoy opened this issue Apr 12, 2018 · 3 comments · Fixed by #6339

Comments

@wmacevoy
Copy link

wmacevoy commented Apr 12, 2018

Basic Infos

  • [ X] This issue complies with the issue POLICY doc.
  • [X ] I have read the documentation at readthedocs and the issue is not addressed there.
  • [ ?] I have tested that the issue is present in current master branch (aka latest git).
    Latest build from README docs build failed (Mac OS 10.12.6) on python get.py with:
    > cd esp8266/tools
    > python get.py
    IOError: [Errno socket error] [SSL: TLSV1_ALERT_PROTOCOL_VERSION] tlsv1 alert protocol version (_ssl.c:590)
  • [ X] I have searched the issue tracker for a similar issue.
  • [ X] If there is a stack dump, I have decoded it.
  • [X ] I have filled out all fields below.

Platform

  • Hardware: Generic ESP8266 device
  • Core Version: 2.4.1 - could not build from git
  • Development Env: Arduino IDE 1.8.5
  • Operating System: MacOS

Settings in IDE

  • Module: Generic ESP8266 Module
  • Flash Mode: qio
  • Flash Size: 512MB
  • lwip Variant: v2mss536
  • Reset Method: ck
  • Flash Frequency: 40Mhz
  • CPU Frequency: 80Mhz
  • Upload Using: na compile time error
  • Upload Speed: na compile time error

Problem Description

g++ supports variadic macros which cleanly reduces to 0 arguments with the notation

#define va_macro(...) va(begin, ##__VA_ARGS__, end)

The ,##__VA_ARGS__ notation expands to nothing if no arguments are given (the leading comma is removed), while the comma remains for 1 or more arguments.

This works correctly on uno, mega and esp32 boards, but fails to expand correctly on esp8266. This notation is important for variable parameter macros. Here is a sketch that compiles under uno, mega, esp32, but has a compile-time-error for esp8266, with log files for uno, mega, esp32 (correct) and esp8266 (compile time error):

https://github.com/mmurdoch/arduinounit/tree/pr65/va_args

#define va_macro(...) va_sub(-9,##__VA_ARGS__,9)

// Unimportant, just to have something the va_macro can compile to
void va_sub(int first, int last) { }
void va_sub(int first, int va1, int last) { }
void va_sub(int first, int va1, int va2, int last) {}

void setup() {
   va_macro(1,2);  // ok: expands to va_sub(-9,1,2,9);
   va_macro(1); // ok: expands to va_sub(-9,1,9); 
   va_macro(); // fail: expands to va_sub(-9,,9) on ESP 8266, but should be va_sub(-9,9);
}

void loop() {

}

Debug Messages

None.  Does not compile.
@igrr
Copy link
Member

igrr commented Apr 13, 2018

IIRc, this is a GNU extension. In this project C++ code is compiled with -std=c++11, so GNU extensions are disabled.

@earlephilhower
Copy link
Collaborator

Yup, that'd be the cause. Here is G++ 5.4 under Linux, same behavior.

earle@server:~/Arduino/ss1$ g++ -c /tmp/a.c 
<no error>

earle@server:~/Arduino/ss1$ g++ -c --std=c++11 /tmp/a.c 
/tmp/a.c: In function ‘void setup()’:
/tmp/a.c:1:46: error: expected primary-expression before ‘,’ token
 #define va_macro(...) va_sub(-9,##__VA_ARGS__,9)
                                              ^
/tmp/a.c:14:4: note: in expansion of macro ‘va_macro’
    va_macro(); // compile time error just on ESP 8266
    ^

@wmacevoy
Copy link
Author

It is a GNU extension, but AVR and ESP32 boards are compiled with those extensions enabled. And I can find no resolution to this without those options. This breaks ArduinoUnit on the ESP 8266. c++20 seems to finally have a fix; but until then the standard is broken. (see VA_OPT in https://en.cppreference.com/w/cpp/compiler_support). How horrible is it to compile with -std=gnu++11, which is what the other systems ArduinoUnit supports does? See https://github.com/mmurdoch/arduinounit/blob/pr65/README.md#known-bugs

earlephilhower added a commit to earlephilhower/Arduino that referenced this issue Jul 24, 2019
Looking at other projects, they're all building using gnu extensions
(-std=gnu++11).  We're already embedding gnu-specific pragmas for
compile options, so I see no reason not to fall in line w/others on
this.

Fixes esp8266#4639
d-a-v pushed a commit to d-a-v/Arduino that referenced this issue Jul 25, 2019
Looking at other projects, they're all building using gnu extensions
(-std=gnu++11).  We're already embedding gnu-specific pragmas for
compile options, so I see no reason not to fall in line w/others on
this.

Fixes esp8266#4639
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants