Fix GCC Wreorder Warnings by DhairyaBahl · Pull Request #1750 · synfig/synfig

DhairyaBahl · 2020-10-08T07:36:39Z

Fix GCC Wreorder warning in the line 209. Do i have to change it elsewhere. Kindly guide

Thanks :)

synfig-studio/src/gui/workarea.h

DhairyaBahl · 2020-10-08T10:02:14Z

@Keyikedalube Yes i am able to understand what is happening here. But i don't know about Wreorder or what is it .

Keyikedalube · 2020-10-08T10:30:09Z

Wreorder message is triggered when object member initialization is not in order with the class declaration. So in C++ you have class declaration in *.h file and class definition in *.cpp file. What you are looking to fix for is the order of the class members to remain the same

// declaration.h
class A {
    int a;
    float b;
}

// definition.cpp
A::A():
    a(0),
    b(0.0) // <--- notice the initialization order
           // a was declared first so it's initialized first
           // and then b is initialized
{
}

What we have in synfig is something like this:

// definition.cpp
A::A():
    b(0.0),
    a(0) // <--- notice the initialization order
           // b went first instead of a
           // that's why Wreorder warning is triggered
{
}

a should be at top and then b below.

Read this section Using Initialization Lists to Initialize Fields to better understand the topic

ice0 · 2020-10-08T11:13:48Z

It warns you because of this:

// declaration.h
class A {
    int a;
    int b;
}

// definition.cpp
A::A():
    b(5),
    a(b) // <--- You think were will be 5, because you initialized it with b, 
         //      but b will be initialized AFTER a (as you declared)
{
}

DhairyaBahl · 2020-10-08T12:22:45Z

@ice0 @Keyikedalube Thanks for putting this much effort for me. I will study these and then review and improve my changes .

Thanks :)

DhairyaBahl · 2020-10-08T17:19:11Z

@rodolforg Okay, If i am not wrong, I dont have to made changes in .h file rather than i have to make them in .cpp file !!
correct me if i am wrong.

Thanks :)

DhairyaBahl · 2020-10-09T08:18:09Z

@rodolforg @ice0 @Keyikedalube

highlight_active_bone(false) was getting declared after onion_skin and I shifted it to above show_grid(false) that must be giving the compile time error of Wreorder warnings

Kindly look after my PR.

Thanks :)

Keyikedalube · 2020-10-09T14:58:11Z

You fixed the issue with this commit d099194 :)

But your PR is dirty with other commits and an unneeded merge. You should only rebase when you wish to update your working branch, not merge. Also, rebasing is never really needed unless someone's PR was merged first && he modified the same file that you are working on.

Upto project maintainers to decide what's next.
Good job fixing the issue 👍

DhairyaBahl · 2020-10-09T15:16:04Z

@Keyikedalube Okay Thanks for the advice. Actually @AYESDIE Yesterday told me to rebase properly so that PRs will be approved quickly. But I did it with every branch I have, I was practicing git commands . Sorry :)

DhairyaBahl · 2020-10-09T15:18:48Z

@Keyikedalube Thanks a lot for giving me that much of hints. Without them I wouldn't be able to solve the issue.

Thanks :)

AYESDIE · 2020-10-09T16:12:21Z

@Keyikedalube There were some issues that were there with the Appveyor CI (some key related issue) which were fixed later in master so the PRs needed a rebase for the checks to be passed.

And I don't think there's need for 6 commits for a small change, so this one does need cleaning in my opinion. Rebasing and squasing commits will help (^_^)

DhairyaBahl · 2020-10-09T16:15:54Z

@Keyikedalube Actually I was confused about it too.
When I undo my doing in any file it also counts it as another commit. IDK why ?

Anyways Thank You :)

Keyikedalube · 2020-10-10T00:37:48Z

@DhairyaBahl Happens to everyone :)
Now you know how git works. Every changes you make && commit are recorded.

@AYESDIE I remember rebasing and pushing updates would end up looking like this "user force-pushed ..." but his update says "merge" so he could have done it some other way. IDK how.

And yes he needs to squash those commits together.

But @DhairyaBahl there's more warning messages about -Wreorder
Go trach those and resolve 'em :)
Then squash later not now OK

ice0 · 2020-10-10T05:48:35Z

It would also be nice if you give good name for your PR. For example "Fix gcc wreorder warning" or something like that.

DhairyaBahl · 2020-10-10T06:40:49Z

@Keyikedalube Actually I reverse merged the commits from synfig/master to my own dhairyabahl/master that's why it shows merged.

I will now fix the other wreorder warnings.

