chore(docs): Adds graph debug documentation to book by afreeland · Pull Request #379 · maciejhirsz/logos

afreeland · 2024-02-19T17:04:04Z

I ran across this comment while having some issues with regex support. Thought this was beneficial information and wanted to try and include it in the book so it was a little more visible to people that may be trying to debug and understand how things are working

jeertmans

Hey! Thank you some much for your contribution! I left a few review comments that I'd like you to address before we can merge :-)

Feel free to accept or reject my suggestions!

jeertmans · 2024-02-19T19:46:27Z

book/src/debug-graph.md

+# Debug Graph
+
+It may be beneficial during debugging to be able to visualize how Logos has generated its graph. 


Maybe add some introduction to the fact that Logos construct a graph internally to generate its code. Here, this comes a bit out of nowhere :-)

jeertmans · 2024-02-19T19:47:49Z

book/src/SUMMARY.md

 + [Using `Extras`](./extras.md)
 + [Using callbacks](./callbacks.md)
 + [Common regular expressions](./common-regex.md)
+ [Debug Graph](./debug-graph.md)


I'd maybe add just a Debugging section, and, in the debug.md file, have one section talking about the graph. So we can later add some other sections, e.g., to talk about regex-syntax's HIR or else.

jeertmans · 2024-02-19T19:49:50Z

book/src/debug-graph.md

+}
+```
+
+With our graph debugging enabled, we see the following output:


Suggested change

With our graph debugging enabled, we see the following output:

Logos actually constructs a graph that contains the logic for matching tokens:

jeertmans · 2024-02-19T20:00:13Z

book/src/debug-graph.md

+}
+```
+
+Lets look at `@9` for the character `.` we see that it will jump `=>` to `2`.  We can then follow that by looking at `@2` which resolves to our `::Period` token. It will then continue to look for any matches in the case there is potential continuation after the `.` character. In the _input_ we provided there is not, since it is the end of our input.


Suggested change

Lets look at `@9` for the character `.` we see that it will jump `=>` to `2`. We can then follow that by looking at `@2` which resolves to our `::Period` token. It will then continue to look for any matches in the case there is potential continuation after the `.` character. In the _input_ we provided there is not, since it is the end of our input.

This graph can help us understand how our patterns are matched, and maybe understand why we have a bug at some point.

Let's look at the last node, `9`, for the character `.`. We see that it will jump (`=>`) to `2`. We can then follow that by looking at `2` which resolves to our `::Period` token. It will then continue to look for any matches in the case there is potential continuation after the `.` character. In the _input_ we provided, there is not, since it is the end of our input.

Note that I have to read a bit more of the code to feel confident about how Graph actually works.

Also, this may be a bit counter-intuitive to take the example of ., where this is the last bit of our input string.

The thing I was to know (I guess the information is somewhere in the source code) is whether we always enter the graph via its last entry, or not.

Totally agree about it being counter-intuitive about the . example. I used the . since it seemed a little more straight forward on the graph to understand. Could probably smooth over that some if I reworked some of the wording to lead into a bit more, instead of BLAM, understand this lol

jeertmans · 2024-02-19T20:02:18Z

book/src/debug-graph.md

+
+Lets look at `@9` for the character `.` we see that it will jump `=>` to `2`.  We can then follow that by looking at `@2` which resolves to our `::Period` token. It will then continue to look for any matches in the case there is potential continuation after the `.` character. In the _input_ we provided there is not, since it is the end of our input.
+
+We also can try to identify how `fast` works by looking at `@9` that `f` jumps to `7`. This will then resolve the last letters of our word _fast_ by matching `ast` which jumps to `8`. Since our provided _input_ to the lexer does not include alphanumeric character after the word "fast" but rather whitespace, the token `::Fast` will be recognized. Then the graph will look for further potential continuation (ie `[g-z] => 4`)


Suggested change

We also can try to identify how `fast` works by looking at `@9` that `f` jumps to `7`. This will then resolve the last letters of our word _fast_ by matching `ast` which jumps to `8`. Since our provided _input_ to the lexer does not include alphanumeric character after the word "fast" but rather whitespace, the token `::Fast` will be recognized. Then the graph will look for further potential continuation (ie `[g-z] => 4`)

