On Thu, 2023-12-07 at 14:18 +0800, Yang Yujie wrote: > On Thu, Dec 07, 2023 at 11:02:58AM +0800, Xi Ruoyao wrote: > > > > I don't like this pair of {} for the for statement. It's not necessary > > and it changes the indent level, causing the diff hard to review. > > > > Otherwise LGTM. I'm not sure why I didn't notice the eh_return issue > > when I learnt shrink wrapping from RISC-V... > > > > Thanks for the review! This problem on LoongArch was first noticed in a > failed libphobos test case, and the fix is partially borrowed from i386, > which seemed to be the only architecture without this issue. > > So despite the extra braces (which I'd say I prefer to have because of the > new block of comment inserted on top of the if statement :P), I am going to > ask Lulu for pushing this.
I understand and I don't think adding {} is wrong. The problem is the indent change causes a large chunk of diff and it makes reviewing more difficult. Thus generally we should not mix real code change and format change in a commit. i. e. it would be better to separate it into two patches, the first adds {} and changes the indent, and the second changes the logic. But now I don't think it's needed to make a V4, just pushing this should be fine. -- Xi Ruoyao <xry...@xry111.site> School of Aerospace Science and Technology, Xidian University