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

[mizvekov]

Summary

This expands NRVO propagation for more cases:

Parse analysis improvement:

• Lambdas and Blocks with dependent return type can have their variables marked as NRVO Candidates.

Variable instantiation improvements:

• Fixes crash when instantiating NRVO variables in Blocks.
• Functions, Lambdas, and Blocks which have auto return type have their variables' NRVO status propagated. For Blocks with non-auto return type, as a limitation, this propagation does not consider the actual return type.

This also implements exclusion of VarDecls which are references to
dependent types.

Signed-off-by: Matheus Izvekov <[e-mail]>

[• Quuxplusone]

Wow, nice catches. For posterity, here's the missed-optimization being addressed IIUC: https://godbolt.org/z/MEoKGs7cE

[mizvekov]

In D99696#2662816, @Quuxplusone wrote:

Wow, nice catches. For posterity, here's the missed-optimization being addressed IIUC: https://godbolt.org/z/MEoKGs7cE

Thanks! And again thank you for providing test cases, it's what I planned to start doing next :)

[mizvekov]

fix clang-tidy warning.

[mizvekov]

Fixes for auto&, decltype(auto).
Exclude candidates from functions with dependent / undeduced return types which are references.
Adjusts the CodeGen tests from D99225.

[mizvekov]

Rebased.

[mizvekov]

.

[mizvekov]

• small Doxygen fix.

[aaronpuchert]

Didn't really check for correctness yet, just a superficial review.

I like the idea, splitting the functionality up definitely helps understanding this.

clang/include/clang/Sema/Sema.h

4741

Nitpick: I'd go with != None here.

clang/lib/Sema/SemaCoroutine.cpp

1585–1587

Perhaps this should just be direct initialization? Can't really find anything in the standard though.

clang/lib/Sema/SemaExprCXX.cpp

854

Any reason why you've moved that away from the comment that wants to explain it?

clang/lib/Sema/SemaStmt.cpp

3029–3031

The way you've designed the enumeration, couldn't you use std::min here?

3037

The second backslash seems superfluous.

3053–3057

These comments seem to be separated from their context now...

3054

Braces are usually omitted if both branches are single statements.

3068

Just remove "Return false if VD is a __block variable." No need to repeat the code, what follows is important.

3102

Can we maybe move this function up one place to make the diff a bit smaller? It appears to contain the first part of the original getCopyElisionCandidate.

3135

NRVOResult seems to be small, why not make this a proper function and let it return the result?

3151

parenthesized

3152–3154

Can't we just do this when we know what it deduces to? It sounds weird to handle dependent contexts here because we shouldn't attempt any return value initialization then.

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

1062

llvm_unreachable

[mizvekov]

clang/lib/Sema/SemaCoroutine.cpp

1585–1587

So I just checked this again. Regarding our conversation on IRC, what I said initially was correct: ReturnValue is always a member function expression, built by makeReturnObject -> buildPromiseCall -> buildMemberCall. So implicit move would never trigger here, and as far as I see there is no reason for this code to have used PerformMoveOrCopyInitialization in the first place.

clang/lib/Sema/SemaExprCXX.cpp

854

Yes, on C++2b this might modify Ex, and I moved it so this would happen before we do the check with the type of the expression just below here.

clang/lib/Sema/SemaStmt.cpp

3029–3031

Yes exactly!
The previous version of this patch I submitted used exactly that, but then for some reason I thought people could find std::min a bit odd here, and so I changed it.
But now I think I was right the first time around :)

3135

It does use the result parameter as input and output. This function can only "downgrade" a result it received previously.
I could make it receive a result and return the result, but then would probably just use it like this:

NRVORes = updNRVOResultWithRetType(NRVORes, ...);

Do you think that looks clearer?

[mizvekov]

clang/lib/Sema/SemaCoroutine.cpp

1585–1587

Err I meant: member function *call* expression

[mizvekov]

clang/lib/Sema/SemaExprCXX.cpp

854

But yeah I forgot to move the comment, good catch.

[mizvekov]

clang/lib/Sema/SemaCoroutine.cpp

1585–1587

Also, I don't think we could force direct initialization here, if the object returned by Gro is volatile for example.
With that said, copy elision from the sema action on the return statement of get_return_object should take care of removing this copy here, I think, I don't see any reason it would not work here just as well as for expressions coming from the parser.

clang/lib/Sema/SemaExprCXX.cpp

854

Actually, had my wires crossed there with the P2266 DR, I only need to move this line there.

clang/lib/Sema/SemaStmt.cpp

3152–3154

I think there are problems regarding the limitations of the current NRVO algorithm, but I had not had much time to study it or think of improvements, so I follow the current approach of trying to use as much available information as possible in order to eliminate candidates early.

With that said, this is not well covered by the test suite and I could probably get away with a lot less without breaking any existing tests.

[mizvekov]

• Addresses aaronpuchert's review points.

[mizvekov]

clang/lib/Sema/SemaStmt.cpp

3053–3057

This one comment here is talking about the line just below it.
Any other examples where it seems separated?

[aaronpuchert]

clang/lib/Sema/SemaStmt.cpp

3029–3031

Perhaps you can just add a comment to the enumeration saying that the values are totally ordered with regards to implication so that ⇒ is >=.

3053–3057

I meant the context of the comment itself. Originally this was quoting a clause

// - in a return statement in a function [where] ...
// ... the expression is the name of a non-volatile automatic object ...

and then additional parts later. Now you're having that initial part in a different function and it's not immediately clear what you're quoting here.

To be clear, this was about "(other than a function ... parameter)" below.

3135

Yes, that would seem more natural to me. Technically a caller could decide to not use the same variable in both places, for example it could pass a temporary or pass the result on directly.

[mizvekov]

clang/lib/Sema/SemaStmt.cpp

3135

But for this function, I don't see how it could make sense to actually use it like that.

You are either using it in a context where you don't have a ReturnType (throw, co_return), and you only care about the initial NRVOResult and don't call this function at all, or you are in a return statement, where you will call this function passing the initial NRVOResult, which will then have no further utility.

[• Quuxplusone]

clang/lib/Sema/SemaStmt.cpp

3135

I haven't looked at the code, but FWIW, I'm always in favor of more "function-style" functions. Which would you rather deal with — void square(int& x) or int square(int x)? The same rationale applies uniformly everywhere. Even if we are "consuming" the argument in this particular instance, that might change after another few years of maintenance; and regardless, it might be easier for the maintainer to understand "function-style" code even if they aren't going to change it.

[aaronpuchert]

clang/lib/Sema/SemaCoroutine.cpp

1585–1587

In InitializationKind there is a distinction between IK_Copy (basically T x = init;) and IK_Direct (basically T x(init);). I don't know how this would be related to volatile, but it is related to explicit.

[mizvekov]

• Removes code that tried to allow copy elision for functions with dependent 'auto' return types. The reason is explained in new comments. Will try to address these in future work.
• Addresses aaronpuchert's review points.

[mizvekov]

clang/lib/Sema/SemaStmt.cpp

3135

I changed the function name and made it return instead the candidate itself in case it was copy elisible, which allowed simplifying some other expressions afterwards.

But in answer to the points made in this thread: I will generally prefer the syntax which mostly matches the intended usage, making it easier to do the right thing and harder to do the wrong thing.

This function implements a fragment of the C++ standard text in a very specific context within clang. It is called from two places.
It's not a major piece of infrastructure or anything like that, to be concerned with unforeseen possible future uses :)

[rsmith]

clang/include/clang/Sema/Sema.h

4734–4741

We generally use FooResult to mean "either a Foo or an indicator that an error has been diagnosed". Would the name NRVOInfo work instead?

Actually, I think the "O" here is misleading, because we're not talking about performing the optimization here, just about what variable was named in a return statement. And I think calling this "NRVO" is a bit misleading because it also covers implicit move, which isn't traditionally part of NRVO. So how about NamedReturnValueInfo or NamedReturnInfo or NamedReturnValue or ReturnValueName or something like that?

clang/lib/Sema/SemaStmt.cpp

3048–3049

I find this name a little unclear (what do we mean by "downgrade"?). Can we call this something a bit more specific, such as disallowCopyElision or disallowNRVO?

3104

This comment needs re-flowing.

3110

3116–3119

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

3118–3119

3122

Please add braces rather than using a comma expression here and below.

3269

Should we skip this if NRVORes.isMoveEligible() && getLangOpts().CPlusPlus20? In that case, we already tried an unrestricted move a couple of lines ago, so we know that adding std::move won't help. (I think we can still hit the "adding std::move will help" case in C++20, but only in obscure corners such as a volatile-qualified local variable.)

3449

I find the pre-existing NRVOCandidate != nullptr more readable than this !!NRVOCandidate.

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

1059

Can you use getCurBlock()->FunctionType for this? We won't have a Scope, but we should still have a ScopeInfo.

[mizvekov]

clang/lib/Sema/SemaStmt.cpp

3116–3119

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.

[mizvekov]

clang/lib/Sema/SemaStmt.cpp

3116–3119

Compiler explorer link for the accidental optimization I am talking about: https://godbolt.org/z/czdWodW87

[rsmith]

clang/lib/Sema/SemaStmt.cpp

3116–3119

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.

[mizvekov]

• Address rsmith review comments.

[mizvekov]

clang/include/clang/Sema/Sema.h

4734–4741

I like NamedReturnInfo, will change.

clang/lib/Sema/SemaStmt.cpp

3048–3049

How about demoteNRVOStatus?

3122

Using short utility lambda as an alternative to keep the main logic clear from clutter.

3269

This goes to the documentation on getNamedReturnInfo, return value, where it says that the Canidate member will be non-null in case implicit move would be permitted under the most permissive language standard.

So if Try

So if we are guarded under that condition, we would never

3269

Skipping it is just a small compilation performance optimization:

We are going to try that TryMoveInitialization again just below, this time with equivalent of ForceCXX20 = true. If we were already in c++20 mode, then that second call is going to be made with basically the same parameters and perform basically the same work and reach the same conclusion: Faliure, and the diagnostic won't be produced.

Now my take on why that looks confusing: To me TryMoveInitialization has a similar problem to the one I addressed in this patch for getCopyElisionCandidate, it relies on you calling it multiple times, once to try the the actual work, and if that fails, then you have to call it again, this time in 'permissive' more, to get the information you need for the diagnostics.

Now I did not try giving it the same treatment of returning an aggregate with all the information you need, so you can call it just once with one less parameter.
If you agree on that take, I'd be happy to try a similar improvement there as well.
If you don't think it's worth the noise, then it's all good, no worries :)

As a side note, I just noticed that the
if (QT->isLValueReferenceType()) { below is dead code, we are never going to consider this move eligible in any mode, and so Res.Candidate is going to be null.

We should not need to repeat here the checks we already do in getNRVOResult, so I am going to go ahead and remove it.

I think it may have been added because until just recently getCopyElisionCandidate was bugged and accepted LValue references, and I guess the problem was "fixed" in this diagnostic first.

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

1059

Thanks a lot! That works and now we get copy elision on dependent blocks :)

[mizvekov]

clang/lib/Sema/SemaStmt.cpp

3269

Disregard this comment, I realized I was answering the wrong question and for some reason it was not discarded.

[• Quuxplusone]

clang/lib/Sema/SemaStmt.cpp

3048–3049

I'm with Richard here: if you mean "disallow copy elision," say so!
IMO this should be member functions of NamedReturnInfo, and it should just be

void disallowCopyElision() {
    S &= ~CopyElidable;  // obviously this "should" work;
}                    // ensure that your enum values actually make it work

void disallowImplicitMove() {
    S = None;
}

The way you use it below is equivalent to e.g.

if (VD->isExceptionVariable()) {
  Info.disallowCopyElision();
  if (!hasCXX20)
    Info.disallowImplicitMove();
}

which I think is a lot easier to puzzle out what's going on and how it corresponds to the standardese.

3065–3066

Nit: please declare variables one-per-line. Change that , to a ; and write bool again on line 3085 instead of four space characters.

[mizvekov]

clang/lib/Sema/SemaStmt.cpp

3048–3049

@Quuxplusone But I did not mean "disallow copy elision", the function will also disallow implicit move based on a parameter, so would be a bit misleading.

That solution you proposed though is more repetitive and prone to error: it does not enforce the invariant that you should always disallow copy elision when disallowing implicit move.
Even if you amend that to having one manipulator function which enforces the invariant, you still have two bools totaling four states, where only three states make sense.

I would consider a better name though, not necessarily happy with this one either.

[mizvekov]

clang/lib/Sema/SemaStmt.cpp

3048–3049

Also I did consider making this a method, but this function is more of an implementation detail that is better to hide anyway.

[mizvekov]

@rsmith:

Actually, there is still something not quite right with this patch that I have to finish investigating.

The B5 test should not have gotten copy elision. I thought at first that we had gotten the function return type deduced during VarDecl instantiation, but we actually have a dependant type instead:

dump of FT:

