[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added a comment. In D106030#3018754 , @aeubanks wrote: > Would it be ok to split this into two attributes, dontcall-warn and > dontcall-error? So we rely less on the Clang AST when determining if a > dontcall call should be emitted as a

[PATCH] D106030: [Clang] add support for error+warning fn attrs

aeubanks added a comment. Would it be ok to split this into two attributes, dontcall-warn and dontcall-error? So we rely less on the Clang AST when determining if a dontcall call should be emitted as a warning or an error. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added a comment. Fixup: https://reviews.llvm.org/D108718 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106030/new/ https://reviews.llvm.org/D106030 ___ cfe-commits mailing list cfe-commit

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added a comment. I'm getting test failures for the newly added llvm/test/CodeGen/X86/attr-dontcall.ll, thought there's not much info other than > Exit Code: 1 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106030/new/ https://revie

[PATCH] D106030: [Clang] add support for error+warning fn attrs

This revision was automatically updated to reflect the committed changes. Closed by commit rG846e562dcc6a: [Clang] add support for error+warning fn attrs (authored by nickdesaulniers). Changed prior to commit: https://reviews.llvm.org/D106030?vs=366717&id=368678#toc Repository: rG LLVM Githu

[PATCH] D106030: [Clang] add support for error+warning fn attrs

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Clang parts LGTM, thank you for this! Comment at: clang/test/Sema/attr-error.c:31-32 + +__attribute__((error("foo"))) int bad5(void); // expected-error {{'err

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/test/Sema/attr-error.c:31-32 + +__attribute__((error("foo"))) int bad5(void); // expected-error {{'error' and 'warning' attributes are not compatible}} +__attribute__((warning("foo"))) int bad5(void); // expected-note {{

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 366717. nickdesaulniers added a comment. - reorder diag vs note Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106030/new/ https://reviews.llvm.org/D106030 Files: clang/docs/ReleaseNotes.rst clang/i

[PATCH] D106030: [Clang] add support for error+warning fn attrs

aaron.ballman added inline comments. Comment at: clang/test/Sema/attr-error.c:31-32 + +__attribute__((error("foo"))) int bad5(void); // expected-error {{'error' and 'warning' attributes are not compatible}} +__attribute__((warning("foo"))) int bad5(void); // expected-note {{co

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/test/Sema/attr-error.c:31-32 + +__attribute__((error("foo"))) int bad5(void); // expected-error {{'error' and 'warning' attributes are not compatible}} +__attribute__((warning("foo"))) int bad5(void); // expected-note {{

[PATCH] D106030: [Clang] add support for error+warning fn attrs

aaron.ballman added inline comments. Comment at: clang/test/Sema/attr-error.c:31-32 + +__attribute__((error("foo"))) int bad5(void); // expected-error {{'error' and 'warning' attributes are not compatible}} +__attribute__((warning("foo"))) int bad5(void); // expected-note {{co

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 366356. nickdesaulniers added a comment. - remove unnecessary parens Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106030/new/ https://reviews.llvm.org/D106030 Files: clang/docs/ReleaseNotes.rst cl

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:3368 + Diag(Old->getLocation(), diag::note_previous_declaration); + New->dropAttr(); +} Should be dropping `ErrorAttr`. Comment at: clang/test/Sema/att

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 366355. nickdesaulniers marked 4 inline comments as done. nickdesaulniers added a comment. - fix tests on windows, let diag eng format decl, drop correct attr, refactor to handle merging as InheritableAttr Repository: rG LLVM Github Monorepo CHAN

[PATCH] D106030: [Clang] add support for error+warning fn attrs

aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CodeGenAction.cpp:783 +: diag::warn_fe_backend_warning_attr) +<< FD->getDeclName() << EA->getUserDiagnostic(); + } The diagnostics engine knows

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:964-965 +bool match = +(EA->isError() && NewAttrSpellingIndex < ErrorAttr::GNU_warning) || +(EA->isWarning() && NewAttrSpellingIndex >= ErrorAttr::GNU_warning); +if (!match)

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 366050. nickdesaulniers marked 4 inline comments as done. nickdesaulniers added a comment. - combine c++ w/ c test - use -verify= rather than -D - remove REQUIRES - go back to string comparison Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D106030: [Clang] add support for error+warning fn attrs

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:964-965 +bool match = +(EA->isError() && NewAttrSpellingIndex < ErrorAttr::GNU_warning) || +(EA->isWarning() && NewAttrSpellingIndex >= ErrorAttr::GNU_warning); +if (!match) {

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11172 +def err_attribute_removed_on_redeclaration : Error< + "'%0' attribute removed from redeclared variable">; aaron.ballman wrote: > This diagnostic is a bit

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 364892. nickdesaulniers marked 8 inline comments as done. nickdesaulniers added a comment. - rebase on D107613 , reuse diag::err_attribute_missing_on_first_decl - test matching redeclarations - remove getSpelling meth

[PATCH] D106030: [Clang] add support for error+warning fn attrs

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticFrontendKinds.td:76 +def err_fe_backend_error_attr : + Error<"call to %0 declared with attribute error: %1">, BackendInfo; +def warn_fe_backend_warning_attr : ===

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers marked an inline comment as done. nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1205 def BackendOptimizationFailure : DiagGroup<"pass-failed">; +def BackendUserDiagnostic : DiagGroup<"user-diagnostic">; ---

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 364280. nickdesaulniers marked 5 inline comments as done. nickdesaulniers edited the summary of this revision. nickdesaulniers added a comment. - remove stale todos, change warning to -Wwarning-attributes to match GCC, update docs Repository: rG L

[PATCH] D106030: [Clang] add support for error+warning fn attrs

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:6057 +assertions that depend on optimizations, while providing diagnostics +pointing to precise locations of the call site in the source. + }]; nickdesaulniers wrote:

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:6057 +assertions that depend on optimizations, while providing diagnostics +pointing to precise locations of the call site in the source. + }]; @aaron.ballman anythi

[PATCH] D106030: [Clang] add support for error+warning fn attrs

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3819 + let Args = [StringArgument<"UserDiagnostic">]; + let Subjects = SubjectList<[Function], ErrorDiag>; + let Documentation = [ErrorAttrDocs]; nickdesaulniers wrote: > aaron.

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers planned changes to this revision. nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3819 + let Args = [StringArgument<"UserDiagnostic">]; + let Subjects = SubjectList<[Function], ErrorDiag>; + let Documentation = [ErrorAttrDoc

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 363888. nickdesaulniers marked 8 inline comments as done. nickdesaulniers edited the summary of this revision. nickdesaulniers added a comment. Herald added subscribers: ormris, steven_wu. - LTO test, diagnose inheritability Repository: rG LLVM Git

[PATCH] D106030: [Clang] add support for error+warning fn attrs

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3816 + +def Error : Attr { + let Spellings = [GCC<"error">]; nickdesaulniers wrote: > aaron.ballman wrote: > > nickdesaulniers wrote: > > > aaron.ballman wrote: > > > > I think th

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3816 + +def Error : Attr { + let Spellings = [GCC<"error">]; aaron.ballman wrote: > nickdesaulniers wrote: > > aaron.ballman wrote: > > > I think this should be an inheritable a

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/lib/CodeGen/CodeGenAction.cpp:783 + else +Diags.Report(DiagID) << CalleeName << D.getMsgStr(); +} nickdesaulniers wrote: > aaron.ballman wrote: > > nickdesaulniers wrote: > > > aaron.ballman wrote: > >

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:6026 +depend on optimizations, while providing diagnostics pointing to precise +locations of the call site in the source. + }]; aaron.ballman wrote: > nickdesaulnier

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 363581. nickdesaulniers marked 2 inline comments as done. nickdesaulniers edited the summary of this revision. nickdesaulniers added a comment. - reusing existing Diag/Notes, remove test comment, add test for indirect call Repository: rG LLVM Githu

