Don't use a refrence into an rvalue object #11965
Conversation
I would've guessed that it was valid due to lifetime extension, but gcc and clang disagree. |
| // For _WIN32 there are two types std::string and std::wstring | ||
| // we must pick one explicit, | ||
| // to prevent "use of overloaded operator '<<' is ambiguous" error | ||
| // on clang-cl builds. | ||
| TagLib::FileName filename = file.name(); |
There was a problem hiding this comment.
I think the best way to fix this would be something like this:
#ifdef _WIN32
TagLib::FileName filename_owning = file.name();
auto filename = std::basic_string_view(filename_owning.wstr());
#else
auto filename = std::basic_string_view(file.name());
#endifthis is polymorphic over the CharT thanks to CTAD and does not involve conversion. Could be even simpler if we use just convert to char* on windows directly, or was there a reason to use the wide variant?
There was a problem hiding this comment.
I wonder what implications the unfortunate API design has for ownership? Eg is file.name() just leaking the string if not on windows?
There was a problem hiding this comment.
file.name() is just a non owning pointer to the original string on Linux/MacOS
I have kept the native types for filename to have this issue more visible.
There was a problem hiding this comment.
okay, so no leaks, just extra memory allocation on windows it seems.
This should fix the empty filename in the taglib error message
Co-authored-by: Swiftb0y <[email protected]>
|
I should start caring about the whitespace in my suggestions. These pre-commit trailing whitespace failures are annoying. I'm sorry. Can you please fix and amend? Thank you. |
This should fix the empty filename in the taglib error message
#11964
Maybe one can share some C++ knowledge, was
not allowed or is it a compiler bug?
with
const std::wstring &wstr() const;and file name() returns by value.