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

String: API must be fixed #8430

Closed
d-a-v opened this issue Jan 3, 2022 · 1 comment · Fixed by #8526
Closed

String: API must be fixed #8430

d-a-v opened this issue Jan 3, 2022 · 1 comment · Fixed by #8526

Comments

@d-a-v
Copy link
Collaborator

d-a-v commented Jan 3, 2022

Per this comment,
String API has changed since release 2.7.4 and should be fixed.

(cc: @Ciapas-Linux)

@d-a-v d-a-v added this to the 3.1 milestone Jan 3, 2022
@mcspr
Copy link
Collaborator

mcspr commented Jan 3, 2022

Notice that this is esp8266 & esp32 Core feature, ArduinoCore-API's String does not implement this and will fail with ambiguous overload for ‘operator=’

diff --git a/test/src/String/test_operators.cpp b/test/src/String/test_operators.cpp
index 67cb39b..e48fc4f 100644
--- a/test/src/String/test_operators.cpp
+++ b/test/src/String/test_operators.cpp
@@ -164,3 +164,10 @@ TEST_CASE ("Testing & String::operator = (StringSumHelper &&)", "[String-operato
   str1 = static_cast<arduino::StringSumHelper&&>(str+ch);
   REQUIRE(str1 == "Hello!");
 }
+
+TEST_CASE ("Testing assignment operator")
+{
+  arduino::String str;
+  str = 12345;
+  REQUIRE(str == "12345");
+}
/home/runner/dev/ArduinoCore-API/test/src/String/test_operators.cpp: In function ‘void ____C_A_T_C_H____T_E_S_T____36()’:
/home/runner/dev/ArduinoCore-API/test/src/String/test_operators.cpp:171:9: error: ambiguous overload for ‘operator=’ (operand types are ‘arduino::String’ and ‘int’)
  171 |   str = 12345;
      |         ^~~~~

https://github.com/arduino/ArduinoCore-API/blob/e03b65374c614130aa1b11597e07b3b5089a726d/api/String.h#L102

With 2.7.4, and earlier versions, this would've called String::operator=(StringSumHelper&&) and converted the int through String::String(int, base=10)
https://github.com/espressif/arduino-esp32/blob/caef4006af491130136b219c1205bdcf8f08bf2b/cores/esp32/WString.h#L105
(and, esp32 Core still does this :)

String::operator=(char) was added after looking at std::string API
gcc.gnu.org/onlinedocs/gcc-11.2.0/libstdc++/api/a02039.html#a79cf5ca6fa3860d67ddde51fc19f4a2d
And only our string has it, I wonder if ArduinoCore API should have it as well

@d-a-v d-a-v changed the title String: API should be fixed String: API must be fixed Mar 14, 2022
mcspr added a commit that referenced this issue Apr 5, 2022
Restore the pre-3.0.0 behaviour when we could assign numeric values to
the string object. After introducing operator =(char), everything was
converted to char instead of the expected 'stringification' of the
number (built-in int, long, unsigned int, unsigned long, long long,
unsigned long long, float and double)

Add toString() that handles conversions, re-use it through out the class

Fix #8430
hasenradball pushed a commit to hasenradball/Arduino that referenced this issue Nov 18, 2024
Restore the pre-3.0.0 behaviour when we could assign numeric values to
the string object. After introducing operator =(char), everything was
converted to char instead of the expected 'stringification' of the
number (built-in int, long, unsigned int, unsigned long, long long,
unsigned long long, float and double)

Add toString() that handles conversions, re-use it through out the class

Fix esp8266#8430
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.

2 participants