aaron.ballman added inline comments.
================ Comment at: clang/lib/Sema/SemaDeclCXX.cpp:6007 + // non-abstract types. + if (!FD->hasSkippedBody()) return; ---------------- This doesn't seem quite right -- we skip bodies for various reasons: https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Parse/Parser.h#L413 Which test case needed this change? ================ Comment at: clang/test/CXX/class.derived/class.abstract/p3.cpp:38-41 +void f(A){}; // expected-error {{abstract class}} +void f(A[1]){}; // expected-error {{abstract class}} +void f(B){}; // expected-error {{abstract class}} +void f(B[1]){}; // expected-error {{abstract class}} ---------------- Whole lotta trailing semi colons that should be removed (you should get all of them, not just theses). ================ Comment at: clang/test/CXX/class.derived/class.abstract/p3.cpp:77-79 // FIXME: These should be handled consistently. We currently reject the first // two early because we (probably incorrectly, depending on dr1640) take // abstractness into account in forming implicit conversion sequences. ---------------- FIXME can be removed now? Do we need a DR test case for this? ================ Comment at: clang/test/CXX/drs/dr6xx.cpp:707-708 // FIXME: The following examples demonstrate that we might be accepting the // above cases for the wrong reason. ---------------- Is this fixme still accurate? Should we be changing the resolution from partial to full? ================ Comment at: clang/test/SemaCXX/abstract.cpp:369 +foo g(); +} ---------------- A mildly horrifying test case to consider: ``` struct S { virtual void func() = 0; }; void S::func() {} static_assert(__is_abstract(S)); struct T { void func(S) = delete; void other(S); void yet_another(S) {} }; ``` We currently don't do good things... https://godbolt.org/z/7vEY3Kz9c Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152096/new/ https://reviews.llvm.org/D152096 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits