Use raw identifier for reserved keywords #295
Conversation
| fn strip_raw_ident(ident: &str) -> &str { | ||
| const RAW_IDENT_PREFIX: &str = "r#"; | ||
|
|
||
| if ident.starts_with(RAW_IDENT_PREFIX) { | ||
| &ident[RAW_IDENT_PREFIX.len()..] | ||
| } else { | ||
| ident | ||
| } | ||
| } |
There was a problem hiding this comment.
I did not see a convenient way to share this logic between c2rust-bitfields-derive and c2rust-transpiler because they did not share a common crate. Is it worth removing the duplication?
There was a problem hiding this comment.
It's a small function, probably not worth de-duplicating.
| fn raw_identifier_if_reserved_name(basename: &str) -> Option<String> { | ||
| if RESERVED_NAMES.contains(&basename) && !RESERVED_PATH_SEGMENT_NAMES.contains(&basename) { | ||
| Some(format!("r#{}", basename)) | ||
| } else { | ||
| None | ||
| } | ||
| } |
There was a problem hiding this comment.
It is a bummer that RESERVED_NAMES and RESERVED_PATH_SEGMENT_NAMES leak into Renamer. I could make the Renamer take a function allowing the detection of reserved names to be configurable. Would this be preferred?
There was a problem hiding this comment.
Renamer already takes a reference to RESERVED_NAMES as the argument to its new constructor, so you could take that reference and stick it inside Renamer but I'm not sure that's worth it.
There was a problem hiding this comment.
Agreed, that was my initial approach (ie treat reserved_names as names that should be converted to a raw identifier). However, when I had to add RESERVED_PATH_SEGMENT_NAMES there did not seem to be value in keeping the reserved list internally.
|
@davidMcneil I've fixed the problem that caused Azure pipelines to fail. Can you rebase your PR on top of master? |
Signed-off-by: David McNeil <[email protected]>
|
@thedataking I rebased, but it is still failing. It looks like there is a problem reading this token? |
|
Correct, David that the github actions fails because of the secret token. I turned off actions on forked PRs for that reason so we can ignore that failure. If Azure Pipelines testing succeeds and Andrei is happy, we can can merge this PR. |
|
Should be this PR rebased and updated? @davidMcneil |
|
If I'm reading the code correctly (which I might not be, it is late in the evening) this will not handle the case I ran into where a C file had a name starting with a number: I'm not sure if that would be a separate PR or should be handled here. |
Resolves #279
Unfortunately, we cannot use a raw identifier for all keywords see the comment here.
Signed-off-by: David McNeil [email protected]