[Lldb-commits] [PATCH] D12658: Search variables based on clang::DeclContext and clang::Decl tree

[Paul Herman via lldb-commits]

paulherman added a comment.

I agree with most of your comments and will fix them.

One thing I noticed is that Block::GetDeclContext() does not do the right thing (see inline comment).

About creating a CompilerDecl class, I agree with you. I used CompilerDeclContext since it was just a void* and it was convenient. Do you think that the FindVariable method should be in ClangASTContext since there is the need to actually interpret the CompilerDeclContext according to clang rules?

Regarding ParseImportedNamespace, it also deals with "using NS::var" directives. Is there a method similar to FindNamespace that I can use?


================
Comment at: include/lldb/Symbol/Block.h:362-366
@@ -360,1 +361,7 @@
 
+    CompilerDeclContext 
+    GetClangDecl()
+    {
+        return m_clang_decl_context;
+    }
+
----------------
clayborg wrote:
> Why did you add this function? See the function just above it? Why did you not use that one? This should be removed and it should be lazily determined by Block::GetDeclContext(). The current implementation should be all you need.
This function is needed as currently it returns the block of the function, not the block of the variable. See example below:


```
void function() 
{ // block 1 start
    ...
    { // block 2 start
        { // block 3 start
             // if we ask GetDeclContext() it returns the decl context of block 1 instead of block 2
        } // block 3 end
    } // block 2 end
} // block 1 end
```


http://reviews.llvm.org/D12658