Thanks for support :)

DhairyaBahl · 2020-10-10T08:54:44Z

@Keyikedalube I tried to find out other wreorder issues under the workarea class but everything seems to be in proper position.
are the other wreorder warnings present under the same class or under a diff. class ?

Thanks :)

Keyikedalube · 2020-10-10T11:38:53Z

@DhairyaBahl I guess you are building synfig without cleaning. That's why you are not seeing other -Wreorder messages.

You must clean your previous build files and build from start again to see all the warning messages pop up on your console or IDE build log.

The thing about the buildsystem is it is smart to not compile unmodified files all over again every time you build. When you modified the workarea file, make (or whatever build generator you used) compiled only that file. All the other cpp source files weren't needed to be compiled again. Saving you build time...

Keyikedalube · 2020-10-10T11:41:19Z

are the other wreorder warnings present under the same class or under a diff. class ?

different class

DhairyaBahl · 2020-10-10T12:02:13Z

@Keyikedalube Okay thanks for guidance. I will find those errors and rectify them ASAP.

Thanks :)

DhairyaBahl · 2020-10-14T13:45:36Z

@ice0 @Keyikedalube I have made changes for all the wreorder warnings that were shown in the console. Kindly look into my PR.

Thanks :)

ice0 · 2020-10-14T14:55:46Z

@DhairyaBahl please fix indents. Synfig uses tabs instead of spaces. You can see difference on this tab: https://github.com/synfig/synfig/pull/1750/files

DhairyaBahl · 2020-10-14T14:58:23Z

@ice0 Ok. I am on it .

Thanks 👍

DhairyaBahl · 2020-10-14T15:27:15Z

@ice0 @Keyikedalube I rectified the unnecessary use of spaces and properly used tabs there. Can anyone tell me now how to convert 4 commits into 1 ??
Is squashing is the term required here ??

Thanks :)

ghost · 2020-10-14T16:10:02Z

DeepCode failed to analyze this pull request

Something went wrong despite trying multiple times, sorry about that.
Please comment this pull request with "Retry DeepCode" to manually retry, or contact us so that a human can look into the issue.

Fixing Wreorder warnings Update vectorizersettings.cpp Update state_bone.cpp

DhairyaBahl · 2020-10-14T16:14:22Z

Retry DeepCode

Keyikedalube · 2020-10-15T00:12:12Z

Is squashing is the term required here ??

Yes

Keyikedalube · 2020-10-15T00:15:32Z

@DhairyaBahl But wait for ice0 to tell you when. Don't squash now.
He might need to review it first ok

DhairyaBahl · 2020-10-15T04:28:19Z

@Keyikedalube sorry, I already did that 😅

Keyikedalube · 2020-10-15T04:30:18Z

@DhairyaBahl Doesn't matter :) You can still squash even after making review changes.

DhairyaBahl · 2020-10-15T04:31:36Z

@Keyikedalube got it. Thanks :)

DhairyaBahl · 2020-10-15T05:50:53Z

@ice0 @Keyikedalube I just completed my 4 valid hacktoberfest PRs. what should I do next?? Take another issue ??

Thanks for help ;)

ice0 · 2020-10-15T07:44:51Z

@DhairyaBahl of course! Any help is appreciated!
Even if you just check if an some IDE can open and build Synfig, it also helps a lot, because it all takes time and attention.

ice0 · 2020-10-15T07:45:52Z

Merged. Thank you!

DhairyaBahl · 2020-10-15T08:40:09Z

@Keyikedalube @ice0 Please guide me what steps should I take if I want to get in GSOC with this organisation ??
Should I keep contributing to these issues ? OR should i work on some projects for the time being ??

@AYESDIE what should I do ?? I mean I saw ur name in the Ex-Gsocer ? Bro Kindly guide me !!

Thanks :)

ice0 · 2020-10-15T15:57:42Z

Just continue to learn the code and make contributions.
Also you can check last year GSoC ideas list here (https://synfig-docs-dev.readthedocs.io/en/latest/gsoc/2020/ideas.html)

We haven't decided on ideas for next year, so maybe you can come up with your own idea.

DhairyaBahl · 2020-10-15T16:00:05Z

@ice0 I will keep contributing in this and will also read that linked documentation just keep guiding me like you are doing sir. Thanks 👍

Will Do My Best !!

Keyikedalube · 2020-10-16T00:02:11Z

Please guide me what steps should I take if I want to get in GSOC with this organisation ??

@DhairyaBahl I opened issues that are too easy to solve so new contributors like you can get familiar with synfig codebase.

GSOC will be more problem solving rather than minor fixes here and there like mine. I would suggest you to take up issues that need creative problem solving. That way I guarantee you'll be ahead of the beginners curve :)

Should I keep contributing to these issues ? OR should i work on some projects for the time being ??

Up to you :)

