[PATCH] D31584: [coroutines] Add support for allocation elision

2017-05-23 Thread Aleksey Shlyapnikov via Phabricator via cfe-commits
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

2017-05-23 Thread Aleksey Shlyapnikov via Phabricator via cfe-commits
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.

2017-05-23 Thread Bruno Cardoso Lopes via cfe-commits
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

2017-05-23 Thread Bruno Cardoso Lopes via cfe-commits
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

2017-05-23 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
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

2017-05-23 Thread marcel via Phabricator via cfe-commits
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

2017-05-23 Thread Saleem Abdulrasool via Phabricator via cfe-commits
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

2017-05-23 Thread Argyrios Kyrtzidis via cfe-commits
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"

2017-05-23 Thread Michael Park via Phabricator via cfe-commits
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

2017-05-23 Thread Dave Lee via Phabricator via cfe-commits
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

2017-05-23 Thread Michael Park via Phabricator via cfe-commits
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.

2017-05-23 Thread Argyrios Kyrtzidis via cfe-commits
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

2017-05-23 Thread Gor Nishanov via Phabricator via cfe-commits
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

2017-05-23 Thread Gor Nishanov via cfe-commits
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

2017-05-23 Thread Gor Nishanov via Phabricator via cfe-commits
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

2017-05-23 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
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

2017-05-23 Thread Eugene Zelenko via Phabricator via cfe-commits
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

2017-05-23 Thread Gor Nishanov via Phabricator via cfe-commits
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

2017-05-23 Thread Gor Nishanov via Phabricator via cfe-commits
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

2017-05-23 Thread Gor Nishanov via cfe-commits
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

2017-05-23 Thread Gor Nishanov via Phabricator via cfe-commits
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

2017-05-23 Thread Gor Nishanov via Phabricator via cfe-commits
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

2017-05-23 Thread Gor Nishanov via Phabricator via cfe-commits
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

2017-05-23 Thread Ronald Wampler via Phabricator via cfe-commits
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

2017-05-23 Thread Gor Nishanov via Phabricator via cfe-commits
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

2017-05-23 Thread Gor Nishanov via Phabricator via cfe-commits
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

2017-05-23 Thread Gor Nishanov via Phabricator via cfe-commits
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

2017-05-23 Thread Gor Nishanov via Phabricator via cfe-commits
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

2017-05-23 Thread Erik Pilkington via Phabricator via cfe-commits
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

2017-05-23 Thread Erik Pilkington via Phabricator via cfe-commits
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

2017-05-23 Thread Erik Pilkington via cfe-commits
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

2017-05-23 Thread Phabricator via Phabricator via cfe-commits
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=

2017-05-23 Thread Dean Michael Berris via cfe-commits
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=

2017-05-23 Thread Dean Michael Berris via Phabricator via cfe-commits
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

2017-05-23 Thread George Burgess IV via Phabricator via cfe-commits
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

2017-05-23 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits
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

2017-05-23 Thread George Burgess IV via Phabricator via cfe-commits
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

2017-05-23 Thread Francois Ferrand via Phabricator via cfe-commits
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

2017-05-23 Thread Francois Ferrand via Phabricator via cfe-commits
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

2017-05-23 Thread Francois Ferrand via Phabricator via cfe-commits
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

2017-05-23 Thread Jennifer Yu via Phabricator via cfe-commits
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).

2017-05-23 Thread Ilya Biryukov via cfe-commits
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).

2017-05-23 Thread Phabricator via Phabricator via cfe-commits
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

2017-05-23 Thread Alexander Richardson via Phabricator via cfe-commits
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.

2017-05-23 Thread Benjamin Kramer via Phabricator via cfe-commits
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

2017-05-23 Thread Francois Ferrand via Phabricator via cfe-commits
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

2017-05-23 Thread Simon Dardis via cfe-commits
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

2017-05-23 Thread Simon Dardis via Phabricator via cfe-commits
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.

2017-05-23 Thread Benjamin Kramer via Phabricator via cfe-commits
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...

2017-05-23 Thread Krasimir Georgiev via Phabricator via cfe-commits
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...

2017-05-23 Thread Krasimir Georgiev via Phabricator via cfe-commits
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.

2017-05-23 Thread Benjamin Kramer via cfe-commits
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

2017-05-23 Thread Krasimir Georgiev via Phabricator via cfe-commits
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

2017-05-23 Thread Manuel Klimek via Phabricator via cfe-commits
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

2017-05-23 Thread Alexander Kornienko via Phabricator via cfe-commits
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.

2017-05-23 Thread Benjamin Kramer via cfe-commits
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

2017-05-23 Thread Sven van Haastregt via cfe-commits
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

2017-05-23 Thread Sven van Haastregt via Phabricator via cfe-commits
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

2017-05-23 Thread Roger Ferrer Ibanez via Phabricator via cfe-commits
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.

2017-05-23 Thread Alexey Bader via Phabricator via cfe-commits
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

2017-05-23 Thread Daniel Jasper via Phabricator via cfe-commits
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

2017-05-23 Thread Daniel Jasper via Phabricator via cfe-commits
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.

2017-05-23 Thread Ilya Biryukov via cfe-commits
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.

2017-05-23 Thread Phabricator via Phabricator via cfe-commits
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

2017-05-23 Thread Krasimir Georgiev via Phabricator via cfe-commits
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

2017-05-23 Thread Francois Ferrand via Phabricator via cfe-commits
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

2017-05-23 Thread Krasimir Georgiev via Phabricator via cfe-commits
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

2017-05-23 Thread Francois Ferrand via Phabricator via cfe-commits
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...

2017-05-23 Thread Ilya Biryukov via Phabricator via cfe-commits
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...

2017-05-23 Thread Ilya Biryukov via Phabricator via cfe-commits
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...

2017-05-23 Thread Ilya Biryukov via Phabricator via cfe-commits
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...

2017-05-23 Thread Krasimir Georgiev via Phabricator via cfe-commits
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

2017-05-23 Thread Piotr Padlewski via Phabricator via cfe-commits
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

2017-05-23 Thread Francois Ferrand via Phabricator via cfe-commits
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

2017-05-23 Thread Francois Ferrand via Phabricator via cfe-commits
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

2017-05-23 Thread Francois Ferrand via Phabricator via cfe-commits
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

2017-05-23 Thread Daniel Jasper via Phabricator via cfe-commits
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

2017-05-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


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

2017-05-23 Thread Daniel Jasper via Phabricator via cfe-commits
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

2017-05-23 Thread Francois Ferrand via Phabricator via cfe-commits
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

2017-05-23 Thread Francois Ferrand via Phabricator via cfe-commits
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

2017-05-23 Thread Francois Ferrand via Phabricator via cfe-commits
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

2017-05-23 Thread Aaron Ballman via Phabricator via cfe-commits
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

2017-05-23 Thread Francois Ferrand via Phabricator via cfe-commits
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

2017-05-23 Thread Piotr Padlewski via Phabricator via cfe-commits
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.

2017-05-23 Thread Yaxun Liu via Phabricator via cfe-commits
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.

2017-05-23 Thread Ilya Biryukov via Phabricator via cfe-commits
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...

2017-05-23 Thread Ilya Biryukov via cfe-commits
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...

2017-05-23 Thread Phabricator via Phabricator via cfe-commits
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.

2017-05-23 Thread Ilya Biryukov via Phabricator via cfe-commits
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

2017-05-23 Thread Krasimir Georgiev via Phabricator via cfe-commits
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

2017-05-23 Thread Alex Lorenz via Phabricator via cfe-commits
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

2017-05-23 Thread Krasimir Georgiev via cfe-commits
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

2017-05-23 Thread Alex Lorenz via Phabricator via cfe-commits
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

2017-05-23 Thread Tom Stellard via Phabricator via cfe-commits
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

2017-05-23 Thread Liu, Yaxun (Sam) via cfe-commits
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

2017-05-23 Thread Ilya Biryukov via cfe-commits
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

2017-05-23 Thread Francois Ferrand via Phabricator via cfe-commits
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

2017-05-23 Thread Gor Nishanov via Phabricator via cfe-commits
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.

2017-05-23 Thread Malhar Thakkar via Phabricator via cfe-commits
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


  1   2   >