Generate a CMake-time error if compiler+flags lacks deducing this support #12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…port This improves the user experience when incompatible flags are used. It also is a first step toward resolving bemanproject#10.
camio
added a commit
to camio/iterator_interface26
that referenced
this pull request
Oct 18, 2024
Added a build option, BEMAN_ITERATOR_INTERFACE26_USE_DEDUCING_THIS, that switches between the deducing this implementation and that used by stl_interfaces. It defaults to whether the compiler supports deducing this. This element of configuration is placed in a generated config.hpp. This was chosen, as opposed to using a define passed as a compiler flag, to avoid ODR violations with larger projects with incoherent flag usage. The `iterator_interface_access` struct was moved to its own header since it is needed by both the deducing this and stl_interfaces implementations. The stl_interfaces implementation was modifed to use this so it better conforms to the paper. Some cleanup is still needed before this gets merged in: - There are optional26 remnants where stl_interfaces was brought in from. - Documentation is needed to explain when the extra template parameter is required. BEMAN_ITERATOR_INTERFACE26_USE_DEDUCING_THIS() should probably also be mentioned in that context. The intent is to address these once the overall approach has consensus. Fixes bemanproject#10 and depends on bemanproject#12.
neatudarius
approved these changes
Oct 19, 2024
Member
Author
|
Weird, it looks like clang18 has deducing this support, but isn't advertising it with the |
Member
Author
|
I added a workaround for it and filed a bug report. The build now succeeds on clang 18 and 14 and fails on clang 17 and gcc 13, which is expected. |
camio
added a commit
to camio/iterator_interface26
that referenced
this pull request
Oct 21, 2024
Added a build option, BEMAN_ITERATOR_INTERFACE26_USE_DEDUCING_THIS, that switches between the deducing this implementation and that used by stl_interfaces. It defaults to whether the compiler supports deducing this. This element of configuration is placed in a generated config.hpp. This was chosen, as opposed to using a define passed as a compiler flag, to avoid ODR violations with larger projects with incoherent flag usage. The `iterator_interface_access` struct was moved to its own header since it is needed by both the deducing this and stl_interfaces implementations. The stl_interfaces implementation was modifed to use this so it better conforms to the paper. Some cleanup is still needed before this gets merged in: - There are optional26 remnants where stl_interfaces was brought in from. - Documentation is needed to explain when the extra template parameter is required. BEMAN_ITERATOR_INTERFACE26_USE_DEDUCING_THIS() should probably also be mentioned in that context. The intent is to address these once the overall approach has consensus. Fixes bemanproject#10 and depends on bemanproject#12.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This improves the user experience when incompatible flags are used. It
also is a first step toward resolving #10.