steakhal marked 3 inline comments as done.
steakhal added inline comments.

================
Comment at: clang/lib/StaticAnalyzer/Core/MemRegion.cpp:978
+    assert(!Ty.isNull());
+    if (Ty.isConstQualified() && Ty->isArithmeticType()) {
       // TODO: We could walk the complex types here and see if everything is
----------------
xazax.hun wrote:
> steakhal wrote:
> > xazax.hun wrote:
> > > Why do we need this extra condition? I see that this was the original 
> > > behavior, just wondering if we actually know. I think there are more 
> > > types than arithmetic that can be stored in the read only memory.
> > Well, seems reasonable. Do you think I should remove it here or in a 
> > separate patch?
> Let's keep it separate, so it can be reverted individually. Just in case we 
> find out the reason the hard way :)
D127763.


================
Comment at: clang/lib/StaticAnalyzer/Core/MemRegion.cpp:979-980
+    if (Ty.isConstQualified() && Ty->isArithmeticType()) {
       // TODO: We could walk the complex types here and see if everything is
       // constified.
+      sReg = getGlobalsRegion(MemRegion::GlobalImmutableSpaceRegionKind);
----------------
steakhal wrote:
> martong wrote:
> > Is this comment still meaningful? I don't think so because if the type is 
> > const, then it does not matter what sub-types it has.
> Makes sense. However, I'm not sure. Is it the case @xazax.hun?
D127763


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127306

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

Reply via email to