[PATCH] D87808: [DebugInfo] Fix bug in constructor homing where it would use ctor homing when a class only has copy/move constructors

2020-09-24 Thread Amy Huang via Phabricator via cfe-commits
akhuang updated this revision to Diff 294149. akhuang added a comment. Add to comment for lambdas. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87808/new/ https://reviews.llvm.org/D87808 Files: clang/lib/CodeGen/CGDebugInfo.cpp

[PATCH] D87808: [DebugInfo] Fix bug in constructor homing where it would use ctor homing when a class only has copy/move constructors

2020-09-23 Thread David Blaikie via Phabricator via cfe-commits
dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Generally looks good to me! Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2287 + // don't have trivial or constexpr constructors, or can be created from + // aggregate

[PATCH] D87808: [DebugInfo] Fix bug in constructor homing where it would use ctor homing when a class only has copy/move constructors

2020-09-23 Thread Amy Huang via Phabricator via cfe-commits
akhuang updated this revision to Diff 293817. akhuang marked an inline comment as done. akhuang added a comment. Add comments to tests, and add a test for non instantiated trivial ctor and one for lambdas. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D87808: [DebugInfo] Fix bug in constructor homing where it would use ctor homing when a class only has copy/move constructors

2020-09-22 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2292-2300 + bool hasCtor = false; + for (const auto *Ctor : RD->ctors()) { if (Ctor->isTrivial() && !Ctor->isCopyOrMoveConstructor()) return false; +if

[PATCH] D87808: [DebugInfo] Fix bug in constructor homing where it would use ctor homing when a class only has copy/move constructors

2020-09-22 Thread Amy Huang via Phabricator via cfe-commits
akhuang updated this revision to Diff 293579. akhuang added a comment. Update ctor homing check, and add some test cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87808/new/ https://reviews.llvm.org/D87808 Files:

[PATCH] D87808: [DebugInfo] Fix bug in constructor homing where it would use ctor homing when a class only has copy/move constructors

2020-09-22 Thread Amy Huang via Phabricator via cfe-commits
akhuang added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2292-2300 + bool hasCtor = false; + for (const auto *Ctor : RD->ctors()) { if (Ctor->isTrivial() && !Ctor->isCopyOrMoveConstructor()) return false; +if

[PATCH] D87808: [DebugInfo] Fix bug in constructor homing where it would use ctor homing when a class only has copy/move constructors

2020-09-22 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D87808#2288186 , @dblaikie wrote: > In D87808#2286664 , @rsmith wrote: > >> But I think this code should give the same outcome either way, because a >> class with any constructor other

[PATCH] D87808: [DebugInfo] Fix bug in constructor homing where it would use ctor homing when a class only has copy/move constructors

2020-09-22 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D87808#2286664 , @rsmith wrote: > In D87808#2280197 , @dblaikie wrote: > >> @rsmith What's the deal with these anonymous structs/unions? Why do they >> have copy/move constructors (are

[PATCH] D87808: [DebugInfo] Fix bug in constructor homing where it would use ctor homing when a class only has copy/move constructors

2020-09-21 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D87808#2280197 , @dblaikie wrote: > @rsmith What's the deal with these anonymous structs/unions? Why do they have > copy/move constructors (are those technically called from the enclosing > class's copy/move constructors?) but

[PATCH] D87808: [DebugInfo] Fix bug in constructor homing where it would use ctor homing when a class only has copy/move constructors

2020-09-21 Thread Amy Huang via Phabricator via cfe-commits
akhuang added a comment. In D87808#2282223 , @rnk wrote: > In D87808#2280197 , @dblaikie wrote: > >> @rsmith What's the deal with these anonymous structs/unions? Why do they >> have copy/move constructors (are

[PATCH] D87808: [DebugInfo] Fix bug in constructor homing where it would use ctor homing when a class only has copy/move constructors

2020-09-18 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. In D87808#2280197 , @dblaikie wrote: > @rsmith What's the deal with these anonymous structs/unions? Why do they have > copy/move constructors (are those technically called from the enclosing > class's copy/move constructors?) but no

[PATCH] D87808: [DebugInfo] Fix bug in constructor homing where it would use ctor homing when a class only has copy/move constructors

2020-09-17 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a subscriber: rsmith. dblaikie added a comment. @rsmith What's the deal with these anonymous structs/unions? Why do they have copy/move constructors (are those technically called from the enclosing class's copy/move constructors?) but no default constructor to be called from the

[PATCH] D87808: [DebugInfo] Fix bug in constructor homing where it would use ctor homing when a class only has copy/move constructors

2020-09-16 Thread Amy Huang via Phabricator via cfe-commits
akhuang created this revision. akhuang added a reviewer: dblaikie. Herald added a project: clang. Herald added a subscriber: cfe-commits. akhuang requested review of this revision. Basically, the code was checking if there were no constructors at all, but didn't check that there were any non