[PATCH] D27900: [ELF] - Keep the source file/line location information separate from the object file location information.

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

In https://reviews.llvm.org/D27900#640961, @ruiu wrote:

> How about this? You can change the type of the function from
>
>   void error(const Twine &Msg);
>   
>
> to
>
>   void error(const Twine &Msg, const Twine &Note = "");
>   
>
> Then you can eliminate note() function, and it will also solve the problem that you cannot call note() function after fatal().


That works good generally except one place:

  template <class ELFT>
  static void reportDuplicate(SymbolBody *Existing,
                              InputSectionBase<ELFT> *ErrSec,
                              typename ELFT::uint ErrOffset) {
  ....
    print(toString(ErrSec->getFile()) + ": duplicate symbol '" +
              toString(*Existing) + "'",
          "definition found in " + D->Section->getLocation(D->Value));
    print("'" + toString(*Existing) + "' previous definition was here: " +
              toString(D->Section->getFile()),
          "definition found in " + ErrSec->getLocation(ErrOffset));
  }

Previous iteration generated error + 3 notes. Current one generates error+note, error+note.
That is not ideal as generating 2 errors instead of one does not look clean, also them may be
emited far from each other in multithreaded case.

I see next possible solutions:

1. Change signatures to something like: void error(const Twine &Msg, const std::vector<std::string> &Notes) and pass multiple notes at once. Problem is that is looks not possible to have a vector of Twines.

2. Implement error(), warn(), fatal() using variardic template for Notes argument. That way we should be able to use const Twine& arguments for notes as usual I think. Something like:

  template<typename... Note>  
  void error(const Twine& Msg, const Note&... args)  { ... }



3. Protect print calls in reportDuplicate() with std::lock_guard + Mu That way Mu type needs to be changed to std::recursive_mutex and exported I think.



  std::lock_guard<std::reqursive_mutex> Lock(Mu);
  print(toString(ErrSec->getFile()) + ": duplicate symbol '" +
            toString(*Existing) + "'",
        "definition found in " + D->Section->getLocation(D->Value));
  print("'" + toString(*Existing) + "' previous definition was here: " +
            toString(D->Section->getFile()),
        "definition found in " + ErrSec->getLocation(ErrOffset));

  		 

4. Use multiline note:

         print(toString(ErrSec->getFile()) + ": duplicate symbol '" +
               toString(*Existing) + "'",
           "definition found in " + D->Section->getLocation(D->Value) + 
  		 "\nprevious definition was here: " + toString(D->Section->getFile()) +
  		 "\ndefinition found in " + ErrSec->getLocation(ErrOffset));

That way it will emit error + single multiline note.

     


https://reviews.llvm.org/D27900