Skip to content

added (partial) support for specifying C++23 and support more "-std" options#3212

Merged
danmar merged 9 commits intocppcheck-opensource:mainfrom
firewave:std
Apr 15, 2022
Merged

added (partial) support for specifying C++23 and support more "-std" options#3212
danmar merged 9 commits intocppcheck-opensource:mainfrom
firewave:std

Conversation

@firewave
Copy link
Copy Markdown
Collaborator

No description provided.

@firewave firewave changed the title added support for specifying C++23 and some more "-std" aliases added support for specifying C++23 and some more "-std" options Apr 17, 2021
Comment thread lib/standards.h
} else if (std == "c++20") {
return Standards::CPP20;
} else if (std == "c++23") {
return Standards::CPP20;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

should write CPP23 here

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done.

Comment thread lib/standards.h Outdated
static cstd_t getC(const std::string &std) {
if (std == "c89") {
return Standards::C89;
} else if (std == "c99") {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit; I'd rather not write these redundant else.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done

Comment thread lib/standards.h Outdated
cpp = CPP20;
return true;
}
if (str == "c++23" || str == "C++23") {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

could we lowercase str and reuse getCpp somehow? something like:

stdValue = str;
cpp = getCpp(lower(str));
return cpp != CPPLatest || lower(str) == getCPP();

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done - but with some slightly different logic

Comment thread lib/standards.h Outdated
static cppstd_t getCPP(const std::string &std) {
if (std == "c++03") {
return Standards::CPP03;
} else if (std == "c++11") {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit; I don't have a strong opinion but would not write these redundant else.

Copy link
Copy Markdown
Collaborator Author

@firewave firewave Apr 17, 2021

Choose a reason for hiding this comment

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

So does clang-tidy...too hasty copy and paste.

@firewave firewave marked this pull request as draft April 17, 2021 17:44
@firewave
Copy link
Copy Markdown
Collaborator Author

The second time the build broke for me since the GUI tests are not being built by CMake - working on a branch where I added at least the building for now.

@amai2012
Copy link
Copy Markdown
Collaborator

I would prefer to delay this change:
C++23 is still work-in-progress and think we're not even close on getting C++20 done (I've created https://trac.cppcheck.net/ticket/10251 for tracking).

Offering this option might be misleading due to a lack of support of almost feature.

@firewave
Copy link
Copy Markdown
Collaborator Author

firewave commented Sep 4, 2021

If (hopefully it's "when" at some point) I will return to action I will exclude/disable the UI changes and just keep the internal -std= improvements. I should probably add some unit tests as well.

@firewave
Copy link
Copy Markdown
Collaborator Author

I backed out the GUI support.

@firewave firewave marked this pull request as ready for review October 24, 2021 12:19
@firewave firewave marked this pull request as draft October 24, 2021 12:35
@firewave
Copy link
Copy Markdown
Collaborator Author

firewave commented Jan 4, 2022

This also requires changes to simplecpp to work properly.

@firewave
Copy link
Copy Markdown
Collaborator Author

firewave commented Feb 24, 2022

Will update this after cppcheck-opensource/simplecpp#242 was merged and bumped to this repo which makes things a bit cleaner.

@firewave firewave changed the title added support for specifying C++23 and some more "-std" options added (partial) support for specifying C++23 and support more "-std" options Mar 24, 2022
@firewave
Copy link
Copy Markdown
Collaborator Author

This is again ready for review.

To actually use simplecpp::DUI::std a few more internal changes are necessary first.

@firewave firewave marked this pull request as ready for review March 24, 2022 11:59
@firewave firewave marked this pull request as draft March 24, 2022 12:12
@firewave
Copy link
Copy Markdown
Collaborator Author

Demoted to draft again since there's some issues in the preprocessor - pre-existing and introduced by my changes.

@firewave firewave marked this pull request as ready for review April 14, 2022 16:57
@firewave
Copy link
Copy Markdown
Collaborator Author

This is finally ready for review again.

@danmar danmar merged commit 8f728cb into cppcheck-opensource:main Apr 15, 2022
@firewave firewave deleted the std branch April 15, 2022 14:23
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

Successfully merging this pull request may close these issues.

3 participants