aaron.ballman added a comment. In D77701#1969687 <https://reviews.llvm.org/D77701#1969687>, @nickdesaulniers wrote:
> In D77701#1969132 <https://reviews.llvm.org/D77701#1969132>, @aaron.ballman > wrote: > > > OTOH, this is reasonable, NFC, and I tend to agree about it being a code > > smell. > > OTOH, this makes parsing Sema.h that much slower and adds even more text > > for us to wade through in that header file. > > > > Given that the approaches are equivalent except for smell and that Sema.h > > is already 12kLoC long, I would rather leave this as-is or possibly even > > see us go in the opposite direction and use static functions whenever > > there's a private function only used in a single .cpp file and making it > > static doesn't make it overly awkward for some reason. > > > Totally, this was a yak shave no one asked for. I was looking at > `DiagnoseNoDiscard` in D77611 <https://reviews.llvm.org/D77611>. Fixing that > case there then checking for other instances in the TU turned up more. I > pulled a thread that unwound the sweater. Heh, yeah, I've been there before. > I don't have any attachment to this patch, so I'm ok with it being rejected. > If there's concerns with Sema's compile time, please do consider splitting it > up further though. I am curious to hear what other reviewers think, so I wouldn't reject based on my observations alone. I think splitting sema.h would be an interesting prospect, but the bulk of the file are declarations within the `Sema` class, so splitting may be nontrivial. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77701/new/ https://reviews.llvm.org/D77701 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
