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

[#455] If constructor arguments fail to resolve, propagate failure #456

Merged
merged 1 commit into from
Dec 28, 2023

Conversation

davmac314
Copy link
Contributor

This change prevents invalid partial specialisations from being chosen when
instantiating a template in cases where the expression for the type/value of a template parameter involves a constructor call.

A test is included.

@davmac314 davmac314 changed the title [#599] If constructor arguments fail to resolve, propagate failure [#455] If constructor arguments fail to resolve, propagate failure Jul 12, 2023
@davmac314
Copy link
Contributor Author

Latest push corrects the issue number in commit comment, no code change from previous.

@jonahgraham
Copy link
Member

Thanks for the PR - for now I make the same comment as #448 (comment)

@jonahgraham jonahgraham added the language C/C++ Language Support label Jul 19, 2023
@davmac314
Copy link
Contributor Author

Rebased on current main.

… failure

This change prevents invalid partial specialisations from being chosen
when
instantiating a template in cases where the expression for the
type/value of a template parameter involves a constructor call.
@i-garrison
Copy link
Contributor

This change looks good and functions as advertised.

@ghentschke
Copy link
Contributor

@jonahgraham Could this be merged?

Copy link
Member

@jonahgraham jonahgraham left a comment

Choose a reason for hiding this comment

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

At recent CDT calls we discussed how to handle reviews where there is insufficient expertise to review amongst existing committers and decided to accept such changes that look reasonable and appear sufficiently tested.

@jonahgraham jonahgraham merged commit be3a2eb into eclipse-cdt:main Dec 28, 2023
5 checks passed
@jonahgraham
Copy link
Member

Thank you @davmac314 for this improvement!

@jonahgraham jonahgraham added this to the 11.5.0 milestone Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language C/C++ Language Support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants