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

Daniel Jasper djasper at google.com
Sat Oct 26 09:51:09 PDT 2013



================
Comment at: include/clang/Basic/Module.h:94
@@ +93,3 @@
+  /// the required state of that feature.
+  typedef std::pair<std::string, bool> Requirement;
+
----------------
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.

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

================
Comment at: lib/Basic/Module.cpp:78
@@ -78,1 +77,3 @@
+              Current->Requires[I].second) {
+        Req = Current->Requires[I];
         return false;
----------------
Probably unimportant: AFAICT, this now copies the string whereas it just copied a StringRef before. Maybe one more reason for string interning.

================
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.
----------------
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".


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



More information about the cfe-commits mailing list