[Lldb-commits] [lldb] r367726 - Fix ClangASTContext::CreateParameterDeclaration to not call addDecl

[Davide Italiano via lldb-commits]

On Fri, Aug 2, 2019 at 2:40 PM Shafik Yaghmour via lldb-commits
<lldb-commits at lists.llvm.org> wrote:
>
> Author: shafik
> Date: Fri Aug  2 14:41:50 2019
> New Revision: 367726
>
> URL: http://llvm.org/viewvc/llvm-project?rev=367726&view=rev
> Log:
> Fix ClangASTContext::CreateParameterDeclaration to not call addDecl
>
> Summary:
> The change https://reviews.llvm.org/D55575 modified ClangASTContext::CreateParameterDeclaration to call decl_ctx->addDecl(decl); this caused a regression since the existing code in DWARFASTParserClang::ParseChildParameters is called with the containing DeclContext. So when end up with cases where we are parsing a parameter for a member function and the parameter is added to the CXXRecordDecl as opposed to the CXXMethodDecl. This example is given in the regression test TestBreakpointInMemberFuncWNonPrimitiveParams.py which without this fix in a modules build leads to assert on setting a breakpoint in a member function with non primitive parameters. This scenario would be common when debugging LLDB or clang.
>
> Differential Revision: https://reviews.llvm.org/D65414
>
> Added:
>     lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/breakpoint_in_member_func_w_non_primitive_params/
>     lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/breakpoint_in_member_func_w_non_primitive_params/Makefile
>     lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/breakpoint_in_member_func_w_non_primitive_params/TestBreakpointInMemberFuncWNonPrimitiveParams.py
>     lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/breakpoint_in_member_func_w_non_primitive_params/a.cpp
>     lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/breakpoint_in_member_func_w_non_primitive_params/a.h
>     lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/breakpoint_in_member_func_w_non_primitive_params/main.cpp
>     lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/breakpoint_in_member_func_w_non_primitive_params/module.modulemap
> Modified:
>     lldb/trunk/include/lldb/Symbol/ClangASTContext.h
>     lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
>     lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
>     lldb/trunk/source/Symbol/ClangASTContext.cpp
>
> Modified: lldb/trunk/include/lldb/Symbol/ClangASTContext.h
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTContext.h?rev=367726&r1=367725&r2=367726&view=diff
> ==============================================================================
> --- lldb/trunk/include/lldb/Symbol/ClangASTContext.h (original)
> +++ lldb/trunk/include/lldb/Symbol/ClangASTContext.h Fri Aug  2 14:41:50 2019
> @@ -395,7 +395,8 @@ public:
>    clang::ParmVarDecl *CreateParameterDeclaration(clang::DeclContext *decl_ctx,
>                                                   const char *name,
>                                                   const CompilerType &param_type,
> -                                                 int storage);
> +                                                 int storage,
> +                                                 bool add_decl=false);
>

Sorry for arriving late to the party, but what's the reason for this
parameter? Maybe we should add a comment, given that the commit message
doesn't really explain why the PDB parser skips adding the declaration either?

Thanks,

--
Davide