erichkeane updated this revision to Diff 280101.
erichkeane marked 9 inline comments as done.
erichkeane added a comment.
Fix all the things that @rsmith suggested.
Thanks for the feedback, it is looking much better!
I ended up using a MapVector instead of std::map for the
RecordType*->SmallVector map, since we iterate over it. While the iteration
SHOULDN'T matter (it is just the order that bases are added), if there is any
bug/unconsidered situation, we might diagnose in a non-deterministic manner.
Let me know if I'm being overly concerned there and I can switch it to std::map
(DenseMap ends up being a bad choice according to the docs).
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84048/new/
https://reviews.llvm.org/D84048
Files:
clang/lib/Sema/SemaTemplateDeduction.cpp
clang/test/CXX/drs/dr23xx.cpp
clang/www/cxx_dr_status.html
Index: clang/www/cxx_dr_status.html
===================================================================
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -13633,7 +13633,7 @@
<td><a href="https://wg21.link/cwg2303";>2303</a></td>
<td>DRWP</td>
<td>Partial ordering and recursive variadic inheritance</td>
- <td class="none" align="center">Unknown</td>
+ <td class="unreleased" align="center">Clang 12</td>
</tr>
<tr id="2304">
<td><a href="https://wg21.link/cwg2304";>2304</a></td>
Index: clang/test/CXX/drs/dr23xx.cpp
===================================================================
--- clang/test/CXX/drs/dr23xx.cpp
+++ clang/test/CXX/drs/dr23xx.cpp
@@ -113,3 +113,35 @@
extern template const int d<const int>;
#endif
}
+
+#if __cplusplus >= 201103L
+namespace dr2303 { // dr2303: 12
+template <typename... T>
+struct A;
+template <>
+struct A<> {};
+template <typename T, typename... Ts>
+struct A<T, Ts...> : A<Ts...> {};
+struct B : A<int, int> {};
+struct C : A<int, int>, A<int> {}; // expected-warning {{direct base 'A<int>' is inaccessible}}
+struct D : A<int>, A<int, int> {}; // expected-warning {{direct base 'A<int>' is inaccessible}}
+struct E : A<int, int> {};
+struct F : B, E {};
+
+template <typename... T>
+void f(const A<T...> &) {
+ static_assert(sizeof...(T) == 2, "Should only match A<int,int>");
+}
+template <typename... T>
+void f2(const A<T...> *);
+
+void g() {
+ f(B{}); // This is no longer ambiguous.
+ B b;
+ f2(&b);
+ f(C{});
+ f(D{});
+ f(F{}); // expected-error {{ambiguous conversion from derived class}}
+}
+} //namespace dr2303
+#endif
Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -1201,6 +1201,120 @@
return false;
}
+/// Attempt to deduce the template arguments by checking the base types
+/// according to (C++ [temp.deduct.call] p4b3.
+///
+/// \param S the semantic analysis object within which we are deducing
+///
+/// \param RecordT the top level record object we are deducing against.
+///
+/// \param TemplateParams the template parameters that we are deducing
+///
+/// \param SpecParam the template specialization parameter type.
+///
+/// \param Info information about the template argument deduction itself
+///
+/// \param Deduced the deduced template arguments
+///
+/// \returns the result of template argument deduction with the bases. "invalid"
+/// means no matches, "success" found a single item, and the
+/// "MiscellaneousDeductionFailure" result happens when the match is ambiguous.
+static Sema::TemplateDeductionResult DeduceTemplateBases(
+ Sema &S, const RecordType *RecordT, TemplateParameterList *TemplateParams,
+ const TemplateSpecializationType *SpecParam, TemplateDeductionInfo &Info,
+ SmallVectorImpl<DeducedTemplateArgument> &Deduced) {
+ // C++14 [temp.deduct.call] p4b3:
+ // If P is a class and P has the form simple-template-id, then the
+ // transformed A can be a derived class of the deduced A. Likewise if
+ // P is a pointer to a class of the form simple-template-id, the
+ // transformed A can be a pointer to a derived class pointed to by the
+ // deduced A. However, if there is a class C that is a (direct or
+ // indirect) base class of D and derived (directly or indirectly) from a
+ // class B and that would be a valid deduced A, the deduced A cannot be
+ // B or pointer to B, respectively.
+ //
+ // These alternatives are considered only if type deduction would
+ // otherwise fail. If they yield more than one possible deduced A, the
+ // type deduction fails.
+
+ // Use a breadth-first search through the bases to collect the set of
+ // successful matches. Visited contains the set of nodes we have already
+ // visited, while ToVisit is our stack of records that we still need to
+ // visit. Matches contains a list of matches that have yet to be
+ // disqualified.
+ llvm::SmallPtrSet<const RecordType *, 8> Visited;
+ SmallVector<const RecordType *, 8> ToVisit;
+ // We iterate over this later, so we have to use MapVector to ensure
+ // determinism.
+ llvm::MapVector<const RecordType *, SmallVector<DeducedTemplateArgument, 8>>
+ Matches;
+
+ auto AddBases = [&Visited, &ToVisit](const RecordType *RT) {
+ CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
+ for (const auto &Base : RD->bases()) {
+ assert(Base.getType()->isRecordType() &&
+ "Base class that isn't a record?");
+ const RecordType *RT = Base.getType()->getAs<RecordType>();
+ if (Visited.insert(RT).second)
+ ToVisit.push_back(Base.getType()->getAs<RecordType>());
+ }
+ };
+
+ // Set up the loop by adding all the bases.
+ AddBases(RecordT);
+
+ // Search each path of bases until we either run into a successful match
+ // (where all bases of it are invalid), or we run out of bases.
+ while (!ToVisit.empty()) {
+ const RecordType *NextT = ToVisit.pop_back_val();
+
+ SmallVector<DeducedTemplateArgument, 8> DeducedCopy(Deduced.begin(),
+ Deduced.end());
+ TemplateDeductionInfo BaseInfo(TemplateDeductionInfo::ForBase, Info);
+ Sema::TemplateDeductionResult BaseResult =
+ DeduceTemplateArguments(S, TemplateParams, SpecParam,
+ QualType(NextT, 0), BaseInfo, DeducedCopy);
+
+ // If this was a successful deduction, add it to the list of matches,
+ // otherwise we need to continue searching its bases.
+ if (BaseResult == Sema::TDK_Success)
+ Matches.insert({NextT, DeducedCopy});
+ else
+ AddBases(NextT);
+ }
+
+ // At this point, 'Matches' contains a list of seemingly valid bases, however
+ // in the event that we have more than 1 match, it is possible that the base
+ // of one of the matches might be disqualified for being a base of another
+ // valid match. We can count on cyclical instantiations being invalid to
+ // simplify the disqualifications. That is, if A & B are both matches, and B
+ // inherits from A (disqualifying A), we know that A cannot inherit from B.
+ if (Matches.size() > 1) {
+ Visited.clear();
+ for (const auto &Match : Matches)
+ AddBases(Match.first);
+
+ // We can give up once we have a single item (or have run out of things to
+ // search) since cyclical inheritence isn't valid.
+ while (Matches.size() > 1 && !ToVisit.empty()) {
+ const RecordType *NextT = ToVisit.pop_back_val();
+ Matches.erase(NextT);
+
+ // Always add all bases, since the inheritence tree can contain
+ // disqualifications for multiple matches.
+ AddBases(NextT);
+ }
+ }
+
+ if (Matches.empty())
+ return Sema::TDK_Invalid;
+ if (Matches.size() > 1)
+ return Sema::TDK_MiscellaneousDeductionFailure;
+
+ std::swap(Matches.front().second, Deduced);
+ return Sema::TDK_Success;
+}
+
/// Deduce the template arguments by comparing the parameter type and
/// the argument type (C++ [temp.deduct.type]).
///
@@ -1787,78 +1901,15 @@
if (!S.isCompleteType(Info.getLocation(), Arg))
return Result;
- // C++14 [temp.deduct.call] p4b3:
- // If P is a class and P has the form simple-template-id, then the
- // transformed A can be a derived class of the deduced A. Likewise if
- // P is a pointer to a class of the form simple-template-id, the
- // transformed A can be a pointer to a derived class pointed to by the
- // deduced A.
- //
- // These alternatives are considered only if type deduction would
- // otherwise fail. If they yield more than one possible deduced A, the
- // type deduction fails.
-
// Reset the incorrectly deduced argument from above.
Deduced = DeducedOrig;
- // Use data recursion to crawl through the list of base classes.
- // Visited contains the set of nodes we have already visited, while
- // ToVisit is our stack of records that we still need to visit.
- llvm::SmallPtrSet<const RecordType *, 8> Visited;
- SmallVector<const RecordType *, 8> ToVisit;
- ToVisit.push_back(RecordT);
- bool Successful = false;
- SmallVector<DeducedTemplateArgument, 8> SuccessfulDeduced;
- while (!ToVisit.empty()) {
- // Retrieve the next class in the inheritance hierarchy.
- const RecordType *NextT = ToVisit.pop_back_val();
-
- // If we have already seen this type, skip it.
- if (!Visited.insert(NextT).second)
- continue;
-
- // If this is a base class, try to perform template argument
- // deduction from it.
- if (NextT != RecordT) {
- TemplateDeductionInfo BaseInfo(TemplateDeductionInfo::ForBase, Info);
- Sema::TemplateDeductionResult BaseResult =
- DeduceTemplateArguments(S, TemplateParams, SpecParam,
- QualType(NextT, 0), BaseInfo, Deduced);
-
- // If template argument deduction for this base was successful,
- // note that we had some success. Otherwise, ignore any deductions
- // from this base class.
- if (BaseResult == Sema::TDK_Success) {
- // If we've already seen some success, then deduction fails due to
- // an ambiguity (temp.deduct.call p5).
- if (Successful)
- return Sema::TDK_MiscellaneousDeductionFailure;
-
- Successful = true;
- std::swap(SuccessfulDeduced, Deduced);
-
- Info.Param = BaseInfo.Param;
- Info.FirstArg = BaseInfo.FirstArg;
- Info.SecondArg = BaseInfo.SecondArg;
- }
-
- Deduced = DeducedOrig;
- }
-
- // Visit base classes
- CXXRecordDecl *Next = cast<CXXRecordDecl>(NextT->getDecl());
- for (const auto &Base : Next->bases()) {
- assert(Base.getType()->isRecordType() &&
- "Base class that isn't a record?");
- ToVisit.push_back(Base.getType()->getAs<RecordType>());
- }
- }
-
- if (Successful) {
- std::swap(SuccessfulDeduced, Deduced);
- return Sema::TDK_Success;
- }
+ // Check bases according to C++14 [temp.deduct.call] p4b3:
+ Sema::TemplateDeductionResult BaseResult = DeduceTemplateBases(
+ S, RecordT, TemplateParams, SpecParam, Info, Deduced);
+ if (BaseResult != Sema::TDK_Invalid)
+ return BaseResult;
return Result;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
