[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/Sema/SemaChecking.cpp:10127-10129
+    if (ImplicitMatch != ArgType::NoMatchPromotionTypeConfusion &&
+        ImplicitMatch != ArgType::NoMatchTypeConfusion &&
+        !IsCharacterLiteralInt)
----------------
There's something about this conditional...I can't quite put my finger on.

Isn't `ImplicitMatch` only updated in the `const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E)` branch above, where `IsCharacterLiteralInt` cannot be?

Simultaneously, isn't `IsCharacterLiteralInt` only updated in the `const CharacterLiteral *CL = dyn_cast<CharacterLiteral>(E)` branch above, where `ImplicitMatch` cannot be?

I wonder if we should instead hoist:
```
if (Match == ArgType::MatchPromotion) {
  if (ImplicitMatch != ArgType::NoMatchPromotionTypeConfusion &&
      ImplicitMatch != ArgType::NoMatchTypeConfusion)
    return true;
  Match = ArgType::NoMatch;
}
```
into the `const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E)` branch after `ImplicitMatch` is updated.

Then hoist
```
if (Match == ArgType::MatchPromotion)
  return true;
```
into the `const CharacterLiteral *CL = dyn_cast<CharacterLiteral>(E)` branch after `IsCharacterLiteralInt` is updated? Then we could delete the `IsCharacterLiteralInt` variable perhaps?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132568