diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td index 43107044720e..bf188cf14f91 100644 --- a/clang/include/clang/Basic/DiagnosticParseKinds.td +++ b/clang/include/clang/Basic/DiagnosticParseKinds.td @@ -166,6 +166,8 @@ def err_use_of_tag_name_without_tag : Error< "use of tagged type %0 without '%1' tag">; def err_expected_ident_in_using : Error< "expected an identifier in using directive">; +def err_unexected_colon_in_nested_name_spec : Error< + "unexpected ':' in nested name specifier">; /// Objective-C parser diagnostics def err_objc_no_attributes_on_category : Error< diff --git a/clang/include/clang/Parse/Action.h b/clang/include/clang/Parse/Action.h index fc56cc68476e..200bd8a6b21e 100644 --- a/clang/include/clang/Parse/Action.h +++ b/clang/include/clang/Parse/Action.h @@ -334,6 +334,20 @@ public: bool EnteringContext) { return 0; } + + /// IsInvalidUnlessNestedName - This method is used for error recovery + /// purposes to determine whether the specified identifier is only valid as + /// a nested name specifier, for example a namespace name. It is + /// conservatively correct to always return false from this method. + /// + /// The arguments are the same as those passed to ActOnCXXNestedNameSpecifier. + virtual bool IsInvalidUnlessNestedName(Scope *S, + const CXXScopeSpec &SS, + IdentifierInfo &II, + TypeTy *ObjectType, + bool EnteringContext) { + return false; + } /// ActOnCXXNestedNameSpecifier - Called during parsing of a /// nested-name-specifier that involves a template-id, e.g., diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index d906b09d9bb7..3814cb9cc1a2 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -887,7 +887,8 @@ private: bool ParseOptionalCXXScopeSpecifier(CXXScopeSpec &SS, TypeTy *ObjectType, - bool EnteringContext); + bool EnteringContext, + bool ColonIsSacred = false); //===--------------------------------------------------------------------===// // C++ 5.2p1: C++ Casts diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index d852127b1fb1..238048f6153e 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -132,7 +132,7 @@ Parser::DeclPtrTy Parser::ParseNamespaceAlias(SourceLocation NamespaceLoc, CXXScopeSpec SS; // Parse (optional) nested-name-specifier. - ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/0, false); + ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/0, false, false); if (SS.isInvalid() || Tok.isNot(tok::identifier)) { Diag(Tok, diag::err_expected_namespace_name); @@ -260,7 +260,7 @@ Parser::DeclPtrTy Parser::ParseUsingDirective(unsigned Context, CXXScopeSpec SS; // Parse (optional) nested-name-specifier. - ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/0, false); + ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/0, false, false); IdentifierInfo *NamespcName = 0; SourceLocation IdentLoc = SourceLocation(); @@ -322,7 +322,7 @@ Parser::DeclPtrTy Parser::ParseUsingDeclaration(unsigned Context, IsTypeName = false; // Parse nested-name-specifier. - ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/0, false); + ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/0, false, false); AttributeList *AttrList = 0; @@ -601,7 +601,7 @@ void Parser::ParseClassSpecifier(tok::TokenKind TagTokKind, // Parse the (optional) nested-name-specifier. CXXScopeSpec SS; if (getLang().CPlusPlus && - ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/0, true)) + ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/0, true, true)) if (Tok.isNot(tok::identifier) && Tok.isNot(tok::annot_template_id)) Diag(Tok, diag::err_expected_ident); diff --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp index 4eb6cd5daa46..ae2a47befd41 100644 --- a/clang/lib/Parse/ParseExprCXX.cpp +++ b/clang/lib/Parse/ParseExprCXX.cpp @@ -45,10 +45,14 @@ using namespace clang; /// \param EnteringContext whether we will be entering into the context of /// the nested-name-specifier after parsing it. /// +/// \param ColonIsSacred - If this is true, then a colon is valid after the +/// specifier, so we should not try to recover from colons aggressively. +/// /// \returns true if a scope specifier was parsed. bool Parser::ParseOptionalCXXScopeSpecifier(CXXScopeSpec &SS, Action::TypeTy *ObjectType, - bool EnteringContext) { + bool EnteringContext, + bool ColonIsSacred) { assert(getLang().CPlusPlus && "Call sites of this function should be guarded by checking for C++"); @@ -214,11 +218,29 @@ bool Parser::ParseOptionalCXXScopeSpecifier(CXXScopeSpec &SS, // namespace-name '::' // nested-name-specifier identifier '::' Token Next = NextToken(); + + // If we get foo:bar, this is almost certainly a typo for foo::bar. Recover + // and emit a fixit hint for it. + if (Next.is(tok::colon) && !ColonIsSacred && + Actions.IsInvalidUnlessNestedName(CurScope, SS, II, ObjectType, + EnteringContext) && + // If the token after the colon isn't an identifier, it's still an + // error, but they probably meant something else strange so don't + // recover like this. + PP.LookAhead(1).is(tok::identifier)) { + Diag(Next, diag::err_unexected_colon_in_nested_name_spec) + << CodeModificationHint::CreateReplacement(Next.getLocation(), "::"); + + // Recover as if the user wrote '::'. + Next.setKind(tok::coloncolon); + } + if (Next.is(tok::coloncolon)) { // We have an identifier followed by a '::'. Lookup this name // as the name in a nested-name-specifier. SourceLocation IdLoc = ConsumeToken(); - assert(Tok.is(tok::coloncolon) && "NextToken() not working properly!"); + assert((Tok.is(tok::coloncolon) || Tok.is(tok::colon)) && + "NextToken() not working properly!"); SourceLocation CCLoc = ConsumeToken(); if (!HasScopeSpecifier) { diff --git a/clang/lib/Sema/Sema.h b/clang/lib/Sema/Sema.h index 9606821f78a5..2fb317635f4e 100644 --- a/clang/lib/Sema/Sema.h +++ b/clang/lib/Sema/Sema.h @@ -2000,7 +2000,8 @@ public: IdentifierInfo &II, QualType ObjectType, NamedDecl *ScopeLookupResult, - bool EnteringContext); + bool EnteringContext, + bool ErrorRecoveryLookup); virtual CXXScopeTy *ActOnCXXNestedNameSpecifier(Scope *S, const CXXScopeSpec &SS, @@ -2010,6 +2011,12 @@ public: TypeTy *ObjectType, bool EnteringContext); + virtual bool IsInvalidUnlessNestedName(Scope *S, + const CXXScopeSpec &SS, + IdentifierInfo &II, + TypeTy *ObjectType, + bool EnteringContext); + /// ActOnCXXNestedNameSpecifier - Called during parsing of a /// nested-name-specifier that involves a template-id, e.g., /// "foo::bar::", and now we need to build a scope diff --git a/clang/lib/Sema/SemaCXXScopeSpec.cpp b/clang/lib/Sema/SemaCXXScopeSpec.cpp index 14db77440fed..3c5dd547d626 100644 --- a/clang/lib/Sema/SemaCXXScopeSpec.cpp +++ b/clang/lib/Sema/SemaCXXScopeSpec.cpp @@ -330,6 +330,12 @@ NamedDecl *Sema::FindFirstQualifierInScope(Scope *S, NestedNameSpecifier *NNS) { /// that it contains an extra parameter \p ScopeLookupResult, which provides /// the result of name lookup within the scope of the nested-name-specifier /// that was computed at template definitino time. +/// +/// If ErrorRecoveryLookup is true, then this call is used to improve error +/// recovery. This means that it should not emit diagnostics, it should +/// just return null on failure. It also means it should only return a valid +/// scope if it *knows* that the result is correct. It should not return in a +/// dependent context, for example. Sema::CXXScopeTy *Sema::BuildCXXNestedNameSpecifier(Scope *S, const CXXScopeSpec &SS, SourceLocation IdLoc, @@ -337,7 +343,8 @@ Sema::CXXScopeTy *Sema::BuildCXXNestedNameSpecifier(Scope *S, IdentifierInfo &II, QualType ObjectType, NamedDecl *ScopeLookupResult, - bool EnteringContext) { + bool EnteringContext, + bool ErrorRecoveryLookup) { NestedNameSpecifier *Prefix = static_cast(SS.getScopeRep()); @@ -403,6 +410,10 @@ Sema::CXXScopeTy *Sema::BuildCXXNestedNameSpecifier(Scope *S, ObjectTypeSearchedInScope = true; } } else if (isDependent) { + // Don't speculate if we're just trying to improve error recovery. + if (ErrorRecoveryLookup) + return 0; + // We were not able to compute the declaration context for a dependent // base object type or prior nested-name-specifier, so this // nested-name-specifier refers to an unknown specialization. Just build @@ -442,14 +453,17 @@ Sema::CXXScopeTy *Sema::BuildCXXNestedNameSpecifier(Scope *S, !Context.hasSameType( Context.getTypeDeclType(cast(OuterDecl)), Context.getTypeDeclType(cast(SD))))) { + if (ErrorRecoveryLookup) + return 0; + Diag(IdLoc, diag::err_nested_name_member_ref_lookup_ambiguous) << &II; Diag(SD->getLocation(), diag::note_ambig_member_ref_object_type) << ObjectType; Diag(OuterDecl->getLocation(), diag::note_ambig_member_ref_scope); - // Fall through so that we'll pick the name we found in the object type, - // since that's probably what the user wanted anyway. + // Fall through so that we'll pick the name we found in the object + // type, since that's probably what the user wanted anyway. } } @@ -469,6 +483,11 @@ Sema::CXXScopeTy *Sema::BuildCXXNestedNameSpecifier(Scope *S, T.getTypePtr()); } + // Otherwise, we have an error case. If we don't want diagnostics, just + // return an error now. + if (ErrorRecoveryLookup) + return 0; + // If we didn't find anything during our lookup, try again with // ordinary name lookup, which can help us produce better error // messages. @@ -509,7 +528,23 @@ Sema::CXXScopeTy *Sema::ActOnCXXNestedNameSpecifier(Scope *S, bool EnteringContext) { return BuildCXXNestedNameSpecifier(S, SS, IdLoc, CCLoc, II, QualType::getFromOpaquePtr(ObjectTypePtr), - /*ScopeLookupResult=*/0, EnteringContext); + /*ScopeLookupResult=*/0, EnteringContext, + false); +} + +/// IsInvalidUnlessNestedName - This method is used for error recovery +/// purposes to determine whether the specified identifier is only valid as +/// a nested name specifier, for example a namespace name. It is +/// conservatively correct to always return false from this method. +/// +/// The arguments are the same as those passed to ActOnCXXNestedNameSpecifier. +bool Sema::IsInvalidUnlessNestedName(Scope *S, const CXXScopeSpec &SS, + IdentifierInfo &II, TypeTy *ObjectType, + bool EnteringContext) { + return BuildCXXNestedNameSpecifier(S, SS, SourceLocation(), SourceLocation(), + II, QualType::getFromOpaquePtr(ObjectType), + /*ScopeLookupResult=*/0, EnteringContext, + true); } Sema::CXXScopeTy *Sema::ActOnCXXNestedNameSpecifier(Scope *S, diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index b029a3796715..8c251e70403e 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -5407,7 +5407,7 @@ TreeTransform::RebuildNestedNameSpecifier(NestedNameSpecifier *Prefix, Range.getEnd(), II, ObjectType, FirstQualifierInScope, - false)); + false, false)); } template diff --git a/clang/test/Parser/cxx-decl.cpp b/clang/test/Parser/cxx-decl.cpp index 3fa284282ad6..308700e50d66 100644 --- a/clang/test/Parser/cxx-decl.cpp +++ b/clang/test/Parser/cxx-decl.cpp @@ -1,3 +1,33 @@ // RUN: clang-cc -verify -fsyntax-only %s int x(*g); // expected-error {{use of undeclared identifier 'g'}} + + +// PR4451 - We should recover well from the typo of '::' as ':' in a2. +namespace y { + struct a { }; +} + +y::a a1; +y:a a2; // expected-error {{unexpected ':' in nested name specifier}} +y::a a3 = a2; + +// Some valid colons: +void foo() { +y: // label + y::a s; + + int a = 4; + a = a ? a : a+1; +} + +struct b : y::a {}; + +template +class someclass { + + int bar() { + T *P; + return 1 ? P->x : P->y; + } +}; diff --git a/clang/test/SemaCXX/nested-name-spec.cpp b/clang/test/SemaCXX/nested-name-spec.cpp index 83c72417d590..6a51261e26fc 100644 --- a/clang/test/SemaCXX/nested-name-spec.cpp +++ b/clang/test/SemaCXX/nested-name-spec.cpp @@ -186,12 +186,10 @@ class foo { }; -// PR4452 -// FIXME: This error recovery sucks. -foo a2; // expected-error {{unexpected namespace name 'somens': expected expression}} \ -expected-error {{C++ requires a type specifier for all declarations}} +// PR4452 / PR4451 +foo a2; // expected-error {{unexpected ':' in nested name specifier}} -somens::a a3 = a2; +somens::a a3 = a2; // expected-error {{cannot initialize 'a3' with an lvalue of type 'foo'}}