On Wed, May 28, 2025 at 02:14:06PM -0400, Patrick Palka wrote:
On Tue, 27 May 2025, Nathaniel Shead wrote:
On Wed, Nov 27, 2024 at 11:45:40AM -0500, Patrick Palka wrote:
On Fri, 8 Nov 2024, Nathaniel Shead wrote:
Does this approach seem reasonable? I'm pretty sure that the way I've
handled the templating here is unideal but I'm not sure what a neat way
to do what I'm trying to do here would be; any comments are welcome.
Clever approach, I like it!
-- >8 --
Currently, concept failures of standard type traits just report
'expression X<T> evaluates to false'. However, many type traits are
actually defined in terms of compiler builtins; we can do better here.
For instance, 'is_constructible_v' could go on to explain why the type
is not constructible, or 'is_invocable_v' could list potential
candidates.
That'd be great improvement.
As a first step to supporting that we need to be able to map the
standard type traits to the builtins that they use. Rather than adding
another list that would need to be kept up-to-date whenever a builtin is
added, this patch instead tries to detect any variable template defined
directly in terms of a TRAIT_EXPR.
To avoid false positives, we ignore any variable templates that have any
specialisations (partial or explicit), even if we wouldn't have chosen
that specialisation anyway. This shouldn't affect any of the standard
library type traits that I could see.
You should be able to tsubst the TEMPLATE_ID_EXPR directly and look at
its TI_PARTIAL_INFO in order to determine which (if any) partial
specialization was selected. And if an explicit specialization was
selected the resulting VAR_DECL will have DECL_TEMPLATE_SPECIALIZATION
set.
...[snip]...
If we substituted the TEMPLATE_ID_EXPR as a whole we could use the
DECL_TI_ARGS of that IIUC?
Thanks for your comments, they were very helpful. Here's a totally new
approach which I'm much happier with. I've also removed the "disable in
case any specialisation exists" logic, as on further reflection I don't
imagine this to be the kind of issue I thought it might have been.
With this patch,
template <typename T>
constexpr bool is_default_constructible_v = __is_constructible(T);
template <typename T>
concept default_constructible = is_default_constructible_v<T>;
static_assert(default_constructible<void>);
now emits the following error:
test.cpp:6:15: error: static assertion failed
6 | static_assert(default_constructible<void>);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
test.cpp:6:15: note: constraints not satisfied
test.cpp:4:9: required by the constraints of ‘template<class T> concept
default_constructible’
test.cpp:4:33: note: ‘void’ is not default constructible
4 | concept default_constructible = is_default_constructible_v<T>;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
There's still a lot of improvements to be made in this area, I think:
- I haven't yet looked into updating the specific diagnostics emitted by
the traits; I'd like to try to avoid too much code duplication with
the implementation in cp/semantics.cc. (I also don't think the manual
indentation at the start of the message is particularly helpful?)
For is_xible / is_convertible etc, perhaps they could use a 'complain'
parameter that they propagate through instead of always passing tf_none,
similar to build_invoke? Then we can call those predicates directly
from diagnose_trait_expr with complain=tf_error so that they elaborate
why they failed.
Done; for is_xible I ended up slightly preferring a 'bool explain'
(since it doesn't really make sense to talk about "complaining" for a
predicate?) but happy to swap over if that's more consistent.
Agreed about the extra indentation
- The message doesn't print the mapping '[with T = void]'; I tried a
couple of things but this doesn't currently look especially
straight-forward, as we don't currently associate the args with the
normalised atomic constraint of the declaration.
Maybe we can still print the
note: the expression ‘normal<T> [with T = void]’ evaluated to ‘false’
note alongside the extended diagnostics? Which would mean moving the
maybe_diagnose_standard_trait call a bit lower in
diagnose_atomic_constraint.
This would arguably make the diagnostic even noiser, but IMHO the
parameter mapping is an important piece of information to omit.
Agreed, can always look at condensing things later but I think this is a
good improvement.
- Just generally I think there's still a lot of noise in the diagnostic,
and I find the back-to-front ordering of 'required by...' confusing.
Agreed in general, but in the case of these type trait diagnostics I
think noisiness is kind of inevitable especially if we start explaining
in detail why the trait is unsatisified (as e.g. PR117294 requests).
Thanks for your comments! With this patch, the following example:
#include <type_traits>
#include <concepts>
struct S {
S(int);
};
static_assert(std::is_nothrow_constructible_v<S, int>);
template <std::regular T> void foo(T t);
int main() {
foo(S{});
}
with '-fdiagnostics-set-output=text:experimental-nesting=yes' now emits:
test.cpp:6:20: error: static assertion failed
6 | static_assert(std::is_nothrow_constructible_v<S, int>);
| ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
• ‘S’ is not nothrow constructible from ‘int’, because
• ‘S::S(int)’ is not ‘noexcept’
test.cpp:4:3:
4 | S(int);
| ^
test.cpp: In function ‘int main()’:
test.cpp:9:6: error: no matching function for call to ‘foo(S)’
9 | foo(S{123});
| ~~~^~~~~~~~
• there is 1 candidate
• candidate 1: ‘template<class T> requires regular<T> void foo(T)’
test.cpp:7:32:
7 | template <std::regular T> void foo(T t);
| ^~~
• template argument deduction/substitution failed:
• constraints not satisfied
In file included from test.cpp:2:
• /home/wreien/.local/include/c++/16.0.0/concepts: In substitution of
‘template<class T> requires regular<T> void foo(T) [with T = S]’:
• required from here
test.cpp:9:6:
9 | foo(S{123});
| ~~~^~~~~~~~
• required for the satisfaction of ‘constructible_from<_Tp>’ [with
_Tp = S]
/home/wreien/.local/include/c++/16.0.0/concepts:161:13:
161 | concept constructible_from
| ^~~~~~~~~~~~~~~~~~
• required for the satisfaction of ‘default_initializable<_Tp>’
[with _Tp = S]
/home/wreien/.local/include/c++/16.0.0/concepts:166:13:
166 | concept default_initializable = constructible_from<_Tp>
| ^~~~~~~~~~~~~~~~~~~~~
• required for the satisfaction of ‘semiregular<_Tp>’ [with _Tp = S]
/home/wreien/.local/include/c++/16.0.0/concepts:282:13:
282 | concept semiregular = copyable<_Tp> &&
default_initializable<_Tp>;
| ^~~~~~~~~~~
• required for the satisfaction of ‘regular<T>’ [with T = S]
/home/wreien/.local/include/c++/16.0.0/concepts:356:13:
356 | concept regular = semiregular<_Tp> &&
equality_comparable<_Tp>;
| ^~~~~~~
• the expression ‘is_constructible_v<_Tp, _Args ...> [with _Tp = S;
_Args = {}]’ evaluated to ‘false’
/home/wreien/.local/include/c++/16.0.0/concepts:162:30:
162 | = destructible<_Tp> && is_constructible_v<_Tp,
_Args...>;
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
• ‘S’ is not default constructible, because
• error: no matching function for call to ‘S::S()’
• there are 3 candidates
• candidate 1: ‘S::S(int)’
test.cpp:4:3:
4 | S(int);
| ^
• candidate expects 1 argument, 0 provided
• candidate 2: ‘constexpr S::S(const S&)’
test.cpp:3:8:
3 | struct S {
| ^
• candidate expects 1 argument, 0 provided
• candidate 3: ‘constexpr S::S(S&&)’
• candidate expects 1 argument, 0 provided
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
-- >8 --
Currently, concept failures of standard type traits just report
'expression X<T> evaluates to false'. However, many type traits are
actually defined in terms of compiler builtins; we can do better here.
For instance, 'is_constructible_v' could go on to explain why the type
is not constructible, or 'is_invocable_v' could list potential
candidates.
Apart from concept diagnostics, this is also useful when using such
traits in a 'static_assert' directly, so this patch also adjusts the
diagnostics in that context.
As a first step to supporting that we need to be able to map the
standard type traits to the builtins that they use. Rather than adding
another list that would need to be kept up-to-date whenever a builtin is
added, this patch instead tries to detect any variable template defined
directly in terms of a TRAIT_EXPR.
This patch also adjusts 'diagnose_trait_expr' to provide more helpful
diagnostics for these cases. Not all type traits have yet been updated,
this patch just updates those that seem particularly valuable or
straight-forward. The function also gets moved to cp/semantics.cc to be
closer to 'trait_expr_value'.
