[PATCH] c++: error message for dependent template members [PR70417]

Anthony Sharp anthonysharp15@gmail.com
Fri Aug 20 16:56:19 GMT 2021


Hi, hope everyone is well. I have a patch here for issue 70417
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70417). I'm still a GCC
noob, and this is probably the hardest thing I have ever coded in my
life, so please forgive any initial mistakes!

TLDR

This patch introduces a helpful error message when the user attempts
to put a template-id after a member access token (., -> or ::) in a
dependent context without having put the "template" keyword after the
access token.

CONTEXT

In C++, when referencing a class member (using ., -> or ::) in a
dependent context, if that member is a template, the template keyword
is required after the access token. For example:

template<class T> void MyDependentTemplate(T t)
{
  t.DoSomething<T>(); /* Error - t is dependent. "<" is treated as a
less-than sign because DoSomething is assumed to be a non-template
identifier. */
  t.template DoSomething<T>(); // Good.
}

PATCH EXPLANATION

In order to throw an appropriate error message we need to identify
and/or create a point in the compiler where the following conditions
are all simultaneously satisfied:

A) a class member access token (., ->, ::)
B) a dependent context
C) the thing being accessed is a template-id
D) No template keyword

A, B and D are relatively straightforward and I won't go into detail
about how that was achieved. The real challenge is C - figuring out
whether a name is a template-id.

Usually, we would look up the name and use that to determine whether
the name is a template or not. But, we cannot do that. We are in a
dependent context, so lookup cannot (in theory) find anything at the
point the access expression is parsed.

We (maybe) could defer checking until the template is actually
instantiated. But this is not the approach I have gone for, since this
to me sounded unnecessarily awkward and clunky. In my mind this also
seems like a syntax error, and IMO it seems more natural that syntax
errors should get caught as soon as they are parsed.

So, the solution I went for was to figure out whether the name was a
template by looking at the surrounding tokens. The key to this lies in
being able to differentiate between the start and end of a template
parameter list (< and >) and greater-than and less-than tokens (and
perhaps also things like << or >>). If the final > (if we find one at
all) does indeed mark the end of a class or function template, then it
will be followed by something like (, ::, or even just ;. On the other
hand, a greater-than operator would be followed by a name.

PERFORMANCE IMPACT

My concern with this approach was that checking this would actually
slow the compiler down. In the end it seemed the opposite was true. By
parsing the tokens to determine whether the name looks like a
template-id, we can cut out a lot of the template-checking code that
gets run trying to find a template when there is none, making compile
time generally faster. That being said, I'm not sure if the
improvement will be that substantial, so I did not feel it necessary
to introduce the template checking method everywhere where we could
possibly have run into a template.

I ran an ABAB test with the following code repeated enough times to
fill up about 10,000 lines:

ai = 1;
bi = 2;
ci = 3;
c.x = 4;
(&c)->x = 5;
mytemplateclass<Y>::y = 6;
c.template class_func<Y>();
(&c)->template class_func<Y>();
mytemplateclass<Y>::template class_func_static<Y>();
c2.class_func<myclass>();
(&c2)->class_func<myclass>();
myclass::class_func_static<myclass>();
ai = 6;
bi = 5;
ci = 4;
c.x = 3;
(&c)->x = 2;
mytemplateclass<Y>::y = 1;
c.template class_func<Y>();
(&c)->template class_func<Y>();
mytemplateclass<Y>::template class_func_static<Y>();
c2.class_func<myclass>();
(&c2)->class_func<myclass>();
myclass::class_func_static<myclass>();

It basically just contains a mixture of class access expressions plus
some regular assignments for good measure. The compiler was compiled
without any optimisations (and so slower than usual). In hindsight,
perhaps this test case assumes the worst-ish-case scenario since it
contains a lot of templates; most of the benefits of this patch occur
when parsing non-templates.

The compiler (clean) and the compiler with the patch applied (patched)
compiled the above code 20 times each in ABAB fashion. On average, the
clean compiler took 2.26295 seconds and the patched compiler took
2.25145 seconds (about 0.508% faster). Whether this improvement
remains or disappears when the compiler is built with optimisations
turned on I haven't tested. My main concern was just making sure it
didn't get any slower.

AWKWARD TEST CASES

You will see from the patch that it required the updating of a few
testcases (as well as one place within the compiler). I believe this
is because up until now, the compiler allowed the omission of the
template keyword in some places it shouldn't have. Take decltype34.C
for example:

struct impl
{
  template <class T> static T create();
};

template<class T, class U, class =
decltype(impl::create<T>()->impl::create<U>())>
struct tester{};

GCC currently accepts this code. My patch causes it to be rejected.
For what it's worth, Clang also rejects this code:

1824786093/source.cpp:6:70: error: use 'template' keyword to treat
'create' as a dependent template name
template<class T, class U, class =
decltype(impl::create<T>()->impl::create<U>())>

                                      ^ template

I think Clang is correct since from what I understand it should be
written as impl::create<T>()->impl::template create<U>(). Here,
create<T>() is dependent, so it makes sense that we would need
"template" before the scope operator. Then again, I could well be
wrong. The rules around dependent template names are pretty crazy.

REGRESSION TESTING

No differences between clean/patched g++.sum, gcc.sum and
libstdc++.sum. Bootstraps fine. Built on x86_64-pc-linux-gnu for
x86_64-pc-linux-gnu.

CONCLUSION

Some of the changes are a bit debatable, e.g. passing and adding new
arguments to various functions just for the sake of a slight
performance improvement - I don't really have the high-level
experience to be able to judge whether that is worth the increased
code complexity or not, so I just went with my gut.

Please find patch attached :)

Kind regards,
Anthony Sharp
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr70417.patch
Type: text/x-patch
Size: 34195 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20210820/8d2ce748/attachment-0001.bin>


More information about the Gcc-patches mailing list