malcolm.parsons added a comment. In https://reviews.llvm.org/D25659#585234, @aaron.ballman wrote:
> In https://reviews.llvm.org/D25659#585154, @malcolm.parsons wrote: > > > In https://reviews.llvm.org/D25659#584986, @aaron.ballman wrote: > > > > > (1) I think that the aliases and the originals should be listed with > > > -list-checks, because these are names under which the checks may be run > > > (and sometimes the name may even imply different semantics for the check). > > > > > > I've always found it odd that -list-checks only lists the enabled checks. > > I think a list of enabled checks shouldn't report aliases that will not be > > used. > > > Okay, that's a good point, I forgot that -list-checks only lists the enabled > checks, not all of them. With that in mind, here's what I would expect to see: > > (0) -checks=* -list-checks > (1) -checks=-*, cert-* -list-checks > (2) -checks=*, -cert-* -list checks > > In (0), I would expect all checks to be listed, even aliases. Aliases may > have semantic distinctions from other checks and the user said "run > everything." However, for aliases which have no semantic distinction, there's > no point to running the check twice even if the user said to run everything. > Leaving aliases in or out results in an unsatisfying situation either way, > unless we know whether an alias supplies different option values. > In (1), I would expect only the cert checks to be listed, even if they alias > to something outside of the cert module. It would be strange to use those > options and see something outside of the cert module listed, even if it had > no semantic distinction, just because of cert rules that are an alias to > something else. > In (2), I would expect to see every check *except* cert checks listed, > regardless of aliasing in or out of the cert module. Similar rationale as (1). > > If I understand the output from this patch, only (0) is modified to not list > the alias name because aliases are never enabled with -checks=* in this > patch, but (1) and (2) do not change behavior? 0) doesn't list aliases. I'd like a -list-all-checks option. 1. lists all the cert checks, as they aren't aliases for other cert checks. 2. doesn't list any cert checks, as none are enabled. > Does -dump-config only list enabled checks as well, or does it list the > configuration for all checks? If it's all checks, then the alias should be > present because it may be configured differently. If it's only enabled > checks, then I agree that it should only list what is being used. The config is only dumped for the enabled checks. > I don't think that preferring the original check is bad so long as we get the > semantic options correct; the ability for an alias to be defined as "that > check with these options being set this way" is important. That might mean preferring the alias. But there might be more than one. https://reviews.llvm.org/D25659 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
