[PATCH] D100742: [clangd] Parameter hints for dependent calls

[Nathan Ridge via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1f8963c80195: [clangd] Parameter hints for dependent calls 
(authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100742

Files:
  clang-tools-extra/clangd/HeuristicResolver.cpp
  clang-tools-extra/clangd/HeuristicResolver.h
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -244,19 +244,35 @@
ExpectedHint{"p3: ", "p3"});
 }
 
-TEST(ParameterHints, DependentCall) {
-  // FIXME: This doesn't currently produce a hint but should.
+TEST(ParameterHints, DependentCalls) {
   assertParameterHints(R"cpp(
 template 
-void foo(T param);
+void nonmember(T par1);
+
+template 
+struct A {
+  void member(T par2);
+  static void static_member(T par3);
+};
+
+void overload(int anInt);
+void overload(double aDouble);
 
 template 
 struct S {
-  void bar(T par) {
-foo($param[[par]]);
+  void bar(A a, T t) {
+nonmember($par1[[t]]);
+a.member($par2[[t]]);
+// FIXME: This one does not work yet.
+A::static_member($par3[[t]]);
+// We don't want to arbitrarily pick between
+// "anInt" or "aDouble", so just show no hint.
+overload(T{});
   }
 };
-  )cpp");
+  )cpp",
+   ExpectedHint{"par1: ", "par1"},
+   ExpectedHint{"par2: ", "par2"});
 }
 
 TEST(ParameterHints, VariadicFunction) {
@@ -362,4 +378,4 @@
 
 } // namespace
 } // namespace clangd
-} // namespace clang
+} // namespace clang
\ No newline at end of file
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -6,6 +6,7 @@
 //
 //===--===//
 #include "InlayHints.h"
+#include "HeuristicResolver.h"
 #include "ParsedAST.h"
 #include "support/Logger.h"
 #include "clang/AST/DeclarationName.h"
@@ -20,7 +21,8 @@
 public:
   InlayHintVisitor(std::vector &Results, ParsedAST &AST)
   : Results(Results), AST(AST.getASTContext()),
-MainFileID(AST.getSourceManager().getMainFileID()) {
+MainFileID(AST.getSourceManager().getMainFileID()),
+Resolver(AST.getHeuristicResolver()) {
 bool Invalid = false;
 llvm::StringRef Buf =
 AST.getSourceManager().getBufferData(MainFileID, &Invalid);
@@ -50,9 +52,18 @@
 if (isa(E) || isa(E))
   return true;
 
-processCall(E->getRParenLoc(),
-dyn_cast_or_null(E->getCalleeDecl()),
-{E->getArgs(), E->getNumArgs()});
+auto CalleeDecls = Resolver->resolveCalleeOfCallExpr(E);
+if (CalleeDecls.size() != 1)
+  return true;
+const FunctionDecl *Callee = nullptr;
+if (const auto *FD = dyn_cast(CalleeDecls[0]))
+  Callee = FD;
+else if (const auto *FTD = dyn_cast(CalleeDecls[0]))
+  Callee = FTD->getTemplatedDecl();
+if (!Callee)
+  return true;
+
+processCall(E->getRParenLoc(), Callee, {E->getArgs(), E->getNumArgs()});
 return true;
   }
 
@@ -266,6 +277,7 @@
   ASTContext *
   FileID MainFileID;
   StringRef MainFileBuf;
+  const HeuristicResolver *Resolver;
 };
 
 std::vector inlayHints(ParsedAST &AST) {
Index: clang-tools-extra/clangd/HeuristicResolver.h
===
--- clang-tools-extra/clangd/HeuristicResolver.h
+++ clang-tools-extra/clangd/HeuristicResolver.h
@@ -56,6 +56,8 @@
   std::vector
   resolveTypeOfCallExpr(const CallExpr *CE) const;
   std::vector
+  resolveCalleeOfCallExpr(const CallExpr *CE) const;
+  std::vector
   resolveUsingValueDecl(const UnresolvedUsingValueDecl *UUVD) const;
   std::vector
   resolveDependentNameType(const DependentNameType *DNT) const;
@@ -87,6 +89,7 @@
   // Try to heuristically resolve the type of a possibly-dependent expression
   // `E`.
   const Type *resolveExprToType(const Expr *E) const;
+  std::vector resolveExprToDecls(const Expr *E) const;
 
   // Given the type T of a dependent expression that appears of the LHS of a
   // "->", heuristically find a corresponding pointee type in whose scope we
Index: clang-tools-extra/clangd/HeuristicResolver.cpp
===
--- clang-tools-extra/clangd/HeuristicResolver.cpp
+++ clang-tools-extra/clangd/HeuristicResolver.cpp
@@ -130,6 +130,15 @@
   return {};
 }
 
+std::vector
+HeuristicResol

[clang-tools-extra] 1f8963c - [clangd] Parameter hints for dependent calls

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2021-05-03T02:03:16-04:00
New Revision: 1f8963c80195aa86d02e81acedcf1ff3da127842

URL: 
https://github.com/llvm/llvm-project/commit/1f8963c80195aa86d02e81acedcf1ff3da127842
DIFF: 
https://github.com/llvm/llvm-project/commit/1f8963c80195aa86d02e81acedcf1ff3da127842.diff

LOG: [clangd] Parameter hints for dependent calls

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

Added: 


Modified: 
clang-tools-extra/clangd/HeuristicResolver.cpp
clang-tools-extra/clangd/HeuristicResolver.h
clang-tools-extra/clangd/InlayHints.cpp
clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/HeuristicResolver.cpp 
b/clang-tools-extra/clangd/HeuristicResolver.cpp
index aacc90369fec5..316b7d11224b0 100644
--- a/clang-tools-extra/clangd/HeuristicResolver.cpp
+++ b/clang-tools-extra/clangd/HeuristicResolver.cpp
@@ -130,6 +130,15 @@ HeuristicResolver::resolveTypeOfCallExpr(const CallExpr 
*CE) const {
   return {};
 }
 
