Coding Conventions

[kaeluka]

This commit expands the coding conventions and implements part of them
for the whole code base.

[kaeluka]

Making this commit was possible only due to the combined brilliance of several emacs features. I'll so use this as a war story in the IOOPM lab.

[albertnetymk]

1. I am more leaned towards to fixing style violations while working on a real change. More
2. Since the runtime uses snake, it would take two searches to find all places encore_create is used, one for runtime using snake, the other for the compiler using encoreCreate after this PR. Currently, it requires one, for CCodeName module, one wrapper over runtime API, defines functions with API as their prefix.
3. Enforcing code style even for local functions/variables is too demanding, IMO.
4. If we really go for this full-automatic conversion, why is camel chosen over snake? snake seems a more sensible choice, for the runtime is using snake, and it's part of the repo.

[kikofernandez]

any other tool apart from hlint that help us keep the style?

[kaeluka]

kiko: we could be using a tool, but this would become a de-facto dependency (it'd be practically impossible to write "legal" code without the tool)

[kaeluka]

I am more leaned towards to fixing style violations while working on a real change.

We decided not to do that this time.

As for the other points: a majority wanted camel case. It also does not matter what it looks like, it matters that it is consistent.

[EliasC]

Since the runtime uses snake, it would take two searches to find all places encore_create is used

This argument could easily be reversed to say that in the current situation there would be noise from the C backend if we need to find all places where a call to encore_create is generated in Haskell.

Enforcing code style even for local functions/variables is too demanding, IMO.

I (unsurprisingly) disagree. See our discussion here about coding conventions and local functions. This piece of code shows that we need to respect conventions even in local functions:

          resovle_capa :: Type -> TypecheckM Type
          resovle_capa t
            | emptyCapability t = return t
            | singleCapability t = resolveType $ head $ typesFromCapability t
            | otherwise =
              mapM_ resolveSingleTrait (typesFromCapability ty) >> return ty

          resolveSingleTrait t = do
            result <- asks $ traitLookup t
            when (isNothing result) $
                   tcError $ "Couldn't find trait '" ++ getId t ++ "'"

snake seems a more sensible choice, for the runtime is using snake, and it's part of the repo.

...but the Haskell (standard) libraries are using camelCase, so if we want the Haskell code to be internally consistent, snake_case would not work. But @kaeluka's comment is the most important:

It also does not matter what it looks like, it matters that it is consistent.

[TobiasWrigstad]

I strongly argue that we should keep the PonyRT's coding style for the C code that we generate. (Not sure anyone is arguing against that but it is an important point to make.) It will be very strange otherwise. There is no big gain in enforcing another style on the generated C code as it will (ideally) not be read.

Coding style should naturally be enforced everywhere — what would be the point otherwise?

[EliasC]

@TobiasWrigstad: +1 (but I don't think anyone was arguing for -1)

[TobiasWrigstad]

@albertnetymk Apparently there was a camel case vote and the good guys lost ;) Now we just have to deal.

Different coding convention for the Haskell code we write and the C we (mostly) generate does not seem to be a problem.

For clarification: is there a coding style for the C code part of Encore?

[kaeluka]

I wasn't arguing for that and the PR does not alter the C-code output at all (if you find that to be wrong, that's a bug).

[EliasC]

is there a coding style for the C code part of Encore?

Nothing official, but there should be! And there you can have all the snake_case you want ;)

[TobiasWrigstad]

@EliasC Great. Then I think @albertnetymk and I can hash that one out since that's our preferred level of abstraction ;)

[EliasC]

All variables must be prefixed by _enc__camel_case_sucks_

[TobiasWrigstad]

As if we didn't have too much prefix anyways!