/rust

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

Improve OwnedSlice and use it in HIR #30095

Closed
wants to merge 1 commit into
base: master
from

Conversation

Reviewers
No reviews
Assignees

@nrc nrc

Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@petrochenkov @eddyb @Gankro @bors @nrc
@petrochenkov
Contributor

petrochenkov commented Nov 28, 2015

Most of the changes are to support switching between vector-like containers in HIR and beefing up OwnedSlice's interface.
The actual change from Vec to OwnedSlice is one line, and it can be changed back if necessary.

Memory consumption for librustc, libcore and libstd is below under the spoilers - the changes are small, but consistent.
libcore is the most noticeable, 5.7% less memory immediately after lowering to HIR, 4% less memory during type checking.

Before:

rustc: x86_64-pc-windows-gnu/stage1/lib/rustlib/x86_64-pc-windows-gnu/lib/librustc
time: 0.149; rss: 61MB  parsing
time: 0.050; rss: 61MB  configuration 1
time: 0.000; rss: 61MB  recursion limit
time: 0.004; rss: 61MB  gated macro checking
time: 0.000; rss: 61MB  crate injection
time: 0.019; rss: 64MB  macro loading
time: 0.000; rss: 64MB  plugin loading
time: 0.000; rss: 64MB  plugin registration
time: 0.655; rss: 99MB  expansion
time: 0.026; rss: 99MB  complete gated feature checking 1
time: 0.111; rss: 99MB  configuration 2
time: 0.000; rss: 99MB  gated configuration checking
time: 0.059; rss: 100MB maybe building test harness
time: 0.054; rss: 100MB prelude injection
time: 0.013; rss: 100MB checking that all macro invocations are gone
time: 0.000; rss: 100MB checking for inline asm in case the target doesn't support it
time: 0.027; rss: 100MB complete gated feature checking 2
time: 0.058; rss: 100MB assigning node ids
time: 0.094; rss: 151MB lowering ast -> hir
time: 0.048; rss: 167MB indexing hir
time: 0.000; rss: 167MB attribute checking
time: 0.035; rss: 167MB early lint checks
time: 0.011; rss: 183MB external crate/lib resolution
time: 0.010; rss: 183MB language item collection
time: 0.219; rss: 233MB resolution
time: 0.019; rss: 233MB lifetime resolution
time: 0.000; rss: 233MB looking for entry point
time: 0.000; rss: 233MB looking for plugin registrar
time: 0.071; rss: 252MB region resolution
time: 0.010; rss: 252MB loop checking
time: 0.012; rss: 252MB static item recursion checking
time: 0.069; rss: 261MB type collecting
time: 0.002; rss: 261MB variance inference
time: 0.059; rss: 283MB coherence checking
time: 0.103; rss: 288MB wf checking (old)
time: 0.166; rss: 290MB item-types checking
time: 5.307; rss: 402MB item-bodies checking
time: 0.000; rss: 402MB drop-impl checking
time: 0.270; rss: 402MB wf checking (new)
time: 0.586; rss: 419MB const checking
time: 0.117; rss: 420MB privacy checking
time: 0.018; rss: 420MB stability index
time: 0.034; rss: 420MB intrinsic checking
time: 0.023; rss: 420MB effect checking
time: 0.093; rss: 420MB match checking
time: 0.430; rss: 486MB MIR dump
time: 0.068; rss: 487MB liveness checking
time: 0.613; rss: 488MB borrow checking
time: 0.327; rss: 489MB rvalue checking
time: 0.024; rss: 489MB reachability checking
time: 0.076; rss: 490MB death checking
time: 0.066; rss: 492MB stability checking
time: 0.000; rss: 492MB unused lib feature checking
time: 0.281; rss: 492MB lint checking
time: 0.001; rss: 492MB resolving dependency formats
time: 0.070; rss: 497MB erasing regions from MIR
time: 7.883; rss: 1028MB        translation
  time: 1.817; rss: 549MB       llvm function passes [0]
  time: 32.515; rss: 667MB      llvm module passes [0]
  time: 10.598; rss: 685MB      codegen passes [0]
  time: 0.004; rss: 166MB       codegen passes [0]
