search

Michael2109 / cobalt

The Cobalt programming language
GNU Lesser General Public License v3.0
37 stars 10 forks source link

Cobalt 437 #487

Closed Michael2109 closed 6 years ago

Michael2109 commented 6 years ago

Added match parser.

FilipJanitor commented 6 years ago

Just one little thing - what is the purpose of wrapping the cases in assignment data? In the do one it is even wrapped twice - StmtAssignment . BlockStmt

Michael2109 commented 6 years ago

Isn't this required? It could either be an expression or a statement depending on whether it is inline or not. We wrap it in this like in method definitions to allow for both cases.

FilipJanitor commented 6 years ago

I understand what you mean and the similarity between those two usages, but still, even the name suggests that the data has something to with assignments which might not be the case for patternmatching. I am not sure, whether to wrap the inline one into ExprAsStmt or rename the data to some more general name that would suggest, that its purpose is to distinguish between inline and block stuff (which does not necessarly imply assignment context)

Michael2109 commented 6 years ago

I think the usages are very similar so renaming it sounds best to me. Will need to think of something.

Michael2109 commented 6 years ago

I'm a bit unsure of what to call this. We could call it Value or something. I'm struggling to think of a word for it.

FilipJanitor commented 6 years ago

It is tough decision. How about something like InlineDoBlock? That might be confusing too

Michael2109 commented 6 years ago

Okay how about Block, with Inline and DoBlock? That might be okay?

FilipJanitor commented 6 years ago

That sounds good.

Michael2109 commented 6 years ago

All looks to be working fine. My tests are passing. There seems to be tests failing on the build server with the master branch at the moment that I need to look into.

FilipJanitor commented 6 years ago

Sorry, my VM clock got crazy so that is why my commit looks from the past. Tests seem to pass, all looks great.

Michael2109 commented 6 years ago

I'm going to merge this ignoring the failing build as it is a separate issue.