Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Allow building with VS2015 installed#822

Merged
jkotas merged 1 commit intodotnet:masterfrom
DickvdBrink:vs2015-build
Apr 26, 2015
Merged

Allow building with VS2015 installed#822
jkotas merged 1 commit intodotnet:masterfrom
DickvdBrink:vs2015-build

Conversation

@DickvdBrink
Copy link

Previous it was required to have VS2013 but now VS2015 can also be used. The default is still 2013 though.
On a machine with both version installed it is possible to force VS2013/VS2015 with a parameter.

Note: Still some work has to be done to make compilation successfull on VS2015.

There are some warnings which are now on by default (I think) and some new warnings which are an issue with TreatWarningAsError. The errors I'm talking about:

  • 4302 The compiler detected a conversion from a larger type to a smaller type. Information may be lost
  • 4311This warning detects 64-bit pointer truncation issues. For example, if code is compiled for a 64-bit architecture, the value of a pointer (64 bits) will be truncated if it is assigned to an int (32 bits)
  • 4312 This warning detects an attempt to assign a 32-bit value to a 64-bit pointer type, for example, casting a 32-bit int or long to a 64-bit pointer

hash_map/hash_set are deprecated causing the build to fail #821

So this pull request adds effectively nothing because VS2015 still doesn't work but I just wanted to throw this out here for review. If this isn't the right way feel free to close :)

@jkotas
Copy link
Member

jkotas commented Apr 26, 2015

Thanks a lot for doing this work! The small incremental changes like this one are exactly the right way.

@DickvdBrink DickvdBrink changed the title Allow buidling with VS2015 installed Allow building with VS2015 installed Apr 26, 2015
@DickvdBrink
Copy link
Author

@jkotas, thanks for the feedback, repushed a new version which should build.

I did a force-push so your comments are gone :s

edit Still fails, will investigate
I tried building on a machine with VS2013 but it fails building mscorlib because of this line.

So I didn't test everything because of that, sorry.. might need to fix that first if I find how to actually fix that.

edit2 Did you want me to remove the VSVersion parameter info from the usage stuff to?

Previous it was required to have VS2013 but now VS2015 can also be used. The default is still 2013 though.
On a machine with both version installed it is possible to force VS2013/VS2015 with a parameter.

Note: Still some work has to be done to make compilation successfull on VS2015.
@davidfowl
Copy link
Member

😄

@richlander
Copy link
Member

@DickvdBrink Could you also appropriately update this file as part of your change? https://github.com/dotnet/coreclr/blob/master/Documentation/windows-instructions.md

@DickvdBrink
Copy link
Author

@richlander, with this PR building doesn't work out of the box due to the deprecated stuff I mentioned in the first message and the new warnings introduced by vs2015. The TreatWarningsAsErrors is enabled so it doesn't work out of the box yet. So I think it is better to wait for that stuff (which I might fix after this :).

That is why I said this in the PR:

So this pull request adds effectively nothing because VS2015 still doesn't work but I just wanted to throw this out here for review

@richlander
Copy link
Member

@DickvdBrink Got it. Thanks for the clarification.

The best thing would be to create an issue, reference the issue from each of the PRs (GitHub will do its magic) and then reference the issue within the instructions that so that interested parties can follow the progress / get involved. Interested in doing that?

@DickvdBrink
Copy link
Author

Will do that! Hopefully tonight because we have a holiday in the Netherlands tomorrow.

@richlander
Copy link
Member

@DickvdBrink Great!

If you could transport that holiday over here for tomorrow, that would be greatly appreciated!

jkotas added a commit that referenced this pull request Apr 26, 2015
Allow building with VS2015 installed
@jkotas jkotas merged commit fb91a04 into dotnet:master Apr 26, 2015
@DickvdBrink
Copy link
Author

@richlander Referenced two issues to #30 and with this message this one too for reference :)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants