Enhanced resolver for Macro Expansion #67
Conversation
There was a problem hiding this comment.
Thanks for the macro improvements! Looks good to me. I made a couple minor comments and requested changes below.
Requested changes:
- fix the minor comments
- fix the build tests (build, checks, fmt are failing so far)
- update
make test(did the test data on themacro_testpackage change at all with these changes)? - If not, can you add additional macro examples to
macro_testwhich utilize the features you added? I see thatmacro_testhasn't changed since the last PR.
Thanks! Caleb
r? @lzoghbi ?
src/effect.rs
Outdated
| is_unsafe: bool, | ||
| ffi: Option<CanonicalPath>, | ||
| sinks: &HashSet<IdentPath>, | ||
| macro_loc: Option<SrcLoc>, |
There was a problem hiding this comment.
add a small doc to this like: overwrites the source location if provided
There was a problem hiding this comment.
(I guess filepath and callsite are not used in the macro case?)
There was a problem hiding this comment.
honestly it would possibly be better to have different constructors for different cases, the effect model design for pub fn new_call(...) is a bit of a mess right now. But this isn't something we have to fix right now.
There was a problem hiding this comment.
I just tried to split this into normal and macro cases for now
There was a problem hiding this comment.
@puguo can you add a doc to what this parameter does?
like: overwrites the source location if provided
|
Hi @puguo - hope you are doing well! What is the status of this PR? Thanks! |
Hey Caleb, thanks for checking! I tried some fix in June but wasn't able to finish it, and then I got busy with my internship, will resume working on this after my internship ends |
|
@puguo Sounds good, thanks! :) |
2 participants
This PR updates resolver to allow all effect types to be included in the expanded macro call
File changed
I modified resolver to accept HIRFileId instead of regular FileId, which allows macro call ids to be passed in, this allows us to retrieve the cached expansion in RA database.
I also modified macro_expansion logic and added an offset mapping file to ensure syn and Rust Analyzer can share the cached expansion.