tahonermann added inline comments.
================ Comment at: clang/lib/Format/Format.cpp:3486-3489 + Expanded.InsertBraces = true; + Passes.emplace_back([&](const Environment &Env) { + return BracesInserter(Env, Expanded).process(/*SkipAnnotation=*/true); }); ---------------- HazardyKnusperkeks wrote: > HazardyKnusperkeks wrote: > > HazardyKnusperkeks wrote: > > > MyDeveloperDay wrote: > > > > owenpan wrote: > > > > > tahonermann wrote: > > > > > > How about using an init capture instead? This will suffice to avoid > > > > > > one of the copies but means that `InsertBraces` doesn't get set > > > > > > until the lambda is invoked. I wouldn't expect that to matter > > > > > > though. > > > > > I'm not sure if it's worth the trouble, but if I really had to > > > > > bother, I would do something like the above. > > > > Apart from perhaps the unnecessary copying from Expanded -> S.. doesn't > > > > this bascially do the same without us having to remember to put > > > > Expanded back afterwards? I don't see how using Expanded is any > > > > different from using S (other than the copy) > > > > > > > > ``` > > > > if (Style.InsertBraces) { > > > > FormatStyle S = Expanded; > > > > S.InsertBraces = true; > > > > Passes.emplace_back([&](const Environment &Env) { > > > > return BracesInserter(Env, S).process(/*SkipAnnotation=*/true); > > > > }); > > > > } > > > > ``` > > > > I'm not sure if it's worth the trouble, but if I really had to bother, > > > > I would do something like the above. > > > > > > That wouldn't work, since the pass is only executed afterwards. > > > Apart from perhaps the unnecessary copying from Expanded -> S.. doesn't > > > this bascially do the same without us having to remember to put Expanded > > > back afterwards? I don't see how using Expanded is any different from > > > using S (other than the copy) > > > > > > ``` > > > if (Style.InsertBraces) { > > > FormatStyle S = Expanded; > > > S.InsertBraces = true; > > > Passes.emplace_back([&](const Environment &Env) { > > > return BracesInserter(Env, S).process(/*SkipAnnotation=*/true); > > > }); > > > } > > > ``` > > > > That would have a dangling reference to S and most likely don't work as > > wished. > This should work. One could replace the two assignments with a RAII wrapper > which restores the old value. Here is another option; it is just the original code but with an init capture that does a move instead of a copy. Coverity shouldn't complain about a move capture (if it does, I'll file a support case with Synopsys). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149647/new/ https://reviews.llvm.org/D149647 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits