[clang] [analyzer] Wrap SymbolicRegions by ElementRegions before getting a FieldRegion (PR #85211)

[via cfe-commits]

================
@@ -226,6 +226,20 @@ ProgramStateRef ProgramState::killBinding(Loc LV) const {
   return makeWithStore(newStore);
 }
 
+/// FieldRegions are expected to be wrapped by an ElementRegion as a canonical
+/// representation. See f8643a9b31c4029942f67d4534c9139b45173504 why.
+SVal ProgramState::wrapSymbolicRegion(SVal Base) const {
+  const auto *SymbolicBase =
+      dyn_cast_or_null<SymbolicRegion>(Base.getAsRegion());
+
+  if (!SymbolicBase)
+    return Base;
+
+  StoreManager &SM = getStateManager().getStoreManager();
+  QualType ElemTy = SymbolicBase->getPointeeStaticType();
+  return loc::MemRegionVal{SM.GetElementZeroRegion(SymbolicBase, ElemTy)};
+}
----------------
NagyDonat wrote:

I have several minor issues with the name choices:
- `wrapSymbolicRegion` sounds like a method that expects a symbolic region as an argument and always wraps it; I think the effects of this method are better represented by something like `wrapRegionIfSymbolic`.
- For me, the variable name `SymbolicBase` was confusing because it strongly suggested a connection with `MemRegion::getSymbolicBase()`. ("Why are we taking the symbolic base here? Oh, this isn't the symbolic base region of a region, this is just the region "base" if it happens to be a symbolic region...")
- The name "Base" is also a bit unfortunate, because in the context of this method it's not the "base" of anything. I'd prefer to use something more generic like `SV` or `Val` -- or perhaps use a method name like `wrapBaseIfSymbolic` to indicate that the purpose of this method is wrapping a base region.

https://github.com/llvm/llvm-project/pull/85211