[PATCH] D73098: [clang-tidy] readability-identifier-naming disregards parameters restrictions on main like functions

[Nathan James via Phabricator via cfe-commits]

njames93 marked an inline comment as done.
njames93 added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:383
+  } else {
+    static llvm::Regex Matcher("(^((W[Mm])|(wm))ain([_A-Z]|$))|([a-z0-9_]W[Mm]"
+                               "ain([_A-Z]|$))|(_wmain(_|$))",
----------------
aaron.ballman wrote:
> njames93 wrote:
> > Any thoughts on whether I should detect WMain or just Wmain. WMain looks more pleasing but using CamelCase rules it should likely be Wmain
> I think this needs to be controlled by an option if we want to keep it, because you can still have names like `terminateMain(int exitCode, char *msgs[]);` At the end of the day, no matter what regex we come up with, we can probably pick an identifier that subverts the check. Also, the name `main` might have different meanings if the function is a member function or a member of a namespace (or, alternatively, it may actually relate to the main entrypoint). Ultimately, this seems too much like guessing at user intent, which is why I think it should be an option (and probably off by default).
> 
> That said, if we're searching for main-like names, I think we need to find names like wmain, Wmain, wMain, WMain optionally with a prefix or suffix (perhaps with underscores). I am less certain how to treat member functions, but suspect the reasonable thing to do is treat them as being potentially main-like regardless of whether they're static or not, but the function still needs to be publicly available rather than private or protected (those can't be main-like, unless someone friends main... which is another twist).
Its current behaviour now is to just flag main, but there is an option IgnoreMainLikeFunctions(defaults to off) that if turned on will look for functions with the right sig that have the word main, Main, WMain, Wmain or wmain in there. I'll add in the public access though, that was an oversight


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73098