Skip to content

Fix for some 3180 warnings #841

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
dsyme merged 1 commit into from
Jan 8, 2016
Merged

Fix for some 3180 warnings #841

dsyme merged 1 commit into from
Jan 8, 2016

Conversation

@mpetruska
Copy link
Contributor

@mpetruska mpetruska commented Dec 28, 2015

This tiny PR contains the fix for issue #840.

@msftclas
Copy link

msftclas commented Dec 28, 2015

Hi @mpetruska, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, MSBOT;

enricosada
enricosada reviewed Dec 28, 2015
View reviewed changes
src/fsharp/tast.fs
Copy link
Contributor

@enricosada enricosada Dec 28, 2015

Choose a reason for hiding this comment

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

Mutable is ok, ref also ( there is an old school fan club 😄 )
Maybe there is safer to use ref than remove the warning
👍 for @mpetruska

Copy link
Contributor Author

@mpetruska mpetruska Dec 28, 2015

Choose a reason for hiding this comment

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

Thanks, my thoughts exactly. Not too experienced with the build system yet, so I'd rather modify code than build parameters.

@mpetruska mpetruska mentioned this pull request Jan 2, 2016
Closed
@dsyme
Copy link
Contributor

dsyme commented Jan 5, 2016

This can be merged. I'm Ok with making all allocations explicit in the compiler.

@enricosada
Copy link
Contributor

enricosada commented Jan 7, 2016

@KevinRansom can you check and merge this? the debug build doesnt compile atm

@dsyme
Copy link
Contributor

dsyme commented Jan 8, 2016

I'll merge this (as I need the debug build in any case)

dsyme added a commit that referenced this pull request Jan 8, 2016
@dsyme
Merge pull request #841 from mpetruska/fix-fsharp-compiler-build
2c6fedf 
Fix for some 3180 warnings
@dsyme dsyme merged commit 2c6fedf into dotnet:master Jan 8, 2016
@enricosada
Copy link
Contributor

enricosada commented Jan 8, 2016

really thx @dsyme
Was really annoying to cherry-pick and remove the fix, and really bad for contributors

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mpetruska @msftclas @dsyme @enricosada