date:20181217

Re: r349063 - [CodeComplete] Adhere to LLVM naming style in CodeCompletionTest. NFC

2018-12-17 Thread Yvan Roux via cfe-commits

Hi Ilya,

I'm not sure which one of the commits in that series is to blame, but
ARM bots are broken due to a failure in CodeCompleteTest.cpp, most
recent logs are available here:

http://lab.llvm.org:8011/builders/clang-cmake-armv7-quick/builds/6090/steps/ninja%20check%201/logs/FAIL%3A%20Clang-Unit%3A%3APreferredTypeTest.BinaryExpr

Cheers,
Yvan

On Thu, 13 Dec 2018 at 18:35, Ilya Biryukov via cfe-commits
 wrote:
>
> Author: ibiryukov
> Date: Thu Dec 13 09:32:38 2018
> New Revision: 349063
>
> URL: http://llvm.org/viewvc/llvm-project?rev=349063&view=rev
> Log:
> [CodeComplete] Adhere to LLVM naming style in CodeCompletionTest. NFC
>
> Also reuses the same var for multiple to reduce the chance of
> accidentally referecing the previous test.
>
> Modified:
> cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
>
> Modified: cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Sema/CodeCompleteTest.cpp?rev=349063&r1=349062&r2=349063&view=diff
> ==
> --- cfe/trunk/unittests/Sema/CodeCompleteTest.cpp (original)
> +++ cfe/trunk/unittests/Sema/CodeCompleteTest.cpp Thu Dec 13 09:32:38 2018
> @@ -183,79 +183,80 @@ TEST(SemaCodeCompleteTest, VisitedNSWith
>
>  TEST(PreferredTypeTest, BinaryExpr) {
>// Check various operations for arithmetic types.
> -  StringRef code1 = R"cpp(
> +  StringRef Code = R"cpp(
>  void test(int x) {
>x = ^10;
>x += ^10; x -= ^10; x *= ^10; x /= ^10; x %= ^10;
>x + ^10; x - ^10; x * ^10; x / ^10; x % ^10;
>  })cpp";
> -  EXPECT_THAT(collectPreferredTypes(code1), Each("int"));
> -  StringRef code2 = R"cpp(
> +  EXPECT_THAT(collectPreferredTypes(Code), Each("int"));
> +
> +  Code = R"cpp(
>  void test(float x) {
>x = ^10;
>x += ^10; x -= ^10; x *= ^10; x /= ^10; x %= ^10;
>x + ^10; x - ^10; x * ^10; x / ^10; x % ^10;
>  })cpp";
> -  EXPECT_THAT(collectPreferredTypes(code2), Each("float"));
> +  EXPECT_THAT(collectPreferredTypes(Code), Each("float"));
>
>// Pointer types.
> -  StringRef code3 = R"cpp(
> +  Code = R"cpp(
>  void test(int *ptr) {
>ptr - ^ptr;
>ptr = ^ptr;
>  })cpp";
> -  EXPECT_THAT(collectPreferredTypes(code3), Each("int *"));
> +  EXPECT_THAT(collectPreferredTypes(Code), Each("int *"));
>
> -  StringRef code4 = R"cpp(
> +  Code = R"cpp(
>  void test(int *ptr) {
>ptr + ^10;
>ptr += ^10;
>ptr -= ^10;
>  })cpp";
> -  EXPECT_THAT(collectPreferredTypes(code4), Each("long")); // long is 
> normalized 'ptrdiff_t'.
> +  EXPECT_THAT(collectPreferredTypes(Code), Each("long")); // long is 
> normalized 'ptrdiff_t'.
>
>// Comparison operators.
> -  StringRef code5 = R"cpp(
> +  Code = R"cpp(
>  void test(int i) {
>i <= ^1; i < ^1; i >= ^1; i > ^1; i == ^1; i != ^1;
>  }
>)cpp";
> -  EXPECT_THAT(collectPreferredTypes(code5), Each("int"));
> +  EXPECT_THAT(collectPreferredTypes(Code), Each("int"));
>
> -  StringRef code6 = R"cpp(
> +  Code = R"cpp(
>  void test(int *ptr) {
>ptr <= ^ptr; ptr < ^ptr; ptr >= ^ptr; ptr > ^ptr;
>ptr == ^ptr; ptr != ^ptr;
>  }
>)cpp";
> -  EXPECT_THAT(collectPreferredTypes(code6), Each("int *"));
> +  EXPECT_THAT(collectPreferredTypes(Code), Each("int *"));
>
>// Relational operations.
> -  StringRef code7 = R"cpp(
> +  Code = R"cpp(
>  void test(int i, int *ptr) {
>i && ^1; i || ^1;
>ptr && ^1; ptr || ^1;
>  }
>)cpp";
> -  EXPECT_THAT(collectPreferredTypes(code7), Each("_Bool"));
> +  EXPECT_THAT(collectPreferredTypes(Code), Each("_Bool"));
>
>// Bitwise operations.
> -  StringRef code8 = R"cpp(
> +  Code = R"cpp(
>  void test(long long ll) {
>ll | ^1; ll & ^1;
>  }
>)cpp";
> -  EXPECT_THAT(collectPreferredTypes(code8), Each("long long"));
> +  EXPECT_THAT(collectPreferredTypes(Code), Each("long long"));
>
> -  StringRef code9 = R"cpp(
> +  Code = R"cpp(
>  enum A {};
>  void test(A a) {
>a | ^1; a & ^1;
>  }
>)cpp";
> -  EXPECT_THAT(collectPreferredTypes(code9), Each("enum A"));
> +  EXPECT_THAT(collectPreferredTypes(Code), Each("enum A"));
>
> -  StringRef code10 = R"cpp(
> +  Code = R"cpp(
>  enum class A {};
>  void test(A a) {
>// This is technically illegal with the 'enum class' without overloaded
> @@ -263,10 +264,10 @@ TEST(PreferredTypeTest, BinaryExpr) {
>a | ^a; a & ^a;
>  }
>)cpp";
> -  EXPECT_THAT(collectPreferredTypes(code10), Each("enum A"));
> +  EXPECT_THAT(collectPreferredTypes(Code), Each("enum A"));
>
>// Binary shifts.
> -  StringRef code11 = R"cpp(
> +  Code = R"cpp(
>  void test(int i, long long ll) {
>i << ^1; ll << ^1;
>i <<= ^1; i <<= ^1;
> @@ -274,10 +275,10 @@ TEST(PreferredTypeTest, BinaryExpr) {
>i >>= ^1; i >>= ^1;
>  }
>)cpp";
> -  EXPECT_THAT(collectPreferredTypes(code11), E

[PATCH] D54834: [analyzer][MallocChecker] Improve warning messages on double-delete errors

2018-12-17 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus added a comment.

Interesting, I've been watching the bots closely, but got no mail after a 
while. I'm not sure what the cause is, so I'll revert one-by-one.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54834/new/

https://reviews.llvm.org/D54834



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

[PATCH] D54918: [analyzer] Apply clang-format to GenericTaintChecker.cpp

2018-12-17 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus added a comment.

@boga95 Do you have commit access, or need this to be commited on your behalf?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54918/new/

https://reviews.llvm.org/D54918



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

[PATCH] D38675: [analyzer] MoveChecker Pt.10: Move the checker out of alpha state.

2018-12-17 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus added a comment.

A little late to the party, but I don't see anything against this.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D38675/new/

https://reviews.llvm.org/D38675



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

[PATCH] D55280: [CTU] Make loadExternalAST return with non nullptr on success

2018-12-17 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

Ping @a_sidorin Could you please take another look and consider my previous 
comment?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55280/new/

https://reviews.llvm.org/D55280



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

[PATCH] D53699: [ASTImporter] Fix inequality of functions with different attributes

2018-12-17 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

Ping @shafik, I have addressed you comments, could you please take another 
look? If you don't have any objections, could you please approve?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53699/new/

https://reviews.llvm.org/D53699



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

[PATCH] D55428: [Docs] Expand -fstack-protector and -fstack-protector-all info

2018-12-17 Thread Carey Williams via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL349335: [Docs] Expand -fstack-protector and 
-fstack-protector-all (authored by carwil, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55428?vs=177828&id=178434#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55428/new/

https://reviews.llvm.org/D55428

Files:
  cfe/trunk/include/clang/Driver/Options.td


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1639,11 +1639,18 @@
 Flags<[CC1Option]>, HelpText<"Char is unsigned">;
 def fsplit_stack : Flag<["-"], "fsplit-stack">, Group;
 def fstack_protector_all : Flag<["-"], "fstack-protector-all">, Group,
-  HelpText<"Force the usage of stack protectors for all functions">;
+  HelpText<"Enable stack protectors for all functions">;
 def fstack_protector_strong : Flag<["-"], "fstack-protector-strong">, 
Group,
-  HelpText<"Use a strong heuristic to apply stack protectors to functions">;
+  HelpText<"Enable stack protectors for some functions vulnerable to stack 
smashing. "
+   "Compared to -fstack-protector, this uses a stronger heuristic "
+   "that includes functions containing arrays of any size (and any 
type), "
+   "as well as any calls to alloca or the taking of an address from a 
local variable">;
 def fstack_protector : Flag<["-"], "fstack-protector">, Group,
-  HelpText<"Enable stack protectors for functions potentially vulnerable to 
stack smashing">;
+  HelpText<"Enable stack protectors for some functions vulnerable to stack 
smashing. "
+   "This uses a loose heuristic which considers functions vulnerable "
+   "if they contain a char (or 8bit integer) array or constant sized 
calls to "
+   "alloca, which are of greater size than ssp-buffer-size (default: 8 
bytes). "
+   "All variable sized calls to alloca are considered vulnerable">;
 def fstandalone_debug : Flag<["-"], "fstandalone-debug">, Group, 
Flags<[CoreOption]>,
   HelpText<"Emit full debug info for all types used by the program">;
 def fno_standalone_debug : Flag<["-"], "fno-standalone-debug">, 
Group, Flags<[CoreOption]>,


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1639,11 +1639,18 @@
 Flags<[CC1Option]>, HelpText<"Char is unsigned">;
 def fsplit_stack : Flag<["-"], "fsplit-stack">, Group;
 def fstack_protector_all : Flag<["-"], "fstack-protector-all">, Group,
-  HelpText<"Force the usage of stack protectors for all functions">;
+  HelpText<"Enable stack protectors for all functions">;
 def fstack_protector_strong : Flag<["-"], "fstack-protector-strong">, Group,
-  HelpText<"Use a strong heuristic to apply stack protectors to functions">;
+  HelpText<"Enable stack protectors for some functions vulnerable to stack smashing. "
+   "Compared to -fstack-protector, this uses a stronger heuristic "
+   "that includes functions containing arrays of any size (and any type), "
+   "as well as any calls to alloca or the taking of an address from a local variable">;
 def fstack_protector : Flag<["-"], "fstack-protector">, Group,
-  HelpText<"Enable stack protectors for functions potentially vulnerable to stack smashing">;
+  HelpText<"Enable stack protectors for some functions vulnerable to stack smashing. "
+   "This uses a loose heuristic which considers functions vulnerable "
+   "if they contain a char (or 8bit integer) array or constant sized calls to "
+   "alloca, which are of greater size than ssp-buffer-size (default: 8 bytes). "
+   "All variable sized calls to alloca are considered vulnerable">;
 def fstandalone_debug : Flag<["-"], "fstandalone-debug">, Group, Flags<[CoreOption]>,
   HelpText<"Emit full debug info for all types used by the program">;
 def fno_standalone_debug : Flag<["-"], "fno-standalone-debug">, Group, Flags<[CoreOption]>,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55640: [clang-tidy] Implement a check for large Objective-C type encodings 🔍

2018-12-17 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.



Comment at: clang-tidy/objc/TypeEncodingSizeCheck.cpp:36
+  objcPropertyDecl(unless(isExpansionInSystemHeader()),
+   
anyOf(hasAncestor(objcInterfaceDecl().bind("interface")),
+ hasAncestor(objcCategoryDecl().bind("category"

`hasAncestor` is an expensive matcher, does `hasDeclContext` meet your use 
cases?



Comment at: clang-tidy/objc/TypeEncodingSizeCheck.cpp:60
+
+  const auto *EncodedDecl = Result.Nodes.getNodeAs("decl");
+

nit: add an assertion `assert(EncodedDecl)`.



Comment at: clang-tidy/objc/TypeEncodingSizeCheck.cpp:63
+  std::string TypeEncoding;
+  if (const auto *IvarDecl = dyn_cast(EncodedDecl)) {
+IvarDecl->getASTContext().getObjCEncodingForType(IvarDecl->getType(),

Do you forget to register the matcher for `ObjCIvarDecl`? In the matcher you 
register it for `ObjCPropertyDecl`, and `ObjCInterfaceDecl`, so this branch 
will never be executed.



Comment at: docs/clang-tidy/checks/objc-type-encoding-size.rst:13
+
+   Flag Objective-C type encodings that exceed this number of bytes.

nit: mention the default value.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55640/new/

https://reviews.llvm.org/D55640



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

r349336 - Reverting bitfield size to attempt to fix a windows buildbot

2018-12-17 Thread Kristof Umann via cfe-commits

Author: szelethus
Date: Mon Dec 17 02:31:35 2018
New Revision: 349336

URL: http://llvm.org/viewvc/llvm-project?rev=349336&view=rev
Log:
Reverting bitfield size to attempt to fix a windows buildbot

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=349336&r1=349335&r2=349336&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Mon Dec 17 02:31:35 
2018
@@ -137,7 +137,7 @@ class RefState {
   const Stmt *S;
 
   Kind K : 3;
-  AllocationFamily Family : 3;
+  AllocationFamily Family : 29;
 
   RefState(Kind k, const Stmt *s, AllocationFamily family)
 : S(s), K(k), Family(family) {


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

[PATCH] D55437: [Index] Index declarations in lambda expression.

2018-12-17 Thread Haojian Wu via Phabricator via cfe-commits

hokein updated this revision to Diff 178437.
hokein added a comment.

Update the patch, add comments.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55437/new/

https://reviews.llvm.org/D55437

Files:
  lib/Index/IndexBody.cpp
  test/Index/cxx11-lambdas.cpp


Index: test/Index/cxx11-lambdas.cpp
===
--- test/Index/cxx11-lambdas.cpp
+++ test/Index/cxx11-lambdas.cpp
@@ -31,3 +31,4 @@
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: localA | USR: 
c:cxx11-lambdas.cpp@100@S@X@F@f#@localA | lang: C | cursor: 
DeclRefExpr=localA:6:9 | loc: 8:14
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: localB | USR: 
c:cxx11-lambdas.cpp@100@S@X@F@f#@localB | lang: C | cursor: 
DeclRefExpr=localB:6:17 | loc: 8:23
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: x | USR: 
c:cxx11-lambdas.cpp@157@S@X@F@f#@Sa@F@operator()#I#1@x | lang: C | cursor: 
DeclRefExpr=x:7:46 | loc: 8:32
+// CHECK-INDEX: [indexDeclaration]: kind: variable | name: x | USR: 
c:cxx11-lambdas.cpp@157@S@X@F@f#@Sa@F@operator()#I#1@x | lang: C | cursor: 
ParmDecl=x:7:46 (Definition) | loc: 7:46
\ No newline at end of file
Index: lib/Index/IndexBody.cpp
===
--- lib/Index/IndexBody.cpp
+++ lib/Index/IndexBody.cpp
@@ -399,6 +399,19 @@
 return true;
   }
 
+  bool TraverseLambdaExpr(LambdaExpr* LE) {
+// Clang's default behavior is only visiting the visible parts of lambda
+// expressions, thus the implicitly (and importantly) generated call
+// operator is not visited, which results in ignoring parameters of
+// the lambda expressions.
+bool Result = RecursiveASTVisitor::TraverseLambdaExpr(LE);
+// Index parameters of the lambda expressions.
+if (IndexCtx.shouldIndexFunctionLocalSymbols())
+  for (const auto* PI : LE->getCallOperator()->parameters())
+IndexCtx.handleDecl(PI);
+return Result;
+  }
+
   // RecursiveASTVisitor visits both syntactic and semantic forms, duplicating
   // the things that we visit. Make sure to only visit the semantic form.
   // Also visit things that are in the syntactic form but not the semantic one,


Index: test/Index/cxx11-lambdas.cpp
===
--- test/Index/cxx11-lambdas.cpp
+++ test/Index/cxx11-lambdas.cpp
@@ -31,3 +31,4 @@
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: localA | USR: c:cxx11-lambdas.cpp@100@S@X@F@f#@localA | lang: C | cursor: DeclRefExpr=localA:6:9 | loc: 8:14
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: localB | USR: c:cxx11-lambdas.cpp@100@S@X@F@f#@localB | lang: C | cursor: DeclRefExpr=localB:6:17 | loc: 8:23
 // CHECK-INDEX: [indexEntityReference]: kind: variable | name: x | USR: c:cxx11-lambdas.cpp@157@S@X@F@f#@Sa@F@operator()#I#1@x | lang: C | cursor: DeclRefExpr=x:7:46 | loc: 8:32
+// CHECK-INDEX: [indexDeclaration]: kind: variable | name: x | USR: c:cxx11-lambdas.cpp@157@S@X@F@f#@Sa@F@operator()#I#1@x | lang: C | cursor: ParmDecl=x:7:46 (Definition) | loc: 7:46
\ No newline at end of file
Index: lib/Index/IndexBody.cpp
===
--- lib/Index/IndexBody.cpp
+++ lib/Index/IndexBody.cpp
@@ -399,6 +399,19 @@
 return true;
   }
 
+  bool TraverseLambdaExpr(LambdaExpr* LE) {
+// Clang's default behavior is only visiting the visible parts of lambda
+// expressions, thus the implicitly (and importantly) generated call
+// operator is not visited, which results in ignoring parameters of
+// the lambda expressions.
+bool Result = RecursiveASTVisitor::TraverseLambdaExpr(LE);
+// Index parameters of the lambda expressions.
+if (IndexCtx.shouldIndexFunctionLocalSymbols())
+  for (const auto* PI : LE->getCallOperator()->parameters())
+IndexCtx.handleDecl(PI);
+return Result;
+  }
+
   // RecursiveASTVisitor visits both syntactic and semantic forms, duplicating
   // the things that we visit. Make sure to only visit the semantic form.
   // Also visit things that are in the syntactic form but not the semantic one,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] r349339 - [AArch64][libunwind] Unwinding support for return address signing with B Key

2018-12-17 Thread Luke Cheeseman via cfe-commits

Author: lukecheeseman
Date: Mon Dec 17 03:43:24 2018
New Revision: 349339

URL: http://llvm.org/viewvc/llvm-project?rev=349339&view=rev
Log:
[AArch64][libunwind] Unwinding support for return address signing with B Key

- Support for the case where the return address has been signed with the B key
- When the B key is used, a 'B' character is present in the augmentation string
  of CIE associated with the FDE for the function.

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


Modified:
libunwind/trunk/src/DwarfInstructions.hpp
libunwind/trunk/src/DwarfParser.hpp

Modified: libunwind/trunk/src/DwarfInstructions.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfInstructions.hpp?rev=349339&r1=349338&r2=349339&view=diff
==
--- libunwind/trunk/src/DwarfInstructions.hpp (original)
+++ libunwind/trunk/src/DwarfInstructions.hpp Mon Dec 17 03:43:24 2018
@@ -211,9 +211,13 @@ int DwarfInstructions::stepWithDwa
 register unsigned long long x17 __asm("x17") = returnAddress;
 register unsigned long long x16 __asm("x16") = cfa;
 
-// This is the autia1716 instruction. The hint instruction is used here
-// as gcc does not assemble autia1716 for pre armv8.3a targets.
-asm("hint 0xc": "+r"(x17): "r"(x16));
+// These are the autia1716/autib1716 instructions. The hint 
instructions
+// are used here as gcc does not assemble autia1716/autib1716 for pre
+// armv8.3a targets.
+if (cieInfo.addressesSignedWithBKey)
+  asm("hint 0xe" : "+r"(x17) : "r"(x16)); // autib1716
+else
+  asm("hint 0xc" : "+r"(x17) : "r"(x16)); // autia1716
 returnAddress = x17;
 #endif
   }

Modified: libunwind/trunk/src/DwarfParser.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfParser.hpp?rev=349339&r1=349338&r2=349339&view=diff
==
--- libunwind/trunk/src/DwarfParser.hpp (original)
+++ libunwind/trunk/src/DwarfParser.hpp Mon Dec 17 03:43:24 2018
@@ -49,6 +49,9 @@ public:
 bool  isSignalFrame;
 bool  fdesHaveAugmentationData;
 uint8_t   returnAddressRegister;
+#if defined(_LIBUNWIND_TARGET_AARCH64)
+bool  addressesSignedWithBKey;
+#endif
   };
 
   /// Information about an FDE (Frame Description Entry)
@@ -263,6 +266,9 @@ const char *CFI_Parser::parseCIE(A &a
   cieInfo->dataAlignFactor = 0;
   cieInfo->isSignalFrame = false;
   cieInfo->fdesHaveAugmentationData = false;
+#if defined(_LIBUNWIND_TARGET_AARCH64)
+  cieInfo->addressesSignedWithBKey = false;
+#endif
   cieInfo->cieStart = cie;
   pint_t p = cie;
   pint_t cieLength = (pint_t)addressSpace.get32(p);
@@ -326,6 +332,11 @@ const char *CFI_Parser::parseCIE(A &a
   case 'S':
 cieInfo->isSignalFrame = true;
 break;
+#if defined(_LIBUNWIND_TARGET_AARCH64)
+  case 'B':
+cieInfo->addressesSignedWithBKey = true;
+break;
+#endif
   default:
 // ignore unknown letters
 break;


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

r349340 - Revert rC349281 '[analyzer][MallocChecker][NFC] Document and reorganize some functions'

2018-12-17 Thread Kristof Umann via cfe-commits

Author: szelethus
Date: Mon Dec 17 04:07:57 2018
New Revision: 349340

URL: http://llvm.org/viewvc/llvm-project?rev=349340&view=rev
Log:
Revert rC349281 '[analyzer][MallocChecker][NFC] Document and reorganize some 
functions'

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

cfe/trunk/test/Analysis/Inputs/expected-plists/NewDelete-path-notes.cpp.plist
cfe/trunk/test/Analysis/Malloc+MismatchedDeallocator+NewDelete.cpp
cfe/trunk/test/Analysis/NewDelete-checker-test.cpp
cfe/trunk/test/Analysis/NewDelete-path-notes.cpp
cfe/trunk/test/Analysis/dtor.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=349340&r1=349339&r2=349340&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Mon Dec 17 04:07:57 
2018
@@ -137,7 +137,7 @@ class RefState {
   const Stmt *S;
 
   Kind K : 3;
-  AllocationFamily Family : 29;
+  AllocationFamily Family : 3;
 
   RefState(Kind k, const Stmt *s, AllocationFamily family)
 : S(s), K(k), Family(family) {
@@ -1431,8 +1431,7 @@ ProgramStateRef MallocChecker::addExtent
 
 void MallocChecker::checkPreStmt(const CXXDeleteExpr *DE,
  CheckerContext &C) const {
-  // This will regard deleting freed() regions as a use-after-free, rather then
-  // a double-free or double-delete error.
+
   if (!ChecksEnabled[CK_NewDeleteChecker])
 if (SymbolRef Sym = C.getSVal(DE->getArgument()).getAsSymbol())
   checkUseAfterFree(Sym, C, DE->getArgument());
@@ -1629,8 +1628,7 @@ ProgramStateRef MallocChecker::FreeMemAu
 }
 
 /// Checks if the previous call to free on the given symbol failed - if free
-/// failed, returns true. Also, stores the corresponding return value symbol in
-/// \p RetStatusSymbol.
+/// failed, returns true. Also, returns the corresponding return value symbol.
 static bool didPreviousFreeFail(ProgramStateRef State,
 SymbolRef Sym, SymbolRef &RetStatusSymbol) {
   const SymbolRef *Ret = State->get(Sym);
@@ -2291,12 +2289,6 @@ void MallocChecker::ReportDoubleFree(Che
   if (!CheckKind.hasValue())
 return;
 
-  // If this is a double delete error, print the appropiate warning message.
-  if (CheckKind == CK_NewDeleteChecker) {
-ReportDoubleDelete(C, Sym);
-return;
-  }
-
   if (ExplodedNode *N = C.generateErrorNode()) {
 if (!BT_DoubleFree[*CheckKind])
   BT_DoubleFree[*CheckKind].reset(new BugType(

Modified: 
cfe/trunk/test/Analysis/Inputs/expected-plists/NewDelete-path-notes.cpp.plist
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/expected-plists/NewDelete-path-notes.cpp.plist?rev=349340&r1=349339&r2=349340&view=diff
==
--- 
cfe/trunk/test/Analysis/Inputs/expected-plists/NewDelete-path-notes.cpp.plist 
(original)
+++ 
cfe/trunk/test/Analysis/Inputs/expected-plists/NewDelete-path-notes.cpp.plist 
Mon Dec 17 04:07:57 2018
@@ -194,17 +194,17 @@
  
  depth0
  extended_message
- Attempt to delete released memory
+ Attempt to free released memory
  message
- Attempt to delete released memory
+ Attempt to free released memory
 

-   descriptionAttempt to delete released memory
+   descriptionAttempt to free released memory
categoryMemory error
-   typeDouble delete
+   typeDouble free
check_namecplusplus.NewDelete

-   
issue_hash_content_of_line_in_context593b185245106bed5175ccf2753039b2
+   
issue_hash_content_of_line_in_contextbd8e324d09c70b9e2be6f824a4942e5a
   issue_context_kindfunction
   issue_contexttest
   issue_hash_function_offset8
@@ -423,17 +423,17 @@
  
  depth0
  extended_message
- Attempt to delete released memory
+ Attempt to free released memory
  message
- Attempt to delete released memory
+ Attempt to free released memory
 

-   descriptionAttempt to delete released memory
+   descriptionAttempt to free released memory
categoryMemory error
-   typeDouble delete
+   typeDouble free
check_namecplusplus.NewDelete

-   
issue_hash_content_of_line_in_context6484e9b006ede7362edef2187ba6eb37
+   
issue_hash_content_of_line_in_context8bf1a5b9fdae9d86780aa6c4cdce2605
   issue_context_kindfunction
   issue_contexttest
   issue_hash_function_offset3

Modified: cfe/trunk/test/Analysis/Malloc+MismatchedDeallocator+NewDelete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Malloc%2BMismatchedDeallocator%2BNewDelete.cpp?rev=349340&r1=349339&r2=349340&view=diff
==
--- cfe/trunk/test/Analysis/Malloc+MismatchedDeallocator+NewDelete.cpp 
(original)
+++ cfe/trunk/test/Analysis/M

[clang-tools-extra] r349341 - Revert rCTE349288 'Fix a lit test failure after MallocChecker changes'

2018-12-17 Thread Kristof Umann via cfe-commits

Author: szelethus
Date: Mon Dec 17 04:08:39 2018
New Revision: 349341

URL: http://llvm.org/viewvc/llvm-project?rev=349341&view=rev
Log:
Revert rCTE349288 'Fix a lit test failure after MallocChecker changes'

Modified:
clang-tools-extra/trunk/test/clang-tidy/static-analyzer.cpp

Modified: clang-tools-extra/trunk/test/clang-tidy/static-analyzer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/static-analyzer.cpp?rev=349341&r1=349340&r2=349341&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/static-analyzer.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/static-analyzer.cpp Mon Dec 17 
04:08:39 2018
@@ -7,7 +7,7 @@ void f() {
   int *p = new int(42);
   delete p;
   delete p;
-  // CHECK: warning: Attempt to delete released memory 
[clang-analyzer-cplusplus.NewDelete]
+  // CHECK: warning: Attempt to free released memory 
[clang-analyzer-cplusplus.NewDelete]
 }
 
 void g() {


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

[PATCH] D54834: [analyzer][MallocChecker] Improve warning messages on double-delete errors

2018-12-17 Thread Kristóf Umann via Phabricator via cfe-commits

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

Reverted in rC349340 . With the wrong 
revision name... oops...


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54834/new/

https://reviews.llvm.org/D54834



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

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

2018-12-17 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

Good point, we should definitely state it's a clangd-specific extension. We 
could come up with a naming scheme for our extensions. I would propose 
something like `clangd:$methodName`, i.e. the current extensions would be 
`clangd:textDocument/fileStatus`.
WDYT?


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55363/new/

https://reviews.llvm.org/D55363



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

[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

2018-12-17 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clangd/index/Background.h:121
   bool ShouldStop = false;
-  std::deque Queue;
+  std::deque> Tasks;
   std::vector ThreadPool; // FIXME: Abstract this away.

This rename makes the diff look a bit more complicated than it actually is. I 
personally like the new name better, but the old name also seemed ok.
Maybe considering keeping the old name to make the diff simpler?

Up to you, though, this does not seem terribly important.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55315/new/

https://reviews.llvm.org/D55315



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

r349342 - Fix "enumeral mismatch in conditional expression" gcc7 warnings. NFCI.

2018-12-17 Thread Simon Pilgrim via cfe-commits

Author: rksimon
Date: Mon Dec 17 04:17:37 2018
New Revision: 349342

URL: http://llvm.org/viewvc/llvm-project?rev=349342&view=rev
Log:
Fix "enumeral mismatch in conditional expression" gcc7 warnings. NFCI.

Modified:
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=349342&r1=349341&r2=349342&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Mon Dec 17 04:17:37 2018
@@ -6103,9 +6103,10 @@ static void ProcessDeclAttribute(Sema &S
   // though they were unknown attributes.
   if (AL.getKind() == ParsedAttr::UnknownAttribute ||
   !AL.existsInTarget(S.Context.getTargetInfo())) {
-S.Diag(AL.getLoc(), AL.isDeclspecAttribute()
-? diag::warn_unhandled_ms_attribute_ignored
-: diag::warn_unknown_attribute_ignored)
+S.Diag(AL.getLoc(),
+   AL.isDeclspecAttribute()
+   ? (unsigned)diag::warn_unhandled_ms_attribute_ignored
+   : (unsigned)diag::warn_unknown_attribute_ignored)
 << AL;
 return;
   }

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=349342&r1=349341&r2=349342&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Dec 17 04:17:37 2018
@@ -2347,8 +2347,8 @@ Sema::ActOnBaseSpecifier(Decl *classdecl
 if (AL.isInvalid() || AL.getKind() == ParsedAttr::IgnoredAttribute)
   continue;
 Diag(AL.getLoc(), AL.getKind() == ParsedAttr::UnknownAttribute
-  ? diag::warn_unknown_attribute_ignored
-  : diag::err_base_specifier_attribute)
+  ? (unsigned)diag::warn_unknown_attribute_ignored
+  : (unsigned)diag::err_base_specifier_attribute)
 << AL.getName();
   }
 
@@ -2867,14 +2867,14 @@ static const ParsedAttr *getMSPropertyAt
   return nullptr;
 }
 
-// Check if there is a field shadowing.
-void Sema::CheckShadowInheritedFields(const SourceLocation &Loc,
-  DeclarationName FieldName,
-  const CXXRecordDecl *RD,
-  bool DeclIsField) {
-  if (Diags.isIgnored(diag::warn_shadow_field, Loc))
-return;
-
+// Check if there is a field shadowing.
+void Sema::CheckShadowInheritedFields(const SourceLocation &Loc,
+  DeclarationName FieldName,
+  const CXXRecordDecl *RD,
+  bool DeclIsField) {
+  if (Diags.isIgnored(diag::warn_shadow_field, Loc))
+return;
+
   // To record a shadowed field in a base
   std::map Bases;
   auto FieldShadowed = [&](const CXXBaseSpecifier *Specifier,
@@ -2908,13 +2908,13 @@ void Sema::CheckShadowInheritedFields(co
   continue;
 auto BaseField = It->second;
 assert(BaseField->getAccess() != AS_private);
-if (AS_none !=
-CXXRecordDecl::MergeAccess(P.Access, BaseField->getAccess())) {
-  Diag(Loc, diag::warn_shadow_field)
-<< FieldName << RD << Base << DeclIsField;
-  Diag(BaseField->getLocation(), diag::note_shadow_field);
-  Bases.erase(It);
-}
+if (AS_none !=
+CXXRecordDecl::MergeAccess(P.Access, BaseField->getAccess())) {
+  Diag(Loc, diag::warn_shadow_field)
+<< FieldName << RD << Base << DeclIsField;
+  Diag(BaseField->getLocation(), diag::note_shadow_field);
+  Bases.erase(It);
+}
   }
 }
 


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

[PATCH] D55417: [clangd] Change disk-backed storage to be atomic

2018-12-17 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM with a few nits




Comment at: clangd/index/BackgroundIndexStorage.cpp:39
+template 
+llvm::Error writeAtomically(llvm::StringRef OutPath, WriterFunction &&Writer) {
+  // Write to a temporary file first.

NIT: use `llvm::function_ref` here. It forces to clearly state the signature of 
the function and allows to avoid templates.



Comment at: clangd/index/BackgroundIndexStorage.cpp:44
+  auto EC = llvm::sys::fs::createUniqueFile(
+  llvm::Twine(OutPath, ".tmp."), FD, TempPath);
+  if (EC)

NIT: I believe there's an overloaded `operator +(StringRef, const char*)` that 
would produce a Twine, so maybe simplify to `OutPath + ".tmp.%"`?
Just a suggestion, up to you.



Comment at: clangd/index/BackgroundIndexStorage.cpp:60
+  // If everything went well, we already moved the file to another name. So
+  // don't delete it, as it might be taken by another file.
+  RemoveOnFail.release();

NIT: `s/it/the name/`


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55417/new/

https://reviews.llvm.org/D55417



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

r349343 - Fix "enumeral mismatch in conditional expression" gcc7 warning. NFCI.

2018-12-17 Thread Simon Pilgrim via cfe-commits

Author: rksimon
Date: Mon Dec 17 04:25:42 2018
New Revision: 349343

URL: http://llvm.org/viewvc/llvm-project?rev=349343&view=rev
Log:
Fix "enumeral mismatch in conditional expression" gcc7 warning. NFCI.

Modified:
cfe/trunk/lib/Sema/SemaStmtAttr.cpp

Modified: cfe/trunk/lib/Sema/SemaStmtAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmtAttr.cpp?rev=349343&r1=349342&r2=349343&view=diff
==
--- cfe/trunk/lib/Sema/SemaStmtAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmtAttr.cpp Mon Dec 17 04:25:42 2018
@@ -316,9 +316,10 @@ static Attr *ProcessStmtAttribute(Sema &
   SourceRange Range) {
   switch (A.getKind()) {
   case ParsedAttr::UnknownAttribute:
-S.Diag(A.getLoc(), A.isDeclspecAttribute() ?
-   diag::warn_unhandled_ms_attribute_ignored :
-   diag::warn_unknown_attribute_ignored) << A.getName();
+S.Diag(A.getLoc(), A.isDeclspecAttribute()
+   ? 
(unsigned)diag::warn_unhandled_ms_attribute_ignored
+   : (unsigned)diag::warn_unknown_attribute_ignored)
+<< A.getName();
 return nullptr;
   case ParsedAttr::AT_FallThrough:
 return handleFallThroughAttr(S, St, A, Range);


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

r349344 - Revert rC349281 '[analyzer][MallocChecker][NFC] Document and reorganize some functions'

2018-12-17 Thread Kristof Umann via cfe-commits

Author: szelethus
Date: Mon Dec 17 04:25:48 2018
New Revision: 349344

URL: http://llvm.org/viewvc/llvm-project?rev=349344&view=rev
Log:
Revert rC349281 '[analyzer][MallocChecker][NFC] Document and reorganize some 
functions'

Accidentally commited earlier with the same commit title, but really it
should've been
"Revert rC349283 '[analyzer][MallocChecker] Improve warning messages on 
double-delete errors'"

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=349344&r1=349343&r2=349344&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Mon Dec 17 04:25:48 
2018
@@ -7,41 +7,8 @@
 //
 
//===--===//
 //
-// This file defines a variety of memory management related checkers, such as
-// leak, double free, and use-after-free.
-//
-// The following checkers are defined here:
-//
-//   * MallocChecker
-//   Despite its name, it models all sorts of memory allocations and
-//   de- or reallocation, including but not limited to malloc, free,
-//   relloc, new, delete. It also reports on a variety of memory misuse
-//   errors.
-//   Many other checkers interact very closely with this checker, in fact,
-//   most are merely options to this one. Other checkers may register
-//   MallocChecker, but do not enable MallocChecker's reports (more details
-//   to follow around its field, ChecksEnabled).
-//   It also has a boolean "Optimistic" checker option, which if set to 
true
-//   will cause the checker to model user defined memory management related
-//   functions annotated via the attribute ownership_takes, ownership_holds
-//   and ownership_returns.
-//
-//   * NewDeleteChecker
-//   Enables the modeling of new, new[], delete, delete[] in MallocChecker,
-//   and checks for related double-free and use-after-free errors.
-//
-//   * NewDeleteLeaksChecker
-//   Checks for leaks related to new, new[], delete, delete[].
-//   Depends on NewDeleteChecker.
-//
-//   * MismatchedDeallocatorChecker
-//   Enables checking whether memory is deallocated with the correspending
-//   allocation function in MallocChecker, such as malloc() allocated
-//   regions are only freed by free(), new by delete, new[] by delete[].
-//
-//  InnerPointerChecker interacts very closely with MallocChecker, but unlike
-//  the above checkers, it has it's own file, hence the many 
InnerPointerChecker
-//  related headers and non-static functions.
+// This file defines malloc/free checker, which checks for potential memory
+// leaks, double free, and use-after-free problems.
 //
 
//===--===//
 
@@ -70,10 +37,6 @@
 using namespace clang;
 using namespace ento;
 
-//===--===//
-// The types of allocation we're modeling.
-//===--===//
-
 namespace {
 
 // Used to check correspondence between allocators and deallocators.
@@ -87,59 +50,28 @@ enum AllocationFamily {
   AF_InnerBuffer
 };
 
-struct MemFunctionInfoTy;
-
-} // end of anonymous namespace
-
-/// Determine family of a deallocation expression.
-static AllocationFamily getAllocationFamily(
-const MemFunctionInfoTy &MemFunctionInfo, CheckerContext &C, const Stmt 
*S);
-
-/// Print names of allocators and deallocators.
-///
-/// \returns true on success.
-static bool printAllocDeallocName(raw_ostream &os, CheckerContext &C,
-  const Expr *E);
-
-/// Print expected name of an allocator based on the deallocator's
-/// family derived from the DeallocExpr.
-static void printExpectedAllocName(raw_ostream &os,
-   const MemFunctionInfoTy  &MemFunctionInfo,
-   CheckerContext &C, const Expr *E);
-
-/// Print expected name of a deallocator based on the allocator's
-/// family.
-static void printExpectedDeallocName(raw_ostream &os, AllocationFamily Family);
-
-//===--===//
-// The state of a symbol, in terms of memory management.
-//===--===//
-
-namespace {
-
 class RefState {
-  enum Kind {
-// Reference to allocated memory.
-Allocated,
-// Reference to zero-allocated memory.
-AllocatedOfSizeZero,
-// Reference to released/freed memory.
-Released,
-// The responsibility for freeing resources has transferred from
-// this reference. A relinquished s

[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

2018-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 178443.
kadircet marked an inline comment as done.
kadircet added a comment.

- Revert name from Tasks to Queue


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55315/new/

https://reviews.llvm.org/D55315

Files:
  clangd/Threading.cpp
  clangd/Threading.h
  clangd/index/Background.cpp
  clangd/index/Background.h

Index: clangd/index/Background.h
===
--- clangd/index/Background.h
+++ clangd/index/Background.h
@@ -13,6 +13,7 @@
 #include "Context.h"
 #include "FSProvider.h"
 #include "GlobalCompilationDatabase.h"
+#include "Threading.h"
 #include "index/FileIndex.h"
 #include "index/Index.h"
 #include "index/Serialization.h"
@@ -110,14 +111,14 @@
   // queue management
   using Task = std::function;
   void run(); // Main loop executed by Thread. Runs tasks from Queue.
-  void enqueueTask(Task T);
+  void enqueueTask(Task T, ThreadPriority Prioirty);
   void enqueueLocked(tooling::CompileCommand Cmd,
  BackgroundIndexStorage *IndexStorage);
   std::mutex QueueMu;
   unsigned NumActiveTasks = 0; // Only idle when queue is empty *and* no tasks.
   std::condition_variable QueueCV;
   bool ShouldStop = false;
-  std::deque Queue;
+  std::deque> Queue;
   std::vector ThreadPool; // FIXME: Abstract this away.
   GlobalCompilationDatabase::CommandChanged::Subscription CommandsChanged;
 };
Index: clangd/index/Background.cpp
===
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -22,6 +22,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/SHA1.h"
@@ -102,14 +103,8 @@
   })) {
   assert(ThreadPoolSize > 0 && "Thread pool size can't be zero.");
   assert(this->IndexStorageFactory && "Storage factory can not be null!");
-  while (ThreadPoolSize--) {
+  while (ThreadPoolSize--)
 ThreadPool.emplace_back([this] { run(); });
-// Set priority to low, since background indexing is a long running task we
-// do not want to eat up cpu when there are any other high priority threads.
-// FIXME: In the future we might want a more general way of handling this to
-// support tasks with various priorities.
-setThreadPriority(ThreadPool.back(), ThreadPriority::Low);
-  }
 }
 
 BackgroundIndex::~BackgroundIndex() {
@@ -130,6 +125,7 @@
   WithContext Background(BackgroundContext.clone());
   while (true) {
 Optional Task;
+ThreadPriority Priority;
 {
   std::unique_lock Lock(QueueMu);
   QueueCV.wait(Lock, [&] { return ShouldStop || !Queue.empty(); });
@@ -139,10 +135,16 @@
 return;
   }
   ++NumActiveTasks;
-  Task = std::move(Queue.front());
+  std::tie(Task, Priority) = std::move(Queue.front());
   Queue.pop_front();
 }
+
+if (Priority != ThreadPriority::Normal)
+  setCurrentThreadPriority(Priority);
 (*Task)();
+if (Priority != ThreadPriority::Normal)
+  setCurrentThreadPriority(ThreadPriority::Normal);
+
 {
   std::unique_lock Lock(QueueMu);
   assert(NumActiveTasks > 0 && "before decrementing");
@@ -160,44 +162,60 @@
 }
 
 void BackgroundIndex::enqueue(const std::vector &ChangedFiles) {
-  enqueueTask([this, ChangedFiles] {
-trace::Span Tracer("BackgroundIndexEnqueue");
-// We're doing this asynchronously, because we'll read shards here too.
-// FIXME: read shards here too.
-
-log("Enqueueing {0} commands for indexing", ChangedFiles.size());
-SPAN_ATTACH(Tracer, "files", int64_t(ChangedFiles.size()));
-
-// We shuffle the files because processing them in a random order should
-// quickly give us good coverage of headers in the project.
-std::vector Permutation(ChangedFiles.size());
-std::iota(Permutation.begin(), Permutation.end(), 0);
-std::mt19937 Generator(std::random_device{}());
-std::shuffle(Permutation.begin(), Permutation.end(), Generator);
-
-for (const unsigned I : Permutation)
-  enqueue(ChangedFiles[I]);
-  });
+  enqueueTask(
+  [this, ChangedFiles] {
+trace::Span Tracer("BackgroundIndexEnqueue");
+// We're doing this asynchronously, because we'll read shards here too.
+// FIXME: read shards here too.
+
+log("Enqueueing {0} commands for indexing", ChangedFiles.size());
+SPAN_ATTACH(Tracer, "files", int64_t(ChangedFiles.size()));
+
+// We shuffle the files because processing them in a random order should
+// quickly give us good coverage of headers in the project.
+std::vector Permutation(ChangedFiles.size());
+std::iota(Permutation.begin(), Permutation.end(), 0);
+std::mt19937 Generator(std::random_device{}());
+std::shuffle(Permuta

[PATCH] D54823: [analyzer][MallocChecker][NFC] Document and reorganize some functions

2018-12-17 Thread Kristóf Umann via Phabricator via cfe-commits

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

Reverted in rC349344 .


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54823/new/

https://reviews.llvm.org/D54823



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

r349335 - [Docs] Expand -fstack-protector and -fstack-protector-all

2018-12-17 Thread Carey Williams via cfe-commits

Author: carwil
Date: Mon Dec 17 02:11:35 2018
New Revision: 349335

URL: http://llvm.org/viewvc/llvm-project?rev=349335&view=rev
Log:
[Docs] Expand -fstack-protector and -fstack-protector-all

Improve the description of these command line options
by providing specific heuristic information, as outlined
for the ssp function attribute(s) in LLVM's documentation.

Also rewords -fstack-protector-all for affinity.

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

Modified:
cfe/trunk/include/clang/Driver/Options.td

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=349335&r1=349334&r2=349335&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Mon Dec 17 02:11:35 2018
@@ -1639,11 +1639,18 @@ def fno_signed_char : Flag<["-"], "fno-s
 Flags<[CC1Option]>, HelpText<"Char is unsigned">;
 def fsplit_stack : Flag<["-"], "fsplit-stack">, Group;
 def fstack_protector_all : Flag<["-"], "fstack-protector-all">, Group,
-  HelpText<"Force the usage of stack protectors for all functions">;
+  HelpText<"Enable stack protectors for all functions">;
 def fstack_protector_strong : Flag<["-"], "fstack-protector-strong">, 
Group,
-  HelpText<"Use a strong heuristic to apply stack protectors to functions">;
+  HelpText<"Enable stack protectors for some functions vulnerable to stack 
smashing. "
+   "Compared to -fstack-protector, this uses a stronger heuristic "
+   "that includes functions containing arrays of any size (and any 
type), "
+   "as well as any calls to alloca or the taking of an address from a 
local variable">;
 def fstack_protector : Flag<["-"], "fstack-protector">, Group,
-  HelpText<"Enable stack protectors for functions potentially vulnerable to 
stack smashing">;
+  HelpText<"Enable stack protectors for some functions vulnerable to stack 
smashing. "
+   "This uses a loose heuristic which considers functions vulnerable "
+   "if they contain a char (or 8bit integer) array or constant sized 
calls to "
+   "alloca, which are of greater size than ssp-buffer-size (default: 8 
bytes). "
+   "All variable sized calls to alloca are considered vulnerable">;
 def fstandalone_debug : Flag<["-"], "fstandalone-debug">, Group, 
Flags<[CoreOption]>,
   HelpText<"Emit full debug info for all types used by the program">;
 def fno_standalone_debug : Flag<["-"], "fno-standalone-debug">, 
Group, Flags<[CoreOption]>,


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

[clang-tools-extra] r349345 - [clangd] Only reduce priority of a thread for indexing.

2018-12-17 Thread Kadir Cetinkaya via cfe-commits

Author: kadircet
Date: Mon Dec 17 04:30:27 2018
New Revision: 349345

URL: http://llvm.org/viewvc/llvm-project?rev=349345&view=rev
Log:
[clangd] Only reduce priority of a thread for indexing.

Summary:
We'll soon have tasks pending for reading shards from disk, we want
them to have normal priority. Because:
- They are not CPU intensive, mostly IO bound.
- Give a good coverage for the project at startup, therefore it is worth
  spending some cycles.
- We have only one task per whole CDB rather than one task per file.

Reviewers: ilya-biryukov

Subscribers: ioeric, MaskRay, jkorous, arphaman, jfb, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/Threading.cpp
clang-tools-extra/trunk/clangd/Threading.h
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/index/Background.h

Modified: clang-tools-extra/trunk/clangd/Threading.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Threading.cpp?rev=349345&r1=349344&r2=349345&view=diff
==
--- clang-tools-extra/trunk/clangd/Threading.cpp (original)
+++ clang-tools-extra/trunk/clangd/Threading.cpp Mon Dec 17 04:30:27 2018
@@ -112,13 +112,13 @@ void wait(std::unique_lock &
 
 static std::atomic AvoidThreadStarvation = {false};
 
-void setThreadPriority(std::thread &T, ThreadPriority Priority) {
+void setCurrentThreadPriority(ThreadPriority Priority) {
   // Some *really* old glibcs are missing SCHED_IDLE.
 #if defined(__linux__) && defined(SCHED_IDLE)
   sched_param priority;
   priority.sched_priority = 0;
   pthread_setschedparam(
-  T.native_handle(),
+  pthread_self(),
   Priority == ThreadPriority::Low && !AvoidThreadStarvation ? SCHED_IDLE
 : SCHED_OTHER,
   &priority);

Modified: clang-tools-extra/trunk/clangd/Threading.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Threading.h?rev=349345&r1=349344&r2=349345&view=diff
==
--- clang-tools-extra/trunk/clangd/Threading.h (original)
+++ clang-tools-extra/trunk/clangd/Threading.h Mon Dec 17 04:30:27 2018
@@ -122,7 +122,7 @@ enum class ThreadPriority {
   Low = 0,
   Normal = 1,
 };
-void setThreadPriority(std::thread &T, ThreadPriority Priority);
+void setCurrentThreadPriority(ThreadPriority Priority);
 // Avoid the use of scheduler policies that may starve low-priority threads.
 // This prevents tests from timing out on loaded systems.
 // Affects subsequent setThreadPriority() calls.

Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=349345&r1=349344&r2=349345&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Mon Dec 17 04:30:27 2018
@@ -22,6 +22,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/SHA1.h"
@@ -135,14 +136,8 @@ BackgroundIndex::BackgroundIndex(
   })) {
   assert(ThreadPoolSize > 0 && "Thread pool size can't be zero.");
   assert(this->IndexStorageFactory && "Storage factory can not be null!");
-  while (ThreadPoolSize--) {
+  while (ThreadPoolSize--)
 ThreadPool.emplace_back([this] { run(); });
-// Set priority to low, since background indexing is a long running task we
-// do not want to eat up cpu when there are any other high priority 
threads.
-// FIXME: In the future we might want a more general way of handling this 
to
-// support tasks with various priorities.
-setThreadPriority(ThreadPool.back(), ThreadPriority::Low);
-  }
 }
 
 BackgroundIndex::~BackgroundIndex() {
@@ -163,6 +158,7 @@ void BackgroundIndex::run() {
   WithContext Background(BackgroundContext.clone());
   while (true) {
 Optional Task;
+ThreadPriority Priority;
 {
   std::unique_lock Lock(QueueMu);
   QueueCV.wait(Lock, [&] { return ShouldStop || !Queue.empty(); });
@@ -172,10 +168,16 @@ void BackgroundIndex::run() {
 return;
   }
   ++NumActiveTasks;
-  Task = std::move(Queue.front());
+  std::tie(Task, Priority) = std::move(Queue.front());
   Queue.pop_front();
 }
+
+if (Priority != ThreadPriority::Normal)
+  setCurrentThreadPriority(Priority);
 (*Task)();
+if (Priority != ThreadPriority::Normal)
+  setCurrentThreadPriority(ThreadPriority::Normal);
+
 {
   std::unique_lock Lock(QueueMu);
   assert(NumActiveTasks > 0 && "before decrementing");
@@ -193,44 +195,60 @@ bool Back

[PATCH] D55315: [clangd] Only reduce priority of a thread for indexing.

2018-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE349345: [clangd] Only reduce priority of a thread for 
indexing. (authored by kadircet, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55315?vs=178443&id=178445#toc

Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55315/new/

https://reviews.llvm.org/D55315

Files:
  clangd/Threading.cpp
  clangd/Threading.h
  clangd/index/Background.cpp
  clangd/index/Background.h

Index: clangd/Threading.cpp
===
--- clangd/Threading.cpp
+++ clangd/Threading.cpp
@@ -112,13 +112,13 @@
 
 static std::atomic AvoidThreadStarvation = {false};
 
-void setThreadPriority(std::thread &T, ThreadPriority Priority) {
+void setCurrentThreadPriority(ThreadPriority Priority) {
   // Some *really* old glibcs are missing SCHED_IDLE.
 #if defined(__linux__) && defined(SCHED_IDLE)
   sched_param priority;
   priority.sched_priority = 0;
   pthread_setschedparam(
-  T.native_handle(),
+  pthread_self(),
   Priority == ThreadPriority::Low && !AvoidThreadStarvation ? SCHED_IDLE
 : SCHED_OTHER,
   &priority);
Index: clangd/Threading.h
===
--- clangd/Threading.h
+++ clangd/Threading.h
@@ -122,7 +122,7 @@
   Low = 0,
   Normal = 1,
 };
-void setThreadPriority(std::thread &T, ThreadPriority Priority);
+void setCurrentThreadPriority(ThreadPriority Priority);
 // Avoid the use of scheduler policies that may starve low-priority threads.
 // This prevents tests from timing out on loaded systems.
 // Affects subsequent setThreadPriority() calls.
Index: clangd/index/Background.h
===
--- clangd/index/Background.h
+++ clangd/index/Background.h
@@ -13,6 +13,7 @@
 #include "Context.h"
 #include "FSProvider.h"
 #include "GlobalCompilationDatabase.h"
+#include "Threading.h"
 #include "index/FileIndex.h"
 #include "index/Index.h"
 #include "index/Serialization.h"
@@ -110,14 +111,14 @@
   // queue management
   using Task = std::function;
   void run(); // Main loop executed by Thread. Runs tasks from Queue.
-  void enqueueTask(Task T);
+  void enqueueTask(Task T, ThreadPriority Prioirty);
   void enqueueLocked(tooling::CompileCommand Cmd,
  BackgroundIndexStorage *IndexStorage);
   std::mutex QueueMu;
   unsigned NumActiveTasks = 0; // Only idle when queue is empty *and* no tasks.
   std::condition_variable QueueCV;
   bool ShouldStop = false;
-  std::deque Queue;
+  std::deque> Queue;
   std::vector ThreadPool; // FIXME: Abstract this away.
   GlobalCompilationDatabase::CommandChanged::Subscription CommandsChanged;
 };
Index: clangd/index/Background.cpp
===
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -22,6 +22,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/SHA1.h"
@@ -135,14 +136,8 @@
   })) {
   assert(ThreadPoolSize > 0 && "Thread pool size can't be zero.");
   assert(this->IndexStorageFactory && "Storage factory can not be null!");
-  while (ThreadPoolSize--) {
+  while (ThreadPoolSize--)
 ThreadPool.emplace_back([this] { run(); });
-// Set priority to low, since background indexing is a long running task we
-// do not want to eat up cpu when there are any other high priority threads.
-// FIXME: In the future we might want a more general way of handling this to
-// support tasks with various priorities.
-setThreadPriority(ThreadPool.back(), ThreadPriority::Low);
-  }
 }
 
 BackgroundIndex::~BackgroundIndex() {
@@ -163,6 +158,7 @@
   WithContext Background(BackgroundContext.clone());
   while (true) {
 Optional Task;
+ThreadPriority Priority;
 {
   std::unique_lock Lock(QueueMu);
   QueueCV.wait(Lock, [&] { return ShouldStop || !Queue.empty(); });
@@ -172,10 +168,16 @@
 return;
   }
   ++NumActiveTasks;
-  Task = std::move(Queue.front());
+  std::tie(Task, Priority) = std::move(Queue.front());
   Queue.pop_front();
 }
+
+if (Priority != ThreadPriority::Normal)
+  setCurrentThreadPriority(Priority);
 (*Task)();
+if (Priority != ThreadPriority::Normal)
+  setCurrentThreadPriority(ThreadPriority::Normal);
+
 {
   std::unique_lock Lock(QueueMu);
   assert(NumActiveTasks > 0 && "before decrementing");
@@ -193,44 +195,60 @@
 }
 
 void BackgroundIndex::enqueue(const std::vector &ChangedFiles) {
-  enqueueTask([this, ChangedFiles] {
-trace::Span Tracer("BackgroundIndexEnqueue");
-// We're doing this asynchronously, becaus

[clang-tools-extra] r349348 - [clangd] Change diskbackedstorage to be atomic

2018-12-17 Thread Kadir Cetinkaya via cfe-commits

Author: kadircet
Date: Mon Dec 17 04:38:22 2018
New Revision: 349348

URL: http://llvm.org/viewvc/llvm-project?rev=349348&view=rev
Log:
[clangd] Change diskbackedstorage to be atomic

Summary:
There was a chance that multiple clangd instances could try to write
same shard, in which case we would get a malformed file most likely. This patch
changes the writing mechanism to first write to a temporary file and then rename
it to fit real destination. Which is guaranteed to be atomic by POSIX.

Reviewers: ilya-biryukov

Subscribers: ioeric, MaskRay, jkorous, arphaman, jfb, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/index/BackgroundIndexStorage.cpp

Modified: clang-tools-extra/trunk/clangd/index/BackgroundIndexStorage.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/BackgroundIndexStorage.cpp?rev=349348&r1=349347&r2=349348&view=diff
==
--- clang-tools-extra/trunk/clangd/index/BackgroundIndexStorage.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/BackgroundIndexStorage.cpp Mon Dec 17 
04:38:22 2018
@@ -9,6 +9,8 @@
 
 #include "Logger.h"
 #include "index/Background.h"
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -33,6 +35,34 @@ std::string getShardPathFromFilePath(llv
   return ShardRootSS.str();
 }
 
+llvm::Error
+writeAtomically(llvm::StringRef OutPath,
+llvm::function_ref Writer) {
+  // Write to a temporary file first.
+  llvm::SmallString<128> TempPath;
+  int FD;
+  auto EC =
+  llvm::sys::fs::createUniqueFile(OutPath + ".tmp.", FD, TempPath);
+  if (EC)
+return llvm::errorCodeToError(EC);
+  // Make sure temp file is destroyed on failure.
+  auto RemoveOnFail =
+  llvm::make_scope_exit([TempPath] { llvm::sys::fs::remove(TempPath); });
+  llvm::raw_fd_ostream OS(FD, /*shouldClose=*/true);
+  Writer(OS);
+  OS.close();
+  if (OS.has_error())
+return llvm::errorCodeToError(OS.error());
+  // Then move to real location.
+  EC = llvm::sys::fs::rename(TempPath, OutPath);
+  if (EC)
+return llvm::errorCodeToError(EC);
+  // If everything went well, we already moved the file to another name. So
+  // don't delete the file, as the name might be taken by another file.
+  RemoveOnFail.release();
+  return llvm::ErrorSuccess();
+}
+
 // Uses disk as a storage for index shards. Creates a directory called
 // ".clangd-index/" under the path provided during construction.
 class DiskBackedIndexStorage : public BackgroundIndexStorage {
@@ -70,14 +100,9 @@ public:
 
   llvm::Error storeShard(llvm::StringRef ShardIdentifier,
  IndexFileOut Shard) const override {
-auto ShardPath = getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
-std::error_code EC;
-llvm::raw_fd_ostream OS(ShardPath, EC);
-if (EC)
-  return llvm::errorCodeToError(EC);
-OS << Shard;
-OS.close();
-return llvm::errorCodeToError(OS.error());
+return writeAtomically(
+getShardPathFromFilePath(DiskShardRoot, ShardIdentifier),
+[&Shard](llvm::raw_ostream &OS) { OS << Shard; });
   }
 };
 


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

[PATCH] D55417: [clangd] Change disk-backed storage to be atomic

2018-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 178448.
kadircet marked 3 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55417/new/

https://reviews.llvm.org/D55417

Files:
  clangd/index/BackgroundIndexStorage.cpp


Index: clangd/index/BackgroundIndexStorage.cpp
===
--- clangd/index/BackgroundIndexStorage.cpp
+++ clangd/index/BackgroundIndexStorage.cpp
@@ -9,6 +9,8 @@
 
 #include "Logger.h"
 #include "index/Background.h"
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -33,6 +35,34 @@
   return ShardRootSS.str();
 }
 
+llvm::Error
+writeAtomically(llvm::StringRef OutPath,
+llvm::function_ref Writer) {
+  // Write to a temporary file first.
+  llvm::SmallString<128> TempPath;
+  int FD;
+  auto EC =
+  llvm::sys::fs::createUniqueFile(OutPath + ".tmp.", FD, TempPath);
+  if (EC)
+return llvm::errorCodeToError(EC);
+  // Make sure temp file is destroyed on failure.
+  auto RemoveOnFail =
+  llvm::make_scope_exit([TempPath] { llvm::sys::fs::remove(TempPath); });
+  llvm::raw_fd_ostream OS(FD, /*shouldClose=*/true);
+  Writer(OS);
+  OS.close();
+  if (OS.has_error())
+return llvm::errorCodeToError(OS.error());
+  // Then move to real location.
+  EC = llvm::sys::fs::rename(TempPath, OutPath);
+  if (EC)
+return llvm::errorCodeToError(EC);
+  // If everything went well, we already moved the file to another name. So
+  // don't delete the file, as the name might be taken by another file.
+  RemoveOnFail.release();
+  return llvm::ErrorSuccess();
+}
+
 // Uses disk as a storage for index shards. Creates a directory called
 // ".clangd-index/" under the path provided during construction.
 class DiskBackedIndexStorage : public BackgroundIndexStorage {
@@ -70,14 +100,9 @@
 
   llvm::Error storeShard(llvm::StringRef ShardIdentifier,
  IndexFileOut Shard) const override {
-auto ShardPath = getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
-std::error_code EC;
-llvm::raw_fd_ostream OS(ShardPath, EC);
-if (EC)
-  return llvm::errorCodeToError(EC);
-OS << Shard;
-OS.close();
-return llvm::errorCodeToError(OS.error());
+return writeAtomically(
+getShardPathFromFilePath(DiskShardRoot, ShardIdentifier),
+[&Shard](llvm::raw_ostream &OS) { OS << Shard; });
   }
 };
 


Index: clangd/index/BackgroundIndexStorage.cpp
===
--- clangd/index/BackgroundIndexStorage.cpp
+++ clangd/index/BackgroundIndexStorage.cpp
@@ -9,6 +9,8 @@
 
 #include "Logger.h"
 #include "index/Background.h"
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -33,6 +35,34 @@
   return ShardRootSS.str();
 }
 
+llvm::Error
+writeAtomically(llvm::StringRef OutPath,
+llvm::function_ref Writer) {
+  // Write to a temporary file first.
+  llvm::SmallString<128> TempPath;
+  int FD;
+  auto EC =
+  llvm::sys::fs::createUniqueFile(OutPath + ".tmp.", FD, TempPath);
+  if (EC)
+return llvm::errorCodeToError(EC);
+  // Make sure temp file is destroyed on failure.
+  auto RemoveOnFail =
+  llvm::make_scope_exit([TempPath] { llvm::sys::fs::remove(TempPath); });
+  llvm::raw_fd_ostream OS(FD, /*shouldClose=*/true);
+  Writer(OS);
+  OS.close();
+  if (OS.has_error())
+return llvm::errorCodeToError(OS.error());
+  // Then move to real location.
+  EC = llvm::sys::fs::rename(TempPath, OutPath);
+  if (EC)
+return llvm::errorCodeToError(EC);
+  // If everything went well, we already moved the file to another name. So
+  // don't delete the file, as the name might be taken by another file.
+  RemoveOnFail.release();
+  return llvm::ErrorSuccess();
+}
+
 // Uses disk as a storage for index shards. Creates a directory called
 // ".clangd-index/" under the path provided during construction.
 class DiskBackedIndexStorage : public BackgroundIndexStorage {
@@ -70,14 +100,9 @@
 
   llvm::Error storeShard(llvm::StringRef ShardIdentifier,
  IndexFileOut Shard) const override {
-auto ShardPath = getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
-std::error_code EC;
-llvm::raw_fd_ostream OS(ShardPath, EC);
-if (EC)
-  return llvm::errorCodeToError(EC);
-OS << Shard;
-OS.close();
-return llvm::errorCodeToError(OS.error());
+return writeAtomically(
+getShardPathFromFilePath(DiskShardRoot, ShardIdentifier),
+[&Shard](llvm::raw_ostream &OS) { OS << Shard; });
   }
 };
 
___
cfe-commits mailing list
cfe-commits@li

[PATCH] D55417: [clangd] Change disk-backed storage to be atomic

2018-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE349348: [clangd] Change diskbackedstorage to be atomic 
(authored by kadircet, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55417?vs=178448&id=178449#toc

Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55417/new/

https://reviews.llvm.org/D55417

Files:
  clangd/index/BackgroundIndexStorage.cpp


Index: clangd/index/BackgroundIndexStorage.cpp
===
--- clangd/index/BackgroundIndexStorage.cpp
+++ clangd/index/BackgroundIndexStorage.cpp
@@ -9,6 +9,8 @@
 
 #include "Logger.h"
 #include "index/Background.h"
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -33,6 +35,34 @@
   return ShardRootSS.str();
 }
 
+llvm::Error
+writeAtomically(llvm::StringRef OutPath,
+llvm::function_ref Writer) {
+  // Write to a temporary file first.
+  llvm::SmallString<128> TempPath;
+  int FD;
+  auto EC =
+  llvm::sys::fs::createUniqueFile(OutPath + ".tmp.", FD, TempPath);
+  if (EC)
+return llvm::errorCodeToError(EC);
+  // Make sure temp file is destroyed on failure.
+  auto RemoveOnFail =
+  llvm::make_scope_exit([TempPath] { llvm::sys::fs::remove(TempPath); });
+  llvm::raw_fd_ostream OS(FD, /*shouldClose=*/true);
+  Writer(OS);
+  OS.close();
+  if (OS.has_error())
+return llvm::errorCodeToError(OS.error());
+  // Then move to real location.
+  EC = llvm::sys::fs::rename(TempPath, OutPath);
+  if (EC)
+return llvm::errorCodeToError(EC);
+  // If everything went well, we already moved the file to another name. So
+  // don't delete the file, as the name might be taken by another file.
+  RemoveOnFail.release();
+  return llvm::ErrorSuccess();
+}
+
 // Uses disk as a storage for index shards. Creates a directory called
 // ".clangd-index/" under the path provided during construction.
 class DiskBackedIndexStorage : public BackgroundIndexStorage {
@@ -70,14 +100,9 @@
 
   llvm::Error storeShard(llvm::StringRef ShardIdentifier,
  IndexFileOut Shard) const override {
-auto ShardPath = getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
-std::error_code EC;
-llvm::raw_fd_ostream OS(ShardPath, EC);
-if (EC)
-  return llvm::errorCodeToError(EC);
-OS << Shard;
-OS.close();
-return llvm::errorCodeToError(OS.error());
+return writeAtomically(
+getShardPathFromFilePath(DiskShardRoot, ShardIdentifier),
+[&Shard](llvm::raw_ostream &OS) { OS << Shard; });
   }
 };
 


Index: clangd/index/BackgroundIndexStorage.cpp
===
--- clangd/index/BackgroundIndexStorage.cpp
+++ clangd/index/BackgroundIndexStorage.cpp
@@ -9,6 +9,8 @@
 
 #include "Logger.h"
 #include "index/Background.h"
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -33,6 +35,34 @@
   return ShardRootSS.str();
 }
 
+llvm::Error
+writeAtomically(llvm::StringRef OutPath,
+llvm::function_ref Writer) {
+  // Write to a temporary file first.
+  llvm::SmallString<128> TempPath;
+  int FD;
+  auto EC =
+  llvm::sys::fs::createUniqueFile(OutPath + ".tmp.", FD, TempPath);
+  if (EC)
+return llvm::errorCodeToError(EC);
+  // Make sure temp file is destroyed on failure.
+  auto RemoveOnFail =
+  llvm::make_scope_exit([TempPath] { llvm::sys::fs::remove(TempPath); });
+  llvm::raw_fd_ostream OS(FD, /*shouldClose=*/true);
+  Writer(OS);
+  OS.close();
+  if (OS.has_error())
+return llvm::errorCodeToError(OS.error());
+  // Then move to real location.
+  EC = llvm::sys::fs::rename(TempPath, OutPath);
+  if (EC)
+return llvm::errorCodeToError(EC);
+  // If everything went well, we already moved the file to another name. So
+  // don't delete the file, as the name might be taken by another file.
+  RemoveOnFail.release();
+  return llvm::ErrorSuccess();
+}
+
 // Uses disk as a storage for index shards. Creates a directory called
 // ".clangd-index/" under the path provided during construction.
 class DiskBackedIndexStorage : public BackgroundIndexStorage {
@@ -70,14 +100,9 @@
 
   llvm::Error storeShard(llvm::StringRef ShardIdentifier,
  IndexFileOut Shard) const override {
-auto ShardPath = getShardPathFromFilePath(DiskShardRoot, ShardIdentifier);
-std::error_code EC;
-llvm::raw_fd_ostream OS(ShardPath, EC);
-if (EC)
-  return llvm::errorCodeToError(EC);
-OS << Shard;
-OS.close();
-return llvm::errorCodeToError(OS.error());
+return writeAtomically(
+getShardPathFromFilePath(DiskShardRoot, ShardIdentifier),
+[&Shard](llv

[PATCH] D53655: [ASTImporter] Fix redecl chain of classes and class templates

2018-12-17 Thread Gabor Marton via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rC349349: [ASTImporter] Fix redecl chain of classes and class 
templates (authored by martong, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53655?vs=175641&id=178450#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53655/new/

https://reviews.llvm.org/D53655

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/AST/ASTImporter.cpp
  lib/AST/DeclBase.cpp
  lib/ASTMatchers/ASTMatchersInternal.cpp
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/AST/ASTImporterTest.cpp
  unittests/AST/StructuralEquivalenceTest.cpp

Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -300,6 +300,16 @@
 
 
 
+MatcherDecl>indirectFieldDeclMatcherIndirectFieldDecl>...
+Matches indirect field declarations.
+
+Given
+  struct X { struct { int a; }; };
+indirectFieldDecl()
+  matches 'a'.
+
+
+
 MatcherDecl>labelDeclMatcherLabelDecl>...
 Matches a declaration of label.
 
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -1158,6 +1158,17 @@
 ///   matches 'm'.
 extern const internal::VariadicDynCastAllOfMatcher fieldDecl;
 
+/// Matches indirect field declarations.
+///
+/// Given
+/// \code
+///   struct X { struct { int a; }; };
+/// \endcode
+/// indirectFieldDecl()
+///   matches 'a'.
+extern const internal::VariadicDynCastAllOfMatcher
+indirectFieldDecl;
+
 /// Matches function declarations.
 ///
 /// Example matches f
Index: lib/AST/DeclBase.cpp
===
--- lib/AST/DeclBase.cpp
+++ lib/AST/DeclBase.cpp
@@ -1463,7 +1463,9 @@
   if (Map) {
 StoredDeclsMap::iterator Pos = Map->find(ND->getDeclName());
 assert(Pos != Map->end() && "no lookup entry for decl");
-if (Pos->second.getAsVector() || Pos->second.getAsDecl() == ND)
+// Remove the decl only if it is contained.
+StoredDeclsList::DeclsTy *Vec = Pos->second.getAsVector();
+if ((Vec && is_contained(*Vec, ND)) || Pos->second.getAsDecl() == ND)
   Pos->second.remove(ND);
   }
 } while (DC->isTransparentContext() && (DC = DC->getParent()));
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -122,6 +122,8 @@
   return getCanonicalForwardRedeclChain(FD);
 if (auto *VD = dyn_cast(D))
   return getCanonicalForwardRedeclChain(VD);
+if (auto *TD = dyn_cast(D))
+  return getCanonicalForwardRedeclChain(TD);
 llvm_unreachable("Bad declaration kind");
   }
 
@@ -2607,10 +2609,9 @@
   return std::move(Err);
 IDNS = Decl::IDNS_Ordinary;
   } else if (Importer.getToContext().getLangOpts().CPlusPlus)
-IDNS |= Decl::IDNS_Ordinary;
+IDNS |= Decl::IDNS_Ordinary | Decl::IDNS_TagFriend;
 
   // We may already have a record of the same name; try to find and match it.
-  RecordDecl *AdoptDecl = nullptr;
   RecordDecl *PrevDecl = nullptr;
   if (!DC->isFunctionOrMethod()) {
 SmallVector ConflictingDecls;
@@ -2643,26 +2644,22 @@
   }
 
   if (auto *FoundRecord = dyn_cast(Found)) {
-if (!SearchName) {
+// Do not emit false positive diagnostic in case of unnamed
+// struct/union and in case of anonymous structs.  Would be false
+// because there may be several anonymous/unnamed structs in a class.
+// E.g. these are both valid:
+//  struct A { // unnamed structs
+//struct { struct A *next; } entry0;
+//struct { struct A *next; } entry1;
+//  };
+//  struct X { struct { int a; }; struct { int b; }; }; // anon structs
+if (!SearchName)
   if (!IsStructuralMatch(D, FoundRecord, false))
 continue;
-} else {
-  if (!IsStructuralMatch(D, FoundRecord)) {
-ConflictingDecls.push_back(FoundDecl);
-continue;
-  }
-}
 
-PrevDecl = FoundRecord;
-
-if (RecordDecl *FoundDef = FoundRecord->getDefinition()) {
-  if ((SearchName && !D->isCompleteDefinition() && !IsFriendTemplate)
-  || (D->isCompleteDefinition() &&
-  D->isAnonymousStructOrUnion()
-== FoundDef->isAnonymousStructOrUnion())) {
-// The record types structurally match, or the "from" translation
-

r349349 - [ASTImporter] Fix redecl chain of classes and class templates

2018-12-17 Thread Gabor Marton via cfe-commits

Author: martong
Date: Mon Dec 17 04:42:12 2018
New Revision: 349349

URL: http://llvm.org/viewvc/llvm-project?rev=349349&view=rev
Log:
[ASTImporter] Fix redecl chain of classes and class templates

Summary:
The crux of the issue that is being fixed is that lookup could not find
previous decls of a friend class. The solution involves making the
friend declarations visible in their decl context (i.e. adding them to
the lookup table).
Also, we simplify `VisitRecordDecl` greatly.

This fix involves two other repairs (without these the unittests fail):
(1) We could not handle the addition of injected class types properly
when a redecl chain was involved, now this is fixed.
(2) DeclContext::removeDecl failed if the lookup table in Vector form
did not contain the to be removed element. This caused troubles in
ASTImporter::ImportDeclContext. This is also fixed.

Reviewers: a_sidorin, balazske, a.sidorin

Subscribers: rnkovacs, dkrupp, Szelethus, cfe-commits

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

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/lib/AST/DeclBase.cpp
cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp
cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp
cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=349349&r1=349348&r2=349349&view=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Mon Dec 17 04:42:12 2018
@@ -300,6 +300,16 @@ Example matches f
 
 
 
+MatcherDecl>indirectFieldDeclMatcherIndirectFieldDecl>...
+Matches indirect 
field declarations.
+
+Given
+  struct X { struct { int a; }; };
+indirectFieldDecl()
+  matches 'a'.
+
+
+
 MatcherDecl>labelDeclMatcherLabelDecl>...
 Matches a declaration of 
label.
 

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=349349&r1=349348&r2=349349&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Mon Dec 17 04:42:12 2018
@@ -1158,6 +1158,17 @@ extern const internal::VariadicDynCastAl
 ///   matches 'm'.
 extern const internal::VariadicDynCastAllOfMatcher fieldDecl;
 
+/// Matches indirect field declarations.
+///
+/// Given
+/// \code
+///   struct X { struct { int a; }; };
+/// \endcode
+/// indirectFieldDecl()
+///   matches 'a'.
+extern const internal::VariadicDynCastAllOfMatcher
+indirectFieldDecl;
+
 /// Matches function declarations.
 ///
 /// Example matches f

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=349349&r1=349348&r2=349349&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Mon Dec 17 04:42:12 2018
@@ -122,6 +122,8 @@ namespace clang {
   return getCanonicalForwardRedeclChain(FD);
 if (auto *VD = dyn_cast(D))
   return getCanonicalForwardRedeclChain(VD);
+if (auto *TD = dyn_cast(D))
+  return getCanonicalForwardRedeclChain(TD);
 llvm_unreachable("Bad declaration kind");
   }
 
@@ -2607,10 +2609,9 @@ ExpectedDecl ASTNodeImporter::VisitRecor
   return std::move(Err);
 IDNS = Decl::IDNS_Ordinary;
   } else if (Importer.getToContext().getLangOpts().CPlusPlus)
-IDNS |= Decl::IDNS_Ordinary;
+IDNS |= Decl::IDNS_Ordinary | Decl::IDNS_TagFriend;
 
   // We may already have a record of the same name; try to find and match it.
-  RecordDecl *AdoptDecl = nullptr;
   RecordDecl *PrevDecl = nullptr;
   if (!DC->isFunctionOrMethod()) {
 SmallVector ConflictingDecls;
@@ -2643,26 +2644,22 @@ ExpectedDecl ASTNodeImporter::VisitRecor
   }
 
   if (auto *FoundRecord = dyn_cast(Found)) {
-if (!SearchName) {
+// Do not emit false positive diagnostic in case of unnamed
+// struct/union and in case of anonymous structs.  Would be false
+// because there may be several anonymous/unnamed structs in a class.
+// E.g. these are both valid:
+//  struct A { // unnamed structs
+//struct { struct A *next; } entry0;
+//struct { struct A *next; } entry1;
+//  };
+//

[PATCH] D55683: [analyzer] Tests for scan-build?

2018-12-17 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

Could we make use of scan-build `-o` to specify the exact output location, in 
conjunction with `stable-report-filename=true` to remove the non-determinism? 
Also, I like the idea of maybe someday making scan-build-py test the same file. 
However, it seems like that project is basically unmaintained so it may effort 
to reach parity before that can be enabled.

Thank you for working on the testing infrastructure for this! I think it's 
super helpful because it gives us a path forward to eventually rewrite 
scan-build to be more appropriate for all the platforms Clang runs on (like 
Windows, where the reliance on Perl is a serious hurdle).


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55683/new/

https://reviews.llvm.org/D55683



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

[PATCH] D53708: [ASTImporter] Add importer specific lookup

2018-12-17 Thread Gabor Marton via Phabricator via cfe-commits

martong updated this revision to Diff 178452.
martong added a comment.

- Rebase to master (trunk)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53708/new/

https://reviews.llvm.org/D53708

Files:
  include/clang/AST/ASTImporter.h
  include/clang/AST/ASTImporterLookupTable.h
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/AST/ASTImporter.cpp
  lib/AST/ASTImporterLookupTable.cpp
  lib/AST/CMakeLists.txt
  lib/CrossTU/CrossTranslationUnit.cpp
  lib/Frontend/ASTMerge.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -15,6 +15,8 @@
 #include "MatchVerifier.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclContextInternals.h"
+#include "clang/AST/ASTImporter.h"
+#include "clang/AST/ASTImporterLookupTable.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
@@ -308,24 +310,27 @@
   Unit->enableSourceFileDiagnostics();
 }
 
-void lazyInitImporter(ASTUnit *ToAST) {
+void lazyInitImporter(ASTImporterLookupTable &LookupTable, ASTUnit *ToAST) {
   assert(ToAST);
   if (!Importer) {
-Importer.reset(new ASTImporter(
-ToAST->getASTContext(), ToAST->getFileManager(),
-Unit->getASTContext(), Unit->getFileManager(), false));
+Importer.reset(
+new ASTImporter(ToAST->getASTContext(), ToAST->getFileManager(),
+Unit->getASTContext(), Unit->getFileManager(),
+false, &LookupTable));
   }
   assert(&ToAST->getASTContext() == &Importer->getToContext());
   createVirtualFileIfNeeded(ToAST, FileName, Code);
 }
 
-Decl *import(ASTUnit *ToAST, Decl *FromDecl) {
-  lazyInitImporter(ToAST);
+Decl *import(ASTImporterLookupTable &LookupTable, ASTUnit *ToAST,
+ Decl *FromDecl) {
+  lazyInitImporter(LookupTable, ToAST);
   return Importer->Import(FromDecl);
- }
+}
 
-QualType import(ASTUnit *ToAST, QualType FromType) {
-  lazyInitImporter(ToAST);
+QualType import(ASTImporterLookupTable &LookupTable, ASTUnit *ToAST,
+QualType FromType) {
+  lazyInitImporter(LookupTable, ToAST);
   return Importer->Import(FromType);
 }
   };
@@ -339,13 +344,23 @@
   // vector is expanding, with the list we won't have these issues.
   std::list FromTUs;
 
-  void lazyInitToAST(Language ToLang) {
+  // Initialize the lookup table if not initialized already.
+  void lazyInitLookupTable(TranslationUnitDecl *ToTU) {
+assert(ToTU);
+if (!LookupTablePtr)
+  LookupTablePtr = llvm::make_unique(*ToTU);
+  }
+
+  void lazyInitToAST(Language ToLang, StringRef ToSrcCode, StringRef FileName) {
 if (ToAST)
   return;
 ArgVector ToArgs = getArgVectorForLanguage(ToLang);
+// Source code must be a valid live buffer through the tests lifetime.
+ToCode = ToSrcCode;
 // Build the AST from an empty file.
-ToAST = tooling::buildASTFromCodeWithArgs(/*Code=*/"", ToArgs, "empty.cc");
+ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, FileName);
 ToAST->enableSourceFileDiagnostics();
+lazyInitLookupTable(ToAST->getASTContext().getTranslationUnitDecl());
   }
 
   TU *findFromTU(Decl *From) {
@@ -359,6 +374,10 @@
 return &*It;
   }
 
+protected:
+
+  std::unique_ptr LookupTablePtr;
+
 public:
   // We may have several From context but only one To context.
   std::unique_ptr ToAST;
@@ -375,26 +394,23 @@
 FromTUs.emplace_back(FromSrcCode, InputFileName, FromArgs);
 TU &FromTU = FromTUs.back();
 
-ToCode = ToSrcCode;
 assert(!ToAST);
-ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, OutputFileName);
-ToAST->enableSourceFileDiagnostics();
+lazyInitToAST(ToLang, ToSrcCode, OutputFileName);
 
 ASTContext &FromCtx = FromTU.Unit->getASTContext();
 
-createVirtualFileIfNeeded(ToAST.get(), InputFileName, FromTU.Code);
-
 IdentifierInfo *ImportedII = &FromCtx.Idents.get(Identifier);
 assert(ImportedII && "Declaration with the given identifier "
  "should be specified in test!");
 DeclarationName ImportDeclName(ImportedII);
-SmallVector FoundDecls;
+SmallVector FoundDecls;
 FromCtx.getTranslationUnitDecl()->localUncachedLookup(ImportDeclName,
   FoundDecls);
 
 assert(FoundDecls.size() == 1);
 
-Decl *Imported = FromTU.import(ToAST.get(), FoundDecls.front());
+Decl *Imported =
+FromTU.import(*LookupTablePtr, ToAST.get(), FoundDecls.front());
 
 assert(Imported);
 return std::make_tuple(*FoundDecls.begin(), Imported);
@@ -420,11 +436,8 @@
   // Creates the To context with the given source code and returns the TU decl.
   Translat

[PATCH] D54823: [analyzer][MallocChecker][NFC] Document and reorganize some functions

2018-12-17 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus added a comment.

Lit test failures are gone for Windows after reverting this. Will probably deal 
with this revision after 8.0.0.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54823/new/

https://reviews.llvm.org/D54823



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

[PATCH] D55656: [OpenCL] Address space for default class members

2018-12-17 Thread Anastasia Stulova via Phabricator via cfe-commits

Anastasia added inline comments.



Comment at: lib/AST/ASTContext.cpp:2781
+
+  return getAddrSpaceQualType(NewT, Orig.getAddressSpace());
 }

rjmccall wrote:
> You're trying to handle a method qualifier, not a type a functions that are 
> themselves in some non-standard address space, right?  The method qualifier 
> should already be part of `Proto->getExtProtoInfo()`, so if there's an 
> address space qualifier out here, something is very wrong.
As far as I understand the new design, we have an address space qualifier on a 
method and as a part of the function prototype too. Are you saying that we need 
to make sure the prototype has an address space too?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55656/new/

https://reviews.llvm.org/D55656



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

[PATCH] D53708: [ASTImporter] Add importer specific lookup

2018-12-17 Thread Bruno Ricci via Phabricator via cfe-commits

riccibruno added a comment.

This is a perhaps a naive comment, but why is `localUncachedLookup` used
instead of `noload_lookup` ? There is a fixme dating from 2013 stating
that `noload_lookup` should be used instead.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53708/new/

https://reviews.llvm.org/D53708



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

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

2018-12-17 Thread Haojian Wu via Phabricator via cfe-commits

hokein updated this revision to Diff 178455.
hokein added a comment.

Update:

- avoid using terms that C++ programmers are unfamiliar with.
- textDocument/fileStatus => textDocument/clangd.fileStatus


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55363/new/

https://reviews.llvm.org/D55363

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  test/clangd/filestatus.test

Index: test/clangd/filestatus.test
===
--- /dev/null
+++ test/clangd/filestatus.test
@@ -0,0 +1,14 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"int x; int y = x;"}}}
+#  CHECK:  "method": "textDocument/clangd.fileStatus",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:"details": [],
+# CHECK-NEXT:"state": "Parsing includes",
+# CHECK-NEXT:"uri": "{{.*}}/main.cpp"
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -77,6 +77,8 @@
 /// Indicates whether we reused the prebuilt AST.
 bool ReuseAST = false;
   };
+  /// Serialize this to an LSP file status item.
+  FileStatus render(PathRef File) const;
 
   TUAction Action;
   BuildDetails Details;
Index: clangd/TUScheduler.cpp
===
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -729,6 +729,34 @@
   return wait(Lock, RequestsCV, Timeout, [&] { return Requests.empty(); });
 }
 
+// Render a TUAction to a user-facing string representation.
+// TUAction represents clangd-internal states, we don't intend to expose them
+// to users (say C++ programmers) directly to avoid confusion, we use terms that
+// are familiar by C++ programmers.
+std::string renderTUAction(const TUAction& Action) {
+  std::string Result;
+  raw_string_ostream OS(Result);
+  switch (Action.S) {
+  case TUAction::Queued:
+OS << "Queued";
+break;
+  case TUAction::RunningAction:
+OS << "RunningAction"
+   << "(" << Action.Name << ")";
+break;
+  case TUAction::BuildingPreamble:
+OS << "Parsing includes";
+break;
+  case TUAction::BuildingFile:
+OS << "Parsing main file";
+break;
+  case TUAction::Idle:
+OS << "Idle";
+break;
+  }
+  return OS.str();
+}
+
 } // namespace
 
 unsigned getDefaultAsyncThreadsCount() {
@@ -741,6 +769,17 @@
   return HardwareConcurrency;
 }
 
+FileStatus TUStatus::render(PathRef File) const {
+  FileStatus FStatus;
+  FStatus.uri = URIForFile::canonicalize(File, /*TUPath=*/File);
+  FStatus.state = renderTUAction(Action);
+  if (Details.BuildFailed)
+FStatus.details.push_back({MessageType::Error, "failed to build the file"});
+  if (Details.ReuseAST)
+FStatus.details.push_back({MessageType::Info, "reused the AST"});
+  return FStatus;
+}
+
 struct TUScheduler::FileData {
   /// Latest inputs, passed to TUScheduler::update().
   std::string Contents;
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -973,6 +973,33 @@
 };
 bool fromJSON(const llvm::json::Value &, ReferenceParams &);
 
+enum class MessageType {
+  Error = 1,
+  Warning = 2,
+  Info = 3,
+  Log = 4,
+};
+struct ShowMessageParams {
+  /// The message type.
+  MessageType type;
+  /// The actual message.
+  std::string message;
+};
+llvm::json::Value toJSON(const ShowMessageParams &);
+
+/// Clangd extension: indicates the current state of the file in clangd,
+/// sent from server via the `textDocument/fileStatus` notification.
+struct FileStatus {
+  /// The text document's URI.
+  URIForFile uri;
+  /// The human-readable string presents the current state of the file, can be
+  /// shown in the UI (e.g. status bar).
+  std::string state;
+  /// Details of the state that are worth sufacing to users.
+  std::vector details;
+};
+llvm::json::Value toJSON(const FileStatus &FStatus);
+
 } // namespace clangd
 } // namespace clang
 
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -716,6 +716,20 @@
   };
 }
 
+llvm::json::Value toJSON(const ShowMessageParams &S) {
+  return json::Object{
+  {"type", static_cast(S.type)}, {"message", S.message},
+  };
+}
+
+llvm::json::Value toJSON(const FileStatus &FStatus) {
+  return json::Object{
+  {"uri", FStatus.uri},
+  {"state", FStatus.state},
+  {"details", FS

[PATCH] D55363: [clangd] Expose FileStatus in LSP.

2018-12-17 Thread Haojian Wu via Phabricator via cfe-commits

hokein added a comment.

> Sorry if I missed any important design discussions here, but wanted to clear 
> up what information we are trying to convey to the user with the status 
> messages?
>  E.g. why seeing "building preamble", "building file" or "queued" in the 
> status bar can be useful to the user? Those messages mention internals of 
> clangd, I'm not sure how someone unfamiliar with internals of clangd should 
> interpret this information.

Leaking some details to users is fine -- it can help users build the model of 
how clangd work, even though they don't understand all details, users will 
workout "building file" will take a while before "idle", and when the file is 
idle, clangd is responsive for upcoming requests.
I also agree that we should try to avoid using terms that C++ programmers are 
unfamiliar with.

> Good point, we should definitely state it's a clangd-specific extension. We 
> could come up with a naming scheme for our extensions. I would propose 
> something like clangd:$methodName, i.e. the current extensions would be 
> clangd:textDocument/fileStatus.

Adding clang-specific words to clangd-extension methods sounds good to me (to 
avoid confusion, the extension `textDocument/switchFromHeader` confused me 
before). I'd vote for `textDocument/clangd.FileStatus`.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55363/new/

https://reviews.llvm.org/D55363



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

r349351 - [ASTImporter] Add importer specific lookup

2018-12-17 Thread Gabor Marton via cfe-commits

Author: martong
Date: Mon Dec 17 05:53:12 2018
New Revision: 349351

URL: http://llvm.org/viewvc/llvm-project?rev=349351&view=rev
Log:
[ASTImporter] Add importer specific lookup

Summary:
There are certain cases when normal C/C++ lookup (localUncachedLookup)
does not find AST nodes. E.g.:

Example 1:

  template 
  struct X {
friend void foo(); // this is never found in the DC of the TU.
  };

Example 2:

  // The fwd decl to Foo is not found in the lookupPtr of the DC of the
  // translation unit decl.
  struct A { struct Foo *p; };

In these cases we create a new node instead of returning with the old one.
To fix it we create a new lookup table which holds every node and we are
not interested in any C++ specific visibility considerations.
Simply, we must know if there is an existing Decl in a given DC.

Reviewers: a_sidorin, a.sidorin

Subscribers: mgorny, rnkovacs, dkrupp, Szelethus, cfe-commits

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

Added:
cfe/trunk/include/clang/AST/ASTImporterLookupTable.h
cfe/trunk/lib/AST/ASTImporterLookupTable.cpp
Modified:
cfe/trunk/include/clang/AST/ASTImporter.h
cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/lib/AST/CMakeLists.txt
cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp
cfe/trunk/lib/Frontend/ASTMerge.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/include/clang/AST/ASTImporter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTImporter.h?rev=349351&r1=349350&r2=349351&view=diff
==
--- cfe/trunk/include/clang/AST/ASTImporter.h (original)
+++ cfe/trunk/include/clang/AST/ASTImporter.h Mon Dec 17 05:53:12 2018
@@ -33,6 +33,7 @@
 namespace clang {
 
 class ASTContext;
+class ASTImporterLookupTable;
 class CXXBaseSpecifier;
 class CXXCtorInitializer;
 class Decl;
@@ -80,12 +81,21 @@ class Attr;
   /// Imports selected nodes from one AST context into another context,
   /// merging AST nodes where appropriate.
   class ASTImporter {
+friend class ASTNodeImporter;
   public:
 using NonEquivalentDeclSet = llvm::DenseSet>;
 using ImportedCXXBaseSpecifierMap =
 llvm::DenseMap;
 
   private:
+
+/// Pointer to the import specific lookup table, which may be shared
+/// amongst several ASTImporter objects.
+/// This is an externally managed resource (and should exist during the
+/// lifetime of the ASTImporter object)
+/// If not set then the original C/C++ lookup is used.
+ASTImporterLookupTable *LookupTable = nullptr;
+
 /// The contexts we're importing to and from.
 ASTContext &ToContext, &FromContext;
 
@@ -123,9 +133,13 @@ class Attr;
 /// (which we have already complained about).
 NonEquivalentDeclSet NonEquivalentDecls;
 
+using FoundDeclsTy = SmallVector;
+FoundDeclsTy findDeclsInToCtx(DeclContext *DC, DeclarationName Name);
+
+void AddToLookupTable(Decl *ToD);
+
   public:
-/// Create a new AST importer.
-///
+
 /// \param ToContext The context we'll be importing into.
 ///
 /// \param ToFileManager The file manager we'll be importing into.
@@ -137,9 +151,14 @@ class Attr;
 /// \param MinimalImport If true, the importer will attempt to import
 /// as little as it can, e.g., by importing declarations as forward
 /// declarations that can be completed at a later point.
+///
+/// \param LookupTable The importer specific lookup table which may be
+/// shared amongst several ASTImporter objects.
+/// If not set then the original C/C++ lookup is used.
 ASTImporter(ASTContext &ToContext, FileManager &ToFileManager,
 ASTContext &FromContext, FileManager &FromFileManager,
-bool MinimalImport);
+bool MinimalImport,
+ASTImporterLookupTable *LookupTable = nullptr);
 
 virtual ~ASTImporter();
 

Added: cfe/trunk/include/clang/AST/ASTImporterLookupTable.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTImporterLookupTable.h?rev=349351&view=auto
==
--- cfe/trunk/include/clang/AST/ASTImporterLookupTable.h (added)
+++ cfe/trunk/include/clang/AST/ASTImporterLookupTable.h Mon Dec 17 05:53:12 
2018
@@ -0,0 +1,75 @@
+//===- ASTImporterLookupTable.h - ASTImporter specific lookup--*- C++ 
-*---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  This file defines the ASTImporterLookupTable class which implements a
+//  lookup procedure for the import mechanism.
+//
+//===--===//
+
+#ifndef LLV

[PATCH] D53708: [ASTImporter] Add importer specific lookup

2018-12-17 Thread Gabor Marton via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL349351: [ASTImporter] Add importer specific lookup (authored 
by martong, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53708/new/

https://reviews.llvm.org/D53708

Files:
  cfe/trunk/include/clang/AST/ASTImporter.h
  cfe/trunk/include/clang/AST/ASTImporterLookupTable.h
  cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/lib/AST/ASTImporterLookupTable.cpp
  cfe/trunk/lib/AST/CMakeLists.txt
  cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp
  cfe/trunk/lib/Frontend/ASTMerge.cpp
  cfe/trunk/unittests/AST/ASTImporterTest.cpp

Index: cfe/trunk/lib/AST/ASTImporterLookupTable.cpp
===
--- cfe/trunk/lib/AST/ASTImporterLookupTable.cpp
+++ cfe/trunk/lib/AST/ASTImporterLookupTable.cpp
@@ -0,0 +1,129 @@
+//===- ASTImporterLookupTable.cpp - ASTImporter specific lookup ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  This file defines the ASTImporterLookupTable class which implements a
+//  lookup procedure for the import mechanism.
+//
+//===--===//
+
+#include "clang/AST/ASTImporterLookupTable.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+
+namespace clang {
+
+namespace {
+
+struct Builder : RecursiveASTVisitor {
+  ASTImporterLookupTable <
+  Builder(ASTImporterLookupTable <) : LT(LT) {}
+  bool VisitNamedDecl(NamedDecl *D) {
+LT.add(D);
+return true;
+  }
+  bool VisitFriendDecl(FriendDecl *D) {
+if (D->getFriendType()) {
+  QualType Ty = D->getFriendType()->getType();
+  // FIXME Can this be other than elaborated?
+  QualType NamedTy = cast(Ty)->getNamedType();
+  if (!NamedTy->isDependentType()) {
+if (const auto *RTy = dyn_cast(NamedTy))
+  LT.add(RTy->getAsCXXRecordDecl());
+else if (const auto *SpecTy =
+ dyn_cast(NamedTy)) {
+  LT.add(SpecTy->getAsCXXRecordDecl());
+}
+  }
+}
+return true;
+  }
+
+  // Override default settings of base.
+  bool shouldVisitTemplateInstantiations() const { return true; }
+  bool shouldVisitImplicitCode() const { return true; }
+};
+
+} // anonymous namespace
+
+ASTImporterLookupTable::ASTImporterLookupTable(TranslationUnitDecl &TU) {
+  Builder B(*this);
+  B.TraverseDecl(&TU);
+}
+
+void ASTImporterLookupTable::add(DeclContext *DC, NamedDecl *ND) {
+  DeclList &Decls = LookupTable[DC][ND->getDeclName()];
+  // Inserts if and only if there is no element in the container equal to it.
+  Decls.insert(ND);
+}
+
+void ASTImporterLookupTable::remove(DeclContext *DC, NamedDecl *ND) {
+  DeclList &Decls = LookupTable[DC][ND->getDeclName()];
+  bool EraseResult = Decls.remove(ND);
+  (void)EraseResult;
+  assert(EraseResult == true && "Trying to remove not contained Decl");
+}
+
+void ASTImporterLookupTable::add(NamedDecl *ND) {
+  assert(ND);
+  DeclContext *DC = ND->getDeclContext()->getPrimaryContext();
+  add(DC, ND);
+  DeclContext *ReDC = DC->getRedeclContext()->getPrimaryContext();
+  if (DC != ReDC)
+add(ReDC, ND);
+}
+
+void ASTImporterLookupTable::remove(NamedDecl *ND) {
+  assert(ND);
+  DeclContext *DC = ND->getDeclContext()->getPrimaryContext();
+  remove(DC, ND);
+  DeclContext *ReDC = DC->getRedeclContext()->getPrimaryContext();
+  if (DC != ReDC)
+remove(ReDC, ND);
+}
+
+ASTImporterLookupTable::LookupResult
+ASTImporterLookupTable::lookup(DeclContext *DC, DeclarationName Name) const {
+  auto DCI = LookupTable.find(DC->getPrimaryContext());
+  if (DCI == LookupTable.end())
+return {};
+
+  const auto &FoundNameMap = DCI->second;
+  auto NamesI = FoundNameMap.find(Name);
+  if (NamesI == FoundNameMap.end())
+return {};
+
+  return NamesI->second;
+}
+
+void ASTImporterLookupTable::dump(DeclContext *DC) const {
+  auto DCI = LookupTable.find(DC->getPrimaryContext());
+  if (DCI == LookupTable.end())
+llvm::errs() << "empty\n";
+  const auto &FoundNameMap = DCI->second;
+  for (const auto &Entry : FoundNameMap) {
+DeclarationName Name = Entry.first;
+llvm::errs() << " Name: ";
+Name.dump();
+const DeclList& List = Entry.second;
+for (NamedDecl *ND : List) {
+  ND->dump();
+}
+  }
+}
+
+void ASTImporterLookupTable::dump() const {
+  for (const auto &Entry : LookupTable) {
+DeclContext *DC = Entry.first;
+StringRef Primary = DC->getPrimaryContext() ? " primary" : "";
+llvm::errs() << "== DC:" << cast(DC) << Primary << "\n";
+dump(DC);
+  }
+}
+
+} // namespace clang
Index: cfe/trunk

[PATCH] D53708: [ASTImporter] Add importer specific lookup

2018-12-17 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

> This is a perhaps a naive comment, but why is localUncachedLookup used

instead of noload_lookup ? There is a fixme dating from 2013 stating
that noload_lookup should be used instead.

This is not a naive question. I was wondering myself on the very same thing 
when I started working on the ASTImporter at 2017. 
I think when `noload_lookup` was introduced the author of that function tried 
to replace `localUncachedLookup` in ASTImporter, but that broke some tests. The 
major difference between the two functions is that the latter can find 
declarations even if they are not stored in the `LookupPtr`. This is rather 
unusual from the C/C++ point of view of lookup, and not working in all cases 
(see the introduction of this patch). Ironically, I can't just get rid of 
`localUncachedLookup` because it would break some LLDB test cases.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53708/new/

https://reviews.llvm.org/D53708



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

[PATCH] D51866: [analyzer][UninitializedObjectChecker] New flag to ignore guarded uninitialized fields

2018-12-17 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus added a comment.

Ping.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D51866/new/

https://reviews.llvm.org/D51866



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

2018-12-17 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: include/clang/Basic/Attr.td:2871-2872
   /// distribute: attempt to distribute loop if State == Enable
+  /// pipeline: disable pipelining loop if State == Disable
+  /// pipeline_ii_count: try to pipeline loop for only 'Value' value
 

Missing full stops at the end of the comments. Also, having read "for only 
'Value' value", I'm still not certain what's happening. I think this is a count 
of some kind, so perhaps "Tries to pipeline 'Values' times." but I don't know 
what the verb "pipeline" means in this context.

Are users going to understand what the `ii` means in the user-facing name?



Comment at: include/clang/Basic/AttrDocs.td:2583
+interleaving, and unrolling to be enabled or disabled. Pipelining could be 
disabled.
+Vector width as well as interleave, unrolling count and Initiation interval 
for pipelining
+can be manually specified. See `language extensions

Vector width, interleave count, unrolling count, and the initiation interval 
for pipelining can be explicitly specified.



Comment at: include/clang/Basic/AttrDocs.td:2654
+Specifying ``#pragma clang loop pipeline(disable)`` avoids software pipelining 
optimization.
+Only `disable` state could be specified for ``#pragma clang loop pipeline``:
+

The `disable` state can only be specified for ``#pragma clang loop pipeline``.



Comment at: include/clang/Basic/AttrDocs.td:2663
+
+Specifying the ii count parameter for ``#pragma loop pipeline_ii_count`` 
instructs software
+pipeliner to use only specified initiation interval :

Spell out `ii`

instructs software -> instructs the software



Comment at: include/clang/Basic/AttrDocs.td:2664
+Specifying the ii count parameter for ``#pragma loop pipeline_ii_count`` 
instructs software
+pipeliner to use only specified initiation interval :
+

to use only specified -> to use the specified

Remove the space before the colon as well.

Having read the docs, I would have no idea how to pick a value for the 
initiation interval. I'm still not even certain what it controls or does. Can 
you expand the documentation for that (feel free to link to other sources if 
there are authoritative places we can point the user to)?



Comment at: include/clang/Basic/AttrDocs.td:2676
+`_
+for further details including limitations of the pipeline hints.
+  }];

details including -> details, including



Comment at: include/clang/Basic/DiagnosticParseKinds.td:1178
   "%select{invalid|missing}0 option%select{ %1|}0; expected vectorize, "
-  "vectorize_width, interleave, interleave_count, unroll, unroll_count, or 
distribute">;
+  "vectorize_width, interleave, interleave_count, unroll, unroll_count, 
pipeline, pipeline_ii_count or distribute">;
 

80-col



Comment at: include/clang/Basic/DiagnosticParseKinds.td:1192
+def err_pragma_pipeline_invalid_keyword : Error<
+"invalid argument; expected 'disable'">;
 

Can you roll this into the above diagnostic with another `%select`, or does 
that make it too unreadable?



Comment at: lib/CodeGen/CGLoopInfo.cpp:29
   Attrs.UnrollAndJamCount == 0 &&
+  Attrs.PipelineDisabled == false &&
+  Attrs.PipelineIICount == 0 &&

`!Attrs.PipelineDisabled`



Comment at: lib/CodeGen/CGLoopInfo.cpp:127
 
+  if(Attrs.PipelineDisabled){
+  Metadata *Vals[] = {MDString::get(Ctx, "llvm.loop.pipeline.disable"),

Formatting is incorrect here (and below) -- you should run the patch through 
clang-format.



Comment at: lib/CodeGen/CGLoopInfo.h:70
+
+  /// Value for llvm.loop.pipeline.disable metadata
+  bool PipelineDisabled;

Missing full-stop. Here and elsewhere -- can you look at all the comments and 
make sure they're properly punctuated?



Comment at: lib/CodeGen/CGLoopInfo.h:71
+  /// Value for llvm.loop.pipeline.disable metadata
+  bool PipelineDisabled;
+

One good refactoring (don't feel obligated to do this) would be to use in-class 
initializers here.



Comment at: test/Parser/pragma-pipeline.cpp:24
+/* expected-error {{use of undeclared identifier 'error'}} */ #pragma clang 
loop pipeline_ii_count(error)
+/* expected-error {{expected '('}} */ #pragma clang loop pipeline_ii_count 1 2
+  for (int i = 0; i < Length; i++) {

Can you add a few tests:

`#pragma clang loop pipeline_ii_count(1` to show that we also diagnose the 
missing closing paren
`#pragma clang loop pipeline_ii_count(1.0)` to show that we diagnose a 
non-integer literal count
`#pragma clang loop pipeline enable` to show that we diagnose the missing open

[PATCH] D55741: Implementation Feature Test Macros for P0722R3

2018-12-17 Thread Chris Kennelly via Phabricator via cfe-commits

ckennelly added a comment.

Yes, I need someone to commit for me.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55741/new/

https://reviews.llvm.org/D55741



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

[PATCH] D55719: [OpenMP] parsing and sema support for 'close' map-type-modifier

2018-12-17 Thread Alexey Bataev via Phabricator via cfe-commits

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

LG


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55719/new/

https://reviews.llvm.org/D55719



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

[PATCH] D55224: [clangd] Introduce loading of shards within auto-index

2018-12-17 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

Probably the most important suggestion from my side is trying to clearly 
separate the `enqueue` calls (which actually schedule rebuilds of TUs) using 
clang and the `loadShards` function.
The index loading should be fast, so I assume we won't win much latency by 
scheduling the indexing actions in the middle of the shard-loading.
OTOH, separating those two concerns should make the code way more readable (I 
think, at least).

The concrete proposal is to make `loadShards` actually **only** the shards and 
return a list of TUs that still need to be rebuilt. As the second step, we can 
`enqueue` the rebuild of those TUs.

Another important thing that's worth doing is documenting the correctness 
trade-offs we have in the current model, i.e. when following the include edges 
of a file (say, `foo.h`) that had its digest changed, we can potentially:

1. load stale symbols for some other file (say, `bar.h`) previously included by 
it,
2. never update those symbols to fresh ones if the include edge was dropped, 
i.e. `foo.h` does not include `bar.h` anymore.

The described situation can lead to stale symbols for `bar.h` being kept 
indefinitely in some cases, but has the advantage of reparsing less TUs when 
rebuilding the index. Overall it's definitely fine to have this trade-off for 
the time-being, but would be nice if we could document it.

The rest is mostly NITs and code style comments.

And thanks for the change, it's an impressive piece of work!

Comment at: clangd/SourceCode.h:96
+
+// Handle the symbolic link path case where the current working directory
+// (getCurrentWorkingDirectory) is a symlink./ We always want to the real

We might want to have a guidance on when this function should be used, similar 
to `getRealPath`. Also mention why it's different from the above.

My understanding is that it's only used for canonicalizing the file path for 
storing them in the index and nowhere else.
Should we discourage usages outside the index? Maybe we can give a more obscure 
name for this purpose, e.g. `canonicalizeForIndex` or similar?

Comment at: clangd/SourceCode.h:107
+// "/tmp/build/foo.h"
+std::string makeCanonicalPath(llvm::StringRef AbsolutePath,
+  const SourceManager &SM);

NIT: maybe name it `canonicalizePath`? A bit shorter. But up to you

Comment at: clangd/SourceCode.h:107
+// "/tmp/build/foo.h"
+std::string makeCanonicalPath(llvm::StringRef AbsolutePath,
+  const SourceManager &SM);

ilya-biryukov wrote:
> NIT: maybe name it `canonicalizePath`? A bit shorter. But up to you
Maybe put this function right after `getRealPath`? They both "canonicalize" 
paths, so it makes sense for them to live together.

Comment at: clangd/index/Background.cpp:90
+
+llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand &Cmd) {
+  llvm::SmallString<128> AbsolutePath;

Why not `vfs->makeAbsolute` instead of this function? Maybe worth a comment?

Comment at: clangd/index/Background.cpp:189
+  [this, Storage](tooling::CompileCommand Cmd) {
+Cmd.CommandLine.push_back("-resource-dir=" + ResourceDir);
+const std::string FileName = Cmd.Filename;

Ah, it feels we should move this into a helper function and reuse everywhere.
It's a small detail, but it's so easy to forget about it...

Just complaining, no need to do anything about it...

Comment at: clangd/index/Background.cpp:259
+  // if this thread sees the older version but finishes later. This should
+  // be rare in practice.
+  DigestIt.first->second = Hash;

> "should be rare in practice"
And yet, can we avoid this altogether?

Also, I believe it won't be rare. When processing multiple different TUs, we 
can easily run into the same header multiple times, possibly with the different 
contents.
E.g. imagine the index for the whole of clang is being built and the user is 
editing `Sema.h` at the same time. We'll definitely be running into this case 
over and over again.

Comment at: clangd/index/Background.cpp:309
 if (std::error_code EC =
 SM.getFileManager().getVirtualFileSystem()->makeAbsolute(AbsPath)) 
{
   elog("Warning: could not make absolute file: {0}", EC.message());

Does it make sense for it to be part of `makeCanonicalPath`?
Both call sites call `vfs->getAbsolutePath` before calling the function?

Comment at: clangd/index/Background.cpp:338
 std::lock_guard Lock(DigestsMu);
-if (IndexedFileDigests.lookup(AbsolutePath) == Hash) {
-  vlog("No need to index {0}, already up to date", AbsolutePath);

This looks like an important optimization. Did we move it to some other place? 
Why should we remove it?

Co

[PATCH] D53708: [ASTImporter] Add importer specific lookup

2018-12-17 Thread Bruno Ricci via Phabricator via cfe-commits

riccibruno added a comment.

In D53708#1332868 , @martong wrote:

> > This is a perhaps a naive comment, but why is localUncachedLookup used
> >  instead of noload_lookup ? There is a fixme dating from 2013 stating
> >  that noload_lookup should be used instead.
>
> This is not a naive question. I was wondering myself on the very same thing 
> when I started working on the ASTImporter at 2017. 
>  I think when `noload_lookup` was introduced the author of that function 
> tried to replace `localUncachedLookup` in ASTImporter, but that broke some 
> tests. The major difference between the two functions is that the latter can 
> find declarations even if they are not stored in the `LookupPtr`. This is 
> rather unusual from the C/C++ point of view of lookup, and not working in all 
> cases (see the introduction of this patch). Ironically, I can't just get rid 
> of `localUncachedLookup` because it would break some LLDB test cases.

Ah I think I understand. For my understanding (and please correct me if I am 
wrong here):

`localUncachedLookup` will first try to do a normal lookup, and if that fails 
it will try to look in the declarations in the declaration context.
Now some declarations (including declarations which are not `NamedDecl` and 
declarations which matches `shouldBeHidden`)
will not be added to the `LookupPtr` (via `makeDeclVisibleInContextWithFlags`) 
and so will not be found though the normal lookup mechanism.
You therefore need to use `localUncachedLookup` to find these.

Does this make sense ?

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53708/new/

https://reviews.llvm.org/D53708

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

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

2018-12-17 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

Alexey, simply put, the crux of the problem is this:
The imported decl context is different than what we get by calling the 
`getDeclContext()` on an imported field/friend of the "from" decl context.

With other words:
`import(cast(FromDC)` gives us a different value than what we get from 
`ToD->getDeclContext()` inside the for loop which iterates over the 
fields/friends of `FromDC`.

This seems abnormal, because we would expect the two to be equal. This is why I 
suspect something related to an LLDB specific use case (and what's worse it 
happens only on macOS). In LLDB, I have seen they first do a minimal import 
then they require the definition with `ASTImporter::importDefinition` (but I am 
not sure if this is the real root cause here). I guess this is a scenario we 
don't have any unittest neither lit tests for in Clang. In an ideal world we 
should fix this inequivalence first and then could we apply this patch.

The workaround I provided is to use the  decl context of the first imported 
field and use that as target decl context (and not use the decl context given 
by import(FromDC)).

Hope this helps


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D44100/new/

https://reviews.llvm.org/D44100



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

[PATCH] D53708: [ASTImporter] Add importer specific lookup

2018-12-17 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

In D53708#1332922 , @riccibruno wrote:

> In D53708#1332868 , @martong wrote:
>
> > > This is a perhaps a naive comment, but why is localUncachedLookup used
> > >  instead of noload_lookup ? There is a fixme dating from 2013 stating
> > >  that noload_lookup should be used instead.
> >
> > This is not a naive question. I was wondering myself on the very same thing 
> > when I started working on the ASTImporter at 2017. 
> >  I think when `noload_lookup` was introduced the author of that function 
> > tried to replace `localUncachedLookup` in ASTImporter, but that broke some 
> > tests. The major difference between the two functions is that the latter 
> > can find declarations even if they are not stored in the `LookupPtr`. This 
> > is rather unusual from the C/C++ point of view of lookup, and not working 
> > in all cases (see the introduction of this patch). Ironically, I can't just 
> > get rid of `localUncachedLookup` because it would break some LLDB test 
> > cases.
>
>
> Ah I think I understand. For my understanding (and please correct me if I am 
> wrong here):
>
> `localUncachedLookup` will first try to do a normal lookup, and if that fails 
> it will try to look in the declarations in the declaration context.
>  Now some declarations (including declarations which are not `NamedDecl` and 
> declarations which matches `shouldBeHidden`)
>  will not be added to the `LookupPtr` (via 
> `makeDeclVisibleInContextWithFlags`) and so will not be found though the 
> normal lookup mechanism. You therefore need to use `localUncachedLookup` to 
> find these.
>
> Does this make sense ?


Exactly. And this seems to be an important feature in ASTImporter, because this 
makes it possible that we can chain into the redecl chain a newly imported AST 
node to existing and structurally equivalent nodes. (The existing nodes are 
found by the lookup and then we iterate over the lookup results searching for 
structurally equivalent ones.)


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53708/new/

https://reviews.llvm.org/D53708



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

r349357 - Build ASTImporterTest.cpp with /bigobj on MSVC builds to keep llvm-clang-x86_64-expensive-checks-win buildbot happy

2018-12-17 Thread Simon Pilgrim via cfe-commits

Author: rksimon
Date: Mon Dec 17 07:14:08 2018
New Revision: 349357

URL: http://llvm.org/viewvc/llvm-project?rev=349357&view=rev
Log:
Build ASTImporterTest.cpp with /bigobj on MSVC builds to keep 
llvm-clang-x86_64-expensive-checks-win buildbot happy

Modified:
cfe/trunk/unittests/AST/CMakeLists.txt

Modified: cfe/trunk/unittests/AST/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/CMakeLists.txt?rev=349357&r1=349356&r2=349357&view=diff
==
--- cfe/trunk/unittests/AST/CMakeLists.txt (original)
+++ cfe/trunk/unittests/AST/CMakeLists.txt Mon Dec 17 07:14:08 2018
@@ -2,6 +2,10 @@ set(LLVM_LINK_COMPONENTS
   Support
   )
 
+if (MSVC)
+  set_source_files_properties(ASTImporterTest.cpp PROPERTIES COMPILE_FLAGS 
/bigobj)
+endif()
+
 add_clang_unittest(ASTTests
   ASTContextParentMapTest.cpp
   ASTImporterTest.cpp


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

[PATCH] D53708: [ASTImporter] Add importer specific lookup

2018-12-17 Thread Bruno Ricci via Phabricator via cfe-commits

riccibruno added a comment.

> Exactly. And this seems to be an important feature in ASTImporter, because 
> this makes it possible that we can chain into the redecl chain a newly 
> imported AST node to existing and structurally equivalent > nodes. (The 
> existing nodes are found by the lookup and then we iterate over the lookup 
> results searching for structurally equivalent ones.)

I see. I am unfortunately not familiar at all with the ASTImporter, but thanks 
for clearing this up.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53708/new/

https://reviews.llvm.org/D53708



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

2018-12-17 Thread Alexey Lapshin via Phabricator via cfe-commits

alexey.lapshin marked 2 inline comments as done.
alexey.lapshin added a comment.

Ok, I will address all issues.




Comment at: include/clang/Basic/Attr.td:2871-2872
   /// distribute: attempt to distribute loop if State == Enable
+  /// pipeline: disable pipelining loop if State == Disable
+  /// pipeline_ii_count: try to pipeline loop for only 'Value' value
 

aaron.ballman wrote:
> Missing full stops at the end of the comments. Also, having read "for only 
> 'Value' value", I'm still not certain what's happening. I think this is a 
> count of some kind, so perhaps "Tries to pipeline 'Values' times." but I 
> don't know what the verb "pipeline" means in this context.
> 
> Are users going to understand what the `ii` means in the user-facing name?
As to ii - yes that should be understood by users, because it is important 
property of software pipelining optimization. Software Pipelining optimization 
tries to reorder instructions of original loop(from different iterations) so 
that resulting loop body takes less cycles. It started from some minimal value 
of ii and stopped with some maximal value.  i.e. it tries to built schedule for 
min_ii, then min_ii+1, ... until schedule is found or until max_ii reached.  If 
resulting value of ii already known then instead of searching in range min_ii, 
max_ii - it is possible to create schedule for already known ii. 

probably following spelling would be better : 

pipeline_ii_count: create loop schedule with initiation interval equals 'Value'



Comment at: include/clang/Basic/DiagnosticParseKinds.td:1192
+def err_pragma_pipeline_invalid_keyword : Error<
+"invalid argument; expected 'disable'">;
 

aaron.ballman wrote:
> Can you roll this into the above diagnostic with another `%select`, or does 
> that make it too unreadable?
yes, it makes things too complicated. Though I could do it if necessary.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55710/new/

https://reviews.llvm.org/D55710



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

[PATCH] D55765: Fix use-after-free bug in Tooling.

2018-12-17 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits

ymandel created this revision.
ymandel added a reviewer: alexfh.
Herald added a subscriber: cfe-commits.

`buildASTFromCodeWithArgs()` was creating a memory buffer referencing a
stack-allocated string.  This diff changes the implementation to copy the code
string into the memory buffer so that said buffer owns the memory.


Repository:
  rC Clang

https://reviews.llvm.org/D55765

Files:
  lib/Tooling/Tooling.cpp


Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -603,9 +603,8 @@
   &Action, Files.get(), std::move(PCHContainerOps));
 
   SmallString<1024> CodeStorage;
-  InMemoryFileSystem->addFile(FileNameRef, 0,
-  llvm::MemoryBuffer::getMemBuffer(
-  
Code.toNullTerminatedStringRef(CodeStorage)));
+  InMemoryFileSystem->addFile(
+  FileNameRef, 0, 
llvm::MemoryBuffer::getMemBufferCopy(Code.toStringRef()));
   if (!Invocation.run())
 return nullptr;
 


Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -603,9 +603,8 @@
   &Action, Files.get(), std::move(PCHContainerOps));
 
   SmallString<1024> CodeStorage;
-  InMemoryFileSystem->addFile(FileNameRef, 0,
-  llvm::MemoryBuffer::getMemBuffer(
-  Code.toNullTerminatedStringRef(CodeStorage)));
+  InMemoryFileSystem->addFile(
+  FileNameRef, 0, llvm::MemoryBuffer::getMemBufferCopy(Code.toStringRef()));
   if (!Invocation.run())
 return nullptr;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

2018-12-17 Thread Bevin Hansson via Phabricator via cfe-commits

ebevhan added inline comments.



Comment at: lib/CodeGen/CGCall.cpp:77
+  if (MD)
+RecTy = Context.getAddrSpaceQualType(RecTy, 
MD->getType().getAddressSpace());
   return Context.getPointerType(CanQualType::CreateUnsafe(RecTy));

I'm a bit late to the party, but shouldn't this be 
`MD->getTypeQualifiers().getAddressSpace()`?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54862/new/

https://reviews.llvm.org/D54862



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

[PATCH] D55765: Fix use-after-free bug in Tooling.

2018-12-17 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits

ymandel updated this revision to Diff 178468.
ymandel added a comment.

Change buildAST functions to take a StringRef instead of a Twine.

In practice, code is almost never constructed on the fly using a Twine, so 
StringRef is simpler and avoids needing a copy when constructing the MemBuffer.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55765/new/

https://reviews.llvm.org/D55765

Files:
  lib/Tooling/Tooling.cpp


Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -581,7 +581,7 @@
 }
 
 std::unique_ptr buildASTFromCodeWithArgs(
-const Twine &Code, const std::vector &Args,
+StringRef Code, const std::vector &Args,
 const Twine &FileName, const Twine &ToolName,
 std::shared_ptr PCHContainerOps,
 ArgumentsAdjuster Adjuster) {
@@ -602,10 +602,8 @@
   getSyntaxOnlyToolArgs(ToolName, Adjuster(Args, FileNameRef), 
FileNameRef),
   &Action, Files.get(), std::move(PCHContainerOps));
 
-  SmallString<1024> CodeStorage;
   InMemoryFileSystem->addFile(FileNameRef, 0,
-  llvm::MemoryBuffer::getMemBuffer(
-  
Code.toNullTerminatedStringRef(CodeStorage)));
+  llvm::MemoryBuffer::getMemBufferCopy(Code));
   if (!Invocation.run())
 return nullptr;
 


Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -581,7 +581,7 @@
 }
 
 std::unique_ptr buildASTFromCodeWithArgs(
-const Twine &Code, const std::vector &Args,
+StringRef Code, const std::vector &Args,
 const Twine &FileName, const Twine &ToolName,
 std::shared_ptr PCHContainerOps,
 ArgumentsAdjuster Adjuster) {
@@ -602,10 +602,8 @@
   getSyntaxOnlyToolArgs(ToolName, Adjuster(Args, FileNameRef), FileNameRef),
   &Action, Files.get(), std::move(PCHContainerOps));
 
-  SmallString<1024> CodeStorage;
   InMemoryFileSystem->addFile(FileNameRef, 0,
-  llvm::MemoryBuffer::getMemBuffer(
-  Code.toNullTerminatedStringRef(CodeStorage)));
+  llvm::MemoryBuffer::getMemBufferCopy(Code));
   if (!Invocation.run())
 return nullptr;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r349053 - [CodeComplete] Fill preferred type on binary expressions

2018-12-17 Thread Ilya Biryukov via cfe-commits

Hi Douglas, Reid,

Sorry for the breakage, I'll take a look at the bug.


On Sat, Dec 15, 2018 at 12:18 AM  wrote:

> Hi, I think this test is still broken on Windows when you are building a
> cross compiler as we at Sony do. I have put the details in PR40033. Can
> somebody take a look?
>
>
>
> Douglas Yung
>
>
>
> *From:* cfe-commits  *On Behalf Of *Reid
> Kleckner via cfe-commits
> *Sent:* Thursday, December 13, 2018 13:51
> *To:* Ilya Biryukov 
> *Cc:* cfe-commits 
> *Subject:* Re: r349053 - [CodeComplete] Fill preferred type on binary
> expressions
>
>
>
> r349086 should take care of it, but you may want to tweak it.
>
>
>
> On Thu, Dec 13, 2018 at 1:30 PM Reid Kleckner  wrote:
>
> This new test doesn't pass on Windows. I think it's an LLP64-ness bug
> based on the output:
>
> Note: Google Test filter = PreferredTypeTest.Binar*
>
> [==] Running 1 test from 1 test case.
>
> [--] Global test environment set-up.
>
> [--] 1 test from PreferredTypeTest
>
> [ RUN  ] PreferredTypeTest.BinaryExpr
>
> test.cc:4:45: error: invalid operands to binary expression ('float' and
> 'int')
>
>   x += 10; x -= 10; x *= 10; x /= 10; x %= 10;
>
>   ~ ^  ~~
>
> 1 error generated.
>
> test.cc:4:45: error: invalid operands to binary expression ('float' and
> 'int')
>
>   x += 10; x -= 10; x *= 10; x /= 10; x %= 10;
>
>   ~ ^  ~~
>
> 1 error generated.
>
> test.cc:4:45: error: invalid operands to binary expression ('float' and
> 'int')
>
>   x += 10; x -= 10; x *= 10; x /= 10; x %= 10;
>
>   ~ ^  ~~
>
> 1 error generated.
>
> test.cc:4:45: error: invalid operands to binary expression ('float' and
> 'int')
>
>   x += 10; x -= 10; x *= 10; x /= 10; x %= 10;
>
>   ~ ^  ~~
>
> 1 error generated.
>
> test.cc:4:45: error: invalid operands to binary expression ('float' and
> 'int')
>
>   x += 10; x -= 10; x *= 10; x /= 10; x %= 10;
>
>   ~ ^  ~~
>
> 1 error generated.
>
> C:\src\llvm-project\clang\unittests\Sema\CodeCompleteTest.cpp(216): error:
> Value of: collectPreferredTypes(Code)
>
> Expected: only contains elements that is equal to "long"
>
>   Actual: { "long long", "long long", "long long" }, whose element #0
> doesn't match
>
>
>
> On Thu, Dec 13, 2018 at 8:09 AM Ilya Biryukov via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: ibiryukov
> Date: Thu Dec 13 08:06:11 2018
> New Revision: 349053
>
> URL: http://llvm.org/viewvc/llvm-project?rev=349053&view=rev
> Log:
> [CodeComplete] Fill preferred type on binary expressions
>
> Reviewers: kadircet
>
> Reviewed By: kadircet
>
> Subscribers: arphaman, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D55648
>
> Modified:
> cfe/trunk/include/clang/Sema/Sema.h
> cfe/trunk/lib/Parse/ParseExpr.cpp
> cfe/trunk/lib/Sema/SemaCodeComplete.cpp
> cfe/trunk/test/Index/complete-exprs.c
> cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=349053&r1=349052&r2=349053&view=diff
>
> ==
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Thu Dec 13 08:06:11 2018
> @@ -10350,7 +10350,7 @@ public:
>void CodeCompleteInitializer(Scope *S, Decl *D);
>void CodeCompleteReturn(Scope *S);
>void CodeCompleteAfterIf(Scope *S);
> -  void CodeCompleteAssignmentRHS(Scope *S, Expr *LHS);
> +  void CodeCompleteBinaryRHS(Scope *S, Expr *LHS, tok::TokenKind Op);
>
>void CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
> bool EnteringContext, QualType BaseType);
>
> Modified: cfe/trunk/lib/Parse/ParseExpr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExpr.cpp?rev=349053&r1=349052&r2=349053&view=diff
>
> ==
> --- cfe/trunk/lib/Parse/ParseExpr.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseExpr.cpp Thu Dec 13 08:06:11 2018
> @@ -393,10 +393,11 @@ Parser::ParseRHSOfBinaryExpression(ExprR
>}
>  }
>
> -// Code completion for the right-hand side of an assignment expression
> -// goes through a special hook that takes the left-hand side into
> account.
> -if (Tok.is(tok::code_completion) && NextTokPrec == prec::Assignment) {
> -  Actions.CodeCompleteAssignmentRHS(getCurScope(), LHS.get());
> +// Code completion for the right-hand side of a binary expression goes
> +// through a special hook that takes the left-hand side into account.
> +if (Tok.is(tok::code_completion)) {
> +  Actions.CodeCompleteBinaryRHS(getCurScope(), LHS.get(),
> +OpToken.getKind());
>

[PATCH] D55765: Fix use-after-free bug in Tooling.

2018-12-17 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits

ymandel updated this revision to Diff 178470.
ymandel added a comment.

Change header correspondingly.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55765/new/

https://reviews.llvm.org/D55765

Files:
  include/clang/Tooling/Tooling.h
  lib/Tooling/Tooling.cpp


Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -581,7 +581,7 @@
 }
 
 std::unique_ptr buildASTFromCodeWithArgs(
-const Twine &Code, const std::vector &Args,
+StringRef Code, const std::vector &Args,
 const Twine &FileName, const Twine &ToolName,
 std::shared_ptr PCHContainerOps,
 ArgumentsAdjuster Adjuster) {
@@ -602,10 +602,8 @@
   getSyntaxOnlyToolArgs(ToolName, Adjuster(Args, FileNameRef), 
FileNameRef),
   &Action, Files.get(), std::move(PCHContainerOps));
 
-  SmallString<1024> CodeStorage;
   InMemoryFileSystem->addFile(FileNameRef, 0,
-  llvm::MemoryBuffer::getMemBuffer(
-  
Code.toNullTerminatedStringRef(CodeStorage)));
+  llvm::MemoryBuffer::getMemBufferCopy(Code));
   if (!Invocation.run())
 return nullptr;
 
Index: include/clang/Tooling/Tooling.h
===
--- include/clang/Tooling/Tooling.h
+++ include/clang/Tooling/Tooling.h
@@ -205,7 +205,7 @@
 ///
 /// \return The resulting AST or null if an error occurred.
 std::unique_ptr
-buildASTFromCode(const Twine &Code, const Twine &FileName = "input.cc",
+buildASTFromCode(StringRef Code, const Twine &FileName = "input.cc",
  std::shared_ptr PCHContainerOps =
  std::make_shared());
 
@@ -223,7 +223,7 @@
 ///
 /// \return The resulting AST or null if an error occurred.
 std::unique_ptr buildASTFromCodeWithArgs(
-const Twine &Code, const std::vector &Args,
+StringRef Code, const std::vector &Args,
 const Twine &FileName = "input.cc", const Twine &ToolName = "clang-tool",
 std::shared_ptr PCHContainerOps =
   std::make_shared(),


Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -581,7 +581,7 @@
 }
 
 std::unique_ptr buildASTFromCodeWithArgs(
-const Twine &Code, const std::vector &Args,
+StringRef Code, const std::vector &Args,
 const Twine &FileName, const Twine &ToolName,
 std::shared_ptr PCHContainerOps,
 ArgumentsAdjuster Adjuster) {
@@ -602,10 +602,8 @@
   getSyntaxOnlyToolArgs(ToolName, Adjuster(Args, FileNameRef), FileNameRef),
   &Action, Files.get(), std::move(PCHContainerOps));
 
-  SmallString<1024> CodeStorage;
   InMemoryFileSystem->addFile(FileNameRef, 0,
-  llvm::MemoryBuffer::getMemBuffer(
-  Code.toNullTerminatedStringRef(CodeStorage)));
+  llvm::MemoryBuffer::getMemBufferCopy(Code));
   if (!Invocation.run())
 return nullptr;
 
Index: include/clang/Tooling/Tooling.h
===
--- include/clang/Tooling/Tooling.h
+++ include/clang/Tooling/Tooling.h
@@ -205,7 +205,7 @@
 ///
 /// \return The resulting AST or null if an error occurred.
 std::unique_ptr
-buildASTFromCode(const Twine &Code, const Twine &FileName = "input.cc",
+buildASTFromCode(StringRef Code, const Twine &FileName = "input.cc",
  std::shared_ptr PCHContainerOps =
  std::make_shared());
 
@@ -223,7 +223,7 @@
 ///
 /// \return The resulting AST or null if an error occurred.
 std::unique_ptr buildASTFromCodeWithArgs(
-const Twine &Code, const std::vector &Args,
+StringRef Code, const std::vector &Args,
 const Twine &FileName = "input.cc", const Twine &ToolName = "clang-tool",
 std::shared_ptr PCHContainerOps =
   std::make_shared(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54438: [analyzer] Reimplement dependencies between checkers

2018-12-17 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus added a comment.

> I still expect some skeletons to fall out of the closet

This patch doesn't handle -analyzer-disable-checker, which leads to assertation 
failures in later pathes. Since the way which checker should/shouldnt be 
enabled is implemented rather poorly, I'll probably try to find a long-term 
solution.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54438/new/

https://reviews.llvm.org/D54438



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

[PATCH] D55770: [clangd] BackgroundIndex rebuilds symbol index periodically.

2018-12-17 Thread Eric Liu via Phabricator via cfe-commits

ioeric created this revision.
ioeric added reviewers: ilya-biryukov, kadircet.
Herald added subscribers: cfe-commits, jfb, arphaman, jkorous, MaskRay.

Currently, background index rebuilds symbol index on every indexed file,
which can be inefficient. This patch makes it only rebuild symbol index 
periodically.
As the rebuild no longer happens too often, we could also build more efficient
dex index.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D55770

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/index/Background.cpp
  clangd/index/Background.h
  clangd/tool/ClangdMain.cpp
  test/clangd/background-index.test
  unittests/clangd/BackgroundIndexTests.cpp

Index: unittests/clangd/BackgroundIndexTests.cpp
===
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -2,11 +2,14 @@
 #include "TestFS.h"
 #include "index/Background.h"
 #include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/Threading.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 
 using testing::_;
 using testing::AllOf;
+using testing::ElementsAre;
 using testing::Not;
 using testing::UnorderedElementsAre;
 
@@ -240,5 +243,39 @@
   EmptyIncludeNode());
 }
 
+TEST_F(BackgroundIndexTest, PeriodicalIndex) {
+  MockFSProvider FS;
+  llvm::StringMap Storage;
+  size_t CacheHits = 0;
+  MemoryShardStorage MSS(Storage, CacheHits);
+  OverlayCDB CDB(/*Base=*/nullptr);
+  BackgroundIndex Idx(
+  Context::empty(), "", FS, CDB, [&](llvm::StringRef) { return &MSS; },
+  /*BuildIndexPeriodMs=*/10);
+
+  FS.Files[testPath("root/A.cc")] = "#include \"A.h\"";
+
+  tooling::CompileCommand Cmd;
+  FS.Files[testPath("root/A.h")] = "class X {};";
+  Cmd.Filename = testPath("root/A.cc");
+  Cmd.CommandLine = {"clang++", Cmd.Filename};
+  CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
+
+  ASSERT_TRUE(Idx.blockUntilIdleForTest());
+  EXPECT_THAT(runFuzzyFind(Idx, ""), ElementsAre());
+  std::this_thread::sleep_for(std::chrono::milliseconds(15));
+  EXPECT_THAT(runFuzzyFind(Idx, ""), ElementsAre(Named("X")));
+
+  FS.Files[testPath("root/A.h")] = "class Y {};";
+  FS.Files[testPath("root/A.cc")] += " "; // Force reindex the file.
+  Cmd.CommandLine = {"clang++", "-DA=1", testPath("root/A.cc")};
+  CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
+
+  ASSERT_TRUE(Idx.blockUntilIdleForTest());
+  EXPECT_THAT(runFuzzyFind(Idx, ""), ElementsAre(Named("X")));
+  std::this_thread::sleep_for(std::chrono::milliseconds(15));
+  EXPECT_THAT(runFuzzyFind(Idx, ""), ElementsAre(Named("Y")));
+}
+
 } // namespace clangd
 } // namespace clang
Index: test/clangd/background-index.test
===
--- test/clangd/background-index.test
+++ test/clangd/background-index.test
@@ -10,7 +10,7 @@
 # We're editing bar.cpp, which includes foo.h.
 # foo() is declared in foo.h and defined in foo.cpp.
 # The background index should allow us to go-to-definition on foo().
-# RUN: clangd -background-index -lit-test < %t/definition.jsonrpc | FileCheck %t/definition.jsonrpc
+# RUN: clangd -background-index -background-index-rebuild-period=0 -lit-test < %t/definition.jsonrpc | FileCheck %t/definition.jsonrpc
 
 # Test that the index is writing files in the expected location.
 # RUN: ls %t/.clangd-index/foo.cpp.*.idx
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -170,6 +170,14 @@
  "Experimental"),
 cl::init(false), cl::Hidden);
 
+static cl::opt BackgroundIndexRebuildPeriod(
+"background-index-rebuild-period",
+cl::desc(
+"If set to non-zero, the background index rebuilds the symbol index "
+"periodically every X milliseconds; otherwise, the "
+"symbol index will be updated for each indexed file."),
+cl::init(5000), cl::Hidden);
+
 enum CompileArgsFrom { LSPCompileArgs, FilesystemCompileArgs };
 static cl::opt CompileArgsFrom(
 "compile_args_from", cl::desc("The source of compile commands"),
@@ -352,6 +360,7 @@
   Opts.BuildDynamicSymbolIndex = EnableIndex;
   Opts.HeavyweightDynamicSymbolIndex = UseDex;
   Opts.BackgroundIndex = EnableBackgroundIndex;
+  Opts.BackgroundIndexRebuildPeriodMs = BackgroundIndexRebuildPeriod;
   std::unique_ptr StaticIdx;
   std::future AsyncIndexLoad; // Block exit while loading the index.
   if (EnableIndex && !IndexFile.empty()) {
Index: clangd/index/Background.h
===
--- clangd/index/Background.h
+++ clangd/index/Background.h
@@ -21,8 +21,10 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/Support/SHA1.h"
 #include "llvm/Support/Threading.h"
+#include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -63,11 +65,15 @@
 // FIXME: it should watch for changes to files on

[PATCH] D55771: [AST] Store the callee and argument expressions of CallExpr in a trailing array.

2018-12-17 Thread Bruno Ricci via Phabricator via cfe-commits

riccibruno created this revision.
riccibruno added a reviewer: rjmccall.
riccibruno added a project: clang.
Herald added subscribers: cfe-commits, javed.absar.
Herald added a reviewer: shafik.

Since `CallExpr::setNumArgs` has been removed, it is now possible to store the 
callee expression and the argument expressions of `CallExpr` in a trailing 
array. This saves one pointer per `CallExpr`, `CXXOperatorCallExpr`,
`CXXMemberCallExpr`, `CUDAKernelCallExpr` and `UserDefinedLiteral`.

Since `CallExpr` is used as a base of the above classes we cannot use 
`llvm::TrailingObjects`. Instead we store
the offset in bytes from the `this` pointer to the start of the trailing 
objects and manually do the casts + arithmetic.

Some notes:

1. I did not try to fit the number of arguments in the bit-fields of `Stmt`. 
This leaves some space for future additions and avoid the discussion about 
whether x bits are sufficient to hold the number of arguments.
2. It would be perfectly possible to recompute the offset to the trailing 
objects before accessing the trailing objects. However the trailing objects are 
frequently accessed and benchmarks show that it is slightly faster to just load 
the offset from the bit-fields. Additionally, because of 1), we have plenty of 
space in the bit-fields of `Stmt`.


Repository:
  rC Clang

https://reviews.llvm.org/D55771

Files:
  include/clang/AST/Expr.h
  include/clang/AST/ExprCXX.h
  include/clang/AST/Stmt.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprCXX.cpp
  lib/Analysis/BodyFarm.cpp
  lib/CodeGen/CGObjC.cpp
  lib/Frontend/Rewrite/RewriteModernObjC.cpp
  lib/Frontend/Rewrite/RewriteObjC.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp

Index: lib/Serialization/ASTReaderStmt.cpp
===
--- lib/Serialization/ASTReaderStmt.cpp
+++ lib/Serialization/ASTReaderStmt.cpp
@@ -2481,9 +2481,8 @@
   break;
 
 case EXPR_CALL:
-  S = new (Context) CallExpr(
-  Context, /* NumArgs=*/Record[ASTStmtReader::NumExprFields],
-  Empty);
+  S = CallExpr::CreateEmpty(
+  Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields], Empty);
   break;
 
 case EXPR_MEMBER: {
@@ -3073,15 +3072,13 @@
 }
 
 case EXPR_CXX_OPERATOR_CALL:
-  S = new (Context) CXXOperatorCallExpr(
-  Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields],
-  Empty);
+  S = CXXOperatorCallExpr::CreateEmpty(
+  Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields], Empty);
   break;
 
 case EXPR_CXX_MEMBER_CALL:
-  S = new (Context) CXXMemberCallExpr(
-  Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields],
-  Empty);
+  S = CXXMemberCallExpr::CreateEmpty(
+  Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields], Empty);
   break;
 
 case EXPR_CXX_CONSTRUCT:
@@ -3121,7 +3118,7 @@
   break;
 
 case EXPR_USER_DEFINED_LITERAL:
-  S = new (Context) UserDefinedLiteral(
+  S = UserDefinedLiteral::CreateEmpty(
   Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields], Empty);
   break;
 
@@ -3292,7 +3289,7 @@
   break;
 
 case EXPR_CUDA_KERNEL_CALL:
-  S = new (Context) CUDAKernelCallExpr(
+  S = CUDAKernelCallExpr::CreateEmpty(
   Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields], Empty);
   break;
 
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -3136,7 +3136,7 @@
CK_BuiltinFnToFnPtr).get();
 
 // Build the CallExpr
-ExprResult TheCall = new (SemaRef.Context) CallExpr(
+ExprResult TheCall = CallExpr::Create(
 SemaRef.Context, Callee, SubExprs, Builtin->getCallResultType(),
 Expr::getValueKindForType(Builtin->getReturnType()), RParenLoc);
 
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -7004,13 +7004,17 @@
   // there are 0 arguments (i.e., nothing is allocated using ASTContext's
   // allocator).
   QualType CallResultType = ConversionType.getNonLValueExprType(Context);
-  CallExpr Call(Context, &ConversionFn, None, CallResultType, VK,
-From->getBeginLoc());
+
+  llvm::AlignedCharArray
+  Buffer;
+  CallExpr *TheTemporaryCall = CallExpr::CreateTemporary(
+  Buffer.buffer, &ConversionFn, CallResultType, VK, From->getBeginLoc());
+
   ImplicitConversionSequence ICS =
-TryCopyInitialization(*this, &Call, ToType,
-  /*SuppressUserConversions=*/true,
-  /*InOverloadResolution=*/false,
-  /*AllowObjCWritebackConversion=*/false);
+  TryC

[PATCH] D54676: [AST] Pack CallExpr

2018-12-17 Thread Bruno Ricci via Phabricator via cfe-commits

riccibruno abandoned this revision.
riccibruno added a comment.

Abandoned in favor of D55771 


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54676/new/

https://reviews.llvm.org/D54676



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

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

2018-12-17 Thread Adrian Prantl via Phabricator via cfe-commits

aprantl added inline comments.



Comment at: lib/AST/ASTImporter.cpp:1672
   // FIXME: Handle this case somehow better.
-  consumeError(ImportedOrErr.takeError());
+  else
+consumeError(ImportedOrErr.takeError());

this else is redundant.



Comment at: lib/AST/ASTImporter.cpp:1696
+if (isa(D) || isa(D)) {
+  Decl *ToD = Importer.GetAlreadyImportedOrNull(D);
+  assert(ToRD == ToD->getDeclContext() && ToRD->containsDecl(ToD));

Can you add a comment explaining why this function named ...OrNull never 
returns a nullptr?



Comment at: lib/AST/ASTImporter.cpp:1703
+  if (!ToRD->hasExternalLexicalStorage())
+assert(ToRD->field_empty());
+

` assert(ToRD->hasExternalLexicalStorage() || ToRD->field_empty());`



Comment at: lib/AST/ASTImporter.cpp:1708
+  Decl *ToD = Importer.GetAlreadyImportedOrNull(D);
+  assert(ToD);
+  assert(ToRD == ToD->getDeclContext());

This is redundant with the next assertion.



Comment at: lib/AST/ASTImporter.cpp:1712
+  if (!ToRD->hasExternalLexicalStorage())
+assert(!ToRD->containsDecl(ToD));
+

ditto


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D44100/new/

https://reviews.llvm.org/D44100



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

r349362 - [CodeComplete] Fix test failure on different host and target configs

2018-12-17 Thread Ilya Biryukov via cfe-commits

Author: ibiryukov
Date: Mon Dec 17 08:37:52 2018
New Revision: 349362

URL: http://llvm.org/viewvc/llvm-project?rev=349362&view=rev
Log:
[CodeComplete] Fix test failure on different host and target configs

This should fix PR40033.

Modified:
cfe/trunk/unittests/Sema/CodeCompleteTest.cpp

Modified: cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Sema/CodeCompleteTest.cpp?rev=349362&r1=349361&r2=349362&view=diff
==
--- cfe/trunk/unittests/Sema/CodeCompleteTest.cpp (original)
+++ cfe/trunk/unittests/Sema/CodeCompleteTest.cpp Mon Dec 17 08:37:52 2018
@@ -31,6 +31,9 @@ const char TestCCName[] = "test.cc";
 struct CompletionContext {
   std::vector VisitedNamespaces;
   std::string PreferredType;
+  // String representation of std::ptrdiff_t on a given platform. This is a 
hack
+  // to properly account for different configurations of clang.
+  std::string PtrDiffType;
 };
 
 class VisitedContextFinder : public CodeCompleteConsumer {
@@ -47,6 +50,8 @@ public:
 ResultCtx.VisitedNamespaces =
 getVisitedNamespace(Context.getVisitedContexts());
 ResultCtx.PreferredType = Context.getPreferredType().getAsString();
+ResultCtx.PtrDiffType =
+S.getASTContext().getPointerDiffType().getAsString();
   }
 
   CodeCompletionAllocator &getAllocator() override {
@@ -133,11 +138,19 @@ CompletionContext runCodeCompleteOnCode(
   return runCompletion(P.Code, P.Points.front());
 }
 
-std::vector collectPreferredTypes(StringRef AnnotatedCode) {
+std::vector
+collectPreferredTypes(StringRef AnnotatedCode,
+  std::string *PtrDiffType = nullptr) {
   ParsedAnnotations P = parseAnnotations(AnnotatedCode);
   std::vector Types;
-  for (size_t Point : P.Points)
-Types.push_back(runCompletion(P.Code, Point).PreferredType);
+  for (size_t Point : P.Points) {
+auto Results = runCompletion(P.Code, Point);
+if (PtrDiffType) {
+  assert(PtrDiffType->empty() || *PtrDiffType == Results.PtrDiffType);
+  *PtrDiffType = Results.PtrDiffType;
+}
+Types.push_back(Results.PreferredType);
+  }
   return Types;
 }
 
@@ -213,9 +226,11 @@ TEST(PreferredTypeTest, BinaryExpr) {
   ptr += ^10;
   ptr -= ^10;
 })cpp";
-  // Expect the normalized ptrdiff_t type, which is typically long or long 
long.
-  const char *PtrDiff = sizeof(void *) == sizeof(long) ? "long" : "long long";
-  EXPECT_THAT(collectPreferredTypes(Code), Each(PtrDiff));
+  {
+std::string PtrDiff;
+auto Types = collectPreferredTypes(Code, &PtrDiff);
+EXPECT_THAT(Types, Each(PtrDiff));
+  }
 
   // Comparison operators.
   Code = R"cpp(


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

[PATCH] D54918: [analyzer] Apply clang-format to GenericTaintChecker.cpp

2018-12-17 Thread Borsik Gábor via Phabricator via cfe-commits

boga95 added a comment.

I don't have commit access. Can you please do it on my behalf?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54918/new/

https://reviews.llvm.org/D54918



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

[PATCH] D53928: Enable builtins necessary for SLEEF [AArch64] vectorized trigonometry libm functions

2018-12-17 Thread Stefan Teleman via Phabricator via cfe-commits

steleman updated this revision to Diff 178477.
steleman added a comment.
Herald added a subscriber: arphaman.

Updated version of this changeset/patch, as per Renato's latest comments from 
D53927  (https://reviews.llvm.org/D53927).

New benchmark comparing SLEEF performance when built with GCC 8.2 vs. clang ToT 
from 12/15/2018:

https://docs.google.com/spreadsheets/d/1QgdJjgCTFA9I2_5MvJU0uR4JT4knoW5xu-ujxszRbd0/edit?usp=sharing


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53928/new/

https://reviews.llvm.org/D53928

Files:
  include/clang/Basic/Builtins.def
  include/clang/Basic/CodeGenOptions.h
  include/clang/Driver/Options.td
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Index/CommentToXML.cpp

Index: include/clang/Basic/CodeGenOptions.h
===
--- include/clang/Basic/CodeGenOptions.h
+++ include/clang/Basic/CodeGenOptions.h
@@ -54,7 +54,9 @@
   enum VectorLibrary {
 NoLibrary,  // Don't use any vector library.
 Accelerate, // Use the Accelerate framework.
-SVML// Intel short vector math library.
+SVML,   // Intel short vector math library.
+// SLEEF is experimental for now.
+SLEEF   // SLEEF - SIMD Library for Evaluating Elementary Functions.
   };
 
 
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1404,7 +1404,7 @@
   Group, Flags<[CC1Option]>,
   HelpText<"Disables an experimental new pass manager in LLVM.">;
 def fveclib : Joined<["-"], "fveclib=">, Group, Flags<[CC1Option]>,
-HelpText<"Use the given vector functions library">, Values<"Accelerate,SVML,none">;
+HelpText<"Use the given vector functions library">, Values<"Accelerate,SVML,SLEEF (experimental),none">;
 def fno_lax_vector_conversions : Flag<["-"], "fno-lax-vector-conversions">, Group,
   HelpText<"Disallow implicit conversions between vectors with a different number of elements or different element types">, Flags<[CC1Option]>;
 def fno_merge_all_constants : Flag<["-"], "fno-merge-all-constants">, Group,
Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -196,6 +196,9 @@
 BUILTIN(__builtin_exp2 , "dd"  , "Fne")
 BUILTIN(__builtin_exp2f, "ff"  , "Fne")
 BUILTIN(__builtin_exp2l, "LdLd", "Fne")
+BUILTIN(__builtin_exp10 , "dd"  , "Fne")
+BUILTIN(__builtin_exp10f, "ff"  , "Fne")
+BUILTIN(__builtin_exp10l, "LdLd", "Fne")
 BUILTIN(__builtin_expm1 , "dd", "Fne")
 BUILTIN(__builtin_expm1f, "ff", "Fne")
 BUILTIN(__builtin_expm1l, "LdLd", "Fne")
@@ -1137,6 +1140,10 @@
 LIBBUILTIN(exp2f, "ff", "fne", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(exp2l, "LdLd", "fne", "math.h", ALL_LANGUAGES)
 
+LIBBUILTIN(exp10, "dd", "fne", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(exp10f, "ff", "fne", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(exp10l, "LdLd", "fne", "math.h", ALL_LANGUAGES)
+
 LIBBUILTIN(expm1, "dd", "fne", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(expm1f, "ff", "fne", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(expm1l, "LdLd", "fne", "math.h", ALL_LANGUAGES)
@@ -1374,10 +1381,6 @@
 LIBBUILTIN(__tanpi, "dd", "fne", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(__tanpif, "ff", "fne", "math.h", ALL_LANGUAGES)
 
-// Similarly, __exp10 is OS X only
-LIBBUILTIN(__exp10, "dd", "fne", "math.h", ALL_LANGUAGES)
-LIBBUILTIN(__exp10f, "ff", "fne", "math.h", ALL_LANGUAGES)
-
 // Blocks runtime Builtin math library functions
 LIBBUILTIN(_Block_object_assign, "vv*vC*iC", "f", "Blocks.h", ALL_LANGUAGES)
 LIBBUILTIN(_Block_object_dispose, "vvC*iC", "f", "Blocks.h", ALL_LANGUAGES)
Index: lib/Index/CommentToXML.cpp
===
--- lib/Index/CommentToXML.cpp
+++ lib/Index/CommentToXML.cpp
@@ -227,27 +227,27 @@
   { }
 
   // Inline content.
-  void visitTextComment(const TextComment *C);
-  void visitInlineCommandComment(const InlineCommandComment *C);
-  void visitHTMLStartTagComment(const HTMLStartTagComment *C);
-  void visitHTMLEndTagComment(const HTMLEndTagComment *C);
+  void VisitTextComment(const TextComment *C);
+  void VisitInlineCommandComment(const InlineCommandComment *C);
+  void VisitHTMLStartTagComment(const HTMLStartTagComment *C);
+  void VisitHTMLEndTagComment(const HTMLEndTagComment *C);
 
   // Block content.
-  void visitParagraphComment(const ParagraphComment *C);
-  void visitBlockCommandComment(const BlockCommandComment *C);
-  void visitParamCommandComment(const ParamCommandComment *C);
-  void visitTParamCommandComment(const TParamCommandComment *C);
-  void visitVerbatimBlockComment(const VerbatimBlockComment *C);
-  void visitVerbatimBlockLineComment(const VerbatimBlockLineComment *C);
-  void visitVerbatimLineComment

[PATCH] D54834: [analyzer][MallocChecker] Improve warning messages on double-delete errors

2018-12-17 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added a comment.

In D54834#1332679 , @Szelethus wrote:

> Interesting, I've been watching the bots closely, but got no mail after a 
> while. I'm not sure what the cause is, so I'll revert one-by-one.

One of the common reasons for that is that the buildbot was already failing 
when you committed your stuff. Which is why for many buildbots there are 
special people who look at them and tell people to fix their stuff manually.

This time it wasn't the case though: build 2661 was green. So dunno. I 
definitely did receive a mail from this buildbot when it failed on my patch.

But generally it's fine if you don't notice that you're breaking a buildbot or 
two. It's not something to worry about. Sooner or later, they will come after 
you :)

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54834/new/

https://reviews.llvm.org/D54834

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

[PATCH] D55712: [Driver][PS4] Do not implicitly link against asan or ubsan if -nostdlib or -nodefaultlibs on PS4.

2018-12-17 Thread Paul Robinson via Phabricator via cfe-commits

probinson added a comment.

LGTM but I will defer to @filcab as he is more familiar with the area.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55712/new/

https://reviews.llvm.org/D55712



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

[PATCH] D55437: [Index] Index declarations in lambda expression.

2018-12-17 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

Update after investigating with @hokein offline: the `RecursiveASTVisitor` has 
custom code to visit lambda parameters and return type, but it fallback to 
visiting typelocs when both parameters and a return type are specified. 
Unfortunately this fallback does not work when `shouldWalkTypeLocs()` is set to 
false, which is the case for our visitor out here.
It seems reasonable to always visit parameters and return type, rather than 
relying on traversing the full type-loc of the lamda's function type.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55437/new/

https://reviews.llvm.org/D55437



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

Re: r349063 - [CodeComplete] Adhere to LLVM naming style in CodeCompletionTest. NFC

2018-12-17 Thread Ilya Biryukov via cfe-commits

Hi Yvan, sorry for the inconvenience.

I believe this is the same as https://llvm.org/PR40033, should be
fixed by r349362
(the build hasn't finished yet).
I'll double check it's fixed tomorrow and make sure to take another look if
not.

On Mon, Dec 17, 2018 at 10:46 AM Yvan Roux  wrote:

> Hi Ilya,
>
> I'm not sure which one of the commits in that series is to blame, but
> ARM bots are broken due to a failure in CodeCompleteTest.cpp, most
> recent logs are available here:
>
>
> http://lab.llvm.org:8011/builders/clang-cmake-armv7-quick/builds/6090/steps/ninja%20check%201/logs/FAIL%3A%20Clang-Unit%3A%3APreferredTypeTest.BinaryExpr
>
> Cheers,
> Yvan
>
> On Thu, 13 Dec 2018 at 18:35, Ilya Biryukov via cfe-commits
>  wrote:
> >
> > Author: ibiryukov
> > Date: Thu Dec 13 09:32:38 2018
> > New Revision: 349063
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=349063&view=rev
> > Log:
> > [CodeComplete] Adhere to LLVM naming style in CodeCompletionTest. NFC
> >
> > Also reuses the same var for multiple to reduce the chance of
> > accidentally referecing the previous test.
> >
> > Modified:
> > cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
> >
> > Modified: cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Sema/CodeCompleteTest.cpp?rev=349063&r1=349062&r2=349063&view=diff
> >
> ==
> > --- cfe/trunk/unittests/Sema/CodeCompleteTest.cpp (original)
> > +++ cfe/trunk/unittests/Sema/CodeCompleteTest.cpp Thu Dec 13 09:32:38
> 2018
> > @@ -183,79 +183,80 @@ TEST(SemaCodeCompleteTest, VisitedNSWith
> >
> >  TEST(PreferredTypeTest, BinaryExpr) {
> >// Check various operations for arithmetic types.
> > -  StringRef code1 = R"cpp(
> > +  StringRef Code = R"cpp(
> >  void test(int x) {
> >x = ^10;
> >x += ^10; x -= ^10; x *= ^10; x /= ^10; x %= ^10;
> >x + ^10; x - ^10; x * ^10; x / ^10; x % ^10;
> >  })cpp";
> > -  EXPECT_THAT(collectPreferredTypes(code1), Each("int"));
> > -  StringRef code2 = R"cpp(
> > +  EXPECT_THAT(collectPreferredTypes(Code), Each("int"));
> > +
> > +  Code = R"cpp(
> >  void test(float x) {
> >x = ^10;
> >x += ^10; x -= ^10; x *= ^10; x /= ^10; x %= ^10;
> >x + ^10; x - ^10; x * ^10; x / ^10; x % ^10;
> >  })cpp";
> > -  EXPECT_THAT(collectPreferredTypes(code2), Each("float"));
> > +  EXPECT_THAT(collectPreferredTypes(Code), Each("float"));
> >
> >// Pointer types.
> > -  StringRef code3 = R"cpp(
> > +  Code = R"cpp(
> >  void test(int *ptr) {
> >ptr - ^ptr;
> >ptr = ^ptr;
> >  })cpp";
> > -  EXPECT_THAT(collectPreferredTypes(code3), Each("int *"));
> > +  EXPECT_THAT(collectPreferredTypes(Code), Each("int *"));
> >
> > -  StringRef code4 = R"cpp(
> > +  Code = R"cpp(
> >  void test(int *ptr) {
> >ptr + ^10;
> >ptr += ^10;
> >ptr -= ^10;
> >  })cpp";
> > -  EXPECT_THAT(collectPreferredTypes(code4), Each("long")); // long is
> normalized 'ptrdiff_t'.
> > +  EXPECT_THAT(collectPreferredTypes(Code), Each("long")); // long is
> normalized 'ptrdiff_t'.
> >
> >// Comparison operators.
> > -  StringRef code5 = R"cpp(
> > +  Code = R"cpp(
> >  void test(int i) {
> >i <= ^1; i < ^1; i >= ^1; i > ^1; i == ^1; i != ^1;
> >  }
> >)cpp";
> > -  EXPECT_THAT(collectPreferredTypes(code5), Each("int"));
> > +  EXPECT_THAT(collectPreferredTypes(Code), Each("int"));
> >
> > -  StringRef code6 = R"cpp(
> > +  Code = R"cpp(
> >  void test(int *ptr) {
> >ptr <= ^ptr; ptr < ^ptr; ptr >= ^ptr; ptr > ^ptr;
> >ptr == ^ptr; ptr != ^ptr;
> >  }
> >)cpp";
> > -  EXPECT_THAT(collectPreferredTypes(code6), Each("int *"));
> > +  EXPECT_THAT(collectPreferredTypes(Code), Each("int *"));
> >
> >// Relational operations.
> > -  StringRef code7 = R"cpp(
> > +  Code = R"cpp(
> >  void test(int i, int *ptr) {
> >i && ^1; i || ^1;
> >ptr && ^1; ptr || ^1;
> >  }
> >)cpp";
> > -  EXPECT_THAT(collectPreferredTypes(code7), Each("_Bool"));
> > +  EXPECT_THAT(collectPreferredTypes(Code), Each("_Bool"));
> >
> >// Bitwise operations.
> > -  StringRef code8 = R"cpp(
> > +  Code = R"cpp(
> >  void test(long long ll) {
> >ll | ^1; ll & ^1;
> >  }
> >)cpp";
> > -  EXPECT_THAT(collectPreferredTypes(code8), Each("long long"));
> > +  EXPECT_THAT(collectPreferredTypes(Code), Each("long long"));
> >
> > -  StringRef code9 = R"cpp(
> > +  Code = R"cpp(
> >  enum A {};
> >  void test(A a) {
> >a | ^1; a & ^1;
> >  }
> >)cpp";
> > -  EXPECT_THAT(collectPreferredTypes(code9), Each("enum A"));
> > +  EXPECT_THAT(collectPreferredTypes(Code), Each("enum A"));
> >
> > -  StringRef code10 = R"cpp(
> > +  Code = R"cpp(
> >  enum class A {};
> >  void test(A a) {
> >// This is technically illegal with the 'enum class' without
> overloaded
> > @@ -263,1

[PATCH] D54176: [PGO] clang part of change for context-sensitive PGO.

2018-12-17 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

Clang reviews should mainly go to cfe-commits, not llvm-commits. (all that will 
happen automagically if one sets the correct repo for the differential..)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54176/new/

https://reviews.llvm.org/D54176



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

[PATCH] D55770: [clangd] BackgroundIndex rebuilds symbol index periodically.

2018-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added inline comments.



Comment at: clangd/index/Background.cpp:464
+  if (BuildIndexPeriodMs > 0)
+SymbolsUpdatedSinceLastIndex = true;
+  else

why not simply check if `BuildIndexPeriodMs` has passed and issue rebuild here? 
That way we won't spawn an additional thread, and get rid of the CV


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55770/new/

https://reviews.llvm.org/D55770



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

[PATCH] D55775: [Driver] Don't override '-march' when using '-arch x86_64h'

2018-12-17 Thread Francis Visoiu Mistrih via Phabricator via cfe-commits

thegameg created this revision.
thegameg added reviewers: vsk, dexonsmith, ab, t.p.northover, Gerolf.

On Darwin, using '-arch x86_64h' would always override the option passed 
through '-march'.

This patch allows users to use '-march' with x86_64h, while keeping the default 
to 'core-avx2'


https://reviews.llvm.org/D55775

Files:
  lib/Driver/ToolChains/Arch/X86.cpp
  lib/Driver/ToolChains/Darwin.cpp
  test/Driver/clang-translation.c


Index: test/Driver/clang-translation.c
===
--- test/Driver/clang-translation.c
+++ test/Driver/clang-translation.c
@@ -33,6 +33,11 @@
 // AVX2: "-target-cpu"
 // AVX2: "core-avx2"
 
+// RUN: %clang -target x86_64h-apple-darwin -march=skx -### %s -o /dev/null 
2>&1 | \
+// RUN: FileCheck -check-prefix=X8664HSKX %s
+// X8664HSKX: "-target-cpu"
+// X8664HSKX: "skx"
+
 // RUN: %clang -target i386-apple-macosx10.12 -### -S %s -o %t.s 2>&1 | \
 // RUN: FileCheck -check-prefix=PENRYN %s
 // RUN: %clang -target x86_64-apple-macosx10.12 -### -S %s -o %t.s 2>&1 | \
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -1967,12 +1967,8 @@
 else if (Name == "pentIIm3")
   DAL->AddJoinedArg(nullptr, MArch, "pentium2");
 
-else if (Name == "x86_64")
+else if (Name == "x86_64" || Name == "x86_64h")
   DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_m64));
-else if (Name == "x86_64h") {
-  DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_m64));
-  DAL->AddJoinedArg(nullptr, MArch, "x86_64h");
-}
 
 else if (Name == "arm")
   DAL->AddJoinedArg(nullptr, MArch, "armv4t");
Index: lib/Driver/ToolChains/Arch/X86.cpp
===
--- lib/Driver/ToolChains/Arch/X86.cpp
+++ lib/Driver/ToolChains/Arch/X86.cpp
@@ -23,12 +23,8 @@
 const char *x86::getX86TargetCPU(const ArgList &Args,
  const llvm::Triple &Triple) {
   if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) {
-if (StringRef(A->getValue()) != "native") {
-  if (Triple.isOSDarwin() && Triple.getArchName() == "x86_64h")
-return "core-avx2";
-
+if (StringRef(A->getValue()) != "native")
   return A->getValue();
-}
 
 // FIXME: Reject attempts to use -march=native unless the target matches
 // the host.


Index: test/Driver/clang-translation.c
===
--- test/Driver/clang-translation.c
+++ test/Driver/clang-translation.c
@@ -33,6 +33,11 @@
 // AVX2: "-target-cpu"
 // AVX2: "core-avx2"
 
+// RUN: %clang -target x86_64h-apple-darwin -march=skx -### %s -o /dev/null 2>&1 | \
+// RUN: FileCheck -check-prefix=X8664HSKX %s
+// X8664HSKX: "-target-cpu"
+// X8664HSKX: "skx"
+
 // RUN: %clang -target i386-apple-macosx10.12 -### -S %s -o %t.s 2>&1 | \
 // RUN: FileCheck -check-prefix=PENRYN %s
 // RUN: %clang -target x86_64-apple-macosx10.12 -### -S %s -o %t.s 2>&1 | \
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -1967,12 +1967,8 @@
 else if (Name == "pentIIm3")
   DAL->AddJoinedArg(nullptr, MArch, "pentium2");
 
-else if (Name == "x86_64")
+else if (Name == "x86_64" || Name == "x86_64h")
   DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_m64));
-else if (Name == "x86_64h") {
-  DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_m64));
-  DAL->AddJoinedArg(nullptr, MArch, "x86_64h");
-}
 
 else if (Name == "arm")
   DAL->AddJoinedArg(nullptr, MArch, "armv4t");
Index: lib/Driver/ToolChains/Arch/X86.cpp
===
--- lib/Driver/ToolChains/Arch/X86.cpp
+++ lib/Driver/ToolChains/Arch/X86.cpp
@@ -23,12 +23,8 @@
 const char *x86::getX86TargetCPU(const ArgList &Args,
  const llvm::Triple &Triple) {
   if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) {
-if (StringRef(A->getValue()) != "native") {
-  if (Triple.isOSDarwin() && Triple.getArchName() == "x86_64h")
-return "core-avx2";
-
+if (StringRef(A->getValue()) != "native")
   return A->getValue();
-}
 
 // FIXME: Reject attempts to use -march=native unless the target matches
 // the host.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

2018-12-17 Thread Steven Wu via Phabricator via cfe-commits

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

Other than a small design choice commented inline, LGTM.




Comment at: include/clang/Driver/DarwinSDKInfo.h:36
+/// SDK has no SDKSettings.json, or a valid \c DarwinSDKInfo otherwise.
+Expected> parseDarwinSDKInfo(llvm::vfs::FileSystem 
&VFS,
+ StringRef SDKRootPath);

arphaman wrote:
> steven_wu wrote:
> > Isn't parseSDKSettings enough? And it can just return 
> > Optional?
> We will support other fields besides `VersionTuple` in the SDKSettings, so 
> that's why we have a structure.
I feel like for this usage, it is better to return Expected with 
all the fields being Optional?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55673/new/

https://reviews.llvm.org/D55673



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

[PATCH] D55685: Update Microsoft name mangling scheme for exception specifiers in the type system

2018-12-17 Thread Zachary Henkel via Phabricator via cfe-commits

zahen updated this revision to Diff 178490.
zahen added a comment.

Added support for msvc minor version as requested.  Tied the updated mangling 
scheme to C++17 & compatibility mode > 1912 (15.5).  Added additional tests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55685/new/

https://reviews.llvm.org/D55685

Files:
  include/clang/Basic/LangOptions.h
  lib/AST/MicrosoftMangle.cpp
  lib/Driver/ToolChains/MSVC.cpp
  test/CodeGenCXX/mangle-ms-exception-spec.cpp

Index: test/CodeGenCXX/mangle-ms-exception-spec.cpp
===
--- test/CodeGenCXX/mangle-ms-exception-spec.cpp
+++ test/CodeGenCXX/mangle-ms-exception-spec.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -std=c++11 -fms-extensions -emit-llvm %s -o - -triple=x86_64-pc-win32 -Wno-noexcept-type -fms-compatibility-version=19.12 | FileCheck %s --check-prefix=CHECK --check-prefix=CXX11
+// RUN: %clang_cc1 -std=c++17 -fms-extensions -emit-llvm %s -o - -triple=x86_64-pc-win32 | FileCheck %s --check-prefix=CHECK --check-prefix=NOCOMPAT
+// RUN: %clang_cc1 -std=c++17 -fms-extensions -emit-llvm %s -o - -triple=x86_64-pc-win32 -fms-compatibility-version=19.12 | FileCheck %s --check-prefix=CHECK --check-prefix=CXX17
+
+// Prove that mangling only changed for noexcept types under /std:C++17, not all noexcept functions
+// CHECK-DAG: @"?nochange@@YAXXZ"
+void nochange() noexcept {}
+
+// CXX11-DAG: @"?a@@YAXP6AHXZ@Z"
+// NOCOMPAT-DAG: @"?a@@YAXP6AHXZ@Z"
+// CXX17-DAG: @"?a@@YAXP6AHX_E@Z"
+void a(int() noexcept) {}
+// CHECK-DAG: @"?b@@YAXP6AHXZ@Z"
+void b(int() noexcept(false)) {}
+// CXX11-DAG: @"?c@@YAXP6AHXZ@Z"
+// NOCOMPAT-DAG: @"?c@@YAXP6AHXZ@Z"
+// CXX17-DAG: @"?c@@YAXP6AHX_E@Z"
+void c(int() noexcept(true)) {}
+// CHECK-DAG: @"?d@@YAXP6AHXZ@Z"
+void d(int()) {}
+
+template 
+class e;
+template 
+class e {
+  // CXX11-DAG: @"?ee@?$e@$$A6AXXZ@@EEAAXXZ"
+  // NOCOMPAT-DAG: @"?ee@?$e@$$A6AXXZ@@EEAAXXZ"
+  // CXX17-DAG: @"?ee@?$e@$$A6AXX_E@@EEAAXXZ"
+  virtual T ee(U &&...) noexcept {};
+};
+
+e e1;
+
+template 
+class f;
+template 
+class f {
+  // CHECK-DAG: @"?ff@?$f@$$A6AXXZ@@EEAAXXZ"
+  virtual T ff(U &&...) noexcept {};
+};
+
+f f1;
Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -1286,7 +1286,7 @@
   if (MSVT.empty() &&
   Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
IsWindowsMSVC)) {
-// -fms-compatibility-version=19.11 is default, aka 2017
+// -fms-compatibility-version=19.11 is default, aka 2017, 15.3
 MSVT = VersionTuple(19, 11);
   }
   return MSVT;
Index: lib/AST/MicrosoftMangle.cpp
===
--- lib/AST/MicrosoftMangle.cpp
+++ lib/AST/MicrosoftMangle.cpp
@@ -315,7 +315,8 @@
   QualifierMangleMode QMM = QMM_Mangle);
   void mangleFunctionType(const FunctionType *T,
   const FunctionDecl *D = nullptr,
-  bool ForceThisQuals = false);
+  bool ForceThisQuals = false,
+  bool MangleExceptionSpec = true);
   void mangleNestedName(const NamedDecl *ND);
 
 private:
@@ -512,7 +513,7 @@
 
 mangleFunctionClass(FD);
 
-mangleFunctionType(FT, FD);
+mangleFunctionType(FT, FD, false, false);
   } else {
 Out << '9';
   }
@@ -2061,7 +2062,8 @@
 
 void MicrosoftCXXNameMangler::mangleFunctionType(const FunctionType *T,
  const FunctionDecl *D,
- bool ForceThisQuals) {
+ bool ForceThisQuals,
+ bool MangleExceptionSpec) {
   //  ::=  
   //   
   const FunctionProtoType *Proto = dyn_cast(T);
@@ -2194,7 +2196,12 @@
   Out << '@';
   }
 
-  mangleThrowSpecification(Proto);
+  if (MangleExceptionSpec && getASTContext().getLangOpts().CPlusPlus17 &&
+  getASTContext().getLangOpts().isCompatibleWithMSVC(
+  LangOptions::MSVC2017_5))
+mangleThrowSpecification(Proto);
+  else
+Out << 'Z';
 }
 
 void MicrosoftCXXNameMangler::mangleFunctionClass(const FunctionDecl *FD) {
@@ -2299,15 +2306,15 @@
 void MicrosoftCXXNameMangler::mangleCallingConvention(const FunctionType *T) {
   mangleCallingConvention(T->getCallConv());
 }
+
 void MicrosoftCXXNameMangler::mangleThrowSpecification(
 const FunctionProtoType *FT) {
-  //  ::= Z # throw(...) (default)
-  //  ::= @ # throw() or __declspec/__attribute__((nothrow))
-  //  ::= +
-  // NOTE: Since the Microsoft compiler ignores throw specifications, they are
-  // all actually mangled as 'Z'. (They're ignored because their associated
-  // functionality isn't implemented, and probably never will be.

[PATCH] D55749: [Driver] Automatically enable -munwind-tables if -fseh-exceptions is enabled

2018-12-17 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:3929
+  options::OPT_fno_asynchronous_unwind_tables,
+  (TC.IsUnwindTablesDefault(Args) ||
+   (ExceptionArg &&

Can you put this logic in the Windows or mingw toolchain instead by overriding 
IsUnwindTablesDefault? -fseh-exceptions feels fairly target specific.

If not, this boolean default feels fairly complicated. Here's how I would 
restructure it:
```
bool UnwindTables = false;
if (!Freestanding)
  UnwindTables = TC.IsUnwindTablesDefault(Args) || 
TC.getSanitizerArgs().needsUnwindTables();
UnwindTables = Args.hasFlag(OPT_fasync..., OPT_fno_async..., UnwindTables);
UnwindTables = Args.hasFlag(OPT_munwind_tables, OPT_mno_unwind_tables, 
UnwindTables);
```
Basically, we have some defaults dictated by the toolchain, and then two 
boolean flags, where one takes precedence over the other. -fno-async by itself 
will disable unwind tables, but `-munwind-tables 
-fno-asynchronous-unwind-tables` will leave them enabled, despite not following 
the usual "last flag wins" logic.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55749/new/

https://reviews.llvm.org/D55749



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

2018-12-17 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: include/clang/Basic/Attr.td:2871-2872
   /// distribute: attempt to distribute loop if State == Enable
+  /// pipeline: disable pipelining loop if State == Disable
+  /// pipeline_ii_count: try to pipeline loop for only 'Value' value
 

alexey.lapshin wrote:
> aaron.ballman wrote:
> > Missing full stops at the end of the comments. Also, having read "for only 
> > 'Value' value", I'm still not certain what's happening. I think this is a 
> > count of some kind, so perhaps "Tries to pipeline 'Values' times." but I 
> > don't know what the verb "pipeline" means in this context.
> > 
> > Are users going to understand what the `ii` means in the user-facing name?
> As to ii - yes that should be understood by users, because it is important 
> property of software pipelining optimization. Software Pipelining 
> optimization tries to reorder instructions of original loop(from different 
> iterations) so that resulting loop body takes less cycles. It started from 
> some minimal value of ii and stopped with some maximal value.  i.e. it tries 
> to built schedule for min_ii, then min_ii+1, ... until schedule is found or 
> until max_ii reached.  If resulting value of ii already known then instead of 
> searching in range min_ii, max_ii - it is possible to create schedule for 
> already known ii. 
> 
> probably following spelling would be better : 
> 
> pipeline_ii_count: create loop schedule with initiation interval equals 
> 'Value'
> because it is important property of software pipelining optimization. 

My point is that I have no idea what "ii" means and I doubt I'll be alone -- 
does the "ii" differentiate this from other kinds of pipeline loop pragmas we 
plan to support, or is expected that "pipeline_ii_count" be the only pipeline 
count option? Can we drop the "ii" and not lose anything?

> pipeline_ii_count: create loop schedule with initiation interval equals 
> 'Value'

equals 'Value' -> equal to 'Value'



Comment at: include/clang/Basic/DiagnosticParseKinds.td:1192
+def err_pragma_pipeline_invalid_keyword : Error<
+"invalid argument; expected 'disable'">;
 

alexey.lapshin wrote:
> aaron.ballman wrote:
> > Can you roll this into the above diagnostic with another `%select`, or does 
> > that make it too unreadable?
> yes, it makes things too complicated. Though I could do it if necessary.
Not required, but I also didn't think the duplication here and below was a good 
approach either. But yeah, I'm not certain there's a better way to do this even 
if the list were rearranged.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55710/new/

https://reviews.llvm.org/D55710



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

[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

2018-12-17 Thread Alex Lorenz via Phabricator via cfe-commits

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



Comment at: include/clang/Driver/DarwinSDKInfo.h:36
+/// SDK has no SDKSettings.json, or a valid \c DarwinSDKInfo otherwise.
+Expected> parseDarwinSDKInfo(llvm::vfs::FileSystem 
&VFS,
+ StringRef SDKRootPath);

steven_wu wrote:
> arphaman wrote:
> > steven_wu wrote:
> > > Isn't parseSDKSettings enough? And it can just return 
> > > Optional?
> > We will support other fields besides `VersionTuple` in the SDKSettings, so 
> > that's why we have a structure.
> I feel like for this usage, it is better to return Expected 
> with all the fields being Optional?
Hmm, we want to assume that version exists for future uses. I feel like the 
current type captures the intention better.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55673/new/

https://reviews.llvm.org/D55673



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

2018-12-17 Thread Alexey Lapshin via Phabricator via cfe-commits

alexey.lapshin marked an inline comment as done.
alexey.lapshin added inline comments.



Comment at: include/clang/Basic/Attr.td:2871-2872
   /// distribute: attempt to distribute loop if State == Enable
+  /// pipeline: disable pipelining loop if State == Disable
+  /// pipeline_ii_count: try to pipeline loop for only 'Value' value
 

aaron.ballman wrote:
> alexey.lapshin wrote:
> > aaron.ballman wrote:
> > > Missing full stops at the end of the comments. Also, having read "for 
> > > only 'Value' value", I'm still not certain what's happening. I think this 
> > > is a count of some kind, so perhaps "Tries to pipeline 'Values' times." 
> > > but I don't know what the verb "pipeline" means in this context.
> > > 
> > > Are users going to understand what the `ii` means in the user-facing name?
> > As to ii - yes that should be understood by users, because it is important 
> > property of software pipelining optimization. Software Pipelining 
> > optimization tries to reorder instructions of original loop(from different 
> > iterations) so that resulting loop body takes less cycles. It started from 
> > some minimal value of ii and stopped with some maximal value.  i.e. it 
> > tries to built schedule for min_ii, then min_ii+1, ... until schedule is 
> > found or until max_ii reached.  If resulting value of ii already known then 
> > instead of searching in range min_ii, max_ii - it is possible to create 
> > schedule for already known ii. 
> > 
> > probably following spelling would be better : 
> > 
> > pipeline_ii_count: create loop schedule with initiation interval equals 
> > 'Value'
> > because it is important property of software pipelining optimization. 
> 
> My point is that I have no idea what "ii" means and I doubt I'll be alone -- 
> does the "ii" differentiate this from other kinds of pipeline loop pragmas we 
> plan to support, or is expected that "pipeline_ii_count" be the only pipeline 
> count option? Can we drop the "ii" and not lose anything?
> 
> > pipeline_ii_count: create loop schedule with initiation interval equals 
> > 'Value'
> 
> equals 'Value' -> equal to 'Value'
Do you think spelling out  ii would help ? 

pipeline_initiation_interval(number)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55710/new/

https://reviews.llvm.org/D55710



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

[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

2018-12-17 Thread Steven Wu via Phabricator via cfe-commits

steven_wu added inline comments.



Comment at: include/clang/Driver/DarwinSDKInfo.h:36
+/// SDK has no SDKSettings.json, or a valid \c DarwinSDKInfo otherwise.
+Expected> parseDarwinSDKInfo(llvm::vfs::FileSystem 
&VFS,
+ StringRef SDKRootPath);

arphaman wrote:
> steven_wu wrote:
> > arphaman wrote:
> > > steven_wu wrote:
> > > > Isn't parseSDKSettings enough? And it can just return 
> > > > Optional?
> > > We will support other fields besides `VersionTuple` in the SDKSettings, 
> > > so that's why we have a structure.
> > I feel like for this usage, it is better to return Expected 
> > with all the fields being Optional?
> Hmm, we want to assume that version exists for future uses. I feel like the 
> current type captures the intention better.
I think it is fine for current use. We can always change in the future.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55673/new/

https://reviews.llvm.org/D55673



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

2018-12-17 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: include/clang/Basic/Attr.td:2871-2872
   /// distribute: attempt to distribute loop if State == Enable
+  /// pipeline: disable pipelining loop if State == Disable
+  /// pipeline_ii_count: try to pipeline loop for only 'Value' value
 

alexey.lapshin wrote:
> aaron.ballman wrote:
> > alexey.lapshin wrote:
> > > aaron.ballman wrote:
> > > > Missing full stops at the end of the comments. Also, having read "for 
> > > > only 'Value' value", I'm still not certain what's happening. I think 
> > > > this is a count of some kind, so perhaps "Tries to pipeline 'Values' 
> > > > times." but I don't know what the verb "pipeline" means in this context.
> > > > 
> > > > Are users going to understand what the `ii` means in the user-facing 
> > > > name?
> > > As to ii - yes that should be understood by users, because it is 
> > > important property of software pipelining optimization. Software 
> > > Pipelining optimization tries to reorder instructions of original 
> > > loop(from different iterations) so that resulting loop body takes less 
> > > cycles. It started from some minimal value of ii and stopped with some 
> > > maximal value.  i.e. it tries to built schedule for min_ii, then 
> > > min_ii+1, ... until schedule is found or until max_ii reached.  If 
> > > resulting value of ii already known then instead of searching in range 
> > > min_ii, max_ii - it is possible to create schedule for already known ii. 
> > > 
> > > probably following spelling would be better : 
> > > 
> > > pipeline_ii_count: create loop schedule with initiation interval equals 
> > > 'Value'
> > > because it is important property of software pipelining optimization. 
> > 
> > My point is that I have no idea what "ii" means and I doubt I'll be alone 
> > -- does the "ii" differentiate this from other kinds of pipeline loop 
> > pragmas we plan to support, or is expected that "pipeline_ii_count" be the 
> > only pipeline count option? Can we drop the "ii" and not lose anything?
> > 
> > > pipeline_ii_count: create loop schedule with initiation interval equals 
> > > 'Value'
> > 
> > equals 'Value' -> equal to 'Value'
> Do you think spelling out  ii would help ? 
> 
> pipeline_initiation_interval(number)
I think it would help. I'm less concerned about the internal names of things 
than I am for the user-facing identifiers and whether it will be understandable 
to people other than compiler and optimizer writers.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55710/new/

https://reviews.llvm.org/D55710



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

[PATCH] D55770: [clangd] BackgroundIndex rebuilds symbol index periodically.

2018-12-17 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added a comment.

(from the peanut gallery): Usually what I'd expect here is "reindexing will 
occur X milliseconds after a write, covering anything written up until that 
point" - so it doesn't reindex unnecessarily if the files are 
unmodified/sitting idle, and it doesn't thrash reindexing for multiple changes 
in rapid succession (such as hitting the "save all" button in some text editor 
- or having made a few other quick changes one after another maybe), but still 
provides reasonable up-to-date data for the user.

(but I don't know anything about clangd & maybe what I'm saying makes no 
sense/isn't relevant)


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55770/new/

https://reviews.llvm.org/D55770



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

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

2018-12-17 Thread Leonard Chan via Phabricator via cfe-commits

leonardchan added a comment.

In D53738#1326071 , @rjmccall wrote:

> I'm fine with making this change under the assumption that we've gotten the 
> language rule right.  Even if that weren't abstractly reasonable for general 
> language work — and I do think it's reasonable when we have a good-faith 
> question about the right semantics — this is clearly still an experimental 
> implementation and will be for several months yet, and hopefully it won't 
> take that long for us to get a response.


@rjmccall Have you received a response yet? If not, do you think you have an 
estimate on the response time, or also mind sharing the contact information if 
that's ok?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53738/new/

https://reviews.llvm.org/D53738



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

[PATCH] D55731: [darwin][arm64] use the "cyclone" CPU for Darwin even when `-arch` is not specified

2018-12-17 Thread Ahmed Bougacha via Phabricator via cfe-commits

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

LGTM, thanks!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55731/new/

https://reviews.llvm.org/D55731



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

[PATCH] D54604: Automatic variable initialization

2018-12-17 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added a comment.

Thanks, you've addressed my comments to my satisfaction. (Not approving as I've 
not done a detailed review of the implementation, only of the approach, but it 
looks like your review with @pcc is nearly complete.)

In D54604#1330356 , @jfb wrote:

> In D54604#1327893 , @rsmith wrote:
>
> > For the record: I'm OK with this direction. (I somewhat prefer removing the 
> > `-enable-long-wordy-thing` option and instead automatically disabling the 
> > `zero` option for clang release builds, but I'm OK with either approach.)
>
>
> I'm inclined to go with what @kcc wants, because I want the data he'll 
> gather. For my own use, I don't really care. One thing I could do: check 
> `__DATE__` and generate a warning if it's above a certain value and `zero` is 
> used? i.e. check the date at which the compiler was compiled (not when it's 
> used!) and error out if it was compiled after a certain point in time? I'm 
> throwing out options that could makes folks more comfortable with `zero`...


If you want to go there, I'd be OK with that. I don't personally think it's 
necessary.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54604/new/

https://reviews.llvm.org/D54604



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

r349380 - [darwin] parse the SDK settings from SDKSettings.json if it exists and

2018-12-17 Thread Alex Lorenz via cfe-commits

Author: arphaman
Date: Mon Dec 17 11:19:15 2018
New Revision: 349380

URL: http://llvm.org/viewvc/llvm-project?rev=349380&view=rev
Log:
[darwin] parse the SDK settings from SDKSettings.json if it exists and
pass in the -target-sdk-version to the compiler and backend

This commit adds support for reading the SDKSettings.json file in the Darwin
driver. This file is used by the driver to determine the SDK's version, and it
uses that information to pass it down to the compiler using the new
-target-sdk-version= option. This option is then used to set the appropriate
SDK Version module metadata introduced in r349119.

Note: I had to adjust the two ast tests as the SDKROOT environment variable
on macOS caused SDK version to be picked up for the compilation of source file
but not the AST.

rdar://45774000

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

Added:
cfe/trunk/include/clang/Driver/DarwinSDKInfo.h
cfe/trunk/lib/Driver/DarwinSDKInfo.cpp
cfe/trunk/test/CodeGen/darwin-sdk-version.c
cfe/trunk/test/Driver/Inputs/MacOSX10.14.sdk/
cfe/trunk/test/Driver/Inputs/MacOSX10.14.sdk/SDKSettings.json
cfe/trunk/test/Driver/darwin-sdk-version.c
Modified:
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/include/clang/Basic/TargetInfo.h
cfe/trunk/include/clang/Basic/TargetOptions.h
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/lib/CodeGen/ModuleBuilder.cpp
cfe/trunk/lib/Driver/CMakeLists.txt
cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
cfe/trunk/lib/Driver/ToolChains/Darwin.h
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/Frontend/ast-main.c
cfe/trunk/test/Frontend/ast-main.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=349380&r1=349379&r2=349380&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Mon Dec 17 11:19:15 
2018
@@ -405,4 +405,8 @@ def warn_drv_experimental_isel_incomplet
 def warn_drv_moutline_unsupported_opt : Warning<
   "The '%0' architecture does not support -moutline; flag ignored">,
   InGroup;
+
+def warn_drv_darwin_sdk_invalid_settings : Warning<
+  "SDK settings were ignored as 'SDKSettings.json' could not be parsed">,
+  InGroup>;
 }

Modified: cfe/trunk/include/clang/Basic/TargetInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TargetInfo.h?rev=349380&r1=349379&r2=349380&view=diff
==
--- cfe/trunk/include/clang/Basic/TargetInfo.h (original)
+++ cfe/trunk/include/clang/Basic/TargetInfo.h Mon Dec 17 11:19:15 2018
@@ -1321,6 +1321,12 @@ public:
 return None;
   }
 
+  /// \returns The version of the SDK which was used during the compilation if
+  /// one was specified, or an empty version otherwise.
+  const llvm::VersionTuple &getSDKVersion() const {
+return getTargetOpts().SDKVersion;
+  }
+
   /// Check the target is valid after it is fully initialized.
   virtual bool validateTarget(DiagnosticsEngine &Diags) const {
 return true;

Modified: cfe/trunk/include/clang/Basic/TargetOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TargetOptions.h?rev=349380&r1=349379&r2=349380&view=diff
==
--- cfe/trunk/include/clang/Basic/TargetOptions.h (original)
+++ cfe/trunk/include/clang/Basic/TargetOptions.h Mon Dec 17 11:19:15 2018
@@ -15,10 +15,11 @@
 #ifndef LLVM_CLANG_BASIC_TARGETOPTIONS_H
 #define LLVM_CLANG_BASIC_TARGETOPTIONS_H
 
-#include 
-#include 
 #include "clang/Basic/OpenCLOptions.h"
+#include "llvm/Support/VersionTuple.h"
 #include "llvm/Target/TargetOptions.h"
+#include 
+#include 
 
 namespace clang {
 
@@ -73,6 +74,9 @@ public:
   // "default" for the case when the user has not explicitly specified a
   // code model.
   std::string CodeModel;
+
+  /// The version of the SDK which was used during the compilation.
+  VersionTuple SDKVersion;
 };
 
 }  // end namespace clang

Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=349380&r1=349379&r2=349380&view=diff
==
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)
+++ cfe/trunk/include/clang/Driver/CC1Options.td Mon Dec 17 11:19:15 2018
@@ -27,6 +27,8 @@ def triple : Separate<["-"], "triple">,
   HelpText<"Specify target triple (e.g. i686-apple-darwin9)">;
 def target_abi : Separate<["-"], "target-abi">,
   HelpText<"Target a particular ABI type">;
+def target_sdk_version_EQ : Joined<["-"], "target-sdk-version=">,
+  HelpText<"The version of target SDK used

[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

2018-12-17 Thread Alex Lorenz via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rC349380: [darwin] parse the SDK settings from 
SDKSettings.json if it exists and (authored by arphaman, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55673?vs=178309&id=178501#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55673/new/

https://reviews.llvm.org/D55673

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Basic/TargetInfo.h
  include/clang/Basic/TargetOptions.h
  include/clang/Driver/CC1Options.td
  include/clang/Driver/DarwinSDKInfo.h
  lib/CodeGen/ModuleBuilder.cpp
  lib/Driver/CMakeLists.txt
  lib/Driver/DarwinSDKInfo.cpp
  lib/Driver/ToolChains/Darwin.cpp
  lib/Driver/ToolChains/Darwin.h
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/darwin-sdk-version.c
  test/Driver/Inputs/MacOSX10.14.sdk/SDKSettings.json
  test/Driver/darwin-sdk-version.c
  test/Frontend/ast-main.c
  test/Frontend/ast-main.cpp

Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -3193,6 +3193,14 @@
   Opts.ForceEnableInt128 = Args.hasArg(OPT_fforce_enable_int128);
   Opts.NVPTXUseShortPointers = Args.hasFlag(
   options::OPT_fcuda_short_ptr, options::OPT_fno_cuda_short_ptr, false);
+  if (Arg *A = Args.getLastArg(options::OPT_target_sdk_version_EQ)) {
+llvm::VersionTuple Version;
+if (Version.tryParse(A->getValue()))
+  Diags.Report(diag::err_drv_invalid_value)
+  << A->getAsString(Args) << A->getValue();
+else
+  Opts.SDKVersion = Version;
+  }
 }
 
 bool CompilerInvocation::CreateFromArgs(CompilerInvocation &Res,
Index: lib/CodeGen/ModuleBuilder.cpp
===
--- lib/CodeGen/ModuleBuilder.cpp
+++ lib/CodeGen/ModuleBuilder.cpp
@@ -132,6 +132,9 @@
 
   M->setTargetTriple(Ctx->getTargetInfo().getTriple().getTriple());
   M->setDataLayout(Ctx->getTargetInfo().getDataLayout());
+  const auto &SDKVersion = Ctx->getTargetInfo().getSDKVersion();
+  if (!SDKVersion.empty())
+M->setSDKVersion(SDKVersion);
   Builder.reset(new CodeGen::CodeGenModule(Context, HeaderSearchOpts,
PreprocessorOpts, CodeGenOpts,
*M, Diags, CoverageInfo));
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -1287,6 +1287,18 @@
 return DarwinPlatform(InferredFromArch, getPlatformFromOS(OS), Value);
   }
 
+  /// Constructs an inferred SDKInfo value based on the version inferred from
+  /// the SDK path itself. Only works for values that were created by inferring
+  /// the platform from the SDKPath.
+  DarwinSDKInfo inferSDKInfo() {
+assert(Kind == InferredFromSDK && "can infer SDK info only");
+llvm::VersionTuple Version;
+bool IsValid = !Version.tryParse(OSVersion);
+(void)IsValid;
+assert(IsValid && "invalid SDK version");
+return DarwinSDKInfo(Version);
+  }
+
 private:
   DarwinPlatform(SourceKind Kind, DarwinPlatformKind Platform, Arg *Argument)
   : Kind(Kind), Platform(Platform), Argument(Argument) {}
@@ -1420,8 +1432,11 @@
 }
 
 /// Tries to infer the deployment target from the SDK specified by -isysroot
-/// (or SDKROOT).
-Optional inferDeploymentTargetFromSDK(DerivedArgList &Args) {
+/// (or SDKROOT). Uses the version specified in the SDKSettings.json file if
+/// it's available.
+Optional
+inferDeploymentTargetFromSDK(DerivedArgList &Args,
+ const Optional &SDKInfo) {
   const Arg *A = Args.getLastArg(options::OPT_isysroot);
   if (!A)
 return None;
@@ -1429,28 +1444,37 @@
   StringRef SDK = Darwin::getSDKName(isysroot);
   if (!SDK.size())
 return None;
-  // Slice the version number out.
-  // Version number is between the first and the last number.
-  size_t StartVer = SDK.find_first_of("0123456789");
-  size_t EndVer = SDK.find_last_of("0123456789");
-  if (StartVer != StringRef::npos && EndVer > StartVer) {
-StringRef Version = SDK.slice(StartVer, EndVer + 1);
-if (SDK.startswith("iPhoneOS") || SDK.startswith("iPhoneSimulator"))
-  return DarwinPlatform::createFromSDK(
-  Darwin::IPhoneOS, Version,
-  /*IsSimulator=*/SDK.startswith("iPhoneSimulator"));
-else if (SDK.startswith("MacOSX"))
-  return DarwinPlatform::createFromSDK(Darwin::MacOS,
-   getSystemOrSDKMacOSVersion(Version));
-else if (SDK.startswith("WatchOS") || SDK.startswith("WatchSimulator"))
-  return DarwinPlatform::createFromSDK(
-  Darwin::WatchOS, Version,
-  /*IsSimulator=*/SDK.startswith("WatchSimulator"));
-else if (SDK.startswith("AppleTVOS") ||

[PATCH] D55775: [Driver] Don't override '-march' when using '-arch x86_64h'

2018-12-17 Thread Gerolf Hoflehner via Phabricator via cfe-commits

Gerolf added reviewers: ahatanak, qcolombet.
Gerolf added a comment.

+ Akira, Quentin for their driver / x86_64h experience for a quick double 
check. Fwiw, LGTM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55775/new/

https://reviews.llvm.org/D55775



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

[PATCH] D55775: [Driver] Don't override '-march' when using '-arch x86_64h'

2018-12-17 Thread Alex Lorenz via Phabricator via cfe-commits

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

LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55775/new/

https://reviews.llvm.org/D55775



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

2018-12-17 Thread Hal Finkel via Phabricator via cfe-commits

hfinkel added inline comments.



Comment at: include/clang/Basic/Attr.td:2871-2872
   /// distribute: attempt to distribute loop if State == Enable
+  /// pipeline: disable pipelining loop if State == Disable
+  /// pipeline_ii_count: try to pipeline loop for only 'Value' value
 

aaron.ballman wrote:
> alexey.lapshin wrote:
> > aaron.ballman wrote:
> > > alexey.lapshin wrote:
> > > > aaron.ballman wrote:
> > > > > Missing full stops at the end of the comments. Also, having read "for 
> > > > > only 'Value' value", I'm still not certain what's happening. I think 
> > > > > this is a count of some kind, so perhaps "Tries to pipeline 'Values' 
> > > > > times." but I don't know what the verb "pipeline" means in this 
> > > > > context.
> > > > > 
> > > > > Are users going to understand what the `ii` means in the user-facing 
> > > > > name?
> > > > As to ii - yes that should be understood by users, because it is 
> > > > important property of software pipelining optimization. Software 
> > > > Pipelining optimization tries to reorder instructions of original 
> > > > loop(from different iterations) so that resulting loop body takes less 
> > > > cycles. It started from some minimal value of ii and stopped with some 
> > > > maximal value.  i.e. it tries to built schedule for min_ii, then 
> > > > min_ii+1, ... until schedule is found or until max_ii reached.  If 
> > > > resulting value of ii already known then instead of searching in range 
> > > > min_ii, max_ii - it is possible to create schedule for already known 
> > > > ii. 
> > > > 
> > > > probably following spelling would be better : 
> > > > 
> > > > pipeline_ii_count: create loop schedule with initiation interval equals 
> > > > 'Value'
> > > > because it is important property of software pipelining optimization. 
> > > 
> > > My point is that I have no idea what "ii" means and I doubt I'll be alone 
> > > -- does the "ii" differentiate this from other kinds of pipeline loop 
> > > pragmas we plan to support, or is expected that "pipeline_ii_count" be 
> > > the only pipeline count option? Can we drop the "ii" and not lose 
> > > anything?
> > > 
> > > > pipeline_ii_count: create loop schedule with initiation interval equals 
> > > > 'Value'
> > > 
> > > equals 'Value' -> equal to 'Value'
> > Do you think spelling out  ii would help ? 
> > 
> > pipeline_initiation_interval(number)
> I think it would help. I'm less concerned about the internal names of things 
> than I am for the user-facing identifiers and whether it will be 
> understandable to people other than compiler and optimizer writers.
Yes, I also think that it would help to spell this out.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55710/new/

https://reviews.llvm.org/D55710



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

[PATCH] D55775: [Driver] Don't override '-march' when using '-arch x86_64h'

2018-12-17 Thread Francis Visoiu Mistrih via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL349381: [Driver] Don't override '-march' when 
using '-arch x86_64h' (authored by thegameg, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55775?vs=178486&id=178504#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55775/new/

https://reviews.llvm.org/D55775

Files:
  cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp
  cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
  cfe/trunk/test/Driver/clang-translation.c


Index: cfe/trunk/test/Driver/clang-translation.c
===
--- cfe/trunk/test/Driver/clang-translation.c
+++ cfe/trunk/test/Driver/clang-translation.c
@@ -33,6 +33,11 @@
 // AVX2: "-target-cpu"
 // AVX2: "core-avx2"
 
+// RUN: %clang -target x86_64h-apple-darwin -march=skx -### %s -o /dev/null 
2>&1 | \
+// RUN: FileCheck -check-prefix=X8664HSKX %s
+// X8664HSKX: "-target-cpu"
+// X8664HSKX: "skx"
+
 // RUN: %clang -target i386-apple-macosx10.12 -### -S %s -o %t.s 2>&1 | \
 // RUN: FileCheck -check-prefix=PENRYN %s
 // RUN: %clang -target x86_64-apple-macosx10.12 -### -S %s -o %t.s 2>&1 | \
Index: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
@@ -2017,12 +2017,8 @@
 else if (Name == "pentIIm3")
   DAL->AddJoinedArg(nullptr, MArch, "pentium2");
 
-else if (Name == "x86_64")
+else if (Name == "x86_64" || Name == "x86_64h")
   DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_m64));
-else if (Name == "x86_64h") {
-  DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_m64));
-  DAL->AddJoinedArg(nullptr, MArch, "x86_64h");
-}
 
 else if (Name == "arm")
   DAL->AddJoinedArg(nullptr, MArch, "armv4t");
Index: cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp
@@ -23,12 +23,8 @@
 const char *x86::getX86TargetCPU(const ArgList &Args,
  const llvm::Triple &Triple) {
   if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) {
-if (StringRef(A->getValue()) != "native") {
-  if (Triple.isOSDarwin() && Triple.getArchName() == "x86_64h")
-return "core-avx2";
-
+if (StringRef(A->getValue()) != "native")
   return A->getValue();
-}
 
 // FIXME: Reject attempts to use -march=native unless the target matches
 // the host.


Index: cfe/trunk/test/Driver/clang-translation.c
===
--- cfe/trunk/test/Driver/clang-translation.c
+++ cfe/trunk/test/Driver/clang-translation.c
@@ -33,6 +33,11 @@
 // AVX2: "-target-cpu"
 // AVX2: "core-avx2"
 
+// RUN: %clang -target x86_64h-apple-darwin -march=skx -### %s -o /dev/null 2>&1 | \
+// RUN: FileCheck -check-prefix=X8664HSKX %s
+// X8664HSKX: "-target-cpu"
+// X8664HSKX: "skx"
+
 // RUN: %clang -target i386-apple-macosx10.12 -### -S %s -o %t.s 2>&1 | \
 // RUN: FileCheck -check-prefix=PENRYN %s
 // RUN: %clang -target x86_64-apple-macosx10.12 -### -S %s -o %t.s 2>&1 | \
Index: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
@@ -2017,12 +2017,8 @@
 else if (Name == "pentIIm3")
   DAL->AddJoinedArg(nullptr, MArch, "pentium2");
 
-else if (Name == "x86_64")
+else if (Name == "x86_64" || Name == "x86_64h")
   DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_m64));
-else if (Name == "x86_64h") {
-  DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_m64));
-  DAL->AddJoinedArg(nullptr, MArch, "x86_64h");
-}
 
 else if (Name == "arm")
   DAL->AddJoinedArg(nullptr, MArch, "armv4t");
Index: cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp
@@ -23,12 +23,8 @@
 const char *x86::getX86TargetCPU(const ArgList &Args,
  const llvm::Triple &Triple) {
   if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) {
-if (StringRef(A->getValue()) != "native") {
-  if (Triple.isOSDarwin() && Triple.getArchName() == "x86_64h")
-return "core-avx2";
-
+if (StringRef(A->getValue()) != "native")
   return A->getValue();
-}
 
 // FIXME: Reject attempts to use -march=native unless the target matches
 // the host.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.

r349381 - [Driver] Don't override '-march' when using '-arch x86_64h'

2018-12-17 Thread Francis Visoiu Mistrih via cfe-commits

Author: thegameg
Date: Mon Dec 17 11:29:27 2018
New Revision: 349381

URL: http://llvm.org/viewvc/llvm-project?rev=349381&view=rev
Log:
[Driver] Don't override '-march' when using '-arch x86_64h'

On Darwin, using '-arch x86_64h' would always override the option passed
through '-march'.

This patch allows users to use '-march' with x86_64h, while keeping the
default to 'core-avx2'

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

Modified:
cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp
cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
cfe/trunk/test/Driver/clang-translation.c

Modified: cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp?rev=349381&r1=349380&r2=349381&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp Mon Dec 17 11:29:27 2018
@@ -23,12 +23,8 @@ using namespace llvm::opt;
 const char *x86::getX86TargetCPU(const ArgList &Args,
  const llvm::Triple &Triple) {
   if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) {
-if (StringRef(A->getValue()) != "native") {
-  if (Triple.isOSDarwin() && Triple.getArchName() == "x86_64h")
-return "core-avx2";
-
+if (StringRef(A->getValue()) != "native")
   return A->getValue();
-}
 
 // FIXME: Reject attempts to use -march=native unless the target matches
 // the host.

Modified: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Darwin.cpp?rev=349381&r1=349380&r2=349381&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp Mon Dec 17 11:29:27 2018
@@ -2017,12 +2017,8 @@ DerivedArgList *MachO::TranslateArgs(con
 else if (Name == "pentIIm3")
   DAL->AddJoinedArg(nullptr, MArch, "pentium2");
 
-else if (Name == "x86_64")
+else if (Name == "x86_64" || Name == "x86_64h")
   DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_m64));
-else if (Name == "x86_64h") {
-  DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_m64));
-  DAL->AddJoinedArg(nullptr, MArch, "x86_64h");
-}
 
 else if (Name == "arm")
   DAL->AddJoinedArg(nullptr, MArch, "armv4t");

Modified: cfe/trunk/test/Driver/clang-translation.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/clang-translation.c?rev=349381&r1=349380&r2=349381&view=diff
==
--- cfe/trunk/test/Driver/clang-translation.c (original)
+++ cfe/trunk/test/Driver/clang-translation.c Mon Dec 17 11:29:27 2018
@@ -33,6 +33,11 @@
 // AVX2: "-target-cpu"
 // AVX2: "core-avx2"
 
+// RUN: %clang -target x86_64h-apple-darwin -march=skx -### %s -o /dev/null 
2>&1 | \
+// RUN: FileCheck -check-prefix=X8664HSKX %s
+// X8664HSKX: "-target-cpu"
+// X8664HSKX: "skx"
+
 // RUN: %clang -target i386-apple-macosx10.12 -### -S %s -o %t.s 2>&1 | \
 // RUN: FileCheck -check-prefix=PENRYN %s
 // RUN: %clang -target x86_64-apple-macosx10.12 -### -S %s -o %t.s 2>&1 | \


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

r349382 - [darwin][arm64] use the "cyclone" CPU for Darwin even when `-arch`

2018-12-17 Thread Alex Lorenz via cfe-commits

Author: arphaman
Date: Mon Dec 17 11:30:46 2018
New Revision: 349382

URL: http://llvm.org/viewvc/llvm-project?rev=349382&view=rev
Log:
[darwin][arm64] use the "cyclone" CPU for Darwin even when `-arch`
is not specified

The -target option allows the user to specify the build target using LLVM
triple. The triple includes the arch, and so the -arch option is redundant.
This should work just as well without the -arch. However, the driver has a bug
in which it doesn't target the "Cyclone" CPU for darwin if -target is used
without -arch. This commit fixes this issue.

rdar://46743182

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

Modified:
cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp
cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.h
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
cfe/trunk/test/Driver/aarch64-cpus.c

Modified: cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp?rev=349382&r1=349381&r2=349382&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.cpp Mon Dec 17 11:30:46 2018
@@ -19,10 +19,17 @@ using namespace clang::driver::tools;
 using namespace clang;
 using namespace llvm::opt;
 
+/// \returns true if the given triple can determine the default CPU type even
+/// if -arch is not specified.
+static bool isCPUDeterminedByTriple(const llvm::Triple &Triple) {
+  return Triple.isOSDarwin();
+}
+
 /// getAArch64TargetCPU - Get the (LLVM) name of the AArch64 cpu we are
 /// targeting. Set \p A to the Arg corresponding to the -mcpu argument if it is
 /// provided, or to nullptr otherwise.
-std::string aarch64::getAArch64TargetCPU(const ArgList &Args, Arg *&A) {
+std::string aarch64::getAArch64TargetCPU(const ArgList &Args,
+ const llvm::Triple &Triple, Arg *&A) {
   std::string CPU;
   // If we have -mcpu, use that.
   if ((A = Args.getLastArg(options::OPT_mcpu_EQ))) {
@@ -36,9 +43,9 @@ std::string aarch64::getAArch64TargetCPU
   else if (CPU.size())
 return CPU;
 
-  // Make sure we pick "cyclone" if -arch is used.
-  // FIXME: Should this be picked by checking the target triple instead?
-  if (Args.getLastArg(options::OPT_arch))
+  // Make sure we pick "cyclone" if -arch is used or when targetting a Darwin
+  // OS.
+  if (Args.getLastArg(options::OPT_arch) || Triple.isOSDarwin())
 return "cyclone";
 
   return "generic";
@@ -152,7 +159,9 @@ getAArch64MicroArchFeaturesFromMcpu(cons
   return getAArch64MicroArchFeaturesFromMtune(D, CPU, Args, Features);
 }
 
-void aarch64::getAArch64TargetFeatures(const Driver &D, const ArgList &Args,
+void aarch64::getAArch64TargetFeatures(const Driver &D,
+   const llvm::Triple &Triple,
+   const ArgList &Args,
std::vector &Features) {
   Arg *A;
   bool success = true;
@@ -162,9 +171,9 @@ void aarch64::getAArch64TargetFeatures(c
 success = getAArch64ArchFeaturesFromMarch(D, A->getValue(), Args, 
Features);
   else if ((A = Args.getLastArg(options::OPT_mcpu_EQ)))
 success = getAArch64ArchFeaturesFromMcpu(D, A->getValue(), Args, Features);
-  else if (Args.hasArg(options::OPT_arch))
-success = getAArch64ArchFeaturesFromMcpu(D, getAArch64TargetCPU(Args, A),
- Args, Features);
+  else if (Args.hasArg(options::OPT_arch) || isCPUDeterminedByTriple(Triple))
+success = getAArch64ArchFeaturesFromMcpu(
+D, getAArch64TargetCPU(Args, Triple, A), Args, Features);
 
   if (success && (A = Args.getLastArg(clang::driver::options::OPT_mtune_EQ)))
 success =
@@ -172,9 +181,10 @@ void aarch64::getAArch64TargetFeatures(c
   else if (success && (A = Args.getLastArg(options::OPT_mcpu_EQ)))
 success =
 getAArch64MicroArchFeaturesFromMcpu(D, A->getValue(), Args, Features);
-  else if (success && Args.hasArg(options::OPT_arch))
+  else if (success &&
+   (Args.hasArg(options::OPT_arch) || isCPUDeterminedByTriple(Triple)))
 success = getAArch64MicroArchFeaturesFromMcpu(
-D, getAArch64TargetCPU(Args, A), Args, Features);
+D, getAArch64TargetCPU(Args, Triple, A), Args, Features);
 
   if (!success)
 D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);

Modified: cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.h?rev=349382&r1=349381&r2=349382&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/Arch/AArch64.h Mon Dec 17 11:30:46 2018
@@ -21,11 +21,12 @@ namespace driver {
 namespace tools {

[PATCH] D55731: [darwin][arm64] use the "cyclone" CPU for Darwin even when `-arch` is not specified

2018-12-17 Thread Alex Lorenz via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rC349382: [darwin][arm64] use the "cyclone" CPU for 
Darwin even when `-arch` (authored by arphaman, committed by ).

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55731/new/

https://reviews.llvm.org/D55731

Files:
  lib/Driver/ToolChains/Arch/AArch64.cpp
  lib/Driver/ToolChains/Arch/AArch64.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/CommonArgs.cpp
  test/Driver/aarch64-cpus.c

Index: test/Driver/aarch64-cpus.c
===
--- test/Driver/aarch64-cpus.c
+++ test/Driver/aarch64-cpus.c
@@ -21,7 +21,10 @@
 // ARM64-NATIVE-NOT: "-cc1"{{.*}} "-triple" "arm64{{.*}}" "-target-cpu" "native"
 
 // RUN: %clang -target arm64-apple-darwin -arch arm64 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-DARWIN %s
+// RUN: %clang -target arm64-apple-darwin -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-DARWIN %s
+// RUN: %clang -target arm64-apple-ios12.0 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-DARWIN %s
 // ARM64-DARWIN: "-cc1"{{.*}} "-triple" "arm64{{.*}}" "-target-cpu" "cyclone"
+// ARM64-DARWIN-SAME: "-target-feature" "+aes"
 
 // RUN: %clang -target aarch64 -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
 // RUN: %clang -target aarch64 -mlittle-endian -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -271,7 +271,7 @@
 
   case llvm::Triple::aarch64:
   case llvm::Triple::aarch64_be:
-return aarch64::getAArch64TargetCPU(Args, A);
+return aarch64::getAArch64TargetCPU(Args, T, A);
 
   case llvm::Triple::arm:
   case llvm::Triple::armeb:
Index: lib/Driver/ToolChains/Arch/AArch64.h
===
--- lib/Driver/ToolChains/Arch/AArch64.h
+++ lib/Driver/ToolChains/Arch/AArch64.h
@@ -21,11 +21,12 @@
 namespace tools {
 namespace aarch64 {
 
-void getAArch64TargetFeatures(const Driver &D, const llvm::opt::ArgList &Args,
+void getAArch64TargetFeatures(const Driver &D, const llvm::Triple &Triple,
+  const llvm::opt::ArgList &Args,
   std::vector &Features);
 
 std::string getAArch64TargetCPU(const llvm::opt::ArgList &Args,
-llvm::opt::Arg *&A);
+const llvm::Triple &Triple, llvm::opt::Arg *&A);
 
 } // end namespace aarch64
 } // end namespace target
Index: lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- lib/Driver/ToolChains/Arch/AArch64.cpp
+++ lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -19,10 +19,17 @@
 using namespace clang;
 using namespace llvm::opt;
 
+/// \returns true if the given triple can determine the default CPU type even
+/// if -arch is not specified.
+static bool isCPUDeterminedByTriple(const llvm::Triple &Triple) {
+  return Triple.isOSDarwin();
+}
+
 /// getAArch64TargetCPU - Get the (LLVM) name of the AArch64 cpu we are
 /// targeting. Set \p A to the Arg corresponding to the -mcpu argument if it is
 /// provided, or to nullptr otherwise.
-std::string aarch64::getAArch64TargetCPU(const ArgList &Args, Arg *&A) {
+std::string aarch64::getAArch64TargetCPU(const ArgList &Args,
+ const llvm::Triple &Triple, Arg *&A) {
   std::string CPU;
   // If we have -mcpu, use that.
   if ((A = Args.getLastArg(options::OPT_mcpu_EQ))) {
@@ -36,9 +43,9 @@
   else if (CPU.size())
 return CPU;
 
-  // Make sure we pick "cyclone" if -arch is used.
-  // FIXME: Should this be picked by checking the target triple instead?
-  if (Args.getLastArg(options::OPT_arch))
+  // Make sure we pick "cyclone" if -arch is used or when targetting a Darwin
+  // OS.
+  if (Args.getLastArg(options::OPT_arch) || Triple.isOSDarwin())
 return "cyclone";
 
   return "generic";
@@ -152,7 +159,9 @@
   return getAArch64MicroArchFeaturesFromMtune(D, CPU, Args, Features);
 }
 
-void aarch64::getAArch64TargetFeatures(const Driver &D, const ArgList &Args,
+void aarch64::getAArch64TargetFeatures(const Driver &D,
+   const llvm::Triple &Triple,
+   const ArgList &Args,
std::vector &Features) {
   Arg *A;
   bool success = true;
@@ -162,9 +171,9 @@
 success = getAArch64ArchFeaturesFromMarch(D, A->getValue(), Args, Features);
   else if ((A = Args.getLastArg(options::OPT_mcpu_EQ)))
 success = getAArch64ArchFeaturesFromMcpu(D, A->getValue(), Args, Features);
-  else if (Args.hasArg(options::OPT_arch))
-success = getAArch64ArchFeaturesFromMcpu(D, getAArch64TargetCPU(Args, A),
- Args, Fe

Re: r349212 - Mangle calling conventions into function pointer types where GCC does

2018-12-17 Thread Reid Kleckner via cfe-commits

I sent https://reviews.llvm.org/D55781 and cc'd the original reviewers so
we can decide if it's worth the cost.

On Fri, Dec 14, 2018 at 5:17 PM Richard Smith  wrote:

> Should this change be disableable by -fclang-abi-compat=7 or below?
>
> On Fri, 14 Dec 2018, 15:46 Reid Kleckner via cfe-commits <
> cfe-commits@lists.llvm.org wrote:
>
>> Author: rnk
>> Date: Fri Dec 14 15:42:59 2018
>> New Revision: 349212
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=349212&view=rev
>> Log:
>> Mangle calling conventions into function pointer types where GCC does
>>
>> Summary:
>> GCC 5.1 began mangling these Windows calling conventions into function
>> types, since they can be used for overloading. They've always been
>> mangled in the MS ABI, but they are new to the Itanium mangler. Note
>> that the calling convention doesn't appear as part of the main
>> declaration, it only appears on function parameter types and other
>> types.
>>
>> Fixes PR39860
>>
>> Reviewers: rjmccall, efriedma
>>
>> Subscribers: cfe-commits
>>
>> Differential Revision: https://reviews.llvm.org/D55672
>>
>> Added:
>> cfe/trunk/test/CodeGenCXX/mangle-win-ccs.cpp
>> cfe/trunk/test/CodeGenCXX/mangle-win64-ccs.cpp
>> Modified:
>> cfe/trunk/lib/AST/ItaniumMangle.cpp
>>
>> Modified: cfe/trunk/lib/AST/ItaniumMangle.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ItaniumMangle.cpp?rev=349212&r1=349211&r2=349212&view=diff
>>
>> ==
>> --- cfe/trunk/lib/AST/ItaniumMangle.cpp (original)
>> +++ cfe/trunk/lib/AST/ItaniumMangle.cpp Fri Dec 14 15:42:59 2018
>> @@ -2648,13 +2648,8 @@ StringRef CXXNameMangler::getCallingConv
>>case CC_C:
>>  return "";
>>
>> -  case CC_X86StdCall:
>> -  case CC_X86FastCall:
>> -  case CC_X86ThisCall:
>>case CC_X86VectorCall:
>>case CC_X86Pascal:
>> -  case CC_Win64:
>> -  case CC_X86_64SysV:
>>case CC_X86RegCall:
>>case CC_AAPCS:
>>case CC_AAPCS_VFP:
>> @@ -2667,6 +2662,16 @@ StringRef CXXNameMangler::getCallingConv
>>  // FIXME: we should be mangling all of the above.
>>  return "";
>>
>> +  case CC_X86StdCall:
>> +return "stdcall";
>> +  case CC_X86FastCall:
>> +return "fastcall";
>> +  case CC_X86ThisCall:
>> +return "thiscall";
>> +  case CC_X86_64SysV:
>> +return "sysv_abi";
>> +  case CC_Win64:
>> +return "ms_abi";
>>case CC_Swift:
>>  return "swiftcall";
>>}
>>
>> Added: cfe/trunk/test/CodeGenCXX/mangle-win-ccs.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/mangle-win-ccs.cpp?rev=349212&view=auto
>>
>> ==
>> --- cfe/trunk/test/CodeGenCXX/mangle-win-ccs.cpp (added)
>> +++ cfe/trunk/test/CodeGenCXX/mangle-win-ccs.cpp Fri Dec 14 15:42:59 2018
>> @@ -0,0 +1,61 @@
>> +// RUN: %clang_cc1 %s -emit-llvm -triple i686-windows-gnu -o - |
>> FileCheck %s
>> +// RUN: %clang_cc1 %s -emit-llvm -triple i686-windows-itanium -o - |
>> FileCheck %s
>> +
>> +// GCC 5.1 began mangling these Windows calling conventions into function
>> +// types, since they can be used for overloading. They've always been
>> mangled
>> +// in the MS ABI, but they are new to the Itanium mangler. Note that the
>> main
>> +// function definition does not use a calling convention. Only function
>> types
>> +// that appear later use it.
>> +
>> +template  static int func_as_ptr(Fn fn) { return int(fn); }
>> +
>> +void f_cdecl(int, int);
>> +void __attribute__((stdcall)) f_stdcall(int, int);
>> +void __attribute__((fastcall)) f_fastcall(int, int);
>> +void __attribute__((thiscall)) f_thiscall(int, int);
>> +
>> +int as_cdecl() { return func_as_ptr(f_cdecl); }
>> +int as_stdcall() { return func_as_ptr(f_stdcall); }
>> +int as_fastcall() { return func_as_ptr(f_fastcall); }
>> +int as_thiscall() { return func_as_ptr(f_thiscall); }
>> +
>> +// CHECK: define dso_local i32 @_Z8as_cdeclv()
>> +// CHECK:   call i32 @_ZL11func_as_ptrIPFviiEEiT_(void (i32, i32)*
>> @_Z7f_cdeclii)
>> +
>> +// CHECK: define dso_local i32 @_Z10as_stdcallv()
>> +// CHECK:   call i32 @_ZL11func_as_ptrIPU7stdcallFviiEEiT_(void (i32,
>> i32)* @"\01__Z9f_stdcallii@8")
>> +
>> +// CHECK: define dso_local i32 @_Z11as_fastcallv()
>> +// CHECK:   call i32 @_ZL11func_as_ptrIPU8fastcallFviiEEiT_(void (i32,
>> i32)* @"\01@_Z10f_fastcallii@8")
>> +
>> +// CHECK: define dso_local i32 @_Z11as_thiscallv()
>> +// CHECK:   call i32 @_ZL11func_as_ptrIPU8thiscallFviiEEiT_(void (i32,
>> i32)* @_Z10f_thiscallii)
>> +
>> +// CHECK: define dso_local void @_Z11funcRefTypeRU8fastcallFviiE(void
>> (i32, i32)* %fr)
>> +void funcRefType(void(__attribute__((fastcall)) & fr)(int, int)) {
>> +  fr(1, 2);
>> +}
>> +
>> +// CHECK: define dso_local void
>> @_Z12memptrCCTypeR3FooMS_U8fastcallFviiE(%struct.Foo* {{.*}}, { i32, i32 }*
>> byval{{.*}})
>> +struct Foo { void bar(int, int); };
>> +void memptrCCType(Foo &o, void (__attribute__((fastcall))

[PATCH] D55781: Make CC mangling conditional on the ABI version

2018-12-17 Thread Reid Kleckner via Phabricator via cfe-commits

rnk created this revision.
rnk added reviewers: rjmccall, rsmith.
Herald added a subscriber: krytarowski.

https://reviews.llvm.org/D55781

Files:
  clang/lib/AST/ItaniumMangle.cpp
  clang/test/CodeGenCXX/mangle-win-ccs-old.cpp

Index: clang/test/CodeGenCXX/mangle-win-ccs-old.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/mangle-win-ccs-old.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 %s -emit-llvm -triple i686-windows-gnu -o - | FileCheck %s --check-prefix=MINGW32
+// RUN: %clang_cc1 %s -emit-llvm -triple i686-windows-gnu -o - -fclang-abi-compat=7 | FileCheck %s --check-prefix=MINGW32-OLD
+// RUN: %clang_cc1 %s -emit-llvm -triple x86_64-windows-gnu -o - | FileCheck %s --check-prefix=MINGW64
+// RUN: %clang_cc1 %s -emit-llvm -triple x86_64-windows-gnu -o - -fclang-abi-compat=7 | FileCheck %s --check-prefix=MINGW64-OLD
+// RUN: %clang_cc1 %s -emit-llvm -triple x86_64-linux-gnu -o - | FileCheck %s --check-prefix=LINUX
+// RUN: %clang_cc1 %s -emit-llvm -triple x86_64-linux-gnu -o - -fclang-abi-compat=7 | FileCheck %s --check-prefix=LINUX-OLD
+
+#if defined(__i386__) && defined(__MINGW32__)
+void cb_cdecl   (void (  *cb)(int)) { cb(42); }
+void cb_stdcall (void (__attribute__((stdcall )) *cb)(int)) { cb(42); }
+void cb_fastcall(void (__attribute__((fastcall)) *cb)(int)) { cb(42); }
+void cb_thiscall(void (__attribute__((thiscall)) *cb)(int)) { cb(42); }
+
+// MINGW32: define dso_local void @_Z8cb_cdeclPFviE(void (i32)* %cb)
+// MINGW32: define dso_local void @_Z10cb_stdcallPU7stdcallFviE(void (i32)* %cb)
+// MINGW32: define dso_local void @_Z11cb_fastcallPU8fastcallFviE(void (i32)* %cb)
+// MINGW32: define dso_local void @_Z11cb_thiscallPU8thiscallFviE(void (i32)* %cb)
+
+// MINGW32-OLD: define dso_local void @_Z8cb_cdeclPFviE(void (i32)* %cb)
+// MINGW32-OLD: define dso_local void @_Z10cb_stdcallPFviE(void (i32)* %cb)
+// MINGW32-OLD: define dso_local void @_Z11cb_fastcallPFviE(void (i32)* %cb)
+// MINGW32-OLD: define dso_local void @_Z11cb_thiscallPFviE(void (i32)* %cb)
+#endif
+
+#if defined(__x86_64__) && defined(__MINGW32__)
+void cb_sysvabi(void (__attribute__((sysv_abi)) *cb)(int)) { cb(42); }
+void cb_msabi  (void (__attribute__((ms_abi  )) *cb)(int)) { cb(42); }
+
+// MINGW64: define dso_local void @_Z10cb_sysvabiPU8sysv_abiFviE(void (i32)* %cb)
+// MINGW64: define dso_local void @_Z8cb_msabiPFviE(void (i32)* %cb)
+
+// MINGW64-OLD: define dso_local void @_Z10cb_sysvabiPFviE(void (i32)* %cb)
+// MINGW64-OLD: define dso_local void @_Z8cb_msabiPFviE(void (i32)* %cb)
+#endif
+
+#if defined(__x86_64__) && defined(__linux__)
+void cb_sysvabi(void (__attribute__((sysv_abi)) *cb)(int)) { cb(42); }
+void cb_msabi  (void (__attribute__((ms_abi  )) *cb)(int)) { cb(42); }
+
+// LINUX: define void @_Z10cb_sysvabiPFviE(void (i32)* %cb)
+// LINUX: define void @_Z8cb_msabiPU6ms_abiFviE(void (i32)* %cb)
+
+// LINUX-OLD: define void @_Z10cb_sysvabiPFviE(void (i32)* %cb)
+// LINUX-OLD: define void @_Z8cb_msabiPFviE(void (i32)* %cb)
+#endif
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -516,7 +516,9 @@
 
   void mangleType(const TagType*);
   void mangleType(TemplateName);
-  static StringRef getCallingConvQualifierName(CallingConv CC);
+  StringRef getCallingConvQualifierName(CallingConv CC);
+  StringRef maybeQualifyCallingConv(StringRef CCQual,
+LangOptions::ClangABI PrevVer);
   void mangleExtParameterInfo(FunctionProtoType::ExtParameterInfo info);
   void mangleExtFunctionInfo(const FunctionType *T);
   void mangleBareFunctionType(const FunctionProtoType *T, bool MangleReturnType,
@@ -2643,6 +2645,15 @@
   }
 }
 
+/// Use a vendor extension qualifier for this calling convention if the current
+/// version is after the version before this mangling was introduced in clang.
+StringRef
+CXXNameMangler::maybeQualifyCallingConv(StringRef CCQual,
+LangOptions::ClangABI PrevVer) {
+  auto CurrentABIVersion = getASTContext().getLangOpts().getClangABICompat();
+  return (CurrentABIVersion > PrevVer) ? CCQual : "";
+}
+
 StringRef CXXNameMangler::getCallingConvQualifierName(CallingConv CC) {
   switch (CC) {
   case CC_C:
@@ -2663,15 +2674,16 @@
 return "";
 
   case CC_X86StdCall:
-return "stdcall";
+return maybeQualifyCallingConv("stdcall", LangOptions::ClangABI::Ver7);
   case CC_X86FastCall:
-return "fastcall";
+return maybeQualifyCallingConv("fastcall", LangOptions::ClangABI::Ver7);
   case CC_X86ThisCall:
-return "thiscall";
+return maybeQualifyCallingConv("thiscall", LangOptions::ClangABI::Ver7);
   case CC_X86_64SysV:
-return "sysv_abi";
+return maybeQualifyCallingConv("sysv_abi", LangOptions::ClangABI::Ver7);
   case CC_Win64:
-return "ms_abi";
+return maybeQualifyCallingConv("ms_ab

[PATCH] D35783: Ignore shadowing for declarations coming from within macros.

2018-12-17 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added a comment.

I think the ideal solution would be something like:

If a variable produced by a macro expansion shadows a variable not produced by 
a macro expansion, do not warn immediately. Instead, warn if we see a use of 
the inner variable in a context that is not produced by the same macro 
expansion (more broadly: for each point of use, if there is no macro expansion 
that contains both the point of declaration of the inner variable and all 
points of use, then warn). As an approximation to this, we could suppress the 
warning only if the (start of the) scope containing the inner variable and the 
inner variable itself were produced by the same macro expansion.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D35783/new/

https://reviews.llvm.org/D35783



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

[PATCH] D55781: Make CC mangling conditional on the ABI version

2018-12-17 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added a comment.

@rsmith asked if we should make this mangling conditional on the ABI version. I 
implemented it here, let me know if you think it's worth the extra code 
complexity. Our mingw ABI hasn't been particularly stable, but I think going 
forward we'll want to start mangling new calling convention attributes 
(regcall, maybe?) this way, and we'll want to make it conditional on the ABI 
version using something like this code structure.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55781/new/

https://reviews.llvm.org/D55781



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

[PATCH] D55765: Fix use-after-free bug in Tooling.

2018-12-17 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits

ymandel updated this revision to Diff 178507.
ymandel added a comment.

Update test that was calling builAST with an actual Twine.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55765/new/

https://reviews.llvm.org/D55765

Files:
  include/clang/Tooling/Tooling.h
  lib/Tooling/Tooling.cpp
  unittests/Analysis/ExprMutationAnalyzerTest.cpp


Index: unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -11,6 +11,7 @@
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/SmallString.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
@@ -32,7 +33,9 @@
 std::unique_ptr
 buildASTFromCodeWithArgs(const Twine &Code,
  const std::vector &Args) {
-  auto AST = tooling::buildASTFromCodeWithArgs(Code, Args);
+  SmallString<1024> CodeStorage;
+  auto AST =
+  tooling::buildASTFromCodeWithArgs(Code.toStringRef(CodeStorage), Args);
   EXPECT_FALSE(AST->getDiagnostics().hasErrorOccurred());
   return AST;
 }
Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -574,15 +574,15 @@
 namespace tooling {
 
 std::unique_ptr
-buildASTFromCode(const Twine &Code, const Twine &FileName,
+buildASTFromCode(StringRef Code, const Twine &FileName,
  std::shared_ptr PCHContainerOps) {
   return buildASTFromCodeWithArgs(Code, std::vector(), FileName,
   "clang-tool", std::move(PCHContainerOps));
 }
 
 std::unique_ptr buildASTFromCodeWithArgs(
-const Twine &Code, const std::vector &Args,
-const Twine &FileName, const Twine &ToolName,
+StringRef Code, const std::vector &Args, const Twine 
&FileName,
+const Twine &ToolName,
 std::shared_ptr PCHContainerOps,
 ArgumentsAdjuster Adjuster) {
   SmallString<16> FileNameStorage;
@@ -602,10 +602,8 @@
   getSyntaxOnlyToolArgs(ToolName, Adjuster(Args, FileNameRef), 
FileNameRef),
   &Action, Files.get(), std::move(PCHContainerOps));
 
-  SmallString<1024> CodeStorage;
   InMemoryFileSystem->addFile(FileNameRef, 0,
-  llvm::MemoryBuffer::getMemBuffer(
-  
Code.toNullTerminatedStringRef(CodeStorage)));
+  llvm::MemoryBuffer::getMemBufferCopy(Code));
   if (!Invocation.run())
 return nullptr;
 
Index: include/clang/Tooling/Tooling.h
===
--- include/clang/Tooling/Tooling.h
+++ include/clang/Tooling/Tooling.h
@@ -205,7 +205,7 @@
 ///
 /// \return The resulting AST or null if an error occurred.
 std::unique_ptr
-buildASTFromCode(const Twine &Code, const Twine &FileName = "input.cc",
+buildASTFromCode(StringRef Code, const Twine &FileName = "input.cc",
  std::shared_ptr PCHContainerOps =
  std::make_shared());
 
@@ -223,7 +223,7 @@
 ///
 /// \return The resulting AST or null if an error occurred.
 std::unique_ptr buildASTFromCodeWithArgs(
-const Twine &Code, const std::vector &Args,
+StringRef Code, const std::vector &Args,
 const Twine &FileName = "input.cc", const Twine &ToolName = "clang-tool",
 std::shared_ptr PCHContainerOps =
   std::make_shared(),


Index: unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -11,6 +11,7 @@
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/SmallString.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
@@ -32,7 +33,9 @@
 std::unique_ptr
 buildASTFromCodeWithArgs(const Twine &Code,
  const std::vector &Args) {
-  auto AST = tooling::buildASTFromCodeWithArgs(Code, Args);
+  SmallString<1024> CodeStorage;
+  auto AST =
+  tooling::buildASTFromCodeWithArgs(Code.toStringRef(CodeStorage), Args);
   EXPECT_FALSE(AST->getDiagnostics().hasErrorOccurred());
   return AST;
 }
Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -574,15 +574,15 @@
 namespace tooling {
 
 std::unique_ptr
-buildASTFromCode(const Twine &Code, const Twine &FileName,
+buildASTFromCode(StringRef Code, const Twine &FileName,
  std::shared_ptr PCHContainerOps) {
   return buildASTFromCodeWithArgs(Code, std::vector(), FileName,
   "clang-tool", std::move(PCHContainerOps));
 }
 
 std::unique_ptr buildASTFromCodeWithArgs(
-const T

[PATCH] D55662: [Sema][ObjC] Do not warn about repeated uses of weak variables when the variables are accessed in an unevaluated context.

2018-12-17 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

Do you have an example, and is there a reasonable place to call 
`CheckPlaceholderExpr` on those paths?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55662/new/

https://reviews.llvm.org/D55662



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

r349384 - [NFC] Test commit: tweak whitespace in comment

2018-12-17 Thread Tan S. B. via cfe-commits

Author: cpplearner
Date: Mon Dec 17 11:53:22 2018
New Revision: 349384

URL: http://llvm.org/viewvc/llvm-project?rev=349384&view=rev
Log:
[NFC] Test commit: tweak whitespace in comment

Modified:
cfe/trunk/test/CXX/temp/temp.param/p3.cpp

Modified: cfe/trunk/test/CXX/temp/temp.param/p3.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/temp/temp.param/p3.cpp?rev=349384&r1=349383&r2=349384&view=diff
==
--- cfe/trunk/test/CXX/temp/temp.param/p3.cpp (original)
+++ cfe/trunk/test/CXX/temp/temp.param/p3.cpp Mon Dec 17 11:53:22 2018
@@ -16,7 +16,7 @@ template class Y> stru
 // [Note: because of the name lookup rules, a template-parameter that
 // could be interpreted as either a non-type template-parameter or a
 // type-parameter (because its identifier is the name of an already
-// existing class) is taken as a type-parameter. For example, 
+// existing class) is taken as a type-parameter. For example,
 class T { /* ... */ };  // expected-note{{candidate constructor (the implicit 
copy constructor) not viable}}
 #if __cplusplus >= 201103L // C++11 or later
 // expected-note@-2 {{candidate constructor (the implicit move constructor) 
not viable}}
@@ -27,7 +27,7 @@ int i;
 template struct X2 {
   void f(T t) 
   { 
-T t1 = i; //template-parameters T and i 
+T t1 = i; // template-parameters T and i
 ::T t2 = ::i; // global namespace members T and i  \
 // expected-error{{no viable conversion}}
   } 


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

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

2018-12-17 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

In D53738#1333276 , @leonardchan wrote:

> In D53738#1326071 , @rjmccall wrote:
>
> > I'm fine with making this change under the assumption that we've gotten the 
> > language rule right.  Even if that weren't abstractly reasonable for 
> > general language work — and I do think it's reasonable when we have a 
> > good-faith question about the right semantics — this is clearly still an 
> > experimental implementation and will be for several months yet, and 
> > hopefully it won't take that long for us to get a response.
>
>
> @rjmccall Have you received a response yet? If not, do you think you have an 
> estimate on the response time, or also mind sharing the contact information 
> if that's ok?


I just have a coworker who's part of the committee.  I think you might be 
over-opimistic about how quickly things get answered with the C committee, 
though.  We should not allow our work to be blocked waiting for a response.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53738/new/

https://reviews.llvm.org/D53738



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

1 2 >

1 - 100 of 148 matches

Mail list logo