rsmith accepted this revision. rsmith added a comment. Some cleanups (mainly parameters that are no longer necessary), then this is good to go. Thanks!
================ Comment at: clang/include/clang/Basic/LangOptions.def:146 LANGOPT(DllExportInlines , 1, 1, "dllexported classes dllexport inline methods") -LANGOPT(RelaxedTemplateTemplateArgs, 1, 0, "C++17 relaxed matching of template template arguments") +LANGOPT(RelaxedTemplateTemplateArgs, 1, 1, "C++17 relaxed matching of template template arguments") ---------------- This change has no effect because the default value is unconditionally overwritten in `CompilerInvocation.cpp`. Revert for now; we can flip this flag as a separate change. ================ Comment at: clang/lib/AST/DeclTemplate.cpp:110-114 + if (const auto *TTP = dyn_cast<TemplateTypeParmDecl>(P)) + if (TTP->isExpandedParameterPack()) { + NumRequiredArgs += TTP->getNumExpansionParameters(); + continue; + } ---------------- Do we need something like this for expanded template template parameter packs? ================ Comment at: clang/lib/Parse/ParseExprCXX.cpp:2713 SourceLocation *TemplateKWLoc, - UnqualifiedId &Result) { + UnqualifiedId &Result, bool SuppressDiag) { if (TemplateKWLoc) ---------------- This `SuppressDiag` parameter is never set by any caller; please remove it. ================ Comment at: clang/lib/Parse/ParseTemplate.cpp:684 + // user intended and parse a non-type parameter with an error type. + /*ErrorInTypeSpec=*/ScopeError); +} ---------------- `ScopeError` is always `false` here; you can remove the `ErrorInTypeSpec` parameter and the comment here. ================ Comment at: clang/lib/Parse/ParseTemplate.cpp:707 +bool Parser::TryAnnotateTypeConstraint(CXXScopeSpec &SS) { + if (!getLangOpts().ConceptsTS || Tok.is(tok::annot_template_id) || + Tok.isNot(tok::identifier)) ---------------- The `Tok.is` check here is redundant; it's a subset of the following `Tok.isNot` check. ================ Comment at: clang/lib/Sema/SemaTemplate.cpp:136 + bool &MemberOfUnknownSpecialization, + NamedDecl **FoundDecl) { assert(getLangOpts().CPlusPlus && "No template names in C!"); ---------------- This new parameter appears to be unused now. ================ Comment at: clang/lib/Sema/SemaTemplate.cpp:7088 + // concepts. + if (getLangOpts().RelaxedTemplateTemplateArgs || getLangOpts().ConceptsTS) { // Quick check for the common case: ---------------- This is not appropriate; `-fconcepts-ts` (or hopefully soon `-std=c++2a`) should not override the value of this `LangOpt`. Please revert this change and manually turn on `-frelaxed-template-template-args` as necessary in the corresponding test cases. ================ Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:645-659 static Optional<unsigned> getExpandedPackSize(NamedDecl *Param) { + if (auto *TTP = dyn_cast<TemplateTypeParmDecl>(Param)) + if (TTP->isExpandedParameterPack()) + return TTP->getNumExpansionParameters(); + if (auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(Param)) if (NTTP->isExpandedParameterPack()) ---------------- Looks like we repeat this (at least) three times. (And one of the instances is missing the template template parameter case.) Please consider moving this to somewhere more central (`DeclTemplate.h`?) -- as a separate patch, though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D44352/new/ https://reviews.llvm.org/D44352 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits