fcloutier added inline comments.

================
Comment at: clang/test/SemaCXX/attr-format.cpp:76
+  format("bare string");
+  format("%s", 123); // expected-warning{{format specifies type 'char *' but 
the argument has type 'int'}}
+  format("%s %s %u %d %i %p\n", "hello", s, 10u, x, y, &do_format);
----------------
aaron.ballman wrote:
> This pointed out an interesting test case. What should the behavior be for:
> ```
> format("%p", 0);
> ```
> Because that sure feels like a more reasonable thing for someone to write 
> expecting it to be treated as a null pointer constant.
I think that the current behavior is the right one:

```
test.c:4:17: warning: format specifies type 'void *' but the argument has type 
'int' [-Wformat]
        printf("%p\n", 0);
                ~~     ^
                %d
```

The warning goes away if you use `(void *)0`, as expected. 
`__attribute__((format))` has no semantic meaning, so we can't (and shouldn't) 
infer that 0 is a pointer based on the usage of %p.


================
Comment at: clang/test/SemaCXX/attr-format.cpp:77-78
+  format("%s", 123); // expected-warning{{format specifies type 'char *' but 
the argument has type 'int'}}
+  format("%s %s %u %d %i %p\n", "hello", s, 10u, x, y, &do_format);
+  format("%s %s %u %d %i %p\n", "hello", s, 10u, x, y, do_format);
+  format("bad format %s"); // expected-warning{{more '%' conversions than data 
arguments}}
----------------
aaron.ballman wrote:
> This likely isn't specific to your changes, but the `%p` in these examples 
> should be warning the user (a function or function pointer is not a pointer 
> to void or a pointer to a character type, so that call is UB).
This is already a -Wformat-pedantic warning, which IMO is the right warning 
group for it:

```
test.c:4:17: warning: format specifies type 'void *' but the argument has type 
'int (*)()' [-Wformat-pedantic]
        printf("%p\n", main);
                ~~     ^~~~
1 warning generated.
```

The relevant bit is clang/lib/AST/FormatString.cpp:

```
    case CPointerTy:
      if (argTy->isVoidPointerType()) {
        return Match;
      } if (argTy->isPointerType() || argTy->isObjCObjectPointerType() ||
            argTy->isBlockPointerType() || argTy->isNullPtrType()) {
        return NoMatchPedantic;
      } else {
        return NoMatch;
      }
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112579

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to