patterncppMinor

Nothing compiler/interpreter, Part 2

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

interpreterpartnothingcompiler

Problem

Part 1

I've followed some of the suggestions:

Adding newlines to error messages

Implementing options (I decided to use boost instead of getopt)

Using strerror for fstream failure

Allow multiple files to be compiled (to produce one binary - I wasn't sure if the suggestion meant one binary for each file)

Things I did not implement yet:

Localization

Reading code from standard input instead of files

Allowing the user to choose which architecture and platform the binary should be for (trivial)

Things I'm looking for in the review:

-
Readability. Especially on the program structure and comments

-
Whether or not it is intuitive for somebody who's used to a compiler like GCC. For example, I added --silent as a suggestion from the previous question, but it's not an option that I'm too familiar with. Although it DOES save people the trouble of piping strerr to /dev/null/

-
Should the error for chmod be handled at the caller or inside set_executable_permissions?

Example run:

% ./nothing
nothing: fatal error: no input files
compilation terminated.
% ./nothing --help
usage: ./nothing [OPTIONS] ...
Allowed options:
  --help                            produce help message
  --silent                          suppress error messages
  -o [ --output-file ] arg (=a.out) output file
% ./nothing asdf
nothing: error: asdf: No such file or directory
nothing: fatal error: no input files
compilation terminated.
% ./nothing --silent asdf
% touch empty.not
% ./nothing -o my_program empty.not
% objdump -d ./my_program

./my_program:     file format elf64-x86-64

Disassembly of section .text:

0000000008048000 :
 8048000:   bb 00 00 00 00          mov    $0x0,%ebx
 8048005:   b8 01 00 00 00          mov    $0x1,%eax
 804800a:   cd 80                   int    $0x80

And the code dump:

```
/*
* A 'Nothing' compiler
*
* September 2010, RoPe Development Inc.
*
* Author: authorname
*/
#include
#include

#include
#include
#include
#include
#

Solution

This seems like a pretty solid solution. I have mainly minor comments.

Comments

The purpose of comments is to explain logic to readers of your code. To that end, try to avoid comments that are trivial. Like:

#include  // EXIT_FAILURE
#include  // std::strerror
#include   // std::char_traits::eof

#include  // chmod

Those comments really achieve nothing for you. Besides, what you need ` for is std::string

, so besides providing no value they're actually misleading.

Similarly, you have this huge block comment explaining about

errno

 using thread-local storage - but you have no threads, so I don't see how that comment is remotely relevant. Just print the error. 

Setting Permissions

stat()

 returns 0 on success, -1 on failure, so checking for >= 0 is weird. Also you're taking the filename by value, which triggers an unnecssary copy. Instead you could do:

int set_executable_permissions(const char* filename)
{
    struct stat st;
    if (stat(filename, &st) == 0) {
        return chmod(filename, st.st_mode | ... );
    }
    else {
        return -1;
    }
}


As-is, if

stat() fails, you're returning 0, which seems pretty misleading! Also, people know what chmod is, so you can just do st.st_mode | 0111

. 

main()

I'd move the whole body into a separate function just so you can remove one layer of indentation:

try {
    return run_compiler(argc, argv);
}
catch (std::exception& e) {
    std::cerr << "nothing: error: " << e.what() << "\n";
    return EXIT_FAILURE;
}


Checking file emptiness

Your check:

program.peek() != std::char_traits::eof()


Preferred:

program.peek() != std::ifstream::traits_type::eof();


Use the

traits_type from the stream you're using. It's clearer and doesn't lead to the question of where char_traits

 comes from.

Also, if the program doesn't exist but

silent is false`, we still want to fail. So the flow should probably be:

if (!program) {
    if (!silent) {
        std::cerr << ...;
    }
    return EXIT_FAILURE;
}
else if (program.peek() != ... ) {
    if (!silent) {
        std::cerr << ...;
    }
    return EXIT_FAILURE;
}

Furthermore, this sort of calls for a separate function:

bool is_empty_file(std::string const& input_file)
{
    ...
}

that you could just call on each of the files:

if (!std::all_of(input_files.begin(), input_files.end(), is_empty_file))
{
    return EXIT_FAILURE;
}

Code Snippets

#include <cstdlib> // EXIT_FAILURE
#include <cstring> // std::strerror
#include <string>  // std::char_traits<char>::eof

#include <sys/stat.h> // chmod

int set_executable_permissions(const char* filename)
{
    struct stat st;
    if (stat(filename, &st) == 0) {
        return chmod(filename, st.st_mode | ... );
    }
    else {
        return -1;
    }
}

try {
    return run_compiler(argc, argv);
}
catch (std::exception& e) {
    std::cerr << "nothing: error: " << e.what() << "\n";
    return EXIT_FAILURE;
}

program.peek() != std::char_traits<char>::eof()

program.peek() != std::ifstream::traits_type::eof();

Context

StackExchange Code Review Q#102733, answer score: 2

Revisions (0)

No revisions yet.