String constructor and concat with int64_t results in "ld"

[TD-er]

Board

Any

Device Description

Hardware should not matter, but this test was done on a:
ESP32-D0WDQ5-V3

Hardware Configuration

Not important

Version

v2.0.6

IDE Name

PlatformIO

Operating System

Windows 11

Flash frequency

40MHz

PSRAM enabled

no

Upload speed

115200

Description

Constructor and concat with long long (int64_t) is not working as it should.

Sketch

++
  int64_t testvalue = 123;
  String test_constructor(testvalue);
  String test_concat;
  test_concat = F("test_concat: '");
  test_concat += testvalue;
  test_concat += F("' 123");


Result:
test_constructor: `ld`
test_concat: `test_concat: 'ld' 123`

Debug Message

-

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

• I confirm I have checked existing issues, online documentation and Troubleshooting guide.

[mrengineer7777]

That's going to be an issue with WString.cpp/h. Still learning argument matching for C++, but here goes:

int64_t testvalue = 123; should be a long int. Possible constructor matches in WString.h:

        explicit String(long, unsigned char base = 10);
        explicit String(long long, unsigned char base = 10);

Possible += matches in WString.h:

        friend StringSumHelper & operator +(const StringSumHelper &lhs, long num);
        friend StringSumHelper & operator +(const StringSumHelper &lhs, long long num);

class StringSumHelper: public String {
    public:
  ...
        StringSumHelper(long num) :
                String(num) {
        }
...
        StringSumHelper(long long num) :
                String(num) {
        }
...
};

        String & operator +=(long num) {
            concat(num);
            return (*this);
        }
        String & operator +=(long long num) {
            concat(num);
            return (*this);
        }

        unsigned char concat(long num);
        unsigned char concat(long long num);

Possible matches in WString.cpp:

String::String(long value, unsigned char base) {
    init();
    char buf[2 + 8 * sizeof(long)];
    if (base==10) {
        sprintf(buf, "%ld", value);
    } else {
        ltoa(value, buf, base);
    }
    *this = buf;
}

String::String(long long value, unsigned char base) {
    init();
    char buf[2 + 8 * sizeof(long long)];
    if (base==10) {
        sprintf(buf, "%lld", value);   // NOT SURE - NewLib Nano ... does it support %lld? 
    } else {
        lltoa(value, buf, base);
    }
    *this = buf;
}

NOTE: I have tried using sprintf(buf, "%lld", value) in the past but it doesn't print correctly. I believe support for '%lld' isn't compiled into Arduino.

[TD-er]

That's going to be an issue with WString.cpp/h. Still learning argument matching for C++, but here goes:

int64_t testvalue = 123; should be a long int. Possible constructor matches in WString.h:

        explicit String(long, unsigned char base = 10);
        explicit String(long long, unsigned char base = 10);

Nope, I guess the constructor is one of the few things I'm quite certain there will not be any mismatch as it is declared with explicit and int64_t is (as far as I know) a typedef for long long.

NOTE: I have tried using sprintf(buf, "%lld", value) in the past but it doesn't print correctly. I believe support for '%lld' isn't compiled into Arduino.

I think that's the real problem here.

As a work-around for my own code, I have made these functions.

String ull2String(uint64_t value, uint8_t base) {
  String res;

  if (value == 0) {
    res = '0';
    return res;
  }

  while (value > 0) {
    res   += String(static_cast<uint32_t>(value % base), base);
    value /= base;
  }

  int endpos   = res.length() - 1;
  int beginpos = 0;

  while (endpos > beginpos) {
    const char c = res[beginpos];
    res[beginpos] = res[endpos];
    res[endpos]   = c;
    ++beginpos;
    --endpos;
  }

  return res;
}

String ll2String(int64_t value, uint8_t  base) {
  if (value < 0) {
    String res;
    res = '-';
    res += ull2String(value * -1ll, base);
    return res;
  } else {
    return ull2String(value, base);
  }
}

But of course, I have no clue where other code may be using those String class functions with a long long type.

[mrengineer7777]

@TD-er I've been thinking about this and decided to investigate String.h by comparing against related projects.

https://github.com/arduino/ArduinoCore-API/blob/master/api/String.cpp
https://github.com/esp8266/Arduino/blob/master/cores/esp8266/WString.cpp
https://github.com/espressif/arduino-esp32/blob/master/cores/esp32/WString.cpp

"long long" (int64_t) support:

arduino-esp32
2022.05.18 Added in #6768. sprintf(buf, "%lld", value); // NOT SURE - NewLib Nano ... does it support %lld?
Did int64_t ever work here?

ArduinoCore-API
No "long long". "long" uses ltoa().

ESP8266
2021.02.19 Added "long long" in esp8266/Arduino#7888
2022.04.11 esp8266/Arduino#8531
Resolved int32/int64 conversion by replacing sprintf with itoa/ltoa/lltoa.

Proposed fixes:
Replace sprintf with itoa/ltoa/lltoa. I can submit a PR for this if you'd like.
Consider bringing in other fixes to WString.cpp from ESP8266.
Implementation of lltoa() seems slow. Consider a better implementation. ESP8266 is at least smaller in code. https://github.com/esp8266/Arduino/blob/master/cores/esp8266/stdlib_noniso.cpp

https://github.com/espressif/arduino-esp32/commits/master/cores/esp32/WString.cpp
https://github.com/esp8266/Arduino/commits/master/cores/esp8266/WString.cpp

[TD-er]

What does surprise me is that the Arduino String class among ESP8266 and ESP32 are not the same.
If there is some very useful tweak possible to make something a lot more efficient on ESP32, I am all in for using that, but I don't think there is any good reason why this class has different implementations on both Arduino platforms.
So my preferred fix would be to have both classes to be the same with only minimal differences where it makes sense to have them different.

[mrengineer7777]

I also think they should match. However they are maintained by different owners. I know that ESP8266 handles constants in flash differently than ESP32. Right now the files are too different for me to sync up, but I can bring over a few changes that I do understand.

[mrengineer7777]

@TD-er Try the proposed PR and see if that fixes your issue. Testing appreciated!

[TD-er]

I will tomorrow, but probably not before the evening.

[SuGlider]

Thank you @TD-er and @mrengineer7777 for investigating it!
I also agree that we shall bring most ESP8266 improvements to ESP32 Arduino Core.