GSoC 2021 Final Report

It has rightly been said – “All good things come to an end”. Google Summer of Code too was one of the good experiences I’ve had, in the sense that I didn’t know anything about the Open Source world. It provided the exact platform that I needed to kickstart my open source contributions.

About my project:

Haiku has its own coding standards which describe how the code should be formatted. Haiku-format is a tool that reformats code according to Haiku coding style but it is not giving desired results. So, we need to format the code such that when this code is run on Haiku the coding style of code gets updated according to haiku guidelines, but it has to be compiled on the developer machine and then run manually.

The tool ‘ haiku-format ‘ is work in progress which can be used to check the format according to guidelines which will be integrated with concourse to automate the process of checking coding style.

What we achieved?

I started with the input preferences directory and started solving the issues according to haiku guidelines. Following erros were solved:

• Preserve the indentation of comments
• Removal of braces for multiline control statements
• Line break after return type in function definitions
• Unusual indentation of BLayoutBuilder block
• Extra space before ++ operator in for loop
• Tabs are converted into spaces
• Remove extra spacing for std::nothrow.
• ‘ : ‘ should come on its own line and initializers in following lines
• Empty functions to inline

Along with the errors in input directory, same errors persist in other directories too. So they also got solved. With these major bugs got solved and the clang-format further doesn’t lead to break the code or build anymore.

But where’s the code?

What I did?	Merge Request or Code Link
Removes extra space between new and (std::nothrow)	Removes extra space between new and (std::nothrow)
‘ : ‘ should come on its own line and initializers in following lines	https://github.com/viveris/llvm-project/commit/df2208a9e66f87a1bd6f0bd68ed973ace901427b
Empty functions to inline	https://github.com/viveris/llvm-project/commit/df2208a9e66f87a1bd6f0bd68ed973ace901427b
Preserve indentation of comments	https://github.com/viveris/llvm-project/commit/f85ea5013b42a3dd2e983e1912d52c83e917e01b https://github.com/viveris/llvm-project/commit/51b07e72dea5ce2db8be569ab2f47c86b7ea0bf7
Removal of braces for multiline control statements	https://github.com/viveris/llvm-project/commit/ea033bd4925eb9551d2d40625b490f82c98f5196
Line break after return type in function definitions	https://github.com/viveris/llvm-project/commit/220a2bbe6bdf23c291cd3f5205cfdadf05622a9b
Unusual whitespace for multiline for loop	https://github.com/viveris/llvm-project/commit/6fd97f0d9705d9597d09abdad3742d52de124ae2
Unusual indentation of BLayoutBuilder block	https://github.com/saloniig/llvm-project/commit/c63b4b2d001406ca178bbdd02a5b3da8866f459d
Tabs are converted into spaces	https://github.com/saloniig/llvm-project/commit/efb5ae15b5a19b23f54b717925a07592085f2911
Input preferences: test reformatting with haiku-format	https://review.haiku-os.org/c/haiku/+/3826
Updates code according to haiku guidelines with clang-format	https://review.haiku-os.org/c/haiku/+/4098
Updates code according to haiku guidelines with haiku-format-10.x	https://review.haiku-os.org/c/haiku/+/4325

What’s next?

Time and again, I’ve written about my love for open source software and the freedom it brings. Since, I’m a proponent of it, I’ve decided that I’ll continue contributing to open source in my free/spare time. I’ll keep contributing to these projects because the codebase is really interesting and that they’ve provided me the stepping stones into the open source world.

What we under-estimated?

I wrote in my proposal was to solve input directory errors which was supposed to be done before the first evaluation but I did the mistake of under-estimating how much time it takes to produce efficient result. This work takes a lot of time because it needs a complete understanding of how the llvm code works. But Pulkomandy and Preetpal explained me how the code works. As there was number of files, it was difficult to figure out which file should contain the solution of the problem

Next problem which I face was solving the unusual indentation of BLayoutBuilder block. As it was almost impossible for the clang-format to do this. Another one was tabs were converted into spaces.

The basic idea was :

indented 1 tab: qualifiers (volatile, virtual, static)  
indented 3 tabs: type of fields and return type of functions  
indented 2 tabs: function or field name

