Builtin lines function part I - Refactoring #565
Conversation
| let id = meta.gen_array_id(); | ||
| meta.stmt_queue.push_back(format!("__AMBER_ARRAY_ADD_{id}=({left} {right})")); | ||
| format!("{quote}${{__AMBER_ARRAY_ADD_{id}[@]}}{quote}") | ||
| let name = format!("__AMBER_ARRAY_ADD_{id}"); |
There was a problem hiding this comment.
We'll need to pass name into a new TranslateModule function in the next PR, so it makes sense to store it in a local variable here.
| } | ||
| } | ||
| Type::Array(_) => { | ||
| if self.is_ref { |
There was a problem hiding this comment.
I unrolled some of the code which was previously shared across expression types in this match statement; this is necessary so we can append additional code to the Array case in the next PR.
There was a problem hiding this comment.
This should also make it slightly easier to implement @Ph0enixKM's proposed new translation layer.
| let name = match self.global_id { | ||
| Some(id) => format!("__{id}_{}", self.var), | ||
| None => if self.is_ref { format!("${{{}}}", self.var) } else { self.var.clone() } | ||
| let name = if let Some(id) = self.global_id { |
There was a problem hiding this comment.
Changed to match the shorthand add operator.
| match self.global_id { | ||
| Some(id) => format!("__{id}_{name}={expr}"), | ||
| None => format!("{local}{name}={expr}") | ||
| if let Some(id) = self.global_id { |
There was a problem hiding this comment.
We previously tested self.global_id after self.is_fun_ctx, but I switched the order to match the shorthand operators. I think this is safe, because global_id and is_fun_ctx cannot both be enabled at the same time.
| .map(|index| format!("[{}]", self.is_ref | ||
| .then(|| index.translate_eval(meta, true)) | ||
| .unwrap_or_else(|| index.translate(meta)))) | ||
| .map(|index| self.translate_eval_if_ref(index, meta)) |
There was a problem hiding this comment.
Added helper function translate_eval_if_ref() to remove duplicated code here. I would have moved this new function to the definition of TranslateModule, but it only seems to be required here.
| } | ||
| if self.is_ref { | ||
| if let Some(id) = self.global_id { |
There was a problem hiding this comment.
We previously tested self.global_id after self.is_ref, but I switched the order to match the shorthand operators. I think this is safe, because global_id and is_ref cannot both be enabled at the same time.
|
@hdwalters should I wait for this PR until I implement the TranslationModules? |
|
Please wait for this PR and the next one, which I have not submitted yet. |
hdwalters
changed the title
Refactor the
+=shorthand add operator, by consolidating the variousself.is_reftests for text, array and numeric addition. This is necessary because in a subsequent PR, we will append functionality for array addition only, and this is not possible with the current implementation. Make corresponding changes to other shorthand operators, initialisation and setter statements.