time: 45.969; rss: 161MB        LLVM passes
  time: 0.612; rss: 161MB       running linker
time: 1.105; rss: 164MB linking
rustc: x86_64-pc-windows-gnu/stage2/lib/rustlib/x86_64-pc-windows-gnu/lib/libcore
time: 0.089; rss: 41MB  parsing
time: 0.024; rss: 43MB  configuration 1
time: 0.000; rss: 43MB  recursion limit
time: 0.002; rss: 43MB  gated macro checking
time: 0.000; rss: 43MB  crate injection
time: 0.001; rss: 43MB  macro loading
time: 0.000; rss: 43MB  plugin loading
time: 0.000; rss: 43MB  plugin registration
time: 0.428; rss: 71MB  expansion
time: 0.012; rss: 71MB  complete gated feature checking 1
time: 0.090; rss: 71MB  configuration 2
time: 0.000; rss: 71MB  gated configuration checking
time: 0.047; rss: 71MB  maybe building test harness
time: 0.000; rss: 71MB  prelude injection
time: 0.005; rss: 71MB  checking that all macro invocations are gone
time: 0.000; rss: 71MB  checking for inline asm in case the target doesn't support it
time: 0.012; rss: 71MB  complete gated feature checking 2
time: 0.046; rss: 71MB  assigning node ids
time: 0.039; rss: 105MB lowering ast -> hir
time: 0.085; rss: 122MB indexing hir
time: 0.000; rss: 122MB attribute checking
time: 0.019; rss: 122MB early lint checks
time: 0.000; rss: 122MB external crate/lib resolution
time: 0.004; rss: 122MB language item collection
time: 0.076; rss: 139MB resolution
time: 0.011; rss: 139MB lifetime resolution
time: 0.000; rss: 139MB looking for entry point
time: 0.000; rss: 139MB looking for plugin registrar
time: 0.028; rss: 146MB region resolution
time: 0.003; rss: 146MB loop checking
time: 0.004; rss: 146MB static item recursion checking
time: 0.070; rss: 156MB type collecting
time: 0.001; rss: 156MB variance inference
time: 0.311; rss: 157MB coherence checking
time: 0.380; rss: 160MB wf checking (old)
time: 0.514; rss: 165MB item-types checking
time: 3.256; rss: 195MB item-bodies checking
time: 0.000; rss: 195MB drop-impl checking
time: 0.685; rss: 196MB wf checking (new)
time: 0.144; rss: 197MB const checking
time: 0.057; rss: 197MB privacy checking
time: 0.009; rss: 197MB stability index
time: 0.033; rss: 197MB intrinsic checking
time: 0.011; rss: 197MB effect checking
time: 0.052; rss: 197MB match checking
time: 0.178; rss: 218MB MIR dump
time: 0.024; rss: 219MB liveness checking
time: 0.285; rss: 219MB borrow checking
time: 0.146; rss: 219MB rvalue checking
time: 0.073; rss: 220MB reachability checking
time: 0.035; rss: 220MB death checking
time: 0.032; rss: 221MB stability checking
time: 0.000; rss: 221MB unused lib feature checking
time: 0.434; rss: 221MB lint checking
time: 0.000; rss: 221MB resolving dependency formats
time: 0.022; rss: 222MB erasing regions from MIR
time: 1.182; rss: 292MB translation
  time: 0.157; rss: 171MB       llvm function passes [0]
  time: 1.464; rss: 180MB       llvm module passes [0]
  time: 1.217; rss: 183MB       codegen passes [0]
  time: 0.004; rss: 148MB       codegen passes [0]
time: 2.897; rss: 143MB LLVM passes
time: 0.051; rss: 143MB linking
rustc: x86_64-pc-windows-gnu/stage2/lib/rustlib/x86_64-pc-windows-gnu/lib/libstd
time: 0.143; rss: 60MB  parsing
time: 0.041; rss: 62MB  configuration 1
time: 0.000; rss: 62MB  recursion limit
time: 0.002; rss: 62MB  gated macro checking
time: 0.000; rss: 62MB  crate injection
time: 0.006; rss: 63MB  macro loading
time: 0.000; rss: 63MB  plugin loading
time: 0.000; rss: 63MB  plugin registration
time: 0.165; rss: 64MB  expansion
time: 0.007; rss: 64MB  complete gated feature checking 1
time: 0.039; rss: 64MB  configuration 2
time: 0.000; rss: 64MB  gated configuration checking
time: 0.020; rss: 64MB  maybe building test harness
time: 0.019; rss: 64MB  prelude injection
time: 0.003; rss: 64MB  checking that all macro invocations are gone
time: 0.000; rss: 64MB  checking for inline asm in case the target doesn't support it
time: 0.007; rss: 64MB  complete gated feature checking 2
time: 0.020; rss: 64MB  assigning node ids
time: 0.021; rss: 72MB  lowering ast -> hir
time: 0.012; rss: 80MB  indexing hir
time: 0.000; rss: 80MB  attribute checking
time: 0.010; rss: 80MB  early lint checks
time: 0.003; rss: 80MB  external crate/lib resolution
time: 0.002; rss: 80MB  language item collection
time: 0.057; rss: 106MB resolution
time: 0.004; rss: 106MB lifetime resolution
time: 0.000; rss: 106MB looking for entry point
time: 0.000; rss: 106MB looking for plugin registrar
time: 0.014; rss: 111MB region resolution
time: 0.002; rss: 111MB loop checking
time: 0.002; rss: 111MB static item recursion checking
time: 0.025; rss: 112MB type collecting
time: 0.001; rss: 112MB variance inference
time: 0.035; rss: 122MB coherence checking
time: 0.043; rss: 123MB wf checking (old)
time: 0.065; rss: 126MB item-types checking
time: 1.068; rss: 153MB item-bodies checking
time: 0.000; rss: 153MB drop-impl checking
time: 0.111; rss: 153MB wf checking (new)
time: 0.109; rss: 154MB const checking
time: 0.026; rss: 155MB privacy checking
time: 0.004; rss: 155MB stability index
time: 0.010; rss: 155MB intrinsic checking
time: 0.005; rss: 155MB effect checking
time: 0.026; rss: 155MB match checking
time: 0.117; rss: 171MB MIR dump
time: 0.014; rss: 172MB liveness checking
time: 0.185; rss: 172MB borrow checking
time: 0.127; rss: 172MB rvalue checking
time: 0.009; rss: 172MB reachability checking
time: 0.016; rss: 172MB death checking
time: 0.016; rss: 173MB stability checking
time: 0.000; rss: 173MB unused lib feature checking
time: 0.096; rss: 173MB lint checking
time: 0.000; rss: 173MB resolving dependency formats
time: 0.015; rss: 175MB erasing regions from MIR
time: 0.987; rss: 267MB translation
  time: 0.202; rss: 213MB       llvm function passes [0]
  time: 3.011; rss: 228MB       llvm module passes [0]
  time: 1.258; rss: 233MB       codegen passes [0]
  time: 0.004; rss: 178MB       codegen passes [0]
time: 4.568; rss: 175MB LLVM passes
  time: 0.003; rss: 175MB       altering collections-71b07a99.rlib
  time: 0.001; rss: 175MB       altering alloc-71b07a99.rlib
  time: 0.009; rss: 175MB       altering rustc_unicode-71b07a99.rlib
  time: 0.003; rss: 175MB       altering alloc_jemalloc-71b07a99.rlib
  time: 0.001; rss: 175MB       altering libc-71b07a99.rlib
  time: 0.003; rss: 175MB       altering rand-71b07a99.rlib
  time: 0.004; rss: 175MB       altering core-71b07a99.rlib
  time: 0.249; rss: 176MB       running linker
time: 0.343; rss: 176MB linking

After:

rustc: x86_64-pc-windows-gnu/stage1/lib/rustlib/x86_64-pc-windows-gnu/lib/librustc
time: 0.148; rss: 61MB  parsing
time: 0.050; rss: 61MB  configuration 1
time: 0.000; rss: 61MB  recursion limit
time: 0.005; rss: 62MB  gated macro checking
time: 0.000; rss: 62MB  crate injection
time: 0.018; rss: 64MB  macro loading
time: 0.000; rss: 64MB  plugin loading
time: 0.000; rss: 64MB  plugin registration
time: 0.668; rss: 100MB expansion
time: 0.026; rss: 100MB complete gated feature checking 1
time: 0.112; rss: 100MB configuration 2
time: 0.000; rss: 100MB gated configuration checking
time: 0.059; rss: 100MB maybe building test harness
time: 0.054; rss: 100MB prelude injection
time: 0.013; rss: 100MB checking that all macro invocations are gone
time: 0.000; rss: 100MB checking for inline asm in case the target doesn't support it
time: 0.028; rss: 100MB complete gated feature checking 2
time: 0.057; rss: 100MB assigning node ids
time: 0.080; rss: 148MB lowering ast -> hir
time: 0.045; rss: 164MB indexing hir
time: 0.000; rss: 164MB attribute checking
time: 0.035; rss: 164MB early lint checks
time: 0.011; rss: 180MB external crate/lib resolution
time: 0.010; rss: 180MB language item collection
time: 0.211; rss: 231MB resolution
time: 0.017; rss: 231MB lifetime resolution
time: 0.000; rss: 231MB looking for entry point
time: 0.000; rss: 231MB looking for plugin registrar
time: 0.066; rss: 249MB region resolution
time: 0.008; rss: 249MB loop checking
time: 0.009; rss: 249MB static item recursion checking
time: 0.066; rss: 257MB type collecting
time: 0.002; rss: 257MB variance inference
time: 0.058; rss: 281MB coherence checking
time: 0.100; rss: 286MB wf checking (old)
time: 0.164; rss: 288MB item-types checking
time: 5.349; rss: 399MB item-bodies checking
time: 0.000; rss: 399MB drop-impl checking
time: 0.274; rss: 400MB wf checking (new)
time: 0.387; rss: 417MB const checking
time: 0.115; rss: 417MB privacy checking
time: 0.016; rss: 417MB stability index
time: 0.036; rss: 417MB intrinsic checking
time: 0.026; rss: 417MB effect checking
time: 0.093; rss: 417MB match checking
time: 0.430; rss: 483MB MIR dump
time: 0.065; rss: 484MB liveness checking
time: 0.619; rss: 485MB borrow checking
time: 0.341; rss: 486MB rvalue checking
time: 0.023; rss: 486MB reachability checking
time: 0.075; rss: 487MB death checking
time: 0.068; rss: 489MB stability checking
time: 0.000; rss: 489MB unused lib feature checking
time: 0.282; rss: 489MB lint checking
time: 0.002; rss: 489MB resolving dependency formats
time: 0.071; rss: 493MB erasing regions from MIR
time: 7.926; rss: 1014MB        translation
  time: 1.861; rss: 549MB       llvm function passes [0]
  time: 32.975; rss: 665MB      llvm module passes [0]
  time: 10.806; rss: 667MB      codegen passes [0]
  time: 0.004; rss: 166MB       codegen passes [0]
time: 46.722; rss: 160MB        LLVM passes
  time: 0.626; rss: 160MB       running linker
time: 1.142; rss: 163MB linking
rustc: x86_64-pc-windows-gnu/stage2/lib/rustlib/x86_64-pc-windows-gnu/lib/libcore
time: 0.072; rss: 42MB  parsing
time: 0.026; rss: 43MB  configuration 1
time: 0.000; rss: 43MB  recursion limit
time: 0.002; rss: 43MB  gated macro checking
time: 0.000; rss: 43MB  crate injection
time: 0.001; rss: 43MB  macro loading
time: 0.000; rss: 43MB  plugin loading
time: 0.000; rss: 43MB  plugin registration
time: 0.435; rss: 71MB  expansion
time: 0.010; rss: 71MB  complete gated feature checking 1
time: 0.093; rss: 71MB  configuration 2
time: 0.000; rss: 71MB  gated configuration checking
time: 0.045; rss: 71MB  maybe building test harness
time: 0.000; rss: 71MB  prelude injection
time: 0.005; rss: 71MB  checking that all macro invocations are gone
time: 0.000; rss: 71MB  checking for inline asm in case the target doesn't support it
time: 0.013; rss: 71MB  complete gated feature checking 2
time: 0.046; rss: 71MB  assigning node ids
time: 0.036; rss: 99MB  lowering ast -> hir
time: 0.084; rss: 116MB indexing hir
time: 0.000; rss: 116MB attribute checking
time: 0.018; rss: 116MB early lint checks
time: 0.000; rss: 116MB external crate/lib resolution
time: 0.003; rss: 116MB language item collection
time: 0.076; rss: 131MB resolution
time: 0.010; rss: 131MB lifetime resolution
time: 0.000; rss: 131MB looking for entry point
time: 0.000; rss: 131MB looking for plugin registrar
time: 0.029; rss: 141MB region resolution
time: 0.003; rss: 141MB loop checking
time: 0.003; rss: 141MB static item recursion checking
time: 0.068; rss: 149MB type collecting
time: 0.001; rss: 150MB variance inference
time: 0.313; rss: 151MB coherence checking
time: 0.403; rss: 154MB wf checking (old)
time: 0.543; rss: 158MB item-types checking
time: 3.340; rss: 187MB item-bodies checking
time: 0.000; rss: 187MB drop-impl checking
time: 0.689; rss: 188MB wf checking (new)
time: 0.144; rss: 189MB const checking
time: 0.055; rss: 189MB privacy checking
time: 0.008; rss: 189MB stability index
time: 0.032; rss: 189MB intrinsic checking
time: 0.010; rss: 189MB effect checking
time: 0.051; rss: 189MB match checking
time: 0.178; rss: 212MB MIR dump
time: 0.024; rss: 213MB liveness checking
time: 0.284; rss: 213MB borrow checking
time: 0.147; rss: 213MB rvalue checking
time: 0.073; rss: 214MB reachability checking
time: 0.033; rss: 214MB death checking
time: 0.032; rss: 215MB stability checking
time: 0.000; rss: 215MB unused lib feature checking
time: 0.436; rss: 215MB lint checking
time: 0.000; rss: 215MB resolving dependency formats
time: 0.023; rss: 217MB erasing regions from MIR
time: 1.166; rss: 288MB translation
  time: 0.148; rss: 176MB       llvm function passes [0]
  time: 1.406; rss: 185MB       llvm module passes [0]
  time: 1.199; rss: 187MB       codegen passes [0]
  time: 0.005; rss: 151MB       codegen passes [0]
