The attached patch fixes PR11358:
pr11358.cc:11:5: error: missing 'typename' prior to dependent type name
'Container::iterator'
Container::iterator i = c.begin();
^~~~~~~~~~~~~~~~~~~
typename
It works by teaching Parser::isCXXDeclarationSpecifier() to, in the event
of a cxxscope annotation followed by an identifier, ("Container::" and
"iterator" in the above example), look at the next token. Yes, this means
doing two-tokens of look-ahead. If that next token is an identifier or
cvr-qualifier, we conclude that this can't possibly be an expression and
return a parse error.
This is pretty important because some developers don't seem to understand
the rule for where they need to put typename, and Clang has exacerbated
this by teaching them that they only need to put typename where the
compiler tells them to put typename. Consequently, when we don't tell them
to put typename there, that isn't one of the things they'll try any more.
The condition under which we'll do an extra token of look-ahead is when we
see a dependent nested-name-specifier followed by an identifier while doing
the tentative parse to determine whether the statement is a declaration or
an expression. This is reasonably rare. Here's some build times of boost:
run of "bootstrap"
with patch: 0m26.460s 0m26.530s
without patch: 0m25.510s 0m26.110s
run of "b2"
with patch: 13m12.050s 12m58.670s
without patch: 12m26.260s 13m9.030s
So we conclude that my testing machine is crazy noisy, but at least the
patch doesn't cause a horrible performance regression. To give you another
statistic, the number of times we need to look ahead by one more token is
slightly less than 250,000 times across all translation units in an
llvm+clang build. I haven't checked how many of those are unique.
Please review!
Nick
Index: test/SemaCXX/PR11358.cpp
===================================================================
--- test/SemaCXX/PR11358.cpp (revision 0)
+++ test/SemaCXX/PR11358.cpp (revision 0)
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -verify
+// PR11358
+
+template<typename T>
+struct container {
+ class iterator {};
+ iterator begin() { return iterator(); }
+};
+
+template<typename T>
+struct Test {
+ typedef container<T> Container;
+ void test() {
+ Container::iterator i = c.begin(); // expected-error{{missing 'typename'}}
+ }
+ Container c;
+};
+
Index: lib/Parse/ParseTentative.cpp
===================================================================
--- lib/Parse/ParseTentative.cpp (revision 148493)
+++ lib/Parse/ParseTentative.cpp (working copy)
@@ -864,7 +864,7 @@
if (TryAnnotateTypeOrScopeToken())
return TPResult::Error();
return isCXXDeclarationSpecifier();
-
+
// decl-specifier:
// storage-class-specifier
// type-specifier
@@ -950,8 +950,23 @@
// We've already annotated a scope; try to annotate a type.
if (TryAnnotateTypeOrScopeToken())
return TPResult::Error();
- if (!Tok.is(tok::annot_typename))
+ if (!Tok.is(tok::annot_typename)) {
+ // If the next token is an identifier or a type qualifier, then this
+ // can't possibly be a valid expression either.
+ if (Tok.is(tok::annot_cxxscope) && NextToken().is(tok::identifier)) {
+ CXXScopeSpec SS;
+ Actions.RestoreNestedNameSpecifierAnnotation(Tok.getAnnotationValue(),
+ Tok.getAnnotationRange(),
+ SS);
+ if (SS.getScopeRep() && SS.getScopeRep()->isDependent()) {
+ tok::TokenKind After = PP.LookAhead(1).getKind();
+ if (After == tok::identifier || After == tok::kw_const ||
+ After == tok::kw_volatile || After == tok::kw_restrict)
+ return TPResult::Error();
+ }
+ }
return TPResult::False();
+ }
// If that succeeded, fallthrough into the generic simple-type-id case.
// The ambiguity resides in a simple-type-specifier/typename-specifier
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
