From 99aaf69b1337765354b8a1397661c45cc5a1f77c Mon Sep 17 00:00:00 2001 From: mappu Date: Wed, 18 Sep 2024 12:11:48 +1200 Subject: [PATCH] genbindings: remove uintptr fallbacks for more and accurate enum/typedef handling --- cmd/genbindings/emitcabi.go | 20 ++++---- cmd/genbindings/emitgo.go | 17 ++++--- cmd/genbindings/exceptions.go | 22 +++++++-- cmd/genbindings/intermediate.go | 57 ++++++++++++++--------- cmd/genbindings/transformtypedefs.go | 12 +++-- cmd/genbindings/transformtypedefs_test.go | 34 +++++++++++++- 6 files changed, 111 insertions(+), 51 deletions(-) diff --git a/cmd/genbindings/emitcabi.go b/cmd/genbindings/emitcabi.go index c99c22d..a7446f6 100644 --- a/cmd/genbindings/emitcabi.go +++ b/cmd/genbindings/emitcabi.go @@ -55,9 +55,9 @@ func (p CppParameter) RenderTypeCabi() string { ret = "size_t" case "qreal": ret = "double" - case "qintptr": + case "qintptr", "QIntegerForSizeof::Signed": ret = "intptr_t" - case "quintptr", "uintptr": + case "quintptr", "uintptr", "QIntegerForSizeof::Unsigned": ret = "uintptr_t" case "qptrdiff": ret = "ptrdiff_t" @@ -76,9 +76,6 @@ func (p CppParameter) RenderTypeCabi() string { } else if e, ok := KnownEnums[p.ParameterType]; ok { ret = e.UnderlyingType.RenderTypeCabi() - } else if strings.Contains(p.ParameterType, `::`) { - // Inner class - ret = cabiClassName(p.ParameterType) } if p.Pointer { @@ -91,7 +88,7 @@ func (p CppParameter) RenderTypeCabi() string { } func (p CppParameter) RenderTypeQtCpp() string { - cppType := p.UnderlyingType() + cppType := p.GetQtCppType() if p.Const { cppType = "const " + cppType @@ -212,6 +209,9 @@ func emitCABI2CppForwarding(p CppParameter, indent string) (preamble string, for // Cast to the Qt enum type so that we get the correct overload return preamble, "static_cast<" + p.RenderTypeQtCpp() + ">(" + p.ParameterName + ")" + } else if p.IsFlagType() { + return preamble, "static_cast<" + p.RenderTypeQtCpp() + ">(" + p.ParameterName + ")" + } else if p.IntType() { // Use the raw ParameterType to select an explicit integer overload // Don't use RenderTypeCabi() since it canonicalizes some int types for CABI @@ -366,19 +366,19 @@ func emitAssignCppToCabi(assignExpression string, p CppParameter, rvalue string) // Elide temporary and emit directly from the rvalue return indent + assignExpression + "new " + p.ParameterType + "(" + rvalue + ");\n" - } else if p.Const { - shouldReturn += "(" + p.RenderTypeCabi() + ") " - } else if p.IsFlagType() { // Needs an explicit int cast shouldReturn = p.RenderTypeQtCpp() + " " + namePrefix + "_ret = " afterCall += indent + "" + assignExpression + "static_cast(" + namePrefix + "_ret);\n" - } else if p.IsKnownEnum() { + } else if p.IsKnownEnum() || p.QtCppOriginalType != "" { // Needs an explicit uintptr cast shouldReturn = p.RenderTypeQtCpp() + " " + namePrefix + "_ret = " afterCall += indent + "" + assignExpression + "static_cast<" + p.RenderTypeCabi() + ">(" + namePrefix + "_ret);\n" + } else if p.Const { + shouldReturn += "(" + p.RenderTypeCabi() + ") " + } return indent + shouldReturn + rvalue + ";\n" + afterCall diff --git a/cmd/genbindings/emitgo.go b/cmd/genbindings/emitgo.go index 428d27e..82afadb 100644 --- a/cmd/genbindings/emitgo.go +++ b/cmd/genbindings/emitgo.go @@ -82,21 +82,20 @@ func (p CppParameter) RenderTypeGo() string { } else { ret += "uint64" } - case "qintptr", "uintptr_t", "intptr_t", "quintptr": + case "qintptr", "uintptr_t", "intptr_t", "quintptr", "QIntegerForSizeof::Unsigned", "QIntegerForSizeof::Signed": ret += "uintptr" default: if p.IsFlagType() { ret += "int" - } else if strings.Contains(p.ParameterType, `::`) { - if p.IsKnownEnum() { - ret += cabiClassName(p.ParameterType) + } else if p.IsKnownEnum() { + ret += cabiClassName(p.ParameterType) + + } else if strings.Contains(p.ParameterType, `::`) { + // Inner class + ret += cabiClassName(p.ParameterType) - } else { - // Inner class - ret += cabiClassName(p.ParameterType) - } } else { // Do not transform this type ret += p.ParameterType @@ -371,7 +370,7 @@ func (gfs *goFileState) emitCabiToGo(assignExpr string, rt CppParameter, rvalue } } - } else if rt.IntType() || rt.ParameterType == "bool" { + } else if rt.IntType() || rt.IsKnownEnum() || rt.IsFlagType() || rt.ParameterType == "bool" || rt.QtCppOriginalType != "" { // Need to cast Cgo type to Go int type // Optimize assignment to avoid temporary return assignExpr + "(" + rt.RenderTypeGo() + ")(" + rvalue + ")\n" diff --git a/cmd/genbindings/exceptions.go b/cmd/genbindings/exceptions.go index c4cd263..bc739fa 100644 --- a/cmd/genbindings/exceptions.go +++ b/cmd/genbindings/exceptions.go @@ -107,6 +107,18 @@ func CheckComplexity(p CppParameter, isReturnType bool) error { if strings.HasPrefix(p.ParameterType, "StringResult<") { return ErrTooComplex // e.g. qcborstreamreader.h } + if strings.HasPrefix(p.ParameterType, "QScopedPointer<") { + return ErrTooComplex // e.g. qbrush.h + } + if strings.HasPrefix(p.ParameterType, "QExplicitlySharedDataPointer<") { + return ErrTooComplex // e.g. qpicture.h + } + if strings.HasPrefix(p.ParameterType, "QSharedDataPointer<") { + return ErrTooComplex // e.g. qurlquery.h + } + if strings.HasPrefix(p.ParameterType, "QTypedArrayData<") { + return ErrTooComplex // e.g. qbitarray.h + } if strings.HasPrefix(p.ParameterType, "QGenericMatrix<") { return ErrTooComplex // e.g. qmatrix4x4.h } @@ -130,9 +142,13 @@ func CheckComplexity(p CppParameter, isReturnType bool) error { if strings.Contains(p.ParameterType, `::QPrivate`) { return ErrTooComplex // e.g. QAbstractItemModel::QPrivateSignal } + if strings.Contains(p.GetQtCppType(), `::DataPtr`) { + return ErrTooComplex // e.g. QImage::data_ptr() + } // Some QFoo constructors take a QFooPrivate - if p.ParameterType[0] == 'Q' && strings.HasSuffix(p.ParameterType, "Private") && !isReturnType { + // QIcon also returns a QIconPrivate + if p.ParameterType[0] == 'Q' && strings.HasSuffix(p.ParameterType, "Private") { return ErrTooComplex } @@ -217,11 +233,11 @@ func CheckComplexity(p CppParameter, isReturnType bool) error { // generated headers (generated on Linux) with other OSes such as Windows. // These methods will be blocked on non-Linux OSes. func LinuxWindowsCompatCheck(p CppParameter) bool { - if p.TypeAlias == "Q_PID" { + if p.GetQtCppType() == "Q_PID" { return true // int64 on Linux, _PROCESS_INFORMATION* on Windows } - if p.ParameterType == "QSocketDescriptor::DescriptorType" { + if p.GetQtCppType() == "QSocketDescriptor::DescriptorType" { return true // uintptr_t-compatible on Linux, void* on Windows } return false diff --git a/cmd/genbindings/intermediate.go b/cmd/genbindings/intermediate.go index e22e235..4069b03 100644 --- a/cmd/genbindings/intermediate.go +++ b/cmd/genbindings/intermediate.go @@ -25,38 +25,44 @@ func init() { // QString is deleted from this binding KnownTypedefs["QStringList"] = CppTypedef{"QStringList", parseSingleTypeString("QList")} + // Not sure why this isn't picked up automatically + // FIXME because QFile inherits QFileDevice(!!) and the name refers to its parent class + KnownTypedefs["QFile::FileTime"] = CppTypedef{"QFile::FileTime", parseSingleTypeString("QFileDevice::FileTime")} + + // n.b. Qt 5 only + KnownTypedefs["QLineF::IntersectionType"] = CppTypedef{"QLineF::IntersectionType", parseSingleTypeString("QLineF::IntersectType")} + + // Not sure the reason for this one + KnownTypedefs["QSocketDescriptor::DescriptorType"] = CppTypedef{"QSocketDescriptor::DescriptorType", parseSingleTypeString("QSocketNotifier::Type")} } type CppParameter struct { - ParameterName string - ParameterType string - TypeAlias string // If we rewrote QStringList->QList, this field contains the original QStringList - Const bool - Pointer bool - PointerCount int - ByRef bool - Optional bool + ParameterName string + ParameterType string + QtCppOriginalType string // If we rewrote QStringList->QList, this field contains the original QStringList. Otherwise, it's blank + Const bool + Pointer bool + PointerCount int + ByRef bool + Optional bool } func (p *CppParameter) AssignAlias(newType string) { - if p.TypeAlias == "" { - p.TypeAlias = p.ParameterType // Overwrite once only, at the earliest base type + if p.QtCppOriginalType == "" { + p.QtCppOriginalType = p.ParameterType // Overwrite once only, at the earliest base type } p.ParameterType = newType } -func (p *CppParameter) CopyWithAlias(alias CppParameter) CppParameter { - ret := *p // copy - ret.ParameterName = alias.ParameterName - ret.TypeAlias = alias.ParameterType +func (p *CppParameter) ApplyTypedef(matchedUnderlyingType CppParameter) { + p.AssignAlias(matchedUnderlyingType.ParameterType) // If this was a pointer to a typedef'd type, or a typedef of a pointer type, we need to preserve that - // WARNING: This can't work for double indirection - ret.Const = ret.Const || alias.Const - ret.Pointer = ret.Pointer || alias.Pointer - ret.PointerCount += alias.PointerCount - ret.ByRef = ret.ByRef || alias.ByRef - return ret + p.Const = p.Const || matchedUnderlyingType.Const + p.Pointer = p.Pointer || matchedUnderlyingType.Pointer + p.PointerCount += matchedUnderlyingType.PointerCount + p.ByRef = p.ByRef || matchedUnderlyingType.ByRef + p.Optional = p.Optional || matchedUnderlyingType.Optional } func (p *CppParameter) PointerTo() CppParameter { @@ -72,9 +78,9 @@ func (p *CppParameter) ConstCast(isConst bool) CppParameter { return ret } -func (p *CppParameter) UnderlyingType() string { - if p.TypeAlias != "" { - return p.TypeAlias +func (p *CppParameter) GetQtCppType() string { + if p.QtCppOriginalType != "" { + return p.QtCppOriginalType } return p.ParameterType @@ -85,6 +91,10 @@ func (p CppParameter) IsFlagType() bool { return true // This catches most cases through the typedef system } + if strings.HasPrefix(p.GetQtCppType(), `QFlags<`) { + return true // This catches most cases through the typedef system + } + switch p.ParameterType { case "QTouchEvent::TouchPoint::InfoFlags", "QFile::Permissions", @@ -166,6 +176,7 @@ func (p CppParameter) IntType() bool { "longlong", "ulonglong", "qlonglong", "qulonglong", "qint64", "quint64", "int64_t", "uint64_t", "long long", "unsigned long long", "qintptr", "quintptr", "uintptr_t", "intptr_t", "qsizetype", "size_t", + "QIntegerForSizeof::Unsigned", "qptrdiff", "ptrdiff_t", "double", "float", "qreal": return true diff --git a/cmd/genbindings/transformtypedefs.go b/cmd/genbindings/transformtypedefs.go index 4d86b2d..307c0bd 100644 --- a/cmd/genbindings/transformtypedefs.go +++ b/cmd/genbindings/transformtypedefs.go @@ -6,20 +6,24 @@ import ( func applyTypedefs(p CppParameter) CppParameter { - if td, ok := KnownTypedefs[p.ParameterType]; ok { - p = td.UnderlyingType.CopyWithAlias(p) + for { + td, ok := KnownTypedefs[p.ParameterType] + if !ok { + break + } + p.ApplyTypedef(td.UnderlyingType) } if t, ok := p.QListOf(); ok { t2 := applyTypedefs(t) // recursive // Wipe out so that RenderTypeQtCpp() does not see it - t2.TypeAlias = "" + t2.QtCppOriginalType = "" // QListOf returns for either QList< or QVector< // Patch it up to the first < position and last character bpos := strings.Index(p.ParameterType, `<`) - p.ParameterType = p.ParameterType[0:bpos] + `<` + t2.RenderTypeQtCpp() + `>` + p.AssignAlias(p.ParameterType[0:bpos] + `<` + t2.RenderTypeQtCpp() + `>`) } return p diff --git a/cmd/genbindings/transformtypedefs_test.go b/cmd/genbindings/transformtypedefs_test.go index dcce727..88c5d6a 100644 --- a/cmd/genbindings/transformtypedefs_test.go +++ b/cmd/genbindings/transformtypedefs_test.go @@ -1,6 +1,7 @@ package main import ( + "strings" "testing" ) @@ -30,15 +31,44 @@ func TestTransformTypedefs(t *testing.T) { runTest := func(check string, expect string) { parsed := makeTest(check) + astTransformTypedefs(&parsed) - got := parsed.Classes[0].Ctors[0].Parameters[0].ParameterType + resultP := parsed.Classes[0].Ctors[0].Parameters[0] + got := resultP.ParameterType + + if resultP.Const { + got = "const " + got + } + if resultP.Pointer { + got += strings.Repeat("*", resultP.PointerCount) + } + if resultP.ByRef { + got += "&" + } + if got != expect { - t.Errorf("Transform of WId got %q, expected %q", got, expect) + t.Errorf("Transform of %q got %q, expected %q", check, got, expect) } } runTest("WId", "uintptr_t") runTest("QList", "QList") + runTest("QStringList", "QList") runTest("QVector", "QVector") + + KnownTypedefs["_test_known_typedef_recursion"] = CppTypedef{"_test_known_typedef_recursion", parseSingleTypeString("WId")} + runTest("_test_known_typedef_recursion", "uintptr_t") + + // Pointer tests + runTest("WId*", "uintptr_t*") + runTest("QVector", "QVector") + + // Const tests + runTest("const QVector", "const QVector") + + // Typedefs changing pointer values + KnownTypedefs["_test_iterator"] = CppTypedef{"_test_iterator", parseSingleTypeString("char*")} + runTest("_test_iterator", "char*") + }