ni / VireoSDK

Compact runtime for a subset of LabVIEW
Other
49 stars 44 forks source link

Add an option to disable parsing of debug point instructions #697

Closed shivaprasad-basavaraj closed 2 years ago

shivaprasad-basavaraj commented 2 years ago

Added an option that can be passed into the TDViaParser to instruct it to skip parsing the DebugPoint instructions. Made changes to the eggshell loadVia call as well so that the disable debugging option can be passed in.

Added tests for the new option.

shivaprasad-basavaraj commented 2 years ago

@spathiwa @rajsite @sanmut In the initial draft PR for this change @rajsite had suggested that the option to disable debugging be part of the TypeManager so that we don't have to pass the option around. I wasn't sure if TypeManager is the right place since the option has meaning only in the context of a parse. Hence I have made it a variable in the TDViaParser class which can be used to decide whether or not to parse the debug point instruction.

Unfortunately, we create new parsers during the parse of a Via file and hence had to explicitly pass the option to the new parsers that were created. I also had to add the option as a parameter to the FinalizeVILoad and FinalizeModuleLoad methods since these were static. I am not sure why that is since these methods aren't being used outside the TDViaParser.

spathiwa commented 2 years ago

I don't think there is any good reason for FInalizeVILoad and FInalizeModuleLoad to be static other than they didn't happen to need to access any member variables. I think making them non-static to avoid needing to pass the new parameter would be fine.

spathiwa commented 2 years ago

Otherwise, looks good to me. I agree that TDViaParser is a logical place for the variable versus the type manager.

shivaprasad-basavaraj commented 2 years ago

I don't think there is any good reason for FInalizeVILoad and FInalizeModuleLoad to be static other than they didn't happen to need to access any member variables. I think making them non-static to avoid needing to pass the new parameter would be fine.

I have made the two methods non-static to avoid passing the parameter around.