search

lcompilers / lpython

Python compiler
https://lpython.org/
Other
1.5k stars 162 forks source link

Remove `const` from all visitors #946

Open certik opened 2 years ago

certik commented 2 years ago

Most visitors need to modify the ASR, and duplicating them just to be able to modify ASR will add more code, slower compilation, etc.

I think the AST->ASR does not modify AST, so having that visitor const is good to make sure we don't modify it accidentally.

The ASR->LLVM also does not modify ASR, so having that visitor const is also good, for code safety.

But almost all ASR passes modify ASR in place, so all those base visitors should not be const.

xmnlab commented 2 years ago

hey @certik if you don't mind, I would love to contribute to this issue. as it would be my first contribution to the project maybe it could take some time. do you have any deadline in mind for this issue?

Thirumalai-Shaktivel commented 2 years ago

Thanks for your interest in contributing to LPython. We appreciate it!  Go ahead, make the required changes, and submit the PR.  If you have any doubts, please ask.

xmnlab commented 2 years ago

thank you so much! Probably I will have questions about that :) I started to take a look into that ... and tomorrow I have a meeting with @czgdp1807 and I will post here any question I have :)

thank you so much

khushi-411 commented 1 year ago

Hi, @xmnlab, I wonder if you are still planning to work on this issue. If not, I'd like to pick this. :-) I am looking forward to hearing from you. Thank you!

xmnlab commented 1 year ago

Hi @khushi-411 ! Sorry, I've been very busy and I didn't have time to move this forward. please go ahead! Thanks

khushi-411 commented 1 year ago

Thanks for confirming, @xmnlab! Happy to work on it