Skip to content

Conversation

ggwpez
Copy link

@ggwpez ggwpez commented Mar 3, 2023

Closes #58

Currently a file is generated in a proc macro, which is hacky as also stated in the code itself.

// FIXME: This is a silly hack to prevent output file from being tracking by
// cargo. Another better solution should be considered.

The cargo build-cache evasion seems to not reliably work, which results in a nondeterministic and occasional compile error.

Now it instead uses a string of the tokens and re-expands it - completely without any file-system operations.
I did not pay close attention to the surrounding code, but it did not break any tests or my website 😄

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@Kogia-sima
Copy link
Member

@ggwpez Thank you very much for your efforts.

At first, we must ask you one question. You mean this change also fixes an error which occured in #58 ? If it is the case, then the error might be due to Rust's builtin include! macro, rather than sailfish.

Also include! hack is required to improve some sementic error messages. Look at the following template.

<div>
<% for i in 0..10 { %>
  <div>head</div>
  <% if i < "string" { continue; } %>
  <div><%= i %></div>
<% } %>
</div>

sailfish v0.6.0 generates the following error for this.

error[E0277]: can't compare `{integer}` with `&str`
 --> /home/kogia-sima/PG/Rust/sailfish/target/debug/build/sailfish-compiler-8b7fd8b9e3b1455f/out/templates/continue-break-a658f671bde5c8c7:5:14
  |
5 |         if i < "string" {
  |              ^ no implementation for `{integer} < &str` and `{integer} > &str`
  |
  = help: the trait `PartialOrd<&str>` is not implemented for `{integer}`
  = help: the following other types implement trait `PartialOrd<Rhs>`:
            f32
            f64
            i128
            i16
            i32
            i64
            i8
            isize
          and 6 others

Althogh the file name is not human-readable, it would help users to find error sources. However, with this OR, the error becomes helpless.

error[E0277]: can't compare `{integer}` with `&str`
   --> sailfish-tests/integration-tests/tests/template_once.rs:103:10
    |
103 | #[derive(TemplateOnce)]
    |          ^^^^^^^^^^^^ no implementation for `{integer} < &str` and `{integer} > &str`
    |
    = help: the trait `PartialOrd<&str>` is not implemented for `{integer}`
    = help: the following other types implement trait `PartialOrd<Rhs>`:
              f32
              f64
              i128
              i16
              i32
              i64
              i8
              isize
            and 6 others
    = note: this error originates in the derive macro `TemplateOnce` (in Nightly builds, run with -Z macro-backtrace for more info)

I still can't reproduce #58 error on my machine but, since filetime modification is applied soon after generation, filetime hack is not related to the error.

Anyway, we need more detail information about #58 before merging the PR.

@Kogia-sima Kogia-sima added the Status: Need More Info Lacks enough info to make progress label Mar 5, 2023
@ggwpez
Copy link
Author

ggwpez commented Mar 6, 2023

I played around a bit with the quote_spanned macro, but there is non valid span that could be used here.
So it seems that your include! solution is the only one that will keep the proper error span.

I will try to get it working than with that approach. Maybe somehow the filesystem is not synced after the set_file_times… my CI is running on a slow HDD inside docker. So maybe there is an issue with file system sync.

@ggwpez
Copy link
Author

ggwpez commented Jul 28, 2023

Closing in favour of #125

@ggwpez ggwpez closed this Jul 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Need More Info Lacks enough info to make progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

expected expression, found <eof>

2 participants