Implements skel functions for scope refactoring by boohyunsik · Pull Request #358 · DE-labtory/koa

boohyunsik · 2019-02-26T15:41:10Z

[WIP] Refactoring scope

zeroFruit

How about changing function from ScopingXXX to ResolveXXX, later these functions should do type checking.
How about changing Symbol to Object?

zeroFruit · 2019-02-28T04:12:26Z

-	}
+// Define scope errors
+type DupError struct {
+	Identifier ast.Identifier


I think it is better DupError has Symbol not Identifier

zeroFruit · 2019-02-28T04:15:12Z

+	s = NewEnclosedScope(s)
+
+	if err := ScopingFunctionParameter(f.Parameters, s); err != nil {
+		return nil


zeroFruit · 2019-02-28T04:16:43Z


-func (s *Scope) GetOuter() *Scope {
-	return s.outer
+func ScopingFunctionParameter(f []*ast.ParameterLiteral, s *Scope) error {


why f ? could have better name

zeroFruit · 2019-02-28T04:16:58Z

+	for _, p := range f {
+		switch p.Type {
+		case ast.IntType:
+			{


remove {} plz..

zeroFruit · 2019-02-28T04:19:58Z

+		err = ScopingIfStatement(implStmt, scope)
+	}
+
+	if err != nil {


I think this if statement don't need, just return err enough

zeroFruit · 2019-02-28T05:59:19Z

+func NewScope() *Scope {
+	return &Scope{
+		store:  make(map[string]Symbol),
+		parent: nil,


don't need this line

These are all WIP..:-)

hihiboss · 2019-03-03T13:13:44Z

+}
+
+func (s *Scope) Get(name string) Object {
+	scope := s


Why use scope variable instead of s??

If you want to clone Scope, scope := *s or func (s Scope) Get(name string) Object {} might be right

hihiboss · 2019-03-03T13:26:39Z

+	r.scope = NewEnclosedScope(r.scope)
+
+	if err := ResolveFunctionParameter(f.Parameters, r); err != nil {
+		return nil


Why return nil?

hihiboss · 2019-03-03T13:36:16Z

+
+		var obj Object
+		var t ObjectType
+		switch p.Type {


(refactor)

How about define func resolveParamObj like

func resolveParamObj(p ast.ParameterLiteral) (Object, ObjectType) { switch p.Type { case ast.IntType: return &Integer{Name: p.Identifier}, IntegerObject ... }

And,

in func ResolveFunctionParameter()

for _, p := range p { if obj, ok := r.scope.store[p.Identifier.Name]; ok { return DupError{ object: obj, } } obj, t := resolveParamObj(p) r.scope.store[p.Identifier.Name] = obj r.types[p] = t r.defs[p.Identifier] = obj } return nil

hihiboss · 2019-03-03T13:44:53Z

 }
+
+func ResolveFunctionParameter(p []*ast.ParameterLiteral, r *Resolver) error {
+	for _, p := range p {


Variable name is duplicated!
Although there is not any errors, i think it should be changed for readability.
ex) for _, param := params

hihiboss · 2019-03-03T13:46:05Z

+
+func ResolveStatement(stmt ast.Statement, r *Resolver) error {
+	var err error
+	switch implStmt := stmt.(type) {


case *ast.AssignStatement: return ResolveAssignStatement(implStmt, r)

is better!

boohyunsik force-pushed the refactor/scope branch 2 times, most recently from 0a19770 to 2744225 Compare February 27, 2019 14:51

zeroFruit reviewed Feb 28, 2019

View reviewed changes

boohyunsik added 3 commits February 28, 2019 21:30

Implements skel functions for scope refactoring

8f84de2

Implements default tcs for scoping assign, if

916ce5e

Implements ScopingAssignmentStatement and tcs

6f84f82

boohyunsik force-pushed the refactor/scope branch 7 times, most recently from 13d2a62 to 97a9e19 Compare March 3, 2019 09:07

hihiboss reviewed Mar 3, 2019

View reviewed changes

boohyunsik force-pushed the refactor/scope branch from 97a9e19 to 8639abd Compare March 5, 2019 15:55

Add resolving assignment stmt functions and tcs

69a9ce2

boohyunsik force-pushed the refactor/scope branch from 8639abd to 69a9ce2 Compare March 5, 2019 16:09

Impl ResolveCallExpression and tcs

8ec4c1a

boohyunsik force-pushed the refactor/scope branch from caac93f to 8ec4c1a Compare March 13, 2019 16:03

Conversation

boohyunsik commented Feb 26, 2019

Uh oh!

zeroFruit left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boohyunsik Feb 28, 2019 • 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!

Choose a reason for hiding this comment

Uh oh!

hihiboss Mar 3, 2019 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

boohyunsik Feb 28, 2019 •

edited

Loading

hihiboss Mar 3, 2019 •

edited

Loading