[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

labath added a comment. That is interesting. Can you send me the binary produced by this test? I'd like to compare it with what I get locally. (I can also send you mine if you're interested) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

teemperor added a comment. Whoops, I meant skipIf'ing, not x-failing. Anyway, I can reproduce the SIBABRT fail on my Arch Linux CI, so it seems that Stella's ubuntu failure isn't at fault here but instead some other issue. If I disable unwind-on-error I get this: lldb version 10.0.0

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

labath added a comment. In D68130#1715433 , @teemperor wrote: > Well, I'm fine with x-failing it on Linux, even though I guess at some point > someone (i.e., probably me) has to figure out why this stuff is broken in the > expression parser. I said

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

teemperor added a comment. Well, I'm fine with x-failing it on Linux, even though I guess at some point someone (i.e., probably me) has to figure out why this stuff is broken in the expression parser. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

labath added a comment. It doesn't seem to be failing here http://lab.llvm.org:8014/builders/lldb-x86_64-debian. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68130/new/ https://reviews.llvm.org/D68130

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

teemperor added a comment. Sorry, seems like I forgot to delete these tests when I extracted them into their own test function. About the ubuntu failure: I think I have a similar failure on my Arch Linux machine when constructing an object in another test, so I fear that the whole object

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

stella.stamenova added a comment. Oh, and TestCallOverridenMethod still has some failures on Windows: http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/9996/steps/test/logs/stdio Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

stella.stamenova added a comment. In D68130#1713620 , @teemperor wrote: > Yeah, seems like constructing objects in expressions isn't implemented on > Windows. I'm not sure if there is a reliable way to test that constructors > aren't shadowed by these

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

teemperor added a comment. Yeah, seems like constructing objects in expressions isn't implemented on Windows. I'm not sure if there is a reliable way to test that constructors aren't shadowed by these global functions if constructors themselves don't work on Windows, but I filed

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

stella.stamenova added a comment. Unfortunately, this is failing on the windows bot: http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/9961 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68130/new/ https://reviews.llvm.org/D68130

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

This revision was automatically updated to reflect the committed changes. Closed by commit rG6237c9fe6ce9: [lldb] Dont emit artificial constructor declarations as global functions (authored by teemperor). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

teemperor added a comment. So, are there any concerns about this (beside the typo which I'll fix pre-commit) and Shafik's request for documentation (which will be another NFC commit)? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68130/new/ https://reviews.llvm.org/D68130

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

labath added a comment. In D68130#1686184 , @vsk wrote: > Great find. The changes in this patch makes sense to me locally, but I'm > having trouble picking up on the context necessary to confidently 'lgtm'. + > @JDevlieghere & @labath to get some more

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

shafik added a comment. In D68130#1686549 , @jingham wrote: > > IIRC, it is because we can't count on which artificial functions get > generated in the debug information in any given CU. It depends on what gets > used. If we write artificial

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

jingham added a comment. In D68130#1686530 , @shafik wrote: > So if I look at `ClangASTContext::AddMethodToCXXRecordType(...)` it has the > following: > > if (is_artificial) > return nullptr; // skip everything artificial > > > but why? if I

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

shafik added a comment. So if I look at `ClangASTContext::AddMethodToCXXRecordType(...)` it has the following: if (is_artificial) return nullptr; // skip everything artificial but why? if I look at the godbolt w/ an AST dump it is not creating as a global

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

vsk added a subscriber: labath. vsk added a comment. Great find. The changes in this patch makes sense to me locally, but I'm having trouble picking up on the context necessary to confidently 'lgtm'. + @JDevlieghere & @labath to get some more experienced people. I'd love to see the big switch

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

teemperor updated this revision to Diff 222162. teemperor added a comment. - Document dedicated test a bit better. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68130/new/ https://reviews.llvm.org/D68130 Files:

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

teemperor updated this revision to Diff 222140. teemperor edited the summary of this revision. teemperor added a comment. - Enabled test on Linux. That's only failing on my box (Thanks Pavel) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68130/new/ https://reviews.llvm.org/D68130

[Lldb-commits] [PATCH] D68130: [lldb] Don't emit artificial constructor declarations as global functions

teemperor created this revision. teemperor added reviewers: aprantl, vsk. Herald added subscribers: lldb-commits, JDevlieghere. Herald added a project: LLDB. When we have a artificial constructor DIE, we currently create from that a global function with the name of that class. That ends up