[clang] 04e6178 - [Sema] tolerate more promotion matches in format string checking

2023-08-25 Thread Félix Cloutier via cfe-commits 

Author: Félix Cloutier
Date: 2023-08-25T10:14:01-07:00
New Revision: 04e6178ae932c9a1d939dcfe3ef1189f4bbb21aa

URL: 
https://github.com/llvm/llvm-project/commit/04e6178ae932c9a1d939dcfe3ef1189f4bbb21aa
DIFF: 
https://github.com/llvm/llvm-project/commit/04e6178ae932c9a1d939dcfe3ef1189f4bbb21aa.diff

LOG: [Sema] tolerate more promotion matches in format string checking

It's been reported that when using __attribute__((format)) on non-variadic
functions, certain values that normally get promoted when passed as variadic
arguments now unconditionally emit a diagnostic:

```c
void foo(const char *fmt, float f) __attribute__((format(printf, 1, 2)));
void bar(void) {
foo("%g", 123.f);
//   ^ format specifies type 'double' but the argument has type 'float'
}
```

This is normally not an issue because float values get promoted to doubles when
passed as variadic arguments, but needless to say, variadic argument promotion
does not apply to non-variadic arguments.

While this can be fixed by adjusting the prototype of `foo`, this is sometimes
undesirable in C (for instance, if `foo` is ABI). In C++, using variadic
templates, this might instead require call-site fixing, which is tedious and
arguably needless work:

```c++
template
void foo(const char *fmt, Args &&...args) __attribute__((format(printf, 1, 2)));
void bar(void) {
foo("%g", 123.f);
//   ^ format specifies type 'double' but the argument has type 'float'
}
```

To address this issue, we teach FormatString about a few promotions that have
always been around but that have never been exercised in the direction that
FormatString checks for:

* `char`, `unsigned char` -> `int`, `unsigned`
* `half`, `float16`, `float` -> `double`

This addresses issue https://github.com/llvm/llvm-project/issues/59824.

Added: 
clang/test/SemaCXX/format-strings-scanf.cpp

Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/AST/FormatString.cpp
clang/test/Sema/attr-format.c
clang/test/SemaCXX/attr-format.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f0e601fc2df88b..8580b2ccb20c24 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -128,6 +128,13 @@ Removed Compiler Flags
 Attribute Changes in Clang
 --
 
+- When a non-variadic function is decorated with the ``format`` attribute,
+  Clang now checks that the format string would match the function's 
parameters'
+  types after default argument promotion. As a result, it's no longer an
+  automatic diagnostic to use parameters of types that the format style
+  supports but that are never the result of default argument promotion, such as
+  ``float``. (`#59824: `_)
+
 Improvements to Clang's diagnostics
 ---
 - Clang constexpr evaluator now prints template arguments when displaying

