Parse left- and right-assoc #2252

ttuegel · 2020-11-06T22:33:32Z

Reviewer checklist

Test coverage: stack test --coverage
Public API documentation: stack haddock

Now with better error messages!

The new name aligns with other modules in kore and megaparsec.

ana-pantilie

The parser looks much cleaner to me now. I'm also wondering why the sentence type used to take a type variable in the first place. I have some questions but they're mostly for my own understanding, and I also have a couple of minor suggestions.

ana-pantilie · 2020-11-11T17:23:06Z

kore/src/Kore/Parser/Lexer.hs

-sortIdParser :: Parser Id
-sortIdParser = idParser
+parseSortId :: Parser Id
+parseSortId = parseId <?> "sort identifier"


Sorry, I'm not very familiar with <?>. From the documentation, I see it says that <?> replaces the parsed token with "sort identifier" if it fails to parse any input. Does this mean that "sort identifier" will be part of the error message?

Yes, the error message will be:

unexpected ... expected sort identifier

kore/src/Kore/Parser/Parser.hs

ana-pantilie · 2020-11-12T10:19:48Z

kore/src/Kore/Repl/Parser.hs

-type Parser = Parsec String String
+type Parser = Parsec ReplParseError String
+
+newtype ReplParseError = ReplParseError { unReplParseError :: String }


What are the benefits of using a newtype with an IsString instance here rather than String directly?

When we use String directly, we have to declare an orphan instance.

Oh, that's right, we needed the ShowErrorComponent instance.

ana-pantilie · 2020-11-12T10:24:32Z

kore/src/Kore/Syntax/Pattern.hs

+        (Pattern variable Attribute.Null)
+  where
+    from = Recursive.embed . (:<) Attribute.Null . from
+    {-# INLINE CONLIKE from #-}


I don't think I fully understand what CONLIKE does from the GHC documentation, I'll keep in mind to try to learn more about it, but could you give me some hints related to why it was necessary here? Did something not work properly or is there something specific about this code which makes a pragma like this necessary?

CONLIKE is short for "constructor-like". This tells GHC that this function is inexpensive to compute, like a constructor. This will make the compiler very aggressive about inlining it. I'm using it here because I know that all these functions put together are just applying several constructors on top of the argument.

I understand now, thank you!

Co-authored-by: ana-pantilie <[email protected]>

ttuegel · 2020-11-12T21:19:23Z

I'm also wondering why the sentence type used to take a type variable in the first place.

It was due to the class AsSentence, which I removed. We needed all the branches of the Sentence sum type to have the same parameters because there was once a functional dependency on this class. Without the functional dependency, we would need a type annotation in every place that asSentence was used. I was able to replace it with From, and I only had to add two or three type applications to make everything typecheck.

MirceaS · 2020-11-12T16:46:50Z

kore/src/Kore/Parser/ParserUtils.hs

-takeWhile :: (Char -> Bool) -> Parser String
-takeWhile = takeWhileP Nothing
-
-{-|'endOfInput' is similar to Attoparsec's 'endOfInput'. It matches only the
-end-of-input position.
-}
-endOfInput :: Parser ()
-endOfInput = eof
-
-instance ShowErrorComponent String where
-    showErrorComponent str = str


Why are we removing these utilities? (along with the rest from this file)

They are all either re-exporting a library function under a non-standard name, or they are redefining a library function.

ttuegel force-pushed the feature--assoc-parser branch from a3c1247 to 7e0553f Compare November 10, 2020 18:40

ttuegel and others added 22 commits November 10, 2020 12:43

Clean up parser

2885d92

Now with better error messages!

Parse left-assoc

3549dbc

parseSortId: add description for errors

dae6219

parseSort: Use monadic style

d5cce6b

parseSymbolId: add description for errors

5dee042

Kore.Parser.Parser: Clean up

6fa366c

parseEntryType: Generalize parser type

c8f5cfa

Remove orphan instances for parser error types

aff8ee4

ParserUtils: Remove endOfInput

c407fee

keywordEndParser: Use notFollowedBy

5fa25f3

Remove Kore.Parser.Lexeme.keywordBasedParsers

a5ff3d1

Kore.Parser.Lexeme: Use standard names

59d9a78

mv Kore.Parser.Lexeme Kore.Parser.Lexer

1f6c887

The new name aligns with other modules in kore and megaparsec.

Sentence: Add Injection instances

9716e64

Parser: Use Injection instances

233a8fd

Kore.Syntax.Sentence: Remove unused type parameters

4855f90

GlobalMain: Fix Lexer.parseModuleName import

cfe6825

Kore.Parser.Parser: De-sugar \right-assoc

b6d0858

PatternF: add From instances

6497673

Pattern: add From instances

c6e272d

Kore.Parser.Parser: use From instances

f108d9d

kore/package.yaml: extra-source-files: remove test/resources

06bf285

ttuegel force-pushed the feature--assoc-parser branch from 7e0553f to 06bf285 Compare November 10, 2020 18:50

ttuegel marked this pull request as ready for review November 10, 2020 18:59

ttuegel requested review from ana-pantilie and MirceaS November 10, 2020 18:59

ttuegel added 2 commits November 10, 2020 14:47

Remove lint

a787301

test/parser: make golden

fb75961

ana-pantilie approved these changes Nov 12, 2020

View reviewed changes

ttuegel and others added 2 commits November 12, 2020 15:15

Update kore/src/Kore/Parser/Parser.hs

e0a6848

Co-authored-by: ana-pantilie <[email protected]>

parseKoreRemainder: add \right-assoc to documentation

aeb7fb1

Merge branch 'master' into feature--assoc-parser

09b1557

MirceaS approved these changes Nov 12, 2020

View reviewed changes

Merge branch 'master' into feature--assoc-parser

e623afe

ttuegel merged commit 7579c77 into runtimeverification:master Nov 13, 2020

ttuegel deleted the feature--assoc-parser branch November 13, 2020 11:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Parse left- and right-assoc #2252

Parse left- and right-assoc #2252

Uh oh!

ttuegel commented Nov 6, 2020

Uh oh!

ana-pantilie left a comment •

edited

Loading

Uh oh!

ana-pantilie Nov 11, 2020

Uh oh!

ttuegel Nov 12, 2020

Uh oh!

Uh oh!

Uh oh!

ana-pantilie Nov 12, 2020

Uh oh!

ttuegel Nov 12, 2020

Uh oh!

ana-pantilie Nov 13, 2020

Uh oh!

ana-pantilie Nov 12, 2020

Uh oh!

ttuegel Nov 12, 2020

Uh oh!

ana-pantilie Nov 13, 2020

Uh oh!

ttuegel commented Nov 12, 2020

Uh oh!

MirceaS Nov 12, 2020

Uh oh!

ttuegel Nov 12, 2020

Uh oh!

Uh oh!

Parse left- and right-assoc #2252

Parse left- and right-assoc #2252

Uh oh!

Conversation

ttuegel commented Nov 6, 2020

Reviewer checklist

Uh oh!

ana-pantilie left a comment • 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!

Uh oh!

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!

ttuegel commented Nov 12, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ana-pantilie left a comment •

edited

Loading