From cbdda95909e02d7e1c453f4cd9d3074da00467dd Mon Sep 17 00:00:00 2001 From: Joe George Date: Mon, 9 Oct 2023 10:45:03 -0400 Subject: [PATCH 1/7] slic2java: fix constructor parameter count Closes #1539 --- cpp/src/slice2java/Gen.cpp | 28 +++++++++++++++++++++++++--- cpp/src/slice2java/GenCompat.cpp | 28 +++++++++++++++++++++++++--- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/cpp/src/slice2java/Gen.cpp b/cpp/src/slice2java/Gen.cpp index dbd2ddfd280..ff29ec4fe06 100644 --- a/cpp/src/slice2java/Gen.cpp +++ b/cpp/src/slice2java/Gen.cpp @@ -136,6 +136,28 @@ string ofFactory(const TypePtr& type) return "java.util.Optional.ofNullable"; } +// Returns the "length" of the a constructor parameter list from the given data member list. +// All types are counted as 1, except for long and double which are counted as 2. +// See https://docs.oracle.com/javase/specs/jvms/se20/html/jvms-4.html#jvms-4.3.3 +int +constructorParameterLength(const DataMemberList& members) +{ + int length = 0; + for(DataMemberList::const_iterator i = members.begin(); i != members.end(); ++i) + { + BuiltinPtr builtin = BuiltinPtr::dynamicCast((*i)->type()); + if(builtin && (builtin->kind() == Builtin::KindLong || builtin->kind() == Builtin::KindDouble)) + { + length += 2; + } + else + { + length++; + } + } + return length; +} + } Slice::JavaVisitor::JavaVisitor(const string& dir) : @@ -2554,7 +2576,7 @@ Slice::Gen::TypesVisitor::visitClassDefStart(const ClassDefPtr& p) // // A method cannot have more than 255 parameters (including the implicit "this" argument). // - if(allDataMembers.size() < 255) + if(constructorParameterLength(allDataMembers) < 255) { DataMemberList baseDataMembers; if(baseClass) @@ -2947,7 +2969,7 @@ Slice::Gen::TypesVisitor::visitExceptionStart(const ExceptionPtr& p) // // A method cannot have more than 255 parameters (including the implicit "this" argument). // - if(allDataMembers.size() < 255) + if(constructorParameterLength(allDataMembers) < 255) { if(hasRequiredMembers && hasOptionalMembers) { @@ -3384,7 +3406,7 @@ Slice::Gen::TypesVisitor::visitStructEnd(const StructPtr& p) // // A method cannot have more than 255 parameters (including the implicit "this" argument). // - if(members.size() < 255) + if(constructorParameterLength(members) < 255) { vector paramDecl; vector paramNames; diff --git a/cpp/src/slice2java/GenCompat.cpp b/cpp/src/slice2java/GenCompat.cpp index b3f5504f741..79214ef6b02 100644 --- a/cpp/src/slice2java/GenCompat.cpp +++ b/cpp/src/slice2java/GenCompat.cpp @@ -210,6 +210,28 @@ writeParamList(Output& out, vector params, bool end = true, bool newLine } } +// Returns the "length" of the a constructor parameter list from the given data member list. +// All types are counted as 1, except for long and double which are counted as 2. +// See https://docs.oracle.com/javase/specs/jvms/se20/html/jvms-4.html#jvms-4.3.3 +int +constructorParameterLength(const DataMemberList& members) +{ + int length = 0; + for(DataMemberList::const_iterator i = members.begin(); i != members.end(); ++i) + { + BuiltinPtr builtin = BuiltinPtr::dynamicCast((*i)->type()); + if(builtin && (builtin->kind() == Builtin::KindLong || builtin->kind() == Builtin::KindDouble)) + { + length += 2; + } + else + { + length++; + } + } + return length; +} + } Slice::JavaCompatVisitor::JavaCompatVisitor(const string& dir) : @@ -2947,7 +2969,7 @@ Slice::GenCompat::TypesVisitor::visitClassDefStart(const ClassDefPtr& p) // // A method cannot have more than 255 parameters (including the implicit "this" argument). // - if(allDataMembers.size() < 255) + if(constructorParameterLength(allDataMembers) < 255) { DataMemberList baseDataMembers; if(baseClass) @@ -3257,7 +3279,7 @@ Slice::GenCompat::TypesVisitor::visitExceptionStart(const ExceptionPtr& p) // // A method cannot have more than 255 parameters (including the implicit "this" argument). // - if(allDataMembers.size() < 255) + if(constructorParameterLength(allDataMembers) < 255) { if(hasRequiredMembers && hasOptionalMembers) { @@ -3672,7 +3694,7 @@ Slice::GenCompat::TypesVisitor::visitStructEnd(const StructPtr& p) // // A method cannot have more than 255 parameters (including the implicit "this" argument). // - if(members.size() < 255) + if(constructorParameterLength(members) < 255) { vector paramDecl; vector paramNames; From c7f0461dc8120da88ae5caf7a8b8aff25cbc29a4 Mon Sep 17 00:00:00 2001 From: Joe George Date: Mon, 9 Oct 2023 11:28:26 -0400 Subject: [PATCH 2/7] Update cpp/src/slice2java/Gen.cpp Co-authored-by: Jose --- cpp/src/slice2java/Gen.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/slice2java/Gen.cpp b/cpp/src/slice2java/Gen.cpp index ff29ec4fe06..34ba65b95d6 100644 --- a/cpp/src/slice2java/Gen.cpp +++ b/cpp/src/slice2java/Gen.cpp @@ -136,7 +136,7 @@ string ofFactory(const TypePtr& type) return "java.util.Optional.ofNullable"; } -// Returns the "length" of the a constructor parameter list from the given data member list. +// Returns the "length" of the constructor parameter list from the given data member list. // All types are counted as 1, except for long and double which are counted as 2. // See https://docs.oracle.com/javase/specs/jvms/se20/html/jvms-4.html#jvms-4.3.3 int From cfed163787de7d971c50df93361317919e2198f1 Mon Sep 17 00:00:00 2001 From: Joe George Date: Mon, 9 Oct 2023 11:28:31 -0400 Subject: [PATCH 3/7] Update cpp/src/slice2java/GenCompat.cpp Co-authored-by: Jose --- cpp/src/slice2java/GenCompat.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/slice2java/GenCompat.cpp b/cpp/src/slice2java/GenCompat.cpp index 79214ef6b02..4064c090967 100644 --- a/cpp/src/slice2java/GenCompat.cpp +++ b/cpp/src/slice2java/GenCompat.cpp @@ -210,7 +210,7 @@ writeParamList(Output& out, vector params, bool end = true, bool newLine } } -// Returns the "length" of the a constructor parameter list from the given data member list. +// Returns the "length" of the constructor parameter list from the given data member list. // All types are counted as 1, except for long and double which are counted as 2. // See https://docs.oracle.com/javase/specs/jvms/se20/html/jvms-4.html#jvms-4.3.3 int From 143d504bd8ced1fe4405707bc476dff41b634c99 Mon Sep 17 00:00:00 2001 From: Joe George Date: Mon, 9 Oct 2023 11:54:12 -0400 Subject: [PATCH 4/7] Update cpp/src/slice2java/GenCompat.cpp Co-authored-by: Austin Henriksen --- cpp/src/slice2java/GenCompat.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/slice2java/GenCompat.cpp b/cpp/src/slice2java/GenCompat.cpp index 4064c090967..888f62cf7a9 100644 --- a/cpp/src/slice2java/GenCompat.cpp +++ b/cpp/src/slice2java/GenCompat.cpp @@ -211,7 +211,7 @@ writeParamList(Output& out, vector params, bool end = true, bool newLine } // Returns the "length" of the constructor parameter list from the given data member list. -// All types are counted as 1, except for long and double which are counted as 2. +// All types are counted as 1 unit, except for long and double which are counted as 2 units. // See https://docs.oracle.com/javase/specs/jvms/se20/html/jvms-4.html#jvms-4.3.3 int constructorParameterLength(const DataMemberList& members) From c8a75133cc47363522bc61b1c1e3b023c7cf16e3 Mon Sep 17 00:00:00 2001 From: Joe George Date: Mon, 9 Oct 2023 11:54:19 -0400 Subject: [PATCH 5/7] Update cpp/src/slice2java/Gen.cpp Co-authored-by: Austin Henriksen --- cpp/src/slice2java/Gen.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/slice2java/Gen.cpp b/cpp/src/slice2java/Gen.cpp index 34ba65b95d6..0c05f351c9d 100644 --- a/cpp/src/slice2java/Gen.cpp +++ b/cpp/src/slice2java/Gen.cpp @@ -137,7 +137,7 @@ string ofFactory(const TypePtr& type) } // Returns the "length" of the constructor parameter list from the given data member list. -// All types are counted as 1, except for long and double which are counted as 2. +// All types are counted as 1 unit, except for long and double which are counted as 2 units. // See https://docs.oracle.com/javase/specs/jvms/se20/html/jvms-4.html#jvms-4.3.3 int constructorParameterLength(const DataMemberList& members) From ea366461db0b6c801587736daed44d4b6a6fc6ce Mon Sep 17 00:00:00 2001 From: Joe George Date: Mon, 9 Oct 2023 12:26:21 -0400 Subject: [PATCH 6/7] Review fixes --- cpp/src/Slice/JavaUtil.cpp | 22 +++++++++++++++++ cpp/src/Slice/JavaUtil.h | 9 +++++++ cpp/src/slice2java/Gen.cpp | 42 +++++++++----------------------- cpp/src/slice2java/GenCompat.cpp | 42 +++++++++----------------------- 4 files changed, 53 insertions(+), 62 deletions(-) diff --git a/cpp/src/Slice/JavaUtil.cpp b/cpp/src/Slice/JavaUtil.cpp index ad20211a1c7..9ad435fbae7 100644 --- a/cpp/src/Slice/JavaUtil.cpp +++ b/cpp/src/Slice/JavaUtil.cpp @@ -669,6 +669,28 @@ Slice::computeSerialVersionUUID(const ExceptionPtr& p) return hashCode; } +bool +Slice::isValidMethodParameterList(const DataMemberList& members, int additionalUnits) +{ + // The maximum length of a method parameter list is 255 units, including the implicit 'this' parameter. + // Each parameter is 1 unit, except for long and double parameters, which are 2 units. + // Start the length at 1 to account for the implicit 'this' parameter (plus any additional units). + int length = 1 + additionalUnits; + for(DataMemberList::const_iterator p = members.begin(); p != members.end(); ++p) + { + BuiltinPtr builtin = BuiltinPtr::dynamicCast((*p)->type()); + if(builtin && (builtin->kind() == Builtin::KindLong || builtin->kind() == Builtin::KindDouble)) + { + length += 2; + } + else + { + length++; + } + } + return length <= 255; +} + Slice::JavaOutput::JavaOutput() { } diff --git a/cpp/src/Slice/JavaUtil.h b/cpp/src/Slice/JavaUtil.h index 326d7adfa1a..431ef018966 100644 --- a/cpp/src/Slice/JavaUtil.h +++ b/cpp/src/Slice/JavaUtil.h @@ -29,6 +29,15 @@ computeSerialVersionUUID(const ExceptionPtr&); long computeSerialVersionUUID(const StructPtr&); +// +// Returns true if we can generate a method from the given data member list. A Java method +// can have a maximum of 255 parameters (including the implicit 'this') where each parameter +// is counted as 1 unit, except for long and double which are counted as 2 units. +// See https://docs.oracle.com/javase/specs/jvms/se20/html/jvms-4.html#jvms-4.3.3 +// +bool +isValidMethodParameterList(const DataMemberList &, int additionalUnits = 0); + class JavaOutput : public ::IceUtilInternal::Output { public: diff --git a/cpp/src/slice2java/Gen.cpp b/cpp/src/slice2java/Gen.cpp index 0c05f351c9d..7cd4c75f268 100644 --- a/cpp/src/slice2java/Gen.cpp +++ b/cpp/src/slice2java/Gen.cpp @@ -136,28 +136,6 @@ string ofFactory(const TypePtr& type) return "java.util.Optional.ofNullable"; } -// Returns the "length" of the constructor parameter list from the given data member list. -// All types are counted as 1 unit, except for long and double which are counted as 2 units. -// See https://docs.oracle.com/javase/specs/jvms/se20/html/jvms-4.html#jvms-4.3.3 -int -constructorParameterLength(const DataMemberList& members) -{ - int length = 0; - for(DataMemberList::const_iterator i = members.begin(); i != members.end(); ++i) - { - BuiltinPtr builtin = BuiltinPtr::dynamicCast((*i)->type()); - if(builtin && (builtin->kind() == Builtin::KindLong || builtin->kind() == Builtin::KindDouble)) - { - length += 2; - } - else - { - length++; - } - } - return length; -} - } Slice::JavaVisitor::JavaVisitor(const string& dir) : @@ -2574,9 +2552,9 @@ Slice::Gen::TypesVisitor::visitClassDefStart(const ClassDefPtr& p) out << eb; // - // A method cannot have more than 255 parameters (including the implicit "this" argument). + // Generate constructor if the parameter list is not too large. // - if(constructorParameterLength(allDataMembers) < 255) + if(isValidMethodParameterList(allDataMembers)) { DataMemberList baseDataMembers; if(baseClass) @@ -2967,9 +2945,9 @@ Slice::Gen::TypesVisitor::visitExceptionStart(const ExceptionPtr& p) } // - // A method cannot have more than 255 parameters (including the implicit "this" argument). + // Generate constructor if the parameter list is not too large. // - if(constructorParameterLength(allDataMembers) < 255) + if(isValidMethodParameterList(allDataMembers)) { if(hasRequiredMembers && hasOptionalMembers) { @@ -3032,8 +3010,9 @@ Slice::Gen::TypesVisitor::visitExceptionStart(const ExceptionPtr& p) // // Create constructor that takes all data members plus a Throwable. + // Do this only when the parameter list is not too large. // - if(allDataMembers.size() < 254) + if(isValidMethodParameterList(allDataMembers, 1)) { const string causeParamName = getEscapedParamName(allDataMembers, "cause"); @@ -3112,9 +3091,10 @@ Slice::Gen::TypesVisitor::visitExceptionStart(const ExceptionPtr& p) out << eb; // - // Create constructor that takes all data members plus a Throwable + // Create constructor that takes all data members plus a Throwable. + // Do this only when the parameter list is not too large. // - if(allDataMembers.size() < 254) + if(isValidMethodParameterList(allDataMembers, 1)) { const string causeParamName = getEscapedParamName(allDataMembers, "cause"); @@ -3404,9 +3384,9 @@ Slice::Gen::TypesVisitor::visitStructEnd(const StructPtr& p) out << eb; // - // A method cannot have more than 255 parameters (including the implicit "this" argument). + // Generate constructor if the parameter list is not too large. // - if(constructorParameterLength(members) < 255) + if(isValidMethodParameterList(members)) { vector paramDecl; vector paramNames; diff --git a/cpp/src/slice2java/GenCompat.cpp b/cpp/src/slice2java/GenCompat.cpp index 888f62cf7a9..eb76ecbe9cf 100644 --- a/cpp/src/slice2java/GenCompat.cpp +++ b/cpp/src/slice2java/GenCompat.cpp @@ -210,28 +210,6 @@ writeParamList(Output& out, vector params, bool end = true, bool newLine } } -// Returns the "length" of the constructor parameter list from the given data member list. -// All types are counted as 1 unit, except for long and double which are counted as 2 units. -// See https://docs.oracle.com/javase/specs/jvms/se20/html/jvms-4.html#jvms-4.3.3 -int -constructorParameterLength(const DataMemberList& members) -{ - int length = 0; - for(DataMemberList::const_iterator i = members.begin(); i != members.end(); ++i) - { - BuiltinPtr builtin = BuiltinPtr::dynamicCast((*i)->type()); - if(builtin && (builtin->kind() == Builtin::KindLong || builtin->kind() == Builtin::KindDouble)) - { - length += 2; - } - else - { - length++; - } - } - return length; -} - } Slice::JavaCompatVisitor::JavaCompatVisitor(const string& dir) : @@ -2967,9 +2945,9 @@ Slice::GenCompat::TypesVisitor::visitClassDefStart(const ClassDefPtr& p) out << eb; // - // A method cannot have more than 255 parameters (including the implicit "this" argument). + // Generate constructor if the parameter list is not too large. // - if(constructorParameterLength(allDataMembers) < 255) + if(isValidMethodParameterList(members)) { DataMemberList baseDataMembers; if(baseClass) @@ -3277,9 +3255,9 @@ Slice::GenCompat::TypesVisitor::visitExceptionStart(const ExceptionPtr& p) } // - // A method cannot have more than 255 parameters (including the implicit "this" argument). + // Generate constructor if the parameter list is not too large. // - if(constructorParameterLength(allDataMembers) < 255) + if(isValidMethodParameterList(members)) { if(hasRequiredMembers && hasOptionalMembers) { @@ -3341,8 +3319,9 @@ Slice::GenCompat::TypesVisitor::visitExceptionStart(const ExceptionPtr& p) // // Create constructor that takes all data members plus a Throwable. + // Do this only when the parameter list is not too large. // - if(allDataMembers.size() < 254) + if(isValidMethodParameterList(allDataMembers, 1)) { const string causeParamName = getEscapedParamName(allDataMembers, "cause"); paramDecl.push_back("Throwable " + causeParamName); @@ -3419,9 +3398,10 @@ Slice::GenCompat::TypesVisitor::visitExceptionStart(const ExceptionPtr& p) out << eb; // - // Create constructor that takes all data members plus a Throwable + // Create constructor that takes all data members plus a Throwable. + // Do this only when the parameter list is not too large. // - if(allDataMembers.size() < 254) + if(isValidMethodParameterList(allDataMembers, 1)) { const string causeParamName = getEscapedParamName(allDataMembers, "cause"); paramDecl.push_back("Throwable " + causeParamName); @@ -3692,9 +3672,9 @@ Slice::GenCompat::TypesVisitor::visitStructEnd(const StructPtr& p) out << eb; // - // A method cannot have more than 255 parameters (including the implicit "this" argument). + // Generate constructor if the parameter list is not too large. // - if(constructorParameterLength(members) < 255) + if(isValidMethodParameterList(members)) { vector paramDecl; vector paramNames; From ed0141cf9f289381368c167cc285473d6d184f70 Mon Sep 17 00:00:00 2001 From: Joe George Date: Mon, 9 Oct 2023 13:47:25 -0400 Subject: [PATCH 7/7] Update cpp/src/Slice/JavaUtil.h Co-authored-by: Austin Henriksen --- cpp/src/Slice/JavaUtil.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/Slice/JavaUtil.h b/cpp/src/Slice/JavaUtil.h index 431ef018966..d9c6e440d0c 100644 --- a/cpp/src/Slice/JavaUtil.h +++ b/cpp/src/Slice/JavaUtil.h @@ -36,7 +36,7 @@ computeSerialVersionUUID(const StructPtr&); // See https://docs.oracle.com/javase/specs/jvms/se20/html/jvms-4.html#jvms-4.3.3 // bool -isValidMethodParameterList(const DataMemberList &, int additionalUnits = 0); +isValidMethodParameterList(const DataMemberList&, int additionalUnits = 0); class JavaOutput : public ::IceUtilInternal::Output {