Investigate upgrade to clang 13

sourcegraph / lsif-clang

Language Server Indexing Format (LSIF) generator for C, C++ and Objective C

https://lsif.dev/

35 stars 8 forks source link

Investigate upgrade to clang 13 #89

Open varungandhi-src opened 2 years ago

varungandhi-src commented 2 years ago

It seems like there were some concurrency related bugs fixed between clang 11 and clang 13. (discussion)

I'm a little hesitant to make such a big change* without testing infrastructure... Ideally, we'd have at least some testing with sanitizers + on higher core count machines, whereas GitHub Action's default runners are relatively underpowered at 2 vCPUs. 😐

Maybe we can do it smaller pieces:

[ ] Shortlist some projects + set up a CI job to build those. https://github.com/sourcegraph/lsif-clang/issues/90
[ ] Set up a build with sanitizers + CI job for that
[ ] Perform the merge: This part seems simple based on this comment.

The "investigate" in the title is deliberate; this issue isn't a commitment to upgrade to LLVM 13.

*The merge may be small, but there is a chance around broken invariants, or some random null pointers in places where they weren't coming up before.

jlisee commented 2 years ago

Notes on my Clang 13 based version:

Branch jlisee/llvm-13.0.1-upgrade-v2
Main changes are in this commit https://github.com/jlisee/lsif-clang/commit/df04f9e0d0bac389ea83fea309dc56c8fcb152dd

I am most likely going to be force pushing that branch occasionally as I re-merge the upstream main branch with the llvm-main branch in my fork from the to capture the latest lsif-clang changes.

kkpattern commented 2 years ago

On recent macOS, clang-11 fails to build some projects. For example https://trac.macports.org/ticket/64152. With lsif-clang built with llvm-11 on macOS, we encounter a similar problem when lsif-clang processing our source code. It would be nice to be able to upgrade lsif-clang to clang-13.

varungandhi-src commented 2 years ago

@kkpattern did you try using the fork mentioned above and did that fix the issue? (Trying to understand if your point is mainly that we should merge those changes in or whether you tried the clang-13 based fork and it did work.)

kkpattern commented 2 years ago

@kkpattern did you try using the fork mentioned above and did that fix the issue? (Trying to understand if your point is mainly that we should merge those changes in or whether you tried the clang-13 based fork and it did work.)

I haven't tried the forked yet. For other clang-based tools we use, switch clang-11 to clang-13 will solve the problem. I will try the above fork soon once I get some time.

kkpattern commented 2 years ago

I have tried the https://github.com/jlisee/lsif-clang/tree/jlisee/llvm-13.0.1-upgrade-v2 branch. At first, it could not correctly find the clang config on my machine. In this line it set the clang_search_path to "${LLVM_DIR}/../lib/cmake/clang". In my environment, the LLVM_DIR is set to /path/to/llvm/lib/cmake/llvm so this line needs to be changed to set(clang_search_path "${LLVM_DIR}/../clang").

After that, the build succeeds with LLVM-13 on macOS Monterey.