lebedev.ri added a comment. In https://reviews.llvm.org/D53817#1278970, @aaron.ballman wrote:
> In https://reviews.llvm.org/D53817#1278933, @lebedev.ri wrote: > > > In https://reviews.llvm.org/D53817#1278916, @JonasToth wrote: > > > > > How does this patch change the behaviour for macros without location? > > > > > > It doesn't. > > > > > They will be diagnosed at the beginning of the TU? > > > > > > > > > It is probably better to ignore these kind of macros, as there is no way > > > around them (compile time configuration can only be done with macros?!) > > > and therefore warnings for theses macros are false positives. Using > > > `constexpr` constructs does not help either. > > > > I agree that it might be good idea to add an *option* to not diagnose such > > macros. > > But i think it is not the spirit of the check/rule to ignore them > > completely [by default], > > because those macros are still horrible, and can affect all kinds of > > things, > > just like the 'normal' macros. > > > I'm not certain an option makes sense in practice. Predefined macros and > macros that come from the command line are a different beast from macros > defined by the developer -- I don't think we should be diagnosing those cases > as they're generally outside of programmer control. Predefined macros are > entirely outside of the user's control. Macros from the command line are most > often going to be controlling build toggles, but there is no reason to assume > the user can control those (like 3rd party packages) or that they can be > replaced with an alternative like `constexpr`. I personally haven't yet seen this check firing on *predefined* macros. The only problem i have are these command-line-based macros. They *may* be outside of programmer's control, but if one has access to the buildsystem (as in, not *just* to the source code to be compiled, but to the build system definitions that say *how* to build it), then one can easily add such command-line-based macros. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53817 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
