<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div dir="auto" style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div dir="auto" style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div dir="auto" style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div dir="auto" style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;"><div dir="auto" style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;">Hello,<div class=""><br class=""></div><div class="">I’d like to propose improving the quality, consistency, and future preparedness of diagnostics through simplification. Right now, a lot of diagnostics output tends to look like this (for example):</div><div class=""><br class=""></div><div class=""><font face="Courier" class=""><span class="Apple-tab-span" style="white-space:pre"> </span>type ‘Foo’ cannot subclass type ‘Bar’</font></div><div class=""><br class=""></div><div class="">Instead of hard coding the word “type” into the message, I’d like to change the diagnostics engine to dynamically introspect the types and print out the type kind instead. Therefore, after this change, the message output would look like this:</div><div class=""><br class=""></div><div class=""><font face="Courier" class=""><span class="Apple-tab-span" style="white-space:pre"> </span>class ‘Foo’ cannot subclass struct ‘Bar’<br class=""></font><br class=""></div><div class="">Which is clearly more better. And it requires less work and specialization on behalf of diagnostics clients because the messages within the compiler “defs” files would change from:</div><div class=""><br class=""></div><div class=""><font face="Courier" class=""><span class="Apple-tab-span" style="white-space:pre"> </span>“type %0 cannot subclass type %1”<br class=""></font><br class=""></div><div class="">To:</div><div class=""><br class=""></div><div class=""><font face="Courier" class=""><span class="Apple-tab-span" style="white-space:pre"> </span>“%0 cannot subclass %1”<br class=""></font></div><div class=""><br class=""></div><div class="">Which is obviously simpler, better, and more future proof. In particular, clients of the diagnostics engine would no longer need to haphazardly customize messages to acknowledge optionals, metatypes, etc as they do today. The engine itself would deal with that.</div><div class=""><br class=""></div><div class="">What do people think? The meat of the change is tiny and below. The vast majority of the work is the long and painful fallout in the test suite, which I’m cautiously (and perhaps foolishly) stepping up to doing.</div><div class=""><br class=""></div><div class="">Cheers,</div><div class="">Dave</div><div class=""><br class=""></div><div class=""><br class=""></div><div class=""><br class=""></div><div class=""><br class=""></div><div class=""><font face="Courier" class="">diff --git c/lib/AST/DiagnosticEngine.cpp i/lib/AST/DiagnosticEngine.cpp<br class="">index c99833d373..32ba196f04 100644<br class="">--- c/lib/AST/DiagnosticEngine.cpp<br class="">+++ i/lib/AST/DiagnosticEngine.cpp<br class="">@@ -373,6 +373,121 @@ static bool shouldShowAKA(Type type, StringRef typeName) {<br class=""> return true;<br class=""> }<br class=""> <br class="">+static std::string computeDiagnosticTypeKindString(Type &type) {<br class="">+ switch (type->getKind()) {<br class="">+#define TYPE(id, parent)<br class="">+#define BUILTIN_TYPE(id, parent) \<br class="">+ case TypeKind::id: return "builtin ";<br class="">+#include <swift/AST/TypeNodes.def><br class="">+ case TypeKind::InOut:<br class="">+ type = type->getInOutObjectType();<br class="">+ return "inout " + computeDiagnosticTypeKindString(type);<br class="">+ case TypeKind::Function: return "function ";<br class="">+ case TypeKind::GenericFunction: return "function ";<br class="">+ case TypeKind::Struct: return "struct ";<br class="">+ case TypeKind::BoundGenericStruct: return "struct ";<br class="">+ case TypeKind::Class: return "class ";<br class="">+ case TypeKind::BoundGenericClass: return "class ";<br class="">+ case TypeKind::Enum: return "enum ";<br class="">+ case TypeKind::BoundGenericEnum: return "enum ";<br class="">+ case TypeKind::ProtocolComposition: return "protocol ";<br class="">+ case TypeKind::Protocol: return "protocol ";<br class="">+ case TypeKind::Module: return "module ";<br class="">+ case TypeKind::Unresolved: return "unresolved type ";<br class="">+ case TypeKind::DynamicSelf: return "dynamic type ";<br class="">+ case TypeKind::TypeVariable: return "type-variable ";<br class="">+ case TypeKind::SILFunction: return "SIL function ";<br class="">+ case TypeKind::SILBlockStorage: return "SIL block storage ";<br class="">+ case TypeKind::SILBox: return "SIL box ";<br class="">+ case TypeKind::GenericTypeParam: return "generic parameter ";<br class="">+ case TypeKind::Archetype: {<br class="">+ auto archetypeTy = dyn_cast<ArchetypeType>(type.getPointer());<br class="">+ if (auto openedTy = archetypeTy->getOpenedExistentialType()) {<br class="">+ type = openedTy;<br class="">+ return computeDiagnosticTypeKindString(type);<br class="">+ }<br class="">+ if (archetypeTy->getAssocType())<br class="">+ return "associated type ";<br class="">+ return "generic parameter ";<br class="">+ }<br class="">+ case TypeKind::DependentMember: {<br class="">+ while (auto depTy = dyn_cast<DependentMemberType>(type.getPointer()))<br class="">+ type = depTy->getBase();<br class="">+ return "dependent " + computeDiagnosticTypeKindString(type);<br class="">+ }<br class="">+ case TypeKind::Metatype: {<br class="">+ auto metaTy = cast<MetatypeType>(type.getPointer());<br class="">+ type = metaTy->getInstanceType();<br class="">+ if (!type->isExistentialType())<br class="">+ return "metatype of " + computeDiagnosticTypeKindString(type);<br class="">+ return "concrete metatype of " + computeDiagnosticTypeKindString(type);<br class="">+ }<br class="">+ case TypeKind::ExistentialMetatype: {<br class="">+ auto metaTy = cast<ExistentialMetatypeType>(type.getPointer());<br class="">+ type = metaTy->getInstanceType();<br class="">+ return "metatype of " + computeDiagnosticTypeKindString(type);<br class="">+ }<br class="">+ case TypeKind::ArraySlice:<br class="">+ // we could in dig further, but then diag messages might become unwieldy<br class="">+ return "array slice ";<br class="">+ case TypeKind::Dictionary:<br class="">+ // we could in dig further, but then diag messages would become unwieldy<br class="">+ return "dictionary ";<br class="">+ case TypeKind::Optional:<br class="">+ type = type->getAnyOptionalObjectType();<br class="">+ return "optional " + computeDiagnosticTypeKindString(type);<br class="">+ case TypeKind::ImplicitlyUnwrappedOptional:<br class="">+ type = type->getAnyOptionalObjectType();<br class="">+ return "implicitly unwrapped optional "<br class="">+ + computeDiagnosticTypeKindString(type);<br class="">+ case TypeKind::Paren:<br class="">+ type = type->getWithoutParens();<br class="">+ return computeDiagnosticTypeKindString(type);<br class="">+ case TypeKind::Tuple: {<br class="">+ auto tempTy = type->getWithoutImmediateLabel();<br class="">+ if (tempTy->isEqual(type))<br class="">+ return "tuple ";<br class="">+ type = tempTy;<br class="">+ return computeDiagnosticTypeKindString(type);<br class="">+ }<br class="">+ case TypeKind::NameAlias: {<br class="">+ // We need to stop updating our caller's 'type' at this point.<br class="">+ auto aliasTy = cast<NameAliasType>(type.getPointer());<br class="">+ Type discardTy = aliasTy->getSinglyDesugaredType();<br class="">+ return computeDiagnosticTypeKindString(discardTy);<br class="">+ }<br class="">+ case TypeKind::Error: {<br class="">+ auto errorTy = cast<ErrorType>(type.getPointer());<br class="">+ if (auto origTy = errorTy->getOriginalType()) {<br class="">+ type = origTy;<br class="">+ return "invalid " + computeDiagnosticTypeKindString(type);<br class="">+ }<br class="">+ return "invalid type ";<br class="">+ }</font></div><div class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">case TypeKind::UnboundGeneric:</span><br style="font-family: Courier;" class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">switch (type->castTo<UnboundGenericType>()->getDecl()->getKind()) {</span><br style="font-family: Courier;" class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">case DeclKind::TypeAlias: return "typealias ";</span><br style="font-family: Courier;" class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">case DeclKind::Protocol:</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">return "protocol ";</span><br style="font-family: Courier;" class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">case DeclKind::Struct:</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">return "struct ";</span><br style="font-family: Courier;" class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">case DeclKind::Class: </span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">return "class ";</span><br style="font-family: Courier;" class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">case DeclKind::Enum:</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">return "enum ";</span><br style="font-family: Courier;" class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">default:</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">break;</span><br style="font-family: Courier;" class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">}</span><br style="font-family: Courier;" class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">llvm_unreachable("Unknown unbound generic");</span><br style="font-family: Courier;" class=""><font face="Courier" class="">+ }</font></div><div class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">case TypeKind::LValue: {</span><br style="font-family: Courier;" class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">// While LValues are not a part of the user visible type system, they do</span><br style="font-family: Courier;" class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">// sometimes appear inside of tuple expressions. Just ignore them.</span><br style="font-family: Courier;" class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">auto lvTy = cast<LValueType>(type.getPointer());</span><br style="font-family: Courier;" class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">type = lvTy->getObjectType();</span><br style="font-family: Courier;" class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">return computeDiagnosticTypeKindString(type);</span><br style="font-family: Courier;" class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">}</span><br style="font-family: Courier;" class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">case TypeKind::WeakStorage:</span><br style="font-family: Courier;" class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">case TypeKind::UnownedStorage:</span><br style="font-family: Courier;" class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">case TypeKind::UnmanagedStorage:</span><br style="font-family: Courier;" class=""><span style="font-family: Courier;" class="">+</span><span style="font-family: Courier;" class=""> </span><span style="font-family: Courier;" class="">llvm_unreachable("Not a part of the user facing type system");</span><br style="font-family: Courier;" class=""><font face="Courier" class="">+}<br class="">+<br class=""> /// \brief Format a single diagnostic argument and write it to the given<br class=""> /// stream.<br class=""> static void formatDiagnosticArgument(StringRef Modifier, <br class="">@@ -439,16 +554,18 @@ static void formatDiagnosticArgument(StringRef Modifier,<br class=""> <br class=""> // Strip extraneous parentheses; they add no value.<br class=""> auto type = Arg.getAsType()->getWithoutParens();<br class="">+ auto origType = type->getInOutObjectType();<br class="">+ std::string declKind = computeDiagnosticTypeKindString(type);<br class=""> std::string typeName = type->getString();<br class=""> <br class=""> if (shouldShowAKA(type, typeName)) {<br class=""> llvm::SmallString<256> AkaText;<br class=""> llvm::raw_svector_ostream OutAka(AkaText);<br class="">- OutAka << type->getCanonicalType();<br class="">- Out << llvm::format(FormatOpts.AKAFormatString.c_str(), typeName.c_str(),<br class="">- AkaText.c_str());<br class="">+ OutAka << origType->getCanonicalType();<br class="">+ Out << declKind << llvm::format(FormatOpts.AKAFormatString.c_str(),<br class="">+ typeName.c_str(), AkaText.c_str());<br class=""> } else {<br class="">- Out << FormatOpts.OpeningQuotationMark << typeName<br class="">+ Out << declKind << FormatOpts.OpeningQuotationMark << typeName<br class=""> << FormatOpts.ClosingQuotationMark;<br class=""> }<br class=""> break;</font><br class=""></div></div></div></div></div></div></body></html>