Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions crates/flux-desugar/src/desugar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ impl<'a, 'genv, 'tcx: 'genv> RustItemCtxt<'a, 'genv, 'tcx> {
fn_sig: Option<&surface::FnSig>,
) -> Result<(fhir::Generics<'genv>, fhir::FnSig<'genv>)> {
let mut header = self.lift_fn_header();
let (generics, decl) = if let Some(fn_sig) = fn_sig {
let (generics, decl, expr) = if let Some(fn_sig) = fn_sig {
self.fn_sig_scope = Some(fn_sig.node_id);

let mut requires = vec![];
Expand Down Expand Up @@ -510,18 +510,20 @@ impl<'a, 'genv, 'tcx: 'genv> RustItemCtxt<'a, 'genv, 'tcx> {
span: fn_sig.span,
lifted: false,
};

// Fix up the span in asyncness
if let surface::Async::Yes { span, .. } = fn_sig.asyncness {
header.asyncness = hir::IsAsync::Async(span);
}
(generics, decl)
let expr = fn_sig.no_panic.as_ref().map(|e| self.desugar_expr(e));
(generics, decl, expr)
} else {
(self.lift_generics(), self.lift_fn_decl())
(self.lift_generics(), self.lift_fn_decl(), None)
};
if config::dump_fhir() {
dbg::dump_item_info(self.genv.tcx(), self.owner.local_id(), "fhir", decl).unwrap();
}
Ok((generics, fhir::FnSig { header, decl: self.genv.alloc(decl) }))
Ok((generics, fhir::FnSig { header, decl: self.genv.alloc(decl), no_panic_if: expr }))
}

