[PATCH] D70935: [CodeGen][ObjC] Emit a primitive store to store a __strong field in ExpandTypeFromArgs

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak closed this revision.
ahatanak added a comment.

Fixed in d8136f14f125fb27f2326f397df0964bf62078ca 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70935



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

[clang] d8136f1 - [CodeGen][ObjC] Emit a primitive store to store a __strong field in

[Akira Hatanaka via cfe-commits]

Author: Akira Hatanaka
Date: 2019-12-03T23:44:30-08:00
New Revision: d8136f14f125fb27f2326f397df0964bf62078ca

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

LOG: [CodeGen][ObjC] Emit a primitive store to store a __strong field in
ExpandTypeFromArgs

This fixes a bug in IRGen where a call to `llvm.objc.storeStrong` was
being emitted to initialize a __strong field of an uninitialized
temporary struct, which caused crashes at runtime.

rdar://problem/51807365

Added: 
clang/test/CodeGenObjC/nontrivial-struct-param-init.m

Modified: 
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGExpr.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index f992f904fe75..ca6b1d409c24 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -1047,8 +1047,13 @@ void CodeGenFunction::ExpandTypeFromArgs(
 auto imagValue = *AI++;
 EmitStoreOfComplex(ComplexPairTy(realValue, imagValue), LV, /*init*/ true);
   } else {
+// Call EmitStoreOfScalar except when the lvalue is a bitfield to emit a
+// primitive store.
 assert(isa(Exp.get()));
-EmitStoreThroughLValue(RValue::get(*AI++), LV);
+if (LV.isBitField())
+  EmitStoreThroughLValue(RValue::get(*AI++), LV);
+else
+  EmitStoreOfScalar(*AI++, LV);
   }
 }
 

diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 6e3a26e2c78a..cd42faefab00 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -4544,7 +4544,11 @@ RValue CodeGenFunction::EmitRValueForField(LValue LV,
 // don't load reference fields.
 if (FD->getType()->isReferenceType())
   return RValue::get(FieldLV.getPointer(*this));
-return EmitLoadOfLValue(FieldLV, Loc);
+// Call EmitLoadOfScalar except when the lvalue is a bitfield to emit a
+// primitive load.
+if (FieldLV.isBitField())
+  return EmitLoadOfLValue(FieldLV, Loc);
+return RValue::get(EmitLoadOfScalar(FieldLV, Loc));
   }
   llvm_unreachable("bad evaluation kind");
 }

diff  --git a/clang/test/CodeGenObjC/nontrivial-struct-param-init.m 
b/clang/test/CodeGenObjC/nontrivial-struct-param-init.m
new file mode 100644
index ..96a63b83ac76
--- /dev/null
+++ b/clang/test/CodeGenObjC/nontrivial-struct-param-init.m
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple i386-apple-watchos6.0-simulator -emit-llvm -fblocks 
-fobjc-arc -o - %s | FileCheck %s
+
+// CHECK: %[[STRUCT_S:.*]] = type { i8* }
+
+typedef struct {
+  id x;
+} S;
+
+// CHECK: define void @test0(i8* %[[A_0:.*]])
+// CHECK: %[[A:.*]] = alloca %[[STRUCT_S]], align 4
+// CHECK: %[[X:.*]] = getelementptr inbounds %[[STRUCT_S]], %[[STRUCT_S]]* 
%[[A]], i32 0, i32 0
+// CHECK: store i8* %[[A_0]], i8** %[[X]], align 4
+// CHECK: %[[V0:.*]] = bitcast %[[STRUCT_S]]* %[[A]] to i8**
+// CHECK: call void @__destructor_4_s0(i8** %[[V0]]) #2
+
+void test0(S a) {
+}



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

[PATCH] D70656: [clangd] Define out-of-line qualify function name

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGddcce0f3d665: [clangd] Define out-of-line qualify function 
name (authored by kadircet).

Changed prior to commit:
  https://reviews.llvm.org/D70656?vs=231513=232042#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70656

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2004,7 +2004,7 @@
 Bar foo() ;
   };
 })cpp",
-   "a::Foo::Bar foo() { return {}; }\n"},
+   "a::Foo::Bar a::Foo::foo() { return {}; }\n"},
   {R"cpp(
 class Foo;
 Foo fo^o() { return; })cpp",
@@ -2022,6 +2022,58 @@
   }
 }
 
+TEST_F(DefineOutlineTest, QualifyFunctionName) {
+  FileName = "Test.hpp";
+  struct {
+llvm::StringRef TestHeader;
+llvm::StringRef TestSource;
+llvm::StringRef ExpectedHeader;
+llvm::StringRef ExpectedSource;
+  } Cases[] = {
+  {
+  R"cpp(
+namespace a {
+  namespace b {
+class Foo {
+  void fo^o() {}
+};
+  }
+})cpp",
+  "",
+  R"cpp(
+namespace a {
+  namespace b {
+class Foo {
+  void foo() ;
+};
+  }
+})cpp",
+  "void a::b::Foo::foo() {}\n",
+  },
+  {
+  "namespace a { namespace b { void f^oo() {} } }",
+  "namespace a{}",
+  "namespace a { namespace b { void foo() ; } }",
+  "namespace a{void b::foo() {} }",
+  },
+  {
+  "namespace a { namespace b { void f^oo() {} } }",
+  "using namespace a;",
+  "namespace a { namespace b { void foo() ; } }",
+  // FIXME: Take using namespace directives in the source file into
+  // account. This can be spelled as b::foo instead.
+  "using namespace a;void a::b::foo() {} ",
+  },
+  };
+  llvm::StringMap EditedFiles;
+  for (auto  : Cases) {
+ExtraFiles["Test.cpp"] = Case.TestSource;
+EXPECT_EQ(apply(Case.TestHeader, ), Case.ExpectedHeader);
+EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+ testPath("Test.cpp"), Case.ExpectedSource)))
+<< Case.TestHeader;
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -136,7 +136,6 @@
 // Contains function signature, body and template parameters if applicable.
 // No need to qualify parameters, as they are looked up in the context
 // containing the function/method.
-// FIXME: Qualify function name depending on the target context.
 llvm::Expected
 getFunctionSourceCode(const FunctionDecl *FD, llvm::StringRef TargetNamespace) {
   auto  = FD->getASTContext().getSourceManager();
@@ -149,16 +148,17 @@
   llvm::Error Errors = llvm::Error::success();
   tooling::Replacements QualifierInsertions;
 
-  // Finds the first unqualified name in function return type and qualifies it
-  // to be valid in TargetContext.
+  // Finds the first unqualified name in function return type and name, then
+  // qualifies those to be valid in TargetContext.
   findExplicitReferences(FD, [&](ReferenceLoc Ref) {
 // It is enough to qualify the first qualifier, so skip references with a
 // qualifier. Also we can't do much if there are no targets or name is
 // inside a macro body.
 if (Ref.Qualifier || Ref.Targets.empty() || Ref.NameLoc.isMacroID())
   return;
-// Qualify return type
-if (Ref.NameLoc != FD->getReturnTypeSourceRange().getBegin())
+// Only qualify return type and function name.
+if (Ref.NameLoc != FD->getReturnTypeSourceRange().getBegin() &&
+Ref.NameLoc != FD->getLocation())
   return;
 
 for (const NamedDecl *ND : Ref.Targets) {
@@ -293,9 +293,7 @@
 auto FuncDef =
 getFunctionSourceCode(Source, InsertionPoint->EnclosingNamespace);
 if (!FuncDef)
-  return llvm::createStringError(
-  llvm::inconvertibleErrorCode(),
-  "Couldn't get full source for function definition.");
+  return FuncDef.takeError();
 
 SourceManagerForFile SMFF(*CCFile, Contents);
 const tooling::Replacement InsertFunctionDef(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D70535: [clangd] Define out-of-line qualify return value

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe4609ec0e8cf: [clangd] Define out-of-line qualify return 
value (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70535

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1972,6 +1972,56 @@
   }
 }
 
+TEST_F(DefineOutlineTest, QualifyReturnValue) {
+  FileName = "Test.hpp";
+  ExtraFiles["Test.cpp"] = "";
+
+  struct {
+llvm::StringRef Test;
+llvm::StringRef ExpectedHeader;
+llvm::StringRef ExpectedSource;
+  } Cases[] = {
+  {R"cpp(
+namespace a { class Foo; }
+using namespace a;
+Foo fo^o() { return; })cpp",
+   R"cpp(
+namespace a { class Foo; }
+using namespace a;
+Foo foo() ;)cpp",
+   "a::Foo foo() { return; }"},
+  {R"cpp(
+namespace a {
+  class Foo {
+class Bar {};
+Bar fo^o() { return {}; }
+  };
+})cpp",
+   R"cpp(
+namespace a {
+  class Foo {
+class Bar {};
+Bar foo() ;
+  };
+})cpp",
+   "a::Foo::Bar foo() { return {}; }\n"},
+  {R"cpp(
+class Foo;
+Foo fo^o() { return; })cpp",
+   R"cpp(
+class Foo;
+Foo foo() ;)cpp",
+   "Foo foo() { return; }"},
+  };
+  llvm::StringMap EditedFiles;
+  for (auto  : Cases) {
+apply(Case.Test, );
+EXPECT_EQ(apply(Case.Test, ), Case.ExpectedHeader);
+EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+ testPath("Test.cpp"), Case.ExpectedSource)));
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -6,13 +6,17 @@
 //
 //===--===//
 
+#include "AST.h"
+#include "FindTarget.h"
 #include "HeaderSourceSwitch.h"
+#include "Logger.h"
 #include "Path.h"
 #include "Selection.h"
 #include "SourceCode.h"
 #include "refactor/Tweak.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Stmt.h"
 #include "clang/Basic/SourceLocation.h"
@@ -20,10 +24,13 @@
 #include "clang/Driver/Types.h"
 #include "clang/Format/Format.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -57,31 +64,136 @@
   return getCorrespondingHeaderOrSource(FileName, Sel.AST, Sel.Index);
 }
 
-// Creates a modified version of function definition that can be inserted at a
-// different location. Contains both function signature and body.
-llvm::Optional getFunctionSourceCode(const FunctionDecl *FD) {
-  auto  = FD->getASTContext().getSourceManager();
-  auto CharRange = toHalfOpenFileRange(SM, FD->getASTContext().getLangOpts(),
-   FD->getSourceRange());
-  if (!CharRange)
+// Synthesize a DeclContext for TargetNS from CurContext. TargetNS must be empty
+// for global namespace, and endwith "::" otherwise.
+// Returns None if TargetNS is not a prefix of CurContext.
+llvm::Optional
+findContextForNS(llvm::StringRef TargetNS, const DeclContext *CurContext) {
+  assert(TargetNS.empty() || TargetNS.endswith("::"));
+  // Skip any non-namespace contexts, e.g. TagDecls, functions/methods.
+  CurContext = CurContext->getEnclosingNamespaceContext();
+  // If TargetNS is empty, it means global ns, which is translation unit.
+  if (TargetNS.empty()) {
+while (!CurContext->isTranslationUnit())
+  CurContext = CurContext->getParent();
+return CurContext;
+  }
+  // Otherwise we need to drop any trailing namespaces from CurContext until
+  // we reach TargetNS.
+  std::string TargetContextNS =
+  CurContext->isNamespace()
+  ? llvm::cast(CurContext)->getQualifiedNameAsString()
+  : "";
+  TargetContextNS.append("::");
+
+  llvm::StringRef CurrentContextNS(TargetContextNS);
+  // If TargetNS is not a prefix of CurrentContext, there's no way to reach
+  // it.
+  if (!CurrentContextNS.startswith(TargetNS))
 return llvm::None;
+
+  while (CurrentContextNS != TargetNS) {
+CurContext = 

[PATCH] D69298: [clangd] Define out-of-line initial apply logic

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGce2189202245: [clangd] Define out-of-line initial apply 
logic (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69298

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1868,6 +1868,110 @@
 })cpp");
 }
 
+TEST_F(DefineOutlineTest, FailsWithoutSource) {
+  FileName = "Test.hpp";
+  llvm::StringRef Test = "void fo^o() { return; }";
+  llvm::StringRef Expected =
+  "fail: Couldn't find a suitable implementation file.";
+  EXPECT_EQ(apply(Test), Expected);
+}
+
+TEST_F(DefineOutlineTest, ApplyTest) {
+  llvm::StringMap EditedFiles;
+  ExtraFiles["Test.cpp"] = "";
+  FileName = "Test.hpp";
+
+  struct {
+llvm::StringRef Test;
+llvm::StringRef ExpectedHeader;
+llvm::StringRef ExpectedSource;
+  } Cases[] = {
+  // Simple check
+  {
+  "void fo^o() { return; }",
+  "void foo() ;",
+  "void foo() { return; }",
+  },
+  // Templated function.
+  {
+  "template  void fo^o(T, T x) { return; }",
+  "template  void foo(T, T x) ;",
+  "template  void foo(T, T x) { return; }",
+  },
+  {
+  "template  void fo^o() { return; }",
+  "template  void foo() ;",
+  "template  void foo() { return; }",
+  },
+  // Template specialization.
+  {
+  R"cpp(
+template  void foo();
+template <> void fo^o() { return; })cpp",
+  R"cpp(
+template  void foo();
+template <> void foo() ;)cpp",
+  "template <> void foo() { return; }",
+  },
+  };
+  for (const auto  : Cases) {
+SCOPED_TRACE(Case.Test);
+EXPECT_EQ(apply(Case.Test, ), Case.ExpectedHeader);
+EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+ testPath("Test.cpp"), Case.ExpectedSource)));
+  }
+}
+
+TEST_F(DefineOutlineTest, HandleMacros) {
+  llvm::StringMap EditedFiles;
+  ExtraFiles["Test.cpp"] = "";
+  FileName = "Test.hpp";
+
+  struct {
+llvm::StringRef Test;
+llvm::StringRef ExpectedHeader;
+llvm::StringRef ExpectedSource;
+  } Cases[] = {
+  {R"cpp(
+  #define BODY { return; }
+  void f^oo()BODY)cpp",
+   R"cpp(
+  #define BODY { return; }
+  void foo();)cpp",
+   "void foo()BODY"},
+
+  {R"cpp(
+  #define BODY return;
+  void f^oo(){BODY})cpp",
+   R"cpp(
+  #define BODY return;
+  void foo();)cpp",
+   "void foo(){BODY}"},
+
+  {R"cpp(
+  #define TARGET void foo()
+  [[TARGET]]{ return; })cpp",
+   R"cpp(
+  #define TARGET void foo()
+  TARGET;)cpp",
+   "TARGET{ return; }"},
+
+  {R"cpp(
+  #define TARGET foo
+  void [[TARGET]](){ return; })cpp",
+   R"cpp(
+  #define TARGET foo
+  void TARGET();)cpp",
+   "void TARGET(){ return; }"},
+  };
+  for (const auto  : Cases) {
+SCOPED_TRACE(Case.Test);
+EXPECT_EQ(apply(Case.Test, ), Case.ExpectedHeader);
+EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+ testPath("Test.cpp"), Case.ExpectedSource)));
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/TweakTesting.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTesting.cpp
+++ clang-tools-extra/clangd/unittests/TweakTesting.cpp
@@ -127,7 +127,7 @@
 ADD_FAILURE() << "There were changes to additional files, but client "
  "provided a nullptr for EditedFiles.";
   else
-EditedFiles->try_emplace(It.first(), Unwrapped.str());
+EditedFiles->insert_or_assign(It.first(), Unwrapped.str());
 }
   }
   return EditedMainFile;
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -13,9 +13,17 @@
 #include "refactor/Tweak.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Stmt.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Driver/Types.h"
+#include "clang/Format/Format.h"
+#include "clang/Tooling/Core/Replacement.h"
 

[PATCH] D69266: [clangd] Define out-of-line availability checks

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9f251eece466: [clangd] Define out-of-line availability 
checks (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69266

Files:
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/TweakTesting.h
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1807,6 +1807,67 @@
   EXPECT_EQ(apply(Test), Expected) << Test;
 }
 
