[clang] [clang][dataflow] Add `Environment::get<>()`. (PR #76027)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 20 23:40:47 PST 2023
================
@@ -489,8 +489,7 @@ Environment Environment::pushCall(const CallExpr *Call) const {
if (const auto *MethodCall = dyn_cast<CXXMemberCallExpr>(Call)) {
if (const Expr *Arg = MethodCall->getImplicitObjectArgument()) {
if (!isa<CXXThisExpr>(Arg))
- Env.ThisPointeeLoc =
- cast<RecordStorageLocation>(getStorageLocation(*Arg));
+ Env.ThisPointeeLoc = get<RecordStorageLocation>(*Arg);
----------------
martinboehme wrote:
> should we check the result here, since the original code had a plain cast?
Thanks, good catch!
I've chosen to revert this for the moment, as I'm not sure `get<>(); asssert();` is better than the existing `cast<>(getStorageLocation())`. If anything, we probably want an additional convenience function for this (see below).
> +1, alternatively we could introduce a bool template argument that would determine whether `null` is allowed with a default value.
Yes, we probably want some mechanism to express the combined effect of `cast<>(getStorageLocation())` too (i.e. disallowing null).
I'm not sure a bool argument is the best choice though -- as it's often hard to tell at the callsite what it means:
```cxx
auto *Val = Env.get<PointerValue, true /* 🤔 */>(E);
```
As an alternative, we could use an enum with a more self-documenting name, but at that point we could also simply introduce a different routine. I haven't yet found a great name for this that I'm satisfied with -- `getMustExist()` is the best I can come up with, but it's long and clumsy. Any suggestions?
Anyway, I'll leave this for future work.
https://github.com/llvm/llvm-project/pull/76027
More information about the cfe-commits
mailing list