forked from OSchip/llvm-project
d46fa023ca
Fixes [[ https://github.com/llvm/llvm-project/issues/38995 | #38995 ]] This is an attempt to modify the regular expression to identify `@import` and `import` alongside the regular `#include`. The challenging part was not to support `@` in addition to `#` but how to handle everything that comes after the `include|import` keywords. Previously everything that wasn't `"` or `<` was consumed. But as you can see in this example from the issue #38995, there is no `"` or `<` following the keyword: ``` @import Foundation; ``` I experimented with a lot of fancy and useful expressions in [this online regex tool](https://regex101.com) only to find out that some things are simply not supported by the regex implementation in LLVM. * For example the beginning `[\t\ ]*` should be replacable by the horizontal whitespace character `\h*` but this will break the `SortIncludesTest.LeadingWhitespace` test. That's why I've chosen to come back to the basic building blocks. The essential change in this patch is the change from this regular expression: ``` ^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]) ~ ~~~~~~~~~~~~~~ ^ ^ | | only support # prefix not @ | only support "" and <> as delimiters no support for C++ modules and ; ending. Also this allows for "> or <" or "" or <> which all seems either off or wrong. ``` to this: ``` ^[\t\ ]*[@#][\t\ ]*(import|include)([^"]*("[^"]+")|[^<]*(<[^>]+>)|[\t\ ]*([^;]+;)) ~~~~ ~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~ ^ ^ ^ ^ ^ | | | | | Now support @ and #. Clearly support "" and <> as well as an include name without enclosing characters. Allows for no mixture of "> or <" or empty include names. ``` Here is how I've tested this patch: ``` ninja clang-Format ninja FormatTests ./tools/clang/unittests/Format/FormatTests --gtest_filter=SortIncludesTest* ``` And if that worked I doubled checked that nothing else broke by running all format checks: ``` ./tools/clang/unittests/Format/FormatTests ``` One side effect of this change is it should partially support [C++20 Module](https://en.cppreference.com/w/cpp/language/modules) `import` lines without the optional `export` in front. Adding this can be a change on its own that shouldn't be too hard. I say partially because the `@` or `#` are currently *NOT* optional in the regular expression. I see an opportunity to optimized the matching to exclude `@include` for example. But eventually these should be caught by the compiler, so... With my change, the matching group is not at a fixed position any longer. I decided to choose the last match (group) that is not empty. Reviewed By: HazardyKnusperkeks Differential Revision: https://reviews.llvm.org/D121370 |
||
|---|---|---|
| .. | ||
| ASTDiff | ||
| Core | ||
| DependencyScanning | ||
| DumpTool | ||
| Inclusions | ||
| Refactoring | ||
| Syntax | ||
| Transformer | ||
| AllTUsExecution.cpp | ||
| ArgumentsAdjusters.cpp | ||
| CMakeLists.txt | ||
| CommonOptionsParser.cpp | ||
| CompilationDatabase.cpp | ||
| EmptyNodeIntrospection.inc.in | ||
| Execution.cpp | ||
| ExpandResponseFilesCompilationDatabase.cpp | ||
| FileMatchTrie.cpp | ||
| FixIt.cpp | ||
| GuessTargetAndModeCompilationDatabase.cpp | ||
| InterpolatingCompilationDatabase.cpp | ||
| JSONCompilationDatabase.cpp | ||
| NodeIntrospection.cpp | ||
| Refactoring.cpp | ||
| RefactoringCallbacks.cpp | ||
| StandaloneExecution.cpp | ||
| Tooling.cpp | ||