sunfish updated this revision to Diff 197215.
sunfish added a comment.

Implemented proper diagnostics for import_name/import_module on functions with 
definitions, and updated the test.

I'm unsure of the `DIAG_SIZE_SEMA` change, but without it, the build fails with 
this error:

  /llvm/tools/clang/lib/Basic/DiagnosticIDs.cpp:72:3: error: static assertion 
failed: DIAG_SIZE_SEMA is insufficient to contain all diagnostics, it may need 
to be made larger in DiagnosticIDs.h.
     static_assert(                                                             
  \
     ^
  /llvm/tools/clang/lib/Basic/DiagnosticIDs.cpp:88:1: note: in expansion of 
macro ‘VALIDATE_DIAG_SIZE’
   VALIDATE_DIAG_SIZE(SEMA)
   ^~~~~~~~~~~~~~~~~~


Repository:
  rC Clang

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

https://reviews.llvm.org/D59520

Files:
  include/clang/Basic/DiagnosticIDs.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/attr-wasm.c

Index: test/Sema/attr-wasm.c
===================================================================
--- test/Sema/attr-wasm.c
+++ test/Sema/attr-wasm.c
@@ -0,0 +1,23 @@
+// 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")));
+
+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")));
+
+void both() __attribute__((import_name("foo"), import_module("bar")));
Index: lib/Sema/SemaDeclAttr.cpp
===================================================================
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5755,50 +5755,80 @@
   handleSimpleAttribute<AVRSignalAttr>(S, D, AL);
 }
 
-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, SourceRange Range,
+                            StringRef Name,
+                            unsigned AttrSpellingListIndex) {
+  auto *FD = cast<FunctionDecl>(D);
+
+  if (WebAssemblyImportModuleAttr *ExistingAttr =
+          FD->getAttr<WebAssemblyImportModuleAttr>()) {
+    if (ExistingAttr->getImportModule() == Name)
+      return nullptr;
+    Diag(ExistingAttr->getLocation(), diag::warn_mismatched_import_module);
+    Diag(Range.getBegin(), diag::note_previous_attribute);
+    return nullptr;
+  }
+  if (FD->hasBody()) {
+    Diag(Range.getBegin(), diag::warn_import_module_on_definition);
+    return nullptr;
   }
+  return ::new (Context) WebAssemblyImportModuleAttr(Range, Context, Name,
+                                                     AttrSpellingListIndex);
+}
 
+WebAssemblyImportNameAttr *
+Sema::mergeImportNameAttr(Decl *D, SourceRange Range,
+                          StringRef Name,
+                          unsigned AttrSpellingListIndex) {
   auto *FD = cast<FunctionDecl>(D);
-  if (FD->isThisDeclarationADefinition()) {
-    S.Diag(D->getLocation(), diag::err_alias_is_definition) << FD << 0;
-    return;
+
+  if (WebAssemblyImportNameAttr *ExistingAttr =
+          FD->getAttr<WebAssemblyImportNameAttr>()) {
+    if (ExistingAttr->getImportName() == Name)
+      return nullptr;
+    Diag(ExistingAttr->getLocation(), diag::warn_mismatched_import_name);
+    Diag(Range.getBegin(), diag::note_previous_attribute);
+    return nullptr;
+  }
+  if (FD->hasBody()) {
+    Diag(Range.getBegin(), diag::warn_import_name_on_definition);
+    return nullptr;
   }
+  return ::new (Context) WebAssemblyImportNameAttr(Range, Context, Name,
+                                                   AttrSpellingListIndex);
+}
+
+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;
 
-  FD->addAttr(::new (S.Context) WebAssemblyImportModuleAttr(
-      AL.getRange(), S.Context, Str,
-      AL.getAttributeSpellingListIndex()));
+  unsigned Index = AL.getAttributeSpellingListIndex();
+  WebAssemblyImportModuleAttr *NewAttr =
+      S.mergeImportModuleAttr(FD, AL.getRange(), Str, Index);
+  if (NewAttr)
+    FD->addAttr(NewAttr);
 }
 
-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;
 
-  FD->addAttr(::new (S.Context) WebAssemblyImportNameAttr(
-      AL.getRange(), S.Context, Str,
-      AL.getAttributeSpellingListIndex()));
+  unsigned Index = AL.getAttributeSpellingListIndex();
+  WebAssemblyImportNameAttr *NewAttr =
+      S.mergeImportNameAttr(FD, AL.getRange(), Str, Index);
+  if (NewAttr)
+    FD->addAttr(NewAttr);
 }
 
 static void handleRISCVInterruptAttr(Sema &S, Decl *D,
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -2500,6 +2500,14 @@
     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->getRange(),
+                                      IMA->getImportModule(),
+                                      AttrSpellingListIndex);
+  else if (const auto *INA = dyn_cast<WebAssemblyImportNameAttr>(Attr))
+    NewAttr = S.mergeImportNameAttr(D, INA->getRange(),
+                                    INA->getImportName(),
+                                    AttrSpellingListIndex);
   else if (Attr->shouldInheritEvenIfAlreadyPresent() || !DeclHasAttr(D, Attr))
     NewAttr = cast<InheritableAttr>(Attr->clone(S.Context));
 
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -2613,6 +2613,12 @@
                                                 const InternalLinkageAttr &AL);
   CommonAttr *mergeCommonAttr(Decl *D, const ParsedAttr &AL);
   CommonAttr *mergeCommonAttr(Decl *D, const CommonAttr &AL);
+  WebAssemblyImportNameAttr *mergeImportNameAttr(
+      Decl *D, SourceRange Range, StringRef Name,
+      unsigned AttrSpellingListIndex);
+  WebAssemblyImportModuleAttr *mergeImportModuleAttr(
+      Decl *D, SourceRange Range, StringRef Name,
+      unsigned AttrSpellingListIndex);
 
   void mergeDeclAttributes(NamedDecl *New, Decl *Old,
                            AvailabilityMergeKind AMK = AMK_Redeclaration);
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -9646,4 +9646,13 @@
   "%select{non-pointer|function pointer|void pointer}0 argument to "
   "'__builtin_launder' is not allowed">;
 
+def warn_mismatched_import_module : Warning<
+  "import module does not match previous declaration">;
+def warn_mismatched_import_name : Warning<
+  "import name does not match previous declaration">;
+def warn_import_module_on_definition : Warning<
+  "import module cannot be applied to a function with a definition">;
+def warn_import_name_on_definition : Warning<
+  "import name cannot be applied to a function with a definition">;
+
 } // end of sema component.
Index: include/clang/Basic/DiagnosticIDs.h
===================================================================
--- include/clang/Basic/DiagnosticIDs.h
+++ include/clang/Basic/DiagnosticIDs.h
@@ -36,7 +36,7 @@
       DIAG_SIZE_AST           =  150,
       DIAG_SIZE_COMMENT       =  100,
       DIAG_SIZE_CROSSTU       =  100,
-      DIAG_SIZE_SEMA          = 3500,
+      DIAG_SIZE_SEMA          = 3504,
       DIAG_SIZE_ANALYSIS      =  100,
       DIAG_SIZE_REFACTORING   = 1000,
     };
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D59520: [WebAssembly... Dan Gohman via Phabricator via cfe-commits

Reply via email to