[PATCH] D137751: Produce a more informative diagnostics when Clang runs out of source locations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for working on this, I think this will help folks who run out of source locations. Can you also add a release note and public docs for this extension?



================
Comment at: clang/include/clang/Basic/SourceManager.h:1695-1696
+  // Produce notes describing the current source location address space usage.
+  void noteSLocAddressSpaceUsage(DiagnosticsEngine &Diag,
+                                 unsigned MaxNotes = 50) const;
+
----------------
Not that I'm opposed, but how did you arrive at 50?


================
Comment at: clang/lib/Basic/SourceManager.cpp:675
   LocalSLocEntryTable.push_back(SLocEntry::get(NextLocalOffset, Info));
+  // TODO: Produce a proper diagnostic for this case.
   assert(NextLocalOffset + Length + 1 > NextLocalOffset &&
----------------
Are you planning to do this as part of this patch, or is this more of an aspirational FIXME for the future?


================
Comment at: clang/lib/Basic/SourceManager.cpp:2281-2282
+  SortedUsage.reserve(Usage.size());
+  for (auto It = Usage.begin(); It != Usage.end(); ++It)
+    SortedUsage.push_back(It);
+  auto Cmp = [](UsageMap::iterator A, UsageMap::iterator B) {
----------------
llvm::copy?


================
Comment at: clang/lib/Lex/Pragma.cpp:1187
+      // specifically report information about.
+      uint64_t MaxNotes = (uint64_t)-1;
+      Token ArgToken;
----------------
`~0ULL`?


================
Comment at: clang/lib/Lex/Pragma.cpp:1190-1195
+      if (ArgToken.isNot(tok::eod)) {
+        if (ArgToken.isNot(tok::numeric_constant) ||
+            !PP.parseSimpleIntegerLiteral(ArgToken, MaxNotes)) {
+          PP.Diag(ArgToken, diag::warn_pragma_debug_unexpected_argument);
+        }
+      }
----------------
These predicates can be combined into one `if` statement.


================
Comment at: clang/test/Misc/sloc-usage.cpp:1
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
----------------
Should we do anything special about source locations from the command line, or do those not contribute source locations? (e.g., `-D` or `-U`, force include file, etc flags)

(Testing a forced include would be interesting regardless, just to ensure we catch those the same as we do an `#include`.)


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

https://reviews.llvm.org/D137751