We also can try to identify how `fast` works by looking at `9`, first, and seeing that `f` makes it jump to `7`. This will then resolve the last letters of our word _fast_ by matching `ast` which jumps to `8`. Since our provided _input_ to the lexer does not include alphanumeric character after the word "fast" but rather whitespace, the token `::Fast` will be recognized. Then the graph will look for further potential continuation (i.e., `[g-z] => 4`)

book/src/debug-graph.md

afreeland · 2024-02-20T02:35:01Z

Yeah, sorry for the kind of rushed PR...trying to sneak this in-between family time lol

Made some improvements to language and file structure based off of your recommendations. Hopefully, a little less jarring to someone reading it.

As for the enable debugging part, I totally agree, I am new to rust but I can try to put something together that properly uses a debug-mode kind of flag. Will keep it as a separate commit or could be a follow up PR if I get a chance. Would make life much nicer if something was an easy feature bit to flip.

This adds a debug feature, similar to what Clap does, as requested by #379. The current implementation only debugs a few parts of the code, and it may be interesting to add more debug messages in the different crates.

jeertmans · 2024-02-20T09:40:45Z

Yeah, sorry for the kind of rushed PR...trying to sneak this in-between family time lol

Made some improvements to language and file structure based off of your recommendations. Hopefully, a little less jarring to someone reading it.

As for the enable debugging part, I totally agree, I am new to rust but I can try to put something together that properly uses a debug-mode kind of flag. Will keep it as a separate commit or could be a follow up PR if I get a chance. Would make life much nicer if something was an easy feature bit to flip.

No issue, I did it myself since this was an easy fix. The issue you had was that you needed to declare a debug feature in sub-crates. See #382 for more details :-)

* chore(lib): add debug feature This adds a debug feature, similar to what Clap does, as requested by #379. The current implementation only debugs a few parts of the code, and it may be interesting to add more debug messages in the different crates. * chore(lint): run cargo fmt

jeertmans

