[PATCH] Add missing explicit instantiation for std::lower_bound template

[PATCH] Add missing explicit instantiation for std::lower_bound template

[Dodji Seketeli]

Hello,

Since commit r195676[1], it looks like
libstdc++-v3/src/c++11/hashtable_c++0x.cc is missing an explicit
instantiation for std::lower_bound.  This leads to libstdc++.so having
the symbol for that (missing) instantiation be undefined, thus
preventing executables from being linked with libstdc++.

The patchlet below seems to fixed the issue for me.

Boostrapped and tested on x86_64-unknown-linux-gnu.

[1]:

    commit bc36b44c7cb0e5e97ac807b8fb17507e0fb09008
    Author: fdumont <fdumont@138bc75d-0d04-0410-961f-82ee72b054a4>
    Date:   Fri Feb 1 20:44:41 2013 +0000

	2013-02-01  François Dumont  <fdumont@gcc.gnu.org>

	    * include/bits/hashtable_policy.h
	    (_Prime_rehash_policy::_M_next_bkt)
	    (_Prime_rehash_policy::_M_need_rehash): Move definition...
	    * src/c++11/hashtable_c++0x.cc: ... here.
	    * src/shared/hashtable-aux.cc: Remove c++config.h include.
	    * config/abi/gnu.ver (GLIBCXX_3.4.18): Export _Prime_rehash_policy
	    symbols.

	git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@195676 138bc75d-0d04-0410-961f-82ee72b054a4

libstdc++-v3/ChangeLog

	* libstdc++-v3/src/c++11/hashtable_c++0x.cc (namespace std): Add
	missing instantiation for std::lower_bound template.
---
 libstdc++-v3/ChangeLog                    | 6 ++++++
 libstdc++-v3/src/c++11/hashtable_c++0x.cc | 7 +++++++
 2 files changed, 13 insertions(+)

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 1a8a822..8ae9343 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,3 +1,9 @@
+2013-02-04  Dodji Seketeli  <dodji@redhat.com>
+
+	Add missing explicit instantiation for std::lower_bound template
+	* libstdc++-v3/src/c++11/hashtable_c++0x.cc (namespace std): Add
+	missing instantiation for std::lower_bound template.
+
 2013-02-03  Richard Sandiford  <rdsandiford@googlemail.com>
 
 	Update copyright years.
diff --git a/libstdc++-v3/src/c++11/hashtable_c++0x.cc b/libstdc++-v3/src/c++11/hashtable_c++0x.cc
index 7617c58..942d119 100644
--- a/libstdc++-v3/src/c++11/hashtable_c++0x.cc
+++ b/libstdc++-v3/src/c++11/hashtable_c++0x.cc
@@ -94,4 +94,11 @@ namespace __detail
 
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace __detail
+
+ // Instantiations.
+ template unsigned long const*
+ lower_bound<unsigned long const *,
+	     unsigned long>(unsigned long const*,
+			    unsigned long const*,
+			    const unsigned long &);
 } // namespace std
-- 
		Dodji

Re: [PATCH] Add missing explicit instantiation for std::lower_bound template

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 885 bytes --]

Hi Dodji,

On 02/04/2013 04:53 PM, Dodji Seketeli wrote:
> Hello,
>
> Since commit r195676[1], it looks like
> libstdc++-v3/src/c++11/hashtable_c++0x.cc is missing an explicit
> instantiation for std::lower_bound.  This leads to libstdc++.so having
> the symbol for that (missing) instantiation be undefined, thus
> preventing executables from being linked with libstdc++.
Note that I can confirm this only if I build with less optimization than 
the default -O2, say -O. That may explain why nobody noticed earlier.
> The patchlet below seems to fixed the issue for me.

Indeed, I think we want something like that. Thanks. In terms of nits, 
formatting and types (who knows, maybe on some systems a std::size_t is 
an unsigned long long), I would write something like the attached. 
Please double check that it works for you and go ahead.

Thanks again,
Paolo.

//////////////////



[-- Attachment #2: p --]
[-- Type: text/plain, Size: 474 bytes --]

Index: hashtable_c++0x.cc
===================================================================
--- hashtable_c++0x.cc	(revision 195759)
+++ hashtable_c++0x.cc	(working copy)
@@ -94,4 +94,11 @@ namespace __detail
 
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace __detail
+
+  // Instantiation.
+  template
+  const unsigned long*
+  lower_bound<const unsigned long*, size_t>(const unsigned long*,
+					    const unsigned long*,
+					    const size_t&);
 } // namespace std

