[PATCH] D132568: [clang][Sema] check default argument promotions for printf

[YingChi Long via Phabricator via cfe-commits]

inclyc added inline comments.


================
Comment at: clang/lib/AST/FormatString.cpp:401
+      if (const auto *BT = argTy->getAs<BuiltinType>()) {
+        if (!Ptr) {
+          switch (BT->getKind()) {
----------------
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? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132568/new/

https://reviews.llvm.org/D132568