Skip to content

Visual Studio compilation documentation #6111

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
Merged
merged 2 commits into from
May 19, 2021
Merged

Visual Studio compilation documentation #6111

merged 2 commits into from
May 19, 2021

Conversation

NlightNFotis
Copy link
Contributor

@NlightNFotis NlightNFotis commented May 12, 2021 •
edited
Loading

This PR serves two purposes:

  1. Add documentation on building with Visual Studio on Windows to the COMPILING.md file.
  2. Do some further minor cleanup on top of Restructure compilation documentation #6109 and implement some suggestions from the
    review comments there.
  • Each commit message has a non-empty body, explaining why the change was made.
  • Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • White-space or formatting changes outside the feature-related changed lines are in commits of their own.

@NlightNFotis NlightNFotis self-assigned this May 12, 2021
@NlightNFotis NlightNFotis force-pushed the tas/compiling_docs_refactor_windows_vs branch from 1a8fc0b to d8824ff Compare May 12, 2021 16:30
@codecov
Copy link

codecov bot commented May 12, 2021 •
edited
Loading

Codecov Report

Merging #6111 (d63f9b7) into develop (f564088) will decrease coverage by 0.08%.
The diff coverage is 82.38%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #6111      +/-   ##
===========================================
- Coverage    75.53%   75.45%   -0.09%     
===========================================
  Files         1447     1447              
  Lines       158116   158093      -23     
===========================================
- Hits        119431   119282     -149     
- Misses       38685    38811     +126
Impacted Files Coverage Δ
src/goto-instrument/accelerate/accelerate.cpp 34.89% <0.00%> (ø)
src/goto-instrument/concurrency.cpp 0.00% <0.00%> (ø)
src/goto-instrument/nondet_volatile.cpp 86.55% <ø> (-0.15%) ⬇️
src/goto-instrument/replace_calls.cpp 89.55% <0.00%> (+1.31%) ⬆️
src/goto-programs/goto_convert_class.h 87.30% <ø> (ø)
src/goto-programs/goto_convert.cpp 91.69% <33.33%> (-0.24%) ⬇️
src/util/simplify_expr_struct.cpp 74.38% <50.00%> (+0.78%) ⬆️
src/goto-instrument/goto_program2code.cpp 69.20% <60.00%> (+0.06%) ⬆️
src/util/simplify_expr.cpp 78.14% <65.00%> (-6.93%) ⬇️
src/goto-programs/goto_program.h 90.71% <66.66%> (-0.93%) ⬇️
... and 72 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ea7f13...d63f9b7. Read the comment docs.

TGWDB
TGWDB approved these changes May 13, 2021
View reviewed changes
COMPILING.md Outdated
to something like `x64-Debug (Default)` or whatever the equivalent is
for your system.

The above instructions hae been tested against Visual Studio 2019.
Copy link
Contributor

Choose a reason for hiding this comment

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

Above says VS 2017, which version is it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have tested it with Visual Studio 2019, but 2017 is the first version that added support for this.

Do I convey this accurately through the text?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am OK with the two different version numbers above, as the versions match the facts. Maybe add 2019 to the section title though.

tautschnig
tautschnig approved these changes May 13, 2021
View reviewed changes
COMPILING.md Outdated
@@ -114,6 +114,25 @@ files.
complete set of built binaries can be found in `./build/bin` once the build
is complete.

## Compilation on Windows using Visual Studio

Follow these instructions to work on Windows with Visual Studio:
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be useful to make clear whether that's on top of the above instructions, or a replacement of (some of) the steps.

thomasspriggs
thomasspriggs reviewed May 14, 2021
View reviewed changes
Copy link
Contributor

@thomasspriggs thomasspriggs left a comment

Choose a reason for hiding this comment

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

Needs a rebase to get rid of the commits which have already been merged as separate PRs.

COMPILING.md Outdated
to something like `x64-Debug (Default)` or whatever the equivalent is
for your system.

The above instructions hae been tested against Visual Studio 2019.
Copy link
Contributor

Choose a reason for hiding this comment

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

I am OK with the two different version numbers above, as the versions match the facts. Maybe add 2019 to the section title though.

NlightNFotis and others added 2 commits May 17, 2021 10:34
@NlightNFotis
Add instructions on building on Windows with Visual Studio.
49f603a
@NlightNFotis
Implement suggestions from diffblue#6109 and some further cleanup.
d63f9b7 
Implement some suggestions presented as comments in diffblue#6109 (we
had to merge the PR before we could get a chance to incorporate
them, so we are compensating now) and do some other minor
cleanup of the COMPILING.md file.
@NlightNFotis NlightNFotis force-pushed the tas/compiling_docs_refactor_windows_vs branch from d8824ff to d63f9b7 Compare May 17, 2021 10:05
@NlightNFotis
Copy link
Contributor Author

Hi @tautschnig, can you please re-review the second commit in this PR?

We had to merge #6109 before we could incorporate your suggested fixes, so this is now including them.

@thomasspriggs thomasspriggs merged commit 0589e94 into diffblue:develop May 19, 2021
@NlightNFotis NlightNFotis deleted the tas/compiling_docs_refactor_windows_vs branch May 19, 2021 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tautschnig tautschnig tautschnig approved these changes

@thomasspriggs thomasspriggs thomasspriggs approved these changes

@TGWDB TGWDB TGWDB approved these changes

@chrisr-diffblue chrisr-diffblue Awaiting requested review from chrisr-diffblue

@kroening kroening Awaiting requested review from kroening kroening is a code owner

@peterschrammel peterschrammel Awaiting requested review from peterschrammel peterschrammel is a code owner

@smowton smowton Awaiting requested review from smowton

Assignees

@NlightNFotis NlightNFotis

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@NlightNFotis @tautschnig @thomasspriggs @TGWDB