Coding style: `isa<> && cast<>` pattern

LLVM Project
1

The isa<> && cast<> pattern seems to be discouraged by the coding style:

https://llvm.org/docs/ProgrammersManual.html#the-isa-cast-and-dyn-cast-templates

Quote:
Note that you should not use an isa<> test followed by a cast<>, for that use the dyn_cast<> operator.

But it’s found in the codebase in patterns like this:

return isa<Foo>(F) && cast<Foo>(F)->somePredicate();

where the dyn_cast equivalent would be more verbose, perhaps something like:

if (auto *FooF = dyn_cast<Foo>(F))
  return FooF->somePredicate();
return false;

The isa<> && cast<> pattern is particularly convenient in longer chains, like:

return someCondition() ||
       (isa<Foo>(F) && cast<Foo>(F)->somePredicate()) ||
       someOtherCondition();

Where the equivalent code that uses dyn_cast would look more verbose and not as easy to read IMO, for example like:

if (someCondition())
   return true;
if (auto *FooF = dyn_cast<Foo>(F))
   if (FooF->somePredicate())
      return true;
return someOtherCondition();

Does/should the coding style allow the isa<>() && cast<>() pattern?

2

Declaring FooF in outer scope gives you

return ((FooF = dyn_cast(F)) && FooF->somePredicate());

3

The reason not to allow isa<> && cast<> is that cast basically repeats the isa operation (and asserts if it fails, which in this case of course it won’t). But still, it’s more efficient to use dyn_cast so the (implicit) isa happens only once.

4

Interesting – do the compilers these day can optimize out the second redundant typecheck? And if not, is there something to be done, so both assert and second isa<> would be optimized out?

5

This is true in assert builds, but is there the same redundancy in release / non-assert builds?

6

From the programming manual:

Typically, the dyn_cast<> operator is used in an if statement or some other flow control statement like this:

if (auto *AI = dyn_cast<AllocationInst>(Val)) {
  // ...
}
This form of the if statement effectively combines together a call to isa<> and a call to cast<> into one statement, which is very convenient.
7

I think, like for all rules in practice, there are exceptions. I used this a couple of times and I don’t feel guilty for doing so.

Although, frequently introducing a variable for the result of the dyncast and then have a ptr && ptr->foo() usually leads to more readable code.

Personally, I consider the coding style as a style guide. You should follow it unless you have strong opinion diverging with objective benefit for doing so.

8

In some simple cases, it can: