Document use of mem::transmute

[jyn514]

This removes the mem::transmute in max_level in favor of an explicit match.

Previously, the LevelFilter had #[repr(usize)]. set_max_level would call level as usize and max_level would use transmute::<usize, LevelFilter>. This does not seem to be defined behavior - although repr(usize) guarantees that LevelFilter will be represented as a usize, it does not guarantee that every usize is a valid LevelFilter.

Furthermore, although in practice as usize will return 0, 1, ... for the enum variants, this is not guaranteed by the compiler, even though other parts of the code depend on this.

This explicitly specifies the discriminant values for LevelFilter and changes max_level to use LevelFilter::from_usize instead of transmute. Additionally, this reduces the scope of a few unsafe blocks (I can remove this if you like).

cc @Shnatsel

[sfackler]

This does not seem to be defined behavior - although repr(usize) guarantees that LevelFilter will be represented as a usize, it does not guarantee that every usize is a valid LevelFilter.

My understanding is that those conversions are defined as long as the usize is a valid discriminant. If that's not the case, then isn't every use of a #[repr(C)] enum in FFI context undefined behavior?

Furthermore, although in practice as usize will return 0, 1, ... for the enum variants, this is not guaranteed by the compiler,

It is in fact guaranteed by the compiler:

If the first variant in the declaration is unspecified, then it is set to zero. For every other unspecified discriminant, it is set to one higher than the previous variant in the declaration.

https://doc.rust-lang.org/reference/items/enumerations.html#custom-discriminant-values-for-field-less-enumerations

This explicitly specifies the discriminant values for LevelFilter and changes max_level to use LevelFilter::from_usize instead of transmute.

This adds a function call in the logging hot path and and increases its size by adding an unwinding codepath:

Before:

00000000000042c0 <_ZN3foo4main17hb023c46eff9ccee7E>:
    42c0:       48 8d 05 89 cd 02 00    lea    0x2cd89(%rip),%rax        # 31050 <_ZN3log20MAX_LOG_LEVEL_FILTER17hc37ee47e3c945211E>
    42c7:       48 8b 00                mov    (%rax),%rax
    42ca:       48 83 f8 02             cmp    $0x2,%rax
    42ce:       72 1e                   jb     42ee <_ZN3foo4main17hb023c46eff9ccee7E+0x2e>
    42d0:       48 8d 3d 29 fd 01 00    lea    0x1fd29(%rip),%rdi        # 24000 <_fini+0xb24>
    42d7:       48 8d 0d c2 b2 02 00    lea    0x2b2c2(%rip),%rcx        # 2f5a0 <__dso_handle+0x8>
    42de:       be 0b 00 00 00          mov    $0xb,%esi
    42e3:       ba 02 00 00 00          mov    $0x2,%edx
    42e8:       ff 25 b2 c8 02 00       jmpq   *0x2c8b2(%rip)        # 30ba0 <_GLOBAL_OFFSET_TABLE_+0x208>
    42ee:       c3                      retq   
    42ef:       90                      nop

after:

00000000000042c0 <_ZN3foo4main17h9be0dd2b2eb4c81eE>:
    42c0:       50                      push   %rax
    42c1:       48 8d 05 88 cd 02 00    lea    0x2cd88(%rip),%rax        # 31050 <_ZN3log20MAX_LOG_LEVEL_FILTER17h11eb5420c64f0e85E>
    42c8:       48 8b 38                mov    (%rax),%rdi
    42cb:       ff 15 d7 c7 02 00       callq  *0x2c7d7(%rip)        # 30aa8 <_GLOBAL_OFFSET_TABLE_+0x118>
    42d1:       3c 06                   cmp    $0x6,%al
    42d3:       74 25                   je     42fa <_ZN3foo4main17h9be0dd2b2eb4c81eE+0x3a>
    42d5:       3c 02                   cmp    $0x2,%al
    42d7:       72 1f                   jb     42f8 <_ZN3foo4main17h9be0dd2b2eb4c81eE+0x38>
    42d9:       48 8d 3d 96 fd 01 00    lea    0x1fd96(%rip),%rdi        # 24076 <_fini+0xb5a>
    42e0:       48 8d 0d b1 b2 02 00    lea    0x2b2b1(%rip),%rcx        # 2f598 <__dso_handle+0x20>
    42e7:       be 0b 00 00 00          mov    $0xb,%esi
    42ec:       ba 02 00 00 00          mov    $0x2,%edx
    42f1:       58                      pop    %rax
    42f2:       ff 25 88 c9 02 00       jmpq   *0x2c988(%rip)        # 30c80 <_GLOBAL_OFFSET_TABLE_+0x2f0>
    42f8:       58                      pop    %rax
    42f9:       c3                      retq   
    42fa:       48 8d 3d 4a fd 01 00    lea    0x1fd4a(%rip),%rdi        # 2404b <_fini+0xb2f>
    4301:       48 8d 15 78 b2 02 00    lea    0x2b278(%rip),%rdx        # 2f580 <__dso_handle+0x8>
    4308:       be 2b 00 00 00          mov    $0x2b,%esi
    430d:       ff 15 85 ca 02 00       callq  *0x2ca85(%rip)        # 30d98 <_GLOBAL_OFFSET_TABLE_+0x408>
    4313:       0f 0b                   ud2    
    4315:       66 2e 0f 1f 84 00 00    nopw   %cs:0x0(%rax,%rax,1)
    431c:       00 00 00 
    431f:       90                      nop

[jyn514]

My understanding is that those conversions are defined as long as the usize is a valid discriminant. If that's not the case, then isn't every use of a #[repr(C)] enum in FFI context undefined behavior?

It is defined only if the usize is valid, right. You addressed this point below.

It is in fact guaranteed by the compiler.

I didn't know this, thank you. In that case it looks like the behavior is sound, just confusing on a first read. If you like I can make a PR documentating why the transmute is safe.

[sfackler]

In that case it looks like the behavior is sound, just confusing on a first read. If you like I can make a PR documentating why the transmute is safe.

Please do!

[jyn514]

I pushed some documentation to this PR.

[sfackler]

Thanks!