Skip to content
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

weird error in arduino.h on makeWord () with core 3.0.0 #8089

Closed
stef-ladefense opened this issue May 31, 2021 · 17 comments
Closed

weird error in arduino.h on makeWord () with core 3.0.0 #8089

stef-ladefense opened this issue May 31, 2021 · 17 comments

Comments

@stef-ladefense
Copy link

stef-ladefense commented May 31, 2021

hi,
i have a weird error in arduino.h on makeWord ().
I use word (h, l) in my code and with core 3.0.0 it gives a fatal error during compilation.

In file included from C:\Users\stephane\AppData\Local\Arduino15\packages\esp8266\hardware\esp8266\3.0.0\libraries\ESP8266WiFi\src/WiFiClient.h:25,
                 from C:\Users\stephane\AppData\Local\Arduino15\packages\esp8266\hardware\esp8266\3.0.0\libraries\ESP8266WiFi\src/ESP8266WiFi.h:39,

C:\Users\stephane\AppData\Local\Arduino15\packages\esp8266\hardware\esp8266\3.0.0\cores\esp8266/Arduino.h:247:19: error: 'uint16_t makeWord' redeclared as different kind of entity
  247 | uint16_t makeWord(byte h, byte l);
      |                   ^~~~
C:\Users\stephane\AppData\Local\Arduino15\packages\esp8266\hardware\esp8266\3.0.0\cores\esp8266/Arduino.h:246:10: note: previous declaration 'uint16_t makeWord(uint16_t)'
  246 | uint16_t makeWord(uint16_t w);
      |          ^~~~~~~~
C:\Users\stephane\AppData\Local\Arduino15\packages\esp8266\hardware\esp8266\3.0.0\cores\esp8266/Arduino.h:247:19: error: reference to 'byte' is ambiguous
  247 | uint16_t makeWord(byte h, byte l);
      |                   ^~~~

i got around the problem by changing in arduino.h

uint16_t makeWord (byte h, byte l); -> uint16_t makeWord (uint8_t h, uint8_t l);

What do you think ?

@d-a-v
Copy link
Collaborator

d-a-v commented May 31, 2021

Are you using a git installation, and in that case did you upgrade the toolchain with the script tools/get.py ?

(normally I should close because you deleted the issue template and the answers of my question would be in it after you filled the fields)

@stef-ladefense
Copy link
Author

hi,
no i use classic install with boards manager
(oh escuse me)

@d-a-v
Copy link
Collaborator

d-a-v commented May 31, 2021

We need a way to reproduce, with a MCVE.
@stef-ladefense can you provide one simple example that would allow us to reproduce, without your changes in Arduino.h ?

side note to maintainers:
the core need this change to match Arduino.h (to avoid undefined reference to '_Z8makeWordt')
(but that won't solve your issue @stef-ladefense)
(core is updated)

@dok-net
Copy link
Contributor

dok-net commented May 31, 2021

I've a few observations. The use of the identifier "byte" is a bit onerous. In Arduino.h, there is a typedef uint8_t byte;, whereas core_esp8266_si2c.cpp and Esp.cpp use byte as object identifier. On top of that, the signature in Arduino.h is not just different for makeWord(unsigned int), but also literally different for uint16_t makeWord(byte h, byte l); (Arduino.h) vs. unsigned int makeWord(unsigned char h, unsigned char l) (WMath.cpp).
According to https://en.cppreference.com/w/cpp/types/byte , this also was introduced as an official type in the std namespace with C++17.
As a typename, byte is hardly ever used in ESP8266 Core for Arduino (trivial to fix one-time slip being in core_esp8266_main.cpp). In libraries/ESP8266WiFiMesh/src/TypeConversionFunctions.cpp, it also is used in error, as libraries/ESP8266WiFiMesh/src/TypeConversionFunctions.h specifies the affected 2 functions using uint8_t.

I'm providing PR #8090 as a suggested fix to this, including @d-a-v's change.

@stef-ladefense
Copy link
Author

first I apologize for not following the issue template, but I did not imagine the benefit.
I'm happy with the outcome, it's going in the right direction
thank you all

@dok-net
Copy link
Contributor

dok-net commented May 31, 2021

With the PR, this compiles fine now:

byte h = 0;
byte l = 0;

void setup()
{
	Serial.begin(74880);
	delay(500);
	Serial.println("makeword");
	auto w1 = word(1025);
	h = w1 >> 8;
	l = w1 & 0xff;
	auto w2 = word(h, l);
	if (w1 == w2)
		Serial.println("OK");
	else
		Serial.println("oops");
}

void loop()
{
}

@earlephilhower Is this convincing proof that uint8_tin uint16_t makeWord(uint8_t h, uint8_t l) is non-breaking vs. Arduino AVR?

@d-a-v d-a-v added the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Jun 2, 2021
@dok-net
Copy link
Contributor

dok-net commented Jun 3, 2021

@d-a-v #8097 builds and works with byte, uint8_t, and unsigned char for the makeWord(h, l) arguments. I'm keeping #8090 for the time being, in case someone considers removing byte has any value of itself after all.

@earlephilhower
Copy link
Collaborator

Closing as no feedback and a merged PR which we believe fixes things.

@Sai-Kiran-31
Copy link

thank you all

Sir, How did you resolve the problem I too facing the same

@earlephilhower
Copy link
Collaborator

@Sai-Kiran-31 you need to either use the git master branch, or wait for 3.0.1 to be released to get the change incorporated.

@bill-orange
Copy link

bill-orange commented Jun 28, 2021

I re-tested with 3.0.1 and the noted error still exists. Thoughts? 2.7.4 works fine.

> C:\Users\William\AppData\Local\Arduino15\packages\esp8266\hardware\esp8266\3.0.1\cores\esp8266/Arduino.h:246:10: note: previous declaration 'uint16_t makeWord(uint16_t)'
> 
>   246 | uint16_t makeWord(uint16_t w);
> 
>       |          ^~~~~~~~
> 
> C:\Users\William\AppData\Local\Arduino15\packages\esp8266\hardware\esp8266\3.0.1\cores\esp8266/Arduino.h:247:19: error: reference to 'byte' is ambiguous
> 
>   247 | uint16_t makeWord(byte h, byte l);
> 
>       |                   ^~~~
> 
> In file included from c:\users\william\appdata\local\arduino15\packages\esp8266\tools\xtensa-lx106-elf-gcc\3.0.3-gcc10.3-9bcba0b\xtensa-lx106-elf\include\c++\10.3.0\bits\stl_algobase.h:61,
> 

Plenty more of the same followed

@stef-ladefense
Copy link
Author

stef-ladefense commented Jun 28, 2021

tested 3.00 > 3.01 and bug

fixed with change uint16_t makeWord(uint8_t h, uint8_t l); in arduino.h


In file included from C:\Users\stephane\Documents\Arduino\libraries\NtpClient-3.0.2-beta\src\NtpClientLib.h:59,
                 from C:\Users\stephane\Documents\Arduino\libraries\NtpClient-3.0.2-beta\src\NTPClientLib.cpp:32:
C:\Users\stephane\AppData\Local\Arduino15\packages\esp8266\hardware\esp8266\3.0.1\cores\esp8266/Arduino.h:247:19: error: 'uint16_t makeWord' redeclared as different kind of entity
  247 | uint16_t makeWord(byte h, byte l);
      |                   ^~~~
C:\Users\stephane\AppData\Local\Arduino15\packages\esp8266\hardware\esp8266\3.0.1\cores\esp8266/Arduino.h:246:10: note: previous declaration 'uint16_t makeWord(uint16_t)'
  246 | uint16_t makeWord(uint16_t w);
      |          ^~~~~~~~
C:\Users\stephane\AppData\Local\Arduino15\packages\esp8266\hardware\esp8266\3.0.1\cores\esp8266/Arduino.h:247:19: error: reference to 'byte' is ambiguous
  247 | uint16_t makeWord(byte h, byte l);
      |                   ^~~~
In file included from c:\users\stephane\appdata\local\arduino15\packages\esp8266\tools\xtensa-lx106-elf-gcc\3.0.3-gcc10.3-9bcba0b\xtensa-lx106-elf\include\c++\10.3.0\bits\stl_algobase.h:61,
                 from c:\users\stephane\appdata\local\arduino15\packages\esp8266\tools\xtensa-lx106-elf-gcc\3.0.3-gcc10.3-9bcba0b\xtensa-lx106-elf\include\c++\10.3.0\array:40,
                 from c:\users\stephane\appdata\local\arduino15\packages\esp8266\tools\xtensa-lx106-elf-gcc\3.0.3-gcc10.3-9bcba0b\xtensa-lx106-elf\include\c++\10.3.0\tuple:39,
                 from c:\users\stephane\appdata\local\arduino15\packages\esp8266\tools\xtensa-lx106-elf-gcc\3.0.3-gcc10.3-9bcba0b\xtensa-lx106-elf\include\c++\10.3.0\functional:54,
                 from C:\Users\stephane\Documents\Arduino\libraries\NtpClient-3.0.2-beta\src\NtpClientLib.h:44,
                 from C:\Users\stephane\Documents\Arduino\libraries\NtpClient-3.0.2-beta\src\NTPClientLib.cpp:32:
c:\users\stephane\appdata\local\arduino15\packages\esp8266\tools\xtensa-lx106-elf-gcc\3.0.3-gcc10.3-9bcba0b\xtensa-lx106-elf\include\c++\10.3.0\bits\cpp_type_traits.h:404:30: note: candidates are: 'enum class std::byte'
  404 |   enum class byte : unsigned char;
      |                              ^~~~
In file included from C:\Users\stephane\Documents\Arduino\libraries\NtpClient-3.0.2-beta\src\NtpClientLib.h:59,
                 from C:\Users\stephane\Documents\Arduino\libraries\NtpClient-3.0.2-beta\src\NTPClientLib.cpp:32:
C:\Users\stephane\AppData\Local\Arduino15\packages\esp8266\hardware\esp8266\3.0.1\cores\esp8266/Arduino.h:160:17: note:                 'typedef uint8_t byte'
  160 | typedef uint8_t byte;
      |                 ^~~~
In file included from C:\Users\stephane\Documents\Arduino\libraries\NtpClient-3.0.2-beta\src\NtpClientLib.h:59,
                 from C:\Users\stephane\Documents\Arduino\libraries\NtpClient-3.0.2-beta\src\NTPClientLib.cpp:32:
C:\Users\stephane\AppData\Local\Arduino15\packages\esp8266\hardware\esp8266\3.0.1\cores\esp8266/Arduino.h:247:27: error: reference to 'byte' is ambiguous
  247 | uint16_t makeWord(byte h, byte l);
      |                           ^~~~
In file included from c:\users\stephane\appdata\local\arduino15\packages\esp8266\tools\xtensa-lx106-elf-gcc\3.0.3-gcc10.3-9bcba0b\xtensa-lx106-elf\include\c++\10.3.0\bits\stl_algobase.h:61,
                 from c:\users\stephane\appdata\local\arduino15\packages\esp8266\tools\xtensa-lx106-elf-gcc\3.0.3-gcc10.3-9bcba0b\xtensa-lx106-elf\include\c++\10.3.0\array:40,
                 from c:\users\stephane\appdata\local\arduino15\packages\esp8266\tools\xtensa-lx106-elf-gcc\3.0.3-gcc10.3-9bcba0b\xtensa-lx106-elf\include\c++\10.3.0\tuple:39,
                 from c:\users\stephane\appdata\local\arduino15\packages\esp8266\tools\xtensa-lx106-elf-gcc\3.0.3-gcc10.3-9bcba0b\xtensa-lx106-elf\include\c++\10.3.0\functional:54,
                 from C:\Users\stephane\Documents\Arduino\libraries\NtpClient-3.0.2-beta\src\NtpClientLib.h:44,
                 from C:\Users\stephane\Documents\Arduino\libraries\NtpClient-3.0.2-beta\src\NTPClientLib.cpp:32:
c:\users\stephane\appdata\local\arduino15\packages\esp8266\tools\xtensa-lx106-elf-gcc\3.0.3-gcc10.3-9bcba0b\xtensa-lx106-elf\include\c++\10.3.0\bits\cpp_type_traits.h:404:30: note: candidates are: 'enum class std::byte'
  404 |   enum class byte : unsigned char;
      |                              ^~~~
In file included from C:\Users\stephane\Documents\Arduino\libraries\NtpClient-3.0.2-beta\src\NtpClientLib.h:59,
                 from C:\Users\stephane\Documents\Arduino\libraries\NtpClient-3.0.2-beta\src\NTPClientLib.cpp:32:
C:\Users\stephane\AppData\Local\Arduino15\packages\esp8266\hardware\esp8266\3.0.1\cores\esp8266/Arduino.h:160:17: note:                 'typedef uint8_t byte'
  160 | typedef uint8_t byte;
      |                 ^~~~
exit status 1
Erreur de compilation pour la carte Generic ESP8266 Module

@dok-net
Copy link
Contributor

dok-net commented Jun 28, 2021

Food for thought, from NtpClientLib.h:

#if defined ESP8266 || defined ESP32
#include <functional>
using namespace std;

[...]

vs.
enum class byte : unsigned char {} ; (since C++17)
vs.
Arduino.h

typedef uint8_t byte;

@d-a-v
Copy link
Collaborator

d-a-v commented Jun 28, 2021

I could reproduce.
That is because of using namespace std which is incompatible with byte.
See gmag11/NtpClient#115

edit:
Keeping this opened even if there's nothing to do anymore in this core,
but tracking PR from NtpClient library which is widely used.

@d-a-v
Copy link
Collaborator

d-a-v commented Jul 10, 2021

It appears that there are a number of libraries with using namespace std; and using byte.
Arduino's byte and using namespace std; are incompatible.
The fix should be to remove all using namespace std; and use std::stuff instead of stuff where relevant but is is not possible to go and fix all external projects or libraries.
Removing Arduino's byte in favor of using std::byte; is not an option because std::byte test = 0; is an error (int incompatible with std::byte…)
I wonder whether there's another path than to patch the toolchain and make std::byte definition optional for this core.

@d-a-v d-a-v removed the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Jul 10, 2021
@mcspr
Copy link
Collaborator

mcspr commented Jul 10, 2021

std::byte test{0}; is allowed (but might be looking foreign to anyone coming from C), and so is std::byte test = (std::byte)1; btw.

With the linked issue, the general problem is with using namespace std; in the global namespace, which should probably be discouraged instead of making it silently working with a ton of workarounds?
Also see https://www.cryptopp.com/wiki/Std::byte and this small comment here

@d-a-v
Copy link
Collaborator

d-a-v commented Jul 10, 2021

I am now closing again this issue that I re-opened.
esp8266/Arduino maintainers won't be doing anything at all to try to keep backward compatibility with already existing codes which are broken when using this core-v3 with the introduction of c++17's std::byte when bad c++ practices are involved (such as using namespace std;).

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

Successfully merging a pull request may close this issue.

7 participants