[Basic] Extend DiagnosticEngine to store and format Qualifiers

[Anastasia]

Summary

This is a follow up patch from the following review comment: https://reviews.llvm.org/D55850?id=178767#inline-495051

[rjmccall]

include/clang/AST/Type.h

6705

Unpaired apostrophe, here and below.

include/clang/Basic/DiagnosticSemaKinds.td

1812

The line break is no longer necessary.

test/SemaCXX/addr-of-overloaded-function.cpp

224–230

Hmm. I think '' is acceptable for the unqualified case in this diagnostic because at least one of the operands has to be non-empty, but it might be better overall to say something like unqualified (not in quotes, of course).

I like the general effect of this, though; this is definitely a more readable diagnostic in the complex cases than it used to be.

[Anastasia]

Addressed review comments.

[rjmccall]

Thanks, LGTM.

include/clang/AST/Type.h

inline const Type *Type::getPointeeOrArrayElementType() const {
   const Type *type = this;
   if (type->isAnyPointerType())
     return type->getPointeeType().getTypePtr();
   else if (type->isArrayType())
     return type->getBaseElementTypeUnsafe();
   return type;
 }
 
+/// Insertion operator for diagnostics. This allows sending Qualifiers into a
+/// diagnostic with <<.
+inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB,
+                                           Qualifiers Q) {
+  DB.AddTaggedVal(Q.getAsOpaqueValue(),
+                  DiagnosticsEngine::ArgumentKind::ak_qual);
+  return DB;
+}
+
+/// Insertion operator for partial diagnostics. This allows sending Qualifiers
+/// into a diagnostic with <<.
+inline const PartialDiagnostic &operator<<(const PartialDiagnostic &PD,
+                                           Qualifiers Q) {
+  PD.AddTaggedVal(Q.getAsOpaqueValue(),
+                  DiagnosticsEngine::ArgumentKind::ak_qual);
+  return PD;
+}
+
 /// Insertion operator for diagnostics.  This allows sending QualType's into a
 /// diagnostic with <<.
 inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB,
                                            QualType T) {
   DB.AddTaggedVal(reinterpret_cast<intptr_t>(T.getAsOpaquePtr()),
                   DiagnosticsEngine::ak_qualtype);
   return DB;
 }

include/clang/Basic/Diagnostic.h

enum ArgumentKind {
     ak_uint,
 
     /// enum TokenKind : unsigned
     ak_tokenkind,
 
     /// IdentifierInfo
     ak_identifierinfo,
 
+    /// Qualifiers
+    ak_qual,
+
     /// QualType
     ak_qualtype,
 
     /// DeclarationName
     ak_declarationname,
 
     /// NamedDecl *
     ak_nameddecl,

include/clang/Basic/DiagnosticSemaKinds.td

def err_init_conversion_failed : Error<
   "block element|a complex element|a lambda capture|a compound literal "
   "initializer|a related result|a parameter of CF audited function}0 "
   "%diff{of type $ with an %select{rvalue|lvalue}2 of type $|"
   "with an %select{rvalue|lvalue}2 of incompatible type}1,3"
   "%select{|: different classes%diff{ ($ vs $)|}5,6"
   "|: different number of parameters (%5 vs %6)"
   "|: type mismatch at %ordinal5 parameter%diff{ ($ vs $)|}6,7"
   "|: different return type%diff{ ($ vs $)|}5,6"
-  "|: different qualifiers ("
-  "%select{none|const|restrict|const and restrict|volatile|const and volatile|"
-  "volatile and restrict|const, volatile, and restrict}5 vs "
-  "%select{none|const|restrict|const and restrict|volatile|const and volatile|"
-  "volatile and restrict|const, volatile, and restrict}6)"
+  "|: different qualifiers (%5 vs %6)"
   "|: different exception specifications}4">;
 
 def err_lvalue_to_rvalue_ref : Error<"rvalue reference %diff{to type $ cannot "
   "bind to lvalue of type $|cannot bind to incompatible lvalue}0,1">;
 def err_lvalue_reference_bind_to_initlist : Error<
   "%select{non-const|volatile}0 lvalue reference to type %1 cannot bind to an "
   "initializer list temporary">;
 def err_lvalue_reference_bind_to_temporary : Error<

 
 def note_ovl_candidate : Note<
     "candidate %sub{select_ovl_candidate_kind}0,1,3"
     "%select{| has different class%diff{ (expected $ but has $)|}5,6"
     "| has different number of parameters (expected %5 but has %6)"
     "| has type mismatch at %ordinal5 parameter"
     "%diff{ (expected $ but has $)|}6,7"
     "| has different return type%diff{ ($ expected but has $)|}5,6"
-    "| has different qualifiers (expected "
-    "%select{none|const|restrict|const and restrict|volatile|const and volatile"
-    "|volatile and restrict|const, volatile, and restrict}5 but found "
-    "%select{none|const|restrict|const and restrict|volatile|const and volatile"
-    "|volatile and restrict|const, volatile, and restrict}6)"
+    "| has different qualifiers (expected %5 but found %6)"
     "| has different exception specification}4">;
 
