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

PoYao Chang via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 5 20:14:27 PDT 2022


rZhBoYao marked 2 inline comments as done.
rZhBoYao added inline comments.


================
Comment at: clang/lib/Parse/Parser.cpp:1339
 
     bool Delete = false;
     SourceLocation KWLoc;
----------------
I'm thinking should we merge `FnDeleted` and `Deleted` into one variable or leave it as is? On the one hand, `Deleted`'s scope is tight so very readable. On the other hand, it seems redundant.


================
Comment at: clang/lib/Parse/Parser.cpp:1346
         << 1 /* deleted */;
       Actions.SetDeclDeleted(Res, KWLoc);
       Delete = true;
----------------
`FunctionDecl::setDeletedAsWritten` not called until this `SetDeclDeleted` call.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:14457
   // 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).
----------------
ChuanqiXu wrote:
> 
I think the sentence is not yet complete there so I mimic the old one on the left.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:14461
   if (!ResultType->isDependentType() && !ResultType->isVoidType() &&
-      !FD->isInvalidDecl() &&
+      !FD->isInvalidDecl() && !FnDeleted &&
       RequireCompleteType(FD->getLocation(), ResultType,
----------------
ChuanqiXu wrote:
> I think we could remove the use of `FnDeleted` by the use of `FD->isDeleted()`. Also we should constrain the behavior only if std >= 11.
I tried `FD->isDeleted()` at first only to find out it always returns `false`. Looks like it's not handled until [[ https://github.com/llvm/llvm-project/blob/634bf829a8d289371d5b5a50b787596124228898/clang/lib/Parse/Parser.cpp#L1342 | Parser.cpp:1342 ]]

Also, looks like deleted functions are implemented as an extension. A warning is issued at [[ https://github.com/llvm/llvm-project/blob/634bf829a8d289371d5b5a50b787596124228898/clang/lib/Parse/Parser.cpp#L1338-L1341 | Parser.cpp:1338 ]] if language mode < C++11


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122981



More information about the cfe-commits mailing list