[PATCH] D50055: Update the coding standard about NFC changes and whitespace

aaron.ballman closed this revision. aaron.ballman marked 2 inline comments as done. aaron.ballman added a comment. Thank you for the review and discussion -- I've commit in r339455. https://reviews.llvm.org/D50055 ___ cfe-commits mailing list cfe-co

[PATCH] D49885: Thread safety analysis: Allow relockable scopes

aaron.ballman closed this revision. aaron.ballman added a comment. Committed in r339456; if @delesley has concerns with the commit, they can be addressed post-commit. Thank you for the patch! Repository: rC Clang https://reviews.llvm.org/D49885

[PATCH] D50526: Model type attributes as regular Attrs

aaron.ballman added a comment. Thank you for working on this -- the approach you've taken looks good, and I mostly have little nits here and there. I think this is a great refactoring overall though -- much needed! Comment at: include/clang/AST/Type.h:1856 + + /// Was this t

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

aaron.ballman requested changes to this revision. aaron.ballman added inline comments. This revision now requires changes to proceed. Comment at: lib/Sema/SemaChecking.cpp:10424 + // We don't want to warn for system macro. + S.SourceMgr.isInSystemMacro(E->getOperatorLoc

[PATCH] D49511: [Sema/Attribute] Check for noderef attribute

aaron.ballman added inline comments. Comment at: lib/Sema/SemaExpr.cpp:14251-14253 +if (const auto *Ptr = dyn_cast(Ty)) + Inner = Ptr->getPointeeType(); +else if (const auto *Arr = dyn_cast(Ty)) I don't think this strips off sugar from the type, so I

[PATCH] D49511: [Sema/Attribute] Check for noderef attribute

aaron.ballman added a comment. Oops, I hit "Send" too soon. I was going to say that you should also keep an eye on https://reviews.llvm.org/D50526 because that may impact this patch (or vice versa if this one lands first). Repository: rC Clang https://reviews.llvm.org/D49511

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:10424 + // We don't want to warn for system macro. + S.SourceMgr.isInSystemMacro(E->getOperatorLoc())) +// warn about dropping FP rank. nickdesaulniers wrote: > aaron.ballman

[PATCH] D50606: [ASTMatchers] Add matchers unresolvedMemberExpr, cxxDependentScopeMemberExpr

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, but please split out the unrelated changes into their own commit. Comment at: docs/LibASTMatchersReference.html:1168-1173 +Matcher

[PATCH] D50605: [ASTMatchers] Let hasAnyArgument also support CXXUnresolvedConstructExpr

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. Please be sure to regenerate the AST matcher documentation as well, as I'd expect to see some documentation changes from this. Comment at: include/cl

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Alex has had plenty of time to respond, so I'm fine handling any concerns he has post-commit. Do you need me to commit this on your behalf? Repository: rCTE Clang Tools Extra

[PATCH] D50580: [clang-tidy] Abseil: no namespace check

aaron.ballman added inline comments. Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:23 + + Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"), + this); I think this needs a `not(isExpansionInSystemHeader())` in there

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

aaron.ballman added inline comments. Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:24 + + // TODO(hugoeg): refactor matcher to be configurable or just match on any internal access from outside the enclosing namespace. + JonasToth wrote: > Nit: This co

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman closed this revision. aaron.ballman added a comment. Thank you for the patch and great discussion! I've commit in r339516. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49114 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

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

[PATCH] D50526: Model type attributes as regular Attrs

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Thank you for this fantastic work! Comment at: include/clang/Basic/Attr.td:1510 + let Spellings = [Keyword<"__unsafe_unretained">]; + let Documentation

[PATCH] D50214: Add inherited attributes before parsed attributes.

aaron.ballman added subscribers: rsmith, echristo, dlj, nicholas. aaron.ballman edited reviewers, added: rsmith; removed: nicholas, dlj, echristo. aaron.ballman added a comment. In general, I think this is the right way to go. I've added @rsmith to the reviewers because I'm curious what his thoug

[PATCH] D49910: [clang-tidy] Recognize [[clang::reinitializes]] attribute in bugprone-use-after-move

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! An attribute really is the right way to go for this sort of thing, and it sounds like you've got a base of users looking for the functionality. Repository: rCTE Clang To

[PATCH] D50605: [ASTMatchers] Let hasAnyArgument also support CXXUnresolvedConstructExpr

aaron.ballman added inline comments. Comment at: include/clang/AST/ExprCXX.h:3436 using const_arg_iterator = const Expr* const *; + using arg_const_range = llvm::iterator_range; shuaiwang wrote: > aaron.ballman wrote: > > Please name this `const_arg_range`

[PATCH] D50605: [ASTMatchers] Let hasAnyArgument also support CXXUnresolvedConstructExpr

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

[PATCH] D50580: [clang-tidy] Abseil: no namespace check

aaron.ballman added inline comments. Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:23 + + Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"), + this); hokein wrote: > aaron.ballman wrote: > > I think this needs a `n

[PATCH] D50647: [Sema] fix -Wfloat-conversion test case.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, though you don't need review for fixing bots. Repository: rC Clang https://reviews.llvm.org/D50647 ___ cfe-commits mailing

[PATCH] D49511: [Sema/Attribute] Check for noderef attribute

aaron.ballman added inline comments. Comment at: lib/Sema/SemaExpr.cpp:14249 + +if (Sema::TypeHasNoDeref(Inner)) + DeclRef = E; The sugar was stripped off at the pointer level, but not at the pointee level. e.g., ``` typedef int (bobble); typedef bobble

[PATCH] D49511: [Sema/Attribute] Check for noderef attribute

aaron.ballman added inline comments. Comment at: lib/Sema/SemaExpr.cpp:14249 + +if (Sema::TypeHasNoDeref(Inner)) + DeclRef = E; leonardchan wrote: > aaron.ballman wrote: > > The sugar was stripped off at the pointer level, but not at the pointee > > lev

[PATCH] D50214: Add inherited attributes before parsed attributes.

aaron.ballman added inline comments. Comment at: lib/AST/DeclBase.cpp:854-859 + auto I = Attrs.begin(), E = Attrs.end(); + for (; I != E; ++I) { +if (!(*I)->isInherited()) + break; + } + Attrs.insert(I, A); Meinersbur wrote: > aaron.ballman wrote: >

[PATCH] D49511: [Sema/Attribute] Check for noderef attribute

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. A few more minor nits to be cleared up, but otherwise LGTM. You should wait for @rsmith to sign off before committing in case he has further comments, however.

[PATCH] D50994: Add a new flag and attributes to control static destructor registration

aaron.ballman added a comment. Some minor nits that can be handled post-commit. Comment at: lib/Sema/SemaDeclAttr.cpp:5921 +static void handleDestroyAttr(Sema &S, Decl *D, const ParsedAttr &A) { + if (!isa(D) || !cast(D)->hasGlobalStorage()) { +S.Diag(D->getLocation(), dia

[PATCH] D43778: [clang-tidy] Adding RestrictIncludes check to Fuchsia module

aaron.ballman added inline comments. Comment at: clang-tidy/fuchsia/FuchsiaTidyModule.cpp:41 +CheckFactories.registerCheck( +"fuchsia-restrict-includes"); CheckFactories.registerCheck( I think this should be named `fuchsia-restrict-system-include

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

aaron.ballman added a reviewer: rjmccall. aaron.ballman added a subscriber: rjmccall. aaron.ballman added a comment. In https://reviews.llvm.org/D42933#1091502, @jfb wrote: > In https://reviews.llvm.org/D42933#1091234, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D42933#1090268, @jfb w

[PATCH] D43778: [clang-tidy] Adding RestrictIncludes check to Fuchsia module

aaron.ballman added inline comments. Comment at: clang-tidy/fuchsia/RestrictSystemIncludesCheck.cpp:67 + llvm::SmallDenseMap IncludeDirectives; + llvm::StringMap IsSystem; + Rather than use this `StringMap`, can you change `InclusionDirective()` to filter out

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

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:1494 +def NoStackProtector : InheritableAttr { + let Spellings = [GCC<"no_stack_protector">]; + let Subjects = SubjectList<[Function]>; rnk wrote: > aaron.ballman wrote: > > manojgupta

[PATCH] D46614: [clang] Adding CharacteristicKind to PPCallbacks::InclusionDirective

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with a small nit. Comment at: unittests/Lex/PPCallbacksTest.cpp:53 +this->Imported = Imported; +this->FileType = FileType; } Ca

