[libcxx-commits] [PATCH] D111509: [clang] use getCommonSugar in an assortment of places

[Richard Smith - zygoloid via Phabricator via libcxx-commits]

rsmith accepted this revision.
rsmith added a comment.

Looks good to me.



================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:6504-6516
     // If we have function pointer types, unify them anyway to unify their
     // exception specifications, if any.
     if (LTy->isFunctionPointerType() || LTy->isMemberFunctionPointerType()) {
       Qualifiers Qs = LTy.getQualifiers();
       LTy = FindCompositePointerType(QuestionLoc, LHS, RHS,
                                      /*ConvertArgs*/false);
       LTy = Context.getQualifiedType(LTy, Qs);
----------------
Can we remove this `if` now? `getCommonSugaredType` should unify the exception specifications so I think this stops being a special case.


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:6565-6572
     // If we have function pointer types, unify them anyway to unify their
     // exception specifications, if any.
     if (LTy->isFunctionPointerType() || LTy->isMemberFunctionPointerType()) {
       LTy = FindCompositePointerType(QuestionLoc, LHS, RHS);
       assert(!LTy.isNull() && "failed to find composite pointer type for "
                               "canonically equivalent function ptr types");
+      return LTy;
----------------
Likewise here, can we remove this `if`?


================
Comment at: clang/test/SemaObjC/format-strings-objc.m:271
 void testTypeOf(NSInteger dW, NSInteger dH) {
-  NSLog(@"dW %d  dH %d",({ __typeof__(dW) __a = (dW); __a < 0 ? -__a : __a; }),({ __typeof__(dH) __a = (dH); __a < 0 ? -__a : __a; })); // expected-warning 2 {{format specifies type 'int' but the argument has type 'long'}}
+  NSLog(@"dW %d  dH %d",({ __typeof__(dW) __a = (dW); __a < 0 ? -__a : __a; }),({ __typeof__(dH) __a = (dH); __a < 0 ? -__a : __a; })); // expected-warning 2 {{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
 }
----------------
Not related to this patch, but this new diagnostic is in some ways worse than the old one: casting to `long` doesn't fix the problem, given that the format specifier here `%d` expects an `int`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111509