Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix handling of -Infinity during json parsing #706

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions source/core/TDCodecVia.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1074,6 +1074,21 @@ Boolean TDViaParser::EatJSONItem(SubString* input)
return true;
Copy link
Member

@rajsite rajsite Jul 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shivaprasad-basavaraj Our PR workflow also includes making sure the squad workflow tests are run with the local changes and including a link to the CI build (Ideally also with some tests added on the ASW side as well). Example Vireo PR with referenced build links.

}

bool IsInfinityOrNan(SubString token)
{
bool isInfinityOrNan = false;
ConstCStr specialIEEEVals[] = { "infinity", "-infinity", "inf", "-inf", "nan", nullptr};
ConstCStr* specValPtr = specialIEEEVals;
while (*specValPtr) {
if (token.ComparePrefixCStrIgnoreCase(*specValPtr)) {
Comment on lines +1080 to +1083
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behavior of the UnflattenFromJSON node should match LabVIEW's Unflatten From JSON which is stricter about the allowed representations:

image

The TDViaParser is used both for parsing VIA text content and for implementing the UnflattenFromJSON node. We kind of use Fmt().UseFieldNames() as a hint that we are doing the UnflattenFromJSON semantics for parsing. We should either use that as a hint to make sure that the UnflattenFromJSON node will align behavior with LabVIEW or add a new hint for this use-case, something like Fmt().LabVIEWJSONExtensions() since this is JSON extensions that are LabVIEW-specific.

image

isInfinityOrNan = true;
break;
}
++specValPtr;
}
return isInfinityOrNan;
}

//------------------------------------------------------------
// ParseData - parse a value from the string based on the type
// If the text makes sense then kNIError_Success is returned.
Expand Down Expand Up @@ -1202,9 +1217,7 @@ Int32 TDViaParser::ParseData(TypeRef type, void* pData)
TokenTraits tt = _string.ReadToken(&token, suppressInfNaN);
token.TrimQuotedString(tt);
Double value = 0.0;
Utf8Char leadingChar = token.StringLength() > 0 ? token.Begin()[0] : 0;
Boolean isMundane = (tt == TokenTraits_IEEE754 || tt == TokenTraits_Integer) &&
(leadingChar == '.' || leadingChar == '-' || isdigit(leadingChar)); // not inf, nan
Boolean isMundane = (tt == TokenTraits_IEEE754 || tt == TokenTraits_Integer) && !IsInfinityOrNan(token); // not inf, nan
Boolean readSuccess = token.ParseDouble(&value, suppressInfNaN, &errCode);
if (!readSuccess) {
if (_options._allowNulls && tt == TokenTraits_SymbolName && token.CompareCStr("null"))
Expand Down
3 changes: 3 additions & 0 deletions test-it/ExpectedResults/UnflattenJSONWithInfinity.vtr
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
000 All tests pass: 'true'.
010 No error: 'true'.
020 Correct values: 'true'.
48 changes: 48 additions & 0 deletions test-it/ViaTests/UnflattenJSONWithInfinity.via
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Autogenerated Vireo assembly file
// Generated on 07/16/2024 14:20:22
//---------------------------------------------------
//VI Definition: ::VireoDevSystem::Interactive::WebApp::UnflattenJSONWithInfinity.gviweb
define (WebApp%3A%3AUnflattenJSONWithInfinity%2Egviweb dv(.VirtualInstrument (
Locals: c( // Data Space
e(dv(.String 'WebApp::UnflattenJSONWithInfinity.gviweb')local0)
ce(dv(.ErrorCluster (false 0 '' ))c1)
ce(dv(.String '[1,2,-Infinity,inf,5]')c2)
ce(dv(a(.Double *) ())c3)
e(a(.Double *) local4)
e(.ErrorCluster local5)
e(.ErrorCluster local6)
e(.Int32 local7)
e(.Boolean local8)
de(dv(.Boolean false)dataItem_No_Error)
e(.Boolean local10)
ce(dv(a(.Double *) (1 2 -inf inf 5))c11)
e(.Boolean local12)
de(dv(.Boolean false)dataItem_CorrectValues)
e(.Boolean local14)
e(.Boolean local15)
de(dv(.Boolean false)dataItem_AllPass)
e(.Boolean local17)
e(a(.String *) local18)
e(dv(.Boolean true)local19)
e(dv(.Boolean false)local20)
e(dv(.Boolean false)local21)
)
clump(1
Copy(c1 local5)
Copy(c3 local4)
UnflattenFromJSON(c2 local4 local18 local19 local20 local21 local5 )
Copy(local5 local6)
Copy(local6.code local7)
IsEQ0(local7 local8)
Copy(local8 dataItem_No_Error)
IsEQ(c11 local4 local12)
Copy(local12 dataItem_CorrectValues)
And(local12 local8 local15)
Copy(local15 dataItem_AllPass)
Printf ("000 All tests pass: '%s'.\n" dataItem_AllPass)
Printf ("010 No error: '%s'.\n" dataItem_No_Error)
Printf ("020 Correct values: '%s'.\n" dataItem_CorrectValues)
/* Clump Ended. */ )
)))
enqueue (WebApp%3A%3AUnflattenJSONWithInfinity%2Egviweb)
//Finished!! :D
1 change: 1 addition & 0 deletions test-it/testList.json
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@
"UnflattenJSONNested.via",
"UnflattenJSONPath.via",
"UnflattenJSONStrict.via",
"UnflattenJSONWithInfinity.via",
"UnflattenJSONWithNullValues.via",
"UnflattenPathToIndex.via",
"UnflattenStrictValidation.via",
Expand Down