Getting rid of warnings in PrettyPrinter.hs

[EliasC]

This commit gets rid of the warnings given by missing cases in the
pretty-printer. It had just been bugging me for ages.

[albertnetymk]

On the master branch, I don't get any warning for clean build.

[EliasC]

Here is what I get from a clean build on master

bash-3.2$ make
...
[ 9 of 36] Compiling AST.PrettyPrinter ( src/ir/AST/PrettyPrinter.hs, dist/build/encorec/encorec-tmp/AST/PrettyPrinter.o )

src/ir/AST/PrettyPrinter.hs:66:1: Warning:
    Pattern match(es) are non-exhaustive
    In an equation for ‘ppQName’: Patterns not matched: []

src/ir/AST/PrettyPrinter.hs:85:1: Warning:
    Pattern match(es) are non-exhaustive
    In an equation for ‘ppImportDecl’:
        Patterns not matched: PulledImport _ _ _ _

src/ir/AST/PrettyPrinter.hs:141:1: Warning:
    Pattern match(es) are non-exhaustive
    In an equation for ‘ppExpr’:
        Patterns not matched:
            Foreach _ _ _ _
            FinishAsync _ _

src/ir/AST/PrettyPrinter.hs:209:1: Warning:
    Pattern match(es) are non-exhaustive
    In an equation for ‘ppUnary’:
        Patterns not matched:
            AND
            OR
            Identifiers.LT
            Identifiers.GT
            ...

src/ir/AST/PrettyPrinter.hs:212:1: Warning:
    Pattern match(es) are non-exhaustive
    In an equation for ‘ppBinop’: Patterns not matched: NOT
...

[albertnetymk]

This is so odd. BTW, the branching you are working on, https://github.com/EliasC/encore/tree/fix/ppWarnings, is 6 commits behind encore/master.

[EliasC]

Thanks! I had forgotten to update my own master branch! In this branch I don't see the warnings either, which is actually weird. For example there should be a warning for the following function:

ppQName :: QName -> Doc
ppQName [a] = ppName a 
ppQName (a:as) = ppName a <+> text "." <+> ppQName as

Do you know if one of the latest commits disables any warnings?

Still, I think the refactoring in this PR makes sense. Let me know if you would like me to reword it.

[albertnetymk]

The non-exhaustive thing in this particular file, if happens in runtime, it indicates a bug from the compiler developers. IOW, we know for sure it would not happen, so I disabled it, I think. For the refactoring you can create a PR now or you could attach this refactoring along with some other change. Either way makes sense.

[EliasC]

I added the warning there so that you could get a warning at compile time instead of at runtime! The idea was that if you add something to the AST you should get warnings that help you find all the places you need to update. By disabling it we require that all programmers know how the whole compiler works. I would like to re-add the warning (and for future warnings to not be disabled unless there is a clear commit message highlighting and motivating why that warning should be disabled).

[albertnetymk]

That's a fair argument. I failed to anticipate the evolving of the AST. Thanks.

[EliasC]

@albertnetymk I added the warning after the fifth person came to me asking why their new addition to the AST didn't work properly :)

[albertnetymk]

What?! I thought there are totally four (if I count myself) persons working on this. Besides, the runtime error should have occurred, shouldn't it?

[EliasC]

(You, me, Kiko, Stephan, Tobias, Dave, plus a bunch of students)

Not literally the "fifth" person. I should have said "there were reoccurring errors stemming from people (including myself) forgetting to extend PrettyPrinter.hs, so I added the warning to help everyone out". If I recall correctly there was a catch-all case, so there were no runtime errors.

[albertnetymk]

This would surpass any compiling warning, even if a new node is added, right?

[EliasC]

That's a fair point. Fixed in the latest commit.

[albertnetymk]

The intention of this PR is possibly changed, so you may want to rebase. There's support for git for trailing spaces, here. These are minor things; if you don't plan to do any future update, I think it's ready to be merged.

[EliasC]

Pushed a last minor layout fix. I'm done with this PR now.

[albertnetymk]

Do you think if it's good to do a rebase to squeeze/rename some of commits before merging?

[EliasC]

Just realized I based the squash on an unmerged PR... Hold on...

[EliasC]

There! Squashed, reworded and ready for merge.