[PATCH] D31584: [coroutines] Add support for allocation elision
alekseyshl added a comment. This bot reports leaks in coro-alloc.cpp: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/1401/steps/check-clang%20asan/logs/stdio. Please fix. Repository: rL LLVM https://reviews.llvm.org/D31584 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31608: [coroutines] Add emission of initial and final suspends
alekseyshl added a comment. Leaks and warnings are reported in coro-await.cpp: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/1400/steps/check-clang%20asan/logs/stdio. Please fix. https://reviews.llvm.org/D31608 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r303630 - Allow to use vfs::FileSystem for file accesses inside ASTUnit.
Any specific reason why this doesn't contain a testcase? On Tue, May 23, 2017 at 4:37 AM, Ilya Biryukov via cfe-commits wrote: > Author: ibiryukov > Date: Tue May 23 06:37:52 2017 > New Revision: 303630 > > URL: http://llvm.org/viewvc/llvm-project?rev=303630&view=rev > Log: > Allow to use vfs::FileSystem for file accesses inside ASTUnit. > > Reviewers: bkramer, krasimir, arphaman, akyrtzi > > Reviewed By: bkramer > > Subscribers: klimek, cfe-commits > > Differential Revision: https://reviews.llvm.org/D33397 > > Modified: > cfe/trunk/include/clang/Frontend/ASTUnit.h > cfe/trunk/include/clang/Frontend/CompilerInvocation.h > cfe/trunk/lib/Frontend/ASTUnit.cpp > cfe/trunk/lib/Frontend/CompilerInvocation.cpp > > Modified: cfe/trunk/include/clang/Frontend/ASTUnit.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/ASTUnit.h?rev=303630&r1=303629&r2=303630&view=diff > == > --- cfe/trunk/include/clang/Frontend/ASTUnit.h (original) > +++ cfe/trunk/include/clang/Frontend/ASTUnit.h Tue May 23 06:37:52 2017 > @@ -59,6 +59,10 @@ class TargetInfo; > class FrontendAction; > class ASTDeserializationListener; > > +namespace vfs { > +class FileSystem; > +} > + > /// \brief Utility class for loading a ASTContext from an AST file. > /// > class ASTUnit : public ModuleLoader { > @@ -420,7 +424,8 @@ private: >explicit ASTUnit(bool MainFileIsAST); > >bool Parse(std::shared_ptr PCHContainerOps, > - std::unique_ptr OverrideMainBuffer); > + std::unique_ptr OverrideMainBuffer, > + IntrusiveRefCntPtr VFS); > >struct ComputedPreamble { > llvm::MemoryBuffer *Buffer; > @@ -434,11 +439,13 @@ private: >PreambleEndsAtStartOfLine(PreambleEndsAtStartOfLine) {} >}; >ComputedPreamble ComputePreamble(CompilerInvocation &Invocation, > - unsigned MaxLines); > + unsigned MaxLines, > + IntrusiveRefCntPtr VFS); > >std::unique_ptr getMainBufferWithPrecompiledPreamble( >std::shared_ptr PCHContainerOps, > - const CompilerInvocation &PreambleInvocationIn, bool AllowRebuild = > true, > + const CompilerInvocation &PreambleInvocationIn, > + IntrusiveRefCntPtr VFS, bool AllowRebuild = true, >unsigned MaxLines = 0); >void RealizeTopLevelDeclsFromPreamble(); > > @@ -731,11 +738,17 @@ private: >/// of this translation unit should be precompiled, to improve the > performance >/// of reparsing. Set to zero to disable preambles. >/// > + /// \param VFS - A vfs::FileSystem to be used for all file accesses. Note > that > + /// preamble is saved to a temporary directory on a RealFileSystem, so in > order > + /// for it to be loaded correctly, VFS should have access to it(i.e., be an > + /// overlay over RealFileSystem). > + /// >/// \returns \c true if a catastrophic failure occurred (which means that > the >/// \c ASTUnit itself is invalid), or \c false otherwise. >bool LoadFromCompilerInvocation( >std::shared_ptr PCHContainerOps, > - unsigned PrecompilePreambleAfterNParses); > + unsigned PrecompilePreambleAfterNParses, > + IntrusiveRefCntPtr VFS); > > public: > > @@ -826,6 +839,11 @@ public: >/// (e.g. because the PCH could not be loaded), this accepts the ASTUnit >/// mainly to allow the caller to see the diagnostics. >/// > + /// \param VFS - A vfs::FileSystem to be used for all file accesses. Note > that > + /// preamble is saved to a temporary directory on a RealFileSystem, so in > order > + /// for it to be loaded correctly, VFS should have access to it(i.e., be an > + /// overlay over RealFileSystem). RealFileSystem will be used if \p VFS is > nullptr. > + /// >// FIXME: Move OnlyLocalDecls, UseBumpAllocator to setters on the ASTUnit, > we >// shouldn't need to specify them at construction time. >static ASTUnit *LoadFromCommandLine( > @@ -842,15 +860,23 @@ public: >bool AllowPCHWithCompilerErrors = false, bool SkipFunctionBodies = > false, >bool UserFilesAreVolatile = false, bool ForSerialization = false, >llvm::Optional ModuleFormat = llvm::None, > - std::unique_ptr *ErrAST = nullptr); > + std::unique_ptr *ErrAST = nullptr, > + IntrusiveRefCntPtr VFS = nullptr); > >/// \brief Reparse the source files using the same command-line options > that >/// were originally used to produce this translation unit. >/// > + /// \param VFS - A vfs::FileSystem to be used for all file accesses. Note > that > + /// preamble is saved to a temporary directory on a RealFileSystem, so in > order > + /// for it to be loaded correctly, VFS should give an access to this(i.e. > be an > + /// overlay over RealFileSystem). FileMgr->getVirtualFileSystem() will be > used if > + /// \p VFS is nullptr. >
r303705 - [Modules] Fix overly conservative assertion for import diagnostic
Author: bruno Date: Tue May 23 18:53:17 2017 New Revision: 303705 URL: http://llvm.org/viewvc/llvm-project?rev=303705&view=rev Log: [Modules] Fix overly conservative assertion for import diagnostic We currenltly assert when want to diagnose a missing import and the decl in question is already visible. It turns out that the decl in question might be visible because another decl from the same module actually made the module visible in a previous error diagnostic. Remove the assertion and avoid re-exporting the module if it's already visible. rdar://problem/27975402 Differential Revision: https://reviews.llvm.org/D32828 Added: cfe/trunk/test/Modules/Inputs/diagnose-missing-import/ cfe/trunk/test/Modules/Inputs/diagnose-missing-import/a.h cfe/trunk/test/Modules/Inputs/diagnose-missing-import/module.modulemap cfe/trunk/test/Modules/diagnose-missing-import.m Modified: cfe/trunk/lib/Sema/SemaDecl.cpp cfe/trunk/lib/Sema/SemaLookup.cpp Modified: cfe/trunk/lib/Sema/SemaDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=303705&r1=303704&r2=303705&view=diff == --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue May 23 18:53:17 2017 @@ -16097,7 +16097,8 @@ void Sema::ActOnModuleEnd(SourceLocation void Sema::createImplicitModuleImportForErrorRecovery(SourceLocation Loc, Module *Mod) { // Bail if we're not allowed to implicitly import a module here. - if (isSFINAEContext() || !getLangOpts().ModulesErrorRecovery) + if (isSFINAEContext() || !getLangOpts().ModulesErrorRecovery || + VisibleModules.isVisible(Mod)) return; // Create the implicit import declaration. Modified: cfe/trunk/lib/Sema/SemaLookup.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=303705&r1=303704&r2=303705&view=diff == --- cfe/trunk/lib/Sema/SemaLookup.cpp (original) +++ cfe/trunk/lib/Sema/SemaLookup.cpp Tue May 23 18:53:17 2017 @@ -4933,8 +4933,6 @@ static NamedDecl *getDefinitionToImport( void Sema::diagnoseMissingImport(SourceLocation Loc, NamedDecl *Decl, MissingImportKind MIK, bool Recover) { - assert(!isVisible(Decl) && "missing import for non-hidden decl?"); - // Suggest importing a module providing the definition of this entity, if // possible. NamedDecl *Def = getDefinitionToImport(Decl); Added: cfe/trunk/test/Modules/Inputs/diagnose-missing-import/a.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/diagnose-missing-import/a.h?rev=303705&view=auto == --- cfe/trunk/test/Modules/Inputs/diagnose-missing-import/a.h (added) +++ cfe/trunk/test/Modules/Inputs/diagnose-missing-import/a.h Tue May 23 18:53:17 2017 @@ -0,0 +1,8 @@ +#ifndef A_h +#define A_h + +@class NSString; +static NSString * const xyzRiskyCloseOpenParam = @"riskyCloseParam"; +static inline void XYZLogEvent(NSString* eventName, NSString* params); + +#endif Added: cfe/trunk/test/Modules/Inputs/diagnose-missing-import/module.modulemap URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/diagnose-missing-import/module.modulemap?rev=303705&view=auto == --- cfe/trunk/test/Modules/Inputs/diagnose-missing-import/module.modulemap (added) +++ cfe/trunk/test/Modules/Inputs/diagnose-missing-import/module.modulemap Tue May 23 18:53:17 2017 @@ -0,0 +1,3 @@ +module NCI { + explicit module A { header "a.h" } +} Added: cfe/trunk/test/Modules/diagnose-missing-import.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/diagnose-missing-import.m?rev=303705&view=auto == --- cfe/trunk/test/Modules/diagnose-missing-import.m (added) +++ cfe/trunk/test/Modules/diagnose-missing-import.m Tue May 23 18:53:17 2017 @@ -0,0 +1,14 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I%S/Inputs/diagnose-missing-import \ +// RUN: -Werror=implicit-function-declaration -fsyntax-only \ +// RUN: -fimplicit-module-maps -verify %s +@import NCI; + +void foo() { + XYZLogEvent(xyzRiskyCloseOpenParam, xyzRiskyCloseOpenParam); // expected-error {{implicit declaration of function 'XYZLogEvent'}} expected-error {{declaration of 'XYZLogEvent' must be imported}} expected-error {{declaration of 'xyzRiskyCloseOpenParam' must be imported from module 'NCI.A'}} expected-error {{declaration of 'xyzRiskyCloseOpenParam' must be imported from module 'NCI.A'}} +} + +// expected-note@Inputs/diagnose-missing-import/a.h:5 {{previous declaration is here}} +// expected-note@Inputs/diagnose-missing-import/a.h:5 {{previous d
[PATCH] D32828: [Modules] Fix conservative assertion for import diagnostics
This revision was automatically updated to reflect the committed changes. Closed by commit rL303705: [Modules] Fix overly conservative assertion for import diagnostic (authored by bruno). Changed prior to commit: https://reviews.llvm.org/D32828?vs=100016&id=100023#toc Repository: rL LLVM https://reviews.llvm.org/D32828 Files: cfe/trunk/lib/Sema/SemaDecl.cpp cfe/trunk/lib/Sema/SemaLookup.cpp cfe/trunk/test/Modules/Inputs/diagnose-missing-import/a.h cfe/trunk/test/Modules/Inputs/diagnose-missing-import/module.modulemap cfe/trunk/test/Modules/diagnose-missing-import.m Index: cfe/trunk/test/Modules/diagnose-missing-import.m === --- cfe/trunk/test/Modules/diagnose-missing-import.m +++ cfe/trunk/test/Modules/diagnose-missing-import.m @@ -0,0 +1,14 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I%S/Inputs/diagnose-missing-import \ +// RUN: -Werror=implicit-function-declaration -fsyntax-only \ +// RUN: -fimplicit-module-maps -verify %s +@import NCI; + +void foo() { + XYZLogEvent(xyzRiskyCloseOpenParam, xyzRiskyCloseOpenParam); // expected-error {{implicit declaration of function 'XYZLogEvent'}} expected-error {{declaration of 'XYZLogEvent' must be imported}} expected-error {{declaration of 'xyzRiskyCloseOpenParam' must be imported from module 'NCI.A'}} expected-error {{declaration of 'xyzRiskyCloseOpenParam' must be imported from module 'NCI.A'}} +} + +// expected-note@Inputs/diagnose-missing-import/a.h:5 {{previous declaration is here}} +// expected-note@Inputs/diagnose-missing-import/a.h:5 {{previous declaration is here}} +// expected-note@Inputs/diagnose-missing-import/a.h:6 {{previous declaration is here}} + Index: cfe/trunk/test/Modules/Inputs/diagnose-missing-import/module.modulemap === --- cfe/trunk/test/Modules/Inputs/diagnose-missing-import/module.modulemap +++ cfe/trunk/test/Modules/Inputs/diagnose-missing-import/module.modulemap @@ -0,0 +1,3 @@ +module NCI { + explicit module A { header "a.h" } +} Index: cfe/trunk/test/Modules/Inputs/diagnose-missing-import/a.h === --- cfe/trunk/test/Modules/Inputs/diagnose-missing-import/a.h +++ cfe/trunk/test/Modules/Inputs/diagnose-missing-import/a.h @@ -0,0 +1,8 @@ +#ifndef A_h +#define A_h + +@class NSString; +static NSString * const xyzRiskyCloseOpenParam = @"riskyCloseParam"; +static inline void XYZLogEvent(NSString* eventName, NSString* params); + +#endif Index: cfe/trunk/lib/Sema/SemaDecl.cpp === --- cfe/trunk/lib/Sema/SemaDecl.cpp +++ cfe/trunk/lib/Sema/SemaDecl.cpp @@ -16097,7 +16097,8 @@ void Sema::createImplicitModuleImportForErrorRecovery(SourceLocation Loc, Module *Mod) { // Bail if we're not allowed to implicitly import a module here. - if (isSFINAEContext() || !getLangOpts().ModulesErrorRecovery) + if (isSFINAEContext() || !getLangOpts().ModulesErrorRecovery || + VisibleModules.isVisible(Mod)) return; // Create the implicit import declaration. Index: cfe/trunk/lib/Sema/SemaLookup.cpp === --- cfe/trunk/lib/Sema/SemaLookup.cpp +++ cfe/trunk/lib/Sema/SemaLookup.cpp @@ -4933,8 +4933,6 @@ void Sema::diagnoseMissingImport(SourceLocation Loc, NamedDecl *Decl, MissingImportKind MIK, bool Recover) { - assert(!isVisible(Decl) && "missing import for non-hidden decl?"); - // Suggest importing a module providing the definition of this entity, if // possible. NamedDecl *Def = getDefinitionToImport(Decl); Index: cfe/trunk/test/Modules/diagnose-missing-import.m === --- cfe/trunk/test/Modules/diagnose-missing-import.m +++ cfe/trunk/test/Modules/diagnose-missing-import.m @@ -0,0 +1,14 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I%S/Inputs/diagnose-missing-import \ +// RUN: -Werror=implicit-function-declaration -fsyntax-only \ +// RUN: -fimplicit-module-maps -verify %s +@import NCI; + +void foo() { + XYZLogEvent(xyzRiskyCloseOpenParam, xyzRiskyCloseOpenParam); // expected-error {{implicit declaration of function 'XYZLogEvent'}} expected-error {{declaration of 'XYZLogEvent' must be imported}} expected-error {{declaration of 'xyzRiskyCloseOpenParam' must be imported from module 'NCI.A'}} expected-error {{declaration of 'xyzRiskyCloseOpenParam' must be imported from module 'NCI.A'}} +} + +// expected-note@Inputs/diagnose-missing-import/a.h:5 {{previous declaration is here}} +// expected-note@Inputs/diagnose-missing-import/a.h:5 {{previous declaration is here}} +// expected-note@Inputs/diagnose-missing-import/a.h:6 {{previous declaration is here}} + Index: cfe/trunk/test/Mod
[PATCH] D33393: [PATCH] Libcxxabi Demangler PR32890
marcel abandoned this revision. marcel added a comment. Closed. Patch https://reviews.llvm.org/D33368 addresses PR32890 more comprehensively. https://reviews.llvm.org/D33393 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33259: Don't defer to the GCC driver for linking arm-baremetal
compnerd added a comment. Looks generally pretty good. This is going to be a pretty cool addition! Comment at: lib/Driver/ToolChains/BareMetal.cpp:68 + SmallString<128> Dir(getDriver().ResourceDir); + llvm::sys::path::append(Dir, "lib", "baremetal"); + return Dir.str(); jroelofs wrote: > compnerd wrote: > > jroelofs wrote: > > > compnerd wrote: > > > > jroelofs wrote: > > > > > compnerd wrote: > > > > > > Why not just the standard `arm` directory? > > > > > There are a few differences between the stuff in the existing ones, > > > > > and what is needed on baremetal. For example __enable_execute_stack, > > > > > emutls, as well as anything else that assumes existence of pthreads > > > > > support shouldn't be there. > > > > Well, I think that "baremetal" here is a bad idea. How about using the > > > > android approach? Use `clang_rt.builtins-arm-baremetal.a` ? > > > Why? Given the way the cmake goop works in lib/runtimes + compiler-rt, > > > the folder name there has to be the same as the CMAKE_SYSTEM_NAME. The > > > alternative, I guess, is to call it 'generic', but I'm not convinced > > > that's better than 'baremetal'. > > Because I can have a baremetal environment that uses a different > > architecture. How do you differentiate between the MIPS and ARM bare metal > > runtimes? The way that the compiler actually looks up the builtins is that > > it uses `clang_rt.[component]-[arch][-variant]` > Yes, and that's still how they're being looked up (and built/installed), even > in this patch: > > `lib/clang/[version]/lib/[cmake_system_name]/libclangrt.[component]-[arch][-variant].a` > > Having arch+variant in the name means they won't intersect, just as they > don't for any other system. The only difference here is that baremetal > doesn't really have a "system" per se, and it's not appropriate to use the > darwin/linux/whatever ones, hence the 'baremetal' folder. Ah. I see, I was visualizing the tree incorrectly. Using `baremetal` this way is fine by me. Comment at: lib/Driver/ToolChains/BareMetal.cpp:110 + SmallString<128> Dir(SysRoot); + llvm::sys::path::append(Dir, "include", "c++", "v1"); + return Dir.str(); Is this layout consistent between libc++ and libstdc++? Comment at: lib/Driver/ToolChains/BareMetal.cpp:130-133 +if (Value == "libc++") + return ToolChain::CST_Libcxx; +else if (Value == "libstdc++") + return ToolChain::CST_Libstdcxx; Use `StringSwitch`? Comment at: lib/Driver/ToolChains/BareMetal.h:39 + bool isPICDefaultForced() const override { return false; } + bool SupportsProfiling() const override { return true; } + bool SupportsObjCGC() const override { return false; } Is the profiler support in compiler-rt sufficiently standalone to build it for baremetal? https://reviews.llvm.org/D33259 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303712 - Enhance the 'diagnose_if' attribute so that we can apply it for ObjC methods and properties as well
Author: akirtzidis Date: Tue May 23 19:46:27 2017 New Revision: 303712 URL: http://llvm.org/viewvc/llvm-project?rev=303712&view=rev Log: Enhance the 'diagnose_if' attribute so that we can apply it for ObjC methods and properties as well This is an initial commit to allow using it with constant expressions, a follow-up commit will enable full support for it in ObjC methods. Added: cfe/trunk/test/SemaObjC/diagnose_if.m Modified: cfe/trunk/include/clang/Basic/Attr.td cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/include/clang/Sema/AttributeList.h cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/Lex/PPMacroExpansion.cpp cfe/trunk/lib/Sema/SemaDeclAttr.cpp cfe/trunk/lib/Sema/SemaExpr.cpp cfe/trunk/lib/Sema/SemaOverload.cpp cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp Modified: cfe/trunk/include/clang/Basic/Attr.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=303712&r1=303711&r2=303712&view=diff == --- cfe/trunk/include/clang/Basic/Attr.td (original) +++ cfe/trunk/include/clang/Basic/Attr.td Tue May 23 19:46:27 2017 @@ -149,6 +149,9 @@ class ExprArgument : Argument; +class NamedArgument : Argument; class TypeArgument : Argument; class UnsignedArgument : Argument; class VariadicUnsignedArgument : Argument; @@ -1819,14 +1822,14 @@ def Unavailable : InheritableAttr { def DiagnoseIf : InheritableAttr { let Spellings = [GNU<"diagnose_if">]; - let Subjects = SubjectList<[Function]>; + let Subjects = SubjectList<[Function, ObjCMethod, ObjCProperty]>; let Args = [ExprArgument<"Cond">, StringArgument<"Message">, EnumArgument<"DiagnosticType", "DiagnosticType", ["error", "warning"], ["DT_Error", "DT_Warning"]>, BoolArgument<"ArgDependent", 0, /*fake*/ 1>, - FunctionArgument<"Parent", 0, /*fake*/ 1>]; + NamedArgument<"Parent", 0, /*fake*/ 1>]; let DuplicatesAllowedWhileMerging = 1; let LateParsed = 1; let AdditionalMembers = [{ Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=303712&r1=303711&r2=303712&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue May 23 19:46:27 2017 @@ -2771,6 +2771,7 @@ def warn_attribute_wrong_decl_type : War "|types and namespaces" "|Objective-C interfaces" "|methods and properties" + "|functions, methods and properties" "|struct or union" "|struct, union or class" "|types" Modified: cfe/trunk/include/clang/Sema/AttributeList.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/AttributeList.h?rev=303712&r1=303711&r2=303712&view=diff == --- cfe/trunk/include/clang/Sema/AttributeList.h (original) +++ cfe/trunk/include/clang/Sema/AttributeList.h Tue May 23 19:46:27 2017 @@ -915,6 +915,7 @@ enum AttributeDeclKind { ExpectedTypeOrNamespace, ExpectedObjectiveCInterface, ExpectedMethodOrProperty, + ExpectedFunctionOrMethodOrProperty, ExpectedStructOrUnion, ExpectedStructOrUnionOrClass, ExpectedType, Modified: cfe/trunk/include/clang/Sema/Sema.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=303712&r1=303711&r2=303712&view=diff == --- cfe/trunk/include/clang/Sema/Sema.h (original) +++ cfe/trunk/include/clang/Sema/Sema.h Tue May 23 19:46:27 2017 @@ -2727,7 +2727,7 @@ public: /// of a function. /// /// Returns true if any errors were emitted. - bool diagnoseArgIndependentDiagnoseIfAttrs(const FunctionDecl *Function, + bool diagnoseArgIndependentDiagnoseIfAttrs(const NamedDecl *ND, SourceLocation Loc); /// Returns whether the given function's address can be taken or not, Modified: cfe/trunk/lib/Lex/PPMacroExpansion.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPMacroExpansion.cpp?rev=303712&r1=303711&r2=303712&view=diff == --- cfe/trunk/lib/Lex/PPMacroExpansion.cpp (original) +++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp Tue May 23 19:46:27 2017 @@ -1166,6 +1166,7 @@ static bool HasFeature(const Preprocesso .Case("objc_generics", LangOpts.ObjC2) .Case("objc_generics_variance", LangOpts.ObjC2) .Case("objc_class_property", LangOpts.ObjC2) + .Case("objc_diagnose_if_attr", LangOpts.ObjC2) // C11 features .Case("c_alignas", LangOpts.C11) .Case("c_alignof", L
[PATCH] D32515: [libcxx] [test] Changes to accommodate LWG 2904 "Make variant move-assignment more exception safe"
mpark added a comment. Thanks for the tests! I'll try this out with an implementation shortly. https://reviews.llvm.org/D32515 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33474: Print symbols from COFF import libraries
kastiglione created this revision. This change allows `llvm-nm` to print symbols found in import libraries, in part by allowing `COFFImportFile`s to be casted to `SymbolicFile`s. https://reviews.llvm.org/D33474 Files: include/llvm/Object/Binary.h test/tools/llvm-nm/X86/Inputs/example.lib test/tools/llvm-nm/X86/importlibrary.test tools/llvm-nm/llvm-nm.cpp Index: tools/llvm-nm/llvm-nm.cpp === --- tools/llvm-nm/llvm-nm.cpp +++ tools/llvm-nm/llvm-nm.cpp @@ -24,6 +24,7 @@ #include "llvm/IR/Module.h" #include "llvm/Object/Archive.h" #include "llvm/Object/COFF.h" +#include "llvm/Object/COFFImportFile.h" #include "llvm/Object/ELFObjectFile.h" #include "llvm/Object/IRObjectFile.h" #include "llvm/Object/MachO.h" @@ -269,7 +270,7 @@ static char isSymbolList64Bit(SymbolicFile &Obj) { if (auto *IRObj = dyn_cast(&Obj)) return Triple(IRObj->getTargetTriple()).isArch64Bit(); - if (isa(Obj)) + if (isa(Obj) || isa(Obj)) return false; if (isa(Obj)) return false; @@ -849,6 +850,18 @@ return '?'; } +static char getSymbolNMTypeChar(COFFImportFile &Obj) { + switch (Obj.getCOFFImportHeader()->getType()) { + case COFF::IMPORT_CODE: +return 't'; + case COFF::IMPORT_DATA: +return 'd'; + case COFF::IMPORT_CONST: +return 'r'; + } + return '?'; +} + static char getSymbolNMTypeChar(MachOObjectFile &Obj, basic_symbol_iterator I) { DataRefImpl Symb = I->getRawDataRefImpl(); uint8_t NType = Obj.is64Bit() ? Obj.getSymbol64TableEntry(Symb).n_type @@ -932,6 +945,8 @@ Ret = getSymbolNMTypeChar(*IR, I); else if (COFFObjectFile *COFF = dyn_cast(&Obj)) Ret = getSymbolNMTypeChar(*COFF, I); + else if (COFFImportFile *COFFImport = dyn_cast(&Obj)) +Ret = getSymbolNMTypeChar(*COFFImport); else if (MachOObjectFile *MachO = dyn_cast(&Obj)) Ret = getSymbolNMTypeChar(*MachO, I); else if (WasmObjectFile *Wasm = dyn_cast(&Obj)) Index: test/tools/llvm-nm/X86/importlibrary.test === --- /dev/null +++ test/tools/llvm-nm/X86/importlibrary.test @@ -0,0 +1,7 @@ +# RUN: llvm-nm -B %S/Inputs/example.lib | FileCheck --match-full-lines %s + +CHECK: R __imp__constant +CHECK: R _constant +CHECK: D __imp__data +CHECK: T __imp__function +CHECK: T _function Index: include/llvm/Object/Binary.h === --- include/llvm/Object/Binary.h +++ include/llvm/Object/Binary.h @@ -96,7 +96,7 @@ } bool isSymbolic() const { -return isIR() || isObject(); +return isIR() || isObject() || isCOFFImportFile(); } bool isArchive() const { Index: tools/llvm-nm/llvm-nm.cpp === --- tools/llvm-nm/llvm-nm.cpp +++ tools/llvm-nm/llvm-nm.cpp @@ -24,6 +24,7 @@ #include "llvm/IR/Module.h" #include "llvm/Object/Archive.h" #include "llvm/Object/COFF.h" +#include "llvm/Object/COFFImportFile.h" #include "llvm/Object/ELFObjectFile.h" #include "llvm/Object/IRObjectFile.h" #include "llvm/Object/MachO.h" @@ -269,7 +270,7 @@ static char isSymbolList64Bit(SymbolicFile &Obj) { if (auto *IRObj = dyn_cast(&Obj)) return Triple(IRObj->getTargetTriple()).isArch64Bit(); - if (isa(Obj)) + if (isa(Obj) || isa(Obj)) return false; if (isa(Obj)) return false; @@ -849,6 +850,18 @@ return '?'; } +static char getSymbolNMTypeChar(COFFImportFile &Obj) { + switch (Obj.getCOFFImportHeader()->getType()) { + case COFF::IMPORT_CODE: +return 't'; + case COFF::IMPORT_DATA: +return 'd'; + case COFF::IMPORT_CONST: +return 'r'; + } + return '?'; +} + static char getSymbolNMTypeChar(MachOObjectFile &Obj, basic_symbol_iterator I) { DataRefImpl Symb = I->getRawDataRefImpl(); uint8_t NType = Obj.is64Bit() ? Obj.getSymbol64TableEntry(Symb).n_type @@ -932,6 +945,8 @@ Ret = getSymbolNMTypeChar(*IR, I); else if (COFFObjectFile *COFF = dyn_cast(&Obj)) Ret = getSymbolNMTypeChar(*COFF, I); + else if (COFFImportFile *COFFImport = dyn_cast(&Obj)) +Ret = getSymbolNMTypeChar(*COFFImport); else if (MachOObjectFile *MachO = dyn_cast(&Obj)) Ret = getSymbolNMTypeChar(*MachO, I); else if (WasmObjectFile *Wasm = dyn_cast(&Obj)) Index: test/tools/llvm-nm/X86/importlibrary.test === --- /dev/null +++ test/tools/llvm-nm/X86/importlibrary.test @@ -0,0 +1,7 @@ +# RUN: llvm-nm -B %S/Inputs/example.lib | FileCheck --match-full-lines %s + +CHECK: R __imp__constant +CHECK: R _constant +CHECK: D __imp__data +CHECK: T __imp__function +CHECK: T _function Index: include/llvm/Object/Binary.h === --- include/llvm/Object/Binary.h +++ include/llvm/Object/Binary.h @@ -96,7 +96,7 @@ } bool isSym
[PATCH] D32671: [libcxx] [test] variant: test coverage for P0602 extension
mpark added a comment. @CaseyCarter: Does this not pass with the current version? Also, have you seen the tests in `test/libcxx/utilities/variant/variant.variant`? If yes, do those tests and the ones in this diff overlap at all? Curious as to how we should merge them. https://reviews.llvm.org/D32671 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303713 - Change __has_feature(objc_diagnose_if_attr) to __has_feature(attribute_diagnose_if_objc) for consistency with rest of attribute checks.
Author: akirtzidis Date: Tue May 23 20:38:00 2017 New Revision: 303713 URL: http://llvm.org/viewvc/llvm-project?rev=303713&view=rev Log: Change __has_feature(objc_diagnose_if_attr) to __has_feature(attribute_diagnose_if_objc) for consistency with rest of attribute checks. Modified: cfe/trunk/lib/Lex/PPMacroExpansion.cpp cfe/trunk/test/SemaObjC/diagnose_if.m Modified: cfe/trunk/lib/Lex/PPMacroExpansion.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPMacroExpansion.cpp?rev=303713&r1=303712&r2=303713&view=diff == --- cfe/trunk/lib/Lex/PPMacroExpansion.cpp (original) +++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp Tue May 23 20:38:00 2017 @@ -1125,6 +1125,7 @@ static bool HasFeature(const Preprocesso .Case("attribute_overloadable", true) .Case("attribute_unavailable_with_message", true) .Case("attribute_unused_on_fields", true) + .Case("attribute_diagnose_if_objc", true) .Case("blocks", LangOpts.Blocks) .Case("c_thread_safety_attributes", true) .Case("cxx_exceptions", LangOpts.CXXExceptions) @@ -1166,7 +1167,6 @@ static bool HasFeature(const Preprocesso .Case("objc_generics", LangOpts.ObjC2) .Case("objc_generics_variance", LangOpts.ObjC2) .Case("objc_class_property", LangOpts.ObjC2) - .Case("objc_diagnose_if_attr", LangOpts.ObjC2) // C11 features .Case("c_alignas", LangOpts.C11) .Case("c_alignof", LangOpts.C11) Modified: cfe/trunk/test/SemaObjC/diagnose_if.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/diagnose_if.m?rev=303713&r1=303712&r2=303713&view=diff == --- cfe/trunk/test/SemaObjC/diagnose_if.m (original) +++ cfe/trunk/test/SemaObjC/diagnose_if.m Tue May 23 20:38:00 2017 @@ -1,6 +1,6 @@ // RUN: %clang_cc1 %s -verify -fno-builtin -_Static_assert(__has_feature(objc_diagnose_if_attr), "feature check failed?"); +_Static_assert(__has_feature(attribute_diagnose_if_objc), "feature check failed?"); #define _diagnose_if(...) __attribute__((diagnose_if(__VA_ARGS__))) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31608: [coroutines] Add emission of initial and final suspends
GorNishanov added a comment. In https://reviews.llvm.org/D31608#762783, @alekseyshl wrote: > Leaks and warnings are reported in coro-await.cpp: > http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/1400/steps/check-clang%20asan/logs/stdio. > Please fix. Thank you! The fix is incoming in a minute. https://reviews.llvm.org/D31608 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303714 - [coroutines] Fix leak in CGCoroutine.cpp
Author: gornishanov Date: Tue May 23 20:54:37 2017 New Revision: 303714 URL: http://llvm.org/viewvc/llvm-project?rev=303714&view=rev Log: [coroutines] Fix leak in CGCoroutine.cpp FinalBB need to be emitted even when unused to make sure it is deleted Modified: cfe/trunk/lib/CodeGen/CGCoroutine.cpp Modified: cfe/trunk/lib/CodeGen/CGCoroutine.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCoroutine.cpp?rev=303714&r1=303713&r2=303714&view=diff == --- cfe/trunk/lib/CodeGen/CGCoroutine.cpp (original) +++ cfe/trunk/lib/CodeGen/CGCoroutine.cpp Tue May 23 20:54:37 2017 @@ -430,6 +430,10 @@ void CodeGenFunction::EmitCoroutineBody( CurCoro.Data->CurrentAwaitKind = AwaitKind::Final; EmitStmt(S.getFinalSuspendStmt()); } +else { + // We don't need FinalBB. Emit it to make sure the block is deleted. + EmitBlock(FinalBB, /*IsFinished=*/true); +} } EmitBlock(RetBB); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31608: [coroutines] Add emission of initial and final suspends
GorNishanov added a comment. Fixed: r303714 = 8832327ab89f3668378d70d1c4e5a218446ce36e https://reviews.llvm.org/D31608 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33368: [libcxxabi][demangler] Fix a crash in the demangler
dexonsmith added inline comments. Comment at: src/cxa_demangle.cpp:3036 break; -if (db.names.size() < 2) +assert(k0 <= k1 && "parse_type() mutated the name stack"); +if (k1 == k0) There's no `#include ` in this file. Depending on the standard library you're building against, you made need that here. https://reviews.llvm.org/D33368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33470: [clang-tidy] Add misc-default-numerics
Eugene.Zelenko added inline comments. Comment at: docs/clang-tidy/checks/misc-default-numerics.rst:11 +Consider scenario: +1. Have `typedef long long BigInt` in source code +2. Use `std::numeric_limits::min()` May be code-block will be better? https://reviews.llvm.org/D33470 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31670: [coroutines] Implement correct GRO lifetime
GorNishanov updated this revision to Diff 100027. GorNishanov added a comment. merge with tot, preparing to land https://reviews.llvm.org/D31670 Files: lib/CodeGen/CGCoroutine.cpp test/CodeGenCoroutines/coro-gro.cpp Index: test/CodeGenCoroutines/coro-gro.cpp === --- /dev/null +++ test/CodeGenCoroutines/coro-gro.cpp @@ -0,0 +1,86 @@ +// Verifies lifetime of __gro local variable +// Verify that coroutine promise and allocated memory are freed up on exception. +// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -disable-llvm-passes | FileCheck %s + +namespace std::experimental { +template struct coroutine_traits; + +template struct coroutine_handle { + coroutine_handle() = default; + static coroutine_handle from_address(void *) noexcept; +}; +template <> struct coroutine_handle { + static coroutine_handle from_address(void *) noexcept; + coroutine_handle() = default; + template + coroutine_handle(coroutine_handle) noexcept; +}; +} + +struct suspend_always { + bool await_ready() noexcept; + void await_suspend(std::experimental::coroutine_handle<>) noexcept; + void await_resume() noexcept; +}; + +struct GroType { + ~GroType(); + operator int() noexcept; +}; + +template <> struct std::experimental::coroutine_traits { + struct promise_type { +GroType get_return_object() noexcept; +suspend_always initial_suspend() noexcept; +suspend_always final_suspend() noexcept; +void return_void() noexcept; +promise_type(); +~promise_type(); +void unhandled_exception() noexcept; + }; +}; + +struct Cleanup { ~Cleanup(); }; +void doSomething() noexcept; + +// CHECK: define i32 @_Z1fv( +int f() { + // CHECK: %[[RetVal:.+]] = alloca i32 + // CHECK: %[[GroActive:.+]] = alloca i1 + + // CHECK: %[[Size:.+]] = call i64 @llvm.coro.size.i64() + // CHECK: call i8* @_Znwm(i64 %[[Size]]) + // CHECK: store i1 false, i1* %[[GroActive]] + // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_typeC1Ev( + // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_type17get_return_objectEv( + // CHECK: store i1 true, i1* %[[GroActive]] + + Cleanup cleanup; + doSomething(); + co_return; + + // CHECK: call void @_Z11doSomethingv( + // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_type11return_voidEv( + // CHECK: call void @_ZN7CleanupD1Ev( + + // Destroy promise and free the memory. + + // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_typeD1Ev( + // CHECK: %[[Mem:.+]] = call i8* @llvm.coro.free( + // CHECK: call void @_ZdlPv(i8* %[[Mem]]) + + // Initialize retval from Gro and destroy Gro + + // CHECK: %[[Conv:.+]] = call i32 @_ZN7GroTypecviEv( + // CHECK: store i32 %[[Conv]], i32* %[[RetVal]] + // CHECK: %[[IsActive:.+]] = load i1, i1* %[[GroActive]] + // CHECK: br i1 %[[IsActive]], label %[[CleanupGro:.+]], label %[[Done:.+]] + + // CHECK: [[CleanupGro]]: + // CHECK: call void @_ZN7GroTypeD1Ev( + // CHECK: br label %[[Done]] + + // CHECK: [[Done]]: + // CHECK: %[[LoadRet:.+]] = load i32, i32* %[[RetVal]] + // CHECK: ret i32 %[[LoadRet]] +} Index: lib/CodeGen/CGCoroutine.cpp === --- lib/CodeGen/CGCoroutine.cpp +++ lib/CodeGen/CGCoroutine.cpp @@ -11,6 +11,7 @@ // //===--===// +#include "CGCleanup.h" #include "CodeGenFunction.h" #include "llvm/ADT/ScopeExit.h" #include "clang/AST/StmtCXX.h" @@ -326,6 +327,72 @@ }; } +namespace { +struct GetReturnObjectManager { + CodeGenFunction &CGF; + CGBuilderTy &Builder; + const CoroutineBodyStmt &S; + + Address GroActiveFlag; + CodeGenFunction::AutoVarEmission GroEmission; + + GetReturnObjectManager(CodeGenFunction &CGF, const CoroutineBodyStmt &S) + : CGF(CGF), Builder(CGF.Builder), S(S), GroActiveFlag(Address::invalid()), +GroEmission(CodeGenFunction::AutoVarEmission::invalid()) {} + + // The gro variable has to outlive coroutine frame and coroutine promise, but, + // it can only be initialized after coroutine promise was created, thus, we + // split its emission in two parts. EmitGroAlloca emits an alloca and sets up + // cleanups. Later when coroutine promise is available we initialize the gro + // and sets the flag that the cleanup is now active. + + void EmitGroAlloca() { +auto *GroDeclStmt = dyn_cast(S.getResultDecl()); +if (!GroDeclStmt) { + // If get_return_object returns void, no need to do an alloca. + return; +} + +auto *GroVarDecl = cast(GroDeclStmt->getSingleDecl()); + +// Set GRO flag that it is not initialized yet +GroActiveFlag = + CGF.CreateTempAlloca(Builder.getInt1Ty(), CharUnits::One(), "gro.active"); +Builder.CreateStore(Builder.getFalse(), GroActiveFlag); + +GroEmission = CGF.EmitAutoVarAlloca(*GroVar
[PATCH] D33477: [coroutines] Implement correct GRO lifetime
GorNishanov created this revision. Herald added a subscriber: EricWF. Sema creates a declaration for gro variable as: auto $gro = $promise.get_return_object(); However, gro variable has to outlive coroutine frame and coroutine promise, but, it can only be initialized after the coroutine promise was created, thus, we split its emission in two parts: EmitGroAlloca emits an alloca and sets up the cleanups. Later when the coroutine promise is available we initialize the gro and set the flag that the cleanup is now active. Duplicate of: https://reviews.llvm.org/D31670 (which arc patch refuses to apply for some reason) https://reviews.llvm.org/D33477 Files: lib/CodeGen/CGCoroutine.cpp test/CodeGenCoroutines/coro-gro.cpp Index: test/CodeGenCoroutines/coro-gro.cpp === --- /dev/null +++ test/CodeGenCoroutines/coro-gro.cpp @@ -0,0 +1,86 @@ +// Verifies lifetime of __gro local variable +// Verify that coroutine promise and allocated memory are freed up on exception. +// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -disable-llvm-passes | FileCheck %s + +namespace std::experimental { +template struct coroutine_traits; + +template struct coroutine_handle { + coroutine_handle() = default; + static coroutine_handle from_address(void *) noexcept; +}; +template <> struct coroutine_handle { + static coroutine_handle from_address(void *) noexcept; + coroutine_handle() = default; + template + coroutine_handle(coroutine_handle) noexcept; +}; +} + +struct suspend_always { + bool await_ready() noexcept; + void await_suspend(std::experimental::coroutine_handle<>) noexcept; + void await_resume() noexcept; +}; + +struct GroType { + ~GroType(); + operator int() noexcept; +}; + +template <> struct std::experimental::coroutine_traits { + struct promise_type { +GroType get_return_object() noexcept; +suspend_always initial_suspend() noexcept; +suspend_always final_suspend() noexcept; +void return_void() noexcept; +promise_type(); +~promise_type(); +void unhandled_exception() noexcept; + }; +}; + +struct Cleanup { ~Cleanup(); }; +void doSomething() noexcept; + +// CHECK: define i32 @_Z1fv( +int f() { + // CHECK: %[[RetVal:.+]] = alloca i32 + // CHECK: %[[GroActive:.+]] = alloca i1 + + // CHECK: %[[Size:.+]] = call i64 @llvm.coro.size.i64() + // CHECK: call i8* @_Znwm(i64 %[[Size]]) + // CHECK: store i1 false, i1* %[[GroActive]] + // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_typeC1Ev( + // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_type17get_return_objectEv( + // CHECK: store i1 true, i1* %[[GroActive]] + + Cleanup cleanup; + doSomething(); + co_return; + + // CHECK: call void @_Z11doSomethingv( + // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_type11return_voidEv( + // CHECK: call void @_ZN7CleanupD1Ev( + + // Destroy promise and free the memory. + + // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_typeD1Ev( + // CHECK: %[[Mem:.+]] = call i8* @llvm.coro.free( + // CHECK: call void @_ZdlPv(i8* %[[Mem]]) + + // Initialize retval from Gro and destroy Gro + + // CHECK: %[[Conv:.+]] = call i32 @_ZN7GroTypecviEv( + // CHECK: store i32 %[[Conv]], i32* %[[RetVal]] + // CHECK: %[[IsActive:.+]] = load i1, i1* %[[GroActive]] + // CHECK: br i1 %[[IsActive]], label %[[CleanupGro:.+]], label %[[Done:.+]] + + // CHECK: [[CleanupGro]]: + // CHECK: call void @_ZN7GroTypeD1Ev( + // CHECK: br label %[[Done]] + + // CHECK: [[Done]]: + // CHECK: %[[LoadRet:.+]] = load i32, i32* %[[RetVal]] + // CHECK: ret i32 %[[LoadRet]] +} Index: lib/CodeGen/CGCoroutine.cpp === --- lib/CodeGen/CGCoroutine.cpp +++ lib/CodeGen/CGCoroutine.cpp @@ -11,6 +11,7 @@ // //===--===// +#include "CGCleanup.h" #include "CodeGenFunction.h" #include "llvm/ADT/ScopeExit.h" #include "clang/AST/StmtCXX.h" @@ -326,6 +327,72 @@ }; } +namespace { +struct GetReturnObjectManager { + CodeGenFunction &CGF; + CGBuilderTy &Builder; + const CoroutineBodyStmt &S; + + Address GroActiveFlag; + CodeGenFunction::AutoVarEmission GroEmission; + + GetReturnObjectManager(CodeGenFunction &CGF, const CoroutineBodyStmt &S) + : CGF(CGF), Builder(CGF.Builder), S(S), GroActiveFlag(Address::invalid()), +GroEmission(CodeGenFunction::AutoVarEmission::invalid()) {} + + // The gro variable has to outlive coroutine frame and coroutine promise, but, + // it can only be initialized after coroutine promise was created, thus, we + // split its emission in two parts. EmitGroAlloca emits an alloca and sets up + // cleanups. Later when coroutine promise is available we initialize the gro + // and sets the flag that the cleanup is now active. + + void EmitGro
r303716 - [coroutines] Implement correct GRO lifetime
Author: gornishanov Date: Tue May 23 21:38:26 2017 New Revision: 303716 URL: http://llvm.org/viewvc/llvm-project?rev=303716&view=rev Log: [coroutines] Implement correct GRO lifetime Summary: Sema creates a declaration for gro variable as: auto $gro = $promise.get_return_object(); However, gro variable has to outlive coroutine frame and coroutine promise, but, it can only be initialized after the coroutine promise was created, thus, we split its emission in two parts: EmitGroAlloca emits an alloca and sets up the cleanups. Later when the coroutine promise is available we initialize the gro and set the flag that the cleanup is now active. Duplicate of: https://reviews.llvm.org/D31670 (which arc patch refuses to apply for some reason) Reviewers: GorNishanov, rsmith Reviewed By: GorNishanov Subscribers: EricWF, cfe-commits Differential Revision: https://reviews.llvm.org/D33477 Added: cfe/trunk/test/CodeGenCoroutines/coro-gro.cpp Modified: cfe/trunk/lib/CodeGen/CGCoroutine.cpp Modified: cfe/trunk/lib/CodeGen/CGCoroutine.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCoroutine.cpp?rev=303716&r1=303715&r2=303716&view=diff == --- cfe/trunk/lib/CodeGen/CGCoroutine.cpp (original) +++ cfe/trunk/lib/CodeGen/CGCoroutine.cpp Tue May 23 21:38:26 2017 @@ -11,6 +11,7 @@ // //===--===// +#include "CGCleanup.h" #include "CodeGenFunction.h" #include "llvm/ADT/ScopeExit.h" #include "clang/AST/StmtCXX.h" @@ -326,6 +327,72 @@ struct CallCoroDelete final : public EHS }; } +namespace { +struct GetReturnObjectManager { + CodeGenFunction &CGF; + CGBuilderTy &Builder; + const CoroutineBodyStmt &S; + + Address GroActiveFlag; + CodeGenFunction::AutoVarEmission GroEmission; + + GetReturnObjectManager(CodeGenFunction &CGF, const CoroutineBodyStmt &S) + : CGF(CGF), Builder(CGF.Builder), S(S), GroActiveFlag(Address::invalid()), +GroEmission(CodeGenFunction::AutoVarEmission::invalid()) {} + + // The gro variable has to outlive coroutine frame and coroutine promise, but, + // it can only be initialized after coroutine promise was created, thus, we + // split its emission in two parts. EmitGroAlloca emits an alloca and sets up + // cleanups. Later when coroutine promise is available we initialize the gro + // and sets the flag that the cleanup is now active. + + void EmitGroAlloca() { +auto *GroDeclStmt = dyn_cast(S.getResultDecl()); +if (!GroDeclStmt) { + // If get_return_object returns void, no need to do an alloca. + return; +} + +auto *GroVarDecl = cast(GroDeclStmt->getSingleDecl()); + +// Set GRO flag that it is not initialized yet +GroActiveFlag = + CGF.CreateTempAlloca(Builder.getInt1Ty(), CharUnits::One(), "gro.active"); +Builder.CreateStore(Builder.getFalse(), GroActiveFlag); + +GroEmission = CGF.EmitAutoVarAlloca(*GroVarDecl); + +// Remember the top of EHStack before emitting the cleanup. +auto old_top = CGF.EHStack.stable_begin(); +CGF.EmitAutoVarCleanups(GroEmission); +auto top = CGF.EHStack.stable_begin(); + +// Make the cleanup conditional on gro.active +for (auto b = CGF.EHStack.find(top), e = CGF.EHStack.find(old_top); + b != e; b++) { + if (auto *Cleanup = dyn_cast(&*b)) { +assert(!Cleanup->hasActiveFlag() && "cleanup already has active flag?"); +Cleanup->setActiveFlag(GroActiveFlag); +Cleanup->setTestFlagInEHCleanup(); +Cleanup->setTestFlagInNormalCleanup(); + } +} + } + + void EmitGroInit() { +if (!GroActiveFlag.isValid()) { + // No Gro variable was allocated. Simply emit the call to + // get_return_object. + CGF.EmitStmt(S.getResultDecl()); + return; +} + +CGF.EmitAutoVarInit(GroEmission); +Builder.CreateStore(Builder.getTrue(), GroActiveFlag); + } +}; +} + static void emitBodyAndFallthrough(CodeGenFunction &CGF, const CoroutineBodyStmt &S, Stmt *Body) { CGF.EmitStmt(Body); @@ -390,14 +457,18 @@ void CodeGenFunction::EmitCoroutineBody( CGM.getIntrinsic(llvm::Intrinsic::coro_begin), {CoroId, Phi}); CurCoro.Data->CoroBegin = CoroBegin; + GetReturnObjectManager GroManager(*this, S); + GroManager.EmitGroAlloca(); + CurCoro.Data->CleanupJD = getJumpDestInCurrentScope(RetBB); { CodeGenFunction::RunCleanupsScope ResumeScope(*this); EHStack.pushCleanup(NormalAndEHCleanup, S.getDeallocate()); EmitStmt(S.getPromiseDeclStmt()); -EmitStmt(S.getResultDecl()); // FIXME: Gro lifetime is wrong. +// Now we have the promise, initialize the GRO +GroManager.EmitGroInit(); EHStack.pushCleanup(EHCleanup); CurCoro.Data->FinalJD = getJumpDestInCurrentScope(FinalBB); Added: cfe/trunk/test/CodeGenCoroutines/coro-gro.cpp URL: http://llvm.org/viewvc/llvm-project/cfe
[PATCH] D33477: [coroutines] Implement correct GRO lifetime
This revision was automatically updated to reflect the committed changes. Closed by commit rL303716: [coroutines] Implement correct GRO lifetime (authored by GorNishanov). Changed prior to commit: https://reviews.llvm.org/D33477?vs=100029&id=100030#toc Repository: rL LLVM https://reviews.llvm.org/D33477 Files: cfe/trunk/lib/CodeGen/CGCoroutine.cpp cfe/trunk/test/CodeGenCoroutines/coro-gro.cpp Index: cfe/trunk/test/CodeGenCoroutines/coro-gro.cpp === --- cfe/trunk/test/CodeGenCoroutines/coro-gro.cpp +++ cfe/trunk/test/CodeGenCoroutines/coro-gro.cpp @@ -0,0 +1,86 @@ +// Verifies lifetime of __gro local variable +// Verify that coroutine promise and allocated memory are freed up on exception. +// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -disable-llvm-passes | FileCheck %s + +namespace std::experimental { +template struct coroutine_traits; + +template struct coroutine_handle { + coroutine_handle() = default; + static coroutine_handle from_address(void *) noexcept; +}; +template <> struct coroutine_handle { + static coroutine_handle from_address(void *) noexcept; + coroutine_handle() = default; + template + coroutine_handle(coroutine_handle) noexcept; +}; +} + +struct suspend_always { + bool await_ready() noexcept; + void await_suspend(std::experimental::coroutine_handle<>) noexcept; + void await_resume() noexcept; +}; + +struct GroType { + ~GroType(); + operator int() noexcept; +}; + +template <> struct std::experimental::coroutine_traits { + struct promise_type { +GroType get_return_object() noexcept; +suspend_always initial_suspend() noexcept; +suspend_always final_suspend() noexcept; +void return_void() noexcept; +promise_type(); +~promise_type(); +void unhandled_exception() noexcept; + }; +}; + +struct Cleanup { ~Cleanup(); }; +void doSomething() noexcept; + +// CHECK: define i32 @_Z1fv( +int f() { + // CHECK: %[[RetVal:.+]] = alloca i32 + // CHECK: %[[GroActive:.+]] = alloca i1 + + // CHECK: %[[Size:.+]] = call i64 @llvm.coro.size.i64() + // CHECK: call i8* @_Znwm(i64 %[[Size]]) + // CHECK: store i1 false, i1* %[[GroActive]] + // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_typeC1Ev( + // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_type17get_return_objectEv( + // CHECK: store i1 true, i1* %[[GroActive]] + + Cleanup cleanup; + doSomething(); + co_return; + + // CHECK: call void @_Z11doSomethingv( + // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_type11return_voidEv( + // CHECK: call void @_ZN7CleanupD1Ev( + + // Destroy promise and free the memory. + + // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_typeD1Ev( + // CHECK: %[[Mem:.+]] = call i8* @llvm.coro.free( + // CHECK: call void @_ZdlPv(i8* %[[Mem]]) + + // Initialize retval from Gro and destroy Gro + + // CHECK: %[[Conv:.+]] = call i32 @_ZN7GroTypecviEv( + // CHECK: store i32 %[[Conv]], i32* %[[RetVal]] + // CHECK: %[[IsActive:.+]] = load i1, i1* %[[GroActive]] + // CHECK: br i1 %[[IsActive]], label %[[CleanupGro:.+]], label %[[Done:.+]] + + // CHECK: [[CleanupGro]]: + // CHECK: call void @_ZN7GroTypeD1Ev( + // CHECK: br label %[[Done]] + + // CHECK: [[Done]]: + // CHECK: %[[LoadRet:.+]] = load i32, i32* %[[RetVal]] + // CHECK: ret i32 %[[LoadRet]] +} Index: cfe/trunk/lib/CodeGen/CGCoroutine.cpp === --- cfe/trunk/lib/CodeGen/CGCoroutine.cpp +++ cfe/trunk/lib/CodeGen/CGCoroutine.cpp @@ -11,6 +11,7 @@ // //===--===// +#include "CGCleanup.h" #include "CodeGenFunction.h" #include "llvm/ADT/ScopeExit.h" #include "clang/AST/StmtCXX.h" @@ -326,6 +327,72 @@ }; } +namespace { +struct GetReturnObjectManager { + CodeGenFunction &CGF; + CGBuilderTy &Builder; + const CoroutineBodyStmt &S; + + Address GroActiveFlag; + CodeGenFunction::AutoVarEmission GroEmission; + + GetReturnObjectManager(CodeGenFunction &CGF, const CoroutineBodyStmt &S) + : CGF(CGF), Builder(CGF.Builder), S(S), GroActiveFlag(Address::invalid()), +GroEmission(CodeGenFunction::AutoVarEmission::invalid()) {} + + // The gro variable has to outlive coroutine frame and coroutine promise, but, + // it can only be initialized after coroutine promise was created, thus, we + // split its emission in two parts. EmitGroAlloca emits an alloca and sets up + // cleanups. Later when coroutine promise is available we initialize the gro + // and sets the flag that the cleanup is now active. + + void EmitGroAlloca() { +auto *GroDeclStmt = dyn_cast(S.getResultDecl()); +if (!GroDeclStmt) { + // If get_return_object returns void, no need to do an alloca. + return; +} + +auto *GroVarDecl = cast(GroDeclStmt->getSingleDecl()
[PATCH] D31670: [coroutines] Implement correct GRO lifetime
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. Landed as: https://reviews.llvm.org/rL303716 https://reviews.llvm.org/D31670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31670: [coroutines] Implement correct GRO lifetime
GorNishanov closed this revision. GorNishanov added a comment. Closed by commit https://reviews.llvm.org/rL303716: [coroutines] Implement correct GRO lifetime (authored by GorNishanov) https://reviews.llvm.org/D31670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33478: [libclang] When getting platform availabilities, merge multiple declarations if possible
rdwampler created this revision. https://reviews.llvm.org/D33478 Files: test/Index/availability.c tools/libclang/CIndex.cpp Index: tools/libclang/CIndex.cpp === --- tools/libclang/CIndex.cpp +++ tools/libclang/CIndex.cpp @@ -7200,15 +7200,12 @@ return Out; } -static int getCursorPlatformAvailabilityForDecl(const Decl *D, -int *always_deprecated, -CXString *deprecated_message, -int *always_unavailable, -CXString *unavailable_message, - CXPlatformAvailability *availability, -int availability_size) { +static void getCursorPlatformAvailabilityForDecl( +const Decl *D, int *always_deprecated, CXString *deprecated_message, +int *always_unavailable, CXString *unavailable_message, +SmallVectorImpl &AvailabilityAttrs) { bool HadAvailAttr = false; - int N = 0; + ASTContext &Ctx = D->getASTContext(); for (auto A : D->attrs()) { if (DeprecatedAttr *Deprecated = dyn_cast(A)) { HadAvailAttr = true; @@ -7220,7 +7217,7 @@ } continue; } - + if (UnavailableAttr *Unavailable = dyn_cast(A)) { HadAvailAttr = true; if (always_unavailable) @@ -7231,38 +7228,71 @@ } continue; } - + if (AvailabilityAttr *Avail = dyn_cast(A)) { + AvailabilityAttrs.push_back(Avail); HadAvailAttr = true; - if (N < availability_size) { -availability[N].Platform - = cxstring::createDup(Avail->getPlatform()->getName()); -availability[N].Introduced = convertVersion(Avail->getIntroduced()); -availability[N].Deprecated = convertVersion(Avail->getDeprecated()); -availability[N].Obsoleted = convertVersion(Avail->getObsoleted()); -availability[N].Unavailable = Avail->getUnavailable(); -availability[N].Message = cxstring::createDup(Avail->getMessage()); - } - ++N; } } + if (!AvailabilityAttrs.empty()) { +std::sort(AvailabilityAttrs.begin(), AvailabilityAttrs.end(), + [](AvailabilityAttr *LHS, AvailabilityAttr *RHS) { +return LHS->getPlatform() > RHS->getPlatform(); + }); +auto It = std::unique( +AvailabilityAttrs.begin(), AvailabilityAttrs.end(), +[&Ctx](AvailabilityAttr *LHS, AvailabilityAttr *RHS) { + if (LHS->getPlatform() == RHS->getPlatform()) { +if (LHS->getIntroduced() == RHS->getIntroduced() && +LHS->getDeprecated() == RHS->getDeprecated() && +LHS->getObsoleted() == RHS->getObsoleted() && +LHS->getMessage() == RHS->getMessage() && +LHS->getReplacement() == RHS->getReplacement()) + return true; + +if ((!LHS->getIntroduced().empty() && + !RHS->getIntroduced().empty()) || +(!LHS->getDeprecated().empty() && + !RHS->getDeprecated().empty()) || +(!LHS->getObsoleted().empty() && + !RHS->getObsoleted().empty()) || +(!LHS->getMessage().empty() && !RHS->getMessage().empty())) + return false; + +if (LHS->getIntroduced().empty() && !RHS->getIntroduced().empty()) + LHS->setIntroduced(Ctx, RHS->getIntroduced()); + +if (LHS->getDeprecated().empty() && !RHS->getDeprecated().empty()) { + LHS->setDeprecated(Ctx, RHS->getDeprecated()); + if (!LHS->getMessage().empty()) +LHS->setMessage(Ctx, RHS->getMessage()); + if (!LHS->getReplacement().empty()) +LHS->setReplacement(Ctx, RHS->getReplacement()); +} + +if (LHS->getObsoleted().empty() && !RHS->getObsoleted().empty()) { + LHS->setObsoleted(Ctx, RHS->getObsoleted()); + if (!LHS->getMessage().empty()) +LHS->setMessage(Ctx, RHS->getMessage()); +} + +return true; + } + return false; +}); +AvailabilityAttrs.erase(It, AvailabilityAttrs.end()); + } + if (!HadAvailAttr) if (const EnumConstantDecl *EnumConst = dyn_cast(D)) - return getCursorPlatformAvailabilityForDecl( -cast(EnumConst->getDeclContext()), - always_deprecated, - deprecated_message, - always_unavailable, - unavailable_message, - availability, - availability_size); - - return N
[PATCH] D33479: [coroutines] [NFC] Add tests for return_void, unhandled_exception and promise dtor
GorNishanov created this revision. - Test that coroutine promise destructor is called. - Test that we call return_void on fallthrough - Test that we call unhandled exception in a try catch surrounding the body https://reviews.llvm.org/D33479 Files: test/CodeGenCoroutines/Inputs/coroutine.h test/CodeGenCoroutines/coro-promise-dtor.cpp test/CodeGenCoroutines/coro-ret-void.cpp test/CodeGenCoroutines/coro-unhandled-exception.cpp Index: test/CodeGenCoroutines/coro-unhandled-exception.cpp === --- /dev/null +++ test/CodeGenCoroutines/coro-unhandled-exception.cpp @@ -0,0 +1,72 @@ +// RUN: %clang_cc1 -std=c++14 -fcoroutines-ts -triple=x86_64-pc-windows-msvc18.0.0 -emit-llvm %s -o - -fexceptions -fcxx-exceptions -disable-llvm-passes | FileCheck %s +// RUN: %clang_cc1 -std=c++14 -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -fexceptions -fcxx-exceptions -disable-llvm-passes | FileCheck --check-prefix=CHECK-LPAD %s + +#include "Inputs/coroutine.h" + +namespace coro = std::experimental::coroutines_v1; + +namespace std { + using exception_ptr = int; + exception_ptr current_exception(); +} + +struct coro_t { + struct promise_type { +coro_t get_return_object() { + coro::coroutine_handle{}; + return {}; +} +coro::suspend_never initial_suspend() { return {}; } +coro::suspend_never final_suspend() { return {}; } +void return_void(){} +void unhandled_exception() noexcept; + }; +}; + +struct Cleanup { ~Cleanup(); }; +void may_throw(); + +coro_t f() { + Cleanup x; + may_throw(); + co_return; +} + +// CHECK: @"\01?f@@YA?AUcoro_t@@XZ"( +// CHECK: invoke void @"\01?may_throw@@YAXXZ"() +// CHECK: to label %{{.+}} unwind label %[[EHCLEANUP:.+]] +// CHECK: [[EHCLEANUP]]: +// CHECK: %[[INNERPAD:.+]] = cleanuppad within none [] +// CHECK: call void @"\01??_DCleanup@@QEAAXXZ"( +// CHECK: cleanupret from %[[INNERPAD]] unwind label %[[CATCHSW:.+]] +// CHECK: [[CATCHSW]]: +// CHECK: %[[CATCHSWTOK:.+]] = catchswitch within none [label %[[CATCH:.+]]] unwind label +// CHECK: [[CATCH]]: +// CHECK: %[[CATCHTOK:.+]] = catchpad within [[CATCHSWTOK:.+]] +// CHECK: call void @"\01?unhandled_exception@promise_type@coro_t@@QEAAXXZ" +// CHECK: catchret from %[[CATCHTOK]] to label %[[CATCHRETDEST:.+]] +// CHECK: [[CATCHRETDEST]]: +// CHECK-NEXT: br label %[[TRYCONT:.+]] +// CHECK: [[TRYCONT]]: +// CHECK-NEXT: br label %[[COROFIN:.+]] +// CHECK: [[COROFIN]]: +// CHECK-NEXT: invoke void @"\01?final_suspend@promise_type@coro_t@@QEAA?AUsuspend_never@coroutines_v1@experimental@std@@XZ"( + +// CHECK-LPAD: @_Z1fv( +// CHECK-LPAD: invoke void @_Z9may_throwv() +// CHECK-LPAD: to label %[[CONT:.+]] unwind label %[[CLEANUP:.+]] +// CHECK-LPAD: [[CLEANUP]]: +// CHECK-LPAD: call void @_ZN7CleanupD1Ev(%struct.Cleanup* %x) #2 +// CHECK-LPAD: br label %[[CATCH:.+]] + +// CHECK-LPAD: [[CATCH]]: +// CHECK-LPAD:call i8* @__cxa_begin_catch +// CHECK-LPAD:call void @_ZN6coro_t12promise_type19unhandled_exceptionEv(%"struct.coro_t::promise_type"* %__promise) #2 +// CHECK-LPAD:invoke void @__cxa_end_catch() +// CHECK-LPAD-NEXT: to label %[[CATCHRETDEST:.+]] unwind label +// CHECK-LPAD: [[CATCHRETDEST]]: +// CHECK-LPAD-NEXT: br label %[[TRYCONT:.+]] +// CHECK-LPAD: [[TRYCONT]]: +// CHECK-LPAD-NEXT: br label %[[COROFIN:.+]] +// CHECK-LPAD: [[COROFIN]]: +// CHECK-LPAD-NEXT: invoke void @_ZN6coro_t12promise_type13final_suspendEv( Index: test/CodeGenCoroutines/coro-ret-void.cpp === --- /dev/null +++ test/CodeGenCoroutines/coro-ret-void.cpp @@ -0,0 +1,39 @@ +// RUN: %clang_cc1 -std=c++14 -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s + +#include "Inputs/coroutine.h" + +namespace coro = std::experimental::coroutines_v1; + +struct coro1 { + struct promise_type { +coro1 get_return_object(); +coro::suspend_never initial_suspend(); +coro::suspend_never final_suspend(); +void return_void(); + }; +}; + +coro1 f() { + co_await coro::suspend_never{}; +} + +// CHECK-LABEL: define void @_Z1fv( +// CHECK: call void @_ZNSt12experimental13coroutines_v113suspend_never12await_resumeEv(%"struct.std::experimental::coroutines_v1::suspend_never"* +// CHECK: call void @_ZN5coro112promise_type11return_voidEv(%"struct.coro1::promise_type"* %__promise) + +struct coro2 { + struct promise_type { +coro2 get_return_object(); +coro::suspend_never initial_suspend(); +coro::suspend_never final_suspend(); +void return_value(int); + }; +}; + +coro2 g() { + co_return 42; +} + +// CHECK-LABEL: define void @_Z1gv( +// CHECK: call void @_ZNSt12experimental13coroutines_v113suspend_never12await_resumeEv(%"struct.std::experimental::coroutines_v1::suspend_never"* +// CHECK: call void @_ZN5coro212promise_type12return_valueEi(%"struct.coro2::promise_type"* %__promise, i32 42) Index: t
[PATCH] D33479: [coroutines] [NFC] Add tests for return_void, unhandled_exception and promise dtor
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D33479 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33481: [coroutines] Improved diagnostics when unhandled_exception is missing in the promise_type
GorNishanov created this revision. Now we helpfully provide a note pointing at the promise_type in question. https://reviews.llvm.org/D33481 Files: lib/Sema/SemaCoroutine.cpp test/SemaCXX/coroutine-unhandled_exception-warning.cpp test/SemaCXX/coroutines.cpp Index: test/SemaCXX/coroutines.cpp === --- test/SemaCXX/coroutines.cpp +++ test/SemaCXX/coroutines.cpp @@ -540,7 +540,7 @@ } template coro bad_implicit_return_dependent(bad_promise_6); // expected-note {{in instantiation}} -struct bad_promise_7 { +struct bad_promise_7 { // expected-note 2 {{defined here}} coro get_return_object(); suspend_always initial_suspend(); suspend_always final_suspend(); Index: test/SemaCXX/coroutine-unhandled_exception-warning.cpp === --- test/SemaCXX/coroutine-unhandled_exception-warning.cpp +++ test/SemaCXX/coroutine-unhandled_exception-warning.cpp @@ -16,7 +16,11 @@ using std::experimental::suspend_always; using std::experimental::suspend_never; +#ifndef DISABLE_WARNING +struct promise_void { // expected-note {{defined here}} +#else struct promise_void { +#endif void get_return_object(); suspend_always initial_suspend(); suspend_always final_suspend(); Index: lib/Sema/SemaCoroutine.cpp === --- lib/Sema/SemaCoroutine.cpp +++ lib/Sema/SemaCoroutine.cpp @@ -120,8 +120,7 @@ return PromiseType; } -/// Look up the std::coroutine_traits<...>::promise_type for the given -/// function type. +/// Look up the std::experimental::coroutine_handle. static QualType lookupCoroutineHandleType(Sema &S, QualType PromiseType, SourceLocation Loc) { if (PromiseType.isNull()) @@ -729,8 +728,7 @@ } if (isa(Body)) { -// FIXME(EricWF): Nothing todo. the body is already a transformed coroutine -// body statement. +// Nothing todo. the body is already a transformed coroutine body statement. return; } @@ -1030,6 +1028,8 @@ : diag:: warn_coroutine_promise_unhandled_exception_required_with_exceptions; S.Diag(Loc, DiagID) << PromiseRecordDecl; +S.Diag(PromiseRecordDecl->getLocation(), diag::note_defined_here) +<< PromiseRecordDecl; return !RequireUnhandledException; } Index: test/SemaCXX/coroutines.cpp === --- test/SemaCXX/coroutines.cpp +++ test/SemaCXX/coroutines.cpp @@ -540,7 +540,7 @@ } template coro bad_implicit_return_dependent(bad_promise_6); // expected-note {{in instantiation}} -struct bad_promise_7 { +struct bad_promise_7 { // expected-note 2 {{defined here}} coro get_return_object(); suspend_always initial_suspend(); suspend_always final_suspend(); Index: test/SemaCXX/coroutine-unhandled_exception-warning.cpp === --- test/SemaCXX/coroutine-unhandled_exception-warning.cpp +++ test/SemaCXX/coroutine-unhandled_exception-warning.cpp @@ -16,7 +16,11 @@ using std::experimental::suspend_always; using std::experimental::suspend_never; +#ifndef DISABLE_WARNING +struct promise_void { // expected-note {{defined here}} +#else struct promise_void { +#endif void get_return_object(); suspend_always initial_suspend(); suspend_always final_suspend(); Index: lib/Sema/SemaCoroutine.cpp === --- lib/Sema/SemaCoroutine.cpp +++ lib/Sema/SemaCoroutine.cpp @@ -120,8 +120,7 @@ return PromiseType; } -/// Look up the std::coroutine_traits<...>::promise_type for the given -/// function type. +/// Look up the std::experimental::coroutine_handle. static QualType lookupCoroutineHandleType(Sema &S, QualType PromiseType, SourceLocation Loc) { if (PromiseType.isNull()) @@ -729,8 +728,7 @@ } if (isa(Body)) { -// FIXME(EricWF): Nothing todo. the body is already a transformed coroutine -// body statement. +// Nothing todo. the body is already a transformed coroutine body statement. return; } @@ -1030,6 +1028,8 @@ : diag:: warn_coroutine_promise_unhandled_exception_required_with_exceptions; S.Diag(Loc, DiagID) << PromiseRecordDecl; +S.Diag(PromiseRecordDecl->getLocation(), diag::note_defined_here) +<< PromiseRecordDecl; return !RequireUnhandledException; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33481: [coroutines] Improved diagnostics when unhandled_exception is missing in the promise_type
GorNishanov accepted this revision. GorNishanov added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D33481 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33450: Warn about uses of `@available` that can't suppress the -Wunguarded-availability warnings
erik.pilkington accepted this revision. erik.pilkington added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: lib/Sema/SemaExpr.cpp:15747 + // warn when it's used inappropriately (i.e. not if(@available)). + if (getCurFunctionOrMethodDecl()) +getEnclosingFunction()->HasPotentialAvailabilityViolations = true; erik.pilkington wrote: > I believe this will not be set if we're rebuilding a templated function, we > should also flag this from `TransformObjCAvailabilityCheckExpr` in > TreeTransform.h. On second thought, there is no point in doing that because we'd have already emitted a diagnostic from the pattern. This is correct as is. Repository: rL LLVM https://reviews.llvm.org/D33450 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33368: [libcxxabi][demangler] Fix a crash in the demangler
erik.pilkington added inline comments. Comment at: src/cxa_demangle.cpp:3036 break; -if (db.names.size() < 2) +assert(k0 <= k1 && "parse_type() mutated the name stack"); +if (k1 == k0) dexonsmith wrote: > There's no `#include ` in this file. Depending on the standard > library you're building against, you made need that here. Sure, I'll commit this with that change. https://reviews.llvm.org/D33368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxxabi] r303718 - [demangler] Fix a crash in the demangler during parsing of a lamdba
Author: epilk Date: Wed May 24 00:44:19 2017 New Revision: 303718 URL: http://llvm.org/viewvc/llvm-project?rev=303718&view=rev Log: [demangler] Fix a crash in the demangler during parsing of a lamdba The problem is that multiple types could have been parsed from parse_type(), which the lamdba parameter parsing didn't handle. Differential revision: https://reviews.llvm.org/D33368 Modified: libcxxabi/trunk/src/cxa_demangle.cpp libcxxabi/trunk/test/test_demangle.pass.cpp Modified: libcxxabi/trunk/src/cxa_demangle.cpp URL: http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/cxa_demangle.cpp?rev=303718&r1=303717&r2=303718&view=diff == --- libcxxabi/trunk/src/cxa_demangle.cpp (original) +++ libcxxabi/trunk/src/cxa_demangle.cpp Wed May 24 00:44:19 2017 @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -3015,6 +3016,7 @@ parse_unnamed_type_name(const char* firs break; case 'l': { +size_t lambda_pos = db.names.size(); db.names.push_back(typename C::String("'lambda'(")); const char* t0 = first+2; if (first[2] == 'v') @@ -3024,36 +3026,41 @@ parse_unnamed_type_name(const char* firs } else { -const char* t1 = parse_type(t0, last, db); -if (t1 == t0) -{ -if(!db.names.empty()) -db.names.pop_back(); -return first; -} -if (db.names.size() < 2) -return first; -auto tmp = db.names.back().move_full(); -db.names.pop_back(); -db.names.back().first.append(tmp); -t0 = t1; +bool is_first_it = true; while (true) { -t1 = parse_type(t0, last, db); +long k0 = static_cast(db.names.size()); +const char* t1 = parse_type(t0, last, db); +long k1 = static_cast(db.names.size()); if (t1 == t0) break; -if (db.names.size() < 2) +assert(k0 <= k1 && "parse_type() mutated the name stack"); +if (k1 == k0) return first; -tmp = db.names.back().move_full(); -db.names.pop_back(); -if (!tmp.empty()) -{ -db.names.back().first.append(", "); -db.names.back().first.append(tmp); -} +// If the call to parse_type above found a pack expansion +// substitution, then multiple names could have been +// inserted into the name table. Walk through the names, +// appending each onto the lambda's parameter list. +std::for_each(db.names.begin() + k0, db.names.begin() + k1, + [&](typename C::sub_type::value_type &pair) { + if (pair.empty()) + return; + auto &lambda = db.names[lambda_pos].first; + if (!is_first_it) + lambda.append(", "); + is_first_it = false; + lambda.append(pair.move_full()); + }); +db.names.erase(db.names.begin() + k0, db.names.end()); t0 = t1; } -if(db.names.empty()) +if (is_first_it) +{ +if (!db.names.empty()) +db.names.pop_back(); +return first; +} +if (db.names.empty() || db.names.size() - 1 != lambda_pos) return first; db.names.back().first.append(")"); } @@ -4964,6 +4971,7 @@ struct string_pair string_pair(const char (&s)[N]) : first(s, N-1) {} size_t size() const {return first.size() + second.size();} +bool empty() const { return first.empty() && second.empty(); } StrT full() const {return first + second;} StrT move_full() {return std::move(first) + std::move(second);} }; Modified: libcxxabi/trunk/test/test_demangle.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/test/test_demangle.pass.cpp?rev=303718&r1=303717&r2=303718&view=diff == --- libcxxabi/trunk/test/test_demangle.pass.cpp (original) +++ libcxxabi/trunk/test/test_demangle.pass.cpp Wed May 24 00:44:19 2017
[PATCH] D33368: [libcxxabi][demangler] Fix a crash in the demangler
This revision was automatically updated to reflect the committed changes. Closed by commit rL303718: [demangler] Fix a crash in the demangler during parsing of a lamdba (authored by epilk). Changed prior to commit: https://reviews.llvm.org/D33368?vs=99930&id=100041#toc Repository: rL LLVM https://reviews.llvm.org/D33368 Files: libcxxabi/trunk/src/cxa_demangle.cpp libcxxabi/trunk/test/test_demangle.pass.cpp Index: libcxxabi/trunk/src/cxa_demangle.cpp === --- libcxxabi/trunk/src/cxa_demangle.cpp +++ libcxxabi/trunk/src/cxa_demangle.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -3015,6 +3016,7 @@ break; case 'l': { +size_t lambda_pos = db.names.size(); db.names.push_back(typename C::String("'lambda'(")); const char* t0 = first+2; if (first[2] == 'v') @@ -3024,36 +3026,41 @@ } else { -const char* t1 = parse_type(t0, last, db); -if (t1 == t0) -{ -if(!db.names.empty()) -db.names.pop_back(); -return first; -} -if (db.names.size() < 2) -return first; -auto tmp = db.names.back().move_full(); -db.names.pop_back(); -db.names.back().first.append(tmp); -t0 = t1; +bool is_first_it = true; while (true) { -t1 = parse_type(t0, last, db); +long k0 = static_cast(db.names.size()); +const char* t1 = parse_type(t0, last, db); +long k1 = static_cast(db.names.size()); if (t1 == t0) break; -if (db.names.size() < 2) +assert(k0 <= k1 && "parse_type() mutated the name stack"); +if (k1 == k0) return first; -tmp = db.names.back().move_full(); -db.names.pop_back(); -if (!tmp.empty()) -{ -db.names.back().first.append(", "); -db.names.back().first.append(tmp); -} +// If the call to parse_type above found a pack expansion +// substitution, then multiple names could have been +// inserted into the name table. Walk through the names, +// appending each onto the lambda's parameter list. +std::for_each(db.names.begin() + k0, db.names.begin() + k1, + [&](typename C::sub_type::value_type &pair) { + if (pair.empty()) + return; + auto &lambda = db.names[lambda_pos].first; + if (!is_first_it) + lambda.append(", "); + is_first_it = false; + lambda.append(pair.move_full()); + }); +db.names.erase(db.names.begin() + k0, db.names.end()); t0 = t1; } -if(db.names.empty()) +if (is_first_it) +{ +if (!db.names.empty()) +db.names.pop_back(); +return first; +} +if (db.names.empty() || db.names.size() - 1 != lambda_pos) return first; db.names.back().first.append(")"); } @@ -4964,6 +4971,7 @@ string_pair(const char (&s)[N]) : first(s, N-1) {} size_t size() const {return first.size() + second.size();} +bool empty() const { return first.empty() && second.empty(); } StrT full() const {return first + second;} StrT move_full() {return std::move(first) + std::move(second);} }; Index: libcxxabi/trunk/test/test_demangle.pass.cpp === --- libcxxabi/trunk/test/test_demangle.pass.cpp +++ libcxxabi/trunk/test/test_demangle.pass.cpp @@ -29604,6 +29604,7 @@ {"PFvRmOE", "void (*)(unsigned long&) &&"}, {"_ZTW1x", "thread-local wrapper routine for x"}, {"_ZTHN3fooE", "thread-local initialization routine for foo"}, +{"_Z4algoIJiiiEEvZ1gEUlT_E_", "void algo(g::'lambda'(int, int, int))"}, }; const unsigned N = sizeof(cases) / sizeof(cases[0]); @@ -29664,7 +29665,8 @@ "\x44\x74\x71\x75\x35\x2A\xDF\x74\x44\x61\x73\x63\x35\x2A\x3B\x41\x72\x4D\x6E\x65\x34\x9F\xC1\x63\x41\x72\x4D\x6E\x77\x38\x9A\x8E\x44\x6F\x64\x6C\x53\
r303719 - [XRay][clang] Allow imbuing arg1 logging attribute via -fxray-always-instrument=
Author: dberris Date: Wed May 24 00:46:36 2017 New Revision: 303719 URL: http://llvm.org/viewvc/llvm-project?rev=303719&view=rev Log: [XRay][clang] Allow imbuing arg1 logging attribute via -fxray-always-instrument= Summary: This change allows us to add arg1 logging support to functions through the special case list provided through -fxray-always-instrument=. This is useful for adding arg1 logging to functions that are either in headers that users don't have control over (i.e. cannot change the source) or would rather not do. It only takes effect when the pattern is matched through the "fun:" special case, as a category. As in: fun:*pattern=arg1 Reviewers: pelikan, rnk Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D33392 Added: cfe/trunk/test/CodeGen/xray-imbue-arg1.cpp Modified: cfe/trunk/include/clang/Basic/XRayLists.h cfe/trunk/lib/Basic/XRayLists.cpp cfe/trunk/lib/CodeGen/CodeGenModule.cpp Modified: cfe/trunk/include/clang/Basic/XRayLists.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/XRayLists.h?rev=303719&r1=303718&r2=303719&view=diff == --- cfe/trunk/include/clang/Basic/XRayLists.h (original) +++ cfe/trunk/include/clang/Basic/XRayLists.h Wed May 24 00:46:36 2017 @@ -37,6 +37,7 @@ public: NONE, ALWAYS, NEVER, +ALWAYS_ARG1, }; ImbueAttribute shouldImbueFunction(StringRef FunctionName) const; Modified: cfe/trunk/lib/Basic/XRayLists.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/XRayLists.cpp?rev=303719&r1=303718&r2=303719&view=diff == --- cfe/trunk/lib/Basic/XRayLists.cpp (original) +++ cfe/trunk/lib/Basic/XRayLists.cpp Wed May 24 00:46:36 2017 @@ -26,6 +26,8 @@ XRayFunctionFilter::ImbueAttribute XRayFunctionFilter::shouldImbueFunction(StringRef FunctionName) const { // First apply the always instrument list, than if it isn't an "always" see // whether it's treated as a "never" instrument function. + if (AlwaysInstrument->inSection("fun", FunctionName, "arg1")) +return ImbueAttribute::ALWAYS_ARG1; if (AlwaysInstrument->inSection("fun", FunctionName)) return ImbueAttribute::ALWAYS; if (NeverInstrument->inSection("fun", FunctionName)) Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=303719&r1=303718&r2=303719&view=diff == --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Wed May 24 00:46:36 2017 @@ -1508,6 +1508,10 @@ bool CodeGenModule::imbueXRayAttrs(llvm: case ImbueAttr::ALWAYS: Fn->addFnAttr("function-instrument", "xray-always"); break; + case ImbueAttr::ALWAYS_ARG1: +Fn->addFnAttr("function-instrument", "xray-always"); +Fn->addFnAttr("xray-log-args", "1"); +break; case ImbueAttr::NEVER: Fn->addFnAttr("function-instrument", "xray-never"); break; Added: cfe/trunk/test/CodeGen/xray-imbue-arg1.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/xray-imbue-arg1.cpp?rev=303719&view=auto == --- cfe/trunk/test/CodeGen/xray-imbue-arg1.cpp (added) +++ cfe/trunk/test/CodeGen/xray-imbue-arg1.cpp Wed May 24 00:46:36 2017 @@ -0,0 +1,12 @@ +// RUN: echo "fun:*arg1*=arg1" >> %t.always-instrument +// RUN: %clang_cc1 -fxray-instrument -x c++ -std=c++11 -fxray-always-instrument=%t.always-instrument -emit-llvm -o - %s -triple x86_64-unknown-linux-gnu | FileCheck %s + +void foo() {} + +void arg1(void*) {} + +// CHECK: define void @_Z3foov() #[[FOO:[0-9]+]] { +// CHECK: define void {{.*}}arg1{{.*}} #[[ALWAYSARG1:[0-9]+]] { + +// CHECK: attributes #[[FOO]] = {{.*}} +// CHECK: attributes #[[ALWAYSARG1]] = {{.*}} "function-instrument"="xray-always" {{.*}} "xray-log-args"="1" ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33392: [XRay][clang] Allow imbuing arg1 logging attribute via -fxray-always-instrument=
This revision was automatically updated to reflect the committed changes. Closed by commit rL303719: [XRay][clang] Allow imbuing arg1 logging attribute via -fxray-always-instrument= (authored by dberris). Changed prior to commit: https://reviews.llvm.org/D33392?vs=99715&id=100042#toc Repository: rL LLVM https://reviews.llvm.org/D33392 Files: cfe/trunk/include/clang/Basic/XRayLists.h cfe/trunk/lib/Basic/XRayLists.cpp cfe/trunk/lib/CodeGen/CodeGenModule.cpp cfe/trunk/test/CodeGen/xray-imbue-arg1.cpp Index: cfe/trunk/lib/CodeGen/CodeGenModule.cpp === --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp @@ -1508,6 +1508,10 @@ case ImbueAttr::ALWAYS: Fn->addFnAttr("function-instrument", "xray-always"); break; + case ImbueAttr::ALWAYS_ARG1: +Fn->addFnAttr("function-instrument", "xray-always"); +Fn->addFnAttr("xray-log-args", "1"); +break; case ImbueAttr::NEVER: Fn->addFnAttr("function-instrument", "xray-never"); break; Index: cfe/trunk/lib/Basic/XRayLists.cpp === --- cfe/trunk/lib/Basic/XRayLists.cpp +++ cfe/trunk/lib/Basic/XRayLists.cpp @@ -26,6 +26,8 @@ XRayFunctionFilter::shouldImbueFunction(StringRef FunctionName) const { // First apply the always instrument list, than if it isn't an "always" see // whether it's treated as a "never" instrument function. + if (AlwaysInstrument->inSection("fun", FunctionName, "arg1")) +return ImbueAttribute::ALWAYS_ARG1; if (AlwaysInstrument->inSection("fun", FunctionName)) return ImbueAttribute::ALWAYS; if (NeverInstrument->inSection("fun", FunctionName)) Index: cfe/trunk/include/clang/Basic/XRayLists.h === --- cfe/trunk/include/clang/Basic/XRayLists.h +++ cfe/trunk/include/clang/Basic/XRayLists.h @@ -37,6 +37,7 @@ NONE, ALWAYS, NEVER, +ALWAYS_ARG1, }; ImbueAttribute shouldImbueFunction(StringRef FunctionName) const; Index: cfe/trunk/test/CodeGen/xray-imbue-arg1.cpp === --- cfe/trunk/test/CodeGen/xray-imbue-arg1.cpp +++ cfe/trunk/test/CodeGen/xray-imbue-arg1.cpp @@ -0,0 +1,12 @@ +// RUN: echo "fun:*arg1*=arg1" >> %t.always-instrument +// RUN: %clang_cc1 -fxray-instrument -x c++ -std=c++11 -fxray-always-instrument=%t.always-instrument -emit-llvm -o - %s -triple x86_64-unknown-linux-gnu | FileCheck %s + +void foo() {} + +void arg1(void*) {} + +// CHECK: define void @_Z3foov() #[[FOO:[0-9]+]] { +// CHECK: define void {{.*}}arg1{{.*}} #[[ALWAYSARG1:[0-9]+]] { + +// CHECK: attributes #[[FOO]] = {{.*}} +// CHECK: attributes #[[ALWAYSARG1]] = {{.*}} "function-instrument"="xray-always" {{.*}} "xray-log-args"="1" Index: cfe/trunk/lib/CodeGen/CodeGenModule.cpp === --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp @@ -1508,6 +1508,10 @@ case ImbueAttr::ALWAYS: Fn->addFnAttr("function-instrument", "xray-always"); break; + case ImbueAttr::ALWAYS_ARG1: +Fn->addFnAttr("function-instrument", "xray-always"); +Fn->addFnAttr("xray-log-args", "1"); +break; case ImbueAttr::NEVER: Fn->addFnAttr("function-instrument", "xray-never"); break; Index: cfe/trunk/lib/Basic/XRayLists.cpp === --- cfe/trunk/lib/Basic/XRayLists.cpp +++ cfe/trunk/lib/Basic/XRayLists.cpp @@ -26,6 +26,8 @@ XRayFunctionFilter::shouldImbueFunction(StringRef FunctionName) const { // First apply the always instrument list, than if it isn't an "always" see // whether it's treated as a "never" instrument function. + if (AlwaysInstrument->inSection("fun", FunctionName, "arg1")) +return ImbueAttribute::ALWAYS_ARG1; if (AlwaysInstrument->inSection("fun", FunctionName)) return ImbueAttribute::ALWAYS; if (NeverInstrument->inSection("fun", FunctionName)) Index: cfe/trunk/include/clang/Basic/XRayLists.h === --- cfe/trunk/include/clang/Basic/XRayLists.h +++ cfe/trunk/include/clang/Basic/XRayLists.h @@ -37,6 +37,7 @@ NONE, ALWAYS, NEVER, +ALWAYS_ARG1, }; ImbueAttribute shouldImbueFunction(StringRef FunctionName) const; Index: cfe/trunk/test/CodeGen/xray-imbue-arg1.cpp === --- cfe/trunk/test/CodeGen/xray-imbue-arg1.cpp +++ cfe/trunk/test/CodeGen/xray-imbue-arg1.cpp @@ -0,0 +1,12 @@ +// RUN: echo "fun:*arg1*=arg1" >> %t.always-instrument +// RUN: %clang_cc1 -fxray-instrument -x c++ -std=c++11 -fxray-always-instrument=%t.always-instrument -emit-llvm -o - %s -triple x86_64-unknown-linux-gnu | FileCheck %s + +void foo() {} + +void arg1(void*) {} + +// CHECK: define void @_Z3fo
[PATCH] D32332: Add support for transparent overloadable functions in clang
george.burgess.iv updated this revision to Diff 99851. george.burgess.iv added a comment. Remove the `transparent_overloadable` attribute entirely. This approach presents one problem that I didn't see until I implemented it: I'd like to have something to detect that this feature exists. The quick fix seems to be "readd `transparently_overloadable`, make it equivalent to not having the attribute at all, and be happy," but that feels really icky (as does adding a special `__has_enhanced_overloadable` or whatever macro just for this). Do we have a standard way of saying "does clang support an enhanced version of attribute X"? If not, I'm happy to put together a patch so people can query for that in a somewhat uniform way. This would let users write something like `__has_attribute_enhancement(overloadable, unmarked_overloads)`, and would be more broadly useful if we decide to ever add features to another attribute in the future (`diagnose_if` comes to mind if I can ever find time to get back to it...). Apologies for the lag; life is busy. :) https://reviews.llvm.org/D32332 Files: include/clang/Basic/AttrDocs.td include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaDecl.cpp test/CodeGen/mangle-ms.c test/CodeGen/mangle.c test/CodeGenCXX/mangle-ms.cpp test/PCH/attrs.c test/Sema/overloadable.c Index: test/Sema/overloadable.c === --- test/Sema/overloadable.c +++ test/Sema/overloadable.c @@ -3,8 +3,8 @@ int var __attribute__((overloadable)); // expected-error{{'overloadable' attribute only applies to functions}} void params(void) __attribute__((overloadable(12))); // expected-error {{'overloadable' attribute takes no arguments}} -int *f(int) __attribute__((overloadable)); // expected-note 2{{previous overload of function is here}} -float *f(float); // expected-error{{overloaded function 'f' must have the 'overloadable' attribute}} +int *f(int) __attribute__((overloadable)); // expected-note{{previous overload of function is here}} +float *f(float); int *f(int); // expected-error{{redeclaration of 'f' must have the 'overloadable' attribute}} \ // expected-note{{previous declaration is here}} double *f(double) __attribute__((overloadable)); // okay, new @@ -71,19 +71,19 @@ f1(); } -void before_local_1(int) __attribute__((overloadable)); // expected-note {{here}} +void before_local_1(int) __attribute__((overloadable)); void before_local_2(int); // expected-note {{here}} void before_local_3(int) __attribute__((overloadable)); void local() { - void before_local_1(char); // expected-error {{must have the 'overloadable' attribute}} - void before_local_2(char) __attribute__((overloadable)); // expected-error {{conflicting types}} + void before_local_1(char); + void before_local_2(char); // expected-error {{conflicting types}} void before_local_3(char) __attribute__((overloadable)); - void after_local_1(char); // expected-note {{here}} - void after_local_2(char) __attribute__((overloadable)); // expected-note {{here}} + void after_local_1(char); + void after_local_2(char) __attribute__((overloadable)); void after_local_3(char) __attribute__((overloadable)); } -void after_local_1(int) __attribute__((overloadable)); // expected-error {{conflicting types}} -void after_local_2(int); // expected-error {{must have the 'overloadable' attribute}} +void after_local_1(int) __attribute__((overloadable)); +void after_local_2(int); void after_local_3(int) __attribute__((overloadable)); // Make sure we allow C-specific conversions in C. @@ -106,8 +106,8 @@ void foo(char *c) __attribute__((overloadable)); void (*ptr1)(void *) = &foo; void (*ptr2)(char *) = &foo; - void (*ambiguous)(int *) = &foo; // expected-error{{initializing 'void (*)(int *)' with an expression of incompatible type ''}} expected-note@105{{candidate function}} expected-note@106{{candidate function}} - void *vp_ambiguous = &foo; // expected-error{{initializing 'void *' with an expression of incompatible type ''}} expected-note@105{{candidate function}} expected-note@106{{candidate function}} + void (*ambiguous)(int *) = &foo; // expected-error{{initializing 'void (*)(int *)' with an expression of incompatible type ''}} expected-note@-4{{candidate function}} expected-note@-3{{candidate function}} + void *vp_ambiguous = &foo; // expected-error{{initializing 'void *' with an expression of incompatible type ''}} expected-note@-5{{candidate function}} expected-note@-4{{candidate function}} void (*specific1)(int *) = (void (*)(void *))&foo; // expected-warning{{incompatible function pointer types initializing 'void (*)(int *)' with an expression of type 'void (*)(void *)'}} void *specific2 = (void (*)(void *))&foo; @@ -117,8 +117,8 @@ void disabled(char *c) __attribute__((overloadable, enable_if(1, "The function name lies."))); // To be clear, these should all point to the last
[PATCH] D33398: Remove __unaligned preventively when mangling types in Itanium ABI
rogfer01 added inline comments. Comment at: lib/AST/ItaniumMangle.cpp:2329-2333 + // __unaligned is not currently mangled in any way. This implies that it is + // not a relevant qualifier for substitutions (while CVR and maybe others + // are). This triggers an assertion when this is the only qualifier and the + // unqualified type is a class. So let's remove it preventively here. + quals.removeUnaligned(); rsmith wrote: > I don't think this is the right place/way to handle this: given > > ``` > void f(struct X __unaligned *p, struct X *q) {} > ``` > > it looks like we'll mangle as `_Z1fP1XP1X` with this patch, which seems > wrong: this should presumably instead be `_Z1fP1XS0_`. > > But regardless, I think the right thing to do is to invent a mangling for > `__unaligned`, since we support overloading on it; the most appropriate > mangling would be `U11__unaligned`, per > http://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangling-type. (You'll need > to extend `mangleQualifiers` to emit this and > `hasMangledSubstitutionQualifiers` to ignore it.) Oh. I see. Thanks for the review. I'll update this patch and then I'll post another one for the mangling of unaligned. https://reviews.llvm.org/D33398 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32332: Add support for transparent overloadable functions in clang
george.burgess.iv added a comment. Shortly after I pressed submit, I realized that this patch allows the following code if you tweak an assert to check for `overloadable` on the most recent redecl of a function: void foo(int); void foo(int) __attribute__((overloadable)); void foo(float); void foo(float) __attribute__((overloadable)); void foo(double); void foo(double) __attribute__((overloadable)); ...Which is bad. I'll fix that soon. :) https://reviews.llvm.org/D32332 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32479: clang-format: Introduce BreakConstructorInitializers option
Typz updated this revision to Diff 99854. Typz marked 5 inline comments as done. Typz added a comment. respond to review comments https://reviews.llvm.org/D32479 Files: include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/Format.cpp lib/Format/TokenAnnotator.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -56,16 +56,17 @@ return *Result; } - FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) { -FormatStyle Style = getLLVMStyle(); + FormatStyle getStyleWithColumns(FormatStyle Style, unsigned ColumnLimit) { Style.ColumnLimit = ColumnLimit; return Style; } + FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) { +return getStyleWithColumns(getLLVMStyle(), ColumnLimit); + } + FormatStyle getGoogleStyleWithColumns(unsigned ColumnLimit) { -FormatStyle Style = getGoogleStyle(); -Style.ColumnLimit = ColumnLimit; -return Style; +return getStyleWithColumns(getGoogleStyle(), ColumnLimit); } void verifyFormat(llvm::StringRef Code, @@ -2699,6 +2700,165 @@ "() {}")); } +TEST_F(FormatTest, BreakConstructorInitializersAfterColon) { + FormatStyle Style = getLLVMStyle(); + Style.BreakConstructorInitializers = FormatStyle::BCIS_AfterColon; + + verifyFormat("Constructor() : Initializer(FitsOnTheLine) {}"); + verifyFormat("Constructor() : Inttializer(FitsOnTheLine) {}", + getStyleWithColumns(Style, 45)); + verifyFormat("Constructor() :\n" + "Inttializer(FitsOnTheLine) {}", + getStyleWithColumns(Style, 44)); + verifyFormat("Constructor() :\n" + "Inttializer(FitsOnTheLine) {}", + getStyleWithColumns(Style, 43)); + + verifyFormat("template \n" + "Constructor() : Initializer(FitsOnTheLine) {}", + getStyleWithColumns(Style, 50)); + + verifyFormat( + "SomeClass::Constructor() :\n" + "a(aa), aaa() {}", + Style); + + verifyFormat( + "SomeClass::Constructor() :\n" + "a(aa), a(aa),\n" + "a(aa) {}", + Style); + verifyFormat( + "SomeClass::Constructor() :\n" + "aa(aa),\n" + "aaa() {}", + Style); + verifyFormat("Constructor(aa ,\n" + "aa ) :\n" + "aa(aa) {}", + Style); + + verifyFormat("Constructor() :\n" + "(aaa),\n" + "(aaa,\n" + " aaa),\n" + "aaa() {}", + Style); + + verifyFormat("Constructor() :\n" + "(\n" + "a) {}", + Style); + + verifyFormat("Constructor(int Parameter = 0) :\n" + "aa(a),\n" + "(a) {}", + Style); + verifyFormat("Constructor() :\n" + "aa(a), (b) {\n" + "}", + getStyleWithColumns(Style, 60)); + verifyFormat("Constructor() :\n" + "a(\n" + "a(, )) {}", + Style); + + // Here a line could be saved by splitting the second initializer onto two + // lines, but that is not desirable. + verifyFormat("Constructor() :\n" + "(),\n" + "aaa(aaa),\n" + "at() {}", + Style); + + FormatStyle OnePerLine = Style; + OnePerLine.ConstructorInitializerAllOnOneLineOrOnePerLine = true; + OnePerLine.AllowAllParametersOfDeclarationOnNextLine = false; + verifyFormat("SomeClass::Constructor() :\n" + "a(aa),\n" + "a(aa),\n" + "a(aa) {}", + OnePerLine); + verifyFormat("SomeClass::Constructor() :\n" + "a(aa), // Some comment\n" + "a(aa),\n" + "a(aa) {}", + OnePerLine); + verifyFormat("MyC
[PATCH] D32479: clang-format: Introduce BreakConstructorInitializers option
Typz added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:196 + FormatStyle::BCIS_AfterColonAndComma) && + (State.Column + State.Line->Last->TotalLength - Previous.TotalLength > + getColumnLimit(State) || djasper wrote: > Why can you drop the "+2" here? > > Also, I'd like to structure this so we have to duplicate less of the logic. > But I am not really sure it's possible. the +2 here was needed to keep identifiers aligned when breaking after colon but before the command (e.g. the 4th combination, not defined anymore): Foo() : field(1) , field(2) Comment at: lib/Format/ContinuationIndenter.cpp:196 + FormatStyle::BCIS_AfterColonAndComma) && + (State.Column + State.Line->Last->TotalLength - Previous.TotalLength > + getColumnLimit(State) || Typz wrote: > djasper wrote: > > Why can you drop the "+2" here? > > > > Also, I'd like to structure this so we have to duplicate less of the logic. > > But I am not really sure it's possible. > the +2 here was needed to keep identifiers aligned when breaking after colon > but before the command (e.g. the 4th combination, not defined anymore): > > Foo() : > field(1) > , field(2) I can avoid some duplication like this,m but i am not convinced it helps : const FormatToken &ColonToken = Style.BreakConstructorInitializers != FormatStyle::BCIS_AfterColon ? Current : Previous; if (ColonToken.is(TT_CtorInitializerColon) && (State.Column + State.Line->Last->TotalLength - ColonToken.TotalLength + (Style.BreakConstructorInitializers != FormatStyle::BCIS_AfterColon ? 2 : 0) > getColumnLimit(State) || State.Stack.back().BreakBeforeParameter) && (Style.AllowShortFunctionsOnASingleLine != FormatStyle::SFS_All || Style.BreakConstructorInitializers != FormatStyle::BCIS_BeforeColon || Style.ColumnLimit != 0)) return true; what do you think? https://reviews.llvm.org/D32479 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32479: clang-format: Introduce BreakConstructorInitializers option
Typz updated this revision to Diff 99856. Typz added a comment. Add missing doc for BreakConstructorInitializers https://reviews.llvm.org/D32479 Files: include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/Format.cpp lib/Format/TokenAnnotator.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -56,16 +56,17 @@ return *Result; } - FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) { -FormatStyle Style = getLLVMStyle(); + FormatStyle getStyleWithColumns(FormatStyle Style, unsigned ColumnLimit) { Style.ColumnLimit = ColumnLimit; return Style; } + FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) { +return getStyleWithColumns(getLLVMStyle(), ColumnLimit); + } + FormatStyle getGoogleStyleWithColumns(unsigned ColumnLimit) { -FormatStyle Style = getGoogleStyle(); -Style.ColumnLimit = ColumnLimit; -return Style; +return getStyleWithColumns(getGoogleStyle(), ColumnLimit); } void verifyFormat(llvm::StringRef Code, @@ -2699,6 +2700,165 @@ "() {}")); } +TEST_F(FormatTest, BreakConstructorInitializersAfterColon) { + FormatStyle Style = getLLVMStyle(); + Style.BreakConstructorInitializers = FormatStyle::BCIS_AfterColon; + + verifyFormat("Constructor() : Initializer(FitsOnTheLine) {}"); + verifyFormat("Constructor() : Inttializer(FitsOnTheLine) {}", + getStyleWithColumns(Style, 45)); + verifyFormat("Constructor() :\n" + "Inttializer(FitsOnTheLine) {}", + getStyleWithColumns(Style, 44)); + verifyFormat("Constructor() :\n" + "Inttializer(FitsOnTheLine) {}", + getStyleWithColumns(Style, 43)); + + verifyFormat("template \n" + "Constructor() : Initializer(FitsOnTheLine) {}", + getStyleWithColumns(Style, 50)); + + verifyFormat( + "SomeClass::Constructor() :\n" + "a(aa), aaa() {}", + Style); + + verifyFormat( + "SomeClass::Constructor() :\n" + "a(aa), a(aa),\n" + "a(aa) {}", + Style); + verifyFormat( + "SomeClass::Constructor() :\n" + "aa(aa),\n" + "aaa() {}", + Style); + verifyFormat("Constructor(aa ,\n" + "aa ) :\n" + "aa(aa) {}", + Style); + + verifyFormat("Constructor() :\n" + "(aaa),\n" + "(aaa,\n" + " aaa),\n" + "aaa() {}", + Style); + + verifyFormat("Constructor() :\n" + "(\n" + "a) {}", + Style); + + verifyFormat("Constructor(int Parameter = 0) :\n" + "aa(a),\n" + "(a) {}", + Style); + verifyFormat("Constructor() :\n" + "aa(a), (b) {\n" + "}", + getStyleWithColumns(Style, 60)); + verifyFormat("Constructor() :\n" + "a(\n" + "a(, )) {}", + Style); + + // Here a line could be saved by splitting the second initializer onto two + // lines, but that is not desirable. + verifyFormat("Constructor() :\n" + "(),\n" + "aaa(aaa),\n" + "at() {}", + Style); + + FormatStyle OnePerLine = Style; + OnePerLine.ConstructorInitializerAllOnOneLineOrOnePerLine = true; + OnePerLine.AllowAllParametersOfDeclarationOnNextLine = false; + verifyFormat("SomeClass::Constructor() :\n" + "a(aa),\n" + "a(aa),\n" + "a(aa) {}", + OnePerLine); + verifyFormat("SomeClass::Constructor() :\n" + "a(aa), // Some comment\n" + "a(aa),\n" + "a(aa) {}", + OnePerLine); + verifyFormat("MyClass::MyClass(int
[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier
jyu2 updated this revision to Diff 99857. jyu2 added a comment. I missed two place which Aaron point out. 1> using isNothrow function instead NR_Nothrow. 2> A format problem. Changed. Thanks. https://reviews.llvm.org/D3 Files: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaExprCXX.cpp lib/Sema/TreeTransform.h test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.spec.concept/p1.cpp test/CXX/except/except.spec/p11.cpp test/SemaCXX/warn-throw-in-nothrowing-function.cpp Index: lib/Sema/SemaExprCXX.cpp === --- lib/Sema/SemaExprCXX.cpp +++ lib/Sema/SemaExprCXX.cpp @@ -687,6 +687,44 @@ return BuildCXXThrow(OpLoc, Ex, IsThrownVarInScope); } +static bool isNoexcept(const FunctionDecl * FD) { + if (const auto *FPT = FD->getType()->castAs()) +if (FPT->getExceptionSpecType() != EST_None && + FPT->isNothrow(FD->getASTContext())) + return true; + return false; +} + +static bool isNoexceptTrue(const FunctionDecl * FD) { + // Avoid emitting error twice. + if (const auto * TempFD = FD->getTemplateInstantiationPattern()) +if (isNoexcept(TempFD)) + return false; + return isNoexcept(FD); +} + +void Sema::CheckCXXThrowInNonThrowingFunc(SourceLocation OpLoc) { + bool isInCXXTryBlock = false; + for (Scope *S = getCurScope(); S; S = S->getParent()) +if (S->isTryScope()) { + isInCXXTryBlock = true; + break; +} else if (S->isFunctionScope()) + break; + if (const FunctionDecl *FD = getCurFunctionDecl()) +if (!isInCXXTryBlock && !getSourceManager().isInSystemHeader(OpLoc)) + if (isNoexceptTrue(FD)) { +Diag(OpLoc, diag::warn_throw_in_noexcept_func) << FD; +if (getLangOpts().CPlusPlus11 && +(isa(FD) || + FD->getDeclName().getCXXOverloadedOperator() == OO_Delete || + FD->getDeclName().getCXXOverloadedOperator() == OO_Array_Delete)) + Diag(FD->getLocation(), diag::note_throw_in_dtor); +else + Diag(FD->getLocation(), diag::note_throw_in_function); + } +} + ExprResult Sema::BuildCXXThrow(SourceLocation OpLoc, Expr *Ex, bool IsThrownVarInScope) { // Don't report an error if 'throw' is used in system headers. @@ -702,6 +740,8 @@ if (getCurScope() && getCurScope()->isOpenMPSimdDirectiveScope()) Diag(OpLoc, diag::err_omp_simd_region_cannot_use_stmt) << "throw"; + CheckCXXThrowInNonThrowingFunc(OpLoc); + if (Ex && !Ex->isTypeDependent()) { QualType ExceptionObjectTy = Context.getExceptionObjectType(Ex->getType()); if (CheckCXXThrowOperand(OpLoc, ExceptionObjectTy, Ex)) Index: lib/Sema/TreeTransform.h === --- lib/Sema/TreeTransform.h +++ lib/Sema/TreeTransform.h @@ -9873,9 +9873,10 @@ if (SubExpr.isInvalid()) return ExprError(); - if (!getDerived().AlwaysRebuild() && - SubExpr.get() == E->getSubExpr()) + if (!getDerived().AlwaysRebuild() && SubExpr.get() == E->getSubExpr()) { +getSema().CheckCXXThrowInNonThrowingFunc(E->getThrowLoc()); return E; + } return getDerived().RebuildCXXThrowExpr(E->getThrowLoc(), SubExpr.get(), E->isThrownVariableInScope()); Index: include/clang/Sema/Sema.h === --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -4967,6 +4967,9 @@ ExprResult BuildCXXThrow(SourceLocation OpLoc, Expr *Ex, bool IsThrownVarInScope); bool CheckCXXThrowOperand(SourceLocation ThrowLoc, QualType ThrowTy, Expr *E); + /// Check if throw is used in function with a non-throwing noexcept + /// specifier. + void CheckCXXThrowInNonThrowingFunc(SourceLocation OpLoc); /// ActOnCXXTypeConstructExpr - Parse construction of a specified type. /// Can be interpreted either as function-style casting ("int(x)") Index: include/clang/Basic/DiagnosticSemaKinds.td === --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -6331,6 +6331,15 @@ "cannot use '%0' with exceptions disabled">; def err_objc_exceptions_disabled : Error< "cannot use '%0' with Objective-C exceptions disabled">; +def warn_throw_in_noexcept_func +: Warning<"%0 has a non-throwing exception specification but can still " + "throw, may result in unexpected program termination.">, + InGroup; +def note_throw_in_dtor +: Note<"destructor or deallocator has a (possible implicit) non-throwing " + "excepton specification">; +def note_throw_in_function +: Note<"nonthrowing function declare here">; def err_seh_try_outside_functions : Error< "cannot use SEH '__try' in blocks, captured regions, or Obj-C method decls">; def err_mixing_cxx_try_seh_try : Error< Index: test/SemaCXX/warn-
[clang-tools-extra] r303616 - [clangd] Split clangd into library+executable (mainly for unit tests).
Author: ibiryukov Date: Tue May 23 03:12:45 2017 New Revision: 303616 URL: http://llvm.org/viewvc/llvm-project?rev=303616&view=rev Log: [clangd] Split clangd into library+executable (mainly for unit tests). Summary: This commit itself doesn't add any unit tests, but one that does will follow shortly. Reviewers: krasimir, bkramer Reviewed By: bkramer Subscribers: mgorny, klimek, cfe-commits Tags: #clang-tools-extra Differential Revision: https://reviews.llvm.org/D33395 Added: clang-tools-extra/trunk/clangd/tool/ clang-tools-extra/trunk/clangd/tool/CMakeLists.txt - copied, changed from r303554, clang-tools-extra/trunk/clangd/CMakeLists.txt clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp - copied, changed from r303554, clang-tools-extra/trunk/clangd/ClangdMain.cpp Removed: clang-tools-extra/trunk/clangd/ClangdMain.cpp Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=303616&r1=303615&r2=303616&view=diff == --- clang-tools-extra/trunk/clangd/CMakeLists.txt (original) +++ clang-tools-extra/trunk/clangd/CMakeLists.txt Tue May 23 03:12:45 2017 @@ -1,6 +1,5 @@ -add_clang_executable(clangd +add_clang_library(clangDaemon ClangdLSPServer.cpp - ClangdMain.cpp ClangdServer.cpp ClangdUnit.cpp ClangdUnitStore.cpp @@ -11,9 +10,7 @@ add_clang_executable(clangd ProtocolHandlers.cpp ) -install(TARGETS clangd RUNTIME DESTINATION bin) - -target_link_libraries(clangd +target_link_libraries(clangDaemon clangBasic clangFormat clangFrontend @@ -22,3 +19,5 @@ target_link_libraries(clangd clangToolingCore LLVMSupport ) + +add_subdirectory(tool) Removed: clang-tools-extra/trunk/clangd/ClangdMain.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdMain.cpp?rev=303615&view=auto == --- clang-tools-extra/trunk/clangd/ClangdMain.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdMain.cpp (removed) @@ -1,40 +0,0 @@ -//===--- ClangdMain.cpp - clangd server loop --===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===--===// - -#include "ClangdLSPServer.h" -#include "JSONRPCDispatcher.h" -#include "llvm/Support/CommandLine.h" -#include "llvm/Support/FileSystem.h" -#include "llvm/Support/Program.h" - -#include -#include -#include - -using namespace clang; -using namespace clang::clangd; - -static llvm::cl::opt -RunSynchronously("run-synchronously", - llvm::cl::desc("parse on main thread"), - llvm::cl::init(false), llvm::cl::Hidden); - -int main(int argc, char *argv[]) { - llvm::cl::ParseCommandLineOptions(argc, argv, "clangd"); - - llvm::raw_ostream &Outs = llvm::outs(); - llvm::raw_ostream &Logs = llvm::errs(); - JSONOutput Out(Outs, Logs); - - // Change stdin to binary to not lose \r\n on windows. - llvm::sys::ChangeStdinToBinary(); - - ClangdLSPServer LSPServer(Out, RunSynchronously); - LSPServer.run(std::cin); -} Copied: clang-tools-extra/trunk/clangd/tool/CMakeLists.txt (from r303554, clang-tools-extra/trunk/clangd/CMakeLists.txt) URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/CMakeLists.txt?p2=clang-tools-extra/trunk/clangd/tool/CMakeLists.txt&p1=clang-tools-extra/trunk/clangd/CMakeLists.txt&r1=303554&r2=303616&rev=303616&view=diff == --- clang-tools-extra/trunk/clangd/CMakeLists.txt (original) +++ clang-tools-extra/trunk/clangd/tool/CMakeLists.txt Tue May 23 03:12:45 2017 @@ -1,20 +1,14 @@ +include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..) + add_clang_executable(clangd - ClangdLSPServer.cpp ClangdMain.cpp - ClangdServer.cpp - ClangdUnit.cpp - ClangdUnitStore.cpp - DraftStore.cpp - GlobalCompilationDatabase.cpp - JSONRPCDispatcher.cpp - Protocol.cpp - ProtocolHandlers.cpp ) install(TARGETS clangd RUNTIME DESTINATION bin) target_link_libraries(clangd clangBasic + clangDaemon clangFormat clangFrontend clangSema Copied: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (from r303554, clang-tools-extra/trunk/clangd/ClangdMain.cpp) URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?p2=clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp&p1=clang-tools-extra/trunk/clangd/ClangdMain.cpp&r1=303554&r2=303616&rev=303616&view=diff == (empty) ___
[PATCH] D33395: [clangd] Split clangd into library+executable (mainly for unit tests).
This revision was automatically updated to reflect the committed changes. Closed by commit rL303616: [clangd] Split clangd into library+executable (mainly for unit tests). (authored by ibiryukov). Changed prior to commit: https://reviews.llvm.org/D33395?vs=99774&id=99858#toc Repository: rL LLVM https://reviews.llvm.org/D33395 Files: clang-tools-extra/trunk/clangd/CMakeLists.txt clang-tools-extra/trunk/clangd/ClangdMain.cpp clang-tools-extra/trunk/clangd/tool/CMakeLists.txt clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp === --- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp +++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp @@ -0,0 +1,40 @@ +//===--- ClangdMain.cpp - clangd server loop --===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "ClangdLSPServer.h" +#include "JSONRPCDispatcher.h" +#include "llvm/Support/CommandLine.h" +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/Program.h" + +#include +#include +#include + +using namespace clang; +using namespace clang::clangd; + +static llvm::cl::opt +RunSynchronously("run-synchronously", + llvm::cl::desc("parse on main thread"), + llvm::cl::init(false), llvm::cl::Hidden); + +int main(int argc, char *argv[]) { + llvm::cl::ParseCommandLineOptions(argc, argv, "clangd"); + + llvm::raw_ostream &Outs = llvm::outs(); + llvm::raw_ostream &Logs = llvm::errs(); + JSONOutput Out(Outs, Logs); + + // Change stdin to binary to not lose \r\n on windows. + llvm::sys::ChangeStdinToBinary(); + + ClangdLSPServer LSPServer(Out, RunSynchronously); + LSPServer.run(std::cin); +} Index: clang-tools-extra/trunk/clangd/tool/CMakeLists.txt === --- clang-tools-extra/trunk/clangd/tool/CMakeLists.txt +++ clang-tools-extra/trunk/clangd/tool/CMakeLists.txt @@ -0,0 +1,18 @@ +include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..) + +add_clang_executable(clangd + ClangdMain.cpp + ) + +install(TARGETS clangd RUNTIME DESTINATION bin) + +target_link_libraries(clangd + clangBasic + clangDaemon + clangFormat + clangFrontend + clangSema + clangTooling + clangToolingCore + LLVMSupport + ) Index: clang-tools-extra/trunk/clangd/CMakeLists.txt === --- clang-tools-extra/trunk/clangd/CMakeLists.txt +++ clang-tools-extra/trunk/clangd/CMakeLists.txt @@ -1,6 +1,5 @@ -add_clang_executable(clangd +add_clang_library(clangDaemon ClangdLSPServer.cpp - ClangdMain.cpp ClangdServer.cpp ClangdUnit.cpp ClangdUnitStore.cpp @@ -11,14 +10,14 @@ ProtocolHandlers.cpp ) -install(TARGETS clangd RUNTIME DESTINATION bin) - -target_link_libraries(clangd +target_link_libraries(clangDaemon clangBasic clangFormat clangFrontend clangSema clangTooling clangToolingCore LLVMSupport ) + +add_subdirectory(tool) Index: clang-tools-extra/trunk/clangd/ClangdMain.cpp === --- clang-tools-extra/trunk/clangd/ClangdMain.cpp +++ clang-tools-extra/trunk/clangd/ClangdMain.cpp @@ -1,40 +0,0 @@ -//===--- ClangdMain.cpp - clangd server loop --===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===--===// - -#include "ClangdLSPServer.h" -#include "JSONRPCDispatcher.h" -#include "llvm/Support/CommandLine.h" -#include "llvm/Support/FileSystem.h" -#include "llvm/Support/Program.h" - -#include -#include -#include - -using namespace clang; -using namespace clang::clangd; - -static llvm::cl::opt -RunSynchronously("run-synchronously", - llvm::cl::desc("parse on main thread"), - llvm::cl::init(false), llvm::cl::Hidden); - -int main(int argc, char *argv[]) { - llvm::cl::ParseCommandLineOptions(argc, argv, "clangd"); - - llvm::raw_ostream &Outs = llvm::outs(); - llvm::raw_ostream &Logs = llvm::errs(); - JSONOutput Out(Outs, Logs); - - // Change stdin to binary to not lose \r\n on windows. - llvm::sys::ChangeStdinToBinary(); - - ClangdLSPServer LSPServer(Out, RunSynchronously); - LSPServer.run(std::cin); -} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32425: [mips] Make checks in CodeGen/mips-varargs.c less fragile
arichardson added a comment. I don't have commit access, could you commit for me please? https://reviews.llvm.org/D32425 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33397: Allow to use vfs::FileSystem for file accesses inside ASTUnit.
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. lg https://reviews.llvm.org/D33397 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32478: [clang-format] Fix AlignOperands when BreakBeforeBinaryOperators is set
Typz added inline comments. Comment at: unittests/Format/FormatTest.cpp:2476 "bool value = a\n" - " + a\n" - " + a\n" - " == a\n" - "* b\n" - "+ b\n" - " && a\n" - "* a\n" - "> c;", + " + a\n" + " + a\n" djasper wrote: > Typz wrote: > > Typz wrote: > > > djasper wrote: > > > > This looks very inconsistent to me. > > > not sure what you mean, I do not really understand how this expression > > > was aligned before the patch... > > > it is not so much better in this case with the patch, but the '&&' is > > > actually right-aligned with the '=' sign. > > Seeing the test just before, I see (when breaking after operators) that the > > operands are actually right-aligned, e.g. all operators are on the same > > column. > > > > So should it not be the same when breaking before the operator as well > > (independently from my patch, actually)? > > > > bool value = a\n" > > + a\n" > > + a\n" > > == a\n" > > * b\n" > > + b\n" > > && a\n" > > * a\n" > > > c; > > > > Not sure I like this right-alignment thing, but at least I start to > > understand how we get this output (and this may be another option to prefer > > left-alignment?) > The right alignment is not relevant. I think I just got "playful" when > writing this test. > > What's happening here is that we align the operators of subsequent lines to > the previous operand. You are right that this is not intuitive wrt. the > option's name, but it is the behavior that we intended and that we had seen > in styles that use this. > > Now, what we also do is add another ContinuationIndentWidth to highlight > operator precedence. Basically think of this as replacing parentheses that > you would put there instead: > > int x = (a >* b) // The "*" is intendet a space because of the paren. > + c; > > int x = a > * b // Instead we use ContinuationIndentWidth here. > + c; > > What I mean about this being inconsistent is that now you align the > highest-level operands because as you say that's what's expected from the > option, but you still don't align other precedences, which seems wrong. If > you really wanted to align all operands, that would look like: > > bool value = a > + a > + a > == a > * b > + b > && a > * a > > c; > > But then, that's really a tough expression to understand. I mean, this is a > fabricated example and possibly doesn't happen often in real life, but I am > slightly worried about it and the inconsistency here. So how should we proceed? As I understand it, there are 2 separate issues here: * The current behavior is actually the expected one, and should not be modified * The indentation due to 'fake parenthesis' is not very explicit (even with the current implementation) Changing AlignOperands into an OperandAlignment enum, with 3 options (`OA_NotAligned`, same as `AlignOperands=false`; `OA_AlignOperator` (or OA_Align), same the current `AlignOperands=true`; and `OA_AlignOperands` (or OA_StrictAlign), with my new behavior), would solve the first problem. Would it be acceptable? I don't think this is related to the extra indentation issue, the problem is already present, and the existing behavior is not that consistent or clear... But I think the 'fake parenthesis' indentation could easily be changed, independently from
r303619 - [mips] Make checks in CodeGen/mips-varargs.c less fragile
Author: sdardis Date: Tue May 23 04:42:50 2017 New Revision: 303619 URL: http://llvm.org/viewvc/llvm-project?rev=303619&view=rev Log: [mips] Make checks in CodeGen/mips-varargs.c less fragile This test was failing on our fork of clang because it was not capturing [[ARG]] in the N32 case. Therefore it used the value from the last function which does not always have to be the same. All other cases were already capturing ARG so this appears to be an oversight. The test now uses -enable-var-scope to prevent such errors in the future. Reviewers: sdardis, atanasyan Patch by: Alexander Richardson Differential Revision: https://reviews.llvm.org/D32425 Modified: cfe/trunk/test/CodeGen/mips-varargs.c Modified: cfe/trunk/test/CodeGen/mips-varargs.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/mips-varargs.c?rev=303619&r1=303618&r2=303619&view=diff == --- cfe/trunk/test/CodeGen/mips-varargs.c (original) +++ cfe/trunk/test/CodeGen/mips-varargs.c Tue May 23 04:42:50 2017 @@ -1,9 +1,9 @@ -// RUN: %clang_cc1 -triple mips-unknown-linux -o - -emit-llvm %s | FileCheck %s -check-prefix=ALL -check-prefix=O32 -// RUN: %clang_cc1 -triple mipsel-unknown-linux -o - -emit-llvm %s | FileCheck %s -check-prefix=ALL -check-prefix=O32 -// RUN: %clang_cc1 -triple mips64-unknown-linux -o - -emit-llvm -target-abi n32 %s | FileCheck %s -check-prefix=ALL -check-prefix=N32 -check-prefix=NEW -// RUN: %clang_cc1 -triple mips64-unknown-linux -o - -emit-llvm -target-abi n32 %s | FileCheck %s -check-prefix=ALL -check-prefix=N32 -check-prefix=NEW -// RUN: %clang_cc1 -triple mips64-unknown-linux -o - -emit-llvm %s | FileCheck %s -check-prefix=ALL -check-prefix=N64 -check-prefix=NEW -// RUN: %clang_cc1 -triple mips64el-unknown-linux -o - -emit-llvm %s | FileCheck %s -check-prefix=ALL -check-prefix=N64 -check-prefix=NEW +// RUN: %clang_cc1 -triple mips-unknown-linux -o - -emit-llvm %s | FileCheck %s -check-prefixes=ALL,O32 -enable-var-scope +// RUN: %clang_cc1 -triple mipsel-unknown-linux -o - -emit-llvm %s | FileCheck %s -check-prefixes=ALL,O32 -enable-var-scope +// RUN: %clang_cc1 -triple mips64-unknown-linux -o - -emit-llvm -target-abi n32 %s | FileCheck %s -check-prefixes=ALL,N32,NEW -enable-var-scope +// RUN: %clang_cc1 -triple mips64-unknown-linux -o - -emit-llvm -target-abi n32 %s | FileCheck %s -check-prefixes=ALL,N32,NEW -enable-var-scope +// RUN: %clang_cc1 -triple mips64-unknown-linux -o - -emit-llvm %s | FileCheck %s -check-prefixes=ALL,N64,NEW -enable-var-scope +// RUN: %clang_cc1 -triple mips64el-unknown-linux -o - -emit-llvm %s | FileCheck %s -check-prefixes=ALL,N64,NEW -enable-var-scope #include @@ -21,19 +21,19 @@ int test_i32(char *fmt, ...) { // ALL-LABEL: define i32 @test_i32(i8*{{.*}} %fmt, ...) // -// O32: %va = alloca i8*, align [[PTRALIGN:4]] -// N32: %va = alloca i8*, align [[PTRALIGN:4]] -// N64: %va = alloca i8*, align [[PTRALIGN:8]] +// O32: %va = alloca i8*, align [[$PTRALIGN:4]] +// N32: %va = alloca i8*, align [[$PTRALIGN:4]] +// N64: %va = alloca i8*, align [[$PTRALIGN:8]] // ALL: [[V:%.*]] = alloca i32, align 4 // NEW: [[PROMOTION_TEMP:%.*]] = alloca i32, align 4 // // ALL: [[VA:%.+]] = bitcast i8** %va to i8* // ALL: call void @llvm.va_start(i8* [[VA]]) -// ALL: [[AP_CUR:%.+]] = load i8*, i8** %va, align [[PTRALIGN]] -// O32: [[AP_NEXT:%.+]] = getelementptr inbounds i8, i8* [[AP_CUR]], [[INTPTR_T:i32]] [[CHUNKSIZE:4]] -// NEW: [[AP_NEXT:%.+]] = getelementptr inbounds i8, i8* [[AP_CUR]], [[INTPTR_T:i32|i64]] [[CHUNKSIZE:8]] +// ALL: [[AP_CUR:%.+]] = load i8*, i8** %va, align [[$PTRALIGN]] +// O32: [[AP_NEXT:%.+]] = getelementptr inbounds i8, i8* [[AP_CUR]], [[$INTPTR_T:i32]] [[$CHUNKSIZE:4]] +// NEW: [[AP_NEXT:%.+]] = getelementptr inbounds i8, i8* [[AP_CUR]], [[$INTPTR_T:i32|i64]] [[$CHUNKSIZE:8]] // -// ALL: store i8* [[AP_NEXT]], i8** %va, align [[PTRALIGN]] +// ALL: store i8* [[AP_NEXT]], i8** %va, align [[$PTRALIGN]] // // O32: [[AP_CAST:%.+]] = bitcast i8* [[AP_CUR]] to [[CHUNK_T:i32]]* // O32: [[ARG:%.+]] = load i32, i32* [[AP_CAST]], align [[CHUNKALIGN:4]] @@ -63,10 +63,10 @@ long long test_i64(char *fmt, ...) { // ALL-LABEL: define i64 @test_i64(i8*{{.*}} %fmt, ...) // -// ALL: %va = alloca i8*, align [[PTRALIGN]] +// ALL: %va = alloca i8*, align [[$PTRALIGN]] // ALL: [[VA:%.+]] = bitcast i8** %va to i8* // ALL: call void @llvm.va_start(i8* [[VA]]) -// ALL: [[AP_CUR:%.+]] = load i8*, i8** %va, align [[PTRALIGN]] +// ALL: [[AP_CUR:%.+]] = load i8*, i8** %va, align [[$PTRALIGN]] // // i64 is 8-byte aligned, while this is within O32's stack alignment there's no // guarantee that the offset is still 8-byte aligned after earlier reads. @@ -75,8 +75,8 @@ long long test_i64(char *fmt, ...) { // O32: [[TMP3:%.+]] = and i32 [[TMP2]], -8 // O32: [[AP_CUR:%.+]] = inttoptr i32 [[TMP3]] to i8* // -// ALL: [[AP
[PATCH] D32425: [mips] Make checks in CodeGen/mips-varargs.c less fragile
This revision was automatically updated to reflect the committed changes. Closed by commit rL303619: [mips] Make checks in CodeGen/mips-varargs.c less fragile (authored by sdardis). Changed prior to commit: https://reviews.llvm.org/D32425?vs=98059&id=99868#toc Repository: rL LLVM https://reviews.llvm.org/D32425 Files: cfe/trunk/test/CodeGen/mips-varargs.c Index: cfe/trunk/test/CodeGen/mips-varargs.c === --- cfe/trunk/test/CodeGen/mips-varargs.c +++ cfe/trunk/test/CodeGen/mips-varargs.c @@ -1,9 +1,9 @@ -// RUN: %clang_cc1 -triple mips-unknown-linux -o - -emit-llvm %s | FileCheck %s -check-prefix=ALL -check-prefix=O32 -// RUN: %clang_cc1 -triple mipsel-unknown-linux -o - -emit-llvm %s | FileCheck %s -check-prefix=ALL -check-prefix=O32 -// RUN: %clang_cc1 -triple mips64-unknown-linux -o - -emit-llvm -target-abi n32 %s | FileCheck %s -check-prefix=ALL -check-prefix=N32 -check-prefix=NEW -// RUN: %clang_cc1 -triple mips64-unknown-linux -o - -emit-llvm -target-abi n32 %s | FileCheck %s -check-prefix=ALL -check-prefix=N32 -check-prefix=NEW -// RUN: %clang_cc1 -triple mips64-unknown-linux -o - -emit-llvm %s | FileCheck %s -check-prefix=ALL -check-prefix=N64 -check-prefix=NEW -// RUN: %clang_cc1 -triple mips64el-unknown-linux -o - -emit-llvm %s | FileCheck %s -check-prefix=ALL -check-prefix=N64 -check-prefix=NEW +// RUN: %clang_cc1 -triple mips-unknown-linux -o - -emit-llvm %s | FileCheck %s -check-prefixes=ALL,O32 -enable-var-scope +// RUN: %clang_cc1 -triple mipsel-unknown-linux -o - -emit-llvm %s | FileCheck %s -check-prefixes=ALL,O32 -enable-var-scope +// RUN: %clang_cc1 -triple mips64-unknown-linux -o - -emit-llvm -target-abi n32 %s | FileCheck %s -check-prefixes=ALL,N32,NEW -enable-var-scope +// RUN: %clang_cc1 -triple mips64-unknown-linux -o - -emit-llvm -target-abi n32 %s | FileCheck %s -check-prefixes=ALL,N32,NEW -enable-var-scope +// RUN: %clang_cc1 -triple mips64-unknown-linux -o - -emit-llvm %s | FileCheck %s -check-prefixes=ALL,N64,NEW -enable-var-scope +// RUN: %clang_cc1 -triple mips64el-unknown-linux -o - -emit-llvm %s | FileCheck %s -check-prefixes=ALL,N64,NEW -enable-var-scope #include @@ -21,19 +21,19 @@ // ALL-LABEL: define i32 @test_i32(i8*{{.*}} %fmt, ...) // -// O32: %va = alloca i8*, align [[PTRALIGN:4]] -// N32: %va = alloca i8*, align [[PTRALIGN:4]] -// N64: %va = alloca i8*, align [[PTRALIGN:8]] +// O32: %va = alloca i8*, align [[$PTRALIGN:4]] +// N32: %va = alloca i8*, align [[$PTRALIGN:4]] +// N64: %va = alloca i8*, align [[$PTRALIGN:8]] // ALL: [[V:%.*]] = alloca i32, align 4 // NEW: [[PROMOTION_TEMP:%.*]] = alloca i32, align 4 // // ALL: [[VA:%.+]] = bitcast i8** %va to i8* // ALL: call void @llvm.va_start(i8* [[VA]]) -// ALL: [[AP_CUR:%.+]] = load i8*, i8** %va, align [[PTRALIGN]] -// O32: [[AP_NEXT:%.+]] = getelementptr inbounds i8, i8* [[AP_CUR]], [[INTPTR_T:i32]] [[CHUNKSIZE:4]] -// NEW: [[AP_NEXT:%.+]] = getelementptr inbounds i8, i8* [[AP_CUR]], [[INTPTR_T:i32|i64]] [[CHUNKSIZE:8]] +// ALL: [[AP_CUR:%.+]] = load i8*, i8** %va, align [[$PTRALIGN]] +// O32: [[AP_NEXT:%.+]] = getelementptr inbounds i8, i8* [[AP_CUR]], [[$INTPTR_T:i32]] [[$CHUNKSIZE:4]] +// NEW: [[AP_NEXT:%.+]] = getelementptr inbounds i8, i8* [[AP_CUR]], [[$INTPTR_T:i32|i64]] [[$CHUNKSIZE:8]] // -// ALL: store i8* [[AP_NEXT]], i8** %va, align [[PTRALIGN]] +// ALL: store i8* [[AP_NEXT]], i8** %va, align [[$PTRALIGN]] // // O32: [[AP_CAST:%.+]] = bitcast i8* [[AP_CUR]] to [[CHUNK_T:i32]]* // O32: [[ARG:%.+]] = load i32, i32* [[AP_CAST]], align [[CHUNKALIGN:4]] @@ -63,20 +63,20 @@ // ALL-LABEL: define i64 @test_i64(i8*{{.*}} %fmt, ...) // -// ALL: %va = alloca i8*, align [[PTRALIGN]] +// ALL: %va = alloca i8*, align [[$PTRALIGN]] // ALL: [[VA:%.+]] = bitcast i8** %va to i8* // ALL: call void @llvm.va_start(i8* [[VA]]) -// ALL: [[AP_CUR:%.+]] = load i8*, i8** %va, align [[PTRALIGN]] +// ALL: [[AP_CUR:%.+]] = load i8*, i8** %va, align [[$PTRALIGN]] // // i64 is 8-byte aligned, while this is within O32's stack alignment there's no // guarantee that the offset is still 8-byte aligned after earlier reads. // O32: [[TMP1:%.+]] = ptrtoint i8* [[AP_CUR]] to i32 // O32: [[TMP2:%.+]] = add i32 [[TMP1]], 7 // O32: [[TMP3:%.+]] = and i32 [[TMP2]], -8 // O32: [[AP_CUR:%.+]] = inttoptr i32 [[TMP3]] to i8* // -// ALL: [[AP_NEXT:%.+]] = getelementptr inbounds i8, i8* [[AP_CUR]], [[INTPTR_T]] 8 -// ALL: store i8* [[AP_NEXT]], i8** %va, align [[PTRALIGN]] +// ALL: [[AP_NEXT:%.+]] = getelementptr inbounds i8, i8* [[AP_CUR]], [[$INTPTR_T]] 8 +// ALL: store i8* [[AP_NEXT]], i8** %va, align [[$PTRALIGN]] // // ALL: [[AP_CAST:%.*]] = bitcast i8* [[AP_CUR]] to i64* // ALL: [[ARG:%.+]] = load i64, i64* [[AP_CAST]], align 8 @@ -97,14 +97,14 @@ // ALL-LABEL: define i8* @test_ptr(i8*{{.*}} %fmt, ...) // -// ALL: %va = alloca i8*, align [[PTRALIGN]] -// AL
[PATCH] D33416: [clangd] Allow to use vfs::FileSystem for file accesses.
bkramer added inline comments. Comment at: unittests/clangd/ClangdTests.cpp:118 + llvm::SmallString<128> TmpDir2; + llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true, TmpDir2); + We should delete those when we're done, otherwise the unit test will litter /tmp on every run. https://reviews.llvm.org/D33416 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33415: [clangd] Replaced WorkerRequest with std::function...
krasimir added inline comments. Comment at: clangd/ClangdServer.cpp:85 +Request = std::move(RequestQueue.front()); +RequestQueue.pop_front(); } // unlock Mutex Why are we taking it from the front and not from the back? Maybe at least add a comment about this behavior. Comment at: clangd/ClangdServer.cpp:102 } // unlock Mutex + RequestCV.notify_one(); Worker.join(); Why did this get out? Comment at: clangd/ClangdServer.cpp:230 +Unit.dumpAST(ResultOS); + } + DumpPromise.set_value(std::move(Result)); Maybe an explicit flush and no block? Comment at: clangd/ClangdUnitStore.h:56 + template + void runOnExistingUnit(PathRef File, Func Action) { +std::lock_guard Lock(Mutex); Maybe make it less generic and put the implementation in the source file? It uses std::function anyways. https://reviews.llvm.org/D33415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33415: [clangd] Replaced WorkerRequest with std::function...
krasimir added inline comments. Comment at: clangd/ClangdUnitStore.h:56 + template + void runOnExistingUnit(PathRef File, Func Action) { +std::lock_guard Lock(Mutex); krasimir wrote: > Maybe make it less generic and put the implementation in the source file? It > uses std::function anyways. Sorry, I didn't see the surroundings. Disregard this comment. Comment at: clangd/ClangdUnitStore.h:62 + +Action(It->second); + } Why can't we use `runOnUnitImpl` here? https://reviews.llvm.org/D33415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r303623 - [clangd] Pick up deps via LLVM components, which will hopefully include pthread.
Author: d0k Date: Tue May 23 05:17:48 2017 New Revision: 303623 URL: http://llvm.org/viewvc/llvm-project?rev=303623&view=rev Log: [clangd] Pick up deps via LLVM components, which will hopefully include pthread. Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=303623&r1=303622&r2=303623&view=diff == --- clang-tools-extra/trunk/clangd/CMakeLists.txt (original) +++ clang-tools-extra/trunk/clangd/CMakeLists.txt Tue May 23 05:17:48 2017 @@ -1,3 +1,7 @@ +set(LLVM_LINK_COMPONENTS + Support + ) + add_clang_library(clangDaemon ClangdLSPServer.cpp ClangdServer.cpp @@ -8,16 +12,14 @@ add_clang_library(clangDaemon JSONRPCDispatcher.cpp Protocol.cpp ProtocolHandlers.cpp - ) -target_link_libraries(clangDaemon + LINK_LIBS clangBasic clangFormat clangFrontend clangSema clangTooling clangToolingCore - LLVMSupport ) add_subdirectory(tool) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32351: [Tooling][libclang] Remove unused CompilationDatabase::MappedSources
krasimir added a comment. @klimek: ping https://reviews.llvm.org/D32351 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32351: [Tooling][libclang] Remove unused CompilationDatabase::MappedSources
klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D32351 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33272: Method loadFromCommandLine should be able to report errors
alexfh added inline comments. Comment at: lib/Tooling/CommonOptionsParser.cpp:122 + FixedCompilationDatabase::loadFromCommandLine(argc, argv, ErrorMessage); + if (!Compilations && StringRef(ErrorMessage).startswith("error")) +llvm::errs() << ErrorMessage; Getting into the business of encoding the type of error into the string is not what we want in this particular case ;] Let's just check whether ErrorMessage is empty (we could go further and make it an Optional, but an empty string seems to be equally reasonable here). Comment at: lib/Tooling/CompilationDatabase.cpp:292 + if (Argc == 0) { +ErrorMsg = "error: no arguments specified\n"; +return nullptr; sepavloff wrote: > alexfh wrote: > > The lack of the arguments (or the `--` below) shouldn't be treated as an > > error (at least an error that is worth showing to the user). The caller > > will just move on to trying the next kind of a compilation database. The > > only actual errors we can get here may be coming from `stripPositionalArgs`. > > > > The caller should be modified accordingly (i.e. not output anything when > > both the return value is `nullptr` and `ErrorMsg` is empty). > What if error message contains a prefix that indicates if this is error or > warning? Errors could be printed and warnings ignored in > `CommonOptionsParser.cpp:122`. Such solution would allow clients to > investigate failures in more details. > I don't think it's valuable in this specific code. I'd just return nullptr without touching ErrorMsg and change the caller to only print something when the ErrorMsg is not empty. The lack of the `--` in the command line is not an error, it's just a way to automatically find a compilation database from the file system (which is probably even more frequently used feature than the fixed compilation database, so we definitely should not treat this case as an error). https://reviews.llvm.org/D33272 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r303625 - [clangd] Explicitly link against pthread.
Author: d0k Date: Tue May 23 05:29:54 2017 New Revision: 303625 URL: http://llvm.org/viewvc/llvm-project?rev=303625&view=rev Log: [clangd] Explicitly link against pthread. Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=303625&r1=303624&r2=303625&view=diff == --- clang-tools-extra/trunk/clangd/CMakeLists.txt (original) +++ clang-tools-extra/trunk/clangd/CMakeLists.txt Tue May 23 05:29:54 2017 @@ -20,6 +20,7 @@ add_clang_library(clangDaemon clangSema clangTooling clangToolingCore + ${LLVM_PTHREAD_LIB} ) add_subdirectory(tool) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303626 - [libclang] [OpenCL] Expose more OpenCL CIndex types
Author: svenvh Date: Tue May 23 05:36:43 2017 New Revision: 303626 URL: http://llvm.org/viewvc/llvm-project?rev=303626&view=rev Log: [libclang] [OpenCL] Expose more OpenCL CIndex types Expose pipe, sampler_t, clk_event_t, queue_t, reserve_id_t, and all image types. Update the opencl-types.cl test RUN line such that we can test the OpenCL 2.0 types. Patch by Simon Perretta. Differential Revision: https://reviews.llvm.org/D33197 Modified: cfe/trunk/bindings/python/clang/cindex.py cfe/trunk/include/clang-c/Index.h cfe/trunk/test/Index/opencl-types.cl cfe/trunk/tools/libclang/CXType.cpp Modified: cfe/trunk/bindings/python/clang/cindex.py URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/clang/cindex.py?rev=303626&r1=303625&r2=303626&view=diff == --- cfe/trunk/bindings/python/clang/cindex.py (original) +++ cfe/trunk/bindings/python/clang/cindex.py Tue May 23 05:36:43 2017 @@ -1963,6 +1963,47 @@ TypeKind.DEPENDENTSIZEDARRAY = TypeKind( TypeKind.MEMBERPOINTER = TypeKind(117) TypeKind.AUTO = TypeKind(118) TypeKind.ELABORATED = TypeKind(119) +TypeKind.PIPE = TypeKind(120) +TypeKind.OCLIMAGE1DRO = TypeKind(121) +TypeKind.OCLIMAGE1DARRAYRO = TypeKind(122) +TypeKind.OCLIMAGE1DBUFFERRO = TypeKind(123) +TypeKind.OCLIMAGE2DRO = TypeKind(124) +TypeKind.OCLIMAGE2DARRAYRO = TypeKind(125) +TypeKind.OCLIMAGE2DDEPTHRO = TypeKind(126) +TypeKind.OCLIMAGE2DARRAYDEPTHRO = TypeKind(127) +TypeKind.OCLIMAGE2DMSAARO = TypeKind(128) +TypeKind.OCLIMAGE2DARRAYMSAARO = TypeKind(129) +TypeKind.OCLIMAGE2DMSAADEPTHRO = TypeKind(130) +TypeKind.OCLIMAGE2DARRAYMSAADEPTHRO = TypeKind(131) +TypeKind.OCLIMAGE3DRO = TypeKind(132) +TypeKind.OCLIMAGE1DWO = TypeKind(133) +TypeKind.OCLIMAGE1DARRAYWO = TypeKind(134) +TypeKind.OCLIMAGE1DBUFFERWO = TypeKind(135) +TypeKind.OCLIMAGE2DWO = TypeKind(136) +TypeKind.OCLIMAGE2DARRAYWO = TypeKind(137) +TypeKind.OCLIMAGE2DDEPTHWO = TypeKind(138) +TypeKind.OCLIMAGE2DARRAYDEPTHWO = TypeKind(139) +TypeKind.OCLIMAGE2DMSAAWO = TypeKind(140) +TypeKind.OCLIMAGE2DARRAYMSAAWO = TypeKind(141) +TypeKind.OCLIMAGE2DMSAADEPTHWO = TypeKind(142) +TypeKind.OCLIMAGE2DARRAYMSAADEPTHWO = TypeKind(143) +TypeKind.OCLIMAGE3DWO = TypeKind(144) +TypeKind.OCLIMAGE1DRW = TypeKind(145) +TypeKind.OCLIMAGE1DARRAYRW = TypeKind(146) +TypeKind.OCLIMAGE1DBUFFERRW = TypeKind(147) +TypeKind.OCLIMAGE2DRW = TypeKind(148) +TypeKind.OCLIMAGE2DARRAYRW = TypeKind(149) +TypeKind.OCLIMAGE2DDEPTHRW = TypeKind(150) +TypeKind.OCLIMAGE2DARRAYDEPTHRW = TypeKind(151) +TypeKind.OCLIMAGE2DMSAARW = TypeKind(152) +TypeKind.OCLIMAGE2DARRAYMSAARW = TypeKind(153) +TypeKind.OCLIMAGE2DMSAADEPTHRW = TypeKind(154) +TypeKind.OCLIMAGE2DARRAYMSAADEPTHRW = TypeKind(155) +TypeKind.OCLIMAGE3DRW = TypeKind(156) +TypeKind.OCLSAMPLER = TypeKind(157) +TypeKind.OCLEVENT = TypeKind(158) +TypeKind.OCLQUEUE = TypeKind(159) +TypeKind.OCLRESERVEID = TypeKind(160) class RefQualifierKind(BaseEnumeration): """Describes a specific ref-qualifier of a type.""" Modified: cfe/trunk/include/clang-c/Index.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=303626&r1=303625&r2=303626&view=diff == --- cfe/trunk/include/clang-c/Index.h (original) +++ cfe/trunk/include/clang-c/Index.h Tue May 23 05:36:43 2017 @@ -32,7 +32,7 @@ * compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable. */ #define CINDEX_VERSION_MAJOR 0 -#define CINDEX_VERSION_MINOR 39 +#define CINDEX_VERSION_MINOR 40 #define CINDEX_VERSION_ENCODE(major, minor) ( \ ((major) * 1) \ @@ -3076,7 +3076,52 @@ enum CXTypeKind { * * E.g., struct S, or via a qualified name, e.g., N::M::type, or both. */ - CXType_Elaborated = 119 + CXType_Elaborated = 119, + + /* OpenCL PipeType. */ + CXType_Pipe = 120, + + /* OpenCL builtin types. */ + CXType_OCLImage1dRO = 121, + CXType_OCLImage1dArrayRO = 122, + CXType_OCLImage1dBufferRO = 123, + CXType_OCLImage2dRO = 124, + CXType_OCLImage2dArrayRO = 125, + CXType_OCLImage2dDepthRO = 126, + CXType_OCLImage2dArrayDepthRO = 127, + CXType_OCLImage2dMSAARO = 128, + CXType_OCLImage2dArrayMSAARO = 129, + CXType_OCLImage2dMSAADepthRO = 130, + CXType_OCLImage2dArrayMSAADepthRO = 131, + CXType_OCLImage3dRO = 132, + CXType_OCLImage1dWO = 133, + CXType_OCLImage1dArrayWO = 134, + CXType_OCLImage1dBufferWO = 135, + CXType_OCLImage2dWO = 136, + CXType_OCLImage2dArrayWO = 137, + CXType_OCLImage2dDepthWO = 138, + CXType_OCLImage2dArrayDepthWO = 139, + CXType_OCLImage2dMSAAWO = 140, + CXType_OCLImage2dArrayMSAAWO = 141, + CXType_OCLImage2dMSAADepthWO = 142, + CXType_OCLImage2dArrayMSAADepthWO = 143, + CXType_OCLImage3dWO = 144, + CXType_OCLImage1dRW = 145, + CXType_OCLImage1dArrayRW = 146, + CXType_OCLImage1dBufferRW = 147, + CXType_OCLImage2dRW = 148, + CXType_OCLImage2dArrayRW = 149, + CXType_OCLIm
[PATCH] D33197: [libclang] [OpenCL] Expose more OpenCL CIndex types
This revision was automatically updated to reflect the committed changes. Closed by commit rL303626: [libclang] [OpenCL] Expose more OpenCL CIndex types (authored by svenvh). Changed prior to commit: https://reviews.llvm.org/D33197?vs=99574&id=99877#toc Repository: rL LLVM https://reviews.llvm.org/D33197 Files: cfe/trunk/bindings/python/clang/cindex.py cfe/trunk/include/clang-c/Index.h cfe/trunk/test/Index/opencl-types.cl cfe/trunk/tools/libclang/CXType.cpp Index: cfe/trunk/bindings/python/clang/cindex.py === --- cfe/trunk/bindings/python/clang/cindex.py +++ cfe/trunk/bindings/python/clang/cindex.py @@ -1963,6 +1963,47 @@ TypeKind.MEMBERPOINTER = TypeKind(117) TypeKind.AUTO = TypeKind(118) TypeKind.ELABORATED = TypeKind(119) +TypeKind.PIPE = TypeKind(120) +TypeKind.OCLIMAGE1DRO = TypeKind(121) +TypeKind.OCLIMAGE1DARRAYRO = TypeKind(122) +TypeKind.OCLIMAGE1DBUFFERRO = TypeKind(123) +TypeKind.OCLIMAGE2DRO = TypeKind(124) +TypeKind.OCLIMAGE2DARRAYRO = TypeKind(125) +TypeKind.OCLIMAGE2DDEPTHRO = TypeKind(126) +TypeKind.OCLIMAGE2DARRAYDEPTHRO = TypeKind(127) +TypeKind.OCLIMAGE2DMSAARO = TypeKind(128) +TypeKind.OCLIMAGE2DARRAYMSAARO = TypeKind(129) +TypeKind.OCLIMAGE2DMSAADEPTHRO = TypeKind(130) +TypeKind.OCLIMAGE2DARRAYMSAADEPTHRO = TypeKind(131) +TypeKind.OCLIMAGE3DRO = TypeKind(132) +TypeKind.OCLIMAGE1DWO = TypeKind(133) +TypeKind.OCLIMAGE1DARRAYWO = TypeKind(134) +TypeKind.OCLIMAGE1DBUFFERWO = TypeKind(135) +TypeKind.OCLIMAGE2DWO = TypeKind(136) +TypeKind.OCLIMAGE2DARRAYWO = TypeKind(137) +TypeKind.OCLIMAGE2DDEPTHWO = TypeKind(138) +TypeKind.OCLIMAGE2DARRAYDEPTHWO = TypeKind(139) +TypeKind.OCLIMAGE2DMSAAWO = TypeKind(140) +TypeKind.OCLIMAGE2DARRAYMSAAWO = TypeKind(141) +TypeKind.OCLIMAGE2DMSAADEPTHWO = TypeKind(142) +TypeKind.OCLIMAGE2DARRAYMSAADEPTHWO = TypeKind(143) +TypeKind.OCLIMAGE3DWO = TypeKind(144) +TypeKind.OCLIMAGE1DRW = TypeKind(145) +TypeKind.OCLIMAGE1DARRAYRW = TypeKind(146) +TypeKind.OCLIMAGE1DBUFFERRW = TypeKind(147) +TypeKind.OCLIMAGE2DRW = TypeKind(148) +TypeKind.OCLIMAGE2DARRAYRW = TypeKind(149) +TypeKind.OCLIMAGE2DDEPTHRW = TypeKind(150) +TypeKind.OCLIMAGE2DARRAYDEPTHRW = TypeKind(151) +TypeKind.OCLIMAGE2DMSAARW = TypeKind(152) +TypeKind.OCLIMAGE2DARRAYMSAARW = TypeKind(153) +TypeKind.OCLIMAGE2DMSAADEPTHRW = TypeKind(154) +TypeKind.OCLIMAGE2DARRAYMSAADEPTHRW = TypeKind(155) +TypeKind.OCLIMAGE3DRW = TypeKind(156) +TypeKind.OCLSAMPLER = TypeKind(157) +TypeKind.OCLEVENT = TypeKind(158) +TypeKind.OCLQUEUE = TypeKind(159) +TypeKind.OCLRESERVEID = TypeKind(160) class RefQualifierKind(BaseEnumeration): """Describes a specific ref-qualifier of a type.""" Index: cfe/trunk/include/clang-c/Index.h === --- cfe/trunk/include/clang-c/Index.h +++ cfe/trunk/include/clang-c/Index.h @@ -32,7 +32,7 @@ * compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable. */ #define CINDEX_VERSION_MAJOR 0 -#define CINDEX_VERSION_MINOR 39 +#define CINDEX_VERSION_MINOR 40 #define CINDEX_VERSION_ENCODE(major, minor) ( \ ((major) * 1) \ @@ -3076,7 +3076,52 @@ * * E.g., struct S, or via a qualified name, e.g., N::M::type, or both. */ - CXType_Elaborated = 119 + CXType_Elaborated = 119, + + /* OpenCL PipeType. */ + CXType_Pipe = 120, + + /* OpenCL builtin types. */ + CXType_OCLImage1dRO = 121, + CXType_OCLImage1dArrayRO = 122, + CXType_OCLImage1dBufferRO = 123, + CXType_OCLImage2dRO = 124, + CXType_OCLImage2dArrayRO = 125, + CXType_OCLImage2dDepthRO = 126, + CXType_OCLImage2dArrayDepthRO = 127, + CXType_OCLImage2dMSAARO = 128, + CXType_OCLImage2dArrayMSAARO = 129, + CXType_OCLImage2dMSAADepthRO = 130, + CXType_OCLImage2dArrayMSAADepthRO = 131, + CXType_OCLImage3dRO = 132, + CXType_OCLImage1dWO = 133, + CXType_OCLImage1dArrayWO = 134, + CXType_OCLImage1dBufferWO = 135, + CXType_OCLImage2dWO = 136, + CXType_OCLImage2dArrayWO = 137, + CXType_OCLImage2dDepthWO = 138, + CXType_OCLImage2dArrayDepthWO = 139, + CXType_OCLImage2dMSAAWO = 140, + CXType_OCLImage2dArrayMSAAWO = 141, + CXType_OCLImage2dMSAADepthWO = 142, + CXType_OCLImage2dArrayMSAADepthWO = 143, + CXType_OCLImage3dWO = 144, + CXType_OCLImage1dRW = 145, + CXType_OCLImage1dArrayRW = 146, + CXType_OCLImage1dBufferRW = 147, + CXType_OCLImage2dRW = 148, + CXType_OCLImage2dArrayRW = 149, + CXType_OCLImage2dDepthRW = 150, + CXType_OCLImage2dArrayDepthRW = 151, + CXType_OCLImage2dMSAARW = 152, + CXType_OCLImage2dArrayMSAARW = 153, + CXType_OCLImage2dMSAADepthRW = 154, + CXType_OCLImage2dArrayMSAADepthRW = 155, + CXType_OCLImage3dRW = 156, + CXType_OCLSampler = 157, + CXType_OCLEvent = 158, + CXType_OCLQueue = 159, + CXType_OCLReserveID = 160 }; /** Index: cfe/trunk/test/Index/opencl-types.cl === --- cfe/trunk/test/Index/opencl-types.cl +++ cfe/trunk/te
[PATCH] D33398: Mangle __unaligned in Itanium ABI
rogfer01 updated this revision to Diff 99876. rogfer01 retitled this revision from "Remove __unaligned preventively when mangling types in Itanium ABI" to "Mangle __unaligned in Itanium ABI". rogfer01 edited the summary of this revision. rogfer01 added a reviewer: majnemer. rogfer01 added a comment. Abandon the old approach which cannot possibly work and just opt to mangle `__unaligned`. https://reviews.llvm.org/D33398 Files: lib/AST/ItaniumMangle.cpp test/CodeGenCXX/pr33080.cpp test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp test/CodeGenCXX/unaligned-member-qualifier.cpp Index: test/CodeGenCXX/unaligned-member-qualifier.cpp === --- /dev/null +++ test/CodeGenCXX/unaligned-member-qualifier.cpp @@ -0,0 +1,21 @@ +// RUN: %clang_cc1 -triple %itanium_abi_triple -fms-extensions -emit-llvm %s -o- | FileCheck %s + +struct A +{ +void foo() __unaligned; +void foo() const __unaligned; +void foo() volatile __unaligned; +void foo() const volatile __unaligned; +}; + +void A::foo() __unaligned { } +// CHECK: define void @_ZNU11__unaligned1A3fooEv( + +void A::foo() const __unaligned { } +// CHECK: define void @_ZNU11__unalignedK1A3fooEv( + +void A::foo() volatile __unaligned { } +// CHECK: define void @_ZNU11__unalignedV1A3fooEv( + +void A::foo() const volatile __unaligned { } +// CHECK: define void @_ZNU11__unalignedVK1A3fooEv( Index: test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp === --- test/CodeGenCXX/unaligned-duplicated-mangle-name.cpp +++ /dev/null @@ -1,20 +0,0 @@ -// RUN: %clang_cc1 -triple %itanium_abi_triple -fms-extensions -emit-llvm-only %s -verify - -struct A -{ -int x; -void foo() __unaligned; -void foo(); -}; - -void A::foo() __unaligned -{ -this->x++; -} - -void A::foo() // expected-error {{definition with same mangled name as another definition}} - // expected-note@-6 {{previous definition is here}} -{ -this->x++; -} - Index: test/CodeGenCXX/pr33080.cpp === --- /dev/null +++ test/CodeGenCXX/pr33080.cpp @@ -0,0 +1,19 @@ +// RUN: %clang_cc1 -triple %itanium_abi_triple -fms-extensions -emit-llvm -o- %s | FileCheck %s + +void fa(__unaligned struct A*) {} +// CHECK: define void @_Z2faPU11__unaligned1A( + +void ga(struct A*, struct A*) {} +// CHECK: define void @_Z2gaP1AS0_( + +void gb(__unaligned struct A*, struct A*) {} +// CHECK: define void @_Z2gbPU11__unaligned1APS_( + +void gc(struct A*, __unaligned struct A*) {} +// CHECK: define void @_Z2gcP1APU11__unalignedS_( + +void gd(__unaligned struct A*, __unaligned struct A*) {} +// CHECK: define void @_Z2gdPU11__unaligned1AS1_( + +void hb(__unaligned struct A*, __unaligned const struct A*) {} +// CHECK: define void @_Z2hbPU11__unaligned1APU11__unalignedKS_( Index: lib/AST/ItaniumMangle.cpp === --- lib/AST/ItaniumMangle.cpp +++ lib/AST/ItaniumMangle.cpp @@ -1459,8 +1459,6 @@ // We do not consider restrict a distinguishing attribute for overloading // purposes so we must not mangle it. MethodQuals.removeRestrict(); -// __unaligned is not currently mangled in any way, so remove it. -MethodQuals.removeUnaligned(); mangleQualifiers(MethodQuals); mangleRefQualifier(Method->getRefQualifier()); } @@ -2208,6 +2206,9 @@ break; } + if (Quals.hasUnaligned()) + mangleVendorQualifier("__unaligned"); + // ::= [r] [V] [K]# restrict (C99), volatile, const if (Quals.hasRestrict()) Out << 'r'; @@ -4327,7 +4328,7 @@ /// substitutions. static bool hasMangledSubstitutionQualifiers(QualType T) { Qualifiers Qs = T.getQualifiers(); - return Qs.getCVRQualifiers() || Qs.hasAddressSpace(); + return Qs.getCVRQualifiers() || Qs.hasAddressSpace() || Qs.hasUnaligned(); } bool CXXNameMangler::mangleSubstitution(QualType T) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32896: [OpenCL] Make CLK_NULL_RESERVE_ID invalid reserve id.
bader added inline comments. Comment at: lib/Headers/opencl-c.h:16020 +// The macro CLK_NULL_RESERVE_ID refers to an invalid reservation ID. +#define CLK_NULL_RESERVE_ID (__builtin_astype((void *)0, reserve_id_t)) bool __ovld is_valid_reserve_id(reserve_id_t reserve_id); yaxunl wrote: > bader wrote: > > yaxunl wrote: > > > bader wrote: > > > > yaxunl wrote: > > > > > yaxunl wrote: > > > > > > bader wrote: > > > > > > > yaxunl wrote: > > > > > > > > Anastasia wrote: > > > > > > > > > echuraev wrote: > > > > > > > > > > yaxunl wrote: > > > > > > > > > > > Anastasia wrote: > > > > > > > > > > > > yaxunl wrote: > > > > > > > > > > > > > Anastasia wrote: > > > > > > > > > > > > > > Looks good from my side. > > > > > > > > > > > > > > > > > > > > > > > > > > > > @yaxunl , since you originally committed this. > > > > > > > > > > > > > > Could you please verify that changing from > > > > > > > > > > > > > > `SIZE_MAX` to `0` would be fine. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Btw, we have a similar definition for > > > > > > > > > > > > > > `CLK_NULL_EVENT`. > > > > > > > > > > > > > `__PIPE_RESERVE_ID_VALID_BIT` is implementation > > > > > > > > > > > > > detail and not part of the spec. I would suggest to > > > > > > > > > > > > > remove it from this header file. > > > > > > > > > > > > > > > > > > > > > > > > > > The spec only requires CLK_NULL_RESERVE_ID to be > > > > > > > > > > > > > defined but does not define its value. Naturally a > > > > > > > > > > > > > valid id starts from 0 and increases. I don't see > > > > > > > > > > > > > significant advantage to change CLK_NULL_RESERVE_ID > > > > > > > > > > > > > from __SIZE_MAX to 0. > > > > > > > > > > > > > > > > > > > > > > > > > > Is there any reason that this change is needed? > > > > > > > > > > > > I don't see issues to commit things outside of spec as > > > > > > > > > > > > soon as they prefixed properly with "__". But I agree > > > > > > > > > > > > it would be nice to see if it's any useful and what the > > > > > > > > > > > > motivation is for having different implementation. > > > > > > > > > > > For `__PIPE_RESERVE_ID_VALID_BIT`, it assumes that the > > > > > > > > > > > implementation uses one specific bit of a reserve id to > > > > > > > > > > > indicate that the reserve id is valid. Not all > > > > > > > > > > > implementations assume that. Actually I am curious why > > > > > > > > > > > that is needed too. > > > > > > > > > > About `CLK_NULL_RESERVE_ID`: we check that reserve id is > > > > > > > > > > valid if significant bit equal to one. `CLK_NULL_RESERVE_ID > > > > > > > > > > refers to an invalid reservation, so if > > > > > > > > > > `CLK_NULL_RESERVE_ID equal to 0, we can be sure that > > > > > > > > > > significant bit doesn't equal to 1 and it is invalid > > > > > > > > > > reserve id. Also it is more obviously if > > > > > > > > > > CLK_**NULL**_RESERVE_ID is equal to 0. > > > > > > > > > > > > > > > > > > > > What about `__PIPE_RESERVE_ID_VALID_BIT`: As I understand > > > > > > > > > > previous implementation also assumes that one specific bit > > > > > > > > > > was of a reverse id was used to indicate that the reserve > > > > > > > > > > id is valid. So, we just increased reserve id size by one > > > > > > > > > > bit on 32-bit platforms and by 33 bits on 64-bit platforms. > > > > > > > > > It is more logical to me that `CLK_NULL_RESERVE_ID` is 0, but > > > > > > > > > spec doesn't define it of course. > > > > > > > > In our implementation, valid reserve id starts at 0 and > > > > > > > > increasing linearly until `__SIZE_MAX-1`. This change will > > > > > > > > break our implementation. > > > > > > > > > > > > > > > > However, we can modify our implementation to adopt this change > > > > > > > > since it brings about benefits overall. > > > > > > > Ideally it would be great to have unified implementation, but we > > > > > > > can define device specific value for CLK_NULL_RESERVE_ID by using > > > > > > > ifdef directive. > > > > > > How about > > > > > > > > > > > > ``` > > > > > > __attribute__((const)) size_t __clk_null_reserve_id(); > > > > > > #define CLK_NULL_RESERVE_ID __clk_null_reserve_id() > > > > > > > > > > > > ``` > > > > > > I think the spec does not require it to be compile time constant. > > > > > > Then each library can implement its own __clk_null_reserve_id() > > > > > > whereas the IR is target independent. > > > > > Or we only do this for SPIR and define it as target specific value > > > > > for other targets. > > > > Defining CLK_NULL_RESERVE_ID as a function call should also work, but > > > > IMHO compile time constant is preferable option. > > > > I don't think making it compile time constant for SPIR only makes sense > > > > to me - in this case we can use constant for all targets. > > > > > > > > How about following approach: use 0 by default and allow other targets > > > > re-define CLK_NULL_RESERVE_I
[PATCH] D32480: clang-format: Add CompactNamespaces option
djasper added a comment. That's what I meant by "The name NamespaceIndentation might then be a bit confusing, but not sure whether it's worth changing it.". I am honestly not sure. Let's get a third opinion. If we add this additional option, I think we need to fix the behavior and make what people expect: With NI_None: namespace A { namespace B { int i; namespace C { int j; } // namespace C }} // namespace A::B With NI_Inner: namespace A { namespace B { int i; namespace C { int j; } // namespace C }} // namespace A::B With NI_All: namespace A { namespace B { int i; namespace C { int j; } // namespace C }} // namespace A::B https://reviews.llvm.org/D32480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32479: clang-format: Introduce BreakConstructorInitializers option
djasper added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:196 + FormatStyle::BCIS_AfterColonAndComma) && + (State.Column + State.Line->Last->TotalLength - Previous.TotalLength > + getColumnLimit(State) || Typz wrote: > Typz wrote: > > djasper wrote: > > > Why can you drop the "+2" here? > > > > > > Also, I'd like to structure this so we have to duplicate less of the > > > logic. But I am not really sure it's possible. > > the +2 here was needed to keep identifiers aligned when breaking after > > colon but before the command (e.g. the 4th combination, not defined > > anymore): > > > > Foo() : > > field(1) > > , field(2) > I can avoid some duplication like this,m but i am not convinced it helps : > > const FormatToken &ColonToken = > Style.BreakConstructorInitializers != FormatStyle::BCIS_AfterColon > ? Current : Previous; > if (ColonToken.is(TT_CtorInitializerColon) && > (State.Column + State.Line->Last->TotalLength - ColonToken.TotalLength + >(Style.BreakConstructorInitializers != > FormatStyle::BCIS_AfterColon ? 2 : 0) > >getColumnLimit(State) || >State.Stack.back().BreakBeforeParameter) && > (Style.AllowShortFunctionsOnASingleLine != FormatStyle::SFS_All || >Style.BreakConstructorInitializers != FormatStyle::BCIS_BeforeColon || >Style.ColumnLimit != 0)) > return true; > > what do you think? The +2 here has nothing todo with how the things are aligned. This is about whether the entire constructor with initializer fits on one line. Can you try out (or even add tests) for cases where the entire constructor is 80 and 81 columns long? I think I like the condensed version a bit better, but yeah, it's not beautiful either ;). Comment at: lib/Format/ContinuationIndenter.cpp:468 + } else if (Previous.is(TT_CtorInitializerColon) && + Style.BreakConstructorInitializers == + FormatStyle::BCIS_AfterColon) { Does this fit on one line now? Comment at: lib/Format/Format.cpp:309 +// If BreakConstructorInitializersBeforeComma was specified but +// BreakConstructorInitializers was not, initialize the latter from the +// former for backwards compatibility. How do you know that BreakConstructorInitializers was not specified? https://reviews.llvm.org/D32479 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303630 - Allow to use vfs::FileSystem for file accesses inside ASTUnit.
Author: ibiryukov Date: Tue May 23 06:37:52 2017 New Revision: 303630 URL: http://llvm.org/viewvc/llvm-project?rev=303630&view=rev Log: Allow to use vfs::FileSystem for file accesses inside ASTUnit. Reviewers: bkramer, krasimir, arphaman, akyrtzi Reviewed By: bkramer Subscribers: klimek, cfe-commits Differential Revision: https://reviews.llvm.org/D33397 Modified: cfe/trunk/include/clang/Frontend/ASTUnit.h cfe/trunk/include/clang/Frontend/CompilerInvocation.h cfe/trunk/lib/Frontend/ASTUnit.cpp cfe/trunk/lib/Frontend/CompilerInvocation.cpp Modified: cfe/trunk/include/clang/Frontend/ASTUnit.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/ASTUnit.h?rev=303630&r1=303629&r2=303630&view=diff == --- cfe/trunk/include/clang/Frontend/ASTUnit.h (original) +++ cfe/trunk/include/clang/Frontend/ASTUnit.h Tue May 23 06:37:52 2017 @@ -59,6 +59,10 @@ class TargetInfo; class FrontendAction; class ASTDeserializationListener; +namespace vfs { +class FileSystem; +} + /// \brief Utility class for loading a ASTContext from an AST file. /// class ASTUnit : public ModuleLoader { @@ -420,7 +424,8 @@ private: explicit ASTUnit(bool MainFileIsAST); bool Parse(std::shared_ptr PCHContainerOps, - std::unique_ptr OverrideMainBuffer); + std::unique_ptr OverrideMainBuffer, + IntrusiveRefCntPtr VFS); struct ComputedPreamble { llvm::MemoryBuffer *Buffer; @@ -434,11 +439,13 @@ private: PreambleEndsAtStartOfLine(PreambleEndsAtStartOfLine) {} }; ComputedPreamble ComputePreamble(CompilerInvocation &Invocation, - unsigned MaxLines); + unsigned MaxLines, + IntrusiveRefCntPtr VFS); std::unique_ptr getMainBufferWithPrecompiledPreamble( std::shared_ptr PCHContainerOps, - const CompilerInvocation &PreambleInvocationIn, bool AllowRebuild = true, + const CompilerInvocation &PreambleInvocationIn, + IntrusiveRefCntPtr VFS, bool AllowRebuild = true, unsigned MaxLines = 0); void RealizeTopLevelDeclsFromPreamble(); @@ -731,11 +738,17 @@ private: /// of this translation unit should be precompiled, to improve the performance /// of reparsing. Set to zero to disable preambles. /// + /// \param VFS - A vfs::FileSystem to be used for all file accesses. Note that + /// preamble is saved to a temporary directory on a RealFileSystem, so in order + /// for it to be loaded correctly, VFS should have access to it(i.e., be an + /// overlay over RealFileSystem). + /// /// \returns \c true if a catastrophic failure occurred (which means that the /// \c ASTUnit itself is invalid), or \c false otherwise. bool LoadFromCompilerInvocation( std::shared_ptr PCHContainerOps, - unsigned PrecompilePreambleAfterNParses); + unsigned PrecompilePreambleAfterNParses, + IntrusiveRefCntPtr VFS); public: @@ -826,6 +839,11 @@ public: /// (e.g. because the PCH could not be loaded), this accepts the ASTUnit /// mainly to allow the caller to see the diagnostics. /// + /// \param VFS - A vfs::FileSystem to be used for all file accesses. Note that + /// preamble is saved to a temporary directory on a RealFileSystem, so in order + /// for it to be loaded correctly, VFS should have access to it(i.e., be an + /// overlay over RealFileSystem). RealFileSystem will be used if \p VFS is nullptr. + /// // FIXME: Move OnlyLocalDecls, UseBumpAllocator to setters on the ASTUnit, we // shouldn't need to specify them at construction time. static ASTUnit *LoadFromCommandLine( @@ -842,15 +860,23 @@ public: bool AllowPCHWithCompilerErrors = false, bool SkipFunctionBodies = false, bool UserFilesAreVolatile = false, bool ForSerialization = false, llvm::Optional ModuleFormat = llvm::None, - std::unique_ptr *ErrAST = nullptr); + std::unique_ptr *ErrAST = nullptr, + IntrusiveRefCntPtr VFS = nullptr); /// \brief Reparse the source files using the same command-line options that /// were originally used to produce this translation unit. /// + /// \param VFS - A vfs::FileSystem to be used for all file accesses. Note that + /// preamble is saved to a temporary directory on a RealFileSystem, so in order + /// for it to be loaded correctly, VFS should give an access to this(i.e. be an + /// overlay over RealFileSystem). FileMgr->getVirtualFileSystem() will be used if + /// \p VFS is nullptr. + /// /// \returns True if a failure occurred that causes the ASTUnit not to /// contain any translation-unit information, false otherwise. bool Reparse(std::shared_ptr PCHContainerOps, - ArrayRef RemappedFiles = None); + ArrayRef RemappedFiles = None, + IntrusiveRefCntPtr VFS = nullptr); /// \brief Perfor
[PATCH] D33397: Allow to use vfs::FileSystem for file accesses inside ASTUnit.
This revision was automatically updated to reflect the committed changes. Closed by commit rL303630: Allow to use vfs::FileSystem for file accesses inside ASTUnit. (authored by ibiryukov). Changed prior to commit: https://reviews.llvm.org/D33397?vs=99725&id=99880#toc Repository: rL LLVM https://reviews.llvm.org/D33397 Files: cfe/trunk/include/clang/Frontend/ASTUnit.h cfe/trunk/include/clang/Frontend/CompilerInvocation.h cfe/trunk/lib/Frontend/ASTUnit.cpp cfe/trunk/lib/Frontend/CompilerInvocation.cpp Index: cfe/trunk/lib/Frontend/ASTUnit.cpp === --- cfe/trunk/lib/Frontend/ASTUnit.cpp +++ cfe/trunk/lib/Frontend/ASTUnit.cpp @@ -90,6 +90,21 @@ /// \brief Erase temporary files and the preamble file. void Cleanup(); }; + + template + std::unique_ptr valueOrNull(llvm::ErrorOr> Val) { +if (!Val) + return nullptr; +return std::move(*Val); + } + + template + bool moveOnNoError(llvm::ErrorOr Val, T &Output) { +if (!Val) + return false; +Output = std::move(*Val); +return true; + } } static llvm::sys::SmartMutex &getOnDiskMutex() { @@ -1019,15 +1034,22 @@ /// \returns True if a failure occurred that causes the ASTUnit not to /// contain any translation-unit information, false otherwise. bool ASTUnit::Parse(std::shared_ptr PCHContainerOps, -std::unique_ptr OverrideMainBuffer) { +std::unique_ptr OverrideMainBuffer, +IntrusiveRefCntPtr VFS) { SavedMainFileBuffer.reset(); if (!Invocation) return true; // Create the compiler instance to use for building the AST. std::unique_ptr Clang( new CompilerInstance(std::move(PCHContainerOps))); + if (FileMgr && VFS) { +assert(VFS == FileMgr->getVirtualFileSystem() && + "VFS passed to Parse and VFS in FileMgr are different"); + } else if (VFS) { +Clang->setVirtualFileSystem(VFS); + } // Recover resources if we crash before exiting this method. llvm::CrashRecoveryContextCleanupRegistrar @@ -1170,7 +1192,8 @@ /// that corresponds to the main file along with a pair (bytes, start-of-line) /// that describes the preamble. ASTUnit::ComputedPreamble -ASTUnit::ComputePreamble(CompilerInvocation &Invocation, unsigned MaxLines) { +ASTUnit::ComputePreamble(CompilerInvocation &Invocation, unsigned MaxLines, + IntrusiveRefCntPtr VFS) { FrontendOptions &FrontendOpts = Invocation.getFrontendOpts(); PreprocessorOptions &PreprocessorOpts = Invocation.getPreprocessorOpts(); @@ -1180,28 +1203,32 @@ llvm::MemoryBuffer *Buffer = nullptr; std::unique_ptr BufferOwner; std::string MainFilePath(FrontendOpts.Inputs[0].getFile()); - llvm::sys::fs::UniqueID MainFileID; - if (!llvm::sys::fs::getUniqueID(MainFilePath, MainFileID)) { + auto MainFileStatus = VFS->status(MainFilePath); + if (MainFileStatus) { +llvm::sys::fs::UniqueID MainFileID = MainFileStatus->getUniqueID(); + // Check whether there is a file-file remapping of the main file for (const auto &RF : PreprocessorOpts.RemappedFiles) { std::string MPath(RF.first); - llvm::sys::fs::UniqueID MID; - if (!llvm::sys::fs::getUniqueID(MPath, MID)) { + auto MPathStatus = VFS->status(MPath); + if (MPathStatus) { +llvm::sys::fs::UniqueID MID = MPathStatus->getUniqueID(); if (MainFileID == MID) { // We found a remapping. Try to load the resulting, remapped source. - BufferOwner = getBufferForFile(RF.second); + BufferOwner = valueOrNull(VFS->getBufferForFile(RF.second)); if (!BufferOwner) return ComputedPreamble(nullptr, nullptr, 0, true); } } } - + // Check whether there is a file-buffer remapping. It supercedes the // file-file remapping. for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) { std::string MPath(RB.first); - llvm::sys::fs::UniqueID MID; - if (!llvm::sys::fs::getUniqueID(MPath, MID)) { + auto MPathStatus = VFS->status(MPath); + if (MPathStatus) { +llvm::sys::fs::UniqueID MID = MPathStatus->getUniqueID(); if (MainFileID == MID) { // We found a remapping. BufferOwner.reset(); @@ -1213,7 +1240,7 @@ // If the main source file was not remapped, load it now. if (!Buffer && !BufferOwner) { -BufferOwner = getBufferForFile(FrontendOpts.Inputs[0].getFile()); +BufferOwner = valueOrNull(VFS->getBufferForFile(FrontendOpts.Inputs[0].getFile())); if (!BufferOwner) return ComputedPreamble(nullptr, nullptr, 0, true); } @@ -1324,16 +1351,19 @@ std::unique_ptr ASTUnit::getMainBufferWithPrecompiledPreamble( std::shared_ptr PCHContainerOps, -const CompilerInvocation &PreambleInvocationIn, bool AllowRebuild, +const CompilerInvocation &PreambleInvocationIn, +IntrusiveRefCntPtr VFS, bool AllowRebuild,
[PATCH] D32480: clang-format: Add CompactNamespaces option
krasimir added a comment. This change should also adapt NamespaceEndCommentFixer to respect the new option and not introduce/remove/change the comments unexpectedly. https://reviews.llvm.org/D32480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32480: clang-format: Add CompactNamespaces option
Typz added a comment. Just to be clear: as it is, the patch does not implement what you describe, both on the 'indent' side (as expected, we are discussing it), but also on the "merging" of braces side. Consecutive namespace opening are merged on one side; and consecutive namespace closing are merged on the other side; but there is no relation between the two. What this means is that at the moment your exemple would be formatted (without indent) like this: namespace A { namespace B { int i; namespace C { int j; }}} // namespace A::B::C https://reviews.llvm.org/D32480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32480: clang-format: Add CompactNamespaces option
krasimir added a comment. In any case, adding a namespace end comment to a line closing multiple namespaces is super confusing for me: what does the comment refer to: the inner one, the outer one, or both? `}} // namespace A::B` https://reviews.llvm.org/D32480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32480: clang-format: Add CompactNamespaces option
Typz added a comment. In https://reviews.llvm.org/D32480#761859, @krasimir wrote: > This change should also adapt NamespaceEndCommentFixer to respect the new > option and not introduce/remove/change the comments unexpectedly. This should be handled already (pending further changes to the patch, obviously); did you see (or think of) any specific broken or dangerous case? https://reviews.llvm.org/D32480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33415: [clangd] Replaced WorkerRequest with std::function...
ilya-biryukov updated this revision to Diff 99883. ilya-biryukov marked 2 inline comments as done. ilya-biryukov added a comment. Added more comments to addToEnd/addToFront, added explicit flush instead of relying on raw_ostream's destructor to do it implicitly. https://reviews.llvm.org/D33415 Files: clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/ClangdUnitStore.h Index: clangd/ClangdUnitStore.h === --- clangd/ClangdUnitStore.h +++ clangd/ClangdUnitStore.h @@ -50,6 +50,18 @@ std::forward(Action)); } + /// Run the specified \p Action on the ClangdUnit for \p File. + /// Unit for \p File should exist in the store. + template + void runOnExistingUnit(PathRef File, Func Action) { +std::lock_guard Lock(Mutex); + +auto It = OpenedFiles.find(File); +assert(It != OpenedFiles.end() && "File is not in OpenedFiles"); + +Action(It->second); + } + /// Remove ClangdUnit for \p File, if any void removeUnitIfPresent(PathRef File); Index: clangd/ClangdUnit.h === --- clangd/ClangdUnit.h +++ clangd/ClangdUnit.h @@ -16,6 +16,10 @@ #include "clang/Tooling/Core/Replacement.h" #include +namespace llvm { +class raw_ostream; +} + namespace clang { class ASTUnit; class PCHContainerOperations; @@ -52,6 +56,10 @@ /// located in the current file. std::vector getLocalDiagnostics() const; + /// For testing/debugging purposes. Note that this method deserializes all + /// unserialized Decls, so use with care. + void dumpAST(llvm::raw_ostream &OS) const; + private: Path FileName; std::unique_ptr Unit; Index: clangd/ClangdUnit.cpp === --- clangd/ClangdUnit.cpp +++ clangd/ClangdUnit.cpp @@ -222,3 +222,7 @@ } return Result; } + +void ClangdUnit::dumpAST(llvm::raw_ostream &OS) const { + Unit->getASTContext().getTranslationUnitDecl()->dump(OS, true); +} Index: clangd/ClangdServer.h === --- clangd/ClangdServer.h +++ clangd/ClangdServer.h @@ -24,6 +24,7 @@ #include "Protocol.h" #include +#include #include #include #include @@ -49,30 +50,25 @@ std::vector Diagnostics) = 0; }; -enum class WorkerRequestKind { ParseAndPublishDiagnostics, RemoveDocData }; - -/// A request to the worker thread -class WorkerRequest { -public: - WorkerRequest() = default; - WorkerRequest(WorkerRequestKind Kind, Path File, DocVersion Version); - - WorkerRequestKind Kind; - Path File; - DocVersion Version; -}; - class ClangdServer; /// Handles running WorkerRequests of ClangdServer on a separate threads. /// Currently runs only one worker thread. class ClangdScheduler { public: - ClangdScheduler(ClangdServer &Server, bool RunSynchronously); + ClangdScheduler(bool RunSynchronously); ~ClangdScheduler(); - /// Enqueue WorkerRequest to be run on a worker thread - void enqueue(ClangdServer &Server, WorkerRequest Request); + /// Add \p Request to the start of the queue. \p Request will be run on a + /// separate worker thread. + /// \p Request is scheduled to be executed before all currently added + /// requests. + void addToFront(std::function Request); + /// Add \p Request to the end of the queue. \p Request will be run on a + /// separate worker thread. + /// \p Request is scheduled to be executed after all currently added + /// requests. + void addToEnd(std::function Request); private: bool RunSynchronously; @@ -83,11 +79,10 @@ std::thread Worker; /// Setting Done to true will make the worker thread terminate. bool Done = false; - /// A LIFO queue of requests. Note that requests are discarded if the - /// `version` field is not equal to the one stored inside DraftStore. + /// A queue of requests. /// FIXME(krasimir): code completion should always have priority over parsing /// for diagnostics. - std::deque RequestQueue; + std::deque> RequestQueue; /// Condition variable to wake up the worker thread. std::condition_variable RequestCV; }; @@ -126,12 +121,12 @@ /// conversions in outside code, maybe there's a way to get rid of it. std::string getDocument(PathRef File); -private: - friend class ClangdScheduler; - - /// This function is called on a worker thread. - void handleRequest(WorkerRequest Request); + /// Only for testing purposes. + /// Waits until all requests to worker thread are finished and dumps AST for + /// \p File. \p File must be in the list of added documents. + std::string dumpAST(PathRef File); +private: std::unique_ptr CDB; std::unique_ptr DiagConsumer; DraftStore DraftMgr; Index: clangd/ClangdServer.cpp === --- clangd/ClangdServer.cpp +++ clangd/ClangdServe
[PATCH] D33415: [clangd] Replaced WorkerRequest with std::function...
ilya-biryukov marked 2 inline comments as done. ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.cpp:85 +Request = std::move(RequestQueue.front()); +RequestQueue.pop_front(); } // unlock Mutex krasimir wrote: > Why are we taking it from the front and not from the back? Maybe at least add > a comment about this behavior. Added comments here and at the addToEnd/addToFront methods. Comment at: clangd/ClangdServer.cpp:230 +Unit.dumpAST(ResultOS); + } + DumpPromise.set_value(std::move(Result)); krasimir wrote: > Maybe an explicit flush and no block? Ok. Comment at: clangd/ClangdUnitStore.h:62 + +Action(It->second); + } krasimir wrote: > Why can't we use `runOnUnitImpl` here? runOnUnitImpl does parsing/reparsing if the ASTFile is not there, which this function doesn't do specifically. https://reviews.llvm.org/D33415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33415: [clangd] Replaced WorkerRequest with std::function...
ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.cpp:102 } // unlock Mutex + RequestCV.notify_one(); Worker.join(); krasimir wrote: > Why did this get out? "The notifying thread does not need to hold the lock on the same mutex as the one held by the waiting thread(s); in fact doing so is a pessimization, since the notified thread would immediately block again, waiting for the notifying thread to release the lock" (from: http://en.cppreference.com/w/cpp/thread/condition_variable/notify_one) https://reviews.llvm.org/D33415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33415: [clangd] Replaced WorkerRequest with std::function...
krasimir accepted this revision. krasimir added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D33415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33437: Emit available_externally vtables opportunistically
Prazek created this revision. We can emit vtable definition having inline function if they are all emitted. https://reviews.llvm.org/D33437 Files: include/clang/AST/VTableBuilder.h lib/CodeGen/CGVTables.cpp lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.h lib/CodeGen/ItaniumCXXABI.cpp test/CodeGenCXX/vtable-available-externally.cpp test/CodeGenCXX/vtable-linkage.cpp Index: test/CodeGenCXX/vtable-linkage.cpp === --- test/CodeGenCXX/vtable-linkage.cpp +++ test/CodeGenCXX/vtable-linkage.cpp @@ -145,12 +145,14 @@ // F is an explicit template instantiation declaration without a // key function, so its vtable should have external linkage. // CHECK-DAG: @_ZTV1FIiE = external unnamed_addr constant -// CHECK-OPT-DAG: @_ZTV1FIiE = external unnamed_addr constant +// CHECK-OPT-DAG: @_ZTV1FIiE = available_externally unnamed_addr constant // E is an explicit template instantiation declaration. It has a // key function is not instantiated, so we know that vtable definition // will be generated in TU where key function will be defined -// so we can mark it as available_externally (only with optimizations) +// so we can mark it as external (without optimizations) and +// available_externally (with optimizations) because all of the inline +// virtual functions have been emitted. // CHECK-DAG: @_ZTV1EIiE = external unnamed_addr constant // CHECK-OPT-DAG: @_ZTV1EIiE = available_externally unnamed_addr constant Index: test/CodeGenCXX/vtable-available-externally.cpp === --- test/CodeGenCXX/vtable-available-externally.cpp +++ test/CodeGenCXX/vtable-available-externally.cpp @@ -12,6 +12,7 @@ // RUN: FileCheck --check-prefix=CHECK-TEST14 %s < %t.opt // RUN: FileCheck --check-prefix=CHECK-TEST15 %s < %t.opt // RUN: FileCheck --check-prefix=CHECK-TEST16 %s < %t.opt +// RUN: FileCheck --check-prefix=CHECK-TEST17 %s < %t.opt #include @@ -274,8 +275,8 @@ virtual D& operator=(const D&); }; -// Cannot emit B's vtable available_externally, because we cannot create -// a reference to the inline virtual B::operator= function. +// Cannot emit D's vtable available_externally, because we cannot create +// a reference to the inline virtual D::operator= function. // CHECK-TEST11: @_ZTVN6Test111DE = external unnamed_addr constant struct D : C { virtual void key(); @@ -391,3 +392,30 @@ } } +namespace Test17 { +// This test checks if we emit vtables opportunistically. +// CHECK-TEST17-DAG: @_ZTVN6Test171AE = available_externally +// CHECK-TEST17-DAG: @_ZTVN6Test171BE = external + +struct A { + virtual void key(); + virtual void bar() {} +}; + +// We won't gonna use deleting destructor for this type, which will disallow +// emitting vtable as available_externally +struct B { + virtual void key(); + virtual ~B() {} +}; + +void testcaseA() { + A a; + a.bar(); // this forces to emit definition of bar +} + +void testcaseB() { + B b; // This only forces emitting of complete object destructor +} + +} // namespace Test17 Index: lib/CodeGen/ItaniumCXXABI.cpp === --- lib/CodeGen/ItaniumCXXABI.cpp +++ lib/CodeGen/ItaniumCXXABI.cpp @@ -366,20 +366,30 @@ void emitCXXStructor(const CXXMethodDecl *MD, StructorType Type) override; private: - bool hasAnyVirtualInlineFunction(const CXXRecordDecl *RD) const { -const auto &VtableLayout = -CGM.getItaniumVTableContext().getVTableLayout(RD); - -for (const auto &VtableComponent : VtableLayout.vtable_components()) { - // Skip empty slot. - if (!VtableComponent.isUsedFunctionPointerKind()) -continue; - - const CXXMethodDecl *Method = VtableComponent.getFunctionDecl(); - if (Method->getCanonicalDecl()->isInlined()) -return true; -} -return false; + bool hasAnyUnusedVirtualInlineFunction(const CXXRecordDecl *RD) const { + const auto &VtableLayout = + CGM.getItaniumVTableContext().getVTableLayout(RD); + + for (const auto &VtableComponent : VtableLayout.vtable_components()) { + // Skip empty slot. + if (!VtableComponent.isUsedFunctionPointerKind()) + continue; + + const CXXMethodDecl *Method = VtableComponent.getFunctionDecl(); + if (!Method->getCanonicalDecl()->isInlined()) + continue; + + StringRef Name = CGM.getMangledName(VtableComponent.getGlobalDecl()); + auto *Entry = CGM.GetGlobalValue(Name); + // This checks if virtual inline function has already been emitted. + // Note that it is possible that this inline function would be emitted + // after trying to emit vtable speculatively. Because of this we do + // an extra pass after emitting all deferred vtables to find and emit + // these vtables opportunistically. + if (!Entry || Entry->isDeclaration()) + return true; + } + retur
[PATCH] D32479: clang-format: Introduce BreakConstructorInitializers option
Typz marked 6 inline comments as done. Typz added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:196 + FormatStyle::BCIS_AfterColonAndComma) && + (State.Column + State.Line->Last->TotalLength - Previous.TotalLength > + getColumnLimit(State) || djasper wrote: > Typz wrote: > > Typz wrote: > > > djasper wrote: > > > > Why can you drop the "+2" here? > > > > > > > > Also, I'd like to structure this so we have to duplicate less of the > > > > logic. But I am not really sure it's possible. > > > the +2 here was needed to keep identifiers aligned when breaking after > > > colon but before the command (e.g. the 4th combination, not defined > > > anymore): > > > > > > Foo() : > > > field(1) > > > , field(2) > > I can avoid some duplication like this,m but i am not convinced it helps : > > > > const FormatToken &ColonToken = > > Style.BreakConstructorInitializers != FormatStyle::BCIS_AfterColon > > ? Current : Previous; > > if (ColonToken.is(TT_CtorInitializerColon) && > > (State.Column + State.Line->Last->TotalLength - > > ColonToken.TotalLength + > >(Style.BreakConstructorInitializers != > > FormatStyle::BCIS_AfterColon ? 2 : 0) > > >getColumnLimit(State) || > >State.Stack.back().BreakBeforeParameter) && > > (Style.AllowShortFunctionsOnASingleLine != FormatStyle::SFS_All || > >Style.BreakConstructorInitializers != FormatStyle::BCIS_BeforeColon > > || > >Style.ColumnLimit != 0)) > > return true; > > > > what do you think? > The +2 here has nothing todo with how the things are aligned. This is about > whether the entire constructor with initializer fits on one line. Can you try > out (or even add tests) for cases where the entire constructor is 80 and 81 > columns long? > > I think I like the condensed version a bit better, but yeah, it's not > beautiful either ;). My mistake, I read to quickly and talked about another +2 I removed from an earlier patch. As far as I understand it, this +2 accounts for the the "upcoming" space and colon, when checking if breaking _before_ the colon (e.g. before it was added to the line). Since this case is trying to break _after_ the colon, the space and colon have already been added to line (i.e. removed the column limit). The tests are already included (and I have just double-checked: `Constructor() : Initializer(FitsOnTheLine) {}` is indeed 45 characters) : verifyFormat("Constructor() : Initializer(FitsOnTheLine) {}", getStyleWithColumns(Style, 45)); verifyFormat("Constructor() :\n" "Initializer(FitsOnTheLine) {}", getStyleWithColumns(Style, 44)); verifyFormat("Constructor() :\n" "Initializer(FitsOnTheLine) {}", getStyleWithColumns(Style, 43)); Comment at: lib/Format/ContinuationIndenter.cpp:468 + } else if (Previous.is(TT_CtorInitializerColon) && + Style.BreakConstructorInitializers == + FormatStyle::BCIS_AfterColon) { djasper wrote: > Does this fit on one line now? no, still 83 characters... Comment at: lib/Format/Format.cpp:309 +// If BreakConstructorInitializersBeforeComma was specified but +// BreakConstructorInitializers was not, initialize the latter from the +// former for backwards compatibility. djasper wrote: > How do you know that BreakConstructorInitializers was not specified? Technically, I don't ; but this is the default value, so it actually has the same effect... https://reviews.llvm.org/D32479 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32479: clang-format: Introduce BreakConstructorInitializers option
Typz updated this revision to Diff 99886. Typz marked 3 inline comments as done. Typz added a comment. Refactor to avoid duplicating code, and fix typo in test. https://reviews.llvm.org/D32479 Files: include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/Format.cpp lib/Format/TokenAnnotator.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -56,16 +56,17 @@ return *Result; } - FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) { -FormatStyle Style = getLLVMStyle(); + FormatStyle getStyleWithColumns(FormatStyle Style, unsigned ColumnLimit) { Style.ColumnLimit = ColumnLimit; return Style; } + FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) { +return getStyleWithColumns(getLLVMStyle(), ColumnLimit); + } + FormatStyle getGoogleStyleWithColumns(unsigned ColumnLimit) { -FormatStyle Style = getGoogleStyle(); -Style.ColumnLimit = ColumnLimit; -return Style; +return getStyleWithColumns(getGoogleStyle(), ColumnLimit); } void verifyFormat(llvm::StringRef Code, @@ -2699,6 +2700,165 @@ "() {}")); } +TEST_F(FormatTest, BreakConstructorInitializersAfterColon) { + FormatStyle Style = getLLVMStyle(); + Style.BreakConstructorInitializers = FormatStyle::BCIS_AfterColon; + + verifyFormat("Constructor() : Initializer(FitsOnTheLine) {}"); + verifyFormat("Constructor() : Initializer(FitsOnTheLine) {}", + getStyleWithColumns(Style, 45)); + verifyFormat("Constructor() :\n" + "Initializer(FitsOnTheLine) {}", + getStyleWithColumns(Style, 44)); + verifyFormat("Constructor() :\n" + "Initializer(FitsOnTheLine) {}", + getStyleWithColumns(Style, 43)); + + verifyFormat("template \n" + "Constructor() : Initializer(FitsOnTheLine) {}", + getStyleWithColumns(Style, 50)); + + verifyFormat( + "SomeClass::Constructor() :\n" + "a(aa), aaa() {}", + Style); + + verifyFormat( + "SomeClass::Constructor() :\n" + "a(aa), a(aa),\n" + "a(aa) {}", + Style); + verifyFormat( + "SomeClass::Constructor() :\n" + "aa(aa),\n" + "aaa() {}", + Style); + verifyFormat("Constructor(aa ,\n" + "aa ) :\n" + "aa(aa) {}", + Style); + + verifyFormat("Constructor() :\n" + "(aaa),\n" + "(aaa,\n" + " aaa),\n" + "aaa() {}", + Style); + + verifyFormat("Constructor() :\n" + "(\n" + "a) {}", + Style); + + verifyFormat("Constructor(int Parameter = 0) :\n" + "aa(a),\n" + "(a) {}", + Style); + verifyFormat("Constructor() :\n" + "aa(a), (b) {\n" + "}", + getStyleWithColumns(Style, 60)); + verifyFormat("Constructor() :\n" + "a(\n" + "a(, )) {}", + Style); + + // Here a line could be saved by splitting the second initializer onto two + // lines, but that is not desirable. + verifyFormat("Constructor() :\n" + "(),\n" + "aaa(aaa),\n" + "at() {}", + Style); + + FormatStyle OnePerLine = Style; + OnePerLine.ConstructorInitializerAllOnOneLineOrOnePerLine = true; + OnePerLine.AllowAllParametersOfDeclarationOnNextLine = false; + verifyFormat("SomeClass::Constructor() :\n" + "a(aa),\n" + "a(aa),\n" + "a(aa) {}", + OnePerLine); + verifyFormat("SomeClass::Constructor() :\n" + "a(aa), // Some comment\n" + "a(aa),\n" + "a(aa) {}", + On
[PATCH] D32525: [clang-format] Add SpaceBeforeColon option
Typz added a comment. In https://reviews.llvm.org/D32525#760710, @djasper wrote: > Then you also (accidentally?) change the spacing in dict literals. However, > the latter is already controlled by SpacesInContainerLiterals. not really accidentally: I want the space on the 'outside' of dict literals, but not before the the colons. Something like: [ a: 1, b: 2 ] f({ a: 1, b: 2, c: 3 }); (btw this option does not seem to affect the space on the 'outside' of dict literals with curly braces. Is this a bug/side effect, or an actual expected behavior? This specific case would also happen for C99 designated literals) The "logic" behind this patch is that it affects all cases where the colon is used as a separator, but not as a label or operator: e.g. where it would make some sense to format it similarly to a semi-colon. I am not sure where the assembly stands in this regard, though. https://reviews.llvm.org/D32525 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32479: clang-format: Introduce BreakConstructorInitializers option
djasper added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:196 + FormatStyle::BCIS_AfterColonAndComma) && + (State.Column + State.Line->Last->TotalLength - Previous.TotalLength > + getColumnLimit(State) || Typz wrote: > djasper wrote: > > Typz wrote: > > > Typz wrote: > > > > djasper wrote: > > > > > Why can you drop the "+2" here? > > > > > > > > > > Also, I'd like to structure this so we have to duplicate less of the > > > > > logic. But I am not really sure it's possible. > > > > the +2 here was needed to keep identifiers aligned when breaking after > > > > colon but before the command (e.g. the 4th combination, not defined > > > > anymore): > > > > > > > > Foo() : > > > > field(1) > > > > , field(2) > > > I can avoid some duplication like this,m but i am not convinced it helps : > > > > > > const FormatToken &ColonToken = > > > Style.BreakConstructorInitializers != FormatStyle::BCIS_AfterColon > > > ? Current : Previous; > > > if (ColonToken.is(TT_CtorInitializerColon) && > > > (State.Column + State.Line->Last->TotalLength - > > > ColonToken.TotalLength + > > >(Style.BreakConstructorInitializers != > > > FormatStyle::BCIS_AfterColon ? 2 : 0) > > > >getColumnLimit(State) || > > >State.Stack.back().BreakBeforeParameter) && > > > (Style.AllowShortFunctionsOnASingleLine != FormatStyle::SFS_All || > > >Style.BreakConstructorInitializers != > > > FormatStyle::BCIS_BeforeColon || > > >Style.ColumnLimit != 0)) > > > return true; > > > > > > what do you think? > > The +2 here has nothing todo with how the things are aligned. This is about > > whether the entire constructor with initializer fits on one line. Can you > > try out (or even add tests) for cases where the entire constructor is 80 > > and 81 columns long? > > > > I think I like the condensed version a bit better, but yeah, it's not > > beautiful either ;). > My mistake, I read to quickly and talked about another +2 I removed from an > earlier patch. > > As far as I understand it, this +2 accounts for the the "upcoming" space and > colon, when checking if breaking _before_ the colon (e.g. before it was added > to the line). > > Since this case is trying to break _after_ the colon, the space and colon > have already been added to line (i.e. removed the column limit). > > The tests are already included (and I have just double-checked: > `Constructor() : Initializer(FitsOnTheLine) {}` is indeed 45 characters) : > > verifyFormat("Constructor() : Initializer(FitsOnTheLine) {}", >getStyleWithColumns(Style, 45)); > verifyFormat("Constructor() :\n" >"Initializer(FitsOnTheLine) {}", >getStyleWithColumns(Style, 44)); > verifyFormat("Constructor() :\n" >"Initializer(FitsOnTheLine) {}", >getStyleWithColumns(Style, 43)); Ah, right. So as we are on the next token, State.Column will already include the +2. However, I think we should change that and make this always: State.Column + State.Line->Last->TotalLength - Previous.TotalLength > getColumnLimit(State) I think this should automatically add the "+2" or actually +1 should we go forward with your patch to not have a space before the colon at some point. https://reviews.llvm.org/D32479 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33430: [clang-tidy] Do not dereference a null BaseType
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D33430 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32525: [clang-format] Add SpaceBeforeColon option
djasper added a comment. Don't C99 designated literals use "=" instead of ":"? https://reviews.llvm.org/D32525 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32479: clang-format: Introduce BreakConstructorInitializers option
Typz updated this revision to Diff 99887. Typz added a comment. Cleanup according to review comment https://reviews.llvm.org/D32479 Files: include/clang/Format/Format.h lib/Format/ContinuationIndenter.cpp lib/Format/Format.cpp lib/Format/TokenAnnotator.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -56,16 +56,17 @@ return *Result; } - FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) { -FormatStyle Style = getLLVMStyle(); + FormatStyle getStyleWithColumns(FormatStyle Style, unsigned ColumnLimit) { Style.ColumnLimit = ColumnLimit; return Style; } + FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) { +return getStyleWithColumns(getLLVMStyle(), ColumnLimit); + } + FormatStyle getGoogleStyleWithColumns(unsigned ColumnLimit) { -FormatStyle Style = getGoogleStyle(); -Style.ColumnLimit = ColumnLimit; -return Style; +return getStyleWithColumns(getGoogleStyle(), ColumnLimit); } void verifyFormat(llvm::StringRef Code, @@ -2699,6 +2700,165 @@ "() {}")); } +TEST_F(FormatTest, BreakConstructorInitializersAfterColon) { + FormatStyle Style = getLLVMStyle(); + Style.BreakConstructorInitializers = FormatStyle::BCIS_AfterColon; + + verifyFormat("Constructor() : Initializer(FitsOnTheLine) {}"); + verifyFormat("Constructor() : Initializer(FitsOnTheLine) {}", + getStyleWithColumns(Style, 45)); + verifyFormat("Constructor() :\n" + "Initializer(FitsOnTheLine) {}", + getStyleWithColumns(Style, 44)); + verifyFormat("Constructor() :\n" + "Initializer(FitsOnTheLine) {}", + getStyleWithColumns(Style, 43)); + + verifyFormat("template \n" + "Constructor() : Initializer(FitsOnTheLine) {}", + getStyleWithColumns(Style, 50)); + + verifyFormat( + "SomeClass::Constructor() :\n" + "a(aa), aaa() {}", + Style); + + verifyFormat( + "SomeClass::Constructor() :\n" + "a(aa), a(aa),\n" + "a(aa) {}", + Style); + verifyFormat( + "SomeClass::Constructor() :\n" + "aa(aa),\n" + "aaa() {}", + Style); + verifyFormat("Constructor(aa ,\n" + "aa ) :\n" + "aa(aa) {}", + Style); + + verifyFormat("Constructor() :\n" + "(aaa),\n" + "(aaa,\n" + " aaa),\n" + "aaa() {}", + Style); + + verifyFormat("Constructor() :\n" + "(\n" + "a) {}", + Style); + + verifyFormat("Constructor(int Parameter = 0) :\n" + "aa(a),\n" + "(a) {}", + Style); + verifyFormat("Constructor() :\n" + "aa(a), (b) {\n" + "}", + getStyleWithColumns(Style, 60)); + verifyFormat("Constructor() :\n" + "a(\n" + "a(, )) {}", + Style); + + // Here a line could be saved by splitting the second initializer onto two + // lines, but that is not desirable. + verifyFormat("Constructor() :\n" + "(),\n" + "aaa(aaa),\n" + "at() {}", + Style); + + FormatStyle OnePerLine = Style; + OnePerLine.ConstructorInitializerAllOnOneLineOrOnePerLine = true; + OnePerLine.AllowAllParametersOfDeclarationOnNextLine = false; + verifyFormat("SomeClass::Constructor() :\n" + "a(aa),\n" + "a(aa),\n" + "a(aa) {}", + OnePerLine); + verifyFormat("SomeClass::Constructor() :\n" + "a(aa), // Some comment\n" + "a(aa),\n" + "a(aa) {}", + OnePerLine); + verifyFormat("MyClass::MyClass(int var) :\n" +
[PATCH] D32479: clang-format: Introduce BreakConstructorInitializers option
Typz added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:196 + FormatStyle::BCIS_AfterColonAndComma) && + (State.Column + State.Line->Last->TotalLength - Previous.TotalLength > + getColumnLimit(State) || djasper wrote: > Typz wrote: > > djasper wrote: > > > Typz wrote: > > > > Typz wrote: > > > > > djasper wrote: > > > > > > Why can you drop the "+2" here? > > > > > > > > > > > > Also, I'd like to structure this so we have to duplicate less of > > > > > > the logic. But I am not really sure it's possible. > > > > > the +2 here was needed to keep identifiers aligned when breaking > > > > > after colon but before the command (e.g. the 4th combination, not > > > > > defined anymore): > > > > > > > > > > Foo() : > > > > > field(1) > > > > > , field(2) > > > > I can avoid some duplication like this,m but i am not convinced it > > > > helps : > > > > > > > > const FormatToken &ColonToken = > > > > Style.BreakConstructorInitializers != FormatStyle::BCIS_AfterColon > > > > ? Current : Previous; > > > > if (ColonToken.is(TT_CtorInitializerColon) && > > > > (State.Column + State.Line->Last->TotalLength - > > > > ColonToken.TotalLength + > > > >(Style.BreakConstructorInitializers != > > > > FormatStyle::BCIS_AfterColon ? 2 : 0) > > > > >getColumnLimit(State) || > > > >State.Stack.back().BreakBeforeParameter) && > > > > (Style.AllowShortFunctionsOnASingleLine != FormatStyle::SFS_All || > > > >Style.BreakConstructorInitializers != > > > > FormatStyle::BCIS_BeforeColon || > > > >Style.ColumnLimit != 0)) > > > > return true; > > > > > > > > what do you think? > > > The +2 here has nothing todo with how the things are aligned. This is > > > about whether the entire constructor with initializer fits on one line. > > > Can you try out (or even add tests) for cases where the entire > > > constructor is 80 and 81 columns long? > > > > > > I think I like the condensed version a bit better, but yeah, it's not > > > beautiful either ;). > > My mistake, I read to quickly and talked about another +2 I removed from an > > earlier patch. > > > > As far as I understand it, this +2 accounts for the the "upcoming" space > > and colon, when checking if breaking _before_ the colon (e.g. before it was > > added to the line). > > > > Since this case is trying to break _after_ the colon, the space and colon > > have already been added to line (i.e. removed the column limit). > > > > The tests are already included (and I have just double-checked: > > `Constructor() : Initializer(FitsOnTheLine) {}` is indeed 45 characters) : > > > > verifyFormat("Constructor() : Initializer(FitsOnTheLine) {}", > >getStyleWithColumns(Style, 45)); > > verifyFormat("Constructor() :\n" > >"Initializer(FitsOnTheLine) {}", > >getStyleWithColumns(Style, 44)); > > verifyFormat("Constructor() :\n" > >"Initializer(FitsOnTheLine) {}", > >getStyleWithColumns(Style, 43)); > Ah, right. So as we are on the next token, State.Column will already include > the +2. However, I think we should change that and make this always: > > State.Column + State.Line->Last->TotalLength - Previous.TotalLength > > getColumnLimit(State) > > I think this should automatically add the "+2" or actually +1 should we go > forward with your patch to not have a space before the colon at some point. Seems to work indeed, looking much better! I had some trouble deciphering this when making my initial patch, and did not take the chance/risk to try to improve the 'regular' case. https://reviews.llvm.org/D32479 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32480: clang-format: Add CompactNamespaces option
Typz added a comment. In https://reviews.llvm.org/D32480#761862, @krasimir wrote: > In any case, adding a namespace end comment to a line closing multiple > namespaces is super confusing for me: what does the comment refer to: the > inner one, the outer one, or both? > `}} // namespace A::B` it refers to both, similar to C++17' _nested namespace_ definition: namespace A::B { }} // namespace A::B https://reviews.llvm.org/D32480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33398: Mangle __unaligned in Itanium ABI
aaron.ballman added inline comments. Comment at: test/CodeGenCXX/pr33080.cpp:19 +void hb(__unaligned struct A*, __unaligned const struct A*) {} +// CHECK: define void @_Z2hbPU11__unaligned1APU11__unalignedKS_( Can we also get a test like `struct A * __unaligned * __unaligned *`? Also, perhaps a test case involving templates would be good. https://reviews.llvm.org/D33398 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32525: [clang-format] Add SpaceBeforeColon option
Typz added a comment. Indeed, so this is why :-) https://reviews.llvm.org/D32525 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33437: Emit available_externally vtables opportunistically
Prazek updated this revision to Diff 99897. Prazek added a comment. Removed debug print https://reviews.llvm.org/D33437 Files: include/clang/AST/VTableBuilder.h lib/CodeGen/CGVTables.cpp lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.h lib/CodeGen/ItaniumCXXABI.cpp test/CodeGenCXX/vtable-available-externally.cpp test/CodeGenCXX/vtable-linkage.cpp Index: test/CodeGenCXX/vtable-linkage.cpp === --- test/CodeGenCXX/vtable-linkage.cpp +++ test/CodeGenCXX/vtable-linkage.cpp @@ -145,12 +145,14 @@ // F is an explicit template instantiation declaration without a // key function, so its vtable should have external linkage. // CHECK-DAG: @_ZTV1FIiE = external unnamed_addr constant -// CHECK-OPT-DAG: @_ZTV1FIiE = external unnamed_addr constant +// CHECK-OPT-DAG: @_ZTV1FIiE = available_externally unnamed_addr constant // E is an explicit template instantiation declaration. It has a // key function is not instantiated, so we know that vtable definition // will be generated in TU where key function will be defined -// so we can mark it as available_externally (only with optimizations) +// so we can mark it as external (without optimizations) and +// available_externally (with optimizations) because all of the inline +// virtual functions have been emitted. // CHECK-DAG: @_ZTV1EIiE = external unnamed_addr constant // CHECK-OPT-DAG: @_ZTV1EIiE = available_externally unnamed_addr constant Index: test/CodeGenCXX/vtable-available-externally.cpp === --- test/CodeGenCXX/vtable-available-externally.cpp +++ test/CodeGenCXX/vtable-available-externally.cpp @@ -12,6 +12,7 @@ // RUN: FileCheck --check-prefix=CHECK-TEST14 %s < %t.opt // RUN: FileCheck --check-prefix=CHECK-TEST15 %s < %t.opt // RUN: FileCheck --check-prefix=CHECK-TEST16 %s < %t.opt +// RUN: FileCheck --check-prefix=CHECK-TEST17 %s < %t.opt #include @@ -274,8 +275,8 @@ virtual D& operator=(const D&); }; -// Cannot emit B's vtable available_externally, because we cannot create -// a reference to the inline virtual B::operator= function. +// Cannot emit D's vtable available_externally, because we cannot create +// a reference to the inline virtual D::operator= function. // CHECK-TEST11: @_ZTVN6Test111DE = external unnamed_addr constant struct D : C { virtual void key(); @@ -391,3 +392,30 @@ } } +namespace Test17 { +// This test checks if we emit vtables opportunistically. +// CHECK-TEST17-DAG: @_ZTVN6Test171AE = available_externally +// CHECK-TEST17-DAG: @_ZTVN6Test171BE = external + +struct A { + virtual void key(); + virtual void bar() {} +}; + +// We won't gonna use deleting destructor for this type, which will disallow +// emitting vtable as available_externally +struct B { + virtual void key(); + virtual ~B() {} +}; + +void testcaseA() { + A a; + a.bar(); // this forces to emit definition of bar +} + +void testcaseB() { + B b; // This only forces emitting of complete object destructor +} + +} // namespace Test17 Index: lib/CodeGen/ItaniumCXXABI.cpp === --- lib/CodeGen/ItaniumCXXABI.cpp +++ lib/CodeGen/ItaniumCXXABI.cpp @@ -366,20 +366,30 @@ void emitCXXStructor(const CXXMethodDecl *MD, StructorType Type) override; private: - bool hasAnyVirtualInlineFunction(const CXXRecordDecl *RD) const { -const auto &VtableLayout = -CGM.getItaniumVTableContext().getVTableLayout(RD); - -for (const auto &VtableComponent : VtableLayout.vtable_components()) { - // Skip empty slot. - if (!VtableComponent.isUsedFunctionPointerKind()) -continue; - - const CXXMethodDecl *Method = VtableComponent.getFunctionDecl(); - if (Method->getCanonicalDecl()->isInlined()) -return true; -} -return false; + bool hasAnyUnusedVirtualInlineFunction(const CXXRecordDecl *RD) const { + const auto &VtableLayout = + CGM.getItaniumVTableContext().getVTableLayout(RD); + + for (const auto &VtableComponent : VtableLayout.vtable_components()) { + // Skip empty slot. + if (!VtableComponent.isUsedFunctionPointerKind()) + continue; + + const CXXMethodDecl *Method = VtableComponent.getFunctionDecl(); + if (!Method->getCanonicalDecl()->isInlined()) + continue; + + StringRef Name = CGM.getMangledName(VtableComponent.getGlobalDecl()); + auto *Entry = CGM.GetGlobalValue(Name); + // This checks if virtual inline function has already been emitted. + // Note that it is possible that this inline function would be emitted + // after trying to emit vtable speculatively. Because of this we do + // an extra pass after emitting all deferred vtables to find and emit + // these vtables opportunistically. + if (!Entry || Entry->isDeclaration()) + return true; + } + return false; } b
[PATCH] D32896: [OpenCL] Make CLK_NULL_RESERVE_ID invalid reserve id.
yaxunl added inline comments. Comment at: lib/Headers/opencl-c.h:16020 +// The macro CLK_NULL_RESERVE_ID refers to an invalid reservation ID. +#define CLK_NULL_RESERVE_ID (__builtin_astype((void *)0, reserve_id_t)) bool __ovld is_valid_reserve_id(reserve_id_t reserve_id); bader wrote: > yaxunl wrote: > > bader wrote: > > > yaxunl wrote: > > > > bader wrote: > > > > > yaxunl wrote: > > > > > > yaxunl wrote: > > > > > > > bader wrote: > > > > > > > > yaxunl wrote: > > > > > > > > > Anastasia wrote: > > > > > > > > > > echuraev wrote: > > > > > > > > > > > yaxunl wrote: > > > > > > > > > > > > Anastasia wrote: > > > > > > > > > > > > > yaxunl wrote: > > > > > > > > > > > > > > Anastasia wrote: > > > > > > > > > > > > > > > Looks good from my side. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > @yaxunl , since you originally committed this. > > > > > > > > > > > > > > > Could you please verify that changing from > > > > > > > > > > > > > > > `SIZE_MAX` to `0` would be fine. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Btw, we have a similar definition for > > > > > > > > > > > > > > > `CLK_NULL_EVENT`. > > > > > > > > > > > > > > `__PIPE_RESERVE_ID_VALID_BIT` is implementation > > > > > > > > > > > > > > detail and not part of the spec. I would suggest to > > > > > > > > > > > > > > remove it from this header file. > > > > > > > > > > > > > > > > > > > > > > > > > > > > The spec only requires CLK_NULL_RESERVE_ID to be > > > > > > > > > > > > > > defined but does not define its value. Naturally a > > > > > > > > > > > > > > valid id starts from 0 and increases. I don't see > > > > > > > > > > > > > > significant advantage to change CLK_NULL_RESERVE_ID > > > > > > > > > > > > > > from __SIZE_MAX to 0. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Is there any reason that this change is needed? > > > > > > > > > > > > > I don't see issues to commit things outside of spec > > > > > > > > > > > > > as soon as they prefixed properly with "__". But I > > > > > > > > > > > > > agree it would be nice to see if it's any useful and > > > > > > > > > > > > > what the motivation is for having different > > > > > > > > > > > > > implementation. > > > > > > > > > > > > For `__PIPE_RESERVE_ID_VALID_BIT`, it assumes that the > > > > > > > > > > > > implementation uses one specific bit of a reserve id to > > > > > > > > > > > > indicate that the reserve id is valid. Not all > > > > > > > > > > > > implementations assume that. Actually I am curious why > > > > > > > > > > > > that is needed too. > > > > > > > > > > > About `CLK_NULL_RESERVE_ID`: we check that reserve id is > > > > > > > > > > > valid if significant bit equal to one. > > > > > > > > > > > `CLK_NULL_RESERVE_ID refers to an invalid reservation, so > > > > > > > > > > > if `CLK_NULL_RESERVE_ID equal to 0, we can be sure that > > > > > > > > > > > significant bit doesn't equal to 1 and it is invalid > > > > > > > > > > > reserve id. Also it is more obviously if > > > > > > > > > > > CLK_**NULL**_RESERVE_ID is equal to 0. > > > > > > > > > > > > > > > > > > > > > > What about `__PIPE_RESERVE_ID_VALID_BIT`: As I understand > > > > > > > > > > > previous implementation also assumes that one specific > > > > > > > > > > > bit was of a reverse id was used to indicate that the > > > > > > > > > > > reserve id is valid. So, we just increased reserve id > > > > > > > > > > > size by one bit on 32-bit platforms and by 33 bits on > > > > > > > > > > > 64-bit platforms. > > > > > > > > > > It is more logical to me that `CLK_NULL_RESERVE_ID` is 0, > > > > > > > > > > but spec doesn't define it of course. > > > > > > > > > In our implementation, valid reserve id starts at 0 and > > > > > > > > > increasing linearly until `__SIZE_MAX-1`. This change will > > > > > > > > > break our implementation. > > > > > > > > > > > > > > > > > > However, we can modify our implementation to adopt this > > > > > > > > > change since it brings about benefits overall. > > > > > > > > Ideally it would be great to have unified implementation, but > > > > > > > > we can define device specific value for CLK_NULL_RESERVE_ID by > > > > > > > > using ifdef directive. > > > > > > > How about > > > > > > > > > > > > > > ``` > > > > > > > __attribute__((const)) size_t __clk_null_reserve_id(); > > > > > > > #define CLK_NULL_RESERVE_ID __clk_null_reserve_id() > > > > > > > > > > > > > > ``` > > > > > > > I think the spec does not require it to be compile time constant. > > > > > > > Then each library can implement its own __clk_null_reserve_id() > > > > > > > whereas the IR is target independent. > > > > > > Or we only do this for SPIR and define it as target specific value > > > > > > for other targets. > > > > > Defining CLK_NULL_RESERVE_ID as a function call should also work, but > > > > > IMHO compile time constant is preferable option. > > > > > I don't think making it compile time c
[PATCH] D33416: [clangd] Allow to use vfs::FileSystem for file accesses.
ilya-biryukov added inline comments. Comment at: unittests/clangd/ClangdTests.cpp:118 + llvm::SmallString<128> TmpDir2; + llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true, TmpDir2); + bkramer wrote: > We should delete those when we're done, otherwise the unit test will litter > /tmp on every run. ASTUnit handles that, right? It's only here to provide access to PCHs ASTUnit creates, we don't actually write to those dirs ourselves. https://reviews.llvm.org/D33416 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r303634 - [clangd] Replaced WorkerRequest with std::function...
Author: ibiryukov Date: Tue May 23 08:42:59 2017 New Revision: 303634 URL: http://llvm.org/viewvc/llvm-project?rev=303634&view=rev Log: [clangd] Replaced WorkerRequest with std::function... Summary: And implemented a helper function to dump an AST of a file for testing/debugging purposes. Reviewers: bkramer, krasimir Reviewed By: krasimir Subscribers: klimek, cfe-commits Tags: #clang-tools-extra Differential Revision: https://reviews.llvm.org/D33415 Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp clang-tools-extra/trunk/clangd/ClangdServer.h clang-tools-extra/trunk/clangd/ClangdUnit.cpp clang-tools-extra/trunk/clangd/ClangdUnit.h clang-tools-extra/trunk/clangd/ClangdUnitStore.h Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=303634&r1=303633&r2=303634&view=diff == --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue May 23 08:42:59 2017 @@ -15,6 +15,8 @@ #include "clang/Tooling/CompilationDatabase.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/Support/FileSystem.h" +#include "llvm/Support/raw_ostream.h" +#include using namespace clang; using namespace clang::clangd; @@ -56,11 +58,7 @@ Position clangd::offsetToPosition(String return {Lines, Cols}; } -WorkerRequest::WorkerRequest(WorkerRequestKind Kind, Path File, - DocVersion Version) -: Kind(Kind), File(File), Version(Version) {} - -ClangdScheduler::ClangdScheduler(ClangdServer &Server, bool RunSynchronously) +ClangdScheduler::ClangdScheduler(bool RunSynchronously) : RunSynchronously(RunSynchronously) { if (RunSynchronously) { // Don't start the worker thread if we're running synchronously @@ -69,9 +67,9 @@ ClangdScheduler::ClangdScheduler(ClangdS // Initialize Worker in ctor body, rather than init list to avoid potentially // using not-yet-initialized members - Worker = std::thread([&Server, this]() { + Worker = std::thread([this]() { while (true) { - WorkerRequest Request; + std::function Request; // Pick request from the queue { @@ -83,19 +81,15 @@ ClangdScheduler::ClangdScheduler(ClangdS assert(!RequestQueue.empty() && "RequestQueue was empty"); -Request = std::move(RequestQueue.back()); -RequestQueue.pop_back(); - -// Skip outdated requests -if (Request.Version != Server.DraftMgr.getVersion(Request.File)) { - // FIXME(ibiryukov): Logging - // Output.log("Version for " + Twine(Request.File) + - //" in request is outdated, skipping request\n"); - continue; -} +// We process requests starting from the front of the queue. Users of +// ClangdScheduler have a way to prioritise their requests by putting +// them to the either side of the queue (using either addToEnd or +// addToFront). +Request = std::move(RequestQueue.front()); +RequestQueue.pop_front(); } // unlock Mutex - Server.handleRequest(std::move(Request)); + Request(); } }); } @@ -108,19 +102,34 @@ ClangdScheduler::~ClangdScheduler() { std::lock_guard Lock(Mutex); // Wake up the worker thread Done = true; -RequestCV.notify_one(); } // unlock Mutex + RequestCV.notify_one(); Worker.join(); } -void ClangdScheduler::enqueue(ClangdServer &Server, WorkerRequest Request) { +void ClangdScheduler::addToFront(std::function Request) { if (RunSynchronously) { -Server.handleRequest(Request); +Request(); return; } - std::lock_guard Lock(Mutex); - RequestQueue.push_back(Request); + { +std::lock_guard Lock(Mutex); +RequestQueue.push_front(Request); + } + RequestCV.notify_one(); +} + +void ClangdScheduler::addToEnd(std::function Request) { + if (RunSynchronously) { +Request(); +return; + } + + { +std::lock_guard Lock(Mutex); +RequestQueue.push_back(Request); + } RequestCV.notify_one(); } @@ -129,19 +138,34 @@ ClangdServer::ClangdServer(std::unique_p bool RunSynchronously) : CDB(std::move(CDB)), DiagConsumer(std::move(DiagConsumer)), PCHs(std::make_shared()), - WorkScheduler(*this, RunSynchronously) {} + WorkScheduler(RunSynchronously) {} void ClangdServer::addDocument(PathRef File, StringRef Contents) { - DocVersion NewVersion = DraftMgr.updateDraft(File, Contents); - WorkScheduler.enqueue( - *this, WorkerRequest(WorkerRequestKind::ParseAndPublishDiagnostics, File, - NewVersion)); + DocVersion Version = DraftMgr.updateDraft(File, Contents); + Path FileStr = File; + WorkScheduler.addToFront([this, FileStr, Version]() { +auto FileContents = DraftMgr.getDraft(FileStr); +if (FileCont
[PATCH] D33415: [clangd] Replaced WorkerRequest with std::function...
This revision was automatically updated to reflect the committed changes. Closed by commit rL303634: [clangd] Replaced WorkerRequest with std::function... (authored by ibiryukov). Changed prior to commit: https://reviews.llvm.org/D33415?vs=99883&id=99900#toc Repository: rL LLVM https://reviews.llvm.org/D33415 Files: clang-tools-extra/trunk/clangd/ClangdServer.cpp clang-tools-extra/trunk/clangd/ClangdServer.h clang-tools-extra/trunk/clangd/ClangdUnit.cpp clang-tools-extra/trunk/clangd/ClangdUnit.h clang-tools-extra/trunk/clangd/ClangdUnitStore.h Index: clang-tools-extra/trunk/clangd/ClangdUnit.h === --- clang-tools-extra/trunk/clangd/ClangdUnit.h +++ clang-tools-extra/trunk/clangd/ClangdUnit.h @@ -16,6 +16,10 @@ #include "clang/Tooling/Core/Replacement.h" #include +namespace llvm { +class raw_ostream; +} + namespace clang { class ASTUnit; class PCHContainerOperations; @@ -52,6 +56,10 @@ /// located in the current file. std::vector getLocalDiagnostics() const; + /// For testing/debugging purposes. Note that this method deserializes all + /// unserialized Decls, so use with care. + void dumpAST(llvm::raw_ostream &OS) const; + private: Path FileName; std::unique_ptr Unit; Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp === --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp @@ -222,3 +222,7 @@ } return Result; } + +void ClangdUnit::dumpAST(llvm::raw_ostream &OS) const { + Unit->getASTContext().getTranslationUnitDecl()->dump(OS, true); +} Index: clang-tools-extra/trunk/clangd/ClangdServer.h === --- clang-tools-extra/trunk/clangd/ClangdServer.h +++ clang-tools-extra/trunk/clangd/ClangdServer.h @@ -24,6 +24,7 @@ #include "Protocol.h" #include +#include #include #include #include @@ -49,30 +50,25 @@ std::vector Diagnostics) = 0; }; -enum class WorkerRequestKind { ParseAndPublishDiagnostics, RemoveDocData }; - -/// A request to the worker thread -class WorkerRequest { -public: - WorkerRequest() = default; - WorkerRequest(WorkerRequestKind Kind, Path File, DocVersion Version); - - WorkerRequestKind Kind; - Path File; - DocVersion Version; -}; - class ClangdServer; /// Handles running WorkerRequests of ClangdServer on a separate threads. /// Currently runs only one worker thread. class ClangdScheduler { public: - ClangdScheduler(ClangdServer &Server, bool RunSynchronously); + ClangdScheduler(bool RunSynchronously); ~ClangdScheduler(); - /// Enqueue WorkerRequest to be run on a worker thread - void enqueue(ClangdServer &Server, WorkerRequest Request); + /// Add \p Request to the start of the queue. \p Request will be run on a + /// separate worker thread. + /// \p Request is scheduled to be executed before all currently added + /// requests. + void addToFront(std::function Request); + /// Add \p Request to the end of the queue. \p Request will be run on a + /// separate worker thread. + /// \p Request is scheduled to be executed after all currently added + /// requests. + void addToEnd(std::function Request); private: bool RunSynchronously; @@ -83,11 +79,10 @@ std::thread Worker; /// Setting Done to true will make the worker thread terminate. bool Done = false; - /// A LIFO queue of requests. Note that requests are discarded if the - /// `version` field is not equal to the one stored inside DraftStore. + /// A queue of requests. /// FIXME(krasimir): code completion should always have priority over parsing /// for diagnostics. - std::deque RequestQueue; + std::deque> RequestQueue; /// Condition variable to wake up the worker thread. std::condition_variable RequestCV; }; @@ -126,12 +121,12 @@ /// conversions in outside code, maybe there's a way to get rid of it. std::string getDocument(PathRef File); -private: - friend class ClangdScheduler; - - /// This function is called on a worker thread. - void handleRequest(WorkerRequest Request); + /// Only for testing purposes. + /// Waits until all requests to worker thread are finished and dumps AST for + /// \p File. \p File must be in the list of added documents. + std::string dumpAST(PathRef File); +private: std::unique_ptr CDB; std::unique_ptr DiagConsumer; DraftStore DraftMgr; Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp === --- clang-tools-extra/trunk/clangd/ClangdServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp @@ -15,6 +15,8 @@ #include "clang/Tooling/CompilationDatabase.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/Support/FileSystem.h" +#include "llvm/Support/raw_ostream.h" +#include using namespace clang; using namespace clang::clangd; @@
[PATCH] D33416: [clangd] Allow to use vfs::FileSystem for file accesses.
ilya-biryukov updated this revision to Diff 99902. ilya-biryukov added a comment. Changed tests to use Windows-style paths for virtual directory when running on Windows. https://reviews.llvm.org/D33416 Files: clangd/ClangdLSPServer.cpp clangd/ClangdServer.cpp clangd/ClangdServer.h clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/ClangdUnitStore.h unittests/CMakeLists.txt unittests/clangd/CMakeLists.txt unittests/clangd/ClangdTests.cpp Index: unittests/clangd/ClangdTests.cpp === --- /dev/null +++ unittests/clangd/ClangdTests.cpp @@ -0,0 +1,366 @@ +//===-- ClangdTes.cpp - Change namespace unit tests ---*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "ClangdServer.h" +#include "clang/Basic/VirtualFileSystem.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringMap.h" +#include "llvm/Config/config.h" +#include "llvm/Support/Errc.h" +#include "llvm/Support/Path.h" +#include "llvm/Support/Regex.h" +#include "gtest/gtest.h" +#include +#include +#include +#include + +namespace clang { +namespace vfs { + +/// An implementation of vfs::FileSystem that only allows access to +/// files and folders inside a set of whitelisted directories. +/// +/// FIXME(ibiryukov): should it also emulate access to parents of whitelisted +/// directories with only whitelisted contents? +class FilteredFileSystem : public vfs::FileSystem { +public: + /// The paths inside \p WhitelistedDirs should be absolute + FilteredFileSystem(std::vector WhitelistedDirs, + IntrusiveRefCntPtr InnerFS) + : WhitelistedDirs(std::move(WhitelistedDirs)), InnerFS(InnerFS) { +assert(std::all_of(WhitelistedDirs.begin(), WhitelistedDirs.end(), + [](const std::string &Path) -> bool { + return llvm::sys::path::is_absolute(Path); + }) && + "Not all WhitelistedDirs are absolute"); + } + + virtual llvm::ErrorOr status(const Twine &Path) { +if (!isInsideWhitelistedDir(Path)) + return llvm::errc::no_such_file_or_directory; +return InnerFS->status(Path); + } + + virtual llvm::ErrorOr> + openFileForRead(const Twine &Path) { +if (!isInsideWhitelistedDir(Path)) + return llvm::errc::no_such_file_or_directory; +return InnerFS->openFileForRead(Path); + } + + llvm::ErrorOr> + getBufferForFile(const Twine &Name, int64_t FileSize = -1, + bool RequiresNullTerminator = true, + bool IsVolatile = false) { +if (!isInsideWhitelistedDir(Name)) + return llvm::errc::no_such_file_or_directory; +return InnerFS->getBufferForFile(Name, FileSize, RequiresNullTerminator, + IsVolatile); + } + + virtual directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) { +if (!isInsideWhitelistedDir(Dir)) { + EC = llvm::errc::no_such_file_or_directory; + return directory_iterator(); +} +return InnerFS->dir_begin(Dir, EC); + } + + virtual std::error_code setCurrentWorkingDirectory(const Twine &Path) { +return InnerFS->setCurrentWorkingDirectory(Path); + } + + virtual llvm::ErrorOr getCurrentWorkingDirectory() const { +return InnerFS->getCurrentWorkingDirectory(); + } + + bool exists(const Twine &Path) { +if (!isInsideWhitelistedDir(Path)) + return false; +return InnerFS->exists(Path); + } + + std::error_code makeAbsolute(SmallVectorImpl &Path) const { +return InnerFS->makeAbsolute(Path); + } + +private: + bool isInsideWhitelistedDir(const Twine &InputPath) const { +SmallString<128> Path; +InputPath.toVector(Path); + +if (makeAbsolute(Path)) + return false; + +for (const auto &Dir : WhitelistedDirs) { + if (Path.startswith(Dir)) +return true; +} +return false; + } + + std::vector WhitelistedDirs; + IntrusiveRefCntPtr InnerFS; +}; + +/// Create a vfs::FileSystem that has access only to temporary directories +/// (obtained by calling system_temp_directory). +IntrusiveRefCntPtr getTempOnlyFS() { + llvm::SmallString<128> TmpDir1; + llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, TmpDir1); + llvm::SmallString<128> TmpDir2; + llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true, TmpDir2); + + std::vector TmpDirs; + TmpDirs.push_back(TmpDir1.str()); + if (TmpDir2 != TmpDir2) +TmpDirs.push_back(TmpDir2.str()); + return new vfs::FilteredFileSystem(std::move(TmpDirs), + vfs::getRealFileSystem()); +} +} // namespace vfs + +namespace clangd { +namespace { + +class ErrorCheckingDiagConsumer : public DiagnosticsConsumer { +public: + void onDiagnosticsReady(PathRef Fi
[PATCH] D32351: [Tooling][libclang] Remove unused CompilationDatabase::MappedSources
This revision was automatically updated to reflect the committed changes. Closed by commit rL303635: [Tooling][libclang] Remove unused CompilationDatabase::MappedSources (authored by krasimir). Changed prior to commit: https://reviews.llvm.org/D32351?vs=96138&id=99905#toc Repository: rL LLVM https://reviews.llvm.org/D32351 Files: cfe/trunk/include/clang/Tooling/CompilationDatabase.h cfe/trunk/tools/libclang/CXCompilationDatabase.cpp Index: cfe/trunk/tools/libclang/CXCompilationDatabase.cpp === --- cfe/trunk/tools/libclang/CXCompilationDatabase.cpp +++ cfe/trunk/tools/libclang/CXCompilationDatabase.cpp @@ -145,36 +145,23 @@ unsigned clang_CompileCommand_getNumMappedSources(CXCompileCommand CCmd) { - if (!CCmd) -return 0; - - return static_cast(CCmd)->MappedSources.size(); + // Left here for backward compatibility. No mapped sources exists in the C++ + // backend anymore. + return 0; } CXString clang_CompileCommand_getMappedSourcePath(CXCompileCommand CCmd, unsigned I) { - if (!CCmd) -return cxstring::createNull(); - - CompileCommand *Cmd = static_cast(CCmd); - - if (I >= Cmd->MappedSources.size()) -return cxstring::createNull(); - - return cxstring::createRef(Cmd->MappedSources[I].first.c_str()); + // Left here for backward compatibility. No mapped sources exists in the C++ + // backend anymore. + return cxstring::createNull(); } CXString clang_CompileCommand_getMappedSourceContent(CXCompileCommand CCmd, unsigned I) { - if (!CCmd) -return cxstring::createNull(); - - CompileCommand *Cmd = static_cast(CCmd); - - if (I >= Cmd->MappedSources.size()) -return cxstring::createNull(); - - return cxstring::createRef(Cmd->MappedSources[I].second.c_str()); + // Left here for backward compatibility. No mapped sources exists in the C++ + // backend anymore. + return cxstring::createNull(); } Index: cfe/trunk/include/clang/Tooling/CompilationDatabase.h === --- cfe/trunk/include/clang/Tooling/CompilationDatabase.h +++ cfe/trunk/include/clang/Tooling/CompilationDatabase.h @@ -60,16 +60,6 @@ /// The output file associated with the command. std::string Output; - - /// \brief An optional mapping from each file's path to its content for all - /// files needed for the compilation that are not available via the file - /// system. - /// - /// Note that a tool implementation is required to fall back to the file - /// system if a source file is not provided in the mapped sources, as - /// compilation databases will usually not provide all files in mapped sources - /// for performance reasons. - std::vector > MappedSources; }; /// \brief Interface for compilation databases. Index: cfe/trunk/tools/libclang/CXCompilationDatabase.cpp === --- cfe/trunk/tools/libclang/CXCompilationDatabase.cpp +++ cfe/trunk/tools/libclang/CXCompilationDatabase.cpp @@ -145,36 +145,23 @@ unsigned clang_CompileCommand_getNumMappedSources(CXCompileCommand CCmd) { - if (!CCmd) -return 0; - - return static_cast(CCmd)->MappedSources.size(); + // Left here for backward compatibility. No mapped sources exists in the C++ + // backend anymore. + return 0; } CXString clang_CompileCommand_getMappedSourcePath(CXCompileCommand CCmd, unsigned I) { - if (!CCmd) -return cxstring::createNull(); - - CompileCommand *Cmd = static_cast(CCmd); - - if (I >= Cmd->MappedSources.size()) -return cxstring::createNull(); - - return cxstring::createRef(Cmd->MappedSources[I].first.c_str()); + // Left here for backward compatibility. No mapped sources exists in the C++ + // backend anymore. + return cxstring::createNull(); } CXString clang_CompileCommand_getMappedSourceContent(CXCompileCommand CCmd, unsigned I) { - if (!CCmd) -return cxstring::createNull(); - - CompileCommand *Cmd = static_cast(CCmd); - - if (I >= Cmd->MappedSources.size()) -return cxstring::createNull(); - - return cxstring::createRef(Cmd->MappedSources[I].second.c_str()); + // Left here for backward compatibility. No mapped sources exists in the C++ + // backend anymore. + return cxstring::createNull(); } Index: cfe/trunk/include/clang/Tooling/CompilationDatabase.h === --- cfe/trunk/include/clang/Tooling/CompilationDatabase.h +++ cfe/trunk/include/clang/Tooling/CompilationDatabase.h @@ -60,16 +60,6 @@ /// The output file associated with the command. std::string Output; - - /// \brief An optional mapping from each file's path to its content for all - /// files needed for the compilation that are not available via the file - /// system. - /// - /// Note that a tool implementation is required to fall back to the file - /// system if a source file is not provided in the mapped sources, as - /// compilation data
[PATCH] D33357: Avoid calling report_fatal_error in the destructor of raw_fd_ostream when saving a module timestamp file
arphaman added a comment. In https://reviews.llvm.org/D33357#761400, @bruno wrote: > Any idea why we're hitting this issue in the first place? The error that gets > cleaned up is reported at some point before? Seems to me that we're going to > fail to update the timestamp but continue as nothing happened, I wonder how > many other issues this might trigger... I don't know, I only have access to a crashlog without a reproducer. Correct me if I'm wrong though, but judging by the way the timestamp is used, it seems to me that if we fail to update the timestamp nothing bad will happen, except the build will have to do more work as it will have to validate the system input files in that module for each CU until the first write to the module's timestamp succeeds. I actually don't need the first change in the `if` it turns out, because that doesn't trigger a crash. The open file error is cleaned up and ignored, so I don't think that cleaning up an error on closure will break things. >> I couldn't think of a way to test this, do you think it's possible to have a >> test for this? > > If you can come up with a testcase it would be awesome, but for this to > trigger you'd have to reproduce the concurrent scenario in the testcase, > which I don't see how. It seems that I can't really test a failure on write/close unfortunately. Repository: rL LLVM https://reviews.llvm.org/D33357 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303635 - [Tooling][libclang] Remove unused CompilationDatabase::MappedSources
Author: krasimir Date: Tue May 23 08:50:43 2017 New Revision: 303635 URL: http://llvm.org/viewvc/llvm-project?rev=303635&view=rev Log: [Tooling][libclang] Remove unused CompilationDatabase::MappedSources Summary: This field is never assigned to and it's only ever read from libclang. This patch removes it and adapts libclang to return constants. Reviewers: klimek, bkramer Reviewed By: klimek Subscribers: cfe-commits, klimek Differential Revision: https://reviews.llvm.org/D32351 Modified: cfe/trunk/include/clang/Tooling/CompilationDatabase.h cfe/trunk/tools/libclang/CXCompilationDatabase.cpp Modified: cfe/trunk/include/clang/Tooling/CompilationDatabase.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/CompilationDatabase.h?rev=303635&r1=303634&r2=303635&view=diff == --- cfe/trunk/include/clang/Tooling/CompilationDatabase.h (original) +++ cfe/trunk/include/clang/Tooling/CompilationDatabase.h Tue May 23 08:50:43 2017 @@ -60,16 +60,6 @@ struct CompileCommand { /// The output file associated with the command. std::string Output; - - /// \brief An optional mapping from each file's path to its content for all - /// files needed for the compilation that are not available via the file - /// system. - /// - /// Note that a tool implementation is required to fall back to the file - /// system if a source file is not provided in the mapped sources, as - /// compilation databases will usually not provide all files in mapped sources - /// for performance reasons. - std::vector > MappedSources; }; /// \brief Interface for compilation databases. Modified: cfe/trunk/tools/libclang/CXCompilationDatabase.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CXCompilationDatabase.cpp?rev=303635&r1=303634&r2=303635&view=diff == --- cfe/trunk/tools/libclang/CXCompilationDatabase.cpp (original) +++ cfe/trunk/tools/libclang/CXCompilationDatabase.cpp Tue May 23 08:50:43 2017 @@ -145,36 +145,23 @@ clang_CompileCommand_getArg(CXCompileCom unsigned clang_CompileCommand_getNumMappedSources(CXCompileCommand CCmd) { - if (!CCmd) -return 0; - - return static_cast(CCmd)->MappedSources.size(); + // Left here for backward compatibility. No mapped sources exists in the C++ + // backend anymore. + return 0; } CXString clang_CompileCommand_getMappedSourcePath(CXCompileCommand CCmd, unsigned I) { - if (!CCmd) -return cxstring::createNull(); - - CompileCommand *Cmd = static_cast(CCmd); - - if (I >= Cmd->MappedSources.size()) -return cxstring::createNull(); - - return cxstring::createRef(Cmd->MappedSources[I].first.c_str()); + // Left here for backward compatibility. No mapped sources exists in the C++ + // backend anymore. + return cxstring::createNull(); } CXString clang_CompileCommand_getMappedSourceContent(CXCompileCommand CCmd, unsigned I) { - if (!CCmd) -return cxstring::createNull(); - - CompileCommand *Cmd = static_cast(CCmd); - - if (I >= Cmd->MappedSources.size()) -return cxstring::createNull(); - - return cxstring::createRef(Cmd->MappedSources[I].second.c_str()); + // Left here for backward compatibility. No mapped sources exists in the C++ + // backend anymore. + return cxstring::createNull(); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33357: Avoid calling report_fatal_error in the destructor of raw_fd_ostream when saving a module timestamp file
arphaman updated this revision to Diff 99904. arphaman added a comment. Remove the first redundant error check. Repository: rL LLVM https://reviews.llvm.org/D33357 Files: lib/Serialization/ASTReader.cpp Index: lib/Serialization/ASTReader.cpp === --- lib/Serialization/ASTReader.cpp +++ lib/Serialization/ASTReader.cpp @@ -3655,6 +3655,8 @@ if (EC) return; OS << "Timestamp file\n"; + OS.close(); + OS.clear_error(); // Avoid triggering a fatal error. } /// \brief Given a cursor at the start of an AST file, scan ahead and drop the Index: lib/Serialization/ASTReader.cpp === --- lib/Serialization/ASTReader.cpp +++ lib/Serialization/ASTReader.cpp @@ -3655,6 +3655,8 @@ if (EC) return; OS << "Timestamp file\n"; + OS.close(); + OS.clear_error(); // Avoid triggering a fatal error. } /// \brief Given a cursor at the start of an AST file, scan ahead and drop the ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32248: CodeGen: Cast alloca to expected address space
tstellar added inline comments. Comment at: cfe/trunk/lib/Basic/Targets.cpp:2037-2038 -static const LangAS::Map AMDGPUPrivateIsZeroMap = { -4, // Default -1, // opencl_global -3, // opencl_local -2, // opencl_constant -4, // opencl_generic -1, // cuda_device -2, // cuda_constant -3 // cuda_shared -}; -static const LangAS::Map AMDGPUGenericIsZeroMap = { -0, // Default -1, // opencl_global -3, // opencl_local -2, // opencl_constant -0, // opencl_generic -1, // cuda_device -2, // cuda_constant -3 // cuda_shared +static const LangAS::Map AMDGPUNonOpenCLPrivateIsZeroMap = { +4, // Default +1, // opencl_global Shouldn't this be 0 ? Repository: rL LLVM https://reviews.llvm.org/D32248 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
RE: D32248: CodeGen: Cast alloca to expected address space
I think if there is no triple environment or if the triple environment is "opencl", I should always map default address space to 0. Then it should not break libclc and previous OpenCL applications. Sam -Original Message- From: Tom Stellard via Phabricator [mailto:revi...@reviews.llvm.org] Sent: Tuesday, May 23, 2017 9:52 AM To: Liu, Yaxun (Sam) ; rjmcc...@gmail.com; Tye, Tony ; anastasia.stul...@arm.com Cc: tstel...@redhat.com; nhaeh...@gmail.com; Chan, SiuChi ; whch...@gmail.com; Sumner, Brian ; cfe-commits@lists.llvm.org Subject: [PATCH] D32248: CodeGen: Cast alloca to expected address space tstellar added inline comments. Comment at: cfe/trunk/lib/Basic/Targets.cpp:2037-2038 -static const LangAS::Map AMDGPUPrivateIsZeroMap = { -4, // Default -1, // opencl_global -3, // opencl_local -2, // opencl_constant -4, // opencl_generic -1, // cuda_device -2, // cuda_constant -3 // cuda_shared -}; -static const LangAS::Map AMDGPUGenericIsZeroMap = { -0, // Default -1, // opencl_global -3, // opencl_local -2, // opencl_constant -0, // opencl_generic -1, // cuda_device -2, // cuda_constant -3 // cuda_shared +static const LangAS::Map AMDGPUNonOpenCLPrivateIsZeroMap = { +4, // Default +1, // opencl_global Shouldn't this be 0 ? Repository: rL LLVM https://reviews.llvm.org/D32248 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r303636 - [clangd] Added a missing dependency on clangdAST to fix the build
Author: ibiryukov Date: Tue May 23 08:59:37 2017 New Revision: 303636 URL: http://llvm.org/viewvc/llvm-project?rev=303636&view=rev Log: [clangd] Added a missing dependency on clangdAST to fix the build Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=303636&r1=303635&r2=303636&view=diff == --- clang-tools-extra/trunk/clangd/CMakeLists.txt (original) +++ clang-tools-extra/trunk/clangd/CMakeLists.txt Tue May 23 08:59:37 2017 @@ -14,6 +14,7 @@ add_clang_library(clangDaemon ProtocolHandlers.cpp LINK_LIBS + clangAST clangBasic clangFormat clangFrontend ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33440: clang-format: properly handle Q_UNUSED and QT_REQUIRE_VERSION
Typz created this revision. Herald added a subscriber: klimek. These macros are used in the body of function, and actually contain the trailing semicolon: they should thus be automatically followed by a new line, and not get merged with the next line. void foo(int a, int b) { Q_UNUSED(a) return b; } This patch deals with this case, also handling the case where the macro would be immediately followed by semicolon. https://reviews.llvm.org/D33440 Files: lib/Format/FormatToken.h lib/Format/UnwrappedLineParser.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -1976,6 +1976,16 @@ getLLVMStyleWithColumns(40))); verifyFormat("MACRO(>)"); + + // Some macros contain an implicit semicolon + verifyFormat("Q_UNUSED(a)\n" + "int b = 0;"); + verifyFormat("Q_UNUSED(a);\n" + "int b = 0;"); + verifyFormat("QT_REQUIRE_VERSION(argc, argv, \"4.0.2\")\n" + "int b = 0;"); + verifyFormat("QT_REQUIRE_VERSION(argc, argv, \"4.0.2\");\n" + "int b = 0;"); } TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) { Index: lib/Format/UnwrappedLineParser.cpp === --- lib/Format/UnwrappedLineParser.cpp +++ lib/Format/UnwrappedLineParser.cpp @@ -926,6 +926,17 @@ return; } } +if (Style.isCpp() && FormatTok->isOneOf(Keywords.kw_qunused, +Keywords.kw_qtrequireversion)) { + nextToken(); + if (FormatTok->is(tok::l_paren)) { +parseParens(); +if (FormatTok->is(tok::semi)) + nextToken(); +addUnwrappedLine(); +return; + } +} // In all other cases, parse the declaration. break; default: Index: lib/Format/FormatToken.h === --- lib/Format/FormatToken.h +++ lib/Format/FormatToken.h @@ -652,6 +652,8 @@ kw_qsignals = &IdentTable.get("Q_SIGNALS"); kw_slots = &IdentTable.get("slots"); kw_qslots = &IdentTable.get("Q_SLOTS"); +kw_qunused = &IdentTable.get("Q_UNUSED"); +kw_qtrequireversion = &IdentTable.get("QT_REQUIRE_VERSION"); } // Context sensitive keywords. @@ -711,6 +713,8 @@ IdentifierInfo *kw_qsignals; IdentifierInfo *kw_slots; IdentifierInfo *kw_qslots; + IdentifierInfo *kw_qunused; + IdentifierInfo *kw_qtrequireversion; }; } // namespace format Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -1976,6 +1976,16 @@ getLLVMStyleWithColumns(40))); verifyFormat("MACRO(>)"); + + // Some macros contain an implicit semicolon + verifyFormat("Q_UNUSED(a)\n" + "int b = 0;"); + verifyFormat("Q_UNUSED(a);\n" + "int b = 0;"); + verifyFormat("QT_REQUIRE_VERSION(argc, argv, \"4.0.2\")\n" + "int b = 0;"); + verifyFormat("QT_REQUIRE_VERSION(argc, argv, \"4.0.2\");\n" + "int b = 0;"); } TEST_F(FormatTest, LayoutMacroDefinitionsStatementsSpanningBlocks) { Index: lib/Format/UnwrappedLineParser.cpp === --- lib/Format/UnwrappedLineParser.cpp +++ lib/Format/UnwrappedLineParser.cpp @@ -926,6 +926,17 @@ return; } } +if (Style.isCpp() && FormatTok->isOneOf(Keywords.kw_qunused, +Keywords.kw_qtrequireversion)) { + nextToken(); + if (FormatTok->is(tok::l_paren)) { +parseParens(); +if (FormatTok->is(tok::semi)) + nextToken(); +addUnwrappedLine(); +return; + } +} // In all other cases, parse the declaration. break; default: Index: lib/Format/FormatToken.h === --- lib/Format/FormatToken.h +++ lib/Format/FormatToken.h @@ -652,6 +652,8 @@ kw_qsignals = &IdentTable.get("Q_SIGNALS"); kw_slots = &IdentTable.get("slots"); kw_qslots = &IdentTable.get("Q_SLOTS"); +kw_qunused = &IdentTable.get("Q_UNUSED"); +kw_qtrequireversion = &IdentTable.get("QT_REQUIRE_VERSION"); } // Context sensitive keywords. @@ -711,6 +713,8 @@ IdentifierInfo *kw_qsignals; IdentifierInfo *kw_slots; IdentifierInfo *kw_qslots; + IdentifierInfo *kw_qunused; + IdentifierInfo *kw_qtrequireversion; }; } // namespace format ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31670: [coroutines] Implement correct GRO lifetime
GorNishanov updated this revision to Diff 99911. GorNishanov added a comment. merged with top of the trunk https://reviews.llvm.org/D31670 Files: lib/CodeGen/CGCoroutine.cpp test/CodeGenCoroutines/coro-gro.cpp Index: test/CodeGenCoroutines/coro-gro.cpp === --- /dev/null +++ test/CodeGenCoroutines/coro-gro.cpp @@ -0,0 +1,86 @@ +// Verifies lifetime of __gro local variable +// Verify that coroutine promise and allocated memory are freed up on exception. +// RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -disable-llvm-passes | FileCheck %s + +namespace std::experimental { +template struct coroutine_traits; + +template struct coroutine_handle { + coroutine_handle() = default; + static coroutine_handle from_address(void *) noexcept; +}; +template <> struct coroutine_handle { + static coroutine_handle from_address(void *) noexcept; + coroutine_handle() = default; + template + coroutine_handle(coroutine_handle) noexcept; +}; +} + +struct suspend_always { + bool await_ready() noexcept; + void await_suspend(std::experimental::coroutine_handle<>) noexcept; + void await_resume() noexcept; +}; + +struct GroType { + ~GroType(); + operator int() noexcept; +}; + +template <> struct std::experimental::coroutine_traits { + struct promise_type { +GroType get_return_object() noexcept; +suspend_always initial_suspend() noexcept; +suspend_always final_suspend() noexcept; +void return_void() noexcept; +promise_type(); +~promise_type(); +void unhandled_exception() noexcept; + }; +}; + +struct Cleanup { ~Cleanup(); }; +void doSomething() noexcept; + +// CHECK: define i32 @_Z1fv( +int f() { + // CHECK: %[[RetVal:.+]] = alloca i32 + // CHECK: %[[GroActive:.+]] = alloca i1 + + // CHECK: %[[Size:.+]] = call i64 @llvm.coro.size.i64() + // CHECK: call i8* @_Znwm(i64 %[[Size]]) + // CHECK: store i1 false, i1* %[[GroActive]] + // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_typeC1Ev( + // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_type17get_return_objectEv( + // CHECK: store i1 true, i1* %[[GroActive]] + + Cleanup cleanup; + doSomething(); + co_return; + + // CHECK: call void @_Z11doSomethingv( + // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_type11return_voidEv( + // CHECK: call void @_ZN7CleanupD1Ev( + + // Destroy promise and free the memory. + + // CHECK: call void @_ZNSt12experimental16coroutine_traitsIJiEE12promise_typeD1Ev( + // CHECK: %[[Mem:.+]] = call i8* @llvm.coro.free( + // CHECK: call void @_ZdlPv(i8* %[[Mem]]) + + // Initialize retval from Gro and destroy Gro + + // CHECK: %[[Conv:.+]] = call i32 @_ZN7GroTypecviEv( + // CHECK: store i32 %[[Conv]], i32* %[[RetVal]] + // CHECK: %[[IsActive:.+]] = load i1, i1* %[[GroActive]] + // CHECK: br i1 %[[IsActive]], label %[[CleanupGro:.+]], label %[[Done:.+]] + + // CHECK: [[CleanupGro]]: + // CHECK: call void @_ZN7GroTypeD1Ev( + // CHECK: br label %[[Done]] + + // CHECK: [[Done]]: + // CHECK: %[[LoadRet:.+]] = load i32, i32* %[[RetVal]] + // CHECK: ret i32 %[[LoadRet]] +} Index: lib/CodeGen/CGCoroutine.cpp === --- lib/CodeGen/CGCoroutine.cpp +++ lib/CodeGen/CGCoroutine.cpp @@ -11,6 +11,7 @@ // //===--===// +#include "CGCleanup.h" #include "CodeGenFunction.h" #include "llvm/ADT/ScopeExit.h" #include "clang/AST/StmtCXX.h" @@ -326,6 +327,72 @@ }; } +namespace { +struct GetReturnObjectManager { + CodeGenFunction &CGF; + CGBuilderTy &Builder; + const CoroutineBodyStmt &S; + + Address GroActiveFlag; + CodeGenFunction::AutoVarEmission GroEmission; + + GetReturnObjectManager(CodeGenFunction &CGF, const CoroutineBodyStmt &S) + : CGF(CGF), Builder(CGF.Builder), S(S), GroActiveFlag(Address::invalid()), +GroEmission(CodeGenFunction::AutoVarEmission::invalid()) {} + + // The gro variable has to outlive coroutine frame and coroutine promise, but, + // it can only be initialized after coroutine promise was created, thus, we + // split its emission in two parts. EmitGroAlloca emits an alloca and sets up + // cleanups. Later when coroutine promise is available we initialize the gro + // and sets the flag that the cleanup is now active. + + void EmitGroAlloca() { +auto *GroDeclStmt = dyn_cast(S.getResultDecl()); +if (!GroDeclStmt) { + // If get_return_object returns void, no need to do an alloca. + return; +} + +auto *GroVarDecl = cast(GroDeclStmt->getSingleDecl()); + +// Set GRO flag that it is not initialized yet +GroActiveFlag = + CGF.CreateTempAlloca(Builder.getInt1Ty(), CharUnits::One(), "gro.active"); +Builder.CreateStore(Builder.getFalse(), GroActiveFlag); + +GroEmission = CGF.EmitAutoVarAlloca(*GroVarDecl);
[PATCH] D32449: Modifying PthreadLockChecker.cpp to reduce false positives.
malhar1995 added a comment. In https://reviews.llvm.org/D32449#761303, @NoQ wrote: > Thanks, this is great! Two more things: > > - You have touched other code, unrelated to your patch, with clang-format; > we're usually trying to avoid that, because it creates merge conflicts out of > nowhere, and because some of that code actually seems formatted by hand > intentionally. It's better to revert these changes; you can use the `git > clang-format` thing to format only actual changes. I did not apply clang-format to any file except for PthreadLockChecker.cpp. Do you think the merge conflict is due to me not applying clang-format to test/Analysis/pthreadlock.c? The only files I changed were .gitignore, PthreadLockChecker.cpp and test/Analysis/pthreadlock.c. Also, when you asked me to revert the changes, did you mean revert the changes made by clang-format? If yes, how do I do that? I apologize for asking such silly questions. The thing is I'm new to all this and I don't really know how to proceed further. > > > - Updating .gitignore sounds like the right thing to do (llvm's .gitignore > already has this), but i guess we'd better make a separate commit for that. Repository: rL LLVM https://reviews.llvm.org/D32449 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits