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

Cleanups: Test options #3527

Merged

Conversation

StephanTLavavej
Copy link
Member

  • Remove workarounds for VSO-1350252 "ASan libraries do not provide variants for each _ITERATOR_DEBUG_LEVEL", fixed in VS 2022 17.5.
    • @strega-nil-ms explained to me that ASan was using the STL internally, which caused IDL mix-and-match problems, with /fno-sanitize-address-vcasan-lib being the workaround. Now that ASan has stopped using the STL internally, we can drop this workaround option.
  • Drop -Xclang before -Wno-deprecated-declarations and -Wno-self-assign.
    • At some point, this was necessary, although I never really understood why. (My vague understanding is that early on, clang-cl really insisted on taking only MSVC-compatible options, and needed -Xclang before any Clang-specific options.) This is clearly no longer necessary, and we can directly pass the Clang warning suppression options, as we're already doing for -Wno-unqualified-std-cast-call.
  • We don't need /Zc:strictStrings in the universal options anymore.
    • This is implied by /permissive- (and therefore /std:c++20), so we have extensive coverage.
C:\Temp>type meow.cpp
#include <iostream>

int main() {
    char* ptr = "Goodbye, deprecated world!\n";
    std::cout << ptr;
}
C:\Temp>cl /EHsc /nologo /W4 meow.cpp && meow
meow.cpp
Goodbye, deprecated world!

C:\Temp>cl /EHsc /nologo /W4 /Zc:strictStrings meow.cpp && meow
meow.cpp
meow.cpp(4): error C2440: 'initializing': cannot convert from 'const char [28]' to 'char *'
meow.cpp(4): note: Conversion from string literal loses const qualifier (see /Zc:strictStrings)

C:\Temp>cl /EHsc /nologo /W4 /permissive- meow.cpp && meow
meow.cpp
meow.cpp(4): error C2440: 'initializing': cannot convert from 'const char [28]' to 'char *'
meow.cpp(4): note: Conversion from string literal loses const qualifier (see /Zc:strictStrings)

C:\Temp>cl /EHsc /nologo /W4 /std:c++20 meow.cpp && meow
meow.cpp
meow.cpp(4): error C2440: 'initializing': cannot convert from 'const char [28]' to 'char *'
meow.cpp(4): note: Conversion from string literal loses const qualifier (see /Zc:strictStrings)

This is implied by /permissive- (and therefore /std:c++20), so we have extensive coverage.

    C:\Temp>type meow.cpp
    #include <iostream>

    int main() {
        char* ptr = "Goodbye, deprecated world!\n";
        std::cout << ptr;
    }

    C:\Temp>cl /EHsc /nologo /W4 meow.cpp && meow
    meow.cpp
    Goodbye, deprecated world!

    C:\Temp>cl /EHsc /nologo /W4 /Zc:strictStrings meow.cpp && meow
    meow.cpp
    meow.cpp(4): error C2440: 'initializing': cannot convert from 'const char [28]' to 'char *'
    meow.cpp(4): note: Conversion from string literal loses const qualifier (see /Zc:strictStrings)

    C:\Temp>cl /EHsc /nologo /W4 /permissive- meow.cpp && meow
    meow.cpp
    meow.cpp(4): error C2440: 'initializing': cannot convert from 'const char [28]' to 'char *'
    meow.cpp(4): note: Conversion from string literal loses const qualifier (see /Zc:strictStrings)

    C:\Temp>cl /EHsc /nologo /W4 /std:c++20 meow.cpp && meow
    meow.cpp
    meow.cpp(4): error C2440: 'initializing': cannot convert from 'const char [28]' to 'char *'
    meow.cpp(4): note: Conversion from string literal loses const qualifier (see /Zc:strictStrings)
@StephanTLavavej StephanTLavavej added the test Related to test code label Mar 3, 2023
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner March 3, 2023 19:59
@github-actions github-actions bot added this to Initial Review in Code Reviews Mar 3, 2023
@StephanTLavavej StephanTLavavej added the ASan Address Sanitizer label Mar 3, 2023
@StephanTLavavej StephanTLavavej moved this from Initial Review to Final Review in Code Reviews Mar 3, 2023
@strega-nil-ms strega-nil-ms moved this from Final Review to Ready To Merge in Code Reviews Mar 6, 2023
@StephanTLavavej StephanTLavavej self-assigned this Mar 7, 2023
@StephanTLavavej
Copy link
Member Author

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 33e4723 into microsoft:main Mar 7, 2023
Code Reviews automation moved this from Ready To Merge to Done Mar 7, 2023
@StephanTLavavej StephanTLavavej deleted the stl-cleanups-test-options branch March 7, 2023 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ASan Address Sanitizer test Related to test code
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants