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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
