[PATCH] D148723: [clang] Enforce internal linkage for inline builtin

Thu Apr 20 02:24:41 PDT 2023

serge-sans-paille added a comment.

In D148723#4280916 <https://reviews.llvm.org/D148723#4280916>, @efriedma wrote:

> This seems like a weird way to fix this.

I agree, not a big fan either. But wanting to start the bike shedding in some way.

> The point of an "inline builtin" is that the inline function is actually the original function; it's just the inline implementation is only used in limited circumstances (in particular, it can't be used recursively).  Changing the linkage could have unexpected side-effects.

That's not my understanding. An inline builtin provides an alternate implementation of the builtin that usually wraps the original builtin in some way. It's not meant to be externally visible (it would probably collide with libc's implementation in that case).

> Maybe it makes sense to restore some form of the "GNUInlineAttr" check in isInlineBuiltinDeclaration.  But instead of actually checking for the attribute, check that the function would be emitted with available_externally linkage.  So "inline builtins" don't exist on Windows because inline functions are linkonce_odr.  But we still detect builtins that are declared `inline` instead of `extern inline __attribute((gnu_inline))`.

I'll give this some thoughts / experiments. Thanks!

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148723/new/

https://reviews.llvm.org/D148723