-
Notifications
You must be signed in to change notification settings - Fork 1
Rustup + The Great Refactoring #36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Main changes: * Updated Rust version * Split up the code into more modules * Removed ast_map.rs, which was a copy of part of librustc/front/map/mod.rs. The only difference with the original map was removing a `TypedArena` from the Forest struct Todo: * Fix lifetime walker, since `visit_opt_lifetime_ref` doesn't exist anymore (see rust-lang/rust#28715)
UPDATE 2: the crashes seem to be related to the Windows issue described in the readme. I am going to install Ubuntu to check if it works and eventually continue development there. UPDATE: I am now able to run the tests (switching to the msvc ABI solved it)... However, they don't pass after my changes. I had expected everything to succeed, except lifetime elision and reification. I'll try to fix it, but I still don't know what to do about the lifetime walker. @nrc I was unable to run the tests because of a strange error (see below). Would you mind to run the tests yourself? I expect the lifetime related refactorings to fail, but the others should be ok (I hope so, after such a huge refactoring). BTW, do you think this is a rustc bug? Running tests works normally in other projects... |
@aochagavia did you have any success? Sorry I haven't looked at this - I just got back from vacation. I'll try and take a look this week. |
@nrc The tests compile and run now in Ubuntu, but they fail. The problem is that the compiler doesn't link the correct libraries when we call it from the refactorer. The instructions in the readme haven't helped me so far and I have run out of time, so I think I will wait until Frebruary to try again. I hope I will be able to make the tool easier to set up, since the current procedure seems very cumbersome and could make adoption difficult. BTW:
|
@GSam - did you ever have this problem running tests? Are there paths you need to set before running? @aochagavia why do you use the HIR rather than the AST? In general, you should prefer the AST since that is an interface for tools, but the HIR is compiler-internal. If there is something we can fix in Rust, we should do that rather than change to using the HIR here. |
@nrc I moved stuff to the HIR just to get things to compile again. I guess I could migrate it back once I get the refactorer working. |
We should definitely do that and be aware of how much effort it would be (if we need to fix stuff on the compiler side, in particular). One thing that would be nice to do is to use rustfmt for the output, and that requires AST, rather than HIR. |
Would it be desirable to use syntex instead of the AST? |
The only thing you should need to set is the RUST_FOLDER environment variable which needs to point to the lib folder for Rust (either stage2/lib or stage2/lib/rustlib/[some environment]/lib). I can't really say much more unless there's a specific error message that you see. |
@aochagavia we could. The only benefit of Syntex is that it allows us to work on stable Rust. However, since the tool interacts with other parts of the compiler, I don't think we can be stable anyway. It might be worth looking at later on. In the short term, I don't think it matters too much one way or the other. @GSam thanks! |
@GSam So all I need is building a stage2 compiler, compiling the tool itself using it and setting the right environment variables? I am going to try it out, thanks! |
@aochagavia It should probably still work with a regular system install (pointing to some other lib folder), but it might have its own issues to be looked into. Try it and see how it goes. |
I had a quick look through this and it (other than the HIR thing) these all seem like reasonable changes. |
Main changes:
librustc/front/map/mod.rs. The only difference with the original map was
removing a
TypedArena
from the Forest structTodo:
visit_opt_lifetime_ref
doesn't existanymore (see Fill in some missing parts in the default AST and HIR visitors rust-lang/rust#28715)