Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

seastar.cc, tls: include used header #2175

Closed
wants to merge 2 commits into from

Conversation

tchaikov
Copy link
Contributor

@tchaikov tchaikov commented Apr 3, 2024

this series add two missing #include:s in tls.cc and seastar.cc. for similar but different reasons:

  • in tls.cc, we use unordered_set<> template without including it. this compiles fine without enabling C++20 modules, but this behavior is fragile -- we could remove the #include <unordered_set> macro in a random header file (indirectly) included by this source file in future. so we should include it where the template is used.
  • in seastar.cc, the purview is supposed to include all external headers. if it fails to do so, the implementation units would fail to access a single copy of the symbols, and causes confusions of the compiler.

this should address the build failure with modules enabled:

```
FAILED: src/CMakeFiles/seastar-module.dir/net/tls.cc.o
/home/kefu/.local/bin/clang++ -DFMT_SHARED -DSEASTAR_API_LEVEL=7 -DSEASTAR_COROUTINES_ENABLED -DSEASTAR_MODULE -DSEASTAR_SCHEDULING_GROUPS_COUNT=16 -DSEASTAR_SSTRING -Dseastar_module_EXPORTS -I/home/kefu/dev/seastar/include -I/home/kefu/d
ev/seastar/build/debug/gen/include -I/home/kefu/dev/seastar/src -g -std=c++23 -fPIC -U_FORTIFY_SOURCE -Wno-include-angled-in-module-purview -MD -MT src/CMakeFiles/seastar-module.dir/net/tls.cc.o -MF src/CMakeFiles/seastar-module.dir/net/t
ls.cc.o.d @src/CMakeFiles/seastar-module.dir/net/tls.cc.o.modmap -o src/CMakeFiles/seastar-module.dir/net/tls.cc.o -c /home/kefu/dev/seastar/src/net/tls.cc                                                                                   /home/kefu/dev/seastar/src/net/tls.cc:930:14: error: missing '#include <bits/unordered_set.h>'; 'unordered_set' must be declared before it is used
  930 |         std::unordered_set<sstring> _all_files;
      |              ^
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/unordered_set.h:104:11: note: declaration here is not visible
  104 |     class unordered_set
      |           ^
/home/kefu/dev/seastar/src/net/tls.cc:892:44: error: missing '#include <bits/unordered_set.h>'; 'unordered_set' must be declared before it is used                                                                                              892 |                 _cb(boost::copy_range<std::unordered_set<sstring>>(_files | boost::adaptors::map_keys), std::move(ep));
      |                                            ^
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/unordered_set.h:104:11: note: declaration here is not visible
  104 |     class unordered_set
      |           ^
/home/kefu/dev/seastar/src/net/tls.cc:1574:73: error: missing '#include <bits/unordered_set.h>'; 'unordered_set' must be declared before it is used
 1574 |     future<std::vector<subject_alt_name>> get_alt_name_information(std::unordered_set<subject_alt_name_type> types) {
      |                                                                         ^
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/unordered_set.h:104:11: note: declaration here is not visible
  104 |     class unordered_set
      |           ^
/home/kefu/dev/seastar/src/net/tls.cc:1793:73: error: missing '#include <bits/unordered_set.h>'; 'unordered_set' must be declared before it is used
 1793 |     future<std::vector<subject_alt_name>> get_alt_name_information(std::unordered_set<subject_alt_name_type> types) {                                                                                                                       |                                                                         ^
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/unordered_set.h:104:11: note: declaration here is not visible
  104 |     class unordered_set
      |           ^
/home/kefu/dev/seastar/src/net/tls.cc:1977:105: error: missing '#include <bits/unordered_set.h>'; 'unordered_set' must be declared before it is used
 1977 | future<std::vector<tls::subject_alt_name>> tls::get_alt_name_information(connected_socket& socket, std::unordered_set<subject_alt_name_type> types) {
      |                                                                                                         ^
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/unordered_set.h:104:11: note: declaration here is not visible
  104 |     class unordered_set
      |           ^
```

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
this allows all module implementation units to use a single definition
without worrying about using different definitions in different units.
without this change, clang-18 and libstdc++ shipped by GCC-13 fails
to build the tree like:

```
FAILED: src/CMakeFiles/seastar-module.dir/core/memory.cc.o
/home/kefu/.local/bin/clang++ -DFMT_SHARED -DSEASTAR_API_LEVEL=7 -DSEASTAR_COROUTINES_ENABLED -DSEASTAR_MODULE -DSEASTAR_SCHEDULING_GROUPS_COUNT=16 -DSEASTAR_SSTRING -Dseastar_module_EXPORTS -I/home/kefu/dev/seastar/include -I/home/kefu/d
ev/seastar/build/debug/gen/include -I/home/kefu/dev/seastar/src -g -std=c++23 -fPIC -U_FORTIFY_SOURCE -Wno-include-angled-in-module-purview -MD -MT src/CMakeFiles/seastar-module.dir/core/memory.cc.o -MF src/CMakeFiles/seastar-module.dir/c
ore/memory.cc.o.d @src/CMakeFiles/seastar-module.dir/core/memory.cc.o.modmap -o src/CMakeFiles/seastar-module.dir/core/memory.cc.o -c /home/kefu/dev/seastar/src/core/memory.cc
In module 'seastar' imported from /home/kefu/dev/seastar/src/core/memory.cc:111:
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/unordered_set.h:1827:5: error: 'std::operator==' has different definitions in different modules; definition in module 'seastar' first difference is function body
 1826 |     inline bool
      |     ~~~~~~~~~~~
 1827 |     operator==(const unordered_multiset<_Value, _Hash, _Pred, _Alloc>& __x,
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1828 |                const unordered_multiset<_Value, _Hash, _Pred, _Alloc>& __y)
      |                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1829 |     { return __x._M_h._M_equal(__y._M_h); }
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/unordered_set.h:1827:5: note: but in '' found a different body
 1826 |     inline bool
      |     ~~~~~~~~~~~
 1827 |     operator==(const unordered_multiset<_Value, _Hash, _Pred, _Alloc>& __x,
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1828 |                const unordered_multiset<_Value, _Hash, _Pred, _Alloc>& __y)
      |                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1829 |     { return __x._M_h._M_equal(__y._M_h); }
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In module 'seastar' imported from /home/kefu/dev/seastar/src/core/memory.cc:111:
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/unordered_set.h:1813:5: error: 'std::operator==' has different definitions in different modules; definition in module 'seastar' first difference is function body
 1812 |     inline bool
      |     ~~~~~~~~~~~
 1813 |     operator==(const unordered_set<_Value, _Hash, _Pred, _Alloc>& __x,
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1814 |                const unordered_set<_Value, _Hash, _Pred, _Alloc>& __y)
      |                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1815 |     { return __x._M_h._M_equal(__y._M_h); }
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
@nyh nyh closed this in b74a027 Apr 3, 2024
@tchaikov tchaikov deleted the include-used-headers branch April 3, 2024 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant