Quuxplusone added inline comments.
================
Comment at: lib/Sema/SemaDecl.cpp:12760
// to deduce an implicit return type.
- if (FD->getReturnType()->isRecordType() &&
- (!getLangOpts().CPlusPlus || !FD->isDependentContext()))
+ if (!FD->getReturnType()->isScalarType())
computeNRVO(Body, getCurFunction());
----------------
What is the purpose of this change?
If it's "no functional change" it should be done separately IMHO; if it is
supposed to change codegen, then it needs some codegen tests. (From looking at
the code: maybe this affects codegen on functions that return member function
pointers by value?)
================
Comment at: test/CodeGenCXX/nrvo.cpp:139
}
- // FIXME: we should NRVO this variable too.
- X x;
+ // CHECK: tail call {{.*}} @_ZN1XC1ERKS_
return x;
----------------
You've changed this function from testing one thing with a FIXME, to testing a
completely different thing. Could you add your new code as a new function, and
leave the old FIXME alone until it's fixed?
Alternatively, if your patch actually fixes the FIXME, could you just replace
the FIXME comment with a CHECK and leave the rest of this function alone?
================
Comment at: test/CodeGenCXX/nrvo.cpp:254
+ T t;
+ return t;
+}
----------------
Just for my own curiosity: this new test case is surely unaffected by this
patch, right?
Repository:
rC Clang
https://reviews.llvm.org/D47067
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
