ASDenysPetrov added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1649-1653
+  // NOTE: `VD` is always non-null if `Init` is non-null, so we can check for
+  // null only one of them.
+  const Expr *Init = VD->getAnyInitializer(VD);
   if (!Init)
     return None;
----------------
martong wrote:
> steakhal wrote:
> > Wait. But if `VD` is null, you get a null-dereference.
> > But you already dereferenced `VD` multiple times, so it cannot be null.
> > 
> > Oh, but the `getAnyInitializer()` will overwrite it! That's a surprise.
> > TBH I would rather pass a fresh uninitialized pointer if you really need 
> > the exact decl which actually provided the initialized expression to make 
> > this behavior explicit.
> > 
> > That way, with a properly chosen name you could spare the NOTE comment as 
> > well.
> Yes I agree, probably it is easier to follow the code if you make it explicit 
> that `VD` is overwritten here.
> Wait. But if VD is null, you get a null-dereference.
We have an assertion above.
> Oh, but the getAnyInitializer() will overwrite it! That's a surprise.
So was for me :) I'll add a NOTE in the comment.


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

https://reviews.llvm.org/D111542

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to