[clang] Clean up array allocation logic in Sema::BuildCXXNew (PR #186617)

Oliver Hunt via cfe-commits cfe-commits at lists.llvm.org
Sun Mar 15 12:49:32 PDT 2026


ojhunt wrote:

> > I don't believe this is a good change sorry - it's a relatively large abstraction layer to "simplify" maybe two actual expressions.
> > If we really wanted to do something here I would probably just replace the optional<> with
> > ```c++
> > bool IsArraySizeDependent = false;
> > bool ... = false;
> > if (ArraySize) {
> >   set all the "there is an array size" variables as appropriate
> > }
> > ```
> >   
> > It seems the main concern is the compound conditions, which is consistent with what other code does, but I would prefer to reduce that in general.
> > But the issue here is not made better with an abstraction (the better option would be actual language support for optional in c++)
> > I've marked as "request changes" but I think this is probably the wrong tack, and I'm not sure the problem with new/delete Sema is this bit. There's so much more that is so much worse.
> > While not an option for clang any time soon, I have wondered if reflection could be used in a way to make this kind of thing more expensive.
> 
> Many thanks for the thorough review. And from my side, same comments I did to @tbaederr. You're very right that this current code is overkill. I will explore the variant solution.

Have a quick look at just turning the various checks into a set of bools first and see what that does to the overall appearance. I'll try to refresh exactly what it was I didn't like about array size specifically as there's a lot of clunkiness in this code in general (I really wish I put down more information at the time)

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


More information about the cfe-commits mailing list