aaron.ballman added inline comments.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:2594-2597
+ 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);
----------------
You should probably have some tests for the redeclaration behavior somewhere.
The way you usually do this is to declare the function with the attribute, then
declare the function again without the attribute but see that it is in fact
inherited. We sometimes use AST dumping tests for this.
================
Comment at: clang/test/Sema/attr-wasm.c:11
+
+__attribute__((import_name("foo"))) void name_e() {} //FIXME-expected-error
{{import name cannot be applied to a function with a definition}}
+
----------------
Are you intending to fix this as part of the patch?
================
Comment at: clang/test/Sema/attr-wasm.c:15
+
+void name_z() __attribute__((import_name("bar"))); //expected-warning {{import
name does not match previous declaration}}
+
----------------
Uncertain how important you think this may be: should we include the import
names in this diagnostic? e.g., `import name (%0) does not match the import
name (%1) of the previous declaration` or somesuch? I don't know how often
users will hide these attributes behind macros, which is the case I was worried
might be confusing.
================
Comment at: clang/test/Sema/attr-wasm.c:25
+
+void module_e() __attribute__((import_module("foo"))) {}
//FIXME-expected-error {{import module cannot be applied to a function with a
definition}}
+
----------------
Same question about this FIXME.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59520/new/
https://reviews.llvm.org/D59520
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
