Hi.
While investigating a fix for C++/PR115030 (a bug in constrained auto
deduction), I was wondering why we are not substituting constraint args of an
auto node in tsubst (pt.cc:16533). Instead, this substitution is delayed until
do_auto_deduction (pt.cc), where we attempt to find the substituted args of the
enclosing scope. At that point, it becomes difficult to find partially
specialised args, as they are erased in the process. I propose modifying tsubst
to eagerly substitute the constraint args of an auto node.
By making this change, we would not need to provide outer_targs for
do_auto_deduction in cases where tsubst has been called for the type, which
covers most scenarios. However, we still need outer_targs for cases like:
template <typename T, C<T> auto V>
struct S { };
Hence, outer_targs cannot be completely removed but will be set to TREE_NULL in
all calls except the one in convert_template_argument (pt.cc:8788).
Additionally, the tmpl argument of do_auto_deduction, which helps to provide
outer args in the scope, will no longer be necessary and can be safely
removed.
Also, the trimming hack proposed in
https://gcc.gnu.org/pipermail/gcc-patches/2024-June/654724.html to address
C++/PR114915 is no longer needed. We will add an assertion to ensure that the
missing levels do not become negative.
Substituting constraint arguments earlier will slightly alter error messages
(see testsuite/g++.dg/cpp2a/concepts-placeholder3.C as an example).
To summarise, I have made the following changes:
- Modified tsubst to substitute the constraint args.
- Fixed shallow checks for using the cache from TEMPLATE_TYPE_DESCENDANTS
(pt.cc:16513).
- Substituted the constraint args and the auto node itself, while retaining all
other parameters as is (pt.cc:16533).
- Removed now unnecessary code that attempted to find outer scope template info
and args in various locations.
- Updated the missing levels hack (pt.cc:31320) to work with the substituted
constraints.
- Used level instead of orig_level to find the missing levels.
- Added an assertion for future safety.
Details of these changes can be found in the patch "[PATCH v1] c++: Eagerly
substitute auto constraint args in tsubst [PR115030]" that will be replied to
this thread.
This patch, in my opinion, improves code quality by removing an argument from
do_auto_deduction and eliminating the out-of-scope task of finding outer_targs
within do_auto_deduction. It simplifies the usage of do_auto_deduction and
resolves C++/PR115030 without complex calculations for specialised args. It
also enhances the consistency of tsubst behaviour by not leaving constraints
un-substituted. However, these changes make args in constraints_satisfied_p
(being called from do_auto_deduction) a bit misleading, as it will not carry
the actual args of the constraint and can even be an empty vec.
I have added a testsuite for C++/PR115030, and as far as I have tested (only
c++ dg.exp on x86_64-linux), there are no regressions.
Extra:
While doing this, I also realised that the hash function in ctp_hasher (for
canonical type of template parameters) slightly differs from its equality
function. Specifically, the equality function uses comptypes (typeck.cc) (with
COMPARE_STRUCTURAL) and compares placeholder constraints (typeck.cc:1586),
while the hash function ignores them (pt.cc:4528). As a result, we can have two
types with equal hashes that are unequal. For example, assuming:
template <typename T, typename U>
concept C1 = ...
template <typename T, typename U>
concept C2 = ...
"C1<U> auto" and "C2<V> auto" have the same hash value but are unequal. This
issue does not cause any error (it is a hash collision and it is handled), but
it can be avoided by using hash_placeholder_constraint (constraint.cc:1150).
Therefore, I have made the following changes:
- Fixed the hash function to calculate the hash of the constraint (pt.cc:4528).
- Slightly modified the existing hash_placeholder_constraint
(constraint.cc:1150) to accept the initial hash value as an argument.
Details of these changes can be found in the patch "[PATCH v1] c++: Hash
placeholder constraint in ctp_hasher" that will be replied to this email.
Thanks. I really appreciate your comments and feedback on these proposed
changes.
