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

unique_ptr fails to build with GCC 9 (and 10) #204

Open
shenki opened this issue Mar 16, 2022 · 3 comments
Open

unique_ptr fails to build with GCC 9 (and 10) #204

shenki opened this issue Mar 16, 2022 · 3 comments

Comments

@shenki
Copy link
Member

shenki commented Mar 16, 2022

When building hostboot with a newer GCC (see open-power/op-build#4595):

    ../../../../src/include/util/impl/unique_ptr.H:408:30: error: cannot
    bind rvalue reference of type ‘std::enable_if<true, STDUniquePtrTest::practice_struct&&>::type’
    {aka ‘STDUniquePtrTest::practice_struct&&’} to lvalue of type ‘STDUniquePtrTest::practice_struct’
      408 |                 return iv_ptr[i];

The failure is in the test case (no in hostboot seems to trigger it) here https://github.com/open-power/hostboot/blob/master-p10/src/usr/testcore/lib/unique_ptr.H#L116

            i[4].x = 99;

            if (i[4].x != 99)
            {
                TS_FAIL("Array access through unique_ptr returned incorrect value (expected 99)");
            }

            if (i[3].x != 4)
            {
                TS_FAIL("Array access through unique_ptr returned incorrect value (expected 4)");
            }

Removing this code allows the build to complete. However, this code seems to be valid. If we build it using GCC's unique_ptr.h, it compiles and the test case runs.

I don't understand what the templates are doing here, but it seems they are the cause of the problem:

            /* @brief Index operator for unique_ptr<T[]>.
             *
             * operator[] is not callable on a unique_ptr to non-array types.
             *
             * @param[in] i   Index into the array
             * @return        Value of T::operator[](i)
             */
            template<typename Index, typename X = T>
            typename enable_if<is_array<X>::value, decltype(declval<X>()[declval<Index>()])>::type
            operator[](const Index i)
            {
                return iv_ptr[i];
            }
@dcrowell77
Copy link
Collaborator

We've got some time scheduled later this year to bump up our compiler. In the meantime I'll see if someone can take a quick peek.

@ibmzach
Copy link
Member

ibmzach commented Mar 16, 2022

Looks like the way that decltype works has changed perhaps. Anyhow, fixed the issue with a commit that should make its way out of gerrit soon.

@shenki
Copy link
Member Author

shenki commented Mar 17, 2022

Thanks @ibmzach

op-jenkins pushed a commit that referenced this issue Mar 22, 2022
This commit makes a small change to fix operator[] for the unique_ptr
class for GCC 9 and 10. On GCC 9 and 10, operator[] was not returning
a reference and our testcases were failing to compile, even though
GCC 8 does compile it to return a reference.

Also adds a testcase for const unique_ptr and unique_ptr-to-const.

Resolves #204
Change-Id: Ia8bca57fd70656446eb81f8bcca4122cb4d280ff
Reviewed-on: http://rchgit01.rchland.ibm.com/gerrit1/128882
Tested-by: Jenkins OP Build CI <op-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins Server <pfd-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins Combined Simics CI <combined-simics-ci+hostboot@us.ibm.com>
Tested-by: FSP CI Jenkins <fsp-CI-jenkins+hostboot@us.ibm.com>
Tested-by: Jenkins OP HW <op-hw-jenkins+hostboot@us.ibm.com>
Tested-by: Hostboot CI <hostboot-ci+hostboot@us.ibm.com>
Reviewed-by: Roland Veloz <rveloz@us.ibm.com>
Reviewed-by: Isaac Salem <isaac.salem@ibm.com>
Reviewed-by: Daniel M Crowell <dcrowell@us.ibm.com>
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

No branches or pull requests

3 participants