[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/lib/Sema/SemaStmt.cpp:3150-3153
+  // If we got a non-deduced auto ReturnType, we are in a dependent context and
+  // there is no point in allowing copy elision since we won't have it deduced
+  // by the point the VardDecl is instantiated, which is the last chance we have
+  // of deciding if the candidate is really copy elisible.
----------------
mizvekov wrote:
> mizvekov wrote:
> > rsmith wrote:
> > > How does this happen? Are there any cases where we could do NRVO or should do an implicit move that are blocked by this?
> > > 
> > > It seems to me that we should (nearly always) be able to work out whether copy elision is possible here without knowing the deduced type:
> > > -- if the return type is //cv// `auto` then it will always be deduced to the type of the returned variable, so we can always perform copy elision
> > > -- if the return type is `decltype(auto)`, then we can perform copy elision if the expression is unparenthesized and otherwise cannot; we could perhaps track whether the expression was parenthesized in `NRVOResult`, and can conservatively disallow copy elision if we don't know (eg, from template instantiation, where we're only looking at the variable and not the return statements)
> > > -- if the return type is anything else involving `auto`, it can't possibly instantiate to a class type, so we'll never perform copy elision
> > Yeah, what you suggested is what I tried on a previous patch in this DR, but then studying the NRVO tracker carefully I thought about this counter example:
> > ```
> > template<bool B> static auto bar() {
> >   {                                 
> >     Foo foo;                        
> >     if constexpr(B)                 
> >       return foo;                  
> >   }                                 
> >   {                                 
> >     Bar bar;                        
> >     if constexpr(!B)                
> >       return bar;                   
> >   }                                 
> > }                                   
> > ````
> > 
> > Since we run the tracker before instantiation, we would see both return statements and mark both foo and bar as NRVO variables.
> > Ofcourse in the B = false case, we would end up constructing a Foo in a Bar return slot....
> > 
> > As a side note, It is actually funny that we currently perform this optimization (most likely accidentally):
> > ```
> > template<bool B> static Foo bar() {
> >   {                                 
> >     Foo foo1;                        
> >     if constexpr(B)                 
> >       return foo1;                  
> >   }                                 
> >   {
> >     Foo foo2;
> >     return foo2                                 
> >   }                                 
> > }
> > ```
> > In the B = false case, we end up constructing foo1 in the return slot even though we actually never return it.
> Compiler explorer link for the accidental optimization I am talking about: https://godbolt.org/z/czdWodW87
That's amazing, and makes total sense. Yeah, `if constexpr` pretty thoroughly destroys the assumptions we make in the `Scope`-based NRVO computation about what a templated function will look like after instantiation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99696/new/

https://reviews.llvm.org/D99696