Hi rsmith,
Fix PR18792 - Bad diagnostic messages for std::enable_if_t uses
This patch improves the diagnostic messages for overloads disabled with
enable_if, in particular those where the failing typename enable_if<...>::type
lookup happens during the instantiation of an alias template like
std::enable_if_t.
Previously the diagnostic note for an overload disabled with std::enable_if_t
(or a similar alias template) only contained the location of the typename
enable_if<...>::type lookup and did not contain the location of the disabled
function overload, which could be quite annoying when you had many overloads.
This patch makes clang generate a note with the name and location of the
outermost alias template in the SFINAE context in which the enable_if<...>::type
lookup failed. Typically this location will be inside the source range for the
function template declaration, i.e. exactly where we want it. In the rare
circumstances where this is not the case, see e.g. the new test case with the
FIXME in overload-candidates.cpp, the implementation makes sure that the
diagnostic is placed at the disabled overload.
I'm not certain whether using the "disabled by %0" message even for template
aliases whose names don't start with "enable" or "disable" is optimal, but I'm
pretty sure that in the vast majority of cases the generated message should
be quite understandable and certainly better than the one generated when
not using the enable_if logic. Though, if someone has a good idea for the
wording, it would be easy to use a different message for such aliases.
This patch also improves the source range highlighting for regular enable_if
uses and always prints out the fully qualified name of the enable_if template
(as was requested by a FIXME in SemaOverload.cpp).
http://llvm-reviews.chandlerc.com/D3061
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Basic/SourceLocation.h
include/clang/Sema/Sema.h
lib/Sema/SemaOverload.cpp
lib/Sema/SemaTemplate.cpp
lib/Sema/SemaTemplateInstantiate.cpp
test/SemaTemplate/constexpr-instantiate.cpp
test/SemaTemplate/overload-candidates.cpp
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2658,7 +2658,7 @@
def note_ovl_candidate_substitution_failure : Note<
"candidate template ignored: substitution failure%0%1">;
def note_ovl_candidate_disabled_by_enable_if : Note<
- "candidate template ignored: disabled by %0%1">;
+ "candidate template ignored: disabled by %q0%1">;
def note_ovl_candidate_disabled_by_enable_if_attr : Note<
"candidate disabled: %0">;
def note_ovl_candidate_failed_overload_resolution : Note<
Index: include/clang/Basic/SourceLocation.h
===================================================================
--- include/clang/Basic/SourceLocation.h
+++ include/clang/Basic/SourceLocation.h
@@ -213,6 +213,12 @@
bool operator!=(const SourceRange &X) const {
return B != X.B || E != X.E;
}
+
+ /// \brief Indicates whether
+ /// `getBegin() <= other.getBegin() && other.getEnd() <= getEnd()`.
+ bool contains(SourceRange other) const {
+ return !(other.B < B || E < other.E);
+ }
};
/// \brief Represents a character-granular source range.
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -6051,6 +6051,9 @@
/// \brief The source range that covers the construct that cause
/// the instantiation, e.g., the template-id that causes a class
/// template instantiation.
+ ///
+ /// For alias template instantiations this member is used to store the
+ /// source range of the first template argument.
SourceRange InstantiationRange;
ActiveTemplateInstantiation()
Index: lib/Sema/SemaOverload.cpp
===================================================================
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -8878,21 +8878,56 @@
case Sema::TDK_SubstitutionFailure: {
// Format the template argument list into the argument string.
SmallString<128> TemplateArgString;
+ TemplateDecl *TemplDecl = getDescribedTemplate(Templated);
if (TemplateArgumentList *Args =
DeductionFailure.getTemplateArgumentList()) {
TemplateArgString = " ";
TemplateArgString += S.getTemplateArgumentBindingsText(
- getDescribedTemplate(Templated)->getTemplateParameters(), *Args);
+ TemplDecl->getTemplateParameters(), *Args);
}
// If this candidate was disabled by enable_if, say so.
PartialDiagnosticAt *PDiag = DeductionFailure.getSFINAEDiagnostic();
if (PDiag && PDiag->second.getDiagID() ==
diag::err_typename_nested_not_found_enable_if) {
- // FIXME: Use the source range of the condition, and the fully-qualified
- // name of the enable_if template. These are both present in PDiag.
- S.Diag(PDiag->first, diag::note_ovl_candidate_disabled_by_enable_if)
- << "'enable_if'" << TemplateArgString;
+ PartialDiagnostic &PD = PDiag->second;
+ ArrayRef<CharSourceRange> PDRanges = PD.getRanges();
+ // We expect the PartialDiagnostic in the form it is generated by the
+ // isEnableIf case in Sema::CheckTypenameType.
+ // The first argument should always be the DeclContext of the enable_if
+ // specialization for which the 'type' lookup failed.
+ // The first range should always be the source range for the first
+ // argument in the `typename enable_if<arg1, ...>::type` expression.
+ // If the failed enable_if lookup occurred within an alias template
+ // instantiation, the PartialDiagnostic should additionally contain a
+ // pointer to the TypeAliasTemplateDecl and a source range that marks
+ // a relevant part of the alias template instantiation.
+ assert(PD.getNumArgs() >= 1 && PDRanges.size() >= 1 &&
+ PD.getArgKind(0) == DiagnosticsEngine::ak_declcontext &&
+ (PD.getNumArgs() == 1 ||
+ (PD.getArgKind(1) == DiagnosticsEngine::ak_nameddecl &&
+ PDRanges.size() >= 2)) &&
+ "an err_typename_nested_not_found_enable_if diagnostic does"
+ " not contain the expected arguments and source ranges");
+ const bool IsAlias = PDRanges.size() > 1;
+ const NamedDecl *EnableIfOrAliasDecl =
+ IsAlias ? reinterpret_cast<NamedDecl *>(PD.getRawArg(1))
+ : cast<ClassTemplateSpecializationDecl>(
+ reinterpret_cast<DeclContext *>(PD.getRawArg(0)))
+ ->getSpecializedTemplate();
+ SourceRange Range = PDRanges[IsAlias ? 1 : 0].getAsRange();
+ // If an alias template that contains an enable_if type lookup is
+ // instantiated with a dependent type, the failing type lookup
+ // may occur when the alias template is no longer on the instantiation
+ // stack and we may end up with a source range that is far away from
+ // the disabled overload. To make sure that the diagnostic always
+ // contains the location of the disabled overload, we check whether the
+ // range is contained in the source range of the template declaration.
+ S.Diag(TemplDecl->getSourceRange().contains(Range)
+ ? Range.getBegin()
+ : Templated->getLocation(),
+ diag::note_ovl_candidate_disabled_by_enable_if)
+ << EnableIfOrAliasDecl << TemplateArgString << Range;
return;
}
Index: lib/Sema/SemaTemplate.cpp
===================================================================
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -2005,6 +2005,12 @@
if (Pattern->isInvalidDecl())
return QualType();
+ // We use this range for improving the diagnostic messages for enable_if
+ // SFINAE errors.
+ SourceRange Arg0Range = TemplateArgs.size()
+ ? TemplateArgs[0].getSourceRange()
+ : SourceRange(TemplateLoc);
+
TemplateArgumentList TemplateArgs(TemplateArgumentList::OnStack,
Converted.data(), Converted.size());
@@ -2016,7 +2022,7 @@
TemplateArgLists.addOuterTemplateArguments(None);
LocalInstantiationScope Scope(*this);
- InstantiatingTemplate Inst(*this, TemplateLoc, Template);
+ InstantiatingTemplate Inst(*this, TemplateLoc, AliasTemplate, Arg0Range);
if (Inst.isInvalid())
return QualType();
@@ -7789,15 +7795,51 @@
// ... called "enable_if".
const IdentifierInfo *EnableIfII =
- EnableIfDecl->getDeclName().getAsIdentifierInfo();
+ EnableIfDecl->getDeclName().getAsIdentifierInfo();
if (!EnableIfII || !EnableIfII->isStr("enable_if"))
return false;
// Assume the first template argument is the condition.
CondRange = EnableIfTSTLoc.getArgLoc(0).getSourceRange();
return true;
}
+/// \brief Returns a pointer to the ActiveTemplateInstantiation record for the
+/// outermost type alias instantiation in the current SFINAE context, or null
+/// if there is no such instantiation.
+static const Sema::ActiveTemplateInstantiation *
+findOutermostAliasTemplateInstantiationInSFINAEContext(const Sema &S) {
+ if (S.InNonInstantiationSFINAEContext)
+ return nullptr;
+ const Sema::ActiveTemplateInstantiation *ATI = nullptr;
+ // See the comments in Sema::isSFINAEContext.
+ for (auto A = S.ActiveTemplateInstantiations.rbegin(),
+ E = S.ActiveTemplateInstantiations.rend();
+ A != E; ++A) {
+ switch (A->Kind) {
+ case Sema::ActiveTemplateInstantiation::TemplateInstantiation:
+ if (isa<TypeAliasTemplateDecl>(A->Entity)) {
+ ATI = &*A;
+ continue;
+ }
+ // Fall through.
+ case Sema::ActiveTemplateInstantiation::DefaultFunctionArgumentInstantiation:
+ case Sema::ActiveTemplateInstantiation::ExceptionSpecInstantiation:
+ return nullptr; // No SFINAE.
+
+ case Sema::ActiveTemplateInstantiation::DefaultTemplateArgumentInstantiation:
+ case Sema::ActiveTemplateInstantiation::PriorTemplateArgumentSubstitution:
+ case Sema::ActiveTemplateInstantiation::DefaultTemplateArgumentChecking:
+ continue; // Look further up the stack.
+
+ case Sema::ActiveTemplateInstantiation::ExplicitTemplateArgumentSubstitution:
+ case Sema::ActiveTemplateInstantiation::DeducedTemplateArgumentSubstitution:
+ return ATI;
+ }
+ }
+ return nullptr;
+}
+
/// \brief Build the type that describes a C++ typename specifier,
/// e.g., "typename T::type".
QualType
@@ -7839,8 +7881,42 @@
// a more specific diagnostic.
SourceRange CondRange;
if (isEnableIf(QualifierLoc, II, CondRange)) {
- Diag(CondRange.getBegin(), diag::err_typename_nested_not_found_enable_if)
- << Ctx << CondRange;
+ auto D = Diag(CondRange.getBegin(),
+ diag::err_typename_nested_not_found_enable_if)
+ << Ctx << CondRange;
+ if (const Sema::ActiveTemplateInstantiation *ATI =
+ findOutermostAliasTemplateInstantiationInSFINAEContext(*this)) {
+ // For a failed enable_if lookup within an alias template instantiation
+ // we also store the source range of the instantiation and a pointer to
+ // the TypeAliasTemplateDecl in the PartialDiagnostic.
+ // This allows us is Sema::DiagnoseBadDeduction to highlight a source
+ // range that is actually *within* the declaration of the disabled
+ // overload (and not in the declaration of the alias template).
+ // We don't restrict this handling to alias templates named enable_if_t
+ // because if a failed 'typename enable_if<...>::type' lookup occurs
+ // within an alias template instantiation in a SFINAE context, it is
+ // highly likely that the alias is used similarly to std::enable_if_t.
+ auto *AliasDecl = cast<TypeAliasTemplateDecl>(ATI->Entity);
+ // Sema::CheckTemplateIdType stores the source range for the first
+ // template argument in ATI->InstantiationRange.
+ SourceRange Arg0Range = ATI->InstantiationRange;
+ auto *EnableIfII = AliasDecl->getDeclName().getAsIdentifierInfo();
+ TemplateParameterList *TPL = AliasDecl->getTemplateParameters();
+ if (Arg0Range.isInvalid() || TPL->size() == 0)
+ CondRange = ATI->PointOfInstantiation;
+ else if (TPL->size() == 1 || EnableIfII->isStr("enable_if_t"))
+ CondRange = Arg0Range;
+ else {
+ // If the alias template has more than one parameter and is not
+ // called enable_if_t, we can't be sure that the first argument is
+ // actually the one that caused the error. So instead of underlining
+ // the full argument, we only point to the beginning of it, which
+ // should avoid misunderstandings while still pointing (close) to the
+ // origin of the error.
+ CondRange = Arg0Range.getBegin();
+ }
+ D << AliasDecl << CondRange;
+ }
return QualType();
}
Index: lib/Sema/SemaTemplateInstantiate.cpp
===================================================================
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -451,8 +451,9 @@
} else {
Diags.Report(Active->PointOfInstantiation,
diag::note_template_type_alias_instantiation_here)
- << cast<TypeAliasTemplateDecl>(D)
- << Active->InstantiationRange;
+ << cast<TypeAliasTemplateDecl>(D);
+ // For an alias template Active->InstantiationRange is used
+ // to store the source range of the first template argument.
}
break;
}
Index: test/SemaTemplate/constexpr-instantiate.cpp
===================================================================
--- test/SemaTemplate/constexpr-instantiate.cpp
+++ test/SemaTemplate/constexpr-instantiate.cpp
@@ -173,7 +173,8 @@
static constexpr bool f() { return sizeof(T) < U::size; }
template<typename U>
- static typename enable_if<f<U>(), void>::type g() {} // expected-note {{disabled by 'enable_if'}}
+ static typename enable_if<f<U>(), void>::type g() {}
+ // expected-note@-1 {{disabled by 'Unevaluated::PR13423::enable_if'}}
};
struct U { static constexpr int size = 2; };
Index: test/SemaTemplate/overload-candidates.cpp
===================================================================
--- test/SemaTemplate/overload-candidates.cpp
+++ test/SemaTemplate/overload-candidates.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
template<typename T>
const T& min(const T&, const T&); // expected-note{{candidate template ignored: deduced conflicting types for parameter 'T' ('int' vs. 'long')}}
@@ -45,15 +45,15 @@
template<bool, typename = void> struct enable_if {};
template<typename T> struct enable_if<true, T> { typedef T type; };
}
-template<typename T> typename boost::enable_if<sizeof(T) == 4, int>::type if_size_4(); // expected-note{{candidate template ignored: disabled by 'enable_if' [with T = char]}}
+template<typename T> typename boost::enable_if<sizeof(T) == 4, int>::type if_size_4(); // expected-note{{candidate template ignored: disabled by 'boost::enable_if' [with T = char]}}
int k = if_size_4<char>(); // expected-error{{no matching function}}
namespace llvm {
template<typename Cond, typename T = void> struct enable_if : boost::enable_if<Cond::value, T> {};
}
template<typename T> struct is_int { enum { value = false }; };
template<> struct is_int<int> { enum { value = true }; };
-template<typename T> typename llvm::enable_if<is_int<T> >::type if_int(); // expected-note{{candidate template ignored: disabled by 'enable_if' [with T = char]}}
+template<typename T> typename llvm::enable_if<is_int<T> >::type if_int(); // expected-note{{candidate template ignored: disabled by 'llvm::enable_if' [with T = char]}}
void test_if_int() {
if_int<char>(); // expected-error{{no matching function}}
}
@@ -84,6 +84,9 @@
template<bool, typename = void> struct enable_if {};
template<typename T> struct enable_if<true, T> { typedef T type; };
+ template <bool b, typename T = void>
+ using enable_if_t = typename enable_if<b, T>::type;
+
template<typename T, T V> struct integral_constant { static const T value = V; };
typedef integral_constant<bool, false> false_type;
typedef integral_constant<bool, true> true_type;
@@ -94,8 +97,8 @@
struct a_trait : std::false_type {};
template<typename T,
- typename Requires = typename std::enable_if<a_trait<T>::value>::type> // expected-warning {{C++11 extension}}
- // expected-note@-1 {{candidate template ignored: disabled by 'enable_if' [with T = int]}}
+ typename Requires = typename std::enable_if<a_trait<T>::value>::type>
+ // expected-note@-1 {{candidate template ignored: disabled by 'std::enable_if' [with T = int]}}
void foo() {}
void bar() { foo<int>(); } // expected-error {{no matching function for call to 'foo'}}
@@ -108,19 +111,67 @@
struct a_pony : std::enable_if<some_trait<T>::value> {};
template<typename T,
- typename Requires = typename a_pony<T>::type> // expected-warning {{C++11 extension}}
+ typename Requires = typename a_pony<T>::type>
// FIXME: The source location here is poor.
void baz() { } // expected-note {{candidate template ignored: substitution failure [with T = int]: no type named 'type' in 'PR15673::a_pony<int>'}}
void quux() { baz<int>(); } // expected-error {{no matching function for call to 'baz'}}
- // FIXME: This note doesn't make it clear which candidate we rejected.
template <typename T>
- using unicorns = typename std::enable_if<some_trait<T>::value>::type; // expected-warning {{C++11 extension}}
- // expected-note@-1 {{candidate template ignored: disabled by 'enable_if' [with T = int]}}
+ using unicorns = typename std::enable_if<some_trait<T>::value>::type;
template<typename T,
- typename Requires = unicorns<T> > // expected-warning {{C++11 extension}}
+ typename Requires = unicorns<T> > // expected-note {{candidate template ignored: disabled by 'PR15673::unicorns' [with T = int]}}
void wibble() {}
void wobble() { wibble<int>(); } // expected-error {{no matching function for call to 'wibble'}}
}
+
+template <typename T,
+ std::enable_if_t<sizeof(T) == 1, int> = 0> // expected-note {{candidate template ignored: disabled by 'std::enable_if_t' [with T = int]}}
+void test1() {}
+
+template <typename T>
+auto test2()
+-> std::enable_if_t<sizeof(T) == 1> // expected-note {{candidate template ignored: disabled by 'std::enable_if_t' [with T = int]}}
+{}
+
+template <bool b>
+using enable_if_t2 = std::enable_if_t<b>;
+
+template <typename T,
+ typename = enable_if_t2<sizeof(T) == 1>> // expected-note {{candidate template ignored: disabled by 'enable_if_t2' [with T = int]}}
+void test3() {}
+
+template <bool b, typename T = enable_if_t2<b>>
+using enable_if_t3 = T;
+
+template <typename T,
+ typename = enable_if_t3<sizeof(T) == 1>>
+ // FIXME: This note should mention enable_if_t3.
+void test4() {} // expected-note {{candidate template ignored: disabled by 'enable_if_t2' [with T = int]}}
+
+template <bool b>
+using AnotherEnableIf =
+ typename std::enable_if<b, int>::type;
+// expected-error@-1 {{no type named 'type' in 'std::enable_if<false, int>'; 'enable_if' cannot be used to disable this declaration}}
+
+template <bool b>
+struct BrokenEnableIf {
+ typedef AnotherEnableIf<b> type;
+// expected-note@-1 {{in instantiation of template type alias 'AnotherEnableIf' requested here}}
+};
+
+template <typename T,
+ typename BrokenEnableIf<sizeof(T) == 1>::type = 0>
+ // expected-note@-1 {{in instantiation of template class 'BrokenEnableIf<false>'}}
+void test5() {} // expected-note {{candidate template ignored: substitution failure [with T = int]}}
+// expected-note@-1 {{while substituting prior template arguments into non-type template parameter [with T = int]}}
+
+void tests() {
+ test1<int>(); // expected-error {{no matching function for call to 'test1'}}
+ test2<int>(); // expected-error {{no matching function for call to 'test2'}}
+ test3<int>(); // expected-error {{no matching function for call to 'test3'}}
+ test4<int>(); // expected-error {{no matching function for call to 'test4'}}
+ test5<int>(); // expected-error {{no matching function for call to 'test5'}}
+// expected-note@-1 {{while substituting deduced template arguments into function template 'test5'}}
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
