[PATCH] D122981: [Clang] Diagnose incomplete return/param types only when function is not deleted

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

This definitely looks like it is 'nicer' than before,  a few smaller/nit-ish comments.

Additionally, Phab made a REAL mess of the diff, if you could give a quick summary of what changes were actually made (vs things that were moved slightly and causing massive red/green diffs), it would be helpful.



================
Comment at: clang/include/clang/Sema/Sema.h:2906
+
+    /// Not yet parsed
+    /// Could be one of:
----------------



================
Comment at: clang/include/clang/Sema/Sema.h:2910
+    ///   function-try-block
+    NotYetParsed
+  };
----------------
Might suggest 'Other'?  

Also, the EqDelete and EqDefault could probably just be Delete/Default and be equally as clear.


================
Comment at: clang/lib/Parse/Parser.cpp:1363
+    if (BodyKind == Sema::FnBodyKind::EqDelete)
+      Actions.SetDeclDeleted(Res, KWLoc);
+    else if (BodyKind == Sema::FnBodyKind::EqDefault)
----------------
Since the FnBodyKind is in Sema, I would suggest just adding a new function to replace all of this, "SetFunctionBodyKind(Res, BodyKind)", and make Sema do the 'switch' here.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:14474
   // The return type of a function definition must be complete
-  // (C99 6.9.1p3, C++ [dcl.fct]p6).
+  // unless the function is deleted
+  // (C99 6.9.1p3, C++ [dcl.fct.def.general]p2).
----------------
This is C++ specific, so could we be specific about that here?


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

https://reviews.llvm.org/D122981