[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

labath added a comment. In e3645fdff , that is (the first has is a local commit I forgot to push). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88483/new/

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

labath added a comment. The `num_compile_units==3` assumption is also fragile, as files like crtbegin/end, etc. can come with debug info too (ran into that downstream). Changed the assertion to `>=3` in fd06f6441. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum added a comment. Refactored test according to recommendations given during review 1267bb2e416e Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88483/new/

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

labath added a comment. It looks like the test has found an actual bug in handling of the dwo format. 8485ee781f fixes that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

JDevlieghere added a comment. It was still failing so I've temporarily XFAIL'ed it in 30e7df0d58542ad35d517eace70a4cea40e6fa7a Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

JDevlieghere added a comment. In D88483#2363529 , @fallkrum wrote: > make types extern. > @jingham please commit this patch to check if it will fix DWO issue, the > rest of your recommendations will add later. Jim asked me to follow up on this so I've

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG56282cf7e271: [lldb] Update TestTypeGetModule.py (authored by fallkrum, committed by JDevlieghere). Changed prior to commit:

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum updated this revision to Diff 301796. fallkrum added a comment. make types extern. @jingham please commit this patch to check if it will fix DWO issue, the rest of your recommendations will add later. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

jingham added a comment. The test failed in the new form at line 77. So we got the CU for compile_unit1.c and it was valid. But it didn't contain an SBType for compile_unit1_type. Probably the same thing is true in compile_unit2.c you just didn't get there. Maybe your executable is too

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

jingham added a comment. Sorry to be picky, but you can compress this by putting the IsValid checks into your find_* routines. And the: self.assertEqual(cu_type.GetName(), type2_name) calls aren't necessary, since you already found the type by matching the name. So I think you can

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum added a comment. In D88483#2363277 , @jingham wrote: > This test is still assuming (1) that main.c is CompUnit(0), etc. You don't > know that's true. And that there is only one type in each CU (and type 0 is > the one you want). Neither of

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum updated this revision to Diff 301783. fallkrum added a comment. Api test fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88483/new/ https://reviews.llvm.org/D88483 Files:

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

jingham added a comment. This test is still assuming (1) that main.c is CompUnit(0), etc. You don't know that's true. And that there is only one type in each CU (and type 0 is the one you want). Neither of these is guaranteed. Can you check all these things, I'd rather not have to go a

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum added a comment. In D88483#2363075 , @fallkrum wrote: > @JDevlieghere @jingham api test failed after my commit: > http://lab.llvm.org:8011/#/builders/68/builds/1040 > Had no such an error on my computer, please help to figure out what went >

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum updated this revision to Diff 301768. fallkrum added a comment. Api test improvements. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88483/new/ https://reviews.llvm.org/D88483 Files:

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum added a comment. Could you explain how to commit that Differential revision is closed by commit? Commited like described in here https://llvm.org/docs/Phabricator.html#committing-someone-s-change-from-phabricator git pull --rebase https://github.com/llvm/llvm-project.git master

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

jingham added a comment. In D88483#2363075 , @fallkrum wrote: > @JDevlieghere @jingham api test failed after my commit: > http://lab.llvm.org:8011/#/builders/68/builds/1040 > Had no such an error on my computer, please help to figure out what went >

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum added a comment. @JDevlieghere @jingham api test failed after my commit: http://lab.llvm.org:8011/#/builders/68/builds/1040 Had no such an error on my computer, please help to figure out what went wrong. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum added a comment. Please commit it for me. Ilya Bukonkin fallk...@yahoo.com Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88483/new/ https://reviews.llvm.org/D88483 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum updated this revision to Diff 301271. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88483/new/ https://reviews.llvm.org/D88483 Files: lldb/bindings/interface/SBType.i lldb/include/lldb/API/SBModule.h lldb/include/lldb/API/SBType.h

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land. LGTM with the few nits addressed. Comment at: lldb/include/lldb/Symbol/Type.h:269 + lldb::ModuleSP GetModule(); + Comment

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum added a comment. @JDevlieghere ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88483/new/ https://reviews.llvm.org/D88483 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum added a comment. In D88483#2343694 , @labath wrote: > > It sounds like that could be an artefact from how we parse function types > from dwarf. I haven't checked if that's what happens in this particular case, > but I know that we parse

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

labath added a comment. It sounds like that could be an artefact from how we parse function types from dwarf. I haven't checked if that's what happens in this particular case, but I know that we parse function types by just taking the function DIE (which describes the whole function, not just

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

jingham added a comment. In D88483#2341538 , @fallkrum wrote: > Thanks a lot Jim for explanations, now it makes sense to me. > Have one more question on class Type. Writing API test for the patch defined > 2 functions like this: > > void *func1(int) {

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum added a comment. Thanks a lot Jim for explanations, now it makes sense to me. Have one more question on class Type. Writing API test for the patch defined 2 functions like this: void *func1(int) { return NULL; } void *func2(int) { return NULL; } Tried to

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum added a comment. In D88483#2339256 , @jingham wrote: > In D88483#2338408 , @fallkrum wrote: > >> Can you please give me a couple of words on what m_static_type and >> m_dynamic_type is in TypeImpl class,

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

jingham added a comment. You can make up types in the expression parser. So for instance, I can do: In D88483#2338408 , @fallkrum wrote: > Can you please give me a couple of words on what m_static_type and > m_dynamic_type is in TypeImpl class, in what

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum added a comment. Can you please give me a couple of words on what m_static_type and m_dynamic_type is in TypeImpl class, in what cases can it be initialized with both of those types? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum marked 2 inline comments as done. fallkrum added a comment. Did not noticed it must be 3 slashes instead of 2 :) > Also, please make sure that your patch is well formatted (git clang-format > HEAD~) before submitting it. git-clang-format HEAD~ says the patch is well formatted.

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

mib accepted this revision. mib added a comment. LGTM provided that you address my last comments :) Also, please make sure that your patch is well formatted (`git clang-format HEAD~`) before submitting it. Thanks. Comment at: lldb/include/lldb/Symbol/Type.h:111-114 //

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum updated this revision to Diff 298596. fallkrum set the repository for this revision to rG LLVM Github Monorepo. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88483/new/

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

mib requested changes to this revision. mib added a comment. This revision now requires changes to proceed. Could please follow PEP8 style guide regarding variable names : Function names should be lowercase, with words

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum updated this revision to Diff 298445. fallkrum added a comment. API test added. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88483/new/ https://reviews.llvm.org/D88483 Files: lldb/bindings/interface/SBType.i lldb/include/lldb/API/SBModule.h

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum updated this revision to Diff 298184. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88483/new/ https://reviews.llvm.org/D88483 Files: lldb/bindings/interface/SBType.i lldb/include/lldb/API/SBModule.h lldb/include/lldb/API/SBType.h

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

JDevlieghere requested changes to this revision. JDevlieghere added inline comments. This revision now requires changes to proceed. Comment at: lldb/source/API/SBType.cpp:504 +if (!IsValid()) +return sb_module; + mib wrote: > It would be nice to

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88483/new/ https://reviews.llvm.org/D88483 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum added a comment. In D88483#2300716 , @mib wrote: > Spotted some little things here and there ... This patch lacks a test and > would need some reformatting (`git clang-format HEAD~`) but other than that, > looks ok to me. Can I format and add

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

mib added a comment. Spotted some little things here and there ... This patch lacks a test and would need some reformatting (`git clang-format HEAD~`) but other than that, looks ok to me. Comment at: lldb/include/lldb/Symbol/Type.h:118 +/* + GetModule may return

[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

fallkrum created this revision. fallkrum added reviewers: jingham, LLDB. fallkrum added a project: LLDB. Herald added subscribers: lldb-commits, JDevlieghere. Herald added a reviewer: JDevlieghere. fallkrum requested review of this revision. Repository: rG LLVM Github Monorepo