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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits