-
Notifications
You must be signed in to change notification settings - Fork 165
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
Docs: Update CONTRIBUTING.md #2539
Open
kmr-srbh
wants to merge
9
commits into
lcompilers:main
Choose a base branch
from
kmr-srbh:docs
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
cab92bc
Update CONTRIBUTING.md
kmr-srbh 05d93cd
Merge branch 'main' into docs
kmr-srbh 5f2869e
Added another option for opening a PR
kmr-srbh c7cf6fb
Corrected the file path for CONTRIBUTING.md
kmr-srbh 064a6d2
Merge branch 'main' into docs
kmr-srbh 320acd5
Merge branch 'main' into docs
kmr-srbh 5a98b0c
Merge branch 'lcompilers:main' into docs
kmr-srbh 7315c19
Added the LFortran tutorial
kmr-srbh fbf788a
Merge branch 'main' into docs
kmr-srbh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,253 +1,65 @@ | ||
| # Contributing | ||
|
|
||
| We welcome contributions from anyone, even if you are new to open source. It | ||
| might sound daunting to contribute to a compiler at first, but please do, it is | ||
| not complicated. We will help you with any technical issues and help improve | ||
| your contribution so that it can be merged. | ||
| Hey! Thanks for showing interest in LPython! | ||
|
|
||
| ## Basic Setup | ||
| We welcome contributions from anyone, even if you are new to compilers or open source in general. | ||
| It might sound daunting at first to contribute to a compiler, but do not worry, it is not that complicated. | ||
| We will help you with any technical issues you face and provide support so your contribution gets merged. | ||
|
|
||
| To contribute, make sure your set up: | ||
| Code is not all we have though. You can help us write documentations and tutorials for other contributors. | ||
|
|
||
| * Your username + email | ||
| * Your `~/.gitconfig` | ||
| * Your shell prompt to display the current branch name | ||
| Let's get started! | ||
|
|
||
| ### Fork LFortran | ||
| ### Basic Setup | ||
|
|
||
| Step 1. Create a fork of the [project repository](https://gitlab.com/lfortran/lfortran) | ||
| Make sure you have set up LPython for development on your local machine and can do a successful build. If not, follow the instructions provided in the [installation-docs](./installation.md) for getting started. | ||
|
|
||
| Step 2. Set up your [SSH key](https://docs.gitlab.com/ee/ssh/) with GitLab | ||
| ### Creating a contribution | ||
|
|
||
| Step 3. Clone the project repository from GitLab and set up your remote repository | ||
| All contributions to a public repository on GitHub require the contributor to open a Pull Request (PR) against the repository. Follow the steps below to open a new PR. | ||
|
|
||
| ``` | ||
| git clone https://gitlab.com/lfortran/lfortran.git | ||
| cd lfortran | ||
| git remote add REMOTE_NAME [email protected]:YOUR_GITLAB_ID/lfortran.git | ||
| ``` | ||
| :fontawesome-solid-edit: `REMOTE_NAME` is the name of your remote repository and could be any name you like, for example your first name. | ||
| - Create a new branch | ||
| ```bash | ||
| git checkout -b [branch-name] | ||
| ``` | ||
| Replace `[branch-name]` with a suitable name relevant to the changes you desire to make. For example, if you plan to fix an issue, you can name your branch `fixes-issue25`. | ||
|
|
||
| :fontawesome-solid-edit: `YOUR_GITLAB_ID` is your user ID on GitLab and should be part of your account path. | ||
|
|
||
| You can use `git remote -v` to check if the new remote is set up correctly. | ||
| - Make changes in the relevant file(s). | ||
| Test the changes you make. Try building the project. Run some tests to verify your code does not break anything. | ||
|
|
||
| ### Send a New Merge Request | ||
| Check the [installation-docs](./installation.md) for more details on building and testing. | ||
|
|
||
| Step 1. Create a new branch | ||
| ``` | ||
| git checkout -b fix1 | ||
| ``` | ||
| - Once you are assured, go ahead and push a commit with the changed files. | ||
| ```bash | ||
| git add FILE1 (FILE2 ...) | ||
| git commit -m "YOUR_COMMIT_MESSAGE" | ||
| ``` | ||
| Replace `FILE1 (FILE2 ...)` with the file names (with file extension) of changed files. | ||
|
|
||
| Step 2. Make changes in relevant file(s) | ||
| Write a short but descriptive commit message. [Here](https://chris.beams.io/posts/git-commit/) are some great tips on writing good commit messages. | ||
|
|
||
| Step 3. Commit the changes: | ||
| - Open a Pull Request (PR) | ||
| - Go to your project fork on GitHub. | ||
| - Select the branch which contains your commit. | ||
|  | ||
| - Above the list of files, select the green button which says <kbd>Compare & pull request</kbd>. If you do not see this option, select the <kbd>Contribute</kbd> option. | ||
|  | ||
| - Type a descriptive title. Provide further details in the Description section below. If your PR fixes an issue, link it with the issue by appending `fixes #[issue-number]` to your PR title. | ||
|  | ||
| - Once you are done, select the <kbd>Create Pull Request</kbd> button to open a new PR. | ||
|
|
||
| Congratulations! You just made your first contribution! | ||
|
|
||
| ### What next? | ||
| Sit back and relax or go on a bug hunt. | ||
|
|
||
| ``` | ||
| git add FILE1 (FILE2 ...) | ||
| git commit -m "YOUR_COMMIT_MESSAGE" | ||
| ``` | ||
| We too have day jobs and reviewing a Pull Request (PR) may take some time. We request you to patiently wait for any feedback or action on your PR. If you are asked to make changes, push commits to the same branch to automatically include them in the open PR. | ||
|
|
||
| [Here](https://chris.beams.io/posts/git-commit/) are some great tips on writing good commit messages. | ||
|
|
||
| Step 4. Check to ensure that your changes look good | ||
|
|
||
| ``` | ||
| git log --pretty=oneline --graph --decorate --all | ||
| ``` | ||
| Step 5. Send the merge request | ||
|
|
||
| ``` | ||
| git push REMOTE_NAME fix1 | ||
| ``` | ||
|
|
||
| The command will push the new branch `fix1` into your remote repository `REMOTE_NAME` that you created earlier. Additionally, it will also display a link that you can click on to open the new merge request. After clicking on the link, write a title and a concise description then click the "Create" button. Yay you are now all set. | ||
|
|
||
| ## Add New Features | ||
|
|
||
| The example below shows the steps it would take to create a caret binary operator **^** which computes the average value of the two operands. | ||
|
|
||
| ### Create New Token(s) | ||
|
|
||
| We extend the *tokenizer.re* as well as *parser.yy* to add the new token **^**. | ||
| We also tell LFortran how to print the new token in *parser.cpp*. | ||
|
|
||
| :fontawesome-solid-code: *src/lfortran/parser/tokenizer.re* | ||
| ``` | ||
| // "^" { RET(TK_CARET) } | ||
| ``` | ||
|
|
||
| :fontawesome-solid-code: *src/lfortran/parser/parser.yy* | ||
| ``` | ||
| %token TK_CARET "^" | ||
| ``` | ||
|
|
||
| :fontawesome-solid-code: *src/lfortran/parser/parser.cpp* | ||
| ``` | ||
| std:string token2text(const int token) | ||
| { | ||
| switch (token) { | ||
| T(TK_CARET, "^") | ||
| } | ||
| } | ||
| ``` | ||
| The added code is tested with `lfortran --show-tokens examples2/expr2.f90` | ||
|
|
||
| ### Parse the New Token | ||
|
|
||
| Now we have to parse the new operator. We add it to the AST by extending the BinOp with a caret operator and modifying the *AST.asdl* file. Then we add it in *parse.yy* to properly parse and generate the new AST in *semantics.h*.Finally we extend *pickle.cpp* so that the new operator can print itself. | ||
|
|
||
|
|
||
|
|
||
| :fontawesome-solid-code:*grammar/AST.asdl* | ||
| ``` | ||
| operator = Add | Sub | Mul | Div | Pow | Caret | ||
| ``` | ||
|
|
||
| :fontawesome-solid-code:*src/lfortran/parser/parser.yy* | ||
| ``` | ||
| %left "^" | ||
|
|
||
| expr | ||
| : id { $$=$1; } | ||
| | expr "^" expr { $$ = CARET($1, $3, @$); } | ||
| ``` | ||
|
|
||
| :fontawesome-solid-code:*src/lfortran/parser/semantics.h* | ||
| ``` | ||
| #define CARET(x,y,l) make_BinOp_t(p.m_a, l, EXPR(x), operatorType::Caret, EXPR(y)) | ||
| ``` | ||
|
|
||
| :fontawesome-solid-code:*src/lfortran/pickle.cpp* | ||
|
|
||
| ``` | ||
| std::string op2str(const operatorType type) | ||
| { | ||
| switch (type) { | ||
| case (operatorType::Caret) : return "^"; | ||
| } // now the caret operator can print itself | ||
|
|
||
| } | ||
| ``` | ||
| The section is tested with `lfortran --show-ast examples/expr2.f90` | ||
|
|
||
| ### Implement the Semantics of the New Token | ||
|
|
||
| We first extend the ASR in *ASR.asdl* and add ^ as a BinOp operator option. | ||
|
|
||
| :fontawesome-solid-code:*src/libasr/ASR.asdl* | ||
| ``` | ||
| binop = Add | Sub | Mul | Div | Pow | Caret | ||
| ``` | ||
| :fontawesome-solid-code:*src/lfortran/semantics/ast_common_visitor.h* | ||
| ``` | ||
| namespace LFortran { | ||
| class CommonVisitorMethods { | ||
| public: | ||
| inline static void visit_BinOp(Allocator &al, const AST::BinOp_t &x, | ||
| ASR::expr_t *&left, ASR::expr_t *&right, | ||
| ASR::asr_t *&asr) { | ||
| ASR::binopType op; | ||
| switch (x.m_op) { | ||
|
|
||
| case (AST::Caret): | ||
| op = ASR::Caret; | ||
| break; | ||
| } | ||
| if (LFortran::ASRUtils::expr_value(left) != nullptr && | ||
| LFortran::ASRUtils::expr_value(right) != nullptr) { | ||
| if (ASR::is_a<LFortran::ASR::Integer_t>(*dest_type)) { | ||
| int64_t left_value = ASR::down_cast<ASR::ConstantInteger_t>( | ||
| LFortran::ASRUtils::expr_value(left)) | ||
| ->m_n; | ||
| int64_t right_value = ASR::down_cast<ASR::ConstantInteger_t>( | ||
| LFortran::ASRUtils::expr_value(right)) | ||
| ->m_n; | ||
| int64_t result; | ||
| switch (op) { | ||
| case (ASR::Caret): | ||
| result = (left_value + right_value)/2; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| Then we transform it from AST to ASR by extending *src/lfortran/semantics/ast_common_visitor.h*. | ||
|
|
||
| We also add it into compile time evaluation triggered by expressions such as `e = (2+3)^5` which is evaluated at compile time. An expression such as `e = x^5` is evaluated at run time only. | ||
|
|
||
| The section is tested with `lfortran --show-asr examples/expr2.f90` | ||
| ### Implement the New Token in LLVM | ||
|
|
||
| To implement in LLVM, we extend the BinOp | ||
| translation by handling the new operator. We first add the two numbers then | ||
| divide by two. | ||
| :fontawesome-solid-code:*src/lfortran/codegen/asr_to_llvm.cpp* | ||
| ``` | ||
| void visit_BinOp(const ASR::BinOp_t &x) { | ||
| if (x.m_value) { | ||
| this->visit_expr_wrapper(x.m_value, true); | ||
| return; | ||
| } | ||
| this->visit_expr_wrapper(x.m_left, true); | ||
| llvm::Value *left_val = tmp; | ||
| this->visit_expr_wrapper(x.m_right, true); | ||
| llvm::Value *right_val = tmp; | ||
| if (x.m_type->type == ASR::ttypeType::Integer || | ||
| x.m_type->type == ASR::ttypeType::IntegerPointer) { | ||
| switch (x.m_op) { | ||
|
|
||
| case ASR::binopType::Caret: { | ||
| tmp = builder->CreateAdd(left_val, right_val); | ||
| llvm::Value *two = llvm::ConstantInt::get(context, | ||
| llvm::APInt(32, 2, true)); | ||
| tmp = builder->CreateUDiv(tmp, two); | ||
| break; | ||
| }; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| ``` | ||
|
|
||
| The section is tested with `lfortran --show-llvm examples/expr2.f90` | ||
|
|
||
| Now when LLVM works, we can test the final executable by: | ||
| ``` | ||
| lfortran examples/expr2.f90 | ||
| ./a.out | ||
| ``` | ||
| And it should print 6. | ||
|
|
||
| It also works interactively: | ||
| ``` | ||
| $ lfortran | ||
| Interactive Fortran. Experimental prototype, not ready for end users. | ||
| * Use Ctrl-D to exit | ||
| * Use Enter to submit | ||
| * Use Alt-Enter or Ctrl-N to make a new line | ||
| - Editing (Keys: Left, Right, Home, End, Backspace, Delete) | ||
| - History (Keys: Up, Down) | ||
| >>> 4^8 1,4 ] | ||
| 6 | ||
| >>> integer :: x 1,13 ] | ||
| >>> x = 4 1,6 ] | ||
| >>> x^8 1,4 ] | ||
| 6 | ||
| ``` | ||
| ## Reach Out | ||
|
|
||
| If you have any questions or need help, please ask as at our | ||
| [mailinglist](https://groups.io/g/lfortran) or a | ||
| [chat](https://lfortran.zulipchat.com/). | ||
| If you have any questions or need help, please ask as on our [mailinglist](https://groups.io/g/lfortran) or headover to [Zulip Chat](https://lfortran.zulipchat.com/). | ||
|
|
||
| Please note that all participants of this project are expected to follow our | ||
| Code of Conduct. By participating in this project you agree to abide by its | ||
| terms. See [CODE_OF_CONDUCT.md](CODE_OF_CONDUCT.md). | ||
| Please note that all participants of this project are expected to follow our Code of Conduct. By participating in this project you agree to abide by its terms. See [CODE_OF_CONDUCT.md](CODE_OF_CONDUCT.md). | ||
|
|
||
| By submitting an MR you agree to license your contribution under | ||
| the LFortran's BSD [license](LICENSE) unless explicitly noted otherwise. | ||
| By submitting a PR you agree to license your contribution under LPython's BSD [license](LICENSE) unless explicitly noted otherwise. | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if we need this line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand your point @Shaikh-Ubaid . I too was hesitant adding this paragraph seeing the sheer amount of students contributing to the project.
Please tell me the changes you recommend. Shall we remove it completely, or make adjustments? Reviewing a PR does take time, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@certik could you share/clarify what should be done here?