saleyn / eixx

Erlang C++ Interface
Apache License 2.0
137 stars 26 forks source link

Fix compiler warnings #50

Closed heri16 closed 2 years ago

heri16 commented 3 years ago

These changes prepare us for compatibility with Apple M1 Silicon (or 64-bit ARM architecture such as AWS Graviton).

heri16 commented 3 years ago

Preferred to throw an encoding exception if term could not be distributed due to length exceeded. In case future versions of the erlang distribution protocol or external term format allow for a longer length.

heri16 commented 3 years ago

Now compiles with Clang without producing segfaults.

There are now only left two files still producing conversion warnings that I would need help with.

Side note: int, size_t, intptr_t, uintptr_t are types that do not have a fixed witdth across architectures and platforms, as such C-style casts should be avoided in favor of static_cast().

heri16 commented 3 years ago

The compiler checks are here to help us.

@saleyn I suggest trying compiling this branch with the flags on to better understand the tradeoffs.

heri16 commented 3 years ago

Sorry about the last false notice (as test folder was skipped on macOs). Warnings have been reduced to just 2 files (which i may need help for).

saleyn commented 3 years ago

Now builds without errors. I'll try to review the warnings within the next couple of days.

saleyn commented 2 years ago

@heri16, could you address my questions in the review, so that I can merge?

heri16 commented 2 years ago

Apologies. Currently a little under the weather. Will try address them on Monday.

On Sat, 18 Sep 2021 at 8:55 PM, Serge Aleynikov @.***> wrote:

@heri16 https://github.com/heri16, could you address my questions in the review, so that I can merge?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/saleyn/eixx/pull/50#issuecomment-922272609, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEAV7O7V2PJGF3JJO5P62LUCSD2RANCNFSM5CZWTUSQ .

saleyn commented 2 years ago

Apologies. Currently a little under the weather. Will try address them on Monday.

👍

heri16 commented 2 years ago

./include/eixx/connect/transport_otp_connection.hxx and ./include/eixx/util/hashtable.hpp still has warnings.

Otherwise ready to merge. @saleyn