+TWEAK_TEST(DefineOutline);
+TEST_F(DefineOutlineTest, TriggersOnFunctionDecl) {
+  FileName = "Test.cpp";
+  // Not available unless in a header file.
+  EXPECT_UNAVAILABLE(R"cpp(
+[[void [[f^o^o]]() [[{
+  return;
+})cpp");
+
+  FileName = "Test.hpp";
+  // Not available unless function name or fully body is selected.
+  EXPECT_UNAVAILABLE(R"cpp(
+// Not a definition
+vo^i[[d^ ^f]]^oo();
+
+[[vo^id ]]foo[[()]] {[[
+  [[(void)(5+3);
+  return;]]
+}]])cpp");
+
+  // Available even if there are no implementation files.
+  EXPECT_AVAILABLE(R"cpp(
+[[void [[f^o^o]]() [[{
+  return;
+})cpp");
+
+  // Not available for out-of-line methods.
+  EXPECT_UNAVAILABLE(R"cpp(
+class Bar {
+  void baz();
+};
+
+[[void [[Bar::[[b^a^z() [[{
+  return;
+})cpp");
+
+  // Basic check for function body and signature.
+  EXPECT_AVAILABLE(R"cpp(
+class Bar {
+  [[void [[f^o^o]]() [[{ return; }
+};
+
+void foo();
+[[void [[f^o^o]]() [[{
+  return;
+})cpp");
+
+  // Not available on defaulted/deleted members.
+  EXPECT_UNAVAILABLE(R"cpp(
+class Foo {
+  Fo^o() = default;
+  F^oo(const Foo&) = delete;
+};)cpp");
+
+  // Not available within templated classes, as it is hard to spell class name
+  // out-of-line in such cases.
+  EXPECT_UNAVAILABLE(R"cpp(
+template  struct Foo { void fo^o(){} };
+})cpp");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/TweakTesting.h
===
--- clang-tools-extra/clangd/unittests/TweakTesting.h
+++ clang-tools-extra/clangd/unittests/TweakTesting.h
@@ -12,6 +12,7 @@
 #include "TestTU.h"
 #include "index/Index.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
@@ -62,6 +63,8 @@
   // testcases.
   std::string Header;
 
+  llvm::StringRef FileName = "TestTU.cpp";
+
   // Extra flags passed to the compilation in apply().
   std::vector ExtraArgs;
 
Index: clang-tools-extra/clangd/unittests/TweakTesting.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTesting.cpp
+++ clang-tools-extra/clangd/unittests/TweakTesting.cpp
@@ -63,12 +63,14 @@
   cantFail(positionToOffset(A.code(), SelectionRng.end))};
 }
 
-MATCHER_P6(TweakIsAvailable, TweakID, Ctx, Header, ExtraArgs, ExtraFiles, Index,
+MATCHER_P7(TweakIsAvailable, TweakID, Ctx, Header, ExtraArgs, ExtraFiles, Index,
+   FileName,
(TweakID + (negation ? " is unavailable" : " is available")).str()) {
   std::string WrappedCode = wrap(Ctx, arg);
   Annotations Input(WrappedCode);
   auto Selection = rangeOrPoint(Input);
   TestTU TU;
+  TU.Filename = FileName;
   TU.HeaderCode = Header;
   TU.Code = Input.code();
   TU.ExtraArgs = ExtraArgs;
@@ -91,6 +93,7 @@
   auto Selection = rangeOrPoint(Input);
 
   TestTU TU;
+  TU.Filename = FileName;
   TU.HeaderCode = Header;
   TU.AdditionalFiles = std::move(ExtraFiles);
   TU.Code = Input.code();
@@ -132,7 +135,7 @@
 
 ::testing::Matcher TweakTest::isAvailable() const {
   return TweakIsAvailable(llvm::StringRef(TweakID), Context, Header, ExtraArgs,
-  ExtraFiles, Index.get());
+  ExtraFiles, Index.get(), FileName);
 }
 
 std::vector TweakTest::expandCases(llvm::StringRef MarkedCode) {
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -0,0 +1,109 @@
+//===--- DefineOutline.cpp ---*- 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
+//

[PATCH] D70157: Align branches within 32-Byte boundary

[annita.zhang via Phabricator via cfe-commits]

annita.zhang added a comment.

In D70157#1768319 , @chandlerc wrote:

> I'm seeing lots of updates to fix bugs, but no movement for many days on both 
> my meta comments and (in some ways more importantly) James's meta comments. 
> (And thanks Philip for chiming in too!)
>
> Meanwhile, we really, really need to get this functionality in place. The 
> entire story for minimizing the new microcode performance hit hinges on these 
> patches, and I'm really worried by how little progress we're seeing here.


Sorry for belated response. We're working hard to go through some paper work to 
get the performance data ready. I think maybe it's better to open a mailing 
thread in llvm-dev to post those performance data and discuss those suggestions.

Thanks,
Annita


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

https://reviews.llvm.org/D70157



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

[PATCH] D70535: [clangd] Define out-of-line qualify return value

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 60447 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70535



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

[clang-tools-extra] ce21892 - [clangd] Define out-of-line initial apply logic

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-12-04T08:21:09+01:00
New Revision: ce2189202245953cbbfff100e6e5e9c1acb664ad

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

LOG: [clangd] Define out-of-line initial apply logic

Summary:
Initial implementation for apply logic, replaces function body with a
semicolon in source location and copies the full function definition into target
location.

Will handle qualification of return type and function name in following patches
to keep the changes small.

Reviewers: hokein

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
clang-tools-extra/clangd/unittests/TweakTesting.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
index c06945a632cc..102c650eec11 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -13,9 +13,17 @@
 #include "refactor/Tweak.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Stmt.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Driver/Types.h"
+#include "clang/Format/Format.h"
+#include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/Optional.h"
-#include "llvm/Support/Path.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include 
 
 namespace clang {
 namespace clangd {
@@ -40,6 +48,51 @@ const FunctionDecl *getSelectedFunction(const 
SelectionTree::Node *SelNode) {
   return nullptr;
 }
 
+llvm::Optional getSourceFile(llvm::StringRef FileName,
+   const Tweak::Selection ) {
+  if (auto Source = getCorrespondingHeaderOrSource(
+  FileName,
+  ().getFileManager().getVirtualFileSystem()))
+return *Source;
+  return getCorrespondingHeaderOrSource(FileName, Sel.AST, Sel.Index);
+}
+
+// Creates a modified version of function definition that can be inserted at a
+// 
diff erent location. Contains both function signature and body.
+llvm::Optional getFunctionSourceCode(const FunctionDecl *FD) {
+  auto  = FD->getASTContext().getSourceManager();
+  auto CharRange = toHalfOpenFileRange(SM, FD->getASTContext().getLangOpts(),
+   FD->getSourceRange());
+  if (!CharRange)
+return llvm::None;
+  // Include template parameter list.
+  if (auto *FTD = FD->getDescribedFunctionTemplate())
+CharRange->setBegin(FTD->getBeginLoc());
+
+  // FIXME: Qualify return type.
+  // FIXME: Qualify function name depending on the target context.
+  return toSourceCode(SM, *CharRange);
+}
+
+// Returns the most natural insertion point for \p QualifiedName in \p 
Contents.
+// This currently cares about only the namespace proximity, but in feature it
+// should also try to follow ordering of declarations. For example, if decls
+// come in order `foo, bar, baz` then this function should return some point
+// between foo and baz for inserting bar.
+llvm::Expected getInsertionOffset(llvm::StringRef Contents,
+  llvm::StringRef QualifiedName,
+  const format::FormatStyle ) {
+  auto Region = getEligiblePoints(Contents, QualifiedName, Style);
+
+  assert(!Region.EligiblePoints.empty());
+  // FIXME: This selection can be made smarter by looking at the definition
+  // locations for adjacent decls to Source. Unfortunately psudeo parsing in
+  // getEligibleRegions only knows about namespace begin/end events so we
+  // can't match function start/end positions yet.
+  auto InsertionPoint = Region.EligiblePoints.back();
+  return positionToOffset(Contents, InsertionPoint);
+}
+
 /// Moves definition of a function/method to an appropriate implementation 
file.
 ///
 /// Before:
@@ -94,8 +147,64 @@ class DefineOutline : public Tweak {
   }
 
   Expected apply(const Selection ) override {
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "Not implemented yet");
+const SourceManager  = Sel.AST.getSourceManager();
+auto MainFileName =
+getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
+if (!MainFileName)
+  return llvm::createStringError(
+  llvm::inconvertibleErrorCode(),
+  "Couldn't get absolute path for mainfile.");
+
+auto CCFile = getSourceFile(*MainFileName, Sel);
+if (!CCFile)
+  return 

[clang-tools-extra] 9f251ee - [clangd] Define out-of-line availability checks

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-12-04T08:21:09+01:00
New Revision: 9f251eece46675ba0095baddebe90512abde6337

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

LOG: [clangd] Define out-of-line availability checks

Summary:
Initial availability checks for performing define out-of-line code
action, which is a refactoring that will help users move function/method
definitions from headers to implementation files.

Proposed implementation only checks whether we have an interesting selection,
namely function name or full function definition/body.

Reviewers: hokein

Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, 
cfe-commits

Tags: #clang

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

Added: 
clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp

Modified: 
clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
clang-tools-extra/clangd/unittests/TweakTesting.cpp
clang-tools-extra/clangd/unittests/TweakTesting.h
clang-tools-extra/clangd/unittests/TweakTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt 
b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
index ddf10a2ca2ba..cd68d7afe6ab 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
+++ b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
@@ -15,6 +15,7 @@ add_clang_library(clangDaemonTweaks OBJECT
   AnnotateHighlightings.cpp
   DumpAST.cpp
   DefineInline.cpp
+  DefineOutline.cpp
   ExpandAutoType.cpp
   ExpandMacro.cpp
   ExtractFunction.cpp

diff  --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
new file mode 100644
index ..c06945a632cc
--- /dev/null
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -0,0 +1,109 @@
+//===--- DefineOutline.cpp ---*- 
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
+//
+//===--===//
+
+#include "HeaderSourceSwitch.h"
+#include "Path.h"
+#include "Selection.h"
+#include "SourceCode.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Stmt.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/Support/Path.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+// Deduces the FunctionDecl from a selection. Requires either the function body
+// or the function decl to be selected. Returns null if none of the above
+// criteria is met.
+// FIXME: This is shared with define inline, move them to a common header once
+// we have a place for such.
+const FunctionDecl *getSelectedFunction(const SelectionTree::Node *SelNode) {
+  if (!SelNode)
+return nullptr;
+  const ast_type_traits::DynTypedNode  = SelNode->ASTNode;
+  if (const FunctionDecl *FD = AstNode.get())
+return FD;
+  if (AstNode.get() &&
+  SelNode->Selected == SelectionTree::Complete) {
+if (const SelectionTree::Node *P = SelNode->Parent)
+  return P->ASTNode.get();
+  }
+  return nullptr;
+}
+
+/// Moves definition of a function/method to an appropriate implementation 
file.
+///
+/// Before:
+/// a.h
+///   void foo() { return; }
+/// a.cc
+///   #include "a.h"
+///
+/// 
+///
+/// After:
+/// a.h
+///   void foo();
+/// a.cc
+///   #include "a.h"
+///   void foo() { return; }
+class DefineOutline : public Tweak {
+public:
+  const char *id() const override;
+
+  bool hidden() const override { return true; }
+  Intent intent() const override { return Intent::Refactor; }
+  std::string title() const override {
+return "Move function body to out-of-line.";
+  }
+
+  bool prepare(const Selection ) override {
+// Bail out if we are not in a header file.
+// FIXME: We might want to consider moving method definitions below class
+// definition even if we are inside a source file.
+if (!isHeaderFile(Sel.AST.getSourceManager().getFilename(Sel.Cursor),
+  Sel.AST.getASTContext().getLangOpts()))
+  return false;
+
+Source = getSelectedFunction(Sel.ASTSelection.commonAncestor());
+// Bail out if the selection is not a in-line function definition.
+if (!Source || !Source->doesThisDeclarationHaveABody() ||
+Source->isOutOfLine())
+  return false;
+
+// Bail out in templated classes, as it is hard to spell the class name, 
i.e
+// if the template parameter is unnamed.
+if (auto *MD = llvm::dyn_cast(Source)) {
+  if (MD->getParent()->isTemplated())
+return 

[PATCH] D70857: [llvm][Support] Take in CurrentDirectory as a parameter in ExpandResponseFiles

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: llvm/lib/Support/CommandLine.cpp:1183
 ExpandResponseFile(FName, Saver, Tokenizer, ExpandedArgv, MarkEOLs,
-   RelativeNames, FS)) {
+   RelativeNames, FS, CurrentDir)) {
   // We couldn't read this file, so we leave it in the argument stream and

sammccall wrote:
> this is subtle, consider a comment: CurrentDir is only relevant for 
> "top-level" expansions || !RelativeNames, but nested ones always have 
> absolute paths if RelativeNames so CurrentDir is ignored.
did better, changed ExpandResponseFile to take FName as an absolute path, PTAL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70857



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

[PATCH] D70535: [clangd] Define out-of-line qualify return value

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Fix tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70535

Files:
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -1974,6 +1974,56 @@
   }
 }
 
+TEST_F(DefineOutlineTest, QualifyReturnValue) {
+  FileName = "Test.hpp";
+  ExtraFiles["Test.cpp"] = "";
+
+  struct {
+llvm::StringRef Test;
+llvm::StringRef ExpectedHeader;
+llvm::StringRef ExpectedSource;
+  } Cases[] = {
+  {R"cpp(
+namespace a { class Foo; }
+using namespace a;
+Foo fo^o() { return; })cpp",
+   R"cpp(
+namespace a { class Foo; }
+using namespace a;
+Foo foo() ;)cpp",
+   "a::Foo foo() { return; }"},
+  {R"cpp(
+namespace a {
+  class Foo {
+class Bar {};
+Bar fo^o() { return {}; }
+  };
+})cpp",
+   R"cpp(
+namespace a {
+  class Foo {
+class Bar {};
+Bar foo() ;
+  };
+})cpp",
+   "a::Foo::Bar foo() { return {}; }\n"},
+  {R"cpp(
+class Foo;
+Foo fo^o() { return; })cpp",
+   R"cpp(
+class Foo;
+Foo foo() ;)cpp",
+   "Foo foo() { return; }"},
+  };
+  llvm::StringMap EditedFiles;
+  for (auto  : Cases) {
+apply(Case.Test, );
+EXPECT_EQ(apply(Case.Test, ), Case.ExpectedHeader);
+EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents(
+ testPath("Test.cpp"), Case.ExpectedSource)));
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -6,13 +6,17 @@
 //
 //===--===//
 
+#include "AST.h"
+#include "FindTarget.h"
 #include "HeaderSourceSwitch.h"
+#include "Logger.h"
 #include "Path.h"
 #include "Selection.h"
 #include "SourceCode.h"
 #include "refactor/Tweak.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Stmt.h"
 #include "clang/Basic/SourceLocation.h"
@@ -20,10 +24,13 @@
 #include "clang/Driver/Types.h"
 #include "clang/Format/Format.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -57,31 +64,136 @@
   return getCorrespondingHeaderOrSource(FileName, Sel.AST, Sel.Index);
 }
 
-// Creates a modified version of function definition that can be inserted at a
-// different location. Contains both function signature and body.
-llvm::Optional getFunctionSourceCode(const FunctionDecl *FD) {
-  auto  = FD->getASTContext().getSourceManager();
-  auto CharRange = toHalfOpenFileRange(SM, FD->getASTContext().getLangOpts(),
-   FD->getSourceRange());
-  if (!CharRange)
+// Synthesize a DeclContext for TargetNS from CurContext. TargetNS must be empty
+// for global namespace, and endwith "::" otherwise.
+// Returns None if TargetNS is not a prefix of CurContext.
+llvm::Optional
+findContextForNS(llvm::StringRef TargetNS, const DeclContext *CurContext) {
+  assert(TargetNS.empty() || TargetNS.endswith("::"));
+  // Skip any non-namespace contexts, e.g. TagDecls, functions/methods.
+  CurContext = CurContext->getEnclosingNamespaceContext();
+  // If TargetNS is empty, it means global ns, which is translation unit.
+  if (TargetNS.empty()) {
+while (!CurContext->isTranslationUnit())
+  CurContext = CurContext->getParent();
+return CurContext;
+  }
+  // Otherwise we need to drop any trailing namespaces from CurContext until
+  // we reach TargetNS.
+  std::string TargetContextNS =
+  CurContext->isNamespace()
+  ? llvm::cast(CurContext)->getQualifiedNameAsString()
+  : "";
+  TargetContextNS.append("::");
+
+  llvm::StringRef CurrentContextNS(TargetContextNS);
+  // If TargetNS is not a prefix of CurrentContext, there's no way to reach
+  // it.
+  if (!CurrentContextNS.startswith(TargetNS))
 return llvm::None;
+
+  while (CurrentContextNS != TargetNS) {
+CurContext = CurContext->getParent();
+// These colons always exists since TargetNS is a prefix of
+// 

[PATCH] D70157: Align branches within 32-Byte boundary

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h:158
+case X86::AND16i16:
+case X86::AND16mr:
+case X86::AND16ri:

None of the AND/ADD/SUB instructions ending in mr are eligible for macrofusion 
as far as I know. Those all involve a load and a store which is not supported 
by macrofusion.

We also lost all the ADD*_DB instructions from the macrofusion list. I believe 
they are in the existing list incorrectly. So removing them is correct, but as 
far as I can see that change was not mentioned in the description of this patch.

Can we split the macrofusion refactoring out of this patch so we can review it 
separately and hopefully get it committed sooner than the other review feedback.


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

https://reviews.llvm.org/D70157



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

[PATCH] D70157: Align branches within 32-Byte boundary

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc added a comment.

I'm seeing lots of updates to fix bugs, but no movement for many days on both 
my meta comments and (in some ways more importantly) James's meta comments. 
(And thanks Philip for chiming in too!)

Meanwhile, we really, really need to get this functionality in place. The 
entire story for minimizing the new microcode performance hit hinges on these 
patches, and I'm really worried by how little progress we're seeing here.


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

https://reviews.llvm.org/D70157



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

[PATCH] D70935: [CodeGen][ObjC] Emit a primitive store to store a __strong field in ExpandTypeFromArgs

[John McCall via Phabricator via cfe-commits]

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

Yes, that's great, thank you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70935



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

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Thx for this patch. Only two minor comments.




Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1230
-  "unknown '%0' device kind trait in the 'device' context selector set, 
expected"
-  " one of 'host', 'nohost', 'cpu', 'gpu' or 'fpga'">;
 

I would have expected this error to be still accurate, maybe with the addition 
", or quoted versions thereof".



Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:424
+if (ER.isUsable())
+  Values.push_back(ER);
   }

When would ER not be usable here? Is there a valid use case or should we assert 
it is?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70973



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

[PATCH] D70935: [CodeGen][ObjC] Emit a primitive store to store a __strong field in ExpandTypeFromArgs

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:1054
+ "pointer to __strong expected");
+  EmitStoreOfScalar(*AI++, LV);
+} else

rjmccall wrote:
> This definitely deserves a comment.
> 
> I don't think the assertion is right; the condition is that the type is legal 
> for a field in a struct that can be passed directly, and while that does 
> exclude `__weak` (because the struct will have to be passed indirectly) and 
> `__autoreleasing` (because that's not legal in a struct), it doesn't exclude 
> `__unsafe_unretained`.
> 
> This function is implementing an operation that's broadly meaningful (it's a 
> store-init of an owned value, in contrast to a store-init with an unowned 
> value which is what `isInit` is implementing) but not extensively used in the 
> C frontend.  On some level, I feel like we should probably teach 
> `EmitStoreThroughLValue` to handle that properly, but that's a more 
> significant refactor.
> 
> It does look like this change isn't enough to handle `__ptrauth`, which will 
> assume that the source value is signed appropriately for the unqualified type 
> when probably it should be signed appropriately for the qualifier (which, 
> like `__weak`, cannot be address-discriminated because it would stop being 
> passed directly).  Probably the default case should be to use 
> `EmitStoreOfScalar`, and `EmitStoreThroughLValue` should only be used if the 
> l-value is a bit-field (the only non-simple case that can actually happen 
> from drilling down to a field).
> 
> The same logic applies on the load side in the abstract, except that it is 
> only causing problems for `__ptrauth` (well, if we change the behavior above) 
> because loads from the ARC qualifiers don't implicitly retain.  Regardless, 
> analogously to this, `EmitRValueForField` should only be calling 
> `EmitLoadOfLValue` for bit-fields and should otherwise call 
> `EmitLoadOfScalar`.  Please add a comment on both sides making it clear that 
> the logic is expected to be parallel.
Ah right, the assertion isn't correct. My intention was to catch `__weak` 
pointers.

Let me know if the comment I added is OK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70935



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

[PATCH] D70439: [Analyzer][Docs][NFC] Add CodeChecker to the command line tools

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Hey, thanks for waiting on me! I'm slow these days, just 50 more mails to go >.<

This review strikes me as a wiki material. I wonder what's the community stance 
on making wikis. #showerthoughs




Comment at: clang/www/analyzer/codechecker.html:13
+
+
+

Note related to your patch, but SSI seem to be completely broken these days; 
previously the dropdown menus header kept working on the front page but now 
even that's missing. Patches are very welcome >.<



Comment at: clang/www/analyzer/codechecker.html:23
+
+CodeChecker check -b "cd ~/your-project && make clean && make" -o ~/results
+

I think you should start with installing CodeChecker. I.e.,
1. Clone it from github: `git clone ...`.
2. ???



Comment at: clang/www/analyzer/codechecker.html:28
+
+The check command will print an overview of the issues found in your 
project by the analyzers.
+

I'm confused. I obtained an overview, but what are the steps that i need to do 
if i want to actually view the issues? I have to start the web server anyway, 
right? Then what's the point of of invoking check separately? Can you invoke 
check from the web server directly?

I wish this document looked more like a step-by-step guide on how to obtain the 
results (or how to set up a collaborative server). Right now it's a collection 
of seemingly unrelated solutions for sub-problems that I don't immediately 
understand how to combine in order to obtain the desired result. 



Comment at: clang/www/analyzer/command-line.html:18
+
+The following tools are used commonly to run the analyzer from the command 
line.
+Both tools are wrapper scripts to drive the analysis and the underlying 
invocations of the Clang compiler:

The first thing that we want to underline here is that the user should 
ABSOLUTELY NOT try to read warnings from the command line. I still see a lot of 
users who try to read scan-build's standard output and understand warnings from 
there without seeing any path notes.

I suggest the following intro:

"Static Analyzer is by design a GUI tool. Its purpose is to find buggy 
execution paths in the program, and such paths are very hard to comprehend by 
looking at a non-interactive standard output. It is possible, however, to 
invoke the Static Analyzer from the command line in order to obtain analysis 
results, and then later view them interactively in a graphical interface."



Comment at: clang/www/analyzer/command-line.html:19
+The following tools are used commonly to run the analyzer from the command 
line.
+Both tools are wrapper scripts to drive the analysis and the underlying 
invocations of the Clang compiler:
+

Do you plan to eventually mention clang-tidy as well?



Comment at: clang/www/analyzer/command-line.html:22-27
+  
+Preferred on macOS.
+In tree, part of the LLVM project.
+Provides limited and unsupported https://clang.llvm.org/docs/analyzer/user-docs/CrossTranslationUnit.html;>Cross
 Translation Unit (CTU) analysis capabilities.
+
+  

WDYT of the following:

Scan-Build is an old and simple command-line tool that emits static analyzer 
warnings as HTML files while compiling your project. You can view analysis 
results in your web browser.
- Useful for individual developers who simply want to view static analysis 
results at their desk, or in very simple collaborative environment.
- Works on all major platforms (Windows, Linux, macOS) and is available as a 
package in many linux distributions.
- Does not include support for cross-translation-unit analysis.



Comment at: clang/www/analyzer/command-line.html:28
+  
+  CodeChecker
+  

Let's describe the tool a bit as well. Maybe something like this:

CodeChecker is a web server that runs the Static Analyzer on your projects on 
demand and maintains a database of issues.

- Perfect for managing large amounts of Static Analyzer warnings in a 
collaborative environment.
- Generally much more feature-rich than scan-build.
- (more stuff here)



Comment at: clang/www/analyzer/command-line.html:31
+Preferred on Linux.
+Out-of-tree, not part of the LLVM project, hosted on github.
+On Linux there are many projects for embedded devices which are built 
only with GCC.

LLVM is now also hosted on github, so not sure what the message is. We should 
probably either provide a link or just say that it's out-of-tree.



Comment at: clang/www/analyzer/command-line.html:32-33
+Out-of-tree, not part of the LLVM project, hosted on github.
+On Linux there are many projects for embedded devices which are built 
only with GCC.
+CodeChecker converts these GCC invocations to the appropriate Clang 
invocations, this way the CSA can work on these projects seamlessly.
+

[PATCH] D70935: [CodeGen][ObjC] Emit a primitive store to store a __strong field in ExpandTypeFromArgs

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 232027.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.

Call `EmitStoreThroughLValue` and `EmitLoadOfLValue` only when the lvalue is a 
bitfield.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70935

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGenObjC/nontrivial-struct-param-init.m


Index: clang/test/CodeGenObjC/nontrivial-struct-param-init.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/nontrivial-struct-param-init.m
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple i386-apple-watchos6.0-simulator -emit-llvm -fblocks 
-fobjc-arc -o - %s | FileCheck %s
+
+// CHECK: %[[STRUCT_S:.*]] = type { i8* }
+
+typedef struct {
+  id x;
+} S;
+
+// CHECK: define void @test0(i8* %[[A_0:.*]])
+// CHECK: %[[A:.*]] = alloca %[[STRUCT_S]], align 4
+// CHECK: %[[X:.*]] = getelementptr inbounds %[[STRUCT_S]], %[[STRUCT_S]]* 
%[[A]], i32 0, i32 0
+// CHECK: store i8* %[[A_0]], i8** %[[X]], align 4
+// CHECK: %[[V0:.*]] = bitcast %[[STRUCT_S]]* %[[A]] to i8**
+// CHECK: call void @__destructor_4_s0(i8** %[[V0]]) #2
+
+void test0(S a) {
+}
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -4544,7 +4544,11 @@
 // don't load reference fields.
 if (FD->getType()->isReferenceType())
   return RValue::get(FieldLV.getPointer(*this));
-return EmitLoadOfLValue(FieldLV, Loc);
+// Call EmitLoadOfScalar except when the lvalue is a bitfield to emit a
+// primitive load.
+if (FieldLV.isBitField())
+  return EmitLoadOfLValue(FieldLV, Loc);
+return RValue::get(EmitLoadOfScalar(FieldLV, Loc));
   }
   llvm_unreachable("bad evaluation kind");
 }
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1047,8 +1047,13 @@
 auto imagValue = *AI++;
 EmitStoreOfComplex(ComplexPairTy(realValue, imagValue), LV, /*init*/ true);
   } else {
+// Call EmitStoreOfScalar except when the lvalue is a bitfield to emit a
+// primitive store.
 assert(isa(Exp.get()));
-EmitStoreThroughLValue(RValue::get(*AI++), LV);
+if (LV.isBitField())
+  EmitStoreThroughLValue(RValue::get(*AI++), LV);
+else
+  EmitStoreOfScalar(*AI++, LV);
   }
 }
 


Index: clang/test/CodeGenObjC/nontrivial-struct-param-init.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/nontrivial-struct-param-init.m
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple i386-apple-watchos6.0-simulator -emit-llvm -fblocks -fobjc-arc -o - %s | FileCheck %s
+
+// CHECK: %[[STRUCT_S:.*]] = type { i8* }
+
+typedef struct {
+  id x;
+} S;
+
+// CHECK: define void @test0(i8* %[[A_0:.*]])
+// CHECK: %[[A:.*]] = alloca %[[STRUCT_S]], align 4
+// CHECK: %[[X:.*]] = getelementptr inbounds %[[STRUCT_S]], %[[STRUCT_S]]* %[[A]], i32 0, i32 0
+// CHECK: store i8* %[[A_0]], i8** %[[X]], align 4
+// CHECK: %[[V0:.*]] = bitcast %[[STRUCT_S]]* %[[A]] to i8**
+// CHECK: call void @__destructor_4_s0(i8** %[[V0]]) #2
+
+void test0(S a) {
+}
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -4544,7 +4544,11 @@
 // don't load reference fields.
 if (FD->getType()->isReferenceType())
   return RValue::get(FieldLV.getPointer(*this));
-return EmitLoadOfLValue(FieldLV, Loc);
+// Call EmitLoadOfScalar except when the lvalue is a bitfield to emit a
+// primitive load.
+if (FieldLV.isBitField())
+  return EmitLoadOfLValue(FieldLV, Loc);
+return RValue::get(EmitLoadOfScalar(FieldLV, Loc));
   }
   llvm_unreachable("bad evaluation kind");
 }
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1047,8 +1047,13 @@
 auto imagValue = *AI++;
 EmitStoreOfComplex(ComplexPairTy(realValue, imagValue), LV, /*init*/ true);
   } else {
+// Call EmitStoreOfScalar except when the lvalue is a bitfield to emit a
+// primitive store.
 assert(isa(Exp.get()));
-EmitStoreThroughLValue(RValue::get(*AI++), LV);
+if (LV.isBitField())
+  EmitStoreThroughLValue(RValue::get(*AI++), LV);
+else
+  EmitStoreOfScalar(*AI++, LV);
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69876: Allow output constraints on "asm goto"

[Bill Wendling via Phabricator via cfe-commits]

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



Comment at: clang/test/Analysis/uninit-asm-goto.cpp:10
+return -1;
+}

rnk wrote:
> This could use a test for the case where an input is uninitialized, and where 
> an uninitialized value is used along the error path.
I have a follow-up patch that I'll upload soon that warns about uninitialized 
variable use on the indirect paths. I wanted to separate it to keep this patch 
size down. If you'd prefer I can just add it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69876



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

[PATCH] D70575: [Clang] Define Fuchsia C++ABI

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9c3f9b9c12b0: [Clang] Define Fuchsia C++ABI (authored by 
phosek).

Changed prior to commit:
  https://reviews.llvm.org/D70575?vs=230827=232023#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70575

Files:
  clang/include/clang/Basic/TargetCXXABI.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/constructor-destructor-return-this.cpp

Index: clang/test/CodeGenCXX/constructor-destructor-return-this.cpp
===
--- clang/test/CodeGenCXX/constructor-destructor-return-this.cpp
+++ clang/test/CodeGenCXX/constructor-destructor-return-this.cpp
@@ -3,6 +3,8 @@
 //RUN: %clang_cc1 %s -emit-llvm -o - -triple=thumbv7-apple-ios5.0 -target-abi apcs-gnu | FileCheck --check-prefix=CHECKIOS5 %s
 //RUN: %clang_cc1 %s -emit-llvm -o - -triple=wasm32-unknown-unknown \
 //RUN:   | FileCheck --check-prefix=CHECKARM %s
+//RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-unknown-fuchsia | FileCheck --check-prefix=CHECKFUCHSIA %s
+//RUN: %clang_cc1 %s -emit-llvm -o - -triple=aarch64-unknown-fuchsia | FileCheck --check-prefix=CHECKFUCHSIA %s
 //RUN: %clang_cc1 %s -emit-llvm -o - -triple=i386-pc-win32 -fno-rtti | FileCheck --check-prefix=CHECKMS %s
 // FIXME: these tests crash on the bots when run with -triple=x86_64-pc-win32
 
@@ -45,6 +47,11 @@
 // CHECKIOS5-LABEL: define %class.B* @_ZN1BD2Ev(%class.B* %this)
 // CHECKIOS5-LABEL: define %class.B* @_ZN1BD1Ev(%class.B* %this)
 
+// CHECKFUCHSIA-LABEL: define %class.B* @_ZN1BC2EPi(%class.B* returned %this, i32* %i)
+// CHECKFUCHSIA-LABEL: define %class.B* @_ZN1BC1EPi(%class.B* returned %this, i32* %i)
+// CHECKFUCHSIA-LABEL: define %class.B* @_ZN1BD2Ev(%class.B* returned %this)
+// CHECKFUCHSIA-LABEL: define %class.B* @_ZN1BD1Ev(%class.B* returned %this)
+
 // CHECKMS-LABEL: define dso_local x86_thiscallcc %class.B* @"??0B@@QAE@PAH@Z"(%class.B* returned %this, i32* %i)
 // CHECKMS-LABEL: define dso_local x86_thiscallcc void @"??1B@@UAE@XZ"(%class.B* %this)
 
@@ -83,6 +90,14 @@
 // CHECKIOS5-LABEL: define void @_ZN1CD0Ev(%class.C* %this)
 // CHECKIOS5-LABEL: define void @_ZThn8_N1CD0Ev(%class.C* %this)
 
+// CHECKFUCHSIA-LABEL: define %class.C* @_ZN1CC2EPiPc(%class.C* returned %this, i32* %i, i8* %c)
+// CHECKFUCHSIA-LABEL: define %class.C* @_ZN1CC1EPiPc(%class.C* returned %this, i32* %i, i8* %c)
+// CHECKFUCHSIA-LABEL: define %class.C* @_ZN1CD2Ev(%class.C* returned %this)
+// CHECKFUCHSIA-LABEL: define %class.C* @_ZN1CD1Ev(%class.C* returned %this)
+// CHECKFUCHSIA-LABEL: define %class.C* @_ZThn16_N1CD1Ev(%class.C* %this)
+// CHECKFUCHSIA-LABEL: define void @_ZN1CD0Ev(%class.C* %this)
+// CHECKFUCHSIA-LABEL: define void @_ZThn16_N1CD0Ev(%class.C* %this)
+
 // CHECKMS-LABEL: define dso_local x86_thiscallcc %class.C* @"??0C@@QAE@PAHPAD@Z"(%class.C* returned %this, i32* %i, i8* %c)
 // CHECKMS-LABEL: define dso_local x86_thiscallcc void @"??1C@@UAE@XZ"(%class.C* %this)
 
@@ -110,6 +125,11 @@
 // CHECKIOS5-LABEL: define %class.D* @_ZN1DD2Ev(%class.D* %this, i8** %vtt)
 // CHECKIOS5-LABEL: define %class.D* @_ZN1DD1Ev(%class.D* %this)
 
+// CHECKFUCHSIA-LABEL: define %class.D* @_ZN1DC2Ev(%class.D* returned %this, i8** %vtt)
+// CHECKFUCHSIA-LABEL: define %class.D* @_ZN1DC1Ev(%class.D* returned %this)
+// CHECKFUCHSIA-LABEL: define %class.D* @_ZN1DD2Ev(%class.D* returned %this, i8** %vtt)
+// CHECKFUCHSIA-LABEL: define %class.D* @_ZN1DD1Ev(%class.D* returned %this)
+
 // CHECKMS-LABEL: define dso_local x86_thiscallcc %class.D* @"??0D@@QAE@XZ"(%class.D* returned %this, i32 %is_most_derived)
 // CHECKMS-LABEL: define dso_local x86_thiscallcc void @"??1D@@UAE@XZ"(%class.D* %this)
 
@@ -127,15 +147,15 @@
   e2->~E();
 }
 
-// CHECKARM-LABEL: define void @_Z15test_destructorv()
+// CHECKARM-LABEL,CHECKFUCHSIA-LABEL: define void @_Z15test_destructorv()
 
 // Verify that virtual calls to destructors are not marked with a 'returned'
 // this parameter at the call site...
-// CHECKARM: [[VFN:%.*]] = getelementptr inbounds %class.E* (%class.E*)*, %class.E* (%class.E*)**
-// CHECKARM: [[THUNK:%.*]] = load %class.E* (%class.E*)*, %class.E* (%class.E*)** [[VFN]]
-// CHECKARM: call %class.E* [[THUNK]](%class.E* %
+// CHECKARM,CHECKFUCHSIA: [[VFN:%.*]] = getelementptr inbounds %class.E* (%class.E*)*, %class.E* (%class.E*)**
+// CHECKARM,CHECKFUCHSIA: [[THUNK:%.*]] = load %class.E* (%class.E*)*, %class.E* (%class.E*)** [[VFN]]
+// CHECKARM,CHECKFUCHSIA: call %class.E* [[THUNK]](%class.E* %
 
 // ...but static calls create declarations with 'returned' this
-// CHECKARM: {{%.*}} = call %class.E* @_ZN1ED1Ev(%class.E* %
+// CHECKARM,CHECKFUCHSIA: {{%.*}} = call %class.E* @_ZN1ED1Ev(%class.E* %
 
-// CHECKARM: declare %class.E* @_ZN1ED1Ev(%class.E* 

[clang] 9c3f9b9 - [Clang] Define Fuchsia C++ABI

[Petr Hosek via cfe-commits]

Author: Petr Hosek
Date: 2019-12-03T18:35:57-08:00
New Revision: 9c3f9b9c12b0f79b74d1d349bbac46cadaca7dbf

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

LOG: [Clang] Define Fuchsia C++ABI

Currently, it is a modified version of the Itanium ABI, with the only
change being that constructors and destructors return 'this'.

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

Added: 


Modified: 
clang/include/clang/Basic/TargetCXXABI.h
clang/lib/AST/ASTContext.cpp
clang/lib/Basic/Targets/OSTargets.h
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/test/CodeGenCXX/constructor-destructor-return-this.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/TargetCXXABI.h 
b/clang/include/clang/Basic/TargetCXXABI.h
index 60343fe99c1d..1ab45d2ce9a1 100644
--- a/clang/include/clang/Basic/TargetCXXABI.h
+++ b/clang/include/clang/Basic/TargetCXXABI.h
@@ -103,6 +103,12 @@ class TargetCXXABI {
 /// of these details is necessarily final yet.
 WebAssembly,
 
+/// The Fuchsia ABI is a modified version of the Itanium ABI.
+///
+/// The relevant changes from the Itanium ABI are:
+///   - constructors and destructors return 'this', as in ARM.
+Fuchsia,
+
 /// The Microsoft ABI is the ABI used by Microsoft Visual Studio (and
 /// compatible compilers).
 ///
@@ -133,6 +139,7 @@ class TargetCXXABI {
   /// Does this ABI generally fall into the Itanium family of ABIs?
   bool isItaniumFamily() const {
 switch (getKind()) {
+case Fuchsia:
 case GenericAArch64:
 case GenericItanium:
 case GenericARM:
@@ -152,6 +159,7 @@ class TargetCXXABI {
   /// Is this ABI an MSVC-compatible ABI?
   bool isMicrosoft() const {
 switch (getKind()) {
+case Fuchsia:
 case GenericAArch64:
 case GenericItanium:
 case GenericARM:
@@ -182,6 +190,7 @@ class TargetCXXABI {
 case WebAssembly:
   // WebAssembly doesn't require any special alignment for member 
functions.
   return false;
+case Fuchsia:
 case GenericARM:
 case GenericAArch64:
 case GenericMIPS:
@@ -257,6 +266,7 @@ class TargetCXXABI {
   /// done on a generic Itanium platform.
   bool canKeyFunctionBeInline() const {
 switch (getKind()) {
+case Fuchsia:
 case GenericARM:
 case iOS64:
 case WebAssembly:
@@ -309,6 +319,7 @@ class TargetCXXABI {
 
 // iOS on ARM64 and WebAssembly use the C++11 POD rules.  They do not honor
 // the Itanium exception about classes with over-large bitfields.
+case Fuchsia:
 case iOS64:
 case WebAssembly:
 case WatchOS:

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index eb02a61b234f..006eb1e0defb 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -771,6 +771,7 @@ CXXABI *ASTContext::createCXXABI(const TargetInfo ) {
   if (!LangOpts.CPlusPlus) return nullptr;
 
   switch (T.getCXXABI().getKind()) {
+  case TargetCXXABI::Fuchsia:
   case TargetCXXABI::GenericARM: // Same as Itanium at this level
   case TargetCXXABI::iOS:
   case TargetCXXABI::iOS64:
@@ -10169,6 +10170,7 @@ MangleContext *ASTContext::createMangleContext(const 
TargetInfo *T) {
   if (!T)
 T = Target;
   switch (T->getCXXABI().getKind()) {
+  case TargetCXXABI::Fuchsia:
   case TargetCXXABI::GenericAArch64:
   case TargetCXXABI::GenericItanium:
   case TargetCXXABI::GenericARM:

diff  --git a/clang/lib/Basic/Targets/OSTargets.h 
b/clang/lib/Basic/Targets/OSTargets.h
index cc72a0a39f30..756cb7a8bbe3 100644
--- a/clang/lib/Basic/Targets/OSTargets.h
+++ b/clang/lib/Basic/Targets/OSTargets.h
@@ -808,6 +808,7 @@ class LLVM_LIBRARY_VISIBILITY FuchsiaTargetInfo : public 
OSTargetInfo {
   FuchsiaTargetInfo(const llvm::Triple , const TargetOptions )
   : OSTargetInfo(Triple, Opts) {
 this->MCountName = "__mcount";
+this->TheCXXABI.set(TargetCXXABI::Fuchsia);
   }
 };
 

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 9793a5ef4729..4959b80faec7 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -73,6 +73,7 @@ static const char AnnotationSection[] = "llvm.metadata";
 
 static CGCXXABI *createCXXABI(CodeGenModule ) {
   switch (CGM.getTarget().getCXXABI().getKind()) {
+  case TargetCXXABI::Fuchsia:
   case TargetCXXABI::GenericAArch64:
   case TargetCXXABI::GenericARM:
   case TargetCXXABI::iOS:

diff  --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp 
b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index 8f9b16470b64..515eb3f1f168 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -487,6 +487,19 @@ class iOS64CXXABI : public ARMCXXABI {
   bool shouldRTTIBeUnique() const override { 

[PATCH] D70958: [compiler-rt] [test] Disable ASLR on ASAN/MSAN/TSAN tests on NetBSD

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

In D70958#1767912 , @eugenis wrote:

> Could this go into the common lit config in compiler-rt/test?


Do you mean to run this for all tests in compiler-rt? We intend to disable it 
only when needed and prevent leaking regressions that violate ASLR to other 
components of LLVM.

> What is the problem with ASLR in NetBSD? Is this about fixed shadow location 
> conflicts with the binary & library mappings?

PaX ASLR on NetBSD is too aggressive with the layout randomization and we 
cannot predict ranges of the heap and the stack. Next, we cannot map reliably 
the memory used into shadow buffer that crashes in a cryptic way. We are fine 
with disabling ASLR for the sanitizers that need shadow buffers and origins.




Comment at: compiler-rt/test/sanitizer_common/netbsd_commands/run_noaslr.sh:4
+PATH=${PATH}:/usr/sbin
+paxctl +a "${1}"
+exec "${@}"

I propose to use `/usr/bin/paxctl` as it will be PATH and environment 
independent. We already use this direct path approach in pkgsrc.


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

https://reviews.llvm.org/D70958



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

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

[Florin Iucha via Phabricator via cfe-commits]

0x8000- added a comment.

In D45444#1766852 , @JonasToth wrote:

>




> Thank you for testing, I would appreciate if you test later versions, too!
>  I will focus on landing the utility function for adding `const` first. I 
> noticed false positives in LLVM as well, I try to reproduce them all and 
> fix/workaround the issues.

I will test it as often as you rebase it :) Thank you for the useful tool!

Are you going to keep updating it on 
https://github.com/JonasToth/llvm-project/commits/feature_check_const ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D45444



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

[clang] 59312cb - Fix warning on unused variable. NFC.

[Michael Liao via cfe-commits]

Author: Michael Liao
Date: 2019-12-03T21:16:10-05:00
New Revision: 59312cb0b81ca13f0674dde66b8e87a8d51d4dda

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

LOG: Fix warning on unused variable. NFC.

Added: 


Modified: 
clang/lib/AST/Expr.cpp

Removed: 




diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index a73531ad5fad..3bc2ea60aa14 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1678,7 +1678,7 @@ MemberExpr *MemberExpr::Create(
   MemberExpr *E = new (Mem) MemberExpr(Base, IsArrow, OperatorLoc, MemberDecl,
NameInfo, T, VK, OK, NOUR);
 
-  if (FieldDecl *Field = dyn_cast(MemberDecl)) {
+  if (isa(MemberDecl)) {
 DeclContext *DC = MemberDecl->getDeclContext();
 // dyn_cast_or_null is used to handle objC variables which do not
 // have a declaration context.



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

In D70696#1767616 , @dblaikie wrote:

> Many of the test cases could be collapsed into one file - using different 
> variables that are used, unused, locally or globally declared, etc.


Okay. Will try to consolidate into one or fewer files. Originally, I am using 
different files to avoid cases where in the future clang may generate different 
ordering w.r.t. different global variables.

> Is this new code only active for C compilations? (does clang reject requests 
> for the bpf target when the input is C++?) I ask due to the concerns around 
> globals used in inline functions where the inline function is unused - though 
> C has inline functions too, so I guess the question stands: Is that a 
> problem? What happens?

Currently, yes. my implementation only active for C compilation.
In the kernel documentation 
(https://www.kernel.org/doc/Documentation/networking/filter.txt), we have:

  The new instruction set was originally designed with the possible goal in
  mind to write programs in "restricted C" and compile into eBPF with a optional
  GCC/LLVM backend, so that it can just-in-time map to modern 64-bit CPUs with
  minimal performance overhead over two steps, that is, C -> eBPF -> native 
code.

For LLVM itself, people can compile a C++ program into BPF target. But 
"officially" we do not
support this. That is why I restricted to C only. For C++ programs, we don't 
get much usage/tests
from users.

Do you have a concrete example for this? I tried the following:

  -bash-4.4$ cat t.h
  inline int foo() { extern int g; return g; }
  -bash-4.4$ cat t.c
  int bar() { return 0; }
  -bash-4.4$ clang -target bpf -g -O0 -S -emit-llvm t.c

`foo` is not used, clang seems smart enough to deduce `g` is not used, so no 
debuginfo is emitted in this case.

In general, if an inline function is not used but an external variable is used 
inside that inline function, the worst case is extra debuginfo for that 
external variable. Since it is not used, it won't impact bpf loader.

> Should this be driven by a lower level of code generation - ie: is it OK to 
> only produce debug info descriptions for variables that are referenced in the 
> resulting LLVM IR? (compile time constants wouldn't be described then, for 
> instance - since they won't be code generated, loaded from memory, etc)

Yes, it is OK to only produce debug info only for variables that are referenced 
in the resulting LLVM IR. But we are discussing extern variables and no compile 
time constants here. Maybe I miss something?

> Is there somewhere I should be reading about the design requirement for these 
> global variable descriptions to understand the justification for them & the 
> ramifications if there are bugs that cause them not to be emitted?

We do not have design documents yet. The following are two links and I can 
explain more:

1. 
https://lore.kernel.org/bpf/CAEf4BzYCNo5GeVGMhp3fhysQ=_axAf=23ptwazs-yayafmx...@mail.gmail.com/T/#t

The typical config is at /boot/config-<...> in a linux machine. The config 
entry typically look like:

  CONFIG_CC_IS_GCC=y
  CONFIG_GCC_VERSION=40805
  CONFIG_INITRAMFS_SOURCE=""

Suppose a bpf program wants to check config value and based on its value to do 
something, user can write:

  extern bool CONFIG_CC_IS_GCC;
  extern int CONFIG_GCC_VERSION;
  extern char CONFIG_INITRAMFS_SOURCE[20];
  ...
  if (CONFIG_CC_IS_GCC) ...
  map_val = CONFIG_GCC_VERSION; 
  __builtin_memcpy(map_value, 8, CONFIG_INITRAMFS_SOURCE);

bpfloader will create a data section store all the above info and patch the 
correct address to the code.
Without extern var type info, it becomes a guess game what type/size the user 
is using.
Based on precise type information, bpf loader is able to do relocation much 
easily.

2. 
https://lore.kernel.org/bpf/87eez4odqp@toke.dk/T/#m8d5c3e87ffe7f2764e02d722cb0d8cbc136880ed

This is for bpf program verification.
For example,
bpf_prog1:

  foo(...) {
... x ... y ...
z =  bar(x /*struct t * */, y /* int */);
...
  }

and there is no bar body available yet.
The kernel verifier still able to verify program "foo"
and makes sure type leading to bar for all parameters
are correct.

Later, if there is a program
prog2(struct t *a, int b)
which is verified independently.

The in kernel, prog1 can call prog2 if there parameter types
and return types match. This is the BPF-way dynamic linking.
The types for external used functions can help cut down
verification cost at linking time.

If there is no debug information for these extern variables, the current
proposal is to fail the bpf loader and verifier. User can always workaround
such issues to create bpf maps for the first use case (which is more expensive 
and not user friendly) and do static
linking before loading into the kernel for the second use case.


Repository:
  rG LLVM Github Monorepo

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

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

As an FYI, this appears to cause several false positive warnings with the Linux 
kernel:

  ../drivers/video/fbdev/core/fbmem.c:665:3: warning: misleading indentation; 
statement is not part of the previous 'else' [-Wmisleading-indentation]
  if (fb_logo.depth > 4 && depth > 4) {
  ^
  ../drivers/video/fbdev/core/fbmem.c:661:2: note: previous statement is here
  else
  ^
  ../drivers/video/fbdev/core/fbmem.c:1075:3: warning: misleading indentation; 
statement is not part of the previous 'if' [-Wmisleading-indentation]
  return ret;
  ^
  ../drivers/video/fbdev/core/fbmem.c:1072:2: note: previous statement is here
  if (!ret)
  ^
  2 warnings generated.

Corresponding to:

https://elixir.bootlin.com/linux/v5.4.1/source/drivers/video/fbdev/core/fbmem.c#L665

and

https://elixir.bootlin.com/linux/v5.4.1/source/drivers/video/fbdev/core/fbmem.c#L1072


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70638



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

[PATCH] D69876: Allow output constraints on "asm goto"

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a subscriber: rsmith.
rnk added a comment.

Changes seem fine to me, FWIW.
+@rsmith




Comment at: clang/test/Analysis/uninit-asm-goto.cpp:10
+return -1;
+}

This could use a test for the case where an input is uninitialized, and where 
an uninitialized value is used along the error path.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69876



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

[PATCH] D62686: [RISCV] Add support for save/restore of callee-saved registers via libcalls

[Ana Pazos via Phabricator via cfe-commits]

apazos added a comment.

Lewis, try rebasing it, not applying cleanly nor https://reviews.llvm.org/D62190


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62686



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

[clang] b0df904 - [Diagnostic][test] Remove an unneeded change to pragma_diagnostic_sections.cpp after D70638

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2019-12-03T16:43:35-08:00
New Revision: b0df90488ce9c703d1bbc3d02c4b29e85d0f706b

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

LOG: [Diagnostic][test] Remove an unneeded change to 
pragma_diagnostic_sections.cpp after D70638

Added: 


Modified: 
clang/test/Preprocessor/pragma_diagnostic_sections.cpp

Removed: 




diff  --git a/clang/test/Preprocessor/pragma_diagnostic_sections.cpp 
b/clang/test/Preprocessor/pragma_diagnostic_sections.cpp
index 2bba7595a810..b680fae5b993 100644
--- a/clang/test/Preprocessor/pragma_diagnostic_sections.cpp
+++ b/clang/test/Preprocessor/pragma_diagnostic_sections.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -Wall -Wunused-macros -Wunused-parameter 
-Wno-uninitialized -Wno-misleading-indentation -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wall -Wunused-macros -Wunused-parameter 
-Wno-uninitialized -verify %s
 
 // rdar://8365684
 struct S {



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

[PATCH] D70568: [Support] Possibly use exception handler in the Crash Recovery Context in the same way as global exceptions

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

dexonsmith added a comment.

In D70568#1768129 , @dexonsmith wrote:

> Can we make this configurable somehow?  (E.g., leave behind an `-ffork-cc1` 
> `-fno-fork-cc1` and a CMake flag to pick?  Or just the CMake flag?)


(I'm not sure we'd want to expose this on the driver command-line.  But making 
it just a CMake flag is awkward because then you can't run tests for both 
without configuring twice.  Another option that would keep both paths tested is 
to have an environment variable such as `CLANG_FORK_CC1={0,1}`, with a CMake 
flag to pick the default behaviour.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70568



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

[PATCH] D70568: [Support] Possibly use exception handler in the Crash Recovery Context in the same way as global exceptions

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

dexonsmith added a comment.

This is really cool; we've wanted this for a long time.

One concern I have is that I think this will interfere (effectively disable) 
automatic OS handling of these crashes, which means they won't be collated and 
reported anymore.  Can we make this configurable somehow?  (E.g., leave behind 
an `-ffork-cc1` `-fno-fork-cc1` and a CMake flag to pick?  Or just the CMake 
flag?)

(I also have a couple of minor nits inline.)




Comment at: llvm/lib/Support/CrashRecoveryContext.cpp:114
 
+CrashRecoveryContext::CrashRecoveryContext() : Impl(nullptr), head(nullptr) {
+  Install();

Can you move the `nullptr` default assignments back to the header, so it's 
still obvious how they are set up?



Comment at: llvm/lib/Support/Unix/Signals.inc:348
 
+signed sys::InvokeExceptionHandler(int , void *) {
+  SignalHandler(RetCode);

I'm not sure I've seen `signed` in code before.  Might it be simpler to just 
say `int` for the return type?  Or is that somehow incorrect/confusing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70568



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

[PATCH] D70746: [clangd] Highlighting dependent types in more contexts

[Nathan Ridge via Phabricator via cfe-commits]

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

Address nit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70746

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -630,7 +630,24 @@
   R"cpp(
   template 
   struct $Class[[TupleSize]] {
-static const int $StaticField[[size]] = 
sizeof...($TemplateParameter[[Elements]]);
+static const int $StaticField[[size]] =
+sizeof...($TemplateParameter[[Elements]]);
+  };
+)cpp",
+  // More dependent types
+  R"cpp(
+  template 
+  struct $Class[[Waldo]] {
+using $Typedef[[Location1]] = typename $TemplateParameter[[T]]
+::$DependentType[[Resolver]]::$DependentType[[Location]];
+using $Typedef[[Location2]] = typename $TemplateParameter[[T]]
+::template $DependentType[[Resolver]]<$TemplateParameter[[T]]>
+::$DependentType[[Location]];
+using $Typedef[[Location3]] = typename $TemplateParameter[[T]]
+::$DependentType[[Resolver]]
+::template $DependentType[[Location]]<$TemplateParameter[[T]]>;
+static const int $StaticField[[Value]] = $TemplateParameter[[T]]
+::$DependentType[[Resolver]]::$DependentName[[Value]];
   };
 )cpp"};
   for (const auto  : TestCases) {
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -258,6 +258,25 @@
 return true;
   }
 
+  bool VisitDependentTemplateSpecializationTypeLoc(
+  DependentTemplateSpecializationTypeLoc L) {
+H.addToken(L.getTemplateNameLoc(), HighlightingKind::DependentType);
+return true;
+  }
+
+  // findExplicitReferences will walk nested-name-specifiers and
+  // find anything that can be resolved to a Decl. However, non-leaf
+  // components of nested-name-specifiers which are dependent names
+  // (kind "Identifier") cannot be resolved to a decl, so we visit
+  // them here.
+  bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc Q) {
+if (NestedNameSpecifier *NNS = Q.getNestedNameSpecifier()) {
+  if (NNS->getKind() == NestedNameSpecifier::Identifier)
+H.addToken(Q.getLocalBeginLoc(), HighlightingKind::DependentType);
+}
+return RecursiveASTVisitor::TraverseNestedNameSpecifierLoc(Q);
+  }
+
 private:
   HighlightingsBuilder 
 };


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -630,7 +630,24 @@
   R"cpp(
   template 
   struct $Class[[TupleSize]] {
-static const int $StaticField[[size]] = sizeof...($TemplateParameter[[Elements]]);
+static const int $StaticField[[size]] =
+sizeof...($TemplateParameter[[Elements]]);
+  };
+)cpp",
+  // More dependent types
+  R"cpp(
+  template 
+  struct $Class[[Waldo]] {
+using $Typedef[[Location1]] = typename $TemplateParameter[[T]]
+::$DependentType[[Resolver]]::$DependentType[[Location]];
+using $Typedef[[Location2]] = typename $TemplateParameter[[T]]
+::template $DependentType[[Resolver]]<$TemplateParameter[[T]]>
+::$DependentType[[Location]];
+using $Typedef[[Location3]] = typename $TemplateParameter[[T]]
+::$DependentType[[Resolver]]
+::template $DependentType[[Location]]<$TemplateParameter[[T]]>;
+static const int $StaticField[[Value]] = $TemplateParameter[[T]]
+::$DependentType[[Resolver]]::$DependentName[[Value]];
   };
 )cpp"};
   for (const auto  : TestCases) {
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -258,6 +258,25 @@
 return true;
   }
 
+  bool VisitDependentTemplateSpecializationTypeLoc(
+  DependentTemplateSpecializationTypeLoc L) {
+H.addToken(L.getTemplateNameLoc(), HighlightingKind::DependentType);
+return true;
+  }
+
+  // findExplicitReferences will walk nested-name-specifiers and
+  // find anything that can be resolved to a Decl. However, non-leaf
+  // components of nested-name-specifiers which are dependent names
+  // 

[PATCH] D70740: [clangd] Find reference to template parameter in 'sizeof...' expression

[Nathan Ridge via Phabricator via cfe-commits]

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

Add FindExplicitReferences test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70740

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -625,6 +625,13 @@
 $InactiveCode[[]]  #else
   int $Variable[[Active2]];
   #endif
+)cpp",
+  // Argument to 'sizeof...'
+  R"cpp(
+  template 
+  struct $Class[[TupleSize]] {
+static const int $StaticField[[size]] = sizeof...($TemplateParameter[[Elements]]);
+  };
 )cpp"};
   for (const auto  : TestCases) {
 checkHighlightings(TestCase);
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -260,6 +260,15 @@
   )cpp";
   // FIXME: deduced type missing in AST. https://llvm.org/PR42914
   EXPECT_DECLS("AutoTypeLoc");
+
+  Code = R"cpp(
+template 
+struct S {
+  static const int size = sizeof...([[E]]);
+};
+  )cpp";
+  // FIXME: We don't do a good job printing TemplateTypeParmDecls, apparently!
+  EXPECT_DECLS("SizeOfPackExpr", "");
 }
 
 TEST_F(TargetDeclTest, ClassTemplate) {
@@ -576,28 +585,27 @@
 
 TEST_F(FindExplicitReferencesTest, All) {
   std::pair Cases[] =
-  {
-  // Simple expressions.
-  {R"cpp(
+  {// Simple expressions.
+   {R"cpp(
 int global;
 int func();
 void foo(int param) {
   $0^global = $1^param + $2^func();
 }
 )cpp",
-   "0: targets = {global}\n"
-   "1: targets = {param}\n"
-   "2: targets = {func}\n"},
-  {R"cpp(
+"0: targets = {global}\n"
+"1: targets = {param}\n"
+"2: targets = {func}\n"},
+   {R"cpp(
 struct X { int a; };
 void foo(X x) {
   $0^x.$1^a = 10;
 }
 )cpp",
-   "0: targets = {x}\n"
-   "1: targets = {X::a}\n"},
-  // Namespaces and aliases.
-  {R"cpp(
+"0: targets = {x}\n"
+"1: targets = {X::a}\n"},
+   // Namespaces and aliases.
+   {R"cpp(
   namespace ns {}
   namespace alias = ns;
   void foo() {
@@ -605,19 +613,19 @@
 using namespace $1^alias;
   }
 )cpp",
-   "0: targets = {ns}\n"
-   "1: targets = {alias}\n"},
-  // Using declarations.
-  {R"cpp(
+"0: targets = {ns}\n"
+"1: targets = {alias}\n"},
+   // Using declarations.
+   {R"cpp(
   namespace ns { int global; }
   void foo() {
 using $0^ns::$1^global;
   }
 )cpp",
-   "0: targets = {ns}\n"
-   "1: targets = {ns::global}, qualifier = 'ns::'\n"},
-  // Simple types.
-  {R"cpp(
+"0: targets = {ns}\n"
+"1: targets = {ns::global}, qualifier = 'ns::'\n"},
+   // Simple types.
+   {R"cpp(
  struct Struct { int a; };
  using Typedef = int;
  void foo() {
@@ -626,13 +634,13 @@
static_cast<$4^Struct*>(0);
  }
)cpp",
-   "0: targets = {Struct}\n"
-   "1: targets = {x}, decl\n"
-   "2: targets = {Typedef}\n"
-   "3: targets = {y}, decl\n"
-   "4: targets = {Struct}\n"},
-  // Name qualifiers.
-  {R"cpp(
+"0: targets = {Struct}\n"
+"1: targets = {x}, decl\n"
+"2: targets = {Typedef}\n"
+"3: targets = {y}, decl\n"
+"4: targets = {Struct}\n"},
+   // Name qualifiers.
+   {R"cpp(
  namespace a { namespace b { struct S { typedef int type; }; } }
  void foo() {
$0^a::$1^b::$2^S $3^x;
@@ -640,17 +648,17 @@
$6^S::$7^type $8^y;
  }
 )cpp",
-   "0: targets = {a}\n"
-   "1: targets = {a::b}, qualifier = 'a::'\n"
-   "2: targets = {a::b::S}, qualifier = 'a::b::'\n"
-   "3: targets = {x}, decl\n"
-   "4: targets = {a}\n"
-   "5: targets = {a::b}, qualifier = 'a::'\n"
-   "6: targets = {a::b::S}\n"
-   "7: targets = {a::b::S::type}, qualifier = 'struct S::'\n"
-   "8: targets = {y}, decl\n"},
-  // Simple templates.
-  {R"cpp(
+"0: targets = {a}\n"
+"1: targets = {a::b}, qualifier = 'a::'\n"
+"2: targets = {a::b::S}, qualifier = 

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

@aganea Please disable the new behavior for the Darwin platform. We rely on the 
fact that Clang `-cc1` processes crash to report crashes using system's crash 
reporting infrastructure.


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

https://reviews.llvm.org/D69825



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

[clang] 3278948 - Fix `sed -e s@FOO@%/S@` and similar when there's @'s in the working directory

[Daniel Sanders via cfe-commits]

Author: Daniel Sanders
Date: 2019-12-03T15:44:01-08:00
New Revision: 327894859cc41c1730807f8a179aa880203262f5

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

LOG: Fix `sed -e s@FOO@%/S@` and similar when there's @'s in the working 
directory

Jenkins sometimes starts a new working directory by appending @2 (or
incrementing the number if the @n suffix is already there). This causes
several clang tests to fail as:
  s@INPUT_DIR@%/S/Inputs@g
gets expanded to the invalid:
  s@INPUT_DIR@/path/to/workdir@2/Inputs@g
   ~~
where the part marked with ~'s is interpreted as the flags. These are
invalid and the test fails.

Previous fixes simply exchanged the @ character for another like | but
that's just moving the problem. Address it by adding an expansion that
escapes the @ character we're using as a delimiter as well as other magic
characters in the replacement of sed's s@@@.

There's still room for expansions to cause trouble though. One I ran into
while testing this was that having a directory called foo@bar causes lots
of `CHECK-NOT: foo` directives to match. There's also things like
directories containing `\1`

Added: 


Modified: 
clang/test/Index/index-module-with-vfs.m
clang/test/Modules/crash-vfs-ivfsoverlay.m
clang/test/Modules/double-quotes.m
clang/test/Modules/framework-public-includes-private.m
clang/test/VFS/external-names.c
clang/test/VFS/framework-import.m
clang/test/VFS/implicit-include.c
clang/test/VFS/include-mixed-real-and-virtual.c
clang/test/VFS/include-real-from-virtual.c
clang/test/VFS/include-virtual-from-real.c
clang/test/VFS/include.c
clang/test/VFS/incomplete-umbrella.m
clang/test/VFS/module-import.m
clang/test/VFS/module_missing_vfs.m
clang/test/VFS/real-path-found-first.m
clang/test/VFS/relative-path.c
clang/test/VFS/test_nonmodular.c
clang/test/VFS/umbrella-framework-import-skipnonexist.m
clang/test/VFS/vfsroot-include.c
clang/test/VFS/vfsroot-module.m
clang/test/VFS/vfsroot-with-overlay.c
llvm/utils/lit/lit/TestRunner.py

Removed: 




diff  --git a/clang/test/Index/index-module-with-vfs.m 
b/clang/test/Index/index-module-with-vfs.m
index 46fa68dfa130..06944d372d49 100644
--- a/clang/test/Index/index-module-with-vfs.m
+++ b/clang/test/Index/index-module-with-vfs.m
@@ -6,7 +6,7 @@ void foo() {
 }
 
 // RUN: rm -rf %t.cache
-// RUN: sed -e "s@INPUT_DIR@%/S/Inputs@g" -e "s@OUT_DIR@%/t@g" 
%S/Inputs/vfsoverlay.yaml > %t.yaml
+// RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e 
"s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/vfsoverlay.yaml > %t.yaml
 // RUN: c-index-test -index-file %s -fmodules-cache-path=%t.cache -fmodules -F 
%t -I %t \
 // RUN:  -ivfsoverlay %t.yaml -Xclang -fdisable-module-hash | 
FileCheck %s
 

diff  --git a/clang/test/Modules/crash-vfs-ivfsoverlay.m 
b/clang/test/Modules/crash-vfs-ivfsoverlay.m
index 00992aa19fad..d2d2ccbd2546 100644
--- a/clang/test/Modules/crash-vfs-ivfsoverlay.m
+++ b/clang/test/Modules/crash-vfs-ivfsoverlay.m
@@ -3,7 +3,7 @@
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/m
 // RUN: cp %S/../VFS/Inputs/actual_module2.map %t/actual_module2.map
-// RUN: sed -e "s@INPUT_DIR@%/t@g" -e "s@OUT_DIR@%/t/example@g" \
+// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" -e 
"s@OUT_DIR@%{/t:regex_replacement}/example@g" \
 // RUN:   %S/../VFS/Inputs/vfsoverlay2.yaml > %t/srcvfs.yaml
 
 // RUN: env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \

diff  --git a/clang/test/Modules/double-quotes.m 
b/clang/test/Modules/double-quotes.m
index 4ce712ccc6c5..99187fc26654 100644
--- a/clang/test/Modules/double-quotes.m
+++ b/clang/test/Modules/double-quotes.m
@@ -4,7 +4,7 @@
 // RUN: %hmaptool write %S/Inputs/double-quotes/a.hmap.json %t/a.hmap
 // RUN: %hmaptool write %S/Inputs/double-quotes/x.hmap.json %t/x.hmap
 
-// RUN: sed -e "s@TEST_DIR@%/S/Inputs/double-quotes@g" \
+// RUN: sed -e "s@TEST_DIR@%{/S:regex_replacement}/Inputs/double-quotes@g" \
 // RUN:   %S/Inputs/double-quotes/z.yaml > %t/z.yaml
 
 // The output with and without modules should be the same

diff  --git a/clang/test/Modules/framework-public-includes-private.m 
b/clang/test/Modules/framework-public-includes-private.m
index 0f1e3a242a15..37c43e9a6390 100644
--- a/clang/test/Modules/framework-public-includes-private.m
+++ b/clang/test/Modules/framework-public-includes-private.m
@@ -4,7 +4,7 @@
 // RUN: %hmaptool write 
%S/Inputs/framework-public-includes-private/a.hmap.json %t/a.hmap
 // RUN: %hmaptool write 
%S/Inputs/framework-public-includes-private/z.hmap.json %t/z.hmap
 
-// RUN: sed -e "s@TEST_DIR@%/S/Inputs/framework-public-includes-private@g" \
+// RUN: sed -e 

[clang] 878a24e - Reapply "Fix crash on switch conditions of non-integer types in templates"

[Elizabeth Andrews via cfe-commits]

Author: Elizabeth Andrews
Date: 2019-12-03T15:27:19-08:00
New Revision: 878a24ee244a24c39d1c57e9af2e88c621f7cce9

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

LOG: Reapply "Fix crash on switch conditions of non-integer types in templates"

This patch reapplies commit 759948467ea. Patch was reverted due to a
clang-tidy test fail on Windows. The test has been modified. There
are no additional code changes.

Patch was tested with ninja check-all on Windows and Linux.

Summary of code changes:

Clang currently crashes for switch statements inside a template when the
condition is a non-integer field member because contextual implicit
conversion is skipped when parsing the condition. This conversion is
however later checked in an assert when the case statement is handled.
The conversion is skipped when parsing the condition because
the field member is set as type-dependent based on its containing class.
This patch sets the type dependency based on the field's type instead.

This patch fixes Bug 40982.

Added: 
clang/test/SemaTemplate/non-integral-switch-cond.cpp

Modified: 

clang-tools-extra/test/clang-tidy/checkers/bugprone-string-integer-assignment.cpp
clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
clang/lib/AST/Expr.cpp
clang/lib/Sema/SemaChecking.cpp
clang/test/SemaCXX/constant-expression-cxx2a.cpp
clang/test/SemaTemplate/dependent-names.cpp
clang/test/SemaTemplate/enum-argument.cpp
clang/test/SemaTemplate/member-access-expr.cpp

Removed: 




diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-integer-assignment.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-integer-assignment.cpp
index 18fe5ef4e5c2..2c288e0bbddf 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-integer-assignment.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-string-integer-assignment.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s bugprone-string-integer-assignment %t
+// RUN: %check_clang_tidy %s bugprone-string-integer-assignment %t -- -- 
-fno-delayed-template-parsing
 
 namespace std {
 template
@@ -103,6 +103,8 @@ struct S {
   static constexpr T t = 0x8000;
   std::string s;
   void f(char c) { s += c | static_cast(t); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: an integer is interpreted as a 
chara
+  // CHECK-FIXES: {{^}}  void f(char c) { s += std::to_string(c | 
static_cast(t)); } 
 };
 
 template S;

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
index 119eff67318e..8e546b44ab74 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
@@ -233,7 +233,7 @@ struct a {
 template 
 class d {
   a e;
-  void f() { e.b(); }
+  void f() { e.b(0); }
 };
 }  // namespace
 }  // namespace PR38055

diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 322b3a7fa740..a73531ad5fad 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1678,6 +1678,15 @@ MemberExpr *MemberExpr::Create(
   MemberExpr *E = new (Mem) MemberExpr(Base, IsArrow, OperatorLoc, MemberDecl,
NameInfo, T, VK, OK, NOUR);
 
+  if (FieldDecl *Field = dyn_cast(MemberDecl)) {
+DeclContext *DC = MemberDecl->getDeclContext();
+// dyn_cast_or_null is used to handle objC variables which do not
+// have a declaration context.
+CXXRecordDecl *RD = dyn_cast_or_null(DC);
+if (RD && RD->isDependentContext() && RD->isCurrentInstantiation(DC))
+  E->setTypeDependent(T->isDependentType());
+  }
+
   if (HasQualOrFound) {
 // FIXME: Wrong. We should be looking at the member declaration we found.
 if (QualifierLoc && QualifierLoc.getNestedNameSpecifier()->isDependent()) {

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ed42833531d4..825e0faa3037 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -14706,6 +14706,8 @@ void Sema::RefersToMemberWithReducedAlignment(
   bool AnyIsPacked = false;
   do {
 QualType BaseType = ME->getBase()->getType();
+if (BaseType->isDependentType())
+  return;
 if (ME->isArrow())
   BaseType = BaseType->getPointeeType();
 RecordDecl *RD = BaseType->castAs()->getDecl();

diff  --git a/clang/test/SemaCXX/constant-expression-cxx2a.cpp 
b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
index 8db705dcdc67..c2e443b9bec1 100644
--- a/clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -18,6 +18,7 @@ namespace std {
 [[nodiscard]] void 

[PATCH] D70854: [Clang] In tests, do not always assume others permissions are set

[Alexandre Ganea via Phabricator via cfe-commits]

aganea marked 3 inline comments as done.
aganea added inline comments.



Comment at: clang/test/Misc/permissions.cpp:8
 
 // RUN: umask 002
 // RUN: %clang_cc1 -emit-llvm-bc %s -o %t

rnk wrote:
> aganea wrote:
> > rnk wrote:
> > > If you change this to `umask 022`, does that result in `rw-r-`? That 
> > > would make the test meaningful on your system.
> > No, using `umask 022` has no effect, still yields 'rw':
> > ```
> > $ umask
> > 
> > $ umask 0077
> > $ touch test
> > $ ls -l
> > -rw-rw 1 aganea sudosgroup0 Dec  3 11:02 test
> > ```
> > So this seems to be related to the interaction between ACL and `umask`. The 
> > [[ http://man7.org/linux/man-pages/man2/umask.2.html | umask doc ]] says 
> > they should interact, but that doesn't seem to work on my Ubuntu 18.04.01. 
> > No matter what I set in the umask mode, creating a new file inherits the 
> > default ACL.
> > All my folders have ACL enabled:
> > ```
> > $ ls -l /mnt/
> > drwxrws--x+ 8 root sudosgroup 4096 Dec  3 17:07 f   <-- note the + 
> > which indicates ACL is being used
> > ```
> > 
> > I could give 'rw' permissions to others:
> > ```
> > $ setfacl -R -d -m o::rw /mnt/f
> > ```
> > However even with that, the test fails (because umask has no effect).
> > I'm not sure what the right fix would be here. I can investigate other 
> > things. Any suggestions?
> > I'm not sure what the right fix would be here. I can investigate other 
> > things. Any suggestions?
> 
> I don't think it's worth it. Let's go with your fix and make the test pass.
Good. I'll add a comment above in the test to explain the situation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70854



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

[PATCH] D70905: Actually delay processing DelayedDllExportClasses until the outermost class is finished (PR40006)

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

It occurs to me that this will cause some strange ordering in some cases. 
Consider:

  namespace pr40006 {
  // Delay emitting the method also past the instantiation of Tmpl, i.e.
  // until the top-level class Outer is completely finished.
  template struct Tmpl {};
  struct Outer {
  struct Inner {
  __declspec(dllexport) Inner() = default;
  unsigned int x = 0;
  };
  // not instantiated
  };
  
  struct Other {
  Tmpl y;
  };
  // M32-DAG: define weak_odr dso_local dllexport x86_thiscallcc 
%"struct.InClassInits::pr40006::Outer::Inner"* 
@"??0Inner@Outer@pr40006@InClassInits@@QAE@XZ"
  }

When we'd process `y` before, we would immediately emit delayed dllexported 
things. Now we will wait until Other is done parsing, which is fine, but not as 
eager as we could be. That's fine, but it's a slight behavior change.




Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:2232
+  if (ParsingClassDepth == 0)
+ActOnFinishCXXNonNestedClass(Instantiation);
 

This function doesn't use its argument. Can you remove it so that is clear that 
we don't need to track this somewhere else?


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

https://reviews.llvm.org/D70905



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

[PATCH] D70905: Actually delay processing DelayedDllExportClasses until the outermost class is finished (PR40006)

[Reid Kleckner via Phabricator via cfe-commits]

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

I'll add, looks good with suggested fix.


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

https://reviews.llvm.org/D70905



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

[PATCH] D70854: [Clang] In tests, do not always assume others permissions are set

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm




Comment at: clang/test/Misc/permissions.cpp:8
 
 // RUN: umask 002
 // RUN: %clang_cc1 -emit-llvm-bc %s -o %t

aganea wrote:
> rnk wrote:
> > If you change this to `umask 022`, does that result in `rw-r-`? That 
> > would make the test meaningful on your system.
> No, using `umask 022` has no effect, still yields 'rw':
> ```
> $ umask
> 
> $ umask 0077
> $ touch test
> $ ls -l
> -rw-rw 1 aganea sudosgroup0 Dec  3 11:02 test
> ```
> So this seems to be related to the interaction between ACL and `umask`. The 
> [[ http://man7.org/linux/man-pages/man2/umask.2.html | umask doc ]] says they 
> should interact, but that doesn't seem to work on my Ubuntu 18.04.01. No 
> matter what I set in the umask mode, creating a new file inherits the default 
> ACL.
> All my folders have ACL enabled:
> ```
> $ ls -l /mnt/
> drwxrws--x+ 8 root sudosgroup 4096 Dec  3 17:07 f   <-- note the + which 
> indicates ACL is being used
> ```
> 
> I could give 'rw' permissions to others:
> ```
> $ setfacl -R -d -m o::rw /mnt/f
> ```
> However even with that, the test fails (because umask has no effect).
> I'm not sure what the right fix would be here. I can investigate other 
> things. Any suggestions?
> I'm not sure what the right fix would be here. I can investigate other 
> things. Any suggestions?

I don't think it's worth it. Let's go with your fix and make the test pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70854



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

[PATCH] D70931: [MS] Emit exported complete/vbase destructors

[Reid Kleckner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG705a6aef3502: [MS] Emit exported complete/vbase destructors 
(authored by rnk).

Changed prior to commit:
  https://reviews.llvm.org/D70931?vs=231796=231998#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70931

Files:
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/test/CodeGenCXX/dllexport-dtor-thunks.cpp
  clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp


Index: clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp
===
--- clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp
+++ clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp
@@ -19,9 +19,9 @@
   virtual ~ImportOverrideVDtor() {}
 };
 
-// Virtually inherits from a non-dllimport base class. This time we need to 
call
-// the complete destructor and emit it inline. It's not exported from the DLL,
-// and it must be emitted.
+// Virtually inherits from a non-dllimport base class. In this case, we can
+// expect the DLL to provide a definition of the complete dtor. See
+// dllexport-dtor-thunks.cpp.
 struct __declspec(dllimport) ImportVBaseOverrideVDtor
 : public virtual BaseClass {
   virtual ~ImportVBaseOverrideVDtor() {}
Index: clang/test/CodeGenCXX/dllexport-dtor-thunks.cpp
===
--- clang/test/CodeGenCXX/dllexport-dtor-thunks.cpp
+++ clang/test/CodeGenCXX/dllexport-dtor-thunks.cpp
@@ -1,5 +1,12 @@
 // RUN: %clang_cc1 -mconstructor-aliases -fms-extensions %s -emit-llvm -o - 
-triple x86_64-windows-msvc | FileCheck %s
 
+namespace test1 {
+struct A { ~A(); };
+struct __declspec(dllexport) B : virtual A { };
+// CHECK: define weak_odr dso_local dllexport void @"??1B@test1@@QEAA@XZ"
+// CHECK: define weak_odr dso_local dllexport void @"??_DB@test1@@QEAAXXZ"
+}
+
 struct __declspec(dllexport) A { virtual ~A(); };
 struct __declspec(dllexport) B { virtual ~B(); };
 struct __declspec(dllexport) C : A, B { virtual ~C(); };
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1343,6 +1343,13 @@
   // The TU defining a dtor is only guaranteed to emit a base destructor.  All
   // other destructor variants are delegating thunks.
   CGM.EmitGlobal(GlobalDecl(D, Dtor_Base));
+
+  // If the class is dllexported, emit the complete (vbase) destructor wherever
+  // the base dtor is emitted.
+  // FIXME: To match MSVC, this should only be done when the class is exported
+  // with -fdllexport-inlines enabled.
+  if (D->getParent()->getNumVBases() > 0 && D->hasAttr())
+CGM.EmitGlobal(GlobalDecl(D, Dtor_Complete));
 }
 
 CharUnits


Index: clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp
===
--- clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp
+++ clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp
@@ -19,9 +19,9 @@
   virtual ~ImportOverrideVDtor() {}
 };
 
-// Virtually inherits from a non-dllimport base class. This time we need to call
-// the complete destructor and emit it inline. It's not exported from the DLL,
-// and it must be emitted.
+// Virtually inherits from a non-dllimport base class. In this case, we can
+// expect the DLL to provide a definition of the complete dtor. See
+// dllexport-dtor-thunks.cpp.
 struct __declspec(dllimport) ImportVBaseOverrideVDtor
 : public virtual BaseClass {
   virtual ~ImportVBaseOverrideVDtor() {}
Index: clang/test/CodeGenCXX/dllexport-dtor-thunks.cpp
===
--- clang/test/CodeGenCXX/dllexport-dtor-thunks.cpp
+++ clang/test/CodeGenCXX/dllexport-dtor-thunks.cpp
@@ -1,5 +1,12 @@
 // RUN: %clang_cc1 -mconstructor-aliases -fms-extensions %s -emit-llvm -o - -triple x86_64-windows-msvc | FileCheck %s
 
+namespace test1 {
+struct A { ~A(); };
+struct __declspec(dllexport) B : virtual A { };
+// CHECK: define weak_odr dso_local dllexport void @"??1B@test1@@QEAA@XZ"
+// CHECK: define weak_odr dso_local dllexport void @"??_DB@test1@@QEAAXXZ"
+}
+
 struct __declspec(dllexport) A { virtual ~A(); };
 struct __declspec(dllexport) B { virtual ~B(); };
 struct __declspec(dllexport) C : A, B { virtual ~C(); };
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1343,6 +1343,13 @@
   // The TU defining a dtor is only guaranteed to emit a base destructor.  All
   // other destructor variants are delegating thunks.
   CGM.EmitGlobal(GlobalDecl(D, Dtor_Base));
+
+  // If the class is dllexported, emit the complete (vbase) destructor wherever
+  // the base dtor is emitted.
+  // FIXME: To match MSVC, this should only be done when the 

[PATCH] D70854: [Clang] In tests, do not always assume others permissions are set

[Alexandre Ganea via Phabricator via cfe-commits]

aganea marked 3 inline comments as done.
aganea added inline comments.



Comment at: clang/test/Misc/permissions.cpp:8
 
 // RUN: umask 002
 // RUN: %clang_cc1 -emit-llvm-bc %s -o %t

rnk wrote:
> If you change this to `umask 022`, does that result in `rw-r-`? That 
> would make the test meaningful on your system.
No, using `umask 022` has no effect, still yields 'rw':
```
$ umask

$ umask 0077
$ touch test
$ ls -l
-rw-rw 1 aganea sudosgroup0 Dec  3 11:02 test
```
So this seems to be related to the interaction between ACL and `umask`. The [[ 
http://man7.org/linux/man-pages/man2/umask.2.html | umask doc ]] says they 
should interact, but that doesn't seem to work on my Ubuntu 18.04.01. No matter 
what I set in the umask mode, creating a new file inherits the default ACL.
All my folders have ACL enabled:
```
$ ls -l /mnt/
drwxrws--x+ 8 root sudosgroup 4096 Dec  3 17:07 f   <-- note the + which 
indicates ACL is being used
```

I could give 'rw' permissions to others:
```
$ setfacl -R -d -m o::rw /mnt/f
```
However even with that, the test fails (because umask has no effect).
I'm not sure what the right fix would be here. I can investigate other things. 
Any suggestions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70854



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

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

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

xazax.hun marked an inline comment as done.
xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:307
 
+  std::vector> notes;
   SymbolRef ResultSymbol = nullptr;

I will capitalize this name.


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

https://reviews.llvm.org/D70725



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

[clang] 705a6ae - [MS] Emit exported complete/vbase destructors

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2019-12-03T14:46:32-08:00
New Revision: 705a6aef350246c790ff8e73864dd27a640c59c8

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

LOG: [MS] Emit exported complete/vbase destructors

Summary:
Fixes PR44205

I checked, and deleting destructors are not affected.

Reviewers: hans

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/CodeGen/MicrosoftCXXABI.cpp
clang/test/CodeGenCXX/dllexport-dtor-thunks.cpp
clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp 
b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
index 8196df614cee..800d02d5d039 100644
--- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1343,6 +1343,13 @@ void MicrosoftCXXABI::EmitCXXDestructors(const 
CXXDestructorDecl *D) {
   // The TU defining a dtor is only guaranteed to emit a base destructor.  All
   // other destructor variants are delegating thunks.
   CGM.EmitGlobal(GlobalDecl(D, Dtor_Base));
+
+  // If the class is dllexported, emit the complete (vbase) destructor wherever
+  // the base dtor is emitted.
+  // FIXME: To match MSVC, this should only be done when the class is exported
+  // with -fdllexport-inlines enabled.
+  if (D->getParent()->getNumVBases() > 0 && D->hasAttr())
+CGM.EmitGlobal(GlobalDecl(D, Dtor_Complete));
 }
 
 CharUnits

diff  --git a/clang/test/CodeGenCXX/dllexport-dtor-thunks.cpp 
b/clang/test/CodeGenCXX/dllexport-dtor-thunks.cpp
index bda126eba855..d2aa195e8cc3 100644
--- a/clang/test/CodeGenCXX/dllexport-dtor-thunks.cpp
+++ b/clang/test/CodeGenCXX/dllexport-dtor-thunks.cpp
@@ -1,5 +1,12 @@
 // RUN: %clang_cc1 -mconstructor-aliases -fms-extensions %s -emit-llvm -o - 
-triple x86_64-windows-msvc | FileCheck %s
 
+namespace test1 {
+struct A { ~A(); };
+struct __declspec(dllexport) B : virtual A { };
+// CHECK: define weak_odr dso_local dllexport void @"??1B@test1@@QEAA@XZ"
+// CHECK: define weak_odr dso_local dllexport void @"??_DB@test1@@QEAAXXZ"
+}
+
 struct __declspec(dllexport) A { virtual ~A(); };
 struct __declspec(dllexport) B { virtual ~B(); };
 struct __declspec(dllexport) C : A, B { virtual ~C(); };

diff  --git a/clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp 
b/clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp
index da3227a49a4b..53aa2cdbf3ee 100644
--- a/clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp
+++ b/clang/test/CodeGenCXX/dllimport-dtor-thunks.cpp
@@ -19,9 +19,9 @@ struct __declspec(dllimport) ImportOverrideVDtor : public 
BaseClass {
   virtual ~ImportOverrideVDtor() {}
 };
 
-// Virtually inherits from a non-dllimport base class. This time we need to 
call
-// the complete destructor and emit it inline. It's not exported from the DLL,
-// and it must be emitted.
+// Virtually inherits from a non-dllimport base class. In this case, we can
+// expect the DLL to provide a definition of the complete dtor. See
+// dllexport-dtor-thunks.cpp.
 struct __declspec(dllimport) ImportVBaseOverrideVDtor
 : public virtual BaseClass {
   virtual ~ImportVBaseOverrideVDtor() {}



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

[PATCH] D70931: [MS] Emit exported complete/vbase destructors

[Reid Kleckner via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/MicrosoftCXXABI.cpp:1349
+  // the base dtor is emitted.
+  // FIXME: To match MSVC, this should only be done when the class was
+  // dllexported inlines are being exported.

hans wrote:
> The grammar looks funny here: "when the class was dllexported inlines are 
> being exported".
Thanks, too much editing leads to mistakes. =/ Cleaned up to:
  // FIXME: To match MSVC, this should only be done when the class is exported
  // with -fdllexport-inlines enabled.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70931



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

[PATCH] D69855: [clang-tidy] Fix llvm-namespace-comment for macro expansions

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

@twardakm: I'm not convinced this feature is worth implementing at all, because 
there's a good alternative to a macro here -- a namespace alias. What is the 
reason to use a macro instead of a namespace alias?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69855



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

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

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

xazax.hun updated this revision to Diff 231996.
xazax.hun added a comment.

- Only add note tag if we have actual notes.


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

https://reviews.llvm.org/D70725

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
  clang/test/Analysis/fuchsia_handle.cpp

Index: clang/test/Analysis/fuchsia_handle.cpp
===
--- clang/test/Analysis/fuchsia_handle.cpp
+++ clang/test/Analysis/fuchsia_handle.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,fuchsia.HandleChecker -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,fuchsia.HandleChecker -analyzer-output=text \
+// RUN: -verify %s
 
 typedef __typeof__(sizeof(int)) size_t;
 typedef int zx_status_t;
@@ -101,31 +102,73 @@
 void checkLeak01(int tag) {
   zx_handle_t sa, sb;
   zx_channel_create(0, , );
+  // expected-note@-1 {{Handle allocated here}}
   use1();
-  if (tag)
+  if (tag) // expected-note {{Assuming 'tag' is 0}}
 zx_handle_close(sa);
+  // expected-note@-2 {{Taking false branch}}
   use2(sb); // expected-warning {{Potential leak of handle}}
+  // expected-note@-1 {{Potential leak of handle}}
   zx_handle_close(sb);
 }
 
+void checkReportLeakOnOnePath(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  // expected-note@-1 {{Handle allocated here}}
+  zx_handle_close(sb);
+  switch(tag) { // expected-note {{Control jumps to the 'default' case at line}} 
+case 0:
+  use2(sa);
+  return;
+case 1:
+  use2(sa);
+  return;
+case 2:
+  use2(sa);
+  return;
+case 3:
+  use2(sa);
+  return;
+case 4:
+  use2(sa);
+  return;
+default:
+  use2(sa);
+  return; // expected-warning {{Potential leak of handle}}
+  // expected-note@-1 {{Potential leak of handle}}
+  }
+}
+
 void checkDoubleRelease01(int tag) {
   zx_handle_t sa, sb;
   zx_channel_create(0, , );
-  if (tag)
-zx_handle_close(sa);
+  // expected-note@-1 {{Handle allocated here}}
+  if (tag) // expected-note {{Assuming 'tag' is not equal to 0}}
+zx_handle_close(sa); // expected-note {{Handle released here}}
+  // expected-note@-2 {{Taking true branch}}
   zx_handle_close(sa); // expected-warning {{Releasing a previously released handle}}
+  // expected-note@-1 {{Releasing a previously released handle}}
   zx_handle_close(sb);
 }
 
 void checkUseAfterFree01(int tag) {
   zx_handle_t sa, sb;
   zx_channel_create(0, , );
+  // expected-note@-1 {{Handle allocated here}}
+  // expected-note@-2 {{Handle allocated here}}
+  // expected-note@+2 {{Taking true branch}}
+  // expected-note@+1 {{Taking false branch}}
   if (tag) {
-zx_handle_close(sa);
+// expected-note@-1 {{Assuming 'tag' is not equal to 0}}
+zx_handle_close(sa); // expected-note {{Handle released here}}
 use1(); // expected-warning {{Using a previously released handle}}
+// expected-note@-1 {{Using a previously released handle}}
   }
-  zx_handle_close(sb);
+  // expected-note@-6 {{Assuming 'tag' is 0}}
+  zx_handle_close(sb); // expected-note {{Handle released here}}
   use2(sb); // expected-warning {{Using a previously released handle}}
+  // expected-note@-1 {{Using a previously released handle}}
 }
 
 void checkMemberOperatorIndices() {
Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -170,8 +170,8 @@
   /*SuppressOnSink=*/true};
   BugType DoubleReleaseBugType{this, "Fuchsia handle double release",
"Fuchsia Handle Error"};
-  BugType UseAfterFreeBugType{this, "Fuchsia handle use after release",
-  "Fuchsia Handle Error"};
+  BugType UseAfterRelease{this, "Fuchsia handle use after release",
+  "Fuchsia Handle Error"};
 
 public:
   void checkPreCall(const CallEvent , CheckerContext ) const;
@@ -204,6 +204,28 @@
 
 REGISTER_MAP_WITH_PROGRAMSTATE(HStateMap, SymbolRef, HandleState)
 
+static const ExplodedNode *getAcquireSite(const ExplodedNode *N, SymbolRef Sym,
+  CheckerContext ) {
+  ProgramStateRef State = N->getState();
+  // When bug type is handle leak, exploded node N does not have state info for
+  // leaking handle. Get the predecessor of N instead.
+  if (!State->get(Sym))
+N = N->getFirstPred();
+
+  const ExplodedNode *Pred = N;
+  while (N) {
+State = N->getState();
+if (!State->get(Sym)) {
+  const HandleState *HState = Pred->getState()->get(Sym);
+  if (HState && (HState->isAllocated() || HState->maybeAllocated()))
+return N;
+}
+Pred = N;
+N = N->getFirstPred();
+  }
+  return 

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

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

xazax.hun added a comment.

In D70725#1767942 , @NoQ wrote:

> Mmm, right, i guess you should also skip adding the tag if the notes array is 
> empty.
>
> There might be more complicated use-cases (especially in `ExprEngine` itself) 
> where you may end up adding a tag even if the state doesn't change. For 
> instance, when changing an Environment value from absent to `UnknownVal`, the 
> state doesn't get actually updated. Or making range assumptions over 
> `UnknownVal`. But i'd argue that we do indeed want the note tag in such cases 
> because it is still worth annotating the `ExplodedGraph` with an explanation 
> of what's happening. Eg., a note "Assuming a condition is false" makes sense 
> even if the condition is an `UnknownVal` and no actual constraints are added.
>
> Another example: in a recent `StreamChecker` review we've been talking about 
> a peculiar stream function that by design closes a file descriptor if it's 
> open but does nothing if the descriptor is already closed. I believe this 
> event deserves a note "The descriptor remains closed" (possibly prunable) 
> even though there is no change in the state. This is something we couldn't do 
> with our usual visitor-based approach which involves observing changes in the 
> state (we technically could, via pattern-matching over the program point, but 
> that'd directly duplicate a lot more of checker logic than usual).


I see, that makes sense. I updated the code. I was hoping for a low hanging 
fruit to make this more user friendly :)


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

https://reviews.llvm.org/D70725



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

[PATCH] D70157: Align branches within 32-Byte boundary

[Fedor Sergeev via Phabricator via cfe-commits]

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

In D70157#1767212 , @fedor.sergeev 
wrote:

> Working on getting upstream reproducer


Hangs on a moderately small piece  (~150 lines) of x86 assembly, filed a bug on 
it here:

https://bugs.llvm.org/show_bug.cgi?id=44215


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

https://reviews.llvm.org/D70157



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

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Mmm, right, i guess you should also skip adding the tag if the notes array is 
empty.

There might be more complicated use-cases (especially in `ExprEngine` itself) 
where you may end up adding a tag even if the state doesn't change. For 
instance, when changing an Environment value from absent to `UnknownVal`, the 
state doesn't get actually updated. Or making range assumptions over 
`UnknownVal`. But i'd argue that we do indeed want the note tag in such cases 
because it is still worth annotating the `ExplodedGraph` with an explanation of 
what's happening. Eg., a note "Assuming a condition is false" makes sense even 
if the condition is an `UnknownVal` and no actual constraints are added.

Another example: in a recent `StreamChecker` review we've been talking about a 
peculiar stream function that by design closes a file descriptor if it's open 
but does nothing if the descriptor is already closed. I believe this event 
deserves a note "The descriptor remains closed" (possibly prunable) even though 
there is no change in the state. This is something we couldn't do with our 
usual visitor-based approach which involves observing changes in the state (we 
technically could, via pattern-matching over the program point, but that'd 
directly duplicate a lot more of checker logic than usual).


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

https://reviews.llvm.org/D70725



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

[PATCH] D70987: [HIP] Improve opt-level handling

[Scott Linder via Phabricator via cfe-commits]

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

The HIP toolchain invokes `llc` without an explicit opt-level, meaning
it always uses the default (-O2). This makes it impossible to use -O1,
for example. The HIP toolchain also coerces -Os/-Oz to -O2 even when
invoking opt, and it coerces -Og to -O2 rather than -O1.

Forward the opt-level to `llc` as well as `opt`, and only coerce levels
where it is required.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70987

Files:
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/test/Driver/hip-toolchain-opt.hip

Index: clang/test/Driver/hip-toolchain-opt.hip
===
--- /dev/null
+++ clang/test/Driver/hip-toolchain-opt.hip
@@ -0,0 +1,101 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### \
+// RUN:   -target x86_64-unknown-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx900 \
+// RUN:   -c -nogpulib \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck --check-prefixes=ALL,DEFAULT %s
+
+// RUN: %clang -### -O0 \
+// RUN:   -target x86_64-unknown-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx900 \
+// RUN:   -c -nogpulib \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck --check-prefixes=ALL,O0 %s
+
+// RUN: %clang -### -O1 \
+// RUN:   -target x86_64-unknown-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx900 \
+// RUN:   -c -nogpulib \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck --check-prefixes=ALL,O1 %s
+
+// RUN: %clang -### -O2 \
+// RUN:   -target x86_64-unknown-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx900 \
+// RUN:   -c -nogpulib \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck --check-prefixes=ALL,O2 %s
+
+// RUN: %clang -### -O3 \
+// RUN:   -target x86_64-unknown-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx900 \
+// RUN:   -c -nogpulib \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck --check-prefixes=ALL,O3 %s
+
+// RUN: %clang -### -Os \
+// RUN:   -target x86_64-unknown-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx900 \
+// RUN:   -c -nogpulib \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck --check-prefixes=ALL,Os %s
+
+// RUN: %clang -### -Oz \
+// RUN:   -target x86_64-unknown-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx900 \
+// RUN:   -c -nogpulib \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck --check-prefixes=ALL,Oz %s
+
+// RUN: %clang -### -Og \
+// RUN:   -target x86_64-unknown-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx900 \
+// RUN:   -c -nogpulib \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck --check-prefixes=ALL,Og %s
+
+// ALL: "{{.*}}clang{{.*}}" "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// DEFAULT-NOT: "-O{{.}}"
+// O0-SAME: "-O0"
+// O1-SAME: "-O1"
+// O2-SAME: "-O2"
+// O3-SAME: "-O3"
+// Os-SAME: "-Os"
+// Oz-SAME: "-Oz"
+// Og-SAME: "-Og"
+
+// ALL: "{{.*}}opt"
+// DEFAULT-NOT: "-O{{.}}"
+// O0-SAME: "-O0"
+// O1-SAME: "-O1"
+// O2-SAME: "-O2"
+// O3-SAME: "-O3"
+// Os-SAME: "-Os"
+// Oz-SAME: "-Oz"
+// Og-SAME: "-O1"
+// ALL-SAME: "-mtriple=amdgcn-amd-amdhsa"
+
+// ALL: "{{.*}}llc"
+// DEFAULT-NOT: "-O{{.}}"
+// O0-SAME: "-O0"
+// O1-SAME: "-O1"
+// O2-SAME: "-O2"
+// O3-SAME: "-O3"
+// Os-SAME: "-O2"
+// Oz-SAME: "-O2"
+// Og-SAME: "-O1"
+// ALL-SAME: "-mtriple=amdgcn-amd-amdhsa"
+
+// ALL: "{{.*}}clang{{.*}}" "-cc1" "-triple" "x86_64-unknown-linux-gnu"
+// DEFAULT-NOT: "-O{{.}}"
+// O0-SAME: "-O0"
+// O1-SAME: "-O1"
+// O2-SAME: "-O2"
+// O3-SAME: "-O3"
+// Os-SAME: "-Os"
+// Oz-SAME: "-Oz"
+// Og-SAME: "-Og"
Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -62,6 +62,34 @@
   }
   return OutputFileName;
 }
+
+static void addOptLevelArgs(const llvm::opt::ArgList ,
+llvm::opt::ArgStringList ,
+bool IsLlc = false) {
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+StringRef OOpt = "3";
+if (A->getOption().matches(options::OPT_O4) ||
+A->getOption().matches(options::OPT_Ofast))
+  OOpt = "3";
+else if (A->getOption().matches(options::OPT_O0))
+  OOpt = "0";
+else if (A->getOption().matches(options::OPT_O)) {
+  // Clang and opt support -Os/-Oz; llc only supports -O0, -O1, -O2 and -O3
+  // so we map -Os/-Oz to -O2.
+  // Only clang supports -Og, and maps it to -O1.
+  // We map anything else to -O2.
+  OOpt = llvm::StringSwitch(A->getValue())
+ .Case("1", "1")
+ .Case("2", "2")
+ .Case("3", "3")
+ .Case("s", IsLlc ? "2" : "s")
+ .Case("z", 

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D68101#1767732 , @nickdesaulniers 
wrote:

> > But ELF allows object files to contain an arbitrary number of what I've 
> > been calling "section units" that will be assembled into a single section 
> > in the image.
>
> More precisely, in assembler, you can specify sections dis-jointly, but they 
> will be rejoined when assembled into an ELF object, as ELF section names are 
> unique and cannot be discontinuous.


The ELF spec is explicit that files can contain multiple sections with the same 
name.  This is necessary when working with groups but isn't restricted to that; 
for example, LLVM will also emit multiple sections for a single name+group pair 
when it has an associated section.  The linker may join these in the image 
after it's done all the necessary processing, but I don't think it's actually 
required to.

As for what ELF assemblers actually support, that's of course a separate story. 
 They presumably produce unique sections by at least name+group pairs, or else 
COMDATs will be totally broken.  I don't know how the handling for associated 
sections works when LLVM emits to assembly vs. just producing the object file 
directly.

It's not abstractly unreasonable to also emit different sections based on 
mergeability and entry size.  However, if doing so breaks current tools, that's 
not a reasonable option.  The next best option would be to emit a single 
section per name+group, but only flag it mergeable if all the objects are 
identically mergeable.  Unfortunately, I think MC isn't architected for that; 
the assembly writer wants to process globals one-by-one and can't retroactively 
change flags.  So the final option is to stop trying to use ELF mergability for 
unnamed_addr globals entirely, which I think we all agree is undesirable.

I don't think any sort of frontend-based solution is reasonable.  IR should be 
able to freely mark globals with sections and unnamed_addr, and it's the 
backend's responsibility to emit the best code it can for what's been written.  
If the current state of the rest of the toolchain means we can't pursue some 
optimization, so be it.


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

https://reviews.llvm.org/D68101



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

[PATCH] D67463: [MS] Warn when shadowing template parameters under -fms-compatibility

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

@hans, are we still accepting 9.0.1 patches? I thought we'd already made a 
release candidate.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67463



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

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

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

xazax.hun added a comment.

In D70725#1767673 , @NoQ wrote:

> In D70725#1767643 , @Charusso wrote:
>
> > In D70725#1767579 , @xazax.hun 
> > wrote:
> >
> > > Just a small side note. If the state was same as the previous we do not 
> > > end up creating a new ExplodedNode right? Is this also the case when we 
> > > add a NoteTag?
> >
> >
> > It only happens for evaluation when you cannot evaluate something. Other 
> > than that `Pre/PostCall` working fine to add a node with the `NoteTag`.
>
>
> Tag pointers are part of the `ProgramPoint`'s identity, which in turn are 
> part of the `ExplodedNode`'s identity. If you make a new note tag and 
> transition into it for the first time, the new node is guaranteed to be fresh.


This is what I suspected. I wonder if all the checks end up using NoteTags will 
the increased number of nodes will ever be a concern? Maybe if is the same as 
the previous we should just omit the note tag?

For example, my checker before the note tag would only generate new nodes if 
there was an actual change to the state. After this change it probably ends up 
adding new nodes with empty note tags all the time.  This could be bad both for 
performance and debugging. What do you think?


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

https://reviews.llvm.org/D70725



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

[PATCH] D70958: [compiler-rt] [test] Disable ASLR on ASAN/MSAN/TSAN tests on NetBSD

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

Could this go into the common lit config in compiler-rt/test?
What is the problem with ASLR in NetBSD? Is this about fixed shadow location 
conflicts with the binary & library mappings?


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

https://reviews.llvm.org/D70958



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

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

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

xazax.hun updated this revision to Diff 231989.
xazax.hun marked 5 inline comments as done.
xazax.hun added a comment.

- Address review comments.


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

https://reviews.llvm.org/D70725

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
  clang/test/Analysis/fuchsia_handle.cpp

Index: clang/test/Analysis/fuchsia_handle.cpp
===
--- clang/test/Analysis/fuchsia_handle.cpp
+++ clang/test/Analysis/fuchsia_handle.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,fuchsia.HandleChecker -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,fuchsia.HandleChecker -analyzer-output=text \
+// RUN: -verify %s
 
 typedef __typeof__(sizeof(int)) size_t;
 typedef int zx_status_t;
@@ -101,31 +102,73 @@
 void checkLeak01(int tag) {
   zx_handle_t sa, sb;
   zx_channel_create(0, , );
+  // expected-note@-1 {{Handle allocated here}}
   use1();
-  if (tag)
+  if (tag) // expected-note {{Assuming 'tag' is 0}}
 zx_handle_close(sa);
+  // expected-note@-2 {{Taking false branch}}
   use2(sb); // expected-warning {{Potential leak of handle}}
+  // expected-note@-1 {{Potential leak of handle}}
   zx_handle_close(sb);
 }
 
+void checkReportLeakOnOnePath(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  // expected-note@-1 {{Handle allocated here}}
+  zx_handle_close(sb);
+  switch(tag) { // expected-note {{Control jumps to the 'default' case at line}} 
+case 0:
+  use2(sa);
+  return;
+case 1:
+  use2(sa);
+  return;
+case 2:
+  use2(sa);
+  return;
+case 3:
+  use2(sa);
+  return;
+case 4:
+  use2(sa);
+  return;
+default:
+  use2(sa);
+  return; // expected-warning {{Potential leak of handle}}
+  // expected-note@-1 {{Potential leak of handle}}
+  }
+}
+
 void checkDoubleRelease01(int tag) {
   zx_handle_t sa, sb;
   zx_channel_create(0, , );
-  if (tag)
-zx_handle_close(sa);
+  // expected-note@-1 {{Handle allocated here}}
+  if (tag) // expected-note {{Assuming 'tag' is not equal to 0}}
+zx_handle_close(sa); // expected-note {{Handle released here}}
+  // expected-note@-2 {{Taking true branch}}
   zx_handle_close(sa); // expected-warning {{Releasing a previously released handle}}
+  // expected-note@-1 {{Releasing a previously released handle}}
   zx_handle_close(sb);
 }
 
 void checkUseAfterFree01(int tag) {
   zx_handle_t sa, sb;
   zx_channel_create(0, , );
+  // expected-note@-1 {{Handle allocated here}}
+  // expected-note@-2 {{Handle allocated here}}
+  // expected-note@+2 {{Taking true branch}}
+  // expected-note@+1 {{Taking false branch}}
   if (tag) {
-zx_handle_close(sa);
+// expected-note@-1 {{Assuming 'tag' is not equal to 0}}
+zx_handle_close(sa); // expected-note {{Handle released here}}
 use1(); // expected-warning {{Using a previously released handle}}
+// expected-note@-1 {{Using a previously released handle}}
   }
-  zx_handle_close(sb);
+  // expected-note@-6 {{Assuming 'tag' is 0}}
+  zx_handle_close(sb); // expected-note {{Handle released here}}
   use2(sb); // expected-warning {{Using a previously released handle}}
+  // expected-note@-1 {{Using a previously released handle}}
 }
 
 void checkMemberOperatorIndices() {
Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -170,7 +170,7 @@
   /*SuppressOnSink=*/true};
   BugType DoubleReleaseBugType{this, "Fuchsia handle double release",
"Fuchsia Handle Error"};
-  BugType UseAfterFreeBugType{this, "Fuchsia handle use after release",
+  BugType UseAfterRelease{this, "Fuchsia handle use after release",
   "Fuchsia Handle Error"};
 
 public:
@@ -204,6 +204,28 @@
 
 REGISTER_MAP_WITH_PROGRAMSTATE(HStateMap, SymbolRef, HandleState)
 
+static const ExplodedNode *getAcquireSite(const ExplodedNode *N, SymbolRef Sym,
+  CheckerContext ) {
+  ProgramStateRef State = N->getState();
+  // When bug type is handle leak, exploded node N does not have state info for
+  // leaking handle. Get the predecessor of N instead.
+  if (!State->get(Sym))
+N = N->getFirstPred();
+
+  const ExplodedNode *Pred = N;
+  while (N) {
+State = N->getState();
+if (!State->get(Sym)) {
+  const HandleState *HState = Pred->getState()->get(Sym);
+  if (HState && (HState->isAllocated() || HState->maybeAllocated()))
+return N;
+}
+Pred = N;
+N = N->getFirstPred();
+  }
+  return nullptr;
+}
+
 /// Returns the symbols extracted from the argument and its corresponding
 

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

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

xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:363-366
+if (() !=  &&
+() !=  &&
+() != )
+  return "";

NoQ wrote:
> Ugh, it sucks that we have to do this by hand.
> 
> Why would the symbol be interesting if the bugtype is wrong? You imagine 
> something like division by a zero handle? Do we really mind the updates to 
> the handle highlighted in this case? I guess we do.
> 
> Maybe we should make "note tag tags" to avoid users writing this by hand. 
> Like, note tags are tagged with note tag tags and the report is also tagged 
> with a note tag tag and the tag is invoked only if the tag tag on the report 
> matches the tag tag on the tag. Setting a tag tag on the report will be 
> equivalent to calling an "addVisitor" on the report in the visitor API, which 
> was actually a good part of the visitor API.
> 
> Just thinking out loud, please ignore. This code is a nice example to 
> generalize from.
I totally agree :) But if we do something like this please do not call the tag 
tag a tag. It will confuse people. :D



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:369
+  std::string text = note(BR);
+  if (!text.empty())
+return text;

NoQ wrote:
> Another option here is to concatenate the notes if there are indeed two 
> interesting handles at once: `Handle is allocated through 2nd parameter; 
> Handle is released through 3rd parameter". In this checker it probably never 
> happens, as every report is about exactly one handle (is it true for leak 
> reports though?).
I think this is true for leaks as well, as we will generate a new report for 
each of the symbols.


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

https://reviews.llvm.org/D70725



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

[PATCH] D70791: Workaround for MSVC 16.3.* pre-c++17 type trait linkage

[Erich Keane via Phabricator via cfe-commits]

erichkeane abandoned this revision.
erichkeane added a comment.

Microsoft recently released 16.4 and has ceased support for the 16.3 line, so 
it isn't really important to me anymore.  I'd thought this was a 'hold our nose 
and do it' type fix, but I don't think that is justified any longer now that 
Microsoft doesn't even support it.


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

https://reviews.llvm.org/D70791



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

[libunwind] b3fdf33 - Enable `-funwind-tables` flag when building libunwind

[Sergej Jaskiewicz via cfe-commits]

Author: Sergej Jaskiewicz
Date: 2019-12-04T00:52:19+03:00
New Revision: b3fdf33ba6aa7ef80621696f74aaf2f6f8e1d1de

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

LOG: Enable `-funwind-tables` flag when building libunwind

Summary:
Or, rather, don't accidentally forget to pass it.

This is aimed to solve the problem discussed in [this 
thread](http://lists.llvm.org/pipermail/llvm-dev/2019-November/136890.html), 
and to fix [a year-old bug](https://bugs.llvm.org/show_bug.cgi?id=38468).

TL;DR: when building libunwind for ARM Linux, we **need** libunwind to be built 
with the `-funwind-tables` flag, because, well ARM EHABI needs unwind info 
produced by this flag. Without the flag all the procedures in libunwind are 
marked `.cantunwind`, which causes all sorts of bad things. From 
`_Unwind_Backtrace` not working, to C++ exceptions not being caught (which is 
the aforementioned bug is about).

Previously, this flag was not added because the CMake check 
`add_compile_flags_if_supported(-funwind-tables)` produced a false negative. 
Why? With this flag, the compiler generates calls to the 
`__aeabi_unwind_cpp_pr0` symbol, which is defined in libunwind itself and 
obviously is not available at configure time, before libunwind is built. This 
led to failure at link time during the CMake check. We handle this by disabling 
the linker for CMake checks in linbunwind.

Also, this patch introduces a lit feature `libunwind-arm-ehabi`, which is used 
to mark the `signal_frame.pass.cpp` test as unsupported (as was advised by 
@miyuki in D70397).

Reviewers: peter.smith, phosek, EricWF, compnerd, jroelofs, saugustine, miyuki, 
jfb

Subscribers: mgorny, kristof.beyls, christof, libcxx-commits, miyuki

Tags: #libc

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

Added: 


Modified: 
libunwind/CMakeLists.txt
libunwind/cmake/config-ix.cmake
libunwind/test/CMakeLists.txt
libunwind/test/libunwind/test/config.py
libunwind/test/lit.site.cfg.in
libunwind/test/signal_frame.pass.cpp

Removed: 




diff  --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt
index 08095d1333a5..7aa1c782ac74 100644
--- a/libunwind/CMakeLists.txt
+++ b/libunwind/CMakeLists.txt
@@ -220,6 +220,21 @@ include(HandleLibunwindFlags)
 # Setup Compiler Flags
 
#===
 
+# Don't run the linker in CMake checks.
+#
+# The reason why this was added is that when building libunwind for
+# ARM Linux, we need to pass the -funwind-tables flag in order for it to
+# work properly with ARM EHABI.
+#
+# However, when performing CMake checks, adding this flag causes the check
+# to produce a false negative, because the compiler generates calls
+# to __aeabi_unwind_cpp_pr0, which is defined in libunwind itself,
+# which isn't built yet, so the linker complains about undefined symbols.
+#
+# This leads to libunwind not being built with this flag, which makes
+# libunwind quite useless in this setup.
+set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY)
+
 # Get required flags.
 add_target_flags_if(LIBUNWIND_BUILD_32_BITS "-m32")
 
@@ -292,6 +307,12 @@ add_cxx_compile_flags_if_supported(-fstrict-aliasing)
 add_cxx_compile_flags_if_supported(-EHsc)
 
 add_compile_flags_if_supported(-funwind-tables)
+
+if (LIBUNWIND_USES_ARM_EHABI AND NOT LIBUNWIND_SUPPORTS_FUNWIND_TABLES_FLAG)
+  message(SEND_ERROR "The -funwind-tables flag must be supported "
+ "because this target uses ARM Exception Handling ABI")
+endif()
+
 add_cxx_compile_flags_if_supported(-fno-exceptions)
 add_cxx_compile_flags_if_supported(-fno-rtti)
 

diff  --git a/libunwind/cmake/config-ix.cmake b/libunwind/cmake/config-ix.cmake
index 02d2f13f2e28..0d833e996ca1 100644
--- a/libunwind/cmake/config-ix.cmake
+++ b/libunwind/cmake/config-ix.cmake
@@ -2,6 +2,7 @@ include(CMakePushCheckState)
 include(CheckCCompilerFlag)
 include(CheckCXXCompilerFlag)
 include(CheckLibraryExists)
+include(CheckSymbolExists)
 include(CheckCSourceCompiles)
 
 check_library_exists(c fopen "" LIBUNWIND_HAS_C_LIB)
@@ -73,3 +74,13 @@ check_cxx_compiler_flag(-nostdinc++ 
LIBUNWIND_HAS_NOSTDINCXX_FLAG)
 # Check libraries
 check_library_exists(dl dladdr "" LIBUNWIND_HAS_DL_LIB)
 check_library_exists(pthread pthread_once "" LIBUNWIND_HAS_PTHREAD_LIB)
+
+# Check symbols
+check_symbol_exists(__arm__ "" LIBUNWIND_TARGET_ARM)
+check_symbol_exists(__USING_SJLJ_EXCEPTIONS__ "" 
LIBUNWIND_USES_SJLJ_EXCEPTIONS)
+check_symbol_exists(__ARM_DWARF_EH__ "" LIBUNWIND_USES_DWARF_EH)
+
+if(LIBUNWIND_TARGET_ARM AND NOT LIBUNWIND_USES_SJLJ_EXCEPTIONS AND NOT 
LIBUNWIND_USES_DWARF_EH)
+  # This condition is copied from __libunwind_config.h
+  set(LIBUNWIND_USES_ARM_EHABI ON)
+endif()

diff  --git 

[PATCH] D70980: [HIP] Remove opencl.amdgcn.lib

[Aaron Enye Shi via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D70980



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

[PATCH] D70726: [OpenMP50] Add parallel master construct

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/test/OpenMP/parallel_master_codegen.cpp:1
+// expected-no-diagnostics
+#ifndef HEADER

A codegen test for `if`, `proc_bind` and `allocate` clauses?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70726



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

[PATCH] D70489: [clangd] Add xref for macro to static index.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

thanks, mostly good, my main concern is that the patch still relies on the 
`CollectMacro` and `CollectMainFileSymbols ` option, see my comments below.




Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:349
SourceLocation Loc) {
   if (!Opts.CollectMacro)
 return true;

again, `CollectMacro` just tells whether we should collect macro symbols, we 
use `RefFilter` to control whether we collect macro references. I think we'd 
need to move this code after the code of collecting macro refs.



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:363
   bool IsMainFileSymbol = SM.isInMainFile(SM.getExpansionLoc(DefLoc));
   if (IsMainFileSymbol && !Opts.CollectMainFileSymbols)
 return false;

and this as well.



Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:472
   }
-  if (Opts.CollectMacro) {
-assert(PP);

this code should not be modified, since we don't use the `CollectMacro` option.



Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:615
+  CollectorOpts.CollectMacro = true;
+  CollectorOpts.CollectMainFileSymbols = true;
   runSymbolCollector(Header.code(),

I think we don't need these flags if we change the symbolcollector as my other 
comments mention.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70489



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

[PATCH] D70980: [HIP] Remove opencl.amdgcn.lib

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 231978.
yaxunl added a comment.

fix test


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

https://reviews.llvm.org/D70980

Files:
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/test/Driver/hip-device-libs.hip


Index: clang/test/Driver/hip-device-libs.hip
===
--- clang/test/Driver/hip-device-libs.hip
+++ clang/test/Driver/hip-device-libs.hip
@@ -22,7 +22,6 @@
 
 // COM: {{"[^"]*clang[^"]*"}}
 // COM-SAME: "-mlink-builtin-bitcode" "{{.*}}hip.amdgcn.bc"
-// COM-SAME: "-mlink-builtin-bitcode" "{{.*}}opencl.amdgcn.bc"
 // COM-SAME: "-mlink-builtin-bitcode" "{{.*}}ocml.amdgcn.bc"
 // COM-SAME: "-mlink-builtin-bitcode" "{{.*}}ockl.amdgcn.bc"
 // FLUSHD-SAME: "-mlink-builtin-bitcode" "{{.*}}oclc_daz_opt_on.amdgcn.bc"
Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -343,9 +343,8 @@
 else
   WaveFrontSizeBC = "oclc_wavefrontsize64_off.amdgcn.bc";
 
-BCLibs.append({"hip.amdgcn.bc", "opencl.amdgcn.bc", "ocml.amdgcn.bc",
-   "ockl.amdgcn.bc", "oclc_finite_only_off.amdgcn.bc",
-   FlushDenormalControlBC,
+BCLibs.append({"hip.amdgcn.bc", "ocml.amdgcn.bc", "ockl.amdgcn.bc",
+   "oclc_finite_only_off.amdgcn.bc", FlushDenormalControlBC,
"oclc_correctly_rounded_sqrt_on.amdgcn.bc",
"oclc_unsafe_math_off.amdgcn.bc", ISAVerBC,
WaveFrontSizeBC});


Index: clang/test/Driver/hip-device-libs.hip
===
--- clang/test/Driver/hip-device-libs.hip
+++ clang/test/Driver/hip-device-libs.hip
@@ -22,7 +22,6 @@
 
 // COM: {{"[^"]*clang[^"]*"}}
 // COM-SAME: "-mlink-builtin-bitcode" "{{.*}}hip.amdgcn.bc"
-// COM-SAME: "-mlink-builtin-bitcode" "{{.*}}opencl.amdgcn.bc"
 // COM-SAME: "-mlink-builtin-bitcode" "{{.*}}ocml.amdgcn.bc"
 // COM-SAME: "-mlink-builtin-bitcode" "{{.*}}ockl.amdgcn.bc"
 // FLUSHD-SAME: "-mlink-builtin-bitcode" "{{.*}}oclc_daz_opt_on.amdgcn.bc"
Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -343,9 +343,8 @@
 else
   WaveFrontSizeBC = "oclc_wavefrontsize64_off.amdgcn.bc";
 
-BCLibs.append({"hip.amdgcn.bc", "opencl.amdgcn.bc", "ocml.amdgcn.bc",
-   "ockl.amdgcn.bc", "oclc_finite_only_off.amdgcn.bc",
-   FlushDenormalControlBC,
+BCLibs.append({"hip.amdgcn.bc", "ocml.amdgcn.bc", "ockl.amdgcn.bc",
+   "oclc_finite_only_off.amdgcn.bc", FlushDenormalControlBC,
"oclc_correctly_rounded_sqrt_on.amdgcn.bc",
"oclc_unsafe_math_off.amdgcn.bc", ISAVerBC,
WaveFrontSizeBC});
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70524: Support DebugInfo generation for auto return type for C++ functions.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

@shafik Can you speak about whether this feature ("auto" return type in DWARF 
v5 section 5.2) would be useful for LLDB?


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

https://reviews.llvm.org/D70524



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

[PATCH] D70974: [clang-tidy] Fix PR26274

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for working on this!




Comment at: 
clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:55
 
-  if (!locationsInSameFile(Sources, ND->getBeginLoc(), ND->getRBraceLoc()))
+  // Ignore namespaces inside macros and namespaces split accross files.
+  if (ND->getBeginLoc().isMacroID() ||

accross -> across



Comment at: 
clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:90
+} else {
+  break;
 }

Will this work okay with inline namespaces, or namespaces with attributes 
written on them? May want test cases for those situations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70974



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

[PATCH] D70981: [opencl] Fix address space deduction on array variables.

[Michael Liao via Phabricator via cfe-commits]

hliao created this revision.
hliao added a reviewer: Anastasia.
Herald added subscribers: cfe-commits, yaxunl.
Herald added a project: clang.

- The deduced address space needs applying to its element type as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70981

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaOpenCL/address-spaces.cl


Index: clang/test/SemaOpenCL/address-spaces.cl
===
--- clang/test/SemaOpenCL/address-spaces.cl
+++ clang/test/SemaOpenCL/address-spaces.cl
@@ -241,3 +241,10 @@
   __private private_int_t var5; // expected-warning {{multiple identical 
address spaces specified for type}}
   __private private_int_t *var6;// expected-warning {{multiple identical 
address spaces specified for type}}
 }
+
+void func_with_array_param(const unsigned data[16]);
+
+__kernel void k() {
+  unsigned data[16];
+  func_with_array_param(data);
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6128,7 +6128,23 @@
 if ((getLangOpts().OpenCLCPlusPlus || getLangOpts().OpenCLVersion >= 200) 
&&
 Var->hasGlobalStorage())
   ImplAS = LangAS::opencl_global;
+// If the original type from a decayed type is an array type and that array
+// type has no address space yet, deduce it now.
+if (auto DT = dyn_cast(Type)) {
+  auto OrigTy = DT->getOriginalType();
+  if (!OrigTy.getQualifiers().hasAddressSpace() && OrigTy->isArrayType()) {
+OrigTy = Context.getAddrSpaceQualType(OrigTy, ImplAS);
+OrigTy = QualType(Context.getAsArrayType(OrigTy), 0);
+Type = Context.getDecayedType(OrigTy);
+  }
+}
 Type = Context.getAddrSpaceQualType(Type, ImplAS);
+// Apply any qualifiers (including address space) from the array type to
+// the element type. This implements C99 6.7.3p8: "If the specification of
+// an array type includes any type qualifiers, the element type is so
+// qualified, not the array type."
+if (Type->isArrayType())
+  Type = QualType(Context.getAsArrayType(Type), 0);
 Decl->setType(Type);
   }
 }


Index: clang/test/SemaOpenCL/address-spaces.cl
===
--- clang/test/SemaOpenCL/address-spaces.cl
+++ clang/test/SemaOpenCL/address-spaces.cl
@@ -241,3 +241,10 @@
   __private private_int_t var5; // expected-warning {{multiple identical address spaces specified for type}}
   __private private_int_t *var6;// expected-warning {{multiple identical address spaces specified for type}}
 }
+
+void func_with_array_param(const unsigned data[16]);
+
+__kernel void k() {
+  unsigned data[16];
+  func_with_array_param(data);
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6128,7 +6128,23 @@
 if ((getLangOpts().OpenCLCPlusPlus || getLangOpts().OpenCLVersion >= 200) &&
 Var->hasGlobalStorage())
   ImplAS = LangAS::opencl_global;
+// If the original type from a decayed type is an array type and that array
+// type has no address space yet, deduce it now.
+if (auto DT = dyn_cast(Type)) {
+  auto OrigTy = DT->getOriginalType();
+  if (!OrigTy.getQualifiers().hasAddressSpace() && OrigTy->isArrayType()) {
+OrigTy = Context.getAddrSpaceQualType(OrigTy, ImplAS);
+OrigTy = QualType(Context.getAsArrayType(OrigTy), 0);
+Type = Context.getDecayedType(OrigTy);
+  }
+}
 Type = Context.getAddrSpaceQualType(Type, ImplAS);
+// Apply any qualifiers (including address space) from the array type to
+// the element type. This implements C99 6.7.3p8: "If the specification of
+// an array type includes any type qualifiers, the element type is so
+// qualified, not the array type."
+if (Type->isArrayType())
+  Type = QualType(Context.getAsArrayType(Type), 0);
 Decl->setType(Type);
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70579: [coroutines][PR41909] Generalize fix from D62550

[Brian Gesiak via Phabricator via cfe-commits]

modocache added a comment.

@GorNishanov, @rsmith, friendly ping! @rsmith this patch addresses your 
requests from https://reviews.llvm.org/D62550.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70579



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

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 updated this revision to Diff 231971.
ghvg1313 marked an inline comment as done.
ghvg1313 added a comment.

- rename option, remove handlers for nested block itself


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1,6 +1,29 @@
   verifyFormat("include \"a.td\"\ninclude \"b.td\"", Style);
 }
 
+TEST_F(FormatTest, DoNotBreakLineBeforeNestedBlockParam) {
+  FormatStyle Style = getLLVMStyle();
+  Style.ObjCBreakBeforeNestedBlockParam = false;
+  Style.ColumnLimit = 0;
+
+  verifyFormat("[self.test1 t:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = v;\n}]", Style);
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = v;\n}]", Style);
+
+  verifyFormat("[self.test1 t:self w:self callback:^(typeof(self) self, " \
+   "NSNumber *u, NSNumber *v) {\n  u = c;\n} w:self " \
+   "callback2:^(typeof(self) self, NSNumber *a, NSNumber *b, " \
+   "NSNumber *c) {\n  b = c;\n}]", Style);
+
+  Style.ColumnLimit = 80;
+  verifyFormat("[self.test_method a:self b:self\n" \
+   "   callback:^(typeof(self) self, NSNumber *u, " \
+   "NSNumber *v) {\n" \
+   " u = v;\n" \"   }]", Style);
+}
+
 TEST_F(FormatTest, ArrayOfTemplates) {
   EXPECT_EQ("auto a = new unique_ptr[10];",
 format("auto a = new unique_ptr [ 10];"));
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -497,6 +497,8 @@
 IO.mapOptional("NamespaceMacros", Style.NamespaceMacros);
 IO.mapOptional("ObjCBinPackProtocolList", Style.ObjCBinPackProtocolList);
 IO.mapOptional("ObjCBlockIndentWidth", Style.ObjCBlockIndentWidth);
+IO.mapOptional("ObjCBreakBeforeNestedBlockParam",
+   Style.ObjCBreakBeforeNestedBlockParam);
 IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty);
 IO.mapOptional("ObjCSpaceBeforeProtocolList",
Style.ObjCSpaceBeforeProtocolList);
@@ -792,6 +794,7 @@
   LLVMStyle.NamespaceIndentation = FormatStyle::NI_None;
   LLVMStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Auto;
   LLVMStyle.ObjCBlockIndentWidth = 2;
+  LLVMStyle.ObjCBreakBeforeNestedBlockParam = true;
   LLVMStyle.ObjCSpaceAfterProperty = false;
   LLVMStyle.ObjCSpaceBeforeProtocolList = true;
   LLVMStyle.PointerAlignment = FormatStyle::PAS_Right;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1380,7 +1380,8 @@
   (!BinPackInconclusiveFunctions &&
Current.PackingKind == PPK_Inconclusive)));
 
-if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen) {
+if (Current.is(TT_ObjCMethodExpr) && Current.MatchingParen &&
+Style.ObjCBreakBeforeNestedBlockParam) {
   if (Style.ColumnLimit) {
 // If this '[' opens an ObjC call, determine whether all parameters fit
 // into one line and put one per line if they don't.
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1646,6 +1646,10 @@
   /// ``@property (readonly)`` instead of ``@property(readonly)``.
   bool ObjCSpaceAfterProperty;
 
+  /// \brief Break parameters list into lines when there is nested block
+  /// parameters in a fuction call.
+  bool ObjCBreakBeforeNestedBlockParam;
+
   /// Add a space in front of an Objective-C protocol list, i.e. use
   /// ``Foo `` instead of ``Foo``.
   bool ObjCSpaceBeforeProtocolList;
@@ -2126,6 +2130,7 @@
NamespaceMacros == R.NamespaceMacros &&
ObjCBinPackProtocolList == R.ObjCBinPackProtocolList &&
ObjCBlockIndentWidth == R.ObjCBlockIndentWidth &&
+   ObjCBreakBeforeNestedBlockParam == R.ObjCBreakBeforeNestedBlockParam &&
ObjCSpaceAfterProperty == R.ObjCSpaceAfterProperty &&
ObjCSpaceBeforeProtocolList == R.ObjCSpaceBeforeProtocolList &&
PenaltyBreakAssignment == R.PenaltyBreakAssignment &&
Index: clang/docs/ReleaseNotes.rst
===
--- 

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/ContinuationIndenter.cpp:869
   State.Stack[i].BreakBeforeParameter = true;
 
   if (PreviousNonComment &&

MyDeveloperDay wrote:
> I'm a little confused by the code, this feels to me like its covering 
> something more than a specific than just the ObjectiveC case, what is meant 
> here by a nestedblockspecialcase? this feels like we are turning off 
> BreakBeforeParameter by another method?
> 
> I get the code below, just wondering why this is also needed.
I think you are right, bin packing is already avoided as the comment says so I 
don't have to do anything extra.

Was trying to handle linebreaking within block as well. Not needed indeed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926



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

Re: [PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

[Sam McCall via cfe-commits]

Right, should be fixed in 1374f7bd9f07115a7c69908bf50ba22e77b0e149

(Just like the last revert, libstdc++ is letting me get away with things
that the others don't...)

On Tue, Dec 3, 2019 at 9:53 PM Nico Weber via Phabricator <
revi...@reviews.llvm.org> wrote:

> thakis added a comment.
>
> Mac is happy now, but it fails to build on Windows:
> http://45.33.8.238/win/3327/step_4.txt
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D70512/new/
>
> https://reviews.llvm.org/D70512
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 1374f7b - [clangd] Fix comparator const after c9c714c7054d555398c767cb39d7d97600b3d9d1

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2019-12-03T22:13:45+01:00
New Revision: 1374f7bd9f07115a7c69908bf50ba22e77b0e149

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

LOG: [clangd] Fix comparator const after 
c9c714c7054d555398c767cb39d7d97600b3d9d1

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/Selection.cpp 
b/clang-tools-extra/clangd/Selection.cpp
index 6ff5eabe566c..ffa48f3a57d9 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -112,7 +112,7 @@ class IntervalSet {
 private:
   using TokenRange = llvm::ArrayRef;
   struct RangeLess {
-bool operator()(llvm::ArrayRef L, llvm::ArrayRef R) {
+bool operator()(llvm::ArrayRef L, llvm::ArrayRef R) const {
   return L.begin() < R.begin();
 }
   };



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

[PATCH] D68101: [MC][ELF] Prevent globals with an explicit section from being mergeable

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

> But ELF allows object files to contain an arbitrary number of what I've been 
> calling "section units" that will be assembled into a single section in the 
> image.

More precisely, in assembler, you can specify sections dis-jointly, but they 
will be rejoined when assembled into an ELF object, as ELF section names are 
unique and cannot be discontinuous.


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

https://reviews.llvm.org/D68101



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

[PATCH] D70980: [HIP] Remove opencl.amdgcn.lib

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: ashi1, b-sumner.
Herald added a subscriber: Anastasia.

https://reviews.llvm.org/D70980

Files:
  clang/lib/Driver/ToolChains/HIP.cpp


Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -343,9 +343,8 @@
 else
   WaveFrontSizeBC = "oclc_wavefrontsize64_off.amdgcn.bc";
 
-BCLibs.append({"hip.amdgcn.bc", "opencl.amdgcn.bc", "ocml.amdgcn.bc",
-   "ockl.amdgcn.bc", "oclc_finite_only_off.amdgcn.bc",
-   FlushDenormalControlBC,
+BCLibs.append({"hip.amdgcn.bc", "ocml.amdgcn.bc", "ockl.amdgcn.bc",
+   "oclc_finite_only_off.amdgcn.bc", FlushDenormalControlBC,
"oclc_correctly_rounded_sqrt_on.amdgcn.bc",
"oclc_unsafe_math_off.amdgcn.bc", ISAVerBC,
WaveFrontSizeBC});


Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -343,9 +343,8 @@
 else
   WaveFrontSizeBC = "oclc_wavefrontsize64_off.amdgcn.bc";
 
-BCLibs.append({"hip.amdgcn.bc", "opencl.amdgcn.bc", "ocml.amdgcn.bc",
-   "ockl.amdgcn.bc", "oclc_finite_only_off.amdgcn.bc",
-   FlushDenormalControlBC,
+BCLibs.append({"hip.amdgcn.bc", "ocml.amdgcn.bc", "ockl.amdgcn.bc",
+   "oclc_finite_only_off.amdgcn.bc", FlushDenormalControlBC,
"oclc_correctly_rounded_sqrt_on.amdgcn.bc",
"oclc_unsafe_math_off.amdgcn.bc", ISAVerBC,
WaveFrontSizeBC});
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 59e69fe - Fix warning on extra ';'. NFC.

[Michael Liao via cfe-commits]

Author: Michael Liao
Date: 2019-12-03T16:02:55-05:00
New Revision: 59e69fefab883984e81c77aef58ba587060e87f2

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

LOG: Fix warning on extra ';'. NFC.

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 93cb36961ee5..d5d394e61926 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2597,7 +2597,7 @@ bool TokenAnnotator::spaceRequiredBeforeParens(const 
FormatToken ) const {
 static bool isKeywordWithCondition(const FormatToken ) {
   return Tok.isOneOf(tok::kw_if, tok::kw_for, tok::kw_while, tok::kw_switch,
  tok::kw_constexpr);
-};
+}
 
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine ,
   const FormatToken ,



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

[PATCH] D70701: Fix more VFS tests on Windows

[Adrian McCarthy via Phabricator via cfe-commits]

amccarth marked 2 inline comments as done.
amccarth added a comment.

A (hopefully) more cogent response than my last round.  I'm still hoping to 
hear from VFS owners.




Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1077-1078
+std::error_code RedirectingFileSystem::makeAbsolute(SmallVectorImpl 
) const {
+  if (llvm::sys::path::is_absolute(Path, llvm::sys::path::Style::posix) ||
+  llvm::sys::path::is_absolute(Path, llvm::sys::path::Style::windows))
+return {};

amccarth wrote:
> rnk wrote:
> > I think Posix-style paths are considered absolute, even on Windows. The 
> > opposite isn't true, a path with a drive letter is considered to be 
> > relative if the default path style is Posix. But, I don't think that 
> > matters. We only end up in this mixed path style situation on Windows.
> > 
> > Does this change end up being necessary? I would expect the existing 
> > implementation of makeAbsolute to do the right thing on Windows as is. I 
> > think the other change seems to be what matters.
> Yes, it's necessary.  The Posix-style path `\tests` is not considered 
> absolute on Windows.  Thus the original makeAboslute would merge it with the 
> current working directory, which gives it a drive letter, like `D:\tests\`.  
> The drive letter component then causes comparisons to fail.
To make sure I wasn't misremembering or hallucinating, I double-checked the 
behavior here:  Posix-style paths like `\tests` are not considered absolute 
paths in Windows-style.



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1448-1449
+  // which style we have, and use it consistently.
+  if (sys::path::is_absolute(Name, sys::path::Style::posix)) {
+path_style = sys::path::Style::posix;
+  } else if (sys::path::is_absolute(Name, sys::path::Style::windows)) {

amccarth wrote:
> rnk wrote:
> > I wonder if there's a simpler fix here. If the working directory is an 
> > absolute Windows-style path, we could prepend the drive letter of the 
> > working directory onto any absolute Posix style paths read from YAML files. 
> > That's somewhat consistent with what native Windows tools do. In cmd, you 
> > can run `cd \Windows`, and that will mean `C:\Windows` if the active driver 
> > letter is C. I think on Windows every drive has an active directory, but 
> > that's not part of the file system model.
> I'm not seeing how this would be simpler.
> 
> I don't understand the analogy of your proposal to what the native Windows 
> tools do.  When you say, `cd \Windows`, the `\Windows` is _not_ an absolute 
> path.  It's relative to the current drive.
> 
> I could be wrong, but I don't think prepending the drive onto absolution 
> Posix style paths read from YAML would work.  That would give us something 
> like `D:/tests` (which is what was happening in makeAbsolute) and that won't 
> match paths generated from non-YAML sources, which will still come out as 
> `/tests/foo.h`.
> 
> > I think on Windows every drive has an active directory 
> 
> Yep.  I think of it as "every drive has a _current_ directory" and each 
> process has a current drive.  When you want the current working directory, 
> you get the current directory of the current drive.
> ... I don't think prepending the drive onto absolution Posix style paths read 
> from YAML would work

I misspoke.  The idea of prepending the drive onto absolute Posix-style paths 
read from the YAML probably could be made to work for the common cases like the 
ones in these tests.

But I still don't think that approach would simplify anything.

Furthermore, I think it could open a potential Pandora's box of weird corner 
cases.  For example, in a system with multiple drives, the current drive may 
not always be the "correct" one to use.  Slapping a drive letter onto a 
Posix-style path creates a Frankenstein hybrid that's neither Windows-style nor 
Posix-style.  Doing so because we know the subsequent code would then recognize 
it as an absolute path seems like a way to create an unnecessary coupling 
between the VFS YAML parser and the LLVM path support.

In my mind, the model here is that these roots can be in either style.  I 
prefer to let the code explicitly reflect that fact rather than trying to 
massage some of the paths into a form that happens to cause the right outcome.


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

https://reviews.llvm.org/D70701



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

[PATCH] D68578: [HIP] Fix device stub name

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 231966.
yaxunl added a comment.
Herald added subscribers: nhaehnle, jvesely.

clean up and fix assertions.


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

https://reviews.llvm.org/D68578

Files:
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/CodeGen/CGCUDARuntime.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
  clang/test/CodeGenCUDA/kernel-stub-name.cu
  clang/test/CodeGenCUDA/unnamed-types.cu

Index: clang/test/CodeGenCUDA/unnamed-types.cu
===
--- clang/test/CodeGenCUDA/unnamed-types.cu
+++ clang/test/CodeGenCUDA/unnamed-types.cu
@@ -36,4 +36,4 @@
   }(p);
 }
 // HOST: @__hip_register_globals
-// HOST: __hipRegisterFunction{{.*}}@_Z2k0IZZ2f1PfENKUlS0_E_clES0_EUlfE_EvS0_T_{{.*}}@0
+// HOST: __hipRegisterFunction{{.*}}@_Z17__device_stub__k0IZZ2f1PfENKUlS0_E_clES0_EUlfE_EvS0_T_{{.*}}@0
Index: clang/test/CodeGenCUDA/kernel-stub-name.cu
===
--- clang/test/CodeGenCUDA/kernel-stub-name.cu
+++ clang/test/CodeGenCUDA/kernel-stub-name.cu
@@ -6,15 +6,50 @@
 
 #include "Inputs/cuda.h"
 
+extern "C" __global__ void ckernel() {}
+
+namespace ns {
+__global__ void nskernel() {}
+} // namespace ns
+
 template
 __global__ void kernelfunc() {}
 
+__global__ void kernel_decl();
+
+// Device side kernel names
+
+// CHECK: @[[CKERN:[0-9]*]] = {{.*}} c"ckernel\00"
+// CHECK: @[[NSKERN:[0-9]*]] = {{.*}} c"_ZN2ns8nskernelEv\00"
+// CHECK: @[[TKERN:[0-9]*]] = {{.*}} c"_Z10kernelfuncIiEvv\00"
+
+// Non-template kernel stub functions
+
+// CHECK: define{{.*}}@[[CSTUB:__device_stub__ckernel]]
+// CHECK: call{{.*}}@hipLaunchByPtr{{.*}}@[[CSTUB]]
+// CHECK: define{{.*}}@[[NSSTUB:_ZN2ns23__device_stub__nskernelEv]]
+// CHECK: call{{.*}}@hipLaunchByPtr{{.*}}@[[NSSTUB]]
+
 // CHECK-LABEL: define{{.*}}@_Z8hostfuncv()
-// CHECK: call void @[[STUB:_Z10kernelfuncIiEvv.stub]]()
-void hostfunc(void) { kernelfunc<<<1, 1>>>(); }
+// CHECK: call void @[[CSTUB]]()
+// CHECK: call void @[[NSSTUB]]()
+// CHECK: call void @[[TSTUB:_Z25__device_stub__kernelfuncIiEvv]]()
+// CHECK: call void @[[DSTUB:_Z26__device_stub__kernel_declv]]()
+void hostfunc(void) {
+  ckernel<<<1, 1>>>();
+  ns::nskernel<<<1, 1>>>();
+  kernelfunc<<<1, 1>>>();
+  kernel_decl<<<1, 1>>>();
+}
+
+// Template kernel stub functions
+
+// CHECK: define{{.*}}@[[TSTUB]]
+// CHECK: call{{.*}}@hipLaunchByPtr{{.*}}@[[TSTUB]]
 
-// CHECK: define{{.*}}@[[STUB]]
-// CHECK: call{{.*}}@hipLaunchByPtr{{.*}}@[[STUB]]
+// CHECK: declare{{.*}}@[[DSTUB]]
 
 // CHECK-LABEL: define{{.*}}@__hip_register_globals
-// CHECK: call{{.*}}@__hipRegisterFunction{{.*}}@[[STUB]]
+// CHECK: call{{.*}}@__hipRegisterFunction{{.*}}@[[CSTUB]]{{.*}}@[[CKERN]]
+// CHECK: call{{.*}}@__hipRegisterFunction{{.*}}@[[NSSTUB]]{{.*}}@[[NSKERN]]
+// CHECK: call{{.*}}@__hipRegisterFunction{{.*}}@[[TSTUB]]{{.*}}@[[TKERN]]
Index: clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
===
--- clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
+++ clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
@@ -13,19 +13,19 @@
 // HOST-NOT: %struct.T.coerce
 
 // CHECK: define amdgpu_kernel void  @_Z7kernel1Pi(i32 addrspace(1)* %x.coerce)
-// HOST: define void @_Z7kernel1Pi.stub(i32* %x)
+// HOST: define void @_Z22__device_stub__kernel1Pi(i32* %x)
 __global__ void kernel1(int *x) {
   x[0]++;
 }
 
 // CHECK: define amdgpu_kernel void  @_Z7kernel2Ri(i32 addrspace(1)* dereferenceable(4) %x.coerce)
-// HOST: define void @_Z7kernel2Ri.stub(i32* dereferenceable(4) %x)
+// HOST: define void @_Z22__device_stub__kernel2Ri(i32* dereferenceable(4) %x)
 __global__ void kernel2(int ) {
   x++;
 }
 
 // CHECK: define amdgpu_kernel void  @_Z7kernel3PU3AS2iPU3AS1i(i32 addrspace(2)* %x, i32 addrspace(1)* %y)
-// HOST: define void @_Z7kernel3PU3AS2iPU3AS1i.stub(i32 addrspace(2)* %x, i32 addrspace(1)* %y)
+// HOST: define void @_Z22__device_stub__kernel3PU3AS2iPU3AS1i(i32 addrspace(2)* %x, i32 addrspace(1)* %y)
 __global__ void kernel3(__attribute__((address_space(2))) int *x,
 __attribute__((address_space(1))) int *y) {
   y[0] = x[0];
@@ -43,7 +43,7 @@
 // `by-val` struct will be coerced into a similar struct with all generic
 // pointers lowerd into global ones.
 // CHECK: define amdgpu_kernel void @_Z7kernel41S(%struct.S.coerce %s.coerce)
-// HOST: define void @_Z7kernel41S.stub(i32* %s.coerce0, float* %s.coerce1)
+// HOST: define void @_Z22__device_stub__kernel41S(i32* %s.coerce0, float* %s.coerce1)
 __global__ void kernel4(struct S s) {
   s.x[0]++;
   s.y[0] += 1.f;
@@ -51,7 +51,7 @@
 
 

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Mac is happy now, but it fails to build on Windows: 
http://45.33.8.238/win/3327/step_4.txt


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70512



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

[PATCH] D70571: [Coverage] Emit a gap region to cover switch bodies

[Vedant Kumar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG859bf4d2bea2: [Coverage] Emit a gap region to cover switch 
bodies (authored by vsk).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D70571?vs=231724=231964#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70571

Files:
  clang/docs/SourceBasedCodeCoverage.rst
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CoverageMapping/switch.cpp
  clang/test/CoverageMapping/switchmacro.c

Index: clang/test/CoverageMapping/switchmacro.c
===
--- clang/test/CoverageMapping/switchmacro.c
+++ clang/test/CoverageMapping/switchmacro.c
@@ -4,7 +4,7 @@
 
 // CHECK: foo
 int foo(int i) { // CHECK-NEXT: File 0, [[@LINE]]:16 -> {{[0-9]+}}:2 = #0
-  switch (i) {
+  switch (i) {   // CHECK-NEXT: Gap,File 0, [[@LINE]]:14 -> {{[0-9]+}}:11 = 0
   default:   // CHECK-NEXT: File 0, [[@LINE]]:3 -> {{[0-9]+}}:11 = #2
 if (i == 1)  // CHECK-NEXT: File 0, [[@LINE]]:9 -> [[@LINE]]:15 = #2
   return 0;  // CHECK: File 0, [[@LINE]]:7 -> [[@LINE]]:15 = #3
Index: clang/test/CoverageMapping/switch.cpp
===
--- clang/test/CoverageMapping/switch.cpp
+++ clang/test/CoverageMapping/switch.cpp
@@ -2,11 +2,11 @@
 
 // CHECK: foo
 void foo(int i) {   // CHECK-NEXT: File 0, [[@LINE]]:17 -> [[@LINE+8]]:2 = #0
-  switch(i) {
+  switch(i) {   // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+4]]:10 = 0
   case 1:   // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:11 = #2
 return;
   case 2:   // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #3
-break;  // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+2]]:3 = #1
+break;  // CHECK-NEXT: Gap,File 0, [[@LINE]]:10 -> [[@LINE+2]]:3 = #1
   }
   int x = 0;// CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:2 = #1
 }
@@ -29,7 +29,7 @@
 nop();
 
   switch (i) {  // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+6]]:2 = #4
-nop();  // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE+2]]:10 = 0
+nop();  // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE+2]]:10 = 0
   case 1:   // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #7
 nop();
   }
@@ -47,7 +47,7 @@
 // CHECK-NEXT: main
 int main() {// CHECK-NEXT: File 0, [[@LINE]]:12 -> [[@LINE+35]]:2 = #0
   int i = 0;
-  switch(i) {
+  switch(i) {   // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+8]]:10 = 0
   case 0:   // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = #2
 i = 1;
 break;
@@ -58,16 +58,16 @@
 break;  // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+2]]:3 = #1
   }
   switch(i) {   // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+23]]:2 = #1
-  case 0:   // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = #6
-i = 1;
+  case 0:   // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:13 -> [[@LINE+6]]:10 = 0
+i = 1;  // CHECK-NEXT: File 0, [[@LINE-1]]:3 -> [[@LINE+1]]:10 = #6
 break;
   case 1:   // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+3]]:10 = #7
 i = 2;
   default:  // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = (#7 + #8)
 break;  // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+3]]:3 = #5
   }
-
-  switch(i) {   // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+13]]:2 = #5
+// CHECK-NEXT: File 0, [[@LINE+1]]:3 -> [[@LINE+14]]:2 = #5
+  switch(i) {   // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+6]]:11 = 0
   case 1:   // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+5]]:11 = #10
   case 2:   // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+4]]:11 = (#10 + #11)
 i = 11;
@@ -82,10 +82,23 @@
   return 0;
 }
 
+ // CHECK: pr44011
+int pr44011(int i) { // CHECK-NEXT: File 0, [[@LINE]]:20 -> {{.*}}:2 = #0
+  switch (i) {   // CHECK-NEXT: Gap,File 0, [[@LINE]]:14 -> [[@LINE+6]]:13 = 0
+
+  case 1:// CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:13 = #2
+return 0;
+
+  default:   // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:13 = #3
+return 1;
+  }
+} // A region for counter #1 is missing due to the missing return.
+
+
 // FIXME: End location for "case 1" shouldn't point at the end of the switch.
  // CHECK: fallthrough
 int fallthrough(int i) { // CHECK-NEXT: File 0, [[@LINE]]:24 -> [[@LINE+12]]:2 = #0
-  switch(i) {
+  switch(i) {   // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+9]]:10 = 0
   case 1:   // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+8]]:10 = #2
 i = 23;
   case 2:   // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = (#2 + #3)
@@ -101,7 +114,7 @@
 void abort(void) 

Re: Github build status reporting

[Alex L via cfe-commits]

Hi Galina,

Thanks for doing this! I'm curious as to how the buildbot CI communicates
with Github, is the buildbot itself capable of performing the Github API
requests, or do you have an external tool/script that does that?

Thanks,
Alex

On Mon, 25 Nov 2019 at 12:59, Galina Kistanova via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Hello everyone,
>
> I have configured buildbot to report build statuses to github. It is
> running in a pilot mode. Only the following 4 builders annotate revisions
> they build for now:
>
> * llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast
> * llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast
> * llvm-clang-x86_64-expensive-checks-ubuntu
> * llvm-clang-x86_64-win-fast
>
> Please let me know if you have any questions, concerns, or see any issues.
>
> Thanks
>
> Galina
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 859bf4d - [Coverage] Emit a gap region to cover switch bodies

[Vedant Kumar via cfe-commits]

Author: Vedant Kumar
Date: 2019-12-03T12:35:54-08:00
New Revision: 859bf4d2bea2404bd2eac92451f2db4371ec6eb4

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

LOG: [Coverage] Emit a gap region to cover switch bodies

Emit a gap region beginning where the switch body begins. This sets line
execution counts in the areas between non-overlapping cases to 0.

This also removes some special handling of the first case in a switch:
these are now treated like any other case.

This does not resolve an outstanding issue with case statement regions
that do not end when a region is terminated. But it should address
llvm.org/PR44011.

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

Added: 


Modified: 
clang/docs/SourceBasedCodeCoverage.rst
clang/lib/CodeGen/CoverageMappingGen.cpp
clang/test/CoverageMapping/switch.cpp
clang/test/CoverageMapping/switchmacro.c

Removed: 




diff  --git a/clang/docs/SourceBasedCodeCoverage.rst 
b/clang/docs/SourceBasedCodeCoverage.rst
index 73197a57713f..7e711819be34 100644
--- a/clang/docs/SourceBasedCodeCoverage.rst
+++ b/clang/docs/SourceBasedCodeCoverage.rst
@@ -302,3 +302,37 @@ Drawbacks and limitations
   If the call to ``may_throw()`` propagates an exception into ``f``, the code
   coverage tool may mark the ``return`` statement as executed even though it is
   not. A call to ``longjmp()`` can have similar effects.
+
+Clang implementation details
+
+
+This section may be of interest to those wishing to understand or improve
+the clang code coverage implementation.
+
+Gap regions
+---
+
+Gap regions are source regions with counts. A reporting tool cannot set a line
+execution count to the count from a gap region unless that region is the only
+one on a line.
+
+Gap regions are used to eliminate unnatural artifacts in coverage reports, such
+as red "unexecuted" highlights present at the end of an otherwise covered line,
+or blue "executed" highlights present at the start of a line that is otherwise
+not executed.
+
+Switch statements
+-
+
+The region mapping for a switch body consists of a gap region that covers the
+entire body (starting from the '{' in 'switch (...) {', and terminating where 
the
+last case ends). This gap region has a zero count: this causes "gap" areas in
+between case statements, which contain no executable code, to appear uncovered.
+
+When a switch case is visited, the parent region is extended: if the parent
+region has no start location, its start location becomes the start of the case.
+This is used to support switch statements without a ``CompoundStmt`` body, in
+which the switch body and the single case share a count.
+
+For switches with ``CompoundStmt`` bodies, a new region is created at the start
+of each switch case.

diff  --git a/clang/lib/CodeGen/CoverageMappingGen.cpp 
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 0a7a4fe33ac2..bdecff39c88f 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1114,8 +1114,8 @@ struct CounterCoverageMappingBuilder
 // Make a region for the body of the switch.  If the body starts with
 // a case, that case will reuse this region; otherwise, this covers
 // the unreachable code at the beginning of the switch body.
-size_t Index =
-pushRegion(Counter::getZero(), getStart(CS->body_front()));
+size_t Index = pushRegion(Counter::getZero(), getStart(CS));
+getRegion().setGap(true);
 for (const auto *Child : CS->children())
   Visit(Child);
 

diff  --git a/clang/test/CoverageMapping/switch.cpp 
b/clang/test/CoverageMapping/switch.cpp
index 30c64922201f..25ea4053f4e2 100644
--- a/clang/test/CoverageMapping/switch.cpp
+++ b/clang/test/CoverageMapping/switch.cpp
@@ -2,11 +2,11 @@
 
 // CHECK: foo
 void foo(int i) {   // CHECK-NEXT: File 0, [[@LINE]]:17 -> [[@LINE+8]]:2 = #0
-  switch(i) {
+  switch(i) {   // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+4]]:10 
= 0
   case 1:   // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:11 = #2
 return;
   case 2:   // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #3
-break;  // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+2]]:3 = #1
+break;  // CHECK-NEXT: Gap,File 0, [[@LINE]]:10 -> [[@LINE+2]]:3 = 
#1
   }
   int x = 0;// CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:2 = #1
 }
@@ -29,7 +29,7 @@ void bar(int i) {   // CHECK-NEXT: File 0, [[@LINE]]:17 -> 
[[@LINE+20]]:2 = #0
 nop();
 
   switch (i) {  // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+6]]:2 = #4
-nop();  // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE+2]]:10 = 0
+nop();  // 

[PATCH] D70726: [OpenMP50] Add parallel master construct

[Chi Chun Chen via Phabricator via cfe-commits]

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



Comment at: clang/test/OpenMP/nesting_of_regions.cpp:3024
+  {
+#pragma omp master // OK, though second 'master' is redundant
+{

ABataev wrote:
> Is this allowed by the standard?
I didn't see any restriction about master directive inside master directive. 
Also, nesting_of_regions also have tests for nested master construct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70726



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

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Yay, it actually works!

I only have minor comments and ignorable hand-waving now.

In D70725#1767643 , @Charusso wrote:

> In D70725#1767579 , @xazax.hun wrote:
>
> > Just a small side note. If the state was same as the previous we do not end 
> > up creating a new ExplodedNode right? Is this also the case when we add a 
> > NoteTag?
>
>
> It only happens for evaluation when you cannot evaluate something. Other than 
> that `Pre/PostCall` working fine to add a node with the `NoteTag`.


Tag pointers are part of the `ProgramPoint`'s identity, which in turn are part 
of the `ExplodedNode`'s identity. If you make a new note tag and transition 
into it for the first time, the new node is guaranteed to be fresh.




Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:215
+  if (!State->get(Sym)) {
+N = N->pred_empty() ? nullptr : *(N->pred_begin());
+  }

`N = N->getFirstPred()`.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:224
+  if (HState && (HState->isAllocated() || HState->maybeAllocated()))
+AcquireNode = N;
+  break;

`return N;`? (eliminates a variable)



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:228
+Pred = N;
+N = N->pred_empty() ? nullptr : *(N->pred_begin());
+  }

`N = N->getFirstPred()`.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:363-366
+if (() !=  &&
+() !=  &&
+() != )
+  return "";

Ugh, it sucks that we have to do this by hand.

Why would the symbol be interesting if the bugtype is wrong? You imagine 
something like division by a zero handle? Do we really mind the updates to the 
handle highlighted in this case? I guess we do.

Maybe we should make "note tag tags" to avoid users writing this by hand. Like, 
note tags are tagged with note tag tags and the report is also tagged with a 
note tag tag and the tag is invoked only if the tag tag on the report matches 
the tag tag on the tag. Setting a tag tag on the report will be equivalent to 
calling an "addVisitor" on the report in the visitor API, which was actually a 
good part of the visitor API.

Just thinking out loud, please ignore. This code is a nice example to 
generalize from.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:369
+  std::string text = note(BR);
+  if (!text.empty())
+return text;

Another option here is to concatenate the notes if there are indeed two 
interesting handles at once: `Handle is allocated through 2nd parameter; Handle 
is released through 3rd parameter". In this checker it probably never happens, 
as every report is about exactly one handle (is it true for leak reports 
though?).


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

https://reviews.llvm.org/D70725



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

[clang] bf830b0 - Switch to opening the temp file in binary mode

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2019-12-03T15:31:46-05:00
New Revision: bf830b01a21e6ff2f44c17be4ad4ee897465a677

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

LOG: Switch to opening the temp file in binary mode

This corrects an issue where the script would write the file with the
incorrect line endings on Windows.

Added: 


Modified: 
clang/test/AST/gen_ast_dump_json_test.py

Removed: 




diff  --git a/clang/test/AST/gen_ast_dump_json_test.py 
b/clang/test/AST/gen_ast_dump_json_test.py
index 87b3318f76cd..f783c79faef8 100644
--- a/clang/test/AST/gen_ast_dump_json_test.py
+++ b/clang/test/AST/gen_ast_dump_json_test.py
@@ -180,7 +180,7 @@ def process_file(source_file, clang_binary, 
cmdline_filters, cmdline_opts,
 
 filter_json(j, filters, out_asts)
 
-with tempfile.NamedTemporaryFile("w", delete=False) as f:
+with tempfile.NamedTemporaryFile("wb", delete=False) as f:
 with open(source_file, "r") as srcf:
 for line in srcf.readlines():
 # copy up to the note:



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

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

[Tyker via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbc840b21e161: [Diagnostic] add a warning which warns about 
misleading indentation (authored by Tyker).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70638

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseStmt.cpp
  clang/test/Index/pragma-diag-reparse.c
  clang/test/Misc/warning-wall.c
  clang/test/Parser/warn-misleading-indentation.cpp
  clang/test/Preprocessor/pragma_diagnostic_sections.cpp

Index: clang/test/Preprocessor/pragma_diagnostic_sections.cpp
===
--- clang/test/Preprocessor/pragma_diagnostic_sections.cpp
+++ clang/test/Preprocessor/pragma_diagnostic_sections.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -Wall -Wunused-macros -Wunused-parameter -Wno-uninitialized -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wall -Wunused-macros -Wunused-parameter -Wno-uninitialized -Wno-misleading-indentation -verify %s
 
 // rdar://8365684
 struct S {
Index: clang/test/Parser/warn-misleading-indentation.cpp
===
--- /dev/null
+++ clang/test/Parser/warn-misleading-indentation.cpp
@@ -0,0 +1,208 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wmisleading-indentation -DWITH_WARN %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify -Wall -Wno-unused -DWITH_WARN -DCXX17 %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify -Wall -Wno-unused -Wno-misleading-indentation -DCXX17 %s
+
+#ifndef WITH_WARN
+// expected-no-diagnostics
+#endif
+
+void f0(int i) {
+  if (i)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i = i + 1;
+int x = 0;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
+#endif
+  return;
+#ifdef CXX17
+  if constexpr (false)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i = 0;
+i += 1;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
+#endif
+#endif
+}
+
+void f1(int i) {
+  for (;i;)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i = i + 1;
+i *= 2;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'for'}}
+#endif
+  return;
+}
+
+void f2(int i) {
+  while (i)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i = i + 1; i *= 2;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'while'}}
+#endif
+  return;
+}
+
+void f3(int i) {
+  if (i)
+i = i + 1;
+  else
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i *= 2;
+const int x = 0;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'else'}}
+#endif
+}
+
+#ifdef CXX17
+struct Range {
+  int *begin() {return nullptr;}
+  int *end() {return nullptr;}
+};
+#endif
+
+void f4(int i) {
+  if (i)
+  i *= 2;
+  return;
+  if (i)
+i *= 2;
+;
+  if (i)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i *= 2;
+typedef int Int;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
+#endif
+#ifdef CXX17
+  Range R;
+  for (auto e : R)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i *= 2;
+using Int2 = int;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'for'}}
+#endif
+#endif
+}
+
+int bar(void);
+
+int foo(int* dst)
+{   
+if (dst)
+   return
+bar();
+  if (dst)
+dst = dst + \
+bar();
+  return 0;
+}
+
+void g(int i) {
+  if (1)
+i = 2;
+  else
+ if (i == 3)
+#ifdef WITH_WARN
+// expected-note@-3 {{here}}
+#endif
+i = 4;
+i = 5;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
+#endif
+}
+
+// Or this
+#define TEST i = 5
+void g0(int i) {
+  if (1)
+i = 2;
+  else
+i = 5;
+TEST;
+}
+
+void g1(int i) {
+  if (1)
+i = 2;
+  else if (i == 3)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+  i = 4;
+  i = 5;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
+#endif
+}
+
+void g2(int i) {
+  if (1)
+i = 2;
+  else
+if (i == 3)
+{i = 4;}
+i = 5;
+}
+
+void g6(int i) {
+if (1)
+if (i == 3)
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+i = 4;
+i = 5;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; 

[PATCH] D70861: [NFCI] update formating for misleading indentation warning

[Tyker via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2f9604727526: [NFCI] update formating for misleading 
indentation warning (authored by Tyker).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70861

Files:
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2525,19 +2525,18 @@
 
 // Find the most recent expression bound to the symbol in the current
 // context.
-  if (!ReferenceRegion) {
-if (const MemRegion *MR = C.getLocationRegionIfPostStore(N)) {
-  SVal Val = State->getSVal(MR);
-  if (Val.getAsLocSymbol() == Sym) {
-const VarRegion* VR = MR->getBaseRegion()->getAs();
-// Do not show local variables belonging to a function other than
-// where the error is reported.
-if (!VR ||
-(VR->getStackFrame() == LeakContext->getStackFrame()))
-  ReferenceRegion = MR;
-  }
+if (!ReferenceRegion) {
+  if (const MemRegion *MR = C.getLocationRegionIfPostStore(N)) {
+SVal Val = State->getSVal(MR);
+if (Val.getAsLocSymbol() == Sym) {
+  const VarRegion *VR = MR->getBaseRegion()->getAs();
+  // Do not show local variables belonging to a function other than
+  // where the error is reported.
+  if (!VR || (VR->getStackFrame() == LeakContext->getStackFrame()))
+ReferenceRegion = MR;
 }
   }
+}
 
 // Allocation node, is the last node in the current or parent context in
 // which the symbol was tracked.


Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2525,19 +2525,18 @@
 
 // Find the most recent expression bound to the symbol in the current
 // context.
-  if (!ReferenceRegion) {
-if (const MemRegion *MR = C.getLocationRegionIfPostStore(N)) {
-  SVal Val = State->getSVal(MR);
-  if (Val.getAsLocSymbol() == Sym) {
-const VarRegion* VR = MR->getBaseRegion()->getAs();
-// Do not show local variables belonging to a function other than
-// where the error is reported.
-if (!VR ||
-(VR->getStackFrame() == LeakContext->getStackFrame()))
-  ReferenceRegion = MR;
-  }
+if (!ReferenceRegion) {
+  if (const MemRegion *MR = C.getLocationRegionIfPostStore(N)) {
+SVal Val = State->getSVal(MR);
+if (Val.getAsLocSymbol() == Sym) {
+  const VarRegion *VR = MR->getBaseRegion()->getAs();
+  // Do not show local variables belonging to a function other than
+  // where the error is reported.
+  if (!VR || (VR->getStackFrame() == LeakContext->getStackFrame()))
+ReferenceRegion = MR;
 }
   }
+}
 
 // Allocation node, is the last node in the current or parent context in
 // which the symbol was tracked.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70726: [OpenMP50] Add parallel master construct

[Chi Chun Chen via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2933
+OMPPrivateScope PrivateScope(CGF);
+bool Copyins = CGF.EmitOMPCopyinClause(S);
+(void)CGF.EmitOMPFirstprivateClause(S, PrivateScope);

ABataev wrote:
> Do we need copyins here at all? We have only master thread active here in 
> this construct.
I think we still need to emit copyins since the assignment to the threadprivate 
copies of the other threads must be observable after the parallel region.



Comment at: clang/test/OpenMP/nesting_of_regions.cpp:2999
+  {
+#pragma omp simd
+for (int i = 0; i < 10; ++i)

ABataev wrote:
> Are simds allowed in master?
I didn't see restriction about simds in master and there is also a test under 
"MASTER DIRECTIVE" allows simd directive inside master construct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70726



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

[clang] 2f96047 - [NFCI] update formating for misleading indentation warning

[via cfe-commits]

Author: Tyker
Date: 2019-12-03T21:21:27+01:00
New Revision: 2f9604727526dedcd20b776ace4adfc058641ab2

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

LOG: [NFCI] update formating for misleading indentation warning

Reviewers: xbolva00

Reviewed By: xbolva00

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index a82449951873..01c7afe52041 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2525,19 +2525,18 @@ MallocChecker::LeakInfo 
MallocChecker::getAllocationSite(const ExplodedNode *N,
 
 // Find the most recent expression bound to the symbol in the current
 // context.
-  if (!ReferenceRegion) {
-if (const MemRegion *MR = C.getLocationRegionIfPostStore(N)) {
-  SVal Val = State->getSVal(MR);
-  if (Val.getAsLocSymbol() == Sym) {
-const VarRegion* VR = MR->getBaseRegion()->getAs();
-// Do not show local variables belonging to a function other than
-// where the error is reported.
-if (!VR ||
-(VR->getStackFrame() == LeakContext->getStackFrame()))
-  ReferenceRegion = MR;
-  }
+if (!ReferenceRegion) {
+  if (const MemRegion *MR = C.getLocationRegionIfPostStore(N)) {
+SVal Val = State->getSVal(MR);
+if (Val.getAsLocSymbol() == Sym) {
+  const VarRegion *VR = MR->getBaseRegion()->getAs();
+  // Do not show local variables belonging to a function other than
+  // where the error is reported.
+  if (!VR || (VR->getStackFrame() == LeakContext->getStackFrame()))
+ReferenceRegion = MR;
 }
   }
+}
 
 // Allocation node, is the last node in the current or parent context in
 // which the symbol was tracked.



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

[clang] bc840b2 - [Diagnostic] add a warning which warns about misleading indentation

[via cfe-commits]

Author: Tyker
Date: 2019-12-03T21:21:27+01:00
New Revision: bc840b21e1612adb603b6c17be0329e3737bb943

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

LOG: [Diagnostic] add a warning which warns about misleading indentation

Summary: Add a warning for misleading indentation similar to GCC's 
-Wmisleading-indentation

Reviewers: aaron.ballman, xbolva00

Reviewed By: aaron.ballman, xbolva00

Subscribers: tstellar, cfe-commits, arphaman, Ka-Ka, thakis

Tags: #clang

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

Added: 
clang/test/Parser/warn-misleading-indentation.cpp

Modified: 
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Lex/Preprocessor.h
clang/include/clang/Parse/Parser.h
clang/lib/Parse/ParseStmt.cpp
clang/test/Index/pragma-diag-reparse.c
clang/test/Misc/warning-wall.c
clang/test/Preprocessor/pragma_diagnostic_sections.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 35e939fda95c..478b217a19f6 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -693,6 +693,7 @@ def ZeroLengthArray : DiagGroup<"zero-length-array">;
 def GNUZeroLineDirective : DiagGroup<"gnu-zero-line-directive">;
 def GNUZeroVariadicMacroArguments : 
DiagGroup<"gnu-zero-variadic-macro-arguments">;
 def Fallback : DiagGroup<"fallback">;
+def MisleadingIndentation : DiagGroup<"misleading-indentation">;
 
 // This covers both the deprecated case (in C++98)
 // and the extension case (in C++11 onwards).
@@ -884,7 +885,7 @@ def Consumed   : DiagGroup<"consumed">;
 // Note that putting warnings in -Wall will not disable them by default. If a
 // warning should be active _only_ when -Wall is passed in, mark it as
 // DefaultIgnore in addition to putting it here.
-def All : DiagGroup<"all", [Most, Parentheses, Switch, SwitchBool]>;
+def All : DiagGroup<"all", [Most, Parentheses, Switch, SwitchBool, 
MisleadingIndentation]>;
 
 // Warnings that should be in clang-cl /w4.
 def : DiagGroup<"CL4", [All, Extra]>;

diff  --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index c94d1b99d0e8..8643e1b867f7 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -61,6 +61,13 @@ def warn_null_statement : Warning<
   "remove unnecessary ';' to silence this warning">,
   InGroup, DefaultIgnore;
 
+def warn_misleading_indentation : Warning<
+  "misleading indentation; statement is not part of "
+  "the previous '%select{if|else|for|while|else if}0'">,
+  InGroup, DefaultIgnore;
+def note_previous_statement : Note<
+  "previous statement is here">;
+
 def ext_thread_before : Extension<"'__thread' before '%0'">;
 def ext_keyword_as_ident : ExtWarn<
   "keyword '%0' will be made available as an identifier "

diff  --git a/clang/include/clang/Lex/Preprocessor.h 
b/clang/include/clang/Lex/Preprocessor.h
index e2ddc80d503f..9716196b95c2 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -932,6 +932,12 @@ class Preprocessor {
 return TheModuleLoader.HadFatalFailure;
   }
 
+  /// Retrieve the number of Directives that have been processed by the
+  /// Preprocessor.
+  unsigned getNumDirectives() const {
+return NumDirectives;
+  }
+
   /// True if we are currently preprocessing a #if or #elif directive
   bool isParsingIfOrElifDirective() const {
 return ParsingIfOrElifDirective;

diff  --git a/clang/include/clang/Parse/Parser.h 
b/clang/include/clang/Parse/Parser.h
index ea1116ff7a23..a1e7bbba9b8e 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1122,6 +1122,11 @@ class Parser : public CodeCompletionHandler {
   /// point for skipping past a simple-declaration.
   void SkipMalformedDecl();
 
+  /// The location of the first statement inside an else that might
+  /// have a missleading indentation. If there is no
+  /// MisleadingIndentationChecker on an else active, this location is invalid.
+  SourceLocation MisleadingIndentationElseLoc;
+
 private:
   
//======//
   // Lexing and parsing of C++ inline methods.

diff  --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index 727ab75adae8..dc951dc22f55 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -1191,6 +1191,59 @@ bool Parser::ParseParenExprOrCondition(StmtResult 
*InitStmt,
   return false;
 }
 
+namespace {
+
+enum MisleadingStatementKind { MSK_if, MSK_else, MSK_for, MSK_while };

[PATCH] D70571: [Coverage] Emit a gap region to cover switch bodies

[Vedant Kumar via Phabricator via cfe-commits]

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



Comment at: clang/docs/SourceBasedCodeCoverage.rst:330
+last case ends). This gap region has a zero count: this causes "gap" areas in
+between case statements, which contain no executable code, to appear uncovered.
+

hans wrote:
> I thought the point is that it //doesn't// appear uncovered in coverage 
> reports? I.e. it will have a zero count, but that's expected.
Yes, I'll make it clear that a zero count is expected here.


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

https://reviews.llvm.org/D70571



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

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D70725#1767579 , @xazax.hun wrote:

> Just a small side note. If the state was same as the previous we do not end 
> up creating a new ExplodedNode right? Is this also the case when we add a 
> NoteTag?


It only happens for evaluation when you cannot evaluate something. Other than 
that `Pre/PostCall` working fine to add a node with the `NoteTag`.


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

https://reviews.llvm.org/D70725



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

[PATCH] D68997: Allow searching for prebuilt implicit modules.

[Alexandre Rames via Phabricator via cfe-commits]

arames added a comment.

Ping.
Still looking for someone to take a look. Happy to answer any questions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68997



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

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@rnk up? the mozilla folks confirmed there's no significant regression (see 
https://bugzilla.mozilla.org/show_bug.cgi?id=1588710#c12).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Many of the test cases could be collapsed into one file - using different 
variables that are used, unused, locally or globally declared, etc.

Is this new code only active for C compilations? (does clang reject requests 
for the bpf target when the input is C++?) I ask due to the concerns around 
globals used in inline functions where the inline function is unused - though C 
has inline functions too, so I guess the question stands: Is that a problem? 
What happens?

Should this be driven by a lower level of code generation - ie: is it OK to 
only produce debug info descriptions for variables that are referenced in the 
resulting LLVM IR? (compile time constants wouldn't be described then, for 
instance - since they won't be code generated, loaded from memory, etc)

Is there somewhere I should be reading about the design requirement for these 
global variable descriptions to understand the justification for them & the 
ramifications if there are bugs that cause them not to be emitted?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

[PATCH] D70974: [clang-tidy] Fix PR26274

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 60438 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70974



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

[PATCH] D70524: Support DebugInfo generation for auto return type for C++ functions.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D70524#1761514 , @awpandey wrote:

> Hi @aprantl  and @dblaikie. Currently, there is no test case for the 
> unspecified type, so I have added that in the regression test suite.


It looks to me like there are a few tests for unspecified_type already:

$ grep -r unspecified_type llvm/test
llvm/test/Assembler/debug-info.ll:; CHECK-NEXT: !7 = !DIBasicType(tag: 
DW_TAG_unspecified_type, name: "decltype(nullptr)")
llvm/test/Assembler/debug-info.ll:!8 = !DIBasicType(tag: 
DW_TAG_unspecified_type, name: "decltype(nullptr)")
llvm/test/DebugInfo/COFF/types-std-nullptr-t.ll:!6 = !DIBasicType(tag: 
DW_TAG_unspecified_type, name: "decltype(nullptr)")
llvm/test/DebugInfo/X86/template.ll:; VERIFY-NOT: error: DIE has DW_AT_type 
with incompatible tag DW_TAG_unspecified_type
llvm/test/DebugInfo/X86/template.ll:; CHECK: [[NULLPTR]]:{{ 
*}}DW_TAG_unspecified_type
llvm/test/DebugInfo/X86/template.ll:!34 = !DIBasicType(tag: 
DW_TAG_unspecified_type, name: "decltype(nullptr)")

@aprantl @probinson - I'm going to voice a little more objection to this 
feature as I think I did when it was being discussed for DWARF standardization 
- feel free to push back/veto me/etc: Describing this seems of limited value to 
consumers - the function cannot be called if the return type hasn't been 
resolved. I think modeling these sort of situations the same way member 
function templates are would be fine - omit the declaration entirely in 
translation units that have no function definition for an auto-returning 
function.

The only difference between this and templates is that we /can/ describe it & 
it can be useful for overload resolution - but the same is true of all 
functions (non-member, member, etc) & no DWARF producer I know of produces all 
function declarations into their output, so I'm not sure "make overload 
resolution work" is really a worthwhile goal. (without template descripttions 
(not the instantiations, but the templates themselves which we currently have 
no way of describing even if we wanted to) I don't think it's actually 
achievable)




Comment at: clang/test/CodeGenCXX/debug-info-auto-return.cpp:12-15
+class myClass {
+  int low, high;
+
+public:

I'd probably make this a struct, rather than a class with a public member - 
access modifiers aren't relevant to this test, so far as I can tell, so might 
as well just have everything public by using a struct.



Comment at: clang/test/CodeGenCXX/debug-info-auto-return.cpp:13
+class myClass {
+  int low, high;
+

No need for any member variables, I think?



Comment at: clang/test/CodeGenCXX/debug-info-auto-return.cpp:20-23
+  if (low > high)
+return 1;
+  else
+return 1;

Could strip out the body here - leave a single "return 0;" in there if that's 
useful/needed.



Comment at: llvm/test/DebugInfo/X86/debug-info-auto-return.ll:5-25
+;class myClass {
+;int low, high;
+;
+;  public:
+;  myClass(int a, int b) {
+;low = a;
+;high = b;

This code seems more complicated than it needs to be (it doesn't need any logic 
in the body of the functions, for instance, I think? Nor any member variables, 
nor 'main', etc) & the indentation should be fixed/made consistent.


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

https://reviews.llvm.org/D70524



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

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

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

xazax.hun added a comment.

You were right, the new API looks cleaner even in this case :)


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

https://reviews.llvm.org/D70725



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

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

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

xazax.hun marked an inline comment as done.
xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:362
   }
-  C.addTransition(State);
+  const NoteTag *T = C.getNoteTag([this, notes](BugReport ) -> std::string {
+if (() !=  &&

Hmm, so we have C++14 now and I can do a move capture right? Will update in the 
next iteration.


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

https://reviews.llvm.org/D70725



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

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 60439 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70973



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

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

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

xazax.hun added a comment.

Just a small side note. If the state was same as the previous we do not end up 
creating a new ExplodedNode right? Is this also the case when we add a NoteTag?


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

https://reviews.llvm.org/D70725



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