dcleblanc / SafeInt

SafeInt is a class library for C++ that manages integer overflows.
MIT License
202 stars 37 forks source link

Inline function declarations in safe_math.h and safe_math_impl.h declarations don't work in C99 #52

Closed sconybeare closed 1 year ago

sconybeare commented 1 year ago

Using Apple clang version 14.0.0, with --std=c2x or --std=c99, I get undefined symbol errors at link time when I try to call any of the inline functions declared in safe_math_impl.h.

AFAICT this is because in C99, inline function definitions don't generate linker symbols, and so in general calls to them will break. find-and-replacing inline with static inline in both headers appears to fix the issue in my own project.

dcleblanc commented 1 year ago

Do you know if it represents with other compilers?

I'll take a look at this in the next few days. Thanks for bringing it to my attention.

Sent from my T-Mobile 5G Device Get Outlook for Androidhttps://aka.ms/AAb9ysg


From: Sebastian Conybeare @.> Sent: Tuesday, March 28, 2023 7:18:36 PM To: dcleblanc/SafeInt @.> Cc: Subscribed @.***> Subject: [dcleblanc/SafeInt] Inline function declarations in safe_math.h and safe_math_impl.h declarations don't work in C99 (Issue #52)

Using Apple clang version 14.0.0, with --std=c2x or --std=c99, I get undefined symbol errors at link time when I try to call any of the inline functions declared in safe_math_impl.h.

AFAICT this is because in C99, inline function definitions don't generate linker symbols, and so in general calls to them will break. find-and-replacing inline with static inline in both headers appears to fix the issue in my own project.

— Reply to this email directly, view it on GitHubhttps://github.com/dcleblanc/SafeInt/issues/52, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AHX6YLZPYNXD7CLSBNNZ3H3W6OLXZANCNFSM6AAAAAAWLKPEUU. You are receiving this because you are subscribed to this thread.Message ID: @.***>

dcleblanc commented 1 year ago

I am currently unable to reproduce this on my MacOS system using Apple clang 14. I took the compile test app in the ClangTest/CMakeFile.txt, and added the flags you have to the build. I think I probably just don't have the right repro - if you could please give me a bit more detail, I'll be happy to fix it.

dcleblanc commented 1 year ago

@sconybeare I am now seeing a repro on some systems. A proposed fix is in the static_fix branch. It builds happily on Linux, other systems not yet tested. Assuming that it works on other systems (can't see any reason why not), I'll merge it into main.

dcleblanc commented 1 year ago

Fixed with previous merge