erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land.
Previous comments on this were here: https://reviews.llvm.org/D147552. I've copy/pasted them into here to keep history at least somewhat reasonable. This LGTM. ================ Comment at: clang/lib/AST/ComputeDependence.cpp:753 // expansions. - for (auto A : E->template_arguments()) + for (const auto &A : E->template_arguments()) Deps |= toExprDependence(A.getArgument().getDependence()); ---------------- Ends up being pretty sizable, TemplateArgumentLoc has a TemplateArgument (which we typically pass around by value), but it also has a TemplateArgumentLocInfo, which is a pair of pointers + a pair of source locations. ================ Comment at: clang/lib/Frontend/FrontendActions.cpp:885 // Now let's print out any modules we did not see as part of the Primary. - for (auto SM : SubModMap) { + for (const auto &SM : SubModMap) { if (!SM.second.Seen && SM.second.Mod) { ---------------- A pair of a string and the other type, definitely worth saving the copies. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147574/new/ https://reviews.llvm.org/D147574 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
