[PATCH] Allow a module to require the absence of a feature

[Richard Smith]

================
Comment at: include/clang/Basic/Module.h:94
@@ +93,3 @@
+  /// the required state of that feature.
+  typedef std::pair<std::string, bool> Requirement;
+
----------------
Daniel Jasper wrote:
> Not really this change, but I'd expect a lot of modules in a single compilation to have a similar set of features. Thus, using the IdentifierTable to intern these strings and then use their pointer identity would be more efficient. Then, you could also represent this using a PointerIntPair. It might be the wrong place for optimization, though.
I tried this, but it didn't really seem to make the code cleaner, and I don't think the memory/disk benefits will be significant, so I backed it out again.

================
Comment at: include/clang/Basic/Module.h:101
@@ -96,3 +100,3 @@
   /// available.
-  SmallVector<std::string, 2> Requires;
+  SmallVector<Requirement, 2> Requires;
 
----------------
Daniel Jasper wrote:
> Maybe now rename this to Requirements?
Done.

================
Comment at: include/clang/Basic/Module.h:373
@@ -368,1 +372,3 @@
   ///
+  /// \param RequiredState The required state of this feature: \c true
+  /// if it must be present, \c false if it must be absent.
----------------
Daniel Jasper wrote:
> I think it is not really a "state", but I also cannot come up with a better name. I thought about "IsRequired", but that does not really intuitively mean that false means "forbidden".
I could replace it with an `enum`. This whole class needs more encapsulation, though, and I'd be inclined to do that all as a separate change.

================
Comment at: lib/Basic/Module.cpp:78
@@ -78,1 +77,3 @@
+              Current->Requires[I].second) {
+        Req = Current->Requires[I];
         return false;
----------------
Daniel Jasper wrote:
> Probably unimportant: AFAICT, this now copies the string whereas it just copied a StringRef before. Maybe one more reason for string interning.
I think we don't care. This code path is only used when issuing a diagnostic.


http://llvm-reviews.chandlerc.com/D2027