Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1427391
Summary: | /usr/include/cppad/local/pod_vector.hpp:58:25: error: redefinition of 'bool CppAD::local::is_pod() | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Ralf Corsepius <rc040203> | ||||
Component: | cppad | Assignee: | Brad Bell <bradbell> | ||||
Status: | CLOSED EOL | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
Severity: | high | Docs Contact: | |||||
Priority: | high | ||||||
Version: | 27 | CC: | anto.trande, bradbell | ||||
Target Milestone: | --- | Keywords: | Reopened | ||||
Target Release: | --- | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Whiteboard: | |||||||
Fixed In Version: | Doc Type: | If docs needed, set a value | |||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2018-11-30 20:07:11 UTC | Type: | Bug | ||||
Regression: | --- | Mount Type: | --- | ||||
Documentation: | --- | CRM: | |||||
Verified Versions: | Category: | --- | |||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
Cloudforms Team: | --- | Target Upstream Version: | |||||
Embargoed: | |||||||
Bug Depends On: | |||||||
Bug Blocks: | 1423298 | ||||||
Attachments: |
|
Description
Ralf Corsepius
2017-02-28 05:07:02 UTC
A diff between an i386-built and a x86_64-built cppad-devel package shows this: # diff -Naur x86_64/usr/include/cppad/configure.hpp i386/usr/include/cppad/configure.hpp --- x86_64/usr/include/cppad/configure.hpp 2017-02-28 05:55:48.000000000 +0100 +++ i386/usr/include/cppad/configure.hpp 2017-02-28 05:49:16.000000000 +0100 @@ -136,7 +136,7 @@ If this symbol is zero, the type size_t is the same as the type unsigned int, otherwise this symbol is one. */ -# define CPPAD_SIZE_T_NOT_UNSIGNED_INT 1 +# define CPPAD_SIZE_T_NOT_UNSIGNED_INT 0 /*! \def CPPAD_TAPE_ADDR_TYPE => This package hard-codes built-time detected values into headers. i.e. depending on whether this package was built on a 64bit or 32bit architectures, the other architecture family will be broken. => If this package wants to stay noarch'ed, CPPAD_SIZE_T_NOT_UNSIGNED_INT must not be hard-coded at build-time, but needs to be derived at compile-time. An alternative would be to eliminate CPPAD_SIZE_T_NOT_UNSIGNED_INT and all use-cases entirely. Actually, I am inclined to be believe (untested), CPPAD_SIZE_T_NOT_UNSIGNED_INT can be removed without substitute from configure.hpp.in and pod_vector.hpp. Created attachment 1258289 [details]
Remove CPPAD_SIZE_T_NOT_UNSIGNED_INT
My gut feeling seems to have been right.
The case CPPAD_SIZE_T_NOT_UNSIGNED_INT tries to cover, already seems to be covered by other cases in cppad/local/pod_vector.hpp, i.e. special-casing size_t should not be neccessary
[Depending on the architecture, size_t either matches with uint32_t or uint64_t, i.e. instead of
template <> inline bool is_pod<size_t>(void),
template <> inline bool is_pod<uint32_t>(void) rsp.
template <> inline bool is_pod<uint64_t>(void)
will be used]
Tested by rebuilding cppad with the patch from the attachment applid to cppad and subsequently building coin-or-OS on i386 and x86_64.
This is_pod<type> function detects which types are plain old data. This is used to increase performance of allocation and deallocation of the corresponding vectors. The current logic for this can be found on lines 33 through 60 of https://github.com/coin-or/CppAD/blob/stable/20170000/cppad/local/pod_vector.hpp the error is being detected because both template <> inline bool is_pod<size_t>(void) template <> inline bool is_pod<unsigned int>(void) are being defined On some machines <unsigned int> and <size_t> are different, and on others they are the same. The logic for, detecting this can be found in lines 228 though 239 of https://github.com/coin-or/CppAD/blob/stable/20170000/cppad/CMakeLists.txt The problem is that CppAD is specified as noarch it its spec file for the Fedora build. See BuildArch: noarch in the file https://src.fedoraproject.org/cgit/rpms/cppad.git/tree/cppad.spec?id=4ecffefcd814e1569d09431652391f265499fd22 The results of the test in CMakeLists.txt depend on the machine. So the CppAD configuration does depend on the architecture and the spec file needs to be changed to reflect this. Do you (Ralf) agree with this assessment ? Do you have access to the machine where it is failing and do you know how to build the rpm from its source ? If so, would you please try building the rpm there and seeing if it works; i.e., cmake sets the proper value for CPPAD_SIZE_T_NOT_UNSIGNED_INT ? (In reply to Brad Bell from comment #3) > This is_pod<type> function detects which types are plain old data. This is > used to increase performance of allocation and deallocation of the > corresponding vectors. Erm, presence of is_pod<size_t> will not provide any performance improvements. The code being generated will by the compiler be the same whether is_pod<size_t> will be present or not. > The current logic for this can be found on lines 33 through 60 of > https://github.com/coin-or/CppAD/blob/stable/20170000/cppad/local/pod_vector. > hpp > > the error is being detected because both > template <> inline bool is_pod<size_t>(void) > template <> inline bool is_pod<unsigned int>(void) > are being defined Yes and no. The error happens because these are identical (== duplicates) on some archs. > On some machines <unsigned int> and <size_t> are different, and on others > they are the same. Except of ancient target and exotic targets, size_t either _is_ "unsigned int" or "unsigned long". > The logic for, detecting this can be found in lines 228 > though 239 of > https://github.com/coin-or/CppAD/blob/stable/20170000/cppad/CMakeLists.txt Yep, I saw this. > The problem is that CppAD is specified as noarch it its spec file for the > Fedora build. See > BuildArch: noarch > in the file > https://src.fedoraproject.org/cgit/rpms/cppad.git/tree/cppad. > spec?id=4ecffefcd814e1569d09431652391f265499fd22 > > The results of the test in CMakeLists.txt depend on the machine. Yep. > So the > CppAD configuration does depend on the architecture and the spec file needs > to be changed to reflect this. > > Do you (Ralf) agree with this assessment ? No. I say, the approach upstream has taken is broken ;) It's unnecessary, doesn't do any good and can be dropped at no loss (This is what my patch does). Making this package arch'ed is just playing with symptoms of a flawed design. > Do you have access to the machine where it is failing and do you know how to > build the rpm from its source ? If so, would you please try building the rpm > there and seeing if it works; i.e., cmake sets the proper value for > CPPAD_SIZE_T_NOT_UNSIGNED_INT ? I do not understand, but yes, I can reproduce the problem with your rpm. (In reply to Ralf Corsepius from comment #4) ... snip ... > Erm, presence of is_pod<size_t> will not provide any performance > improvements. The code being generated will by the compiler be the same > whether is_pod<size_t> will be present or not. > Looking at line 102 in https://github.com/coin-or/CppAD/blob/stable/20170000/cppad/local/pod_vector.hpp we see if( ! is_pod<Type>() ) { // call destructor for each element size_t i; for(i = 0; i < capacity_; i++) (data_ + i)->~Type(); } So its seems to me that the constructor is called for the particular type when pod_vector is false. Maybe what you are saying is that the constructor and destructor calls for size_t are automatically optimized out by the compiler ? ... snip ... > I do not understand, but yes, I can reproduce the problem with your rpm. If you build the rpm from its source, it should re-run the cmake command (on that system) and get the proper value for CPPAD_SIZE_T_NOT_UNSIGNED_INT (for that system). I removed the 'BuildArch: noarch' from the CppAD spec file in rawhide; see http://pkgs.fedoraproject.org/cgit/rpms/cppad.git/commit/?id=25e6f4c7ead73dfc0aab96cc58911f3d5944cbfa I think that this has fixed the problem above; see the build https://koji.fedoraproject.org/koji/taskinfo?taskID=18248088 Please close this bug if you agree with this assessment. I merged the to the master branch (see previous comment) in the following branches: f26, f25, f24, epel7. I think that this has fixed this bug and so am closing it. Current release on fedora >= 25 do not fix this bug; coin-or-OS cannot be built yet. https://koji.fedoraproject.org/koji/taskinfo?taskID=20268084 Starting taskID=20268084, and following the link to buildArch (coin-or-OS-2.10.1-8.fc26.src.rpm, i686) and then the link to 'build.log' one gets https://kojipkgs.fedoraproject.org//work/tasks/8096/20268096/build.log where the following text appears near the end: /usr/include/cppad/local/pod_vector.hpp:58:25: error: redefinition of 'bool CppAD::local::is_pod() [with Type = unsigned int]' template <> inline bool is_pod<size_t>(void) { return true; } Where can I see the cmake output for the corresponding CppAD install, which put the file pod_vector.hpp in /usr/include/cppad/local ? I need to check this output to see the result for -- Performing Test cppad_size_t_not_unsigned_int_result For example, in https://kojipkgs.fedoraproject.org//packages/cppad/20170000.4/1.fc27/data/logs/i686/build.log One sees -- Performing Test cppad_size_t_not_unsigned_int_result - Failed for the same architecture. This should result in the code at line 58 of pod_vector.hpp not being compiled. This bug appears to have been reported against 'rawhide' during the Fedora 27 development cycle. Changing version to '27'. (In reply to Ralf Corsepius from comment #2) > Created attachment 1258289 [details] > Remove CPPAD_SIZE_T_NOT_UNSIGNED_INT > > My gut feeling seems to have been right. > > The case CPPAD_SIZE_T_NOT_UNSIGNED_INT tries to cover, already seems to be > covered by other cases in cppad/local/pod_vector.hpp, i.e. special-casing > size_t should not be neccessary > > [Depending on the architecture, size_t either matches with uint32_t or > uint64_t, i.e. instead of > template <> inline bool is_pod<size_t>(void), > template <> inline bool is_pod<uint32_t>(void) rsp. > template <> inline bool is_pod<uint64_t>(void) > will be used] > > Tested by rebuilding cppad with the patch from the attachment applid to > cppad and subsequently building coin-or-OS on i386 and x86_64. I think I now understand your suggestion and have created a version on the master branch for CppAD that solves the problem. The idea is to define is_pod for each of the C++ Fundamental types, and size_t will be included. See the new file is_pod.hpp on https://github.com/coin-or/CppAD/blob/master/cppad/local/is_pod.hpp If you get a chance would you please look at this solution. I plan to do the same in the current stable version cppad-20170000 and then upload new sources to Fedora to fix the problem. (In reply to Brad Bell from comment #11) > (In reply to Ralf Corsepius from comment #2) > > Created attachment 1258289 [details] > > Remove CPPAD_SIZE_T_NOT_UNSIGNED_INT > > > > My gut feeling seems to have been right. > > > > The case CPPAD_SIZE_T_NOT_UNSIGNED_INT tries to cover, already seems to be > > covered by other cases in cppad/local/pod_vector.hpp, i.e. special-casing > > size_t should not be neccessary > > > > [Depending on the architecture, size_t either matches with uint32_t or > > uint64_t, i.e. instead of > > template <> inline bool is_pod<size_t>(void), > > template <> inline bool is_pod<uint32_t>(void) rsp. > > template <> inline bool is_pod<uint64_t>(void) > > will be used] > > > > Tested by rebuilding cppad with the patch from the attachment applid to > > cppad and subsequently building coin-or-OS on i386 and x86_64. > > I think I now understand your suggestion and have created a version on the > master branch for CppAD that solves the problem. The idea is to define > is_pod for each of the C++ Fundamental types, and size_t will be included. > See the new file is_pod.hpp on > https://github.com/coin-or/CppAD/blob/master/cppad/local/is_pod.hpp > If you get a chance would you please look at this solution. > > I plan to do the same in the current stable version cppad-20170000 and then > upload new sources to Fedora to fix the problem. 'coin-or-OS' successfully rebuilt with 'cppad-20170000.4-3'. (In reply to Antonio Trande from comment #12) ... snip ... > > 'coin-or-OS' successfully rebuilt with 'cppad-20170000.4-3'. Is this bug still open ? Waiting for Ralph... :) I have changed is_pod.hpp on the CppAD master branch to use is_pod.hpp.in instead of is_pod.hpp; see https://github.com/coin-or/CppAD/blob/master/cppad/local/is_pod.hpp.in This enables the camke command to check each Fundamental type is compatible with the previous Fundamental types before adding it the list of is_pod template specializations; see https://github.com/coin-or/CppAD/blob/master/cppad/local/CMakeLists.txt In addition, there is an assertion, in pod_vector.hpp https://github.com/coin-or/CppAD/blob/master/cppad/local/pod_vector.hpp That makes sure size_t has been covered by the Fundamental types that get included; seach for CPPAD_ASSERT_UNKNOWN( is_pod<size_t>() ) I think that this will be robust and not fail silently. As part of the master branch, it will be in the stable version of CppAD that gets created on Janunary of next year (cppad-20180000) and subsequent versions in Fedora. If it would help, I can merge these changes into cppad-20170000 and create a new upstream source for use by Fedora. I think this bug had been fixed and will close it in one weeks time if there are no objections. This message is a reminder that Fedora 27 is nearing its end of life. On 2018-Nov-30 Fedora will stop maintaining and issuing updates for Fedora 27. It is Fedora's policy to close all bug reports from releases that are no longer maintained. At that time this bug will be closed as EOL if it remains open with a Fedora 'version' of '27'. Package Maintainer: If you wish for this bug to remain open because you plan to fix it in a currently maintained version, simply change the 'version' to a later Fedora version. Thank you for reporting this issue and we are sorry that we were not able to fix it before Fedora 27 is end of life. If you would still like to see this bug fixed and are able to reproduce it against a later version of Fedora, you are encouraged change the 'version' to a later Fedora version prior this bug is closed as described in the policy above. Although we aim to fix as many bugs as possible during every release's lifetime, sometimes those efforts are overtaken by events. Often a more recent Fedora release includes newer upstream software that fixes bugs or makes them obsolete. Fedora 27 changed to end-of-life (EOL) status on 2018-11-30. Fedora 27 is no longer maintained, which means that it will not receive any further security or bug fix updates. As a result we are closing this bug. If you can reproduce this bug against a currently maintained version of Fedora please feel free to reopen this bug against that version. If you are unable to reopen this bug, please file a new report against the current release. If you experience problems, please add a comment to this bug. Thank you for reporting this bug and we are sorry it could not be fixed. |