Make some of the ast_util functions into methods #30058
Comments
|
Is this up for grabs? I'd like to start contributing to Rust. |
|
I presume I'd need to update all usage across the codebase. |
|
I've just noticed a bunch of the extremely similar functions declared in here what's up with that? |
|
@faineance These are part of the HIR, they should probably be made into methods as well |
|
What do you think about refactoring them into traits? |
|
I'd prefer plain methods, since that allows to change the HIR without changing the trait or AST - which is basically the point of the HIR. It would also have exactly 2 implementors - AST and HIR - and probably never gain more than that, which makes it a bit useless. |
|
Good point, thanks. |
|
Yes, this is up for grabs. Feel free to work on it! The HIR and AST should stay separate as they will slowly diverge over time. |
Issue: #30058 Updated for: - Stmt - BinOp_ - UnOp - UintTy, IntTy and FloatTy - Lit - Generics A possible inconsistancy? The `Stmt` methods are on the spanned varient: ```rust pub type Stmt = Spanned<Stmt_>; impl Stmt { pub fn id(s: &Stmt) -> Option<NodeId> { match s.node { StmtDecl(_, id) => Some(id), StmtExpr(_, id) => Some(id), StmtSemi(_, id) => Some(id), StmtMac(..) => None, } } } ``` Whilst the methods for BinOp are on the non spanned version. ````rust impl BinOp_ { pub fn to_string(op: BinOp_) -> &'static str { ... } pub fn lazy(b: BinOp_) -> bool { ... } pub fn is_shift(b: BinOp_) -> bool { ... } pub fn is_comparison(b: BinOp_) -> bool { ... } /// Returns `true` if the binary operator takes its arguments by value pub fn is_by_value(b: BinOp_) -> bool { ... } } pub type BinOp = Spanned<BinOp_>; ```` r? @Manishearth
https://github.com/rust-lang/rust/blob/master/src/libsyntax/ast_util.rs
Things like
should instead be
The same goes for many other functions that take their first argument as an AST type (BinOp, Stmt, IntTy, Path, Ident, etc. Similarly, functions like these could be
Generics::empty()instead.(Feel free to keep it in the same file or move it to ast.rs)
The text was updated successfully, but these errors were encountered: