ariccio marked 3 inline comments as done.
ariccio added a comment.
I will remove that `const` later tonight.
================
Comment at: C:/LLVM/llvm/tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp:1396
@@ -1395,3 +1395,3 @@
const auto &ReferencedBlockVars = AC->getReferencedBlockVars(BC->getDecl());
- auto NumBlockVars =
+ const auto NumBlockVars =
std::distance(ReferencedBlockVars.begin(), ReferencedBlockVars.end());
----------------
aaron.ballman wrote:
> ariccio wrote:
> > ariccio wrote:
> > > aaron.ballman wrote:
> > > > No need for a const here; the correct change is to not use auto (here
> > > > and the line above), but instead spell out the type explicitly.
> > > The const is partly to make sure that the `if (NumBlockVars == 0) {` line
> > > never accidentally becomes `if (NumBlockVars = 0) {` like it did in
> > > CPython:
> > >
> > > http://bugs.python.org/issue25844
> > For archival reasons, I'll copy & paste the relevant diff here (I hate dead
> > links):
> >
> >
> > ```
> > --- a/PC/launcher.c
> > +++ b/PC/launcher.c
> > @@ -114,7 +114,7 @@ static wchar_t * get_env(wchar_t * key)
> > if (result >= BUFSIZE) {
> > /* Large environment variable. Accept some leakage */
> > wchar_t *buf2 = (wchar_t*)malloc(sizeof(wchar_t) * (result+1));
> > - if (buf2 = NULL) {
> > + if (buf2 == NULL) {
> > error(RC_NO_MEMORY, L"Could not allocate environment buffer");
> > }
> > GetEnvironmentVariableW(key, buf2, result);
> > ```
> While this form of bug can certainly crop up, it's still a bridge-too-far for
> this project, as I understand it our de facto guidelines on this. I am not
> certain that we want to start sprinkling const onto value types (as opposed
> to reference and pointer types) at this point. If we do, it should certainly
> be something handled a bit more consistently and actively than a general
> clean-up related to unnecessary type casting.
> If we do, it should certainly be something handled a bit more consistently
> and actively than a general clean-up related to unnecessary type casting.
You've got a good point there. I'll upload a final revision without that
`const` a bit later tonight.
http://reviews.llvm.org/D16748
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
