Basic parser implementation #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merged

Cyan-Coder merged 3 commits into OneLoneCoderCommunity:master from AgathokakologicalBit:master

Jun 18, 2018

Merged

Basic parser implementation #2

Cyan-Coder merged 3 commits into OneLoneCoderCommunity:master from AgathokakologicalBit:master

Jun 18, 2018

Conversation

AgathokakologicalBit

Copy link

Member

@AgathokakologicalBit AgathokakologicalBit commented Jun 11, 2018

Compiler can parse:

variable declarations
math expressions
function calls
functions declaration

@AgathokakologicalBit


 Basic parser implementation

db3c4d9

@AgathokakologicalBit AgathokakologicalBit added the enhancement label

Jun 11, 2018

@AgathokakologicalBit AgathokakologicalBit requested review from Hazurl, Cyan-Coder and a team

June 11, 2018 20:41

AgathokakologicalBit added 2 commits

June 12, 2018 07:39

@AgathokakologicalBit


 Updated .gitignore

3147f94

@AgathokakologicalBit


 Parsing update

cac78e2

Added if statement
Added return statement
Implemented script::findSymbol(id) function

Hazurl

Hazurl suggested changes

Jun 12, 2018

View reviewed changes

Copy link

Contributor

@Hazurl Hazurl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry but there is too much thing to change before merging... 😥

compiler/error.h

enum class error_t

{

// Empty

c_no_errors = 0,

Copy link

Contributor

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you care about the value ? this is prone to errors...

compiler/parsing/node.h

{

node_type type;

union

Copy link

Contributor

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You want to kill my self ? 😥
You should create a class for each type of node and use std::variant.
Or use polymorphism.

Copy link

Member Author

@AgathokakologicalBit AgathokakologicalBit Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, maybe you are right. If you can help to rewrite the whole thing - u'r welcome!

compiler/parsing/node.h

{

struct

{

std::vector<node *> *bindings;

Copy link

Contributor

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

naked pointer ? Better use std::unique_ptr

compiler/parsing/node.h

t_value,

};

struct context

Copy link

Contributor

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like snake case for types... I prefer CamelCase

Copy link

Member Author

@AgathokakologicalBit AgathokakologicalBit Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't talk about your preferences. We should define standard naming convention for that.

Copy link

Contributor

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, this is why I make sure to share my opinion.

compiler/parsing/operators.cpp

#define ASSOC_RIGHT true

namespace olcl { namespace parsing {

operator_t const operators_list[24]

Copy link

Contributor

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They should not be hardcoded, an operator can be anything, even !$^%.+-*-<<?

Copy link

Contributor

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could copy haskell about associativity and precedence
So having a statement to set both attributes of an operator.

Copy link

Member Author

@AgathokakologicalBit AgathokakologicalBit Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a good option. Predefined operators list is more precise.

Copy link

Contributor

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but they should not, since we want user definable operators.
If we tool create them in the language, there is no reason to hardcode them.

include/script.h

script(node * ast);

public:

symbol * findSymbol(std::string id) const;

Copy link

Contributor

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no const& ?

Copy link

Member Author

@AgathokakologicalBit AgathokakologicalBit Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too small error, doesn't really affect anything, but I'll improve that place...

include/symbol.h

// TODO: use special type

std::string *number;

struct

Copy link

Contributor

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

anonymous structure 😥

Copy link

Member Author

@AgathokakologicalBit AgathokakologicalBit Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anonymous structures, unions, classes and namespaces are very useful

Copy link

Contributor

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

anonymous structure/unions/classes are evil, why are lazy to not naming them? it save so much when debuging.

api/engine.cpp

}

script * engine::loadScript(std::string filename)

Copy link

Contributor

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::unique_ptr

Copy link

Member Author

@AgathokakologicalBit AgathokakologicalBit Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got you, no need in marking each dangerous place

Copy link

Contributor

@Hazurl Hazurl Jun 12, 2018 •

edited

Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I make sure that you don't forget it 😈

api/script.cpp

{

symbol * val_to_symbol(node * type)

{

if (type->type != node_type::t_value)

Copy link

Contributor

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would not happen with std::variant because you could request the type of a Value Node in parameter instead of a general Node

api/script.cpp

}

script::script(olcl::node * ast_)

Copy link

Contributor

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why would you use assert when type cheking could works even better ? An other reason anonymous structure in union was a bad idea...

Copy link

Member Author

@AgathokakologicalBit AgathokakologicalBit Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got you

Cyan-Coder

Cyan-Coder approved these changes

Jun 18, 2018

View reviewed changes

@Cyan-Coder Cyan-Coder merged commit d7b0784 into OneLoneCoderCommunity:master

Jun 18, 2018

Labels

enhancement

3 participants

@AgathokakologicalBit @Hazurl @Cyan-Coder

Basic parser implementation #2

Basic parser implementation #2

Uh oh!

Conversation

@AgathokakologicalBit AgathokakologicalBit commented Jun 11, 2018

Uh oh!

@Hazurl Hazurl left a comment

Choose a reason for hiding this comment

Uh oh!

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

@AgathokakologicalBit AgathokakologicalBit Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

@AgathokakologicalBit AgathokakologicalBit Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

@AgathokakologicalBit AgathokakologicalBit Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

@AgathokakologicalBit AgathokakologicalBit Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

@AgathokakologicalBit AgathokakologicalBit Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

@AgathokakologicalBit AgathokakologicalBit Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

@Hazurl Hazurl Jun 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

@Hazurl Hazurl Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

@AgathokakologicalBit AgathokakologicalBit Jun 12, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

@Hazurl Hazurl Jun 12, 2018 •

edited

Loading