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.

Sign up for GitHub

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

Jump to bottom

WString: remove operator==(const __FlashStringHelper*) #8569

Merged
merged 8 commits into from
May 17, 2022
Merged

WString: remove operator==(const __FlashStringHelper*) #8569

Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e15fc91
WString: overload ==
d-a-v May 16, 2022
a5cc344
keep only nullptr comparison overload
d-a-v May 16, 2022
e4b47bc
removing ==(FlashStringHelper) overload:
d-a-v May 16, 2022
a02c21c
restore the ==flashstringhelper overload, add ==decltype(NULL)
d-a-v May 16, 2022
bc52bd6
deprecate usage of NULL
d-a-v May 16, 2022
b912a70
Merge branch 'master' into streqeq
d-a-v May 17, 2022
65ad698
removing again ==(FlashStringHelper) overload
d-a-v May 17, 2022
fb666ef
revert changes about operator bool()
d-a-v May 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions cores/esp8266/WString.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,10 +211,9 @@ class String {
return *this;
}

// checks whether the internal buffer pointer is set.
// (should not be the case for us, since we always reset the pointer to the SSO buffer instead of setting it to nullptr)
// checks whether the String is empty
explicit operator bool() const {
return buffer() != nullptr;
return length() != 0;
d-a-v marked this conversation as resolved.
Show resolved Hide resolved
}

int compareTo(const String &s) const;
Expand All @@ -230,6 +229,13 @@ class String {
bool operator ==(const __FlashStringHelper *rhs) const {
return equals(rhs);
}
bool operator ==(std::nullptr_t) const {
return length() == 0;
}
d-a-v marked this conversation as resolved.
Show resolved Hide resolved
[[deprecated("use nullptr instead of NULL")]]
bool operator ==(decltype(NULL)) const {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just note that the trade-off is someone will write obj == 5; returning false positive :)
(...and iirc ide default to 'no warnings', which does not help...)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that std::nullptr_t is a distinguishable type, I tried the same with decltype(NULL).
Despite the existence of the __null internal keyword and the __sentinel__ attribute, I currently cannot manage NULL separately from int/long.

I reverted: ==(FlashStringHelper*) is removed again.

We still have this:
#define NULL nullptr

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they refer to some old version of GCC (pre 4.6?) with the __null stuff.
And I'd like to find some examples of #define NULL std::nullptr, as it seems like something for the system header to have

Copy link
Collaborator Author

@d-a-v d-a-v May 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only one I could find that is installed on my workstation and probably rarely used(*).

(*)given std:: and using are both lacking

Copy link
Collaborator

@mcspr mcspr May 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we could've had this
https://github.com/gcc-mirror/gcc/blob/1815462a6e53465c404f8a5f6116891492d4b50b/gcc/ginclude/stddef.h#L396

But, newlib overrides it
https://github.com/earlephilhower/newlib-xtensa/blob/ebc967552ce827f21fc579fd8c437037c1b472ab/newlib/libc/include/stdlib.h#L60-L62

edit: ...need to check the preprocessor chain, though, but I would guess it overrides it

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, glibc code requesting it
https://code.woboq.org/userspace/glibc/stdlib/stdlib.h.html#_M/__need_NULL

return length() == 0;
}
bool operator !=(const String &rhs) const {
return !equals(rhs);
}
Expand All @@ -239,6 +245,13 @@ class String {
bool operator !=(const __FlashStringHelper *rhs) const {
return !equals(rhs);
}
bool operator !=(std::nullptr_t) const {
return length() != 0;
}
[[deprecated("use nullptr instead of NULL")]]
bool operator !=(decltype(NULL)) const {
return length() != 0;
}
bool operator <(const String &rhs) const;
bool operator >(const String &rhs) const;
bool operator <=(const String &rhs) const;
Expand Down