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

Prevent calls to get_child with temporary default values #115

Conversation

ashtum
Copy link
Collaborator

@ashtum ashtum commented Nov 19, 2023

Resolves #42

@ashtum
Copy link
Collaborator Author

ashtum commented Nov 19, 2023

@pdimov, do you have any suggestions on how to add a test to verify that get_child doesn't compile with temporary default values? Can I use a concept and include it as a test for C++20 and higher? (although it doesn't look elegant)

@pdimov
Copy link
Member

pdimov commented Nov 19, 2023

Use a compile-fail test.

@ashtum
Copy link
Collaborator Author

ashtum commented Nov 19, 2023

Use a compile-fail test.

Just out of curiosity, is there any alternative way to test it without using a separate compilation unit? I mean, in complex scenarios, the compilation failure could be due to other reasons as well.

@pdimov
Copy link
Member

pdimov commented Nov 19, 2023

There probably is but I wouldn't recommend it.

@pdimov
Copy link
Member

pdimov commented Nov 19, 2023

Something like https://godbolt.org/z/j7MaTGTcd.

@ashtum
Copy link
Collaborator Author

ashtum commented Nov 19, 2023

Something like https://godbolt.org/z/j7MaTGTcd.

Do you discourage it because of the possibility of the expression becoming invalid for other reasons and not manifesting itself in the test? If that's the case, can't we mitigate this by also testing valid expressions, like so:

static_assert(mp_valid<accepts_default_of_type, ptree&>::value, "");
static_assert(mp_valid<accepts_default_of_type, const ptree&>::value, "");

@pdimov
Copy link
Member

pdimov commented Nov 19, 2023

It's too complicated, requires specialized knowledge to understand, and is therefore hard to maintain.

@ashtum ashtum force-pushed the delete-get_child-overloads-that-accept-temporaries branch 5 times, most recently from a569269 to fda247a Compare November 20, 2023 10:59
@ashtum ashtum force-pushed the delete-get_child-overloads-that-accept-temporaries branch from fda247a to 5438bf7 Compare November 20, 2023 11:03
@ashtum ashtum requested a review from pdimov November 20, 2023 12:27
@pdimov pdimov merged commit fc51b0e into boostorg:develop Nov 20, 2023
55 checks passed
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.

Passing temporary object as default value to get_child
3 participants