-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Added c++ linker command to allow to include libstdc++ when linking. #838
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
Conversation
This is needed otherwise the template C++ container support are missing. The "compiler.c.elf.cmd" specifies the linker, the C linker was wrong here and does not work for true C++ apps. This change does not hurt because when no libstdc++ features are being used there is no change. Changed the Arduino function map() into Arduino_map(). This is needed otherwise it clashes with the C++ template class. e.g.: #include <map>
PS: I also added the same changes into the 32-bit Arduino SAMD release. |
cores/esp32/Arduino.h
Outdated
@@ -127,7 +127,7 @@ void loop(void); | |||
|
|||
long random(long, long); | |||
void randomSeed(unsigned long); | |||
long map(long, long, long, long, long); | |||
long Arduino_map(long, long, long, long, long); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this is a good idea at all. Maybe undefine the other map? Arduino users expect this to work this way, so any sketch that uses it will break ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other map is the regular C++ libstdc++ map which is being used in all C++ books, C++ projects, etc.. C++ is now here with this for more than 20 years. It is a fault by Arduino to use this name. It is also little being used compared to the C++ map.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand, but this is Arduino framework so it should adhere to the well known Arduino API ;) we can not rename or get rid of this all together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe it can be renamed to Arduino_map and then define map to Arduino_map (which could in turn be later undefined to allow for C++ map)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Arduino uses the gcc compiler and C runtime which brings all the standard runtime libraries. This goes first and than the Arduino libs are on top of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, but you tell me with this change how will all sketches and examples out there that use Arduino's "map" work?
code needs to do a #undef map before including <map> It is now the same as in the SAMD Version
cores/esp32/Arduino.h
Outdated
@@ -127,7 +127,8 @@ void loop(void); | |||
|
|||
long random(long, long); | |||
void randomSeed(unsigned long); | |||
long map(long, long, long, long, long); | |||
long Arduino_map(long, long, long, long, long); | |||
#define map Arduino_map | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to define it as map(a,b,c,d,e)
so you can per say have class members named map
urghh... build breaks... I'll look into this and see what can be done :) |
If I did something wrong please let me know. |
oh no :) but the define somehow breaks builds and I am not sure yet why. I think that you can click on the details like of the travis ci build and see the errors yourself too. Funny enough, the previous wilder definition did not give any issues. |
I don't know what is wrong with my checkin. Can you help. |
@me-no-dev should I reverse the Arduino_map define to be used without parameters? |
sorry, i hoped to have some time for this friday, but could not make it. So far IDF compiles it fine. |
tested :) #include <map>
void setup() {
std::map<char,int> first;
first['a']=10;
first['b']=30;
first['c']=50;
first['d']=70;
map(analogRead(34),0,1024,0,100);
}
void loop(){} |
@me-no-dev thank you for testing. |
@helmut64
chuck. |
@stickbreaker I don't know where the BLE_scan.ino comes from. |
@helmut64 you do not need any of those edits to use C++ map ;) I executed the above code WITHOUT applying this PR and it is fine. |
@me-no-dev when don't redefine the map in Arduino.h, I get the following error: (on SAMD and ESP32)
In file included from /var/folders/r6/_hqzqjnd5wjd5pc7f23rms9c0000gn/T/arduino_build_249479/sketch/RadioTest.ino.cpp:1:0: |
please show me the code you are compiling. |
@me-no-dev the project is huge, here is an extract of the .h file moved into an simple App. |
I changed |
@me-no-dev great, I have many more cases in my code and the map problems are only on Arduino, I do now the following after include: Thank you. In platforms.txt, the compiler.c.elf.cmd=xtensa-esp32-elf-gcc is somehow not right and should be "xtensa-esp32-elf-g++", on the SAMD architecture it does not work without the g++ linker. Should I continue this patch the the platforms change only, or should we close it because it works. |
OK, now let's discuss the g++ thing :) Can you show me an example where g++ will make a difference compared to gcc? Not on SAMD but on ESP32 ;) ESP-IDF also compiles with gcc and AFAIK it should not make any difference. |
The ESP32 Arduino linking works fine and all library C++ container functions are included as exspected. The case can be closed. FYI: Below is the error I am getting on the samd platform, changing the compiler.c.elf.cmd to g++ is the workaround. Somehow on Arduino ESP32 it works fine even using gcc as a linker. RadioShuttle/RadioShuttle.cpp.o: In function |
The issues you see are probably connected to the ARM toolchain that you use for compiling :) |
Dear @me-no-dev , thank you for working with me on this. I have to say your support is supior, which everyone can see following the Issues and Poll requests. Thank you. Helmut (from Hannover Germany) |
@helmut64 thanks for the good words :) You are very welcome! |
This is needed otherwise the template C++ container support
are missing. The "compiler.c.elf.cmd" specifies the linker,
the C linker was wrong here and does not work for true C++ apps.
This change does not hurt because when no libstdc++ features are
being used there is no change.
Changed the Arduino function map() into Arduino_map(). This is needed
otherwise it clashes with the C++ template class. e.g.: #include