time: 2.814; rss: 147MB LLVM passes
time: 0.048; rss: 147MB linking
rustc: x86_64-pc-windows-gnu/stage2/lib/rustlib/x86_64-pc-windows-gnu/lib/libstd
time: 0.152; rss: 60MB  parsing
time: 0.043; rss: 62MB  configuration 1
time: 0.000; rss: 62MB  recursion limit
time: 0.002; rss: 62MB  gated macro checking
time: 0.000; rss: 62MB  crate injection
time: 0.006; rss: 63MB  macro loading
time: 0.000; rss: 63MB  plugin loading
time: 0.000; rss: 63MB  plugin registration
time: 0.179; rss: 64MB  expansion
time: 0.007; rss: 64MB  complete gated feature checking 1
time: 0.039; rss: 64MB  configuration 2
time: 0.000; rss: 64MB  gated configuration checking
time: 0.020; rss: 64MB  maybe building test harness
time: 0.019; rss: 64MB  prelude injection
time: 0.003; rss: 64MB  checking that all macro invocations are gone
time: 0.000; rss: 64MB  checking for inline asm in case the target doesn't support it
time: 0.008; rss: 64MB  complete gated feature checking 2
time: 0.020; rss: 64MB  assigning node ids
time: 0.024; rss: 69MB  lowering ast -> hir
time: 0.012; rss: 76MB  indexing hir
time: 0.000; rss: 76MB  attribute checking
time: 0.012; rss: 76MB  early lint checks
time: 0.003; rss: 77MB  external crate/lib resolution
time: 0.002; rss: 77MB  language item collection
time: 0.057; rss: 103MB resolution
time: 0.004; rss: 103MB lifetime resolution
time: 0.000; rss: 103MB looking for entry point
time: 0.000; rss: 103MB looking for plugin registrar
time: 0.013; rss: 107MB region resolution
time: 0.002; rss: 107MB loop checking
time: 0.002; rss: 107MB static item recursion checking
time: 0.024; rss: 108MB type collecting
time: 0.001; rss: 108MB variance inference
time: 0.033; rss: 119MB coherence checking
time: 0.044; rss: 120MB wf checking (old)
time: 0.065; rss: 122MB item-types checking
time: 1.113; rss: 151MB item-bodies checking
time: 0.000; rss: 151MB drop-impl checking
time: 0.120; rss: 151MB wf checking (new)
time: 0.106; rss: 153MB const checking
time: 0.023; rss: 153MB privacy checking
time: 0.003; rss: 153MB stability index
time: 0.010; rss: 153MB intrinsic checking
time: 0.005; rss: 153MB effect checking
time: 0.025; rss: 153MB match checking
time: 0.122; rss: 169MB MIR dump
time: 0.013; rss: 170MB liveness checking
time: 0.211; rss: 170MB borrow checking
time: 0.127; rss: 170MB rvalue checking
time: 0.007; rss: 170MB reachability checking
time: 0.017; rss: 170MB death checking
time: 0.016; rss: 171MB stability checking
time: 0.000; rss: 171MB unused lib feature checking
time: 0.087; rss: 171MB lint checking
time: 0.000; rss: 171MB resolving dependency formats
time: 0.017; rss: 172MB erasing regions from MIR
time: 1.039; rss: 264MB translation
  time: 0.212; rss: 214MB       llvm function passes [0]
  time: 3.120; rss: 227MB       llvm module passes [0]
  time: 1.344; rss: 233MB       codegen passes [0]
  time: 0.003; rss: 178MB       codegen passes [0]
time: 4.772; rss: 175MB LLVM passes
  time: 0.003; rss: 175MB       altering collections-71b07a99.rlib
  time: 0.001; rss: 175MB       altering alloc-71b07a99.rlib
  time: 0.006; rss: 175MB       altering rustc_unicode-71b07a99.rlib
  time: 0.003; rss: 175MB       altering alloc_jemalloc-71b07a99.rlib
  time: 0.001; rss: 175MB       altering libc-71b07a99.rlib
  time: 0.002; rss: 175MB       altering rand-71b07a99.rlib
  time: 0.003; rss: 175MB       altering core-71b07a99.rlib
  time: 0.260; rss: 175MB       running linker
time: 0.346; rss: 175MB linking

r? @nrc or @eddyb

@rust-highfive rust-highfive assigned nrc Nov 28, 2015

@eddyb

This comment has been minimized.

Sign in to view
Member

eddyb commented Nov 28, 2015

Why aren't we using Box<[T]> here? I would even suggest P<[T]> if that makes things simpler in any way.
@petrochenkov

This comment has been minimized.

Sign in to view
Contributor

petrochenkov commented Nov 28, 2015

Box<[T]> can't be collected into, for example. If I implement FromIterator for Box<[T]>, then 1) I'll have to do it without help of Vec 2) It's an insta-stable addition to the standard library.

OwnedSlice can probably be merged into P, but I haven't tried to do this yet. It looks like their functionality would be similar, but implementations would still be mostly different for T and [T] cases even if they are merged (almost like with Box<T> and Box<[T]>).
@petrochenkov

This comment has been minimized.

Sign in to view
Contributor

petrochenkov commented Nov 28, 2015

Yep, OwnedSlice can be merged into P, nothing unexpected encountered.
I'll update the PR tomorrow.
@petrochenkov

This comment has been minimized.

Sign in to view
Contributor

petrochenkov commented Nov 29, 2015

Updated.
@Gankro

This comment has been minimized.

Sign in to view
Contributor

Gankro commented Nov 29, 2015

For reference, the RawVec type enables Box<[T]> to impl much Vecish code. For instance Clone.
@bors

This comment has been minimized.

Sign in to view
Contributor

