[cfe-dev] Error on valid; and q. about using tentative parse

[Robinson, Paul]

Clang is giving a compile time error on the following test case:

    template<int V0>
    struct T0
    {
        enum { E = V0 };
    };  

    template<int V1>
    struct T1
    {
        // The name declared here is the same as the type template
        // declared above.
        static int const T0 = T0<V1>::E;
    };

with the compile time error:

    test.cpp:12:35: error: no member named 'E' in the global namespace
      static int const T0 = T0<V1>::E;

The error is meaningless, it is just a result of clang not correctly
compiling the scope qualifier.  The issue is that the declaration of
"T0" in "struct T1" is conflicting with the lookup of "T0<V1>::E".

Per C++ standard, 3.4.3p1 [basic.lookup.qual], we believe the test case
is valid C++ code.

    The name of a class or namespace member or enumerator can be
    referred to after the :: scope resolution operator (5.1) applied to
    a nested-name-specifier that denotes its class, namespace, or
    enumeration.  If a :: scope resolution operator in a
    nested-name-specifier is not preceded by a decltype-specifier,
    lookup of the name preceding that :: considers only namespaces,
    types, and templates whose specializations are types.  If the name
    found does not designate a namespace or a class, enumeration, or
    dependent type, the program is ill-formed.

In our testcase "T0<V1>" is a template specialization that resolves to a
type, and it is being used to refer to an enumerator with the :: scope
resolution operator.  The important part is that the lookup of the
nested-name-specifier should only look at types, and should not be
considering "T0" from the declaration "static int const T0".
The decltype-specifier case doesn't apply here.

Inspecting the clang source code, we can see that
  clang/lib/Parse/ParseExprCXX.cpp:  Parser::ParseOptionalCXXScopeSpecifier()
is calling
  clang/lib/Sema/SemaTemplate.cpp:  isTemplateName()
which always calls
  LookupResult R(*this, TName, Name.getLocStart(), LookupOrdinaryName);
to find the name.  The hardcoded "LookupOrdinaryName" is a problem.
When being called in the context of looking up a scope qualifier, it
should be using "LookupNestedNameSpecifierName".

ParseOptionalCXXScopeSpecifier() has at least 4 expected side effects:
  - consume scope qualifer tokens
  - set the CXXScopeSpec &SS parameter
  - generate diagnostics
  - convert template tokens to an annotated token

Any fix must not generate duplicate or incorrect diagnostics, or create
incorrect template annotations (e.g., using the wrong lookup name). It
also must create annotated template tokens even if it is not then
consumed by this function, because following parse code expects
templates to be annotated.

LookupResult in isTemplateName() should behave differently depending on
whether a template is followed by the :: scope operator.  It seems the
only way to do this is to tentatively parse the template, look for a
following :: token and then pass either LookupNestedNameSpecifierName or
LookupOrdinaryName to isTemplateName() to use in the LookupResult
declaration.

The problem with tentative parsing is that the relevant parse code
(e.g., ParseTemplateIdAfterTemplateName()) generates diagnostics with no
reasonable way to turn them off and we didn't want to propagate a
flag down many levels of Parse*() calls.  LookupTemplateName() and
LookupResult() also generate diagnostics, but can be easily patched to
suppress them.  

A general solution would be to disable all diagnostics while doing a 
tentative parse.  

TentativeParsingAction can be used to restore tokens that were consumed
during a tentative parse.  It might be nice if it suppressed or saved
diagnostics.

So, two questions.
1) Is our interpretation of the standard correct, and this is valid code?
2) Would turning off diagnostics during a tentative parse make sense?
If we do that, we think we can fix the error.

Thanks,
--paulr