[cfe-commits] Request feedback for new diagnostic feature - template type diffing
Richard Smith
richard at metafoo.co.uk
Sat Sep 3 20:51:09 PDT 2011
Hi Richard,
On Thu, September 1, 2011 04:57, Richard Trieu wrote:
> I believe that the template diffing now supports most of the template
> parameters now. SubstNonTypeTemplateParmExpr parameters was the last thing I
> added support for.
>
> Per request, template aliasing is supported by finding the most specialized
> template alias common to both types and using it for the diff. May need to add
> some indication that aliases are being stripped off to be more helpful.
>
> The type diffing is working well at the current position. Next move is to
> add support to other diagnostic messages and see if there's any cases I've
> missed.
I'm still not convinced that emitting extra notes is the right approach in the
long term. Some comments on the patch itself below:
> Index: include/clang/Basic/DiagnosticSemaKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticSemaKinds.td (revision 138851)
> +++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
> @@ -14,6 +14,8 @@
> let Component = "Sema" in {
> let CategoryName = "Semantic Issue" in {
>
> +def note_type_diff : Note<"Type %0 is different from type %1">;
Outside the static analyser, clang diagnostics should not start with a capital
letter.
> +
> // Constant expressions
> def err_expr_not_ice : Error<
> "expression is not an integer constant expression">;
> Index: include/clang/Sema/Sema.h
> ===================================================================
> --- include/clang/Sema/Sema.h (revision 138851)
> +++ include/clang/Sema/Sema.h (working copy)
> @@ -3490,6 +3490,8 @@
> void MarkAsLateParsedTemplate(FunctionDecl *FD, bool Flag = true);
> bool IsInsideALocalClassWithinATemplateFunction();
>
> + void EmitTemplateTypeDiff(SourceLocation Loc, QualType FromTy, QualType
ToTy);
> +
> Decl *ActOnStaticAssertDeclaration(SourceLocation StaticAssertLoc,
> Expr *AssertExpr,
> Expr *AssertMessageExpr,
> Index: lib/Sema/SemaTemplate.cpp
> ===================================================================
> --- lib/Sema/SemaTemplate.cpp (revision 138851)
> +++ lib/Sema/SemaTemplate.cpp (working copy)
> @@ -6743,3 +6743,513 @@
> }
> return false;
> }
> +
> +// This iterator is used to enter a TemplateSpecializationType and read
> +// TemplateArguments inside template parameter packs in order with the rest
> +// of the TemplateArguments.
> +struct TSTiterator {
> + const TemplateSpecializationType *TST;
> + unsigned index;
> + TemplateArgument::pack_iterator currentTA;
> + TemplateArgument::pack_iterator endTA;
> +
> + TSTiterator(const TemplateSpecializationType *TST)
> + : TST(TST), index(0), currentTA(0), endTA(0) {
> + for(index = 0; index < TST->getNumArgs(); ++index) {
> + TemplateArgument TA = TST->getArg(index);
> + if (TA.getKind() != TemplateArgument::Pack) {
> + break;
> + } else {
> + currentTA = TA.pack_begin();
> + endTA = TA.pack_end();
> + if (currentTA != endTA) {
> + break;
> + }
> + }
> + }
> + }
Could you refactor this to call operator++?
> +
> + bool operator==(const TSTiterator &Iter) {
> + return (this->TST == Iter.TST && this->index == Iter.index &&
> + this->currentTA == Iter.currentTA && this->endTA == Iter.endTA);
> + }
Redundant parens.
> +
> + bool operator!=(const TSTiterator &Iter) {
> + return !(*this == Iter);
> + }
> +
> + bool isEnd() {
> + return index == TST->getNumArgs();
> + }
> +
> + bool isBegin() {
> + if (index == TST->getNumArgs()) return false;
> + for (unsigned i = 0; i < index; ++i) {
> + TemplateArgument TA = TST->getArg(i);
> + if (TA.getKind() != TemplateArgument::Pack) return false;
> + if (TA.pack_begin() != TA.pack_end()) return false;
> + }
> + TemplateArgument TA = TST->getArg(index);
> + if (TA.getKind() != TemplateArgument::Pack) return true;
> + if (TA.pack_begin() == currentTA) return true;
> + return false;
> + }
This function can be avoided by a small tweak to your main loop. More below.
> +
> + TSTiterator& operator++() {
clang convention is to put & and * next to the identifier, not next to the type.
> + while (index < TST->getNumArgs()) {
This could equally be while (true).
> + if (currentTA != endTA) ++currentTA;
> + if (currentTA == endTA) {
> + ++index;
> + if (index == TST->getNumArgs()) break;
> + TemplateArgument TA = TST->getArg(index);
> + if (TA.getKind() == TemplateArgument::Pack) {
> + currentTA = TA.pack_begin();
> + endTA = TA.pack_end();
> + if (currentTA != endTA) break;
> + } else {
> + break;
> + }
> + }
> + }
> + return *this;
> + }
> +
> + const TemplateArgument& operator*() {
> + assert(index < TST->getNumArgs() && "Index exceeds number of arguments.");
> + if (currentTA == endTA)
> + return TST->getArg(index);
> + else
> + return *currentTA;
> + }
> +
> + const TemplateArgument* operator->() {
> + assert(index < TST->getNumArgs() && "Index exceeds number of arguments.");
> + if (currentTA == endTA)
> + return TST->getArgs() + index;
> + else
> + return currentTA;
You could just return &operator*() here.
> + }
> +};
> +
> +// Special handling of typenames. When printing two types, print the
canonical
> +// type when the original typenames has the same name. If the canonical type
> +// names are also the same, use the original name.
> +static void AppendTypeNames(QualType FromType, QualType ToType,
> + std::string &FromStr, std::string &ToStr) {
> + std::string FromTypeStr = FromType.getAsString();
> + std::string ToTypeStr = ToType.getAsString();
> +
> + const TemplateSpecializationType *FromParam =
> + FromType->getAs<TemplateSpecializationType>();
> + const TemplateSpecializationType *ToParam =
> + ToType->getAs<TemplateSpecializationType>();
> +
> + // For templated types, only print the typename and "< >" to prevent overly
> + // long names.
> + if (FromParam) {
> + FromStr +=
> + FromParam->getTemplateName().getAsTemplateDecl()->getNameAsString();
> + FromStr += "< >";
> + }
> + if (ToParam) {
> + ToStr +=
> + ToParam->getTemplateName().getAsTemplateDecl()->getNameAsString();
> + ToStr += "< >";
> + }
Can we use a different sequence of characters for this? T< > is legal syntax
for an empty template argument list, and that's not what this means. T<[...]>
maybe?
> +
> + if (!FromParam && ToParam) {
> + FromStr += FromType.getAsString();
> + } else if (!ToParam && FromParam) {
> + ToStr += ToType.getAsString();
> + } else if (!FromParam && !ToParam) {
> + std::string FromTypeStr = FromType.getAsString();
> + std::string ToTypeStr = ToType.getAsString();
> +
> + // Switch to canoncial typename if it is better.
> + if (FromTypeStr == ToTypeStr) {
> + std::string FromCanTypeStr =
> + FromType.getCanonicalType().getAsString();
> + std::string ToCanTypeStr = ToType.getCanonicalType().getAsString();
> + if (FromCanTypeStr != ToCanTypeStr) {
> + FromTypeStr = FromCanTypeStr;
> + ToTypeStr = ToCanTypeStr;
> + }
> + }
> + FromStr += FromTypeStr;
> + ToStr += ToTypeStr;
> + }
> +}
> +
> +// Converts an expression parameter into a string.
> +static std::string ExprParamToString(Sema &S, Expr *E) {
This should be 'argument' not 'parameter'.
> + if (E->getType()->isIntegerType()) {
> + return E->EvaluateAsInt(S.Context).toString(10);
> + }
Redundant braces. This also prints chars as numbers; is that what you intended?
> +
> + if (SubstNonTypeTemplateParmExpr *SNTTPE =
> + dyn_cast<SubstNonTypeTemplateParmExpr>(E))
> + E = SNTTPE->getReplacement();
> +
> + if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E))
> + return DRE->getDecl()->getNameAsString();
> +
> + if (UnaryOperator *UO = dyn_cast<UnaryOperator>(E)) {
> + assert(UO->getOpcode() == UO_AddrOf &&
> + "Invalid unary operator for template parameter.");
> + if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(UO->getSubExpr()))
> + return "&" + DRE->getDecl()->getNameAsString();
> + }
> +
> + assert(0 && "Unable to print template parameter.");
This assert will fire once C++11 constexpr lands, and it's not clear to me why
the above list should be exhaustive in C++03 (in particular, you don't seem to
handle parens or casts). Is there some way that you could reuse
TemplateArgument::print for this (possibly by converting the default arguments
into TemplateArguments)?
> +}
> +
> +// Returns the string representation of the TemplateArgument.
> +static std::string TemplateArgumentToString(Sema &S, TemplateArgument TA) {
Could this just use TemplateArgument::print?
> + switch (TA.getKind()) {
> + case TemplateArgument::Null:
> + assert(0 && "Unable to convert template argument to string.");
> + case TemplateArgument::Type:
> + return TA.getAsType().getAsString();
> + case TemplateArgument::Declaration:
> + assert(0 && "Unable to convert template argument to string.");
> + case TemplateArgument::Integral:
> + assert(0 && "Unable to convert template argument to string.");
These asserts worry me. Is there some reason why we won't see these kinds of
template argument here, or is this WIP?
> + case TemplateArgument::Template:
> + return "template " +
> + TA.getAsTemplate().getAsTemplateDecl()->getNameAsString();
Since we're creating a diagnostic which looks like C++, we should probably try
to avoid superficially-legal-but-actually-illegal constructs like 'template
T'. The "template " prefix here doesn't seem to add much; can we drop it?
> + case TemplateArgument::TemplateExpansion:
> + assert(0 && "Unable to convert template argument to string.");
> + case TemplateArgument::Expression:
> + return ExprParamToString(S, TA.getAsExpr());
> + case TemplateArgument::Pack:
> + assert(0 && "Unable to convert template argument to string.");
> + }
> + return "";
> +}
> +
> +// Creates a string which shows the diffs between the two template types.
> +// The index of the differences are stored in the diffs vector, which includes
> +// differences from default arguments. If one template has more arguments
than
> +// the other, those extra parameters will also be included.
> +static void EmitDiffNote(Sema &S, SourceLocation Loc,
> + const TemplateSpecializationType *FromTST,
> + const TemplateSpecializationType *ToTST,
> + SmallVector<unsigned, 1> diffs,
> + TemplateParameterList *DefaultArgs) {
Indentation. Also, have you considered passing a SmallVectorImpl<unsigned> by
reference instead of copying the SmallVector? s/DefaultArgs/Params/?
> +
> + std::string fromStr = "'" +
> + FromTST->getTemplateName().getAsTemplateDecl()->getNameAsString() + "<";
> + std::string toStr = "'" +
> + ToTST->getTemplateName().getAsTemplateDecl()->getNameAsString() + "<";
> +
> + unsigned totalArgs = 0;
> + SmallVector<unsigned, 1>::iterator I = diffs.begin(), E = diffs.end();
> + for (TSTiterator fromIter(FromTST), toIter(ToTST);
> + !fromIter.isEnd() || !toIter.isEnd();
> + ++fromIter, ++toIter, ++totalArgs) {
> + if ((!fromIter.isEnd() && !fromIter.isBegin()) ||
> + (totalArgs != 0 && fromIter.isEnd() && I != E))
> + fromStr += ", ";
> + if ((!toIter.isEnd() && !toIter.isBegin()) ||
> + (totalArgs != 0 && toIter.isEnd() && I != E))
> + toStr += ", ";
If you rotate the increments and these checks round to the end of the loop,
you can avoid the (awkward and slightly inefficient) isBegin() calls and
totalArgs check.
> +
> + if (I == E && (fromIter.isEnd() || toIter.isEnd())) {
> + if (!fromIter.isEnd()) fromStr += TemplateArgumentToString(S,
*fromIter);
> + if (!toIter.isEnd()) toStr += TemplateArgumentToString(S, *toIter);
> + } else if (I == E || totalArgs != *I) {
> + if (!fromIter.isEnd()) fromStr += "[...]";
> + if (!toIter.isEnd()) toStr += "[...]";
> + } else {
> + NamedDecl *ParamND = DefaultArgs->getParam(
> + (totalArgs < DefaultArgs->size()) ? totalArgs
> + : DefaultArgs->size() - 1);
> + if (TemplateTypeParmDecl *DefaultTTPD =
> + dyn_cast<TemplateTypeParmDecl>(ParamND)) {
> + // Handle type parameters.
> + bool hasDefault = DefaultTTPD->hasDefaultArgument();
> + bool FromIsDefault = fromIter.isEnd() && hasDefault;
> + bool ToIsDefault = toIter.isEnd() && hasDefault;
> + QualType FromType = FromIsDefault ? DefaultTTPD->getDefaultArgument()
> + : fromIter->getAsType();
> + QualType ToType = ToIsDefault ? DefaultTTPD->getDefaultArgument()
> + : toIter->getAsType();
The hasDefault check seems either unnecessary or incorrect: this will crash if
fromIter.isEnd() && !hasDefault.
> + if (FromIsDefault) fromStr += DefaultTTPD->getNameAsString() + " = ";
> + if (ToIsDefault) toStr += DefaultTTPD->getNameAsString() + " = ";
> + AppendTypeNames(FromType, ToType, fromStr, toStr);
> + } else if (NonTypeTemplateParmDecl *DefaultNTTPD =
> + dyn_cast<NonTypeTemplateParmDecl>(ParamND)) {
> + // Handle non-type parameters.
> + bool hasDefault = DefaultNTTPD->hasDefaultArgument();
> + bool FromIsDefault = fromIter.isEnd() && hasDefault;
> + bool ToIsDefault = toIter.isEnd() && hasDefault;
> + Expr *FromExpr = FromIsDefault ?
> + DefaultNTTPD->getDefaultArgument() :
> + fromIter->getAsExpr();
> + Expr *ToExpr = ToIsDefault ?
> + DefaultNTTPD->getDefaultArgument() :
> + toIter->getAsExpr();
> + if (FromIsDefault) fromStr += DefaultNTTPD->getNameAsString() + " = ";
> + if (ToIsDefault) toStr += DefaultNTTPD->getNameAsString() + " = ";
> + fromStr += ExprParamToString(S, FromExpr);
> + toStr += ExprParamToString(S, ToExpr);
> + } else if (TemplateTemplateParmDecl *DefaultTTPD =
> + dyn_cast<TemplateTemplateParmDecl>(ParamND)) {
> + bool hasDefault = DefaultTTPD->hasDefaultArgument();
> + bool FromIsDefault = fromIter.isEnd() && hasDefault;
> + bool ToIsDefault = toIter.isEnd() && hasDefault;
> + TemplateArgument DefaultTA =
> + DefaultTTPD->getDefaultArgument().getArgument();
> + TemplateArgument FromTA = FromIsDefault ? DefaultTA :
> + *fromIter;
> + TemplateArgument ToTA = ToIsDefault ? DefaultTA :
> + *toIter;
> + if (FromIsDefault) fromStr += DefaultNTTPD->getNameAsString() + " = ";
> + if (ToIsDefault) toStr += DefaultNTTPD->getNameAsString() + " = ";
> + fromStr += TemplateArgumentToString(S, *fromIter);
> + toStr += TemplateArgumentToString(S, *toIter);
> + } else {
> + assert(0 && "Unknown template parameter type.");
> + }
> + ++I;
> + }
> +
> + }
> + fromStr += ">'";
> + toStr += ">'";
> +
> + S.Diag(Loc, diag::note_type_diff) << fromStr << toStr;
> +
> +}
> +
> +// Returns true if both types are specialized from the same template
> +// declaration. If they come from different template aliases, do a parallel
> +// ascension search to determine the highest template alias in common and set
> +// the arguments to them.
> +static bool hasSameTemplate(const TemplateSpecializationType *&FromTST,
> + const TemplateSpecializationType *&ToTST) {
> + // Check the top templates if they are the same.
> + if (FromTST->getTemplateName().getAsTemplateDecl()->getIdentifier() ==
> + ToTST->getTemplateName().getAsTemplateDecl()->getIdentifier())
> + return true;
Should this really be matching the identifiers and not the canonical decls?
> +
> + // Create vectors of template aliases.
> + SmallVector<const TemplateSpecializationType*, 1> FromTemplateList,
> + ToTemplateList;
> +
> + const TemplateSpecializationType *tempToTST = ToTST, *tempFromTST = FromTST;
> + FromTemplateList.push_back(FromTST);
> + ToTemplateList.push_back(ToTST);
> +
> + // Dump every template alias into the vectors.
> + while (tempFromTST->isTypeAlias()) {
> + tempFromTST =
> + tempFromTST->getAliasedType()->getAs<TemplateSpecializationType>();
These are alias templates, not template aliases: the aliased type need not be
a template specialization type. For instance:
template<typename T> using A = typename T::foo;
Your tests don't check for this, and this code will crash on such cases.
> + FromTemplateList.push_back(tempFromTST);
> + }
> + while (tempToTST->isTypeAlias()) {
> + tempToTST =
> + tempToTST->getAliasedType()->getAs<TemplateSpecializationType>();
> + ToTemplateList.push_back(tempToTST);
> + }
> +
> + // Check if the lowest template types are the same. If not, return.
> + if (tempFromTST->getTemplateName().getAsTemplateDecl()->getIdentifier() !=
> + tempToTST->getTemplateName().getAsTemplateDecl()->getIdentifier())
> + return false;
Indentation.
> +
> + FromTST = tempFromTST;
> + ToTST = tempToTST;
> +
> + SmallVector<const TemplateSpecializationType*, 1>::reverse_iterator
> + FromIter = FromTemplateList.rbegin(), FromEnd = FromTemplateList.rend(),
> + ToIter = ToTemplateList.rbegin(), ToEnd = ToTemplateList.rend();
> +
> + // Begin searching up the template aliases. The bottom most template
matches
> + // so move up until one pair does not match. Use the template right before
> + // that one.
That doesn't necessarily work, even if the alias templates all have just TSTs
on their RHS. Given:
template<typename T, typename U> using A = B<T>;
... we'll incorrectly claim that types A<int, char> and A<int, double> are not
the same.
> + for (;FromIter != FromEnd && ToIter != ToEnd; ++FromIter, ++ToIter) {
> + if ((*FromIter)->getTemplateName().getAsTemplateDecl()->getIdentifier() !=
> + (*ToIter)->getTemplateName().getAsTemplateDecl()->getIdentifier())
> + return true;
> + FromTST = *FromIter;
> + ToTST = *ToIter;
> + }
> +
> + return true;
> +}
> +
> +// This is a prototype system for diffing template types. This method is
> +// experimental.
> +void Sema::EmitTemplateTypeDiff(SourceLocation Loc, QualType FromTy,
> + QualType ToTy) {
> + // Check the limit for diff notes.
> + unsigned TemplateDiffLimit = getDiagnostics().getTemplateDiffLimit();
> + if (TemplateDiffLimit == 0) return;
> +
> + const TemplateSpecializationType *FromOrigTST =
> + FromTy->getAs<TemplateSpecializationType>();
> + const TemplateSpecializationType *ToOrigTST =
> + ToTy->getAs<TemplateSpecializationType>();
> +
> + // Only checking templates.
> + if (!FromOrigTST || !ToOrigTST) return;
> +
> + if (!hasSameTemplate(FromOrigTST, ToOrigTST)) {
> + // Top-level templates are different. Print them and stop.
> + TemplateName FromTN = FromOrigTST->getTemplateName();
> + TemplateName ToTN = ToOrigTST->getTemplateName();
> + std::string FromStr = "'" + FromTN.getAsTemplateDecl()->getNameAsString();
> + FromStr += "< >'";
> + std::string ToStr = "'" + ToTN.getAsTemplateDecl()->getNameAsString();
> + ToStr += "< >'";
> + Diag(Loc, diag::note_type_diff) << FromStr << ToStr;
> + return;
Same comment as before regarding "< >". It's not clear to me that a note for
this case is actually useful, though.
> + }
> +
> + typedef std::pair<const TemplateSpecializationType*,
> + const TemplateSpecializationType*>
> + templatePair;
> + SmallVector<templatePair, 1> queue;
> +
> + // Prime the queue with
Incomplete comment.
> + queue.push_back(templatePair(FromOrigTST, ToOrigTST));
> +
> + // Begin descent into diffing template tree.
> + while (queue.size() > 0) {
> + const TemplateSpecializationType *FromTST = queue.begin()->first;
> + const TemplateSpecializationType *ToTST = queue.begin()->second;
> + queue.erase(queue.begin());
> + TemplateParameterList *DefaultArgs =
> +
FromTST->getTemplateName().getAsTemplateDecl()->getTemplateParameters();
> + unsigned totalArgs = 0; // Total number of args processed
> + bool differentSizeParamList = false;
> + SmallVector<unsigned, 1> Diffs;
> + for (TSTiterator fromIter(FromTST), toIter(ToTST);
> + !fromIter.isEnd() || !toIter.isEnd(); ++fromIter, ++toIter) {
> + if (fromIter.isEnd() || toIter.isEnd()) differentSizeParamList = true;
> + NamedDecl *ParamND = DefaultArgs->getParam(
> + (totalArgs < DefaultArgs->size()) ? totalArgs
> + : DefaultArgs->size() - 1);
> + if (TemplateTypeParmDecl *DefaultTTPD =
> + dyn_cast<TemplateTypeParmDecl>(ParamND)) {
> + // Handle types.
> + if ((!fromIter.isEnd() && !toIter.isEnd()) ||
> + DefaultTTPD->hasDefaultArgument()) {
> + bool isVaridiac = DefaultTTPD->isParameterPack();
> + bool FromIsDefault = fromIter.isEnd() && !isVaridiac;
> + bool ToIsDefault = toIter.isEnd() && !isVaridiac;
> + if (!isVaridiac || (!FromIsDefault && !ToIsDefault)) {
> + QualType FromType = FromIsDefault ?
> + DefaultTTPD->getDefaultArgument() :
> + fromIter->getAsType();
> + QualType ToType = ToIsDefault ?
> + DefaultTTPD->getDefaultArgument() :
> + toIter->getAsType();
> + if (FromType.getCanonicalType() != ToType.getCanonicalType()) {
You can use Context.hasSameType(FromType, ToType) for this check.
> + const TemplateSpecializationType *FromArgTST =
> + FromType->getAs<TemplateSpecializationType>();
> + const TemplateSpecializationType *ToArgTST =
> + ToType->getAs<TemplateSpecializationType>();
> + if (FromArgTST && ToArgTST &&
> + hasSameTemplate(FromArgTST, ToArgTST)) {
> + // If the both are instantiated from the same template
> + // definition push the pair onto the queue for further
> + // checking.
> + queue.push_back(templatePair(FromArgTST, ToArgTST));
> + } else {
> + // The template arguments are different. Store them for
later.
> + Diffs.push_back(totalArgs);
> + }
I'd prefer to see one note per type comparison, rather than one note per
differing template. It's currently not always clear which type the notes are
talking about:
f1(A<B<C<D<void, void>, D<const double, A<int, bool> > >, int>, int>());
error: no matching function for call to 'f1'
f1(A<B<C<D<void, void>, D<const double, A<int, bool> > >, int>, int>());
^~
note: candidate function not viable: no known conversion from 'A<B<C<D<void,
void>, D<const double, A<int, bool> > >, int>, int>' to 'A<B<C<D<void, void>,
D<double, A<const int, bool> > >, int>, int>' for 1st argument;
void f1(A<B<C<D<void, void>, D<double, A<const int, bool> > >, int>, int>) {};
^
note: Type 'D<const double, [...]>' is different from type 'D<double, [...]>'
note: Type 'A<int, [...]>' is different from type 'A<const int, [...]>'
There's lots of Ds here. I have to scan the types in lockstep to deduce which
ones are problematic, which is the task this note was supposed to remove.
Always using (at most) a single note would also allow you to remove the
command-line limit, and would allow merging this function with EmitDiffNote.
This also gives surprising results for cases where a template has a difference
and also a differing nested template, for instance comparing A<B<int>, char>
against A<B<char>, int> gives me a note about A<[...], char> versus A<[...],
int>, and a separate note about B<int> versus B<char>.
> + }
> + }
> + }
> + } else if (NonTypeTemplateParmDecl *DefaultNTTPD =
> + dyn_cast<NonTypeTemplateParmDecl>(ParamND)) {
> + // Handle non-types.
> + if ((!fromIter.isEnd() && !toIter.isEnd()) ||
> + DefaultNTTPD->hasDefaultArgument()) {
> + bool isVaridiac = DefaultNTTPD->isParameterPack();
> + bool FromIsDefault = fromIter.isEnd() && !isVaridiac;
> + bool ToIsDefault = toIter.isEnd() && !isVaridiac;
> + if (!isVaridiac || (!FromIsDefault && !ToIsDefault)) {
> + Expr *FromExpr = FromIsDefault ?
> + DefaultNTTPD->getDefaultArgument() :
> + fromIter->getAsExpr();
> + Expr *ToExpr = ToIsDefault ?
> + DefaultNTTPD->getDefaultArgument() :
> + toIter->getAsExpr();
> + if (SubstNonTypeTemplateParmExpr *SNTTPE =
> + dyn_cast<SubstNonTypeTemplateParmExpr>(FromExpr))
> + FromExpr = SNTTPE->getReplacement();
> + if (SubstNonTypeTemplateParmExpr *SNTTPE =
> + dyn_cast<SubstNonTypeTemplateParmExpr>(ToExpr))
> + ToExpr = SNTTPE->getReplacement();
> + QualType FromType = FromExpr->getType();
> + QualType ToType = ToExpr->getType();
> + DeclRefExpr *FromDRE = dyn_cast<DeclRefExpr>(FromExpr);
> + DeclRefExpr *ToDRE = dyn_cast<DeclRefExpr>(ToExpr);
> + UnaryOperator *FromUO = dyn_cast<UnaryOperator>(FromExpr);
> + UnaryOperator *ToUO = dyn_cast<UnaryOperator>(ToExpr);
> + if (FromType->isIntegerType() && ToType->isIntegerType()) {
> + // Handle integer parameters.
> + if (FromExpr->EvaluateAsInt(Context) !=
> + ToExpr->EvaluateAsInt(Context))
> + Diffs.push_back(totalArgs);
> + } else if (FromDRE && ToDRE) {
> + // Handle
> + if (FromDRE->getDecl() != ToDRE->getDecl())
> + Diffs.push_back(totalArgs);
> + } else if (FromUO && ToUO) {
> + // Handle address of global variables.
> + if (FromUO->getOpcode() != UO_AddrOf ||
> + ToUO->getOpcode() != UO_AddrOf) {
> + Diffs.push_back(totalArgs);
> + } else {
> + DeclRefExpr *FromSubDRE =
> + dyn_cast<DeclRefExpr>(FromUO->getSubExpr());
> + DeclRefExpr *ToSubDRE =
> + dyn_cast<DeclRefExpr>(ToUO->getSubExpr());
> + if (FromSubDRE && ToSubDRE)
> + if (FromSubDRE->getDecl() != ToSubDRE->getDecl())
> + Diffs.push_back(totalArgs);
> + }
> + } else {
> + // Unable to match parameters, have them printed out.
> + Diffs.push_back(totalArgs);
> + }
> + }
> + }
> + } else if (TemplateTemplateParmDecl *DefaultTTPD =
> + dyn_cast<TemplateTemplateParmDecl>(ParamND)) {
> + // Handle template template parameters.
> + if ((!fromIter.isEnd() && !toIter.isEnd()) ||
> + DefaultNTTPD->hasDefaultArgument()) {
> + bool isVaridiac = DefaultTTPD->isParameterPack();
> + bool FromIsDefault = fromIter.isEnd() && !isVaridiac;
> + bool ToIsDefault = toIter.isEnd() && !isVaridiac;
> + if (!isVaridiac || (!FromIsDefault && !ToIsDefault)) {
> + TemplateArgument DefaultTA =
> + DefaultTTPD->getDefaultArgument().getArgument();
> + TemplateArgument FromTA = FromIsDefault ? DefaultTA :
> + *fromIter;
> + TemplateArgument ToTA = ToIsDefault ? DefaultTA :
> + *toIter;
> + if (FromTA.getAsTemplate().getAsTemplateDecl()->getIdentifier() !=
> + ToTA.getAsTemplate().getAsTemplateDecl()->getIdentifier())
Again, this seems like the wrong thing to check (templates in different
namespaces could have the same identifier).
> + Diffs.push_back(totalArgs);
> + }
> + }
> + }
If you can get the arguments into TemplateArgument form, you can use
TemplateArgument::structurallyEquals to replace all of this checking.
> + ++totalArgs;
> + }
> +
> + // Emit a diff note if there are differences, or one template has more
> + // parameters than the other.
> + if (Diffs.size() != 0 || differentSizeParamList) {
> + EmitDiffNote(*this, Loc, FromTST, ToTST, Diffs, DefaultArgs);
> + // Check to see that the number of notes emitted is below the limit.
> + if (--TemplateDiffLimit == 0) return;
> + }
> + }
> +}
> Index: lib/Sema/SemaOverload.cpp
> ===================================================================
> --- lib/Sema/SemaOverload.cpp (revision 138851)
> +++ lib/Sema/SemaOverload.cpp (working copy)
> @@ -6939,6 +6939,8 @@
> FDiag << *HI;
> S.Diag(Fn->getLocation(), FDiag);
>
> + S.EmitTemplateTypeDiff(Fn->getLocation(), FromTy, ToTy);
> +
> MaybeEmitInheritedConstructorNote(S, Fn);
> }
More information about the cfe-commits
mailing list