[PATCH] D40606: [Support/TarWriter] - Don't allow TarWriter to add the same file more than once.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: include/llvm/Support/TarWriter.h:16
 #include "llvm/Support/raw_ostream.h"
+#include <set>
 
----------------
ruiu wrote:
> Please use llvm::StringSet.
It does not work:

```
>D:\Work\llvm\include\llvm/ADT/StringMap.h(209): error C2039: 'Deallocate': is not a member of 'std::basic_string<char,std::char_traits<char>,std::allocator<char>>'
1>  C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\include\xstring(2633): note: see declaration of 'std::basic_string<char,std::char_traits<char>,std::allocator<char>>'
1>  D:\Work\llvm\include\llvm/ADT/StringMap.h(301): note: see reference to function template instantiation 'void llvm::StringMapEntry<ValueTy>::Destroy<AllocatorTy>(AllocatorTy &)' being compiled
1>          with
1>          [
1>              ValueTy=char,
1>              AllocatorTy=std::string
1>          ]
```

I think llvm containers does not know how to handle std::string. So I had to use std::set. It seems common in LLVM to use it for std::string.


================
Comment at: lib/Support/TarWriter.cpp:177-178
+  bool Exist;
+  std::tie(std::ignore, Exist) = Files.insert(Fullpath);
+  if (Exist)
+    return false;
----------------
ruiu wrote:
> Or, just `if (Seen.insert(Fullpath).second) return;`
That looks much better, thanks !


https://reviews.llvm.org/D40606