Instead of passing around an enum, a vector of QualTypes is passed into the
function where the aka strings are generated. This should be the latest
that the comparisons can be held off.
On Thu, May 26, 2011 at 3:06 AM, Chandler Carruth <[email protected]>wrote:
> On Mon, Apr 25, 2011 at 8:38 PM, Richard Trieu <[email protected]> wrote:
>
>> Ping. Any comments on this patch?
>
>
> I like the general direction of this patch, but I have one nit, and one
> more serious comment.
>
> nit: You should switch back to the enum for the ArgumentKind in your
> interface rather than exposing the raw 'char' storage type. This may be made
> irrelevant by the major comment though....
>
> My big issue is similar to Sebastian's. This slows down the *building* of
> diagnostics. It would be much better to do all of this logic in the
> Diagnostic class.
>
> To give a concrete reason, SFINAE: we sometimes build many many diagnostics
> merely to detect the presence of them, discard a particular type with those
> diagnostics, and then begin semantically analyzing something else. We don't
> want to pay for this type of formatting logic in that case. We only want to
> pay for it when we are *emitting* the diagnostic.
>
> Another reason; after we've hit the max diagnostic emitted limit, it would
> be good to cut any and every corner we can to speed things up.
>
> Finally, I think it will be more semantically clear. No need to funnel an
> enum through so many layers, etc.
>
Index: test/Misc/diag-aka-types.cpp
===================================================================
--- test/Misc/diag-aka-types.cpp (revision 133558)
+++ test/Misc/diag-aka-types.cpp (working copy)
@@ -12,3 +12,41 @@
// deduced auto should not produce an aka.
auto aut = X();
char c3 = aut; // expected-error{{from 'X' to 'char'}}
+
+// There are two classes named Foo::foo here. Make sure the message gives
+// a way to them apart.
+namespace Foo {
+ class foo {};
+}
+
+namespace bar {
+ namespace Foo {
+ class foo;
+ }
+ void f(Foo::foo* x); // expected-note{{passing argument to parameter 'x' here}}
+}
+
+void test(Foo::foo* x) {
+ bar::f(x); // expected-error{{cannot initialize a parameter of type 'Foo::foo *' (aka 'bar::Foo::foo *') with an lvalue of type 'Foo::foo *')}}
+}
+
+// PR9548 - "no known conversion from 'vector<string>' to 'vector<string>'"
+// vector<string> refers to two different types here. Make sure the message
+// gives a way to tell them apart.
+class versa_string;
+typedef versa_string string;
+
+namespace std {template <typename T> class vector;}
+using std::vector;
+
+void f(vector<string> v); // expected-note {{candidate function not viable: no known conversion from 'vector<string>' (aka 'std::vector<std::basic_string>') to 'vector<string>' (aka 'std::vector<versa_string>') for 1st argument}}
+
+namespace std {
+ class basic_string;
+ typedef basic_string string;
+ template <typename T> class vector {};
+ void g() {
+ vector<string> v;
+ f(v); // expected-error{{no matching function for call to 'f'}}
+ }
+}
Index: include/clang/Basic/Diagnostic.h
===================================================================
--- include/clang/Basic/Diagnostic.h (revision 133558)
+++ include/clang/Basic/Diagnostic.h (working copy)
@@ -280,7 +280,8 @@
const ArgumentValue *PrevArgs,
unsigned NumPrevArgs,
llvm::SmallVectorImpl<char> &Output,
- void *Cookie);
+ void *Cookie,
+ llvm::SmallVectorImpl<intptr_t> &QualTypeVals);
void *ArgToStringCookie;
ArgToStringFnTy ArgToStringFn;
@@ -457,9 +458,11 @@
const char *Modifier, unsigned ModLen,
const char *Argument, unsigned ArgLen,
const ArgumentValue *PrevArgs, unsigned NumPrevArgs,
- llvm::SmallVectorImpl<char> &Output) const {
+ llvm::SmallVectorImpl<char> &Output,
+ llvm::SmallVectorImpl<intptr_t> &QualTypeVals) const {
ArgToStringFn(Kind, Val, Modifier, ModLen, Argument, ArgLen,
- PrevArgs, NumPrevArgs, Output, ArgToStringCookie);
+ PrevArgs, NumPrevArgs, Output, ArgToStringCookie,
+ QualTypeVals);
}
void SetArgToStringFn(ArgToStringFnTy Fn, void *Cookie) {
Index: include/clang/AST/ASTDiagnostic.h
===================================================================
--- include/clang/AST/ASTDiagnostic.h (revision 133558)
+++ include/clang/AST/ASTDiagnostic.h (working copy)
@@ -42,7 +42,8 @@
const Diagnostic::ArgumentValue *PrevArgs,
unsigned NumPrevArgs,
llvm::SmallVectorImpl<char> &Output,
- void *Cookie);
+ void *Cookie,
+ llvm::SmallVectorImpl<intptr_t> &QualTypeVals);
} // end namespace clang
#endif
Index: lib/Basic/Diagnostic.cpp
===================================================================
--- lib/Basic/Diagnostic.cpp (revision 133558)
+++ lib/Basic/Diagnostic.cpp (working copy)
@@ -26,7 +26,8 @@
const Diagnostic::ArgumentValue *PrevArgs,
unsigned NumPrevArgs,
llvm::SmallVectorImpl<char> &Output,
- void *Cookie) {
+ void *Cookie,
+ llvm::SmallVectorImpl<intptr_t> &QualTypeVals) {
const char *Str = "<can't format argument>";
Output.append(Str, Str+strlen(Str));
}
@@ -542,7 +543,17 @@
/// ConvertArgToString, allowing the implementation to avoid redundancies in
/// obvious cases.
llvm::SmallVector<Diagnostic::ArgumentValue, 8> FormattedArgs;
-
+
+ /// QualTypeVals - Pass a vector of arrays so that QualType names can be
+ /// compared to see if more information is needed to be printed.
+ llvm::SmallVector<intptr_t, 8> QualTypeVals;
+ for (int i = 0; i < getNumArgs(); ++i) {
+ Diagnostic::ArgumentKind Kind = getArgKind(i);
+ if (Kind == Diagnostic::ak_qualtype) {
+ QualTypeVals.push_back(getRawArg(i));
+ }
+ }
+
while (DiagStr != DiagEnd) {
if (DiagStr[0] != '%') {
// Append non-%0 substrings to Str if we have one.
@@ -673,7 +684,7 @@
Modifier, ModifierLen,
Argument, ArgumentLen,
FormattedArgs.data(), FormattedArgs.size(),
- OutStr);
+ OutStr, QualTypeVals);
break;
}
Index: lib/AST/ASTDiagnostic.cpp
===================================================================
--- lib/AST/ASTDiagnostic.cpp (revision 133558)
+++ lib/AST/ASTDiagnostic.cpp (working copy)
@@ -129,7 +129,7 @@
/// \brief Convert the given type to a string suitable for printing as part of
/// a diagnostic.
///
-/// There are three main criteria when determining whether we should have an
+/// There are four main criteria when determining whether we should have an
/// a.k.a. clause when pretty-printing a type:
///
/// 1) Some types provide very minimal sugar that doesn't impede the
@@ -142,16 +142,45 @@
/// want to desugar these, even if we do produce an a.k.a. clause.
/// 3) Some types may have already been desugared previously in this diagnostic.
/// if this is the case, doing another "aka" would just be clutter.
+/// 4) Two different types within the same diagnostic have the same output
+/// string. In this case, force an a.k.a with the desugared type when
+/// doing so will provide additional information.
///
/// \param Context the context in which the type was allocated
/// \param Ty the type to print
+/// \param QualTypeVals pointer values to QualTypes which are used in the
+/// diagnostic message
static std::string
ConvertTypeToDiagnosticString(ASTContext &Context, QualType Ty,
const Diagnostic::ArgumentValue *PrevArgs,
- unsigned NumPrevArgs) {
+ unsigned NumPrevArgs,
+ llvm::SmallVectorImpl<intptr_t> &QualTypeVals) {
// FIXME: Playing with std::string is really slow.
+ bool ForceAKA = false;
+ QualType CanTy = Ty.getCanonicalType();
std::string S = Ty.getAsString(Context.PrintingPolicy);
+ std::string CanS = CanTy.getAsString(Context.PrintingPolicy);
+ for (llvm::SmallVectorImpl<intptr_t>::iterator I = QualTypeVals.begin(),
+ E = QualTypeVals.end(); I != E; ++I) {
+ QualType CompareTy =
+ QualType::getFromOpaquePtr(reinterpret_cast<void*>(*I));
+ if (CompareTy == Ty)
+ continue; // Same types
+ QualType CompareCanTy = CompareTy.getCanonicalType();
+ if (CompareCanTy == CanTy)
+ continue; // Same canonical types
+ std::string CompareS = CompareTy.getAsString(Context.PrintingPolicy);
+ if (CompareS != S)
+ continue; // Original strings are different
+ std::string CompareCanS = CompareCanTy.getAsString(Context.PrintingPolicy);
+ if (CompareCanS == CanS)
+ continue; // No new info from canonical type
+
+ ForceAKA = true;
+ break;
+ }
+
// Check to see if we already desugared this type in this
// diagnostic. If so, don't do it again.
bool Repeated = false;
@@ -172,11 +201,15 @@
if (!Repeated) {
bool ShouldAKA = false;
QualType DesugaredTy = Desugar(Context, Ty, ShouldAKA);
- if (ShouldAKA) {
- S = "'" + S + "' (aka '";
- S += DesugaredTy.getAsString(Context.PrintingPolicy);
- S += "')";
- return S;
+ if (ShouldAKA || ForceAKA) {
+ if (DesugaredTy == Ty) {
+ DesugaredTy = Ty.getCanonicalType();
+ }
+ std::string akaStr = DesugaredTy.getAsString(Context.PrintingPolicy);
+ if (akaStr != S) {
+ S = "'" + S + "' (aka '" + akaStr + "')";
+ return S;
+ }
}
}
@@ -193,7 +226,8 @@
const Diagnostic::ArgumentValue *PrevArgs,
unsigned NumPrevArgs,
llvm::SmallVectorImpl<char> &Output,
- void *Cookie) {
+ void *Cookie,
+ llvm::SmallVectorImpl<intptr_t> &QualTypeVals) {
ASTContext &Context = *static_cast<ASTContext*>(Cookie);
std::string S;
@@ -206,7 +240,8 @@
"Invalid modifier for QualType argument");
QualType Ty(QualType::getFromOpaquePtr(reinterpret_cast<void*>(Val)));
- S = ConvertTypeToDiagnosticString(Context, Ty, PrevArgs, NumPrevArgs);
+ S = ConvertTypeToDiagnosticString(Context, Ty, PrevArgs, NumPrevArgs,
+ QualTypeVals);
NeedQuotes = false;
break;
}
@@ -257,7 +292,7 @@
} else if (TypeDecl *Type = dyn_cast<TypeDecl>(DC)) {
S = ConvertTypeToDiagnosticString(Context,
Context.getTypeDeclType(Type),
- PrevArgs, NumPrevArgs);
+ PrevArgs, NumPrevArgs, QualTypeVals);
} else {
// FIXME: Get these strings from some localized place
NamedDecl *ND = cast<NamedDecl>(DC);
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
