[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + lebedev.r

[PATCH] D52281: [clang-tidy] Add modernize check to use std::invoke in generic code

aaron.ballman added a comment. The review says you abandoned it -- was that accidental? Comment at: clang-tidy/modernize/ReplaceGenericFunctorCallCheck.cpp:72 + +const Expr *Obj = BinOp->getLHS(); +const std::string ObjName = boga95 wrote: > I found an

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + martong w

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + martong w

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + aaron.bal

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewCheck.cpp:51 + // The alignment used by default 'operator new' (in bits). + const unsigned DefaultAlignment = Context.getTargetInfo().getNewAlign(); + balazske

[PATCH] D66856: [Sema] Suppress -Wformat diagnostics for bool types when printed using %hhd

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you for this! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66856/new/ https://reviews.llvm.org/D66856 ___ cfe

[PATCH] D63640: [clang] Improve Serialization/Imporing of APValues

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/APValue.h:512 } - void setVector(const APValue *E, unsigned N) { + void ReserveVector(unsigned N) { assert(isVector() && "Invalid accessor"); `reserveVector` per naming conventions

[PATCH] D67737: [clang-tidy] Add check for classes missing -hash ⚠️

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/objc/MissingHashCheck.cpp:56 + const auto *ID = Result.Nodes.getNodeAs("impl"); + diag(ID->getLocation(), "%0 implements -isEqual: without implementing -hash") + << ID; Do you th

[PATCH] D67825: [AST] Extract Decl::printQualifier helper from Decl::printQualifiedName

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added inline comments. Comment at: clang/include/clang/AST/Decl.h:313 + /// Returns only qualifier from printQualifiedName, including the :: at the + /// end. E.g. This doesn't return anything, so I

[PATCH] D67825: [AST] Extract Decl::printNestedNameSpecifier helper from Decl::printQualifiedName

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Comment at: clang/include/clang/AST/Decl.h:317-318 + ///this function returns "A::B::". + void printQualifier(raw_ostream &OS) const; + void printQualifier(raw_ostream &OS, const PrintingPolicy &P

[PATCH] D67737: [clang-tidy] Add check for classes missing -hash ⚠️

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang-tools-extra/clang-tidy/objc/MissingHashCheck.cpp:56 + const auto *ID = Result.Nodes.getNodeAs("impl"); + diag(ID->getLocation(), "%0 i

[PATCH] D63640: [clang] Improve Serialization/Imporing of APValues

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/APValue.h:537-540 +private: + void setLValueEmptyPath(LValueBase B, const CharUnits &O, unsigned Size, + bool OnePastTheEnd, bool IsNullPtr); + LValuePathEntry *getLValuePathPtr();

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64671/new/ https://reviews.llvm.org/D64671 ___ cfe-commits mailing lis

[PATCH] D63640: [clang] Improve Serialization/Imporing/Dumping of APValues

aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: clang/include/clang/AST/APValue.h:618 } + const CXXRecordDecl **getMemberPointerPathPtr(); }; Tyker wrote: > aaron.ballman wrote: > > We're horribly inconsistent

[PATCH] D68028: [clang] Add no_builtin attribute

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. Thank you for working on this! It looks like you're missing all of the sema tests that check that the attribute only appertains to functions, accepts the proper kind of arguments, etc. Comment at:

[PATCH] D63640: [clang] Improve Serialization/Imporing/Dumping of APValues

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/APValue.h:613 + /// in place as after importing/deserializating then. + void reserveVector(unsigned N) { +assert(isVector() && "Invalid accessor"); `ReserveVector` C

[PATCH] D67919: [Diagnostics] Warn if enumeration type mismatch in conditional expression

aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:11264 + + const EnumType *LHSEnumType = LHSStrippedType->getAs(); + if (!LHSEnumType) `const auto *` (and below as well) since the type is spelled out in the initialization. =

[PATCH] D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour

aaron.ballman added inline comments. Comment at: clang/docs/ReleaseNotes.rst:63 +* As per C++ and C Standards (C++: ``[expr.add]``; C17: 6.5.6p8), applying + non-zero offset to ``nullptr`` (or making non-``nullptr`` a ``nullptr``, rsmith wrote: > In C, even ad

[PATCH] D68028: [clang] Add no_builtin attribute

aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1092 + // Wildcard is a super set of all builtins, we keep only this one. + if (FunctionNames.count(Wildcard) > 0) { +FunctionNames.clear();

[PATCH] D67919: [Diagnostics] Warn if enumeration type mismatch in conditional expression

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a commenting request. Comment at: test/Sema/warn-conditional-emum-types-mismatch.c:19 + +int get_flag_anon_enum(int cond) { + return cond ? A :

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

aaron.ballman added inline comments. Herald added a subscriber: hiraditya. Comment at: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp:68-70 +// MEM +CheckFactories.registerCheck( +"cert-mem57-cpp"); The `MEM` section should come before `MSC`

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

aaron.ballman added a comment. Do you need someone to commit this on your behalf (sorry for not asking that question sooner)? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64671/new/ https://reviews.llvm.org/D64671 ___ cfe-commits mailing

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

aaron.ballman added a comment. In D64671#1690058 , @jpakkane wrote: > In D64671#1688626 , @aaron.ballman > wrote: > > > Do you need someone to commit this on your behalf (sorry for not asking > > that question soo

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

aaron.ballman closed this revision. aaron.ballman added a comment. In D64671#1691538 , @jpakkane wrote: > Rebased against latest Git master. Didn't see any rebase conflicts, though... Oh! I just noticed you made the patch against the monorepo which I'm n

[PATCH] D68410: [AttrDocs] document always_inline

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4400 +Hint that inline substitution should be attempted when optimizations are +disabled. Does not guarantee that inline substitution actually occurs. +}]; It may make sense to

[PATCH] D67159: [clang] New __attribute__((__clang_arm_mve_alias)).

aaron.ballman marked 2 inline comments as done. aaron.ballman added inline comments. Comment at: clang/lib/AST/Decl.cpp:3082 +static bool ArmMveAliasValid(unsigned BuiltinID, StringRef AliasName) { + // This will be filled in by Tablegen which isn't written yet + return false;

[PATCH] D67775: [Sema] Split out -Wformat-type-confusion from -Wformat-pedantic

aaron.ballman added a comment. In D67775#1691999 , @erik.pilkington wrote: > Ping! Sorry for the delayed review -- thank you for working on clearing this up! Comment at: clang/include/clang/AST/FormatString.h:259 +NoMatch = 0, +

[PATCH] D68028: [clang] Add no_builtin attribute

aaron.ballman added a subscriber: rsmith. aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1072 +NoBuiltinAttr * +Sema::mergeNoBuiltinAttr(Sema &S, Decl *D, const AttributeCommonInfo &CI, + llvm::ArrayRef FunctionNames) { --

[PATCH] D68377: [Builtins] Teach Clang about memccpy

aaron.ballman added a comment. Test cases? Comment at: include/clang/Basic/Builtins.def:483 BUILTIN(__builtin_memcpy, "v*v*vC*z", "nF") +BUILTIN(__builtin_memccpy, "v*v*vC*iz", "nF") BUILTIN(__builtin_memmove, "v*v*vC*z", "nF") GCC doesn't seem to have `__bui

[PATCH] D68377: [Builtins] Teach Clang about memccpy

aaron.ballman added a comment. In D68377#1693490 , @xbolva00 wrote: > It is not very obvious for me what kind of test should be done here. I > would be grateful if you could show me an example in tree. I was thinking of something in CodeGen, like Code

[PATCH] D45403: Make [[maybe_unused]] work with static data members

aaron.ballman closed this revision. aaron.ballman added a comment. I've commit in r329904, thank you for the patch! https://reviews.llvm.org/D45403 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinf

[PATCH] D44155: Fix support for try_acquire_capability

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I think this code is ready to go in -- accepting my own revision so that I can close. If @delesley spots any issues, they can be address post commit. https://reviews.llvm.org/D4

[PATCH] D44155: Fix support for try_acquire_capability

aaron.ballman closed this revision. aaron.ballman added a comment. Committed the fix in r329930. https://reviews.llvm.org/D44155 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45601: Warn on bool* to bool conversion

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. I'd also be interested to see the number of false positives and true positives when run over some large code bases (both C and C++). For instance, I would imagine code like this to be somewhat common and very reasonabl

[PATCH] D45615: [builtins] __builtin_dump_struct : added more types format

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. In https://reviews.llvm.org/D45615#1066975, @paulsemel wrote: > In https://reviews.llvm.org/D45615#1066973, @lebedev.ri wrote: > > > Tests? > > > I can do this. But curr

[PATCH] D35181: Defer addition of keywords to identifier table when loading AST

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a few small nits. Comment at: include/clang/Basic/IdentifierTable.h:471 + /// \brief Create the identifier table. + IdentifierTable(Identifier

[PATCH] D45665: [builtin-dump-struct] printf formats generation testing

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

[PATCH] D35181: Defer addition of keywords to identifier table when loading AST

aaron.ballman closed this revision. aaron.ballman added a comment. Committed in r330159. Thank you for the patch! https://reviews.llvm.org/D35181 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

[PATCH] D45665: [builtin-dump-struct] printf formats generation testing

aaron.ballman closed this revision. aaron.ballman added a comment. I've committed in r330185, thank you for the patch! Repository: rC Clang https://reviews.llvm.org/D45665 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.

[PATCH] D45615: [builtins] __builtin_dump_struct : added more types format

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! I will go ahead and commit on your behalf. Btw, if you're thinking of making additional patches, now might be a good time to ask for commit privileges (https://llvm.org/doc

[PATCH] D45615: [builtins] __builtin_dump_struct : added more types format

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

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

aaron.ballman added a comment. In https://reviews.llvm.org/D41648#1047799, @JonasToth wrote: > I checked several codebases and implemented a little helper script to see > which kind of macros exist. Then I added some filter regex as configuration > and tried again, here are the results: > > For

[PATCH] D45601: Warn on bool* to bool conversion

aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:2623 +QualType QT = Pointer->getType()->getPointeeType(); +if (!QT.isNull() && QT->isBooleanType()) + // Warn on bool* to bool conversion.

[PATCH] D45702: [clang-tidy] Add a new check, readability-redundant-data-call, that finds and removes redundant calls to .data().

aaron.ballman added a comment. Have you run this over any large code bases to see whether the check triggers in practice? Comment at: docs/clang-tidy/checks/list.rst:95 fuchsia-default-arguments + fuchsia-header-anon-namespaces (redirects to google-build-namespaces)

[PATCH] D45835: Add new driver mode for dumping compiler options

aaron.ballman created this revision. aaron.ballman added a reviewer: rsmith. Herald added a subscriber: cryptoad. There are situations where an out-of-tree user may need to know information about what compiler options are used and what features/extensions may be available for a given invocation

[PATCH] D45835: Add new driver mode for dumping compiler options

aaron.ballman added a comment. In https://reviews.llvm.org/D45835#1072557, @efriedma wrote: > The list of features/extensions seems okay; it's information which is already > available through a stable interface (specifically, clang -E). JSON also > seems fine as a representation. > > Including

[PATCH] D45835: Add new driver mode for dumping compiler options

aaron.ballman added a comment. In https://reviews.llvm.org/D45835#1073883, @efriedma wrote: > > If any of those options we care about wind up being changed, there's a good > > chance we may need to change something on our end anyway, so breaking us is > > actually useful. > > I'm not sure I fol

[PATCH] D45891: [clang-tidy] Improve bugprone-unused-return-value check

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/D45891 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45944: Disallow pointers to const in __sync_fetch_and_xxx

aaron.ballman created this revision. aaron.ballman added reviewers: rsmith, dblaikie. The following code is currently accepted without a diagnostic when it should be prohibited: void f(const int *ptr) { __sync_fetch_and_add(ptr, 1); } NB: the above code is diagnosed by GCC and ICC. Howe

[PATCH] D44932: [CodeComplete] Fix completion in the middle of ident in ctor lists.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, modulo a tiny commenting nit. Comment at: lib/Lex/Lexer.cpp:1667 + // Note that code completion token is not added as a separate character + //

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

aaron.ballman added inline comments. Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:226 +DiagnosticBuilder &Diag) { + if (getLangOpts().CPlusPlus11) { +StringRef NewFuncName = (Name[0] != 'w') ? "strncpy_s" : "wcsncpy_s"; Charusso wrote

[PATCH] D45944: Disallow pointers to const in __sync_fetch_and_xxx

aaron.ballman updated this revision to Diff 143526. aaron.ballman added a comment. Updated with more context. https://reviews.llvm.org/D45944 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp test/Sema/builtins.c Index: test/Sema/builtins.c

[PATCH] D45944: Disallow pointers to const in __sync_fetch_and_xxx

aaron.ballman added a comment. In https://reviews.llvm.org/D45944#1075055, @lebedev.ri wrote: > Please always upload all patches with full context (`-U9`) I'm not keen on uploading patches with that much context. For instance, it disables syntax highlighting in Phab for many of our files.

[PATCH] D45932: [clang-tidy][modernize-raw-string-literal] Don't replace upper ASCII with raw literals

aaron.ballman added inline comments. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:70-72 + if (Bytes.find_if([](char C) { +return static_cast(C) > 0x7Fu; + }) != StringRef::npos) I think you can use `isASCII()` from CharInfo.h rather th

[PATCH] D45865: [Sema] Emit -Warray-bounds for multiple levels of subscript expressions.

aaron.ballman added a reviewer: rsmith. aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:11284 + case Stmt::MemberExprClass: { +const MemberExpr *ME = cast(expr); +expr = ME->getBase(); Can use `const auto *` here in

[PATCH] D45865: [Sema] Emit -Warray-bounds for multiple levels of subscript expressions.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D45865 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen or wcslen

aaron.ballman added inline comments. Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:226 +DiagnosticBuilder &Diag) { + if (getLangOpts().CPlusPlus11) { +StringRef NewFuncName = (Name[0] != 'w') ? "strncpy_s" : "wcsncpy_s"; Charusso wrote

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

aaron.ballman added inline comments. Comment at: clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp:77 +DiagnosticMessage = +"function like macro used; consider a (constexpr) template function"; + if (Info->isVariadic()) function like -> function-like

[PATCH] D45932: [clang-tidy][modernize-raw-string-literal] Don't replace upper ASCII with raw literals

aaron.ballman added inline comments. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:71 + // supported by specific code pages only. + if (Bytes.find_if_not(isASCII) != StringRef::npos) +return false; I am starting to think that this functionality

[PATCH] D45932: [clang-tidy][modernize-raw-string-literal] Don't replace upper ASCII with raw literals

aaron.ballman added inline comments. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:71 + // supported by specific code pages only. + if (Bytes.find_if_not(isASCII) != StringRef::npos) +return false; zinovy.nis wrote: > aaron.ballman wrote: > > I

[PATCH] D45932: [clang-tidy][modernize-raw-string-literal] Don't replace upper ASCII with raw literals

aaron.ballman added inline comments. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:71 + // supported by specific code pages only. + if (Bytes.find_if_not(isASCII) != StringRef::npos) +return false; zinovy.nis wrote: > aaron.ballman wrote: > > z

[PATCH] D45865: [Sema] Emit -Warray-bounds for multiple levels of subscript expressions.

aaron.ballman closed this revision. aaron.ballman added a comment. Committed in r330759, thank you for the patch! https://reviews.llvm.org/D45865 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

[PATCH] D46033: add check for long double for __builtin_dump_struct

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

[PATCH] D43750: Allow writing calling convention attributes on function types

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

[PATCH] D46027: [clang-tidy] Fix PR35824

aaron.ballman added a comment. > Which solution do you prefer? If I understand the issue properly: both. :-) Having the AST track information that's been folded away is still useful -- some users are using the AST for purposes other than codegen, and the fact that a construct has been folded a

[PATCH] D46112: Allow _Atomic to be specified on incomplete types

aaron.ballman created this revision. aaron.ballman added a reviewer: rsmith. The C standard does not prohibit the _Atomic specifier on incomplete types, which turns out to be sometimes useful. This addresses PR37237. Note that C++ still requires checking if the type is complete in order to sup

[PATCH] D46233: [ASTMatchers] Overload isConstexpr for ifStmts

aaron.ballman accepted this revision. aaron.ballman added a comment. Aside from the issue @alexfh raised, this LGTM. Repository: rC Clang https://reviews.llvm.org/D46233 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.or

[PATCH] D46300: [Clang] Implement function attribute no_stack_protector.

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. Missing Sema tests that the attribute only appertains to functions, accepts no arguments, etc. Comment at: include/clang/Basic/Attr.td:1494 +def NoSt

[PATCH] D44143: [clang-tidy] Create properly seeded random generator check

aaron.ballman added a comment. This is getting closer, but it seems there are still unresolved comments from reviewers. Are you planning to resolve those as well? https://reviews.llvm.org/D44143 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D45702: [clang-tidy] Add a new check, readability-redundant-data-call, that finds and removes redundant calls to .data().

aaron.ballman added a comment. > Have you run this over any large code bases to see whether the check triggers > in practice? I'm still curious about this, btw. Comment at: clang-tidy/readability/RedundantDataCallCheck.cpp:45 + anyOf(TypesMatcher, pointerType

[PATCH] D45702: [clang-tidy] Add a new check, readability-redundant-data-call, that finds and removes redundant calls to .data().

aaron.ballman added inline comments. Comment at: clang-tidy/readability/RedundantDataCallCheck.cpp:31 +void RedundantDataCallCheck::registerMatchers(MatchFinder *Finder) { + using namespace ast_matchers; + No need to register any of these matchers unless in C++

[PATCH] D46317: [clang-tidy] New check bugprone-map-subscript-operator-lookup

aaron.ballman added a comment. I am generally in favor of this but would be curious to see how the check reacts to real world code bases. Facebook's usage experience is compelling, but I'm also wondering how bad the false-positive rate is on existing code. Can you try running the check over LLV

[PATCH] D45944: Disallow pointers to const in __sync_fetch_and_xxx

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

[PATCH] D43750: Allow writing calling convention attributes on function types

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

[PATCH] D45679: [clang-tidy] Add a helper function isModified, that checks whether an expression is modified within a statement.

aaron.ballman added inline comments. Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:42 + const auto Memoized = Results.find(Exp); + if (Memoized != Results.end()) { +return Memoized->second; Can elide the braces. Comment at: clang-

[PATCH] D45702: [clang-tidy] Add a new check, readability-redundant-data-call, that finds and removes redundant calls to .data().

aaron.ballman added a comment. In https://reviews.llvm.org/D45702#1085890, @shuaiwang wrote: > In https://reviews.llvm.org/D45702#1085224, @aaron.ballman wrote: > > > > Have you run this over any large code bases to see whether the check > > > triggers in practice? > > > > I'm still curious abou

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

aaron.ballman added a comment. In https://reviews.llvm.org/D46159#1086472, @lebedev.ri wrote: > In https://reviews.llvm.org/D46159#1086463, @pfultz2 wrote: > > > > As Devin says (and as we discussed this with Anna Zaks) alpha checkers > > > are still in development, so we don't want to expose th

[PATCH] D43750: Allow writing calling convention attributes on function types

aaron.ballman closed this revision. aaron.ballman added a comment. In https://reviews.llvm.org/D43750#1085780, @rsmith wrote: > I'm really sad about this; C++11 attributes were supposed to fix the > undisciplined "do what I mean" behavior of GNU attributes. But you're right, > these really are

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

aaron.ballman added a comment. In https://reviews.llvm.org/D46159#1086553, @pfultz2 wrote: > > I think the premise is a bit off the mark. It's not that these are not for > > the common user -- it's that they're simply not ready for users at all. > > Then why was it merged into clang in the first

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

aaron.ballman added a comment. In https://reviews.llvm.org/D46159#1086627, @alexfh wrote: > In https://reviews.llvm.org/D46159#1086493, @aaron.ballman wrote: > > > I think the premise is a bit off the mark. It's not that these are not for > > the common user -- it's that they're simply not ready

[PATCH] D45835: Add new driver mode for dumping compiler options

aaron.ballman updated this revision to Diff 145077. aaron.ballman added a comment. Updated to remove the contentious parts, though I still hope to add those back in. https://reviews.llvm.org/D45835 Files: include/clang/Basic/Features.def include/clang/Driver/CC1Options.td include/clang/F

[PATCH] D45463: [AST] Print correct tag decl for tag specifier

aaron.ballman added inline comments. Comment at: lib/AST/DeclPrinter.cpp:180-181 if (isFirst) { - if(TD) -SubPolicy.IncludeTagDefinition = true; + if (TD) +SubPolicy.TagSpecifierAs = TD; SubPolicy.SuppressSpecifiers = false;

[PATCH] D45093: [AST] Fix -ast-print for _Bool when have diagnostics

aaron.ballman added a comment. This approach generally looks good to me, but I'd like @rsmith's opinion on whether we should be trying to make -ast-print have good source fidelity or not. I was under the impression we wanted -ast-print to faithfully reproduce code at least as a low priority des

[PATCH] D45702: [clang-tidy] Add a new check, readability-redundant-data-call, that finds and removes redundant calls to .data().

aaron.ballman added a comment. In https://reviews.llvm.org/D45702#1086802, @shuaiwang wrote: > In https://reviews.llvm.org/D45702#1086250, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D45702#1085890, @shuaiwang wrote: > > > > > In https://reviews.llvm.org/D45702#1085224, @aaron.ballman

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

aaron.ballman added inline comments. Comment at: clang-tidy/utils/ExprMutationAnalyzer.cpp:88 + +const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) { + // LHS of any assignment operators. JonasToth wrote: > shuaiwang wrote: > > aaron.ballman w

[PATCH] D45944: Disallow pointers to const in __sync_fetch_and_xxx

aaron.ballman added reviewers: rjmccall, efriedma. aaron.ballman added a comment. Adding a few more potential reviewers. https://reviews.llvm.org/D45944 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/li

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

aaron.ballman added a comment. Test cases? Repository: rC Clang https://reviews.llvm.org/D46441 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

aaron.ballman added a comment. In https://reviews.llvm.org/D45679#1088696, @shuaiwang wrote: > I've reverted the handling of DeclRefExpr to volatile after rethinking about > it. > > The purpose of this analyzer is to find whether the given Expr is mutated > within a scope, but doesn't really ca

[PATCH] D45944: Disallow pointers to const in __sync_fetch_and_xxx

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Committed in r331598, with an additional test case. (Accepting officially on behalf of @rjmccall so that I can close the review.) https://reviews.llvm.org/D45944

[PATCH] D46112: Allow _Atomic to be specified on incomplete types

aaron.ballman updated this revision to Diff 145409. aaron.ballman added a comment. Hopefully address the review comments. https://reviews.llvm.org/D46112 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp lib/Sema/SemaType.cpp test/CodeGen/c11atomics.c test/Se

[PATCH] D46112: Allow _Atomic to be specified on incomplete types

aaron.ballman added inline comments. Comment at: lib/Sema/SemaType.cpp:8022 +else if (LangOpts.CPlusPlus && isCompleteType(Loc, T) && + !T.isTriviallyCopyableType(Context)) // Some other non-trivially-copyable type (probably a C++ class) rs

[PATCH] D45702: [clang-tidy] Add a new check, readability-redundant-data-call, that finds and removes redundant calls to .data().

aaron.ballman added inline comments. Comment at: clang-tidy/readability/RedundantDataCallCheck.cpp:45 + anyOf(TypesMatcher, pointerType(pointee(TypesMatcher)), + callee(namedDecl(hasName("data" + .bind("call", ---

[PATCH] D46535: Correct warning on Float->Integer conversions.

aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:9432 if (IsLiteral) { +// Conversion of a floating point value to a non-bool integer where the +// integral part cannot be represented by the integer type is undefined. floating

[PATCH] D46300: [Clang] Implement function attribute no_stack_protector.

aaron.ballman edited reviewers, added: probinson; removed: void. aaron.ballman added a subscriber: probinson. aaron.ballman added a comment. Adding in @probinson as he originally added the `optnone` attribute. Paul, do you recall why you opted (haha, pun totally intended) to implement `optnone`

[PATCH] D46535: Correct warning on Float->Integer conversions.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D46535 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

[PATCH] D46159: [clang-tidy] Add a flag to enable alpha checkers

aaron.ballman accepted this revision. aaron.ballman added a comment. Aside from a minor commenting nit, also LGTM. Comment at: clang-tidy/tool/ClangTidyMain.cpp:195 +/// This option allows enabling alpha checkers from the static analyzer, that +/// are experimental. This opti

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

aaron.ballman added a comment. In https://reviews.llvm.org/D42933#1090268, @jfb wrote: > I was just looking at this, and I think @arphaman's patch is pretty much the > right approach (with 2 suggested fixes below). > > I don't think the code we're currently warning on is broken: a user code has

[PATCH] D44143: [clang-tidy] Create properly seeded random generator check

aaron.ballman added inline comments. Comment at: clang-tidy/cert/CERTTidyModule.cpp:44 "cert-dcl54-cpp"); -CheckFactories.registerCheck( -"cert-dcl58-cpp"); + CheckFactories.registerCheck("cert-dcl58-cpp"); CheckFactories.registerCheck( -

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

aaron.ballman added inline comments. Herald added a subscriber: the_o. Comment at: lib/Sema/SemaDeclAttr.cpp:5280 + // Check the attribute arguments. + if (AL.getNumArgs() > 1) { +S.Diag(AL.getLoc(), diag::err_attribute_too_many_arguments) apazos wrote: > a

<    13   14   15   16   17   18   19   20   21   22   >