[PATCH] D143096: [clangd] Provide patched diagnostics with preamble patch

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 2 06:45:56 PST 2023


sammccall added a comment.

It looks like this fixes up the location only of diagnostics attached to particular directives (`#include`) based on some "deep" idea about the content of the directive (the spelled header name).

Some shortcomings:

- This misses diagnostics attached to other directives / continued lines in macro definitions / etc
- it allows diagnostics to be translated across lines even if the directive spelling changed (in which case the ranges are incorrect since only line numbers are updated.)
- it requires modelling the directive content in some way that needs to be extended if we find another place that diagnostics can be attached

We discussed the idea of attaching to the text of the line, which seems pretty generic. Needs some handling of duplicate line content but seems like something pretty naive (closest line number with matching content?) would work well, be just as simple and more generic. Any reason this alternative was rejected?



================
Comment at: clang-tools-extra/clangd/Preamble.h:162
+  /// to an #include or #define directive.
+  std::vector<Diag> patchedDiags() const { return PatchedDiags; }
   static constexpr llvm::StringLiteral HeaderName = "__preamble_patch__.h";
----------------
this looks like a gratuitous copy - ParsedAST copies again


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143096



More information about the cfe-commits mailing list