sammccall added inline comments.
================
Comment at: clang/lib/AST/TemplateName.cpp:174
+TemplateNameDependence TemplateName::getDependence() const {
+ auto Dependency = TemplateNameDependence::None;
+ if (QualifiedTemplateName *QTN = getAsQualifiedTemplateName())
----------------
nit: please don't use "Dependency" to mean "Dependence" :-)
I find this easier to read with a short name here (like `D` or the previous
`F`, but up to you.
================
Comment at: clang/lib/AST/TemplateName.cpp:175
+ auto Dependency = TemplateNameDependence::None;
+ if (QualifiedTemplateName *QTN = getAsQualifiedTemplateName())
+ Dependency |=
----------------
I think it would add some useful structure to put everything except the
getAsTemplateDecl() in a `switch (getKind())` rather than ifs scattered around.
This would make it clearer which cases are disjoint.
================
Comment at: clang/lib/AST/TemplateName.cpp:193
+ } else {
+ assert(!getAsOverloadedTemplate() &&
+ "overloaded templates shouldn't survive to here");
----------------
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.
================
Comment at: clang/lib/AST/TemplateName.cpp:201
+ DTN->getQualifier()->containsUnexpandedParameterPack())
+ Dependency |= TemplateNameDependence::UnexpandedPack;
+ if (getAsSubstTemplateTemplateParmPack())
----------------
Why are we querying/propagating individual bits here rather than converting and
adding DTN->getQualifier()->getDependence()? Are we deliberately dropping some
of the bits?
(Checking individual bits makes adding new bits e.g. errors harder, as they
won't be propagated by default)
================
Comment at: clang/lib/AST/TemplateName.cpp:205
+ // Dependent template name is always instantiation dependent.
+ if (Dependency & TemplateNameDependence::Dependent)
+ Dependency |= TemplateNameDependence::DependentInstantiation;
----------------
why not just add DependentInstantiation above in the first place?
Tracking incorrect bits and then fixing them at the end seems a bit confusing.
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
