aaron.ballman added subscribers: jcranmer-intel, zahiraam.
aaron.ballman added inline comments.
================
Comment at: clang/lib/AST/FormatString.cpp:484-487
+ if (const auto *PT = argTy->getAs<PointerType>()) {
+ if (PT->getPointeeType()->isCharType())
+ return Match;
+ }
----------------
We can simplify a bit further these days.
================
Comment at: clang/lib/AST/FormatString.cpp:522-534
+ if (const auto *PT = argTy->getAs<PointerType>()) {
+ QualType pointeeTy = PT->getPointeeType();
+ if (pointeeTy->isVoidType() || (!Ptr && pointeeTy->isCharType()))
+ return Match;
+ return NoMatchPedantic;
+ } else if (argTy->isNullPtrType()) {
+ return MatchPromotion;
----------------
Fixes to match the usual coding style rules.
Wow, Phab's diff engine struggles with that edit. Basically: no `else` after a
`return`, remove braces on single-line `if`, renamed to `PointeeTy`.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:1062-1072
+ // C2x 6.5.2.2p6:
+ // The integer promotions are performed on each trailing argument, and
+ // trailing arguments that have type float are promoted to double. These
are
+ // called the default argument promotions. No other conversions are
+ // performed implicitly.
+
+ // C++ [expr.call]p7, per DR722:
----------------
I think this code should live in `Sema::DefaultArgumentPromotion()` because
it's related specifically to default argument promotions:
https://eel.is/c++draft/expr#call-11.sentence-5
================
Comment at: clang/lib/Sema/SemaExpr.cpp:17317-17319
+ if (TInfo->getType()->isSpecificBuiltinType(BuiltinType::Float) ||
+ TInfo->getType()->isSpecificBuiltinType(BuiltinType::Half))
PromoteType = Context.DoubleTy;
----------------
Hmmm... the existing code seems wrong to me because it's not paying any
attention to `FLT_EVAL_METHOD`, but I think it probably should? CC
@jcranmer-intel @zahiraam for opinions.
Actually, I wonder if the correct approach here is to split
`Sema::DefaultArgumentPromotion()` up so that we can calculate what the default
argument promoted type is of the expression independent of performing the
actual promotion, and call the promotion type calculation logic here?
================
Comment at: clang/test/Sema/format-pointer.c:39
+ printf("%p", np);
+ scanf("%p", &np); // expected-warning {{format specifies type 'void **' but
the argument has type 'nullptr_t *'}}
+ scanf("%p", &npp); // pedantic-warning {{format specifies type 'void **'
but the argument has type 'nullptr_t **'}}
----------------
MitalAshok wrote:
> Should this be a pedantic warning?
>
> If this reads a non-null pointer, the bit pattern of `np` will be messed up
> but `np` will still read as a nullptr.
> If it reads a null pointer, it should be fine.
>
That's a fun scenario, good test case!
C23 7.26.6.2p10: Except in the case of a % specifier, the input item (or, in
the case of a %n directive, the count of input characters) is converted to a
type appropriate to the conversion specifier. If the input item is not a
matching sequence, the execution of the directive fails: this condition is a
matching failure. Unless assignment suppression was indicated by a *, the
result of the conversion is placed in the object pointed to by the first
argument following the format argument that has not already received a
conversion result. If this object does not have an appropriate type, or if the
result of the conversion cannot be represented in the object, the behavior is
undefined.
p12, the p specifier: Matches an implementation-defined set of sequences, which
should be the same as the set of sequences that may be produced by the %p
conversion of the fprintf function. The corresponding argument shall be a
pointer to a pointer of void. The input item is converted to a pointer value in
an implementation-defined manner. If the input item is a value converted
earlier during the same program execution, the pointer that results shall
compare equal to that value; otherwise the behavior of the %p conversion is
undefined.
The corresponding argument is not a pointer to a pointer of void, so it's UB,
not just pedantically so.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156054/new/
https://reviews.llvm.org/D156054
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
