fixup: refactor semantic checks

Change error descriptions and restructure the checks code.

Author: a-nogikh

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -3453,14 +3453,18 @@ def warn_attribute_return_span_only
     : Warning<"%0 attribute only applies to functions that return span-like "
               "structures">,
       InGroup<IgnoredAttributes>;
-def note_span_must_be_struct : Note<"span-like type must be a struct">;
-def note_span_must_be_complete : Note<"span-like type must be complete">;
-def note_wrong_span_field_count : Note<"span-like type must have 2 fields">;
-def note_wrong_span_field_types
-    : Note<"span-like type must have a pointer and an integer field or two "
-           "pointer fields">;
-def note_span_invalid_integer : Note<"the integer field must be an actual "
-                                     "integer that is at least as big as int">;
+def note_returned_not_struct : Note<"returned type is not a struct/class type">;
+def note_returned_incomlete_type : Note<"returned type is incomplete">;
+def note_returned_not_two_field_struct
+    : Note<"returned struct/class has %0 fields, expected 2">;
+def note_returned_not_span_struct
+    : Note<"returned struct/class fields are not a supported combination for a "
+           "span-like type (expected pointer/integer or pointer/pointer)">;
+def note_returned_not_integer_field
+    : Note<"field #%0 expected to be an integer">;
+def note_returned_not_wide_enough_field
+    : Note<"integer field #%0 of span-like type is not wide enough (minimum "
+           "width: %1)">;
 def warn_attribute_return_pointers_refs_only : Warning<
   "%0 attribute only applies to return values that are pointers or references">,
   InGroup<IgnoredAttributes>;

clang/lib/Sema/SemaDeclAttr.cpp