These take alot of time and patience. Even sometimes I messed up with github branches but Pulkomandy always patiently explained me and corrected my mistakes.

I have learnt so many new things in these months about haiku and git.

Work to do:

I have setup a base for us to work upon this but this needs testing on other directories that are pending. As much I have tested the major errors have been solved and there might be some minor errors which will not break code.

Final Note

All in all, this journey of GSoC has been one of the most beautiful ones when it comes to my career. It was a really wholesome experience with the whole community. I’d like to extend my gratitude to each and every person involved with me over this journey.

A special thanks to my mentors Preetpal Kaur and Adrien Destugues. They mentored me really well and was always ready to help. Thank you for being patient with me and clarifying my doubts.

Also, a big thanks to owenca for helping and clarifying my doubts. Thank you Google supporting open source software and for providing students like me with this opportunity.

Weekly Blogs:

https://hackace2.wordpress.com/category/gsoc/

Week-13

This week started with some new things. My mentor had some discussion with owenca who was member of Haiku and it was decided that older version llvm 10 should be used as it would give a clean state to add additional customizations.

After this my mentor created a new branch haiku-format-10.x to test this version with the newer version. As I was working on the bug in which tabs were converted into spaces as :

private:
	BString fTitle;
	BBitmap* fPrimaryIcon;
	bool fSelected;

but the expected output was :

private:

	BString		fTitle;
	BBitmap*	fPrimaryIcon;
	bool		fSelected;

As I have tested it on input directory which was giving correct results. But I wanted to be sure so I tested it on other directories. The result was as expected there were some new cases that were not according to expected one. Then I started working on these and started adding test case for those problems.

Along with it I run 10.x version on the input preferences code and to compare with the new version. I send a commit with 10.x version.

Then, I updated the code for the bug in which tabs were converted into spaces and send a commit. As it was the last week of GSoC coding period so side by side I was writing my final evaluation report.

This week ended with this 🙂

Week-12

This week I started with the bug we left in which an extra indentation was added befor ++ operator :

for (itr = fMouseSettingsObject.begin(); itr != fMouseSettingsObject.end();
	 ++itr)

Last time we just left it but now this week I started with this and I write some code for this and send a commit. But this contains previous code of other bugs like unusual indentation of BLayoutbuilder block. My mentor told me this but I got bit confused and said this branch contains only this code. Then he send me picture of gitk which was used to view the commit tree.

Then I looked into it and get to know I am making mistake. Then I removed previous and unsused commits and firstly made it even with viveris:viv_haiku_format. After that I done my changed and again made a commit and send to my mentor to review.

My mentor merge it and I started looking into previous BLayoutBuilder block indentation as it was going infinite loop at some point then I look into it and find the variable indentLevel was declared as unsigned due to which while calculating line number this variable goes in negative value and goes infinite. On changing it to int it worked fine.

Then I cleaned the code and send a commit for that.

After that I checked my code for previous bug that was the tabs for modifiers, types and name for private, public block. I noticed that still there are problems in other directories and I need to add some checks to the code. Then I started working on it.

This week ended with this only. 🙂

Week-11

We were working on the bug in which tabs were converted into spaces as :

private:
	BString fTitle;
	BBitmap* fPrimaryIcon;
	bool fSelected;

but the expected output was :

private:

	BString		fTitle;
	BBitmap*	fPrimaryIcon;
	bool		fSelected;

Then, I did some changes and send a commit and I have some doubts :

• Here the column limit is exceeding 80 so we need to break it then how much the indentation should be of next line.
• Here the words are joining then how much these should be indented.

So, I asked my mentor about these and he told me

• The correct style is:

void			DrawItem(BView* owner, BRect frame,
					bool complete = false);

That is, the extra parameter is indented one level more than the method name. See “FunctionLotsOfArguments” in the coding guidelines document example: https://www.haiku-os.org/development/coding-guidelines/

• In next case, you need one extra tab here, I think you can add one tab for all the declarations in this private section. The coding style does not specify exactly the number of tabs to use, and generally the idea is that in each block, we put enough tabs so that the typename will fit. But that may be a bit more complicated to handle for clang-format? Sometimes when one single type is very long, we allow ourselves to separate it with a space instead of a tab, and have it not aligned with other declarations. In any case, clang-format should never allow 0 spaces here, because this results in code that does not compile.

