aaron.ballman added a comment. In D149904#4321144 <https://reviews.llvm.org/D149904#4321144>, @cor3ntin wrote:
> I observe there is no documentation or release notes yet :) Those are coming up, I wanted to see if the RFC made any substantive changes to the design before doing the documentation for the extension. ================ Comment at: clang/lib/Sema/SemaExpr.cpp:19851 + bool IsExpr = GSE->isExprPredicate(); + if (IsExpr) + ExOrTy = GSE->getControllingExpr(); ---------------- aaron.ballman wrote: > erichkeane wrote: > > aaron.ballman wrote: > > > erichkeane wrote: > > > > I find myself wondering if a `getControllingOperand` here is valuable? > > > This pattern only comes up once, so I don't think it's worth it, but it's > > > certainly easy enough to add if you disagree. > > Yep, I think I agree, after seeing the rest of the review, I don't see as > > much value. > > > > On this re-read, I kind of which we instead had 2 separate 'create' > > functions here, rather than going through void*, so that this becomes: > > > > ``` > > if (IsExpr) > > return AnyChanged ? S.CreateGenericSelectionExpr(...., > > GSE->getControllingExpr(), ...) : ExprEmpty(); > > > > return AnyChanged ? S.CreateGenericSelectionExpr(...., > > GSE->getControllingType(), ...) : ExprEmpty(); > > ``` > > > > The `void*` version gives me the jeebies (and is perhaps a 'worse' > > interface? Particularly for libclang folks?). > That makes this one use better but at the expense of making that create > function rather awkward. I took another run at this and decided to put a FIXME comment in the header file. I agree that the interface is a bit awkward, but it's also pretty hard to make work in `ActOnGenericSelectionExpr` due to it taking an `Expr *` or a `ParsedType` (currently it takes a `void *` of the parsed type's opaque value). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149904/new/ https://reviews.llvm.org/D149904 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
