[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 26 02:50:23 PDT 2019


kadircet added inline comments.


================
Comment at: clangd/Diagnostics.cpp:78
+// offsets when displaying that information to users.
+Position toOneBased(Position P) {
+  ++P.line;
----------------
ilya-biryukov wrote:
> Could we avoid introducing a function that breaks the invariant of a type?
> Having a function to print `Position` differently would be a much better fit.
No longer needed reverting.


================
Comment at: clangd/Diagnostics.cpp:225
+    const auto &Inc = D.IncludeStack[I];
+    OS << "\nIn file included from: " << Inc.first << ':'
+       << toOneBased(Inc.second);
----------------
ilya-biryukov wrote:
> WDYT about changing this to a shorter form?
> ```
> include stack:
> 
> ./project/foo.h:312
> /usr/include/vector:344
> ```
> Note that it does not mention column numbers as they aren't useful and is shorter.
> Since we're already not 100% aligned with clang, why not have a more concise representation?
As discussed offline getting rid of include stack in this version of the patch.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302





More information about the cfe-commits mailing list