[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

This revision was automatically updated to reflect the committed changes. Closed by commit rG902974277d50: Data formatters: Look through array element typedefs (authored by jarin, committed by teemperor). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

teemperor added a comment. Actually the failures are gmodules only so I'll land this now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72133/new/ https://reviews.llvm.org/D72133 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

teemperor added a comment. In D72133#1813724 , @jarin wrote: > Raphael, could you possibly land the patch for me? Thanks! It's in my queue, but the green dragon bot (and my own CI) are currently dead due to a libc++ change that breaks the data

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

jarin added a comment. Raphael, could you possibly land the patch for me? Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72133/new/ https://reviews.llvm.org/D72133 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Ok thanks for clarifying all my points! Looks good. And it would be nice to fix any locations that were canonicalizing types when they shouldn't be in a future patch. CHANGES SINCE LAST

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

jarin added a comment. In D72133#1806822 , @clayborg wrote: > So as long as the following are true from this patch I am ok: > > - if I ask for the array element type of "str" in the test that was added, it > should return "MCHAR". We shouldn't be

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

teemperor added a comment. In D72133#1806822 , @clayborg wrote: > So as long as the following are true from this patch I am ok: > > - if I ask for the array element type of "str" in the test that was added, it > should return "MCHAR". We shouldn't be

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

jarin marked 3 inline comments as done. jarin added inline comments. Comment at: lldb/source/API/SBType.cpp:215-218 + CompilerType canonical_type = + m_opaque_sp->GetCompilerType(true).GetCanonicalType(); + return LLDB_RECORD_RESULT( + SBType(TypeImplSP(new

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

clayborg added a comment. So as long as the following are true from this patch I am ok: - if I ask for the array element type of "str" in the test that was added, it should return "MCHAR". We shouldn't be removing any typedefs from the type. The user can call SBType::GetCanonicalType() if they

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: lldb/source/API/SBType.cpp:215-218 + CompilerType canonical_type = + m_opaque_sp->GetCompilerType(true).GetCanonicalType(); + return

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

jarin added a comment. Actually, there is something unusually flaky in my current checkout, so you might be right that not canonicalizing is harmless. It still makes more sense to land that separately. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72133/new/

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

teemperor added a comment. Interesting, I actually can't reproduce those failures on any of my machines. Can you post the error output you get? But I agree that we can figure this out in a separate patch, so I'll give this another test run and then land it. Thanks for the patch! CHANGES

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

jarin marked an inline comment as not done. jarin added a comment. In D72133#1802727 , @teemperor wrote: > Also do you need someone to commit this for you? Yes, please (once we agree what to do with SBType.cpp). CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

jarin marked 7 inline comments as done. jarin added a comment. I have addressed the comments, thanks for the quick review. Comment at: lldb/source/API/SBType.cpp:218 + return LLDB_RECORD_RESULT( + SBType(TypeImplSP(new TypeImpl(canonical_type.GetArrayElementType();

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

teemperor added a comment. Just FYI, I meant that `lldb/source/API/SBType.cpp` shouldn't be changed with this patch. I think it's fine that this patch will change the behavior (as the old behavior seems broken). Also do you need someone to commit this for you? CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

jarin updated this revision to Diff 236021. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72133/new/ https://reviews.llvm.org/D72133 Files: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/array_typedef/Makefile

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. I have some comments but otherwise this patch looks good to me. Thanks! Comment at:

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

jarin created this revision. jarin added a project: LLDB. Herald added a subscriber: lldb-commits. Motivation: When formatting an array of typedefed chars, we would like to display the array as a string. The string formatter currently does not trigger because the formatter lookup does not