[PATCH] D60573: [Sema] ADL: Associated namespaces for class types and enumeration types (CWG 1691)

rjmccall added a comment. In D60573#1463641 , @riccibruno wrote: > In D60573#1463569 , @rjmccall wrote: > > > Hmm. Does this never impact code that's just using a locally-defined type > > within its scope? I

[PATCH] D59756: [clangd] Support dependent bases in type hierarchy

nridge marked an inline comment as done. nridge added inline comments. Comment at: clang-tools-extra/unittests/clangd/TypeHierarchyTests.cpp:405 + *Result, + AllOf(WithName("S"), WithKind(SymbolKind::Struct), +Parents(AllOf(WithName("S"),

[PATCH] D58749: [index-while-building] IndexRecordHasher

jkorous added a comment. We did performance tests of alternative approach - just hashing the serialized bit code representation. There's a performance regression in the sense that while the current implementation costs approx. extra 2.2% in build time the alternative approach costs 3.8%. We

[PATCH] D60541: [clang-format] Use SpacesBeforeTrailingComments for "option" directive

dchai removed a subscriber: cfe-commits. dchai added a comment. Can someone please merge this for me? I don't have commit access. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60541/new/ https://reviews.llvm.org/D60541

r358245 - Enable frame pointer elimination for OpenBSD on x86 and mips64.

Author: brad Date: Thu Apr 11 18:29:18 2019 New Revision: 358245 URL: http://llvm.org/viewvc/llvm-project?rev=358245=rev Log: Enable frame pointer elimination for OpenBSD on x86 and mips64. Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp cfe/trunk/test/Driver/frame-pointer-elim.c

[PATCH] D60573: [Sema] ADL: Associated namespaces for class types and enumeration types (CWG 1691)

riccibruno added a comment. In D60573#1463611 , @rsmith wrote: > > I have applied the fix to all language versions, even though I think that > > the DR only applies to C++14 > > DRs don't have a specific version that they are intended to apply to; that's

[PATCH] D60573: [Sema] ADL: Associated namespaces for class types and enumeration types (CWG 1691)

riccibruno added a comment. In D60573#1463569 , @rjmccall wrote: > Hmm. Does this never impact code that's just using a locally-defined type > within its scope? I guess if ADL is involved, unqualified lookup must have > reached the scope of the

[PATCH] D60548: Variable auto-init: also auto-init alloca

jfb added a comment. I got an lgtm from @pcc on IRC, so I'll commit this. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60548/new/ https://reviews.llvm.org/D60548 ___ cfe-commits mailing list

[PATCH] D60548: Variable auto-init: also auto-init alloca

This revision was automatically updated to reflect the committed changes. Closed by commit rC358243: Variable auto-init: also auto-init alloca (authored by jfb, committed by ). Changed prior to commit: https://reviews.llvm.org/D60548?vs=194762=194787#toc Repository: rC Clang CHANGES SINCE

r358243 - Variable auto-init: also auto-init alloca

Author: jfb Date: Thu Apr 11 17:11:27 2019 New Revision: 358243 URL: http://llvm.org/viewvc/llvm-project?rev=358243=rev Log: Variable auto-init: also auto-init alloca Summary: alloca isn’t auto-init’d right now because it’s a different path in clang that all the other stuff we support (it’s a

[libunwind] r358242 - Creating release candidate final from release_710 branch

Author: tstellar Date: Thu Apr 11 16:41:27 2019 New Revision: 358242 URL: http://llvm.org/viewvc/llvm-project?rev=358242=rev Log: Creating release candidate final from release_710 branch Added: libunwind/tags/RELEASE_710/final/ - copied from r358241, libunwind/branches/release_70/

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

hintonda updated this revision to Diff 194782. hintonda added a comment. - Update isa_and_nonnull diagnostic message, and fix typo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59802/new/ https://reviews.llvm.org/D59802 Files:

[PATCH] D60573: [Sema] ADL: Associated namespaces for class types and enumeration types (CWG 1691)

rsmith accepted this revision. rsmith added a comment. > I have applied the fix to all language versions, even though I think that the > DR only applies to C++14 DRs don't have a specific version that they are intended to apply to; that's up to us to determine, and generally we apply them

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

hintonda marked an inline comment as done. hintonda added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.h:37-39 +/// if (var && isa(var)) {} +/// // is replaced by: +/// if (dyn_cast_or_null(var.foo())) {}

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 194781. gtbercea added a comment. - Handle OpenMP simd case. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntime.h

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

hintonda added a comment. In D60151#1463484 , @MyDeveloperDay wrote: > >> I suppose we could keep the names and directory structure and just change > >> the namespace. That would just be a special case in the scripts. Haven't > >> looked into it yet,

[PATCH] D60548: Variable auto-init: also auto-init alloca

jfb marked an inline comment as done. jfb added inline comments. Comment at: lib/CodeGen/PatternInit.h:22 + +llvm::Constant *patternFor(CodeGenModule&, llvm::Type*); + rjmccall wrote: > jfb wrote: > > rjmccall wrote: > > > Please choose names that mean something

[PATCH] D60573: [Sema] ADL: Associated namespaces for class types and enumeration types (CWG 1691)

rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Hmm. Does this never impact code that's just using a locally-defined type within its scope? I guess if ADL is involved, unqualified lookup must have reached the scope of the innermost

[PATCH] D60548: Variable auto-init: also auto-init alloca

rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. Comment at: lib/CodeGen/PatternInit.h:22 + +llvm::Constant *patternFor(CodeGenModule&, llvm::Type*); + jfb wrote: > rjmccall wrote: > > Please

Re: r358104 - Don't emit an unreachable return block.

Thanks, your fix looks good to me. - Marco. Il 11/04/19 05:05, John McCall ha scritto: > > > On 10 Apr 2019, at 21:50, wolfgang.p...@sony.com wrote: > >> Hi, >> This commit seems to be causing a test failure on several buildbots >> (e.g. >>

[PATCH] D59459: [analyzer][NFC] Prefer binary searches in CheckerRegistry

Szelethus marked an inline comment as done. Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:70 + Collection, Info, + FullNameLT{}); +} baloghadamsoftware wrote: > Please note that `llvm::lower_bound()` uses

[PATCH] D59459: [analyzer][NFC] Prefer binary searches in CheckerRegistry

Szelethus updated this revision to Diff 194768. Szelethus added a comment. Change asserts according to reviewer feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59459/new/ https://reviews.llvm.org/D59459 Files: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

MyDeveloperDay added a comment. >> I suppose we could keep the names and directory structure and just change >> the namespace. That would just be a special case in the scripts. Haven't >> looked into it yet, but will do so as soon as I can. > > Isn't that matching done on strings? I.e. is

[PATCH] D60573: [Sema] ADL: Associated namespaces for class types and enumeration types (CWG 1691)

riccibruno updated this revision to Diff 194765. riccibruno added a comment. Removed the call to `isTransparentContext()` in the loop in `CollectEnclosingNamespace`. It is not actually needed since we only care about finding the innermost enclosing namespace. Repository: rC Clang CHANGES

[PATCH] D60570: [Sema] Add more tests for the behavior of argument-dependent name lookup

riccibruno updated this revision to Diff 194763. riccibruno added a comment. Herald added a subscriber: eraman. Also test for inline namespaces. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60570/new/ https://reviews.llvm.org/D60570 Files:

[PATCH] D60548: Variable auto-init: also auto-init alloca

jfb marked 2 inline comments as done. jfb added inline comments. Comment at: lib/CodeGen/PatternInit.h:22 + +llvm::Constant *patternFor(CodeGenModule&, llvm::Type*); + rjmccall wrote: > Please choose names that mean something outside of the mental context you >

[PATCH] D60548: Variable auto-init: also auto-init alloca

jfb updated this revision to Diff 194762. jfb marked 2 inline comments as done. jfb added a comment. - Change name, qualify declaration. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60548/new/ https://reviews.llvm.org/D60548 Files: lib/CodeGen/CGBuiltin.cpp

RE: r357610 - Bug-40323: MS ABI adding template static member in the linker directive section to make sure init function can be called before main.

Thank you so much. Jennifer From: Nico Weber [mailto:tha...@chromium.org] Sent: Thursday, April 11, 2019 2:25 PM To: Yu, Jennifer Cc: cfe-commits Subject: Re: r357610 - Bug-40323: MS ABI adding template static member in the linker directive section to make sure init function can be called

Re: r357610 - Bug-40323: MS ABI adding template static member in the linker directive section to make sure init function can be called before main.

Done in 358232. Thanks! On Thu, Apr 11, 2019 at 5:19 PM Yu, Jennifer wrote: > Hi Nico, > > > > Iet’s revert it now. I will try to re-submit latter. > > Thanks. > > Jennifer > > > > *From:* Nico Weber [mailto:tha...@chromium.org] > *Sent:* Thursday, April 11, 2019 2:04 PM > *To:* Yu, Jennifer

r358232 - Revert r357610, it caused PR41471

Author: nico Date: Thu Apr 11 14:26:34 2019 New Revision: 358232 URL: http://llvm.org/viewvc/llvm-project?rev=358232=rev Log: Revert r357610, it caused PR41471 Removed: cfe/trunk/test/CodeGenCXX/microsoft-abi-template-static-init.cpp Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 194760. gtbercea added a comment. - Split patch. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntime.h

Re: r357340 - Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps.

I tried using this to see why X86ISelLowering.cpp takes so long to build on my system. The output json produced by this flag is at http://s000.tinyupload.com/?file_id=00019982161870973700 It looks like this produces lots of useful information for frontend time, but in this case most time is spent

RE: r357610 - Bug-40323: MS ABI adding template static member in the linker directive section to make sure init function can be called before main.

Hi Nico, Iet’s revert it now. I will try to re-submit latter. Thanks. Jennifer From: Nico Weber [mailto:tha...@chromium.org] Sent: Thursday, April 11, 2019 2:04 PM To: Yu, Jennifer Cc: cfe-commits Subject: Re: r357610 - Bug-40323: MS ABI adding template static member in the linker directive

r358231 - [C++20] Implement context-sensitive header-name lexing and pp-import parsing in the preprocessor.

Author: rsmith Date: Thu Apr 11 14:18:23 2019 New Revision: 358231 URL: http://llvm.org/viewvc/llvm-project?rev=358231=rev Log: [C++20] Implement context-sensitive header-name lexing and pp-import parsing in the preprocessor. Added: cfe/trunk/test/CXX/cpp/cpp.module/

r358230 - Remove use of lookahead from _Pragma handling and from all other

Author: rsmith Date: Thu Apr 11 14:18:22 2019 New Revision: 358230 URL: http://llvm.org/viewvc/llvm-project?rev=358230=rev Log: Remove use of lookahead from _Pragma handling and from all other internal lexing steps in the preprocessor. It is not safe to use the preprocessor's token lookahead

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 194758. gtbercea marked 2 inline comments as done. gtbercea added a comment. - Fix enum. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: lib/CodeGen/CGOpenMPRuntime.cpp

Re: r357610 - Bug-40323: MS ABI adding template static member in the linker directive section to make sure init function can be called before main.

Hi Jennifer, this caused https://bugs.llvm.org/show_bug.cgi?id=41471 Do you think this is easy to fix, or should we revert for now? Thanks, Nico On Wed, Apr 3, 2019 at 1:20 PM Jennifer Yu via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: jyu2 > Date: Wed Apr 3 10:21:40 2019 >

[PATCH] D60548: Variable auto-init: also auto-init alloca

rjmccall added inline comments. Comment at: lib/CodeGen/PatternInit.cpp:17 + +llvm::Constant *patternFor(CodeGenModule , llvm::Type *Ty) { + // The following value is a guaranteed unmappable pointer value and has a Please use a qualified declaration here

[PATCH] D59221: [asan] Add gcc 8's driver option -fsanitize=pointer-compare and -fsanitize=pointer-substract.

thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land. Seems like harmless plumbing. Given that the LLVM side of this is accepted, this lgtm. Comment at: test/Driver/fsanitize.c:837 +// RUN: %clang -target x86_64-linux-gnu

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea marked 4 inline comments as done. gtbercea added inline comments. Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:3870 llvm::Function * +CGOpenMPRuntime::createRequiresDirectiveRegistration() { + // If we don't have entries or if we are emitting code for the device, we

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 194743. gtbercea marked 6 inline comments as done. gtbercea added a comment. - Address comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files:

[PATCH] D59221: [asan] Add gcc 8's driver option -fsanitize=pointer-compare and -fsanitize=pointer-substract.

riccibruno resigned from this revision. riccibruno added a comment. I am not the best person to review this, sorry! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59221/new/ https://reviews.llvm.org/D59221 ___ cfe-commits mailing list

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea updated this revision to Diff 194741. gtbercea added a comment. - Address comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60568/new/ https://reviews.llvm.org/D60568 Files: lib/CodeGen/CGOpenMPRuntime.cpp lib/CodeGen/CGOpenMPRuntime.h

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

riccibruno added a comment. Friendly ping. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57660/new/ https://reviews.llvm.org/D57660 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D59347: [DebugInfo] Combine Trivial and NonTrivial flags

This revision was automatically updated to reflect the committed changes. Closed by commit rL358219: [DebugInfo] Combine Trivial and NonTrivial flags (authored by asmith, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

r358219 - [DebugInfo] Combine Trivial and NonTrivial flags

Author: asmith Date: Thu Apr 11 13:24:54 2019 New Revision: 358219 URL: http://llvm.org/viewvc/llvm-project?rev=358219=rev Log: [DebugInfo] Combine Trivial and NonTrivial flags Summary: These flags are used when emitting debug info and needed to initialize subprogram and member function

[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

rsmith added a comment. This seems liable to be moderately expensive, especially for large argument values, and it's a cost that is unnecessary in the common case where evaluation succeeds. I wonder if we'd be better off speculatively trying the entire evaluation without storing any such

[PATCH] D60548: Variable auto-init: also auto-init alloca

jfb marked 2 inline comments as done. jfb added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:57 + case LangOptions::TrivialAutoVarInitKind::Pattern: +Byte = CGF.Builder.getInt8(0xAA); +break; rjmccall wrote: > Can this value be taken from some

[PATCH] D60548: Variable auto-init: also auto-init alloca

jfb updated this revision to Diff 194736. jfb added a comment. Herald added a subscriber: mgorny. - Move patternFor to a shared file as requested by @rjmccall Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60548/new/ https://reviews.llvm.org/D60548 Files:

[PATCH] D60548: Variable auto-init: also auto-init alloca

rjmccall added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:57 + case LangOptions::TrivialAutoVarInitKind::Pattern: +Byte = CGF.Builder.getInt8(0xAA); +break; Can this value be taken from some common source with the existing code? I mean,

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

ztamas added a comment. I'll update the patch based on the comments. Just a note about the false positive ratio. In LibreOffice we run other static analyzers too, that's why there were less positive catches there than false positives. For example, PVS studio also catches unprotected copy

[PATCH] D60548: Variable auto-init: also auto-init alloca

jfb added a comment. In D60548#1462124 , @jfb wrote: > One downside to alloca is that we can's use `__attribute__((uninitialized))` > because it's a builtin. Maybe we need a separate uninitialized alloca? What > do you all think? Actually I'm

[PATCH] D60548: Variable auto-init: also auto-init alloca

jfb added a comment. In D60548#1463181 , @jfb wrote: > In D60548#1462124 , @jfb wrote: > > > One downside to alloca is that we can's use > > `__attribute__((uninitialized))` because it's a builtin. Maybe we need a

[PATCH] D60572: creduce-clang-crash: Use `!` instead of `not`

akhuang added a comment. Looks good- if we remove `not` there are a few other things that should be removed Comment at: clang/utils/creduce-clang-crash.py:138 if "fatal error:" in msg_re: self.is_crash = False break also wouldn't

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

hintonda added a comment. @klimek, this patch is now ready for review. Thanks... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59746/new/ https://reviews.llvm.org/D59746 ___ cfe-commits mailing list

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

hintonda updated this revision to Diff 194729. hintonda added a comment. Enhance Option::reset to reset the default value and remove the option if it's a cl::DefaultOption. Also, make sure removeOption only removes an option if both the name and the Option matches, not just the name -- different

[PATCH] D60573: [Sema] ADL: Associated namespaces for class types and enumeration types (CWG 1691)

riccibruno created this revision. riccibruno added reviewers: rjmccall, Quuxplusone. riccibruno added a project: clang. Herald added a subscriber: cfe-commits. riccibruno added a parent revision: D60570: [Sema] Add more tests for the behavior of argument-dependent name lookup. CWG 1691 changed

[PATCH] D60572: creduce-clang-crash: Use `!` instead of `not`

thakis abandoned this revision. thakis added a comment. This seems to break the script, nevermind! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60572/new/ https://reviews.llvm.org/D60572 ___ cfe-commits mailing list

[PATCH] D60572: creduce-clang-crash: Use `!` instead of `not`

thakis created this revision. thakis added a reviewer: akhuang. The run script runs in bash, so no need to depend on llvm's `not` binary. https://reviews.llvm.org/D60572 Files: clang/utils/creduce-clang-crash.py Index: clang/utils/creduce-clang-crash.py

[PATCH] D60570: [Sema] Add more tests for the behavior of argument-dependent name lookup

riccibruno created this revision. riccibruno added reviewers: rjmccall, Quuxplusone. riccibruno added a project: clang. Herald added a subscriber: cfe-commits. The goal here is to exercise each rule in [basic.lookup.argdep] at least once. These new tests expose what I believe are 2 issues: 1.

Re: [PATCH] D59605: [clangd] Introduce background-indexer

Thanks - I think there's quite a lot of background missing here: what/who is this for, why a separate binary from clangd-indexer (or and how do they relate, how much of the BackgroundIndex code structure makes sense with multiple users. I assume there's been some offline discussion about this,

[PATCH] D60544: Support objc_nonlazy_class attribute on Objective-C implementations

This revision was automatically updated to reflect the committed changes. Closed by commit rL358201: Support objc_nonlazy_class attribute on Objective-C implementations (authored by epilk, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D60542: Add support for attributes on @implementations in Objective-C

This revision was automatically updated to reflect the committed changes. Closed by commit rC358200: Add support for attributes on @implementations in Objective-C (authored by epilk, committed by ). Changed prior to commit: https://reviews.llvm.org/D60542?vs=194594=194716#toc Repository: rC

r358200 - Add support for attributes on @implementations in Objective-C

Author: epilk Date: Thu Apr 11 10:55:30 2019 New Revision: 358200 URL: http://llvm.org/viewvc/llvm-project?rev=358200=rev Log: Add support for attributes on @implementations in Objective-C We want to make objc_nonlazy_class apply to implementations, but ran into this. There doesn't seem to be

r358201 - Support objc_nonlazy_class attribute on Objective-C implementations

Author: epilk Date: Thu Apr 11 10:55:34 2019 New Revision: 358201 URL: http://llvm.org/viewvc/llvm-project?rev=358201=rev Log: Support objc_nonlazy_class attribute on Objective-C implementations Fixes rdar://49523079 Differential revision: https://reviews.llvm.org/D60544 Modified:

[PATCH] D60544: Support objc_nonlazy_class attribute on Objective-C implementations

rjmccall accepted this revision. rjmccall added a comment. Great, SGTM. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60544/new/ https://reviews.llvm.org/D60544 ___ cfe-commits mailing list

[PATCH] D60548: Variable auto-init: also auto-init alloca

pcc added a comment. I probably wouldn't do anything about suppressing init on alloca for now, but if we did do something I think I'd be in favour of the separate builtin for uninitialized alloca. I also considered the alternative of allowing `__attribute__((uninitialized))` to appear on a

[PATCH] D60542: Add support for attributes on @implementations in Objective-C

aaron.ballman added inline comments. Comment at: clang/test/FixIt/fixit-pragma-attribute.cpp:19-20 #pragma clang attribute push (__attribute__((annotate("subRuleContradictions"))), apply_to = any(variable, variable(is_parameter), function(is_member), variable(is_global))) -//

[PATCH] D60544: Support objc_nonlazy_class attribute on Objective-C implementations

erik.pilkington added a comment. In D60544#1462869 , @rjmccall wrote: > Should we have a special `has_feature` test to check that this attribute is > allowed on implementations? We've done that in the past when expanding the > set of subjects for an

[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace

aaron.ballman added inline comments. Comment at: clang-tidy/abseil/SafelyScopedCheck.cpp:37 + diag(MatchedDecl->getLocation(), + "using declaration %0 not declared in the innermost namespace.") + << MatchedDecl; You should remove the full stop at the

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

lebedev.ri added a comment. In D60151#1462976 , @hintonda wrote: > In D60151#1462850 , @MyDeveloperDay > wrote: > > > are we supporting "-llvm-*" in existing .clang-tidy files? > > > > If people selectively turn

[PATCH] D60542: Add support for attributes on @implementations in Objective-C

erik.pilkington marked an inline comment as done. erik.pilkington added inline comments. Comment at: clang/test/FixIt/fixit-pragma-attribute.cpp:19-20 #pragma clang attribute push (__attribute__((annotate("subRuleContradictions"))), apply_to = any(variable,

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

hintonda added a comment. In D60151#1462850 , @MyDeveloperDay wrote: > are we supporting "-llvm-*" in existing .clang-tidy files? > > If people selectively turn checkers off, won't all of a sudden everyone start > getting llvm-project checks and fixes

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst:10 + +This check corresponds to the CERT C++ Coding Standard rule +`OOP54-CPP. Gracefully handle self-copy assignment Eugene.Zelenko

[PATCH] D55409: [clang-tidy] check for using declarations not in an anonymous namespace when there exists one

aaron.ballman added inline comments. Comment at: clang-tidy/abseil/AnonymousEnclosedAliasesCheck.cpp:41 +// to the vector containing all candidate using declarations. +if (AnonymousNamespaceDecl) { + diag(MatchedUsingDecl->getLocation(),

[PATCH] D60543: [clang] Add matcher for subclasses of Objective-C interfaces 

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:1479 +/// \endcode +AST_MATCHER_P(ObjCInterfaceDecl, isSubclassOfInterface, + internal::Matcher, stephanemoore wrote: > I am still uncertain about the

[libunwind] r358191 - [NFC] Correct outdated links to the Itanium C++ ABI documentation

Author: ldionne Date: Thu Apr 11 09:37:07 2019 New Revision: 358191 URL: http://llvm.org/viewvc/llvm-project?rev=358191=rev Log: [NFC] Correct outdated links to the Itanium C++ ABI documentation Those are now hosted on GitHub. rdar://problem/36557462 Modified:

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1000 +def SYCLDevice : InheritableAttr { + let Spellings = [GNU<"sycl_device">]; + let Subjects = SubjectList<[Function, Var]>; keryell wrote: > Fznamznon wrote: > >

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp:125 +diag(MatchedDecl->getBeginLoc(), + "isa_and_nonnull<> is cleaner and more efficient") +<<

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

klimek added a comment. In D54881#1462893 , @MyDeveloperDay wrote: > > I agree and would be happy with the change if it would only change the > > line-filtered workflow, but this afaict (unless I'm missing something :) > > will also affect the workflow

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

ABataev added inline comments. Comment at: lib/CodeGen/CGDecl.cpp:2576 void CodeGenModule::EmitOMPRequiresDecl(const OMPRequiresDecl *D) { - getOpenMPRuntime().checkArchForUnifiedAddressing(D); } You don't need to pass the reference to CodeGenModule to

[PATCH] D59605: [clangd] Introduce background-indexer

ilya-biryukov added a comment. To make it clear, I think the question is not just "which part of functionality is missing in BackgroundIndex", it's rather "which part of BackgroundIndex we **don't** need". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

hintonda updated this revision to Diff 194708. hintonda added a comment. - Add unittest. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59746/new/ https://reviews.llvm.org/D59746 Files: clang/lib/Tooling/CommonOptionsParser.cpp

[PATCH] D59605: [clangd] Introduce background-indexer

ilya-biryukov added a subscriber: sammccall. ilya-biryukov added a comment. To reiterate the offline discussion: the tool seems useful to me, but it would be best to keep the client code simpler, it's currently fighting with `BackgroundIndex` because it's trying to hijack some of its

[PATCH] D60544: Support objc_nonlazy_class attribute on Objective-C implementations

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, modulo the `__has_feature` question. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60544/new/ https://reviews.llvm.org/D60544

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

MyDeveloperDay added a comment. > I agree and would be happy with the change if it would only change the > line-filtered workflow, but this afaict (unless I'm missing something :) will > also affect the workflow where the provided range is 0-length range, which > has an implicit "format

[PATCH] D60542: Add support for attributes on @implementations in Objective-C

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from a question. Comment at: clang/test/FixIt/fixit-pragma-attribute.cpp:19-20 #pragma clang attribute push (__attribute__((annotate("subRuleContradictions"))), apply_to = any(variable,

[PATCH] D60544: Support objc_nonlazy_class attribute on Objective-C implementations

rjmccall added a comment. Should we have a special `has_feature` test to check that this attribute is allowed on implementations? We've done that in the past when expanding the set of subjects for an attribute. But that's not necessary if we haven't made this attribute part of a public Xcode

[PATCH] D59605: [clangd] Introduce background-indexer

kadircet marked an inline comment as done. kadircet added a comment. Looking at the current state of `BackgroundIndex`, it has the following implementation details: - Loading of shards from storage - Storing of shards to storage - Collecting symbols from a TU - Sharding of symbol information

[PATCH] D60139: [clang-tidy] Add bugprone-placement-new-target-type-mismatch check

aaron.ballman added a comment. In D60139#1461269 , @DennisL wrote: > In D60139#1460233 , @JonasToth wrote: > > > Hey Dennis, > > > > my 2cents on the check. I think it is very good to have! Did you check > >

[PATCH] D60542: Add support for attributes on @implementations in Objective-C

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

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

MyDeveloperDay added a comment. are we supporting "-llvm-*" in existing .clang-tidy files? If people selectively turn checkers off, won't all of a sudden everyone start getting llvm-project checks and fixes turned back on? https://github.com/search?q=-llvm-%2A=Code maybe we need to add

[PATCH] D60568: [OpenMP] Add support for registering requires directives with the runtime

gtbercea created this revision. gtbercea added reviewers: ABataev, AlexEichenberger, caomhin. Herald added subscribers: cfe-commits, jdoerfert, jfb, guansong, jholewinski. Herald added a project: clang. This patch adds support for the registration of the requires directives with the runtime.

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

klimek added a comment. In D54881#1462804 , @russellmcc wrote: > Thanks for the explanation! I do understand your philosophy on this, and > agree with the desired behavior case you brought up where you have put in new > braces. > > After thinking about

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

aaron.ballman added a comment. In D60151#1454802 , @hintonda wrote: > In D60151#1454741 , @alexfh wrote: > > > In D60151#1454105 , @hintonda > > wrote: > > > > > - Rename

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

russellmcc added a comment. Thanks for the explanation! I do understand your philosophy on this, and agree with the desired behavior case you brought up where you have put in new braces. After thinking about this more, the thing I really care about is that clang-format is idempotent with a

[PATCH] D59639: [clangd] Print template arguments helper

kadircet added inline comments. Comment at: clang-tools-extra/clangd/AST.cpp:139 + // location information. + printTemplateArgumentList(OS, Cls->getTemplateArgs().asArray(), Policy); +} ioeric wrote: > Could you also add a test case for this with

[PATCH] D59639: [clangd] Print template arguments helper

kadircet updated this revision to Diff 194686. kadircet marked 6 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59639/new/ https://reviews.llvm.org/D59639 Files:

[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

Tyker updated this revision to Diff 194684. Tyker added a comment. @Quuxplusone edited based of feedback i simplified the test but didn't put the original because this one test arguments for variable and literal and there may be corner case differences CHANGES SINCE LAST ACTION

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

ilya-biryukov updated this revision to Diff 194683. ilya-biryukov added a comment. - Store a source manager in a token buffer Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59887/new/ https://reviews.llvm.org/D59887 Files:

  1   2   >