Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

save-analysis fails with proc macro and nested generic type #47981

Closed
kdy1 opened this issue Feb 3, 2018 · 7 comments
Closed

save-analysis fails with proc macro and nested generic type #47981

kdy1 opened this issue Feb 3, 2018 · 7 comments
Labels
A-save-analysis Area: saving results of analyses such as inference and borrowck results to a file. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.

Comments

@kdy1
Copy link
Contributor

kdy1 commented Feb 3, 2018

rls shows it as warning, but all codes after the field are not analyzed (including sibling modules). And rls ICEs on dependent crate.

Warning from rls-vscode:

[rustc] librustc_save_analysis\span_utils.rs:156: Mis-counted brackets when breaking path? Parsing 'Box>' in ecmascript\ast\src\class.rs, line 10

Backtrace (from wsl, because my windows rustc doesn't produce any stacktrace)
$ CARGO_INCREMENTAL=0  RUST_BACKTRACE=1 rls --cli
Initializing (look for `diagnosticsEnd` message)...
> 1: InitializeResult {
    capabilities: ServerCapabilities {
        text_document_sync: Some(
            Kind(
                Incremental
            )
        ),
        hover_provider: Some(
            true
        ),
        completion_provider: Some(
            CompletionOptions {
                resolve_provider: Some(
                    true
                ),
                trigger_characters: [
                    ".",
                    ":"
                ]
            }
        ),
        signature_help_provider: None,
        definition_provider: Some(
            true
        ),
        references_provider: Some(
            true
        ),
        document_highlight_provider: Some(
            true
        ),
        document_symbol_provider: Some(
            true
        ),
        workspace_symbol_provider: Some(
            true
        ),
        code_action_provider: Some(
            true
        ),
        code_lens_provider: None,
        document_formatting_provider: Some(
            true
        ),
        document_range_formatting_provider: Some(
            false
        ),
        document_on_type_formatting_provider: None,
        rename_provider: Some(
            true
        ),
        execute_command_provider: Some(
            ExecuteCommandOptions {
                commands: [
                    "rls.applySuggestion",
                    "rls.deglobImports"
                ]
            }
        )
    }
}
{"jsonrpc":"2.0","method":"rustDocument/beginBuild"}
thread 'rustc' panicked at 'Box<Any>', librustc_errors/lib.rs:482:9
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::print
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
   2: std::panicking::default_hook::{{closure}}
             at libstd/panicking.rs:380
   3: std::panicking::default_hook
             at libstd/panicking.rs:396
   4: std::panicking::rust_panic_with_hook
             at libstd/panicking.rs:576
   5: std::panicking::begin_panic
   6: rustc_errors::Handler::span_bug
   7: <std::thread::local::LocalKey<T>>::with
   8: rustc::ty::context::tls::with_opt
   9: rustc::session::span_bug_fmt
  10: rustc_save_analysis::span_utils::SpanUtils::sub_span_for_type_name
  11: <rustc_save_analysis::dump_visitor::DumpVisitor<'l, 'tcx, 'll, O> as syntax::visit::Visitor<'l>>::visit_ty
  12: syntax::visit::walk_path_parameters
  13: <rustc_save_analysis::dump_visitor::DumpVisitor<'l, 'tcx, 'll, O> as syntax::visit::Visitor<'l>>::visit_ty
  14: <rustc_save_analysis::dump_visitor::DumpVisitor<'l, 'tcx, 'll, O> as syntax::visit::Visitor<'l>>::visit_item
  15: <rustc_save_analysis::dump_visitor::DumpVisitor<'l, 'tcx, 'll, O> as syntax::visit::Visitor<'l>>::visit_item
  16: <rustc_save_analysis::dump_visitor::DumpVisitor<'l, 'tcx, 'll, O> as syntax::visit::Visitor<'l>>::visit_mod
  17: <rustc_save_analysis::CallbackHandler<'b> as rustc_save_analysis::SaveHandler>::save
  18: rustc_save_analysis::process_crate
  19: <rls::build::rustc::RlsRustcCalls as rustc_driver::CompilerCalls<'a>>::build_controller::{{closure}}
  20: rustc::dep_graph::graph::DepGraph::with_ignore
  21: rustc_driver::driver::compile_input::{{closure}}
  22: <std::thread::local::LocalKey<T>>::with
  23: <std::thread::local::LocalKey<T>>::with
  24: rustc::ty::context::TyCtxt::create_and_enter
  25: rustc_driver::driver::compile_input
  26: rustc_driver::run_compiler

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

note: rustc 1.25.0-nightly (56733bc9f 2018-02-01) running on x86_64-unknown-linux-gnu

{"jsonrpc":"2.0","method":"rustDocument/diagnosticsBegin"}
{"jsonrpc":"2.0","method":"rustDocument/diagnosticsEnd"}
$ rustup -V && rustc -vV  && cargo -vV
rustup 1.10.0 (c6a8f7c60 2018-01-25)
rustc 1.25.0-nightly (616b66dca 2018-02-02)
binary: rustc
commit-hash: 616b66dca25a67321b1654e5a65acc6337d63cf4
commit-date: 2018-02-02
host: x86_64-pc-windows-gnu
release: 1.25.0-nightly
LLVM version: 4.0
cargo 0.26.0-nightly (1d6dfea44 2018-01-26)
release: 0.26.0
commit-hash: 1d6dfea44f97199d5d5c177c7dadcde393eaff9a
commit-date: 2018-01-26

@kdy1
Copy link
Contributor Author

kdy1 commented Feb 3, 2018

rls works well if you use Option<(Box<Expr>)> instead of Option<Box<Expr>>

@nrc nrc added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. A-save-analysis Area: saving results of analyses such as inference and borrowck results to a file. A-rls labels Feb 4, 2018
@nrc
Copy link
Member

nrc commented Feb 4, 2018

Do you have the exact source code which caused this error please?

@kdy1
Copy link
Contributor Author

kdy1 commented Feb 4, 2018

PR which changed Option<Box<Expr>> into Option<(Box<Expr>)>: swc-project/swc#30

This part is relevant

https://github.com/swc-project/swc/blob/fa40c8ddf3079d1bc624efd1d2059277bc72dcf2/ecmascript/ast/src/class.rs#L5-L11

#[ast_node] is an alias for #[derive(AstNode, Fold, Clone, Debug, PartialEq)] and inlining it does not have any effect.


Oh.. I think I found a clue..

// This is broken
#[derive(AstNode, Fold, Clone, Debug, PartialEq)]
pub struct Class {
    pub span: Span,

    pub body: Vec<ClassMethod>,
    pub super_class: Option<Box<Expr>>,
}

// This works
#[derive(Clone, Debug, PartialEq)]
pub struct Class {
    pub span: Span,

    pub body: Vec<ClassMethod>,
    pub super_class: Option<Box<Expr>>,
}

So the issue is about derive macro, and I'm sure that problematic
part is ::swc_common::Folder::<Option<Box<Expr>>>::fold(_folder, _super_class)


Reduced to

#![feature(specialization)]

#[macro_use]
extern crate ast_node;
extern crate swc_common;

#[derive(Fold)]
pub struct Expr {
    pub super_class: Option<Box<Expr>>,
}

But when I copy generated code to other file, it works. (and warns about unused macro import)

cargo expand:

pub struct Expr {
    pub super_class: Option<Box<Expr>>,
}
#[allow(dead_code, non_upper_case_globals)]
const DERIVE_FOLD_FOR_Expr: () =
    {
        extern crate swc_common;
        impl <__Folder> swc_common::FoldWith<__Folder> for Expr {
            fn fold_children(self, __folder: &mut __Folder) -> Self {
                match self {
                    Expr { super_class: _super_class } => {
                        return Expr{super_class:
                                        swc_common::Folder::<Option<Box<Expr>>>::fold(__folder,
                                                                                      _super_class),};
                    }
                }
            }
        }
    };

So I guess it is related to span generated by proc macro..

https://github.com/swc-project/swc/blob/ca663aa33d7e313daf8e9ef138da052738a73ed2/macros/ast_node/src/fold.rs#L59-L65

In this case,

Span of ::swc_common::Folder::<$>::fold(_folder, $) is call_site (= #[derive(Fold)]).

FieldType is Option<Box<Expr>>, and has same span with field's type.

@kdy1
Copy link
Contributor Author

kdy1 commented Feb 4, 2018

I think title should be updated, but I'm not sure if ufcs for generic trait is necessary.

@nrc nrc changed the title save-analysis chokes with nested generic type like Option<Box<Expr>> save-analysis ICEs with proc macro and nested generic type Feb 15, 2018
@nrc nrc changed the title save-analysis ICEs with proc macro and nested generic type save-analysis fails with proc macro and nested generic type Feb 15, 2018
@nrc
Copy link
Member

nrc commented Feb 15, 2018

I've been trying to repro, but it is kind of involved to setup the exact conditions to cause the crash. I currently can't do it outside of the entire project.

@nrc
Copy link
Member

nrc commented Feb 15, 2018

I can repro, and I can see the assertion which is causing the problem and verify that there is a span problem. But there are a lot of places that the error could be introduce - could be in the proc-macro bits of the compiler or in the libs or (quite likely) in Syn. I think rather than trying to track down the error it is better to just make save-analysis more robust.

@nrc
Copy link
Member

nrc commented Feb 15, 2018

Or the parser too, although that seems less likely given that this doesn't repro without the proc macro.

nrc added a commit to nrc/rust that referenced this issue Feb 16, 2018
Closes rust-lang#47981

This is pretty unsatisfying since it is working around a span bug. However, I can't track down the span bug and it could be in the parser, proc macro expansion, the user macro, or Syn (or any other library that can manipulate spans). Given that user code can cause this error, I think we need to be more robust here.
Manishearth added a commit to Manishearth/rust that referenced this issue Feb 19, 2018
save-analysis: power through bracket mis-counts

Closes rust-lang#47981

This is pretty unsatisfying since it is working around a span bug. However, I can't track down the span bug and it could be in the parser, proc macro expansion, the user macro, or Syn (or any other library that can manipulate spans). Given that user code can cause this error, I think we need to be more robust here.

r? @eddyb
Manishearth added a commit to Manishearth/rust that referenced this issue Feb 24, 2018
save-analysis: power through bracket mis-counts

Closes rust-lang#47981

This is pretty unsatisfying since it is working around a span bug. However, I can't track down the span bug and it could be in the parser, proc macro expansion, the user macro, or Syn (or any other library that can manipulate spans). Given that user code can cause this error, I think we need to be more robust here.

r? @eddyb
Manishearth added a commit to Manishearth/rust that referenced this issue Feb 24, 2018
save-analysis: power through bracket mis-counts

Closes rust-lang#47981

This is pretty unsatisfying since it is working around a span bug. However, I can't track down the span bug and it could be in the parser, proc macro expansion, the user macro, or Syn (or any other library that can manipulate spans). Given that user code can cause this error, I think we need to be more robust here.

r? @eddyb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-save-analysis Area: saving results of analyses such as inference and borrowck results to a file. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants