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

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.

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


http://reviews.llvm.org/D16748