Me personally I do my own gtkmm project first. I do my learning there, apply documentation/tutorials knowledge I studied, and then come back here to solve issues that I'm familiar with ;)

DhairyaBahl · 2020-10-16T04:53:27Z

@Keyikedalube Thanks for the advice of solving more creative problems but don't you think its a little early to start my own project.
Still I will find out some good ideas for project to work on !!

Thanks a lot for the whole idea ;)

Keyikedalube reviewed Oct 8, 2020

View reviewed changes

synfig-studio/src/gui/workarea.h Outdated Show resolved Hide resolved

DhairyaBahl changed the title ~~Update workarea.h~~ Fix GCC Wreorder Warnings Oct 10, 2020

DhairyaBahl force-pushed the issue-1744 branch from be1155f to ecd77f5 Compare October 14, 2020 13:43

DhairyaBahl force-pushed the issue-1744 branch 2 times, most recently from 03fab64 to d29cfdb Compare October 14, 2020 15:25

DhairyaBahl force-pushed the issue-1744 branch from d29cfdb to 0048b88 Compare October 14, 2020 16:10

Fixing GCC Wreorder Warnings

a5f1b2d

Fixing Wreorder warnings Update vectorizersettings.cpp Update state_bone.cpp

DhairyaBahl force-pushed the issue-1744 branch from 0048b88 to a5f1b2d Compare October 14, 2020 16:13

ice0 approved these changes Oct 15, 2020

View reviewed changes

ice0 merged commit 0576fa8 into synfig:master Oct 15, 2020

ice0 added Easy Hacktoberfest labels Oct 15, 2020

ice0 mentioned this pull request Oct 15, 2020

Fix GCC Wreorder warnings #1744

Closed

Uh oh!

Conversation

DhairyaBahl commented Oct 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

DhairyaBahl commented Oct 8, 2020

Uh oh!

Keyikedalube commented Oct 8, 2020

Uh oh!

ice0 commented Oct 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DhairyaBahl commented Oct 8, 2020

Uh oh!

DhairyaBahl commented Oct 8, 2020

Uh oh!

DhairyaBahl commented Oct 9, 2020

Uh oh!

Keyikedalube commented Oct 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DhairyaBahl commented Oct 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DhairyaBahl commented Oct 9, 2020

Uh oh!

AYESDIE commented Oct 9, 2020

Uh oh!

DhairyaBahl commented Oct 9, 2020

Uh oh!

Keyikedalube commented Oct 10, 2020

Uh oh!

ice0 commented Oct 10, 2020

Uh oh!

DhairyaBahl commented Oct 10, 2020

Uh oh!

DhairyaBahl commented Oct 10, 2020

Uh oh!

Keyikedalube commented Oct 10, 2020

Uh oh!

Keyikedalube commented Oct 10, 2020

Uh oh!

DhairyaBahl commented Oct 10, 2020

Uh oh!

DhairyaBahl commented Oct 14, 2020

Uh oh!

ice0 commented Oct 14, 2020

Uh oh!

DhairyaBahl commented Oct 14, 2020

Uh oh!

DhairyaBahl commented Oct 14, 2020

Uh oh!

ghost commented Oct 14, 2020 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DeepCode failed to analyze this pull request

Uh oh!

DhairyaBahl commented Oct 14, 2020

Uh oh!

Keyikedalube commented Oct 15, 2020

Uh oh!

Keyikedalube commented Oct 15, 2020

Uh oh!

DhairyaBahl commented Oct 15, 2020

Uh oh!

Keyikedalube commented Oct 15, 2020

Uh oh!

DhairyaBahl commented Oct 15, 2020

Uh oh!

DhairyaBahl commented Oct 15, 2020

Uh oh!

ice0 commented Oct 15, 2020

Uh oh!

ice0 commented Oct 15, 2020

Uh oh!

DhairyaBahl commented Oct 15, 2020

Uh oh!

ice0 commented Oct 15, 2020

Uh oh!

DhairyaBahl commented Oct 15, 2020

DhairyaBahl commented Oct 8, 2020 •

edited

Loading

ice0 commented Oct 8, 2020 •

edited

Loading

Keyikedalube commented Oct 9, 2020 •

edited

Loading

DhairyaBahl commented Oct 9, 2020 •

edited

Loading

ghost commented Oct 14, 2020 •

edited by ghost

Loading