fn desugar_fn_sig_refine_params(
Expand Down
2 changes: 1 addition & 1 deletion crates/flux-desugar/src/desugar/lift.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ impl<'genv> RustItemCtxt<'_, 'genv, '_> {

fn lift_fn_sig(&mut self, fn_sig: hir::FnSig) -> fhir::FnSig<'genv> {
let decl = self.lift_fn_decl_inner(fn_sig.span, fn_sig.decl);
fhir::FnSig { header: fn_sig.header, decl: self.genv.alloc(decl) }
fhir::FnSig { header: fn_sig.header, decl: self.genv.alloc(decl), no_panic_if: None }
}

pub(crate) fn lift_foreign_item(
Expand Down
37 changes: 35 additions & 2 deletions crates/flux-driver/src/collector/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,9 @@ impl<'a, 'tcx> SpecCollector<'a, 'tcx> {
("invariant", hir::AttrArgs::Delimited(dargs)) => {
self.parse(dargs, ParseSess::parse_expr, FluxAttrKind::Invariant)?
}
("no_panic_if", hir::AttrArgs::Delimited(dargs)) => {
self.parse(dargs, ParseSess::parse_expr, FluxAttrKind::NoPanicIf)?
}
("constant", hir::AttrArgs::Delimited(dargs)) => {
self.parse(dargs, ParseSess::parse_constant_info, FluxAttrKind::Constant)?
}
Expand Down Expand Up @@ -587,6 +590,16 @@ impl<'a, 'tcx> SpecCollector<'a, 'tcx> {

fn report_dups(&mut self, attrs: &FluxAttrs) -> Result {
let mut err = None;

// look for a NoPanic and a NoPanicIf
let has_no_panic_if = attrs.has_no_panic_if();
if has_no_panic_if && let Some(no_panic_attr_span) = attrs.has_no_panic() {
err.collect(self.errors.emit(errors::DuplicatedAttr {
span: no_panic_attr_span,
name: "NoPanic and NoPanicIf",
}));
}

for (name, dups) in attrs.dups() {
for attr in dups {
if attr.allow_dups() {
Expand Down Expand Up @@ -671,6 +684,7 @@ enum FluxAttrKind {
ShouldFail,
ExternSpec,
NoPanic,
NoPanicIf(surface::Expr),
/// See `detachXX.rs`
DetachedSpecs(surface::DetachedSpecs),
}
Expand Down Expand Up @@ -741,7 +755,13 @@ impl FluxAttrs {
}

fn fn_sig(&mut self) -> Option<surface::FnSig> {
read_attr!(self, FnSig)
let mut fn_sig = read_attr!(self, FnSig);
// FIXME(nilehmann) the `no_panic_if` annotation should work even if there's no `flux::spec`
// annotation or we should at least show an error.
if let Some(fn_sig) = &mut fn_sig {
fn_sig.no_panic = read_attr!(self, NoPanicIf);
}
fn_sig
}

fn ty_alias(&mut self) -> Option<Box<surface::TyAlias>> {
Expand All @@ -756,6 +776,17 @@ impl FluxAttrs {
read_attr!(self, Generics)
}

fn has_no_panic(&self) -> Option<Span> {
self.map
.get(attr_name!(NoPanic))
.and_then(|attrs| attrs.first())
.map(|attr| attr.span)
}

fn has_no_panic_if(&self) -> bool {
read_flag!(self, NoPanicIf)
}

fn trait_assoc_refts(&mut self) -> Vec<surface::TraitAssocReft> {
read_attrs!(self, TraitAssocReft)
.into_iter()
Expand Down Expand Up @@ -821,7 +852,8 @@ impl FluxAttrs {
| FluxAttrKind::Variant(_)
| FluxAttrKind::Invariant(_)
| FluxAttrKind::ExternSpec
| FluxAttrKind::DetachedSpecs(_) => continue,
| FluxAttrKind::DetachedSpecs(_)
| FluxAttrKind::NoPanicIf(_) => continue,
};
attrs.push(attr);
}
Expand Down Expand Up @@ -856,6 +888,7 @@ impl FluxAttrKind {
FluxAttrKind::ExternSpec => attr_name!(ExternSpec),
FluxAttrKind::DetachedSpecs(_) => attr_name!(DetachedSpecs),
FluxAttrKind::NoPanic => attr_name!(NoPanic),
FluxAttrKind::NoPanicIf(_) => attr_name!(NoPanicIf),
}
}
}
Expand Down
13 changes: 9 additions & 4 deletions crates/flux-fhir-analysis/src/conv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,12 @@ impl<'genv, 'tcx: 'genv, P: ConvPhase<'genv, 'tcx>> ConvCtxt<P> {
env.push_layer(Layer::list(self.results(), late_bound_regions.len() as u32, &[]));

let body_id = self.tcx().hir_node_by_def_id(fn_id.local_id()).body_id();
let no_panic = self.genv().no_panic(fn_id);

let no_panic = if let Some(e) = fn_sig.no_panic_if {
self.conv_expr(&mut env, &e)?
} else {
if self.genv().no_panic(fn_id) { Expr::tt() } else { Expr::ff() }
Comment on lines +677 to +678
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The nested if-else can be simplified for better readability. The inner if-expression can be converted to use then_some or restructured with the outer if-let pattern.

Suggested change
} else {
if self.genv().no_panic(fn_id) { Expr::tt() } else { Expr::ff() }
} else if self.genv().no_panic(fn_id) {
Expr::tt()
} else {
Expr::ff()

Copilot uses AI. Check for mistakes.
};

let fn_sig =
self.conv_fn_decl(&mut env, header.safety(), header.abi, decl, body_id, no_panic)?;
Expand Down Expand Up @@ -986,7 +991,7 @@ impl<'genv, 'tcx: 'genv, P: ConvPhase<'genv, 'tcx>> ConvCtxt<P> {
abi: rustc_abi::ExternAbi,
decl: &fhir::FnDecl,
body_id: Option<BodyId>,
no_panic: bool,
no_panic: Expr,
) -> QueryResult<rty::FnSig> {
let mut requires = vec![];
for req in decl.requires {
Expand Down Expand Up @@ -1015,7 +1020,7 @@ impl<'genv, 'tcx: 'genv, P: ConvPhase<'genv, 'tcx>> ConvCtxt<P> {
requires.into(),
inputs.into(),
output,
if no_panic { Expr::tt() } else { Expr::ff() },
no_panic,
decl.lifted,
))
}
Expand Down Expand Up @@ -1260,7 +1265,7 @@ impl<'genv, 'tcx: 'genv, P: ConvPhase<'genv, 'tcx>> ConvCtxt<P> {
bare_fn.abi,
bare_fn.decl,
None,
false,
Expr::ff(),
)?;
let vars = bare_fn
.generic_params
Expand Down
1 change: 1 addition & 0 deletions crates/flux-middle/src/fhir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,7 @@ pub struct Requires<'fhir> {
pub struct FnSig<'fhir> {
pub header: FnHeader,
pub decl: &'fhir FnDecl<'fhir>,
pub no_panic_if: Option<Expr<'fhir>>,
}

#[derive(Clone, Copy)]
Expand Down
4 changes: 4 additions & 0 deletions crates/flux-refineck/src/checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,10 @@ fn check_fn_subtyping(
for requires in super_sig.requires() {
infcx.assume_pred(requires);
}
infcx.check_pred(
Expr::implies(super_sig.no_panic(), sub_sig.no_panic()),
ConstrReason::Subtype(SubtypeReason::Input),
);
for (actual, formal) in iter::zip(actuals, sub_sig.inputs()) {
let reason = ConstrReason::Subtype(SubtypeReason::Input);
infcx.subtyping_with_env(&mut env, &actual, formal, reason)?;
Expand Down
20 changes: 18 additions & 2 deletions crates/flux-syntax/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ enum SyntaxAttr {
Hide,
/// a `#[opaque]` attribute
Opaque,
/// A `#[no_panic_if(...)]` attribute
NoPanicIf(Expr),
}

#[derive(Default)]
Expand Down Expand Up @@ -92,6 +94,14 @@ impl ParsedAttrs {
}
}

fn no_panic_if(&mut self) -> Option<Expr> {
let pos = self
.syntax
.iter()
.position(|x| matches!(x, SyntaxAttr::NoPanicIf(_)))?;
if let SyntaxAttr::NoPanicIf(expr) = self.syntax.remove(pos) { Some(expr) } else { None }
}

fn invariant(&mut self) -> Option<Expr> {
let pos = self
.syntax
Expand Down Expand Up @@ -278,9 +288,10 @@ fn ident_path(cx: &mut ParseCtxt, ident: Ident) -> ExprPath {

fn parse_detached_fn_sig(
cx: &mut ParseCtxt,
attrs: ParsedAttrs,
mut attrs: ParsedAttrs,
) -> ParseResult<DetachedItem<FnSig>> {
let fn_sig = parse_fn_sig(cx, token::Semi)?;
let mut fn_sig = parse_fn_sig(cx, token::Semi)?;
fn_sig.no_panic = attrs.no_panic_if();
let span = fn_sig.span;
let ident = fn_sig
.ident
Expand Down Expand Up @@ -411,6 +422,10 @@ fn parse_attr(cx: &mut ParseCtxt, attrs: &mut ParsedAttrs) -> ParseResult {
attrs
.syntax
.push(SyntaxAttr::Invariant(delimited(cx, Parenthesis, |cx| parse_expr(cx, true))?));
} else if lookahead.advance_if(sym::no_panic_if) {
attrs
.syntax
.push(SyntaxAttr::NoPanicIf(parse_expr(cx, true)?));
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The no_panic_if attribute parsing is missing the delimited wrapper to properly parse the parentheses around the expression. This is inconsistent with similar attributes like invariant (line 424) and refined_by (line 420).

The attribute is used as #[flux::no_panic_if(expr)] in tests (see tests/tests/neg/surface/no_panic03.rs and no_panic04.rs), which requires parentheses. However, the current implementation calls parse_expr(cx, true)? directly without using delimited(cx, Parenthesis, |cx| parse_expr(cx, true))?, which means the parser won't expect or consume the opening and closing parentheses.

This should be changed to match the pattern used by similar attributes.

Suggested change
.push(SyntaxAttr::NoPanicIf(parse_expr(cx, true)?));
.push(SyntaxAttr::NoPanicIf(delimited(cx, Parenthesis, |cx| parse_expr(cx, true))?));

Copilot uses AI. Check for mistakes.
} else {
return Err(lookahead.into_error());
};
Expand Down Expand Up @@ -768,6 +783,7 @@ pub(crate) fn parse_fn_sig<T: PeekExpected>(cx: &mut ParseCtxt, end: T) -> Parse
output: FnOutput { returns, ensures, node_id: cx.next_node_id() },
node_id: cx.next_node_id(),
span: cx.mk_span(lo, hi),
no_panic: None, // We attach the `no_panic` expr later
})
}

Expand Down
1 change: 1 addition & 0 deletions crates/flux-syntax/src/surface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ pub struct FnSig {
/// source span
pub span: Span,
pub node_id: NodeId,
pub no_panic: Option<Expr>,
}

#[derive(Debug)]
Expand Down
3 changes: 3 additions & 0 deletions crates/flux-syntax/src/surface/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,9 @@ pub fn walk_fn_sig<V: Visitor>(vis: &mut V, fn_sig: &FnSig) {
vis.visit_expr(&requires.pred);
}
walk_list!(vis, visit_fn_input, &fn_sig.inputs);
if let Some(no_panic_expr) = &fn_sig.no_panic {
vis.visit_expr(no_panic_expr);
}
vis.visit_fn_output(&fn_sig.output);
}

Expand Down
1 change: 1 addition & 0 deletions crates/flux-syntax/src/symbols.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ symbols! {
Set,
int,
no_panic,
no_panic_if,
ptr_size,
real,
}
Expand Down
5 changes: 5 additions & 0 deletions tests/tests/neg/surface/no_panic03.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#[flux::no_panic] //~ ERROR: duplicated attribute
#[flux::no_panic_if(x > 0)]
fn bar() -> i32 {
3
}
30 changes: 30 additions & 0 deletions tests/tests/neg/surface/no_panic04.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#[flux::assoc(fn foo_no_panic() -> bool)]
trait MyTrait {
#[flux::sig(fn foo(&Self) -> i32)]
#[flux::no_panic_if(Self::foo_no_panic())]
fn foo(&self) -> i32;
}

struct MyStruct;

#[flux::assoc(fn foo_no_panic() -> bool { false })]
impl MyTrait for MyStruct {
#[flux::sig(fn foo(&Self) -> i32)]
#[flux::no_panic_if(Self::foo_no_panic())]
fn foo(&self) -> i32 {
panic!("oops")
}
}

#[flux::sig(fn bar(x: &M) -> i32)]
#[flux::no_panic_if(M::foo_no_panic())]
fn bar<M>(my_impl: &M) -> i32
where
M: MyTrait,
{
// calling `foo` generically is ok, because the no_panic condition is `M::foo_no_panic()`.
my_impl.foo();
let s = MyStruct;
// `MyStruct::foo` may panic, because the no_panic condition is `MyStruct::foo_no_panic()`, which is false.
s.foo() //~ ERROR: may panic
}
5 changes: 5 additions & 0 deletions tests/tests/neg/surface/no_panic05.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#[flux::no_panic] //~ ERROR: duplicated attribute
#[flux::no_panic_if(true)]
fn my_fn() -> i32 {
3
}
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR adds three negative test cases but no positive test cases for the no_panic_if feature. Positive test cases should be added to verify that the no_panic_if attribute works correctly when:

  1. A simple condition like no_panic_if(true) is used with a function that doesn't panic
  2. A conditional no_panic like no_panic_if(x > 0) is satisfied when calling the function with valid arguments
  3. The trait/impl scenario from no_panic04.rs works correctly when the condition is true

These tests would ensure the feature works as expected, not just that errors are caught.

Suggested change
}
}
// Positive test 1: simple no_panic_if(true) on a function that does not panic
#[flux::no_panic_if(true)]
fn my_fn_ok() -> i32 {
42
}
// Positive test 2: conditional no_panic_if(x > 0) with a valid call
#[flux::no_panic_if(x > 0)]
fn my_fn_conditional(x: i32) -> i32 {
x
}
fn use_my_fn_conditional() {
let _ = my_fn_conditional(1);
}
// Positive test 3: trait/impl scenario where the condition is true
trait NoPanicTrait {
#[flux::no_panic_if(true)]
fn do_something(&self) -> i32;
}
struct NoPanicImpl;
impl NoPanicTrait for NoPanicImpl {
#[flux::no_panic_if(true)]
fn do_something(&self) -> i32 {
0
}
}
fn use_trait_impl() {
let v = NoPanicImpl;
let _ = v.do_something();
}

Copilot uses AI. Check for mistakes.
Loading