[PATCH] D16748: Cleanup MemRegion.cpp/MemRegion.h

[Alexander Riccio via cfe-commits]

ariccio marked an inline comment as done.
ariccio added a comment.

As said in comment, I disagree with the no need for `const` here.


================
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:
> 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

================
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());
----------------
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);
```


http://reviews.llvm.org/D16748