[PATCH] D42993: [AArch64] Fixes for ARMv8.2-A FP16 scalar intrinsic

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

Thanks


https://reviews.llvm.org/D42993



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42344: [libc++] Use multi-key tree search for {map, set}::{count, equal_range}

[N via Phabricator via cfe-commits]

ng added a comment.

Could you please commit this on my behalf? I don't have commit access.


https://reviews.llvm.org/D42344



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36802: AMDGPU: Cleanup most of the macros

[Konstantin Zhuravlyov via Phabricator via cfe-commits]

kzhuravl updated this revision to Diff 133569.
kzhuravl added a comment.

Define only canonical names.


https://reviews.llvm.org/D36802

Files:
  lib/Basic/Targets/AMDGPU.cpp
  lib/Basic/Targets/AMDGPU.h
  test/Driver/amdgpu-macros.cl

Index: test/Driver/amdgpu-macros.cl
===
--- test/Driver/amdgpu-macros.cl
+++ test/Driver/amdgpu-macros.cl
@@ -0,0 +1,136 @@
+// Check that appropriate macros are defined for every supported AMDGPU
+// "-target" and "-mcpu" options.
+
+//
+// R600-based processors.
+//
+
+// RUN: %clang -E -dM -target r600 -mcpu=r600 %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,R600 %s
+// RUN: %clang -E -dM -target r600 -mcpu=rv630 %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,R600 %s
+// RUN: %clang -E -dM -target r600 -mcpu=rv635 %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,R600 %s
+// RUN: %clang -E -dM -target r600 -mcpu=r630 %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,R630 %s
+// RUN: %clang -E -dM -target r600 -mcpu=rs780 %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,RS880 %s
+// RUN: %clang -E -dM -target r600 -mcpu=rs880 %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,RS880 %s
+// RUN: %clang -E -dM -target r600 -mcpu=rv610 %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,RS880 %s
+// RUN: %clang -E -dM -target r600 -mcpu=rv620 %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,RS880 %s
+// RUN: %clang -E -dM -target r600 -mcpu=rv670 %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,RV670 %s
+// RUN: %clang -E -dM -target r600 -mcpu=rv710 %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,RV710 %s
+// RUN: %clang -E -dM -target r600 -mcpu=rv730 %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,RV730 %s
+// RUN: %clang -E -dM -target r600 -mcpu=rv740 %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,RV770 %s
+// RUN: %clang -E -dM -target r600 -mcpu=rv770 %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,RV770 %s
+// RUN: %clang -E -dM -target r600 -mcpu=cedar %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,CEDAR %s
+// RUN: %clang -E -dM -target r600 -mcpu=palm %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,CEDAR %s
+// RUN: %clang -E -dM -target r600 -mcpu=cypress %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,CYPRESS %s
+// RUN: %clang -E -dM -target r600 -mcpu=hemlock %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,CYPRESS %s
+// RUN: %clang -E -dM -target r600 -mcpu=juniper %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,JUNIPER %s
+// RUN: %clang -E -dM -target r600 -mcpu=redwood %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,REDWOOD %s
+// RUN: %clang -E -dM -target r600 -mcpu=sumo %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,SUMO %s
+// RUN: %clang -E -dM -target r600 -mcpu=sumo2 %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,SUMO %s
+// RUN: %clang -E -dM -target r600 -mcpu=barts %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,BARTS %s
+// RUN: %clang -E -dM -target r600 -mcpu=caicos %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,CAICOS %s
+// RUN: %clang -E -dM -target r600 -mcpu=turks %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,TURKS %s
+// RUN: %clang -E -dM -target r600 -mcpu=aruba %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,CAYMAN %s
+// RUN: %clang -E -dM -target r600 -mcpu=cayman %s 2>&1 | FileCheck --check-prefixes=ARCH-R600,CAYMAN %s
+
+// ARCH-R600-DAG: #define __AMD__ 1
+// ARCH-R600-DAG: #define __AMDGPU__ 1
+// ARCH-R600-DAG: #define __R600__ 1
+
+// R600:#define __r600 1
+// R600:#define __r600__ 1
+// R630:#define __r630 1
+// R630:#define __r630__ 1
+// RS880:   #define __rs880 1
+// RS880:   #define __rs880__ 1
+// RV670:   #define __rv670 1
+// RV670:   #define __rv670__ 1
+// RV710:   #define __rv710 1
+// RV710:   #define __rv710__ 1
+// RV730:   #define __rv730 1
+// RV730:   #define __rv730__ 1
+// RV770:   #define __rv770 1
+// RV770:   #define __rv770__ 1
+// CEDAR:   #define __cedar 1
+// CEDAR:   #define __cedar__ 1
+// CYPRESS: #define __cypress 1
+// CYPRESS: #define __cypress__ 1
+// JUNIPER: #define __juniper 1
+// JUNIPER: #define __juniper__ 1
+// REDWOOD: #define __redwood 1
+// REDWOOD: #define __redwood__ 1
+// SUMO:#define __sumo 1
+// SUMO:#define __sumo__ 1
+// BARTS:   #define __barts 1
+// BARTS:   #define __barts__ 1
+// CAICOS:  #define __caicos 1
+// CAICOS:  #define __caicos__ 1
+// TURKS:   #define __turks 1
+// TURKS:   #define __turks__ 1
+// CAYMAN:  #define __cayman 1
+// CAYMAN:  #define __cayman__ 1
+
+//
+// AMDGCN-based processors.
+//
+
+// RUN: %clang -E -dM -target amdgcn -mcpu=gfx600 %s 2>&1 | FileCheck --check-prefixes=ARCH-GCN,GFX600 %s
+// RUN: %clang -E -dM -target amdgcn -mcpu=tahiti %s 2>&1 | FileCheck --check-prefixes=ARCH-GCN,GFX600 %s
+// RUN: %clang -E -dM -target amdgcn -mcpu=gfx601 %s 2>&1 | FileCheck --check-prefixes=ARCH-GCN,GFX601 %s
+// RUN: %clang -E -dM -target amdgcn -mcpu=hainan %s 2>&1 | FileCheck --check-prefixes=ARCH-GCN,GFX601 %s
+// RUN: %clang -E -dM -target amdgcn -mcpu=oland %s 2>&1 | FileCheck --check-prefixes=ARC

Re: r324594 - Fix for #31362 - ms_abi is implemented incorrectly for values >=16 bytes.

[Hans Wennborg via cfe-commits]

Merged to 6.0 in r324718.

On Thu, Feb 8, 2018 at 12:15 PM, Alexander Ivchenko via cfe-commits
 wrote:
> Author: aivchenk
> Date: Thu Feb  8 03:15:21 2018
> New Revision: 324594
>
> URL: http://llvm.org/viewvc/llvm-project?rev=324594&view=rev
> Log:
> Fix for #31362 - ms_abi is implemented incorrectly for values >=16 bytes.
>
> Summary:
> This patch is a fix for following issue:
> https://bugs.llvm.org/show_bug.cgi?id=31362 The problem was caused by front 
> end
> lowering C calling conventions without taking into account calling conventions
> enforced by attribute. In this case win64cc was no correctly lowered on 
> targets
> other than Windows.
>
> Reviewed By: rnk (Reid Kleckner)
>
> Differential Revision: https://reviews.llvm.org/D43016
>
> Author: belickim 
>
>
>
> Modified:
> cfe/trunk/lib/CodeGen/TargetInfo.cpp
> cfe/trunk/test/CodeGen/ms_abi.c
>
> Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=324594&r1=324593&r2=324594&view=diff
> ==
> --- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Thu Feb  8 03:15:21 2018
> @@ -3529,7 +3529,17 @@ ABIArgInfo X86_64ABIInfo::classifyRegCal
>
>  void X86_64ABIInfo::computeInfo(CGFunctionInfo &FI) const {
>
> -  bool IsRegCall = FI.getCallingConvention() == 
> llvm::CallingConv::X86_RegCall;
> +  const unsigned CallingConv = FI.getCallingConvention();
> +  // It is possible to force Win64 calling convention on any x86_64 target by
> +  // using __attribute__((ms_abi)). In such case to correctly emit Win64
> +  // compatible code delegate this call to WinX86_64ABIInfo::computeInfo.
> +  if (CallingConv == llvm::CallingConv::Win64) {
> +WinX86_64ABIInfo Win64ABIInfo(CGT);
> +Win64ABIInfo.computeInfo(FI);
> +return;
> +  }
> +
> +  bool IsRegCall = CallingConv == llvm::CallingConv::X86_RegCall;
>
>// Keep track of the number of assigned registers.
>unsigned FreeIntRegs = IsRegCall ? 11 : 6;
>
> Modified: cfe/trunk/test/CodeGen/ms_abi.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms_abi.c?rev=324594&r1=324593&r2=324594&view=diff
> ==
> --- cfe/trunk/test/CodeGen/ms_abi.c (original)
> +++ cfe/trunk/test/CodeGen/ms_abi.c Thu Feb  8 03:15:21 2018
> @@ -146,3 +146,16 @@ void __attribute__((sysv_abi)) f6(__buil
>// WIN64: %[[AP_VAL:.*]] = load i8*, i8** %[[AP]]
>// WIN64-NEXT: store i8* %[[AP_VAL]], i8** %[[AP2:.*]]
>  }
> +
> +// This test checks if structs are passed according to Win64 calling 
> convention
> +// when it's enforced by __attribute((ms_abi)).
> +struct i128 {
> +  unsigned long long a;
> +  unsigned long long b;
> +};
> +
> +__attribute__((ms_abi)) struct i128 f7(struct i128 a) {
> +  // WIN64: define void @f7(%struct.i128* noalias sret %agg.result, 
> %struct.i128* %a)
> +  // FREEBSD: define win64cc void @f7(%struct.i128* noalias sret 
> %agg.result, %struct.i128* %a)
> +  return a;
> +}
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r324537 - PR36055: fix computation of *-dependence in nested initializer lists.

[Hans Wennborg via cfe-commits]

Merged to 6.0 in r324719.

On Wed, Feb 7, 2018 at 11:25 PM, Richard Smith via cfe-commits
 wrote:
> Author: rsmith
> Date: Wed Feb  7 14:25:16 2018
> New Revision: 324537
>
> URL: http://llvm.org/viewvc/llvm-project?rev=324537&view=rev
> Log:
> PR36055: fix computation of *-dependence in nested initializer lists.
>
> When we synthesize an implicit inner initializer list when analyzing an outer
> initializer list, we add it to the outer list immediately, and then fill in 
> the
> inner list. This gives the outer list no chance to update its *-dependence 
> bits
> with those of the completed inner list. To fix this, re-add the inner list to
> the outer list once it's completed.
>
> Note that we do not recompute the *-dependence bits from scratch when we
> complete an outer list; this would give the wrong result for the case where a
> designated initializer overwrites a dependent initializer with a non-dependent
> one. The resulting list in that case should still be dependent, even though 
> all
> traces of the dependence were removed from the semantic form.
>
> Modified:
> cfe/trunk/lib/Sema/SemaInit.cpp
> cfe/trunk/test/SemaCXX/init-expr-crash.cpp
> cfe/trunk/test/SemaTemplate/instantiate-init.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaInit.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=324537&r1=324536&r2=324537&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaInit.cpp Wed Feb  7 14:25:16 2018
> @@ -352,6 +352,7 @@ class InitListChecker {
> bool FillWithNoInit = false);
>void FillInEmptyInitializations(const InitializedEntity &Entity,
>InitListExpr *ILE, bool 
> &RequiresSecondPass,
> +  InitListExpr *OuterILE, unsigned 
> OuterIndex,
>bool FillWithNoInit = false);
>bool CheckFlexibleArrayInit(const InitializedEntity &Entity,
>Expr *InitExpr, FieldDecl *Field,
> @@ -517,12 +518,13 @@ void InitListChecker::FillInEmptyInitFor
>  ILE->setInit(Init, BaseInit.getAs());
>} else if (InitListExpr *InnerILE =
>   dyn_cast(ILE->getInit(Init))) {
> -FillInEmptyInitializations(BaseEntity, InnerILE,
> -   RequiresSecondPass, FillWithNoInit);
> +FillInEmptyInitializations(BaseEntity, InnerILE, RequiresSecondPass,
> +   ILE, Init, FillWithNoInit);
>} else if (DesignatedInitUpdateExpr *InnerDIUE =
> dyn_cast(ILE->getInit(Init))) {
>  FillInEmptyInitializations(BaseEntity, InnerDIUE->getUpdater(),
> -   RequiresSecondPass, /*FillWithNoInit =*/true);
> +   RequiresSecondPass, ILE, Init,
> +   /*FillWithNoInit =*/true);
>}
>  }
>
> @@ -605,24 +607,43 @@ void InitListChecker::FillInEmptyInitFor
>} else if (InitListExpr *InnerILE
> = dyn_cast(ILE->getInit(Init)))
>  FillInEmptyInitializations(MemberEntity, InnerILE,
> -   RequiresSecondPass, FillWithNoInit);
> +   RequiresSecondPass, ILE, Init, 
> FillWithNoInit);
>else if (DesignatedInitUpdateExpr *InnerDIUE
> = dyn_cast(ILE->getInit(Init)))
>  FillInEmptyInitializations(MemberEntity, InnerDIUE->getUpdater(),
> -   RequiresSecondPass, /*FillWithNoInit =*/ 
> true);
> +   RequiresSecondPass, ILE, Init,
> +   /*FillWithNoInit =*/true);
>  }
>
>  /// Recursively replaces NULL values within the given initializer list
>  /// with expressions that perform value-initialization of the
> -/// appropriate type.
> +/// appropriate type, and finish off the InitListExpr formation.
>  void
>  InitListChecker::FillInEmptyInitializations(const InitializedEntity &Entity,
>  InitListExpr *ILE,
>  bool &RequiresSecondPass,
> +InitListExpr *OuterILE,
> +unsigned OuterIndex,
>  bool FillWithNoInit) {
>assert((ILE->getType() != SemaRef.Context.VoidTy) &&
>   "Should not have void type");
>
> +  // If this is a nested initializer list, we might have changed its contents
> +  // (and therefore some of its properties, such as instantiation-dependence)
> +  // while filling it in. Inform the outer initializer list so that its state
> +  // can be updated to match.
> +  // FIXME: We should fully build the inner initializers before constructing
> +  // the outer InitListExpr instead of mutating AST nodes after they have
> +  // been used as subexpressions o

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Typz added reviewers: krasimir, djasper, klimek.

The blocks used to be formatted using the "default" behavior, and would
thus be mistaken for function calls followed by blocks: this could lead
to unexpected inlining of the block and extra line-break before the
opening brace.

They are now formatted similarly to `@autoreleasepool` blocks, as
expected:

  @synchronized(self) {
  f();
  }


Repository:
  rC Clang

https://reviews.llvm.org/D43114

Files:
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -189,6 +189,24 @@
"}\n");
 }
 
+TEST_F(FormatTestObjC, FormatObjCSynchronized) {
+  verifyFormat("@synchronized(self) {\n"
+   "  f();\n"
+   "}\n"
+   "@synchronized(self) {\n"
+   "  f();\n"
+   "}\n");
+  Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+  verifyFormat("@synchronized(self)\n"
+   "{\n"
+   "  f();\n"
+   "}\n"
+   "@synchronized(self)\n"
+   "{\n"
+   "  f();\n"
+   "}\n");
+}
+
 TEST_F(FormatTestObjC, FormatObjCInterface) {
   verifyFormat("@interface Foo : NSObject  {\n"
"@public\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1121,6 +1121,18 @@
 }
 addUnwrappedLine();
 return;
+  case tok::objc_synchronized:
+nextToken();
+if (FormatTok->Tok.is(tok::l_paren))
+   // Skip synchronization object
+   parseParens();
+if (FormatTok->Tok.is(tok::l_brace)) {
+  if (Style.BraceWrapping.AfterObjCDeclaration)
+addUnwrappedLine();
+  parseBlock(/*MustBeDeclaration=*/false);
+}
+addUnwrappedLine();
+return;
   case tok::objc_try:
 // This branch isn't strictly necessary (the kw_try case below would
 // do this too after the tok::at is parsed above).  But be explicit.


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -189,6 +189,24 @@
"}\n");
 }
 
+TEST_F(FormatTestObjC, FormatObjCSynchronized) {
+  verifyFormat("@synchronized(self) {\n"
+   "  f();\n"
+   "}\n"
+   "@synchronized(self) {\n"
+   "  f();\n"
+   "}\n");
+  Style.BreakBeforeBraces = FormatStyle::BS_Allman;
+  verifyFormat("@synchronized(self)\n"
+   "{\n"
+   "  f();\n"
+   "}\n"
+   "@synchronized(self)\n"
+   "{\n"
+   "  f();\n"
+   "}\n");
+}
+
 TEST_F(FormatTestObjC, FormatObjCInterface) {
   verifyFormat("@interface Foo : NSObject  {\n"
"@public\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1121,6 +1121,18 @@
 }
 addUnwrappedLine();
 return;
+  case tok::objc_synchronized:
+nextToken();
+if (FormatTok->Tok.is(tok::l_paren))
+   // Skip synchronization object
+   parseParens();
+if (FormatTok->Tok.is(tok::l_brace)) {
+  if (Style.BraceWrapping.AfterObjCDeclaration)
+addUnwrappedLine();
+  parseBlock(/*MustBeDeclaration=*/false);
+}
+addUnwrappedLine();
+return;
   case tok::objc_try:
 // This branch isn't strictly necessary (the kw_try case below would
 // do this too after the tok::at is parsed above).  But be explicit.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42942: [clangd] Collect definitions when indexing.

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

LGTM.




Comment at: clangd/index/Index.h:129
   //
-  // A C++ symbol could have multiple declarations and one definition (e.g.
-  // a function is declared in ".h" file, and is defined in ".cc" file).
-  //   * For classes, the canonical declaration is usually definition.
-  //   * For non-inline functions, the canonical declaration is a declaration
-  // (not a definition), which is usually declared in ".h" file.
+  // A C++ symbol may have mulitple declarations, and we pick one to prefer.
+  //   * For classes, the canonical declaration should be the definition.

a typo here? `multiple`?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42942



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: lib/Format/UnwrappedLineParser.cpp:1130
+if (FormatTok->Tok.is(tok::l_brace)) {
+  if (Style.BraceWrapping.AfterObjCDeclaration)
+addUnwrappedLine();

Wondering if formatting with this style is appropriate: the clang-format doc 
points in that direction, but it seems to me both `@synchronized` and 
`@autoreleasepool` are more akin to "control statements".

For consistency (esp. in ObjC++ code), as a user, I would tend to have these 
blocks indented like control statements while interfaces/implementations blocks 
would be indented like classes/structs.

So maybe it would be better to introduce a new BraceWrapping style 
'AfterObjCSpecialBlock` to control these cases, matching the possibilities that 
are given for control statements vs classes. What do you think?


Repository:
  rC Clang

https://reviews.llvm.org/D43114



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r324721 - Fif for an issue when Clang permits assignment to vector/extvector elements in a const method.

[Andrew V. Tischenko via cfe-commits]

Author: avt77
Date: Fri Feb  9 01:30:42 2018
New Revision: 324721

URL: http://llvm.org/viewvc/llvm-project?rev=324721&view=rev
Log:
Fif for an issue when Clang permits assignment to vector/extvector elements in 
a const method.

Modified:
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprMember.cpp
cfe/trunk/test/Sema/assign.c
cfe/trunk/test/Sema/typedef-retain.c
cfe/trunk/test/SemaCXX/err_typecheck_assign_const.cpp

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=324721&r1=324720&r2=324721&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Feb  9 01:30:42 2018
@@ -4395,8 +4395,13 @@ Sema::CreateBuiltinArraySubscriptExpr(Ex
 if (VK != VK_RValue)
   OK = OK_VectorComponent;
 
-// FIXME: need to deal with const...
 ResultType = VTy->getElementType();
+QualType BaseType = BaseExpr->getType();
+Qualifiers BaseQuals = BaseType.getQualifiers();
+Qualifiers MemberQuals = ResultType.getQualifiers();
+Qualifiers Combined = BaseQuals + MemberQuals;
+if (Combined != MemberQuals)
+  ResultType = Context.getQualifiedType(ResultType, Combined);
   } else if (LHSTy->isArrayType()) {
 // If we see an array that wasn't promoted by
 // DefaultFunctionArrayLvalueConversion, it must be an array that
@@ -10434,8 +10439,16 @@ static void DiagnoseConstAssignment(Sema
 // Static fields do not inherit constness from parents.
 break;
   }
-  break;
-} // End MemberExpr
+  break; // End MemberExpr
+} else if (const ArraySubscriptExpr *ASE =
+   dyn_cast(E)) {
+  E = ASE->getBase()->IgnoreParenImpCasts();
+  continue;
+} else if (const ExtVectorElementExpr *EVE =
+   dyn_cast(E)) {
+  E = EVE->getBase()->IgnoreParenImpCasts();
+  continue;
+}
 break;
   }
 

Modified: cfe/trunk/lib/Sema/SemaExprMember.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprMember.cpp?rev=324721&r1=324720&r2=324721&view=diff
==
--- cfe/trunk/lib/Sema/SemaExprMember.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprMember.cpp Fri Feb  9 01:30:42 2018
@@ -1623,10 +1623,14 @@ static ExprResult LookupMemberExpr(Sema
   else
 VK = BaseExpr.get()->getValueKind();
 }
+
 QualType ret = CheckExtVectorComponent(S, BaseType, VK, OpLoc,
Member, MemberLoc);
 if (ret.isNull())
   return ExprError();
+Qualifiers BaseQ =
+S.Context.getCanonicalType(BaseExpr.get()->getType()).getQualifiers();
+ret = S.Context.getQualifiedType(ret, BaseQ);
 
 return new (S.Context)
 ExtVectorElementExpr(ret, VK, BaseExpr.get(), *Member, MemberLoc);

Modified: cfe/trunk/test/Sema/assign.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/assign.c?rev=324721&r1=324720&r2=324721&view=diff
==
--- cfe/trunk/test/Sema/assign.c (original)
+++ cfe/trunk/test/Sema/assign.c Fri Feb  9 01:30:42 2018
@@ -11,10 +11,10 @@ void test2 (const struct {int a;} *x) {
 
 typedef int arr[10];
 void test3() {
-  const arr b;
-  const int b2[10]; 
-  b[4] = 1; // expected-error {{read-only variable is not assignable}}
-  b2[4] = 1; // expected-error {{read-only variable is not assignable}}
+  const arr b;  // expected-note {{variable 'b' declared const here}}
+  const int b2[10]; // expected-note {{variable 'b2' declared const here}}
+  b[4] = 1; // expected-error {{cannot assign to variable 'b' with 
const-qualified type 'const arr' (aka 'int const[10]')}}
+  b2[4] = 1;// expected-error {{cannot assign to variable 'b2' with 
const-qualified type 'const int [10]'}}
 }
 
 typedef struct I {

Modified: cfe/trunk/test/Sema/typedef-retain.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/typedef-retain.c?rev=324721&r1=324720&r2=324721&view=diff
==
--- cfe/trunk/test/Sema/typedef-retain.c (original)
+++ cfe/trunk/test/Sema/typedef-retain.c Fri Feb  9 01:30:42 2018
@@ -16,8 +16,8 @@ void test2(float4 a, int4p result, int i
 typedef int a[5];
 void test3() {
   typedef const a b;
-  b r;
-  r[0]=10;  // expected-error {{read-only variable is not assignable}}
+  b r;   // expected-note {{variable 'r' declared const here}}
+  r[0] = 10; // expected-error {{cannot assign to variable 'r' with 
const-qualified type 'b' (aka 'int const[5]')}}
 }
 
 int test4(const a y) {

Modified: cfe/trunk/test/SemaCXX/err_typecheck_assign_const.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/err_typecheck_assign_const.cpp?rev=324721&r1=324720&r2=324721&view=diff
=

[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 added a comment.

Committed revision 324721.
BTW, could you review https://reviews.llvm.org/D42728: it's rather similar to 
this one and rather small as well.


https://reviews.llvm.org/D42530



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42895: [libclang] Add `CXSymbolRole role` to CXIdxEntityRefInfo

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: include/clang-c/Index.h:6159
+   */
+  CXSymbolRole role;
 } CXIdxEntityRefInfo;

MaskRay wrote:
> ilya-biryukov wrote:
> > MaskRay wrote:
> > > ilya-biryukov wrote:
> > > > Why do we need to store both `CXIdxEntityRefKind` and `CXSymbolRole`? 
> > > > Can we store just `CXSymbolRole`?
> > > > Is this for compatibility with existing clients?
> > > > 
> > > > If so, maybe we could:
> > > > - remove `Implicit` and `Direct` from the `CXSymbolRole`
> > > > - keep it only in `CXIdxEntityRefKind`
> > > > - not deprecate the `CXIdxEntityRefKind`
> > > I think `CXIdxEntityRefKind` should be deprecated but for compatibility 
> > > we do not remove the field for this minor version upgrade. But they can 
> > > be removed after a major version upgrade.
> > > 
> > > For what I have observed, `Implicit` is only used by Objective-C
> > We should definitely loop the owners of libclang in if we want to deprecate 
> > the API, I'm not familiar with the contract of libclang regarding the API 
> > deprecation.
> > I'm not sure who's responsible for that, asking at the cfe-dev mailing list 
> > could be your best bet for that.
> > 
> > The alternative I suggest is removing `Implicit` from `CXSymbolRole`, 
> > making this change an extension of existing API. I expect that this could 
> > be done without more through discussion.
> I'll keep `CXSymbolRole_Implicit` and make it duplicate the functionality 
> provided by CXIdxEntityRefKind, with a comment that CXIdxEntityRefKind may be 
> deprecated in a future version.
> 
> Thanks, I'll also ask the cfe-dev mailing list.
LG, thanks.



Comment at: tools/c-index-test/c-index-test.c:3545
   printCXIndexContainer(info->container);
-  printf(" | refkind: ");
-  switch (info->kind) {

Maybe keep this debug-printing of refkind? We're not removing it in this patch.


Repository:
  rC Clang

https://reviews.llvm.org/D42895



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42942: [clangd] Collect definitions when indexing.

[Eric Liu via Phabricator via cfe-commits]

ioeric accepted this revision.
ioeric added a comment.

Lg




Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:67
+  // XXX this is just to make running the tool fast during dev!
+  bool BeginInvocation(CompilerInstance &CI) override {
+const auto &Inputs = CI.getInvocation().getFrontendOpts().Inputs;

hokein wrote:
> ioeric wrote:
> > sammccall wrote:
> > > hokein wrote:
> > > > It is fine for debugging, but I think we don't want this behavior by 
> > > > default.
> > > > 
> > > > global-symbol-builder also supports running a single TU 
> > > > (`global-symbol-builder /path/to/file`), which is sufficient for 
> > > > debugging, I think?
> > > > 
> > > Yeah, I'm not going to check this in, thus the XXX comment :-)
> > > 
> > > Single TU isn't enough - it doesn't test the reducer. On the other hand 
> > > the full compilation database is too big. So this option would actually 
> > > be useful! But it doesn't belong here.
> > Drive-by: you could also run the tool in the default standalone mode and 
> > provide a list of files. 
> However the default standalone mode is not multiple thread :(.
You are right :) But I don't think this matters much if you only want to test 
reduction.



Comment at: clangd/index/SymbolCollector.cpp:210
+  BasicSymbol = addDeclaration(*ND, std::move(ID));
+if (Roles & static_cast(index::SymbolRole::Definition))
+  addDefinition(*cast(ASTNode.OrigD), *BasicSymbol);

sammccall wrote:
> hokein wrote:
> > ioeric wrote:
> > > It seems that we store definition even if it's the same as declaration, 
> > > which might be true for most classes? This is probably fine, but it would 
> > > be worth documenting. Or maybe consider not storing the same location 
> > > twice?
> > I think it is fine to store the same location twice. We can't tell whether 
> > the CanonicalLoc is a definition or not.
> Documented that these may be the same.
> We wouldn't actually save any memory by avoiding saving this twice - the 
> filename is a stringref to the same data, and the offsets get stored even if 
> they're null.
I'm less concerned about space. I think this would also make it easy to check 
whether a symbol has a separate definition (e.g. we might want to do this for 
go-to-definition/go-to-declaration). A comparison would also work but seems 
less natural. But I'm not sure, up tp you :)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42942



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


Repository:
  rC Clang

https://reviews.llvm.org/D42684



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42787: clang-format: do not add extra indent when wrapping last parameter

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

In https://reviews.llvm.org/D42787#1000687, @krasimir wrote:

> We could adapt the single-argument version instead, turning:
>
>   foo(bb +
>   c);
>
>
> into:
>
>   foo(bb +
>   c);
>


We could indeed, but that would probably make neither djasper or me happy :-)


Repository:
  rC Clang

https://reviews.llvm.org/D42787



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D5767: Template Instantiation Observer + a few other templight-related changes

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: lib/FrontendTool/ExecuteCompilerInvocation.cpp:261
+
+} //namespace

Nit: this should be `// namespace clang`


https://reviews.llvm.org/D5767



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r324725 - [clangd] Remove threading-related code from ClangdUnit.h

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Fri Feb  9 02:17:23 2018
New Revision: 324725

URL: http://llvm.org/viewvc/llvm-project?rev=324725&view=rev
Log:
[clangd] Remove threading-related code from ClangdUnit.h

Reviewers: sammccall, hokein, ioeric

Reviewed By: sammccall

Subscribers: klimek, jkorous-apple, cfe-commits

Differential Revision: https://reviews.llvm.org/D43065

Modified:
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/TUScheduler.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=324725&r1=324724&r2=324725&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Fri Feb  9 02:17:23 2018
@@ -25,6 +25,7 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #include 
+#include 
 #include 
 #include 
 #include 

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=324725&r1=324724&r2=324725&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Fri Feb  9 02:17:23 2018
@@ -29,7 +29,6 @@
 #include "llvm/Support/Format.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
-#include 
 
 using namespace clang::clangd;
 using namespace clang;
@@ -234,10 +233,6 @@ private:
   llvm::Optional LangOpts;
 };
 
