Support --no-loc to skip location info in Json #2709

Shaikh-Ubaid · 2023-10-24T17:00:53Z

This PR supports a --no-loc flag which enables the user to skip the location information in the generated AST/R Json format.

$ lfortran examples/expr2.f90 --show-asr --json --no-loc
{
    "node": "TranslationUnit",
    "fields": {
        "symtab": {
            "node": "SymbolTable1",
            "fields": {
                "expr2": {
                    "node": "Program",
                    "fields": {
                        "symtab": {
                            "node": "SymbolTable2",
                            "fields": {
                                "x": {
                                    "node": "Variable",
                                    "fields": {
                                        "parent_symtab": 2,
                                        "name": "x",
                                        "dependencies": [],
                                        "intent": "Local",
                                        "symbolic_value": [],
                                        "value": [],
                                        "storage": "Default",
                                        "type": {
                                            "node": "Integer",
                                            "fields": {
                                                "kind": 4
                                            }
                                        },
                                        "type_declaration": [],
                                        "abi": "Source",
                                        "access": "Public",
                                        "presence": "Required",
                                        "value_attr": false
                                    }
                                }
                            }
                        },
                        "name": "expr2",
                        "dependencies": [],
                        "body": [
                            {
                                "node": "Assignment",
                                "fields": {
                                    "target": {
                                        "node": "Var",
                                        "fields": {
                                            "v": "x (SymbolTable2)"
                                        }
                                    },
                                    "value": {
                                        "node": "IntegerBinOp",
                                        "fields": {
                                            "left": {
                                                "node": "IntegerBinOp",
                                                "fields": {
                                                    "left": {
                                                        "node": "IntegerConstant",
                                                        "fields": {
                                                            "n": 2,
                                                            "type": {
                                                                "node": "Integer",
                                                                "fields": {
                                                                    "kind": 4
                                                                }
                                                            }
                                                        }
                                                    },
                                                    "op": "Add",
                                                    "right": {
                                                        "node": "IntegerConstant",
                                                        "fields": {
                                                            "n": 3,
                                                            "type": {
                                                                "node": "Integer",
                                                                "fields": {
                                                                    "kind": 4
                                                                }
                                                            }
                                                        }
                                                    },
                                                    "type": {
                                                        "node": "Integer",
                                                        "fields": {
                                                            "kind": 4
                                                        }
                                                    },
                                                    "value": {
                                                        "node": "IntegerConstant",
                                                        "fields": {
                                                            "n": 5,
                                                            "type": {
                                                                "node": "Integer",
                                                                "fields": {
                                                                    "kind": 4
                                                                }
                                                            }
                                                        }
                                                    }
                                                }
                                            },
                                            "op": "Mul",
                                            "right": {
                                                "node": "IntegerConstant",
                                                "fields": {
                                                    "n": 5,
                                                    "type": {
                                                        "node": "Integer",
                                                        "fields": {
                                                            "kind": 4
                                                        }
                                                    }
                                                }
                                            },
                                            "type": {
                                                "node": "Integer",
                                                "fields": {
                                                    "kind": 4
                                                }
                                            },
                                            "value": {
                                                "node": "IntegerConstant",
                                                "fields": {
                                                    "n": 25,
                                                    "type": {
                                                        "node": "Integer",
                                                        "fields": {
                                                            "kind": 4
                                                        }
                                                    }
                                                }
                                            }
                                        }
                                    },
                                    "overloaded": []
                                }
                            },
                            {
                                "node": "Print",
                                "fields": {
                                    "values": [
                                        {
                                            "node": "Var",
                                            "fields": {
                                                "v": "x (SymbolTable2)"
                                            }
                                        }
                                    ],
                                    "separator": [],
                                    "end": []
                                }
                            }
                        ]
                    }
                }
            }
        },
        "items": []
    }
}

Shaikh-Ubaid · 2023-10-24T17:02:46Z

