[PATCH] Refactoring, update MacroInfo interface so a for range can be used to iterate through the tokens

Richard Smith richard at metafoo.co.uk
Tue Apr 28 14:24:35 PDT 2015


> question: is ArrayRef faster/safer/cleaner somehow?


Yes:

1. `ArrayRef` leaks fewer implementation details to the caller; it's just a guaranteed-to-be-contiguous sequence.
2. The reason for dealing in `SmallVectorImpl`s is to mutate them. Using `SmallVectorImpl` for read-only access is surprising.
3. Range accessors are usually assumed to provide a shallow view of some external data. In particular, it's not unreasonable to assume `auto T = MI->tokens();` to be cheap (which it is for `ArrayRef` and not for `SmallVectorImpl`).


================
Comment at: tools/extra/modularize/PreprocessorTracker.cpp:408
@@ -407,4 +407,3 @@
   // Walk over the macro Tokens.
-  typedef clang::MacroInfo::tokens_iterator Iter;
-  for (Iter I = MI->tokens_begin(), E = MI->tokens_end(); I != E; ++I) {
-    clang::IdentifierInfo *II = I->getIdentifierInfo();
+  for (const clang::Token &T : MI->tokens()) {
+    clang::IdentifierInfo *II = T.getIdentifierInfo();
----------------
Use `auto` here to remove the risk of creating a temporary? I think it's sufficiently clear that you get `Token`s from `tokens()`.

http://reviews.llvm.org/D9079

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list