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

[Nick Desaulniers via Phabricator via cfe-commits]

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