[PATCH] D46614: [clang] Adding CharacteristicKind to PPCallbacks::InclusionDirective

aaron.ballman added inline comments. Comment at: unittests/Lex/PPCallbacksTest.cpp:142-143 // the InclusionDirective callback. - CharSourceRange InclusionDirectiveFilenameRange(const char* SourceText, - const char* HeaderPath, bool SystemHeader) { + InclusionDirectiveC

[PATCH] D46614: [clang] Adding CharacteristicKind to PPCallbacks::InclusionDirective

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

[PATCH] D46615: [tools] Updating PPCallbacks::InclusionDirective calls

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from a minor nit, LGTM (no need for more review, you can fix the nit and commit). Comment at: clang-move/ClangMove.cpp:135 + con

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

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

[PATCH] D43778: [clang-tidy] Adding RestrictIncludes check to Fuchsia module

aaron.ballman added inline comments. Comment at: docs/clang-tidy/checks/fuchsia-restrict-system-includes.rst:32 + A string containing a semi-colon separated list of allowed include filenames. + The default is an empty string, which allows all includes. julie

[PATCH] D46614: [clang] Adding CharacteristicKind to PPCallbacks::InclusionDirective

aaron.ballman added inline comments. Comment at: unittests/Lex/PPCallbacksTest.cpp:140 + std::unique_ptr getPreprocessor(const char *SourceText, +const char *HeaderPath, This function appears to be unused?

[PATCH] D46614: [clang] Adding CharacteristicKind to PPCallbacks::InclusionDirective

aaron.ballman added inline comments. Comment at: unittests/Lex/PPCallbacksTest.cpp:155-160 +std::unique_ptr PP = llvm::make_unique( +std::make_shared(), Diags, LangOpts, SourceMgr, +PCMCache, HeaderInfo, ModLoader, +/*IILookup =*/nullptr, +/*Ow

[PATCH] D46614: [clang] Adding CharacteristicKind to PPCallbacks::InclusionDirective

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

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

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

[PATCH] D43778: [clang-tidy] Adding RestrictIncludes check to Fuchsia module

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Thank you for adding the glob feature, I think that will be very powerful in practice. Aside from a minor nit, this LGTM! Comment at: clang-tidy/fuchsia/Restri

[PATCH] D44480: [Sema] Don't skip function bodies with 'auto' without trailing return type

aaron.ballman added a comment. Expounding on my concerns a bit -- I'm worried about all the other places calling `isUndeducedType()` and whether they're also at risk and thus a better fix is to change `isUndeducedType()` to pay attention to the language mode. Comment at: lib/

[PATCH] D43778: [clang-tidy] Adding RestrictIncludes check to Fuchsia module

aaron.ballman added inline comments. Comment at: clang-tidy/fuchsia/RestrictSystemIncludesCheck.h:35 + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + bool contains(StringRef FileName) { +return AllowedIncludesGlobList.contains(FileName); j

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

aaron.ballman updated this revision to Diff 146472. aaron.ballman added a comment. Addresses 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/Sema/atomic-ty

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

aaron.ballman updated this revision to Diff 146474. aaron.ballman added a comment. Modifications based on review feedback. https://reviews.llvm.org/D46112 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/AST/Type.cpp lib/Sema/SemaChecking.cpp lib/Sema/SemaType.cpp test/CodeGen/c1

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

aaron.ballman added a comment. In https://reviews.llvm.org/D46112#1091981, @efriedma wrote: > I think the request was that we check that a type is trivially copyable when > we perform an atomic operation? I don't see the code for that anywhere. Sorry about that -- I didn't notice that GNU was

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

aaron.ballman updated this revision to Diff 146524. aaron.ballman added a comment. Look through atomic types when checking type completeness during template instantiation. https://reviews.llvm.org/D46112 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/AST/Type.cpp lib/Sema/SemaChec

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

aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: test/SemaCXX/atomic-type.cpp:5 template struct atomic { - _Atomic(T) value; + _Atomic(T) value; // expected-error {{field has incomplete type '_Atomic(user::inner)'}} --

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

aaron.ballman updated this revision to Diff 146529. aaron.ballman marked 3 inline comments as done. aaron.ballman added a comment. Updated based on review feedback. https://reviews.llvm.org/D45835 Files: include/clang/Basic/Features.def include/clang/Driver/CC1Options.td include/clang/Fro

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

aaron.ballman added inline comments. Comment at: lib/Frontend/FrontendActions.cpp:779 + { +std::string Str; +#define FEATURE(Name, Predicate) \ lebedev.ri wrote: > This is likely to do an allocation for each fea

[PATCH] D45702: [clang-tidy] Add a new check, readability-simplify-subscript-expr, that simplifies subscript expressions.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with a few minor nits to be fixed. Comment at: clang-tidy/readability/SimplifySubscriptExprCheck.cpp:53-54 + const auto *Call = Result.Nodes.getNodeAs("ca

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

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

[PATCH] D46639: [CodeComplete] Provide completion in decls even for incomplete types

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

[PATCH] D46805: If some platforms do not support an attribute, we should exclude the platform

aaron.ballman added a comment. Thank you for working on this patch -- be sure to add tests that ensure the attribute is properly prohibited on the expected targets. Comment at: include/clang/Basic/Attr.td:299-300 list ObjectFormats; + // It indicates that a certain attribu

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

aaron.ballman updated this revision to Diff 146594. aaron.ballman marked an inline comment as done. aaron.ballman added a comment. Updated based on review comments. https://reviews.llvm.org/D46112 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/AST/Type.cpp lib/Sema/SemaChecking.cpp

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

aaron.ballman marked 8 inline comments as done. aaron.ballman added inline comments. Comment at: lib/Sema/SemaType.cpp:7604-7608 + // When instantiating a class template and reaching an atomic type, the check + // for type completeness should occur on the underlying type and no

[PATCH] D46894: [Attr] Don't print implicit attributes

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

[PATCH] D46905: [Attr] Don't print fake MSInheritance argument

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

[PATCH] D46835: [ASTImporter] Do not try to remove invisible Decls from DeclContext

aaron.ballman added a comment. Can you roll back the changes in MatchVerifier.h? Phab is telling me that the only changes are to whitespace. Comment at: lib/AST/DeclBase.cpp:1353 +static bool shouldBeHidden(NamedDecl *D); + Rather than make a hanging forward

[PATCH] D46439: [Sema] Fix incorrect packed aligned structure layout

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

[PATCH] D45702: [clang-tidy] Add a new check, readability-simplify-subscript-expr, that simplifies subscript expressions.

aaron.ballman added a comment. In https://reviews.llvm.org/D45702#1097294, @shuaiwang wrote: > Addressed review comments. This patch was approved; do you need someone to commit this for you? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45702 _

[PATCH] D45702: [clang-tidy] Add a new check, readability-simplify-subscript-expr, that simplifies subscript expressions.

aaron.ballman closed this revision. aaron.ballman added a comment. Committed in r332519, thank you for the patch! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45702 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://li

[PATCH] D46835: [ASTImporter] Do not try to remove invisible Decls from DeclContext

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/D46835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[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:21-25 + for (const auto *Init : Node.capture_inits()) { +if (Init == E) + return true; + } + return false; This can be replaced with `return llvm::find(Node.cap

[PATCH] D38455: [clang-tidy] new cppcoreguidelines-narrowing-conversions check.

aaron.ballman added inline comments. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:35 + hasSourceExpression(IsFloatExpr), + unless(hasParent(castExpr( +

[PATCH] D38455: [clang-tidy] new cppcoreguidelines-narrowing-conversions check.

aaron.ballman added inline comments. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:35 + hasSourceExpression(IsFloatExpr), + unless(hasParent(castExpr( +

[PATCH] D38455: [clang-tidy] new cppcoreguidelines-narrowing-conversions check.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I think this generally LGTM. You should wait a bit to see if @alexfh has any other concerns. Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck

[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works

aaron.ballman added a reviewer: rsmith. aaron.ballman added a comment. Adding Richard as he has more background in this area. In general, I think it's okay - if we invalidate too often, we do lose some performance, but the correctness should still be there. I'll let Richard provide the final sig

[PATCH] D44539: [Sema][Objective-C] Add check to warn when property of objc type has assign attribute

aaron.ballman added a comment. In https://reviews.llvm.org/D44539#1106152, @rjmccall wrote: > This was approved by the Objective-C language group as a default-off warning. We usually do not expose new default-off warnings because experience shows that they rarely ever get enabled by users. If

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

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

[PATCH] D47122: [clang-tidy] SimplifyBoolenExpr doesn't add parens if unary negotiation is of ExprWithCleanups type

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, with a small nit. Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:78 E = E->IgnoreImpCasts(); + if (auto *EC = dyn_cast(E)) +E = E

[PATCH] D44539: [Sema][Objective-C] Add check to warn when property of objc type has assign attribute

aaron.ballman added a comment. In https://reviews.llvm.org/D44539#1106802, @rjmccall wrote: > In https://reviews.llvm.org/D44539#1106443, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D44539#1106152, @rjmccall wrote: > > > > > This was approved by the Objective-C language group as a def

[PATCH] D47157: Warning for framework headers using double quote includes

aaron.ballman added a comment. > The warning is off by default. We typically do not add off-by-default warnings because experience has shown the rarely get enabled in practice. Can you explain a bit more about why this one is off by default? Repository: rC Clang https://reviews.llvm.org/D4

[PATCH] D47200: [Sema] Add tests for weak functions

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

[PATCH] D46805: If some platforms do not support an attribute, we should exclude the platform

aaron.ballman added a comment. @rsmith -- do the object file formats listed look correct to you? Comment at: include/clang/Basic/Attr.td:322 +def TargetSupportsAlias : TargetSpec { + let ObjectFormats = ["COFF", "ELF", "Wasm"]; +} Did you verify that Wasm supp

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

aaron.ballman added a comment. This is relaxing `-Wformat` and making users instead write `-Wformat-pedantic` to get the strong guarantees, which is the opposite of what I thought the consensus was. Have I misunderstood something? Comment at: include/clang/Analysis/Analyses/F

[PATCH] D47157: Warning for framework headers using double quote includes

aaron.ballman added a comment. In https://reviews.llvm.org/D47157#1107210, @bruno wrote: > > See also PR22165. > > Nice, seems related to this indeed. Are you aware of any development along > those lines in clang-tidy? We would like this to be part of clang and be used > as part of the normal c

[PATCH] D45686: [Driver] Clean up tmp files when deleting Compilation objects

aaron.ballman added a comment. In https://reviews.llvm.org/D45686#1109295, @dstenb wrote: > Ping. > > We have added a lit reproducer for this now in clang-tools-extra: > https://reviews.llvm.org/D47251. The above should be rolled into this patch as the test case verifying the behavioral chang

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

aaron.ballman added a comment. In https://reviews.llvm.org/D47290#1113412, @jfb wrote: > In https://reviews.llvm.org/D47290#1113365, @aaron.ballman wrote: > > > This is relaxing `-Wformat` and making users instead write > > `-Wformat-pedantic` to get the strong guarantees, which is the opposite

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

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

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

aaron.ballman updated this revision to Diff 148753. aaron.ballman marked 4 inline comments as done. aaron.ballman added a comment. Corrected some of the review comments. https://reviews.llvm.org/D46112 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/AST/Type.cpp lib/Sema/SemaCheckin

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

aaron.ballman added a comment. In https://reviews.llvm.org/D46112#1098243, @rsmith wrote: > In https://reviews.llvm.org/D46112#1096893, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D46112#1091981, @efriedma wrote: > > > > > Also needs some test coverage for atomic operations which aren

[PATCH] D47435: Add __builtin_signbit semantic checking

aaron.ballman created this revision. aaron.ballman added reviewers: rengolin, chatur01, rsmith. aaron.ballman added inline comments. Comment at: test/Sema/builtins.c:228 // expected-note {{change size argument to be the size of the destinatio

[PATCH] D47435: Add __builtin_signbit semantic checking

aaron.ballman added inline comments. Comment at: test/Sema/builtins.c:228 // expected-note {{change size argument to be the size of the destination}} - + __builtin___strlcat_chk(buf, b, sizeof(b), _

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

aaron.ballman marked 3 inline comments as done. aaron.ballman added inline comments. Comment at: test/Frontend/compiler-options-dump.cpp:3 +// RUN: %clang_cc1 -compiler-options-dump -std=c++17 %s -o - | FileCheck %s --check-prefix=CXX17 +// RUN: %clang_cc1 -compiler-options-dump

[PATCH] D38209: [Sema] Correct nothrow inherited by noexcept

aaron.ballman added a comment. I'm not certain we have the semantics of `__declspec(nothrow)` exactly correct -- a simple test shows that `__attribute__((nothrow))`, `__declspec(nothrow)`, and `noexcept(true)` are subtly different. When I run this with MSVC 2017, the terminate handler is not ca

[PATCH] D38209: [Sema] Correct nothrow inherited by noexcept

aaron.ballman added a reviewer: rsmith. aaron.ballman added a comment. In https://reviews.llvm.org/D38209#880553, @STL_MSFT wrote: > > do you think `__declspec(nothrow)` calling the terminate handler in Clang > > is a bug? > > It's certainly a behavior difference with potential performance impac

[PATCH] D37436: Initial implementation of C attributes (WG14 N2137)

aaron.ballman added a comment. In https://reviews.llvm.org/D37436#871117, @aaron.ballman wrote: > Updated based on review feedback. > > - Rename CAttributes to DoubleSquareBracketAttributes > - Added Objective-C test cases to demonstrate no regressed behavior (based on > a use-case pointed out d

[PATCH] D38270: [clang] Add getUnsignedPointerDiffType method

aaron.ballman added a comment. > The code for printf diagnostics + new tests are supposed to be added by a > separate diff. I think you should combine that diff and this one into a single patch, otherwise this patch is untestable. Comment at: include/clang/AST/ASTContext.h:1

[PATCH] D38202: Add Documentation to attribute-nothrow. Additionally, limit to functions.

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

[PATCH] D38209: [Sema] Correct nothrow inherited by noexcept

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. This is somewhat complicated in that noexcept, throw(), and __attribute__(())/__declspec no throw are all synonyms for one another except for the type system effect differences b

[PATCH] D38205: [Sema] Warn on attribute nothrow conflicting with language specifiers

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D38205#882737, @erichkeane wrote: > changes that were suggested by @aaron.ballman > > It DOES warn on the template correctly, however I'm not thrilled

[PATCH] D38270: [clang] Add getUnsignedPointerDiffType method

aaron.ballman added inline comments. Comment at: test/FixIt/format.m:256 + // see the comment in PrintfSpecifier::fixType in PrintfFormatString.cpp. +} + Can you add some positive tests that show "%t" and "%tu" working properly without warnings? Repository:

[PATCH] D38342: [C++] Parse (sub) postfix expression after boolean literal

aaron.ballman added inline comments. Comment at: test/CXX/expr/expr.post/expr.sub/p1.cpp:3-6 +void pr34273() { + char Normal = "clang"[true]; // expected-no-diagnostics + char Special = true["clang"]; // expected-no-diagnostics +} I think this test should be ad

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

aaron.ballman added a comment. In https://reviews.llvm.org/D36836#883214, @lebedev.ri wrote: > @alexfh please specify, is that enough or not? I think it's sufficient, but we should put that information somewhere pertinent rather than just the review. In the past, we've put something into LICEN

[PATCH] D38284: [clang-tidy] Fix google-readability-namespace-comments handling of C++17 nested namespaces

aaron.ballman added inline comments. Comment at: clang-tidy/readability/NamespaceCommentCheck.cpp:86 + SourceLocation LBracketLocation = ND->getLocation(); + auto NestedNamespaceBegin = LBracketLocation; + Do not use `auto` here (the type is not spelled out in

[PATCH] D38342: [C++] Parse (sub) postfix expression after boolean literal

aaron.ballman added a comment. In https://reviews.llvm.org/D38342#883872, @Rakete wrote: > - Moved test to test/CXX/ > > Do I actually need to -verify the test if no diagnostics are expected? > > Thanks @aaron.ballman Yes, you need to make sure that verify is on the RUN line. The existi

[PATCH] D38342: [C++] Parse (sub) postfix expression after boolean literal

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Do you need me to commit on your behalf? https://reviews.llvm.org/D38342 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D38342: [C++] Parse (sub) postfix expression after boolean literal

aaron.ballman closed this revision. aaron.ballman added a comment. Committed in r314463. Btw, if you're not in the IRC channel (#llvm on OFTC), you should hang out in there to make sure no build bots break due to the commit. https://reviews.llvm.org/D38342 __

[PATCH] D38270: [clang] Add getUnsignedPointerDiffType method

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

  1   2   3   4   5   6   7   8   9   10   >