[PATCH] D59520: [WebAssembly] Address review comments on r352930

This revision was automatically updated to reflect the committed changes. Closed by commit rG931fcd3ba011: [WebAssembly] Improve clang diagnostics for wasm attributes (authored by sunfishcode). Changed prior to commit: https://reviews.llvm.org/D59520?vs=268329&id=268900#toc Repository: rG LL

[PATCH] D59520: [WebAssembly] Address review comments on r352930

thakis added a comment. This breaks check-clang everywhere, e.g. here http://lab.llvm.org:8011/builders/clang-cmake-x86_64-sde-avx512-linux/builds/39534/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Awarning-flags.c Please take a look and revert if it takes a while to fix. Going forward, ple

[PATCH] D59520: [WebAssembly] Address review comments on r352930

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59520/new/ https://reviews.llvm.org/D59520 _

[PATCH] D59520: [WebAssembly] Address review comments on r352930

sunfish marked 3 inline comments as done. sunfish added a comment. I apologize again for the major delay. I've now updated the patch and addressed all of your comments. Comment at: clang/lib/Sema/SemaDecl.cpp:2594-2597 + else if (const auto *IMA = dyn_cast(Attr)) +NewAttr

[PATCH] D59520: [WebAssembly] Address review comments on r352930

sunfish updated this revision to Diff 268329. sunfish added a comment. - Add tests for redeclaration behavior - Remove disabled tests (previously marked with FIXMEs) - Made the mismatch warning more informative. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D59520: [WebAssembly] Address review comments on r352930

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:2594-2597 + else if (const auto *IMA = dyn_cast(Attr)) +NewAttr = S.mergeImportModuleAttr(D, *IMA); + else if (const auto *INA = dyn_cast(Attr)) +NewAttr = S.mergeImportNameAttr(D, *INA); -

[PATCH] D59520: [WebAssembly] Address review comments on r352930

sunfish marked 10 inline comments as done. sunfish added a comment. I apologize for the extraordinary delays here; at long last, I've now addressed your feedback. Comment at: lib/Sema/SemaDeclAttr.cpp:5781-5783 +Sema::mergeImportNameAttr(Decl *D, SourceRange Range, +

[PATCH] D59520: [WebAssembly] Address review comments on r352930

sunfish updated this revision to Diff 235005. sunfish added a comment. Address review feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59520/new/ https://reviews.llvm.org/D59520 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td cl

[PATCH] D59520: [WebAssembly] Address review comments on r352930

aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticIDs.h:39 DIAG_SIZE_CROSSTU = 100, - DIAG_SIZE_SEMA = 3500, + DIAG_SIZE_SEMA = 3504, DIAG_SIZE_ANALYSIS = 100, I think this should

[PATCH] D59520: [WebAssembly] Address review comments on r352930

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/cla

[PATCH] D59520: [WebAssembly] Address review comments on r352930

aaron.ballman added a comment. In D59520#1435728 , @sunfish wrote: > In D59520#1434854 , @aaron.ballman > wrote: > > > > Removes errnoneous use of diag::err_alias_is_definition, which turned out > > > to be ineffe

[PATCH] D59520: [WebAssembly] Address review comments on r352930

sunfish marked an inline comment as done. sunfish added inline comments. Comment at: test/Sema/attr-wasm.c:3 + +void name_a() {} + aaron.ballman wrote: > Was this intended to be used somewhere? Probably can just be removed if not. No, you're right that we don't n

[PATCH] D59520: [WebAssembly] Address review comments on r352930

sunfish added a comment. In D59520#1434854 , @aaron.ballman wrote: > > 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 dete

[PATCH] D59520: [WebAssembly] Address review comments on r352930

aaron.ballman added a comment. I love functional changes that remove code and add tests -- thank you for these! > 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 detecti

[PATCH] D59520: [WebAssembly] Address review comments on r352930

sunfish created this revision. sunfish added a reviewer: aaron.ballman. Herald added subscribers: aheejin, jgravelle-google, sbc100, dschuff. Herald added a project: clang. This patch addresses the review comments on r352930: - Removes redundant diagnostic checking code - Removes errnoneous use o