yawanng added a comment. In https://reviews.llvm.org/D33304#758871, @aaron.ballman wrote:
> 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. There are also two other requirements(not yet implemented). There will be more checks following up. 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
