On 1/20/24 05:45, Nathaniel Shead wrote:
On Fri, Jan 19, 2024 at 01:57:18PM -0500, Patrick Palka wrote:
On Wed, 3 Jan 2024, Nathaniel Shead wrote:
+ if (DECL_CONTEXT (decl)
+ && RECORD_OR_UNION_TYPE_P (DECL_CONTEXT (decl))
+ && !DECL_TEMPLATE_INFO (decl))
+ note_static_storage_variable (decl);
It seems this should also handle templated inlines via
&& (!DECL_TEMPLATE_INFO (decl)
|| DECL_IMPLICIT_INSTANTIATION (decl))
otherwise the following fails to link:
$ cat init-5_a.H
template<bool _DecOnly>
struct __from_chars_alnum_to_val_table {
static inline int value = 42;
};
inline unsigned char
__from_chars_alnum_to_val() {
return __from_chars_alnum_to_val_table<false>::value;
}
$ cat init-6_b.C
import "init-5_a.H";
int main() {
__from_chars_alnum_to_val();
}
$ g++ -fmodules-ts -std=c++20 init-5_a.H init-5_b.C
/usr/bin/ld: /tmp/ccNRaads.o: in function `__from_chars_alnum_to_val()':
init-6_b.C:(.text._Z25__from_chars_alnum_to_valv[_Z25__from_chars_alnum_to_valv]+0x6):
undefined reference to `__from_chars_alnum_to_val_table<false>::value'
Ah yes, of course, since that's the other context where declarations are
added to 'pending_statics'.
By the way I ran into this when testing out std::print with modules:
$ cat std.C
export module std;
export import <bits/stdc++.h>;
$ cat hello.C
import std;
int main() {
std::print("Hello {}!", "World");
}
$ g++ -fmodules-ts -std=c++26 -x c++-system-header bits/stdc++.h
$ g++ -fmodules-ts -std=c++26 std.C hello.C && ./a.out # before
/usr/bin/ld: /tmp/ccqNgOM1.o: in function `unsigned char
std::__detail::__from_chars_alnum_to_val<false>(unsigned char)':
hello.C:(.text._ZNSt8__detail25__from_chars_alnum_to_valILb0EEEhh[_ZNSt8__detail25__from_chars_alnum_to_valILb0EEEhh]+0x12):
undefined reference to
`std::__detail::__from_chars_alnum_to_val_table<false>::value'
$ g++ -fmodules-ts -std=c++26 std.C hello.C && ./a.out # after
Hello World!
It's great that this is so close to working!
That's great!
Here's a new version of the patch with the above fixed.
I also included
your change to only add class variable templates to 'pending_statics'
(and the normal 'static_decl's for non-class otherwise) as otherwise I
could imagine that they would cause issues with this later too.
That seems wrong; the 'static_decls' vec is just for checking that
static/inline variables got defined.
pending_statics has been used for template instantiations for a long
time, for non-module code; let's not mess with that in a modules patch.
I know that there's been discussion about the correct ABI for inline
declarations, but personally I'd like to have this fixed for normal uses
in GCC14 at least, and we can revisit the specific cases where various
kinds of declarations are emitted in stage 1.
Makes sense.
P.S. As I go to send this, I wonder if maybe something like
'note_static_member_variable' would be a clearer choice of name than
'note_static_storage_variable'?
Let's call it note_vague_linkage_variable, to go with _fn just above.
-- >8 --
Static data members marked 'inline' should be emitted in TUs where they
are ODR-used. We need to make sure that statics imported from modules
are correctly added to the 'pending_statics' map so that they get
emitted if needed, otherwise the attached testcase fails to link.
What about non-member variables marked inline, and non-member variable
template instantiations?
Jason
