<div dir="ltr">Woops. Submitted that last comment too early. Editing it on Phab.</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 4, 2018 at 2:31 AM, Eric Fiselier via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">EricWF added a comment.<br>
<span class=""><br>
In <a href="https://reviews.llvm.org/D45476#1087446" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D45476#1087446</a>, @cfe-commits wrote:<br>
<br>
> I think you and Richard agreed that you weren’t going to synthesize a whole<br>
>  expression tree at every use of the operator, and I agree with that<br>
>  decision. That’s very different from what I’m asking you to do, which is to<br>
>  synthesize in isolation a call to the copy-constructor.<br>
<br>
<br>
</span>Perhaps. My apologies. I'm still quite new to the Clang internals. I appreciate your patience.<br>
<span class=""><br>
> There are several places in the compiler that require these implicit copies which aren’t just<br>
>  normal expressions; this is the common pattern for handling them. The<br>
>  synthesized expression can be emitted multiple times, and it can be freely<br>
>  re-synthesized in different translation units instead of being serialized.<br>
<br>
</span>I'm not sure this is always the case. For example:<br>
<br>
  // foo.h<br>
  #include <compare><br>
<br>
  struct Foo {<br>
    int x;<br>
  };<br>
  inline auto operator<=>(Foo const& LHS, Foo const& RHS) {<br>
<br>
  }<br>
  // foo.cpp<br>
  #include <foo.h> // imported via module.<br>
  auto bar(Foo LHS, Foo RHS) {<br>
    return <br>
<span class="">  }<br>
<br>
<br>
<br>
> You’re already finding and caching a constructor; storing a<br>
>  CXXConstructExpr is basically thr same thing, but in a nicer form that<br>
>  handles more cases and doesn’t require as much redundant code in IRGen.<br>
<br>
</span>I'm not actually caching the copy constructor. And I disagree that storing a<br>
`CXXConstructExpr` is essentially the same thing. I can lookup the `CXXConstructorDecl` without `Sema`,<br>
but I can't build a `CXXConstructExpr` without it.<br>
<span class=""><br>
> STLs *frequently* make use of default arguments on copy constructors (for<br>
>  allocators). I agree that it’s unlikely that that would happen here, but<br>
>  that’s precisely because it’s unlikely that this type would ever be<br>
>  non-trivial.<br>
> <br>
> Mostly, though, I don’t understand the point of imposing a partial set of<br>
>  non-conformant restrictions on the type. It’s easy to support an arbitrary<br>
>  copy constructor by synthesizing a CXXConstructExpr, and this will<br>
>  magically take care of any constexpr issues, as well as removing the need<br>
>  for open-coding a constructor call.<br>
> <br>
> The constexpr issues are that certain references to constexpr variables of<br>
>  literal type (as these types are likely to be) are required to not ODR-use<br>
>  the variable but instead just directly produce the initializer as the<br>
>  expression result.  That’s especially important here because (1) existing<br>
>  STL binaries will not define these variables, and we shouldn’t create<br>
>  artificial deployment problems for this feature, and (2) we’d really rather<br>
>  not emit these expressions as loads from externally-defined variables that<br>
>  the optimizer won’t be able to optimize.<br>
> <br>
> John.<br>
<br>
<br>
</span><a href="https://reviews.llvm.org/D45476" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D45476</a><br>
<br>
<br>
<br>
</blockquote></div><br></div>