rsmith added inline comments.
================
Comment at: clang/lib/Sema/SemaTemplate.cpp:10111-10124
+ // Check the static member's type given in the explicit instantiation
+ // definition against the one in the class template. This won't happen in
+ // explicit instantiation declaration because the instantiated code won't
+ // be generated in that case.
+ if (IsStaticDataMemberInstantiation &&
+ TSK == TSK_ExplicitInstantiationDefinition && Prev &&
+ !Context.hasSameTypeIgnoreLifetime(Prev->getType(), R)) {
----------------
Can we combine this with the previous `if`? This is really checking the same
thing that the `hasSameType` check above checked.
================
Comment at: clang/lib/Sema/SemaTemplate.cpp:10116
+ if (IsStaticDataMemberInstantiation &&
+ TSK == TSK_ExplicitInstantiationDefinition && Prev &&
+ !Context.hasSameTypeIgnoreLifetime(Prev->getType(), R)) {
----------------
The check of `TSK` seems unnecessary (and incorrect) here -- we should check
the type is correct regardless of whether this is an explicit instantiation
declaration or an explicit instantiation definition.
================
Comment at: clang/lib/Sema/SemaTemplate.cpp:10117
+ TSK == TSK_ExplicitInstantiationDefinition && Prev &&
+ !Context.hasSameTypeIgnoreLifetime(Prev->getType(), R)) {
+ Diag(T->getTypeLoc().getBeginLoc(),
----------------
Please can you point us at an example that needs this "ignore lifetime" nuance?
We should check with Apple folks what they want to happen here, and we should
presumably have the same rule for all explicit instantiations of templated
variables -- both in the static data member case here and the variable template
case a few lines above.
My expectation is that we want one of these two rules:
1) the declared lifetime should match exactly between the declaration and the
explicit instantiation, or
2) there cannot be a declared lifetime on the explicit instantiation, and a
lifetime on the template declaration is ignored by the check
(or a combination of these rules where we accept either option). I don't think
that matches what you're doing here -- in particular, I think a wrong declared
lifetime on the explicit instantiation should result in an error.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90448/new/
https://reviews.llvm.org/D90448
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
