[PATCH] D100511: [clang] Modify diagnostic level from err to warn: anyx86_interrupt_regsave
This revision was automatically updated to reflect the committed changes. Closed by commit rG938b863bb53f: [clang][patch] Modify diagnostic level from err to warn… (authored by mibintc). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100511/new/ https://reviews.llvm.org/D100511 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/SemaExpr.cpp clang/test/Sema/attr-x86-interrupt.c Index: clang/test/Sema/attr-x86-interrupt.c === --- clang/test/Sema/attr-x86-interrupt.c +++ clang/test/Sema/attr-x86-interrupt.c @@ -51,7 +51,7 @@ __attribute__((no_caller_saved_registers)) #else // expected-note@+3 {{'foo9' declared here}} -// expected-error@+4 {{interrupt service routine may only call a function with attribute 'no_caller_saved_registers'}} +// expected-warning@+4 {{interrupt service routine should only call a function with attribute 'no_caller_saved_registers'}} #endif void foo9(int *a, Arg2Type b) {} __attribute__((interrupt)) void fooA(int *a, Arg2Type b) { Index: clang/lib/Sema/SemaExpr.cpp === --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -6607,7 +6607,7 @@ } if (Caller->hasAttr() && ((!FDecl || !FDecl->hasAttr( { - Diag(Fn->getExprLoc(), diag::err_anyx86_interrupt_regsave); + Diag(Fn->getExprLoc(), diag::warn_anyx86_interrupt_regsave); if (FDecl) Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl; } Index: clang/include/clang/Basic/DiagnosticSemaKinds.td === --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -293,9 +293,10 @@ "a pointer as the first parameter|a %2 type as the second parameter}1">; def err_anyx86_interrupt_called : Error< "interrupt service routine cannot be called directly">; -def err_anyx86_interrupt_regsave : Error< - "interrupt service routine may only call a function" - " with attribute 'no_caller_saved_registers'">; +def warn_anyx86_interrupt_regsave : Warning< + "interrupt service routine should only call a function" + " with attribute 'no_caller_saved_registers'">, + InGroup>; def warn_arm_interrupt_calling_convention : Warning< "call to function without interrupt attribute could clobber interruptee's VFP registers">, InGroup; Index: clang/test/Sema/attr-x86-interrupt.c === --- clang/test/Sema/attr-x86-interrupt.c +++ clang/test/Sema/attr-x86-interrupt.c @@ -51,7 +51,7 @@ __attribute__((no_caller_saved_registers)) #else // expected-note@+3 {{'foo9' declared here}} -// expected-error@+4 {{interrupt service routine may only call a function with attribute 'no_caller_saved_registers'}} +// expected-warning@+4 {{interrupt service routine should only call a function with attribute 'no_caller_saved_registers'}} #endif void foo9(int *a, Arg2Type b) {} __attribute__((interrupt)) void fooA(int *a, Arg2Type b) { Index: clang/lib/Sema/SemaExpr.cpp === --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -6607,7 +6607,7 @@ } if (Caller->hasAttr() && ((!FDecl || !FDecl->hasAttr( { - Diag(Fn->getExprLoc(), diag::err_anyx86_interrupt_regsave); + Diag(Fn->getExprLoc(), diag::warn_anyx86_interrupt_regsave); if (FDecl) Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl; } Index: clang/include/clang/Basic/DiagnosticSemaKinds.td === --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -293,9 +293,10 @@ "a pointer as the first parameter|a %2 type as the second parameter}1">; def err_anyx86_interrupt_called : Error< "interrupt service routine cannot be called directly">; -def err_anyx86_interrupt_regsave : Error< - "interrupt service routine may only call a function" - " with attribute 'no_caller_saved_registers'">; +def warn_anyx86_interrupt_regsave : Warning< + "interrupt service routine should only call a function" + " with attribute 'no_caller_saved_registers'">, + InGroup>; def warn_arm_interrupt_calling_convention : Warning< "call to function without interrupt attribute could clobber interruptee's VFP registers">, InGroup; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D100511: [clang] Modify diagnostic level from err to warn: anyx86_interrupt_regsave
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100511/new/ https://reviews.llvm.org/D100511 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D100511: [clang] Modify diagnostic level from err to warn: anyx86_interrupt_regsave
mibintc updated this revision to Diff 337789. mibintc marked an inline comment as done. mibintc added a comment. I added the InGroup rule for the new warning diagnostic like Aaron requested Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100511/new/ https://reviews.llvm.org/D100511 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/SemaExpr.cpp clang/test/Sema/attr-x86-interrupt.c Index: clang/test/Sema/attr-x86-interrupt.c === --- clang/test/Sema/attr-x86-interrupt.c +++ clang/test/Sema/attr-x86-interrupt.c @@ -51,7 +51,7 @@ __attribute__((no_caller_saved_registers)) #else // expected-note@+3 {{'foo9' declared here}} -// expected-error@+4 {{interrupt service routine may only call a function with attribute 'no_caller_saved_registers'}} +// expected-warning@+4 {{interrupt service routine should only call a function with attribute 'no_caller_saved_registers'}} #endif void foo9(int *a, Arg2Type b) {} __attribute__((interrupt)) void fooA(int *a, Arg2Type b) { Index: clang/lib/Sema/SemaExpr.cpp === --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -6607,7 +6607,7 @@ } if (Caller->hasAttr() && ((!FDecl || !FDecl->hasAttr( { - Diag(Fn->getExprLoc(), diag::err_anyx86_interrupt_regsave); + Diag(Fn->getExprLoc(), diag::warn_anyx86_interrupt_regsave); if (FDecl) Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl; } Index: clang/include/clang/Basic/DiagnosticSemaKinds.td === --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -293,9 +293,10 @@ "a pointer as the first parameter|a %2 type as the second parameter}1">; def err_anyx86_interrupt_called : Error< "interrupt service routine cannot be called directly">; -def err_anyx86_interrupt_regsave : Error< - "interrupt service routine may only call a function" - " with attribute 'no_caller_saved_registers'">; +def warn_anyx86_interrupt_regsave : Warning< + "interrupt service routine should only call a function" + " with attribute 'no_caller_saved_registers'">, + InGroup>; def warn_arm_interrupt_calling_convention : Warning< "call to function without interrupt attribute could clobber interruptee's VFP registers">, InGroup; Index: clang/test/Sema/attr-x86-interrupt.c === --- clang/test/Sema/attr-x86-interrupt.c +++ clang/test/Sema/attr-x86-interrupt.c @@ -51,7 +51,7 @@ __attribute__((no_caller_saved_registers)) #else // expected-note@+3 {{'foo9' declared here}} -// expected-error@+4 {{interrupt service routine may only call a function with attribute 'no_caller_saved_registers'}} +// expected-warning@+4 {{interrupt service routine should only call a function with attribute 'no_caller_saved_registers'}} #endif void foo9(int *a, Arg2Type b) {} __attribute__((interrupt)) void fooA(int *a, Arg2Type b) { Index: clang/lib/Sema/SemaExpr.cpp === --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -6607,7 +6607,7 @@ } if (Caller->hasAttr() && ((!FDecl || !FDecl->hasAttr( { - Diag(Fn->getExprLoc(), diag::err_anyx86_interrupt_regsave); + Diag(Fn->getExprLoc(), diag::warn_anyx86_interrupt_regsave); if (FDecl) Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl; } Index: clang/include/clang/Basic/DiagnosticSemaKinds.td === --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -293,9 +293,10 @@ "a pointer as the first parameter|a %2 type as the second parameter}1">; def err_anyx86_interrupt_called : Error< "interrupt service routine cannot be called directly">; -def err_anyx86_interrupt_regsave : Error< - "interrupt service routine may only call a function" - " with attribute 'no_caller_saved_registers'">; +def warn_anyx86_interrupt_regsave : Warning< + "interrupt service routine should only call a function" + " with attribute 'no_caller_saved_registers'">, + InGroup>; def warn_arm_interrupt_calling_convention : Warning< "call to function without interrupt attribute could clobber interruptee's VFP registers">, InGroup; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D100511: [clang] Modify diagnostic level from err to warn: anyx86_interrupt_regsave
aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:296-298 +def warn_anyx86_interrupt_regsave : Warning< + "interrupt service routine should only call a function" " with attribute 'no_caller_saved_registers'">; You still need to add the diagnostic to a diagnostic group, otherwise users don't have a way to selectively disable the diagnostic. Feel free to pick a different group for the -W flag; I just took a stab at a possible name. This should fix the failing CI pipelines too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100511/new/ https://reviews.llvm.org/D100511 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D100511: [clang] Modify diagnostic level from err to warn: anyx86_interrupt_regsave
mibintc marked an inline comment as done. mibintc added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:299 + " with attribute 'no_caller_saved_registers'">, + InGroup; def warn_arm_interrupt_calling_convention : Warning< rsmith wrote: > Please don't add warnings directly to `-Wextra`; this should have its own > flag so that people who want to do this can turn the warning off without > turning off all of `-Wextra`. Thanks, I had thought about removing InGroup before uploading to review but hurrying at the end of my work day and forgot. I was expecting the lit test to fail and I'd have to add Wextra onto the RUN line but I didn't need to do that, the diagnostic was produced without enabling. Anyway I got rid of it like you asked. BTW the only other diagnostic in this file marked Wextra is the arm diagnostic. That was added in https://reviews.llvm.org/D28820 "warn_arm_interrupt_calling_convention" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100511/new/ https://reviews.llvm.org/D100511 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D100511: [clang] Modify diagnostic level from err to warn: anyx86_interrupt_regsave
mibintc updated this revision to Diff 337731. mibintc added a comment. I removed the diagnostic from InGroup, that's the only change from previous revision Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100511/new/ https://reviews.llvm.org/D100511 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/SemaExpr.cpp clang/test/Sema/attr-x86-interrupt.c Index: clang/test/Sema/attr-x86-interrupt.c === --- clang/test/Sema/attr-x86-interrupt.c +++ clang/test/Sema/attr-x86-interrupt.c @@ -51,7 +51,7 @@ __attribute__((no_caller_saved_registers)) #else // expected-note@+3 {{'foo9' declared here}} -// expected-error@+4 {{interrupt service routine may only call a function with attribute 'no_caller_saved_registers'}} +// expected-warning@+4 {{interrupt service routine should only call a function with attribute 'no_caller_saved_registers'}} #endif void foo9(int *a, Arg2Type b) {} __attribute__((interrupt)) void fooA(int *a, Arg2Type b) { Index: clang/lib/Sema/SemaExpr.cpp === --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -6607,7 +6607,7 @@ } if (Caller->hasAttr() && ((!FDecl || !FDecl->hasAttr( { - Diag(Fn->getExprLoc(), diag::err_anyx86_interrupt_regsave); + Diag(Fn->getExprLoc(), diag::warn_anyx86_interrupt_regsave); if (FDecl) Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl; } Index: clang/include/clang/Basic/DiagnosticSemaKinds.td === --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -293,8 +293,8 @@ "a pointer as the first parameter|a %2 type as the second parameter}1">; def err_anyx86_interrupt_called : Error< "interrupt service routine cannot be called directly">; -def err_anyx86_interrupt_regsave : Error< - "interrupt service routine may only call a function" +def warn_anyx86_interrupt_regsave : Warning< + "interrupt service routine should only call a function" " with attribute 'no_caller_saved_registers'">; def warn_arm_interrupt_calling_convention : Warning< "call to function without interrupt attribute could clobber interruptee's VFP registers">, Index: clang/test/Sema/attr-x86-interrupt.c === --- clang/test/Sema/attr-x86-interrupt.c +++ clang/test/Sema/attr-x86-interrupt.c @@ -51,7 +51,7 @@ __attribute__((no_caller_saved_registers)) #else // expected-note@+3 {{'foo9' declared here}} -// expected-error@+4 {{interrupt service routine may only call a function with attribute 'no_caller_saved_registers'}} +// expected-warning@+4 {{interrupt service routine should only call a function with attribute 'no_caller_saved_registers'}} #endif void foo9(int *a, Arg2Type b) {} __attribute__((interrupt)) void fooA(int *a, Arg2Type b) { Index: clang/lib/Sema/SemaExpr.cpp === --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -6607,7 +6607,7 @@ } if (Caller->hasAttr() && ((!FDecl || !FDecl->hasAttr( { - Diag(Fn->getExprLoc(), diag::err_anyx86_interrupt_regsave); + Diag(Fn->getExprLoc(), diag::warn_anyx86_interrupt_regsave); if (FDecl) Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl; } Index: clang/include/clang/Basic/DiagnosticSemaKinds.td === --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -293,8 +293,8 @@ "a pointer as the first parameter|a %2 type as the second parameter}1">; def err_anyx86_interrupt_called : Error< "interrupt service routine cannot be called directly">; -def err_anyx86_interrupt_regsave : Error< - "interrupt service routine may only call a function" +def warn_anyx86_interrupt_regsave : Warning< + "interrupt service routine should only call a function" " with attribute 'no_caller_saved_registers'">; def warn_arm_interrupt_calling_convention : Warning< "call to function without interrupt attribute could clobber interruptee's VFP registers">, ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D100511: [clang] Modify diagnostic level from err to warn: anyx86_interrupt_regsave
rsmith added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:299 + " with attribute 'no_caller_saved_registers'">, + InGroup; def warn_arm_interrupt_calling_convention : Warning< Please don't add warnings directly to `-Wextra`; this should have its own flag so that people who want to do this can turn the warning off without turning off all of `-Wextra`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100511/new/ https://reviews.llvm.org/D100511 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D100511: [clang] Modify diagnostic level from err to warn: anyx86_interrupt_regsave
mibintc created this revision. mibintc added a reviewer: aaron.ballman. Herald added a subscriber: pengfei. mibintc requested review of this revision. Herald added a project: clang. I got a bug report that https://reviews.llvm.org/D97764 which introduced this diagnostic, was causing problems in interrupt service routines, complaining about functions like abort, exit, or the x86intrin routines (that are really just inlined asm). I think modifying the diagnostic level from error to warn is the safest way to deal with the problem. Checking "always inline" attribute on the called function isn't itself enough without knowing that the called function is leaf. (Separately, perhaps x86intrin functions should be decorated with attribute 'no_caller_saved_registers' in their declarations.) Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D100511 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/SemaExpr.cpp clang/test/Sema/attr-x86-interrupt.c Index: clang/test/Sema/attr-x86-interrupt.c === --- clang/test/Sema/attr-x86-interrupt.c +++ clang/test/Sema/attr-x86-interrupt.c @@ -51,7 +51,7 @@ __attribute__((no_caller_saved_registers)) #else // expected-note@+3 {{'foo9' declared here}} -// expected-error@+4 {{interrupt service routine may only call a function with attribute 'no_caller_saved_registers'}} +// expected-warning@+4 {{interrupt service routine should only call a function with attribute 'no_caller_saved_registers'}} #endif void foo9(int *a, Arg2Type b) {} __attribute__((interrupt)) void fooA(int *a, Arg2Type b) { Index: clang/lib/Sema/SemaExpr.cpp === --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -6607,7 +6607,7 @@ } if (Caller->hasAttr() && ((!FDecl || !FDecl->hasAttr( { - Diag(Fn->getExprLoc(), diag::err_anyx86_interrupt_regsave); + Diag(Fn->getExprLoc(), diag::warn_anyx86_interrupt_regsave); if (FDecl) Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl; } Index: clang/include/clang/Basic/DiagnosticSemaKinds.td === --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -293,9 +293,10 @@ "a pointer as the first parameter|a %2 type as the second parameter}1">; def err_anyx86_interrupt_called : Error< "interrupt service routine cannot be called directly">; -def err_anyx86_interrupt_regsave : Error< - "interrupt service routine may only call a function" - " with attribute 'no_caller_saved_registers'">; +def warn_anyx86_interrupt_regsave : Warning< + "interrupt service routine should only call a function" + " with attribute 'no_caller_saved_registers'">, + InGroup; def warn_arm_interrupt_calling_convention : Warning< "call to function without interrupt attribute could clobber interruptee's VFP registers">, InGroup; Index: clang/test/Sema/attr-x86-interrupt.c === --- clang/test/Sema/attr-x86-interrupt.c +++ clang/test/Sema/attr-x86-interrupt.c @@ -51,7 +51,7 @@ __attribute__((no_caller_saved_registers)) #else // expected-note@+3 {{'foo9' declared here}} -// expected-error@+4 {{interrupt service routine may only call a function with attribute 'no_caller_saved_registers'}} +// expected-warning@+4 {{interrupt service routine should only call a function with attribute 'no_caller_saved_registers'}} #endif void foo9(int *a, Arg2Type b) {} __attribute__((interrupt)) void fooA(int *a, Arg2Type b) { Index: clang/lib/Sema/SemaExpr.cpp === --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -6607,7 +6607,7 @@ } if (Caller->hasAttr() && ((!FDecl || !FDecl->hasAttr( { - Diag(Fn->getExprLoc(), diag::err_anyx86_interrupt_regsave); + Diag(Fn->getExprLoc(), diag::warn_anyx86_interrupt_regsave); if (FDecl) Diag(FDecl->getLocation(), diag::note_callee_decl) << FDecl; } Index: clang/include/clang/Basic/DiagnosticSemaKinds.td === --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -293,9 +293,10 @@ "a pointer as the first parameter|a %2 type as the second parameter}1">; def err_anyx86_interrupt_called : Error< "interrupt service routine cannot be called directly">; -def err_anyx86_interrupt_regsave : Error< - "interrupt service routine may only call a function" - " with attribute 'no_caller_saved_registers'">; +def warn_anyx86_interrupt_regsave : Warning< + "interrupt service routine should only call a function" + " with attribute 'no_caller_saved_registers'">, + InGroup; def warn_arm_interrup