Fix StackOverFlowException during conversion of complex expression

Description

Project `KKBOX_Swift_4_2.zip`:
https://drive.google.com/drive/folders/1WfrkPl5KydPz0riO2_qrpyiXh5w8jeqB?usp=sharing

File causing StackOverflowException: NSString+TextDirectionality.m (also mentioned in ):
http://take.ms/HomHY
http://take.ms/RMPdP

This needs to be fixed ASAP.
Confirmed with .NET Core version (currently in the Develop branch) on both Windows and macOS,
but seems to originally occurred on our website prior to moving to .NET 4.7.

See for the issues (in the Offline Converter and the website) caused by this issue.

Environment

None

Activity

Show:
Alex Petuschak
November 5, 2018, 2:25 PM

Confirmed that `NSString+TextDirectionality.m` is converted fine after your changes - accepting this as solved.

macOS:

Windows:

Alex Petuschak
November 5, 2018, 4:23 PM
Edited

The whole project conversion (for `KKBOX_Swift_4_2.zip`) on Windows resulted in the same fatal error, although I cannot confirm if it's caused by a Stack Overflow exception:
http://take.ms/BkLMi

Update: Got the same from Develop branch with the latest changes merged from `StackOverflowAndPerformanceFixes` - please try converting the project yourself.

This was for the commit id: `24a17af` - I will pull the latest changes and retest again.
Also, the cause of `ShouldNotBeVisitedException` thrown should be determined and fixed.

P.S. Log from the VS output window is attached.

Alex Petuschak
November 5, 2018, 5:07 PM

P.S. Just caught a case when the `ShouldNotBeVisitedException` is thrown (during conversion of `GPUImageKuwaharaRadius3Filter.h+.m`:
http://take.ms/gNVeW

This is due to TargetTypeVisitor.VisitErrorNode() being called from TargetTypeVisitor.Viist().

Can you suggest a more specific implementation of `TargetTypeVisitor.VisitErrorNode()`?

See XML comments to `TargetTypeVisitor` for how the return value should look like.
Maybe it's worth to return `UnknownTypeName` instead of throwing an exception for error nodes?

Alex Petuschak
November 5, 2018, 5:09 PM

P.P.S. Just restored the file on Goole Drive.

Ivan Kochurkin
November 5, 2018, 5:27 PM

Maybe it's worth to return `UnknownTypeName` instead of throwing an exception for error nodes?

Yes, I think it's a good idea. Exceptions are expensive.

Assignee

Ivan Kochurkin

Reporter

Alex Petuschak

Labels

None

Git Branch Name

StackOverflowAndPerformanceFixes

GitHub Issue

None

Components

Fix versions

Priority

Highest
Configure