Skip to content

update the feature-test macro for P2162R2 for variant#2006

Merged
StephanTLavavej merged 12 commits into
microsoft:mainfrom
fsb4000:1683
Jun 29, 2021
Merged

update the feature-test macro for P2162R2 for variant#2006
StephanTLavavej merged 12 commits into
microsoft:mainfrom
fsb4000:1683

Conversation

@fsb4000
Copy link
Copy Markdown
Contributor

@fsb4000 fsb4000 commented Jun 13, 2021

Fixes #1683

@fsb4000 fsb4000 requested a review from a team as a code owner June 13, 2021 06:19
@SuperWig
Copy link
Copy Markdown
Contributor

You need to also update the feature test macro test.

@SuperWig
Copy link
Copy Markdown
Contributor

Come to think of it. Considering this was implemented in C++17, thus available in that mode, shouldn't the value just be updated instead of under _HAS_CXX23?

@fsb4000
Copy link
Copy Markdown
Contributor Author

fsb4000 commented Jun 13, 2021

Yes. I found it when inspected failed test: "std/language.support/support.limits/support.limits.general/variant.version.pass.cpp"
llvm/llvm-project@0324b46#diff-a9be5c3f8e18be99f40fb35f58960f400413a76252d86b53f80d76cb09cb53ef

@AdamBucior
Copy link
Copy Markdown
Contributor

Do we have test coverage for that to ensure we never accidentally degrade it?

@StephanTLavavej StephanTLavavej added the cxx23 C++23 feature label Jun 14, 2021
Comment thread tests/std/tests/P2162R2_std_visit_for_derived_classes_from_variant/env.lst Outdated
Comment thread tests/std/tests/P2162R2_std_visit_for_derived_classes_from_variant/test.cpp Outdated
Copy link
Copy Markdown
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

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

Looks good! I'll push a couple of trivial test changes for style/comment issues I noticed.

Comment thread tests/std/tests/P2162R2_std_visit_for_derived_classes_from_variant/test.cpp Outdated
@StephanTLavavej StephanTLavavej self-assigned this Jun 24, 2021
@StephanTLavavej StephanTLavavej merged commit aaa5d08 into microsoft:main Jun 29, 2021
@StephanTLavavej
Copy link
Copy Markdown
Member

Thanks for looking into this and adding test coverage - one step closer to C++23 completeness! 🎉 😸 ✔️

@fsb4000 fsb4000 deleted the 1683 branch June 29, 2021 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cxx23 C++23 feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

P2162R2 Inheriting From variant

5 participants