@@ -1850,59 +1850,51 @@ static bool checkSpanLikeType(Sema &S, const ParsedAttr &AL,
   // lead to false positive results.
   auto emitWarning = [&S, &AL](unsigned NoteDiagID) {
     S.Diag(AL.getLoc(), diag::warn_attribute_return_span_only) << AL;
-    S.Diag(AL.getLoc(), NoteDiagID);
+    return S.Diag(AL.getLoc(), NoteDiagID);
   };
+  if (Ty->isIncompleteType())
+    return emitWarning(diag::note_returned_incomlete_type);
   const RecordDecl *RD = Ty->getAsRecordDecl();
-  if (!RD || RD->isUnion()) {
-    emitWarning(diag::note_span_must_be_struct);
-    return false;
-  }
-  const RecordDecl *Def = RD->getDefinition();
-  if (!Def) {
-    emitWarning(diag::note_span_must_be_complete);
-    return false; // This is an incomplete type.
-  }
-  auto FieldsBegin = Def->field_begin();
-  if (std::distance(FieldsBegin, Def->field_end()) != 2) {
-    emitWarning(diag::note_wrong_span_field_count);
-    return false;
-  }
+  if (!RD || RD->isUnion())
+    return emitWarning(diag::note_returned_not_struct);
+  auto FieldsBegin = RD->field_begin();
+  const auto FieldsCount = std::distance(FieldsBegin, RD->field_end());
+  if (FieldsCount != 2)
+    return emitWarning(diag::note_returned_not_two_field_struct) << FieldsCount;
   const QualType FirstFieldType = FieldsBegin->getType();
   const QualType SecondFieldType = std::next(FieldsBegin)->getType();
   auto validatePointerType = [](const QualType &T) {
     // It must not point to functions.
     return T->isPointerType() && !T->isFunctionPointerType();
   };
-  auto checkIntegerType = [&S, emitWarning](const QualType &T) {
-    bool valid = false;
-    // Must be an actual integer and at least as bit as int.
-    if (const auto *BT = dyn_cast<BuiltinType>(T.getCanonicalType())) {
-      const auto IntSize = S.Context.getTypeSize(S.Context.IntTy);
-      valid = BT->isInteger() && S.Context.getTypeSize(BT) >= IntSize;
-    }
-    if (!valid) {
-      emitWarning(diag::note_span_invalid_integer);
-    }
-    return valid;
+  auto checkIntegerType = [&S, emitWarning](const QualType &T,
+                                            const int FieldNo) -> bool {
+    const auto *BT = dyn_cast<BuiltinType>(T.getCanonicalType());
+    if (!BT || !BT->isInteger())
+      return emitWarning(diag::note_returned_not_integer_field) << FieldNo;
+    const auto IntSize = S.Context.getTypeSize(S.Context.IntTy);
+    if (S.Context.getTypeSize(BT) < IntSize)
+      return emitWarning(diag::note_returned_not_wide_enough_field)
+             << FieldNo << IntSize;
+    return false;
   };
   if (validatePointerType(FirstFieldType) &&
       validatePointerType(SecondFieldType)) {
     // Pointer + pointer.
-    return true;
+    return false;
   } else if (validatePointerType(FirstFieldType)) {
     // Pointer + integer?
-    return checkIntegerType(SecondFieldType);
+    return checkIntegerType(SecondFieldType, 2);
   } else if (validatePointerType(SecondFieldType)) {
     // Integer + pointer?
-    return checkIntegerType(FirstFieldType);
+    return checkIntegerType(FirstFieldType, 1);
   }
-  emitWarning(diag::note_wrong_span_field_types);
-  return false;
+  return emitWarning(diag::note_returned_not_span_struct);
 }
 
 static void handleMallocSpanAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   QualType ResultType = getFunctionOrMethodResultType(D);
-  if (checkSpanLikeType(S, AL, ResultType)) {
+  if (!checkSpanLikeType(S, AL, ResultType)) {
     D->addAttr(::new (S.Context) MallocSpanAttr(S.Context, AL));
   }
 }

clang/test/Sema/attr-malloc_span.c

@@ -9,7 +9,6 @@ typedef struct {
 } span;
 span  returns_span  (void) __attribute((malloc_span)); // no-warning
 
-// Try out a different field ordering.
 typedef struct {
   size_t n;
   void *ptr;
@@ -21,8 +20,20 @@ typedef struct {
   void *ptr2;
 } span3;
 span3  returns_span3  (void) __attribute((malloc_span)); // no-warning
+
+typedef struct {
+  void *ptr;
+  int n;
+} span4;
+span4  returns_span4  (void) __attribute((malloc_span)); // no-warning
+
+typedef struct incomplete_span incomplete_span;
+// expected-warning@+2 {{attribute only applies to functions that return span-like structures}}
+// expected-note@+1 {{returned type is incomplete}}
+incomplete_span returns_incomplete_span (void) __attribute((malloc_span));
+
 // expected-warning@+2 {{attribute only applies to functions that return span-like structures}}
-// expected-note@+1 {{span-like type must be a struct}}
+// expected-note@+1 {{returned type is not a struct/class type}}
 int *returns_int_ptr  (void) __attribute((malloc_span));
 
 typedef struct {
@@ -31,7 +42,7 @@ typedef struct {
   size_t n2;
 } too_long_span;
 // expected-warning@+2 {{attribute only applies to functions that return span-like structures}}
-// expected-note@+1 {{span-like type must have 2 fields}}
+// expected-note@+1 {{returned struct/class has 3 fields, expected 2}}
 too_long_span  returns_too_long_span  (void) __attribute((malloc_span));
 
 // Function pointers are not allowed.
@@ -40,7 +51,7 @@ typedef struct {
   size_t n;
 } func_span;
 // expected-warning@+2 {{attribute only applies to functions that return span-like structures}}
-// expected-note@+1 {{span-like type must have a pointer and an integer field or two pointer fields}}
+// expected-note@+1 {{returned struct/class fields are not a supported combination}}
 func_span  returns_func_span  (void) __attribute((malloc_span));
 
 // Integer should not be an enum.
@@ -50,7 +61,7 @@ typedef struct {
   enum some_enum field;
 } enum_span;
 // expected-warning@+2 {{attribute only applies to functions that return span-like structures}}
-// expected-note@+1 {{the integer field must be an actual integer}}
+// expected-note@+1 {{field #2 expected to be an integer}}
 enum_span  returns_enum_span  (void) __attribute((malloc_span));
 
 // Bit integers are also not supported.
@@ -59,7 +70,7 @@ typedef struct {
   _BitInt(16) n;
 } bit_span;
 // expected-warning@+2 {{attribute only applies to functions that return span-like structures}}
-// expected-note@+1 {{the integer field must be an actual integer}}
+// expected-note@+1 {{field #2 expected to be an integer}}
 bit_span  returns_bit_span  (void) __attribute((malloc_span));
 
 // Integer must be at least as big as int.
@@ -68,5 +79,5 @@ typedef struct {
   short n;
 } short_span;
 // expected-warning@+2 {{attribute only applies to functions that return span-like structures}}
-// expected-note@+1 {{the integer field must be an actual integer}}
+// expected-note@+1 {{integer field #2 of span-like type is not wide enough (minimum width: 32)}}
 short_span  returns_short_span  (void) __attribute((malloc_span));

clang/test/SemaCXX/attr-malloc_span.cpp

@@ -12,7 +12,7 @@ struct DataMemberSpan {
 };
 
 // expected-warning@+2 {{attribute only applies to functions that return span-like structures}}
-// expected-note@+1 {{span-like type must have a pointer and an integer field or two pointer fields}}
+// expected-note@+1 {{returned struct/class fields are not a supported combination}}
 DataMemberSpan returns_data_member_span(void) __attribute((malloc_span)) {
   return DataMemberSpan{};
 }
@@ -24,7 +24,7 @@ struct MemberFuncSpan {
 };
 
 // expected-warning@+2 {{attribute only applies to functions that return span-like structures}}
-// expected-note@+1 {{span-like type must have a pointer and an integer field or two pointer fields}}
+// expected-note@+1 {{returned struct/class fields are not a supported combination}}
 MemberFuncSpan returns_member_func_span(void) __attribute((malloc_span)) {
   return MemberFuncSpan{};
 }