Bug 83102 - [8 Regression] go bootstrap error in ast-dump.cc due to __is_invocable failure
Summary: [8 Regression] go bootstrap error in ast-dump.cc due to __is_invocable failure
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: go (show other bugs)
Version: 8.0
: P3 normal
Target Milestone: 8.0
Assignee: Ian Lance Taylor
URL:
Keywords: build
Depends on:
Blocks:
 
Reported: 2017-11-21 23:00 UTC by Martin Sebor
Modified: 2020-05-08 21:58 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-11-22 00:00:00


Attachments
Patch to make comparisons work on const objects. (450 bytes, patch)
2017-11-22 14:10 UTC, Jonathan Wakely
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Sebor 2017-11-21 23:00:19 UTC
Bootstrapping GCC 8.0 (at r255038) on x86_64 fails with the error while compiling gcc/go/gofrontend/ast-dump.cc:

In file included from /opt/notnfs/msebor/build/gcc-svn/prev-x86_64-pc-linux-gnu/libstdc++-v3/include/map:60,
                 from /opt/notnfs/msebor/src/gcc/svn/gcc/go/go-system.h:36,
                 from /opt/notnfs/msebor/src/gcc/svn/gcc/go/gofrontend/ast-dump.cc:7:
/opt/notnfs/msebor/build/gcc-svn/prev-x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_tree.h: In instantiation of ‘class std::_Rb_tree<Import_init*, Import_init*, std::_Identity<Import_init*>, Import_init_lt, std::allocator<Import_init*> >’:
/opt/notnfs/msebor/build/gcc-svn/prev-x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_set.h:133:17:   required from ‘class std::set<Import_init*, Import_init_lt>’
/opt/notnfs/msebor/src/gcc/svn/gcc/go/gofrontend/gogo.h:127:37:   required from here
/opt/notnfs/msebor/build/gcc-svn/prev-x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_tree.h:452:7: error: static assertion failed: comparison object must be invocable with two arguments of key type
       static_assert(__is_invocable<const _Compare&, const _Key&, const _Key&>{},
       ^~~~~~~~~~~~~
/opt/notnfs/msebor/src/gcc/svn/gcc/go/Make-lang.in:239: recipe for target 'go/ast-dump.o' failed


A test case for the error suggests the problem is either in libstdc++ or the C++ front end:

$ cat t.C && g++ -S -Wall ... t.C
#include <set>

struct Import_init { int init_name () const; };


struct Import_init_lt {
  bool operator()(const Import_init* i1, const Import_init* i2)
  {
    return i1->init_name() < i2->init_name();
  }
};

class Import_init_set : public std::set<Import_init*, Import_init_lt> { };
In file included from /opt/notnfs/msebor/build/gcc-svn/prev-x86_64-pc-linux-gnu/libstdc++-v3/include/set:60,
                 from t.C:1:
/opt/notnfs/msebor/build/gcc-svn/prev-x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_tree.h: In instantiation of ‘class std::_Rb_tree<Import_init*, Import_init*, std::_Identity<Import_init*>, Import_init_lt, std::allocator<Import_init*> >’:
/opt/notnfs/msebor/build/gcc-svn/prev-x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_set.h:133:17:   required from ‘class std::set<Import_init*, Import_init_lt>’
t.C:13:37:   required from here
/opt/notnfs/msebor/build/gcc-svn/prev-x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_tree.h:452:7: error: static assertion failed: comparison object must be invocable with two arguments of key type
       static_assert(__is_invocable<const _Compare&, const _Key&, const _Key&>{},
       ^~~~~~~~~~~~~
Comment 1 Jonathan Wakely 2017-11-21 23:06:53 UTC
(In reply to Martin Sebor from comment #0)
> struct Import_init_lt {
>   bool operator()(const Import_init* i1, const Import_init* i2)

This function needs to be const

>   {
>     return i1->init_name() < i2->init_name();
>   }
> };
Comment 2 Jonathan Wakely 2017-11-21 23:14:34 UTC
Because otherwise it's impossible to use a const set:

const Import_init_set s;
auto it = s.lower_bound(0);

This fails, because the comparison object in the set is const, and so it's operator() isn't usable. The Go code apparently never calls const member functions on those sets.
Comment 3 Jonathan Wakely 2017-11-21 23:18:52 UTC
(In reply to Jonathan Wakely from comment #2)
> This fails, because the comparison object in the set is const, and so it's

s/it's/its/


The requirements in the standard were clarified by https://wg21.link/lwg2542 so I suppose strictly-speaking we should only reject it in C++17 mode, but our usual policy is to backport DRs to all modes where relevant (and we never supported the example in the DR, so the static_assert is just enforcing what was already the case in our implementation).
Comment 4 Jonathan Wakely 2017-11-21 23:33:57 UTC
If we want to relax the check this should work:

--- a/libstdc++-v3/include/bits/stl_tree.h
+++ b/libstdc++-v3/include/bits/stl_tree.h
@@ -448,8 +448,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       typedef __gnu_cxx::__alloc_traits<_Node_allocator> _Alloc_traits;
 
-#if __cplusplus >= 201103L
-      static_assert(__is_invocable<const _Compare&, const _Key&, const _Key&>{},
+#if __cplusplus >= 201703L
+      // LWG DR 2542: Missing const requirements for associative containers
+      static_assert(is_invocable_v<const _Compare&, const _Key&, const _Key&>,
+         "comparison object must be invocable with two arguments of key type");
+#elif __cplusplus >= 201103L
+      static_assert(__is_invocable<_Compare&, const _Key&, const _Key&>{},
          "comparison object must be invocable with two arguments of key type");
 #endif
Comment 5 Jonathan Wakely 2017-11-22 11:00:59 UTC
This is the fix for the Go frontend (untested, but obviously correct):

--- a/gcc/go/gofrontend/gogo.h
+++ b/gcc/go/gofrontend/gogo.h
@@ -117,7 +117,7 @@ class Import_init
 // For sorting purposes.
 
 struct Import_init_lt {
-  bool operator()(const Import_init* i1, const Import_init* i2)
+  bool operator()(const Import_init* i1, const Import_init* i2) const
   {
     return i1->init_name() < i2->init_name();
   }
Comment 6 Jonathan Wakely 2017-11-22 11:06:46 UTC
Author: redi
Date: Wed Nov 22 11:06:15 2017
New Revision: 255052

URL: https://gcc.gnu.org/viewcvs?rev=255052&root=gcc&view=rev
Log:
PR go/83102 relax std::set checks for invocable comparison object

	PR go/83102
	* include/bits/stl_tree.h (_Rb_tree): Relax invocable checks for
	comparison object pre-C++17.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/bits/stl_tree.h
Comment 7 Jonathan Wakely 2017-11-22 14:10:47 UTC
Created attachment 42679 [details]
Patch to make comparisons work on const objects.

Parse::Enclosing_var_comparison::operator() needs a fix too.

With this patch I can bootstrap with Go at r255051 (after that it builds without a patch, because I relaxed the static_assert in libstdc++).
Comment 8 ian@gcc.gnu.org 2017-11-22 15:19:15 UTC
Author: ian
Date: Wed Nov 22 15:18:43 2017
New Revision: 255062

URL: https://gcc.gnu.org/viewcvs?rev=255062&root=gcc&view=rev
Log:
    compiler: make comparison operator() methods const
    
    This is required for new versions of libstdc++ in C++17 mode.
    
    Fixes GCC PR 83102.
    
    Reviewed-on: https://go-review.googlesource.com/79396

Modified:
    trunk/gcc/go/gofrontend/MERGE
    trunk/gcc/go/gofrontend/gogo.h
    trunk/gcc/go/gofrontend/parse.cc
    trunk/gcc/go/gofrontend/parse.h
Comment 9 Ian Lance Taylor 2017-11-22 15:20:19 UTC
Should be fixed.