diff  --git a/clang/lib/AST/FormatString.cpp b/clang/lib/AST/FormatString.cpp
index ad5af9508983fe..f86278e4b5163d 100644
--- a/clang/lib/AST/FormatString.cpp
+++ b/clang/lib/AST/FormatString.cpp
@@ -458,6 +458,10 @@ ArgType::matchesType(ASTContext , QualType argTy) const {
 switch (BT->getKind()) {
 default:
   break;
+case BuiltinType::Bool:
+  if (T == C.IntTy || T == C.UnsignedIntTy)
+return MatchPromotion;
+  break;
 case BuiltinType::Int:
 case BuiltinType::UInt:
   if (T == C.SignedCharTy || T == C.UnsignedCharTy ||
@@ -465,6 +469,24 @@ ArgType::matchesType(ASTContext , QualType argTy) const {
   T == C.WideCharTy)
 return MatchPromotion;
   break;
+case BuiltinType::Char_U:
+  if (T == C.UnsignedIntTy)
+return MatchPromotion;
+  if (T == C.UnsignedShortTy)
+return NoMatchPromotionTypeConfusion;
+  break;
+case BuiltinType::Char_S:
+  if (T == C.IntTy)
+return MatchPromotion;
+  if (T == C.ShortTy)
+return NoMatchPromotionTypeConfusion;
+  break;
+case BuiltinType::Half:
+case BuiltinType::Float16:
+case BuiltinType::Float:
+  if (T == C.DoubleTy)
+return MatchPromotion;
+  break;
 case BuiltinType::Short:
 case BuiltinType::UShort:
   if (T == C.SignedCharTy || T == C.UnsignedCharTy)

diff  --git a/clang/test/Sema/attr-format.c b/clang/test/Sema/attr-format.c
index d891828a77a4c6..9cc6b5482144a7 100644
--- a/clang/test/Sema/attr-format.c
+++ b/clang/test/Sema/attr-format.c
@@ -94,13 +94,9 @@ void call_nonvariadic(void) {
   d3("%s", 123); // expected-warning{{format specifies type 'char *' but the

[clang] cd95d79 - [Clang][Sema] Fix attribute((format)) bug on non-variadic functions

2022-12-06 Thread Félix Cloutier via cfe-commits 

Author: Félix Cloutier
Date: 2022-12-06T13:08:18-08:00
New Revision: cd95d7998c1dd30c6353aeca2686697287cb0443

URL: 
https://github.com/llvm/llvm-project/commit/cd95d7998c1dd30c6353aeca2686697287cb0443
DIFF: 
https://github.com/llvm/llvm-project/commit/cd95d7998c1dd30c6353aeca2686697287cb0443.diff

LOG: [Clang][Sema] Fix attribute((format)) bug on non-variadic functions

The [initial implementation][1] of __attribute__((format)) on non-variadic 
functions
accidentally only accepted one data argument. This worked:

```c
__attribute__((format(printf, 1, 2)))
void f(const char *, int);
```

but this didn't:

```c
__attribute__((format(printf, 1, 2)))
void f(const char *, int, int);
```

This is due to an oversight in changing the way diagnostics are emitted for
`attribute((format))`, and to a coincidence in the handling of the variadic 
case. Test
cases only covered the case that worked by coincidence.

Before the previous change, using `__attribute__((format))` on a non-variadic 
function at
all was an error and clang bailed out. After that change, it only generates a 
GCC
compatibility warning. However, as execution falls through, it hits a second 
diagnostic
when the first data argument is neither 0 nor the last parameter of the 
function.

This change updates that check to allow any parameter after the format string 
to be the
first data argument when the function is non-variadic. When the function is 
variadic, it
still needs to be the index of the `...` "parameter". Attribute documentation 
is updated
to reflect the change and new tests are added to verify that it works with 
_two_ data
parameters.

[1]: https://reviews.llvm.org/D112579

Radar-Id: rdar://102069446
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D137603

Added: 
clang/test/FixIt/attr-format.c

Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/AttrDocs.td
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/Sema/attr-format.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index adce36bb92363..e3a395481b8c3 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -319,6 +319,8 @@ Bug Fixes
   `Issue 58302 `_
   `Issue 58753 `_
   `Issue 59100 `_
+- Fix issue using __attribute__((format)) on non-variadic functions that expect
+  more than one formatted argument.
 
 Improvements to Clang's diagnostics
 ^^^

diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 40e1dd9cca5c1..a5ea3915a94d0 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3191,6 +3191,18 @@ the function signature. For example:
 fmt(fmt, "hello", 123); // warning: format string is not a string literal
   }
 
+When using the format attribute on a variadic function, the first data 
parameter
+_must_ be the index of the ellipsis in the parameter list. Clang will generate
+a diagnostic otherwise, as it wouldn't be possible to forward that argument 
list
+to `printf`-family functions. For instance, this is an error:
+
+.. code-block:: c
+
+  __attribute__((__format__(__printf__, 1, 2)))
+  void fmt(const char *s, int b, ...);
+  // ^ error: format attribute parameter 3 is out of bounds
+  // (must be __printf__, 1, 3)
+
 Using the ``format`` attribute on a non-variadic function emits a GCC
 compatibility diagnostic.
   }];

diff  --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 4032e96f0f6b5..dcddf4c910738 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3891,27 +3891,38 @@ static void handleFormatAttr(Sema , Decl *D, const 
ParsedAttr ) {
   if (!checkUInt32Argument(S, AL, FirstArgExpr, FirstArg, 3))
 return;
 
-  // check if the function is variadic if the 3rd argument non-zero
+  // FirstArg == 0 is is always valid.
   if (FirstArg != 0) {
-if (isFunctionOrMethodVariadic(D))
-  ++NumArgs; // +1 for ...
-else
-  S.Diag(D->getLocation(), diag::warn_gcc_requires_variadic_function) << 
AL;
-  }
-
-  // strftime requires FirstArg to be 0 because it doesn't read from any
-  // variable the input is just the current time + the format string.
-  if (Kind == StrftimeFormat) {
-if (FirstArg != 0) {
+if (Kind == StrftimeFormat) {
+  // If the kind is strftime, FirstArg must be 0 because strftime does not
+  // use any variadic arguments.
   S.Diag(AL.getLoc(), diag::err_format_strftime_third_parameter)
-<< FirstArgExpr->getSourceRange();
-  return;
+  << FirstArgExpr->getSourceRange()
+  << FixItHint::CreateReplacement(FirstArgExpr->getSourceRange(), "0");
+  return;
+} else

[clang] 92edd74 - Allow non-variadic functions to be attributed with `__attribute__((format))`

2022-07-05 Thread Félix Cloutier via cfe-commits 

Author: Félix Cloutier
Date: 2022-07-05T17:26:11-07:00
New Revision: 92edd74b37c7a96b1d47dc67cda7f92b65066025

URL: 
https://github.com/llvm/llvm-project/commit/92edd74b37c7a96b1d47dc67cda7f92b65066025
DIFF: 
https://github.com/llvm/llvm-project/commit/92edd74b37c7a96b1d47dc67cda7f92b65066025.diff

LOG: Allow non-variadic functions to be attributed with 
`__attribute__((format))`

Clang only allows you to use __attribute__((format)) on variadic functions. 
There are legit use cases for __attribute__((format)) on non-variadic 
functions, such as:

(1) variadic templates

```c++
template
void print(const char *fmt, Args… &) __attribute__((format(1, 2))); // 
error: format attribute requires variadic function
```

(2) functions which take fixed arguments and a custom format:

```c++
void print_number_string(const char *fmt, unsigned number, const char *string) 
__attribute__((format(1, 2)));
// ^error: format attribute requires variadic function

void foo(void) {
print_number_string(“%08x %s\n”, 0xdeadbeef, “hello”);
print_number_string(“%d %s”, 0xcafebabe, “bar”);
}
```

This change allows Clang users to attach __attribute__((format)) to 
non-variadic functions, including functions with C++ variadic templates. It 
replaces the error with a GCC compatibility warning and improves the type 
checker to ensure that received arrays are treated like pointers (this is a 
possibility in C++ since references to template types can bind to arrays).

Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D112579
rdar://84629099

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/AST/FormatString.cpp
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/Sema/attr-format.c
clang/test/Sema/format-strings.c
clang/test/SemaCXX/attr-format.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1be63b539441f..98153fd6e68d8 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -385,6 +385,10 @@ Attribute Changes in Clang
 
 - Added half float to types that can be represented by 
``__attribute__((mode(XX)))``.
 
+- The ``format`` attribute can now be applied to non-variadic functions. The
+  format string must correctly format the fixed parameter types of the 
function.
+  Using the attribute this way emits a GCC compatibility diagnostic.
+
 Windows Support
 ---
 

diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 4e4d871a58a7f..27a3c2f2a69c3 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3088,8 +3088,8 @@ def FormatDocs : Documentation {
   let Content = [{
 
 Clang supports the ``format`` attribute, which indicates that the function
-accepts a ``printf`` or ``scanf``-like format string and corresponding
-arguments or a ``va_list`` that contains these arguments.
+accepts (among other possibilities) a ``printf`` or ``scanf``-like format 
string
+and corresponding arguments or a ``va_list`` that contains these arguments.
 
 Please see `GCC documentation about format attribute
 `_ to find details
@@ -3143,6 +3143,27 @@ Clang implements two kinds of checks with this attribute.
In this case Clang does not warn because the format string ``s`` and
the corresponding arguments are annotated. If the arguments are
incorrect, the caller of ``foo`` will receive a warning.
+
+As an extension to GCC's behavior, Clang accepts the ``format`` attribute on
+non-variadic functions. Clang checks non-variadic format functions for the same
+classes of issues that can be found on variadic functions, as controlled by the
+same warning flags, except that the types of formatted arguments is forced by
+the function signature. For example:
+
+.. code-block:: c
+
+  __attribute__((__format__(__printf__, 1, 2)))
+  void fmt(const char *s, const char *a, int b);
+
+  void bar(void) {
+fmt("%s %i", "hello", 123); // OK
+fmt("%i %g", "hello", 123); // warning: arguments don't match format
+extern const char *fmt;
+fmt(fmt, "hello", 123); // warning: format string is not a string literal
+  }
+
+Using the ``format`` attribute on a non-variadic function emits a GCC
+compatibility diagnostic.
   }];
 }
 

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index bc7aec3803e82..9e589c70a86d7 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3115,8 +3115,6 @@ def note_ownership_returns_index_mismatch : Note<
   "declared with index %0 here">;
 def

[clang] 91ed7e1 - [clang] Allow all string types for all attribute(format) styles

2022-05-12 Thread Félix Cloutier via cfe-commits 

Author: Félix Cloutier
Date: 2022-05-12T11:12:38-07:00
New Revision: 91ed7e19418181ae385c2626cedd3b08b6ba43a6

URL: 
https://github.com/llvm/llvm-project/commit/91ed7e19418181ae385c2626cedd3b08b6ba43a6
DIFF: 
https://github.com/llvm/llvm-project/commit/91ed7e19418181ae385c2626cedd3b08b6ba43a6.diff

LOG: [clang] Allow all string types for all attribute(format) styles

This allows using any recognized kind of string for any
__attribute__((format)) archetype. Before this change, for instance,
the printf archetype would only accept char pointer types and the
NSString archetype would only accept NSString pointers. This is
more restrictive than necessary as there exist functions to
convert between string types that can be annotated with
__attribute__((format_arg)) to transfer format information.

Reviewed By: ahatanak

Differential Revision: https://reviews.llvm.org/D125254

rdar://89060618

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/SemaObjC/format-strings-objc.m

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 96feef6f4ffce..d8f87747e58d0 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3108,7 +3108,7 @@ def err_format_strftime_third_parameter : Error<
   "strftime format attribute requires 3rd parameter to be 0">;
 def err_format_attribute_requires_variadic : Error<
   "format attribute requires variadic function">;
-def err_format_attribute_not : Error<"format argument not %0">;
+def err_format_attribute_not : Error<"format argument not a string type">;
 def err_format_attribute_result_not : Error<"function does not return %0">;
 def err_format_attribute_implicit_this_format_string : Error<
   "format attribute cannot specify the implicit this argument as the format "

diff  --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index c4a3b18ce2564..1b055543dc485 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3661,8 +3661,7 @@ static void handleFormatArgAttr(Sema , Decl *D, const 
ParsedAttr ) {
   (!Ty->isPointerType() ||
!Ty->castAs()->getPointeeType()->isCharType())) {
 S.Diag(AL.getLoc(), diag::err_format_attribute_not)
-<< "a string type" << IdxExpr->getSourceRange()
-<< getFunctionOrMethodParamRange(D, 0);
+<< IdxExpr->getSourceRange() << getFunctionOrMethodParamRange(D, 0);
 return;
   }
   Ty = getFunctionOrMethodResultType(D);
@@ -3862,27 +3861,12 @@ static void handleFormatAttr(Sema , Decl *D, const 
ParsedAttr ) {
   // make sure the format string is really a string
   QualType Ty = getFunctionOrMethodParamType(D, ArgIdx);
 
-  if (Kind == CFStringFormat) {
-if (!isCFStringType(Ty, S.Context)) {
-  S.Diag(AL.getLoc(), diag::err_format_attribute_not)
-<< "a CFString" << IdxExpr->getSourceRange()
-<< getFunctionOrMethodParamRange(D, ArgIdx);
-  return;
-}
-  } else if (Kind == NSStringFormat) {
-// FIXME: do we need to check if the type is NSString*?  What are the
-// semantics?
-if (!isNSStringType(Ty, S.Context, /*AllowNSAttributedString=*/true)) {
-  S.Diag(AL.getLoc(), diag::err_format_attribute_not)
-<< "an NSString" << IdxExpr->getSourceRange()
-<< getFunctionOrMethodParamRange(D, ArgIdx);
-  return;
-}
-  } else if (!Ty->isPointerType() ||
- !Ty->castAs()->getPointeeType()->isCharType()) {
+  if (!isNSStringType(Ty, S.Context, true) &&
+  !isCFStringType(Ty, S.Context) &&
+  (!Ty->isPointerType() ||
+   !Ty->castAs()->getPointeeType()->isCharType())) {
 S.Diag(AL.getLoc(), diag::err_format_attribute_not)
-  << "a string type" << IdxExpr->getSourceRange()
-  << getFunctionOrMethodParamRange(D, ArgIdx);
+  << IdxExpr->getSourceRange() << getFunctionOrMethodParamRange(D, ArgIdx);
 return;
   }
 

diff  --git a/clang/test/SemaObjC/format-strings-objc.m 
b/clang/test/SemaObjC/format-strings-objc.m
index 6850ebfc5cb95..71f22c07e2323 100644
--- a/clang/test/SemaObjC/format-strings-objc.m
+++ b/clang/test/SemaObjC/format-strings-objc.m
@@ -60,8 +60,23 @@ void check_nslog(unsigned k) {
 }
 
 // Check type validation
-extern void NSLog2(int format, ...) __attribute__((format(__NSString__, 1, 
2))); // expected-error {{format argument not an NSString}}
-extern void CFStringCreateWithFormat2(int *format, ...) 
__attribute__((format(CFString, 1, 2))); // expected-error {{format argument 
not a CFString}}
+extern void NSLog2(int format, ...) __attribute__((format(__NSString__, 1, 
2))); // expected-error {{format argument not a string type}}
+extern void CFStringCreateWithFormat2(int *format, ...) 
__attribute__((format(CFString, 1, 2))); // expected-error {{format

[clang] 12ab3e6 - format_arg attribute does not support nullable instancetype return type

2021-11-12 Thread Félix Cloutier via cfe-commits 

Author: Félix Cloutier
Date: 2021-11-12T13:35:43-08:00
New Revision: 12ab3e6c8402078f58959847277858eb47a43a19

URL: 
https://github.com/llvm/llvm-project/commit/12ab3e6c8402078f58959847277858eb47a43a19
DIFF: 
https://github.com/llvm/llvm-project/commit/12ab3e6c8402078f58959847277858eb47a43a19.diff

LOG: format_arg attribute does not support nullable instancetype return type

* The format_arg attribute tells the compiler that the attributed function
  returns a format string that is compatible with a format string that is being
  passed as a specific argument.
* Several NSString methods return copies of their input, so they would ideally
  have the format_arg attribute. A previous differential (D112670) added
  support for instancetype methods having the format_arg attribute when used
  in the context of NSString method declarations.
* D112670 failed to account that instancetype can be sugared in certain narrow
  (but critical) scenarios, like by using nullability specifiers. This patch
  resolves this problem.

Differential Revision: https://reviews.llvm.org/D113636
Reviewed By: ahatanak

Radar-Id: rdar://85278860

Added: 


Modified: 
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/SemaObjC/format-arg-attribute.m

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 743b292d29759..ef889a36bd55c 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3389,7 +3389,8 @@ static void handleFormatArgAttr(Sema , Decl *D, const 
ParsedAttr ) {
   }
   Ty = getFunctionOrMethodResultType(D);
   // replace instancetype with the class type
-  if (Ty.getTypePtr() == S.Context.getObjCInstanceTypeDecl()->getTypeForDecl())
+  auto Instancetype = S.Context.getObjCInstanceTypeDecl()->getTypeForDecl();
+  if (Ty->getAs() == Instancetype)
 if (auto *OMD = dyn_cast(D))
   if (auto *Interface = OMD->getClassInterface())
 Ty = S.Context.getObjCObjectPointerType(

diff  --git a/clang/test/SemaObjC/format-arg-attribute.m 
b/clang/test/SemaObjC/format-arg-attribute.m
index f137879880768..41b416a6efd42 100644
--- a/clang/test/SemaObjC/format-arg-attribute.m
+++ b/clang/test/SemaObjC/format-arg-attribute.m
@@ -2,7 +2,10 @@
 
 @interface NSString
 +(instancetype)stringWithCString:(const char *)cstr 
__attribute__((format_arg(1)));
-+(instancetype)stringWithString:(NSString *)cstr 
__attribute__((format_arg(1)));
+-(instancetype)initWithString:(NSString *)str __attribute__((format_arg(1)));
+
++(instancetype _Nonnull)nonNullableString:(NSString *)str 
__attribute__((format_arg(1)));
++(instancetype _Nullable)nullableString:(NSString *)str 
__attribute__((format_arg(1)));
 @end
 
 @protocol MaybeString



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

[clang] 6a5f743 - format_arg attribute should allow instancetype in NSString definition

2021-10-28 Thread Félix Cloutier via cfe-commits 

Author: Félix Cloutier
Date: 2021-10-28T20:25:00-07:00
New Revision: 6a5f7437720ea0fa184469584600c27a1a912a41

URL: 
https://github.com/llvm/llvm-project/commit/6a5f7437720ea0fa184469584600c27a1a912a41
DIFF: 
https://github.com/llvm/llvm-project/commit/6a5f7437720ea0fa184469584600c27a1a912a41.diff

LOG: format_arg attribute should allow instancetype in NSString definition

- [[format_arg(N)]] tells Clang that a method returns a format string with
  specifiers equivalent to those passed in the string at argument #N. It
  obviously requires the argument and the return type to be strings both.
- `instancetype` is a special return type available in Objective-C class
  definitions that Clang expands to the most-derived statically known type on
  use.
- In Objective-C mode, NSString is allowed in lieu of a C string, both as input
  and output. However, _in the definition of NSString_, Clang rejects format_arg
  on methods that return NSString. This PR addresses this issue by substituting
  `instancetype` with the enclosing definition's type during the validation of
  `format_arg`.

Reviewed By: ahatanak

Differential Revision: https://reviews.llvm.org/D112670

Radar-Id: rdar://84729746

Added: 


Modified: 
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/SemaObjC/format-arg-attribute.m

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 0c2b506d554b2..743b292d29759 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3388,6 +3388,12 @@ static void handleFormatArgAttr(Sema , Decl *D, const 
ParsedAttr ) {
 return;
   }
   Ty = getFunctionOrMethodResultType(D);
+  // replace instancetype with the class type
+  if (Ty.getTypePtr() == S.Context.getObjCInstanceTypeDecl()->getTypeForDecl())
+if (auto *OMD = dyn_cast(D))
+  if (auto *Interface = OMD->getClassInterface())
+Ty = S.Context.getObjCObjectPointerType(
+QualType(Interface->getTypeForDecl(), 0));
   if (!isNSStringType(Ty, S.Context, /*AllowNSAttributedString=*/true) &&
   !isCFStringType(Ty, S.Context) &&
   (!Ty->isPointerType() ||

diff  --git a/clang/test/SemaObjC/format-arg-attribute.m 
b/clang/test/SemaObjC/format-arg-attribute.m
index ac81bdc21dc16..f137879880768 100644
--- a/clang/test/SemaObjC/format-arg-attribute.m
+++ b/clang/test/SemaObjC/format-arg-attribute.m
@@ -1,6 +1,14 @@
 // RUN: %clang_cc1 -verify -fsyntax-only %s
 
-@class NSString;
+@interface NSString
++(instancetype)stringWithCString:(const char *)cstr 
__attribute__((format_arg(1)));
++(instancetype)stringWithString:(NSString *)cstr 
__attribute__((format_arg(1)));
+@end
+
+@protocol MaybeString
+-(instancetype)maybeString:(const char *)cstr __attribute__((format_arg(1))); 
// expected-error {{function does not return string type}}
+@end
+
 @class NSAttributedString;
 
 extern NSString *fa2 (const NSString *) __attribute__((format_arg(1)));



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

[clang] d378a0f - [Sema] Recognize format argument indicated by format attribute inside blocks

2021-10-27 Thread Félix Cloutier via cfe-commits 

Author: Félix Cloutier
Date: 2021-10-27T15:48:35-07:00
New Revision: d378a0febc7e13aae7853b2e8733626f61140e55

URL: 
https://github.com/llvm/llvm-project/commit/d378a0febc7e13aae7853b2e8733626f61140e55
DIFF: 
https://github.com/llvm/llvm-project/commit/d378a0febc7e13aae7853b2e8733626f61140e55.diff

LOG: [Sema] Recognize format argument indicated by format attribute inside 
blocks

- `[[format(archetype, fmt-idx, ellipsis)]]` specifies that a function accepts a
  format string and arguments according to `archetype`. This is how Clang
  type-checks `printf` arguments based on the format string.
- Clang has a `-Wformat-nonliteral` warning that is triggered when a function
  with the `format` attribute is called with a format string that is not
  inspectable because it isn't constant. This warning is suppressed if the
  caller has the `format` attribute itself and the format argument to the callee
  is the caller's own format parameter.
- When using the `format` attribute on a block, Clang wouldn't recognize its
  format parameter when calling another function with the format attribute. This
  would cause unsuppressed -Wformat-nonliteral warnings for no supported reason.

Reviewed By: ahatanak

Differential Revision: https://reviews.llvm.org/D112569

Radar-Id: rdar://84603673

Added: 


Modified: 
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/format-strings.c

Removed: 




diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 83fb824cb14a6..3eaeae197648a 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -7784,11 +7784,11 @@ checkFormatStringExpr(Sema , const Expr *E, 
ArrayRef Args,
   // }
   if (HasVAListArg) {
 if (const ParmVarDecl *PV = dyn_cast(VD)) {
-  if (const NamedDecl *ND = dyn_cast(PV->getDeclContext())) 
{
+  if (const Decl *D = dyn_cast(PV->getDeclContext())) {
 int PVIndex = PV->getFunctionScopeIndex() + 1;
-for (const auto *PVFormat : ND->specific_attrs()) {
+for (const auto *PVFormat : D->specific_attrs()) {
   // adjust for implicit parameter
-  if (const CXXMethodDecl *MD = dyn_cast(ND))
+  if (const CXXMethodDecl *MD = dyn_cast(D))
 if (MD->isInstance())
   ++PVIndex;
   // We also check if the formats are compatible.

diff  --git a/clang/test/Sema/format-strings.c 
b/clang/test/Sema/format-strings.c
index e8cd478d25126..bbe47636ebb7d 100644
--- a/clang/test/Sema/format-strings.c
+++ b/clang/test/Sema/format-strings.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -Wformat-nonliteral -isystem 
%S/Inputs %s
-// RUN: %clang_cc1 -fsyntax-only -verify -Wformat-nonliteral -isystem 
%S/Inputs -fno-signed-char %s
+// RUN: %clang_cc1 -fblocks -fsyntax-only -verify -Wformat-nonliteral -isystem 
%S/Inputs %s
+// RUN: %clang_cc1 -fblocks -fsyntax-only -verify -Wformat-nonliteral -isystem 
%S/Inputs -fno-signed-char %s
 
 #include 
 #include 
@@ -714,3 +714,30 @@ void PR30481() {
 void test_printf_opaque_ptr(void *op) {
   printf("%s", op); // expected-warning{{format specifies type 'char *' but 
the argument has type 'void *'}}
 }
+
+void test_block() {
+  void __attribute__((__format__(__printf__, 1, 2))) (^printf_arg1)(
+  const char *, ...) =
+  ^(const char *fmt, ...) __attribute__((__format__(__printf__, 1, 2))) {
+va_list ap;
+va_start(ap, fmt);
+vprintf(fmt, ap);
+va_end(ap);
+  };
+
+  printf_arg1("%s string %i\n", "aaa", 123);
+  printf_arg1("%s string\n", 123); // expected-warning{{format specifies type 
'char *' but the argument has type 'int'}}
+
+  void __attribute__((__format__(__printf__, 2, 3))) (^printf_arg2)(
+  const char *, const char *, ...) =
+  ^(const char *not_fmt, const char *fmt, ...)
+  __attribute__((__format__(__printf__, 2, 3))) {
+va_list ap;
+va_start(ap, fmt);
+vprintf(not_fmt, ap); // expected-warning{{format string is not a string 
literal}}
+va_end(ap);
+  };
+
+  printf_arg2("foo", "%s string %i\n", "aaa", 123);
+  printf_arg2("%s string\n", "foo", "bar"); // expected-warning{{data argument 
not used by format string}}
+}



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

[clang] 554cf37 - [clang-tblgen] AnnotateAttr::printPretty has spurious comma when no variadic argument is specified

2021-02-03 Thread Félix Cloutier via cfe-commits 

Author: Félix Cloutier
Date: 2021-02-03T11:41:38-08:00
New Revision: 554cf3729e651b3b5416e081e63670fbe71cf91e

URL: 
https://github.com/llvm/llvm-project/commit/554cf3729e651b3b5416e081e63670fbe71cf91e
DIFF: 
https://github.com/llvm/llvm-project/commit/554cf3729e651b3b5416e081e63670fbe71cf91e.diff

LOG: [clang-tblgen] AnnotateAttr::printPretty has spurious comma when no 
variadic argument is specified

rdar://73742471
Differential Revision: https://reviews.llvm.org/D95695

Added: 


Modified: 
clang/test/AST/ast-print-attr.c
clang/utils/TableGen/ClangAttrEmitter.cpp

Removed: 




diff  --git a/clang/test/AST/ast-print-attr.c b/clang/test/AST/ast-print-attr.c
index 90a396303441..4140ae6ac11f 100644
--- a/clang/test/AST/ast-print-attr.c
+++ b/clang/test/AST/ast-print-attr.c
@@ -26,3 +26,9 @@ enum __attribute__((ns_error_domain(MyErrorDomain))) 
MyErrorEnum {
   MyErrFirst,
   MyErrSecond,
 };
+
+// CHECK: int *fun_returns() __attribute__((ownership_returns(fun_returns)));
+int *fun_returns() __attribute__((ownership_returns(fun_returns)));
+
+// CHECK: void fun_holds(int *a) __attribute__((ownership_holds(fun_holds, 
1)));
+void fun_holds(int *a) __attribute__((ownership_holds(fun_holds, 1)));

diff  --git a/clang/utils/TableGen/ClangAttrEmitter.cpp 
b/clang/utils/TableGen/ClangAttrEmitter.cpp
index d435c5780531..aaef538e9bf9 100644
--- a/clang/utils/TableGen/ClangAttrEmitter.cpp
+++ b/clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -773,10 +773,8 @@ namespace {
 
 void writeValue(raw_ostream ) const override {
   OS << "\";\n";
-  OS << "  bool isFirst = true;\n"
- << "  for (const auto  : " << RangeName << "()) {\n"
- << "if (isFirst) isFirst = false;\n"
- << "else OS << \", \";\n";
+  OS << "  for (const auto  : " << RangeName << "()) {\n"
+ << "DelimitAttributeArgument(OS, IsFirstArgument);\n";
   writeValueImpl(OS);
   OS << "  }\n";
   OS << "  OS << \"";
@@ -1428,10 +1426,12 @@ writePrettyPrintFunction(const Record ,
 return;
   }
 
-  OS << "  switch (getAttributeSpellingListIndex()) {\n"
-"  default:\n"
-"llvm_unreachable(\"Unknown attribute spelling!\");\n"
-"break;\n";
+  OS << "  bool IsFirstArgument = true; (void)IsFirstArgument;\n"
+ << "  unsigned TrailingOmittedArgs = 0; (void)TrailingOmittedArgs;\n"
+ << "  switch (getAttributeSpellingListIndex()) {\n"
+ << "  default:\n"
+ << "llvm_unreachable(\"Unknown attribute spelling!\");\n"
+ << "break;\n";
 
   for (unsigned I = 0; I < Spellings.size(); ++ I) {
 llvm::SmallString<16> Prefix;
@@ -1476,12 +1476,10 @@ writePrettyPrintFunction(const Record ,
 
 Spelling += Name;
 
-OS <<
-  "  case " << I << " : {\n"
-  "OS << \"" << Prefix << Spelling;
+OS << "  case " << I << " : {\n"
+   << "OS << \"" << Prefix << Spelling << "\";\n";
 
 if (Variety == "Pragma") {
-  OS << "\";\n";
   OS << "printPrettyPragma(OS, Policy);\n";
   OS << "OS << \"\\n\";";
   OS << "break;\n";
@@ -1490,19 +1488,18 @@ writePrettyPrintFunction(const Record ,
 }
 
 if (Spelling == "availability") {
-  OS << "(";
+  OS << "OS << \"(";
   writeAvailabilityValue(OS);
-  OS << ")";
+  OS << ")\";\n";
 } else if (Spelling == "deprecated" || Spelling == "gnu::deprecated") {
-  OS << "(";
+  OS << "OS << \"(";
   writeDeprecatedAttrValue(OS, Variety);
-  OS << ")";
+  OS << ")\";\n";
 } else {
   // To avoid printing parentheses around an empty argument list or
   // printing spurious commas at the end of an argument list, we need to
   // determine where the last provided non-fake argument is.
   unsigned NonFakeArgs = 0;
-  unsigned TrailingOptArgs = 0;
   bool FoundNonOptArg = false;
   for (const auto  : llvm::reverse(Args)) {
 if (arg->isFake())
@@ -1516,61 +1513,33 @@ writePrettyPrintFunction(const Record ,
   FoundNonOptArg = true;
   continue;
 }
-if (!TrailingOptArgs++)
-  OS << "\";\n"
- << "unsigned TrailingOmittedArgs = 0;\n";
 OS << "if (" << arg->getIsOmitted() << ")\n"
<< "  ++TrailingOmittedArgs;\n";
   }
-  if (TrailingOptArgs)
-OS << "OS << \"";
-  if (TrailingOptArgs < NonFakeArgs)
-OS << "(";
-  else if (TrailingOptArgs)
-OS << "\";\n"
-   << "if (TrailingOmittedArgs < " << NonFakeArgs << ")\n"
-   << "   OS << \"(\";\n"
-   << "OS << \"";
   unsigned ArgIndex = 0;
   for (const auto  : Args) {
 if (arg->isFake())
   continue;
-if (ArgIndex) {
-  if (ArgIndex >= NonFakeArgs - TrailingOptArgs)
-OS << "\";\n"
-   << "if (" << ArgIndex << " < "