[new-hs-indexer #4] New Haskell Indexer

[simonmar]

Redesigned the schema and rewrote the indexer. Compared with the previous indexer:

• this captures a lot more xrefs (e.g. local variables)
• it has more information (distinguishes functions/classes/constructors etc.)
• it is much simpler and probably faster, because it doesn't go via hiedb, it reads .hie files directly.
• it is probably more correct, I fixed a lot of things.

The schema is carefully designed so that a Name uniquely identifies an entity and corresponds fairly closely to GHC's Name, including OccName. The main difference is we don't store Uniques, instead we distinguish local Names by including their ByteSpan.

There are a couple of snapshot tests, one for the plain indexer and one for the codemarkup layer, and a Glass regression test.

Not done yet:

• we can extract types from the .hie file too, and provide type hovers in Glass. That wouldn't be too hard.

• extracting more structure so that we can reconstruct data/class decls should be possible but it's not straightforward using .hie. I'm still thinking about how best to do that.

[netlify]

✅ Deploy Preview for fb-oss-glean canceled.

Name	Link
🔨 Latest commit	9c72cae
🔍 Latest deploy log	https://app.netlify.com/projects/fb-oss-glean/deploys/6832132ec8befb0008563d57

[pepeiborra]

Something's broken in glass

[facebook-github-bot]

@jjuliamolin has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[jjuliamolin]

Thanks Simon this is great :) I'll have a look at this one, will need to fix some internal dependencies

[simonmar]

@jjuliamolin you probably want to re-import, I just pushed a new version of this branch with:

• rebased
• added Glass Symbol ID handling
• fixed Glass search by name + case insensitive search
• added a Glass regression test
• removed the old indexer
• some bug fixes

Note: stacked on #514

[pepeiborra]

It's going to be quite tricky to merge this as it requires changes to internal TARGETS files. Can you fix the CI errors @simonmar and let us know when to reimport?

[facebook-github-bot]

@jjuliamolin has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[jjuliamolin]

I'll import a temporary one to check the internal build targets, will re-import again if there's CI signals to fix here

[simonmar]

Alright, it looks like the failures are:

• GHC 9.2.8: ordering issues in the snapshot output. This should be fixable
• GHC 9.4.7: failure in glass-snapshot-flow, no idea what might be causing this yet. (EDIT: nevermind, I just pushed a fix, hopefully)
• GHC 8.10.7 and earlier: build failures in the indexer due to differences in the GHC API. I might just disable the indexer for these.

[simonmar]

@jjuliamolin @pepeiborra OK, CI is green. I'm sure this is going to be non-trivial to get working with the internal build system so no worries if it takes a while! I'm not in a rush to get it landed. I'll stop modifying this PR so that you have something stable to work with and create separate PRs for future changes, although note that I'll have to stack new PRs on top of this one.

[facebook-github-bot]

@jjuliamolin has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[jjuliamolin]

Sorry that this is taking longer than anticipated.. We're close to landing it internally, but I found some backward incompatibilities from the generated schema thrift files:

Module_key in hs.thrift
1: hs.PackageId -> hs.ModuleName
2: hs.ModuleName -> hs.UnitName

SourceModule_key in hs.thrift
1: hs.ModuleName -> src.File
2: src.File -> hs.Module

SearchByName_key search_hs.thrift
2: code_hs.Entity -> hs.Name

Can you take a look @simonmar, maybe we need evolve there, in the angle schemas

Also I noticed there's a bug somewhere that creates build errors for us when Entity = Pred , instead of {p = P }, from code.hs if you want to land faster it might be worth just changing this..

[simonmar]

@jjuliamolin

I found some backward incompatibilities from the generated schema thrift files:

you can ignore errors from the Thrift linter about backward-incompatibilities in the Thrift files generated from the schema, they aren't relevant to Glean because Glean has its own backwards-compatibility handling for schemas. As long as the schema passes the validation check, we're all good.

Also I noticed there's a bug somewhere that creates build errors for us when Entity = Pred , instead of {p = P }, from code.hs if you want to land faster it might be worth just changing this..

could you give more details? What fails and what is the error?

[jjuliamolin]

Ah alright great, wasn't sure what the consequences could be..

types_cpp_splits=1 is misconfigured: it can not be greater than the number of objects, which is 0.

it's coming from the types_cpp_splits in targets file generated by
glean/schema/gen/Glean/Schema/Gen/Thrift.hs

haven't had a closer look, but guess we'll just need to change how we count predicates there

[simonmar]

types_cpp_splits=1 is misconfigured: it can not be greater than the number of objects, which is 0.

it's coming from the types_cpp_splits in targets file generated by glean/schema/gen/Glean/Schema/Gen/Thrift.hs

haven't had a closer look, but guess we'll just need to change how we count predicates there

OK, so something weird with compiling the generated Thrift file to C++, which wouldn't show up in the OSS build because we don't do that.

If this is due to type Entity = hs.Name then I want to change that anyway, we should make it into a union type to allow for future extensions. I'll update the PR shortly.

[simonmar]

@jjuliamolin I fixed that issue, and also rebased the whole stack (and added a couple more commits at the bottom). All the PRs are numbered - e.g. this one is [new-hs-indexer #4] - so you can see which order to import/land them.

[facebook-github-bot]

@jjuliamolin has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[jjuliamolin]

Sorry broke the open source regression tests, had to split them into fb/oss.. maybe you can re-generate them @simonmar, or anyone else. I just don't have the access to the actual open source checkout yet so can't generate with the same result

[simonmar]

@jjuliamolin thanks for landing this! I fixed up the tests in this PR: #530 and also rebased the rest of the stack. I'm curious how the output differs when you run the tests internally, maybe that's something we can fix?

[jjuliamolin]

@simonmar we'll pass the hie files to the binary (not using the --with-ghc option), so one difference that I didn't really managed to get around was the filepath. Also "unit" is different, seems to be based on the build targets, but haven't really looked into this one

[jjuliamolin]

from hsDeclarationLocation.out

...
"file": { "key": "fbcode/glean/lang/haskell/tests/code/A.hs" },
"name": {
"key": {
"mod": {
"key": {
"name": { "key": "A" },
"unit": { "key": "fbcode-glean-lang-haskell-tests-code-library" }
..
library is a build target in tests/code

[simonmar]

@jjuliamolin yeah I see. OK having separate output for the internal & OSS tests is probably the right thing to do then. Great that you managed to get the tests working internally.