[PATCH] D70263: [Error] Add source location macro

Lang Hames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 18 13:33:54 PST 2019


lhames added a comment.

In D70263#1748442 <https://reviews.llvm.org/D70263#1748442>, @hintonda wrote:

> In D70263#1748105 <https://reviews.llvm.org/D70263#1748105>, @lhames wrote:
>
> > In my initial commit, I had added a new class specifically for this, SourceLocationError, but @beanz convinced me to reuse FileError, since the only difference was the message formatting.
>
>
> Should I add back SourceLocationError?


We definitely shouldn't re-use FileError, but I'm not sure we want to use SourceLocationError either: On the one hand it gets you the line info, on the other hand it means that your errors have different types in debug builds vs non-debug builds. This could lead to some pretty subtle issues when mixing debug and non-debug code, or trying to reproduce issues from release builds on debug builds.

What if we used a side table and stored the line info for every in-flight error? (We could condition this on LLVM_ENABLE_EXPENSIVE_CHECKS if it proved to be expensive, but I seriously doubt it would be: the cost is marginal and only paid for failure values, not success values).

The hard part would be automating the printing of this source location information. Right now that would require cooperation from every ErrorInfo subclass, since right now rendering is fully controlled by the user supplied log method. We could make the log method private and force everyone to use a new stream operator like this:

  DenseMap<ErrorInfoBase*, std::pair<const char*, unsigned>> ErrorOriginSourceLoc;
  
  raw_ostream &operator<<(raw_ostream &OS, const ErrorInfoBase &EIB) {
    EIB.log(OS);
  #ifndef NDEBUG
  #ifdef ENABLE_EXPENSIVE_CHECKS
    auto LocI = ErrorOriginSourceLoc.find(&EIB);
    if (LocI != ErrorOriginSourceLoc.end()) {
      OS << " (Error originated at " << LocI->second.first << ":" << LocI->second.second << ")";
    }
  #endif // ENABLE_EXPENSIVE_CHECKS
  #endif // NDEBUG
    return OS; 
  }

That way we're free to decorate user supplied log messages in debug builds in any way we want (in this case by adding the source location information).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70263





More information about the llvm-commits mailing list