We have the flag --dump-all-passes, but it only prints the asr in text format. I often use fortran, the json and the visualisation format to debug codes. This PR supports applying all passes to fortran, json, viz and tree formats.

certik · 2023-10-24T17:09:52Z

src/bin/lfortran.cpp

@@ -2058,6 +2063,7 @@ int main(int argc, char *argv[])
        app.add_flag("--legacy-array-sections", compiler_options.legacy_array_sections, "Enables passing array items as sections if required");
        app.add_flag("--ignore-pragma", compiler_options.ignore_pragma, "Ignores all the pragmas");
        app.add_flag("--stack-arrays", compiler_options.stack_arrays, "Allocate memory for arrays on stack");
+        app.add_flag("--all-passes", compiler_options.all_passes, "Apply all ASR passes to --show-asr and --show-fortran");


Why not adding an option like --dump-fortran (or something like that) to dump Fortran together with Clojure in --dump-all-passes.

The same with JSON and --visualize.

certik · 2023-10-24T17:15:30Z

How about this:

--show-asr is telling LFortran to print ASR as Clojure like output, which is exactly 1:1 with the internal ASR representation in memory
--show-fortran is telling LFortran to print ASR as Fortran code (directly)
--show-python (TODO) will print LPython code
--json will print ASR as JSON
--tree
--visualize

These (and more!) are various options that control how ASR is printed.

Then we have options that control which exact ASR gets printed:

By default the above prints ASR before any passes are applied.

Then --dump-all-passes will print the above for all passes.

Let's start with the above. If we need to, we can later add --last-pass to print the ASR after the last pass.

We also have options to control which passes to run, which would affect --dump-all-passes.

Thirumalai-Shaktivel · 2023-10-25T08:26:59Z

@Shaikh-Ubaid I thought of working on --dump-fortran; did you start working on it?

Shaikh-Ubaid · 2023-10-25T08:54:14Z

@Shaikh-Ubaid I thought of working on --dump-fortran; did you start working on it?

I will take care of these Thirumalai. There are tons of other tasks waiting to be fixed. Please focus on them.

Shaikh-Ubaid · 2023-10-25T10:25:58Z

I updated the PR description. This is ready. Please let me know if a reference test is to be added. I feel it is not needed. (Or can be added later if needed).

certik · 2023-10-25T20:35:45Z

src/libasr/pickle.h

-    std::string pickle_json(ASR::asr_t &asr, LocationManager &lm, bool show_intrinsic_modules=false);
-    std::string pickle_json(ASR::TranslationUnit_t &asr, LocationManager &lm, bool show_intrinsic_modules=false);
+    std::string pickle_json(ASR::asr_t &asr, LocationManager &lm, bool no_loc=false, bool show_intrinsic_modules=false);
+    std::string pickle_json(ASR::TranslationUnit_t &asr, LocationManager &lm, bool no_loc=false, bool show_intrinsic_modules=false);


Let's not use default arguments, it leads to issues with overloads and clarity of what the values actually are. For example, if there is any code in LFortran that just called pickle_json(asr, lm, true), then after this PR this very same code now will evaluate to no_loc=true and show_intrinsic_modules=false, which is not what was intended.

To fix this, do it like this:

Suggested change

std::string pickle_json(ASR::TranslationUnit_t &asr, LocationManager &lm, bool no_loc=false, bool show_intrinsic_modules=false);

std::string pickle_json(ASR::TranslationUnit_t &asr, LocationManager &lm, bool no_loc, bool show_intrinsic_modules);

And then modify all call sites to supply all arguments explicitly.

certik · 2023-10-25T20:36:23Z

src/lfortran/pickle.h

-    std::string pickle_json(AST::ast_t &ast, LocationManager &lm);
-    std::string pickle_json(AST::TranslationUnit_t &ast, LocationManager &lm);
+    std::string pickle_json(AST::ast_t &ast, LocationManager &lm, bool no_loc=false);
+    std::string pickle_json(AST::TranslationUnit_t &ast, LocationManager &lm, bool no_loc=false);


