Szelethus marked 6 inline comments as done. Szelethus added a comment. In D82585#2122634 <https://reviews.llvm.org/D82585#2122634>, @balazske wrote:
> It looks like that the dependency manipulation and computation functions I guess there are two ways of looking at this: - CheckerRegistry only contains the logic CheckerRegistryData cannot due to the library layout, but it is otherwise minimal - CheckerRegistryData is data only, and all logic is placed in CheckerRegistry I would prefer the second option -- dependency resolving is one of the most crucial, and very non-trivial task of `CheckerRegistry`, and while moving it to `CheckerRegistryData` wouldn't violate the library layout, it would certainly go outside of what I'd expect a data-centric class to do. I also would like to keep the interface of it minimal. > and the checker and option add functions can be member of > `CheckerRegistryData`. It would be a bit more simple (no `Data.` call), and > these belong naturally to there. I don't want to encourage the modification of the data by anyone other then CheckerRegistry. On another note, this would break all plugin code. We haven't shied away from that in the past, but historically speaking, plugins and the unit test files only interacted with CheckerRegistry, and seeing how it is the logic behind checker registration, I think its fitting. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82585/new/ https://reviews.llvm.org/D82585 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
