[PATCH] D112914: Misleading identifier detection

2021-11-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-misleading-identifier.rst:12 + +.. code-block:: c + aaronpuchert wrote: > Sphinx can't parse that (perhaps unsurprisingly), could you declare that as > having no lang

[PATCH] D51650: Implement target_clones multiversioning

2021-11-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D51650#3126569 , @akuegel wrote: > Since it is not clear whether the semantic change was intended, I think it > makes sense to revert the patch for now. If it is intended, it might be good > to mention it in the change d

[PATCH] D97411: [DebugInfo] Add an attribute to force type info to be emitted for types that are required to be complete.

2021-02-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Can you also add semantic tests that verify the attribute only works in C++, only appertains to a record (as opposed to other kinds of declarations like functions), and doesn't accept arguments. Some other tests that may be interesting: // Should test redeclara

[PATCH] D97512: [clang] removes check against integral-to-pointer conversion...

2021-02-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:10320 + + const clang::CastKind Kind = Cast->getCastKind(); + if (Kind == clang::CK_BitCast && We don't typically use top-level `const` on locals or params. Comm

[PATCH] D97544: [clang-tidy] Remove some test c++ mode restrictions.

2021-02-26 Thread Aaron Ballman via Phabricator via cfe-commits
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/D97544/new/ https://reviews.llvm.org/D97544 __

[PATCH] D97553: [clang][NFC] pack StaticDiagInfoRec

2021-02-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/lib/Basic/DiagnosticIDs.cpp:171 const StaticDiagInfoRec StaticDiagInfo[] = { +// clang-format off #define DIAG(ENUM, CLASS, DEFAULT_SE

[PATCH] D97488: [clang-tidy][NFC] Tweak some generation of diag messages

2021-02-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Thanks, these are really nice cleanups! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97488/new/ https://reviews.llvm.org/D9

[PATCH] D97411: [DebugInfo] Add an attribute to force type info to be emitted for types that are required to be complete.

2021-02-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Thanks! I am still curious about the forward declare/redeclaration behavior and whether that is a situation that makes sense or not. I suspect this case may make sense (and likely already works): // Should test redeclaration behavior. struct [[clang::standalon

[PATCH] D84924: [clang-tidy] Added command line option `fix-notes`

2021-03-01 Thread Aaron Ballman via Phabricator via cfe-commits
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/D84924/new/ https://reviews.llvm.org/D84924 __

[PATCH] D97614: [clang-tidy] Remove OptionError

2021-03-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, nice cleanup! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97614/new/ https://reviews.llvm.org/D97614

[PATCH] D97681: [ASTMatchers] Add matchers for `CXXDeleteExpr`

2021-03-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7621 +/// matches the expression 'delete Ptr'. +AST_MATCHER_P_OVERLOAD(CXXDeleteExpr, deletes, internal::Matcher, + InnerMatcher, 1) { Why add t

[PATCH] D97491: [clang-tidy] Deprecate readability-deleted-default check

2021-03-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D97491#2589098 , @Eugene.Zelenko wrote: > In D97491#2589097 , @njames93 wrote: > >> In D97491

[PATCH] D97681: [ASTMatchers] Add matchers for `CXXDeleteExpr`

2021-03-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7621 +/// matches the expression 'delete Ptr'. +AST_MATCHER_P_OVERLOAD(CXXDeleteExpr, deletes, internal::Matcher, + InnerMatcher, 1) { njames93

[PATCH] D97632: [clang-tidy] Simplify diagnostics for UniqueptrResetRelease check

2021-03-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a nit with the diagnostic wording. Comment at: clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp:115 + auto D = diag(ResetMember

[PATCH] D97313: [clang-tidy] Tweak misc-static-assert fix in c++17

2021-03-01 Thread Aaron Ballman via Phabricator via cfe-commits
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/D97313/new/ https://reviews.llvm.org/D97313 __

[PATCH] D95396: Improve static_assert/_Static_assert diagnostics

2021-03-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95396/new/ https://reviews.llvm.org/D95396 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97717: [SYCL] Rework the SYCL driver options

2021-03-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision. aaron.ballman added reviewers: erichkeane, bader. Herald added subscribers: jansvoboda11, dexonsmith, dang, Anastasia, ebevhan, yaxunl. aaron.ballman requested review of this revision. Herald added a project: clang. SYCL compilations initiated by the driver wi

[PATCH] D97630: [clang-tidy] Added option to uniqueptr delete release check

2021-03-01 Thread Aaron Ballman via Phabricator via cfe-commits
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/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp:40 + + delete (P2.release()); + // CHECK-MESSAGES-NULLP

[PATCH] D97630: [clang-tidy] Added option to uniqueptr delete release check

2021-03-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-uniqueptr-delete-release.cpp:40 + + delete (P2.release()); + // CHECK-MESSAGES-NULLPTR: :[[@LINE-1]]:3: warning: prefer '= nullptr' njames93 wrote: > aaron.

[PATCH] D97362: [clang][parser] Allow attributes in explicit template instantiations

2021-03-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/Parse/Parser.h:1586-1589 + for (const ParsedAttr &A : *this) { +if (A.getSyntax() != ParsedAttr::Syntax::AS_GNU) + return false; + } Comment at: cl

[PATCH] D97717: [SYCL] Rework the SYCL driver options

2021-03-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 327412. aaron.ballman added a comment. Rebased and added a Frontend-specific test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97717/new/ https://reviews.llvm.org/D97717 Files: clang/include/clang/Basic/LangOptions.def clang/include/cla

[PATCH] D97764: [clang][patch] To solve PR26413, x86 interrupt routines may only call routines with no_saved_reg

2021-03-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:297-298 +def err_anyx86_interrupt_regsave : Error< + "interrupt service routine may only call routine" + " with attribute no_caller_saved_registers">; def warn_arm_interrupt_calli

[PATCH] D95396: Improve static_assert/_Static_assert diagnostics

2021-03-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 327426. aaron.ballman added a comment. Address review comments from @rnk Only define the `static_assert` macro if one is not already defined and test the expected behavior. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95396/new/ https://rev

[PATCH] D97362: [clang][parser] Allow attributes in explicit template instantiations

2021-03-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1828-1830 + // Allow only GNU attributes here. + if (!attrs.hasOnlyGNUAttributes()) +ProhibitAttributes(attrs); tbaeder wrote: > aaron.ballman wrote: > > We really

[PATCH] D97771: [cte][NFC] Remove all references to stdlib stream headers.

2021-03-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:38 + if (!Buffer) { +llvm::errs() << "Error cannot open JSON request file:" << FileName << ": " + << Buffer.getError().message() << "\n";

[PATCH] D97764: [clang][patch] To solve PR26413, x86 interrupt routines may only call routines with no_saved_reg

2021-03-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a small change to the diagnostic (routine -> a function, and adding single quotes around the syntax element in the diagnostic). Comment at: cla

[PATCH] D97717: [SYCL] Rework the SYCL driver options

2021-03-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D97717#2599728 , @bader wrote: > @aaron.ballman, it looks like unittests should be updated as well. Please, > take a look at failures in pre-merge checks. Thanks, I'll correct those! Comment at: clang

[PATCH] D97806: [clang-query] Fix help text after D91918

2021-03-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97806/new/ https://reviews.llvm.org/D97806 ___ cfe-commits mailing list cfe-commit

[PATCH] D95396: Improve static_assert/_Static_assert diagnostics

2021-03-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision. aaron.ballman added a comment. In D95396#2598932 , @rsmith wrote: > Looks like `test/FixIt/fixit-static-assert.cpp` is failing in Phabricator's > pre-merge checks: B91556 . Please take a

[PATCH] D97362: [clang][parser] Allow attributes in explicit template instantiations

2021-03-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Parse/ParseDecl.cpp:1618 + +if (Tok && (*Tok).is(tok::l_square)) { + Diag(Attrs.Range.getBegin(), diag::err_attributes_not_allowed) << Attrs.Range; This will incorrectly classify an empty MS att

[PATCH] D97717: [SYCL] Rework the SYCL driver options

2021-03-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 327769. aaron.ballman added a comment. Fixed the failing unit tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97717/new/ https://reviews.llvm.org/D97717 Files: clang/include/clang/Basic/LangOptions.def clang/include/clang/Driver/Opti

[PATCH] D97849: [AST][PCH][ASTImporter] Fix UB caused by uninited SwitchStmt member

2021-03-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. This looks reasonable to me (good catch!), but is there a way for us to add a regression test for it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97849/new/ https://reviews.llvm.org/D97849

[PATCH] D97805: [clang-query] Add option to enable only displaying main file matches

2021-03-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-query/Query.cpp:60-61 +"main-file-only " +"Only match nodes from the main source file. This mode is the " +"default.\n" " set output " ---

[PATCH] D95691: Implement P2173 for attributes on lambdas

2021-03-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision. aaron.ballman added a comment. I've commit in b2bc0a32545f9b066fed1631c6fba92a2a5a6d84 and will work on a patch for the remainder of the diagnostic groups. Thanks for the reviews! CHANGES

[PATCH] D97871: Update diagnostic groups for pre-compat warnings

2021-03-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision. aaron.ballman added reviewers: rsmith, rjmccall. aaron.ballman requested review of this revision. Herald added a project: clang. As a follow-up to D95691 , add new diagnostic groups named `pre-c++N-compat` to replace the old di

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

2021-03-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Just a few minor nits from me, but I'm mostly wondering: where are we at with this and are there still substantive changes required? (I looked through the comments, but there's a lot of back-and-forth since Oct and I'm not certain what's holding the patch back cur

[PATCH] D97849: [AST][PCH][ASTImporter] Fix UB caused by uninited SwitchStmt member

2021-03-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I don't think we have any tests that explicitly use valgrind and testing with valgrind enabled is an optional thing. I would be fine landing this as-is without a test case if it

[PATCH] D97512: [clang] removes check against integral-to-pointer conversion...

2021-03-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! I think we can resolve the question of which way to ignore implicit nodes and make any necessary changes post-commit. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D94640: adds more checks to -Wfree-nonheap-object

2021-03-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D94640#2603054 , @lebedev.ri wrote: >> <...> > > Is the patch still not reverted? > That should have happened almost a week ago. FWIW, the fix for the issue was just approved. Repository: rG LLVM Github Monorepo CHAN

[PATCH] D97805: [clang-query] Add option to enable only displaying main file matches

2021-03-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a typo fix. Comment at: clang-tools-extra/clang-query/Query.cpp:56 +"include-headers " +"Match nodes include

[PATCH] D97889: [clang-tidy] Fix assert in modernize-loop-convert

2021-03-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:316-317 return nullptr; + if (!Member->getMemberDecl()->getDeclName().isIdentifier()) +return nullptr; StringRef Name = Member->getMemberDecl()->getName(); --

[PATCH] D97889: [clang-tidy] Fix assert in modernize-loop-convert

2021-03-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp:316-317 return nullptr; + if (!Member->getMemberDecl()->getDeclName().isIdentifier()) +return nullptr; StringRef Name = Member->getMemberDecl()->getName(); --

[PATCH] D97940: [clang-tidy] Extend LoopConvert on array with `!=` comparison

2021-03-04 Thread Aaron Ballman via Phabricator via cfe-commits
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/D97940/new/ https://reviews.llvm.org/D97940 __

[PATCH] D73363: Verify that clang's max alignment is <= LLVM's max alignment

2021-03-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CGValue.h:18 #include "clang/AST/ASTContext.h" +#include "clang/Sema/Sema.h" #include "clang/AST/Type.h" rnk wrote: > This includes Sema.h into every codegen file that uses CGValue.h (most of >

[PATCH] D73363: Verify that clang's max alignment is <= LLVM's max alignment

2021-03-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CGValue.h:18 #include "clang/AST/ASTContext.h" +#include "clang/Sema/Sema.h" #include "clang/AST/Type.h" davezarzycki wrote: > aaron.ballman wrote: > > rnk wrote: > > > This includes Sema.h into

[PATCH] D73363: Verify that clang's max alignment is <= LLVM's max alignment

2021-03-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CGValue.h:18 #include "clang/AST/ASTContext.h" +#include "clang/Sema/Sema.h" #include "clang/AST/Type.h" davezarzycki wrote: > aaron.ballman wrote: > > davezarzycki wrote: > > > aaron.ballman wr

[PATCH] D73363: Verify that clang's max alignment is <= LLVM's max alignment

2021-03-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CGValue.h:18 #include "clang/AST/ASTContext.h" +#include "clang/Sema/Sema.h" #include "clang/AST/Type.h" davezarzycki wrote: > aaron.ballman wrote: > > davezarzycki wrote: > > > aaron.ballman wr

[PATCH] D98055: [ExtVectorType] Support conditional select operator for C++.

2021-03-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Comment at: clang/lib/Sema/SemaExprCXX.cpp:6172-6174 + if (IsVectorConditional) { +return CheckVectorConditionalTypes(Cond, LHS, RHS, QuestionLoc); + } Repository: rG LLVM Git

[PATCH] D97951: [Sema] Fix diagnostics for one-byte length modifier

2021-03-09 Thread Aaron Ballman via Phabricator via cfe-commits
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/D97951/new/ https://reviews.llvm.org/D97951 ___ c

[PATCH] D98337: Add support for digit separators in C2x

2021-03-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision. aaron.ballman added reviewers: rsmith, jyknight, rjmccall. aaron.ballman requested review of this revision. Herald added a project: clang. WG14 adopted N2626 at the meetings this week. This proposal adds support for using `'` as a digit separator in a numeric

[PATCH] D97717: [SYCL] Rework the SYCL driver options

2021-03-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97717/new/ https://reviews.llvm.org/D97717 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97871: Update diagnostic groups for pre-compat warnings

2021-03-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97871/new/ https://reviews.llvm.org/D97871 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

[PATCH] D98211: Solve PR49479, File scope fp pragma should propagate to functions nested in struct, and initialization expressions

2021-03-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM as well, but feel free to give the other reviewers a chance to weigh in. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98211/new/ https://reviews.llvm.org/D98211 ___

[PATCH] D98337: Add support for digit separators in C2x

2021-03-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision. aaron.ballman added a comment. Thanks for the review, I've commit in e448310059053d69fbdd6c66dd95ad5d3cfc6243 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D97411: [DebugInfo] Add an attribute to force type info to be emitted for types that are required to be complete.

2021-03-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM with a small, technical nit to be fixed. Comment at: clang/include/clang/Basic/Attr.td:1664 +def StandaloneDebug : InheritableAttr { + let Spellings = [Clang<"standalone_debug">]; + let Subjects = Subje

[PATCH] D97411: [DebugInfo] Add an attribute to force type info to be emitted for types that are required to be complete.

2021-03-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. I should note, you need to also fix the CI build failures: https://reviews.llvm.org/harbormaster/unit/view/421174/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97411/new/ https://reviews.llvm.org/D97411 ___

[PATCH] D98450: [clang] store an identifer instead of declref for ns_error_domain attribute

2021-03-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D98450#2621877 , @MForster wrote: > That's ok for me. I'd suggest to wait for Aaron's feedback, because IIRC it > was his suggestion in the first place to go to declref. I have the same concerns with this patch as I did

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

2021-03-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D89909#2623211 , @Anastasia wrote: > In D89909#2617194 , @bader wrote: > >> @Anastasia, do you suggest we copy >> https://github.com/intel/llvm/blob/sycl/sycl/doc/CompilerAndRuntim

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

2021-03-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D89909#2623887 , @Anastasia wrote: > In D89909#2623250 , @aaron.ballman > wrote: > >> In D89909#2623211 , @Anastasia >> wrote: >> >>> In D

[PATCH] D98610: [RISCV] Support clang -fpatchable-function-entry && GNU function attribute 'patchable_function_entry'

2021-03-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. The attribute bits look fine to me aside from the documentation request. Comment at: clang/include/clang/Basic/Attr.td:738 let Args = [UnsignedArgument<"Count">, DefaultIntArgument<"Offset", 0>]; let Documentation = [PatchableFunctionEntryDo

[PATCH] D98134: [RFC][POC] Introduce callback argument encoding mode into callback metadata

2021-03-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:432 If a statement is marked ``nomerge`` and contains call expressions, those call -expressions inside the statement will not be merged during optimization. This +expressions inside the state

[PATCH] D98521: [clang-tidy] Fix readability-identifer-naming duplicating prefix or suffix for replacements.

2021-03-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Good catch, LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98521/new/ https://reviews.llvm.org/D98521 __

[PATCH] D98295: [Clang] Add addrsig attribute to mark global functions/variables as address significant.

2021-03-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. I'm not opposed to the attribute per se, but I'd like to understand the motivation behind it a bit more. It seems like it solves a special edge case for a specific linker and I'm wondering if that's sufficient motivation for the community to support the attribute

[PATCH] D98416: [clang-tidy] Fix cppcoreguidelines-narrowing-conversions false positive

2021-03-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Herald added a project: clang-tools-extra. > However this has opened a can of worms. Currenty float a = > std::numeric_limits::infinity(); is marked as narrowing. I think this is technically not narrowing, per http://eel.is/c++draft/dcl.init.list#7.2, because the

[PATCH] D98433: [clang] [C++2b] [P1102] Accept lambdas without parameter list ().

2021-03-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Parse/ParseExprCXX.cpp:1438 -// Parse trailing-return-type[opt]. -if (Tok.is(tok::arrow)) { - FunLocalRangeEnd = Tok.getLocation(); - SourceRange Range; - TrailingReturnType = - ParseTrai

[PATCH] D98635: [libtooling][clang-tidy] Fix diagnostics not respecting and highlighting fed SourceRanges

2021-03-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Thank you for looking into this! It looks like there are some unit test failures from the change: https://buildkite.com/llvm-project/premerge-checks/builds/29754#a9b04ab5-e326-4ff1-beea-773f21a33349 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D98635: [libtooling][clang-tidy] Fix diagnostics not respecting and highlighting fed SourceRanges

2021-03-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D98635#2626485 , @whisperity wrote: > In D98635#2626464 , @whisperity > wrote: > >> Strange because I specifically ran both `check-clang` **and** >> `check-clang-tools` locally, b

[PATCH] D98363: [Sema] Fold VLA types in compound literals to constant arrays.

2021-03-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/test/Sema/vla.c:134 + // expected-warning@+1{{variable length array folded to constant array as an extension}} + char (*a9)[] = (char[2][ksize]) {{1,2,3,4},{4,3,2,1}}; + Doesn't this violate the constraint

[PATCH] D98363: [Sema] Fold VLA types in compound literals to constant arrays.

2021-03-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/test/Sema/vla.c:134 + // expected-warning@+1{{variable length array folded to constant array as an extension}} + char (*a9)[] = (char[2][ksize]) {{1,2,3,4},{4,3,2,1}}; + efriedma wrote: > aaron.ballman wro

[PATCH] D97717: [SYCL] Rework the SYCL driver options

2021-03-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 330786. aaron.ballman added a comment. Update based on review feedback to gate use of -sycl-std on use of -fsycl-is-device or -fsycl-is-host. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97717/new/ https://reviews.llvm.org/D97717 Files: c

[PATCH] D97717: [SYCL] Rework the SYCL driver options

2021-03-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: clang/unittests/Frontend/CompilerInvocationTest.cpp:547 + // there is no syntax I could find that would allow it). However, the option + // is handled properly on a real invocation.

[PATCH] D98363: [Sema] Fold VLA types in compound literals to constant arrays.

2021-03-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/test/Sema/vla.c:134 + // expected-warning@+1{{variable length array folded to constant array as an extension}} + char (*a9)[] = (char[2][ksize]) {{1,2,3,4},{4,3,2,1}}; + efriedma wrote: > aaron.ballman wro

[PATCH] D113847: [clang-tidy][NFC] Simplify ClangTidyStats

2021-11-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Btw, I'm in WG14 (C) standards meetings all week this week and on vacation next week, so my availability for code reviews is pretty limited for the next while. So despite my comments here, if you don't get a response back from me in a timely manner, don't hold up

[PATCH] D113148: Add new clang-tidy check for string_view(nullptr)

2021-11-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D113148#3130779 , @whisperity wrote: > In D113148#3109190 , @CJ-Johnson > wrote: > >> In D113148#3108993 , >> @aaron.ballman wrote: >>

[PATCH] D113847: [clang-tidy][NFC] Simplify ClangTidyStats

2021-11-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:50 struct ClangTidyStats { - ClangTidyStats() - : ErrorsDisplayed(0), ErrorsIgnoredCheckFilter(0), ErrorsIgnoredNOLINT(0), -ErrorsIgnoredNonUserCode(0), Err

[PATCH] D113847: [clang-tidy][NFC] Simplify ClangTidyStats

2021-11-15 Thread Aaron Ballman via Phabricator via cfe-commits
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/ClangTidyDiagnosticConsumer.h:61-64 - unsigned errorsIgnored() const { + int errorsIgnored() const { retur

[PATCH] D113575: Add `isInitCapture` and `forEachLambdaCapture` matchers.

2021-11-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113575/new/ https://reviews.llvm.org/D113575 ___ cfe-commits mailing list cfe-comm

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

2021-11-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D64454#3130696 , @whisperity wrote: > In D64454#3124312 , @aaron.ballman > wrote: > >> Yeah, I was worried this would get out of sycn and it really did not take >> long for that t

[PATCH] D113585: [clang-tidy] Fix false positive in `bugprone-throw-keyword-missing` check

2021-11-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, that is certainly not a place where one would put `throw`. :-D Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113585/new/ htt

[PATCH] D113585: [clang-tidy] Fix false positive in `bugprone-throw-keyword-missing` check

2021-11-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D113585#3132793 , @fwolff wrote: > Thanks for reviewing this! Can you also commit it for me? Sure can! What name and email address would you like used for patch attribution? I'm in C standards meetings all this week so

[PATCH] D112648: [clang-tidy] Improve fix-its for `modernize-pass-by-value` check

2021-11-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added reviewers: aaron.ballman, whisperity, hokein. aaron.ballman added a comment. Herald added a subscriber: rnkovacs. In D112648#3132157 , @avogelsgesang wrote: > @ymandel, @whisperity, @aaron.ballman could one of you review this/point m

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

2021-11-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added reviewers: alexfh, whisperity. aaron.ballman added a comment. In D112646#3132141 , @avogelsgesang wrote: > @xazax.hun / @whisperity could you review this change for me, please :) > Or, alternatively: can you direct me to somebody who

[PATCH] D113585: [clang-tidy] Fix false positive in `bugprone-throw-keyword-missing` check

2021-11-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision. aaron.ballman added a comment. I've commit on your behalf in 738e7f1231949ec248c1d8d154783338215613d1 , thank you for the fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D112773: Allow __attribute__((swift_attr)) in attribute push pragmas

2021-11-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D112773#3135975 , @beccadax wrote: >> My reading of >> https://clang.llvm.org/docs/LanguageExtensions.html#specifying-an-attribute-fo

[PATCH] D112773: Allow __attribute__((swift_attr)) in attribute push pragmas

2021-11-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D112773#3137338 , @aaron.ballman wrote: > Thank you for diving into those details! I'm now sold on the idea that `any` > with no subjects is dangerous and we will diagnose if the user tries this. So > I think the only

[PATCH] D114080: [SYCL] Diagnose uses of zero length arrays

2021-11-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D114080#3137462 , @Naghasan wrote: > right, TIL `sizeof (ContainsArr)` is 0 according to clang. I can see trouble > arising. > > @aaron.ballman maybe you know better, is that intended for the extension ? I don't *know*,

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

2021-11-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Thanks for your patience with this review! I'm currently in WG14 meetings this week and out on vacation next week, so my review availability is a bit limited at the moment (sorry for that). I think this is heading in the right direction, but there are some lifetim

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

2021-11-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D110215#3139589 , @ChuanqiXu wrote: > Change includes: > > - Add parent for newly created global module fragment. > - Add a test. > - Rename test directory. Suddenly I found that the names of the tests in CXX > directory

[PATCH] D114149: [clang-tidy] Fix pr48613: "llvm-header-guard uses a reserved identifier"

2021-11-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman requested changes to this revision. aaron.ballman added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp:28 +/// Replace invalid characters in the identifier with '_'. +static std::string

[PATCH] D114025: [clang][NFC] Inclusive terms: replace some uses of sanity in clang

2021-11-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114025/new/ https://reviews.llvm.org/D114025 _

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

2021-11-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:5973 +``mymemset(n, c, s)`` will diagnose overflows as if it were the call +``__builtin_memset(s, c, n)``; +}]; One thing that's not quite clear from the docs is how to handle

[PATCH] D114149: [clang-tidy] Fix pr48613: "llvm-header-guard uses a reserved identifier"

2021-11-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp:42-46 +/// Remove all '_' characters at the beginning of the identifier. Only reserved +/// identifiers are allowed to start with these. +static StringRef dropLeadingUnderscores(St

[PATCH] D114025: [clang][NFC] Inclusive terms: replace some uses of sanity in clang

2021-11-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D114025#3140161 , @martong wrote: > Do we have a comprehensive list of non-inclusive terms and their inclusive > correspondent somewhere available? > I mean `master` -> `main`, `white list` -> `inclusive list`, `sanity`

[PATCH] D114149: [clang-tidy] Fix pr48613: "llvm-header-guard uses a reserved identifier"

2021-11-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp:28 +/// Replace invalid characters in the identifier with '_'. +static std::string replaceInvalidChars(StringRef Identifier) { + std::string Fixed{Identifier}; sa

[PATCH] D113946: [libc][clang-tidy] fix namespace check for externals

2021-11-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:39 +// intercepted. +static const llvm::StringSet<> FUNCTIONS_TO_IGNORE_NAMESPACE = { +"__errno_location", "malloc", "calloc", "realloc", "free"};

[PATCH] D108643: Introduce _BitInt, deprecate _ExtInt

2021-11-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D108643#3093323 , @aaron.ballman wrote: > In D108643#3106438 , @aaron.ballman > wrote: > >> Ping > > Ping. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108643/

[PATCH] D114025: [clang][NFC] Inclusive terms: replace some uses of sanity in clang

2021-11-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. In D114025#3141414 , @keryell wrote: > In D114025#3140192 , @Quuxplusone > wrote: > >> I think "sanity-check" could be reasonably replaced with

[PATCH] D114639: Raise the minimum Visual Studio version to VS2019

2021-11-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Have you checked whether there are any bots in the lab that will need to be updated? Comment at: clang/docs/UsersManual.rst:3546 -cmake -G"Visual Studio 15 2017" -T LLVM .. +cmake -G"Visual Studio 17 2022" -T LLVM ..

[PATCH] D108643: Introduce _BitInt, deprecate _ExtInt

2021-11-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D108643#3140927 , @aaron.ballman wrote: > In D108643#3093323 , @aaron.ballman > wrote: > >> In D108643#3106438 , >> @aaron.ballman wro

[PATCH] D112881: [clang-tidy] Allow disabling integer to floating-point narrowing conversions for cppcoreguidelines-narrowing-conversions

2021-11-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, with a few small nits. It'd be nice to update the patch summary with more information about why the option is needed. Comment at: clang-tools-extra/docs/

<    46   47   48   49   50   51   52   53   54   55   >