-template  bool futureIsReady(std::shared_future const &Future) {
-  return Future.wait_for(std::chrono::seconds(0)) == std::future_status::ready;
-}
-
 } // namespace
 
 void clangd::dumpAST(ParsedAST &AST, llvm::raw_ostream &OS) {
@@ -250,7 +245,6 @@ ParsedAST::Build(std::unique_ptr Buffer,
  std::shared_ptr PCHs,
  IntrusiveRefCntPtr VFS) {
-
   std::vector ASTDiags;
   StoreDiagsConsumer UnitDiagsConsumer(/*ref*/ ASTDiags);
 
@@ -380,344 +374,145 @@ ParsedAST::ParsedAST(std::shared_ptrAction);
 }
 
-ParsedASTWrapper::ParsedASTWrapper(ParsedASTWrapper &&Wrapper)
-: AST(std::move(Wrapper.AST)) {}
-
-ParsedASTWrapper::ParsedASTWrapper(llvm::Optional AST)
-: AST(std::move(AST)) {}
-
-std::shared_ptr
-CppFile::Create(PathRef FileName, bool StorePreamblesInMemory,
-std::shared_ptr PCHs,
-ASTParsedCallback ASTCallback) {
-  return std::shared_ptr(new CppFile(FileName, StorePreamblesInMemory,
-  std::move(PCHs),
-  std::move(ASTCallback)));
-}
-
 CppFile::CppFile(PathRef FileName, bool StorePreamblesInMemory,
  std::shared_ptr PCHs,
  ASTParsedCallback ASTCallback)
 : FileName(FileName), StorePreamblesInMemory(StorePreamblesInMemory),
-  RebuildCounter(0), RebuildInProgress(false), ASTMemUsage(0),
-  PreambleMemUsage(0), PCHs(std::move(PCHs)),
-  ASTCallback(std::move(ASTCallback)) {
+  PCHs(std::move(PCHs)), ASTCallback(std::move(ASTCallback)) {
   log("Created CppFile for " + FileName);
-
-  std::lock_guard Lock(Mutex);
-  LatestAvailablePreamble = nullptr;
-  PreamblePromise.set_value(nullptr);
-  PreambleFuture = PreamblePromise.get_future();
-
-  ASTPromise.set_value(std::make_shared(llvm::None));
-  ASTFuture = ASTPromise.get_future();
-}
-
-void CppFile::cancelRebuild() { deferCancelRebuild()(); }
-
-UniqueFunction CppFile::deferCancelRebuild() {
-  std::unique_lock Lock(Mutex);
-  // Cancel an ongoing rebuild, if any, and wait for it to finish.
-  unsigned RequestRebuildCounter = ++this->RebuildCounter;
-  // Rebuild asserts that futures aren't ready if rebuild is cancelled.
-  // We want to keep this invariant.
-  if (futureIsReady(PreambleFuture)) {
-PreamblePromise = std::promise>();
-PreambleFuture = PreamblePromise.get_future();
-  }
-  if (futureIsReady(ASTFuture)) {
-ASTPromise = std::promise>();
-ASTFuture = ASTPromise.get_future();
-  }
-
-  Lock.unlock();
-  // Notify about changes to RebuildCounter.
-  RebuildCond.notify_all();
-
-  std::shared_ptr That = shared_from_this();
-  return [That, RequestRebuildCounter]() {
-std::unique_lock Lock(That->Mutex);
-CppFile *This = &*That;
-This->RebuildCond.wait(Lock, [This, RequestRebuildCounter]() {
-  return !This->RebuildInProgress ||
- This->RebuildCounter != RequestRebuildCounter;
-});
-
-// This computation got cancelled itself, do nothing.
-if (This->RebuildCounter != RequestRebuildCounter)
-  return;
-
-// Set empty results for Promises.
-That->PreambleMemUsage = 0;
-That->PreamblePromise.set_value(nullptr);
-That->ASTMemUsage = 0;
-That->ASTPr

[PATCH] D43065: [clangd] Remove threading-related code from ClangdUnit.h

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL324725: [clangd] Remove threading-related code from 
ClangdUnit.h (authored by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D43065

Files:
  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/TUScheduler.cpp

Index: clang-tools-extra/trunk/clangd/ClangdUnit.h
===
--- clang-tools-extra/trunk/clangd/ClangdUnit.h
+++ clang-tools-extra/trunk/clangd/ClangdUnit.h
@@ -18,10 +18,9 @@
 #include "clang/Serialization/ASTBitCodes.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Core/Replacement.h"
-#include 
-#include 
 #include 
-#include 
+#include 
+#include 
 
 namespace llvm {
 class raw_ostream;
@@ -126,151 +125,44 @@
   bool PreambleDeclsDeserialized;
 };
 
-// Provides thread-safe access to ParsedAST.
-class ParsedASTWrapper {
-public:
-  ParsedASTWrapper(ParsedASTWrapper &&Wrapper);
-  ParsedASTWrapper(llvm::Optional AST);
-
-  /// Runs \p F on wrapped ParsedAST under lock. Ensures it is not accessed
-  /// concurrently.
-  template  void runUnderLock(Func F) const {
-std::lock_guard Lock(Mutex);
-F(AST ? AST.getPointer() : nullptr);
-  }
-
-private:
-  // This wrapper is used as an argument to std::shared_future (and it returns a
-  // const ref in get()), but we need to have non-const ref in order to
-  // implement some features.
-  mutable std::mutex Mutex;
-  mutable llvm::Optional AST;
-};
-
 using ASTParsedCallback = std::function;
 
 /// Manages resources, required by clangd. Allows to rebuild file with new
 /// contents, and provides AST and Preamble for it.
-/// NOTE: Threading-related bits of CppFile are now deprecated and will be
-/// removed soon.
-class CppFile : public std::enable_shared_from_this {
+class CppFile {
 public:
-  // We only allow to create CppFile as shared_ptr, because a future returned by
-  // deferRebuild will hold references to it.
-  static std::shared_ptr
-  Create(PathRef FileName, bool StorePreamblesInMemory,
- std::shared_ptr PCHs,
- ASTParsedCallback ASTCallback);
-
-private:
   CppFile(PathRef FileName, bool StorePreamblesInMemory,
   std::shared_ptr PCHs,
   ASTParsedCallback ASTCallback);
 
-public:
-  CppFile(CppFile const &) = delete;
-  CppFile(CppFile &&) = delete;
-
-  /// Cancels a scheduled rebuild, if any, and sets AST and Preamble to nulls.
-  /// If a rebuild is in progress, will wait for it to finish.
-  void cancelRebuild();
-
-  /// Similar to deferRebuild, but sets both Preamble and AST to nulls instead
-  /// of doing an actual parsing. Returned function is a deferred computation
-  /// that will wait for any ongoing rebuilds to finish and actually set the AST
-  /// and Preamble to nulls. It can be run on a different thread. This function
-  /// is useful to cancel ongoing rebuilds, if any, before removing CppFile.
-  /// DEPRECATED. This function will be removed soon, please do not use it.
-  UniqueFunction deferCancelRebuild();
-
-  /// Rebuild AST and Preamble synchronously on the calling thread.
-  /// Returns a list of diagnostics or a llvm::None, if another rebuild was
-  /// requested in parallel (effectively cancelling this rebuild) before
-  /// diagnostics were produced.
+  /// Rebuild the AST and the preamble.
+  /// Returns a list of diagnostics or llvm::None, if an error occured.
   llvm::Optional> rebuild(ParseInputs &&Inputs);
-
-  /// Schedule a rebuild and return a deferred computation that will finish the
-  /// rebuild, that can be called on a different thread.
-  /// After calling this method, resources, available via futures returned by
-  /// getPreamble() and getAST(), will be waiting for rebuild to finish. A
-  /// continuation fininshing rebuild, returned by this function, must be
-  /// computed(i.e., operator() must be called on it) in order to make those
-  /// resources ready. If deferRebuild is called again before the rebuild is
-  /// finished (either because returned future had not been called or because it
-  /// had not returned yet), the previous rebuild request is cancelled and the
-  /// resource futures (returned by getPreamble() or getAST()) that were not
-  /// ready will be waiting for the last rebuild to finish instead.
-  /// The future to finish rebuild returns a list of diagnostics built during
-  /// reparse, or None, if another deferRebuild was called before this
-  /// rebuild was finished.
-  /// DEPRECATED. This function will be removed soon, please do not use it.
-  UniqueFunction>()>
-  deferRebuild(ParseInputs &&Inputs);
-
-  /// Returns a future to get the most fresh PreambleData for a file. The
-  /// future will wait until the Preamble is rebuilt.
-  std::shared_future> getPreamble() c

[PATCH] D5767: Template Instantiation Observer + a few other templight-related changes

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: lib/Sema/SemaTemplateInstantiate.cpp:646
   }
+}
+

Use either `LLVM_FALLTHROUGH;` here or break to avoid compiler warnings.


https://reviews.llvm.org/D5767



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43012: [ASTImporter] Fix lexical DC for templated decls; support VarTemplatePartialSpecDecl

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: martong.

Looks good to me.  Only found a few nits.




Comment at: lib/AST/ASTImporter.cpp:4296
   // Create the declaration that is being templated.
-  SourceLocation StartLoc = Importer.Import(DTemplated->getLocStart());
-  SourceLocation IdLoc = Importer.Import(DTemplated->getLocation());
-  TypeSourceInfo *TInfo = Importer.Import(DTemplated->getTypeSourceInfo());
-  VarDecl *D2Templated = VarDecl::Create(Importer.getToContext(), DC, StartLoc,
- IdLoc, Name.getAsIdentifierInfo(), T,
- TInfo, DTemplated->getStorageClass());
-  D2Templated->setAccess(DTemplated->getAccess());
-  
D2Templated->setQualifierInfo(Importer.Import(DTemplated->getQualifierLoc()));
-  D2Templated->setLexicalDeclContext(LexicalDC);
-
-  // Importer.Imported(DTemplated, D2Templated);
-  // LexicalDC->addDeclInternal(D2Templated);
-
-  // Merge the initializer.
-  if (ImportDefinition(DTemplated, D2Templated))
+  VarDecl *ToTemplated = 
dyn_cast_or_null(Importer.Import(DTemplated));
+  if (!ToTemplated)

`auto *` to not repeat type.



Comment at: lib/AST/ASTImporter.cpp:4399
+if (ImportTemplateArgumentListInfo(D->getTemplateArgsInfo(),
+  ToTAInfo))
+  return nullptr;

The formatting might be a bit off here.


Repository:
  rC Clang

https://reviews.llvm.org/D43012



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43075: [clang-move] Don't dump macro symbols.

[Eric Liu via Phabricator via cfe-commits]

ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.

Ooops, forgot to stamp!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43075



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus created this revision.
Szelethus added reviewers: cfe-commits, xazax.hun.
Szelethus added a project: clang-tools-extra.
Herald added subscribers: hintonda, rnkovacs, mgorny.

New checker called misc-throw-keyword-missing warns about cases where a 
temporary object's type is (likely) an exception but is not thrown. This is 
done by checking whether the type's name (or one of its baseclass' name) 
contains the substring "EXCEPTION", "Exception" or "exception".

  void f(int i){
  if(i = 0)
  // Exception is created but not thrown.
  std::runtime_error("Wrong argument");
  }


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43120

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/ThrowKeywordMissingCheck.cpp
  clang-tidy/misc/ThrowKeywordMissingCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-throw-keyword-missing.rst
  test/clang-tidy/misc-throw-keyword-missing.cpp

Index: test/clang-tidy/misc-throw-keyword-missing.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-throw-keyword-missing.cpp
@@ -0,0 +1,152 @@
+// RUN: %check_clang_tidy %s misc-throw-keyword-missing %t
+
+namespace std {
+
+// std::string declaration (taken from test/clang-tidy/readability-redundant-string-cstr-msvc.cpp)
+template 
+class allocator {};
+template 
+class char_traits {};
+template , typename A = std::allocator>
+struct basic_string {
+  basic_string();
+  basic_string(const basic_string&);
+  // MSVC headers define two constructors instead of using optional arguments.
+  basic_string(const C *);
+  basic_string(const C *, const A &);
+  ~basic_string();
+};
+typedef basic_string string;
+typedef basic_string wstring;
+
+// std::exception and std::runtime_error declaration
+struct exception{
+  exception();
+  exception( const exception& other);
+  virtual ~exception();
+};
+
+struct runtime_error : public exception{
+  explicit runtime_error( const std::string& what_arg );
+};
+
+} //namespace std
+
+// The usage of this class should never emit a warning
+struct RegularClass {};
+
+// Classname contains the substring "exception", in certain cases using this class should emit a warning
+struct RegularException {
+  RegularException() {}
+
+  // Constructors with a single argument are treated differently (cxxFunctionalCastExpr) 
+  RegularException(int) {}
+};
+
+// --
+
+void stdExceptionNotTrown(int i) {
+  if (i < 0)
+//CHECK-MESSAGES: :[[@LINE+1]]:5: warning: exception created but is not thrown [misc-throw-keyword-missing]
+std::exception();
+
+  if (i > 0)
+//CHECK-MESSAGES: :[[@LINE+1]]:5: warning: exception created but is not thrown [misc-throw-keyword-missing]
+std::runtime_error("Unexpected argument");
+
+}
+
+void stdExceptionThrown(int i) {
+  if (i < 0)
+throw std::exception();
+
+  if (i > 0)
+throw std::runtime_error("Unexpected argument");
+}
+
+void regularClassNotThrown(int i) {
+  if (i < 0)
+RegularClass();
+}
+
+void regularClassThrown(int i) {
+  if (i < 0)
+throw RegularClass();
+}
+
+void nameContainsExceptionNotThrown(int i) {
+  if (i < 0)
+//CHECK-MESSAGES: :[[@LINE+1]]:5: warning: exception created but is not thrown [misc-throw-keyword-missing]
+RegularException();
+
+  if (i > 0)
+//CHECK-MESSAGES: :[[@LINE+1]]:5: warning: exception created but is not thrown [misc-throw-keyword-missing]
+RegularException(5);
+}
+
+void nameContainsExceptionThrown(int i) {
+  if (i < 0)
+throw RegularException();
+
+  if (i > 0)
+throw RegularException(5);
+}
+
+
+template 
+void f(int i, Exception excToBeThrown){}
+
+void funcCallWithTempExcTest(){
+  f(5, RegularException());
+}
+
+// Global variable initilization test
+RegularException exc = RegularException();
+RegularException* excptr = new RegularException();
+
+void localVariableInitTest(){
+  RegularException exc = RegularException();
+  RegularException* excptr = new RegularException();
+}
+
+class CtorInitializerListTest{
+  RegularException exc;
+
+  CtorInitializerListTest() : exc(RegularException()) {}
+
+  CtorInitializerListTest(int) 
+try : exc(RegularException()) {
+  //Constructor
+}
+catch(...){
+  //CHECK-MESSAGES: :[[@LINE+1]]:7: warning: exception created but is not thrown [misc-throw-keyword-missing]
+  RegularException();
+}
+
+  CtorInitializerListTest(float);
+};
+
+CtorInitializerListTest::CtorInitializerListTest(float) 
+  try : exc(RegularException()) {
+//Constructor
+  }
+  catch(...){
+//CHECK-MESSAGES: :[[@LINE+1]]:5: warning: exception created but is not thrown [misc-throw-keyword-missing]
+RegularException();
+  }
+
+RegularException funcReturningExceptioniTest(int i){
+  return RegularException();
+}
+
+struct ClassBracedInitListTest{
+  ClassBracedInitListTest(RegularException exc) {}
+};
+
+void functionBracedInitListTest(RegularExce

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

[Francois Ferrand via Phabricator via cfe-commits]

Typz created this revision.
Typz added reviewers: krasimir, djasper, klimek.

When the target object expression is short and the first selector name is
long, clang-format used to break the colon alignment:

  [I performSelectorOnMainThread:@selector(loadAccessories)
   withObject:nil
waitUntilDone:false];

This happens because the colon is placed at `ContinuationIndent +
LongestObjCSelectorName`, so that any selector can be wrapped. This is
however not needed in case the longest selector is the first one, and not
wrapped.

To overcome this, this patch does not include the first selector in
`LongestObjCSelectorName` computation (in TokenAnnotator), and lets
ContinuationIndenter account decide how to account for the first selector
when wrapping. (Note this was already partly the case, see line 521 of
ContinuationIndenter.cpp)

This way, the code gets properly aligned whenever possible without
breaking the continuation indent.

  [I performSelectorOnMainThread:@selector(loadAccessories)
  withObject:nil
   waitUntilDone:false];
  [I // force break
  performSelectorOnMainThread:@selector(loadAccessories)
   withObject:nil
waitUntilDone:false];
  [I perform:@selector(loadAccessories)
  withSelectorOnMainThread:true
 waitUntilDone:false];


Repository:
  rC Clang

https://reviews.llvm.org/D43121

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -652,8 +652,8 @@
   // Formats pair-parameters.
   verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
   verifyFormat("[I drawRectOn:surface //\n"
-   "ofSize:aa:bbb\n"
-   "  atOrigin:cc:dd];");
+   "   ofSize:aa:bbb\n"
+   " atOrigin:cc:dd];");
 
   Style.ColumnLimit = 70;
   verifyFormat(
@@ -686,7 +686,19 @@
   "  backing:NSBackingStoreBuffered\n"
   "defer:NO]);\n"
   "}");
-}
+
+  // Respect continuation indent and colon alignment (e.g. when object name is
+  // short, and first selector is the longest one)
+  Style = getLLVMStyle();
+  Style.Language = FormatStyle::LK_ObjC;
+  Style.ContinuationIndentWidth = 8;
+  verifyFormat("[self performSelectorOnMainThread:@selector(loadAccessories)\n"
+   "   withObject:nil\n"
+   "waitUntilDone:false];");
+  verifyFormat("[self performSelector:@selector(loadAccessories)\n"
+   "withObjectOnMainThread:nil\n"
+   " waitUntilDone:false];");
+  }
 
 TEST_F(FormatTestObjC, ObjCAt) {
   verifyFormat("@autoreleasepool");
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -571,12 +571,12 @@
 BeforePrevious->is(tok::r_square) ||
 Contexts.back().LongestObjCSelectorName == 0) {
   Tok->Previous->Type = TT_SelectorName;
-  if (Tok->Previous->ColumnWidth >
-  Contexts.back().LongestObjCSelectorName)
-Contexts.back().LongestObjCSelectorName =
-Tok->Previous->ColumnWidth;
   if (!Contexts.back().FirstObjCSelectorName)
 Contexts.back().FirstObjCSelectorName = Tok->Previous;
+  else if (Tok->Previous->ColumnWidth >
+   Contexts.back().LongestObjCSelectorName)
+Contexts.back().LongestObjCSelectorName =
+Tok->Previous->ColumnWidth;
 }
   } else if (Contexts.back().ColonIsForRangeExpr) {
 Tok->Type = TT_RangeBasedForLoopColon;
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -696,7 +696,8 @@
  ? std::max(State.Stack.back().Indent,
 State.FirstIndent + Style.ContinuationIndentWidth)
  : State.Stack.back().Indent) +
-NextNonComment->LongestObjCSelectorName;
+std::max(NextNonComment->LongestObjCSelectorName,
+ unsigned(NextNonComment->TokenText.size()));
   }
 } else if (State.Stack.back().AlignColons &&
State.Stack.back().ColonPos <= NextNonComment->ColumnWidth) {
@@ -895,7 +896,8 @@
   ? std::max(State.Stack.back().Indent,
  State.FirstIndent + Style.ContinuationIndentWidth)
   : State.Stack.back().Indent) +
- NextNonComment->LongestObjCSelectorName -
+ std::max(Next

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 133589.
Typz added a comment.

fix type in commit message


Repository:
  rC Clang

https://reviews.llvm.org/D43121

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -652,8 +652,8 @@
   // Formats pair-parameters.
   verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
   verifyFormat("[I drawRectOn:surface //\n"
-   "ofSize:aa:bbb\n"
-   "  atOrigin:cc:dd];");
+   "   ofSize:aa:bbb\n"
+   " atOrigin:cc:dd];");
 
   Style.ColumnLimit = 70;
   verifyFormat(
@@ -686,7 +686,19 @@
   "  backing:NSBackingStoreBuffered\n"
   "defer:NO]);\n"
   "}");
-}
+
+  // Respect continuation indent and colon alignment (e.g. when object name is
+  // short, and first selector is the longest one)
+  Style = getLLVMStyle();
+  Style.Language = FormatStyle::LK_ObjC;
+  Style.ContinuationIndentWidth = 8;
+  verifyFormat("[self performSelectorOnMainThread:@selector(loadAccessories)\n"
+   "   withObject:nil\n"
+   "waitUntilDone:false];");
+  verifyFormat("[self performSelector:@selector(loadAccessories)\n"
+   "withObjectOnMainThread:nil\n"
+   " waitUntilDone:false];");
+  }
 
 TEST_F(FormatTestObjC, ObjCAt) {
   verifyFormat("@autoreleasepool");
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -571,12 +571,12 @@
 BeforePrevious->is(tok::r_square) ||
 Contexts.back().LongestObjCSelectorName == 0) {
   Tok->Previous->Type = TT_SelectorName;
-  if (Tok->Previous->ColumnWidth >
-  Contexts.back().LongestObjCSelectorName)
-Contexts.back().LongestObjCSelectorName =
-Tok->Previous->ColumnWidth;
   if (!Contexts.back().FirstObjCSelectorName)
 Contexts.back().FirstObjCSelectorName = Tok->Previous;
+  else if (Tok->Previous->ColumnWidth >
+   Contexts.back().LongestObjCSelectorName)
+Contexts.back().LongestObjCSelectorName =
+Tok->Previous->ColumnWidth;
 }
   } else if (Contexts.back().ColonIsForRangeExpr) {
 Tok->Type = TT_RangeBasedForLoopColon;
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -696,7 +696,8 @@
  ? std::max(State.Stack.back().Indent,
 State.FirstIndent + Style.ContinuationIndentWidth)
  : State.Stack.back().Indent) +
-NextNonComment->LongestObjCSelectorName;
+std::max(NextNonComment->LongestObjCSelectorName,
+ unsigned(NextNonComment->TokenText.size()));
   }
 } else if (State.Stack.back().AlignColons &&
State.Stack.back().ColonPos <= NextNonComment->ColumnWidth) {
@@ -895,7 +896,8 @@
   ? std::max(State.Stack.back().Indent,
  State.FirstIndent + Style.ContinuationIndentWidth)
   : State.Stack.back().Indent) +
- NextNonComment->LongestObjCSelectorName -
+ std::max(NextNonComment->LongestObjCSelectorName,
+  unsigned(NextNonComment->TokenText.size())) -
  NextNonComment->ColumnWidth;
 }
 if (!State.Stack.back().AlignColons)


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -652,8 +652,8 @@
   // Formats pair-parameters.
   verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
   verifyFormat("[I drawRectOn:surface //\n"
-   "ofSize:aa:bbb\n"
-   "  atOrigin:cc:dd];");
+   "   ofSize:aa:bbb\n"
+   " atOrigin:cc:dd];");
 
   Style.ColumnLimit = 70;
   verifyFormat(
@@ -686,7 +686,19 @@
   "  backing:NSBackingStoreBuffered\n"
   "defer:NO]);\n"
   "}");
-}
+
+  // Respect continuation indent and colon alignment (e.g. when object name is
+  // short, and first selector is the longest one)
+  Style = getLLVMStyle();
+  Style.Language = FormatStyle::LK_ObjC;
+  Style.ContinuationIndentWidth = 8;
+  verifyFormat("[self performSelectorOnMainThread:@selector(loadAccessories)\n"
+   "   withObject:nil\n"
+ 

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

[Daniel Jasper via Phabricator via cfe-commits]

djasper added inline comments.



Comment at: lib/Format/ContinuationIndenter.cpp:900
+ std::max(NextNonComment->LongestObjCSelectorName,
+  unsigned(NextNonComment->TokenText.size())) -
  NextNonComment->ColumnWidth;

I'd prefer to use std::max( .. )

(and we generally don't use c-style casts)


Repository:
  rC Clang

https://reviews.llvm.org/D43121



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: unittests/Format/FormatTestObjC.cpp:700
+   "withObjectOnMainThread:nil\n"
+   " waitUntilDone:false];");
+  }

Could you add an instance where the first one is the longest and is wrapped?


Repository:
  rC Clang

https://reviews.llvm.org/D43121



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41698: [DebugInfo] Enable debug information for C99 VLA types

[Carlos Alberto Enciso via Phabricator via cfe-commits]

CarlosAlbertoEnciso added a comment.

Hi @sdesmalen!

First of all my apologies for commenting after the issue has been closed, but I 
do not have an account to add a comment to the associated bugzilla.

I have found what it seems to be an issue with the current implementation.

For the given test case

  int main() {
int size = 2;
  
int vla_expr[size];
vla_expr[1] = 1;
  
return 0;
  }

and while debugging with LLDB, the following error is generated:

  (lldb) n
  Process 21014 stopped
  * thread #1, name = 'bad.out', stop reason = step over
  frame #0: 0x00400502 bad.out`main at vla_2.cpp:7
 4int vla_expr[size];
 5vla_expr[1] = 1;
 6  
  -> 7return 0;
 8  }
  
  (lldb) p vla_expr
  (unsigned long) $0 = 2
  
  (lldb) p vla_expr[1]
  error: subscripted value is not an array, pointer, or vector
  
  (lldb) 

Looking at the DWARF generated, there are 2 variables with the same name at the 
same scope

  DW_TAG_subprogram "main"
...
DW_TAG_variable "size"
DW_TAG_variable "vla_expr"
DW_TAG_variable "vla_expr"

I think there are 2 issues:

The compiler generated variable 'vla_expr'

- should be flagged as artificial (DW_AT_artificial)
- its name should start with double underscore to avoid conflicting with 
user-defined names.

Thanks,
Carlos


Repository:
  rL LLVM

https://reviews.llvm.org/D41698



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43122: [clangd] Fix crash when CompilerInvocation can't be created.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: hokein, ioeric, sammccall.
Herald added subscribers: jkorous-apple, klimek.

This can happen if the CompileCommand provided by compilation database
is malformed.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43122

Files:
  clangd/ClangdUnit.cpp
  clangd/CodeComplete.cpp
  unittests/clangd/ClangdTests.cpp

Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -38,6 +38,25 @@
 
 namespace {
 
+// EXPECT_ERROR seems like a pretty generic name, make sure it's not defined
+// already.
+#ifdef EXPECT_ERROR
+#error "Refusing to redefine EXPECT_ERROR"
+#endif
+
+// Checks there is an error and marks it as handled.
+#define EXPECT_ERROR(expectedValue)\
+  do { \
+auto &&ComputedValue = (expectedValue);\
+if (ComputedValue) {   \
+  ADD_FAILURE() << "expected an error";\
+  break;   \
+}  \
+handleAllErrors(ComputedValue.takeError(), \
+[](const llvm::ErrorInfoBase &) {});   \
+   \
+  } while (false)
+
 // Don't wait for async ops in clangd test more than that to avoid blocking
 // indefinitely in case of bugs.
 static const std::chrono::seconds DefaultFutureTimeout =
@@ -453,6 +472,40 @@
   EXPECT_THAT(Server.getUsedBytesPerFile(), IsEmpty());
 }
 
+TEST_F(ClangdVFSTest, InvalidCompileCommand) {
+  MockFSProvider FS;
+  ErrorCheckingDiagConsumer DiagConsumer;
+  MockCompilationDatabase CDB;
+
+  ClangdServer Server(CDB, DiagConsumer, FS,
+  /*AsyncThreadsCount=*/0,
+  /*StorePreamblesInMemory=*/true);
+
+  auto FooCpp = getVirtualTestFilePath("foo.cpp");
+  // clang cannot create CompilerInvocation if we pass two files in the
+  // CompileCommand. We pass the file in ExtraFlags once and CDB adds another
+  // one in getCompileCommand().
+  CDB.ExtraClangFlags.push_back(FooCpp.str());
+
+  // Clang can't parse command args in that case, but we shouldn't crash.
+  Server.addDocument(FooCpp, "int main() {}");
+
+  EXPECT_EQ(Server.dumpAST(FooCpp), "");
+  EXPECT_ERROR(Server.findDefinitions(FooCpp, Position{0, 0}));
+  EXPECT_ERROR(Server.findDocumentHighlights(FooCpp, Position{0, 0}));
+  EXPECT_ERROR(Server.rename(FooCpp, Position{0, 0}, "new_name"));
+  // FIXME: codeComplete and signatureHelp should also return errors when they
+  // can't parse the file.
+  EXPECT_THAT(
+  Server.codeComplete(FooCpp, Position{0, 0}, clangd::CodeCompleteOptions())
+  .get()
+  .Value.items,
+  IsEmpty());
+  EXPECT_THAT(
+  Server.signatureHelp(FooCpp, Position{0, 0}).get().Value.signatures,
+  IsEmpty());
+}
+
 class ClangdThreadingTest : public ClangdVFSTest {};
 
 TEST_F(ClangdThreadingTest, StressTest) {
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -650,7 +650,10 @@
   CompilerInstance::createDiagnostics(new DiagnosticOptions,
   &DummyDiagsConsumer, false),
   Input.VFS);
