EricWF added inline comments.

================
Comment at: lib/CodeGen/CGExprAgg.cpp:1002
+  return EmitFinalDestCopy(
+      E->getType(), CGF.MakeNaturalAlignAddrLValue(Select, E->getType()));
+}
----------------
EricWF wrote:
> rjmccall wrote:
> > Is there something in Sema actually validating that the comparison types is 
> > trivially copyable?  Because EmitFinalDestCopy does not work on non-trivial 
> > types.
> > 
> > Also, are we certain that we're allowed to do a copy from an actual global 
> > variable here instead of potentially a constexpr evaluation of the variable 
> > reference?  And if we are doing a copy, are we registering ODR-uses of all 
> > the variables in Sema?
> > 
> > I don't think you should worry about trying to generate a select between 
> > the "actual" comparison result values.  At least not yet.
> There is nothing is Sema validating that these comparison types are trivially 
> copyable, and according to the standard they don't need to be.
> I assumed we only ended up in `CGExprAgg` if the types were trivially 
> copyable. But I'll look into implementing this for non-trivially copyable 
> comparison types (although we'll probably never actually encounter them).
> 
> > Also, are we certain that we're allowed to do a copy from an actual global 
> > variable here instead of potentially a constexpr evaluation of the variable 
> > reference?
> 
> I'm not sure exactly what this means. How would I observe the difference?
> 
> >And if we are doing a copy, are we registering ODR-uses of all the variables 
> >in Sema?
> 
> Probably not. I'll double check that.
> 
> > I don't think you should worry about trying to generate a select between 
> > the "actual" comparison result values. At least not yet.
> 
> I'm not sure exactly what you mean by this.
To follow up:

>> And if we are doing a copy, are we registering ODR-uses of all the variables 
>> in Sema?
>
> Probably not. I'll double check that.

We do mark the potential result variables as ODR-used when we first check them 
when building builtin and defaulted comparison operators.

>> I don't think you should worry about trying to generate a select between the 
>> "actual" comparison result values. At least not yet.
>
> I'm not sure exactly what you mean by this.

Sorry, I see what you mean. You're talking about the comment. Richard asked me 
to leave that TODO there.


https://reviews.llvm.org/D45476



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATC... Eric Fiselier via Phabricator via cfe-commits
  • [PATC... Richard Smith - zygoloid via Phabricator via cfe-commits
  • [PATC... Richard Smith - zygoloid via Phabricator via cfe-commits
  • [PATC... Eric Fiselier via Phabricator via cfe-commits
  • [PATC... Eric Fiselier via Phabricator via cfe-commits
  • [PATC... Eric Fiselier via Phabricator via cfe-commits
  • [PATC... Eric Fiselier via Phabricator via cfe-commits
  • [PATC... Richard Smith - zygoloid via Phabricator via cfe-commits
  • [PATC... John McCall via Phabricator via cfe-commits
  • [PATC... Eric Fiselier via Phabricator via cfe-commits
  • [PATC... Eric Fiselier via Phabricator via cfe-commits
  • [PATC... John McCall via Phabricator via cfe-commits
  • [PATC... Eric Fiselier via Phabricator via cfe-commits
    • ... John McCall via cfe-commits
      • ... Mailing List "cfe-commits" via Phabricator via cfe-commits
  • [PATC... Eric Fiselier via Phabricator via cfe-commits
    • ... Eric Fiselier via cfe-commits
  • [PATC... John McCall via Phabricator via cfe-commits
  • [PATC... Eric Fiselier via Phabricator via cfe-commits
  • [PATC... Richard Smith - zygoloid via Phabricator via cfe-commits
  • [PATC... John McCall via Phabricator via cfe-commits

Reply via email to