From 19c81b11b3b3ed1949b64134184c4bf5144f3564 Mon Sep 17 00:00:00 2001 From: Masato Nagai Date: Sat, 17 Feb 2018 01:32:50 +0900 Subject: [PATCH] [C++] better type mismatch error (#4623) * better parse error * pass str as a pointer instead of a reference for more efficient performance --- include/flatbuffers/idl.h | 4 +-- src/idl_parser.cpp | 26 ++++++++++-------- tests/monster_test.bfbs | Bin 5400 -> 5440 bytes .../namespace_test2_generated.ts | 20 +++++++------- 4 files changed, 26 insertions(+), 24 deletions(-) diff --git a/include/flatbuffers/idl.h b/include/flatbuffers/idl.h index 34441750f..3cf0b1a11 100644 --- a/include/flatbuffers/idl.h +++ b/include/flatbuffers/idl.h @@ -647,11 +647,11 @@ class Parser : public ParserState { size_t fieldn, const StructDef *parent_struct_def); FLATBUFFERS_CHECKED_ERROR ParseMetaData(SymbolTable *attributes); - FLATBUFFERS_CHECKED_ERROR TryTypedValue(int dtoken, bool check, Value &e, + FLATBUFFERS_CHECKED_ERROR TryTypedValue(const std::string *name, int dtoken, bool check, Value &e, BaseType req, bool *destmatch); FLATBUFFERS_CHECKED_ERROR ParseHash(Value &e, FieldDef* field); FLATBUFFERS_CHECKED_ERROR TokenError(); - FLATBUFFERS_CHECKED_ERROR ParseSingleValue(Value &e); + FLATBUFFERS_CHECKED_ERROR ParseSingleValue(const std::string *name, Value &e); FLATBUFFERS_CHECKED_ERROR ParseEnumFromString(Type &type, int64_t *result); StructDef *LookupCreateStruct(const std::string &name, bool create_if_new = true, diff --git a/src/idl_parser.cpp b/src/idl_parser.cpp index 786dc63b8..6ebf7e9f5 100644 --- a/src/idl_parser.cpp +++ b/src/idl_parser.cpp @@ -648,7 +648,7 @@ CheckedError Parser::ParseField(StructDef &struct_def) { if (token_ == '=') { NEXT(); - ECHECK(ParseSingleValue(field->value)); + ECHECK(ParseSingleValue(&field->name, field->value)); if (!IsScalar(type.base_type) || (struct_def.fixed && field->value.constant != "0")) return Error( @@ -878,11 +878,11 @@ CheckedError Parser::ParseAnyValue(Value &val, FieldDef *field, (token_ == kTokenIdentifier || token_ == kTokenStringConstant)) { ECHECK(ParseHash(val, field)); } else { - ECHECK(ParseSingleValue(val)); + ECHECK(ParseSingleValue(field ? &field->name : nullptr, val)); } break; } - default: ECHECK(ParseSingleValue(val)); break; + default: ECHECK(ParseSingleValue(field ? &field->name : nullptr, val)); break; } return NoError(); } @@ -1201,7 +1201,7 @@ CheckedError Parser::ParseMetaData(SymbolTable *attributes) { attributes->Add(name, e); if (Is(':')) { NEXT(); - ECHECK(ParseSingleValue(*e)); + ECHECK(ParseSingleValue(&name, *e)); } if (Is(')')) { NEXT(); @@ -1213,7 +1213,7 @@ CheckedError Parser::ParseMetaData(SymbolTable *attributes) { return NoError(); } -CheckedError Parser::TryTypedValue(int dtoken, bool check, Value &e, +CheckedError Parser::TryTypedValue(const std::string *name, int dtoken, bool check, Value &e, BaseType req, bool *destmatch) { bool match = dtoken == token_; if (match) { @@ -1225,7 +1225,9 @@ CheckedError Parser::TryTypedValue(int dtoken, bool check, Value &e, } else { return Error(std::string("type mismatch: expecting: ") + kTypeNames[e.type.base_type] + - ", found: " + kTypeNames[req]); + ", found: " + kTypeNames[req] + + ", name: " + (name ? *name : "") + + ", value: " + e.constant); } } NEXT(); @@ -1310,13 +1312,13 @@ CheckedError Parser::TokenError() { return Error("cannot parse value starting with: " + TokenToStringId(token_)); } -CheckedError Parser::ParseSingleValue(Value &e) { +CheckedError Parser::ParseSingleValue(const std::string *name, Value &e) { // First see if this could be a conversion function: if (token_ == kTokenIdentifier && *cursor_ == '(') { auto functionname = attribute_; NEXT(); EXPECT('('); - ECHECK(ParseSingleValue(e)); + ECHECK(ParseSingleValue(name, e)); EXPECT(')'); // clang-format off #define FLATBUFFERS_FN_DOUBLE(name, op) \ @@ -1362,17 +1364,17 @@ CheckedError Parser::ParseSingleValue(Value &e) { } } else { bool match = false; - ECHECK(TryTypedValue(kTokenIntegerConstant, IsScalar(e.type.base_type), e, + ECHECK(TryTypedValue(name, kTokenIntegerConstant, IsScalar(e.type.base_type), e, BASE_TYPE_INT, &match)); - ECHECK(TryTypedValue(kTokenFloatConstant, IsFloat(e.type.base_type), e, + ECHECK(TryTypedValue(name, kTokenFloatConstant, IsFloat(e.type.base_type), e, BASE_TYPE_FLOAT, &match)); - ECHECK(TryTypedValue(kTokenStringConstant, + ECHECK(TryTypedValue(name, kTokenStringConstant, e.type.base_type == BASE_TYPE_STRING, e, BASE_TYPE_STRING, &match)); auto istrue = IsIdent("true"); if (istrue || IsIdent("false")) { attribute_ = NumToString(istrue); - ECHECK(TryTypedValue(kTokenIdentifier, IsBool(e.type.base_type), e, + ECHECK(TryTypedValue(name, kTokenIdentifier, IsBool(e.type.base_type), e, BASE_TYPE_BOOL, &match)); } if (!match) return TokenError(); diff --git a/tests/monster_test.bfbs b/tests/monster_test.bfbs index 843e308c5204d9c36bad66ae49ac2d37041b4cce..4cb46a7ca5a2f046e7e4daa2b4c5d2a7e1bc5bf5 100644 GIT binary patch literal 5440 zcmai2U1%KF6+SDiR;%AtY(-HWRUM~Bh+;%6slu3IWYu<*5X;6^)jkxfBkinr!tBg4 zJG+&P$wMf{lwd+BCQl`lU`hyn2*wYl48Tl*VFI2cSah` ztkWxw?%Z?F|2=c=*%=d&h2slnFkuYJsN|(6{W2uOScx12>;nt~ihy#LNGIl+=Ymq? z#K|SaOWY~In>i7uLm)xq*Cmlz(BFZe#a$wAJtJ}#d$U6#`Q4BWxChvP@G9mZXs7OO zJW(V6W8`M>Zwx@0)XDMLU&0U*v}xQU%zpsX0i=aGIkEYYS+k~&ZJ4#XV@(}#H${FQ zh9Uj=1qdjgcE{uJ=SpU&8;IusntwIopI$xTxsAZ`VaFKk7*e*X;+poww!Hr&4CA=G zgJZL5^@m@gW+sUA6`Af)-Gj(&_nmX=#wU^y>r&*>G{rIC#^ipfl z{^o%tz84Ttub7U=6X;q5F1B0qY70LG{vE(|81XYeKj0DI0$>tw7kjrbFJs;Z*aJw# ze|fWR9dT^4VKqc$b@`;L5K0`MSevqj1utB#3zE6R86@b)G zXB^K5ur^HBuyj!~%HDdjX1ReG*q&>wdo?Z&87R*zRff0rnzbH)=evM;6plCX@-g>r z;rS9?qblY*m}dc(K=TOm^@2zM*ay0Mm~X*>0MCf$wS@HJiyP+)<-5B|M!JQHWm;`ysyWwq=zQH=3w$;dy#(2%1UzIQcW z53aKWpX0gb8uAKwz9S16TR-@>(p7@%f^Z2<`#$FLpdXj{jE5g2>d2a{Q>yL78YsTX3?5L6##Eim;|*2x*0MdQyawiB0PB?=#d*#n(1U4P z9(0q#_y&33k~vi;M?uMY>t1~Ri?8?g%;Qmw*CLs5%qze?1a4GjGyYuAaqOU+L>$&U z&tW>q1nT5-GM(YOt#$M=kH}Xw8&%Y^)r#9XU;=j*d?WH=hOeONuJk1N8VSBD*nd{0 zGJJbqlDv@N zyR7*-SVwq%JaE#FJPN}>WM8_MSkZo!P{O1q;<0M=0PUo_0-gaG&*-@KU-gdWnfKJ; z0<3byF@v>crGh3B<05$3OQ!XG6zR)>hIwqcR*kxDfSz|h>of}lYXxm0KcCsJ8?~kr z*pBU5M&NnqQ;7Tj_eD^(8^+IV$1yCoY&7eJ?HXv2jC0WKo3I>Li|UPjiCzqvO0+G` ztElz_Y-;)9oJGBC*LQr5{uI0eDLzyy)E)3$0U!I2Nj&qc>60G4IQIhj-lFQgs6Sa( zXF>D{c-fnzW4Q}4!!}by$>Uk@xdSzjJxZE>LDSQ{t+QSY(1Bt7A?#gz+c+Ou2>^XJkPySKt zsF7p8a|(|~0m*lR4sgv5mi;pAq`lP3y#oN^>b~vywn}BWs_q3*??uJ8kqHjcj=g5m z22u9X3T;C!*RGfPQp*~kK3>o}>pCM44}DLt&)Nr6T-VF-b~G=jS}k47_iCNU39@L(zEylJ|W^B!>Rddt(AR`tj&{0Ny+K4p8$^O|meakI<# z4W;Um_dw47;eXEC#RSa;Xl@|J^d+6g)b%Q00+7nzWtiLU69y>b zzK!*PClrtPjPpC{CeJlFZtC;iilMHXtk3ZmA<7(8-*xbiV~!F-In04L54gSnI0$HS z?zA5}d%&wJBlBi^Xj^lTnvUIv>fRG&-x~ObppiMgbM#|J=Ot~6ZDK@U5u{@ zn1f8}Nyi_k#P_H8`_=SR`~%7T20qG*&g^CPl=^|h&m8La5#_!I-lv^8?Vye7>(mjE zH6wrAB0E#xVwCoU?K9Fgfh8K*eAB7}H%;?m27N<4OWsZd!c$72VROJ3#*Yd4`eD1!Fi%Q8B4 z^{vk=9I$<4hwM9!z@7BcQSBMX%b#w_AD=#vCbfE8byvK+{taM0$g*sR{zoV{N{?Bs zCK+eqTK#lM`kg7vl;iJ~>hp(_M#bH3%8(EG+%|l%s*mK!Ios%3N;B4eL5JSIm`(j! zf9Jnh)@fzAHv4HT#e&|{t)YZoI;mL^m3)M^bGszTwaam z+=9sxRy#_A0kd0p+4ll80d0qxHlD1{$JHq!<@7>fHXb`tO)IY?v*pH|6qNi3I`_Hx zb7Ro2581LK*s0y5e=pSJK3gcu-}-ZI$6vjlM;S}zYtS&%=I1k)7UF~DQlS>ty!P*c z`62WcOoyNq#-Gxkhx(wK%zr^IyMm8DTgkE~kRJRmy`;lpalc&P=I_gvOWJ zd-T7?vL)y!xKHSBp@8CVl=1KAUq{9-(e`7g&RPVm@cetsm(cEy(h%Chd^i2u&`mI( zFu%&u2M};Gcn^J4NTS#$>1ZyUZB*i2pH`>S z;`ChFAR%)N-CJL--j_-EBHnh23+1&4;x(7!E2qhYn)|KivDjRM=NoR)c(#BQpMpZF zeT)A04^4i*+@^K|{Bv%ig%9_)SdN#pzSHCJrgL||e%-xf*x(1fS0uYotrjk&#adde z$8)uMwSng*|Lyk@uiD^0vuqo0OxY7blA(8Jp1)o#B^Snt8^7<9EW2ayJ?jpv(UI^l zzT#fz?xPn|yX&{f$-`u*V0*-ZZ#@$&9^~&rCeMvW*>RiQt1fPh&-I>X!1CtT-2OH4 zk}J=ye7Sw&MDS}7@N!PxL&ihA0rGb0{^abONz<~9+gRz5|}m?0cW_0MYr_{hIcPuj={CzAqT}yAk8lJqopm&QAv}{B@7F^B#GS zWt+GO+DG^ZlH$V0m4!Mbq$_$q^cw^7v+z9Wo--cZpPi6mT|9~S=8EM)eWpR_z)5%}=Qn{4GQ9VtmS@i7x=i&N%sTO^|R4zwxG8Z)#qEZr3K1CPMw_3nb zQnuAi7s-F2u85E=zqIK|Mk259`~h_+T3KGX4qvDrtTgaJr z;R|&~@S|+i8PK_5{6?n`wbn~63-w3)IZxNA!az-Vp5S_S89h~t1iXB2!RbYsp0mwD z`3=w8s*9J|FVrUyc-f`6DkY1&uhOc{x`#h+9TT@n>eG}5^;;|0Y++5T3|Sro(4lHU zhRI(MbPAGQ#cGLDDoeDxV!7LO${59(s%X{7!inTF^$XQfS}j?aJ7M=gz71QIxT=RU zUB#(77BxCTy(C%kkqpp_n9g-uPIY$1dLO7}5+>+8Cs4B~R_7PE|DH$3PaDQIuXVR+ z>!0dKwZCesa4z!gPs1|3^r#e)f*$=iCv(PpvRZ42U-7h%*4#1guYSFI)IMV_A4|6U zG74>m+Ul*-6fP!1*z?dnNNxG6)jBcwM~2O&jigS$gLOLF?gr!bT2Xo}@`GZ^<1Cks zm@TXNORjuZ!kLR6Xa1hDeB(N_F1_Usowde~s2LB=kQWpIlBHNV1F5})oDR74ES#O> z%$2A$l36?F&;7o7OdIB;bY}Uc`1NPTphIi&RbKaTudm7IwcOx!-`T`Q^e;e<@c-F( zcCR_{b=HqUl6w#O5c&zE){V^d=%&53t{Bu=C|B&IKjLj@<(B>6yRi6Cc*>UG>(*T_ zf?tPK0WV(#`MOVo-X<-(YmD1}KPbMIp#vUg_0ao_gYSdCgwy4o$>Dy+Sc3DICFWOh znD_9n*5yOVP`?9ei{&%8<58a`j7Rstw@{}kc;(H|!Wnn9d@LTtl$wuy|54#L{=8fM zQw8d4KYitq%pt2u8CVP}4kSl$tnnk_K)M^I%z9o|ACJDKWU1Lg4(YTr%lmEy#7FiX zwQJ367@hprg~09mKFdS-Gna1*c%_FoRx^*tY&VWP?IGQTk$$V+lfw6^@v+vAH`diI zHu**wi%(GClAN+1Kl+qmzkOJ0wHr-d;l-<7I@wpJgZ^0$bG2o&cQvmB9{FO% z)?B|OAVUvwLaSd}6nTR@mwme-?WJCKtf^<)_ZHJ&lI4~!G|uOartkb?B>VTxY95MM x(LM#M50qcRyVCVHe@yUf5Sx{Y{Fg)BeCbKEZ-+rw`S$U;ekW