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

[cnd] add C17, C23, C++20, C++23 language standards #6440

Merged
merged 2 commits into from
Oct 2, 2023

Conversation

bitrunner
Copy link

This pull request adds C17, C23, C++20, and C++23 language standards. This only seems useful for netbeans managed makefile projects. I tested by making a new project and changing the standard and observing the build output on Linux with GCC 13.2.0 contains the expected compiler flag to enforce the selected standard. I also tested opening projects created prior to this change and verified their existing language standard settings are unchanged by this.

This does not update the custom C/C++ lexical analyzer in netbeans for these standard revisions as that seems superseded by the switch to LSP.

@mbien mbien added the C/C++ label Sep 14, 2023
@vieiro
Copy link
Contributor

vieiro commented Sep 30, 2023

Looks like we have C++20's std::numbers::pi now...

imagen

I'll try to review this during the busy weekend...

Copy link
Contributor

@vieiro vieiro left a comment

Choose a reason for hiding this comment

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

Hi @bitrunner ,

Would it be possible to add the TODO_CND comments specified above?

Alternativelly, we could add new CndLexerUtilities.getHeaderCppXXXFilter() and CndLexerUtilities.getHeaderCXXFilter() methods (that may delegate to existing latest standard methods).

By doing so it'll be easier for us to implement those CndLexerUtilities new methods for the new standards in the future.

@bitrunner
Copy link
Author

bitrunner commented Oct 2, 2023

Thanks for spending time on this and your helpful comments @vieiro!

I pushed an additional commit that updates cnd.lexer for the new language standards too. I didn't realize that was still useful in a LSP backed CND. I have much to learn.

The updates are based on the keywords listed at cppreference.com for C and C++.

I took a few liberties with that CndLexerUtilities file. Maintaining standard and GCC versions of those filters was unwieldy with so many standards versions and the "standard" versions of the filters weren't used anyway, so I just made one set of them with the GCC tokens included. It was always using the GCC ones before anyway.

Copy link
Contributor

@vieiro vieiro left a comment

Choose a reason for hiding this comment

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

Thank you, @bitrunner for this. Excellent contribution.

@mbien
Copy link
Member

mbien commented Oct 2, 2023

@vieiro to fix the CI issue, #6469 would have to be applied to the cnd branch. Not sure what the best way to do that? Simply cherry pick the commit and add to this PR?

although outside of the build, CI is of limited use for cnd atm since no cnd tests are hooked up

@vieiro
Copy link
Contributor

vieiro commented Oct 2, 2023

@vieiro to fix the CI issue, #6469 would have to be applied to the cnd branch. Not sure what the best way to do that? Simply cherry pick the commit and add to this PR?

@mbien Let's merge this as is. I'll add JDK21-ga in a future PR by cherry-picking ac9bdbb, thanks for the pointer!

@vieiro vieiro merged commit a3e5b5a into apache:cnd Oct 2, 2023
30 of 31 checks passed
@matthiasblaesing
Copy link
Contributor

matthiasblaesing commented Oct 2, 2023

I'll add JDK21-ga in a future PR by cherry-picking ac9bdbb, thanks for the pointer!

I suggest to not cherry-pick, but merge master. #6439 demonstrated that a direct merge works and in my experience the less you mess with history, the easier merging becomes.

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

Successfully merging this pull request may close these issues.

4 participants