Febbe added inline comments.
================
Comment at: clang/unittests/Format/SortIncludesTest.cpp:548
+ EXPECT_EQ("#include \"a.h\"\n"
+ "#include \"llvm/a.h\"\n"
+ "#include \"b.h\"\n"
----------------
HazardyKnusperkeks wrote:
> While I may accept there are more than one //main// header, this should not
> happen in my opinion.
Yes, that is a conflict of interests. There is no perfect way of implementing
this, without the help of clang-tidy / the clang language server:
To arguably define a file as a main include, all declarations must be analyzed,
whether they are predeclared in the include or not.
But we don't have the help of the clang AST.
My expectation is, that when I randomly shuffle the includes, that they are
always sorted to the same result.
Currently, this is not the case, and depending on the rules it can furthermore
happen, that a main include "a.h" is exchanged with the "llvm/a.h".
This should also not happen, but it does, and it is not covered by the tests.
I consider the false negative of a correct main include worse than a false
positive.
Therefore, I changed it.
In addition, my approach has the advantage that all includes are sorted in the
same way, regardless of the order.
But if you want, we could introduce a new Option like: `enum
FindMainIncludes{FMI_First, FMI_All, FMI_Off = 0};`
With `First` to match the current behavior, `All` for the new behavior.
But then matched includes with a negative priority would be swapped with the
other possible main_include at each run.
I don't know how to prevent this.
The best solution I can imagine, is still a comment of the programmer, that the
respective include is or is not a main include.
E.g. `// clang-format pragma: no_main_include`
Another idea would be, to insert all main includes into a list with the
matchers' priority, and then take the last negative or the first positive, but
only if it is not partially sortable with the neighbors.
This might be a good heuristic, whether the include was previously detected as
main include.
But I don't know if this is implementable with the current algorithm.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143691/new/
https://reviews.llvm.org/D143691
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