bors commented Dec 4, 2015

☔️ The latest upstream changes (presumably #29850) made this pull request unmergeable. Please resolve the merge conflicts.

@petrochenkov petrochenkov force-pushed the petrochenkov:owned branch from ee95763 to 1aa472d Dec 5, 2015

@petrochenkov

This comment has been minimized.

Sign in to view
Contributor

petrochenkov commented Dec 5, 2015

Rebased.
@bors

This comment has been minimized.

Sign in to view
Contributor

bors commented Dec 6, 2015

☔️ The latest upstream changes (presumably #30241) made this pull request unmergeable. Please resolve the merge conflicts.

@petrochenkov petrochenkov force-pushed the petrochenkov:owned branch from 1aa472d to 98ce21f Dec 6, 2015

@petrochenkov

This comment has been minimized.

Sign in to view
Contributor

petrochenkov commented Dec 6, 2015

Rebased.
@nrc

This comment has been minimized.

Sign in to view
Member

nrc commented Dec 6, 2015

Still failing a test and looks genuine
@nrc

This comment has been minimized.

Sign in to view
Member

nrc commented Dec 6, 2015

I think the end result would be less confusing if you don't use the name Vec for the HIR version - OwnedSlice or P<[T]> would be better - Vec is standard enough that when people see it they will assume the standard library version.
@nrc

This comment has been minimized.

Sign in to view
Member

nrc commented Dec 7, 2015

r+ with the above two comments addressed
@petrochenkov

This comment has been minimized.

Sign in to view
Contributor

petrochenkov commented Dec 7, 2015

I think the end result would be less confusing if you don't use the name Vec for the HIR version - OwnedSlice or P<[T]> would be better - Vec is standard enough that when people see it they will assume the standard library version.

The idea was to 1) Keep the code changes small (so, the name Vec is kept) 2) Do not commit to a concrete storage type (so, a HIR specific type alias is introduced)
If some extra code churn is ok, then I'll mass-rename hir::Vec to HirVec.

Still failing a test and looks genuine