FunctionProtoType 0x18597a3ed10 '<dependent type> (void)' dependent cdecl
`-BuiltinType 0x185960144e0 '<dependent type>' dependent

I think I assumed wrong that these blocks would behave similarly to lambdas with auto return.

[mizvekov]

• Changed the downgrade* function name to disallowNRVO, given second thought, I think it's appropriate name.
• Change to one variable declaration per statement as per Arthur's review.

[aaronpuchert]

clang/lib/Sema/SemaStmt.cpp

3048–3049

You can have more aptly named functions without dropping the invariant. I'd say you only need disallowCopyElision though, instead of disallowImplicitMove you could just directly overwrite with None.

3065–3066

I don't think the coding standards have a rule about this. I tend to advocate the opposite in most cases to avoid repetition, but for bool it's probably fine to repeat.

3109–3110

So the return value is trivially deducible from the post-state of Info (as Info.isCopyElidable() ? Info.Candidate : nullptr)? I guess it makes some code shorter to write, but generally it's preferable to have fewer implicit invariants.

Also it's a getter function that modifies its arguments... not saying there isn't precedent in the code, but I'd say less surprising function signatures are still better where we can have them.

[mizvekov]

clang/lib/Sema/SemaStmt.cpp

3048–3049

But the dominant use case here is wanting to choose either one based on the language mode. So the signature taking a bool is more convenient.

[• Quuxplusone]

clang/lib/Sema/SemaStmt.cpp

3048–3049

Just to be clear, I don't see the use-case anywhere as wanting to "choose" anything based on a parameter. As my code indicated, I see the logic here as being simply "For exception variables [or whatever], disallow copy elision. _Additionally_, pre-'20, disallow even implicit move for them." And so on. No functions of boolean parameters, just straightforward English if-thens, based on the English in the standard.
I get that you're probably not going to write the straightforward code in the end; but I just wanted to push a tiny bit harder to see if anyone else sees what I see. :)

[aaronpuchert]

clang/lib/Sema/SemaStmt.cpp

3048–3049

Your argument makes sense to me, but the function is called 5 times with a runtime parameter, so it's going to be quite a bit more code.

just straightforward English if-thens, based on the English in the standard.

Technically aren't we handling different standards at once here? So there is actually no language in any standard that contains these particular if-thens.

[mizvekov]

• Added doc to disallowNRVO
• Also detect implicit return type for blocks.

[mizvekov]

Ping.

[• Quuxplusone]

@mizvekov, my understanding is:

• D99696 was temporarily blocked on D103720, but now D103720 is landed and D99696 is unblocked
• D99696 is a codegen improvement in all language modes, and is also blocking D99005
• D99005 is blocking our getting experience with p2266 for C++23

IMHO we should just plow ahead here. @aaronpuchert what's your take on D99696 at this point; is it good to go or still unresolved issues?

[mizvekov]

In D99696#2808592, @Quuxplusone wrote:

@mizvekov, my understanding is:

• D99696 was temporarily blocked on D103720, but now D103720 is landed and D99696 is unblocked
• D99696 is a codegen improvement in all language modes, and is also blocking D99005
• D99005 is blocking our getting experience with p2266 for C++23

IMHO we should just plow ahead here. @aaronpuchert what's your take on D99696 at this point; is it good to go or still unresolved issues?

No, actually, D99696 (This DR) was not blocked on any other DRs, I just put it as parent here for my convenience, since I had them all stacked on my workstation to make my life easier.
Sorry if that caused confusion and anyone thought it was blocked....

D99005 (The P2266 implementation) is blocked just on the present DR, but that was because there are common changes and I decided to do them in this order since it was not clear when P2266 impl could be merged.

D103720 was a dependency for D100733 proposed by @rsmith, which is a dependency of D100000 (This is the one that implements the 'almost xvalue' thing that Jason Merril was pushing on the reflector, in order to replace the two-phase overload resolution).

[aaronpuchert]

In D99696#2808592, @Quuxplusone wrote:

@aaronpuchert what's your take on D99696 at this point; is it good to go or still unresolved issues?

Overall this looks sensible to me, and I don't really have anything to add. Parts of the code seem a bit complex, but that just seems to reflect the complexity of the standard(s).

[stella.stamenova]

It looks like this caused a couple of failures on the Windows LLDB bot due to crashes in clang:

https://lab.llvm.org/buildbot/#/builders/83/builds/7142

[aeubanks]

Crashes on a stage 2 build on Windows:

../rel/bin/clang-cl /nologo /showIncludes /Foobj/llvm/lib/MC/MCParser/MCParser.MasmParser.obj /c ../../llvm/lib/MC/MCParser/MasmParser.cpp -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_HAS_EXCEPTIONS=0 -D_UNICODE -DUNICODE -I../../llvm/include -Igen/llvm/include /Zi /FS -gline-tables-only -gcodeview-ghash /O2 /Gw /Zc:inline /EHs-c- /W4 -Wno-unused-parameter -Wdelete-non-virtual-dtor -Wstring-conversion -no-canonical-prefixes -Werror=date-time -fmsc-version=1916 /Brepro /winsysroot../../../sysroot -Wcovered-switch-default /GR-
Assertion failed: isa<X>(Val) && "cast<Ty>() argument of incompatible type!", file ../../llvm/include/llvm/Support/Casting.h, line 262
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.      Program arguments: ../rel/bin/clang-cl /nologo /showIncludes /Foobj/llvm/lib/MC/MCParser/MCParser.MasmParser.obj /c ../../llvm/lib/MC/MCParser/MasmParser.cpp -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_HAS_EXCEPTIONS=0 -D_UNICODE -DUNICODE -I../../llvm/include -Igen/llvm/include /Zi /FS -gline-tables-only -gcodeview-ghash /O2 /Gw /Zc:inline /EHs-c- /W4 -Wno-unused-parameter -Wdelete-non-virtual-dtor -Wstring-conversion -no-canonical-prefixes -Werror=date-time -fmsc-version=1916 /Brepro /winsysroot../../../sysroot -Wcovered-switch-default /GR-
1.      <eof> parser at end of file
2.      ../../../sysroot\VC\Tools\MSVC\14.16.27023\include\ostream:320:36: instantiating function definition 'std::basic_ostream<char>::operator<<'
3.      ../../../sysroot\VC\Tools\MSVC\14.16.27023\include\xlocale:503:26: instantiating function definition 'std::use_facet<std::num_put<char>>'
4.      ../../../sysroot\VC\Tools\MSVC\14.16.27023\include\xlocnum:1417:35: instantiating function definition 'std::num_put<char>::_Getcat'
5.      ../../../sysroot\VC\Tools\MSVC\14.16.27023\include\xlocnum:1446:21: instantiating function definition 'std::num_put<char>::num_put'
6.      ../../../sysroot\VC\Tools\MSVC\14.16.27023\include\xlocnum:1504:36: instantiating function definition 'std::num_put<char>::do_put'
7.      ../../../sysroot\VC\Tools\MSVC\14.16.27023\include\xlocale:503:26: instantiating function definition 'std::use_facet<std::numpunct<char>>'
8.      ../../../sysroot\VC\Tools\MSVC\14.16.27023\include\xlocnum:163:16: instantiating function definition 'std::numpunct<char>::_Getcat'
9.      ../../../sysroot\VC\Tools\MSVC\14.16.27023\include\xlocnum:157:2: instantiating function definition 'std::numpunct<char>::numpunct'
10.     ../../../sysroot\VC\Tools\MSVC\14.16.27023\include\xlocnum:199:7: instantiating function definition 'std::numpunct<char>::_Init'
11.     ../../../sysroot\VC\Tools\MSVC\14.16.27023\include\xlocale:662:19: instantiating function definition 'std::_Maklocstr<char>'
 #0 0x00007ff7a6b48596 HandleAbort /b/f/w/llvm-project/build/rel/../../llvm/lib/Support/Windows/Signals.inc:408:0
 #1 0x00007ff7a9cee690 raise C:\src\llvm-project\build\rel\minkernel\crts\ucrt\src\appcrt\misc\signal.cpp:547:0
 #2 0x00007ff7a9ce3fd0 abort C:\src\llvm-project\build\rel\minkernel\crts\ucrt\src\appcrt\startup\abort.cpp:71:0
 #3 0x00007ff7a9ce4672 common_assert_to_stderr<wchar_t> C:\src\llvm-project\build\rel\minkernel\crts\ucrt\src\appcrt\startup\assert.cpp:186:0
 #4 0x00007ff7a9ce451a _wassert C:\src\llvm-project\build\rel\minkernel\crts\ucrt\src\appcrt\startup\assert.cpp:443:0
 #5 0x00007ff7a8899933 llvm::PointerUnion<const clang::Type *,const clang::ExtQuals *>::isNull /b/f/w/llvm-project/build/rel/../../llvm/include/llvm/ADT/PointerUnion.h:172:0
 #6 0x00007ff7a8899933 clang::QualType::isNull /b/f/w/llvm-project/build/rel/../../clang/include/clang/AST/Type.h:734:0
 #7 0x00007ff7a8899933 clang::QualType::getCommonPtr /b/f/w/llvm-project/build/rel/../../clang/include/clang/AST/Type.h:684:0
 #8 0x00007ff7a8899933 clang::QualType::getTypePtr /b/f/w/llvm-project/build/rel/../../clang/include/clang/AST/Type.h:6428:0
 #9 0x00007ff7a8899933 llvm::simplify_type<clang::QualType>::getSimplifiedValue /b/f/w/llvm-project/build/rel/../../clang/include/clang/AST/Type.h:1318:0
#10 0x00007ff7a8899933 llvm::simplify_type<const clang::QualType>::getSimplifiedValue /b/f/w/llvm-project/build/rel/../../llvm/include/llvm/Support/Casting.h:48:0
#11 0x00007ff7a8899933 llvm::isa_impl_wrap<clang::FunctionType,const clang::QualType,const clang::Type *>::doit /b/f/w/llvm-project/build/rel/../../llvm/include/llvm/Support/Casting.h:121:0
#12 0x00007ff7a8899933 llvm::isa /b/f/w/llvm-project/build/rel/../../llvm/include/llvm/Support/Casting.h:142:0
#13 0x00007ff7a8899933 llvm::cast /b/f/w/llvm-project/build/rel/../../llvm/include/llvm/Support/Casting.h:263:0
#14 0x00007ff7a8899933 clang::TemplateDeclInstantiator::VisitVarDecl(class clang::VarDecl *, bool, class llvm::ArrayRef<class clang::BindingDecl *> *) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1110:0
#15 0x00007ff7a88b10f8 clang::Sema::SubstDecl::<lambda_0>::operator() /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:3837:0
#16 0x00007ff7a88b10f8 llvm::function_ref<void ()>::callback_fn<`lambda at ../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:3836:49'> /b/f/w/llvm-project/build/rel/../../llvm/include/llvm/ADT/STLExtras.h:177:0
#17 0x00007ff7a72917fd clang::Sema::runWithSufficientStackSpace(class clang::SourceLocation, class llvm::function_ref<(void)>) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/Sema.cpp:463:0
#18 0x00007ff7a88a9dce clang::Sema::SubstDecl(class clang::Decl *, class clang::DeclContext *, class clang::MultiLevelTemplateArgumentList const &) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:3839:0
#19 0x00007ff7a8a11c2d `anonymous namespace'::TemplateInstantiator::TransformDefinition /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiate.cpp:1253:0
#20 0x00007ff7a8a11c2d clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformDeclStmt /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/TreeTransform.h:7603:0
#21 0x00007ff7a89e3a61 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformStmt /b/f/w/llvm-project/build/rel/gen/clang/include/clang/AST/StmtNodes.inc:97:0
#22 0x00007ff7a8a05ab8 clang::ActionResult<clang::Stmt *,1>::isInvalid /b/f/w/llvm-project/build/rel/../../clang/include/clang/Sema/Ownership.h:207:0
#23 0x00007ff7a8a05ab8 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformCompoundStmt /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/TreeTransform.h:7209:0
#24 0x00007ff7a8a11450 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformCompoundStmt /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/TreeTransform.h:7192:0
#25 0x00007ff7a89e39a0 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformStmt /b/f/w/llvm-project/build/rel/gen/clang/include/clang/AST/StmtNodes.inc:73:0
#26 0x00007ff7a89e36a4 llvm::DenseMap<clang::Decl *,clang::Decl *,llvm::DenseMapInfo<clang::Decl *>,llvm::detail::DenseMapPair<clang::Decl *,clang::Decl *> >::~DenseMap /b/f/w/llvm-project/build/rel/../../llvm/include/llvm/ADT/DenseMap.h:755:0
#27 0x00007ff7a89e36a4 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::~TreeTransform /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/TreeTransform.h:101:0
#28 0x00007ff7a89e36a4 clang::Sema::SubstStmt(class clang::Stmt *, class clang::MultiLevelTemplateArgumentList const &) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiate.cpp:3463:0
#29 0x00007ff7a88ac61a clang::Sema::InstantiateFunctionDefinition(class clang::SourceLocation, class clang::FunctionDecl *, bool, bool, bool) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:4961:0
#30 0x00007ff7a88af1ac clang::FunctionDecl::isDefined /b/f/w/llvm-project/build/rel/../../clang/include/clang/AST/Decl.h:2084:0
#31 0x00007ff7a88af1ac clang::Sema::PerformPendingInstantiations(bool) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:6207:0
#32 0x00007ff7a88ac6fa clang::Sema::InstantiateFunctionDefinition(class clang::SourceLocation, class clang::FunctionDecl *, bool, bool, bool) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:4986:0
#33 0x00007ff7a88af1ac clang::FunctionDecl::isDefined /b/f/w/llvm-project/build/rel/../../clang/include/clang/AST/Decl.h:2084:0
#34 0x00007ff7a88af1ac clang::Sema::PerformPendingInstantiations(bool) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:6207:0
#35 0x00007ff7a88ac6fa clang::Sema::InstantiateFunctionDefinition(class clang::SourceLocation, class clang::FunctionDecl *, bool, bool, bool) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:4986:0
#36 0x00007ff7a88af1ac clang::FunctionDecl::isDefined /b/f/w/llvm-project/build/rel/../../clang/include/clang/AST/Decl.h:2084:0
#37 0x00007ff7a88af1ac clang::Sema::PerformPendingInstantiations(bool) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:6207:0
#38 0x00007ff7a88ac6fa clang::Sema::InstantiateFunctionDefinition(class clang::SourceLocation, class clang::FunctionDecl *, bool, bool, bool) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:4986:0
#39 0x00007ff7a88af1ac clang::FunctionDecl::isDefined /b/f/w/llvm-project/build/rel/../../clang/include/clang/AST/Decl.h:2084:0
#40 0x00007ff7a88af1ac clang::Sema::PerformPendingInstantiations(bool) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:6207:0
#41 0x00007ff7a88ac6fa clang::Sema::InstantiateFunctionDefinition(class clang::SourceLocation, class clang::FunctionDecl *, bool, bool, bool) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:4986:0
#42 0x00007ff7a88af1ac clang::FunctionDecl::isDefined /b/f/w/llvm-project/build/rel/../../clang/include/clang/AST/Decl.h:2084:0
#43 0x00007ff7a88af1ac clang::Sema::PerformPendingInstantiations(bool) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:6207:0
#44 0x00007ff7a88ac6fa clang::Sema::InstantiateFunctionDefinition(class clang::SourceLocation, class clang::FunctionDecl *, bool, bool, bool) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:4986:0
#45 0x00007ff7a88af1ac clang::FunctionDecl::isDefined /b/f/w/llvm-project/build/rel/../../clang/include/clang/AST/Decl.h:2084:0
#46 0x00007ff7a88af1ac clang::Sema::PerformPendingInstantiations(bool) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:6207:0
#47 0x00007ff7a88ac6fa clang::Sema::InstantiateFunctionDefinition(class clang::SourceLocation, class clang::FunctionDecl *, bool, bool, bool) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:4986:0
#48 0x00007ff7a88af1ac clang::FunctionDecl::isDefined /b/f/w/llvm-project/build/rel/../../clang/include/clang/AST/Decl.h:2084:0
#49 0x00007ff7a88af1ac clang::Sema::PerformPendingInstantiations(bool) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:6207:0
#50 0x00007ff7a88ac6fa clang::Sema::InstantiateFunctionDefinition(class clang::SourceLocation, class clang::FunctionDecl *, bool, bool, bool) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:4986:0
#51 0x00007ff7a88af1ac clang::FunctionDecl::isDefined /b/f/w/llvm-project/build/rel/../../clang/include/clang/AST/Decl.h:2084:0
#52 0x00007ff7a88af1ac clang::Sema::PerformPendingInstantiations(bool) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:6207:0
#53 0x00007ff7a88ac6fa clang::Sema::InstantiateFunctionDefinition(class clang::SourceLocation, class clang::FunctionDecl *, bool, bool, bool) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:4986:0
#54 0x00007ff7a88af1ac clang::FunctionDecl::isDefined /b/f/w/llvm-project/build/rel/../../clang/include/clang/AST/Decl.h:2084:0
#55 0x00007ff7a88af1ac clang::Sema::PerformPendingInstantiations(bool) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:6207:0
#56 0x00007ff7a88ac6fa clang::Sema::InstantiateFunctionDefinition(class clang::SourceLocation, class clang::FunctionDecl *, bool, bool, bool) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:4986:0
#57 0x00007ff7a88af1ac clang::FunctionDecl::isDefined /b/f/w/llvm-project/build/rel/../../clang/include/clang/AST/Decl.h:2084:0
#58 0x00007ff7a88af1ac clang::Sema::PerformPendingInstantiations(bool) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:6207:0
#59 0x00007ff7a7293711 llvm::TimeTraceScope::~TimeTraceScope /b/f/w/llvm-project/build/rel/../../llvm/include/llvm/Support/TimeProfiler.h:86:0
#60 0x00007ff7a7293711 clang::Sema::ActOnEndOfTranslationUnitFragment(enum clang::Sema::TUFragmentKind) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/Sema.cpp:1001:0
#61 0x00007ff7a72943ac clang::Sema::ActOnEndOfTranslationUnit(void) /b/f/w/llvm-project/build/rel/../../clang/lib/Sema/Sema.cpp:1042:0
#62 0x00007ff7a98dc220 clang::Parser::ParseTopLevelDecl(class clang::OpaquePtr<class clang::DeclGroupRef> &, bool) C:\src\llvm-project\clang\lib\Parse\Parser.cpp:693:0
#63 0x00007ff7a8bfe1ae clang::ParseAST(class clang::Sema &, bool, bool) C:\src\llvm-project\clang\lib\Parse\ParseAST.cpp:157:0
#64 0x00007ff7a7378a64 clang::FrontendAction::Execute(void) C:\src\llvm-project\clang\lib\Frontend\FrontendAction.cpp:955:0
#65 0x00007ff7a6830593 llvm::Error::getPtr C:\src\llvm-project\llvm\include\llvm\Support\Error.h:273:0
#66 0x00007ff7a6830593 llvm::Error::operator bool C:\src\llvm-project\llvm\include\llvm\Support\Error.h:236:0
#67 0x00007ff7a6830593 clang::CompilerInstance::ExecuteAction(class clang::FrontendAction &) C:\src\llvm-project\clang\lib\Frontend\CompilerInstance.cpp:977:0
#68 0x00007ff7a68c5ba3 clang::ExecuteCompilerInvocation(class clang::CompilerInstance *) C:\src\llvm-project\clang\lib\FrontendTool\ExecuteCompilerInvocation.cpp:278:0
#69 0x00007ff7a67b1a81 cc1_main(class llvm::ArrayRef<char const *>, char const *, void *) /b/f/w/llvm-project/build/rel/../../clang/tools/driver/cc1_main.cpp:246:0
#70 0x00007ff7a67c4df4 ExecuteCC1Tool /b/f/w/llvm-project/build/rel/../../clang/tools/driver/driver.cpp:338:0
#71 0x00007ff7a71584e6 clang::driver::CC1Command::Execute::<lambda_1>::operator() C:\src\llvm-project\clang\lib\Driver\Job.cpp:404:0
#72 0x00007ff7a71584e6 llvm::function_ref<void ()>::callback_fn<`lambda at ../../clang/lib/Driver/Job.cpp:404:22'> C:\src\llvm-project\llvm\include\llvm\ADT\STLExtras.h:177:0
#73 0x00007ff7a6b08cf2 llvm::CrashRecoveryContext::RunSafely(class llvm::function_ref<(void)>) /b/f/w/llvm-project/build/rel/../../llvm/lib/Support/CrashRecoveryContext.cpp:233:0
#74 0x00007ff7a7158171 clang::driver::CC1Command::Execute(class llvm::ArrayRef<class llvm::Optional<class llvm::StringRef>>, class std::basic_string<char, struct std::char_traits<char>, class std::allocator<char>> *, bool *) const C:\src\llvm-project\clang\lib\Driver\Job.cpp:404:0
#75 0x00007ff7a67f2345 clang::driver::Compilation::ExecuteCommand(class clang::driver::Command const &, class clang::driver::Command const *&) const /b/f/w/llvm-project/build/rel/../../clang/lib/Driver/Compilation.cpp:196:0
#76 0x00007ff7a67f282b clang::driver::Compilation::ExecuteJobs(class clang::driver::JobList const &, class llvm::SmallVectorImpl<struct std::pair<int, class clang::driver::Command const *>> &) const /b/f/w/llvm-project/build/rel/../../clang/lib/Driver/Compilation.cpp:249:0
#77 0x00007ff7a6809107 llvm::SmallVectorBase<unsigned int>::empty C:\src\llvm-project\llvm\include\llvm\ADT\SmallVector.h:72:0
#78 0x00007ff7a6809107 clang::driver::Driver::ExecuteCompilation(class clang::driver::Compilation &, class llvm::SmallVectorImpl<struct std::pair<int, class clang::driver::Command const *>> &) C:\src\llvm-project\clang\lib\Driver\Driver.cpp:1571:0
#79 0x00007ff7a67c468e main /b/f/w/llvm-project/build/rel/../../clang/tools/driver/driver.cpp:510:0
#80 0x00007ff7a9cd11e8 invoke_main d:\agent\_work\2\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78:0
#81 0x00007ff7a9cd11e8 __scrt_common_main_seh d:\agent\_work\2\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288:0
#82 0x00007fff3e777c24 (C:\Windows\System32\KERNEL32.DLL+0x17c24)
#83 0x00007fff3ecad721 (C:\Windows\SYSTEM32\ntdll.dll+0x6d721)
clang-cl: error: clang frontend command failed due to signal (use -v to see invocation)
clang version 13.0.0
Target: x86_64-pc-windows-msvc
Thread model: posix
InstalledDir: ../rel/bin
clang-cl: note: diagnostic msg:
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang-cl: note: diagnostic msg: C:\src\tmp\MasmParser-573e32.sh
clang-cl: note: diagnostic msg:

