[PATCH] D90121: clang-format: Add a consumer to diagnostics engine

dmikis added a comment. > I took a look and in its simplest form moving TextDiagnosticPrinter.cpp to > lib/Basic but also requires DiagnosticRender.cpp and TextDiagnostic.cpp to > also move to lib/Basic too, > > But DiagnosticRender.cpp has a dependency on lib/Edit so clang-format now > needs t

[PATCH] D90121: clang-format: Add a consumer to diagnostics engine

dmikis added a comment. @MyDeveloperDay That change introduced a regression: clang-format crashes instead of reporting errors in certain situations. See my comment with a test case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90121/new/ https:/

[PATCH] D90121: clang-format: Add a consumer to diagnostics engine

dmikis added a comment. @thakis I've got test working, here's the code: // RUN: chmod +w %t.dir || true // RUN: rm -rf %t.dir // RUN: mkdir %t.dir // RUN: cp %s %t.dir/read-only.cpp // RUN: chmod -w %t.dir // RUN: clang-format -style=LLVM -i %t.dir/read-only.cpp || test $? == 1 i

[PATCH] D90121: clang-format: Add a consumer to diagnostics engine

dmikis added a comment. @thakis We've stumbled upon this problem when tried to format in-place a read-only file. `clang-format` tries to report this problem via `DiagnosticEngine`, and that fails on assert

[PATCH] D90121: clang-format: Add a consumer to diagnostics engine

dmikis added a comment. @thakis There's a simple option of moving `TextDiagnosticPrinter` into `libBasic` thus eliminating neccessety to depend on `libFrontend`. How does that sound? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90121/new/ https:

[PATCH] D90121: clang-format: Add a consumer to diagnostics engine

dmikis added a comment. @krasimir Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90121/new/ https://reviews.llvm.org/D90121 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.ll

[PATCH] D90121: clang-format: Add a consumer to diagnostics engine

dmikis added a comment. @krasimir What would our next steps be to get this change into LLVM source tree? JIC, I don't have commit rights :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90121/new/ https://reviews.llvm.org/D90121 _

[PATCH] D90121: clang-format: Add a consumer to diagnostics engine

dmikis added a comment. > If this usage leads to crashes, isn't the issue in DiagnosticsEngine itself? I don't know design intent behind `DiagnosticsEngine`. As far as I can understand from source code it doesn't try to create any consumers by itself yet requires one to be provided (there're as

[PATCH] D90121: Add a consumer to diagnostics engine

dmikis created this revision. dmikis added reviewers: djasper, bkramer, krasimir. dmikis added a project: clang-format. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. dmikis requested review of this revision. Otherwise problems like trying to format readonly file in-