hokein marked an inline comment as done. hokein added inline comments.
================ Comment at: clang/include/clang/Tooling/Core/Diagnostic.h:67-71 + // Get the chosen fix to apply for this diagnostic. + // FIXME: currently we choose the first non-empty fix, extend it to support + // fix selection. + const llvm::StringMap<Replacements> *getChosenFix() const; + ---------------- alexfh wrote: > hokein wrote: > > alexfh wrote: > > > Do we actually need this method here? This whole structure is sort of a > > > data-only serialization helper, and this method is adding some > > > (arbitrary) logic into it. > > We have a few places using this method, or we could move this method out of > > this structure? > I'd move it out closer to the code which uses it and call it > "selectFirstFix", for example (the name should be a verb and it should be > less vague about what the function is doing). hmm, this function is used in `ApplyReplacements.cpp`, `ClangTidy.cpp`, `ClangTidyTest.h`, `ClangTidyDiagnosticConsumer.cpp`. `Diagnostic.h` seems the most suitable place. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59932/new/ https://reviews.llvm.org/D59932 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
