[PATCH] D14691: clang side of http://reviews.llvm.org/D14690

Chandler Carruth via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 2 11:45:25 PST 2015


chandlerc added inline comments.

================
Comment at: include/clang/Lex/HeaderSearchOptions.h:145
@@ -143,2 +144,3 @@
   /// of computing the module hash.
-  llvm::SetVector<std::string> ModulesIgnoreMacros;
+  llvm::SetVector<std::string, std::vector<std::string>, std::set<std::string>>
+      ModulesIgnoreMacros;
----------------
There is clearly a use case for a fast StringSetVector. If it is something other than SetVector<std::string>, we should put it in LLVM's ADT and use it everywhere rather than reproducing this huge type.

================
Comment at: include/clang/Sema/Sema.h:6847
@@ -6844,3 +6846,3 @@
 
-  typedef llvm::SmallSet<SourceLocation, 2> SrcLocSet;
+  typedef llvm::SmallSet<SourceLocation, 2, std::set<SourceLocation>> SrcLocSet;
   typedef llvm::DenseMap<IdentifierInfo *, SrcLocSet> IdentifierSourceLocations;
----------------
Wow, do we really not have DenseMapInfo for SourceLocation? It seems like this would be a great candidate for DenseSet....

================
Comment at: lib/Driver/Tools.cpp:6227
@@ -6226,3 +6226,3 @@
   CmdArgs.push_back("-flavor");
-  CmdArgs.push_back("gnu");
+  CmdArgs.push_back("old-gnu");
   CmdArgs.push_back("-target");
----------------
Does this belong is this patch? If so, why?


http://reviews.llvm.org/D14691





More information about the cfe-commits mailing list