[PATCH] D64098: [NFC][clang] Refactor getCompilationPhases step 1: Move list of phases into Types.def table.

Saleem Abdulrasool via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 2 14:06:37 PDT 2019


compnerd added a comment.

The explicit list I think is way better for readability, this is a nice starting point for cleaning this up.



================
Comment at: clang/include/clang/Driver/Types.def:18
 
 // TYPE(NAME, ID, PP_TYPE, TEMP_SUFFIX, FLAGS)
 
----------------
Please update the comment here indicating the new field


================
Comment at: clang/lib/Driver/Driver.cpp:3304
 
-    for (SmallVectorImpl<phases::ID>::iterator i = PL.begin(), e = PL.end();
-         i != e; ++i) {
+    for (auto i = PL.begin(), e = PL.end(); i != e; ++i) {
       phases::ID Phase = *i;
----------------
Why can this not be a range based for loop?


================
Comment at: clang/lib/Driver/Types.cpp:271
+//        the old behavior and a subsequent change will delete most of the body.
+const llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> &types::getCompilationPhases(ID Id) {
+  llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> P;
----------------
Can we return `llvm::SmallVectorImpl<phases::ID>` instead?  The size is immaterial to this I think.


================
Comment at: clang/lib/Driver/Types.cpp:303
+  return Phases;
 }
 
----------------
A comment to indicate that the concrete list will become the implementation would be nice.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64098/new/

https://reviews.llvm.org/D64098





More information about the cfe-commits mailing list