[PATCH] D66273: [Tooling] Add a hack to work around issues with matcher binding in r368681.

[David L. Jones via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL368958: [Tooling] Add a hack to work around issues with 
matcher binding in r368681. (authored by dlj, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66273?vs=215316=215317#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66273

Files:
  cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp


Index: cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
@@ -98,8 +98,10 @@
   Matchers.reserve(Cases.size());
   for (const auto  : Cases) {
 std::string Tag = (TagBase + Twine(Case.first)).str();
-auto M = Case.second.Matcher.tryBind(Tag);
-assert(M && "RewriteRule matchers should be bindable.");
+// HACK: Many matchers are not bindable, so ensure that tryBind will work.
+DynTypedMatcher BoundMatcher(Case.second.Matcher);
+BoundMatcher.setAllowBind(true);
+auto M = BoundMatcher.tryBind(Tag);
 Matchers.push_back(*std::move(M));
   }
   return Matchers;


Index: cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/Transformer.cpp
@@ -98,8 +98,10 @@
   Matchers.reserve(Cases.size());
   for (const auto  : Cases) {
 std::string Tag = (TagBase + Twine(Case.first)).str();
-auto M = Case.second.Matcher.tryBind(Tag);
-assert(M && "RewriteRule matchers should be bindable.");
+// HACK: Many matchers are not bindable, so ensure that tryBind will work.
+DynTypedMatcher BoundMatcher(Case.second.Matcher);
+BoundMatcher.setAllowBind(true);
+auto M = BoundMatcher.tryBind(Tag);
 Matchers.push_back(*std::move(M));
   }
   return Matchers;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66273: [Tooling] Add a hack to work around issues with matcher binding in r368681.

[David L. Jones via Phabricator via cfe-commits]

dlj created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The change in r368681 contains a (probably unintentional) behavioral change for
rewrite rules with a single matcher. Previously, the single matcher would not
need to be bound (`joinCaseMatchers` returned it directly), even though a final
DynTypeMatcher was created and bound by `buildMatcher`. With the new change, a
single matcher will be bound, in addition to the final binding (which is now in
`buildMatchers`, but happens roughly at the same point in the overall flow).

This patch simply duplicates the "final matcher" trick: it creates an extra
DynTypedMatcher for each rewrite rule case matcher, and unconditionally makes it
bindable. This is probably not the right long-term fix, but it does allow
existing code to continue to work with this interface.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66273

Files:
  clang/lib/Tooling/Refactoring/Transformer.cpp


Index: clang/lib/Tooling/Refactoring/Transformer.cpp
===
--- clang/lib/Tooling/Refactoring/Transformer.cpp
+++ clang/lib/Tooling/Refactoring/Transformer.cpp
@@ -98,8 +98,10 @@
   Matchers.reserve(Cases.size());
   for (const auto  : Cases) {
 std::string Tag = (TagBase + Twine(Case.first)).str();
-auto M = Case.second.Matcher.tryBind(Tag);
-assert(M && "RewriteRule matchers should be bindable.");
+// HACK: Many matchers are not bindable, so ensure that tryBind will work.
+DynTypedMatcher BoundMatcher(Case.second.Matcher);
+BoundMatcher.setAllowBind(true);
+auto M = BoundMatcher.tryBind(Tag);
 Matchers.push_back(*std::move(M));
   }
   return Matchers;


Index: clang/lib/Tooling/Refactoring/Transformer.cpp
===
--- clang/lib/Tooling/Refactoring/Transformer.cpp
+++ clang/lib/Tooling/Refactoring/Transformer.cpp
@@ -98,8 +98,10 @@
   Matchers.reserve(Cases.size());
   for (const auto  : Cases) {
 std::string Tag = (TagBase + Twine(Case.first)).str();
-auto M = Case.second.Matcher.tryBind(Tag);
-assert(M && "RewriteRule matchers should be bindable.");
+// HACK: Many matchers are not bindable, so ensure that tryBind will work.
+DynTypedMatcher BoundMatcher(Case.second.Matcher);
+BoundMatcher.setAllowBind(true);
+auto M = BoundMatcher.tryBind(Tag);
 Matchers.push_back(*std::move(M));
   }
   return Matchers;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57303: [ToolChains] [NetBSD] Append -rpath for shared compiler-rt runtimes

[David L. Jones via Phabricator via cfe-commits]

dlj accepted this revision.
dlj added a comment.

In D57303#1373163 , @mgorny wrote:

> Given that this path changes with every clang release, so you're effectively 
> making a hard dependency on the clang version used to build the program, I 
> dare say some systems may either decide not to support shared runtimes at all 
> or use `ld.so.conf` or equivalent that can be dynamically changed.


Yeah, I don't think we should be stamping an `-rpath` unconditionally across 
targets. On top of static builds and `ld.so.conf`, this could end up nominating 
the wrong one of `DT_RPATH` and `DT_RUNPATH`, which is, unfortunately, 
//still// a target-specific distinction. By my quick reading of 
Driver/ToolChains/Linux.cpp, I think the combination of where we pass 
`--enable-new-dtags` and where we would pass `-rpath` would get this wrong on 
some systems (which would, infuriatingly, only //maybe// cause problems; and if 
so, only at runtime).


Repository:
  rC Clang

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

https://reviews.llvm.org/D57303



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

[PATCH] D48242: [ASTMatchers] Add support for matching the type of a friend decl.

[David L. Jones via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL334930: [ASTMatchers] Add support for matching the type of a 
friend decl. (authored by dlj, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D48242?vs=151593=151671#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D48242

Files:
  cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
  cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
  cfe/trunk/unittests/ASTMatchers/ASTMatchersNodeTest.cpp


Index: cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -38,6 +38,7 @@
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
@@ -120,10 +121,14 @@
 inline QualType getUnderlyingType(const ValueDecl ) {
   return Node.getType();
 }
-
 inline QualType getUnderlyingType(const TypedefNameDecl ) {
   return Node.getUnderlyingType();
 }
+inline QualType getUnderlyingType(const FriendDecl ) {
+  if (const TypeSourceInfo *TSI = Node.getFriendType())
+return TSI->getType();
+  return QualType();
+}
 
 /// Unifies obtaining the FunctionProtoType pointer from both
 /// FunctionProtoType and FunctionDecl nodes..
Index: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
===
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
@@ -2860,13 +2860,17 @@
 /// Example matches x (matcher = expr(hasType(cxxRecordDecl(hasName("X")
 /// and z (matcher = varDecl(hasType(cxxRecordDecl(hasName("X")
 /// and U (matcher = typedefDecl(hasType(asString("int")))
+/// and friend class X (matcher = friendDecl(hasType("X"))
 /// \code
 ///  class X {};
 ///  void y(X ) { x; X z; }
 ///  typedef int U;
+///  class Y { friend class X; };
 /// \endcode
 AST_POLYMORPHIC_MATCHER_P_OVERLOAD(
-hasType, AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, TypedefNameDecl, ValueDecl),
+hasType,
+AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, FriendDecl, TypedefNameDecl,
+ValueDecl),
 internal::Matcher, InnerMatcher, 0) {
   QualType QT = internal::getUnderlyingType(Node);
   if (!QT.isNull())
@@ -2885,18 +2889,21 @@
 ///
 /// Example matches x (matcher = expr(hasType(cxxRecordDecl(hasName("X")
 /// and z (matcher = varDecl(hasType(cxxRecordDecl(hasName("X")
+/// and friend class X (matcher = friendDecl(hasType("X"))
 /// \code
 ///  class X {};
 ///  void y(X ) { x; X z; }
+///  class Y { friend class X; };
 /// \endcode
 ///
 /// Usable as: Matcher, Matcher
-AST_POLYMORPHIC_MATCHER_P_OVERLOAD(hasType,
-   AST_POLYMORPHIC_SUPPORTED_TYPES(Expr,
-   ValueDecl),
-   internal::Matcher, InnerMatcher, 1) {
-  return qualType(hasDeclaration(InnerMatcher))
-  .matches(Node.getType(), Finder, Builder);
+AST_POLYMORPHIC_MATCHER_P_OVERLOAD(
+hasType, AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, FriendDecl, ValueDecl),
+internal::Matcher, InnerMatcher, 1) {
+  QualType QT = internal::getUnderlyingType(Node);
+  if (!QT.isNull())
+return qualType(hasDeclaration(InnerMatcher)).matches(QT, Finder, Builder);
+  return false;
 }
 
 /// Matches if the type location of the declarator decl's type matches
Index: cfe/trunk/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -160,6 +160,16 @@
   valueDecl(hasType(asString("void (void)");
 }
 
+TEST(FriendDecl, Matches) {
+  EXPECT_TRUE(matches("class Y { friend class X; };",
+  friendDecl(hasType(asString("class X");
+  EXPECT_TRUE(matches("class Y { friend class X; };",
+  friendDecl(hasType(recordDecl(hasName("X"));
+
+  EXPECT_TRUE(matches("class Y { friend void f(); };",
+  functionDecl(hasName("f"), hasParent(friendDecl();
+}
+
 TEST(Enum, DoesNotMatchClasses) {
   EXPECT_TRUE(notMatches("class X {};", enumDecl(hasName("X";
 }


Index: cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -38,6 +38,7 @@
 #include "clang/AST/ASTTypeTraits.h"
 #include 

[PATCH] D48242: [ASTMatchers] Add support for matching the type of a friend decl.

[David L. Jones via Phabricator via cfe-commits]

dlj added a comment.

Ping for Manuel...


Repository:
  rC Clang

https://reviews.llvm.org/D48242



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

[PATCH] D48269: [ASTMatchers] Don't assert-fail in specifiesTypeLoc().

[David L. Jones via Phabricator via cfe-commits]

dlj created this revision.
dlj added a reviewer: klimek.
dlj added a project: clang.
Herald added a subscriber: cfe-commits.

The specifiesTypeLoc() matcher narrows a nestedNameSpecifier matcher based on a
typeloc within the NNS. However, the matcher does not guard against NNS which
are a namespace, and cause getTypeLoc to assert-fail.


Repository:
  rC Clang

https://reviews.llvm.org/D48269

Files:
  include/clang/ASTMatchers/ASTMatchers.h
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp


Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1450,6 +1450,10 @@
 "struct A { struct B { struct C {}; }; }; A::B::C c;",
 nestedNameSpecifierLoc(hasPrefix(
   specifiesTypeLoc(loc(qualType(asString("struct A";
+  EXPECT_TRUE(matches(
+"namespace N { struct A { struct B { struct C {}; }; }; } N::A::B::C c;",
+nestedNameSpecifierLoc(hasPrefix(
+  specifiesTypeLoc(loc(qualType(asString("struct N::A";
 }
 
 
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -5536,7 +5536,8 @@
 ///   matches "A::"
 AST_MATCHER_P(NestedNameSpecifierLoc, specifiesTypeLoc,
   internal::Matcher, InnerMatcher) {
-  return Node && InnerMatcher.matches(Node.getTypeLoc(), Finder, Builder);
+  return Node && Node.getNestedNameSpecifier()->getAsType() &&
+ InnerMatcher.matches(Node.getTypeLoc(), Finder, Builder);
 }
 
 /// Matches on the prefix of a \c NestedNameSpecifier.


Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1450,6 +1450,10 @@
 "struct A { struct B { struct C {}; }; }; A::B::C c;",
 nestedNameSpecifierLoc(hasPrefix(
   specifiesTypeLoc(loc(qualType(asString("struct A";
+  EXPECT_TRUE(matches(
+"namespace N { struct A { struct B { struct C {}; }; }; } N::A::B::C c;",
+nestedNameSpecifierLoc(hasPrefix(
+  specifiesTypeLoc(loc(qualType(asString("struct N::A";
 }
 
 
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -5536,7 +5536,8 @@
 ///   matches "A::"
 AST_MATCHER_P(NestedNameSpecifierLoc, specifiesTypeLoc,
   internal::Matcher, InnerMatcher) {
-  return Node && InnerMatcher.matches(Node.getTypeLoc(), Finder, Builder);
+  return Node && Node.getNestedNameSpecifier()->getAsType() &&
+ InnerMatcher.matches(Node.getTypeLoc(), Finder, Builder);
 }
 
 /// Matches on the prefix of a \c NestedNameSpecifier.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48242: [ASTMatchers] Add support for matching the type of a friend decl.

[David L. Jones via Phabricator via cfe-commits]

dlj added inline comments.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:2900-2904
+AST_POLYMORPHIC_MATCHER_P_OVERLOAD(
+hasType,
+AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, FriendDecl, ValueDecl),
+internal::Matcher, InnerMatcher, 1) {
+  QualType QT = internal::getUnderlyingType(Node);

aaron.ballman wrote:
> I actually prefer the previous formatting, can you restore that?
Hmmm, it barely fits in 80 cols if I use the previous flow, but looks worse to 
me (it adds another line just for FriendDecl). I've updated to exactly what 
clang-format prefers, which happens to be fairly consistent with the other 
declaration of hasType.



Comment at: include/clang/ASTMatchers/ASTMatchers.h:2904
+internal::Matcher, InnerMatcher, 1) {
+  QualType QT = internal::getUnderlyingType(Node);
+  if (!QT.isNull())

klimek wrote:
> In which cases can getUnderlyingType return a null type?
QT can be null for a FriendDecl that that doesn't name a type. That seems to 
DTRT here.


Repository:
  rC Clang

https://reviews.llvm.org/D48242



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

[PATCH] D48242: [ASTMatchers] Add support for matching the type of a friend decl.

[David L. Jones via Phabricator via cfe-commits]

dlj updated this revision to Diff 151593.
dlj marked 3 inline comments as done.

Repository:
  rC Clang

https://reviews.llvm.org/D48242

Files:
  include/clang/ASTMatchers/ASTMatchers.h
  include/clang/ASTMatchers/ASTMatchersInternal.h
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp


Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -160,6 +160,16 @@
   valueDecl(hasType(asString("void (void)");
 }
 
+TEST(FriendDecl, Matches) {
+  EXPECT_TRUE(matches("class Y { friend class X; };",
+  friendDecl(hasType(asString("class X");
+  EXPECT_TRUE(matches("class Y { friend class X; };",
+  friendDecl(hasType(recordDecl(hasName("X"));
+
+  EXPECT_TRUE(matches("class Y { friend void f(); };",
+  functionDecl(hasName("f"), hasParent(friendDecl();
+}
+
 TEST(Enum, DoesNotMatchClasses) {
   EXPECT_TRUE(notMatches("class X {};", enumDecl(hasName("X";
 }
Index: include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- include/clang/ASTMatchers/ASTMatchersInternal.h
+++ include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -38,6 +38,7 @@
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
@@ -120,10 +121,14 @@
 inline QualType getUnderlyingType(const ValueDecl ) {
   return Node.getType();
 }
-
 inline QualType getUnderlyingType(const TypedefNameDecl ) {
   return Node.getUnderlyingType();
 }
+inline QualType getUnderlyingType(const FriendDecl ) {
+  if (const TypeSourceInfo *TSI = Node.getFriendType())
+return TSI->getType();
+  return QualType();
+}
 
 /// Unifies obtaining the FunctionProtoType pointer from both
 /// FunctionProtoType and FunctionDecl nodes..
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -2860,13 +2860,17 @@
 /// Example matches x (matcher = expr(hasType(cxxRecordDecl(hasName("X")
 /// and z (matcher = varDecl(hasType(cxxRecordDecl(hasName("X")
 /// and U (matcher = typedefDecl(hasType(asString("int")))
+/// and friend class X (matcher = friendDecl(hasType("X"))
 /// \code
 ///  class X {};
 ///  void y(X ) { x; X z; }
 ///  typedef int U;
+///  class Y { friend class X; };
 /// \endcode
 AST_POLYMORPHIC_MATCHER_P_OVERLOAD(
-hasType, AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, TypedefNameDecl, ValueDecl),
+hasType,
+AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, FriendDecl, TypedefNameDecl,
+ValueDecl),
 internal::Matcher, InnerMatcher, 0) {
   QualType QT = internal::getUnderlyingType(Node);
   if (!QT.isNull())
@@ -2885,18 +2889,21 @@
 ///
 /// Example matches x (matcher = expr(hasType(cxxRecordDecl(hasName("X")
 /// and z (matcher = varDecl(hasType(cxxRecordDecl(hasName("X")
+/// and friend class X (matcher = friendDecl(hasType("X"))
 /// \code
 ///  class X {};
 ///  void y(X ) { x; X z; }
+///  class Y { friend class X; };
 /// \endcode
 ///
 /// Usable as: Matcher, Matcher
-AST_POLYMORPHIC_MATCHER_P_OVERLOAD(hasType,
-   AST_POLYMORPHIC_SUPPORTED_TYPES(Expr,
-   ValueDecl),
-   internal::Matcher, InnerMatcher, 1) {
-  return qualType(hasDeclaration(InnerMatcher))
-  .matches(Node.getType(), Finder, Builder);
+AST_POLYMORPHIC_MATCHER_P_OVERLOAD(
+hasType, AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, FriendDecl, ValueDecl),
+internal::Matcher, InnerMatcher, 1) {
+  QualType QT = internal::getUnderlyingType(Node);
+  if (!QT.isNull())
+return qualType(hasDeclaration(InnerMatcher)).matches(QT, Finder, Builder);
+  return false;
 }
 
 /// Matches if the type location of the declarator decl's type matches


Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -160,6 +160,16 @@
   valueDecl(hasType(asString("void (void)");
 }
 
+TEST(FriendDecl, Matches) {
+  EXPECT_TRUE(matches("class Y { friend class X; };",
+  friendDecl(hasType(asString("class X");
+  EXPECT_TRUE(matches("class Y { friend class X; };",
+  friendDecl(hasType(recordDecl(hasName("X"));
+
+  EXPECT_TRUE(matches("class Y { friend void f(); };",
+  

[PATCH] D48242: [ASTMatchers] Add support for matching the type of a friend decl.

[David L. Jones via Phabricator via cfe-commits]

dlj created this revision.
dlj added a reviewer: klimek.
dlj added a project: clang.

This allows matchers like:

  friendDecl(hasType(cxxRecordDecl(...)))
  friendDecl(hasType(asString(...)))

It seems that hasType is probably the most reasonable narrowing matcher to
overload, since it is already used to narrow to other declaration kinds.


Repository:
  rC Clang

https://reviews.llvm.org/D48242

Files:
  include/clang/ASTMatchers/ASTMatchers.h
  include/clang/ASTMatchers/ASTMatchersInternal.h
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -160,6 +160,16 @@
   valueDecl(hasType(asString("void (void)");
 }
 
+TEST(FriendDecl, Matches) {
+  EXPECT_TRUE(matches("class Y { friend class X; };",
+  friendDecl(hasType(asString("class X");
+  EXPECT_TRUE(matches("class Y { friend class X; };",
+  friendDecl(hasType(recordDecl(hasName("X"));
+
+  EXPECT_TRUE(matches("class Y { friend void f(); };",
+  functionDecl(hasName("f"), hasParent(friendDecl();
+}
+
 TEST(Enum, DoesNotMatchClasses) {
   EXPECT_TRUE(notMatches("class X {};", enumDecl(hasName("X";
 }
Index: include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- include/clang/ASTMatchers/ASTMatchersInternal.h
+++ include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -38,6 +38,7 @@
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
@@ -120,10 +121,14 @@
 inline QualType getUnderlyingType(const ValueDecl ) {
   return Node.getType();
 }
-
 inline QualType getUnderlyingType(const TypedefNameDecl ) {
   return Node.getUnderlyingType();
 }
+inline QualType getUnderlyingType(const FriendDecl ) {
+  if (const TypeSourceInfo *tsi = Node.getFriendType())
+return tsi->getType();
+  return QualType();
+}
 
 /// Unifies obtaining the FunctionProtoType pointer from both
 /// FunctionProtoType and FunctionDecl nodes..
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -2860,13 +2860,17 @@
 /// Example matches x (matcher = expr(hasType(cxxRecordDecl(hasName("X")
 /// and z (matcher = varDecl(hasType(cxxRecordDecl(hasName("X")
 /// and U (matcher = typedefDecl(hasType(asString("int")))
+/// and friend class X (matcher = friendDecl(hasType("X"))
 /// \code
 ///  class X {};
 ///  void y(X ) { x; X z; }
 ///  typedef int U;
+///  class Y { friend class X; };
 /// \endcode
 AST_POLYMORPHIC_MATCHER_P_OVERLOAD(
-hasType, AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, TypedefNameDecl, ValueDecl),
+hasType,
+AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, FriendDecl, TypedefNameDecl,
+ValueDecl),
 internal::Matcher, InnerMatcher, 0) {
   QualType QT = internal::getUnderlyingType(Node);
   if (!QT.isNull())
@@ -2885,18 +2889,22 @@
 ///
 /// Example matches x (matcher = expr(hasType(cxxRecordDecl(hasName("X")
 /// and z (matcher = varDecl(hasType(cxxRecordDecl(hasName("X")
+/// and friend class X (matcher = friendDecl(hasType("X"))
 /// \code
 ///  class X {};
 ///  void y(X ) { x; X z; }
+///  class Y { friend class X; };
 /// \endcode
 ///
 /// Usable as: Matcher, Matcher
-AST_POLYMORPHIC_MATCHER_P_OVERLOAD(hasType,
-   AST_POLYMORPHIC_SUPPORTED_TYPES(Expr,
-   ValueDecl),
-   internal::Matcher, InnerMatcher, 1) {
-  return qualType(hasDeclaration(InnerMatcher))
-  .matches(Node.getType(), Finder, Builder);
+AST_POLYMORPHIC_MATCHER_P_OVERLOAD(
+hasType,
+AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, FriendDecl, ValueDecl),
+internal::Matcher, InnerMatcher, 1) {
+  QualType QT = internal::getUnderlyingType(Node);
+  if (!QT.isNull())
+return qualType(hasDeclaration(InnerMatcher)).matches(QT, Finder, Builder);
+  return false;
 }
 
 /// Matches if the type location of the declarator decl's type matches
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47759: [Format] Do not use a global static value for EOF within ScopedMacroState.

[David L. Jones via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL334801: [Format] Do not use a global static value for EOF 
within ScopedMacroState. (authored by dlj, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47759?vs=149900=151458#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47759

Files:
  cfe/trunk/lib/Format/UnwrappedLineParser.cpp


Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -83,6 +83,8 @@
   : Line(Line), TokenSource(TokenSource), ResetToken(ResetToken),
 PreviousLineLevel(Line.Level), PreviousTokenSource(TokenSource),
 Token(nullptr), PreviousToken(nullptr) {
+FakeEOF.Tok.startToken();
+FakeEOF.Tok.setKind(tok::eof);
 TokenSource = this;
 Line.Level = 0;
 Line.InPPDirective = true;
@@ -102,7 +104,7 @@
 PreviousToken = Token;
 Token = PreviousTokenSource->getNextToken();
 if (eof())
-  return getFakeEOF();
+  return 
 return Token;
   }
 
@@ -121,17 +123,7 @@
  /*MinColumnToken=*/PreviousToken);
   }
 
-  FormatToken *getFakeEOF() {
-static bool EOFInitialized = false;
-static FormatToken FormatTok;
-if (!EOFInitialized) {
-  FormatTok.Tok.startToken();
-  FormatTok.Tok.setKind(tok::eof);
-  EOFInitialized = true;
-}
-return 
-  }
-
+  FormatToken FakeEOF;
   UnwrappedLine 
   FormatTokenSource *
   FormatToken *


Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -83,6 +83,8 @@
   : Line(Line), TokenSource(TokenSource), ResetToken(ResetToken),
 PreviousLineLevel(Line.Level), PreviousTokenSource(TokenSource),
 Token(nullptr), PreviousToken(nullptr) {
+FakeEOF.Tok.startToken();
+FakeEOF.Tok.setKind(tok::eof);
 TokenSource = this;
 Line.Level = 0;
 Line.InPPDirective = true;
@@ -102,7 +104,7 @@
 PreviousToken = Token;
 Token = PreviousTokenSource->getNextToken();
 if (eof())
-  return getFakeEOF();
+  return 
 return Token;
   }
 
@@ -121,17 +123,7 @@
  /*MinColumnToken=*/PreviousToken);
   }
 
-  FormatToken *getFakeEOF() {
-static bool EOFInitialized = false;
-static FormatToken FormatTok;
-if (!EOFInitialized) {
-  FormatTok.Tok.startToken();
-  FormatTok.Tok.setKind(tok::eof);
-  EOFInitialized = true;
-}
-return 
-  }
-
+  FormatToken FakeEOF;
   UnwrappedLine 
   FormatTokenSource *
   FormatToken *
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47759: [Format] Do not use a global static value for EOF within ScopedMacroState.

[David L. Jones via Phabricator via cfe-commits]

dlj added a comment.

Ping...


Repository:
  rC Clang

https://reviews.llvm.org/D47759



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

[PATCH] D47759: [Format] Do not use a global static value for EOF within ScopedMacroState.

[David L. Jones via Phabricator via cfe-commits]

dlj created this revision.
dlj added a reviewer: djasper.
dlj added a project: clang.
Herald added a subscriber: klimek.

ScopedMacroState injects its own EOF token under certain conditions, and the
returned token may be modified in several different locations. If multiple
reformat operations are started in different threads, then they will both see
the same fake EOF token, and may both try to modify it. This is a data race.

This bug was caught with tsan.


Repository:
  rC Clang

https://reviews.llvm.org/D47759

Files:
  lib/Format/UnwrappedLineParser.cpp


Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -83,6 +83,8 @@
   : Line(Line), TokenSource(TokenSource), ResetToken(ResetToken),
 PreviousLineLevel(Line.Level), PreviousTokenSource(TokenSource),
 Token(nullptr), PreviousToken(nullptr) {
+FakeEOF.Tok.startToken();
+FakeEOF.Tok.setKind(tok::eof);
 TokenSource = this;
 Line.Level = 0;
 Line.InPPDirective = true;
@@ -102,7 +104,7 @@
 PreviousToken = Token;
 Token = PreviousTokenSource->getNextToken();
 if (eof())
-  return getFakeEOF();
+  return 
 return Token;
   }
 
@@ -121,17 +123,7 @@
  /*MinColumnToken=*/PreviousToken);
   }
 
-  FormatToken *getFakeEOF() {
-static bool EOFInitialized = false;
-static FormatToken FormatTok;
-if (!EOFInitialized) {
-  FormatTok.Tok.startToken();
-  FormatTok.Tok.setKind(tok::eof);
-  EOFInitialized = true;
-}
-return 
-  }
-
+  FormatToken FakeEOF;
   UnwrappedLine 
   FormatTokenSource *
   FormatToken *


Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -83,6 +83,8 @@
   : Line(Line), TokenSource(TokenSource), ResetToken(ResetToken),
 PreviousLineLevel(Line.Level), PreviousTokenSource(TokenSource),
 Token(nullptr), PreviousToken(nullptr) {
+FakeEOF.Tok.startToken();
+FakeEOF.Tok.setKind(tok::eof);
 TokenSource = this;
 Line.Level = 0;
 Line.InPPDirective = true;
@@ -102,7 +104,7 @@
 PreviousToken = Token;
 Token = PreviousTokenSource->getNextToken();
 if (eof())
-  return getFakeEOF();
+  return 
 return Token;
   }
 
@@ -121,17 +123,7 @@
  /*MinColumnToken=*/PreviousToken);
   }
 
-  FormatToken *getFakeEOF() {
-static bool EOFInitialized = false;
-static FormatToken FormatTok;
-if (!EOFInitialized) {
-  FormatTok.Tok.startToken();
-  FormatTok.Tok.setKind(tok::eof);
-  EOFInitialized = true;
-}
-return 
-  }
-
+  FormatToken FakeEOF;
   UnwrappedLine 
   FormatTokenSource *
   FormatToken *
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44123: [AST] Enhance comment accessing interface. (NFC)

[David L. Jones via Phabricator via cfe-commits]

dlj created this revision.
dlj added a reviewer: rsmith.
dlj added a project: clang.
Herald added a subscriber: sanjoy.

Previously, these methods were available for looking up comments:

1. getCommentForDecl: returns a FullComment attached to a decl, or any of its 
redecls, or any of its bases.
2. getRawCommentForAnyRedecl: returns a RawComment attached to a decl, or any 
of its redecls.
3. getLocalCommentForDeclUncached: returns a FullComment attached to a decl, 
but also doesn't update the internal cache.

This change reorganizes into these methods:

1. getRawCommentForDecl: returns a RawComment attached to a decl (and nowhere 
else).
2. getRawCommentForAnyRedecl: as before.
3. getParsedCommentForAnyRedecl: previously called getCommentForDecl.
4. getParsedCommentForDeclUncached: previously getLocalCommentFor


Repository:
  rC Clang

https://reviews.llvm.org/D44123

Files:
  include/clang/AST/ASTContext.h
  lib/AST/ASTContext.cpp
  lib/AST/ASTDumper.cpp
  lib/Sema/SemaDecl.cpp
  tools/libclang/CXComment.cpp

Index: tools/libclang/CXComment.cpp
===
--- tools/libclang/CXComment.cpp
+++ tools/libclang/CXComment.cpp
@@ -34,7 +34,8 @@
 
   const Decl *D = getCursorDecl(C);
   const ASTContext  = getCursorContext(C);
-  const FullComment *FC = Context.getCommentForDecl(D, /*PP=*/nullptr);
+  const FullComment *FC =
+  Context.getParsedCommentForAnyRedecl(D, /*PP=*/nullptr);
 
   return createCXComment(FC, getCursorTU(C));
 }
@@ -403,4 +404,3 @@
   ->convertCommentToXML(FC, XML, cxtu::getASTUnit(TU)->getASTContext());
   return cxstring::createDup(XML.str());
 }
-
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11915,7 +11915,7 @@
 // the lookahead in the lexer: we've consumed the semicolon and looked
 // ahead through comments.
 for (unsigned i = 0, e = Group.size(); i != e; ++i)
-  Context.getCommentForDecl(Group[i], );
+  Context.getParsedCommentForAnyRedecl(Group[i], );
   }
 }
 
Index: lib/AST/ASTDumper.cpp
===
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -1078,7 +1078,7 @@
   dumpAttr(*I);
 
 if (const FullComment *Comment =
-D->getASTContext().getLocalCommentForDeclUncached(D))
+D->getASTContext().getParsedCommentForDeclUncached(D))
   dumpFullComment(Comment);
 
 // Decls within functions are visited by the body.
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -407,6 +407,20 @@
   return Raw;
 }
 
+const RawComment *
+ASTContext::getRawCommentForDecl(const Decl *D) const {
+  const RawCommentAndCacheFlags  = getRawCommentForDeclCached(D);
+  const RawComment *RC = Raw.getRaw();
+  if (RC) {
+// Make sure this cache entry wasn't updated by getRawCommentForAnyRedecl.
+if (Raw.getOriginalDecl() != D)
+  return nullptr;
+return RC;
+  }
+  assert(Raw.getKind() == RawCommentAndCacheFlags::NoCommentInDecl);
+  return nullptr;
+}
+
 const RawComment *
 ASTContext::getRawCommentForAnyRedecl(const Decl *D,
   const Decl **OriginalDecl) const {
@@ -483,14 +497,15 @@
   return CFC;
 }
 
-comments::FullComment *ASTContext::getLocalCommentForDeclUncached(const Decl *D) const {
+comments::FullComment *
+ASTContext::getParsedCommentForDeclUncached(const Decl *D) const {
   const RawComment *RC = getRawCommentForDeclNoCache(D);
   return RC ? RC->parse(*this, nullptr, D) : nullptr;
 }
 
 comments::FullComment *
-ASTContext::getCommentForDecl(const Decl *D,
-const Preprocessor *PP) const {
+ASTContext::getParsedCommentForAnyRedecl(const Decl *D,
+ const Preprocessor *PP) const {
   if (D->isInvalidDecl())
 return nullptr;
   D = adjustDeclToTemplate(D);
@@ -517,34 +532,36 @@
   const ObjCMethodDecl *OMD = dyn_cast(D);
   if (OMD && OMD->isPropertyAccessor())
 if (const ObjCPropertyDecl *PDecl = OMD->findPropertyDecl())
-  if (comments::FullComment *FC = getCommentForDecl(PDecl, PP))
+  if (comments::FullComment *FC =
+  getParsedCommentForAnyRedecl(PDecl, PP))
 return cloneFullComment(FC, D);
   if (OMD)
 addRedeclaredMethods(OMD, Overridden);
   getOverriddenMethods(dyn_cast(D), Overridden);
   for (unsigned i = 0, e = Overridden.size(); i < e; i++)
-if (comments::FullComment *FC = getCommentForDecl(Overridden[i], PP))
+if (comments::FullComment *FC =
+getParsedCommentForAnyRedecl(Overridden[i], PP))
   return cloneFullComment(FC, D);
 }
 else if (const TypedefNameDecl *TD = dyn_cast(D)) {
   // Attach any tag type's documentation to its 

[PATCH] D44122: [AST] Factor out RawComment lookup and caching. (NFC)

[David L. Jones via Phabricator via cfe-commits]

dlj created this revision.
dlj added a reviewer: rsmith.
dlj added a project: clang.
Herald added a subscriber: sanjoy.

This changes how RawComments are looked up, so that they can be added to the
cache or updated independently of parsing.

Loosely, the comment cache has two conceptually separate separate layers:

  RedeclComments - maps Decls to RawComments, which may have come from the Decl
  that is the key (a 'local' comment), or one of its redeclarations. The cache
  entry can keep track of whether the comment is local or a redecl.
  
  ParsedComments - maps Decls to FullComments, which may have come from the Decl
  that is the key, one of its redecls, or one of its bases (a base class, and
  overridden class method, ObjC interface, typedef, etc.).

This patch factors out the logic that maintains the lower RedeclComments portion
of the cache.


Repository:
  rC Clang

https://reviews.llvm.org/D44122

Files:
  include/clang/AST/ASTContext.h
  lib/AST/ASTContext.cpp

Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -379,53 +379,54 @@
   return D;
 }
 
-const RawComment *ASTContext::getRawCommentForAnyRedecl(
-const Decl *D,
-const Decl **OriginalDecl) const {
+ASTContext::RawCommentAndCacheFlags &
+ASTContext::getRawCommentForDeclCached(const Decl *D) const {
   D = adjustDeclToTemplate(D);
 
   // Check whether we have cached a comment for this declaration already.
-  {
-llvm::DenseMap::iterator Pos =
-RedeclComments.find(D);
-if (Pos != RedeclComments.end()) {
-  const RawCommentAndCacheFlags  = Pos->second;
-  if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) {
-if (OriginalDecl)
-  *OriginalDecl = Raw.getOriginalDecl();
-return Raw.getRaw();
-  }
-}
+  auto ItInserted = RedeclComments.insert({D, RawCommentAndCacheFlags()});
+  RawCommentAndCacheFlags& Raw = ItInserted.first->second;
+  if (!ItInserted.second)
+return Raw;
+
+  // 'Raw' is a new entry in the cache. Populate it with any comments for the
+  // local definition.
+  Raw.setOriginalDecl(D);
+  const RawComment *RC = getRawCommentForDeclNoCache(D);
+  if (!RC) {
+Raw.setKind(RawCommentAndCacheFlags::NoCommentInDecl);
+return Raw;
+  }
+
+  // If we found a comment, it should be a documentation comment.
+  assert(RC->isDocumentation() || LangOpts.CommentOpts.ParseAllComments);
+  // Call order swapped to work around ICE in VS2015 RTM (Release Win32)
+  // https://connect.microsoft.com/VisualStudio/feedback/details/1741530
+  Raw.setKind(RawCommentAndCacheFlags::FromDecl);
+  Raw.setRaw(RC);
+  return Raw;
+}
+
+const RawComment *
+ASTContext::getRawCommentForAnyRedecl(const Decl *D,
+  const Decl **OriginalDecl) const {
+  RawCommentAndCacheFlags  = getRawCommentForDeclCached(D);
+  if (OrigRaw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) {
+if (OriginalDecl)
+  *OriginalDecl = OrigRaw.getOriginalDecl();
+return OrigRaw.getRaw();
   }
 
   // Search for comments attached to declarations in the redeclaration chain.
   const RawComment *RC = nullptr;
+
   const Decl *OriginalDeclForRC = nullptr;
   for (auto I : D->redecls()) {
-llvm::DenseMap::iterator Pos =
-RedeclComments.find(I);
-if (Pos != RedeclComments.end()) {
-  const RawCommentAndCacheFlags  = Pos->second;
-  if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) {
-RC = Raw.getRaw();
-OriginalDeclForRC = Raw.getOriginalDecl();
-break;
-  }
-} else {
-  RC = getRawCommentForDeclNoCache(I);
-  OriginalDeclForRC = I;
-  RawCommentAndCacheFlags Raw;
-  if (RC) {
-// Call order swapped to work around ICE in VS2015 RTM (Release Win32)
-// https://connect.microsoft.com/VisualStudio/feedback/details/1741530
-Raw.setKind(RawCommentAndCacheFlags::FromDecl);
-Raw.setRaw(RC);
-  } else
-Raw.setKind(RawCommentAndCacheFlags::NoCommentInDecl);
-  Raw.setOriginalDecl(I);
-  RedeclComments[I] = Raw;
-  if (RC)
-break;
+RawCommentAndCacheFlags  = getRawCommentForDeclCached(I);
+if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) {
+  RC = Raw.getRaw();
+  OriginalDeclForRC = Raw.getOriginalDecl();
+  break;
 }
   }
 
@@ -487,9 +488,9 @@
   return RC ? RC->parse(*this, nullptr, D) : nullptr;
 }
 
-comments::FullComment *ASTContext::getCommentForDecl(
-  const Decl *D,
-  const Preprocessor *PP) const {
+comments::FullComment *
+ASTContext::getCommentForDecl(const Decl *D,
+const Preprocessor *PP) const {
   if (D->isInvalidDecl())
 return nullptr;
   D = 

[PATCH] D43663: [NFC] Move CommentOpts checks to the call sites that depend on it.

[David L. Jones via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL326501: [NFC] Move CommentOpts checks to the call sites that 
depend on it. (authored by dlj, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D43663?vs=135573=136610#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D43663

Files:
  cfe/trunk/include/clang/AST/ASTContext.h
  cfe/trunk/include/clang/AST/RawCommentList.h
  cfe/trunk/lib/AST/ASTContext.cpp
  cfe/trunk/lib/AST/RawCommentList.cpp
  cfe/trunk/lib/Sema/Sema.cpp
  cfe/trunk/lib/Serialization/ASTReader.cpp

Index: cfe/trunk/include/clang/AST/ASTContext.h
===
--- cfe/trunk/include/clang/AST/ASTContext.h
+++ cfe/trunk/include/clang/AST/ASTContext.h
@@ -784,7 +784,7 @@
   void addComment(const RawComment ) {
 assert(LangOpts.RetainCommentsFromSystemHeaders ||
!SourceMgr.isInSystemHeader(RC.getSourceRange().getBegin()));
-Comments.addComment(RC, BumpAlloc);
+Comments.addComment(RC, LangOpts.CommentOpts, BumpAlloc);
   }
 
   /// \brief Return the documentation comment attached to a given declaration.
Index: cfe/trunk/include/clang/AST/RawCommentList.h
===
--- cfe/trunk/include/clang/AST/RawCommentList.h
+++ cfe/trunk/include/clang/AST/RawCommentList.h
@@ -41,7 +41,7 @@
   RawComment() : Kind(RCK_Invalid), IsAlmostTrailingComment(false) { }
 
   RawComment(const SourceManager , SourceRange SR,
- bool Merged, bool ParseAllComments);
+ const CommentOptions , bool Merged);
 
   CommentKind getKind() const LLVM_READONLY {
 return (CommentKind) Kind;
@@ -83,20 +83,14 @@
 
   /// Returns true if this comment is not a documentation comment.
   bool isOrdinary() const LLVM_READONLY {
-return ((Kind == RCK_OrdinaryBCPL) || (Kind == RCK_OrdinaryC)) &&
-!ParseAllComments;
+return ((Kind == RCK_OrdinaryBCPL) || (Kind == RCK_OrdinaryC));
   }
 
   /// Returns true if this comment any kind of a documentation comment.
   bool isDocumentation() const LLVM_READONLY {
 return !isInvalid() && !isOrdinary();
   }
 
-  /// Returns whether we are parsing all comments.
-  bool isParseAllComments() const LLVM_READONLY {
-return ParseAllComments;
-  }
-
   /// Returns raw comment text with comment markers.
   StringRef getRawText(const SourceManager ) const {
 if (RawTextValid)
@@ -139,18 +133,12 @@
   bool IsTrailingComment : 1;
   bool IsAlmostTrailingComment : 1;
 
-  /// When true, ordinary comments starting with "//" and "/*" will be
-  /// considered as documentation comments.
-  bool ParseAllComments : 1;
-
   /// \brief Constructor for AST deserialization.
   RawComment(SourceRange SR, CommentKind K, bool IsTrailingComment,
- bool IsAlmostTrailingComment,
- bool ParseAllComments) :
+ bool IsAlmostTrailingComment) :
 Range(SR), RawTextValid(false), BriefTextValid(false), Kind(K),
 IsAttached(false), IsTrailingComment(IsTrailingComment),
-IsAlmostTrailingComment(IsAlmostTrailingComment),
-ParseAllComments(ParseAllComments)
+IsAlmostTrailingComment(IsAlmostTrailingComment)
   { }
 
   StringRef getRawTextSlow(const SourceManager ) const;
@@ -183,7 +171,8 @@
 public:
   RawCommentList(SourceManager ) : SourceMgr(SourceMgr) {}
 
-  void addComment(const RawComment , llvm::BumpPtrAllocator );
+  void addComment(const RawComment , const CommentOptions ,
+  llvm::BumpPtrAllocator );
 
   ArrayRef getComments() const {
 return Comments;
Index: cfe/trunk/lib/AST/RawCommentList.cpp
===
--- cfe/trunk/lib/AST/RawCommentList.cpp
+++ cfe/trunk/lib/AST/RawCommentList.cpp
@@ -107,21 +107,22 @@
 }
 
 RawComment::RawComment(const SourceManager , SourceRange SR,
-   bool Merged, bool ParseAllComments) :
+   const CommentOptions , bool Merged) :
 Range(SR), RawTextValid(false), BriefTextValid(false),
-IsAttached(false), IsTrailingComment(false), IsAlmostTrailingComment(false),
-ParseAllComments(ParseAllComments) {
+IsAttached(false), IsTrailingComment(false),
+IsAlmostTrailingComment(false) {
   // Extract raw comment text, if possible.
   if (SR.getBegin() == SR.getEnd() || getRawText(SourceMgr).empty()) {
 Kind = RCK_Invalid;
 return;
   }
 
   // Guess comment kind.
-  std::pair K = getCommentKind(RawText, ParseAllComments);
+  std::pair K =
+  getCommentKind(RawText, CommentOpts.ParseAllComments);
 
   // Guess whether an ordinary comment is trailing.
-  if (ParseAllComments && isOrdinaryKind(K.first)) {
+  if (CommentOpts.ParseAllComments && isOrdinaryKind(K.first)) {
 FileID BeginFileID;
 unsigned BeginOffset;
 std::tie(BeginFileID, BeginOffset) =
@@ -270,6 +271,7 @@
 

[PATCH] D43800: [ASTMatchers] Allow file-based narrowing matches to work with NestedNameSpecifierLocs.

[David L. Jones via Phabricator via cfe-commits]

dlj created this revision.
dlj added a reviewer: rsmith.
Herald added subscribers: sanjoy, klimek.

The narrowing matchers isExpansionInMainFile, isExpansionInSystemHeader, and
isExpansionInFileMatching work for TypeLocs, but not NestedNameSpecifierLocs.
This changes the matchers to use getSourceRange() to get location information,
which not only works for the previous cases, but also for NNSLocs.


Repository:
  rC Clang

https://reviews.llvm.org/D43800

Files:
  include/clang/ASTMatchers/ASTMatchers.h


Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -240,12 +240,14 @@
 ///   class Y {};
 /// \endcode
 ///
-/// Usable as: Matcher, Matcher, Matcher
+/// Usable as: Matcher, Matcher, Matcher,
+/// Matcher
 AST_POLYMORPHIC_MATCHER(isExpansionInMainFile,
-AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc)) {
+AST_POLYMORPHIC_SUPPORTED_TYPES(
+Decl, Stmt, TypeLoc, NestedNameSpecifierLoc)) {
   auto  = Finder->getASTContext().getSourceManager();
   return SourceManager.isInMainFile(
-  SourceManager.getExpansionLoc(Node.getLocStart()));
+  SourceManager.getExpansionLoc(Node.getSourceRange().getBegin()));
 }
 
 /// \brief Matches AST nodes that were expanded within system-header-files.
@@ -261,11 +263,14 @@
 ///   class Y {};
 /// \endcode
 ///
-/// Usable as: Matcher, Matcher, Matcher
+/// Usable as: Matcher, Matcher, Matcher,
+/// Matcher
 AST_POLYMORPHIC_MATCHER(isExpansionInSystemHeader,
-AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc)) {
+AST_POLYMORPHIC_SUPPORTED_TYPES(
+Decl, Stmt, TypeLoc, NestedNameSpecifierLoc)) {
   auto  = Finder->getASTContext().getSourceManager();
-  auto ExpansionLoc = SourceManager.getExpansionLoc(Node.getLocStart());
+  auto ExpansionLoc =
+  SourceManager.getExpansionLoc(Node.getSourceRange().getBegin());
   if (ExpansionLoc.isInvalid()) {
 return false;
   }
@@ -286,12 +291,15 @@
 ///   class Y {};
 /// \endcode
 ///
-/// Usable as: Matcher, Matcher, Matcher
+/// Usable as: Matcher, Matcher, Matcher,
+/// Matcher
 AST_POLYMORPHIC_MATCHER_P(isExpansionInFileMatching,
-  AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc),
+  AST_POLYMORPHIC_SUPPORTED_TYPES(
+  Decl, Stmt, TypeLoc, NestedNameSpecifierLoc),
   std::string, RegExp) {
   auto  = Finder->getASTContext().getSourceManager();
-  auto ExpansionLoc = SourceManager.getExpansionLoc(Node.getLocStart());
+  auto ExpansionLoc =
+  SourceManager.getExpansionLoc(Node.getSourceRange().getBegin());
   if (ExpansionLoc.isInvalid()) {
 return false;
   }


Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -240,12 +240,14 @@
 ///   class Y {};
 /// \endcode
 ///
-/// Usable as: Matcher, Matcher, Matcher
+/// Usable as: Matcher, Matcher, Matcher,
+/// Matcher
 AST_POLYMORPHIC_MATCHER(isExpansionInMainFile,
-AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc)) {
+AST_POLYMORPHIC_SUPPORTED_TYPES(
+Decl, Stmt, TypeLoc, NestedNameSpecifierLoc)) {
   auto  = Finder->getASTContext().getSourceManager();
   return SourceManager.isInMainFile(
-  SourceManager.getExpansionLoc(Node.getLocStart()));
+  SourceManager.getExpansionLoc(Node.getSourceRange().getBegin()));
 }
 
 /// \brief Matches AST nodes that were expanded within system-header-files.
@@ -261,11 +263,14 @@
 ///   class Y {};
 /// \endcode
 ///
-/// Usable as: Matcher, Matcher, Matcher
+/// Usable as: Matcher, Matcher, Matcher,
+/// Matcher
 AST_POLYMORPHIC_MATCHER(isExpansionInSystemHeader,
-AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc)) {
+AST_POLYMORPHIC_SUPPORTED_TYPES(
+Decl, Stmt, TypeLoc, NestedNameSpecifierLoc)) {
   auto  = Finder->getASTContext().getSourceManager();
-  auto ExpansionLoc = SourceManager.getExpansionLoc(Node.getLocStart());
+  auto ExpansionLoc =
+  SourceManager.getExpansionLoc(Node.getSourceRange().getBegin());
   if (ExpansionLoc.isInvalid()) {
 return false;
   }
@@ -286,12 +291,15 @@
 ///   class Y {};
 /// \endcode
 ///
-/// Usable as: Matcher, Matcher, Matcher
+/// Usable as: Matcher, Matcher, Matcher,
+/// Matcher
 AST_POLYMORPHIC_MATCHER_P(isExpansionInFileMatching,
-  AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc),
+  AST_POLYMORPHIC_SUPPORTED_TYPES(
+  Decl, Stmt, TypeLoc, NestedNameSpecifierLoc),
   

[PATCH] D43663: [NFC] Move CommentOpts checks to the call sites that depend on it.

[David L. Jones via Phabricator via cfe-commits]

dlj created this revision.
dlj added a reviewer: rsmith.
Herald added a subscriber: sanjoy.

When parsing comments, for example, for -Wdocumentation, slightly different
behaviour occurs when -fparse-all-comments is specified. However, these
differences are subtle:

1. All comments are saved during parsing, regardless of whether they are doc 
comments or not.
2. "Maybe-doc" comments, like //<, //!, etc, are saved as such, instead of 
marking them as ordinary comments. The maybe-doc type of comment is never saved 
otherwise. (Warning on these is the impetus of -Wdocumentation.)
3. All comments are treated as doc comments in ASTContext, even if they are 
ordinary.

This change moves the logic for checking CommentOptions.ParseAllComments closer
to where it has an effect. The overall logic is unchanged, but checks of the
ParseAllComments flag are now done where the effect will be clearer.


Repository:
  rC Clang

https://reviews.llvm.org/D43663

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/RawCommentList.h
  lib/AST/ASTContext.cpp
  lib/AST/RawCommentList.cpp
  lib/Sema/Sema.cpp
  lib/Serialization/ASTReader.cpp

Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -9067,8 +9067,7 @@
 bool IsTrailingComment = Record[Idx++];
 bool IsAlmostTrailingComment = Record[Idx++];
 Comments.push_back(new (Context) RawComment(
-SR, Kind, IsTrailingComment, IsAlmostTrailingComment,
-Context.getLangOpts().CommentOpts.ParseAllComments));
+SR, Kind, IsTrailingComment, IsAlmostTrailingComment));
 break;
   }
   }
Index: lib/Sema/Sema.cpp
===
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -1463,8 +1463,7 @@
   if (!LangOpts.RetainCommentsFromSystemHeaders &&
   SourceMgr.isInSystemHeader(Comment.getBegin()))
 return;
-  RawComment RC(SourceMgr, Comment, false,
-LangOpts.CommentOpts.ParseAllComments);
+  RawComment RC(SourceMgr, Comment, LangOpts.CommentOpts, false);
   if (RC.isAlmostTrailingComment()) {
 SourceRange MagicMarkerRange(Comment.getBegin(),
  Comment.getBegin().getLocWithOffset(3));
Index: lib/AST/RawCommentList.cpp
===
--- lib/AST/RawCommentList.cpp
+++ lib/AST/RawCommentList.cpp
@@ -107,21 +107,22 @@
 }
 
 RawComment::RawComment(const SourceManager , SourceRange SR,
-   bool Merged, bool ParseAllComments) :
+   const CommentOptions , bool Merged) :
 Range(SR), RawTextValid(false), BriefTextValid(false),
-IsAttached(false), IsTrailingComment(false), IsAlmostTrailingComment(false),
-ParseAllComments(ParseAllComments) {
+IsAttached(false), IsTrailingComment(false),
+IsAlmostTrailingComment(false) {
   // Extract raw comment text, if possible.
   if (SR.getBegin() == SR.getEnd() || getRawText(SourceMgr).empty()) {
 Kind = RCK_Invalid;
 return;
   }
 
   // Guess comment kind.
-  std::pair K = getCommentKind(RawText, ParseAllComments);
+  std::pair K =
+  getCommentKind(RawText, CommentOpts.ParseAllComments);
 
   // Guess whether an ordinary comment is trailing.
-  if (ParseAllComments && isOrdinaryKind(K.first)) {
+  if (CommentOpts.ParseAllComments && isOrdinaryKind(K.first)) {
 FileID BeginFileID;
 unsigned BeginOffset;
 std::tie(BeginFileID, BeginOffset) =
@@ -270,6 +271,7 @@
 }
 
 void RawCommentList::addComment(const RawComment ,
+const CommentOptions ,
 llvm::BumpPtrAllocator ) {
   if (RC.isInvalid())
 return;
@@ -284,7 +286,7 @@
   }
 
   // Ordinary comments are not interesting for us.
-  if (RC.isOrdinary())
+  if (RC.isOrdinary() && !CommentOpts.ParseAllComments)
 return;
 
   // If this is the first Doxygen comment, save it (because there isn't
@@ -317,8 +319,7 @@
   onlyWhitespaceBetween(SourceMgr, C1.getLocEnd(), C2.getLocStart(),
 /*MaxNewlinesAllowed=*/1)) {
 SourceRange MergedRange(C1.getLocStart(), C2.getLocEnd());
-*Comments.back() = RawComment(SourceMgr, MergedRange, true,
-  RC.isParseAllComments());
+*Comments.back() = RawComment(SourceMgr, MergedRange, CommentOpts, true);
   } else {
 Comments.push_back(new (Allocator) RawComment(RC));
   }
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -226,8 +226,7 @@
 // for is usually among the last two comments we parsed -- check them
 // first.
 RawComment CommentAtDeclLoc(
-SourceMgr, SourceRange(DeclLoc), false,
-

[PATCH] D38757: [libc++] Fix PR34898 - vector iterator constructors and assign method perform push_back instead of emplace_back.

[David L. Jones via Phabricator via cfe-commits]

dlj requested changes to this revision.
dlj added a comment.
This revision now requires changes to proceed.

Hmm, looking more at this change... while it does make the behaviour consistent 
for Forward and Input iterators, I think it's just making them both do the 
wrong thing.

Specifically, based on this:

"... i and j denote iterators satisfying input iterator requirements and refer 
to elements implicitly convertible to value_­type..."

https://timsong-cpp.github.io/cppwp/n4659/container.requirements#sequence.reqmts-3

So, for example, in test_emplacable_concept, the vector constructor should be 
diagnosed, because there is no way to *implicitly* convert from the 
dereferenced iterator type to the inserted type. The selected constructor is 
explicit. Using emplacement just omits a *second* potentially-expensive 
conversion: the explicit constructor behaviour (invoked through forwarding) may 
still be undesired.


https://reviews.llvm.org/D38757



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

[PATCH] D38059: Rename list::base to list::__base.

[David L. Jones via Phabricator via cfe-commits]

dlj created this revision.
Herald added a subscriber: sanjoy.

Even though the base typedef is private, it still participates in lookup. For 
example:

  void base() {}
  struct X : private std::list {
X() { base(); }
  };


https://reviews.llvm.org/D38059

Files:
  include/list

Index: include/list
===
--- include/list
+++ include/list
@@ -805,30 +805,30 @@
 class _LIBCPP_TEMPLATE_VIS list
 : private __list_imp<_Tp, _Alloc>
 {
-typedef __list_imp<_Tp, _Alloc> base;
-typedef typename base::__node  __node;
-typedef typename base::__node_allocator__node_allocator;
-typedef typename base::__node_pointer  __node_pointer;
-typedef typename base::__node_alloc_traits __node_alloc_traits;
-typedef typename base::__node_base __node_base;
-typedef typename base::__node_base_pointer __node_base_pointer;
-typedef typename base::__link_pointer __link_pointer;
+typedef __list_imp<_Tp, _Alloc> __base;
+typedef typename __base::__node  __node;
+typedef typename __base::__node_allocator__node_allocator;
+typedef typename __base::__node_pointer  __node_pointer;
+typedef typename __base::__node_alloc_traits __node_alloc_traits;
+typedef typename __base::__node_base __node_base;
+typedef typename __base::__node_base_pointer __node_base_pointer;
+typedef typename __base::__link_pointer  __link_pointer;
 
 public:
 typedef _Tp  value_type;
 typedef _Alloc   allocator_type;
 static_assert((is_same::value),
   "Invalid allocator::value_type");
 typedef value_type&  reference;
 typedef const value_type&const_reference;
-typedef typename base::pointer   pointer;
-typedef typename base::const_pointer const_pointer;
-typedef typename base::size_type size_type;
-typedef typename base::difference_type   difference_type;
-typedef typename base::iterator  iterator;
-typedef typename base::const_iteratorconst_iterator;
-typedef _VSTD::reverse_iterator reverse_iterator;
-typedef _VSTD::reverse_iterator   const_reverse_iterator;
+typedef typename __base::pointer pointer;
+typedef typename __base::const_pointer   const_pointer;
+typedef typename __base::size_type   size_type;
+typedef typename __base::difference_type difference_type;
+typedef typename __base::iteratoriterator;
+typedef typename __base::const_iterator  const_iterator;
+typedef _VSTD::reverse_iteratorreverse_iterator;
+typedef _VSTD::reverse_iterator  const_reverse_iterator;
 
 _LIBCPP_INLINE_VISIBILITY
 list()
@@ -839,7 +839,7 @@
 #endif
 }
 _LIBCPP_INLINE_VISIBILITY
-explicit list(const allocator_type& __a) : base(__a)
+explicit list(const allocator_type& __a) : __base(__a)
 {
 #if _LIBCPP_DEBUG_LEVEL >= 2
 __get_db()->__insert_c(this);
@@ -895,29 +895,29 @@
 allocator_type get_allocator() const _NOEXCEPT;
 
 _LIBCPP_INLINE_VISIBILITY
-size_type size() const _NOEXCEPT {return base::__sz();}
+size_type size() const _NOEXCEPT {return __base::__sz();}
 _LIBCPP_INLINE_VISIBILITY
-bool empty() const _NOEXCEPT {return base::empty();}
+bool empty() const _NOEXCEPT {return __base::empty();}
 _LIBCPP_INLINE_VISIBILITY
 size_type max_size() const _NOEXCEPT
 {
 return std::min(
-base::__node_alloc_max_size(),
+__base::__node_alloc_max_size(),
 numeric_limits::max());
 }
 
 _LIBCPP_INLINE_VISIBILITY
-  iterator begin() _NOEXCEPT{return base::begin();}
+  iterator begin() _NOEXCEPT{return __base::begin();}
 _LIBCPP_INLINE_VISIBILITY
-const_iterator begin()  const _NOEXCEPT {return base::begin();}
+const_iterator begin()  const _NOEXCEPT {return __base::begin();}
 _LIBCPP_INLINE_VISIBILITY
-  iterator end() _NOEXCEPT  {return base::end();}
+  iterator end() _NOEXCEPT  {return __base::end();}
 _LIBCPP_INLINE_VISIBILITY
-const_iterator end()const _NOEXCEPT {return base::end();}
+const_iterator end()const _NOEXCEPT {return __base::end();}
 _LIBCPP_INLINE_VISIBILITY
-const_iterator cbegin() const _NOEXCEPT {return base::begin();}
+const_iterator cbegin() const _NOEXCEPT {return __base::begin();}
 _LIBCPP_INLINE_VISIBILITY
-const_iterator cend()   const _NOEXCEPT {return base::end();}
+const_iterator cend()   const _NOEXCEPT {return __base::end();}
 
 _LIBCPP_INLINE_VISIBILITY
 

[PATCH] D37538: [libc++] Remove problematic ADL in container implementations.

[David L. Jones via Phabricator via cfe-commits]

dlj added a comment.

In https://reviews.llvm.org/D37538#871515, @EricWF wrote:

> @dlj I went ahead and committed the fixes to `std::allocator_traits` in 
> r313324, because I think we agree those are bugs, and I didn't want this 
> discussion to hold up that fix. I hope you don't mind.


Nope, not a problem.


https://reviews.llvm.org/D37538



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

[PATCH] D37538: [libc++] Remove problematic ADL in container implementations.

[David L. Jones via Phabricator via cfe-commits]

dlj updated this revision to Diff 115165.
dlj added a comment.

- Remove deque from the test for now.


https://reviews.llvm.org/D37538

Files:
  include/__split_buffer
  include/memory
  test/std/containers/containers.general/construct_destruct.pass.cpp

Index: test/std/containers/containers.general/construct_destruct.pass.cpp
===
--- /dev/null
+++ test/std/containers/containers.general/construct_destruct.pass.cpp
@@ -0,0 +1,128 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+// Holder is a do-nothing wrapper around a value of the given type.
+//
+// Since unqualified name lookup and operator overload lookup participate in
+// ADL, the dependent type ns::T contributes to the overload set considered by
+// ADL. In other words, ADL will consider all associated namespaces: this
+// includes not only N, but also any inline friend functions declared by ns::T.
+//
+// The purpose of this test is to ensure that simply constructing or destroying
+// a container does not invoke ADL which would be problematic for unknown types.
+// By using the type 'Holder *' as a container value, we can avoid the
+// obvious problems with unknown types: a pointer is trivial, and its size is
+// known. However, any ADL which includes ns::T as an associated namesapce will
+// fail.
+//
+// For example:
+//
+//   namespace ns {
+// class T {
+//   // 13.5.7 [over.inc]:
+//   friend std::list::const_iterator
+//   operator++(std::list::const_iterator it) {
+// return /* ... */;
+//   }
+// };
+//   }
+//
+// The C++ standard stipulates that some functions, such as std::advance, use
+// overloaded operators (in C++14: 24.4.4 [iterator.operations]). The
+// implication is that calls to such a function are dependent on knowing the
+// overload set of operators in all associated namespaces; under ADL, this
+// includes the private friend function in the example above.
+//
+// However, for some operations, such as construction and destruction, no such
+// ADL is required. This can be important, for example, when using the Pimpl
+// pattern:
+//
+//   // Defined in a header file:
+//   class InterfaceList {
+// // Defined in a .cpp file:
+// class Impl;
+// vector impls;
+//   public:
+// ~InterfaceList();
+//   };
+//
+template 
+class Holder { T value; };
+
+namespace ns { class Fwd; }
+
+// TestSequencePtr and TestMappingPtr ensure that a given container type can be
+// default-constructed and destroyed with an incomplete value pointer type.
+template  class Container>
+void TestSequencePtr() {
+  using X = Container;
+  {
+X u;
+assert(u.empty());
+  }
+  {
+auto u = X();
+assert(u.empty());
+  }
+}
+
+template  class Container>
+void TestMappingPtr() {
+  {
+using X = Container;
+X u1;
+assert(u1.empty());
+auto u2 = X();
+assert(u2.empty());
+  }
+  {
+using X = Container;
+X u1;
+assert(u1.empty());
+auto u2 = X();
+assert(u2.empty());
+  }
+}
+
+int main()
+{
+  // Sequence containers:
+  {
+std::array a;
+(void)a;
+  }
+  TestSequencePtr();
+  TestSequencePtr();
+  TestSequencePtr();
+
+  // Associative containers, non-mapping:
+  TestSequencePtr();
+  TestSequencePtr();
+
+  // Associative containers, mapping:
+  TestMappingPtr();
+  TestMappingPtr();
+
+  // Container adapters:
+  TestSequencePtr();
+  TestSequencePtr();
+}
Index: include/memory
===
--- include/memory
+++ include/memory
@@ -1346,9 +1346,9 @@
 struct __has_construct
 : integral_constant(),
-  declval<_Pointer>(),
-  declval<_Args>()...)),
+decltype(_VSTD::__has_construct_test(declval<_Alloc>(),
+ declval<_Pointer>(),
+ declval<_Args>()...)),
 true_type>::value>
 {
 };
@@ -1367,8 +1367,8 @@
 struct __has_destroy
 : integral_constant(),
-declval<_Pointer>())),
+decltype(_VSTD::__has_destroy_test(declval<_Alloc>(),
+   declval<_Pointer>())),
 

[PATCH] D37538: [libc++] Remove problematic ADL in container implementations.

[David L. Jones via Phabricator via cfe-commits]

dlj updated this revision to Diff 115164.
dlj added a comment.

- Remove deque from the test for now.


https://reviews.llvm.org/D37538

Files:
  include/__split_buffer
  include/deque
  include/memory
  test/std/containers/containers.general/construct_destruct.pass.cpp

Index: test/std/containers/containers.general/construct_destruct.pass.cpp
===
--- /dev/null
+++ test/std/containers/containers.general/construct_destruct.pass.cpp
@@ -0,0 +1,128 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+// Holder is a do-nothing wrapper around a value of the given type.
+//
+// Since unqualified name lookup and operator overload lookup participate in
+// ADL, the dependent type ns::T contributes to the overload set considered by
+// ADL. In other words, ADL will consider all associated namespaces: this
+// includes not only N, but also any inline friend functions declared by ns::T.
+//
+// The purpose of this test is to ensure that simply constructing or destroying
+// a container does not invoke ADL which would be problematic for unknown types.
+// By using the type 'Holder *' as a container value, we can avoid the
+// obvious problems with unknown types: a pointer is trivial, and its size is
+// known. However, any ADL which includes ns::T as an associated namesapce will
+// fail.
+//
+// For example:
+//
+//   namespace ns {
+// class T {
+//   // 13.5.7 [over.inc]:
+//   friend std::list::const_iterator
+//   operator++(std::list::const_iterator it) {
+// return /* ... */;
+//   }
+// };
+//   }
+//
+// The C++ standard stipulates that some functions, such as std::advance, use
+// overloaded operators (in C++14: 24.4.4 [iterator.operations]). The
+// implication is that calls to such a function are dependent on knowing the
+// overload set of operators in all associated namespaces; under ADL, this
+// includes the private friend function in the example above.
+//
+// However, for some operations, such as construction and destruction, no such
+// ADL is required. This can be important, for example, when using the Pimpl
+// pattern:
+//
+//   // Defined in a header file:
+//   class InterfaceList {
+// // Defined in a .cpp file:
+// class Impl;
+// vector impls;
+//   public:
+// ~InterfaceList();
+//   };
+//
+template 
+class Holder { T value; };
+
+namespace ns { class Fwd; }
+
+// TestSequencePtr and TestMappingPtr ensure that a given container type can be
+// default-constructed and destroyed with an incomplete value pointer type.
+template  class Container>
+void TestSequencePtr() {
+  using X = Container;
+  {
+X u;
+assert(u.empty());
+  }
+  {
+auto u = X();
+assert(u.empty());
+  }
+}
+
+template  class Container>
+void TestMappingPtr() {
+  {
+using X = Container;
+X u1;
+assert(u1.empty());
+auto u2 = X();
+assert(u2.empty());
+  }
+  {
+using X = Container;
+X u1;
+assert(u1.empty());
+auto u2 = X();
+assert(u2.empty());
+  }
+}
+
+int main()
+{
+  // Sequence containers:
+  {
+std::array a;
+(void)a;
+  }
+  TestSequencePtr();
+  TestSequencePtr();
+  TestSequencePtr();
+
+  // Associative containers, non-mapping:
+  TestSequencePtr();
+  TestSequencePtr();
+
+  // Associative containers, mapping:
+  TestMappingPtr();
+  TestMappingPtr();
+
+  // Container adapters:
+  TestSequencePtr();
+  TestSequencePtr();
+}
Index: include/memory
===
--- include/memory
+++ include/memory
@@ -1346,9 +1346,9 @@
 struct __has_construct
 : integral_constant(),
-  declval<_Pointer>(),
-  declval<_Args>()...)),
+decltype(_VSTD::__has_construct_test(declval<_Alloc>(),
+ declval<_Pointer>(),
+ declval<_Args>()...)),
 true_type>::value>
 {
 };
@@ -1367,8 +1367,8 @@
 struct __has_destroy
 : integral_constant(),
-declval<_Pointer>())),
+decltype(_VSTD::__has_destroy_test(declval<_Alloc>(),
+   declval<_Pointer>())),

[PATCH] D37538: [libc++] Remove problematic ADL in container implementations.

[David L. Jones via Phabricator via cfe-commits]

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



Comment at: include/deque:1167-1168
 allocator_type& __a = __alloc();
-for (iterator __i = begin(), __e = end(); __i != __e; ++__i)
-__alloc_traits::destroy(__a, _VSTD::addressof(*__i));
+for (iterator __i = begin(), __e = end(); __i.__ptr_ != __e.__ptr_; 
__i.operator++())
+__alloc_traits::destroy(__a, _VSTD::addressof(__i.operator*()));
 size() = 0;

EricWF wrote:
> rsmith wrote:
> > The other changes all look like obvious improvements to me. This one is a 
> > little more subtle, but if we want types like `deque` 
> > to be destructible, I think we need to do something equivalent to this.
> That's really yucky! I'm OK with this, but I really don't like it.
> 
> Fundamentally this can't work, at least not generically. When the allocator 
> produces a fancy pointer type, the operator lookups need to be performed 
> using ADL.
> 
> In this specific case, we control the iterator type, but this isn't always 
> true. for example like in `__split_buffer`, which uses the allocators pointer 
> type as an iterator.
> 
> @dlj I updated your test case to demonstrate the fancy-pointer problem: 
> https://gist.github.com/EricWF/b465fc475f55561ba972b6dd87e7e7ea
> 
> So I think we have a couple choices:
> 
> (1) Do nothing, since we can't universally support the behavior, so we 
> shouldn't attempt to support any form of this behavior.
> (2) Fix only the non-fancy pointer case (which is what this patch does).
> (3) Attempt to fix *all* cases, including the fancy-pointer ones. This won't 
> be possible, but perhaps certain containers can tolerate incomplete fancy 
> pointers?
> 
> Personally I'm leaning towards (1), since we can't claim to support the use 
> case, at least not universally. If we select (1) we should probably encode 
> the restriction formally in standardese.
> 
> 
So I think there are at least a couple of issues here, but first I want to get 
clarification on this point:

> Do nothing, since we can't universally support the behavior, so we shouldn't 
> attempt to support any form of this behavior.

To me, this sounds like it could be equivalent to the statement "someone might 
do a bad job of implementing type-erasure for fancy pointers, so we should 
never attempt to preserve type erasure of any pointers." Now, that statement 
seems tenuous //at best// (and a straw man on a slippery slope otherwise), so 
I'm guessing that's not quite what you meant. ;-)

That said, it does sort of make sense to ignore deque for now, so I'll drop it 
from the patch; but for other containers, I don't really see the argument.

As for #3: I'm able to make (most of) your gist pass for everything but 
deque... calls to _VSTD::__to_raw_pointer are enough to avoid ADL around 
operators, although a few other changes are needed, too. That probably makes 
sense to do as a follow-up.

As for deque in particular, I think there may just be some missing functions on 
the __deque_iterator, but it probably warrants a closer look before adding to 
the interface.



Comment at: include/memory:1349
 is_same<
-decltype(__has_construct_test(declval<_Alloc>(),
-  declval<_Pointer>(),
-  declval<_Args>()...)),
+decltype(_VSTD::__has_construct_test(declval<_Alloc>(),
+ declval<_Pointer>(),

EricWF wrote:
> Quuxplusone wrote:
> > EricWF wrote:
> > > Wouldn't the `declval` calls also need qualification?
> > (Sorry I'm jumping in before I know the full backstory here.) I would 
> > expect that the "declval" call doesn't need qualification because its 
> > argument-list is empty. ADL doesn't apply to template arguments AFAIK.
> > (Test case proving it to myself: 
> > https://wandbox.org/permlink/i0YarAfjOYKOhLFP )
> > 
> > Now, IMHO, `__has_construct_test` also doesn't need guarding against ADL, 
> > because it contains a double underscore and is thus firmly in the library's 
> > namespace. If the user has declared an ADL-findable entity 
> > `my::__has_construct_test`, they're already in undefined behavior land and 
> > you don't need to uglify libc++'s code just to coddle such users.
> > 
> > And, IMO, to the extent that ADL *does* work on the old code here, that's a 
> > feature, not a bug. As the implementor of some weird fancy pointer or 
> > iterator type, I might *like* having the power to hook into libc++'s 
> > customization points here. Of course I wouldn't try to hook into 
> > `__has_construct_test`, but `__to_raw_pointer` feels more likely. (At my 
> > current low level of understanding, that is.)
> >  If the user has declared an ADL-findable entity my::__has_construct_test, 
> > they're already in undefined behavior land 
> 
> The problem isn't that it will find a `my::__has_construct_test`, but that 
> 

[PATCH] D37818: [lit] Update clang and lld to use the new shared LLVMConfig stuff

[David L. Jones via Phabricator via cfe-commits]

dlj added inline comments.



Comment at: clang/test/lit.cfg:23
 # the test runner updated.
-config.test_format = lit.formats.ShTest(execute_external)
+config.test_format = lit.formats.ShTest(not llvm_config.use_lit_shell)
 

Minor nit: it seems reasonable enough to name this parameter:

  ShTest(execute_external=(not llvm_config.use_lit_shell))

(I don't think that's a problem for ShTest.__init__, and has the benefit of 
failing if the parameter name changes. Imagine if someone added another 
default-valued bool argument to ShTest.__init__...)



Comment at: llvm/utils/lit/lit/llvm/config.py:93
 def with_environment(self, variable, value, append_path = False):
-if append_path and variable in self.config.environment:
+if append_path:
+# For paths, we should be able to take a list of them and process 
all

Looking at the callers of this... should the path-append logic be a separate 
function?

For example:

llvm_config.with_environment('PATH', [config.llvm_tools_dir, 
config.clang_tools_dir], append_path=True)

versus maybe this:

llvm_config.append_env_pathvar('PATH', [config.llvm_tools_dir, 
config.clang_tools_dir])




Comment at: llvm/utils/lit/lit/llvm/config.py:128-129
+def clear_environment(self, variables):
+for name in variables:
+if name in self.config.environment:
+del self.config.environment[name]

You could also say:


```
for name in variables:
self.config.environment.pop(name, None)
```



Comment at: llvm/utils/lit/lit/llvm/config.py:132
+
+def feature_config(self, features, encoding = 'ascii'):
+# Ask llvm-config about the specified feature.

I think the expected format of the 'features' arg is complex enough that you 
should add a docstring that explains it...



Comment at: llvm/utils/lit/lit/llvm/config.py:134
+# Ask llvm-config about the specified feature.
+arguments = [x for (x, _) in features]
 try:

I think you could probably just use features.keys() for this, no?



Comment at: llvm/utils/lit/lit/llvm/config.py:147
+output = output.decode(encoding)
+lines = output.split('\n')
+for (line, (_, patterns)) in zip(lines, features):

You might want to use splitlines() for Windows compatibility.


https://reviews.llvm.org/D37818



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

[PATCH] D37538: [libc++] Remove problematic ADL in container implementations.

[David L. Jones via Phabricator via cfe-commits]

dlj created this revision.
Herald added a subscriber: sanjoy.
Herald added a reviewer: EricWF.

Some container operations require ADL. For example, std::advance is
required to use specific operators, which will participate in ADL.

However, implementation details which rely on SFINAE should be careful not to
inadvertently invoke ADL. Otherwise, the SFINAE lookup will (incorrectly)
consider namespaces other than std::. This is particularly problematic with
incomplete types, since the set of associated namespaces for a class includes
the body of the class (which may contain inline friend function overloads). If a
type is incomplete, its body cannot be added to the set of associated
namespaces; this results in an error.

The changes in this patch mostly appear to be omissions. Several of the changes
are for SFINAE overloads with internal names (in some cases, there are other
uses which are already correctly qualified). In a few cases, the implementation
details of iterators are directly to avoid invoking ADL (this seems unfortunate;
but on balance, better than failing when type erasure is otherwise plausible).


https://reviews.llvm.org/D37538

Files:
  include/__split_buffer
  include/deque
  include/memory
  test/std/containers/containers.general/construct_destruct.pass.cpp

Index: test/std/containers/containers.general/construct_destruct.pass.cpp
===
--- /dev/null
+++ test/std/containers/containers.general/construct_destruct.pass.cpp
@@ -0,0 +1,130 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+// Holder is a do-nothing wrapper around a value of the given type.
+//
+// Since unqualified name lookup and operator overload lookup participate in
+// ADL, the dependent type ns::T contributes to the overload set considered by
+// ADL. In other words, ADL will consider all associated namespaces: this
+// includes not only N, but also any inline friend functions declared by ns::T.
+//
+// The purpose of this test is to ensure that simply constructing or destroying
+// a container does not invoke ADL which would be problematic for unknown types.
+// By using the type 'Holder *' as a container value, we can avoid the
+// obvious problems with unknown types: a pointer is trivial, and its size is
+// known. However, any ADL which includes ns::T as an associated namesapce will
+// fail.
+//
+// For example:
+//
+//   namespace ns {
+// class T {
+//   // 13.5.7 [over.inc]:
+//   friend std::list::const_iterator
+//   operator++(std::list::const_iterator it) {
+// return /* ... */;
+//   }
+// };
+//   }
+//
+// The C++ standard stipulates that some functions, such as std::advance, use
+// overloaded operators (in C++14: 24.4.4 [iterator.operations]). The
+// implication is that calls to such a function are dependent on knowing the
+// overload set of operators in all associated namespaces; under ADL, this
+// includes the private friend function in the example above.
+//
+// However, for some operations, such as construction and destruction, no such
+// ADL is required. This can be important, for example, when using the Pimpl
+// pattern:
+//
+//   // Defined in a header file:
+//   class InterfaceList {
+// // Defined in a .cpp file:
+// class Impl;
+// vector impls;
+//   public:
+// ~InterfaceList();
+//   };
+//
+template 
+class Holder { T value; };
+
+namespace ns { class Fwd; }
+
+// TestSequencePtr and TestMappingPtr ensure that a given container type can be
+// default-constructed and destroyed with an incomplete value pointer type.
+template  class Container>
+void TestSequencePtr() {
+  using X = Container;
+  {
+X u;
+assert(u.empty());
+  }
+  {
+auto u = X();
+assert(u.empty());
+  }
+}
+
+template  class Container>
+void TestMappingPtr() {
+  {
+using X = Container;
+X u1;
+assert(u1.empty());
+auto u2 = X();
+assert(u2.empty());
+  }
+  {
+using X = Container;
+X u1;
+assert(u1.empty());
+auto u2 = X();
+assert(u2.empty());
+  }
+}
+
+int main()
+{
+  // Sequence containers:
+  {
+std::array a;
+(void)a;
+  }
+  TestSequencePtr();
+  TestSequencePtr();
+  TestSequencePtr();
+  TestSequencePtr();
+
+  // Associative containers, non-mapping:
+  TestSequencePtr();
+  TestSequencePtr();
+
+  // Associative containers, mapping:
+  TestMappingPtr();
+  TestMappingPtr();
+
+  // Container adapters:
+  TestSequencePtr();
+  

[PATCH] D35337: [Solaris] gcc runtime dropped support for .ctors, switch to .init_array

[David L. Jones via Phabricator via cfe-commits]

dlj accepted this revision.
dlj added a comment.

I think this change is structurally fine, but I'll defer to others on whether 
it's actually doing the right thing. (I'm pretty sure it is, but I'm far from 
an expert on Solaris or Sparc.)




Comment at: lib/Driver/ToolChains/Gnu.cpp:2467
   const Generic_GCC::GCCVersion  = GCCInstallation.getVersion();
   bool UseInitArrayDefault =
   getTriple().getArch() == llvm::Triple::aarch64 ||

fedor.sergeev wrote:
> davide wrote:
> > this is really a mess.
> Any suggestions on how to improve? :)
From my perspective, this is really no worse than what's already here. I tried 
to avoid changing logic in https://reviews.llvm.org/rL297250, but this function 
is an example of something that still can use a lot of work.

Since this variable depends on all the parts of the triple, it probably won't 
break down very cleanly among the various subclasses. That said, if you do want 
to refactor this, you'll of course want to spend some time to think through 
exactly how this should work, and spend some time gathering feedback from other 
Clang devs... so it's probably out of the scope of this patch. I would be happy 
to help review that change... but, more pragmatically, I'd say land this change 
first before starting any surgery.



Comment at: test/Driver/constructors.c:83
+// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1\
+// RUN: -target i386-pc-solaris2.11 \
+// RUN:   | FileCheck --check-prefix=CHECK-INIT-ARRAY %s

Thank you for adding these. :-D Most of the toolchain classes lack explanatory 
comments about which platform combinations are expected to work for different 
features, so test cases like this will be invaluable to anyone who wants to 
refactor parts of the driver.


https://reviews.llvm.org/D35337



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

[PATCH] D34853: Fix (benignly) incorrect GoogleTest specs in various lit configs.

[David L. Jones via Phabricator via cfe-commits]

dlj added a comment.

In https://reviews.llvm.org/D34853#798699, @andrewng wrote:

> Hi,
>
> I believe that this "build mode" is intended for the Visual Studio MSVC 
> build. This build is special in that it can produce builds for multiple 
> configurations, e.g. Debug, Release & RelWithDebInfo, within the same top 
> level build output directory. It is this configuration type that defines the 
> "build mode". This means that the unit tests will only pick up the 
> configuration that matches that of the lit that was run. Without the "build 
> mode" I believe lit might end up running all configurations of unit tests 
> that have been built, which is probably not the intended behaviour.
>
> Cheers,
> Andrew


Ah, excellent. I suspected something like this.

(I don't usually have access to Windows, and even then I typically use Ninja 
instead of VS... so cleanups like this could be hit-or-miss, I guess.)


Repository:
  rL LLVM

https://reviews.llvm.org/D34853



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

[PATCH] D34853: Fix (benignly) incorrect GoogleTest specs in various lit configs.

[David L. Jones via Phabricator via cfe-commits]

dlj created this revision.
dlj added projects: lld, clang.
Herald added subscribers: mehdi_amini, sanjoy.

The GoogleTest lit format accepts two parameters to its constructor: a 
subdirectory to find test binaries, and a required suffix for the test 
filenames. Typically, the config should look like this:

  config.test_format = lit.formats.GoogleTest('.', 'Tests')

This will search the current directory for filenames ending with 'Tests', and 
run the matching binaries it finds as GoogleTests. It appears that several lit 
configs pass a different value for the subdirectory, however. They use 
"config.llvm_build_mode", which would be set by @LLVM_BUILD_MODE@, but never 
is. Fortunately, this means that the Python lookup passes and finds an empty 
string, which means that lit searches the current directory.

I can imagine cases where looking for a build-specific subdirectory might have 
worked in the paste, but I suspect it is no longer the right behaviour to check 
with cmake. So, I'm removing the (somewhat confusing) LLVM_BUILD_MODE logic 
altogether.


Repository:
  rL LLVM

https://reviews.llvm.org/D34853

Files:
  cfe/trunk/test/Unit/lit.cfg
  cfe/trunk/test/Unit/lit.site.cfg.in
  compiler-rt/trunk/test/lit.common.configured.in
  compiler-rt/trunk/unittests/lit.common.unit.cfg
  compiler-rt/trunk/unittests/lit.common.unit.configured.in
  lld/trunk/test/Unit/lit.cfg
  lld/trunk/test/Unit/lit.site.cfg.in
  lldb/trunk/lit/Unit/lit.site.cfg.in
  llvm/trunk/test/Unit/lit.site.cfg.in

Index: llvm/trunk/test/Unit/lit.site.cfg.in
===
--- llvm/trunk/test/Unit/lit.site.cfg.in
+++ llvm/trunk/test/Unit/lit.site.cfg.in
@@ -5,15 +5,13 @@
 config.llvm_src_root = "@LLVM_SOURCE_DIR@"
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
-config.llvm_build_mode = "@LLVM_BUILD_MODE@"
 config.enable_shared = @ENABLE_SHARED@
 config.shlibdir = "@SHLIBDIR@"
 
-# Support substitution of the tools_dir and build_mode with user parameters.
+# Support substitution of the tools_dir with user parameters.
 # This is used when we can't determine the tool dir at configuration time.
 try:
 config.llvm_tools_dir = config.llvm_tools_dir % lit_config.params
-config.llvm_build_mode = config.llvm_build_mode % lit_config.params
 except KeyError:
 e = sys.exc_info()[1]
 key, = e.args
Index: lldb/trunk/lit/Unit/lit.site.cfg.in
===
--- lldb/trunk/lit/Unit/lit.site.cfg.in
+++ lldb/trunk/lit/Unit/lit.site.cfg.in
@@ -5,7 +5,6 @@
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
-config.llvm_build_mode = "@LLVM_BUILD_MODE@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.lldb_obj_root = "@LLDB_BINARY_DIR@"
 config.lldb_src_root = "@LLDB_SOURCE_DIR@"
@@ -17,7 +16,6 @@
 try:
 config.llvm_tools_dir = config.llvm_tools_dir % lit_config.params
 config.llvm_libs_dir = config.llvm_libs_dir % lit_config.params
-config.llvm_build_mode = config.llvm_build_mode % lit_config.params
 except KeyError as e:
 key, = e.args
 lit_config.fatal("unable to find %r parameter, use '--param=%s=VALUE'" % (key,key))
Index: lld/trunk/test/Unit/lit.site.cfg.in
===
--- lld/trunk/test/Unit/lit.site.cfg.in
+++ lld/trunk/test/Unit/lit.site.cfg.in
@@ -4,7 +4,6 @@
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
-config.llvm_build_mode = "@LLVM_BUILD_MODE@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.lld_obj_root = "@LLD_BINARY_DIR@"
 config.lld_src_root = "@LLD_SOURCE_DIR@"
@@ -16,7 +15,6 @@
 try:
 config.llvm_tools_dir = config.llvm_tools_dir % lit_config.params
 config.llvm_libs_dir = config.llvm_libs_dir % lit_config.params
-config.llvm_build_mode = config.llvm_build_mode % lit_config.params
 except KeyError as e:
 key, = e.args
 lit_config.fatal("unable to find %r parameter, use '--param=%s=VALUE'" % (key,key))
Index: lld/trunk/test/Unit/lit.cfg
===
--- lld/trunk/test/Unit/lit.cfg
+++ lld/trunk/test/Unit/lit.cfg
@@ -18,6 +18,4 @@
 config.test_exec_root = config.test_source_root
 
 # testFormat: The test format to use to interpret tests.
-if not hasattr(config, 'llvm_build_mode'):
-lit_config.fatal("unable to find llvm_build_mode value on config")
-config.test_format = lit.formats.GoogleTest(config.llvm_build_mode, 'Tests')
+config.test_format = lit.formats.GoogleTest('.', 'Tests')
Index: compiler-rt/trunk/unittests/lit.common.unit.configured.in
===
--- compiler-rt/trunk/unittests/lit.common.unit.configured.in
+++ compiler-rt/trunk/unittests/lit.common.unit.configured.in
@@ -7,15 +7,13 @@
 

[PATCH] D30372: [Driver] Consolidate tools and toolchains by target platform. (NFC)

[David L. Jones via Phabricator via cfe-commits]

dlj added a comment.

In https://reviews.llvm.org/D30372#688154, @mehdi_amini wrote:

> In https://reviews.llvm.org/D30372#687060, @ahatanak wrote:
>
> > In https://reviews.llvm.org/D30372#687054, @dlj wrote:
> >
> > > In https://reviews.llvm.org/D30372#686871, @ahatanak wrote:
> > >
> > > > Have you considered using "include_directories" in CMakeLists.txt to 
> > > > avoid including "../Something.h"?
> > >
> > >
> > > I don't want to take that approach, and there's a specific reason why: my 
> > > concern isn't with three extra bytes, it's the fact that the headers are 
> > > in an unusual place. It's typical to include headers from a subdirectory, 
> > > or from the includes directory, but far less common to include from a 
> > > parent lib directory. Hiding that fact seems strictly worse to me.
> >
> >
> > OK. I was suggesting that only because LLVM already uses 
> > include_directories to include a header that exists in the parent directory 
> > in some cases. Apparently that's not what you wanted.
>
>
> Yes it seems "common" in LLVM to do this in CMake: `include_directories( 
> ${CMAKE_CURRENT_BINARY_DIR}/.. ${CMAKE_CURRENT_SOURCE_DIR}/.. )`


Actually, since I'm still keeping these files in the clangDriver rule, the 
lib/Driver directory is already on the search path, so I can use a subpath of 
Driver already.


https://reviews.llvm.org/D30372



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

[PATCH] D30372: [Driver] Consolidate tools and toolchains by target platform. (NFC)

[David L. Jones via Phabricator via cfe-commits]

dlj added a comment.

In https://reviews.llvm.org/D30372#686871, @ahatanak wrote:

> Have you considered using "include_directories" in CMakeLists.txt to avoid 
> including "../Something.h"?


I don't want to take that approach, and there's a specific reason why: my 
concern isn't with three extra bytes, it's the fact that the headers are in an 
unusual place. It's typical to include headers from a subdirectory, or from the 
includes directory, but far less common to include from a parent lib directory. 
Hiding that fact seems strictly worse to me.


https://reviews.llvm.org/D30372



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

[PATCH] D29737: Updates documentation to include command to run clang-tidy tests.

[David L. Jones via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL294689: Adds the commandline need to run clang-tidy tests. 
(authored by dlj).

Changed prior to commit:
  https://reviews.llvm.org/D29737?vs=87728=87930#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D29737

Files:
  clang-tools-extra/trunk/docs/clang-tidy/index.rst


Index: clang-tools-extra/trunk/docs/clang-tidy/index.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/index.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/index.rst
@@ -537,6 +537,12 @@
 Testing Checks
 --
 
+To run tests for :program:`clang-tidy` use the command:
+
+.. code-block:: console
+
+  $ ninja check-clang-tools
+
 :program:`clang-tidy` checks can be tested using either unit tests or
 `lit`_ tests. Unit tests may be more convenient to test complex replacements
 with strict checks. `Lit`_ tests allow using partial text matching and regular


Index: clang-tools-extra/trunk/docs/clang-tidy/index.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/index.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/index.rst
@@ -537,6 +537,12 @@
 Testing Checks
 --
 
+To run tests for :program:`clang-tidy` use the command:
+
+.. code-block:: console
+
+  $ ninja check-clang-tools
+
 :program:`clang-tidy` checks can be tested using either unit tests or
 `lit`_ tests. Unit tests may be more convenient to test complex replacements
 with strict checks. `Lit`_ tests allow using partial text matching and regular
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29737: Updates documentation to include command to run clang-tidy tests.

[David L. Jones via Phabricator via cfe-commits]

dlj added a comment.

LGTM. I will land shortly.


https://reviews.llvm.org/D29737



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

[PATCH] D28478: Check for musl-libc's max_align_t in addition to other variants.

[David L. Jones via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL294683: Check for musl-libc's max_align_t in addition to 
other variants. (authored by dlj).

Changed prior to commit:
  https://reviews.llvm.org/D28478?vs=83681=87925#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28478

Files:
  libcxx/trunk/include/cstddef
  libcxx/trunk/include/stddef.h


Index: libcxx/trunk/include/stddef.h
===
--- libcxx/trunk/include/stddef.h
+++ libcxx/trunk/include/stddef.h
@@ -53,7 +53,8 @@
 }
 
 // Re-use the compiler's  max_align_t where possible.
-#if !defined(__CLANG_MAX_ALIGN_T_DEFINED) && !defined(_GCC_MAX_ALIGN_T)
+#if !defined(__CLANG_MAX_ALIGN_T_DEFINED) && !defined(_GCC_MAX_ALIGN_T) && \
+!defined(__DEFINED_max_align_t)
 typedef long double max_align_t;
 #endif
 
Index: libcxx/trunk/include/cstddef
===
--- libcxx/trunk/include/cstddef
+++ libcxx/trunk/include/cstddef
@@ -48,7 +48,8 @@
 using ::ptrdiff_t;
 using ::size_t;
 
-#if defined(__CLANG_MAX_ALIGN_T_DEFINED) || defined(_GCC_MAX_ALIGN_T)
+#if defined(__CLANG_MAX_ALIGN_T_DEFINED) || defined(_GCC_MAX_ALIGN_T) || \
+defined(__DEFINED_max_align_t)
 // Re-use the compiler's  max_align_t where possible.
 using ::max_align_t;
 #else


Index: libcxx/trunk/include/stddef.h
===
--- libcxx/trunk/include/stddef.h
+++ libcxx/trunk/include/stddef.h
@@ -53,7 +53,8 @@
 }
 
 // Re-use the compiler's  max_align_t where possible.
-#if !defined(__CLANG_MAX_ALIGN_T_DEFINED) && !defined(_GCC_MAX_ALIGN_T)
+#if !defined(__CLANG_MAX_ALIGN_T_DEFINED) && !defined(_GCC_MAX_ALIGN_T) && \
+!defined(__DEFINED_max_align_t)
 typedef long double max_align_t;
 #endif
 
Index: libcxx/trunk/include/cstddef
===
--- libcxx/trunk/include/cstddef
+++ libcxx/trunk/include/cstddef
@@ -48,7 +48,8 @@
 using ::ptrdiff_t;
 using ::size_t;
 
-#if defined(__CLANG_MAX_ALIGN_T_DEFINED) || defined(_GCC_MAX_ALIGN_T)
+#if defined(__CLANG_MAX_ALIGN_T_DEFINED) || defined(_GCC_MAX_ALIGN_T) || \
+defined(__DEFINED_max_align_t)
 // Re-use the compiler's  max_align_t where possible.
 using ::max_align_t;
 #else
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28478: Check for musl-libc's max_align_t in addition to other variants.

[David L. Jones via Phabricator via cfe-commits]

dlj added a comment.

In https://reviews.llvm.org/D28478#672959, @EricWF wrote:

> IDK how to meaningly test this though.


Heh. Well... I can tell you that with this change (and a couple of others), I'm 
able to bootstrap Clang (2-stage) using libc++ on Alpine Linux. It totally 
works, I promise! ;-)


https://reviews.llvm.org/D28478



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

[PATCH] D16135: Macro Debug Info support in Clang

[David L. Jones via Phabricator via cfe-commits]

dlj added inline comments.



Comment at: cfe/trunk/lib/CodeGen/MacroPPCallbacks.cpp:125
+  switch (Status) {
+  default:
+llvm_unreachable("Do not expect to enter a file from current scope");

As a heads up... this fails under -Werror:

llvm/tools/clang/lib/CodeGen/MacroPPCallbacks.cpp:125:3: error: default label 
in switch which covers all enumeration values [-Werror,-Wcovered-switch-default]
  default:
  ^
1 error generated.


Repository:
  rL LLVM

https://reviews.llvm.org/D16135



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

[PATCH] D28007: Switch TableGen to emit calls to ASTRecordReader for AttrPCHRead.

[David L. Jones via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL292868: Switch TableGen to emit calls to ASTRecordReader for 
AttrPCHRead. (authored by dlj).

Changed prior to commit:
  https://reviews.llvm.org/D28007?vs=85493=85498#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28007

Files:
  cfe/trunk/include/clang/Serialization/ASTReader.h
  cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
  cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp

Index: cfe/trunk/include/clang/Serialization/ASTReader.h
===
--- cfe/trunk/include/clang/Serialization/ASTReader.h
+++ cfe/trunk/include/clang/Serialization/ASTReader.h
@@ -2098,8 +2098,7 @@
  unsigned );
 
   /// \brief Reads attributes from the current stream position.
-  void ReadAttributes(ModuleFile , AttrVec ,
-  const RecordData , unsigned );
+  void ReadAttributes(ASTRecordReader , AttrVec );
 
   /// \brief Reads a statement.
   Stmt *ReadStmt(ModuleFile );
@@ -2284,6 +2283,14 @@
   /// \brief Reads a sub-expression operand during statement reading.
   Expr *readSubExpr() { return Reader->ReadSubExpr(); }
 
+  /// \brief Reads a declaration with the given local ID in the given module.
+  ///
+  /// \returns The requested declaration, casted to the given return type.
+  template
+  T *GetLocalDeclAs(uint32_t LocalID) {
+return cast_or_null(Reader->GetLocalDecl(*F, LocalID));
+  }
+
   /// \brief Reads a TemplateArgumentLocInfo appropriate for the
   /// given TemplateArgument kind, advancing Idx.
   TemplateArgumentLocInfo
@@ -2455,7 +2462,7 @@
 
   /// \brief Reads attributes from the current stream position, advancing Idx.
   void readAttributes(AttrVec ) {
-return Reader->ReadAttributes(*F, Attrs, Record, Idx);
+return Reader->ReadAttributes(*this, Attrs);
   }
 
   /// \brief Reads a token out of a record, advancing Idx.
Index: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
===
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
@@ -1831,7 +1831,7 @@
   SourceLocation *StoredLocs = D->getTrailingObjects();
   for (unsigned I = 0, N = Record.back(); I != N; ++I)
 StoredLocs[I] = ReadSourceLocation();
-  (void)Record.readInt(); // The number of stored source locations.
+  Record.skipInts(1); // The number of stored source locations.
 }
 
 void ASTDeclReader::VisitAccessSpecDecl(AccessSpecDecl *D) {
@@ -2471,12 +2471,11 @@
 //===--===//
 
 /// \brief Reads attributes from the current stream position.
-void ASTReader::ReadAttributes(ModuleFile , AttrVec ,
-   const RecordData , unsigned ) {
-  for (unsigned i = 0, e = Record[Idx++]; i != e; ++i) {
+void ASTReader::ReadAttributes(ASTRecordReader , AttrVec ) {
+  for (unsigned i = 0, e = Record.readInt(); i != e; ++i) {
 Attr *New = nullptr;
-attr::Kind Kind = (attr::Kind)Record[Idx++];
-SourceRange Range = ReadSourceRange(F, Record, Idx);
+attr::Kind Kind = (attr::Kind)Record.readInt();
+SourceRange Range = Record.readSourceRange();
 
 #include "clang/Serialization/AttrPCHRead.inc"
 
Index: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
===
--- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
+++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
@@ -90,13 +90,13 @@
 
 static std::string ReadPCHRecord(StringRef type) {
   return StringSwitch(type)
-.EndsWith("Decl *", "GetLocalDeclAs<" 
-  + std::string(type, 0, type.size()-1) + ">(F, Record[Idx++])")
-.Case("TypeSourceInfo *", "GetTypeSourceInfo(F, Record, Idx)")
-.Case("Expr *", "ReadExpr(F)")
-.Case("IdentifierInfo *", "GetIdentifierInfo(F, Record, Idx)")
-.Case("StringRef", "ReadString(Record, Idx)")
-.Default("Record[Idx++]");
+.EndsWith("Decl *", "Record.GetLocalDeclAs<" 
+  + std::string(type, 0, type.size()-1) + ">(Record.readInt())")
+.Case("TypeSourceInfo *", "Record.getTypeSourceInfo()")
+.Case("Expr *", "Record.readExpr()")
+.Case("IdentifierInfo *", "Record.getIdentifierInfo()")
+.Case("StringRef", "Record.readString()")
+.Default("Record.readInt()");
 }
 
 // Get a type that is suitable for storing an object of the specified type.
@@ -413,7 +413,7 @@
 
 void writePCHReadDecls(raw_ostream ) const override {
   OS << "std::string " << getLowerName()
- << "= ReadString(Record, Idx);\n";
+ << "= Record.readString();\n";
 }
 
 void writePCHReadArgs(raw_ostream ) const override {
@@ -539,13 +539,13 @@
 }
 
 void writePCHReadDecls(raw_ostream ) const override {
-  OS << "bool is" << getLowerName() << "Expr = Record[Idx++];\n";
+  OS << "bool is" << getLowerName() << "Expr = 

[PATCH] D28007: Switch TableGen to emit calls to ASTRecordReader for AttrPCHRead.

[David L. Jones via Phabricator via cfe-commits]

dlj updated this revision to Diff 85493.
dlj added a comment.

- Pull, merge, etc.


https://reviews.llvm.org/D28007

Files:
  include/clang/Serialization/ASTReader.h
  lib/Serialization/ASTReaderDecl.cpp
  utils/TableGen/ClangAttrEmitter.cpp

Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -90,13 +90,13 @@
 
 static std::string ReadPCHRecord(StringRef type) {
   return StringSwitch(type)
-.EndsWith("Decl *", "GetLocalDeclAs<" 
-  + std::string(type, 0, type.size()-1) + ">(F, Record[Idx++])")
-.Case("TypeSourceInfo *", "GetTypeSourceInfo(F, Record, Idx)")
-.Case("Expr *", "ReadExpr(F)")
-.Case("IdentifierInfo *", "GetIdentifierInfo(F, Record, Idx)")
-.Case("StringRef", "ReadString(Record, Idx)")
-.Default("Record[Idx++]");
+.EndsWith("Decl *", "Record.GetLocalDeclAs<" 
+  + std::string(type, 0, type.size()-1) + ">(Record.readInt())")
+.Case("TypeSourceInfo *", "Record.getTypeSourceInfo()")
+.Case("Expr *", "Record.readExpr()")
+.Case("IdentifierInfo *", "Record.getIdentifierInfo()")
+.Case("StringRef", "Record.readString()")
+.Default("Record.readInt()");
 }
 
 // Get a type that is suitable for storing an object of the specified type.
@@ -413,7 +413,7 @@
 
 void writePCHReadDecls(raw_ostream ) const override {
   OS << "std::string " << getLowerName()
- << "= ReadString(Record, Idx);\n";
+ << "= Record.readString();\n";
 }
 
 void writePCHReadArgs(raw_ostream ) const override {
@@ -539,13 +539,13 @@
 }
 
 void writePCHReadDecls(raw_ostream ) const override {
-  OS << "bool is" << getLowerName() << "Expr = Record[Idx++];\n";
+  OS << "bool is" << getLowerName() << "Expr = Record.readInt();\n";
   OS << "void *" << getLowerName() << "Ptr;\n";
   OS << "if (is" << getLowerName() << "Expr)\n";
-  OS << "  " << getLowerName() << "Ptr = ReadExpr(F);\n";
+  OS << "  " << getLowerName() << "Ptr = Record.readExpr();\n";
   OS << "else\n";
   OS << "  " << getLowerName()
- << "Ptr = GetTypeSourceInfo(F, Record, Idx);\n";
+ << "Ptr = Record.getTypeSourceInfo();\n";
 }
 
 void writePCHWrite(raw_ostream ) const override {
@@ -658,7 +658,7 @@
 }
 
 void writePCHReadDecls(raw_ostream ) const override {
-  OS << "unsigned " << getLowerName() << "Size = Record[Idx++];\n";
+  OS << "unsigned " << getLowerName() << "Size = Record.readInt();\n";
   OS << "SmallVector<" << getType() << ", 4> "
  << getLowerName() << ";\n";
   OS << "" << getLowerName() << ".reserve(" << getLowerName()
@@ -783,7 +783,7 @@
 void writePCHReadDecls(raw_ostream ) const override {
   OS << "" << getAttrName() << "Attr::" << type << " " << getLowerName()
  << "(static_cast<" << getAttrName() << "Attr::" << type
- << ">(Record[Idx++]));\n";
+ << ">(Record.readInt()));\n";
 }
 
 void writePCHReadArgs(raw_ostream ) const override {
@@ -906,14 +906,14 @@
 }
 
 void writePCHReadDecls(raw_ostream ) const override {
-  OS << "unsigned " << getLowerName() << "Size = Record[Idx++];\n";
+  OS << "unsigned " << getLowerName() << "Size = Record.readInt();\n";
   OS << "SmallVector<" << QualifiedTypeName << ", 4> " << getLowerName()
  << ";\n";
   OS << "" << getLowerName() << ".reserve(" << getLowerName()
  << "Size);\n";
   OS << "for (unsigned i = " << getLowerName() << "Size; i; --i)\n";
   OS << "  " << getLowerName() << ".push_back(" << "static_cast<"
- << QualifiedTypeName << ">(Record[Idx++]));\n";
+ << QualifiedTypeName << ">(Record.readInt()));\n";
 }
 
 void writePCHWrite(raw_ostream ) const override {
@@ -996,7 +996,7 @@
 
 void writePCHReadDecls(raw_ostream ) const override {
   OS << "VersionTuple " << getLowerName()
- << "= ReadVersionTuple(Record, Idx);\n";
+ << "= Record.readVersionTuple();\n";
 }
 
 void writePCHReadArgs(raw_ostream ) const override {
@@ -2126,9 +2126,9 @@
 
 OS << "  case attr::" << R.getName() << ": {\n";
 if (R.isSubClassOf(InhClass))
-  OS << "bool isInherited = Record[Idx++];\n";
-OS << "bool isImplicit = Record[Idx++];\n";
-OS << "unsigned Spelling = Record[Idx++];\n";
+  OS << "bool isInherited = Record.readInt();\n";
+OS << "bool isImplicit = Record.readInt();\n";
+OS << "unsigned Spelling = Record.readInt();\n";
 ArgRecords = R.getValueAsListOfDefs("Args");
 Args.clear();
 for (const auto *Arg : ArgRecords) {
Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ 

[PATCH] D28478: Check for musl-libc's max_align_t in addition to other variants.

[David L. Jones via Phabricator via cfe-commits]

dlj added a comment.

Ping?


https://reviews.llvm.org/D28478



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

[PATCH] D28477: Add LF_ prefix to LibFunc enums in TargetLibraryInfo.

[David L. Jones via Phabricator via cfe-commits]

dlj added a comment.

Ping?


https://reviews.llvm.org/D28477



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

[PATCH] D28007: Switch TableGen to emit calls to ASTRecordReader for AttrPCHRead.

[David L. Jones via Phabricator via cfe-commits]

dlj added a comment.

Ping?


https://reviews.llvm.org/D28007



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

[PATCH] D28427: Allow constexpr construction of subobjects unconditionally, not just in C++14.

[David L. Jones via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL291480: Allow constexpr construction of subobjects 
unconditionally, not just in C++14. (authored by dlj).

Changed prior to commit:
  https://reviews.llvm.org/D28427?vs=83650=83686#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28427

Files:
  cfe/trunk/lib/AST/ExprConstant.cpp
  cfe/trunk/test/CXX/basic/basic.start/basic.start.init/p2.cpp
  cfe/trunk/test/CodeGenCXX/global-array-destruction.cpp


Index: cfe/trunk/test/CodeGenCXX/global-array-destruction.cpp
===
--- cfe/trunk/test/CodeGenCXX/global-array-destruction.cpp
+++ cfe/trunk/test/CodeGenCXX/global-array-destruction.cpp
@@ -39,17 +39,17 @@
 T t[2][3] = { 1.0, 2, 3.0, 4, 5.0, 6, 7.0, 8, 9.0, 10, 11.0, 12 };
 
 // CHECK: call {{.*}} @__cxa_atexit
-// CHECK: getelementptr inbounds ({{.*}} bitcast {{.*}}* @t to %struct.T*), 
i64 6
+// CHECK: getelementptr inbounds ({{.*}} getelementptr inbounds ([2 x [3 x 
{{.*}}]], [2 x [3 x {{.*}}]]* @t, i32 0, i32 0, i32 0), i64 6)
 // CHECK: call void @_ZN1TD1Ev
 // CHECK: icmp eq {{.*}} @t
 // CHECK: br i1 {{.*}}
 
 static T t2[2][3] = { 1.0, 2, 3.0, 4, 5.0, 6, 7.0, 8, 9.0, 10, 11.0, 12 };
 
 // CHECK: call {{.*}} @__cxa_atexit
-// CHECK: getelementptr inbounds ({{.*}} bitcast {{.*}}* @_ZL2t2 to 
%struct.T*), i64 6
+// CHECK: getelementptr inbounds ({{.*}} getelementptr inbounds ([2 x [3 x 
{{.*}}]], [2 x [3 x {{.*}}]]* @_ZL2t2, i32 0, i32 0, i32 0), i64 6)
 // CHECK: call void @_ZN1TD1Ev
-// CHECK: icmp eq {{.*}} @_ZL2t
+// CHECK: icmp eq {{.*}} @_ZL2t2
 // CHECK: br i1 {{.*}}
 
 using U = T[2][3];
Index: cfe/trunk/test/CXX/basic/basic.start/basic.start.init/p2.cpp
===
--- cfe/trunk/test/CXX/basic/basic.start/basic.start.init/p2.cpp
+++ cfe/trunk/test/CXX/basic/basic.start/basic.start.init/p2.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -verify %s -pedantic-errors -std=c++11
+// RUN: %clang_cc1 -verify %s -pedantic-errors -std=c++14
+// expected-no-diagnostics
+
+struct foo_t {
+  union {
+int i;
+volatile int j;
+  } u;
+};
+
+__attribute__((__require_constant_initialization__))
+static const foo_t x = {{0}};
+
+union foo_u {
+  int i;
+  volatile int j;
+};
+
+__attribute__((__require_constant_initialization__))
+static const foo_u y = {0};
Index: cfe/trunk/lib/AST/ExprConstant.cpp
===
--- cfe/trunk/lib/AST/ExprConstant.cpp
+++ cfe/trunk/lib/AST/ExprConstant.cpp
@@ -1627,8 +1627,17 @@
   // C++1y: A constant initializer for an object o [...] may also invoke
   // constexpr constructors for o and its subobjects even if those objects
   // are of non-literal class types.
-  if (Info.getLangOpts().CPlusPlus14 && This &&
-  Info.EvaluatingDecl == This->getLValueBase())
+  //
+  // C++11 missed this detail for aggregates, so classes like this:
+  //   struct foo_t { union { int i; volatile int j; } u; };
+  // are not (obviously) initializable like so:
+  //   __attribute__((__require_constant_initialization__))
+  //   static const foo_t x = {{0}};
+  // because "i" is a subobject with non-literal initialization (due to the
+  // volatile member of the union). See:
+  //   http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1677
+  // Therefore, we use the C++1y behavior.
+  if (This && Info.EvaluatingDecl == This->getLValueBase())
 return true;
 
   // Prvalue constant expressions must be of literal types.


Index: cfe/trunk/test/CodeGenCXX/global-array-destruction.cpp
===
--- cfe/trunk/test/CodeGenCXX/global-array-destruction.cpp
+++ cfe/trunk/test/CodeGenCXX/global-array-destruction.cpp
@@ -39,17 +39,17 @@
 T t[2][3] = { 1.0, 2, 3.0, 4, 5.0, 6, 7.0, 8, 9.0, 10, 11.0, 12 };
 
 // CHECK: call {{.*}} @__cxa_atexit
-// CHECK: getelementptr inbounds ({{.*}} bitcast {{.*}}* @t to %struct.T*), i64 6
+// CHECK: getelementptr inbounds ({{.*}} getelementptr inbounds ([2 x [3 x {{.*}}]], [2 x [3 x {{.*}}]]* @t, i32 0, i32 0, i32 0), i64 6)
 // CHECK: call void @_ZN1TD1Ev
 // CHECK: icmp eq {{.*}} @t
 // CHECK: br i1 {{.*}}
 
 static T t2[2][3] = { 1.0, 2, 3.0, 4, 5.0, 6, 7.0, 8, 9.0, 10, 11.0, 12 };
 
 // CHECK: call {{.*}} @__cxa_atexit
-// CHECK: getelementptr inbounds ({{.*}} bitcast {{.*}}* @_ZL2t2 to %struct.T*), i64 6
+// CHECK: getelementptr inbounds ({{.*}} getelementptr inbounds ([2 x [3 x {{.*}}]], [2 x [3 x {{.*}}]]* @_ZL2t2, i32 0, i32 0, i32 0), i64 6)
 // CHECK: call void @_ZN1TD1Ev
-// CHECK: icmp eq {{.*}} @_ZL2t
+// CHECK: icmp eq {{.*}} @_ZL2t2
 // CHECK: br i1 {{.*}}
 
 using U = T[2][3];
Index: cfe/trunk/test/CXX/basic/basic.start/basic.start.init/p2.cpp
===
--- cfe/trunk/test/CXX/basic/basic.start/basic.start.init/p2.cpp
+++ 

[PATCH] D28478: Check for musl-libc's max_align_t in addition to other variants.

[David L. Jones via Phabricator via cfe-commits]

dlj created this revision.
dlj added a reviewer: mclow.lists.
dlj added a subscriber: cfe-commits.

Libcxx will define its own max_align_t when it is not available. However, the
availability checks today only check for Clang's definition and GCC's
definition. In particular, it does not check for musl's definition, which is the
same as GCC's but guarded with a different macro.


https://reviews.llvm.org/D28478

Files:
  include/cstddef
  include/stddef.h


Index: include/stddef.h
===
--- include/stddef.h
+++ include/stddef.h
@@ -53,7 +53,8 @@
 }
 
 // Re-use the compiler's  max_align_t where possible.
-#if !defined(__CLANG_MAX_ALIGN_T_DEFINED) && !defined(_GCC_MAX_ALIGN_T)
+#if !defined(__CLANG_MAX_ALIGN_T_DEFINED) && !defined(_GCC_MAX_ALIGN_T) && \
+!defined(__DEFINED_max_align_t)
 typedef long double max_align_t;
 #endif
 
Index: include/cstddef
===
--- include/cstddef
+++ include/cstddef
@@ -48,7 +48,8 @@
 using ::ptrdiff_t;
 using ::size_t;
 
-#if defined(__CLANG_MAX_ALIGN_T_DEFINED) || defined(_GCC_MAX_ALIGN_T)
+#if defined(__CLANG_MAX_ALIGN_T_DEFINED) || defined(_GCC_MAX_ALIGN_T) || \
+defined(__DEFINED_max_align_t)
 // Re-use the compiler's  max_align_t where possible.
 using ::max_align_t;
 #else


Index: include/stddef.h
===
--- include/stddef.h
+++ include/stddef.h
@@ -53,7 +53,8 @@
 }
 
 // Re-use the compiler's  max_align_t where possible.
-#if !defined(__CLANG_MAX_ALIGN_T_DEFINED) && !defined(_GCC_MAX_ALIGN_T)
+#if !defined(__CLANG_MAX_ALIGN_T_DEFINED) && !defined(_GCC_MAX_ALIGN_T) && \
+!defined(__DEFINED_max_align_t)
 typedef long double max_align_t;
 #endif
 
Index: include/cstddef
===
--- include/cstddef
+++ include/cstddef
@@ -48,7 +48,8 @@
 using ::ptrdiff_t;
 using ::size_t;
 
-#if defined(__CLANG_MAX_ALIGN_T_DEFINED) || defined(_GCC_MAX_ALIGN_T)
+#if defined(__CLANG_MAX_ALIGN_T_DEFINED) || defined(_GCC_MAX_ALIGN_T) || \
+defined(__DEFINED_max_align_t)
 // Re-use the compiler's  max_align_t where possible.
 using ::max_align_t;
 #else
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28477: Add LF_ prefix to LibFunc enums in TargetLibraryInfo.

[David L. Jones via Phabricator via cfe-commits]

dlj created this revision.
dlj added a reviewer: rsmith.
dlj added a subscriber: cfe-commits.

The LibFunc::Func enum holds enumerators named for libc functions.
Unfortunately, there are real situations, including libc implementations, where
function names are actually macros (musl uses "#define fopen64 fopen", for
example; any other transitively visible macro would have similar effects).

Strictly speaking, a conforming C++ Standard Library should provide any such
macros as functions instead (via ). However, there are some "library"
functions which are not part of the standard, and thus not subject to this
rule (fopen64, for example). So, in order to be both portable and consistent,
the enum should not use the bare function names.

The old enum naming used a namespace LibFunc and an enum Func, with bare
enumerators. This patch changes LibFunc to be an enum with enumerators prefixed
with "LF_". (Unfortunately, a scoped enum is not sufficient to override macros.)

These changes are for clang. See https://reviews.llvm.org/D28476 for LLVM.


https://reviews.llvm.org/D28477

Files:
  lib/CodeGen/BackendUtil.cpp


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -262,7 +262,7 @@
 TLII->disableAllFunctions();
   else {
 // Disable individual libc/libm calls in TargetLibraryInfo.
-LibFunc::Func F;
+LibFunc F;
 for (auto  : CodeGenOpts.getNoBuiltinFuncs())
   if (TLII->getLibFunc(FuncName, F))
 TLII->setUnavailable(F);


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -262,7 +262,7 @@
 TLII->disableAllFunctions();
   else {
 // Disable individual libc/libm calls in TargetLibraryInfo.
-LibFunc::Func F;
+LibFunc F;
 for (auto  : CodeGenOpts.getNoBuiltinFuncs())
   if (TLII->getLibFunc(FuncName, F))
 TLII->setUnavailable(F);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28427: Allow constexpr construction of subobjects unconditionally, not just in C++14.

[David L. Jones via Phabricator via cfe-commits]

dlj updated this revision to Diff 83650.
dlj added a comment.

- Fix lit checks.


https://reviews.llvm.org/D28427

Files:
  lib/AST/ExprConstant.cpp
  test/CXX/basic/basic.start/basic.start.init/p2.cpp
  test/CodeGenCXX/global-array-destruction.cpp


Index: test/CodeGenCXX/global-array-destruction.cpp
===
--- test/CodeGenCXX/global-array-destruction.cpp
+++ test/CodeGenCXX/global-array-destruction.cpp
@@ -39,17 +39,17 @@
 T t[2][3] = { 1.0, 2, 3.0, 4, 5.0, 6, 7.0, 8, 9.0, 10, 11.0, 12 };
 
 // CHECK: call {{.*}} @__cxa_atexit
-// CHECK: getelementptr inbounds ({{.*}} bitcast {{.*}}* @t to %struct.T*), 
i64 6
+// CHECK: getelementptr inbounds ({{.*}} getelementptr inbounds ([2 x [3 x 
{{.*}}]], [2 x [3 x {{.*}}]]* @t, i32 0, i32 0, i32 0), i64 6)
 // CHECK: call void @_ZN1TD1Ev
 // CHECK: icmp eq {{.*}} @t
 // CHECK: br i1 {{.*}}
 
 static T t2[2][3] = { 1.0, 2, 3.0, 4, 5.0, 6, 7.0, 8, 9.0, 10, 11.0, 12 };
 
 // CHECK: call {{.*}} @__cxa_atexit
-// CHECK: getelementptr inbounds ({{.*}} bitcast {{.*}}* @_ZL2t2 to 
%struct.T*), i64 6
+// CHECK: getelementptr inbounds ({{.*}} getelementptr inbounds ([2 x [3 x 
{{.*}}]], [2 x [3 x {{.*}}]]* @_ZL2t2, i32 0, i32 0, i32 0), i64 6)
 // CHECK: call void @_ZN1TD1Ev
-// CHECK: icmp eq {{.*}} @_ZL2t
+// CHECK: icmp eq {{.*}} @_ZL2t2
 // CHECK: br i1 {{.*}}
 
 using U = T[2][3];
Index: test/CXX/basic/basic.start/basic.start.init/p2.cpp
===
--- /dev/null
+++ test/CXX/basic/basic.start/basic.start.init/p2.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -verify %s -pedantic-errors -std=c++11
+// RUN: %clang_cc1 -verify %s -pedantic-errors -std=c++14
+// expected-no-diagnostics
+
+struct foo_t {
+  union {
+int i;
+volatile int j;
+  } u;
+};
+
+__attribute__((__require_constant_initialization__))
+static const foo_t x = {{0}};
+
+union foo_u {
+  int i;
+  volatile int j;
+};
+
+__attribute__((__require_constant_initialization__))
+static const foo_u y = {0};
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -1627,8 +1627,17 @@
   // C++1y: A constant initializer for an object o [...] may also invoke
   // constexpr constructors for o and its subobjects even if those objects
   // are of non-literal class types.
-  if (Info.getLangOpts().CPlusPlus14 && This &&
-  Info.EvaluatingDecl == This->getLValueBase())
+  //
+  // C++11 missed this detail for aggregates, so classes like this:
+  //   struct foo_t { union { int i; volatile int j; } u; };
+  // are not (obviously) initializable like so:
+  //   __attribute__((__require_constant_initialization__))
+  //   static const foo_t x = {{0}};
+  // because "i" is a subobject with non-literal initialization (due to the
+  // volatile member of the union). See:
+  //   http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1677
+  // Therefore, we use the C++1y behavior.
+  if (This && Info.EvaluatingDecl == This->getLValueBase())
 return true;
 
   // Prvalue constant expressions must be of literal types.


Index: test/CodeGenCXX/global-array-destruction.cpp
===
--- test/CodeGenCXX/global-array-destruction.cpp
+++ test/CodeGenCXX/global-array-destruction.cpp
@@ -39,17 +39,17 @@
 T t[2][3] = { 1.0, 2, 3.0, 4, 5.0, 6, 7.0, 8, 9.0, 10, 11.0, 12 };
 
 // CHECK: call {{.*}} @__cxa_atexit
-// CHECK: getelementptr inbounds ({{.*}} bitcast {{.*}}* @t to %struct.T*), i64 6
+// CHECK: getelementptr inbounds ({{.*}} getelementptr inbounds ([2 x [3 x {{.*}}]], [2 x [3 x {{.*}}]]* @t, i32 0, i32 0, i32 0), i64 6)
 // CHECK: call void @_ZN1TD1Ev
 // CHECK: icmp eq {{.*}} @t
 // CHECK: br i1 {{.*}}
 
 static T t2[2][3] = { 1.0, 2, 3.0, 4, 5.0, 6, 7.0, 8, 9.0, 10, 11.0, 12 };
 
 // CHECK: call {{.*}} @__cxa_atexit
-// CHECK: getelementptr inbounds ({{.*}} bitcast {{.*}}* @_ZL2t2 to %struct.T*), i64 6
+// CHECK: getelementptr inbounds ({{.*}} getelementptr inbounds ([2 x [3 x {{.*}}]], [2 x [3 x {{.*}}]]* @_ZL2t2, i32 0, i32 0, i32 0), i64 6)
 // CHECK: call void @_ZN1TD1Ev
-// CHECK: icmp eq {{.*}} @_ZL2t
+// CHECK: icmp eq {{.*}} @_ZL2t2
 // CHECK: br i1 {{.*}}
 
 using U = T[2][3];
Index: test/CXX/basic/basic.start/basic.start.init/p2.cpp
===
--- /dev/null
+++ test/CXX/basic/basic.start/basic.start.init/p2.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -verify %s -pedantic-errors -std=c++11
+// RUN: %clang_cc1 -verify %s -pedantic-errors -std=c++14
+// expected-no-diagnostics
+
+struct foo_t {
+  union {
+int i;
+volatile int j;
+  } u;
+};
+
+__attribute__((__require_constant_initialization__))
+static const foo_t x = {{0}};
+
+union foo_u {
+  int i;
+  volatile int j;
+};
+
+__attribute__((__require_constant_initialization__))
+static const foo_u y = {0};
Index: 

[PATCH] D28427: Allow constexpr construction of subobjects unconditionally, not just in C++14.

[David L. Jones via Phabricator via cfe-commits]

dlj added a comment.

Test added, and fixed another one that I missed before.


https://reviews.llvm.org/D28427



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

[PATCH] D28427: Allow constexpr construction of subobjects unconditionally, not just in C++14.

[David L. Jones via Phabricator via cfe-commits]

dlj created this revision.
dlj added a reviewer: rsmith.
dlj added subscribers: cfe-commits, EricWF.

Per https://wg21.link/CWG1677, the C++11 standard did not clarify that constant
initialization of an object allowed constexpr brace-or-equal initialization of
subobjects:

  struct foo_t { union { int i; volatile int j; } u; };
  
  __attribute__((__require_constant_initialization__))
  static const foo_t x = {{0}};

Because foo_t::u has a volatile member, the initializer for x fails. However,
there is really no good reason, because this:

  union foo_u { int i; volatile int j; };
  __attribute__((__require_constant_initialization__))
  static const foo_u x = {0};

does have a constant initializer.

(This was triggered by musl's pthread_mutex_t type when building under C++11.)


https://reviews.llvm.org/D28427

Files:
  lib/AST/ExprConstant.cpp


Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -1627,8 +1627,17 @@
   // C++1y: A constant initializer for an object o [...] may also invoke
   // constexpr constructors for o and its subobjects even if those objects
   // are of non-literal class types.
-  if (Info.getLangOpts().CPlusPlus14 && This &&
-  Info.EvaluatingDecl == This->getLValueBase())
+  //
+  // C++11 missed this detail for aggregates, so classes like this:
+  //   struct foo_t { union { int i; volatile int j; } u; };
+  // are not (obviously) initializable like so:
+  //   __attribute__((__require_constant_initialization__))
+  //   static const foo_t x = {{0}};
+  // because "i" is a subobject with non-literal initialization (due to the
+  // volatile member of the union). See:
+  //   http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1677
+  // Therefore, we use the C++1y behavior.
+  if (This && Info.EvaluatingDecl == This->getLValueBase())
 return true;
 
   // Prvalue constant expressions must be of literal types.


Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -1627,8 +1627,17 @@
   // C++1y: A constant initializer for an object o [...] may also invoke
   // constexpr constructors for o and its subobjects even if those objects
   // are of non-literal class types.
-  if (Info.getLangOpts().CPlusPlus14 && This &&
-  Info.EvaluatingDecl == This->getLValueBase())
+  //
+  // C++11 missed this detail for aggregates, so classes like this:
+  //   struct foo_t { union { int i; volatile int j; } u; };
+  // are not (obviously) initializable like so:
+  //   __attribute__((__require_constant_initialization__))
+  //   static const foo_t x = {{0}};
+  // because "i" is a subobject with non-literal initialization (due to the
+  // volatile member of the union). See:
+  //   http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1677
+  // Therefore, we use the C++1y behavior.
+  if (This && Info.EvaluatingDecl == This->getLValueBase())
 return true;
 
   // Prvalue constant expressions must be of literal types.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28008: Rename several methods on ASTRecordReader to follow LLVM style (lowerCamelCase).

[David L. Jones via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL290236: Rename several methods on ASTRecordReader to follow 
LLVM style (lowerCamelCase). (authored by dlj).

Changed prior to commit:
  https://reviews.llvm.org/D28008?vs=82183=82197#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28008

Files:
  cfe/trunk/include/clang/Serialization/ASTReader.h
  cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
  cfe/trunk/lib/Serialization/ASTReaderStmt.cpp

Index: cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
===
--- cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
+++ cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
@@ -31,37 +31,37 @@
 llvm::BitstreamCursor 
 
 SourceLocation ReadSourceLocation() {
-  return Record.ReadSourceLocation();
+  return Record.readSourceLocation();
 }
 
 SourceRange ReadSourceRange() {
-  return Record.ReadSourceRange();
+  return Record.readSourceRange();
 }
 
 std::string ReadString() {
-  return Record.ReadString();
+  return Record.readString();
 }
 
 TypeSourceInfo *GetTypeSourceInfo() {
-  return Record.GetTypeSourceInfo();
+  return Record.getTypeSourceInfo();
 }
 
 Decl *ReadDecl() {
-  return Record.ReadDecl();
+  return Record.readDecl();
 }
 
 template
 T *ReadDeclAs() {
-  return Record.ReadDeclAs();
+  return Record.readDeclAs();
 }
 
 void ReadDeclarationNameLoc(DeclarationNameLoc ,
 DeclarationName Name) {
-  Record.ReadDeclarationNameLoc(DNLoc, Name);
+  Record.readDeclarationNameLoc(DNLoc, Name);
 }
 
 void ReadDeclarationNameInfo(DeclarationNameInfo ) {
-  Record.ReadDeclarationNameInfo(NameInfo);
+  Record.readDeclarationNameInfo(NameInfo);
 }
 
   public:
@@ -99,7 +99,7 @@
   ArgInfo.setLAngleLoc(ReadSourceLocation());
   ArgInfo.setRAngleLoc(ReadSourceLocation());
   for (unsigned i = 0; i != NumTemplateArgs; ++i)
-ArgInfo.addArgument(Record.ReadTemplateArgumentLoc());
+ArgInfo.addArgument(Record.readTemplateArgumentLoc());
   Args.initializeFrom(TemplateKWLoc, ArgInfo, ArgsLocArray);
 }
 
@@ -118,72 +118,72 @@
   SmallVector Stmts;
   unsigned NumStmts = Record.readInt();
   while (NumStmts--)
-Stmts.push_back(Record.ReadSubStmt());
+Stmts.push_back(Record.readSubStmt());
   S->setStmts(Record.getContext(), Stmts);
   S->LBraceLoc = ReadSourceLocation();
   S->RBraceLoc = ReadSourceLocation();
 }
 
 void ASTStmtReader::VisitSwitchCase(SwitchCase *S) {
   VisitStmt(S);
-  Record.RecordSwitchCaseID(S, Record.readInt());
+  Record.recordSwitchCaseID(S, Record.readInt());
   S->setKeywordLoc(ReadSourceLocation());
   S->setColonLoc(ReadSourceLocation());
 }
 
 void ASTStmtReader::VisitCaseStmt(CaseStmt *S) {
   VisitSwitchCase(S);
-  S->setLHS(Record.ReadSubExpr());
-  S->setRHS(Record.ReadSubExpr());
-  S->setSubStmt(Record.ReadSubStmt());
+  S->setLHS(Record.readSubExpr());
+  S->setRHS(Record.readSubExpr());
+  S->setSubStmt(Record.readSubStmt());
   S->setEllipsisLoc(ReadSourceLocation());
 }
 
 void ASTStmtReader::VisitDefaultStmt(DefaultStmt *S) {
   VisitSwitchCase(S);
-  S->setSubStmt(Record.ReadSubStmt());
+  S->setSubStmt(Record.readSubStmt());
 }
 
 void ASTStmtReader::VisitLabelStmt(LabelStmt *S) {
   VisitStmt(S);
   LabelDecl *LD = ReadDeclAs();
   LD->setStmt(S);
   S->setDecl(LD);
-  S->setSubStmt(Record.ReadSubStmt());
+  S->setSubStmt(Record.readSubStmt());
   S->setIdentLoc(ReadSourceLocation());
 }
 
 void ASTStmtReader::VisitAttributedStmt(AttributedStmt *S) {
   VisitStmt(S);
   uint64_t NumAttrs = Record.readInt();
   AttrVec Attrs;
-  Record.ReadAttributes(Attrs);
+  Record.readAttributes(Attrs);
   (void)NumAttrs;
   assert(NumAttrs == S->NumAttrs);
   assert(NumAttrs == Attrs.size());
   std::copy(Attrs.begin(), Attrs.end(), S->getAttrArrayPtr());
-  S->SubStmt = Record.ReadSubStmt();
+  S->SubStmt = Record.readSubStmt();
   S->AttrLoc = ReadSourceLocation();
 }
 
 void ASTStmtReader::VisitIfStmt(IfStmt *S) {
   VisitStmt(S);
   S->setConstexpr(Record.readInt());
-  S->setInit(Record.ReadSubStmt());
+  S->setInit(Record.readSubStmt());
   S->setConditionVariable(Record.getContext(), ReadDeclAs());
-  S->setCond(Record.ReadSubExpr());
-  S->setThen(Record.ReadSubStmt());
-  S->setElse(Record.ReadSubStmt());
+  S->setCond(Record.readSubExpr());
+  S->setThen(Record.readSubStmt());
+  S->setElse(Record.readSubStmt());
   S->setIfLoc(ReadSourceLocation());
   S->setElseLoc(ReadSourceLocation());
 }
 
 void ASTStmtReader::VisitSwitchStmt(SwitchStmt *S) {
   VisitStmt(S);
-  S->setInit(Record.ReadSubStmt());
+  S->setInit(Record.readSubStmt());
   S->setConditionVariable(Record.getContext(), ReadDeclAs());
-  S->setCond(Record.ReadSubExpr());
-  S->setBody(Record.ReadSubStmt());
+  S->setCond(Record.readSubExpr());
+  S->setBody(Record.readSubStmt());
   

[PATCH] D28008: Rename several methods on ASTRecordReader to follow LLVM style (lowerCamelCase).

[David L. Jones via Phabricator via cfe-commits]

dlj created this revision.
dlj added a reviewer: rsmith.
dlj added a subscriber: cfe-commits.

This follows up to r290217, and makes functions on ASTRecordReader consistent
and valid style.


https://reviews.llvm.org/D28008

Files:
  include/clang/Serialization/ASTReader.h
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTReaderStmt.cpp

Index: lib/Serialization/ASTReaderStmt.cpp
===
--- lib/Serialization/ASTReaderStmt.cpp
+++ lib/Serialization/ASTReaderStmt.cpp
@@ -31,37 +31,37 @@
 llvm::BitstreamCursor 
 
 SourceLocation ReadSourceLocation() {
-  return Record.ReadSourceLocation();
+  return Record.readSourceLocation();
 }
 
 SourceRange ReadSourceRange() {
-  return Record.ReadSourceRange();
+  return Record.readSourceRange();
 }
 
 std::string ReadString() {
-  return Record.ReadString();
+  return Record.readString();
 }
 
 TypeSourceInfo *GetTypeSourceInfo() {
-  return Record.GetTypeSourceInfo();
+  return Record.getTypeSourceInfo();
 }
 
 Decl *ReadDecl() {
-  return Record.ReadDecl();
+  return Record.readDecl();
 }
 
 template
 T *ReadDeclAs() {
-  return Record.ReadDeclAs();
+  return Record.readDeclAs();
 }
 
 void ReadDeclarationNameLoc(DeclarationNameLoc ,
 DeclarationName Name) {
-  Record.ReadDeclarationNameLoc(DNLoc, Name);
+  Record.readDeclarationNameLoc(DNLoc, Name);
 }
 
 void ReadDeclarationNameInfo(DeclarationNameInfo ) {
-  Record.ReadDeclarationNameInfo(NameInfo);
+  Record.readDeclarationNameInfo(NameInfo);
 }
 
   public:
@@ -99,7 +99,7 @@
   ArgInfo.setLAngleLoc(ReadSourceLocation());
   ArgInfo.setRAngleLoc(ReadSourceLocation());
   for (unsigned i = 0; i != NumTemplateArgs; ++i)
-ArgInfo.addArgument(Record.ReadTemplateArgumentLoc());
+ArgInfo.addArgument(Record.readTemplateArgumentLoc());
   Args.initializeFrom(TemplateKWLoc, ArgInfo, ArgsLocArray);
 }
 
@@ -118,72 +118,72 @@
   SmallVector Stmts;
   unsigned NumStmts = Record.readInt();
   while (NumStmts--)
-Stmts.push_back(Record.ReadSubStmt());
+Stmts.push_back(Record.readSubStmt());
   S->setStmts(Record.getContext(), Stmts);
   S->LBraceLoc = ReadSourceLocation();
   S->RBraceLoc = ReadSourceLocation();
 }
 
 void ASTStmtReader::VisitSwitchCase(SwitchCase *S) {
   VisitStmt(S);
-  Record.RecordSwitchCaseID(S, Record.readInt());
+  Record.recordSwitchCaseID(S, Record.readInt());
   S->setKeywordLoc(ReadSourceLocation());
   S->setColonLoc(ReadSourceLocation());
 }
 
 void ASTStmtReader::VisitCaseStmt(CaseStmt *S) {
   VisitSwitchCase(S);
-  S->setLHS(Record.ReadSubExpr());
-  S->setRHS(Record.ReadSubExpr());
-  S->setSubStmt(Record.ReadSubStmt());
+  S->setLHS(Record.readSubExpr());
+  S->setRHS(Record.readSubExpr());
+  S->setSubStmt(Record.readSubStmt());
   S->setEllipsisLoc(ReadSourceLocation());
 }
 
 void ASTStmtReader::VisitDefaultStmt(DefaultStmt *S) {
   VisitSwitchCase(S);
-  S->setSubStmt(Record.ReadSubStmt());
+  S->setSubStmt(Record.readSubStmt());
 }
 
 void ASTStmtReader::VisitLabelStmt(LabelStmt *S) {
   VisitStmt(S);
   LabelDecl *LD = ReadDeclAs();
   LD->setStmt(S);
   S->setDecl(LD);
-  S->setSubStmt(Record.ReadSubStmt());
+  S->setSubStmt(Record.readSubStmt());
   S->setIdentLoc(ReadSourceLocation());
 }
 
 void ASTStmtReader::VisitAttributedStmt(AttributedStmt *S) {
   VisitStmt(S);
   uint64_t NumAttrs = Record.readInt();
   AttrVec Attrs;
-  Record.ReadAttributes(Attrs);
+  Record.readAttributes(Attrs);
   (void)NumAttrs;
   assert(NumAttrs == S->NumAttrs);
   assert(NumAttrs == Attrs.size());
   std::copy(Attrs.begin(), Attrs.end(), S->getAttrArrayPtr());
-  S->SubStmt = Record.ReadSubStmt();
+  S->SubStmt = Record.readSubStmt();
   S->AttrLoc = ReadSourceLocation();
 }
 
 void ASTStmtReader::VisitIfStmt(IfStmt *S) {
   VisitStmt(S);
   S->setConstexpr(Record.readInt());
-  S->setInit(Record.ReadSubStmt());
+  S->setInit(Record.readSubStmt());
   S->setConditionVariable(Record.getContext(), ReadDeclAs());
-  S->setCond(Record.ReadSubExpr());
-  S->setThen(Record.ReadSubStmt());
-  S->setElse(Record.ReadSubStmt());
+  S->setCond(Record.readSubExpr());
+  S->setThen(Record.readSubStmt());
+  S->setElse(Record.readSubStmt());
   S->setIfLoc(ReadSourceLocation());
   S->setElseLoc(ReadSourceLocation());
 }
 
 void ASTStmtReader::VisitSwitchStmt(SwitchStmt *S) {
   VisitStmt(S);
-  S->setInit(Record.ReadSubStmt());
+  S->setInit(Record.readSubStmt());
   S->setConditionVariable(Record.getContext(), ReadDeclAs());
-  S->setCond(Record.ReadSubExpr());
-  S->setBody(Record.ReadSubStmt());
+  S->setCond(Record.readSubExpr());
+  S->setBody(Record.readSubStmt());
   S->setSwitchLoc(ReadSourceLocation());
   if (Record.readInt())
 S->setAllEnumCasesCovered();
@@ -204,27 +204,27 @@
   VisitStmt(S);
   S->setConditionVariable(Record.getContext(), 

[PATCH] D28007: Switch TableGen to emit calls to ASTRecordReader for AttrPCHRead.

[David L. Jones via Phabricator via cfe-commits]

dlj created this revision.
dlj added a reviewer: rsmith.
dlj added a subscriber: cfe-commits.

This patch changes TableGen-generated code in AttrPCHRead to call functions on
ASTRecordReader, instead of passing separate parameters to ASTReader. This is a
follow-up to r290217.


https://reviews.llvm.org/D28007

Files:
  include/clang/Serialization/ASTReader.h
  lib/Serialization/ASTReaderDecl.cpp
  utils/TableGen/ClangAttrEmitter.cpp

Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -90,13 +90,13 @@
 
 static std::string ReadPCHRecord(StringRef type) {
   return StringSwitch(type)
-.EndsWith("Decl *", "GetLocalDeclAs<" 
-  + std::string(type, 0, type.size()-1) + ">(F, Record[Idx++])")
-.Case("TypeSourceInfo *", "GetTypeSourceInfo(F, Record, Idx)")
-.Case("Expr *", "ReadExpr(F)")
-.Case("IdentifierInfo *", "GetIdentifierInfo(F, Record, Idx)")
-.Case("StringRef", "ReadString(Record, Idx)")
-.Default("Record[Idx++]");
+.EndsWith("Decl *", "Record.GetLocalDeclAs<" 
+  + std::string(type, 0, type.size()-1) + ">(Record.readInt())")
+.Case("TypeSourceInfo *", "Record.GetTypeSourceInfo()")
+.Case("Expr *", "Record.ReadExpr()")
+.Case("IdentifierInfo *", "Record.GetIdentifierInfo()")
+.Case("StringRef", "Record.ReadString()")
+.Default("Record.readInt()");
 }
 
 // Get a type that is suitable for storing an object of the specified type.
@@ -414,7 +414,7 @@
 
 void writePCHReadDecls(raw_ostream ) const override {
   OS << "std::string " << getLowerName()
- << "= ReadString(Record, Idx);\n";
+ << "= Record.ReadString();\n";
 }
 
 void writePCHReadArgs(raw_ostream ) const override {
@@ -540,13 +540,13 @@
 }
 
 void writePCHReadDecls(raw_ostream ) const override {
-  OS << "bool is" << getLowerName() << "Expr = Record[Idx++];\n";
+  OS << "bool is" << getLowerName() << "Expr = Record.readInt();\n";
   OS << "void *" << getLowerName() << "Ptr;\n";
   OS << "if (is" << getLowerName() << "Expr)\n";
-  OS << "  " << getLowerName() << "Ptr = ReadExpr(F);\n";
+  OS << "  " << getLowerName() << "Ptr = Record.ReadExpr();\n";
   OS << "else\n";
   OS << "  " << getLowerName()
- << "Ptr = GetTypeSourceInfo(F, Record, Idx);\n";
+ << "Ptr = Record.GetTypeSourceInfo();\n";
 }
 
 void writePCHWrite(raw_ostream ) const override {
@@ -659,7 +659,7 @@
 }
 
 void writePCHReadDecls(raw_ostream ) const override {
-  OS << "unsigned " << getLowerName() << "Size = Record[Idx++];\n";
+  OS << "unsigned " << getLowerName() << "Size = Record.readInt();\n";
   OS << "SmallVector<" << getType() << ", 4> "
  << getLowerName() << ";\n";
   OS << "" << getLowerName() << ".reserve(" << getLowerName()
@@ -784,7 +784,7 @@
 void writePCHReadDecls(raw_ostream ) const override {
   OS << "" << getAttrName() << "Attr::" << type << " " << getLowerName()
  << "(static_cast<" << getAttrName() << "Attr::" << type
- << ">(Record[Idx++]));\n";
+ << ">(Record.readInt()));\n";
 }
 
 void writePCHReadArgs(raw_ostream ) const override {
@@ -907,14 +907,14 @@
 }
 
 void writePCHReadDecls(raw_ostream ) const override {
-  OS << "unsigned " << getLowerName() << "Size = Record[Idx++];\n";
+  OS << "unsigned " << getLowerName() << "Size = Record.readInt();\n";
   OS << "SmallVector<" << QualifiedTypeName << ", 4> " << getLowerName()
  << ";\n";
   OS << "" << getLowerName() << ".reserve(" << getLowerName()
  << "Size);\n";
   OS << "for (unsigned i = " << getLowerName() << "Size; i; --i)\n";
   OS << "  " << getLowerName() << ".push_back(" << "static_cast<"
- << QualifiedTypeName << ">(Record[Idx++]));\n";
+ << QualifiedTypeName << ">(Record.readInt()));\n";
 }
 
 void writePCHWrite(raw_ostream ) const override {
@@ -997,7 +997,7 @@
 
 void writePCHReadDecls(raw_ostream ) const override {
   OS << "VersionTuple " << getLowerName()
- << "= ReadVersionTuple(Record, Idx);\n";
+ << "= Record.ReadVersionTuple();\n";
 }
 
 void writePCHReadArgs(raw_ostream ) const override {
@@ -2127,9 +2127,9 @@
 
 OS << "  case attr::" << R.getName() << ": {\n";
 if (R.isSubClassOf(InhClass))
-  OS << "bool isInherited = Record[Idx++];\n";
-OS << "bool isImplicit = Record[Idx++];\n";
-OS << "unsigned Spelling = Record[Idx++];\n";
+  OS << "bool isInherited = Record.readInt();\n";
+OS << "bool isImplicit = Record.readInt();\n";
+OS << "unsigned Spelling = Record.readInt();\n";
 ArgRecords = R.getValueAsListOfDefs("Args");
 Args.clear();
 for (const auto *Arg : 

[PATCH] D27836: Store the "current position" index within the ASTRecordReader.

[David L. Jones via Phabricator via cfe-commits]

dlj added a comment.

In https://reviews.llvm.org/D27836#628029, @rsmith wrote:

> LGTM, any chance I can tempt you to lowerCamelCase all the other 
> ASTRecordReader members to match the new ones as a follow-up change?


Yup, will do.


https://reviews.llvm.org/D27836



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

[PATCH] D27836: Store the "current position" index within the ASTRecordReader.

[David L. Jones via Phabricator via cfe-commits]

dlj added a comment.

Yeah, that makes more sense. Switched to readInt/peekInt/skipInts, let me know 
if you have a better idea for the names.


https://reviews.llvm.org/D27836



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

[PATCH] D27784: Add a class ASTRecordReader which wraps an ASTReader, a RecordData, and ModuleFile.

[David L. Jones via Phabricator via cfe-commits]

dlj added a comment.




https://reviews.llvm.org/D27784



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