[PATCH] D26511: [clang-tidy] Rename modernize-use-default to modernize-use-equals-default

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.


================
Comment at: clang-tidy/modernize/ModernizeTidyModule.cpp:58
         "modernize-use-bool-literals");
-    CheckFactories.registerCheck<UseDefaultCheck>("modernize-use-default");
+    CheckFactories.registerCheck<UseEqualsDefaultCheck>("modernize-use-equals-default");
     CheckFactories.registerCheck<UseEmplaceCheck>("modernize-use-emplace");
----------------
malcolm.parsons wrote:
> alexfh wrote:
> > aaron.ballman wrote:
> > > malcolm.parsons wrote:
> > > > aaron.ballman wrote:
> > > > > What do we want to do, if anything, for people who have scripts using the old name? Do we want to keep the old name as an alias to the new name for some period of time?
> > > > An alias helps if the check was enabled by name, but not if it was disabled by name.
> > > > If the alias is temporary, would you want a deprecation warning?
> > > > I wouldn't want to warn about `-checks=modernize*`, but maybe warning for `-checks=modernize-use-default` would be useful.
> > > > An alias helps if the check was enabled by name, but not if it was disabled by name.
> > > 
> > > Oye, this is true and unfortunate.
> > > 
> > > > If the alias is temporary, would you want a deprecation warning?
> > > > I wouldn't want to warn about -checks=modernize*, but maybe warning for -checks=modernize-use-default would be useful.
> > > 
> > > I think a deprecation warning would be a helpful feature, but not required. I do agree that I would not want a warning for wildcard matches.
> > > 
> > > I would also be fine if we simply had the documentation for `modernize-use-default` forward to the documentation for `modernize-use-equals-default` and put a note in there about the old name being deprecated and leave in an alias to the old name.
> > > 
> > > To be complete, I would also be fine if we remove the old name as in this patch. I am mostly thinking about what default policy we want to have when this situation arises. FWIW, the check was exposed under this name around Oct 2015, so it's been in the wild for over a year, and in a public release.
> > I'd personally prefer to leave the old documentation file with a redirect and a note about the renaming.  Similar to how we treat aliases. WDYT?
> If it has a redirect then add_new_check.py will add it to list.rst using the same wording as an alias.
> Is that what you want?
> Should it be an alias?
I don't care about leaving the old name for the check (probably, better to remove it right away), but leaving the old documentation page with a proper redirect seems useful as a means to resolve users' confusion. I wouldn't call it "alias" in this case though, and including it to the list doesn't seem to be useful (so the document will need to be marked orphan, IIUC).


https://reviews.llvm.org/D26511