Then, I corrected these things and again send a commit . After that my mentor reveiwed and asked for some changes:

• There is an extra space before “type“
• Indentation is too high here. It should be one level more than DrawItem, now it’s 3 levels.
• Here

static" should be alone in the first column                                                                                                                                                                 "InputIcons*", "BString" and "input_type" on the second column sIcons,                                                                                                                                                                                  fTitle and fInputType on the third

After that I did some changes which took 2 days and I send a commit with changes. My mentor said everything was fine except one change in which functions were not properly aligned.

Then, I started working on this. I did some changes and again send a commit with latest changes. Now, these errors got solved now I need to send my code. But I need to clean it and remove some redundant code. I started working on it.

This week ends with this 🙂

Week-10

Now we have started with new bug in which tabs were converted into spaces as :

private:
	BString fTitle;
	BBitmap* fPrimaryIcon;
	bool fSelected;

but the expected output was

private:

	BString		fTitle;
	BBitmap*	fPrimaryIcon;
	bool		fSelected;

The mentor told me the basic idea is:

indented 1 tab: qualifiers (volatile, virtual, static)
indented 3 tabs: type of fields and return type of functions
indented 8 tabs: function or field name

I somewhat understood it and started writing code for this. I does some changes but I was not sure whether I am working in right direction or not. So, I send a commit to confirm whether the output given by this is somewhat expected or not. I told my mentor that code needs to be clean there is redundant code so I just want to verify the output. He asked me it will be easier for him to review the result if I update the gerrit change with the latest version of the reformatted code. Otherwise he need to build clang-format and run it which will consume a time.

So, I just pushed my changes on gerrit. Then he said me it’s starting to look good! It looks like there are problems with constructors/destructors, statics/virtuals. He put some comments also which was helpful for me to what results are required.

• Modifiers (static, virtual) go on a first column
• Types go on a second one
• Names go on a third

Then I started working on this. It was little bit difficult as there was no inbuilt code to identify the modifiers and names. So, I need to set up and define all these according to the requirements.

Week-9

I passed the first evaluation of GSoC 2021.  The evaluation means My mentor and I have to complete a survey to pass the evaluation. The evaluation period started was active during July 12 — July 16, 2021.

The survey asks some questions about myself, Work done and feedback for the Organization and GSoC Program like these following,

First Evaluation Questions

About you

• Did you contribute to Haiku before February 2021?
• Did you participate in open source before GSoC 2021?
• Were you a GSoC student before GSoC 2021?

Communication

• When did you first communicate with Haiku this year?
• How often do you interact with your mentor(s)?
• How do you communicate with your mentor(s)?
• Rate the quality of interactions with your mentor(s):
• How responsive are your mentor(s)?

Experience

• What is your favorite part of participating in GSoC?
• What is the most challenging part of participating in GSoC?

Anything else?

• Feedback for Haiku and mentors
• Anything else you’d like to tell Google or suggestions on how we could improve the program?

After the evaluation period, I received a mail about the Evaluation results. I have passed successfully 🙂 .

Now continuing to the work done this week, the last commit send was a bit large with a lot of changes. So, it took some time for my mentor to review and then he came up with a new configuration. In the clang-format documentation he saw config option MacroBlockBegin and MacroBlockEnd, which can be used to affect indentation levels. He asked me to try if setting MacroBlockBegin to “BLayoutBuilder::Group” and MacroBlockEnd to “.End()” would work?

As it was new for me, I tried different expressions with

MacroBlockEnd = .End 

MacroBlockBegin = .AddGroup([^)]*) 

but nothing of them worked. Then he said me to look whether the code used in this can be helpful for us. I tried to understand it and came with some point:

• It picks only one token at time like it picks . only and doesn’t pick . and AddGroup simultaneously.
• After setting MacroBlockEnd as End() it will only set the indentation of End not of the next line which should be 1 level unindented as compare to End.
• It also merges the lines that are not set as MacroBlockBegin and MacroBlockEnd as they doesnt end with semicolon.

So, then my mentor confirms that it can’t be used and we will go with previous commit. But last time I made a mistake, I forgot to run clang-format on whole directory, I just run it on InputKeyboard.cpp file and when I tested it on whole directory a unusual change occurs. In SettingsView.cpp file the code breaks the comment to next line

// Horizontal :                                                                                                                                                                        // A|B

When I looked into it I found that these changes are due to getCommentSplit function On commenting the if condition it works fine. So, now I started looking why this is happening.

I was not able to understand the cause of this. So, I started working on next bug in which the tabs were converted into spaces. The clang-format was converting block into :

private:
	BString fTitle;
	BBitmap* fPrimaryIcon;
	bool fSelected;

but the expected output was :

private:

	BString		fTitle;
	BBitmap*	fPrimaryIcon;
	bool		fSelected;

I started writing code for this and simultaneously was looking for indentation of BLayoutBuilder. This week ended with this.

Week-8

This week started with solving the unusual indentation for .SetInsets line in which I was getting output as

.SetInsets(
    B_USE_WINDOW_SPACING, B_USE_WINDOW_SPACING, B_USE_WINDOW_SPACING, 0)

But expected output was :

.SetInsets(B_USE_WINDOW_SPACING, B_USE_WINDOW_SPACING,
   B_USE_WINDOW_SPACING, 0)

So, there needs to be performed two tasks :

• Break the line when column limit is greater than 80.
• Merge the lines.

I started working on this. As it was little bit difficult, so firstly I set up debugger in visual studio code and get my hands on it. Firstly, it was difficult to work and understand it’s benefits but this proves to be really helpful.

After spending 2-3 days on it I was somewhat able to get the correct output but not the exact one. It was only possible with the debugger and after that I send a commit to my mentor for review.

.SetInsets(
    B_USE_WINDOW_SPACING, B_USE_WINDOW_SPACING,
    B_USE_WINDOW_SPACING, 0)

Now, I was getting output as shown above. So, now I am waiting for my mentor to confirm whether this will be acceptable or not.

This week proves to be a great learning one 🙂

Week-7

This week begin with strange problem that was when I pass no or more than 1 argument, it formatted correctly as here but on passing 1 argument it is not working. So, I looked into code and tried different configuration for this and came to know that  Style.BraceWrapping.AfterFunction configuration was creating problem. When I set it to true it was giving result as:

void MouseView::MouseUp(BPoint)
{
	fButtons = 0;
	Invalidate(_ButtonsRect());
	fOldButtons = fButtons;
}

and on setting it to false the result was:

void
MouseView::MouseUp(BPoint) {
	fButtons = 0;
	Invalidate(_ButtonsRect());
	fOldButtons = fButtons;
}

On false also the position of braces changed which was against haiku guidelines. So, this was problem from llvm code. So, I started looking into the code regarding this and started searching for the appropriate position and file to write code regarding this.

Then, after spending 2-3 days, I was able to do this and after that I send a commit to to mentor.

After that my mentor asked me for some changes :

1. calculateFormattingInformation could be inside isFunctionDeclarationName function as it was related to it.
2. This code was specific to Haiku as I have added a constraint that this code will work for Haiku only. So he advised that as it is a bug in llvm so we want this to be specific to haiku

I did that changes and again send a commit for that. Waiting for my mentor to review, I started on working previous bug of break and merge.

With solving a error this week ended. Meet in next week with solution of next bug. 🙂

Week-6

This week started with solving the unusual indentation for .SetInsets line in which I was getting output as

.SetInsets(
    B_USE_WINDOW_SPACING, B_USE_WINDOW_SPACING,
               B_USE_WINDOW_SPACING, 0)

But expected output was :

.SetInsets(B_USE_WINDOW_SPACING, B_USE_WINDOW_SPACING,
   B_USE_WINDOW_SPACING, 0)

So, there needs to be performed two tasks :

• Break the line when column limit is greater than 80.
• Merge the lines.

Firstly, I tried writing code and send a commit for breaking the line when it is greater than 80 column limit. But my mentor said there is already some code for this and need to look into it and made changes accordingly. Then I started looking for it and tried calling different functions and different places and nothing seemed to work.

