hokein added inline comments.
================ Comment at: clang/lib/AST/TemplateName.cpp:193 + } else { + assert(!getAsOverloadedTemplate() && + "overloaded templates shouldn't survive to here"); ---------------- sammccall wrote: > As far as I can tell, an overloaded template will always return null for > getAsTemplateDecl(). So this assert can be hoisted to the top, which I think > would be clearer. I didn't get your point here, did you mean moving the assert to the `switch (getKind())` like ``` switch (getKind()) { case OverloadedTemplate: assert(!GetAsTemplateDecl()); } ``` ? ================ Comment at: clang/lib/AST/TemplateName.cpp:205 + // Dependent template name is always instantiation dependent. + if (Dependency & TemplateNameDependence::Dependent) + Dependency |= TemplateNameDependence::DependentInstantiation; ---------------- sammccall wrote: > why not just add DependentInstantiation above in the first place? > Tracking incorrect bits and then fixing them at the end seems a bit confusing. oops, I was confused by the `DependentInstantiation` and `Dependent` bits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71920/new/ https://reviews.llvm.org/D71920 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits