Skip to content
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

genbindings doesn't work with clang-19 #116

Open
arnetheduck opened this issue Jan 3, 2025 · 3 comments · May be fixed by #118
Open

genbindings doesn't work with clang-19 #116

arnetheduck opened this issue Jan 3, 2025 · 3 comments · May be fixed by #118
Labels

Comments

@arnetheduck
Copy link
Contributor

Just tried the bindings generator on a more recent distro than bookworm, where clang 19 is available - the json output has changed and the way constructor types are specified has changed as well:

			{
				"id": "0x564dc729da10",
				"inner": [
					{
						"id": "0x564dc729d8f0",
						"kind": "ParmVarDecl",
						"loc": {
							"col": 26,
							"offset": 13690,
							"tokLen": 4
						},
						"name": "type",
						"range": {
							"begin": {
								"col": 21,
								"offset": 13685,
								"tokLen": 4
							},
							"end": {
								"col": 26,
								"offset": 13690,
								"tokLen": 4
							}
						},
						"type": {
							"desugaredQualType": "QEvent::Type",
							"qualType": "Type"
						}
					}
				],
				"kind": "CXXConstructorDecl",
				"loc": {
					"col": 14,
					"line": 297,
					"offset": 13678,
					"tokLen": 6
				},
				"mangledName": "_ZN6QEventC1ENS_4TypeE",
				"name": "QEvent",
				"range": {
					"begin": {
						"col": 5,
						"offset": 13669,
						"tokLen": 8
					},
					"end": {
						"col": 30,
						"offset": 13694,
						"tokLen": 1
					}
				},
				"type": {
					"qualType": "void (Type)"
				}
			},

This is the QEvent constructor taking a QEvent::Type - in clang-14, the constructor type is void (QEvent::Type) while in clang-19 it's void (Type) - the consequence is that Type is no longer found as an "enum" leading to lots of downstream problems.

To support clang-19, one would have to dig into inner and fetch the type from desugaredQualType instead of parsing the prototype - opening this issue mostly to document the problem, even if this is not a supported way of running the generator today.

rcalixte added a commit to rcalixte/miqt that referenced this issue Jan 3, 2025
@mappu mappu added the wishlist label Jan 4, 2025
rcalixte added a commit to rcalixte/miqt that referenced this issue Jan 4, 2025
rcalixte added a commit to rcalixte/miqt that referenced this issue Jan 4, 2025
rcalixte added a commit to rcalixte/miqt that referenced this issue Jan 4, 2025
rcalixte added a commit to rcalixte/miqt that referenced this issue Jan 4, 2025
rcalixte added a commit to rcalixte/miqt that referenced this issue Jan 11, 2025
rcalixte added a commit to rcalixte/miqt that referenced this issue Jan 11, 2025
rcalixte added a commit to rcalixte/miqt that referenced this issue Jan 11, 2025
rcalixte added a commit to rcalixte/miqt that referenced this issue Jan 11, 2025
rcalixte added a commit to rcalixte/miqt that referenced this issue Jan 11, 2025
rcalixte added a commit to rcalixte/miqt that referenced this issue Jan 20, 2025
rcalixte added a commit to rcalixte/miqt that referenced this issue Jan 20, 2025
rcalixte added a commit to rcalixte/miqt that referenced this issue Jan 20, 2025
rcalixte added a commit to rcalixte/miqt that referenced this issue Jan 20, 2025
rcalixte added a commit to rcalixte/miqt that referenced this issue Jan 20, 2025
@rcalixte
Copy link
Contributor

rcalixte commented Apr 2, 2025

I've been playing with this on and off for some time now but I think we've finally got our smoking gun:

I think what you want, if I got that right, can be achieved by using the QualTypeNames module, specifically clang::TypeName::getFullyQualifiedName.

That begets the next problem. I don't see any libraries for Go that are up-to-date for LLVM/Clang 19 that fully bind this. One option is writing a tool in C++ that we could execute from Go (I know... 😔) that would handle this. It would be a performance and run-time hit when the cache is empty but other than that, it should be negligible. Additionally, it should be stable long-term as it is consuming Clang's API directly.

The proposed structure of the directory for this code under genbindings would be:

tools/
└── qualtype
    ├── build_qualtype.sh
    └── qualtype.cpp

Assuming a useful binary can be compiled and then executed, this poses at least a path forward for type resolutions?

Another option I considered was that we could rewrite this functionality in the Go code. I briefly explored this. Basically, we know what headers are being included in each header that we are processing which means that we can pre-process the Enum types and perform recursive lookups until a match is found. A simple example that I've been using is the type.qualType value from the QSurface::surfaceType method:

Clang 14:

{
        "id": "0x7ffa53bc2578",
        "kind": "CXXMethodDecl",
        "loc": {
                "col": 25,
                "line": 48,
                "offset": 962,
                "tokLen": 11
        },
        "mangledName": "_ZNK8QSurface11surfaceTypeEv",
        "name": "surfaceType",
        "pure": true,
        "range": {
                "begin": {
                        "col": 5,
                        "offset": 942,
                        "tokLen": 7
                },
                "end": {
                        "col": 47,
                        "offset": 984,
                        "tokLen": 1
                }
        },
        "type": {
                "qualType": "QSurface::SurfaceType () const"
        },
        "virtual": true
},

Clang 19:

{
        "id": "0x7fc3aa020028",
        "kind": "CXXMethodDecl",
        "loc": {
                "col": 25,
                "line": 48,
                "offset": 962,
                "tokLen": 11
        },
        "mangledName": "_ZNK8QSurface11surfaceTypeEv",
        "name": "surfaceType",
        "pure": true,
        "range": {
                "begin": {
                        "col": 5,
                        "offset": 942,
                        "tokLen": 7
                },
                "end": { 
                        "col": 47,
                        "offset": 984,
                        "tokLen": 1
                }
        },
        "type": {
                "qualType": "SurfaceType () const"
        },
        "virtual": true
},

This is the simplest case where the enum's fully qualified type is defined in the local class. Any enum values not found in the local class or the included headers would then fall to recursing through parent classes until a match is found. Once the lookup is complete, we save the fully qualified result type and continue parsing. (Maybe this requires three passes in all? I've been considering this in general since I think we can stand to gain quite a bit from additional pre-processing.)

Both options are expensive when the cache is empty but only for parsing types that are known enums (or flags?). I'd love to hear some thoughts on this. 😅

@mappu
Copy link
Owner

mappu commented Apr 4, 2025

Both the multi-pass parsing idea, or the cpp helper idea sound good to me.

I don't see any libraries for Go that are up-to-date for LLVM/Clang 19 that fully bind this

A Go binding for the clang API might not be required for a few single functions, embedded cgo can call a C++ function (as demonstrated by miqt)

Additionally, it should be stable long-term as it is consuming Clang's API directly

I think this might not be a panacea, Clang is somewhat well-known for breaking its C++ API too.

@rcalixte
Copy link
Contributor

rcalixte commented Apr 4, 2025

I think this might not be a panacea, Clang is somewhat well-known for breaking its C++ API too.

Haha yeah, I guess I'm being optimistic about this API given that it was just "fixed" despite the fallout.

Regardless, I think the multi-pass is going to be more straightforward as it can be 100% in Go and there's been things I've wanted to precompute for each language binding to potentially remove some of the (crudely) hard-coded things anyways. Once this code is done, the biggest blocker for a Qt 6.8 release (and beyond?) should also be removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants