This is an archive of the discontinued LLVM Phabricator instance.

Use primary template parameter names for variable template debug info
ClosedPublic

Authored by rnk on May 1 2019, 3:14 PM.

Details

Summary

Fixes PR41677

Consider:

template <typename LHS, typename RHS> constexpr bool is_same_v = false;
template <typename T> constexpr bool is_same_v<T, T> = true;
template constexpr bool is_same_v<int, int>;

Before this change, when emitting debug info for the
is_same_v<int, int> global variable, clang would crash because it
would try to use the template parameter list from the partial
specialization to give parameter names to template arguments. This
doesn't work in general, since a partial specialization can have fewer
arguments than the primary template. Therefore, always use the primary
template. Hypothetically we could try to use the parameter names from
the partial specialization when possible, but it's not clear this really
helps debugging in practice.

Diff Detail

Repository
rL LLVM

Event Timeline

rnk created this revision.May 1 2019, 3:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 1 2019, 3:14 PM
dblaikie accepted this revision.May 1 2019, 4:05 PM

Seems totally reasonable to me & it's what we do for other templates (function and class), it seems.

Might want to check alias templates too?

This revision is now accepted and ready to land.May 1 2019, 4:05 PM

Seems totally reasonable to me & it's what we do for other templates (function and class), it seems.

Might want to check alias templates too?

Ah, nevermind - alias templates can't be partially specialized. No worries then :)

rnk added a comment.May 2 2019, 10:29 AM

Ah, nevermind - alias templates can't be partially specialized. No worries then :)

That, and I think alias templates can only be created for types? So I don't think we can get to this codepath with an alias from using X = Y.

Thanks for the review, landing soon with minorly improved comments.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 2 2019, 10:43 AM