Add CMake/Clang support for the Calculator Engine #211

fwcd · 2019-03-08T22:58:01Z

Fixes #109 .

Description of the changes:

Added CMake with a library target for the engine
Added cross-platform headers for common Win32 error codes
Added typealiases for Win32 data types

How changes were validated:

Manual testing using Apple LLVM version 10.0.0 (clang-1000.11.45.5) on macOS 10.13.6 High Sierra

CMakeLists.txt

src/CalcManager/CEngine/calc.cpp

src/CalcManager/win_data_types_cross_platform.h

janisozaur · 2019-03-08T23:10:43Z

@fwcd please see #212, I was halfway through that when you submitted your changes. I asked Microsoft team to explain how would they want to pursue these changes.

fwcd · 2019-03-08T23:11:33Z

@janisozaur Looks great, that is definitely a better solution than mine!

src/CalcManager/win_data_types_cross_platform.h

fwcd · 2019-03-09T00:54:12Z

This branch now builds successfully both on Windows with msbuild and on macOS using xcodebuild (through CMake).

HowardWolosky · 2019-03-09T10:19:37Z

@mcooley -- What would it take to get the pipeline updated as part of this change so that we could have PR/CI validation moving forward to ensure that a change doesn't break this support?

mcooley · 2019-03-12T21:22:28Z

@mcooley -- What would it take to get the pipeline updated as part of this change so that we could have PR/CI validation moving forward to ensure that a change doesn't break this support?

Agreed, we should have some CI support so we don't break this. We currently have a job per configuration and currently one configuration per architecture--we could have an additional configuration for macOS or Linux. I'll look into this more.

danbelcher-MSFT · 2019-03-13T22:35:01Z

Just to recap, there are two PRs right now that start to address this issue, #211 and #212. We're holding off on reviewing and merging those PRs until we can build in a macOS or Linux environment as part of our CI, so we don't regress this capability later.

src/CalcManager/sal_cross_platform.h

src/CalcManager/CEngine/Number.cpp

CMakeLists.txt

src/CalcManager/win_data_types_cross_platform.h

@janisozaur

This is extract from microsoft#211 that enables compilation with GCC. With microsoft#211 now in the state of bitrot, I would rather try approaching it in smaller steps that can be hopefully merged quicker, even if it does not provide full support for all the features microsoft#211 provided. This will _compile_ correctly with my (@janisozaur) GCC, but clang is more picky about flexible array memebers and refuses to compile it yet. I will extract remaining parts of microsoft#211 in future PRs. I marked @fwcd as author, as he did most of the work in microsoft#211.

@janisozaur

This is extract from #211 that enables compilation with GCC. With #211 now in the state of bitrot, I would rather try approaching it in smaller steps that can be hopefully merged quicker, even if it does not provide full support for all the features #211 provided. This will _compile_ correctly with my (@janisozaur) GCC, but clang is more picky about flexible array members and refuses to compile it yet. I will extract remaining parts of #211 in future PRs. I marked @fwcd as author, as he did most of the work in #211.

@fwcd

These are the changes extracted from microsoft#211 that port number's mantissa from flexible member arrays, a non-standard C++ extension, to std::vector, which enables compilation and linkage with clang. @fwcd is the original author of these changes, I (@janisozaur) merely rebased them on top of current master. The rebase was luckily fairly straight-forward, with the only real conflict coming from 7a7ceb5 (microsoft#412), but was trivial to fix.

@fwcd

These are the changes extracted from microsoft#211 that port number's mantissa from flexible member arrays, a non-standard C++ extension, to std::vector, which enables compilation and linkage with clang. @fwcd is the original author of these changes, I (@janisozaur) merely rebased them on top of current master. The rebase was luckily fairly straight-forward, with the only real conflict coming from 7a7ceb5 (microsoft#412), but was trivial to fix.

@fwcd

These are the changes extracted from microsoft#211 that port number's mantissa from flexible member arrays, a non-standard C++ extension, to std::vector, which enables compilation and linkage with clang. @fwcd is the original author of these changes, I (@janisozaur) merely rebased them on top of current master. The rebase was luckily fairly straight-forward, with the only real conflict coming from 7a7ceb5 (microsoft#412), but was trivial to fix.

@fwcd

These are the changes extracted from microsoft#211 that port number's mantissa from flexible member arrays, a non-standard C++ extension, to std::vector, which enables compilation and linkage with clang. @fwcd is the original author of these changes, I (@janisozaur) merely rebased them on top of current master. The rebase was luckily fairly straight-forward, with the only real conflict coming from 7a7ceb5 (microsoft#412), but was trivial to fix.

@fwcd

These are the changes extracted from microsoft#211 that port number's mantissa from flexible member arrays, a non-standard C++ extension, to std::vector, which enables compilation and linkage with clang. @fwcd is the original author of these changes, I (@janisozaur) merely rebased them on top of current master. The rebase was luckily fairly straight-forward, with the only real conflict coming from 7a7ceb5 (microsoft#412), but was trivial to fix.

@fwcd

These are the changes extracted from microsoft#211 that port number's mantissa from flexible member arrays, a non-standard C++ extension, to std::vector, which enables compilation and linkage with clang. @fwcd is the original author of these changes, I (@janisozaur) merely rebased them on top of current master. The rebase was luckily fairly straight-forward, with the only real conflict coming from 7a7ceb5 (microsoft#412), but was trivial to fix.

ghost · 2019-07-13T04:00:17Z

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. Thank you for your contributions to Windows Calculator!

@fwcd

These are the changes extracted from microsoft#211 that port number's mantissa from flexible member arrays, a non-standard C++ extension, to std::vector, which enables compilation and linkage with clang. @fwcd is the original author of these changes, I (@janisozaur) merely rebased them on top of current master. The rebase was luckily fairly straight-forward, with the only real conflict coming from 7a7ceb5 (microsoft#412), but was trivial to fix.

ghost · 2019-12-10T22:00:21Z

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. Thank you for your contributions to Windows Calculator!

@fwcd

These are the changes extracted from microsoft#211 that port number's mantissa from flexible member arrays, a non-standard C++ extension, to std::vector, which enables compilation and linkage with clang. @fwcd is the original author of these changes, I (@janisozaur) merely rebased them on top of current master. The rebase was luckily fairly straight-forward, with the only real conflict coming from 7a7ceb5 (microsoft#412), but was trivial to fix.

@fwcd

These are the changes extracted from microsoft#211 that port number's mantissa from flexible member arrays, a non-standard C++ extension, to std::vector, which enables compilation and linkage with clang. @fwcd is the original author of these changes, I (@janisozaur) merely rebased them on top of current master. The rebase was luckily fairly straight-forward, with the only real conflict coming from 7a7ceb5 (microsoft#412), but was trivial to fix.

ghost · 2020-01-21T12:01:01Z

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. Thank you for your contributions to Windows Calculator!

mcooley · 2020-03-13T16:19:20Z

Given how stale this PR has become, it doesn't look like we'll be able to review and merge this as-is. Thanks for the contribution; we hope that some of these changes can make it in in smaller pieces in the future.

@fwcd

These are the changes extracted from microsoft#211 that port number's mantissa from flexible member arrays, a non-standard C++ extension, to std::vector, which enables compilation and linkage with clang. @fwcd is the original author of these changes, I (@janisozaur) merely rebased them on top of current master. The rebase was luckily fairly straight-forward, with the only real conflict coming from 7a7ceb5 (microsoft#412), but was trivial to fix.

@fwcd

These are the changes extracted from microsoft#211 that port number's mantissa from flexible member arrays, a non-standard C++ extension, to std::vector, which enables compilation and linkage with clang. @fwcd is the original author of these changes, I (@janisozaur) merely rebased them on top of current master. The rebase was luckily fairly straight-forward, with the only real conflict coming from 7a7ceb5 (microsoft#412), but was trivial to fix.

These are the changes based on microsoft#211 that port number's mantissa from flexible member arrays, a non-standard C++ extension, to std::vector, which enables compilation and linkage with clang.

janisozaur reviewed Mar 8, 2019

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

janisozaur reviewed Mar 8, 2019

View reviewed changes

src/CalcManager/CEngine/calc.cpp Outdated Show resolved Hide resolved

janisozaur reviewed Mar 8, 2019

View reviewed changes

src/CalcManager/win_data_types_cross_platform.h Outdated Show resolved Hide resolved

janisozaur reviewed Mar 8, 2019

View reviewed changes

src/CalcManager/win_data_types_cross_platform.h Outdated Show resolved Hide resolved

janisozaur mentioned this pull request Mar 8, 2019

Replace custom types with standard ones #212

Merged

fwcd commented Mar 8, 2019

View reviewed changes

src/CalcManager/win_data_types_cross_platform.h Outdated Show resolved Hide resolved

fwcd mentioned this pull request Mar 9, 2019

Alternative compilers #109

Closed

fwcd changed the title ~~Added Clang support for the Calculator Engine~~ Added CMake/Clang support for the Calculator Engine Mar 9, 2019

HowardWolosky added No Behavior Change labels Mar 9, 2019

This was referenced Mar 9, 2019

Windows Calculator does not work on gnu/linux #221

Closed

rewrite ui using material design, it has ugly windows style now #224

Closed

HowardWolosky assigned mcooley, danbelcher-MSFT and HowardWolosky Mar 11, 2019

fwcd mentioned this pull request Mar 11, 2019

Eliminate redundant copies of EMPTY_UNIT object, saving 2.5 KB. #235

Merged

mcooley closed this Mar 12, 2019

mcooley reopened this Mar 12, 2019

janisozaur reviewed Mar 15, 2019

View reviewed changes

src/CalcManager/sal_cross_platform.h Show resolved Hide resolved

HowardWolosky referenced this pull request Mar 15, 2019

fix for loop

372ddc1

janisozaur reviewed Mar 15, 2019

View reviewed changes

src/CalcManager/CEngine/Number.cpp Outdated Show resolved Hide resolved

janisozaur reviewed Mar 15, 2019

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

janisozaur reviewed Mar 16, 2019

View reviewed changes

src/CalcManager/win_data_types_cross_platform.h Outdated Show resolved Hide resolved

janisozaur reviewed Mar 16, 2019

View reviewed changes

src/CalcManager/win_data_types_cross_platform.h Outdated Show resolved Hide resolved

janisozaur mentioned this pull request Jun 6, 2019

Port number's mantissa to std::vector #544

Closed

ghost added the no recent activity label Jul 13, 2019

HowardWolosky assigned rudyhuyn Dec 3, 2019

ghost removed the no recent activity label Dec 3, 2019

HowardWolosky removed their assignment Dec 3, 2019

ghost added the no recent activity label Dec 10, 2019

grochocki added the fixing approved issue label Jan 21, 2020

ghost added no recent activity and removed no recent activity labels Jan 21, 2020

mcooley closed this Mar 13, 2020

ghost removed the no recent activity label Mar 13, 2020

Add CMake/Clang support for the Calculator Engine #211

Add CMake/Clang support for the Calculator Engine #211

Uh oh!

Conversation

fwcd commented Mar 8, 2019

Fixes #109 .

Description of the changes:

How changes were validated:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

janisozaur commented Mar 8, 2019

Uh oh!

fwcd commented Mar 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

fwcd commented Mar 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HowardWolosky commented Mar 9, 2019

Uh oh!

mcooley commented Mar 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danbelcher-MSFT commented Mar 13, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ghost commented Jul 13, 2019

Uh oh!

ghost commented Dec 10, 2019

Uh oh!

ghost commented Jan 21, 2020

Uh oh!

mcooley commented Mar 13, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

fwcd commented Mar 8, 2019 •

edited

Loading

fwcd commented Mar 9, 2019 •

edited

Loading

mcooley commented Mar 12, 2019 •

edited

Loading