 def note_ovl_candidate_inherited_constructor : Note<
     "constructor from base class %0 inherited here">;
 def note_ovl_candidate_inherited_constructor_slice : Note<
     "candidate %select{constructor|template}0 ignored: "
     "inherited constructor cannot be used to %select{copy|move}1 object">;
 def note_ovl_candidate_illegal_constructor : Note<
def warn_overloaded_virtual : Warning<
   "%q0 hides overloaded virtual %select{function|functions}1">,
   InGroup<OverloadedVirtual>, DefaultIgnore;
 def note_hidden_overloaded_virtual_declared_here : Note<
   "hidden overloaded virtual function %q0 declared here"
   "%select{|: different classes%diff{ ($ vs $)|}2,3"
   "|: different number of parameters (%2 vs %3)"
   "|: type mismatch at %ordinal2 parameter%diff{ ($ vs $)|}3,4"
   "|: different return type%diff{ ($ vs $)|}2,3"
-  "|: different qualifiers ("
-  "%select{none|const|restrict|const and restrict|volatile|const and volatile|"
-  "volatile and restrict|const, volatile, and restrict}2 vs "
-  "%select{none|const|restrict|const and restrict|volatile|const and volatile|"
-  "volatile and restrict|const, volatile, and restrict}3)"
+  "|: different qualifiers (%2 vs %3)"
   "|: different exception specifications}1">;
 def warn_using_directive_in_header : Warning<
   "using namespace directive in global context in header">,
   InGroup<HeaderHygiene>, DefaultIgnore;
 def warn_overaligned_type : Warning<
   "type %0 requires %1 bytes of alignment and the default allocator only "
   "guarantees %2 bytes">,
   InGroup<OveralignedType>, DefaultIgnore;
def err_typecheck_convert_incompatible : Error<
   "%select{|; dereference with *|"
   "; take the address with &|"
   "; remove *|"
   "; remove &}3"
   "%select{|: different classes%diff{ ($ vs $)|}5,6"
   "|: different number of parameters (%5 vs %6)"
   "|: type mismatch at %ordinal5 parameter%diff{ ($ vs $)|}6,7"
   "|: different return type%diff{ ($ vs $)|}5,6"
-  "|: different qualifiers ("
-  "%select{none|const|restrict|const and restrict|volatile|const and volatile|"
-  "volatile and restrict|const, volatile, and restrict}5 vs "
-  "%select{none|const|restrict|const and restrict|volatile|const and volatile|"
-  "volatile and restrict|const, volatile, and restrict}6)"
+  "|: different qualifiers (%5 vs %6)"
   "|: different exception specifications}4">;
 def err_typecheck_missing_return_type_incompatible : Error<
   "%diff{return type $ must match previous return type $|"
   "return type must match previous return type}0,1 when %select{block "
   "literal|lambda expression}2 has unspecified explicit return type">;
 
