Differential D5149

Fix the build with GCC < 8
ClosedPublic
Actions

Authored by Fabien on Feb 4 2020, 09:50.

Tags

None

Subscribers

Details

Reviewers

Group Reviewers

Restricted Project

Commits

rSTAGINGecfd419e5236: Fix the build with GCC < 8
rABCecfd419e5236: Fix the build with GCC < 8

Summary

Fix the `sorry, unimplemented: non-trivial designated initializers not
supported` failure with GCC version < 8.

See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55606 and
https://stackoverflow.com/questions/31215971/non-trivial-designated-initializers-not-supported.

Test Plan

Build with GCC 7 and check the error is gone.

Diff Detail

Repository

rABC Bitcoin ABC

Branch

fix_gcc_build

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 9265
Build 16472: Default Diff Build & Tests
Build 16471: arc lint + arc unit

Event Timeline

Fabien created this revision.Feb 4 2020, 09:50

Herald added a reviewer: Restricted Project. · View Herald TranscriptFeb 4 2020, 09:50

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B9256: Diff 15991.Feb 4 2020, 09:55

Looks like a C++20 feature. https://en.cppreference.com/w/cpp/language/aggregate_initialization

deadalnix requested changes to this revision.Feb 4 2020, 15:01

deadalnix added a subscriber: deadalnix.

deadalnix added inline comments.

src/chainparams.cpp
484 ↗	(On Diff #15991)	This is used here, so clearly reverting it is not the right path forward, at least not without going through the trouble of figuring out why it works in one case and not the other.

This revision now requires changes to proceed.Feb 4 2020, 15:01

Fabien requested review of this revision.Feb 4 2020, 15:08

Fabien added inline comments.

src/chainparams.cpp
484 ↗	(On Diff #15991)	This is a GCC limitation. It works only if the named initializers are in order with no missing one in the middle: https://stackoverflow.com/a/46067271

deadalnix requested changes to this revision.Feb 4 2020, 15:19

deadalnix added inline comments.

src/chainparams.cpp
484 ↗	(On Diff #15991)	Then it sounds like we might want to do that. Or wonder if we actually support gcc 8, and if we do than why the hell did D5139 pass CI?

This revision now requires changes to proceed.Feb 4 2020, 15:19

Insert missing initializer rather than doing it core way.

Build Bitcoin-ABC / Diffs / Diff Testing started.

Fabien added inline comments.Feb 4 2020, 16:09

src/chainparams.cpp
484 ↗	(On Diff #15991)	The CI runs gcc-8. I'm working toward adding other machines with various new builds.

Harbormaster completed remote builds in B9265: Diff 16005.Feb 4 2020, 16:13

Build Bitcoin-ABC / Diffs / Diff Testing passed.

deadalnix accepted this revision.Feb 4 2020, 16:13

This revision is now accepted and ready to land.Feb 4 2020, 16:13

Closed by commit rABCecfd419e5236: Fix the build with GCC < 8 (authored by Fabien). · Explain WhyFeb 4 2020, 16:47

This revision was automatically updated to reflect the committed changes.

Fabien added a commit: rABCecfd419e5236: Fix the build with GCC < 8.

Fabien added a commit: rSTAGINGecfd419e5236: Fix the build with GCC < 8.Apr 7 2020, 22:48

Revision Contents
Changeset List

Path

Size

src/

chainparams.cpp

2 lines

Diff 16005

src/chainparams.cpp

Loading...