Rework diagnostics

Author: delcypher

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -4445,7 +4445,6 @@ def err_mismatched_visibility: Error<"visibility does not match previous declara
 def note_previous_attribute : Note<"previous attribute is here">;
 def note_conflicting_attribute : Note<"conflicting attribute is here">;
 def note_attribute : Note<"attribute is here">;
-def note_named_attribute : Note<"%0 attribute is here">;
 def err_mismatched_ms_inheritance : Error<
   "inheritance model does not match %select{definition|previous declaration}0">;
 def warn_ignored_ms_inheritance : Warning<
@@ -6713,40 +6712,31 @@ def err_counted_by_attr_pointee_unknown_size : Error<
   "}2">;
 def err_counted_by_on_incomplete_type_on_assign : Error <
   "cannot %select{"
-    "assign to %select{object|'%1'}2 that has|" // AA_Assigning,
-    "pass argument to %select{parameter|parameter '%1'}2 that has|" // AA_Passing,
+    "assign to %select{object|'%1'}2 with|" // AA_Assigning,
+    "pass argument to %select{parameter|parameter '%1'}2 with|" // AA_Passing,
     "return|" // AA_Returning,
     "convert to|" // AA_Converting (UNUSED)
-    "%select{|implicitly }3initialize %select{object|'%1'}2 that has|" // AA_Initializing,
-    "pass argument to parameter that has|" // AA_Sending (UNUSED)
+    "%select{|implicitly }3initialize %select{object|'%1'}2 with|" // AA_Initializing,
+    "pass argument to parameter with|" // AA_Sending (UNUSED)
     "cast to|" // AA_Casting (UNUSED)
-    "pass argument to parameter that has" // AA_Passing_CFAudited (UNUSED)
-  "}0 type %4 because the pointee type %6 is incomplete and the '%5' attribute "
-  "requires the pointee type be complete when %select{"
-    "assigning|" // AA_Assigning,
-    "passing|" // AA_Passing,
-    "returning|" // AA_Returning,
-    "converting|" // AA_Converting (UNUSED)
-    "initializing|" // AA_Initializing,
-    "passing|" // AA_Sending (UNUSED)
-    "casting|" // AA_Casting (UNUSED)
-    "passing" // AA_Passing_CFAudited (UNUSED)
-  "}0; "
-  "consider providing a complete definition for %6 or using the "
-  "'__sized_by%select{|_or_null}7' attribute"
->;
+    "pass argument to parameter with" // AA_Passing_CFAudited (UNUSED)
+  "}0 '%5' attributed type %4 because the pointee type %6 is incomplete">;
+
 def err_counted_by_on_incomplete_type_on_use : Error <
   "cannot %select{"
     "use '%1'|" // Generic expr
     "call '%1'" // CallExpr
   "}0 with%select{"
     "|"       // Generic expr
     " return" // CallExpr
-  "}0 type %2 because the pointee type %3 is incomplete "
-  "and the '%4' attribute requires the pointee type be complete in this context; "
-  "consider providing a complete definition for %3 or using the "
-  "'__sized_by%select{|_or_null}5' attribute"
->;
+  "}0 '%4' attributed type %2 because the pointee type %3 is incomplete">;
+
+def note_counted_by_consider_completing_pointee_ty : Note<
+  "consider providing a complete definition for %0">;
+  
+def note_counted_by_consider_using_sized_by : Note<
+  "consider using '__sized_by%select{|_or_null}0' instead of "
+  "__counted_by%select{|_or_null}0">;
 
 def warn_counted_by_attr_elt_type_unknown_size :
   Warning<err_counted_by_attr_pointee_unknown_size.Summary>,

clang/lib/Sema/SemaBoundsSafety.cpp

@@ -220,41 +220,40 @@ bool Sema::CheckCountedByAttrOnField(FieldDecl *FD, Expr *E, bool CountInBytes,
 
 static void EmitIncompleteCountedByPointeeNotes(Sema &S,
                                                 const CountAttributedType *CATy,
-                                                NamedDecl *IncompleteTyDecl,
-                                                bool NoteAttrLocation = true) {
+                                                NamedDecl *IncompleteTyDecl) {
   assert(IncompleteTyDecl == nullptr || isa<TypeDecl>(IncompleteTyDecl));
 
-  if (NoteAttrLocation) {
-    // Note where the attribute is declared
-    // This is an approximation that's not quite right. This points to the
-    // the expression inside the attribute rather than the attribute itself.
+  if (IncompleteTyDecl) {
+    // Suggest completing the pointee type if its a named typed (i.e.
+    // IncompleteTyDecl isn't nullptr). Suggest this first as it is more likely
+    // to be the correct fix.
     //
-    // TODO: Implement logic to find the relevant TypeLoc for the attribute and
-    // get the SourceRange from that (#113582).
-    SourceRange AttrSrcRange = CATy->getCountExpr()->getSourceRange();
-    S.Diag(AttrSrcRange.getBegin(), diag::note_named_attribute)
-        << CATy->getAttributeName(/*WithMacroPrefix=*/true) << AttrSrcRange;
+    // Note the `IncompleteTyDecl` type is the underlying type which might not
+    // be the same as `CATy->getPointeeType()` which could be a typedef.
+    //
+    // The diagnostic printed will be at the location of the underlying type but
+    // the diagnostic text will print the type of `CATy->getPointeeType()` which
+    // could be a typedef name rather than the underlying type. This is ok
+    // though because the diagnostic will print the underlying type name too.
+    S.Diag(IncompleteTyDecl->getBeginLoc(),
+           diag::note_counted_by_consider_completing_pointee_ty)
+        << CATy->getPointeeType();
   }
 
-  if (!IncompleteTyDecl)
-    return;
-
-  // If there's an associated forward declaration display it to emphasize
-  // why the type is incomplete (all we have is a forward declaration).
-
-  // Note the `IncompleteTyDecl` type is the underlying type which might not
-  // be the same as `CATy->getPointeeType()` which could be a typedef.
+  // Suggest using __sized_by(_or_null) instead of __counted_by(_or_null) as
+  // __sized_by(_or_null) doesn't have the complete type restriction.
+  //
+  // We use the source range of the expression on the CountAttributedType as an
+  // approximation for the source range of the attribute. This isn't quite right
+  // but isn't easy to fix right now.
   //
-  // The diagnostic printed will be at the location of the underlying type but
-  // the diagnostic text will print the type of `CATy->getPointeeType()` which
-  // could be a typedef name rather than the underlying type. This is ok
-  // though because the diagnostic will print the underlying type name too.
-  // E.g:
+  // TODO: Implement logic to find the relevant TypeLoc for the attribute and
+  // get the SourceRange from that (#113582).
   //
-  // `forward declaration of 'Incomplete_Struct_t'
-  //  (aka 'struct IncompleteStructTy')`
-  S.Diag(IncompleteTyDecl->getBeginLoc(), diag::note_forward_declaration)
-      << CATy->getPointeeType();
+  // TODO: We should emit a fix-it here.
+  SourceRange AttrSrcRange = CATy->getCountExpr()->getSourceRange();
+  S.Diag(AttrSrcRange.getBegin(), diag::note_counted_by_consider_using_sized_by)
+      << CATy->isOrNull() << AttrSrcRange;
 }
 
 static std::tuple<const CountAttributedType *, QualType>