[PATCH] D116378: [clang-tidy] Disable clang-tidy warnings from system macros

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:23-40
+namespace {
+class VaArgPPCallbacks : public PPCallbacks {
+public:
+  VaArgPPCallbacks(ProTypeVarargCheck *Check) : Check(Check) {}
+
+  void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD,
+                    SourceRange Range, const MacroArgs *Args) override {
----------------
carlosgalvezp wrote:
> salman-javed-nz wrote:
> > Should this be in another patch?
> > 
> > Line 722 in ClangTidyDiagnosticConsumer.cpp makes it so that clang-tidy filters warnings from system macros. This would benefit all checks.
> > 
> > This VaArgPPCallBack change here only benefits `cppcoreguidelines-pro-type-vararg`.
> The change to `ClangTidyDiagnosticConsumer.cpp` broke this check, because it was checking for the use of `vararg` indirectly, checking for the use of `__builtin_vararg`, which was expanded from the system macro `vararg`. This patch updates the code so it checks directly what the rule is about "do not use `vararg`".  I should perhaps have been more clear about that in the commit message :) 
Messed up names above, I meant `va_arg` and `__builtin_va_arg`.


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:46-78
+    "__builtin_isgreater",
+    "__builtin_isgreaterequal",
     "__builtin_isless",
-    "__builtin_islessequal", 
-    "__builtin_islessgreater", 
+    "__builtin_islessequal",
+    "__builtin_islessgreater",
     "__builtin_isunordered",
+    "__builtin_fpclassify",
----------------
carlosgalvezp wrote:
> salman-javed-nz wrote:
> > Formatting changed for code not directly involved in this patch. Should be moved to a separate NFC commit that you don't have to run by us.
> Yes, it's a bit unfortunate, this was an automatic change on save from my editor. Is it really worth it to revert this change though? I will need to disable this feature on my IDE, revert each line manually by adding a space, push, then re-enable the feature. Sounds like a lot of time spent on reverting a change that anyway improves the state of the code.
Fixed.


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

https://reviews.llvm.org/D116378