[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGb706f56133a7: [lldb] Automatically unwrap parameter packs in template argument accessors (authored by JDevlieghere).

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

JDevlieghere marked 8 inline comments as done. JDevlieghere added a comment. Thanks for the review Michael. I've addressed all your comments in the version I landed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51387/new/ https://reviews.llvm.org/D51387

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

Michael137 accepted this revision. Michael137 added a comment. LGTM Just had some minor stylistic/documentation suggestions Comment at: lldb/source/API/SBType.cpp:576 lldb::TemplateArgumentKind SBType::GetTemplateArgumentKind(uint32_t idx) { LLDB_INSTRUMENT_VA(this,

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

JDevlieghere added a comment. In D51387#1387074 , @clayborg wrote: > Do we have any callers that call GetNumTemplateArguments with false? If not, > remove the argument? Yup, there's a bunch of uses in `Language/CPlusPlus/LibCxx*.cpp` CHANGES SINCE

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

JDevlieghere updated this revision to Diff 451714. JDevlieghere added a comment. - Rebase - Address Jim's code review feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51387/new/ https://reviews.llvm.org/D51387 Files: lldb/include/lldb/Symbol/CompilerType.h

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

JDevlieghere commandeered this revision. JDevlieghere added a reviewer: friss. JDevlieghere added a comment. Herald added a reviewer: shafik. Herald added a project: All. Commandeering this from Fred CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51387/new/

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

clayborg added a comment. Do we have any callers that call GetNumTemplateArguments with false? If not, remove the argument? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51387/new/ https://reviews.llvm.org/D51387 ___ lldb-commits mailing

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. I'm a little worried that we have asserts with no backstops here. We ship in general with asserts off, so if some type is not copasetic we will probably crash shortly afterwards

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

friss added a comment. @jingham ping https://reviews.llvm.org/D51387 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good to me. Jim should take a look as well. https://reviews.llvm.org/D51387 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

friss updated this revision to Diff 163399. friss added a comment. Address feedback https://reviews.llvm.org/D51387 Files: include/lldb/Symbol/ClangASTContext.h include/lldb/Symbol/CompilerType.h include/lldb/Symbol/TypeSystem.h

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

friss added inline comments. Comment at: source/Symbol/ClangASTContext.cpp:7664 + + assert(args.size() && + "We shouldn't have a template specialization without any args"); friss wrote: > shafik wrote: > > The asset before we do math making this

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

friss marked 2 inline comments as done. friss added inline comments. Comment at: include/lldb/Symbol/ClangASTContext.h:284 + ((bool)pack_name == (bool)packed_args) && + (!packed_args || !packed_args->args.empty()); } friss wrote: >

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

friss added inline comments. Comment at: include/lldb/Symbol/ClangASTContext.h:284 + ((bool)pack_name == (bool)packed_args) && + (!packed_args || !packed_args->args.empty()); } shafik wrote: > Is this saying that an empty parameter

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

shafik added inline comments. Comment at: include/lldb/Symbol/ClangASTContext.h:284 + ((bool)pack_name == (bool)packed_args) && + (!packed_args || !packed_args->args.empty()); } Is this saying that an empty parameter pack is invalid?

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

clayborg added a comment. Looks fine. A bit of cleanup might be nice on the TypeSystem.h to provide default implementations that just return 0 for some of these that every type system currently is required to override for no reason (all template related type system calls seem to have default

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

friss created this revision. friss added reviewers: jingham, clayborg. Herald added a subscriber: JDevlieghere. When looking at template arguments in LLDB, we usually care about what the user passed in his code, not whether some of those arguments where passed as a variadic parameter pack. This