[clang] 0d4e243 - [WebAssembly] Improve clang diagnostics for wasm attributes
Dan Gohman via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 5 14:34:44 PDT 2020
Author: Dan Gohman
Date: 2020-06-05T14:32:51-07:00
New Revision: 0d4e243456809eabd6914669753eda242b5da4cb
URL: https://github.com/llvm/llvm-project/commit/0d4e243456809eabd6914669753eda242b5da4cb
DIFF: https://github.com/llvm/llvm-project/commit/0d4e243456809eabd6914669753eda242b5da4cb.diff
LOG: [WebAssembly] Improve clang diagnostics for wasm attributes
This patch addresses the review comments on r352930:
- Removes redundant diagnostic checking code
- Removes errnoneous use of diag::err_alias_is_definition, which
turned out to be ineffective anyway since functions can be defined later
in the translation unit and avoid detection.
- Adds a test for various invalid cases for import_name and import_module.
This reapplies D59520, with the addition of adding
`InGroup<IgnoredAttributes>` to the new warnings, to fix the
Misc/warning-flags.c failure.
Differential Revision: https://reviews.llvm.org/D59520
Added:
clang/test/AST/ast-dump-wasm-attr-export.c
clang/test/AST/ast-dump-wasm-attr-import.c
clang/test/Sema/attr-wasm.c
Modified:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclAttr.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 0a284b9a8906..45a7f1c700b4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10770,6 +10770,14 @@ def err_matrix_separate_incomplete_index: Error<
def err_matrix_subscript_comma: Error<
"comma expressions are not allowed as indices in matrix subscript expressions">;
+def warn_mismatched_import : Warning<
+ "import %select{module|name}0 (%1) does not match the import %select{module|name}0 (%2) of the "
+ "previous declaration">,
+ InGroup<IgnoredAttributes>;
+def warn_import_on_definition : Warning<
+ "import %select{module|name}0 cannot be applied to a function with a definition">,
+ InGroup<IgnoredAttributes>;
+
def err_preserve_field_info_not_field : Error<
"__builtin_preserve_field_info argument %0 not a field access">;
def err_preserve_field_info_not_const: Error<
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 5f0a03b1c93f..cef25fc927aa 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2999,6 +2999,10 @@ class Sema final {
const InternalLinkageAttr &AL);
CommonAttr *mergeCommonAttr(Decl *D, const ParsedAttr &AL);
CommonAttr *mergeCommonAttr(Decl *D, const CommonAttr &AL);
+ WebAssemblyImportNameAttr *mergeImportNameAttr(
+ Decl *D, const WebAssemblyImportNameAttr &AL);
+ WebAssemblyImportModuleAttr *mergeImportModuleAttr(
+ Decl *D, const WebAssemblyImportModuleAttr &AL);
void mergeDeclAttributes(NamedDecl *New, Decl *Old,
AvailabilityMergeKind AMK = AMK_Redeclaration);
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 6e52c95ad488..025b09de0ad1 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -2598,6 +2598,10 @@ static bool mergeDeclAttribute(Sema &S, NamedDecl *D,
NewAttr = S.mergeSpeculativeLoadHardeningAttr(D, *SLHA);
else if (const auto *SLHA = dyn_cast<NoSpeculativeLoadHardeningAttr>(Attr))
NewAttr = S.mergeNoSpeculativeLoadHardeningAttr(D, *SLHA);
+ else if (const auto *IMA = dyn_cast<WebAssemblyImportModuleAttr>(Attr))
+ NewAttr = S.mergeImportModuleAttr(D, *IMA);
+ else if (const auto *INA = dyn_cast<WebAssemblyImportNameAttr>(Attr))
+ NewAttr = S.mergeImportNameAttr(D, *INA);
else if (Attr->shouldInheritEvenIfAlreadyPresent() || !DeclHasAttr(D, Attr))
NewAttr = cast<InheritableAttr>(Attr->clone(S.Context));
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index df44b6fcf2af..763db5b41bb8 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -5892,45 +5892,75 @@ static void handleWebAssemblyExportNameAttr(Sema &S, Decl *D, const ParsedAttr &
D->addAttr(UsedAttr::CreateImplicit(S.Context));
}
-static void handleWebAssemblyImportModuleAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
- if (!isFunctionOrMethod(D)) {
- S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
- << "'import_module'" << ExpectedFunction;
- return;
+WebAssemblyImportModuleAttr *
+Sema::mergeImportModuleAttr(Decl *D, const WebAssemblyImportModuleAttr &AL) {
+ auto *FD = cast<FunctionDecl>(D);
+
+ if (const auto *ExistingAttr = FD->getAttr<WebAssemblyImportModuleAttr>()) {
+ if (ExistingAttr->getImportModule() == AL.getImportModule())
+ return nullptr;
+ Diag(ExistingAttr->getLocation(), diag::warn_mismatched_import) << 0
+ << ExistingAttr->getImportModule() << AL.getImportModule();
+ Diag(AL.getLoc(), diag::note_previous_attribute);
+ return nullptr;
+ }
+ if (FD->hasBody()) {
+ Diag(AL.getLoc(), diag::warn_import_on_definition) << 0;
+ return nullptr;
}
+ return ::new (Context) WebAssemblyImportModuleAttr(Context, AL,
+ AL.getImportModule());
+}
+WebAssemblyImportNameAttr *
+Sema::mergeImportNameAttr(Decl *D, const WebAssemblyImportNameAttr &AL) {
auto *FD = cast<FunctionDecl>(D);
- if (FD->isThisDeclarationADefinition()) {
- S.Diag(D->getLocation(), diag::err_alias_is_definition) << FD << 0;
- return;
+
+ if (const auto *ExistingAttr = FD->getAttr<WebAssemblyImportNameAttr>()) {
+ if (ExistingAttr->getImportName() == AL.getImportName())
+ return nullptr;
+ Diag(ExistingAttr->getLocation(), diag::warn_mismatched_import) << 1
+ << ExistingAttr->getImportName() << AL.getImportName();
+ Diag(AL.getLoc(), diag::note_previous_attribute);
+ return nullptr;
+ }
+ if (FD->hasBody()) {
+ Diag(AL.getLoc(), diag::warn_import_on_definition) << 1;
+ return nullptr;
}
+ return ::new (Context) WebAssemblyImportNameAttr(Context, AL,
+ AL.getImportName());
+}
+
+static void
+handleWebAssemblyImportModuleAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+ auto *FD = cast<FunctionDecl>(D);
StringRef Str;
SourceLocation ArgLoc;
if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, &ArgLoc))
return;
+ if (FD->hasBody()) {
+ S.Diag(AL.getLoc(), diag::warn_import_on_definition) << 0;
+ return;
+ }
FD->addAttr(::new (S.Context)
WebAssemblyImportModuleAttr(S.Context, AL, Str));
}
-static void handleWebAssemblyImportNameAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
- if (!isFunctionOrMethod(D)) {
- S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
- << "'import_name'" << ExpectedFunction;
- return;
- }
-
+static void
+handleWebAssemblyImportNameAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
auto *FD = cast<FunctionDecl>(D);
- if (FD->isThisDeclarationADefinition()) {
- S.Diag(D->getLocation(), diag::err_alias_is_definition) << FD << 0;
- return;
- }
StringRef Str;
SourceLocation ArgLoc;
if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, &ArgLoc))
return;
+ if (FD->hasBody()) {
+ S.Diag(AL.getLoc(), diag::warn_import_on_definition) << 1;
+ return;
+ }
FD->addAttr(::new (S.Context) WebAssemblyImportNameAttr(S.Context, AL, Str));
}
diff --git a/clang/test/AST/ast-dump-wasm-attr-export.c b/clang/test/AST/ast-dump-wasm-attr-export.c
new file mode 100644
index 000000000000..951b6cf8b083
--- /dev/null
+++ b/clang/test/AST/ast-dump-wasm-attr-export.c
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -triple wasm32-unknown-unknown -ast-dump %s | FileCheck --strict-whitespace %s
+
+// Test that functions can be redeclared and they retain their attributes.
+
+__attribute__((export_name("export_red"))) void red(void) {}
+__attribute__((export_name("export_orange"))) void orange(void) {}
+__attribute__((export_name("export_yellow"))) void yellow(void) {}
+
+void red(void);
+void orange(void);
+void yellow(void);
+
+// CHECK: |-FunctionDecl {{.+}} used red 'void (void)'
+// CHECK: | |-CompoundStmt {{.+}}
+// CHECK: | |-WebAssemblyExportNameAttr {{.+}} "export_red"
+// CHECK: | `-UsedAttr {{.+}} Implicit
+// CHECK: |-FunctionDecl {{.+}} used orange 'void (void)'
+// CHECK: | |-CompoundStmt {{.+}}
+// CHECK: | |-WebAssemblyExportNameAttr {{.+}} "export_orange"
+// CHECK: | `-UsedAttr {{.+}} Implicit
+// CHECK: |-FunctionDecl {{.+}} used yellow 'void (void)'
+// CHECK: | |-CompoundStmt {{.+}}
+// CHECK: | |-WebAssemblyExportNameAttr {{.+}} "export_yellow"
+// CHECK: | `-UsedAttr {{.+}} Implicit
+// CHECK: |-FunctionDecl {{.+}} used red 'void (void)'
+// CHECK: | |-UsedAttr {{.+}} Inherited Implicit
+// CHECK: | `-WebAssemblyExportNameAttr {{.+}} Inherited "export_red"
+// CHECK: |-FunctionDecl {{.+}} used orange 'void (void)'
+// CHECK: | |-UsedAttr {{.+}} Inherited Implicit
+// CHECK: | `-WebAssemblyExportNameAttr {{.+}} Inherited "export_orange"
+// CHECK: `-FunctionDecl {{.+}} used yellow 'void (void)'
+// CHECK: |-UsedAttr {{.+}} Inherited Implicit
+// CHECK: `-WebAssemblyExportNameAttr {{.+}} Inherited "export_yellow"
diff --git a/clang/test/AST/ast-dump-wasm-attr-import.c b/clang/test/AST/ast-dump-wasm-attr-import.c
new file mode 100644
index 000000000000..c4690eb15f27
--- /dev/null
+++ b/clang/test/AST/ast-dump-wasm-attr-import.c
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -triple wasm32-unknown-unknown -ast-dump %s | FileCheck --strict-whitespace %s
+
+// Test that functions can be redeclared and they retain their attributes.
+
+__attribute__((import_name("import_red"), import_module("mod"))) void red(void);
+__attribute__((import_name("import_orange"), import_module("mod"))) void orange(void);
+__attribute__((import_name("import_yellow"), import_module("mod"))) void yellow(void);
+
+void red(void);
+void orange(void);
+void yellow(void);
+
+void calls(void) {
+ red();
+ orange();
+ yellow();
+}
+
+// CHECK: |-FunctionDecl {{.+}} used red 'void (void)'
+// CHECK: | |-WebAssemblyImportNameAttr {{.+}} "import_red"
+// CHECK: | `-WebAssemblyImportModuleAttr {{.+}} "mod"
+// CHECK: |-FunctionDecl {{.+}} used orange 'void (void)'
+// CHECK: | |-WebAssemblyImportNameAttr {{.+}} "import_orange"
+// CHECK: | `-WebAssemblyImportModuleAttr {{.+}} "mod"
+// CHECK: |-FunctionDecl {{.+}} used yellow 'void (void)'
+// CHECK: | |-WebAssemblyImportNameAttr {{.+}} "import_yellow"
+// CHECK: | `-WebAssemblyImportModuleAttr {{.+}} "mod"
+// CHECK: |-FunctionDecl {{.+}} used red 'void (void)'
+// CHECK: | |-WebAssemblyImportNameAttr {{.+}} Inherited "import_red"
+// CHECK: | `-WebAssemblyImportModuleAttr {{.+}} Inherited "mod"
+// CHECK: |-FunctionDecl {{.+}} used orange 'void (void)'
+// CHECK: | |-WebAssemblyImportNameAttr {{.+}} Inherited "import_orange"
+// CHECK: | `-WebAssemblyImportModuleAttr {{.+}} Inherited "mod"
+// CHECK: |-FunctionDecl {{.+}} used yellow 'void (void)'
+// CHECK: | |-WebAssemblyImportNameAttr {{.+}} Inherited "import_yellow"
+// CHECK: | `-WebAssemblyImportModuleAttr {{.+}} Inherited "mod"
diff --git a/clang/test/Sema/attr-wasm.c b/clang/test/Sema/attr-wasm.c
new file mode 100644
index 000000000000..dc30ba46ea88
--- /dev/null
+++ b/clang/test/Sema/attr-wasm.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -triple wasm32-unknown-unknown -fsyntax-only -verify %s
+
+void name_a() __attribute__((import_name)); //expected-error {{'import_name' attribute takes one argument}}
+
+int name_b __attribute__((import_name("foo"))); //expected-error {{'import_name' attribute only applies to functions}}
+
+void name_c() __attribute__((import_name("foo", "bar"))); //expected-error {{'import_name' attribute takes one argument}}
+
+void name_d() __attribute__((import_name("foo", "bar", "qux"))); //expected-error {{'import_name' attribute takes one argument}}
+
+void name_z() __attribute__((import_name("foo"))); //expected-note {{previous attribute is here}}
+
+void name_z() __attribute__((import_name("bar"))); //expected-warning {{import name (bar) does not match the import name (foo) of the previous declaration}}
+
+void module_a() __attribute__((import_module)); //expected-error {{'import_module' attribute takes one argument}}
+
+int module_b __attribute__((import_module("foo"))); //expected-error {{'import_module' attribute only applies to functions}}
+
+void module_c() __attribute__((import_module("foo", "bar"))); //expected-error {{'import_module' attribute takes one argument}}
+
+void module_d() __attribute__((import_module("foo", "bar", "qux"))); //expected-error {{'import_module' attribute takes one argument}}
+
+void module_z() __attribute__((import_module("foo"))); //expected-note {{previous attribute is here}}
+
+void module_z() __attribute__((import_module("bar"))); //expected-warning {{import module (bar) does not match the import module (foo) of the previous declaration}}
+
+void both() __attribute__((import_name("foo"), import_module("bar")));
More information about the cfe-commits
mailing list