[PATCH] D30170: Function definition may have uninstantiated body

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

rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.


================
Comment at: include/clang/AST/Decl.h:1840
+  /// there is one).
+  ///
   bool hasBody(const FunctionDecl *&Definition) const;
----------------
Please remove trailing blank comment lines here and below.


================
Comment at: lib/Sema/SemaDecl.cpp:11986
+      !FD->isDefined(Definition)
+      && FD->getDeclContext()->isFileContext()) {
+    // If this is a friend function defined in a class template, it does not
----------------
`&&` on the end of the previous line, please.

If the intent here is to detect non-member functions, using `!FD->isCXXClassMember()` or `!isa<CXXMethodDecl>(FD)` would be clearer.


================
Comment at: lib/Sema/SemaDecl.cpp:11995-12006
+    for (auto I : FD->redecls()) {
+      if (I != FD && !I->isInvalidDecl() &&
+          I->getFriendObjectKind() != Decl::FOK_None) {
+        if (FunctionDecl *Original = I->getInstantiatedFromMemberFunction()) {
+          if (Original->isThisDeclarationADefinition()) {
+            Definition = I;
+            break;
----------------
We should include a comment here explaining why we need to do this (that is, why this doesn't just fall out from the normal `isDefined` check). You can just quote C++ [temp.inst]p2:

> For the purpose of determining whether an instantiated redeclaration is valid according to [basic.def.odr] and [class.mem], a declaration that corresponds to a definition in the template is considered to be a definition.


https://reviews.llvm.org/D30170