On Tue, 25 Mar 2025 21:19:03 GMT, Doug Simon <dnsi...@openjdk.org> wrote:
> Thanks for all the comments so far. > > First thing is that my tool does nothing about re-ordering block of > conditional includes vs unconditional includes. I briefly looked into that > but it gets very complicated, very quickly. That kind of re-ordering will > have to continue to be done manually or someone is going to have to invest > significant time in enhancing/replacing the tool I wrote. Yup. The extra effort needed to get the tool fully solve this is the reason why we haven't built a tool for this. There are a few scripts around but none is good enough to be promoted as *the* tool to correctly fix our includes. It is still going to be a great tool for the devs. > > Also, the tool tries to not change the number of lines in the original file. > It should only add blank lines where necessary to separate user includes from > sys includes. This explains some of the extra blank lines. For example, if > the original was: > > ``` > 1: #include "a.h" > 2: > 3: #include "b.h" > 4: > 5: #include <c.h> > 6: > 7: #include <d.h> > ``` > > the output is: > > ``` > 1: #include "a.h" > 2: #include "b.h" > 3: > 4: #include <c.h> > 5: #include <d.h> > 6: > 7: > ``` > > Once again, I'd prefer to keep the tool simple and focused on the main task > of ordering includes. Cleaning up extraneous blank lines can be done manually > after running the tool. OK. We'll just have to make sure to clean these out after having the initial run of this tool. > > I'm currently working on converting `sort_includes.py` to > `SortIncludes.java`. Once done, I'll open a second PR and limit changes to > the C++ files I'm comfortable with changing and testing (namely in JVMCI > directories). I will include a jtreg test to ensure these changes do not > regress. > > Follow up issues can then be opened for working on the remaining C++ files. > The main point of this initial PR is to show that such a tool can be useful. Sounds good to me. ------------- PR Comment: https://git.openjdk.org/jdk/pull/24180#issuecomment-2753413905
