aaron.ballman added a comment. In https://reviews.llvm.org/D33304#758808, @alexfh wrote:
> In https://reviews.llvm.org/D33304#758713, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D33304#758624, @srhines wrote: > > > > > In https://reviews.llvm.org/D33304#758621, @joerg wrote: > > > > > > > I find the use of "must" at the very least inappropriate. If there was > > > > no use case for not including it, it wouldn't be an option. There is > > > > also nothing really Android-specific here beside maybe the open64 mess. > > > > > > > > > On Android, we are requiring this flag. That is why this is part of a new > > > category of Android-specific tidy rules. If you think this belongs more > > > generally in a different category for tidy, can you suggest somewhere > > > else to put it? We didn't want to impose these restrictions for platforms > > > that might not want to be so strict. Also, as with any static analysis, > > > there is the possibility that the original code author intended to > > > "break" the rules, but that is what NOLINT is for. > > > > > > I'm not keen on putting this in an Android module either, as it's not > > really Android-specific behavior. For instance, this is also part of a > > recommended compliant solution for CERT FIO22-C. > > > I think AOSP has enough specific guidelines and requirements to warrant a > separate module (especially, if Android folks have plans to contribute more > than one check into it ;). As for this check, if the relevant requirements of > CERT and Android are really identical, we could make an alias for the check > in the CERT module (or vice versa). Another possibility that comes to mind is > to create a new "posix" module specifically for things related to POSIX APIs > (or "unix", if we want it to be slightly broader). WDYT? If there are plans to add more checks, then yes. However, I think I'd prefer to see at least 2-3 checks in the work (or have some commitment for doing at least that many checks) before we add a module for it. I mostly worry about adding a single check and then nothing else. (No matter what module name we're talking about, btw.) I'd be fine with android, posix, or unix, depending on the nature of the checks. >> I think this should probably be in misc, or the bugprone module that @alexfh >> has mentioned previously. > > I'm strongly against bloating "misc" module. It's more or less the last > resort, a place for checks we have found no better place for. The proposed > "bugprone" module is an attempt to address this by pulling out a large part > of "misc" to a place with more definite name and purpose. However, in the > case of this check we seem to have enough good (and more specific) > alternatives to default to "misc" or even "bugprone". I was hesitant to suggest misc, but I was hoping to avoid adding a module with a single check under it and no commitment for further ones. Repository: rL LLVM https://reviews.llvm.org/D33304 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
