Fix broken simplecpp builds with libc++

[pfultz2]

Cppcheck passes a pointer and size to simplecpp:TokenList, so we need those APIs enabled even if there is string_view and span.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[danmar]

lgtm.. @firewave ?

[firewave]

It would be helpful if the error which is fixed by this could be provided.

Also I think this has nothing to do with libc++ but actually with a compiler which bypasses the standard version we are setting. I think the "proper" fix would be #7962.

[firewave]

Also I think this has nothing to do with libc++ but actually with a compiler which bypasses the standard version we are setting.

I am not able to reproduce it, so either your compiler is misbehaving or you have CMake misconfigured.

[pfultz2]

This is the error I see:

/Users/paul/github/danmar/cppcheck/lib/cppcheck.cpp:877:16: error: no matching constructor for initialization of 'simplecpp::TokenList'
return simplecpp::TokenList{data, size, files, file.spath(), outputList};
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/paul/github/danmar/cppcheck/externals/simplecpp/./simplecpp.h:230:9: note: candidate constructor not viable: requires at most 4 arguments, but 5 were provided
TokenList(std::istream &istr, std::vector<std::string> &filenames, const std::string &filename=std::string(), OutputList *outputList = nullptr);
^
/Users/paul/github/danmar/cppcheck/externals/simplecpp/./simplecpp.h:254:9: note: candidate constructor not viable: requires at most 4 arguments, but 5 were provided
TokenList(std::string_view data, std::vector<std::string> &filenames, const std::string &filename=std::string(), OutputList *outputList = nullptr)
^
/Users/paul/github/danmar/cppcheck/externals/simplecpp/./simplecpp.h:345:9: note: candidate constructor not viable: requires 6 arguments, but 5 were provided
TokenList(const unsigned char* data, std::size_t size, std::vector<std::string> &filenames, const std::string &filename, OutputList *outputList, int unused);
^
/Users/paul/github/danmar/cppcheck/externals/simplecpp/./simplecpp.h:271:9: note: candidate constructor not viable: requires at most 3 arguments, but 5 were provided
TokenList(const std::string &filename, std::vector<std::string> &filenames, OutputList *outputList = nullptr);
^
/Users/paul/github/danmar/cppcheck/externals/simplecpp/./simplecpp.h:228:18: note: candidate constructor not viable: requires single argument 'filenames', but 5 arguments were provided
explicit TokenList(std::vector<std::string> &filenames);
^
/Users/paul/github/danmar/cppcheck/externals/simplecpp/./simplecpp.h:272:9: note: candidate constructor not viable: requires single argument 'other', but 5 arguments were provided
TokenList(const TokenList &other);
^
/Users/paul/github/danmar/cppcheck/externals/simplecpp/./simplecpp.h:273:9: note: candidate constructor not viable: requires single argument 'other', but 5 arguments were provided
TokenList(TokenList &&other);
^

Its calling the constructor that takes a pointer and size:

https://github.com/danmar/cppcheck/blob/bdf2f42f2b66ddaaa3345396b48158c554fd139c/lib/cppcheck.cpp#L877

but the constructor is not available when SIMPLECPP_TOKENLIST_ALLOW_PTR is not defined. That is only defined when string_view and span is not available:

https://github.com/danmar/cppcheck/blob/bdf2f42f2b66ddaaa3345396b48158c554fd139c/externals/simplecpp/simplecpp.h#L60

Since this is c++17, string_view is available but span is not, so this macro doesnt get defined.

I think the "proper" fix would be #7962.

I agree but since this fails to build completely, I think this can be merged now.

[pfultz2]

Can we merge this in? As I cannot build cppcheck locally on my mac.

[chrchr-github]

This looks related to https://trac.cppcheck.net/ticket/14267

[firewave]

This has been "superseded" by #7962.

[pfultz2]

This has been "superseded" by #7962.

Thats not even merged in and its a draft PR. This should be merged first since it fixes the build issues as cppcheck wont even build without this change.

Its a one line change that should be easy to undo in #7962.

[firewave]

Thats not even merged in and its a draft PR. This should be merged first since it fixes the build issues as cppcheck wont even build without this change.

Then the tests that C++17 actually builds also need to be moved here (and it doesn't with -Werror so needs other fixes as well). Also would trigger the CI which would block the work on the other PR and I don't feel like waiting an hour until I can finish that up because of an unnecessary "one line change".

That other PR will be the final one before we release so it is currently my only priority. It is also ready now except for looking into your feedback.

[danmar]

Can we merge this in? As I cannot build cppcheck locally on my mac.

this is BAD. :-(
do you think it's possible to reproduce on a macos-latest runner or something?

[danmar]

do you think it's possible to reproduce on a macos-latest runner or something?

I have a macincloud account. Unfortunately I don't have cmake and don't have permissions to install it. If you share the simplecpp.cpp compilation command from compile_commands.json and I can see if I can reproduce using that ?

[firewave]

Since #7962 has been merged this can be closed.

@pfultz2 I am still interested though how it happens to use C++17 for you. We are having macOS runners in the CI and they are not doing that. That means there is something on your systems which is forcing that.