+std::vector
+HeuristicResolver::resolveCalleeOfCallExpr(const CallExpr *CE) const {
+  if (const auto *ND = dyn_cast_or_null(CE->getCalleeDecl())) {
+return {ND};
+  }
+
+  return resolveExprToDecls(CE->getCallee());
+}
+
 std::vector HeuristicResolver::resolveUsingValueDecl(
 const UnresolvedUsingValueDecl *UUVD) const {
   return resolveDependentMember(UUVD->getQualifier()->getAsType(),
@@ -163,18 +172,30 @@ const Type *resolveDeclsToType(const std::vector &Decls) {
   return nullptr;
 }
 
-const Type *HeuristicResolver::resolveExprToType(const Expr *E) const {
+std::vector
+HeuristicResolver::resolveExprToDecls(const Expr *E) const {
   if (const auto *ME = dyn_cast(E)) {
-return resolveDeclsToType(resolveMemberExpr(ME));
+return resolveMemberExpr(ME);
   }
   if (const auto *RE = dyn_cast(E)) {
-return resolveDeclsToType(resolveDeclRefExpr(RE));
+return resolveDeclRefExpr(RE);
+  }
+  if (const auto *OE = dyn_cast(E)) {
+return {OE->decls_begin(), OE->decls_end()};
   }
   if (const auto *CE = dyn_cast(E)) {
-return resolveDeclsToType(resolveTypeOfCallExpr(CE));
+return resolveTypeOfCallExpr(CE);
   }
   if (const auto *ME = dyn_cast(E))
-return resolveDeclsToType({ME->getMemberDecl()});
+return {ME->getMemberDecl()};
+
+  return {};
+}
+
+const Type *HeuristicResolver::resolveExprToType(const Expr *E) const {
+  std::vector Decls = resolveExprToDecls(E);
+  if (!Decls.empty())
+return resolveDeclsToType(Decls);
 
   return E->getType().getTypePtr();
 }

diff  --git a/clang-tools-extra/clangd/HeuristicResolver.h 
b/clang-tools-extra/clangd/HeuristicResolver.h
index dd906245da524..c66e1d1895699 100644
--- a/clang-tools-extra/clangd/HeuristicResolver.h
+++ b/clang-tools-extra/clangd/HeuristicResolver.h
@@ -56,6 +56,8 @@ class HeuristicResolver {
   std::vector
   resolveTypeOfCallExpr(const CallExpr *CE) const;
   std::vector
+  resolveCalleeOfCallExpr(const CallExpr *CE) const;
+  std::vector
   resolveUsingValueDecl(const UnresolvedUsingValueDecl *UUVD) const;
   std::vector
   resolveDependentNameType(const DependentNameType *DNT) const;
@@ -87,6 +89,7 @@ class HeuristicResolver {
   // Try to heuristically resolve the type of a possibly-dependent expression
   // `E`.
   const Type *resolveExprToType(const Expr *E) const;
+  std::vector resolveExprToDecls(const Expr *E) const;
 
   // Given the type T of a dependent expression that appears of the LHS of a
   // "->", heuristically find a corresponding pointee type in whose scope we

diff  --git a/clang-tools-extra/clangd/InlayHints.cpp 
b/clang-tools-extra/clangd/InlayHints.cpp
index a9f3100a187e1..3880273f85908 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -6,6 +6,7 @@
 //
 
//===--===//
 #include "InlayHints.h"
+#include "HeuristicResolver.h"
 #include "ParsedAST.h"
 #include "support/Logger.h"
 #include "clang/AST/DeclarationName.h"
@@ -20,7 +21,8 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
 public:
   InlayHintVisitor(std::vector &Results, ParsedAST &AST)
   : Results(Results), AST(AST.getASTContext()),
-MainFileID(AST.getSourceManager().getMainFileID()) {
+MainFileID(AST.getSourceManager().getMainFileID()),
+Resolver(AST.getHeuristicResolver()) {
 bool Invalid = false;
 llvm::StringRef Buf =
 AST.getSourceManager().getBufferData(MainFileID, &Invalid);
@@ -50,9 +52,18 @@ class InlayHintVisitor : public 
RecursiveASTVisitor {
 if (isa(E) || isa(E))
   return true;
 
-processCall(E->getRParenLoc(),
-dyn_cast_or_null(E->getCalleeDecl()),
-{E->getArgs(), E->getNumArgs()});
+auto CalleeDecls = Resolver->resolveCalleeOfCallExpr(E);
+if (CalleeDecls.size() != 1)
+  return true;
+const Fu

[PATCH] D101741: [clangd] Improve resolution of static method calls in HeuristicResolver

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
nridge added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
nridge requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101741

Files:
  clang-tools-extra/clangd/HeuristicResolver.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -263,7 +263,6 @@
   void bar(A a, T t) {
 nonmember($par1[[t]]);
 a.member($par2[[t]]);
-// FIXME: This one does not work yet.
 A::static_member($par3[[t]]);
 // We don't want to arbitrarily pick between
 // "anInt" or "aDouble", so just show no hint.
@@ -272,7 +271,8 @@
 };
   )cpp",
ExpectedHint{"par1: ", "par1"},
-   ExpectedHint{"par2: ", "par2"});
+   ExpectedHint{"par2: ", "par2"},
+   ExpectedHint{"par3: ", "par3"});
 }
 
 TEST(ParameterHints, VariadicFunction) {
Index: clang-tools-extra/clangd/HeuristicResolver.cpp
===
--- clang-tools-extra/clangd/HeuristicResolver.cpp
+++ clang-tools-extra/clangd/HeuristicResolver.cpp
@@ -16,6 +16,7 @@
 
 // Convenience lambdas for use as the 'Filter' parameter of
 // HeuristicResolver::resolveDependentMember().
+const auto NoFilter = [](const NamedDecl *D) { return true; };
 const auto NonStaticFilter = [](const NamedDecl *D) {
   return D->isCXXInstanceMember();
 };
@@ -90,6 +91,28 @@
 
 std::vector HeuristicResolver::resolveMemberExpr(
 const CXXDependentScopeMemberExpr *ME) const {
+  // If the expression has a qualifier, first try resolving the member
+  // inside the qualifier's type.
+  // Note that we cannot use a NonStaticFilter in either case, for a couple
+  // of reasons:
+  //   1. It's valid to access a static member using instance member syntax,
+  //  e.g. `instance.static_member`.
+  //   2. We can sometimes get a CXXDependentScopeMemberExpr for static
+  //  member syntax too, e.g. if `X::static_member` occurs inside
+  //  an instance method, it's represented as a CXXDependentScopeMemberExpr
+  //  with `this` as the base expression as `X` as the qualifier
+  //  (which could be valid if `X` names a base class after instantiation).
+  if (NestedNameSpecifier *NNS = ME->getQualifier()) {
+if (const Type *QualifierType = resolveNestedNameSpecifierToType(NNS)) {
+  auto Decls =
+  resolveDependentMember(QualifierType, ME->getMember(), NoFilter);
+  if (!Decls.empty())
+return Decls;
+}
+  }
+
+  // If that didn't yield any results, try resolving the member inside
+  // the expression's base type.
   const Type *BaseType = ME->getBaseType().getTypePtrOrNull();
   if (ME->isArrow()) {
 BaseType = getPointeeType(BaseType);
@@ -105,7 +128,7 @@
   BaseType = resolveExprToType(Base);
 }
   }
-  return resolveDependentMember(BaseType, ME->getMember(), NonStaticFilter);
+  return resolveDependentMember(BaseType, ME->getMember(), NoFilter);
 }
 
 std::vector HeuristicResolver::resolveDeclRefExpr(


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -263,7 +263,6 @@
   void bar(A a, T t) {
 nonmember($par1[[t]]);
 a.member($par2[[t]]);
-// FIXME: This one does not work yet.
 A::static_member($par3[[t]]);
 // We don't want to arbitrarily pick between
 // "anInt" or "aDouble", so just show no hint.
@@ -272,7 +271,8 @@
 };
   )cpp",
ExpectedHint{"par1: ", "par1"},
-   ExpectedHint{"par2: ", "par2"});
+   ExpectedHint{"par2: ", "par2"},
+   ExpectedHint{"par3: ", "par3"});
 }
 
 TEST(ParameterHints, VariadicFunction) {
Index: clang-tools-extra/clangd/HeuristicResolver.cpp
===
--- clang-tools-extra/clangd/HeuristicResolver.cpp
+++ clang-tools-extra/clangd/HeuristicResolver.cpp
@@ -16,6 +16,7 @@
 
 // Convenience lambdas for use as the 'Filter' parameter of
 // HeuristicResolver::resolveDependentMember().
+const auto NoFilter = [](const NamedDecl *D) { return true; };
 const auto NonStaticFilter = [](const NamedDecl *D) {
   return D->isCXXInstanceMember();
 };
@@ -90,6 +91,28 @@
 
 std::vector HeuristicResolver::resolveMemberExpr(
 const CXXDependentScopeMemberExpr *ME) const {
+  // If the expression has 

[PATCH] D100742: [clangd] Parameter hints for dependent calls

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:264
+// FIXME: This one does not work yet.
+A::static_member($par3[[t]]);
   }

sammccall wrote:
> nridge wrote:
> > This is an interesting case. Clang builds a `CXXDependentScopeMemberExpr` 
> > for this callee, but `HeuristicResolver` [currently 
> > assumes](https://searchfox.org/llvm/rev/92880ab7a2b2145f0605f367cd6d53d6892903c3/clang-tools-extra/clangd/HeuristicResolver.cpp#108)
> >  that such expressions are only built for non-static member accesses 
> > (since, for static member accesses, clang usually builds a 
> > `DependentScopeDeclRefExpr` instead).
> > 
> > The `CXXDependentScopeMemberExpr` is created 
> > [here](https://searchfox.org/llvm/rev/92880ab7a2b2145f0605f367cd6d53d6892903c3/clang/lib/Sema/SemaTemplate.cpp#757),
> >  and I note the dependence on whether the //enclosing context// is an 
> > [instance 
> > method](https://searchfox.org/llvm/rev/92880ab7a2b2145f0605f367cd6d53d6892903c3/clang/lib/Sema/SemaTemplate.cpp#750).
> >  I guess it must think that, after instantiation, `A` could turn out to 
> > be a base class, and thus this could be a "non-static member access with 
> > qualifier".
> > 
> > I don't see anything obvious on `CXXDependentScopeMemberExpr` that would 
> > let us tell apart "definitely a non-static member" from "maybe static, 
> > maybe non-static", so I guess the appropriate solution is to drop the 
> > `NonStaticFilter` 
> > [here](https://searchfox.org/llvm/rev/92880ab7a2b2145f0605f367cd6d53d6892903c3/clang-tools-extra/clangd/HeuristicResolver.cpp#108)
> >  altogether?
> > I guess it must think that, after instantiation, A could turn out to be 
> > a base class, and thus this could be a "non-static member access with 
> > qualifier".
> 
> Argh, C++ is so tricky :-( That sounds plausible to me.
> 
> > drop the NonStaticFilter here altogether
> 
> Yeah. The other thing is that `some_instance.some_static_member` is perfectly 
> legal I think? So the NonStaticFilter is probably not correct anyway.
This is sufficiently non-trivial to fix (requires more than just removing the 
filter) that I'll leave it for a separate patch / review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100742

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

[PATCH] D100742: [clangd] Parameter hints for dependent calls

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 342314.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100742

Files:
  clang-tools-extra/clangd/HeuristicResolver.cpp
  clang-tools-extra/clangd/HeuristicResolver.h
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -244,19 +244,35 @@
ExpectedHint{"p3: ", "p3"});
 }
 
-TEST(ParameterHints, DependentCall) {
-  // FIXME: This doesn't currently produce a hint but should.
+TEST(ParameterHints, DependentCalls) {
   assertParameterHints(R"cpp(
 template 
-void foo(T param);
+void nonmember(T par1);
+
+template 
+struct A {
+  void member(T par2);
+  static void static_member(T par3);
+};
+
+void overload(int anInt);
+void overload(double aDouble);
 
 template 
 struct S {
-  void bar(T par) {
-foo($param[[par]]);
+  void bar(A a, T t) {
+nonmember($par1[[t]]);
+a.member($par2[[t]]);
+// FIXME: This one does not work yet.
+A::static_member($par3[[t]]);
+// We don't want to arbitrarily pick between
+// "anInt" or "aDouble", so just show no hint.
+overload(T{});
   }
 };
-  )cpp");
+  )cpp",
+   ExpectedHint{"par1: ", "par1"},
+   ExpectedHint{"par2: ", "par2"});
 }
 
 TEST(ParameterHints, VariadicFunction) {
@@ -362,4 +378,4 @@
 
 } // namespace
 } // namespace clangd
-} // namespace clang
+} // namespace clang
\ No newline at end of file
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -6,6 +6,7 @@
 //
 //===--===//
 #include "InlayHints.h"
+#include "HeuristicResolver.h"
 #include "ParsedAST.h"
 #include "support/Logger.h"
 #include "clang/AST/DeclarationName.h"
@@ -20,7 +21,8 @@
 public:
   InlayHintVisitor(std::vector &Results, ParsedAST &AST)
   : Results(Results), AST(AST.getASTContext()),
-MainFileID(AST.getSourceManager().getMainFileID()) {
+MainFileID(AST.getSourceManager().getMainFileID()),
+Resolver(AST.getHeuristicResolver()) {
 bool Invalid = false;
 llvm::StringRef Buf =
 AST.getSourceManager().getBufferData(MainFileID, &Invalid);
@@ -50,9 +52,18 @@
 if (isa(E) || isa(E))
   return true;
 
-processCall(E->getRParenLoc(),
-dyn_cast_or_null(E->getCalleeDecl()),
-{E->getArgs(), E->getNumArgs()});
+auto CalleeDecls = Resolver->resolveCalleeOfCallExpr(E);
+if (CalleeDecls.size() != 1)
+  return true;
+const FunctionDecl *Callee = nullptr;
+if (const auto *FD = dyn_cast(CalleeDecls[0]))
+  Callee = FD;
+else if (const auto *FTD = dyn_cast(CalleeDecls[0]))
+  Callee = FTD->getTemplatedDecl();
+if (!Callee)
+  return true;
+
+processCall(E->getRParenLoc(), Callee, {E->getArgs(), E->getNumArgs()});
 return true;
   }
 
@@ -266,6 +277,7 @@
   ASTContext *
   FileID MainFileID;
   StringRef MainFileBuf;
+  const HeuristicResolver *Resolver;
 };
 
 std::vector inlayHints(ParsedAST &AST) {
Index: clang-tools-extra/clangd/HeuristicResolver.h
===
--- clang-tools-extra/clangd/HeuristicResolver.h
+++ clang-tools-extra/clangd/HeuristicResolver.h
@@ -56,6 +56,8 @@
   std::vector
   resolveTypeOfCallExpr(const CallExpr *CE) const;
   std::vector
+  resolveCalleeOfCallExpr(const CallExpr *CE) const;
+  std::vector
   resolveUsingValueDecl(const UnresolvedUsingValueDecl *UUVD) const;
   std::vector
   resolveDependentNameType(const DependentNameType *DNT) const;
@@ -87,6 +89,7 @@
   // Try to heuristically resolve the type of a possibly-dependent expression
   // `E`.
   const Type *resolveExprToType(const Expr *E) const;
+  std::vector resolveExprToDecls(const Expr *E) const;
 
   // Given the type T of a dependent expression that appears of the LHS of a
   // "->", heuristically find a corresponding pointee type in whose scope we
Index: clang-tools-extra/clangd/HeuristicResolver.cpp
===
--- clang-tools-extra/clangd/HeuristicResolver.cpp
+++ clang-tools-extra/clangd/HeuristicResolver.cpp
@@ -130,6 +130,15 @@
   return {};
 }
 
+std::vector
+HeuristicResolver::resolveCalleeOfCallExpr(const CallExpr *CE) const {
+  if (const auto *ND = dyn_cast_or_null(CE->getCalleeDecl())) {
+return {N

[clang-tools-extra] 3504e50 - [clangd] Fix test failure in initialize-params.test

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2021-05-03T01:37:09-04:00
New Revision: 3504e50b6db5ec71cf0df5a14a2576c1003614c9

URL: 
https://github.com/llvm/llvm-project/commit/3504e50b6db5ec71cf0df5a14a2576c1003614c9
DIFF: 
https://github.com/llvm/llvm-project/commit/3504e50b6db5ec71cf0df5a14a2576c1003614c9.diff

LOG: [clangd] Fix test failure in initialize-params.test

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

Added: 


Modified: 
clang-tools-extra/clangd/test/initialize-params.test

Removed: 




diff  --git a/clang-tools-extra/clangd/test/initialize-params.test 
b/clang-tools-extra/clangd/test/initialize-params.test
index 6cc6ac7272c58..4b40b17b2d64e 100644
--- a/clang-tools-extra/clangd/test/initialize-params.test
+++ b/clang-tools-extra/clangd/test/initialize-params.test
@@ -7,7 +7,6 @@
 # CHECK-NEXT:"capabilities": {
 # CHECK-NEXT:  "astProvider": true,
 # CHECK-NEXT:  "callHierarchyProvider": true,
-# CHECK-NEXT:  "clangdInlayHintsProvider": true,
 # CHECK-NEXT:  "codeActionProvider": true,
 # CHECK-NEXT:  "compilationDatabase": {
 # CHECK-NEXT:"automaticReload": true



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

[PATCH] D101740: [clangd] Fix test failure in initialize-params.test

[Nathan Ridge via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3504e50b6db5: [clangd] Fix test failure in 
initialize-params.test (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101740

Files:
  clang-tools-extra/clangd/test/initialize-params.test


Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -7,7 +7,6 @@
 # CHECK-NEXT:"capabilities": {
 # CHECK-NEXT:  "astProvider": true,
 # CHECK-NEXT:  "callHierarchyProvider": true,
-# CHECK-NEXT:  "clangdInlayHintsProvider": true,
 # CHECK-NEXT:  "codeActionProvider": true,
 # CHECK-NEXT:  "compilationDatabase": {
 # CHECK-NEXT:"automaticReload": true


Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -7,7 +7,6 @@
 # CHECK-NEXT:"capabilities": {
 # CHECK-NEXT:  "astProvider": true,
 # CHECK-NEXT:  "callHierarchyProvider": true,
-# CHECK-NEXT:  "clangdInlayHintsProvider": true,
 # CHECK-NEXT:  "codeActionProvider": true,
 # CHECK-NEXT:  "compilationDatabase": {
 # CHECK-NEXT:"automaticReload": true
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101740: [clangd] Fix test failure in initialize-params.test

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

Broke this in D101275  which put the 
capability behind a flag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101740

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

[PATCH] D101740: [clangd] Fix test failure in initialize-params.test

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
nridge requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101740

Files:
  clang-tools-extra/clangd/test/initialize-params.test


Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -7,7 +7,6 @@
 # CHECK-NEXT:"capabilities": {
 # CHECK-NEXT:  "astProvider": true,
 # CHECK-NEXT:  "callHierarchyProvider": true,
-# CHECK-NEXT:  "clangdInlayHintsProvider": true,
 # CHECK-NEXT:  "codeActionProvider": true,
 # CHECK-NEXT:  "compilationDatabase": {
 # CHECK-NEXT:"automaticReload": true


Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -7,7 +7,6 @@
 # CHECK-NEXT:"capabilities": {
 # CHECK-NEXT:  "astProvider": true,
 # CHECK-NEXT:  "callHierarchyProvider": true,
-# CHECK-NEXT:  "clangdInlayHintsProvider": true,
 # CHECK-NEXT:  "codeActionProvider": true,
 # CHECK-NEXT:  "compilationDatabase": {
 # CHECK-NEXT:"automaticReload": true
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 1f1fb5e - [clangd] Fix build error in SemanticHighlighting.cpp

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2021-05-03T01:19:07-04:00
New Revision: 1f1fb5e8e6b214f7f13a4e4731b6d6add33508aa

URL: 
https://github.com/llvm/llvm-project/commit/1f1fb5e8e6b214f7f13a4e4731b6d6add33508aa
DIFF: 
https://github.com/llvm/llvm-project/commit/1f1fb5e8e6b214f7f13a4e4731b6d6add33508aa.diff

LOG: [clangd] Fix build error in SemanticHighlighting.cpp

Added: 


Modified: 
clang-tools-extra/clangd/SemanticHighlighting.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index ffd6fdc361d2..12ccdff82d02 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -8,6 +8,7 @@
 
 #include "SemanticHighlighting.h"
 #include "FindTarget.h"
+#include "HeuristicResolver.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
 #include "SourceCode.h"



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

[PATCH] D101275: [clangd] Hide inlay hints capability behind a command-line flag

[Nathan Ridge via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcea736e5b8a4: [clangd] Hide inlay hints capability behind a 
command-line flag (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101275

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


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -308,6 +308,9 @@
 Hidden,
 };
 
+opt InlayHints{"inlay-hints", cat(Features),
+ desc("Enable preview of InlayHints feature"), 
init(false)};
+
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -829,6 +832,7 @@
   }
   Opts.AsyncThreadsCount = WorkerThreadsCount;
   Opts.FoldingRanges = FoldingRanges;
+  Opts.InlayHints = InlayHints;
   Opts.MemoryCleanup = getMemoryCleanupFunction();
 
   Opts.CodeComplete.IncludeIneligibleResults = IncludeIneligibleResults;
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -62,6 +62,9 @@
 std::function TweakFilter = [](const Tweak &T) {
   return !T.hidden(); // only enable non-hidden tweaks.
 };
+
+/// Enable preview of InlayHints feature.
+bool InlayHints = false;
   };
 
   ClangdLSPServer(Transport &Transp, const ThreadsafeFS &TFS,
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -571,7 +571,6 @@
   {"referencesProvider", true},
   {"astProvider", true}, // clangd extension
   {"typeHierarchyProvider", true},
-  {"clangdInlayHintsProvider", true},
   {"memoryUsageProvider", true}, // clangd extension
   {"compilationDatabase",// clangd extension
llvm::json::Object{{"automaticReload", true}}},
@@ -608,6 +607,9 @@
   if (Opts.FoldingRanges)
 ServerCaps["foldingRangeProvider"] = true;
 
+  if (Opts.InlayHints)
+ServerCaps["clangdInlayHintsProvider"] = true;
+
   std::vector Commands;
   for (llvm::StringRef Command : Handlers.CommandHandlers.keys())
 Commands.push_back(Command);


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -308,6 +308,9 @@
 Hidden,
 };
 
+opt InlayHints{"inlay-hints", cat(Features),
+ desc("Enable preview of InlayHints feature"), init(false)};
+
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -829,6 +832,7 @@
   }
   Opts.AsyncThreadsCount = WorkerThreadsCount;
   Opts.FoldingRanges = FoldingRanges;
+  Opts.InlayHints = InlayHints;
   Opts.MemoryCleanup = getMemoryCleanupFunction();
 
   Opts.CodeComplete.IncludeIneligibleResults = IncludeIneligibleResults;
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -62,6 +62,9 @@
 std::function TweakFilter = [](const Tweak &T) {
   return !T.hidden(); // only enable non-hidden tweaks.
 };
+
+/// Enable preview of InlayHints feature.
+bool InlayHints = false;
   };
 
   ClangdLSPServer(Transport &Transp, const ThreadsafeFS &TFS,
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -571,7 +571,6 @@
   {"referencesProvider", true},
   {"astProvider", true}, // clangd extension
   {"typeHierarchyProvider", true},
-  {"clangdInlayHintsProvider", true},
   {"memoryUsageProvider", true}, // clangd extension
   {"compilationDatabase",// clangd extension
llvm::json::Object{{"automaticReload", true}}},
@@ -608,6 +607,9 @@
   if (Opts.FoldingRanges)
 ServerCaps["foldingRangeProvider"] = true;
 
+  if (Opts.InlayHints)
+ServerCaps["clangdInlayHintsProvider"] = true;
+
   std::vector Commands;
   for (llvm::StringRef Command : Handlers.CommandHandlers.keys())
 Commands.push_back(Command);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] cea736e - [clangd] Hide inlay hints capability behind a command-line flag

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2021-05-03T01:01:57-04:00
New Revision: cea736e5b8a48065007a591d71699b53c04d95b3

URL: 
https://github.com/llvm/llvm-project/commit/cea736e5b8a48065007a591d71699b53c04d95b3
DIFF: 
https://github.com/llvm/llvm-project/commit/cea736e5b8a48065007a591d71699b53c04d95b3.diff

LOG: [clangd] Hide inlay hints capability behind a command-line flag

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/tool/ClangdMain.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index a7cc1ed42818f..77fc948cbfe08 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -571,7 +571,6 @@ void ClangdLSPServer::onInitialize(const InitializeParams 
&Params,
   {"referencesProvider", true},
   {"astProvider", true}, // clangd extension
   {"typeHierarchyProvider", true},
-  {"clangdInlayHintsProvider", true},
   {"memoryUsageProvider", true}, // clangd extension
   {"compilationDatabase",// clangd extension
llvm::json::Object{{"automaticReload", true}}},
@@ -608,6 +607,9 @@ void ClangdLSPServer::onInitialize(const InitializeParams 
&Params,
   if (Opts.FoldingRanges)
 ServerCaps["foldingRangeProvider"] = true;
 
+  if (Opts.InlayHints)
+ServerCaps["clangdInlayHintsProvider"] = true;
+
   std::vector Commands;
   for (llvm::StringRef Command : Handlers.CommandHandlers.keys())
 Commands.push_back(Command);

diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.h 
b/clang-tools-extra/clangd/ClangdLSPServer.h
index 577ee66b34243..3f5cc9abc688a 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -62,6 +62,9 @@ class ClangdLSPServer : private ClangdServer::Callbacks,
 std::function TweakFilter = [](const Tweak &T) {
   return !T.hidden(); // only enable non-hidden tweaks.
 };
+
+/// Enable preview of InlayHints feature.
+bool InlayHints = false;
   };
 
   ClangdLSPServer(Transport &Transp, const ThreadsafeFS &TFS,

diff  --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp 
b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index 8fe14231bfee1..07fec2fc76358 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -308,6 +308,9 @@ opt FoldingRanges{
 Hidden,
 };
 
+opt InlayHints{"inlay-hints", cat(Features),
+ desc("Enable preview of InlayHints feature"), 
init(false)};
+
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -829,6 +832,7 @@ clangd accepts flags on the commandline, and in the 
CLANGD_FLAGS environment var
   }
   Opts.AsyncThreadsCount = WorkerThreadsCount;
   Opts.FoldingRanges = FoldingRanges;
+  Opts.InlayHints = InlayHints;
   Opts.MemoryCleanup = getMemoryCleanupFunction();
 
   Opts.CodeComplete.IncludeIneligibleResults = IncludeIneligibleResults;



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

[PATCH] D101275: [clangd] Hide inlay hints capability behind a command-line flag

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 342310.
nridge marked 4 inline comments as done.
nridge added a comment.

Address review comments, including making the flag not hidden


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101275

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


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -307,6 +307,9 @@
 Hidden,
 };
 
+opt InlayHints{"inlay-hints", cat(Features),
+ desc("Enable preview of InlayHints feature"), 
init(false)};
+
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -815,6 +818,7 @@
   }
   Opts.AsyncThreadsCount = WorkerThreadsCount;
   Opts.FoldingRanges = FoldingRanges;
+  Opts.InlayHints = InlayHints;
   Opts.MemoryCleanup = getMemoryCleanupFunction();
 
   Opts.CodeComplete.IncludeIneligibleResults = IncludeIneligibleResults;
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -62,6 +62,9 @@
 std::function TweakFilter = [](const Tweak &T) {
   return !T.hidden(); // only enable non-hidden tweaks.
 };
+
+/// Enable preview of InlayHints feature.
+bool InlayHints = false;
   };
 
   ClangdLSPServer(Transport &Transp, const ThreadsafeFS &TFS,
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -571,7 +571,6 @@
   {"referencesProvider", true},
   {"astProvider", true}, // clangd extension
   {"typeHierarchyProvider", true},
-  {"clangdInlayHintsProvider", true},
   {"memoryUsageProvider", true}, // clangd extension
   {"compilationDatabase",// clangd extension
llvm::json::Object{{"automaticReload", true}}},
@@ -608,6 +607,9 @@
   if (Opts.FoldingRanges)
 ServerCaps["foldingRangeProvider"] = true;
 
+  if (Opts.InlayHints)
+ServerCaps["clangdInlayHintsProvider"] = true;
+
   std::vector Commands;
   for (llvm::StringRef Command : Handlers.CommandHandlers.keys())
 Commands.push_back(Command);


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -307,6 +307,9 @@
 Hidden,
 };
 
+opt InlayHints{"inlay-hints", cat(Features),
+ desc("Enable preview of InlayHints feature"), init(false)};
+
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -815,6 +818,7 @@
   }
   Opts.AsyncThreadsCount = WorkerThreadsCount;
   Opts.FoldingRanges = FoldingRanges;
+  Opts.InlayHints = InlayHints;
   Opts.MemoryCleanup = getMemoryCleanupFunction();
 
   Opts.CodeComplete.IncludeIneligibleResults = IncludeIneligibleResults;
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -62,6 +62,9 @@
 std::function TweakFilter = [](const Tweak &T) {
   return !T.hidden(); // only enable non-hidden tweaks.
 };
+
+/// Enable preview of InlayHints feature.
+bool InlayHints = false;
   };
 
   ClangdLSPServer(Transport &Transp, const ThreadsafeFS &TFS,
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -571,7 +571,6 @@
   {"referencesProvider", true},
   {"astProvider", true}, // clangd extension
   {"typeHierarchyProvider", true},
-  {"clangdInlayHintsProvider", true},
   {"memoryUsageProvider", true}, // clangd extension
   {"compilationDatabase",// clangd extension
llvm::json::Object{{"automaticReload", true}}},
@@ -608,6 +607,9 @@
   if (Opts.FoldingRanges)
 ServerCaps["foldingRangeProvider"] = true;
 
+  if (Opts.InlayHints)
+ServerCaps["clangdInlayHintsProvider"] = true;
+
   std::vector Commands;
   for (llvm::StringRef Command : Handlers.CommandHandlers.keys())
 Commands.push_back(Command);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101270: [clangd] Avoid including HeuristicResolver.h from ParsedAST.h

[Nathan Ridge via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG43cbf2bb84eb: [clangd] Avoid including HeuristicResolver.h 
from ParsedAST.h (authored by nridge).

Changed prior to commit:
  https://reviews.llvm.org/D101270?vs=340402&id=342309#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101270

Files:
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h


Index: clang-tools-extra/clangd/ParsedAST.h
===
--- clang-tools-extra/clangd/ParsedAST.h
+++ clang-tools-extra/clangd/ParsedAST.h
@@ -24,7 +24,6 @@
 #include "Compiler.h"
 #include "Diagnostics.h"
 #include "Headers.h"
-#include "HeuristicResolver.h"
 #include "Preamble.h"
 #include "index/CanonicalIncludes.h"
 #include "support/Path.h"
@@ -43,6 +42,7 @@
 
 namespace clang {
 namespace clangd {
+class HeuristicResolver;
 class SymbolIndex;
 
 /// Stores and provides access to parsed AST.
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -16,6 +16,7 @@
 #include "Diagnostics.h"
 #include "FeatureModule.h"
 #include "Headers.h"
+#include "HeuristicResolver.h"
 #include "IncludeFixer.h"
 #include "Preamble.h"
 #include "SourceCode.h"


Index: clang-tools-extra/clangd/ParsedAST.h
===
--- clang-tools-extra/clangd/ParsedAST.h
+++ clang-tools-extra/clangd/ParsedAST.h
@@ -24,7 +24,6 @@
 #include "Compiler.h"
 #include "Diagnostics.h"
 #include "Headers.h"
-#include "HeuristicResolver.h"
 #include "Preamble.h"
 #include "index/CanonicalIncludes.h"
 #include "support/Path.h"
@@ -43,6 +42,7 @@
 
 namespace clang {
 namespace clangd {
+class HeuristicResolver;
 class SymbolIndex;
 
 /// Stores and provides access to parsed AST.
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -16,6 +16,7 @@
 #include "Diagnostics.h"
 #include "FeatureModule.h"
 #include "Headers.h"
+#include "HeuristicResolver.h"
 #include "IncludeFixer.h"
 #include "Preamble.h"
 #include "SourceCode.h"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 43cbf2b - [clangd] Avoid including HeuristicResolver.h from ParsedAST.h

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2021-05-03T00:55:22-04:00
New Revision: 43cbf2bb84eb319ff3e95b3316344ece35ea59b1

URL: 
https://github.com/llvm/llvm-project/commit/43cbf2bb84eb319ff3e95b3316344ece35ea59b1
DIFF: 
https://github.com/llvm/llvm-project/commit/43cbf2bb84eb319ff3e95b3316344ece35ea59b1.diff

LOG: [clangd] Avoid including HeuristicResolver.h from ParsedAST.h

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

Added: 


Modified: 
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/ParsedAST.h

Removed: 




diff  --git a/clang-tools-extra/clangd/ParsedAST.cpp 
b/clang-tools-extra/clangd/ParsedAST.cpp
index 6cb76f32ced0d..0d7e4631d6601 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -16,6 +16,7 @@
 #include "Diagnostics.h"
 #include "FeatureModule.h"
 #include "Headers.h"
+#include "HeuristicResolver.h"
 #include "IncludeFixer.h"
 #include "Preamble.h"
 #include "SourceCode.h"

diff  --git a/clang-tools-extra/clangd/ParsedAST.h 
b/clang-tools-extra/clangd/ParsedAST.h
index c1ce6fce70297..703ae38254284 100644
--- a/clang-tools-extra/clangd/ParsedAST.h
+++ b/clang-tools-extra/clangd/ParsedAST.h
@@ -24,7 +24,6 @@
 #include "Compiler.h"
 #include "Diagnostics.h"
 #include "Headers.h"
-#include "HeuristicResolver.h"
 #include "Preamble.h"
 #include "index/CanonicalIncludes.h"
 #include "support/Path.h"
@@ -43,6 +42,7 @@
 
 namespace clang {
 namespace clangd {
+class HeuristicResolver;
 class SymbolIndex;
 
 /// Stores and provides access to parsed AST.



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

[PATCH] D101739: [OpenMP] Fix non-determinism in clang task codegen

[Giorgis Georgakoudis via Phabricator via cfe-commits]

ggeorgakoudis created this revision.
Herald added subscribers: mgrang, guansong, yaxunl.
ggeorgakoudis requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101739

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp


Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -4339,7 +4339,8 @@
   auto &&CodeGen = [&Data, &S, CS, &BodyGen, &LastprivateDstsOrigs,
 CapturedRegion](CodeGenFunction &CGF,
 PrePostActionTy &Action) {
-llvm::DenseMap, std::pair>
+llvm::MapVector,
+std::pair>
 UntiedLocalVars;
 // Set proper addresses for generated private copies.
 OMPPrivateScope Scope(CGF);
@@ -4392,7 +4393,12 @@
   Ty = CGF.getContext().getPointerType(Ty);
 Address PrivatePtr = CGF.CreateMemTemp(
 CGF.getContext().getPointerType(Ty), ".local.ptr.addr");
-UntiedLocalVars.try_emplace(VD, PrivatePtr, Address::invalid());
+auto Result = UntiedLocalVars.insert(
+std::make_pair(VD, std::make_pair(PrivatePtr, 
Address::invalid(;
+// If key exists update in place.
+if (Result.second == false)
+  *Result.first = std::make_pair(
+  VD, std::make_pair(PrivatePtr, Address::invalid()));
 CallArgs.push_back(PrivatePtr.getPointer());
 ParamTypes.push_back(PrivatePtr.getType());
   }
@@ -4424,14 +4430,14 @@
 if (isAllocatableDecl(Pair.first)) {
   llvm::Value *Ptr = CGF.Builder.CreateLoad(Pair.second.first);
   Address Replacement(Ptr, CGF.getPointerAlign());
-  Pair.getSecond().first = Replacement;
+  Pair.second.first = Replacement;
   Ptr = CGF.Builder.CreateLoad(Replacement);
   Replacement = Address(Ptr, 
CGF.getContext().getDeclAlign(Pair.first));
-  Pair.getSecond().second = Replacement;
+  Pair.second.second = Replacement;
 } else {
   llvm::Value *Ptr = CGF.Builder.CreateLoad(Pair.second.first);
   Address Replacement(Ptr, CGF.getContext().getDeclAlign(Pair.first));
-  Pair.getSecond().first = Replacement;
+  Pair.second.first = Replacement;
 }
   }
 }
Index: clang/lib/CodeGen/CGOpenMPRuntime.h
===
--- clang/lib/CodeGen/CGOpenMPRuntime.h
+++ clang/lib/CodeGen/CGOpenMPRuntime.h
@@ -253,8 +253,8 @@
   public:
 UntiedTaskLocalDeclsRAII(
 CodeGenFunction &CGF,
-const llvm::DenseMap,
- std::pair> &LocalVars);
+const llvm::MapVector,
+  std::pair> &LocalVars);
 ~UntiedTaskLocalDeclsRAII();
   };
 
@@ -723,8 +723,8 @@
   llvm::SmallVector NontemporalDeclsStack;
 
   using UntiedLocalVarsAddressesMap =
-  llvm::DenseMap,
- std::pair>;
+  llvm::MapVector,
+  std::pair>;
   llvm::SmallVector UntiedLocalVarsStack;
 
   /// Stack for list of addresses of declarations in current context marked as
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -12107,8 +12107,8 @@
 
 CGOpenMPRuntime::UntiedTaskLocalDeclsRAII::UntiedTaskLocalDeclsRAII(
 CodeGenFunction &CGF,
-const llvm::DenseMap,
- std::pair> &LocalVars)
+const llvm::MapVector,
+  std::pair> &LocalVars)
 : CGM(CGF.CGM), NeedToPush(!LocalVars.empty()) {
   if (!NeedToPush)
 return;


Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -4339,7 +4339,8 @@
   auto &&CodeGen = [&Data, &S, CS, &BodyGen, &LastprivateDstsOrigs,
 CapturedRegion](CodeGenFunction &CGF,
 PrePostActionTy &Action) {
-llvm::DenseMap, std::pair>
+llvm::MapVector,
+std::pair>
 UntiedLocalVars;
 // Set proper addresses for generated private copies.
 OMPPrivateScope Scope(CGF);
@@ -4392,7 +4393,12 @@
   Ty = CGF.getContext().getPointerType(Ty);
 Address PrivatePtr = CGF.CreateMemTemp(
 CGF.getContext().getPointerType(Ty), ".local.ptr.addr");
-UntiedLocalVars.try_emplace(VD, PrivatePtr, Address::invalid());
+auto Result = UntiedLocalVars.insert(
+std::make_pair(VD, std::make_pair(PrivatePtr, Address::invalid(;
+

[PATCH] D90110: [clang-tidy] Use --use-color in run-clang-tidy.py

[Toni Neubert via Phabricator via cfe-commits]

Toni added a comment.

Just a note for future changes: This change breaks parsers which relied on no 
colored input.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90110

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

[PATCH] D101735: [WebAssembly] Reenable end-to-end test in wasm-eh.cpp

[Heejin Ahn via Phabricator via cfe-commits]

aheejin created this revision.
aheejin added a reviewer: tlively.
Herald added subscribers: wingo, ecnelises, sunfish, jgravelle-google, sbc100, 
dschuff.
aheejin requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This was temporarily disabled while we were reimplementing the new spec.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101735

Files:
  clang/test/CodeGenCXX/wasm-eh.cpp


Index: clang/test/CodeGenCXX/wasm-eh.cpp
===
--- clang/test/CodeGenCXX/wasm-eh.cpp
+++ clang/test/CodeGenCXX/wasm-eh.cpp
@@ -405,8 +405,7 @@
 // Here we only check if the command enables wasm exception handling in the
 // backend so that exception handling instructions can be generated in .s file.
 
-// TODO Reenable these lines after updating the backend to the new spec
-// R UN: %clang_cc1 %s -triple wasm32-unknown-unknown -fms-extensions 
-fexceptions -fcxx-exceptions -exception-model=wasm -target-feature 
+exception-handling -S -o - -std=c++11 | FileCheck %s --check-prefix=ASSEMBLY
+// RUN: %clang_cc1 %s -triple wasm32-unknown-unknown -fms-extensions 
-fexceptions -fcxx-exceptions -exception-model=wasm -target-feature 
+exception-handling -S -o - -std=c++11 | FileCheck %s --check-prefix=ASSEMBLY
 
 // ASSEMBLY: try
 // ASSEMBLY: catch


Index: clang/test/CodeGenCXX/wasm-eh.cpp
===
--- clang/test/CodeGenCXX/wasm-eh.cpp
+++ clang/test/CodeGenCXX/wasm-eh.cpp
@@ -405,8 +405,7 @@
 // Here we only check if the command enables wasm exception handling in the
 // backend so that exception handling instructions can be generated in .s file.
 
-// TODO Reenable these lines after updating the backend to the new spec
-// R UN: %clang_cc1 %s -triple wasm32-unknown-unknown -fms-extensions -fexceptions -fcxx-exceptions -exception-model=wasm -target-feature +exception-handling -S -o - -std=c++11 | FileCheck %s --check-prefix=ASSEMBLY
+// RUN: %clang_cc1 %s -triple wasm32-unknown-unknown -fms-extensions -fexceptions -fcxx-exceptions -exception-model=wasm -target-feature +exception-handling -S -o - -std=c++11 | FileCheck %s --check-prefix=ASSEMBLY
 
 // ASSEMBLY: try
 // ASSEMBLY: catch
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101684: [WebAssembly] Add end-to-end codegen tests for wasm_simd128.h

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

I thought maybe /some/ of the other targets used end-to-end clang tests to test 
intrinsics, but I can't seem to find any (they seem to be a small minority, if 
there are any):

  grep -r -l intrin.h clang/test/ | xargs grep -L emit-llvm.*FileCheck | xargs 
grep RUN | less

(then looking for any RUN lines that use FileCheck but don't use emit-llvm)

Unless there's something really different about WebAssembly here, or some 
evidence that the existing test strategy has been problematic - I think it'd be 
good to stick with this general approach to testing WebAssembly's intrinsics 
too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101684

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

[PATCH] D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:836-837
+  NewState, NewNode,
+  C.getNoteTag([Msg](PathSensitiveBugReport &BR,
+ llvm::raw_ostream &OS) { OS << Msg; }));
 }

martong wrote:
> steakhal wrote:
> > This way each and every applied constraint will be displayed even if the 
> > given argument does not constitute to the bug condition.
> > I recommend you branching within the lambda, on the interestingness of the 
> > given argument constraint.
> Okay, good point, thanks for the feedback! I am planning to change to this 
> direction.
Excellent catch @steakhal!

I think you can always emit the note but only mark it as //unprunable// when 
the argument is interesting. This way it'd work identically to our normal 
"Assuming..." notes.



Comment at: 
clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp:16
+int test_note(int x, int y) {
+__single_val_1(x);  // expected-note{{Applied constraint: The 1st arg 
should be within the range [1, 1]}}
+return y / (1 - x); // expected-warning{{Division by zero}} \

martong wrote:
> NoQ wrote:
> > This has to be a user-friendly message.
> > * "Constraints" is compiler jargon.
> > * We cannot afford shortening "argument" to "arg".
> > * Generally, the less machine-generated it looks the better (":" is 
> > definitely robotic).
> Okay, thanks for your comment. I can make it to be more similar to the other 
> notes we already have. What about this?
> ```
> Assuming the 1st argument is within the range [1, 1]
> ```
> 
> > We cannot afford shortening "argument" to "arg".
> I'd like to address this in another following patch if you don't mind.
This sounds good for a generic message. I still think that most of the time 
these messages should be part of the summary. Eg., 
```
Assuming the 1st argument is within range [33, 47] U [58, 64] U [91, 96] U 
[123, 125]
```
ideally should be rephrased as
```
Assuming the argument is a punctuation character
```
in the summary of `ispunct()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101526

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

[PATCH] D101684: [WebAssembly] Add end-to-end codegen tests for wasm_simd128.h

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added a comment.

In D101684#2732551 , @dblaikie wrote:

> In D101684#2732522 , @aheejin wrote:
>
>> I think there's a clear upside on keeping this within clang/.
>>
>> 1. As @tlively said, there are many number of instructions to test and 
>> keeping "C function - LLVM intrinsic" and "LLVM intrinsic - Wasm 
>> instruction" tests in sync without autogeneration will be hard and 
>> error-prone.
>
> I don't necessarily see that they should be kept in sync - in the same way 
> that we don't do this for other IR targets and other features, like ABIs - 
> it's important they are lowered to specific instructions, but we don't 
> generally validate that through end to end tests.

What I meant by keeping in sync was, because there are many instructions and 
some of them are added and deleted as we progress, so making sure the two tests 
test the same set of instructions without missing anything is different, 
without resorting to some kind of autogeneration tool.

>> 2. Also it is not always the case that we have 1-1-1 relationship of C 
>> intrinsic function - LLVM intrinsic - Wasm instruction. For some of these we 
>> don't have our own intrinsics but rather use LLVM's common intrinsics, and 
>> they don't always boil down to a single LLVM intrinsic. There are cases 
>> where a single C function call can be lowered to multiple instructions in 
>> the LLVM IR, which we try to pattern match and lower to a single Wasm 
>> instruction. This kind of relationship can't be tested easily so it will 
>> take significantly longer time to check if a single C function call will 
>> result in a single Wasm instruction.
>
> The lack of 1-to-1 is part of the reason I prefer these tests to be separate. 
> If one C level intrinsic lowers to multiple IR operations - then it's good to 
> test each of those IR operations separately from each other. So that all 
> their uses can be validated. Then, knowing they are validated - the Clang 
> C-to-IR test can test that suitable operations are generated, knowing that 
> they're tested in LLVM to work as specified.

We have tests in LLVM too. But they can't check whether a single C function 
call boils down to a single Wasm instruction. And it is not always easy to look 
at those patterns and come up with the reverse mapping that created it. We 
match and optimize a lot of patterns, some of them generated from a single C 
function call but others not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101684

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

[PATCH] D101684: [WebAssembly] Add end-to-end codegen tests for wasm_simd128.h

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D101684#2732522 , @aheejin wrote:

> I think there's a clear upside on keeping this within clang/.
>
> 1. As @tlively said, there are many number of instructions to test and 
> keeping "C function - LLVM intrinsic" and "LLVM intrinsic - Wasm instruction" 
> tests in sync without autogeneration will be hard and error-prone.

I don't necessarily see that they should be kept in sync - in the same way that 
we don't do this for other IR targets and other features, like ABIs - it's 
important they are lowered to specific instructions, but we don't generally 
validate that through end to end tests.

> 2. Also it is not always the case that we have 1-1-1 relationship of C 
> intrinsic function - LLVM intrinsic - Wasm instruction. For some of these we 
> don't have our own intrinsics but rather use LLVM's common intrinsics, and 
> they don't always boil down to a single LLVM intrinsic. There are cases where 
> a single C function call can be lowered to multiple instructions in the LLVM 
> IR, which we try to pattern match and lower to a single Wasm instruction. 
> This kind of relationship can't be tested easily so it will take 
> significantly longer time to check if a single C function call will result in 
> a single Wasm instruction.

The lack of 1-to-1 is part of the reason I prefer these tests to be separate. 
If one C level intrinsic lowers to multiple IR operations - then it's good to 
test each of those IR operations separately from each other. So that all their 
uses can be validated. Then, knowing they are validated - the Clang C-to-IR 
test can test that suitable operations are generated, knowing that they're 
tested in LLVM to work as specified.

> It might be good to move this to clang/test/CodeGen though, if that's more 
> suitable.

I'm still not following why this is different from existing testing - many 
targets already exist in LLVM and already test their intrinsics in various 
ways, generally, so far as I know (though I haven't looked comprehensively - 
might be worth you taking a look at existing test strategies for intrinsics to 
compare/contrast?) . Why is WebAssembly different here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101684

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

[PATCH] D101684: [WebAssembly] Add end-to-end codegen tests for wasm_simd128.h

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added a comment.

I think there's a clear upside on keeping this within clang/.

1. As @tlively said, there are many number of instructions to test and keeping 
"C function - LLVM intrinsic" and "LLVM intrinsic - Wasm instruction" tests in 
sync without autogeneration will be hard and error-prone.

2. Also it is not always the case that we have 1-1-1 relationship of C 
intrinsic function - LLVM intrinsic - Wasm instruction. For some of these we 
don't have our own intrinsics but rather use LLVM's common intrinsics, and they 
don't always boil down to a single LLVM intrinsic. There are cases where a 
single C function call can be lowered to multiple instructions in the LLVM IR, 
which we try to pattern match and lower to a single Wasm instruction. This kind 
of relationship can't be tested easily so it will take significantly longer 
time to check if a single C function call will result in a single Wasm 
instruction.

It might be good to move this to clang/test/CodeGen though, if that's more 
suitable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101684

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

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

[Frank Derry Wanye via Phabricator via cfe-commits]

ffrankies updated this revision to Diff 342280.
ffrankies marked 12 inline comments as done.
ffrankies added a comment.

Removed unused import statements from IdDependentBackwardBranch.cpp


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

https://reviews.llvm.org/D70094

Files:
  clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
  clang-tools-extra/clang-tidy/altera/CMakeLists.txt
  clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
  clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/altera-id-dependent-backward-branch.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/altera-id-dependent-backward-branch.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/altera-id-dependent-backward-branch.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/altera-id-dependent-backward-branch.cpp
@@ -0,0 +1,86 @@
+// RUN: %check_clang_tidy %s altera-id-dependent-backward-branch %t -- -header-filter=.* "--" -cl-std=CL1.2 -c --include opencl-c.h
+
+typedef struct ExampleStruct {
+  int IDDepField;
+} ExampleStruct;
+
+void error() {
+  //  Conditional Expressions 
+  int accumulator = 0;
+  for (int i = 0; i < get_local_id(0); i++) {
+// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is ID-dependent due to ID function call and may cause performance degradation [altera-id-dependent-backward-branch]
+accumulator++;
+  }
+
+  int j = 0;
+  while (j < get_local_id(0)) {
+// CHECK-NOTES: :[[@LINE-1]]:10: warning: backward branch (while loop) is ID-dependent due to ID function call and may cause performance degradation [altera-id-dependent-backward-branch]
+accumulator++;
+  }
+
+  do {
+accumulator++;
+  } while (j < get_local_id(0));
+  // CHECK-NOTES: :[[@LINE-1]]:12: warning: backward branch (do loop) is ID-dependent due to ID function call and may cause performance degradation [altera-id-dependent-backward-branch]
+
+  //  Assignments 
+  int ThreadID = get_local_id(0);
+
+  while (j < ThreadID) {
+// CHECK-NOTES: :[[@LINE-3]]:3: note: assignment of ID-dependent variable ThreadID
+// CHECK-NOTES: :[[@LINE-2]]:10: warning: backward branch (while loop) is ID-dependent due to variable reference to 'ThreadID' and may cause performance degradation [altera-id-dependent-backward-branch]
+accumulator++;
+  }
+
+  ExampleStruct Example;
+  Example.IDDepField = get_local_id(0);
+
+  //  Inferred Assignments 
+  int ThreadID2 = ThreadID * get_local_size(0);
+
+  int ThreadID3 = Example.IDDepField; // OK: not used in any loops
+
+  ExampleStruct UnusedStruct = {
+  ThreadID * 2 // OK: not used in any loops
+  };
+
+  for (int i = 0; i < ThreadID2; i++) {
+// CHECK-NOTES: :[[@LINE-9]]:3: note: inferred assignment of ID-dependent value from ID-dependent variable ThreadID
+// CHECK-NOTES: :[[@LINE-2]]:19: warning: backward branch (for loop) is ID-dependent due to variable reference to 'ThreadID2' and may cause performance degradation [altera-id-dependent-backward-branch]
+accumulator++;
+  }
+
+  do {
+accumulator++;
+  } while (j < ThreadID);
+  // CHECK-NOTES: :[[@LINE-29]]:3: note: assignment of ID-dependent variable ThreadID
+  // CHECK-NOTES: :[[@LINE-2]]:12: warning: backward branch (do loop) is ID-dependent due to variable reference to 'ThreadID' and may cause performance degradation [altera-id-dependent-backward-branch]
+
+  for (int i = 0; i < Example.IDDepField; i++) {
+// CHECK-NOTES: :[[@LINE-24]]:3: note: assignment of ID-dependent field IDDepField
+// CHECK-NOTES: :[[@LINE-2]]:19: warning: backward branch (for loop) is ID-dependent due to member reference to 'IDDepField' and may cause performance degradation [altera-id-dependent-backward-branch]
+accumulator++;
+  }
+
+  while (j < Example.IDDepField) {
+// CHECK-NOTES: :[[@LINE-30]]:3: note: assignment of ID-dependent field IDDepField
+// CHECK-NOTES: :[[@LINE-2]]:10: warning: backward branch (while loop) is ID-dependent due to member reference to 'IDDepField' and may cause performance degradation [altera-id-dependent-backward-branch]
+accumulator++;
+  }
+
+  do {
+accumulator++;
+  } while (j < Example.IDDepField);
+  // CHECK-NOTES: :[[@LINE-38]]:3: note: assignment of ID-dependent field IDDepField
+  // CHECK-NOTES: :[[@LINE-2]]:12: warning: backward branch (do loop) is ID-dependent due to member reference to 'IDDepField' and may cause performance degradation [altera-id-dependent-backward-branch]
+}
+
+void success() {
+  int accumulator = 0;
+
+  for (int i = 0; i < 1000; i++) {
+if (i < get_local_id(0)) {
+  accumulator++;
+}
+  }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===

[PATCH] D101684: [WebAssembly] Add end-to-end codegen tests for wasm_simd128.h

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D101684#2732408 , @tlively wrote:

> In D101684#2732395 , @dblaikie 
> wrote:
>
>> In D101684#2732366 , @tlively 
>> wrote:
>>
>>> In order to get the benefit of this end-to-end test from split tests like 
>>> that, the LLVM test would have to be automatically generated from the clang 
>>> test.
>>
>> Why is that? We don't do that for other test surface area between clang and 
>> LLVM.
>
> The question this test answers is "Do the intrinsic functions generate the 
> proper WebAssembly instructions?"  (Notably, the test reveals that in 
> multiple cases, they don't). If we had separate C->IR and and IR->Wasm tests, 
> they would be able to answer this question only if we were sure that the 
> output of the C test matched the source of the IR test, and generating the IR 
> test from the C test would be the best way to ensure that.
>
> I understand your point that clang tests typically do not try to answer this 
> kind of question, but this is an important question to be able to answer for 
> the folks working on WebAssembly SIMD.

Is it fundamentally differently/more important than all the other questions 
like ABI compatibility? In what way?

> So the options I see are:
>
> 1. Have this abnormal end-to-end test in clang.
> 2. Autogenerate an IR test from the C test so the composition of tests tells 
> us what we want to know.
> 3. Host the test in some other repository.
>
> Among those, the first is both the easiest to maintain and the most useful.

My main objection is to the testing itself compared to the rest of the 
clang/llvm test philosophy - mostly in the hopes that splitting such testing, 
the same as nearly all other testing, would be sufficient here. If not, it 
might be nice to understand what kinds of test/properties make this suitable 
here despite it generally not being suitable for what seem like similar issues 
elsewhere in the compiler.

Also, other platforms seem to be OK with this sort of split testing - there's 
lots of testing of intrinsics (mostly in clang/test/CodeGen, rather than 
clang/test/Headers, by the looks of it) which, at a cursory glance, seems to 
generally use emit-llvm+FileCheck, not going all the way to assembly. I don't 
know I've seen much discussion that this has been a problematic gap in testing 
for LLVM targets so far.

(all that said, if it's really needed, there's something that makes it 
fundamentally different from what LLVM's done historically here, or evidence 
that's been problematic/costly in terms of allowing regressions, I don't mind 
them being in the clang test directory - though there's also the 
currently-being-refactored debuginfo-tests directory which will also be for 
higher level more end-to-end tests and these tests might be suitable there... 
though again, be good to understand why the current testing for other targets 
has been inadequate or what makes WebAssembly different here)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101684

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

[PATCH] D70094: [clang-tidy] new altera ID dependent backward branch check

[Frank Derry Wanye via Phabricator via cfe-commits]

ffrankies updated this revision to Diff 342278.
ffrankies added a comment.

Addressed comments by @aaron.ballman

- Changed capitalization in enum
- Used `std::move` in `IdDependencyRecord` constructors
- Initialized `VariableDeclaration` and `FieldDeclaration` to `nullptr`
- Used `isAssignmentOperator()` instead of listing the assingment operators in 
the matchers
- Simplified code around if statement expressions
- Switched to `llvm::Twine` and `llvm::raw_string_ostream` when constructing 
warning and note messages
- Changed `getLoopType()` body to a switch statement instead of a series of 
if-else statements


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

https://reviews.llvm.org/D70094

Files:
  clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp
  clang-tools-extra/clang-tidy/altera/CMakeLists.txt
  clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.cpp
  clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/altera-id-dependent-backward-branch.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/altera-id-dependent-backward-branch.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/altera-id-dependent-backward-branch.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/altera-id-dependent-backward-branch.cpp
@@ -0,0 +1,86 @@
+// RUN: %check_clang_tidy %s altera-id-dependent-backward-branch %t -- -header-filter=.* "--" -cl-std=CL1.2 -c --include opencl-c.h
+
+typedef struct ExampleStruct {
+  int IDDepField;
+} ExampleStruct;
+
+void error() {
+  //  Conditional Expressions 
+  int accumulator = 0;
+  for (int i = 0; i < get_local_id(0); i++) {
+// CHECK-NOTES: :[[@LINE-1]]:19: warning: backward branch (for loop) is ID-dependent due to ID function call and may cause performance degradation [altera-id-dependent-backward-branch]
+accumulator++;
+  }
+
+  int j = 0;
+  while (j < get_local_id(0)) {
+// CHECK-NOTES: :[[@LINE-1]]:10: warning: backward branch (while loop) is ID-dependent due to ID function call and may cause performance degradation [altera-id-dependent-backward-branch]
+accumulator++;
+  }
+
+  do {
+accumulator++;
+  } while (j < get_local_id(0));
+  // CHECK-NOTES: :[[@LINE-1]]:12: warning: backward branch (do loop) is ID-dependent due to ID function call and may cause performance degradation [altera-id-dependent-backward-branch]
+
+  //  Assignments 
+  int ThreadID = get_local_id(0);
+
+  while (j < ThreadID) {
+// CHECK-NOTES: :[[@LINE-3]]:3: note: assignment of ID-dependent variable ThreadID
+// CHECK-NOTES: :[[@LINE-2]]:10: warning: backward branch (while loop) is ID-dependent due to variable reference to 'ThreadID' and may cause performance degradation [altera-id-dependent-backward-branch]
+accumulator++;
+  }
+
+  ExampleStruct Example;
+  Example.IDDepField = get_local_id(0);
+
+  //  Inferred Assignments 
+  int ThreadID2 = ThreadID * get_local_size(0);
+
+  int ThreadID3 = Example.IDDepField; // OK: not used in any loops
+
+  ExampleStruct UnusedStruct = {
+  ThreadID * 2 // OK: not used in any loops
+  };
+
+  for (int i = 0; i < ThreadID2; i++) {
+// CHECK-NOTES: :[[@LINE-9]]:3: note: inferred assignment of ID-dependent value from ID-dependent variable ThreadID
+// CHECK-NOTES: :[[@LINE-2]]:19: warning: backward branch (for loop) is ID-dependent due to variable reference to 'ThreadID2' and may cause performance degradation [altera-id-dependent-backward-branch]
+accumulator++;
+  }
+
+  do {
+accumulator++;
+  } while (j < ThreadID);
+  // CHECK-NOTES: :[[@LINE-29]]:3: note: assignment of ID-dependent variable ThreadID
+  // CHECK-NOTES: :[[@LINE-2]]:12: warning: backward branch (do loop) is ID-dependent due to variable reference to 'ThreadID' and may cause performance degradation [altera-id-dependent-backward-branch]
+
+  for (int i = 0; i < Example.IDDepField; i++) {
+// CHECK-NOTES: :[[@LINE-24]]:3: note: assignment of ID-dependent field IDDepField
+// CHECK-NOTES: :[[@LINE-2]]:19: warning: backward branch (for loop) is ID-dependent due to member reference to 'IDDepField' and may cause performance degradation [altera-id-dependent-backward-branch]
+accumulator++;
+  }
+
+  while (j < Example.IDDepField) {
+// CHECK-NOTES: :[[@LINE-30]]:3: note: assignment of ID-dependent field IDDepField
+// CHECK-NOTES: :[[@LINE-2]]:10: warning: backward branch (while loop) is ID-dependent due to member reference to 'IDDepField' and may cause performance degradation [altera-id-dependent-backward-branch]
+accumulator++;
+  }
+
+  do {
+accumulator++;
+  } while (j < Example.IDDepField);
+  // CHECK-NOTES: :[[@LINE-38]]:3: note: assignment of ID-dependent field IDDepField
+  // CHECK-NOTES: :[[@LINE-2]]:12: warning: backward

[PATCH] D101684: [WebAssembly] Add end-to-end codegen tests for wasm_simd128.h

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

In D101684#2732395 , @dblaikie wrote:

> In D101684#2732366 , @tlively wrote:
>
>> In order to get the benefit of this end-to-end test from split tests like 
>> that, the LLVM test would have to be automatically generated from the clang 
>> test.
>
> Why is that? We don't do that for other test surface area between clang and 
> LLVM.

The question this test answers is "Do the intrinsic functions generate the 
proper WebAssembly instructions?"  (Notably, the test reveals that in multiple 
cases, they don't). If we had separate C->IR and and IR->Wasm tests, they would 
be able to answer this question only if we were sure that the output of the C 
test matched the source of the IR test, and generating the IR test from the C 
test would be the best way to ensure that.

I understand your point that clang tests typically do not try to answer this 
kind of question, but this is an important question to be able to answer for 
the folks working on WebAssembly SIMD. So the options I see are:

1. Have this abnormal end-to-end test in clang.
2. Autogenerate an IR test from the C test so the composition of tests tells us 
what we want to know.
3. Host the test in some other repository.

Among those, the first is both the easiest to maintain and the most useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101684

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

[PATCH] D101684: [WebAssembly] Add end-to-end codegen tests for wasm_simd128.h

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D101684#2732366 , @tlively wrote:

> In D101684#2732310 , @dblaikie 
> wrote:
>
>>> Assembly tests are generally discouraged in clang, but in this case we 
>>> actually care about the specific instruction being generated from the 
>>> intrinsics.
>>
>> I don't think this is a sound reason to add an end-to-end test in clang. The 
>> same is true of all clang tests, right? We ultimately care that accessing a 
>> parameter lowers to a certain register (because we're trying to implement a 
>> certain ABI) but we don't test that in clang - we test that we lower to 
>> certain IR which is guaranteed to lower to a certain register use - and then 
>> in LLVM we test that that IR does lower to that register.
>>
>> I think the same holds true here - and a clang test should verify the IR and 
>> an LLVM test should verify the assembly.
>
> In order to get the benefit of this end-to-end test from split tests like 
> that, the LLVM test would have to be automatically generated from the clang 
> test.

Why is that? We don't do that for other test surface area between clang and 
LLVM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101684

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

[PATCH] D101684: [WebAssembly] Add end-to-end codegen tests for wasm_simd128.h

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

In D101684#2732310 , @dblaikie wrote:

>> Assembly tests are generally discouraged in clang, but in this case we 
>> actually care about the specific instruction being generated from the 
>> intrinsics.
>
> I don't think this is a sound reason to add an end-to-end test in clang. The 
> same is true of all clang tests, right? We ultimately care that accessing a 
> parameter lowers to a certain register (because we're trying to implement a 
> certain ABI) but we don't test that in clang - we test that we lower to 
> certain IR which is guaranteed to lower to a certain register use - and then 
> in LLVM we test that that IR does lower to that register.
>
> I think the same holds true here - and a clang test should verify the IR and 
> an LLVM test should verify the assembly.

In order to get the benefit of this end-to-end test from split tests like that, 
the LLVM test would have to be automatically generated from the clang test. 
This wouldn't be so bad to do as long as the LLVM test also used autogenerated 
checks, but overall that would be extra complexity, verbosity, and indirection 
for no additional testing benefit, especially given that clang and LLVM are now 
in the same monorepo and can trivially be kept in sync. Do you think that extra 
complexity is worth it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101684

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

[PATCH] D101696: [Matrix] Implement C-style explicit type conversions in CXX for matrix types

[Saurabh Jha via Phabricator via cfe-commits]

SaurabhJha added a comment.

All comments addressed now :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101696

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

[PATCH] D101696: [Matrix] Implement C-style explicit type conversions in CXX for matrix types

[Saurabh Jha via Phabricator via cfe-commits]

SaurabhJha updated this revision to Diff 342266.
SaurabhJha added a comment.

Revert change of sext -> zext


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101696

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/test/CodeGen/matrix-cast.c
  clang/test/CodeGenCXX/matrix-casts.cpp
  clang/test/SemaCXX/matrix-casts.cpp

Index: clang/test/SemaCXX/matrix-casts.cpp
===
--- clang/test/SemaCXX/matrix-casts.cpp
+++ clang/test/SemaCXX/matrix-casts.cpp
@@ -14,7 +14,6 @@
 typedef int vec __attribute__((vector_size(4)));
 
 void f1() {
-  // TODO: Update this test once the support of C-style casts for C++ is implemented.
   matrix_4_4 m1;
   matrix_4_4 m2;
   matrix_4_4 m3;
@@ -23,45 +22,46 @@
   vec v;
   test_struct *s;
 
-  (matrix_4_4)m1;   // expected-error {{C-style cast from 'matrix_4_4' (aka 'char __attribute__((matrix_type(4, \
-4)))') to 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, 4)))') is not allowed}}
-  (matrix_4_4)m2; // expected-error {{C-style cast from 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, \
-4)))') to 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, 4)))') is not allowed}}
-  (matrix_5_5)m3;   // expected-error {{C-style cast from 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, \
-4)))') to 'matrix_5_5' (aka 'int __attribute__((matrix_type(5, 5)))') is not allowed}}
+  m2 = (matrix_4_4)m1;
+  m2 = m1; // expected-error {{assigning to 'matrix_4_4' from incompatible type 'matrix_4_4'}}
+  m3 = (matrix_4_4)m2;
+  (matrix_5_5)m3; // expected-error {{conversion between matrix types 'matrix_5_5' (aka 'int __attribute__\
+((matrix_type(5, 5)))') and 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, 4)))') of different size is not\
+ allowed}}
 
-  (int)m3;// expected-error {{C-style cast from 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, \
-4)))') to 'int'}}
-  (matrix_4_4)i; // expected-error {{C-style cast from 'int' to 'matrix_4_4' (aka 'int __attribute__((\
-matrix_type(4, 4)))') is not allowed}}
+  (int)m3;// expected-error {{conversion between matrix type 'matrix_4_4' (aka 'short __attribute__\
+((matrix_type(4, 4)))') and incompatible type 'int' is not allowed}}
+  (matrix_4_4)i; // expected-error {{conversion between matrix type 'matrix_4_4' (aka 'int __attribute__\
+((matrix_type(4, 4)))') and incompatible type 'int' is not allowed}}
 
-  (vec) m2;// expected-error {{C-style cast from 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, 4)))') \
-to 'vec' (vector of 1 'int' value) is not allowed}}
-  (matrix_4_4)v; // expected-error {{C-style cast from 'vec' (vector of 1 'int' value) to 'matrix_4_4' \
-(aka 'char __attribute__((matrix_type(4, 4)))') is not allowed}}
+  (vec) m2;// expected-error {{conversion between matrix type 'matrix_4_4' (aka 'int __attribute__\
+((matrix_type(4, 4)))') and incompatible type 'vec' (vector of 1 'int' value) is not allowed}}
+  (matrix_4_4)v; // expected-error {{conversion between matrix type 'matrix_4_4' (aka 'char __attribute__\
+((matrix_type(4, 4)))') and incompatible type 'vec' (vector of 1 'int' value) is not allowed}}
 
-  (test_struct *)m1;// expected-error {{cannot cast from type 'matrix_4_4' (aka 'char __attribute__\
-((matrix_type(4, 4)))') to pointer type 'test_struct *}}'
-  (matrix_5_5)s; // expected-error {{C-style cast from 'test_struct *' to 'matrix_5_5' (aka 'float __attribute__\
-((matrix_type(5, 5)))') is not allowed}}'
+  (test_struct *)m1;// expected-error {{conversion between matrix type 'matrix_4_4' (aka 'char __attribute__\
+((matrix_type(4, 4)))') and incompatible type 'test_struct *' is not allowed}}'
+  (matrix_5_5)s; // expected-error {{conversion between matrix type 'matrix_5_5' (aka 'float __attribute__\
+((matrix_type(5, 5)))') and incompatible type 'test_struct *' is not allowed}}'
 }
 
 void f2() {
-  // TODO: Update this test once the support of C-style casts for C++ is implemented.
   matrix_4_4 m1;
-  matrix_5_5 m2;
-  matrix_5_5 m3;
-  matrix_4_4 m4;
+  matrix_4_4 m2;
+  matrix_5_5 m3;
+  matrix_5_5 m4;
+  matrix_4_4 m5;
+  matrix_5_5 m6;
   float f;
 
-  (matrix_4_4)m1;   // expected-error {{C-style cast from 'matrix_4_4' (aka 'float __attribute__\
-((matrix_type(4, 4)))') to 'matrix_4_4' (aka 'double __attribute__((matrix_type(4, 4)))') is not allowed}}
-  (matrix_5_5)m2;// expected-error {{C-style cast from 'matrix_5_5' (aka 'double __attribute__\
-((matrix_type(5, 5)))') to 'matrix_5_5' (aka 'float __attribute__((matrix_type(5, 5)))') is not allowed}}
-  (matrix_5_5)m3; // expected-error {{C-style cast from 'matrix_5_5' (aka 'int __attribute__\
-((matrix_type(5, 5)))') to 'matrix_5_5' (aka 'unsigned int __attribute__((matrix_type(5, 5)))') \
-is not allowed}}
-  (matrix_4_4)m4;  // expected-error {{C-style cast from '

[PATCH] D101561: [Prototype] Introduce attribute for ignoring C++ ABI

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

ah I wasn't aware of that, thanks! sorry for the extra noise


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101561

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

[PATCH] D101684: [WebAssembly] Add end-to-end codegen tests for wasm_simd128.h

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

> Assembly tests are generally discouraged in clang, but in this case we 
> actually care about the specific instruction being generated from the 
> intrinsics.

I don't think this is a sound reason to add an end-to-end test in clang. The 
same is true of all clang tests, right? We ultimately care that accessing a 
parameter lowers to a certain register (because we're trying to implement a 
certain ABI) but we don't test that in clang - we test that we lower to certain 
IR which is guaranteed to lower to a certain register use - and then in LLVM we 
test that that IR does lower to that register.

I think the same holds true here - and a clang test should verify the IR and an 
LLVM test should verify the assembly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101684

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

[PATCH] D101671: Modules: Remove an extra early return, NFC

[David Blaikie via Phabricator via cfe-commits]

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

Looks good to me


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101671

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

[PATCH] D101696: [Matrix] Implement C-style explicit type conversions in CXX for matrix types

[Saurabh Jha via Phabricator via cfe-commits]

SaurabhJha added inline comments.



Comment at: clang/test/CodeGenCXX/matrix-casts.cpp:26
+  // CHECK:   [[C:%.*]] = load <25 x i8>, <25 x i8>* {{.*}}, align 1
+  // CHECK-NEXT:  [[CONV:%.*]] = sext <25 x i8> [[C]] to <25 x i32>
+  // CHECK-NEXT:  [[CONV1:%.*]] = bitcast [25 x i32]* {{.*}} to <25 x i32>*

fhahn wrote:
> SaurabhJha wrote:
> > fhahn wrote:
> > > Shouldn't this use `zext` for the conversion? Possibly that's an issue 
> > > with the existing conversion code for matrixes?
> > Would definitely debug it but would like to quickly clarify this.
> > 
> > [[ 
> > https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGExprScalar.cpp#L1225-L1236
> >  | This ]] is the code that's being executed. For int->int conversion, we 
> > are just checking the whether input is signed and regardless of whether 
> > output type is signed, we do `CreateIntCast`. We probably need to check 
> > `OutputSigned` too, right?
> Nevermind, the scalar version I checked used `zext` due to some AArch64 
> specific optimization! So `sext` should be fine here. But that still leaves 
> the int->float case. https://clang.godbolt.org/z/a5Tjed7sP
Whoops, did not see your updated comment. Reverting that change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101696

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

[PATCH] D101696: [Matrix] Implement C-style explicit type conversions in CXX for matrix types

[Saurabh Jha via Phabricator via cfe-commits]

SaurabhJha updated this revision to Diff 342262.
SaurabhJha added a comment.

Remove unnecessary newline


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101696

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/test/CodeGen/matrix-cast.c
  clang/test/CodeGenCXX/matrix-casts.cpp
  clang/test/SemaCXX/matrix-casts.cpp

Index: clang/test/SemaCXX/matrix-casts.cpp
===
--- clang/test/SemaCXX/matrix-casts.cpp
+++ clang/test/SemaCXX/matrix-casts.cpp
@@ -14,7 +14,6 @@
 typedef int vec __attribute__((vector_size(4)));
 
 void f1() {
-  // TODO: Update this test once the support of C-style casts for C++ is implemented.
   matrix_4_4 m1;
   matrix_4_4 m2;
   matrix_4_4 m3;
@@ -23,45 +22,46 @@
   vec v;
   test_struct *s;
 
-  (matrix_4_4)m1;   // expected-error {{C-style cast from 'matrix_4_4' (aka 'char __attribute__((matrix_type(4, \
-4)))') to 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, 4)))') is not allowed}}
-  (matrix_4_4)m2; // expected-error {{C-style cast from 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, \
-4)))') to 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, 4)))') is not allowed}}
-  (matrix_5_5)m3;   // expected-error {{C-style cast from 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, \
-4)))') to 'matrix_5_5' (aka 'int __attribute__((matrix_type(5, 5)))') is not allowed}}
+  m2 = (matrix_4_4)m1;
+  m2 = m1; // expected-error {{assigning to 'matrix_4_4' from incompatible type 'matrix_4_4'}}
+  m3 = (matrix_4_4)m2;
+  (matrix_5_5)m3; // expected-error {{conversion between matrix types 'matrix_5_5' (aka 'int __attribute__\
+((matrix_type(5, 5)))') and 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, 4)))') of different size is not\
+ allowed}}
 
-  (int)m3;// expected-error {{C-style cast from 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, \
-4)))') to 'int'}}
-  (matrix_4_4)i; // expected-error {{C-style cast from 'int' to 'matrix_4_4' (aka 'int __attribute__((\
-matrix_type(4, 4)))') is not allowed}}
+  (int)m3;// expected-error {{conversion between matrix type 'matrix_4_4' (aka 'short __attribute__\
+((matrix_type(4, 4)))') and incompatible type 'int' is not allowed}}
+  (matrix_4_4)i; // expected-error {{conversion between matrix type 'matrix_4_4' (aka 'int __attribute__\
+((matrix_type(4, 4)))') and incompatible type 'int' is not allowed}}
 
-  (vec) m2;// expected-error {{C-style cast from 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, 4)))') \
-to 'vec' (vector of 1 'int' value) is not allowed}}
-  (matrix_4_4)v; // expected-error {{C-style cast from 'vec' (vector of 1 'int' value) to 'matrix_4_4' \
-(aka 'char __attribute__((matrix_type(4, 4)))') is not allowed}}
+  (vec) m2;// expected-error {{conversion between matrix type 'matrix_4_4' (aka 'int __attribute__\
+((matrix_type(4, 4)))') and incompatible type 'vec' (vector of 1 'int' value) is not allowed}}
+  (matrix_4_4)v; // expected-error {{conversion between matrix type 'matrix_4_4' (aka 'char __attribute__\
+((matrix_type(4, 4)))') and incompatible type 'vec' (vector of 1 'int' value) is not allowed}}
 
-  (test_struct *)m1;// expected-error {{cannot cast from type 'matrix_4_4' (aka 'char __attribute__\
-((matrix_type(4, 4)))') to pointer type 'test_struct *}}'
-  (matrix_5_5)s; // expected-error {{C-style cast from 'test_struct *' to 'matrix_5_5' (aka 'float __attribute__\
-((matrix_type(5, 5)))') is not allowed}}'
+  (test_struct *)m1;// expected-error {{conversion between matrix type 'matrix_4_4' (aka 'char __attribute__\
+((matrix_type(4, 4)))') and incompatible type 'test_struct *' is not allowed}}'
+  (matrix_5_5)s; // expected-error {{conversion between matrix type 'matrix_5_5' (aka 'float __attribute__\
+((matrix_type(5, 5)))') and incompatible type 'test_struct *' is not allowed}}'
 }
 
 void f2() {
-  // TODO: Update this test once the support of C-style casts for C++ is implemented.
   matrix_4_4 m1;
-  matrix_5_5 m2;
-  matrix_5_5 m3;
-  matrix_4_4 m4;
+  matrix_4_4 m2;
+  matrix_5_5 m3;
+  matrix_5_5 m4;
+  matrix_4_4 m5;
+  matrix_5_5 m6;
   float f;
 
-  (matrix_4_4)m1;   // expected-error {{C-style cast from 'matrix_4_4' (aka 'float __attribute__\
-((matrix_type(4, 4)))') to 'matrix_4_4' (aka 'double __attribute__((matrix_type(4, 4)))') is not allowed}}
-  (matrix_5_5)m2;// expected-error {{C-style cast from 'matrix_5_5' (aka 'double __attribute__\
-((matrix_type(5, 5)))') to 'matrix_5_5' (aka 'float __attribute__((matrix_type(5, 5)))') is not allowed}}
-  (matrix_5_5)m3; // expected-error {{C-style cast from 'matrix_5_5' (aka 'int __attribute__\
-((matrix_type(5, 5)))') to 'matrix_5_5' (aka 'unsigned int __attribute__((matrix_type(5, 5)))') \
-is not allowed}}
-  (matrix_4_4)m4;  // expected-error {{C-style cast from 'mat

[PATCH] D101561: [Prototype] Introduce attribute for ignoring C++ ABI

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D101561#2730201 , @aeubanks wrote:

> I'm first testing out if there are even noticeable benefits to doing this, 
> then if there are I'll put something together to post to cfe-dev. 
> https://crbug.com/1028731 for some background. I'm aware of trivial_abi and 
> will consider wrapping this up into that, although we may want to separate 
> the two.
>
> (I didn't mean to add you as a reviewer yet, Phab automatically did that, you 
> can ignore this for now if you'd like)

FWIW, if you weren't ready for this to be seen by others/reviewed, you can make 
a draft review (I haven't tested - maybe this is still /accessible/ so you can 
share a link in an RFC, etc) - discussed here: 
https://reviews.llvm.org/D99650#2671915


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101561

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

[PATCH] D101696: [Matrix] Implement C-style explicit type conversions in CXX for matrix types

[Saurabh Jha via Phabricator via cfe-commits]

SaurabhJha added a comment.

Thanks for your review @fhahn . I have made some changes in CGExprScalar.cpp to 
address your comments on IR. I have also added a statement to test assignment 
to incorrect type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101696

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

[PATCH] D101696: [Matrix] Implement C-style explicit type conversions in CXX for matrix types

[Saurabh Jha via Phabricator via cfe-commits]

SaurabhJha updated this revision to Diff 342261.
SaurabhJha added a comment.

Updating D101696 : [Matrix] Implement C-style 
explicit type conversions in CXX for matrix types

1. Remove bitcast when types are of the same size.
2. Make int conversion depend on whether both input and output are signed.
3. Add a test case of assigning to an incorrect type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101696

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/test/CodeGen/matrix-cast.c
  clang/test/CodeGenCXX/matrix-casts.cpp
  clang/test/SemaCXX/matrix-casts.cpp

Index: clang/test/SemaCXX/matrix-casts.cpp
===
--- clang/test/SemaCXX/matrix-casts.cpp
+++ clang/test/SemaCXX/matrix-casts.cpp
@@ -14,7 +14,6 @@
 typedef int vec __attribute__((vector_size(4)));
 
 void f1() {
-  // TODO: Update this test once the support of C-style casts for C++ is implemented.
   matrix_4_4 m1;
   matrix_4_4 m2;
   matrix_4_4 m3;
@@ -23,45 +22,46 @@
   vec v;
   test_struct *s;
 
-  (matrix_4_4)m1;   // expected-error {{C-style cast from 'matrix_4_4' (aka 'char __attribute__((matrix_type(4, \
-4)))') to 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, 4)))') is not allowed}}
-  (matrix_4_4)m2; // expected-error {{C-style cast from 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, \
-4)))') to 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, 4)))') is not allowed}}
-  (matrix_5_5)m3;   // expected-error {{C-style cast from 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, \
-4)))') to 'matrix_5_5' (aka 'int __attribute__((matrix_type(5, 5)))') is not allowed}}
+  m2 = (matrix_4_4)m1;
+  m2 = m1; // expected-error {{assigning to 'matrix_4_4' from incompatible type 'matrix_4_4'}}
+  m3 = (matrix_4_4)m2;
+  (matrix_5_5)m3; // expected-error {{conversion between matrix types 'matrix_5_5' (aka 'int __attribute__\
+((matrix_type(5, 5)))') and 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, 4)))') of different size is not\
+ allowed}}
 
-  (int)m3;// expected-error {{C-style cast from 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, \
-4)))') to 'int'}}
-  (matrix_4_4)i; // expected-error {{C-style cast from 'int' to 'matrix_4_4' (aka 'int __attribute__((\
-matrix_type(4, 4)))') is not allowed}}
+  (int)m3;// expected-error {{conversion between matrix type 'matrix_4_4' (aka 'short __attribute__\
+((matrix_type(4, 4)))') and incompatible type 'int' is not allowed}}
+  (matrix_4_4)i; // expected-error {{conversion between matrix type 'matrix_4_4' (aka 'int __attribute__\
+((matrix_type(4, 4)))') and incompatible type 'int' is not allowed}}
 
-  (vec) m2;// expected-error {{C-style cast from 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, 4)))') \
-to 'vec' (vector of 1 'int' value) is not allowed}}
-  (matrix_4_4)v; // expected-error {{C-style cast from 'vec' (vector of 1 'int' value) to 'matrix_4_4' \
-(aka 'char __attribute__((matrix_type(4, 4)))') is not allowed}}
+  (vec) m2;// expected-error {{conversion between matrix type 'matrix_4_4' (aka 'int __attribute__\
+((matrix_type(4, 4)))') and incompatible type 'vec' (vector of 1 'int' value) is not allowed}}
+  (matrix_4_4)v; // expected-error {{conversion between matrix type 'matrix_4_4' (aka 'char __attribute__\
+((matrix_type(4, 4)))') and incompatible type 'vec' (vector of 1 'int' value) is not allowed}}
 
-  (test_struct *)m1;// expected-error {{cannot cast from type 'matrix_4_4' (aka 'char __attribute__\
-((matrix_type(4, 4)))') to pointer type 'test_struct *}}'
-  (matrix_5_5)s; // expected-error {{C-style cast from 'test_struct *' to 'matrix_5_5' (aka 'float __attribute__\
-((matrix_type(5, 5)))') is not allowed}}'
+  (test_struct *)m1;// expected-error {{conversion between matrix type 'matrix_4_4' (aka 'char __attribute__\
+((matrix_type(4, 4)))') and incompatible type 'test_struct *' is not allowed}}'
+  (matrix_5_5)s; // expected-error {{conversion between matrix type 'matrix_5_5' (aka 'float __attribute__\
+((matrix_type(5, 5)))') and incompatible type 'test_struct *' is not allowed}}'
 }
 
 void f2() {
-  // TODO: Update this test once the support of C-style casts for C++ is implemented.
   matrix_4_4 m1;
-  matrix_5_5 m2;
-  matrix_5_5 m3;
-  matrix_4_4 m4;
+  matrix_4_4 m2;
+  matrix_5_5 m3;
+  matrix_5_5 m4;
+  matrix_4_4 m5;
+  matrix_5_5 m6;
   float f;
 
-  (matrix_4_4)m1;   // expected-error {{C-style cast from 'matrix_4_4' (aka 'float __attribute__\
-((matrix_type(4, 4)))') to 'matrix_4_4' (aka 'double __attribute__((matrix_type(4, 4)))') is not allowed}}
-  (matrix_5_5)m2;// expected-error {{C-style cast from 'matrix_5_5' (aka 'double __attribute__\
-((matrix_type(5, 5)))') to 'matrix_5_5' (aka 'float __attribute__((matrix_type(5, 5)))') is not allowed

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

>> When I create the new RFC patch, I'll try to get details on that and include 
>> it in the description.

Great! Thanks.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63423

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

[PATCH] D101700: [RISCV] Reorder masked builtin operands. Use clang_builtin_alias for all overloaded vector builtins.

[Craig Topper via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcfe3b0005f6e: [RISCV] Reorder masked builtin operands. Use 
clang_builtin_alias for all… (authored by craig.topper).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101700

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vaadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamoadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamoand.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamomax.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamomin.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamoor.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamoswap.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamoxor.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vand.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vasub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vcompress.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vdiv.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfabs.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfclass.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfcvt.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfdiv.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfirst.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmax.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmerge.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmin.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfmul.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfncvt.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfneg.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfnmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfnmadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfnmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfnmsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfrdiv.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfrec7.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfredmax.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfredmin.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfredsum.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfrsqrt7.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfrsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfsgnj.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfslide1down.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfslide1up.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfsqrt.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwcvt.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwmul.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwnmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwnmsac.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwredsum.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vfwsub.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vid.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/viota.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vle.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vloxei.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vlse.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vluxei.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmacc.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmax.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmerge.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmfeq.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmfge.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmfgt.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmfle.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmflt.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmfne.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmin.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmsbf.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmseq.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmsge.c
  clang

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

In D63423#2732209 , @xbolva00 wrote:

>>> Perhaps that should warn even if the RHS is in hex form
>
> It would be kinda strange, since in one clang release we ask users to silence 
> warning with hex form and newer release would warn anyway. Not a fan of this 
> decision.

Fair point, but the clang suggestion is always to turn the LHS into hex form, 
never to turn the RHS in hex form, so if users followed clang's suggested 
silencing, it would continue to work. That's how I didn't even notice that I 
could change the RHS to hex form in the case I asked about.

And fully agreed that if we warn about any cases where we didn't warn before, 
we need to be very careful about the risk of false positives. When I create the 
new RFC patch, I'll try to get details on that and include it in the 
description.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63423

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

[PATCH] D101236: [ASTImporter] Import definitions required for layout of [[no_unique_address]] from LLDB

[Jan Kratochvil via Phabricator via cfe-commits]

jankratochvil added a comment.

In D101236#2716317 , @martong wrote:

> You can create a similar descendant class, but with the MinimalImport flag 
> set to true. Then you could call `ImportDefinition` subsequently after an 
> `Import` call. Perhaps that could trigger your assertion.

Do you have another hint or should I try harder? Thanks.

  clang/lib/Basic/SourceManager.cpp:865: clang::FileID 
clang::SourceManager::getFileIDLoaded(unsigned int) const: Assertion `0 && 
"Invalid SLocOffset or bad function choice"' failed.
  * thread #1, name = 'ASTTests', stop reason = hit program assert
  frame #4: 0x028d7195 
ASTTests`clang::SourceManager::getFileIDLoaded(this=0x04ecf640, 
SLocOffset=14405) const at SourceManager.cpp:865:5
 862  FileID SourceManager::getFileIDLoaded(unsigned SLocOffset) const {
 863// Sanity checking, otherwise a bug may lead to hanging in release 
build.
 864if (SLocOffset < CurrentLoadedOffset) {
  -> 865  assert(0 && "Invalid SLocOffset or bad function choice");
 866  return FileID();
 867}
 868
  (lldb) p SLocOffset
  (unsigned int) $0 = 14405
  (lldb) p CurrentLoadedOffset
  (unsigned int) $1 = 2147483648
  (lldb) bt
  frame # 3: libc.so.6`__assert_fail + 70
* frame # 4: 
ASTTests`clang::SourceManager::getFileIDLoaded(this=0x04ecf640, 
SLocOffset=14405) const at SourceManager.cpp:865:5
  frame # 5: 
ASTTests`clang::SourceManager::getFileIDSlow(this=0x04ecf640, 
SLocOffset=14405) const at SourceManager.cpp:773:10
  frame # 6: 
ASTTests`clang::SourceManager::getFileID(clang::SourceLocation) const at 
SourceManager.h:1107:12
  frame # 7: 
ASTTests`clang::SourceManager::getDecomposedExpansionLoc(this=0x04ecf640,
 Loc=(ID = 14405)) const at SourceManager.h:1247:18
  frame # 8: 
ASTTests`clang::SourceManager::getPresumedLoc(this=0x04ecf640, Loc=(ID 
= 14405), UseLineDirectives=true) const at SourceManager.cpp:1521:41
  frame # 9: 
ASTTests`clang::SourceManager::isWrittenInBuiltinFile(this=0x04ecf640, 
Loc=(ID = 14405)) const at SourceManager.h:1468:24
  frame #10: ASTTests`clang::ASTImporter::Import(this=0x778f9010, 
FromLoc=(ID = 14405)) at ASTImporter.cpp:8834:27
  frame #11: ASTTests`llvm::Error 
clang::ASTImporter::importInto(this=0x778f9010, 
To=0x7fffbe68, From=0x7fffb8e8) at ASTImporter.h:336:22
  frame #12: ASTTests`llvm::Error 
clang::ASTNodeImporter::importInto(this=0x7fffbf60,
 To=0x7fffbe68, From=0x7fffb8e8) at ASTImporter.cpp:148:23
  frame #13: 
ASTTests`clang::ASTNodeImporter::ImportDeclParts(this=0x7fffbf60, 
D=0x04eb8190, DC=0x7fffbe80, LexicalDC=0x7fffbe78, 
Name=0x7fffbe70, ToD=0x7fffbe60, Loc=0x7fffbe68) at 
ASTImporter.cpp:1654:19
  frame #14: 
ASTTests`clang::ASTNodeImporter::VisitRecordDecl(this=0x7fffbf60, 
D=0x04eb8190) at ASTImporter.cpp:2772:19
  frame #15: ASTTests`clang::declvisitor::Base 
>::VisitCXXRecordDecl(this=0x7fffbf60, D=0x04eb8190) at 
DeclNodes.inc:263:1
  frame #16: ASTTests`clang::declvisitor::Base 
>::Visit(this=0x7fffbf60, D=0x04eb8190) at DeclNodes.inc:263:1
  frame #17: 
ASTTests`clang::ASTImporter::ImportImpl(this=0x778f9010, 
FromD=0x04eb8190) at ASTImporter.cpp:8216:19
  frame #18: ASTTests`clang::ASTImporter::Import(this=0x778f9010, 
FromD=0x04eb8190) at ASTImporter.cpp:8387:27
  frame #19: ASTTests`llvm::Expected 
clang::ASTNodeImporter::import(this=0x7fffc6c8, 
From=0x04eb8190) at ASTImporter.cpp:164:31
  frame #20: 
ASTTests`clang::ASTNodeImporter::ImportDeclContext(this=0x7fffc6c8, 
FromDC=0x04e886a8, ForceImport=true) at ASTImporter.cpp:1777:34
  frame #21: 
ASTTests`clang::ASTImporter::ImportDefinition(this=0x778f9010, 
From=0x04e88668) at ASTImporter.cpp:9072:19
  frame #22: 
ASTTests`clang::ast_matchers::LLDBMinimalImport_LLDBImportNoUniqueAddress_Test::TestBody()
 at ASTImporterTest.cpp:6408:50


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101236

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

[PATCH] D101561: [Prototype] Introduce attribute for ignoring C++ ABI

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D101561#2730201 , @aeubanks wrote:

> I'm first testing out if there are even noticeable benefits to doing this, 
> then if there are I'll put something together to post to cfe-dev. 
> https://crbug.com/1028731 for some background. I'm aware of trivial_abi and 
> will consider wrapping this up into that, although we may want to separate 
> the two.
>
> (I didn't mean to add you as a reviewer yet, Phab automatically did that, you 
> can ignore this for now if you'd like)

Ah, thanks for the info!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101561

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

[PATCH] D101236: [ASTImporter] Import definitions required for layout of [[no_unique_address]] from LLDB

[Jan Kratochvil via Phabricator via cfe-commits]

jankratochvil updated this revision to Diff 342249.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101236

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -6323,6 +6323,105 @@
 ToD->getTemplateSpecializationKind());
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportNoUniqueAddress) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  struct A {};
+  struct B {
+[[no_unique_address]] A a;
+  };
+  struct C : B {
+char c;
+  } c;
+  )", Lang_CXX20);
+
+  auto *BFromD = FirstDeclMatcher().match(
+  FromTU, cxxRecordDecl(hasName("B")));
+  ASSERT_TRUE(BFromD);
+  auto *BToD = Import(BFromD, Lang_CXX20);
+  EXPECT_TRUE(BToD);
+
+  auto *CFromD = FirstDeclMatcher().match(
+  FromTU, cxxRecordDecl(hasName("C")));
+  ASSERT_TRUE(CFromD);
+  auto *CToD = Import(CFromD, Lang_CXX20);
+  EXPECT_TRUE(CToD);
+
+  QualType CFType = FromTU->getASTContext().getRecordType(CFromD);
+  TypeInfo TFI = FromTU->getASTContext().getTypeInfo(CFType);
+  ASSERT_EQ(TFI.Width, 8U);
+  QualType CTType = FromTU->getASTContext().getRecordType(CToD);
+  TypeInfo TTI = FromTU->getASTContext().getTypeInfo(CTType);
+  // Here we test the NoUniqueAddressAttr import.
+  // But the Record definition part is not tested here.
+  ASSERT_EQ(TTI.Width, TFI.Width);
+}
+
+struct LLDBMinimalImport : ASTImporterOptionSpecificTestBase {
+  LLDBMinimalImport() {
+Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
+ ASTContext &FromContext, FileManager &FromFileManager,
+ bool MinimalImport,
+ const std::shared_ptr &SharedState) {
+  return new ASTImporter(ToContext, ToFileManager, FromContext,
+ FromFileManager, true /*MinimalImport*/,
+ // We use the regular lookup.
+ /*SharedState=*/nullptr);
+};
+  }
+};
+
+TEST_P(LLDBMinimalImport, LLDBImportNoUniqueAddress) {
+  TranslationUnitDecl *ToTU = getToTuDecl(
+  R"(
+  struct A {};
+  struct B {
+[[no_unique_address]] A a;
+  };
+  struct C : B {
+char c;
+  } c;
+  )", Lang_CXX20);
+  auto *ToC = FirstDeclMatcher().match(
+  ToTU, cxxRecordDecl(hasName("C")));
+
+  // Set up a stub external storage.
+  ToTU->setHasExternalLexicalStorage(true);
+  // Set up DeclContextBits.HasLazyExternalLexicalLookups to true.
+  ToTU->setMustBuildLookupTable();
+  struct TestExternalASTSource : ExternalASTSource {};
+  ToTU->getASTContext().setExternalSource(new TestExternalASTSource());
+
+  Decl *FromTU = getTuDecl(
+  R"(
+struct C;
+  )",
+  Lang_CXX20);
+  auto *FromC = FirstDeclMatcher().match(
+  FromTU, cxxRecordDecl(hasName("C")));
+  auto *ImportedC = Import(FromC, Lang_CXX20);
+  // The lookup must find the existing class definition in the LinkageSpecDecl.
+  // Then the importer renders the existing and the new decl into one chain.
+  EXPECT_EQ(ImportedC->getCanonicalDecl(), ToC->getCanonicalDecl());
+
+  // Import the definition of the created class.
+  llvm::Error Err = findFromTU(FromC)->Importer->ImportDefinition(ToC);
+  EXPECT_FALSE((bool)Err);
+  consumeError(std::move(Err));
+
+  QualType CTType = ToTU->getASTContext().getRecordType(ToC);
+  TypeInfo TTI = ToTU->getASTContext().getTypeInfo(CTType);
+  fprintf(stderr,"LLDB To Width=%lu\n",TTI.Width);
+#if 0 // getASTRecordLayout(const clang::RecordDecl *) const: Assertion `D && "Cannot get layout of forward declarations!"' failed.
+  QualType CFType = FromTU->getASTContext().getRecordType(FromC);
+  TypeInfo TFI = FromTU->getASTContext().getTypeInfo(CFType);
+  fprintf(stderr,"LLDB From Width=%lu\n",TFI.Width);
+#endif
+  QualType CIType = FromTU->getASTContext().getRecordType(ImportedC);
+  TypeInfo TII = FromTU->getASTContext().getTypeInfo(CIType);
+  fprintf(stderr,"LLDB Imported Width=%lu\n",TII.Width);
+}
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
 DefaultTestValuesForRunOptions, );
 
@@ -6397,5 +6496,8 @@
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportWithExternalSource,
 DefaultTestValuesForRunOptions, );
 
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, LLDBMinimalImport,
+DefaultTestValuesForRunOptions, );
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -3673,6 +3673,30 @@
   D->getInClassInitStyle()))
 return ToField;
 
+  /

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

>> Perhaps that should warn even if the RHS is in hex form

It would be kinda strange, since in one clang release we ask users to silence 
warning with hex form and newer release would warn anyway. Not a fan of this 
decision.

>> , or is an enumerator constant, or

This looks like a good idea. +1.

>> is not even constant at all.

Depends, needs to be carefully evaluated (true positives vs false positives 
ratio).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63423

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

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

In D63423#2732199 , @xbolva00 wrote:

> No, we want to preserve warning for 2 ^ MACRO or 10 ^ MACRO.

Perhaps that should warn even if the RHS is in hex form, or is an enumerator 
constant, or is not even constant at all.

libpng: #if-ed out but just waiting for someone to copy and paste: `for 
(i=11;i>=0;--i){ print i, " ", (1 - e(-(2^i)/65536*l(2))) * 2^(32-i), "\n"}`.

https://github.com/mongodb/mongo/blob/master/src/third_party/IntelRDFPMathLib20U1/LIBRARY/float128/mphoc_macros.h:

  #define MPHOC_MAX_CHAR  ((2 ^ (BITS_PER_CHAR - 1)) - 1)
  #define MPHOC_MAX_SHORT ((2 ^ (BITS_PER_SHORT - 1)) - 1)
  #define MPHOC_MAX_INT   ((2 ^ (BITS_PER_INT - 1)) - 1)
  #define MPHOC_MAX_LONG  ((2 ^ (BITS_PER_LONG - 1)) - 1)
  #define MPHOC_MAX_WORD  ((2 ^ (BITS_PER_WORD - 1)) - 1)

(and many more in there)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63423

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

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

No, we want to preserve warning for 2 ^ MACRO or 10 ^ MACRO.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63423

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

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

In D63423#2732186 , @xbolva00 wrote:

> I remember that was interest to support macros too :) tbh I cant remember now 
> such real world case where “macro ^ cst” was an issue but.. it was a long 
> time ago ;)
>
> If you are want, you can send patch to to avoid warning in this case, we can 
> discuss it.

Will do; the change is trivial after D97445 : 
it's just changing that to use `||` rather than `&&`. I'll do that and update 
the tests to match. I don't know whether that'll be what we want, but it may be 
easier to talk about once we have a good picture from the test case update what 
it affects.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63423

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

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

I remember that was interest to support macros too :) tbh I cant remember now 
such real world case where “macro ^ cst” was an issue but.. it was a long time 
ago ;)

If you are want, you can send patch to to avoid warning in this case, we can 
discuss it.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63423

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

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

In D63423#2732158 , @Quuxplusone wrote:

> In D63423#2732152 , @hvdijk wrote:
>
>> It's bad enough that this warns for
>>
>> #define A 2
>> int f() { return A ^ 1; }
>>
>> where as far as the users of A are concerned...
>
> I see how this warning is arguably overzealous in the //very special case// 
> of "raising" to the constant `1` (since nobody would ever write that by 
> accident). However, if the user of A wants to indicate that they understand 
> this is bitwise-xor, they can simply change the body of their `f` to `return 
> A ^ 0x1;` — hex notation suppresses the warning.  (Changing the definition of 
> `A` as well is perhaps a good idea but not technically required.)  IMO this 
> is good enough and we should leave it.  (What do you think, having seen the 
> `A ^ 0x1` workaround? Does that sufficiently address your needs?)

I missed that hex notation for the RHS also suppresses the warning. Thanks, 
that's good to know. Combined with the fact that the case where LHS and RHS are 
both macros now no longer triggers the warning, that means one of LHS or RHS 
must be an integer literal, so there will always be a viable workaround once 
clang 13 is released. That may be good enough.

>> [...] I'm not seeing from the previous discussion whether this case was 
>> based on real world programmer errors or not
>
> The description links to a couple of tweets showing examples from the wild:
> https://twitter.com/jfbastien/status/1139298419988549632
> https://twitter.com/mikemx7f/status/1139335901790625793

Those are different cases, they literally have `2 ^` in there. In the case I 
was asking about, the `2` comes from a macro expansion, but the `^` does not. 
That is much less likely to be a mistake, and one where I didn't see whether 
the warning for that was based on real-world concerns.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63423

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

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

In D63423#2732152 , @hvdijk wrote:

> It's bad enough that this warns for
>
> #define A 2
> int f() { return A ^ 1; }
>
> where as far as the users of A are concerned...

I see how this warning is arguably overzealous in the //very special case// of 
"raising" to the constant `1` (since nobody would ever write that by accident). 
However, if the user of A wants to indicate that they understand this is 
bitwise-xor, they can simply change the body of their `f` to `return A ^ 0x1;` 
— hex notation suppresses the warning.  (Changing the definition of `A` as well 
is perhaps a good idea but not technically required.)  IMO this is good enough 
and we should leave it.  (What do you think, having seen the `A ^ 0x1` 
workaround? Does that sufficiently address your needs?)

> [...] I'm not seeing from the previous discussion whether this case was based 
> on real world programmer errors or not

The description links to a couple of tweets showing examples from the wild:
https://twitter.com/jfbastien/status/1139298419988549632
https://twitter.com/mikemx7f/status/1139335901790625793


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63423

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

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk added a comment.

It's bad enough that this warns for

#define A 2
#define B 0
int f() { return A ^ B; }

where as far as the users of A are concerned, A is just some arbitrary value, 
it's just random chance it happens to be two and there is no chance of it being 
misinterpreted as exponentiation, but it's much worse that this cannot be 
worked around by changing the definition of A to 0x2, the only way to suppress 
the warning with a hexadecimal constant is by explicitly using that constant in 
the use, i.e. by changing the function to int f() { return 0x2 ^ B; }, which is 
very much inappropriate when the users of A are not supposed to care about 
which specific value it has.

I'd like to see this changed so that at least the #define A 0x2 case no longer 
triggers the warning, but ideally I'd like to see no warning for this example 
in its current state either. I see in the test case that that warning is 
intentional, but I'm not seeing from the previous discussion whether this case 
was based on real world programmer errors or not. Do you recall? If it was, 
then fair enough, I'll propose to leave that as it is.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63423

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

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:124-126
   DiagnosticBuilder
   configurationDiag(StringRef Description,
 DiagnosticIDs::Level Level = DiagnosticIDs::Warning);

njames93 wrote:
> You don't need a ClangTidyContext, this function is all that's needed.
But the `getFileStyleFromOptions()` is a static function in 
`IdentifierNamingCheck.cpp` file. It can't access `configurationDiag()` 
directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

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

[PATCH] D101696: [Matrix] Implement C-style explicit type conversions in CXX for matrix types

[Florian Hahn via Phabricator via cfe-commits]

fhahn added inline comments.



Comment at: clang/test/CodeGenCXX/matrix-casts.cpp:26
+  // CHECK:   [[C:%.*]] = load <25 x i8>, <25 x i8>* {{.*}}, align 1
+  // CHECK-NEXT:  [[CONV:%.*]] = sext <25 x i8> [[C]] to <25 x i32>
+  // CHECK-NEXT:  [[CONV1:%.*]] = bitcast [25 x i32]* {{.*}} to <25 x i32>*

SaurabhJha wrote:
> fhahn wrote:
> > Shouldn't this use `zext` for the conversion? Possibly that's an issue with 
> > the existing conversion code for matrixes?
> Would definitely debug it but would like to quickly clarify this.
> 
> [[ 
> https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGExprScalar.cpp#L1225-L1236
>  | This ]] is the code that's being executed. For int->int conversion, we are 
> just checking the whether input is signed and regardless of whether output 
> type is signed, we do `CreateIntCast`. We probably need to check 
> `OutputSigned` too, right?
Nevermind, the scalar version I checked used `zext` due to some AArch64 
specific optimization! So `sext` should be fine here. But that still leaves the 
int->float case. https://clang.godbolt.org/z/a5Tjed7sP


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101696

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

[PATCH] D101696: [Matrix] Implement C-style explicit type conversions in CXX for matrix types

[Saurabh Jha via Phabricator via cfe-commits]

SaurabhJha added inline comments.



Comment at: clang/test/CodeGenCXX/matrix-casts.cpp:26
+  // CHECK:   [[C:%.*]] = load <25 x i8>, <25 x i8>* {{.*}}, align 1
+  // CHECK-NEXT:  [[CONV:%.*]] = sext <25 x i8> [[C]] to <25 x i32>
+  // CHECK-NEXT:  [[CONV1:%.*]] = bitcast [25 x i32]* {{.*}} to <25 x i32>*

fhahn wrote:
> Shouldn't this use `zext` for the conversion? Possibly that's an issue with 
> the existing conversion code for matrixes?
Would definitely debug it but would like to quickly clarify this.

[[ 
https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGExprScalar.cpp#L1225-L1236
 | This ]] is the code that's being executed. For int->int conversion, we are 
just checking the whether input is signed and regardless of whether output type 
is signed, we do `CreateIntCast`. We probably need to check `OutputSigned` too, 
right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101696

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

[PATCH] D101700: [RISCV] Reorder masked builtin operands. Use clang_builtin_alias for all overloaded vector builtins.

[Zakk Chen via Phabricator via cfe-commits]

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

LGTM. Thanks for improvement!




Comment at: clang/include/clang/Basic/riscv_vector.td:192
 
-  // When the order of the parameters of clang builtin do not match the order 
of
-  // C/C++ api, we use permutation index to mapping the operand from clang

craig.topper wrote:
> I remove this and added a couple ManualCodeGen blocks to permute builtin to 
> IR intrinsics operand order to make up for it. I could have used 
> PermuteOperands but then we would have had to translate it into C code to do 
> the permutation in CGBuiltin.cpp.
Cool, using ManualCodeGen is more simple.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101700

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

[PATCH] D101696: [Matrix] Implement C-style explicit type conversions in CXX for matrix types

[Florian Hahn via Phabricator via cfe-commits]

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

Thanks for the patch!

It looks like there might be a few cases where the wrong conversion is used at 
the moment.




Comment at: clang/test/CodeGenCXX/matrix-casts.cpp:26
+  // CHECK:   [[C:%.*]] = load <25 x i8>, <25 x i8>* {{.*}}, align 1
+  // CHECK-NEXT:  [[CONV:%.*]] = sext <25 x i8> [[C]] to <25 x i32>
+  // CHECK-NEXT:  [[CONV1:%.*]] = bitcast [25 x i32]* {{.*}} to <25 x i32>*

Shouldn't this use `zext` for the conversion? Possibly that's an issue with the 
existing conversion code for matrixes?



Comment at: clang/test/CodeGenCXX/matrix-casts.cpp:66
+  // CHECK-NEXT:  [[CONV]] = bitcast <25 x i32> %1 to <25 x float>
+  // CHECK-NEXT:  [[CONV1]] = bitcast [25 x float]* {{.*}} to <25 x float>*
+  // CHECK-NEXT:  store <25 x float> [[CONV]], <25 x float>* [[CONV1]], align 4

Shouldn't this use `sitofp` for the conversion? Possibly that's an issue with 
the existing conversion code for matrixes?



Comment at: clang/test/SemaCXX/matrix-casts.cpp:25
 
-  (matrix_4_4)m1;   // expected-error {{C-style cast from 
'matrix_4_4' (aka 'char __attribute__((matrix_type(4, \
-4)))') to 'matrix_4_4' (aka 'int __attribute__((matrix_type(4, 4)))') is 
not allowed}}
-  (matrix_4_4)m2; // expected-error {{C-style cast from 
'matrix_4_4' (aka 'int __attribute__((matrix_type(4, \
-4)))') to 'matrix_4_4' (aka 'short __attribute__((matrix_type(4, 4)))') 
is not allowed}}
-  (matrix_5_5)m3;   // expected-error {{C-style cast from 
'matrix_4_4' (aka 'short __attribute__((matrix_type(4, \
-4)))') to 'matrix_5_5' (aka 'int __attribute__((matrix_type(5, 5)))') is 
not allowed}}
+  m2 = (matrix_4_4)m1;
+  m3 = (matrix_4_4)m2;

can you also add a test with an assignment without cast? This should be an 
error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101696

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

[PATCH] D101721: [clang-tidy][NFC] Update tests and Default options to use boolean value

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added a reviewer: aaron.ballman.
Herald added subscribers: kbarton, xazax.hun, nemanjai.
njames93 requested review of this revision.
Herald added subscribers: cfe-commits, aheejin.
Herald added a project: clang-tools-extra.

Change instances where options which are boolean are assigned the value 1|0 to 
use true|false instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101721

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment-ignore-single-argument.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment-literals.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-argument-comment-strict.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-implicit-widening-of-multiplication-result-array-subscript-expression.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-implicit-widening-of-multiplication-result-int.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-implicit-widening-of-multiplication-result-pointer-offset.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-misplaced-widening-cast-explicit-only.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-misplaced-widening-cast-implicit-enabled.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-not-null-terminated-result-memcpy-before-safe.c
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier-invert.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-enum-usage-strict.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-enum-usage.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-string-compare.c
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-string-compare.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-unhandled-self-assignment-warn-only-if-this-has-suspicious-field.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage-caps-only.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-macro-usage-command-line-macros.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-pedanticmode-option.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-modernize-use-default-member-init-assignment.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init-use-assignment.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-allow-missing-move-when-copy-is-deleted.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-special-member-functions-relaxed.cpp
  
clang-tools-extra/test/clang-tidy/checkers/hicpp-multiway-paths-covered-else.cpp
  
clang-tools-extra/test/clang-tidy/checkers/hicpp-signed-bitwise-integer-literals.cpp
  
clang-tools-extra/test/clang-tidy/checkers/misc-non-private-member-variables-in-classes.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters-strict.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-avoid-bind-permissive-parameter-list.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique-macros.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-raw-string-literal-delimiter.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-raw-string-literal.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-auto-cast-remove-stars.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-auto-min-type-name-length.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-auto-new-remove-stars.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-bool-literals-ignore-macros.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-bool-literals.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init-assignment.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init-macros.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-emplace-ignore-implicit-constructors.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-macros.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-delete-macros.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-noexcept-opt.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-override-allow-override-and-final.cpp
  
clang-tools-extra/test/clang-tid

[PATCH] D101239: [clang-tidy] Enable the use of IgnoreArray flag in pro-type-member-init rule

[Hana Joo via Phabricator via cfe-commits]

h-joo updated this revision to Diff 342233.
h-joo added a comment.

Change `1` to `true` in the test flag as requested by the reviewer.


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

https://reviews.llvm.org/D101239

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.ignorearrays.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.ignorearrays.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.ignorearrays.cpp
@@ -0,0 +1,11 @@
+// RUN: %check_clang_tidy %s \
+// RUN: cppcoreguidelines-pro-type-member-init %t \
+// RUN: -config="{CheckOptions: \
+// RUN: [{key: cppcoreguidelines-pro-type-member-init.IgnoreArrays, value: 
true} ]}"
+
+struct HasArrayMember {
+  HasArrayMember() {}
+  // CHECK-MESSAGES: warning: constructor does not initialize these fields: 
Number
+  int Array[4];
+  int Number;
+};
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -402,6 +402,8 @@
   // Gather all fields (direct and indirect) that need to be initialized.
   SmallPtrSet FieldsToInit;
   forEachField(ClassDecl, ClassDecl.fields(), [&](const FieldDecl *F) {
+if (IgnoreArrays && F->getType()->isArrayType())
+  return;
 if (!F->hasInClassInitializer() &&
 utils::type_traits::isTriviallyDefaultConstructible(F->getType(),
 Context) &&


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.ignorearrays.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.ignorearrays.cpp
@@ -0,0 +1,11 @@
+// RUN: %check_clang_tidy %s \
+// RUN: cppcoreguidelines-pro-type-member-init %t \
+// RUN: -config="{CheckOptions: \
+// RUN: [{key: cppcoreguidelines-pro-type-member-init.IgnoreArrays, value: true} ]}"
+
+struct HasArrayMember {
+  HasArrayMember() {}
+  // CHECK-MESSAGES: warning: constructor does not initialize these fields: Number
+  int Array[4];
+  int Number;
+};
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -402,6 +402,8 @@
   // Gather all fields (direct and indirect) that need to be initialized.
   SmallPtrSet FieldsToInit;
   forEachField(ClassDecl, ClassDecl.fields(), [&](const FieldDecl *F) {
+if (IgnoreArrays && F->getType()->isArrayType())
+  return;
 if (!F->hasInClassInitializer() &&
 utils::type_traits::isTriviallyDefaultConstructible(F->getType(),
 Context) &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added a comment.

(BTW, if the patch is reviewed, then I believe we are finally ready to land 
this patch.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101191

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

[PATCH] D101191: [InstCombine] Fully disable select to and/or i1 folding

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added a comment.

I made D101720  to cover the remaining cases 
except `Transforms/InstCombine/and2.ll`.
Supporting `and2.ll` doesn't seem super-straightforward, but maybe some minor 
tweaks can make it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101191

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

[PATCH] D101239: [clang-tidy] Enable the use of IgnoreArray flag in pro-type-member-init rule

[Nathan James via Phabricator via cfe-commits]

njames93 added reviewers: aaron.ballman, njames93.
njames93 added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.ignorearrays.cpp:4
+// RUN: -config="{CheckOptions: \
+// RUN: [{key: cppcoreguidelines-pro-type-member-init.IgnoreArrays, value: 1} 
]}"
+

Small nit. 


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

https://reviews.llvm.org/D101239

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

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:124-126
   DiagnosticBuilder
   configurationDiag(StringRef Description,
 DiagnosticIDs::Level Level = DiagnosticIDs::Warning);

You don't need a ClangTidyContext, this function is all that's needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

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

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added a comment.

Hi @njames93,

That's ok. You have helped me a lots. Thank you.




Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:450
+if (!isHungarianNotationSupportedStyle(I) && HPTOpt.hasValue())
+  Options.diagnoseInvalidConfigOption(StyleString);
+StyleString.resize(StyleSize);

njames93 wrote:
> 
To use configurationDiag() function in getFileStyleFromOptions(), seems it 
needs to pass a ClangTidyContext object to getFileStyleFromOptions(). How about 
changing like the following?

```
getFileStyleFromOptions(const ClangTidyCheck::OptionsView &Options,
ClangTidyContext &Context) {
...

if (!isHungarianNotationSupportedStyle(I) && HPTOpt.hasValue())
  Context.configurationDiag("invalid identifier naming option '%0'")
  << StyleString;

...
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

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