This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PR c++/20103] failure to gimplify constructors for addressabletypes


Alexandre Oliva wrote:

So think of it this way: if we adopted COMPOUND_LITERAL_EXPR like
you're inclined to do, we'd have to do the very same kind of type
propagation after resolving the complete type of the initializer.
This is no different from what I'm proposing to do here.

Except that (a) we'd have no change of doing it when we don't want to do it (which we do with your patch, because if the two things just happen to have the same type before substitution, we'll preserve that, even if we shouldn't, perhaps for some case/language feature/extension that hasn't even been invented yet), and (b) we'd be guaranteed to keep doing it when we do want it to happen (which we won't, with your patch, if something changes in get_target_expr).


And there's more: since there's no compiler-enforced equality AFAICT
between the type of the COMPOUND_LITERAL_EXPR and that of the VAR_DECL
enclosed in it, we'd have o make the propagation conditional in just
the same way.

No, because there would be no TARGET_EXPR. In a template, you would just have a COMPOUND_LITERAL_EXPR, with no TARGET_EXPR, because we want a syntactic representation of the input program.


Note that we never needed this condition to hold before; TARGET_EXPRs
just were not handled at all: we never emitted TARGET_EXPRs whose
types were arrays of unknown bounds, to be inferred from the
initializer length, before this change.

Yes, and introducing TARGET_EXPRs into templates *is a bug* because in templates we want a syntactic representation. The closest thing we have to a syntactic representation of a compound literal is a CONSTRUCTOR; it's certainly not a TARGET_EXPR wrapped around a CONSTRUCTOR. It may be just fine to use CONSTRUCTOR, instead of introducing COMPOUND_LITERAL_EXPR, but TARGET_EXPRs should not be appearing here at all.


Further investigation has shown that TARGET_EXPR's SLOTs always have
the same type as the enclosing TARGET_EXPR.  Whether SLOTs and
INITIALs always have the same type is not as obvious, but it appears
to hold as well.  How about making the assignments unconditional,
then, with an assertion that the equality holds?

Copying the SLOT type to the EXPR type would make a lot more sense. That's along the lines of what I originally suggested: substitute into the operands, and then form the TARGET_EXPR with an appropriate type. You've just demonstrated a way to do that, in a uniform way.


Unfortunately, you've also caused me to think about this long enough to realize that having the TARGET_EXPR here is wrong in the first place, as per above.

/* If the type of the initializer was used to create the original
   TARGET_EXPR, make sure we adjust the type of the tsubsted
   TARGET_EXPR, should the type of the initializer change in
   unpredictable ways during tsubsting (e.g., the range of an array is
   inferred from a CONSTRUCTOR length).  */

See?  No need to change any other piece of code anywhere else.  It's
really that simple.

That's not a justification; that's just a statement of what the code does. Why would we copy this type sometimes and not others?


Fundamentally, what your patch says is that the semantics of TARGET_EXPR depend on whether or not the type of INITIAL matches the type of the TARGET_EXPR. If that weren't true, then we wouldn't have to check that condition before doing whatever we do. But, there's nothing about the actual semantics of TARGET_EXPR that depend on this type equality; ergo, the patch does not make sense.

But, as I've said above, I think this is all moot; TARGET_EXPRs should not appear in template bodies at all. I think you've confirmed that by saying that you had to add this code. If you look at the other code that handles expressions in templates, you'll see how we preserve the syntactic form, precisely so that we can reuse the appropriate semantics.c routines at the time of instantiation.

--
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]