Comments (12)
I really like this output it makes it more in like with how -fdump-tree-gimple looks also
from gccrs.
There's two main comments I'd like to make here:
-
Some of the non-dump-related code uses the as_string() method to convert some things into string form, such as paths (e.g. converting
something::something_else
to "something::something_else"). As such, if you convert all object dumping processes to output the same form, either the code requesting a string form would have to be changed (i.e. new method), or you might have to add a new method liketo_dump_string()
or something. -
The main issue that I experienced with creating a nice-looking dump is that indentation levels are pretty much impossible to create with the current system of recursive
as_string()
calls, as the called object has no conception of the indentation level of the parent, which prevents any indentation from being preserved if the called object adds a new line to the string. You'd probably have to pretty heavily modify the dumping mechanism to do this - the best solution that I can currently think of would be a separate "Dump Creator" object that holds state on the current indentation level and the string so far, and a theoreticaldump_string()
method could add new lines to that object, and then the final string could be extracted from it at the end. This would probably be a lot of work and may involve unforeseen issues, however.
There's also one minor nitpick with the current example you have shown - strictly speaking, BlockExpr
includes the curly braces surrounding it (as that is how BlockExpr is defined).
from gccrs.
@SimplyTheOther
I don't expect to make it perfect in a short time. Actually, I think we need a more readable output to help us to debug the very beginning syntax parsing, and this may help to attract more contributions. So we may improve it gradually. Of course, we could also rewrite the AST (iff it's not good enough) in the future for better dumping work. If the indentation is finally proved to be a problem, we may have to use an external indent tool to help us. After all, it's just for the very beginning debugging.
Thanks for the mention. I haven't read BlockExpr
part. I only tried to improve the function and parameters for a prove of concept. I'll keep updating it.
from gccrs.
BTW, if you're going to create a PR for indentation, please consider merging this branch first to avoid too many conflicts, then indent the whole code. I'm sorry it's still half baked, but I don't think it will break existing things at least.
However, here's a thing I have to mention. I've patched the string literal token to add double-quote to the value. I think this should be done when lexing the string literal. Otherwise, there are no double quotes around string when we print them, say println!(Hello World!)
from gccrs.
I've updated for indentation, so far so good. However, the global variable has to be used since the current AST doesn't support the indentation well. But I think it's fine. We don't have to spend time optimizing AST dump. It's just for debugging support. Let's save time for type checking and IR transformation.
Crate:
inner attributes: none
items:
u32 abc(x : u32, y : u32)
BlockExpr:
{
outer attributes: none
inner attributes: none
statements:
ExprStmtWithoutBlock: return ArithmeticOrLogicalExpr: x + y
final expression: none
}
void main()
BlockExpr:
{
outer attributes: none
inner attributes: none
statements:
ExprStmtWithBlock:
BlockExpr:
{
outer attributes: none
inner attributes: none
statements:
ExprStmtWithoutBlock: ArithmeticOrLogicalExpr: 1 + 1
final expression: none
}
ExprStmtWithoutBlock: println!("Hello World!")
final expression: none
}
from gccrs.
@NalaGinrut @philberty Kind of off-topic, but do you think this is a good template to show others what we’re working on at the moment (i.e. create an issue)? Or is there a better way (e.g. draft pull requests, projects maybe)?
BTW, if you're going to create a PR for indentation, please consider merging this branch first to avoid too many conflicts, then indent the whole code. I'm sorry it's still half baked, but I don't think it will break existing things at least.
However, here's a thing I have to mention. I've patched the string literal token to add double-quote to the value. I think this should be done when lexing the string literal. Otherwise, there are no double quotes around string when we print them, say
println!(Hello World!)
Depending on what exactly you mean by that (i.e. when you add the double quotes - at the lexer stage or later), that might break some of the code comparing the values of string literals (e.g. cfg attribute handling). I think it would be better to store the value of the string value only and then add the double quotes in the as_string() method, if that’s not what you’ve done already (since adding characters is easier than removing them).
from gccrs.
-
If you want to show AST dump to promote gccrs, I think it's at least an explicit and understandable work to common users. It shows the parser work for Rust syntax. Personally, I think it's OK.
The only concern is the time. I don't think it can be complete for every corner case of Rust syntax in a short time. However, it's not good to keep it unmerged for a long time since it should be helpful for our next step development. How about this, I will raise a PR after a few tweaks. Then we can promote gccrs by indicating AST dump, and we can fix the indentation issue after this PR soon. -
Adding necessary double-quotes would be better, however, I didn't find the correct place in AST dump. I've tried some, but it doesn't work. I think it's fine. I can remove the lexer modification, and postpone this tiny fix.
from gccrs.
1. If you want to show AST dump to promote gccrs, I think it's at least an explicit and understandable work to common users. It shows the parser work for Rust syntax. Personally, I think it's OK. The only concern is the time. I don't think it can be complete for every corner case of Rust syntax in a short time. However, it's not good to keep it unmerged for a long time since it should be helpful for our next step development. How about this, I will raise a PR after a few tweaks. Then we can promote gccrs by indicating AST dump, and we can fix the indentation issue after this PR soon.
I see your reasoning. I'm just a little wary about the possibility of constructing a system that is difficult to change afterwards and having to do massive refactoring at some point in the future. But yeah, right now a predominantly working parser is probably more important.
2. Adding necessary double-quotes would be better, however, I didn't find the correct place in AST dump. I've tried some, but it doesn't work. I think it's fine. I can remove the lexer modification, and postpone this tiny fix.
You would add the double quotes in the as_string()
method in Literal
in rust-ast.h
if the literal type is a string (so you'd probably do a switch, since characters should also have single quotes too then). I would then also add a raw_value()
method (or some other name) that returns the raw value_as_string
so that comparisons with raw value don't have to strip the quotes off.
from gccrs.
-
Yes, although we can rethink many things at this stage, there're a lot of other works to do. I think we can polish in the future. If you were asking about the WIP template, I think the answer is yes. For larger work, it's better to raise an issue with WIP, and contributors may follow the related branch. I never used
project
, I don't know if it's proper for the case. -
For the string literal part, I have removed my modification in the PR. I may need some time to figure it out. We don't have to fix it in the same PR.
from gccrs.
Oh, I just tried projects
, there's kanban, and we can drag related issues in. I think we can take advantage of it.
from gccrs.
I think PR's should be for things you want to get merged into Master when you think they are ready, otherwise in my experience PR's have a tendency to be neglected by reviewers.
For distributing work i started to create a github kanban board i am not sure if you guys have access to create/update on there if not i will take a look at fix that for you guys. I think we should use itas much as we can if a piece of work is of bigger scope or are not sure about putting on the kanban board lets create an issue to discuss it.
What do you think? I am happy to change things but lets try this for now for a few weeks.
from gccrs.
@philberty I think the Github kanban can be based on issues, say, you can just reference the issue number with a hash to reference, you don't even have to write any description. Then we can connect kanban and issues together.
However, if the new feature is working by just one person, the kanban could be ignored, the issue is enough. But if we want to attract more people to work together on the same sub-project, the kanban is still important.
So I think, at least in this stage, maybe kanban is not necessary, depends on you whether to use it. Personally, I'm glad to try. But issues should play an important role in any time.
from gccrs.
Related Issues (20)
- Prevent `#[may_dangle]` attribute from being used in safe impl
- ICE when on invalid-code when construction enum
- Do not link with `-ldl` and `-lpthread` unless necessary
- Handle outer attributes properly for inline assembly
- Improve `-ldl -lpthread` checks in `./configure`
- Safe guard InlineAsm-related structs
- Remove unnecessary #include from rust-expr.h HOT 2
- Fully incorporate tl::expected into InlineAsm parsing
- asm! macro failed to exhaustively parse all of options(), clobber_abis() and register operands
- Typechecking of asm! failed in `let _` HOT 1
- Add ExprType::InlineAsm variant to ExprType enum HOT 1
- Cleanup vector copies from Rust to C++ in BIR HOT 2
- Add special `LangItemPath` AST node
- Make asm parser stores parse result
- Union pattern matching expecting all fields
- asm parser lacking `label` parse functionality
- Unable to find associated `type` in `trait` HOT 1
- ICE in retrieving ambiguous associated type - `E0223`
- `include_str!(file!())` does not work on godbolt HOT 2
- Test proc macros
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from gccrs.