aaron.ballman added inline comments.
================
Comment at: clang/include/clang/Frontend/SARIFDiagnosticPrinter.h:31
+class SARIFDiagnosticPrinter : public DiagnosticConsumer {
+ raw_ostream &OS;
+ IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts;
----------------
cjdb wrote:
> cjdb wrote:
> > Please make OS a pointer instead of a reference, since member references
> > make usage very unpleasant.
> Nit: please move all private members below the public interface, where
> possible.
> Please make OS a pointer instead of a reference, since member references make
> usage very unpleasant.
Err... I disagree (and just deleted a comment above asking for `OS` to turn
into a reference rather than a pointer because it can never be null). I think
we want to use a reference here because 1) it conveys the correct "I can never
be null" semantics so maintainers don't have to ask when that can happen, and
2) copies of this class are a bad idea to begin with (we should probably delete
the copy and move operations).
We use reference members elsewhere:
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Sema/Sema.h#L407
(not that I would hold Sema up as an example of best practices, lol).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131632/new/
https://reviews.llvm.org/D131632
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
