search

facebook / infer

A static analyzer for Java, C, C++, and Objective-C
http://fbinfer.com/
MIT License
14.93k stars 2.01k forks source link

MinGW support, take 2 #1755

Closed yakobowski closed 1 year ago

yakobowski commented 1 year ago

This is a new iteration on #1394. The code in that one was so old, and has changed so much, that it did not seem worth it to push on top of it. Some comments may still apply, and I apologize in advance for the (partial) double review.

This MR is not complete, in the sense that the code that uses Core_kernel instead of Core is not here. We are too far apart from you (in terms of OCaml and Core versions) for our code to be helpful. But the "meat" of the functionality is in this MR.

yakobowski commented 1 year ago

Note that 22a326b is intentional: 71d2f3f is much more readable (as a diff) with the indentation hack it contains, and we remove this hack in the next commit.

jeremydubreil commented 1 year ago

Would the GitHub Actions tests work after this PR if we add windows-latest to the matrix of operating systems?

jvillard commented 1 year ago

@jeremydubreil I don't think this builds on Windows yet. There's a second part to this which is to stub out the parts of Core that we use and are not available on Windows. Fortunately those parts are well separated in Core now so it shouldn't be too messy.

yakobowski commented 1 year ago

Are you ok with the shortened filenames as is? If so, I can try updating the (non-clang) tests.

facebook-github-bot commented 1 year ago

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

jvillard commented 1 year ago

@yakobowski looking great! I've updated the test results on our end to account for the shorter hashes and kicked our CI again, let's see.

facebook-github-bot commented 1 year ago

@jvillard merged this pull request in facebook/infer@159a29b029f787ebaa5c6096f3c1a784997cd889.

jvillard commented 1 year ago

many thanks @yakobowski, very excited to have this big step towards windows support checked in! :fire:

I made a few follow-up changes, just fyi: 12718677946f51b59795f37933b3ceb2dfaedfe0, bf340c25da2b6ab90c85b3466d910cf71703305a

yakobowski commented 1 year ago

Thank you! I'm delighted to see this one merged :tada: I've looked at the followup commits, and indeed those are nice improvements.