[PATCH] D66621: [clang] Devirtualization for classes with destructors marked as 'final'

2019-08-31 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. >> For now, I need help committing this, if anyone would be so kind! rL370597 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66621/new/ https://reviews.llvm.org/D66621 ___

[PATCH] D66621: [clang] Devirtualization for classes with destructors marked as 'final'

2019-08-31 Thread Dávid Bolvanský via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL370597: [clang] Devirtualization for classes with destructors marked as 'final' (authored by xbolva00, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior

[PATCH] D66621: [clang] Devirtualization for classes with destructors marked as 'final'

2019-08-29 Thread Logan Smith via Phabricator via cfe-commits
logan-5 added a comment. //ping// Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66621/new/ https://reviews.llvm.org/D66621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailma

[PATCH] D66621: [clang] Devirtualization for classes with destructors marked as 'final'

2019-08-23 Thread Logan Smith via Phabricator via cfe-commits
logan-5 updated this revision to Diff 217009. logan-5 added a comment. Add a missing null check. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66621/new/ https://reviews.llvm.org/D66621 Files: clang/lib/AST/DeclCXX.cpp clang/test/CodeGenCXX/devirtualize-virt

[PATCH] D66621: [clang] Devirtualization for classes with destructors marked as 'final'

2019-08-23 Thread Hiroshi Yamauchi via Phabricator via cfe-commits
yamauchi added a comment. Nice. In D66621#1642142 , @rsmith wrote: > This seems subtle, but I believe it is correct. > > I wonder whether we should provide a warning for a non-final class has a > final destructor, since moving the `final` from the destru

[PATCH] D66621: [clang] Devirtualization for classes with destructors marked as 'final'

2019-08-22 Thread Logan Smith via Phabricator via cfe-commits
logan-5 marked an inline comment as done. logan-5 added a comment. @rsmith I agree having a final destructor in a non-final class smells weird. I'd be interested in working on adding a warning like that, if it sounds like a useful thing. For now, I need help committing this, if anyone would be

[PATCH] D66621: [clang] Devirtualization for classes with destructors marked as 'final'

2019-08-22 Thread Logan Smith via Phabricator via cfe-commits
logan-5 updated this revision to Diff 216757. logan-5 added a comment. Tweaked comment wording for accuracy. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66621/new/ https://reviews.llvm.org/D66621 Files: clang/lib/AST/DeclCXX.cpp clang/test/CodeGenCXX/devir

[PATCH] D66621: [clang] Devirtualization for classes with destructors marked as 'final'

2019-08-22 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. This seems subtle, but I believe it is correct. I wonder whether we should provide a warning for a non-final class has a final destructor, since moving the `final` from the destructor to the c

[PATCH] D66621: [clang] Devirtualization for classes with destructors marked as 'final'

2019-08-22 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. Aha okay :) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66621/new/ https://reviews.llvm.org/D66621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/ma

[PATCH] D66621: [clang] Devirtualization for classes with destructors marked as 'final'

2019-08-22 Thread Logan Smith via Phabricator via cfe-commits
logan-5 added a comment. That appears sort of tangential to me. To clarify, this PR is not (necessarily) about devirtualizing destructors themselves, but rather devirtualizing OTHER member function calls for classes whose destructor is marked `final`, since that is sort of morally equivalent to

[PATCH] D66621: [clang] Devirtualization for classes with destructors marked as 'final'

2019-08-22 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment. https://reviews.llvm.org/rGcb30590da10ba80a133222588efedab7e5fc9bdd does not help here? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66621/new/ https://reviews.llvm.org/D66621 ___ cfe-comm

[PATCH] D66621: [clang] Devirtualization for classes with destructors marked as 'final'

2019-08-22 Thread Logan Smith via Phabricator via cfe-commits
logan-5 created this revision. logan-5 added a reviewer: rsmith. logan-5 added a project: clang. Herald added subscribers: cfe-commits, Prazek. A class with a destructor marked `final` cannot be derived from, so it should afford the same devirtualization opportunities as marking the entire class