Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang]Treat arguments to builtin type traits as template type arguments #87132

Merged

Conversation

AMP999
Copy link
Contributor

@AMP999 AMP999 commented Mar 30, 2024

This change improves error messages for builtins in case of empty parentheses.

Fixes #86997

This change improves error messages for builtins in case of empty parentheses.

Fixes llvm#86997
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 30, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 30, 2024

@llvm/pr-subscribers-clang

Author: Amirreza Ashouri (AMP999)

Changes

This change improves error messages for builtins in case of empty parentheses.

Fixes llvm#86997


Full diff: https://github.com/llvm/llvm-project/pull/87132.diff

4 Files Affected:

  • (modified) clang/lib/Parse/ParseExprCXX.cpp (+6-6)
  • (modified) clang/test/Sema/static-assert.c (+4-5)
  • (modified) clang/test/SemaCXX/builtins.cpp (+5)
  • (modified) clang/test/SemaCXX/deprecated-builtins.cpp (+5)
diff --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp
index 9471f6f725efb1..efe0c587830392 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -3908,10 +3908,10 @@ ExprResult Parser::ParseTypeTrait() {
   SmallVector<ParsedType, 2> Args;
   do {
     // Parse the next type.
-    TypeResult Ty =
-        ParseTypeName(/*SourceRange=*/nullptr,
-                      getLangOpts().CPlusPlus ? DeclaratorContext::TemplateArg
-                                              : DeclaratorContext::TypeName);
+    TypeResult Ty = ParseTypeName(/*SourceRange=*/nullptr,
+                                  getLangOpts().CPlusPlus
+                                      ? DeclaratorContext::TemplateTypeArg
+                                      : DeclaratorContext::TypeName);
     if (Ty.isInvalid()) {
       Parens.skipToEnd();
       return ExprError();
@@ -3953,8 +3953,8 @@ ExprResult Parser::ParseArrayTypeTrait() {
   if (T.expectAndConsume())
     return ExprError();
 
-  TypeResult Ty =
-      ParseTypeName(/*SourceRange=*/nullptr, DeclaratorContext::TemplateArg);
+  TypeResult Ty = ParseTypeName(/*SourceRange=*/nullptr,
+                                DeclaratorContext::TemplateTypeArg);
   if (Ty.isInvalid()) {
     SkipUntil(tok::comma, StopAtSemi);
     SkipUntil(tok::r_paren, StopAtSemi);
diff --git a/clang/test/Sema/static-assert.c b/clang/test/Sema/static-assert.c
index ae5e8076e0beda..4e9e6b7ee558bd 100644
--- a/clang/test/Sema/static-assert.c
+++ b/clang/test/Sema/static-assert.c
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -std=c11 -Wgnu-folding-constant -fsyntax-only -verify=expected,c %s
-// RUN: %clang_cc1 -fms-compatibility -Wgnu-folding-constant -DMS -fsyntax-only -verify=expected,ms,c %s
-// RUN: %clang_cc1 -std=c99 -pedantic -Wgnu-folding-constant -fsyntax-only -verify=expected,ext,c %s
+// RUN: %clang_cc1 -std=c11 -Wgnu-folding-constant -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fms-compatibility -Wgnu-folding-constant -DMS -fsyntax-only -verify=expected,ms %s
+// RUN: %clang_cc1 -std=c99 -pedantic -Wgnu-folding-constant -fsyntax-only -verify=expected,ext %s
 // RUN: %clang_cc1 -xc++ -std=c++11 -pedantic -fsyntax-only -verify=expected,ext,cxx %s
 
 _Static_assert("foo", "string is nonzero"); // ext-warning {{'_Static_assert' is a C11 extension}}
@@ -57,8 +57,7 @@ UNION(char[2], short) u2 = { .one = { 'a', 'b' } }; // ext-warning 3 {{'_Static_
 typedef UNION(char, short) U3; // expected-error {{static assertion failed due to requirement 'sizeof(char) == sizeof(short)': type size mismatch}} \
                                // expected-note{{evaluates to '1 == 2'}} \
                                // ext-warning 3 {{'_Static_assert' is a C11 extension}}
-typedef UNION(float, 0.5f) U4; // c-error {{expected a type}} \
-                               // cxx-error {{type name requires a specifier or qualifier}} \
+typedef UNION(float, 0.5f) U4; // expected-error {{expected a type}} \
                                // ext-warning 3 {{'_Static_assert' is a C11 extension}}
 
 // After defining the assert macro in MS-compatibility mode, we should
diff --git a/clang/test/SemaCXX/builtins.cpp b/clang/test/SemaCXX/builtins.cpp
index 567094c94c171b..080b4476c7eec1 100644
--- a/clang/test/SemaCXX/builtins.cpp
+++ b/clang/test/SemaCXX/builtins.cpp
@@ -76,6 +76,11 @@ using ConstMemFnType = int (Dummy::*)() const;
 
 void foo() {}
 
+void test_builtin_empty_parentheses_diags() {
+  __is_trivially_copyable(); // expected-error {{expected a type}}
+  __is_trivially_copyable(1); // expected-error {{expected a type}}
+}
+
 void test_builtin_launder_diags(void *vp, const void *cvp, FnType *fnp,
                                 MemFnType mfp, ConstMemFnType cmfp, int (&Arr)[5]) {
   __builtin_launder(vp);   // expected-error {{void pointer argument to '__builtin_launder' is not allowed}}
diff --git a/clang/test/SemaCXX/deprecated-builtins.cpp b/clang/test/SemaCXX/deprecated-builtins.cpp
index 849b9b014fff25..fafc1da4da13eb 100644
--- a/clang/test/SemaCXX/deprecated-builtins.cpp
+++ b/clang/test/SemaCXX/deprecated-builtins.cpp
@@ -17,3 +17,8 @@ void f() {
     a = __has_trivial_destructor(A);  // expected-warning-re {{__has_trivial_destructor {{.*}} use __is_trivially_destructible}}
 
 }
+
+void test_builtin_empty_parentheses_diags(void) {
+    __has_nothrow_copy(); // expected-error {{expected a type}}
+    __has_nothrow_copy(1); // expected-error {{expected a type}}
+}

Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but I want @fahadnayyar to verify this addresses his concerns.

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you! No need for a release note because the changes are correcting an issue introduced in 19.x.

@AMP999
Copy link
Contributor Author

AMP999 commented Apr 8, 2024

@fahadnayyar, does this PR address #86997?

Copy link
Contributor

@fahadnayyar fahadnayyar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@AMP999
Copy link
Contributor Author

AMP999 commented Apr 9, 2024

@shafik, gentle ping!

@AMP999 AMP999 requested a review from shafik April 16, 2024 16:05
@cor3ntin
Copy link
Contributor

@AMP999 Feel free to merge!

@AMP999
Copy link
Contributor Author

AMP999 commented Apr 17, 2024

@cor3ntin Thank you! I don't have the permission to merge. Could you merge it for me?

@cor3ntin cor3ntin merged commit b1dc62f into llvm:main Apr 17, 2024
7 checks passed
@AMP999 AMP999 deleted the pr12-builtin-empty-parentheses-error-message branch April 17, 2024 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error message got changed for certain type traits due to 690bf64f077a
6 participants