carlosgalvezp added inline comments.

================
Comment at: clang-tools-extra/clang-tidy/GlobList.h:50-52
+/// A \p GlobList that caches search results, so that search is performed only
+/// once for the same query.
+class CachedGlobList {
----------------
carlosgalvezp wrote:
> whisperity wrote:
> > If `CachedGlobList` //is-a// `GlobList`, wouldn't it be better to express 
> > the inheritance relationship?
> Yes, that makes total sense to me. I wasn't sure if it was OK to do the move 
> + change the code itself in the same patch. Will fix!
On the other hand, do we intend to use the class polymorphically? I wonder if 
private inheritance would be a better choice. Otherwise I'll need to start 
making the functions virtual and so on.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113422/new/

https://reviews.llvm.org/D113422

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to