On 11/24/23 20:14, waffl3x wrote:
OKAY, I figured out SOMETHING, I think this should be fine. As noted in
the comments, this might be a better way of handling the static lambda
case too. This is still a nasty hack so it should probably be done
differently, but I question if making a whole new fntype node in
tsubst_lambda_expr makes sense. On the other hand, maybe there will be
problems if a lambda is already instantiated? I'm not sure, mutating
things this way makes me uneasy though.
A problem with changing TYPE_METHOD_BASETYPE for a FUNCTION_TYPE is that
all equivalent FUNCTION_TYPE share the same tree node (through
type_hash_canon), so you're messing with the type of unrelated functions
at the same time. I think it's better to stick with the way static
lambdas are handled.
I don't like that pt.cc feels like it has a ton of hidden mutations,
it's really hard to follow through it. Would you agree it's in need for
cleanup or am I just not experienced enough in this area yet?
I'm sure there are things that could use cleaning up, but I'm not
thinking of anything specific offhand. Any particular examples?
Regarding the error handling, I just had a thought about it, I have a
hunch it definitely needs to go in tsubst_template_decl or
tsubst_function_decl. There might need to be more changes to determine
the actual type of the lambda in there, but everything else I've done
changes the implicit object argument to be treated more like a regular
argument, doing error handling for the object type in
tsubst_lambda_expr would be inconsistent with that.
But the rule about unrelated type is specific to lambdas, so doing it in
tsubst_lambda_expr seems preferable to adding a lambda special case to
generic code.
The other problem I'm having is
auto f0 = [n = 5, &m](this auto const&){ n = 10; };
This errors just fine, the lambda is unconditionally const so
LAMBDA_EXPR_MUTABLE_P flag is set for the closure.
This on the other hand does not. The constness of the captures depends
on (I assume) LAMBDA_EXPR_MUTABLE_P so all the captures are non-const
here.
auto f1 = [n = 5](this auto&& self){ n = 10; };
as_const(f1)();
That sounds correct, why would this be an error?
The constness of the captures doesn't depend on LAMBDA_EXPR_MUTABLE_P,
it depends on the type of the object parameter, which in this case is
non-const, so so are the captures.
I was going to close out this message there but I just realized why
exactly you think erroring in instantiate_body is too late, it's
because at that point an error is an error, while if we error a little
bit earlier during substitution it's not a hard error. I'm glad I
realized this now, because I am much more confident in how to implement
the errors for unrelated type now. I'm still a little confused on what
the exact protocol for emitting errors at this stage is, as there
aren't many explicit errors written in. It seems to me like the errors
are supposed to be emitted when calling back into non-template stages
of the compiler with substituted types. It also seems like that hasn't
always been strictly followed, and I hate to contribute to code debt
but I'm not sure how I would do it in this case, nor if I actually have
a valid understanding of how this all works.
Most errors that could occur in template substitution are guarded by if
(complain & tf_error) so that they aren't emitted during deduction
substitution. It's better if those are in code that's shared with the
non-template case, but sometimes that isn't feasible.
One side note before I close up here, I don't like when *_P macros are
used to set flags. Is this something else we should clean up in the future?
I don't think so, a wholesale renaming would just confuse existing
developers.
I'm beginning to
wonder if an overhaul that gets rid of the macros from the public
interface is a good idea. I'm reluctant to suggest that as I've really
warmed up to the macros a lot. They are used in a consistent and easy
to understand way which is highly unlike the bad uses of macros that
I've seen before that really obscure what's actually going on. But they
are still macros, so maybe moving away from them is the right call,
especially since there has started to be a mix-up of macros and
functions for the same purposes. I'm mildly of the opinion that only
one style should be used (in the public interface) because mixing them
causes confusion, it did for me anyway. Perhaps I should open a thread
on the general mail list and see what others think, get some input
before I decide which direction to go with it. To be clear, when I say
getting rid of macros, I want to emphasize I mean only in the public
interface, I don't see any value in getting rid of macros under the
hood as the way the checking macros are implemented is already really
good and works. It would only cause problems to try to move away from
that. I think I'll probably start to mess around with this idea as soon
as this patch is done.
There has been some movement from macros toward inline functions since
the switch to C++, but that also has complications with
const-correctness, declaration order, and bit-fields. It's probably
good to use inlines instead of larger macros when feasible, but the
accessors should probably stay macros.
Jason
