4
\$\begingroup\$

I wrote a little C++ program that lets me transpile a syntax that allows running Shell in Python to legal Python and then execute it. Here is an example input:

filename = f'./data/lines.txt'
n_lines = int(`wc -l {filename}`.split()[0])
print(n_lines, 'lines in file')

This is transpiled to:

import subprocess

filename = f'./data/lines.txt'
_ = subprocess.run(f'wc -l {filename}'.split(), capture_output=True).stdout.decode('utf-8').strip()
n_lines = int(_.split()[0])
print(n_lines, 'lines in file')

and then executed. My main code is:

main.py

#include <regex>
#include <string>
#include <vector>
#include <sstream>
#include <cctype>
#include <memory>
#include <fstream>
#include <cstdlib>
#include <stdlib.h>
#include <iostream>
#include <filesystem>
#include "formatter.h"

/** Process a single line.
 *
 * @param line - The line to process
 * @return The processed Python code
 */
std::string process_line(std::string& line)
{
    std::ostringstream generated;

    // Parse template args in the string.
    if (line.find("`") != std::string::npos) {
        // Find all indices.
        std::vector<size_t> cmd_idx;
        size_t cur_tick_idx = 0;
        size_t next_tick_idx;

        // Find all backticks
        while ((next_tick_idx = line.find("`", cur_tick_idx)) != std::string::npos) {
            // First, check that it is not escaped.
            if (next_tick_idx <= 0 || line[next_tick_idx - 1] != '\\')
                cmd_idx.push_back(next_tick_idx);

            cur_tick_idx = next_tick_idx + 1;
        }

        // Ensure we have an even number of indices
        if (cmd_idx.size() % 2 == 1)
            throw "Invalid number of template quotes";

        // Begin substitution using formatters.
        for (size_t i{}, j{1}; i < cmd_idx.size(); i += 2, j += 2) {
            std::string substr = line.substr(cmd_idx[i] + 1, cmd_idx[j] - cmd_idx[i] - 1);

            generated << "_ = subprocess.run(f'" << substr << "'.split(), capture_output=True).stdout.decode('utf-8').strip()\n";

            // Check for formatters
            if (cmd_idx[i] > 0 && (std::isalnum(line[cmd_idx[i] - 1]) || line[cmd_idx[i] - 1] == '_')) {
                size_t k;
                for (k = cmd_idx[i] - 2; k >= 0; --k) {
                    if (!std::isalnum(line[k]) && line[k] != '_')
                        break;
                }

                std::string format = line.substr(k + 1, cmd_idx[i] - k - 1);

                // Apply formatter
                // If "str", do nothing.
                if (format != "str") {
                    std::unique_ptr<type_formatter> formatter = std::make_unique<type_formatter>(format);
                    generated << formatter->format();
                }
            }

            // Now, replace the part in quotes with our variable
            generated << line.replace(cmd_idx[i], cmd_idx[j] - cmd_idx[i] + 1, "_");
        }
    } else {
        generated << line;
    }

    return generated.str();
}

int main(int argc, char* argv[])
{
    // TODO: Change this to a filename input
    std::ifstream fin(argv[1]);
    std::ofstream fout("out.py");

    fout << "import subprocess\n\n";

    std::string line;

    while (std::getline(fin, line)) {
        fout << process_line(line) << std::endl;
    }

    fout.close();

    // Run the code
    const char* path = std::getenv("PATH");
    std::filesystem::path cur_path = std::filesystem::current_path();
    std::string new_path = std::string(path) + ":" + cur_path.string();

    if (setenv("PATH", new_path.c_str(), 1) != 0)
        throw "Failed to set PATH";

    std::system("python out.py");

    return 0;
}

and my formatter code is pretty simple:

formatter.cpp

#include "formatter.h"

type_formatter::type_formatter(const std::string& fmt) {
    this->fmt = fmt;
}

/**
 * Returns Python code that checks whether the string
 * can be safely casted to the desired type.
 * 
 * TODO: Check indent level
 */
std::string type_formatter::get_safe_formatter() {
    std::string check_cast_code = "try:\n\t"
        "_ = " + fmt + "(_)\nexcept ValueError:\n\t"
        "raise";

    return check_cast_code;
}

std::string type_formatter::format()
{
    if (fmt == "int" ||
        fmt == "float")
        return get_safe_formatter();

    if (fmt == "list")
        return "_ = _.split('\\n')\n";

    if (fmt.starts_with("list.")) {
        std::string list_type = fmt.substr(5);
        return "_ = [" + list_type + "(x) for x in _.split('\\n')]\n";
    }

    throw "Formatter for type does not exist.";
};

formatter.h

#ifndef FORMATTER_H
#define FORMATTER_H

#include <string>

/**
 * The base class for formatters. This is an abstract class
 * and should be extended to implement specific formatters.
 * Formatters must implement the `format()` function, which
 * should return a std::string containing Python code to process
 * a variable called _, which will contain the output from shell
 * code in the transpiled program. As a template, the code should
 * end up assigning _ to the correct type. The Python code should
 * end in a newline.
 */
