]> granicus.if.org Git - clang/commitdiff
When performing typo correction, look through the set of known
authorDouglas Gregor <dgregor@apple.com>
Thu, 14 Oct 2010 22:11:03 +0000 (22:11 +0000)
committerDouglas Gregor <dgregor@apple.com>
Thu, 14 Oct 2010 22:11:03 +0000 (22:11 +0000)
identifiers to determine good typo-correction candidates. Once we've
identified those candidates, we perform name lookup on each of them
and the consider the results.

This optimization makes typo correction > 2x faster on a benchmark
example using a single typo (NSstring) in a tiny file that includes
Cocoa.h from a precompiled header, since we are deserializing far less
information now during typo correction.

There is a semantic change here, which is interesting. The presence of
a similarly-named entity that is not visible can now affect typo
correction. This is both good (you won't get weird corrections if the
thing you wanted isn't in scope) and bad (you won't get good
corrections if there is a similarly-named-but-completely-unrelated
thing). Time will tell whether it was a good choice or not.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@116528 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Basic/IdentifierTable.h
include/clang/Basic/OnDiskHashTable.h
include/clang/Serialization/ASTReader.h
lib/Basic/IdentifierTable.cpp
lib/Sema/SemaExprObjC.cpp
lib/Sema/SemaLookup.cpp
lib/Serialization/ASTReader.cpp
test/PCH/Inputs/typo.h [new file with mode: 0644]
test/PCH/typo.m [new file with mode: 0644]
test/SemaObjC/synth-provisional-ivars.m

index 2cb68b39bedff8f8e29eee5a0d9e5087f9396019..66c5deea547072db7b2c8535ae14b02ac28f8fbd 100644 (file)
@@ -254,6 +254,35 @@ private:
   }
 };
 
+/// \brief An iterator that walks over all of the known identifiers
+/// in the lookup table.
+///
+/// Since this iterator uses an abstract interface via virtual
+/// functions, it uses an object-oriented interface rather than the
+/// more standard C++ STL iterator interface. In this OO-style
+/// iteration, the single function \c Next() provides dereference,
+/// advance, and end-of-sequence checking in a single
+/// operation. Subclasses of this iterator type will provide the
+/// actual functionality.
+class IdentifierIterator {
+private:
+  IdentifierIterator(const IdentifierIterator&); // Do not implement
+  IdentifierIterator &operator=(const IdentifierIterator&); // Do not implement
+
+protected:
+  IdentifierIterator() { }
+  
+public:
+  virtual ~IdentifierIterator();
+
+  /// \brief Retrieve the next string in the identifier table and
+  /// advances the iterator for the following string.
+  ///
+  /// \returns The next string in the identifier table. If there is
+  /// no such string, returns an empty \c llvm::StringRef.
+  virtual llvm::StringRef Next() = 0;
+};
+
 /// IdentifierInfoLookup - An abstract class used by IdentifierTable that
 ///  provides an interface for performing lookups from strings
 /// (const char *) to IdentiferInfo objects.
@@ -266,6 +295,18 @@ public:
   ///  of a reference.  If the pointer is NULL then the IdentifierInfo cannot
   ///  be found.
   virtual IdentifierInfo* get(llvm::StringRef Name) = 0;
+
+  /// \brief Retrieve an iterator into the set of all identifiers
+  /// known to this identifier lookup source.
+  ///
+  /// This routine provides access to all of the identifiers known to
+  /// the identifier lookup, allowing access to the contents of the
+  /// identifiers without introducing the overhead of constructing
+  /// IdentifierInfo objects for each.
+  ///
+  /// \returns A new iterator into the set of known identifiers. The
+  /// caller is responsible for deleting this iterator.
+  virtual IdentifierIterator *getIdentifiers() const;
 };
 
 /// \brief An abstract class used to resolve numerical identifier
@@ -304,6 +345,11 @@ public:
     ExternalLookup = IILookup;
   }
 
+  /// \brief Retrieve the external identifier lookup object, if any.
+  IdentifierInfoLookup *getExternalIdentifierLookup() const {
+    return ExternalLookup;
+  }
+  
   llvm::BumpPtrAllocator& getAllocator() {
     return HashTable.getAllocator();
   }
index 8909e47146a7f418357a657ef23bd61e587839ae..30bf39ef43355f3d5d220021c0038130ca1b7d20 100644 (file)
@@ -334,6 +334,70 @@ public:
 
   iterator end() const { return iterator(); }
 
+  /// \brief Iterates over all of the keys in the table.
+  class key_iterator {
+    const unsigned char* Ptr;
+    unsigned NumItemsInBucketLeft;
+    unsigned NumEntriesLeft;
+    Info *InfoObj;
+  public:
+    typedef external_key_type value_type;
+
+    key_iterator(const unsigned char* const Ptr, unsigned NumEntries,
+                  Info *InfoObj)
+      : Ptr(Ptr), NumItemsInBucketLeft(0), NumEntriesLeft(NumEntries),
+        InfoObj(InfoObj) { }
+    key_iterator()
+      : Ptr(0), NumItemsInBucketLeft(0), NumEntriesLeft(0), InfoObj(0) { }
+
+    friend bool operator==(const key_iterator &X, const key_iterator &Y) {
+      return X.NumEntriesLeft == Y.NumEntriesLeft;
+    }
+    friend bool operator!=(const key_iterator& X, const key_iterator &Y) {
+      return X.NumEntriesLeft != Y.NumEntriesLeft;
+    }
+    
+    key_iterator& operator++() {  // Preincrement
+      if (!NumItemsInBucketLeft) {
+        // 'Items' starts with a 16-bit unsigned integer representing the
+        // number of items in this bucket.
+        NumItemsInBucketLeft = io::ReadUnalignedLE16(Ptr);
+      }
+      Ptr += 4; // Skip the hash.
+      // Determine the length of the key and the data.
+      const std::pair<unsigned, unsigned>& L = Info::ReadKeyDataLength(Ptr);
+      Ptr += L.first + L.second;
+      assert(NumItemsInBucketLeft);
+      --NumItemsInBucketLeft;
+      assert(NumEntriesLeft);
+      --NumEntriesLeft;
+      return *this;
+    }
+    key_iterator operator++(int) {  // Postincrement
+      key_iterator tmp = *this; ++*this; return tmp;
+    }
+
+    value_type operator*() const {
+      const unsigned char* LocalPtr = Ptr;
+      if (!NumItemsInBucketLeft)
+        LocalPtr += 2; // number of items in bucket
+      LocalPtr += 4; // Skip the hash.
+
+      // Determine the length of the key and the data.
+      const std::pair<unsigned, unsigned>& L
+        = Info::ReadKeyDataLength(LocalPtr);
+
+      // Read the key.
+      const internal_key_type& Key = InfoObj->ReadKey(LocalPtr, L.first);
+      return InfoObj->GetExternalKey(Key);
+    }
+  };
+
+  key_iterator key_begin() {
+    return key_iterator(Base + 4, getNumEntries(), &InfoObj);
+  }
+  key_iterator key_end() { return key_iterator(); }
+
   /// \brief Iterates over all the entries in the table, returning
   /// a key/data pair.
   class item_iterator {
index f275c76f0c503eddb2956c18cc810ba0437001b9..01ccc062fac49399b930eda0300cd0c3c2ca74c1 100644 (file)
@@ -47,6 +47,7 @@ namespace clang {
 class AddrLabelExpr;
 class ASTConsumer;
 class ASTContext;
+class ASTIdentifierIterator;
 class Attr;
 class Decl;
 class DeclContext;
@@ -180,6 +181,7 @@ public:
   friend class PCHValidator;
   friend class ASTDeclReader;
   friend class ASTStmtReader;
+  friend class ASTIdentifierIterator;
   friend class ASTIdentifierLookupTrait;
   friend class TypeLocReader;
 private:
@@ -969,6 +971,10 @@ public:
     return get(Name.begin(), Name.end());
   }
 
+  /// \brief Retrieve an iterator into the set of all identifiers
+  /// in all loaded AST files.
+  virtual IdentifierIterator *getIdentifiers() const;
+
   /// \brief Load the contents of the global method pool for a given
   /// selector.
   ///
index bd30c68da2b71734cbfebeaca9f8e686f0b31dad..4ea6cedeb56e93661777f506d48d51c77f605f32 100644 (file)
@@ -44,8 +44,24 @@ IdentifierInfo::IdentifierInfo() {
 // IdentifierTable Implementation
 //===----------------------------------------------------------------------===//
 
+IdentifierIterator::~IdentifierIterator() { }
+
 IdentifierInfoLookup::~IdentifierInfoLookup() {}
 
+namespace {
+  /// \brief A simple identifier lookup iterator that represents an
+  /// empty sequence of identifiers.
+  class EmptyLookupIterator : public IdentifierIterator
+  {
+  public:
+    virtual llvm::StringRef Next() { return llvm::StringRef(); }
+  };
+}
+
+IdentifierIterator *IdentifierInfoLookup::getIdentifiers() const {
+  return new EmptyLookupIterator();
+}
+
 ExternalIdentifierLookup::~ExternalIdentifierLookup() {}
 
 IdentifierTable::IdentifierTable(const LangOptions &LangOpts,
index 055edd971005f8d3a55b39a4fa4374a042066882..1a90a2aaff0497668450b4521174b6d47dd41ec8 100644 (file)
@@ -557,9 +557,10 @@ Sema::ObjCMessageKind Sema::getObjCMessageKind(Scope *S,
   ReceiverType = ParsedType();
 
   // If the identifier is "super" and there is no trailing dot, we're
-  // messaging super.
-  if (IsSuper && !HasTrailingDot && S->isInObjcMethodScope())
-    return ObjCSuperMessage;
+  // messaging super. If the identifier is "super" and there is a
+  // trailing dot, it's an instance message.
+  if (IsSuper && S->isInObjcMethodScope())
+    return HasTrailingDot? ObjCInstanceMessage : ObjCSuperMessage;
   
   LookupResult Result(*this, Name, NameLoc, LookupOrdinaryName);
   LookupName(Result, S);
@@ -568,14 +569,15 @@ Sema::ObjCMessageKind Sema::getObjCMessageKind(Scope *S,
   case LookupResult::NotFound:
     // Normal name lookup didn't find anything. If we're in an
     // Objective-C method, look for ivars. If we find one, we're done!
-    // FIXME: This is a hack. Ivar lookup should be part of normal lookup.
+    // FIXME: This is a hack. Ivar lookup should be part of normal
+    // lookup.
     if (ObjCMethodDecl *Method = getCurMethodDecl()) {
       ObjCInterfaceDecl *ClassDeclared;
       if (Method->getClassInterface()->lookupInstanceVariable(Name, 
                                                               ClassDeclared))
         return ObjCInstanceMessage;
     }
-      
+  
     // Break out; we'll perform typo correction below.
     break;
 
index b363e5795181be645029b144fd54061eacd70f5e..f12ac22f3de5783d49e7e7b9bf39d865d505e305 100644 (file)
@@ -2694,6 +2694,7 @@ public:
       BestEditDistance((std::numeric_limits<unsigned>::max)()) { }
 
   virtual void FoundDecl(NamedDecl *ND, NamedDecl *Hiding, bool InBaseClass);
+  void FoundName(llvm::StringRef Name);
   void addKeywordResult(ASTContext &Context, llvm::StringRef Keyword);
 
   typedef llvm::StringMap<bool, llvm::BumpPtrAllocator>::iterator iterator;
@@ -2721,10 +2722,17 @@ void TypoCorrectionConsumer::FoundDecl(NamedDecl *ND, NamedDecl *Hiding,
   if (!Name)
     return;
 
+  FoundName(Name->getName());
+}
+
+void TypoCorrectionConsumer::FoundName(llvm::StringRef Name) {
   // Compute the edit distance between the typo and the name of this
   // entity. If this edit distance is not worse than the best edit
   // distance we've seen so far, add it to the list of results.
-  unsigned ED = Typo.edit_distance(Name->getName());
+  unsigned ED = Typo.edit_distance(Name);
+  if (ED == 0)
+    return;
+  
   if (ED < BestEditDistance) {
     // This result is better than any we've seen before; clear out
     // the previous results.
@@ -2735,12 +2743,12 @@ void TypoCorrectionConsumer::FoundDecl(NamedDecl *ND, NamedDecl *Hiding,
     // ignore it.
     return;
   }
-
+  
   // Add this name to the list of results. By not assigning a value, we
   // keep the current value if we've seen this name before (either as a
   // keyword or as a declaration), or get the default value (not a keyword)
   // if we haven't seen it before.
-  (void)BestResults[Name->getName()];
+  (void)BestResults[Name];  
 }
 
 void TypoCorrectionConsumer::addKeywordResult(ASTContext &Context, 
@@ -2842,7 +2850,25 @@ DeclarationName Sema::CorrectTypo(LookupResult &Res, Scope *S, CXXScopeSpec *SS,
     
     LookupVisibleDecls(DC, Res.getLookupKind(), Consumer);
   } else {
-    LookupVisibleDecls(S, Res.getLookupKind(), Consumer);
+    // For unqualified lookup, look through all of the names that we have
+    // seen in this translation unit.
+    for (IdentifierTable::iterator I = Context.Idents.begin(), 
+                                IEnd = Context.Idents.end();
+         I != IEnd; ++I)
+      Consumer.FoundName(I->getKey());
+    
+    // Walk through identifiers in external identifier sources.
+    if (IdentifierInfoLookup *External
+                              = Context.Idents.getExternalIdentifierLookup()) {
+      IdentifierIterator *Iter = External->getIdentifiers();
+      do {
+        llvm::StringRef Name = Iter->Next();
+        if (Name.empty())
+          break;
+
+        Consumer.FoundName(Name);
+      } while (true);
+    }
   }
 
   // Add context-dependent keywords.
@@ -3017,7 +3043,7 @@ DeclarationName Sema::CorrectTypo(LookupResult &Res, Scope *S, CXXScopeSpec *SS,
   // Make sure that the user typed at least 3 characters for each correction 
   // made. Otherwise, we don't even both looking at the results.
   unsigned ED = Consumer.getBestEditDistance();
-  if (ED == 0 || (Typo->getName().size() / ED) < 3)
+  if (ED > 0 && Typo->getName().size() / ED < 3)
     return DeclarationName();
 
   // Weed out any names that could not be found by name lookup.
index 2002ccd1c2ba692f7c3272404a4e07afb03c7620..0f7486f82afc552926a4ded6449043870091d048 100644 (file)
@@ -603,6 +603,10 @@ public:
   static const internal_key_type&
   GetInternalKey(const external_key_type& x) { return x; }
 
+  // This hopefully will just get inlined and removed by the optimizer.
+  static const external_key_type&
+  GetExternalKey(const internal_key_type& x) { return x; }
+
   static std::pair<unsigned, unsigned>
   ReadKeyDataLength(const unsigned char*& d) {
     using namespace clang::io;
@@ -3571,6 +3575,64 @@ IdentifierInfo* ASTReader::get(const char *NameStart, const char *NameEnd) {
   return 0;
 }
 
+namespace clang {
+  /// \brief An identifier-lookup iterator that enumerates all of the
+  /// identifiers stored within a set of AST files.
+  class ASTIdentifierIterator : public IdentifierIterator {
+    /// \brief The AST reader whose identifiers are being enumerated.
+    const ASTReader &Reader;
+
+    /// \brief The current index into the chain of AST files stored in
+    /// the AST reader.
+    unsigned Index;
+
+    /// \brief The current position within the identifier lookup table
+    /// of the current AST file.
+    ASTIdentifierLookupTable::key_iterator Current;
+
+    /// \brief The end position within the identifier lookup table of
+    /// the current AST file.
+    ASTIdentifierLookupTable::key_iterator End;
+
+  public:
+    explicit ASTIdentifierIterator(const ASTReader &Reader);
+
+    virtual llvm::StringRef Next();
+  };
+}
+
+ASTIdentifierIterator::ASTIdentifierIterator(const ASTReader &Reader)
+  : Reader(Reader), Index(Reader.Chain.size() - 1) {
+  ASTIdentifierLookupTable *IdTable
+    = (ASTIdentifierLookupTable *)Reader.Chain[Index]->IdentifierLookupTable;
+  Current = IdTable->key_begin();
+  End = IdTable->key_end();
+}
+
+llvm::StringRef ASTIdentifierIterator::Next() {
+  while (Current == End) {
+    // If we have exhausted all of our AST files, we're done.
+    if (Index == 0)
+      return llvm::StringRef();
+
+    --Index;
+    ASTIdentifierLookupTable *IdTable
+      = (ASTIdentifierLookupTable *)Reader.Chain[Index]->IdentifierLookupTable;
+    Current = IdTable->key_begin();
+    End = IdTable->key_end();
+  }
+
+  // We have any identifiers remaining in the current AST file; return
+  // the next one.
+  std::pair<const char*, unsigned> Key = *Current;
+  ++Current;
+  return llvm::StringRef(Key.first, Key.second);
+}
+
+IdentifierIterator *ASTReader::getIdentifiers() const {
+  return new ASTIdentifierIterator(*this);
+}
+
 std::pair<ObjCMethodList, ObjCMethodList>
 ASTReader::ReadMethodPool(Selector Sel) {
   // Find this selector in a hash table. We want to find the most recent entry.
diff --git a/test/PCH/Inputs/typo.h b/test/PCH/Inputs/typo.h
new file mode 100644 (file)
index 0000000..63b553b
--- /dev/null
@@ -0,0 +1,6 @@
+
+
+@interface NSString
++ (id)alloc;
+@end
+
diff --git a/test/PCH/typo.m b/test/PCH/typo.m
new file mode 100644 (file)
index 0000000..c6f0275
--- /dev/null
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -x objective-c-header -emit-pch -o %t %S/Inputs/typo.h
+// RUN: %clang_cc1 -include-pch %t -verify %s
+// In header: expected-note{{declared here}}
+void f() {
+  [NSstring alloc]; // expected-error{{unknown receiver 'NSstring'; did you mean 'NSString'?}}
+}
index 973c771ad7797a09291f231966c077f395e8b0f1..8ad2233ba4a80f59cdeb0899ea1d4b87114f723b 100644 (file)
@@ -18,7 +18,7 @@ int bar;
 @end
 
 @implementation I
-- (int) Meth { return PROP; }  // expected-note {{'PROP' declared here}}
+- (int) Meth { return PROP; }
 
 @dynamic PROP1;
 - (int) Meth1 { return PROP1; }  // expected-error {{use of undeclared identifier 'PROP1'}}