ICE: "impossible range in AST" with a lone ... (inclusive range syntax)

[nodakai]

#![feature(inclusive_range_syntax)]

fn main() {
    ...;
}

$ RUST_BACKTRACE=1 rustc lone-incl-range.rs
error: internal compiler error: unexpected panic
note: the compiler unexpectedly panicked. this is a bug.
note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports
thread 'rustc' panicked at 'impossible range in AST', src/librustc_front/lowering.rs:1291
stack backtrace:
   1:     0x7f355e9a7080 - sys::backtrace::tracing::imp::write::h6cb2e6cc562f1157ajv
   2:     0x7f355e9b127f - panicking::default_handler::_$u7b$$u7b$closure$u7d$$u7d$::closure.44459
   3:     0x7f355e9b0df3 - panicking::default_handler::h60d5c200be7610ffr7z
   4:     0x7f355e9791dc - sys_common::unwind::begin_unwind_inner::h97e84635bfe5e882t7t
   5:     0x7f355b13787f - sys_common::unwind::begin_unwind::h8452793534382620991
   6:     0x7f355b1510f0 - lowering::lower_expr::_$u7b$$u7b$closure$u7d$$u7d$::closure.31555
   7:     0x7f355b13be50 - lowering::lower_expr::h65bdcc84b4cf88da2vr
   8:     0x7f355b142dd4 - vec::Vec<T>.FromIterator<T>::from_iter::h12825153023271112045
   9:     0x7f355b142bc4 - lowering::lower_block::h8dc738a2417bb8b6Mar
  10:     0x7f355b143adb - lowering::lower_item_kind::hc3b0928913e39eb6Dbr
  11:     0x7f355b148dce - lowering::lower_item::h92bd16a94ad331ab3mr
  12:     0x7f355b145e8e - lowering::ItemLowerer<'lcx, 'interner>.Visitor<'lcx>::visit_item::h5af224959762239erkr
  13:     0x7f355b14a46b - lowering::lower_crate::ha58482b99d537c2e0kr
  14:     0x7f355eeb9176 - driver::compile_input::hbe58952840e9fa26Pca
  15:     0x7f355eea81b6 - run_compiler::h43154ab0ccc9972aJPc
  16:     0x7f355eea5931 - sys_common::unwind::try::try_fn::h13700893182557476600
  17:     0x7f355e9a48fb - __rust_try
  18:     0x7f355e9a488d - sys_common::unwind::inner_try::h602712f93ad93b38v4t
  19:     0x7f355eea617a - boxed::F.FnBox<A>::call_box::h17985710921385728096
  20:     0x7f355e9af389 - sys::thread::Thread::new::thread_start::hc6901d1da7801591h4y
  21:     0x7f3557d7d9d0 - start_thread
  22:     0x7f355e6398fc - clone
  23:                0x0 - <unknown>
$ rustc --version --verbose
rustc 1.9.0-dev (211296dda 2016-03-13)
binary: rustc
commit-hash: 211296ddabb6a307a0f60af1cb93f39b85ba5bbd
commit-date: 2016-03-13
host: x86_64-unknown-linux-gnu
release: 1.9.0-dev

[durka]

This is my fault. I thought I had prevented that case in the parser.

[petrochenkov]

@durka

I thought I had prevented that case in the parser.

In general, all kinds of weird things can be generated by syntax extensions, so erroneous situations are better tested post-expansion even if they can't be produced by the parser.

[durka]

@petrochenkov maybe I will loosen the restrictions in the parser and turn
this panic into a real error?

On Mon, Mar 14, 2016 at 12:38 PM, Vadim Petrochenkov <
[email protected]> wrote:

@durka https://github.com/durka

I thought I had prevented that case in the parser.

In general, all kinds of weird things can be generated by syntax
extensions, so erroneous situations are better tested post-expansion even
if they can't be produced by a parser.

—
Reply to this email directly or view it on GitHub
#32245 (comment).

[petrochenkov]

maybe I will loosen the restrictions in the parser and turn this panic into a real error?

I haven't looked carefully at this particular case, but probably yes.

[nodakai]

FYI, this is accepted as a valid Rust program:

fn main() {
    ..;
    0..;
    ..1;
    0..1;
}

[durka]

@nodakai yeah, all types of half-open ranges are allowed but currently only
double-bounded (a...b) and end-only (...b) inclusive ranges are
allowed. This is enforced by the panic you found in HIR lowering, which I
am going to replace with an error to the effect of "blah blah syntax is
still being considered, see tracking issue".

On Mon, Mar 14, 2016 at 9:02 PM, Kai Noda [email protected] wrote:

FYI, this is accepted as a valid Rust program:

fn main() {
..;
0..;
..1;
0..1;
}

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#32245 (comment)

[durka]

Hmm actually the HIR lowering panics in several places (if it finds unexpanded macros etc) and doesn't seem to emit any errors or warnings currently, contradicting @petrochenkov's advice. I'm not even sure if the span_err machinery is hooked up in there. Is this the wrong place to enforce stuff like this? cc @nrc

[nrc]

There is no convention for where well-formedness constraints on the AST/HIR should be enforced. I feel like the HIR lowering is a good place to do that since once we are in the compiler proper, the user is much less able to change things, whereas (as @petrochenkov points out) the parser can be subverted via syntax extensions. However, we are not very good at doing this.

The span_err stuff should work fine everywhere in the compiler, and should definitely be OK to do in the HIR lowering.

Note particularly on unexpanded macros, I think that is ok to assume, since it is guaranteed by the macro expander, not the parser and so cannot (afaik) be subverted by syntax extensions. However, if one wanted to be super-struct one could imagine a safety boundary between libsyntax and the compiler and then we should not assume all macros are expanded because it is enforced by libsyntax, not the compiler.

[durka]

@nrc How can I signal an error with just a LoweringContext? It's got a reference to the Session but only as a &NodeIdAssigner so I think I can't really do anything with it unless I change that type to store a &Session. Also, what should I leave behind in the HIR after signaling an error? Perhaps a lowered loop {}, or a new ExprError?

[nrc]

@durka adding a &Session field sounds fine. For things that just shouldn't happen (e.g., due to a bad syntax extension) then a fatal error is the best bet, then we don't need to cope with it later. Are there actual user errors we might catch in lowering? If so then an ExprError, but I'd really like to avoid that if possible (maybe make all errors fatal, if that is not too obnoxious).

[durka]

@nrc well it is a &Session, in the driver, but LoweringContext::new casts it to a &NodeIdAssigner trait object. The only places it's constructed without a real Session are in tests, so I guess that can be papered over somehow.

[durka]

Maybe just adding a NodeIdAssigner::as_session(&self) -> Option<&Session>.