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

2023-04-20 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. In D148723#4283094 , @serge-sans-paille wrote: > In D148723#4280916 , @efriedma > wrote: > >> The point of an "inline builtin" is that the inline function is actually the >> original f

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

2023-04-20 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 added a comment. I think something like #ifdef RINZOCORE_SHARED #define RINZO_LIB __declspec(dllexport) #else #define RINZO_LIB __declspec(dllimport) #endif RINZO_LIB inline func() {} Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148723/new/

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

2023-04-20 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:11538 const FunctionDecl *FD) { - if (!FD->isExternallyVisible()) + if (!FD->isExternallyVisible() || FD->isInlineBuiltinDeclaration()) return GVA_Inter

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

2023-04-20 Thread serge via Phabricator via cfe-commits
serge-sans-paille added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5192 - if (const auto *FD = D->getAsFunction()) + if (const auto *FD = D->getAsFunction()) { if (FD->isMultiVersion() && Linkage == GVA_AvailableExternally) erichkea

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

2023-04-20 Thread serge via Phabricator via cfe-commits
serge-sans-paille added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:11538 const FunctionDecl *FD) { - if (!FD->isExternallyVisible()) + if (!FD->isExternallyVisible() || FD->isInlineBuiltinDeclaration()) return GV

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

2023-04-20 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment. In 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

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

2023-04-19 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. This seems like a weird way to fix this. 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

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

2023-04-19 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5192 - if (const auto *FD = D->getAsFunction()) + if (const auto *FD = D->getAsFunction()) { if (FD->isMultiVersion() && Linkage == GVA_AvailableExternally) Is this an unre

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

2023-04-19 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 added a comment. Hi @serge-sans-paille, thanks for the fix. Could you please also try some test with dllimport/dllexport with inline function? Jennifer Comment at: clang/lib/AST/ASTContext.cpp:11538 const FunctionDecl *FD) {

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

2023-04-19 Thread serge via Phabricator via cfe-commits
serge-sans-paille created this revision. serge-sans-paille added reviewers: jyu2, efriedma. Herald added a project: All. serge-sans-paille requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Otherwise we end up with link once odr because of the