********************
ninja: build stopped: subcommand failed.

MasmParser-44ec46.cpp7 MBDownload

MasmParser-44ec46.sh4 KBDownload

[mizvekov]

Thank you @stella.stamenova and @aeubanks for reporting and reverting this.

Turns out was a silly mistake where we were not digging down through AttributedType nodes in order to get the function type.
My bad and will try landing one more time once I do a stage2 build locally.

[mizvekov]

Look through AttributedType when obtaining FunctionDecl return type.

Adds a couple more test cases to cover this.

[hans]

We're seeing new build errors in Chromium after this (http://crbug.com/1219457). Here's a reduced example:

$ cat /tmp/a.mm
#include <utility>

struct Callback {
  // Move-only!
  Callback(const Callback&) = delete;
  Callback& operator=(const Callback&) = delete;
  Callback(Callback&&) = default;
  Callback& operator=(Callback&&) = default;

  void operator()() {}
};

void f(Callback c) {
  __block auto x = std::move(c);

  (void)^(void) {
    x();
  };
}

$ build.release/bin/clang++ -c /tmp/a.mm -target x86_64-apple-darwin10 -stdlib=libc++
/tmp/a.mm:14:16: error: call to deleted constructor of 'Callback'
  __block auto x = std::move(c);
               ^
/tmp/a.mm:5:3: note: 'Callback' has been explicitly marked deleted here
  Callback(const Callback&) = delete;
  ^
1 error generated.

I don't know enough about blocks to know whether the new error makes sense or not.

+rjmccall who knows about blocks.

[• Quuxplusone]

@hans: FYI, that looks related to the immediately following D99005, not D99696 specifically. I suspect that reverting D99005 (but leaving D99696 in trunk) would suffice to fix that symptom. But I agree I don't see why it should be happening. That block code looks reasonable to me at first glance and wasn't intended to be affected by D99005 AFAIK.
@mizvekov: See why I'm skeptical about merging big patches back-to-back and/or on Fridays? ;)

[hans]

@hans: FYI, that looks related to the immediately following D99005, not D99696 specifically.

No, my test case passes at 0d9e8f5f4b68252c6caa1ef81a30777b2f5d7242 but fails at 1e50c3d785f4563873ab1ce86559f2a1285b5678 (D99696), so this does seem to be the one that introduced the problem.

[hans]

Since it seems more discussion is needed here, I've reverted in c60dd3b2626a4d9eefd9f82f9a406b0d28d3fd72. Since they were hard to tease apart, the revert is for both D99696 and D99005.

[rjmccall]

When __block variables get moved to the heap, they're supposed to be moved if possible, not copied. It looks like PerformMoveOrCopyInitialization requires NRVO info to be passed in to ever do a move. Maybe it's being passed in wrong when building __block copy expressions in some situation.

Was there really not a test case covering those semantics?

[mizvekov]

In D99696#2817126, @rjmccall wrote:

When __block variables get moved to the heap, they're supposed to be moved if possible, not copied. It looks like PerformMoveOrCopyInitialization requires NRVO info to be passed in to ever do a move. Maybe it's being passed in wrong when building __block copy expressions in some situation.

Found the culprit, that functionality was implemented in checkEscapingByref, and it was a bit of a shocker that such a thing was just hiding in a function with such an unassuming name :)
Also surprising to find here another user of the two-step overload resolution, that also escaped our radar.
There is a C++ proposal (P2266) to replace that mechanism with something much simpler.

So since this is an ObjectiveC thing, how is the semantics of this interaction defined, just in terms of C++ NRVO?
As this reuses PerformMoveOrCopyInitialization, it is also affected by the change going into C++20 where the restriction on only converting constructors was lifted. Was this accidental and nobody noticed?

For this patch I am going to revert to the original behavior, but I think this could be revisited in the future so it gets aligned with the C++2b rules after P2266, which would be just having VarRef be an XValue unconditionally and perform the regular copy initialization.

Was there really not a test case covering those semantics?

Nope...

[mizvekov]

-bugs
+tests

[mizvekov]