[PATCH] D106030: [Clang] add support for error+warning fn attrs

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:375-378 +def warn_changed_error_warning_attr_text : Warning< + "duplicate attribute changes text from %0 to %1">, + InGroup; + FWIW, elsewhere we typically use `

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/test/Frontend/backend-attribute-error-warning.c:29 + duplicate_errors(); // expected-error {{call to 'duplicate_errors' declared with attribute error: two}} + // TODO: why is this a warning not an error + duplicate_

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3823 + +def Warning : Attr { + let Spellings = [GCC<"warning">]; aaron.ballman wrote: > nickdesaulniers wrote: > > aaron.ballman wrote: > > > Given that the only functional diff

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 360324. nickdesaulniers marked 4 inline comments as done. nickdesaulniers edited the summary of this revision. nickdesaulniers added a comment. - merge ErrorAttr with WarningAttr - fix dumb short circuit bug - rename diag group - handle multiple instan

[PATCH] D106030: [Clang] add support for error+warning fn attrs

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3816 + +def Error : Attr { + let Spellings = [GCC<"error">]; nickdesaulniers wrote: > aaron.ballman wrote: > > I think this should be an inheritable attribute (same below) so that

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3816 + +def Error : Attr { + let Spellings = [GCC<"error">]; aaron.ballman wrote: > I think this should be an inheritable attribute (same below) so that > redeclarations get th

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 359912. nickdesaulniers marked 5 inline comments as done. nickdesaulniers edited the summary of this revision. nickdesaulniers added a comment. Herald added a subscriber: pengfei. - change IR Attr to dontcall - check during ISel(s) - rename td diag - h

[PATCH] D106030: [Clang] add support for error+warning fn attrs

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3816 + +def Error : Attr { + let Spellings = [GCC<"error">]; I think this should be an inheritable attribute (same below) so that redeclarations get the marking as well. However

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers added a comment. In D106030#2878897 , @arsenm wrote: > Adding something to the IR for the sole purpose of producing a diagnostic > feels really weird. I'm not sure I see why the frontend can't see this > attribute and directly warn I in

[PATCH] D106030: [Clang] add support for error+warning fn attrs

george.burgess.iv added a comment. > Adding something to the IR for the sole purpose of producing a diagnostic > feels really weird. I'm not sure I see why the frontend can't see this > attribute and directly warn To add a bit more clarification, the goal of this attribute is specifically to e

[PATCH] D106030: [Clang] add support for error+warning fn attrs

efriedma added a comment. In D106030#2878897 , @arsenm wrote: > Adding something to the IR for the sole purpose of producing a diagnostic > feels really weird. I'm not sure I see why the frontend can't see this > attribute and directly warn In C++, you

[PATCH] D106030: [Clang] add support for error+warning fn attrs

arsenm added a comment. Adding something to the IR for the sole purpose of producing a diagnostic feels really weird. I'm not sure I see why the frontend can't see this attribute and directly warn Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1060

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers updated this revision to Diff 358814. nickdesaulniers added a comment. - remove accidentally committed logging statement Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106030/new/ https://reviews.llvm.org/D106030 Files: clang/docs

[PATCH] D106030: [Clang] add support for error+warning fn attrs

arsenm added inline comments. Comment at: llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp:208 } +// TODO: is this the best pass for this? +for (BasicBlock &BB : F) { Why not just do in in codegen in IRTranslator/SelectionDAGBuilder?

[PATCH] D106030: [Clang] add support for error+warning fn attrs

nickdesaulniers created this revision. nickdesaulniers added reviewers: rsmith, aaron.ballman, craig.topper, efriedma, lebedev.ri, jdoerfert, arsenm. Herald added subscribers: dexonsmith, hiraditya. nickdesaulniers requested review of this revision. Herald added subscribers: llvm-commits, cfe-comm