nickdesaulniers added inline comments.
================
Comment at: clang/lib/AST/FormatString.cpp:401
+ if (const auto *BT = argTy->getAs<BuiltinType>()) {
+ if (!Ptr) {
+ switch (BT->getKind()) {
----------------
inclyc wrote:
> nickdesaulniers wrote:
> > aaron.ballman wrote:
> > > It's a bit strange that we have two switches over the same
> > > `BT->getKind()` and the only difference is `!Ptr`; would it be easier to
> > > read if we combined the two switches into one and had logic in the
> > > individual cases for `Ptr` vs not `Ptr`?
> > I almost made the same recommendation myself. For the below switch pair,
> > and the pair above.
> > It's a bit strange that we have two switches over the same `BT->getKind()`
> > and the only difference is `!Ptr`; would it be easier to read if we
> > combined the two switches into one and had logic in the individual cases
> > for `Ptr` vs not `Ptr`?
>
> These two switch pairs have different functions. The lower one is only
> responsible for checking whether there is a signed or unsigned integer, and
> the upper one is checking whether there is a promotion (or type confusing).
> Will they be more difficult to understand if they are written together?
Perhaps. I think the comments you added to all switches are helpful!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132568/new/
https://reviews.llvm.org/D132568
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
