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

Compensate for new warnings and STL changes in VS 16.7 #7319

Merged
3 commits merged into from
Aug 18, 2020
Merged

Compensate for new warnings and STL changes in VS 16.7 #7319

3 commits merged into from
Aug 18, 2020

Conversation

miniksa
Copy link
Member

@miniksa miniksa commented Aug 17, 2020 •
edited by DHowett
Loading

New warnings were added in VS 16.7 and std::map::erase is now noexcept.
Update our code to be compatible with the new enforcement.

PR Checklist

  • Closes broken audit in main after Agents updated over the weekend.
  • I work here.
  • Audit mode passes now
  • Am core contributor.

Validation Steps Performed

  • Ran audit mode locally

@miniksa miniksa added Product-Conhost For issues in the Console codebase Issue-Bug It either shouldn't be doing this or needs an investigation. Area-Build Issues pertaining to the build system, CI, infrastructure, meta Product-Terminal The new Windows Terminal. labels Aug 17, 2020
@DHowett DHowett mentioned this pull request Aug 17, 2020
Merged
@skyline75489
Copy link
Collaborator

Wait a minute they are allowed to do that? I thought noexcept was only a nightmare in users’ code. Isn’t there some kind of regulation to prevent the signatures of std functions from changing?

@DHowett
Copy link
Member

DHowett commented Aug 18, 2020

I believe that removing noexcept is considered a breaking change, but adding it may not be?

@DHowett
Copy link
Member

DHowett commented Aug 18, 2020

@msftbot merge this in like 5 minutes

@ghost ghost added the AutoMerge Marked for automatic merge by the bot when requirements are met label Aug 18, 2020
@ghost
Copy link

ghost commented Aug 18, 2020

Hello @DHowett!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

  • I won't merge this pull request until after the UTC date Tue, 18 Aug 2020 00:14:10 GMT, which is in 5 minutes

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

@miniksa
Copy link
Member Author

miniksa commented Aug 18, 2020

I swear I'm not crazy. On my machine (with 16.7.1) it says that erase is noexcept despite the CI failing the static analysis build saying it is not:

image

@miniksa
Copy link
Member Author

miniksa commented Aug 18, 2020

My machine is using this path for STL: C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.27.29110\include
The build machine is using this path for STL:
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.26.28801\include

Which would mean the build machine is on Visual Studio 16.6.5.

@miniksa
Copy link
Member Author

miniksa commented Aug 18, 2020

I'll yank the noexcept from erase until the build machines rev to 16.7. But I have no idea why them going to (apparently) 16.6.5 this weekend broke all the other stuff.

Basically what I'm saying is... GUH.

@ghost ghost merged commit a50c48c into master Aug 18, 2020
@ghost ghost deleted the dev/miniksa/vs167_audit_changes branch August 18, 2020 16:59
DHowett pushed a commit that referenced this pull request Aug 20, 2020
@miniksa @DHowett
Compensate for new warnings and STL changes in VS 16.7 (#7319)
b7e4fb1 
New warnings were added in VS 16.7 and `std::map::erase` is now `noexcept`.
Update our code to be compatible with the new enforcement.

## PR Checklist
* [x] Closes broken audit in main after Agents updated over the weekend.
* [x] I work here.
* [x] Audit mode passes now
* [x] Am core contributor.

## Validation Steps Performed
* [x] Ran audit mode locally

(cherry picked from commit a50c48c)
@zadjii-msft zadjii-msft mentioned this pull request Aug 24, 2020
Merged
ghost pushed a commit that referenced this pull request Aug 24, 2020
@zadjii-msft
Compensate for VS 16.7, part 2 (#7383)
55b6ace 
This is just the `noexcept` part of #7319, because the CI apparently got updated overnight.
DHowett added a commit that referenced this pull request Sep 3, 2020
@DHowett
Merged PR 5097423: Migrate OSS up to 0488c53
f357e37 
Dustin L. Howett
* Clear the last error before calling Mb2Wc in ConvertToW (GH-7391)
* Update clang-format to 10.0 (GH-7389)
* Add til::static_map, a constexpr key-value store (GH-7323)

James Holderness
* Refactor VT control sequence identification (CC-7304)

Mike Griese
* Compensate for VS 16.7, part 2 (GH-7383)
* Add support for iterable, nested commands (GH-6856)

Michael Niksa
* Helix Testing (GH-6992)
* Compensate for new warnings and STL changes in VS 16.7 (GH-7319)

nathpete-msft
* Fix environment block creation (GH-7401)

Chester Liu
* Add initial support for VT DCS sequences (CC-6328)

Related work items: #28791050
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DHowett DHowett DHowett approved these changes

@zadjii-msft zadjii-msft zadjii-msft approved these changes

Assignees

@miniksa miniksa

Labels
Area-Build Issues pertaining to the build system, CI, infrastructure, meta AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@miniksa @skyline75489 @DHowett @zadjii-msft