llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clangd Author: kadir çetinkaya (kadircet) <details> <summary>Changes</summary> False negative/positive rate has decreased to the degree that these extra confirmations no longer provide any value, but only create friction in the happy case. When we have false analysis, people usually need to apply the fixes and run the builds to discover the failure. At that point they can add relevant IWYU pragmas to guide analysis, and will be more likely to create bug reports due to extra annoyance :) --- Full diff: https://github.com/llvm/llvm-project/pull/76826.diff 2 Files Affected: - (modified) clang-tools-extra/clangd/IncludeCleaner.cpp (+1-26) - (modified) clang-tools-extra/clangd/test/include-cleaner-batch-fix.test (-68) ``````````diff diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp index 563a1f5d22f5b5..672140a6f2b4d8 100644 --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -48,7 +48,6 @@ #include <cassert> #include <iterator> #include <map> -#include <memory> #include <optional> #include <string> #include <utility> @@ -237,18 +236,6 @@ removeAllUnusedIncludes(llvm::ArrayRef<Diag> UnusedIncludes) { Diag.Fixes.front().Edits.begin(), Diag.Fixes.front().Edits.end()); } - - // TODO(hokein): emit a suitable text for the label. - ChangeAnnotation Annotation = {/*label=*/"", - /*needsConfirmation=*/true, - /*description=*/""}; - static const ChangeAnnotationIdentifier RemoveAllUnusedID = - "RemoveAllUnusedIncludes"; - for (unsigned I = 0; I < RemoveAll.Edits.size(); ++I) { - ChangeAnnotationIdentifier ID = RemoveAllUnusedID + std::to_string(I); - RemoveAll.Edits[I].annotationId = ID; - RemoveAll.Annotations.push_back({ID, Annotation}); - } return RemoveAll; } @@ -268,20 +255,8 @@ addAllMissingIncludes(llvm::ArrayRef<Diag> MissingIncludeDiags) { Edits.try_emplace(Edit.newText, Edit); } } - // FIXME(hokein): emit used symbol reference in the annotation. - ChangeAnnotation Annotation = {/*label=*/"", - /*needsConfirmation=*/true, - /*description=*/""}; - static const ChangeAnnotationIdentifier AddAllMissingID = - "AddAllMissingIncludes"; - unsigned I = 0; - for (auto &It : Edits) { - ChangeAnnotationIdentifier ID = AddAllMissingID + std::to_string(I++); + for (auto &It : Edits) AddAllMissing.Edits.push_back(std::move(It.second)); - AddAllMissing.Edits.back().annotationId = ID; - - AddAllMissing.Annotations.push_back({ID, Annotation}); - } return AddAllMissing; } Fix fixAll(const Fix &RemoveAllUnused, const Fix &AddAllMissing) { diff --git a/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test b/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test index af7cdba5b05f43..07ebe1009a78f6 100644 --- a/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test +++ b/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test @@ -157,21 +157,10 @@ # CHECK-NEXT: { # CHECK-NEXT: "arguments": [ # CHECK-NEXT: { -# CHECK-NEXT: "changeAnnotations": { -# CHECK-NEXT: "AddAllMissingIncludes0": { -# CHECK-NEXT: "label": "", -# CHECK-NEXT: "needsConfirmation": true -# CHECK-NEXT: }, -# CHECK-NEXT: "AddAllMissingIncludes1": { -# CHECK-NEXT: "label": "", -# CHECK-NEXT: "needsConfirmation": true -# CHECK-NEXT: } -# CHECK-NEXT: }, # CHECK-NEXT: "documentChanges": [ # CHECK-NEXT: { # CHECK-NEXT: "edits": [ # CHECK-NEXT: { -# CHECK-NEXT: "annotationId": "AddAllMissingIncludes0", # CHECK-NEXT: "newText": "#include {{.*}}bar.h{{.*}}", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { @@ -185,7 +174,6 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: { -# CHECK-NEXT: "annotationId": "AddAllMissingIncludes1", # CHECK-NEXT: "newText": "#include {{.*}}foo.h{{.*}}", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { @@ -213,29 +201,10 @@ # CHECK-NEXT: { # CHECK-NEXT: "arguments": [ # CHECK-NEXT: { -# CHECK-NEXT: "changeAnnotations": { -# CHECK-NEXT: "AddAllMissingIncludes0": { -# CHECK-NEXT: "label": "", -# CHECK-NEXT: "needsConfirmation": true -# CHECK-NEXT: }, -# CHECK-NEXT: "AddAllMissingIncludes1": { -# CHECK-NEXT: "label": "", -# CHECK-NEXT: "needsConfirmation": true -# CHECK-NEXT: }, -# CHECK-NEXT: "RemoveAllUnusedIncludes0": { -# CHECK-NEXT: "label": "", -# CHECK-NEXT: "needsConfirmation": true -# CHECK-NEXT: }, -# CHECK-NEXT: "RemoveAllUnusedIncludes1": { -# CHECK-NEXT: "label": "", -# CHECK-NEXT: "needsConfirmation": true -# CHECK-NEXT: } -# CHECK-NEXT: }, # CHECK-NEXT: "documentChanges": [ # CHECK-NEXT: { # CHECK-NEXT: "edits": [ # CHECK-NEXT: { -# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes0", # CHECK-NEXT: "newText": "", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { @@ -249,7 +218,6 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: { -# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes1", # CHECK-NEXT: "newText": "", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { @@ -263,7 +231,6 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: { -# CHECK-NEXT: "annotationId": "AddAllMissingIncludes0", # CHECK-NEXT: "newText": "#include {{.*}}bar.h{{.*}}", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { @@ -277,7 +244,6 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: { -# CHECK-NEXT: "annotationId": "AddAllMissingIncludes1", # CHECK-NEXT: "newText": "#include {{.*}}foo.h{{.*}}", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { @@ -342,21 +308,10 @@ # CHECK-NEXT: { # CHECK-NEXT: "arguments": [ # CHECK-NEXT: { -# CHECK-NEXT: "changeAnnotations": { -# CHECK-NEXT: "RemoveAllUnusedIncludes0": { -# CHECK-NEXT: "label": "", -# CHECK-NEXT: "needsConfirmation": true -# CHECK-NEXT: }, -# CHECK-NEXT: "RemoveAllUnusedIncludes1": { -# CHECK-NEXT: "label": "", -# CHECK-NEXT: "needsConfirmation": true -# CHECK-NEXT: } -# CHECK-NEXT: }, # CHECK-NEXT: "documentChanges": [ # CHECK-NEXT: { # CHECK-NEXT: "edits": [ # CHECK-NEXT: { -# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes0", # CHECK-NEXT: "newText": "", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { @@ -370,7 +325,6 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: { -# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes1", # CHECK-NEXT: "newText": "", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { @@ -398,29 +352,10 @@ # CHECK-NEXT: { # CHECK-NEXT: "arguments": [ # CHECK-NEXT: { -# CHECK-NEXT: "changeAnnotations": { -# CHECK-NEXT: "AddAllMissingIncludes0": { -# CHECK-NEXT: "label": "", -# CHECK-NEXT: "needsConfirmation": true -# CHECK-NEXT: }, -# CHECK-NEXT: "AddAllMissingIncludes1": { -# CHECK-NEXT: "label": "", -# CHECK-NEXT: "needsConfirmation": true -# CHECK-NEXT: }, -# CHECK-NEXT: "RemoveAllUnusedIncludes0": { -# CHECK-NEXT: "label": "", -# CHECK-NEXT: "needsConfirmation": true -# CHECK-NEXT: }, -# CHECK-NEXT: "RemoveAllUnusedIncludes1": { -# CHECK-NEXT: "label": "", -# CHECK-NEXT: "needsConfirmation": true -# CHECK-NEXT: } -# CHECK-NEXT: }, # CHECK-NEXT: "documentChanges": [ # CHECK-NEXT: { # CHECK-NEXT: "edits": [ # CHECK-NEXT: { -# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes0", # CHECK-NEXT: "newText": "", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { @@ -434,7 +369,6 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: { -# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes1", # CHECK-NEXT: "newText": "", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { @@ -448,7 +382,6 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: { -# CHECK-NEXT: "annotationId": "AddAllMissingIncludes0", # CHECK-NEXT: "newText": "#include {{.*}}bar.h{{.*}}", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { @@ -462,7 +395,6 @@ # CHECK-NEXT: } # CHECK-NEXT: }, # CHECK-NEXT: { -# CHECK-NEXT: "annotationId": "AddAllMissingIncludes1", # CHECK-NEXT: "newText": "#include {{.*}}foo.h{{.*}}", # CHECK-NEXT: "range": { # CHECK-NEXT: "end": { `````````` </details> https://github.com/llvm/llvm-project/pull/76826 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
