(feat) code generator addition #7

Defancet · 2022-10-13T20:46:39Z

Started working on code generator, added working with the frame and calling functions instructions, as well as arithmetic, relational, boolean and conversion instructions

…alling functions instructions, as well as arithmetic, relational, boolean and conversion instructions

NickSettler

The following changes can be made:

addition of the tests using googletest
proper file and function comments according to the task and code style in the project
addition of the missing functions
fixing some existing functions (if needed)
using string_t in all functions
refactoring some functions not to repeat them

NickSettler · 2022-10-13T21:45:45Z

src/code_generator.c

+#include <stdio.h>
+#include <stdbool.h>
+
+// Assign a value to a variable


Take a look here:

Functions should be commented in Doxygen style.

For example this function declaration should be commented like

/** * Creates tree node with value * @param value value in the node * @return pointer to the tree node */ tree_node *create_node(int value);

It is enough to comment just function declarations, without commenting implementation in C files. Your IDE will probably automatically highlight such a comment as a function comment. First and next lines contain function description. There may be some keywords starting with @ sign after them. IDE may also suggest some keywords. Probably will be needed only @param which is used to describe function parameter and @return which is used to describe what is returned from function.

So don't write any comments in .c files. It is better to move them to .h files and follow Doxygen format

NickSettler · 2022-10-13T21:46:39Z

src/code_generator.c

+#include "code_generator.h"
+
+#include <stdio.h>
+#include <stdbool.h>


Includes should be placed in .h file. The only .h file which should be included in .c file - is the header file. In your case - code_generator.h.

NickSettler · 2022-10-13T21:50:27Z

src/code_generator.c