@rjmccall Added some tests covering these semantics: clang/test/SemaObjCXX/block-capture.mm and also some extra documentation to checkEscapingByref. Not sure if I got the terminology right here..
@Quuxplusone any other interesting corner cases for these new tests?

[• Quuxplusone]

Some tiny nits. I can't think of any interesting block-related cases (but my knowledge of blocks is apparently very rusty at this point).

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

1065

/depdendent/dependent/

clang/test/CodeGen/nrvo-tracking.cpp

206 ↗

(On Diff #352271)

Formatting nit: I'd think

auto t = []<class T = X>() {
    return ^X() [[clang::vectorcall]] {
        T t;
        return t;
    };
}()();

would be less confusing to the eyes.

clang/test/SemaObjCXX/block-capture.mm

43 ↗

(On Diff #352271)

Nit: ;; here

[mizvekov]

Fix Arthur's nits.

[anhtuyen]

Hi Matheus @mizvekov,
The commit 12c90e2e25dfd1d38250055efc21acb42d158912 from this patch seems to cause a regression, where an assertion failure starts to occur with testcases such as

template <int A> int foo() {
  int a alignas(A) = 0;
  return a;
}

To reproduce the issue, please try:

1. Reset the HEAD of your branch to commit 12c90e2e25dfd1d38250055efc21acb42d158912, which was just before yours:

git reset --hard 12c90e2e25dfd1d38250055efc21acb42d158912

2. Build llvm/clamg
3. Compile the above reduced testcase (let's call it test.cpp)

bin/clang -std=c++11 -c test.cpp

4. There should be no warning nor error

5. Now, please pull in commit 12c90e2e25dfd1d38250055efc21acb42d158912

git pull origin 12c90e2e25dfd1d38250055efc21acb42d158912

6. Rebuild and rerun the above compilation command, you will see the following assertion failure

clang: tools/clang/include/clang/AST/AttrImpl.inc:1750: unsigned int clang::AlignedAttr::getAlignment(clang::ASTContext &) const: Assertion `!isAlignmentDependent()' failed.

GCC does not have that assertion, either.

Can you have a look, please!

[mizvekov]

In D99696#2856285, @anhtuyen wrote:

Hi Matheus @mizvekov,
The commit 12c90e2e25dfd1d38250055efc21acb42d158912 from this patch seems to cause a regression, where an assertion failure starts to occur with testcases such as

Hi Anh, thanks for reporting this problem!

I confirm it, can reproduce locally.
I will be working on a solution.

[mizvekov]

@anhtuyen

I pushed a DR with a preliminary fix for it: https://reviews.llvm.org/D105380

This is not ready to merge, still have to add test cases and also decide between a pointed fix like this, or improving the ergonomics of getDeclAlign by returning possible failure.

But it does fix your repro.

[anhtuyen]

In D99696#2856370, @mizvekov wrote:

@anhtuyen

I pushed a DR with a preliminary fix for it: https://reviews.llvm.org/D105380

This is not ready to merge, still have to add test cases and also decide between a pointed fix like this, or improving the ergonomics of getDeclAlign by returning possible failure.

But it does fix your repro.

Thank you very much Matheus @mizvekov for a quick fix.
I have tried your patch, and it worked for my testcase. I will wait for the official patch when you commit the changes.
Thanks, again!

clang/include/clang/Sema/Sema.h

bool CheckMemberPointerConversion(Expr *From, QualType ToType,
                                     bool IgnoreBaseAccess);
   bool IsQualificationConversion(QualType FromType, QualType ToType,
                                  bool CStyle, bool &ObjCLifetimeConversion);
   bool IsFunctionConversion(QualType FromType, QualType ToType,
                             QualType &ResultTy);
   bool DiagnoseMultipleUserDefinedConversion(Expr *From, QualType ToType);
   bool isSameOrCompatibleFunctionType(CanQualType Param, CanQualType Arg);
 
-  ExprResult PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
-                                             const VarDecl *NRVOCandidate,
-                                             QualType ResultType,
-                                             Expr *Value,
-                                             bool AllowNRVO = true);
-
   bool CanPerformAggregateInitializationForOverloadResolution(
       const InitializedEntity &Entity, InitListExpr *From);
 
   bool IsStringInit(Expr *Init, const ArrayType *AT);
 
   bool CanPerformCopyInitialization(const InitializedEntity &Entity,
                                     ExprResult Init);
   ExprResult PerformCopyInitialization(const InitializedEntity &Entity,
void ActOnCapturedRegionStart(SourceLocation Loc, Scope *CurScope,
                                 ArrayRef<CapturedParamNameType> Params,
                                 unsigned OpenMPCaptureLevel = 0);
   StmtResult ActOnCapturedRegionEnd(Stmt *S);
   void ActOnCapturedRegionError();
   RecordDecl *CreateCapturedStmtRecordDecl(CapturedDecl *&CD,
                                            SourceLocation Loc,
                                            unsigned NumParams);
 
-  enum CopyElisionSemanticsKind {
-    CES_Strict = 0,
-    CES_AllowParameters = 1,
-    CES_AllowDifferentTypes = 2,
-    CES_AllowExceptionVariables = 4,
-    CES_AllowRValueReferenceType = 8,
-    CES_ImplicitlyMovableCXX11CXX14CXX17 =
-        (CES_AllowParameters | CES_AllowDifferentTypes),
-    CES_ImplicitlyMovableCXX20 =
-        (CES_AllowParameters | CES_AllowDifferentTypes |
-         CES_AllowExceptionVariables | CES_AllowRValueReferenceType),
-  };
-
-  VarDecl *getCopyElisionCandidate(QualType ReturnType, Expr *E,
-                                   CopyElisionSemanticsKind CESK);
-  bool isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD,
-                              CopyElisionSemanticsKind CESK);
+  struct NRVOResult {
+    const VarDecl *Candidate;
+
+    enum Status { None, MoveEligible, MoveEligibleAndCopyElidable };
+    Status S;
+
+    bool isMoveEligible() const { return S >= MoveEligible; };
+    bool isCopyElidable() const { return S == MoveEligibleAndCopyElidable; }
+  };
+  NRVOResult getNRVOResult(const Expr *E, bool ForceCXX20 = false);
+  NRVOResult getNRVOResult(const VarDecl *VD, bool ForceCXX20 = false);
+  void updNRVOResultWithRetType(NRVOResult &Res, QualType ReturnType);
+
+  ExprResult PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
+                                             const NRVOResult &NRVORes,
+                                             Expr *Value);
 
   StmtResult ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
                              Scope *CurScope);
   StmtResult BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp);
-  StmtResult ActOnCapScopeReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp);
+  StmtResult ActOnCapScopeReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
+                                     NRVOResult &NRVORes);
 
   StmtResult ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple,
                              bool IsVolatile, unsigned NumOutputs,
                              unsigned NumInputs, IdentifierInfo **Names,
                              MultiExprArg Constraints, MultiExprArg Exprs,
                              Expr *AsmString, MultiExprArg Clobbers,
                              unsigned NumLabels,
                              SourceLocation RParenLoc);

clang/lib/Sema/Sema.cpp

 // resolve its destructor's exception specification.
 static void checkEscapingByref(VarDecl *VD, Sema &S) {
   QualType T = VD->getType();
   EnterExpressionEvaluationContext scope(
       S, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
   SourceLocation Loc = VD->getLocation();
   Expr *VarRef =
       new (S.Context) DeclRefExpr(S.Context, VD, false, T, VK_LValue, Loc);
-  ExprResult Result = S.PerformMoveOrCopyInitialization(
-      InitializedEntity::InitializeBlock(Loc, T, false), VD, VD->getType(),
-      VarRef, /*AllowNRVO=*/true);
+  ExprResult Result = S.PerformCopyInitialization(
+      InitializedEntity::InitializeBlock(Loc, T, false), SourceLocation(),
+      VarRef);
+
   if (!Result.isInvalid()) {
     Result = S.MaybeCreateExprWithCleanups(Result);
     Expr *Init = Result.getAs<Expr>();
     S.Context.setBlockVarCopyInit(VD, Init, S.canThrow(Init));
   }
 
   // The destructor's exception specification is needed when IRGen generates
   // block copy/destroy functions. Resolve it here.

clang/lib/Sema/SemaCoroutine.cpp

StmtResult Sema::BuildCoreturnStmt(SourceLocation Loc, Expr *E,
   if (E && E->getType()->isPlaceholderType() &&
       !E->getType()->isSpecificPlaceholderType(BuiltinType::Overload)) {
     ExprResult R = CheckPlaceholderExpr(E);
     if (R.isInvalid()) return StmtError();
     E = R.get();
   }
 
   // Move the return value if we can
-  if (E) {
-    const VarDecl *NRVOCandidate = this->getCopyElisionCandidate(
-        E->getType(), E, CES_ImplicitlyMovableCXX20);
-    if (NRVOCandidate) {
-      InitializedEntity Entity =
-          InitializedEntity::InitializeResult(Loc, E->getType(), NRVOCandidate);
-      ExprResult MoveResult = this->PerformMoveOrCopyInitialization(
-          Entity, NRVOCandidate, E->getType(), E);
-      if (MoveResult.get())
-        E = MoveResult.get();
-    }
-  }
+  NRVOResult NRVORes = getNRVOResult(E, /*ForceCXX20=*/true);
+  if (NRVORes.isMoveEligible()) {
+    InitializedEntity Entity = InitializedEntity::InitializeResult(
+        Loc, E->getType(), NRVORes.Candidate);
+    ExprResult MoveResult = PerformMoveOrCopyInitialization(Entity, NRVORes, E);
 
   // FIXME: If the operand is a reference to a variable that's about to go out
   // of scope, we should treat the operand as an xvalue for this overload
   // resolution.
   VarDecl *Promise = FSI->CoroutinePromise;
   ExprResult PC;
   if (E && (isa<InitListExpr>(E) || !E->getType()->isVoidType())) {
     PC = buildPromiseCall(*this, Promise, Loc, "return_value", E);
if (FnRetType->isVoidType()) {
     this->ResultDecl = Res.get();
     return true;
   }
 
   if (GroType->isVoidType()) {
     // Trigger a nice error message.
     InitializedEntity Entity =
         InitializedEntity::InitializeResult(Loc, FnRetType, false);
-    S.PerformMoveOrCopyInitialization(Entity, nullptr, FnRetType, ReturnValue);
+    S.PerformCopyInitialization(Entity, SourceLocation(), ReturnValue);
     noteMemberDeclaredHere(S, ReturnValue, Fn);
     return false;
   }
 
   auto *GroDecl = VarDecl::Create(
       S.Context, &FD, FD.getLocation(), FD.getLocation(),
       &S.PP.getIdentifierTable().get("__coro_gro"), GroType,
       S.Context.getTrivialTypeSourceInfo(GroType, Loc), SC_None);
   GroDecl->setImplicit();
 
   S.CheckVariableDeclarationType(GroDecl);
   if (GroDecl->isInvalidDecl())
     return false;
 
   InitializedEntity Entity = InitializedEntity::InitializeVariable(GroDecl);
-  ExprResult Res = S.PerformMoveOrCopyInitialization(Entity, nullptr, GroType,
-                                                     this->ReturnValue);
+  ExprResult Res =
+      S.PerformCopyInitialization(Entity, SourceLocation(), ReturnValue);
   if (Res.isInvalid())
     return false;
 
   Res = S.ActOnFinishFullExpr(Res.get(), /*DiscardedValue*/ false);
   if (Res.isInvalid())
     return false;
 
   S.AddInitializerToDecl(GroDecl, Res.get(),
                          /*DirectInit=*/false);

clang/lib/Sema/SemaExprCXX.cpp

ExprResult Sema::BuildCXXThrow(SourceLocation OpLoc, Expr *Ex,
   if (getLangOpts().CUDA)
     CUDADiagIfDeviceCode(OpLoc, diag::err_cuda_device_exceptions)
         << "throw" << CurrentCUDATarget();
 
   if (getCurScope() && getCurScope()->isOpenMPSimdDirectiveScope())
     Diag(OpLoc, diag::err_omp_simd_region_cannot_use_stmt) << "throw";
 
   if (Ex && !Ex->isTypeDependent()) {
+    NRVOResult NRVORes = IsThrownVarInScope ? getNRVOResult(Ex) : NRVOResult();
+
     QualType ExceptionObjectTy = Context.getExceptionObjectType(Ex->getType());
     if (CheckCXXThrowOperand(OpLoc, ExceptionObjectTy, Ex))
       return ExprError();
 
     // Initialize the exception result.  This implicitly weeds out
     // abstract types or types with inaccessible copy constructors.
 
     // C++0x [class.copymove]p31:
     //   When certain criteria are met, an implementation is allowed to omit the
     //   copy/move construction of a class object [...]
     //
     //     - in a throw-expression, when the operand is the name of a
     //       non-volatile automatic object (other than a function or
     //       catch-clause
     //       parameter) whose scope does not extend beyond the end of the
     //       innermost enclosing try-block (if there is one), the copy/move
     //       operation from the operand to the exception object (15.1) can be
     //       omitted by constructing the automatic object directly into the
     //       exception object
-    const VarDecl *NRVOVariable = nullptr;
-    if (IsThrownVarInScope)
-      NRVOVariable = getCopyElisionCandidate(QualType(), Ex, CES_Strict);
 
     InitializedEntity Entity = InitializedEntity::InitializeException(
         OpLoc, ExceptionObjectTy,
-        /*NRVO=*/NRVOVariable != nullptr);
-    ExprResult Res = PerformMoveOrCopyInitialization(
-        Entity, NRVOVariable, QualType(), Ex, IsThrownVarInScope);
+        /*NRVO=*/NRVORes.isCopyElidable());
+    ExprResult Res = PerformMoveOrCopyInitialization(Entity, NRVORes, Ex);
     if (Res.isInvalid())
       return ExprError();
     Ex = Res.get();
   }
 
   // PPC MMA non-pointer types are not allowed as throw expr types.
   if (Ex && Context.getTargetInfo().getTriple().isPPC64())
     CheckPPCMMAType(Ex->getType(), Ex->getBeginLoc());

clang/lib/Sema/SemaStmt.cpp

Sema::ActOnBreakStmt(SourceLocation BreakLoc, Scope *CurScope) {
   if (S->isOpenMPLoopScope())
     return StmtError(Diag(BreakLoc, diag::err_omp_loop_cannot_use_stmt)
                      << "break");
   CheckJumpOutOfSEHFinally(*this, BreakLoc, *S);
 
   return new (Context) BreakStmt(BreakLoc);
 }
 
-/// Determine whether the given expression is a candidate for
-/// copy elision in either a return statement or a throw expression.
-///
-/// \param ReturnType If we're determining the copy elision candidate for
-/// a return statement, this is the return type of the function. If we're
-/// determining the copy elision candidate for a throw expression, this will
-/// be a NULL type.
+static void downgradeNRVOResult(Sema::NRVOResult &Res, bool CanMove) {
+  Res.S = CanMove ? std::min(Res.S, Sema::NRVOResult::MoveEligible)
+                  : Sema::NRVOResult::None;
+}
+
+/// Determine whether the given NRVO candidate variable is move-eligible or
+/// copy-elision eligible, without considering function return type.
 ///
-/// \param E The expression being returned from the function or block, or
-/// being thrown.
+/// \param VD The NRVO candidate variable.
 ///
-/// \param CESK Whether we allow function parameters or
-/// id-expressions that could be moved out of the function to be considered NRVO
-/// candidates. C++ prohibits these for NRVO itself, but we re-use this logic to
-/// determine whether we should try to move as part of a return or throw (which
-/// does allow function parameters).
+/// \param ForceCXX20 Overrides detection of current language mode
+/// and uses the rules for C++20.
 ///
-/// \returns The NRVO candidate variable, if the return statement may use the
-/// NRVO, or NULL if there is no such candidate.
-VarDecl *Sema::getCopyElisionCandidate(QualType ReturnType, Expr *E,
-                                       CopyElisionSemanticsKind CESK) {
-  // - in a return statement in a function [where] ...
-  // ... the expression is the name of a non-volatile automatic object ...
-  DeclRefExpr *DR = dyn_cast<DeclRefExpr>(E->IgnoreParens());
-  if (!DR || DR->refersToEnclosingVariableOrCapture())
-    return nullptr;
-  VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl());
+/// \returns An aggregate which contains the Candidate and isMoveEligible
+/// and isCopyElidable methods. If Candidate is non-null, it means
+/// isMoveEligible() would be true under the most permissive language standard.
+Sema::NRVOResult Sema::getNRVOResult(const VarDecl *VD, bool ForceCXX20) {
   if (!VD)
-    return nullptr;
-
-  if (isCopyElisionCandidate(ReturnType, VD, CESK))
-    return VD;
-  return nullptr;
-}
+    return NRVOResult();
 
-bool Sema::isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD,
-                                  CopyElisionSemanticsKind CESK) {
-  QualType VDType = VD->getType();
-  // - in a return statement in a function with ...
-  // ... a class return type ...
-  if (!ReturnType.isNull() && !ReturnType->isDependentType()) {
-    if (!ReturnType->isRecordType())
-      return false;
-    // ... the same cv-unqualified type as the function return type ...
-    // When considering moving this expression out, allow dissimilar types.
-    if (!(CESK & CES_AllowDifferentTypes) && !VDType->isDependentType() &&
-        !Context.hasSameUnqualifiedType(ReturnType, VDType))
-      return false;
-  }
-
-  // ...object (other than a function or catch-clause parameter)...
-  if (VD->getKind() != Decl::Var &&
-      !((CESK & CES_AllowParameters) && VD->getKind() == Decl::ParmVar))
-    return false;
-  if (!(CESK & CES_AllowExceptionVariables) && VD->isExceptionVariable())
-    return false;
+  bool hasCXX11 = getLangOpts().CPlusPlus11 || ForceCXX20,
+       hasCXX20 = getLangOpts().CPlusPlus20 || ForceCXX20;
+  NRVOResult Res{VD, NRVOResult::MoveEligibleAndCopyElidable};
+
+  // (other than a function ... parameter)
+  if (VD->getKind() == Decl::ParmVar) {
+    downgradeNRVOResult(Res, hasCXX11);
+  } else if (VD->getKind() != Decl::Var) {
+    return NRVOResult();
+  }
+
+  // (other than ... a catch-clause parameter)
+  if (VD->isExceptionVariable())
+    downgradeNRVOResult(Res, hasCXX20);
 
   // ...automatic...
-  if (!VD->hasLocalStorage()) return false;
+  if (!VD->hasLocalStorage())
+    return NRVOResult();
 
   // Return false if VD is a __block variable. We don't want to implicitly move
   // out of a __block variable during a return because we cannot assume the
   // variable will no longer be used.
   if (VD->hasAttr<BlocksAttr>())
-    return false;
+    return NRVOResult();
 
+  QualType VDType = VD->getType();
   if (VDType->isObjectType()) {
     // C++17 [class.copy.elision]p3:
     // ...non-volatile automatic object...
     if (VDType.isVolatileQualified())
-      return false;
+      return NRVOResult();
   } else if (VDType->isRValueReferenceType()) {
     // C++20 [class.copy.elision]p3:
-    // ...either a non-volatile object or an rvalue reference to a non-volatile object type...
-    if (!(CESK & CES_AllowRValueReferenceType))
-      return false;
+    // ...either a non-volatile object or an rvalue reference to a non-volatile
+    // object type...
     QualType VDReferencedType = VDType.getNonReferenceType();
-    if (VDReferencedType.isVolatileQualified() || !VDReferencedType->isObjectType())
-      return false;
+    if (VDReferencedType.isVolatileQualified() ||
+        !VDReferencedType->isObjectType())
+      return NRVOResult();
+    downgradeNRVOResult(Res, hasCXX20);
   } else {
-    return false;
+    return NRVOResult();
   }
 
-  if (CESK & CES_AllowDifferentTypes)
-    return true;
-
   // Variables with higher required alignment than their type's ABI
   // alignment cannot use NRVO.
   if (!VDType->isDependentType() && VD->hasAttr<AlignedAttr>() &&
       Context.getDeclAlign(VD) > Context.getTypeAlignInChars(VDType))
-    return false;
+    downgradeNRVOResult(Res, hasCXX11);
 
-  return true;
+  return Res;
+}
+
+/// Determine whether the given expression might be move-eligible or
+/// copy-elidable in either a (co_)return statement or throw expression,
+/// without considering function return type, if applicable.
+///
+/// \param E The expression being returned from the function or block,
+/// being thrown, or being co_returned from a coroutine.
+///
+/// \param ForceCXX20 Overrides detection of current language mode
+/// and uses the rules for C++20.
+///
+/// \returns An aggregate which contains the Candidate and isMoveEligible
+/// and isCopyElidable methods. If Candidate is non-null, it means
+/// isMoveEligible() would be true under the most permissive language standard.
+Sema::NRVOResult Sema::getNRVOResult(const Expr *E, bool ForceCXX20) {
+  if (!E)
+    return NRVOResult();
+  // - in a return statement in a function [where] ...
+  // ... the expression is the name of a non-volatile automatic object ...
+  const auto *DR = dyn_cast<DeclRefExpr>(E->IgnoreParens());
+  if (!DR || DR->refersToEnclosingVariableOrCapture())
+    return NRVOResult();
+  return getNRVOResult(dyn_cast<VarDecl>(DR->getDecl()),
+                       /*ForceCXX20=*/ForceCXX20);
+}
+
+/// Updates given NRVOResult's move-eligible and
+/// copy-elidable statuses, considering the function return type criteria
+/// as applicable to return statements.
+///
+/// \param Res The NRVOResult object to update.
+///
+/// \param ReturnType This is the return type of the function.
+void Sema::updNRVOResultWithRetType(NRVOResult &Res, QualType ReturnType) {
+  assert(!ReturnType->isUndeducedType());
+
+  if (!Res.Candidate || ReturnType->isDependentType())
+    return;
+  // - in a return statement in a function with ...
+  // ... a class return type ...
+  if (!ReturnType->isRecordType()) {
+    Res = NRVOResult();
+    return;
+  }
+  QualType VDType = Res.Candidate->getType();
+  // ... the same cv-unqualified type as the function return type ...
+  // When considering moving this expression out, allow dissimilar types.
+  if (!VDType->isDependentType() &&
+      !Context.hasSameUnqualifiedType(ReturnType, VDType))
+    downgradeNRVOResult(Res, getLangOpts().CPlusPlus11);
 }
 
 /// Try to perform the initialization of a potentially-movable value,
 /// which is the operand to a return or throw statement.
 ///
 /// This routine implements C++20 [class.copy.elision]p3, which attempts to
 /// treat returned lvalues as rvalues in certain cases (to prefer move
 /// construction), then falls back to treating them as lvalues if that failed.
 ///
 /// \param ConvertingConstructorsOnly If true, follow [class.copy.elision]p3 and
 /// reject resolutions that find non-constructors, such as derived-to-base
 /// conversions or `operator T()&&` member functions. If false, do consider such
 /// conversion sequences.
 ///
 /// \param Res We will fill this in if move-initialization was possible.
 /// If move-initialization is not possible, such that we must fall back to
 /// treating the operand as an lvalue, we will leave Res in its original
 /// invalid state.
 ///
 /// \returns Whether we need to do the second overload resolution. If the first
 /// overload resolution fails, or if the first overload resolution succeeds but
 /// the selected constructor/operator doesn't match the additional criteria, we
 /// need to do the second overload resolution.
 static bool TryMoveInitialization(Sema &S, const InitializedEntity &Entity,
-                                  const VarDecl *NRVOCandidate,
-                                  QualType ResultType, Expr *&Value,
+                                  const VarDecl *NRVOCandidate, Expr *&Value,
                                   bool ConvertingConstructorsOnly,
                                   bool IsDiagnosticsCheck, ExprResult &Res) {
   ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, Value->getType(),
                             CK_NoOp, Value, VK_XValue, FPOptionsOverride());
 
   Expr *InitExpr = &AsRvalue;
 
   InitializationKind Kind = InitializationKind::CreateCopy(

 }
 
 /// Perform the initialization of a potentially-movable value, which
 /// is the result of return value.
 ///
 /// This routine implements C++20 [class.copy.elision]p3, which attempts to
 /// treat returned lvalues as rvalues in certain cases (to prefer move
 /// construction), then falls back to treating them as lvalues if that failed.
-ExprResult Sema::PerformMoveOrCopyInitialization(
-    const InitializedEntity &Entity, const VarDecl *NRVOCandidate,
-    QualType ResultType, Expr *Value, bool AllowNRVO) {
-  ExprResult Res = ExprError();
-  bool NeedSecondOverloadResolution = true;
-
-  if (AllowNRVO) {
-    CopyElisionSemanticsKind CESK = CES_Strict;
-    if (getLangOpts().CPlusPlus20) {
-      CESK = CES_ImplicitlyMovableCXX20;
-    } else if (getLangOpts().CPlusPlus11) {
-      CESK = CES_ImplicitlyMovableCXX11CXX14CXX17;
-    }
-
-    if (!NRVOCandidate) {
-      NRVOCandidate = getCopyElisionCandidate(ResultType, Value, CESK);
-    }
+ExprResult
+Sema::PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
+                                      const NRVOResult &NRVORes, Expr *Value) {
 
-    if (NRVOCandidate) {
-      NeedSecondOverloadResolution =
-          TryMoveInitialization(*this, Entity, NRVOCandidate, ResultType, Value,
-                                !getLangOpts().CPlusPlus20, false, Res);
+  if (NRVORes.Candidate) {
+    if (NRVORes.isMoveEligible()) {
+      ExprResult Res;
+      if (!TryMoveInitialization(*this, Entity, NRVORes.Candidate, Value,
+                                 !getLangOpts().CPlusPlus20, false, Res))
+        return Res;
     }
-
-    if (!getLangOpts().CPlusPlus20 && NeedSecondOverloadResolution &&
-        !getDiagnostics().isIgnored(diag::warn_return_std_move,
+    if (!getDiagnostics().isIgnored(diag::warn_return_std_move,
                                     Value->getExprLoc())) {
-      const VarDecl *FakeNRVOCandidate = getCopyElisionCandidate(
-          QualType(), Value, CES_ImplicitlyMovableCXX20);
-      if (FakeNRVOCandidate) {
-        QualType QT = FakeNRVOCandidate->getType();
-        if (QT->isLValueReferenceType()) {
-          // Adding 'std::move' around an lvalue reference variable's name is
-          // dangerous. Don't suggest it.
-        } else if (QT.getNonReferenceType()
-                       .getUnqualifiedType()
-                       .isTriviallyCopyableType(Context)) {
-          // Adding 'std::move' around a trivially copyable variable is probably
-          // pointless. Don't suggest it.
-        } else {
-          ExprResult FakeRes = ExprError();
-          Expr *FakeValue = Value;
-          TryMoveInitialization(*this, Entity, FakeNRVOCandidate, ResultType,
-                                FakeValue, false, true, FakeRes);
-          if (!FakeRes.isInvalid()) {
-            bool IsThrow =
-                (Entity.getKind() == InitializedEntity::EK_Exception);
-            SmallString<32> Str;
-            Str += "std::move(";
-            Str += FakeNRVOCandidate->getDeclName().getAsString();
-            Str += ")";
-            Diag(Value->getExprLoc(), diag::warn_return_std_move)
-                << Value->getSourceRange()
-                << FakeNRVOCandidate->getDeclName() << IsThrow;
-            Diag(Value->getExprLoc(), diag::note_add_std_move)
-                << FixItHint::CreateReplacement(Value->getSourceRange(), Str);
-          }
-        }
-      }
-    }
-  }
+      QualType QT = NRVORes.Candidate->getType();
+        TryMoveInitialization(*this, Entity, NRVORes.Candidate, FakeValue,
+                              false, true, FakeRes);
+          bool IsThrow = (Entity.getKind() == InitializedEntity::EK_Exception);
+          Str += NRVORes.Candidate->getDeclName().getAsString();
+              << Value->getSourceRange() << NRVORes.Candidate->getDeclName()
+              << IsThrow;
 
   // Either we didn't meet the criteria for treating an lvalue as an rvalue,
   // above, or overload resolution failed. Either way, we need to try
   // (again) now with the return value expression as written.
-  if (NeedSecondOverloadResolution)
-    Res = PerformCopyInitialization(Entity, SourceLocation(), Value);
-
-  return Res;
+  return PerformCopyInitialization(Entity, SourceLocation(), Value);
 }
 
 /// Determine whether the declared return type of the specified function
 /// contains 'auto'.
 static bool hasDeducedReturnType(FunctionDecl *FD) {
   const FunctionProtoType *FPT =
       FD->getTypeSourceInfo()->getType()->castAs<FunctionProtoType>();
   return FPT->getReturnType()->isUndeducedType();
 }
 
 /// ActOnCapScopeReturnStmt - Utility routine to type-check return statements
 /// for capturing scopes.
 ///
-StmtResult
-Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
+StmtResult Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc,
+                                         Expr *RetValExp, NRVOResult &NRVORes) {
   // If this is the first return we've seen, infer the return type.
   // [expr.prim.lambda]p4 in C++11; block literals follow the same rules.
   CapturingScopeInfo *CurCap = cast<CapturingScopeInfo>(getCurFunction());
   QualType FnRetType = CurCap->ReturnType;
   LambdaScopeInfo *CurLambda = dyn_cast<LambdaScopeInfo>(CurCap);
   bool HasDeducedReturnType =
       CurLambda && hasDeducedReturnType(CurLambda->CallOperator);
 
if (RetValExp && !isa<InitListExpr>(RetValExp)) {
       FnRetType = Context.VoidTy;
     }
 
     // Although we'll properly infer the type of the block once it's completed,
     // make sure we provide a return type now for better error recovery.
     if (CurCap->ReturnType.isNull())
       CurCap->ReturnType = FnRetType;
   }
-  assert(!FnRetType.isNull());
+  updNRVOResultWithRetType(NRVORes, FnRetType);
 
   if (auto *CurBlock = dyn_cast<BlockScopeInfo>(CurCap)) {
     if (CurBlock->FunctionType->castAs<FunctionType>()->getNoReturnAttr()) {
       Diag(ReturnLoc, diag::err_noreturn_block_has_return_expr);
       return StmtError();
     }
   } else if (auto *CurRegion = dyn_cast<CapturedRegionScopeInfo>(CurCap)) {
     Diag(ReturnLoc, diag::err_return_in_captured_stmt) << CurRegion->getRegionName();
     return StmtError();
   } else {
     assert(CurLambda && "unknown kind of captured scope");
     if (CurLambda->CallOperator->getType()
             ->castAs<FunctionType>()
             ->getNoReturnAttr()) {
       Diag(ReturnLoc, diag::err_noreturn_lambda_has_return_expr);
       return StmtError();
     }
   }
 
   // Otherwise, verify that this result type matches the previous one.  We are
   // pickier with blocks than for normal functions because we don't have GCC
   // compatibility to worry about here.
-  const VarDecl *NRVOCandidate = nullptr;
   if (FnRetType->isDependentType()) {
     // Delay processing for now.  TODO: there are lots of dependent
     // types we can conclusively prove aren't void.
   } else if (FnRetType->isVoidType()) {
     if (RetValExp && !isa<InitListExpr>(RetValExp) &&
         !(getLangOpts().CPlusPlus &&
           (RetValExp->isTypeDependent() ||
            RetValExp->getType()->isVoidType()))) {
if (FnRetType->isDependentType()) {
     // we have a non-void block with an expression, continue checking
 
     // C99 6.8.6.4p3(136): The return statement is not an assignment. The
     // overlap restriction of subclause 6.5.16.1 does not apply to the case of
     // function return.
 
     // In C++ the return statement is handled via a copy initialization.
     // the C version of which boils down to CheckSingleAssignmentConstraints.
-    NRVOCandidate = getCopyElisionCandidate(FnRetType, RetValExp, CES_Strict);
-    InitializedEntity Entity = InitializedEntity::InitializeResult(ReturnLoc,
-                                                                   FnRetType,
-                                                      NRVOCandidate != nullptr);
-    ExprResult Res = PerformMoveOrCopyInitialization(Entity, NRVOCandidate,
-                                                     FnRetType, RetValExp);
+    InitializedEntity Entity = InitializedEntity::InitializeResult(
+        ReturnLoc, FnRetType, NRVORes.isCopyElidable());
+    ExprResult Res =
+        PerformMoveOrCopyInitialization(Entity, NRVORes, RetValExp);
     if (Res.isInvalid()) {
       // FIXME: Cleanup temporaries here, anyway?
       return StmtError();
     }
     RetValExp = Res.get();
     CheckReturnValExpr(RetValExp, FnRetType, ReturnLoc);
-  } else {
-    NRVOCandidate = getCopyElisionCandidate(FnRetType, RetValExp, CES_Strict);
   }
 
   if (RetValExp) {
     ExprResult ER =
         ActOnFinishFullExpr(RetValExp, ReturnLoc, /*DiscardedValue*/ false);
     if (ER.isInvalid())
       return StmtError();
     RetValExp = ER.get();
   }
-  auto *Result =
-      ReturnStmt::Create(Context, ReturnLoc, RetValExp, NRVOCandidate);
+  auto *Result = ReturnStmt::Create(Context, ReturnLoc, RetValExp,
+                                    NRVORes.isCopyElidable() ? NRVORes.Candidate
+                                                             : nullptr);
 
   // If we need to check for the named return value optimization,
   // or if we need to infer the return type,
   // save the return statement in our scope for later processing.
-  if (CurCap->HasImplicitReturnType || NRVOCandidate)
+  if (CurCap->HasImplicitReturnType || NRVORes.isCopyElidable())
     FunctionScopes.back()->Returns.push_back(Result);
 
   if (FunctionScopes.back()->FirstReturnLoc.isInvalid())
     FunctionScopes.back()->FirstReturnLoc = ReturnLoc;
 
   return Result;
 }
 
Sema::ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
   return R;
 }
 
 StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
   // Check for unexpanded parameter packs.
   if (RetValExp && DiagnoseUnexpandedParameterPack(RetValExp))
     return StmtError();
 
+  NRVOResult NRVORes = getNRVOResult(RetValExp);
+
   if (isa<CapturingScopeInfo>(getCurFunction()))
-    return ActOnCapScopeReturnStmt(ReturnLoc, RetValExp);
+    return ActOnCapScopeReturnStmt(ReturnLoc, RetValExp, NRVORes);
 
   QualType FnRetType;
   QualType RelatedRetType;
   const AttrVec *Attrs = nullptr;
   bool isObjCMethod = false;
 
   if (const FunctionDecl *FD = getCurFunctionDecl()) {
     FnRetType = FD->getReturnType();
if (AutoType *AT = FnRetType->getContainedAutoType()) {
       if (DeduceFunctionTypeFromReturnExpr(FD, ReturnLoc, RetValExp, AT)) {
         FD->setInvalidDecl();
         return StmtError();
       } else {
         FnRetType = FD->getReturnType();
       }
     }
   }
+  updNRVOResultWithRetType(NRVORes, FnRetType);
 
   bool HasDependentReturnType = FnRetType->isDependentType();
 
   ReturnStmt *Result = nullptr;
   if (FnRetType->isVoidType()) {
     if (RetValExp) {
       if (isa<InitListExpr>(RetValExp)) {
         // We simply never allow init lists as the return value of void
if (getLangOpts().CPlusPlus11 && FD && FD->isConstexpr()) {
           IsMethod ? cast<NamedDecl>(getCurMethodDecl()) : cast<NamedDecl>(FD);
       Diag(ReturnLoc, DiagID) << ND << IsMethod;
     }
 
     Result = ReturnStmt::Create(Context, ReturnLoc, /* RetExpr=*/nullptr,
                                 /* NRVOCandidate=*/nullptr);
   } else {
     assert(RetValExp || HasDependentReturnType);
-    const VarDecl *NRVOCandidate = nullptr;
-
     QualType RetType = RelatedRetType.isNull() ? FnRetType : RelatedRetType;
 
     // C99 6.8.6.4p3(136): The return statement is not an assignment. The
     // overlap restriction of subclause 6.5.16.1 does not apply to the case of
     // function return.
 
     // In C++ the return statement is handled via a copy initialization,
     // the C version of which boils down to CheckSingleAssignmentConstraints.
-    if (RetValExp)
-      NRVOCandidate = getCopyElisionCandidate(FnRetType, RetValExp, CES_Strict);
     if (!HasDependentReturnType && !RetValExp->isTypeDependent()) {
       // we have a non-void function with an expression, continue checking
-      InitializedEntity Entity = InitializedEntity::InitializeResult(ReturnLoc,
-                                                                     RetType,
-                                                      NRVOCandidate != nullptr);
-      ExprResult Res = PerformMoveOrCopyInitialization(Entity, NRVOCandidate,
-                                                       RetType, RetValExp);
+      InitializedEntity Entity = InitializedEntity::InitializeResult(
+          ReturnLoc, RetType, NRVORes.isCopyElidable());
+      ExprResult Res =
+          PerformMoveOrCopyInitialization(Entity, NRVORes, RetValExp);
       if (Res.isInvalid()) {
         // FIXME: Clean up temporaries here anyway?
         return StmtError();
       }
       RetValExp = Res.getAs<Expr>();
 
       // If we have a related result type, we need to implicitly
       // convert back to the formal result type.  We can't pretend to
if (FnRetType->isVoidType()) {
 
     if (RetValExp) {
       ExprResult ER =
           ActOnFinishFullExpr(RetValExp, ReturnLoc, /*DiscardedValue*/ false);
       if (ER.isInvalid())
         return StmtError();
       RetValExp = ER.get();
     }
-    Result = ReturnStmt::Create(Context, ReturnLoc, RetValExp, NRVOCandidate);
+    Result = ReturnStmt::Create(Context, ReturnLoc, RetValExp,
+                                NRVORes.isCopyElidable() ? NRVORes.Candidate
+                                                         : nullptr);
   }
 
   // If we need to check for the named return value optimization, save the
   // return statement in our scope for later processing.
   if (Result->getNRVOCandidate())
     FunctionScopes.back()->Returns.push_back(Result);
 
   if (FunctionScopes.back()->FirstReturnLoc.isInvalid())

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

Decl *TemplateDeclInstantiator::VisitVarDecl(VarDecl *D,
   // Substitute the nested name specifier, if any.
   if (SubstQualifier(D, Var))
     return nullptr;
 
   SemaRef.BuildVariableInstantiation(Var, D, TemplateArgs, LateAttrs, Owner,
                                      StartingScope, InstantiatingVarTemplate);
 
   if (D->isNRVOVariable()) {
-    QualType ReturnType = cast<FunctionDecl>(DC)->getReturnType();
-    if (SemaRef.isCopyElisionCandidate(ReturnType, Var, Sema::CES_Strict))
-      Var->setNRVOVariable(true);
+    Sema::NRVOResult Res = SemaRef.getNRVOResult(Var);
+
+    QualType ReturnType;
+    if (auto *F = dyn_cast<FunctionDecl>(DC))
+      ReturnType = F->getReturnType();
+    else if (auto *F = dyn_cast<BlockDecl>(DC))
+      // FIXME: get the return type here somehow...
+      ReturnType = SemaRef.Context.getAutoDeductType();
+    else
+      assert(false && "Unknown context type");
+
+    if (!ReturnType->isUndeducedType())
+      SemaRef.updNRVOResultWithRetType(Res, ReturnType);
+    Var->setNRVOVariable(Res.isCopyElidable());
   }
 
   Var->setImplicit(D->isImplicit());
 
   if (Var->isStaticLocal())
     SemaRef.CheckStaticLocalForDllExport(Var);
 
   return Var;