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

c_std, cpp_std: Change to a list of desired versions in preference order #10332

Merged
merged 3 commits into from
Aug 30, 2023

Conversation

xclaesse
Copy link
Member

Projects that prefer GNU C but can fallback to ISO C can now set for
example `default_options: 'c_std=gnu11,c11'` and it will use gnu11 when
available, fallback to c11 otherwise. It is an error only if none of the
values are supported by the current compiler.

This allows to deprecate gnuXX values from MSVC compiler, that means
that `default_options: 'c_std=gnu11'` will now print warning with MSVC
but still fallback to 'c11' value. No warning is printed if at least one
of the values is valid, i.e. `default_options: 'c_std=gnu11,c11'`.

In the future that deprecation warning will become an hard error because
`c_std=gnu11` should mean GNU is required, for projects that cannot be
built with MSVC for example.

@codecov
Copy link

codecov bot commented Apr 28, 2022

Codecov Report

Merging #10332 (939c744) into master (7833765) will decrease coverage by 4.79%.
Report is 7 commits behind head on master.
The diff coverage is n/a.

❗ Current head 939c744 differs from pull request most recent head 82a8c72. Consider uploading reports for the commit 82a8c72 to get more accurate results

@@            Coverage Diff             @@
##           master   #10332      +/-   ##
==========================================
- Coverage   70.10%   65.32%   -4.79%     
==========================================
  Files         218      203      -15     
  Lines       47689    43592    -4097     
  Branches    11368     9526    -1842     
==========================================
- Hits        33434    28475    -4959     
- Misses      11773    12854    +1081     
+ Partials     2482     2263     -219     

see 421 files with indirect coverage changes

@xclaesse
Copy link
Member Author

@dcbaker @eli-schwartz @jpakkane I know there are been recurrent requests for more flexible way of defining the std. What do you think about this proposal? Are there use-cases not covered?

@eli-schwartz
Copy link
Member

The first thing I thought about when reading this is, it seems like it might break any meson.build that does get_option('c_std').

On second look, it seems like at evaluation time it always yields whichever one gets selected in the end? Clever...

@xclaesse
Copy link
Member Author

xclaesse commented May 3, 2022

On second look, it seems like at evaluation time it always yields whichever one gets selected in the end? Clever...

Yes exactly, that's the trick, you set a list but it selects just one.

@nirbheek
Copy link
Member

I don't see any blockers for merging this? Other than resolving conflicts and fixing mypy.

@dcbaker
Copy link
Member

dcbaker commented Jan 14, 2023

It conflicts pretty bad #11280… I think getting rid of the individual functions is going the wrong way

@dcbaker
Copy link
Member

dcbaker commented Jan 14, 2023

But I think that this is the right language change, I’m just not sure about the implementation

@eli-schwartz
Copy link
Member

It conflicts pretty bad #11280… I think getting rid of the individual functions is going the wrong way

And merged, so this needs fixing.

@xclaesse
Copy link
Member Author

xclaesse commented May 3, 2023

Rebased.

@tristan957
Copy link
Contributor

@dcbaker looks like you had a concern here

@xclaesse
Copy link
Member Author

xclaesse commented May 6, 2023

@dcbaker looks like you had a concern here

I solved the conflicts and kept the functions in optinterpreter.

mesonbuild/coredata.py Show resolved Hide resolved
docs/markdown/Builtin-options.md Outdated Show resolved Hide resolved
docs/markdown/Builtin-options.md Outdated Show resolved Hide resolved
docs/markdown/Builtin-options.md Outdated Show resolved Hide resolved
docs/markdown/Builtin-options.md Outdated Show resolved Hide resolved
docs/markdown/snippets/cstd.md Outdated Show resolved Hide resolved
mesonbuild/coredata.py Outdated Show resolved Hide resolved
mesonbuild/coredata.py Outdated Show resolved Hide resolved
mesonbuild/coredata.py Outdated Show resolved Hide resolved
mesonbuild/coredata.py Outdated Show resolved Hide resolved
The only place it can be set to False is from optinterpreter. Better
check value there and deprecate string usage.
@xclaesse xclaesse force-pushed the std-opt branch 3 times, most recently from a9d0f26 to 639b8c2 Compare August 6, 2023 16:45
mesonbuild/optinterpreter.py Outdated Show resolved Hide resolved
docs/markdown/Builtin-options.md Outdated Show resolved Hide resolved
Projects that prefer GNU C but can fallback to ISO C can now set for
example `default_options: 'c_std=gnu11,c11'` and it will use gnu11 when
available, fallback to c11 otherwise. It is an error only if none of the
values are supported by the current compiler.

This allows to deprecate gnuXX values from MSVC compiler, that means
that `default_options: 'c_std=gnu11'` will now print warning with MSVC
but still fallback to 'c11' value. No warning is printed if at least one
of the values is valid, i.e. `default_options: 'c_std=gnu11,c11'`.

In the future that deprecation warning will become an hard error because
`c_std=gnu11` should mean GNU is required, for projects that cannot be
built with MSVC for example.
@jpakkane
Copy link
Member

The obvious question to this is whether this works only for this option or should it work for all options with choices?

I don't think it should work with anything except this, but OTOH it does add a behavioural inconsistency (though a mild and arguably good in any case).

@xclaesse
Copy link
Member Author

@jpakkane I don't think we can do it for project options. The trick here is Meson selects which of the choices is used, so get_option() only return one value, but for project option you would have to return all values because only the project can select... Which is no different from a regular array option.

I think it is a specific behavior for a very specific problem, I don't currently see how it could be generalized.

@jpakkane
Copy link
Member

I agree this should not be a generally available thing, but I suspect people are going to ask for it anyway, so it's better to be prepared.

@jpakkane jpakkane merged commit 08d83a4 into mesonbuild:master Aug 30, 2023
lazka added a commit to lazka/meson that referenced this pull request Sep 23, 2023
c++26 supoprt was added in mesonbuild#11986, but regressed in mesonbuild#10332 because
the versions now get checked against the global _ALL_STDS list, and
c++26 was missing there.

Fix by adding c++26 to _ALL_STDS
lazka added a commit to lazka/meson that referenced this pull request Sep 23, 2023
c++26 support was added in mesonbuild#11986, but regressed in mesonbuild#10332 because
the versions now get checked against the global _ALL_STDS list, and
c++26 was missing there.

Fix by adding c++26 to _ALL_STDS
lazka added a commit to lazka/meson that referenced this pull request Sep 23, 2023
c++26 support was added in mesonbuild#11986, but regressed in mesonbuild#10332 because
the versions now get checked against the global _ALL_STDS list, and
c++26 was missing there.

Fix by adding c++26 to _ALL_STDS
lazka added a commit to lazka/meson that referenced this pull request Sep 24, 2023
c++26 support was added in mesonbuild#11986, but regressed in mesonbuild#10332 because
the versions now get checked against the global _ALL_STDS list, and
c++26 was missing there.

Fix by adding c++26 to _ALL_STDS
xclaesse pushed a commit that referenced this pull request Sep 24, 2023
c++26 support was added in #11986, but regressed in #10332 because
the versions now get checked against the global _ALL_STDS list, and
c++26 was missing there.

Fix by adding c++26 to _ALL_STDS
robtaylor pushed a commit to robtaylor/meson that referenced this pull request Oct 14, 2023
c++26 support was added in mesonbuild#11986, but regressed in mesonbuild#10332 because
the versions now get checked against the global _ALL_STDS list, and
c++26 was missing there.

Fix by adding c++26 to _ALL_STDS
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.

6 participants