Nice work @afreeland! I left some more comments (sorry to be picky about some ^^'), but I hope you don't mind..

Note that #382 was merged and just added a new debug feature.

book/src/debugging.md

Co-authored-by: João Marcos <marcospb19@hotmail.com>

abhillman · 2024-06-02T23:18:08Z

Users wishing to use this functionality may do so, pending this patch making it upstream to crates.io, with the following:

logos = { git = "https://github.com/maciejhirsz/logos.git#afreeland:debug-graph", features = ["debug"]}

abhillman · 2024-06-02T23:32:42Z

I haven't had a chance to dive in yet, but does logos have a runtime-accessible public API for fetching, manipulating, and constructing graphs exist besides using macros? Some reasons I can think of why those APIs would be useful:

fetching [related to this PR]

if a project contains multiple lexers, being able to fetch a graph and print could be easier for users than printing all graphs, as is done in this PR
in many cases, such an API would enable a developer to easily use the logos macro DSL to produce a lexer file for other tools (flex and its many derivations, for example)

manipulating/construction [out of scope for this PR, yet topically related]:

although this may require additional changes, this would allow for producing lexers at runtime

What this PR might look like if there is/were a fetch API

The documentation would mention something like this:

let graph = Token::graph();
println!("{:#?}", graph)

Continuing to use a feature flag to enable ::graph() (if something like it doesn't already exist) would still also probably make sense.

Thoughts?

jeertmans · 2024-06-03T07:54:09Z

I haven't had a chance to dive in yet, but does logos have a runtime-accessible public API for fetching, manipulating, and constructing graphs exist besides using macros? Some reasons I can think of why those APIs would be useful:

fetching [related to this PR]

if a project contains multiple lexers, being able to fetch a graph and print could be easier for users than printing all graphs, as is done in this PR

in many cases, such an API would enable a developer to easily use the logos macro DSL to produce a lexer file for other tools (flex and its many derivations, for example)

manipulating/construction [out of scope for this PR, yet topically related]:

although this may require additional changes, this would allow for producing lexers at runtime

What this PR might look like if there is/were a fetch API

The documentation would mention something like this:
let graph = Token::graph();
println!("{:#?}", graph)
Continuing to use a feature flag to enable ::graph() (if something like it doesn't already exist) would still also probably make sense.

Thoughts?

This is true that is might be nice to expose that as part of the public API. Would you mind creating an issue for that feature request?

jeertmans · 2024-02-24T21:26:35Z

logos-codegen/src/generator/mod.rs

+        #[cfg(feature = "debug")]
+        dbg!(graph);
+


I think you don't need this, because I already added a debug print of the graph in another place

See here:

logos/logos-codegen/src/lib.rs

Line 298 in 066e125

debug!("Generating code from graph: {graph:#?}");

Hey @afreeland! Could you address this review please? So I can accept your PR and merge it :-)

jeertmans added the book Related to the book label Feb 19, 2024

jeertmans requested changes Feb 19, 2024

View reviewed changes

jeertmans mentioned this pull request Feb 20, 2024

chore(lib): add debug feature #382

Merged

jeertmans requested changes Feb 20, 2024

View reviewed changes

marcospb19 reviewed Feb 21, 2024

View reviewed changes

book/src/debugging.md Outdated Show resolved Hide resolved

afreeland force-pushed the debug-graph branch from f23af3f to 39a91fd Compare February 24, 2024 17:11

Add debug support and documentation for internal graph output

7ce0526

afreeland force-pushed the debug-graph branch from 39a91fd to 7ce0526 Compare February 24, 2024 17:17

Refine enabling feature

f342865

marcospb19 reviewed Feb 28, 2024

View reviewed changes

book/src/debugging.md Outdated Show resolved Hide resolved

Update book/src/debugging.md

ae18909

Co-authored-by: João Marcos <marcospb19@hotmail.com>

jeertmans mentioned this pull request Jun 2, 2024

Repeated Group Fails #394

Open

jeertmans changed the title ~~Adds graph debug documentation to book~~ chore(docs): Adds graph debug documentation to book Jun 10, 2024

chore(docs): add required changes

64b1e7f

jeertmans approved these changes Jun 10, 2024

View reviewed changes

jeertmans merged commit 640882a into maciejhirsz:master Jun 10, 2024

eikopf mentioned this pull request Jul 29, 2024

"Debug" Feature not Found #408

Closed

		# Debug Graph

		It may be beneficial during debugging to be able to visualize how Logos has generated its graph.

	With our graph debugging enabled, we see the following output:
	Logos actually constructs a graph that contains the logic for matching tokens:

-Lets look at `@9` for the character `.` we see that it will jump `=>` to `2`.  We can then follow that by looking at `@2` which resolves to our `::Period` token. It will then continue to look for any matches in the case there is potential continuation after the `.` character. In the _input_ we provided there is not, since it is the end of our input.
+This graph can help us understand how our patterns are matched, and maybe understand why we have a bug at some point.
+Let's look at the last node, `9`, for the character `.`. We see that it will jump (`=>`) to `2`.  We can then follow that by looking at `2` which resolves to our `::Period` token. It will then continue to look for any matches in the case there is potential continuation after the `.` character. In the _input_ we provided, there is not, since it is the end of our input.


		Lets look at `@9` for the character `.` we see that it will jump `=>` to `2`. We can then follow that by looking at `@2` which resolves to our `::Period` token. It will then continue to look for any matches in the case there is potential continuation after the `.` character. In the _input_ we provided there is not, since it is the end of our input.

		We also can try to identify how `fast` works by looking at `@9` that `f` jumps to `7`. This will then resolve the last letters of our word _fast_ by matching `ast` which jumps to `8`. Since our provided _input_ to the lexer does not include alphanumeric character after the word "fast" but rather whitespace, the token `::Fast` will be recognized. Then the graph will look for further potential continuation (ie `[g-z] => 4`)

Uh oh!

Conversation

afreeland commented Feb 19, 2024

Uh oh!

jeertmans 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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

afreeland commented Feb 20, 2024

Uh oh!

jeertmans commented Feb 20, 2024

Uh oh!

jeertmans left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

abhillman commented Jun 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

abhillman commented Jun 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeertmans commented Jun 3, 2024

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!

4 participants

abhillman commented Jun 2, 2024 •

edited

Loading

abhillman commented Jun 2, 2024 •

edited

Loading