test astencode::test_basic ... �[32mok�(B�[0m
test astencode::test_more ... �[32mok�(B�[0m
Segmentation fault (core dumped)
make: *** [tmp/check-stage2-T-x86_64-unknown-linux-gnu-H-x86_64-unknown-linux-gnu-rustc_metadata.ok] Error 139

Hmm, it certainly doesn't segfault on my machine, I'll try to rerun on something else.
(The error: macro undefined: 'z!' thing visible in the partial Travis log is expected output from a #[should_panic] test)
@nrc

This comment has been minimized.

Sign in to view
Member

nrc commented Dec 7, 2015

If some extra code churn is ok, then I'll mass-rename hir::Vec to HirVec.

there is already code churn from Vec to hir::Vec or vec::Vec, a little more is OK I think, if it spares us having vec::Vec, etc.
@petrochenkov

This comment has been minimized.

Sign in to view
Contributor

petrochenkov commented Dec 7, 2015

I've renamed hir::Vec to HirVec, but the segfault is legitimate, I've reproduced it on 32-bit Linux.
Something is wrong with code generation for DST arrays in Box.
I'll try to come up with a minimized example.
@petrochenkov

This comment has been minimized.

Sign in to view
Contributor

petrochenkov commented Dec 8, 2015

Some investigation results:
The segfault doesn't occur in Debug mode, but, fortunately, it occur with enabled debuginfo.
The segfault happens only in stage2 (in libsyntax) when stage2 compiler tries to compile stage2 libcore. This is strange, because this PR doesn't modify any translation code.
The segfault doesn't reproduce on 64-bit Windows, but reproduce on 32-bit Linux. (I haven't tried other setups.)
The segfault is transient - it often goes away when debug printfs are insterted near segfaulting location.
The segfault happens because P<[T]> can contain [0x1d1d1d1d, 0x1d1d1d1d] or other trash bytes after unwrapping from Result<P<[T]>, Error> instead of expected [ptr, len]. 0x1d1d1d1d looks like POST_DROP_U32.
P<[T]> still can contain [ptr, len] if printf is inserted near it, but not always.
I wasn't able to construct a minimal example, you have to build libcore with stage2 compiler to reproduce it.

First of all, I don't know if this PR introduces this codegen bug, or just uncovers some existing one.
@eddyb Maybe you have some thoughts how to track it further?
@bors

This comment has been minimized.

Sign in to view
Contributor

bors commented Dec 8, 2015

☔️ The latest upstream changes (presumably #30087) made this pull request unmergeable. Please resolve the merge conflicts.
@nrc

This comment has been minimized.

Sign in to view
Member

nrc commented Dec 8, 2015

This sounds like a great case for RR

@petrochenkov petrochenkov force-pushed the petrochenkov:owned branch from 9514ce3 to 3068d80 Dec 8, 2015

@bors

This comment has been minimized.

Sign in to view
Contributor

bors commented Dec 9, 2015

☔️ The latest upstream changes (presumably #30145) made this pull request unmergeable. Please resolve the merge conflicts.

@petrochenkov petrochenkov force-pushed the petrochenkov:owned branch from 3068d80 to 0dcb5cc Dec 15, 2015

@bors

This comment has been minimized.

Sign in to view
Contributor

bors commented Dec 16, 2015

☔️ The latest upstream changes (presumably #30206) made this pull request unmergeable. Please resolve the merge conflicts.

@petrochenkov petrochenkov force-pushed the petrochenkov:owned branch from 0dcb5cc to e46db1d Dec 16, 2015

@petrochenkov petrochenkov referenced this pull request Dec 16, 2015

Merged

libsyntax: Merge OwnedSlice into ptr::P #30420

@bors

This comment has been minimized.

Sign in to view
Contributor

bors commented Dec 17, 2015

☔️ The latest upstream changes (presumably #30341) made this pull request unmergeable. Please resolve the merge conflicts.

Manishearth added a commit to Manishearth/rust that referenced this pull request Dec 17, 2015

@Manishearth
Rollup merge of rust-lang#30420 - petrochenkov:owned2, r=nrc 
Part of rust-lang#30095 not causing mysterious segfaults.

r? @nrc
c629b33

Manishearth added a commit to Manishearth/rust that referenced this pull request Dec 18, 2015

@Manishearth
Rollup merge of rust-lang#30420 - petrochenkov:owned2, r=nrc 
Part of rust-lang#30095 not causing mysterious segfaults.

r? @nrc
5bad0e8

Manishearth added a commit to Manishearth/rust that referenced this pull request Dec 18, 2015

@Manishearth
Rollup merge of rust-lang#30420 - petrochenkov:owned2, r=nrc 
Part of rust-lang#30095 not causing mysterious segfaults.

r? @nrc
597c3d9

Manishearth added a commit to Manishearth/rust that referenced this pull request Dec 18, 2015

@Manishearth
Rollup merge of rust-lang#30420 - petrochenkov:owned2, r=nrc 
Part of rust-lang#30095 not causing mysterious segfaults.

r? @nrc
25ad961

Manishearth added a commit to Manishearth/rust that referenced this pull request Dec 18, 2015

@Manishearth
Rollup merge of rust-lang#30420 - petrochenkov:owned2, r=nrc 
Part of rust-lang#30095 not causing mysterious segfaults.

r? @nrc
9e953df

@petrochenkov petrochenkov force-pushed the petrochenkov:owned branch from e46db1d to 2730981 Dec 18, 2015

This was referenced Dec 19, 2015

Merged
Closed

@petrochenkov petrochenkov closed this Dec 19, 2015

bors added a commit that referenced this pull request Dec 20, 2015

@bors
Auto merge of #30470 - petrochenkov:owned5, r=nrc 
cc #30095

r? @nrc
3d15039

@petrochenkov petrochenkov deleted the petrochenkov:owned branch Sep 21, 2016

critiqjo pushed a commit to critiqjo/rustdoc that referenced this pull request Dec 16, 2016

@Manishearth
Rollup merge of #30420 - petrochenkov:owned2, r=nrc 
Part of rust-lang/rust#30095 not causing mysterious segfaults.

r? @nrc
1a0b05f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment