[PATCH] D47187: [clangd] Skip .inc headers when canonicalizing header #include.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed May 23 02:50:41 PDT 2018

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.


Comment at: clangd/index/CanonicalIncludes.cpp:50
+  if (I == Headers.end())
+    return Headers[0]; // Fallback to the defining header.
+  llvm::StringRef Header = *I;
nit: here and elsewhere, you probably want "declaring" rather than defining

Comment at: clangd/index/CanonicalIncludes.h:52
+  /// Returns the canonical include for symbol with \p QualifiedName. \p Headers
+  /// is a stack of #includes that starts from the last included header (i.e.
+  /// header that defines the symbol) to the header file that is directly
This might be clearer slightly more concretely:
\p Headers is the include stack: Headers.front() is the file declaring the symbol, and Headers.back() is the main file```
(or "included by the main file", but having it be main-file might be neater)

Comment at: clangd/index/CanonicalIncludes.h:54
+  /// header that defines the symbol) to the header file that is directly
+  /// included by the main file. When mapping headers, this skips files that are
+  /// not supposed to be #included directly e.g. .inc file.
Nit: "this" is a bit ambiguous here, the parameter or the algorithm?
Consider just dropping this new part of the comment, it's kind of an implementation detail (hiding behind the word "canonical")

Comment at: clangd/index/SymbolCollector.cpp:209
+  while (true) {
+    if (!Loc.isValid() || SM.isInMainFile(Loc))
+      break;
(as above, maybe want to include the main file for simplicity/symmetry)

  rCTE Clang Tools Extra


More information about the cfe-commits mailing list