[PATCH] D32906: [Analyzer] Iterator Checker - Part 10: Support for iterators passed as parameter

2017-05-05 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added inline comments. Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:711 +void IteratorChecker::checkBeginFunction(CheckerContext ) const { + // Copy state of iterator arguments to iterator parameters takuto.ikuta wrote: > Can we

[PATCH] D32906: [Analyzer] Iterator Checker - Part 10: Support for iterators passed as parameter

2017-05-05 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added inline comments. Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:711 +void IteratorChecker::checkBeginFunction(CheckerContext ) const { + // Copy state of iterator arguments to iterator parameters Can we use `const

[PATCH] D32642: [Analyzer] Iterator Checker - Part 2: Increment, decrement operators and ahead-of-begin checks

2017-05-05 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added inline comments. Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:357 +return; + } else if (isEndCall(Func)) { handleEnd(C, OrigExpr, Call.getReturnValue(), We cannot use else after return?

[PATCH] D32747: [Analyzer] Iterator Checker - Part 3: Invalidation check, first for (copy) assignments

2017-05-05 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added inline comments. Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:271 + InvalidatedBugType.reset( + new BugType(this, "Iterator invalidated", "Misuse of STL APIs")); + InvalidatedBugType->setSuppressOnSink(true); OK to use

[PATCH] D32903: Remove unused variable and argument from Lex/HeaderSearch.cpp

2017-05-05 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 97949. takuto.ikuta added a comment. Remove IncludeLoc https://reviews.llvm.org/D32903 Files: clang/include/clang/Lex/DirectoryLookup.h clang/include/clang/Lex/HeaderSearch.h clang/lib/Lex/HeaderSearch.cpp Index: clang/lib/Lex/HeaderSearch.cpp

[PATCH] D32903: Remove unused variable and argument from Lex/HeaderSearch.cpp

2017-05-05 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta created this revision. https://reviews.llvm.org/D32903 Files: clang/lib/Lex/HeaderSearch.cpp Index: clang/lib/Lex/HeaderSearch.cpp === --- clang/lib/Lex/HeaderSearch.cpp +++ clang/lib/Lex/HeaderSearch.cpp @@ -234,9

[PATCH] D47578: Do not enforce absolute path argv0 in windows

2018-06-11 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 150685. takuto.ikuta added a comment. Follow llvm style https://reviews.llvm.org/D47578 Files: llvm/lib/Support/Windows/Process.inc llvm/unittests/Support/CommandLineTest.cpp Index: llvm/unittests/Support/CommandLineTest.cpp

[PATCH] D47578: Do not enforce absolute path argv0 in windows

2018-06-11 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta marked 5 inline comments as done. takuto.ikuta added a comment. Rui-san, can I ask you to merge this? https://reviews.llvm.org/D47578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D47578: Do not enforce absolute path argv0 in windows

2018-06-08 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 150515. https://reviews.llvm.org/D47578 Files: llvm/lib/Support/Windows/Process.inc llvm/unittests/Support/CommandLineTest.cpp Index: llvm/unittests/Support/CommandLineTest.cpp === ---

[PATCH] D47578: Do not enforce absolute path argv0 in windows

2018-06-08 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta marked 2 inline comments as done. takuto.ikuta added a comment. Can the patch be merged this time? https://reviews.llvm.org/D47578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D47578: Do not enforce absolute path argv0 in windows

2018-06-07 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 150300. https://reviews.llvm.org/D47578 Files: llvm/lib/Support/Windows/Process.inc llvm/unittests/Support/CommandLineTest.cpp Index: llvm/unittests/Support/CommandLineTest.cpp === ---

[PATCH] D47578: Do not enforce absolute path argv0 in windows

2018-06-07 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. I see. Changed not to convert many times. I confirmed that this passed check-lld, check-llvm and check-clang. If this looks good for you, can I ask you to merge this? Thanks. https://reviews.llvm.org/D47578 ___

[PATCH] D47578: Do not enforce absolute path argv0 in windows

2018-06-13 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. Thank you! https://reviews.llvm.org/D47578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47578: Do not enforce absolute path argv0 in windows

2018-05-31 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta created this revision. takuto.ikuta added reviewers: thakis, rnk. Herald added a subscriber: hiraditya. Herald added a reviewer: alexshap. Even if we support no-canonical-prefix on clang-cl(https://reviews.llvm.org/D47480), argv0 becomes absolute path in clang-cl and that embeds

[PATCH] D47480: clang-cl: Expose -no-canonical-prefixes

2018-05-29 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. On windows, argv0 is changed to absolute path before we call GetExecutablePath. That makes resource-dir absolute. I need clang-cl on windows has relative --resource-dir. See around heres.

[PATCH] D47480: clang-cl: Expose -no-canonical-prefixes

2018-05-29 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. GetExecutablePath is called here. http://llvm-cs.pcc.me.uk/tools/clang/tools/driver/driver.cpp#427 You'll understand what I said if you see the behavior of clang-cl on windows. Comment at: test/Driver/cl-options.c:595 +// RUN:

[PATCH] D47578: Do not enforce absolute path argv0 in windows

2018-05-31 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. In https://reviews.llvm.org/D47578#1117540, @ruiu wrote: > Looks like this patch contains two unrelated changes. Please separate your > change from the lit config changes. First patch made on some wrong assumption, fixed and reverted test config change. In

[PATCH] D47578: Do not enforce absolute path argv0 in windows

2018-05-31 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 149304. takuto.ikuta edited the summary of this revision. https://reviews.llvm.org/D47578 Files: llvm/lib/Support/Windows/Process.inc Index: llvm/lib/Support/Windows/Process.inc === ---

[PATCH] D47578: Do not enforce absolute path argv0 in windows

2018-06-04 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta marked 3 inline comments as done. takuto.ikuta added a comment. Thank you for review Comment at: llvm/lib/Support/Windows/Process.inc:251 + // This may change argv0 like below, + // * clang -> clang.exe (just add extension) + // * CLANG_~1.EXE -> clang++.exe

[PATCH] D47578: Do not enforce absolute path argv0 in windows

2018-06-04 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 149694. takuto.ikuta added a comment. Address comments https://reviews.llvm.org/D47578 Files: llvm/lib/Support/Windows/Process.inc llvm/unittests/Support/CommandLineTest.cpp Index: llvm/unittests/Support/CommandLineTest.cpp

[PATCH] D47578: Do not enforce absolute path argv0 in windows

2018-05-31 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta marked an inline comment as done. takuto.ikuta added a comment. In https://reviews.llvm.org/D47578#1117874, @rnk wrote: > I think this would be easy to unit test in > llvm/unittests/Support/CommandLine.cpp. We'd just check that the filename is > "SupportTests.exe" on Windows and

[PATCH] D47578: Do not enforce absolute path argv0 in windows

2018-05-31 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 149394. takuto.ikuta added a comment. Add test and split function https://reviews.llvm.org/D47578 Files: llvm/lib/Support/Windows/Process.inc llvm/unittests/Support/CommandLineTest.cpp Index: llvm/unittests/Support/CommandLineTest.cpp

[PATCH] D47480: clang-cl: Expose -no-canonical-prefixes

2018-06-01 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta accepted this revision. takuto.ikuta added a comment. I confirmed this CL and https://reviews.llvm.org/D47578 remove absolute path from /showIncludes when include paths are given in relative. https://reviews.llvm.org/D47480 ___

[PATCH] D47480: clang-cl: Expose -no-canonical-prefixes

2018-05-29 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. Thank you for quick supporting, but this is not sufficient for clang-cl on windows. llvm::InitLLVM calls windows::GetCommandLineArguments, and argv0 is changed to absolute path.

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-09-07 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. In https://reviews.llvm.org/D51340#1226989, @takuto.ikuta wrote: > In https://reviews.llvm.org/D51340#1222013, @hans wrote: > > > Did both your builds use PCH? It'd be interesting to see the difference > > without PCH too; the effect should be even larger. > > >

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-09-06 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. I found that current ToT with original Nico's patch does not allow to link ui_base.dll https://github.com/atetubou/llvm-project-20170507/tree/totwin_dbg_1236_nico gives the following link error https://pastebin.com/9LVRbRVn I will do bisection to find when some

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-09-06 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. In https://reviews.llvm.org/D51340#1225805, @takuto.ikuta wrote: > I found that current ToT with original Nico's patch does not allow to link > ui_base.dll > > https://github.com/atetubou/llvm-project-20170507/tree/totwin_dbg_1236_nico > gives the following link

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-09-07 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta marked 2 inline comments as done. takuto.ikuta added a comment. In https://reviews.llvm.org/D51340#1222013, @hans wrote: > Did both your builds use PCH? It'd be interesting to see the difference > without PCH too; the effect should be even larger. Added stats of without PCH

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-09-06 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 164353. takuto.ikuta edited the summary of this revision. takuto.ikuta added a comment. Herald added a subscriber: dschuff. Make patch closer to Nico's original implementation, but warns local static variable instead of detecting it. In

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-09-07 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta marked 4 inline comments as done. takuto.ikuta added inline comments. Comment at: clang/include/clang/Basic/LangOptions.def:117 LANGOPT(Digraphs , 1, 0, "digraphs") +LANGOPT(DllexportInlines , 1, 1, "If dllexport a class should dllexport implicit inline

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-09-07 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 164379. takuto.ikuta edited the summary of this revision. https://reviews.llvm.org/D51340 Files: clang/include/clang/Driver/CLCompatOptions.td clang/lib/Driver/ToolChains/Clang.cpp clang/lib/Sema/SemaDeclCXX.cpp

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-09-10 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5244 + false)) +CmdArgs.push_back("-fvisibility-inlines-hidden"); + hans wrote: > Huh, does this actually affect whether functions get dllexported or not?

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-09-11 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 164822. takuto.ikuta added a comment. I'm trying to handle local static var correctly. https://reviews.llvm.org/D51340 Files: clang/include/clang/Driver/CLCompatOptions.td clang/include/clang/Sema/Sema.h clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-09-11 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5244 + false)) +CmdArgs.push_back("-fvisibility-inlines-hidden"); + hans wrote: > takuto.ikuta wrote: > > hans wrote: > > > Huh, does this actually affect

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-11 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:11976 + +while (FD && !getDLLAttr(FD) && + !FD->hasAttr() && hans wrote: > takuto.ikuta wrote: > > hans wrote: > > > Why does this need to be a loop? I don't think

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-15 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 169652. takuto.ikuta retitled this revision from "[WIP] Add /Zc:DllexportInlines option to clang-cl" to "Add /Zc:DllexportInlines option to clang-cl". takuto.ikuta added a comment. Export function inside explicit template instantiation definition

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-16 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 169791. takuto.ikuta added a comment. Fix linkage for inline function of explicit template instantiation declaration https://reviews.llvm.org/D51340 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/LangOptions.h

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-16 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. Herald added a subscriber: nhaehnle. Hans, I addressed all your comments. How do you think about current implementation? Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719 + TSK != TSK_ExplicitInstantiationDeclaration && + TSK !=

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-16 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 169792. takuto.ikuta added a comment. remove comment out code https://reviews.llvm.org/D51340 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/LangOptions.h clang/include/clang/Driver/CC1Options.td

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-19 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. ping? Can I go forward in this way? https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-12 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:11976 + +while (FD && !getDLLAttr(FD) && + !FD->hasAttr() && hans wrote: > takuto.ikuta wrote: > > hans wrote: > > > takuto.ikuta wrote: > > > > hans wrote: > > > > > Why

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-12 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 169365. takuto.ikuta added a comment. Address comment https://reviews.llvm.org/D51340 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/LangOptions.h clang/include/clang/Driver/CC1Options.td

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-15 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 169783. takuto.ikuta added a comment. Remove unnecessary attr creation https://reviews.llvm.org/D51340 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/LangOptions.h clang/include/clang/Driver/CC1Options.td

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-29 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 171466. takuto.ikuta marked an inline comment as done. takuto.ikuta added a comment. Added explanation comment for added attributes and rebased https://reviews.llvm.org/D51340 Files: clang/include/clang/Basic/Attr.td

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-29 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:9552 +// overwrite linkage of explicit template instantiation +// definition/declaration. +return GVA_DiscardableODR; hans wrote: > Can you give an example for why this is

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-31 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta marked an inline comment as done. takuto.ikuta added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:9552 +// overwrite linkage of explicit template instantiation +// definition/declaration. +return GVA_DiscardableODR; hans

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-31 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 171916. takuto.ikuta added a comment. export/import explicit template instantiation function https://reviews.llvm.org/D51340 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/LangOptions.h

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-11-01 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 172090. takuto.ikuta marked 2 inline comments as done. takuto.ikuta added a comment. Added option to LangOptions.def https://reviews.llvm.org/D51340 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/LangOptions.def

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-11-01 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. Thank you for review! Comment at: clang/include/clang/Basic/LangOptions.h:246 + /// If set, dllexported classes dllexport their inline methods. + bool DllExportInlines = true; + rnk wrote: > We should define this in the

[PATCH] D53870: [clang-cl] Put dllexport attrs on static locals also in template instantiations (PR39496)

2018-10-31 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta accepted this revision. takuto.ikuta added a comment. LGTM, thank you for fix! https://reviews.llvm.org/D53870 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-31 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 171877. takuto.ikuta added a comment. Rebased to take r345699 https://reviews.llvm.org/D51340 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/LangOptions.h clang/include/clang/Driver/CC1Options.td

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-31 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:9552 +// overwrite linkage of explicit template instantiation +// definition/declaration. +return GVA_DiscardableODR; hans wrote: > takuto.ikuta wrote: > > takuto.ikuta

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-30 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. In https://reviews.llvm.org/D51340#1278799, @hans wrote: > I've been thinking more about your example with static locals in lambda and > how this works with regular dllexport. > > It got me thinking more about the problem of exposing inline functions from a >

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-31 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. Thank you for quick fix! Comment at: clang/lib/AST/ASTContext.cpp:9552 +// overwrite linkage of explicit template instantiation +// definition/declaration. +return GVA_DiscardableODR; takuto.ikuta wrote: > hans wrote:

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-11-02 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 172317. takuto.ikuta added a comment. Added cl driver flag test https://reviews.llvm.org/D51340 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/LangOptions.def clang/include/clang/Driver/CC1Options.td

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-11-02 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta marked an inline comment as done. takuto.ikuta added a comment. I missed the comment about driver flag test. https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-11-02 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. Thank you for quick review! Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5715 + (MD->isThisDeclarationADefinition() || + MD->isImplicitlyInstantiable()) && + TSK != TSK_ExplicitInstantiationDeclaration &&

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-11-02 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 172332. https://reviews.llvm.org/D51340 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/LangOptions.def clang/include/clang/Driver/CC1Options.td clang/include/clang/Driver/CLCompatOptions.td

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-11-02 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 172331. takuto.ikuta added a comment. added a few more check https://reviews.llvm.org/D51340 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/LangOptions.def clang/include/clang/Driver/CC1Options.td

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-11-02 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 172330. takuto.ikuta marked 10 inline comments as done. takuto.ikuta added a comment. Address comment https://reviews.llvm.org/D51340 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/LangOptions.def

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-11-01 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. Hans, thank you for review! I addressed all your comment and fixed small behavior. Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:76 + // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@ + __forceinline void

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-11-01 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 172304. takuto.ikuta marked 17 inline comments as done. takuto.ikuta added a comment. Simplify test and fix some behavior. https://reviews.llvm.org/D51340 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/LangOptions.def

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-11-02 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 172348. takuto.ikuta marked an inline comment as done. takuto.ikuta added a comment. Fix check-prefix https://reviews.llvm.org/D51340 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/LangOptions.def

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-11-02 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. Thank you! I'll submit this if other people does not have other comments. Comment at: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp:9 +// RUN: -emit-llvm -O1 -o - | \ +// RUN: FileCheck

[PATCH] D54319: clang-cl: Add documentation for /Zc:dllexportInlines-

2018-11-10 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta accepted this revision. takuto.ikuta added a comment. This revision is now accepted and ready to land. Seems good document! https://reviews.llvm.org/D54319 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D54426: [clang-cl] Do not allow using both /Zc:dllexportInlines- and /fallback flag

2018-11-12 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 173810. takuto.ikuta marked an inline comment as done. takuto.ikuta added a comment. rebase Repository: rL LLVM https://reviews.llvm.org/D54426 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D54426: [clang-cl] Do not allow using both /Zc:dllexportInlines- and /fallback flag

2018-11-12 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 173670. takuto.ikuta added a comment. short diag Repository: rL LLVM https://reviews.llvm.org/D54426 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/lib/Driver/ToolChains/Clang.cpp clang/lib/Driver/ToolChains/MSVC.cpp

[PATCH] D54426: [clang-cl] Do not allow using both /Zc:dllexportInlines- and /fallback flag

2018-11-12 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta marked 2 inline comments as done. takuto.ikuta added a comment. Thank you for review! Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5529 + false)) { + Arg *dllexportInlines = Args.getLastArg(options::OPT__SLASH_Zc_dllexportInlines_); + if

[PATCH] D54298: [clang-cl] Add warning for /Zc:dllexportInlines- when the flag is used with /fallback

2018-11-09 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta marked an inline comment as done. takuto.ikuta added a comment. Thank you for review! Repository: rL LLVM https://reviews.llvm.org/D54298 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D54298: [clang-cl] Add warning for /Zc:dllexportInlines- when the flag is used with /fallback

2018-11-09 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 173302. takuto.ikuta added a comment. warn in GetCommand Repository: rL LLVM https://reviews.llvm.org/D54298 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/lib/Driver/ToolChains/MSVC.cpp clang/test/Driver/cl-options.c

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-05 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. In https://reviews.llvm.org/D51340#1256299, @hans wrote: > In https://reviews.llvm.org/D51340#1254898, @takuto.ikuta wrote: > > > Ping? > > > Sorry, I know I'm being slow to look at this :-( I have not forgotten, it's > just a lot of other things happening at the

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-01 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 167683. takuto.ikuta added a comment. Update comment for Sema::ActOnFinishInlineFunctionDef https://reviews.llvm.org/D51340 Files: clang/include/clang/Basic/LangOptions.h clang/include/clang/Driver/CC1Options.td

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-01 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 167682. takuto.ikuta added a comment. Extract inline function export check to function https://reviews.llvm.org/D51340 Files: clang/include/clang/Basic/LangOptions.h clang/include/clang/Driver/CC1Options.td

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-01 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. Thank you for review! In https://reviews.llvm.org/D51340#1246508, @hans wrote: > The static local stuff still makes me nervous. How much does Chromium break > if we just ignore the problem? And how does -fvisibility-inlines-hidden > handle the issue? I'm not

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-01 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:12624 + isa(D)) { +CXXMethodDecl *MD = dyn_cast(D); +CXXRecordDecl *Class = MD->getParent(); hans wrote: > Hmm, now we're adding an AST walk over all inline methods which

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-10 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 168979. takuto.ikuta added a comment. address comment https://reviews.llvm.org/D51340 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/LangOptions.h clang/include/clang/Driver/CC1Options.td

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-10 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. Thank you for review! Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5599 +bool Sema::isInlineFunctionDLLExportable(const CXXMethodDecl *MD) { + assert(MD->isInlined()); hans wrote: > Okay, breaking out this logic is a little

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-11 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 169179. takuto.ikuta marked an inline comment as done. takuto.ikuta added a comment. address comment https://reviews.llvm.org/D51340 Files: clang/include/clang/Basic/Attr.td clang/include/clang/Basic/LangOptions.h

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-11 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. Thank you for review! I updated the code. Comment at: clang/lib/Sema/SemaDecl.cpp:11976 + +while (FD && !getDLLAttr(FD) && + !FD->hasAttr() && hans wrote: > Why does this need to be a loop? I don't think

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-10-04 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. Ping? https://reviews.llvm.org/D51340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-09-21 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 166448. takuto.ikuta edited the summary of this revision. takuto.ikuta added a comment. Remove unnecessary willHaveBody check condition https://reviews.llvm.org/D51340 Files: clang/include/clang/Basic/LangOptions.h

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-09-21 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 166450. https://reviews.llvm.org/D51340 Files: clang/include/clang/Basic/LangOptions.h clang/include/clang/Driver/CC1Options.td clang/include/clang/Driver/CLCompatOptions.td clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-09-24 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. Ping? This patch reduced obj size largely, and I expect this makes distributed build (like goma) faster by reducing data transferred on network. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5244 + false)) +

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-09-24 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. In https://reviews.llvm.org/D51340#1243453, @hans wrote: > In https://reviews.llvm.org/D51340#1243331, @takuto.ikuta wrote: > > > Ping? > > > > This patch reduced obj size largely, and I expect this makes distributed > > build (like goma) faster by reducing data

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-09-19 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. PTAL again. I confirmed that current patch can link chrome and functions with local static variable are exported. But current ToT clang was not improved well by this patch. I guess there is some change recently making effect of this patch smaller. Or chromium has

[PATCH] D51340: [WIP] Add /Zc:DllexportInlines option to clang-cl

2018-09-12 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5244 + false)) +CmdArgs.push_back("-fvisibility-inlines-hidden"); + rnk wrote: > takuto.ikuta wrote: > > hans wrote: > > > takuto.ikuta wrote: > > > >

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

2018-09-19 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 166087. takuto.ikuta retitled this revision from "[WIP] Add /Zc:DllexportInlines option to clang-cl" to "Add /Zc:DllexportInlines option to clang-cl". takuto.ikuta edited the summary of this revision. https://reviews.llvm.org/D51340 Files:

[PATCH] D51340: [WIP] Add /Zc:DllexportInlines option to clang-cl

2018-09-18 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta updated this revision to Diff 165966. takuto.ikuta edited the summary of this revision. takuto.ikuta added a comment. Current implementation cannot build chrome when pch is enabled. undefined symbol error happens during linking blink_modules.dll https://reviews.llvm.org/D51340

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

2019-03-27 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added inline comments. Comment at: clang/lib/Parse/ParseAST.cpp:154 if (HaveLexer) { +llvm::TimeTraceScope TimeScope("Frontend", StringRef("")); P.Initialize(); anton-afanasyev wrote: > takuto.ikuta wrote: > > Remove StringRef? > I use

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

2019-03-26 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:1352 + + llvm::TimeTraceScope TimeScope("Backend", StringRef("")); + We don't need explicit StringRef constructor call here. Just passing "" is enough. Comment

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

2019-03-23 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added inline comments. Comment at: llvm/lib/Support/TimeProfiler.cpp:28 + +static std::string escapeString(const char *Src) { + std::string OS; you can pass StringRef here and remove .data() or .c_str() from caller. Comment at:

[PATCH] D75323: Support relative dest paths in headermap files

2020-02-27 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta requested changes to this revision. takuto.ikuta added a comment. This revision now requires changes to proceed. add test? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75323/new/ https://reviews.llvm.org/D75323

[PATCH] D75323: Support relative dest paths in headermap files

2020-02-28 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added inline comments. Comment at: clang/test/Headers/headermap_relpath.cpp:8 + +// RUN: cd %S; %clang_cc1 -I %S/Inputs/empty.hmap %s + generate header map using hmaptool like

[PATCH] D75323: Support relative dest paths in headermap files

2020-03-06 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added a comment. Nico, can we merge this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75323/new/ https://reviews.llvm.org/D75323 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D75323: Support relative dest paths in headermap files

2020-03-01 Thread Takuto Ikuta via Phabricator via cfe-commits
takuto.ikuta added inline comments. Comment at: clang/test/Headers/Inputs/headermap_relpath/empty.hmap.json:1 +{"mappings":{"empty/empty.h":"include/empty.h"}} better to format like other hmap.json? CHANGES SINCE LAST ACTION