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

Encoding problem with std::string_view on macOS #881

Open
Jean1995 opened this issue May 10, 2023 · 2 comments
Open

Encoding problem with std::string_view on macOS #881

Jean1995 opened this issue May 10, 2023 · 2 comments

Comments

@Jean1995
Copy link

I have the following minimal example:

#include "CLI11.hpp"

#include <iostream>

int main(int argc, char** argv) {
    CLI::App app{"App description"};

    app.add_option("--input", "input option")
     ->default_val("optionA")
     ->check(CLI::IsMember({"optionA", "optionB", "optionC"}));

    CLI11_PARSE(app, argc, argv);

    auto const inputStr = app["--input"]->as<std::string_view>();

    std::cout << inputStr << std::endl;

    return 0;
}

If I run the program without any input arguments on Linux (g++), I get the expected output optionA.

However, if I run it on macOS (clang++), I get the output ����G, so some encoding problems. If I specify an argument via --input, the problem is gone. The problem is also gone by replacing std::string_view with std::string, so this appears to be a bug specific to the usage of std::string_view.

@phlptp
Copy link
Collaborator

phlptp commented May 10, 2023

I can verify this does produce an inconsistent result.
Not entirely sure what to do about it though. The fact that it works on g++ is either an inconsistent fluke or some internal compiler magic.

as<T>() when no arguments have been passed does some processing on the default_str as a temporary and does a conversion to get the results in the desired type. String_view is a view on a string so in this case is just creating a view of a temporary which is undefined behavior, as seen by sometimes working, sometimes not. When you have passed the variable it is stored internally so a view of it works fine.

I need to think a bit to see if there is a way to make this work. I agree it is an inconsistency in the API for this not to work, but not entirely sure it should work or we need to make the fail more explicit. stay tuned.

@phlptp
Copy link
Collaborator

phlptp commented Jun 16, 2023

There are a couple different ways I see to fix this. They all have some drawbacks.
The first is to make one of the internal results arrays mutable. Then when a results method is called if no arguments have been passed then it gets copied to the internal array. This internal array is already used for that purpose so add no new structure and doesn't impact existing operations in any way also doesn't add any computation. The downside is there is now a mutable structure and in a few select cases around the operations, this method is now not thread compatible, and internal things can change if the same method is executed on multiple threads could cause a race condition. This is not a race condition that affects state.

The other method would involve executing part of the processing on all options all the time, instead of just as needed. This gets around the race condition but does add extra computation that in most cases is unnecessary.

Not entirely sure which method is preferred.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants