[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