-
Notifications
You must be signed in to change notification settings - Fork 0
Fix single element tuple like handling #71
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
Open
yaito3014
wants to merge
53
commits into
main
Choose a base branch
from
fix-single-element-tuple-like-handling
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 16 commits
Commits
Show all changes
53 commits
Select commit
Hold shift + click to select a range
a653da4
Rename and drop aliases
yaito3014 6b0c90f
Add test
yaito3014 f4430ee
Move single element tuple like traits
yaito3014 cfe7cb6
Unwrap single element tuple like in `string_parse`
yaito3014 6a17b00
Add test
yaito3014 a75a454
Fix test
yaito3014 2aa10ab
Unwrap single element tuple recursively
yaito3014 9e240ee
Add special handling for rule
yaito3014 bc08648
Fix
yaito3014 1461752
Revise single element tuple like type handling
yaito3014 765fe91
Add tests
yaito3014 d333ca9
Suppress MSVC warning
yaito3014 41e0f0c
Split test case
yaito3014 c022926
Reduce stack usage
yaito3014 f05a9e6
Add newline at end
yaito3014 d1bcdaf
Unwrap single element tuple like once in rule parser
yaito3014 114486d
Fix indentation
yaito3014 59c5661
Use remove_cvref_t consistently
yaito3014 7ff873e
Remove deprecated overload of `move_to`
yaito3014 acd02df
Add notes for AI generated content
saki7 fa0cb78
Remove unneeded branch from `pass_sequence_attribute`
yaito3014 38fb7fc
Add comment and adjust noexcept
yaito3014 f8401c3
Reorganize includes
yaito3014 0e7dc8c
Styling fix
yaito3014 e11c5a7
Remove unused forward declaration
yaito3014 14eea5f
Remove `pass` from `partition_attribute`
yaito3014 e05a682
Remove unneeded metafunctions
yaito3014 308f9d8
Use metafunction to simplify constexpr if
yaito3014 2a11631
Remove unused include
yaito3014 f15176e
Rename `SET` to `SES`
yaito3014 ebc71c2
Rename to clearer name and resolve noexcept issue
yaito3014 42627b0
Fix comment
yaito3014 5a3b28e
Add variant case
yaito3014 2b5d184
Use `is_convertible_without_narrowing`
yaito3014 bf81ed2
Resolve several issues
yaito3014 3f80fe5
Pass original attribute to `on_success`
yaito3014 ff1886a
Add static_assertion that checks narrowing assignment
yaito3014 15d3f80
Merge branch 'main' into fix-single-element-tuple-like-handling
yaito3014 ae16031
Refine static_assert of `partition_attribute`
yaito3014 34261db
Minor styling fix
yaito3014 ab7b0ef
Use more accurate type to detect narrowing
yaito3014 7567791
Add narrowing check test
yaito3014 2480d49
Change `can_hold` primary definition to `is_assignable`
yaito3014 483439d
Update iris
yaito3014 00c88bc
Update iris
yaito3014 80ce6f0
Move `is_assignable_without_narrowing` into submodule
yaito3014 a3dcecc
Revert "Move `is_assignable_without_narrowing` into submodule"
yaito3014 4b5c385
Update iris
yaito3014 6fc643f
Rename
yaito3014 511efa6
Introduce `is_lossy_assignment` customization point and rename narrow…
yaito3014 8d4c51d
Adjust comment styling
yaito3014 ba098db
Reorder and add tests
yaito3014 f70cb12
Remove redundant static_assert
yaito3014 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -178,6 +178,18 @@ struct rule_impl | |
|
|
||
| It start = first; // backup | ||
|
|
||
| auto&& unwrapped_attr = [&]() -> decltype(auto) { | ||
| if constexpr (traits::is_single_element_tuple_like<RHSAttr>::value) { | ||
| if constexpr (traits::can_hold<typename parser_traits<RHS>::attribute_type, alloy::tuple_element_t<0, RHSAttr>>::value) { | ||
| return alloy::get<0>(rhs_attr); | ||
| } else { | ||
| return rhs_attr; | ||
| } | ||
| } else { | ||
| return rhs_attr; | ||
| } | ||
| }(); | ||
|
|
||
|
Comment on lines
+192
to
+199
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to emit |
||
| // | ||
| // NOTE: The branches below are intentionally written verbosely to make sure | ||
| // we have the minimal call stack. DON'T extract these procedures into a | ||
|
|
@@ -187,21 +199,21 @@ struct rule_impl | |
|
|
||
| bool ok; | ||
| if constexpr (SkipDefinitionInjection || !is_default_parse_rule) { | ||
| ok = rhs.parse(first, last, rcontext, rhs_attr); | ||
| ok = rhs.parse(first, last, rcontext, unwrapped_attr); | ||
|
|
||
| } else { | ||
| // If there is no `IRIS_X4_DEFINE` for this rule, | ||
| // we'll make a context for this rule tagged by its `RuleID` | ||
| // so we can extract the rule later on in the default | ||
| // `parse_rule` overload. | ||
| auto const rule_id_context = x4::make_context<RuleID>(rhs, rcontext); | ||
| ok = rhs.parse(first, last, rule_id_context, rhs_attr); | ||
| ok = rhs.parse(first, last, rule_id_context, unwrapped_attr); | ||
| } | ||
|
|
||
| if constexpr (need_on_success<It, RContext, RHSAttr>) { | ||
| if (ok) { | ||
| x4::skip_over(start, first, rcontext); | ||
| RuleID{}.on_success(std::as_const(start), std::as_const(first), rcontext, rhs_attr); | ||
| RuleID{}.on_success(std::as_const(start), std::as_const(first), rcontext, unwrapped_attr); | ||
| return true; | ||
| } | ||
|
|
||
|
|
@@ -390,6 +402,7 @@ struct rule : parser<rule<RuleID, RuleAttr, ForceAttr>> | |
| static_assert(X4Attribute<RuleAttr>); | ||
| static_assert(X4UnusedAttribute<RuleAttr> || !std::is_const_v<RuleAttr>, "Rule attribute cannot be const qualified"); | ||
| static_assert(!std::is_same_v<std::remove_const_t<RuleAttr>, unused_container_type>, "`rule` with `unused_container_type` is not supported"); | ||
| static_assert(!is_ttp_specialization_of<RuleAttr, alloy::tuple>::value, "alloy::tuple is intended for internal use only"); | ||
|
|
||
| using id = RuleID; | ||
| using attribute_type = RuleAttr; | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this forward declaration, the corresponding code was removed in fa0cb78.
And please be more careful when making changes spotted by review. If you're going to remove something according to a review, you should double-check the surrounding context and apply corresponding changes.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also: in the previous review, I wrote:
If the current code is working with the rule-specific handling being removed, that means our current code magically unwraps incompatible single-element tuple-like for
class ruleand its child parsers. That should trigger narrowing conversion check, and emit assertion error on such cases.Maybe it should be handled here. #71 (comment)
We actually need some code that actually triggers the
static_assert. Since we currently don't have a test system to assert hard errors (compilation failures), I'd like to have those cases written in a commented-out code insidesingle_element_tuple_like.cpp.I will manually uncomment the test case in my local environment and check if the assertion actually works or not.