[PATCH] D45679: [clang-tidy] Add ExprMutationAnalyzer, that analyzes whether an expression is mutated within a statement.

aaron.ballman reopened this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I had to revert due to failing tests. The revert was done in r334606 and this is an example of a failing bot: http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubu

[PATCH] D48100: Append new attributes to the end of an AttributeList.

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. Thank you for working on this odd detail of attributes! Can you also simplify `hasSameOverloadableAttrs()` in ASTReaderDecl.cpp similar to what you did in SemaOverload.cpp (the copy seems spurious)?

[PATCH] D48242: [ASTMatchers] Add support for matching the type of a friend decl.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some minor nits. Comment at: include/clang/ASTMatchers/ASTMatchers.h:2900-2904 +AST_POLYMORPHIC_MATCHER_P_OVERLOAD( +hasType, +AST_POLYM

[PATCH] D47435: Add __builtin_signbit semantic checking

aaron.ballman closed this revision. aaron.ballman added a comment. I've committed in r335048; if @rsmith has concerns, they can be address post commit. https://reviews.llvm.org/D47435 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://l

[PATCH] D48100: Append new attributes to the end of an AttributeList.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you for this! Repository: rC Clang https://reviews.llvm.org/D48100 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D47953: [builtin] Add bitfield support for __builtin_dump_struct

aaron.ballman added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:1250 + if (Info.IsSigned) { +unsigned HighBits = Info.StorageSize - Info.Offset - Info.Size; +if (HighBits) What happens if this overflows due to being < 0? Reposito

[PATCH] D47953: [builtin] Add bitfield support for __builtin_dump_struct

aaron.ballman added reviewers: echristo, rsmith, dblaikie. aaron.ballman added a comment. In https://reviews.llvm.org/D47953#1137375, @paulsemel wrote: > I will for sure add tests @lebedev.ri . Fact is that, for the moment, this is > not working as expected. > This is why I am asking for a bit

[PATCH] D48390: ASan docs: no_sanitize("address") works on globals.

aaron.ballman added a comment. Do you also want to update AttrDocs.td with some similar information? https://reviews.llvm.org/D48390 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48412: [RISCV] Add support for interrupt attribute

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:8966 + +const RISCVInterruptAttr *Attr = FD->getAttr(); +if (!Attr) You can use `const auto *` here instead of repeating the type.

[PATCH] D52859: [clang-query] Add option to print matcher expression

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52859 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D52857: Deprecate 'set output foo' API of clang-query

aaron.ballman added a comment. I'm not all that keen on the idea of deprecating `set output` like this -- that command has been around since forever, and scripts outside of our control are likely to rely on it. However, it seems like you should be able to make `set output` accept a comma-delimi

[PATCH] D52857: Deprecate 'set output foo' API of clang-query

aaron.ballman added a comment. In https://reviews.llvm.org/D52857#1258893, @steveire wrote: > - The scripts will continue to work at least until `set output` is removed, > which is not going to happen soon. Certainly, but given that deprecation implies eventual removal, people are still being

[PATCH] D52857: Deprecate 'set output foo' API of clang-query

aaron.ballman added a comment. In https://reviews.llvm.org/D52857#1259522, @steveire wrote: > > What's more, given that clang-output has no real documentation to speak of, > > how will users even *know* to update their scripts? > > The release notes will tell them. That's not user friendly. >

[PATCH] D52857: Deprecate 'set output foo' API of clang-query

aaron.ballman added a comment. In https://reviews.llvm.org/D52857#1260455, @steveire wrote: > > you have to find the right place to stick the `set dump-output true` in > > order to enable it. > > What do you mean "the right place"? The point from which they want to enable dump outputting.

[PATCH] D52857: Deprecate 'set output foo' API of clang-query

aaron.ballman added a comment. @steveire and I chatted over IRC and I wanted to capture another alternate proposal idea as part of the review (we did not reach agreement on this proposal, however). I've provided it side-by-side with the other proposals for comparison purposes. # Original pro

[PATCH] D52384: [Sema] Fix redeclaration contexts for enumerators in C

aaron.ballman added a comment. Ping https://reviews.llvm.org/D52384 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52840: Include Python binding tests in CMake rules

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I'm not a CMake expert, but when I apply this patch locally, I'm able to run cmake to completion without errors, so LGTM! https://reviews.llvm.org/D52840

[PATCH] D52400: Improve -Wshadow warnings with enumerators

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Committed in r344259 on @erik.pilkington 's LGTM. Self-accepting so I can close the review. https://reviews.llvm.org/D52400 ___ cf

[PATCH] D52301: [clang] Set TypeSourceInfo for vardecl's in addition to type when we can deduce.

aaron.ballman added a reviewer: rsmith. aaron.ballman added a comment. Can you give a bit more background on what problem you're trying to solve with this patch? Comment at: lib/Sema/SemaChecking.cpp:852 - if (RT->getPointeeType().getAddressSpace() != LangAS::opencl_generic

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

aaron.ballman added a comment. In https://reviews.llvm.org/D41648#1253647, @JonasToth wrote: > @aaron.ballman What do you think of the current version of this check? > As migration strategy the option for `CAPS_ONLY` is provided, otherwise the > filtering is provided to silence necessary macros

[PATCH] D52892: [Clang-tidy] readability check to convert numerical constants to std::numeric_limits

aaron.ballman added a comment. I like the thrust of this check, but we already have the readability-magic-numbers check and I think that this functionality would fit naturally there. That check finds numerical constants and recommends turning them into named constants, but for special values it

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! https://reviews.llvm.org/D45050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52400: Improve -Wshadow warnings with enumerators

aaron.ballman added a comment. In https://reviews.llvm.org/D52400#1266307, @sberg wrote: > doesnt this make -Wshadow more aggressive for enumerators than for other > entities? It does, but whether that's an issue with the enumerator shadow diagnosing or with the other entities not diagnosing,

[PATCH] D52400: Improve -Wshadow warnings with enumerators

aaron.ballman added a comment. In https://reviews.llvm.org/D52400#1267499, @sberg wrote: > In https://reviews.llvm.org/D52400#1266341, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D52400#1266307, @sberg wrote: > > > > > > > > > > [...] > > > Then again, this is a case where you don't g

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some minor nits. Comment at: clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp:71 +void MacroUsageCheck::warnMacro(const MacroDirective *MD) { +

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from minor wording nits. Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:31-32 + static constexpr llvm::StringLiteral SkipFirst =

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

aaron.ballman added a comment. In https://reviews.llvm.org/D52771#1263404, @lebedev.ri wrote: > FIXME: `IgnoreClassesWithAllMemberVariablesBeingPublic` needs to be somehow > enabled for cppcoreguidelines check. > I don't know if it is possible, and how. IIRC, the idea is to override `getModul

[PATCH] D52301: [clang] Set TypeSourceInfo for vardecl's in addition to type when we can deduce.

aaron.ballman added a comment. Thank you for the context and explanation! I don't feel like I have enough familiarity with this machinery to be an effective reviewer for signing off on anything. I've commented with some nits, but @rsmith should give the sign-off on this one.

[PATCH] D53025: [clang-tidy] implement new check for const return types.

aaron.ballman added inline comments. Comment at: test/clang-tidy/readability-const-value-return.cpp:174 +int **const * n_multiple_ptr(); +int *const & n_pointer_ref(); I'd like to see some more complex examples, like: ``` int const foo(); int const * const foo();

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

aaron.ballman added a comment. In https://reviews.llvm.org/D52670#1268170, @lebedev.ri wrote: > - Apply minor wording nits. > - For `cert-dcl16-c`, **only** consider `L`, `LL` suffixes, not **anything** > else (not even `llu`). I'll find out about the DCL16-C recommendation, as I suspect the i

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a minor nit. Comment at: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:84 +ClangTidyOptions Options; +auto &Opts = Option

[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.

aaron.ballman added inline comments. Comment at: test/clang-tidy/readability-else-after-return-if-constexpr.cpp:9 +return; + // CHECK-MESSAGES: [[@LINE-2]]:3: warning: + Please add some of the warning text -- any warning will match this. C

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

aaron.ballman added a comment. In https://reviews.llvm.org/D52670#1268372, @lebedev.ri wrote: > In https://reviews.llvm.org/D52670#1268347, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D52670#1268170, @lebedev.ri wrote: > > > > > - Apply minor wording nits. > > > - For `cert-dcl16-c`,

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

aaron.ballman added a comment. In https://reviews.llvm.org/D52670#1268569, @lebedev.ri wrote: > In https://reviews.llvm.org/D52670#1268564, @aaron.ballman wrote: > > > I talked to someone at CERT responsible for maintaining DCL16-C to get > > their opinion on tightening the wording of the rule a

[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.

aaron.ballman added a comment. Hmm, the latest patch only seems to have the changes to the test but not the implementation? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53372 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Thanks for the changes! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D52400: Improve -Wshadow warnings with enumerators

aaron.ballman added a comment. In https://reviews.llvm.org/D52400#1267731, @aaron.ballman wrote: > In https://reviews.llvm.org/D52400#1267499, @sberg wrote: > > > (and in any case, "declaration shadows a variable" sounds wrong when the > > shadowed entity is a class type. thats why I thought som

[PATCH] D53498: Re-word command help for clang-query

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some minor wording nits. Comment at: clang-query/Query.cpp:53 +" print " +"pretty-print bound nodes

[PATCH] D53500: Add 'detailed-ast' output as an alias for 'dump'

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with some fixes to the wording. Comment at: clang-query/Query.cpp:57 +" detailed-ast " +"Detailed AST output for boun

[PATCH] D53501: [clang-query] Refactor Output settings to booleans

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from minor commenting nits. Comment at: clang-query/Query.h:17 #include "llvm/ADT/Optional.h" + #include Spurious newline, or did

[PATCH] D52857: [clang-query] Add non-exclusive output API

aaron.ballman added a comment. In https://reviews.llvm.org/D52857#1272276, @steveire wrote: > I prefer the API from Peter. I think it's a good additional step from where > Aaron and I reached in IRC discussion (this patch currently). > > I can change the patch to use that later if you agree Aaro

[PATCH] D52857: [clang-query] Add non-exclusive output API

aaron.ballman added a comment. In https://reviews.llvm.org/D52857#1272420, @steveire wrote: > Yep, that's the suggestion. That will result in commands such as: > > - `enable output detailed-ast` > - `disable output detailed-ast` > - `set output detailed-ast` > - `enable output diag` > - `disable

[PATCH] D53591: Support the __gnu__ scoped attribute token

aaron.ballman created this revision. aaron.ballman added reviewers: rsmith, dblaikie, echristo. Herald added a subscriber: krytarowski. GCC is currently considering a patch to accept `__gnu__` as a scoped attribute namespace that aliases to `gnu`. This is useful for libstdc++ so that they don't

[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check

aaron.ballman added inline comments. Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:29-31 +llvm::APSInt ApInt(/*BitWidth=*/64, /*isUnsigned=*/false); +ApInt = static_cast(Value); +return ApInt; I believe you can do `return APSInt::get(sta

[PATCH] D52384: [Sema] Fix redeclaration contexts for enumerators in C

aaron.ballman closed this revision. aaron.ballman added a comment. Committed (with improved comment) in r345073. https://reviews.llvm.org/D52384 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D53591: Support the __gnu__ scoped attribute token

aaron.ballman closed this revision. aaron.ballman added a comment. In https://reviews.llvm.org/D53591#1273689, @rsmith wrote: > Looks good. Thanks! I've commit in r345132. > We should provide some reserved-name way of naming clang attributes too, but > I'm not sure what would be best (`__clan

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

aaron.ballman added inline comments. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:56-57 -void NarrowingConversionsCheck::check(const MatchFinder::MatchResult &Result) { - if (const auto *Op = Result.Nodes.getNodeAs("op")) { -if (Op->getBeginLoc().

[PATCH] D53700: Support _Clang as a scoped attribute identifier

aaron.ballman created this revision. aaron.ballman added reviewers: rsmith, echristo, dblaikie. Herald added a subscriber: krytarowski. Currently, we only accept `clang` as the scoped attribute identifier for double square bracket attributes provided by Clang, but this has the potential to confl

[PATCH] D53025: [clang-tidy] implement new check for const return types.

aaron.ballman added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 +diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler optimizations") +<< Def->getRetur

[PATCH] D53621: Support for groups of attributes in #pragma clang attribute

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some small nits. Comment at: clang/docs/LanguageExtensions.rst:2666 - #pragma clang attribute push(__attribute__((annotate("custom"))), apply

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

aaron.ballman added a comment. Mostly minor nits that clean up wording and comments. Comment at: clang-tidy/readability/IsolateDeclarationCheck.cpp:52 + const LangOptions &LangOpts) { + assert(Indirections >= 0 && "Indirections must

[PATCH] D53025: [clang-tidy] implement new check for const return types.

aaron.ballman added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 +diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler optimizations") +<< Def->getRetur

[PATCH] D53025: [clang-tidy] implement new check for const return types.

aaron.ballman added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 +diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler optimizations") +<< Def->getRetur

[PATCH] D53781: [ASTMatchers] add a matcher for static locals

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Do you need someone to commit on your behalf? Repository: rC Clang https://reviews.llvm.org/D53781 ___ cfe-commits mailin

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

aaron.ballman added inline comments. Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:19 +// of casting an integer value that is out of range +//===--===// + ZaMaZaN4iK wro

[PATCH] D52857: [clang-query] Add non-exclusive output API

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with some minor fixes. At some point (not necessarily as part of this patch), you should also update `docs\ReleaseNotes.rst` to broadly list the new features you've been add

[PATCH] D53814: Allow the analyzer to output to a SARIF file

aaron.ballman created this revision. aaron.ballman added reviewers: dcoughlin, zaks.anna. Herald added subscribers: dkrupp, donat.nagy, Szelethus, a.sidorin, mgorny. Herald added a reviewer: george.karpenkov. SARIF (https://github.com/oasis-tcs/sarif-spec) is a new draft standard interchange form

[PATCH] D53781: [ASTMatchers] add a matcher for static locals

aaron.ballman closed this revision. aaron.ballman added a comment. Committed in r345502, thank you for the patch! Repository: rC Clang https://reviews.llvm.org/D53781 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

aaron.ballman added a comment. 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

[PATCH] D53814: Allow the analyzer to output to a SARIF file

aaron.ballman added inline comments. Comment at: Analysis/diagnostics/sarif-check.py:22 +passes = 0 +with open(testfile) as testfh: +lineno = 0 george.karpenkov wrote: > Wow, this is super neat! > Since you are quite active in LLVM community, would you think

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

aaron.ballman added a comment. In https://reviews.llvm.org/D53817#1279053, @JonasToth wrote: > > I personally haven't yet seen this check firing on *predefined* macros. > > I did specifically check the compiler defined macros I could see in the AST > and they were not diagnosed from the beginnin

[PATCH] D53814: Allow the analyzer to output to a SARIF file

aaron.ballman updated this revision to Diff 171545. aaron.ballman marked 3 inline comments as done. aaron.ballman added a comment. Updated based on initial review feedback, and added more context to the patch. https://reviews.llvm.org/D53814 Files: Analysis/diagnostics/sarif-check.py Analys

[PATCH] D53814: Allow the analyzer to output to a SARIF file

aaron.ballman marked 9 inline comments as done. aaron.ballman added inline comments. Comment at: Analysis/diagnostics/sarif-diagnostics-taint-test.c:18 +// Test the basics for sanity. +// CHECK: sarifLog['version'].startswith("2.0.0") +// CHECK: sarifLog['runs'][0]['tool']['fullN

[PATCH] D53814: Allow the analyzer to output to a SARIF file

aaron.ballman updated this revision to Diff 171685. aaron.ballman added a comment. Updated based on review feedback -- removed the `Sarif` path generation scheme as it isn't currently needed, and replaced a FIXME with a better comment. Testing remains an open question, however. https://reviews

[PATCH] D53025: [clang-tidy] implement new check for const return types.

aaron.ballman added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 +diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler optimizations") +<< Def->getRetur

[PATCH] D53814: Allow the analyzer to output to a SARIF file

aaron.ballman updated this revision to Diff 171712. aaron.ballman added a comment. Switched to using diff for testing. https://reviews.llvm.org/D53814 Files: Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif Analysis/diagnostics/sarif-diagnostics-taint-test.c

[PATCH] D53814: Allow the analyzer to output to a SARIF file

aaron.ballman added inline comments. Comment at: Analysis/diagnostics/sarif-diagnostics-taint-test.c:18 +// Test the basics for sanity. +// CHECK: sarifLog['version'].startswith("2.0.0") +// CHECK: sarifLog['runs'][0]['tool']['fullName'] == "clang static analyzer" ---

[PATCH] D53817: [clang-tidy] cppcoreguidelines-macro-usage: print macro names

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! 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/c

[PATCH] D53025: [clang-tidy] implement new check for const return types.

aaron.ballman added inline comments. Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 +diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler optimizations") +<< Def->getRetur

[PATCH] D53814: Allow the analyzer to output to a SARIF file

aaron.ballman closed this revision. aaron.ballman added a comment. In https://reviews.llvm.org/D53814#1280581, @george.karpenkov wrote: > I much prefer this version. > We've had the same problem with diffing plist output. > One thing we have learned is using a FileCheck was definitely a bad ide

[PATCH] D53025: [clang-tidy] implement new check for const return types.

aaron.ballman added a comment. I think this is getting really close! One question: would it make more sense to name this `readability-const-type-return` or `readability-const-return-type` instead? It's not that the functions return a const *value* that's the issue, it's that the declared return

[PATCH] D53025: [clang-tidy] implement new check for const return types.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. This LGTM, thank you for working on this check! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 ___ cfe-commi

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: docs/clang-tidy/checks/readability-isolate-declaration.rst:45 + +Global variables and member variables are excluded. + JonasT

[PATCH] D55492: Implement Attr dumping in terms of visitors

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: lib/AST/ASTDumper.cpp:89 // Utilities -void dumpType(QualType T) { NodeDumper.dumpType(T); } void dumpTypeAsChild(QualType T); -

[PATCH] D56555: Add Attribute to define nonlazy objc classes

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. The patch is missing SemaObjC tests that ensure the attribute only appertains to the expected subjects, accepts no args, etc. Comment at: clang/inclu

[PATCH] D55483: Introduce the callback attribute and emit !callback metadata

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:1204 + VariadicUnsignedArgument<"PayloadIndices">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; jdoerfert wrote: > jdoerfert wrote: > >

[PATCH] D55491: Implement TemplateArgument dumping in terms of Visitor

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from nits that aren't complete yet, LGTM. Comment at: include/clang/AST/TemplateArgumentVisitor.h:24 +/// A simple visitor class that helps create templat

[PATCH] D56639: NFC: Move Type Visit implementation to TextNodeDumper

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some minor nits. Comment at: lib/AST/ASTDumper.cpp:434 +NodeDumper.Visit(T); if (!T) { return; Elide braces.

[PATCH] D56640: NFC: Canonicalize handling of TypeLocInfo

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56640/new/ https://reviews.llvm.org/D56640 ___

[PATCH] D56641: NFC: Move dumping of QualType node to TextNodeDumper

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56641/new/ https://reviews.llvm.org/D56641 ___

[PATCH] D56643: NFC: Move Decl node handling to TextNodeDumper

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some minor nits. Comment at: lib/AST/ASTDumper.cpp:518 +NodeDumper.Visit(D); if (!D) { return; Elide braces.

[PATCH] D56642: NFC: Move dump of type nodes to NodeDumper

aaron.ballman added a comment. Mostly looks good (a few minor nits), but I did have a design question about whether we should prefer calling a `Visit` method for type hierarchies instead of reimplementing the functionality. Comment at: lib/AST/ASTDumper.cpp:145 void Visi

[PATCH] D56642: NFC: Move dump of type nodes to NodeDumper

aaron.ballman added inline comments. Comment at: lib/AST/ASTDumper.cpp:145 void VisitVariableArrayType(const VariableArrayType *T) { - OS << " "; - NodeDumper.dumpSourceRange(T->getBracketsRange()); - VisitArrayType(T); + dumpTypeAsChild(T->getElementType

[PATCH] D55483: Introduce the callback attribute and emit !callback metadata

aaron.ballman added inline comments. Comment at: include/clang/Basic/AttrDocs.td:3780 +represent an implicit "this" pointer in class methods. If there is no implicit +"this" pointer it shall not be referenced. The index '-1', or the name "?", +represents an unknown callback calle

[PATCH] D55394: Re-order type param children of ObjC nodes

aaron.ballman added a reviewer: rjmccall. aaron.ballman added a subscriber: rjmccall. aaron.ballman added a comment. Adding @rjmccall as an Obj-C expert to see if he has opinions on the output changes. Also, pinging @rsmith in case he'd like to weigh in. I think the change in behavior here is re

[PATCH] D56555: Add Attribute to define nonlazy objc classes

aaron.ballman added a comment. It's still missing the tests in SemaObjC -- you can use clang\test\SemaObjC\attr-root-class.m as an example of what I'm talking about. Comment at: clang/include/clang/Basic/Attr.td:1697 + let LangOpts = [ObjC] + let Documentation = [Undocumente

[PATCH] D55394: Re-order type param children of ObjC nodes

aaron.ballman added a comment. In D55394#1356837 , @rjmccall wrote: > I don't really have an opinion about this, sorry. Do you think ObjC users will find the new output to be harder to read or less understandable than the old output? Repository: rC

[PATCH] D56642: NFC: Move dump of type nodes to NodeDumper

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from `auto` nits. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56642/new/ https://reviews.llvm.org/D56642

[PATCH] D56707: Implement CXXCtorInitializer dump in terms of Visitor

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56707/new/ https://reviews.llvm.org/D56707 ___

[PATCH] D56708: NFC: Implement OMPClause dump in terms of visitors

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a nit. Comment at: lib/AST/ASTDumper.cpp:1462 const OMPExecutableDirective *Node) { - for (auto *C : Node->clauses()) { -dumpChild([=]

[PATCH] D56709: Implement BlockDecl::Capture dump in terms of visitors

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56709/new/ https://reviews.llvm.org/D56709 ___

[PATCH] D55394: Re-order type param children of ObjC nodes

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D55394#1356941 , @rjmccall wrote: > This is the AST dumper. This is not a user feature. Clang developers are users, too. However, th

[PATCH] D55083: Re-arrange content in FunctionDecl dump

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some nit cleanup. Comment at: lib/AST/ASTDumper.cpp:648 + + if (const CXXConstructorDecl *C = dyn_cast(D)) +for (CXXConstructorDecl::init_c

[PATCH] D56555: Add Attribute to define nonlazy objc classes

aaron.ballman added a comment. In terms of implementation, the attribute code looks pretty close (just a few nits). However, someone more familiar with Obj-C should chime in with whether the attribute semantics make sense and whether the documentation will be sufficiently clear for an Objective

[PATCH] D56532: [clang-tidy] Add the abseil-duration-conversion-cast check

aaron.ballman added inline comments. Comment at: clang-tidy/abseil/DurationComparisonCheck.cpp:47-48 // if nothing needs to be done. - if (!IsValidMacro(Result, Binop->getLHS()) || - !IsValidMacro(Result, Binop->getRHS())) + if (!isNotInMacro(Result, Binop->getLHS()) ||

[PATCH] D56657: [clang-tidy] bugprone-string-constructor: Catch string from nullptr.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a nit and a test case request. Comment at: clang-tidy/bugprone/StringConstructorCheck.cpp:141 } + } else if (const Expr* Ptr = Result.Node

[PATCH] D56663: [MSP430] Improve support of 'interrupt' attribute

aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:277-280 +def warn_msp430_interrupt_attribute : Warning< + "MSP430 'interrupt' attribute only applies to functions that have " + "%select{no parameters|a 'void' return type}0">, + InG

[PATCH] D55483: Introduce the callback attribute and emit !callback metadata

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some minor nits. However, please hold off on committing this until *after* the 8.0 branch happens. While I don't expect there to be issues with this patch, it doe

[PATCH] D55447: [Sema] Fix Modified Type in address_space AttributedType

aaron.ballman added inline comments. Comment at: clang/tools/libclang/CXType.cpp:132 + if (!(TU->ParsingOptions & CXTranslationUnit_IncludeAttributedTypes) && + ATT->getAttrKind() != attr::AddressSpace) { return MakeCXType(ATT->getModifiedType(), TU); -

<    1   2   3   4   5   6   7   8   9   10   >