r320135 - In stdbool.h, define bool, false, true only in gnu++98

[Stephan Bergmann via cfe-commits]

Author: sberg
Date: Fri Dec  8 00:28:08 2017
New Revision: 320135

URL: http://llvm.org/viewvc/llvm-project?rev=320135&view=rev
Log:
In stdbool.h, define bool, false, true only in gnu++98

GCC has meanwhile corrected that with the similar
 "C++11
explicitly forbids macros for bool, true and false."

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

Modified:
cfe/trunk/lib/Headers/stdbool.h
cfe/trunk/test/Headers/stdbool.cpp

Modified: cfe/trunk/lib/Headers/stdbool.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/stdbool.h?rev=320135&r1=320134&r2=320135&view=diff
==
--- cfe/trunk/lib/Headers/stdbool.h (original)
+++ cfe/trunk/lib/Headers/stdbool.h Fri Dec  8 00:28:08 2017
@@ -32,12 +32,15 @@
 #define true 1
 #define false 0
 #elif defined(__GNUC__) && !defined(__STRICT_ANSI__)
-/* Define _Bool, bool, false, true as a GNU extension. */
+/* Define _Bool as a GNU extension. */
 #define _Bool bool
+#if __cplusplus < 201103L
+/* For C++98, define bool, false, true as a GNU extension. */
 #define bool  bool
 #define false false
 #define true  true
 #endif
+#endif
 
 #define __bool_true_false_are_defined 1
 

Modified: cfe/trunk/test/Headers/stdbool.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/stdbool.cpp?rev=320135&r1=320134&r2=320135&view=diff
==
--- cfe/trunk/test/Headers/stdbool.cpp (original)
+++ cfe/trunk/test/Headers/stdbool.cpp Fri Dec  8 00:28:08 2017
@@ -1,13 +1,19 @@
-// RUN: %clang_cc1 -E -dM %s | FileCheck --check-prefix=CHECK-GNU-COMPAT %s
+// RUN: %clang_cc1 -std=gnu++98 -E -dM %s | FileCheck 
--check-prefix=CHECK-GNU-COMPAT-98 %s
+// RUN: %clang_cc1 -std=gnu++11 -E -dM %s | FileCheck 
--check-prefix=CHECK-GNU-COMPAT-11 %s
 // RUN: %clang_cc1 -std=c++98 -E -dM %s | FileCheck 
--check-prefix=CHECK-CONFORMING %s
 // RUN: %clang_cc1 -fsyntax-only -std=gnu++98 -verify -Weverything %s
 #include 
 #define zzz
 
-// CHECK-GNU-COMPAT: #define _Bool bool
-// CHECK-GNU-COMPAT: #define bool bool
-// CHECK-GNU-COMPAT: #define false false
-// CHECK-GNU-COMPAT: #define true true
+// CHECK-GNU-COMPAT-98: #define _Bool bool
+// CHECK-GNU-COMPAT-98: #define bool bool
+// CHECK-GNU-COMPAT-98: #define false false
+// CHECK-GNU-COMPAT-98: #define true true
+
+// CHECK-GNU-COMPAT-11: #define _Bool bool
+// CHECK-GNU-COMPAT-11-NOT: #define bool bool
+// CHECK-GNU-COMPAT-11-NOT: #define false false
+// CHECK-GNU-COMPAT-11-NOT: #define true true
 
 // CHECK-CONFORMING-NOT: #define _Bool
 // CHECK-CONFORMING: #define __CHAR_BIT__


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

[PATCH] D40167: In stdbool.h, define bool, false, true only in gnu++98

[Stephan Bergmann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC320135: In stdbool.h, define bool, false, true only in 
gnu++98 (authored by sberg).

Changed prior to commit:
  https://reviews.llvm.org/D40167?vs=123296&id=126095#toc

Repository:
  rC Clang

https://reviews.llvm.org/D40167

Files:
  lib/Headers/stdbool.h
  test/Headers/stdbool.cpp


Index: test/Headers/stdbool.cpp
===
--- test/Headers/stdbool.cpp
+++ test/Headers/stdbool.cpp
@@ -1,13 +1,19 @@
-// RUN: %clang_cc1 -E -dM %s | FileCheck --check-prefix=CHECK-GNU-COMPAT %s
+// RUN: %clang_cc1 -std=gnu++98 -E -dM %s | FileCheck 
--check-prefix=CHECK-GNU-COMPAT-98 %s
+// RUN: %clang_cc1 -std=gnu++11 -E -dM %s | FileCheck 
--check-prefix=CHECK-GNU-COMPAT-11 %s
 // RUN: %clang_cc1 -std=c++98 -E -dM %s | FileCheck 
--check-prefix=CHECK-CONFORMING %s
 // RUN: %clang_cc1 -fsyntax-only -std=gnu++98 -verify -Weverything %s
 #include 
 #define zzz
 
-// CHECK-GNU-COMPAT: #define _Bool bool
-// CHECK-GNU-COMPAT: #define bool bool
-// CHECK-GNU-COMPAT: #define false false
-// CHECK-GNU-COMPAT: #define true true
+// CHECK-GNU-COMPAT-98: #define _Bool bool
+// CHECK-GNU-COMPAT-98: #define bool bool
+// CHECK-GNU-COMPAT-98: #define false false
+// CHECK-GNU-COMPAT-98: #define true true
+
+// CHECK-GNU-COMPAT-11: #define _Bool bool
+// CHECK-GNU-COMPAT-11-NOT: #define bool bool
+// CHECK-GNU-COMPAT-11-NOT: #define false false
+// CHECK-GNU-COMPAT-11-NOT: #define true true
 
 // CHECK-CONFORMING-NOT: #define _Bool
 // CHECK-CONFORMING: #define __CHAR_BIT__
Index: lib/Headers/stdbool.h
===
--- lib/Headers/stdbool.h
+++ lib/Headers/stdbool.h
@@ -32,12 +32,15 @@
 #define true 1
 #define false 0
 #elif defined(__GNUC__) && !defined(__STRICT_ANSI__)
-/* Define _Bool, bool, false, true as a GNU extension. */
+/* Define _Bool as a GNU extension. */
 #define _Bool bool
+#if __cplusplus < 201103L
+/* For C++98, define bool, false, true as a GNU extension. */
 #define bool  bool
 #define false false
 #define true  true
 #endif
+#endif
 
 #define __bool_true_false_are_defined 1
 


Index: test/Headers/stdbool.cpp
===
--- test/Headers/stdbool.cpp
+++ test/Headers/stdbool.cpp
@@ -1,13 +1,19 @@
-// RUN: %clang_cc1 -E -dM %s | FileCheck --check-prefix=CHECK-GNU-COMPAT %s
+// RUN: %clang_cc1 -std=gnu++98 -E -dM %s | FileCheck --check-prefix=CHECK-GNU-COMPAT-98 %s
+// RUN: %clang_cc1 -std=gnu++11 -E -dM %s | FileCheck --check-prefix=CHECK-GNU-COMPAT-11 %s
 // RUN: %clang_cc1 -std=c++98 -E -dM %s | FileCheck --check-prefix=CHECK-CONFORMING %s
 // RUN: %clang_cc1 -fsyntax-only -std=gnu++98 -verify -Weverything %s
 #include 
 #define zzz
 
-// CHECK-GNU-COMPAT: #define _Bool bool
-// CHECK-GNU-COMPAT: #define bool bool
-// CHECK-GNU-COMPAT: #define false false
-// CHECK-GNU-COMPAT: #define true true
+// CHECK-GNU-COMPAT-98: #define _Bool bool
+// CHECK-GNU-COMPAT-98: #define bool bool
+// CHECK-GNU-COMPAT-98: #define false false
+// CHECK-GNU-COMPAT-98: #define true true
+
+// CHECK-GNU-COMPAT-11: #define _Bool bool
+// CHECK-GNU-COMPAT-11-NOT: #define bool bool
+// CHECK-GNU-COMPAT-11-NOT: #define false false
+// CHECK-GNU-COMPAT-11-NOT: #define true true
 
 // CHECK-CONFORMING-NOT: #define _Bool
 // CHECK-CONFORMING: #define __CHAR_BIT__
Index: lib/Headers/stdbool.h
===
--- lib/Headers/stdbool.h
+++ lib/Headers/stdbool.h
@@ -32,12 +32,15 @@
 #define true 1
 #define false 0
 #elif defined(__GNUC__) && !defined(__STRICT_ANSI__)
-/* Define _Bool, bool, false, true as a GNU extension. */
+/* Define _Bool as a GNU extension. */
 #define _Bool bool
+#if __cplusplus < 201103L
+/* For C++98, define bool, false, true as a GNU extension. */
 #define bool  bool
 #define false false
 #define true  true
 #endif
+#endif
 
 #define __bool_true_false_are_defined 1
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41001: [change-namespace] Fix crash when injected base-class name is used in friend declarations.

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
Herald added subscribers: cfe-commits, klimek.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41001

Files:
  change-namespace/ChangeNamespace.cpp
  unittests/change-namespace/ChangeNamespaceTests.cpp


Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- unittests/change-namespace/ChangeNamespaceTests.cpp
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -2154,6 +2154,60 @@
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+TEST_F(ChangeNamespaceTest, InjectedClassNameInFriendDecl) {
+  OldNamespace = "d";
+  NewNamespace = "e";
+  std::string Code = "namespace a{\n"
+ "template \n"
+ "class Base {\n"
+ " public:\n"
+ "  void f() {\n"
+ "T t;\n"
+ "t.priv();\n"
+ "  }\n"
+ "};\n"
+ "}  // namespace a\n"
+ "namespace d {\n"
+ "class D : public a::Base {\n"
+ " private:\n"
+ "  friend class Base;\n"
+ "  void priv() {}\n"
+ "  Base b;\n"
+ "};\n"
+ "\n"
+ "void f() {\n"
+ "  D d;\n"
+ "  a:: Base b;\n"
+ "  b.f();\n"
+ "}\n"
+ "}  // namespace d\n";
+  std::string Expected = "namespace a{\n"
+ "template \n"
+ "class Base {\n"
+ " public:\n"
+ "  void f() {\n"
+ "T t;\n"
+ "t.priv();\n"
+ "  }\n"
+ "};\n"
+ "}  // namespace a\n"
+ "\n"
+ "namespace e {\n"
+ "class D : public a::Base {\n"
+ " private:\n"
+ "  friend class Base;\n"
+ "  void priv() {}\n"
+ "  a::Base b;\n"
+ "};\n"
+ "\n"
+ "void f() {\n"
+ "  D d;\n"
+ "  a::Base b;\n"
+ "  b.f();\n"
+ "}\n"
+ "}  // namespace e\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
 
 } // anonymous namespace
 } // namespace change_namespace
Index: change-namespace/ChangeNamespace.cpp
===
--- change-namespace/ChangeNamespace.cpp
+++ change-namespace/ChangeNamespace.cpp
@@ -552,6 +552,10 @@
 if (Loc.getTypeLocClass() == TypeLoc::Elaborated) {
   NestedNameSpecifierLoc NestedNameSpecifier =
   Loc.castAs().getQualifierLoc();
+  // This happens for friend declaration of a base class with injected 
class
+  // name.
+  if (!NestedNameSpecifier.getNestedNameSpecifier())
+return;
   const Type *SpecifierType =
   NestedNameSpecifier.getNestedNameSpecifier()->getAsType();
   if (SpecifierType && SpecifierType->isRecordType())


Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- unittests/change-namespace/ChangeNamespaceTests.cpp
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -2154,6 +2154,60 @@
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+TEST_F(ChangeNamespaceTest, InjectedClassNameInFriendDecl) {
+  OldNamespace = "d";
+  NewNamespace = "e";
+  std::string Code = "namespace a{\n"
+ "template \n"
+ "class Base {\n"
+ " public:\n"
+ "  void f() {\n"
+ "T t;\n"
+ "t.priv();\n"
+ "  }\n"
+ "};\n"
+ "}  // namespace a\n"
+ "namespace d {\n"
+ "class D : public a::Base {\n"
+ " private:\n"
+ "  friend class Base;\n"
+ "  void priv() {}\n"
+ "  Base b;\n"
+ "};\n"
+ "\n"
+ "void f() {\n"
+ "  D d;\n"
+ "  a:: Base b;\n"
+ "  b.f();\n"
+ "}\n"
+ "}  // namespace d\n";
+  std::string Expected = "namespace a{\n"
+ "template \n"
+ "class Base {\n"
+ " public:\n"
+ "  void f() {\n"
+

[PATCH] D40909: [clang-format] Reorganize raw string delimiters

[Daniel Jasper via Phabricator via cfe-commits]

djasper added inline comments.



Comment at: include/clang/Format/Format.h:1375
+std::vector EnclosingFunctionNames;
+/// \brief The canonical delimiter for this language.
+std::string CanonicalDelimiter;

krasimir wrote:
> djasper wrote:
> > Can you pull apart this patch?
> > 
> > In my view, it has three parts that have an ordering, but are actually 
> > fairly independent:
> > 
> > 1. Propagate all configured languages to the formatting library. First 
> > patch to land, should not affect the visible behavior.
> > 2. Restructure RawStringFormat to be centered around each language. This is 
> > a restructuring to make things easier and use #1.
> > 3. Add a CanonicalDelimiter and make clang-format canonicalize it.
> > 
> > I'll focus my comments on what's required for #1 for now as that is already 
> > complicated (IMO).
> I believe these should all go together: the reason that we propagate all 
> configured languages to the formatting library is to be able to use them as a 
> replacement for the BasedOnStyle in RawStringFormat. To make this possible, 
> we need to update the internal structure of RawStringFormat itself to base it 
> around each language. The canonical delimiter part is just a convenience for 
> this I guess, which could be split.
> 
> My biggest concern with (1) is that since it has no visible behavior and no 
> other uses except for the adaptation of (2), it is not testable by itself and 
> it's not evident that a patch doing just (1) would handle the things 
> correctly.
Ok, if you wish, this is not an unreasonable argument. But let's still do the 
code review in two steps: Lets for now just get the part of handling multiple 
languages straight and figure out the rest once we are sure that that part is 
fine.

(I do think you can test it, though - but it depends on whether I can convince 
you to go with the FormatStyleSet approach ;) )



Comment at: lib/Format/Format.cpp:920
+  if (LanguageFound) {
+for (int i = Styles.size() - 1; i >= 0; --i) {
+  if (Styles[i].Language == FormatStyle::LK_None) {

krasimir wrote:
> djasper wrote:
> > I think this is getting a bit convoluted and I don't even understand 
> > whether we are doing what is document (even before this patch).
> > 
> > So in lines 892-905, we verify that:
> > - Only the first Style in the file is allowed be LK_None.
> > - No language is duplicated.
> > 
> > That seems good.
> > 
> > According to the documentation: "The first section may have no language 
> > set, it will set the default style options for all lanugages.". Does the 
> > latter part actually happen? Seems to me that we are just setting "Style" 
> > to the style configured for a specific language, completely ignoring values 
> > that might have been set in the LK_None style. Or is that somehow happening 
> > when reading the JSON?
> > 
> > Independent of that, I think we should use this structure more explicitly. 
> > I think we should create an additional class (FormatStyles or 
> > FormatStyleSet or something) that is returned by this function and handed 
> > to the formatting library. This function then doesn't need to look at the 
> > language anymore.
> > 
> > That class should then have a function getFormatStyle(LanguageKind 
> > Language); that returns the style for a particular language (doing the 
> > default logic, etc.). Internally, it can likely just have a map 
> > and I don't think you need to pre-fill that for all language kinds. If a 
> > language kind is not in the map, you can just return what's stored for 
> > LK_None. WDYT?
> Yes, defaulting to the None for missing language specifications is handled at 
> line 912:
> ```
> if (!LanguageFound && (Styles[i].Language == Language ||
>Styles[i].Language == FormatStyle::LK_None
> ```
> 
> I was thinking of the FormatStyleSet approach but the problem is that this 
> has repercussions all over the library. We could indeed update this specific 
> function that way, but fundamentally the additional language styles are part 
> of the FormatStyle and need to somehow be recorded inside there. That's why I 
> went with KISS and directly made this function handle that case.
But it's not just defaulting to LK_None what we are saying we are implementing. 
I think the documentation suggestion that we implement very basic inheritance. 
E.g. if the style for LK_None set the ColumnLimit to 42, I would expect that 
the styles for all other languages that don't explicitly set a ColumnLimit 
would also use 42. I don't think this is currently implemented and I don't 
think this patch implements it. But I think we should :).

I agree that the FormatStyleSet approach would have some consequences, but I 
also think that it is much cleaner. Your current solution feels like we us 
working around technical debt and creating more technical debt to do it :(. 
Maybe Manuel has thoughts here?


Repository:
  rC 

[PATCH] D41001: [change-namespace] Fix crash when injected base-class name is used in friend declarations.

[Haojian Wu via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41001



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

[clang-tools-extra] r320139 - [change-namespace] Fix crash when injected base-class name is used in friend declarations.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Fri Dec  8 02:06:16 2017
New Revision: 320139

URL: http://llvm.org/viewvc/llvm-project?rev=320139&view=rev
Log:
[change-namespace] Fix crash when injected base-class name is used in friend 
declarations.

Reviewers: hokein

Subscribers: klimek, cfe-commits

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

Modified:
clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp

Modified: clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp?rev=320139&r1=320138&r2=320139&view=diff
==
--- clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp (original)
+++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp Fri Dec  8 
02:06:16 2017
@@ -552,6 +552,10 @@ void ChangeNamespaceTool::run(
 if (Loc.getTypeLocClass() == TypeLoc::Elaborated) {
   NestedNameSpecifierLoc NestedNameSpecifier =
   Loc.castAs().getQualifierLoc();
+  // This happens for friend declaration of a base class with injected 
class
+  // name.
+  if (!NestedNameSpecifier.getNestedNameSpecifier())
+return;
   const Type *SpecifierType =
   NestedNameSpecifier.getNestedNameSpecifier()->getAsType();
   if (SpecifierType && SpecifierType->isRecordType())

Modified: 
clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp?rev=320139&r1=320138&r2=320139&view=diff
==
--- clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp 
(original)
+++ clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp 
Fri Dec  8 02:06:16 2017
@@ -2154,6 +2154,60 @@ TEST_F(ChangeNamespaceTest, DefaultMoveC
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+TEST_F(ChangeNamespaceTest, InjectedClassNameInFriendDecl) {
+  OldNamespace = "d";
+  NewNamespace = "e";
+  std::string Code = "namespace a{\n"
+ "template \n"
+ "class Base {\n"
+ " public:\n"
+ "  void f() {\n"
+ "T t;\n"
+ "t.priv();\n"
+ "  }\n"
+ "};\n"
+ "}  // namespace a\n"
+ "namespace d {\n"
+ "class D : public a::Base {\n"
+ " private:\n"
+ "  friend class Base;\n"
+ "  void priv() {}\n"
+ "  Base b;\n"
+ "};\n"
+ "\n"
+ "void f() {\n"
+ "  D d;\n"
+ "  a:: Base b;\n"
+ "  b.f();\n"
+ "}\n"
+ "}  // namespace d\n";
+  std::string Expected = "namespace a{\n"
+ "template \n"
+ "class Base {\n"
+ " public:\n"
+ "  void f() {\n"
+ "T t;\n"
+ "t.priv();\n"
+ "  }\n"
+ "};\n"
+ "}  // namespace a\n"
+ "\n"
+ "namespace e {\n"
+ "class D : public a::Base {\n"
+ " private:\n"
+ "  friend class Base;\n"
+ "  void priv() {}\n"
+ "  a::Base b;\n"
+ "};\n"
+ "\n"
+ "void f() {\n"
+ "  D d;\n"
+ "  a::Base b;\n"
+ "  b.f();\n"
+ "}\n"
+ "}  // namespace e\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
 
 } // anonymous namespace
 } // namespace change_namespace


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

[PATCH] D41001: [change-namespace] Fix crash when injected base-class name is used in friend declarations.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL320139: [change-namespace] Fix crash when injected 
base-class name is used in friend… (authored by ioeric).

Repository:
  rL LLVM

https://reviews.llvm.org/D41001

Files:
  clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
  clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp


Index: clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
===
--- clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
+++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
@@ -552,6 +552,10 @@
 if (Loc.getTypeLocClass() == TypeLoc::Elaborated) {
   NestedNameSpecifierLoc NestedNameSpecifier =
   Loc.castAs().getQualifierLoc();
+  // This happens for friend declaration of a base class with injected 
class
+  // name.
+  if (!NestedNameSpecifier.getNestedNameSpecifier())
+return;
   const Type *SpecifierType =
   NestedNameSpecifier.getNestedNameSpecifier()->getAsType();
   if (SpecifierType && SpecifierType->isRecordType())
Index: 
clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
+++ clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -2154,6 +2154,60 @@
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+TEST_F(ChangeNamespaceTest, InjectedClassNameInFriendDecl) {
+  OldNamespace = "d";
+  NewNamespace = "e";
+  std::string Code = "namespace a{\n"
+ "template \n"
+ "class Base {\n"
+ " public:\n"
+ "  void f() {\n"
+ "T t;\n"
+ "t.priv();\n"
+ "  }\n"
+ "};\n"
+ "}  // namespace a\n"
+ "namespace d {\n"
+ "class D : public a::Base {\n"
+ " private:\n"
+ "  friend class Base;\n"
+ "  void priv() {}\n"
+ "  Base b;\n"
+ "};\n"
+ "\n"
+ "void f() {\n"
+ "  D d;\n"
+ "  a:: Base b;\n"
+ "  b.f();\n"
+ "}\n"
+ "}  // namespace d\n";
+  std::string Expected = "namespace a{\n"
+ "template \n"
+ "class Base {\n"
+ " public:\n"
+ "  void f() {\n"
+ "T t;\n"
+ "t.priv();\n"
+ "  }\n"
+ "};\n"
+ "}  // namespace a\n"
+ "\n"
+ "namespace e {\n"
+ "class D : public a::Base {\n"
+ " private:\n"
+ "  friend class Base;\n"
+ "  void priv() {}\n"
+ "  a::Base b;\n"
+ "};\n"
+ "\n"
+ "void f() {\n"
+ "  D d;\n"
+ "  a::Base b;\n"
+ "  b.f();\n"
+ "}\n"
+ "}  // namespace e\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
 
 } // anonymous namespace
 } // namespace change_namespace


Index: clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
===
--- clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
+++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
@@ -552,6 +552,10 @@
 if (Loc.getTypeLocClass() == TypeLoc::Elaborated) {
   NestedNameSpecifierLoc NestedNameSpecifier =
   Loc.castAs().getQualifierLoc();
+  // This happens for friend declaration of a base class with injected class
+  // name.
+  if (!NestedNameSpecifier.getNestedNameSpecifier())
+return;
   const Type *SpecifierType =
   NestedNameSpecifier.getNestedNameSpecifier()->getAsType();
   if (SpecifierType && SpecifierType->isRecordType())
Index: clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
+++ clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -2154,6 +2154,60 @@
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+TEST_F(ChangeNamespaceTest, InjectedClassNameInFriendDecl) {
+  OldNames

[PATCH] D40548: [clangd] Symbol index interfaces and index-based code completion.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

In https://reviews.llvm.org/D40548#949182, @malaperle wrote:

> As a follow-up, here's the interface for querying the index that I am using 
> right now. It's meant to be able to retrieve from any kind of "backend", i.e. 
> in-memory, ClangdIndexDataStore, libIndexStore, etc. I was able to implement 
> "Open Workspace Symbol" (which is close to code completion in concept), Find 
> References and Find Definitions.
>
>   using USR = llvm::SmallString<256>;
>  
>   class ClangdIndexDataOccurrence;
>  
>   class ClangdIndexDataSymbol {
>   public:
> virtual index::SymbolKind getKind() = 0;
> /// For example, for mynamespace::myclass::mymethod, this will be
> /// mymethod.
> virtual std::string getName() = 0;
> /// For example, for mynamespace::myclass::mymethod, this will be
> /// mynamespace::myclass::
> virtual std::string getQualifier() = 0;
> virtual std::string getUsr() = 0;
>  
> virtual void foreachOccurrence(index::SymbolRoleSet Roles, 
> llvm::function_ref Receiver) = 0;
>  
> virtual ~ClangdIndexDataSymbol() = default;
>   };
>  
>   class ClangdIndexDataOccurrence {
>   public:
> enum class OccurrenceType : uint16_t {
>OCCURRENCE,
>DEFINITION_OCCURRENCE
>  };
>  
> virtual OccurrenceType getKind() const = 0;
> virtual std::string getPath() = 0;
> /// Get the start offset of the symbol occurrence. The SourceManager can 
> be
> /// used for implementations that need to convert from a line/column
> /// representation to an offset.
> virtual uint32_t getStartOffset(SourceManager &SM) = 0;
> /// Get the end offset of the symbol occurrence. The SourceManager can be
> /// used for implementations that need to convert from a line/column
> /// representation to an offset.
> virtual uint32_t getEndOffset(SourceManager &SM) = 0;
> virtual ~ClangdIndexDataOccurrence() = default;
> //TODO: Add relations
>  
> static bool classof(const ClangdIndexDataOccurrence *O) { return 
> O->getKind() == OccurrenceType::OCCURRENCE; }
>   };
>  
>   /// An occurrence that also has definition with a body that requires 
> additional
>   /// locations to keep track of the beginning and end of the body.
>   class ClangdIndexDataDefinitionOccurrence : public 
> ClangdIndexDataOccurrence {
>   public:
> virtual uint32_t getDefStartOffset(SourceManager &SM) = 0;
> virtual uint32_t getDefEndOffset(SourceManager &SM) = 0;
>  
> static bool classof(const ClangdIndexDataOccurrence *O) { return 
> O->getKind() == OccurrenceType::DEFINITION_OCCURRENCE; }
>   };
>  
>   class ClangdIndexDataProvider {
>   public:
>  
> virtual void foreachSymbols(StringRef Pattern, 
> llvm::function_ref Receiver) = 0;
> virtual void foreachSymbols(const USR &Usr, 
> llvm::function_ref Receiver) = 0;
>  
> virtual ~ClangdIndexDataProvider() = default;
>   };
>


I think some of the ideas here could be useful. This patch focuses mostly on 
index interfaces and https://reviews.llvm.org/D40897 emphasizes on the design 
of symbol structure. The way symbols are stored and used in this patch is 
likely to change depending on how https://reviews.llvm.org/D40897 goes.

> The "Clangd" prefix adds a bit much of clutter so maybe it should be removed. 
>  I think the main points are that having generic 
> foreachSymbols/foreachOccurrence with callbacks is well suited to implement 
> multiple features with minimal copying.

Although I'm not sure if `foreachSymbols`/... would be feasible for all indexes 
yet, we do plan to switch to callback-based index interfaces, which Sam also 
proposed in the review comments.

There have been some offline discussions happening around clangd's indexing, 
and sorry that we have not been able to keep you up to date. I think it might 
be more efficient if we could meet via VC/Hangouts and sync on our designs. If 
you don't mind a meeting, I am happy to arrange it via emails.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40548



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

[PATCH] D39375: [clang] Add PPCallbacks list to preprocessor when building a preacompiled preamble.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: lib/Frontend/PrecompiledPreamble.cpp:242
 std::shared_ptr PCHContainerOps, bool 
StoreInMemory,
-PreambleCallbacks &Callbacks) {
+PreambleCallbacks &Callbacks, std::unique_ptr PPCallbacks) {
   assert(VFS && "VFS is null");

Nebiroth wrote:
> ilya-biryukov wrote:
> > Could we add a method to `PreambleCallbacks` to create `PPCallbacks` 
> > instead?
> > Otherwise we have both `MacroDefined` in `PreambleCallbacks` and a separate 
> > set of PPCallbacks, so we have two ways of doing the same thing.
> > 
> > ```
> > class PreambleCallbacks {
> > public:
> > // ...
> > 
> >   /// Remove this.
> >   virtual void HandleMacroDefined(...);
> > 
> >   // Add this instead.
> >   virtual std::unique_ptr createPPCallbacks();
> > 
> > }
> > ```
> > 
> > Alternatively, follow the current pattern in `PreambleCallbacks` and add 
> > some extra functions from the `PPCallbacks` interface to it. This would not 
> > require changes to the existing usages of `PrecompiledPreamble` in 
> > `ASTUnit`. Albeit, I'd prefer the first option.
> > ```
> > class PreambleCallbacks {
> > public:
> > // ...
> > 
> >   // Keep this
> >   virtual void HandleMacroDefined(...);
> >   // Add the ones you need, e.g.
> >   virtual void InclusionDirective(...);
> >   virtual void FileChanged(...);
> > };
> > ```
> If we were to do that, one would then be required to define a wrapper class 
> for PPCallbacks and create an instance of it inside createPPCallbacks() for 
> the purpose of creating a unique_ptr? Then that unique_ptr would be sent from 
> within the PreambleCallbacks parameter in the Build function?
We're already passing our own wrapper around `PreambleCallbacks` anyway (see 
`PreambleMacroCallbacks`), we'll pass the `unique_ptr` instead.
But, yes, the clients will have to write their own implementation of 
`PPCallbacks` and pass it as `unique_ptr`. Is there something wrong with that?

Or, have I misunderstood the question entirely?


Repository:
  rC Clang

https://reviews.llvm.org/D39375



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

[PATCH] D40909: [clang-format] Reorganize raw string delimiters

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: include/clang/Format/Format.h:1375
+std::vector EnclosingFunctionNames;
+/// \brief The canonical delimiter for this language.
+std::string CanonicalDelimiter;

djasper wrote:
> krasimir wrote:
> > djasper wrote:
> > > Can you pull apart this patch?
> > > 
> > > In my view, it has three parts that have an ordering, but are actually 
> > > fairly independent:
> > > 
> > > 1. Propagate all configured languages to the formatting library. First 
> > > patch to land, should not affect the visible behavior.
> > > 2. Restructure RawStringFormat to be centered around each language. This 
> > > is a restructuring to make things easier and use #1.
> > > 3. Add a CanonicalDelimiter and make clang-format canonicalize it.
> > > 
> > > I'll focus my comments on what's required for #1 for now as that is 
> > > already complicated (IMO).
> > I believe these should all go together: the reason that we propagate all 
> > configured languages to the formatting library is to be able to use them as 
> > a replacement for the BasedOnStyle in RawStringFormat. To make this 
> > possible, we need to update the internal structure of RawStringFormat 
> > itself to base it around each language. The canonical delimiter part is 
> > just a convenience for this I guess, which could be split.
> > 
> > My biggest concern with (1) is that since it has no visible behavior and no 
> > other uses except for the adaptation of (2), it is not testable by itself 
> > and it's not evident that a patch doing just (1) would handle the things 
> > correctly.
> Ok, if you wish, this is not an unreasonable argument. But let's still do the 
> code review in two steps: Lets for now just get the part of handling multiple 
> languages straight and figure out the rest once we are sure that that part is 
> fine.
> 
> (I do think you can test it, though - but it depends on whether I can 
> convince you to go with the FormatStyleSet approach ;) )
On a philosophical level, something that has no visible behavior, and just 
restructures the code, should be tested by existing tests?

Enclosing function names also seems like an extra feature that could be pulled 
out, btw.



Comment at: lib/Format/Format.cpp:920
+  if (LanguageFound) {
+for (int i = Styles.size() - 1; i >= 0; --i) {
+  if (Styles[i].Language == FormatStyle::LK_None) {

djasper wrote:
> krasimir wrote:
> > djasper wrote:
> > > I think this is getting a bit convoluted and I don't even understand 
> > > whether we are doing what is document (even before this patch).
> > > 
> > > So in lines 892-905, we verify that:
> > > - Only the first Style in the file is allowed be LK_None.
> > > - No language is duplicated.
> > > 
> > > That seems good.
> > > 
> > > According to the documentation: "The first section may have no language 
> > > set, it will set the default style options for all lanugages.". Does the 
> > > latter part actually happen? Seems to me that we are just setting "Style" 
> > > to the style configured for a specific language, completely ignoring 
> > > values that might have been set in the LK_None style. Or is that somehow 
> > > happening when reading the JSON?
> > > 
> > > Independent of that, I think we should use this structure more 
> > > explicitly. I think we should create an additional class (FormatStyles or 
> > > FormatStyleSet or something) that is returned by this function and handed 
> > > to the formatting library. This function then doesn't need to look at the 
> > > language anymore.
> > > 
> > > That class should then have a function getFormatStyle(LanguageKind 
> > > Language); that returns the style for a particular language (doing the 
> > > default logic, etc.). Internally, it can likely just have a map > > Style> and I don't think you need to pre-fill that for all language 
> > > kinds. If a language kind is not in the map, you can just return what's 
> > > stored for LK_None. WDYT?
> > Yes, defaulting to the None for missing language specifications is handled 
> > at line 912:
> > ```
> > if (!LanguageFound && (Styles[i].Language == Language ||
> >Styles[i].Language == FormatStyle::LK_None
> > ```
> > 
> > I was thinking of the FormatStyleSet approach but the problem is that this 
> > has repercussions all over the library. We could indeed update this 
> > specific function that way, but fundamentally the additional language 
> > styles are part of the FormatStyle and need to somehow be recorded inside 
> > there. That's why I went with KISS and directly made this function handle 
> > that case.
> But it's not just defaulting to LK_None what we are saying we are 
> implementing. I think the documentation suggestion that we implement very 
> basic inheritance. E.g. if the style for LK_None set the ColumnLimit to 42, I 
> would expect that the styles for all other languages that don't explicitly 
> set a Colum

[PATCH] D40952: [clangd] Convert lit code completion tests to unit-tests. NFC

[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: unittests/clangd/CodeCompleteTests.cpp:289
+};
+void Foo::pub() { this->^ }
+  )cpp");

The `^` symbol conflicts with the corresponding operator.
Even though it's not widely used, I'm wondering whether we should use a 
different marker for completion position.



Comment at: unittests/clangd/CodeCompleteTests.cpp:335
+  )cpp",
+ Opts);
+  EXPECT_THAT(Results.items,

The formatting is a bit weird. Is this a `clang-format` bug?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40952



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

[PATCH] D40952: [clangd] Convert lit code completion tests to unit-tests. NFC

[Eric Liu via Phabricator via cfe-commits]

ioeric accepted this revision.
ioeric added a comment.

lg




Comment at: unittests/clangd/Matchers.h:54
+  bool MatchAndExplain(const std::vector &V,
+   ::testing::MatchResultListener *L) const {
+std::vector Matches(Matchers.size());

`override`?



Comment at: unittests/clangd/Matchers.h:77
+
+template  class PolySubsequenceMatcher {
+  std::tuple Matchers;

It would be helpful to provide a bit documentation here.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40952



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

[PATCH] D40489: [clangd] Changed tracing interfaces

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/Trace.cpp:138
 return;
-  if (!Args)
-Args = llvm::make_unique();
-  T->event(Ctx, "E",
-   Args ? json::obj{{"args", std::move(*Args)}} : json::obj{});
+  assert(Args && "Args can't be null at this point");
+  T->end_event(Ctx, Name, std::move(*Args));

sammccall wrote:
> why not?
Because `T` must outlive the `Span`, so we can't really have `if (!T)` evaluate 
to different things in constructor and destructor.
Am I missing something?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40489



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

[PATCH] D40952: [clangd] Convert lit code completion tests to unit-tests. NFC

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 126114.
sammccall marked an inline comment as done.
sammccall added a comment.

Reformat around code-complete testcases


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40952

Files:
  test/clangd/completion-items-kinds.test
  test/clangd/completion-priorities.test
  test/clangd/completion-qualifiers.test
  test/clangd/completion-snippet.test
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/Matchers.h

Index: unittests/clangd/Matchers.h
===
--- /dev/null
+++ unittests/clangd/Matchers.h
@@ -0,0 +1,103 @@
+//===-- Matchers.h --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// GMock matchers that aren't specific to particular tests.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_MATCHERS_H
+#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_MATCHERS_H
+#include "gmock/gmock.h"
+
+namespace clang {
+namespace clangd {
+using ::testing::Matcher;
+
+// EXPECT_IFF expects matcher if condition is true, and Not(matcher) if false.
+// This is hard to write as a function, because matchers may be polymorphic.
+#define EXPECT_IFF(condition, value, matcher)  \
+  do { \
+if (condition) \
+  EXPECT_THAT(value, matcher); \
+else   \
+  EXPECT_THAT(value, ::testing::Not(matcher)); \
+  } while (0)
+
+// HasSubsequence(m1, m2, ...) matches a vector containing elements that match
+// m1, m2 ... in that order.
+template 
+class SubsequenceMatcher
+: public ::testing::MatcherInterface &> {
+  std::vector> Matchers;
+
+public:
+  SubsequenceMatcher(std::vector> M) : Matchers(M) {}
+
+  void DescribeTo(std::ostream *OS) const {
+*OS << "Contains the subsequence [";
+const char *Sep = "";
+for (const auto &M : Matchers) {
+  *OS << Sep;
+  M.DescribeTo(OS);
+  Sep = ", ";
+}
+*OS << "]";
+  }
+
+  bool MatchAndExplain(const std::vector &V,
+   ::testing::MatchResultListener *L) const {
+std::vector Matches(Matchers.size());
+size_t I = 0;
+for (size_t J = 0; I < Matchers.size() && J < V.size(); ++J)
+  if (Matchers[I].Matches(V[J]))
+Matches[I++] = J;
+if (I == Matchers.size()) // We exhausted all matchers.
+  return true;
+if (L->IsInterested()) {
+  *L << "\n  Matched:";
+  for (size_t K = 0; K < I; ++K) {
+*L << "\n\t";
+Matchers[K].DescribeTo(L->stream());
+*L << " ==> " << ::testing::PrintToString(V[Matches[K]]);
+  }
+  *L << "\n\t";
+  Matchers[I].DescribeTo(L->stream());
+  *L << " ==> no subsequent match";
+}
+return false;
+  }
+};
+
+template  class PolySubsequenceMatcher {
+  std::tuple Matchers;
+
+public:
+  PolySubsequenceMatcher(M &&... Args)
+  : Matchers(std::make_tuple(std::forward(Args)...)) {}
+
+  template  operator Matcher &>() const {
+return ::testing::MakeMatcher(new SubsequenceMatcher(
+TypedMatchers(llvm::index_sequence_for{})));
+  }
+
+private:
+  template 
+  std::vector> TypedMatchers(llvm::index_sequence) const {
+return {std::get(Matchers)...};
+  }
+};
+
+template 
+PolySubsequenceMatcher HasSubsequence(Args &&... M) {
+  return PolySubsequenceMatcher(std::forward(M)...);
+}
+
+} // namespace clangd
+} // namespace clang
+#endif
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -8,6 +8,7 @@
 //===--===//
 #include "ClangdServer.h"
 #include "Compiler.h"
+#include "Matchers.h"
 #include "Protocol.h"
 #include "TestFS.h"
 #include "gmock/gmock.h"
@@ -18,15 +19,23 @@
 // Let GMock print completion items.
 void PrintTo(const CompletionItem &I, std::ostream *O) {
   llvm::raw_os_ostream OS(*O);
-  OS << toJSON(I);
+  OS << I.label << " - " << toJSON(I);
+}
+void PrintTo(const std::vector &V, std::ostream *O) {
+  *O << "{\n";
+  for (const auto &I : V) {
+*O << "\t";
+PrintTo(I, O);
+*O << "\n";
+  }
+  *O << "}";
 }
 
 namespace {
 using namespace llvm;
 using ::testing::AllOf;
 using ::testing::Contains;
 using ::testing::ElementsAre;
-using ::testing::Matcher;
 using ::testing::Not;
 
 class Igno

[PATCH] D40712: Add cc1 flag enabling the function stack size section that was added in r319430

[Sean Eveson via Phabricator via cfe-commits]

seaneveson added a comment.

Ping.


https://reviews.llvm.org/D40712



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

[PATCH] D40489: [clangd] Changed tracing interfaces

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clangd/Trace.cpp:138
 return;
-  if (!Args)
-Args = llvm::make_unique();
-  T->event(Ctx, "E",
-   Args ? json::obj{{"args", std::move(*Args)}} : json::obj{});
+  assert(Args && "Args can't be null at this point");
+  T->end_event(Ctx, Name, std::move(*Args));

ilya-biryukov wrote:
> sammccall wrote:
> > why not?
> Because `T` must outlive the `Span`, so we can't really have `if (!T)` 
> evaluate to different things in constructor and destructor.
> Am I missing something?
I was being disingenuous, I agree with you.
But can you change the message to "Changed tracer during a span"?

assert(Args) and "Args can't be null at this point" are basically synonyms :-)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40489



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

[PATCH] D40897: [clangd] Introduce a "Symbol" class.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 126115.
hokein marked 4 inline comments as done.
hokein added a comment.

Address review comments.

- Use SymbolSlab, for allowing future space optimization.
- Fix a Cast issue when building debug unittest.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40897

Files:
  clangd/CMakeLists.txt
  clangd/Symbol.cpp
  clangd/Symbol.h
  unittests/clangd/CMakeLists.txt
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- /dev/null
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -0,0 +1,112 @@
+//===-- SymbolCollectorTests.cpp  ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Symbol.h"
+#include "clang/Index/IndexingAction.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "gtest/gtest.h"
+#include "gmock/gmock.h"
+
+#include 
+#include 
+
+using testing::ElementsAre;
+using testing::Eq;
+using testing::Field;
+
+namespace clang {
+namespace clangd {
+
+namespace {
+class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
+ public:
+  SymbolIndexActionFactory() = default;
+
+  clang::FrontendAction *create() override {
+index::IndexingOptions IndexOpts;
+IndexOpts.SystemSymbolFilter =
+index::IndexingOptions::SystemSymbolFilterKind::All;
+IndexOpts.IndexFunctionLocals = false;
+Collector = std::make_shared();
+FrontendAction *Action =
+index::createIndexingAction(Collector, IndexOpts, nullptr).release();
+return Action;
+  }
+
+  std::shared_ptr Collector;
+};
+
+class SymbolCollectorTest : public ::testing::Test {
+public:
+  bool runSymbolCollector(StringRef HeaderCode, StringRef MainCode) {
+llvm::IntrusiveRefCntPtr InMemoryFileSystem(
+new vfs::InMemoryFileSystem);
+llvm::IntrusiveRefCntPtr Files(
+new FileManager(FileSystemOptions(), InMemoryFileSystem));
+
+const std::string FileName = "symbol.cc";
+const std::string HeaderName = "symbols.h";
+auto Factory = llvm::make_unique();
+
+tooling::ToolInvocation Invocation(
+{"symbol_collector", "-fsyntax-only", "-std=c++11", FileName},
+Factory->create(), Files.get(),
+std::make_shared());
+
+InMemoryFileSystem->addFile(HeaderName, 0,
+llvm::MemoryBuffer::getMemBuffer(HeaderCode));
+
+std::string Content = "#include\"" + std::string(HeaderName) + "\"";
+Content += "\n" + MainCode.str();
+InMemoryFileSystem->addFile(FileName, 0,
+llvm::MemoryBuffer::getMemBuffer(Content));
+Invocation.run();
+Symbols = Factory->Collector->takeSymbols();
+
+EXPECT_EQ(FileName, Factory->Collector->getFilename());
+return true;
+  }
+
+protected:
+  SymbolSlab Symbols;
+};
+
+TEST_F(SymbolCollectorTest, CollectSymbol) {
+  const std::string Header = R"(
+class Foo {
+  void f();
+};
+void f1();
+inline void f2() {}
+  )";
+  const std::string Main = R"(
+namespace {
+void ff() {} // ignore
+}
+void f1() {}
+  )";
+  runSymbolCollector(Header, Main);
+  EXPECT_THAT(Symbols,
+  UnorderedElementsAre(Field(&Symbol::QualifiedName, Eq("Foo")),
+   Field(&Symbol::QualifiedName, Eq("Foo::f")),
+   Field(&Symbol::QualifiedName, Eq("f1")),
+   Field(&Symbol::QualifiedName, Eq("f2";
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: unittests/clangd/CMakeLists.txt
===
--- unittests/clangd/CMakeLists.txt
+++ unittests/clangd/CMakeLists.txt
@@ -15,6 +15,7 @@
   JSONExprTests.cpp
   TestFS.cpp
   TraceTests.cpp
+  SymbolCollectorTests.cpp
   )
 
 target_link_libraries(ClangdTests
Index: clangd/Symbol.h
===
--- /dev/null
+++ clangd/Symbol.h
@@ -0,0 +1,133 @@
+//===--- Symbol.h ---*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===-===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SYMBOL

[PATCH] D38639: [clangd] #include statements support for Open definition

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/ClangdUnit.cpp:38
 
+class DelegatingPPCallbacks : public PPCallbacks {
+

Nebiroth wrote:
> ilya-biryukov wrote:
> > What's the purpose of this class?
> We need to be able to use a wrapper class to be able to make a unique_ptr to 
> be sent to PrecompiledPreamble::Build in order to add the list of 
> preprocessor Callbacks.
Could we implement an instance of `PPCallbacks` that contains  
`CppFilePreambleCallbacks` and forwards to that specific method instead?

The reason is that we're not really delegating other methods in this calls(nor 
should we, the implementation would be too compilcated).
Having a class that contains `CppFilePreambleCallbacks &Collector` and calling 
`Collector.InclusionDirective` seems perfectly fine, though: its purpose is 
clear and the implementation is easy.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D38639



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

[PATCH] D40897: [clangd] Introduce a "Symbol" class.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Thanks for the useful comments!




Comment at: clangd/Symbol.h:1
+//===--- Symbol.h ---*- C++-*-===//
+//

sammccall wrote:
> sammccall wrote:
> > I think that:
> >  - there's other places in clangd that deal with symbols too, this is 
> > specifically for indexing
> >  - the index interface belongs alongside Symbol
> > I'd suggest calling this Index.h
> I don't think having `Symbol`s be completely self-contained objects and 
> passing them around in standard containers like `set`s will prove to be ideal.
> It means they can't share storage for e.g. location filename, that it's hard 
> for us to arena-allocate them, etc.
> 
> I think we could use the concept of a set of symbols which share a lifetime. 
> An initial version might just be
> 
> class SymbolSlab {
> public:
>   using iterator = DenseSet::iterator;
>   iterator begin();
>   iterator end();
> private:
>   DenseSet Symbols;
> }
> 
> But it's easy to add `StringPool` etc there.
> Then this is the natural unit of granularity of large sets of symbols:  a 
> dynamic index that deals with one file at a time would operate on (Filename, 
> SymbolSlab) pairs. SymbolCollector would return a SymbolSlab, etc.
> 
> Then indexes can be built on top of this using non-owning pointers.
+1 It makes sense. 

For initial version, the `Symbol` structure is still owning its fields naively, 
we could improve it (change to pointer or references) in the future.



Comment at: clangd/Symbol.h:23
+  // The path of the source file where a symbol occurs.
+  std::string FilePath;
+  // The offset to the first character of the symbol from the beginning of the

malaperle wrote:
> malaperle wrote:
> > sammccall wrote:
> > > ioeric wrote:
> > > > Is this relative or absolute?
> > > Having every symbol own a copy of the filepath seems wasteful.
> > > It seems likely that the index will have per-file information too, so 
> > > this representation should likely be a key to that. Hash of the filepath 
> > > might work?
> > How we model it is that a symbol doesn't have a "location", but its 
> > occurrence do. One could consider the location of a symbol to be either its 
> > declaration occurrence (SymbolRole::Declaration) or its definition 
> > (SymbolRole::Definition).
> > What we do to get the location path is each occurrence has a pointer (a 
> > "database" pointer, but it doesn't matter) to a file entry and then we get 
> > the path from the entry.
> > 
> > So conceptually, it works a bit like this (although it fetches information 
> > on disk).
> > ```
> > class IndexOccurrence {
> > IndexOccurrence *FilePtr;
> > 
> > std::string Occurrence::getPath() {
> >   return FilePtr->getPath();
> > }
> > };
> > ```
> Oops, wrong type for the field, it should have been:
> ```
> class IndexOccurrence {
> IndexFile *FilePtr;
> 
> std::string Occurrence::getPath() {
>   return FilePtr->getPath();
> }
> };
> ```
> Is this relative or absolute?

 Whether the file path is relative or absolute depends on the build system, the 
file path could be relative (for header files) or absolute (for .cc files).

> How we model it is that a symbol doesn't have a "location", but its 
> occurrence do.

We will also have a SymbolOccurence structure alongside with Symbol (but it is 
not  in this patch). The "Location" will be a part of SymbolOccurrence.
 



Comment at: clangd/Symbol.h:26
+  // source file.
+  unsigned StartOffset;
+  // The offset to the last character of the symbol from the beginning of the

ioeric wrote:
> 0-based or 1-based?
The offset is equivalent to FileOffset in `SourceLocation`. I can't find any 
document about the FileOffset in LLVM, but it is 0-based.



Comment at: clangd/Symbol.h:32
+
+struct CodeCompletionInfo {
+  // FIXME: add fields here.

sammccall wrote:
> Let's not add this until we know what's in it.
> There's gong to be an overlap between information needed for CC and other use 
> cases, so this structure might not help the user navigate.
Removed it.



Comment at: clangd/Symbol.h:37
+// The class presents a C++ symbol, e.g. class, function.
+struct Symbol {
+  // The symbol identifier, using USR.

malaperle wrote:
> sammccall wrote:
> > hokein wrote:
> > > malaperle wrote:
> > > > I think it would be nice to have methods as an interface to get this 
> > > > data instead of storing them directly. So that an index-on-disk could 
> > > > go fetch the data. Especially the occurrences which can take a lot of 
> > > > memory (I'm working on a branch that does that). But perhaps defining 
> > > > that interface is not within the scope of this patch and could be 
> > > > better discussed in D40548 ?
> > > I agree. We can't load all the symbol occurrences into the memory since 
> > > they are too large. We need to design 

[PATCH] D40952: [clangd] Convert lit code completion tests to unit-tests. NFC

[Sam McCall via Phabricator via cfe-commits]

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

Add documentation to matcher implementation.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40952

Files:
  test/clangd/completion-items-kinds.test
  test/clangd/completion-priorities.test
  test/clangd/completion-qualifiers.test
  test/clangd/completion-snippet.test
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/Matchers.h

Index: unittests/clangd/Matchers.h
===
--- /dev/null
+++ unittests/clangd/Matchers.h
@@ -0,0 +1,112 @@
+//===-- Matchers.h --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// GMock matchers that aren't specific to particular tests.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_MATCHERS_H
+#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_MATCHERS_H
+#include "gmock/gmock.h"
+
+namespace clang {
+namespace clangd {
+using ::testing::Matcher;
+
+// EXPECT_IFF expects matcher if condition is true, and Not(matcher) if false.
+// This is hard to write as a function, because matchers may be polymorphic.
+#define EXPECT_IFF(condition, value, matcher)  \
+  do { \
+if (condition) \
+  EXPECT_THAT(value, matcher); \
+else   \
+  EXPECT_THAT(value, ::testing::Not(matcher)); \
+  } while (0)
+
+// HasSubsequence(m1, m2, ...) matches a vector containing elements that match
+// m1, m2 ... in that order.
+//
+// SubsequenceMatcher implements this once the type of vector is known.
+template 
+class SubsequenceMatcher
+: public ::testing::MatcherInterface &> {
+  std::vector> Matchers;
+
+public:
+  SubsequenceMatcher(std::vector> M) : Matchers(M) {}
+
+  void DescribeTo(std::ostream *OS) const override {
+*OS << "Contains the subsequence [";
+const char *Sep = "";
+for (const auto &M : Matchers) {
+  *OS << Sep;
+  M.DescribeTo(OS);
+  Sep = ", ";
+}
+*OS << "]";
+  }
+
+  bool MatchAndExplain(const std::vector &V,
+   ::testing::MatchResultListener *L) const override {
+std::vector Matches(Matchers.size());
+size_t I = 0;
+for (size_t J = 0; I < Matchers.size() && J < V.size(); ++J)
+  if (Matchers[I].Matches(V[J]))
+Matches[I++] = J;
+if (I == Matchers.size()) // We exhausted all matchers.
+  return true;
+if (L->IsInterested()) {
+  *L << "\n  Matched:";
+  for (size_t K = 0; K < I; ++K) {
+*L << "\n\t";
+Matchers[K].DescribeTo(L->stream());
+*L << " ==> " << ::testing::PrintToString(V[Matches[K]]);
+  }
+  *L << "\n\t";
+  Matchers[I].DescribeTo(L->stream());
+  *L << " ==> no subsequent match";
+}
+return false;
+  }
+};
+
+// PolySubsequenceMatcher implements a "polymorphic" SubsequenceMatcher.
+// It captures the types of the element matchers, and can be converted to
+// Matcher> if each matcher can be converted to Matcher.
+// This allows HasSubsequence() to accept polymorphic matchers like Not().
+template  class PolySubsequenceMatcher {
+  std::tuple Matchers;
+
+public:
+  PolySubsequenceMatcher(M &&... Args)
+  : Matchers(std::make_tuple(std::forward(Args)...)) {}
+
+  template  operator Matcher &>() const {
+return ::testing::MakeMatcher(new SubsequenceMatcher(
+TypedMatchers(llvm::index_sequence_for{})));
+  }
+
+private:
+  template 
+  std::vector> TypedMatchers(llvm::index_sequence) const {
+return {std::get(Matchers)...};
+  }
+};
+
+// HasSubsequence(m1, m2, ...) matches a vector containing elements that match
+// m1, m2 ... in that order.
+// The real implementation is in SubsequenceMatcher.
+template 
+PolySubsequenceMatcher HasSubsequence(Args &&... M) {
+  return PolySubsequenceMatcher(std::forward(M)...);
+}
+
+} // namespace clangd
+} // namespace clang
+#endif
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -8,6 +8,7 @@
 //===--===//
 #include "ClangdServer.h"
 #include "Compiler.h"
+#include "Matchers.h"
 #include "Protocol.h"
 #include "TestFS.h"
 #include "gmock/gmock.h"
@@ -18,15 +19,23 @@
 // Let GMock print completion items.
 void Print

[PATCH] D40548: [clangd] Symbol index interfaces and index-based code completion.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Thanks for the feedback, Marc!

Yeah, I think the ClangdIndexDataSymbol and ClangdIndexDataOccurrence are 
something similar to what https://reviews.llvm.org/D40897 trying to address, 
maybe put the discussion there? Before that, I think having a sym meeting is a 
good idea to keep us in the same page.

In https://reviews.llvm.org/D40548#949279, @ioeric wrote:

> >   
>
> I think some of the ideas here could be useful. This patch focuses mostly on 
> index interfaces and https://reviews.llvm.org/D40897 emphasizes on the design 
> of symbol structure. The way symbols are stored and used in this patch is 
> likely to change depending on how https://reviews.llvm.org/D40897 goes.
>
> > The "Clangd" prefix adds a bit much of clutter so maybe it should be 
> > removed.  I think the main points are that having generic 
> > foreachSymbols/foreachOccurrence with callbacks is well suited to implement 
> > multiple features with minimal copying.
>
> Although I'm not sure if `foreachSymbols`/... would be feasible for all 
> indexes yet, we do plan to switch to callback-based index interfaces, which 
> Sam also proposed in the review comments.
>
> There have been some offline discussions happening around clangd's indexing, 
> and sorry that we have not been able to keep you up to date. I think it might 
> be more efficient if we could meet via VC/Hangouts and sync on our designs. 
> If you don't mind a meeting, I am happy to arrange it via emails.





Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40548



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

[PATCH] D38425: [clangd] Document highlights for clangd

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Overall looks good, just a few minor comments.
Let me know if need someone to land this patch for you after you fix those.




Comment at: clangd/ClangdUnit.cpp:249
+// Don't keep the same Macro info multiple times.
+// This can happen when nodes in the AST are visited twice.
+std::sort(MacroInfos.begin(), MacroInfos.end());

Mentioning AST in this comment is weird, macros have nothing to do with the 
AST. We should remove/rephrase the comment.
I'm not sure if multiple occurences of `MacroInfo` are even possible here, but 
we could leave the code as is anyway.



Comment at: clangd/Protocol.h:562
+
+///
+/// A document highlight is a range inside a text document which deserves

NIT: remove this empty comment line and all the others.



Comment at: clangd/Protocol.h:581
+const DocumentHighlight &RHS) {
+return std::tie(LHS.range) < std::tie(RHS.range) &&
+   std::tie(LHS.kind) < std::tie(RHS.kind);

This comparison does not provide a total order.
Please use `std::tie(LHS.range, int(LHS.kind)) < std::tie(RHS.range, 
int(RHS.kind))` instead.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D38425



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

[PATCH] D40561: [libclang] Fix cursors for functions with trailing return type

[Nikolai Kosjar via Phabricator via cfe-commits]

nik added a comment.

Ping


https://reviews.llvm.org/D40561



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

[PATCH] D40481: [libclang] Fix cursors for arguments of Subscript and Call operators

[Nikolai Kosjar via Phabricator via cfe-commits]

nik added a comment.

Ping


https://reviews.llvm.org/D40481



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

[PATCH] D40948: Switch Clang's default C++ language target to C++14

[Tim Northover via Phabricator via cfe-commits]

t.p.northover updated this revision to Diff 126129.
t.p.northover added a comment.

Updating with tentative fixes to review comments.


Repository:
  rC Clang

https://reviews.llvm.org/D40948

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCXX/new-overflow.cpp
  clang/test/CodeGenCXX/new.cpp
  clang/test/CodeGenCXX/vtable-available-externally.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/half-literal.cpp
  clang/test/OpenMP/taskloop_reduction_codegen.cpp
  clang/test/OpenMP/taskloop_simd_reduction_codegen.cpp
  clang/test/Parser/cxx1z-nested-namespace-definition.cpp
  clang/test/SemaCXX/new-array-size-conv.cpp
  clang/test/SemaCXX/new-delete.cpp
  clang/test/SemaCXX/unknown-type-name.cpp
  clang/test/SemaTemplate/class-template-decl.cpp
  clang/test/SemaTemplate/explicit-instantiation.cpp

Index: clang/test/SemaTemplate/explicit-instantiation.cpp
===
--- clang/test/SemaTemplate/explicit-instantiation.cpp
+++ clang/test/SemaTemplate/explicit-instantiation.cpp
@@ -124,10 +124,10 @@
 namespace undefined_static_data_member {
   template struct A {
 static int a; // expected-note {{here}}
-template static int b; // expected-note {{here}} expected-warning {{extension}}
+template static int b; // expected-note {{here}} expected-warning 0+ {{extension}}
   };
   struct B {
-template static int c; // expected-note {{here}} expected-warning {{extension}}
+template static int c; // expected-note {{here}} expected-warning 0+ {{extension}}
   };
 
   template int A::a; // expected-error {{explicit instantiation of undefined static data member 'a' of class template 'undefined_static_data_member::A'}}
@@ -137,14 +137,14 @@
 
   template struct C {
 static int a;
-template static int b; // expected-warning {{extension}}
+template static int b; // expected-warning 0+ {{extension}}
   };
   struct D {
-template static int c; // expected-warning {{extension}}
+template static int c; // expected-warning 0+ {{extension}}
   };
   template int C::a;
-  template template int C::b; // expected-warning {{extension}}
-  template int D::c; // expected-warning {{extension}}
+  template template int C::b; // expected-warning 0+ {{extension}}
+  template int D::c; // expected-warning 0+ {{extension}}
 
   template int C::a;
   template int C::b;
Index: clang/test/SemaTemplate/class-template-decl.cpp
===
--- clang/test/SemaTemplate/class-template-decl.cpp
+++ clang/test/SemaTemplate/class-template-decl.cpp
@@ -57,8 +57,7 @@
   template class X; // expected-error{{expression}}
 }
 
-template class X1 var; // expected-warning{{variable templates are a C++14 extension}} \
-   // expected-error {{variable has incomplete type 'class X1'}} \
+template class X1 var; // expected-error {{variable has incomplete type 'class X1'}} \
// expected-note {{forward declaration of 'X1'}}
 
 namespace M {
Index: clang/test/SemaCXX/unknown-type-name.cpp
===
--- clang/test/SemaCXX/unknown-type-name.cpp
+++ clang/test/SemaCXX/unknown-type-name.cpp
@@ -95,18 +95,23 @@
 template int h(T::type, int); // expected-error{{missing 'typename'}}
 template int h(T::type x, char); // expected-error{{missing 'typename'}}
 
-template int junk1(T::junk); // expected-warning{{variable templates are a C++14 extension}}
+template int junk1(T::junk);
+#if __cplusplus <= 201103L
+// expected-warning@-2 {{variable templates are a C++14 extension}}
+#endif
 template int junk2(T::junk) throw(); // expected-error{{missing 'typename'}}
 template int junk3(T::junk) = delete; // expected-error{{missing 'typename'}}
 #if __cplusplus <= 199711L
 //expected-warning@-2 {{deleted function definitions are a C++11 extension}}
 #endif
 
 template int junk4(T::junk j); // expected-error{{missing 'typename'}}
 
-// FIXME: We can tell this was intended to be a function because it does not
-//have a dependent nested name specifier.
-template int i(T::type, int()); // expected-warning{{variable templates are a C++14 extension}}
+template int i(T::type, int());
+#if __cplusplus <= 201103L
+// expected-warning@-2 {{variable templates are a C++14 extension}}
+#endif
+
 
 // FIXME: We know which type specifier should have been specified here. Provide
 //a fix-it to add 'typename A::type'
Index: clang/test/SemaCXX/new-delete.cpp
===
--- clang/test/SemaCXX/new-delete.cpp
+++ clang/test/SemaCXX/new-delete.cpp
@@ -80,27 +80,43 @@
   (void)new int[1.1];
 #if __cplusplus <= 199711L
   // expected-error@-2 {{array size expression must have integral or enumeration type, not 'double'}}
-#else
+#elif __cplusplus <= 201103L
   // expected-error@-4 {{array size expression must have integral or unscoped 

[PATCH] D40897: [clangd] Introduce a "Symbol" class.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

More comments, but only two major things really:

- I'd like to clearly separate USR from SymbolID (even if you want to keep 
using USRs as their basis for now)
- the file organization (code within files, and names of files) needs some work 
I think

Everything else is details, this looks good




Comment at: clangd/Symbol.h:23
+  // The path of the source file where a symbol occurs.
+  std::string FilePath;
+  // The offset to the first character of the symbol from the beginning of the

hokein wrote:
> malaperle wrote:
> > malaperle wrote:
> > > sammccall wrote:
> > > > ioeric wrote:
> > > > > Is this relative or absolute?
> > > > Having every symbol own a copy of the filepath seems wasteful.
> > > > It seems likely that the index will have per-file information too, so 
> > > > this representation should likely be a key to that. Hash of the 
> > > > filepath might work?
> > > How we model it is that a symbol doesn't have a "location", but its 
> > > occurrence do. One could consider the location of a symbol to be either 
> > > its declaration occurrence (SymbolRole::Declaration) or its definition 
> > > (SymbolRole::Definition).
> > > What we do to get the location path is each occurrence has a pointer (a 
> > > "database" pointer, but it doesn't matter) to a file entry and then we 
> > > get the path from the entry.
> > > 
> > > So conceptually, it works a bit like this (although it fetches 
> > > information on disk).
> > > ```
> > > class IndexOccurrence {
> > > IndexOccurrence *FilePtr;
> > > 
> > > std::string Occurrence::getPath() {
> > >   return FilePtr->getPath();
> > > }
> > > };
> > > ```
> > Oops, wrong type for the field, it should have been:
> > ```
> > class IndexOccurrence {
> > IndexFile *FilePtr;
> > 
> > std::string Occurrence::getPath() {
> >   return FilePtr->getPath();
> > }
> > };
> > ```
> > Is this relative or absolute?
> 
>  Whether the file path is relative or absolute depends on the build system, 
> the file path could be relative (for header files) or absolute (for .cc 
> files).
> 
> > How we model it is that a symbol doesn't have a "location", but its 
> > occurrence do.
> 
> We will also have a SymbolOccurence structure alongside with Symbol (but it 
> is not  in this patch). The "Location" will be a part of SymbolOccurrence.
>  
> Whether the file path is relative or absolute depends on the build system, 
> the file path could be relative (for header files) or absolute (for .cc 
> files).

I'm not convinced this actually works. There's multiple codepaths to the index, 
how can we ensure we don't end up using inconsistent paths?
e.g. we open up a project that includes a system header using a relative path, 
and then open up that system header from file->open (editor knows only the 
absolute path) and do "find references".

I think we need to canonicalize the paths. Absolute is probably easiest.



Comment at: clangd/Symbol.h:37
+// The class presents a C++ symbol, e.g. class, function.
+struct Symbol {
+  // The symbol identifier, using USR.

hokein wrote:
> malaperle wrote:
> > sammccall wrote:
> > > hokein wrote:
> > > > malaperle wrote:
> > > > > I think it would be nice to have methods as an interface to get this 
> > > > > data instead of storing them directly. So that an index-on-disk could 
> > > > > go fetch the data. Especially the occurrences which can take a lot of 
> > > > > memory (I'm working on a branch that does that). But perhaps defining 
> > > > > that interface is not within the scope of this patch and could be 
> > > > > better discussed in D40548 ?
> > > > I agree. We can't load all the symbol occurrences into the memory since 
> > > > they are too large. We need to design interface for the symbol 
> > > > occurrences. 
> > > > 
> > > > We could discuss the interface here, but CodeCompletion is the main 
> > > > thing which this patch focuses on. 
> > > > We can't load all the symbol occurrences into the memory since they are 
> > > > too large
> > > 
> > > I've heard this often, but never backed up by data :-)
> > > 
> > > Naively an array of references for a symbol could be doc ID + offset + 
> > > length, let's say 16 bytes.
> > > 
> > > If a source file consisted entirely of references to 1-character symbols 
> > > separated by punctuation (1 reference per 2 bytes) then the total size of 
> > > these references would be 8x the size of the source file - in practice 
> > > much less. That's not very big.
> > > 
> > > (Maybe there are edge cases with macros/templates, but we can keep them 
> > > under control)
> > I'd have to break down how much memory it used by what, I'll come back to 
> > you on that. Indexing llvm with ClangdIndexDataStorage, which is pretty 
> > packed is about 200MB. That's already a lot considering we want to index 
> > code bases many times bigger. But I'll try to come up with more precise 
> > numbers. I'm open to different strategies.

[PATCH] D39903: [libclang] Allow pretty printing declarations

[Nikolai Kosjar via Phabricator via cfe-commits]

nik added a comment.

In https://reviews.llvm.org/D39903#944182, @cameron314 wrote:

> Locally we've done something similar (adding a 
> `clang_getCursorPrettyPrintedDeclaration` function, though without exposing 
> the `PrintingPolicy`) and overhauled `DeclPrinter` to produce proper pretty 
> names. This is a hack that works well for us, but can never be upstreamed 
> since it breaks too much existing code (and some of the printing decisions 
> are subjective). Your way is better.


You might consider to enhance PrintingPolicy for your use cases?

> I can point out differences in our implementations if you like. The diff is 
> rather long, though.

That would be interesting, yes, but rather later.

First I would like to get a review for this one...

Ping.


Repository:
  rC Clang

https://reviews.llvm.org/D39903



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

[PATCH] D40948: Switch Clang's default C++ language target to C++14

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: clang/test/CodeGenCXX/new-overflow.cpp:88
   // CHECK:  [[N:%.*]] = sext i16 {{%.*}} to i32
-  // CHECK-NEXT: [[T0:%.*]] = icmp slt i32 [[N]], 0
-  // CHECK-NEXT: [[T1:%.*]] = select i1 [[T0]], i32 -1, i32 [[N]]
-  // CHECK-NEXT: call i8* @_Znaj(i32 [[T1]])
+  // CHECK-NEXT: call i8* @_Znaj(i32 [[N]])
   // CHECK:  getelementptr inbounds {{.*}}, i32 [[N]]

rsmith wrote:
> The changes in this file are a regression; C++14 requires us to check whether 
> the array bound prior to promotion is negative. Can you file a bug on that?
I've filed https://llvm.org/PR35573. Not quite sure what to do about this test 
until it's fixed though. Add a second RUN line to check both variants and then 
XFAIL it?



Comment at: clang/test/CodeGenCXX/vtable-available-externally.cpp:275
 struct C {
   virtual D& operator=(const D&);
 };

rsmith wrote:
> To make this test work in C++11 onwards, you need to add a virtual move 
> assignment operator here:
> 
> ```
> virtual D& operator=(D&&);
> ```
That didn't quite work. The issue appears to be that D has both of those 
implicitly defined in C++14 mode, but only the move assignment operator is used 
below. Speculative VTable emission requires all of them to be used.

So adding a "d = d;" line to the second g function fixes the test. Does that 
sound sane to you, or am I missing the point?



Comment at: clang/test/SemaCXX/unknown-type-name.cpp:98
 
-template int junk1(T::junk); // expected-warning{{variable 
templates are a C++14 extension}}
+template int junk1(T::junk);
+#if __cplusplus <= 201103L

rsmith wrote:
> Huh, we should probably have a `-Wvexing-parse` warning for this. Can you 
> file a bug?
I've filed https://llvm.org/PR35576. You may want to sanity check it though, I 
was pretty light on the detail because I wasn't sure of the exact diagnostic 
being proposed.


Repository:
  rC Clang

https://reviews.llvm.org/D40948



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

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

[Nikolai Kosjar via Phabricator via cfe-commits]

nik created this revision.

When a preamble is created an unsaved file not existing on disk is
already part of PrecompiledPreamble::FilesInPreamble. However, when
checking whether the preamble can be re-used, a failed stat of such an
unsaved file invalidated the preamble, which led to pointless and time
consuming preamble regenerations on subsequent reparses.

Do not require anymore that unsaved files should exist on disk.

This avoids costly preamble invalidations depending on timing issues for
the cases where the file on disk might be removed just to be regenerated
a bit later.

It also allows an IDE to provide in-memory files that might not exist on
disk, e.g. because the build system hasn't generated those yet.


Repository:
  rC Clang

https://reviews.llvm.org/D41005

Files:
  include/clang/Frontend/ASTUnit.h
  include/clang/Frontend/PrecompiledPreamble.h
  lib/Frontend/ASTUnit.cpp
  lib/Frontend/PrecompiledPreamble.cpp
  unittests/Frontend/PCHPreambleTest.cpp

Index: unittests/Frontend/PCHPreambleTest.cpp
===
--- unittests/Frontend/PCHPreambleTest.cpp
+++ unittests/Frontend/PCHPreambleTest.cpp
@@ -124,6 +124,23 @@
   }
 };
 
+TEST_F(PCHPreambleTest, ReparseDoesNotInvalidatePreambleDueToNotExistingUnsavedFile) {
+  std::string Header1 = "//./header1.h";
+  std::string MainName = "//./main.cpp";
+  AddFile(MainName, "#include \"//./header1.h\"\n"
+"int main() { return ZERO; }");
+
+  std::unique_ptr AST(ParseAST(MainName));
+  ASSERT_TRUE(AST.get());
+  auto InitialPreambleCreationTimePoint = AST->getPreambleCreationTimePoint();
+  ASSERT_NE(std::chrono::steady_clock::time_point(), InitialPreambleCreationTimePoint);
+
+  RemapFile(Header1, "#define ZERO 0\n");
+  ASSERT_TRUE(ReparseAST(AST));
+
+  ASSERT_EQ(InitialPreambleCreationTimePoint, AST->getPreambleCreationTimePoint());
+}
+
 TEST_F(PCHPreambleTest, ReparseWithOverriddenFileDoesNotInvalidatePreamble) {
   std::string Header1 = "//./header1.h";
   std::string Header2 = "//./header2.h";
Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -390,10 +390,10 @@
   return PreambleBounds(PreambleBytes.size(), PreambleEndsAtStartOfLine);
 }
 
-bool PrecompiledPreamble::CanReuse(const CompilerInvocation &Invocation,
-   const llvm::MemoryBuffer *MainFileBuffer,
-   PreambleBounds Bounds,
-   vfs::FileSystem *VFS) const {
+bool PrecompiledPreamble::CanReuse(
+const CompilerInvocation &Invocation,
+const llvm::MemoryBuffer *MainFileBuffer, PreambleBounds Bounds,
+IntrusiveRefCntPtr VFS) const {
 
   assert(
   Bounds.Size <= MainFileBuffer->getBufferSize() &&
@@ -434,19 +434,22 @@
 Status.getSize(), llvm::sys::toTimeT(Status.getLastModificationTime()));
   }
 
+  vfs::OverlayFileSystem OFS(VFS);
+  IntrusiveRefCntPtr IMFS(
+  new vfs::InMemoryFileSystem());
+  OFS.pushOverlay(IMFS);
   for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) {
+IMFS->addFile(RB.first, 0, llvm::MemoryBuffer::getMemBuffer(""));
 vfs::Status Status;
-if (!moveOnNoError(VFS->status(RB.first), Status))
-  return false;
-
+assert(moveOnNoError(IMFS->status(RB.first), Status));
 OverriddenFiles[Status.getUniqueID()] =
 PreambleFileHash::createForMemoryBuffer(RB.second);
   }
 
   // Check whether anything has changed.
   for (const auto &F : FilesInPreamble) {
 vfs::Status Status;
-if (!moveOnNoError(VFS->status(F.first()), Status)) {
+if (!moveOnNoError(OFS.status(F.first()), Status)) {
   // If we can't stat the file, assume that something horrible happened.
   return false;
 }
@@ -495,7 +498,8 @@
 llvm::StringMap FilesInPreamble)
 : Storage(std::move(Storage)), FilesInPreamble(std::move(FilesInPreamble)),
   PreambleBytes(std::move(PreambleBytes)),
-  PreambleEndsAtStartOfLine(PreambleEndsAtStartOfLine) {
+  PreambleEndsAtStartOfLine(PreambleEndsAtStartOfLine),
+  CreationTimePoint(std::chrono::steady_clock::now()){
   assert(this->Storage.getKind() != PCHStorage::Kind::Empty);
 }
 
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1244,7 +1244,7 @@
 
   if (Preamble) {
 if (Preamble->CanReuse(PreambleInvocationIn, MainFileBuffer.get(), Bounds,
-   VFS.get())) {
+   VFS)) {
   // Okay! We can re-use the precompiled preamble.
 
   // Set the state of the diagnostic object to mimic its state
Index: include/clang/Frontend/PrecompiledPreamble.h
===
--- include/clang/Frontend/PrecompiledPreamble.h
+++ include/clang/Frontend

[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions

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

xazax.hun added inline comments.



Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:59
+  if (!BeginLoc.isMacroID()) {
+Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
+   "s");

aaron.ballman wrote:
> JonasToth wrote:
> > koldaniel wrote:
> > > JonasToth wrote:
> > > > Could the location not simply be `EndLoc`?
> > > As I could see, when EndLoc was used in Diag (diag(..) of 
> > > CreateInsertion(...) methods,  it still pointed to the beginning of the 
> > > token marking the whole call. So if the CreateInsertion function looked 
> > > like this: 
> > > 
> > >   Diag << FixItHint::CreateInsertion(EndLoc.getLocWithOffset(TextLength), 
> > > "s");
> > > 
> > > had the same effect after applying the fix its as
> > > 
> > > Diag << 
> > > FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength), "s");
> > > 
> > > for calls like 'uncaught_exception()' (without std:: namespace written at 
> > > the beginning, because it increases TextLength, and in case of EndLoc the 
> > > offset will be counted from the beginning of the function name, not the 
> > > namespace identifier).
> > Thats odd. You could do a Replacement with `getSourceRange` probably. 
> > Sounds more inefficient, but might be shorter in Code.
> This fixit can break code if the code disallows narrowing conversions. e.g.,
> ```
> bool b{std::uncaught_exception()};
> ```
> In this case, the fixit will take the deprecated code and make it ill-formed 
> instead. Perhaps a better fix-it would be to transform the code into 
> `(std::uncaught_exceptions() > 0)` to keep the resulting expression type a 
> `bool` and still not impact operator precedence?
Good point, but this kind of fix it is a bit ugly. Maybe skipping the fixit in 
narrowing cases or only generate the more complicated replacement in the 
narrowing case would be nicer. 



Comment at: test/clang-tidy/modernize-replace-uncaught-exception.cpp:16
+template 
+int doSomething(T t) { 
+return t();

It would be great to have a test where the template parameter is a function 
pointer and it is called with `uncaught_exception`. And with a check fixes make 
sure that the definition of the template is untouched.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40787



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

[PATCH] D39375: [clang] Add PPCallbacks list to preprocessor when building a preacompiled preamble.

[William Enright via Phabricator via cfe-commits]

Nebiroth added inline comments.



Comment at: lib/Frontend/PrecompiledPreamble.cpp:242
 std::shared_ptr PCHContainerOps, bool 
StoreInMemory,
-PreambleCallbacks &Callbacks) {
+PreambleCallbacks &Callbacks, std::unique_ptr PPCallbacks) {
   assert(VFS && "VFS is null");

ilya-biryukov wrote:
> Nebiroth wrote:
> > ilya-biryukov wrote:
> > > Could we add a method to `PreambleCallbacks` to create `PPCallbacks` 
> > > instead?
> > > Otherwise we have both `MacroDefined` in `PreambleCallbacks` and a 
> > > separate set of PPCallbacks, so we have two ways of doing the same thing.
> > > 
> > > ```
> > > class PreambleCallbacks {
> > > public:
> > > // ...
> > > 
> > >   /// Remove this.
> > >   virtual void HandleMacroDefined(...);
> > > 
> > >   // Add this instead.
> > >   virtual std::unique_ptr createPPCallbacks();
> > > 
> > > }
> > > ```
> > > 
> > > Alternatively, follow the current pattern in `PreambleCallbacks` and add 
> > > some extra functions from the `PPCallbacks` interface to it. This would 
> > > not require changes to the existing usages of `PrecompiledPreamble` in 
> > > `ASTUnit`. Albeit, I'd prefer the first option.
> > > ```
> > > class PreambleCallbacks {
> > > public:
> > > // ...
> > > 
> > >   // Keep this
> > >   virtual void HandleMacroDefined(...);
> > >   // Add the ones you need, e.g.
> > >   virtual void InclusionDirective(...);
> > >   virtual void FileChanged(...);
> > > };
> > > ```
> > If we were to do that, one would then be required to define a wrapper class 
> > for PPCallbacks and create an instance of it inside createPPCallbacks() for 
> > the purpose of creating a unique_ptr? Then that unique_ptr would be sent 
> > from within the PreambleCallbacks parameter in the Build function?
> We're already passing our own wrapper around `PreambleCallbacks` anyway (see 
> `PreambleMacroCallbacks`), we'll pass the `unique_ptr` instead.
> But, yes, the clients will have to write their own implementation of 
> `PPCallbacks` and pass it as `unique_ptr`. Is there something wrong with that?
> 
> Or, have I misunderstood the question entirely?
No this is fine. I was just making sure I understood correctly.


Repository:
  rC Clang

https://reviews.llvm.org/D39375



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

[PATCH] D40685: [libunwind] Switch to add_llvm_install_targets

[Will Dietz via Phabricator via cfe-commits]

dtzWill added a comment.

Hmm, this change means very recent LLVM is required to build libunwind-- 
previously I was able to use recent libunwind/libcxx/libcxxabi with LLVM 5 and 
LLVM 4.

Other runtime projects (compiler-rt, libcxx, libcxxabi) were modified to 
support the new "strip-and-install" targets such that they didn't depend on 
LLVM cmake bits, could that sort of approach work here?


Repository:
  rL LLVM

https://reviews.llvm.org/D40685



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

[PATCH] D40372: [ExprConstant] Fix assert when initializing constexpr array

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a comment.

Ping!


https://reviews.llvm.org/D40372



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

[PATCH] D40952: [clangd] Convert lit code completion tests to unit-tests. NFC

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 2 inline comments as done.
sammccall added inline comments.



Comment at: unittests/clangd/CodeCompleteTests.cpp:289
+};
+void Foo::pub() { this->^ }
+  )cpp");

ilya-biryukov wrote:
> The `^` symbol conflicts with the corresponding operator.
> Even though it's not widely used, I'm wondering whether we should use a 
> different marker for completion position.
Almost all characters conflict with something in C++ :-(
^ is rarely going to cause problems and is suggestive of an insertion point.

The failure modes here don't seem worth worrying about:
 - we add a test that needs to use `^`. The assert fires, it's easy to tell 
what happened, and we have to add escaping or change the notation. Annoying, 
but easy to detect and pretty unlikely.
 - we add a test that needs to use exactly one `^`, *and* we forget to add the 
cursor marker to the test. The assert doesn't fire, the test fails in some 
random way. This is really annoying, but also vanishingly unlikely.

(I'd feel differently if this wasn't just a local test helper of course)




Comment at: unittests/clangd/CodeCompleteTests.cpp:335
+  )cpp",
+ Opts);
+  EXPECT_THAT(Results.items,

ilya-biryukov wrote:
> The formatting is a bit weird. Is this a `clang-format` bug?
Weird formatting is a clang-format *feature*.
I reformatted all the tests, now they're weird in a different way.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40952



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

[PATCH] D40948: Switch Clang's default C++ language target to C++14

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1733
   // The PS4 uses C++11 as the default C++ standard.
-  if (T.isPS4())
-LangStd = LangStandard::lang_gnucxx11;
-  else
-LangStd = LangStandard::lang_gnucxx98;
+  LangStd = LangStandard::lang_gnucxx14;
   break;

Why are you changing the PS4 default too?


Repository:
  rC Clang

https://reviews.llvm.org/D40948



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

[clang-tools-extra] r320148 - [clangd] Convert lit code completion tests to unit-tests. NFC

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri Dec  8 07:00:59 2017
New Revision: 320148

URL: http://llvm.org/viewvc/llvm-project?rev=320148&view=rev
Log:
[clangd] Convert lit code completion tests to unit-tests. NFC

Summary: This improves readability of tests and error messages.

Reviewers: ioeric

Subscribers: klimek, ilya-biryukov, cfe-commits

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

Added:
clang-tools-extra/trunk/unittests/clangd/Matchers.h
Removed:
clang-tools-extra/trunk/test/clangd/completion-items-kinds.test
clang-tools-extra/trunk/test/clangd/completion-priorities.test
clang-tools-extra/trunk/test/clangd/completion-qualifiers.test
clang-tools-extra/trunk/test/clangd/completion-snippet.test
Modified:
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Removed: clang-tools-extra/trunk/test/clangd/completion-items-kinds.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/completion-items-kinds.test?rev=320147&view=auto
==
--- clang-tools-extra/trunk/test/clangd/completion-items-kinds.test (original)
+++ clang-tools-extra/trunk/test/clangd/completion-items-kinds.test (removed)
@@ -1,42 +0,0 @@
-# RUN: clangd -enable-snippets -run-synchronously < %s | FileCheck %s
-# It is absolutely vital that this file has CRLF line endings.
-#
-Content-Length: 125
-
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
-Content-Length: 220
-
-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"#define
 MACRO X\nint variable;\nstruct Struct {};\n int function();\nint X = "}}}
-Content-Length: 148
-
-{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":4,"character":7}}}
-# CHECK: {"id":1,"jsonrpc":"2.0","result":{"isIncomplete":false,"items":
-#
-# Function
-# CHECK: 
{"detail":"int","filterText":"function","insertText":"function()","insertTextFormat":1,"kind":3,"label":"function()","sortText":"{{.*}}function"}
-#
-# Variable
-# CHECK: 
{"detail":"int","filterText":"variable","insertText":"variable","insertTextFormat":1,"kind":6,"label":"variable","sortText":"{{.*}}variable"}
-#
-# Keyword
-# CHECK: 
{"filterText":"int","insertText":"int","insertTextFormat":1,"kind":14,"label":"int","sortText":"{{.*}}int"}
-#
-# Struct
-# CHECK: 
{"filterText":"Struct","insertText":"Struct","insertTextFormat":1,"kind":7,"label":"Struct","sortText":"{{.*}}Struct"}
-#
-# Macro
-# CHECK: 
{"filterText":"MACRO","insertText":"MACRO","insertTextFormat":1,"kind":1,"label":"MACRO","sortText":"{{.*}}MACRO"}
-#
-# CHECK-SAME: ]}}
-Content-Length: 146
-
-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"nam"}}}
-Content-Length: 148
-
-{"jsonrpc":"2.0","id":2,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":1,"character":3}}}
-# Code pattern (unfortunately there are none in expression context)
-# CHECK-DAG: {"filterText":"namespace","insertText":"namespace 
${1:identifier}{${2:declarations}\n}","insertTextFormat":2,"kind":15,"label":"namespace
 identifier{declarations}","sortText":"{{.*}}namespace"}
-#
-Content-Length: 58
-
-{"jsonrpc":"2.0","id":3,"method":"shutdown","params":null}

Removed: clang-tools-extra/trunk/test/clangd/completion-priorities.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/completion-priorities.test?rev=320147&view=auto
==
--- clang-tools-extra/trunk/test/clangd/completion-priorities.test (original)
+++ clang-tools-extra/trunk/test/clangd/completion-priorities.test (removed)
@@ -1,73 +0,0 @@
-# RUN: clangd -pretty -run-synchronously < %s | FileCheck -strict-whitespace %s
-# It is absolutely vital that this file has CRLF line endings.
-#
-
-Content-Length: 127
-
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
-
-Content-Length: 312
-
-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"class
 Foo {\npublic:\n  void pub();\n\nprotected:\n  void prot();\n\nprivate:\n  
void priv();\n};\n\nvoid Foo::pub() {\n  this->\n}\n\nvoid test() {\n  Foo f;\n 
 f.\n}"}}}
-
-Content-Length: 151
-
-{"jsonrpc":"2.0","id":2,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":12,"character":8}}}
-#  CHECK:  "id": 2,
-# CHECK-NEXT:  "jsonrpc": "2.0",
-# CHECK-NEXT:  "result": {
-# CHECK-NEXT:"isIncomplete": false,
-# CHECK-NEXT:"items": [
-# CHECK-NEXT:  {
-# CHECK-NEXT:"detail":

[PATCH] D40952: [clangd] Convert lit code completion tests to unit-tests. NFC

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL320148: [clangd] Convert lit code completion tests to 
unit-tests. NFC (authored by sammccall).

Repository:
  rL LLVM

https://reviews.llvm.org/D40952

Files:
  clang-tools-extra/trunk/test/clangd/completion-items-kinds.test
  clang-tools-extra/trunk/test/clangd/completion-priorities.test
  clang-tools-extra/trunk/test/clangd/completion-qualifiers.test
  clang-tools-extra/trunk/test/clangd/completion-snippet.test
  clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
  clang-tools-extra/trunk/unittests/clangd/Matchers.h

Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
@@ -8,6 +8,7 @@
 //===--===//
 #include "ClangdServer.h"
 #include "Compiler.h"
+#include "Matchers.h"
 #include "Protocol.h"
 #include "TestFS.h"
 #include "gmock/gmock.h"
@@ -18,15 +19,23 @@
 // Let GMock print completion items.
 void PrintTo(const CompletionItem &I, std::ostream *O) {
   llvm::raw_os_ostream OS(*O);
-  OS << toJSON(I);
+  OS << I.label << " - " << toJSON(I);
+}
+void PrintTo(const std::vector &V, std::ostream *O) {
+  *O << "{\n";
+  for (const auto &I : V) {
+*O << "\t";
+PrintTo(I, O);
+*O << "\n";
+  }
+  *O << "}";
 }
 
 namespace {
 using namespace llvm;
 using ::testing::AllOf;
 using ::testing::Contains;
 using ::testing::ElementsAre;
-using ::testing::Matcher;
 using ::testing::Not;
 
 class IgnoreDiagnostics : public DiagnosticsConsumer {
@@ -57,22 +66,25 @@
 
 // GMock helpers for matching completion items.
 MATCHER_P(Named, Name, "") { return arg.insertText == Name; }
+MATCHER_P(Labeled, Label, "") { return arg.label == Label; }
+MATCHER_P(Kind, K, "") { return arg.kind == K; }
+MATCHER_P(PlainText, Text, "") {
+  return arg.insertTextFormat == clangd::InsertTextFormat::PlainText &&
+ arg.insertText == Text;
+}
+MATCHER_P(Snippet, Text, "") {
+  return arg.insertTextFormat == clangd::InsertTextFormat::Snippet &&
+ arg.insertText == Text;
+}
 // Shorthand for Contains(Named(Name)).
 Matcher &> Has(std::string Name) {
   return Contains(Named(std::move(Name)));
 }
-MATCHER(IsDocumented, "") { return !arg.documentation.empty(); }
-MATCHER(IsSnippet, "") {
-  return arg.kind == clangd::CompletionItemKind::Snippet;
+Matcher &> Has(std::string Name,
+ CompletionItemKind K) {
+  return Contains(AllOf(Named(std::move(Name)), Kind(K)));
 }
-// This is hard to write as a function, because matchers may be polymorphic.
-#define EXPECT_IFF(condition, value, matcher)  \
-  do { \
-if (condition) \
-  EXPECT_THAT(value, matcher); \
-else   \
-  EXPECT_THAT(value, ::testing::Not(matcher)); \
-  } while (0)
+MATCHER(IsDocumented, "") { return !arg.documentation.empty(); }
 
 CompletionList completions(StringRef Text,
clangd::CodeCompleteOptions Opts = {}) {
@@ -133,93 +145,97 @@
 }
 
 void TestAfterDotCompletion(clangd::CodeCompleteOptions Opts) {
-  auto Results = completions(R"cpp(
-#define MACRO X
+  auto Results = completions(
+  R"cpp(
+  #define MACRO X
 
-int global_var;
+  int global_var;
 
-int global_func();
+  int global_func();
 
-struct GlobalClass {};
+  struct GlobalClass {};
 
-struct ClassWithMembers {
-  /// Doc for method.
-  int method();
+  struct ClassWithMembers {
+/// Doc for method.
+int method();
 
-  int field;
-private:
-  int private_field;
-};
+int field;
+  private:
+int private_field;
+  };
 
-int test() {
-  struct LocalClass {};
+  int test() {
+struct LocalClass {};
 
-  /// Doc for local_var.
-  int local_var;
+/// Doc for local_var.
+int local_var;
 
-  ClassWithMembers().^
-}
-)cpp",
- Opts)
- .items;
+ClassWithMembers().^
+  }
+  )cpp",
+  Opts);
 
   // Class members. The only items that must be present in after-dot
   // completion.
-  EXPECT_THAT(Results, AllOf(Has(Opts.EnableSnippets ? "method()" : "method"),
- Has("field")));
-  EXPECT_IFF(Opts.IncludeIneligibleResults, Results, Has("private_field"));
+  EXPECT_THAT(
+  Results.items,
+  AllOf(Has(Opts.EnableSnippets ? "method()" : "method"), Has("field")));
+  EXPECT_IFF(Opts.IncludeIneligibleResults, Results.items,
+ Has("pri

[PATCH] D14104: clang-format: Add an additional value to AlignAfterOpenBracket: AlwaysBreak.

[Paweł Bylica via Phabricator via cfe-commits]

chfast added inline comments.



Comment at: cfe/trunk/docs/ClangFormatStyleOptions.rst:173
+
+  someLongFunction(argument1,
+  argument2);

I think this example is not achievable. clang-format seems to prefer putting 
all arguments on the next line if they all fit there. So this one will be 
formatted the same as the example for `AlwaysBreak`. 


Repository:
  rL LLVM

https://reviews.llvm.org/D14104



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

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov requested changes to this revision.
ilya-biryukov added a comment.
This revision now requires changes to proceed.

This looks useful. Could we avoid creating the `OverlayFileSystem`, though?




Comment at: include/clang/Frontend/PrecompiledPreamble.h:109
 
+  std::chrono::steady_clock::time_point getCreationTimePoint() const {
+return CreationTimePoint;

Having this time stamp in the interface of `PrecompiledPreamble` doesn't really 
makes sense.

I propose we remove this method and test in a different way instead. 

For example, we could add a counter to `ASTUnit` that increases when preamble 
is built and check this counter.
Or we could add a unit-test that uses `PrecompiledPreamble` directly.



Comment at: lib/Frontend/PrecompiledPreamble.cpp:437
 
+  vfs::OverlayFileSystem OFS(VFS);
+  IntrusiveRefCntPtr IMFS(

Can we do the same thing without creating an additional `OverlayFileSystem`?

If we add a separate map to check for those:
```
llvm::StringMap OverriddenFilesWithoutFS.
// Populate the map with paths not existing in VFS.

for (const auto &F : FilesInPreamble) {
vfs::Status Status;
if (!moveOnNoError(OFS.status(F.first()), Status)) {
  // If we can't stat the file, check whether it was overriden
  auto It = OverriddenFilesWithoutFS[F.first()];
  if (It == OverriddenFilesWithoutFS.end())
return false;  
  //.
}
//..

}
```



Comment at: lib/Frontend/PrecompiledPreamble.cpp:444
 vfs::Status Status;
-if (!moveOnNoError(VFS->status(RB.first), Status))
-  return false;
-
+assert(moveOnNoError(IMFS->status(RB.first), Status));
 OverriddenFiles[Status.getUniqueID()] =

`assert` macro is a no-op when `NDEBUG` is defined.
One must never put an operation with side-effects inside `assert`.


Repository:
  rC Clang

https://reviews.llvm.org/D41005



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

Re: [PATCH] D40548: [clangd] Symbol index interfaces and index-based code completion.

[Marc-André Laperle via cfe-commits]

A Hangouts meeting sounds good! Yes, let's arrange via emails.


From: Haojian Wu via Phabricator 
Sent: Friday, December 8, 2017 7:14:42 AM
To: ioe...@google.com; Marc-André Laperle; sammcc...@google.com
Cc: hok...@google.com; kli...@google.com; mgo...@gentoo.org; 
ibiryu...@google.com; cfe-commits@lists.llvm.org
Subject: [PATCH] D40548: [clangd] Symbol index interfaces and index-based code 
completion.

hokein added a comment.

Thanks for the feedback, Marc!

Yeah, I think the ClangdIndexDataSymbol and ClangdIndexDataOccurrence are 
something similar to what https://reviews.llvm.org/D40897 trying to address, 
maybe put the discussion there? Before that, I think having a sym meeting is a 
good idea to keep us in the same page.

In https://reviews.llvm.org/D40548#949279, @ioeric wrote:

> >
>
> I think some of the ideas here could be useful. This patch focuses mostly on 
> index interfaces and https://reviews.llvm.org/D40897 emphasizes on the design 
> of symbol structure. The way symbols are stored and used in this patch is 
> likely to change depending on how https://reviews.llvm.org/D40897 goes.
>
> > The "Clangd" prefix adds a bit much of clutter so maybe it should be 
> > removed.  I think the main points are that having generic 
> > foreachSymbols/foreachOccurrence with callbacks is well suited to implement 
> > multiple features with minimal copying.
>
> Although I'm not sure if `foreachSymbols`/... would be feasible for all 
> indexes yet, we do plan to switch to callback-based index interfaces, which 
> Sam also proposed in the review comments.
>
> There have been some offline discussions happening around clangd's indexing, 
> and sorry that we have not been able to keep you up to date. I think it might 
> be more efficient if we could meet via VC/Hangouts and sync on our designs. 
> If you don't mind a meeting, I am happy to arrange it via emails.





Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40548



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

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

[Cameron via Phabricator via cfe-commits]

cameron314 added a comment.

It's been a while since I was in this code, but I seem to recall the file 
needed to exist on disk in order to uniquely identify it (via inode). Does this 
break the up-to-date check?


Repository:
  rC Clang

https://reviews.llvm.org/D41005



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

[PATCH] D40948: Switch Clang's default C++ language target to C++14

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1733
   // The PS4 uses C++11 as the default C++ standard.
-  if (T.isPS4())
-LangStd = LangStandard::lang_gnucxx11;
-  else
-LangStd = LangStandard::lang_gnucxx98;
+  LangStd = LangStandard::lang_gnucxx14;
   break;

filcab wrote:
> Why are you changing the PS4 default too?
Paul Robinson indicated that it was feasible back in March: 
http://lists.llvm.org/pipermail/cfe-dev/2017-March/052986.html. If that's 
changed I'd be happy to put it back to C++11, but he's one of the main people 
(rightly) bugging me about this so I'd be slightly surprised.

I also notice the comment crept back in. Bother.


Repository:
  rC Clang

https://reviews.llvm.org/D40948



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

[PATCH] D40897: [clangd] Introduce a "Symbol" class.

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added a comment.

(From https://reviews.llvm.org/D40548) Here's the interface for querying the 
index that I am using right now. It's meant to be able to retrieve from any 
kind of "backend", i.e. in-memory, ClangdIndexDataStore, libIndexStore, etc. I 
was able to implement "Open Workspace Symbol" (which is close to code 
completion in concept), Find References and Find Definitions.

  using USR = llvm::SmallString<256>;
  
  class ClangdIndexDataOccurrence;
  
  class ClangdIndexDataSymbol {
  public:
virtual index::SymbolKind getKind() = 0;
/// For example, for mynamespace::myclass::mymethod, this will be
/// mymethod.
virtual std::string getName() = 0;
/// For example, for mynamespace::myclass::mymethod, this will be
/// mynamespace::myclass::
virtual std::string getQualifier() = 0;
virtual std::string getUsr() = 0;
  
virtual void foreachOccurrence(index::SymbolRoleSet Roles, 
llvm::function_ref Receiver) = 0;
  
virtual ~ClangdIndexDataSymbol() = default;
  };
  
  class ClangdIndexDataOccurrence {
  public:
enum class OccurrenceType : uint16_t {
   OCCURRENCE,
   DEFINITION_OCCURRENCE
 };
  
virtual OccurrenceType getKind() const = 0;
virtual std::string getPath() = 0;
/// Get the start offset of the symbol occurrence. The SourceManager can be
/// used for implementations that need to convert from a line/column
/// representation to an offset.
virtual uint32_t getStartOffset(SourceManager &SM) = 0;
/// Get the end offset of the symbol occurrence. The SourceManager can be
/// used for implementations that need to convert from a line/column
/// representation to an offset.
virtual uint32_t getEndOffset(SourceManager &SM) = 0;
virtual ~ClangdIndexDataOccurrence() = default;
//TODO: Add relations
  
static bool classof(const ClangdIndexDataOccurrence *O) { return 
O->getKind() == OccurrenceType::OCCURRENCE; }
  };
  
  /// An occurrence that also has definition with a body that requires 
additional
  /// locations to keep track of the beginning and end of the body.
  class ClangdIndexDataDefinitionOccurrence : public ClangdIndexDataOccurrence {
  public:
virtual uint32_t getDefStartOffset(SourceManager &SM) = 0;
virtual uint32_t getDefEndOffset(SourceManager &SM) = 0;
  
static bool classof(const ClangdIndexDataOccurrence *O) { return 
O->getKind() == OccurrenceType::DEFINITION_OCCURRENCE; }
  };
  
  class ClangdIndexDataProvider {
  public:
  
virtual void foreachSymbols(StringRef Pattern, 
llvm::function_ref Receiver) = 0;
virtual void foreachSymbols(const USR &Usr, 
llvm::function_ref Receiver) = 0;
  
virtual ~ClangdIndexDataProvider() = default;
  };

The "Clangd" prefix adds a bit much of clutter so maybe it should be removed. I 
think the main points are that having generic foreachSymbols/foreachOccurrence 
with callbacks is well suited to implement multiple features with minimal 
copying.




Comment at: clangd/Symbol.h:37
+// The class presents a C++ symbol, e.g. class, function.
+struct Symbol {
+  // The symbol identifier, using USR.

sammccall wrote:
> hokein wrote:
> > malaperle wrote:
> > > sammccall wrote:
> > > > hokein wrote:
> > > > > malaperle wrote:
> > > > > > I think it would be nice to have methods as an interface to get 
> > > > > > this data instead of storing them directly. So that an 
> > > > > > index-on-disk could go fetch the data. Especially the occurrences 
> > > > > > which can take a lot of memory (I'm working on a branch that does 
> > > > > > that). But perhaps defining that interface is not within the scope 
> > > > > > of this patch and could be better discussed in D40548 ?
> > > > > I agree. We can't load all the symbol occurrences into the memory 
> > > > > since they are too large. We need to design interface for the symbol 
> > > > > occurrences. 
> > > > > 
> > > > > We could discuss the interface here, but CodeCompletion is the main 
> > > > > thing which this patch focuses on. 
> > > > > We can't load all the symbol occurrences into the memory since they 
> > > > > are too large
> > > > 
> > > > I've heard this often, but never backed up by data :-)
> > > > 
> > > > Naively an array of references for a symbol could be doc ID + offset + 
> > > > length, let's say 16 bytes.
> > > > 
> > > > If a source file consisted entirely of references to 1-character 
> > > > symbols separated by punctuation (1 reference per 2 bytes) then the 
> > > > total size of these references would be 8x the size of the source file 
> > > > - in practice much less. That's not very big.
> > > > 
> > > > (Maybe there are edge cases with macros/templates, but we can keep them 
> > > > under control)
> > > I'd have to break down how much memory it used by what, I'll come back to 
> > > you on that. Indexing llvm with ClangdIndexDataStorage, which is pretty 
> > > packed is about 200MB. That's already a lot considering

[PATCH] D40897: [clangd] Introduce a "Symbol" class.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 126156.
hokein marked 6 inline comments as done.
hokein added a comment.

- reorganize files, move to index subdirectory.
- change symbol ID to a hash value, instead of couple with USR.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40897

Files:
  clangd/CMakeLists.txt
  clangd/index/CMakeLists.txt
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  unittests/clangd/CMakeLists.txt
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- /dev/null
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -0,0 +1,112 @@
+//===-- SymbolCollectorTests.cpp  ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "index/SymbolCollector.h"
+#include "clang/Index/IndexingAction.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "gtest/gtest.h"
+#include "gmock/gmock.h"
+
+#include 
+#include 
+
+using testing::UnorderedElementsAre;
+using testing::Eq;
+using testing::Field;
+
+// GMock helpers for matching Symbol.
+MATCHER_P(QName, Name, "") { return arg.QualifiedName == Name; }
+
+namespace clang {
+namespace clangd {
+
+namespace {
+class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
+ public:
+  SymbolIndexActionFactory() = default;
+
+  clang::FrontendAction *create() override {
+index::IndexingOptions IndexOpts;
+IndexOpts.SystemSymbolFilter =
+index::IndexingOptions::SystemSymbolFilterKind::All;
+IndexOpts.IndexFunctionLocals = false;
+Collector = std::make_shared();
+FrontendAction *Action =
+index::createIndexingAction(Collector, IndexOpts, nullptr).release();
+return Action;
+  }
+
+  std::shared_ptr Collector;
+};
+
+class SymbolCollectorTest : public ::testing::Test {
+public:
+  bool runSymbolCollector(StringRef HeaderCode, StringRef MainCode) {
+llvm::IntrusiveRefCntPtr InMemoryFileSystem(
+new vfs::InMemoryFileSystem);
+llvm::IntrusiveRefCntPtr Files(
+new FileManager(FileSystemOptions(), InMemoryFileSystem));
+
+const std::string FileName = "symbol.cc";
+const std::string HeaderName = "symbols.h";
+auto Factory = llvm::make_unique();
+
+tooling::ToolInvocation Invocation(
+{"symbol_collector", "-fsyntax-only", "-std=c++11", FileName},
+Factory->create(), Files.get(),
+std::make_shared());
+
+InMemoryFileSystem->addFile(HeaderName, 0,
+llvm::MemoryBuffer::getMemBuffer(HeaderCode));
+
+std::string Content = "#include\"" + std::string(HeaderName) + "\"";
+Content += "\n" + MainCode.str();
+InMemoryFileSystem->addFile(FileName, 0,
+llvm::MemoryBuffer::getMemBuffer(Content));
+Invocation.run();
+Symbols = Factory->Collector->takeSymbols();
+
+EXPECT_EQ(FileName, Factory->Collector->getFilename());
+return true;
+  }
+
+protected:
+  SymbolSlab Symbols;
+};
+
+TEST_F(SymbolCollectorTest, CollectSymbol) {
+  const std::string Header = R"(
+class Foo {
+  void f();
+};
+void f1();
+inline void f2() {}
+  )";
+  const std::string Main = R"(
+namespace {
+void ff() {} // ignore
+}
+void f1() {}
+  )";
+  runSymbolCollector(Header, Main);
+  EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Foo::f"),
+QName("f1"), QName("f2")));
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: unittests/clangd/CMakeLists.txt
===
--- unittests/clangd/CMakeLists.txt
+++ unittests/clangd/CMakeLists.txt
@@ -15,12 +15,14 @@
   JSONExprTests.cpp
   TestFS.cpp
   TraceTests.cpp
+  SymbolCollectorTests.cpp
   )
 
 target_link_libraries(ClangdTests
   PRIVATE
   clangBasic
   clangDaemon
+  clangdIndex
   clangFormat
   clangFrontend
   clangSema
Index: clangd/index/SymbolCollector.h
===
--- /dev/null
+++ clangd/index/SymbolCollector.h
@@ -0,0 +1,50 @@
+//===--- SymbolCollector.h ---*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. S

Re: r305110 - [ODRHash] Add support for TemplateArgument types.

[Vassil Vassilev via cfe-commits]

Hi Richard,

  Is there a way to get an ODRHashing which is stable across 
translation units? I'd like to use the TemplateArgument ODRHash to 
lookup template specializations and deserialize them only if they are 
required.


Many thanks!
Vassil
On 6/9/17 11:00 PM, Richard Trieu via cfe-commits wrote:

Author: rtrieu
Date: Fri Jun  9 16:00:10 2017
New Revision: 305110

URL: http://llvm.org/viewvc/llvm-project?rev=305110&view=rev
Log:
[ODRHash] Add support for TemplateArgument types.

Recommit r304592 that was reverted in r304618.  r305104 should have fixed the
issue.

Modified:
 cfe/trunk/lib/AST/ODRHash.cpp
 cfe/trunk/test/Modules/odr_hash.cpp

Modified: cfe/trunk/lib/AST/ODRHash.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ODRHash.cpp?rev=305110&r1=305109&r2=305110&view=diff
==
--- cfe/trunk/lib/AST/ODRHash.cpp (original)
+++ cfe/trunk/lib/AST/ODRHash.cpp Fri Jun  9 16:00:10 2017
@@ -140,7 +140,25 @@ void ODRHash::AddTemplateName(TemplateNa
}
  }
  
-void ODRHash::AddTemplateArgument(TemplateArgument TA) {}

+void ODRHash::AddTemplateArgument(TemplateArgument TA) {
+  auto Kind = TA.getKind();
+  ID.AddInteger(Kind);
+
+  switch (Kind) {
+  case TemplateArgument::Null:
+  case TemplateArgument::Declaration:
+  case TemplateArgument::NullPtr:
+  case TemplateArgument::Integral:
+  case TemplateArgument::Template:
+  case TemplateArgument::TemplateExpansion:
+  case TemplateArgument::Expression:
+  case TemplateArgument::Pack:
+break;
+  case TemplateArgument::Type:
+AddQualType(TA.getAsType());
+break;
+  }
+}
  void ODRHash::AddTemplateParameterList(const TemplateParameterList *TPL) {}
  
  void ODRHash::clear() {


Modified: cfe/trunk/test/Modules/odr_hash.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/odr_hash.cpp?rev=305110&r1=305109&r2=305110&view=diff
==
--- cfe/trunk/test/Modules/odr_hash.cpp (original)
+++ cfe/trunk/test/Modules/odr_hash.cpp Fri Jun  9 16:00:10 2017
@@ -900,6 +900,24 @@ S2 s2;
  #endif
  }
  
+namespace TemplateArgument {

+#if defined(FIRST)
+template struct U1 {};
+struct S1 {
+  U1 u;
+};
+#elif defined(SECOND)
+template struct U1 {};
+struct S1 {
+  U1 u;
+};
+#else
+S1 s1;
+// expected-error@first.h:* {{'TemplateArgument::S1::u' from module 
'FirstModule' is not present in definition of 'TemplateArgument::S1' in module 
'SecondModule'}}
+// expected-note@second.h:* {{declaration of 'u' does not match}}
+#endif
+}
+
  // Interesting cases that should not cause errors.  struct S should not error
  // while struct T should error at the access specifier mismatch at the end.
  namespace AllDecls {


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



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

[PATCH] D40897: [clangd] Introduce a "Symbol" class.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Reorganizing the source files made all the comments invalid in the latest 
version :(. Feel free to comment on the old version or the new version.




Comment at: clangd/Symbol.h:23
+  // The path of the source file where a symbol occurs.
+  std::string FilePath;
+  // The offset to the first character of the symbol from the beginning of the

sammccall wrote:
> hokein wrote:
> > malaperle wrote:
> > > malaperle wrote:
> > > > sammccall wrote:
> > > > > ioeric wrote:
> > > > > > Is this relative or absolute?
> > > > > Having every symbol own a copy of the filepath seems wasteful.
> > > > > It seems likely that the index will have per-file information too, so 
> > > > > this representation should likely be a key to that. Hash of the 
> > > > > filepath might work?
> > > > How we model it is that a symbol doesn't have a "location", but its 
> > > > occurrence do. One could consider the location of a symbol to be either 
> > > > its declaration occurrence (SymbolRole::Declaration) or its definition 
> > > > (SymbolRole::Definition).
> > > > What we do to get the location path is each occurrence has a pointer (a 
> > > > "database" pointer, but it doesn't matter) to a file entry and then we 
> > > > get the path from the entry.
> > > > 
> > > > So conceptually, it works a bit like this (although it fetches 
> > > > information on disk).
> > > > ```
> > > > class IndexOccurrence {
> > > > IndexOccurrence *FilePtr;
> > > > 
> > > > std::string Occurrence::getPath() {
> > > >   return FilePtr->getPath();
> > > > }
> > > > };
> > > > ```
> > > Oops, wrong type for the field, it should have been:
> > > ```
> > > class IndexOccurrence {
> > > IndexFile *FilePtr;
> > > 
> > > std::string Occurrence::getPath() {
> > >   return FilePtr->getPath();
> > > }
> > > };
> > > ```
> > > Is this relative or absolute?
> > 
> >  Whether the file path is relative or absolute depends on the build system, 
> > the file path could be relative (for header files) or absolute (for .cc 
> > files).
> > 
> > > How we model it is that a symbol doesn't have a "location", but its 
> > > occurrence do.
> > 
> > We will also have a SymbolOccurence structure alongside with Symbol (but it 
> > is not  in this patch). The "Location" will be a part of SymbolOccurrence.
> >  
> > Whether the file path is relative or absolute depends on the build system, 
> > the file path could be relative (for header files) or absolute (for .cc 
> > files).
> 
> I'm not convinced this actually works. There's multiple codepaths to the 
> index, how can we ensure we don't end up using inconsistent paths?
> e.g. we open up a project that includes a system header using a relative 
> path, and then open up that system header from file->open (editor knows only 
> the absolute path) and do "find references".
> 
> I think we need to canonicalize the paths. Absolute is probably easiest.
Absolute path for .cc file is fine, I was a bit concerned about the path for .h 
file, especially we might use it in `#include`, but we can figure out later.  
Changed to absolute file path.



Comment at: clangd/Symbol.h:51
+  //   * For classes, the canonical location is where they are defined.
+  SymbolLocation CanonicalLocation;
+  // Information for code completion.

sammccall wrote:
> hokein wrote:
> > ioeric wrote:
> > > For functions and classes, should we store both declaration and 
> > > definition locations?
> > I think the symbol occurrences would include both declaration and 
> > definition locations. 
> > 
> > The `CanonicalLocation` is providing a fast/convenient way to find the most 
> > interested location of the symbol (e.g. for code navigation, or include the 
> > missing path for a symbol), without symbol occurrences. 
> I'd be +1 on including both a definition location (if known) and a preferred 
> declaration location, because there's enough use cases that might want 
> definition even if it's not the preferred declaration.
> 
> But i'm fine if we want to omit the separate definition for now. In that 
> case, call this CanonicalDeclaration?
OK, changed it to `CanonicalDeclarationLoc`, and added a FIXME for the 
definition.



Comment at: clangd/Symbol.h:68
+// changed.
+class SymbolCollector : public index::IndexDataConsumer {
+public:

sammccall wrote:
> hokein wrote:
> > sammccall wrote:
> > > Please pull this into a separate file. Someone providing e.g. symbols 
> > > from a YAML file shouldn't need to pull in AST stuff.
> > > Mabye `IndexFromAST`, which would sort nicely next to `Index`?
> > I can see various meaning of "Index" here:
> > 
> > 1. `Index` in `index::IndexDataConsumer`, which collects and contructs all 
> > symbols by traversing the AST.
> > 2. `Index` term using in clangd, specially for build index on top of these 
> > collected symbols.
> > 
> > I think we should be consistent the index for 2), and `SymbolCollector` is 
> > more d

[PATCH] D41012: [OpenMP] Further adjustments of nvptx runtime functions

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.
Herald added subscribers: cfe-commits, jholewinski.

Pass in default value of 1, similar to previous commit 
https://reviews.llvm.org/rL318836.


Repository:
  rC Clang

https://reviews.llvm.org/D41012

Files:
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  test/OpenMP/nvptx_data_sharing.cpp
  test/OpenMP/nvptx_target_teams_codegen.cpp

Index: test/OpenMP/nvptx_target_teams_codegen.cpp
===
--- test/OpenMP/nvptx_target_teams_codegen.cpp
+++ test/OpenMP/nvptx_target_teams_codegen.cpp
@@ -60,7 +60,7 @@
   //
   // CHECK: [[AWAIT_WORK]]
   // CHECK: call void @llvm.nvvm.barrier0()
-  // CHECK: [[KPR:%.+]] = call i1 @__kmpc_kernel_parallel(i8** [[OMP_WORK_FN]], i8*** %shared_args)
+  // CHECK: [[KPR:%.+]] = call i1 @__kmpc_kernel_parallel(i8** [[OMP_WORK_FN]], i8*** %shared_args, i16 1)
   // CHECK: [[KPRB:%.+]] = zext i1 [[KPR]] to i8
   // store i8 [[KPRB]], i8* [[OMP_EXEC_STATUS]], align 1
   // CHECK: [[WORK:%.+]] = load i8*, i8** [[OMP_WORK_FN]],
@@ -148,7 +148,7 @@
   //
   // CHECK: [[AWAIT_WORK]]
   // CHECK: call void @llvm.nvvm.barrier0()
-  // CHECK: [[KPR:%.+]] = call i1 @__kmpc_kernel_parallel(i8** [[OMP_WORK_FN]], i8*** %shared_args)
+  // CHECK: [[KPR:%.+]] = call i1 @__kmpc_kernel_parallel(i8** [[OMP_WORK_FN]], i8*** %shared_args, i16 1)
   // CHECK: [[KPRB:%.+]] = zext i1 [[KPR]] to i8
   // store i8 [[KPRB]], i8* [[OMP_EXEC_STATUS]], align 1
   // CHECK: [[WORK:%.+]] = load i8*, i8** [[OMP_WORK_FN]],
Index: test/OpenMP/nvptx_data_sharing.cpp
===
--- test/OpenMP/nvptx_data_sharing.cpp
+++ test/OpenMP/nvptx_data_sharing.cpp
@@ -24,15 +24,15 @@
 
 // CK1: define internal void @__omp_offloading_{{.*}}test_ds{{.*}}worker(){{.*}}{
 // CK1: [[SHAREDARGS:%.+]] = alloca i8**
-// CK1: call i1 @__kmpc_kernel_parallel(i8** %work_fn, i8*** [[SHAREDARGS]])
+// CK1: call i1 @__kmpc_kernel_parallel(i8** %work_fn, i8*** [[SHAREDARGS]], i16 1)
 // CK1: [[SHARGSTMP:%.+]] = load i8**, i8*** [[SHAREDARGS]]
 // CK1: call void @__omp_outlined___wrapper{{.*}}({{.*}}, i8** [[SHARGSTMP]])
 
 /// = In the kernel function = ///
 
 // CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}()
 // CK1: [[SHAREDARGS1:%.+]] = alloca i8**
-// CK1: call void @__kmpc_kernel_prepare_parallel({{.*}}, i8*** [[SHAREDARGS1]], i32 1)
+// CK1: call void @__kmpc_kernel_prepare_parallel({{.*}}, i8*** [[SHAREDARGS1]], i32 1, i16 1)
 // CK1: [[SHARGSTMP1:%.+]] = load i8**, i8*** [[SHAREDARGS1]]
 // CK1: [[SHARGSTMP2:%.+]] = getelementptr inbounds i8*, i8** [[SHARGSTMP1]]
 // CK1: [[SHAREDVAR:%.+]] = bitcast i32* {{.*}} to i8*
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -33,10 +33,11 @@
   /// \brief Call to void __kmpc_spmd_kernel_deinit();
   OMPRTL_NVPTX__kmpc_spmd_kernel_deinit,
   /// \brief Call to void __kmpc_kernel_prepare_parallel(void
-  /// *outlined_function, void ***args, kmp_int32 nArgs);
+  /// *outlined_function, void ***args, kmp_int32 nArgs, int16_t
+  /// IsOMPRuntimeInitialized);
   OMPRTL_NVPTX__kmpc_kernel_prepare_parallel,
   /// \brief Call to bool __kmpc_kernel_parallel(void **outlined_function, void
-  /// ***args);
+  /// ***args, int16_t IsOMPRuntimeInitialized);
   OMPRTL_NVPTX__kmpc_kernel_parallel,
   /// \brief Call to void __kmpc_kernel_end_parallel();
   OMPRTL_NVPTX__kmpc_kernel_end_parallel,
@@ -521,7 +522,9 @@
   // Set up shared arguments
   Address SharedArgs =
   CGF.CreateDefaultAlignTempAlloca(CGF.Int8PtrPtrTy, "shared_args");
-  llvm::Value *Args[] = {WorkFn.getPointer(), SharedArgs.getPointer()};
+  // TODO: Optimize runtime initialization and pass in correct value.
+  llvm::Value *Args[] = {WorkFn.getPointer(), SharedArgs.getPointer(),
+ /*RequiresOMPRuntime=*/Bld.getInt16(1)};
   llvm::Value *Ret = CGF.EmitRuntimeCall(
   createNVPTXRuntimeFunction(OMPRTL_NVPTX__kmpc_kernel_parallel), Args);
   Bld.CreateStore(Bld.CreateZExt(Ret, CGF.Int8Ty), ExecStatus);
@@ -637,18 +640,21 @@
   }
   case OMPRTL_NVPTX__kmpc_kernel_prepare_parallel: {
 /// Build void __kmpc_kernel_prepare_parallel(
-/// void *outlined_function, void ***args, kmp_int32 nArgs);
+/// void *outlined_function, void ***args, kmp_int32 nArgs, int16_t
+/// IsOMPRuntimeInitialized);
 llvm::Type *TypeParams[] = {CGM.Int8PtrTy,
-CGM.Int8PtrPtrTy->getPointerTo(0), CGM.Int32Ty};
+CGM.Int8PtrPtrTy->getPointerTo(0), CGM.Int32Ty,
+CGM.Int16Ty};
 llvm::FunctionType *FnTy =
 llvm::FunctionType::get(CGM.VoidTy, TypeParams, /*isVarArg*/ false);
 RTLFn = CGM.CreateRuntimeFunction(FnTy, "__kmpc_kernel_prepare_parallel");
 break;
   }
   case OMPRTL_NVPTX__kmpc_kernel_parallel: {
-/// B

[PATCH] D40968: [OpenMP] Diagnose function name on the link clause

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/Sema/SemaOpenMP.cpp:12671
+void Sema::checkDeclIsAllowedInOpenMPTarget(Expr *E, Decl *D,
+const DeclarationNameInfo *Id) {
   if (!D || D->isInvalidDecl())

You can get `DeclarationNameInfo` from the `FunctionDecl`:
```
FD->getNameInfo()
```


https://reviews.llvm.org/D40968



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

[PATCH] D41012: [OpenMP] Further adjustments of nvptx runtime functions

[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


Repository:
  rC Clang

https://reviews.llvm.org/D41012



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

r320162 - Revert "Unify implementation of our two different flavours of -Wtautological-compare."

[Hans Wennborg via cfe-commits]

Author: hans
Date: Fri Dec  8 08:54:08 2017
New Revision: 320162

URL: http://llvm.org/viewvc/llvm-project?rev=320162&view=rev
Log:
Revert "Unify implementation of our two different flavours of 
-Wtautological-compare."

> Unify implementation of our two different flavours of -Wtautological-compare.
>
> In so doing, fix a handful of remaining bugs where we would report false
> positives or false negatives if we promote a signed value to an unsigned type
> for the comparison.

This caused a new warning in Chromium:

../../base/trace_event/trace_log.cc:1545:29: error: comparison of constant 64
with expression of type 'unsigned int' is always true
[-Werror,-Wtautological-constant-out-of-range-compare]
  DCHECK(handle.event_index < TraceBufferChunk::kTraceBufferChunkSize);
 ~~ ^ ~~~

The 'unsigned int' is really a 6-bit bitfield, which is why it's always
less than 64.

I thought we didn't use to warn (with out-of-range-compare) when comparing
against the boundaries of a type?

Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/tautological-constant-compare.c
cfe/trunk/test/Sema/tautological-constant-enum-compare.c
cfe/trunk/test/SemaCXX/compare.cpp

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=320162&r1=320161&r2=320162&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Dec  8 08:54:08 2017
@@ -8662,113 +8662,54 @@ static bool isKnownToHaveUnsignedValue(E
 }
 
 namespace {
-/// The promoted range of values of a type. In general this has the
-/// following structure:
-///
-/// |---| . . . |---|
-/// ^   ^   ^   ^
-///Min   HoleMin  HoleMax  Max
-///
-/// ... where there is only a hole if a signed type is promoted to unsigned
-/// (in which case Min and Max are the smallest and largest representable
-/// values).
-struct PromotedRange {
-  // Min, or HoleMax if there is a hole.
-  llvm::APSInt PromotedMin;
-  // Max, or HoleMin if there is a hole.
-  llvm::APSInt PromotedMax;
-
-  PromotedRange(IntRange R, unsigned BitWidth, bool Unsigned) {
-if (R.Width == 0)
-  PromotedMin = PromotedMax = llvm::APSInt(BitWidth, Unsigned);
-else {
-  PromotedMin = llvm::APSInt::getMinValue(R.Width, R.NonNegative)
-.extOrTrunc(BitWidth);
-  PromotedMin.setIsUnsigned(Unsigned);
-
-  PromotedMax = llvm::APSInt::getMaxValue(R.Width, R.NonNegative)
-.extOrTrunc(BitWidth);
-  PromotedMax.setIsUnsigned(Unsigned);
-}
-  }
 
-  // Determine whether this range is contiguous (has no hole).
-  bool isContiguous() const { return PromotedMin <= PromotedMax; }
+enum class LimitType {
+  Max = 1U << 0U,  // e.g. 32767 for short
+  Min = 1U << 1U,  // e.g. -32768 for short
+  Both = Max | Min // When the value is both the Min and the Max limit at the
+   // same time; e.g. in C++, A::a in enum A { a = 0 };
+};
 
-  // Where a constant value is within the range.
-  enum ComparisonResult {
-LT = 0x1,
-LE = 0x2,
-GT = 0x4,
-GE = 0x8,
-EQ = 0x10,
-NE = 0x20,
-InRangeFlag = 0x40,
-
-Less = LE | LT | NE,
-Min = LE | InRangeFlag,
-InRange = InRangeFlag,
-Max = GE | InRangeFlag,
-Greater = GE | GT | NE,
+} // namespace
 
-OnlyValue = LE | GE | EQ | InRangeFlag,
-InHole = NE
-  };
+/// Checks whether Expr 'Constant' may be the
+/// std::numeric_limits<>::max() or std::numeric_limits<>::min()
+/// of the Expr 'Other'. If true, then returns the limit type (min or max).
+/// The Value is the evaluation of Constant
+static llvm::Optional IsTypeLimit(Sema &S, Expr *Constant,
+ Expr *Other,
+ const llvm::APSInt &Value) {
+  if (IsEnumConstOrFromMacro(S, Constant))
+return llvm::Optional();
 
-  ComparisonResult compare(const llvm::APSInt &Value) const {
-assert(Value.getBitWidth() == PromotedMin.getBitWidth() &&
-   Value.isUnsigned() == PromotedMin.isUnsigned());
-if (!isContiguous()) {
-  assert(Value.isUnsigned() && "discontiguous range for signed compare");
-  if (Value.isMinValue()) return Min;
-  if (Value.isMaxValue()) return Max;
-  if (Value >= PromotedMin) return InRange;
-  if (Value <= PromotedMax) return InRange;
-  return InHole;
-}
+  if (isKnownToHaveUnsignedValue(Other) && Value == 0)
+return LimitType::Min;
 
-switch (llvm::APSInt::compareValues(Value, PromotedMin)) {
-case -1: return Less;
-case 0: return PromotedMin == PromotedMax ? OnlyValue : Min;
-case 1:
-  switch (llvm::APSInt::compareValues(Value, PromotedMax)) {
-  case -1: return InRange;
-  case 0: return Max;

Re: r320124 - Fold together the in-range and out-of-range portions of -Wtautological-compare.

[Hans Wennborg via cfe-commits]

Sorry, it seems it was r320124 that caused this; I shouldn't be
sheriffing after-hours.

I've reverted that one in r320162.

On Thu, Dec 7, 2017 at 9:20 PM, Hans Wennborg  wrote:
> I've reverted in r320133 since it caused new warnings in Chromium that
> I'm not sure were intentional. See comment on the revert.
>
> On Thu, Dec 7, 2017 at 5:00 PM, Richard Smith via cfe-commits
>  wrote:
>> Author: rsmith
>> Date: Thu Dec  7 17:00:27 2017
>> New Revision: 320124
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=320124&view=rev
>> Log:
>> Fold together the in-range and out-of-range portions of 
>> -Wtautological-compare.
>>
>> Modified:
>> cfe/trunk/lib/Sema/SemaChecking.cpp
>>
>> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=320124&r1=320123&r2=320124&view=diff
>> ==
>> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Dec  7 17:00:27 2017
>> @@ -8801,12 +8801,7 @@ static bool CheckTautologicalComparison(
>>  Expr *Constant, Expr *Other,
>>  const llvm::APSInt &Value,
>>  bool RhsConstant) {
>> -  // Disable warning in template instantiations
>> -  // and only analyze <, >, <= and >= operations.
>> -  if (S.inTemplateInstantiation() || !E->isRelationalOp())
>> -return false;
>> -
>> -  if (IsEnumConstOrFromMacro(S, Constant))
>> +  if (S.inTemplateInstantiation())
>>  return false;
>>
>>Expr *OriginalOther = Other;
>> @@ -8833,94 +8828,23 @@ static bool CheckTautologicalComparison(
>>OtherRange.Width =
>>std::min(Bitfield->getBitWidthValue(S.Context), OtherRange.Width);
>>
>> -  // Check whether the constant value can be represented in OtherRange. Bail
>> -  // out if so; this isn't an out-of-range comparison.
>> +  // Determine the promoted range of the other type and see if a comparison 
>> of
>> +  // the constant against that range is tautological.
>>PromotedRange OtherPromotedRange(OtherRange, Value.getBitWidth(),
>> Value.isUnsigned());
>> -
>>auto Cmp = OtherPromotedRange.compare(Value);
>> -  if (Cmp != PromotedRange::Min && Cmp != PromotedRange::Max &&
>> -  Cmp != PromotedRange::OnlyValue)
>> -return false;
>> -
>>auto Result = PromotedRange::constantValue(E->getOpcode(), Cmp, 
>> RhsConstant);
>>if (!Result)
>>  return false;
>>
>> -  // Should be enough for uint128 (39 decimal digits)
>> -  SmallString<64> PrettySourceValue;
>> -  llvm::raw_svector_ostream OS(PrettySourceValue);
>> -  OS << Value;
>> -
>> -  // FIXME: We use a somewhat different formatting for the cases involving
>> -  // boolean values for historical reasons. We should pick a consistent way
>> -  // of presenting these diagnostics.
>> -  if (Other->isKnownToHaveBooleanValue()) {
>> -S.DiagRuntimeBehavior(
>> -  E->getOperatorLoc(), E,
>> -  S.PDiag(diag::warn_tautological_bool_compare)
>> -  << OS.str() << classifyConstantValue(Constant)
>> -  << OtherT << !OtherT->isBooleanType() << *Result
>> -  << E->getLHS()->getSourceRange() << 
>> E->getRHS()->getSourceRange());
>> -return true;
>> -  }
>> -
>> -  unsigned Diag = (isKnownToHaveUnsignedValue(OriginalOther) && Value == 0)
>> -  ? (HasEnumType(OriginalOther)
>> - ? 
>> diag::warn_unsigned_enum_always_true_comparison
>> - : diag::warn_unsigned_always_true_comparison)
>> -  : diag::warn_tautological_constant_compare;
>> -
>> -  S.Diag(E->getOperatorLoc(), Diag)
>> -  << RhsConstant << OtherT << E->getOpcodeStr() << OS.str() << *Result
>> -  << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
>> -
>> -  return true;
>> -}
>> -
>> -static bool DiagnoseOutOfRangeComparison(Sema &S, BinaryOperator *E,
>> - Expr *Constant, Expr *Other,
>> - const llvm::APSInt &Value,
>> - bool RhsConstant) {
>> -  // Disable warning in template instantiations.
>> -  if (S.inTemplateInstantiation())
>> -return false;
>> -
>> -  Constant = Constant->IgnoreParenImpCasts();
>> -  Other = Other->IgnoreParenImpCasts();
>> -
>> -  // TODO: Investigate using GetExprRange() to get tighter bounds
>> -  // on the bit ranges.
>> -  QualType OtherT = Other->getType();
>> -  if (const auto *AT = OtherT->getAs())
>> -OtherT = AT->getValueType();
>> -  IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
>> -
>> -  // Whether we're treating Other as being a bool because of the form of
>> -  // expression despite it having another type (typically 'int' in C).
>> -  bool OtherIsBooleanDespiteType =

[PATCH] D41016: [Sema] Fix crash in unused-lambda-capture warning for VLAs

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons created this revision.
Herald added a subscriber: cfe-commits.

Clang was crashing when diagnosing an unused-lambda-capture for a VLA because
From.getVariable() is null for the capture of a VLA bound.
Warning about the VLA bound capture is not helpful, so only warn for the VLA
itself.

Fixes: PR3


Repository:
  rC Clang

https://reviews.llvm.org/D41016

Files:
  lib/Sema/SemaLambda.cpp
  test/SemaCXX/warn-unused-lambda-capture.cpp


Index: test/SemaCXX/warn-unused-lambda-capture.cpp
===
--- test/SemaCXX/warn-unused-lambda-capture.cpp
+++ test/SemaCXX/warn-unused-lambda-capture.cpp
@@ -191,3 +191,12 @@
 void test_use_template() {
   test_templated(); // expected-note{{in instantiation of function 
template specialization 'test_templated' requested here}}
 }
+
+namespace pr3 {
+int a;
+void b() {
+  int c[a];
+  auto vla_used = [&c] { return c[0]; };
+  auto vla_unused = [&c] {}; // expected-warning{{lambda capture 'c' is not 
used}}
+}
+} // namespace pr3
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -1481,6 +1481,9 @@
   if (CaptureHasSideEffects(From))
 return;
 
+  if (From.isVLATypeCapture())
+return;
+
   auto diag = Diag(From.getLocation(), diag::warn_unused_lambda_capture);
   if (From.isThisCapture())
 diag << "'this'";


Index: test/SemaCXX/warn-unused-lambda-capture.cpp
===
--- test/SemaCXX/warn-unused-lambda-capture.cpp
+++ test/SemaCXX/warn-unused-lambda-capture.cpp
@@ -191,3 +191,12 @@
 void test_use_template() {
   test_templated(); // expected-note{{in instantiation of function template specialization 'test_templated' requested here}}
 }
+
+namespace pr3 {
+int a;
+void b() {
+  int c[a];
+  auto vla_used = [&c] { return c[0]; };
+  auto vla_unused = [&c] {}; // expected-warning{{lambda capture 'c' is not used}}
+}
+} // namespace pr3
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -1481,6 +1481,9 @@
   if (CaptureHasSideEffects(From))
 return;
 
+  if (From.isVLATypeCapture())
+return;
+
   auto diag = Diag(From.getLocation(), diag::warn_unused_lambda_capture);
   if (From.isThisCapture())
 diag << "'this'";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] r320163 - [libunwind] Create install-unwind-stripped target manually

[Shoaib Meenai via cfe-commits]

Author: smeenai
Date: Fri Dec  8 09:15:05 2017
New Revision: 320163

URL: http://llvm.org/viewvc/llvm-project?rev=320163&view=rev
Log:
[libunwind] Create install-unwind-stripped target manually

This supports using a newer libunwind with an older installation of LLVM
(whose cmake modules wouldn't have add_llvm_install_targets).

Modified:
libunwind/trunk/src/CMakeLists.txt

Modified: libunwind/trunk/src/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/CMakeLists.txt?rev=320163&r1=320162&r2=320163&view=diff
==
--- libunwind/trunk/src/CMakeLists.txt (original)
+++ libunwind/trunk/src/CMakeLists.txt Fri Dec  8 09:15:05 2017
@@ -139,7 +139,15 @@ if (LIBUNWIND_INSTALL_LIBRARY)
 endif()
 
 if (NOT CMAKE_CONFIGURATION_TYPES AND LIBUNWIND_INSTALL_LIBRARY)
-  add_llvm_install_targets(install-unwind
-   DEPENDS unwind
-   COMPONENT unwind)
+  add_custom_target(install-unwind
+DEPENDS unwind
+COMMAND "${CMAKE_COMMAND}"
+-DCMAKE_INSTALL_COMPONENT=unwind
+-P "${LIBUNWIND_BINARY_DIR}/cmake_install.cmake")
+  add_custom_target(install-unwind-stripped
+DEPENDS unwind
+COMMAND "${CMAKE_COMMAND}"
+-DCMAKE_INSTALL_COMPONENT=unwind
+-DCMAKE_INSTALL_DO_STRIP=1
+-P "${LIBUNWIND_BINARY_DIR}/cmake_install.cmake")
 endif()


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

[PATCH] D40685: [libunwind] Switch to add_llvm_install_targets

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

The other runtime projects had the stripped installation targets added manually 
because they officially support being built completely standalone (i.e. without 
any LLVM components), so we can't rely on LLVM's CMake there. That's really 
unfortunate IMO, since it leads to a bunch of duplication, but it is what it is.

libunwind doesn't support building completely standalone AFAIK, so I think 
assuming trunk libunwind will be paired with trunk LLVM is pretty reasonable in 
general. It's easy enough to not break your use case here, however, so I went 
ahead and did so in r320163.


Repository:
  rL LLVM

https://reviews.llvm.org/D40685



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

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In https://reviews.llvm.org/D40671#945158, @xgsa wrote:

> In https://reviews.llvm.org/D40671#944906, @alexfh wrote:
>
> > BTW, how will this feature interact with cpplint.py's way of handling 
> > specific NOLINT directives that use different lint rule names, which 
> > sometimes refer to the same rule (e.g. `// NOLINT(runtime/explicit)` 
> > suppresses the `runtime/explicit` cpplint rule that enforces the same style 
> > rule as the google-runtime-explicit check)?
>
>
> This feature accepts only full check names.


How are unknown check names handled? More specifically: will the `// 
NOLINT(runtime/explicit)` comment disable all clang-tidy checks or none?


https://reviews.llvm.org/D40671



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

[PATCH] D40625: Harmonizing attribute GNU/C++ spellings

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: include/clang/Basic/Attr.td:602
 def AnalyzerNoReturn : InheritableAttr {
-  let Spellings = [GNU<"analyzer_noreturn">];
+  let Spellings = [Clang<"analyzer_noreturn">];
   let Documentation = [Undocumented];

aaron.ballman wrote:
> dcoughlin wrote:
> > aaron.ballman wrote:
> > > dcoughlin wrote:
> > > > aaron.ballman wrote:
> > > > > rsmith wrote:
> > > > > > Hmm, should the clang static analyzer reuse the `clang::` 
> > > > > > namespace, or should it get its own?
> > > > > Good question, I don't have strong opinions on the answer here, but 
> > > > > perhaps @dcoughlin does?
> > > > > 
> > > > > If we want to use a separate namespace for the analyzer, would we 
> > > > > want to use that same namespace for any clang-tidy specific 
> > > > > attributes? Or should clang-tidy get its own namespace? (Do we ever 
> > > > > plan to execute clang-tidy through the clang driver? That might 
> > > > > change our answer.)
> > > > How would this look if we added a special namespace for the clang 
> > > > static analyzer? Would this lead to duplication (say, 
> > > > [[clang_analyzer::analyzer_noreturn]]) so that we keep the "analyzer_" 
> > > > prefix for __attribute__((analyzer_noreturn))? Or could we have the 
> > > > "analyzer_" prefix only for GNU-style attributes but not for C++ (for 
> > > > example, [[clang_analyzer::noreturn]])?
> > > > 
> > > > As for clang-tidy, I think it probably makes sense for it to have its 
> > > > own namespace, but we should ask @alexfh.
> > > > How would this look if we added a special namespace for the clang 
> > > > static analyzer? Would this lead to duplication (say, 
> > > > [[clang_analyzer::analyzer_noreturn]]) so that we keep the "analyzer_" 
> > > > prefix for attribute((analyzer_noreturn))? Or could we have the 
> > > > "analyzer_" prefix only for GNU-style attributes but not for C++ (for 
> > > > example, [[clang_analyzer::noreturn]])?
> > > 
> > > We have the ability to do whatever we'd like there. Given that the 
> > > semantics are so similar to `[[noreturn]]`, I think it would be 
> > > reasonable to use `[[clang_analyzer::noreturn]]` and 
> > > `__attribute__((analyzer_noreturn))` if that's the direction you think is 
> > > best.
> > > 
> > > > As for clang-tidy, I think it probably makes sense for it to have its 
> > > > own namespace, but we should ask @alexfh.
> > > 
> > > I'm less enthusiastic about giving clang-tidy a vendor namespace that's 
> > > separate from the static analyzer, should the need arise. My biggest 
> > > concern there is that I would *really* like to see clang-tidy be more 
> > > tightly integrated with the clang driver (so users don't have to manually 
> > > execute a secondary tool). If that were to happen, then the user 
> > > experience would be that there are two vendor namespaces both related to 
> > > analyzer attributes.
> > > 
> > > That said, I would also not be opposed to putting all of these attributes 
> > > under the `clang` vendor namespace and not having a separate vendor for 
> > > the analyzer or clang-tidy.
> > I would be find with keeping all of these things under the `clang` 
> > namespace, too.
> > 
> > That said, I do think there is some value in having a namespace for 
> > analyzer attributes separate from clang proper because the namespace would 
> > make it more clear that the attribute doesn't affect code generation.
> I've changed this one back to the GNU spelling to give us time to decide how 
> we want to handle analyzer attributes.
> 
> I'm not certain "does not affect codegen" is the correct measure to use for 
> this, however. We have other attributes that muddy the water, such as 
> `annotate`, or the format specifier attributes -- these don't (really) impact 
> codegen in any way, but do impact more than just the analyzer. Given the 
> integration of the analyzer with Clang (and the somewhat fluid nature of what 
> is responsible for issuing diagnostics), I think we should proceed with 
> caution on the idea of an analyzer-specific namespace.
> 
> However, do you have a list of attributes you think might qualify as being 
> analyzer-only? I can make sure we leave those spellings alone in this patch.
An argument against clang_tidy and clang_analyzer vendor namespaces is that the 
choice of where to implement a certain check would be connected to adding an 
attribute in one or both of these namespaces, which would complicate things a 
bit. In case both clang-tidy and static analyzer use the same argument, we'd 
need to have duplicate attributes. I definitely don't think we need three 
`noreturn` attributes, for example.


https://reviews.llvm.org/D40625



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

Re: r320162 - Revert "Unify implementation of our two different flavours of -Wtautological-compare."

[Richard Smith via cfe-commits]

On 8 Dec 2017 08:54, "Hans Wennborg via cfe-commits" <
cfe-commits@lists.llvm.org> wrote:

Author: hans
Date: Fri Dec  8 08:54:08 2017
New Revision: 320162

URL: http://llvm.org/viewvc/llvm-project?rev=320162&view=rev
Log:
Revert "Unify implementation of our two different flavours of
-Wtautological-compare."

> Unify implementation of our two different flavours of
-Wtautological-compare.
>
> In so doing, fix a handful of remaining bugs where we would report false
> positives or false negatives if we promote a signed value to an unsigned
type
> for the comparison.

This caused a new warning in Chromium:

../../base/trace_event/trace_log.cc:1545:29: error: comparison of constant
64
with expression of type 'unsigned int' is always true
[-Werror,-Wtautological-constant-out-of-range-compare]
  DCHECK(handle.event_index < TraceBufferChunk::kTraceBufferChunkSize);
 ~~ ^ ~~~

The 'unsigned int' is really a 6-bit bitfield, which is why it's always
less than 64.

I thought we didn't use to warn (with out-of-range-compare) when comparing
against the boundaries of a type?


This isn't a "boundary of the type" case, though -- 64 is out of range for
a 6 bit bitfield. Your CHECK does nothing. Do you think that it's not
reasonable to warn on this bug, or that it's not a bug?

Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/tautological-constant-compare.c
cfe/trunk/test/Sema/tautological-constant-enum-compare.c
cfe/trunk/test/SemaCXX/compare.cpp

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
SemaChecking.cpp?rev=320162&r1=320161&r2=320162&view=diff

==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Dec  8 08:54:08 2017
@@ -8662,113 +8662,54 @@ static bool isKnownToHaveUnsignedValue(E
 }

 namespace {
-/// The promoted range of values of a type. In general this has the
-/// following structure:
-///
-/// |---| . . . |---|
-/// ^   ^   ^   ^
-///Min   HoleMin  HoleMax  Max
-///
-/// ... where there is only a hole if a signed type is promoted to unsigned
-/// (in which case Min and Max are the smallest and largest representable
-/// values).
-struct PromotedRange {
-  // Min, or HoleMax if there is a hole.
-  llvm::APSInt PromotedMin;
-  // Max, or HoleMin if there is a hole.
-  llvm::APSInt PromotedMax;
-
-  PromotedRange(IntRange R, unsigned BitWidth, bool Unsigned) {
-if (R.Width == 0)
-  PromotedMin = PromotedMax = llvm::APSInt(BitWidth, Unsigned);
-else {
-  PromotedMin = llvm::APSInt::getMinValue(R.Width, R.NonNegative)
-.extOrTrunc(BitWidth);
-  PromotedMin.setIsUnsigned(Unsigned);
-
-  PromotedMax = llvm::APSInt::getMaxValue(R.Width, R.NonNegative)
-.extOrTrunc(BitWidth);
-  PromotedMax.setIsUnsigned(Unsigned);
-}
-  }

-  // Determine whether this range is contiguous (has no hole).
-  bool isContiguous() const { return PromotedMin <= PromotedMax; }
+enum class LimitType {
+  Max = 1U << 0U,  // e.g. 32767 for short
+  Min = 1U << 1U,  // e.g. -32768 for short
+  Both = Max | Min // When the value is both the Min and the Max limit at
the
+   // same time; e.g. in C++, A::a in enum A { a = 0 };
+};

-  // Where a constant value is within the range.
-  enum ComparisonResult {
-LT = 0x1,
-LE = 0x2,
-GT = 0x4,
-GE = 0x8,
-EQ = 0x10,
-NE = 0x20,
-InRangeFlag = 0x40,
-
-Less = LE | LT | NE,
-Min = LE | InRangeFlag,
-InRange = InRangeFlag,
-Max = GE | InRangeFlag,
-Greater = GE | GT | NE,
+} // namespace

-OnlyValue = LE | GE | EQ | InRangeFlag,
-InHole = NE
-  };
+/// Checks whether Expr 'Constant' may be the
+/// std::numeric_limits<>::max() or std::numeric_limits<>::min()
+/// of the Expr 'Other'. If true, then returns the limit type (min or max).
+/// The Value is the evaluation of Constant
+static llvm::Optional IsTypeLimit(Sema &S, Expr *Constant,
+ Expr *Other,
+ const llvm::APSInt &Value) {
+  if (IsEnumConstOrFromMacro(S, Constant))
+return llvm::Optional();

-  ComparisonResult compare(const llvm::APSInt &Value) const {
-assert(Value.getBitWidth() == PromotedMin.getBitWidth() &&
-   Value.isUnsigned() == PromotedMin.isUnsigned());
-if (!isContiguous()) {
-  assert(Value.isUnsigned() && "discontiguous range for signed
compare");
-  if (Value.isMinValue()) return Min;
-  if (Value.isMaxValue()) return Max;
-  if (Value >= PromotedMin) return InRange;
-  if (Value <= PromotedMax) return InRange;
-  return InHole;
-}
+  if (isKnownToHaveUnsignedValue(Other) && Value == 0)
+return LimitType::Min;

-sw

r320165 - Fix a comment in the code

[Kamil Rytarowski via cfe-commits]

Author: kamil
Date: Fri Dec  8 09:38:25 2017
New Revision: 320165

URL: http://llvm.org/viewvc/llvm-project?rev=320165&view=rev
Log:
Fix a comment in the code

The -ldl library is missing on NetBSD too, make the comment more generic.

Modified:
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=320165&r1=320164&r2=320165&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Fri Dec  8 09:38:25 2017
@@ -544,7 +544,7 @@ void tools::linkSanitizerRuntimeDeps(con
 CmdArgs.push_back("-lrt");
   }
   CmdArgs.push_back("-lm");
-  // There's no libdl on FreeBSD or RTEMS.
+  // There's no libdl on all OSes.
   if (TC.getTriple().getOS() != llvm::Triple::FreeBSD &&
   TC.getTriple().getOS() != llvm::Triple::NetBSD &&
   TC.getTriple().getOS() != llvm::Triple::RTEMS)


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

[PATCH] D39363: [clang-tidy] Correctly classify constant arrays and constant strings as constants when checking identifiers naming

[Alexander Kornienko via Phabricator via cfe-commits]

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

Looks good!

Thank you for the fix! Do you need someone to commit it for you? If yes, please 
rebase the fix on top of the current HEAD and let me know.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D39363



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

[PATCH] D40671: [clang-tidy] Support specific checks for NOLINT directive

[Anton via Phabricator via cfe-commits]

xgsa added a comment.

In https://reviews.llvm.org/D40671#949687, @alexfh wrote:

> How are unknown check names handled? More specifically: will the `// 
> NOLINT(runtime/explicit)` comment disable all clang-tidy checks or none?


None. If comment is syntactically correct and contains parenthesis, it works 
only for the known checks inside.

Still, I don't think it worth mentioning all the corner cases in documentation 
to keep things simple.


https://reviews.llvm.org/D40671



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

[PATCH] D40948: Switch Clang's default C++ language target to C++14

[Filipe Cabecinhas via Phabricator via cfe-commits]

filcab added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1733
   // The PS4 uses C++11 as the default C++ standard.
-  if (T.isPS4())
-LangStd = LangStandard::lang_gnucxx11;
-  else
-LangStd = LangStandard::lang_gnucxx98;
+  LangStd = LangStandard::lang_gnucxx14;
   break;

t.p.northover wrote:
> filcab wrote:
> > Why are you changing the PS4 default too?
> Paul Robinson indicated that it was feasible back in March: 
> http://lists.llvm.org/pipermail/cfe-dev/2017-March/052986.html. If that's 
> changed I'd be happy to put it back to C++11, but he's one of the main people 
> (rightly) bugging me about this so I'd be slightly surprised.
> 
> I also notice the comment crept back in. Bother.
Sounds good, then. If Paul is happy with that change, I don't see a problem 
there (assuming you do get rid of the comment for good :-) ).

Thank you,

 Filipe


Repository:
  rC Clang

https://reviews.llvm.org/D40948



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

[PATCH] D40996: Add --no-cuda-version-check in unknown-std.cpp

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

Ideally the tests should be hermetic and should use mock CUDA installation that 
comes with the tests. E.g. `--cuda-path=%S/Inputs/CUDA/usr/local/cuda`


https://reviews.llvm.org/D40996



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

r320168 - [hwasan] typo in docs

[Kostya Serebryany via cfe-commits]

Author: kcc
Date: Fri Dec  8 10:14:03 2017
New Revision: 320168

URL: http://llvm.org/viewvc/llvm-project?rev=320168&view=rev
Log:
[hwasan] typo in docs

Modified:
cfe/trunk/docs/HardwareAssistedAddressSanitizerDesign.rst

Modified: cfe/trunk/docs/HardwareAssistedAddressSanitizerDesign.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/HardwareAssistedAddressSanitizerDesign.rst?rev=320168&r1=320167&r2=320168&view=diff
==
--- cfe/trunk/docs/HardwareAssistedAddressSanitizerDesign.rst (original)
+++ cfe/trunk/docs/HardwareAssistedAddressSanitizerDesign.rst Fri Dec  8 
10:14:03 2017
@@ -80,7 +80,7 @@ Errors are generated by `__builtin_trap`
 Attribute
 -
 
-HWASAN uses it's own LLVM IR Attribute `sanitize_hwaddress` and a matching
+HWASAN uses its own LLVM IR Attribute `sanitize_hwaddress` and a matching
 C function attribute. An alternative would be to re-use ASAN's attribute
 `sanitize_address`. The reasons to use a separate attribute are:
 


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

[PATCH] D39363: [clang-tidy] Correctly classify constant arrays and constant strings as constants when checking identifiers naming

[Beren Minor via Phabricator via cfe-commits]

berenm updated this revision to Diff 126177.
berenm added a comment.

Rebase patch on upstream HEAD


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D39363

Files:
  clang-tidy/readability/IdentifierNamingCheck.cpp
  test/clang-tidy/readability-identifier-naming.cpp

Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -405,6 +405,38 @@
 
   using ::FOO_NS::InlineNamespace::CE_function;
 // CHECK-FIXES: {{^}}  using ::foo_ns::inline_namespace::ce_function;{{$}}
+
+  unsigned MY_LOCAL_array[] = {1,2,3};
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: invalid case style for local variable 'MY_LOCAL_array'
+// CHECK-FIXES: {{^}}  unsigned my_local_array[] = {1,2,3};{{$}}
+
+  unsigned const MyConstLocal_array[] = {1,2,3};
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for local constant 'MyConstLocal_array'
+// CHECK-FIXES: {{^}}  unsigned const kMyConstLocalArray[] = {1,2,3};{{$}}
+
+  static unsigned MY_STATIC_array[] = {1,2,3};
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: invalid case style for static variable 'MY_STATIC_array'
+// CHECK-FIXES: {{^}}  static unsigned s_myStaticArray[] = {1,2,3};{{$}}
+
+  static unsigned const MyConstStatic_array[] = {1,2,3};
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: invalid case style for static constant 'MyConstStatic_array'
+// CHECK-FIXES: {{^}}  static unsigned const MY_CONST_STATIC_ARRAY[] = {1,2,3};{{$}}
+
+  char MY_LOCAL_string[] = "123";
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local variable 'MY_LOCAL_string'
+// CHECK-FIXES: {{^}}  char my_local_string[] = "123";{{$}}
+
+  char const MyConstLocal_string[] = "123";
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for local constant 'MyConstLocal_string'
+// CHECK-FIXES: {{^}}  char const kMyConstLocalString[] = "123";{{$}}
+
+  static char MY_STATIC_string[] = "123";
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for static variable 'MY_STATIC_string'
+// CHECK-FIXES: {{^}}  static char s_myStaticString[] = "123";{{$}}
+
+  static char const MyConstStatic_string[] = "123";
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: invalid case style for static constant 'MyConstStatic_string'
+// CHECK-FIXES: {{^}}  static char const MY_CONST_STATIC_STRING[] = "123";{{$}}
 }
 
 #define MY_TEST_Macro(X) X()
@@ -418,6 +450,27 @@
 
 template  struct a {
   typename t_t::template b<> c;
+
+  char const MY_ConstMember_string[4] = "123";
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for constant member 'MY_ConstMember_string'
+// CHECK-FIXES: {{^}}  char const my_const_member_string[4] = "123";{{$}}
+
+  static char const MyConstClass_string[];
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: invalid case style for class constant 'MyConstClass_string'
+// CHECK-FIXES: {{^}}  static char const kMyConstClassString[];{{$}}
 };
 
+template
+char const a::MyConstClass_string[] = "123";
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for class constant 'MyConstClass_string'
+// CHECK-FIXES: {{^}}char const a::kMyConstClassString[] = "123";{{$}}
+
 template  class A> struct b { A c; };
+
+unsigned MY_GLOBAL_array[] = {1,2,3};
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global variable 'MY_GLOBAL_array'
+// CHECK-FIXES: {{^}}unsigned g_my_global_array[] = {1,2,3};{{$}}
+
+unsigned const MyConstGlobal_array[] = {1,2,3};
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global constant 'MyConstGlobal_array'
+// CHECK-FIXES: {{^}}unsigned const MY_CONST_GLOBAL_ARRAY[] = {1,2,3};{{$}}
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -449,13 +449,13 @@
   if (const auto *Decl = dyn_cast(D)) {
 QualType Type = Decl->getType();
 
-if (!Type.isNull() && Type.isLocalConstQualified() &&
-NamingStyles[SK_ConstantMember])
-  return SK_ConstantMember;
+if (!Type.isNull() && Type.isConstQualified()) {
+  if (NamingStyles[SK_ConstantMember])
+return SK_ConstantMember;
 
-if (!Type.isNull() && Type.isLocalConstQualified() &&
-NamingStyles[SK_Constant])
-  return SK_Constant;
+  if (NamingStyles[SK_Constant])
+return SK_Constant;
+}
 
 if (Decl->getAccess() == AS_private && NamingStyles[SK_PrivateMember])
   return SK_PrivateMember;
@@ -478,13 +478,13 @@
 if (Decl->isConstexpr() && NamingStyles[SK_ConstexprVariable])
   return SK_ConstexprVariable;
 
-if (!Type.isNull() && Type.isLocalConstQualified() &&
-NamingStyles[SK_ConstantParameter])
-  return SK_ConstantParameter;
+if (!Type.isNull() && Type.isConstQualified()) {
+

[PATCH] D39363: [clang-tidy] Correctly classify constant arrays and constant strings as constants when checking identifiers naming

[Beren Minor via Phabricator via cfe-commits]

berenm added a comment.

Yes, please could you commit it for me? Thanks!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D39363



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

[PATCH] D40968: [OpenMP] Diagnose function name on the link clause

[Kelvin Li via Phabricator via cfe-commits]

kkwli0 added inline comments.



Comment at: lib/Sema/SemaOpenMP.cpp:12671
+void Sema::checkDeclIsAllowedInOpenMPTarget(Expr *E, Decl *D,
+const DeclarationNameInfo *Id) {
   if (!D || D->isInvalidDecl())

ABataev wrote:
> You can get `DeclarationNameInfo` from the `FunctionDecl`:
> ```
> FD->getNameInfo()
> ```
This FD->getNameInfo() will only give the name info from the function 
definition.  What we need here is the name info for 'foo' that appears on the 
pragma in order to give us

```
d2.c:2:33: error: function name is not allowed in 'link' clause
#pragma omp declare target link(foo)
^
```


https://reviews.llvm.org/D40968



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

Re: r320162 - Revert "Unify implementation of our two different flavours of -Wtautological-compare."

[Hans Wennborg via cfe-commits]

On Fri, Dec 8, 2017 at 9:30 AM, Richard Smith via cfe-commits
 wrote:
> On 8 Dec 2017 08:54, "Hans Wennborg via cfe-commits"
>  wrote:
>
> Author: hans
> Date: Fri Dec  8 08:54:08 2017
> New Revision: 320162
>
> URL: http://llvm.org/viewvc/llvm-project?rev=320162&view=rev
> Log:
> Revert "Unify implementation of our two different flavours of
> -Wtautological-compare."
>
>> Unify implementation of our two different flavours of
>> -Wtautological-compare.
>>
>> In so doing, fix a handful of remaining bugs where we would report false
>> positives or false negatives if we promote a signed value to an unsigned
>> type
>> for the comparison.
>
> This caused a new warning in Chromium:
>
> ../../base/trace_event/trace_log.cc:1545:29: error: comparison of constant
> 64
> with expression of type 'unsigned int' is always true
> [-Werror,-Wtautological-constant-out-of-range-compare]
>   DCHECK(handle.event_index < TraceBufferChunk::kTraceBufferChunkSize);
>  ~~ ^ ~~~
>
> The 'unsigned int' is really a 6-bit bitfield, which is why it's always
> less than 64.
>
> I thought we didn't use to warn (with out-of-range-compare) when comparing
> against the boundaries of a type?
>
>
> This isn't a "boundary of the type" case, though -- 64 is out of range for a
> 6 bit bitfield. Your CHECK does nothing. Do you think that it's not
> reasonable to warn on this bug, or that it's not a bug?

I'm not sure it's a bug; and the CHECK will fire in case someone
widens that bitfield and stores a larger value into it.

I suppose I should rewrite the code as "handle.event_index <=
TraceBufferChunk::kTraceBufferChunkSize -1"?

The reason I thought this was unintentional is because I thought we
don't warn in cases like:

void f(int x) {
if (x <= INT_MAX) {
foo();
}
}

and that's essentially what the code is doing, except it's using <
instead of <=.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40941: [ubsan] Use pass_object_size info in bounds checks (compiler-rt)

[Vedant Kumar via Phabricator via cfe-commits]

vsk closed this revision.
vsk added a comment.

Landed in r320129.


https://reviews.llvm.org/D40941



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

[PATCH] D41016: [Sema] Fix crash in unused-lambda-capture warning for VLAs

[Dimitry Andric via Phabricator via cfe-commits]

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

For the rest, LGTM.  It fixes the segfault, for both the full original test 
case, and my reduced version.




Comment at: lib/Sema/SemaLambda.cpp:1491
   else
 diag << From.getVariable();
   diag << From.isNonODRUsed();

Just for sanity's sake, I would still put an assert here, that `getVariable()` 
does not return `nullptr`.  That might catch similar issues in the future.  
(And it is better than a segfault... :) )



Repository:
  rC Clang

https://reviews.llvm.org/D41016



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

[PATCH] D40940: [ubsan] Use pass_object_size info in bounds checks

[Vedant Kumar via Phabricator via cfe-commits]

vsk closed this revision.
vsk added a comment.

Landed in r320128.


https://reviews.llvm.org/D40940



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

[PATCH] D40685: [libunwind] Switch to add_llvm_install_targets

[Will Dietz via Phabricator via cfe-commits]

dtzWill added a comment.

Okay, that makes sense.  Thanks for accommodating my use-case anyway! :D.


Repository:
  rL LLVM

https://reviews.llvm.org/D40685



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

Re: r320162 - Revert "Unify implementation of our two different flavours of -Wtautological-compare."

[Hans Wennborg via cfe-commits]

On Fri, Dec 8, 2017 at 11:10 AM, Hans Wennborg  wrote:
> On Fri, Dec 8, 2017 at 9:30 AM, Richard Smith via cfe-commits
>  wrote:
>> On 8 Dec 2017 08:54, "Hans Wennborg via cfe-commits"
>>  wrote:
>>
>> Author: hans
>> Date: Fri Dec  8 08:54:08 2017
>> New Revision: 320162
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=320162&view=rev
>> Log:
>> Revert "Unify implementation of our two different flavours of
>> -Wtautological-compare."
>>
>>> Unify implementation of our two different flavours of
>>> -Wtautological-compare.
>>>
>>> In so doing, fix a handful of remaining bugs where we would report false
>>> positives or false negatives if we promote a signed value to an unsigned
>>> type
>>> for the comparison.
>>
>> This caused a new warning in Chromium:
>>
>> ../../base/trace_event/trace_log.cc:1545:29: error: comparison of constant
>> 64
>> with expression of type 'unsigned int' is always true
>> [-Werror,-Wtautological-constant-out-of-range-compare]
>>   DCHECK(handle.event_index < TraceBufferChunk::kTraceBufferChunkSize);
>>  ~~ ^ ~~~
>>
>> The 'unsigned int' is really a 6-bit bitfield, which is why it's always
>> less than 64.
>>
>> I thought we didn't use to warn (with out-of-range-compare) when comparing
>> against the boundaries of a type?
>>
>>
>> This isn't a "boundary of the type" case, though -- 64 is out of range for a
>> 6 bit bitfield. Your CHECK does nothing. Do you think that it's not
>> reasonable to warn on this bug, or that it's not a bug?
>
> I'm not sure it's a bug; and the CHECK will fire in case someone
> widens that bitfield and stores a larger value into it.
>
> I suppose I should rewrite the code as "handle.event_index <=
> TraceBufferChunk::kTraceBufferChunkSize -1"?
>
> The reason I thought this was unintentional is because I thought we
> don't warn in cases like:
>
> void f(int x) {
> if (x <= INT_MAX) {
> foo();
> }
> }
>
> and that's essentially what the code is doing, except it's using <
> instead of <=.

We do warn for

void f(int x) {
if (x < (long long)INT_MAX + 1) {
foo();
}
}

so I suppose your patch is consistent with the existing warning.

Sorry for reverting, go ahead and reland or let me know if you'd like
me to do it.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40940: [ubsan] Use pass_object_size info in bounds checks

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> It's interesting to me that these array-bound checks don't seem to use 
> @llvm.objectsize in some form already.

That would be a cool experiment.  That said, one of the upsides of the current 
ubsan is that whether it will produce a diagnostic is predictable (as long as 
you don't use uninitialized data); you lose that to some extent with 
llvm.objectsize because it depends on the optimizer.




Comment at: lib/CodeGen/CGExpr.cpp:833
+  // Arrays don't have pass_object_size attributes, but if they have a constant
+  // size modifier it's the array size (C99 6.5.7.2p1).
+  if (auto *DecayedArrayTy = dyn_cast(ParamDecl->getType()))

"int f(int a[10])" might look like an array, but it isn't: it's just a 
different syntax to declare a pointer.  So it's legal to "lie" in the 
signature.  (If you want to actually pass a pointer to an array, you have to 
write "int (*a)[10]".)  And the definition of "static" says "an array with at 
least as many elements as specified by the size expression", which isn't a 
maximum, so that doesn't really help either.

Most people would consider it bad style to put a number into the array bound 
which doesn't reflect reality, but I think we shouldn't try to check it unless 
the user explicitly requests it.


https://reviews.llvm.org/D40940



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

r320185 - [ubsan] array-bounds: Ignore params with constant size

[Vedant Kumar via cfe-commits]

Author: vedantk
Date: Fri Dec  8 11:51:42 2017
New Revision: 320185

URL: http://llvm.org/viewvc/llvm-project?rev=320185&view=rev
Log:
[ubsan] array-bounds: Ignore params with constant size

This is a follow-up to r320128. Eli pointed out that there is some gray
area in the language standard about whether the constant size is exact,
or a lower bound.

https://reviews.llvm.org/D40940

Modified:
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/test/CodeGen/ubsan-pass-object-size.c

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=320185&r1=320184&r2=320185&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Fri Dec  8 11:51:42 2017
@@ -829,14 +829,6 @@ llvm::Value *CodeGenFunction::LoadPassed
   if (!ParamDecl)
 return nullptr;
 
-  // Arrays don't have pass_object_size attributes, but if they have a constant
-  // size modifier it's the array size (C99 6.5.7.2p1).
-  if (auto *DecayedArrayTy = dyn_cast(ParamDecl->getType()))
-if (auto *ArrayTy =
-dyn_cast(DecayedArrayTy->getOriginalType()))
-  return llvm::ConstantInt::get(SizeTy,
-ArrayTy->getSize().getLimitedValue());
-
   auto *POSAttr = ParamDecl->getAttr();
   if (!POSAttr)
 return nullptr;

Modified: cfe/trunk/test/CodeGen/ubsan-pass-object-size.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-pass-object-size.c?rev=320185&r1=320184&r2=320185&view=diff
==
--- cfe/trunk/test/CodeGen/ubsan-pass-object-size.c (original)
+++ cfe/trunk/test/CodeGen/ubsan-pass-object-size.c Fri Dec  8 11:51:42 2017
@@ -55,8 +55,7 @@ int pat(int *const p __attribute__((pass
 
 // CHECK-LABEL: define i32 @cat(
 int cat(int p[static 10], int n) {
-  // CHECK: icmp ult i64 {{.*}}, 10, !nosanitize
-  // CHECK: __ubsan_handle_out_of_bounds
+  // CHECK-NOT: __ubsan_handle_out_of_bounds
   // CHECK: ret i32
   return p[n];
 }


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

[PATCH] D40940: [ubsan] Use pass_object_size info in bounds checks

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

I backed out the part of this patch which deals with array parameters declared 
like p[10] or p[static 10]: r320185.




Comment at: lib/CodeGen/CGExpr.cpp:833
+  // Arrays don't have pass_object_size attributes, but if they have a constant
+  // size modifier it's the array size (C99 6.5.7.2p1).
+  if (auto *DecayedArrayTy = dyn_cast(ParamDecl->getType()))

efriedma wrote:
> "int f(int a[10])" might look like an array, but it isn't: it's just a 
> different syntax to declare a pointer.  So it's legal to "lie" in the 
> signature.  (If you want to actually pass a pointer to an array, you have to 
> write "int (*a)[10]".)  And the definition of "static" says "an array with at 
> least as many elements as specified by the size expression", which isn't a 
> maximum, so that doesn't really help either.
> 
> Most people would consider it bad style to put a number into the array bound 
> which doesn't reflect reality, but I think we shouldn't try to check it 
> unless the user explicitly requests it.
My copy of the C99 draft (n1256) is a little fuzzy on this point [*]. There's 
enough of a gray area here that it seems appropriate to back out this part of 
the patch.

* It states: "In addition to optional type qualifiers and the keyword static, 
the [ and ] may delimit an expression or *. If they delimit an expression 
(which specifies the size of an array) ...". I took the parenthetical 
literally, and didn't know about the 'at least as many' language.


https://reviews.llvm.org/D40940



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

[PATCH] D40968: [OpenMP] Diagnose function name on the link clause

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/Sema/SemaOpenMP.cpp:12671
+void Sema::checkDeclIsAllowedInOpenMPTarget(Expr *E, Decl *D,
+const DeclarationNameInfo *Id) {
   if (!D || D->isInvalidDecl())

kkwli0 wrote:
> ABataev wrote:
> > You can get `DeclarationNameInfo` from the `FunctionDecl`:
> > ```
> > FD->getNameInfo()
> > ```
> This FD->getNameInfo() will only give the name info from the function 
> definition.  What we need here is the name info for 'foo' that appears on the 
> pragma in order to give us
> 
> ```
> d2.c:2:33: error: function name is not allowed in 'link' clause
> #pragma omp declare target link(foo)
> ^
> ```
Then just pass `SourceLocation`


https://reviews.llvm.org/D40968



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

[PATCH] D40940: [ubsan] Use pass_object_size info in bounds checks

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added a comment.

> That said, one of the upsides of the current ubsan is that whether it will 
> produce a diagnostic is predictable (as long as you don't use uninitialized 
> data); you lose that to some extent with llvm.objectsize because it depends 
> on the optimizer.

True. If that's not desirable to have in `array-bounds`, we could potentially 
move these checks under `-fsanitize=object-size` instead. We'd just have to be 
careful about not emitting `object-size` and `array-bounds` checks for the same 
array access.


https://reviews.llvm.org/D40940



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

Re: r305110 - [ODRHash] Add support for TemplateArgument types.

[Richard Trieu via cfe-commits]

Vassil,

It depends on which parts of the AST you want to be stable.  The ODRHashing
is stable across TU's, but it may depend on information that you don't want
to be part of the hash.  For instance, if you have "using F = float;" in
you code, then the hash of TemplateArgument for "float" and for "F" would
be different.  If the template specializations you are dealing with are
canonicalized, then the TemplateArgument hashes should be the same.
Otherwise, ODRHash would need to be changed to suit your requirements.

On Fri, Dec 8, 2017 at 8:16 AM, Vassil Vassilev 
wrote:

> Hi Richard,
>
>   Is there a way to get an ODRHashing which is stable across translation
> units? I'd like to use the TemplateArgument ODRHash to lookup template
> specializations and deserialize them only if they are required.
>
> Many thanks!
> Vassil
>
> On 6/9/17 11:00 PM, Richard Trieu via cfe-commits wrote:
>
>> Author: rtrieu
>> Date: Fri Jun  9 16:00:10 2017
>> New Revision: 305110
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=305110&view=rev
>> Log:
>> [ODRHash] Add support for TemplateArgument types.
>>
>> Recommit r304592 that was reverted in r304618.  r305104 should have fixed
>> the
>> issue.
>>
>> Modified:
>>  cfe/trunk/lib/AST/ODRHash.cpp
>>  cfe/trunk/test/Modules/odr_hash.cpp
>>
>> Modified: cfe/trunk/lib/AST/ODRHash.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ODRHas
>> h.cpp?rev=305110&r1=305109&r2=305110&view=diff
>> 
>> ==
>> --- cfe/trunk/lib/AST/ODRHash.cpp (original)
>> +++ cfe/trunk/lib/AST/ODRHash.cpp Fri Jun  9 16:00:10 2017
>> @@ -140,7 +140,25 @@ void ODRHash::AddTemplateName(TemplateNa
>> }
>>   }
>>   -void ODRHash::AddTemplateArgument(TemplateArgument TA) {}
>> +void ODRHash::AddTemplateArgument(TemplateArgument TA) {
>> +  auto Kind = TA.getKind();
>> +  ID.AddInteger(Kind);
>> +
>> +  switch (Kind) {
>> +  case TemplateArgument::Null:
>> +  case TemplateArgument::Declaration:
>> +  case TemplateArgument::NullPtr:
>> +  case TemplateArgument::Integral:
>> +  case TemplateArgument::Template:
>> +  case TemplateArgument::TemplateExpansion:
>> +  case TemplateArgument::Expression:
>> +  case TemplateArgument::Pack:
>> +break;
>> +  case TemplateArgument::Type:
>> +AddQualType(TA.getAsType());
>> +break;
>> +  }
>> +}
>>   void ODRHash::AddTemplateParameterList(const TemplateParameterList
>> *TPL) {}
>> void ODRHash::clear() {
>>
>> Modified: cfe/trunk/test/Modules/odr_hash.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/
>> odr_hash.cpp?rev=305110&r1=305109&r2=305110&view=diff
>> 
>> ==
>> --- cfe/trunk/test/Modules/odr_hash.cpp (original)
>> +++ cfe/trunk/test/Modules/odr_hash.cpp Fri Jun  9 16:00:10 2017
>> @@ -900,6 +900,24 @@ S2 s2;
>>   #endif
>>   }
>>   +namespace TemplateArgument {
>> +#if defined(FIRST)
>> +template struct U1 {};
>> +struct S1 {
>> +  U1 u;
>> +};
>> +#elif defined(SECOND)
>> +template struct U1 {};
>> +struct S1 {
>> +  U1 u;
>> +};
>> +#else
>> +S1 s1;
>> +// expected-error@first.h:* {{'TemplateArgument::S1::u' from module
>> 'FirstModule' is not present in definition of 'TemplateArgument::S1' in
>> module 'SecondModule'}}
>> +// expected-note@second.h:* {{declaration of 'u' does not match}}
>> +#endif
>> +}
>> +
>>   // Interesting cases that should not cause errors.  struct S should not
>> error
>>   // while struct T should error at the access specifier mismatch at the
>> end.
>>   namespace AllDecls {
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36915: [Sema] Diagnose local variables and parameters captured by lambda and block expressions in a default argument

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

ping


https://reviews.llvm.org/D36915



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

[PATCH] D36918: [Sema] Take into account the current context when checking the accessibility of a member function pointer

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

ping


https://reviews.llvm.org/D36918



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

[PATCH] D40940: [ubsan] Use pass_object_size info in bounds checks

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:833
+  // Arrays don't have pass_object_size attributes, but if they have a constant
+  // size modifier it's the array size (C99 6.5.7.2p1).
+  if (auto *DecayedArrayTy = dyn_cast(ParamDecl->getType()))

vsk wrote:
> efriedma wrote:
> > "int f(int a[10])" might look like an array, but it isn't: it's just a 
> > different syntax to declare a pointer.  So it's legal to "lie" in the 
> > signature.  (If you want to actually pass a pointer to an array, you have 
> > to write "int (*a)[10]".)  And the definition of "static" says "an array 
> > with at least as many elements as specified by the size expression", which 
> > isn't a maximum, so that doesn't really help either.
> > 
> > Most people would consider it bad style to put a number into the array 
> > bound which doesn't reflect reality, but I think we shouldn't try to check 
> > it unless the user explicitly requests it.
> My copy of the C99 draft (n1256) is a little fuzzy on this point [*]. There's 
> enough of a gray area here that it seems appropriate to back out this part of 
> the patch.
> 
> * It states: "In addition to optional type qualifiers and the keyword static, 
> the [ and ] may delimit an expression or *. If they delimit an expression 
> (which specifies the size of an array) ...". I took the parenthetical 
> literally, and didn't know about the 'at least as many' language.
"which specifies the size of an array" in this context just means that it's the 
part of the array declarator which specifies the size of the array, not that 
it's the size of any particular array.  The decay rules for function parameters 
are the relevant bit of the standard.

And yes, this is all very confusing; this is why C++ has std::array.


https://reviews.llvm.org/D40940



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

r320191 - [OPENMP] Simplify codegen for loop iteration variables in loop preamble.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Dec  8 12:18:58 2017
New Revision: 320191

URL: http://llvm.org/viewvc/llvm-project?rev=320191&view=rev
Log:
[OPENMP] Simplify codegen for loop iteration variables in loop preamble.

Initial patch could cause trouble in the optimized code because of the
incorrectly generated lifetime intrinsics.

Modified:
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/test/OpenMP/distribute_parallel_for_lastprivate_codegen.cpp
cfe/trunk/test/OpenMP/distribute_parallel_for_simd_lastprivate_codegen.cpp
cfe/trunk/test/OpenMP/for_codegen.cpp
cfe/trunk/test/OpenMP/for_lastprivate_codegen.cpp
cfe/trunk/test/OpenMP/for_linear_codegen.cpp
cfe/trunk/test/OpenMP/parallel_for_linear_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp?rev=320191&r1=320190&r2=320191&view=diff
==
--- cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp Fri Dec  8 12:18:58 2017
@@ -122,7 +122,12 @@ public:
 class OMPLoopScope : public CodeGenFunction::RunCleanupsScope {
   void emitPreInitStmt(CodeGenFunction &CGF, const OMPLoopDirective &S) {
 CodeGenFunction::OMPPrivateScope PreCondScope(CGF);
-CGF.EmitOMPPrivateLoopCounters(S, PreCondScope);
+for (auto *E : S.counters()) {
+  const auto *VD = cast(cast(E)->getDecl());
+  (void)PreCondScope.addPrivate(VD, [&CGF, VD]() {
+return CGF.CreateMemTemp(VD->getType().getNonReferenceType());
+  });
+}
 (void)PreCondScope.Privatize();
 if (auto *LD = dyn_cast(&S)) {
   if (auto *PreInits = cast_or_null(LD->getPreInits())) {

Modified: cfe/trunk/test/OpenMP/distribute_parallel_for_lastprivate_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/distribute_parallel_for_lastprivate_codegen.cpp?rev=320191&r1=320190&r2=320191&view=diff
==
--- cfe/trunk/test/OpenMP/distribute_parallel_for_lastprivate_codegen.cpp 
(original)
+++ cfe/trunk/test/OpenMP/distribute_parallel_for_lastprivate_codegen.cpp Fri 
Dec  8 12:18:58 2017
@@ -164,7 +164,7 @@ int main() {
   // LAMBDA: [[SFVAR_IN_REF:%.+]] = load float*, float** 
[[SFVAR_PRIVATE_ADDR]],
 
   // LAMBDA: [[G1_IN_REF:%.+]] = load double*, double** 
[[G1_PRIVATE_ADDR]],
-  // LAMBDA: store double* [[G1_PRIVATE]], double** [[TMP_G1]],
+  // LAMBDA: store double* [[G1_PRIVATE]], double** [[TMP_G1_PRIVATE]],
 
   // LAMBDA: call {{.*}}void @__kmpc_for_static_init_4(
 

Modified: 
cfe/trunk/test/OpenMP/distribute_parallel_for_simd_lastprivate_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/distribute_parallel_for_simd_lastprivate_codegen.cpp?rev=320191&r1=320190&r2=320191&view=diff
==
--- cfe/trunk/test/OpenMP/distribute_parallel_for_simd_lastprivate_codegen.cpp 
(original)
+++ cfe/trunk/test/OpenMP/distribute_parallel_for_simd_lastprivate_codegen.cpp 
Fri Dec  8 12:18:58 2017
@@ -173,7 +173,7 @@ int main() {
   // LAMBDA: [[SFVAR_IN_REF:%.+]] = load float*, float** 
[[SFVAR_PRIVATE_ADDR]],
 
   // LAMBDA: [[G1_IN_REF:%.+]] = load double*, double** 
[[G1_PRIVATE_ADDR]],
-  // LAMBDA: store double* [[G1_PRIVATE]], double** [[TMP_G1]],
+  // LAMBDA: store double* [[G1_PRIVATE]], double** [[TMP_G1_PRIVATE]],
 
   // LAMBDA: call {{.*}}void @__kmpc_for_static_init_4(
 

Modified: cfe/trunk/test/OpenMP/for_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/for_codegen.cpp?rev=320191&r1=320190&r2=320191&view=diff
==
--- cfe/trunk/test/OpenMP/for_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/for_codegen.cpp Fri Dec  8 12:18:58 2017
@@ -386,7 +386,9 @@ void parallel_for(float *a) {
 char i = 1, j = 2, k = 3;
 // CHECK-LABEL: for_with_global_lcv
 void for_with_global_lcv() {
+// CHECK: alloca i8,
 // CHECK: [[I_ADDR:%.+]] = alloca i8,
+// CHECK: alloca i8,
 // CHECK: [[J_ADDR:%.+]] = alloca i8,
 
 // CHECK: call void @__kmpc_for_static_init_4(

Modified: cfe/trunk/test/OpenMP/for_lastprivate_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/for_lastprivate_codegen.cpp?rev=320191&r1=320190&r2=320191&view=diff
==
--- cfe/trunk/test/OpenMP/for_lastprivate_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/for_lastprivate_codegen.cpp Fri Dec  8 12:18:58 2017
@@ -614,6 +614,7 @@ int main() {
 // CHECK: ret void
 
 // CHECK: define internal void [[MAIN_MICROTASK3]](i{{[0-9]+}}* noalias 
[[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}})
+// CHECK: alloca i8,
 // CHECK: [[CNT_PRIV:%.+]] = alloca i8,
 
 // CHECK: [[GTID_REF:%.+]] = load i{{[0-9]+}}*, 

[PATCH] D41016: [Sema] Fix crash in unused-lambda-capture warning for VLAs

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D41016



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

[PATCH] D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/modernize/ReplaceUncaughtExceptionCheck.cpp:59
+  if (!BeginLoc.isMacroID()) {
+Diag << FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength),
+   "s");

xazax.hun wrote:
> aaron.ballman wrote:
> > JonasToth wrote:
> > > koldaniel wrote:
> > > > JonasToth wrote:
> > > > > Could the location not simply be `EndLoc`?
> > > > As I could see, when EndLoc was used in Diag (diag(..) of 
> > > > CreateInsertion(...) methods,  it still pointed to the beginning of the 
> > > > token marking the whole call. So if the CreateInsertion function looked 
> > > > like this: 
> > > > 
> > > >   Diag << 
> > > > FixItHint::CreateInsertion(EndLoc.getLocWithOffset(TextLength), "s");
> > > > 
> > > > had the same effect after applying the fix its as
> > > > 
> > > > Diag << 
> > > > FixItHint::CreateInsertion(BeginLoc.getLocWithOffset(TextLength), "s");
> > > > 
> > > > for calls like 'uncaught_exception()' (without std:: namespace written 
> > > > at the beginning, because it increases TextLength, and in case of 
> > > > EndLoc the offset will be counted from the beginning of the function 
> > > > name, not the namespace identifier).
> > > Thats odd. You could do a Replacement with `getSourceRange` probably. 
> > > Sounds more inefficient, but might be shorter in Code.
> > This fixit can break code if the code disallows narrowing conversions. e.g.,
> > ```
> > bool b{std::uncaught_exception()};
> > ```
> > In this case, the fixit will take the deprecated code and make it 
> > ill-formed instead. Perhaps a better fix-it would be to transform the code 
> > into `(std::uncaught_exceptions() > 0)` to keep the resulting expression 
> > type a `bool` and still not impact operator precedence?
> Good point, but this kind of fix it is a bit ugly. Maybe skipping the fixit 
> in narrowing cases or only generate the more complicated replacement in the 
> narrowing case would be nicer. 
I would be fine with only using the uglier version if there's a narrowing 
conversion, or removing the fixit entirely in the narrowing case.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40787



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

[PATCH] D40948: Switch Clang's default C++ language target to C++14

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1733
   // The PS4 uses C++11 as the default C++ standard.
-  if (T.isPS4())
-LangStd = LangStandard::lang_gnucxx11;
-  else
-LangStd = LangStandard::lang_gnucxx98;
+  LangStd = LangStandard::lang_gnucxx14;
   break;

filcab wrote:
> t.p.northover wrote:
> > filcab wrote:
> > > Why are you changing the PS4 default too?
> > Paul Robinson indicated that it was feasible back in March: 
> > http://lists.llvm.org/pipermail/cfe-dev/2017-March/052986.html. If that's 
> > changed I'd be happy to put it back to C++11, but he's one of the main 
> > people (rightly) bugging me about this so I'd be slightly surprised.
> > 
> > I also notice the comment crept back in. Bother.
> Sounds good, then. If Paul is happy with that change, I don't see a problem 
> there (assuming you do get rid of the comment for good :-) ).
> 
> Thank you,
> 
>  Filipe
Yes, PS4 product management is happy to move forward. 


Repository:
  rC Clang

https://reviews.llvm.org/D40948



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

[PATCH] D40625: Harmonizing attribute GNU/C++ spellings

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/Attr.td:602
 def AnalyzerNoReturn : InheritableAttr {
-  let Spellings = [GNU<"analyzer_noreturn">];
+  let Spellings = [Clang<"analyzer_noreturn">];
   let Documentation = [Undocumented];

alexfh wrote:
> aaron.ballman wrote:
> > dcoughlin wrote:
> > > aaron.ballman wrote:
> > > > dcoughlin wrote:
> > > > > aaron.ballman wrote:
> > > > > > rsmith wrote:
> > > > > > > Hmm, should the clang static analyzer reuse the `clang::` 
> > > > > > > namespace, or should it get its own?
> > > > > > Good question, I don't have strong opinions on the answer here, but 
> > > > > > perhaps @dcoughlin does?
> > > > > > 
> > > > > > If we want to use a separate namespace for the analyzer, would we 
> > > > > > want to use that same namespace for any clang-tidy specific 
> > > > > > attributes? Or should clang-tidy get its own namespace? (Do we ever 
> > > > > > plan to execute clang-tidy through the clang driver? That might 
> > > > > > change our answer.)
> > > > > How would this look if we added a special namespace for the clang 
> > > > > static analyzer? Would this lead to duplication (say, 
> > > > > [[clang_analyzer::analyzer_noreturn]]) so that we keep the 
> > > > > "analyzer_" prefix for __attribute__((analyzer_noreturn))? Or could 
> > > > > we have the "analyzer_" prefix only for GNU-style attributes but not 
> > > > > for C++ (for example, [[clang_analyzer::noreturn]])?
> > > > > 
> > > > > As for clang-tidy, I think it probably makes sense for it to have its 
> > > > > own namespace, but we should ask @alexfh.
> > > > > How would this look if we added a special namespace for the clang 
> > > > > static analyzer? Would this lead to duplication (say, 
> > > > > [[clang_analyzer::analyzer_noreturn]]) so that we keep the 
> > > > > "analyzer_" prefix for attribute((analyzer_noreturn))? Or could we 
> > > > > have the "analyzer_" prefix only for GNU-style attributes but not for 
> > > > > C++ (for example, [[clang_analyzer::noreturn]])?
> > > > 
> > > > We have the ability to do whatever we'd like there. Given that the 
> > > > semantics are so similar to `[[noreturn]]`, I think it would be 
> > > > reasonable to use `[[clang_analyzer::noreturn]]` and 
> > > > `__attribute__((analyzer_noreturn))` if that's the direction you think 
> > > > is best.
> > > > 
> > > > > As for clang-tidy, I think it probably makes sense for it to have its 
> > > > > own namespace, but we should ask @alexfh.
> > > > 
> > > > I'm less enthusiastic about giving clang-tidy a vendor namespace that's 
> > > > separate from the static analyzer, should the need arise. My biggest 
> > > > concern there is that I would *really* like to see clang-tidy be more 
> > > > tightly integrated with the clang driver (so users don't have to 
> > > > manually execute a secondary tool). If that were to happen, then the 
> > > > user experience would be that there are two vendor namespaces both 
> > > > related to analyzer attributes.
> > > > 
> > > > That said, I would also not be opposed to putting all of these 
> > > > attributes under the `clang` vendor namespace and not having a separate 
> > > > vendor for the analyzer or clang-tidy.
> > > I would be find with keeping all of these things under the `clang` 
> > > namespace, too.
> > > 
> > > That said, I do think there is some value in having a namespace for 
> > > analyzer attributes separate from clang proper because the namespace 
> > > would make it more clear that the attribute doesn't affect code 
> > > generation.
> > I've changed this one back to the GNU spelling to give us time to decide 
> > how we want to handle analyzer attributes.
> > 
> > I'm not certain "does not affect codegen" is the correct measure to use for 
> > this, however. We have other attributes that muddy the water, such as 
> > `annotate`, or the format specifier attributes -- these don't (really) 
> > impact codegen in any way, but do impact more than just the analyzer. Given 
> > the integration of the analyzer with Clang (and the somewhat fluid nature 
> > of what is responsible for issuing diagnostics), I think we should proceed 
> > with caution on the idea of an analyzer-specific namespace.
> > 
> > However, do you have a list of attributes you think might qualify as being 
> > analyzer-only? I can make sure we leave those spellings alone in this patch.
> An argument against clang_tidy and clang_analyzer vendor namespaces is that 
> the choice of where to implement a certain check would be connected to adding 
> an attribute in one or both of these namespaces, which would complicate 
> things a bit. In case both clang-tidy and static analyzer use the same 
> argument, we'd need to have duplicate attributes. I definitely don't think we 
> need three `noreturn` attributes, for example.
Yeah, that's basically the concern I was getting at -- it really ties our hands 
on where the various checks live in a syntactic fashion

[PATCH] D41030: [libcxx] [test] Fix MSVC warnings, null pointer deref.

[Stephan T. Lavavej via Phabricator via cfe-commits]

STL_MSFT created this revision.

[libcxx] [test] Fix MSVC warnings, null pointer deref.

test/std/algorithms/alg.modifying.operations/alg.generate/generate_n.pass.cpp
Silence MSVC warning C4244. This is expected when passing
floating-point values for size.

test/std/utilities/template.bitset/bitset.members/to_ullong.pass.cpp
test/std/utilities/template.bitset/bitset.members/to_ulong.pass.cpp
Avoid MSVC "warning C4293: '<<': shift count negative or too big,
undefined behavior". MSVC sees (1ULL << N) and warns - being guarded
by const bool canFit is insufficient. A small change to the code
avoids the warning without the need for a pragma.

Remove a spurious printf() declaration from to_ullong.pass.cpp.

Change ULL to UL in to_ulong.pass.cpp. The ULL suffix was
probably copy-pasted.

test/std/utilities/tuple/tuple.general/ignore.pass.cpp
Use LIBCPP_STATIC_ASSERT for consistency with other files.

test/support/container_test_types.h
Fix a null pointer dereference, found by MSVC /analyze
warning C6011 "Dereferencing NULL pointer 'm_expected_args'."


https://reviews.llvm.org/D41030

Files:
  test/std/algorithms/alg.modifying.operations/alg.generate/generate_n.pass.cpp
  test/std/utilities/template.bitset/bitset.members/to_ullong.pass.cpp
  test/std/utilities/template.bitset/bitset.members/to_ulong.pass.cpp
  test/std/utilities/tuple/tuple.general/ignore.pass.cpp
  test/support/container_test_types.h


Index: test/support/container_test_types.h
===
--- test/support/container_test_types.h
+++ test/support/container_test_types.h
@@ -167,8 +167,10 @@
   // Return true if the construction was expected and false otherwise.
   // This should only be called by 'Allocator.construct'.
   bool check(detail::TypeID const& tid) {
-if (!m_expected_args)
+if (!m_expected_args) {
   assert(m_allow_unchecked);
+  return m_allow_unchecked;
+}
 bool res = *m_expected_args == tid;
 if (m_expected_count == -1 || --m_expected_count == -1)
   m_expected_args = nullptr;
Index: test/std/utilities/tuple/tuple.general/ignore.pass.cpp
===
--- test/std/utilities/tuple/tuple.general/ignore.pass.cpp
+++ test/std/utilities/tuple/tuple.general/ignore.pass.cpp
@@ -48,9 +48,7 @@
 {
 static_assert(test_ignore_constexpr(), "");
 }
-#if defined(_LIBCPP_VERSION)
 {
-static_assert(std::is_trivial::value, "");
+LIBCPP_STATIC_ASSERT(std::is_trivial::value, 
"");
 }
-#endif
 }
Index: test/std/utilities/template.bitset/bitset.members/to_ulong.pass.cpp
===
--- test/std/utilities/template.bitset/bitset.members/to_ulong.pass.cpp
+++ test/std/utilities/template.bitset/bitset.members/to_ulong.pass.cpp
@@ -39,9 +39,9 @@
 }
 
 { // test values bigger than can fit into the bitset
-const unsigned long val = 0x5AA5ULL;
+const unsigned long val = 0x5AA5UL;
 const bool canFit = N < sizeof(unsigned long) * CHAR_BIT;
-const unsigned long mask = canFit ? (1ULL << N) - 1 : (unsigned long)(-1);
+const unsigned long mask = canFit ? (1UL << (canFit ? N : 0)) - 1 : 
(unsigned long)(-1); // avoid compiler warnings
 std::bitset v(val);
 assert(v.to_ulong() == (val & mask)); // we shouldn't return bit patterns 
from outside the limits of the bitset.
 }
Index: test/std/utilities/template.bitset/bitset.members/to_ullong.pass.cpp
===
--- test/std/utilities/template.bitset/bitset.members/to_ullong.pass.cpp
+++ test/std/utilities/template.bitset/bitset.members/to_ullong.pass.cpp
@@ -8,7 +8,6 @@
 
//===--===//
 
 // test unsigned long long to_ullong() const;
-extern "C" int printf(const char *, ...);
 
 #include 
 #include 
@@ -40,7 +39,7 @@
 { // test values bigger than can fit into the bitset
 const unsigned long long val = 0x5555ULL;
 const bool canFit = N < sizeof(unsigned long long) * CHAR_BIT;
-const unsigned long long mask = canFit ? (1ULL << N) - 1 : (unsigned long 
long)(-1);
+const unsigned long long mask = canFit ? (1ULL << (canFit ? N : 0)) - 1 : 
(unsigned long long)(-1); // avoid compiler warnings
 std::bitset v(val);
 assert(v.to_ullong() == (val & mask)); // we shouldn't return bit patterns 
from outside the limits of the bitset.
 }
Index: 
test/std/algorithms/alg.modifying.operations/alg.generate/generate_n.pass.cpp
===
--- 
test/std/algorithms/alg.modifying.operations/alg.generate/generate_n.pass.cpp
+++ 
test/std/algorithms/alg.modifying.operations/alg.generate/generate_n.pass.cpp
@@ -15,6 +15,10 @@
 //   void
 //   generate_n(Iter first, Size n, Generator gen);
 
+#ifdef _MSC_VER
+#pragma warning(disable: 4244) // convers

[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

The "enforcement" listed on the C++ Core Guidelines is very unhelpful, so it 
might be worth bringing it up as an issue on their bug tracker. ES.100 
basically says "you know what we mean, wink wink" as enforcement and doesn't 
give any guidance as to what is safe or unsafe. It gives no exceptions, which I 
think is unlikely to be useful to most developers. For instance: `void 
f(unsigned i) { if (i > 0) {} }`, this is a mixed signed and unsigned 
comparison (literals are signed by default) -- should this check diagnose? How 
about `unsigned int i = 12; i += 1;`? ES.102 is equally as strange with "Flag 
unsigned literals (e.g. -2) used as container subscripts." That's a signed 
literal (2) with a negation operator -- do they mean "Flag container subscripts 
whose value is known to be negative", or something else?




Comment at: clang-tidy/cppcoreguidelines/MixedIntArithmeticCheck.cpp:31
+  binaryOperator(allOf(anyOf(hasOperatorName("+"), hasOperatorName("-"),
+ hasOperatorName("*"), hasOperatorName("/")),
+   hasEitherOperand(UnsignedIntegerOperand),

What about `%` or comparisons operators?



Comment at: test/clang-tidy/cppcoreguidelines-mixed-int-arithmetic.cpp:167-168
+void pure_signed() {
+  int SInt1 = 42u;
+  signed char SChar1 = 42u;
+  SignedEnum SE1 = SEnum1;

I think these are intended to be flagged under ES.102. "Flag results of 
unsigned arithmetic assigned to or printed as signed."


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40854



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

[PATCH] D40948: Switch Clang's default C++ language target to C++14

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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

LGTM with the PS4 comment removed. Thank you!

Please also update the documentation and the release notes.




Comment at: clang/test/CodeGenCXX/vtable-available-externally.cpp:275
 struct C {
   virtual D& operator=(const D&);
 };

t.p.northover wrote:
> rsmith wrote:
> > To make this test work in C++11 onwards, you need to add a virtual move 
> > assignment operator here:
> > 
> > ```
> > virtual D& operator=(D&&);
> > ```
> That didn't quite work. The issue appears to be that D has both of those 
> implicitly defined in C++14 mode, but only the move assignment operator is 
> used below. Speculative VTable emission requires all of them to be used.
> 
> So adding a "d = d;" line to the second g function fixes the test. Does that 
> sound sane to you, or am I missing the point?
This sounds like a good approach, thanks.


Repository:
  rC Clang

https://reviews.llvm.org/D40948



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

[PATCH] D40712: Add cc1 flag enabling the function stack size section that was added in r319430

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

Looks good to me, but I'm not too familiar with the clang driver code. So it 
would be good if you could at least add some more experienced clang developers 
into the reviewer list and wait some more days before committing.

I also wonder whether it would be possible to move the fact that it defaults to 
on for PS4 into the PS4CPU.cpp file somehow (not that things are distributed 
that cleanly right now anyway).


https://reviews.llvm.org/D40712



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

Re: r320162 - Revert "Unify implementation of our two different flavours of -Wtautological-compare."

[Bill Seurer via cfe-commits]

It also caused all the sanitizer builds to fail.  For example:

http://lab.llvm.org:8011/builders/sanitizer-ppc64le-linux/builds/3654

/home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm/utils/TableGen/X86RecognizableInstr.cpp:789:21: 
error: comparison of constant -1 with expression of type 
'llvm::X86Disassembler::OpcodeType' is always true 
[-Werror,-Wtautological-constant-out-of-range-compare]

  assert(opcodeType != (OpcodeType)-1 &&
 ~~ ^  ~~
/usr/include/assert.h:86:5: note: expanded from macro 'assert'
  ((expr)   \
^~~~
1 error generated.
utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/build.make:1022: recipe 
for target 
'utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/X86RecognizableInstr.cpp.o' 
failed
make[3]: *** 
[utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/X86RecognizableInstr.cpp.o] 
Error 1




And there are lots more warnings (which are flagged as errors for the 
sanitizer builds) when I run a build by hand.  For example:


[4001/4008] Building CXX object 
tools/clang/tools/libclang/CMakeFiles/libclang.dir/CIndex.cpp.o
In file included from 
/home/seurer/llvm/llvm-test2/tools/clang/tools/libclang/CIndex.cpp:23:
In file included from 
/home/seurer/llvm/llvm-test2/tools/clang/tools/libclang/CursorVisitor.h:16:
In file included from 
/home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/DeclVisitor.h:19:
In file included from 
/home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/DeclCXX.h:21:
In file included from 
/home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/Attr.h:19:
/home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/Expr.h:2745:17: 
warning: comparison of constant -1 with expression of type 'const 
clang::CastKind' is always true 
[-Wtautological-constant-out-of-range-compare]

assert(kind != CK_Invalid && "creating cast with invalid cast kind");
    ^  ~~
/usr/include/assert.h:89:5: note: expanded from macro 'assert'
  ((expr)   \
^~~~


On 12/08/2017 10:54 AM, Hans Wennborg via cfe-commits wrote:

Author: hans
Date: Fri Dec  8 08:54:08 2017
New Revision: 320162

URL: http://llvm.org/viewvc/llvm-project?rev=320162&view=rev
Log:
Revert "Unify implementation of our two different flavours of 
-Wtautological-compare."


Unify implementation of our two different flavours of -Wtautological-compare.

In so doing, fix a handful of remaining bugs where we would report false
positives or false negatives if we promote a signed value to an unsigned type
for the comparison.


This caused a new warning in Chromium:

../../base/trace_event/trace_log.cc:1545:29: error: comparison of constant 64
with expression of type 'unsigned int' is always true
[-Werror,-Wtautological-constant-out-of-range-compare]
   DCHECK(handle.event_index < TraceBufferChunk::kTraceBufferChunkSize);
  ~~ ^ ~~~

The 'unsigned int' is really a 6-bit bitfield, which is why it's always
less than 64.

I thought we didn't use to warn (with out-of-range-compare) when comparing
against the boundaries of a type?

Modified:
 cfe/trunk/lib/Sema/SemaChecking.cpp
 cfe/trunk/test/Sema/tautological-constant-compare.c
 cfe/trunk/test/Sema/tautological-constant-enum-compare.c
 cfe/trunk/test/SemaCXX/compare.cpp

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=320162&r1=320161&r2=320162&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Dec  8 08:54:08 2017
@@ -8662,113 +8662,54 @@ static bool isKnownToHaveUnsignedValue(E
  }
  
  namespace {

-/// The promoted range of values of a type. In general this has the
-/// following structure:
-///
-/// |---| . . . |---|
-/// ^   ^   ^   ^
-///Min   HoleMin  HoleMax  Max
-///
-/// ... where there is only a hole if a signed type is promoted to unsigned
-/// (in which case Min and Max are the smallest and largest representable
-/// values).
-struct PromotedRange {
-  // Min, or HoleMax if there is a hole.
-  llvm::APSInt PromotedMin;
-  // Max, or HoleMin if there is a hole.
-  llvm::APSInt PromotedMax;
-
-  PromotedRange(IntRange R, unsigned BitWidth, bool Unsigned) {
-if (R.Width == 0)
-  PromotedMin = PromotedMax = llvm::APSInt(BitWidth, Unsigned);
-else {
-  PromotedMin = llvm::APSInt::getMinValue(R.Width, R.NonNegative)
-.extOrTrunc(BitWidth);
-  PromotedMin.setIsUnsigned(Unsigned);
-
-  PromotedMax = llvm::APSInt::getMaxValue(R.Width, R.NonNegative)
-.extOrTrunc(BitWidth);
-  PromotedMax.setIsUnsigned(Unsigned);
-}
-  }
  
-  // Determine whether this range is co

Re: r320162 - Revert "Unify implementation of our two different flavours of -Wtautological-compare."

[Richard Smith via cfe-commits]

I'm going to reland without the changes to bit-field handling. If we want
those changes (which I think we do, based on the bugs it found in
selfhost), that'll need to be done more carefully, and I don't want to get
the refactoring and bugfixes here tangled up with that.

On 8 December 2017 at 13:27, Bill Seurer via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> It also caused all the sanitizer builds to fail.  For example:
>
> http://lab.llvm.org:8011/builders/sanitizer-ppc64le-linux/builds/3654
>
> /home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/
> llvm/utils/TableGen/X86RecognizableInstr.cpp:789:21: error: comparison of
> constant -1 with expression of type 'llvm::X86Disassembler::OpcodeType'
> is always true [-Werror,-Wtautological-constant-out-of-range-compare]
>   assert(opcodeType != (OpcodeType)-1 &&
>  ~~ ^  ~~
> /usr/include/assert.h:86:5: note: expanded from macro 'assert'
>   ((expr)   \
> ^~~~
> 1 error generated.
> utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/build.make:1022: recipe for
> target 
> 'utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/X86RecognizableInstr.cpp.o'
> failed
> make[3]: *** 
> [utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/X86RecognizableInstr.cpp.o]
> Error 1
>
>
>
> And there are lots more warnings (which are flagged as errors for the
> sanitizer builds) when I run a build by hand.  For example:
>
> [4001/4008] Building CXX object tools/clang/tools/libclang/CMa
> keFiles/libclang.dir/CIndex.cpp.o
> In file included from /home/seurer/llvm/llvm-test2/t
> ools/clang/tools/libclang/CIndex.cpp:23:
> In file included from /home/seurer/llvm/llvm-test2/t
> ools/clang/tools/libclang/CursorVisitor.h:16:
> In file included from /home/seurer/llvm/llvm-test2/t
> ools/clang/include/clang/AST/DeclVisitor.h:19:
> In file included from /home/seurer/llvm/llvm-test2/t
> ools/clang/include/clang/AST/DeclCXX.h:21:
> In file included from /home/seurer/llvm/llvm-test2/t
> ools/clang/include/clang/AST/Attr.h:19:
> /home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/Expr.h:2745:17:
> warning: comparison of constant -1 with expression of type 'const
> clang::CastKind' is always true [-Wtautological-constant-out-o
> f-range-compare]
> assert(kind != CK_Invalid && "creating cast with invalid cast kind");
> ^  ~~
> /usr/include/assert.h:89:5: note: expanded from macro 'assert'
>   ((expr)   \
> ^~~~
>
>
>
> On 12/08/2017 10:54 AM, Hans Wennborg via cfe-commits wrote:
>
>> Author: hans
>> Date: Fri Dec  8 08:54:08 2017
>> New Revision: 320162
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=320162&view=rev
>> Log:
>> Revert "Unify implementation of our two different flavours of
>> -Wtautological-compare."
>>
>> Unify implementation of our two different flavours of
>>> -Wtautological-compare.
>>>
>>> In so doing, fix a handful of remaining bugs where we would report false
>>> positives or false negatives if we promote a signed value to an unsigned
>>> type
>>> for the comparison.
>>>
>>
>> This caused a new warning in Chromium:
>>
>> ../../base/trace_event/trace_log.cc:1545:29: error: comparison of
>> constant 64
>> with expression of type 'unsigned int' is always true
>> [-Werror,-Wtautological-constant-out-of-range-compare]
>>DCHECK(handle.event_index < TraceBufferChunk::kTraceBufferChunkSize);
>>   ~~ ^ ~~~
>>
>> The 'unsigned int' is really a 6-bit bitfield, which is why it's always
>> less than 64.
>>
>> I thought we didn't use to warn (with out-of-range-compare) when comparing
>> against the boundaries of a type?
>>
>> Modified:
>>  cfe/trunk/lib/Sema/SemaChecking.cpp
>>  cfe/trunk/test/Sema/tautological-constant-compare.c
>>  cfe/trunk/test/Sema/tautological-constant-enum-compare.c
>>  cfe/trunk/test/SemaCXX/compare.cpp
>>
>> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaC
>> hecking.cpp?rev=320162&r1=320161&r2=320162&view=diff
>> 
>> ==
>> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Dec  8 08:54:08 2017
>> @@ -8662,113 +8662,54 @@ static bool isKnownToHaveUnsignedValue(E
>>   }
>> namespace {
>> -/// The promoted range of values of a type. In general this has the
>> -/// following structure:
>> -///
>> -/// |---| . . . |---|
>> -/// ^   ^   ^   ^
>> -///Min   HoleMin  HoleMax  Max
>> -///
>> -/// ... where there is only a hole if a signed type is promoted to
>> unsigned
>> -/// (in which case Min and Max are the smallest and largest representable
>> -/// values).
>> -struct PromotedRange {
>> -  // Min, or HoleMax if there is a hole.
>> -  llvm::APSInt Prom

Re: r320162 - Revert "Unify implementation of our two different flavours of -Wtautological-compare."

[Hans Wennborg via cfe-commits]

For what it's worth, Chromium's bitfield warning is easy to fix. The
"-1 vs enum" comparisons are more plentiful and less straightforward.
I'm guessing it's the same for the self-host.

On Fri, Dec 8, 2017 at 1:33 PM, Richard Smith via cfe-commits
 wrote:
> I'm going to reland without the changes to bit-field handling. If we want
> those changes (which I think we do, based on the bugs it found in selfhost),
> that'll need to be done more carefully, and I don't want to get the
> refactoring and bugfixes here tangled up with that.
>
> On 8 December 2017 at 13:27, Bill Seurer via cfe-commits
>  wrote:
>>
>> It also caused all the sanitizer builds to fail.  For example:
>>
>> http://lab.llvm.org:8011/builders/sanitizer-ppc64le-linux/builds/3654
>>
>>
>> /home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm/utils/TableGen/X86RecognizableInstr.cpp:789:21:
>> error: comparison of constant -1 with expression of type
>> 'llvm::X86Disassembler::OpcodeType' is always true
>> [-Werror,-Wtautological-constant-out-of-range-compare]
>>   assert(opcodeType != (OpcodeType)-1 &&
>>  ~~ ^  ~~
>> /usr/include/assert.h:86:5: note: expanded from macro 'assert'
>>   ((expr)   \
>> ^~~~
>> 1 error generated.
>> utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/build.make:1022: recipe for
>> target
>> 'utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/X86RecognizableInstr.cpp.o'
>> failed
>> make[3]: ***
>> [utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/X86RecognizableInstr.cpp.o]
>> Error 1
>>
>>
>>
>> And there are lots more warnings (which are flagged as errors for the
>> sanitizer builds) when I run a build by hand.  For example:
>>
>> [4001/4008] Building CXX object
>> tools/clang/tools/libclang/CMakeFiles/libclang.dir/CIndex.cpp.o
>> In file included from
>> /home/seurer/llvm/llvm-test2/tools/clang/tools/libclang/CIndex.cpp:23:
>> In file included from
>> /home/seurer/llvm/llvm-test2/tools/clang/tools/libclang/CursorVisitor.h:16:
>> In file included from
>> /home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/DeclVisitor.h:19:
>> In file included from
>> /home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/DeclCXX.h:21:
>> In file included from
>> /home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/Attr.h:19:
>> /home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/Expr.h:2745:17:
>> warning: comparison of constant -1 with expression of type 'const
>> clang::CastKind' is always true
>> [-Wtautological-constant-out-of-range-compare]
>> assert(kind != CK_Invalid && "creating cast with invalid cast kind");
>> ^  ~~
>> /usr/include/assert.h:89:5: note: expanded from macro 'assert'
>>   ((expr)   \
>> ^~~~
>>
>>
>>
>> On 12/08/2017 10:54 AM, Hans Wennborg via cfe-commits wrote:
>>>
>>> Author: hans
>>> Date: Fri Dec  8 08:54:08 2017
>>> New Revision: 320162
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=320162&view=rev
>>> Log:
>>> Revert "Unify implementation of our two different flavours of
>>> -Wtautological-compare."
>>>
 Unify implementation of our two different flavours of
 -Wtautological-compare.

 In so doing, fix a handful of remaining bugs where we would report false
 positives or false negatives if we promote a signed value to an unsigned
 type
 for the comparison.
>>>
>>>
>>> This caused a new warning in Chromium:
>>>
>>> ../../base/trace_event/trace_log.cc:1545:29: error: comparison of
>>> constant 64
>>> with expression of type 'unsigned int' is always true
>>> [-Werror,-Wtautological-constant-out-of-range-compare]
>>>DCHECK(handle.event_index < TraceBufferChunk::kTraceBufferChunkSize);
>>>   ~~ ^ ~~~
>>>
>>> The 'unsigned int' is really a 6-bit bitfield, which is why it's always
>>> less than 64.
>>>
>>> I thought we didn't use to warn (with out-of-range-compare) when
>>> comparing
>>> against the boundaries of a type?
>>>
>>> Modified:
>>>  cfe/trunk/lib/Sema/SemaChecking.cpp
>>>  cfe/trunk/test/Sema/tautological-constant-compare.c
>>>  cfe/trunk/test/Sema/tautological-constant-enum-compare.c
>>>  cfe/trunk/test/SemaCXX/compare.cpp
>>>
>>> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=320162&r1=320161&r2=320162&view=diff
>>>
>>> ==
>>> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Dec  8 08:54:08 2017
>>> @@ -8662,113 +8662,54 @@ static bool isKnownToHaveUnsignedValue(E
>>>   }
>>> namespace {
>>> -/// The promoted range of values of a type. In general this has the
>>> -/// following structure:
>>> -///
>>> -/// |---| . . . |---|
>>> -/// ^   ^

[PATCH] D40968: [OpenMP] Diagnose function name on the link clause

[Kelvin Li via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaOpenMP.cpp:12671
+void Sema::checkDeclIsAllowedInOpenMPTarget(Expr *E, Decl *D,
+const DeclarationNameInfo *Id) {
   if (!D || D->isInvalidDecl())

ABataev wrote:
> kkwli0 wrote:
> > ABataev wrote:
> > > You can get `DeclarationNameInfo` from the `FunctionDecl`:
> > > ```
> > > FD->getNameInfo()
> > > ```
> > This FD->getNameInfo() will only give the name info from the function 
> > definition.  What we need here is the name info for 'foo' that appears on 
> > the pragma in order to give us
> > 
> > ```
> > d2.c:2:33: error: function name is not allowed in 'link' clause
> > #pragma omp declare target link(foo)
> > ^
> > ```
> Then just pass `SourceLocation`
Sure.


https://reviews.llvm.org/D40968



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

[libcxx] r320201 - [libc++] Unbreak Apple buildbots

[Shoaib Meenai via cfe-commits]

Author: smeenai
Date: Fri Dec  8 13:50:32 2017
New Revision: 320201

URL: http://llvm.org/viewvc/llvm-project?rev=320201&view=rev
Log:
[libc++] Unbreak Apple buildbots

These buildbots are using the deprecated target name install-libcxx-headers
instead of the more up to date install-cxx-headers, so I need to add an
install-libcxx-headers-stripped target to satisfy them.

Modified:
libcxx/trunk/include/CMakeLists.txt

Modified: libcxx/trunk/include/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/CMakeLists.txt?rev=320201&r1=320200&r2=320201&view=diff
==
--- libcxx/trunk/include/CMakeLists.txt (original)
+++ libcxx/trunk/include/CMakeLists.txt Fri Dec  8 13:50:32 2017
@@ -63,6 +63,7 @@ if (LIBCXX_INSTALL_HEADERS)
 
 add_custom_target(libcxx-headers)
 add_custom_target(install-libcxx-headers DEPENDS install-cxx-headers)
+add_custom_target(install-libcxx-headers-stripped DEPENDS 
install-cxx-headers-stripped)
   endif()
 
 endif()


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

[PATCH] D41031: [clangd] (Attempt to) read clang-format file for document formatting

[Raoul Wols via Phabricator via cfe-commits]

rwols created this revision.

Takes into account the clang-format file of the project, if any.
Reverts to LLVM if nothing is found. Replies with an error if any error occured.
For instance, a parse error in the clang-format YAML file.


https://reviews.llvm.org/D41031

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h

Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -286,12 +286,19 @@
   /// given a header file and vice versa.
   llvm::Optional switchSourceHeader(PathRef Path);
 
-  /// Run formatting for \p Rng inside \p File.
-  std::vector formatRange(PathRef File, Range Rng);
-  /// Run formatting for the whole \p File.
-  std::vector formatFile(PathRef File);
-  /// Run formatting after a character was typed at \p Pos in \p File.
-  std::vector formatOnType(PathRef File, Position Pos);
+  /// Run formatting for \p Rng inside \p File with content \p Code.
+  llvm::Expected formatRange(StringRef Code,
+PathRef File, Range Rng);
+
+  /// Run formatting for the whole \p File with content \p Code.
+  llvm::Expected formatFile(StringRef Code,
+   PathRef File);
+
+  /// Run formatting after a character was typed at \p Pos in \p File with
+  /// content \p Code.
+  llvm::Expected
+  formatOnType(StringRef Code, PathRef File, Position Pos);
+
   /// Rename all occurrences of the symbol at the \p Pos in \p File to
   /// \p NewName.
   Expected> rename(PathRef File, Position Pos,
@@ -311,6 +318,13 @@
   void onFileEvent(const DidChangeWatchedFilesParams &Params);
 
 private:
+
+  /// FIXME: This stats several files to find a .clang-format file. I/O can be
+  /// slow. Think of a way to cache this.
+  llvm::Expected
+  formatCode(llvm::StringRef Code, PathRef File,
+ ArrayRef Ranges);
+
   std::future
   scheduleReparseAndDiags(PathRef File, VersionedDraft Contents,
   std::shared_ptr Resources,
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -38,16 +38,6 @@
   std::promise &Promise;
 };
 
-std::vector formatCode(StringRef Code, StringRef Filename,
- ArrayRef Ranges) {
-  // Call clang-format.
-  // FIXME: Don't ignore style.
-  format::FormatStyle Style = format::getLLVMStyle();
-  auto Result = format::reformat(Style, Code, Ranges, Filename);
-
-  return std::vector(Result.begin(), Result.end());
-}
-
 std::string getStandardResourceDir() {
   static int Dummy; // Just an address in this process.
   return CompilerInvocation::GetResourcesPath("clangd", (void *)&Dummy);
@@ -331,26 +321,23 @@
   return make_tagged(std::move(Result), TaggedFS.Tag);
 }
 
-std::vector ClangdServer::formatRange(PathRef File,
-Range Rng) {
-  std::string Code = getDocument(File);
-
+llvm::Expected
+ClangdServer::formatRange(StringRef Code, PathRef File, Range Rng) {
   size_t Begin = positionToOffset(Code, Rng.start);
   size_t Len = positionToOffset(Code, Rng.end) - Begin;
   return formatCode(Code, File, {tooling::Range(Begin, Len)});
 }
 
-std::vector ClangdServer::formatFile(PathRef File) {
+llvm::Expected ClangdServer::formatFile(StringRef Code,
+   PathRef File) {
   // Format everything.
-  std::string Code = getDocument(File);
   return formatCode(Code, File, {tooling::Range(0, Code.size())});
 }
 
-std::vector ClangdServer::formatOnType(PathRef File,
- Position Pos) {
+llvm::Expected
+ClangdServer::formatOnType(StringRef Code, PathRef File, Position Pos) {
   // Look for the previous opening brace from the character position and
   // format starting from there.
-  std::string Code = getDocument(File);
   size_t CursorPos = positionToOffset(Code, Pos);
   size_t PreviousLBracePos = StringRef(Code).find_last_of('{', CursorPos);
   if (PreviousLBracePos == StringRef::npos)
@@ -509,6 +496,20 @@
   return llvm::None;
 }
 
+llvm::Expected
+ClangdServer::formatCode(llvm::StringRef Code, PathRef File,
+ ArrayRef Ranges) {
+  // Call clang-format.
+  auto TaggedFS = FSProvider.getTaggedFileSystem(File);
+  auto StyleOrError =
+  format::getStyle("file", File, "LLVM", Code, TaggedFS.Value.get());
+  if (!StyleOrError) {
+return StyleOrError.takeError();
+  } else {
+return format::reformat(StyleOrError.get(), Code, Ranges, File);
+  }
+}
+
 std::future ClangdServer::scheduleReparseAndDiags(
 PathRef File, VersionedDraft Contents, std::s

[PATCH] D39430: [clangd] various fixes

[Raoul Wols via Phabricator via cfe-commits]

rwols added a comment.

This differential is now split into https://reviews.llvm.org/D41031 and 
https://reviews.llvm.org/D40860.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D39430



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

[PATCH] D41032: [CodeGen] Implement _InterlockedCompareExchange128 intrinsic

[Colden Cullen via Phabricator via cfe-commits]

colden created this revision.

InterlockedCompareExchange128 is a bit more complicated than the other 
InterlockedCompareExchange functions, so it requires a bit more work. It 
doesn't directly refer to 128bit ints, instead it takes pointers to 64bit ints 
for Destination and ComparandResult, and exchange is taken as two 64bit ints 
(high & low). The previous value is written to ComparandResult, and success is 
returned. This implementation does the following in order to produce a cmpxchg 
instruction:

1. Cast everything to 128bit ints or int pointers, and glues together the 
Exchange values
2. Reads from CompareandResult to get the comparand
3. Calls cmpxchg volatile (on X86 this will produce a lock cmpxchg16b 
instruction)
  1. Result 0 (previous value) is written back to ComparandResult
  2. Result 1 (success bool) is zext'ed to a uchar and returned

Resolves bug 35251 


https://reviews.llvm.org/D41032

Files:
  llvm/tools/clang/include/clang/Basic/Builtins.def
  llvm/tools/clang/lib/CodeGen/CGBuiltin.cpp
  llvm/tools/clang/test/CodeGen/ms-intrinsics.c


Index: llvm/tools/clang/test/CodeGen/ms-intrinsics.c
===
--- llvm/tools/clang/test/CodeGen/ms-intrinsics.c
+++ llvm/tools/clang/test/CodeGen/ms-intrinsics.c
@@ -329,6 +329,25 @@
 // CHECK: ret i64 [[RESULT]]
 // CHECK: }
 
+unsigned char test_InterlockedCompareExchange128(__int64 volatile 
*Destination, __int64 ExchangeHigh, __int64 ExchangeLow, __int64* 
ComparandResult) {
+  return _InterlockedCompareExchange128(Destination, ExchangeHigh, 
ExchangeLow, ComparandResult);
+}
+// CHECK-X64: define{{.*}}i8 @test_InterlockedCompareExchange128(i64*{{[a-z_ 
]*}}%Destination, i64{{[a-z_ ]*}}%ExchangeHigh, i64{{[a-z_ ]*}}%ExchangeLow, 
i64*{{[a-z_ ]*}}%ComparandResult){{.*}}{
+// CHECK-X64: [[DST:%[0-9]+]] = bitcast i64* %Destination to i128*
+// CHECK-X64: [[EH:%[0-9]+]] = zext i64 %ExchangeHigh to i128
+// CHECK-X64: [[EL:%[0-9]+]] = zext i64 %ExchangeLow to i128
+// CHECK-X64: [[CNR:%[0-9]+]] = bitcast i64* %ComparandResult to i128*
+// CHECK-X64: [[EHS:%[0-9]+]] = shl nuw i128 [[EH]], 64
+// CHECK-X64: [[EXP:%[0-9]+]] = or i128 [[EHS]], [[EL]]
+// CHECK-X64: [[ORG:%[0-9]+]] = load i128, i128* [[CNR]], align 16
+// CHECK-X64: [[RES:%[0-9]+]] = cmpxchg volatile i128* [[DST]], i128 [[ORG]], 
i128 [[EXP]] seq_cst seq_cst
+// CHECK-X64: [[OLD:%[0-9]+]] = extractvalue { i128, i1 } [[RES]], 0
+// CHECK-X64: store i128 [[OLD]], i128* [[CNR]], align 16
+// CHECK-X64: [[SUC1:%[0-9]+]] = extractvalue { i128, i1 } [[RES]], 1
+// CHECK-X64: [[SUC8:%[0-9]+]] = zext i1 [[SUC1]] to i8
+// CHECK-X64: ret i8 [[SUC8]]
+// CHECK-X64: }
+
 short test_InterlockedIncrement16(short volatile *Addend) {
   return _InterlockedIncrement16(Addend);
 }
Index: llvm/tools/clang/lib/CodeGen/CGBuiltin.cpp
===
--- llvm/tools/clang/lib/CodeGen/CGBuiltin.cpp
+++ llvm/tools/clang/lib/CodeGen/CGBuiltin.cpp
@@ -2521,6 +2521,47 @@
   CXI->setVolatile(true);
   return RValue::get(Builder.CreateExtractValue(CXI, 0));
   }
+  case Builtin::BI_InterlockedCompareExchange128: {
+// InterlockedCompareExchange128 doesn't directly refer to 128bit ints,
+// instead it takes pointers to 64bit ints for Destination and
+// ComparandResult, and exchange is taken as two 64bit ints (high & low).
+// The previous value is written to ComparandResult, and success is 
returned.
+
+llvm::Type *Int128Ty = llvm::IntegerType::get(getLLVMContext(), 128);
+llvm::Type *Int128PtrTy = Int128Ty->getPointerTo();
+
+Value *Destination = Builder.CreateBitCast(
+  EmitScalarExpr(E->getArg(0)), Int128PtrTy);
+Value *ExchangeHigh128 = Builder.CreateZExt(
+  EmitScalarExpr(E->getArg(1)), Int128Ty);
+Value *ExchangeLow128 = Builder.CreateZExt(
+  EmitScalarExpr(E->getArg(2)), Int128Ty);
+Address ComparandResult(
+  Builder.CreateBitCast(EmitScalarExpr(E->getArg(3)), Int128PtrTy),
+  getContext().toCharUnitsFromBits(128));
+
+Value *Exchange = Builder.CreateOr(
+  Builder.CreateShl(ExchangeHigh128, 64, "", false, false),
+  ExchangeLow128);
+
+Value *Comparand = Builder.CreateLoad(ComparandResult);
+
+AtomicCmpXchgInst *CXI = Builder.CreateAtomicCmpXchg(
+  Destination, Comparand, Exchange,
+  AtomicOrdering::SequentiallyConsistent,
+  AtomicOrdering::SequentiallyConsistent);
+CXI->setVolatile(true);
+
+// Write the result back to the inout pointer
+Builder.CreateStore(Builder.CreateExtractValue(CXI, 0), ComparandResult);
+
+// Get success boolean
+Value *Success = Builder.CreateExtractValue(CXI, 1);
+
+// zext the success boolean and return it
+return RValue::get(
+  Builder.CreateZExt(Success, ConvertType(E->getType(;
+  }
   case Builtin::BI_InterlockedIncrement16:
   case Builtin::BI_InterlockedIncrement:
 return RVal

Re: r320162 - Revert "Unify implementation of our two different flavours of -Wtautological-compare."

[Richard Smith via cfe-commits]

Selfhost failures are ultimately caused by:

https://github.com/llvm-mirror/llvm/blob/master/utils/TableGen/X86RecognizableInstr.cpp#L709
https://github.com/llvm-mirror/clang/blob/master/include/clang/AST/OperationKinds.h#L26

These are genuine bugs: assigning -1 into an enumeration with no negative
enumerators results in undefined behavior. We should probably have a
warning for the casts here rather than only detecting the problem under
-Wtautological-compare, but I would not be surprised to find that Clang
miscompiles itself with -fstrict-enums due to the above two issues.

On 8 December 2017 at 13:40, Hans Wennborg via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> For what it's worth, Chromium's bitfield warning is easy to fix. The
> "-1 vs enum" comparisons are more plentiful and less straightforward.
> I'm guessing it's the same for the self-host.
>
> On Fri, Dec 8, 2017 at 1:33 PM, Richard Smith via cfe-commits
>  wrote:
> > I'm going to reland without the changes to bit-field handling. If we want
> > those changes (which I think we do, based on the bugs it found in
> selfhost),
> > that'll need to be done more carefully, and I don't want to get the
> > refactoring and bugfixes here tangled up with that.
> >
> > On 8 December 2017 at 13:27, Bill Seurer via cfe-commits
> >  wrote:
> >>
> >> It also caused all the sanitizer builds to fail.  For example:
> >>
> >> http://lab.llvm.org:8011/builders/sanitizer-ppc64le-linux/builds/3654
> >>
> >>
> >> /home/buildbots/ppc64le-sanitizer/sanitizer-ppc64le/
> build/llvm/utils/TableGen/X86RecognizableInstr.cpp:789:21:
> >> error: comparison of constant -1 with expression of type
> >> 'llvm::X86Disassembler::OpcodeType' is always true
> >> [-Werror,-Wtautological-constant-out-of-range-compare]
> >>   assert(opcodeType != (OpcodeType)-1 &&
> >>  ~~ ^  ~~
> >> /usr/include/assert.h:86:5: note: expanded from macro 'assert'
> >>   ((expr)
>  \
> >> ^~~~
> >> 1 error generated.
> >> utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/build.make:1022: recipe
> for
> >> target
> >> 'utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/
> X86RecognizableInstr.cpp.o'
> >> failed
> >> make[3]: ***
> >> [utils/TableGen/CMakeFiles/obj.llvm-tblgen.dir/
> X86RecognizableInstr.cpp.o]
> >> Error 1
> >>
> >>
> >>
> >> And there are lots more warnings (which are flagged as errors for the
> >> sanitizer builds) when I run a build by hand.  For example:
> >>
> >> [4001/4008] Building CXX object
> >> tools/clang/tools/libclang/CMakeFiles/libclang.dir/CIndex.cpp.o
> >> In file included from
> >> /home/seurer/llvm/llvm-test2/tools/clang/tools/libclang/CIndex.cpp:23:
> >> In file included from
> >> /home/seurer/llvm/llvm-test2/tools/clang/tools/libclang/
> CursorVisitor.h:16:
> >> In file included from
> >> /home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/
> DeclVisitor.h:19:
> >> In file included from
> >> /home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/
> DeclCXX.h:21:
> >> In file included from
> >> /home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/Attr.h:19:
> >> /home/seurer/llvm/llvm-test2/tools/clang/include/clang/AST/
> Expr.h:2745:17:
> >> warning: comparison of constant -1 with expression of type 'const
> >> clang::CastKind' is always true
> >> [-Wtautological-constant-out-of-range-compare]
> >> assert(kind != CK_Invalid && "creating cast with invalid cast
> kind");
> >> ^  ~~
> >> /usr/include/assert.h:89:5: note: expanded from macro 'assert'
> >>   ((expr)
>  \
> >> ^~~~
> >>
> >>
> >>
> >> On 12/08/2017 10:54 AM, Hans Wennborg via cfe-commits wrote:
> >>>
> >>> Author: hans
> >>> Date: Fri Dec  8 08:54:08 2017
> >>> New Revision: 320162
> >>>
> >>> URL: http://llvm.org/viewvc/llvm-project?rev=320162&view=rev
> >>> Log:
> >>> Revert "Unify implementation of our two different flavours of
> >>> -Wtautological-compare."
> >>>
>  Unify implementation of our two different flavours of
>  -Wtautological-compare.
> 
>  In so doing, fix a handful of remaining bugs where we would report
> false
>  positives or false negatives if we promote a signed value to an
> unsigned
>  type
>  for the comparison.
> >>>
> >>>
> >>> This caused a new warning in Chromium:
> >>>
> >>> ../../base/trace_event/trace_log.cc:1545:29: error: comparison of
> >>> constant 64
> >>> with expression of type 'unsigned int' is always true
> >>> [-Werror,-Wtautological-constant-out-of-range-compare]
> >>>DCHECK(handle.event_index < TraceBufferChunk::
> kTraceBufferChunkSize);
> >>>   ~~ ^ ~~~
> >>>
> >>> The 'unsigned int' is really a 6-bit bitfield, which is why it's always
> >>> less than 64.
> >>>
> >>> I thought we didn't use to warn (with out-of-range-compare) when
> >>> comparing
> >>> against the boundaries of a type?
> >>>
> >>> Modified:
> >>>  cfe/trunk/lib/Sema/SemaChecking.cpp
> >>>  cfe/trunk/test/Sema/tautological-co

r320207 - [Lex] Fix some Clang-tidy modernize and Include What You Use warnings; other minor fixes (NFC).

[Eugene Zelenko via cfe-commits]

Author: eugenezelenko
Date: Fri Dec  8 14:39:26 2017
New Revision: 320207

URL: http://llvm.org/viewvc/llvm-project?rev=320207&view=rev
Log:
[Lex] Fix some Clang-tidy modernize and Include What You Use warnings; other 
minor fixes (NFC).

Modified:
cfe/trunk/include/clang/Lex/Lexer.h
cfe/trunk/include/clang/Lex/Pragma.h
cfe/trunk/include/clang/Lex/PreprocessingRecord.h
cfe/trunk/include/clang/Lex/PreprocessorOptions.h
cfe/trunk/include/clang/Lex/TokenLexer.h
cfe/trunk/lib/Lex/Lexer.cpp
cfe/trunk/lib/Lex/Pragma.cpp
cfe/trunk/lib/Lex/PreprocessingRecord.cpp
cfe/trunk/lib/Lex/TokenLexer.cpp

Modified: cfe/trunk/include/clang/Lex/Lexer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Lexer.h?rev=320207&r1=320206&r2=320207&view=diff
==
--- cfe/trunk/include/clang/Lex/Lexer.h (original)
+++ cfe/trunk/include/clang/Lex/Lexer.h Fri Dec  8 14:39:26 2017
@@ -1,4 +1,4 @@
-//===--- Lexer.h - C Language Family Lexer --*- C++ 
-*-===//
+//===- Lexer.h - C Language Family Lexer *- C++ 
-*-===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -15,25 +15,39 @@
 #define LLVM_CLANG_LEX_LEXER_H
 
 #include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/PreprocessorLexer.h"
+#include "clang/Lex/Token.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
 #include 
+#include 
 #include 
 
+namespace llvm {
+
+class MemoryBuffer;
+
+} // namespace llvm
+
 namespace clang {
-class DiagnosticsEngine;
-class SourceManager;
-class Preprocessor;
+
 class DiagnosticBuilder;
+class Preprocessor;
+class SourceManager;
 
 /// ConflictMarkerKind - Kinds of conflict marker which the lexer might be
 /// recovering from.
 enum ConflictMarkerKind {
   /// Not within a conflict marker.
   CMK_None,
+
   /// A normal or diff3 conflict marker, initiated by at least 7 "<"s,
   /// separated by at least 7 "="s or "|"s, and terminated by at least 7 ">"s.
   CMK_Normal,
+
   /// A Perforce-style conflict marker, initiated by 4 ">"s,
   /// separated by 4 "="s, and terminated by 4 "<"s.
   CMK_Perforce
@@ -43,17 +57,17 @@ enum ConflictMarkerKind {
 /// PreprocessorOptions::PrecompiledPreambleBytes.
 /// The preamble includes the BOM, if any.
 struct PreambleBounds {
-  PreambleBounds(unsigned Size, bool PreambleEndsAtStartOfLine)
-: Size(Size),
-  PreambleEndsAtStartOfLine(PreambleEndsAtStartOfLine) {}
-
   /// \brief Size of the preamble in bytes.
   unsigned Size;
+
   /// \brief Whether the preamble ends at the start of a new line.
   ///
   /// Used to inform the lexer as to whether it's starting at the beginning of
   /// a line after skipping the preamble.
   bool PreambleEndsAtStartOfLine;
+
+  PreambleBounds(unsigned Size, bool PreambleEndsAtStartOfLine)
+  : Size(Size), PreambleEndsAtStartOfLine(PreambleEndsAtStartOfLine) {}
 };
 
 /// Lexer - This provides a simple interface that turns a text buffer into a
@@ -61,15 +75,27 @@ struct PreambleBounds {
 /// or buffering/seeking of tokens, only forward lexing is supported.  It 
relies
 /// on the specified Preprocessor object to handle preprocessor directives, 
etc.
 class Lexer : public PreprocessorLexer {
+  friend class Preprocessor;
+
   void anchor() override;
 
   
//======//
   // Constant configuration values for this lexer.
-  const char *BufferStart;   // Start of the buffer.
-  const char *BufferEnd; // End of the buffer.
-  SourceLocation FileLoc;// Location for start of file.
-  LangOptions LangOpts;  // LangOpts enabled by this language (cache).
-  bool Is_PragmaLexer;   // True if lexer for _Pragma handling.
+
+  // Start of the buffer.
+  const char *BufferStart;
+
+  // End of the buffer.
+  const char *BufferEnd;
+
+  // Location for start of file.
+  SourceLocation FileLoc;
+
+  // LangOpts enabled by this language (cache).
+  LangOptions LangOpts;
+
+  // True if lexer for _Pragma handling.
+  bool Is_PragmaLexer;
 
   
//======//
   // Context-specific lexing flags set by the preprocessor.
@@ -106,13 +132,9 @@ class Lexer : public PreprocessorLexer {
   // CurrentConflictMarkerState - The kind of conflict marker we are handling.
   ConflictMarkerKind CurrentConflictMarkerState;
 
-  Lexer(const Lexer &) = delete;
-  void operator=(const Lexer &) = delete;
-  friend class Preprocessor;
-
   void InitLexer(const char *BufStart, const char *BufPtr, const char *BufEnd);
-public:
 
+public:
   /// Lexer constructor - Create a new lexer object for the specified buffer
   /// with the specified preprocessor managing the lexing process.  This lexer
   /// assumes that the associa