Improving Amber Compiler Architecture with Translation Modules

[Ph0enixKM]

Preface

Hi all, it seems that as we add new features, a need arises to create a separate pass for manipulating translated Bash code. I encountered this problem recently when implementing the len built-in, which, if implemented efficiently, would require checking whether the translated expression is represented as a variable. The challenge is that checking the length in Bash involves using the ${#variable_name} syntax, which does not accept string literals for example - only variables. Without knowing if the expression in len is represented as a variable, we cannot efficiently implement this feature without creating additional temporary variables.

Proposition

Implementing another intermediate translation layer that will translate syntax module to a new translation module and then it to string just like we did before. This will come with a few improvements:

1. Easier implementation of an optimization layer - we will be able to better optimize resulting Bash code
2. Better control over shell features - if we define a translation module that implements certain feature then if we want we can easily switch which version are we going to render (Bash or sh... or any other version) all in one place without having to check for it in every single place where we might use it.
3. Better predictability - less places to make mistakes means that less bugs can happen

How will these translation modules look like?

This depends on the compiler’s needs. It’s not necessary to implement each shell feature as a separate module. There could be translation modules for variables, literals, arithmetic, etc., or a single module that represents some Bash. This is especially useful in contexts where precise syntax definition isn’t required. Built-in commands like echo are a good example, as they are self-sufficient statements that can likely be translated without a dedicated module. This also has a benefit of easier onboarding for new members. They don't have to learn all the translation modules to start contributing.

The architecture

The architecture that we have built so far looks as follows:

The proposition would modify the last block Translate by splitting it into Translate to AST implemented on Syntax Modules and Translate to String which now will be implemented on the new Translation Modules.

Summary

I believe that it's a step in a correct direction bringing us closer to making Amber output code that is more:

• Readable and Performant - by utilizing the optimization opportunity later on in the future (adding the Optimize layer)
• Predictable - by having all Bash translation in the same place (in one and single Translation Module)
• Portable - by having a better control over what's being translated

[b1ek]

idk about this. do you really want to maintain a bash AST serializer/deserializer? maybe make it like as a separate library that we depend on. i wouldn't want to have that as a part of amber itself

[Ph0enixKM]

@b1ek It's not a Bash AST. It's a representation of the output Bash code in a way that is more refined. For instance when we return a translation module that is a variable, we can get the variable name or the variable array representation with a simple function call. This way we don't have to do witchcraft that we already do with checking strings like if string.startsWith("$") && !string.startsWith("$("). Ofc this is just an example.

[hdwalters]

I also dislike manipulating Bash code as a string; it's prone to developer error and unforeseen edge cases, and in some cases makes the code harder to modify with new features.

For example, I'm currently implementing the new lines builtin (requiring changes to the iteration loop, let statement, binop and shorthand addition operators). I'm having particular trouble with variables passed by reference, as the Bash code has to be modified to run in this context. If we could defer this step (including escaping backslashes, doubling up curly braces and embedding in an eval statement) to a new translation layer, it would make the entire process a lot cleaner.

You mentioned removing additional Bash variables to efficiently implement the len builtin; we might also be able to remove the additional intermediate variables we seem to need in array assignment. And another thing that came out of the len PR was the possibility of generating unique temporary Bash variable names in a consistent, centralised manner.

However, this will be a large piece of work, touching most of the source code tree; it will almost certainly interfere with other development work, so we would have to plan it out carefully. In particular, it would be helpful if the new layer could be implemented as an optional feature, with fallback to the old method, so that existing builtins can be upgraded over time. Once everything has been upgraded, we can remove the scaffolding.

In order to better understand what you have in mind, can you provide pseudo-code for some typical use cases?

[Ph0enixKM]

@hdwalters sure, simple example for translate function of VariableGet:

Currently we do (or something similar to the following example):

impl TranslateModule for VariableGet {
    fn translate(meta: TranslateMetadata) -> String {
        let value = self.variable_name.clone();
        format!("\"${{{value}}}\"")
    }
}

The proposition mentioned above would transform it to the following pseudo code:

impl TranslateModule for VariableGet {
    fn translate(meta: TranslateMetadata) -> TranslationModule {
        VariableGetTranslation::new(
            &meta,
            self.variable_name,
            self.variable_type.clone()
        )
    }
}

// The implementation of the translation module:

trait TranslationModule {
    // Returns shell representation.
    fn to_string() -> String;
    // ...
}

struct VariableGetTranslation {
    name: String;
    type: Type;
}

// ...

impl VariableGetTranslation {
    // Meta is required for some cases to know if we're in eval context for instance
    fn new(&meta: TranslateMetadata, name: &str, type Type) {
        // ...
    }

    fn get_name() {
        self.name.clone()
    }
    
    fn get_name_expression() {
        let expr = self.name.clone()
        if self.type.is_array() {
            expr + "[@]"
        } else {
            expr
        }
    }
    
    fn get_expression() {
        let expr = self.get_name_expression();
        format!("\"${{{expr}}}\"")
    }
}

[KrosFire]

So what you want to achive is to be able to analyze and optimize the AST, so that you won't have create unnecessary boilerplate?

Sounds pretty reasonable but the biggest question is about optimization. We would really need to study and benchmark these solutions to be sure that it doesn't break anything and makes things faster, not slower. Another thing is that this optimization could be optional.

I think it's a good idea to better analyze what we will produce and what shortcuts can we make, but I think some mvp is needed

[Ph0enixKM]

Sure, @KrosFire. I think that we can start with some small output "AST" that is not precise at all and handles just the shell syntax that is the most tricky to translate. When it comes to benchmarks - we can leave that for later. Speed is not at the essence of shell scripting yet I understand that it's pretty important.

[hdwalters]

AIUI, the main thing is to simplify development of new features. As long as the performance is about the same or better, I think we'll be happy.