dblaikie added inline comments.
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:881
+ // Sort options by key to avoid relying on StringMap iteration order.
+ SmallVector<std::tuple<llvm::StringRef, std::string>, 4> SortedConfigOpts;
for (const auto &C : Opts.Config) {
----------------
jansvoboda11 wrote:
> jansvoboda11 wrote:
> > I think you should be able to drop `llvm::`.
> No reason to allocate for this temporary storage IMO, you could replace
> `std::string` with `StringRef`.
Perhaps std::pair would be slightly easier to read than std::tuple?
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:882-884
for (const auto &C : Opts.Config) {
+ SortedConfigOpts.emplace_back(C.getKey(), C.getValue());
+ }
----------------
jansvoboda11 wrote:
> Per our coding style, we don't put braces around bodies with just a single
> statement.
We usually skip braces on single line blocks
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:889-891
+ for (const auto &C : SortedConfigOpts) {
+ const llvm::StringRef &Key = std::get<0>(C);
+ const std::string &Value = std::get<1>(C);
----------------
jansvoboda11 wrote:
> You could use C++17 structured bindings: `for (const auto &[Key, Value] :
> SortedConfigOpts) { ... }`.
maybe a structured binding here?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142861/new/
https://reviews.llvm.org/D142861
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
