================
@@ -508,6 +508,11 @@ class ResultObjectVisitor : public 
RecursiveASTVisitor<ResultObjectVisitor> {
         isa<CXXStdInitializerListExpr>(E)) {
       return;
     }
+    if (auto *Op = dyn_cast<BinaryOperator>(E);
----------------
martinboehme wrote:

> I guess the `isa` calls were what we really jumped out since they amount to N 
> calls and tests on `getStmtClass` rather than one and a jump. But, 
> readability wins over performance here.

I see what you mean -- yes, the switch statement would likely perform better 
here, but I agree that readability is more important than this 
micro-optimization.

> But, I do wonder about readability win from converting to a switch because it 
> will directly express which cases are covered in this function, rather than 
> leaving it implicit in a series of `if`s. FWIW.

I still have my doubts whether this will really be more readable, but I'll 
explore this. I'd prefer to do this as a separate PR though, as I'd like to 
keep this PR focused on the fix I'm making here. (If I convert the function to 
a switch statement in this PR, those code changes would drown out the few lines 
of actual behavioral change I'm making.)

https://github.com/llvm/llvm-project/pull/88726
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to