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.

Sign up for GitHub

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

Jump to bottom

Prettify STL error output #2897

Merged
merged 9 commits into from
Aug 3, 2022
Merged

Prettify STL error output #2897

merged 9 commits into from
Aug 3, 2022

Conversation

strega-nil-ms
Copy link
Contributor

@strega-nil-ms strega-nil-ms commented Jul 22, 2022 •
edited
Loading

Fixes #2896

Before:

PS ...\stl-tests> cl /Tc .\stl-warnings.cxx
Microsoft (R) C/C++ Optimizing Compiler Version 19.33.31517 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

stl-warnings.cxx
...\yvals_core.h(23): fatal error C1189: #error:  STL1003: Unexpected compiler, expected C++ compiler.

After

PS ...\stl-tests> cl /Tc .\stl-warnings.cxx
Microsoft (R) C/C++ Optimizing Compiler Version 19.33.31517 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

stl-warnings.cxx
...\yvals_core.h(432): error STL1003: Unexpected compiler, expected C++ compiler.
...\yvals_core.h(432): error C2338: static_assert failed: 'Error in C++ Standard Library usage.'

@strega-nil
Prettify STL error output
98e603e 
Before:

```
PS ...\stl-tests> cl /Tc .\stl-warnings.cxx
Microsoft (R) C/C++ Optimizing Compiler Version 19.33.31517 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

stl-warnings.cxx
...\yvals_core.h(23): fatal error C1189: #error:  STL1003: Unexpected compiler, expected C++ compiler.
```

After

```
PS ...\stl-tests> cl /Tc .\stl-warnings.cxx
Microsoft (R) C/C++ Optimizing Compiler Version 19.33.31517 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

stl-warnings.cxx
...\yvals_core.h(432): error STL1003: Unexpected compiler, expected C++ compiler.
...\yvals_core.h(432): error C2338: static_assert failed: 'Error in C++ Standard Library usage.'
```
@strega-nil-ms strega-nil-ms added enhancement Something can be improved decision needed We need to choose something before working on this labels Jul 22, 2022
StephanTLavavej
StephanTLavavej reviewed Jul 23, 2022
View reviewed changes
stl/inc/yvals.h Outdated Show resolved Hide resolved
stl/inc/yvals_core.h Outdated Show resolved Hide resolved
stl/inc/yvals_core.h Outdated Show resolved Hide resolved
stl/inc/yvals.h Outdated Show resolved Hide resolved
@strega-nil-ms
Copy link
Contributor Author

Given that people generally seem happy with the idea of the changes, marking non-draft

@strega-nil-ms strega-nil-ms marked this pull request as ready for review July 26, 2022 00:37
@strega-nil-ms strega-nil-ms requested a review from a team as a code owner July 26, 2022 00:37
@strega-nil-ms strega-nil-ms removed the decision needed We need to choose something before working on this label Jul 26, 2022
@StephanTLavavej
Copy link
Member

Need to merge main in order to pick up #2061.

StephanTLavavej
StephanTLavavej requested changes Jul 28, 2022
View reviewed changes
stl/inc/coroutine Show resolved Hide resolved
CaseyCarter
CaseyCarter requested changes Jul 29, 2022
View reviewed changes
stl/inc/yvals_core.h Outdated Show resolved Hide resolved
stl/inc/yvals_core.h Show resolved Hide resolved
stl/inc/yvals_core.h Show resolved Hide resolved
stl/inc/yvals_core.h Show resolved Hide resolved
CaseyCarter
CaseyCarter approved these changes Jul 29, 2022
View reviewed changes
stl/inc/yvals_core.h Outdated Show resolved Hide resolved
stl/inc/yvals_core.h Outdated Show resolved Hide resolved
stl/inc/yvals_core.h Show resolved Hide resolved
stl/inc/yvals_core.h Show resolved Hide resolved
stl/inc/yvals_core.h Show resolved Hide resolved
StephanTLavavej
StephanTLavavej reviewed Jul 30, 2022
View reviewed changes
stl/inc/coroutine Outdated Show resolved Hide resolved
stl/inc/yvals_core.h Outdated Show resolved Hide resolved
stl/inc/coroutine Outdated Show resolved Hide resolved
@StephanTLavavej
Copy link
Member

@strega-nil-ms @CaseyCarter I've pushed a correctness fix, message appearance fix, and completed workaround removal - please double-check 😸

@strega-nil-ms
Copy link
Contributor Author

Thanks @StephanTLavavej, looks good to me :)

@StephanTLavavej StephanTLavavej self-assigned this Aug 3, 2022
@StephanTLavavej
Copy link
Member

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

@StephanTLavavej StephanTLavavej merged commit 222a558 into microsoft:main Aug 3, 2022
@StephanTLavavej
Copy link
Member

Thanks for improving how these errors are displayed! ✨ 🎉 😺

strega-nil added a commit to strega-nil/stl that referenced this pull request Aug 6, 2022
@strega-nil-ms @strega-nil @StephanTLavavej
Prettify STL error output (microsoft#2897)
651f8d6 
Co-authored-by: Nicole Mazzuca <mazzucan@outlook.com>
Co-authored-by: Stephan T. Lavavej <stl@nuwen.net>
fsb4000 pushed a commit to fsb4000/STL that referenced this pull request Aug 13, 2022
@strega-nil-ms @strega-nil
@StephanTLavavej @fsb4000
Prettify STL error output (microsoft#2897)
9ea1d98 
Co-authored-by: Nicole Mazzuca <mazzucan@outlook.com>
Co-authored-by: Stephan T. Lavavej <stl@nuwen.net>
@StephanTLavavej StephanTLavavej mentioned this pull request Aug 30, 2022
Merged
@strega-nil-ms strega-nil-ms deleted the better-error-message branch December 19, 2022 23:42
@StephanTLavavej StephanTLavavej mentioned this pull request Sep 5, 2023
Closed
@StephanTLavavej StephanTLavavej mentioned this pull request Jan 17, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@strega-nil strega-nil strega-nil left review comments

@Osyotr Osyotr Osyotr left review comments

@CaseyCarter CaseyCarter CaseyCarter approved these changes

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

Assignees

@StephanTLavavej StephanTLavavej

Labels
enhancement Something can be improved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Should STL errors use pragma message in order to make the output prettier/more obvious?
5 participants
@strega-nil-ms @StephanTLavavej @CaseyCarter @strega-nil @Osyotr
  翻译：