Skip to content

Conversation

@graebm
Copy link
Contributor

@graebm graebm commented Jun 5, 2023

Rust was having trouble calling aws_future_bool functions, due to them only being available as inline.

These functions were inline so that the we'd only need 1 declaration macro in a header. It wasn't a performance thing.

Fix is to have the macros generate normal non-inline functions, with a DECLARATION macro that goes in a header, and an IMPLEMENTATION macro that goes in a C file.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

graebm added 2 commits June 5, 2023 10:16
Rust was having trouble calling these functions. aws_future didn't really NEED the functions to be inline. It wasn't really a performance thing, I just liked how inline let there be a single DECLARATION in a header.

Solution is to have separate DECLARATION macro to go in a header, and an IMPLEMENTATION macro to go in a C file. Now they're normal functions.
@graebm graebm changed the title Future no inline aws_future functions no longer inline Jun 5, 2023
@codecov-commenter
Copy link

Codecov Report

Patch coverage: 81.25% and no project coverage change.

Comparison is base (396491c) 79.86% compared to head (5bf8487) 79.86%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #577   +/-   ##
=======================================
  Coverage   79.86%   79.86%           
=======================================
  Files          27       27           
  Lines        5805     5837   +32     
=======================================
+ Hits         4636     4662   +26     
- Misses       1169     1175    +6     
Impacted Files Coverage Δ
source/future.c 90.64% <81.25%> (-1.76%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@graebm graebm merged commit af3af99 into main Jun 5, 2023
@graebm graebm deleted the future-no-inline branch June 5, 2023 18:42
graebm added a commit to awslabs/aws-c-http that referenced this pull request Jun 5, 2023
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.

3 participants