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

Mark as inline functions implemented in src/serialize.h #44739

Merged
merged 3 commits into from
Apr 1, 2022

Conversation

giordano
Copy link
Contributor

In #44634 (comment) it was suggested that these functions were implemented in the header to allow inlining, but they weren't annotated with inline, thus causing a warning when the functions were unused.

Also, remove duplicate definitions from src/staticdata.c and include
serialize.h in there.

@fingolfin
Copy link
Member

Hehe, I was just about to make a similar PR :-). Glad I looked here first. Hopefully this can be merged soon, it seems innocent enough. The buildkite failure stems from an unrelated test (apparently "128 == 16" does not hold... 😂)

src/serialize.h Show resolved Hide resolved
@giordano giordano force-pushed the mg/inline-serialize branch from dc48c2b to 6a38f3c Compare March 31, 2022 17:36
@staticfloat staticfloat requested a review from vtjnash March 31, 2022 17:55
@giordano giordano added the triage This should be discussed on a triage call label Mar 31, 2022
@giordano
Copy link
Contributor Author

With #44350 now merged, in a local rebase of this PR onto latest master I can build cleanly with CFLAGS=-Werror CXXFLAGS=-Werror with GCC on x86_64 linux.

@vtjnash vtjnash added merge me PR is reviewed. Merge when all tests are passing and removed triage This should be discussed on a triage call labels Mar 31, 2022
@vtjnash
Copy link
Member

vtjnash commented Mar 31, 2022

triage is busy, they should not be getting asked to review this type of PR

@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Apr 1, 2022
@DilumAluthge
Copy link
Member

analyzegc needs to be fixed.

giordano added 3 commits April 1, 2022 13:08
Also, remove duplicate definitions from `src/staticdata.c` and include
`serialize.h` in there.
@giordano giordano force-pushed the mg/inline-serialize branch from 6a38f3c to 9ed8033 Compare April 1, 2022 12:09
@giordano
Copy link
Contributor Author

giordano commented Apr 1, 2022

I reverted the conversion of macros to functions, now analyzegc is happy again

@KristofferC KristofferC merged commit 081ae64 into JuliaLang:master Apr 1, 2022
@giordano giordano deleted the mg/inline-serialize branch April 1, 2022 18:02
KristofferC pushed a commit that referenced this pull request Oct 28, 2022
* Mark as `inline` functions implemented in `src/serialize.h`

Also, remove duplicate definitions from `src/staticdata.c` and include
`serialize.h` in there.

(cherry picked from commit 081ae64)
@KristofferC KristofferC added the backport 1.8 Change should be backported to release-1.8 label Oct 28, 2022
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label Nov 8, 2022
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.

5 participants