Re: [PATCH] Add missing explicit instantiation for std::lower_bound template

[Dodji Seketeli]

Paolo Carlini <paolo.carlini@oracle.com> writes:

>> Since commit r195676[1], it looks like
>> libstdc++-v3/src/c++11/hashtable_c++0x.cc is missing an explicit
>> instantiation for std::lower_bound.  This leads to libstdc++.so having
>> the symbol for that (missing) instantiation be undefined, thus
>> preventing executables from being linked with libstdc++.
> Note that I can confirm this only if I build with less optimization
> than the default -O2, say -O. That may explain why nobody noticed
> earlier.

Ah, right.  It's true that I build on my machine without optimization so
that I can debug problems that show up on my local machine.  And I
always forget that I do that.

>> The patchlet below seems to fixed the issue for me.
>
> Indeed, I think we want something like that. Thanks. In terms of nits,
> formatting and types (who knows, maybe on some systems a std::size_t
> is an unsigned long long), I would write something like the
> attached. Please double check that it works for you and go ahead.

Great!  Your amendments work well for me.  I committed it to trunk.

Thank you.

-- 
		Dodji

Re: [PATCH] Add missing explicit instantiation for std::lower_bound template

[Benjamin De Kosnik]

[-- Attachment #1: Type: text/plain, Size: 1087 bytes --]


> >> Since commit r195676[1], it looks like
> >> libstdc++-v3/src/c++11/hashtable_c++0x.cc is missing an explicit
> >> instantiation for std::lower_bound. 
                                                               
it's missing an implicit instantiation of std::lower_bound.

>>This leads to libstdc++.so
> >> having the symbol for that (missing) instantiation be undefined,
> >> thus preventing executables from being linked with libstdc++.
> > Note that I can confirm this only if I build with less optimization
> > than the default -O2, say -O. That may explain why nobody noticed
> > earlier.

Yes, indeed.

Certainly, the explicit instantiation is ok for this non-usual -O
compile. The full instantiation set depends on specific flag and
perhaps platform. Let's not special case all of these, and instead just
allow implicit template instantiations via -fimplicit-templates. 

Thus, what is really needed (and also in cases like PR52887)
is to just allow implicit template instantiations. Done as attached.

-benjamin

tested x86/linux
tested x86/linux -O0
tested x86/linux -O

[-- Attachment #2: 20130211-2.patch --]
[-- Type: text/x-patch, Size: 1606 bytes --]

2013-02-11  Benjamin Kosnik  <bkoz@redhat.com>

	    * src/c++11/Makefile.am (hashtable_c++0x.lo, hashtable_c++0x.o):
	    Use -fimplicit-templates.
	    * src/c++11/Makefile.in: Regenerate.
	    * src/c++11/hashtable_c++0x.cc: Remove instantiation for
	    std::lower_bound template.


diff --git a/libstdc++-v3/src/c++11/Makefile.am b/libstdc++-v3/src/c++11/Makefile.am
index 89ee335..e7b48ac 100644
--- a/libstdc++-v3/src/c++11/Makefile.am
+++ b/libstdc++-v3/src/c++11/Makefile.am
@@ -60,6 +60,13 @@ vpath % $(top_srcdir)/src/c++11
 
 libc__11convenience_la_SOURCES = $(sources)  $(inst_sources)
 
+# Use special rules for the hashtable.cc file so that all
+# the generated template functions are also instantiated. 
+hashtable_c++0x.lo: hashtable_c++0x.cc
+	$(LTCXXCOMPILE) -fimplicit-templates -c $<
+hashtable_c++0x.o: hashtable_c++0x.cc
+	$(CXXCOMPILE) -fimplicit-templates -c $<
+
 # AM_CXXFLAGS needs to be in each subdirectory so that it can be
 # modified in a per-library or per-sub-library way.  Need to manually
 # set this option because CONFIG_CXXFLAGS has to be after
diff --git a/libstdc++-v3/src/c++11/hashtable_c++0x.cc b/libstdc++-v3/src/c++11/hashtable_c++0x.cc
index b6a56bc..7617c58 100644
--- a/libstdc++-v3/src/c++11/hashtable_c++0x.cc
+++ b/libstdc++-v3/src/c++11/hashtable_c++0x.cc
@@ -94,11 +94,4 @@ namespace __detail
 
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace __detail
-
- // Instantiations.
- template
- const unsigned long*
- lower_bound<const unsigned long*, size_t>(const unsigned long*,
-					   const unsigned long*,
-					   const size_t&);
 } // namespace std