| Message ID | 20231025183245.837312-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++/modules: fix up recent testcases | expand |
Patrick, thanks for noticing this, and this is a suitable workaround for another bug. We should either be emitting the definition of that member function in the object file of its containing function. Or it should be implicitly inline. I know in module perview the in-class defined member functions at namespace scope are /not/ implicitly inline. But I can't recall what the std says about non-namespace scope classes. Ah, it appears to be the former we should be doing: [class.mfct] If a member function is attached to the global module and is defined (9.5) in its class definition, it is inline (9.2.8). Notice we can get into the weird situation that the member functions of a local class of a module-purview inline function might not themselves be inline! On 10/25/23 14:32, Patrick Palka wrote: > Tested on x86_64-pc-linux-gnu, does this look OK for trunk? > > Declaring get() inline seems necessary to avoid link failure: > > /usr/bin/ld: /tmp/ccwdv6Co.o: in function `g3@pr105322.Decltype()': > decltype-1_b.C:(.text._ZW8pr105322W8Decltype2g3v[_ZW8pr105322W8Decltype2g3v]+0x18): undefined reference to `f@pr105322.Decltype()::A::get()' > > Not sure if that's expected? > > -- >8 -- > > This fixes some minor issues with the testcases from > r14-4806-g084addf8a700fa. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/decltype-1_a.C: Add missing } to dg-module-do > directive. Declare f()::A::get() inline. > * g++.dg/modules/lambda-5_a.C: Add missing } to dg-module-do > directive. > --- > gcc/testsuite/g++.dg/modules/decltype-1_a.C | 4 ++-- > gcc/testsuite/g++.dg/modules/lambda-5_a.C | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/gcc/testsuite/g++.dg/modules/decltype-1_a.C b/gcc/testsuite/g++.dg/modules/decltype-1_a.C > index ca66e8b598a..6512f151aae 100644 > --- a/gcc/testsuite/g++.dg/modules/decltype-1_a.C > +++ b/gcc/testsuite/g++.dg/modules/decltype-1_a.C > @@ -1,5 +1,5 @@ > // PR c++/105322 > -// { dg-module-do link > +// { dg-module-do link } > // { dg-additional-options -fmodules-ts } > // { dg-module-cmi pr105322.Decltype } > > @@ -7,7 +7,7 @@ export module pr105322.Decltype; > > auto f() { > struct A { int m; > - int get () { return m; } > + inline int get () { return m; } > }; > return A{}; > } > diff --git a/gcc/testsuite/g++.dg/modules/lambda-5_a.C b/gcc/testsuite/g++.dg/modules/lambda-5_a.C > index 6b589d4965c..37d0e77b1e1 100644 > --- a/gcc/testsuite/g++.dg/modules/lambda-5_a.C > +++ b/gcc/testsuite/g++.dg/modules/lambda-5_a.C > @@ -1,5 +1,5 @@ > // PR c++/105322 > -// { dg-module-do link > +// { dg-module-do link } > // { dg-additional-options -fmodules-ts } > // { dg-module-cmi pr105322.Lambda } >
On 10/25/23 14:32, Patrick Palka wrote: > Tested on x86_64-pc-linux-gnu, does this look OK for trunk? > > Declaring get() inline seems necessary to avoid link failure: > > /usr/bin/ld: /tmp/ccwdv6Co.o: in function `g3@pr105322.Decltype()': > decltype-1_b.C:(.text._ZW8pr105322W8Decltype2g3v[_ZW8pr105322W8Decltype2g3v]+0x18): undefined reference to `f@pr105322.Decltype()::A::get()' > > Not sure if that's expected? That seems like a bug. My guess is that the bug is treating f()::A::get as internal linkage, but Nathan should have a better understanding of how it's supposed to work. > -- >8 -- > > This fixes some minor issues with the testcases from > r14-4806-g084addf8a700fa. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/decltype-1_a.C: Add missing } to dg-module-do > directive. Declare f()::A::get() inline. > * g++.dg/modules/lambda-5_a.C: Add missing } to dg-module-do > directive. > --- > gcc/testsuite/g++.dg/modules/decltype-1_a.C | 4 ++-- > gcc/testsuite/g++.dg/modules/lambda-5_a.C | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/gcc/testsuite/g++.dg/modules/decltype-1_a.C b/gcc/testsuite/g++.dg/modules/decltype-1_a.C > index ca66e8b598a..6512f151aae 100644 > --- a/gcc/testsuite/g++.dg/modules/decltype-1_a.C > +++ b/gcc/testsuite/g++.dg/modules/decltype-1_a.C > @@ -1,5 +1,5 @@ > // PR c++/105322 > -// { dg-module-do link > +// { dg-module-do link } > // { dg-additional-options -fmodules-ts } > // { dg-module-cmi pr105322.Decltype } > > @@ -7,7 +7,7 @@ export module pr105322.Decltype; > > auto f() { > struct A { int m; > - int get () { return m; } > + inline int get () { return m; } > }; > return A{}; > } > diff --git a/gcc/testsuite/g++.dg/modules/lambda-5_a.C b/gcc/testsuite/g++.dg/modules/lambda-5_a.C > index 6b589d4965c..37d0e77b1e1 100644 > --- a/gcc/testsuite/g++.dg/modules/lambda-5_a.C > +++ b/gcc/testsuite/g++.dg/modules/lambda-5_a.C > @@ -1,5 +1,5 @@ > // PR c++/105322 > -// { dg-module-do link > +// { dg-module-do link } > // { dg-additional-options -fmodules-ts } > // { dg-module-cmi pr105322.Lambda } >
diff --git a/gcc/testsuite/g++.dg/modules/decltype-1_a.C b/gcc/testsuite/g++.dg/modules/decltype-1_a.C index ca66e8b598a..6512f151aae 100644 --- a/gcc/testsuite/g++.dg/modules/decltype-1_a.C +++ b/gcc/testsuite/g++.dg/modules/decltype-1_a.C @@ -1,5 +1,5 @@ // PR c++/105322 -// { dg-module-do link +// { dg-module-do link } // { dg-additional-options -fmodules-ts } // { dg-module-cmi pr105322.Decltype } @@ -7,7 +7,7 @@ export module pr105322.Decltype; auto f() { struct A { int m; - int get () { return m; } + inline int get () { return m; } }; return A{}; } diff --git a/gcc/testsuite/g++.dg/modules/lambda-5_a.C b/gcc/testsuite/g++.dg/modules/lambda-5_a.C index 6b589d4965c..37d0e77b1e1 100644 --- a/gcc/testsuite/g++.dg/modules/lambda-5_a.C +++ b/gcc/testsuite/g++.dg/modules/lambda-5_a.C @@ -1,5 +1,5 @@ // PR c++/105322 -// { dg-module-do link +// { dg-module-do link } // { dg-additional-options -fmodules-ts } // { dg-module-cmi pr105322.Lambda }