hokein added inline comments.

================
Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:438
+                         .map("Experiment",
+                              Config::IncludesPolicy::Strict) // for backward
+                                                              // compatibility
----------------
kadircet wrote:
> i think we should at least be emitting a diagnostics to encourage people for 
> moving back to strict, so what about something like:
> ```
> if (F.UnusuedIncludes) {
>   auto Val = compileEnum....; // only for Strict and None
>   if (!Val && **F.UnusedIncludes == "Experiment") {
>        diag(Warning, "Experiment is deprecated for UnusedIncludes, use Strict 
> instead.", F.UnusedIncludes.Range);
>        Val = Config::IncludesPolicy::Strict;
>   }
> }
> ```
I thought it was not worth a diagnostic because this flag was introduced 
recently, and we have never advertised it to open-source users.

But the flag is in the recent clangd16 release, so it probably justifies the 
value. 

BTW, looks like we forgot to update the release notes for clangd16, 
https://github.com/llvm/llvm-project/blob/release/16.x/clang-tools-extra/docs/ReleaseNotes.rst#improvements-to-clangd
 is empty.



================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:769
-      Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::Strict
-          ? computeUnusedIncludes(AST)
-          : Findings.UnusedIncludes,
----------------
kadircet wrote:
> can you also delete `computeUnusedIncludes` and its friends (also from the 
> tests)?
this is in plan, but in a separate patch, https://reviews.llvm.org/D145776


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145773

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to