Modifications to type handling logic. We no longer

perform recursive type lookups, because these are not
required for full type fidelity.  We also make the
SelectorTable last for the full lifetime of the Clang
compiler; this was the source of many bugs.

llvm-svn: 119835

Author: scallanan

lldb/include/lldb/Expression/ClangExpressionDeclMap.h

@@ -385,15 +385,7 @@ class ClangExpressionDeclMap
     ConstString                 m_result_name;              ///< The name of the result variable ($1, for example)
     TypeFromUser                m_object_pointer_type;      ///< The type of the "this" variable, if one exists.
 
-    llvm::DenseMap <const char*, bool>  m_lookedup_types;   ///< Contains each type that has been looked up in the current type lookup stack.
-                                                            ///< m_lookedup_types is used to gate the type search in GetDecls().  If a name is
-                                                            ///< not in it, the following procedure occurs:
-                                                            ///<   1 The name is added to m_lookedup_types.
-                                                            ///<   2 The type is looked up and added, potentially causing more type loookups.
-                                                            ///<   3 The name is removed from m_lookedup_types.
-                                                            ///< There must be no non-fatal error path that permits the type search to complete
-                                                            ///< without removing the name from m_lookedup_types at the end.
-                                                            ///< m_lookedup_type assumes single threadedness.
+    bool                        m_ignore_lookups;           ///< True during an import when we should be ignoring type lookups.
 
     //------------------------------------------------------------------
     /// Given a stack frame, find a variable that matches the given name and 
@@ -611,6 +603,27 @@ class ClangExpressionDeclMap
                                   TypeFromUser type,
                                   lldb::addr_t addr, 
                                   Error &err);
+    
+    //------------------------------------------------------------------
+    /// A wrapper for ClangASTContext::CopyType that sets a flag that
+    /// indicates that we should not respond to queries during import.
+    ///
+    /// @param[in] dest_context
+    ///     The target AST context, typically the parser's AST context.
+    ///
+    /// @param[in] source_context
+    ///     The source AST context, typically the AST context of whatever
+    ///     symbol file the type was found in.
+    ///
+    /// @param[in] clang_type
+    ///     The source type.
+    ///
+    /// @return
+    ///     The imported type.
+    //------------------------------------------------------------------
+    void *GuardedCopyType (clang::ASTContext *dest_context, 
+                           clang::ASTContext *source_context,
+                           void *clang_type);
 };
 
 } // namespace lldb_private

lldb/include/lldb/Expression/ClangExpressionParser.h

@@ -177,6 +177,7 @@ class ClangExpressionParser
     std::auto_ptr<clang::FileSystemOptions>     m_file_system_options;  ///< The Clang file system options object used by the compiler
     std::auto_ptr<clang::CompilerInstance>      m_compiler;             ///< The Clang compiler used to parse expressions into IR
     std::auto_ptr<clang::Builtin::Context>      m_builtin_context;      ///< Context for Clang built-ins
+    std::auto_ptr<clang::SelectorTable>         m_selector_table;       ///< Selector table for Objective-C methods
     std::auto_ptr<clang::ASTContext>            m_ast_context;          ///< The AST context used to hold types and names for the parser
     std::auto_ptr<clang::CodeGenerator>         m_code_generator;       ///< [owned by the Execution Engine] The Clang object that generates IR
     RecordingMemoryManager                     *m_jit_mm;               ///< The memory manager for the LLVM JIT

lldb/source/Expression/ClangExpressionDeclMap.cpp

@@ -57,7 +57,7 @@ ClangExpressionDeclMap::ClangExpressionDeclMap (ExecutionContext *exe_ctx) :
     m_materialized_location (0),
     m_result_name (),
     m_object_pointer_type (),
