[clang] [Clang](NFC) Add coverage for VTable debug info (PR #151818)
https://github.com/jalopezg-git commented: Thank you! I have added some minor comments / suggestions that add to other reviewer's comments :slightly_smiling_face:. https://github.com/llvm/llvm-project/pull/151818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang](NFC) Add coverage for VTable debug info (PR #151818)
@@ -0,0 +1,109 @@ +// For CInlined (member functions are inlined), we check in case of: +// - The definition of its destructor is visible: +// * The vtable is generated with comdat +// * Its '_vtable$' is generated +// - Otherwise: +// * The vtable is declared +// * Its '_vtable$' is NOT generated +// +// For CNoInline (member functions are defined as non-inline), we check in case of: +// - The definition of its destructor is visible: +// * The vtable is generated +// * Its '_vtable$' is generated +// - Otherwise: +// * The vtable is generated +// * Its '_vtable$' is generated +// +// For CNoFnDef (member functions are declared only), we check in case of: +// * The vtable is NOT generated +// * Its '_vtable$' is generated only if optimized jalopezg-git wrote: Let's improve wording a bit also here, e.g. ```suggestion // For the `CInlined` struct, where all member functions are inlined, we check the following cases: // - If the definition of its destructor is visible: // * The vtable is generated with a COMDAT specifier // * Its '_vtable$' is generated // - Otherwise: // * The vtable is declared // * Its '_vtable$' is NOT generated // // For `CNoInline` struct, where member functions are defined out-of-line, we check the following: // * The vtable is generated // * Its '_vtable$' is generated // // For `CNoFnDef` (member functions are declared only), we check that: // * The vtable is NOT generated // * Its '_vtable$' is generated only if optimizations are enabled ``` https://github.com/llvm/llvm-project/pull/151818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang](NFC) Add coverage for VTable debug info (PR #151818)
@@ -30,23 +28,26 @@ struct CDerived : NSP_1::CBaseOne, NSP_2::CBaseTwo {
int six() override { return 66; }
};
+void use(void *, ...);
jalopezg-git wrote:
(Very minor) nit-pick: add empty line to make it look similar as other test
files (e.g. clang/test/DebugInfo/CXX/vtable-inheritance-diamond.cpp).
```suggestion
void use(void *, ...);
```
https://github.com/llvm/llvm-project/pull/151818
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang](NFC) Add coverage for VTable debug info (PR #151818)
@@ -1,5 +1,3 @@
-// REQUIRES: target={{x86_64.*-linux.*}}
jalopezg-git wrote:
Even if `-emit-llvm` is part of the command line, I think the target triple
affects the `DataLayout` of the emitted LLVM module (unsure as to if that is
important for this PR). IMHO, there should not be a problem to leave it.
https://github.com/llvm/llvm-project/pull/151818
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang](NFC) Add coverage for VTable debug info (PR #151818)
https://github.com/jalopezg-git edited https://github.com/llvm/llvm-project/pull/151818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang](NFC) Add coverage for VTable debug info (PR #151818)
@@ -0,0 +1,96 @@ +// For CTemplate we check in case of: +// - Implicitly instantiate whole class by up-casting: +// * The vtable is generated with comdat +// * Its '_vtable$' is generated +// - Implicitly instantiate member function only: +// # when optimized: +// * The vtable is vanished (no declaration) +// * Its '_vtable$' is generated without associating to vtable +// # when non-optimized: +// * The vtable is generated with comdat only if non-optimized +// * Its '_vtable$' is generated regardless optimized or not +// - Define explicitly instantiation: +// * The vtable is generated with comdat +// * Its '_vtable$' is generated +// - Declare explicitly instantiation as extern: +// * The vtable is declared +// * Its '_vtable$' is generated only if optimized jalopezg-git wrote: Let's improve the wording here a bit (for future readers). ```suggestion // For the `CTemplate` templated class below, check the following cases: // - Implicitly instantiate whole class by up-casting (`NOCAST` not defined): // * The vtable is generated with a COMDAT specifier // * Its '_vtable$' is generated // - Implicitly instantiate member function only (`NOCAST` defined): // # when optimized: // * The vtable has no declaration // * Its '_vtable$' is generated, but not associated to vtable declaration // # when non-optimized: // * The vtable is generated with a COMDAT specifier // * Its '_vtable$' is generated regardless of optimization level // - Define explicitly instantiation (`EXPLICIT` defined): // * The vtable is generated with a COMDAT specifier // * Its '_vtable$' is generated // - Declare explicitly instantiation as `extern` (`EXTERN` defined): // * The vtable is declared // * Its '_vtable$' is generated only if optimized ``` https://github.com/llvm/llvm-project/pull/151818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang](NFC) Add coverage for VTable debug info (PR #151818)
@@ -0,0 +1,96 @@
+// For CTemplate we check in case of:
+// - Implicitly instantiate whole class by up-casting:
+// * The vtable is generated with comdat
+// * Its '_vtable$' is generated
+// - Implicitly instantiate member function only:
+// # when optimized:
+// * The vtable is vanished (no declaration)
+// * Its '_vtable$' is generated without associating to vtable
+// # when non-optimized:
+// * The vtable is generated with comdat only if non-optimized
+// * Its '_vtable$' is generated regardless optimized or not
+// - Define explicitly instantiation:
+// * The vtable is generated with comdat
+// * Its '_vtable$' is generated
+// - Declare explicitly instantiation as extern:
+// * The vtable is declared
+// * Its '_vtable$' is generated only if optimized
+
+struct CBase {
+ virtual void f() noexcept {}
+};
+
+template
+struct CTemplate: CBase {
+ void f() noexcept override;
+ virtual ~CTemplate() noexcept;
+};
+template
+void CTemplate::f() noexcept {}
+template
+CTemplate::~CTemplate() noexcept {}
+
+#ifdef EXPLICIT
+template struct CTemplate;
+#endif
+#ifdef EXTERN
+extern template struct CTemplate;
+#endif
jalopezg-git wrote:
What about just
```suggestion
#if defined(EXPLICIT) || defined(EXTERN)
EXTERN template struct CTemplate;
#endif
```
and specifying `-DEXTERN=extern` on the command line, where needed.
https://github.com/llvm/llvm-project/pull/151818
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang](NFC) Add coverage for VTable debug info (PR #151818)
jalopezg-git wrote: I am sorry for the delay in providing a review for this PR; it's on my TODO list for the next 24h though! https://github.com/llvm/llvm-project/pull/151818 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
