[cfe-commits] patch: detect missing typename in dependent context.

[Nick Lewycky]

Ping! Sorry for the <1 week ping, but I just got another bug report about
this issue. Testcase:

#include <hash_map>
using namespace std;
using namespace __gnu_cxx;
template <typename KeyType, typename ValueType>
void MapTest(hash_map<KeyType, ValueType> map) {
  for (hash_map<KeyType, ValueType>::const_iterator it = map.begin(); it !=
map.end(); it++) {
  }
}

without this patch produces:

z.cc:6:53: error: expected ';' in 'for' statement specifier
  for (hash_map<KeyType, ValueType>::const_iterator it = map.begin(); ...
                                                    ^
z.cc:6:53: error: use of undeclared identifier 'it'
z.cc:6:71: error: use of undeclared identifier 'it'
  for (hash_map<KeyType, ValueType>::const_iterator it = map.begin(); it ...
                                                                      ^
z.cc:6:86: error: expected ')'
  ...ValueType>::const_iterator it = map.begin(); it != map.end(); it++) {
                                                                 ^
z.cc:6:7: note: to match this '('
  for (hash_map<KeyType, ValueType>::const_iterator it = map.begin(); ...
      ^
z.cc:6:88: error: use of undeclared identifier 'it'
  ...ValueType>::const_iterator it = map.begin(); it != map.end(); it++) {
                                                                   ^

but with this patch produces:

z.cc:6:8: error: missing 'typename' prior to dependent type name
      'hash_map<KeyType, ValueType>::const_iterator'
  for (hash_map<KeyType, ValueType>::const_iterator it = map.begin(); ...
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
       typename

Same patch attached again. Please review!

Nick


On 20 January 2012 17:54, Nick Lewycky <nlewycky at google.com> wrote:

> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120124/faa40b8d/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr11358-1.patch
Type: text/x-patch
Size: 2200 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120124/faa40b8d/attachment.bin>