Bugzilla

(In reply to Daniel Holbert [:dholbert] from comment #151)
> Note: when reviewing the layout patch, I'm noticing that we're ending up with a lot of lines like this in .cpp files:
> 
> ```c++
> nsPageBreakFrame::~nsPageBreakFrame() = default;
> ```
> 
> (...where this patch has replaced `{}` with `= default;`)
> 
> These lines end up feeling kinda silly & extraneous now... 

> I think originally with `{}` the goal was to provide a spot where you could place a gdb breakpoint, but I suspect (?) that doesn't work now that they're `= default`. (though maybe it does? I haven't tried.)

I am not sure if you can set a line breakpoint, but unless the destructor is being inlined, still a regular function is generated, which you can set a breakpoint in by name.

> Anyway -- it might be nice to do a followup bug, with a grep for `::~` followed by `default` on the same line, to find candidates like the above where we can clean up a bit by moving the `default` up to the function declaration inside of the scope of the struct/class {...} section.  Not super high-priority, just a possible next-step here.

Yes, that is certainly a good follow-up cleanup. In several cases, the destructor declaration is entirely redundant and can be removed, i.e. when it is public and not explicitly declared virtual. There is no automated tool available which does this, which is why I didn't do this as part of this bug.

(In reply to Daniel Holbert [:dholbert] from comment #151)
> Note: when reviewing the layout patch, I'm noticing that we're ending up with a lot of lines like this in .cpp files:
> 
> ```c++
> nsPageBreakFrame::~nsPageBreakFrame() = default;
> ```
> 
> (...where this patch has replaced `{}` with `= default;`)
> 
> These lines end up feeling kinda silly & extraneous now... 

> I think originally with `{}` the goal was to provide a spot where you could place a gdb breakpoint, but I suspect (?) that doesn't work now that they're `= default`. (though maybe it does? I haven't tried.)

I am not sure if you can set a line breakpoint, but unless the destructor is being inlined, still a regular function is generated, which you can set a breakpoint in by name.

> Anyway -- it might be nice to do a followup bug, with a grep for `::~` followed by `default` on the same line, to find candidates like the above where we can clean up a bit by moving the `default` up to the function declaration inside of the scope of the struct/class {...} section.  Not super high-priority, just a possible next-step here.

Yes, that is certainly a good follow-up cleanup: note that this might not be possible in all cases, where some member's destructor's definition is not available in the header. In several cases, the destructor declaration is entirely redundant and can be removed, i.e. when it is public and not explicitly declared virtual. There is no automated tool available which does this, which is why I didn't do this as part of this bug.