[PATCH] D150185: [include-cleaner] Allow multiple strategies for spelling includes.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo added a comment.

Thanks for the comments!



================
Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h:104
+  ApplyFirstIncludeSpeller() {
+    for (const auto &Strategy :
+         include_cleaner::IncludeSpellingStrategy::entries()) {
----------------
kadircet wrote:
> instead of a constructor you can have:
> ```
> static auto *Strategies = []{
>   auto *Result = new llvm::SmallVector<std::unique_ptr<include_cleaner::IncludeSpeller>>;
>   for(auto &Strategy: include_cleaner::IncludeSpellingStrategy::entries()) {
>    Result->push_back(Strategy.instantiate());
>   }
> }();
> ```
> 
> in the functor implementation.
Ok, in that case I'll get rid of the class altogether.


================
Comment at: clang-tools-extra/include-cleaner/lib/Analysis.cpp:110
+                 Ref.RT == RefType::Explicit) {
+               ApplyFirstIncludeSpeller Speller;
+               Missing.insert(
----------------
kadircet wrote:
> we should instantiate this outside the callback instead (to make sure we do it once). it would become obsolete if you're to use a static variable to store the strategies though.
Using a static variable now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150185/new/

https://reviews.llvm.org/D150185