+
+// Assign a value to a variable
+void generate_move(char *parameter_frame, char *parameter, char *variable_frame, char *variable) {
+    fprintf(fd, "MOVE %s%s %s%s\n", parameter_frame, parameter, variable_frame, variable);


Following the task, there must be @ signs between frame and variable name in the MOVE instruction

By the way, maybe it would be better to name function parameters according to the task - first and second are variable frame and variable, third and fourth - symbol frame and symbol.

NickSettler · 2022-10-13T21:54:58Z

src/code_generator.h

+
+FILE* fd; //file for testing
+
+void generate_move(char *parameter_frame, char *parameter, char *variable_frame, char *variable);


I suppose we should add to every function a parameter. String to which function will append instruction string. So these functions can be easily used for tests. For this purpose is appropriate to use string_t type. Its function string_append_string can be used to append a custom char*.

src/code_generator.h

NickSettler · 2022-10-13T22:00:21Z

src/code_generator.h

+#ifndef IFJ_PROJ_2022_CODE_GENERATOR_H
+#define IFJ_PROJ_2022_CODE_GENERATOR_H
+
+FILE* fd; //file for testing


For testing you can use GoogleTest framework. Take a look at the tests directory. The LexicalAnalyzerTest for now is the largest test. You can use it as an example for writing own tests. IMHO, it maybe enough to add a test for each function and tests, which will have more than a one function - just to check line breaks and stuff like that.

NickSettler · 2022-10-13T22:04:48Z

src/code_generator.h

+void generate_idiv (char* result, char* symbol1, char* symbol2);
+void generate_lt (char* result, char* symbol1, char* symbol2);
+void generate_gt (char* result, char* symbol1, char* symbol2);
+void generate_eq (char* result, char* symbol1, char* symbol2);


Some functions are missing. Instruction set in the task file contains about 40 instructions. I've counted 14 here

NickSettler · 2022-10-13T22:05:18Z

src/code_generator.c

+    printf("\nRETURN");
+}
+
+int test_code_generator() {


This function should not exist, when tests with googletest will be added

NickSettler · 2022-10-13T22:12:41Z

src/code_generator.c

+
+// Sum of two numerical values
+void generate_add(char *result, char *symbol1, char *symbol2) {
+    fprintf(fd, "ADD LF@%s LF@%s LF@%s\n", result, symbol1, symbol2);


This and the following instructions always contains LF as a frame. Can this frame be changed or there is always used LF as a frame in these instructions?

NickSettler · 2022-10-13T22:15:52Z

src/code_generator.c

+}
+
+//  Compare two values with logical OR
+void generate_or(char *result, char *symbol1, char *symbol2) {


Many functions are repeating in their arguments. Maybe it might be useful to union them into one universal function? For passing instruction keyword to this function can be used predefined enum with all instructions of similar type.

functions

Defancet · 2022-10-15T23:43:08Z

Needs to be done:

understand how to use string_t type
learn how to write google tests

…de_generator_addition

NickSettler

Request of the following changes:

Code style changes
Comments fixing
Refactor and performance changes

NickSettler · 2022-10-23T18:28:15Z

src/code_generator.c

+ * @author xkalut00, Maksim Kalutski
+ * @file   code_generator.c
+ * @brief  Code generator source file
+ * @date   ??.10.2022


Date should be set

NickSettler · 2022-10-23T18:33:28Z

src/code_generator.c

+void generate_operation(instructions_t instruction, char *result, char *symbol1, char *symbol2) {
+    switch (instruction) {
+        case ADD:
+            fprintf(fd, "ADD LF@%s LF@%s LF@%s\n", result, symbol1, symbol2);


As we have discussed, this can be changed to other way without switch case. You can create an enum with string key values. Or you can create a numbered enum an string array with values ordered in the same order as enum. So than you'll be able to use its values instead of switch..case like this

fprintf(fd, operations[instruction], ...);

Take a look at #13 for an example. In summary, we should avoid such large repeating constructions in our code

NickSettler · 2022-10-23T18:34:13Z

src/code_generator.c

+}
+
+void generate_stack_operation(stack_instructions_t instruction, char *result, char *symbol1, char *symbol2) {
+    switch (instruction) {


This switch case can be also changed in the way described previously. Also switch..case constructions down can be changed this way

NickSettler · 2022-10-23T18:42:38Z

src/code_generator.c

+
+#include "code_generator.h"
+
+void generate_move(char *variable_frame, char *variable, char *symbol_frame, char *symbol) {


Frames are actually predefined: GF, LF and TF. Maybe we can list them as an enumeration and pass them to code generator functions instead of passing them as a string

NickSettler · 2022-10-23T18:48:24Z

src/code_generator.h

+/**
+ * @brief function for choosing an arithmetic operation using a switch case
+ *        ADDS/SUBS/MULS/DIVS/IDIVS are stack versions of ADD/SUB/MUL/DIV/IDIV
+ * @param *operation stack operation


Parameter name in documentation doesn't match with the name of function parameter. They should be the same. I'm not sured, maybe other comments also have wrong parameter name

NickSettler · 2022-10-23T18:48:45Z

src/errors.c

@@ -1 +1,2 @@
 #include "errors.h"
+typedef int make_iso_compilers_happy;


This should be removed

NickSettler

Some refactoring changes

NickSettler · 2022-10-28T12:23:11Z

src/code_generator.c

+
+void generate_clear_stack() {
+    fprintf(fd, "LABEL clear_stack\n");
+    fprintf(fd, "MOVE GT@clear_var bool@true\n");


You are using frpintf here, so in these big functions you can also replace frame with its variable using enum

NickSettler · 2022-10-28T12:25:40Z

src/code_generator.h

+    CONCAT,
+    STRLEN,
+    GETCHAR,
+    SETCHAR,


Maybe it would be good to add some prefixes to all these enum members and to make something like CODE_GEN_ADD_INSTRUCTION. Other modules may have also something names ADD, so we need to be able to differ one thing from another

Started working on code generator, added working with the frame and c…

d967f35

…alling functions instructions, as well as arithmetic, relational, boolean and conversion instructions

Defancet added the feature New feature or request label Oct 13, 2022

Defancet requested review from NickSettler and pasynkovnikita October 13, 2022 20:46

Defancet assigned NickSettler, pasynkovnikita and LenaMarochkina Oct 13, 2022

Defancet changed the title ~~(feat)code generator addition~~ (feat) code generator addition Oct 13, 2022

Defancet assigned Defancet and unassigned LenaMarochkina, NickSettler and pasynkovnikita Oct 13, 2022

NickSettler requested changes Oct 13, 2022

View reviewed changes

NickSettler added this to the Sprint 2 milestone Oct 13, 2022

NickSettler linked an issue Oct 13, 2022 that may be closed by this pull request

Code generator functions addition #3

Closed

Refactoring code according to Nick's comments, added new

9f54bf8

functions

Defancet added 4 commits October 16, 2022 02:15

(feat) started working on code generator

3e7c8cc

(refactor) refactoring code

f6ef8e2

Merge remote-tracking branch 'origin/code_generator_addition' into co…

9e4f3cd

…de_generator_addition

(fix) fixed small bug

3a9ebda

NickSettler modified the milestones: Sprint 2, Sprint 3 Oct 17, 2022

NickSettler requested changes Oct 23, 2022

View reviewed changes

Defancet added 2 commits October 28, 2022 01:48

(refactor) code refactoring

250d967

(test) added code generator test file

f613b93

NickSettler requested changes Oct 28, 2022

View reviewed changes

Defancet added 2 commits October 28, 2022 20:49

(refactor) changed enum names,

b383f4f

(feat) added frames for some functions

2136c84

NickSettler previously approved these changes Oct 28, 2022

View reviewed changes

(bug) make changes to CMake file

0488a10

Defancet dismissed NickSettler’s stale review via 0488a10 October 28, 2022 19:39

Defancet and others added 4 commits October 28, 2022 21:45

(bug) make changes to CMake file

8c4594e

(bug) some more changes to CMake file

8f3e3cf

Merge branch 'master' into code_generator_addition

7c999b4

Update CMakeLists.txt

6c07ed7

NickSettler previously approved these changes Oct 29, 2022

View reviewed changes

(bug) remove test file

36d23ca

Defancet dismissed NickSettler’s stale review via 36d23ca October 29, 2022 14:43

(bug) typo fixed

ee7382d

NickSettler approved these changes Oct 29, 2022

View reviewed changes

pasynkovnikita approved these changes Oct 29, 2022

View reviewed changes

Defancet merged commit 6316154 into master Oct 29, 2022

NickSettler deleted the code_generator_addition branch November 8, 2022 16:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(feat) code generator addition #7

(feat) code generator addition #7

Defancet commented Oct 13, 2022 •

edited

Loading

NickSettler left a comment

NickSettler Oct 13, 2022

NickSettler Oct 13, 2022

NickSettler Oct 13, 2022

NickSettler Oct 13, 2022

NickSettler Oct 13, 2022

NickSettler Oct 13, 2022

NickSettler Oct 13, 2022

NickSettler Oct 13, 2022

NickSettler Oct 13, 2022

Defancet commented Oct 15, 2022

NickSettler left a comment

NickSettler Oct 23, 2022

NickSettler Oct 23, 2022

NickSettler Oct 23, 2022

NickSettler Oct 23, 2022

NickSettler Oct 23, 2022

NickSettler Oct 23, 2022

NickSettler left a comment

NickSettler Oct 28, 2022

NickSettler Oct 28, 2022


		FILE* fd; //file for testing

		void generate_move(char parameter_frame, char parameter, char variable_frame, char variable);


		#include "code_generator.h"

		void generate_move(char variable_frame, char variable, char symbol_frame, char symbol) {

		@@ -1 +1,2 @@
		#include "errors.h"
		typedef int make_iso_compilers_happy;

(feat) code generator addition #7

(feat) code generator addition #7

Conversation

Defancet commented Oct 13, 2022 • edited Loading

NickSettler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Defancet commented Oct 15, 2022

NickSettler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NickSettler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Defancet commented Oct 13, 2022 •

edited

Loading