Which casting style should we pursue?
We already use the
dyn_cast style with other analyzer-specific types, such as
SymExprs. And I’m about to land the by-value
SVals in D125709.
At this point, the
castAs member functions would became redundant.
We should consider dropping them in favor of
cast<T> to make it more consistent with how we cast
Alternatively, we could introduce
SymExpr::getAs<T>() just making a syntactic sugar for
dyn_cast and prefer using those.
IMO this is worth a discussion since consistency is important.
Unfortunately, we currently don’t have an
isa<Ts...>() member function variant, but if we choose the member function style D125749 implements it.
In addition to this, I would prefer not to spell out the type in the following scenarios.
So I would prefer this:
auto Var = dyn_cast<T>(V);
auto Var = cast<T>(V);
Compared to this:
Optional<T> Var = dyn_cast<T>(V); // or V.getAs<T>()
T Var = cast<T>(V); // or V.castAs<T>()
What’s your opinion?
@NoQ @Xazax-hun @martong @ASDenysPetrov @Szelethus
Some prior discussion in the area: ⚙ D33672 [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker
SVal class is pretty unique in how it combines non-trivial inheritance hierarchy with pass-by-value. This is only possible because all subclasses are of the same size (in rare cases when we run over the limit, out-of-line storage is used). This is a very unusual situation and I don’t think consistency can be expected.
dyn_cast<>() are used by other classes but they have completely different meaning there. In particular,
SVals are supposed to be copied by these functions, whereas other classes aren’t.
I don’t see much benefit in transitioning from one to the other; they’ll still have nothing to do with actual functions with the same name in other classes. I’d rather have them have completely different names to attract attention to different semantics.
If we really want this, I’d advocate for having
castAs() as member functions and
dyn_cast() as global functions, because this matches other classes’ behavior.
I think it would be great to have a uniform way of casting in the CSA codebase. And I vote for the traditional global
isa<T>, cast<T> and
My understanding is that D125709 implements these functions in a way that they do copy. I agree that the cast-by-value thing is unique in the llvm-project code-base, however, it is still a cast operation. And as such, perhaps it should follow the traditional LLVM casting conventions.