[PATCH] Fix fallout from r219557

[Richard Smith]

On Wed, Nov 12, 2014 at 4:10 PM, Anton Korobeynikov <anton at korobeynikov.info
> wrote:

> Ok, the reason for objc tests to fail is that we're loosing typedefs when
> using the type from UsualArithmeticConversions().
>
> Consider the following code:
>
> ```
> typedef long NSInteger;
>
> int printf(const char * restrict, ...);
>
> void foo(void) {
>   NSInteger i = 0;
>   printf("%s", i ? i : i);
> }
> ```
>
> AST before was:
>
> ```
>      `-ConditionalOperator 0x7f9d1086e978 <col:16, col:24>
> 'NSInteger':'long'
>         |-ImplicitCastExpr 0x7f9d1086e930 <col:16> 'NSInteger':'long'
> <LValueToRValue>
>         | `-DeclRefExpr 0x7f9d1086e8b8 <col:16> 'NSInteger':'long' lvalue
> Var 0x7f9d1086e780 'i' 'NSInteger':'long'
>         |-ImplicitCastExpr 0x7f9d1086e948 <col:20> 'NSInteger':'long'
> <LValueToRValue>
>         | `-DeclRefExpr 0x7f9d1086e8e0 <col:20> 'NSInteger':'long' lvalue
> Var 0x7f9d1086e780 'i' 'NSInteger':'long'
>         `-ImplicitCastExpr 0x7f9d1086e960 <col:24> 'NSInteger':'long'
> <LValueToRValue>
>           `-DeclRefExpr 0x7f9d1086e908 <col:24> 'NSInteger':'long' lvalue
> Var 0x7f9d1086e780 'i' 'NSInteger':'long'
> ```
>
> AST now is:
>
> ```
>       `-ConditionalOperator 0x7ff93a01c178 <col:16, col:24> 'long'
>         |-ImplicitCastExpr 0x7ff93a01c130 <col:16> 'NSInteger':'long'
> <LValueToRValue>
>         | `-DeclRefExpr 0x7ff93a01c0b8 <col:16> 'NSInteger':'long' lvalue
> Var 0x7ff93a01bf80 'i' 'NSInteger':'long'
>         |-ImplicitCastExpr 0x7ff93a01c148 <col:20> 'NSInteger':'long'
> <LValueToRValue>
>         | `-DeclRefExpr 0x7ff93a01c0e0 <col:20> 'NSInteger':'long' lvalue
> Var 0x7ff93a01bf80 'i' 'NSInteger':'long'
>         `-ImplicitCastExpr 0x7ff93a01c160 <col:24> 'NSInteger':'long'
> <LValueToRValue>
>           `-DeclRefExpr 0x7ff93a01c108 <col:24> 'NSInteger':'long' lvalue
> Var 0x7ff93a01bf80 'i' 'NSInteger':'long'
> ```
>
> Note that the type of ConditionalOperator changed. Is this expected?


It's not especially surprising, but not ideal. If the two inputs had the
same type sugar, we should probably preserve that sugar. If we had

  NSInteger a; long b;
  auto x = cond ? a : b;

... it's less clear that we should preserve the sugar. (We used to preserve
the sugar from the LHS, which is rather broken.)

I think it'd be better if CheckPrintfHandler walked into the operands of a
?: to check if they're both NSInteger, if it really wants to care about
type sugar.

The special (darwin) case which looks for typedef is inside
> CheckPrintfHandler::checkFormatExpr()
>
> http://reviews.llvm.org/D6217
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141112/da7e43ed/attachment.html>