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

[PROPOSAL] useconstexpr instead of inline constexpr for functions #59

Merged
merged 1 commit into from
Jan 27, 2024

Conversation

vil02
Copy link
Contributor

@vil02 vil02 commented Jan 27, 2024

Description

While browsing though the C++ code I have noticed that many functions have inline constexpr specifiers. According to C++ reference for constexpr: A constexpr specifier used in a function or static data member(since C++17) declaration implies inline.

Build and tests pass on my end.

Checklist

  • A description of the changes in this PR is mentioned above.
  • All the new and existing tests pass.
  • The code follows the code style and conventions of the project.
  • No plagiarized, duplicated, or repetitive code that has been directly copied from another source.
  • I have read the whole Contributing guidelines of the project and its resources/related pages.

Screenshots (if any)

Note to reviewers

Make all inline constexpr functions constexpr.

@vil02 vil02 marked this pull request as ready for review January 27, 2024 20:10
@mertcandav
Copy link
Member

Hi.

You are right, but Jule also supports C++14 standard. This changes still implements inline for C++14 standard?
We can create preprocessor define for C++14 standard. If C++14 standard is used, this define represents inline, if not, represents nothing. So we can have inline for C++14 standard compilations.

Also, I think we can continue to use inline constexpr instead of constexpr if there is no problem (such as compiler errors or etc.) about it.

Thanks for your effort.

@mertcandav mertcandav added proposal A proposal for new feature or changings api About API labels Jan 27, 2024
@mertcandav mertcandav changed the title style: constexpr functions are inline [PROPOSAL] useconstexpr instead of inline constexpr for functions Jan 27, 2024
@vil02
Copy link
Contributor Author

vil02 commented Jan 27, 2024

This has nothing to do with the version of C++: constexpr function is inline in C++11, C++14, C++17 etc. (constexpr was introduced in C++11).

@mertcandav
Copy link
Member

mertcandav commented Jan 27, 2024

Right but reference says: constexpr implies inline for functions since C++17.
So constexpr not implies inline in C++14, by reference.

@vil02
Copy link
Contributor Author

vil02 commented Jan 27, 2024

No.

Please have a look at the cpp reference. There are two statements here:

  • A constexpr specifier used in a function declaration implies inline.
  • (since C++17) A constexpr specifier used in a static data member declaration implies inline.

@mertcandav
Copy link
Member

Oh, yes. Sorry.
I just missed separated part for static members.

I'll merge your PR.
Thanks for your contribution.

@mertcandav mertcandav self-requested a review January 27, 2024 20:42
@mertcandav mertcandav merged commit 60ec0a1 into julelang:master Jan 27, 2024
4 checks passed
@vil02
Copy link
Contributor Author

vil02 commented Jan 27, 2024

No worries. The formatting there is tricky.

Thanks!

@vil02 vil02 deleted the avoid_inline_constexpr_functions branch January 28, 2024 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api About API proposal A proposal for new feature or changings
Projects
Status: Accepted
Development

Successfully merging this pull request may close these issues.

2 participants