Skip to content

Conversation

@amyw-msft
Copy link
Member

There are projects in Windows that are using the STL in a header-only capacity, but using vector algorithms requires that the .lib be linked in. This change allows _USE_STD_VECTOR_ALGORITHMS to be manually set by the project in order to opt-out of requiring this .lib dependency. These projects used to specify /D_HAS_IF_CONSTEXPR=0 to avoid this dependency, but that has been removed and using /D_USE_STD_VECTOR_ALGORITHMS=0 is more targeted towards their actual desire anyway.

@amyw-msft amyw-msft requested a review from a team as a code owner March 17, 2021 17:34
@CaseyCarter CaseyCarter added the enhancement Something can be improved label Mar 17, 2021
Unwrap line that fits in 120 columns.
@BillyONeal
Copy link
Member

It seems like a test with a comment inside describing this specific-to-windows situation would make sense?

Is there any reason we can't provide the actual vectorized versions in a static lib for them or similar?

@StephanTLavavej StephanTLavavej removed their assignment Mar 20, 2021
@StephanTLavavej
Copy link
Member

@BillyONeal (oops, approved before reading your comment)

It seems like a test with a comment inside describing this specific-to-windows situation would make sense?

Yes, that would be easy to test (I'm not too concerned about test coverage since we inherently exercise the "off" codepath for ARM/ARM64, but the preprocessor logic may as well be tested). I can push changes.

Is there any reason we can't provide the actual vectorized versions in a static lib for them or similar?

Supporting an extra lib only used by Windows, in perpetuity, sounds like something we shouldn't sign up to do.

I would be more interested in refactoring the intrinsic headers to make this header-only (the only reason we aren't is because the necessary files are enormous).

@StephanTLavavej
Copy link
Member

FYI @CaseyCarter I pushed a test change after you approved.

@BillyONeal
Copy link
Member

Supporting an extra lib only used by Windows, in perpetuity, sounds like something we shouldn't sign up to do.

Supporting an extra lib seems cheaper than supporting an extra macro to me.

I would be more interested in refactoring the intrinsic headers to make this header-only (the only reason we aren't is because the necessary files are enormous).

The other reason it is out of line is /clr.

Header only is always worse for throughput; making all users pay because Windows did an unsupported thing doesn't seem like the right tradeoff to me 🤷‍♂️.

@StephanTLavavej StephanTLavavej merged commit e002186 into microsoft:main Mar 24, 2021
@StephanTLavavej
Copy link
Member

Thanks for implementing this escape hatch! 🚪 🏃 😹

@amyw-msft
Copy link
Member Author

Thanks for merging this (in both repos)!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Something can be improved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants