[PR32667] -Wdocumentation should allow @param/@returns for fields/variables

that have a function/block pointer type

This commit improves the -Wdocumentation warning by making sure that @param and
@returns commands won't trigger warnings when used for fields, variables,
or properties whose type is a function/block pointer type. The
function/block pointer type must be specified directly with the declaration,
and when a typedef is used the warning is still emitted.

In the future we might also want to handle the std::function type as well.

rdar://24978538

llvm-svn: 300981

clang/include/clang/AST/CommentSema.h

@@ -208,6 +208,10 @@ public:
   /// \returns \c true if declaration that this comment is attached to declares
   /// a function pointer.
   bool isFunctionPointerVarDecl();
+  /// \returns \c true if the declaration that this comment is attached to
+  /// declares a variable or a field whose type is a function or a block
+  /// pointer.
+  bool isFunctionOrBlockPointerVarLikeDecl();
   bool isFunctionOrMethodVariadic();
   bool isObjCMethodDecl();
   bool isObjCPropertyDecl();

clang/lib/AST/Comment.cpp

@@ -280,8 +280,25 @@ void DeclInfo::fill() {
   case Decl::EnumConstant:
   case Decl::ObjCIvar:
   case Decl::ObjCAtDefsField:
+  case Decl::ObjCProperty: {
+    const TypeSourceInfo *TSI;
+    if (const auto *VD = dyn_cast<DeclaratorDecl>(CommentDecl))
+      TSI = VD->getTypeSourceInfo();
+    else if (const auto *PD = dyn_cast<ObjCPropertyDecl>(CommentDecl))
+      TSI = PD->getTypeSourceInfo();
+    else
+      TSI = nullptr;
+    if (TSI) {
+      TypeLoc TL = TSI->getTypeLoc().getUnqualifiedLoc();
+      FunctionTypeLoc FTL;
+      if (getFunctionTypeLoc(TL, FTL)) {
+        ParamVars = FTL.getParams();
+        ReturnType = FTL.getReturnLoc().getType();
+      }
+    }
     Kind = VariableKind;
     break;
+  }
   case Decl::Namespace:
     Kind = NamespaceKind;
     break;

clang/lib/AST/CommentSema.cpp

@@ -86,7 +86,7 @@ ParamCommandComment *Sema::actOnParamCommandStart(
       new (Allocator) ParamCommandComment(LocBegin, LocEnd, CommandID,
                                           CommandMarker);
 
-  if (!isFunctionDecl())
+  if (!isFunctionDecl() && !isFunctionOrBlockPointerVarLikeDecl())
     Diag(Command->getLocation(),
          diag::warn_doc_param_not_attached_to_a_function_decl)
       << CommandMarker
@@ -584,7 +584,7 @@ void Sema::checkReturnsCommand(const BlockCommandComment *Command) {
 
   assert(ThisDeclInfo && "should not call this check on a bare comment");
 
-  if (isFunctionDecl()) {
+  if (isFunctionDecl() || isFunctionOrBlockPointerVarLikeDecl()) {
     if (ThisDeclInfo->ReturnType->isVoidType()) {
       unsigned DiagKind;
       switch (ThisDeclInfo->CommentDecl->getKind()) {
@@ -844,6 +844,30 @@ bool Sema::isFunctionPointerVarDecl() {
   return false;
 }
 
+bool Sema::isFunctionOrBlockPointerVarLikeDecl() {
+  if (!ThisDeclInfo)
+    return false;
+  if (!ThisDeclInfo->IsFilled)
+    inspectThisDecl();
+  if (ThisDeclInfo->getKind() != DeclInfo::VariableKind ||
+      !ThisDeclInfo->CurrentDecl)
+    return false;
+  QualType QT;
+  if (const auto *VD = dyn_cast<DeclaratorDecl>(ThisDeclInfo->CurrentDecl))
+    QT = VD->getType();
+  else if (const auto *PD =
+               dyn_cast<ObjCPropertyDecl>(ThisDeclInfo->CurrentDecl))
+    QT = PD->getType();
+  else
+    return false;
+  // We would like to warn about the 'returns'/'param' commands for
+  // variables that don't directly specify the function type, so type aliases
+  // can be ignored.
+  if (QT->getAs<TypedefType>())
+    return false;
+  return QT->isFunctionPointerType() || QT->isBlockPointerType();
+}
+
 bool Sema::isObjCPropertyDecl() {
   if (!ThisDeclInfo)
     return false;

clang/test/Sema/warn-documentation.cpp

@@ -1210,3 +1210,75 @@ template <class T> T test_function (T arg);
 /*!     @function test_function<int>
 */
 template <> int test_function<int> (int arg);
+
+namespace AllowParamAndReturnsOnFunctionPointerVars {
+
+/**
+ * functionPointerVariable
+ *
+ * @param i is integer.
+ * @returns integer.
+ */
+int (*functionPointerVariable)(int i);
+
+struct HasFields {
+  /**
+   * functionPointerField
+   *
+   * @param i is integer.
+   * @returns integer.
+   */
+  int (*functionPointerField)(int i);
+};
+
+// expected-warning@+5 {{'\returns' command used in a comment that is attached to a function returning void}}
+/**
+ * functionPointerVariable
+ *
+ * \param p not here.
+ * \returns integer.
+ */
+void (*functionPointerVariableThatLeadsNowhere)();
+
+// Still warn about param/returns commands for variables that don't specify
+// the type directly:
+
+/**
+ * FunctionPointerTypedef
+ *
+ * \param i is integer.
+ * \returns integer.
+ */
+typedef int (*FunctionPointerTypedef)(int i);
+
+/**
+ * FunctionPointerTypealias
+ *
+ * \param i is integer.
+ * \returns integer.
+ */
+using FunctionPointerTypealias = int (*)(int i);
+
+// expected-warning@+5 {{'@param' command used in a comment that is not attached to a function declaration}}
+// expected-warning@+5 {{'@returns' command used in a comment that is not attached to a function or method declaration}}
+/**
+ * functionPointerVariable
+ *
+ * @param i is integer.
+ * @returns integer.
+ */
+FunctionPointerTypedef functionPointerTypedefVariable;
+
+struct HasMoreFields {
+  // expected-warning@+5 {{'\param' command used in a comment that is not attached to a function declaration}}
+  // expected-warning@+5 {{'\returns' command used in a comment that is not attached to a function or method declaration}}
+  /**
+   * functionPointerTypealiasField
+   *
+   * \param i is integer.
+   * \returns integer.
+   */
+  FunctionPointerTypealias functionPointerTypealiasField;
+};
+
+}

clang/test/Sema/warn-documentation.m

@@ -229,3 +229,65 @@ int FooBar();
 - (void) VarArgMeth : (id)arg, ... {}
 @end
 
+/**
+ * blockPointerVariable
+ *
+ * @param i is integer.
+ * @returns integer.
+ */
+int (^blockPointerVariable)(int i);
+
+struct HasFields {
+  /**
+   * blockPointerField
+   *
+   * \param i is integer.
+   * \returns integer.
+   */
+  int (^blockPointerFields)(int i);
+};
+
+// expected-warning@+5 {{'\returns' command used in a comment that is attached to a function returning void}}
+/**
+ * functionPointerVariable
+ *
+ * \param p not here.
+ * \returns integer.
+ */
+void (^blockPointerVariableThatLeadsNowhere)();
+
+@interface CheckFunctionBlockPointerVars {
+  /**
+   * functionPointerIVar
+   *
+   * @param i is integer.
+   * @returns integer.
+   */
+  int (*functionPointerIVar)(int i);
+
+  /**
+   * blockPointerIVar
+   *
+   * \param i is integer.
+   * \returns integer.
+   */
+  int (^blockPointerIVar)(int i);
+}
+
+/**
+ * functionPointerProperty
+ *
+ * @param i is integer.
+ * @returns integer.
+ */
+@property int (*functionPointerProperty)(int i);
+
+/**
+ * blockPointerProperty
+ *
+ * \param i is integer.
+ * \returns integer.
+ */
+@property int (^blockPointerProperty)(int i);
+
+@end