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

Fix 12371: C++20: Requires clause on a constructor #5972

Merged
merged 2 commits into from
Feb 17, 2024

Conversation

pfultz2
Copy link
Contributor

@pfultz2 pfultz2 commented Feb 12, 2024

No description provided.

@chrchr-github
Copy link
Collaborator

chrchr-github commented Feb 12, 2024

We have other issues with requires too. Should we simplify (i.e. remove) those clauses somehow?

@pfultz2
Copy link
Contributor Author

pfultz2 commented Feb 12, 2024

Should we simplify (i.e. remove) those clauses somehow?

How would we simplify them? If we remove them then we cant apply checkers to the clause.

@chrchr-github
Copy link
Collaborator

Not applying checkers (which don't exist yet) seems better than throwing various syntax errors. What errors could we detect in a requires clause anyway?

@pfultz2
Copy link
Contributor Author

pfultz2 commented Feb 17, 2024

Not applying checkers (which don't exist yet)

There are checkers that already exist but may need some tweaking to detect the issue.

What errors could we detect in a requires clause anyway?

If you wrote something like requires std::integral<T> || std::integral<T> we could detect the same expression on both sides.

@chrchr-github
Copy link
Collaborator

If you wrote something like requires std::integral<T> || std::integral<T> we could detect the same expression on both sides.

Fair enough. I wonder though how many places need to be tweaked so that requires is handled correctly.

@chrchr-github chrchr-github merged commit 20b3792 into danmar:main Feb 17, 2024
64 checks passed
@pfultz2 pfultz2 deleted the error-requires-true branch February 17, 2024 22:26
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.

3 participants