martong accepted this revision.
martong added a comment.
In D111542#3074926 <https://reviews.llvm.org/D111542#3074926>, @ASDenysPetrov
wrote:
> Updated according to the latest suggestions.
> @martong I think now this patch is way more simple and clear than before.
Thanks, indeed! LGTM (with nits about `VD`)!
================
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;
----------------
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.
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
