What is the correct way to use GSL with VS 2017 RC Core Guideline checker?

[KrisTC]

This might not be quite the right place to ask this. Please redirect me if needed. But what it the correct way to use the GSL with VS2017 RC?
This a pre-cursor to doing something more useful, we have managed to get permission to move from VS2013 to VS2017 within the year. I am hoping we can introduce GSL and Core Guidelines at the same time. Especially given the build in Core Guidelines Checker.

I have made a super simple win32 console project in visual studio 2017 RC and put it on my github: https://github.com/KrisTC/CoreGuidlinesTestAppVs17Rc

The source file is very simple (and bad):

#include "stdafx.h"
#include <algorithm>
#include <gsl/gsl>

int main()
{
    // Super simple bad code :)
    int *p = new int[4];
    std::fill(p, p + 4, 0);
    return *p; 
}

I had hoped I would get error about the lack of delete, but I don't.
Instead I get the following errors in the GSL:

gsl\gsl\string_span(161): warning C26496: Variable len is assigned only once, use const. (con.4: https://go.microsoft.com/fwlink/p/?LinkID=784969)
gsl\gsl\string_span(159): warning C26461: The input pointer / reference argument __$ReturnUdt in function gsl::ensure_z can be marked as const. (con.3: https://go.microsoft.com/fwlink/p/?LinkID=786684)
gsl\gsl\string_span(168): warning C26496: Variable len is assigned only once, use const. (con.4: https://go.microsoft.com/fwlink/p/?LinkID=784969)
gsl\gsl\string_span(166): warning C26461: The input pointer / reference argument __$ReturnUdt in function gsl::ensure_z can be marked as const. (con.3: https://go.microsoft.com/fwlink/p/?LinkID=786684)
gsl\gsl\string_span(175): warning C26496: Variable len is assigned only once, use const. (con.4: https://go.microsoft.com/fwlink/p/?LinkID=784969)
gsl\gsl\string_span(173): warning C26461: The input pointer / reference argument __$ReturnUdt in function gsl::ensure_z can be marked as const. (con.3: https://go.microsoft.com/fwlink/p/?LinkID=786684)
gsl\gsl\string_span(182): warning C26496: Variable len is assigned only once, use const. (con.4: https://go.microsoft.com/fwlink/p/?LinkID=784969)
gsl\gsl\string_span(180): warning C26461: The input pointer / reference argument __$ReturnUdt in function gsl::ensure_z can be marked as const. (con.3: https://go.microsoft.com/fwlink/p/?LinkID=786684)

And this error in my program:

coreguidlinestestappvs17rc.cpp(11): warning C26496: Variable p is assigned only once, use const. (con.4: https://go.microsoft.com/fwlink/p/?LinkID=784969)
coreguidlinestestappvs17rc.cpp(12): warning C26481: Don't use pointer arithmetic. Use span instead. (bounds.1: http://go.microsoft.com/fwlink/p/?LinkID=620413)

This error looks wrong to me, I am assigning to it and I will delete it too so it can't be const. Even if I make the pointer const int* const p = new int[4]; I still get the warning.

If I turn on experimental core guidline checks I also get the error I hoped for:

coreguidlinestestappvs17rc.cpp(11): warning C26423: The allocation was not directly assigned to an owner.

Is this a problem in the GSL or the CoreChecker?
The "warning C26423: The allocation was not directly assigned to an owner." check was experimental in VS2015 too will it be non-experimental by VS2017 is released?

[KrisTC]

@neilmacintosh , Neil is this the correct place to ask this question?
What is the best way to provide feedback to the visual studio VS2017 RC team?

I am happy to do a PR for the warnings:

gsl\gsl\string_span(161): warning C26496: Variable len is assigned only once, use const. (con.4: https://go.microsoft.com/fwlink/p/?LinkID=784969)

But I don't think the others are correct.

Thanks

Kris

[MikeGitb]

I think coreguidlinestestappvs17rc.cpp(11): warning C26496... and C26481 are valid too (although I dont understand, why const qualifying the pointer doesn't resolve the warning. Only C26461 seems to refer to something that only exists internaly in the compiler and should therefor not be presented to the user.

[neilmacintosh]

@KrisTC Firstly, thanks very much for trying out VS 2017 and providing some feedback! I'll try to address your questions here (but possibly out-of-order).

Firstly, this is an ok place to ask these questions, given you are asking about the GSL and VS 2017 and this is the GSL repo....so no harm done there ;-)

Next, the best way to provide feedback to the VS 2017 RC team is via the "send feedback" button on the top right of the title bar. That feedback gets turned into bugs and routed to the appropriate people.

Next, I wouldn't pay too much attention to the results of the const-related warnings in the CppCoreCheck that shipped with RC. We have done a substantial revamp of those warnings that will show up in the final release of VS 2017 and they will be much quieter and more sensible.

You can feel free to offer a PR that addresses any issues you think are valid that were found in your run. However, I do plan to do a pass withe the final release version just before it ships and do updates then, too.

Finally, the check you were looking for - about not having a delete to match your new is really only experimental at this point. That set of checks (we broadly refer to them as the "lifetimes" checks) will not be shipping in the release version of VS 2017. They will follow in a subsequent release.

Hope that all helps!

[KrisTC]

@neilmacintosh
Thank you for your feedback and happy new year :)
I will make these changes to string and make PR.

Amusingly my "Send Feedback -> Report a problem" button in VS2017RC doesn't work. It complains about being unable to install something (I will try re-installing the latest version).

Are you able to share when VS 2017 with the improved checker will be available for me to test?
When you say the lifetimes checks will come in a subsequent release, do you mean an update to 2017 or a later VS version altogether?

[neilmacintosh]

@KrisTC I asked around and the "Report a button" problem was a known issue with VS 2017 RC, I'm sorry to hear you hit that. I believe that re-installing the latest version should fix it, so that you can let us know more directly when things don't work in VS :-)

The final shipping VS 2017 will contain the version of CppCoreCheck with updated const-related checks (as well as a bunch of bugfixes for the existing checks). Hopefully you'll find them much quieter and more accurate. I'm afraid that I can't share release dates for the final product, or I would spoil the fun for the product managers I work with. ;-)

I haven't made a final decision on how we'll ship the lifetime-related checks yet. It seems most likely that they'll ship (at least at first) as a drop-in addition to VS 2017. This would be similar (in spirit, if not execution) to the way we initially released CppCoreCheck. Once those checks are stable and we've had a chance to hear user feedback, then they'll make their way into CppCoreCheck itself as part of a VS release. I honestly couldn't tell you whether that would be an update or full release at this stage.

[neilmacintosh]

I am no longer the right person to talk to about CppCoreCheck, but @annagrin might be interested in updating/closing this thread.