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

Why are these functions preserved? #3926

Closed
AlexGuteniev opened this issue Aug 6, 2023 · 4 comments · Fixed by #3936
Closed

Why are these functions preserved? #3926

AlexGuteniev opened this issue Aug 6, 2023 · 4 comments · Fixed by #3936
Labels
enhancement Something can be improved fixed Something works now, yay!

Comments

@AlexGuteniev
Copy link
Contributor

I see that other preserved functions have _CRTIMP2 or similar, but not these.
Should these really be preserved?

STL/stl/src/winapisupp.cpp

Lines 157 to 175 in ed8150e

// TRANSITION, ABI: preserved for binary compatibility
extern "C" DWORD __cdecl __crtFlsAlloc(_In_opt_ PFLS_CALLBACK_FUNCTION const lpCallback) {
return FlsAlloc(lpCallback);
}
// TRANSITION, ABI: preserved for binary compatibility
extern "C" BOOL __cdecl __crtFlsFree(_In_ DWORD const dwFlsIndex) {
return FlsFree(dwFlsIndex);
}
// TRANSITION, ABI: preserved for binary compatibility
extern "C" PVOID __cdecl __crtFlsGetValue(_In_ DWORD const dwFlsIndex) {
return FlsGetValue(dwFlsIndex);
}
// TRANSITION, ABI: preserved for binary compatibility
extern "C" BOOL __cdecl __crtFlsSetValue(_In_ DWORD const dwFlsIndex, _In_opt_ PVOID const lpFlsData) {
return FlsSetValue(dwFlsIndex, lpFlsData);
}

@AlexGuteniev AlexGuteniev added the question Further information is requested label Aug 6, 2023
@frederick-vs-ja
Copy link
Contributor

frederick-vs-ja commented Aug 6, 2023

I guess they don't actually need to be preserved. They don't appear in the result of dumpbin /exports, so perhaps we can remove them now.

@StephanTLavavej
Copy link
Member

We can't remove things that were ever needed from the import lib, but since these weren't being injected there, it does seem quite plausible that they can be removed, and that the "preserved for ABI" comments were mistakenly added too mechanically.

@AlexGuteniev
Copy link
Contributor Author

The header declarations were removed in #1194 .

Looks like it was my decision to keep these, after failure with removing _CRTIMP2, #1194 (comment):

Actually probably need to keep not only _CRTIMP2, but every function visible from header

And I wasn't corrected. Probably the PR was scary enough to do more removal.

@StephanTLavavej
Copy link
Member

Yeah, there was a ton going on in that PR, and I didn't notice that they were never declared as exported. We've gotten better at noticing this as the codebase has gotten simpler (a lot of the weirdness in winapisupp was because it had organically grown so huge).

@StephanTLavavej StephanTLavavej added enhancement Something can be improved and removed question Further information is requested labels Aug 8, 2023
@StephanTLavavej StephanTLavavej added the fixed Something works now, yay! label Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved fixed Something works now, yay!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants