-
Notifications
You must be signed in to change notification settings - Fork 79
[new-hs-indexer #6] Workarounds to handle #included source files #521
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
✅ Deploy Preview for fb-oss-glean canceled.
|
|
Happy to review but not sure I have enough context here. Happy to also test in phabricator and approve if you think that's safe enough. |
|
@CatherineGasnier let me know if there's anything I can do to help. |
|
Ok some context questions I have:
Not sure it really matters for me to understand the details though. Meanwhile I'll go ahead an "import to fbsource" in the hope that it gets tested in phabricator. |
|
@CatherineGasnier has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
In Haskell you can use the C preprocessor by adding Fortunately it's rare to use
The solution is a workaround because it's just ignoring any symbols and references that come from a file that is not the primary source file. The right solution would be to track the other source files correctly. Hope this helps! |
|
@CatherineGasnier note that this PR is stacked on #518 so you probably want to import the earlier PRs in the stack first. |
c2e43f9 to
0620fac
Compare
0620fac to
2c8b0ae
Compare
It's useful to be able to know whether a ref is in an import or export, e.g. for dead code detection, so let's tag refs with an enum to say what kind of ref it is. I changed field names so that the schema change shouldn't trigger a validation failure (I hope).
1. Ignore Names that come from another source file (TODO) 2. Ignore Names that come from the current source file but have a line number outside the valid range. No idea why this happens, I didn't investigate.
2c8b0ae to
2c83bdb
Compare
|
@bochko has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary: Mostly reconstructing sensible source file paths. See the comments. With this change I can index all of Hackage (2900+ packages). Note: stacked on #521 Pull Request resolved: #522 Test Plan: Imported from GitHub, without a `Test Plan:` line. Rollback Plan: Reviewed By: donsbot Differential Revision: D76544337 Pulled By: bochko fbshipit-source-id: f526e70745343e233743a069cb62ad8e2f79f9af
line number outside the valid range. No idea why this happens, I
didn't investigate.
Note: stacked on #518