-    m_lookedup_types ()
+    m_ignore_lookups (false)
 {
     if (exe_ctx)
     {
@@ -990,6 +990,13 @@ ClangExpressionDeclMap::GetDecls (NameSearchContext &context, const ConstString
     // Back out in all cases where we're not fully initialized
     if (m_exe_ctx.frame == NULL)
         return;
+    
+    if (m_ignore_lookups)
+    {
+        if (log)
+            log->Printf("Ignoring a query during an import");
+        return;
+    }
 
     SymbolContextList sc_list;
 
@@ -1117,36 +1124,22 @@ ClangExpressionDeclMap::GetDecls (NameSearchContext &context, const ConstString
             AddOneVariable(context, pvar);
     }
 
-    
-    // See information on gating of this operation next to the definition for
-    // m_lookedup_types.
-
-    if (m_lookedup_types.find(name_unique_cstr) == m_lookedup_types.end())
-    {
-        // 1 The name is added to m_lookedup_types.
-        m_lookedup_types.insert(std::pair<const char*, bool>(name_unique_cstr, true));
-        
-        // 2 The type is looked up and added, potentially causing more type loookups.
-        lldb::TypeSP type_sp (m_sym_ctx.FindTypeByName (name));
+    lldb::TypeSP type_sp (m_sym_ctx.FindTypeByName (name));
 
-        if (type_sp)
+    if (type_sp)
+    {
+        if (log)
         {
-            if (log)
-            {
-                log->Printf ("Matching type found for \"%s\": ", name.GetCString());
-                StreamString strm;
-                type_sp->Dump(&strm, true);
-                log->PutCString (strm.GetData());
-            }
+            log->Printf ("Matching type found for \"%s\": ", name.GetCString());
+            StreamString strm;
+            type_sp->Dump(&strm, true);
+            log->PutCString (strm.GetData());
+        }
 
-            TypeFromUser user_type(type_sp->GetClangType(),
+        TypeFromUser user_type(type_sp->GetClangType(),
                                    type_sp->GetClangAST());
 
-            AddOneType(context, user_type, false);
-        }
-        
-        // 3 The name is removed from m_lookedup_types.
-        m_lookedup_types.erase(name_unique_cstr);
+        AddOneType(context, user_type, false);
     }
 }
 
@@ -1225,7 +1218,7 @@ ClangExpressionDeclMap::GetVariableValue
 
     if (parser_ast_context)
     {
-        type_to_use = ClangASTContext::CopyType(parser_ast_context, var_ast_context, var_opaque_type);
+        type_to_use = GuardedCopyType(parser_ast_context, var_ast_context, var_opaque_type);
 
         if (parser_type)
             *parser_type = TypeFromParser(type_to_use, parser_ast_context);
@@ -1310,9 +1303,9 @@ ClangExpressionDeclMap::AddOneVariable(NameSearchContext &context,
 
     TypeFromUser user_type = pvar->m_user_type;
 
-    TypeFromParser parser_type(ClangASTContext::CopyType(context.GetASTContext(), 
-                                                         user_type.GetASTContext(), 
-                                                         user_type.GetOpaqueQualType()),
+    TypeFromParser parser_type(GuardedCopyType(context.GetASTContext(), 
+                                               user_type.GetASTContext(), 
+                                               user_type.GetOpaqueQualType()),
                                context.GetASTContext());
 
     NamedDecl *var_decl = context.AddVarDecl(parser_type.GetOpaqueQualType());
@@ -1386,7 +1379,7 @@ ClangExpressionDeclMap::AddOneFunction(NameSearchContext &context,
 
         TypeList *type_list = fun_type->GetTypeList();
         fun_ast_context = type_list->GetClangASTContext().getASTContext();
-        void *copied_type = ClangASTContext::CopyType(context.GetASTContext(), fun_ast_context, fun_opaque_type);
+        void *copied_type = GuardedCopyType(context.GetASTContext(), fun_ast_context, fun_opaque_type);
 
         fun_decl = context.AddFunDecl(copied_type);
     }
@@ -1436,7 +1429,7 @@ ClangExpressionDeclMap::AddOneType(NameSearchContext &context,
     clang::ASTContext *parser_ast_context = context.GetASTContext();
     clang::ASTContext *user_ast_context = ut.GetASTContext();
 
-    void *copied_type = ClangASTContext::CopyType(parser_ast_context, user_ast_context, ut.GetOpaqueQualType());
+    void *copied_type = GuardedCopyType(parser_ast_context, user_ast_context, ut.GetOpaqueQualType());
 
     TypeFromParser parser_type(copied_type, parser_ast_context);
 
@@ -1471,3 +1464,19 @@ ClangExpressionDeclMap::AddOneType(NameSearchContext &context,
 
     context.AddTypeDecl(copied_type);
 }
+
+void * 
+ClangExpressionDeclMap::GuardedCopyType (ASTContext *dest_context, 
+                                         ASTContext *source_context,
+                                         void *clang_type)
+{
+    m_ignore_lookups = true;
+    
+    void *ret = ClangASTContext::CopyType (dest_context,
+                                           source_context,
+                                           clang_type);
+    
+    m_ignore_lookups = false;
+    
+    return ret;
+}

lldb/source/Expression/ClangExpressionParser.cpp

@@ -254,14 +254,14 @@ ClangExpressionParser::ClangExpressionParser(const char *target_triple,
 
     // 6. Most of this we get from the CompilerInstance, but we 
     // also want to give the context an ExternalASTSource.
-    SelectorTable selector_table;
+    m_selector_table.reset(new SelectorTable());
     m_builtin_context.reset(new Builtin::Context(m_compiler->getTarget()));
 
     std::auto_ptr<clang::ASTContext> ast_context(new ASTContext(m_compiler->getLangOpts(),
                                                                 m_compiler->getSourceManager(),
                                                                 m_compiler->getTarget(),
                                                                 m_compiler->getPreprocessor().getIdentifierTable(),
-                                                                selector_table,
+                                                                *m_selector_table.get(),
                                                                 *m_builtin_context.get(),
                                                                 0));