-  assert(CI && "Couldn't create CompilerInvocation");
+  if (!CI) {
+log("Couldn't create CompilerInvocation");;
+return false;
+  }
   CI->getFrontendOpts().DisableFree = false;
 
   std::unique_ptr ContentsBuffer =
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -405,6 +405,12 @@
 &IgnoreDiagnostics, false);
 CI = createInvocationFromCommandLine(ArgStrs, CommandLineDiagsEngine,
  Inputs.FS);
+if (!CI) {
+  log("Could not build CompilerInvocation for file " + FileName);
+  AST = llvm::None;
+  Preamble = nullptr;
+  return llvm::None;
+}
 // createInvocationFromCommandLine sets DisableFree.
 CI->getFrontendOpts().DisableFree = false;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43122: [clangd] Fix crash when CompilerInvocation can't be created.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

just nits




Comment at: clangd/ClangdUnit.cpp:417
   }
   assert(CI && "Couldn't create CompilerInvocation");
 

remove?



Comment at: unittests/clangd/ClangdTests.cpp:47
+
+// Checks there is an error and marks it as handled.
+#define EXPECT_ERROR(expectedValue)
\

doc that this consumes Expected



Comment at: unittests/clangd/ClangdTests.cpp:48
+// Checks there is an error and marks it as handled.
+#define EXPECT_ERROR(expectedValue)
\
+  do { 
\

This helper should go in Matchers.h I think



Comment at: unittests/clangd/ClangdTests.cpp:52
+if (ComputedValue) {   
\
+  ADD_FAILURE() << "expected an error";
\
+  break;   
\

More context:
```<< " from " << #expectedValue << " but got " << 
::testing::PrintToString(*ComputedValue)```

(please check the error message works!)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43122



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43110: [Sema] Don't mark plain MS enums as fixed

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Nice!

Test?


https://reviews.llvm.org/D43110



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

[Whisperity via Phabricator via cfe-commits]

whisperity requested changes to this revision.
whisperity added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tidy/misc/ThrowKeywordMissingCheck.cpp:21
+void ThrowKeywordMissingCheck::registerMatchers(MatchFinder *Finder) {
+  // This is a C++ only checker.
+  if (!getLangOpts().CPlusPlus)

Superfluous comment, this can be removed.



Comment at: clang-tidy/misc/ThrowKeywordMissingCheck.cpp:32
+   hasType(cxxRecordDecl(
+   isSameOrDerivedFrom(matchesName("[Ee]xception|EXCEPTION",
+   unless(anyOf(hasAncestor(stmt(

`matchesName` says

> Does not match typedefs of an underlying type with the given name.

Does your check find the following case?

```
typedef std::exception ERROR_BASE;
class my_error : public ERROR_BASE {
  /* Usual yadda. */
};

int main() {
  my_error();
}
```




Comment at: clang-tidy/misc/ThrowKeywordMissingCheck.cpp:46
+  Result.Nodes.getNodeAs("temporary-exception-not-thrown");
+  diag(TemporaryExpr->getLocStart(), "exception created but is not thrown");
+}

Either use passive for both subsentences or don't use passive at all. Maybe one 
of the following could be a better wording:

> exception instantied and not thrown

> exception is constructed but is not thrown

> exception construction without a throw statement



Comment at: test/clang-tidy/misc-throw-keyword-missing.cpp:23
+// std::exception and std::runtime_error declaration
+struct exception{
+  exception();

Please format the code. Why isn't there a space before `{`?



Comment at: test/clang-tidy/misc-throw-keyword-missing.cpp:29
+
+struct runtime_error : public exception{
+  explicit runtime_error( const std::string& what_arg );

Format here too.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43120



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43123: [clangd] Log all ignored diagnostics.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: ioeric, hokein, sammccall.
Herald added subscribers: jkorous-apple, klimek.

To aid debugging failures and crashes.
Only part of ignored diagnostics was logged before, now we log all of
them.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43123

Files:
  clangd/ClangdUnit.cpp
  clangd/Compiler.cpp
  clangd/Compiler.h

Index: clangd/Compiler.h
===
--- clangd/Compiler.h
+++ clangd/Compiler.h
@@ -24,8 +24,11 @@
 
 class IgnoreDiagnostics : public DiagnosticConsumer {
 public:
+  static void logIgnoredDiag(DiagnosticsEngine::Level DiagLevel,
+ const clang::Diagnostic &Info);
+
   void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
-const clang::Diagnostic &Info) override {}
+const clang::Diagnostic &Info) override;
 };
 
 /// Creates a compiler instance, configured so that:
Index: clangd/Compiler.cpp
===
--- clangd/Compiler.cpp
+++ clangd/Compiler.cpp
@@ -8,12 +8,38 @@
 //===-===//
 
 #include "Compiler.h"
+#include "Logger.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/PreprocessorOptions.h"
+#include "llvm/Support/Format.h"
+#include "llvm/Support/FormatVariadic.h"
 
 namespace clang {
 namespace clangd {
 
+void IgnoreDiagnostics::logIgnoredDiag(DiagnosticsEngine::Level DiagLevel,
+   const clang::Diagnostic &Info) {
+  SmallString<64> Message;
+  Info.FormatDiagnostic(Message);
+
+  SmallString<64> Location;
+  if (Info.hasSourceManager() && Info.getLocation().isValid()) {
+auto &SourceMgr = Info.getSourceManager();
+auto Loc = SourceMgr.getFileLoc(Info.getLocation());
+llvm::raw_svector_ostream OS(Location);
+Loc.print(OS, SourceMgr);
+OS << ":";
+  }
+
+  log(llvm::formatv("Ignored diagnostic. {0}{1}", Location,
+Message));
+}
+
+void IgnoreDiagnostics::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
+ const clang::Diagnostic &Info) {
+  logIgnoredDiag(DiagLevel, Info);
+}
+
 std::unique_ptr
 prepareCompilerInstance(std::unique_ptr CI,
 const PrecompiledPreamble *Preamble,
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -26,7 +26,6 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/CrashRecoveryContext.h"
-#include "llvm/Support/Format.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 
@@ -178,27 +177,15 @@
 llvm::Optional toClangdDiag(const clang::Diagnostic &D,
 DiagnosticsEngine::Level Level,
 const LangOptions &LangOpts) {
-  SmallString<64> Message;
-  D.FormatDiagnostic(Message);
-
   if (!D.hasSourceManager() || !D.getLocation().isValid() ||
   !D.getSourceManager().isInMainFile(D.getLocation())) {
-
-SmallString<64> Location;
-if (D.hasSourceManager() && D.getLocation().isValid()) {
-  auto &SourceMgr = D.getSourceManager();
-  auto Loc = SourceMgr.getFileLoc(D.getLocation());
-  llvm::raw_svector_ostream OS(Location);
-  Loc.print(OS, SourceMgr);
-} else {
-  Location = "";
-}
-
-log(llvm::formatv("Ignored diagnostic outside main file. {0}: {1}",
-  Location, Message));
+IgnoreDiagnostics::logIgnoredDiag(Level, D);
 return llvm::None;
   }
 
+  SmallString<64> Message;
+  D.FormatDiagnostic(Message);
+
   DiagWithFixIts Result;
   Result.Diag.range = diagnosticRange(D, LangOpts);
   Result.Diag.severity = getSeverity(Level);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43122: [clangd] Fix crash when CompilerInvocation can't be created.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 133593.
ilya-biryukov marked 4 inline comments as done.
ilya-biryukov added a comment.

- Removed assert that is now redundant
- Moved EXPECT_ERROR to Matchers.h
- Added more context into EXPECT_ERROR's error message


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43122

Files:
  clangd/ClangdUnit.cpp
  clangd/CodeComplete.cpp
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/Matchers.h

Index: unittests/clangd/Matchers.h
===
--- unittests/clangd/Matchers.h
+++ unittests/clangd/Matchers.h
@@ -108,6 +108,28 @@
   return PolySubsequenceMatcher(std::forward(M)...);
 }
 
+// EXPECT_ERROR seems like a pretty generic name, make sure it's not defined
+// already.
+#ifdef EXPECT_ERROR
+#error "Refusing to redefine EXPECT_ERROR"
+#endif
+
+// Consumes llvm::Expected, checks it contains an error and marks it as
+// handled.
+#define EXPECT_ERROR(expectedValue)\
+  do { \
+auto &&ComputedValue = (expectedValue);\
+if (ComputedValue) {   \
+  ADD_FAILURE() << "expected an error from " << #expectedValue \
+<< " but got " \
+<< ::testing::PrintToString(*ComputedValue);   \
+  break;   \
+}  \
+handleAllErrors(ComputedValue.takeError(), \
+[](const llvm::ErrorInfoBase &) {});   \
+   \
+  } while (false)
+
 } // namespace clangd
 } // namespace clang
 #endif
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -9,6 +9,7 @@
 
 #include "ClangdLSPServer.h"
 #include "ClangdServer.h"
+#include "Matchers.h"
 #include "TestFS.h"
 #include "clang/Config/config.h"
 #include "llvm/ADT/SmallVector.h"
@@ -453,6 +454,40 @@
   EXPECT_THAT(Server.getUsedBytesPerFile(), IsEmpty());
 }
 
+TEST_F(ClangdVFSTest, InvalidCompileCommand) {
+  MockFSProvider FS;
+  ErrorCheckingDiagConsumer DiagConsumer;
+  MockCompilationDatabase CDB;
+
+  ClangdServer Server(CDB, DiagConsumer, FS,
+  /*AsyncThreadsCount=*/0,
+  /*StorePreamblesInMemory=*/true);
+
+  auto FooCpp = getVirtualTestFilePath("foo.cpp");
+  // clang cannot create CompilerInvocation if we pass two files in the
+  // CompileCommand. We pass the file in ExtraFlags once and CDB adds another
+  // one in getCompileCommand().
+  CDB.ExtraClangFlags.push_back(FooCpp.str());
+
+  // Clang can't parse command args in that case, but we shouldn't crash.
+  Server.addDocument(FooCpp, "int main() {}");
+
+  EXPECT_EQ(Server.dumpAST(FooCpp), "");
+  EXPECT_ERROR(Server.findDefinitions(FooCpp, Position{0, 0}));
+  EXPECT_ERROR(Server.findDocumentHighlights(FooCpp, Position{0, 0}));
+  EXPECT_ERROR(Server.rename(FooCpp, Position{0, 0}, "new_name"));
+  // FIXME: codeComplete and signatureHelp should also return errors when they
+  // can't parse the file.
+  EXPECT_THAT(
+  Server.codeComplete(FooCpp, Position{0, 0}, clangd::CodeCompleteOptions())
+  .get()
+  .Value.items,
+  IsEmpty());
+  EXPECT_THAT(
+  Server.signatureHelp(FooCpp, Position{0, 0}).get().Value.signatures,
+  IsEmpty());
+}
+
 class ClangdThreadingTest : public ClangdVFSTest {};
 
 TEST_F(ClangdThreadingTest, StressTest) {
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -650,7 +650,10 @@
   CompilerInstance::createDiagnostics(new DiagnosticOptions,
   &DummyDiagsConsumer, false),
   Input.VFS);
-  assert(CI && "Couldn't create CompilerInvocation");
+  if (!CI) {
+log("Couldn't create CompilerInvocation");;
+return false;
+  }
   CI->getFrontendOpts().DisableFree = false;
 
   std::unique_ptr ContentsBuffer =
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -405,10 +405,15 @@
 &IgnoreDiagnostics, false);
 CI = createInvocationFromCommandLine(ArgStrs, CommandLineDiagsEngine,
  Inputs.FS);
+if (!CI) {
+  log("Could not build CompilerInvocation for file " + FileName);
+  AST = llvm::None;
+  Preamble = nullptr;
+  return llvm

[clang-tools-extra] r324732 - [clangd] Fix crash when CompilerInvocation can't be created.

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Fri Feb  9 05:51:57 2018
New Revision: 324732

URL: http://llvm.org/viewvc/llvm-project?rev=324732&view=rev
Log:
[clangd] Fix crash when CompilerInvocation can't be created.

Summary:
This can happen if the CompileCommand provided by compilation database
is malformed.

Reviewers: hokein, ioeric, sammccall

Reviewed By: sammccall

Subscribers: klimek, jkorous-apple, cfe-commits

Differential Revision: https://reviews.llvm.org/D43122

Modified:
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
clang-tools-extra/trunk/unittests/clangd/Matchers.h

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=324732&r1=324731&r2=324732&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Fri Feb  9 05:51:57 2018
@@ -405,10 +405,15 @@ CppFile::rebuild(ParseInputs &&Inputs) {
 &IgnoreDiagnostics, false);
 CI = createInvocationFromCommandLine(ArgStrs, CommandLineDiagsEngine,
  Inputs.FS);
+if (!CI) {
+  log("Could not build CompilerInvocation for file " + FileName);
+  AST = llvm::None;
+  Preamble = nullptr;
+  return llvm::None;
+}
 // createInvocationFromCommandLine sets DisableFree.
 CI->getFrontendOpts().DisableFree = false;
   }
-  assert(CI && "Couldn't create CompilerInvocation");
 
   std::unique_ptr ContentsBuffer =
   llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents, FileName);

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=324732&r1=324731&r2=324732&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Fri Feb  9 05:51:57 2018
@@ -650,7 +650,10 @@ bool semaCodeComplete(std::unique_ptrgetFrontendOpts().DisableFree = false;
 
   std::unique_ptr ContentsBuffer =

Modified: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp?rev=324732&r1=324731&r2=324732&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Fri Feb  9 
05:51:57 2018
@@ -9,6 +9,7 @@
 
 #include "ClangdLSPServer.h"
 #include "ClangdServer.h"
+#include "Matchers.h"
 #include "TestFS.h"
 #include "clang/Config/config.h"
 #include "llvm/ADT/SmallVector.h"
@@ -453,6 +454,40 @@ struct Something {
   EXPECT_THAT(Server.getUsedBytesPerFile(), IsEmpty());
 }
 
+TEST_F(ClangdVFSTest, InvalidCompileCommand) {
+  MockFSProvider FS;
+  ErrorCheckingDiagConsumer DiagConsumer;
+  MockCompilationDatabase CDB;
+
+  ClangdServer Server(CDB, DiagConsumer, FS,
+  /*AsyncThreadsCount=*/0,
+  /*StorePreamblesInMemory=*/true);
+
+  auto FooCpp = getVirtualTestFilePath("foo.cpp");
+  // clang cannot create CompilerInvocation if we pass two files in the
+  // CompileCommand. We pass the file in ExtraFlags once and CDB adds another
+  // one in getCompileCommand().
+  CDB.ExtraClangFlags.push_back(FooCpp.str());
+
+  // Clang can't parse command args in that case, but we shouldn't crash.
+  Server.addDocument(FooCpp, "int main() {}");
+
+  EXPECT_EQ(Server.dumpAST(FooCpp), "");
+  EXPECT_ERROR(Server.findDefinitions(FooCpp, Position{0, 0}));
+  EXPECT_ERROR(Server.findDocumentHighlights(FooCpp, Position{0, 0}));
+  EXPECT_ERROR(Server.rename(FooCpp, Position{0, 0}, "new_name"));
+  // FIXME: codeComplete and signatureHelp should also return errors when they
+  // can't parse the file.
+  EXPECT_THAT(
+  Server.codeComplete(FooCpp, Position{0, 0}, 
clangd::CodeCompleteOptions())
+  .get()
+  .Value.items,
+  IsEmpty());
+  EXPECT_THAT(
+  Server.signatureHelp(FooCpp, Position{0, 0}).get().Value.signatures,
+  IsEmpty());
+}
+
 class ClangdThreadingTest : public ClangdVFSTest {};
 
 TEST_F(ClangdThreadingTest, StressTest) {

Modified: clang-tools-extra/trunk/unittests/clangd/Matchers.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/Matchers.h?rev=324732&r1=324731&r2=324732&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/Matchers.h (original)
+++ clang-tools-extra/trunk/unittests/clangd/Matchers.h Fri Feb  9 05:51:57 2018
@@ -108,6 +108,28 @@ PolySubsequenceMatcher Ha

[PATCH] D43122: [clangd] Fix crash when CompilerInvocation can't be created.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL324732: [clangd] Fix crash when CompilerInvocation 
can't be created. (authored by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D43122

Files:
  clang-tools-extra/trunk/clangd/ClangdUnit.cpp
  clang-tools-extra/trunk/clangd/CodeComplete.cpp
  clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
  clang-tools-extra/trunk/unittests/clangd/Matchers.h

Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp
@@ -405,10 +405,15 @@
 &IgnoreDiagnostics, false);
 CI = createInvocationFromCommandLine(ArgStrs, CommandLineDiagsEngine,
  Inputs.FS);
+if (!CI) {
+  log("Could not build CompilerInvocation for file " + FileName);
+  AST = llvm::None;
+  Preamble = nullptr;
+  return llvm::None;
+}
 // createInvocationFromCommandLine sets DisableFree.
 CI->getFrontendOpts().DisableFree = false;
   }
-  assert(CI && "Couldn't create CompilerInvocation");
 
   std::unique_ptr ContentsBuffer =
   llvm::MemoryBuffer::getMemBufferCopy(Inputs.Contents, FileName);
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -650,7 +650,10 @@
   CompilerInstance::createDiagnostics(new DiagnosticOptions,
   &DummyDiagsConsumer, false),
   Input.VFS);
-  assert(CI && "Couldn't create CompilerInvocation");
+  if (!CI) {
+log("Couldn't create CompilerInvocation");;
+return false;
+  }
   CI->getFrontendOpts().DisableFree = false;
 
   std::unique_ptr ContentsBuffer =
Index: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
@@ -9,6 +9,7 @@
 
 #include "ClangdLSPServer.h"
 #include "ClangdServer.h"
+#include "Matchers.h"
 #include "TestFS.h"
 #include "clang/Config/config.h"
 #include "llvm/ADT/SmallVector.h"
@@ -453,6 +454,40 @@
   EXPECT_THAT(Server.getUsedBytesPerFile(), IsEmpty());
 }
 
+TEST_F(ClangdVFSTest, InvalidCompileCommand) {
+  MockFSProvider FS;
+  ErrorCheckingDiagConsumer DiagConsumer;
+  MockCompilationDatabase CDB;
+
+  ClangdServer Server(CDB, DiagConsumer, FS,
+  /*AsyncThreadsCount=*/0,
+  /*StorePreamblesInMemory=*/true);
+
+  auto FooCpp = getVirtualTestFilePath("foo.cpp");
+  // clang cannot create CompilerInvocation if we pass two files in the
+  // CompileCommand. We pass the file in ExtraFlags once and CDB adds another
+  // one in getCompileCommand().
+  CDB.ExtraClangFlags.push_back(FooCpp.str());
+
+  // Clang can't parse command args in that case, but we shouldn't crash.
+  Server.addDocument(FooCpp, "int main() {}");
+
+  EXPECT_EQ(Server.dumpAST(FooCpp), "");
+  EXPECT_ERROR(Server.findDefinitions(FooCpp, Position{0, 0}));
+  EXPECT_ERROR(Server.findDocumentHighlights(FooCpp, Position{0, 0}));
+  EXPECT_ERROR(Server.rename(FooCpp, Position{0, 0}, "new_name"));
+  // FIXME: codeComplete and signatureHelp should also return errors when they
+  // can't parse the file.
+  EXPECT_THAT(
+  Server.codeComplete(FooCpp, Position{0, 0}, clangd::CodeCompleteOptions())
+  .get()
+  .Value.items,
+  IsEmpty());
+  EXPECT_THAT(
+  Server.signatureHelp(FooCpp, Position{0, 0}).get().Value.signatures,
+  IsEmpty());
+}
+
 class ClangdThreadingTest : public ClangdVFSTest {};
 
 TEST_F(ClangdThreadingTest, StressTest) {
Index: clang-tools-extra/trunk/unittests/clangd/Matchers.h
===
--- clang-tools-extra/trunk/unittests/clangd/Matchers.h
+++ clang-tools-extra/trunk/unittests/clangd/Matchers.h
@@ -108,6 +108,28 @@
   return PolySubsequenceMatcher(std::forward(M)...);
 }
 
+// EXPECT_ERROR seems like a pretty generic name, make sure it's not defined
+// already.
+#ifdef EXPECT_ERROR
+#error "Refusing to redefine EXPECT_ERROR"
+#endif
+
+// Consumes llvm::Expected, checks it contains an error and marks it as
+// handled.
+#define EXPECT_ERROR(expectedValue)\
+  do { \
+auto &&ComputedValue = (expectedValue);\
+if (ComputedValue) {   \
+  ADD_FAILURE() << "expected an error from " << #expectedValue \
+ 

[PATCH] D5767: Template Instantiation Observer + a few other templight-related changes

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

In https://reviews.llvm.org/D5767#1000347, @xazax.hun wrote:

> In https://reviews.llvm.org/D5767#999143, @sabel83 wrote:
>
> > 2. What do you mean by regression tests? We have run the clang-test target 
> > successfully on the patched code (which has the hook). Note that the hook 
> > this pull request provides is implemented as a ProgramAction. It is not 
> > intended to be used together with other program actions, I don't see how we 
> > could turn it on for other tests (if that is what you referred to).
>
>
> I would bet the sema tests are full of tricky edge cases. So running 
> templight on those tests would be a helpful exercise. I am only interested in 
> assertion fails, so we do not need to check the output.  One option to do so 
> would be to add the -templight-dump option to the %clang_cc1 variable when 
> running the tests. Note that the tests are likely to fail because the output 
> will change, but if there are no crashes, it is fine.


I did just that, and nothing crashed during the regression tests (in fact, 
every single one passed)!


https://reviews.llvm.org/D5767



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 133595.
ioeric marked 5 inline comments as done.
ioeric added a comment.

- Addressed review comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42640

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/CodeComplete.cpp
  clangd/Headers.cpp
  clangd/Headers.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/clients/clangd-vscode/package.json
  clangd/global-symbol-builder/CMakeLists.txt
  clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
  clangd/index/CanonicalIncludes.cpp
  clangd/index/CanonicalIncludes.h
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/Merge.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  clangd/index/SymbolYAML.cpp
  clangd/tool/ClangdMain.cpp
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test
  test/clangd/insert-include.test
  unittests/clangd/CMakeLists.txt
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/HeadersTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -47,6 +47,12 @@
 }
 MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
 MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.FileURI == P; }
+MATCHER(HasIncludeHeader, "") {
+  return arg.Detail && !arg.Detail->IncludeHeader.empty();
+}
+MATCHER_P(IncludeHeader, P, "") {
+  return arg.Detail && arg.Detail->IncludeHeader == P;
+}
 MATCHER_P(LocationOffsets, Offsets, "") {
   // Offset range in SymbolLocation is [start, end] while in Clangd is [start,
   // end).
@@ -60,41 +66,62 @@
 namespace {
 class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
 public:
-  SymbolIndexActionFactory(SymbolCollector::Options COpts)
-  : COpts(std::move(COpts)) {}
+  SymbolIndexActionFactory(SymbolCollector::Options COpts,
+   CommentHandler *PragmaHandler)
+  : COpts(std::move(COpts)), PragmaHandler(PragmaHandler) {}
 
   clang::FrontendAction *create() override {
+class WrappedIndexAction : public WrapperFrontendAction {
+public:
+  WrappedIndexAction(std::shared_ptr C,
+ const index::IndexingOptions &Opts,
+ CommentHandler *PragmaHandler)
+  : WrapperFrontendAction(
+index::createIndexingAction(C, Opts, nullptr)),
+PragmaHandler(PragmaHandler) {}
+
+  std::unique_ptr
+  CreateASTConsumer(CompilerInstance &CI, StringRef InFile) override {
+if (PragmaHandler)
+  CI.getPreprocessor().addCommentHandler(PragmaHandler);
+return WrapperFrontendAction::CreateASTConsumer(CI, InFile);
+  }
+
+private:
+  index::IndexingOptions IndexOpts;
+  CommentHandler *PragmaHandler;
+};
 index::IndexingOptions IndexOpts;
 IndexOpts.SystemSymbolFilter =
 index::IndexingOptions::SystemSymbolFilterKind::All;
 IndexOpts.IndexFunctionLocals = false;
 Collector = std::make_shared(COpts);
-FrontendAction *Action =
-index::createIndexingAction(Collector, IndexOpts, nullptr).release();
-return Action;
+return new WrappedIndexAction(Collector, std::move(IndexOpts),
+  PragmaHandler);
   }
 
   std::shared_ptr Collector;
   SymbolCollector::Options COpts;
+  CommentHandler *PragmaHandler;
 };
 
 class SymbolCollectorTest : public ::testing::Test {
 public:
   SymbolCollectorTest()
-  : TestHeaderName(getVirtualTestFilePath("symbol.h").str()),
+  : InMemoryFileSystem(new vfs::InMemoryFileSystem),
+TestHeaderName(getVirtualTestFilePath("symbol.h").str()),
 TestFileName(getVirtualTestFilePath("symbol.cc").str()) {
 TestHeaderURI = URI::createFile(TestHeaderName).toString();
 TestFileURI = URI::createFile(TestFileName).toString();
   }
 
   bool runSymbolCollector(StringRef HeaderCode, StringRef MainCode,
   const std::vector &ExtraArgs = {}) {
-llvm::IntrusiveRefCntPtr InMemoryFileSystem(
-new vfs::InMemoryFileSystem);
 llvm::IntrusiveRefCntPtr Files(
 new FileManager(FileSystemOptions(), InMemoryFileSystem));
 
-auto Factory = llvm::make_unique(CollectorOpts);
+auto Factory = llvm::make_unique(
+CollectorOpts, PragmaHandler.get());
 
 std::vector Args = {"symbol_collector", "-fsyntax-only",
  "-std=c++11", TestFileName};
@@ -120,12 +147,14 @@
   }
 
 protected:
+  llvm::IntrusiveRefCntPtr InMemoryFileSystem;
   std::string TestHeaderName;
   std::string TestHeaderURI;
   std::string TestFileName;
   std::string TestFileURI;
   SymbolSlab Symbols;
   SymbolCollector::Options CollectorOpts;
+  std::unique_ptr PragmaHan

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 133596.
Typz marked 2 inline comments as done.
Typz added a comment.

Address review comments


Repository:
  rC Clang

https://reviews.llvm.org/D43121

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -652,8 +652,8 @@
   // Formats pair-parameters.
   verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
   verifyFormat("[I drawRectOn:surface //\n"
-   "ofSize:aa:bbb\n"
-   "  atOrigin:cc:dd];");
+   "   ofSize:aa:bbb\n"
+   " atOrigin:cc:dd];");
 
   Style.ColumnLimit = 70;
   verifyFormat(
@@ -686,6 +686,26 @@
   "  backing:NSBackingStoreBuffered\n"
   "defer:NO]);\n"
   "}");
+
+  // Respect continuation indent and colon alignment (e.g. when object name is
+  // short, and first selector is the longest one)
+  Style = getLLVMStyle();
+  Style.Language = FormatStyle::LK_ObjC;
+  Style.ContinuationIndentWidth = 8;
+  verifyFormat("[self performSelectorOnMainThread:@selector(loadAccessories)\n"
+   "   withObject:nil\n"
+   "waitUntilDone:false];");
+  verifyFormat("[self performSelector:@selector(loadAccessories)\n"
+   "withObjectOnMainThread:nil\n"
+   " waitUntilDone:false];");
+  verifyFormat("[a\n"
+   "
performSelectorOnMainThread:@selector(loadAccessories)\n"
+   " withObject:nil\n"
+   "  waitUntilDone:false];");
+  verifyFormat("[self // force wrapping\n"
+   "
performSelectorOnMainThread:@selector(loadAccessories)\n"
+   " withObject:nil\n"
+   "  waitUntilDone:false];");
 }
 
 TEST_F(FormatTestObjC, ObjCAt) {
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -571,12 +571,12 @@
 BeforePrevious->is(tok::r_square) ||
 Contexts.back().LongestObjCSelectorName == 0) {
   Tok->Previous->Type = TT_SelectorName;
-  if (Tok->Previous->ColumnWidth >
-  Contexts.back().LongestObjCSelectorName)
-Contexts.back().LongestObjCSelectorName =
-Tok->Previous->ColumnWidth;
   if (!Contexts.back().FirstObjCSelectorName)
 Contexts.back().FirstObjCSelectorName = Tok->Previous;
+  else if (Tok->Previous->ColumnWidth >
+   Contexts.back().LongestObjCSelectorName)
+Contexts.back().LongestObjCSelectorName =
+Tok->Previous->ColumnWidth;
 }
   } else if (Contexts.back().ColonIsForRangeExpr) {
 Tok->Type = TT_RangeBasedForLoopColon;
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -696,7 +696,8 @@
  ? std::max(State.Stack.back().Indent,
 State.FirstIndent + Style.ContinuationIndentWidth)
  : State.Stack.back().Indent) +
-NextNonComment->LongestObjCSelectorName;
+std::max(NextNonComment->LongestObjCSelectorName,
+ NextNonComment->ColumnWidth);
   }
 } else if (State.Stack.back().AlignColons &&
State.Stack.back().ColonPos <= NextNonComment->ColumnWidth) {
@@ -895,7 +896,8 @@
   ? std::max(State.Stack.back().Indent,
  State.FirstIndent + Style.ContinuationIndentWidth)
   : State.Stack.back().Indent) +
- NextNonComment->LongestObjCSelectorName -
+ std::max(NextNonComment->LongestObjCSelectorName,
+  NextNonComment->ColumnWidth) -
  NextNonComment->ColumnWidth;
 }
 if (!State.Stack.back().AlignColons)


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -652,8 +652,8 @@
   // Formats pair-parameters.
   verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
   verifyFormat("[I drawRectOn:surface //\n"
-   "ofSize:aa:bbb\n"
-   "  atOrigin:cc:dd];");
+   "   ofSize:aa:bbb\n"
+   " atOrigin:cc:dd];");
 
   Style.ColumnLimit = 70;
   verifyFormat(
@@ -686,6 +686,26 @@
   "  backing:

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: lib/Format/ContinuationIndenter.cpp:900
+ std::max(NextNonComment->LongestObjCSelectorName,
+  unsigned(NextNonComment->TokenText.size())) -
  NextNonComment->ColumnWidth;

djasper wrote:
> I'd prefer to use std::max( .. )
> 
> (and we generally don't use c-style casts)
this is C++-style C-style cast :-)

anyway changed to use ColumnWidth, as this is also what is used for 
initializing LongestObjCSelectorName.


Repository:
  rC Clang

https://reviews.llvm.org/D43121



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

Thanks! PTAL




Comment at: clangd/ClangdServer.cpp:465
+auto &HeaderSearchInfo = Clang->getPreprocessor().getHeaderSearchInfo();
+std::string Suggested = HeaderSearchInfo.suggestPathToFileForDiagnostics(
+*Resolved, CompileCommand.Directory);

sammccall wrote:
> ioeric wrote:
> > sammccall wrote:
> > > do we handle the case that the suggestion is already included?
> > > (including the case where it's included by a different name)
> > Yes and no. The current implementation only does textual matching against 
> > existing #includes in the current file and inserts the header if no same 
> > header was found. This complies with IWYU. But we are not handling the case 
> > where the same header is included by different names. I added a `FIXME` for 
> > this.
> I can't see the FIXME? (There's one in the header, but it doesn't seem to 
> really cover this case)
> 
> So this doesn't seem so hard: we can pass the file content, turn off 
> recursive PP, and add PP callbacks to capture the names of each included file 
> (the include-scanner patch does this).
> I'm not sure it's worth deferring, at least we should fix it soon before we 
> lose context.
> 
> But up to you, I'd suggest putting the fixme where we expect to fix it.
Since you prefer, I have included the change in the patch. I wanted to get to 
this as soon as this patch is landed. 



Comment at: clangd/ClangdServer.cpp:368
 
+/// Calculates the shortest possible include path when inserting \p Header to 
\p
+/// File, by matching \p Header against all include search directories for \p

sammccall wrote:
> This has a fair amount of logic (well, plumbing :-D) and uses relatively 
> little from ClangdServer.
> Can we move `shortenIncludePath` to a separate file and pass in the FS and 
> command?
> 
> I'd suggest maybe `Headers.h` - The formatting part of `insertInclude` could 
> also fit there, as well as probably the logic from `switchSourceHeader` I 
> think (the latter not in this patch of course).
> 
> This won't really help much with testing, I just find it gets hard to 
> navigate files that have lots of details of unrelated features. ClangdUnit 
> and ClangdServer both have this potential to grow without bound, though 
> they're in reasonable shape now. Interested what you think!
Done.

I didn't move the formatting code, as half of the code is pulling out the 
style, which we might want to share/change depending on other clangd logics 
that might use style.



Comment at: unittests/clangd/ClangdTests.cpp:849
 
+TEST_F(ClangdVFSTest, InsertIncludes) {
+  MockFSProvider FS;

sammccall wrote:
> Similarly, it'd be nice to pull these tests out into a test file parallel to 
> the header.
> (As with the other tests, often it's easiest to actually test through the 
> ClangdServer interface - this is mostly just for navigation)
Done. I left a simple test for replacements.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42640



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 133597.
Szelethus added a comment.

Changes made according to @whisperity's comments.


https://reviews.llvm.org/D43120

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/ThrowKeywordMissingCheck.cpp
  clang-tidy/misc/ThrowKeywordMissingCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-throw-keyword-missing.rst
  test/clang-tidy/misc-throw-keyword-missing.cpp

Index: test/clang-tidy/misc-throw-keyword-missing.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-throw-keyword-missing.cpp
@@ -0,0 +1,154 @@
+// RUN: %check_clang_tidy %s misc-throw-keyword-missing %t
+
+namespace std {
+
+// std::string declaration (taken from test/clang-tidy/readability-redundant-string-cstr-msvc.cpp).
+template 
+class allocator {};
+template 
+class char_traits {};
+template , typename A = std::allocator>
+struct basic_string {
+  basic_string();
+  basic_string(const basic_string &);
+  // MSVC headers define two constructors instead of using optional arguments.
+  basic_string(const C *);
+  basic_string(const C *, const A &);
+  ~basic_string();
+};
+typedef basic_string string;
+typedef basic_string wstring;
+
+// std::exception and std::runtime_error declaration.
+struct exception {
+  exception();
+  exception(const exception &other);
+  virtual ~exception();
+};
+
+struct runtime_error : public exception {
+  explicit runtime_error(const std::string &what_arg);
+};
+
+} //namespace std
+
+// The usage of this class should never emit a warning.
+struct RegularClass {};
+
+// Classname contains the substring "exception", in certain cases using this class should emit a warning.
+struct RegularException {
+  RegularException() {}
+
+  // Constructors with a single argument are treated differently (cxxFunctionalCastExpr).
+  RegularException(int) {}
+};
+
+// --
+
+void stdExceptionNotTrown(int i) {
+  if (i < 0)
+//CHECK-MESSAGES: :[[@LINE+1]]:5: warning: exception instantiated but not bound (did you intend to 'throw'?) [misc-throw-keyword-missing]
+std::exception();
+
+  if (i > 0)
+//CHECK-MESSAGES: :[[@LINE+1]]:5: warning: exception instantiated but not bound (did you intend to 'throw'?) [misc-throw-keyword-missing]
+std::runtime_error("Unexpected argument");
+}
+
+void stdExceptionThrown(int i) {
+  if (i < 0)
+throw std::exception();
+
+  if (i > 0)
+throw std::runtime_error("Unexpected argument");
+}
+
+void regularClassNotThrown(int i) {
+  if (i < 0)
+RegularClass();
+}
+
+void regularClassThrown(int i) {
+  if (i < 0)
+throw RegularClass();
+}
+
+void nameContainsExceptionNotThrown(int i) {
+  if (i < 0)
+//CHECK-MESSAGES: :[[@LINE+1]]:5: warning: exception instantiated but not bound (did you intend to 'throw'?) [misc-throw-keyword-missing]
+RegularException();
+
+  if (i > 0)
+//CHECK-MESSAGES: :[[@LINE+1]]:5: warning: exception instantiated but not bound (did you intend to 'throw'?) [misc-throw-keyword-missing]
+RegularException(5);
+}
+
+void nameContainsExceptionThrown(int i) {
+  if (i < 0)
+throw RegularException();
+
+  if (i > 0)
+throw RegularException(5);
+}
+
+template 
+void f(int i, Exception excToBeThrown) {}
+
+void funcCallWithTempExcTest() {
+  f(5, RegularException());
+}
+
+// Global variable initilization test.
+RegularException exc = RegularException();
+RegularException *excptr = new RegularException();
+
+void localVariableInitTest() {
+  RegularException exc = RegularException();
+  RegularException *excptr = new RegularException();
+}
+
+class CtorInitializerListTest {
+  RegularException exc;
+
+  CtorInitializerListTest() : exc(RegularException()) {}
+
+  CtorInitializerListTest(int) try : exc(RegularException()) {
+//Constructor body
+  } catch (...) {
+//CHECK-MESSAGES: :[[@LINE+1]]:5: warning: exception instantiated but not bound (did you intend to 'throw'?) [misc-throw-keyword-missing]
+RegularException();
+  }
+
+  CtorInitializerListTest(float);
+};
+
+CtorInitializerListTest::CtorInitializerListTest(float) try : exc(RegularException()) {
+  //Constructor body
+} catch (...) {
+  //CHECK-MESSAGES: :[[@LINE+1]]:3: warning: exception instantiated but not bound (did you intend to 'throw'?) [misc-throw-keyword-missing]
+  RegularException();
+}
+
+RegularException funcReturningExceptioniTest(int i) {
+  return RegularException();
+}
+
+struct ClassBracedInitListTest {
+  ClassBracedInitListTest(RegularException exc) {}
+};
+
+void functionBracedInitListTest(RegularException, ClassBracedInitListTest) {}
+
+void bracedInitListTest() {
+  RegularException exc{};
+  ClassBracedInitListTest test = {RegularException()};
+  functionBracedInitListTest({}, {RegularException()});
+}
+
+typedef std::exception ERROR_BASE;
+class RegularError : public ERROR_BASE {};
+
+void preprocessorCheck() {
+  //CHECK-MESSAGES: :[[@LINE+1]]:3: 

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus marked 5 inline comments as done.
Szelethus added inline comments.



Comment at: clang-tidy/misc/ThrowKeywordMissingCheck.cpp:32
+   hasType(cxxRecordDecl(
+   isSameOrDerivedFrom(matchesName("[Ee]xception|EXCEPTION",
+   unless(anyOf(hasAncestor(stmt(

whisperity wrote:
> `matchesName` says
> 
> > Does not match typedefs of an underlying type with the given name.
> 
> Does your check find the following case?
> 
> ```
> typedef std::exception ERROR_BASE;
> class my_error : public ERROR_BASE {
>   /* Usual yadda. */
> };
> 
> int main() {
>   my_error();
> }
> ```
> 
It does! I've added this case to the test file.


https://reviews.llvm.org/D43120



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 133599.
ioeric added a comment.

- fix a leftover bug


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42640

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/CodeComplete.cpp
  clangd/Headers.cpp
  clangd/Headers.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/clients/clangd-vscode/package.json
  clangd/global-symbol-builder/CMakeLists.txt
  clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
  clangd/index/CanonicalIncludes.cpp
  clangd/index/CanonicalIncludes.h
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/Merge.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  clangd/index/SymbolYAML.cpp
  clangd/tool/ClangdMain.cpp
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test
  test/clangd/insert-include.test
  unittests/clangd/CMakeLists.txt
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/HeadersTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -47,6 +47,12 @@
 }
 MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
 MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.FileURI == P; }
+MATCHER(HasIncludeHeader, "") {
+  return arg.Detail && !arg.Detail->IncludeHeader.empty();
+}
+MATCHER_P(IncludeHeader, P, "") {
+  return arg.Detail && arg.Detail->IncludeHeader == P;
+}
 MATCHER_P(LocationOffsets, Offsets, "") {
   // Offset range in SymbolLocation is [start, end] while in Clangd is [start,
   // end).
@@ -60,41 +66,62 @@
 namespace {
 class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
 public:
-  SymbolIndexActionFactory(SymbolCollector::Options COpts)
-  : COpts(std::move(COpts)) {}
+  SymbolIndexActionFactory(SymbolCollector::Options COpts,
+   CommentHandler *PragmaHandler)
+  : COpts(std::move(COpts)), PragmaHandler(PragmaHandler) {}
 
   clang::FrontendAction *create() override {
+class WrappedIndexAction : public WrapperFrontendAction {
+public:
+  WrappedIndexAction(std::shared_ptr C,
+ const index::IndexingOptions &Opts,
+ CommentHandler *PragmaHandler)
+  : WrapperFrontendAction(
+index::createIndexingAction(C, Opts, nullptr)),
+PragmaHandler(PragmaHandler) {}
+
+  std::unique_ptr
+  CreateASTConsumer(CompilerInstance &CI, StringRef InFile) override {
+if (PragmaHandler)
+  CI.getPreprocessor().addCommentHandler(PragmaHandler);
+return WrapperFrontendAction::CreateASTConsumer(CI, InFile);
+  }
+
+private:
+  index::IndexingOptions IndexOpts;
+  CommentHandler *PragmaHandler;
+};
 index::IndexingOptions IndexOpts;
 IndexOpts.SystemSymbolFilter =
 index::IndexingOptions::SystemSymbolFilterKind::All;
 IndexOpts.IndexFunctionLocals = false;
 Collector = std::make_shared(COpts);
-FrontendAction *Action =
-index::createIndexingAction(Collector, IndexOpts, nullptr).release();
-return Action;
+return new WrappedIndexAction(Collector, std::move(IndexOpts),
+  PragmaHandler);
   }
 
   std::shared_ptr Collector;
   SymbolCollector::Options COpts;
+  CommentHandler *PragmaHandler;
 };
 
 class SymbolCollectorTest : public ::testing::Test {
 public:
   SymbolCollectorTest()
-  : TestHeaderName(getVirtualTestFilePath("symbol.h").str()),
+  : InMemoryFileSystem(new vfs::InMemoryFileSystem),
+TestHeaderName(getVirtualTestFilePath("symbol.h").str()),
 TestFileName(getVirtualTestFilePath("symbol.cc").str()) {
 TestHeaderURI = URI::createFile(TestHeaderName).toString();
 TestFileURI = URI::createFile(TestFileName).toString();
   }
 
   bool runSymbolCollector(StringRef HeaderCode, StringRef MainCode,
   const std::vector &ExtraArgs = {}) {
-llvm::IntrusiveRefCntPtr InMemoryFileSystem(
-new vfs::InMemoryFileSystem);
 llvm::IntrusiveRefCntPtr Files(
 new FileManager(FileSystemOptions(), InMemoryFileSystem));
 
-auto Factory = llvm::make_unique(CollectorOpts);
+auto Factory = llvm::make_unique(
+CollectorOpts, PragmaHandler.get());
 
 std::vector Args = {"symbol_collector", "-fsyntax-only",
  "-std=c++11", TestFileName};
@@ -120,12 +147,14 @@
   }
 
 protected:
+  llvm::IntrusiveRefCntPtr InMemoryFileSystem;
   std::string TestHeaderName;
   std::string TestHeaderURI;
   std::string TestFileName;
   std::string TestFileURI;
   SymbolSlab Symbols;
   SymbolCollector::Options CollectorOpts;
+  std::unique_ptr PragmaHandler;
 };
 
 TEST_F(SymbolCollectorTest, CollectS

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 133600.
Szelethus marked an inline comment as done.

https://reviews.llvm.org/D43120

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/ThrowKeywordMissingCheck.cpp
  clang-tidy/misc/ThrowKeywordMissingCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-throw-keyword-missing.rst
  test/clang-tidy/misc-throw-keyword-missing.cpp

Index: test/clang-tidy/misc-throw-keyword-missing.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-throw-keyword-missing.cpp
@@ -0,0 +1,154 @@
+// RUN: %check_clang_tidy %s misc-throw-keyword-missing %t
+
+namespace std {
+
+// std::string declaration (taken from test/clang-tidy/readability-redundant-string-cstr-msvc.cpp).
+template 
+class allocator {};
+template 
+class char_traits {};
+template , typename A = std::allocator>
+struct basic_string {
+  basic_string();
+  basic_string(const basic_string &);
+  // MSVC headers define two constructors instead of using optional arguments.
+  basic_string(const C *);
+  basic_string(const C *, const A &);
+  ~basic_string();
+};
+typedef basic_string string;
+typedef basic_string wstring;
+
+// std::exception and std::runtime_error declaration.
+struct exception {
+  exception();
+  exception(const exception &other);
+  virtual ~exception();
+};
+
+struct runtime_error : public exception {
+  explicit runtime_error(const std::string &what_arg);
+};
+
+} //namespace std
+
+// The usage of this class should never emit a warning.
+struct RegularClass {};
+
+// Classname contains the substring "exception", in certain cases using this class should emit a warning.
+struct RegularException {
+  RegularException() {}
+
+  // Constructors with a single argument are treated differently (cxxFunctionalCastExpr).
+  RegularException(int) {}
+};
+
+// --
+
+void stdExceptionNotTrown(int i) {
+  if (i < 0)
+//CHECK-MESSAGES: :[[@LINE+1]]:5: warning: exception instantiated but not bound (did you intend to 'throw'?) [misc-throw-keyword-missing]
+std::exception();
+
+  if (i > 0)
+//CHECK-MESSAGES: :[[@LINE+1]]:5: warning: exception instantiated but not bound (did you intend to 'throw'?) [misc-throw-keyword-missing]
+std::runtime_error("Unexpected argument");
+}
+
+void stdExceptionThrown(int i) {
+  if (i < 0)
+throw std::exception();
+
+  if (i > 0)
+throw std::runtime_error("Unexpected argument");
+}
+
+void regularClassNotThrown(int i) {
+  if (i < 0)
+RegularClass();
+}
+
+void regularClassThrown(int i) {
+  if (i < 0)
+throw RegularClass();
+}
+
+void nameContainsExceptionNotThrown(int i) {
+  if (i < 0)
+//CHECK-MESSAGES: :[[@LINE+1]]:5: warning: exception instantiated but not bound (did you intend to 'throw'?) [misc-throw-keyword-missing]
+RegularException();
+
+  if (i > 0)
+//CHECK-MESSAGES: :[[@LINE+1]]:5: warning: exception instantiated but not bound (did you intend to 'throw'?) [misc-throw-keyword-missing]
+RegularException(5);
+}
+
+void nameContainsExceptionThrown(int i) {
+  if (i < 0)
+throw RegularException();
+
+  if (i > 0)
+throw RegularException(5);
+}
+
+template 
+void f(int i, Exception excToBeThrown) {}
+
+void funcCallWithTempExcTest() {
+  f(5, RegularException());
+}
+
+// Global variable initilization test.
+RegularException exc = RegularException();
+RegularException *excptr = new RegularException();
+
+void localVariableInitTest() {
+  RegularException exc = RegularException();
+  RegularException *excptr = new RegularException();
+}
+
+class CtorInitializerListTest {
+  RegularException exc;
+
+  CtorInitializerListTest() : exc(RegularException()) {}
+
+  CtorInitializerListTest(int) try : exc(RegularException()) {
+//Constructor body
+  } catch (...) {
+//CHECK-MESSAGES: :[[@LINE+1]]:5: warning: exception instantiated but not bound (did you intend to 'throw'?) [misc-throw-keyword-missing]
+RegularException();
+  }
+
+  CtorInitializerListTest(float);
+};
+
+CtorInitializerListTest::CtorInitializerListTest(float) try : exc(RegularException()) {
+  //Constructor body
+} catch (...) {
+  //CHECK-MESSAGES: :[[@LINE+1]]:3: warning: exception instantiated but not bound (did you intend to 'throw'?) [misc-throw-keyword-missing]
+  RegularException();
+}
+
+RegularException funcReturningExceptioniTest(int i) {
+  return RegularException();
+}
+
+struct ClassBracedInitListTest {
+  ClassBracedInitListTest(RegularException exc) {}
+};
+
+void functionBracedInitListTest(RegularException, ClassBracedInitListTest) {}
+
+void bracedInitListTest() {
+  RegularException exc{};
+  ClassBracedInitListTest test = {RegularException()};
+  functionBracedInitListTest({}, {RegularException()});
+}
+
+typedef std::exception ERROR_BASE;
+class RegularError : public ERROR_BASE {};
+
+void typedefTest() {
+  //CHECK-MESSAGES: :[[@LINE+1]]:3: warning: exception instantiated but not b

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

[Whisperity via Phabricator via cfe-commits]

whisperity resigned from this revision.
whisperity added a comment.

Works for me but I haven't any sayings in these. 😇


https://reviews.llvm.org/D43120



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42581: [NVPTX] Emit debug info in DWARF-2 by default for Cuda devices.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Thanks, will commit it after the commit of the LLVM part


Repository:
  rC Clang

https://reviews.llvm.org/D42581



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43124: Improve ObjC headers detection

[Jacek Olesiak via Phabricator via cfe-commits]

jolesiak created this revision.
jolesiak added a reviewer: benhamilton.
Herald added subscribers: cfe-commits, klimek.

Improve ObjC headers detection by adding additional keywords.


Repository:
  rC Clang

https://reviews.llvm.org/D43124

Files:
  lib/Format/Format.cpp


Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1441,6 +1441,7 @@
 "NSArray",
 "NSAttributedString",
 "NSCache",
+"NSCalendar",
 "NSCharacterSet",
 "NSCountedSet",
 "NSData",
@@ -1466,6 +1467,7 @@
 "NSMutableString",
 "NSNumber",
 "NSNumberFormatter",
+"NSObject",
 "NSOrderedSet",
 "NSPoint",
 "NSPointerArray",
@@ -1475,11 +1477,13 @@
 "NSSet",
 "NSSize",
 "NSString",
+"NSTimeZone",
 "NSUInteger",
 "NSURL",
 "NSURLComponents",
 "NSURLQueryItem",
 "NSUUID",
+"NSValue",
 };
 
 for (auto &Line : AnnotatedLines) {


Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1441,6 +1441,7 @@
 "NSArray",
 "NSAttributedString",
 "NSCache",
+"NSCalendar",
 "NSCharacterSet",
 "NSCountedSet",
 "NSData",
@@ -1466,6 +1467,7 @@
 "NSMutableString",
 "NSNumber",
 "NSNumberFormatter",
+"NSObject",
 "NSOrderedSet",
 "NSPoint",
 "NSPointerArray",
@@ -1475,11 +1477,13 @@
 "NSSet",
 "NSSize",
 "NSString",
+"NSTimeZone",
 "NSUInteger",
 "NSURL",
 "NSURLComponents",
 "NSURLQueryItem",
 "NSUUID",
+"NSValue",
 };
 
 for (auto &Line : AnnotatedLines) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

[Daniel Jasper via Phabricator via cfe-commits]

djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.

Looks good.


Repository:
  rC Clang

https://reviews.llvm.org/D43121



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43127: [clangd] Stop exposing Futures from ClangdServer operations.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, ioeric, jkorous-apple, klimek.

LSP has asynchronous semantics, being able to block on an async operation
completing is unneccesary and leads to tighter coupling with the threading.

In practice only tests depend on this, so we add a general-purpose "block until
idle" function to the scheduler which will work for all operations.

To get this working, fix a latent condition-variable bug in ASTWorker, and make
AsyncTaskRunner const-correct.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43127

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  clangd/Threading.cpp
  clangd/Threading.h
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -120,7 +120,8 @@
   /*StorePreamblesInMemory=*/true);
   auto File = getVirtualTestFilePath("foo.cpp");
   Annotations Test(Text);
-  Server.addDocument(File, Test.code()).wait();
+  Server.addDocument(File, Test.code());
+  EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
   auto CompletionList =
   Server.codeComplete(File, Test.point(), Opts).get().Value;
   // Sanity-check that filterText is valid.
@@ -553,21 +554,24 @@
   void f() { ns::^; }
   void f() { ns::preamble().$2^; }
   )cpp");
-  Server.addDocument(File, Test.code()).wait();
+  Server.addDocument(File, Test.code());
+  ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
   clangd::CodeCompleteOptions Opts = {};
 
-  auto WithoutIndex = Server.codeComplete(File, Test.point(), Opts).get().Value;
-  EXPECT_THAT(WithoutIndex.items,
-  UnorderedElementsAre(Named("local"), Named("preamble")));
-
   auto I = memIndex({var("ns::index")});
   Opts.Index = I.get();
   auto WithIndex = Server.codeComplete(File, Test.point(), Opts).get().Value;
   EXPECT_THAT(WithIndex.items,
   UnorderedElementsAre(Named("local"), Named("index")));
   auto ClassFromPreamble =
   Server.codeComplete(File, Test.point("2"), Opts).get().Value;
   EXPECT_THAT(ClassFromPreamble.items, Contains(Named("member")));
+
+  Opts.Index = nullptr;
+  auto WithoutIndex = Server.codeComplete(File, Test.point(), Opts).get().Value;
+  EXPECT_THAT(WithoutIndex.items,
+  UnorderedElementsAre(Named("local"), Named("preamble")));
+
 }
 
 TEST(CompletionTest, DynamicIndexMultiFile) {
@@ -578,11 +582,10 @@
   /*StorePreamblesInMemory=*/true,
   /*BuildDynamicSymbolIndex=*/true);
 
-  Server
-  .addDocument(getVirtualTestFilePath("foo.cpp"), R"cpp(
+  Server.addDocument(getVirtualTestFilePath("foo.cpp"), R"cpp(
   namespace ns { class XYZ {}; void foo(int x) {} }
-  )cpp")
-  .wait();
+  )cpp");
+  ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
 
   auto File = getVirtualTestFilePath("bar.cpp");
   Annotations Test(R"cpp(
@@ -593,7 +596,8 @@
   }
   void f() { ns::^ }
   )cpp");
-  Server.addDocument(File, Test.code()).wait();
+  Server.addDocument(File, Test.code());
+  ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
 
   auto Results = Server.codeComplete(File, Test.point(), {}).get().Value;
   // "XYZ" and "foo" are not included in the file being completed but are still
@@ -623,6 +627,7 @@
   auto File = getVirtualTestFilePath("foo.cpp");
   Annotations Test(Text);
   Server.addDocument(File, Test.code());
+  EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
   auto R = Server.signatureHelp(File, Test.point());
   assert(R);
   return R.get().Value;
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -38,11 +38,6 @@
 
 namespace {
 
-// Don't wait for async ops in clangd test more than that to avoid blocking
-// indefinitely in case of bugs.
-static const std::chrono::seconds DefaultFutureTimeout =
-std::chrono::seconds(10);
-
 static bool diagsContainErrors(ArrayRef Diagnostics) {
   for (const auto &DiagAndFixIts : Diagnostics) {
 // FIXME: severities returned by clangd should have a descriptive
@@ -140,15 +135,9 @@
 
 FS.ExpectedFile = SourceFilename;
 
-// Have to sync reparses because requests are processed on the calling
-// thread.
-auto AddDocFuture = Server.addDocument(SourceFilename, SourceContents);
-
+Server.addDocument(SourceFilename, SourceContents);
 auto Result = dumpASTWithoutMemoryLocs(Server, SourceFilename);
-
-// Wait for reparse to finish before checking for errors.
-EXPECT_EQ(AddDocFuture.wait_for(DefaultFutureTimeout),
- 

[PATCH] D43124: Improve ObjC headers detection

[Jacek Olesiak via Phabricator via cfe-commits]

jolesiak updated this revision to Diff 133608.
jolesiak added a comment.

Add NSBundle.


Repository:
  rC Clang

https://reviews.llvm.org/D43124

Files:
  lib/Format/Format.cpp


Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1440,7 +1440,9 @@
 "NSAffineTransform",
 "NSArray",
 "NSAttributedString",
+"NSBundle",
 "NSCache",
+"NSCalendar",
 "NSCharacterSet",
 "NSCountedSet",
 "NSData",
@@ -1466,6 +1468,7 @@
 "NSMutableString",
 "NSNumber",
 "NSNumberFormatter",
+"NSObject",
 "NSOrderedSet",
 "NSPoint",
 "NSPointerArray",
@@ -1475,11 +1478,13 @@
 "NSSet",
 "NSSize",
 "NSString",
+"NSTimeZone",
 "NSUInteger",
 "NSURL",
 "NSURLComponents",
 "NSURLQueryItem",
 "NSUUID",
+"NSValue",
 };
 
 for (auto &Line : AnnotatedLines) {


Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1440,7 +1440,9 @@
 "NSAffineTransform",
 "NSArray",
 "NSAttributedString",
+"NSBundle",
 "NSCache",
+"NSCalendar",
 "NSCharacterSet",
 "NSCountedSet",
 "NSData",
@@ -1466,6 +1468,7 @@
 "NSMutableString",
 "NSNumber",
 "NSNumberFormatter",
+"NSObject",
 "NSOrderedSet",
 "NSPoint",
 "NSPointerArray",
@@ -1475,11 +1478,13 @@
 "NSSet",
 "NSSize",
 "NSString",
+"NSTimeZone",
 "NSUInteger",
 "NSURL",
 "NSURLComponents",
 "NSURLQueryItem",
 "NSUUID",
+"NSValue",
 };
 
 for (auto &Line : AnnotatedLines) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43127: [clangd] Stop exposing Futures from ClangdServer operations.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 133609.
sammccall added a comment.

Tidy up comment, and revert notify_all to notify_one - it was a red herring.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43127

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  clangd/Threading.cpp
  clangd/Threading.h
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -120,7 +120,8 @@
   /*StorePreamblesInMemory=*/true);
   auto File = getVirtualTestFilePath("foo.cpp");
   Annotations Test(Text);
-  Server.addDocument(File, Test.code()).wait();
+  Server.addDocument(File, Test.code());
+  EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
   auto CompletionList =
   Server.codeComplete(File, Test.point(), Opts).get().Value;
   // Sanity-check that filterText is valid.
@@ -553,21 +554,24 @@
   void f() { ns::^; }
   void f() { ns::preamble().$2^; }
   )cpp");
-  Server.addDocument(File, Test.code()).wait();
+  Server.addDocument(File, Test.code());
+  ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
   clangd::CodeCompleteOptions Opts = {};
 
-  auto WithoutIndex = Server.codeComplete(File, Test.point(), Opts).get().Value;
-  EXPECT_THAT(WithoutIndex.items,
-  UnorderedElementsAre(Named("local"), Named("preamble")));
-
   auto I = memIndex({var("ns::index")});
   Opts.Index = I.get();
   auto WithIndex = Server.codeComplete(File, Test.point(), Opts).get().Value;
   EXPECT_THAT(WithIndex.items,
   UnorderedElementsAre(Named("local"), Named("index")));
   auto ClassFromPreamble =
   Server.codeComplete(File, Test.point("2"), Opts).get().Value;
   EXPECT_THAT(ClassFromPreamble.items, Contains(Named("member")));
+
+  Opts.Index = nullptr;
+  auto WithoutIndex = Server.codeComplete(File, Test.point(), Opts).get().Value;
+  EXPECT_THAT(WithoutIndex.items,
+  UnorderedElementsAre(Named("local"), Named("preamble")));
+
 }
 
 TEST(CompletionTest, DynamicIndexMultiFile) {
@@ -578,11 +582,10 @@
   /*StorePreamblesInMemory=*/true,
   /*BuildDynamicSymbolIndex=*/true);
 
-  Server
-  .addDocument(getVirtualTestFilePath("foo.cpp"), R"cpp(
+  Server.addDocument(getVirtualTestFilePath("foo.cpp"), R"cpp(
   namespace ns { class XYZ {}; void foo(int x) {} }
-  )cpp")
-  .wait();
+  )cpp");
+  ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
 
   auto File = getVirtualTestFilePath("bar.cpp");
   Annotations Test(R"cpp(
@@ -593,7 +596,8 @@
   }
   void f() { ns::^ }
   )cpp");
-  Server.addDocument(File, Test.code()).wait();
+  Server.addDocument(File, Test.code());
+  ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
 
   auto Results = Server.codeComplete(File, Test.point(), {}).get().Value;
   // "XYZ" and "foo" are not included in the file being completed but are still
@@ -623,6 +627,7 @@
   auto File = getVirtualTestFilePath("foo.cpp");
   Annotations Test(Text);
   Server.addDocument(File, Test.code());
+  EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
   auto R = Server.signatureHelp(File, Test.point());
   assert(R);
   return R.get().Value;
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -38,11 +38,6 @@
 
 namespace {
 
-// Don't wait for async ops in clangd test more than that to avoid blocking
-// indefinitely in case of bugs.
-static const std::chrono::seconds DefaultFutureTimeout =
-std::chrono::seconds(10);
-
 static bool diagsContainErrors(ArrayRef Diagnostics) {
   for (const auto &DiagAndFixIts : Diagnostics) {
 // FIXME: severities returned by clangd should have a descriptive
@@ -140,15 +135,9 @@
 
 FS.ExpectedFile = SourceFilename;
 
-// Have to sync reparses because requests are processed on the calling
-// thread.
-auto AddDocFuture = Server.addDocument(SourceFilename, SourceContents);
-
+Server.addDocument(SourceFilename, SourceContents);
 auto Result = dumpASTWithoutMemoryLocs(Server, SourceFilename);
-
-// Wait for reparse to finish before checking for errors.
-EXPECT_EQ(AddDocFuture.wait_for(DefaultFutureTimeout),
-  std::future_status::ready);
+EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
 EXPECT_EQ(ExpectErrors, DiagConsumer.hadErrorInLastDiags());
 return Result;
   }
@@ -208,25 +197,19 @@
   FS.Files[FooCpp] = SourceContents;
   FS.ExpectedFile = FooCpp;
 
-  // To sync reparses before checking for errors.
-  std::future ParseFuture;
-
-  ParseFuture = Serve

[PATCH] D42942: [clangd] Collect definitions when indexing.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked an inline comment as done.
sammccall added inline comments.



Comment at: clangd/index/SymbolCollector.cpp:210
+  BasicSymbol = addDeclaration(*ND, std::move(ID));
+if (Roles & static_cast(index::SymbolRole::Definition))
+  addDefinition(*cast(ASTNode.OrigD), *BasicSymbol);

ioeric wrote:
> sammccall wrote:
> > hokein wrote:
> > > ioeric wrote:
> > > > It seems that we store definition even if it's the same as declaration, 
> > > > which might be true for most classes? This is probably fine, but it 
> > > > would be worth documenting. Or maybe consider not storing the same 
> > > > location twice?
> > > I think it is fine to store the same location twice. We can't tell 
> > > whether the CanonicalLoc is a definition or not.
> > Documented that these may be the same.
> > We wouldn't actually save any memory by avoiding saving this twice - the 
> > filename is a stringref to the same data, and the offsets get stored even 
> > if they're null.
> I'm less concerned about space. I think this would also make it easy to check 
> whether a symbol has a separate definition (e.g. we might want to do this for 
> go-to-definition/go-to-declaration). A comparison would also work but seems 
> less natural. But I'm not sure, up tp you :)
Yeah sometimes we'll definitely want to check if they're different, but other 
times we'll want to use one without caring if they are. Comparison seems less 
surprising than either of the fields sometimes being empty even though data is 
available - that seems likely to lead to bugs I think.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42942



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35894: [clangd] Code hover for Clangd

[Simon Marchi via Phabricator via cfe-commits]

simark added a comment.

Another example where pretty-printing the AST gives better results:

  int x, y = 5, z = 6;

Hover the `z` will now show `int z = 6`, before it would have shown `int x, y = 
5, z = 6`.  I think new version is better because it only shows the variable we 
care about.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D35894



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r324735 - [clangd] Collect definitions when indexing.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri Feb  9 06:42:01 2018
New Revision: 324735

URL: http://llvm.org/viewvc/llvm-project?rev=324735&view=rev
Log:
[clangd] Collect definitions when indexing.

Within a TU:
 - as now, collect a declaration from the first occurrence of a symbol
   (taking clang's canonical declaration)
 - when we first see a definition occurrence, copy the symbol and add it
Across TUs/sources:
 - mergeSymbol in Merge.h is responsible for combining matching Symbols.
   This covers dynamic/static merges and cross-TU merges in the static index.
 - it prefers declarations from Symbols that have a definition.
 - GlobalSymbolBuilderMain is modified to use mergeSymbol as a reduce step.
Random cleanups (can be pulled out):
 - SymbolFromYAML -> SymbolsFromYAML, new singular SymbolFromYAML added
 - avoid uninit'd SymbolLocations. Add an idiomatic way to check "absent".
 - CanonicalDeclaration (as well as Definition) are mapped as optional in YAML.
 - added operator<< for Symbol & SymbolLocation, for debugging

Reviewers: ioeric, hokein

Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits

Differential Revision: https://reviews.llvm.org/D42942

Modified:

clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/Merge.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.h
clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
clang-tools-extra/trunk/clangd/index/SymbolYAML.h
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp

Modified: 
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp?rev=324735&r1=324734&r2=324735&view=diff
==
--- 
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
 (original)
+++ 
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
 Fri Feb  9 06:42:01 2018
@@ -14,9 +14,11 @@
 //===-===//
 
 #include "index/Index.h"
+#include "index/Merge.h"
 #include "index/SymbolCollector.h"
 #include "index/SymbolYAML.h"
 #include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Index/IndexDataConsumer.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Tooling/CommonOptionsParser.h"
@@ -34,6 +36,7 @@ using clang::clangd::SymbolSlab;
 
 namespace clang {
 namespace clangd {
+namespace {
 
 static llvm::cl::opt AssumedHeaderDir(
 "assume-header-dir",
@@ -91,6 +94,25 @@ public:
   tooling::ExecutionContext *Ctx;
 };
 
+// Combine occurrences of the same symbol across translation units.
+SymbolSlab mergeSymbols(tooling::ToolResults *Results) {
+  SymbolSlab::Builder UniqueSymbols;
+  llvm::BumpPtrAllocator Arena;
+  Symbol::Details Scratch;
+  Results->forEachResult([&](llvm::StringRef Key, llvm::StringRef Value) {
+Arena.Reset();
+auto Sym = clang::clangd::SymbolFromYAML(Value, Arena);
+clang::clangd::SymbolID ID;
+Key >> ID;
+if (const auto *Existing = UniqueSymbols.find(ID))
+  UniqueSymbols.insert(mergeSymbol(*Existing, Sym, &Scratch));
+else
+  UniqueSymbols.insert(Sym);
+  });
+  return std::move(UniqueSymbols).build();
+}
+
+} // namespace
 } // namespace clangd
 } // namespace clang
 
@@ -115,6 +137,7 @@ int main(int argc, const char **argv) {
 return 1;
   }
 
+  // Map phase: emit symbols found in each translation unit.
   auto Err = Executor->get()->execute(
   llvm::make_unique(
   Executor->get()->getExecutionContext()));
@@ -122,22 +145,11 @@ int main(int argc, const char **argv) {
 llvm::errs() << llvm::toString(std::move(Err)) << "\n";
   }
 
-  // Deduplicate the result by key and keep the longest value.
-  // FIXME(ioeric): Merge occurrences, rather than just dropping all but one.
-  // Definitions and forward declarations have the same key and may both have
-  // information. Usage count will need to be aggregated across occurrences,
-  // too.
-  llvm::StringMap UniqueSymbols;
-  Executor->get()->getToolResults()->forEachResult(
-  [&UniqueSymbols](llvm::StringRef Key, llvm::StringRef Value) {
-auto Ret = UniqueSymbols.try_emplace(Key, Value);
-if (!Ret.second) {
-  // If key already exists, keep the longest value.
-  llvm::StringRef &V = Ret.first->second;
-  V = V.size() < Value.size() ? Value : V;
-}
-  });
-  for (const auto &Sym : UniqueSymbols)
-llvm::outs() << Sym.second;
+  // Reduc

[PATCH] D42942: [clangd] Collect definitions when indexing.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL324735: [clangd] Collect definitions when indexing. 
(authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D42942?vs=133502&id=133612#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D42942

Files:
  
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
  clang-tools-extra/trunk/clangd/index/Index.cpp
  clang-tools-extra/trunk/clangd/index/Index.h
  clang-tools-extra/trunk/clangd/index/Merge.cpp
  clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
  clang-tools-extra/trunk/clangd/index/SymbolCollector.h
  clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
  clang-tools-extra/trunk/clangd/index/SymbolYAML.h
  clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
  clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
  clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp

Index: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
@@ -47,12 +47,17 @@
 }
 MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
 MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.FileURI == P; }
-MATCHER_P(LocationOffsets, Offsets, "") {
+MATCHER_P(DeclRange, Offsets, "") {
   // Offset range in SymbolLocation is [start, end] while in Clangd is [start,
   // end).
+  // FIXME: SymbolLocation should be [start, end).
   return arg.CanonicalDeclaration.StartOffset == Offsets.first &&
   arg.CanonicalDeclaration.EndOffset == Offsets.second - 1;
 }
+MATCHER_P(DefRange, Offsets, "") {
+  return arg.Definition.StartOffset == Offsets.first &&
+ arg.Definition.EndOffset == Offsets.second - 1;
+}
 
 namespace clang {
 namespace clangd {
@@ -97,23 +102,18 @@
 auto Factory = llvm::make_unique(CollectorOpts);
 
 std::vector Args = {"symbol_collector", "-fsyntax-only",
- "-std=c++11", TestFileName};
+ "-std=c++11",   "-include",
+ TestHeaderName, TestFileName};
 Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
 tooling::ToolInvocation Invocation(
 Args,
 Factory->create(), Files.get(),
 std::make_shared());
 
 InMemoryFileSystem->addFile(TestHeaderName, 0,
 llvm::MemoryBuffer::getMemBuffer(HeaderCode));
-
-std::string Content = MainCode;
-if (!HeaderCode.empty())
-  Content = (llvm::Twine("#include\"") +
- llvm::sys::path::filename(TestHeaderName) + "\"\n" + Content)
-.str();
 InMemoryFileSystem->addFile(TestFileName, 0,
-llvm::MemoryBuffer::getMemBuffer(Content));
+llvm::MemoryBuffer::getMemBuffer(MainCode));
 Invocation.run();
 Symbols = Factory->Collector->takeSymbols();
 return true;
@@ -176,6 +176,42 @@
QName("foo::bar::v2"), QName("foo::baz")}));
 }
 
+TEST_F(SymbolCollectorTest, Locations) {
+  // FIXME: the behavior here is bad: chopping tokens, including more than the
+  // ident range, using half-open ranges. See fixmes in getSymbolLocation().
+  CollectorOpts.IndexMainFiles = true;
+  Annotations Header(R"cpp(
+// Declared in header, defined in main.
+$xdecl[[extern int X]];
+$clsdecl[[class C]]ls;
+$printdecl[[void print()]];
+
+// Declared in header, defined nowhere.
+$zdecl[[extern int Z]];
+  )cpp");
+  Annotations Main(R"cpp(
+$xdef[[int X = 4]]2;
+$clsdef[[class Cls {}]];
+$printdef[[void print() {}]]
+
+// Declared/defined in main only.
+$y[[int Y]];
+  )cpp");
+  runSymbolCollector(Header.code(), Main.code());
+  EXPECT_THAT(
+  Symbols,
+  UnorderedElementsAre(
+  AllOf(QName("X"), DeclRange(Header.offsetRange("xdecl")),
+DefRange(Main.offsetRange("xdef"))),
+  AllOf(QName("Cls"), DeclRange(Header.offsetRange("clsdecl")),
+DefRange(Main.offsetRange("clsdef"))),
+  AllOf(QName("print"), DeclRange(Header.offsetRange("printdecl")),
+DefRange(Main.offsetRange("printdef"))),
+  AllOf(QName("Z"), DeclRange(Header.offsetRange("zdecl"))),
+  AllOf(QName("Y"), DeclRange(Main.offsetRange("y")),
+DefRange(Main.offsetRange("y");
+}
+
 TEST_F(SymbolCollectorTest, SymbolRelativeNoFallback) {
   CollectorOpts.IndexMainFiles = false;
   runSymbolCollector("class Foo {};", /*Main=*/"");
@@ -280,10 +316,9 @@
   EXPECT_THAT(
   Symbols,
   UnorderedElementsAre(
-  AllOf(QName("abc_Test"),
-LocationOffsets(Header.offsetRange("expa

[PATCH] D42942: [clangd] Collect definitions when indexing.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE324735: [clangd] Collect definitions when indexing. 
(authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D42942?vs=133502&id=133613#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D42942

Files:
  clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/Merge.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  clangd/index/SymbolYAML.cpp
  clangd/index/SymbolYAML.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/IndexTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -248,6 +248,28 @@
   EXPECT_EQ(M.Detail->Documentation, "--doc--");
 }
 
+TEST(MergeTest, PreferSymbolWithDefn) {
+  Symbol L, R;
+  Symbol::Details Scratch;
+
+  L.ID = R.ID = SymbolID("hello");
+  L.CanonicalDeclaration.FileURI = "file:/left.h";
+  R.CanonicalDeclaration.FileURI = "file:/right.h";
+  L.CompletionPlainInsertText = "left-insert";
+  R.CompletionPlainInsertText = "right-insert";
+
+  Symbol M = mergeSymbol(L, R, &Scratch);
+  EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:/left.h");
+  EXPECT_EQ(M.Definition.FileURI, "");
+  EXPECT_EQ(M.CompletionPlainInsertText, "left-insert");
+
+  R.Definition.FileURI = "file:/right.cpp"; // Now right will be favored.
+  M = mergeSymbol(L, R, &Scratch);
+  EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:/right.h");
+  EXPECT_EQ(M.Definition.FileURI, "file:/right.cpp");
+  EXPECT_EQ(M.CompletionPlainInsertText, "right-insert");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -47,12 +47,17 @@
 }
 MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
 MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.FileURI == P; }
-MATCHER_P(LocationOffsets, Offsets, "") {
+MATCHER_P(DeclRange, Offsets, "") {
   // Offset range in SymbolLocation is [start, end] while in Clangd is [start,
   // end).
+  // FIXME: SymbolLocation should be [start, end).
   return arg.CanonicalDeclaration.StartOffset == Offsets.first &&
   arg.CanonicalDeclaration.EndOffset == Offsets.second - 1;
 }
+MATCHER_P(DefRange, Offsets, "") {
+  return arg.Definition.StartOffset == Offsets.first &&
+ arg.Definition.EndOffset == Offsets.second - 1;
+}
 
 namespace clang {
 namespace clangd {
@@ -97,23 +102,18 @@
 auto Factory = llvm::make_unique(CollectorOpts);
 
 std::vector Args = {"symbol_collector", "-fsyntax-only",
- "-std=c++11", TestFileName};
+ "-std=c++11",   "-include",
+ TestHeaderName, TestFileName};
 Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
 tooling::ToolInvocation Invocation(
 Args,
 Factory->create(), Files.get(),
 std::make_shared());
 
 InMemoryFileSystem->addFile(TestHeaderName, 0,
 llvm::MemoryBuffer::getMemBuffer(HeaderCode));
-
-std::string Content = MainCode;
-if (!HeaderCode.empty())
-  Content = (llvm::Twine("#include\"") +
- llvm::sys::path::filename(TestHeaderName) + "\"\n" + Content)
-.str();
 InMemoryFileSystem->addFile(TestFileName, 0,
-llvm::MemoryBuffer::getMemBuffer(Content));
+llvm::MemoryBuffer::getMemBuffer(MainCode));
 Invocation.run();
 Symbols = Factory->Collector->takeSymbols();
 return true;
@@ -176,6 +176,42 @@
QName("foo::bar::v2"), QName("foo::baz")}));
 }
 
+TEST_F(SymbolCollectorTest, Locations) {
+  // FIXME: the behavior here is bad: chopping tokens, including more than the
+  // ident range, using half-open ranges. See fixmes in getSymbolLocation().
+  CollectorOpts.IndexMainFiles = true;
+  Annotations Header(R"cpp(
+// Declared in header, defined in main.
+$xdecl[[extern int X]];
+$clsdecl[[class C]]ls;
+$printdecl[[void print()]];
+
+// Declared in header, defined nowhere.
+$zdecl[[extern int Z]];
+  )cpp");
+  Annotations Main(R"cpp(
+$xdef[[int X = 4]]2;
+$clsdef[[class Cls {}]];
+$printdef[[void print() {}]]
+
+// Declared/defined in main only.
+$y[[int Y]];
+  )cpp");
+  runSymbolCollector(Header.code(), Main.code());
+  EXPECT_THAT(
+  Symbols,
+  UnorderedElementsAre(
+  AllOf(QName("X"), DeclRange(Header.offsetRange("xdecl")),
+DefRange(Main.offsetRange("xdef"))),
+  AllOf

[PATCH] D5767: Template Instantiation Observer + a few other templight-related changes

[Ábel Sinkovics via Phabricator via cfe-commits]

sabel83 updated this revision to Diff 133614.

https://reviews.llvm.org/D5767

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/FrontendActions.h
  include/clang/Frontend/FrontendOptions.h
  include/clang/FrontendTool/Utils.h
  include/clang/Sema/Sema.h
  include/clang/Sema/TemplateInstCallback.h
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendActions.cpp
  lib/FrontendTool/ExecuteCompilerInvocation.cpp
  lib/Parse/ParseAST.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaTemplateInstantiate.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/SemaType.cpp
  test/Templight/templight-deduced-func.cpp
  test/Templight/templight-default-arg-inst.cpp
  test/Templight/templight-default-func-arg.cpp
  test/Templight/templight-default-template-arg.cpp
  test/Templight/templight-exception-spec-func.cpp
  test/Templight/templight-explicit-template-arg.cpp
  test/Templight/templight-memoization.cpp
  test/Templight/templight-nested-memoization.cpp
  test/Templight/templight-nested-template-instantiation.cpp
  test/Templight/templight-one-instantiation.cpp
  test/Templight/templight-prior-template-arg.cpp

Index: test/Templight/templight-prior-template-arg.cpp
===
--- test/Templight/templight-prior-template-arg.cpp
+++ test/Templight/templight-prior-template-arg.cpp
@@ -0,0 +1,72 @@
+// RUN: %clang_cc1 -templight-dump %s 2>&1 | FileCheck %s
+template
+class A {};
+
+template  class Outer>
+class B {};
+
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+'B::Outer'$}}
+// CHECK: {{^kind:[ ]+PriorTemplateArgumentSubstitution$}}
+// CHECK: {{^event:[ ]+Begin$}}
+// CHECK: {{^orig:[ ]+'.*templight-prior-template-arg.cpp:5:40'}}
+// CHECK: {{^poi:[ ]+'.*templight-prior-template-arg.cpp:72:1'$}}
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+'B::Outer'$}}
+// CHECK: {{^kind:[ ]+PriorTemplateArgumentSubstitution$}}
+// CHECK: {{^event:[ ]+End$}}
+// CHECK: {{^orig:[ ]+'.*templight-prior-template-arg.cpp:5:40'}}
+// CHECK: {{^poi:[ ]+'.*templight-prior-template-arg.cpp:72:1'$}}
+//
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+'B'$}}
+// CHECK: {{^kind:[ ]+TemplateInstantiation$}}
+// CHECK: {{^event:[ ]+Begin$}}
+// CHECK: {{^orig:[ ]+'.*templight-prior-template-arg.cpp:6:7'}}
+// CHECK: {{^poi:[ ]+'.*templight-prior-template-arg.cpp:72:6'$}}
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+'B'$}}
+// CHECK: {{^kind:[ ]+TemplateInstantiation$}}
+// CHECK: {{^event:[ ]+End$}}
+// CHECK: {{^orig:[ ]+'.*templight-prior-template-arg.cpp:6:7'}}
+// CHECK: {{^poi:[ ]+'.*templight-prior-template-arg.cpp:72:6'$}}
+//
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+'B'$}}
+// CHECK: {{^kind:[ ]+TemplateInstantiation$}}
+// CHECK: {{^event:[ ]+Begin$}}
+// CHECK: {{^orig:[ ]+'.*templight-prior-template-arg.cpp:6:7'}}
+// CHECK: {{^poi:[ ]+'.*templight-prior-template-arg.cpp:72:6'$}}
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+'B'$}}
+// CHECK: {{^kind:[ ]+TemplateInstantiation$}}
+// CHECK: {{^event:[ ]+End$}}
+// CHECK: {{^orig:[ ]+'.*templight-prior-template-arg.cpp:6:7'}}
+// CHECK: {{^poi:[ ]+'.*templight-prior-template-arg.cpp:72:6'$}}
+//
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+'B'$}}
+// CHECK: {{^kind:[ ]+Memoization$}}
+// CHECK: {{^event:[ ]+Begin$}}
+// CHECK: {{^orig:[ ]+'.*templight-prior-template-arg.cpp:6:7'}}
+// CHECK: {{^poi:[ ]+'.*templight-prior-template-arg.cpp:72:6'$}}
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+'B'$}}
+// CHECK: {{^kind:[ ]+Memoization$}}
+// CHECK: {{^event:[ ]+End$}}
+// CHECK: {{^orig:[ ]+'.*templight-prior-template-arg.cpp:6:7'}}
+// CHECK: {{^poi:[ ]+'.*templight-prior-template-arg.cpp:72:6'$}}
+//
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+'B'$}}
+// CHECK: {{^kind:[ ]+Memoization$}}
+// CHECK: {{^event:[ ]+Begin$}}
+// CHECK: {{^orig:[ ]+'.*templight-prior-template-arg.cpp:6:7'}}
+// CHECK: {{^poi:[ ]+'.*templight-prior-template-arg.cpp:72:6'$}}
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+'B'$}}
+// CHECK: {{^kind:[ ]+Memoization$}}
+// CHECK: {{^event:[ ]+End$}}
+// CHECK: {{^orig:[ ]+'.*templight-prior-template-arg.cpp:6:7'}}
+// CHECK: {{^poi:[ ]+'.*templight-prior-template-arg.cpp:72:6'$}}
+B b;
Index: test/Templight/templight-one-instantiation.cpp
===
--- test/Templight/templight-one-instantiation.cpp
+++ test/Templight/templight-one-instantiation.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -templight-dump %s 2>&1 | FileCheck %s
+
+template 
+struct foo {};
+
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+'foo'$}}
+// CHECK: {{^kind:[ ]+TemplateInstantiation$}}
+// CHECK: {{^event:[ ]+Begin$}}
+// CHECK: {{^orig:[ ]+'.*templight-one-instantiation.cpp:4:8'}}
+// CHECK: {{^poi:[ ]+'.*templight-one-instantiation.cpp:18:10'$}}
+// CHECK-LABEL: {{^---$}}
+// CHECK: {{^name:[ ]+'foo'$}}
+// CHECK: {{^kind:[ ]+TemplateInstantiation$}}
+// CHECK: {{^event:[ ]+End$}}
+// CHECK: {{^orig:[ ]+'.*te

