[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

In https://reviews.llvm.org/D45476#1087446, @cfe-commits wrote:

> I think you and Richard agreed that you weren’t going to synthesize a whole
>  expression tree at every use of the operator, and I agree with that
>  decision. That’s very different from what I’m asking you to do, which is to
>  synthesize in isolation a call to the copy-constructor.


Perhaps. My apologies. I'm still quite new to the Clang internals. I appreciate your patience.

> There are several places in the compiler that require these implicit copies which aren’t just
>  normal expressions; this is the common pattern for handling them. The
>  synthesized expression can be emitted multiple times, and it can be freely
>  re-synthesized in different translation units instead of being serialized.

I'm not sure this is always the case. For example:

  // foo.h
  #include <compare>
  
  struct Foo {
    int x;
  };
  inline auto operator<=>(Foo const& LHS, Foo const& RHS) {
  
  }
  // foo.cpp
  #include <foo.h> // imported via module.
  auto bar(Foo LHS, Foo RHS) {
    return 
  }



> You’re already finding and caching a constructor; storing a
>  CXXConstructExpr is basically thr same thing, but in a nicer form that
>  handles more cases and doesn’t require as much redundant code in IRGen.

I'm not actually caching the copy constructor. And I disagree that storing a
`CXXConstructExpr` is essentially the same thing. I can lookup the `CXXConstructorDecl` without `Sema`,
but I can't build a `CXXConstructExpr` without it.

> STLs *frequently* make use of default arguments on copy constructors (for
>  allocators). I agree that it’s unlikely that that would happen here, but
>  that’s precisely because it’s unlikely that this type would ever be
>  non-trivial.
> 
> Mostly, though, I don’t understand the point of imposing a partial set of
>  non-conformant restrictions on the type. It’s easy to support an arbitrary
>  copy constructor by synthesizing a CXXConstructExpr, and this will
>  magically take care of any constexpr issues, as well as removing the need
>  for open-coding a constructor call.
> 
> The constexpr issues are that certain references to constexpr variables of
>  literal type (as these types are likely to be) are required to not ODR-use
>  the variable but instead just directly produce the initializer as the
>  expression result.  That’s especially important here because (1) existing
>  STL binaries will not define these variables, and we shouldn’t create
>  artificial deployment problems for this feature, and (2) we’d really rather
>  not emit these expressions as loads from externally-defined variables that
>  the optimizer won’t be able to optimize.
> 
> John.


https://reviews.llvm.org/D45476