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