same here, no default arguments.

certik · 2023-10-25T20:36:53Z

Otherwise it looks good.

certik reviewed Oct 24, 2023

View reviewed changes

Shaikh-Ubaid mentioned this pull request Oct 25, 2023

Support --dump-all-passes-fortran option #2712

Merged

Shaikh-Ubaid force-pushed the add_flags_all_passes_and_no_loc branch from a11c1a8 to ba03b81 Compare October 25, 2023 10:21

Shaikh-Ubaid changed the title ~~Add --all-passes and --no-loc flags~~ Support --no-loc to skip location info in Json Oct 25, 2023

Shaikh-Ubaid requested a review from certik October 25, 2023 20:31

certik reviewed Oct 25, 2023

View reviewed changes

certik marked this pull request as draft October 25, 2023 20:36

Shaikh-Ubaid force-pushed the add_flags_all_passes_and_no_loc branch from ba03b81 to a9d8cbc Compare October 25, 2023 20:54

Shaikh-Ubaid added 3 commits October 26, 2023 02:34

Provide value for use_loop_variable_after_loop to pass_options

ab9ae63

Add --no-loc flag to skip loc info in AST/R Json

e3a0df8

Remove default arguments from pickle_json()

698cdfa

Shaikh-Ubaid force-pushed the add_flags_all_passes_and_no_loc branch from a9d8cbc to 698cdfa Compare October 25, 2023 21:04

Shaikh-Ubaid marked this pull request as ready for review October 25, 2023 21:04

Shaikh-Ubaid requested a review from certik October 25, 2023 21:05

certik approved these changes Oct 25, 2023

View reviewed changes

certik merged commit 176d878 into lfortran:main Oct 25, 2023
20 checks passed

Shaikh-Ubaid deleted the add_flags_all_passes_and_no_loc branch October 25, 2023 22:18

Shaikh-Ubaid mentioned this pull request Oct 26, 2023

Support dumping ASR pass outputs in Json, Tree and Visualization forms #2733

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support --no-loc to skip location info in Json #2709

Support --no-loc to skip location info in Json #2709

Shaikh-Ubaid commented Oct 24, 2023 •

edited

Shaikh-Ubaid commented Oct 24, 2023

certik Oct 24, 2023 •

edited

certik commented Oct 24, 2023 •

edited

Thirumalai-Shaktivel commented Oct 25, 2023

Shaikh-Ubaid commented Oct 25, 2023 •

edited

Shaikh-Ubaid commented Oct 25, 2023

certik Oct 25, 2023 •

edited

certik Oct 25, 2023

certik commented Oct 25, 2023

	std::string pickle_json(ASR::TranslationUnit_t &asr, LocationManager &lm, bool no_loc=false, bool show_intrinsic_modules=false);
	std::string pickle_json(ASR::TranslationUnit_t &asr, LocationManager &lm, bool no_loc, bool show_intrinsic_modules);

Support --no-loc to skip location info in Json #2709

Support --no-loc to skip location info in Json #2709

Conversation

Shaikh-Ubaid commented Oct 24, 2023 • edited

Shaikh-Ubaid commented Oct 24, 2023

certik Oct 24, 2023 • edited

Choose a reason for hiding this comment

certik commented Oct 24, 2023 • edited

Thirumalai-Shaktivel commented Oct 25, 2023

Shaikh-Ubaid commented Oct 25, 2023 • edited

Shaikh-Ubaid commented Oct 25, 2023

certik Oct 25, 2023 • edited

Choose a reason for hiding this comment

certik Oct 25, 2023

Choose a reason for hiding this comment

certik commented Oct 25, 2023

Shaikh-Ubaid commented Oct 24, 2023 •

edited

certik Oct 24, 2023 •

edited

certik commented Oct 24, 2023 •

edited

Shaikh-Ubaid commented Oct 25, 2023 •

edited

certik Oct 25, 2023 •

edited