f6a164819 We don't want to throw lvalue-to-rvalue casts (...) of certain types in C++ (was: Allow implicit copy constructor (...))

John, I kindly ask for an explanation of following change introduced by you in f6a164819:

git diff f6a164819^! lib/Sema/SemaExpr.cpp

+ // We don't want to throw lvalue-to-rvalue casts on top of
+ // expressions of certain types in C++.
+ if (getLangOptions().CPlusPlus &&
+ (E->getType() == Context.OverloadTy ||
+ T->isDependentType() ||
+ T->isRecordType()))
+ return;

The problem with this change is that I explicitly request to use C style assignment (ignoring constructors) on POD type variable having `pod_assign` attribute, but condition above prevents proper cast to be emitted.

So far I have simply disabled, since I no longer have direct access to the Entity (VarDecl) to check for PODAssignAttrt, but I am not sure what are consequences of such step.

FYI:

https://github.com/ujhpc/clang/commit/61823c2bc5be78f793bdc8c11365d2bcb50455cc

+++ b/lib/Sema/SemaInit.cpp
@@ -4599,6 +4599,10 @@ void InitializationSequence::InitializeFrom(Sema &S,
       
   // - If the destination type is a (possibly cv-qualified) class type:
   if (DestType->isRecordType()) {
+ if (Entity.getDecl()->hasAttr<PODAssignAttr>()) {
+ AddCAssignmentStep(DestType);
+ return;
+ }

Regards,

John, I kindly ask for an explanation of following change introduced by
you in f6a164819:

git diff f6a164819^! lib/Sema/SemaExpr.cpp

+ // We don't want to throw lvalue-to-rvalue casts on top of
+ // expressions of certain types in C++.
+ if (getLangOptions().CPlusPlus &&
+ (E->getType() == Context.OverloadTy ||
+ T->isDependentType() ||
+ T->isRecordType()))
+ return;

The problem with this change is that I explicitly request to use C style
assignment (ignoring constructors) on POD type variable having `pod_assign`
attribute, but condition above prevents proper cast to be emitted.

You're trying to shoehorn non-C++ semantics into C++, so you're going to
have a bad time. The above check is appropriate for C++ (CK_LValueToRValue
is not something that should ever happen to a record type in C++, because
that's not what the lvalue-to-rvalue conversion does on record types).

It is correct language behavior that, in C++, default l-value conversions — dropping cv-qualifiers and changing the value kind — don’t generally apply to expressions of class type. We rely on this extensively through the compiler; in particular, we assume that non-POD record types will always be copied by an CXXOperatorCallExpr or CXXConstructExpr. So removing this will cause assertions in a number of places, and if you simply disable all those assertions, you’ll massively break C++ semantics.

I don’t think you actually need to change this in Sema, though, unless it’s critical that you don’t trigger instantiation of copy constructors and/or assignment operators. It should be fine to proceed through type-checking normally, maybe marking a few copy expressions as special (e.g. the same way that CXXConstructExprs are marked as elidable, you could alternatively mark them as implementable with memcpy) and then adding special-case logic to IR-generation.

You’re just planning to do this for your edification, right? I'd have strong reservations about casually accepting work like this into trunk; changing type behavior like this has some pretty serious language design repercussions. What does it mean, exactly, to use C-style assignment on a specific variable? Does it only affect initializations of that variable, or does it also affect assignments to it, destruction of it, reads out of it, etc.? Does your use case really require direct language support, and if so, is there a less invasive change that would let you do the rest in a library?

John.

For the record there's already an active thread on this where alternatives have been suggested (Adam, can you keep the discussion in one place so as not to duplicate effort?)

Top tip: If you want to see what a piece of code is for, try removing that code and running the tests. The failing tests will indicate which feature it was supporting without having to go to the author.

Alp.

For the record there's already an active thread on this where alternatives have been suggested (Adam, can you keep the discussion in one place so as not to duplicate effort?)

Apparently I did something wrong when I was trying to just change the subject but reference original thread. I am sorry for this duplication.

Top tip: If you want to see what a piece of code is for, try removing that code and running the tests. The failing tests will indicate which feature it was supporting without having to go to the author.

Okay. Thanks, gonna try that first next time.

So let's move back to original thread.

Regards,

You’re just planning to do this for your edification, right?

Yes, that was the point.

Cheers,