From d471c874e35b380cf3d576540503b0ac8de102d2 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Thu, 13 Jun 2024 09:40:59 -0700 Subject: [PATCH 01/32] Turn on 'CheckRequired' option on upb decode PiperOrigin-RevId: 643028693 --- rust/upb/wire.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/rust/upb/wire.rs b/rust/upb/wire.rs index aa335cc020..e8fa533902 100644 --- a/rust/upb/wire.rs +++ b/rust/upb/wire.rs @@ -26,6 +26,14 @@ pub enum DecodeStatus { } // LINT.ThenChange() +#[repr(i32)] +enum DecodeOption { + AliasString = 1, + CheckRequired = 2, + ExperimentalAllowUnlinked = 4, + AlwaysValidateUtf8 = 8, +} + /// If Err, then EncodeStatus != Ok. /// /// SAFETY: @@ -67,11 +75,13 @@ pub unsafe fn decode( ) -> Result<(), DecodeStatus> { let len = buf.len(); let buf = buf.as_ptr(); + let options = DecodeOption::CheckRequired as i32; + // SAFETY: // - `mini_table` is the one associated with `msg` // - `buf` is legally readable for at least `buf_size` bytes. // - `extreg` is null. - let status = upb_Decode(buf, len, msg, mini_table, std::ptr::null(), 0, arena.raw()); + let status = upb_Decode(buf, len, msg, mini_table, std::ptr::null(), options, arena.raw()); match status { DecodeStatus::Ok => Ok(()), _ => Err(status), From bf306b3bea5b3d11af56c77ac8f85c2feae88c52 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Thu, 13 Jun 2024 10:06:03 -0700 Subject: [PATCH 02/32] Update the comment for ctype to note that it's deprecated. New code should prefer `features.(pb.cpp).string_type`. We can't use the `deprecated=true` option for now due to failing tests. PiperOrigin-RevId: 643036543 --- src/google/protobuf/descriptor.proto | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/google/protobuf/descriptor.proto b/src/google/protobuf/descriptor.proto index 1c6115de8b..dfabac4168 100644 --- a/src/google/protobuf/descriptor.proto +++ b/src/google/protobuf/descriptor.proto @@ -635,13 +635,14 @@ message MessageOptions { } message FieldOptions { + // NOTE: ctype is deprecated. Use `features.(pb.cpp).string_type` instead. // The ctype option instructs the C++ code generator to use a different // representation of the field than it normally would. See the specific // options below. This option is only implemented to support use of // [ctype=CORD] and [ctype=STRING] (the default) on non-repeated fields of - // type "bytes" in the open source release -- sorry, we'll try to include - // other types in a future version! - optional CType ctype = 1 [default = STRING]; + // type "bytes" in the open source release. + // TODO: make ctype actually deprecated. + optional CType ctype = 1 [/*deprecated = true,*/ default = STRING]; enum CType { // Default mode. STRING = 0; From d5bd5b90daeb502540cd1bb6c17b3d5362655587 Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Thu, 13 Jun 2024 10:06:47 -0700 Subject: [PATCH 03/32] Fixed a bug with tree shaking: use a separate MiniTable for statically tree shaken messages. Since statically tree shaken messages can never later become linked, we should not need to use any of the special code in the decoder. By using a distinct "empty" message type, we avoid triggering any of this special behavior. This avoids bugs around hazzers and other presence checks. Also fixed a bug in the cmake staleness test that was causing test failures. PiperOrigin-RevId: 643036818 --- upb/mini_table/internal/message.c | 22 +++++++++++++++++++++- upb_generator/bootstrap_compiler.bzl | 9 ++++++++- upb_generator/protoc-gen-upb_minitable.cc | 6 +++--- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/upb/mini_table/internal/message.c b/upb/mini_table/internal/message.c index f278b36577..2f23df4118 100644 --- a/upb/mini_table/internal/message.c +++ b/upb/mini_table/internal/message.c @@ -14,7 +14,12 @@ // Must be last. #include "upb/port/def.inc" -// A MiniTable for an empty message, used for unlinked sub-messages. +// A MiniTable for an empty message, used for unlinked sub-messages that are +// built via MiniDescriptors. Messages that use this MiniTable may possibly +// be linked later, in which case this MiniTable will be replaced with a real +// one. This pattern is known as "dynamic tree shaking", and it introduces +// complication because sub-messages may either be the "empty" type or the +// "real" type. A tagged bit indicates the difference. const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_Empty) = { .UPB_PRIVATE(subs) = NULL, .UPB_PRIVATE(fields) = NULL, @@ -25,3 +30,18 @@ const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_Empty) = { .UPB_PRIVATE(table_mask) = -1, .UPB_PRIVATE(required_count) = 0, }; + +// A MiniTable for a statically tree shaken message. Messages that use this +// MiniTable are guaranteed to remain unlinked; unlike the empty message, this +// MiniTable is never replaced, which greatly simplifies everything, because the +// type of a sub-message is always known, without consulting a tagged bit. +const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_StaticallyTreeShaken) = { + .UPB_PRIVATE(subs) = NULL, + .UPB_PRIVATE(fields) = NULL, + .UPB_PRIVATE(size) = sizeof(struct upb_Message), + .UPB_PRIVATE(field_count) = 0, + .UPB_PRIVATE(ext) = kUpb_ExtMode_NonExtendable, + .UPB_PRIVATE(dense_below) = 0, + .UPB_PRIVATE(table_mask) = -1, + .UPB_PRIVATE(required_count) = 0, +}; diff --git a/upb_generator/bootstrap_compiler.bzl b/upb_generator/bootstrap_compiler.bzl index 4a3bd8ccdd..e92fbafe75 100644 --- a/upb_generator/bootstrap_compiler.bzl +++ b/upb_generator/bootstrap_compiler.bzl @@ -126,7 +126,14 @@ def _cmake_staleness_test(name, base_dir, src_files, proto_lib_deps, **kwargs): name = name + "_copy_gencode_%d" % genrule, outs = ["generated_sources/" + src], srcs = [name, name + "_minitable"], - cmd = "mkdir -p $(@D); for src in $(SRCS); do cp -f $$src $(@D) || echo 'copy failed!'; done", + cmd = """ + mkdir -p $(@D) + for src in $(SRCS); do + if [[ $$src == *%s ]]; then + cp -f $$src $(@D) || echo 'copy failed!' + fi + done + """ % src[src.rfind("/"):], ) # Keep bazel gencode in sync with our checked-in sources needed for cmake builds. diff --git a/upb_generator/protoc-gen-upb_minitable.cc b/upb_generator/protoc-gen-upb_minitable.cc index 2ed83dd1c1..dbc959f82f 100644 --- a/upb_generator/protoc-gen-upb_minitable.cc +++ b/upb_generator/protoc-gen-upb_minitable.cc @@ -367,8 +367,8 @@ void WriteMessage(upb::MessageDefPtr message, const DefPoolPair& pools, IsCrossFile(field)) { if (seen.insert(pools.GetMiniTable64(field.message_type())).second) { output( - "__attribute__((weak)) const upb_MiniTable* $0 = " - "&UPB_PRIVATE(_kUpb_MiniTable_Empty);\n", + "__attribute__((weak)) const upb_MiniTable* $0 =" + " &UPB_PRIVATE(_kUpb_MiniTable_StaticallyTreeShaken);\n", MessagePtrName(field.message_type())); } } @@ -571,7 +571,7 @@ void WriteMiniTableSourceIncludes(upb::FileDefPtr file, Output& output) { output( "extern const struct upb_MiniTable " - "UPB_PRIVATE(_kUpb_MiniTable_Empty);\n"); + "UPB_PRIVATE(_kUpb_MiniTable_StaticallyTreeShaken);\n"); } void WriteMiniTableSource(const DefPoolPair& pools, upb::FileDefPtr file, From 56da5eecf62e748959d0ade4b0106718ae4936e1 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Thu, 13 Jun 2024 17:22:42 +0000 Subject: [PATCH 04/32] Auto-generate files after cl/643036543 --- csharp/src/Google.Protobuf.Test/testprotos.pb | Bin 454059 -> 454107 bytes .../Reflection/Descriptor.pb.cs | 5 +++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/csharp/src/Google.Protobuf.Test/testprotos.pb b/csharp/src/Google.Protobuf.Test/testprotos.pb index fa7127004038113c9ef63c62ae7fb87b4d5ed1e0..97f279d92ea0286466fea7ed16a9202c87819148 100644 GIT binary patch delta 10631 zcmY*fdvsORna@7=?7h!9H#fPtHwln{B)lJyhn;bJfR2K-%TA>ootZT{Lq*IKrcTQM zw%X~eTNo)4q!A08NSst11$BtP0=!0-uR+lOK`gdw zVwQ3HU9UX&zeM1=#ZFPiy_Y-JRc%Ss+&JUr>#nGoz3`rS^)+)B)Z9@&Z+`vkSqtm$ zm{9X~3+iib`q-y)ip$_QJbn-FfFdg~o#*6>y5gRiQ*s5KN}+SHyiU zFRQdLL2yu&g&x7`=@!b2JLz6Nby`qDNxDTDv4lGh@`V2G7QxsOE{lVqfuPUC!qV{A z239+KDR+aQG>}^CK+aPo!J32_2*t+y_*T}$ZffMN7mZ{X*~pW{aHuT;ujFn;G{Au- z1nF3yMr6x_eYz0%zBXUEk?m(w*K)_Qz&OM_S!xnvG>WG!3k)o*B^K6lRZN&%pC7ZC zJ?!Mx$2J@=xt=G>j17upJcJL*$@P55B^H9o^?b}^LnO$;Z)4T`*7i901PE-82~5CgJ5QIJ^b(}R2Uso=NWPs9 z8fU*E`F1{HY_O3j4`|Rm;X}`}&A0A}9k_6?CwAb%!5*F-YD~Cru!j#DVIdss;Uh1# z5Dxb6Nq-r9q#eLiXZXYm?2{jN#tt}1V+R}#I(gc5K8J%&KI9@}k;6eJA3DiGIOyc# zCI(ll1keaw+^MUY9DaU1yCkf6kp;IxV4F?AQ&;Rsz*ASum4K%%K46eBE#Rq(SB&k)aWW*uvd>H=vCH7kG5SNu^>N(Vdhq+s28V?w5Ah2^00(8z{K5$T^fdNMfHK?s2 zh&8Auj}&SE|B1VUjE|h-(5!-_Jn~BI`6p_vVnscY>E({?EDx;&Xo>ewm|mW=HT7Vt zmsi`S@xV(jA3Zi`b;uN2N_@(luXv*BVW;NiIasRmSG~e&S?*IV2bi1^#KLFX9c&>tl#*WL5|rXe?vAi5KmZj;hDJ{GkYU3k z73A`HVOTUFA&A3D!06|NVS%bsad8q3%`gaTaUg)kDHJCcIYFD9E>xj813|39p>jH| zXM&cB zVeWp$JVbPui6%fUOI>%FfLImM-wr#S??6YUU<0?&9$6*VxrpFBXxB zBup$8$qK_=5+)XlO1latVPdhU9Ue?qRASd$BAhUs@j9E9St7t;CsX;V{c>6O`0MPQ z>B~TXNe^O}UnU~vAb^8qqGEJB0xp(`apQwu8cP_kB7A8FyJz}}LQ4#mRuozy=2jG1 zBIZ_*rC%6JXsMOqf4sr&zHViqB?mQFDWV!MHQ85+@;D|3mR6Fbmuz|vtO+lBlf9B# zQ)nuIpw<)yMXSIXQDIh+1aoL(UL!`2G36}IxJ|;DXxJ`BIRR}si(#!vMCGItZj-38 zbD$WOnnbE@LNG&>GnH|h!$Z5+_rtf^nYgk!wv&RLW|6c-Ou~ zG!(qtXgnGo*~zZ_{-Yw9G8$CALI@27iv!0^WyIs6sMVbGDgMU=PWsfwDcTR#31@g% z+r=&oTXr!q1yECnk4Sx;ZD&BJuCwh7h}74K+F^>p6wT46h`}FS`BZFy`s^u79>&lh zOCBIBJY~sCfxHbw-aq^x#Szv9%ODWc05yYv$Zi8MNZFNS#70rnVX`a1$c@6w2Dc%* z63T8%nC@V=Pu*f;f)I|j2y=J_gvu6?*2^CtV%j1u8X6p>14Rj?)fR5-V8{NkEjB?5 zPg`t)(rOdt@C6l^XcOk}1&9T{O_;+MAhw7$fy38ORI>#2e<3`%ht*Da!PdV-)xQuT zUj^T!oxX%N&+YlE-eNbfncL%-Xd-TpW1_&fi?lvdLI*Kz7nfh5`UMa%Z5LPGs5T*j z>hqKzsIDlX!8>eMTnmVq1+3cw5JlZ#+X4`tcG$K6RLZ6;N*t)RtBG|D{!bvb0SI<>+vX_Eh_^&huW61_jC@OY z1B0)qw58M?onhl2S>Hc*+MpnWr%p>FAapuyX#o*br!6fYXzaA51w?5(skAh=N~r=} zmc?m+$R1d;2#D;vEQ^3@wAm{F;i!uy*E2d+WYrVS>|~p6>4{BH$$R2h$zhK$2j@~% z@*aU*lol00#M&cj^%V{fvG$0%$-(zjtZ;WQoU@yHw$SN}_(&9;H-%1Pi@{l21q`B5wqpQcZ+?KN!dX!>Sy2jzDx4ML zCn*loG_(6Ohxk7t?Tfw8S+vh`h_CR{XE_9fmp;p38Ugj|$&w}x`z?n+Py^H)0>WXx z<*+OxE{LK(Nc|#IhLIP9R}s8Jxt9@%4bOJ7ciGGaX?hq!cxsSlDFuX1gEZ?PAcAU; zCUroh-XKltfJnVTBK6Oy!^`M3rcpY-4=??Iot)Mv&7B2ua{YibcNT@=1_FIRRsy8) zfW)0;1kNfXz<`zEoC9psu$9u)$3rE?xRtVUK=4;)9PoqTrUUHq+=J3BQKu*xM|F5m zmSxnLw+zVPI`4wv`}KTXms7=ZB8B+;uh8_Nha%kc0n zcEhy*Fw`6yqG{bM&4~tIVW?S}6AhqJH7A-f6u3o(Wjop5|3w+~Qj0X_8X)Ka>Rtjw z>@5MkR_ScYKXwEgSgutDM6;j|ROn}< zJ51FVfT?1%M(UVzTnwM9Mg{izO)7L~m zr8{c02@0yuNymPq?EuhPqPMZ<;-F|dcuwZPH*Hkl1g4&g*~Z{D>Dc5kfS@%+uV8I) zP;{ejlV!Fv^a|ES&7kXs$~-R}J&Q9AT8^N#^c{NUdOnVduKLf*jDB{)Xqb9lju;tq zQ+H=GVuvhxH9zGTd!OZY;7v*`6F7yyBO z9#8_*^G#`<2S|Vc?b6lVsjwF41|~s{9$?yKX*zNOyt7b+Y(NmJ&^oiTPy^ZRlI}p& z@nn;#3xQrbl>i#MXz8S9fGpAw9n$HNE^3zJBmkR`Xf^@Cb%#vqwg-f{4moJFsxTnj zbx7O@OHpt@kF3>ofi+qQNKl3sP-$Kf2xr0DTM=(0fW^Jiy(mZrIjvHe5pPR(pX53T zj46jwr*yB21yI`mEdjE6N4ok4fh-zPyeFLxqUzB?@Sc>VhFEnke?KmkzJI@;ljaEt z2Gk?s`_eojk^m;UrFrjGSPS%T1WJGyx~2JzfCRuj(k(T$&=+b5^zRR9fEt0{A9O8p z2>XC^j;Uf0pR~k5U~579(*e68;VS|-KuadgHbB(k0L>9y3+zuYHtbkb3r4j#7}tWS zS{x$kX}yGjoGQj)T@0!P^H7T;()}>11yi*+N=zFiuN-0YO7|L_1ijmSES=9H>WR0H zr5tRiSI@VfNOz23ML*v{fQ3CrVYvGJ_7jOENHq}zFmXI45?Bi?kr05$<5CfcxS;|3 zX*_ZC^DP8+;y|EAsHsNN08yWiIp?d0dfKv2#MIN4eL^Pn5gfx2zzJDEteb-bw0tdbY1Y)^O&G6&G4=Y<7y?VZ zel-3jrd~Z7_s8=@zZpYd=LrPx(NFV4^{q?0(|K}HlA|>P7_ z=MD0{Af5X>TEl22kVOdWw4qbd1!>-o@D&hY^4uk!UBVKocE9y7OZ2pX06I&(XvK0f zq9Nxwzk^#%3vH$ip844$>$sdo2!(Ov+o1$Hlh z02=puYA+z>bC@`IE5wf%hI&2qAke?^krO=vzshOq5q6{Jgc0?0K!CtfPxr+}&;0O_ zRfnWTuUh}61PJOIy$KWHjmqZcKz*Y(DU7IhiTVekq#ajLzcQvC0_5~yOub9g|K4*~ zN7UoDa4@&VbN@z%j5CXP$aDTXBASjG4|x(9>Hg!y^xBwMV2pr3UsJL!1fXfHXMS2C z0R}XC=H8*O7Wjro41h+nSDZ4n;Y8!ZUNrN7p^<|`KmC&pJp@nxnrVdH;yF)7Ow;)Z z0?RZ9(=DF4Q)C@Vs>Q3+haNyM-QwZUqnXC9h%h$l$%tr9ME@ZdGtCv#k9batRo#)Pobg1jsd+xRBZxyn5Gke-|O4X?@>le)>m@Lr~ zK)sz6LU=PU;by_c2G4mBY|x1?OBb3AUP{0C1HlWRzR&V~1I;grYho7Ei zBkNv{HE3!8S`8}K%cOCI(Rd}C`URUZ@f9zX(c!6@7eZ(#xWU1(vK*&Jujp|8$uHP= zmVLwXs)9{)X)32n({9h{%J+WBCbIh79)202>5323b{&>*g;8M$^q!>zsBwp9el<`6 zMBU+;-&>Rbb?%@g;6`H;1NMf$I?H}IeQ%*9I!WyHQo5r_k@DZ`4XjgzQUX}o>rI># z{LolJe*40buh`$__7z&9jd5S0C2Hh--oR0jCE6VKd1J>Z-e4L<-%m@7N!v%-`#rPT z(`PE)w*_#m1X?3kM8A^l_pk<0Cz1+9|80R%!3bJQF#nNqqwXWVM7x)Y;La1g+2S$;S3!xc%Afq2-RsR<<(mv4h3Qqkx#$AI^eVqx|7s5i)2n>7 ztH1>H?`q%q$k*ExFh;NTarJMIXcS;_(wcue;oP6dwfd%el^heV)yMpyswn|fTm7tV zLM4E!Rv*i1Gy-odt^VNgxEpv{g?>S8^@ofL{^%HYFtXKm>#AOLYOcBSm$U9!P%~%N zf|~lf@ux`n9_dVFbMRTuA@V^t@~wqYP%*X^MnUD+S{Mbr^lUARf>9J(DT*hGO%xCu z%`ftu(IR)W&>dadkNW1FgZxwBj{4>LmoO!OyQ4m??$nzk0PgkOaS;i0nt>orf$s0U zg*NF#)9aVh>Mq@!vZ0KO85P4VS#$?wD0^b&^-r?(Wm{OJZPlfA%V%q znQ%_VS^1MQHYp`WQfF+jl>nM&Y_XL9p3c}}D*=jq#ui%(P}?(9>>0)xY@7>k%Q`=r zey-3Nb=En{AZb!(o%74}A4@0!oSpOW#}f3LE(u_{KRlCl?zyhN&=NIGe_`0vH2r>7 z-|CejEcN?%ed@LhK(Hv>Qto_k{i48>l?GC;EDFq|Qv#S;6lC==LkUPTz%k=rbx`V$ z=>qruu&ctEn_Uw4Wx;nb`!G~LdMpiZs&u|LYH1*Xpe*q12c)tzz$!i^8Vg6u!dEMu zt41vggkLb0HrHhV&O(<(V!WWiXXXn9bgFYXjB`gTQNUI7)>0^P_; zfX-SGr1al{C;0fbi&a5~lp=qRXDr{kmcsjaPJnL?Kp3;3X` zT^E!hAQOXt2U4R>r=XVbP{50}qn0&jgdkl6f|3HF1RNBF0VEoR>F?W*b1r|d7QgT8 z@Adn>{k-Yvsae-kGj9v~XTf$A{zb5_#|J&z6M@X|#**g3qgSz8<~00Vupk*E+)|ct zFS=eXcs3EZZmCm}ai8SQ71d8B3M1FC8(F??+_<{gGiS}3+f?`1w2==!ST|?Z+`2h4 z>t;Okz=Mzc?X;k7=A7x%X4TD^`|z~c5Z2va_tQt_JThxq-J{cH&7Lvyq52?51)MCq z%5($;!DNN~ia4I+=_(5o1P4_Ie_)h7!ZqXUXGyTJDJY|$-I9!0z@240p+CDNFt&ip z(x5gF^qI0+7!Fv^>W3}lZV;3QQi~nPd8#aUE@1{jvAJ;XdbWz)-OODt8p$xSnI}u( zP+J6U;cjI#z=0+N6|q2#$mW7BO)S2zF5I$_?Prr$bH}p4IK(_zZt`L@idR?`7+6?M zEVOb}9GF~N`19A;^G<$kY{LPQYk4wlY)~X?d1j!6u(6ic47LzNuH{3nv=BtD<)bDV zB3VYf%$-dUkrdC%T&9ANDs4tYzRKN9bvSzq8_|IOrcA>4gy>%773pBK84KO^@U1Ou zY(snTIHIz>cpUJ?P;O;o>o*pUb4X6(RtGA1Eq_jbg|CqQ6FOke^|J9tIT zq?aHiKEQI3K=K`Y&=~s_$#?MKql1l1c|e2i4nKH{y>{>J*ntZNyJH6~9PH*5wZ?=C z2fKO0a0}sJHy?48g>bN&PxyZDxpn|kUE%)!VkiH)D|Wz18av={(8VjNjR_71U3|z; z3*n%P*G{ky4!Zc5@xcu$0W?B4cbcjvhNo|3SB7`~k_GodV4F?AQ+Mo1z*Be3m4K&i zK46eBE#Rq}SC6$2p1S#nF+r$P0X!IP{Uuv{-@({{goA^z0|^HQd4+8~2?qyxZN0H5 z;ou-|_@0Gik&nAF*r6P_PDUKz&Szo!+w52QBV1OQspn7+e!|^q(|Eve1A(235TJ8D z;R6Rn8W?c2ScBRcf>?uk@@TOJ@Tc4zWPIcuhh`NdxyUQE=cm+KrHXna)5{&(Ssq#m z&=T*VFugo!YwE#PFR!sp8&K2kGXWaALt%>OLp#ecvEP%#&+5ymId2FIk=;O|4GiQ8o2WZ>O zN0xm&S!2J#SRWre%tDl=k2j7&xahJ3_RR0N^IN;rC#ZkE^mw3{yWC3hl;?;I;2C#IAr^byBCaam7 z%((MIwu9Yp!+a5$NW#Q?k*qY_C1GN|sIseY5+>%0`eDITMJ0C41;TkQ%EiJZ0%q=apM9eKEOOF~$XsMQP-TUnEn_G%4IjBL4h-$#pWN#6{TFBDd zHa!SdhSNLPyZM#HrVs_h&og{4rWnyv`$P~}Wz+}5z7ll>_C-3Lru*BaYN!A`44+9IZ4r&ZM2 z{U-%Gt>UU{f_vyx3@+NjAM9j*+0-Ud=J>#=NPy;`#c7*q6O{vl!K8ti`C{0ylMQWp zQQ(}Yj;%gv6hmkzc#Y9`Da`C**Zt@vkxUs4Dqk^#hJvMmGYE+6))9l0U0FtK5G9=^ zyE2U2AiQkwH^{DxvU@%J!EW}G$**CQ}r)~$XCG*+Ud(^^W0u|Xb-!M-M2lCi6-LqI3@~wyQt7-O6VY_?c$ni zRlfitrtRXo+tnsSP<@{CW7QR9G>{D=HOheO5P)|i_)S3h**0>y}rT$BGw+!G%@(0iWTk-hpYCp+a?{3?Ngx- zi)5p*K!rXmM(UO-SA{-IEk!G6ITiY-E;Rm6Ahtt=K57e%uds8}78($~kJ>_)Bc)?{ z6)2}dAG3u9f*PPMG$1HBW(!@B5hp~+MN{Ysj65N{DpiLHs`E+Os;`-X|0A-KHZnj& zcG4CB5Q-;lD+403leA%fOqcNr;`e-E=K119u@e-B6*RN^G>7;a|j5B{g%UYMqCmlFG&5ulg7wP!mA9rDfcvyxVSK(hkeBETP#fzLkd@m zrCCb>p|n_Ph288qFaeVzk4rO@90e1Bi0Qd=GKhy%DG^a~^g`ZHG6$21{ zLWyRj>6j+eD#LUqyX__b7;23T(Y$Vz=17CDFw`o|kp@tynj=jb1#Xk!jXT+&{$84z zsZE-L4G{DIbu$4X_BM%a`RN4q={Bf3D2%uv&?jIeKsf8AIc+KdVpuP8Lt+&~zh2hWM>a8FgLH>Q z1E_cq7@K*zB*5qf%7Y&G$S75CgUk$#1u(inVl}2)dxpZ?B%L9ugg`I|sJj6Wy_vls z&C@H!!qgiwt?!9|O1E*O2@0wO>DZ679{^fQ^f*?CgQESQAoJjxb}DcJQ-zpq4BjRk zn>+>(w5I48Y+D=@UFo;Uv@H!igKeW`&~-y)-jt4>#Tf@JN6=dO9=&tD8AnBT{WoPs zzdK1@}87A)sBGF zJMT;L6rwN&K%n0TlmPX7Uz+y;5@0}wbai(stOdG(Nsy-(m=0N95jg?=pjd@$KoG0Y zI`cuX2C~~J-GQp($tG170=;x90W@~f(n;?CS)?I4rPD24)GW_Q05&1fYyyJoPMOqg z4+wLea?nUsVL-U+l(-U>qu_uZS!?PBYqS!OAdM$bX`T@XXTjTt5pN`b#XZs;8dL;% ztx}Z{|0~_SlItWeCI_W1>E0X*ptSEY0kYaJ-6=XNG@|%OI{z0{j~0TDq%1eYs*Cx@ zak2CT{NubdZ%A1e5?K6LnpZ>;z{Ejm9=sLS0{tU_5Bk%)+u0jmN*D(Eogr_WLG47MF59r$)wo^h*}(?IihQU{Rzez zjz_g%RExuLEtsms5u%>fOBl$jVtk^DLA78WYH?J$pGCD`sustHX`|%j5JsK(synQa^U_-rnzx}mzM;TW1`z-`m*z**Is~>QGEwKctCV~JaPR2w6Yk?&a z0uXspDk2d#G=RT|CysuQgfBd=pVmTlT4#dfKv2$)rAmV>kjh zCF`#?)YE!&NjtWEwVqTV6uzly1M6!l-n(*Ock%%6^_cZm8k(mfkd?<>HIdLp5}g^l$C}@^=SO9bn6WD`q3By{Q#MD^N@g+ZzXQcntD76 z<5@7KUcVYcV5!%y#^1%%t5@Uxc%JA-V+ia#fdD@GX`ZOQb!m6HNG{6qv}OQfwqGQR zs>28)h|2}Apago}An!}kdCH?TjAjB^guqT4Iwf6_<^c&`VeOJM4@iKh-6h#H4&K!I zhaZVx?TV*7yM`sGc)yE_mr%u<=h?O2fdD4wduk1%X_KJZJ>j_vJiCM?RPFxN!z|I$ z1_J0T^r98Z&4|T$&-oeLYFcPBUF@0PJ+iJ#8_p8XuB3!900RBmss@0SB_8gHx^^zr z?rAUD3xKu2?gbD)<7rRr1;l(F69kPgSnNS`-eJYoLR&w&v`x~nvNQ)Jc*2S|8Zh^bxbTUMnIsi zDOnc+(6rh!zpan}16n^rt=d7mT3;A+dOlp$U2l%n^&a|J%C`k&BLKbGtEJCo7ebqL^LO&U&zNy zbH((FmkD6%7WA z!7#kkLDhXb9DJ6IXnH%=ps4|9HK<%~lg711QL_C3$54mQ!HDMy#4U7pijs5;NavuV3L{4_$-6(6YWIxXP}qrwpA zJxd8t<4(`~Y@h^)y3;d1xF`YY+(}Eo?Zzes>5e8v%72eH zut^n431DfDH-18Jr?G_m_J-GAV1Ja~TWpCo#=XUssFC-20~;etv^nneMvqav!8D4# zkCqsdwvV*;d1kez&s4n21#qndS|eXVKa=hAum({lk_ttCxj?C41g)hQf!~Yv(@JQ3 z2JpF`f~Mxl5%C8+XQCdg4#0qVV&NkaJm4ku4G56c6CU1VMkYa zwqSBkY=h|T@shfAFop(sS$z`%goz#x*T99^1Y$oDKKw0Pbp4Uo1o3<%HbH)l#3qR2 zBV^(rL-$qU5zQ<4JiO{V_Ho4t5ASDBq!C;tCAz#&+s_tw_bvBLhLFPaa^GBZ0bzQ% zpVL3v1H$xjU+pR|LH)bJcTV_vn*zq@6+W*1OC%Zvn4Gj1nCmP}1RlAR6H5!38mUe&eSlkUftwKMcw);cI1bZFh4n{WnZd3LDICVEZ z_}Kk(XV*=?e|Fup$M1h&PNCFuCbQ}ItmhEG0jO^H2Akwmk`AFoe6mH`Ong`ee|L$}TgOj&6l^~$`!Ogbfisd+(GA2XDI zGy@zn{$2;A{+cdupT>)%Gb6ho@YBI9n0*+kA3YX^PgXfUY+M+KAV>$k{eV;!23W-> zMPuP;QFx}xxxR5xApD}Sw7D({a2C2c8Vkvi@J|Lf*XEZ5B3U$+C|VMf>5Dssi@seN zm{&lBwLmwr5}>n|1}Xi=AWDEhmIioUrAAf)WV|%MBVhkO23I~Xd;0ye9v=VjteJCW e&L00rIP-4Dd#Y-ngMSJ9bMcQCss@T3`~M#%R=&Cb diff --git a/csharp/src/Google.Protobuf/Reflection/Descriptor.pb.cs b/csharp/src/Google.Protobuf/Reflection/Descriptor.pb.cs index 38afae477f..ba3fee586c 100644 --- a/csharp/src/Google.Protobuf/Reflection/Descriptor.pb.cs +++ b/csharp/src/Google.Protobuf/Reflection/Descriptor.pb.cs @@ -8027,12 +8027,13 @@ namespace Google.Protobuf.Reflection { private global::Google.Protobuf.Reflection.FieldOptions.Types.CType ctype_; /// + /// NOTE: ctype is deprecated. Use `features.(pb.cpp).string_type` instead. /// The ctype option instructs the C++ code generator to use a different /// representation of the field than it normally would. See the specific /// options below. This option is only implemented to support use of /// [ctype=CORD] and [ctype=STRING] (the default) on non-repeated fields of - /// type "bytes" in the open source release -- sorry, we'll try to include - /// other types in a future version! + /// type "bytes" in the open source release. + /// TODO: make ctype actually deprecated. /// [global::System.Diagnostics.DebuggerNonUserCodeAttribute] [global::System.CodeDom.Compiler.GeneratedCode("protoc", null)] From 2c6d319e69a9fc9f73e957bdbd732f61a4a7d593 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Thu, 13 Jun 2024 17:25:48 +0000 Subject: [PATCH 05/32] Auto-generate files after cl/643036818 --- php/ext/google/protobuf/php-upb.c | 24 +++++++++++++++++-- ruby/ext/google/protobuf_c/ruby-upb.c | 24 +++++++++++++++++-- .../protobuf/descriptor.upb_minitable.c | 2 +- .../protobuf/descriptor.upb_minitable.c | 2 +- .../protobuf/compiler/plugin.upb_minitable.c | 2 +- 5 files changed, 47 insertions(+), 7 deletions(-) diff --git a/php/ext/google/protobuf/php-upb.c b/php/ext/google/protobuf/php-upb.c index 022fab0104..8a24966067 100644 --- a/php/ext/google/protobuf/php-upb.c +++ b/php/ext/google/protobuf/php-upb.c @@ -487,7 +487,7 @@ void upb_Status_VAppendErrorFormat(upb_Status* status, const char* fmt, // Must be last. -extern const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_Empty); +extern const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_StaticallyTreeShaken); static const upb_MiniTableSubInternal google_protobuf_FileDescriptorSet_submsgs[1] = { {.UPB_PRIVATE(submsg) = &google__protobuf__FileDescriptorProto_msg_init_ptr}, }; @@ -12615,7 +12615,12 @@ char* upb_MtDataEncoder_EndEnum(upb_MtDataEncoder* e, char* ptr) { // Must be last. -// A MiniTable for an empty message, used for unlinked sub-messages. +// A MiniTable for an empty message, used for unlinked sub-messages that are +// built via MiniDescriptors. Messages that use this MiniTable may possibly +// be linked later, in which case this MiniTable will be replaced with a real +// one. This pattern is known as "dynamic tree shaking", and it introduces +// complication because sub-messages may either be the "empty" type or the +// "real" type. A tagged bit indicates the difference. const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_Empty) = { .UPB_PRIVATE(subs) = NULL, .UPB_PRIVATE(fields) = NULL, @@ -12627,6 +12632,21 @@ const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_Empty) = { .UPB_PRIVATE(required_count) = 0, }; +// A MiniTable for a statically tree shaken message. Messages that use this +// MiniTable are guaranteed to remain unlinked; unlike the empty message, this +// MiniTable is never replaced, which greatly simplifies everything, because the +// type of a sub-message is always known, without consulting a tagged bit. +const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_StaticallyTreeShaken) = { + .UPB_PRIVATE(subs) = NULL, + .UPB_PRIVATE(fields) = NULL, + .UPB_PRIVATE(size) = sizeof(struct upb_Message), + .UPB_PRIVATE(field_count) = 0, + .UPB_PRIVATE(ext) = kUpb_ExtMode_NonExtendable, + .UPB_PRIVATE(dense_below) = 0, + .UPB_PRIVATE(table_mask) = -1, + .UPB_PRIVATE(required_count) = 0, +}; + // Must be last. diff --git a/ruby/ext/google/protobuf_c/ruby-upb.c b/ruby/ext/google/protobuf_c/ruby-upb.c index 2ffd880477..9c007d789d 100644 --- a/ruby/ext/google/protobuf_c/ruby-upb.c +++ b/ruby/ext/google/protobuf_c/ruby-upb.c @@ -487,7 +487,7 @@ void upb_Status_VAppendErrorFormat(upb_Status* status, const char* fmt, // Must be last. -extern const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_Empty); +extern const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_StaticallyTreeShaken); static const upb_MiniTableSubInternal google_protobuf_FileDescriptorSet_submsgs[1] = { {.UPB_PRIVATE(submsg) = &google__protobuf__FileDescriptorProto_msg_init_ptr}, }; @@ -12103,7 +12103,12 @@ char* upb_MtDataEncoder_EndEnum(upb_MtDataEncoder* e, char* ptr) { // Must be last. -// A MiniTable for an empty message, used for unlinked sub-messages. +// A MiniTable for an empty message, used for unlinked sub-messages that are +// built via MiniDescriptors. Messages that use this MiniTable may possibly +// be linked later, in which case this MiniTable will be replaced with a real +// one. This pattern is known as "dynamic tree shaking", and it introduces +// complication because sub-messages may either be the "empty" type or the +// "real" type. A tagged bit indicates the difference. const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_Empty) = { .UPB_PRIVATE(subs) = NULL, .UPB_PRIVATE(fields) = NULL, @@ -12115,6 +12120,21 @@ const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_Empty) = { .UPB_PRIVATE(required_count) = 0, }; +// A MiniTable for a statically tree shaken message. Messages that use this +// MiniTable are guaranteed to remain unlinked; unlike the empty message, this +// MiniTable is never replaced, which greatly simplifies everything, because the +// type of a sub-message is always known, without consulting a tagged bit. +const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_StaticallyTreeShaken) = { + .UPB_PRIVATE(subs) = NULL, + .UPB_PRIVATE(fields) = NULL, + .UPB_PRIVATE(size) = sizeof(struct upb_Message), + .UPB_PRIVATE(field_count) = 0, + .UPB_PRIVATE(ext) = kUpb_ExtMode_NonExtendable, + .UPB_PRIVATE(dense_below) = 0, + .UPB_PRIVATE(table_mask) = -1, + .UPB_PRIVATE(required_count) = 0, +}; + // Must be last. diff --git a/upb/cmake/google/protobuf/descriptor.upb_minitable.c b/upb/cmake/google/protobuf/descriptor.upb_minitable.c index 05bece8f65..08fa451f75 100644 --- a/upb/cmake/google/protobuf/descriptor.upb_minitable.c +++ b/upb/cmake/google/protobuf/descriptor.upb_minitable.c @@ -13,7 +13,7 @@ // Must be last. #include "upb/port/def.inc" -extern const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_Empty); +extern const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_StaticallyTreeShaken); static const upb_MiniTableSubInternal google_protobuf_FileDescriptorSet_submsgs[1] = { {.UPB_PRIVATE(submsg) = &google__protobuf__FileDescriptorProto_msg_init_ptr}, }; diff --git a/upb/reflection/cmake/google/protobuf/descriptor.upb_minitable.c b/upb/reflection/cmake/google/protobuf/descriptor.upb_minitable.c index 05bece8f65..08fa451f75 100644 --- a/upb/reflection/cmake/google/protobuf/descriptor.upb_minitable.c +++ b/upb/reflection/cmake/google/protobuf/descriptor.upb_minitable.c @@ -13,7 +13,7 @@ // Must be last. #include "upb/port/def.inc" -extern const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_Empty); +extern const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_StaticallyTreeShaken); static const upb_MiniTableSubInternal google_protobuf_FileDescriptorSet_submsgs[1] = { {.UPB_PRIVATE(submsg) = &google__protobuf__FileDescriptorProto_msg_init_ptr}, }; diff --git a/upb_generator/cmake/google/protobuf/compiler/plugin.upb_minitable.c b/upb_generator/cmake/google/protobuf/compiler/plugin.upb_minitable.c index fab151bf1f..0aaf0cd87b 100644 --- a/upb_generator/cmake/google/protobuf/compiler/plugin.upb_minitable.c +++ b/upb_generator/cmake/google/protobuf/compiler/plugin.upb_minitable.c @@ -14,7 +14,7 @@ // Must be last. #include "upb/port/def.inc" -extern const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_Empty); +extern const struct upb_MiniTable UPB_PRIVATE(_kUpb_MiniTable_StaticallyTreeShaken); static const upb_MiniTableField google_protobuf_compiler_Version__fields[4] = { {1, 12, 64, kUpb_NoSub, 5, (int)kUpb_FieldMode_Scalar | ((int)kUpb_FieldRep_4Byte << kUpb_FieldRep_Shift)}, {2, 16, 65, kUpb_NoSub, 5, (int)kUpb_FieldMode_Scalar | ((int)kUpb_FieldRep_4Byte << kUpb_FieldRep_Shift)}, From 8ef32176e34cf585a635b894c5b6363375963d9e Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Thu, 13 Jun 2024 10:27:50 -0700 Subject: [PATCH 06/32] Expose upb_Message_IsEqual to Rust upb. PiperOrigin-RevId: 643044309 --- rust/upb/BUILD | 1 + rust/upb/lib.rs | 2 +- rust/upb/message.rs | 7 +++++++ rust/upb/upb_api.c | 3 ++- 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/rust/upb/BUILD b/rust/upb/BUILD index 563794994b..83322121d1 100644 --- a/rust/upb/BUILD +++ b/rust/upb/BUILD @@ -45,6 +45,7 @@ cc_library( deps = [ "//upb:mem", "//upb:message", + "//upb:message_compare", "//upb:message_copy", "//upb/mini_table", ], diff --git a/rust/upb/lib.rs b/rust/upb/lib.rs index 643018afe0..9d3490d73a 100644 --- a/rust/upb/lib.rs +++ b/rust/upb/lib.rs @@ -22,7 +22,7 @@ pub use map::{ mod message; pub use message::{ - upb_Message, upb_Message_DeepClone, upb_Message_DeepCopy, upb_Message_New, + upb_Message, upb_Message_DeepClone, upb_Message_DeepCopy, upb_Message_IsEqual, upb_Message_New, upb_Message_SetBaseField, RawMessage, }; diff --git a/rust/upb/message.rs b/rust/upb/message.rs index a4159a1fb6..1aad3a4745 100644 --- a/rust/upb/message.rs +++ b/rust/upb/message.rs @@ -28,4 +28,11 @@ extern "C" { mini_table: *const upb_MiniTableField, val: *const std::ffi::c_void, ); + + pub fn upb_Message_IsEqual( + m1: RawMessage, + m2: RawMessage, + mini_table: *const upb_MiniTable, + options: i32, + ) -> bool; } diff --git a/rust/upb/upb_api.c b/rust/upb/upb_api.c index e6d508db1d..8842aa6d12 100644 --- a/rust/upb/upb_api.c +++ b/rust/upb/upb_api.c @@ -10,11 +10,12 @@ #define UPB_BUILD_API +#include "upb/message/accessors.h" // IWYU pragma: keep #include "upb/mem/arena.h" // IWYU pragma: keep #include "upb/message/array.h" // IWYU pragma: keep +#include "upb/message/compare.h" // IWYU pragma: keep #include "upb/message/copy.h" // IWYU pragma: keep #include "upb/message/map.h" // IWYU pragma: keep -#include "upb/message/accessors.h" // IWYU pragma: keep #include "upb/mini_table/message.h" // IWYU pragma: keep const size_t __rust_proto_kUpb_Map_Begin = kUpb_Map_Begin; From 091492ede1fc65a89299045eda72d4614fc4ca6d Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Thu, 13 Jun 2024 10:47:51 -0700 Subject: [PATCH 07/32] Change MapEntry instances to also generate a TDP table. PiperOrigin-RevId: 643051561 --- src/google/protobuf/compiler/cpp/helpers.cc | 5 ++-- src/google/protobuf/compiler/cpp/message.cc | 17 +++--------- src/google/protobuf/map_entry.h | 29 ++++++++++++++++----- 3 files changed, 27 insertions(+), 24 deletions(-) diff --git a/src/google/protobuf/compiler/cpp/helpers.cc b/src/google/protobuf/compiler/cpp/helpers.cc index 047a52709e..78e2832771 100644 --- a/src/google/protobuf/compiler/cpp/helpers.cc +++ b/src/google/protobuf/compiler/cpp/helpers.cc @@ -215,7 +215,7 @@ internal::field_layout::TransformValidation GetLazyStyle( absl::flat_hash_map MessageVars( const Descriptor* desc) { - absl::string_view prefix = IsMapEntryMessage(desc) ? "" : "_impl_."; + absl::string_view prefix = "_impl_."; return { {"any_metadata", absl::StrCat(prefix, "_any_metadata_")}, {"cached_size", absl::StrCat(prefix, "_cached_size_")}, @@ -540,8 +540,7 @@ std::string FieldName(const FieldDescriptor* field) { } std::string FieldMemberName(const FieldDescriptor* field, bool split) { - absl::string_view prefix = - IsMapEntryMessage(field->containing_type()) ? "" : "_impl_."; + absl::string_view prefix = "_impl_."; absl::string_view split_prefix = split ? "_split_->" : ""; if (field->real_containing_oneof() == nullptr) { return absl::StrCat(prefix, split_prefix, FieldName(field), "_"); diff --git a/src/google/protobuf/compiler/cpp/message.cc b/src/google/protobuf/compiler/cpp/message.cc index 61ace77f73..37af63340a 100644 --- a/src/google/protobuf/compiler/cpp/message.cc +++ b/src/google/protobuf/compiler/cpp/message.cc @@ -1286,6 +1286,7 @@ void MessageGenerator::GenerateMapEntryClassDefinition(io::Printer* p) { &_$classname$_default_instance_); } )cc"); + parse_function_generator_->GenerateDataDecls(p); p->Emit(R"cc( const $superclass$::ClassData* GetClassData() const PROTOBUF_FINAL; static const $superclass$::ClassDataFull _class_data_; @@ -2155,6 +2156,7 @@ void MessageGenerator::GenerateClassMethods(io::Printer* p) { $verify$; $class_data$; )cc"); + parse_function_generator_->GenerateDataDefinitions(p); return; } if (IsAnyMessage(descriptor_)) { @@ -3720,19 +3722,6 @@ void MessageGenerator::GenerateClassData(io::Printer* p) { {"is_initialized", is_initialized}, {"pin_weak_descriptor", pin_weak_descriptor}, {"custom_vtable_methods", custom_vtable_methods}, - {"table", - [&] { - // Map entries use the dynamic parser. - if (IsMapEntryMessage(descriptor_)) { - p->Emit(R"cc( - nullptr, // tc_table - )cc"); - } else { - p->Emit(R"cc( - &_table_.header, - )cc"); - } - }}, {"tracker_on_get_metadata", [&] { if (HasTracker(descriptor_, options_)) { @@ -3752,7 +3741,7 @@ void MessageGenerator::GenerateClassData(io::Printer* p) { const ::$proto_ns$::MessageLite::ClassDataFull $classname$::_class_data_ = { $superclass$::ClassData{ - $table$, + &_table_.header, $on_demand_register_arena_dtor$, $is_initialized$, &$classname$::MergeImpl, diff --git a/src/google/protobuf/map_entry.h b/src/google/protobuf/map_entry.h index 16145c9b04..71dda48b9f 100644 --- a/src/google/protobuf/map_entry.h +++ b/src/google/protobuf/map_entry.h @@ -95,9 +95,9 @@ class MapEntry : public Message { ~MapEntry() PROTOBUF_OVERRIDE { if (GetArena() != nullptr) return; - Message::_internal_metadata_.template Delete(); - KeyTypeHandler::DeleteNoArena(key_); - ValueTypeHandler::DeleteNoArena(value_); + this->_internal_metadata_.template Delete(); + KeyTypeHandler::DeleteNoArena(_impl_.key_); + ValueTypeHandler::DeleteNoArena(_impl_.value_); } using InternalArenaConstructable_ = void; @@ -107,14 +107,29 @@ class MapEntry : public Message { return Arena::Create(arena); } + struct _Internal; + protected: friend class google::protobuf::Arena; - HasBits<1> _has_bits_{}; - mutable CachedSize _cached_size_{}; + // Field naming follows the convention of generated messages to make code + // sharing easier. + struct { + HasBits<1> _has_bits_{}; + mutable CachedSize _cached_size_{}; + + KeyOnMemory key_{KeyTypeHandler::Constinit()}; + ValueOnMemory value_{ValueTypeHandler::Constinit()}; + } _impl_; +}; - KeyOnMemory key_{KeyTypeHandler::Constinit()}; - ValueOnMemory value_{ValueTypeHandler::Constinit()}; +template +struct MapEntry::_Internal { + static constexpr ::int32_t kHasBitsOffset = + 8 * PROTOBUF_FIELD_OFFSET(MapEntry, _impl_._has_bits_); }; } // namespace internal From ce7446a2c489dc7582d760195e2c2ab2186e5b46 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Thu, 13 Jun 2024 18:01:41 +0000 Subject: [PATCH 08/32] Auto-generate files after cl/643051561 --- src/google/protobuf/struct.pb.cc | 53 +++++++++++++++++++++++++++++--- src/google/protobuf/struct.pb.h | 5 +++ 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/src/google/protobuf/struct.pb.cc b/src/google/protobuf/struct.pb.cc index af5522d6a2..86f1aeb854 100644 --- a/src/google/protobuf/struct.pb.cc +++ b/src/google/protobuf/struct.pb.cc @@ -127,7 +127,7 @@ static constexpr const ::_pb::ServiceDescriptor** const ::uint32_t TableStruct_google_2fprotobuf_2fstruct_2eproto::offsets[] ABSL_ATTRIBUTE_SECTION_VARIABLE( protodesc_cold) = { - PROTOBUF_FIELD_OFFSET(::google::protobuf::Struct_FieldsEntry_DoNotUse, _has_bits_), + PROTOBUF_FIELD_OFFSET(::google::protobuf::Struct_FieldsEntry_DoNotUse, _impl_._has_bits_), PROTOBUF_FIELD_OFFSET(::google::protobuf::Struct_FieldsEntry_DoNotUse, _internal_metadata_), ~0u, // no _extensions_ ~0u, // no _oneof_case_ @@ -135,8 +135,8 @@ const ::uint32_t ~0u, // no _inlined_string_donated_ ~0u, // no _split_ ~0u, // no sizeof(Split) - PROTOBUF_FIELD_OFFSET(::google::protobuf::Struct_FieldsEntry_DoNotUse, key_), - PROTOBUF_FIELD_OFFSET(::google::protobuf::Struct_FieldsEntry_DoNotUse, value_), + PROTOBUF_FIELD_OFFSET(::google::protobuf::Struct_FieldsEntry_DoNotUse, _impl_.key_), + PROTOBUF_FIELD_OFFSET(::google::protobuf::Struct_FieldsEntry_DoNotUse, _impl_.value_), 0, 1, ~0u, // no _has_bits_ @@ -249,7 +249,7 @@ bool NullValue_IsValid(int value) { const ::google::protobuf::MessageLite::ClassDataFull Struct_FieldsEntry_DoNotUse::_class_data_ = { ::google::protobuf::Message::ClassData{ - nullptr, // tc_table + &_table_.header, nullptr, // OnDemandRegisterArenaDtor nullptr, // IsInitialized &Struct_FieldsEntry_DoNotUse::MergeImpl, @@ -259,7 +259,7 @@ bool NullValue_IsValid(int value) { ::google::protobuf::Message::ClearImpl, ::google::protobuf::Message::ByteSizeLongImpl, ::google::protobuf::Message::_InternalSerializeImpl, #endif // PROTOBUF_CUSTOM_VTABLE - PROTOBUF_FIELD_OFFSET(Struct_FieldsEntry_DoNotUse, _cached_size_), + PROTOBUF_FIELD_OFFSET(Struct_FieldsEntry_DoNotUse, _impl_._cached_size_), false, }, &Struct_FieldsEntry_DoNotUse::kDescriptorMethods, @@ -271,6 +271,49 @@ bool NullValue_IsValid(int value) { ::google::protobuf::internal::PrefetchToLocalCache(_class_data_.tc_table); return _class_data_.base(); } +PROTOBUF_CONSTINIT PROTOBUF_ATTRIBUTE_INIT_PRIORITY1 +const ::_pbi::TcParseTable<1, 2, 1, 46, 2> Struct_FieldsEntry_DoNotUse::_table_ = { + { + PROTOBUF_FIELD_OFFSET(Struct_FieldsEntry_DoNotUse, _impl_._has_bits_), + 0, // no _extensions_ + 2, 8, // max_field_number, fast_idx_mask + offsetof(decltype(_table_), field_lookup_table), + 4294967292, // skipmap + offsetof(decltype(_table_), field_entries), + 2, // num_field_entries + 1, // num_aux_entries + offsetof(decltype(_table_), aux_entries), + &_Struct_FieldsEntry_DoNotUse_default_instance_._instance, + nullptr, // post_loop_handler + ::_pbi::TcParser::DiscardEverythingFallback, // fallback + #ifdef PROTOBUF_PREFETCH_PARSE_TABLE + ::_pbi::TcParser::GetTable<::google::protobuf::Struct_FieldsEntry_DoNotUse>(), // to_prefetch + #endif // PROTOBUF_PREFETCH_PARSE_TABLE + }, {{ + // .google.protobuf.Value value = 2; + {::_pbi::TcParser::FastMtS1, + {18, 0, 0, PROTOBUF_FIELD_OFFSET(Struct_FieldsEntry_DoNotUse, _impl_.value_)}}, + // string key = 1; + {::_pbi::TcParser::FastUS1, + {10, 63, 0, PROTOBUF_FIELD_OFFSET(Struct_FieldsEntry_DoNotUse, _impl_.key_)}}, + }}, {{ + 65535, 65535 + }}, {{ + // string key = 1; + {PROTOBUF_FIELD_OFFSET(Struct_FieldsEntry_DoNotUse, _impl_.key_), -1, 0, + (0 | ::_fl::kFcSingular | ::_fl::kUtf8String | ::_fl::kRepAString)}, + // .google.protobuf.Value value = 2; + {PROTOBUF_FIELD_OFFSET(Struct_FieldsEntry_DoNotUse, _impl_.value_), _Internal::kHasBitsOffset + 0, 0, + (0 | ::_fl::kFcOptional | ::_fl::kMessage | ::_fl::kTvTable)}, + }}, {{ + {::_pbi::TcParser::GetTable<::google::protobuf::Value>()}, + }}, {{ + "\42\3\0\0\0\0\0\0" + "google.protobuf.Struct.FieldsEntry" + "key" + }}, +}; + // =================================================================== class Struct::_Internal { diff --git a/src/google/protobuf/struct.pb.h b/src/google/protobuf/struct.pb.h index 2fc87821fa..f8fc679eb3 100644 --- a/src/google/protobuf/struct.pb.h +++ b/src/google/protobuf/struct.pb.h @@ -488,6 +488,11 @@ class Struct_FieldsEntry_DoNotUse final return reinterpret_cast( &_Struct_FieldsEntry_DoNotUse_default_instance_); } +friend class ::google::protobuf::internal::TcParser; +static const ::google::protobuf::internal::TcParseTable< + 1, 2, 1, + 46, 2> + _table_; const ::google::protobuf::Message::ClassData* GetClassData() const PROTOBUF_FINAL; static const ::google::protobuf::Message::ClassDataFull _class_data_; friend struct ::TableStruct_google_2fprotobuf_2fstruct_2eproto; From 901b2e548c0c7b52ce824423fd92fef870ba6e2e Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Thu, 13 Jun 2024 12:15:11 -0700 Subject: [PATCH 09/32] Reorganize our bootstrapped feature gencode. PiperOrigin-RevId: 643082031 --- editions/BUILD | 16 ++-------------- java/core/BUILD.bazel | 6 ++++++ src/google/protobuf/BUILD.bazel | 6 ++++++ src/google/protobuf/compiler/cpp/helpers.cc | 2 ++ src/google/protobuf/compiler/java/BUILD.bazel | 1 + src/google/protobuf/compiler/java/generator.h | 2 +- .../protobuf/compiler/java/internal_helpers.h | 2 +- .../protobuf/compiler/java/kotlin_generator.h | 2 +- 8 files changed, 20 insertions(+), 17 deletions(-) diff --git a/editions/BUILD b/editions/BUILD index 09838afc80..fd5ddddaf0 100644 --- a/editions/BUILD +++ b/editions/BUILD @@ -87,18 +87,6 @@ cc_library( ], ) -cc_proto_library( - name = "cpp_features_cc_proto", - testonly = True, - deps = ["//src/google/protobuf:cpp_features_proto"], -) - -cc_proto_library( - name = "java_features_cc_proto", - testonly = True, - deps = ["//java/core:java_features_proto"], -) - cc_test( name = "defaults_test", srcs = ["defaults_test.cc"], @@ -109,11 +97,11 @@ cc_test( ":test_defaults_future", ], deps = [ - ":cpp_features_cc_proto", ":defaults_test_embedded", - ":java_features_cc_proto", "//:protobuf", + "//java/core:java_features_cc_proto", "//src/google/protobuf", + "//src/google/protobuf:cpp_features_cc_proto", "//src/google/protobuf:port", "//src/google/protobuf:protobuf_lite", "//src/google/protobuf:test_textproto", diff --git a/java/core/BUILD.bazel b/java/core/BUILD.bazel index 7ca8a85636..d695ab0ad0 100644 --- a/java/core/BUILD.bazel +++ b/java/core/BUILD.bazel @@ -196,6 +196,12 @@ proto_library( deps = ["//:descriptor_proto"], ) +cc_proto_library( + name = "java_features_cc_proto", + visibility = ["//editions:__pkg__"], + deps = [":java_features_proto"], +) + filegroup( name = "java_features_proto_srcs", srcs = ["src/main/resources/google/protobuf/java_features.proto"], diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel index 97caa0ea8e..0b2db2c77b 100644 --- a/src/google/protobuf/BUILD.bazel +++ b/src/google/protobuf/BUILD.bazel @@ -231,6 +231,12 @@ proto_library( deps = [":descriptor_proto"], ) +cc_proto_library( + name = "cpp_features_cc_proto", + visibility = ["//editions:__pkg__"], + deps = [":cpp_features_proto"], +) + ################################################################################ # C++ Runtime Library ################################################################################ diff --git a/src/google/protobuf/compiler/cpp/helpers.cc b/src/google/protobuf/compiler/cpp/helpers.cc index 78e2832771..dbfc5f8e9a 100644 --- a/src/google/protobuf/compiler/cpp/helpers.cc +++ b/src/google/protobuf/compiler/cpp/helpers.cc @@ -1664,6 +1664,8 @@ bool GetBootstrapBasename(const Options& options, absl::string_view basename, "third_party/protobuf/descriptor"}, {"third_party/protobuf/cpp_features", "third_party/protobuf/cpp_features"}, + {"third_party/java/protobuf/java_features", + "third_party/java/protobuf/java_features_bootstrap"}, {"third_party/protobuf/compiler/plugin", "third_party/protobuf/compiler/plugin"}, {"net/proto2/compiler/proto/profile", diff --git a/src/google/protobuf/compiler/java/BUILD.bazel b/src/google/protobuf/compiler/java/BUILD.bazel index 40cd4a5fd8..f188e07ca8 100644 --- a/src/google/protobuf/compiler/java/BUILD.bazel +++ b/src/google/protobuf/compiler/java/BUILD.bazel @@ -86,6 +86,7 @@ cc_library( srcs = ["java_features.pb.cc"], hdrs = ["java_features.pb.h"], strip_include_prefix = "/src", + visibility = ["//editions:__pkg__"], deps = [ "//src/google/protobuf", "//src/google/protobuf:arena", diff --git a/src/google/protobuf/compiler/java/generator.h b/src/google/protobuf/compiler/java/generator.h index 53cb616072..1c49b86668 100644 --- a/src/google/protobuf/compiler/java/generator.h +++ b/src/google/protobuf/compiler/java/generator.h @@ -18,8 +18,8 @@ #include #include -#include "google/protobuf/compiler/code_generator.h" #include "google/protobuf/compiler/java/java_features.pb.h" +#include "google/protobuf/compiler/code_generator.h" #include "google/protobuf/descriptor.pb.h" #include "google/protobuf/port.h" diff --git a/src/google/protobuf/compiler/java/internal_helpers.h b/src/google/protobuf/compiler/java/internal_helpers.h index 607669b26a..720a34dccb 100644 --- a/src/google/protobuf/compiler/java/internal_helpers.h +++ b/src/google/protobuf/compiler/java/internal_helpers.h @@ -12,8 +12,8 @@ #ifndef GOOGLE_PROTOBUF_COMPILER_JAVA_INTERNAL_HELPERS_H__ #define GOOGLE_PROTOBUF_COMPILER_JAVA_INTERNAL_HELPERS_H__ -#include "google/protobuf/compiler/java/generator.h" #include "google/protobuf/compiler/java/java_features.pb.h" +#include "google/protobuf/compiler/java/generator.h" #include "google/protobuf/compiler/java/names.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" diff --git a/src/google/protobuf/compiler/java/kotlin_generator.h b/src/google/protobuf/compiler/java/kotlin_generator.h index afc4085646..a0d36816fb 100644 --- a/src/google/protobuf/compiler/java/kotlin_generator.h +++ b/src/google/protobuf/compiler/java/kotlin_generator.h @@ -12,8 +12,8 @@ #include -#include "google/protobuf/compiler/code_generator.h" #include "google/protobuf/compiler/java/java_features.pb.h" +#include "google/protobuf/compiler/code_generator.h" #include "google/protobuf/descriptor.pb.h" // Must be included last. From d879311cac9aa81f02644bf445313c2b022c1c9c Mon Sep 17 00:00:00 2001 From: Jie Luo Date: Thu, 13 Jun 2024 15:21:50 -0700 Subject: [PATCH 10/32] Add type name info for upb python parse error message in MergeFromStrig(). To keep same with cpp extension. PiperOrigin-RevId: 643138099 --- python/message.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/python/message.c b/python/message.c index 8e418f0d7e..719f8581d8 100644 --- a/python/message.c +++ b/python/message.c @@ -1350,7 +1350,9 @@ PyObject* PyUpb_Message_MergeFromString(PyObject* _self, PyObject* arg) { upb_Decode(buf, size, self->ptr.msg, layout, extreg, options, arena); Py_XDECREF(bytes); if (status != kUpb_DecodeStatus_Ok) { - PyErr_Format(state->decode_error_class, "Error parsing message"); + PyErr_Format(state->decode_error_class, + "Error parsing message with type '%s'", + upb_MessageDef_FullName(msgdef)); return NULL; } PyUpb_Message_SyncSubobjs(self); From a30b25578a0435c9c692f9a54b7b060a234dc6b3 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Thu, 13 Jun 2024 17:31:37 -0700 Subject: [PATCH 11/32] Automated rollback of commit 901b2e548c0c7b52ce824423fd92fef870ba6e2e. PiperOrigin-RevId: 643173753 --- editions/BUILD | 16 ++++++++++++++-- java/core/BUILD.bazel | 6 ------ src/google/protobuf/BUILD.bazel | 6 ------ src/google/protobuf/compiler/cpp/helpers.cc | 2 -- src/google/protobuf/compiler/java/BUILD.bazel | 1 - src/google/protobuf/compiler/java/generator.h | 2 +- .../protobuf/compiler/java/internal_helpers.h | 2 +- .../protobuf/compiler/java/kotlin_generator.h | 2 +- 8 files changed, 17 insertions(+), 20 deletions(-) diff --git a/editions/BUILD b/editions/BUILD index fd5ddddaf0..09838afc80 100644 --- a/editions/BUILD +++ b/editions/BUILD @@ -87,6 +87,18 @@ cc_library( ], ) +cc_proto_library( + name = "cpp_features_cc_proto", + testonly = True, + deps = ["//src/google/protobuf:cpp_features_proto"], +) + +cc_proto_library( + name = "java_features_cc_proto", + testonly = True, + deps = ["//java/core:java_features_proto"], +) + cc_test( name = "defaults_test", srcs = ["defaults_test.cc"], @@ -97,11 +109,11 @@ cc_test( ":test_defaults_future", ], deps = [ + ":cpp_features_cc_proto", ":defaults_test_embedded", + ":java_features_cc_proto", "//:protobuf", - "//java/core:java_features_cc_proto", "//src/google/protobuf", - "//src/google/protobuf:cpp_features_cc_proto", "//src/google/protobuf:port", "//src/google/protobuf:protobuf_lite", "//src/google/protobuf:test_textproto", diff --git a/java/core/BUILD.bazel b/java/core/BUILD.bazel index d695ab0ad0..7ca8a85636 100644 --- a/java/core/BUILD.bazel +++ b/java/core/BUILD.bazel @@ -196,12 +196,6 @@ proto_library( deps = ["//:descriptor_proto"], ) -cc_proto_library( - name = "java_features_cc_proto", - visibility = ["//editions:__pkg__"], - deps = [":java_features_proto"], -) - filegroup( name = "java_features_proto_srcs", srcs = ["src/main/resources/google/protobuf/java_features.proto"], diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel index 0b2db2c77b..97caa0ea8e 100644 --- a/src/google/protobuf/BUILD.bazel +++ b/src/google/protobuf/BUILD.bazel @@ -231,12 +231,6 @@ proto_library( deps = [":descriptor_proto"], ) -cc_proto_library( - name = "cpp_features_cc_proto", - visibility = ["//editions:__pkg__"], - deps = [":cpp_features_proto"], -) - ################################################################################ # C++ Runtime Library ################################################################################ diff --git a/src/google/protobuf/compiler/cpp/helpers.cc b/src/google/protobuf/compiler/cpp/helpers.cc index dbfc5f8e9a..78e2832771 100644 --- a/src/google/protobuf/compiler/cpp/helpers.cc +++ b/src/google/protobuf/compiler/cpp/helpers.cc @@ -1664,8 +1664,6 @@ bool GetBootstrapBasename(const Options& options, absl::string_view basename, "third_party/protobuf/descriptor"}, {"third_party/protobuf/cpp_features", "third_party/protobuf/cpp_features"}, - {"third_party/java/protobuf/java_features", - "third_party/java/protobuf/java_features_bootstrap"}, {"third_party/protobuf/compiler/plugin", "third_party/protobuf/compiler/plugin"}, {"net/proto2/compiler/proto/profile", diff --git a/src/google/protobuf/compiler/java/BUILD.bazel b/src/google/protobuf/compiler/java/BUILD.bazel index f188e07ca8..40cd4a5fd8 100644 --- a/src/google/protobuf/compiler/java/BUILD.bazel +++ b/src/google/protobuf/compiler/java/BUILD.bazel @@ -86,7 +86,6 @@ cc_library( srcs = ["java_features.pb.cc"], hdrs = ["java_features.pb.h"], strip_include_prefix = "/src", - visibility = ["//editions:__pkg__"], deps = [ "//src/google/protobuf", "//src/google/protobuf:arena", diff --git a/src/google/protobuf/compiler/java/generator.h b/src/google/protobuf/compiler/java/generator.h index 1c49b86668..53cb616072 100644 --- a/src/google/protobuf/compiler/java/generator.h +++ b/src/google/protobuf/compiler/java/generator.h @@ -18,8 +18,8 @@ #include #include -#include "google/protobuf/compiler/java/java_features.pb.h" #include "google/protobuf/compiler/code_generator.h" +#include "google/protobuf/compiler/java/java_features.pb.h" #include "google/protobuf/descriptor.pb.h" #include "google/protobuf/port.h" diff --git a/src/google/protobuf/compiler/java/internal_helpers.h b/src/google/protobuf/compiler/java/internal_helpers.h index 720a34dccb..607669b26a 100644 --- a/src/google/protobuf/compiler/java/internal_helpers.h +++ b/src/google/protobuf/compiler/java/internal_helpers.h @@ -12,8 +12,8 @@ #ifndef GOOGLE_PROTOBUF_COMPILER_JAVA_INTERNAL_HELPERS_H__ #define GOOGLE_PROTOBUF_COMPILER_JAVA_INTERNAL_HELPERS_H__ -#include "google/protobuf/compiler/java/java_features.pb.h" #include "google/protobuf/compiler/java/generator.h" +#include "google/protobuf/compiler/java/java_features.pb.h" #include "google/protobuf/compiler/java/names.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" diff --git a/src/google/protobuf/compiler/java/kotlin_generator.h b/src/google/protobuf/compiler/java/kotlin_generator.h index a0d36816fb..afc4085646 100644 --- a/src/google/protobuf/compiler/java/kotlin_generator.h +++ b/src/google/protobuf/compiler/java/kotlin_generator.h @@ -12,8 +12,8 @@ #include -#include "google/protobuf/compiler/java/java_features.pb.h" #include "google/protobuf/compiler/code_generator.h" +#include "google/protobuf/compiler/java/java_features.pb.h" #include "google/protobuf/descriptor.pb.h" // Must be included last. From 071d5351eb4f8ef9d87624a04b87c738ca80ec57 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Fri, 14 Jun 2024 10:53:06 -0700 Subject: [PATCH 12/32] Fix data race in crosslink. This was introduced by the previous fix for delimited inheritance, and was never released. This fix removes all getType() calls from crosslink, where it's not safe to inspect the message type, which is still a placeholder, until after crosslinking. Using the inferred type is not necessary since we treat messages and groups the same during crosslink. PiperOrigin-RevId: 643394981 --- .../main/java/com/google/protobuf/Descriptors.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/java/core/src/main/java/com/google/protobuf/Descriptors.java b/java/core/src/main/java/com/google/protobuf/Descriptors.java index 08f5234af3..adaff5568e 100644 --- a/java/core/src/main/java/com/google/protobuf/Descriptors.java +++ b/java/core/src/main/java/com/google/protobuf/Descriptors.java @@ -1901,7 +1901,9 @@ public final class Descriptors { } } - if (getJavaType() == JavaType.MESSAGE) { + // Use raw type since inferred type considers messageType which may not be fully cross + // linked yet. + if (type.getJavaType() == JavaType.MESSAGE) { if (!(typeDescriptor instanceof Descriptor)) { throw new DescriptorValidationException( this, '\"' + proto.getTypeName() + "\" is not a message type."); @@ -1911,7 +1913,7 @@ public final class Descriptors { if (proto.hasDefaultValue()) { throw new DescriptorValidationException(this, "Messages can't have default values."); } - } else if (getJavaType() == JavaType.ENUM) { + } else if (type.getJavaType() == JavaType.ENUM) { if (!(typeDescriptor instanceof EnumDescriptor)) { throw new DescriptorValidationException( this, '\"' + proto.getTypeName() + "\" is not an enum type."); @@ -1921,7 +1923,7 @@ public final class Descriptors { throw new DescriptorValidationException(this, "Field with primitive type has type_name."); } } else { - if (getJavaType() == JavaType.MESSAGE || getJavaType() == JavaType.ENUM) { + if (type.getJavaType() == JavaType.MESSAGE || type.getJavaType() == JavaType.ENUM) { throw new DescriptorValidationException( this, "Field with message or enum type missing type_name."); } @@ -1942,7 +1944,7 @@ public final class Descriptors { } try { - switch (getType()) { + switch (type) { case INT32: case SINT32: case SFIXED32: @@ -2017,7 +2019,7 @@ public final class Descriptors { if (isRepeated()) { defaultValue = Collections.emptyList(); } else { - switch (getJavaType()) { + switch (type.getJavaType()) { case ENUM: // We guarantee elsewhere that an enum type always has at least // one possible value. @@ -2027,7 +2029,7 @@ public final class Descriptors { defaultValue = null; break; default: - defaultValue = getJavaType().defaultDefault; + defaultValue = type.getJavaType().defaultDefault; break; } } From 40ffd46cec1291e1320e46a134c47dd23a74ff43 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Fri, 14 Jun 2024 11:32:46 -0700 Subject: [PATCH 13/32] Automated rollback of commit a30b25578a0435c9c692f9a54b7b060a234dc6b3. PiperOrigin-RevId: 643407401 --- editions/BUILD | 16 ++-------------- java/core/BUILD.bazel | 6 ++++++ src/google/protobuf/BUILD.bazel | 6 ++++++ src/google/protobuf/compiler/cpp/helpers.cc | 2 ++ src/google/protobuf/compiler/java/BUILD.bazel | 1 + src/google/protobuf/compiler/java/generator.h | 2 +- .../protobuf/compiler/java/internal_helpers.h | 2 +- .../protobuf/compiler/java/kotlin_generator.h | 2 +- 8 files changed, 20 insertions(+), 17 deletions(-) diff --git a/editions/BUILD b/editions/BUILD index 09838afc80..fd5ddddaf0 100644 --- a/editions/BUILD +++ b/editions/BUILD @@ -87,18 +87,6 @@ cc_library( ], ) -cc_proto_library( - name = "cpp_features_cc_proto", - testonly = True, - deps = ["//src/google/protobuf:cpp_features_proto"], -) - -cc_proto_library( - name = "java_features_cc_proto", - testonly = True, - deps = ["//java/core:java_features_proto"], -) - cc_test( name = "defaults_test", srcs = ["defaults_test.cc"], @@ -109,11 +97,11 @@ cc_test( ":test_defaults_future", ], deps = [ - ":cpp_features_cc_proto", ":defaults_test_embedded", - ":java_features_cc_proto", "//:protobuf", + "//java/core:java_features_cc_proto", "//src/google/protobuf", + "//src/google/protobuf:cpp_features_cc_proto", "//src/google/protobuf:port", "//src/google/protobuf:protobuf_lite", "//src/google/protobuf:test_textproto", diff --git a/java/core/BUILD.bazel b/java/core/BUILD.bazel index 7ca8a85636..d695ab0ad0 100644 --- a/java/core/BUILD.bazel +++ b/java/core/BUILD.bazel @@ -196,6 +196,12 @@ proto_library( deps = ["//:descriptor_proto"], ) +cc_proto_library( + name = "java_features_cc_proto", + visibility = ["//editions:__pkg__"], + deps = [":java_features_proto"], +) + filegroup( name = "java_features_proto_srcs", srcs = ["src/main/resources/google/protobuf/java_features.proto"], diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel index 97caa0ea8e..0b2db2c77b 100644 --- a/src/google/protobuf/BUILD.bazel +++ b/src/google/protobuf/BUILD.bazel @@ -231,6 +231,12 @@ proto_library( deps = [":descriptor_proto"], ) +cc_proto_library( + name = "cpp_features_cc_proto", + visibility = ["//editions:__pkg__"], + deps = [":cpp_features_proto"], +) + ################################################################################ # C++ Runtime Library ################################################################################ diff --git a/src/google/protobuf/compiler/cpp/helpers.cc b/src/google/protobuf/compiler/cpp/helpers.cc index 78e2832771..dbfc5f8e9a 100644 --- a/src/google/protobuf/compiler/cpp/helpers.cc +++ b/src/google/protobuf/compiler/cpp/helpers.cc @@ -1664,6 +1664,8 @@ bool GetBootstrapBasename(const Options& options, absl::string_view basename, "third_party/protobuf/descriptor"}, {"third_party/protobuf/cpp_features", "third_party/protobuf/cpp_features"}, + {"third_party/java/protobuf/java_features", + "third_party/java/protobuf/java_features_bootstrap"}, {"third_party/protobuf/compiler/plugin", "third_party/protobuf/compiler/plugin"}, {"net/proto2/compiler/proto/profile", diff --git a/src/google/protobuf/compiler/java/BUILD.bazel b/src/google/protobuf/compiler/java/BUILD.bazel index 40cd4a5fd8..f188e07ca8 100644 --- a/src/google/protobuf/compiler/java/BUILD.bazel +++ b/src/google/protobuf/compiler/java/BUILD.bazel @@ -86,6 +86,7 @@ cc_library( srcs = ["java_features.pb.cc"], hdrs = ["java_features.pb.h"], strip_include_prefix = "/src", + visibility = ["//editions:__pkg__"], deps = [ "//src/google/protobuf", "//src/google/protobuf:arena", diff --git a/src/google/protobuf/compiler/java/generator.h b/src/google/protobuf/compiler/java/generator.h index 53cb616072..1c49b86668 100644 --- a/src/google/protobuf/compiler/java/generator.h +++ b/src/google/protobuf/compiler/java/generator.h @@ -18,8 +18,8 @@ #include #include -#include "google/protobuf/compiler/code_generator.h" #include "google/protobuf/compiler/java/java_features.pb.h" +#include "google/protobuf/compiler/code_generator.h" #include "google/protobuf/descriptor.pb.h" #include "google/protobuf/port.h" diff --git a/src/google/protobuf/compiler/java/internal_helpers.h b/src/google/protobuf/compiler/java/internal_helpers.h index 607669b26a..720a34dccb 100644 --- a/src/google/protobuf/compiler/java/internal_helpers.h +++ b/src/google/protobuf/compiler/java/internal_helpers.h @@ -12,8 +12,8 @@ #ifndef GOOGLE_PROTOBUF_COMPILER_JAVA_INTERNAL_HELPERS_H__ #define GOOGLE_PROTOBUF_COMPILER_JAVA_INTERNAL_HELPERS_H__ -#include "google/protobuf/compiler/java/generator.h" #include "google/protobuf/compiler/java/java_features.pb.h" +#include "google/protobuf/compiler/java/generator.h" #include "google/protobuf/compiler/java/names.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" diff --git a/src/google/protobuf/compiler/java/kotlin_generator.h b/src/google/protobuf/compiler/java/kotlin_generator.h index afc4085646..a0d36816fb 100644 --- a/src/google/protobuf/compiler/java/kotlin_generator.h +++ b/src/google/protobuf/compiler/java/kotlin_generator.h @@ -12,8 +12,8 @@ #include -#include "google/protobuf/compiler/code_generator.h" #include "google/protobuf/compiler/java/java_features.pb.h" +#include "google/protobuf/compiler/code_generator.h" #include "google/protobuf/descriptor.pb.h" // Must be included last. From 415a147189fff5417b18ecc515b8197a085a0d2c Mon Sep 17 00:00:00 2001 From: Sandy Zhang Date: Mon, 17 Jun 2024 08:16:24 -0700 Subject: [PATCH 14/32] Reserialize all unresolved features using java features from the generated pool in case of descriptors from the custom pool. This extends previous workaround for java features in unknown fields, to include features extensions that are known but use a mismatched descriptor. This can happen when users bring their own descriptors via buildFrom. PiperOrigin-RevId: 644013578 --- .../java/com/google/protobuf/Descriptors.java | 37 +++-- .../com/google/protobuf/DescriptorsTest.java | 152 +++++++++++------- 2 files changed, 121 insertions(+), 68 deletions(-) diff --git a/java/core/src/main/java/com/google/protobuf/Descriptors.java b/java/core/src/main/java/com/google/protobuf/Descriptors.java index adaff5568e..569fa26e0e 100644 --- a/java/core/src/main/java/com/google/protobuf/Descriptors.java +++ b/java/core/src/main/java/com/google/protobuf/Descriptors.java @@ -2785,10 +2785,30 @@ public final class Descriptors { public abstract FileDescriptor getFile(); void resolveFeatures(FeatureSet unresolvedFeatures) throws DescriptorValidationException { - // Unknown java features may be passed by users via public buildFrom but should not occur from - // generated code. - if (!unresolvedFeatures.getUnknownFields().isEmpty() - && unresolvedFeatures.getUnknownFields().hasField(JavaFeaturesProto.java_.getNumber())) { + if (this.parent != null + && unresolvedFeatures.equals(FeatureSet.getDefaultInstance()) + && !hasInferredLegacyProtoFeatures()) { + this.features = this.parent.features; + validateFeatures(); + return; + } + + // Java features from a custom pool (i.e. buildFrom) may end up in unknown fields or + // use a different descriptor from the generated pool used by the Java runtime. + boolean hasPossibleCustomJavaFeature = false; + for (FieldDescriptor f : unresolvedFeatures.getExtensionFields().keySet()) { + if (f.getNumber() == JavaFeaturesProto.java_.getNumber() + && f != JavaFeaturesProto.java_.getDescriptor()) { + hasPossibleCustomJavaFeature = true; + continue; + } + } + boolean hasPossibleUnknownJavaFeature = + !unresolvedFeatures.getUnknownFields().isEmpty() + && unresolvedFeatures + .getUnknownFields() + .hasField(JavaFeaturesProto.java_.getNumber()); + if (hasPossibleCustomJavaFeature || hasPossibleUnknownJavaFeature) { ExtensionRegistry registry = ExtensionRegistry.newInstance(); registry.add(JavaFeaturesProto.java_); ByteString bytes = unresolvedFeatures.toByteString(); @@ -2799,14 +2819,7 @@ public final class Descriptors { this, "Failed to parse features with Java feature extension registry.", e); } } - - if (this.parent != null - && unresolvedFeatures.equals(FeatureSet.getDefaultInstance()) - && !hasInferredLegacyProtoFeatures()) { - this.features = this.parent.features; - validateFeatures(); - return; - } + FeatureSet.Builder features; if (this.parent == null) { Edition edition = getFile().getEdition(); diff --git a/java/core/src/test/java/com/google/protobuf/DescriptorsTest.java b/java/core/src/test/java/com/google/protobuf/DescriptorsTest.java index 508344d3bf..5eca109d77 100644 --- a/java/core/src/test/java/com/google/protobuf/DescriptorsTest.java +++ b/java/core/src/test/java/com/google/protobuf/DescriptorsTest.java @@ -352,8 +352,9 @@ public class DescriptorsTest { } @Test - public void testProto2FieldDescriptorLegacyEnumFieldTreatedAsClosed() throws Exception { - // Make an open enum definition. + public void testFieldDescriptorLegacyEnumFieldTreatedAsOpen() throws Exception { + // Make an open enum definition and message that treats enum fields as open. + FileDescriptorProto openEnumFile = FileDescriptorProto.newBuilder() .setName("open_enum.proto") @@ -367,30 +368,9 @@ public class DescriptorsTest { .setNumber(0) .build()) .build()) - .build(); - FileDescriptor openFileDescriptor = - Descriptors.FileDescriptor.buildFrom(openEnumFile, new FileDescriptor[0]); - EnumDescriptor openEnum = openFileDescriptor.getEnumTypes().get(0); - assertThat(openEnum.isClosed()).isFalse(); - - // Create a message that treats enum fields as closed. - FileDescriptorProto closedEnumFile = - FileDescriptorProto.newBuilder() - .setName("closed_enum_field.proto") - .addDependency("open_enum.proto") - .setSyntax("proto2") - .addEnumType( - EnumDescriptorProto.newBuilder() - .setName("TestEnum") - .addValue( - EnumValueDescriptorProto.newBuilder() - .setName("TestEnum_VALUE0") - .setNumber(0) - .build()) - .build()) .addMessageType( DescriptorProto.newBuilder() - .setName("TestClosedEnumField") + .setName("TestOpenEnumField") .addField( FieldDescriptorProto.newBuilder() .setName("int_field") @@ -406,32 +386,21 @@ public class DescriptorsTest { .setTypeName("TestEnumOpen") .setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL) .build()) - .addField( - FieldDescriptorProto.newBuilder() - .setName("closed_enum") - .setNumber(3) - .setType(FieldDescriptorProto.Type.TYPE_ENUM) - .setTypeName("TestEnum") - .setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL) - .build()) .build()) .build(); - Descriptor closedMessage = - Descriptors.FileDescriptor.buildFrom( - closedEnumFile, new FileDescriptor[] {openFileDescriptor}) - .getMessageTypes() - .get(0); - assertThat(closedMessage.findFieldByName("int_field").legacyEnumFieldTreatedAsClosed()) + FileDescriptor openEnumFileDescriptor = + Descriptors.FileDescriptor.buildFrom(openEnumFile, new FileDescriptor[0]); + Descriptor openMessage = openEnumFileDescriptor.getMessageTypes().get(0); + EnumDescriptor openEnum = openEnumFileDescriptor.findEnumTypeByName("TestEnumOpen"); + assertThat(openEnum.isClosed()).isFalse(); + assertThat(openMessage.findFieldByName("int_field").legacyEnumFieldTreatedAsClosed()) + .isFalse(); + assertThat(openMessage.findFieldByName("open_enum").legacyEnumFieldTreatedAsClosed()) .isFalse(); - - assertThat(closedMessage.findFieldByName("closed_enum").legacyEnumFieldTreatedAsClosed()) - .isTrue(); - assertThat(closedMessage.findFieldByName("open_enum").legacyEnumFieldTreatedAsClosed()) - .isTrue(); } @Test - public void testEditionFieldDescriptorLegacyEnumFieldTreatedAsClosed() throws Exception { + public void testEditionFieldDescriptorLegacyEnumFieldTreatedAsClosedUnknown() throws Exception { // Make an open enum definition. FileDescriptorProto openEnumFile = FileDescriptorProto.newBuilder() @@ -536,12 +505,19 @@ public class DescriptorsTest { } @Test - public void testFieldDescriptorLegacyEnumFieldTreatedAsOpen() throws Exception { - // Make an open enum definition and message that treats enum fields as open. + public void testEditionFieldDescriptorLegacyEnumFieldTreatedAsClosedCustomPool() + throws Exception { + + FileDescriptor javaFeaturesDescriptor = + Descriptors.FileDescriptor.buildFrom( + JavaFeaturesProto.getDescriptor().toProto(), + new FileDescriptor[] {DescriptorProtos.getDescriptor()}); + // Make an open enum definition. FileDescriptorProto openEnumFile = FileDescriptorProto.newBuilder() .setName("open_enum.proto") - .setSyntax("proto3") + .setSyntax("editions") + .setEdition(Edition.EDITION_2023) .addEnumType( EnumDescriptorProto.newBuilder() .setName("TestEnumOpen") @@ -551,9 +527,38 @@ public class DescriptorsTest { .setNumber(0) .build()) .build()) + .build(); + FileDescriptor openFileDescriptor = + Descriptors.FileDescriptor.buildFrom(openEnumFile, new FileDescriptor[0]); + EnumDescriptor openEnum = openFileDescriptor.getEnumTypes().get(0); + assertThat(openEnum.isClosed()).isFalse(); + + // Create a message that treats enum fields as closed. + FileDescriptorProto editionsClosedEnumFile = + FileDescriptorProto.newBuilder() + .setName("editions_closed_enum_field.proto") + .addDependency("open_enum.proto") + .setSyntax("editions") + .setEdition(Edition.EDITION_2023) + .setOptions( + FileOptions.newBuilder() + .setFeatures( + DescriptorProtos.FeatureSet.newBuilder() + .setEnumType(DescriptorProtos.FeatureSet.EnumType.CLOSED) + .build()) + .build()) + .addEnumType( + EnumDescriptorProto.newBuilder() + .setName("TestEnum") + .addValue( + EnumValueDescriptorProto.newBuilder() + .setName("TestEnum_VALUE0") + .setNumber(0) + .build()) + .build()) .addMessageType( DescriptorProto.newBuilder() - .setName("TestOpenEnumField") + .setName("TestClosedEnumField") .addField( FieldDescriptorProto.newBuilder() .setName("int_field") @@ -568,18 +573,53 @@ public class DescriptorsTest { .setType(FieldDescriptorProto.Type.TYPE_ENUM) .setTypeName("TestEnumOpen") .setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL) + .setOptions( + DescriptorProtos.FieldOptions.newBuilder() + .setFeatures( + DescriptorProtos.FeatureSet.newBuilder() + .setExtension( + // Extension cannot be directly set using custom + // descriptor, so set using generated for now. + JavaFeaturesProto.java_, + JavaFeaturesProto.JavaFeatures.newBuilder() + .setLegacyClosedEnum(true) + .build()) + .build()) + .build()) + .build()) + .addField( + FieldDescriptorProto.newBuilder() + .setName("closed_enum") + .setNumber(3) + .setType(FieldDescriptorProto.Type.TYPE_ENUM) + .setTypeName("TestEnum") + .setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL) .build()) .build()) .build(); - FileDescriptor openEnumFileDescriptor = - Descriptors.FileDescriptor.buildFrom(openEnumFile, new FileDescriptor[0]); - Descriptor openMessage = openEnumFileDescriptor.getMessageTypes().get(0); - EnumDescriptor openEnum = openEnumFileDescriptor.findEnumTypeByName("TestEnumOpen"); - assertThat(openEnum.isClosed()).isFalse(); - assertThat(openMessage.findFieldByName("int_field").legacyEnumFieldTreatedAsClosed()) - .isFalse(); - assertThat(openMessage.findFieldByName("open_enum").legacyEnumFieldTreatedAsClosed()) + // Reparse using custom java features descriptor. + ExtensionRegistry registry = ExtensionRegistry.newInstance(); + registry.add( + javaFeaturesDescriptor.getExtensions().get(0), + DynamicMessage.getDefaultInstance( + javaFeaturesDescriptor.getExtensions().get(0).getMessageType())); + editionsClosedEnumFile = + FileDescriptorProto.parseFrom(editionsClosedEnumFile.toByteString(), registry); + Descriptor editionsClosedMessage = + Descriptors.FileDescriptor.buildFrom( + editionsClosedEnumFile, + new FileDescriptor[] {openFileDescriptor, javaFeaturesDescriptor}) + .getMessageTypes() + .get(0); + assertThat( + editionsClosedMessage.findFieldByName("int_field").legacyEnumFieldTreatedAsClosed()) .isFalse(); + assertThat( + editionsClosedMessage.findFieldByName("closed_enum").legacyEnumFieldTreatedAsClosed()) + .isTrue(); + assertThat( + editionsClosedMessage.findFieldByName("open_enum").legacyEnumFieldTreatedAsClosed()) + .isTrue(); } @Test From e844510ee30e25017e94f9f43751abab159208c8 Mon Sep 17 00:00:00 2001 From: Simon Berger Date: Mon, 17 Jun 2024 09:25:37 -0700 Subject: [PATCH 15/32] Fix segmentation faults with enabled keep_descriptor_pool_after_request (#16993) Fixes #16894 It is tested on command execution only, where it fixes the issues of reported memory leaks, heap corruption and segmentation fault. I could not run tests on my Mac because of #16944 Closes #16993 COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/16993 from simonberger:bugfix/php-ext-persistent-global-corruption 62b5529b6aadf914c708e031e29517869ce2cb15 PiperOrigin-RevId: 644034181 --- php/ext/google/protobuf/protobuf.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/php/ext/google/protobuf/protobuf.c b/php/ext/google/protobuf/protobuf.c index 1f36ed0952..1f6889ff74 100644 --- a/php/ext/google/protobuf/protobuf.c +++ b/php/ext/google/protobuf/protobuf.c @@ -143,11 +143,11 @@ static PHP_GINIT_FUNCTION(protobuf) { protobuf_globals->global_symtab = NULL; } static PHP_RINIT_FUNCTION(protobuf) { // Create the global generated pool. // Reuse the symtab (if any) left to us by the last request. - upb_DefPool* symtab = PROTOBUF_G(global_symtab); - if (!symtab) { - PROTOBUF_G(global_symtab) = symtab = upb_DefPool_New(); - zend_hash_init(&PROTOBUF_G(name_msg_cache), 64, NULL, NULL, 0); - zend_hash_init(&PROTOBUF_G(name_enum_cache), 64, NULL, NULL, 0); + if (!PROTOBUF_G(global_symtab)) { + zend_bool persistent = PROTOBUF_G(keep_descriptor_pool_after_request); + PROTOBUF_G(global_symtab) = upb_DefPool_New(); + zend_hash_init(&PROTOBUF_G(name_msg_cache), 64, NULL, NULL, persistent); + zend_hash_init(&PROTOBUF_G(name_enum_cache), 64, NULL, NULL, persistent); } zend_hash_init(&PROTOBUF_G(object_cache), 64, NULL, NULL, 0); @@ -308,7 +308,7 @@ zend_module_entry protobuf_module_entry = { protobuf_functions, // function list PHP_MINIT(protobuf), // process startup PHP_MSHUTDOWN(protobuf), // process shutdown - PHP_RINIT(protobuf), // request shutdown + PHP_RINIT(protobuf), // request startup PHP_RSHUTDOWN(protobuf), // request shutdown NULL, // extension info PHP_PROTOBUF_VERSION, // extension version From 299bb044d53270ca8c7b615398ca4f4369c75cc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Quenaudon?= Date: Mon, 17 Jun 2024 09:45:26 -0700 Subject: [PATCH 16/32] Reserve 1090 for Wire enumMode (#17158) release notes: no Closes #17158 COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/17158 from oldergod:patch-1 ad5e6bce8382949b06b7341c15d931af5748088d PiperOrigin-RevId: 644040665 --- docs/options.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/options.md b/docs/options.md index a4986e0283..3f5ac61305 100644 --- a/docs/options.md +++ b/docs/options.md @@ -501,3 +501,8 @@ with info about your project (name and website) so we can add an entry for you. * Website: https://github.com/solo-io/protoc-gen-openapi * Extensions: 1188-1189 + +1. Wire enumMode + + * Website: https://square.github.io/wire/ + * Extensions: 1190 From 115b4c4f831d4e62892ce868ad08972d2b300783 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89amonn=20McManus?= Date: Mon, 17 Jun 2024 13:01:32 -0700 Subject: [PATCH 17/32] In the Kotlin DSL, access `oneof` discriminators as properties not methods. If we have a `oneof Foo` in a message `Bar`, we should access the discriminator as `Bar.Builder.fooCase` rather than `Bar.Builder.getFooCase()`. PiperOrigin-RevId: 644107242 --- .../protobuf/compiler/java/field_common.cc | 21 ----------------- .../protobuf/compiler/java/field_common.h | 23 +++++++++++++++++++ .../protobuf/compiler/java/lite/message.cc | 9 ++++---- 3 files changed, 28 insertions(+), 25 deletions(-) diff --git a/src/google/protobuf/compiler/java/field_common.cc b/src/google/protobuf/compiler/java/field_common.cc index cf5cdcbf0a..d8d7255e7b 100644 --- a/src/google/protobuf/compiler/java/field_common.cc +++ b/src/google/protobuf/compiler/java/field_common.cc @@ -13,8 +13,6 @@ namespace protobuf { namespace compiler { namespace java { -std::string GetKotlinPropertyName(std::string capitalized_name); - void SetCommonFieldVariables( const FieldDescriptor* descriptor, const FieldGeneratorInfo* info, absl::flat_hash_map* variables) { @@ -68,25 +66,6 @@ static bool IsUpper(char c) { static char ToLower(char c) { return IsUpper(c) ? c - 'A' + 'a' : c; } -// Returns the name by which the generated Java getters and setters should be -// referenced from Kotlin as properties. In the simplest case, the original name -// is something like `foo_bar`, which gets translated into `getFooBar()` etc, -// and that in turn can be referenced from Kotlin as `fooBar`. -// -// The algorithm for translating proto names into Java getters and setters is -// straightforward. The first letter of each underscore-separated word gets -// uppercased and the underscores are deleted. There are no other changes, so in -// particular if the proto name has a string of capitals then those remain -// as-is. -// -// The algorithm that the Kotlin compiler uses to derive the property name is -// slightly more complicated. If the first character after `get` (etc) is a -// capital and the second isn't, then the property name is just that string with -// its first letter lowercased. So `getFoo` becomes `foo` and `getX` becomes -// `x`. But if there is more than one capital, then all but the last get -// lowercased. So `getHTMLPage` becomes `htmlPage`. If there are only capitals -// then they all get lowercased, so `getID` becomes `id`. -// TODO: move this to a Kotlin-specific location std::string GetKotlinPropertyName(std::string capitalized_name) { // Find the first non-capital. If it is the second character, then we just // need to lowercase the first one. Otherwise we need to lowercase everything diff --git a/src/google/protobuf/compiler/java/field_common.h b/src/google/protobuf/compiler/java/field_common.h index 54e58a0ecb..c29d1c25e9 100644 --- a/src/google/protobuf/compiler/java/field_common.h +++ b/src/google/protobuf/compiler/java/field_common.h @@ -1,6 +1,8 @@ #ifndef GOOGLE_PROTOBUF_COMPILER_JAVA_FIELD_COMMON_H__ #define GOOGLE_PROTOBUF_COMPILER_JAVA_FIELD_COMMON_H__ +#include + #include "google/protobuf/descriptor.h" namespace google { @@ -36,6 +38,27 @@ void PrintExtraFieldInfo( const absl::flat_hash_map& variables, io::Printer* printer); +// Returns the name by which the generated Java getters and setters should be +// referenced from Kotlin as properties. In the simplest case, the original name +// is something like `foo_bar`, which gets translated into `getFooBar()` etc, +// and that in turn can be referenced from Kotlin as `fooBar`. +// +// The algorithm for translating proto names into Java getters and setters is +// straightforward. The first letter of each underscore-separated word gets +// uppercased and the underscores are deleted. There are no other changes, so in +// particular if the proto name has a string of capitals then those remain +// as-is. +// +// The algorithm that the Kotlin compiler uses to derive the property name is +// slightly more complicated. If the first character after `get` (etc) is a +// capital and the second isn't, then the property name is just that string with +// its first letter lowercased. So `getFoo` becomes `foo` and `getX` becomes +// `x`. But if there is more than one capital, then all but the last get +// lowercased. So `getHTMLPage` becomes `htmlPage`. If there are only capitals +// then they all get lowercased, so `getID` becomes `id`. +// TODO: move this to a Kotlin-specific location +std::string GetKotlinPropertyName(std::string capitalized_name); + } // namespace java } // namespace compiler } // namespace protobuf diff --git a/src/google/protobuf/compiler/java/lite/message.cc b/src/google/protobuf/compiler/java/lite/message.cc index 81e95242aa..1d240672d8 100644 --- a/src/google/protobuf/compiler/java/lite/message.cc +++ b/src/google/protobuf/compiler/java/lite/message.cc @@ -773,16 +773,17 @@ void ImmutableMessageLiteGenerator::GenerateKotlinDsl( for (auto& kv : oneofs_) { const OneofDescriptor* oneof = kv.second; + auto oneof_name = context_->GetOneofGeneratorInfo(oneof)->name; printer->Print( "public val $oneof_name$Case: $message$.$oneof_capitalized_name$Case\n" " @JvmName(\"get$oneof_capitalized_name$Case\")\n" - " get() = _builder.get$oneof_capitalized_name$Case()\n\n" + " get() = _builder.$oneof_property_name$Case\n\n" "public fun clear$oneof_capitalized_name$() {\n" " _builder.clear$oneof_capitalized_name$()\n" "}\n", - "oneof_name", context_->GetOneofGeneratorInfo(oneof)->name, - "oneof_capitalized_name", - context_->GetOneofGeneratorInfo(oneof)->capitalized_name, "message", + "oneof_name", oneof_name, "oneof_capitalized_name", + context_->GetOneofGeneratorInfo(oneof)->capitalized_name, + "oneof_property_name", GetKotlinPropertyName(oneof_name), "message", EscapeKotlinKeywords(name_resolver_->GetClassName(descriptor_, true))); } From 3d36855bd26bece56c71a2701f34dd47e616782b Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Mon, 17 Jun 2024 13:06:24 -0700 Subject: [PATCH 18/32] chore: upgrade zlib to 1.3.1 (#17029) `zlib` cannot be complied in macos (https://github.com/madler/zlib/pull/895). This PR wants to upgrade zlib to 1.3.1 to avoid this issue. Closes #17029 COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/17029 from HerrCai0907:update/zlib fd6c66ba1c2c6b8007a65c7ee9d23ee82aafc28f PiperOrigin-RevId: 644108726 --- MODULE.bazel | 2 +- protobuf_deps.bzl | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/MODULE.bazel b/MODULE.bazel index 46207904a5..a59a2de4fb 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -23,7 +23,7 @@ bazel_dep(name = "rules_pkg", version = "0.7.0") bazel_dep(name = "rules_python", version = "0.28.0") bazel_dep(name = "rules_rust", version = "0.45.1") bazel_dep(name = "platforms", version = "0.0.8") -bazel_dep(name = "zlib", version = "1.2.11") +bazel_dep(name = "zlib", version = "1.3.1") # TODO: remove after toolchain types are moved to protobuf bazel_dep(name = "rules_proto", version = "4.0.0") diff --git a/protobuf_deps.bzl b/protobuf_deps.bzl index a852e49a44..ae2aa5e728 100644 --- a/protobuf_deps.bzl +++ b/protobuf_deps.bzl @@ -54,11 +54,11 @@ def protobuf_deps(): http_archive( name = "zlib", build_file = Label("//:third_party/zlib.BUILD"), - sha256 = "d14c38e313afc35a9a8760dadf26042f51ea0f5d154b0630a31da0540107fb98", - strip_prefix = "zlib-1.2.13", + sha256 = "38ef96b8dfe510d42707d9c781877914792541133e1870841463bfa73f883e32", + strip_prefix = "zlib-1.3.1", urls = [ - "https://github.com/madler/zlib/releases/download/v1.2.13/zlib-1.2.13.tar.xz", - "https://zlib.net/zlib-1.2.13.tar.xz", + "https://github.com/madler/zlib/releases/download/v1.3.1/zlib-1.3.1.tar.xz", + "https://zlib.net/zlib-1.3.1.tar.xz", ], ) From 4d8be99f2bef888921fee52e400a27e72593e1b2 Mon Sep 17 00:00:00 2001 From: Sandy Zhang Date: Mon, 17 Jun 2024 14:33:18 -0700 Subject: [PATCH 19/32] Add stubs for GeneratedMessageV3, RepeatedFieldBuilderV3, SingleFieldBuilderV3 for compatibility with older <4.26.x gencode. These classes are deprecated and will be removed in the next breaking change. Users should update gencode to >= 4.26.x which uses GeneratedMessage instead of GeneratedMessageV3. Tested with //compatibility:java_conformance_v3.25.0 which builds the runtime against 3.25.0 gencode. PiperOrigin-RevId: 644136172 --- .github/workflows/test_java.yml | 8 +- compatibility/BUILD.bazel | 1 - .../google/protobuf/GeneratedMessageV3.java | 86 +++++++++++++++++++ .../protobuf/RepeatedFieldBuilderV3.java | 34 ++++++++ .../google/protobuf/SingleFieldBuilderV3.java | 30 +++++++ 5 files changed, 154 insertions(+), 5 deletions(-) create mode 100644 java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java create mode 100644 java/core/src/main/java/com/google/protobuf/RepeatedFieldBuilderV3.java create mode 100644 java/core/src/main/java/com/google/protobuf/SingleFieldBuilderV3.java diff --git a/.github/workflows/test_java.yml b/.github/workflows/test_java.yml index a9abfacaec..e0cb69558f 100644 --- a/.github/workflows/test_java.yml +++ b/.github/workflows/test_java.yml @@ -22,19 +22,19 @@ jobs: image: us-docker.pkg.dev/protobuf-build/containers/test/linux/java:8-1fdbb997433cb22c1e49ef75ad374a8d6bb88702 # TODO: b/318555165 - enable the layering check. Currently it does # not work correctly with the toolchain in this Docker image. - targets: //java/... //java/internal:java_version --features=-layering_check + targets: //java/... //java/internal:java_version //compatibility/... --features=-layering_check - name: OpenJDK 11 version: '11' image: us-docker.pkg.dev/protobuf-build/containers/test/linux/java:11-1fdbb997433cb22c1e49ef75ad374a8d6bb88702 - targets: //java/... //java/internal:java_version + targets: //java/... //java/internal:java_version //compatibility/... - name: OpenJDK 17 version: '17' image: us-docker.pkg.dev/protobuf-build/containers/test/linux/java:17-1fdbb997433cb22c1e49ef75ad374a8d6bb88702 - targets: //java/... //java/internal:java_version + targets: //java/... //java/internal:java_version //compatibility/... - name: aarch64 version: 'aarch64' image: us-docker.pkg.dev/protobuf-build/containers/test/linux/emulation:aarch64-63dd26c0c7a808d92673a3e52e848189d4ab0f17 - targets: //java/... //src/google/protobuf/compiler:protoc_aarch64_test + targets: //java/... //compatibility/... //src/google/protobuf/compiler:protoc_aarch64_test name: Linux ${{ matrix.name }} runs-on: ubuntu-latest diff --git a/compatibility/BUILD.bazel b/compatibility/BUILD.bazel index e62cb5b133..d55f61956e 100644 --- a/compatibility/BUILD.bazel +++ b/compatibility/BUILD.bazel @@ -16,5 +16,4 @@ java_runtime_conformance( java_runtime_conformance( name = "java_conformance_v3.25.0", gencode_version = "3.25.0", - tags = ["manual"], ) diff --git a/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java b/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java new file mode 100644 index 0000000000..35f0898b76 --- /dev/null +++ b/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java @@ -0,0 +1,86 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd + +package com.google.protobuf; + +import java.util.List; + +import com.google.protobuf.Descriptors.Descriptor; +import com.google.protobuf.Descriptors.FieldDescriptor; +import com.google.protobuf.Extension.ExtensionType; +import com.google.protobuf.GeneratedMessage.ExtendableMessageOrBuilder; +import com.google.protobuf.GeneratedMessage.ExtensionDescriptorRetriever; +import com.google.protobuf.GeneratedMessage.FieldAccessorTable; + +/** + * Stub for GeneratedMessageV3 wrapping GeneratedMessage for compatibility with older gencode. + * + * @deprecated This class is deprecated, and slated for removal in the next Java breaking change + * (5.x in 2025 Q1). Users should update gencode to >= 4.26.x which uses GeneratedMessage + * instead of GeneratedMessageV3. + */ +@Deprecated +public abstract class GeneratedMessageV3 extends GeneratedMessage { + private static final long serialVersionUID = 1L; + + protected GeneratedMessageV3() { + super(); + } + + protected GeneratedMessageV3(Builder builder) { + super(builder); + } + + /** + * Stub for GeneratedMessageV3.ExtendableBuilder wrapping GeneratedMessage.ExtendableBuilder for + * compatibility with older gencode. + * + * @deprecated This class is deprecated, and slated for removal in the next Java breaking change + * (5.x in 2025 Q1). Users should update gencode to >= 4.26.x which uses + * GeneratedMessage.ExtendableBuilder instead of GeneratedMessageV3.ExtendableBuilder. + */ + @Deprecated + public abstract static class ExtendableBuilder< + MessageT extends ExtendableMessage, + BuilderT extends ExtendableBuilder> + extends GeneratedMessage.ExtendableBuilder + implements ExtendableMessageOrBuilder { + protected ExtendableBuilder() { + super(); + } + + protected ExtendableBuilder(BuilderParent parent) { + super(parent); + } + + // Support old gencode override method removed in cl/597677225 + public BuilderT setExtension( + final GeneratedMessage.GeneratedExtension extension, final T value) { + return setExtension((ExtensionLite) extension, value); + } + + // Support old gencode override method removed in cl/597677225 + public BuilderT setExtension( + final GeneratedMessage.GeneratedExtension> extension, + final int index, + final T value) { + return setExtension((ExtensionLite>) extension, index, value); + } + + // Support old gencode override method removed in cl/597677225 + public BuilderT addExtension( + final GeneratedMessage.GeneratedExtension> extension, final T value) { + return addExtension((ExtensionLite>) extension, value); + } + + // Support old gencode override method removed in cl/597677225 + public BuilderT clearExtension( + final GeneratedMessage.GeneratedExtension extension) { + return clearExtension((ExtensionLite) extension); + } + } +} diff --git a/java/core/src/main/java/com/google/protobuf/RepeatedFieldBuilderV3.java b/java/core/src/main/java/com/google/protobuf/RepeatedFieldBuilderV3.java new file mode 100644 index 0000000000..ff807586f0 --- /dev/null +++ b/java/core/src/main/java/com/google/protobuf/RepeatedFieldBuilderV3.java @@ -0,0 +1,34 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd + +package com.google.protobuf; + +import java.util.List; + +/** + * Stub for RepeatedFieldBuilderV3 wrapping RepeatedFieldBuilder for compatibility with older + * gencode. + * + * @deprecated This class is deprecated, and slated for removal in the next breaking change. Users + * should update gencode to >= 4.26.x which replaces RepeatedFieldBuilderV3 with + * RepeatedFieldBuilder. + */ +@Deprecated +public class RepeatedFieldBuilderV3< + MType extends GeneratedMessage, + BType extends GeneratedMessage.Builder, + IType extends MessageOrBuilder> + extends RepeatedFieldBuilder { + + public RepeatedFieldBuilderV3( + List messages, + boolean isMessagesListMutable, + GeneratedMessage.BuilderParent parent, + boolean isClean) { + super(messages, isMessagesListMutable, parent, isClean); + } +} diff --git a/java/core/src/main/java/com/google/protobuf/SingleFieldBuilderV3.java b/java/core/src/main/java/com/google/protobuf/SingleFieldBuilderV3.java new file mode 100644 index 0000000000..b23941174c --- /dev/null +++ b/java/core/src/main/java/com/google/protobuf/SingleFieldBuilderV3.java @@ -0,0 +1,30 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd + +package com.google.protobuf; + +import static com.google.protobuf.Internal.checkNotNull; + +/** + * Stub for SingleFieldBuilderV3 wrapping SingleFieldBuilder for compatibility with older gencode. + * + * @deprecated This class is deprecated, and slated for removal in the next breaking change. Users + * should update gencode to >= 4.26.x which replaces SingleFieldBuilderV3 with + * SingleFieldBuilder. + */ +@Deprecated +public class SingleFieldBuilderV3< + MType extends GeneratedMessage, + BType extends GeneratedMessage.Builder, + IType extends MessageOrBuilder> + extends SingleFieldBuilder { + + public SingleFieldBuilderV3( + MType message, GeneratedMessage.BuilderParent parent, boolean isClean) { + super(message, parent, isClean); + } +} From 44c80977b053514ccc7fd8750d977a1703a7eb19 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Mon, 17 Jun 2024 15:01:45 -0700 Subject: [PATCH 20/32] Silence warning on upb/rust/wire.rs DecodeOption PiperOrigin-RevId: 644144635 --- rust/upb/wire.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rust/upb/wire.rs b/rust/upb/wire.rs index e8fa533902..0b9be44244 100644 --- a/rust/upb/wire.rs +++ b/rust/upb/wire.rs @@ -27,6 +27,7 @@ pub enum DecodeStatus { // LINT.ThenChange() #[repr(i32)] +#[allow(dead_code)] enum DecodeOption { AliasString = 1, CheckRequired = 2, From 32b94fef51cf6c76a21f8d1c321ef123ea26c94e Mon Sep 17 00:00:00 2001 From: Sandy Zhang Date: Mon, 17 Jun 2024 15:33:54 -0700 Subject: [PATCH 21/32] Cleanup imports and comments in V3 stubs. PiperOrigin-RevId: 644154501 --- .../google/protobuf/GeneratedMessageV3.java | 23 ++++++++----------- .../protobuf/RepeatedFieldBuilderV3.java | 2 +- .../google/protobuf/SingleFieldBuilderV3.java | 4 +--- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java b/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java index 35f0898b76..9071495866 100644 --- a/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java +++ b/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java @@ -1,5 +1,5 @@ // Protocol Buffers - Google's data interchange format -// Copyright 2008 Google Inc. All rights reserved. +// Copyright 2024 Google LLC. All rights reserved. // // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file or at @@ -9,13 +9,6 @@ package com.google.protobuf; import java.util.List; -import com.google.protobuf.Descriptors.Descriptor; -import com.google.protobuf.Descriptors.FieldDescriptor; -import com.google.protobuf.Extension.ExtensionType; -import com.google.protobuf.GeneratedMessage.ExtendableMessageOrBuilder; -import com.google.protobuf.GeneratedMessage.ExtensionDescriptorRetriever; -import com.google.protobuf.GeneratedMessage.FieldAccessorTable; - /** * Stub for GeneratedMessageV3 wrapping GeneratedMessage for compatibility with older gencode. * @@ -48,7 +41,7 @@ public abstract class GeneratedMessageV3 extends GeneratedMessage { MessageT extends ExtendableMessage, BuilderT extends ExtendableBuilder> extends GeneratedMessage.ExtendableBuilder - implements ExtendableMessageOrBuilder { + implements GeneratedMessage.ExtendableMessageOrBuilder { protected ExtendableBuilder() { super(); } @@ -57,13 +50,15 @@ public abstract class GeneratedMessageV3 extends GeneratedMessage { super(parent); } - // Support old gencode override method removed in cl/597677225 + // Support old gencode override method removed in + // https://github.com/protocolbuffers/protobuf/commit/7bff169d32710b143951ec6ce2c4ea9a56e2ad24 public BuilderT setExtension( final GeneratedMessage.GeneratedExtension extension, final T value) { return setExtension((ExtensionLite) extension, value); } - // Support old gencode override method removed in cl/597677225 + // Support old gencode override method removed in + // https://github.com/protocolbuffers/protobuf/commit/7bff169d32710b143951ec6ce2c4ea9a56e2ad24 public BuilderT setExtension( final GeneratedMessage.GeneratedExtension> extension, final int index, @@ -71,13 +66,15 @@ public abstract class GeneratedMessageV3 extends GeneratedMessage { return setExtension((ExtensionLite>) extension, index, value); } - // Support old gencode override method removed in cl/597677225 + // Support old gencode override method removed in + // https://github.com/protocolbuffers/protobuf/commit/7bff169d32710b143951ec6ce2c4ea9a56e2ad24 public BuilderT addExtension( final GeneratedMessage.GeneratedExtension> extension, final T value) { return addExtension((ExtensionLite>) extension, value); } - // Support old gencode override method removed in cl/597677225 + // Support old gencode override method removed in + // https://github.com/protocolbuffers/protobuf/commit/7bff169d32710b143951ec6ce2c4ea9a56e2ad24 public BuilderT clearExtension( final GeneratedMessage.GeneratedExtension extension) { return clearExtension((ExtensionLite) extension); diff --git a/java/core/src/main/java/com/google/protobuf/RepeatedFieldBuilderV3.java b/java/core/src/main/java/com/google/protobuf/RepeatedFieldBuilderV3.java index ff807586f0..14a47a5d46 100644 --- a/java/core/src/main/java/com/google/protobuf/RepeatedFieldBuilderV3.java +++ b/java/core/src/main/java/com/google/protobuf/RepeatedFieldBuilderV3.java @@ -1,5 +1,5 @@ // Protocol Buffers - Google's data interchange format -// Copyright 2008 Google Inc. All rights reserved. +// Copyright 2024 Google LLC. All rights reserved. // // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file or at diff --git a/java/core/src/main/java/com/google/protobuf/SingleFieldBuilderV3.java b/java/core/src/main/java/com/google/protobuf/SingleFieldBuilderV3.java index b23941174c..3c6b151de2 100644 --- a/java/core/src/main/java/com/google/protobuf/SingleFieldBuilderV3.java +++ b/java/core/src/main/java/com/google/protobuf/SingleFieldBuilderV3.java @@ -1,5 +1,5 @@ // Protocol Buffers - Google's data interchange format -// Copyright 2008 Google Inc. All rights reserved. +// Copyright 2024 Google LLC. All rights reserved. // // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file or at @@ -7,8 +7,6 @@ package com.google.protobuf; -import static com.google.protobuf.Internal.checkNotNull; - /** * Stub for SingleFieldBuilderV3 wrapping SingleFieldBuilder for compatibility with older gencode. * From 0812bbfcf520ec325227c0d91f2decf4bd20be1c Mon Sep 17 00:00:00 2001 From: Mark Hansen Date: Mon, 17 Jun 2024 16:41:15 -0700 Subject: [PATCH 22/32] Add new runtime API that serializes empty extensions without allocating I've made a new name for the interface, ExtensionSerializer, so we can keep ExtensionWriter in the same place for backwards-compatibility for a little while. PiperOrigin-RevId: 644172922 --- .../com/google/protobuf/GeneratedMessage.java | 43 +++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/java/core/src/main/java/com/google/protobuf/GeneratedMessage.java b/java/core/src/main/java/com/google/protobuf/GeneratedMessage.java index 9576051d00..426adb61bf 100644 --- a/java/core/src/main/java/com/google/protobuf/GeneratedMessage.java +++ b/java/core/src/main/java/com/google/protobuf/GeneratedMessage.java @@ -1025,10 +1025,28 @@ public abstract class GeneratedMessage extends AbstractMessage implements Serial /** * Used by subclasses to serialize extensions. Extension ranges may be interleaved with field - * numbers, but we must write them in canonical (sorted by field number) order. ExtensionWriter - * helps us write individual ranges of extensions at once. + * numbers, but we must write them in canonical (sorted by field number) order. + * ExtensionSerializer helps us write individual ranges of extensions at once. */ - protected class ExtensionWriter { + protected interface ExtensionSerializer { + public void writeUntil(final int end, final CodedOutputStream output) throws IOException; + } + + /** No-op implementation that writes nothing, for messages with no extensions. */ + private static final class NoOpExtensionSerializer implements ExtensionSerializer { + // Singleton instance so we can avoid allocating a new one for each message serialization. + private static final NoOpExtensionSerializer INSTANCE = new NoOpExtensionSerializer(); + + @Override + public void writeUntil(final int end, final CodedOutputStream output) { + // no-op + } + } + + /** + * ExtensionSerializer that writes extensions from the FieldSet, for messages with extensions. + */ + protected class ExtensionWriter implements ExtensionSerializer { // Imagine how much simpler this code would be if Java iterators had // a way to get the next element without advancing the iterator. @@ -1043,6 +1061,7 @@ public abstract class GeneratedMessage extends AbstractMessage implements Serial this.messageSetWireFormat = messageSetWireFormat; } + @Override public void writeUntil(final int end, final CodedOutputStream output) throws IOException { while (next != null && next.getKey().getNumber() < end) { FieldDescriptor descriptor = next.getKey(); @@ -1075,14 +1094,32 @@ public abstract class GeneratedMessage extends AbstractMessage implements Serial } } + // TODO: Remove, replace with newExtensionSerializer(). protected ExtensionWriter newExtensionWriter() { return new ExtensionWriter(false); } + protected ExtensionSerializer newExtensionSerializer() { + // Avoid allocation in the common case of no extensions. + if (extensions.isEmpty()) { + return NoOpExtensionSerializer.INSTANCE; + } + return new ExtensionWriter(false); + } + + // TODO: Remove, replace with newMessageSetExtensionSerializer(). protected ExtensionWriter newMessageSetExtensionWriter() { return new ExtensionWriter(true); } + protected ExtensionSerializer newMessageSetExtensionSerializer() { + // Avoid allocation in the common case of no extensions. + if (extensions.isEmpty()) { + return NoOpExtensionSerializer.INSTANCE; + } + return new ExtensionWriter(true); + } + /** Called by subclasses to compute the size of extensions. */ protected int extensionsSerializedSize() { return extensions.getSerializedSize(); From 41514865ebe792ef0f1bfcebdbb2598fd398236d Mon Sep 17 00:00:00 2001 From: Mark Hansen Date: Mon, 17 Jun 2024 17:16:35 -0700 Subject: [PATCH 23/32] Immutable java: Use new lower-allocation extension serialization APIs from gencode This requires the new methods to be present in the runtime. PiperOrigin-RevId: 644182132 --- src/google/protobuf/compiler/java/full/message.cc | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/google/protobuf/compiler/java/full/message.cc b/src/google/protobuf/compiler/java/full/message.cc index 10abbb90e8..433177312a 100644 --- a/src/google/protobuf/compiler/java/full/message.cc +++ b/src/google/protobuf/compiler/java/full/message.cc @@ -587,15 +587,13 @@ void ImmutableMessageGenerator::GenerateMessageSerializationMethods( if (descriptor_->options().message_set_wire_format()) { printer->Print( "com.google.protobuf.GeneratedMessage\n" - " .ExtendableMessage<$classname$>.ExtensionWriter\n" - " extensionWriter = newMessageSetExtensionWriter();\n", - "classname", name_resolver_->GetImmutableClassName(descriptor_)); + " .ExtendableMessage.ExtensionSerializer\n" + " extensionWriter = newMessageSetExtensionSerializer();\n"); } else { printer->Print( "com.google.protobuf.GeneratedMessage\n" - " .ExtendableMessage<$classname$>.ExtensionWriter\n" - " extensionWriter = newExtensionWriter();\n", - "classname", name_resolver_->GetImmutableClassName(descriptor_)); + " .ExtendableMessage.ExtensionSerializer\n" + " extensionWriter = newExtensionSerializer();\n"); } } From 9f6a8bd1fa12bfd4e5bc276677b3a69fa4709a3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20M=C3=B6ller?= Date: Mon, 17 Jun 2024 19:10:01 -0700 Subject: [PATCH 24/32] Removes msvc warning in map (#17013) Hi, We get a compiler warning in MSVC that a signed/unsigned comparison is executed. The warning is at around line 1073 in src/google/protobuf/map.h ```cpp ABSL_DCHECK_GE(new_num_buckets, kMinTableSize); // <-- signed/unsigned comparison const auto old_table = table_; const map_index_t old_table_size = num_buckets_; num_buckets_ = new_num_buckets; ``` This is due to `kMinTableSize` is a enum value. Probably a C++98 way to enforce constexpr. This PR changes `kMinTableSize` to be a constexpr unsigned `map_index_t` and removes the casts instead of fixing the warning with a cast at ne 1073. It seems like `kMinTableSize` is correlated with the `TableSize` of this class, which is map_index_t. Therefore the min value can at maximum the max value of map_index_t. And the min value of `kMinTableSize` can also not be negative, as this makes no sense. It is also nicer to have fewer casts. Closes #17013 COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/17013 from TinyTinni:removes-msvc-warning-map 4f954e8c9bf99a550134345a0086e07fe1c9707c PiperOrigin-RevId: 644207177 --- src/google/protobuf/map.h | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/google/protobuf/map.h b/src/google/protobuf/map.h index 763582c05e..0953b12d00 100644 --- a/src/google/protobuf/map.h +++ b/src/google/protobuf/map.h @@ -560,7 +560,7 @@ class PROTOBUF_EXPORT UntypedMapBase { protected: // 16 bytes is the minimum useful size for the array cache in the arena. - enum { kMinTableSize = 16 / sizeof(void*) }; + enum : map_index_t { kMinTableSize = 16 / sizeof(void*) }; public: Arena* arena() const { return this->alloc_.arena(); } @@ -645,9 +645,7 @@ class PROTOBUF_EXPORT UntypedMapBase { // Return a power of two no less than max(kMinTableSize, n). // Assumes either n < kMinTableSize or n is a power of two. map_index_t TableSize(map_index_t n) { - return n < static_cast(kMinTableSize) - ? static_cast(kMinTableSize) - : n; + return n < kMinTableSize ? kMinTableSize : n; } template @@ -697,7 +695,7 @@ class PROTOBUF_EXPORT UntypedMapBase { } TableEntryPtr* CreateEmptyTable(map_index_t n) { - ABSL_DCHECK_GE(n, map_index_t{kMinTableSize}); + ABSL_DCHECK_GE(n, kMinTableSize); ABSL_DCHECK_EQ(n & (n - 1), 0u); TableEntryPtr* result = AllocFor(alloc_).allocate(n); memset(result, 0, n * sizeof(result[0])); From 2088bc6e66850aa02cceaaae6096f39d20f1c677 Mon Sep 17 00:00:00 2001 From: Jason Lunn Date: Mon, 17 Jun 2024 20:50:20 -0700 Subject: [PATCH 25/32] Disable upload artifacts action (#17160) Address [test flakes](https://github.com/protocolbuffers/protobuf/actions/runs/9527280868/job/26263735005) introduced by failure of the `upload-artifacts` GHA Closes #17160 COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/17160 from protocolbuffers:disable-upload-artifacts-action 87c458388d10d15b7090bcce1b17491cf4f47fba PiperOrigin-RevId: 644228044 --- .github/workflows/test_ruby.yml | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/.github/workflows/test_ruby.yml b/.github/workflows/test_ruby.yml index 2513dc0ad6..8d754189ce 100644 --- a/.github/workflows/test_ruby.yml +++ b/.github/workflows/test_ruby.yml @@ -42,11 +42,13 @@ jobs: credentials: ${{ secrets.GAR_SERVICE_ACCOUNT }} bazel-cache: ruby_linux/${{ matrix.ruby }}_${{ matrix.bazel }} bazel: test //ruby/... //ruby/tests:ruby_version --test_env=KOKORO_RUBY_VERSION --test_env=BAZEL=true ${{ matrix.ffi == 'FFI' && '--//ruby:ffi=enabled --test_env=PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION=FFI' || '' }} - - name: Archive log artifacts - uses: actions/upload-artifact@v4 - with: - name: test-logs-${{ matrix.ruby }}_${{ matrix.ffi || 'NATIVE' }} - path: logs +# Useful tool for troubleshooting, but the action introduces flakes as well, +# e.g. https://github.com/actions/upload-artifact/issues/569 +# - name: Archive log artifacts +# uses: actions/upload-artifact@v4 +# with: +# name: test-logs-${{ matrix.ruby }}_${{ matrix.ffi || 'NATIVE' }} +# path: logs linux-32bit: name: Linux 32-bit From ccbed29c678be6e69d3e784da9e95ed7d2a6dc64 Mon Sep 17 00:00:00 2001 From: Adam Cozzette Date: Tue, 18 Jun 2024 10:56:44 -0700 Subject: [PATCH 26/32] Rust protobuf: make `serialize()` method return `Vec` `Vec` is a more idiomatic Rust type to return for serialization. For the C++ kernel, we are able to return this type with no extra copying. We still use `SerializedData` type for FFI, but convert the result into a `Vec` using a new `into_vec` method. The upb kernel serializes onto an arena, so for upb we do need to copy the data to get it into a `Vec`. PiperOrigin-RevId: 644444571 --- rust/cpp.rs | 22 ++++++++++++++++++++ rust/test/shared/serialization_test.rs | 2 +- rust/upb/wire.rs | 11 +++++----- src/google/protobuf/compiler/rust/message.cc | 8 +++---- 4 files changed, 32 insertions(+), 11 deletions(-) diff --git a/rust/cpp.rs b/rust/cpp.rs index 44451db1cc..4d5760b255 100644 --- a/rust/cpp.rs +++ b/rust/cpp.rs @@ -164,6 +164,28 @@ impl SerializedData { fn as_mut_ptr(&mut self) -> *mut [u8] { ptr::slice_from_raw_parts_mut(self.data.as_ptr(), self.len) } + + /// Converts into a Vec. + pub fn into_vec(self) -> Vec { + // We need to prevent self from being dropped, because we are going to transfer + // ownership of self.data to the Vec. + let s = std::mem::ManuallyDrop::new(self); + + unsafe { + // SAFETY: + // - `data` was allocated by the Rust global allocator. + // - `data` was allocated with an alignment of 1 for u8. + // - The allocated size was `len`. + // - The length and capacity are equal. + // - All `len` bytes are initialized. + // - The capacity (`len` in this case) is the size the pointer was allocated + // with. + // - The allocated size is no more than isize::MAX, because the protobuf + // serializer will refuse to serialize a message if the output would exceed + // 2^31 - 1 bytes. + Vec::::from_raw_parts(s.data.as_ptr(), s.len, s.len) + } + } } impl Deref for SerializedData { diff --git a/rust/test/shared/serialization_test.rs b/rust/test/shared/serialization_test.rs index ab9dd21ca1..e5b614034f 100644 --- a/rust/test/shared/serialization_test.rs +++ b/rust/test/shared/serialization_test.rs @@ -61,7 +61,7 @@ macro_rules! generate_parameterized_serialization_test { msg.set_optional_bool(true); let mut msg2 = [< $type >]::new(); msg2.set_optional_bytes(msg.serialize().unwrap()); - assert_that!(msg2.optional_bytes(), eq(msg.serialize().unwrap().as_ref())); + assert_that!(msg2.optional_bytes(), eq(msg.serialize().unwrap())); } #[test] diff --git a/rust/upb/wire.rs b/rust/upb/wire.rs index 0b9be44244..7660c84e9c 100644 --- a/rust/upb/wire.rs +++ b/rust/upb/wire.rs @@ -1,4 +1,4 @@ -use crate::{upb_ExtensionRegistry, upb_MiniTable, Arena, OwnedArenaBox, RawArena, RawMessage}; +use crate::{upb_ExtensionRegistry, upb_MiniTable, Arena, RawArena, RawMessage}; use std::ptr::NonNull; // LINT.IfChange(encode_status) @@ -37,12 +37,12 @@ enum DecodeOption { /// If Err, then EncodeStatus != Ok. /// -/// SAFETY: +/// # Safety /// - `msg` must be associated with `mini_table`. pub unsafe fn encode( msg: RawMessage, mini_table: *const upb_MiniTable, -) -> Result, EncodeStatus> { +) -> Result, EncodeStatus> { let arena = Arena::new(); let mut buf: *mut u8 = std::ptr::null_mut(); let mut len = 0usize; @@ -55,8 +55,7 @@ pub unsafe fn encode( if status == EncodeStatus::Ok { assert!(!buf.is_null()); // EncodeStatus Ok should never return NULL data, even for len=0. // SAFETY: upb guarantees that `buf` is valid to read for `len`. - let slice = NonNull::new_unchecked(std::ptr::slice_from_raw_parts_mut(buf, len)); - Ok(OwnedArenaBox::new(slice, arena)) + Ok((*std::ptr::slice_from_raw_parts(buf, len)).to_vec()) } else { Err(status) } @@ -65,7 +64,7 @@ pub unsafe fn encode( /// Decodes into the provided message (merge semantics). If Err, then /// DecodeStatus != Ok. /// -/// SAFETY: +/// # Safety /// - `msg` must be mutable. /// - `msg` must be associated with `mini_table`. pub unsafe fn decode( diff --git a/src/google/protobuf/compiler/rust/message.cc b/src/google/protobuf/compiler/rust/message.cc index 5fb4d01032..68946b795a 100644 --- a/src/google/protobuf/compiler/rust/message.cc +++ b/src/google/protobuf/compiler/rust/message.cc @@ -66,7 +66,7 @@ void MessageSerialize(Context& ctx, const Descriptor& msg) { $serialize_thunk$(self.raw_msg(), &mut serialized_data) }; if success { - Ok(serialized_data) + Ok(serialized_data.into_vec()) } else { Err($pb$::SerializeError) } @@ -939,7 +939,7 @@ void GenerateRs(Context& ctx, const Descriptor& msg) { self.msg } - pub fn serialize(&self) -> Result<$pbr$::SerializedData, $pb$::SerializeError> { + pub fn serialize(&self) -> Result, $pb$::SerializeError> { $Msg::serialize$ } @@ -1015,7 +1015,7 @@ void GenerateRs(Context& ctx, const Descriptor& msg) { self.inner } - pub fn serialize(&self) -> Result<$pbr$::SerializedData, $pb$::SerializeError> { + pub fn serialize(&self) -> Result, $pb$::SerializeError> { $pb$::ViewProxy::as_view(self).serialize() } @@ -1069,7 +1069,7 @@ void GenerateRs(Context& ctx, const Descriptor& msg) { $raw_arena_getter_for_message$ - pub fn serialize(&self) -> Result<$pbr$::SerializedData, $pb$::SerializeError> { + pub fn serialize(&self) -> Result, $pb$::SerializeError> { self.as_view().serialize() } #[deprecated = "Prefer Msg::parse(), or use the new name 'clear_and_parse' to parse into a pre-existing message."] From b4ca493e300c709a3658a9f377354817edb23b36 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Tue, 18 Jun 2024 12:15:38 -0700 Subject: [PATCH 27/32] Avoid mangling of comments of namespaces in conformance/conformance.proto for Github. See cl/638713610 PiperOrigin-RevId: 644472463 --- conformance/conformance.proto | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/conformance/conformance.proto b/conformance/conformance.proto index d2a8c67016..2357d59ad6 100644 --- a/conformance/conformance.proto +++ b/conformance/conformance.proto @@ -86,8 +86,8 @@ message ConformanceRequest { // The full name for the test message to use; for the moment, either: // protobuf_test_messages.proto3.TestAllTypesProto3 or - // protobuf_test_messages.google.protobuf.TestAllTypesProto2 or - // protobuf_test_messages.editions.google.protobuf.TestAllTypesProto2 or + // protobuf_test_messages.proto2.TestAllTypesProto2 or + // protobuf_test_messages.editions.proto2.TestAllTypesProto2 or // protobuf_test_messages.editions.proto3.TestAllTypesProto3 or // protobuf_test_messages.editions.TestAllTypesEdition2023. string message_type = 4; From 84e75a5940909cc0e881f65e62e539ede33c6ebe Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Tue, 18 Jun 2024 19:29:35 +0000 Subject: [PATCH 28/32] Auto-generate files after cl/644472463 --- csharp/src/Google.Protobuf.Conformance/Conformance.pb.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/csharp/src/Google.Protobuf.Conformance/Conformance.pb.cs b/csharp/src/Google.Protobuf.Conformance/Conformance.pb.cs index 5f26120acf..f8a073b938 100644 --- a/csharp/src/Google.Protobuf.Conformance/Conformance.pb.cs +++ b/csharp/src/Google.Protobuf.Conformance/Conformance.pb.cs @@ -495,8 +495,8 @@ namespace Conformance { /// /// The full name for the test message to use; for the moment, either: /// protobuf_test_messages.proto3.TestAllTypesProto3 or - /// protobuf_test_messages.google.protobuf.TestAllTypesProto2 or - /// protobuf_test_messages.editions.google.protobuf.TestAllTypesProto2 or + /// protobuf_test_messages.proto2.TestAllTypesProto2 or + /// protobuf_test_messages.editions.proto2.TestAllTypesProto2 or /// protobuf_test_messages.editions.proto3.TestAllTypesProto3 or /// protobuf_test_messages.editions.TestAllTypesEdition2023. /// From 8b7d866b09abf948499c0043c47f924767d92fcb Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Wed, 19 Jun 2024 03:55:47 -0700 Subject: [PATCH 29/32] Automated Code Change PiperOrigin-RevId: 644698319 --- rust/test/BUILD | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/rust/test/BUILD b/rust/test/BUILD index 0fedc17d2a..b94f3dc2f1 100644 --- a/rust/test/BUILD +++ b/rust/test/BUILD @@ -358,10 +358,7 @@ rust_cc_proto_library( rust_upb_proto_library( name = "nested_upb_rust_proto", testonly = True, - visibility = [ - "//rust/test/shared:__subpackages__", - "//rust/test/upb:__subpackages__", - ], + visibility = ["//rust/test/shared:__subpackages__"], deps = [":nested_proto"], ) From 8ed10a9a464c4fdb9d77fbbd082763d7c29c26a2 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Wed, 19 Jun 2024 22:05:47 -0700 Subject: [PATCH 30/32] Automated Code Change PiperOrigin-RevId: 644893037 --- upb/test/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/upb/test/BUILD b/upb/test/BUILD index 98d00e1236..d85f03e9ca 100644 --- a/upb/test/BUILD +++ b/upb/test/BUILD @@ -68,7 +68,7 @@ proto_library( name = "test_proto", testonly = 1, srcs = ["test.proto"], - visibility = ["//upb:__subpackages__"], + visibility = ["//visibility:private"], ) upb_minitable_proto_library( From 6b7e81434735f9987a435943b5894d3c830df545 Mon Sep 17 00:00:00 2001 From: Marcel Hlopko Date: Thu, 20 Jun 2024 05:54:09 -0700 Subject: [PATCH 31/32] Add ProtoString/Bytes owned types PiperOrigin-RevId: 644999527 --- Cargo.bazel.lock | 324 +++++++++++++----- WORKSPACE | 4 +- rust/cpp.rs | 48 +-- rust/cpp_kernel/cpp_api.cc | 8 +- rust/cpp_kernel/cpp_api.h | 2 +- rust/map.rs | 20 +- rust/proxied.rs | 1 - rust/shared.rs | 2 +- rust/string.rs | 228 ++++++++++-- rust/test/shared/accessors_test.rs | 97 +++++- rust/upb.rs | 55 ++- .../rust/accessors/singular_string.cc | 16 +- src/google/protobuf/compiler/rust/naming.cc | 8 +- 13 files changed, 640 insertions(+), 173 deletions(-) diff --git a/Cargo.bazel.lock b/Cargo.bazel.lock index 3855576c78..0d1aa040f8 100644 --- a/Cargo.bazel.lock +++ b/Cargo.bazel.lock @@ -1,9 +1,10 @@ { - "checksum": "8863e5b8f3da7cf4502f68bea0d455dec4834bf25ff070caaa58a8e1c5ea1a3d", + "checksum": "b4edbf28ea96b685a90948d9efaa14d55f0f01e27b5d774b3ecd6eff3c231517", "crates": { "aho-corasick 1.1.2": { "name": "aho-corasick", "version": "1.1.2", + "package_url": "https://github.com/BurntSushi/aho-corasick", "repository": { "Http": { "url": "https://static.crates.io/crates/aho-corasick/1.1.2/download", @@ -15,9 +16,12 @@ "Library": { "crate_name": "aho_corasick", "crate_root": "src/lib.rs", - "srcs": [ - "**/*.rs" - ] + "srcs": { + "allow_empty": false, + "include": [ + "**/*.rs" + ] + } } } ], @@ -46,11 +50,17 @@ "edition": "2021", "version": "1.1.2" }, - "license": "Unlicense OR MIT" + "license": "Unlicense OR MIT", + "license_ids": [ + "MIT", + "Unlicense" + ], + "license_file": "LICENSE-MIT" }, "autocfg 1.1.0": { "name": "autocfg", "version": "1.1.0", + "package_url": "https://github.com/cuviper/autocfg", "repository": { "Http": { "url": "https://static.crates.io/crates/autocfg/1.1.0/download", @@ -62,9 +72,12 @@ "Library": { "crate_name": "autocfg", "crate_root": "src/lib.rs", - "srcs": [ - "**/*.rs" - ] + "srcs": { + "allow_empty": false, + "include": [ + "**/*.rs" + ] + } } } ], @@ -76,20 +89,29 @@ "edition": "2015", "version": "1.1.0" }, - "license": "Apache-2.0 OR MIT" + "license": "Apache-2.0 OR MIT", + "license_ids": [ + "Apache-2.0", + "MIT" + ], + "license_file": "LICENSE-APACHE" }, "direct-cargo-bazel-deps 0.0.1": { "name": "direct-cargo-bazel-deps", "version": "0.0.1", + "package_url": null, "repository": null, "targets": [ { "Library": { "crate_name": "direct_cargo_bazel_deps", "crate_root": ".direct_cargo_bazel_deps.rs", - "srcs": [ - "**/*.rs" - ] + "srcs": { + "allow_empty": false, + "include": [ + "**/*.rs" + ] + } } } ], @@ -119,11 +141,14 @@ }, "version": "0.0.1" }, - "license": null + "license": null, + "license_ids": [], + "license_file": null }, "googletest 0.11.0": { "name": "googletest", "version": "0.11.0", + "package_url": "https://github.com/google/googletest-rust", "repository": { "Git": { "remote": "https://github.com/google/googletest-rust", @@ -138,9 +163,12 @@ "Library": { "crate_name": "googletest", "crate_root": "src/lib.rs", - "srcs": [ - "**/*.rs" - ] + "srcs": { + "allow_empty": false, + "include": [ + "**/*.rs" + ] + } } } ], @@ -178,11 +206,16 @@ }, "version": "0.11.0" }, - "license": "Apache-2.0" + "license": "Apache-2.0", + "license_ids": [ + "Apache-2.0" + ], + "license_file": "LICENSE" }, "googletest_macro 0.11.0": { "name": "googletest_macro", "version": "0.11.0", + "package_url": "https://github.com/google/googletest-rust", "repository": { "Git": { "remote": "https://github.com/google/googletest-rust", @@ -197,9 +230,12 @@ "ProcMacro": { "crate_name": "googletest_macro", "crate_root": "src/lib.rs", - "srcs": [ - "**/*.rs" - ] + "srcs": { + "allow_empty": false, + "include": [ + "**/*.rs" + ] + } } } ], @@ -224,11 +260,16 @@ "edition": "2021", "version": "0.11.0" }, - "license": "Apache-2.0" + "license": "Apache-2.0", + "license_ids": [ + "Apache-2.0" + ], + "license_file": "LICENSE" }, "memchr 2.6.4": { "name": "memchr", "version": "2.6.4", + "package_url": "https://github.com/BurntSushi/memchr", "repository": { "Http": { "url": "https://static.crates.io/crates/memchr/2.6.4/download", @@ -240,9 +281,12 @@ "Library": { "crate_name": "memchr", "crate_root": "src/lib.rs", - "srcs": [ - "**/*.rs" - ] + "srcs": { + "allow_empty": false, + "include": [ + "**/*.rs" + ] + } } } ], @@ -262,11 +306,17 @@ "edition": "2021", "version": "2.6.4" }, - "license": "Unlicense OR MIT" + "license": "Unlicense OR MIT", + "license_ids": [ + "MIT", + "Unlicense" + ], + "license_file": "LICENSE-MIT" }, "num-traits 0.2.17": { "name": "num-traits", "version": "0.2.17", + "package_url": "https://github.com/rust-num/num-traits", "repository": { "Http": { "url": "https://static.crates.io/crates/num-traits/0.2.17/download", @@ -278,18 +328,24 @@ "Library": { "crate_name": "num_traits", "crate_root": "src/lib.rs", - "srcs": [ - "**/*.rs" - ] + "srcs": { + "allow_empty": false, + "include": [ + "**/*.rs" + ] + } } }, { "BuildScript": { "crate_name": "build_script_build", "crate_root": "build.rs", - "srcs": [ - "**/*.rs" - ] + "srcs": { + "allow_empty": false, + "include": [ + "**/*.rs" + ] + } } } ], @@ -331,11 +387,17 @@ "selects": {} } }, - "license": "MIT OR Apache-2.0" + "license": "MIT OR Apache-2.0", + "license_ids": [ + "Apache-2.0", + "MIT" + ], + "license_file": "LICENSE-APACHE" }, "paste 1.0.14": { "name": "paste", "version": "1.0.14", + "package_url": "https://github.com/dtolnay/paste", "repository": { "Http": { "url": "https://static.crates.io/crates/paste/1.0.14/download", @@ -347,18 +409,24 @@ "ProcMacro": { "crate_name": "paste", "crate_root": "src/lib.rs", - "srcs": [ - "**/*.rs" - ] + "srcs": { + "allow_empty": false, + "include": [ + "**/*.rs" + ] + } } }, { "BuildScript": { "crate_name": "build_script_build", "crate_root": "build.rs", - "srcs": [ - "**/*.rs" - ] + "srcs": { + "allow_empty": false, + "include": [ + "**/*.rs" + ] + } } } ], @@ -384,11 +452,17 @@ "**" ] }, - "license": "MIT OR Apache-2.0" + "license": "MIT OR Apache-2.0", + "license_ids": [ + "Apache-2.0", + "MIT" + ], + "license_file": "LICENSE-APACHE" }, "proc-macro2 1.0.69": { "name": "proc-macro2", "version": "1.0.69", + "package_url": "https://github.com/dtolnay/proc-macro2", "repository": { "Http": { "url": "https://static.crates.io/crates/proc-macro2/1.0.69/download", @@ -400,18 +474,24 @@ "Library": { "crate_name": "proc_macro2", "crate_root": "src/lib.rs", - "srcs": [ - "**/*.rs" - ] + "srcs": { + "allow_empty": false, + "include": [ + "**/*.rs" + ] + } } }, { "BuildScript": { "crate_name": "build_script_build", "crate_root": "build.rs", - "srcs": [ - "**/*.rs" - ] + "srcs": { + "allow_empty": false, + "include": [ + "**/*.rs" + ] + } } } ], @@ -447,11 +527,17 @@ "**" ] }, - "license": "MIT OR Apache-2.0" + "license": "MIT OR Apache-2.0", + "license_ids": [ + "Apache-2.0", + "MIT" + ], + "license_file": "LICENSE-APACHE" }, "quote 1.0.33": { "name": "quote", "version": "1.0.33", + "package_url": "https://github.com/dtolnay/quote", "repository": { "Http": { "url": "https://static.crates.io/crates/quote/1.0.33/download", @@ -463,9 +549,12 @@ "Library": { "crate_name": "quote", "crate_root": "src/lib.rs", - "srcs": [ - "**/*.rs" - ] + "srcs": { + "allow_empty": false, + "include": [ + "**/*.rs" + ] + } } } ], @@ -493,11 +582,17 @@ "edition": "2018", "version": "1.0.33" }, - "license": "MIT OR Apache-2.0" + "license": "MIT OR Apache-2.0", + "license_ids": [ + "Apache-2.0", + "MIT" + ], + "license_file": "LICENSE-APACHE" }, "regex 1.10.0": { "name": "regex", "version": "1.10.0", + "package_url": "https://github.com/rust-lang/regex", "repository": { "Http": { "url": "https://static.crates.io/crates/regex/1.10.0/download", @@ -509,9 +604,12 @@ "Library": { "crate_name": "regex", "crate_root": "src/lib.rs", - "srcs": [ - "**/*.rs" - ] + "srcs": { + "allow_empty": false, + "include": [ + "**/*.rs" + ] + } } } ], @@ -566,11 +664,17 @@ "edition": "2021", "version": "1.10.0" }, - "license": "MIT OR Apache-2.0" + "license": "MIT OR Apache-2.0", + "license_ids": [ + "Apache-2.0", + "MIT" + ], + "license_file": "LICENSE-APACHE" }, "regex-automata 0.4.1": { "name": "regex-automata", "version": "0.4.1", + "package_url": "https://github.com/rust-lang/regex/tree/master/regex-automata", "repository": { "Http": { "url": "https://static.crates.io/crates/regex-automata/0.4.1/download", @@ -582,9 +686,12 @@ "Library": { "crate_name": "regex_automata", "crate_root": "src/lib.rs", - "srcs": [ - "**/*.rs" - ] + "srcs": { + "allow_empty": false, + "include": [ + "**/*.rs" + ] + } } } ], @@ -640,11 +747,17 @@ "edition": "2021", "version": "0.4.1" }, - "license": "MIT OR Apache-2.0" + "license": "MIT OR Apache-2.0", + "license_ids": [ + "Apache-2.0", + "MIT" + ], + "license_file": "LICENSE-APACHE" }, "regex-syntax 0.8.1": { "name": "regex-syntax", "version": "0.8.1", + "package_url": "https://github.com/rust-lang/regex/tree/master/regex-syntax", "repository": { "Http": { "url": "https://static.crates.io/crates/regex-syntax/0.8.1/download", @@ -656,9 +769,12 @@ "Library": { "crate_name": "regex_syntax", "crate_root": "src/lib.rs", - "srcs": [ - "**/*.rs" - ] + "srcs": { + "allow_empty": false, + "include": [ + "**/*.rs" + ] + } } } ], @@ -685,11 +801,17 @@ "edition": "2021", "version": "0.8.1" }, - "license": "MIT OR Apache-2.0" + "license": "MIT OR Apache-2.0", + "license_ids": [ + "Apache-2.0", + "MIT" + ], + "license_file": "LICENSE-APACHE" }, "rustversion 1.0.14": { "name": "rustversion", "version": "1.0.14", + "package_url": "https://github.com/dtolnay/rustversion", "repository": { "Http": { "url": "https://static.crates.io/crates/rustversion/1.0.14/download", @@ -701,18 +823,24 @@ "ProcMacro": { "crate_name": "rustversion", "crate_root": "src/lib.rs", - "srcs": [ - "**/*.rs" - ] + "srcs": { + "allow_empty": false, + "include": [ + "**/*.rs" + ] + } } }, { "BuildScript": { "crate_name": "build_script_build", "crate_root": "build/build.rs", - "srcs": [ - "**/*.rs" - ] + "srcs": { + "allow_empty": false, + "include": [ + "**/*.rs" + ] + } } } ], @@ -738,11 +866,17 @@ "**" ] }, - "license": "MIT OR Apache-2.0" + "license": "MIT OR Apache-2.0", + "license_ids": [ + "Apache-2.0", + "MIT" + ], + "license_file": "LICENSE-APACHE" }, "syn 2.0.43": { "name": "syn", "version": "2.0.43", + "package_url": "https://github.com/dtolnay/syn", "repository": { "Http": { "url": "https://static.crates.io/crates/syn/2.0.43/download", @@ -754,9 +888,12 @@ "Library": { "crate_name": "syn", "crate_root": "src/lib.rs", - "srcs": [ - "**/*.rs" - ] + "srcs": { + "allow_empty": false, + "include": [ + "**/*.rs" + ] + } } } ], @@ -798,11 +935,17 @@ "edition": "2021", "version": "2.0.43" }, - "license": "MIT OR Apache-2.0" + "license": "MIT OR Apache-2.0", + "license_ids": [ + "Apache-2.0", + "MIT" + ], + "license_file": "LICENSE-APACHE" }, "unicode-ident 1.0.12": { "name": "unicode-ident", "version": "1.0.12", + "package_url": "https://github.com/dtolnay/unicode-ident", "repository": { "Http": { "url": "https://static.crates.io/crates/unicode-ident/1.0.12/download", @@ -814,9 +957,12 @@ "Library": { "crate_name": "unicode_ident", "crate_root": "src/lib.rs", - "srcs": [ - "**/*.rs" - ] + "srcs": { + "allow_empty": false, + "include": [ + "**/*.rs" + ] + } } } ], @@ -828,7 +974,13 @@ "edition": "2018", "version": "1.0.12" }, - "license": "(MIT OR Apache-2.0) AND Unicode-DFS-2016" + "license": "(MIT OR Apache-2.0) AND Unicode-DFS-2016", + "license_ids": [ + "Apache-2.0", + "MIT", + "Unicode-DFS-2016" + ], + "license_file": "LICENSE-APACHE" } }, "binary_crates": [], @@ -857,6 +1009,12 @@ "aarch64-unknown-linux-gnu": [ "aarch64-unknown-linux-gnu" ], + "aarch64-unknown-nixos-gnu": [ + "aarch64-unknown-nixos-gnu" + ], + "aarch64-unknown-nto-qnx710": [ + "aarch64-unknown-nto-qnx710" + ], "arm-unknown-linux-gnueabi": [ "arm-unknown-linux-gnueabi" ], @@ -926,8 +1084,16 @@ "x86_64-unknown-linux-gnu": [ "x86_64-unknown-linux-gnu" ], + "x86_64-unknown-nixos-gnu": [ + "x86_64-unknown-nixos-gnu" + ], "x86_64-unknown-none": [ "x86_64-unknown-none" ] - } -} + }, + "direct_deps": [ + "googletest 0.11.0", + "paste 1.0.14" + ], + "direct_dev_deps": [] +} \ No newline at end of file diff --git a/WORKSPACE b/WORKSPACE index 7bc8edb294..d84cb7f4b1 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -202,8 +202,8 @@ fuzzing_py_deps_install_deps() http_archive( name = "rules_rust", - sha256 = "9ecd0f2144f0a24e6bc71ebcc50a1ee5128cedeceb32187004532c9710cb2334", - urls = ["https://github.com/bazelbuild/rules_rust/releases/download/0.29.1/rules_rust-v0.29.1.tar.gz"], + integrity = "sha256-F8U7+AC5MvMtPKGdLLnorVM84cDXKfDRgwd7/dq3rUY=", + urls = ["https://github.com/bazelbuild/rules_rust/releases/download/0.46.0/rules_rust-v0.46.0.tar.gz"], ) load("@rules_rust//rust:repositories.bzl", "rules_rust_dependencies", "rust_register_toolchains") diff --git a/rust/cpp.rs b/rust/cpp.rs index 4d5760b255..de038679f6 100644 --- a/rust/cpp.rs +++ b/rust/cpp.rs @@ -9,8 +9,8 @@ use crate::__internal::{Enum, Private}; use crate::{ - Map, MapIter, Mut, ProtoStr, Proxied, ProxiedInMapValue, ProxiedInRepeated, Repeated, - RepeatedMut, RepeatedView, View, + Map, MapIter, Mut, ProtoBytes, ProtoStr, ProtoString, Proxied, ProxiedInMapValue, + ProxiedInRepeated, Repeated, RepeatedMut, RepeatedView, View, }; use core::fmt::Debug; use paste::paste; @@ -75,6 +75,25 @@ pub type RawRepeatedField = NonNull<_opaque_pointees::RawRepeatedFieldData>; /// A raw pointer to the underlying arena for this runtime. pub type RawMap = NonNull<_opaque_pointees::RawMapData>; +/// Kernel-specific owned `string` and `bytes` field type. +// TODO - b/334788521: Allocate this on the C++ side (maybe as a std::string), and move the +// std::string instead of copying the string_view (which we currently do). +#[derive(Debug)] +pub struct InnerProtoString(Box<[u8]>); + +impl InnerProtoString { + pub(crate) fn as_bytes(&self) -> &[u8] { + self.0.as_ref() + } +} + +impl From<&[u8]> for InnerProtoString { + fn from(val: &[u8]) -> Self { + let owned_copy: Box<[u8]> = val.into(); + InnerProtoString(owned_copy) + } +} + /// Represents an ABI-stable version of `NonNull<[u8]>`/`string_view` (a /// borrowed slice of bytes) for FFI use only. /// @@ -197,13 +216,6 @@ impl Deref for SerializedData { } } -// TODO: remove after IntoProxied has been implemented for bytes. -impl AsRef<[u8]> for SerializedData { - fn as_ref(&self) -> &[u8] { - self - } -} - impl Drop for SerializedData { fn drop(&mut self) { // SAFETY: `data` was allocated by the Rust global allocator with a @@ -376,15 +388,15 @@ macro_rules! impl_cpp_type_conversions_for_scalars { impl_cpp_type_conversions_for_scalars!(i32, u32, i64, u64, f32, f64, bool); -impl CppTypeConversions for ProtoStr { +impl CppTypeConversions for ProtoString { type ElemType = PtrAndLen; - fn elem_to_view<'msg>(v: PtrAndLen) -> View<'msg, ProtoStr> { + fn elem_to_view<'msg>(v: PtrAndLen) -> View<'msg, ProtoString> { ptrlen_to_str(v) } } -impl CppTypeConversions for [u8] { +impl CppTypeConversions for ProtoBytes { type ElemType = PtrAndLen; fn elem_to_view<'msg>(v: Self::ElemType) -> View<'msg, Self> { @@ -392,10 +404,6 @@ impl CppTypeConversions for [u8] { } } -// This type alias is used so macros can generate valid extern "C" symbol names -// for functions working with [u8] types. -type Bytes = [u8]; - macro_rules! impl_repeated_primitives { (@impl $($t:ty => [ $new_thunk:ident, @@ -492,7 +500,7 @@ macro_rules! impl_repeated_primitives { }; } -impl_repeated_primitives!(i32, u32, i64, u64, f32, f64, bool, ProtoStr, Bytes); +impl_repeated_primitives!(i32, u32, i64, u64, f32, f64, bool, ProtoString, ProtoBytes); /// Cast a `RepeatedView` to `RepeatedView`. pub fn cast_enum_repeated_view( @@ -786,8 +794,8 @@ macro_rules! impl_ProxiedInMapValue_for_key_types { i64, i64, identity, identity; u64, u64, identity, identity; bool, bool, identity, identity; - ProtoStr, PtrAndLen, str_to_ptrlen, ptrlen_to_str; - Bytes, PtrAndLen, bytes_to_ptrlen, ptrlen_to_bytes; + ProtoString, PtrAndLen, str_to_ptrlen, ptrlen_to_str; + ProtoBytes, PtrAndLen, bytes_to_ptrlen, ptrlen_to_bytes; ); )* } @@ -800,7 +808,7 @@ impl_ProxiedInMapValue_for_key_types!( i64, i64, identity, identity; u64, u64, identity, identity; bool, bool, identity, identity; - ProtoStr, PtrAndLen, str_to_ptrlen, ptrlen_to_str; + ProtoString, PtrAndLen, str_to_ptrlen, ptrlen_to_str; ); #[cfg(test)] diff --git a/rust/cpp_kernel/cpp_api.cc b/rust/cpp_kernel/cpp_api.cc index a4ba3fe37f..d41a485075 100644 --- a/rust/cpp_kernel/cpp_api.cc +++ b/rust/cpp_kernel/cpp_api.cc @@ -100,8 +100,8 @@ expose_repeated_field_methods(int64_t, i64); r->Reserve(r->size() + additional); \ } -expose_repeated_ptr_field_methods(ProtoStr); -expose_repeated_ptr_field_methods(Bytes); +expose_repeated_ptr_field_methods(ProtoString); +expose_repeated_ptr_field_methods(ProtoBytes); #undef expose_repeated_field_methods #undef expose_repeated_ptr_field_methods @@ -126,11 +126,11 @@ __PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE(uint64_t, u64, uint64_t, __PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE(int64_t, i64, int64_t, value, cpp_value); __PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE( - std::string, Bytes, google::protobuf::rust_internal::PtrAndLen, + std::string, ProtoBytes, google::protobuf::rust_internal::PtrAndLen, std::string(value.ptr, value.len), google::protobuf::rust_internal::PtrAndLen(cpp_value.data(), cpp_value.size())); __PB_RUST_EXPOSE_SCALAR_MAP_METHODS_FOR_VALUE_TYPE( - std::string, ProtoStr, google::protobuf::rust_internal::PtrAndLen, + std::string, ProtoString, google::protobuf::rust_internal::PtrAndLen, std::string(value.ptr, value.len), google::protobuf::rust_internal::PtrAndLen(cpp_value.data(), cpp_value.size())); diff --git a/rust/cpp_kernel/cpp_api.h b/rust/cpp_kernel/cpp_api.h index 9345a01c3f..24765f63a7 100644 --- a/rust/cpp_kernel/cpp_api.h +++ b/rust/cpp_kernel/cpp_api.h @@ -156,7 +156,7 @@ struct PtrAndLen { value_ty, rust_value_ty, ffi_value_ty, \ to_cpp_value, to_ffi_value); \ __PB_RUST_EXPOSE_SCALAR_MAP_METHODS( \ - std::string, ProtoStr, google::protobuf::rust_internal::PtrAndLen, \ + std::string, ProtoString, google::protobuf::rust_internal::PtrAndLen, \ std::string(key.ptr, key.len), \ google::protobuf::rust_internal::PtrAndLen(cpp_key.data(), cpp_key.size()), \ value_ty, rust_value_ty, ffi_value_ty, to_cpp_value, to_ffi_value); diff --git a/rust/map.rs b/rust/map.rs index 3ceb2a7aac..dc852408c2 100644 --- a/rust/map.rs +++ b/rust/map.rs @@ -458,7 +458,7 @@ where #[cfg(test)] mod tests { use super::*; - use crate::ProtoStr; + use crate::{ProtoBytes, ProtoStr, ProtoString}; use googletest::prelude::*; #[test] @@ -489,7 +489,7 @@ mod tests { #[test] fn test_proxied_str() { - let mut map: Map = Map::new(); + let mut map: Map = Map::new(); let mut map_mut = map.as_mut(); map_mut.insert("a", "b"); @@ -514,7 +514,7 @@ mod tests { #[test] fn test_proxied_iter() { - let mut map: Map = Map::new(); + let mut map: Map = Map::new(); let mut map_mut = map.as_mut(); map_mut.insert(15, "fizzbuzz"); map_mut.insert(5, "buzz"); @@ -561,7 +561,7 @@ mod tests { #[test] fn test_overwrite_insert() { - let mut map: Map = Map::new(); + let mut map: Map = Map::new(); let mut map_mut = map.as_mut(); assert!(map_mut.insert(0, "fizz")); // insert should return false when the key is already present @@ -571,7 +571,7 @@ mod tests { #[test] fn test_extend() { - let mut map: Map = Map::new(); + let mut map: Map = Map::new(); let mut map_mut = map.as_mut(); map_mut.extend([(0, ""); 0]); @@ -588,7 +588,7 @@ mod tests { ] ); - let mut map_2: Map = Map::new(); + let mut map_2: Map = Map::new(); let mut map_2_mut = map_2.as_mut(); map_2_mut.extend([(2, "bing"), (3, "bong")]); @@ -608,7 +608,7 @@ mod tests { #[test] fn test_copy_from() { - let mut map: Map = Map::new(); + let mut map: Map = Map::new(); let mut map_mut = map.as_mut(); map_mut.copy_from([(0, "fizz"), (1, "buzz"), (2, "fizzbuzz")]); @@ -621,7 +621,7 @@ mod tests { ] ); - let mut map_2: Map = Map::new(); + let mut map_2: Map = Map::new(); let mut map_2_mut = map_2.as_mut(); map_2_mut.copy_from([(2, "bing"), (3, "bong")]); @@ -651,12 +651,12 @@ mod tests { macro_rules! gen_proto_keys { ($($key_t:ty),*) => { $( - gen_proto_values!($key_t, f32, f64, i32, u32, i64, bool, ProtoStr, [u8]); + gen_proto_values!($key_t, f32, f64, i32, u32, i64, bool, ProtoString, ProtoBytes); )* } } - gen_proto_keys!(i32, u32, i64, u64, bool, ProtoStr); + gen_proto_keys!(i32, u32, i64, u64, bool, ProtoString); } #[test] diff --git a/rust/proxied.rs b/rust/proxied.rs index d3fa32d687..c6aed93f29 100644 --- a/rust/proxied.rs +++ b/rust/proxied.rs @@ -224,7 +224,6 @@ pub trait IntoProxied { mod tests { use super::*; use googletest::prelude::*; - use std::borrow::Cow; #[derive(Debug, Default, PartialEq)] struct MyProxied { diff --git a/rust/shared.rs b/rust/shared.rs index 7cf07ed970..eb8fd21479 100644 --- a/rust/shared.rs +++ b/rust/shared.rs @@ -30,7 +30,7 @@ pub mod __public { pub use crate::repeated::{ ProxiedInRepeated, Repeated, RepeatedIter, RepeatedMut, RepeatedView, }; - pub use crate::string::ProtoStr; + pub use crate::string::{ProtoBytes, ProtoStr, ProtoString}; pub use crate::{ParseError, SerializeError}; } pub use __public::*; diff --git a/rust/string.rs b/rust/string.rs index 9272c8d0f0..589721c42e 100644 --- a/rust/string.rs +++ b/rust/string.rs @@ -10,23 +10,103 @@ #![allow(unused)] use crate::__internal::Private; -use crate::__runtime::{PtrAndLen, RawMessage}; -use crate::{Mut, MutProxied, MutProxy, Optional, Proxied, View, ViewProxy}; +use crate::__runtime::{InnerProtoString, PtrAndLen, RawMessage}; +use crate::{IntoProxied, Mut, MutProxied, MutProxy, Optional, Proxied, View, ViewProxy}; use std::borrow::Cow; use std::cmp::{Eq, Ord, Ordering, PartialEq, PartialOrd}; use std::convert::{AsMut, AsRef}; +use std::ffi::{OsStr, OsString}; use std::fmt; use std::hash::{Hash, Hasher}; use std::iter; use std::ops::{Deref, DerefMut}; +use std::ptr; +use std::rc::Rc; +use std::sync::Arc; use utf8::Utf8Chunks; -impl Proxied for [u8] { +pub struct ProtoBytes { + pub(crate) inner: InnerProtoString, +} + +impl AsRef<[u8]> for ProtoBytes { + fn as_ref(&self) -> &[u8] { + self.inner.as_bytes() + } +} + +impl From<&[u8]> for ProtoBytes { + fn from(v: &[u8]) -> ProtoBytes { + ProtoBytes { inner: InnerProtoString::from(v) } + } +} + +impl From<&[u8; N]> for ProtoBytes { + fn from(v: &[u8; N]) -> ProtoBytes { + ProtoBytes { inner: InnerProtoString::from(v.as_ref()) } + } +} + +impl Proxied for ProtoBytes { type View<'msg> = &'msg [u8]; } +impl IntoProxied for ProtoBytes { + fn into_proxied(self, _private: Private) -> ProtoBytes { + self + } +} + +impl IntoProxied for &[u8] { + fn into_proxied(self, _private: Private) -> ProtoBytes { + ProtoBytes::from(self) + } +} + +impl IntoProxied for &[u8; N] { + fn into_proxied(self, _private: Private) -> ProtoBytes { + ProtoBytes::from(self.as_ref()) + } +} + +impl IntoProxied for Vec { + fn into_proxied(self, _private: Private) -> ProtoBytes { + ProtoBytes::from(AsRef::<[u8]>::as_ref(&self)) + } +} + +impl IntoProxied for &Vec { + fn into_proxied(self, _private: Private) -> ProtoBytes { + ProtoBytes::from(AsRef::<[u8]>::as_ref(self)) + } +} + +impl IntoProxied for Box<[u8]> { + fn into_proxied(self, _private: Private) -> ProtoBytes { + ProtoBytes::from(AsRef::<[u8]>::as_ref(&self)) + } +} + +impl IntoProxied for Cow<'_, [u8]> { + fn into_proxied(self, _private: Private) -> ProtoBytes { + ProtoBytes::from(AsRef::<[u8]>::as_ref(&self)) + } +} + +impl IntoProxied for Rc<[u8]> { + fn into_proxied(self, _private: Private) -> ProtoBytes { + ProtoBytes::from(AsRef::<[u8]>::as_ref(&self)) + } +} + +impl IntoProxied for Arc<[u8]> { + fn into_proxied(self, _private: Private) -> ProtoBytes { + ProtoBytes::from(AsRef::<[u8]>::as_ref(&self)) + } +} + impl<'msg> ViewProxy<'msg> for &'msg [u8] { - type Proxied = [u8]; + type Proxied = ProtoBytes; fn as_view(&self) -> &[u8] { self @@ -50,6 +130,118 @@ impl From for Utf8Error { } } +/// An owned type representing protobuf `string` field's contents. +/// +/// # UTF-8 +/// +/// Protobuf [docs] state that a `string` field contains UTF-8 encoded text. +/// However, not every runtime enforces this, and the Rust runtime is designed +/// to integrate with other runtimes with FFI, like C++. +/// +/// `ProtoString` represents a string type that is expected to contain valid +/// UTF-8. However, `ProtoString` is not validated, so users must +/// call [`ProtoString::to_string`] to perform a (possibly runtime-elided) UTF-8 +/// validation check. This validation should rarely fail in pure Rust programs, +/// but is necessary to prevent UB when interacting with C++, or other languages +/// with looser restrictions. +/// +/// +/// # `Display` and `ToString` +/// `ProtoString` is ordinarily UTF-8 and so implements `Display`. If there are +/// any invalid UTF-8 sequences, they are replaced with [`U+FFFD REPLACEMENT +/// CHARACTER`]. Because anything implementing `Display` also implements +/// `ToString`, `ProtoString::to_string()` is equivalent to +/// `String::from_utf8_lossy(proto_string.as_bytes()).into_owned()`. +/// +/// [`U+FFFD REPLACEMENT CHARACTER`]: std::char::REPLACEMENT_CHARACTER +pub struct ProtoString { + pub(crate) inner: InnerProtoString, +} + +impl ProtoString { + pub fn as_bytes(&self) -> &[u8] { + self.inner.as_bytes() + } +} + +impl From<&str> for ProtoString { + fn from(v: &str) -> Self { + Self::from(v.as_bytes()) + } +} + +impl From<&[u8]> for ProtoString { + fn from(v: &[u8]) -> Self { + Self { inner: InnerProtoString::from(v) } + } +} + +impl IntoProxied for ProtoString { + fn into_proxied(self, _private: Private) -> ProtoString { + self + } +} + +impl IntoProxied for &str { + fn into_proxied(self, _private: Private) -> ProtoString { + ProtoString::from(self) + } +} + +impl IntoProxied for &ProtoStr { + fn into_proxied(self, _private: Private) -> ProtoString { + ProtoString::from(self.as_bytes()) + } +} + +impl IntoProxied for String { + fn into_proxied(self, _private: Private) -> ProtoString { + ProtoString::from(self.as_str()) + } +} + +impl IntoProxied for &String { + fn into_proxied(self, _private: Private) -> ProtoString { + ProtoString::from(self.as_bytes()) + } +} + +impl IntoProxied for OsString { + fn into_proxied(self, private: Private) -> ProtoString { + self.as_os_str().into_proxied(private) + } +} + +impl IntoProxied for &OsStr { + fn into_proxied(self, _private: Private) -> ProtoString { + ProtoString::from(self.as_encoded_bytes()) + } +} + +impl IntoProxied for Box { + fn into_proxied(self, _private: Private) -> ProtoString { + ProtoString::from(AsRef::::as_ref(&self)) + } +} + +impl IntoProxied for Cow<'_, str> { + fn into_proxied(self, _private: Private) -> ProtoString { + ProtoString::from(AsRef::::as_ref(&self)) + } +} + +impl IntoProxied for Rc { + fn into_proxied(self, _private: Private) -> ProtoString { + ProtoString::from(AsRef::::as_ref(&self)) + } +} + +impl IntoProxied for Arc { + fn into_proxied(self, _private: Private) -> ProtoString { + ProtoString::from(AsRef::::as_ref(&self)) + } +} + /// A shared immutable view of a protobuf `string` field's contents. /// /// Like a `str`, it can be cheaply accessed as bytes and @@ -261,12 +453,12 @@ impl Ord for ProtoStr { } } -impl Proxied for ProtoStr { +impl Proxied for ProtoString { type View<'msg> = &'msg ProtoStr; } impl<'msg> ViewProxy<'msg> for &'msg ProtoStr { - type Proxied = ProtoStr; + type Proxied = ProtoString; fn as_view(&self) -> &ProtoStr { self @@ -280,30 +472,6 @@ impl<'msg> ViewProxy<'msg> for &'msg ProtoStr { } } -// TODO: remove after IntoProxied has been implemented for -// ProtoStr. -impl AsRef for String { - fn as_ref(&self) -> &ProtoStr { - ProtoStr::from_str(self.as_str()) - } -} - -// TODO: remove after IntoProxied has been implemented for -// ProtoStr. -impl AsRef for &str { - fn as_ref(&self) -> &ProtoStr { - ProtoStr::from_str(self) - } -} - -// TODO: remove after IntoProxied has been implemented for -// ProtoStr. -impl AsRef for &ProtoStr { - fn as_ref(&self) -> &ProtoStr { - self - } -} - /// Implements `PartialCmp` and `PartialEq` for the `lhs` against the `rhs` /// using `AsRef<[u8]>`. // TODO: consider improving to not require a `<()>` if no generics are diff --git a/rust/test/shared/accessors_test.rs b/rust/test/shared/accessors_test.rs index 956ff06a6b..f108c889c1 100644 --- a/rust/test/shared/accessors_test.rs +++ b/rust/test/shared/accessors_test.rs @@ -8,7 +8,11 @@ //! Tests covering accessors for singular bool, int32, int64, and bytes fields. use googletest::prelude::*; -use protobuf::Optional; +use protobuf::{Optional, ProtoBytes, ProtoStr, ProtoString}; +use std::borrow::Cow; +use std::ffi::OsString; +use std::rc::Rc; +use std::sync::Arc; use unittest_rust_proto::{test_all_types, TestAllTypes}; #[test] @@ -415,6 +419,48 @@ fn test_optional_bytes_accessors() { assert_that!(msg.optional_bytes_opt(), eq(Optional::Set(&b""[..]))); } +#[test] +fn test_into_proxied_for_bytes() { + let mut msg = TestAllTypes::new(); + + // &[u8] + let bytes: &[u8] = b"first"; + msg.set_optional_bytes(bytes); + assert_that!(msg.optional_bytes(), eq(bytes)); + + // &[u8; N] + msg.set_optional_bytes(b"second"); + assert_that!(msg.optional_bytes(), eq(b"second")); + + // Vec + msg.set_optional_bytes(Vec::from(b"third")); + assert_that!(msg.optional_bytes(), eq(b"third")); + + // ProtoBytes + msg.set_optional_bytes(ProtoBytes::from(b"fourth")); + assert_that!(msg.optional_bytes(), eq(b"fourth")); + + // Box<[u8]> + msg.set_optional_bytes(Box::from(b"fifth".to_owned())); + assert_that!(msg.optional_bytes(), eq(b"fifth")); + + // Cow<[u8]> + msg.set_optional_bytes(Cow::from(b"sixth")); + assert_that!(msg.optional_bytes(), eq(b"sixth")); + + // Rc<[u8]> + msg.set_optional_bytes(Rc::from(b"seventh".to_owned())); + assert_that!(msg.optional_bytes(), eq(b"seventh")); + + // Arc<[u8]> + msg.set_optional_bytes(Arc::from(b"eighth".to_owned())); + assert_that!(msg.optional_bytes(), eq(b"eighth")); + + // &Vec + msg.set_optional_bytes(&Vec::from(b"ninth")); + assert_that!(msg.optional_bytes(), eq(b"ninth")); +} + #[test] fn test_nonempty_default_bytes_accessors() { let mut msg = TestAllTypes::new(); @@ -470,6 +516,55 @@ fn test_optional_string_accessors() { assert_that!(msg.optional_string_opt(), eq(Optional::Unset("".into()))); } +#[test] +fn test_into_proxied_for_string() { + let mut msg = TestAllTypes::new(); + + // &str + msg.set_optional_string("first"); + assert_that!(msg.optional_string(), eq("first")); + + // String + msg.set_optional_string("second".to_string()); + assert_that!(msg.optional_string(), eq("second")); + + // ProtoStr + msg.set_optional_string(ProtoStr::from_str("third")); + assert_that!(msg.optional_string(), eq("third")); + + // ProtoString + msg.set_optional_string(ProtoString::from("fourth")); + assert_that!(msg.optional_string(), eq("fourth")); + + // OsString + msg.set_optional_string(OsString::from("fifth")); + assert_that!(msg.optional_string(), eq("fifth")); + + // OsStr + msg.set_optional_string(OsString::from("sixth").as_os_str()); + assert_that!(msg.optional_string(), eq("sixth")); + + // Box + msg.set_optional_string(Box::from("seventh")); + assert_that!(msg.optional_string(), eq("seventh")); + + // Cow + msg.set_optional_string(Cow::from("eighth")); + assert_that!(msg.optional_string(), eq("eighth")); + + // Rc + msg.set_optional_string(Rc::from("ninth")); + assert_that!(msg.optional_string(), eq("ninth")); + + // Arc + msg.set_optional_string(Arc::from("tenth")); + assert_that!(msg.optional_string(), eq("tenth")); + + // &String + msg.set_optional_string(&"eleventh".to_string()); + assert_that!(msg.optional_string(), eq("eleventh")); +} + #[test] fn test_nonempty_default_string_accessors() { let mut msg = TestAllTypes::new(); diff --git a/rust/upb.rs b/rust/upb.rs index 1d5e396131..4b1ab8b0c5 100644 --- a/rust/upb.rs +++ b/rust/upb.rs @@ -9,8 +9,8 @@ use crate::__internal::{Enum, Private}; use crate::{ - Map, MapIter, MapMut, MapView, Mut, ProtoStr, Proxied, ProxiedInMapValue, ProxiedInRepeated, - Repeated, RepeatedMut, RepeatedView, View, ViewProxy, + IntoProxied, Map, MapIter, MapMut, MapView, Mut, ProtoBytes, ProtoStr, ProtoString, Proxied, + ProxiedInMapValue, ProxiedInRepeated, Repeated, RepeatedMut, RepeatedView, View, ViewProxy, }; use core::fmt::Debug; use std::alloc::Layout; @@ -60,6 +60,12 @@ impl ScratchSpace { pub type SerializedData = upb::OwnedArenaBox<[u8]>; +impl IntoProxied for SerializedData { + fn into_proxied(self, _private: Private) -> ProtoBytes { + ProtoBytes { inner: InnerProtoString(self) } + } +} + /// The raw contents of every generated message. #[derive(Debug)] pub struct MessageInner { @@ -144,6 +150,27 @@ fn copy_bytes_in_arena<'msg>(arena: &'msg Arena, val: &'msg [u8]) -> &'msg [u8] } } +/// Kernel-specific owned `string` and `bytes` field type. +pub struct InnerProtoString(OwnedArenaBox<[u8]>); + +impl InnerProtoString { + pub(crate) fn as_bytes(&self) -> &[u8] { + &self.0 + } +} + +impl From<&[u8]> for InnerProtoString { + fn from(val: &[u8]) -> InnerProtoString { + let arena = Arena::new(); + let in_arena_copy = arena.copy_slice_in(val); + // SAFETY: + // - `in_arena_copy` is valid slice that will live for `arena`'s lifetime and + // this is the only reference in the program to it. + // - `in_arena_copy` is a pointer into an allocation on `arena` + InnerProtoString(unsafe { OwnedArenaBox::new(Into::into(in_arena_copy), arena) }) + } +} + /// The raw type-erased version of an owned `Repeated`. #[derive(Debug)] pub struct InnerRepeated { @@ -332,7 +359,7 @@ impl_repeated_primitives!( (u64, u64, uint64_val, upb::CType::UInt64), ); -impl_repeated_bytes!((ProtoStr, upb::CType::String), ([u8], upb::CType::Bytes),); +impl_repeated_bytes!((ProtoString, upb::CType::String), (ProtoBytes, upb::CType::Bytes),); /// Copy the contents of `src` into `dest`. /// @@ -564,18 +591,18 @@ impl_upb_type_conversions_for_scalars!( bool, bool_val, upb::CType::Bool, false; ); -impl UpbTypeConversions for [u8] { +impl UpbTypeConversions for ProtoBytes { fn upb_type() -> upb::CType { upb::CType::Bytes } - fn to_message_value(val: View<'_, [u8]>) -> upb_MessageValue { + fn to_message_value(val: View<'_, ProtoBytes>) -> upb_MessageValue { upb_MessageValue { str_val: val.into() } } unsafe fn to_message_value_copy_if_required( raw_arena: RawArena, - val: View<'_, [u8]>, + val: View<'_, ProtoBytes>, ) -> upb_MessageValue { // SAFETY: The arena memory is not freed due to `ManuallyDrop`. let arena = ManuallyDrop::new(unsafe { Arena::from_raw(raw_arena) }); @@ -584,34 +611,34 @@ impl UpbTypeConversions for [u8] { msg_val } - unsafe fn from_message_value<'msg>(msg: upb_MessageValue) -> View<'msg, [u8]> { + unsafe fn from_message_value<'msg>(msg: upb_MessageValue) -> View<'msg, ProtoBytes> { unsafe { msg.str_val.as_ref() } } } -impl UpbTypeConversions for ProtoStr { +impl UpbTypeConversions for ProtoString { fn upb_type() -> upb::CType { upb::CType::String } - fn to_message_value(val: View<'_, ProtoStr>) -> upb_MessageValue { + fn to_message_value(val: View<'_, ProtoString>) -> upb_MessageValue { upb_MessageValue { str_val: val.as_bytes().into() } } unsafe fn to_message_value_copy_if_required( raw_arena: RawArena, - val: View<'_, ProtoStr>, + val: View<'_, ProtoString>, ) -> upb_MessageValue { // SAFETY: `raw_arena` is valid as promised by the caller unsafe { - <[u8] as UpbTypeConversions>::to_message_value_copy_if_required( + ::to_message_value_copy_if_required( raw_arena, val.as_bytes(), ) } } - unsafe fn from_message_value<'msg>(msg: upb_MessageValue) -> View<'msg, ProtoStr> { + unsafe fn from_message_value<'msg>(msg: upb_MessageValue) -> View<'msg, ProtoString> { unsafe { ProtoStr::from_utf8_unchecked(msg.str_val.as_ref()) } } } @@ -734,13 +761,13 @@ macro_rules! impl_ProxiedInMapValue_for_key_types { ($($t:ty),*) => { $( impl_ProxiedInMapValue_for_non_generated_value_types!( - $t ; f32, f64, i32, u32, i64, u64, bool, ProtoStr, [u8] + $t ; f32, f64, i32, u32, i64, u64, bool, ProtoString, ProtoBytes ); )* } } -impl_ProxiedInMapValue_for_key_types!(i32, u32, i64, u64, bool, ProtoStr); +impl_ProxiedInMapValue_for_key_types!(i32, u32, i64, u64, bool, ProtoString); /// `upb_Map_Insert`, but returns a `bool` for whether insert occurred. /// diff --git a/src/google/protobuf/compiler/rust/accessors/singular_string.cc b/src/google/protobuf/compiler/rust/accessors/singular_string.cc index 0962229bdc..43774deff3 100644 --- a/src/google/protobuf/compiler/rust/accessors/singular_string.cc +++ b/src/google/protobuf/compiler/rust/accessors/singular_string.cc @@ -49,7 +49,7 @@ void SingularString::InMsgImpl(Context& ctx, const FieldDescriptor& field, {"getter", [&] { ctx.Emit(R"rs( - pub fn $field$($view_self$) -> &$view_lifetime$ $proxied_type$ { + pub fn $field$($view_self$) -> $pb$::View<$view_lifetime$, $proxied_type$> { let view = unsafe { $getter_thunk$(self.raw_msg()).as_ref() }; $transform_view$ })rs"); @@ -58,7 +58,7 @@ void SingularString::InMsgImpl(Context& ctx, const FieldDescriptor& field, [&] { if (!field.has_presence()) return; ctx.Emit(R"rs( - pub fn $raw_field_name$_opt($view_self$) -> $pb$::Optional<&$view_lifetime$ $proxied_type$> { + pub fn $raw_field_name$_opt($view_self$) -> $pb$::Optional<$pb$::View<$view_lifetime$, $proxied_type$>> { $pb$::Optional::new( self.$field$(), self.has_$raw_field_name$() @@ -69,13 +69,17 @@ void SingularString::InMsgImpl(Context& ctx, const FieldDescriptor& field, {"setter", [&] { if (accessor_case == AccessorCase::VIEW) return; - ctx.Emit(R"rs( - // TODO: Use IntoProxied once string/bytes types support it. - pub fn set_$raw_field_name$(&mut self, val: impl std::convert::AsRef<$proxied_type$>) { + ctx.Emit({{"as_ref_method", + (field.type() == FieldDescriptor::TYPE_STRING + ? "as_bytes()" + : "as_ref()")}}, + R"rs( + pub fn set_$raw_field_name$(&mut self, val: impl $pb$::IntoProxied<$proxied_type$>) { + let into_proxied = val.into_proxied($pbi$::Private); let string_view: $pbr$::PtrAndLen = $pbr$::copy_bytes_in_arena_if_needed_by_runtime( self.as_mutator_message_ref($pbi$::Private), - val.as_ref().into() + into_proxied.$as_ref_method$, ).into(); unsafe { diff --git a/src/google/protobuf/compiler/rust/naming.cc b/src/google/protobuf/compiler/rust/naming.cc index a0dfb961b6..df910264be 100644 --- a/src/google/protobuf/compiler/rust/naming.cc +++ b/src/google/protobuf/compiler/rust/naming.cc @@ -191,9 +191,9 @@ std::string RsTypePath(Context& ctx, const FieldDescriptor& field) { case RustFieldType::DOUBLE: return "f64"; case RustFieldType::BYTES: - return "[u8]"; + return "::__pb::ProtoBytes"; case RustFieldType::STRING: - return "::__pb::ProtoStr"; + return "::__pb::ProtoString"; case RustFieldType::MESSAGE: return GetFullyQualifiedPath(ctx, *field.message_type()); case RustFieldType::ENUM: @@ -449,8 +449,8 @@ PROTOBUF_CONSTINIT const MapKeyType kMapKeyTypes[] = { /*cc_key_t=*/"bool", /*cc_ffi_key_t=*/"bool", /*cc_from_ffi_key_expr=*/"key", /*cc_to_ffi_key_expr=*/"cpp_key"}, - {/*thunk_ident=*/"ProtoStr", - /*rs_key_t=*/"$pb$::ProtoStr", + {/*thunk_ident=*/"ProtoString", + /*rs_key_t=*/"$pb$::ProtoString", /*rs_ffi_key_t=*/"$pbr$::PtrAndLen", /*rs_to_ffi_key_expr=*/"key.as_bytes().into()", /*rs_from_ffi_key_expr=*/ From 7122ba1d0a93532324ab29cf05bd3fa4fd99e45e Mon Sep 17 00:00:00 2001 From: Mark Hansen Date: Thu, 20 Jun 2024 08:36:52 -0700 Subject: [PATCH 32/32] Java: Deprecate newExtensionWriter. New usages should use newExtensionSerializer, which can avoid allocations for empty field sets. PiperOrigin-RevId: 645044533 --- .../main/java/com/google/protobuf/GeneratedMessage.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/java/core/src/main/java/com/google/protobuf/GeneratedMessage.java b/java/core/src/main/java/com/google/protobuf/GeneratedMessage.java index 426adb61bf..aeeeb94e3d 100644 --- a/java/core/src/main/java/com/google/protobuf/GeneratedMessage.java +++ b/java/core/src/main/java/com/google/protobuf/GeneratedMessage.java @@ -1094,7 +1094,14 @@ public abstract class GeneratedMessage extends AbstractMessage implements Serial } } - // TODO: Remove, replace with newExtensionSerializer(). + /** + * For compatibility with older gencode. + * + *

TODO Remove this in the next breaking release. + * + * @deprecated Use {@link newExtensionSerializer()} instead. + */ + @Deprecated protected ExtensionWriter newExtensionWriter() { return new ExtensionWriter(false); }