Re: [PATCH] D10431: PR21174 - clang only searches current working directory for precompiled include file

On Tue, Aug 18, 2015 at 2:00 AM, Richard Smith rich...@metafoo.co.uk wrote: Should this logic for PCH/PTH scanning move out of the driver and into the frontend? gcc also checks if the gch file is a directory and if so finds the first file with matching language / defines etc. It also uses

Re: [PATCH] D10431: PR21174 - clang only searches current working directory for precompiled include file

kimgr added inline comments. Comment at: lib/Driver/Tools.cpp:398 @@ +397,3 @@ + FoundPTH = !UsePCH; +} + } kimgr wrote: kimgr wrote: kimgr wrote: Add a `break;` here so we don't continue searching after a valid path has

Re: [PATCH] D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk

kimgr added a subscriber: kimgr. kimgr added a comment. Add debugging ideas. Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:86 @@ -69,1 +85,3 @@ +IComponentModel componentModel = GetService(typeof(SComponentModel)) as IComponentModel; +

Re: [PATCH] D12759: [clang-tidy] Add misc-sizeof-container check to find sizeof() uses on stlcontainers.

Late to the party, but I wanted to ask: is there a way to indicate to the checker that we really *did* mean sizeof()? I think I've stumbled over code in our code base that uses sizeof(container) to report memory usage statistics and it seems valid, so it'd be nice if this checker could be

Re: r248984 - Test fix

On Fri, Oct 2, 2015 at 3:09 AM, Richard Smith via cfe-commits wrote: > On Thu, Oct 1, 2015 at 6:01 AM, Renato Golin via cfe-commits > wrote: >> >> Right, I reverted both commits on r249005. Please, let me know if you >> need help testing on

Re: [PATCH] D9600: Add scan-build python implementation

kimgr added a subscriber: kimgr. kimgr added a comment. > > Comment at: tools/scan-build-py/bin/analyze-cc:14 > @@ +13,2 @@ > +from libscanbuild.analyze import wrapper > +sys.exit(wrapper(False)) > > > > It is hard to figure out/search which

Re: r272247 - [Sema] Don't crash when a field w/ a mem-initializer clashes with a record name

On Thu, Jun 9, 2016 at 7:26 AM, David Majnemer via cfe-commits wrote: > Author: majnemer > Date: Thu Jun 9 00:26:56 2016 > New Revision: 272247 > > URL: http://llvm.org/viewvc/llvm-project?rev=272247=rev > Log: > [Sema] Don't crash when a field w/ a mem-initializer

Re: r272247 - [Sema] Don't crash when a field w/ a mem-initializer clashes with a record name

On Thu, Jun 9, 2016 at 4:51 PM, David Majnemer wrote: > It would mean that the instantiation of the class template gained a field > which should be impossible. OK, so assert(Lookup.size() > 0) always holds? Makes sense, thanks. - Kim

Re: r258768 - [Sema] Incomplete types are OK for covariant returns

On Sun, Jan 31, 2016 at 5:24 PM, Kim Gräsman wrote: > On Sun, Jan 31, 2016 at 2:50 PM, David Majnemer > wrote: >> >> It is the same issue as CWG defect report 1250: >> http://wg21.cmeerw.net/cwg/issue1250 >> >> I forget how to tell how far back a

Re: [PATCH] D16259: Add clang-tidy readability-redundant-control-flow check

On Mon, Feb 1, 2016 at 4:32 PM, Aaron Ballman wrote: > > > Comment at: clang-tidy/readability/RedundantControlFlowCheck.cpp:60 > @@ +59,3 @@ > + if (const auto *Return = dyn_cast(*last)) > +issueDiagnostic(Result, Block, Return->getSourceRange(), > +

Re: r258768 - [Sema] Incomplete types are OK for covariant returns

Hi David, Should this be guarded by if(cxx14)? I think the complete type was required by earlier standards. - Kim Den 26 jan 2016 2:40 fm skrev "David Majnemer via cfe-commits" < cfe-commits@lists.llvm.org>: > Author: majnemer > Date: Mon Jan 25 19:37:01 2016 > New Revision: 258768 > > URL:

Re: r258768 - [Sema] Incomplete types are OK for covariant returns

On Sun, Jan 31, 2016 at 2:50 PM, David Majnemer wrote: > > It is the same issue as CWG defect report 1250: > http://wg21.cmeerw.net/cwg/issue1250 > > I forget how to tell how far back a DR applies but I'd guess this one goes > as far back as C++11. Thanks, I didn't know

Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

kimgr added inline comments. Comment at: test/clang-tidy/readability-redundant-return.cpp:21-24 @@ +20,6 @@ + +void g(int i) { + if (i < 0) { +return; + } + if (i < 10) { LegalizeAdulthood wrote: > kimgr wrote: > > What happens to guard clauses invoking

Re: [PATCH] D16259: Add clang-tidy readability-redundant-return check

kimgr added a subscriber: kimgr. kimgr added a comment. Came up with another test case. Comment at: test/clang-tidy/readability-redundant-return.cpp:21-24 @@ +20,6 @@ + +void g(int i) { + if (i < 0) { +return; + } + if (i < 10) { What happens to guard

Re: [PATCH] D16535: [clang-tidy] Check to find unintended semicolons that changes the semantics.

kimgr added a subscriber: kimgr. kimgr added a comment. Cool check, I like how it pays attention to indentation! I found some minor doc nits, see inline. Comment at: docs/clang-tidy/checks/misc-suspicious-semicolon.rst:8-9 @@ +7,4 @@ +the code. More specifically, it looks for

Re: [PATCH] D18852: [clang-tidy] fix a crash with -fdelayed-template-parsing in UnnecessaryValueParamCheck.

kimgr added inline comments. Comment at: test/clang-tidy/performance-unnecessary-value-param-delayed.cpp:1 @@ +1,2 @@ +// RUN: %check_clang_tidy %s performance-unnecessary-value-param %t -- -- -fdelayed-template-parsing + etienneb wrote: > kimgr wrote: > > Are

Re: [PATCH] D18852: [clang-tidy] fix a crash with -fdelayed-template-parsing in UnnecessaryValueParamCheck.

kimgr added a comment. The RUN line looks weird to me, but I'm no lit expert... Comment at: test/clang-tidy/performance-unnecessary-value-param-delayed.cpp:1 @@ +1,2 @@ +// RUN: %check_clang_tidy %s performance-unnecessary-value-param %t -- -- -fdelayed-template-parsing +

Re: [PATCH] D18852: [clang-tidy] fix a crash with -fdelayed-template-parsing in UnnecessaryValueParamCheck.

kimgr added a subscriber: kimgr. kimgr added a comment. A test case would be nice here. You can repro on all systems by passing `-fdelayed-template-parsing` explicitly. http://reviews.llvm.org/D18852 ___ cfe-commits mailing list

Re: [PATCH] D19816: [find-all-symbols] Add IWYU private pragma support.

Re semantics, you may want to link to IWYU's docs at https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md . - Kim Den 3 maj 2016 6:34 em skrev "Manuel Klimek via cfe-commits" < cfe-commits@lists.llvm.org>: > klimek added inline comments. > >

Re: [PATCH] D19816: [find-all-symbols] Add IWYU private pragma support.

kimgr added a subscriber: kimgr. kimgr added a comment. Re semantics, you may want to link to IWYU's docs at https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md . - Kim Den 3 maj 2016 6:34 em skrev "Manuel Klimek via cfe-commits" <

Re: [PATCH] D21459: Implement http://wg21.link/P0254R1: "Integrating std::string_view and std::string"

kimgr added a comment. This is probably not the right place for this discussion, but I thought I'd offer one more note. Comment at: include/string_view:216 @@ +215,3 @@ + basic_string_view(const _CharT* __s) + : __data(__s), __size(_Traits::length(__s)) {}

Re: [PATCH] D21459: Implement http://wg21.link/P0254R1: "Integrating std::string_view and std::string"

kimgr added inline comments. Comment at: include/string_view:216 @@ +215,3 @@ + basic_string_view(const _CharT* __s) + : __data(__s), __size(_Traits::length(__s)) {} + mclow.lists wrote: > mclow.lists wrote: > > kimgr wrote: > > > mclow.lists

Re: [PATCH] D21459: Implement http://wg21.link/P0254R1: "Integrating std::string_view and std::string"

kimgr added a subscriber: kimgr. kimgr added a comment. Inline question on ctor+nullptr Comment at: include/string_view:216 @@ +215,3 @@ + basic_string_view(const _CharT* __s) + : __data(__s), __size(_Traits::length(__s)) {} + I'm working

Re: [PATCH] D21459: Implement http://wg21.link/P0254R1: "Integrating std::string_view and std::string"

kimgr added inline comments. Comment at: include/string_view:216 @@ +215,3 @@ + basic_string_view(const _CharT* __s) + : __data(__s), __size(_Traits::length(__s)) {} + mclow.lists wrote: > kimgr wrote: > > I'm working from the paper at

Re: [PATCH] D22087: [clang-rename] add basic vim integration

kimgr added a subscriber: kimgr. kimgr added a comment. I only caught this typo after it was committed. Comment at: clang-tools-extra/trunk/clang-rename/tool/clang-rename.py:17-18 @@ +16,4 @@ +All you have to do now is to place a cursor on a variable/function/class which +you

Re: r292558 - Add documentation for constexpr string builtin support.

Hi Richard, On Fri, Jan 20, 2017 at 1:58 AM, Richard Smith via cfe-commits wrote: > > +String builtins > +--- > + > +Clang provides constant expression evaluation support for builtins forms of > +the following functions from the C standard library

Re: r296193 - [Test] Make Lit tests C++11 compatible #10

Den 25 feb. 2017 7:23 em skrev "Charles Li via cfe-commits" < cfe-commits@lists.llvm.org>: Author: lcharles Date: Fri Feb 24 17:23:53 2017 New Revision: 296193 URL: http://llvm.org/viewvc/llvm-project?rev=296193=rev Log: [Test] Make Lit tests C++11 compatible #10 Differential Revision:

Re: [PATCH] D23895: [ms] Add support for parsing uuid as a MS attribute

Microsoft used to call their IDL dialect 'MIDL' (which is kind of punny), don't know if it makes sense to stick with that over 'MSIDL'. - Kim Den 26 aug. 2016 4:09 fm skrev "Nico Weber via cfe-commits" < cfe-commits@lists.llvm.org>: > thakis updated this revision to Diff 69312. > thakis marked

Re: [clang-tools-extra] r284894 - [Release notes] Mention removed Clang-tidy misc-pointer-and-integral-operation check

Hi Eugene, On Sat, Oct 22, 2016 at 12:35 AM, Eugene Zelenko via cfe-commits wrote: > Author: eugenezelenko > Date: Fri Oct 21 17:35:58 2016 > New Revision: 284894 > > URL: http://llvm.org/viewvc/llvm-project?rev=284894=rev > Log: > [Release notes] Mention removed

Re: [clang-tools-extra] r304583 - [clang-tidy] Add `const` to operator() to fix a warning.

This is even an error in VS2017, I've just fixed a number of instances of this in an internal codebase. - Kim Den 6 juni 2017 4:32 em skrev "Alexander Kornienko via cfe-commits" < cfe-commits@lists.llvm.org>: > On Mon, Jun 5, 2017 at 7:11 PM, David Blaikie wrote: > >> what

Re: [PATCH] D32696: More detailed docs for UsingShadowDecl

Ping! Any takers? Thanks, - Kim On Mon, May 1, 2017 at 10:27 AM, Kim Gräsman via Phabricator wrote: > kimgr created this revision. > > I couldn't make sense of the docs before, so I sent a question to cfe-dev. > Richard was gracious enough to fill in the blanks: >

Re: [clang-tools-extra] r318809 - Silence some MSVC warnings about not all control paths returning a value; NFC.

Den 21 nov. 2017 11:24 em skrev "Aaron Ballman via cfe-commits" < cfe-commits@lists.llvm.org>: Author: aaronballman Date: Tue Nov 21 14:24:13 2017 New Revision: 318809 URL: http://llvm.org/viewvc/llvm-project?rev=318809=rev Log: Silence some MSVC warnings about not all control paths returning a

Re: [PATCH] D39360: [C++11] Don't put empty quotes in static_assert diagnostic.

A clang-tidy check to remove empty messages from source would be nice, though... - Kim Den 27 okt. 2017 10:24 fm skrev "Nicolas Lesser via Phabricator" < revi...@reviews.llvm.org>: > Rakete abandoned this revision. > Rakete added a comment. > > @kimgr Well, mostly because they bother me

Re: r321312 - [AST] Incorrectly qualified unscoped enumeration as template actual parameter.

This broke a test case in IWYU, so I read the diff a few times more than usual... On Thu, Dec 21, 2017 at 10:47 PM, Paul Robinson via cfe-commits wrote: > >// scope of the enumeration. > - if (ED->isScoped() || ED->getIdentifier()) > + // For the

Re: r337381 - Mention clang-cl improvements from r335466 and r336379 in ReleaseNotes.rst

This is lovely! Found a bit inline... On Wed, Jul 18, 2018, 14:00 Nico Weber via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: nico > Date: Wed Jul 18 04:55:03 2018 > New Revision: 337381 > > URL: http://llvm.org/viewvc/llvm-project?rev=337381=rev > Log: > Mention clang-cl

Re: [PATCH] D30170: Function definition may have uninstantiated body

On Wed, Feb 28, 2018 at 8:21 PM, Richard Smith - zygoloid via Phabricator via cfe-commits wrote: > > Comment at: lib/Sema/SemaDecl.cpp:11986 > + !FD->isDefined(Definition) > + && FD->getDeclContext()->isFileContext()) { > +// If this is a

Re: [PATCH] D43578: -ftime-report switch support in Clang

Hi Richard, On Sat, Apr 14, 2018 at 1:53 AM, Richard Smith - zygoloid via Phabricator via cfe-commits wrote: > rsmith added a comment. > > So I don't think this patch is reasonable for that reason. I'm also not sure > whether this, as is, is addressing a pressing use

Re: r330068 - [Serialization] Fix some Clang-tidy modernize and Include What You Use warnings; other minor fixes (NFC).

That would be a nice outcome of all the "run-tools-on-llvm" changes if any problems were filed as bugs on the tools. We have a number of them filed on iwyu, and they make for nice, concrete bugs to troubleshoot even if we don't always know how to fix them. For this specific clang-tidy issue, do

Re: [PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

I have to say I disagree that either the nested struct/function or macros (in any form) should count toward a function's total variable count. Both are valid forms of abstraction, and they both remove complexity from the containing function since they factor details *out of the function's

Re: [PATCH] D53354: [WIP][NOT FOR COMMIT][PROTOTYPE] clang-scan-deps: dependency scanning tool rough prototype

Hi Alex, On Fri, Oct 19, 2018 at 1:11 AM Alex Lorenz via Phabricator via cfe-commits wrote: > > > Have you checked the tool //Include What You Use//? I'm curious in what way > > the mishappenings of that tool present themselves in yours. There were some > > challenges not overcome in a good

Re: [PATCH] D61909: Add Clang shared library with C++ exports

The Clang module libraries are all called libClang[A-Z][a-zA-Z]+.{a,so}, so libclangcpp doesn't conflict with that, but I wonder if a dash would set it apart even more clearly: libclang-cpp. Or something like clang-all to show that it houses all clang modules? Bikesheds are the best sheds. -

Re: r368874 - Document clang-cpp in the release notes for clang

On Wed, Aug 14, 2019 at 6:48 PM Chris Bieneman via cfe-commits wrote: > > Author: cbieneman > Date: Wed Aug 14 09:49:52 2019 > New Revision: 368874 > -- ... > +- In 9.0.0 and later Clang added a new target, clang-cpp, which generates a > + shared library comprised of all the clang component

Re: [clang] 3ced239 - Refactor CompareReferenceRelationship and its callers in preparation for

Yes, it is this change that broke us. I still don't fully understand what the change was, but it appears to mess things up for IWYU for operator== with templates and const, somehow. Could you expand on what changed here and how the AST might be affected? Thanks, - Kim On Sat, Dec 28, 2019 at

Re: [clang] 3ced239 - Refactor CompareReferenceRelationship and its callers in preparation for

Hi Richard, I see the commit message mentions type sugar here; does this change affect the AST at all? We're seeing test failures in IWYU based on recent Clang, and I'm suspecting this commit (it takes me a while to bisect because of Clang build times on my laptop). Thanks, - Kim On Wed, Dec

[clang] Rework the printing of attributes (PR #87281)

kimgr wrote: A note from left field: I think this PR broke the IWYU test suite. We use `TemplateDecl::print` + some postprocessing to generate template forward-declarations. We're seeing this diff now: ```diff -template class FinalTemplate; +template class FinalTemplate; ``` So a spurious

[clang] Rework the printing of attributes (PR #87281)

kimgr wrote: @erichkeane Thanks! Let me just see if I can bisect it to this commit; I don't have any evidence yet, this PR was just the only significant change to `DeclPrinter.cpp` in the past few days. I need a little while to rebuild my local Clang tree with and without the patch.

[clang] Rework the printing of attributes (PR #87281)

kimgr wrote: I can confirm that the double space comes from this PR; ```diff diff --git a/clang/unittests/AST/DeclPrinterTest.cpp b/clang/unittests/AST/DeclPrinterTest.cpp index c24e442621c9..c2d02e74a62c 100644 --- a/clang/unittests/AST/DeclPrinterTest.cpp +++

[clang] Rework the printing of attributes (PR #87281)

kimgr wrote: (obtw, the double `final` is unrelated, it's tracked here: https://github.com/llvm/llvm-project/issues/56517) https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[clang] Fix the double space and double attribute printing of the final keyword. (PR #88600)

kimgr wrote: Thank you. I believe this should cover both cases, added some attempt at rationale in comments: ```diff diff --git a/clang/unittests/AST/DeclPrinterTest.cpp b/clang/unittests/AST/DeclPrinterTest.cpp index f2b027a25621..8691a6c38f16 100644 ---

[clang] Fix the double space and double attribute printing of the final keyword. (PR #88600)

kimgr wrote: > With .PolishForDeclaration=true, there are NO final specifiers (which is what > we want to produce forward decls in IWYU) This is actually a regression in this PR, and it breaks the clangd test added here:

[clang] Fix the double space and double attribute printing of the final keyword. (PR #88600)

kimgr wrote: Thanks! Could you add this DeclPrinterTest unittest for regression? ``` TEST(DeclPrinter, TestTemplateFinal) { ASSERT_TRUE(PrintedDeclCXX11Matches( "template" "class FinalTemplate final {};", classTemplateDecl(hasName("FinalTemplate")).bind("id"), "template class

[clang] Fix the double space and double attribute printing of the final keyword. (PR #88600)

kimgr wrote: > I am wondering if that'd be interesting and if so, maybe we can share it > between both projects via upstreaming to Clang... That sounds fantastic, but mostly because I don't have anything to offer, and everything to benefit :) I was just thinking about adding a

[clang] Fix the double space and double attribute printing of the final keyword. (PR #88600)

@@ -252,16 +252,19 @@ static DeclPrinter::AttrPosAsWritten getPosAsWritten(const Attr *A, return DeclPrinter::AttrPosAsWritten::Right; } -void DeclPrinter::prettyPrintAttributes(const Decl *D, +// returns true if an attribute was printed. +bool

[clang] Fix the double space and double attribute printing of the final keyword. (PR #88600)

kimgr wrote: Current PR passes all my tests, both Clang (`ninja ASTTests`), Clangd (`ninja ClangdTests`) and IWYU end-to-end tests -- thanks! https://github.com/llvm/llvm-project/pull/88600 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[clang] Fix the double space and double attribute printing of the final keyword. (PR #88600)

kimgr wrote: > If the intent is to produce a forward declaration the final keyword cannot be > attached to a forward declaration. So I am not sure what's the "right" fix > here... I don't believe that's the intent of `DeclPrinter` or `PolishForDeclaration` -- * Clangd uses

[clang] Rework the printing of attributes (PR #87281)

kimgr wrote: @vgvassilev I did expect the input to be valid, yes: ``` template class FinalTemplate final {}; ``` Is it not? https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[clang] Rework the printing of attributes (PR #87281)

kimgr wrote: Ah, that's the expected output -- I can't do anything about that :). See https://github.com/llvm/llvm-project/issues/56517. https://github.com/llvm/llvm-project/pull/87281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[clang] Fix the double space and double attribute printing of the final keyword. (PR #88600)

kimgr wrote: @vgvassilev Thank you! https://github.com/llvm/llvm-project/pull/88600 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits