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

[WIP] FIX: MSVC compiler missing header breaking debugging #40681

Closed

Conversation

RevoluPowered
Copy link
Contributor

@RevoluPowered RevoluPowered commented Jul 24, 2020

Fixes #40666

Caused by a problem in the MSVC compiler, and thread_local on windows.

@RevoluPowered RevoluPowered requested a review from akien-mga as a code owner July 24, 2020 21:13
@akien-mga
Copy link
Member

Can you try to get this upstreamed too? Or a better version of it without the capslocks :P

@RevoluPowered
Copy link
Contributor Author

Don't merge only fixes win32... so not the entire fix.

@RevoluPowered RevoluPowered changed the title FIX: MSVC compiler missing header breaking debugging [WIP] FIX: MSVC compiler missing header breaking debugging Jul 24, 2020
@bruvzg
Copy link
Member

bruvzg commented Jul 24, 2020

Is it related to doctest/doctest#381 ?

@RevoluPowered
Copy link
Contributor Author

Is it related to onqtam/doctest#381 ?

he missed platform fix see new commit

main/tests/test_string.h Outdated Show resolved Hide resolved

DOCTEST_THREAD_LOCAL std::ostringstream g_oss; // NOLINT(cert-err58-cpp)

#if defined(DOCTEST_PLATFORM_WINDOWS)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try to keep the indentation correct even if it's a hotfix :)
And see @bruvzg's comment about editing third-party libs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do sorry MSVC keeps lagging out when I type in it... I will update with a real text editor and squash at the end once we are certain this is gone :D

@Xrayez
Copy link
Contributor

Xrayez commented Jul 24, 2020

It seems like it's possible to get into editor in 50% of times with current changes (compared to 30%), but I still get crashes on startup while debugging. D:

@RevoluPowered
Copy link
Contributor Author

It seems like it's possible to get into editor in 50% of times with current changes (compared to 30%), but I still get crashes on startup while debugging. D:

OK I updated all to use thread_local statically with the windows platform only.

Please let me know if the crash is gone

@Xrayez
Copy link
Contributor

Xrayez commented Jul 24, 2020

No, it may have improved the situation, but I might have just got lucky enough to run it 10 times in a row, but then crashes again 5 times in a row. 😄

@RevoluPowered
Copy link
Contributor Author

Closing: this fix 'kind of worked' but its mostly a hack, we can fix with swapping to /MD but ideally don't want to do this.

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

Successfully merging this pull request may close these issues.

Crash with doctest on attempting to debug the editor when compiling with tests=yes
5 participants