[PATCH] D96690: [clangd] Treat paths case-insensitively depending on the platform

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Path{Match,Exclude} and MountPoint were checking paths case-sensitively
on all platforms, as with other features, this was causing problems on
windows. Since users can have capital drive letters on config files, but
editors might lower-case them.

This patch addresses that issue by:

- Normalizing all the paths being passed to ConfigProvider in ClangdServer.
- Storing MountPoint and regexes used for pathmatching in case-folded manner.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96690

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -99,6 +99,19 @@
   Frag.If.PathMatch.emplace_back("ba*r");
   EXPECT_FALSE(compileAndApply());
   EXPECT_THAT(Diags.Diagnostics, IsEmpty());
+
+#if defined(_WIN32) || defined(__APPLE__)
+  // Only matches case-insensitively.
+  Frag = {};
+  Frag.If.PathMatch.emplace_back("B*R");
+  EXPECT_TRUE(compileAndApply());
+  EXPECT_THAT(Diags.Diagnostics, IsEmpty());
+
+  Frag = {};
+  Frag.If.PathExclude.emplace_back("B*R");
+  EXPECT_FALSE(compileAndApply());
+  EXPECT_THAT(Diags.Diagnostics, IsEmpty());
+#endif
 }
 
 TEST_F(ConfigCompileTests, CompileCommands) {
@@ -372,14 +385,14 @@
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
   ASSERT_TRUE(Conf.Index.External);
-  EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+  EXPECT_TRUE(pathEqual(Conf.Index.External->MountPoint, FooPath));
 
   // None defaults to ".".
   Frag = GetFrag(FooPath, llvm::None);
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
   ASSERT_TRUE(Conf.Index.External);
-  EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+  EXPECT_TRUE(pathEqual(Conf.Index.External->MountPoint, FooPath));
 
   // Without a file, external index is empty.
   Parm.Path = "";
@@ -405,7 +418,21 @@
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
   ASSERT_TRUE(Conf.Index.External);
-  EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+  EXPECT_TRUE(pathEqual(Conf.Index.External->MountPoint, FooPath));
+
+#if defined(_WIN32) || defined(__APPLE__)
+  // Only matches case-insensitively.
+  BazPath = testPath("fOo/baz.h", llvm::sys::path::Style::posix);
+  BazPath = llvm::sys::path::convert_to_slash(BazPath);
+  Parm.Path = BazPath;
+
+  FooPath = testPath("FOO/", llvm::sys::path::Style::posix);
+  Frag = GetFrag("", FooPath.c_str());
+  compileAndApply();
+  ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+  ASSERT_TRUE(Conf.Index.External);
+  EXPECT_TRUE(pathEqual(Conf.Index.External->MountPoint, FooPath));
+#endif
 }
 } // namespace
 } // namespace config
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -31,6 +31,7 @@
 #include "Features.inc"
 #include "TidyProvider.h"
 #include "support/Logger.h"
+#include "support/Path.h"
 #include "support/Trace.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
@@ -101,9 +102,11 @@
   // Normalized Fragment::SourceInfo::Directory.
   std::string FragmentDirectory;
 
-  llvm::Optional compileRegex(const Located &Text) {
+  llvm::Optional
+  compileRegex(const Located &Text,
+   llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags) {
 std::string Anchored = "^(" + *Text + ")$";
-llvm::Regex Result(Anchored);
+llvm::Regex Result(Anchored, Flags);
 std::string RegexError;
 if (!Result.isValid(RegexError)) {
   diag(Error, "Invalid regex " + Anchored + ": " + RegexError, Text.Range);
@@ -184,6 +187,7 @@
   FragmentDirectory = llvm::sys::path::convert_to_slash(F.Source.Directory);
   if (FragmentDirectory.back() != '/')
 FragmentDirectory += '/';
+  FragmentDirectory = maybeCaseFoldPath(FragmentDirectory);
 }
 compile(std::move(F.If));
 compile(std::move(F.CompileFlags));
@@ -195,9 +199,17 @@
 if (F.HasUnrecognizedCondition)
   Out.Conditions.push_back([&](const Params &) { return false; });
 
+#if defined(_WIN32) || defined(__APPLE__)
+// When matching paths via regex on Windows/Apple they should be treated
+// case insensitively.
+llvm::Regex::RegexFlags Flags = llvm::Regex::IgnoreCase;
+#else
+llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags;
+#endif
+
 auto PathMatch = std::make_unique>();
 for (auto &Entry : F.PathMatch) {
-  if (auto RE = compileRegex(

[PATCH] D85817: [analyzer] Fix crash with pointer to members values

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D85817#2560570 , @RedDocMD wrote:

> @vsavchenko, why did you chose NamedDecl instead of ValueDecl to account for 
> the indirect fields? I couldn't quite follow it from the discussion above.

Looking at it now, I also don't know why 🤷


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85817

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

[clang] 21daada - [analyzer] Fix static_cast on pointer-to-member handling

[Valeriy Savchenko via cfe-commits]

Author: Deep Majumder
Date: 2021-02-15T11:44:37+03:00
New Revision: 21daada95079a37c7ca259fabfc735b6d1b362ad

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

LOG: [analyzer] Fix static_cast on pointer-to-member handling

This commit fixes bug #48739. The bug was caused by the way static_casts
on pointer-to-member caused the CXXBaseSpecifier list of a
MemberToPointer to grow instead of shrink.
The list is now grown by implicit casts and corresponding entries are
removed by static_casts. No-op static_casts cause no effect.

Reviewed By: vsavchenko

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

Added: 
clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp

Modified: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
clang/test/Analysis/pointer-to-member.cpp

Removed: 




diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
index 142b1ab11750..9f464e82304f 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
@@ -258,9 +258,9 @@ class BasicValueFactory {
 return CXXBaseListFactory.add(CBS, L);
   }
 
-  const PointerToMemberData *accumCXXBase(
-  llvm::iterator_range PathRange,
-  const nonloc::PointerToMember &PTM);
+  const PointerToMemberData *
+  accumCXXBase(llvm::iterator_range PathRange,
+   const nonloc::PointerToMember &PTM, const clang::CastKind 
&kind);
 
   const llvm::APSInt* evalAPSInt(BinaryOperator::Opcode Op,
  const llvm::APSInt& V1,

diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
index bb295ab591d4..a2354ba62cdb 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -514,7 +514,8 @@ class LazyCompoundVal : public NonLoc {
 /// This SVal is represented by a DeclaratorDecl which can be a member function
 /// pointer or a member data pointer and a list of CXXBaseSpecifiers. This list
 /// is required to accumulate the pointer-to-member cast history to figure out
-/// the correct subobject field.
+/// the correct subobject field. In particular, implicit casts grow this list
+/// and explicit casts like static_cast shrink this list.
 class PointerToMember : public NonLoc {
   friend class ento::SValBuilder;
 

diff  --git a/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp 
b/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
index d1f5ac02278f..40cdaef1bfa7 100644
--- a/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -21,6 +21,7 @@
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/ImmutableList.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include 
 #include 
 #include 
@@ -176,28 +177,73 @@ const PointerToMemberData 
*BasicValueFactory::getPointerToMemberData(
   return D;
 }
 
+LLVM_ATTRIBUTE_UNUSED bool hasNoRepeatedElements(
+llvm::ImmutableList BaseSpecList) {
+  llvm::SmallPtrSet BaseSpecSeen;
+  for (const CXXBaseSpecifier *BaseSpec : BaseSpecList) {
+QualType BaseType = BaseSpec->getType();
+// Check whether inserted
+if (!BaseSpecSeen.insert(BaseType).second)
+  return false;
+  }
+  return true;
+}
+
 const PointerToMemberData *BasicValueFactory::accumCXXBase(
 llvm::iterator_range PathRange,
-const nonloc::PointerToMember &PTM) {
+const nonloc::PointerToMember &PTM, const CastKind &kind) {
+  assert((kind == CK_DerivedToBaseMemberPointer ||
+  kind == CK_BaseToDerivedMemberPointer ||
+  kind == CK_ReinterpretMemberPointer) &&
+ "accumCXXBase called with wrong CastKind");
   nonloc::PointerToMember::PTMDataType PTMDT = PTM.getPTMData();
   const NamedDecl *ND = nullptr;
-  llvm::ImmutableList PathList;
+  llvm::ImmutableList BaseSpecList;
 
   if (PTMDT.isNull() || PTMDT.is()) {
 if (PTMDT.is())
   ND = PTMDT.get();
 
-PathList = CXXBaseListFactory.getEmptyList();
-  } else { // const PointerToMemberData *
+BaseSpecList = CXXBaseListFactory.getEmptyList();
+  } else {
 const PointerToMemberData *PTMD = PTMDT.get();
 ND = PTMD->getDeclaratorDecl();
 
-PathList = PTMD->getCXXBaseList();
+BaseSpecList = PTMD->getCXXBaseList();
   }
 
-  for (const auto &I : llvm::reverse(PathRange))
-PathList = pre

[clang] f8d3f47 - [analyzer] Updated comments to reflect D85817

[Valeriy Savchenko via cfe-commits]

Author: Deep Majumder
Date: 2021-02-15T11:47:21+03:00
New Revision: f8d3f47e1fd09392aa30df83849b25acd8c59a25

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

LOG: [analyzer] Updated comments to reflect D85817

Changed DeclaratorDecl in comment to NamedDecl

Reviewed By: vsavchenko

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
index a2354ba62cdb..b1c33713febd 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -511,11 +511,11 @@ class LazyCompoundVal : public NonLoc {
 /// This value is qualified as NonLoc because neither loading nor storing
 /// operations are applied to it. Instead, the analyzer uses the L-value coming
 /// from pointer-to-member applied to an object.
-/// This SVal is represented by a DeclaratorDecl which can be a member function
-/// pointer or a member data pointer and a list of CXXBaseSpecifiers. This list
-/// is required to accumulate the pointer-to-member cast history to figure out
-/// the correct subobject field. In particular, implicit casts grow this list
-/// and explicit casts like static_cast shrink this list.
+/// This SVal is represented by a NamedDecl which can be a member function
+/// pointer or a member data pointer and an optional list of CXXBaseSpecifiers.
+/// This list is required to accumulate the pointer-to-member cast history to
+/// figure out the correct subobject field. In particular, implicit casts grow
+/// this list and explicit casts like static_cast shrink this list.
 class PointerToMember : public NonLoc {
   friend class ento::SValBuilder;
 



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

[PATCH] D95877: [analyzer] Fix static_cast on pointer-to-member handling

[Valeriy Savchenko 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 rG21daada95079: [analyzer] Fix static_cast on 
pointer-to-member handling (authored by RedDocMD, committed by vsavchenko).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95877

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
  clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/test/Analysis/pointer-to-member.cpp
  clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp

Index: clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp
===
--- /dev/null
+++ clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+// XFAIL: *
+
+void clang_analyzer_eval(bool);
+
+// TODO: The following test will work properly once reinterpret_cast on pointer-to-member is handled properly
+namespace testReinterpretCasting {
+struct Base {
+  int field;
+};
+
+struct Derived : public Base {};
+
+struct DoubleDerived : public Derived {};
+
+struct Some {};
+
+void f() {
+  int DoubleDerived::*ddf = &Base::field;
+  int Base::*bf = reinterpret_cast(reinterpret_cast(reinterpret_cast(ddf)));
+  int Some::*sf = reinterpret_cast(ddf);
+  Base base;
+  base.field = 13;
+  clang_analyzer_eval(base.*bf == 13); // expected-warning{{TRUE}}
+}
+} // namespace testReinterpretCasting
Index: clang/test/Analysis/pointer-to-member.cpp
===
--- clang/test/Analysis/pointer-to-member.cpp
+++ clang/test/Analysis/pointer-to-member.cpp
@@ -287,3 +287,26 @@
   clang_analyzer_eval(a.*ep == 5); // expected-warning{{TRUE}}
 }
 } // namespace testAnonymousMember
+
+namespace testStaticCasting {
+// From bug #48739
+struct Grandfather {
+  int field;
+};
+
+struct Father : public Grandfather {};
+struct Son : public Father {};
+
+void test() {
+  int Son::*sf = &Son::field;
+  Grandfather grandpa;
+  grandpa.field = 10;
+  int Grandfather::*gpf1 = static_cast(sf);
+  int Grandfather::*gpf2 = static_cast(static_cast(sf));
+  int Grandfather::*gpf3 = static_cast(static_cast(static_cast(sf)));
+  clang_analyzer_eval(grandpa.*gpf1 == 10); // expected-warning{{TRUE}}
+  clang_analyzer_eval(grandpa.*gpf2 == 10); // expected-warning{{TRUE}}
+  clang_analyzer_eval(grandpa.*gpf3 == 10); // expected-warning{{TRUE}}
+}
+} // namespace testStaticCasting
+
Index: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -526,10 +526,9 @@
   case CK_ReinterpretMemberPointer: {
 SVal V = state->getSVal(Ex, LCtx);
 if (auto PTMSV = V.getAs()) {
-  SVal CastedPTMSV = svalBuilder.makePointerToMember(
-  getBasicVals().accumCXXBase(
-  llvm::make_range(
-  CastE->path_begin(), CastE->path_end()), *PTMSV));
+  SVal CastedPTMSV =
+  svalBuilder.makePointerToMember(getBasicVals().accumCXXBase(
+  CastE->path(), *PTMSV, CastE->getCastKind()));
   state = state->BindExpr(CastE, LCtx, CastedPTMSV);
   Bldr.generateNode(CastE, Pred, state);
   continue;
Index: clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
===
--- clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -21,6 +21,7 @@
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/ImmutableList.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include 
 #include 
 #include 
@@ -176,28 +177,73 @@
   return D;
 }
 
+LLVM_ATTRIBUTE_UNUSED bool hasNoRepeatedElements(
+llvm::ImmutableList BaseSpecList) {
+  llvm::SmallPtrSet BaseSpecSeen;
+  for (const CXXBaseSpecifier *BaseSpec : BaseSpecList) {
+QualType BaseType = BaseSpec->getType();
+// Check whether inserted
+if (!BaseSpecSeen.insert(BaseType).second)
+  return false;
+  }
+  return true;
+}
+
 const PointerToMemberData *BasicValueFactory::accumCXXBase(
 llvm::iterator_range PathRange,
-const nonloc::PointerToMember &PTM) {
+const nonloc::PointerToMember &PTM, const CastKind &kind) {
+  assert((kind == CK_DerivedToBaseMemberPointer ||
+  kind == CK_BaseToDerivedMemberPointer ||
+  kind == CK_ReinterpretMemberPointer) &&
+ "accumCXXBase called with wrong CastKind");
   nonloc::PointerToMember::PTMDataType PTMDT = PTM.getPTMData();
   const NamedDecl *ND = nullptr;
-  llvm::Immu

[PATCH] D95846: [analyzer] Updated comments to reflect D85817

[Valeriy Savchenko 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 rGf8d3f47e1fd0: [analyzer] Updated comments to reflect D85817 
(authored by RedDocMD, committed by vsavchenko).

Changed prior to commit:
  https://reviews.llvm.org/D95846?vs=320667&id=323678#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95846

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h


Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -511,11 +511,11 @@
 /// This value is qualified as NonLoc because neither loading nor storing
 /// operations are applied to it. Instead, the analyzer uses the L-value coming
 /// from pointer-to-member applied to an object.
-/// This SVal is represented by a DeclaratorDecl which can be a member function
-/// pointer or a member data pointer and a list of CXXBaseSpecifiers. This list
-/// is required to accumulate the pointer-to-member cast history to figure out
-/// the correct subobject field. In particular, implicit casts grow this list
-/// and explicit casts like static_cast shrink this list.
+/// This SVal is represented by a NamedDecl which can be a member function
+/// pointer or a member data pointer and an optional list of CXXBaseSpecifiers.
+/// This list is required to accumulate the pointer-to-member cast history to
+/// figure out the correct subobject field. In particular, implicit casts grow
+/// this list and explicit casts like static_cast shrink this list.
 class PointerToMember : public NonLoc {
   friend class ento::SValBuilder;
 


Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -511,11 +511,11 @@
 /// This value is qualified as NonLoc because neither loading nor storing
 /// operations are applied to it. Instead, the analyzer uses the L-value coming
 /// from pointer-to-member applied to an object.
-/// This SVal is represented by a DeclaratorDecl which can be a member function
-/// pointer or a member data pointer and a list of CXXBaseSpecifiers. This list
-/// is required to accumulate the pointer-to-member cast history to figure out
-/// the correct subobject field. In particular, implicit casts grow this list
-/// and explicit casts like static_cast shrink this list.
+/// This SVal is represented by a NamedDecl which can be a member function
+/// pointer or a member data pointer and an optional list of CXXBaseSpecifiers.
+/// This list is required to accumulate the pointer-to-member cast history to
+/// figure out the correct subobject field. In particular, implicit casts grow
+/// this list and explicit casts like static_cast shrink this list.
 class PointerToMember : public NonLoc {
   friend class ento::SValBuilder;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D96607: [clang-tidy] Add check 'readability-pointer-type-star-placement'.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

I think now that not checkers are the best solution for this problem. There is 
already a separate tool for formatting. And we do not want to have a checker 
for every other formatting rule too. The `clang-format` could be improved to 
support selective formatting (apply only a part of the formatting rules). (And 
it has weaknesses that may prevent users from using it.) It seems difficult to 
filter out changes related to pointer declarations (these rules are not 
line-based and multiplications should be excluded) after the formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96607

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

[PATCH] D96524: [OpenCL] Add support of OpenCL C 3.0 __opencl_c_fp64

[Anton Zabaznov via Phabricator via cfe-commits]

azabaznov added inline comments.



Comment at: clang/include/clang/Basic/OpenCLOptions.h:157
+  // Is OpenCL C feature (OpenCL C 3.0, 6.2.1. Features)
+  bool isFeature(llvm::StringRef Ext) const;
+

svenvh wrote:
> The argument "Ext" suggests "Extension", so perhaps rename if this is about 
> features? (also in the definition in the .cpp file)
Sure, thanks. I was going to rename 'extensions' to other concept which will 
represent extensions and features, but this will be done in a separate  commit



Comment at: clang/lib/Basic/OpenCLOptions.cpp:24
+  auto CLVer = LO.OpenCLCPlusPlus ? 200 : LO.OpenCLVersion;
+  return CLVer >= 300 ? isEnabled("__opencl_c_fp64") : 
isEnabled("cl_khr_fp64");
+}

Anastasia wrote:
> We should at least be checking for `isSupported("__opencl_c_fp64")`, but 
> frankly I would prefer to check for supported and not for enabled for 
> `cl_khr_fp64` too. Note that we don't break backward compatibility if we 
> change this because the existing kernels will still be valid and it makes 
> things easier for writing new kernels too.
I think everything fine with this for now because 
`OpenCLOptions::enableSupportedCore` is called to set all supported core or 
optional core features to enabled state. So you suggest to removing this method 
at all too?

I think with your approach things will be unobvious in code: for some 
extensions there will be check for `isSupported` for some other there will be 
check for `isEnabled`. I think we should stay consistent here and check 
availability of all options in the same manner.



Comment at: clang/lib/Basic/TargetInfo.cpp:395
+
+// Set extensions simultaneosly with correspoding features
+// for OpenCL C 3.0 and higher

svenvh wrote:
> simultaneosly -> simultaneously
> correspoding -> corresponding
Sure, thanks



Comment at: clang/lib/Basic/TargetInfo.cpp:398
+auto CLVer = Opts.OpenCLCPlusPlus ? 200 : Opts.OpenCLVersion;
+if (CLVer >= 300) {
+  auto &OCLOpts = getSupportedOpenCLOpts();

Anastasia wrote:
> I suggest we move this onto `OpenCLOptions::addSupport`.
This should stay here to control simultaneous macro definitions



Comment at: clang/lib/Headers/opencl-c.h:4635
 
-#ifdef cl_khr_fp64
+#if defined(cl_khr_fp64) || defined(__opencl_c_fp64)
 #pragma OPENCL EXTENSION cl_khr_fp64 : enable

Anastasia wrote:
> svenvh wrote:
> > Wondering if we need a similar change in 
> > `clang/lib/Headers/opencl-c-base.h` to guard the double vector types?
> Instead of growing the guard condition, I suggest we only check for one for 
> example the feature macro and then make sure the feature macro is defined in 
> `opencl-c-base.h` if the extensions that aliases to the feature is supported. 
> However, it would also be ok to do the opposite since the extensions and the 
> corresponding features should both be available.
> 
> FYI, something similar was done in https://reviews.llvm.org/D92004.
> 
> This will also help to propagate the functionality into Tablegen header 
> because we won't need to extend it to work with multiple extensions but we 
> might still need to do the rename.
Yeah, I overlooked this place... Thanks!



Comment at: clang/lib/Headers/opencl-c.h:4635
 
-#ifdef cl_khr_fp64
+#if defined(cl_khr_fp64) || defined(__opencl_c_fp64)
 #pragma OPENCL EXTENSION cl_khr_fp64 : enable

azabaznov wrote:
> Anastasia wrote:
> > svenvh wrote:
> > > Wondering if we need a similar change in 
> > > `clang/lib/Headers/opencl-c-base.h` to guard the double vector types?
> > Instead of growing the guard condition, I suggest we only check for one for 
> > example the feature macro and then make sure the feature macro is defined 
> > in `opencl-c-base.h` if the extensions that aliases to the feature is 
> > supported. However, it would also be ok to do the opposite since the 
> > extensions and the corresponding features should both be available.
> > 
> > FYI, something similar was done in https://reviews.llvm.org/D92004.
> > 
> > This will also help to propagate the functionality into Tablegen header 
> > because we won't need to extend it to work with multiple extensions but we 
> > might still need to do the rename.
> Yeah, I overlooked this place... Thanks!
I don't think that growing of this condition will hurt in some cases... Note, 
that unifying condition check makes sense if some features are unconditionally 
supported for OpenCL C 2.0, such as generic address space for example. In other 
cases (such as fp64, 3d image writes, subgroups) we should use OR in guard 
condition.  

Your approach will require extra checks in clang such as, for example, to make 
sure that extension macro is not predefined if the feature macro is defined, 
because there will be redefinition since extension macro is defined in header.



Comment at: clang/lib/Se

[PATCH] D96544: [clangd] Extract binding of typed->untyped LSP handlers to LSPBinder. NFC

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

LGTM, thanks! (I wish outgoing calls were not so different :/)




Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:173
 } else {
-  log("unhandled notification {0}", Method);
+  auto Handler = Server.Handlers.NotificationHandlers.find(Method);
+  if (Handler != Server.Handlers.NotificationHandlers.end()) {

why not keep the old lookup style ? since handlers are unique_functions, 
checking for null should still suffice (and be cheap)



Comment at: clang-tools-extra/clangd/LSPBinder.h:67
+  /// e.g. Bind.command("load", this, &ThisModule::load);
+  /// Handler should be e.g. void peek(const LoadParams&, 
Callback);
+  /// LoadParams must be JSON-parseable and LoadResult must be serializable.

s/peek/load



Comment at: clang-tools-extra/clangd/unittests/LSPBinderTests.cpp:18
+
+using testing::_;
+using testing::HasSubstr;

please fix



Comment at: clang-tools-extra/clangd/unittests/LSPBinderTests.cpp:20
+using testing::HasSubstr;
+using testing::Pair;
+using testing::UnorderedElementsAre;

please fix



Comment at: clang-tools-extra/clangd/unittests/LSPBinderTests.cpp:25
+struct Foo {
+  int x;
+};

please fix (we usually follow camelCase for json-serializable types in clangd, 
but i don't think it is worth doing in tests)



Comment at: clang-tools-extra/clangd/unittests/LSPBinderTests.cpp:28
+bool fromJSON(const llvm::json::Value &V, Foo &F, llvm::json::Path P) {
+  return fromJSON(V, F.x, P);
+}

nit: s/P/P.field("x")



Comment at: clang-tools-extra/clangd/unittests/LSPBinderTests.cpp:51
+void notify(const Foo &Params) { lastNotify = Params.x; }
+int lastNotify = -1;
+  };

please fix, and maybe have a counter, in addition to last value to check for 
"invalid type" case?



Comment at: clang-tools-extra/clangd/unittests/LSPBinderTests.cpp:69
+  RawPlusOne(1, capture(Reply));
+  EXPECT_THAT_EXPECTED(Reply.getValue(), llvm::HasValue(2));
+  RawPlusOne("foo", capture(Reply));

nit: `ASSERT_THAT(Reply.hasValue())`

here and elsewhere


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96544

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

[PATCH] D96693: [Syntax] Model FunctionDefinition.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
hokein requested review of this revision.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96693

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/include/clang/Tooling/Syntax/Nodes.td
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/lib/Tooling/Syntax/Synthesis.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
  clang/unittests/Tooling/Syntax/SynthesisTest.cpp

Index: clang/unittests/Tooling/Syntax/SynthesisTest.cpp
===
--- clang/unittests/Tooling/Syntax/SynthesisTest.cpp
+++ clang/unittests/Tooling/Syntax/SynthesisTest.cpp
@@ -225,15 +225,14 @@
   // nodes in the copy are `modifiable`.
   EXPECT_TRUE(treeDumpEqual(Copy, R"txt(
 TranslationUnit Detached synthesized
-`-SimpleDeclaration synthesized
+`-FunctionDefinition synthesized
   |-'void' synthesized
-  |-DeclaratorList Declarators synthesized
-  | `-SimpleDeclarator ListElement synthesized
-  |   |-'test' synthesized
-  |   `-ParametersAndQualifiers synthesized
-  | |-'(' OpenParen synthesized
-  | `-')' CloseParen synthesized
-  `-CompoundStatement synthesized
+  |-SimpleDeclarator Declarator synthesized
+  | |-'test' synthesized
+  | `-ParametersAndQualifiers synthesized
+  |   |-'(' OpenParen synthesized
+  |   `-')' CloseParen synthesized
+  `-CompoundStatement BodyStatement synthesized
 |-'{' OpenParen synthesized
 |-IfStatement Statement synthesized
 | |-'if' IntroducerKeyword synthesized
Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -91,39 +91,6 @@
 INSTANTIATE_TEST_CASE_P(SyntaxTreeTests, BuildSyntaxTreeTest,
 testing::ValuesIn(allTestClangConfigs()), );
 
-TEST_P(BuildSyntaxTreeTest, Simple) {
-  EXPECT_TRUE(treeDumpEqual(
-  R"cpp(
-int main() {}
-void foo() {}
-)cpp",
-  R"txt(
-TranslationUnit Detached
-|-SimpleDeclaration
-| |-'int'
-| |-DeclaratorList Declarators
-| | `-SimpleDeclarator ListElement
-| |   |-'main'
-| |   `-ParametersAndQualifiers
-| | |-'(' OpenParen
-| | `-')' CloseParen
-| `-CompoundStatement
-|   |-'{' OpenParen
-|   `-'}' CloseParen
-`-SimpleDeclaration
-  |-'void'
-  |-DeclaratorList Declarators
-  | `-SimpleDeclarator ListElement
-  |   |-'foo'
-  |   `-ParametersAndQualifiers
-  | |-'(' OpenParen
-  | `-')' CloseParen
-  `-CompoundStatement
-|-'{' OpenParen
-`-'}' CloseParen
-)txt"));
-}
-
 TEST_P(BuildSyntaxTreeTest, SimpleVariable) {
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(
@@ -151,36 +118,35 @@
 }
 
 TEST_P(BuildSyntaxTreeTest, SimpleFunction) {
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
-void foo(int a, int b) {}
+[[void foo(int a, int b) {}]]
 )cpp",
-  R"txt(
+  {R"txt(
 TranslationUnit Detached
-`-SimpleDeclaration
+`-FunctionDefinition
   |-'void'
-  |-DeclaratorList Declarators
-  | `-SimpleDeclarator ListElement
-  |   |-'foo'
-  |   `-ParametersAndQualifiers
-  | |-'(' OpenParen
-  | |-ParameterDeclarationList Parameters
-  | | |-SimpleDeclaration ListElement
-  | | | |-'int'
-  | | | `-DeclaratorList Declarators
-  | | |   `-SimpleDeclarator ListElement
-  | | | `-'a'
-  | | |-',' ListDelimiter
-  | | `-SimpleDeclaration ListElement
-  | |   |-'int'
-  | |   `-DeclaratorList Declarators
-  | | `-SimpleDeclarator ListElement
-  | |   `-'b'
-  | `-')' CloseParen
-  `-CompoundStatement
+  |-SimpleDeclarator Declarator
+  | |-'foo'
+  | `-ParametersAndQualifiers
+  |   |-'(' OpenParen
+  |   |-ParameterDeclarationList Parameters
+  |   | |-SimpleDeclaration ListElement
+  |   | | |-'int'
+  |   | | `-DeclaratorList Declarators
+  |   | |   `-SimpleDeclarator ListElement
+  |   | | `-'a'
+  |   | |-',' ListDelimiter
+  |   | `-SimpleDeclaration ListElement
+  |   |   |-'int'
+  |   |   `-DeclaratorList Declarators
+  |   | `-SimpleDeclarator ListElement
+  |   |   `-'b'
+  |   `-')' CloseParen
+  `-CompoundStatement BodyStatement
 |-'{' OpenParen
 `-'}' CloseParen
-)txt"));
+)txt"}));
 }
 
 TEST_P(BuildSyntaxTreeTest, Simple_BackslashInsideToken) {
@@ -457,59 +423,49 @@
 TEST_P(BuildSyntaxTreeTest, Expressions) {
   // expressions should be wrapped in 'ExpressionStatement' when they appear
   // in a statement position.
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
 void test() {
-  test();
-  if (1) test(); else test();
+  [[test();]]
+  [[if (1) test(); else test();]]
 }
 )cpp",
-  R"txt(
-TranslationUnit Detached
-`-SimpleDeclaration
-  |-'void'
-  |-DeclaratorList Declarators
-  | `-SimpleDeclarator ListElement
-  |   |-'test'
-  |  

[PATCH] D96625: [clangd] Allow modules to bind LSP methods/notifications/commands

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, lgtm!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96625

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

[PATCH] D96693: [Syntax] Model FunctionDefinition.

[Haojian Wu via Phabricator via cfe-commits]

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

Fix and simplify the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96693

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/include/clang/Tooling/Syntax/Nodes.td
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/lib/Tooling/Syntax/Synthesis.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
  clang/unittests/Tooling/Syntax/SynthesisTest.cpp

Index: clang/unittests/Tooling/Syntax/SynthesisTest.cpp
===
--- clang/unittests/Tooling/Syntax/SynthesisTest.cpp
+++ clang/unittests/Tooling/Syntax/SynthesisTest.cpp
@@ -225,15 +225,14 @@
   // nodes in the copy are `modifiable`.
   EXPECT_TRUE(treeDumpEqual(Copy, R"txt(
 TranslationUnit Detached synthesized
-`-SimpleDeclaration synthesized
+`-FunctionDefinition synthesized
   |-'void' synthesized
-  |-DeclaratorList Declarators synthesized
-  | `-SimpleDeclarator ListElement synthesized
-  |   |-'test' synthesized
-  |   `-ParametersAndQualifiers synthesized
-  | |-'(' OpenParen synthesized
-  | `-')' CloseParen synthesized
-  `-CompoundStatement synthesized
+  |-SimpleDeclarator Declarator synthesized
+  | |-'test' synthesized
+  | `-ParametersAndQualifiers synthesized
+  |   |-'(' OpenParen synthesized
+  |   `-')' CloseParen synthesized
+  `-CompoundStatement BodyStatement synthesized
 |-'{' OpenParen synthesized
 |-IfStatement Statement synthesized
 | |-'if' IntroducerKeyword synthesized
Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -91,39 +91,6 @@
 INSTANTIATE_TEST_CASE_P(SyntaxTreeTests, BuildSyntaxTreeTest,
 testing::ValuesIn(allTestClangConfigs()), );
 
-TEST_P(BuildSyntaxTreeTest, Simple) {
-  EXPECT_TRUE(treeDumpEqual(
-  R"cpp(
-int main() {}
-void foo() {}
-)cpp",
-  R"txt(
-TranslationUnit Detached
-|-SimpleDeclaration
-| |-'int'
-| |-DeclaratorList Declarators
-| | `-SimpleDeclarator ListElement
-| |   |-'main'
-| |   `-ParametersAndQualifiers
-| | |-'(' OpenParen
-| | `-')' CloseParen
-| `-CompoundStatement
-|   |-'{' OpenParen
-|   `-'}' CloseParen
-`-SimpleDeclaration
-  |-'void'
-  |-DeclaratorList Declarators
-  | `-SimpleDeclarator ListElement
-  |   |-'foo'
-  |   `-ParametersAndQualifiers
-  | |-'(' OpenParen
-  | `-')' CloseParen
-  `-CompoundStatement
-|-'{' OpenParen
-`-'}' CloseParen
-)txt"));
-}
-
 TEST_P(BuildSyntaxTreeTest, SimpleVariable) {
   EXPECT_TRUE(treeDumpEqual(
   R"cpp(
@@ -151,36 +118,35 @@
 }
 
 TEST_P(BuildSyntaxTreeTest, SimpleFunction) {
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
-void foo(int a, int b) {}
+[[void foo(int a, int b) {}]]
 )cpp",
-  R"txt(
+  {R"txt(
 TranslationUnit Detached
-`-SimpleDeclaration
+`-FunctionDefinition
   |-'void'
-  |-DeclaratorList Declarators
-  | `-SimpleDeclarator ListElement
-  |   |-'foo'
-  |   `-ParametersAndQualifiers
-  | |-'(' OpenParen
-  | |-ParameterDeclarationList Parameters
-  | | |-SimpleDeclaration ListElement
-  | | | |-'int'
-  | | | `-DeclaratorList Declarators
-  | | |   `-SimpleDeclarator ListElement
-  | | | `-'a'
-  | | |-',' ListDelimiter
-  | | `-SimpleDeclaration ListElement
-  | |   |-'int'
-  | |   `-DeclaratorList Declarators
-  | | `-SimpleDeclarator ListElement
-  | |   `-'b'
-  | `-')' CloseParen
-  `-CompoundStatement
+  |-SimpleDeclarator Declarator
+  | |-'foo'
+  | `-ParametersAndQualifiers
+  |   |-'(' OpenParen
+  |   |-ParameterDeclarationList Parameters
+  |   | |-SimpleDeclaration ListElement
+  |   | | |-'int'
+  |   | | `-DeclaratorList Declarators
+  |   | |   `-SimpleDeclarator ListElement
+  |   | | `-'a'
+  |   | |-',' ListDelimiter
+  |   | `-SimpleDeclaration ListElement
+  |   |   |-'int'
+  |   |   `-DeclaratorList Declarators
+  |   | `-SimpleDeclarator ListElement
+  |   |   `-'b'
+  |   `-')' CloseParen
+  `-CompoundStatement BodyStatement
 |-'{' OpenParen
 `-'}' CloseParen
-)txt"));
+)txt"}));
 }
 
 TEST_P(BuildSyntaxTreeTest, Simple_BackslashInsideToken) {
@@ -457,59 +423,49 @@
 TEST_P(BuildSyntaxTreeTest, Expressions) {
   // expressions should be wrapped in 'ExpressionStatement' when they appear
   // in a statement position.
-  EXPECT_TRUE(treeDumpEqual(
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
   R"cpp(
 void test() {
-  test();
-  if (1) test(); else test();
+  [[test();]]
+  [[if (1) test(); else test();]]
 }
 )cpp",
-  R"txt(
-TranslationUnit Detached
-`-SimpleDeclaration
-  |-'void'
-  |-DeclaratorList Declarators
-  | `-SimpleDeclarator ListEleme

[clang-tools-extra] 5786f64 - [clangd] Extract binding of typed->untyped LSP handlers to LSPBinder. NFC

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-02-15T10:48:14+01:00
New Revision: 5786f64a4ec806877f39fa0c28dcb90f9492b68c

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

LOG: [clangd] Extract binding of typed->untyped LSP handlers to LSPBinder. NFC

The goal is to allow the LSP bindings of features to be defined outside
the ClangdLSPServer class, turning it into less of a monolith.

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

Added: 
clang-tools-extra/clangd/LSPBinder.h
clang-tools-extra/clangd/unittests/LSPBinderTests.cpp

Modified: 
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/unittests/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 1d8efe6d84b7..52939c0dc035 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -13,6 +13,7 @@
 #include "DraftStore.h"
 #include "DumpAST.h"
 #include "GlobalCompilationDatabase.h"
+#include "LSPBinder.h"
 #include "Protocol.h"
 #include "SemanticHighlighting.h"
 #include "SourceCode.h"
@@ -158,6 +159,8 @@ class ClangdLSPServer::MessageHandler : public 
Transport::MessageHandler {
   MessageHandler(ClangdLSPServer &Server) : Server(Server) {}
 
   bool onNotify(llvm::StringRef Method, llvm::json::Value Params) override {
+trace::Span Tracer(Method, LSPLatency);
+SPAN_ATTACH(Tracer, "Params", Params);
 WithContext HandlerContext(handlerContext());
 log("<-- {0}", Method);
 if (Method == "exit")
@@ -166,12 +169,15 @@ class ClangdLSPServer::MessageHandler : public 
Transport::MessageHandler {
   elog("Notification {0} before initialization", Method);
 } else if (Method == "$/cancelRequest") {
   onCancel(std::move(Params));
-} else if (auto Handler = Notifications.lookup(Method)) {
-  Handler(std::move(Params));
-  Server.maybeExportMemoryProfile();
-  Server.maybeCleanupMemory();
 } else {
-  log("unhandled notification {0}", Method);
+  auto Handler = Server.Handlers.NotificationHandlers.find(Method);
+  if (Handler != Server.Handlers.NotificationHandlers.end()) {
+Handler->second(std::move(Params));
+Server.maybeExportMemoryProfile();
+Server.maybeCleanupMemory();
+  } else {
+log("unhandled notification {0}", Method);
+  }
 }
 return true;
   }
@@ -189,11 +195,15 @@ class ClangdLSPServer::MessageHandler : public 
Transport::MessageHandler {
   elog("Call {0} before initialization.", Method);
   Reply(llvm::make_error("server not initialized",
ErrorCode::ServerNotInitialized));
-} else if (auto Handler = Calls.lookup(Method))
-  Handler(std::move(Params), std::move(Reply));
-else
-  Reply(llvm::make_error("method not found",
-   ErrorCode::MethodNotFound));
+} else {
+  auto Handler = Server.Handlers.MethodHandlers.find(Method);
+  if (Handler != Server.Handlers.MethodHandlers.end()) {
+Handler->second(std::move(Params), std::move(Reply));
+  } else {
+Reply(llvm::make_error("method not found",
+ ErrorCode::MethodNotFound));
+  }
+}
 return true;
   }
 
@@ -236,28 +246,6 @@ class ClangdLSPServer::MessageHandler : public 
Transport::MessageHandler {
 return true;
   }
 
-  // Bind an LSP method name to a call.
-  template 
-  void bind(const char *Method,
-void (ClangdLSPServer::*Handler)(const Param &, Callback)) 
{
-Calls[Method] = [Method, Handler, this](llvm::json::Value RawParams,
-ReplyOnce Reply) {
-  auto P = parse(RawParams, Method, "request");
-  if (!P)
-return Reply(P.takeError());
-  (Server.*Handler)(*P, std::move(Reply));
-};
-  }
-
-  template 
-  void bind(const char *Method,
-void (ClangdLSPServer::*Handler)(Callback)) {
-Calls[Method] = [Handler, this](llvm::json::Value RawParams,
-ReplyOnce Reply) {
-  (Server.*Handler)(std::move(Reply));
-};
-  }
-
   // Bind a reply callback to a request. The callback will be invoked when
   // clangd receives the reply from the LSP client.
   // Return a call id of the request.
@@ -286,27 +274,6 @@ class ClangdLSPServer::MessageHandler : public 
Transport::MessageHandler {
 return ID;
   }
 
-  // Bind an LSP method name to a notification.
-  template 
-  void bind(const char *Method,
-void (ClangdLSPServer::*Handler)(const Param &)) {
-Notifications[Method] = [Method, Handler,
- 

[PATCH] D96544: [clangd] Extract binding of typed->untyped LSP handlers to LSPBinder. NFC

[Sam McCall via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
sammccall marked 7 inline comments as done.
Closed by commit rG5786f64a4ec8: [clangd] Extract binding of typed->untyped 
LSP handlers to LSPBinder. NFC (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D96544?vs=323449&id=323694#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96544

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/LSPBinder.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/LSPBinderTests.cpp

Index: clang-tools-extra/clangd/unittests/LSPBinderTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/LSPBinderTests.cpp
@@ -0,0 +1,100 @@
+//===-- LSPBinderTests.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "LSPBinder.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using testing::HasSubstr;
+using testing::UnorderedElementsAre;
+
+// JSON-serializable type for testing.
+struct Foo {
+  int X;
+};
+bool fromJSON(const llvm::json::Value &V, Foo &F, llvm::json::Path P) {
+  return fromJSON(V, F.X, P.field("X"));
+}
+llvm::json::Value toJSON(const Foo &F) { return F.X; }
+
+// Creates a Callback that writes its received value into an Optional.
+template 
+llvm::unique_function)>
+capture(llvm::Optional> &Out) {
+  Out.reset();
+  return [&Out](llvm::Expected V) { Out.emplace(std::move(V)); };
+}
+
+TEST(LSPBinderTest, IncomingCalls) {
+  LSPBinder::RawHandlers RawHandlers;
+  LSPBinder Binder{RawHandlers};
+  struct Handler {
+void plusOne(const Foo &Params, Callback Reply) {
+  Reply(Foo{Params.X + 1});
+}
+void fail(const Foo &Params, Callback Reply) {
+  Reply(error("X={0}", Params.X));
+}
+void notify(const Foo &Params) {
+  LastNotify = Params.X;
+  ++NotifyCount;
+}
+int LastNotify = -1;
+int NotifyCount = 0;
+  };
+
+  Handler H;
+  Binder.method("plusOne", &H, &Handler::plusOne);
+  Binder.method("fail", &H, &Handler::fail);
+  Binder.notification("notify", &H, &Handler::notify);
+  Binder.command("cmdPlusOne", &H, &Handler::plusOne);
+  ASSERT_THAT(RawHandlers.MethodHandlers.keys(),
+  UnorderedElementsAre("plusOne", "fail"));
+  ASSERT_THAT(RawHandlers.NotificationHandlers.keys(),
+  UnorderedElementsAre("notify"));
+  ASSERT_THAT(RawHandlers.CommandHandlers.keys(),
+  UnorderedElementsAre("cmdPlusOne"));
+  llvm::Optional> Reply;
+
+  auto &RawPlusOne = RawHandlers.MethodHandlers["plusOne"];
+  RawPlusOne(1, capture(Reply));
+  ASSERT_TRUE(Reply.hasValue());
+  EXPECT_THAT_EXPECTED(Reply.getValue(), llvm::HasValue(2));
+  RawPlusOne("foo", capture(Reply));
+  ASSERT_TRUE(Reply.hasValue());
+  EXPECT_THAT_EXPECTED(
+  Reply.getValue(),
+  llvm::FailedWithMessage(
+  HasSubstr("failed to decode plusOne request: expected integer")));
+
+  auto &RawFail = RawHandlers.MethodHandlers["fail"];
+  RawFail(2, capture(Reply));
+  ASSERT_TRUE(Reply.hasValue());
+  EXPECT_THAT_EXPECTED(Reply.getValue(), llvm::FailedWithMessage("X=2"));
+
+  auto &RawNotify = RawHandlers.NotificationHandlers["notify"];
+  RawNotify(42);
+  EXPECT_EQ(H.LastNotify, 42);
+  EXPECT_EQ(H.NotifyCount, 1);
+  RawNotify("hi"); // invalid, will be logged
+  EXPECT_EQ(H.LastNotify, 42);
+  EXPECT_EQ(H.NotifyCount, 1);
+
+  auto &RawCmdPlusOne = RawHandlers.CommandHandlers["cmdPlusOne"];
+  RawCmdPlusOne(1, capture(Reply));
+  ASSERT_TRUE(Reply.hasValue());
+  EXPECT_THAT_EXPECTED(Reply.getValue(), llvm::HasValue(2));
+}
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -70,6 +70,7 @@
   IndexTests.cpp
   JSONTransportTests.cpp
   LoggerTests.cpp
+  LSPBinderTests.cpp
   LSPClient.cpp
   ModulesTests.cpp
   ParsedASTTests.cpp
Index: clang-tools-extra/clangd/LSPBinder.h
===
--- /dev/null
+++ clang-tools-extra/clangd/LSPBinder.h
@@ -0,0 +1,147 @@
+//===--- LSPBinder.h - Tables of LSP handlers *- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache Lice

[PATCH] D77598: Integral template argument suffix and cast printing

[Pratyush Das via Phabricator via cfe-commits]

reikdas updated this revision to Diff 323696.
reikdas added a comment.

Address @rsmith comments.


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

https://reviews.llvm.org/D77598

Files:
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/AST/StmtDataCollectors.td
  clang/include/clang/AST/TemplateBase.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/NestedNameSpecifier.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Analysis/PathDiagnostic.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/Analysis/eval-predefined-exprs.cpp
  clang/test/CXX/lex/lex.literal/lex.ext/p12.cpp
  clang/test/CXX/lex/lex.literal/lex.ext/p13.cpp
  clang/test/SemaCXX/builtin-align-cxx.cpp
  clang/test/SemaCXX/cxx11-ast-print.cpp
  clang/test/SemaCXX/cxx1z-ast-print.cpp
  clang/test/SemaCXX/matrix-type-builtins.cpp
  clang/test/SemaCXX/matrix-type-operators.cpp
  clang/test/SemaTemplate/address_space-dependent.cpp
  clang/test/SemaTemplate/delegating-constructors.cpp
  clang/test/SemaTemplate/matrix-type.cpp
  clang/test/SemaTemplate/temp_arg_enum_printing.cpp
  clang/test/SemaTemplate/temp_arg_nontype.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
  clang/tools/libclang/CIndex.cpp
  
clang/unittests/Tooling/RecursiveASTVisitorTests/TemplateArgumentLocTraverser.cpp

Index: clang/unittests/Tooling/RecursiveASTVisitorTests/TemplateArgumentLocTraverser.cpp
===
--- clang/unittests/Tooling/RecursiveASTVisitorTests/TemplateArgumentLocTraverser.cpp
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/TemplateArgumentLocTraverser.cpp
@@ -20,7 +20,7 @@
 llvm::raw_string_ostream Stream(ArgStr);
 const TemplateArgument &Arg = ArgLoc.getArgument();
 
-Arg.print(Context->getPrintingPolicy(), Stream);
+Arg.print(Context->getPrintingPolicy(), Stream, /*IncludeType*/ true);
 Match(Stream.str(), ArgLoc.getLocation());
 return ExpectedLocationVisitor::
   TraverseTemplateArgumentLoc(ArgLoc);
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -5143,8 +5143,9 @@
 SmallString<128> Str;
 llvm::raw_svector_ostream OS(Str);
 OS << *ClassSpec;
-printTemplateArgumentList(OS, ClassSpec->getTemplateArgs().asArray(),
-  Policy);
+printTemplateArgumentList(
+OS, ClassSpec->getTemplateArgs().asArray(), Policy,
+ClassSpec->getSpecializedTemplate()->getTemplateParameters());
 return cxstring::createDup(OS.str());
   }
 
Index: clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -528,3 +528,33 @@
 x1.f(x2);
   }
 }
+
+namespace TypeSuffix {
+  template  struct A {};
+  template <> struct A<1> { using type = int; }; // expected-note {{'A<1>::type' declared here}}
+  A<1L>::type a; // expected-error {{no type named 'type' in 'TypeSuffix::A<1L>'; did you mean 'A<1>::type'?}}
+
+  template  struct B {};
+  template <> struct B<1> { using type = int; }; // expected-note {{'B<1>::type' declared here}}
+  B<2>::type b;  // expected-error {{no type named 'type' in 'TypeSuffix::B<2>'; did you mean 'B<1>::type'?}}
+
+  template  struct C {};
+  template <> struct C<'a'> { using type = signed char; }; // expected-note {{'C<'a'>::type' declared here}}
+  C<(signed char)'a'>::type c; // expected-error {{no type named 'type' in 'TypeSuffix::C<(signed char)'a'>'; did you mean 'C<'a'>::type'?}}
+
+  template  struct D {};
+  template <> struct D<'a'> { using type = signed char; }; // expected-note {{'D<'a'>::type' declared here}}
+  D<'b'>::type d;  // expected-error {{no type named 'type' in 'TypeSuffix::D<'b'>'; did you mean 'D<'a'>::type'?}}
+
+  template  struct E {};
+  template <> struct E<'a'> { using type = unsigned char; }; // expected-note {{'E<'a'>::type' declared here}}
+  E<(unsigned char)'a'>::type e; // expected-error {{no type named 'type' in 'TypeSuffix::E<(unsigned char)'a'>'; did you mean 'E<'a'>::type'?}}
+
+  template  struct F {};
+  template <> struct F<'a'> { using type = unsigned char; }; // expected-note {{'F<'a'>::type' declared here}}
+  F<'b'>::type f;// expected-error {{no type named

[clang-tools-extra] 0b55ecc - [clangd] Allow modules to bind LSP methods/notifications/commands

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-02-15T11:00:14+01:00
New Revision: 0b55ecce45d7cc79b614bcb91cd070ab257227fc

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

LOG: [clangd] Allow modules to bind LSP methods/notifications/commands

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/Module.h
clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 52939c0dc035..0f5fe7c3528c 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -524,98 +524,106 @@ void ClangdLSPServer::onInitialize(const 
InitializeParams &Params,
 BackgroundIndexProgressState = BackgroundIndexProgress::Empty;
   BackgroundIndexSkipCreate = Params.capabilities.ImplicitProgressCreation;
 
+  llvm::json::Object ServerCaps{
+  {"textDocumentSync",
+   llvm::json::Object{
+   {"openClose", true},
+   {"change", (int)TextDocumentSyncKind::Incremental},
+   {"save", true},
+   }},
+  {"documentFormattingProvider", true},
+  {"documentRangeFormattingProvider", true},
+  {"documentOnTypeFormattingProvider",
+   llvm::json::Object{
+   {"firstTriggerCharacter", "\n"},
+   {"moreTriggerCharacter", {}},
+   }},
+  {"completionProvider",
+   llvm::json::Object{
+   {"allCommitCharacters",
+{" ", "\t", "(", ")", "[", "]", "{",  "}", "<",
+ ">", ":",  ";", ",", "+", "-", "/",  "*", "%",
+ "^", "&",  "#", "?", ".", "=", "\"", "'", "|"}},
+   {"resolveProvider", false},
+   // We do extra checks, e.g. that > is part of ->.
+   {"triggerCharacters", {".", "<", ">", ":", "\"", "/"}},
+   }},
+  {"semanticTokensProvider",
+   llvm::json::Object{
+   {"full", llvm::json::Object{{"delta", true}}},
+   {"range", false},
+   {"legend",
+llvm::json::Object{{"tokenTypes", semanticTokenTypes()},
+   {"tokenModifiers", semanticTokenModifiers()}}},
+   }},
+  {"signatureHelpProvider",
+   llvm::json::Object{
+   {"triggerCharacters", {"(", ","}},
+   }},
+  {"declarationProvider", true},
+  {"definitionProvider", true},
+  {"implementationProvider", true},
+  {"documentHighlightProvider", true},
+  {"documentLinkProvider",
+   llvm::json::Object{
+   {"resolveProvider", false},
+   }},
+  {"hoverProvider", true},
+  {"selectionRangeProvider", true},
+  {"documentSymbolProvider", true},
+  {"workspaceSymbolProvider", true},
+  {"referencesProvider", true},
+  {"astProvider", true}, // clangd extension
+  {"typeHierarchyProvider", true},
+  {"memoryUsageProvider", true}, // clangd extension
+  {"compilationDatabase",// clangd extension
+   llvm::json::Object{{"automaticReload", true}}},
+  {"callHierarchyProvider", true},
+  };
+
+  {
+LSPBinder Binder(Handlers);
+if (Opts.Modules)
+  for (auto &Mod : *Opts.Modules)
+Mod.initializeLSP(Binder, Params.capabilities, ServerCaps);
+  }
+
   // Per LSP, renameProvider can be either boolean or RenameOptions.
   // RenameOptions will be specified if the client states it supports prepare.
-  llvm::json::Value RenameProvider =
-  llvm::json::Object{{"prepareProvider", true}};
-  if (!Params.capabilities.RenamePrepareSupport) // Only boolean allowed per 
LSP
-RenameProvider = true;
+  ServerCaps["renameProvider"] =
+  Params.capabilities.RenamePrepareSupport
+  ? llvm::json::Object{{"prepareProvider", true}}
+  : llvm::json::Value(true);
 
-  // Per LSP, codeActionProvide can be either boolean or CodeActionOptions.
+  // Per LSP, codeActionProvider can be either boolean or CodeActionOptions.
   // CodeActionOptions is only valid if the client supports action literal
   // via textDocument.codeAction.codeActionLiteralSupport.
   llvm::json::Value CodeActionProvider = true;
-  if (Params.capabilities.CodeActionStructure)
-CodeActionProvider = llvm::json::Object{
-{"codeActionKinds",
- {CodeAction::QUICKFIX_KIND, CodeAction::REFACTOR_KIND,
-  CodeAction::INFO_KIND}}};
+  ServerCaps["codeActionProvider"] =
+  Params.capabilities.CodeActionStructure
+  ? llvm::json::Object{{"codeActionKinds",
+{CodeAction::QUICKFIX_KIND,
+ CodeAction::REFACTOR_KIND,
+ CodeAction::INFO_KIND}}}
+  : llvm::json::Value(true

[PATCH] D96625: [clangd] Allow modules to bind LSP methods/notifications/commands

[Sam McCall 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 rG0b55ecce45d7: [clangd] Allow modules to bind LSP 
methods/notifications/commands (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96625

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/Module.h
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -221,6 +221,29 @@
   DiagMessage("Use of undeclared identifier 'BAR'";
 }
 
+TEST_F(LSPTest, ModulesTest) {
+  class MathModule : public Module {
+void initializeLSP(LSPBinder &Bind, const ClientCapabilities &ClientCaps,
+   llvm::json::Object &ServerCaps) override {
+  Bind.notification("add", this, &MathModule::add);
+  Bind.method("get", this, &MathModule::get);
+}
+
+void add(const int &X) { Value += X; }
+void get(const std::nullptr_t &, Callback Reply) { Reply(Value); }
+int Value = 0;
+  };
+  std::vector> Mods;
+  Mods.push_back(std::make_unique());
+  ModuleSet ModSet(std::move(Mods));
+  Opts.Modules = &ModSet;
+
+  auto &Client = start();
+  Client.notify("add", 2);
+  Client.notify("add", 8);
+  EXPECT_EQ(10, Client.call("get", nullptr).takeValue());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Module.h
===
--- clang-tools-extra/clangd/Module.h
+++ clang-tools-extra/clangd/Module.h
@@ -1,7 +1,10 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULE_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULE_H
 
+#include "LSPBinder.h"
+#include "Protocol.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/JSON.h"
 #include 
 #include 
 
@@ -10,14 +13,11 @@
 
 /// A Module contributes a vertical feature to clangd.
 ///
-/// FIXME: Extend this with LSP bindings to support reading/updating
-/// capabilities and implementing LSP endpoints.
+/// FIXME: Extend this to support outgoing LSP calls.
 ///
 /// The lifetime of a module is roughly:
 ///  - modules are created before the LSP server, in ClangdMain.cpp
 ///  - these modules are then passed to ClangdLSPServer and ClangdServer
-///FIXME: LSP bindings should be registered at ClangdLSPServer
-///initialization.
 ///  - module hooks can be called at this point.
 ///FIXME: We should make some server facilities like TUScheduler and index
 ///available to those modules after ClangdServer is initalized.
@@ -30,6 +30,17 @@
 class Module {
 public:
   virtual ~Module() = default;
+
+  /// Called by the server to connect this module to LSP.
+  /// The module should register the methods/notifications/commands it handles,
+  /// and update the server capabilities to advertise them.
+  ///
+  /// This is only called if the module is running in ClangdLSPServer!
+  /// Modules with a public interface should satisfy it without LSP bindings.
+  // FIXME: ClientCaps should be a raw json::Object here.
+  virtual void initializeLSP(LSPBinder &Bind,
+ const ClientCapabilities &ClientCaps,
+ llvm::json::Object &ServerCaps) {}
 };
 
 class ModuleSet {
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -524,98 +524,106 @@
 BackgroundIndexProgressState = BackgroundIndexProgress::Empty;
   BackgroundIndexSkipCreate = Params.capabilities.ImplicitProgressCreation;
 
+  llvm::json::Object ServerCaps{
+  {"textDocumentSync",
+   llvm::json::Object{
+   {"openClose", true},
+   {"change", (int)TextDocumentSyncKind::Incremental},
+   {"save", true},
+   }},
+  {"documentFormattingProvider", true},
+  {"documentRangeFormattingProvider", true},
+  {"documentOnTypeFormattingProvider",
+   llvm::json::Object{
+   {"firstTriggerCharacter", "\n"},
+   {"moreTriggerCharacter", {}},
+   }},
+  {"completionProvider",
+   llvm::json::Object{
+   {"allCommitCharacters",
+{" ", "\t", "(", ")", "[", "]", "{",  "}", "<",
+ ">", ":",  ";", ",", "+", "-", "/",  "*", "%",
+ "^", "&",  "#", "?", ".", "=", "\"", "'", "|"}},
+   {"resolveProvider", false},
+   // We do extra checks, e.g. that > is part of ->.
+   {"triggerCharacters", {".", "<", ">", ":", "\"", "/"}},
+   }},
+  {"semanticTokensProvider",
+   llvm::json::Object{

[clang] e54811f - Restore diagnostic handler after CodeGenAction::ExecuteAction

[Marco Antognini via cfe-commits]

Author: Marco Antognini
Date: 2021-02-15T10:33:00Z
New Revision: e54811ff7e0bc99f337bcbb569311bb166187322

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

LOG: Restore diagnostic handler after CodeGenAction::ExecuteAction

Fix dangling pointer to local variable and address some typos.

Reviewed By: xur

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

Added: 


Modified: 
clang/lib/CodeGen/CodeGenAction.cpp
llvm/include/llvm/IR/LLVMContext.h

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenAction.cpp 
b/clang/lib/CodeGen/CodeGenAction.cpp
index 778d4df3c2e9..da352463450b 100644
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -1115,6 +1115,14 @@ void CodeGenAction::ExecuteAction() {
   LLVMContext &Ctx = TheModule->getContext();
   Ctx.setInlineAsmDiagnosticHandler(BitcodeInlineAsmDiagHandler, &Diagnostics);
 
+  // Restore any diagnostic handler previously set before returning from this
+  // function.
+  struct RAII {
+LLVMContext &Ctx;
+std::unique_ptr PrevHandler = 
Ctx.getDiagnosticHandler();
+~RAII() { Ctx.setDiagnosticHandler(std::move(PrevHandler)); }
+  } _{Ctx};
+
   // Set clang diagnostic handler. To do this we need to create a fake
   // BackendConsumer.
   BackendConsumer Result(BA, CI.getDiagnostics(), CI.getHeaderSearchOpts(),

diff  --git a/llvm/include/llvm/IR/LLVMContext.h 
b/llvm/include/llvm/IR/LLVMContext.h
index 1195a9e5fb28..a352deda6bc8 100644
--- a/llvm/include/llvm/IR/LLVMContext.h
+++ b/llvm/include/llvm/IR/LLVMContext.h
@@ -190,10 +190,11 @@ class LLVMContext {
   DiagnosticHandler::DiagnosticHandlerTy DiagHandler,
   void *DiagContext = nullptr, bool RespectFilters = false);
 
-  /// setDiagnosticHandler - This method sets unique_ptr to object of 
DiagnosticHandler
-  /// to provide custom diagnostic handling. The first argument is unique_ptr 
of object
-  /// of type DiagnosticHandler or a derived of that.   The third argument 
should be
-  /// set to true if the handler only expects enabled diagnostics.
+  /// setDiagnosticHandler - This method sets unique_ptr to object of
+  /// DiagnosticHandler to provide custom diagnostic handling. The first
+  /// argument is unique_ptr of object of type DiagnosticHandler or a derived
+  /// of that. The second argument should be set to true if the handler only
+  /// expects enabled diagnostics.
   ///
   /// Ownership of this pointer is moved to LLVMContextImpl.
   void setDiagnosticHandler(std::unique_ptr &&DH,
@@ -211,7 +212,7 @@ class LLVMContext {
   /// setDiagnosticHandler.
   const DiagnosticHandler *getDiagHandlerPtr() const;
 
-  /// getDiagnosticHandler - transfers owenership of DiagnosticHandler 
unique_ptr
+  /// getDiagnosticHandler - transfers ownership of DiagnosticHandler 
unique_ptr
   /// to caller.
   std::unique_ptr getDiagnosticHandler();
 



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

[PATCH] D96487: Restore diagnostic handler after CodeGenAction::ExecuteAction

[Marco Antognini 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 rGe54811ff7e0b: Restore diagnostic handler after 
CodeGenAction::ExecuteAction (authored by mantognini).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96487

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  llvm/include/llvm/IR/LLVMContext.h


Index: llvm/include/llvm/IR/LLVMContext.h
===
--- llvm/include/llvm/IR/LLVMContext.h
+++ llvm/include/llvm/IR/LLVMContext.h
@@ -190,10 +190,11 @@
   DiagnosticHandler::DiagnosticHandlerTy DiagHandler,
   void *DiagContext = nullptr, bool RespectFilters = false);
 
-  /// setDiagnosticHandler - This method sets unique_ptr to object of 
DiagnosticHandler
-  /// to provide custom diagnostic handling. The first argument is unique_ptr 
of object
-  /// of type DiagnosticHandler or a derived of that.   The third argument 
should be
-  /// set to true if the handler only expects enabled diagnostics.
+  /// setDiagnosticHandler - This method sets unique_ptr to object of
+  /// DiagnosticHandler to provide custom diagnostic handling. The first
+  /// argument is unique_ptr of object of type DiagnosticHandler or a derived
+  /// of that. The second argument should be set to true if the handler only
+  /// expects enabled diagnostics.
   ///
   /// Ownership of this pointer is moved to LLVMContextImpl.
   void setDiagnosticHandler(std::unique_ptr &&DH,
@@ -211,7 +212,7 @@
   /// setDiagnosticHandler.
   const DiagnosticHandler *getDiagHandlerPtr() const;
 
-  /// getDiagnosticHandler - transfers owenership of DiagnosticHandler 
unique_ptr
+  /// getDiagnosticHandler - transfers ownership of DiagnosticHandler 
unique_ptr
   /// to caller.
   std::unique_ptr getDiagnosticHandler();
 
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -1115,6 +1115,14 @@
   LLVMContext &Ctx = TheModule->getContext();
   Ctx.setInlineAsmDiagnosticHandler(BitcodeInlineAsmDiagHandler, &Diagnostics);
 
+  // Restore any diagnostic handler previously set before returning from this
+  // function.
+  struct RAII {
+LLVMContext &Ctx;
+std::unique_ptr PrevHandler = 
Ctx.getDiagnosticHandler();
+~RAII() { Ctx.setDiagnosticHandler(std::move(PrevHandler)); }
+  } _{Ctx};
+
   // Set clang diagnostic handler. To do this we need to create a fake
   // BackendConsumer.
   BackendConsumer Result(BA, CI.getDiagnostics(), CI.getHeaderSearchOpts(),


Index: llvm/include/llvm/IR/LLVMContext.h
===
--- llvm/include/llvm/IR/LLVMContext.h
+++ llvm/include/llvm/IR/LLVMContext.h
@@ -190,10 +190,11 @@
   DiagnosticHandler::DiagnosticHandlerTy DiagHandler,
   void *DiagContext = nullptr, bool RespectFilters = false);
 
-  /// setDiagnosticHandler - This method sets unique_ptr to object of DiagnosticHandler
-  /// to provide custom diagnostic handling. The first argument is unique_ptr of object
-  /// of type DiagnosticHandler or a derived of that.   The third argument should be
-  /// set to true if the handler only expects enabled diagnostics.
+  /// setDiagnosticHandler - This method sets unique_ptr to object of
+  /// DiagnosticHandler to provide custom diagnostic handling. The first
+  /// argument is unique_ptr of object of type DiagnosticHandler or a derived
+  /// of that. The second argument should be set to true if the handler only
+  /// expects enabled diagnostics.
   ///
   /// Ownership of this pointer is moved to LLVMContextImpl.
   void setDiagnosticHandler(std::unique_ptr &&DH,
@@ -211,7 +212,7 @@
   /// setDiagnosticHandler.
   const DiagnosticHandler *getDiagHandlerPtr() const;
 
-  /// getDiagnosticHandler - transfers owenership of DiagnosticHandler unique_ptr
+  /// getDiagnosticHandler - transfers ownership of DiagnosticHandler unique_ptr
   /// to caller.
   std::unique_ptr getDiagnosticHandler();
 
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -1115,6 +1115,14 @@
   LLVMContext &Ctx = TheModule->getContext();
   Ctx.setInlineAsmDiagnosticHandler(BitcodeInlineAsmDiagHandler, &Diagnostics);
 
+  // Restore any diagnostic handler previously set before returning from this
+  // function.
+  struct RAII {
+LLVMContext &Ctx;
+std::unique_ptr PrevHandler = Ctx.getDiagnosticHandler();
+~RAII() { Ctx.setDiagnosticHandler(std::move(PrevHandler)); }
+  } _{Ctx};
+
   // Set clang diagnostic handler. To do this we need to create a fake
   // BackendConsumer.
   BackendConsumer Result(BA, CI.getDiagnostics(), CI.getHeaderSearchOpts(),
__

[PATCH] D90691: [analyzer] Add new checker for unchecked return value.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

There is a clang-tidy check bugprone-unused-return-value 

 that may be confusing in relation to this checker. It does similar job for a 
selected set of functions and with different reason.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90691

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

[PATCH] D96344: [flang][driver] Add options for -fdefault* and -flarge-sizes

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

LGTM, thank you for working on this @arnamoy10 !

I left a few _minor_ comments. These can be addressed in the actual commit.




Comment at: clang/include/clang/Driver/Options.td:4224-4231
+def fdefault_double_8 : Flag<["-"],"fdefault-double-8">, Group, 
Flags<[FlangOption,FC1Option,FlangOnlyOption]>,
+  HelpText<"Set the DOUBLE PRECISION type and double real constants like 1.d0 
to an 8 byte wide type.">;
+def fdefault_integer_8 : Flag<["-"],"fdefault-integer-8">, Group, 
Flags<[FlangOption,FC1Option,FlangOnlyOption]>,
+  HelpText<"Set the default integer and logical types to an 8 byte wide 
type.">;
+def fdefault_real_8 : Flag<["-"],"fdefault-real-8">, Group, 
Flags<[FlangOption,FC1Option,FlangOnlyOption]>,
+  HelpText<"Set the default real type to an 8 byte wide type.">;
+def flarge_sizes : Flag<["-"],"flarge-sizes">, Group, 
Flags<[FlangOption,FC1Option,FlangOnlyOption]>,

As these options are defined within this `let` statement:
```
let Flags = [FC1Option, FlangOption, FlangOnlyOption] in {

}
```
there is no need to specify `Flags` independently for each.



Comment at: flang/include/flang/Frontend/CompilerInvocation.h:80-81
+  // Fortran Dialect options
+  std::unique_ptr defaultKinds_;
+  Fortran::frontend::DialectOptions dialectOpts_;
+

awarzynski wrote:
> Wouldn't it be possible to just use `defaultKinds_` here rather than 
> `defaultKinds_` _and_ `dialectOpts_`? Do you think that we will need both?
Thank you for updating this.

One additional point - I would be tempted to use a regular member variable here 
rather than a pointer. I suspect that you were inspired by `semanticContext_`? 
Currently we have to use a pointer for `semanticsContext_` as we can't use the 
default constructor for `SemanticsContext` (it requires arguments that we don't 
have access to at the point of constructing `CompilerInvocation`). That's not 
the case for `IntrinsicTypeDefaultKinds`.

But I might missing something here!



Comment at: flang/test/Flang-Driver/driver-help.f90:31
 ! HELP-NEXT: -ffixed-line-length=
-! HELP-NEXT: Use  as character line width in fixed mode
+! HELP-NEXT:  Use  as character line width in fixed mode
 ! HELP-NEXT: -ffree-formProcess source files in free form

[nit] Unrelated change


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

https://reviews.llvm.org/D96344

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

[PATCH] D95877: [analyzer] Fix static_cast on pointer-to-member handling

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

@RedDocMD Please can you take a look at this buildbot failure: 
http://lab.llvm.org:8011/#/builders/124


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95877

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

[PATCH] D95877: [analyzer] Fix static_cast on pointer-to-member handling

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD added a comment.

In D95877#2563220 , @RKSimon wrote:

> @RedDocMD Please can you take a look at this buildbot failure: 
> http://lab.llvm.org:8011/#/builders/124

Yes. Thanks for the ping 😀


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95877

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

[PATCH] D96407: [flang][driver] Add extension options and -finput-charset

[Faris Rehman via Phabricator via cfe-commits]

FarisRehman updated this revision to Diff 323717.
FarisRehman marked 2 inline comments as done.
FarisRehman added a comment.

Address review comment

This revision addresses a review comment by @awarzynski

Summary of changes:

- Rebase off the latest D96344  revision
- Add a help message to `-finput-charset`
- Remove unnecessary test CHECK lines


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96407

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/FrontendOptions.h
  flang/include/flang/Parser/parsing.h
  flang/lib/Frontend/CompilerInstance.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Flang-Driver/driver-help-hidden.f90
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Flang-Driver/escaped-backslash.f90
  flang/test/Flang-Driver/frontend-forwarding.f90
  flang/test/Flang-Driver/implicit-none.f90
  flang/test/Flang-Driver/pipeline.f90
  flang/test/Semantics/oldparam02.f90
  flang/test/Semantics/resolve64.f90

Index: flang/test/Semantics/resolve64.f90
===
--- flang/test/Semantics/resolve64.f90
+++ flang/test/Semantics/resolve64.f90
@@ -1,4 +1,4 @@
-! RUN: %S/test_errors.sh %s %t %f18 -flogical-abbreviations -fxor-operator
+! RUN: %S/test_errors.sh %s %t %flang -flogical-abbreviations -fxor-operator
 
 ! Like m4 in resolve63 but compiled with different options.
 ! Alternate operators are enabled so treat these as intrinsic.
Index: flang/test/Semantics/oldparam02.f90
===
--- flang/test/Semantics/oldparam02.f90
+++ flang/test/Semantics/oldparam02.f90
@@ -1,4 +1,4 @@
-! RUN: not %f18 -falternative-parameter-statement -fdebug-dump-symbols -fsyntax-only %s 2>&1 | FileCheck %s
+! RUN: not %flang -falternative-parameter-statement -fsyntax-only %s 2>&1 | FileCheck %s
 
 ! Error tests for "old style" PARAMETER statements
 subroutine subr(x1,x2,x3,x4,x5)
Index: flang/test/Flang-Driver/implicit-none.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/implicit-none.f90
@@ -0,0 +1,32 @@
+! Ensure argument -fimplicit-none works as expected.
+
+! REQUIRES: new-flang-driver
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: %flang-new -fsyntax-only %s  2>&1 | FileCheck %s --allow-empty --check-prefix=DEFAULT
+! RUN: %flang-new -fsyntax-only -fimplicit-none -fno-implicit-none %s  2>&1 | FileCheck %s --allow-empty --check-prefix=DEFAULT
+! RUN: not %flang-new -fsyntax-only -fimplicit-none %s  2>&1 | FileCheck %s --check-prefix=WITH_IMPL_NONE
+
+!-
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!-
+! RUN: %flang-new -fc1 -fsyntax-only %s  2>&1 | FileCheck %s --allow-empty --check-prefix=DEFAULT
+! RUN: %flang-new -fc1 -fsyntax-only -fimplicit-none -fno-implicit-none %s  2>&1 | FileCheck %s --allow-empty --check-prefix=DEFAULT
+! RUN: not %flang-new -fc1 -fsyntax-only -fimplicit-none %s  2>&1 | FileCheck %s --check-prefix=WITH_IMPL_NONE
+
+!--
+! EXPECTED OUTPUT FOR NO IMPLICIT NONE
+!--
+! DEFAULT-NOT:error
+
+!--
+! EXPECTED OUTPUT FOR IMPLICIT NONE ALWAYS
+!--
+! WITH_IMPL_NONE:No explicit type declared for 'a'
+! WITH_IMPL_NONE:No explicit type declared for 'b'
+
+function a()
+  a = b
+end
\ No newline at end of file
Index: flang/test/Flang-Driver/frontend-forwarding.f90
===
--- flang/test/Flang-Driver/frontend-forwarding.f90
+++ flang/test/Flang-Driver/frontend-forwarding.f90
@@ -1,11 +1,14 @@
 ! This file tests that flang-new forwards 
 ! all Flang frontend options to flang-new -fc1
 ! as expected.
-!
+
+! REQUIRES: new-flang-driver
+
 ! RUN: %flang-new -fsyntax-only -### %s -o %t 2>&1 \
 ! RUN: -fdefault-double-8 \
 ! RUN: -fdefault-integer-8 \
 ! RUN: -fdefault-real-8 \
+! RUN: -finput-charset=utf-8 \
 ! RUN: -flarge-sizes \
 ! RUN:   | FileCheck %s
 !
@@ -13,5 +16,5 @@
 ! CHECK: "-fdefault-double-8"
 ! CHECK: "-fdefault-integer-8"
 ! CHECK: "-fdefault-real-8"
+! CHECK: "-finput-charset=utf-8"
 ! CHECK: "-flarge-sizes"
-
Index: flang/test/Flang-Driver/escaped-backslash.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/escaped-backslash.f90
@@ -0,0 +1,35 @@
+! Ensure argument -fbackslash works as expected.
+
+! REQUIRES: new-flang-driver
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: %flang-new -E %s  2>&1 | FileCheck %s --check-prefix=ESCAPED
+! RUN: %flang-new -E -fb

[PATCH] D96407: [flang][driver] Add extension options and -finput-charset

[Faris Rehman via Phabricator via cfe-commits]

FarisRehman added inline comments.



Comment at: flang/test/Flang-Driver/implicit-none.f90:10
+! RUN: not %flang-new -fsyntax-only -fimplicit-none %s  2>&1 | FileCheck %s 
--check-prefix=ALWAYS
+! RUN: %flang-new -fsyntax-only -fno-implicit-none %s  2>&1 | FileCheck %s 
--allow-empty --check-prefix=DEFAULT
+

awarzynski wrote:
> What about:
> ```
> ! RUN: %flang-new -fsyntax-only %s  2>&1 | FileCheck %s --allow-empty 
> --check-prefix=DEFAULT
> ```
> and:
> ```
> ! RUN: %flang-new -fimplicit-none -fno-implicit-none -fsyntax-only %s  2>&1 | 
> FileCheck %s --allow-empty --check-prefix=DEFAULT
> ```
> 
> Similar point for other tests.
The first test is already present, and I have now modified the 
`-fno-implicit-none` test to specify `-fimplicit-none` before it to show the 
behaviour more clearly. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96407

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

[PATCH] D96705: [clang][cli] Add explicit round-trip test

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch adds a test that verifies all `CompilerInvocation` members are 
filled correctly during command line round-trip.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96705

Files:
  clang/unittests/Frontend/CompilerInvocationTest.cpp

Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -7,8 +7,10 @@
 //===--===//
 
 #include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Basic/TargetOptions.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/TextDiagnosticBuffer.h"
+#include "clang/Lex/PreprocessorOptions.h"
 #include "llvm/Support/Host.h"
 
 #include "gmock/gmock.h"
@@ -740,4 +742,85 @@
   Invocation.generateCC1CommandLine(GeneratedArgs, *this);
   ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fdigraphs")));
 }
+
+TEST_F(CommandLineTest, RoundTrip) {
+  // Testing one marshalled and one manually generated option from each
+  // CompilerInvocation member.
+  const char *Args[] = {
+  "-round-trip-args",
+  // LanguageOptions
+  "-std=c17",
+  "-fmax-tokens=10",
+  // TargetOptions
+  "-target-sdk-version=1.2.3",
+  "-meabi",
+  "4",
+  // DiagnosticOptions
+  "-Wundef-prefix=XY",
+  "-fdiagnostics-format",
+  "clang",
+  // HeaderSearchOptions
+  "-stdlib=libc++",
+  "-fimplicit-module-maps",
+  // PreprocessorOptions
+  "-DXY=AB",
+  "-include-pch",
+  "a.pch",
+  // AnalyzerOptions
+  "-analyzer-config",
+  "ctu-import-threshold=42",
+  "-unoptimized-cfg",
+  // MigratorOptions (no manually handled arguments)
+  "-no-ns-alloc-error",
+  // CodeGenOptions
+  "-debug-info-kind=limited",
+  "-debug-info-macro",
+  // DependencyOutputOptions
+  "--show-includes",
+  "-H",
+  // FileSystemOptions (no manually handled arguments)
+  "-working-directory",
+  "folder",
+  // FrontendOptions
+  "-load",
+  "plugin",
+  "-ast-merge",
+  // PreprocessorOutputOptions
+  "-dD",
+  "-CC",
+  };
+
+  ASSERT_TRUE(CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags));
+
+  ASSERT_TRUE(Invocation.getLangOpts()->C17);
+  ASSERT_EQ(Invocation.getLangOpts()->MaxTokens, 10u);
+
+  ASSERT_EQ(Invocation.getTargetOpts().SDKVersion, llvm::VersionTuple(1, 2, 3));
+  ASSERT_EQ(Invocation.getTargetOpts().EABIVersion, EABI::EABI4);
+
+  ASSERT_THAT(Invocation.getDiagnosticOpts().UndefPrefixes,
+  Contains(StrEq("XY")));
+  ASSERT_EQ(Invocation.getDiagnosticOpts().getFormat(),
+TextDiagnosticFormat::Clang);
+
+  ASSERT_TRUE(Invocation.getHeaderSearchOpts().UseLibcxx);
+  ASSERT_TRUE(Invocation.getHeaderSearchOpts().ImplicitModuleMaps);
+
+  ASSERT_THAT(Invocation.getPreprocessorOpts().Macros,
+  Contains(std::make_pair(std::string("XY=AB"), false)));
+  ASSERT_EQ(Invocation.getPreprocessorOpts().ImplicitPCHInclude, "a.pch");
+
+  ASSERT_EQ(Invocation.getAnalyzerOpts()->Config["ctu-import-threshold"], "42");
+  ASSERT_TRUE(Invocation.getAnalyzerOpts()->UnoptimizedCFG);
+
+  ASSERT_TRUE(Invocation.getMigratorOpts().NoNSAllocReallocError);
+
+  ASSERT_EQ(Invocation.getCodeGenOpts().getDebugInfo(),
+codegenoptions::DebugInfoKind::LimitedDebugInfo);
+  ASSERT_TRUE(Invocation.getCodeGenOpts().MacroDebugInfo);
+
+  ASSERT_EQ(Invocation.getDependencyOutputOpts().ShowIncludesDest,
+ShowIncludesDestination::Stdout);
+  ASSERT_TRUE(Invocation.getDependencyOutputOpts().ShowHeaderIncludes);
+}
 } // anonymous namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D96163: [analyzer] Add 12.0.0. release notes

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

Szelethus added a comment.

ping


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

https://reviews.llvm.org/D96163

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

[PATCH] D96524: [OpenCL] Add support of OpenCL C 3.0 __opencl_c_fp64

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/lib/Basic/OpenCLOptions.cpp:24
+  auto CLVer = LO.OpenCLCPlusPlus ? 200 : LO.OpenCLVersion;
+  return CLVer >= 300 ? isEnabled("__opencl_c_fp64") : 
isEnabled("cl_khr_fp64");
+}

azabaznov wrote:
> Anastasia wrote:
> > We should at least be checking for `isSupported("__opencl_c_fp64")`, but 
> > frankly I would prefer to check for supported and not for enabled for 
> > `cl_khr_fp64` too. Note that we don't break backward compatibility if we 
> > change this because the existing kernels will still be valid and it makes 
> > things easier for writing new kernels too.
> I think everything fine with this for now because 
> `OpenCLOptions::enableSupportedCore` is called to set all supported core or 
> optional core features to enabled state. So you suggest to removing this 
> method at all too?
> 
> I think with your approach things will be unobvious in code: for some 
> extensions there will be check for `isSupported` for some other there will be 
> check for `isEnabled`. I think we should stay consistent here and check 
> availability of all options in the same manner.
> I think everything fine with this for now because 
> OpenCLOptions::enableSupportedCore is called to set all supported core or 
> optional core features to enabled state. So you suggest to removing this 
> method at all too?

Yes, I find it redundant somehow. Maybe it's best to indeed remove enabling 
functionality for features since we definitely don't plan to use pragmas for 
those? However, I appreciate it might be better to do as a separate change.

 
> I think with your approach things will be unobvious in code: for some 
> extensions there will be check for isSupported for some other there will be 
> check for isEnabled. I think we should stay consistent here and check 
> availability of all options in the same manner.

That's right, we might get some inconsistency at the start. But I think we 
should drive towards checking `isSupported` rather than `isEnabled`. I don't 
think we will have many cases for `isEnabled` at the end.



Comment at: clang/lib/Basic/TargetInfo.cpp:398
+auto CLVer = Opts.OpenCLCPlusPlus ? 200 : Opts.OpenCLVersion;
+if (CLVer >= 300) {
+  auto &OCLOpts = getSupportedOpenCLOpts();

azabaznov wrote:
> Anastasia wrote:
> > I suggest we move this onto `OpenCLOptions::addSupport`.
> This should stay here to control simultaneous macro definitions
Could we leave this bit out? These are set correctly by the targets already... 
and I think targets do need to set those explicitly indeed. I don't see big 
value in this functionality right now.



Comment at: clang/lib/Headers/opencl-c.h:4635
 
-#ifdef cl_khr_fp64
+#if defined(cl_khr_fp64) || defined(__opencl_c_fp64)
 #pragma OPENCL EXTENSION cl_khr_fp64 : enable

azabaznov wrote:
> azabaznov wrote:
> > Anastasia wrote:
> > > svenvh wrote:
> > > > Wondering if we need a similar change in 
> > > > `clang/lib/Headers/opencl-c-base.h` to guard the double vector types?
> > > Instead of growing the guard condition, I suggest we only check for one 
> > > for example the feature macro and then make sure the feature macro is 
> > > defined in `opencl-c-base.h` if the extensions that aliases to the 
> > > feature is supported. However, it would also be ok to do the opposite 
> > > since the extensions and the corresponding features should both be 
> > > available.
> > > 
> > > FYI, something similar was done in https://reviews.llvm.org/D92004.
> > > 
> > > This will also help to propagate the functionality into Tablegen header 
> > > because we won't need to extend it to work with multiple extensions but 
> > > we might still need to do the rename.
> > Yeah, I overlooked this place... Thanks!
> I don't think that growing of this condition will hurt in some cases... Note, 
> that unifying condition check makes sense if some features are 
> unconditionally supported for OpenCL C 2.0, such as generic address space for 
> example. In other cases (such as fp64, 3d image writes, subgroups) we should 
> use OR in guard condition.  
> 
> Your approach will require extra checks in clang such as, for example, to 
> make sure that extension macro is not predefined if the feature macro is 
> defined, because there will be redefinition since extension macro is defined 
> in header.
I think using one macro for everything is just simpler and cleaner.

> Your approach will require extra checks in clang such as, for example, to 
> make sure that extension macro is not predefined if the feature macro is 
> defined, because there will be redefinition since extension macro is defined 
> in header.

I think we should handle this in the headers instead and we should definitely 
define the macros conditionally to avoid redefinitions.



Comment at: clang/test/SemaOpenCL/opencl-fp64-feature.cl:1
+// Test with a target not supporting

[clang] 6f21ada - [analyzer][NFC] Fix test failures for builds w/o assertions

[Valeriy Savchenko via cfe-commits]

Author: Valeriy Savchenko
Date: 2021-02-15T16:38:15+03:00
New Revision: 6f21adac6dd7082f7231ae342d40ed04f4885e79

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

LOG: [analyzer][NFC] Fix test failures for builds w/o assertions

Added: 


Modified: 
clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp

Removed: 




diff  --git a/clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp 
b/clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp
index 1631be70da3e..c457d2230ddd 100644
--- a/clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp
+++ b/clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify 
%s
-// XFAIL: *
+// XFAIL: asserts
 
 void clang_analyzer_eval(bool);
 



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

[PATCH] D95534: clang-cl: Invent a /winsysroot concept

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

In D95534#2550998 , @thakis wrote:

> In D95534#2534510 , @smeenai wrote:
>
>> I saw this on LLVM Weekly, and I like it a lot :)
>>
>> Now if only Windows could case-correct their SDKs so we didn't need VFS 
>> overlays and symlinks for the linker...
>
> We use ciopfs for this, 
> https://source.chromium.org/chromium/chromium/src/+/master:build/vs_toolchain.py;l=485?q=ciopfs
>  That code has worked without changes for the last 2.5 years.

//(sorry a bit off topic for LLVM)//
Hello @thakis ! What is the best way to report Chrome dev issues? I wanted to 
try ciopfs, so I followed the guide here 
, 
but clang-cl then fails with "Too many files open", pointing to files in 
`chromium/src/third_party/depot_tools/win_toolchain/vs_files/`. I've removed 
the ciopfs mount point and instead created a 'casefold' folder at the same 
location (ie. `chattr +F vs_files`), unzipped the SDKs again, and now the build 
compiles fine. Raising `ulimit -n 25` and `fs.file-max = 100` didn't 
help the ciopfs issue in the first place. I'm running on a 48C/96T machine on 
Ubuntu 20.04. There seems to be either a leak of FDs or caching inside ciopfs? 
Any suggestions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95534

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

[PATCH] D96123: [clangd] Expose actOnAllPArentDirectories helper

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clangd/support/Path.h:33
+// deepest directory and going up to root. Stops whenever action succeeds.
+void actOnAllParentDirectories(PathRef FileName,
+   llvm::function_ref Action);

The signature is a bit weird here,
 - prone to boolean-sense mistakes
 - it's not obvious it only works on absolute paths
 - the loop isn't hard to write

maybe we should expose absoluteParent instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96123

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

[PATCH] D96112: [Syntax] No crash on OpaqueValueExpr.

[Kirill Bobyrev via Phabricator via cfe-commits]

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

LGTM, thanks!

Sorry for the delay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96112

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

[PATCH] D96483: [flang][driver] Add options for unparsing

[Faris Rehman via Phabricator via cfe-commits]

FarisRehman added a comment.

Thanks for working on this @awarzynski
Could you please clarify the situation regarding `GetActionKindName` in 
`FrontendOptions.h` as it is currently not being used anywhere and this patch 
currently does not update that method with the 2 new cases.
Other than that, looks good to me!




Comment at: flang/test/lit.cfg.py:82
 
+if config.include_flang_new_driver_test:
+   tools.append(ToolSubst('%flang_fc1', command=FindTool('flang-new'),

[nit] Is there a reason to have this as a separate `if-else` instead of merging 
with the existing `if-else`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96483

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

[PATCH] D96178: [OpenCL] Create VoidPtrTy with generic AS in C++ for OpenCL mode

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96178

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

[PATCH] D96178: [OpenCL] Create VoidPtrTy with generic AS in C++ for OpenCL mode

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

FYI I have created `PR49191` for the issues with address spaces I have 
highlighted earlier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96178

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

[PATCH] D96597: [DebugInfo] Keep the DWARF64 flag in the module metadata

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added a comment.

Thank you, @dblaikie!




Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:396
+  bool Dwarf64 =
+  (Asm->TM.Options.MCOptions.Dwarf64 || MMI->getModule()->isDwarf64()) &&
+  DwarfVersion >= 3 &&   // DWARF64 was introduced in DWARFv3.

dblaikie wrote:
> Once this patch is accepted, it's probably worth going back and removing the 
> MCOption? (I don't think we have MCOptions for other debug related module 
> metadata, like the DWARF version?)
Exactly for the DWARF version, there is a similar option, please take a look at 
line 388.
As for the DWARF64 option here, it only simplifies testing with tools like 
`llc`, `opt`, etc. Originally, `MCTargetOptions::Dwarf64` was aimed to adjust 
generating in the assembler, but the corresponding command-line flag is visible 
in other tools, too. Do you think it is worth it to move the command line 
option from the common library code to specific tools?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96597

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

[PATCH] D95877: [analyzer] Fix static_cast on pointer-to-member handling

[David Zarzycki via Phabricator via cfe-commits]

davezarzycki added a comment.

The test seems to be broken on Fedora 33 (x86-64 with clang-11):

  XPASS: Clang :: Analysis/reinterpret-cast-pointer-to-member.cpp (6531 of 
74360)
   TEST 'Clang :: 
Analysis/reinterpret-cast-pointer-to-member.cpp' FAILED 
  Script:
  --
  : 'RUN: at line 1';   /tmp/_update_lc/r/bin/clang -cc1 -internal-isystem 
/tmp/_update_lc/r/lib/clang/13.0.0/include -nostdsysteminc -analyze 
-analyzer-constraints=range -setup-static-analyzer 
-analyzer-checker=core,debug.ExprInspection -verify 
/home/dave/ro_s/lp/clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp
  --
  Exit Code: 0


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95877

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

[PATCH] D96690: [clangd] Treat paths case-insensitively depending on the platform

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks for the important fix.

This contains a mixture of changing comparison-of-paths to be case insensitive, 
and case-normalizing stored paths so we can use case-sensitive comparisons.

I think we should stick to *only* changing comparison-of-path logic as much as 
possible, because:

- both approaches are ad-hoc and error prone - forgetting to 
fold/insensitive-compare will introduce a bug
- it's more obvious where to fix a sensitive-compare, and it has fewer 
unexpected effects
- in many configs we only ever see one case for a given file, case-folding can 
introduce bugs here where insensitive-comparison cannot
- there's a risk we won't be case-preserving, which can be annoying or really 
matter (e.g. on a mac volume that *is* case sensitive)




Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:202
 
+#if defined(_WIN32) || defined(__APPLE__)
+// When matching paths via regex on Windows/Apple they should be treated

I'm starting to think Path.h should export a CLANGD_PATH_INSENSITIVE macro, 
this condition is in a bunch of places. (particularly tests)

I nearly did this last time and came to the conclusion it wasn't quite enough 
:-\


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96690

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

[PATCH] D95877: [analyzer] Fix static_cast on pointer-to-member handling

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD added a comment.

In D95877#2563383 , @davezarzycki 
wrote:

> The test seems to be broken on Fedora 33 (x86-64 with clang-11):
>
>   XPASS: Clang :: Analysis/reinterpret-cast-pointer-to-member.cpp (6531 of 
> 74360)
>    TEST 'Clang :: 
> Analysis/reinterpret-cast-pointer-to-member.cpp' FAILED 
>   Script:
>   --
>   : 'RUN: at line 1';   /tmp/_update_lc/r/bin/clang -cc1 -internal-isystem 
> /tmp/_update_lc/r/lib/clang/13.0.0/include -nostdsysteminc -analyze 
> -analyzer-constraints=range -setup-static-analyzer 
> -analyzer-checker=core,debug.ExprInspection -verify 
> /home/dave/ro_s/lp/clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp
>   --
>   Exit Code: 0

Yes this has been fixed by @vsavchenko in 
https://github.com/llvm/llvm-project/commit/6f21adac6dd7082f7231ae342d40ed04f4885e79


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95877

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

[PATCH] D95877: [analyzer] Fix static_cast on pointer-to-member handling

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D95877#2563383 , @davezarzycki 
wrote:

> The test seems to be broken on Fedora 33 (x86-64 with clang-11):
>
>   XPASS: Clang :: Analysis/reinterpret-cast-pointer-to-member.cpp (6531 of 
> 74360)
>    TEST 'Clang :: 
> Analysis/reinterpret-cast-pointer-to-member.cpp' FAILED 
>   Script:
>   --
>   : 'RUN: at line 1';   /tmp/_update_lc/r/bin/clang -cc1 -internal-isystem 
> /tmp/_update_lc/r/lib/clang/13.0.0/include -nostdsysteminc -analyze 
> -analyzer-constraints=range -setup-static-analyzer 
> -analyzer-checker=core,debug.ExprInspection -verify 
> /home/dave/ro_s/lp/clang/test/Analysis/reinterpret-cast-pointer-to-member.cpp
>   --
>   Exit Code: 0

Hi there, is it happening after this commit?
https://github.com/llvm/llvm-project/commit/6f21adac6dd7082f7231ae342d40ed04f4885e79


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95877

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

[PATCH] D96709: Add Windows ehcont section support (/guard:ehcont).

[Pengfei Wang via Phabricator via cfe-commits]

pengfei created this revision.
pengfei added reviewers: arlosi, rnk, thakis, ajpaverd, theraven, pcc.
Herald added subscribers: dexonsmith, dang.
Herald added a reviewer: jansvoboda11.
pengfei requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Add option /guard:ehcont


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96709

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/test/CodeGen/cfguardtable.c
  clang/test/Driver/cl-options.c
  llvm/docs/ReleaseNotes.rst

Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -51,6 +51,9 @@
 
Makes programs 10x faster by doing Special New Thing.
 
+* Windows Control-flow Enforcement Technology: the ``-ehcontguard`` option now
+  emits valid unwind entrypoints which are validated when the context is being
+  set during exception handling.
 
 Changes to the LLVM IR
 --
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -614,6 +614,13 @@
 // RUN: %clang_cl /guard:nochecks -### -- %s 2>&1 | FileCheck -check-prefix=CFGUARDNOCHECKSINVALID %s
 // CFGUARDNOCHECKSINVALID: invalid value 'nochecks' in '/guard:'
 
+// RUN: %clang_cl  -### -- %s 2>&1 | FileCheck -check-prefix=NOEHCONTGUARD %s
+// RUN: %clang_cl /guard:ehcont- -### -- %s 2>&1 | FileCheck -check-prefix=NOEHCONTGUARD %s
+// NOEHCONTGUARD-NOT: -ehcontguard
+
+// RUN: %clang_cl /guard:ehcont -### -- %s 2>&1 | FileCheck -check-prefix=EHCONTGUARD %s
+// EHCONTGUARD: -ehcontguard
+
 // RUN: %clang_cl /guard:foo -### -- %s 2>&1 | FileCheck -check-prefix=CFGUARDINVALID %s
 // CFGUARDINVALID: invalid value 'foo' in '/guard:'
 
Index: clang/test/CodeGen/cfguardtable.c
===
--- clang/test/CodeGen/cfguardtable.c
+++ clang/test/CodeGen/cfguardtable.c
@@ -1,8 +1,10 @@
-// RUN: %clang_cc1 -cfguard-no-checks -emit-llvm %s -o - | FileCheck %s -check-prefix=CFGUARDNOCHECKS
-// RUN: %clang_cc1 -cfguard -emit-llvm %s -o - | FileCheck %s -check-prefix=CFGUARD
-
-void f() {}
-
-// Check that the cfguard metadata flag gets correctly set on the module.
-// CFGUARDNOCHECKS: !"cfguard", i32 1}
-// CFGUARD: !"cfguard", i32 2}
+// RUN: %clang_cc1 -cfguard-no-checks -emit-llvm %s -o - | FileCheck %s -check-prefix=CFGUARDNOCHECKS
+// RUN: %clang_cc1 -cfguard -emit-llvm %s -o - | FileCheck %s -check-prefix=CFGUARD
+// RUN: %clang_cc1 -ehcontguard -emit-llvm %s -o - | FileCheck %s -check-prefix=EHCONTGUARD
+
+void f() {}
+
+// Check that the cfguard metadata flag gets correctly set on the module.
+// CFGUARDNOCHECKS: !"cfguard", i32 1}
+// CFGUARD: !"cfguard", i32 2}
+// EHCONTGUARD: !"ehcontguard", i32 1}
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -496,6 +496,10 @@
   CmdArgs.push_back("-guard:cf");
 } else if (GuardArgs.equals_lower("cf-")) {
   CmdArgs.push_back("-guard:cf-");
+} else if (GuardArgs.equals_lower("ehcont")) {
+  CmdArgs.push_back("/guard:ehcont");
+} else if (GuardArgs.equals_lower("ehcont-")) {
+  CmdArgs.push_back("/guard:ehcont-");
 }
   }
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7023,14 +7023,19 @@
 
   if (Arg *A = Args.getLastArg(options::OPT__SLASH_guard)) {
 StringRef GuardArgs = A->getValue();
-// The only valid options are "cf", "cf,nochecks", and "cf-".
+// The only valid options are "cf", "cf,nochecks", "cf-", "ehcont" and
+// "ehcont-".
 if (GuardArgs.equals_lower("cf")) {
   // Emit CFG instrumentation and the table of address-taken functions.
   CmdArgs.push_back("-cfguard");
 } else if (GuardArgs.equals_lower("cf,nochecks")) {
   // Emit only the table of address-taken functions.
   CmdArgs.push_back("-cfguard-no-checks");
-} else if (GuardArgs.equals_lower("cf-")) {
+} else if (GuardArgs.equals_lower("ehcont")) {
+  // Emit EH continuation table.
+  CmdArgs.push_back("-ehcontguard");
+} else if (GuardArgs.equals_lower("cf-") ||
+   GuardArgs.equals_lower("ehcont-")) {
   // Do nothing, but we might want to emit a security warning in future.
 } else {
   D.Diag(diag::err_drv_invalid_value) << A->getSpelling() << GuardArgs;
Index: clang/lib/CodeGen/CodeGenModule.cpp
===

[PATCH] D96344: [flang][driver] Add options for -fdefault* and -flarge-sizes

[Arnamoy B via Phabricator via cfe-commits]

arnamoy10 updated this revision to Diff 323746.
arnamoy10 added a comment.

Addressed latest reviewer's comments, no longer using a pointer for 
`defaultKinds`


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

https://reviews.llvm.org/D96344

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/test/Flang-Driver/driver-help-hidden.f90
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Flang-Driver/fdefault.f90
  flang/test/Flang-Driver/pipeline.f90

Index: flang/test/Flang-Driver/pipeline.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/pipeline.f90
@@ -0,0 +1,17 @@
+! This file tests that flang-new forwards 
+! all Flang frontend options to flang-new -fc1
+! as expected.
+!
+! RUN: %flang-new -fsyntax-only -### %s -o %t 2>&1 \
+! RUN: -fdefault-double-8 \
+! RUN: -fdefault-integer-8 \
+! RUN: -fdefault-real-8 \
+! RUN: -flarge-sizes \
+! RUN:   | FileCheck %s
+!
+!
+! CHECK: "-fdefault-double-8"
+! CHECK: "-fdefault-integer-8"
+! CHECK: "-fdefault-real-8"
+! CHECK: "-flarge-sizes"
+
Index: flang/test/Flang-Driver/fdefault.f90
===
--- /dev/null
+++ flang/test/Flang-Driver/fdefault.f90
@@ -0,0 +1,26 @@
+! Ensure argument -fdefault* works as expected.
+! TODO: Add checks when actual codegen is possible for this family
+
+! REQUIRES: new-flang-driver
+
+!--
+! FLANG DRIVER (flang-new)
+!--
+! RUN: not %flang-new -fsyntax-only -fdefault-double-8 %s  2>&1 | FileCheck %s --check-prefix=DOUBLE
+
+!-
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!-
+! RUN: not %flang-new -fc1 -fsyntax-only -fdefault-double-8 %s  2>&1 | FileCheck %s --check-prefix=DOUBLE
+
+!-
+! EXPECTED OUTPUT FOR PROVIDING ONLY -fdefault-double-8 
+!-
+! DOUBLE:error: Use of `-fdefault-double-8` requires `-fdefault-real-8`
+
+PROGRAM test
+  implicit none
+  real :: x! note kind is not specified
+  x = 3.4
+  print *, "x = ", x
+END PROGRAM
Index: flang/test/Flang-Driver/driver-help.f90
===
--- flang/test/Flang-Driver/driver-help.f90
+++ flang/test/Flang-Driver/driver-help.f90
@@ -23,10 +23,14 @@
 ! HELP-NEXT: -D = Define  to  (or 1 if  omitted)
 ! HELP-NEXT: -E Only run the preprocessor
 ! HELP-NEXT: -fcolor-diagnosticsEnable colors in diagnostics
+! HELP-NEXT: -fdefault-double-8 Set the DOUBLE PRECISION type and double real constants like 1.d0 to an 8 byte wide type.
+! HELP-NEXT: -fdefault-integer-8Set the default integer and logical types to an 8 byte wide type.
+! HELP-NEXT: -fdefault-real-8   Set the default real type to an 8 byte wide type.
 ! HELP-NEXT: -ffixed-form   Process source files in fixed form
 ! HELP-NEXT: -ffixed-line-length=
 ! HELP-NEXT: Use  as character line width in fixed mode
 ! HELP-NEXT: -ffree-formProcess source files in free form
+! HELP-NEXT: -flarge-sizes  Set the default KIND for INTEGER to 8.
 ! HELP-NEXT: -fno-color-diagnostics Disable colors in diagnostics
 ! HELP-NEXT: -fopenacc  Enable OpenACC
 ! HELP-NEXT: -fopenmp   Parse OpenMP pragmas and generate parallel code.
@@ -46,10 +50,14 @@
 ! HELP-FC1-NEXT: -D = Define  to  (or 1 if  omitted)
 ! HELP-FC1-NEXT: -emit-obj Emit native object files
 ! HELP-FC1-NEXT: -E Only run the preprocessor
+! HELP-FC1-NEXT: -fdefault-double-8  Set the DOUBLE PRECISION type and double real constants like 1.d0 to an 8 byte wide type.
+! HELP-FC1-NEXT: -fdefault-integer-8 Set the default integer and logical types to an 8 byte wide type.
+! HELP-FC1-NEXT: -fdefault-real-8Set the default real type to an 8 byte wide type.
 ! HELP-FC1-NEXT: -ffixed-form   Process source files in fixed form
 ! HELP-FC1-NEXT: -ffixed-line-length=
 ! HELP-FC1-NEXT: Use  as character line width in fixed mode
 ! HELP-FC1-NEXT: -ffree-formProcess source files in free form
+! HELP-FC1-NEXT: -flarge-sizes  Set the default KIND for INTEGER to 8.
 ! HELP-FC1-NEXT: -fopenacc  Enable OpenACC
 ! HELP-FC1-NEXT: -fopenmp   Parse OpenMP pragmas and generate parallel code.
 ! HELP-FC1-NEXT: -help  Display available options
Index: flang/test/Flang-Driver/driver-help-hidden.f90
===
--- flang/test/Flang-Driver/driver-help-hidden.f90
+++ flang/test/Flang-Driver/driver-help-hidden.f90
@@ -23,10 +23,14 @@
 ! CHECK-NEXT: -D = Define  to  (or 1 if  omitted)
 ! CHECK-NEXT: -EOnly run the preprocessor
 

[PATCH] D96090: [analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

@ASDenysPetrov Could you please rebase this?
`git apply` does not seem to apply this patch on top of `llvm/main`.
If it depends on any parent revisions please document them as well.


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

https://reviews.llvm.org/D96090

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

[PATCH] D95912: [OpenMP] Attribute target diagnostics properly

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:1798
+  FunctionDecl *FD =
+  isa(C) ? cast(C) : dyn_cast(D);
   auto CheckType = [&](QualType Ty) {

So, we assume that lexical context is a function and if it is not, we assume 
that the value declaration being checked is a function. Can we actually break 
this assumption?
For example, will something like this work correctly:
```
struct S {
__float128 A = 1;
int bar = 2 * A;
};
#pragma omp declare target
  S s;
#pragma omp end declare target
```



Comment at: clang/lib/Sema/Sema.cpp:1835
   }
+  if (const auto *FPTy = dyn_cast(Ty))
+CheckType(FPTy->getReturnType());

I was a bit confused by `FPTy` variable name and already started writing a 
comment that we already checked a case `FunctionProtoType` case, then I 
realized that it is a check for Function*No*Prototype :) . Can we change FPTy 
to something closer to `FunctionNoProtoType` ?



Comment at: clang/test/OpenMP/nvptx_unsupported_type_messages.cpp:42
 #ifndef _ARCH_PPC
-// expected-note@+1 {{'boo' defined here}}
+// expected-error@+2 {{'boo' requires 128 bit size '__float128' type support, 
but device 'nvptx64-unknown-unknown' does not support it}}
+// expected-note@+1 2{{'boo' defined here}}

So,  `boo` is diagnosed two times, at the point where it actually used and 
where it is defined, it looks a bit like a duplication isn't it? I think it 
makes sense to diagnose only usage point with note pointing to the definition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95912

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

[PATCH] D96483: [flang][driver] Add options for unparsing

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski updated this revision to Diff 323756.
awarzynski added a comment.

Updated lit.cfg.py


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96483

Files:
  clang/include/clang/Driver/Options.td
  flang/include/flang/Frontend/FrontendActions.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/lib/Frontend/CMakeLists.txt
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/FrontendOptions.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/lib/Parser/CMakeLists.txt
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Parser/continuation-in-if.f
  flang/test/Parser/pp-dir-comments.f90
  flang/test/Semantics/canondo01.f90
  flang/test/Semantics/canondo02.f90
  flang/test/Semantics/canondo03.f90
  flang/test/Semantics/canondo04.f90
  flang/test/Semantics/canondo05.f90
  flang/test/Semantics/critical04.f90
  flang/test/Semantics/defined-ops.f90
  flang/test/Semantics/doconcurrent02.f90
  flang/test/Semantics/doconcurrent03.f90
  flang/test/Semantics/doconcurrent04.f90
  flang/test/Semantics/doconcurrent07.f90
  flang/test/Semantics/label02.f90
  flang/test/Semantics/label03.f90
  flang/test/Semantics/label04.f90
  flang/test/Semantics/label05.f90
  flang/test/Semantics/label06.f90
  flang/test/Semantics/label07.f90
  flang/test/Semantics/label08.f90
  flang/test/Semantics/label09.f90
  flang/test/Semantics/label10.f90
  flang/test/Semantics/label12.f90
  flang/test/Semantics/label13.f90
  flang/test/Semantics/label15.f90
  flang/test/lit.cfg.py
  flang/tools/f18/f18.cpp

Index: flang/tools/f18/f18.cpp
===
--- flang/tools/f18/f18.cpp
+++ flang/tools/f18/f18.cpp
@@ -538,9 +538,10 @@
   options.instrumentedParse = true;
 } else if (arg == "-fdebug-no-semantics") {
   driver.debugNoSemantics = true;
-} else if (arg == "-funparse") {
+} else if (arg == "-funparse" || arg == "-fdebug-unparse") {
   driver.dumpUnparse = true;
-} else if (arg == "-funparse-with-symbols") {
+} else if (arg == "-funparse-with-symbols" ||
+arg == "-fdebug-unparse-with-symbols") {
   driver.dumpUnparseWithSymbols = true;
 } else if (arg == "-funparse-typed-exprs-to-f18-fc") {
   driver.unparseTypedExprsToF18_FC = true;
Index: flang/test/lit.cfg.py
===
--- flang/test/lit.cfg.py
+++ flang/test/lit.cfg.py
@@ -74,10 +74,15 @@
 if config.include_flang_new_driver_test:
tools.append(ToolSubst('%flang-new', command=FindTool('flang-new'), unresolved='fatal'))
tools.append(ToolSubst('%flang', command=FindTool('flang-new'), unresolved='fatal'))
+   tools.append(ToolSubst('%flang_fc1', command=FindTool('flang-new'),
+extra_args=['-fc1'], unresolved='fatal'))
 else:
tools.append(ToolSubst('%flang', command=FindTool('f18'),
 extra_args=["-intrinsic-module-directory "+config.flang_intrinsic_modules_dir],
 unresolved='fatal'))
+   tools.append(ToolSubst('%flang_fc1', command=FindTool('f18'),
+extra_args=["-intrinsic-module-directory "+config.flang_intrinsic_modules_dir],
+unresolved='fatal'))
 
 if config.flang_standalone_build:
 llvm_config.add_tool_substitutions(tools, [config.flang_llvm_tools_dir])
Index: flang/test/Semantics/label15.f90
===
--- flang/test/Semantics/label15.f90
+++ flang/test/Semantics/label15.f90
@@ -1,4 +1,4 @@
-! RUN: %f18 -funparse %s 2>&1 | FileCheck %s
+! RUN: %flang_fc1 -fdebug-unparse %s 2>&1 | FileCheck %s
 
 !CHECK-NOT: error:
 module mm
Index: flang/test/Semantics/label13.f90
===
--- flang/test/Semantics/label13.f90
+++ flang/test/Semantics/label13.f90
@@ -1,4 +1,4 @@
-! RUN: %f18 -funparse-with-symbols %s 2>&1 | FileCheck %s
+! RUN: %flang_fc1 -fdebug-unparse-with-symbols %s 2>&1 | FileCheck %s
 ! CHECK: branch into loop body from outside
 ! CHECK: the loop branched into
 
Index: flang/test/Semantics/label12.f90
===
--- flang/test/Semantics/label12.f90
+++ flang/test/Semantics/label12.f90
@@ -1,4 +1,4 @@
-! RUN: not %f18 -funparse-with-symbols %s 2>&1 | FileCheck %s
+! RUN: not %flang_fc1 -fdebug-unparse-with-symbols %s 2>&1 | FileCheck %s
 ! CHECK: expected end of statement
 
 subroutine s
Index: flang/test/Semantics/label10.f90
===
--- flang/test/Semantics/label10.f90
+++ flang/test/Semantics/label10.f90
@@ -1,4 +1,4 @@
-! RUN: not %f18 -funparse-with-symbols %s 2>&1 | FileCheck %s
+! RUN: not %flang_fc1 -fdebug-unparse-with-symbols %s 2>&1 | FileCheck %s
 ! CHECK: '60' not a FORMAT
 ! CHECK: data transfer use of '60'
 
Index: flang/test/Semantics/label09.f90
=

[PATCH] D96483: [flang][driver] Add options for unparsing

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

In D96483#2563323 , @FarisRehman wrote:

> Do you know what the use of `GetActionKindName` in `FrontendOptions.h` is, as 
> it is currently not being called anywhere and whilst this patch currently 
> does not update that method with the 2 new cases, even if it did no change 
> would be visible.

I'm not sure why this method is there TBH :) Perhaps @CarolineConcatto would 
know? I suspect that C&P error. I suggest removing it in a separate NFC patch. 
Thank you for pointing that out!




Comment at: flang/test/lit.cfg.py:82
 
+if config.include_flang_new_driver_test:
+   tools.append(ToolSubst('%flang_fc1', command=FindTool('flang-new'),

FarisRehman wrote:
> [nit] Is there a reason to have this as a separate `if-else` instead of 
> merging with the existing `if-else`
Good point, updated!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96483

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

[PATCH] D96716: [flang][driver] Add debug dump options

[Faris Rehman via Phabricator via cfe-commits]

FarisRehman created this revision.
Herald added a reviewer: sscalpone.
Herald added a subscriber: dang.
Herald added a reviewer: awarzynski.
Herald added a reviewer: jansvoboda11.
FarisRehman requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add the following options:

- -fdebug-dump-symbols
- -fdebug-dump-parse-tree
- -fdebug-dump-provenance

Summary of changes:

- Add 3 new frontend actions: DebugDumpSymbolsAction, DebugDumpParseTreeAction 
and DebugDumpProvenanceAction
- Add a unique pointer to the Semantics instance created in PrescanAndSemaAction
- Move fatal semantic error reporting to its own method, 
FrontendActions#reportFatalSemanticErrors
- Port most tests using `-fdebug-dump-symbols` and `-fdebug-dump-parse-tree` to 
the new driver if built, otherwise default to f18


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96716

Files:
  clang/include/clang/Driver/Options.td
  flang/include/flang/Frontend/FrontendActions.h
  flang/include/flang/Frontend/FrontendOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Flang-Driver/debug-provenance.f90
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Semantics/data05.f90
  flang/test/Semantics/data08.f90
  flang/test/Semantics/data09.f90
  flang/test/Semantics/offsets01.f90
  flang/test/Semantics/offsets02.f90
  flang/test/Semantics/offsets03.f90
  flang/test/Semantics/resolve100.f90
  flang/test/Semantics/rewrite01.f90

Index: flang/test/Semantics/rewrite01.f90
===
--- flang/test/Semantics/rewrite01.f90
+++ flang/test/Semantics/rewrite01.f90
@@ -1,4 +1,4 @@
-! RUN: %f18 -fdebug-dump-parse-tree %s 2>&1 | FileCheck %s
+! RUN: %flang_fc1 -fdebug-dump-parse-tree %s 2>&1 | FileCheck %s
 ! Ensure that READ(CVAR) [, item-list] is corrected when CVAR is a
 ! character variable so as to be a formatted read from the default
 ! unit, not an unformatted read from an internal unit (which is not
Index: flang/test/Semantics/resolve100.f90
===
--- flang/test/Semantics/resolve100.f90
+++ flang/test/Semantics/resolve100.f90
@@ -1,4 +1,4 @@
-!RUN: %f18 -fdebug-dump-symbols %s | FileCheck %s
+!RUN: %flang_fc1 -fdebug-dump-symbols %s | FileCheck %s
 
 program p
   ! CHECK: a size=4 offset=0: ObjectEntity type: LOGICAL(4)
Index: flang/test/Semantics/offsets03.f90
===
--- flang/test/Semantics/offsets03.f90
+++ flang/test/Semantics/offsets03.f90
@@ -1,4 +1,4 @@
-!RUN: %f18 -fdebug-dump-symbols %s | FileCheck %s
+!RUN: %flang_fc1 -fdebug-dump-symbols %s | FileCheck %s
 
 ! Size and alignment with EQUIVALENCE and COMMON
 
Index: flang/test/Semantics/offsets02.f90
===
--- flang/test/Semantics/offsets02.f90
+++ flang/test/Semantics/offsets02.f90
@@ -1,4 +1,4 @@
-!RUN: %f18 -fdebug-dump-symbols %s | FileCheck %s
+!RUN: %flang_fc1 -fdebug-dump-symbols %s | FileCheck %s
 
 ! Size and alignment of derived types
 
Index: flang/test/Semantics/offsets01.f90
===
--- flang/test/Semantics/offsets01.f90
+++ flang/test/Semantics/offsets01.f90
@@ -1,4 +1,4 @@
-!RUN: %f18 -fdebug-dump-symbols %s | FileCheck %s
+!RUN: %flang_fc1 -fdebug-dump-symbols %s | FileCheck %s
 
 ! Size and alignment of intrinsic types
 subroutine s1
Index: flang/test/Semantics/data09.f90
===
--- flang/test/Semantics/data09.f90
+++ flang/test/Semantics/data09.f90
@@ -1,4 +1,4 @@
-! RUN: %f18 -fsyntax-only -fdebug-dump-symbols %s 2>&1 | FileCheck %s
+! RUN: %flang_fc1 -fdebug-dump-symbols %s 2>&1 | FileCheck %s
 ! CHECK: init:[INTEGER(4)::1065353216_4,1073741824_4,1077936128_4,1082130432_4]
 ! Verify that the closure of EQUIVALENCE'd symbols with any DATA
 ! initialization produces a combined initializer.
Index: flang/test/Semantics/data08.f90
===
--- flang/test/Semantics/data08.f90
+++ flang/test/Semantics/data08.f90
@@ -1,4 +1,4 @@
-! RUN: %f18 -fdebug-dump-symbols %s 2>&1 | FileCheck %s
+! RUN: %flang_fc1 -fdebug-dump-symbols %s 2>&1 | FileCheck %s
 ! CHECK: DATA statement value initializes 'jx' of type 'INTEGER(4)' with CHARACTER
 ! CHECK: DATA statement value initializes 'jy' of type 'INTEGER(4)' with CHARACTER
 ! CHECK: DATA statement value initializes 'jz' of type 'INTEGER(4)' with CHARACTER
Index: flang/test/Semantics/data05.f90
===
--- flang/test/Semantics/data05.f90
+++ flang/test/Semantics/data05.f90
@@ -1,4 +1,4 @@
-!RUN: %f18 -fdebug-dump-symbols %s | FileCheck %s
+!RUN: %flang_fc1 -fdebug-dump-symbols %s | FileCheck %s
 mod

[PATCH] D96483: [flang][driver] Add options for unparsing

[Faris Rehman via Phabricator via cfe-commits]

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

Thanks for clarifying


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96483

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

[PATCH] D95790: [clang][cli] Documentation of CompilerInvocation parsing/generation

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 323766.
jansvoboda11 added a comment.

Fix typos, add section heading above the list of marshalling classes, add new 
section (high-level overview of adding new command line option)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95790

Files:
  clang/docs/InternalsManual.rst

Index: clang/docs/InternalsManual.rst
===
--- clang/docs/InternalsManual.rst
+++ clang/docs/InternalsManual.rst
@@ -572,6 +572,349 @@
 The Frontend library contains functionality useful for building tools on top of
 the Clang libraries, for example several methods for outputting diagnostics.
 
+Compiler Invocation
+---
+
+One of the classes provided by the Frontend library is ``CompilerInvocation``,
+which holds information that describe current invocation of the Clang frontend.
+The information typically comes from the command line constructed by the Clang
+driver, or directly from clients performing custom initialization. The data
+structure is split into logical units used by different parts of the compiler,
+for example ``PreprocessorOptions``, ``LanguageOptions`` or ``CodeGenOptions``.
+
+Command Line Interface
+--
+
+The command line interface of the Clang ``-cc1`` frontend is defined alongside
+the driver options in ``clang/Driver/Options.td``. The information making up an
+option definition includes the name and prefix (for example ``-std=``), form and
+position of the option value, help text, aliases and more. Each option may
+belong to a certain group and can be marked with zero or more flags. Options
+accepted by the ``-cc1`` frontend are marked with the ``CC1Option`` flag.
+
+Command Line Parsing
+
+
+Option definitions are processed by the ``-gen-opt-parser-defs`` tablegen
+backend, transformed into a list of invocations of the ``OPTION`` macro and
+stored in ``clang/Driver/Driver.inc``. This file is then used to create instance
+of ``llvm::opt::OptTable``, which acts as a command line preprocessor. The
+preprocessed command line is stored in ``llvm::opt::ArgList``, an object that
+provides an API for performing simple queries on the contents of the command
+line.
+
+Finally, the ``CompilerInvocation::CreateFromArgs`` function is responsible for
+the actual parsing of command line arguments. It maps the contents of the
+``ArgList`` onto fields of ``CompilerInvocation``, normalizing the values in the
+process.
+
+Command Line Generation
+---
+
+Any valid ``CompilerInvocation`` created from a ``-cc1`` command line  can be
+also serialized back into semantically equivalent command line in a
+deterministic manner. This enables features such as implicitly discovered,
+explicitly built modules.
+
+..
+  TODO: Create and link corresponding section in Modules.rst.
+
+.. _OptionMarshalling:
+
+Option Marshalling Infrastructure
+-
+
+The obvious way to parse command line arguments is to write the code that
+queries ``ArgList`` and constructs ``CompilerInvocation`` manually. However,
+given that behavior of most command line options is the same, this approach
+would result in lots of repetitive code. This is the reason why most of the
+code for parsing and generating arguments is automatically generated from the
+``Marshalling`` annotations that are part of the option definition in the
+tablegen file. This section goes through the details of the automatic
+marshalling system.
+
+To read and modify contents of ``CompilerInvocation``, the marshalling system
+uses key paths, which are declared in two steps. First, a tablegen definition
+for the ``CompilerInvocation`` member is created by inheriting from
+``KeyPathAndMacro``:
+
+.. code-block::
+
+  // Options.td
+
+  class LangOpts : KeyPathAndMacro<"LangOpts->", field, "LANG_"> {}
+  //   CompilerInvocation member  ^^
+  //OPTION_WITH_MARSHALLING prefix ^
+
+The first argument to the parent class is the beginning of the key path that
+references the ``CompilerInvocation`` member. This argument ends with ``->`` if
+the member is a pointer type or with ``.`` if it's a value type. The child class
+takes a single parameter ``field`` that is forwarded as the second argument to
+the base class. The child class can then be used like so:
+``LangOpts<"IgnoreExceptions">``, constructing a key path to the field
+``LangOpts->IgnoreExceptions``. The third argument passed to the parent class is
+a string that the tablegen backend uses as a prefix to the
+``OPTION_WITH_MARSHALLING`` macro. Using the key path then instructs the backend
+to generate the following code:
+
+.. code-block:: c++
+
+  // Options.inc
+
+  #ifdef LANG_OPTION_WITH_MARSHALLING
+  LANG_OPTION_WITH_MARSHALLING([...], LangOpts->IgnoreExceptions, [...])
+  #endif // LANG_O

[PATCH] D95790: [clang][cli] Documentation of CompilerInvocation parsing/generation

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

@Bigcheese I have addressed your feedback with the latest update. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95790

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

[PATCH] D96344: [flang][driver] Add options for -fdefault* and -flarge-sizes

[Tim Keith via Phabricator via cfe-commits]

tskeith added inline comments.



Comment at: clang/include/clang/Driver/Options.td:4231
+def flarge_sizes : Flag<["-"],"flarge-sizes">, Group, 
+  HelpText<"Set the default KIND for INTEGER to 8.">;
 }

That's not what -flarge-sizes does. Here is the description from 
flang/docs/Extensions.md:

> The default `INTEGER` type is required by the standard to occupy
> the same amount of storage as the default `REAL` type.  Default
> `REAL` is of course 32-bit IEEE-754 floating-point today.  This legacy
> rule imposes an artificially small constraint in some cases
> where Fortran mandates that something have the default `INTEGER`
> type: specifically, the results of references to the intrinsic functions
> `SIZE`, `STORAGE_SIZE`,`LBOUND`, `UBOUND`, `SHAPE`, and the location 
> reductions
> `FINDLOC`, `MAXLOC`, and `MINLOC` in the absence of an explicit
> `KIND=` actual argument.  We return `INTEGER(KIND=8)` by default in
> these cases when the `-flarge-sizes` option is enabled.
> `SIZEOF` and `C_SIZEOF` always return `INTEGER(KIND=8)`.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:269
+}
+res.defaultKinds().set_defaultRealKind(4);
+  }

If `-fdefault-double-8` requires `-fdefault-real-8` then why is the default 
real kind set to 4?

I don't understand the purpose of this option. The kind of DOUBLE PRECISION is 
already 8 so there is no need to change it.


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

https://reviews.llvm.org/D96344

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

[PATCH] D96717: [clangd] Bind outgoing calls through LSPBinder too. NFC

[Sam McCall via Phabricator via cfe-commits]

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

The redundancy around work-done-progress is annoying but ok for now.

There's a weirdness with context lifetimes around outgoing method calls, which
I've preserved to keep this NFC. We should probably fix it though.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96717

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/LSPBinder.h
  clang-tools-extra/clangd/Module.h
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
  clang-tools-extra/clangd/unittests/LSPBinderTests.cpp

Index: clang-tools-extra/clangd/unittests/LSPBinderTests.cpp
===
--- clang-tools-extra/clangd/unittests/LSPBinderTests.cpp
+++ clang-tools-extra/clangd/unittests/LSPBinderTests.cpp
@@ -15,12 +15,15 @@
 namespace clangd {
 namespace {
 
+using testing::ElementsAre;
 using testing::HasSubstr;
+using testing::IsEmpty;
 using testing::UnorderedElementsAre;
 
 // JSON-serializable type for testing.
 struct Foo {
   int X;
+  friend bool operator==(Foo A, Foo B) { return A.X == B.X; }
 };
 bool fromJSON(const llvm::json::Value &V, Foo &F, llvm::json::Path P) {
   return fromJSON(V, F.X, P.field("X"));
@@ -35,9 +38,31 @@
   return [&Out](llvm::Expected V) { Out.emplace(std::move(V)); };
 }
 
+struct OutgoingRecorder : public LSPBinder::RawOutgoing {
+  llvm::StringMap> Received;
+
+  void callMethod(llvm::StringRef Method, llvm::json::Value Params,
+  Callback Reply) override {
+Received[Method].push_back(Params);
+if (Method == "fail")
+  return Reply(error("Params={0}", Params));
+Reply(Params); // echo back the request
+  }
+  void notify(llvm::StringRef Method, llvm::json::Value Params) override {
+Received[Method].push_back(std::move(Params));
+  }
+
+  std::vector take(llvm::StringRef Method) {
+std::vector Result = Received.lookup(Method);
+Received.erase(Method);
+return Result;
+  }
+};
+
 TEST(LSPBinderTest, IncomingCalls) {
   LSPBinder::RawHandlers RawHandlers;
-  LSPBinder Binder{RawHandlers};
+  OutgoingRecorder RawOutgoing;
+  LSPBinder Binder{RawHandlers, RawOutgoing};
   struct Handler {
 void plusOne(const Foo &Params, Callback Reply) {
   Reply(Foo{Params.X + 1});
@@ -94,7 +119,45 @@
   RawCmdPlusOne(1, capture(Reply));
   ASSERT_TRUE(Reply.hasValue());
   EXPECT_THAT_EXPECTED(Reply.getValue(), llvm::HasValue(2));
+
+  // None of this generated any outgoing traffic.
+  EXPECT_THAT(RawOutgoing.Received, IsEmpty());
+}
+
+TEST(LSPBinderTest, OutgoingCalls) {
+  LSPBinder::RawHandlers RawHandlers;
+  OutgoingRecorder RawOutgoing;
+  LSPBinder Binder{RawHandlers, RawOutgoing};
+
+  LSPBinder::OutgoingMethod Echo;
+  Binder.outgoingMethod("echo", Echo);
+  LSPBinder::OutgoingMethod WrongSignature;
+  Binder.outgoingMethod("wrongSignature", WrongSignature);
+  LSPBinder::OutgoingMethod Fail;
+  Binder.outgoingMethod("fail", Fail);
+
+  llvm::Optional> Reply;
+  Echo(Foo{2}, capture(Reply));
+  EXPECT_THAT(RawOutgoing.take("echo"), ElementsAre(llvm::json::Value(2)));
+  ASSERT_TRUE(Reply.hasValue());
+  EXPECT_THAT_EXPECTED(Reply.getValue(), llvm::HasValue(Foo{2}));
+
+  // JSON response is integer, can't be parsed as string.
+  llvm::Optional> WrongTypeReply;
+  WrongSignature(Foo{2}, capture(WrongTypeReply));
+  EXPECT_THAT(RawOutgoing.take("wrongSignature"),
+  ElementsAre(llvm::json::Value(2)));
+  ASSERT_TRUE(Reply.hasValue());
+  EXPECT_THAT_EXPECTED(WrongTypeReply.getValue(),
+   llvm::FailedWithMessage(
+   HasSubstr("failed to decode wrongSignature reply")));
+
+  Fail(Foo{2}, capture(Reply));
+  EXPECT_THAT(RawOutgoing.take("fail"), ElementsAre(llvm::json::Value(2)));
+  ASSERT_TRUE(Reply.hasValue());
+  EXPECT_THAT_EXPECTED(Reply.getValue(), llvm::FailedWithMessage("Params=2"));
 }
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -23,6 +23,7 @@
 namespace clang {
 namespace clangd {
 namespace {
+using testing::ElementsAre;
 
 MATCHER_P(DiagMessage, M, "") {
   if (const auto *O = arg.getAsObject()) {
@@ -223,13 +224,18 @@
 
 TEST_F(LSPTest, ModulesTest) {
   class MathModule : public Module {
+OutgoingNotification Changed;
 void initializeLSP(LSPBinder &Bind, const ClientCapabilities &ClientCaps,
llvm::json::Object &ServerCaps) override {
   Bind.notification("add", this, &MathModul

[PATCH] D96719: [clang-tidy] Add new check 'bugprone-thread-canceltype-asynchronous' and alias 'cert-pos47-c'.

[Balázs Kéri via Phabricator via cfe-commits]

balazske created this revision.
Herald added subscribers: martong, gamesh411, Szelethus, dkrupp, xazax.hun, 
whisperity, mgorny.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Simple check, code based on 'bugprone-bad-signal-to-kill-thread'.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96719

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/ThreadCanceltypeAsynchronousCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ThreadCanceltypeAsynchronousCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-thread-canceltype-asynchronous.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-pos47-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-thread-canceltype-asynchronous.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-thread-canceltype-asynchronous.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-thread-canceltype-asynchronous.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s bugprone-thread-canceltype-asynchronous %t
+
+#define PTHREAD_CANCEL_DEFERRED 0
+#define PTHREAD_CANCEL_ASYNCHRONOUS 1
+
+int pthread_setcanceltype(int type, int *oldtype);
+
+int main() {
+  int result, oldtype;
+
+  if ((result = pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, &oldtype)) != 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: asynchronous cancelability type should not be used [bugprone-thread-canceltype-asynchronous]
+return 1;
+  }
+
+  if ((result = pthread_setcanceltype(PTHREAD_CANCEL_DEFERRED, &oldtype)) != 0) {
+return 1;
+  }
+
+  return 0;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -95,6 +95,7 @@
`bugprone-suspicious-string-compare `_, "Yes"
`bugprone-swapped-arguments `_, "Yes"
`bugprone-terminating-continue `_, "Yes"
+   `bugprone-thread-canceltype-asynchronous `_, "No"
`bugprone-throw-keyword-missing `_,
`bugprone-too-small-loop-variable `_,
`bugprone-undefined-memory-manipulation `_,
@@ -334,6 +335,7 @@
`cert-oop11-cpp `_, `performance-move-constructor-init `_, "Yes"
`cert-oop54-cpp `_, `bugprone-unhandled-self-assignment `_,
`cert-pos44-c `_, `bugprone-bad-signal-to-kill-thread `_,
+   `cert-pos47-c `_, `bugprone-thread-canceltype-asynchronous `_,
`cert-str34-c `_, `bugprone-signed-char-misuse `_,
`clang-analyzer-core.CallAndMessage `_, `Clang Static Analyzer `_,
`clang-analyzer-core.DivideZero `_, `Clang Static Analyzer `_,
Index: clang-tools-extra/docs/clang-tidy/checks/cert-pos47-c.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cert-pos47-c.rst
@@ -0,0 +1,9 @@
+.. title:: clang-tidy - cert-pos47-c
+.. meta::
+   :http-equiv=refresh: 5;URL=bugprone-thread-canceltype-asynchronous.html
+
+cert-pos47-c
+
+
+The cert-pos47-c check is an alias, please see
+`bugprone-thread-canceltype-asynchronous `_ for more information.
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-thread-canceltype-asynchronous.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-thread-canceltype-asynchronous.rst
@@ -0,0 +1,19 @@
+.. title:: clang-tidy - bugprone-thread-canceltype-asynchronous
+
+bugprone-thread-canceltype-asynchronous
+===
+
+  Finds ``pthread_setcanceltype`` function calls where a thread's cancellation
+  type is set to asynchronous (``PTHREAD_CANCEL_ASYNCHRONOUS``). Setting this
+  type to asynchronous is generally unsafe, use type ``PTHREAD_CANCEL_DEFERRED``
+  instead which is the default. Even with deferred cancellation, a cancellation
+  point in an asynchronous signal handler may still be acted upon and the effect
+  is as if it was an asynchronous cancellation.
+
+.. code-block: c++
+
+pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, &oldtype);
+
+This check corresponds to the CERT C Coding Standard rule
+`POS47-C. Do not use threads that can be canceled asynchronously
+`_.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,7 +67,18 @@
 Improvement

[PATCH] D95754: [clang] Print 32 candidates on the first failure, with -fshow-overloads=best.

[Justin Lebar via Phabricator via cfe-commits]

jlebar added a comment.

Thank you for your comments, Aaron!




Comment at: clang/lib/Sema/Sema.cpp:2310
 
+  S.Diags.noteNumOverloadCandidatesShown(ShownOverloads);
+

aaronpuchert wrote:
> Why not in the following `if`? I assume we want to show a long list not 
> necessarily once, but only if it comes with the first error?
My intent was to show the long list once, even if it's not the very first 
error.  My thought process:

All things being equal, it's better to show more information to the user than 
less.  The problem is, at some point, the amount of information we show becomes 
overwhelming and spammy.  Particularly problematic are multiline errors, 
because then you get O(nm) error lines across the whole TU.  We prevent the 
O(nm) overwhelm by limiting the number of lines a particular error can produce 
(using the mechanism in question here, or the template backtrace limit, etc), 
and then also limiting the total number of individual errors before we stop 
printing those.

With this change, we display the full(ish) error the first time it occurs and 
then the truncated error every other time.  So in total it's O(n + m) rather 
than O(nm).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95754

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

[PATCH] D96572: [Clang][ASan] Introduce `-fsanitize-address-destructor-kind=` driver & frontend option.

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D96572#2561216 , @delcypher wrote:

> @jansvoboda11 I noticed that the `Options.td` file is making use of some 
> fancy mixins that mean the option gets automatically marshalled by the 
> frontend into the codegen options :). I tried this out using the patch to 
> this patch and all my tests seem to pass. Should I be using these mixins (I 
> don't know how stable they are because this looks like a new feature). Am I 
> using them properly? It seems that the driver still has to parse the option 
> manually (which is actually what I want) but the frontend automagically 
> parses the option and modifies the right codegen option for me.

Hi, yes, this is a new feature. The option generation is not being enforced in 
any way yet, but the automatic marshalling should work. You can test it by 
running your `clang -cc1` commands with the `-round-trip-args` flag (or `clang` 
with `-Xclang -round-trip-args`). With this option, the original `cc1` command 
line is first parsed into a dummy `CompilerInvocation`, which is used to 
generate new command line. The new command line is what is then used to create 
the real `CompilerInvocation`.

As you've correctly discovered, this does not interact with the driver in any 
way.

I'll be sending an email to cfe-dev about this shortly. Your marshalling code 
seems correct to me. If something does not work, check out the patch with 
documentation D95790  and feel free to let me 
know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96572

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

[PATCH] D95808: [test] Use host platform specific error message substitution in lit tests - continued

[Denys Petrov via Phabricator via cfe-commits]

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

Hi, @abhina.sreeskantharajan.
I've just updated my repo and got this patch. After your changes I caught a 
test fail specifically in my domain of Clang Static Analyzer.
Here is output of `Analysis/taint-generic.c` F15537807: output_taint_test.txt 

Could you please check it and try to fix?
While testing I'm using build on //Win10 + GCC// config.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95808

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

[PATCH] D95912: [OpenMP] Attribute target diagnostics properly

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Thanks for taking a look!




Comment at: clang/lib/Sema/Sema.cpp:1798
+  FunctionDecl *FD =
+  isa(C) ? cast(C) : dyn_cast(D);
   auto CheckType = [&](QualType Ty) {

Fznamznon wrote:
> So, we assume that lexical context is a function and if it is not, we assume 
> that the value declaration being checked is a function. Can we actually break 
> this assumption?
> For example, will something like this work correctly:
> ```
> struct S {
> __float128 A = 1;
> int bar = 2 * A;
> };
> #pragma omp declare target
>   S s;
> #pragma omp end declare target
> ```
This is diagnosed neither with nor without the patch.
I'm not sure we should either because 2 * A is a constant expression.
We can make the example more complex using a constructor but I'd say that is 
not related to this patch either way.



Comment at: clang/test/OpenMP/nvptx_unsupported_type_messages.cpp:42
 #ifndef _ARCH_PPC
-// expected-note@+1 {{'boo' defined here}}
+// expected-error@+2 {{'boo' requires 128 bit size '__float128' type support, 
but device 'nvptx64-unknown-unknown' does not support it}}
+// expected-note@+1 2{{'boo' defined here}}

Fznamznon wrote:
> So,  `boo` is diagnosed two times, at the point where it actually used and 
> where it is defined, it looks a bit like a duplication isn't it? I think it 
> makes sense to diagnose only usage point with note pointing to the definition.
It is diagnosed twice, right. I don't think this is a problem of this patch 
though, see for example line 54 on the left where we emit the boo error 4 times 
before. Also, take a look at 181 and following in this file. With this patch we 
reduce the number of times the error is emitted from 3 to 1. It's a bit of a 
mixed bag but I think this makes it better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95912

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

[PATCH] D95912: [OpenMP] Attribute target diagnostics properly

[Jon Chesterfield via Phabricator via cfe-commits]

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

I agree this is an improvement on the current status and should land.

Reading through it has made me nervous about our current semantic checking. In 
particular, I'm not sure suppressing error messages from code that ultimately 
proves to be discarded as the same as not parsing code that ultimately was not 
required, but that design choice seems to have been made a long time ago. 
Presumably lazy diagnostics had better engineering properties than lazy parsing 
and codegen - I'd be interested to see docs/emails about the early design 
choices for single source target offloading if someone has them on hand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95912

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

[PATCH] D96407: [flang][driver] Add extension options and -finput-charset

[Andrzej Warzynski via Phabricator via cfe-commits]

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

Thank you for addressing my comments. LGTM!

Please add missing `newline` chars before merging ;-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96407

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

[PATCH] D95928: [OpenMP] Delay more diagnostics of potentially non-emitted code

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:18345
   if (LangOpts.OpenMPIsDevice) {
+// In OpenMP device mode we will not emit host only functions, or functions
+// we don't need due to their linkage.

What catches a static function whose address is taken?



Comment at: clang/lib/Sema/SemaDecl.cpp:18376
+
+  if (Final && LangOpts.OpenMP && !LangOpts.CUDA)
+return FunctionEmissionStatus::Emitted;

Does LangOpts.HIP imply LandOpts.CUDA? I think it is intended to. I think I've 
seen this pattern (is openmp and is not cuda) elsewhere, perhaps we're missing 
a predicate.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95928

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

[PATCH] D96612: [clangd] Improve printing of Objective-C categories and methods

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/AST.cpp:224
 
+  if (const auto *C = dyn_cast(&ND))
+return printObjCContainer(*C);

I'm not sure this fits with the contract of printName, which really is to print 
the name of the symbol rather than describe it.

Examples:
 - the title of a hovercard for a method will be "instance-method `-[foo]`" 
instead of "instance-method `foo`", which is inconsistent with c/c++
 - the title of a hovercard for a category will be "extension 
`PotentiallyLongClassName(CatName)`" instead of `extension ClassName`
 - the documentSymbol response will include the extra details in the `name` 
field rather than the `detail` field

We don't actually populate `detail` in documentSymbol, so maybe we do need a 
separate function that gives that level of detail.
Though I wonder whether it should mosty just print the decl (e.g. `class X{};`, 
`@implementation PotentiallyLongClassName(CatName)` etc. WDYT?

See also https://github.com/clangd/clangd/issues/520 
https://github.com/clangd/clangd/issues/601 though both appear to have stalled.



Comment at: clang-tools-extra/clangd/AST.cpp:228
+Out << (Method->isInstanceMethod() ? '-' : '+');
+Method->getSelector().print(Out);
+return Out.str();

in the other patch this was with `-[brackets]`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96612

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

[PATCH] D96608: [clangd] Delay binding LSP methods until initialize. NFC

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:165
   return false;
-if (!Server.Server) {
-  elog("Notification {0} before initialization", Method);
-} else if (Method == "$/cancelRequest") {
+if (Method == "$/cancelRequest") {
   onCancel(std::move(Params));

kadircet wrote:
> this one is a functional change though , previously we would say 
> `notification X before initialization` for cancellations that arrived before 
> init, now we are going to say "bad cancellation request". not that it is bad, 
> but it makes me wonder if it is intentional :D
Oops, yes - unintentional.
(now the structure of onNotify and onCall looks more similar, thanks!)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96608

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

[PATCH] D96608: [clangd] Delay binding LSP methods until initialize. NFC

[Sam McCall via Phabricator via cfe-commits]

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

address comments & rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96608

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


Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -168,6 +168,7 @@
   void applyEdit(WorkspaceEdit WE, llvm::json::Value Success,
  Callback Reply);
 
+  void bindMethods(LSPBinder &);
   std::vector getFixes(StringRef File, const clangd::Diagnostic &D);
 
   /// Checks if completion request should be ignored. We need this due to the
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -165,19 +165,17 @@
 log("<-- {0}", Method);
 if (Method == "exit")
   return false;
-if (!Server.Server) {
+auto Handler = Server.Handlers.NotificationHandlers.find(Method);
+if (Handler != Server.Handlers.NotificationHandlers.end()) {
+  Handler->second(std::move(Params));
+  Server.maybeExportMemoryProfile();
+  Server.maybeCleanupMemory();
+} else if (!Server.Server) {
   elog("Notification {0} before initialization", Method);
 } else if (Method == "$/cancelRequest") {
   onCancel(std::move(Params));
 } else {
-  auto Handler = Server.Handlers.NotificationHandlers.find(Method);
-  if (Handler != Server.Handlers.NotificationHandlers.end()) {
-Handler->second(std::move(Params));
-Server.maybeExportMemoryProfile();
-Server.maybeCleanupMemory();
-  } else {
-log("unhandled notification {0}", Method);
-  }
+  log("unhandled notification {0}", Method);
 }
 return true;
   }
@@ -191,18 +189,16 @@
 SPAN_ATTACH(Tracer, "Params", Params);
 ReplyOnce Reply(ID, Method, &Server, Tracer.Args);
 log("<-- {0}({1})", Method, ID);
-if (!Server.Server && Method != "initialize") {
+auto Handler = Server.Handlers.MethodHandlers.find(Method);
+if (Handler != Server.Handlers.MethodHandlers.end()) {
+  Handler->second(std::move(Params), std::move(Reply));
+} else if (!Server.Server) {
   elog("Call {0} before initialization.", Method);
   Reply(llvm::make_error("server not initialized",
ErrorCode::ServerNotInitialized));
 } else {
-  auto Handler = Server.Handlers.MethodHandlers.find(Method);
-  if (Handler != Server.Handlers.MethodHandlers.end()) {
-Handler->second(std::move(Params), std::move(Reply));
-  } else {
-Reply(llvm::make_error("method not found",
- ErrorCode::MethodNotFound));
-  }
+  Reply(llvm::make_error("method not found",
+   ErrorCode::MethodNotFound));
 }
 return true;
   }
@@ -583,6 +579,7 @@
 
   {
 LSPBinder Binder(Handlers);
+bindMethods(Binder);
 if (Opts.Modules)
   for (auto &Mod : *Opts.Modules)
 Mod.initializeLSP(Binder, Params.capabilities, ServerCaps);
@@ -1457,10 +1454,12 @@
 this->Opts.ContextProvider = ClangdServer::createConfiguredContextProvider(
 Opts.ConfigProvider, this);
   }
-
-  // clang-format off
   LSPBinder Bind(this->Handlers);
   Bind.method("initialize", this, &ClangdLSPServer::onInitialize);
+}
+
+void ClangdLSPServer::bindMethods(LSPBinder &Bind) {
+  // clang-format off
   Bind.notification("initialized", this, &ClangdLSPServer::onInitialized);
   Bind.method("shutdown", this, &ClangdLSPServer::onShutdown);
   Bind.method("sync", this, &ClangdLSPServer::onSync);


Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -168,6 +168,7 @@
   void applyEdit(WorkspaceEdit WE, llvm::json::Value Success,
  Callback Reply);
 
+  void bindMethods(LSPBinder &);
   std::vector getFixes(StringRef File, const clangd::Diagnostic &D);
 
   /// Checks if completion request should be ignored. We need this due to the
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -165,19 +165,17 @@
 log("<-- {0}", Method);
 if (Method == "exit")
   return false;
-if (!Server.Server) {
+auto Handler = Server.Handlers.NotificationHandlers.find(Method);
+if (Handler != Server.Handlers.Notificatio

[clang-tools-extra] 6c5f17e - [clangd] Delay binding LSP methods until initialize. NFC

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-02-15T19:33:40+01:00
New Revision: 6c5f17e701ff586d69a43b3cfc1e25314b84892d

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

LOG: [clangd] Delay binding LSP methods until initialize. NFC

This is NFC because the MessageHandler refused to dispatch to them until the
server is initialized anyway.

This is a more natural time to bind them - it's when they become callable, and
it's when client capabalities are available and server ones can be set.

One module-lifecycle function will be responsible for all three.

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

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 0f5fe7c3528c..c30890f14787 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -165,19 +165,17 @@ class ClangdLSPServer::MessageHandler : public 
Transport::MessageHandler {
 log("<-- {0}", Method);
 if (Method == "exit")
   return false;
-if (!Server.Server) {
+auto Handler = Server.Handlers.NotificationHandlers.find(Method);
+if (Handler != Server.Handlers.NotificationHandlers.end()) {
+  Handler->second(std::move(Params));
+  Server.maybeExportMemoryProfile();
+  Server.maybeCleanupMemory();
+} else if (!Server.Server) {
   elog("Notification {0} before initialization", Method);
 } else if (Method == "$/cancelRequest") {
   onCancel(std::move(Params));
 } else {
-  auto Handler = Server.Handlers.NotificationHandlers.find(Method);
-  if (Handler != Server.Handlers.NotificationHandlers.end()) {
-Handler->second(std::move(Params));
-Server.maybeExportMemoryProfile();
-Server.maybeCleanupMemory();
-  } else {
-log("unhandled notification {0}", Method);
-  }
+  log("unhandled notification {0}", Method);
 }
 return true;
   }
@@ -191,18 +189,16 @@ class ClangdLSPServer::MessageHandler : public 
Transport::MessageHandler {
 SPAN_ATTACH(Tracer, "Params", Params);
 ReplyOnce Reply(ID, Method, &Server, Tracer.Args);
 log("<-- {0}({1})", Method, ID);
-if (!Server.Server && Method != "initialize") {
+auto Handler = Server.Handlers.MethodHandlers.find(Method);
+if (Handler != Server.Handlers.MethodHandlers.end()) {
+  Handler->second(std::move(Params), std::move(Reply));
+} else if (!Server.Server) {
   elog("Call {0} before initialization.", Method);
   Reply(llvm::make_error("server not initialized",
ErrorCode::ServerNotInitialized));
 } else {
-  auto Handler = Server.Handlers.MethodHandlers.find(Method);
-  if (Handler != Server.Handlers.MethodHandlers.end()) {
-Handler->second(std::move(Params), std::move(Reply));
-  } else {
-Reply(llvm::make_error("method not found",
- ErrorCode::MethodNotFound));
-  }
+  Reply(llvm::make_error("method not found",
+   ErrorCode::MethodNotFound));
 }
 return true;
   }
@@ -583,6 +579,7 @@ void ClangdLSPServer::onInitialize(const InitializeParams 
&Params,
 
   {
 LSPBinder Binder(Handlers);
+bindMethods(Binder);
 if (Opts.Modules)
   for (auto &Mod : *Opts.Modules)
 Mod.initializeLSP(Binder, Params.capabilities, ServerCaps);
@@ -1457,10 +1454,12 @@ ClangdLSPServer::ClangdLSPServer(class Transport 
&Transp,
 this->Opts.ContextProvider = ClangdServer::createConfiguredContextProvider(
 Opts.ConfigProvider, this);
   }
-
-  // clang-format off
   LSPBinder Bind(this->Handlers);
   Bind.method("initialize", this, &ClangdLSPServer::onInitialize);
+}
+
+void ClangdLSPServer::bindMethods(LSPBinder &Bind) {
+  // clang-format off
   Bind.notification("initialized", this, &ClangdLSPServer::onInitialized);
   Bind.method("shutdown", this, &ClangdLSPServer::onShutdown);
   Bind.method("sync", this, &ClangdLSPServer::onSync);

diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.h 
b/clang-tools-extra/clangd/ClangdLSPServer.h
index 9207fe7e034f..659d7668591b 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -168,6 +168,7 @@ class ClangdLSPServer : private ClangdServer::Callbacks {
   void applyEdit(WorkspaceEdit WE, llvm::json::Value Success,
  Callback Reply);
 
+  void bindMethods(LSPBinder &);
   std::vector getFixes(StringRef File, const clangd::Diagnostic &D);
 
   /// Checks if completion request should be ignored. We need this due to the


  

[PATCH] D96608: [clangd] Delay binding LSP methods until initialize. NFC

[Sam McCall 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 rG6c5f17e701ff: [clangd] Delay binding LSP methods until 
initialize. NFC (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96608

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


Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -168,6 +168,7 @@
   void applyEdit(WorkspaceEdit WE, llvm::json::Value Success,
  Callback Reply);
 
+  void bindMethods(LSPBinder &);
   std::vector getFixes(StringRef File, const clangd::Diagnostic &D);
 
   /// Checks if completion request should be ignored. We need this due to the
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -165,19 +165,17 @@
 log("<-- {0}", Method);
 if (Method == "exit")
   return false;
-if (!Server.Server) {
+auto Handler = Server.Handlers.NotificationHandlers.find(Method);
+if (Handler != Server.Handlers.NotificationHandlers.end()) {
+  Handler->second(std::move(Params));
+  Server.maybeExportMemoryProfile();
+  Server.maybeCleanupMemory();
+} else if (!Server.Server) {
   elog("Notification {0} before initialization", Method);
 } else if (Method == "$/cancelRequest") {
   onCancel(std::move(Params));
 } else {
-  auto Handler = Server.Handlers.NotificationHandlers.find(Method);
-  if (Handler != Server.Handlers.NotificationHandlers.end()) {
-Handler->second(std::move(Params));
-Server.maybeExportMemoryProfile();
-Server.maybeCleanupMemory();
-  } else {
-log("unhandled notification {0}", Method);
-  }
+  log("unhandled notification {0}", Method);
 }
 return true;
   }
@@ -191,18 +189,16 @@
 SPAN_ATTACH(Tracer, "Params", Params);
 ReplyOnce Reply(ID, Method, &Server, Tracer.Args);
 log("<-- {0}({1})", Method, ID);
-if (!Server.Server && Method != "initialize") {
+auto Handler = Server.Handlers.MethodHandlers.find(Method);
+if (Handler != Server.Handlers.MethodHandlers.end()) {
+  Handler->second(std::move(Params), std::move(Reply));
+} else if (!Server.Server) {
   elog("Call {0} before initialization.", Method);
   Reply(llvm::make_error("server not initialized",
ErrorCode::ServerNotInitialized));
 } else {
-  auto Handler = Server.Handlers.MethodHandlers.find(Method);
-  if (Handler != Server.Handlers.MethodHandlers.end()) {
-Handler->second(std::move(Params), std::move(Reply));
-  } else {
-Reply(llvm::make_error("method not found",
- ErrorCode::MethodNotFound));
-  }
+  Reply(llvm::make_error("method not found",
+   ErrorCode::MethodNotFound));
 }
 return true;
   }
@@ -583,6 +579,7 @@
 
   {
 LSPBinder Binder(Handlers);
+bindMethods(Binder);
 if (Opts.Modules)
   for (auto &Mod : *Opts.Modules)
 Mod.initializeLSP(Binder, Params.capabilities, ServerCaps);
@@ -1457,10 +1454,12 @@
 this->Opts.ContextProvider = ClangdServer::createConfiguredContextProvider(
 Opts.ConfigProvider, this);
   }
-
-  // clang-format off
   LSPBinder Bind(this->Handlers);
   Bind.method("initialize", this, &ClangdLSPServer::onInitialize);
+}
+
+void ClangdLSPServer::bindMethods(LSPBinder &Bind) {
+  // clang-format off
   Bind.notification("initialized", this, &ClangdLSPServer::onInitialized);
   Bind.method("shutdown", this, &ClangdLSPServer::onShutdown);
   Bind.method("sync", this, &ClangdLSPServer::onSync);


Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -168,6 +168,7 @@
   void applyEdit(WorkspaceEdit WE, llvm::json::Value Success,
  Callback Reply);
 
+  void bindMethods(LSPBinder &);
   std::vector getFixes(StringRef File, const clangd::Diagnostic &D);
 
   /// Checks if completion request should be ignored. We need this due to the
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -165,19 +165,17 @@
 log("<-- {0}", Method);
 if (Method == "exit")
   return false;
-if (!Server.Server) {
+auto Handler =

[PATCH] D96163: [analyzer] Add 12.0.0. release notes

[Daniel via Phabricator via cfe-commits]

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

LGTM on my part.


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

https://reviews.llvm.org/D96163

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

[PATCH] D96705: [clang][cli] Add explicit round-trip test

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96705

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

[PATCH] D96725: [clang-tidy] Fix modernize-use-using in extern C code

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh.
Herald added a subscriber: xazax.hun.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The check currently erroneously flags typedefs in extern "C" blocks.
Addresses https://bugs.llvm.org/show_bug.cgi?id=49181


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96725

Files:
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
@@ -302,3 +302,10 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef'
   // CHECK-FIXES: using b = InjectedClassNameWithUnnamedArgument;
 };
+
+extern "C" {
+typedef int CType;
+typedef struct {
+  int b;
+} CStruct;
+}
\ No newline at end of file
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -16,6 +16,10 @@
 namespace tidy {
 namespace modernize {
 
+AST_MATCHER(Decl, isInExternC) {
+  return Node.getDeclContext()->isExternCContext();
+}
+
 UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
@@ -25,8 +29,10 @@
 }
 
 void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(typedefDecl(unless(isInstantiated())).bind("typedef"),
- this);
+  Finder->addMatcher(
+  typedefDecl(unless(isInstantiated()), unless(isInExternC()))
+  .bind("typedef"),
+  this);
   // This matcher used to find tag declarations in source code within typedefs.
   // They appear in the AST just *prior* to the typedefs.
   Finder->addMatcher(tagDecl(unless(isImplicit())).bind("tagdecl"), this);


Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
@@ -302,3 +302,10 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef'
   // CHECK-FIXES: using b = InjectedClassNameWithUnnamedArgument;
 };
+
+extern "C" {
+typedef int CType;
+typedef struct {
+  int b;
+} CStruct;
+}
\ No newline at end of file
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -16,6 +16,10 @@
 namespace tidy {
 namespace modernize {
 
+AST_MATCHER(Decl, isInExternC) {
+  return Node.getDeclContext()->isExternCContext();
+}
+
 UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
@@ -25,8 +29,10 @@
 }
 
 void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(typedefDecl(unless(isInstantiated())).bind("typedef"),
- this);
+  Finder->addMatcher(
+  typedefDecl(unless(isInstantiated()), unless(isInExternC()))
+  .bind("typedef"),
+  this);
   // This matcher used to find tag declarations in source code within typedefs.
   // They appear in the AST just *prior* to the typedefs.
   Finder->addMatcher(tagDecl(unless(isImplicit())).bind("tagdecl"), this);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D96725: [clang-tidy] Fix modernize-use-using in extern C code

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 323796.
njames93 added a comment.

Newline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96725

Files:
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
@@ -302,3 +302,10 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef'
   // CHECK-FIXES: using b = InjectedClassNameWithUnnamedArgument;
 };
+
+extern "C" {
+typedef int CType;
+typedef struct {
+  int b;
+} CStruct;
+}
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -16,6 +16,10 @@
 namespace tidy {
 namespace modernize {
 
+AST_MATCHER(Decl, isInExternC) {
+  return Node.getDeclContext()->isExternCContext();
+}
+
 UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
@@ -25,8 +29,10 @@
 }
 
 void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(typedefDecl(unless(isInstantiated())).bind("typedef"),
- this);
+  Finder->addMatcher(
+  typedefDecl(unless(isInstantiated()), unless(isInExternC()))
+  .bind("typedef"),
+  this);
   // This matcher used to find tag declarations in source code within typedefs.
   // They appear in the AST just *prior* to the typedefs.
   Finder->addMatcher(tagDecl(unless(isImplicit())).bind("tagdecl"), this);


Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
@@ -302,3 +302,10 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef'
   // CHECK-FIXES: using b = InjectedClassNameWithUnnamedArgument;
 };
+
+extern "C" {
+typedef int CType;
+typedef struct {
+  int b;
+} CStruct;
+}
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -16,6 +16,10 @@
 namespace tidy {
 namespace modernize {
 
+AST_MATCHER(Decl, isInExternC) {
+  return Node.getDeclContext()->isExternCContext();
+}
+
 UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
@@ -25,8 +29,10 @@
 }
 
 void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(typedefDecl(unless(isInstantiated())).bind("typedef"),
- this);
+  Finder->addMatcher(
+  typedefDecl(unless(isInstantiated()), unless(isInExternC()))
+  .bind("typedef"),
+  this);
   // This matcher used to find tag declarations in source code within typedefs.
   // They appear in the AST just *prior* to the typedefs.
   Finder->addMatcher(tagDecl(unless(isImplicit())).bind("tagdecl"), this);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95912: [OpenMP] Attribute target diagnostics properly

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 323799.
jdoerfert marked an inline comment as done.
jdoerfert added a comment.

Rename variable as requested


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95912

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/nvptx_unsupported_type_messages.cpp

Index: clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
===
--- clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
+++ clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
@@ -39,10 +39,12 @@
 };
 
 #ifndef _ARCH_PPC
-// expected-note@+1 {{'boo' defined here}}
+// expected-error@+2 {{'boo' requires 128 bit size '__float128' type support, but device 'nvptx64-unknown-unknown' does not support it}}
+// expected-note@+1 2{{'boo' defined here}}
 void boo(__float128 A) { return; }
 #else
-// expected-note@+1 {{'boo' defined here}}
+// expected-error@+2 {{'boo' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
+// expected-note@+1 2{{'boo' defined here}}
 void boo(long double A) { return; }
 #endif
 #pragma omp declare target
@@ -51,10 +53,11 @@
 void foo(T a = T()) {
   a = a + f; // expected-note {{called by 'foo'}}
 #ifndef _ARCH_PPC
-// expected-error@+4 {{'boo' requires 128 bit size '__float128' type support, but device 'nvptx64-unknown-unknown' does not support it}}
+// expected-error@+5 {{'boo' requires 128 bit size '__float128' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 #else
-// expected-error@+2 {{'boo' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
+// expected-error@+3 {{'boo' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 #endif
+// expected-note@+1 {{called by 'foo'}}
   boo(0);
   return;
 }
@@ -98,28 +101,49 @@
   a = &b;
 }
 
-// TODO: We should diagnose the return type and argument type here.
+// expected-note@+2 {{'ld_return1a' defined here}}
+// expected-error@+1 {{'ld_return1a' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 long double ld_return1a() { return 0; }
+// expected-note@+2 {{'ld_arg1a' defined here}}
+// expected-error@+1 {{'ld_arg1a' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 void ld_arg1a(long double ld) {}
 
 // TODO: We should diagnose the return type and argument type here.
 typedef long double ld_ty;
+// expected-note@+2 {{'ld_return1b' defined here}}
+// expected-error@+1 {{'ld_return1b' requires 128 bit size 'ld_ty' (aka 'long double') type support, but device 'nvptx64-unknown-unknown' does not support it}}
 ld_ty ld_return1b() { return 0; }
+// expected-note@+2 {{'ld_arg1b' defined here}}
+// expected-error@+1 {{'ld_arg1b' requires 128 bit size 'ld_ty' (aka 'long double') type support, but device 'nvptx64-unknown-unknown' does not support it}}
 void ld_arg1b(ld_ty ld) {}
 
+// TODO: These errors should not be emitted.
+// expected-note@+2 {{'ld_return1c' defined here}}
+// expected-error@+1 {{'ld_return1c' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 static long double ld_return1c() { return 0; }
+// expected-note@+2 {{'ld_arg1c' defined here}}
+// expected-error@+1 {{'ld_arg1c' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 static void ld_arg1c(long double ld) {}
 
+// TODO: These errors should not be emitted.
+// expected-note@+2 {{'ld_return1d' defined here}}
+// expected-error@+1 {{'ld_return1d' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 inline long double ld_return1d() { return 0; }
+// expected-note@+2 {{'ld_arg1d' defined here}}
+// expected-error@+1 {{'ld_arg1d' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 inline void ld_arg1d(long double ld) {}
 
+// expected-error@+2 {{'ld_return1e' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 // expected-note@+1 {{'ld_return1e' defined here}}
 static long double ld_return1e() { return 0; }
+// expected-error@+2 {{'ld_arg1e' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 // expected-note@+1 {{'ld_arg1e' defined here}}
 static void ld_arg1e(long double ld) {}
 
+// expected-error@+2 {{'ld_return1f' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 // expected-note@+1 {{'ld_return1f' def

[PATCH] D95928: [OpenMP] Delay more diagnostics of potentially non-emitted code

[Johannes Doerfert via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaDecl.cpp:18345
   if (LangOpts.OpenMPIsDevice) {
+// In OpenMP device mode we will not emit host only functions, or functions
+// we don't need due to their linkage.

JonChesterfield wrote:
> What catches a static function whose address is taken?
I don't know where but the existing logic in `GetGVALinkageForFunction` seems 
to handle this just fine. I marked the tests I introduced for such cases below.



Comment at: clang/lib/Sema/SemaDecl.cpp:18376
+
+  if (Final && LangOpts.OpenMP && !LangOpts.CUDA)
+return FunctionEmissionStatus::Emitted;

JonChesterfield wrote:
> Does LangOpts.HIP imply LandOpts.CUDA? I think it is intended to. I think 
> I've seen this pattern (is openmp and is not cuda) elsewhere, perhaps we're 
> missing a predicate.
> 
I don't know. This is a reinterpretation of the conditional we had, which 
checked this in a different way but the intention is the same. There we also 
never looked for HIP, IIRC.



Comment at: clang/test/OpenMP/nvptx_unsupported_type_messages.cpp:164
+// expected-note@+1 {{called by 'external'}}
   void *p6 = reinterpret_cast(&ld_use4);
 }

Here we test static/inline function with taken address.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95928

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

[clang] 02413b0 - [CMake] Delete LLVM_RUNTIME_BUILD_ID_LINK_TARGETS

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2021-02-15T11:06:23-08:00
New Revision: 02413b097e72a3aab17e0504af135a95c0d300a1

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

LOG: [CMake] Delete LLVM_RUNTIME_BUILD_ID_LINK_TARGETS

Announcement: 
https://lists.llvm.org/pipermail/llvm-dev/2021-February/148446.html

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

Added: 


Modified: 
clang/cmake/caches/Fuchsia-stage2.cmake
llvm/runtimes/CMakeLists.txt

Removed: 
llvm/runtimes/llvm-strip-link.in



diff  --git a/clang/cmake/caches/Fuchsia-stage2.cmake 
b/clang/cmake/caches/Fuchsia-stage2.cmake
index 973629e09da1..1c14d2fec404 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -249,7 +249,6 @@ endif()
 
 set(LLVM_BUILTIN_TARGETS "${BUILTIN_TARGETS}" CACHE STRING "")
 set(LLVM_RUNTIME_TARGETS "${RUNTIME_TARGETS}" CACHE STRING "")
-set(LLVM_RUNTIME_BUILD_ID_LINK_TARGETS "${RUNTIME_BUILD_ID_LINK}" CACHE STRING 
"")
 
 # Setup toolchain.
 set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")

diff  --git a/llvm/runtimes/CMakeLists.txt b/llvm/runtimes/CMakeLists.txt
index 243d6ceb2202..1681cd959e07 100644
--- a/llvm/runtimes/CMakeLists.txt
+++ b/llvm/runtimes/CMakeLists.txt
@@ -187,14 +187,6 @@ foreach(entry ${runtimes})
   list(APPEND runtime_names ${projName})
 endforeach()
 
-if(LLVM_RUNTIME_BUILD_ID_LINK_TARGETS)
-  configure_file(
-${CMAKE_CURRENT_SOURCE_DIR}/llvm-strip-link.in
-${CMAKE_CURRENT_BINARY_DIR}/llvm-strip-link
-@ONLY
-  )
-endif()
-
 function(runtime_default_target)
   cmake_parse_arguments(ARG "" "" "DEPENDS;PREFIXES" ${ARGN})
 
@@ -329,10 +321,6 @@ function(runtime_register_target name target)
 list(APPEND ${name}_extra_args 
-DLLVM_ENABLE_RUNTIMES=${LLVM_ENABLE_RUNTIMES_PASSTHROUGH})
   endif()
 
-  if(target IN_LIST LLVM_RUNTIME_BUILD_ID_LINK_TARGETS)
-list(APPEND EXTRA_ARGS STRIP_TOOL 
${CMAKE_CURRENT_BINARY_DIR}/llvm-strip-link)
-  endif()
-
   llvm_ExternalProject_Add(runtimes-${name}
${CMAKE_CURRENT_SOURCE_DIR}/../../runtimes
DEPENDS ${${name}_deps}

diff  --git a/llvm/runtimes/llvm-strip-link.in 
b/llvm/runtimes/llvm-strip-link.in
deleted file mode 100755
index a7b8c567faf6..
--- a/llvm/runtimes/llvm-strip-link.in
+++ /dev/null
@@ -1,27 +0,0 @@
-#!/usr/bin/env python
-# -*- coding: utf-8 -*-
-
-import os
-import sys
-import subprocess
-
-
-ELF_MAGIC = '\x7fELF'
-
-with open(sys.argv[1], "rb") as f:
-buf = f.read(len(ELF_MAGIC))
-if buf != ELF_MAGIC:
-sys.exit(0)
-
-llvm_objcopy = os.path.join('@LLVM_RUNTIME_OUTPUT_INTDIR@', 'llvm-objcopy')
-install_dir = os.path.join(os.getenv('DESTDIR', ''), '@CMAKE_INSTALL_PREFIX@')
-link_dir = os.path.join(install_dir, 'lib', 'debug', '.build-id')
-
-sys.exit(subprocess.call([
-llvm_objcopy,
-'--strip-all',
-'--build-id-link-dir=' + link_dir,
-'--build-id-link-input=.debug',
-'--build-id-link-output=',
-sys.argv[1],
-]))



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

[PATCH] D96360: [CMake] Delete LLVM_RUNTIME_BUILD_ID_LINK_TARGETS

[Fangrui Song 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 rG02413b097e72: [CMake] Delete 
LLVM_RUNTIME_BUILD_ID_LINK_TARGETS (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96360

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  llvm/runtimes/CMakeLists.txt
  llvm/runtimes/llvm-strip-link.in


Index: llvm/runtimes/llvm-strip-link.in
===
--- llvm/runtimes/llvm-strip-link.in
+++ /dev/null
@@ -1,27 +0,0 @@
-#!/usr/bin/env python
-# -*- coding: utf-8 -*-
-
-import os
-import sys
-import subprocess
-
-
-ELF_MAGIC = '\x7fELF'
-
-with open(sys.argv[1], "rb") as f:
-buf = f.read(len(ELF_MAGIC))
-if buf != ELF_MAGIC:
-sys.exit(0)
-
-llvm_objcopy = os.path.join('@LLVM_RUNTIME_OUTPUT_INTDIR@', 'llvm-objcopy')
-install_dir = os.path.join(os.getenv('DESTDIR', ''), '@CMAKE_INSTALL_PREFIX@')
-link_dir = os.path.join(install_dir, 'lib', 'debug', '.build-id')
-
-sys.exit(subprocess.call([
-llvm_objcopy,
-'--strip-all',
-'--build-id-link-dir=' + link_dir,
-'--build-id-link-input=.debug',
-'--build-id-link-output=',
-sys.argv[1],
-]))
Index: llvm/runtimes/CMakeLists.txt
===
--- llvm/runtimes/CMakeLists.txt
+++ llvm/runtimes/CMakeLists.txt
@@ -187,14 +187,6 @@
   list(APPEND runtime_names ${projName})
 endforeach()
 
-if(LLVM_RUNTIME_BUILD_ID_LINK_TARGETS)
-  configure_file(
-${CMAKE_CURRENT_SOURCE_DIR}/llvm-strip-link.in
-${CMAKE_CURRENT_BINARY_DIR}/llvm-strip-link
-@ONLY
-  )
-endif()
-
 function(runtime_default_target)
   cmake_parse_arguments(ARG "" "" "DEPENDS;PREFIXES" ${ARGN})
 
@@ -329,10 +321,6 @@
 list(APPEND ${name}_extra_args 
-DLLVM_ENABLE_RUNTIMES=${LLVM_ENABLE_RUNTIMES_PASSTHROUGH})
   endif()
 
-  if(target IN_LIST LLVM_RUNTIME_BUILD_ID_LINK_TARGETS)
-list(APPEND EXTRA_ARGS STRIP_TOOL 
${CMAKE_CURRENT_BINARY_DIR}/llvm-strip-link)
-  endif()
-
   llvm_ExternalProject_Add(runtimes-${name}
${CMAKE_CURRENT_SOURCE_DIR}/../../runtimes
DEPENDS ${${name}_deps}
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -249,7 +249,6 @@
 
 set(LLVM_BUILTIN_TARGETS "${BUILTIN_TARGETS}" CACHE STRING "")
 set(LLVM_RUNTIME_TARGETS "${RUNTIME_TARGETS}" CACHE STRING "")
-set(LLVM_RUNTIME_BUILD_ID_LINK_TARGETS "${RUNTIME_BUILD_ID_LINK}" CACHE STRING 
"")
 
 # Setup toolchain.
 set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")


Index: llvm/runtimes/llvm-strip-link.in
===
--- llvm/runtimes/llvm-strip-link.in
+++ /dev/null
@@ -1,27 +0,0 @@
-#!/usr/bin/env python
-# -*- coding: utf-8 -*-
-
-import os
-import sys
-import subprocess
-
-
-ELF_MAGIC = '\x7fELF'
-
-with open(sys.argv[1], "rb") as f:
-buf = f.read(len(ELF_MAGIC))
-if buf != ELF_MAGIC:
-sys.exit(0)
-
-llvm_objcopy = os.path.join('@LLVM_RUNTIME_OUTPUT_INTDIR@', 'llvm-objcopy')
-install_dir = os.path.join(os.getenv('DESTDIR', ''), '@CMAKE_INSTALL_PREFIX@')
-link_dir = os.path.join(install_dir, 'lib', 'debug', '.build-id')
-
-sys.exit(subprocess.call([
-llvm_objcopy,
-'--strip-all',
-'--build-id-link-dir=' + link_dir,
-'--build-id-link-input=.debug',
-'--build-id-link-output=',
-sys.argv[1],
-]))
Index: llvm/runtimes/CMakeLists.txt
===
--- llvm/runtimes/CMakeLists.txt
+++ llvm/runtimes/CMakeLists.txt
@@ -187,14 +187,6 @@
   list(APPEND runtime_names ${projName})
 endforeach()
 
-if(LLVM_RUNTIME_BUILD_ID_LINK_TARGETS)
-  configure_file(
-${CMAKE_CURRENT_SOURCE_DIR}/llvm-strip-link.in
-${CMAKE_CURRENT_BINARY_DIR}/llvm-strip-link
-@ONLY
-  )
-endif()
-
 function(runtime_default_target)
   cmake_parse_arguments(ARG "" "" "DEPENDS;PREFIXES" ${ARGN})
 
@@ -329,10 +321,6 @@
 list(APPEND ${name}_extra_args -DLLVM_ENABLE_RUNTIMES=${LLVM_ENABLE_RUNTIMES_PASSTHROUGH})
   endif()
 
-  if(target IN_LIST LLVM_RUNTIME_BUILD_ID_LINK_TARGETS)
-list(APPEND EXTRA_ARGS STRIP_TOOL ${CMAKE_CURRENT_BINARY_DIR}/llvm-strip-link)
-  endif()
-
   llvm_ExternalProject_Add(runtimes-${name}
${CMAKE_CURRENT_SOURCE_DIR}/../../runtimes
DEPENDS ${${name}_deps}
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -249,7 +249,6 @@
 
 set(LLVM_BUILTIN_TARGETS "${BUILTIN_TARGETS}" CACHE STRING "")
 set(LLVM_RUNTIME_TARGETS "${RUNTIME_TARGETS}"

[PATCH] D95928: [OpenMP] Delay more diagnostics of potentially non-emitted code

[Jon Chesterfield via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95928

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

[clang] 3b2f19d - [OpenMP][NFC] Pre-commit test changes regarding PR48933

[Johannes Doerfert via cfe-commits]

Author: Johannes Doerfert
Date: 2021-02-15T13:16:44-06:00
New Revision: 3b2f19d0bc2803697526191a8a607efa0b38f7e4

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

LOG: [OpenMP][NFC] Pre-commit test changes regarding PR48933

This will highlight the effective changes in subsequent commits.

Reviewed By: ABataev

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

Added: 


Modified: 
clang/test/OpenMP/nvptx_unsupported_type_messages.cpp

Removed: 




diff  --git a/clang/test/OpenMP/nvptx_unsupported_type_messages.cpp 
b/clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
index 814a4756c01b..0601728caefe 100644
--- a/clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
+++ b/clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
@@ -77,11 +77,135 @@ T1 bar1() {
 void baz1() {
   T1 t = bar1();
 }
+
+// TODO: We should not emit an error for dead functions we do not emit.
+inline void dead_inline_declare_target() {
+// expected-note@+1 {{'b' defined here}}
+  long double *a, b = 0;
+// expected-error@+1 {{'b' requires 128 bit size 'long double' type support, 
but device 'nvptx64-unknown-unknown' does not support it}}
+  a = &b;
+}
+// TODO: We should not emit an error for dead functions we do not emit.
+static void dead_static_declare_target() {
+// expected-note@+1 {{'b' defined here}}
+  long double *a, b = 0;
+// expected-error@+1 {{'b' requires 128 bit size 'long double' type support, 
but device 'nvptx64-unknown-unknown' does not support it}}
+  a = &b;
+}
+template
+void dead_template_declare_target() {
+  long double *a, b = 0;
+  a = &b;
+}
+
+// TODO: We should diagnose the return type and argument type here.
+long double ld_return1a() { return 0; }
+void ld_arg1a(long double ld) {}
+
+// TODO: We should diagnose the return type and argument type here.
+typedef long double ld_ty;
+ld_ty ld_return1b() { return 0; }
+void ld_arg1b(ld_ty ld) {}
+
+static long double ld_return1c() { return 0; }
+static void ld_arg1c(long double ld) {}
+
+inline long double ld_return1d() { return 0; }
+inline void ld_arg1d(long double ld) {}
+
+// expected-note@+1 {{'ld_return1e' defined here}}
+static long double ld_return1e() { return 0; }
+// expected-note@+1 {{'ld_arg1e' defined here}}
+static void ld_arg1e(long double ld) {}
+
+// expected-note@+1 {{'ld_return1f' defined here}}
+inline long double ld_return1f() { return 0; }
+// expected-note@+1 {{'ld_arg1f' defined here}}
+inline void ld_arg1f(long double ld) {}
+
+inline void ld_use1() {
+// expected-note@+1 {{'ld' defined here}}
+  long double ld = 0;
+// TODO: We should not diagnose this as the function is dead.
+// expected-error@+1 {{'ld' requires 128 bit size 'long double' type support, 
but device 'nvptx64-unknown-unknown' does not support it}}
+  ld += 1;
+}
+static void ld_use2() {
+// expected-note@+1 {{'ld' defined here}}
+  long double ld = 0;
+// TODO: We should not diagnose this as the function is dead.
+// expected-error@+1 {{'ld' requires 128 bit size 'long double' type support, 
but device 'nvptx64-unknown-unknown' does not support it}}
+  ld += 1;
+}
+
+inline void ld_use3() {
+// expected-note@+1 {{'ld' defined here}}
+  long double ld = 0;
+// expected-error@+1 {{'ld' requires 128 bit size 'long double' type support, 
but device 'nvptx64-unknown-unknown' does not support it}}
+  ld += 1;
+}
+static void ld_use4() {
+// expected-note@+1 {{'ld' defined here}}
+  long double ld = 0;
+// expected-error@+1 {{'ld' requires 128 bit size 'long double' type support, 
but device 'nvptx64-unknown-unknown' does not support it}}
+  ld += 1;
+}
+
+void external() {
+// expected-error@+1 {{'ld_return1e' requires 128 bit size 'long double' type 
support, but device 'nvptx64-unknown-unknown' does not support it}}
+  void *p1 = reinterpret_cast(&ld_return1e);
+// expected-error@+1 {{'ld_arg1e' requires 128 bit size 'long double' type 
support, but device 'nvptx64-unknown-unknown' does not support it}}
+  void *p2 = reinterpret_cast(&ld_arg1e);
+// expected-error@+1 {{'ld_return1f' requires 128 bit size 'long double' type 
support, but device 'nvptx64-unknown-unknown' does not support it}}
+  void *p3 = reinterpret_cast(&ld_return1f);
+// expected-error@+1 {{'ld_arg1f' requires 128 bit size 'long double' type 
support, but device 'nvptx64-unknown-unknown' does not support it}}
+  void *p4 = reinterpret_cast(&ld_arg1f);
+  void *p5 = reinterpret_cast(&ld_use3);
+  void *p6 = reinterpret_cast(&ld_use4);
+}
+
+#ifndef _ARCH_PPC
+// TODO: We should diagnose the return type and argument type here.
+__float128 ld_return2a() { return 0; }
+void ld_arg2a(__float128 ld) {}
+
+// TODO: We should diagnose the return type and argument type here.
+typedef __float128 fp128_ty;
+fp128_ty ld_return2b() { return 0; }
+void ld_arg2b(fp128_ty ld) {}

[PATCH] D95903: [OpenMP][NFC] Pre-commit test changes regarding PR48933

[Johannes Doerfert 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 rG3b2f19d0bc28: [OpenMP][NFC] Pre-commit test changes 
regarding PR48933 (authored by jdoerfert).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95903

Files:
  clang/test/OpenMP/nvptx_unsupported_type_messages.cpp

Index: clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
===
--- clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
+++ clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
@@ -77,11 +77,135 @@
 void baz1() {
   T1 t = bar1();
 }
+
+// TODO: We should not emit an error for dead functions we do not emit.
+inline void dead_inline_declare_target() {
+// expected-note@+1 {{'b' defined here}}
+  long double *a, b = 0;
+// expected-error@+1 {{'b' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
+  a = &b;
+}
+// TODO: We should not emit an error for dead functions we do not emit.
+static void dead_static_declare_target() {
+// expected-note@+1 {{'b' defined here}}
+  long double *a, b = 0;
+// expected-error@+1 {{'b' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
+  a = &b;
+}
+template
+void dead_template_declare_target() {
+  long double *a, b = 0;
+  a = &b;
+}
+
+// TODO: We should diagnose the return type and argument type here.
+long double ld_return1a() { return 0; }
+void ld_arg1a(long double ld) {}
+
+// TODO: We should diagnose the return type and argument type here.
+typedef long double ld_ty;
+ld_ty ld_return1b() { return 0; }
+void ld_arg1b(ld_ty ld) {}
+
+static long double ld_return1c() { return 0; }
+static void ld_arg1c(long double ld) {}
+
+inline long double ld_return1d() { return 0; }
+inline void ld_arg1d(long double ld) {}
+
+// expected-note@+1 {{'ld_return1e' defined here}}
+static long double ld_return1e() { return 0; }
+// expected-note@+1 {{'ld_arg1e' defined here}}
+static void ld_arg1e(long double ld) {}
+
+// expected-note@+1 {{'ld_return1f' defined here}}
+inline long double ld_return1f() { return 0; }
+// expected-note@+1 {{'ld_arg1f' defined here}}
+inline void ld_arg1f(long double ld) {}
+
+inline void ld_use1() {
+// expected-note@+1 {{'ld' defined here}}
+  long double ld = 0;
+// TODO: We should not diagnose this as the function is dead.
+// expected-error@+1 {{'ld' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
+  ld += 1;
+}
+static void ld_use2() {
+// expected-note@+1 {{'ld' defined here}}
+  long double ld = 0;
+// TODO: We should not diagnose this as the function is dead.
+// expected-error@+1 {{'ld' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
+  ld += 1;
+}
+
+inline void ld_use3() {
+// expected-note@+1 {{'ld' defined here}}
+  long double ld = 0;
+// expected-error@+1 {{'ld' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
+  ld += 1;
+}
+static void ld_use4() {
+// expected-note@+1 {{'ld' defined here}}
+  long double ld = 0;
+// expected-error@+1 {{'ld' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
+  ld += 1;
+}
+
+void external() {
+// expected-error@+1 {{'ld_return1e' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
+  void *p1 = reinterpret_cast(&ld_return1e);
+// expected-error@+1 {{'ld_arg1e' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
+  void *p2 = reinterpret_cast(&ld_arg1e);
+// expected-error@+1 {{'ld_return1f' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
+  void *p3 = reinterpret_cast(&ld_return1f);
+// expected-error@+1 {{'ld_arg1f' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
+  void *p4 = reinterpret_cast(&ld_arg1f);
+  void *p5 = reinterpret_cast(&ld_use3);
+  void *p6 = reinterpret_cast(&ld_use4);
+}
+
+#ifndef _ARCH_PPC
+// TODO: We should diagnose the return type and argument type here.
+__float128 ld_return2a() { return 0; }
+void ld_arg2a(__float128 ld) {}
+
+// TODO: We should diagnose the return type and argument type here.
+typedef __float128 fp128_ty;
+fp128_ty ld_return2b() { return 0; }
+void ld_arg2b(fp128_ty ld) {}
+#endif
+
 #pragma omp end declare target
 
+// TODO: There should not be an error here, dead_inline is never emitted.
+// expected-note@+1 3{{'f' defined here}}
+inline long double dead_inline(long double f) {
+#pragma omp target map(f)
+// TODO: We should not emit the same error message 3 times, here a

[clang] f9286b4 - [OpenMP] Attribute target diagnostics properly

[Johannes Doerfert via cfe-commits]

Author: Johannes Doerfert
Date: 2021-02-15T13:16:55-06:00
New Revision: f9286b434b764b366f1aad9249c04e7741ed5518

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

LOG: [OpenMP] Attribute target diagnostics properly

Type errors in function declarations were not (always) diagnosed prior
to this patch. Furthermore, certain remarks did not get associated
properly which caused them to be emitted multiple times.

Reviewed By: JonChesterfield

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

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/nvptx_unsupported_type_messages.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 8b5619945865..d0d245bb1267 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -11951,8 +11951,8 @@ class Sema final {
   ///  if (diagIfOpenMPDeviceCode(Loc, diag::err_vla_unsupported))
   ///return ExprError();
   ///  // Otherwise, continue parsing as normal.
-  SemaDiagnosticBuilder diagIfOpenMPDeviceCode(SourceLocation Loc,
-   unsigned DiagID);
+  SemaDiagnosticBuilder
+  diagIfOpenMPDeviceCode(SourceLocation Loc, unsigned DiagID, FunctionDecl 
*FD);
 
   /// Creates a SemaDiagnosticBuilder that emits the diagnostic if the current
   /// context is "used as host code".
@@ -11968,17 +11968,19 @@ class Sema final {
   ///return ExprError();
   ///  // Otherwise, continue parsing as normal.
   SemaDiagnosticBuilder diagIfOpenMPHostCode(SourceLocation Loc,
- unsigned DiagID);
+ unsigned DiagID, FunctionDecl 
*FD);
 
-  SemaDiagnosticBuilder targetDiag(SourceLocation Loc, unsigned DiagID);
+  SemaDiagnosticBuilder targetDiag(SourceLocation Loc, unsigned DiagID,
+   FunctionDecl *FD = nullptr);
   SemaDiagnosticBuilder targetDiag(SourceLocation Loc,
-   const PartialDiagnostic &PD) {
-return targetDiag(Loc, PD.getDiagID()) << PD;
+   const PartialDiagnostic &PD,
+   FunctionDecl *FD = nullptr) {
+return targetDiag(Loc, PD.getDiagID(), FD) << PD;
   }
 
   /// Check if the expression is allowed to be used in expressions for the
   /// offloading devices.
-  void checkDeviceDecl(const ValueDecl *D, SourceLocation Loc);
+  void checkDeviceDecl(ValueDecl *D, SourceLocation Loc);
 
   enum CUDAFunctionTarget {
 CFT_Device,

diff  --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index cb5a84a31235..450f9c020f7f 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -14,6 +14,7 @@
 #include "UsedDeclVisitor.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDiagnostic.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclObjC.h"
@@ -1740,11 +1741,12 @@ Sema::SemaDiagnosticBuilder::~SemaDiagnosticBuilder() {
   }
 }
 
-Sema::SemaDiagnosticBuilder Sema::targetDiag(SourceLocation Loc,
- unsigned DiagID) {
+Sema::SemaDiagnosticBuilder
+Sema::targetDiag(SourceLocation Loc, unsigned DiagID, FunctionDecl *FD) {
+  FD = FD ? FD : getCurFunctionDecl();
   if (LangOpts.OpenMP)
-return LangOpts.OpenMPIsDevice ? diagIfOpenMPDeviceCode(Loc, DiagID)
-   : diagIfOpenMPHostCode(Loc, DiagID);
+return LangOpts.OpenMPIsDevice ? diagIfOpenMPDeviceCode(Loc, DiagID, FD)
+   : diagIfOpenMPHostCode(Loc, DiagID, FD);
   if (getLangOpts().CUDA)
 return getLangOpts().CUDAIsDevice ? CUDADiagIfDeviceCode(Loc, DiagID)
   : CUDADiagIfHostCode(Loc, DiagID);
@@ -1753,7 +1755,7 @@ Sema::SemaDiagnosticBuilder 
Sema::targetDiag(SourceLocation Loc,
 return SYCLDiagIfDeviceCode(Loc, DiagID);
 
   return SemaDiagnosticBuilder(SemaDiagnosticBuilder::K_Immediate, Loc, DiagID,
-   getCurFunctionDecl(), *this);
+   FD, *this);
 }
 
 Sema::SemaDiagnosticBuilder Sema::Diag(SourceLocation Loc, unsigned DiagID,
@@ -1772,15 +1774,14 @@ Sema::SemaDiagnosticBuilder Sema::Diag(SourceLocation 
Loc, unsigned DiagID,
  DiagID, getCurFunctionDecl(), *this);
   }
 
-  SemaDiagnosticBuilder DB =
-  getLangOpts().CUDAIsDevice
-  ? CUDADiagIfDeviceCode(Loc, DiagID)
-  : CUDADiagIfHostCode(Loc, DiagID);
+  SemaDiagnosticBuilder DB = getLangOpts().CUD

[clang] 1dd66e6 - [OpenMP] Delay more diagnostics of potentially non-emitted code

[Johannes Doerfert via cfe-commits]

Author: Johannes Doerfert
Date: 2021-02-15T13:17:05-06:00
New Revision: 1dd66e6111a8247c6c7931143251c0cf1442b905

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

LOG: [OpenMP] Delay more diagnostics of potentially non-emitted code

Even code in target and declare target regions might not be emitted.
With this patch we delay more diagnostics and use laziness and linkage
to determine if a function is emitted (for the device). Note that we
still eagerly emit diagnostics for target regions, unfortunately, see
the TODO for the reason.

This hopefully fixes PR48933.

Reviewed By: JonChesterfield

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

Added: 


Modified: 
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/nvptx_allocate_messages.cpp
clang/test/OpenMP/nvptx_target_exceptions_messages.cpp
clang/test/OpenMP/nvptx_unsupported_type_messages.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 4c2f7f71e443..cee107096947 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -18333,42 +18333,51 @@ Sema::FunctionEmissionStatus 
Sema::getEmissionStatus(FunctionDecl *FD,
   if (FD->isDependentContext())
 return FunctionEmissionStatus::TemplateDiscarded;
 
-  FunctionEmissionStatus OMPES = FunctionEmissionStatus::Unknown;
+  // Check whether this function is an externally visible definition.
+  auto IsEmittedForExternalSymbol = [this, FD]() {
+// We have to check the GVA linkage of the function's *definition* -- if we
+// only have a declaration, we don't know whether or not the function will
+// be emitted, because (say) the definition could include "inline".
+FunctionDecl *Def = FD->getDefinition();
+
+return Def && !isDiscardableGVALinkage(
+  getASTContext().GetGVALinkageForFunction(Def));
+  };
+
   if (LangOpts.OpenMPIsDevice) {
+// In OpenMP device mode we will not emit host only functions, or functions
+// we don't need due to their linkage.
 Optional DevTy =
 OMPDeclareTargetDeclAttr::getDeviceType(FD->getCanonicalDecl());
-if (DevTy.hasValue()) {
+// DevTy may be changed later by
+//  #pragma omp declare target to(*) device_type(*).
+// Therefore DevTyhaving no value does not imply host. The emission status
+// will be checked again at the end of compilation unit with Final = true.
+if (DevTy.hasValue())
   if (*DevTy == OMPDeclareTargetDeclAttr::DT_Host)
-OMPES = FunctionEmissionStatus::OMPDiscarded;
-  else if (*DevTy == OMPDeclareTargetDeclAttr::DT_NoHost ||
-   *DevTy == OMPDeclareTargetDeclAttr::DT_Any) {
-OMPES = FunctionEmissionStatus::Emitted;
-  }
-}
-  } else if (LangOpts.OpenMP) {
-// In OpenMP 4.5 all the functions are host functions.
-if (LangOpts.OpenMP <= 45) {
-  OMPES = FunctionEmissionStatus::Emitted;
-} else {
-  Optional DevTy =
-  OMPDeclareTargetDeclAttr::getDeviceType(FD->getCanonicalDecl());
-  // In OpenMP 5.0 or above, DevTy may be changed later by
-  // #pragma omp declare target to(*) device_type(*). Therefore DevTy
-  // having no value does not imply host. The emission status will be
-  // checked again at the end of compilation unit.
-  if (DevTy.hasValue()) {
-if (*DevTy == OMPDeclareTargetDeclAttr::DT_NoHost) {
-  OMPES = FunctionEmissionStatus::OMPDiscarded;
-} else if (*DevTy == OMPDeclareTargetDeclAttr::DT_Host ||
-   *DevTy == OMPDeclareTargetDeclAttr::DT_Any)
-  OMPES = FunctionEmissionStatus::Emitted;
-  } else if (Final)
-OMPES = FunctionEmissionStatus::Emitted;
-}
-  }
-  if (OMPES == FunctionEmissionStatus::OMPDiscarded ||
-  (OMPES == FunctionEmissionStatus::Emitted && !LangOpts.CUDA))
-return OMPES;
+return FunctionEmissionStatus::OMPDiscarded;
+// If we have an explicit value for the device type, or we are in a target
+// declare context, we need to emit all extern and used symbols.
+if (isInOpenMPDeclareTargetContext() || DevTy.hasValue())
+  if (IsEmittedForExternalSymbol())
+return FunctionEmissionStatus::Emitted;
+// Device mode only emits what it must, if it wasn't tagged yet and needed,
+// we'll omit it.
+if (Final)
+  return FunctionEmissionStatus::OMPDiscarded;
+  } else if (LangOpts.OpenMP > 45) {
+// In OpenMP host compilation prior to 5.0 everything was an emitted host
+// function. In 5.0, no_host was introduced which might cause a function to
+// be ommitted.
+Optional DevTy =
+OMPDeclareTargetDeclAttr::getDeviceType(FD->getCanonicalDecl());
+if (DevTy.hasValue())
+  if (*DevTy

[PATCH] D95912: [OpenMP] Attribute target diagnostics properly

[Johannes Doerfert 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 rGf9286b434b76: [OpenMP] Attribute target diagnostics properly 
(authored by jdoerfert).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95912

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/nvptx_unsupported_type_messages.cpp

Index: clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
===
--- clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
+++ clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
@@ -39,10 +39,12 @@
 };
 
 #ifndef _ARCH_PPC
-// expected-note@+1 {{'boo' defined here}}
+// expected-error@+2 {{'boo' requires 128 bit size '__float128' type support, but device 'nvptx64-unknown-unknown' does not support it}}
+// expected-note@+1 2{{'boo' defined here}}
 void boo(__float128 A) { return; }
 #else
-// expected-note@+1 {{'boo' defined here}}
+// expected-error@+2 {{'boo' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
+// expected-note@+1 2{{'boo' defined here}}
 void boo(long double A) { return; }
 #endif
 #pragma omp declare target
@@ -51,10 +53,11 @@
 void foo(T a = T()) {
   a = a + f; // expected-note {{called by 'foo'}}
 #ifndef _ARCH_PPC
-// expected-error@+4 {{'boo' requires 128 bit size '__float128' type support, but device 'nvptx64-unknown-unknown' does not support it}}
+// expected-error@+5 {{'boo' requires 128 bit size '__float128' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 #else
-// expected-error@+2 {{'boo' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
+// expected-error@+3 {{'boo' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 #endif
+// expected-note@+1 {{called by 'foo'}}
   boo(0);
   return;
 }
@@ -98,28 +101,49 @@
   a = &b;
 }
 
-// TODO: We should diagnose the return type and argument type here.
+// expected-note@+2 {{'ld_return1a' defined here}}
+// expected-error@+1 {{'ld_return1a' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 long double ld_return1a() { return 0; }
+// expected-note@+2 {{'ld_arg1a' defined here}}
+// expected-error@+1 {{'ld_arg1a' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 void ld_arg1a(long double ld) {}
 
 // TODO: We should diagnose the return type and argument type here.
 typedef long double ld_ty;
+// expected-note@+2 {{'ld_return1b' defined here}}
+// expected-error@+1 {{'ld_return1b' requires 128 bit size 'ld_ty' (aka 'long double') type support, but device 'nvptx64-unknown-unknown' does not support it}}
 ld_ty ld_return1b() { return 0; }
+// expected-note@+2 {{'ld_arg1b' defined here}}
+// expected-error@+1 {{'ld_arg1b' requires 128 bit size 'ld_ty' (aka 'long double') type support, but device 'nvptx64-unknown-unknown' does not support it}}
 void ld_arg1b(ld_ty ld) {}
 
+// TODO: These errors should not be emitted.
+// expected-note@+2 {{'ld_return1c' defined here}}
+// expected-error@+1 {{'ld_return1c' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 static long double ld_return1c() { return 0; }
+// expected-note@+2 {{'ld_arg1c' defined here}}
+// expected-error@+1 {{'ld_arg1c' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 static void ld_arg1c(long double ld) {}
 
+// TODO: These errors should not be emitted.
+// expected-note@+2 {{'ld_return1d' defined here}}
+// expected-error@+1 {{'ld_return1d' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 inline long double ld_return1d() { return 0; }
+// expected-note@+2 {{'ld_arg1d' defined here}}
+// expected-error@+1 {{'ld_arg1d' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 inline void ld_arg1d(long double ld) {}
 
+// expected-error@+2 {{'ld_return1e' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 // expected-note@+1 {{'ld_return1e' defined here}}
 static long double ld_return1e() { return 0; }
+// expected-error@+2 {{'ld_arg1e' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 // expected-note@+1 {{'ld_arg1e' defined here}}
 static void ld_arg1e(long double ld) {}
 
+// expected-error@+2 {{'ld_return1f' requires 128 bit size 'long double' type support, but device 'n

[PATCH] D95928: [OpenMP] Delay more diagnostics of potentially non-emitted code

[Johannes Doerfert via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
jdoerfert marked 2 inline comments as done.
Closed by commit rG1dd66e6111a8: [OpenMP] Delay more diagnostics of potentially 
non-emitted code (authored by jdoerfert).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95928

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/nvptx_allocate_messages.cpp
  clang/test/OpenMP/nvptx_target_exceptions_messages.cpp
  clang/test/OpenMP/nvptx_unsupported_type_messages.cpp

Index: clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
===
--- clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
+++ clang/test/OpenMP/nvptx_unsupported_type_messages.cpp
@@ -81,18 +81,12 @@
   T1 t = bar1();
 }
 
-// TODO: We should not emit an error for dead functions we do not emit.
 inline void dead_inline_declare_target() {
-// expected-note@+1 {{'b' defined here}}
   long double *a, b = 0;
-// expected-error@+1 {{'b' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
   a = &b;
 }
-// TODO: We should not emit an error for dead functions we do not emit.
 static void dead_static_declare_target() {
-// expected-note@+1 {{'b' defined here}}
   long double *a, b = 0;
-// expected-error@+1 {{'b' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
   a = &b;
 }
 template
@@ -108,7 +102,6 @@
 // expected-error@+1 {{'ld_arg1a' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 void ld_arg1a(long double ld) {}
 
-// TODO: We should diagnose the return type and argument type here.
 typedef long double ld_ty;
 // expected-note@+2 {{'ld_return1b' defined here}}
 // expected-error@+1 {{'ld_return1b' requires 128 bit size 'ld_ty' (aka 'long double') type support, but device 'nvptx64-unknown-unknown' does not support it}}
@@ -117,48 +110,28 @@
 // expected-error@+1 {{'ld_arg1b' requires 128 bit size 'ld_ty' (aka 'long double') type support, but device 'nvptx64-unknown-unknown' does not support it}}
 void ld_arg1b(ld_ty ld) {}
 
-// TODO: These errors should not be emitted.
-// expected-note@+2 {{'ld_return1c' defined here}}
-// expected-error@+1 {{'ld_return1c' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 static long double ld_return1c() { return 0; }
-// expected-note@+2 {{'ld_arg1c' defined here}}
-// expected-error@+1 {{'ld_arg1c' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 static void ld_arg1c(long double ld) {}
 
-// TODO: These errors should not be emitted.
-// expected-note@+2 {{'ld_return1d' defined here}}
-// expected-error@+1 {{'ld_return1d' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 inline long double ld_return1d() { return 0; }
-// expected-note@+2 {{'ld_arg1d' defined here}}
-// expected-error@+1 {{'ld_arg1d' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 inline void ld_arg1d(long double ld) {}
 
-// expected-error@+2 {{'ld_return1e' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 // expected-note@+1 {{'ld_return1e' defined here}}
 static long double ld_return1e() { return 0; }
-// expected-error@+2 {{'ld_arg1e' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 // expected-note@+1 {{'ld_arg1e' defined here}}
 static void ld_arg1e(long double ld) {}
 
-// expected-error@+2 {{'ld_return1f' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 // expected-note@+1 {{'ld_return1f' defined here}}
 inline long double ld_return1f() { return 0; }
-// expected-error@+2 {{'ld_arg1f' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
 // expected-note@+1 {{'ld_arg1f' defined here}}
 inline void ld_arg1f(long double ld) {}
 
 inline void ld_use1() {
-// expected-note@+1 {{'ld' defined here}}
   long double ld = 0;
-// TODO: We should not diagnose this as the function is dead.
-// expected-error@+1 {{'ld' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
   ld += 1;
 }
 static void ld_use2() {
-// expected-note@+1 {{'ld' defined here}}
   long double ld = 0;
-// TODO: We should not diagnose this as the function is dead.
-// expected-error@+1 {{'ld' requires 128 bit size 'long double' type support, but device 'nvptx64-unknown-unknown' does not support it}}
   ld += 1;
 }
 
@@

[PATCH] D96726: [clangd] Give modules access to filesystem, scheduler, and index.

[Sam McCall via Phabricator via cfe-commits]

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

This finally makes it possible to implement useful modules.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96726

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/Module.cpp
  clang-tools-extra/clangd/Module.h
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -230,7 +230,11 @@
 }
 
 void add(const int &X) { Value += X; }
-void get(const std::nullptr_t &, Callback Reply) { Reply(Value); }
+void get(const std::nullptr_t &, Callback Reply) {
+  scheduler().runQuick(
+  "get", "",
+  [Reply(std::move(Reply)), Value(Value)]() mutable { Reply(Value); });
+}
 int Value = 0;
   };
   std::vector> Mods;
Index: clang-tools-extra/clangd/Module.h
===
--- clang-tools-extra/clangd/Module.h
+++ clang-tools-extra/clangd/Module.h
@@ -1,7 +1,14 @@
+//===--- Module.h - Plugging features into clangd -*-C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULE_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULE_H
 
-#include "LSPBinder.h"
 #include "Protocol.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/JSON.h"
@@ -10,6 +17,10 @@
 
 namespace clang {
 namespace clangd {
+class LSPBinder;
+class SymbolIndex;
+class ThreadsafeFS;
+class TUScheduler;
 
 /// A Module contributes a vertical feature to clangd.
 ///
@@ -17,10 +28,11 @@
 ///
 /// The lifetime of a module is roughly:
 ///  - modules are created before the LSP server, in ClangdMain.cpp
-///  - these modules are then passed to ClangdLSPServer and ClangdServer
-///  - module hooks can be called at this point.
-///FIXME: We should make some server facilities like TUScheduler and index
-///available to those modules after ClangdServer is initalized.
+///  - these modules are then passed to ClangdLSPServer in a ModuleSet
+///  - initializeLSP() is called when the editor calls initialize.
+//   - initialize() is then called by ClangdServer as it is constructed.
+///  - module hooks can be called by the server at this point.
+///Server facilities (scheduler etc) are available.
 ///  - ClangdServer will not be destroyed until all the requests are done.
 ///FIXME: Block server shutdown until all the modules are idle.
 ///  - modules will be destroyed after ClangdLSPServer is destroyed.
@@ -41,6 +53,28 @@
   virtual void initializeLSP(LSPBinder &Bind,
  const ClientCapabilities &ClientCaps,
  llvm::json::Object &ServerCaps) {}
+
+  /// Shared server facilities needed by the module to get its work done.
+  struct Facilities {
+TUScheduler &Scheduler;
+const SymbolIndex *Index;
+const ThreadsafeFS &FS;
+  };
+  /// Called by the server to prepare this module for use.
+  void initialize(const Facilities &F);
+
+protected:
+  /// Accessors for modules to access shared server facilities they depend on.
+  Facilities &facilities();
+  /// The scheduler is used to run tasks on worker threads and access ASTs.
+  TUScheduler &scheduler() { return facilities().Scheduler; }
+  /// The index is used to get information about the whole codebase.
+  const SymbolIndex *index() { return facilities().Index; }
+  /// The filesystem is used to read source files on disk.
+  const ThreadsafeFS &fs() { return facilities().FS; }
+
+private:
+  llvm::Optional Fac;
 };
 
 class ModuleSet {
Index: clang-tools-extra/clangd/Module.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/Module.cpp
@@ -0,0 +1,25 @@
+//===--- Module.cpp - Plugging features into clangd ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Module.h"
+
+namespace clang {
+namespace clangd {
+
+void Module::initialize(const Facilities &F) {
+  assert(!Fac.hasValue() && 

[PATCH] D96717: [clangd] Bind outgoing calls through LSPBinder too. NFC

[Sam McCall via Phabricator via cfe-commits]

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

Remove fixedme


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96717

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/LSPBinder.h
  clang-tools-extra/clangd/Module.h
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
  clang-tools-extra/clangd/unittests/LSPBinderTests.cpp

Index: clang-tools-extra/clangd/unittests/LSPBinderTests.cpp
===
--- clang-tools-extra/clangd/unittests/LSPBinderTests.cpp
+++ clang-tools-extra/clangd/unittests/LSPBinderTests.cpp
@@ -15,12 +15,15 @@
 namespace clangd {
 namespace {
 
+using testing::ElementsAre;
 using testing::HasSubstr;
+using testing::IsEmpty;
 using testing::UnorderedElementsAre;
 
 // JSON-serializable type for testing.
 struct Foo {
   int X;
+  friend bool operator==(Foo A, Foo B) { return A.X == B.X; }
 };
 bool fromJSON(const llvm::json::Value &V, Foo &F, llvm::json::Path P) {
   return fromJSON(V, F.X, P.field("X"));
@@ -35,9 +38,31 @@
   return [&Out](llvm::Expected V) { Out.emplace(std::move(V)); };
 }
 
+struct OutgoingRecorder : public LSPBinder::RawOutgoing {
+  llvm::StringMap> Received;
+
+  void callMethod(llvm::StringRef Method, llvm::json::Value Params,
+  Callback Reply) override {
+Received[Method].push_back(Params);
+if (Method == "fail")
+  return Reply(error("Params={0}", Params));
+Reply(Params); // echo back the request
+  }
+  void notify(llvm::StringRef Method, llvm::json::Value Params) override {
+Received[Method].push_back(std::move(Params));
+  }
+
+  std::vector take(llvm::StringRef Method) {
+std::vector Result = Received.lookup(Method);
+Received.erase(Method);
+return Result;
+  }
+};
+
 TEST(LSPBinderTest, IncomingCalls) {
   LSPBinder::RawHandlers RawHandlers;
-  LSPBinder Binder{RawHandlers};
+  OutgoingRecorder RawOutgoing;
+  LSPBinder Binder{RawHandlers, RawOutgoing};
   struct Handler {
 void plusOne(const Foo &Params, Callback Reply) {
   Reply(Foo{Params.X + 1});
@@ -94,7 +119,45 @@
   RawCmdPlusOne(1, capture(Reply));
   ASSERT_TRUE(Reply.hasValue());
   EXPECT_THAT_EXPECTED(Reply.getValue(), llvm::HasValue(2));
+
+  // None of this generated any outgoing traffic.
+  EXPECT_THAT(RawOutgoing.Received, IsEmpty());
+}
+
+TEST(LSPBinderTest, OutgoingCalls) {
+  LSPBinder::RawHandlers RawHandlers;
+  OutgoingRecorder RawOutgoing;
+  LSPBinder Binder{RawHandlers, RawOutgoing};
+
+  LSPBinder::OutgoingMethod Echo;
+  Binder.outgoingMethod("echo", Echo);
+  LSPBinder::OutgoingMethod WrongSignature;
+  Binder.outgoingMethod("wrongSignature", WrongSignature);
+  LSPBinder::OutgoingMethod Fail;
+  Binder.outgoingMethod("fail", Fail);
+
+  llvm::Optional> Reply;
+  Echo(Foo{2}, capture(Reply));
+  EXPECT_THAT(RawOutgoing.take("echo"), ElementsAre(llvm::json::Value(2)));
+  ASSERT_TRUE(Reply.hasValue());
+  EXPECT_THAT_EXPECTED(Reply.getValue(), llvm::HasValue(Foo{2}));
+
+  // JSON response is integer, can't be parsed as string.
+  llvm::Optional> WrongTypeReply;
+  WrongSignature(Foo{2}, capture(WrongTypeReply));
+  EXPECT_THAT(RawOutgoing.take("wrongSignature"),
+  ElementsAre(llvm::json::Value(2)));
+  ASSERT_TRUE(Reply.hasValue());
+  EXPECT_THAT_EXPECTED(WrongTypeReply.getValue(),
+   llvm::FailedWithMessage(
+   HasSubstr("failed to decode wrongSignature reply")));
+
+  Fail(Foo{2}, capture(Reply));
+  EXPECT_THAT(RawOutgoing.take("fail"), ElementsAre(llvm::json::Value(2)));
+  ASSERT_TRUE(Reply.hasValue());
+  EXPECT_THAT_EXPECTED(Reply.getValue(), llvm::FailedWithMessage("Params=2"));
 }
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -23,6 +23,7 @@
 namespace clang {
 namespace clangd {
 namespace {
+using testing::ElementsAre;
 
 MATCHER_P(DiagMessage, M, "") {
   if (const auto *O = arg.getAsObject()) {
@@ -223,13 +224,18 @@
 
 TEST_F(LSPTest, ModulesTest) {
   class MathModule : public Module {
+OutgoingNotification Changed;
 void initializeLSP(LSPBinder &Bind, const ClientCapabilities &ClientCaps,
llvm::json::Object &ServerCaps) override {
   Bind.notification("add", this, &MathModule::add);
   Bind.method("get", this, &MathModule::get);
+  Bind.outgoingNotification("changed", Changed);
 }
 
-void add(const int &X) { Value += X; }
+void add(const int &X) {
+  Value += X;
+  Changed(Value);
+}
 void get(const std::nullptr_t &, Callback Reply) { Reply(Value); }

[PATCH] D95814: [NFC] Simplify test to use multiple FileCheck prefixes.

[Arnold Schwaighofer via Phabricator via cfe-commits]

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

LGTM

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95814

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

[PATCH] D96725: [clang-tidy] Fix modernize-use-using in extern C code

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp:305
 };
+
+extern "C" {

Can you add tests for typedefs in other scopes like

```

extern "C" {
typedef int CType;

struct CAnother {
};

typedef struct {
  int b;
  typedef struct CAnother AStruct;
} CStruct;

void foo()
{
  typedef struct CAnother AStruct;
}
}

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96725

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

[clang-tools-extra] 4d700fb - [clangd] Pass raw client capabilities to modules. NFC

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-02-15T20:57:14+01:00
New Revision: 4d700fb0603e6fbdd6f597443b29414f7e133912

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

LOG: [clangd] Pass raw client capabilities to modules. NFC

Added: 


Modified: 
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/Module.h
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Protocol.h
clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index c30890f14787..8bc49b5a5817 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -582,7 +582,7 @@ void ClangdLSPServer::onInitialize(const InitializeParams 
&Params,
 bindMethods(Binder);
 if (Opts.Modules)
   for (auto &Mod : *Opts.Modules)
-Mod.initializeLSP(Binder, Params.capabilities, ServerCaps);
+Mod.initializeLSP(Binder, Params.rawCapabilities, ServerCaps);
   }
 
   // Per LSP, renameProvider can be either boolean or RenameOptions.

diff  --git a/clang-tools-extra/clangd/Module.h 
b/clang-tools-extra/clangd/Module.h
index 3f5d5f0b29b3..a99b78d51c44 100644
--- a/clang-tools-extra/clangd/Module.h
+++ b/clang-tools-extra/clangd/Module.h
@@ -2,7 +2,6 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULE_H
 
 #include "LSPBinder.h"
-#include "Protocol.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/JSON.h"
 #include 
@@ -37,9 +36,8 @@ class Module {
   ///
   /// This is only called if the module is running in ClangdLSPServer!
   /// Modules with a public interface should satisfy it without LSP bindings.
-  // FIXME: ClientCaps should be a raw json::Object here.
   virtual void initializeLSP(LSPBinder &Bind,
- const ClientCapabilities &ClientCaps,
+ const llvm::json::Object &ClientCaps,
  llvm::json::Object &ServerCaps) {}
 };
 

diff  --git a/clang-tools-extra/clangd/Protocol.cpp 
b/clang-tools-extra/clangd/Protocol.cpp
index 74e6b8c72a1c..525da502b692 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -430,6 +430,8 @@ bool fromJSON(const llvm::json::Value &Params, 
InitializeParams &R,
   O.map("rootUri", R.rootUri);
   O.map("rootPath", R.rootPath);
   O.map("capabilities", R.capabilities);
+  if (auto *RawCaps = Params.getAsObject()->getObject("capabilities"))
+R.rawCapabilities = *RawCaps;
   O.map("trace", R.trace);
   O.map("initializationOptions", R.initializationOptions);
   return true;

diff  --git a/clang-tools-extra/clangd/Protocol.h 
b/clang-tools-extra/clangd/Protocol.h
index 922e701d77ef..f918183716d2 100644
--- a/clang-tools-extra/clangd/Protocol.h
+++ b/clang-tools-extra/clangd/Protocol.h
@@ -540,6 +540,8 @@ struct InitializeParams {
 
   /// The capabilities provided by the client (editor or tool)
   ClientCapabilities capabilities;
+  /// The same data as capabilities, but not parsed (to expose to modules).
+  llvm::json::Object rawCapabilities;
 
   /// The initial trace setting. If omitted trace is disabled ('off').
   llvm::Optional trace;

diff  --git a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp 
b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
index 240188de0da0..d8674d601bc8 100644
--- a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -223,7 +223,7 @@ TEST_F(LSPTest, CDBConfigIntegration) {
 
 TEST_F(LSPTest, ModulesTest) {
   class MathModule : public Module {
-void initializeLSP(LSPBinder &Bind, const ClientCapabilities &ClientCaps,
+void initializeLSP(LSPBinder &Bind, const llvm::json::Object &ClientCaps,
llvm::json::Object &ServerCaps) override {
   Bind.notification("add", this, &MathModule::add);
   Bind.method("get", this, &MathModule::get);



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

[PATCH] D96123: [clangd] Expose actOnAllPArentDirectories helper

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet marked an inline comment as done.
kadircet added a comment.

> maybe we should expose absoluteParent instead?

doing that instead. also updating tidyprovider and configyamlparser to make use 
of this helper now, PTAL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96123

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

[PATCH] D96123: [clangd] Expose actOnAllPArentDirectories helper

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 323820.
kadircet added a comment.

- Expose absoluteParent instead of whole traverse action
- Use helper in existing places that use path::parent_path for traversal


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96123

Files:
  clang-tools-extra/clangd/ConfigProvider.cpp
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/TidyProvider.cpp
  clang-tools-extra/clangd/support/Path.cpp
  clang-tools-extra/clangd/support/Path.h

Index: clang-tools-extra/clangd/support/Path.h
===
--- clang-tools-extra/clangd/support/Path.h
+++ clang-tools-extra/clangd/support/Path.h
@@ -28,6 +28,8 @@
 std::string maybeCaseFoldPath(PathRef Path);
 bool pathEqual(PathRef, PathRef);
 
+/// Variant of parent_path that operates only on absolute paths.
+PathRef absoluteParent(PathRef Path);
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/support/Path.cpp
===
--- clang-tools-extra/clangd/support/Path.cpp
+++ clang-tools-extra/clangd/support/Path.cpp
@@ -7,9 +7,25 @@
 //===--===//
 
 #include "support/Path.h"
+#include "llvm/Support/Path.h"
+
 namespace clang {
 namespace clangd {
 
+PathRef absoluteParent(PathRef Path) {
+  assert(llvm::sys::path::is_absolute(Path));
+#if defined(_WIN32)
+  // llvm::sys says "C:\" is absolute, and its parent is "C:" which is relative.
+  // This unhelpful behavior seems to have been inherited from boost.
+  if (llvm::sys::path::relative_path(Path).empty()) {
+return PathRef();
+  }
+#endif
+  PathRef Result = llvm::sys::path::parent_path(Path);
+  assert(Result.empty() || llvm::sys::path::is_absolute(Result));
+  return Result;
+}
+
 std::string maybeCaseFoldPath(PathRef Path) {
 #if defined(_WIN32) || defined(__APPLE__)
   return Path.lower();
Index: clang-tools-extra/clangd/TidyProvider.cpp
===
--- clang-tools-extra/clangd/TidyProvider.cpp
+++ clang-tools-extra/clangd/TidyProvider.cpp
@@ -11,6 +11,7 @@
 #include "Config.h"
 #include "support/FileCache.h"
 #include "support/Logger.h"
+#include "support/Path.h"
 #include "support/ThreadsafeFS.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
@@ -102,23 +103,12 @@
 
 // Compute absolute paths to all ancestors (substrings of P.Path).
 // Ensure cache entries for each ancestor exist in the map.
-llvm::StringRef Parent = path::parent_path(AbsPath);
 llvm::SmallVector Caches;
 {
   std::lock_guard Lock(Mu);
-  for (auto I = path::rbegin(Parent), E = path::rend(Parent); I != E; ++I) {
-assert(I->end() >= Parent.begin() && I->end() <= Parent.end() &&
-   "Canonical path components should be substrings");
-llvm::StringRef Ancestor(Parent.begin(), I->end() - Parent.begin());
-#ifdef _WIN32
-// C:\ is an ancestor, but skip its (relative!) parent C:.
-if (Ancestor.size() == 2 && Ancestor.back() == ':')
-  continue;
-#endif
-assert(path::is_absolute(Ancestor));
-
+  for (auto Ancestor = absoluteParent(AbsPath); !Ancestor.empty();
+   Ancestor = absoluteParent(Ancestor)) {
 auto It = Cache.find(Ancestor);
-
 // Assemble the actual config file path only if needed.
 if (It == Cache.end()) {
   llvm::SmallString<256> ConfigPath = Ancestor;
Index: clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
===
--- clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -43,21 +43,6 @@
 namespace clangd {
 namespace {
 
-// Variant of parent_path that operates only on absolute paths.
-PathRef absoluteParent(PathRef Path) {
-  assert(llvm::sys::path::is_absolute(Path));
-#if defined(_WIN32)
-  // llvm::sys says "C:\" is absolute, and its parent is "C:" which is relative.
-  // This unhelpful behavior seems to have been inherited from boost.
-  if (llvm::sys::path::relative_path(Path).empty()) {
-return PathRef();
-  }
-#endif
-  PathRef Result = llvm::sys::path::parent_path(Path);
-  assert(Result.empty() || llvm::sys::path::is_absolute(Result));
-  return Result;
-}
-
 // Runs the given action on all parent directories of filename, starting from
 // deepest directory and going up to root. Stops whenever action succeeds.
 void actOnAllParentDirectories(PathRef FileName,
Index: clang-tools-extra/clangd/ConfigProvider.cpp
===
--- clang-tools-extra/clangd/ConfigProvider.cpp
+++ clang-tools-extra/clangd/ConfigProvider.cpp
@@ -10,8 +10,10 @@
 #include "Config.h"
 #include "ConfigFragment.h"
 #inclu

[PATCH] D96730: [clangd] Modules can have a public API. NFC

[Sam McCall via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96730

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Module.cpp
  clang-tools-extra/clangd/Module.h
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -222,25 +222,26 @@
 }
 
 TEST_F(LSPTest, ModulesTest) {
-  class MathModule : public Module {
+  class MathModule final : public Module {
 void initializeLSP(LSPBinder &Bind, const llvm::json::Object &ClientCaps,
llvm::json::Object &ServerCaps) override {
   Bind.notification("add", this, &MathModule::add);
   Bind.method("get", this, &MathModule::get);
 }
 
+int Value = 0;
+
+  public:
 void add(const int &X) { Value += X; }
 void get(const std::nullptr_t &, Callback Reply) { Reply(Value); }
-int Value = 0;
   };
-  std::vector> Mods;
-  Mods.push_back(std::make_unique());
-  ModuleSet ModSet(std::move(Mods));
-  Opts.Modules = &ModSet;
+  ModuleSet Mods;
+  Mods.add(std::make_unique());
+  Opts.Modules = &Mods;
 
   auto &Client = start();
   Client.notify("add", 2);
-  Client.notify("add", 8);
+  Mods.get()->add(8);
   EXPECT_EQ(10, Client.call("get", nullptr).takeValue());
 }
 
Index: clang-tools-extra/clangd/Module.h
===
--- clang-tools-extra/clangd/Module.h
+++ clang-tools-extra/clangd/Module.h
@@ -5,6 +5,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/JSON.h"
 #include 
+#include 
 #include 
 
 namespace clang {
@@ -41,12 +42,28 @@
  llvm::json::Object &ServerCaps) {}
 };
 
+/// A ModuleSet is a collection of modules installed in clangd.
+///
+/// Modules can be looked up by type, or used through the Module interface.
+/// This allows individual modules to expose a public API.
+/// For this reason, there can be only one module of each type.
+///
+/// ModuleSet owns the modules. It is itself owned by main, not ClangdServer.
 class ModuleSet {
   std::vector> Modules;
+  llvm::DenseMap Map;
+
+  template  struct ID {
+static_assert(std::is_base_of::value &&
+  std::is_final::value,
+  "Modules must be final classes derived from clangd::Module");
+static int Key;
+  };
+
+  bool addImpl(void *Key, std::unique_ptr);
 
 public:
-  explicit ModuleSet(std::vector> Modules)
-  : Modules(std::move(Modules)) {}
+  ModuleSet() = default;
 
   using iterator = llvm::pointee_iterator;
   using const_iterator =
@@ -55,7 +72,20 @@
   iterator end() { return iterator(Modules.end()); }
   const_iterator begin() const { return const_iterator(Modules.begin()); }
   const_iterator end() const { return const_iterator(Modules.end()); }
+
+  template  bool add(std::unique_ptr M) {
+return addImpl(&ID::Key, std::move(M));
+  }
+  template  Mod *get() {
+return static_cast(Map.lookup(&ID::Key));
+  }
+  template  const Mod *get() const {
+return const_cast(this)->get();
+  }
 };
+
+template  int ModuleSet::ID::Key;
+
 } // namespace clangd
 } // namespace clang
 #endif
Index: clang-tools-extra/clangd/Module.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/Module.cpp
@@ -0,0 +1,14 @@
+#include "Module.h"
+
+namespace clang {
+namespace clangd {
+
+bool ModuleSet::addImpl(void *Key, std::unique_ptr M) {
+  if (!Map.try_emplace(Key, M.get()).second)
+return false;
+  Modules.push_back(std::move(M));
+  return true;
+}
+
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -161,6 +161,15 @@
   ClangdServer(const GlobalCompilationDatabase &CDB, const ThreadsafeFS &TFS,
const Options &Opts, Callbacks *Callbacks = nullptr);
 
+  /// Gets the installed module of a given type, if any.
+  /// This exposes access the public interface of modules that have one.
+  template  Mod *getModule() {
+return Modules ? Modules->get() : nullptr;
+  }
+  template  const Mod *getModule() const {
+return Modules ? Modules->get() : nullptr;
+  }
+
   /// Add a \p File to the list of tracked C++ files or update the contents if
   /// \p File is already tracked. Also schedules parsing of t

[PATCH] D96123: [clangd] Expose actOnAllPArentDirectories helper

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.

Thanks!




Comment at: clang-tools-extra/clangd/ConfigProvider.cpp:101
   // Compute absolute paths to all ancestors (substrings of P.Path).
   llvm::StringRef Parent = path::parent_path(P.Path);
   llvm::SmallVector Ancestors;

absoluteParent?

I think this can be moved into the loop init now, it's not used anywhere else



Comment at: clang-tools-extra/clangd/support/Path.h:31
 
+/// Variant of parent_path that operates only on absolute paths.
+PathRef absoluteParent(PathRef Path);

Maybe add a hint why we use this...

"unlike sys::parent_path, does not consider C: a parent of C:\"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96123

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

[PATCH] D94973: [clang][OpenMP] Use OpenMPIRBuilder for workshare loops.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:5215
+  Expr *NegIncAmount =
+  AssertSuccess(Actions.BuildUnaryOp(nullptr, {}, UO_Minus, NewStep));
+  Expr *BackwardDist = AssertSuccess(

Meinersbur wrote:
> jdenny wrote:
> > It looks like the AST nodes `NewStart`, `NewStop`, and `NewStep` are shared 
> > by multiple parent nodes here.  Does that ever happen anywhere else in a 
> > Clang AST?
> Yes. For instance, when instantiating a template, nodes form the 
> TemplatedDecl are reused unless they need to change. Therefore multiple 
> template instantiation can also share entire AST subtrees. Why rebuild them 
> if they are identical?
> 
> However, I also found the following description of 
> TreeTransform::AlwaysRebuild():
> ```
>   /// We must always rebuild all AST nodes when performing variadic template
>   /// pack expansion, in order to avoid violating the AST invariant that each
>   /// statement node appears at most once in its containing declaration.
> ```
> It was added by @rsmith in rG2aa81a718e64fb4ca651ea12ab7afaeffc11e463 to fix 
> llvm.org/PR17800 . The assertion doesn't seem to exist in the current code 
> base.
> 
> Does this apply here? 
I think the invariant exists because codegen has a map from ast expression 
nodes to its temporary memory location. This means that reusing the same AST 
expression node indeed should not be done.

I think the assertion that failed in llvm.org/PR17800 is now here:
https://github.com/llvm/llvm-project/blob/7ba2e1c6011eeb1b91ce3f5d8fa7187b7518e77a/clang/lib/AST/ExprConstant.cpp#L1872

Assertion message was changed in this commit:
https://github.com/llvm/llvm-project/commit/f7f2e4261a98b2da519d58e7f6794b013cda7a4b


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94973

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

[PATCH] D95246: [test] Use host platform specific error message substitution in lit tests

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov reopened this revision.
ASDenysPetrov added a comment.
This revision is now accepted and ready to land.
Herald added a project: lld-macho.
Herald added a reviewer: lld-macho.

This patch causes fails in a bunch of test files. When I roll it back, it 
passes.
Below you can see fail output of `ubsan-blacklist-vfs.c` and 
`basic-block-sections.c`.
F15539561: fail_output.txt 

Please, pay attention to this. 
I'm using build on Win10 + GCC config.

P.S. The similar trouble is in D95808 . I've 
commented there as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95246

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

[PATCH] D96725: [clang-tidy] Fix modernize-use-using in extern C code

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp:305
 };
+
+extern "C" {

steveire wrote:
> Can you add tests for typedefs in other scopes like
> 
> ```
> 
> extern "C" {
> typedef int CType;
> 
> struct CAnother {
> };
> 
> typedef struct {
>   int b;
>   typedef struct CAnother AStruct;
> } CStruct;
> 
> void foo()
> {
>   typedef struct CAnother AStruct;
> }
> }
> 
> ```
I'm not sure those tests add any value. For the struct, you can't have a 
typedef in a struct in c. The function typedef is valid in c, but I still can't 
see a reason to use extern C when defining a function in c++


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96725

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

[PATCH] D96138: [clang-tidy] Simplify delete null ptr check

[Nathan James via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96138

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

[PATCH] D95246: [test] Use host platform specific error message substitution in lit tests

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

One more mention. There are a lot of popups like this which stops the process 
untill press OK.  F15539777: test_err.png 
That's very annoying. Please revert or fix this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95246

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

[PATCH] D96725: [clang-tidy] Fix modernize-use-using in extern C code

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:19
 
+AST_MATCHER(Decl, isInExternC) {
+  return Node.getDeclContext()->isExternCContext();

This matcher may be useful for other checks, for example, 
`modernize-use-nullptr`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96725

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

[PATCH] D96725: [clang-tidy] Fix modernize-use-using in extern C code

[Stephen Kelly via Phabricator via cfe-commits]

steveire accepted this revision.
steveire added inline comments.
This revision is now accepted and ready to land.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp:305
 };
+
+extern "C" {

njames93 wrote:
> steveire wrote:
> > Can you add tests for typedefs in other scopes like
> > 
> > ```
> > 
> > extern "C" {
> > typedef int CType;
> > 
> > struct CAnother {
> > };
> > 
> > typedef struct {
> >   int b;
> >   typedef struct CAnother AStruct;
> > } CStruct;
> > 
> > void foo()
> > {
> >   typedef struct CAnother AStruct;
> > }
> > }
> > 
> > ```
> I'm not sure those tests add any value. For the struct, you can't have a 
> typedef in a struct in c. The function typedef is valid in c, but I still 
> can't see a reason to use extern C when defining a function in c++
Fair enough - Clang accepts the typedef in the struct with a warning, but it 
doesn't get transformed by the check anyway. The one in the function doesn't 
get transformed either, and if functions are so unlikely to appear in extern C, 
that it doesn't need to be covered, that's fine with me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96725

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

[PATCH] D96222: [clang-tidy] Simplify redundant smartptr get check

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp:21-45
+  return expr(
+ anyOf(cxxMemberCallExpr(
+   on(expr(anyOf(hasType(OnClass),
+ hasType(qualType(pointsTo(
+ decl(OnClass).bind("ptr_to_ptr"))
+  .bind("smart_pointer")),
+   unless(callee(

The patch says its simplifying the check, but this doesn't look like its 
simplifying it. Seems to be extending it to support dependent members.



Comment at: 
clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp:158
 
+  auto SR = GetCall->getSourceRange();
+  // CXXDependentScopeMemberExpr source range does not include parens

nit: Spell out the type.



Comment at: 
clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp:161
+  // Extend the source range of the get call to account for them.
+  if (isa(GetCall)) {
+SR.setEnd(Lexer::getLocForEndOfToken(SR.getEnd(), 0, *Result.SourceManager,

nit: Elide braces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96222

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

[PATCH] D95754: [clang] Print 32 candidates on the first failure, with -fshow-overloads=best.

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:2310
 
+  S.Diags.noteNumOverloadCandidatesShown(ShownOverloads);
+

jlebar wrote:
> aaronpuchert wrote:
> > Why not in the following `if`? I assume we want to show a long list not 
> > necessarily once, but only if it comes with the first error?
> My intent was to show the long list once, even if it's not the very first 
> error.  My thought process:
> 
> All things being equal, it's better to show more information to the user than 
> less.  The problem is, at some point, the amount of information we show 
> becomes overwhelming and spammy.  Particularly problematic are multiline 
> errors, because then you get O(nm) error lines across the whole TU.  We 
> prevent the O(nm) overwhelm by limiting the number of lines a particular 
> error can produce (using the mechanism in question here, or the template 
> backtrace limit, etc), and then also limiting the total number of individual 
> errors before we stop printing those.
> 
> With this change, we display the full(ish) error the first time it occurs and 
> then the truncated error every other time.  So in total it's O(n + m) rather 
> than O(nm).
Got it, but currently you'd not be exhausting the option to print a long list 
once: when the first overload resolution error has fewer candidates than the 
limit you'd still say we stop printing long lists of notes from now on. That 
confused me because you're calling `noteNumOverloadCandidatesShown` but you 
might not actually have printed that note.

If you're going by the //O(n+m)// argument, you could put this call into the 
`if (SuppressedOverloads)` and still stay under that limit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95754

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

[PATCH] D96142: [clang-tidy] Simplify too-small loop variable check

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D96142#2547418 , @steveire wrote:

> In D96142#2545078 , @njames93 wrote:
>
>> I'm not sure about this. The warning is good and addresses a real problem.
>
> Well, I've made the diagnostic better anyway.

I'm not sure its an improvement. If the template is never instantiated with 
something that would trigger this, we shouldn't really warn on it.
There are plenty of times when certain template definitions will error if 
certain template parameters are passed to them.
This is well defined and no compiler will ever warn on those situations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96142

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

[PATCH] D96665: Revert "Implement nullPointerConstant() using a better API."

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

I'm a little confused here, Typically if a commit is breaking bots a review 
doesn't need to be opened to revert it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96665

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