Skip to content

Use _STL_REPORT_ERROR in headers#5560

Merged
StephanTLavavej merged 12 commits intomicrosoft:mainfrom
AlexGuteniev:fail-faster-harder-better
Jun 14, 2025
Merged

Use _STL_REPORT_ERROR in headers#5560
StephanTLavavej merged 12 commits intomicrosoft:mainfrom
AlexGuteniev:fail-faster-harder-better

Conversation

@AlexGuteniev
Copy link
Contributor

@AlexGuteniev AlexGuteniev commented May 31, 2025 •
edited by StephanTLavavej
Loading

Resolves #5529.

terminate() still calls abort(). All other places use _STL_REPORT_ERROR.

Headers only. Keep abort() in separately compiled code to avoid losing a customization point.

🚨 Drops abort() from <utility> without any replacement🚨! Core headers should not call any functions.

@AlexGuteniev AlexGuteniev requested a review from a team as a code owner May 31, 2025 12:31
@github-project-automation github-project-automation bot moved this to Initial Review in STL Code Reviews May 31, 2025
AlexGuteniev
AlexGuteniev commented May 31, 2025
View reviewed changes
AlexGuteniev
AlexGuteniev commented May 31, 2025
View reviewed changes
Fulgen301
Fulgen301 reviewed May 31, 2025
View reviewed changes
AlexGuteniev and others added 2 commits May 31, 2025 17:54
@AlexGuteniev @Fulgen301
Update stl/inc/yvals.h
2628a15 
Co-authored-by: Fulgen301 <Fulgen301@users.noreply.github.com>
@AlexGuteniev
Update yvals.h
756bcd5
@StephanTLavavej StephanTLavavej added the enhancement Something can be improved label Jun 1, 2025
@StephanTLavavej StephanTLavavej self-assigned this Jun 1, 2025
StephanTLavavej
StephanTLavavej reviewed Jun 2, 2025
View reviewed changes
@StephanTLavavej StephanTLavavej removed their assignment Jun 2, 2025
@StephanTLavavej StephanTLavavej moved this from Initial Review to Ready To Merge in STL Code Reviews Jun 2, 2025
@StephanTLavavej StephanTLavavej mentioned this pull request Jun 2, 2025
Open
@frederick-vs-ja frederick-vs-ja mentioned this pull request Jun 6, 2025
Merged
@AlexGuteniev
Copy link
Contributor Author

AlexGuteniev commented Jun 6, 2025

Maybe it worth using _STL_REPORT_ERROR instead of _MSVC_STL_DOOM_FUNCTION here, just to be consistent?

@StephanTLavavej StephanTLavavej moved this from Ready To Merge to Final Review in STL Code Reviews Jun 11, 2025
@StephanTLavavej StephanTLavavej self-assigned this Jun 11, 2025
@StephanTLavavej StephanTLavavej changed the title Use _MSVC_STL_DOOM_FUNCTION in headers Use _STL_REPORT_ERROR in headers Jun 11, 2025
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Jun 11, 2025

Thanks, updated PR title and description. The difference is significant, not cosmetic:

STL/stl/inc/yvals.h

Lines 260 to 262 in 7841cf8

#define _STL_REPORT_ERROR(mesg) \
_RPTF0(_CRT_ASSERT, mesg); \
_MSVC_STL_DOOM_FUNCTION(mesg)

The UCRT's _RPTF0 is what pops up the nice assertion dialog in debug mode (or prints to stderr etc. if redirected). Without it, _MSVC_STL_DOOM_FUNCTION is just a __fastfail that makes the process vanish. I should have noticed this earlier - thanks for catching it.

@StephanTLavavej StephanTLavavej removed their assignment Jun 11, 2025
@StephanTLavavej StephanTLavavej moved this from Final Review to Ready To Merge in STL Code Reviews Jun 11, 2025
@StephanTLavavej StephanTLavavej moved this from Ready To Merge to Merging in STL Code Reviews Jun 11, 2025
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Jun 11, 2025

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

@frederick-vs-ja frederick-vs-ja mentioned this pull request Jun 13, 2025
Closed
@StephanTLavavej StephanTLavavej merged commit e2115ba into microsoft:main Jun 14, 2025
48 checks passed
@github-project-automation github-project-automation bot moved this from Merging to Done in STL Code Reviews Jun 14, 2025
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Jun 14, 2025

Thanks for making this consistent, and easier to follow precedent in the future! 😻 🎉 🥳

@AlexGuteniev AlexGuteniev deleted the fail-faster-harder-better branch June 14, 2025 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

+1 more reviewer

@Fulgen301 Fulgen301 Fulgen301 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement Something can be improved

Projects

STL Code Reviews
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Call __fastfail instead of abort() in more places

3 participants

@AlexGuteniev @StephanTLavavej @Fulgen301

Comments