Then I found something which was just a mystery box for me. It was a patch set related to this error. Then I looked into it and found this code is already present. Now my new task was to figure out why this is not running. I thought the function needs to be called at a required place. I spend a day in figure out this but again nothing work.

Then I just used cout to confirm this function is already running then why it was not running. Then a new strange thing happen. I was just testing code on a file with different changes and suddenly I got expected output that was :

.SetInsets(B_USE_WINDOW_SPACING, B_USE_WINDOW_SPACING,
   B_USE_WINDOW_SPACING, 0)

Then there was one good thing and one bad news. Good news was the function was running correctly and the bad thing was or should I say strange that was if there is comment and above that any line that is not ending like:

test
// test
and after this Blayout block everything is working fine like this:

and with same thing another error also got solved.

So, now new task came to figure out this thing and correcting it. After looking into code I came to know that the break and merge code was working only for LineComments. So, I tried to change the conditions so that it will work on such cases also, but nothing worked. Then I mailed to llvm regarding this problem and simultaneously started working on next bug.

Then, I looked into it and there was a strange thing when I pass no or more than 1 argument, it formatted correctly as here but on passing 1 argument it is not working. So, I looked into code and tried different configuration for this and came to know that  Style.BraceWrapping.AfterFunction configuration was creating problem. When I set it to true it was giving result as:

void MouseView::MouseUp(BPoint)
{
	fButtons = 0;
	Invalidate(_ButtonsRect());
	fOldButtons = fButtons;
}

and on setting it to false the result was:

void
MouseView::MouseUp(BPoint) {
	fButtons = 0;
	Invalidate(_ButtonsRect());
	fOldButtons = fButtons;
}

On false also the position of braces changed which was against haiku guidelines. So, this was problem from llvm code and need special treatment. Then, I started writing code about this.

This week fully satisfies this quote 🙂

Every problem raises new unsolved problem

Meet you in next week with solution of these mysteries 🙂

Week-5

This week started with the previous bug only in which the clang-format was showing unusual indentation style for a block of code:

BLayoutBuilder::Group<>(this, B_VERTICAL)
		.AddGroup(B_HORIZONTAL)
		.SetInsets(
			B_USE_WINDOW_SPACING, B_USE_WINDOW_SPACING, B_USE_WINDOW_SPACING, 0)
		.Add(fSettingsView)
		.End()
		.Add(new BSeparatorView(B_HORIZONTAL))
		.AddGroup(B_HORIZONTAL)
		.Add(fDefaultsButton)
		.Add(fRevertButton)
		.AddGlue()
		.End();

But the expected output was:

BLayoutBuilder::Group<>(this, B_VERTICAL)
		.AddGroup(B_HORIZONTAL)
			.SetInsets(B_USE_WINDOW_SPACING, B_USE_WINDOW_SPACING,
				B_USE_WINDOW_SPACING, 0)
			.Add(fSettingsView)
			.End()
		.Add(new BSeparatorView(B_HORIZONTAL))
		.AddGroup(B_HORIZONTAL)
			.Add(fDefaultsButton)
			.Add(fRevertButton)
			.AddGlue()
			.End();

As previously I have send a commit which was giving expected output but it was correct results only for already correct code , but in cases where the initial indent is incorrect, it doesn’t fully fix things. So, it was little bit complex task it took 2-3 days to correct this and then I send a commit. But mentor was ill so he advised me to ask other people on the haiku forum to test the changes on different pieces of code and report the problems they find? So, along with this directory I send the commit for other directories also.

Then they asked me to correct one thing. The .SetInsets line was not getting aligned correctly. I just spend 2 days into this but got output like this:

.SetInsets(
    B_USE_WINDOW_SPACING, B_USE_WINDOW_SPACING,
               B_USE_WINDOW_SPACING, 0)

But the expected output was:

.SetInsets(B_USE_WINDOW_SPACING, B_USE_WINDOW_SPACING,
				B_USE_WINDOW_SPACING, 0)

This week ended with solving  indentation of BLayoutBuilder blocks (where we usually have an indentation level for each AddGroup).