 def note_incomplete_class_and_qualified_id : Note<
   "conformance of forward class %0 to protocol %1 can not be confirmed">;

lib/AST/ASTDiagnostic.cpp

void clang::FormatASTNodeDiagnosticArgument(
   ASTContext &Context = *static_cast<ASTContext*>(Cookie);
 
   size_t OldEnd = Output.size();
   llvm::raw_svector_ostream OS(Output);
   bool NeedQuotes = true;
 
   switch (Kind) {
     default: llvm_unreachable("unknown ArgumentKind");
+    case DiagnosticsEngine::ak_qual: {
+      assert(Modifier.empty() && Argument.empty() &&
+             "Invalid modifier for Qualfiers argument");
+
+      Qualifiers Q(Qualifiers::fromOpaqueValue(Val));
+      auto S = Q.getAsString();
+      if (S.empty()) {
+        OS << "unqualified";
+        NeedQuotes = false;
+      } else {
+        OS << Q.getAsString();
+      }
+      break;
+    }
     case DiagnosticsEngine::ak_qualtype_pair: {
       TemplateDiffTypes &TDT = *reinterpret_cast<TemplateDiffTypes*>(Val);
       QualType FromType =
           QualType::getFromOpaquePtr(reinterpret_cast<void*>(TDT.FromType));
       QualType ToType =
           QualType::getFromOpaquePtr(reinterpret_cast<void*>(TDT.ToType));
 
       if (FormatTemplateTypeDiff(Context, FromType, ToType, TDT.PrintTree,

lib/Basic/Diagnostic.cpp

case DiagnosticsEngine::ak_identifierinfo: {
         const char *S = "(null)";
         OutStr.append(S, S + strlen(S));
         continue;
       }
 
       llvm::raw_svector_ostream(OutStr) << '\'' << II->getName() << '\'';
       break;
     }
+    case DiagnosticsEngine::ak_qual:
     case DiagnosticsEngine::ak_qualtype:
     case DiagnosticsEngine::ak_declarationname:
     case DiagnosticsEngine::ak_nameddecl:
     case DiagnosticsEngine::ak_nestednamespec:
     case DiagnosticsEngine::ak_declcontext:
     case DiagnosticsEngine::ak_attr:
       getDiags()->ConvertArgToString(Kind, getRawArg(ArgNo),
                                      StringRef(Modifier, ModifierLen),

lib/Sema/SemaOverload.cpp

void Sema::HandleFunctionTypeMismatch(PartialDiagnostic &PDiag,
   // Handle different return type.
   if (!Context.hasSameType(FromFunction->getReturnType(),
                            ToFunction->getReturnType())) {
     PDiag << ft_return_type << ToFunction->getReturnType()
           << FromFunction->getReturnType();
     return;
   }
 
-  // FIXME: OpenCL: Need to consider address spaces
-  unsigned FromQuals = FromFunction->getTypeQuals().getCVRUQualifiers();
-  unsigned ToQuals = ToFunction->getTypeQuals().getCVRUQualifiers();
-  if (FromQuals != ToQuals) {
-    PDiag << ft_qualifer_mismatch << ToQuals << FromQuals;
+  if (FromFunction->getTypeQuals() != ToFunction->getTypeQuals()) {
+    PDiag << ft_qualifer_mismatch << ToFunction->getTypeQuals()
+          << FromFunction->getTypeQuals();
     return;
   }
 
   // Handle exception specification differences on canonical type (in C++17
   // onwards).
   if (cast<FunctionProtoType>(FromFunction->getCanonicalTypeUnqualified())
           ->isNothrow() !=
       cast<FunctionProtoType>(ToFunction->getCanonicalTypeUnqualified())

test/SemaCXX/addr-of-overloaded-function.cpp

struct Qualifiers {
     void CR() const __restrict {};
     void VR() volatile __restrict {};
     void CVR() const volatile __restrict {};
   };
 
 
   void QualifierTest() {
     void (Qualifiers::*X)();
-    X = &Qualifiers::C; // expected-error-re {{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const': different qualifiers (none vs const)}}
-    X = &Qualifiers::V; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} volatile': different qualifiers (none vs volatile)}}
-    X = &Qualifiers::R; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} __restrict': different qualifiers (none vs restrict)}}
-    X = &Qualifiers::CV; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const volatile': different qualifiers (none vs const and volatile)}}
-    X = &Qualifiers::CR; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const __restrict': different qualifiers (none vs const and restrict)}}
-    X = &Qualifiers::VR; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} volatile __restrict': different qualifiers (none vs volatile and restrict)}}
-    X = &Qualifiers::CVR; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const volatile __restrict': different qualifiers (none vs const, volatile, and restrict)}}
+    X = &Qualifiers::C; // expected-error-re {{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const': different qualifiers (unqualified vs 'const')}}
+    X = &Qualifiers::V; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} volatile': different qualifiers (unqualified vs 'volatile')}}
+    X = &Qualifiers::R; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} __restrict': different qualifiers (unqualified vs '__restrict')}}
+    X = &Qualifiers::CV; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const volatile': different qualifiers (unqualified vs 'const volatile')}}
+    X = &Qualifiers::CR; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const __restrict': different qualifiers (unqualified vs 'const __restrict')}}
+    X = &Qualifiers::VR; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} volatile __restrict': different qualifiers (unqualified vs 'volatile __restrict')}}
+    X = &Qualifiers::CVR; // expected-error-re{{assigning to 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' from incompatible type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}} const volatile __restrict': different qualifiers (unqualified vs 'const volatile __restrict')}}
   }
 
   struct Dummy {
     void N() {};
   };
 
   void (Qualifiers::*X)() = &Dummy::N; // expected-error-re{{cannot initialize a variable of type 'void (test1::Qualifiers::*)(){{( __attribute__\(\(thiscall\)\))?}}' with an rvalue of type 'void (test1::Dummy::*)(){{( __attribute__\(\(thiscall\)\))?}}': different classes ('test1::Qualifiers' vs 'test1::Dummy')}}
 }
 
 template <typename T> class PR16561 {
   virtual bool f() { if (f) {} return false; } // expected-error {{reference to non-static member function must be called}}
 };

test/SemaCXX/warn-overloaded-virtual.cpp

namespace {
   class A {
     virtual int foo(bool) const;
     // expected-note@-1{{type mismatch at 1st parameter ('bool' vs 'int')}}
     virtual int foo(int, int) const;
     // expected-note@-1{{different number of parameters (2 vs 1)}}
     virtual int foo(int*) const;
     // expected-note@-1{{type mismatch at 1st parameter ('int *' vs 'int')}}
     virtual int foo(int) volatile;
-    // expected-note@-1{{different qualifiers (volatile vs const)}}
+    // expected-note@-1{{different qualifiers ('volatile' vs 'const')}}
   };
 
   class B : public A {
     virtual int foo(int) const;
     // expected-warning@-1{{hides overloaded virtual functions}}
   };
 }