-
Notifications
You must be signed in to change notification settings - Fork 280
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
Make changes to support compiling on Bookworm (with GCC 12) #1344
Make changes to support compiling on Bookworm (with GCC 12) #1344
Conversation
The primary purpose of this is to bring in support for compiling on Debian Bookworm. This brings in the following changes: * Update the Doxyfile for doxygen in Debian Bookworm (opencomputeproject/SAI#1946) * Enable sai_uint16_t in ProcessStructValueType Struct Member (opencomputeproject/SAI#1949) * [meta] Add support for port stat extensions (opencomputeproject/SAI#1947) * [meta] Add custom range start end values check (opencomputeproject/SAI#1945) * Cable diagnostics attribute added (opencomputeproject/SAI#1894) * Add attributes to disable L3 rewrites (opencomputeproject/SAI#1924) * Add MAC remote loopback to the port loopback enums. (opencomputeproject/SAI#1934) * [TAM] Granular counter subscription (opencomputeproject/SAI#1670) Signed-off-by: Saikrishna Arcot <[email protected]>
`#include <array>` needed to be added in two files. Signed-off-by: Saikrishna Arcot <[email protected]>
Signed-off-by: Saikrishna Arcot <[email protected]>
@@ -2,6 +2,7 @@ | |||
|
|||
#include "swss/logger.h" | |||
|
|||
#include <array> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this is added?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GCC 12 complained that std::array
was not completely defined, but is instantiated on line 74.
Snippet of error log:
ServiceMethodTable.cpp: In instantiation of 'constexpr auto declare_static(std::index_sequence<i ...>) [with B = syncd::ServiceMethodTable::SlotBase; D = syncd::ServiceMethodTable::Slot; long unsigned int ...i = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; std::index_sequence<i ...> = std::integer_sequence<long unsigned int, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9>]':
ServiceMethodTable.cpp:81:35: required from 'constexpr auto declare_static() [with B = syncd::ServiceMethodTable::SlotBase; D = syncd::ServiceMethodTable::Slot; long unsigned int size = 10]'
ServiceMethodTable.cpp:86:79: required from here
ServiceMethodTable.cpp:74:56: error: invalid use of incomplete type 'struct std::array<syncd::ServiceMethodTable::SlotBase*, 10>'
74 | return std::array<B*, sizeof...(i)>{{new D<i>()...}};
| ^
In file included from /usr/include/c++/12/bits/stl_map.h:63,
from /usr/include/c++/12/map:61,
from /usr/include/swss/logger.h:6,
from ServiceMethodTable.h:7,
from ServiceMethodTable.cpp:1:
/usr/include/c++/12/tuple:1595:45: note: declaration of 'struct std::array<syncd::ServiceMethodTable::SlotBase*, 10>'
1595 | template<typename _Tp, size_t _Nm> struct array;
| ^~~~~
ServiceMethodTable.cpp: In instantiation of 'constexpr auto declare_static() [with B = syncd::ServiceMethodTable::SlotBase; D = syncd::ServiceMethodTable::Slot; long unsigned int size = 10]':
ServiceMethodTable.cpp:86:79: required from here
ServiceMethodTable.cpp:82:48: error: no matching function for call to 'std::vector<syncd::ServiceMethodTable::SlotBase*>::vector(<brace-enclosed initializer list>)'
82 | return std::vector<B*>{arr.begin(), arr.end()};
| ~~~~~~~^~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3hivh version cimpiler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GCC 12.2, specifically (also using libstdc++ 12.2)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want, I think I can modify the pipeline to build this on Bookworm as well, so that is covered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your title suggest tat this is the change so support bookworm so what other changes would be needed ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure at this point. At this point, only compilation (and the tests that get run as part of the build) have been done; runtime testing has not been done yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then please update title to reflect changes, like GCC latest support or update headers etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
@@ -1,6 +1,7 @@ | |||
#include "SwitchNotifications.h" | |||
|
|||
#include "swss/logger.h" | |||
#include <array> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this isnadded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GCC 12 complained that std::array
was not completely defined, but is instantiated on line 144.
Snippet of error log:
SwitchNotifications.cpp: In instantiation of 'constexpr auto declare_static(std::index_sequence<i ...>) [with B = syncd::SwitchNotifications::SlotBase; D = syncd::SwitchNotifications::Slot; long unsigned int ...i = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}; std::index_sequence<i ...> = std::integer_sequence<long unsigned int, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15>]':
SwitchNotifications.cpp:151:35: required from 'constexpr auto declare_static() [with B = syncd::SwitchNotifications::SlotBase; D = syncd::SwitchNotifications::Slot; long unsigned int size = 16]'
SwitchNotifications.cpp:156:83: required from here
SwitchNotifications.cpp:144:56: error: invalid use of incomplete type 'struct std::array<syncd::SwitchNotifications::SlotBase*, 16>'
144 | return std::array<B*, sizeof...(i)>{{new D<i>()...}};
| ^
In file included from /usr/include/c++/12/bits/stl_map.h:63,
from /usr/include/c++/12/map:61,
from /usr/include/swss/logger.h:6,
from SwitchNotifications.h:7,
from SwitchNotifications.cpp:1:
/usr/include/c++/12/tuple:1595:45: note: declaration of 'struct std::array<syncd::SwitchNotifications::SlotBase*, 16>'
...
This adds support for sairedis to compile on Debian Bookworm (with GCC 12). This PR also includes a SAI submodule update that brings in other unrelated changes; this is needed because of a change in Doxyfile that would cause warnings/errors in newer versions of Doxygen.
The SAI submodule update brings in the following changes:
Note that runtime functionality testing has not been done; only compilation (with the build-time tests) has been done.