[PATCH] D5767: Template Instantiation Observer + a few other templight-related changes

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added a comment.

Thanks, this looks good to me! I will try this out soon and commit after that.


https://reviews.llvm.org/D5767



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43127: [clangd] Stop exposing Futures from ClangdServer operations.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 133615.
sammccall added a comment.

rebase


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43127

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  clangd/Threading.cpp
  clangd/Threading.h
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -120,7 +120,8 @@
   /*StorePreamblesInMemory=*/true);
   auto File = getVirtualTestFilePath("foo.cpp");
   Annotations Test(Text);
-  Server.addDocument(File, Test.code()).wait();
+  Server.addDocument(File, Test.code());
+  EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
   auto CompletionList =
   Server.codeComplete(File, Test.point(), Opts).get().Value;
   // Sanity-check that filterText is valid.
@@ -553,21 +554,24 @@
   void f() { ns::^; }
   void f() { ns::preamble().$2^; }
   )cpp");
-  Server.addDocument(File, Test.code()).wait();
+  Server.addDocument(File, Test.code());
+  ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
   clangd::CodeCompleteOptions Opts = {};
 
-  auto WithoutIndex = Server.codeComplete(File, Test.point(), Opts).get().Value;
-  EXPECT_THAT(WithoutIndex.items,
-  UnorderedElementsAre(Named("local"), Named("preamble")));
-
   auto I = memIndex({var("ns::index")});
   Opts.Index = I.get();
   auto WithIndex = Server.codeComplete(File, Test.point(), Opts).get().Value;
   EXPECT_THAT(WithIndex.items,
   UnorderedElementsAre(Named("local"), Named("index")));
   auto ClassFromPreamble =
   Server.codeComplete(File, Test.point("2"), Opts).get().Value;
   EXPECT_THAT(ClassFromPreamble.items, Contains(Named("member")));
+
+  Opts.Index = nullptr;
+  auto WithoutIndex = Server.codeComplete(File, Test.point(), Opts).get().Value;
+  EXPECT_THAT(WithoutIndex.items,
+  UnorderedElementsAre(Named("local"), Named("preamble")));
+
 }
 
 TEST(CompletionTest, DynamicIndexMultiFile) {
@@ -578,11 +582,10 @@
   /*StorePreamblesInMemory=*/true,
   /*BuildDynamicSymbolIndex=*/true);
 
-  Server
-  .addDocument(getVirtualTestFilePath("foo.cpp"), R"cpp(
+  Server.addDocument(getVirtualTestFilePath("foo.cpp"), R"cpp(
   namespace ns { class XYZ {}; void foo(int x) {} }
-  )cpp")
-  .wait();
+  )cpp");
+  ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
 
   auto File = getVirtualTestFilePath("bar.cpp");
   Annotations Test(R"cpp(
@@ -593,7 +596,8 @@
   }
   void f() { ns::^ }
   )cpp");
-  Server.addDocument(File, Test.code()).wait();
+  Server.addDocument(File, Test.code());
+  ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
 
   auto Results = Server.codeComplete(File, Test.point(), {}).get().Value;
   // "XYZ" and "foo" are not included in the file being completed but are still
@@ -623,6 +627,7 @@
   auto File = getVirtualTestFilePath("foo.cpp");
   Annotations Test(Text);
   Server.addDocument(File, Test.code());
+  EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
   auto R = Server.signatureHelp(File, Test.point());
   assert(R);
   return R.get().Value;
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -39,11 +39,6 @@
 
 namespace {
 
-// Don't wait for async ops in clangd test more than that to avoid blocking
-// indefinitely in case of bugs.
-static const std::chrono::seconds DefaultFutureTimeout =
-std::chrono::seconds(10);
-
 static bool diagsContainErrors(ArrayRef Diagnostics) {
   for (const auto &DiagAndFixIts : Diagnostics) {
 // FIXME: severities returned by clangd should have a descriptive
@@ -141,15 +136,9 @@
 
 FS.ExpectedFile = SourceFilename;
 
-// Have to sync reparses because requests are processed on the calling
-// thread.
-auto AddDocFuture = Server.addDocument(SourceFilename, SourceContents);
-
+Server.addDocument(SourceFilename, SourceContents);
 auto Result = dumpASTWithoutMemoryLocs(Server, SourceFilename);
-
-// Wait for reparse to finish before checking for errors.
-EXPECT_EQ(AddDocFuture.wait_for(DefaultFutureTimeout),
-  std::future_status::ready);
+EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
 EXPECT_EQ(ExpectErrors, DiagConsumer.hadErrorInLastDiags());
 return Result;
   }
@@ -209,25 +198,19 @@
   FS.Files[FooCpp] = SourceContents;
   FS.ExpectedFile = FooCpp;
 
-  // To sync reparses before checking for errors.
-  std::future ParseFuture;
-
-  ParseFuture = Server.addDocument(FooCpp, SourceContents);
+  Server.addDocument(FooCpp, S

[PATCH] D41698: [DebugInfo] Enable debug information for C99 VLA types

[Carlos Alberto Enciso via Phabricator via cfe-commits]

CarlosAlbertoEnciso added a comment.

Hi @sdesmalen!

For the following test case

  int main() {
int size = 2;
  
int var[size];
var[1] = 1;
  
return 0;
  }

I compared the DWARF generated by GCC and it looks like

  DW_TAG_variable "var"
DW_AT_location ...
DW_AT_type DW_FORM_ref4
  DW_TAG_array_type 
DW_AT_type -> "int"
DW_TAG_subrange_type 
  DW_AT_type -> "sizetype"
  DW_AT_upper_bound DW_FORM_exprloc [4] = { DW_OP_fbreg 0xffb8 
DW_OP_deref }

GCC use DW_AT_upper_bound with an associated location expression to describe 
the VLA boundaries.

In order to reduce the side effects created by the artifical-variable as 
described in my previous comment and to keep the generated DWARF within a 
reasonable size, I would suggest the GCC aproach as a size optimization.

The DWARF description of the artificial-variable could be removed and its 
location expression used by the array's subrange_type, instead of the 
subrange_type making a reference to the artificial-variable.


Repository:
  rL LLVM

https://reviews.llvm.org/D41698



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35894: [clangd] Code hover for Clangd

[Simon Marchi via Phabricator via cfe-commits]

simark added a comment.

The only problem I see is that when hovering a function name, it now prints the 
whole function definition.  When talking with @malaperle, he told me that you 
had discussed it before and we should not have the definition in the hover, 
just the prototype.  Glancing quickly at the PrintingPolicy, I don't see a way 
to just print the prototype of the function.  Do you have any idea?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D35894



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r324736 - [clangd] Fix crash in tests in debug mode.

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Fri Feb  9 07:11:07 2018
New Revision: 324736

URL: http://llvm.org/viewvc/llvm-project?rev=324736&view=rev
Log:
[clangd] Fix crash in tests in debug mode.

Caused by the lack of checking of an Expected value in the previous
commit.

Modified:
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp

Modified: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp?rev=324736&r1=324735&r2=324736&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Fri Feb  9 
07:11:07 2018
@@ -483,9 +483,9 @@ TEST_F(ClangdVFSTest, InvalidCompileComm
   .get()
   .Value.items,
   IsEmpty());
-  EXPECT_THAT(
-  Server.signatureHelp(FooCpp, Position{0, 0}).get().Value.signatures,
-  IsEmpty());
+  auto SigHelp = Server.signatureHelp(FooCpp, Position{0, 0});
+  ASSERT_TRUE(bool(SigHelp)) << "signatureHelp returned an error";
+  EXPECT_THAT(SigHelp->Value.signatures, IsEmpty());
 }
 
 class ClangdThreadingTest : public ClangdVFSTest {};


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Insertion side LGTM, feel free to split and land.
Sorry I need to take off and will need to get to indexing on monday :(




Comment at: clangd/ClangdServer.cpp:368
 
+/// Calculates the shortest possible include path when inserting \p Header to 
\p
+/// File, by matching \p Header against all include search directories for \p

ioeric wrote:
> sammccall wrote:
> > This has a fair amount of logic (well, plumbing :-D) and uses relatively 
> > little from ClangdServer.
> > Can we move `shortenIncludePath` to a separate file and pass in the FS and 
> > command?
> > 
> > I'd suggest maybe `Headers.h` - The formatting part of `insertInclude` 
> > could also fit there, as well as probably the logic from 
> > `switchSourceHeader` I think (the latter not in this patch of course).
> > 
> > This won't really help much with testing, I just find it gets hard to 
> > navigate files that have lots of details of unrelated features. ClangdUnit 
> > and ClangdServer both have this potential to grow without bound, though 
> > they're in reasonable shape now. Interested what you think!
> Done.
> 
> I didn't move the formatting code, as half of the code is pulling out the 
> style, which we might want to share/change depending on other clangd logics 
> that might use style.
I'd still think pulling out `Expected insertInclude(File, 
Code, Header, VFS, Style)` would be worthwhile here - the formatting isn't a 
lot of code, but it's a bit magic, plus the quote handling... it's a bit of 
code. It'd make it more obvious what the interactions with ClangdServer's state 
are. But up to you, we can always do this later.



Comment at: clangd/ClangdServer.cpp:370
+/// File, by matching \p Header against all include search directories for \p
+/// File via `clang::HeaderSearch`.
+///

sammccall wrote:
> maybe drop "via clang::HeaderSearch" (it's doc'd in the implementation) and 
> add an example?
> 
> It might be worth explicitly stating that the result is an #include string 
> (with quotes) - you just call it a "path" here.
> 
> "shortest" makes sense as a first implementation, but we may want to document 
> something more like "best" - I know there are codebases we care about where 
> file-relative paths are banned. Also I suspect we don't give 
> "../../../../Foo.h" even if it's shortest :-)
> "shortest" makes sense as a first implementation, but we may want to document 
> something more like "best" - I know there are codebases we care about where 
> file-relative paths are banned. Also I suspect we don't give 
> "../../../../Foo.h" even if it's shortest :-)

I think this part wasn't addressed



Comment at: clangd/Headers.cpp:74
+
+  // We don't need to provide the content of \p File, as we are only interested
+  // in the include search directories in the compile command.

comment is outdated now



Comment at: clangd/Headers.cpp:114
+
+  log("Suggested #include is: " + Suggested);
+  return Suggested;

can you include the Header in this log message? (and possibly File, but that 
might add more noise than signal)



Comment at: clangd/Headers.h:30
+/// \return A quoted "path" or . If \p Header is already (directly)
+/// included in the file (including those included via different paths), an
+/// error will be returned.

header-already-included is not an error condition.

Suggest returning llvm::Expected>, or returning "" for this 
case.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42640



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42840: [docs] Fix duplicate arguments for JoinedAndSeparate

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

Ping


Repository:
  rC Clang

https://reviews.llvm.org/D42840



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35894: [clangd] Code hover for Clangd

[Simon Marchi via Phabricator via cfe-commits]

simark added a comment.

In https://reviews.llvm.org/D35894#1003342, @simark wrote:

> The only problem I see is that when hovering a function/struct name, it now 
> prints the whole function/struct definition.  When talking with @malaperle, 
> he told me that you had discussed it before and we should not have the 
> definition in the hover, just the prototype for a function and the name (e.g. 
> "struct foo") for a struct.  Glancing quickly at the PrintingPolicy, I don't 
> see a way to do that.  Do you have any idea?


Arrg, Sorry, I just re-read your comment and saw that you used TerseOutput 
which does that.  I did not catch that when transcribing the code (I did not 
want to copy paste to understand the code better, but got bitten).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D35894



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

One concern I have is with RAII objects with "exception" in the name. You may 
already properly handle this, but I'd like to see a test case like:

  struct ExceptionRAII {
ExceptionRAII() {}
~ExceptionRAII() {}
  };
  
  void foo() {
ExceptionRAII E; // Don't trigger on this
  }




Comment at: clang-tidy/misc/ThrowKeywordMissingCheck.cpp:45
+  diag(TemporaryExpr->getLocStart(),
+   "exception instantiated but not bound (did you intend to 'throw'?)");
+}

I'm not keen on the wording here ("bound" isn't a common phrase for 
exceptions). How about `"suspicious exception object created but not thrown; 
did you mean 'throw %0'"` and then pass in the text for the object construction?



Comment at: docs/clang-tidy/checks/misc-throw-keyword-missing.rst:6
+
+This check warns about the potentially missing `throw` keyword. If a temporary 
object is created,
+but the object's type derives from (or the same as) a class that has 
'EXCEPTION', 'Exception' or

about the potentially -> about a potentially



Comment at: docs/clang-tidy/checks/misc-throw-keyword-missing.rst:7
+This check warns about the potentially missing `throw` keyword. If a temporary 
object is created,
+but the object's type derives from (or the same as) a class that has 
'EXCEPTION', 'Exception' or
+'exception' in its name, we can assume that the programmer's intention was to 
throw that object.

or the same as -> or is the same as



Comment at: docs/clang-tidy/checks/misc-throw-keyword-missing.rst:14
+  void f(int i) {
+if(i < 0) {
+  // Exception is created but is not thrown

Can you format the code snippet?


https://reviews.llvm.org/D43120



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 133619.
Typz added a comment.

rebase on latest master.


Repository:
  rC Clang

https://reviews.llvm.org/D43121

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -693,8 +693,8 @@
   // Formats pair-parameters.
   verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
   verifyFormat("[I drawRectOn:surface //\n"
-   "ofSize:aa:bbb\n"
-   "  atOrigin:cc:dd];");
+   "   ofSize:aa:bbb\n"
+   " atOrigin:cc:dd];");
 
   // Inline block as a first argument.
   verifyFormat("[object justBlock:^{\n"
@@ -760,6 +760,26 @@
   "  backing:NSBackingStoreBuffered\n"
   "defer:NO]);\n"
   "}");
+
+  // Respect continuation indent and colon alignment (e.g. when object name is
+  // short, and first selector is the longest one)
+  Style = getLLVMStyle();
+  Style.Language = FormatStyle::LK_ObjC;
+  Style.ContinuationIndentWidth = 8;
+  verifyFormat("[self performSelectorOnMainThread:@selector(loadAccessories)\n"
+   "   withObject:nil\n"
+   "waitUntilDone:false];");
+  verifyFormat("[self performSelector:@selector(loadAccessories)\n"
+   "withObjectOnMainThread:nil\n"
+   " waitUntilDone:false];");
+  verifyFormat("[a\n"
+   "
performSelectorOnMainThread:@selector(loadAccessories)\n"
+   " withObject:nil\n"
+   "  waitUntilDone:false];");
+  verifyFormat("[self // force wrapping\n"
+   "
performSelectorOnMainThread:@selector(loadAccessories)\n"
+   " withObject:nil\n"
+   "  waitUntilDone:false];");
 }
 
 TEST_F(FormatTestObjC, ObjCAt) {
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -591,12 +591,12 @@
 BeforePrevious->is(tok::r_square) ||
 Contexts.back().LongestObjCSelectorName == 0) {
   Tok->Previous->Type = TT_SelectorName;
-  if (Tok->Previous->ColumnWidth >
-  Contexts.back().LongestObjCSelectorName)
-Contexts.back().LongestObjCSelectorName =
-Tok->Previous->ColumnWidth;
   if (!Contexts.back().FirstObjCSelectorName)
 Contexts.back().FirstObjCSelectorName = Tok->Previous;
+  else if (Tok->Previous->ColumnWidth >
+   Contexts.back().LongestObjCSelectorName)
+Contexts.back().LongestObjCSelectorName =
+Tok->Previous->ColumnWidth;
 }
   } else if (Contexts.back().ColonIsForRangeExpr) {
 Tok->Type = TT_RangeBasedForLoopColon;
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -701,7 +701,8 @@
  ? std::max(State.Stack.back().Indent,
 State.FirstIndent + Style.ContinuationIndentWidth)
  : State.Stack.back().Indent) +
-NextNonComment->LongestObjCSelectorName;
+std::max(NextNonComment->LongestObjCSelectorName,
+ NextNonComment->ColumnWidth);
   }
 } else if (State.Stack.back().AlignColons &&
State.Stack.back().ColonPos <= NextNonComment->ColumnWidth) {
@@ -900,7 +901,8 @@
   ? std::max(State.Stack.back().Indent,
  State.FirstIndent + Style.ContinuationIndentWidth)
   : State.Stack.back().Indent) +
- NextNonComment->LongestObjCSelectorName -
+ std::max(NextNonComment->LongestObjCSelectorName,
+  NextNonComment->ColumnWidth) -
  NextNonComment->ColumnWidth;
 }
 if (!State.Stack.back().AlignColons)


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -693,8 +693,8 @@
   // Formats pair-parameters.
   verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
   verifyFormat("[I drawRectOn:surface //\n"
-   "ofSize:aa:bbb\n"
-   "  atOrigin:cc:dd];");
+   "   ofSize:aa:bbb\n"
+   " atOrigin:cc:dd];");
 
   // Inline block as a first argument.
   verifyFormat("[object justBlock:^{\n"
@@ -760,6 +760,26 @

r324741 - clang-format: keep ObjC colon alignment with short object name

[Francois Ferrand via cfe-commits]

Author: typz
Date: Fri Feb  9 07:41:56 2018
New Revision: 324741

URL: http://llvm.org/viewvc/llvm-project?rev=324741&view=rev
Log:
clang-format: keep ObjC colon alignment with short object name

Summary:
When the target object expression is short and the first selector name
is long, clang-format used to break the colon alignment:

  [I performSelectorOnMainThread:@selector(loadAccessories)
   withObject:nil
waitUntilDone:false];

This happens because the colon is placed at `ContinuationIndent +
LongestObjCSelectorName`, so that any selector can be wrapped. This is
however not needed in case the longest selector is the firstone, and
not wrapped.

To overcome this, this patch does not include the first selector in
`LongestObjCSelectorName` computation (in TokenAnnotator), and lets
`ContinuationIndenter` decide how to account for the first selector
when wrapping. (Note this was already partly the case, see line 521
of ContinuationIndenter.cpp)

This way, the code gets properly aligned whenever possible without
breaking the continuation indent.

  [I performSelectorOnMainThread:@selector(loadAccessories)
  withObject:nil
   waitUntilDone:false];
  [I // force break
  performSelectorOnMainThread:@selector(loadAccessories)
   withObject:nil
waitUntilDone:false];
  [I perform:@selector(loadAccessories)
  withSelectorOnMainThread:true
 waitUntilDone:false];

Reviewers: krasimir, djasper, klimek

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D43121

Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestObjC.cpp

Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=324741&r1=324740&r2=324741&view=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Fri Feb  9 07:41:56 2018
@@ -701,7 +701,8 @@ unsigned ContinuationIndenter::addTokenO
  ? std::max(State.Stack.back().Indent,
 State.FirstIndent + Style.ContinuationIndentWidth)
  : State.Stack.back().Indent) +
-NextNonComment->LongestObjCSelectorName;
+std::max(NextNonComment->LongestObjCSelectorName,
+ NextNonComment->ColumnWidth);
   }
 } else if (State.Stack.back().AlignColons &&
State.Stack.back().ColonPos <= NextNonComment->ColumnWidth) {
@@ -900,7 +901,8 @@ unsigned ContinuationIndenter::getNewLin
   ? std::max(State.Stack.back().Indent,
  State.FirstIndent + Style.ContinuationIndentWidth)
   : State.Stack.back().Indent) +
- NextNonComment->LongestObjCSelectorName -
+ std::max(NextNonComment->LongestObjCSelectorName,
+  NextNonComment->ColumnWidth) -
  NextNonComment->ColumnWidth;
 }
 if (!State.Stack.back().AlignColons)

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=324741&r1=324740&r2=324741&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Feb  9 07:41:56 2018
@@ -591,12 +591,12 @@ private:
 BeforePrevious->is(tok::r_square) ||
 Contexts.back().LongestObjCSelectorName == 0) {
   Tok->Previous->Type = TT_SelectorName;
-  if (Tok->Previous->ColumnWidth >
-  Contexts.back().LongestObjCSelectorName)
-Contexts.back().LongestObjCSelectorName =
-Tok->Previous->ColumnWidth;
   if (!Contexts.back().FirstObjCSelectorName)
 Contexts.back().FirstObjCSelectorName = Tok->Previous;
+  else if (Tok->Previous->ColumnWidth >
+   Contexts.back().LongestObjCSelectorName)
+Contexts.back().LongestObjCSelectorName =
+Tok->Previous->ColumnWidth;
 }
   } else if (Contexts.back().ColonIsForRangeExpr) {
 Tok->Type = TT_RangeBasedForLoopColon;

Modified: cfe/trunk/unittests/Format/FormatTestObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestObjC.cpp?rev=324741&r1=324740&r2=324741&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp Fri Feb  9 07:41:56 2018
@@ -693,8 +693,8 @@ TEST_F(FormatTestObjC, FormatObjCMethodE
   // Formats pair-parameters.
   verifyFormat("[I drawRectOn:s

[PATCH] D43121: clang-format: keep ObjC colon alignment with short object name

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC324741: clang-format: keep ObjC colon alignment with short 
object name (authored by Typz, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D43121?vs=133619&id=133620#toc

Repository:
  rC Clang

https://reviews.llvm.org/D43121

Files:
  lib/Format/ContinuationIndenter.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -693,8 +693,8 @@
   // Formats pair-parameters.
   verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
   verifyFormat("[I drawRectOn:surface //\n"
-   "ofSize:aa:bbb\n"
-   "  atOrigin:cc:dd];");
+   "   ofSize:aa:bbb\n"
+   " atOrigin:cc:dd];");
 
   // Inline block as a first argument.
   verifyFormat("[object justBlock:^{\n"
@@ -760,6 +760,26 @@
   "  backing:NSBackingStoreBuffered\n"
   "defer:NO]);\n"
   "}");
+
+  // Respect continuation indent and colon alignment (e.g. when object name is
+  // short, and first selector is the longest one)
+  Style = getLLVMStyle();
+  Style.Language = FormatStyle::LK_ObjC;
+  Style.ContinuationIndentWidth = 8;
+  verifyFormat("[self performSelectorOnMainThread:@selector(loadAccessories)\n"
+   "   withObject:nil\n"
+   "waitUntilDone:false];");
+  verifyFormat("[self performSelector:@selector(loadAccessories)\n"
+   "withObjectOnMainThread:nil\n"
+   " waitUntilDone:false];");
+  verifyFormat("[a\n"
+   "
performSelectorOnMainThread:@selector(loadAccessories)\n"
+   " withObject:nil\n"
+   "  waitUntilDone:false];");
+  verifyFormat("[self // force wrapping\n"
+   "
performSelectorOnMainThread:@selector(loadAccessories)\n"
+   " withObject:nil\n"
+   "  waitUntilDone:false];");
 }
 
 TEST_F(FormatTestObjC, ObjCAt) {
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -591,12 +591,12 @@
 BeforePrevious->is(tok::r_square) ||
 Contexts.back().LongestObjCSelectorName == 0) {
   Tok->Previous->Type = TT_SelectorName;
-  if (Tok->Previous->ColumnWidth >
-  Contexts.back().LongestObjCSelectorName)
-Contexts.back().LongestObjCSelectorName =
-Tok->Previous->ColumnWidth;
   if (!Contexts.back().FirstObjCSelectorName)
 Contexts.back().FirstObjCSelectorName = Tok->Previous;
+  else if (Tok->Previous->ColumnWidth >
+   Contexts.back().LongestObjCSelectorName)
+Contexts.back().LongestObjCSelectorName =
+Tok->Previous->ColumnWidth;
 }
   } else if (Contexts.back().ColonIsForRangeExpr) {
 Tok->Type = TT_RangeBasedForLoopColon;
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -701,7 +701,8 @@
  ? std::max(State.Stack.back().Indent,
 State.FirstIndent + Style.ContinuationIndentWidth)
  : State.Stack.back().Indent) +
-NextNonComment->LongestObjCSelectorName;
+std::max(NextNonComment->LongestObjCSelectorName,
+ NextNonComment->ColumnWidth);
   }
 } else if (State.Stack.back().AlignColons &&
State.Stack.back().ColonPos <= NextNonComment->ColumnWidth) {
@@ -900,7 +901,8 @@
   ? std::max(State.Stack.back().Indent,
  State.FirstIndent + Style.ContinuationIndentWidth)
   : State.Stack.back().Indent) +
- NextNonComment->LongestObjCSelectorName -
+ std::max(NextNonComment->LongestObjCSelectorName,
+  NextNonComment->ColumnWidth) -
  NextNonComment->ColumnWidth;
 }
 if (!State.Stack.back().AlignColons)


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -693,8 +693,8 @@
   // Formats pair-parameters.
   verifyFormat("[I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];");
   verifyFormat("[I drawRectOn:surface //\n"
-   "ofSize:aa:bbb\n"
-   "  atOrigin:cc:d

[PATCH] D43124: [clang-format] Improve ObjC headers detection

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton accepted this revision.
benhamilton added a comment.
This revision is now accepted and ready to land.

testcase 



Repository:
  rC Clang

https://reviews.llvm.org/D43124



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.



Comment at: lib/Format/UnwrappedLineParser.cpp:1130
+if (FormatTok->Tok.is(tok::l_brace)) {
+  if (Style.BraceWrapping.AfterObjCDeclaration)
+addUnwrappedLine();

Typz wrote:
> Wondering if formatting with this style is appropriate: the clang-format doc 
> points in that direction, but it seems to me both `@synchronized` and 
> `@autoreleasepool` are more akin to "control statements".
> 
> For consistency (esp. in ObjC++ code), as a user, I would tend to have these 
> blocks indented like control statements while interfaces/implementations 
> blocks would be indented like classes/structs.
> 
> So maybe it would be better to introduce a new BraceWrapping style 
> 'AfterObjCSpecialBlock` to control these cases, matching the possibilities 
> that are given for control statements vs classes. What do you think?
Hmm, I definitely agree with that logic. I don't see them acting as 
declarations in any way, they are definitely like control statements.



Repository:
  rC Clang

https://reviews.llvm.org/D43114



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43075: [clang-move] Don't dump macro symbols.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 133621.
hokein marked an inline comment as done.
hokein added a comment.

Add more tests.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43075

Files:
  clang-move/ClangMove.cpp
  unittests/clang-move/ClangMoveTests.cpp


Index: unittests/clang-move/ClangMoveTests.cpp
===
--- unittests/clang-move/ClangMoveTests.cpp
+++ unittests/clang-move/ClangMoveTests.cpp
@@ -391,6 +391,26 @@
   }
 }
 
