[PATCH] D63276: [AST] Add FunctionDecl::getParametersSourceRange()

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/Decl.h:2331 + /// Attempt to compute an informative source range covering the + /// function parameters. This omits the ellipsis of a variadic function. + SourceRange getParametersSourceRange() const; ---

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

aaron.ballman added a comment. In D63082#1560667 , @xbolva00 wrote: > I wanted to follow GCC’s behaviour -Wall but then dicussion moved to > “tautological compare group” - I was fine with that too.. > I can again revert it to -Wall... > If this should b

[PATCH] D63845: [WIP] Create a clang attribute that lets users specify LLVM attributes

aaron.ballman added a comment. In D63845#1562006 , @wsmoses wrote: > Just to preface these comments -- this is work in progress and not intended > to go in as is / without discussion (but happily is indeed inviting > discussion). Fantastic! > In D6384

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp:40 +return; + Finder->addMatcher(varDecl().bind("var"), this); +} czhang wrote: > lebedev.ri wrote: > > Most of the matching should be

[PATCH] D63954: Add lifetime categories attributes

aaron.ballman added a comment. In D63954#1562928 , @erik.pilkington wrote: > > We explicitly allow to add an annotation after > > the definition of the class to allow adding annotations > > to external source from by the user, e.g. > > > > #include

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1034 + let Spellings = [Clang<"sycl_kernel">]; + let Subjects = SubjectList<[Function]>; + let LangOpts = [SYCL]; Shouldn't this be `FunctionTemplate` instead? ===

[PATCH] D63954: Add lifetime categories attributes

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4553 + // we always add (and check) the attribute to the cannonical decl. + D = D->getCanonicalDecl(); + if(AL.getKind() == ParsedAttr::AT_Owner) { xazax.hun wrote: > aaron.ballm

[PATCH] D61749: [clang-tidy] initial version of readability-static-const-method

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp:96 +CheckFactories.registerCheck( +"readability-static-method"); CheckFactories.registerCheck( I'm not super keen on the check na

[PATCH] D63845: [WIP] Create a clang attribute that lets users specify LLVM attributes

aaron.ballman added a comment. In D63845#1566987 , @jdoerfert wrote: > In D63845#1561983 , @lebedev.ri > wrote: > > > In D63845#1561793 , @jdoerfert > > wrote: > > > > > In

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

aaron.ballman added a comment. In D57086#1551354 , @domdom wrote: > In D57086#1550514 , @aaron.ballman > wrote: > > > In D57086#1549632 , @domdom wrote: > > > > > clang-form

[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

aaron.ballman added a comment. In D55793#1535895 , @m4tx wrote: > @JonasToth thanks for asking! Yes, since I do not have commit access, I need > someone to do the commit for me. I'm sorry for the terribly long delay on getting this committed -- it fell

[PATCH] D63845: [WIP] Create a clang attribute that lets users specify LLVM attributes

aaron.ballman added a comment. In D63845#1571855 , @jdoerfert wrote: > I think we should postpone a detailed discussion until we are ready to send > an RFC on this. Nevertheless, I inlined some responses below. > > In D63845#1570639

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

aaron.ballman closed this revision. aaron.ballman added a comment. I've commit on your behalf in r365498. Thank you for the patch! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57086/new/ https://reviews.llvm.org/D57086 ___ cfe-commits mail

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.cpp:39 +return; + Finder->addMatcher(varDecl(hasGlobalStorage()).bind("var"), this); +} czhang wrote: > aaron.ballman wrote: > > Do you wan

[PATCH] D63954: Add lifetime categories attributes

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2770 +def Owner : InheritableAttr { + let Spellings = [CXX11<"gsl", "Owner">]; + let Subjects = SubjectList<[CXXRecord]>; xazax.hun wrote: > aaron.ballman wrote: > > Has Microsof

[PATCH] D61749: [clang-tidy] initial version of readability-const-method

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp:96 +CheckFactories.registerCheck( +"readability-static-method"); CheckFactories.registerCheck( mgehre wrote: > aaron.ballman wrot

[PATCH] D64380: Add 'require_designated_init' and 'required' attribute to clang

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1947 +def RequireDesignatedInit : InheritableAttr { + let Spellings = [CXX11<"clang", "require_designated_init">]; This should be `RequiresDesignator`. Commen

[PATCH] D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks

aaron.ballman added reviewers: NoQ, Szelethus. aaron.ballman added a subscriber: NoQ. aaron.ballman added a comment. I'm worried that this will continue to drift out of sync with the static analyzer checks unless we find some way to automate it. I wonder if we could write a script to generate th

[PATCH] D63139: [Diagnostics] Implement -Wswitch-unreachable

aaron.ballman added a comment. In D63139#1561891 , @rjmccall wrote: > Are the `CaseStmt`s being dropped in C++ because the expression overflows? I > agree that that's bad AST behavior; we should strive to generate AST whenever > we can, even if it's not

[PATCH] D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks

aaron.ballman added a comment. In D64454#1578704 , @Szelethus wrote: > Just thinking aloud! > > We were tinkering with the idea of a checker creator script similar to what > clang-tidy has, that could help on this potentially. > > Is generating the rst co

[PATCH] D63192: [Diagnostics] Implement -Wswitch-default

aaron.ballman added a comment. I'm wary of adding this as an on-by-default warning, despite its presence in MSVC (and despite submitting a similar patch years ago to implement the same functionality). Machine-generated code runs into this one frequently (we've been bitten by it numerous times w

[PATCH] D63192: [Diagnostics] Implement -Wswitch-default

aaron.ballman added a comment. In D63192#1578867 , @xbolva00 wrote: > I see. I should really check clang-tidy codebase :) I have a hunch that this will be equally as trivially implementable in clang-tidy -- if you go that route, feel free to add me as a

[PATCH] D63139: [Diagnostics] Implement -Wswitch-unreachable

aaron.ballman added a comment. In D63139#1579231 , @rjmccall wrote: > I agree that tools shouldn't be forced to deal with invalid AST that looks > like valid AST. To me that means finding ways to preserve information that > (1) don't badly violate invar

[PATCH] D64543: [Docs] Add standardized header links to analyzer doc

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Do you need someone to commit on your behalf? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64543/new/ https://reviews.llvm.org/D64543 ___

[PATCH] D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks

aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/gen-static-analyzer-docs.py:2-3 +""" +Generates documentation based off the available static analyzers checks +References Checkers.td to determine what checks exist +""" Ad

[PATCH] D64380: Add 'require_designated_init' and 'required' attribute to clang

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1948 +def RequiresDesignator : InheritableAttr { + let Spellings = [Clang<"requires_designator">]; + let Subjects = SubjectList<[Record]>; Hmm, after making this suggestion, I no

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return-type check

aaron.ballman closed this revision. aaron.ballman added a comment. In D56160#1497473 , @bernhardmgruber wrote: > - fixed formatting > - fixed function names in tests > - added `-fexceptions` to test arguments > - fixed typo in release notes Thanks! I co

[PATCH] D61700: [clang-tidy] readability-redundant-declaration: fix false positive with C "extern inline"

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with some testing nits. Comment at: clang-tools-extra/test/clang-tidy/readability-redundant-declaration.c:32 +extern inline void g(); \ No newline at end

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 🔍

aaron.ballman added a comment. In D60543#1497576 , @stephanemoore wrote: > I did some digging and I believe there are two approaches that we can take to > extend `isDerivedFrom` to support Objective-C classes. > > **Option 1: Match on Common Ancestor Dec

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 🔍

aaron.ballman added reviewers: klimek, alexfh. aaron.ballman added a comment. Adding some more AST matcher reviewers to see if there are other options that we've not considered. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60543/new/ https://revi

[PATCH] D61851: [clang-tidy] New option for misc-throw-by-value-catch-by-reference

aaron.ballman added inline comments. Comment at: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp:25 + WarnOnLargeObject(Options.get("WarnOnLargeObject", false)), + // Cannot access `ASTContext` from here so set it to an extremal value + MaxSize(Options.get("M

[PATCH] D24892: [clang-tidy] Add option "LiteralInitializers" to cppcoreguidelines-pro-type-member-init

aaron.ballman added a comment. In D24892#1500371 , @mgehre wrote: > Do I wait for @alexfh to turn his red into a green, too? I think you covered all of the concerns he raised. If he doesn't give you an LG in the next 24 hours, I think you can go ahead a

[PATCH] D61619: Make language option `GNUAsm` discoverable with `__has_extension` macro.

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/D61619/new/ https://reviews.llvm.org/D61619 ___ cfe-commits mailing lis

[PATCH] D61370: Add a C2x mode and allow attributes in it

aaron.ballman added a comment. Ping x2. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61370/new/ https://reviews.llvm.org/D61370 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D60910: [WIP] Dumping the AST to JSON

aaron.ballman updated this revision to Diff 199316. aaron.ballman marked 2 inline comments as done. aaron.ballman added a comment. Switched to using the JSON streaming interface rather than a custom solution Updated the test cases for the new formatting CHANGES SINCE LAST ACTION https://review

[PATCH] D60910: [WIP] Dumping the AST to JSON

aaron.ballman added a comment. In D60910#1495673 , @rsmith wrote: > If you're happy with these two conditions, then I have no concerns with this > moving forward: > > - There is no implied stability for the content or format of the dump between > major r

[PATCH] D60910: [WIP] Dumping the AST to JSON

aaron.ballman closed this revision. aaron.ballman added a comment. Commit in r360622. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60910/new/ https://reviews.llvm.org/D60910 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://l

[PATCH] D61370: Add a C2x mode and allow attributes in it

aaron.ballman closed this revision. aaron.ballman marked an inline comment as done. aaron.ballman added a comment. I've commit in r360667, thanks! Comment at: lib/Frontend/CompilerInvocation.cpp:2593-2596 + // Enable [[]] attributes in C++11 and C2x by default. + Opts.DoubleS

[PATCH] D61874: [clang-tidy] Fix invalid fixit for readability-static-accessed-through-instance (bug 40544)

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61874/new/ https://reviews.llvm.org/D61874

[PATCH] D61835: Extract ASTDumper to a header file

aaron.ballman added a comment. I'm not certain where you're planning to go with this change (or is this the only change you're trying to make in this area?), so it's a bit hard to evaluate this patch. Can you explain a bit more about what you're ultimately trying to accomplish? It might help i

[PATCH] D61834: Add a Visit overload for DynTypedNode to ASTNodeTraverser

aaron.ballman added a comment. What will be making use of/testing this new functionality? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61834/new/ https://reviews.llvm.org/D61834 ___ cfe-commits mailing list cfe-comm

[PATCH] D61837: Make it possible control matcher traversal kind with ASTContext

aaron.ballman added a comment. Thanks for this -- it looks like really interesting functionality! I've mostly found nits thus far, but did have a question about clang-query support for it. Comment at: include/clang/AST/ASTContext.h:562-563 public: + ast_type_traits::Traversa

[PATCH] D61836: Move TraversalKind enum to ast_type_traits

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some commenting requests. Comment at: include/clang/AST/ASTTypeTraits.h:44 +enum TraversalKind { + /// Will traverse any child nodes. + TK_AsI

[PATCH] D61834: Add a Visit overload for DynTypedNode to ASTNodeTraverser

aaron.ballman added a comment. In D61834#1505056 , @steveire wrote: > In D61834#1504665 , @aaron.ballman > wrote: > > > What will be making use of/testing this new functionality? > > > Any code which has a `DynType

[PATCH] D61835: Extract ASTDumper to a header file

aaron.ballman added a comment. In D61835#1505048 , @steveire wrote: > In D61835#1504663 , @aaron.ballman > wrote: > > > I'm not certain where you're planning to go with this change (or is this > > the only change

[PATCH] D61835: Extract ASTDumper to a header file

aaron.ballman added a comment. In D61835#1505202 , @steveire wrote: > In D61835#1505151 , @aaron.ballman > wrote: > > > In D61835#1505048 , @steveire > > wrote: > > > > > T

[PATCH] D61837: Make it possible control matcher traversal kind with ASTContext

aaron.ballman added inline comments. Comment at: include/clang/AST/ASTContext.h:562-563 public: + ast_type_traits::TraversalKind GetTraversalKind() const { return Traversal; } + void SetTraversalKind(ast_type_traits::TraversalKind TK) { Traversal = TK; } + ste

[PATCH] D61835: Extract ASTDumper to a header file

aaron.ballman added a comment. In D61835#1505280 , @steveire wrote: > In D61835#1505228 , @aaron.ballman > wrote: > > > In D61835#1505202 , @steveire > > wrote: > > > > > 3

[PATCH] D61837: Make it possible control matcher traversal kind with ASTContext

aaron.ballman added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:718 +template +internal::Matcher traverse(ast_type_traits::TraversalKind TK, + const internal::Matcher &InnerMatcher) { steveire wrote: > aaron.

[PATCH] D61835: Extract ASTDumper to a header file

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D61835#1505388 , @steveire wrote: > In D61835#1505314 , @aaron.ballman > wrote: > > > In D618

[PATCH] D61837: Make it possible control matcher traversal kind with ASTContext

aaron.ballman added inline comments. Comment at: lib/AST/ASTContext.cpp:120 +ASTContext::TraverseIgnored(const ast_type_traits::DynTypedNode &N) { + if (auto E = N.get()) { +return ast_type_traits::DynTypedNode::create(*TraverseIgnored(E)); aaron.ballman wro

[PATCH] D61834: Add a Visit overload for DynTypedNode to ASTNodeTraverser

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D61834#1505418 , @steveire wrote: > In D61834#1505124 , @aaron.ballman > wrote: > > > In D618

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:292 + // "LangOpts" bound. + string CustomCode = customCode; } If this is code, should it be using a `code` type rather than a `string` type? Comment at: clan

[PATCH] D61508: [clang-tidy] bugprone-header-guard : a simple version of llvm-header-guard

aaron.ballman added inline comments. Comment at: clang-tidy/bugprone/HeaderGuardCheck.cpp:50 + + } else { +if (OldGuard.size()) I think this should be an `else if` rather than an `else`. I'd like to see us diagnose unknown guard styles so that we only accep

[PATCH] D61834: Add a Visit overload for DynTypedNode to ASTNodeTraverser

aaron.ballman added inline comments. Comment at: unittests/AST/ASTTraverserTest.cpp:75 + +template std::string dumpASTString(NodeType &&... N) { + std::string Buffer; steveire wrote: > aaron.ballman wrote: > > Did clang-format produce this formatting? (If not,

[PATCH] D61851: [clang-tidy] New option for misc-throw-by-value-catch-by-reference

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM (with the typo fix). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61851/new/ https://reviews.llvm.org/D61851 ___ cfe

[PATCH] D62065: Move dump method implementations to their respective class files

aaron.ballman added inline comments. Comment at: lib/AST/Comment.cpp:378-380 +LLVM_DUMP_METHOD void Comment::dump() const { + dump(llvm::errs(), nullptr, nullptr); +} If we're moving things around, is there a reason we should keep this in a source file as oppos

[PATCH] D62056: Use ASTDumper to dump the AST from clang-query

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 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62056/new/ https://reviews.llvm.org/D62056 __

[PATCH] D62056: Use ASTDumper to dump the AST from clang-query

aaron.ballman added a comment. If you wanted to expand test coverage for the changes covered in this, that would be a good thing. However, I'm not certain how testable the newly colorized output is, which is why this LG as-is. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION h

[PATCH] D61508: [clang-tidy] bugprone-header-guard : a simple version of llvm-header-guard

aaron.ballman added a comment. In D61508#1509368 , @trixirt wrote: > Latest change addresses most of issues. > Outstanding is adding a test when garbage GuardStyle value is fed into > config. > The trial testcase used only CHECK-*-NOT which caused compl

[PATCH] D62192: [clang-tidy]: Add cert-oop54-cpp alias for bugprone-unhandled-self-assignment

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some nits, thank you for this! Comment at: clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:89-91 +AdditionalMatcher

[PATCH] D61643: [PragmaHandler][NFC] Expose `#pragma` location

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/docs/ClangPlugins.rst:58 ExamplePragmaHandler() : PragmaHandler("example_pragma") { } -void HandlePragma(Preprocessor &PP, Prag

[PATCH] D61508: [clang-tidy] bugprone-header-guard : a simple version of llvm-header-guard

aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: clang-tidy/bugprone/HeaderGuardCheck.cpp:30 +} +std::string BugproneHeaderGuardCheck::getHeaderGuard(StringRef Filename, + StringRe

[PATCH] D61509: [OpenMP] Set pragma start loc to `#pragma` loc

aaron.ballman added a comment. In D61509#1512090 , @jdenny wrote: > Now that D61643 is pushed, we're back to > this patch. My recollection is there are two remaining issues: > > 1. Should we store both the `#pragma` loca

[PATCH] D61509: [OpenMP] Set pragma start loc to `#pragma` loc

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D61509#1512556 , @jdenny wrote: > The overall vote seems to be that this patch is ready to push, and we/I > should work on other pragm

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Comment at: clang/include/clang/Basic/Attr.td:297 def CUDA : LangOpt<"CUDA">; -def COnly : LangOpt<"CPlusPlus", 1>; +def COnly : LangOpt<"COnly", "!LangOpts.CPlusPlus">; def CPlusPlus : LangOpt<"CPlusP

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

aaron.ballman accepted this revision. aaron.ballman added a comment. My only real concern about this is that sometimes the fix-it will be wrong because the issue is a typo in the definition. However, it seems like that could be handled by preferring typo correction when the edit distance is belo

[PATCH] D59402: Fix-it hints for -Wmissing-{prototypes,variable-declarations}

aaron.ballman added a comment. In D59402#1516421 , @aaronpuchert wrote: > I guess you're referring to "[fix-it hints] should only be used when it’s > very likely they match the user’s intent". Also, we're not attempting to recover from the error, which

[PATCH] D62404: [clang-tidy] Fix null pointer dereference in readability-identifier-naming

aaron.ballman added a comment. In D62404#1516243 , @markstegeman wrote: > I haven't yet been able to figure out how to properly add a test for this > bug, since the expected result is a failure to compile while the current > test/clang-tidy/readability-

[PATCH] D62404: [clang-tidy] Fix null pointer dereference in readability-identifier-naming

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/readability/IdentifierNamingCheck.cpp:804 +if (const auto *Typedef = TypePtr->getAs()) { + addU

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 🔍

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2638 + // Check if the node is a C++ struct/union/class. + if (const auto *RecordDecl = dyn_cast(&Node)) +return Finder->classIsDerivedFrom(RecordDecl, Base, Builder); ---

[PATCH] D62470: Clarify when fix-it hints on warnings are appropriate

aaron.ballman added a comment. Thank you for working on this! Comment at: docs/InternalsManual.rst:426-428 +* Fix-it hints on a warning should only clarify the meaning of the existing + code, not change it. Examples for such hints are added parentheses in cases + of counter-i

[PATCH] D62435: Add Attribute NoThrow as an Exception Specifier Type

aaron.ballman added inline comments. Comment at: clang/include/clang-c/Index.h:227 + /** + * The cursor has a declspec(nothrow) exception specification. + */ `__declspec(nothrow)` Comment at: clang/include/clang/Basic/ExceptionSpecificatio

[PATCH] D62437: [clang-tidy] Splits fuchsia-default-arguments

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. It seems that the functional changes are missing from the patch -- all I see are formatting changes; have I missed something? Comment at: clang-tool

[PATCH] D62435: Add Attribute NoThrow as an Exception Specifier Type

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:6970 +if (Proto->hasExceptionSpec()) + return true; + erichkeane wrote: > aaron.ballman wrote: > > I think we should diagnose that the `nothrow` is ignored in this case to > >

[PATCH] D62470: Clarify when fix-it hints on warnings are appropriate

aaron.ballman added inline comments. Comment at: docs/InternalsManual.rst:426-428 +* Fix-it hints on a warning should only clarify the meaning of the existing + code, not change it. Examples for such hints are added parentheses in cases + of counter-intuitive precedence, when t

[PATCH] D62470: Clarify when fix-it hints on warnings are appropriate

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/D62470/new/ https://reviews.llvm.org/D62470 ___

[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); -

[PATCH] D55546: [clang] Add AST matcher for block expressions 🔍

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/D55546/new/ https://reviews.llvm.org/D55546 ___

[PATCH] D55665: Output to SARIF from scan-build

aaron.ballman created this revision. aaron.ballman added reviewers: NoQ, george.karpenkov. This updates the scan-build perl script to allow outputting to sarif in a more natural fashion by specifying `-sarif` as a command line argument, similar to how `-plist` is already supported. There appear

[PATCH] D55665: Output to SARIF from scan-build

aaron.ballman closed this revision. aaron.ballman added a comment. Committed in r349082. I left the test file off; it looks like it might work, but given that scan-build puts its output into a temp directory, I wasn't able to see a good way to handle checking the output. Given the simplicity of

[PATCH] D55628: Add support for "labels" on push/pop directives in #pragma clang attribute

aaron.ballman added inline comments. Comment at: clang/docs/LanguageExtensions.rst:2667 +Because multiple push directives can be nested, if you're writing a macro that +expands to ``_Pragma("clang attribute")`` it's good hygiene to add a label to +your push/pop directives. A pop

[PATCH] D55552: [Sema] Better static assert diagnostics for expressions involving temporaries.

aaron.ballman added inline comments. Comment at: include/clang/AST/Stmt.h:847 + const ASTContext *Context = nullptr, + bool PrintCanonicalTypes = false) const; I'm pretty wary of long lists of parameters that include booleans

[PATCH] D53713: Add extension to always default-initialize nullptr_t.

aaron.ballman added reviewers: NoQ, george.karpenkov. aaron.ballman added a comment. Adding some reviewers for the static analyzer questions raised. I think this is a reasonable approach, but are there situations where we should diagnose this as an extension? I can't think of one, but I figured

[PATCH] D55658: Avoid Decl::getASTContext in hot paths where possible, Part 1

aaron.ballman added a reviewer: rsmith. aaron.ballman added a subscriber: rsmith. aaron.ballman added a comment. I'd like @rsmith's opinion on whether this is a good churn or not. I think it's mostly reasonable, but it's also a lot of changes for identical behavior and in some cases the changes

[PATCH] D55628: Add support for "labels" on push/pop directives in #pragma clang attribute

aaron.ballman added inline comments. Comment at: clang/test/Sema/pragma-attribute-label.c:7 + +#pragma clang attribute pop // expected-error{{'#pragma clang attribute pop' with no matching '#pragma clang attribute push'}} +#pragma clang attribute pop NOT_MY_LABEL // expected-err

[PATCH] D55707: Update to SARIF 11-28

aaron.ballman created this revision. aaron.ballman added reviewers: NoQ, george.karpenkov. This updates our SARIF support from 10-10 to 11-28. Functional changes include: - The run.files property is now an array instead of a mapping. - fileLocation objects now have a fileIndex property specifying

[PATCH] D42682: [clang-tidy] Add io-functions-misused checker

aaron.ballman added inline comments. Comment at: clang-tidy/bugprone/IoFunctionsCheck.cpp:47-49 + "consider to cast the return value of %0 from type integer to type char, " + "possible loss of precision if an error has occurred or the end " + "of file has been rea

[PATCH] D55707: Update to SARIF 11-28

aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:218 const PathDiagnosticLocation &P = Piece->getLocation(); +const auto *CP = dyn_cast(Piece.get()); Locations.push_back(cre

[PATCH] D55628: Add support for "labels" on push/pop directives in #pragma clang attribute

aaron.ballman added inline comments. Comment at: clang/test/Sema/pragma-attribute-label.c:15 +// Out of order! +#pragma clang attribute pop MY_LABEL + dexonsmith wrote: > erik.pilkington wrote: > > aaron.ballman wrote: > > > dexonsmith wrote: > > > > aaron.ballma

[PATCH] D55707: Update to SARIF 11-28

aaron.ballman marked 4 inline comments as done. aaron.ballman added inline comments. Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:129 + unsigned Index = 0; + for (const json::Value &File : Files) { +if (const json::Object *Obj = File.getAsObject()) { ---

[PATCH] D55707: Update to SARIF 11-28

aaron.ballman closed this revision. aaron.ballman added a comment. Committed in r349188. (@NoQ -- if you'd like to see a switch to `llvm::find_if()`, I'm happy to handle it post-commit.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55707/new/ https://reviews.llvm.org/D55707 __

[PATCH] D51211: [Sema] Emit -Wformat properly for bitfield promotions.

aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:7725-7726 +return true; + // Look through vector types, since we do default argument promotion for + // those in OpenCL. + if (const ExtVectorType *VecTy = From->getAs()) Looking at

[PATCH] D55707: Update to SARIF 11-28

aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:129 + unsigned Index = 0; + for (const json::Value &File : Files) { +if (const json::Object *Obj = File.getAsObject()) { ---

[PATCH] D55683: [analyzer] Tests for scan-build?

aaron.ballman added a comment. Could we make use of scan-build `-o` to specify the exact output location, in conjunction with `stable-report-filename=true` to remove the non-determinism? Also, I like the idea of maybe someday making scan-build-py test the same file. However, it seems like that

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:2871-2872 /// distribute: attempt to distribute loop if State == Enable + /// pipeline: disable pipelining loop if State == Disable + /// pipeline_ii_count: try to pipeline loop for only 'Value' val

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:2871-2872 /// distribute: attempt to distribute loop if State == Enable + /// pipeline: disable pipelining loop if State == Disable + /// pipeline_ii_count: try to pipeline loop for only 'Value' val

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:2871-2872 /// distribute: attempt to distribute loop if State == Enable + /// pipeline: disable pipelining loop if State == Disable + /// pipeline_ii_count: try to pipeline loop for only 'Value' val

[PATCH] D55790: [AST] Add accessSpecs() iterator range for CXXRecordDecl

aaron.ballman added a comment. In D55790#1333664 , @m4tx wrote: > In D55790#1333627 , @riccibruno > wrote: > > > Do you really have to add an iterator for this particular thing ? > > Why not just use `specific_dec

[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

aaron.ballman added inline comments. Comment at: clang-tidy/readability/DuplicatedAccessSpecifiersCheck.cpp:21 +void DuplicatedAccessSpecifiersCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + cxxRecordDecl(has(accessSpecDecl())) You sh

<    11   12   13   14   15   16   17   18   19   20   >