[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2021-09-23 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. Fully agree. Such an opt-in feature shouldn't break things. I'd be happy to implement it if people think it's a good idea! Would need some hand-holding though, never touched the LLVM codebase before :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2021-09-23 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. I think this should be as simple as having an opt-in "DisableAllAliaseeChecks" option in `.clang-tidy`, that force-disables all the checks that are actually aliases and not proper checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2021-09-23 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. Herald added a subscriber: jeroen.dobbelaere. Hi! I've been following this issue about aliases in clang-tidy through different bug reports and commit attempts. I see that the author abandoned this commit without an explanation. What's the reason? Where can I

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2021-03-23 Thread Martin G via Phabricator via cfe-commits
MartinG added a comment. In D72566#1815917 , @aaron.ballman wrote: > I'm not certain I agree. Many of the aliases have different sets of default > checking options from the primary check. And that's precisely the problem. Aliases should be just that:

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D72566#1816109 , @lebedev.ri wrote: > I think the fact that this is a fourth (?) different incarnation of a patch > trying to solve the same *major* *ugly* problem, it may be an evidence that > perhaps this problem

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. I think the fact that this is a fourth (?) different incarnation of a patch trying to solve the same *major* *ugly* problem, it may be an evidence that perhaps this problem should not be approached from the 'let's hack it together' approach, but with a concrete plan

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-12 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Another potential solution is leave is as is by default. Then emit a warning if a check is ran twice. The warning could be suppressed by passing a option to say what behaviour you want? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-12 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. How does this sound? I'll put in a config option in there to disable the new alias behaviour. Personally I'd say default to the new behaviour. Then if an organisation relies on the old behaviour then can revert back with a command line option or .clang-tidy file edit

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D72566#1815913 , @njames93 wrote: > In D72566#1815904 , @lebedev.ri > wrote: > > > I agree that the current alias situation is not ideal, though i'm not sure > > how much we can

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D72566#1815913 , @njames93 wrote: > In D72566#1815904 , @lebedev.ri > wrote: > > > I agree that the current alias situation is not ideal, though i'm not sure > > how much we can fix

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-12 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D72566#1815904 , @lebedev.ri wrote: > I agree that the current alias situation is not ideal, though i'm not sure > how much we can fix it since it crosses user interface boundary > (i.e. what fixes won't lead to some

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-12 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. I agree that the current alias situation is not ideal, though i'm not sure how much we can fix it since it crosses user interface boundary (i.e. what fixes won't lead to some regressions from the previous expected behavior) To be noted, this will make adding of new

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-12 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. should warning be printed with all the enabled checks, or just the primary name? warning: statement should be inside braces [readability-braces-around-statements] vs warning: statement should be inside braces [readability-braces-around-statements,

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-12 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment. {icon check-circle color=green} Unit tests: pass. 61774 tests passed, 0 failed and 780 were skipped. {icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings

[PATCH] D72566: [clang-tidy] Clang tidy is now alias aware

2020-01-12 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: aaron.ballman, alexfh, JonasToth, hokein. njames93 added projects: clang-tools-extra, clang. Herald added a subscriber: xazax.hun. Clang-tidy has had an on growing issue with blindly adding all checks from a module using the