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

gcc: Update coroutine header from master #8263

Closed
wants to merge 4 commits into from

Conversation

bustercopley
Copy link
Contributor

@bustercopley bustercopley commented Apr 2, 2021

Third patch prevents multiple definition errors when including header "coroutine" in more than one translation unit. Applying the other two intermediate commits to simplify patch management.

@Biswa96
Copy link
Member

Biswa96 commented Apr 2, 2021

What is build.cmd?

@bustercopley
Copy link
Contributor Author

What is build.cmd?

Oops, that was careless. It's just something I keep around locally for invoking bash commands from a cmd.exe environment. I'll replace the commit.

@lazka
Copy link
Member

lazka commented Apr 2, 2021

will this be in 10.3 (next week)?

@bustercopley
Copy link
Contributor Author

No, not as far as I can see. As of the advertised commit the first of the three commits in my PR had been cherry-picked to the 10.3 release candidate, and the other two commits had not.

@bustercopley
Copy link
Contributor Author

Ah, releases/gcc-10 is the rc branch. Same deal there: just the first patch has been cherry-picked.

@bustercopley
Copy link
Contributor Author

I've mentioned it on the gcc mailing list.

@lazka lazka changed the title Update coroutine header from master gcc: Update coroutine header from master Apr 3, 2021
@lazka
Copy link
Member

lazka commented Apr 4, 2021

Given the upstream feedback do you still think we should backport this?

@bustercopley
Copy link
Contributor Author

Yes, I think so, after we land 10.3. Users will be better served by having something that works, even if it means they need to recompile when the 'final' ABI is settled, potentially in version 10.4. (Just my opinion.)

@lazka
Copy link
Member

lazka commented Apr 4, 2021

ok, sounds good

@lazka
Copy link
Member

lazka commented Apr 11, 2021

gcc update in #8320

@bustercopley
Copy link
Contributor Author

Updated and rebased onto Astrum-polaris:updates.

@bustercopley
Copy link
Contributor Author

Fixed, I think.
This PR consists of one commit (8206161).
It doesn't apply cleanly to current master so I rebased onto f34c221 (PR #8320, gcc update to 10.3).
So this PR branch is 4 commits ahead of master (3 for #8320, 1 for #8263).
But you will probably want to merge #8320 first, then take just the final commit from here.

Hope this helps.

Here are some notes on what's in my commit.

Commits to libstdc++-v3/include/std/coroutine (in reverse chronological order) from git://gcc.gnu.org/git/gcc.git:

  • In branch master as of now (2a26351b598):
[F] 26a3f288f18 * libstdc++: Make coroutine_handle<_Promise>::from_address() noexcept [PR 99021]
[E] 99dee82307f * Update copyright years.
[D] 94fd05f1f76 * libstdc++: Define noop coroutine details private and inline [PR 95917]
[C] 2c2278f300c * libstdc++: Remove inheritance from std::coroutine_handle<> [LWG 3460]
[B] f1b6e46c417 * libstdc++, coroutine: Add missing constexpr markers.
[A] b9c91b7f327 * coroutines: Fix handling of non-class coroutine returns [PR94759]
    dcf69ac5448 * coroutines, libstdc++-v3: Update to n4861 C++20 DIS.
    ee26baf4a81 * coroutines: Rename the coroutines cpp builtin.
    49789fd0837 * [C++ coroutines] Initial implementation.
  • In tag releases/gcc-10.3.0:
[F] fa183497cf2 * libstdc++: Make coroutine_handle<_Promise>::from_address() noexcept [PR 99021]
[B] 517fb88b8a9 * libstdc++, coroutine: Add missing constexpr markers.
[A] b9c91b7f327 * coroutines: Fix handling of non-class coroutine returns [PR94759]
    dcf69ac5448 * coroutines, libstdc++-v3: Update to n4861 C++20 DIS.
    ee26baf4a81 * coroutines: Rename the coroutines cpp builtin.
    49789fd0837 * [C++ coroutines] Initial implementation.

The merge base is [A]. The fix is [D]. It's easiest to include [C] as well. Here's what I've done:

In gcc repo

  • reset to gcc-10.3.0
  • revert [F] fa183497cf2 (cherry pick of 26a3f288f18)
  • cherry pick [C] 2c2278f300c, [D] 94fd05f1f76, [F] 26a3f288f18 from master
  • create patches from those 4 commits (1 revert, 3 cherry picks)

There are now 4 patches for #8263 but in effect they just cherry pick two commits ([C] and [D]).

In MINGW-packages repo

@lazka
Copy link
Member

lazka commented Apr 11, 2021

Thanks. We will likely wait a bit after 10.3 to rule out any regressions before patching more. Please be patient :)

@bustercopley
Copy link
Contributor Author

This is in 11.1 (next week, maybe). I think it can wait until then.

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

4 participants