class basic_formatter
{
public:
    basic_formatter() = default;
    virtual std::string format() = 0; 
};

class type_formatter : public basic_formatter
{
    std::string fmt;
    std::string get_safe_formatter();

public:
    type_formatter(const std::string&);
    type_formatter() = delete;
    virtual std::string format();
};

#endif

I primarily work with JS and Python, so I'm trying to understand how I can write better C++ code, what norms I've broken, what I could do better, etc. I'm using C++20.

\$\endgroup\$
0

2 Answers 2

4
\$\begingroup\$

Code that's ready for review shouldn't have any outstanding TODO comments. You should resolve those.

Instead of including both <cstdlib> and <stdlib.h>, use just the C++ header, and namespace-qualify std::size_t where it's used.

Class basic_formatter is intended for use as a base class. You should provide a virtual destructor so that subclasses are correctly destroyed when deleted via a base-class pointer:

basic_formatter() = default;
virtual ~basic_formatter() = default;

In type_formatter, I don't think we want implicit conversion from strings, so mark that constructor with explicit. Use the constructor's initializer-list to populate fmt, rather than letting it default-construct and then overwriting in the body. And if we pass by value, we can avoid an extra copy in many cases.

explicit type_formatter(const std::string fmt)
    : fmt{std::move(fmt)}
{}

It's not necessary to declare a deleted no-args constructor - just omit this, as the above constructor inhibits generation of a default constructor.

The format() method should be declared override instead of virtual{

std::string format() override;

Consider whether you really want to allow format() to modify the formatter - perhaps it should be const?

When we construct a formatter, we make a smart pointer. But we don't need to do that, as we can construct and use it directly:

            if (format != "str") {
                type_formatter formatter{format};
                generated << formatter.format();
            }

We have a logic error here:

            std::size_t k;
            for (k = cmd_idx[i] - 2; k >= 0; --k) {

That's an infinite loop, because k >= 0 can never be false. That's a sign of not enabling sufficient compiler warnings, and perhaps also of insufficient unit-testing.

When throwing, we should use appropriate subclasses of std::exception. C++ allows us to throw a string literal, but that is generally considered a poor practice. And we never catch anything we throw - that needs to be fixed.

process_line() shouldn't need to write to a string-stream and stringify the result, given that when we call it, we immediately write its result to another stream. Just pass the output stream into the function (as a reference) and write to it directly.

The search for matching backquote characters is flawed. A preceding backslash doesn't escape a backquote if the backslash itself is quoted - we'll need a smarter parser to give us real robustness here.

Lots of ways in which main() can misbehave, none of which give any diagnostic or exit status:

  • input filename isn't readable
  • unable to create or overwrite out.py in current directory (consider using mktemp() or similar to get somewhere safe to write)
  • writing fails later (e.g. disk full)
  • python is not on the path (likely in systems where the interpreters are called python2 and/or python3).

We lose the exit status of the invoked Python program. It's probably better to exec() rather than std::system() on POSIX systems, so that the caller gets the full exit status (including signal and core-dump information if appropriate). Where only standard-library functions are available, consider using the return value from std::system() to determine what to return from main().

\$\endgroup\$
2
  • \$\begingroup\$ This is fantastic and so helpful! Thank you so much! Are these things I can learn from a book (say Stroustrup's book), or is it mainly from practice? \$\endgroup\$
    – Rahul
    Nov 10, 2022 at 20:09
  • \$\begingroup\$ I've been writing C++ for a long time, so the books I learnt from are completely outdated now (thought there's a lot that's still relevant, particularly Scott Myers's Effective Modern C++). Practice is valuable, as is following and understanding reviews here and questions on Stack Overflow. Keep experimenting and bringing your code here for suggestions, and you'll gradually get better and better. :) \$\endgroup\$ Nov 11, 2022 at 8:03
2
\$\begingroup\$

In addition to what Toby Speight mentioned:

Expansion can happen in the wrong context

Consider the following Python input:

print("Never run `rm -rf .`")

Your transpiler will then run:

import subprocess

_ = subprocess.run(f'rm -rf .'.split(), capture_output=True).stdout.decode('utf-8').strip()
print("Never run _")

Deleting everything in the current directory, and printing a nonsensical message.

Why not make a Python function?

Having to need a transpiler means adding dependencies to a project. It also introduces lots of problems, correctly parsing the input as mentioned above just being one of them. Consider that the original input might still work as a Python program, but will do something different if you forget to run it via the transpiler.

Why not just make a Python function that does the substitution for you, with a short name like sh(), so that you can write something like:

filename = f'./data/lines.txt'
n_lines = sh('wc -l < {filename}').as_int()
print(n_lines, 'lines in file')

This would keep everything in Python, so no need for external tools, and no surprises.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for pointing out the edge case! I need to flesh out more scenarios to parse inputs better. re why not Python: I just wanted to play around in C++ :) A Python solution would also help with things like Intellisense, something I would've had to build myself with my current solution--which would also be instructive, of course. \$\endgroup\$
    – Rahul
    Nov 10, 2022 at 21:13

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.