================
@@ -2535,10 +2535,34 @@ Decl
*TemplateDeclInstantiator::VisitVarTemplatePartialSpecializationDecl(
// Lookup the already-instantiated declaration and return that.
DeclContext::lookup_result Found = Owner->lookup(VarTemplate->getDeclName());
+
+ // Normally the primary member variable template has already been
instantiated
+ // into Owner, because it is declared before its partial specializations and
+ // so is visited first while instantiating the enclosing class. However, when
+ // the class template pattern is deserialized from a module or precompiled
+ // preamble, the primary may not have been materialized into Owner yet,
+ // leaving the lookup empty. Previously this asserted (and crashed release
+ // builds with a null dereference). Instantiate the primary on demand and
look
+ // it up again.
+ if (Found.empty()) {
+ if (Decl *InstPrimary = Visit(VarTemplate))
+ if (auto *InstVTD = dyn_cast<VarTemplateDecl>(InstPrimary))
+ Found = Owner->lookup(InstVTD->getDeclName());
+ }
+
+ // After the on-demand instantiation above the primary must be present. Keep
+ // the original invariant assertions so a genuinely-missing primary is still
+ // flagged in +Asserts builds, with a null return as a release-safe guard so
a
+ // stray case degrades gracefully instead of dereferencing an empty lookup
+ // result.
assert(!Found.empty() && "Instantiation found nothing?");
+ if (Found.empty())
+ return nullptr;
VarTemplateDecl *InstVarTemplate = dyn_cast<VarTemplateDecl>(Found.front());
assert(InstVarTemplate && "Instantiation did not find a variable template?");
+ if (!InstVarTemplate)
+ return nullptr;
----------------
RexTechnology1 wrote:
Fair point, and thanks for catching it. The assert next to the release if was
an obvious defect that I should have caught in my review. I've gone back
through the function; here's my understanding:
What this function does is it instantiates a partial specialization of a member
variable template, and to do that it needs the already instantiated primary to
attach the partial spec to. Normally the primary is instantiated first since
it's declared first in the class, so Owner->lookup() finds it. When the
enclosing class template is deserialized from a PCH/module built with errors,
the primary hasn't been instantiated into Owner yet, the lookup comes back
empty, and Found.front() then reads past the end. That's the crash.
So the question is what to do when the primary isn't there. Just returning
nullptr stops the crash but it's wrong. I tried it, and the instantiated class
ends up with no cons_from member at all (error: no member named 'cons_from'),
because nothing else ever creates it. So instead I instantiate the primary on
demand and then carry on as before. With that, wrapper<int,int> comes out
identical to a normal non-deserialized compile (one primary, one partial spec.
I dumped the AST to confirm it's not double instantiating the primary).
```
// The primary member variable template is normally instantiated into Owner
// before its partial specializations, because it's declared first. That may
// not hold when the enclosing class template is deserialized from a PCH or
// module built with errors, so instantiate the primary on demand here.
if (Owner->lookup(VarTemplate->getDeclName()).empty())
Visit(VarTemplate);
DeclContext::lookup_result Found = Owner->lookup(VarTemplate->getDeclName());
if (Found.empty())
return nullptr;
VarTemplateDecl *InstVarTemplate = dyn_cast<VarTemplateDecl>(Found.front());
if (!InstVarTemplate)
return nullptr;```
https://github.com/llvm/llvm-project/pull/202958
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
