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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
