[PATCH] D137933: [llvm-debuginfo-analyzer] 08a - Memory Management

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando added inline comments.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVType.cpp:261
+      else
+        TypesParam->push_back(Type);
+    }
----------------
CarlosAlbertoEnciso wrote:
> Orlando wrote:
> > Before your recent change this branch was taken if `options().getAttributeArgument()` is false, but now this branch is taken if `options().getAttributeArgument()` is true (and both `Type->getIsKindType()`  and `Type->getIsKindScope())` are false). Just checking - is the new behaviour correct?
> > 
> > And if so, what was the result of the previous incorrect behaviour? 
> Very good catch. The behaviour has not being changed.
> 
> Incorrec placement for the closing `}`. The code should be:
> 
> ```
>     if (options().getAttributeArgument()) {
>       if (Type->getIsKindType())
>         TypesParam->push_back(Type->getTypeAsType());
>       else if (Type->getIsKindScope())
>         ScopesParam->push_back(Type->getTypeAsScope());
>     } else
>       TypesParam->push_back(Type);
> ```
nit-pick: when you make the change, please add a set of braces around the final else-block (to keep it uniform since the if-block does use braces (covered [[ https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements | here ]]).

Thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137933/new/

https://reviews.llvm.org/D137933