Call include_file! on loaded pio file #54
Conversation
This works around rp-rs#53. A better fix, proc_macro::tracked_path::path, is unstable.
| // a recompile. Should be replaced by | ||
| // `proc_macro::tracked_path::path` when it is stable. | ||
| let dummy_include = match file { | ||
| Some(file_path) => quote! {let _ = include_bytes!( #file_path );}, |
There was a problem hiding this comment.
This could also be:
| Some(file_path) => quote! {let _ = include_bytes!( #file_path );}, | |
| Some(file_path) => quote! {const _:&[u8] = include_bytes!( #file_path );}, |
Not sure which one is better.
| to_codegen( | ||
| program, | ||
| args.max_program_size, | ||
| args.file_path.into_os_string().into_string().ok(), |
There was a problem hiding this comment.
Is it really ok() that we want? An error would be ignored and yield surprising results
There was a problem hiding this comment.
My reasoning for using ok() was that I thought the existing implementation of the pio_file macro would work with non-UTF-8 filenames. I didn't want to break that, but there's no way to pass a Path or OsString that's not also a valid String to include_bytes.
However I now tested this, and noticed that my assumption was wrong: pio_file only works if it gets passed a literal string, not (eg.) a byte-string (b"....").
BTW, the current behavior if the first argument to pio_file! is not a string literal is also quite surprising: The parameter is silently ignored, and an empty program is parsed.
Anyway, it currently doesn't matter if we call .ok() or .unwrap(), the code in question is only called with paths generated from valid strings.
Would you prefer this change to make the code more robust against future changes?
| args.file_path.into_os_string().into_string().ok(), | |
| Some(args.file_path.into_os_string().into_string().expect("file path must be valid UTF-8")), |
This works around #53. A better fix, proc_macro::tracked_path::path, is unstable.
I did not yet make sure that the compiler optimizes the included bytes out. That should be done before merging.
(And any more elegant alternative solution would be more than welcome, of course!)