Skip to content

Update to v142 build tools for Visual Studio 2019#548

Merged
mcooley merged 8 commits intomicrosoft:masterfrom
mcooley:vs2019
Jun 24, 2019
Merged

Update to v142 build tools for Visual Studio 2019#548
mcooley merged 8 commits intomicrosoft:masterfrom
mcooley:vs2019

Conversation

@mcooley
Copy link
Copy Markdown
Member

@mcooley mcooley commented Jun 13, 2019

Fixes #463

  • Update to build tools v142
  • Update official build pipelines to use VS2019
  • Update instructions to suggest updating to VS2019

@mcooley
Copy link
Copy Markdown
Member Author

mcooley commented Jun 18, 2019

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Jun 18, 2019

Azure Pipelines successfully started running 1 pipeline(s).

@mcooley
Copy link
Copy Markdown
Member Author

mcooley commented Jun 18, 2019 •
edited
Loading

We're getting an access violation when resuming from the coroutine at App.xaml.cpp:174. It looks like this happens only in the Release configuration.

This is crashing the app shortly after launch, which is why the UI tests are failing.

>	Calculator.exe!CalculatorApp::App::SetupJumpList$_ResumeCoro$2() Line 174	C++
 	[Inline Frame] Calculator.exe!std::experimental::coroutine_handle<void>::resume() Line 96	C++
 	[Inline Frame] Calculator.exe!std::experimental::coroutine_handle<void>::operator()() Line 88	C++
 	[Inline Frame] Calculator.exe!Windows::Foundation::await_suspend::__l2::<lambda_230b85c7a6b4a1b5e4c7c54e2106b62b>::operator()(Windows::Foundation::IAsyncAction ^) Line 339	C++
 	Calculator.exe!Platform::Details::__abi_FunctorCapture<<lambda_230b85c7a6b4a1b5e4c7c54e2106b62b>,void,Windows::Foundation::IAsyncAction ^,enum Windows::Foundation::AsyncStatus>::Invoke(Windows::Foundation::IAsyncAction ^ <__args_0>, Windows::Foundation::AsyncStatus <__args_1>) Line 889	C++
 	Calculator.exe!?__abi_Windows_Foundation_?$AsyncOperationCompletedHandler@P$AAVJumpList@StartScreen@UI@Windows@@___abi_IDelegate____abi_Invoke@?Q__abi_IDelegate@?$AsyncOperationCompletedHandler@P$AAVJumpList@StartScreen@UI@Windows@@@Foundation@Windows@@234@U$AAGJP$AAU?$IAsyncOperation@P$AAVJumpList@StartScreen@UI@Windows@@@34@W4AsyncStatus@34@@Z(Windows::Foundation::IAsyncOperation<Windows::UI::StartScreen::JumpList ^> ^ __param0, Windows::Foundation::AsyncStatus __param1)	C++

@mcooley mcooley mentioned this pull request Jun 19, 2019
Open
@mcooley mcooley requested a review from HowardWolosky June 20, 2019 18:29
@HowardWolosky HowardWolosky self-assigned this Jun 24, 2019
@HowardWolosky HowardWolosky added the build Any work that primarily impacts the solution / project files, but not the code itself. label Jun 24, 2019
HowardWolosky
HowardWolosky suggested changes Jun 24, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@HowardWolosky HowardWolosky left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for doing the update (and going the extra mile to update the screenshot as well). Just one minor requested change. I separately have a local build going on right now to provide a smoke test.

src/CalcViewModel/Common/Utils.cpp
wstring ws = RemoveUnwantedCharsFromWstring(input, unWantedChars, 6);
string inputString(ws.begin(), ws.end());
return ::atof(inputString.c_str());
return stod(ws);
Copy link
Copy Markdown
Contributor

@HowardWolosky HowardWolosky Jun 24, 2019

Choose a reason for hiding this comment

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

We should explicitly use the widechar version of functions. We're never going to not ship this in Unicode.

Suggested change
return stod(ws);
return wcstod(ws);

Copy link
Copy Markdown
Member Author

@mcooley mcooley Jun 24, 2019

Choose a reason for hiding this comment

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

stod operates on std::wstring, while wcstod is a C function and it would need to be wcstod(ws.c_str()). I'd prefer to leave it as-is.

@mcooley mcooley merged commit 33b885a into microsoft:master Jun 24, 2019
@LanceMcCarthy
Copy link
Copy Markdown
Contributor

LanceMcCarthy commented Jun 24, 2019 •
edited
Loading

<-- super-happy dancing Ninja Cat goes here -->

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

Reviewers

1 more reviewer

@HowardWolosky HowardWolosky HowardWolosky approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@HowardWolosky HowardWolosky

Labels

build Any work that primarily impacts the solution / project files, but not the code itself.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Retarget to Build Tools 142 to support Visual Studio 2019

3 participants

@mcooley @LanceMcCarthy @HowardWolosky