+TEST(ClangMove, IgnoreMacroSymbolsAndMoveAll) {
+  const char TestCode[] = "#include \"foo.h\"";
+  std::vector TestHeaders = {
+"#define DEFINE_Foo int Foo = 1;\nDEFINE_Foo;\nclass Bar {};\n",
+"#define DEFINE(x) int var_##x = 1;\nDEFINE(foo);\nclass Bar {};\n",
+  };
+  move::MoveDefinitionSpec Spec;
+  Spec.Names.push_back("Bar");
+  Spec.OldHeader = "foo.h";
+  Spec.OldCC = "foo.cc";
+  Spec.NewHeader = "new_foo.h";
+  Spec.NewCC = "new_foo.cc";
+
+  for (const auto& Header : TestHeaders) {
+auto Results = runClangMoveOnCode(Spec, Header.c_str(), TestCode);
+EXPECT_EQ("", Results[Spec.OldHeader]);
+EXPECT_EQ(Header, Results[Spec.NewHeader]);
+  }
+}
+
 TEST(ClangMove, MacroInFunction) {
   const char TestHeader[] = "#define INT int\n"
 "class A {\npublic:\n  int f();\n};\n"
@@ -570,7 +590,9 @@
 "extern int kGlobalInt;\n"
 "extern const char* const kGlobalStr;\n"
 "} // namespace b\n"
-"} // namespace a\n";
+"} // namespace a\n"
+"#define DEFINE_FOO class Foo {};\n"
+"DEFINE_FOO\n";
   const char TestCode[] = "#include \"foo.h\"\n";
   move::MoveDefinitionSpec Spec;
   Spec.Names.push_back("B");
Index: clang-move/ClangMove.cpp
===
--- clang-move/ClangMove.cpp
+++ clang-move/ClangMove.cpp
@@ -523,6 +523,7 @@
   auto AllDeclsInHeader = namedDecl(
   unless(ForwardClassDecls), unless(namespaceDecl()),
   unless(usingDirectiveDecl()), // using namespace decl.
+  notInMacro(),
   InOldHeader,
   hasParent(decl(anyOf(namespaceDecl(), translationUnitDecl(,
   hasDeclContext(decl(anyOf(namespaceDecl(), translationUnitDecl();
@@ -905,10 +906,9 @@
 
   if (RemovedDecls.empty())
 return;
-  // Ignore symbols that are not supported (e.g. typedef and enum) when
-  // checking if there is unremoved symbol in old header. This makes sure that
-  // we always move old files to new files when all symbols produced from
-  // dump_decls are moved.
+  // Ignore symbols that are not supported when checking if there is unremoved
+  // symbol in old header. This makes sure that we always move old files to new
+  // files when all symbols produced from dump_decls are moved.
   auto IsSupportedKind = [](const clang::NamedDecl *Decl) {
 switch (Decl->getKind()) {
 case Decl::Kind::Function:


Index: unittests/clang-move/ClangMoveTests.cpp
===
--- unittests/clang-move/ClangMoveTests.cpp
+++ unittests/clang-move/ClangMoveTests.cpp
@@ -391,6 +391,26 @@
   }
 }
 
+TEST(ClangMove, IgnoreMacroSymbolsAndMoveAll) {
+  const char TestCode[] = "#include \"foo.h\"";
+  std::vector TestHeaders = {
+"#define DEFINE_Foo int Foo = 1;\nDEFINE_Foo;\nclass Bar {};\n",
+"#define DEFINE(x) int var_##x = 1;\nDEFINE(foo);\nclass Bar {};\n",
+  };
+  move::MoveDefinitionSpec Spec;
+  Spec.Names.push_back("Bar");
+  Spec.OldHeader = "foo.h";
+  Spec.OldCC = "foo.cc";
+  Spec.NewHeader = "new_foo.h";
+  Spec.NewCC = "new_foo.cc";
+
+  for (const auto& Header : TestHeaders) {
+auto Results = runClangMoveOnCode(Spec, Header.c_str(), TestCode);
+EXPECT_EQ("", Results[Spec.OldHeader]);
+EXPECT_EQ(Header, Results[Spec.NewHeader]);
+  }
+}
+
 TEST(ClangMove, MacroInFunction) {
   const char TestHeader[] = "#define INT int\n"
 "class A {\npublic:\n  int f();\n};\n"
@@ -570,7 +590,9 @@
 "extern int kGlobalInt;\n"
 "extern const char* const kGlobalStr;\n"
 "} // namespace b\n"
-"} // namespace a\n";
+"} // namespace a\n"
+"#define DEFINE_FOO class Foo {};\n"
+"DEFINE_FOO\n";
   const char TestCode[] = "#include \"foo.h\"\n";
   move::MoveDefinitionSpec Spec;
   Spec.Names.push_back("B");
Index: clang-move/ClangMove.cpp
===
--- clang-move/ClangMove.cpp
+++ clang-move/ClangMove.cpp
@@ -523,6 +523,7 @@
   auto AllDeclsInHeader = namedDecl(
   unless(ForwardClassDecls), unless(namespaceDecl()),
   unless(usingDirectiveDecl()), // using namespace 

[clang-tools-extra] r324742 - [clang-move] Don't dump macro symbols.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Fri Feb  9 07:57:30 2018
New Revision: 324742

URL: http://llvm.org/viewvc/llvm-project?rev=324742&view=rev
Log:
[clang-move] Don't dump macro symbols.

Reviewers: ioeric

Subscribers: klimek, cfe-commits

Differential Revision: https://reviews.llvm.org/D43075

Modified:
clang-tools-extra/trunk/clang-move/ClangMove.cpp
clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp

Modified: clang-tools-extra/trunk/clang-move/ClangMove.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-move/ClangMove.cpp?rev=324742&r1=324741&r2=324742&view=diff
==
--- clang-tools-extra/trunk/clang-move/ClangMove.cpp (original)
+++ clang-tools-extra/trunk/clang-move/ClangMove.cpp Fri Feb  9 07:57:30 2018
@@ -523,6 +523,7 @@ void ClangMoveTool::registerMatchers(ast
   auto AllDeclsInHeader = namedDecl(
   unless(ForwardClassDecls), unless(namespaceDecl()),
   unless(usingDirectiveDecl()), // using namespace decl.
+  notInMacro(),
   InOldHeader,
   hasParent(decl(anyOf(namespaceDecl(), translationUnitDecl(,
   hasDeclContext(decl(anyOf(namespaceDecl(), translationUnitDecl();
@@ -905,10 +906,9 @@ void ClangMoveTool::onEndOfTranslationUn
 
   if (RemovedDecls.empty())
 return;
-  // Ignore symbols that are not supported (e.g. typedef and enum) when
-  // checking if there is unremoved symbol in old header. This makes sure that
-  // we always move old files to new files when all symbols produced from
-  // dump_decls are moved.
+  // Ignore symbols that are not supported when checking if there is unremoved
+  // symbol in old header. This makes sure that we always move old files to new
+  // files when all symbols produced from dump_decls are moved.
   auto IsSupportedKind = [](const clang::NamedDecl *Decl) {
 switch (Decl->getKind()) {
 case Decl::Kind::Function:

Modified: clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp?rev=324742&r1=324741&r2=324742&view=diff
==
--- clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clang-move/ClangMoveTests.cpp Fri Feb  9 
07:57:30 2018
@@ -391,6 +391,26 @@ TEST(ClangMove, DontMoveAll) {
   }
 }
 
+TEST(ClangMove, IgnoreMacroSymbolsAndMoveAll) {
+  const char TestCode[] = "#include \"foo.h\"";
+  std::vector TestHeaders = {
+"#define DEFINE_Foo int Foo = 1;\nDEFINE_Foo;\nclass Bar {};\n",
+"#define DEFINE(x) int var_##x = 1;\nDEFINE(foo);\nclass Bar {};\n",
+  };
+  move::MoveDefinitionSpec Spec;
+  Spec.Names.push_back("Bar");
+  Spec.OldHeader = "foo.h";
+  Spec.OldCC = "foo.cc";
+  Spec.NewHeader = "new_foo.h";
+  Spec.NewCC = "new_foo.cc";
+
+  for (const auto& Header : TestHeaders) {
+auto Results = runClangMoveOnCode(Spec, Header.c_str(), TestCode);
+EXPECT_EQ("", Results[Spec.OldHeader]);
+EXPECT_EQ(Header, Results[Spec.NewHeader]);
+  }
+}
+
 TEST(ClangMove, MacroInFunction) {
   const char TestHeader[] = "#define INT int\n"
 "class A {\npublic:\n  int f();\n};\n"
@@ -570,7 +590,9 @@ TEST(ClangMove, DumpDecls) {
 "extern int kGlobalInt;\n"
 "extern const char* const kGlobalStr;\n"
 "} // namespace b\n"
-"} // namespace a\n";
+"} // namespace a\n"
+"#define DEFINE_FOO class Foo {};\n"
+"DEFINE_FOO\n";
   const char TestCode[] = "#include \"foo.h\"\n";
   move::MoveDefinitionSpec Spec;
   Spec.Names.push_back("B");


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43075: [clang-move] Don't dump macro symbols.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-move/ClangMove.cpp:526
   unless(usingDirectiveDecl()), // using namespace decl.
+  notInMacro(),
   InOldHeader,

ioeric wrote:
> I'd probably relax the condition a bit; theoretically tools would be able to 
> handle entire identifiers that are either spelled in macro or passed in by 
> users. But it's probably rare. Might worth a `FIXME` though?
This mainly affects the dump behavior. Moving symbols spelled in macro is 
tricky, and we can't guarantee always doing right thing. Actually, clang-move 
supports it partially. For the test case of this patch, if you move the `Foo` 
(names=`Foo`), clang-move will move the expansion macro (`DEFINE_Foo;`), 
although `Foo` is not dumped.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43075



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43075: [clang-move] Don't dump macro symbols.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE324742: [clang-move] Don't dump macro symbols. 
(authored by hokein, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D43075?vs=133621&id=133623#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43075

Files:
  clang-move/ClangMove.cpp
  unittests/clang-move/ClangMoveTests.cpp


Index: unittests/clang-move/ClangMoveTests.cpp
===
--- unittests/clang-move/ClangMoveTests.cpp
+++ unittests/clang-move/ClangMoveTests.cpp
@@ -391,6 +391,26 @@
   }
 }
 
+TEST(ClangMove, IgnoreMacroSymbolsAndMoveAll) {
+  const char TestCode[] = "#include \"foo.h\"";
+  std::vector TestHeaders = {
+"#define DEFINE_Foo int Foo = 1;\nDEFINE_Foo;\nclass Bar {};\n",
+"#define DEFINE(x) int var_##x = 1;\nDEFINE(foo);\nclass Bar {};\n",
+  };
+  move::MoveDefinitionSpec Spec;
+  Spec.Names.push_back("Bar");
+  Spec.OldHeader = "foo.h";
+  Spec.OldCC = "foo.cc";
+  Spec.NewHeader = "new_foo.h";
+  Spec.NewCC = "new_foo.cc";
+
+  for (const auto& Header : TestHeaders) {
+auto Results = runClangMoveOnCode(Spec, Header.c_str(), TestCode);
+EXPECT_EQ("", Results[Spec.OldHeader]);
+EXPECT_EQ(Header, Results[Spec.NewHeader]);
+  }
+}
+
 TEST(ClangMove, MacroInFunction) {
   const char TestHeader[] = "#define INT int\n"
 "class A {\npublic:\n  int f();\n};\n"
@@ -570,7 +590,9 @@
 "extern int kGlobalInt;\n"
 "extern const char* const kGlobalStr;\n"
 "} // namespace b\n"
-"} // namespace a\n";
+"} // namespace a\n"
+"#define DEFINE_FOO class Foo {};\n"
+"DEFINE_FOO\n";
   const char TestCode[] = "#include \"foo.h\"\n";
   move::MoveDefinitionSpec Spec;
   Spec.Names.push_back("B");
Index: clang-move/ClangMove.cpp
===
--- clang-move/ClangMove.cpp
+++ clang-move/ClangMove.cpp
@@ -523,6 +523,7 @@
   auto AllDeclsInHeader = namedDecl(
   unless(ForwardClassDecls), unless(namespaceDecl()),
   unless(usingDirectiveDecl()), // using namespace decl.
+  notInMacro(),
   InOldHeader,
   hasParent(decl(anyOf(namespaceDecl(), translationUnitDecl(,
   hasDeclContext(decl(anyOf(namespaceDecl(), translationUnitDecl();
@@ -905,10 +906,9 @@
 
   if (RemovedDecls.empty())
 return;
-  // Ignore symbols that are not supported (e.g. typedef and enum) when
-  // checking if there is unremoved symbol in old header. This makes sure that
-  // we always move old files to new files when all symbols produced from
-  // dump_decls are moved.
+  // Ignore symbols that are not supported when checking if there is unremoved
+  // symbol in old header. This makes sure that we always move old files to new
+  // files when all symbols produced from dump_decls are moved.
   auto IsSupportedKind = [](const clang::NamedDecl *Decl) {
 switch (Decl->getKind()) {
 case Decl::Kind::Function:


Index: unittests/clang-move/ClangMoveTests.cpp
===
--- unittests/clang-move/ClangMoveTests.cpp
+++ unittests/clang-move/ClangMoveTests.cpp
@@ -391,6 +391,26 @@
   }
 }
 
+TEST(ClangMove, IgnoreMacroSymbolsAndMoveAll) {
+  const char TestCode[] = "#include \"foo.h\"";
+  std::vector TestHeaders = {
+"#define DEFINE_Foo int Foo = 1;\nDEFINE_Foo;\nclass Bar {};\n",
+"#define DEFINE(x) int var_##x = 1;\nDEFINE(foo);\nclass Bar {};\n",
+  };
+  move::MoveDefinitionSpec Spec;
+  Spec.Names.push_back("Bar");
+  Spec.OldHeader = "foo.h";
+  Spec.OldCC = "foo.cc";
+  Spec.NewHeader = "new_foo.h";
+  Spec.NewCC = "new_foo.cc";
+
+  for (const auto& Header : TestHeaders) {
+auto Results = runClangMoveOnCode(Spec, Header.c_str(), TestCode);
+EXPECT_EQ("", Results[Spec.OldHeader]);
+EXPECT_EQ(Header, Results[Spec.NewHeader]);
+  }
+}
+
 TEST(ClangMove, MacroInFunction) {
   const char TestHeader[] = "#define INT int\n"
 "class A {\npublic:\n  int f();\n};\n"
@@ -570,7 +590,9 @@
 "extern int kGlobalInt;\n"
 "extern const char* const kGlobalStr;\n"
 "} // namespace b\n"
-"} // namespace a\n";
+"} // namespace a\n"
+"#define DEFINE_FOO class Foo {};\n"
+"DEFINE_FOO\n";
   const char TestCode[] = "#include \"foo.h\"\n";
   move::MoveDefinitionSpec Spec;
   Spec.Names.push_back("B");
Index: clang-move/ClangMove.cpp
===
--- clang-move/ClangMove.cpp
+++ clang-move/ClangMove.cpp
@@ -523,6 +523,7 @@
   auto AllDeclsI

[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks

[Francois Ferrand via Phabricator via cfe-commits]

Typz added inline comments.



Comment at: lib/Format/UnwrappedLineParser.cpp:1130
+if (FormatTok->Tok.is(tok::l_brace)) {
+  if (Style.BraceWrapping.AfterObjCDeclaration)
+addUnwrappedLine();

benhamilton wrote:
> Typz wrote:
> > Wondering if formatting with this style is appropriate: the clang-format 
> > doc points in that direction, but it seems to me both `@synchronized` and 
> > `@autoreleasepool` are more akin to "control statements".
> > 
> > For consistency (esp. in ObjC++ code), as a user, I would tend to have 
> > these blocks indented like control statements while 
> > interfaces/implementations blocks would be indented like classes/structs.
> > 
> > So maybe it would be better to introduce a new BraceWrapping style 
> > 'AfterObjCSpecialBlock` to control these cases, matching the possibilities 
> > that are given for control statements vs classes. What do you think?
> Hmm, I definitely agree with that logic. I don't see them acting as 
> declarations in any way, they are definitely like control statements.
> 
Ok, i can change this. Two questions though:
* Should I do this in this patch, or a separate patch? (won't be a big change 
in any case, but it can still be seen as 2 separate issues/changes)
* Should I introduce a new BraceWrapping flag (`AfterObjCSpecialBlock`), or 
simply apply `AfterControlStatement` to these blocks?


Repository:
  rC Clang

https://reviews.llvm.org/D43114



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43012: [ASTImporter] Fix lexical DC for templated decls; support VarTemplatePartialSpecDecl

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin marked 2 inline comments as done.
a.sidorin added inline comments.



Comment at: lib/AST/ASTImporter.cpp:4296
   // Create the declaration that is being templated.
-  SourceLocation StartLoc = Importer.Import(DTemplated->getLocStart());
-  SourceLocation IdLoc = Importer.Import(DTemplated->getLocation());
-  TypeSourceInfo *TInfo = Importer.Import(DTemplated->getTypeSourceInfo());
-  VarDecl *D2Templated = VarDecl::Create(Importer.getToContext(), DC, StartLoc,
- IdLoc, Name.getAsIdentifierInfo(), T,
- TInfo, DTemplated->getStorageClass());
-  D2Templated->setAccess(DTemplated->getAccess());
-  
D2Templated->setQualifierInfo(Importer.Import(DTemplated->getQualifierLoc()));
-  D2Templated->setLexicalDeclContext(LexicalDC);
-
-  // Importer.Imported(DTemplated, D2Templated);
-  // LexicalDC->addDeclInternal(D2Templated);
-
-  // Merge the initializer.
-  if (ImportDefinition(DTemplated, D2Templated))
+  VarDecl *ToTemplated = 
dyn_cast_or_null(Importer.Import(DTemplated));
+  if (!ToTemplated)

xazax.hun wrote:
> `auto *` to not repeat type.
I usually prefer to keep the type if it doesn't give a large space win because 
it hurts readability a bit. From `VarDecl *`, we can instantly find the type; 
for `auto`, we have to look forward. (Yes, VarTemplatePartialSpecializationDecl 
has to be replaced immediately :) ).
Other issue is that QtCreator I use for development still doesn't have an 
autocompletion for auto types. However, LLVM says: "do use auto with 
initializers like cast(...)", so I'll change this.



Repository:
  rC Clang

https://reviews.llvm.org/D43012



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43012: [ASTImporter] Fix lexical DC for templated decls; support VarTemplatePartialSpecDecl

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin updated this revision to Diff 133626.
a.sidorin marked an inline comment as done.
a.sidorin added a comment.

Fix style issues found on review.


Repository:
  rC Clang

https://reviews.llvm.org/D43012

Files:
  lib/AST/ASTImporter.cpp
  test/ASTMerge/var-cpp/Inputs/var1.cpp
  test/ASTMerge/var-cpp/test.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -542,20 +542,32 @@
 
 TEST(ImportType, ImportTypeAliasTemplate) {
   MatchVerifier Verifier;
-  testImport("template "
- "struct dummy { static const int i = K; };"
- "template  using dummy2 = dummy;"
- "int declToImport() { return dummy2<3>::i; }",
- Lang_CXX11, "", Lang_CXX11, Verifier,
- functionDecl(
-   hasBody(
- compoundStmt(
-   has(
- returnStmt(
-   has(
- implicitCastExpr(
-   has(
- declRefExpr());
+  testImport(
+  "template "
+  "struct dummy { static const int i = K; };"
+  "template  using dummy2 = dummy;"
+  "int declToImport() { return dummy2<3>::i; }",
+  Lang_CXX11, "", Lang_CXX11, Verifier,
+  functionDecl(
+  hasBody(compoundStmt(
+  has(returnStmt(has(implicitCastExpr(has(declRefExpr(,
+  unless(hasAncestor(translationUnitDecl(has(typeAliasDecl()));
+}
+
+const internal::VariadicDynCastAllOfMatcher
+varTemplateSpecializationDecl;
+
+TEST(ImportDecl, ImportVarTemplate) {
+  MatchVerifier Verifier;
+  testImport(
+  "template "
+  "T pi = T(3.1415926535897932385L);"
+  "void declToImport() { pi; }",
+  Lang_CXX11, "", Lang_CXX11, Verifier,
+  functionDecl(
+  hasBody(has(declRefExpr(to(varTemplateSpecializationDecl(),
+  unless(hasAncestor(translationUnitDecl(has(varDecl(
+  hasName("pi"), unless(varTemplateSpecializationDecl();
 }
 
 TEST(ImportType, ImportPackExpansion) {
Index: test/ASTMerge/var-cpp/test.cpp
===
--- /dev/null
+++ test/ASTMerge/var-cpp/test.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -emit-pch -std=c++17 -o %t.1.ast %S/Inputs/var1.cpp
+// RUN: %clang_cc1 -std=c++17 -ast-merge %t.1.ast -fsyntax-only %s 2>&1
+
+static_assert(my_pi == (double)3.1415926535897932385L);
+static_assert(my_pi == '3');
+
+static_assert(Wrapper::my_const == 1.f);
+static_assert(Wrapper::my_const == nullptr);
+static_assert(Wrapper::my_const == a);
Index: test/ASTMerge/var-cpp/Inputs/var1.cpp
===
--- /dev/null
+++ test/ASTMerge/var-cpp/Inputs/var1.cpp
@@ -0,0 +1,17 @@
+
+template 
+constexpr T my_pi = T(3.1415926535897932385L);  // variable template
+
+template <> constexpr char my_pi = '3';   // variable template specialization
+
+template 
+struct Wrapper {
+  template  static constexpr U my_const = U(1);
+   // Variable template partial specialization with member variable.
+  template  static constexpr U *my_const = (U *)(0);
+};
+
+constexpr char a[] = "hello";
+
+template <> template <>
+constexpr const char *Wrapper::my_const = a;
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -23,7 +23,6 @@
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/Support/MemoryBuffer.h"
-#include 
 
 namespace clang {
   class ASTNodeImporter : public TypeVisitor,
@@ -1335,6 +1334,21 @@
   return false;
 }
 
+template <>
+bool ASTNodeImporter::ImportTemplateArgumentListInfo(
+const TemplateArgumentListInfo &From, TemplateArgumentListInfo &Result) {
+  return ImportTemplateArgumentListInfo(
+  From.getLAngleLoc(), From.getRAngleLoc(), From.arguments(), Result);
+}
+
+template <>
+bool ASTNodeImporter::ImportTemplateArgumentListInfo<
+ASTTemplateArgumentListInfo>(const ASTTemplateArgumentListInfo &From,
+ TemplateArgumentListInfo &Result) {
+  return ImportTemplateArgumentListInfo(From.LAngleLoc, From.RAngleLoc,
+From.arguments(), Result);
+}
+
 bool ASTNodeImporter::IsStructuralMatch(RecordDecl *FromRecord, 
 RecordDecl *ToRecord, bool Complain) {
   // Eliminate a potential failure point where we attempt to re-import
@@ -1655,10 +1669,8 @@
   SourceLocation StartL = Importer.Import(D->getLocStart());
   TypedefNameDecl *ToTypedef;
   if (IsAlias)
-ToTypedef = TypeAliasDecl::Create(Importer.getToContext(), DC,
-  StartL, Loc,
-  Name.getAsIdentifierInfo(),
-  

r324748 - AMDGPU: Update for datalayout change

[Matt Arsenault via cfe-commits]

Author: arsenm
Date: Fri Feb  9 08:58:41 2018
New Revision: 324748

URL: http://llvm.org/viewvc/llvm-project?rev=324748&view=rev
Log:
AMDGPU: Update for datalayout change

Modified:
cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
cfe/trunk/test/CodeGen/target-data.c
cfe/trunk/test/CodeGenOpenCL/amdgpu-env-amdgiz.cl

Modified: cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/AMDGPU.cpp?rev=324748&r1=324747&r2=324748&view=diff
==
--- cfe/trunk/lib/Basic/Targets/AMDGPU.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/AMDGPU.cpp Fri Feb  9 08:58:41 2018
@@ -33,12 +33,12 @@ static const char *const DataLayoutStrin
 "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5";
 
 static const char *const DataLayoutStringSIPrivateIsZero =
-"e-p:32:32-p1:64:64-p2:64:64-p3:32:32-p4:64:64-p5:32:32"
+"e-p:32:32-p1:64:64-p2:64:64-p3:32:32-p4:64:64-p5:32:32-p6:32:32"
 "-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128"
 "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64";
 
 static const char *const DataLayoutStringSIGenericIsZero =
-"e-p:64:64-p1:64:64-p2:64:64-p3:32:32-p4:32:32-p5:32:32"
+"e-p:64:64-p1:64:64-p2:64:64-p3:32:32-p4:32:32-p5:32:32-p6:32:32"
 "-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128"
 "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5";
 
@@ -144,7 +144,7 @@ const char *const AMDGPUTargetInfo::GCCR
   "s104", "s105", "s106", "s107", "s108", "s109", "s110", "s111", "s112",
   "s113", "s114", "s115", "s116", "s117", "s118", "s119", "s120", "s121",
   "s122", "s123", "s124", "s125", "s126", "s127", "exec", "vcc", "scc",
-  "m0", "flat_scratch", "exec_lo", "exec_hi", "vcc_lo", "vcc_hi", 
+  "m0", "flat_scratch", "exec_lo", "exec_hi", "vcc_lo", "vcc_hi",
   "flat_scratch_lo", "flat_scratch_hi"
 };
 

Modified: cfe/trunk/test/CodeGen/target-data.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-data.c?rev=324748&r1=324747&r2=324748&view=diff
==
--- cfe/trunk/test/CodeGen/target-data.c (original)
+++ cfe/trunk/test/CodeGen/target-data.c Fri Feb  9 08:58:41 2018
@@ -132,12 +132,12 @@
 
 // RUN: %clang_cc1 -triple amdgcn-unknown -target-cpu hawaii -o - -emit-llvm 
%s \
 // RUN: | FileCheck %s -check-prefix=R600SI
-// R600SI: target datalayout = 
"e-p:64:64-p1:64:64-p2:64:64-p3:32:32-p4:32:32-p5:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5"
+// R600SI: target datalayout = 
"e-p:64:64-p1:64:64-p2:64:64-p3:32:32-p4:32:32-p5:32:32-p6:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5"
 
 // Test default -target-cpu
 // RUN: %clang_cc1 -triple amdgcn-unknown -o - -emit-llvm %s \
 // RUN: | FileCheck %s -check-prefix=R600SIDefault
-// R600SIDefault: target datalayout = 
"e-p:64:64-p1:64:64-p2:64:64-p3:32:32-p4:32:32-p5:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5"
+// R600SIDefault: target datalayout = 
"e-p:64:64-p1:64:64-p2:64:64-p3:32:32-p4:32:32-p5:32:32-p6:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5"
 
 // RUN: %clang_cc1 -triple arm64-unknown -o - -emit-llvm %s | \
 // RUN: FileCheck %s -check-prefix=AARCH64

Modified: cfe/trunk/test/CodeGenOpenCL/amdgpu-env-amdgiz.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/amdgpu-env-amdgiz.cl?rev=324748&r1=324747&r2=324748&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/amdgpu-env-amdgiz.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/amdgpu-env-amdgiz.cl Fri Feb  9 08:58:41 2018
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 %s -O0 -triple amdgcn -emit-llvm -o - | FileCheck %s
 // RUN: %clang_cc1 %s -O0 -triple amdgcn---opencl -emit-llvm -o - | FileCheck 
%s
 
-// CHECK: target datalayout = 
"e-p:64:64-p1:64:64-p2:64:64-p3:32:32-p4:32:32-p5:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5"
+// CHECK: target datalayout = 
"e-p:64:64-p1:64:64-p2:64:64-p3:32:32-p4:32:32-p5:32:32-p6:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5"
 void foo(void) {}


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43128: Introduce an API for LLDB to compute the default module cache path

[Adrian Prantl via Phabricator via cfe-commits]

aprantl created this revision.
aprantl added reviewers: bruno, jingham.

LLDB creates Clang modules and had an incomplete copy of the clang Driver code 
that compute the -fmodule-cache-path. This patch makes the clang driver code 
accessible to LLDB.


https://reviews.llvm.org/D43128

Files:
  include/clang/Driver/Driver.h
  lib/Driver/ToolChains/Clang.cpp


Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2500,6 +2500,13 @@
 CmdArgs.push_back("-fno-math-builtin");
 }
 
+void Driver::getDefaultModuleCachePath(SmallVectorImpl &Result) {
+  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, Result);
+  llvm::sys::path::append(Result, "org.llvm.clang.");
+  appendUserToPath(Result);
+  llvm::sys::path::append(Result, "ModuleCache");
+}
+
 static void RenderModulesOptions(Compilation &C, const Driver &D,
  const ArgList &Args, const InputInfo &Input,
  const InputInfo &Output,
@@ -2560,10 +2567,7 @@
   llvm::sys::path::append(Path, "modules");
 } else if (Path.empty()) {
   // No module path was provided: use the default.
-  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, Path);
-  llvm::sys::path::append(Path, "org.llvm.clang.");
-  appendUserToPath(Path);
-  llvm::sys::path::append(Path, "ModuleCache");
+  Driver::getDefaultModuleCachePath(Path);
 }
 
 const char Arg[] = "-fmodules-cache-path=";
Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -572,6 +572,8 @@
   /// no extra characters remaining at the end.
   static bool GetReleaseVersion(StringRef Str,
 MutableArrayRef Digits);
+  /// Compute the default -fmodule-cache-path.
+  static void getDefaultModuleCachePath(SmallVectorImpl &Result);
 };
 
 /// \return True if the last defined optimization level is -Ofast.


Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2500,6 +2500,13 @@
 CmdArgs.push_back("-fno-math-builtin");
 }
 
+void Driver::getDefaultModuleCachePath(SmallVectorImpl &Result) {
+  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, Result);
+  llvm::sys::path::append(Result, "org.llvm.clang.");
+  appendUserToPath(Result);
+  llvm::sys::path::append(Result, "ModuleCache");
+}
+
 static void RenderModulesOptions(Compilation &C, const Driver &D,
  const ArgList &Args, const InputInfo &Input,
  const InputInfo &Output,
@@ -2560,10 +2567,7 @@
   llvm::sys::path::append(Path, "modules");
 } else if (Path.empty()) {
   // No module path was provided: use the default.
-  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, Path);
-  llvm::sys::path::append(Path, "org.llvm.clang.");
-  appendUserToPath(Path);
-  llvm::sys::path::append(Path, "ModuleCache");
+  Driver::getDefaultModuleCachePath(Path);
 }
 
 const char Arg[] = "-fmodules-cache-path=";
Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -572,6 +572,8 @@
   /// no extra characters remaining at the end.
   static bool GetReleaseVersion(StringRef Str,
 MutableArrayRef Digits);
+  /// Compute the default -fmodule-cache-path.
+  static void getDefaultModuleCachePath(SmallVectorImpl &Result);
 };
 
 /// \return True if the last defined optimization level is -Ofast.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42895: [libclang] Add `CXSymbolRole role` to CXIdxEntityRefInfo

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 133632.
MaskRay added a comment.

Bring back refkind:


Repository:
  rC Clang

https://reviews.llvm.org/D42895

Files:
  include/clang-c/Index.h
  include/clang/Index/IndexSymbol.h
  test/Index/index-refs.cpp
  test/Index/index-subscripting-literals.m
  tools/c-index-test/c-index-test.c
  tools/libclang/CXIndexDataConsumer.cpp
  tools/libclang/CXIndexDataConsumer.h

Index: tools/libclang/CXIndexDataConsumer.h
===
--- tools/libclang/CXIndexDataConsumer.h
+++ tools/libclang/CXIndexDataConsumer.h
@@ -436,13 +436,15 @@
const NamedDecl *Parent,
const DeclContext *DC,
const Expr *E = nullptr,
-   CXIdxEntityRefKind Kind = CXIdxEntityRef_Direct);
+   CXIdxEntityRefKind Kind = CXIdxEntityRef_Direct,
+   CXSymbolRole Role = CXSymbolRole_None);
 
   bool handleReference(const NamedDecl *D, SourceLocation Loc,
const NamedDecl *Parent,
const DeclContext *DC,
const Expr *E = nullptr,
-   CXIdxEntityRefKind Kind = CXIdxEntityRef_Direct);
+   CXIdxEntityRefKind Kind = CXIdxEntityRef_Direct,
+   CXSymbolRole Role = CXSymbolRole_None);
 
   bool isNotFromSourceFile(SourceLocation Loc) const;
 
Index: tools/libclang/CXIndexDataConsumer.cpp
===
--- tools/libclang/CXIndexDataConsumer.cpp
+++ tools/libclang/CXIndexDataConsumer.cpp
@@ -148,6 +148,11 @@
 return true;
   }
 };
+
+CXSymbolRole getSymbolRole(SymbolRoleSet Role) {
+  // CXSymbolRole mirrors low 9 bits of clang::index::SymbolRole.
+  return CXSymbolRole(static_cast(Role) & ((1 << 9) - 1));
+}
 }
 
 bool CXIndexDataConsumer::handleDeclOccurence(const Decl *D,
@@ -184,6 +189,7 @@
 if (Roles & (unsigned)SymbolRole::Implicit) {
   Kind = CXIdxEntityRef_Implicit;
 }
+CXSymbolRole CXRole = getSymbolRole(Roles);
 
 CXCursor Cursor;
 if (ASTNode.OrigE) {
@@ -202,7 +208,7 @@
 }
 handleReference(ND, Loc, Cursor,
 dyn_cast_or_null(ASTNode.Parent),
-ASTNode.ContainerDC, ASTNode.OrigE, Kind);
+ASTNode.ContainerDC, ASTNode.OrigE, Kind, CXRole);
 
   } else {
 const DeclContext *LexicalDC = ASTNode.ContainerDC;
@@ -889,21 +895,23 @@
   const NamedDecl *Parent,
   const DeclContext *DC,
   const Expr *E,
-  CXIdxEntityRefKind Kind) {
+  CXIdxEntityRefKind Kind,
+  CXSymbolRole Role) {
   if (!D || !DC)
 return false;
 
   CXCursor Cursor = E ? MakeCXCursor(E, cast(DC), CXTU)
   : getRefCursor(D, Loc);
-  return handleReference(D, Loc, Cursor, Parent, DC, E, Kind);
+  return handleReference(D, Loc, Cursor, Parent, DC, E, Kind, Role);
 }
 
 bool CXIndexDataConsumer::handleReference(const NamedDecl *D, SourceLocation Loc,
   CXCursor Cursor,
   const NamedDecl *Parent,
   const DeclContext *DC,
   const Expr *E,
-  CXIdxEntityRefKind Kind) {
+  CXIdxEntityRefKind Kind,
+  CXSymbolRole Role) {
   if (!CB.indexEntityReference)
 return false;
 
@@ -939,7 +947,8 @@
   getIndexLoc(Loc),
   &RefEntity,
   Parent ? &ParentEntity : nullptr,
-  &Container };
+  &Container,
+  Role };
   CB.indexEntityReference(ClientData, &Info);
   return true;
 }
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -3326,6 +3326,27 @@
   }
 }
 
+static void printSymbolRole(CXSymbolRole role) {
+  if (role & CXSymbolRole_Declaration)
+printf(" decl");
+  if (role & CXSymbolRole_Definition)
+printf(" def");
+  if (role & CXSymbolRole_Reference)
+printf(" ref");
+  if (role & CXSymbolRole_Read)
+printf(" read");
+  if (role & CXSymbolRole_Write)
+printf(" write");
+  if (role & CXSymbolRole_Call)
+printf(" call");
+  if (role & CXSymbolRole_Dynamic)
+printf(" dyn");
+  if (role & CXSymbolRole_AddressOf)
+printf(" addr");
+  if (role & CXSymbolRole_Implicit)
+printf(" implicit");
+}
+
 static void index_diagnostic(CXClientData client_data,
  CXDi

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 133635.
ioeric marked 3 inline comments as done.
ioeric added a comment.

- Addressed review comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42640

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/CodeComplete.cpp
  clangd/Headers.cpp
  clangd/Headers.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/clients/clangd-vscode/package.json
  clangd/global-symbol-builder/CMakeLists.txt
  clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
  clangd/index/CanonicalIncludes.cpp
  clangd/index/CanonicalIncludes.h
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/Merge.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  clangd/index/SymbolYAML.cpp
  clangd/tool/ClangdMain.cpp
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test
  test/clangd/insert-include.test
  unittests/clangd/CMakeLists.txt
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/HeadersTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -47,6 +47,12 @@
 }
 MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
 MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.FileURI == P; }
+MATCHER(HasIncludeHeader, "") {
+  return arg.Detail && !arg.Detail->IncludeHeader.empty();
+}
+MATCHER_P(IncludeHeader, P, "") {
+  return arg.Detail && arg.Detail->IncludeHeader == P;
+}
 MATCHER_P(LocationOffsets, Offsets, "") {
   // Offset range in SymbolLocation is [start, end] while in Clangd is [start,
   // end).
@@ -60,41 +66,62 @@
 namespace {
 class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
 public:
-  SymbolIndexActionFactory(SymbolCollector::Options COpts)
-  : COpts(std::move(COpts)) {}
+  SymbolIndexActionFactory(SymbolCollector::Options COpts,
+   CommentHandler *PragmaHandler)
+  : COpts(std::move(COpts)), PragmaHandler(PragmaHandler) {}
 
   clang::FrontendAction *create() override {
+class WrappedIndexAction : public WrapperFrontendAction {
+public:
+  WrappedIndexAction(std::shared_ptr C,
+ const index::IndexingOptions &Opts,
+ CommentHandler *PragmaHandler)
+  : WrapperFrontendAction(
+index::createIndexingAction(C, Opts, nullptr)),
+PragmaHandler(PragmaHandler) {}
+
+  std::unique_ptr
+  CreateASTConsumer(CompilerInstance &CI, StringRef InFile) override {
+if (PragmaHandler)
+  CI.getPreprocessor().addCommentHandler(PragmaHandler);
+return WrapperFrontendAction::CreateASTConsumer(CI, InFile);
+  }
+
+private:
+  index::IndexingOptions IndexOpts;
+  CommentHandler *PragmaHandler;
+};
 index::IndexingOptions IndexOpts;
 IndexOpts.SystemSymbolFilter =
 index::IndexingOptions::SystemSymbolFilterKind::All;
 IndexOpts.IndexFunctionLocals = false;
 Collector = std::make_shared(COpts);
-FrontendAction *Action =
-index::createIndexingAction(Collector, IndexOpts, nullptr).release();
-return Action;
+return new WrappedIndexAction(Collector, std::move(IndexOpts),
+  PragmaHandler);
   }
 
   std::shared_ptr Collector;
   SymbolCollector::Options COpts;
+  CommentHandler *PragmaHandler;
 };
 
 class SymbolCollectorTest : public ::testing::Test {
 public:
   SymbolCollectorTest()
-  : TestHeaderName(getVirtualTestFilePath("symbol.h").str()),
+  : InMemoryFileSystem(new vfs::InMemoryFileSystem),
+TestHeaderName(getVirtualTestFilePath("symbol.h").str()),
 TestFileName(getVirtualTestFilePath("symbol.cc").str()) {
 TestHeaderURI = URI::createFile(TestHeaderName).toString();
 TestFileURI = URI::createFile(TestFileName).toString();
   }
 
   bool runSymbolCollector(StringRef HeaderCode, StringRef MainCode,
   const std::vector &ExtraArgs = {}) {
-llvm::IntrusiveRefCntPtr InMemoryFileSystem(
-new vfs::InMemoryFileSystem);
 llvm::IntrusiveRefCntPtr Files(
 new FileManager(FileSystemOptions(), InMemoryFileSystem));
 
-auto Factory = llvm::make_unique(CollectorOpts);
+auto Factory = llvm::make_unique(
+CollectorOpts, PragmaHandler.get());
 
 std::vector Args = {"symbol_collector", "-fsyntax-only",
  "-std=c++11", TestFileName};
@@ -120,12 +147,14 @@
   }
 
 protected:
+  llvm::IntrusiveRefCntPtr InMemoryFileSystem;
   std::string TestHeaderName;
   std::string TestHeaderURI;
   std::string TestFileName;
   std::string TestFileURI;
   SymbolSlab Symbols;
   SymbolCollector::Options CollectorOpts;
+  std::unique_ptr PragmaHan

[PATCH] D42640: [clangd] collect symbol #include & insert #include in global code completion.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/ClangdServer.cpp:368
 
+/// Calculates the shortest possible include path when inserting \p Header to 
\p
+/// File, by matching \p Header against all include search directories for \p

sammccall wrote:
> ioeric wrote:
> > sammccall wrote:
> > > This has a fair amount of logic (well, plumbing :-D) and uses relatively 
> > > little from ClangdServer.
> > > Can we move `shortenIncludePath` to a separate file and pass in the FS 
> > > and command?
> > > 
> > > I'd suggest maybe `Headers.h` - The formatting part of `insertInclude` 
> > > could also fit there, as well as probably the logic from 
> > > `switchSourceHeader` I think (the latter not in this patch of course).
> > > 
> > > This won't really help much with testing, I just find it gets hard to 
> > > navigate files that have lots of details of unrelated features. 
> > > ClangdUnit and ClangdServer both have this potential to grow without 
> > > bound, though they're in reasonable shape now. Interested what you think!
> > Done.
> > 
> > I didn't move the formatting code, as half of the code is pulling out the 
> > style, which we might want to share/change depending on other clangd logics 
> > that might use style.
> I'd still think pulling out `Expected 
> insertInclude(File, Code, Header, VFS, Style)` would be worthwhile here - the 
> formatting isn't a lot of code, but it's a bit magic, plus the quote 
> handling... it's a bit of code. It'd make it more obvious what the 
> interactions with ClangdServer's state are. But up to you, we can always do 
> this later.
So it's just 2 lines for the replacement magic (+1 for comment) now after 
removing some redundant code. 

I like `shortenIncludePath` better because it's more self-contained and easier 
to write tests against, and `insertInclude` doesn't seem to carry much more 
weight while we would need to handle `replacements` logic which has been tested 
in the tests.



Comment at: clangd/ClangdServer.cpp:370
+/// File, by matching \p Header against all include search directories for \p
+/// File via `clang::HeaderSearch`.
+///

sammccall wrote:
> sammccall wrote:
> > maybe drop "via clang::HeaderSearch" (it's doc'd in the implementation) and 
> > add an example?
> > 
> > It might be worth explicitly stating that the result is an #include string 
> > (with quotes) - you just call it a "path" here.
> > 
> > "shortest" makes sense as a first implementation, but we may want to 
> > document something more like "best" - I know there are codebases we care 
> > about where file-relative paths are banned. Also I suspect we don't give 
> > "../../../../Foo.h" even if it's shortest :-)
> > "shortest" makes sense as a first implementation, but we may want to 
> > document something more like "best" - I know there are codebases we care 
> > about where file-relative paths are banned. Also I suspect we don't give 
> > "../../../../Foo.h" even if it's shortest :-)
> 
> I think this part wasn't addressed
I added a comment for this in the header. But I might be misunderstanding you 
suggestion. Did you mean we need a better name for the function?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42640



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42969: [Sema] Fix decltype of static data members

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 accepted this revision.
rogfer01 added a comment.
This revision is now accepted and ready to land.

This LGTM. Thanks for fixing this.

Wait a couple of days before committing in case there are other comments.


https://reviews.llvm.org/D42969



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41316: [libcxx] Allow random_device to be built optionally

[Weiming Zhao via Phabricator via cfe-commits]

weimingz added a comment.

ping?


Repository:
  rCXX libc++

https://reviews.llvm.org/D41316



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/misc-throw-keyword-missing.rst:6
+
+This check warns about the potentially missing `throw` keyword. If a temporary 
object is created,
+but the object's type derives from (or the same as) a class that has 
'EXCEPTION', 'Exception' or

aaron.ballman wrote:
> about the potentially -> about a potentially
Please remove //This check// and enclose throw in ``


https://reviews.llvm.org/D43120



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/misc/ThrowKeywordMissingCheck.cpp:24
+
+  auto CtorInitializerList =
+  cxxConstructorDecl(hasAnyConstructorInitializer(anything()));

Please don't use auto where type could not be easily deduced.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43120



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43128: Introduce an API for LLDB to compute the default module cache path

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno accepted this revision.
bruno added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D43128



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/misc/ThrowKeywordMissingCheck.cpp:24
+
+  auto CtorInitializerList =
+  cxxConstructorDecl(hasAnyConstructorInitializer(anything()));

Eugene.Zelenko wrote:
> Please don't use auto where type could not be easily deduced.
`auto` is appropriate here because these are horrible to try to spell out 
manually.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43120



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43098: [analyzer] [tests] [NFC] Remove a fragile tightly-coupled component emulating parser output

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC324759: [analyzer] [tests] [NFC] Remove a fragile 
tightly-coupled component emulating… (authored by george.karpenkov, committed 
by ).
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D43098

Files:
  utils/analyzer/CmpRuns.py
  utils/analyzer/SATestBuild.py


Index: utils/analyzer/SATestBuild.py
===
--- utils/analyzer/SATestBuild.py
+++ utils/analyzer/SATestBuild.py
@@ -566,7 +566,8 @@
RefDir, NewDir))
 
 PatchedSourceDirPath = os.path.join(Dir, PatchedSourceDirName)
-Opts = CmpRuns.CmpOptions(rootA="", rootB=PatchedSourceDirPath)
+Opts, Args = CmpRuns.generate_option_parser().parse_args(
+["", PatchedSourceDirPath])
 # Scan the results, delete empty plist files.
 NumDiffs, ReportsInRef, ReportsInNew = \
 CmpRuns.dumpScanBuildResultsDiff(RefDir, NewDir, Opts, False)
Index: utils/analyzer/CmpRuns.py
===
--- utils/analyzer/CmpRuns.py
+++ utils/analyzer/CmpRuns.py
@@ -102,20 +102,6 @@
 return self._data
 
 
-class CmpOptions:
-"""
-Fake output of option parser with manually constructed options.
-"""
-
-def __init__(self, verboseLog=None, rootA="", rootB=""):
-self.rootA = rootA
-self.rootB = rootB
-self.verboseLog = verboseLog
-self.relative_path_histogram = False
-self.relative_log_path_histogram = False
-self.absolute_path_histogram = False
-
-
 class AnalysisReport:
 def __init__(self, run, files):
 self.run = run
@@ -322,9 +308,7 @@
 
 return foundDiffs, len(resultsA.diagnostics), len(resultsB.diagnostics)
 
-
-def main():
-from optparse import OptionParser
+def generate_option_parser():
 parser = OptionParser("usage: %prog [options] [dir A] [dir B]")
 parser.add_option("", "--rootA", dest="rootA",
   help="Prefix to ignore on source files for directory A",
@@ -334,24 +318,29 @@
   action="store", type=str, default="")
 parser.add_option("", "--verbose-log", dest="verboseLog",
   help="Write additional information to LOG \
-  [default=None]",
+   [default=None]",
   action="store", type=str, default=None,
   metavar="LOG")
 parser.add_option("--relative-path-differences-histogram",
   action="store_true", dest="relative_path_histogram",
   default=False,
   help="Show histogram of relative paths differences. \
-  Requires matplotlib")
+Requires matplotlib")
 parser.add_option("--relative-log-path-differences-histogram",
   action="store_true", dest="relative_log_path_histogram",
   default=False,
   help="Show histogram of log relative paths differences. \
-  Requires matplotlib")
+Requires matplotlib")
 parser.add_option("--absolute-path-differences-histogram",
   action="store_true", dest="absolute_path_histogram",
   default=False,
   help="Show histogram of absolute paths differences. \
-  Requires matplotlib")
+Requires matplotlib")
+return parser
+
+
+def main():
+parser = generate_option_parser()
 (opts, args) = parser.parse_args()
 
 if len(args) != 2:


Index: utils/analyzer/SATestBuild.py
===
--- utils/analyzer/SATestBuild.py
+++ utils/analyzer/SATestBuild.py
@@ -566,7 +566,8 @@
RefDir, NewDir))
 
 PatchedSourceDirPath = os.path.join(Dir, PatchedSourceDirName)
-Opts = CmpRuns.CmpOptions(rootA="", rootB=PatchedSourceDirPath)
+Opts, Args = CmpRuns.generate_option_parser().parse_args(
+["", PatchedSourceDirPath])
 # Scan the results, delete empty plist files.
 NumDiffs, ReportsInRef, ReportsInNew = \
 CmpRuns.dumpScanBuildResultsDiff(RefDir, NewDir, Opts, False)
Index: utils/analyzer/CmpRuns.py
===
--- utils/analyzer/CmpRuns.py
+++ utils/analyzer/CmpRuns.py
@@ -102,20 +102,6 @@
 return self._data
 
 
-class CmpOptions:
-"""
-Fake output of option parser with manually constructed options.
-"""
-
-def __init__(self, verboseLog=None, rootA="", rootB=""):
-self.rootA = rootA
-self.rootB = rootB
-self.verboseLog = verboseLog
-self.relative_path_histogram = False
-self.relative_lo

r324759 - [analyzer] [tests] [NFC] Remove a fragile tightly-coupled component emulating parser output

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Fri Feb  9 10:39:47 2018
New Revision: 324759

URL: http://llvm.org/viewvc/llvm-project?rev=324759&view=rev
Log:
[analyzer] [tests] [NFC] Remove a fragile tightly-coupled component emulating 
parser output

...when we can just use the real parser instead.

Differential Revision: https://reviews.llvm.org/D43098

Modified:
cfe/trunk/utils/analyzer/CmpRuns.py
cfe/trunk/utils/analyzer/SATestBuild.py

Modified: cfe/trunk/utils/analyzer/CmpRuns.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/CmpRuns.py?rev=324759&r1=324758&r2=324759&view=diff
==
--- cfe/trunk/utils/analyzer/CmpRuns.py (original)
+++ cfe/trunk/utils/analyzer/CmpRuns.py Fri Feb  9 10:39:47 2018
@@ -102,20 +102,6 @@ class AnalysisDiagnostic:
 return self._data
 
 
-class CmpOptions:
-"""
-Fake output of option parser with manually constructed options.
-"""
-
-def __init__(self, verboseLog=None, rootA="", rootB=""):
-self.rootA = rootA
-self.rootB = rootB
-self.verboseLog = verboseLog
-self.relative_path_histogram = False
-self.relative_log_path_histogram = False
-self.absolute_path_histogram = False
-
-
 class AnalysisReport:
 def __init__(self, run, files):
 self.run = run
@@ -322,9 +308,7 @@ def dumpScanBuildResultsDiff(dirA, dirB,
 
 return foundDiffs, len(resultsA.diagnostics), len(resultsB.diagnostics)
 
-
-def main():
-from optparse import OptionParser
+def generate_option_parser():
 parser = OptionParser("usage: %prog [options] [dir A] [dir B]")
 parser.add_option("", "--rootA", dest="rootA",
   help="Prefix to ignore on source files for directory A",
@@ -334,24 +318,29 @@ def main():
   action="store", type=str, default="")
 parser.add_option("", "--verbose-log", dest="verboseLog",
   help="Write additional information to LOG \
-  [default=None]",
+   [default=None]",
   action="store", type=str, default=None,
   metavar="LOG")
 parser.add_option("--relative-path-differences-histogram",
   action="store_true", dest="relative_path_histogram",
   default=False,
   help="Show histogram of relative paths differences. \
-  Requires matplotlib")
+Requires matplotlib")
 parser.add_option("--relative-log-path-differences-histogram",
   action="store_true", dest="relative_log_path_histogram",
   default=False,
   help="Show histogram of log relative paths differences. \
-  Requires matplotlib")
+Requires matplotlib")
 parser.add_option("--absolute-path-differences-histogram",
   action="store_true", dest="absolute_path_histogram",
   default=False,
   help="Show histogram of absolute paths differences. \
-  Requires matplotlib")
+Requires matplotlib")
+return parser
+
+
+def main():
+parser = generate_option_parser()
 (opts, args) = parser.parse_args()
 
 if len(args) != 2:

Modified: cfe/trunk/utils/analyzer/SATestBuild.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/SATestBuild.py?rev=324759&r1=324758&r2=324759&view=diff
==
--- cfe/trunk/utils/analyzer/SATestBuild.py (original)
+++ cfe/trunk/utils/analyzer/SATestBuild.py Fri Feb  9 10:39:47 2018
@@ -566,7 +566,8 @@ def runCmpResults(Dir, Strictness=0):
RefDir, NewDir))
 
 PatchedSourceDirPath = os.path.join(Dir, PatchedSourceDirName)
-Opts = CmpRuns.CmpOptions(rootA="", rootB=PatchedSourceDirPath)
+Opts, Args = CmpRuns.generate_option_parser().parse_args(
+["", PatchedSourceDirPath])
 # Scan the results, delete empty plist files.
 NumDiffs, ReportsInRef, ReportsInNew = \
 CmpRuns.dumpScanBuildResultsDiff(RefDir, NewDir, Opts, False)


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r324761 - Introduce an API for LLDB to compute the default module cache path

[Adrian Prantl via cfe-commits]

Author: adrian
Date: Fri Feb  9 10:43:10 2018
New Revision: 324761

URL: http://llvm.org/viewvc/llvm-project?rev=324761&view=rev
Log:
Introduce an API for LLDB to compute the default module cache path

LLDB creates Clang modules and had an incomplete copy of the clang
Driver code that compute the -fmodule-cache-path. This patch makes the
clang driver code accessible to LLDB.

Differential Revision: https://reviews.llvm.org/D43128

Modified:
cfe/trunk/include/clang/Driver/Driver.h
cfe/trunk/lib/Driver/ToolChains/Clang.cpp

Modified: cfe/trunk/include/clang/Driver/Driver.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Driver.h?rev=324761&r1=324760&r2=324761&view=diff
==
--- cfe/trunk/include/clang/Driver/Driver.h (original)
+++ cfe/trunk/include/clang/Driver/Driver.h Fri Feb  9 10:43:10 2018
@@ -572,6 +572,8 @@ public:
   /// no extra characters remaining at the end.
   static bool GetReleaseVersion(StringRef Str,
 MutableArrayRef Digits);
+  /// Compute the default -fmodule-cache-path.
+  static void getDefaultModuleCachePath(SmallVectorImpl &Result);
 };
 
 /// \return True if the last defined optimization level is -Ofast.

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=324761&r1=324760&r2=324761&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Fri Feb  9 10:43:10 2018
@@ -2500,6 +2500,13 @@ static void RenderBuiltinOptions(const T
 CmdArgs.push_back("-fno-math-builtin");
 }
 
+void Driver::getDefaultModuleCachePath(SmallVectorImpl &Result) {
+  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, Result);
+  llvm::sys::path::append(Result, "org.llvm.clang.");
+  appendUserToPath(Result);
+  llvm::sys::path::append(Result, "ModuleCache");
+}
+
 static void RenderModulesOptions(Compilation &C, const Driver &D,
  const ArgList &Args, const InputInfo &Input,
  const InputInfo &Output,
@@ -2560,10 +2567,7 @@ static void RenderModulesOptions(Compila
   llvm::sys::path::append(Path, "modules");
 } else if (Path.empty()) {
   // No module path was provided: use the default.
-  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, Path);
-  llvm::sys::path::append(Path, "org.llvm.clang.");
-  appendUserToPath(Path);
-  llvm::sys::path::append(Path, "ModuleCache");
+  Driver::getDefaultModuleCachePath(Path);
 }
 
 const char Arg[] = "-fmodules-cache-path=";


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43128: Introduce an API for LLDB to compute the default module cache path

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL324761: Introduce an API for LLDB to compute the default 
module cache path (authored by adrian, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D43128?vs=133631&id=133650#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D43128

Files:
  cfe/trunk/include/clang/Driver/Driver.h
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp


Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -2500,6 +2500,13 @@
 CmdArgs.push_back("-fno-math-builtin");
 }
 
+void Driver::getDefaultModuleCachePath(SmallVectorImpl &Result) {
+  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, Result);
+  llvm::sys::path::append(Result, "org.llvm.clang.");
+  appendUserToPath(Result);
+  llvm::sys::path::append(Result, "ModuleCache");
+}
+
 static void RenderModulesOptions(Compilation &C, const Driver &D,
  const ArgList &Args, const InputInfo &Input,
  const InputInfo &Output,
@@ -2560,10 +2567,7 @@
   llvm::sys::path::append(Path, "modules");
 } else if (Path.empty()) {
   // No module path was provided: use the default.
-  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, Path);
-  llvm::sys::path::append(Path, "org.llvm.clang.");
-  appendUserToPath(Path);
-  llvm::sys::path::append(Path, "ModuleCache");
+  Driver::getDefaultModuleCachePath(Path);
 }
 
 const char Arg[] = "-fmodules-cache-path=";
Index: cfe/trunk/include/clang/Driver/Driver.h
===
--- cfe/trunk/include/clang/Driver/Driver.h
+++ cfe/trunk/include/clang/Driver/Driver.h
@@ -572,6 +572,8 @@
   /// no extra characters remaining at the end.
   static bool GetReleaseVersion(StringRef Str,
 MutableArrayRef Digits);
+  /// Compute the default -fmodule-cache-path.
+  static void getDefaultModuleCachePath(SmallVectorImpl &Result);
 };
 
 /// \return True if the last defined optimization level is -Ofast.


Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -2500,6 +2500,13 @@
 CmdArgs.push_back("-fno-math-builtin");
 }
 
+void Driver::getDefaultModuleCachePath(SmallVectorImpl &Result) {
+  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, Result);
+  llvm::sys::path::append(Result, "org.llvm.clang.");
+  appendUserToPath(Result);
+  llvm::sys::path::append(Result, "ModuleCache");
+}
+
 static void RenderModulesOptions(Compilation &C, const Driver &D,
  const ArgList &Args, const InputInfo &Input,
  const InputInfo &Output,
@@ -2560,10 +2567,7 @@
   llvm::sys::path::append(Path, "modules");
 } else if (Path.empty()) {
   // No module path was provided: use the default.
-  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, Path);
-  llvm::sys::path::append(Path, "org.llvm.clang.");
-  appendUserToPath(Path);
-  llvm::sys::path::append(Path, "ModuleCache");
+  Driver::getDefaultModuleCachePath(Path);
 }
 
 const char Arg[] = "-fmodules-cache-path=";
Index: cfe/trunk/include/clang/Driver/Driver.h
===
--- cfe/trunk/include/clang/Driver/Driver.h
+++ cfe/trunk/include/clang/Driver/Driver.h
@@ -572,6 +572,8 @@
   /// no extra characters remaining at the end.
   static bool GetReleaseVersion(StringRef Str,
 MutableArrayRef Digits);
+  /// Compute the default -fmodule-cache-path.
+  static void getDefaultModuleCachePath(SmallVectorImpl &Result);
 };
 
 /// \return True if the last defined optimization level is -Ofast.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42983: [clang-tidy] Add readability-simd-intrinsics check.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:46
+
+  static const llvm::StringMap Mapping{
+// [simd.alg]

I think you can use `llvm::StringRef` here instead of `std::string`



Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:88
+: ClangTidyCheck(Name, Context),
+  Enabled(Options.getLocalOrGlobal("Enabled", 0) != 0) {}
+

`Enabled` seems too broad to me.
How about `UseStdExperimental`? (still defaults to `false`) 



Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:98
+  if (getLangOpts().CPlusPlus2a) {
+Std = "std";
+  } else if (getLangOpts().CPlusPlus11) {

So sorry for beating the same stuff again, but i believe as of right now, it's 
still only in `std::experimental` namespace, not in `std`?



Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42983



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r324762 - [analyzer] [tests] Fixing an error after non-atomic cherry-pick

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Fri Feb  9 10:48:31 2018
New Revision: 324762

URL: http://llvm.org/viewvc/llvm-project?rev=324762&view=rev
Log:
[analyzer] [tests] Fixing an error after non-atomic cherry-pick

Modified:
cfe/trunk/utils/analyzer/CmpRuns.py

Modified: cfe/trunk/utils/analyzer/CmpRuns.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/CmpRuns.py?rev=324762&r1=324761&r2=324762&view=diff
==
--- cfe/trunk/utils/analyzer/CmpRuns.py (original)
+++ cfe/trunk/utils/analyzer/CmpRuns.py Fri Feb  9 10:48:31 2018
@@ -29,6 +29,7 @@ Usage:
 import os
 import plistlib
 from math import log
+from optparse import OptionParser
 
 
 # Information about analysis run:


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39074: [libunwind][MIPS]: Add support for unwinding in N32 processes.

[John Baldwin via Phabricator via cfe-commits]

bsdjhb updated this revision to Diff 133660.
bsdjhb added a comment.

- Rebase.
- Rework ABI macro checks.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D39074

Files:
  include/__libunwind_config.h
  src/AddressSpace.hpp
  src/DwarfInstructions.hpp
  src/UnwindRegistersRestore.S
  src/UnwindRegistersSave.S
  src/libunwind.cpp

Index: src/libunwind.cpp
===
--- src/libunwind.cpp
+++ src/libunwind.cpp
@@ -61,9 +61,10 @@
 # define REGISTER_KIND Registers_arm
 #elif defined(__or1k__)
 # define REGISTER_KIND Registers_or1k
-#elif defined(__mips__) && defined(_ABIO32) && defined(__mips_soft_float)
+#elif defined(__mips__) && defined(_ABIO32) && _MIPS_SIM == _ABIO32 && \
+defined(__mips_soft_float)
 # define REGISTER_KIND Registers_mips_o32
-#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+#elif defined(__mips64) && defined(__mips_soft_float)
 # define REGISTER_KIND Registers_mips_newabi
 #elif defined(__mips__)
 # warning The MIPS architecture is not supported with this ABI and environment!
Index: src/UnwindRegistersSave.S
===
--- src/UnwindRegistersSave.S
+++ src/UnwindRegistersSave.S
@@ -116,7 +116,8 @@
   xorl  %eax, %eax# return UNW_ESUCCESS
   ret
 
-#elif defined(__mips__) && defined(_ABIO32) && defined(__mips_soft_float)
+#elif defined(__mips__) && defined(_ABIO32) && _MIPS_SIM == _ABIO32 && \
+defined(__mips_soft_float)
 
 #
 # extern int unw_getcontext(unw_context_t* thread_state)
@@ -172,7 +173,7 @@
   or$2, $0, $0
   .set pop
 
-#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+#elif defined(__mips64) && defined(__mips_soft_float)
 
 #
 # extern int unw_getcontext(unw_context_t* thread_state)
Index: src/UnwindRegistersRestore.S
===
--- src/UnwindRegistersRestore.S
+++ src/UnwindRegistersRestore.S
@@ -799,7 +799,8 @@
   l.jr r9
l.nop
 
-#elif defined(__mips__) && defined(_ABIO32) && defined(__mips_soft_float)
+#elif defined(__mips__) && defined(_ABIO32) && _MIPS_SIM == _ABIO32 && \
+defined(__mips_soft_float)
 
 //
 // void libunwind::Registers_mips_o32::jumpto()
@@ -855,7 +856,7 @@
   lw$4, (4 * 4)($4)
   .set pop
 
-#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+#elif defined(__mips64) && defined(__mips_soft_float)
 
 //
 // void libunwind::Registers_mips_newabi::jumpto()
Index: src/DwarfInstructions.hpp
===
--- src/DwarfInstructions.hpp
+++ src/DwarfInstructions.hpp
@@ -82,10 +82,10 @@
 const RegisterLocation &savedReg) {
   switch (savedReg.location) {
   case CFI_Parser::kRegisterInCFA:
-return addressSpace.getP(cfa + (pint_t)savedReg.value);
+return addressSpace.getRegister(cfa + (pint_t)savedReg.value);
 
   case CFI_Parser::kRegisterAtExpression:
-return addressSpace.getP(
+return addressSpace.getRegister(
 evaluateExpression((pint_t)savedReg.value, addressSpace,
 registers, cfa));
 
Index: src/AddressSpace.hpp
===
--- src/AddressSpace.hpp
+++ src/AddressSpace.hpp
@@ -207,6 +207,7 @@
 return val;
   }
   uintptr_t   getP(pint_t addr);
+  uint64_tgetRegister(pint_t addr);
   static uint64_t getULEB128(pint_t &addr, pint_t end);
   static int64_t  getSLEB128(pint_t &addr, pint_t end);
 
@@ -228,6 +229,14 @@
 #endif
 }
 
+inline uint64_t LocalAddressSpace::getRegister(pint_t addr) {
+#if __SIZEOF_POINTER__ == 8 || defined(__mips64)
+  return get64(addr);
+#else
+  return get32(addr);
+#endif
+}
+
 /// Read a ULEB128 into a 64-bit word.
 inline uint64_t LocalAddressSpace::getULEB128(pint_t &addr, pint_t end) {
   const uint8_t *p = (uint8_t *)addr;
@@ -600,6 +609,7 @@
   uint32_t  get32(pint_t addr);
   uint64_t  get64(pint_t addr);
   pint_tgetP(pint_t addr);
+  uint64_t  getRegister(pint_t addr);
   uint64_t  getULEB128(pint_t &addr, pint_t end);
   int64_t   getSLEB128(pint_t &addr, pint_t end);
   pint_tgetEncodedP(pint_t &addr, pint_t end, uint8_t encoding,
@@ -636,7 +646,12 @@
 }
 
 template 
-uint64_t RemoteAddressSpace::getULEB128(pint_t &addr, pint_t end) {
+typename P::uint_t OtherAddressSpace::getRegister(pint_t addr) {
+  return P::getRegister(*(uint64_t *)localCopy(addr));
+}
+
+template 
+uint64_t OtherAddressSpace::getULEB128(pint_t &addr, pint_t end) {
   uintptr_t size = (end - addr);
   LocalAddressSpace::pint_t laddr = (LocalAddressSpace::pint_t) localCopy(addr);
   LocalAddressSpace::pint_t sladdr = laddr;
Index: include/__libunwind_config.h
===
--- include/__libunwind_config.h
+++ include/__libunwind_config.h
@@ -71,11 +71,15 @@
 #  define _LIBUNWIND_CURSOR_SIZE 24
 #  define _LIBUN

[PATCH] D39074: [libunwind][MIPS]: Add support for unwinding in N32 processes.

[John Baldwin via Phabricator via cfe-commits]

bsdjhb marked 7 inline comments as done.
bsdjhb added a comment.

Nice sleuthing!


Repository:
  rUNW libunwind

https://reviews.llvm.org/D39074



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

Python code looks OK to me, I have one last request: could we have a small 
documentation how the whole thing is supposed work in integration, preferably 
on an available open-source project any reader could check out?
I am asking because I have actually tried and failed to launch this CTU patch 
on a project I was analyzing.


https://reviews.llvm.org/D30691



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r324765 - Remove "CHECK: entry" in test case.

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Fri Feb  9 11:25:31 2018
New Revision: 324765

URL: http://llvm.org/viewvc/llvm-project?rev=324765&view=rev
Log:
Remove "CHECK: entry" in test case.

rdar://problem/37397814

Modified:
cfe/trunk/test/CodeGenObjCXX/trivial_abi.mm

Modified: cfe/trunk/test/CodeGenObjCXX/trivial_abi.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/trivial_abi.mm?rev=324765&r1=324764&r2=324765&view=diff
==
--- cfe/trunk/test/CodeGenObjCXX/trivial_abi.mm (original)
+++ cfe/trunk/test/CodeGenObjCXX/trivial_abi.mm Fri Feb  9 11:25:31 2018
@@ -81,7 +81,6 @@ void testCallStrong(Strong *a) {
 }
 
 // CHECK: define i64 @_Z16testReturnStrongP6Strong(%[[STRUCT_STRONG]]* 
%[[A:.*]])
-// CHECK: entry:
 // CHECK: %[[RETVAL:.*]] = alloca %[[STRUCT_STRONG]], align 8
 // CHECK: %[[A_ADDR:.*]] = alloca %[[STRUCT_STRONG]]*, align 8
 // CHECK: store %[[STRUCT_STRONG]]* %[[A]], %[[STRUCT_STRONG]]** %[[A_ADDR]], 
align 8


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43105: [RISCV] Enable __int128_t and uint128_t through clang flag

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

So you want int128_t for compiler-rt itself, so you can use the soft-float 
implementation, but you want to make int128_t opt-in to avoid the possibility 
of someone getting a link error trying to link code built with clang against 
libgcc.a?  That seems a little convoluted, but I guess it's okay.

Not sure I like the name "-fuse-int128"; doesn't really indicate what it's 
doing.  Maybe "-fforce-enable-int128".


Repository:
  rC Clang

https://reviews.llvm.org/D43105



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42745: [analyzer] Add support for __builtin_constant_p to BuiltinFunctionChecker

[Felix Kostenzer via Phabricator via cfe-commits]

sp4r74n-117 added a comment.

I do not have commit access so it would be great if you could do it for me. 
Thanks.


https://reviews.llvm.org/D42745



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits