8000 Support etl::underlying_type with compiler builtin by jiangyilism · Pull Request #1045 · ETLCPP/etl · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Support etl::underlying_type with compiler builtin #1045

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

Merged

Conversation

jiangyilism
Copy link
Contributor

No description provided.

Copy link
Contributor
@rolandreichweinbmw rolandreichweinbmw left a comment

Choose a reason for hiding this comment

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

Thanks! This is useful in some of my use cases.

@jwellbelove
Copy link
Contributor

The unit tests all fail for MSVC.

@jwellbelove
Copy link
Contributor
jwellbelove commented Mar 6, 2025

You need to update /etl/generators/type_traits_generator.h
Test with the python script generator_test.bat

@jiangyilism jiangyilism force-pushed the yichiang/etl_underlyingtype branch from de17a4f to 14b1fe1 Compare March 7, 2025 13:25
@jiangyilism
Copy link
Contributor Author

You need to update /etl/generators/type_traits_generator.h Test with the python script generator_test.bat

Modified /etl/generators/type_traits_generator.h and tested with

python scripts/generator_test.py on a linux PC. MSVC is not tested since I do not have a windows PC by hand. I will try to get one if MSVC tests are still failing.

@jwellbelove
Copy link
Contributor

It fails because __underlying_type is not defined for MSVC.

@jiangyilism jiangyilism force-pushed the yichiang/etl_underlyingtype branch from 14b1fe1 to 81085cb Compare March 7, 2025 21:41
@github-project-automation github-project-automation bot moved this from In progress to Done in Embedded Template Library Mar 10, 2025
@jiangyilism jiangyilism reopened this Mar 10, 2025
@github-project-automation github-project-automation bot moved this from Done to In progress in Embedded Template Library Mar 10, 2025
@jiangyilism
Copy link
Contributor Author

It fails because __underlying_type is not defined for MSVC.

updated to exclude the underlying_type tests if ETL_USING_BUILTIN_UNDERLYING_TYPE is not defined. It is not defined in MSVC builds since ETL_USE_TYPE_TRAITS_BUILTINS is not defined.

A successful MSVC run in my fork
https://github.com/jiangyilism/etl/actions/runs/13730165615

@jiangyilism jiangyilism force-pushed the yichiang/etl_underlyingtype branch from 81085cb to 555d710 Compare March 10, 2025 08:29
Comment on lines 2332 to 2336
template <typename T>
ETL_CONSTEXPR underlying_type_t<T> to_underlying(T val) ETL_NOEXCEPT
{
return static_cast<underlying_type_t<T>>(val);
}
Copy link
Contributor
@drewr95 drewr95 Mar 10, 2025

Choose a reason for hiding this comment

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

Shouldn't this be in "utility.h"? cppreference notes that it is defined in <utility>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for reminding.
Moved to_underlying<> to include/etl/utility.h .

@jiangyilism jiangyilism force-pushed the yichiang/etl_underlyingtype branch from 555d710 to 160cd80 Compare March 11, 2025 07:59
@jiangyilism jiangyilism requested a review from drewr95 March 13, 2025 16:51
@rolandreichweinbmw
Copy link
Contributor

This feature is already available in https://github.com/rolandreichweinbmw/etlplus

@jwellbelove jwellbelove changed the base branch from master to development April 30, 2025 16:00
@jwellbelove jwellbelove merged commit f69da85 into ETLCPP:development Apr 30, 2025
@github-project-automation github-project-automation bot moved this from In progress to Done in Embedded Template Library Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

4 participants
0