[cfe-commits] r90730 - in /cfe/trunk: include/clang/Basic/DiagnosticParseKinds.td include/clang/Parse/Action.h include/clang/Parse/Parser.h lib/Parse/ParseDeclCXX.cpp lib/Parse/ParseExprCXX.cpp lib/Sema/Sema.h lib/Sema/SemaCXXScopeSpec.cpp lib/Sema/TreeTransform.h test/Parser/cxx-decl.cpp test/SemaCXX/nested-name-spec.cpp

Chris Lattner sabre at nondot.org
Sun Dec 6 17:36:54 PST 2009


Author: lattner
Date: Sun Dec  6 19:36:53 2009
New Revision: 90730

URL: http://llvm.org/viewvc/llvm-project?rev=90730&view=rev
Log:
reapply my patch for PR4451, which improves diagnostics for :: vs : confusion.
This time with a fix to bail out when in a dependent context.

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
    cfe/trunk/include/clang/Parse/Action.h
    cfe/trunk/include/clang/Parse/Parser.h
    cfe/trunk/lib/Parse/ParseDeclCXX.cpp
    cfe/trunk/lib/Parse/ParseExprCXX.cpp
    cfe/trunk/lib/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp
    cfe/trunk/lib/Sema/TreeTransform.h
    cfe/trunk/test/Parser/cxx-decl.cpp
    cfe/trunk/test/SemaCXX/nested-name-spec.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=90730&r1=90729&r2=90730&view=diff

==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Sun Dec  6 19:36:53 2009
@@ -166,6 +166,8 @@
   "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<

Modified: cfe/trunk/include/clang/Parse/Action.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Action.h?rev=90730&r1=90729&r2=90730&view=diff

==============================================================================
--- cfe/trunk/include/clang/Parse/Action.h (original)
+++ cfe/trunk/include/clang/Parse/Action.h Sun Dec  6 19:36:53 2009
@@ -334,6 +334,20 @@
                                                   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.,

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=90730&r1=90729&r2=90730&view=diff

==============================================================================
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Sun Dec  6 19:36:53 2009
@@ -887,7 +887,8 @@
 
   bool ParseOptionalCXXScopeSpecifier(CXXScopeSpec &SS,
                                       TypeTy *ObjectType,
-                                      bool EnteringContext);
+                                      bool EnteringContext,
+                                      bool ColonIsSacred = false);
 
   //===--------------------------------------------------------------------===//
   // C++ 5.2p1: C++ Casts

Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=90730&r1=90729&r2=90730&view=diff

==============================================================================
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Sun Dec  6 19:36:53 2009
@@ -132,7 +132,7 @@
   
   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 @@
   
   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 @@
     IsTypeName = false;
 
   // Parse nested-name-specifier.
-  ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/0, false);
+  ParseOptionalCXXScopeSpecifier(SS, /*ObjectType=*/0, false, false);
 
   AttributeList *AttrList = 0;
 
@@ -601,7 +601,7 @@
   // 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);
 

Modified: cfe/trunk/lib/Parse/ParseExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExprCXX.cpp?rev=90730&r1=90729&r2=90730&view=diff

==============================================================================
--- cfe/trunk/lib/Parse/ParseExprCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseExprCXX.cpp Sun Dec  6 19:36:53 2009
@@ -45,10 +45,14 @@
 /// \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 @@
     //   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) {

Modified: cfe/trunk/lib/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=90730&r1=90729&r2=90730&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/Sema.h (original)
+++ cfe/trunk/lib/Sema/Sema.h Sun Dec  6 19:36:53 2009
@@ -2000,7 +2000,8 @@
                                           IdentifierInfo &II,
                                           QualType ObjectType,
                                           NamedDecl *ScopeLookupResult,
-                                          bool EnteringContext);
+                                          bool EnteringContext,
+                                          bool ErrorRecoveryLookup);
 
   virtual CXXScopeTy *ActOnCXXNestedNameSpecifier(Scope *S,
                                                   const CXXScopeSpec &SS,
@@ -2010,6 +2011,12 @@
                                                   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<int, float>::", and now we need to build a scope

Modified: cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp?rev=90730&r1=90729&r2=90730&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp Sun Dec  6 19:36:53 2009
@@ -330,6 +330,12 @@
 /// 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 @@
                                                     IdentifierInfo &II,
                                                     QualType ObjectType,
                                                   NamedDecl *ScopeLookupResult,
-                                                    bool EnteringContext) {
+                                                    bool EnteringContext,
+                                                    bool ErrorRecoveryLookup) {
   NestedNameSpecifier *Prefix
     = static_cast<NestedNameSpecifier *>(SS.getScopeRep());
 
@@ -403,6 +410,10 @@
       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 @@
            !Context.hasSameType(
                             Context.getTypeDeclType(cast<TypeDecl>(OuterDecl)),
                                Context.getTypeDeclType(cast<TypeDecl>(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 @@
                                        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 @@
                                                     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,

Modified: cfe/trunk/lib/Sema/TreeTransform.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=90730&r1=90729&r2=90730&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/TreeTransform.h (original)
+++ cfe/trunk/lib/Sema/TreeTransform.h Sun Dec  6 19:36:53 2009
@@ -5407,7 +5407,7 @@
                                                         Range.getEnd(), II,
                                                         ObjectType,
                                                         FirstQualifierInScope,
-                                                        false));
+                                                        false, false));
 }
 
 template<typename Derived>

Modified: cfe/trunk/test/Parser/cxx-decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx-decl.cpp?rev=90730&r1=90729&r2=90730&view=diff

==============================================================================
--- cfe/trunk/test/Parser/cxx-decl.cpp (original)
+++ cfe/trunk/test/Parser/cxx-decl.cpp Sun Dec  6 19:36:53 2009
@@ -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 <typename T>
+class someclass {
+  
+  int bar() {
+    T *P;
+    return 1 ? P->x : P->y;
+  }
+};

Modified: cfe/trunk/test/SemaCXX/nested-name-spec.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/nested-name-spec.cpp?rev=90730&r1=90729&r2=90730&view=diff

==============================================================================
--- cfe/trunk/test/SemaCXX/nested-name-spec.cpp (original)
+++ cfe/trunk/test/SemaCXX/nested-name-spec.cpp Sun Dec  6 19:36:53 2009
@@ -186,12 +186,10 @@
 };
 
 
-// PR4452
-// FIXME: This error recovery sucks.
-foo<somens:a> a2;  // expected-error {{unexpected namespace name 'somens': expected expression}} \
-expected-error {{C++ requires a type specifier for all declarations}}
+// PR4452 / PR4451
+foo<somens:a> 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<somens::a>'}}
 
 
 





More information about the cfe-commits mailing list