search

jnikula / hawkmoth

Hawkmoth - Sphinx Autodoc for C
https://jnikula.github.io/hawkmoth/
BSD 2-Clause "Simplified" License
74 stars 12 forks source link

parser: fix linkage spec detection #237

Closed BrunoMSantos closed 8 months ago

BrunoMSantos commented 8 months ago

Clang 18 now issues the right cursor kind for extern "C" blocks, so we need to extend the detection code.

Fixes #235.

-->8--

This should reliably fix the detection of those blocks on all supported versions of Clang, but a few improvements might still be possible:

  1. We should probably not check for UNEXPOSED_DECL unless we are on pre-18 versions to keep it neater. This would require a version check on the libclang that is actually loaded.

  2. It is unclear whether we can avoid token matching for v18+. In principle, if the cursor kind is LINKAGE_SPEC, the children cursors are in the C domain. But is that a guarantee or just a strong heuristic? If not, is there a way of checking without token matching?

  3. We should come up with a way to systematically mark code portions that need to be revisited as we update minimum requirements and such to avoid dead code. I can only think of another instance where we might want to do something along those lines (anonymous symbol handling), but I think it may be good to think about this soon-ish rather than later just in case more cases pop up.

BrunoMSantos commented 8 months ago

Point 3. might actually be made redundant by point 1.

jnikula commented 8 months ago

Replying to https://github.com/jnikula/hawkmoth/issues/235#issuecomment-2016783664

I don't think any of those points are particularly interesting as long as it works reliably and I think it does, but I was letting you have a say on them I guess. Point 2 particularly, could be investigated a bit more (I spent maybe 15min on this, most of it setting up my environment to replicate the issue).

I don't think any of the points are blockers for merging this as-is now, but rather things to consider for the future.

There hasn't been much activity lately, but I might do a release sooner rather than later with the fixes.

jnikula commented 8 months ago
  1. This would require a version check on the libclang that is actually loaded.

I don't actually see a way to cleanly check the libclang version from python-clang. Do you have any ideas?

BrunoMSantos commented 8 months ago

I don't think any of the points are blockers for merging this as-is now, but rather things to consider for the future.

Very well, I'm leaning that way too.

I don't actually see a way to cleanly check the libclang version from python-clang. Do you have any ideas?

Not really. Actually I asked you in IRC after having given up already.

Best I could come up with was to look into cindex.Config.library_{file,path} and infer it from there or, if None, look into the LD path and so on. Not clean at all...

jnikula commented 8 months ago

Merged, thanks for the prompt fix @BrunoMSantos!