[PATCH] D64120: Rebase onto current master

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 abandoned this revision.
gamesh411 added a comment.

Missed arcanist diff. Disregard this revision.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64120



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

[PATCH] D59798: [analyzer] Add analyzer option to limit the number of imported TUs

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 207711.
gamesh411 added a comment.

Rebase onto current master


Repository:
  rC Clang

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

https://reviews.llvm.org/D59798

Files:
  include/clang/CrossTU/CrossTranslationUnit.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  lib/CrossTU/CrossTranslationUnit.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/ctu-import-threshold.c
  unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
@@ -70,12 +71,14 @@
 EXPECT_TRUE(llvm::sys::fs::exists(ASTFileName));
 
 // Load the definition from the AST file.
-llvm::Expected NewFDorError =
-CTU.getCrossTUDefinition(FD, "", IndexFileName);
-EXPECT_TRUE((bool)NewFDorError);
-const FunctionDecl *NewFD = *NewFDorError;
+llvm::Expected NewFDorError = handleExpected(
+CTU.getCrossTUDefinition(FD, "", IndexFileName, false),
+[]() { return nullptr; }, [](IndexError &) {});
 
-*Success = NewFD && NewFD->hasBody() && !OrigFDHasBody;
+if (NewFDorError) {
+  const FunctionDecl *NewFD = *NewFDorError;
+  *Success = NewFD && NewFD->hasBody() && !OrigFDHasBody;
+}
   }
 
 private:
@@ -85,26 +88,37 @@
 
 class CTUAction : public clang::ASTFrontendAction {
 public:
-  CTUAction(bool *Success) : Success(Success) {}
+  CTUAction(bool *Success, unsigned OverrideLimit)
+  : Success(Success), OverrideLimit(OverrideLimit) {}
 
 protected:
   std::unique_ptr
   CreateASTConsumer(clang::CompilerInstance &CI, StringRef) override {
+CI.getAnalyzerOpts()->CTUImportThreshold = OverrideLimit;
 return llvm::make_unique(CI, Success);
   }
 
 private:
   bool *Success;
+  const unsigned OverrideLimit;
 };
 
 } // end namespace
 
 TEST(CrossTranslationUnit, CanLoadFunctionDefinition) {
   bool Success = false;
-  EXPECT_TRUE(tooling::runToolOnCode(new CTUAction(&Success), "int f(int);"));
+  EXPECT_TRUE(
+  tooling::runToolOnCode(new CTUAction(&Success, 1u), "int f(int);"));
   EXPECT_TRUE(Success);
 }
 
+TEST(CrossTranslationUnit, RespectsLoadThreshold) {
+  bool Success = false;
+  EXPECT_TRUE(
+  tooling::runToolOnCode(new CTUAction(&Success, 0u), "int f(int);"));
+  EXPECT_FALSE(Success);
+}
+
 TEST(CrossTranslationUnit, IndexFormatCanBeParsed) {
   llvm::StringMap Index;
   Index["a"] = "/b/f1";
Index: test/Analysis/ctu-import-threshold.c
===
--- /dev/null
+++ test/Analysis/ctu-import-threshold.c
@@ -0,0 +1,5 @@
+// Ensure analyzer option 'ctu-import-threshold' is a recognized option.
+//
+// RUN: %clang_cc1 -analyze -analyzer-config ctu-import-threshold=30 -verify %s
+//
+// expected-no-diagnostics
Index: test/Analysis/analyzer-config.c
===
--- test/Analysis/analyzer-config.c
+++ test/Analysis/analyzer-config.c
@@ -27,6 +27,7 @@
 // CHECK-NEXT: cplusplus.Move:WarnOn = KnownsAndLocals
 // CHECK-NEXT: crosscheck-with-z3 = false
 // CHECK-NEXT: ctu-dir = ""
+// CHECK-NEXT: ctu-import-threshold = 100
 // CHECK-NEXT: ctu-index-name = externalDefMap.txt
 // CHECK-NEXT: debug.AnalysisOrder:* = false
 // CHECK-NEXT: debug.AnalysisOrder:Bind = false
@@ -88,4 +89,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 85
+// CHECK-NEXT: num-entries = 86
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -43,6 +43,8 @@
 STATISTIC(NumTripleMismatch, "The # of triple mismatches");
 STATISTIC(NumLangMismatch, "The # of language mismatches");
 STATISTIC(NumLangDialectMismatch, "The # of language dialect mismatches");
+STATISTIC(NumASTLoadThresholdReached,
+  "The # of ASTs not loaded because of threshold");
 
 // Same as Triple's equality operator, but we check a field only if that is
 // known in both instances.
@@ -102,6 +104,8 @@
   return "Language mismatch";
 case index_error_code::lang_dialect_mismatch:
   return "Language dialect mismatch";
+case index_error_code::load_threshold_reached:
+  return "Load threshold reached";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -180,7 +184,8 @@
 }
 
 CrossTranslationUnitContext::CrossTranslationUnitContext(CompilerInstance &CI)
-: CI(CI), Con

[PATCH] D64120: Rebase onto current master

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 created this revision.
Herald added subscribers: cfe-commits, Szelethus, dkrupp.
Herald added a reviewer: martong.
Herald added a project: clang.

Repository:
  rC Clang

https://reviews.llvm.org/D64120

Files:
  include/clang/CrossTU/CrossTranslationUnit.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  lib/CrossTU/CrossTranslationUnit.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/ctu-import-threshold.c
  unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
@@ -70,12 +71,14 @@
 EXPECT_TRUE(llvm::sys::fs::exists(ASTFileName));
 
 // Load the definition from the AST file.
-llvm::Expected NewFDorError =
-CTU.getCrossTUDefinition(FD, "", IndexFileName);
-EXPECT_TRUE((bool)NewFDorError);
-const FunctionDecl *NewFD = *NewFDorError;
+llvm::Expected NewFDorError = handleExpected(
+CTU.getCrossTUDefinition(FD, "", IndexFileName, false),
+[]() { return nullptr; }, [](IndexError &) {});
 
-*Success = NewFD && NewFD->hasBody() && !OrigFDHasBody;
+if (NewFDorError) {
+  const FunctionDecl *NewFD = *NewFDorError;
+  *Success = NewFD && NewFD->hasBody() && !OrigFDHasBody;
+}
   }
 
 private:
@@ -85,26 +88,37 @@
 
 class CTUAction : public clang::ASTFrontendAction {
 public:
-  CTUAction(bool *Success) : Success(Success) {}
+  CTUAction(bool *Success, unsigned OverrideLimit)
+  : Success(Success), OverrideLimit(OverrideLimit) {}
 
 protected:
   std::unique_ptr
   CreateASTConsumer(clang::CompilerInstance &CI, StringRef) override {
+CI.getAnalyzerOpts()->CTUImportThreshold = OverrideLimit;
 return llvm::make_unique(CI, Success);
   }
 
 private:
   bool *Success;
+  const unsigned OverrideLimit;
 };
 
 } // end namespace
 
 TEST(CrossTranslationUnit, CanLoadFunctionDefinition) {
   bool Success = false;
-  EXPECT_TRUE(tooling::runToolOnCode(new CTUAction(&Success), "int f(int);"));
+  EXPECT_TRUE(
+  tooling::runToolOnCode(new CTUAction(&Success, 1u), "int f(int);"));
   EXPECT_TRUE(Success);
 }
 
+TEST(CrossTranslationUnit, RespectsLoadThreshold) {
+  bool Success = false;
+  EXPECT_TRUE(
+  tooling::runToolOnCode(new CTUAction(&Success, 0u), "int f(int);"));
+  EXPECT_FALSE(Success);
+}
+
 TEST(CrossTranslationUnit, IndexFormatCanBeParsed) {
   llvm::StringMap Index;
   Index["a"] = "/b/f1";
Index: test/Analysis/ctu-import-threshold.c
===
--- /dev/null
+++ test/Analysis/ctu-import-threshold.c
@@ -0,0 +1,5 @@
+// Ensure analyzer option 'ctu-import-threshold' is a recognized option.
+//
+// RUN: %clang_cc1 -analyze -analyzer-config ctu-import-threshold=30 -verify %s
+//
+// expected-no-diagnostics
Index: test/Analysis/analyzer-config.c
===
--- test/Analysis/analyzer-config.c
+++ test/Analysis/analyzer-config.c
@@ -27,6 +27,7 @@
 // CHECK-NEXT: cplusplus.Move:WarnOn = KnownsAndLocals
 // CHECK-NEXT: crosscheck-with-z3 = false
 // CHECK-NEXT: ctu-dir = ""
+// CHECK-NEXT: ctu-import-threshold = 100
 // CHECK-NEXT: ctu-index-name = externalDefMap.txt
 // CHECK-NEXT: debug.AnalysisOrder:* = false
 // CHECK-NEXT: debug.AnalysisOrder:Bind = false
@@ -88,4 +89,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 85
+// CHECK-NEXT: num-entries = 86
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -43,6 +43,8 @@
 STATISTIC(NumTripleMismatch, "The # of triple mismatches");
 STATISTIC(NumLangMismatch, "The # of language mismatches");
 STATISTIC(NumLangDialectMismatch, "The # of language dialect mismatches");
+STATISTIC(NumASTLoadThresholdReached,
+  "The # of ASTs not loaded because of threshold");
 
 // Same as Triple's equality operator, but we check a field only if that is
 // known in both instances.
@@ -102,6 +104,8 @@
   return "Language mismatch";
 case index_error_code::lang_dialect_mismatch:
   return "Language dialect mismatch";
+case index_error_code::load_threshold_reached:
+  return "Load threshold reached";
 }
 llvm_unreachable("Unrecognized index_error_code.");
   }
@@ -180,7 +184,8 @@
 }
 
 CrossTranslationUnitContext::CrossTranslationUnitContext(CompilerInstance &CI)
-: CI(CI), Context(CI.getAST

[PATCH] D64018: [clangd] Store hash of command line in index shards.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/index/Serialization.cpp:531
+for (llvm::StringRef C : Cmd.CommandLine)
+  Result.Cmd->CommandLine.emplace_back(C.str());
+  }

nit: push_back, or emplace_back(C)



Comment at: clang-tools-extra/clangd/index/Serialization.cpp:590
 
+  InternedCompileCommand Cmd;
+  if (Data.Cmd) {

nit: internedcmd to avoid confusion below?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64018



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

[PATCH] D64067: [X86][PPC] Support -mlong-double-64

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 207708.
MaskRay added a comment.

Check -malign-double


Repository:
  rC Clang

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

https://reviews.llvm.org/D64067

Files:
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  lib/Basic/TargetInfo.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/long-double-x86.c
  test/CodeGen/ppc64-align-long-double.c
  test/CodeGen/ppc64-long-double.c
  test/CodeGen/x86-long-double.c
  test/Driver/mlong-double-64.c

Index: test/Driver/mlong-double-64.c
===
--- /dev/null
+++ test/Driver/mlong-double-64.c
@@ -0,0 +1,11 @@
+// RUN: %clang -target powerpc-linux-musl -c -### %s -mlong-double-64 2>&1 | FileCheck %s
+// RUN: %clang -target powerpc64-pc-freebsd12 -c -### %s -mlong-double-64 2>&1 | FileCheck %s
+// RUN: %clang -target powerpc64le-linux-musl -c -### %s -mlong-double-64 2>&1 | FileCheck %s
+// RUN: %clang -target i686-linux-gnu -c -### %s -mlong-double-64 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-musl -c -### %s -mlong-double-64 2>&1 | FileCheck %s
+
+// CHECK: "-mlong-double-64"
+
+// RUN: %clang -target aarch64 -c -### %s -mlong-double-64 2>&1 | FileCheck --check-prefix=ERR %s
+
+// ERR: error: unsupported option '-mlong-double-64' for target 'aarch64'
Index: test/CodeGen/x86-long-double.c
===
--- /dev/null
+++ test/CodeGen/x86-long-double.c
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686-apple-darwin9 -mlong-double-64 | \
+// RUN:   FileCheck --check-prefix=FP64-X32 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686-apple-darwin9 | \
+// RUN:   FileCheck --check-prefix=FP80 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-linux-musl -mlong-double-64 | \
+// RUN:   FileCheck --check-prefix=FP64-X64 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-linux-musl | \
+// RUN:   FileCheck --check-prefix=FP80 %s
+
+// Check -malign-double increases the alignment from 4 to 8 on x86-32.
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686-linux-musl -mlong-double-64 \
+// RUN:   -malign-double | FileCheck --check-prefix=FP64-X64 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-linux-musl -mlong-double-64 \
+// RUN:   -malign-double | FileCheck --check-prefix=FP64-X64 %s
+
+long double x = 0;
+int size = sizeof(x);
+
+// FP64-X32: @x = global double {{.*}}, align 4
+// FP64-X32: @size = global i32 8
+// FP64-X64: @x = global double {{.*}}, align 8
+// FP64-X64: @size = global i32 8
+// FP80: @x = global x86_fp80 {{.*}}, align 16
+// FP80: @size = global i32 16
Index: test/CodeGen/ppc64-long-double.c
===
--- /dev/null
+++ test/CodeGen/ppc64-long-double.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple powerpc64-linux-musl -emit-llvm -o - %s -mlong-double-64 | \
+// RUN:   FileCheck --check-prefix=FP64 %s
+// RUN: %clang_cc1 -triple powerpc64-linux-musl -emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefix=IBM128 %s
+
+struct S {
+  char a;
+  long double b;
+};
+
+// FP64: %struct.{{[a-zA-Z0-9]+}} = type { i8, double }
+// IBM128: %struct.{{[a-zA-Z0-9]+}} = type { i8, ppc_fp128 }
+
+long double load(struct S x) {
+  return x.b;
+}
+
+// FP64: %{{[0-9]}} = load double, double* %{{[a-zA-Z0-9]+}}, align 8
+// IBM128: %{{[0-9]}} = load ppc_fp128, ppc_fp128* %{{[a-zA-Z0-9]+}}, align 16
Index: test/CodeGen/ppc64-align-long-double.c
===
--- test/CodeGen/ppc64-align-long-double.c
+++ /dev/null
@@ -1,16 +0,0 @@
-// REQUIRES: powerpc-registered-target
-// RUN: %clang_cc1 -triple powerpc64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s
-
-struct S {
-  double a;
-  long double b;
-};
-
-// CHECK: %struct.{{[a-zA-Z0-9]+}} = type { double, ppc_fp128 }
-
-long double test (struct S x)
-{
-  return x.b;
-}
-
-// CHECK: %{{[0-9]}} = load ppc_fp128, ppc_fp128* %{{[a-zA-Z0-9]+}}, align 16
Index: test/CodeGen/long-double-x86.c
===
--- test/CodeGen/long-double-x86.c
+++ /dev/null
@@ -1,4 +0,0 @@
-// RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686-apple-darwin9 | grep x86_fp80
-
-long double x = 0;
-int checksize[sizeof(x) == 16 ? 1 : -1];
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2738,6 +2738,7 @@
   Opts.PackStruct = getLastArgIntValue(Args, OPT_fpack_struct_EQ, 0, Diags);
   Opts.MaxTypeAlign = getLastArgIntValue(Args, OPT_fmax_type_align_EQ, 0, Diags);
   Opts.AlignDouble = Args.hasArg(OPT_malign_double);
+  Opts.LongDoubleSize = Args.hasArg(OPT_mlong_double_64) ? 64 : 0;
   Opts.PICLevel = getLastArgIntValue(Args, OPT_pic_level, 0, Diags);
   Op

[PATCH] D64119: [PowerPC] Support constraint code "ww"

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: awilfox, echristo, hfinkel, jsji, kbarton, nemanjai.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.

"ww" and "ws" are both constraint codes for VSX vector registers that
holds scalar double data. "ww" is preferred for float while "ws" is
preferred for double.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64119

Files:
  clang/lib/Basic/Targets/PPC.h
  clang/test/CodeGen/ppc64-inline-asm.c
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/test/CodeGen/PowerPC/inlineasm-vsx-reg.ll
  llvm/test/CodeGen/PowerPC/vec-asm-disabled.ll

Index: llvm/test/CodeGen/PowerPC/vec-asm-disabled.ll
===
--- llvm/test/CodeGen/PowerPC/vec-asm-disabled.ll
+++ llvm/test/CodeGen/PowerPC/vec-asm-disabled.ll
@@ -19,5 +19,17 @@
 ; CHECK: error: couldn't allocate output register for constraint 'wi'
 }
 
+define float @test_ww(float %x, float %y) #0 {
+  %1 = tail call float asm "xsmaxdp ${0:x},${1:x},${2:x}", "=^ww,^ww,^ww"(float %x, float %y) #0
+  ret float %1
+; CHECK: error: couldn't allocate output register for constraint 'ww'
+}
+
+define double @test_ws(double %x, double %y) #0 {
+  %1 = tail call double asm "xsmaxdp ${0:x},${1:x},${2:x}", "=^ws,^ws,^ws"(double %x, double %y) #0
+  ret double %1
+; CHECK: error: couldn't allocate output register for constraint 'ws'
+}
+
 attributes #0 = { nounwind "target-features"="-vsx" }
 
Index: llvm/test/CodeGen/PowerPC/inlineasm-vsx-reg.ll
===
--- llvm/test/CodeGen/PowerPC/inlineasm-vsx-reg.ll
+++ llvm/test/CodeGen/PowerPC/inlineasm-vsx-reg.ll
@@ -38,3 +38,12 @@
 ; CHECK: mtvsrd v2, r1
 ; CHECK: #NO_APP
 }
+
+define float @test_ww(float %x, float %y) {
+  %1 = tail call float asm "xsmaxdp ${0:x}, ${1:x}, ${2:x}", "=^ww,^ww,^ww"(float %x, float %y)
+  ret float %1
+; CHECK-LABEL: test_ww:
+; CHECK: #APP
+; CHECK: xsmaxdp f1, f1, f2
+; CHECK: #NO_APP
+}
Index: llvm/lib/Target/PowerPC/PPCISelLowering.cpp
===
--- llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -13958,7 +13958,7 @@
 return C_RegisterClass;
   } else if (Constraint == "wa" || Constraint == "wd" ||
  Constraint == "wf" || Constraint == "ws" ||
- Constraint == "wi") {
+ Constraint == "wi" || Constraint == "ww") {
 return C_RegisterClass; // VSX registers.
   }
   return TargetLowering::getConstraintType(Constraint);
@@ -13986,10 +13986,12 @@
 StringRef(constraint) == "wf") &&
type->isVectorTy())
 return CW_Register;
-  else if (StringRef(constraint) == "ws" && type->isDoubleTy())
-return CW_Register;
   else if (StringRef(constraint) == "wi" && type->isIntegerTy(64))
 return CW_Register; // just hold 64-bit integers data.
+  else if (StringRef(constraint) == "ws" && type->isDoubleTy())
+return CW_Register;
+  else if (StringRef(constraint) == "ww" && type->isFloatTy())
+return CW_Register;
 
   switch (*constraint) {
   default:
@@ -14075,7 +14077,7 @@
  Constraint == "wf" || Constraint == "wi") &&
  Subtarget.hasVSX()) {
 return std::make_pair(0U, &PPC::VSRCRegClass);
-  } else if (Constraint == "ws" && Subtarget.hasVSX()) {
+  } else if ((Constraint == "ws" || Constraint == "ww") && Subtarget.hasVSX()) {
 if (VT == MVT::f32 && Subtarget.hasP8Vector())
   return std::make_pair(0U, &PPC::VSSRCRegClass);
 else
Index: clang/test/CodeGen/ppc64-inline-asm.c
===
--- clang/test/CodeGen/ppc64-inline-asm.c
+++ clang/test/CodeGen/ppc64-inline-asm.c
@@ -24,3 +24,16 @@
 // CHECK: call i8 asm "crand $0, $1, $2", "=^wc,^wc,^wc"(i8 %b1, i8 %b2)
 }
 
+float test_fmaxf(float x, float y) {
+  asm("xsmaxdp %x0, %x1, %x2" : "=ww"(x) : "ww"(x), "ww"(y));
+  return x;
+// CHECK-LABEL: float @test_fmaxf(float %x, float %y)
+// CHECK: call float asm "xsmaxdp ${0:x}, ${1:x}, ${2:x}", "=^ww,^ww,^ww"(float %x, float %y)
+}
+
+double test_fmax(double x, double y) {
+  asm("xsmaxdp %x0, %x1, %x2" : "=ws"(x) : "ws"(x), "ws"(y));
+  return x;
+// CHECK-LABEL: double @test_fmax(double %x, double %y)
+// CHECK: call double asm "xsmaxdp ${0:x}, ${1:x}, ${2:x}", "=^ws,^ws,^ws"(double %x, double %y)
+}
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -207,7 +207,8 @@
   switch (Name[1]) {
   case 'd': // VSX vector register to hold vector double data
   case 'f': // VSX vector register to hold vector float data
-  case 's': // VSX vector register to hold scalar float data
+  case 's': // VSX vector register to hold scalar double data
+  case 'w': // VSX vector re

[PATCH] D64067: [X86][PPC] Support -mlong-double-64

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

Can you please add a test ensuring that -malign-double and -mlong-double-64 
interact properly? I think that, in the current patch, they do (as 
-malign-double is processed first), but I'd prefer that we cover that case 
explicitly.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64067



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

[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

@rsmith Just ping, could you take a look at the clang change? If possible, 
could you share your opinion? Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809



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

[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song updated this revision to Diff 207704.
yonghong-song added a comment.

handle unnamed bitfield properly. these struct/union members are not encoded in 
debuginfo, so skip them during preserve_*_access_index IR intrinsics generation.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809

Files:
  docs/LanguageExtensions.rst
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuilder.h
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Sema/SemaChecking.cpp
  test/CodeGen/bpf-preserve-access-index-2.c
  test/CodeGen/bpf-preserve-access-index.c

Index: test/CodeGen/bpf-preserve-access-index.c
===
--- /dev/null
+++ test/CodeGen/bpf-preserve-access-index.c
@@ -0,0 +1,27 @@
+// RUN: %clang %s -target bpfeb -x c -emit-llvm -S -g -O2 -o - | FileCheck --check-prefix=CHECK %s
+// RUN: %clang %s -target bpfel -x c -emit-llvm -S -g -O2 -o - | FileCheck --check-prefix=CHECK %s
+
+struct t {
+  int i:1;
+  int j:2;
+  int :3;
+  union {
+   int a;
+   int :32;
+   int b;
+  } c[4];
+};
+
+#define _(x) (__builtin_preserve_access_index(x))
+
+const void *test(struct t *arg) {
+  return _(&arg->c[3].b);
+}
+
+// CHECK: llvm.preserve.struct.access.index
+// CHECK: (%struct.t* %0, i32 1, i32 2)
+// CHECK: llvm.preserve.array.access.index
+// CHECK: ([4 x %union.anon]* %2, i32 1, i32 3)
+// CHECK: llvm.preserve.union.access.index
+// CHECK: (%union.anon* %3, i32 1)
+// CHECK-NOT: __builtin_preserve_access_index
Index: test/CodeGen/bpf-preserve-access-index-2.c
===
--- /dev/null
+++ test/CodeGen/bpf-preserve-access-index-2.c
@@ -0,0 +1,24 @@
+// RUN: %clang %s -target bpfeb -x c -emit-llvm -S -g -O2 -o - | FileCheck %s
+// RUN: %clang %s -target bpfel -x c -emit-llvm -S -g -O2 -o - | FileCheck %s
+
+struct t {
+  int i:1;
+  int j:2;
+  int :3;
+  union {
+   int a;
+   int :32;
+   int b;
+  } c[4];
+};
+
+#define _(x) (x)
+
+const void *test(struct t *arg) {
+  return _(&arg->c[3].b);
+}
+
+// CHECK-NOT: llvm.preserve.struct.access.index
+// CHECK-NOT: llvm.preserve.array.access.index
+// CHECK-NOT: llvm.preserve.union.access.index
+// CHECK-NOT: __builtin_preserve_access_index
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -191,6 +191,16 @@
   return false;
 }
 
+/// Check the number of arguments, and set the result type to
+/// the argument type.
+static bool SemaBuiltinPreserveAI(Sema &S, CallExpr *TheCall) {
+  if (checkArgCount(S, TheCall, 1))
+return true;
+
+  TheCall->setType(TheCall->getArg(0)->getType());
+  return false;
+}
+
 static bool SemaBuiltinOverflow(Sema &S, CallExpr *TheCall) {
   if (checkArgCount(S, TheCall, 3))
 return true;
@@ -1409,6 +1419,10 @@
 TheCall->setType(Context.IntTy);
 break;
   }
+  case Builtin::BI__builtin_preserve_access_index:
+if (SemaBuiltinPreserveAI(*this, TheCall))
+  return ExprError();
+break;
   case Builtin::BI__builtin_call_with_static_chain:
 if (SemaBuiltinCallWithStaticChain(*this, TheCall))
   return ExprError();
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -480,6 +480,10 @@
   /// finally block or filter expression.
   bool IsOutlinedSEHHelper = false;
 
+  /// True if CodeGen currently emits code inside presereved access index
+  /// region.
+  bool IsInPreservedAIRegion = false;
+
   const CodeGen::CGBlockInfo *BlockInfo = nullptr;
   llvm::Value *BlockPointer = nullptr;
 
@@ -2648,6 +2652,9 @@
   /// Converts Location to a DebugLoc, if debug information is enabled.
   llvm::DebugLoc SourceLocToDebugLoc(SourceLocation Location);
 
+  /// Get the record field index as represented in debug info.
+  unsigned getDebugInfoFIndex(const RecordDecl *Rec, unsigned FieldIndex);
+
 
   //======//
   //Declaration Emission
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -25,6 +25,7 @@
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/NSAPI.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringExtras.h"
@@ -3418,8 +3419,20 @@
   CharUnits eltAlign =
 getArrayElementAlign(addr.getAlignment(), indices.back(), eltSize);
 
-  llvm::Value *eltPtr = emitArraySubscriptGEP(
-  CGF, addr.getPointer(), indices, inbounds, signedIndices, loc, name);
+  llvm::Value *eltPtr;
+  auto LastIndex = dyn_cast(indices.back());
+  if (!CGF.IsInPreservedAIReg

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:210
   ///the default tag for the checker will be used.
+  /// @param IsPrunable Whether the note is prunable.
   ExplodedNode *

It makes perfectly no sense here. Is it the mentioned "class doc"?


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

https://reviews.llvm.org/D63915



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

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:98-100
+// The APIModeling package is for checkers that model APIs. These checkers are
+// always turned on; this package is intended for API modeling that is not
+// controlled by the target triple.

Szelethus wrote:
> This isn't true: the user may decide to only enable non-pathsensitive 
> checkers.
> 
> I think the comment should rather state that these whether these checkers are 
> enabled shouldn't be explicitly specified, unless in **extreme** 
> circumstances (causes a crash in a release build?).
Well, I have removed it instead. Makes no sense, you are right.



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:145
+
+  // Get the concrete return value.
+  Optional RawConcreteValue = CDM.lookup(*Call);

NoQ wrote:
> NoQ wrote:
> > I think you can squeeze all this code into one big `isInvariantBreak(Call, 
> > ReturnV)` and re-use it across both methods.
> I think it would be more precise to talk about "expected"/"actual" return 
> value. The actual return value may be either concrete (i.e., 
> `nonloc::ConcreteInt`) or symbolic (i.e., `nonloc::SymbolVal`).
> 
> Also, there's a relatively famous rule of a thumb for making good comments: 
> "comments shouldn't explain what does the code do, but rather why does it do 
> it". Instead of these comments i'd rather have one large comment at the 
> beginning of `checkEndFunction` explaining what are we trying to do here.
Great advice, thanks you!


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

https://reviews.llvm.org/D63915



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

[PATCH] D64015: [WIP][CUDA] Use shared MangleContext for CUDA and CXX CG

[Heejin Ahn via Phabricator via cfe-commits]

aheejin resigned from this revision.
aheejin added a comment.

Sorry, I don't think I know enough about this code to review this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64015



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

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 207703.
Charusso marked 10 inline comments as done.
Charusso added a comment.

- Fix.
- Refactor.


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

https://reviews.llvm.org/D63915

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
  clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  clang/test/Analysis/return-value-guaranteed.cpp
  clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

Index: clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
===
--- clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
+++ clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
@@ -33,11 +33,11 @@
 Impl(std::move(Data)) {}
 
   const bool *lookup(const CallEvent &Call) {
-const bool *Result = Impl.lookup(Call);
+Optional Result = Impl.lookup(Call);
 // If it's a function we expected to find, remember that we've found it.
-if (Result && *Result)
+if (Result && *Result && **Result)
   ++Found;
-return Result;
+return *Result;
   }
 
   // Fail the test if we haven't found all the true-calls we were looking for.
Index: clang/test/Analysis/return-value-guaranteed.cpp
===
--- /dev/null
+++ clang/test/Analysis/return-value-guaranteed.cpp
@@ -0,0 +1,99 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,apiModeling.ReturnValue \
+// RUN:  -analyzer-output=text -verify=class %s
+
+struct Foo { int Field; };
+bool problem();
+void doSomething();
+
+// We predefined the return value of 'MCAsmParser::Error' as true and we cannot
+// take the false-branches which leads to a "garbage value" false positive.
+namespace test_classes {
+struct MCAsmParser {
+  static bool Error();
+};
+
+bool parseFoo(Foo &F) {
+  if (problem()) {
+// class-note@-1 {{Assuming the condition is false}}
+// class-note@-2 {{Taking false branch}}
+return MCAsmParser::Error();
+  }
+
+  F.Field = 0;
+  // class-note@-1 {{The value 0 is assigned to 'F.Field'}}
+  return !MCAsmParser::Error();
+  // class-note@-1 {{'MCAsmParser::Error' returns true}}
+}
+
+bool parseFile() {
+  Foo F;
+  if (parseFoo(F)) {
+// class-note@-1 {{Calling 'parseFoo'}}
+// class-note@-2 {{Returning from 'parseFoo'}}
+// class-note@-3 {{Taking false branch}}
+return true;
+  }
+
+  if (F.Field == 0) {
+// class-note@-1 {{Field 'Field' is equal to 0}}
+// class-note@-2 {{Taking true branch}}
+
+// no-warning: "The left operand of '==' is a garbage value" was here.
+doSomething();
+  }
+
+  (void)(1 / F.Field);
+  // class-warning@-1 {{Division by zero}}
+  // class-note@-2 {{Division by zero}}
+  return false;
+}
+} // namespace test_classes
+
+
+// We predefined 'MCAsmParser::Error' as returning true, but now it returns
+// false, which breaks our invariant. Test the notes.
+namespace test_break {
+struct MCAsmParser {
+  static bool Error() {
+return false; // class-note {{'MCAsmParser::Error' returns false}}
+  }
+};
+
+bool parseFoo(Foo &F) {
+  if (problem()) {
+// class-note@-1 {{Assuming the condition is false}}
+// class-note@-2 {{Taking false branch}}
+return !MCAsmParser::Error();
+  }
+
+  F.Field = 0;
+  // class-note@-1 {{The value 0 is assigned to 'F.Field'}}
+  return MCAsmParser::Error();
+  // class-note@-1 {{Calling 'MCAsmParser::Error'}}
+  // class-note@-2 {{Returning from 'MCAsmParser::Error'}}
+}
+
+bool parseFile() {
+  Foo F;
+  if (parseFoo(F)) {
+// class-note@-1 {{Calling 'parseFoo'}}
+// class-note@-2 {{Returning from 'parseFoo'}}
+// class-note@-3 {{Taking false branch}}
+return true;
+  }
+
+  if (F.Field == 0) {
+// class-note@-1 {{Field 'Field' is equal to 0}}
+// class-note@-2 {{Taking true branch}}
+
+// no-warning: "The left operand of '==' is a garbage value" was here.
+doSomething();
+  }
+
+  (void)(1 / F.Field);
+  // class-warning@-1 {{Division by zero}}
+  // class-note@-2 {{Division by zero}}
+  return false;
+}
+} // namespace test_classes
Index: clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -780,6 +780,9 @@
   NewAllocElt->getAllocatorExpr()->getBeginLoc(), SMng);
 }
 llvm_unreachable("Unexpected CFG element at front of block");
+  } else if (Optional FE = P.getAs()) {
+return PathDiagnosticLocation(FE->getStmt(), SMng,
+  FE->getLocationContext());
   } else {
 llvm_u

[PATCH] D62888: [NewPM] Port Sancov

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp:750
   Constant::getNullValue(ArrayTy), "__sancov_gen_");
+  appendToCompilerUsed(*CurModule, {Array});
 

chandlerc wrote:
> There is a `GlobalsToAppendToCompilerUsed` vector that we append `Array` to 
> below (line 759). Why isn't that sufficient instead of this?
Ah, I didn't actually see this for some reason. The reason why it wasn't 
working before was bc I left the code that called 

```
  if (TargetTriple.isOSBinFormatMachO())
appendToUsed(M, GlobalsToAppendToUsed);
  appendToCompilerUsed(M, GlobalsToAppendToCompilerUsed);
```

in `initializeModule`, so this never got appended. Fixed by moving this to a 
`finalizeModule` which runs after the function runs end.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62888



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

[PATCH] D62888: [NewPM] Port Sancov

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 207698.
leonardchan marked 5 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62888

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/sancov-new-pm.c
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/Transforms/Instrumentation.h
  llvm/include/llvm/Transforms/Instrumentation/SanitizerCoverage.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Transforms/Instrumentation/Instrumentation.cpp
  llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp
  llvm/test/Instrumentation/SanitizerCoverage/abort-in-entry-block.ll
  llvm/test/Instrumentation/SanitizerCoverage/backedge-pruning.ll
  llvm/test/Instrumentation/SanitizerCoverage/chains.ll
  llvm/test/Instrumentation/SanitizerCoverage/cmp-tracing-api-x86_32.ll
  llvm/test/Instrumentation/SanitizerCoverage/cmp-tracing-api-x86_64.ll
  llvm/test/Instrumentation/SanitizerCoverage/cmp-tracing.ll
  llvm/test/Instrumentation/SanitizerCoverage/coff-comdat.ll
  
llvm/test/Instrumentation/SanitizerCoverage/coff-pc-table-inline-8bit-counters.ll
  llvm/test/Instrumentation/SanitizerCoverage/coff-used-ctor.ll
  llvm/test/Instrumentation/SanitizerCoverage/const-cmp-tracing.ll
  llvm/test/Instrumentation/SanitizerCoverage/coverage-dbg.ll
  llvm/test/Instrumentation/SanitizerCoverage/coverage.ll
  llvm/test/Instrumentation/SanitizerCoverage/coverage2-dbg.ll
  llvm/test/Instrumentation/SanitizerCoverage/div-tracing.ll
  llvm/test/Instrumentation/SanitizerCoverage/gep-tracing.ll
  llvm/test/Instrumentation/SanitizerCoverage/inline-8bit-counters.ll
  llvm/test/Instrumentation/SanitizerCoverage/interposable-symbol-nocomdat.ll
  llvm/test/Instrumentation/SanitizerCoverage/no-func.ll
  llvm/test/Instrumentation/SanitizerCoverage/pc-table.ll
  llvm/test/Instrumentation/SanitizerCoverage/postdominator_check.ll
  llvm/test/Instrumentation/SanitizerCoverage/seh.ll
  
llvm/test/Instrumentation/SanitizerCoverage/stack-depth-variable-declared-by-user.ll
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
  llvm/test/Instrumentation/SanitizerCoverage/switch-tracing.ll
  llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-comdat.ll
  
llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-inline-8bit-counters.ll
  llvm/test/Instrumentation/SanitizerCoverage/trace-pc-guard-nocomdat.ll
  llvm/test/Instrumentation/SanitizerCoverage/tracing-comdat.ll
  llvm/test/Instrumentation/SanitizerCoverage/tracing.ll
  llvm/test/Instrumentation/SanitizerCoverage/unreachable-critedge.ll
  llvm/test/Instrumentation/SanitizerCoverage/wineh.ll

Index: llvm/test/Instrumentation/SanitizerCoverage/wineh.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/wineh.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/wineh.ll
@@ -1,4 +1,5 @@
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc -S | FileCheck %s --check-prefix=CHECK
+; RUN: opt < %s -passes='module(sancov-module),function(sancov-func)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc -S | FileCheck %s --check-prefix=CHECK
 
 ; Generated from this C++ source:
 ; $ clang -O2 t.cpp -S -emit-llvm
Index: llvm/test/Instrumentation/SanitizerCoverage/unreachable-critedge.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/unreachable-critedge.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/unreachable-critedge.ll
@@ -1,4 +1,5 @@
 ; RUN: opt < %s -S -sancov -sanitizer-coverage-level=3 | FileCheck %s
+; RUN: opt < %s -S -passes='module(sancov-module),function(sancov-func)' -sanitizer-coverage-level=3 | FileCheck %s
 
 ; The critical edges to unreachable_bb should not be split.
 define i32 @foo(i32 %c, i32 %d) {
Index: llvm/test/Instrumentation/SanitizerCoverage/tracing.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/tracing.ll
+++ llvm/test/Instrumentation/SanitizerCoverage/tracing.ll
@@ -3,6 +3,10 @@
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S | FileCheck %s --check-prefix=CHECK_PC_GUARD
 ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S -mtriple=x86_64-apple-macosx | FileCheck %s --check-prefix=CHECK_PC_GUARD_DARWIN
 
+; RUN: opt < %s -passes='module(sancov-module),function(sancov-func)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc  -S | FileCheck %s --check-prefix=CHECK_PC
+; RUN: opt < %s -passes='module(sancov-module),function(sancov-func)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S | FileCheck %s --check-prefix=CHECK_PC_GUARD
+; RUN: opt < %s -passes='module(sancov-module),function(sancov-func)' -sanitizer-coverage-level=3 -sanitizer-coverage-trace-pc-guard  -S -mtriple=x86

[PATCH] D64067: [X86][PPC] Support -mlong-double-64

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 207695.
MaskRay edited the summary of this revision.
MaskRay added a comment.

Implement this in TargetInfo::adjust as rnk suggested


Repository:
  rC Clang

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

https://reviews.llvm.org/D64067

Files:
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  lib/Basic/TargetInfo.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/long-double-x86.c
  test/CodeGen/ppc64-align-long-double.c
  test/CodeGen/ppc64-long-double.c
  test/CodeGen/x86-long-double.c
  test/Driver/mlong-double-64.c

Index: test/Driver/mlong-double-64.c
===
--- /dev/null
+++ test/Driver/mlong-double-64.c
@@ -0,0 +1,11 @@
+// RUN: %clang -target powerpc-linux-musl -c -### %s -mlong-double-64 2>&1 | FileCheck %s
+// RUN: %clang -target powerpc64-pc-freebsd12 -c -### %s -mlong-double-64 2>&1 | FileCheck %s
+// RUN: %clang -target powerpc64le-linux-musl -c -### %s -mlong-double-64 2>&1 | FileCheck %s
+// RUN: %clang -target i686-linux-gnu -c -### %s -mlong-double-64 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-musl -c -### %s -mlong-double-64 2>&1 | FileCheck %s
+
+// CHECK: "-mlong-double-64"
+
+// RUN: %clang -target aarch64 -c -### %s -mlong-double-64 2>&1 | FileCheck --check-prefix=ERR %s
+
+// ERR: error: unsupported option '-mlong-double-64' for target 'aarch64'
Index: test/CodeGen/x86-long-double.c
===
--- /dev/null
+++ test/CodeGen/x86-long-double.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686-apple-darwin9 -mlong-double-64 | \
+// RUN:   FileCheck --check-prefix=FP64-X32 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686-apple-darwin9 | \
+// RUN:   FileCheck --check-prefix=FP80 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-linux-musl -mlong-double-64 | \
+// RUN:   FileCheck --check-prefix=FP64-X64 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-linux-musl | \
+// RUN:   FileCheck --check-prefix=FP80 %s
+
+long double x = 0;
+int size = sizeof(x);
+
+// FP64-X32: @x = global double {{.*}}, align 4
+// FP64-X32: @size = global i32 8
+// FP64-X64: @x = global double {{.*}}, align 8
+// FP64-X64: @size = global i32 8
+// FP80: @x = global x86_fp80 {{.*}}, align 16
+// FP80: @size = global i32 16
Index: test/CodeGen/ppc64-long-double.c
===
--- /dev/null
+++ test/CodeGen/ppc64-long-double.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple powerpc64-linux-musl -emit-llvm -o - %s -mlong-double-64 | \
+// RUN:   FileCheck --check-prefix=FP64 %s
+// RUN: %clang_cc1 -triple powerpc64-linux-musl -emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefix=IBM128 %s
+
+struct S {
+  char a;
+  long double b;
+};
+
+// FP64: %struct.{{[a-zA-Z0-9]+}} = type { i8, double }
+// IBM128: %struct.{{[a-zA-Z0-9]+}} = type { i8, ppc_fp128 }
+
+long double load(struct S x) {
+  return x.b;
+}
+
+// FP64: %{{[0-9]}} = load double, double* %{{[a-zA-Z0-9]+}}, align 8
+// IBM128: %{{[0-9]}} = load ppc_fp128, ppc_fp128* %{{[a-zA-Z0-9]+}}, align 16
Index: test/CodeGen/ppc64-align-long-double.c
===
--- test/CodeGen/ppc64-align-long-double.c
+++ /dev/null
@@ -1,16 +0,0 @@
-// REQUIRES: powerpc-registered-target
-// RUN: %clang_cc1 -triple powerpc64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s
-
-struct S {
-  double a;
-  long double b;
-};
-
-// CHECK: %struct.{{[a-zA-Z0-9]+}} = type { double, ppc_fp128 }
-
-long double test (struct S x)
-{
-  return x.b;
-}
-
-// CHECK: %{{[0-9]}} = load ppc_fp128, ppc_fp128* %{{[a-zA-Z0-9]+}}, align 16
Index: test/CodeGen/long-double-x86.c
===
--- test/CodeGen/long-double-x86.c
+++ /dev/null
@@ -1,4 +0,0 @@
-// RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686-apple-darwin9 | grep x86_fp80
-
-long double x = 0;
-int checksize[sizeof(x) == 16 ? 1 : -1];
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2738,6 +2738,7 @@
   Opts.PackStruct = getLastArgIntValue(Args, OPT_fpack_struct_EQ, 0, Diags);
   Opts.MaxTypeAlign = getLastArgIntValue(Args, OPT_fmax_type_align_EQ, 0, Diags);
   Opts.AlignDouble = Args.hasArg(OPT_malign_double);
+  Opts.LongDoubleSize = Args.hasArg(OPT_mlong_double_64) ? 64 : 0;
   Opts.PICLevel = getLastArgIntValue(Args, OPT_pic_level, 0, Diags);
   Opts.ROPI = Args.hasArg(OPT_fropi);
   Opts.RWPI = Args.hasArg(OPT_frwpi);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4001,6 +4001,17 @@
 
   RenderFloati

[PATCH] D64100: [analyzer] exploded-graph-rewriter: Implement checker messages.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D64100#1567715 , @Charusso wrote:

> Sometimes I was lost in the amount of your work.


Nah, i literally just pushed it.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64100



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

[PATCH] D64100: [analyzer] exploded-graph-rewriter: Implement checker messages.

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D64100#1567706 , @NoQ wrote:

> In D64100#1567230 , @Charusso wrote:
>
> > Nice! Could you add some `#===---===#` separators, please? As it is in the 
> > finishing state, I think now it is appropriate.
>
>
> rC364991 .


Thanks you! Sometimes I was lost in the amount of your work.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64100



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

[PATCH] D64100: [analyzer] exploded-graph-rewriter: Implement checker messages.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D64100#1567230 , @Charusso wrote:

> Nice! Could you add some `#===---===#` separators, please? As it is in the 
> finishing state, I think now it is appropriate.


rC364991 .


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64100



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

[PATCH] D64110: [analyzer] exploded-graph-rewriter: Implement bug nodes and sink nodes.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364992: [analyzer] exploded-graph-rewriter: Implement bug 
nodes and sink nodes. (authored by dergachev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64110?vs=207687&id=207690#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64110

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
  cfe/trunk/test/Analysis/dump_egraph.c
  cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages.dot
  cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages_diff.dot
  cfe/trunk/test/Analysis/exploded-graph-rewriter/constraints.dot
  cfe/trunk/test/Analysis/exploded-graph-rewriter/constraints_diff.dot
  cfe/trunk/test/Analysis/exploded-graph-rewriter/edge.dot
  cfe/trunk/test/Analysis/exploded-graph-rewriter/environment.dot
  cfe/trunk/test/Analysis/exploded-graph-rewriter/environment_diff.dot
  cfe/trunk/test/Analysis/exploded-graph-rewriter/node_labels.dot
  cfe/trunk/test/Analysis/exploded-graph-rewriter/program_points.dot
  cfe/trunk/test/Analysis/exploded-graph-rewriter/store.dot
  cfe/trunk/test/Analysis/exploded-graph-rewriter/store_diff.dot
  cfe/trunk/utils/analyzer/exploded-graph-rewriter.py

Index: cfe/trunk/test/Analysis/exploded-graph-rewriter/store_diff.dot
===
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/store_diff.dot
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/store_diff.dot
@@ -7,6 +7,8 @@
  "{
 { "node_id": 1,
   "pointer": "0x1",
+  "has_report": false,
+  "is_sink": false,
   "state_id": 2,
   "program_points": [],
   "program_state": {
@@ -55,6 +57,8 @@
  "{
 { "node_id": 4,
   "pointer": "0x4",
+  "has_report": false,
+  "is_sink": false,
   "state_id": 5,
   "program_points": [],
   "program_state": {
@@ -89,6 +93,8 @@
  "{
 { "node_id": 6,
   "pointer": "0x6",
+  "has_report": false,
+  "is_sink": false,
   "state_id": 7,
   "program_points": [],
   "program_state": null
Index: cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages_diff.dot
===
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages_diff.dot
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages_diff.dot
@@ -7,6 +7,8 @@
  "{
 { "node_id": 1,
   "pointer": "0x1",
+  "has_report": false,
+  "is_sink": false,
   "state_id": 2,
   "program_points": [],
   "program_state": {
@@ -59,6 +61,8 @@
  "{
 { "node_id": 4,
   "pointer": "0x4",
+  "has_report": false,
+  "is_sink": false,
   "state_id": 5,
   "program_points": [],
   "program_state": {
@@ -86,6 +90,8 @@
  "{
 { "node_id": 6,
   "pointer": "0x6",
+  "has_report": false,
+  "is_sink": false,
   "state_id": 7,
   "program_points": [],
   "program_state": null
Index: cfe/trunk/test/Analysis/exploded-graph-rewriter/node_labels.dot
===
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/node_labels.dot
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/node_labels.dot
@@ -15,7 +15,22 @@
 // CHECK-SAME:   
 Node0x1 [shape=record,label=
  "{
-{ "node_id": 1, "pointer": "0x1",
+{ "node_id": 1, "pointer": "0x1", "has_report": false, "is_sink": false,
+  "program_state": null,
+  "program_points": []
+}
+\l}"];
+
+// CHECK: Node0x2 [
+// CHECK-SAME: 
+// CHECK-SAME:   Bug Report Attached
+// CHECK-SAME: 
+// CHECK-SAME: 
+// CHECK-SAME:   Sink Node
+// CHECK-SAME: 
+Node0x2 [shape=record,label=
+ "{
+{ "node_id": 2, "pointer": "0x2", "has_report": true, "is_sink": true,
   "program_state": null,
   "program_points": []
 }
Index: cfe/trunk/test/Analysis/exploded-graph-rewriter/constraints.dot
===
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/constraints.dot
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/constraints.dot
@@ -14,6 +14,8 @@
  "{
 { "node_id": 1,
   "pointer": "0x1",
+  "has_report": false,
+  "is_sink": false,
   "state_id": 2,
   "program_points": [],
   "program_state": {
Index: cfe/trunk/test/Analysis/exploded-graph-rewriter/environment.dot
===
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/environment.dot
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/environment.dot
@@ -29,6 +29,8 @@
  "{
 { "node_id": 1,
   "pointer": "0x1",
+  "has_report": false,
+  "is_sink": false,
   "state_id": 2,
   "program_points": [],
   "program_state": {
Index: cfe/trunk/test/

[PATCH] D64104: [analyzer] exploded-graph-rewriter: Collapse large statement pretty-prints.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364990: [analyzer] exploded-graph-rewriter: Collapse very 
long statement pretty-prints. (authored by dergachev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64104?vs=207631&id=207689#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64104

Files:
  cfe/trunk/test/Analysis/exploded-graph-rewriter/program_points.dot
  cfe/trunk/utils/analyzer/exploded-graph-rewriter.py


Index: cfe/trunk/test/Analysis/exploded-graph-rewriter/program_points.dot
===
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/program_points.dot
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/program_points.dot
@@ -90,3 +90,27 @@
   }
 ]}
 \l}"];
+
+// Test collapsing large pretty prints with braces.
+
+// CHECK-NEXT: Program point:
+// CHECK-SAME: \{ ... \}
+Node0x3 [shape=record,label=
+ "{
+{ "node_id": 3, "pointer": "0x3",
+  "program_state": null, "program_points": [
+  {
+"kind": "Statement",
+"stmt_kind": "CompoundStmt",
+"stmt_point_kind": "PostStmt",
+"stmd_id": 6,
+"pointer": "0x6",
+"pretty": "{ very very very very very very long pretty print }",
+"location": {
+  "line": 7,
+  "column": 8
+},
+"tag": "ExprEngine : Clean Node"
+  }
+]}
+\l}"];
Index: cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
===
--- cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
+++ cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
@@ -398,6 +398,21 @@
 return '+'
 return '-'
 
+@staticmethod
+def _short_pretty(s):
+if s is None:
+return None
+if len(s) < 20:
+return s
+left = s.find('{')
+right = s.rfind('}')
+if left == -1 or right == -1 or left >= right:
+return s
+candidate = s[0:left + 1] + ' ... ' + s[right:]
+if len(candidate) >= len(s):
+return s
+return candidate
+
 def visit_begin_graph(self, graph):
 self._graph = graph
 self._dump_raw('digraph "ExplodedGraph" {\n')
@@ -433,7 +448,8 @@
% (p.loc.filename, p.loc.line,
   p.loc.col, color, p.stmt_kind,
   stmt_color, p.stmt_point_kind,
-  p.pretty if not skip_pretty else ''))
+  self._short_pretty(p.pretty)
+  if not skip_pretty else ''))
 else:
 self._dump(''
'Invalid Source Location:'
@@ -443,7 +459,8 @@
'%s'
% (color, p.stmt_kind,
   stmt_color, p.stmt_point_kind,
-  p.pretty if not skip_pretty else ''))
+  self._short_pretty(p.pretty)
+  if not skip_pretty else ''))
 elif p.kind == 'Edge':
 self._dump(''
''
@@ -496,7 +513,7 @@
   'lavender' if self._dark_mode else 'darkgreen',
   ('(%s)' % b.kind) if b.kind is not None else ' '
   ),
-  b.pretty, f.bindings[b]))
+  self._short_pretty(b.pretty), f.bindings[b]))
 
 frames_updated = e.diff_frames(prev_e) if prev_e is not None else None
 if frames_updated:


Index: cfe/trunk/test/Analysis/exploded-graph-rewriter/program_points.dot
===
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/program_points.dot
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/program_points.dot
@@ -90,3 +90,27 @@
   }
 ]}
 \l}"];
+
+// Test collapsing large pretty prints with braces.
+
+// CHECK-NEXT: Program point:
+// CHECK-SAME: \{ ... \}
+Node0x3 [shape=record,label=
+ "{
+{ "node_id": 3, "pointer": "0x3",
+  "program_state": null, "program_points": [
+  {
+"kind": "Statement",
+"stmt_kind": "CompoundStmt",
+"stmt_point_kind": "PostStmt",
+"stmd_id": 6,
+"pointer": "0x6",
+"pretty": "{ very very very very very very long pretty print }",
+"location": {
+  "line": 7,
+  "column": 8
+},
+"tag": "ExprEngine : Clean Node"
+  }
+]}
+\l}"];
Index: cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
===
--- cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
+++ cfe/trunk/utils/analyzer/exploded-

[PATCH] D64100: [analyzer] exploded-graph-rewriter: Implement checker messages.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364989: [analyzer] exploded-graph-rewriter: Implement 
checker messages. (authored by dergachev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64100?vs=207625&id=207688#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64100

Files:
  cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages.dot
  cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages_diff.dot
  cfe/trunk/test/Analysis/exploded-graph-rewriter/constraints.dot
  cfe/trunk/test/Analysis/exploded-graph-rewriter/constraints_diff.dot
  cfe/trunk/test/Analysis/exploded-graph-rewriter/environment.dot
  cfe/trunk/test/Analysis/exploded-graph-rewriter/environment_diff.dot
  cfe/trunk/test/Analysis/exploded-graph-rewriter/store.dot
  cfe/trunk/test/Analysis/exploded-graph-rewriter/store_diff.dot
  cfe/trunk/utils/analyzer/exploded-graph-rewriter.py

Index: cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
===
--- cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
+++ cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
@@ -13,6 +13,7 @@
 
 import argparse
 import collections
+import difflib
 import json
 import logging
 import re
@@ -211,6 +212,41 @@
 return len(removed) != 0 or len(added) != 0 or len(updated) != 0
 
 
+# Deserialized messages from a single checker in a single program state.
+# Basically a list of raw strings.
+class CheckerLines(object):
+def __init__(self, json_lines):
+super(CheckerLines, self).__init__()
+self.lines = json_lines
+
+def diff_lines(self, prev):
+lines = difflib.ndiff(prev.lines, self.lines)
+return [l.strip() for l in lines
+if l.startswith('+') or l.startswith('-')]
+
+def is_different(self, prev):
+return len(self.diff_lines(prev)) > 0
+
+
+# Deserialized messages of all checkers, separated by checker.
+class CheckerMessages(object):
+def __init__(self, json_m):
+super(CheckerMessages, self).__init__()
+self.items = collections.OrderedDict(
+[(m['checker'], CheckerLines(m['messages'])) for m in json_m])
+
+def diff_messages(self, prev):
+removed = [k for k in prev.items if k not in self.items]
+added = [k for k in self.items if k not in prev.items]
+updated = [k for k in prev.items if k in self.items
+   and prev.items[k].is_different(self.items[k])]
+return (removed, added, updated)
+
+def is_different(self, prev):
+removed, added, updated = self.diff_messages(prev)
+return len(removed) != 0 or len(added) != 0 or len(updated) != 0
+
+
 # A deserialized program state.
 class ProgramState(object):
 def __init__(self, state_id, json_ps):
@@ -241,7 +277,8 @@
 GenericEnvironment(json_ps['constructing_objects']) \
 if json_ps['constructing_objects'] is not None else None
 
-# TODO: Checker messages.
+self.checker_messages = CheckerMessages(json_ps['checker_messages']) \
+if json_ps['checker_messages'] is not None else None
 
 
 # A deserialized exploded graph node. Has a default constructor because it
@@ -595,16 +632,73 @@
 if m is None:
 self._dump(' Nothing!')
 else:
-if prev_s is not None:
-if prev_m is not None:
-if m.is_different(prev_m):
-self._dump('')
-self.visit_generic_map(m, prev_m)
-else:
-self._dump(' No changes!')
-if prev_m is None:
+if prev_m is not None:
+if m.is_different(prev_m):
+self._dump('')
+self.visit_generic_map(m, prev_m)
+else:
+self._dump(' No changes!')
+else:
 self._dump('')
 self.visit_generic_map(m)
+
+self._dump('')
+
+def visit_checker_messages(self, m, prev_m=None):
+self._dump('')
+
+def dump_line(l, is_added=None):
+self._dump('%s'
+   '%s'
+   % (self._diff_plus_minus(is_added), l))
+
+def dump_chk(chk, is_added=None):
+dump_line('%s:' % chk, is_added)
+
+if prev_m is not None:
+removed, added, updated = m.diff_messages(prev_m)
+for chk in removed:
+dump_chk(chk, False)
+for l in prev_m.items[chk].lines:
+dump_line(l, False)
+for chk in updated:
+dump_chk(chk)
+for l in m.items[chk].diff_lines(prev_m.items[chk]):
+dump_line(l[1:], l.startswith('+'))

[PATCH] D64110: [analyzer] exploded-graph-rewriter: Implement bug nodes and sink nodes.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 207687.
NoQ added a comment.

Fix the `dump_egraph.c` test (whoops).


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

https://reviews.llvm.org/D64110

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/Analysis/dump_egraph.c
  clang/test/Analysis/exploded-graph-rewriter/checker_messages.dot
  clang/test/Analysis/exploded-graph-rewriter/checker_messages_diff.dot
  clang/test/Analysis/exploded-graph-rewriter/constraints.dot
  clang/test/Analysis/exploded-graph-rewriter/constraints_diff.dot
  clang/test/Analysis/exploded-graph-rewriter/edge.dot
  clang/test/Analysis/exploded-graph-rewriter/environment.dot
  clang/test/Analysis/exploded-graph-rewriter/environment_diff.dot
  clang/test/Analysis/exploded-graph-rewriter/node_labels.dot
  clang/test/Analysis/exploded-graph-rewriter/program_points.dot
  clang/test/Analysis/exploded-graph-rewriter/store.dot
  clang/test/Analysis/exploded-graph-rewriter/store_diff.dot
  clang/utils/analyzer/exploded-graph-rewriter.py

Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- clang/utils/analyzer/exploded-graph-rewriter.py
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -299,6 +299,8 @@
 logging.debug('Adding ' + node_id)
 self.node_id = json_node['node_id']
 self.ptr = json_node['pointer']
+self.has_report = json_node['has_report']
+self.is_sink = json_node['is_sink']
 self.points = [ProgramPoint(p) for p in json_node['program_points']]
 self.state = ProgramState(json_node['state_id'],
   json_node['program_state']) \
@@ -754,6 +756,12 @@
% ("gray20" if self._dark_mode else "gray",
   node.node_id, node.ptr, node.state.state_id
   if node.state is not None else 'Unspecified'))
+if node.has_report:
+self._dump('Bug Report Attached'
+   '')
+if node.is_sink:
+self._dump('Sink Node'
+   '')
 self._dump('')
 if len(node.points) > 1:
 self._dump('Program points:')
Index: clang/test/Analysis/exploded-graph-rewriter/store_diff.dot
===
--- clang/test/Analysis/exploded-graph-rewriter/store_diff.dot
+++ clang/test/Analysis/exploded-graph-rewriter/store_diff.dot
@@ -7,6 +7,8 @@
  "{
 { "node_id": 1,
   "pointer": "0x1",
+  "has_report": false,
+  "is_sink": false,
   "state_id": 2,
   "program_points": [],
   "program_state": {
@@ -55,6 +57,8 @@
  "{
 { "node_id": 4,
   "pointer": "0x4",
+  "has_report": false,
+  "is_sink": false,
   "state_id": 5,
   "program_points": [],
   "program_state": {
@@ -89,6 +93,8 @@
  "{
 { "node_id": 6,
   "pointer": "0x6",
+  "has_report": false,
+  "is_sink": false,
   "state_id": 7,
   "program_points": [],
   "program_state": null
Index: clang/test/Analysis/exploded-graph-rewriter/store.dot
===
--- clang/test/Analysis/exploded-graph-rewriter/store.dot
+++ clang/test/Analysis/exploded-graph-rewriter/store.dot
@@ -24,6 +24,8 @@
  "{
 { "node_id": 1,
   "pointer": "0x1",
+  "has_report": false,
+  "is_sink": false,
   "state_id": 2,
   "program_points": [],
   "program_state": {
Index: clang/test/Analysis/exploded-graph-rewriter/program_points.dot
===
--- clang/test/Analysis/exploded-graph-rewriter/program_points.dot
+++ clang/test/Analysis/exploded-graph-rewriter/program_points.dot
@@ -28,7 +28,7 @@
 // CHECK-SAME: 
 Node0x1 [shape=record,label=
  "{
-{ "node_id": 1, "pointer": "0x1",
+{ "node_id": 1, "pointer": "0x1", "has_report": false, "is_sink": false,
   "program_state": null, "program_points": [
   {
 "kind": "Edge",
@@ -73,7 +73,7 @@
 // CHECK-SAME: 
 Node0x2 [shape=record,label=
  "{
-{ "node_id": 2, "pointer": "0x2",
+{ "node_id": 2, "pointer": "0x2", "has_report": false, "is_sink": false,
   "program_state": null, "program_points": [
   {
 "kind": "Statement",
@@ -97,7 +97,7 @@
 // CHECK-SAME: \{ ... \}
 Node0x3 [shape=record,label=
  "{
-{ "node_id": 3, "pointer": "0x3",
+{ "node_id": 3, "pointer": "0x3", "has_report": false, "is_sink": false,
   "program_state": null, "program_points": [
   {
 "kind": "Statement",
Index: clang/test/Analysis/exploded-graph-rewriter/node_labels.dot
===
--- clang/test/Analysis/exploded-graph-rewriter/node_labels.dot
+++ clang/test/Analysis/exploded-graph-rewriter/node_labels.dot
@@ -15,7 +15,22 @@
 // CHECK-SAME:   
 Node0x1 [shape=record,label=
  "{
-{ "node_id": 1, "poin

[PATCH] D64110: [analyzer] exploded-graph-rewriter: Implement bug nodes and sink nodes.

[Artem Dergachev via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:3009-3010
   for (const BugReport &Report : EQ) {
-if (Report.getErrorNode()->getState() == N->getState())
+if (Report.getErrorNode()->getState() == N->getState() &&
+Report.getErrorNode()->getLocation() == N->getLocation())
   return true;

The fix in `test/Analysis/dump_egraph.c` is related to this change. Previously 
multiple nodes in the dump were marked as `"has_report": true` and the `CHECK` 
directives were in a wrong order but went unnoticed.


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

https://reviews.llvm.org/D64110



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

r364992 - [analyzer] exploded-graph-rewriter: Implement bug nodes and sink nodes.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Tue Jul  2 18:26:41 2019
New Revision: 364992

URL: http://llvm.org/viewvc/llvm-project?rev=364992&view=rev
Log:
[analyzer] exploded-graph-rewriter: Implement bug nodes and sink nodes.

Add a label to nodes that have a bug report attached or on which
the analysis was generally interrupted.

Fix printing has_report and implement printing is_sink in the graph dumper.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/test/Analysis/dump_egraph.c
cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages_diff.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/constraints.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/constraints_diff.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/edge.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/environment.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/environment_diff.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/node_labels.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/program_points.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/store.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/store_diff.dot
cfe/trunk/utils/analyzer/exploded-graph-rewriter.py

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=364992&r1=364991&r2=364992&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Tue Jul  2 18:26:41 2019
@@ -3006,7 +3006,8 @@ struct DOTGraphTraits :
 
 for (const auto &EQ : EQClasses) {
   for (const BugReport &Report : EQ) {
-if (Report.getErrorNode()->getState() == N->getState())
+if (Report.getErrorNode()->getState() == N->getState() &&
+Report.getErrorNode()->getLocation() == N->getLocation())
   return true;
   }
 }
@@ -3042,21 +3043,6 @@ struct DOTGraphTraits :
 return false;
   }
 
-  static std::string getNodeAttributes(const ExplodedNode *N,
-   ExplodedGraph *) {
-SmallVector Out;
-auto Noop = [](const ExplodedNode*){};
-if (traverseHiddenNodes(N, Noop, Noop, &nodeHasBugReport)) {
-  Out.push_back("style=filled");
-  Out.push_back("fillcolor=red");
-}
-
-if (traverseHiddenNodes(N, Noop, Noop,
-[](const ExplodedNode *C) { return C->isSink(); }))
-  Out.push_back("color=blue");
-return llvm::join(Out, ",");
-  }
-
   static bool isNodeHidden(const ExplodedNode *N) {
 return N->isTrivial();
   }
@@ -3069,9 +3055,16 @@ struct DOTGraphTraits :
 const unsigned int Space = 1;
 ProgramStateRef State = N->getState();
 
+auto Noop = [](const ExplodedNode*){};
+bool HasReport = traverseHiddenNodes(
+N, Noop, Noop, &nodeHasBugReport);
+bool IsSink = traverseHiddenNodes(
+N, Noop, Noop, [](const ExplodedNode *N) { return N->isSink(); });
+
 Out << "{ \"node_id\": " << N->getID(G) << ", \"pointer\": \""
 << (const void *)N << "\", \"state_id\": " << State->getID()
-<< ", \"has_report\": " << (nodeHasBugReport(N) ? "true" : "false")
+<< ", \"has_report\": " << (HasReport ? "true" : "false")
+<< ", \"is_sink\": " << (IsSink ? "true" : "false")
 << ",\\l";
 
 Indent(Out, Space, IsDot) << "\"program_points\": [\\l";

Modified: cfe/trunk/test/Analysis/dump_egraph.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dump_egraph.c?rev=364992&r1=364991&r2=364992&view=diff
==
--- cfe/trunk/test/Analysis/dump_egraph.c (original)
+++ cfe/trunk/test/Analysis/dump_egraph.c Tue Jul  2 18:26:41 2019
@@ -22,9 +22,9 @@ int foo() {
 
 // CHECK: \"program_points\": [\l\{ \"kind\": 
\"BlockEntrance\", \"block_id\": 1
 
-// CHECK: \"has_report\": true
 
 // CHECK: \"pretty\": \"*x\", \"location\": \{ \"line\": 18, \"column\": 10, 
\"file\": \"{{(.+)}}dump_egraph.c\" \}
 
 // CHECK: \"pretty\": \"'x13'\"
 
+// CHECK: \"has_report\": true

Modified: cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages.dot
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages.dot?rev=364992&r1=364991&r2=364992&view=diff
==
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages.dot 
(original)
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages.dot Tue 
Jul  2 18:26:41 2019
@@ -11,6 +11,8 @@ Node0x1 [shape=record,label=
  "{
 { "node_id": 1,
   "pointer": "0x1",
+  "has_report": false

r364991 - [analyzer] exploded-graph-rewriter: NFC: Add more comments.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Tue Jul  2 18:26:38 2019
New Revision: 364991

URL: http://llvm.org/viewvc/llvm-project?rev=364991&view=rev
Log:
[analyzer] exploded-graph-rewriter: NFC: Add more comments.

Modified:
cfe/trunk/utils/analyzer/exploded-graph-rewriter.py

Modified: cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/exploded-graph-rewriter.py?rev=364991&r1=364990&r2=364991&view=diff
==
--- cfe/trunk/utils/analyzer/exploded-graph-rewriter.py (original)
+++ cfe/trunk/utils/analyzer/exploded-graph-rewriter.py Tue Jul  2 18:26:38 2019
@@ -19,6 +19,11 @@ import logging
 import re
 
 
+#===---===#
+# These data structures represent a deserialized ExplodedGraph.
+#===---===#
+
+
 # A helper function for finding the difference between two dictionaries.
 def diff_dicts(curr, prev):
 removed = [k for k in prev if k not in curr or curr[k] != prev[k]]
@@ -368,6 +373,12 @@ class ExplodedGraph(object):
 logging.debug('Skipping.')
 
 
+#===---===#
+# Visitors traverse a deserialized ExplodedGraph and do different things
+# with every node and edge.
+#===---===#
+
+
 # A visitor that dumps the ExplodedGraph into a DOT file with fancy HTML-based
 # syntax highlighing.
 class DotDumpVisitor(object):
@@ -775,11 +786,17 @@ class DotDumpVisitor(object):
 self._dump_raw('}\n')
 
 
+#===---===#
+# Explorers know how to traverse the ExplodedGraph in a certain order.
+# They would invoke a Visitor on every node or edge they encounter.
+#===---===#
+
+
 # A class that encapsulates traversal of the ExplodedGraph. Different explorer
 # kinds could potentially traverse specific sub-graphs.
-class Explorer(object):
+class BasicExplorer(object):
 def __init__(self):
-super(Explorer, self).__init__()
+super(BasicExplorer, self).__init__()
 
 def explore(self, graph, visitor):
 visitor.visit_begin_graph(graph)
@@ -792,6 +809,11 @@ class Explorer(object):
 visitor.visit_end_of_graph()
 
 
+#===---===#
+# The entry point to the script.
+#===---===#
+
+
 def main():
 parser = argparse.ArgumentParser()
 parser.add_argument('filename', type=str)
@@ -814,7 +836,7 @@ def main():
 raw_line = raw_line.strip()
 graph.add_raw_line(raw_line)
 
-explorer = Explorer()
+explorer = BasicExplorer()
 visitor = DotDumpVisitor(args.diff, args.dark)
 explorer.explore(graph, visitor)
 


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

r364989 - [analyzer] exploded-graph-rewriter: Implement checker messages.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Tue Jul  2 18:26:32 2019
New Revision: 364989

URL: http://llvm.org/viewvc/llvm-project?rev=364989&view=rev
Log:
[analyzer] exploded-graph-rewriter: Implement checker messages.

They are displayed as raw lines and diffed via difflib on a per-checker basis.

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

Added:
cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages_diff.dot
Modified:
cfe/trunk/test/Analysis/exploded-graph-rewriter/constraints.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/constraints_diff.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/environment.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/environment_diff.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/store.dot
cfe/trunk/test/Analysis/exploded-graph-rewriter/store_diff.dot
cfe/trunk/utils/analyzer/exploded-graph-rewriter.py

Added: cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages.dot
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages.dot?rev=364989&view=auto
==
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages.dot (added)
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages.dot Tue 
Jul  2 18:26:32 2019
@@ -0,0 +1,30 @@
+// RUN: %exploded_graph_rewriter %s | FileCheck %s
+
+// FIXME: Substitution doesn't seem to work on Windows.
+// UNSUPPORTED: system-windows
+
+// CHECK: Checker State: 
+// CHECK-SAME: alpha.core.FooChecker:
+// CHECK-SAME: Foo stuff:
+// CHECK-SAME: Foo: Bar
+Node0x1 [shape=record,label=
+ "{
+{ "node_id": 1,
+  "pointer": "0x1",
+  "state_id": 2,
+  "program_points": [],
+  "program_state": {
+"store": null,
+"constraints": null,
+"dynamic_types": null,
+"constructing_objects": null,
+"environment": null,
+"checker_messages": [
+  { "checker": "alpha.core.FooChecker", "messages": [
+"Foo stuff:",
+"Foo: Bar"
+  ]}
+]
+  }
+}
+\l}"];

Added: cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages_diff.dot
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages_diff.dot?rev=364989&view=auto
==
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages_diff.dot 
(added)
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/checker_messages_diff.dot 
Tue Jul  2 18:26:32 2019
@@ -0,0 +1,93 @@
+// RUN: %exploded_graph_rewriter -d %s | FileCheck %s
+
+// FIXME: Substitution doesn't seem to work on Windows.
+// UNSUPPORTED: system-windows
+
+Node0x1 [shape=record,label=
+ "{
+{ "node_id": 1,
+  "pointer": "0x1",
+  "state_id": 2,
+  "program_points": [],
+  "program_state": {
+"environment": null,
+"store": null,
+"constraints": null,
+"dynamic_types": null,
+"constructing_objects": null,
+"checker_messages": [
+  { "checker": "FooChecker", "messages": [
+"Foo: Bar"
+  ]},
+  { "checker": "BarChecker", "messages": [
+"Bar: Foo"
+  ]}
+]
+  }
+}
+\l}"];
+
+Node0x1 -> Node0x4;
+
+
+// CHECK: Node0x4 [
+// CHECK-SAME: 
+// CHECK-SAME:   -
+// CHECK-SAME:   BarChecker:
+// CHECK-SAME: 
+// CHECK-SAME: 
+// CHECK-SAME:   -
+// CHECK-SAME:   Bar: Foo
+// CHECK-SAME: 
+// CHECK-SAME: 
+// CHECK-SAME:   
+// CHECK-SAME:   FooChecker:
+// CHECK-SAME: 
+// CHECK-SAME: 
+// CHECK-SAME:   +
+// CHECK-SAME:Bar: Foo
+// CHECK-SAME: 
+// CHECK-SAME: 
+// CHECK-SAME:   +
+// CHECK-SAME:   DunnoWhateverSomeOtherChecker:
+// CHECK-SAME: 
+// CHECK-SAME: 
+// CHECK-SAME:   +
+// CHECK-SAME:   Dunno, some other message.
+// CHECK-SAME: 
+Node0x4 [shape=record,label=
+ "{
+{ "node_id": 4,
+  "pointer": "0x4",
+  "state_id": 5,
+  "program_points": [],
+  "program_state": {
+"environment": null,
+"store": null,
+"constraints": null,
+"dynamic_types": null,
+"constructing_objects": null,
+"checker_messages": [
+  { "checker": "FooChecker", "messages": [
+"Foo: Bar",
+"Bar: Foo"
+  ]},
+  { "checker": "DunnoWhateverSomeOtherChecker", "messages": [
+"Dunno, some other message."
+  ]}
+]
+  }
+}
+\l}"];
+
+Node0x4 -> Node0x6;
+
+Node0x6 [shape=record,label=
+ "{
+{ "node_id": 6,
+  "pointer": "0x6",
+  "state_id": 7,
+  "program_points": [],
+  "program_state": null
+}
+\l}"];

Modified: cfe/trunk/test/Analysis/exploded-graph-rewriter/constraints.dot
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/exploded-grap

r364990 - [analyzer] exploded-graph-rewriter: Collapse very long statement pretty-prints.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Tue Jul  2 18:26:35 2019
New Revision: 364990

URL: http://llvm.org/viewvc/llvm-project?rev=364990&view=rev
Log:
[analyzer] exploded-graph-rewriter: Collapse very long statement pretty-prints.

When printing various statements that include braces (compound
statements, lambda expressions, statement-expressions, etc.),
replace the code between braces with '...'.

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

Modified:
cfe/trunk/test/Analysis/exploded-graph-rewriter/program_points.dot
cfe/trunk/utils/analyzer/exploded-graph-rewriter.py

Modified: cfe/trunk/test/Analysis/exploded-graph-rewriter/program_points.dot
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/exploded-graph-rewriter/program_points.dot?rev=364990&r1=364989&r2=364990&view=diff
==
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/program_points.dot 
(original)
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/program_points.dot Tue Jul  
2 18:26:35 2019
@@ -90,3 +90,27 @@ Node0x2 [shape=record,label=
   }
 ]}
 \l}"];
+
+// Test collapsing large pretty prints with braces.
+
+// CHECK-NEXT: Program point:
+// CHECK-SAME: \{ ... \}
+Node0x3 [shape=record,label=
+ "{
+{ "node_id": 3, "pointer": "0x3",
+  "program_state": null, "program_points": [
+  {
+"kind": "Statement",
+"stmt_kind": "CompoundStmt",
+"stmt_point_kind": "PostStmt",
+"stmd_id": 6,
+"pointer": "0x6",
+"pretty": "{ very very very very very very long pretty print }",
+"location": {
+  "line": 7,
+  "column": 8
+},
+"tag": "ExprEngine : Clean Node"
+  }
+]}
+\l}"];

Modified: cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/exploded-graph-rewriter.py?rev=364990&r1=364989&r2=364990&view=diff
==
--- cfe/trunk/utils/analyzer/exploded-graph-rewriter.py (original)
+++ cfe/trunk/utils/analyzer/exploded-graph-rewriter.py Tue Jul  2 18:26:35 2019
@@ -398,6 +398,21 @@ class DotDumpVisitor(object):
 return '+'
 return '-'
 
+@staticmethod
+def _short_pretty(s):
+if s is None:
+return None
+if len(s) < 20:
+return s
+left = s.find('{')
+right = s.rfind('}')
+if left == -1 or right == -1 or left >= right:
+return s
+candidate = s[0:left + 1] + ' ... ' + s[right:]
+if len(candidate) >= len(s):
+return s
+return candidate
+
 def visit_begin_graph(self, graph):
 self._graph = graph
 self._dump_raw('digraph "ExplodedGraph" {\n')
@@ -433,7 +448,8 @@ class DotDumpVisitor(object):
% (p.loc.filename, p.loc.line,
   p.loc.col, color, p.stmt_kind,
   stmt_color, p.stmt_point_kind,
-  p.pretty if not skip_pretty else ''))
+  self._short_pretty(p.pretty)
+  if not skip_pretty else ''))
 else:
 self._dump(''
'Invalid Source Location:'
@@ -443,7 +459,8 @@ class DotDumpVisitor(object):
'%s'
% (color, p.stmt_kind,
   stmt_color, p.stmt_point_kind,
-  p.pretty if not skip_pretty else ''))
+  self._short_pretty(p.pretty)
+  if not skip_pretty else ''))
 elif p.kind == 'Edge':
 self._dump(''
''
@@ -496,7 +513,7 @@ class DotDumpVisitor(object):
   'lavender' if self._dark_mode else 'darkgreen',
   ('(%s)' % b.kind) if b.kind is not None else ' '
   ),
-  b.pretty, f.bindings[b]))
+  self._short_pretty(b.pretty), f.bindings[b]))
 
 frames_updated = e.diff_frames(prev_e) if prev_e is not None else None
 if frames_updated:


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

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:107
+
+   // If the invariant is broken we do not return the concrete value.
+if (IsInvariantBreak)

Something's wrong with formatting.



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:109
+if (IsInvariantBreak)
+  Out << " should return ";
+else

I'd still like to avoid "should". We are in no position to teach them what 
should their method return. It's not a matter of convention; it's a matter of 
correctness. We don't want people to go fix their code to return true when they 
see a note. We only need to point out that the return value is not what they 
expect.

I suggest removing this note entirely, i.e., early-return when we see an 
invariant break in `checkPostCall`, because we've already emitted a note in 
`checkEndFunction`.



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:145
+
+  // Get the concrete return value.
+  Optional RawConcreteValue = CDM.lookup(*Call);

NoQ wrote:
> I think you can squeeze all this code into one big `isInvariantBreak(Call, 
> ReturnV)` and re-use it across both methods.
I think it would be more precise to talk about "expected"/"actual" return 
value. The actual return value may be either concrete (i.e., 
`nonloc::ConcreteInt`) or symbolic (i.e., `nonloc::SymbolVal`).

Also, there's a relatively famous rule of a thumb for making good comments: 
"comments shouldn't explain what does the code do, but rather why does it do 
it". Instead of these comments i'd rather have one large comment at the 
beginning of `checkEndFunction` explaining what are we trying to do here.



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:145-157
+  // Get the concrete return value.
+  Optional RawConcreteValue = CDM.lookup(*Call);
+  if (!RawConcreteValue)
+return;
+ 
+  // Get the symbolic return value.
+  SVal ReturnV = State->getSVal(RS->getRetValue(), C.getLocationContext());

I think you can squeeze all this code into one big `isInvariantBreak(Call, 
ReturnV)` and re-use it across both methods.



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:170
+
+   // The following is swapped because the invariant is broken.
+Out << '\'' << Name << "' returns "

Something's wrong with formatting.


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

https://reviews.llvm.org/D63915



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

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:228-229
   /// a bug, you can add notes as you add your transitions.
-  const NoteTag *getNoteTag(NoteTag::Callback &&Cb) {
-return Eng.getNoteTags().makeNoteTag(std::move(Cb));
+  const NoteTag *getNoteTag(NoteTag::Callback &&Cb, bool IsPrunable = false) {
+return Eng.getNoteTags().makeNoteTag(std::move(Cb), IsPrunable);
   }

Szelethus wrote:
> Hmm, we use interestingness (`markInteresting()`) already to determine 
> whether we should prune events or not, maybe we could (in the long term) try 
> to make these mechanisms work in harmony.
> 
> In any case, could you please add comments about the new parameter to the 
> class doc? :)
> Hmm, we use interestingness (`markInteresting()`) already to determine 
> whether we should prune events or not, maybe we could (in the long term) try 
> to make these mechanisms work in harmony.

These are two fairly different mechanisms to prevent pruning, but both seem 
useful. The location context is prunable as long as it's not interesting and 
there are no non-prunable notes emitted within it.

All basic nodes are prunable; all checker notes are non-prunable; some 
`trackExpressionValue` notes are non-prunable. This way the really important 
notes (such as checker notes) never get lost. And it's kinda nice and easy to 
understand.


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

https://reviews.llvm.org/D63915



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

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

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

Szelethus added a comment.

I'd love to chip in later, if you don't mind, but here is just a couple things 
that caught my mind that I'd like to share before falling asleep ;)




Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:98-100
+// The APIModeling package is for checkers that model APIs. These checkers are
+// always turned on; this package is intended for API modeling that is not
+// controlled by the target triple.

This isn't true: the user may decide to only enable non-pathsensitive checkers.

I think the comment should rather state that these whether these checkers are 
enabled shouldn't be explicitly specified, unless in **extreme** circumstances 
(causes a crash in a release build?).



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:228-229
   /// a bug, you can add notes as you add your transitions.
-  const NoteTag *getNoteTag(NoteTag::Callback &&Cb) {
-return Eng.getNoteTags().makeNoteTag(std::move(Cb));
+  const NoteTag *getNoteTag(NoteTag::Callback &&Cb, bool IsPrunable = false) {
+return Eng.getNoteTags().makeNoteTag(std::move(Cb), IsPrunable);
   }

Hmm, we use interestingness (`markInteresting()`) already to determine whether 
we should prune events or not, maybe we could (in the long term) try to make 
these mechanisms work in harmony.

In any case, could you please add comments about the new parameter to the class 
doc? :)


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

https://reviews.llvm.org/D63915



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

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:120
+<< (Value ? "true" : "false")
+<< " according to the LLVM coding standard, but it returns "
+<< (Value ? "false" : "true");

NoQ wrote:
> LLVM coding standard is a fairly specific document: 
> https://llvm.org/docs/CodingStandards.html . It doesn't seem to say anything 
> about parsers.
> 
> Let's make this much softer: `Parser::Error() returns false` and that's it.
> 
> Also given that this note always applies to inlined calls, let's move this 
> logic to `checkEndFunction()`. I.e., we emit the "false" note in 
> `checkEndFunction` but we emit the "true" note in `checkPostCall`.
Well, we have tons of unspoken standards like: `MR, DRE, SE, RD, V` all 
makes sense, but I like your idea more.


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

https://reviews.llvm.org/D63915



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

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 207668.
Charusso marked 7 inline comments as done.
Charusso added a comment.

- Create `FunctionExitPoint` diagnostics.
- Fix.


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

https://reviews.llvm.org/D63915

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
  clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  clang/test/Analysis/return-value-guaranteed.cpp
  clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

Index: clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
===
--- clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
+++ clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
@@ -33,11 +33,11 @@
 Impl(std::move(Data)) {}
 
   const bool *lookup(const CallEvent &Call) {
-const bool *Result = Impl.lookup(Call);
+Optional Result = Impl.lookup(Call);
 // If it's a function we expected to find, remember that we've found it.
-if (Result && *Result)
+if (Result && *Result && **Result)
   ++Found;
-return Result;
+return *Result;
   }
 
   // Fail the test if we haven't found all the true-calls we were looking for.
Index: clang/test/Analysis/return-value-guaranteed.cpp
===
--- /dev/null
+++ clang/test/Analysis/return-value-guaranteed.cpp
@@ -0,0 +1,100 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,apiModeling.ReturnValue \
+// RUN:  -analyzer-output=text -verify=class %s
+
+struct Foo { int Field; };
+bool problem();
+void doSomething();
+
+// We predefined the return value of 'MCAsmParser::Error' as true and we cannot
+// take the false-branches which leads to a "garbage value" false positive.
+namespace test_classes {
+struct MCAsmParser {
+  static bool Error();
+};
+
+bool parseFoo(Foo &F) {
+  if (problem()) {
+// class-note@-1 {{Assuming the condition is false}}
+// class-note@-2 {{Taking false branch}}
+return MCAsmParser::Error();
+  }
+
+  F.Field = 0;
+  // class-note@-1 {{The value 0 is assigned to 'F.Field'}}
+  return !MCAsmParser::Error();
+  // class-note@-1 {{'MCAsmParser::Error' returns true}}
+}
+
+bool parseFile() {
+  Foo F;
+  if (parseFoo(F)) {
+// class-note@-1 {{Calling 'parseFoo'}}
+// class-note@-2 {{Returning from 'parseFoo'}}
+// class-note@-3 {{Taking false branch}}
+return true;
+  }
+
+  if (F.Field == 0) {
+// class-note@-1 {{Field 'Field' is equal to 0}}
+// class-note@-2 {{Taking true branch}}
+
+// no-warning: "The left operand of '==' is a garbage value" was here.
+doSomething();
+  }
+
+  (void)(1 / F.Field);
+  // class-warning@-1 {{Division by zero}}
+  // class-note@-2 {{Division by zero}}
+  return false;
+}
+} // namespace test_classes
+
+
+// We predefined 'MCAsmParser::Error' as returning true, but now it returns
+// false, which breaks an unspoken LLVM coding standard. Test the notes.
+namespace test_break {
+struct MCAsmParser {
+  static bool Error() {
+return false; // class-note {{'MCAsmParser::Error' returns false}}
+  }
+};
+
+bool parseFoo(Foo &F) {
+  if (problem()) {
+// class-note@-1 {{Assuming the condition is false}}
+// class-note@-2 {{Taking false branch}}
+return !MCAsmParser::Error();
+  }
+
+  F.Field = 0;
+  // class-note@-1 {{The value 0 is assigned to 'F.Field'}}
+  return MCAsmParser::Error();
+  // class-note@-1 {{Calling 'MCAsmParser::Error'}}
+  // class-note@-2 {{Returning from 'MCAsmParser::Error'}}
+  // class-note@-3 {{'MCAsmParser::Error' should return true but it returns false}}
+}
+
+bool parseFile() {
+  Foo F;
+  if (parseFoo(F)) {
+// class-note@-1 {{Calling 'parseFoo'}}
+// class-note@-2 {{Returning from 'parseFoo'}}
+// class-note@-3 {{Taking false branch}}
+return true;
+  }
+
+  if (F.Field == 0) {
+// class-note@-1 {{Field 'Field' is equal to 0}}
+// class-note@-2 {{Taking true branch}}
+
+// no-warning: "The left operand of '==' is a garbage value" was here.
+doSomething();
+  }
+
+  (void)(1 / F.Field);
+  // class-warning@-1 {{Division by zero}}
+  // class-note@-2 {{Division by zero}}
+  return false;
+}
+} // namespace test_classes
Index: clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -780,6 +780,9 @@
   NewAllocElt->getAllocatorExpr()->getBeginLoc(), SMng);
 }
 llvm_unreachable("Unexpected CFG element at front of block");
+  } else if (Optional FE = P.getAs()) {
+re

[PATCH] D64098: [NFC][clang] Refactor getCompilationPhases step 1: Move list of phases into Types.def table.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 207659.
plotfi marked 4 inline comments as done.
plotfi added a comment.

Update diff based on @compnerd's feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64098

Files:
  clang/include/clang/Driver/Types.def
  clang/include/clang/Driver/Types.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Types.cpp

Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -10,7 +10,8 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include 
-#include 
+#include 
+#include 
 
 using namespace clang::driver;
 using namespace clang::driver::types;
@@ -20,11 +21,12 @@
   const char *Flags;
   const char *TempSuffix;
   ID PreprocessedType;
+  const std::vector Phases;
 };
 
 static const TypeInfo TypeInfos[] = {
-#define TYPE(NAME, ID, PP_TYPE, TEMP_SUFFIX, FLAGS) \
-  { NAME, FLAGS, TEMP_SUFFIX, TY_##PP_TYPE, },
+#define TYPE(NAME, ID, PP_TYPE, TEMP_SUFFIX, FLAGS, ...) \
+  { NAME, FLAGS, TEMP_SUFFIX, TY_##PP_TYPE, { __VA_ARGS__ }, },
 #include "clang/Driver/Types.def"
 #undef TYPE
 };
@@ -264,7 +266,10 @@
 }
 
 // FIXME: Why don't we just put this list in the defs file, eh.
-void types::getCompilationPhases(ID Id, llvm::SmallVectorImpl &P) {
+// FIXME: The list is now in Types.def but for now this function will verify
+//the old behavior and a subsequent change will delete most of the body.
+const std::vector types::getCompilationPhases(ID Id) {
+  std::vector P;
   if (Id != TY_Object) {
 if (getPreprocessedType(Id) != TY_INVALID) {
   P.push_back(phases::Preprocess);
@@ -288,6 +293,15 @@
   }
   assert(0 < P.size() && "Not enough phases in list");
   assert(P.size() <= phases::MaxNumberOfPhases && "Too many phases in list");
+
+  // TODO: This function is still being used to assert that the phase list in
+  //   Types.def is correct. Everything above this comment will be removed
+  //   in a subsequent NFC commit.
+  auto Phases = getInfo(Id).Phases;
+  assert(Phases.size() == P.size() && "Invalid size.");
+  for (unsigned i = 0; i < Phases.size(); i++)
+assert(Phases[i] == P[i] && "Invalid Phase");
+  return Phases;
 }
 
 ID types::lookupCXXTypeForCType(ID Id) {
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2214,7 +2214,7 @@
   /// Builder interface. It doesn't build anything or keep any state.
   class DeviceActionBuilder {
   public:
-typedef llvm::SmallVector PhasesTy;
+typedef const std::vector PhasesTy;
 
 enum ActionBuilderReturnCode {
   // The builder acted successfully on the current action.
@@ -3228,13 +3228,13 @@
   HeaderModulePrecompileJobAction *HeaderModuleAction = nullptr;
   ActionList LinkerInputs;
 
-  llvm::SmallVector PL;
+  unsigned LastPLSize = 0;
   for (auto &I : Inputs) {
 types::ID InputType = I.first;
 const Arg *InputArg = I.second;
 
-PL.clear();
-types::getCompilationPhases(InputType, PL);
+const std::vector PL = types::getCompilationPhases(InputType);
+LastPLSize = PL.size();
 
 // If the first step comes after the final phase we are doing as part of
 // this compilation, warn the user about it.
@@ -3275,8 +3275,8 @@
   // Add a separate precompile phase for the compile phase.
   if (FinalPhase >= phases::Compile) {
 const types::ID HeaderType = lookupHeaderTypeForSourceType(InputType);
-llvm::SmallVector PCHPL;
-types::getCompilationPhases(HeaderType, PCHPL);
+const std::vector PCHPL =
+types::getCompilationPhases(HeaderType);
 // Build the pipeline for the pch file.
 Action *ClangClPch =
 C.MakeAction(*InputArg, HeaderType);
@@ -3300,9 +3300,7 @@
 if (OffloadBuilder.addHostDependenceToDeviceActions(Current, InputArg))
   break;
 
-for (SmallVectorImpl::iterator i = PL.begin(), e = PL.end();
- i != e; ++i) {
-  phases::ID Phase = *i;
+for (phases::ID Phase : PL) {
 
   // We are done if this step is past what the user requested.
   if (Phase > FinalPhase)
@@ -3316,7 +3314,7 @@
 
   // Queue linker inputs.
   if (Phase == phases::Link) {
-assert((i + 1) == e && "linking must be final compilation step.");
+assert(Phase == PL.back() && "linking must be final compilation step.");
 LinkerInputs.push_back(Current);
 Current = nullptr;
 break;
@@ -3373,7 +3371,8 @@
 
   // If we are linking, claim any options which are obviously only used for
   // compilation.
-  if (FinalPhase == phases::Link && PL.size() == 1) {
+  // FIXME: Understand why the last Phase List length is used here.
+  if (FinalPhase == phases::Link && LastPLSize == 1) {
 Args.ClaimAllArgs(options::OPT_

[PATCH] D64110: [analyzer] exploded-graph-rewriter: Implement bug nodes and sink nodes.

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D64110#1567505 , @NoQ wrote:

> A non-sink node may have no successors because it ran out of statements to 
> execute (i.e., the analysis finishes successfully).


Ah, of course, sorry.

> P.S. I don't have many more plans on implementing more prints in the 
> exploded-graph-rewriter. I have plans to implement different Explorers (which 
> would only visit a part of the graph). You should now be unblocked to take a 
> look at the SVG stuff that you had (but pls keep it at a lower priority than 
> the immediate GSoC stuff).

Thanks for your huge amount of work! "Hold my beer."


Repository:
  rC Clang

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

https://reviews.llvm.org/D64110



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

[PATCH] D64110: [analyzer] exploded-graph-rewriter: Implement bug nodes and sink nodes.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Sink node is not the same thing as "zero successors". A sink node is a node in 
which termination was explictly requested as part of modeling, which is usually 
nice to know. A non-sink node may have no successors because it ran out of 
statements to execute (i.e., the analysis finishes successfully).

P.S. I don't have many more plans on implementing more prints in the 
exploded-graph-rewriter. I have plans to implement different Explorers (which 
would only visit a part of the graph). You should now be unblocked to take a 
look at the SVG stuff that you had (but pls keep it at a lower priority than 
the immediate GSoC stuff).


Repository:
  rC Clang

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

https://reviews.llvm.org/D64110



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

[PATCH] D64110: [analyzer] exploded-graph-rewriter: Implement bug nodes and sink nodes.

[Csaba Dabis via Phabricator via cfe-commits]

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

I have removed that `is_sink` business because we have an actual graph, not 
just a huge painting, so you could mark whether it is sink by it has zero 
successors. I like the idea about refactoring that crazy code, thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D64110



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

[PATCH] D64110: [analyzer] exploded-graph-rewriter: Implement bug nodes and sink nodes.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

F9450610: Screen Shot 2019-07-02 at 4.07.00 PM.png 



Repository:
  rC Clang

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

https://reviews.llvm.org/D64110



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

[PATCH] D64110: [analyzer] exploded-graph-rewriter: Implement bug nodes and sink nodes.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added a reviewer: Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

Add a label to nodes that have a bug report attached or on which the analysis 
was generally interrupted.

Fix printing has_report and implement printing is_sink in the graph dumper.


Repository:
  rC Clang

https://reviews.llvm.org/D64110

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/Analysis/exploded-graph-rewriter/checker_messages.dot
  clang/test/Analysis/exploded-graph-rewriter/checker_messages_diff.dot
  clang/test/Analysis/exploded-graph-rewriter/constraints.dot
  clang/test/Analysis/exploded-graph-rewriter/constraints_diff.dot
  clang/test/Analysis/exploded-graph-rewriter/edge.dot
  clang/test/Analysis/exploded-graph-rewriter/environment.dot
  clang/test/Analysis/exploded-graph-rewriter/environment_diff.dot
  clang/test/Analysis/exploded-graph-rewriter/node_labels.dot
  clang/test/Analysis/exploded-graph-rewriter/program_points.dot
  clang/test/Analysis/exploded-graph-rewriter/store.dot
  clang/test/Analysis/exploded-graph-rewriter/store_diff.dot
  clang/utils/analyzer/exploded-graph-rewriter.py

Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- clang/utils/analyzer/exploded-graph-rewriter.py
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -299,6 +299,8 @@
 logging.debug('Adding ' + node_id)
 self.node_id = json_node['node_id']
 self.ptr = json_node['pointer']
+self.has_report = json_node['has_report']
+self.is_sink = json_node['is_sink']
 self.points = [ProgramPoint(p) for p in json_node['program_points']]
 self.state = ProgramState(json_node['state_id'],
   json_node['program_state']) \
@@ -754,6 +756,12 @@
% ("gray20" if self._dark_mode else "gray",
   node.node_id, node.ptr, node.state.state_id
   if node.state is not None else 'Unspecified'))
+if node.has_report:
+self._dump('Bug Report Attached'
+   '')
+if node.is_sink:
+self._dump('Sink Node'
+   '')
 self._dump('')
 if len(node.points) > 1:
 self._dump('Program points:')
Index: clang/test/Analysis/exploded-graph-rewriter/store_diff.dot
===
--- clang/test/Analysis/exploded-graph-rewriter/store_diff.dot
+++ clang/test/Analysis/exploded-graph-rewriter/store_diff.dot
@@ -7,6 +7,8 @@
  "{
 { "node_id": 1,
   "pointer": "0x1",
+  "has_report": false,
+  "is_sink": false,
   "state_id": 2,
   "program_points": [],
   "program_state": {
@@ -55,6 +57,8 @@
  "{
 { "node_id": 4,
   "pointer": "0x4",
+  "has_report": false,
+  "is_sink": false,
   "state_id": 5,
   "program_points": [],
   "program_state": {
@@ -89,6 +93,8 @@
  "{
 { "node_id": 6,
   "pointer": "0x6",
+  "has_report": false,
+  "is_sink": false,
   "state_id": 7,
   "program_points": [],
   "program_state": null
Index: clang/test/Analysis/exploded-graph-rewriter/store.dot
===
--- clang/test/Analysis/exploded-graph-rewriter/store.dot
+++ clang/test/Analysis/exploded-graph-rewriter/store.dot
@@ -24,6 +24,8 @@
  "{
 { "node_id": 1,
   "pointer": "0x1",
+  "has_report": false,
+  "is_sink": false,
   "state_id": 2,
   "program_points": [],
   "program_state": {
Index: clang/test/Analysis/exploded-graph-rewriter/program_points.dot
===
--- clang/test/Analysis/exploded-graph-rewriter/program_points.dot
+++ clang/test/Analysis/exploded-graph-rewriter/program_points.dot
@@ -28,7 +28,7 @@
 // CHECK-SAME: 
 Node0x1 [shape=record,label=
  "{
-{ "node_id": 1, "pointer": "0x1",
+{ "node_id": 1, "pointer": "0x1", "has_report": false, "is_sink": false,
   "program_state": null, "program_points": [
   {
 "kind": "Edge",
@@ -73,7 +73,7 @@
 // CHECK-SAME: 
 Node0x2 [shape=record,label=
  "{
-{ "node_id": 2, "pointer": "0x2",
+{ "node_id": 2, "pointer": "0x2", "has_report": false, "is_sink": false,
   "program_state": null, "program_points": [
   {
 "kind": "Statement",
@@ -97,7 +97,7 @@
 // CHECK-SAME: \{ ... \}
 Node0x3 [shape=record,label=
  "{
-{ "node_id": 3, "pointer": "0x3",
+{ "node_id": 3, "pointer": "0x3", "has_report": false, "is_sink": false,
   "program_state": null, "program_points": [
   {
 "kind": "Statement",
Index: clang/test/Analysis/exploded-graph-rewriter/node_labels.dot

[PATCH] D64073: [ASTImporter] Fix import of lambda in function param

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Hello Gabor,
There is an inline question about tests; other code looks fine.




Comment at: clang/lib/AST/ASTImporter.cpp:1713
+// In case of lambdas, the class already has a definition ptr set, but
+// the  contained decls are not imported yet. Also, isBeingDefined was
+// set in CXXRecordDecl::CreateLambda.  We must import the contained

Two spaces after 'the'.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:5103
+  // count.
+  for (auto &D : ToL->decls()) {
+(void)D;

Can we use std::distance instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64073



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

[PATCH] D64075: [ASTImporter] Fix structural eq of lambdas

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Hello Gabor,
This looks mostly good but I have a question inline.




Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:1182
 
+  if (D1CXX->isLambda()) {
+if (!D2CXX->isLambda())

Should we return false if `D1CXX->isLambda() != D2CXX->isLambda()`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64075



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

[PATCH] D61809: [BPF] Preserve debuginfo array/union/struct type/access index

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song updated this revision to Diff 207644.
yonghong-song edited the summary of this revision.
yonghong-song added a comment.

change `void *__builtin_preserve_access_index(void *)` to `const void * 
_builtin_preserve_access_index(const void *)`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61809

Files:
  docs/LanguageExtensions.rst
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuilder.h
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Sema/SemaChecking.cpp
  test/CodeGen/bpf-preserve-access-index-2.c
  test/CodeGen/bpf-preserve-access-index.c

Index: test/CodeGen/bpf-preserve-access-index.c
===
--- /dev/null
+++ test/CodeGen/bpf-preserve-access-index.c
@@ -0,0 +1,22 @@
+// RUN: %clang %s -target bpfeb -x c -emit-llvm -S -g -O2 -o - | FileCheck --check-prefix=CHECK %s
+// RUN: %clang %s -target bpfel -x c -emit-llvm -S -g -O2 -o - | FileCheck --check-prefix=CHECK %s
+
+struct t {
+  int i:1;
+  int j:2;
+  union {
+   int a;
+   int b;
+  } c[4];
+};
+
+#define _(x) (__builtin_preserve_access_index(x))
+
+const void *test(struct t *arg) {
+  return _(&arg->c[3].b);
+}
+
+// CHECK: llvm.preserve.struct.access.index
+// CHECK: llvm.preserve.array.access.index
+// CHECK: llvm.preserve.union.access.index
+// CHECK-NOT: __builtin_preserve_access_index
Index: test/CodeGen/bpf-preserve-access-index-2.c
===
--- /dev/null
+++ test/CodeGen/bpf-preserve-access-index-2.c
@@ -0,0 +1,22 @@
+// RUN: %clang %s -target bpfeb -x c -emit-llvm -S -g -O2 -o - | FileCheck %s
+// RUN: %clang %s -target bpfel -x c -emit-llvm -S -g -O2 -o - | FileCheck %s
+
+struct t {
+  int i:1;
+  int j:2;
+  union {
+   int a;
+   int b;
+  } c[4];
+};
+
+#define _(x) (x)
+
+const void *test(struct t *arg) {
+  return _(&arg->c[3].b);
+}
+
+// CHECK-NOT: llvm.preserve.struct.access.index
+// CHECK-NOT: llvm.preserve.array.access.index
+// CHECK-NOT: llvm.preserve.union.access.index
+// CHECK-NOT: __builtin_preserve_access_index
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -191,6 +191,16 @@
   return false;
 }
 
+/// Check the number of arguments, and set the result type to
+/// the argument type.
+static bool SemaBuiltinPreserveAI(Sema &S, CallExpr *TheCall) {
+  if (checkArgCount(S, TheCall, 1))
+return true;
+
+  TheCall->setType(TheCall->getArg(0)->getType());
+  return false;
+}
+
 static bool SemaBuiltinOverflow(Sema &S, CallExpr *TheCall) {
   if (checkArgCount(S, TheCall, 3))
 return true;
@@ -1409,6 +1419,10 @@
 TheCall->setType(Context.IntTy);
 break;
   }
+  case Builtin::BI__builtin_preserve_access_index:
+if (SemaBuiltinPreserveAI(*this, TheCall))
+  return ExprError();
+break;
   case Builtin::BI__builtin_call_with_static_chain:
 if (SemaBuiltinCallWithStaticChain(*this, TheCall))
   return ExprError();
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -480,6 +480,10 @@
   /// finally block or filter expression.
   bool IsOutlinedSEHHelper = false;
 
+  /// True if CodeGen currently emits code inside presereved access index
+  /// region.
+  bool IsInPreservedAIRegion = false;
+
   const CodeGen::CGBlockInfo *BlockInfo = nullptr;
   llvm::Value *BlockPointer = nullptr;
 
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -25,6 +25,7 @@
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/NSAPI.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringExtras.h"
@@ -3418,8 +3419,20 @@
   CharUnits eltAlign =
 getArrayElementAlign(addr.getAlignment(), indices.back(), eltSize);
 
-  llvm::Value *eltPtr = emitArraySubscriptGEP(
-  CGF, addr.getPointer(), indices, inbounds, signedIndices, loc, name);
+  llvm::Value *eltPtr;
+  auto LastIndex = dyn_cast(indices.back());
+  if (!CGF.IsInPreservedAIRegion || !LastIndex) {
+eltPtr = emitArraySubscriptGEP(
+CGF, addr.getPointer(), indices, inbounds, signedIndices,
+loc, name);
+  } else {
+// Remember the original array subscript for bpf target
+unsigned idx = LastIndex->getZExtValue();
+eltPtr = CGF.Builder.CreatePreserveArrayAccessIndex(addr.getPointer(),
+indices.size() - 1,
+idx);
+  }
+
   return Address(eltPtr, eltAlign);
 }
 
@@ -3908,6 +3921,19 @@
   return CGF.Builder.CreateStructGEP(base, idx, fie

[PATCH] D62329: [ASTImporter] Structural eq: handle DependentScopeDeclRefExpr

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added inline comments.



Comment at: cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp:173
+DE2->getQualifier());
+  } else if (auto CastE1 = dyn_cast(E1)) {
+auto *CastE2 = dyn_cast(E2);

Hi Gabor,
Is there any test for this branch?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62329



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

[PATCH] D64104: [analyzer] exploded-graph-rewriter: Collapse large statement pretty-prints.

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D64104#1567326 , @NoQ wrote:

> In D64104#1567317 , @Charusso wrote:
>
> > I like it! What about `BlockEdge` with the long terminators? (c.f. 
> > `Edge.getSrc()->printTerminatorJson()`)
>
>
> We don't process these yet >.< But it should be trivial to cover them in a 
> similar manner once we implement them.


Yes, great patch, thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D64104



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

[PATCH] D64104: [analyzer] exploded-graph-rewriter: Collapse large statement pretty-prints.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D64104#1567317 , @Charusso wrote:

> I like it! What about `BlockEdge` with the long terminators? (c.f. 
> `Edge.getSrc()->printTerminatorJson()`)


We don't process these yet >.< But it should be trivial to cover them in a 
similar manner once we implement them.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64104



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

[PATCH] D64104: [analyzer] exploded-graph-rewriter: Collapse large statement pretty-prints.

[Csaba Dabis via Phabricator via cfe-commits]

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

I like it! What about `BlockEdge` with the long terminators? (c.f. 
`Edge.getSrc()->printTerminatorJson()`)


Repository:
  rC Clang

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

https://reviews.llvm.org/D64104



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

[PATCH] D64104: [analyzer] exploded-graph-rewriter: Collapse large statement pretty-prints.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added a reviewer: Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

When printing various statements that include braces (compound statements, 
lambda expressions, statement-expressions, etc.), replace the code between 
braces with '...'.

I don't mind losing some information here because it's easily recoverable by 
looking at the source code.

Before:
F9450332: Screen Shot 2019-07-02 at 2.25.39 PM.png 


After:
F9450331: Screen Shot 2019-07-02 at 2.25.46 PM.png 



Repository:
  rC Clang

https://reviews.llvm.org/D64104

Files:
  clang/test/Analysis/exploded-graph-rewriter/program_points.dot
  clang/utils/analyzer/exploded-graph-rewriter.py


Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- clang/utils/analyzer/exploded-graph-rewriter.py
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -398,6 +398,21 @@
 return '+'
 return '-'
 
+@staticmethod
+def _short_pretty(s):
+if s is None:
+return None
+if len(s) < 20:
+return s
+left = s.find('{')
+right = s.rfind('}')
+if left == -1 or right == -1 or left >= right:
+return s
+candidate = s[0:left + 1] + ' ... ' + s[right:]
+if len(candidate) >= len(s):
+return s
+return candidate
+
 def visit_begin_graph(self, graph):
 self._graph = graph
 self._dump_raw('digraph "ExplodedGraph" {\n')
@@ -433,7 +448,8 @@
% (p.loc.filename, p.loc.line,
   p.loc.col, color, p.stmt_kind,
   stmt_color, p.stmt_point_kind,
-  p.pretty if not skip_pretty else ''))
+  self._short_pretty(p.pretty)
+  if not skip_pretty else ''))
 else:
 self._dump(''
'Invalid Source Location:'
@@ -443,7 +459,8 @@
'%s'
% (color, p.stmt_kind,
   stmt_color, p.stmt_point_kind,
-  p.pretty if not skip_pretty else ''))
+  self._short_pretty(p.pretty)
+  if not skip_pretty else ''))
 elif p.kind == 'Edge':
 self._dump(''
''
@@ -496,7 +513,7 @@
   'lavender' if self._dark_mode else 'darkgreen',
   ('(%s)' % b.kind) if b.kind is not None else ' '
   ),
-  b.pretty, f.bindings[b]))
+  self._short_pretty(b.pretty), f.bindings[b]))
 
 frames_updated = e.diff_frames(prev_e) if prev_e is not None else None
 if frames_updated:
Index: clang/test/Analysis/exploded-graph-rewriter/program_points.dot
===
--- clang/test/Analysis/exploded-graph-rewriter/program_points.dot
+++ clang/test/Analysis/exploded-graph-rewriter/program_points.dot
@@ -90,3 +90,27 @@
   }
 ]}
 \l}"];
+
+// Test collapsing large pretty prints with braces.
+
+// CHECK-NEXT: Program point:
+// CHECK-SAME: \{ ... \}
+Node0x3 [shape=record,label=
+ "{
+{ "node_id": 3, "pointer": "0x3",
+  "program_state": null, "program_points": [
+  {
+"kind": "Statement",
+"stmt_kind": "CompoundStmt",
+"stmt_point_kind": "PostStmt",
+"stmd_id": 6,
+"pointer": "0x6",
+"pretty": "{ very very very very very very long pretty print }",
+"location": {
+  "line": 7,
+  "column": 8
+},
+"tag": "ExprEngine : Clean Node"
+  }
+]}
+\l}"];


Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- clang/utils/analyzer/exploded-graph-rewriter.py
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -398,6 +398,21 @@
 return '+'
 return '-'
 
+@staticmethod
+def _short_pretty(s):
+if s is None:
+return None
+if len(s) < 20:
+return s
+left = s.find('{')
+right = s.rfind('}')
+if left == -1 or right == -1 or left >= right:
+return s
+candidate = s[0:left + 1] + ' ... ' + s[right:]
+if len(candidate) >= len(s):
+return s
+return candidate
+
 def visit_begin_graph(self, graph):
 self._graph = graph
 self._dump_raw('digraph "ExplodedGraph" {\n')
@@ -433,7 +448,8 @@
% (p.loc.filename, p.loc.line,

[PATCH] D64098: [NFC][clang] Refactor getCompilationPhases step 1: Move list of phases into Types.def table.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.



Comment at: clang/lib/Driver/Types.cpp:29
+  { NAME, FLAGS, TEMP_SUFFIX, TY_##PP_TYPE, PHASES, },
+#define PHASES llvm::SmallVector
 #include "clang/Driver/Types.def"

I think that we can abuse the preprocessor a bit and get something that is 
nicer to read.  Lets make the last parameter a variadic and pass along the 
phases to the list.  Something like:

```
ENTRY('a', "std::string")
ENTRY('b', "std::string", "std::vector")
ENTRY('c', "std::string", "std::vector", "int")

const struct {
  unsigned int id;
  std::vector strings;
} array[] = {
#define ENTRY(id, strings...) { id, { strings } },
#include "reduced.def"
#undef ENTRY
};
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64098



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

[PATCH] D64100: [analyzer] exploded-graph-rewriter: Implement checker messages.

[Csaba Dabis via Phabricator via cfe-commits]

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

Nice! Could you add some `#===---===#` separators, please? As it is in the 
finishing state, I think now it is appropriate.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64100



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

[PATCH] D64100: [analyzer] exploded-graph-rewriter: Implement checker messages.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added a reviewer: Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

They are displayed as raw lines and diffed via difflib on a per-checker basis.


Repository:
  rC Clang

https://reviews.llvm.org/D64100

Files:
  clang/test/Analysis/exploded-graph-rewriter/checker_messages.dot
  clang/test/Analysis/exploded-graph-rewriter/checker_messages_diff.dot
  clang/test/Analysis/exploded-graph-rewriter/constraints.dot
  clang/test/Analysis/exploded-graph-rewriter/constraints_diff.dot
  clang/test/Analysis/exploded-graph-rewriter/environment.dot
  clang/test/Analysis/exploded-graph-rewriter/environment_diff.dot
  clang/test/Analysis/exploded-graph-rewriter/store.dot
  clang/test/Analysis/exploded-graph-rewriter/store_diff.dot
  clang/utils/analyzer/exploded-graph-rewriter.py

Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- clang/utils/analyzer/exploded-graph-rewriter.py
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -13,6 +13,7 @@
 
 import argparse
 import collections
+import difflib
 import json
 import logging
 import re
@@ -211,6 +212,41 @@
 return len(removed) != 0 or len(added) != 0 or len(updated) != 0
 
 
+# Deserialized messages from a single checker in a single program state.
+# Basically a list of raw strings.
+class CheckerLines(object):
+def __init__(self, json_lines):
+super(CheckerLines, self).__init__()
+self.lines = json_lines
+
+def diff_lines(self, prev):
+lines = difflib.ndiff(prev.lines, self.lines)
+return [l.strip() for l in lines
+if l.startswith('+') or l.startswith('-')]
+
+def is_different(self, prev):
+return len(self.diff_lines(prev)) > 0
+
+
+# Deserialized messages of all checkers, separated by checker.
+class CheckerMessages(object):
+def __init__(self, json_m):
+super(CheckerMessages, self).__init__()
+self.items = collections.OrderedDict(
+[(m['checker'], CheckerLines(m['messages'])) for m in json_m])
+
+def diff_messages(self, prev):
+removed = [k for k in prev.items if k not in self.items]
+added = [k for k in self.items if k not in prev.items]
+updated = [k for k in prev.items if k in self.items
+   and prev.items[k].is_different(self.items[k])]
+return (removed, added, updated)
+
+def is_different(self, prev):
+removed, added, updated = self.diff_messages(prev)
+return len(removed) != 0 or len(added) != 0 or len(updated) != 0
+
+
 # A deserialized program state.
 class ProgramState(object):
 def __init__(self, state_id, json_ps):
@@ -241,7 +277,8 @@
 GenericEnvironment(json_ps['constructing_objects']) \
 if json_ps['constructing_objects'] is not None else None
 
-# TODO: Checker messages.
+self.checker_messages = CheckerMessages(json_ps['checker_messages']) \
+if json_ps['checker_messages'] is not None else None
 
 
 # A deserialized exploded graph node. Has a default constructor because it
@@ -595,16 +632,73 @@
 if m is None:
 self._dump(' Nothing!')
 else:
-if prev_s is not None:
-if prev_m is not None:
-if m.is_different(prev_m):
-self._dump('')
-self.visit_generic_map(m, prev_m)
-else:
-self._dump(' No changes!')
-if prev_m is None:
+if prev_m is not None:
+if m.is_different(prev_m):
+self._dump('')
+self.visit_generic_map(m, prev_m)
+else:
+self._dump(' No changes!')
+else:
 self._dump('')
 self.visit_generic_map(m)
+
+self._dump('')
+
+def visit_checker_messages(self, m, prev_m=None):
+self._dump('')
+
+def dump_line(l, is_added=None):
+self._dump('%s'
+   '%s'
+   % (self._diff_plus_minus(is_added), l))
+
+def dump_chk(chk, is_added=None):
+dump_line('%s:' % chk, is_added)
+
+if prev_m is not None:
+removed, added, updated = m.diff_messages(prev_m)
+for chk in removed:
+dump_chk(chk, False)
+for l in prev_m.items[chk].lines:
+dump_line(l, False)
+for chk in updated:
+dump_chk(chk)
+for l in m.items[chk].diff_lines(prev_m.items[chk]):
+dump_line(l[1:], l.startswith('+'))
+for chk in added:
+dump_chk(chk, True)
+for l in m.items[chk].lines:
+dump_line(l, True)
+e

[PATCH] D64098: [NFC][clang] Refactor getCompilationPhases step 1: Move list of phases into Types.def table.

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

The explicit list I think is way better for readability, this is a nice 
starting point for cleaning this up.




Comment at: clang/include/clang/Driver/Types.def:18
 
 // TYPE(NAME, ID, PP_TYPE, TEMP_SUFFIX, FLAGS)
 

Please update the comment here indicating the new field



Comment at: clang/lib/Driver/Driver.cpp:3304
 
-for (SmallVectorImpl::iterator i = PL.begin(), e = PL.end();
- i != e; ++i) {
+for (auto i = PL.begin(), e = PL.end(); i != e; ++i) {
   phases::ID Phase = *i;

Why can this not be a range based for loop?



Comment at: clang/lib/Driver/Types.cpp:271
+//the old behavior and a subsequent change will delete most of the 
body.
+const llvm::SmallVector 
&types::getCompilationPhases(ID Id) {
+  llvm::SmallVector P;

Can we return `llvm::SmallVectorImpl` instead?  The size is 
immaterial to this I think.



Comment at: clang/lib/Driver/Types.cpp:303
+  return Phases;
 }
 

A comment to indicate that the concrete list will become the implementation 
would be nice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64098



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

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

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

xbolva00 added a comment.

In D63082#1564114 , @aaron.ballman 
wrote:

> In D63082#1560667 , @xbolva00 wrote:
>
> > I wanted to follow GCC’s behaviour -Wall but then dicussion moved to 
> > “tautological compare group” - I was fine with that too..
> >  I can again revert it to -Wall...
> >  If this should be off by default even on -Wall, then all work was useless..
>
>
> Personally, my feeling is that this new diagnostic group belongs under the 
> tautological compare group. That group is currently off by default, but I'm 
> wondering if we want it to be on by default (in a subsequent patch), or 
> whether we're okay having parts of it on by default and parts off by default. 
> I'm hoping @rsmith will voice an opinion here.
>
> In D63082#1560684 , @xbolva00 wrote:
>
> > “Perhaps appending something about the resulting value always being 
> > true|false would help most of these diagnostics be more obvious”
> >
> > I dont know, this could only diagnose constant multiplications (maybe this 
> > is already handled by some “always true” check). Anyway, I have no 
> > motivation to diagnose constant muls, sorry. My goal is to make this 
> > warning atleast as good as GCC’s implementation.
> >
> > Okay, we could make it better and append “; did you mean “index * 3 != 0”?
>
>
> I don't think that actually improves anything. I do not think we want a "did 
> you mean" in this case because I'm not confident anyone can guess what a user 
> was trying for in this situation. However, telling the user the result of the 
> computation does give them very useful information -- it tells them the 
> result will always be tautologically true or false (which is what the other 
> tautological warnings do, as well). As it stands, the current diagnostic 
> wording doesn't help the user understand what's wrong with their code. I'd 
> like to correct that deficiency before we commit this.


Thanks

I decided to implement your suggestions.


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

https://reviews.llvm.org/D63082



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

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

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

xbolva00 updated this revision to Diff 207623.
xbolva00 added a comment.

Implemented suggested "mul in bool context" diagnostic.


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

https://reviews.llvm.org/D63082

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/Sema/warn-int-in-bool-context.c
  test/Sema/warn-unreachable.c
  test/SemaCXX/nested-name-spec.cpp
  test/SemaCXX/warn-int-in-bool-context.cpp

Index: test/SemaCXX/warn-int-in-bool-context.cpp
===
--- test/SemaCXX/warn-int-in-bool-context.cpp
+++ test/SemaCXX/warn-int-in-bool-context.cpp
@@ -0,0 +1,172 @@
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wint-in-bool-context %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s
+
+#define ONE 1
+#define TWO 2
+
+enum answer {
+  foo,
+  bar,
+  yes,
+  no,
+  unknown,
+};
+
+enum fruit {
+  orange = -3,
+  lemon,
+  banana,
+  apple,
+};
+
+void g(bool b);
+
+template 
+void h() {}
+template 
+void h() {}
+
+bool test(int a, int b, enum answer ans, enum fruit f) {
+  bool r = TWO * 7; // expected-warning {{'*' in boolean context, the expression will always evaluate to 'true'}}
+  r = 3 * 7;// expected-warning {{'*' in boolean context, the expression will always evaluate to 'true'}}
+  r = a * 7;
+  r = a * b;
+  r = a * 0; // expected-warning {{'*' in boolean context, the expression will always evaluate to 'false'}}
+  r = 0 * b; // expected-warning {{'*' in boolean context, the expression will always evaluate to 'false'}}
+  g(3 * 9);  // expected-warning {{'*' in boolean context, the expression will always evaluate to 'true'}}
+  h<3 * 9>();
+
+  r = TWO << 7; // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+  r = a << 7;   // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+  r = ONE << b; // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+
+  r = a ? ONE : TWO; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? (1) : TWO;
+  r = a ? 3 : TWO; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? -2 : 0;
+  r = a ? 3 : -2;
+  r = a ? 0 : TWO; // expected-warning {{'?:' with integer constants in boolean context}}
+  r = a ? 3 : ONE; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? ONE : 0;
+  r = a ? 0 : 0;
+  r = a ? ONE : 0;
+  r = a ? ONE : ONE;
+  g(a ? 3 : 9); // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+
+  r = a ? yes   // expected-warning {{enum constant in boolean context}}
+: no;   // expected-warning {{enum constant in boolean context}}
+  r = ans == yes || no; // expected-warning {{enum constant in boolean context}}
+  r = yes || ans == no; // expected-warning {{enum constant in boolean context}}
+  r = ans == yes || ans == no;
+  r = !foo;
+  r = !bar;
+  r = !yes;// expected-warning {{enum constant in boolean context}}
+  g(ans == yes || no); // expected-warning {{enum constant in boolean context}}
+
+  if (8 * 4) // expected-warning {{'*' in boolean context, the expression will always evaluate to 'true'}}
+return a;
+
+  if (a * 4)
+return a;
+
+  if (-TWO * b)
+return a;
+
+  if (TWO << 4) // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+return a;
+
+  if (a << TWO) // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+return a;
+
+  if (ONE << b) // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+return a;
+
+  if (a ? ONE : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+return a;
+
+  if (a ? 32 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+return a;
+
+  if (a ? 7 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+return a;
+
+  if (ans == yes || foo)
+return a;
+
+  if (f == apple || orange) // expected-warning {{enum constant in boolean context}}
+return a;
+
+  if (ans == yes || no) // expected-warning {{enum constant in boolean context}}
+return a;
+
+  if (yes || ans == no) // expected-warning {{enum constant in boolean context}}
+return a;
+
+  if (r || a ? 7 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+return a;
+
+  if (b && a ? 7 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+return a;
+
+  if ((a ? 7 : TWO) && b == 32) // 

[PATCH] D64062: Remove both -dumpversion and __VERSION__

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

This isn't firefox per say but thirdparty apps.
If you feel confident, sure, we can land that and see what happens :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D64062



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

[PATCH] D64062: Remove both -dumpversion and __VERSION__

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

But, based on the github search, I don't think we could reasonably change 
dumpversion to print 8.0.0, so I'm not sure it really matters. Firefox builds 
with clang these days, so I'm not sure what they could possibly be using 
-dumpversion for that would matter.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64062



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

[PATCH] D64098: [NFC][clang] Refactor getCompilationPhases step 1: Move list of phases into Types.def table.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
plotfi added a reviewer: compnerd.

Simplifying the phases generation process, first by copying the phases info 
into the Table.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64098

Files:
  clang/include/clang/Driver/Types.def
  clang/include/clang/Driver/Types.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Types.cpp

Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -9,8 +9,8 @@
 #include "clang/Driver/Types.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/ADT/SmallVector.h"
 #include 
-#include 
 
 using namespace clang::driver;
 using namespace clang::driver::types;
@@ -20,11 +20,13 @@
   const char *Flags;
   const char *TempSuffix;
   ID PreprocessedType;
+  const llvm::SmallVector Phases;
 };
 
 static const TypeInfo TypeInfos[] = {
-#define TYPE(NAME, ID, PP_TYPE, TEMP_SUFFIX, FLAGS) \
-  { NAME, FLAGS, TEMP_SUFFIX, TY_##PP_TYPE, },
+#define TYPE(NAME, ID, PP_TYPE, TEMP_SUFFIX, FLAGS, PHASES) \
+  { NAME, FLAGS, TEMP_SUFFIX, TY_##PP_TYPE, PHASES, },
+#define PHASES llvm::SmallVector
 #include "clang/Driver/Types.def"
 #undef TYPE
 };
@@ -264,7 +266,10 @@
 }
 
 // FIXME: Why don't we just put this list in the defs file, eh.
-void types::getCompilationPhases(ID Id, llvm::SmallVectorImpl &P) {
+// FIXME: The list is now in Types.def but for now this function will verify
+//the old behavior and a subsequent change will delete most of the body.
+const llvm::SmallVector &types::getCompilationPhases(ID Id) {
+  llvm::SmallVector P;
   if (Id != TY_Object) {
 if (getPreprocessedType(Id) != TY_INVALID) {
   P.push_back(phases::Preprocess);
@@ -288,6 +293,13 @@
   }
   assert(0 < P.size() && "Not enough phases in list");
   assert(P.size() <= phases::MaxNumberOfPhases && "Too many phases in list");
+
+  const llvm::SmallVector &Phases = getInfo(Id).Phases;
+  assert(Phases.size() == P.size() && "Invalid size.");
+  for (unsigned i = 0; i < Phases.size(); i++)
+assert(Phases[i] == P[i] && "Invalid Phase");
+
+  return Phases;
 }
 
 ID types::lookupCXXTypeForCType(ID Id) {
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2257,7 +2257,7 @@
 virtual ActionBuilderReturnCode
 getDeviceDependences(OffloadAction::DeviceDependences &DA,
  phases::ID CurPhase, phases::ID FinalPhase,
- PhasesTy &Phases) {
+ const PhasesTy &Phases) {
   return ABRT_Inactive;
 }
 
@@ -2519,7 +2519,7 @@
 ActionBuilderReturnCode
 getDeviceDependences(OffloadAction::DeviceDependences &DA,
  phases::ID CurPhase, phases::ID FinalPhase,
- PhasesTy &Phases) override {
+ const PhasesTy &Phases) override {
   if (!IsActive)
 return ABRT_Inactive;
 
@@ -2636,7 +2636,7 @@
 ActionBuilderReturnCode
 getDeviceDependences(OffloadAction::DeviceDependences &DA,
  phases::ID CurPhase, phases::ID FinalPhase,
- PhasesTy &Phases) override {
+ const PhasesTy &Phases) override {
   // amdgcn does not support linking of object files, therefore we skip
   // backend and assemble phases to output LLVM IR. Except for generating
   // non-relocatable device coee, where we generate fat binary for device
@@ -2753,7 +2753,7 @@
 ActionBuilderReturnCode
 getDeviceDependences(OffloadAction::DeviceDependences &DA,
  phases::ID CurPhase, phases::ID FinalPhase,
- PhasesTy &Phases) override {
+ const PhasesTy &Phases) override {
   if (OpenMPDeviceActions.empty())
 return ABRT_Inactive;
 
@@ -2955,7 +2955,7 @@
   Action *
   addDeviceDependencesToHostAction(Action *HostAction, const Arg *InputArg,
phases::ID CurPhase, phases::ID FinalPhase,
-   DeviceActionBuilder::PhasesTy &Phases) {
+   const DeviceActionBuilder::PhasesTy &Phases) {
 if (!IsValid)
   return nullptr;
 
@@ -3228,13 +3228,14 @@
   HeaderModulePrecompileJobAction *HeaderModuleAction = nullptr;
   ActionList LinkerInputs;
 
-  llvm::SmallVector PL;
+  unsigned LastPLSize = 0;
   for (auto &I : Inputs) {
 types::ID InputType = I.first;
 const Arg *InputArg = I.second;
 
-PL.clear();
-types::getCompilationPhases(InputType, PL);
+const llvm::SmallVector &PL =
+types::getCompilationPhases(InputType);
+LastPLSize = PL.size();
 
 // If the first step comes after the final phase we are doing as part of
 

[PATCH] D63623: [clang-tidy] new check: bugprone-posix-return

[Jian Cai via Phabricator via cfe-commits]

jcai19 added a comment.

In D63623#1565377 , @gribozavr wrote:

> Thanks! Do you have commit access? Do you want me to commit this patch for 
> you?


Totally missed your message yesterday. Sorry about that. I do not have the 
access so it will be great if you can commit the change. Thanks a lot.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63623



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

[PATCH] D64089: [Driver] Introduce -stdlib++-isystem

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

In D64089#1567028 , @danalbert wrote:

> > For example, when we're building against the Android NDK, we might want to 
> > use the NDK's C++ headers (which have a custom inline namespace) even if we 
> > have C++ headers installed next to the driver.
>
> Since NDK r19 the NDK libc++ headers are already installed alongside the 
> driver, so this doesn't benefit that use case. The others sound useful, just 
> FYI in case the NDK was the main motivating factor here :)


We're using our own toolchain (which ends up with the upstream libc++ headers 
installed alongside the driver) but building against the NDK, so that doesn't 
work for us, unfortunately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64089



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

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/AST/Type.h:1133
+  /// Check if this is or contains a non-trivial C struct/union type.
+  bool hasNonTrivialPrimitiveCStruct() const;
 

You only want these checks to trigger on unions with non-trivial members (or 
structs containing them), right?  How about something like 
`hasNonTrivialPrimitiveCUnionMember()`?  Or maybe make it more descriptive for 
the use sites, like `isPrimitiveCRestrictedType()`?

Also, it would be nice if the fast path of this could be inlined so that 
clients usually didn't had to make a call at all.  You can write the 
`getBaseElementTypeUnsafe()->getAs()` part in an `inline` 
implementation at the bottom this file.



Comment at: lib/Sema/SemaExpr.cpp:16218
+checkNonTrivialCUnion(E->getType(), E->getExprLoc(),
+  Sema::NTCUC_LValueToRValueVolatile);
+

ahatanak wrote:
> ahatanak wrote:
> > rjmccall wrote:
> > > ahatanak wrote:
> > > > rjmccall wrote:
> > > > > ahatanak wrote:
> > > > > > rjmccall wrote:
> > > > > > > ahatanak wrote:
> > > > > > > > rjmccall wrote:
> > > > > > > > > It looks like you're generally warning about this based on 
> > > > > > > > > the specific context that forced an lvalue-to-rvalue 
> > > > > > > > > conversion.  I'm not sure `volatile` is special except that 
> > > > > > > > > we actually perform the load even in unused-value contexts.  
> > > > > > > > > Is the assumption that you've exhaustively covered all the 
> > > > > > > > > other contexts of lvalue-to-rvalue conversions whose values 
> > > > > > > > > will actually be used?  That seems questionable to me.
> > > > > > > > Yes, that was my assumption. All the other contexts where 
> > > > > > > > lvalue-to-rvalue conversion is performed and the result is used 
> > > > > > > > are already covered by other calls sites of 
> > > > > > > > `checkNonTrivialCUnion`, which informs the users that the 
> > > > > > > > struct/union is being used in an invalid context.
> > > > > > > > 
> > > > > > > > Do you have a case in mind that I didn't think of where a 
> > > > > > > > lvalue-to-rvalue conversion requires a non-trivial 
> > > > > > > > initialization/destruction/copying of a union but clang fails 
> > > > > > > > to emit any diagnostics?
> > > > > > > > 
> > > > > > > > Also I realized that lvalue-to-rvalue conversion of volatile 
> > > > > > > > types doesn't always require non-trivial destruction, so I 
> > > > > > > > think `CheckDestruct` shouldn't be set in this case.
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > void foo(U0 *a, volatile U0 *b) {
> > > > > > > >   // this doesn't require destruction.
> > > > > > > >   // this is perfectly valid if U0 is non-trivial to destruct 
> > > > > > > > but trivial to copy.
> > > > > > > >   *a = *b;  
> > > > > > > > }
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > For the same reason, I think `CheckDestruct` shouldn't be set 
> > > > > > > > for function returns (but it should be set for function 
> > > > > > > > parameters since they are destructed by the callee).
> > > > > > > There are a *lot* of places that trigger lvalue-to-rvalue 
> > > > > > > conversion.  Many of them aren't legal with structs (in C), but 
> > > > > > > I'm worried about approving a pattern with the potential to be 
> > > > > > > wrong by default just because we didn't think about some weird 
> > > > > > > case.  As an example, casts can trigger lvalue-to-rvalue 
> > > > > > > conversion; I think the only casts allowed with structs are the 
> > > > > > > identity cast and the cast to `void`, but those are indeed 
> > > > > > > allowed.  Now, a cast to `void` means the value is ignored, so we 
> > > > > > > can elide a non-volatile load in the operand, and an identity 
> > > > > > > cast isn't terminal; if the idea is that we're checking all the 
> > > > > > > *terminal* uses of a struct r-value, then we're in much more 
> > > > > > > restricted territory (and don't need to worry about things like 
> > > > > > > commas and conditional operators that can propagate values out).  
> > > > > > > But this still worries me.
> > > > > > > 
> > > > > > > I'm not sure we need to be super-pedantic about destructibility 
> > > > > > > vs. copyability in some of this checking.  It's certain possible 
> > > > > > > in C++, but I can't imagine what sort of *primitive* type would 
> > > > > > > be trivially copyable but not trivially destructible.  (The 
> > > > > > > reverse isn't true: something like a relative pointer would be 
> > > > > > > non-trivially copyable but still trivially destructible.)
> > > > > > > 
> > > > > > > Is there any way to make this check cheaper so that we can 
> > > > > > > immediately avoid any further work for types that are obviously 
> > > > > > > copyable/destructible?  All the restricted types are (possibly 
> > > > > > > arrays of) record types, right?
> > > > > > I'm not sure I fully understan

[PATCH] D64067: [X86][PPC] Support -mlong-double-64

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Sounds good to me. I had a minor suggestion, though.




Comment at: lib/Basic/Targets/PPC.cpp:468
 Opts.AltiVec = 1;
-  TargetInfo::adjust(Opts);
+  if (Opts.LongDoubleSize == 64) {
+LongDoubleWidth = LongDoubleAlign = 64;

I think it would be preferable to implement this in the base 
`TargetInfo::adjust` implementation, just to reduce code duplication. The 
driver already checks that the option makes sense for some particular target. I 
think it's actually kind of nice that the user can bypass the driver with 
`-Xclang` and try using it for some unsupported target to see what happens. It 
will probably work anyway.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64067



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

[PATCH] D62619: [analyzer][IDF] Add a control dependency calculator + a new debug checker

[Artem Dergachev via Phabricator via cfe-commits]

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

Sure!


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

https://reviews.llvm.org/D62619



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

[PATCH] D62619: [analyzer][IDF] Add a control dependency calculator + a new debug checker

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

Szelethus added a comment.

Can you formally accept then? :) (I'll address inlines before commiting of 
course!)

In D62619#1566693 , @kuhar wrote:

> The non-static-analyzer bits look good to me, I added a few nits.


Thank you! This part of the project was fear of mine, and I'm super grateful 
for all the guidance you have given!


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

https://reviews.llvm.org/D62619



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

[PATCH] D61749: [clang-tidy] initial version of readability-static-const-method

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added a comment.

Friendly ping. Are there outstanding concerns that I could address?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61749



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

[PATCH] D63954: Add lifetime categories attributes

[Matthias Gehre via Phabricator via cfe-commits]

mgehre updated this revision to Diff 207613.
mgehre added a comment.

- Make the list of attribute properties more consistent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63954

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaCXX/attr-gsl-owner-pointer.cpp

Index: clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
@@ -0,0 +1,60 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int [[gsl::Owner]] i;
+// expected-error@-1 {{'Owner' attribute cannot be applied to types}}
+void [[gsl::Owner]] f();
+// expected-error@-1 {{'Owner' attribute cannot be applied to types}}
+
+[[gsl::Owner]] void f();
+// expected-warning@-1 {{'Owner' attribute only applies to classes}}
+
+struct S {
+};
+
+S [[gsl::Owner]] Instance;
+// expected-error@-1 {{'Owner' attribute cannot be applied to types}}
+
+class [[gsl::Owner]] OwnerMissingParameter{};
+// expected-error@-1 {{'Owner' attribute takes one argument}}
+class [[gsl::Pointer]] PointerMissingParameter{};
+// expected-error@-1 {{'Pointer' attribute takes one argument}}
+
+class [[gsl::Owner(7)]] OwnerDerefNoType{};
+// expected-error@-1 {{expected a type}} expected-error@-1 {{expected ')'}}
+// expected-note@-2 {{to match this '('}}
+
+class [[gsl::Pointer("int")]] PointerDerefNoType{};
+// expected-error@-1 {{expected a type}} expected-error@-1 {{expected ')'}}
+// expected-note@-2 {{to match this '('}}
+
+class [[gsl::Owner(int)]] [[gsl::Pointer(int)]] BothOwnerPointer{};
+// expected-error@-1 {{'Pointer' and 'Owner' attributes are not compatible}}
+// expected-note@-2 {{conflicting attribute is here}}
+
+class [[gsl::Owner(int)]] [[gsl::Owner(int)]] DuplicateOwner{};
+
+class [[gsl::Pointer(int)]] [[gsl::Pointer(int)]] DuplicatePointer{};
+
+class [[gsl::Owner(void)]] OwnerVoidDerefType{};
+// expected-error@-1 {{'void' is an invalid argument to attribute 'Owner'}}
+class [[gsl::Pointer(void)]] PointerVoidDerefType{};
+// expected-error@-1 {{'void' is an invalid argument to attribute 'Pointer'}}
+
+class [[gsl::Owner(int)]] AnOwner{};
+class [[gsl::Pointer(S)]] APointer{};
+
+class AddOwnerLater {};
+class [[gsl::Owner(int)]] AddOwnerLater;
+
+class [[gsl::Pointer(int)]] AddConflictLater{};
+class [[gsl::Owner(int)]] AddConflictLater;
+// expected-error@-1 {{'Owner' and 'Pointer' attributes are not compatible}}
+// expected-note@-3 {{conflicting attribute is here}}
+
+class [[gsl::Owner(int)]] AddConflictLater2{};
+class [[gsl::Owner(float)]] AddConflictLater2;
+// expected-error@-1 {{'Owner' and 'Owner' attributes are not compatible}}
+// expected-note@-3 {{conflicting attribute is here}}
+
+class [[gsl::Owner(int)]] AddTheSameLater{};
+class [[gsl::Owner(int)]] AddTheSameLater;
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -116,8 +116,10 @@
 // CHECK-NEXT: OpenCLNoSVM (SubjectMatchRule_variable)
 // CHECK-NEXT: OptimizeNone (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: Overloadable (SubjectMatchRule_function)
+// CHECK-NEXT: Owner (SubjectMatchRule_record)
 // CHECK-NEXT: ParamTypestate (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: PassObjectSize (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: Pointer (SubjectMatchRule_record)
 // CHECK-NEXT: RenderScriptKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ReqdWorkGroupSize (SubjectMatchRule_function)
 // CHECK-NEXT: RequireConstantInit (SubjectMatchRule_variable_is_global)
Index: clang/test/AST/ast-dump-attr.cpp
===
--- clang/test/AST/ast-dump-attr.cpp
+++ clang/test/AST/ast-dump-attr.cpp
@@ -211,6 +211,15 @@
 }
 }
 
+namespace TestLifetimeCategories {
+class [[gsl::Owner(int)]] AOwner{};
+// CHECK: CXXRecordDecl{{.*}} class AOwner
+// CHECK: OwnerAttr {{.*}} int
+class [[gsl::Pointer(int)]] APointer{};
+// CHECK: CXXRecordDecl{{.*}} class APointer
+// CHECK: PointerAttr {{.*}} int
+} // namespace TestLifetimeCategories
+
 // Verify the order of attributes in the Ast. It must reflect the order
 // in the parsed source.
 int mergeAttrTest() __attribute__((deprecated)) __attribute__((warn_unused_result));
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAtt

[PATCH] D63663: [clang-doc] Add html links to references

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 207611.
DiegoAstiazaran edited the summary of this revision.
DiegoAstiazaran added a parent revision: D63857: [clang-doc] Add a structured 
HTML generator.
DiegoAstiazaran added a comment.

The links are now generated using the nodes structure introduced by D63857 
.


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

https://reviews.llvm.org/D63663

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/Generators.cpp
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/Mapper.cpp
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/Serialize.h
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp

Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -37,7 +37,7 @@
 
   bool VisitNamespaceDecl(const NamespaceDecl *D) {
 auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
+ /*File=*/"test.cpp", Public, "");
 if (I)
   EmittedInfos.emplace_back(std::move(I));
 return true;
@@ -48,7 +48,7 @@
 if (dyn_cast(D))
   return true;
 auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
+ /*File=*/"test.cpp", Public, "");
 if (I)
   EmittedInfos.emplace_back(std::move(I));
 return true;
@@ -56,7 +56,7 @@
 
   bool VisitCXXMethodDecl(const CXXMethodDecl *D) {
 auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
+ /*File=*/"test.cpp", Public, "");
 if (I)
   EmittedInfos.emplace_back(std::move(I));
 return true;
@@ -64,7 +64,7 @@
 
   bool VisitRecordDecl(const RecordDecl *D) {
 auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
+ /*File=*/"test.cpp", Public, "");
 if (I)
   EmittedInfos.emplace_back(std::move(I));
 return true;
@@ -72,7 +72,7 @@
 
   bool VisitEnumDecl(const EnumDecl *D) {
 auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
+ /*File=*/"test.cpp", Public, "");
 if (I)
   EmittedInfos.emplace_back(std::move(I));
 return true;
@@ -142,7 +142,8 @@
   E() {}
 protected:
   void ProtectedMethod();
-};)raw", 3, /*Public=*/false, Infos);
+};)raw",
+   3, /*Public=*/false, Infos);
 
   RecordInfo *E = InfoAsRecord(Infos[0].get());
   RecordInfo ExpectedE(EmptySID, "E");
Index: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -57,7 +57,8 @@
   
 OneFunction
 
-   OneFunction()
+  OneFunction(
+  )
 
   
   Enums
@@ -78,9 +79,10 @@
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
-  I.Members.emplace_back("int", "X", AccessSpecifier::AS_private);
+  I.Members.emplace_back("int", "/path/to", "X", AccessSpecifier::AS_private);
   I.TagType = TagTypeKind::TTK_Class;
-  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record);
+  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record,
+ llvm::SmallString<128>("/path/to"));
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record);
 
   I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
@@ -104,11 +106,14 @@
 Defined at line 10 of test.cpp
   
   
-Inherits from F, G
+Inherits from 
+F
+, 
+G
   
   Members
   
-private int X
+private int X
   
   Records
   
@@ -118,7 +123,8 @@
   
 OneFunction
 
-   OneFunction()
+  OneFunction(
+  )
 
   
   Enums
@@ -139,8 +145,8 @@
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
-  I.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
-  I.Params.empl

[PATCH] D63911: [clang-doc] Serialize child namespaces and records

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364963: [clang-doc] Serialize child namespaces and records 
(authored by juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63911?vs=207605&id=207612#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63911

Files:
  clang-tools-extra/trunk/clang-doc/Mapper.cpp
  clang-tools-extra/trunk/clang-doc/Serialize.cpp
  clang-tools-extra/trunk/clang-doc/Serialize.h
  clang-tools-extra/trunk/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp

Index: clang-tools-extra/trunk/unittests/clang-doc/ClangDocTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/ClangDocTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/ClangDocTest.cpp
@@ -130,11 +130,12 @@
 
   ASSERT_EQ(Expected->ChildNamespaces.size(), Actual->ChildNamespaces.size());
   for (size_t Idx = 0; Idx < Actual->ChildNamespaces.size(); ++Idx)
-EXPECT_EQ(Expected->ChildNamespaces[Idx], Actual->ChildNamespaces[Idx]);
+CheckReference(Expected->ChildNamespaces[Idx],
+   Actual->ChildNamespaces[Idx]);
 
   ASSERT_EQ(Expected->ChildRecords.size(), Actual->ChildRecords.size());
   for (size_t Idx = 0; Idx < Actual->ChildRecords.size(); ++Idx)
-EXPECT_EQ(Expected->ChildRecords[Idx], Actual->ChildRecords[Idx]);
+CheckReference(Expected->ChildRecords[Idx], Actual->ChildRecords[Idx]);
 
   ASSERT_EQ(Expected->ChildFunctions.size(), Actual->ChildFunctions.size());
   for (size_t Idx = 0; Idx < Actual->ChildFunctions.size(); ++Idx)
@@ -167,7 +168,7 @@
 
   ASSERT_EQ(Expected->ChildRecords.size(), Actual->ChildRecords.size());
   for (size_t Idx = 0; Idx < Actual->ChildRecords.size(); ++Idx)
-EXPECT_EQ(Expected->ChildRecords[Idx], Actual->ChildRecords[Idx]);
+CheckReference(Expected->ChildRecords[Idx], Actual->ChildRecords[Idx]);
 
   ASSERT_EQ(Expected->ChildFunctions.size(), Actual->ChildFunctions.size());
   for (size_t Idx = 0; Idx < Actual->ChildFunctions.size(); ++Idx)
Index: clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp
@@ -35,48 +35,30 @@
   ClangDocSerializeTestVisitor(EmittedInfoList &EmittedInfos, bool Public)
   : EmittedInfos(EmittedInfos), Public(Public) {}
 
-  bool VisitNamespaceDecl(const NamespaceDecl *D) {
+  template  bool mapDecl(const T *D) {
 auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
  /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
+if (I.first)
+  EmittedInfos.emplace_back(std::move(I.first));
+if (I.second)
+  EmittedInfos.emplace_back(std::move(I.second));
 return true;
   }
 
+  bool VisitNamespaceDecl(const NamespaceDecl *D) { return mapDecl(D); }
+
   bool VisitFunctionDecl(const FunctionDecl *D) {
 // Don't visit CXXMethodDecls twice
 if (dyn_cast(D))
   return true;
-auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
-return true;
+return mapDecl(D);
   }
 
-  bool VisitCXXMethodDecl(const CXXMethodDecl *D) {
-auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
-return true;
-  }
+  bool VisitCXXMethodDecl(const CXXMethodDecl *D) { return mapDecl(D); }
 
-  bool VisitRecordDecl(const RecordDecl *D) {
-auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
-return true;
-  }
+  bool VisitRecordDecl(const RecordDecl *D) { return mapDecl(D); }
 
-  bool VisitEnumDecl(const EnumDecl *D) {
-auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
-return true;
-  }
+  bool VisitEnumDecl(const EnumDecl *D) { return mapDecl(D); }
 };
 
 void ExtractInfosFromCode(StringRef Code, size_t NumExpectedInfos, bool Public,
@@ -101,19 +83,19 @@
 // Test serialization of namespace declarations.
 TEST(SerializeTest, emitNamespaceInfo) {
   EmittedInfoList Infos;
-  ExtractInfosFromCode("namespace A { namespace B { void f() {} } }", 3,
+  ExtractInfosFromCode("namespace A { namespace B { void f() {} } }", 5,
/*Public=*/false, Info

[clang-tools-extra] r364963 - [clang-doc] Serialize child namespaces and records

[Julie Hockett via cfe-commits]

Author: juliehockett
Date: Tue Jul  2 12:59:56 2019
New Revision: 364963

URL: http://llvm.org/viewvc/llvm-project?rev=364963&view=rev
Log:
[clang-doc] Serialize child namespaces and records

Serialization of child namespaces and records is now handled.
Namespaces can have child records and child namespaces.
Records can only have child records.

Committed on behalf of Diego Astiazarán (diegoaa...@gmail.com).

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

Modified:
clang-tools-extra/trunk/clang-doc/Mapper.cpp
clang-tools-extra/trunk/clang-doc/Serialize.cpp
clang-tools-extra/trunk/clang-doc/Serialize.h
clang-tools-extra/trunk/unittests/clang-doc/ClangDocTest.cpp
clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp

Modified: clang-tools-extra/trunk/clang-doc/Mapper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/Mapper.cpp?rev=364963&r1=364962&r2=364963&view=diff
==
--- clang-tools-extra/trunk/clang-doc/Mapper.cpp (original)
+++ clang-tools-extra/trunk/clang-doc/Mapper.cpp Tue Jul  2 12:59:56 2019
@@ -43,9 +43,12 @@ template  bool MapASTVisitor
 
   // A null in place of I indicates that the serializer is skipping this decl
   // for some reason (e.g. we're only reporting public decls).
-  if (I)
-CDCtx.ECtx->reportResult(llvm::toHex(llvm::toStringRef(I->USR)),
-   serialize::serialize(I));
+  if (I.first)
+CDCtx.ECtx->reportResult(llvm::toHex(llvm::toStringRef(I.first->USR)),
+ serialize::serialize(I.first));
+  if (I.second)
+CDCtx.ECtx->reportResult(llvm::toHex(llvm::toStringRef(I.second->USR)),
+ serialize::serialize(I.second));
   return true;
 }
 

Modified: clang-tools-extra/trunk/clang-doc/Serialize.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/Serialize.cpp?rev=364963&r1=364962&r2=364963&view=diff
==
--- clang-tools-extra/trunk/clang-doc/Serialize.cpp (original)
+++ clang-tools-extra/trunk/clang-doc/Serialize.cpp Tue Jul  2 12:59:56 2019
@@ -335,30 +335,39 @@ static void populateFunctionInfo(Functio
   parseParameters(I, D);
 }
 
-std::unique_ptr emitInfo(const NamespaceDecl *D, const FullComment *FC,
-   int LineNumber, llvm::StringRef File,
-   bool PublicOnly) {
+std::pair, std::unique_ptr>
+emitInfo(const NamespaceDecl *D, const FullComment *FC, int LineNumber,
+ llvm::StringRef File, bool PublicOnly) {
   auto I = llvm::make_unique();
   bool IsInAnonymousNamespace = false;
   populateInfo(*I, D, FC, IsInAnonymousNamespace);
   if (PublicOnly && ((IsInAnonymousNamespace || D->isAnonymousNamespace()) ||
  !isPublic(D->getAccess(), D->getLinkageInternal(
-return nullptr;
+return {};
   I->Name = D->isAnonymousNamespace()
 ? llvm::SmallString<16>("@nonymous_namespace")
 : I->Name;
-  return std::unique_ptr{std::move(I)};
+  if (I->Namespace.empty() && I->USR == SymbolID())
+return {std::unique_ptr{std::move(I)}, nullptr};
+
+  SymbolID ParentUSR = I->Namespace.empty() ? SymbolID() : I->Namespace[0].USR;
+
+  auto Parent = llvm::make_unique();
+  Parent->USR = ParentUSR;
+  Parent->ChildNamespaces.emplace_back(I->USR, I->Name, 
InfoType::IT_namespace);
+  return {std::unique_ptr{std::move(I)},
+  std::unique_ptr{std::move(Parent)}};
 }
 
-std::unique_ptr emitInfo(const RecordDecl *D, const FullComment *FC,
-   int LineNumber, llvm::StringRef File,
-   bool PublicOnly) {
+std::pair, std::unique_ptr>
+emitInfo(const RecordDecl *D, const FullComment *FC, int LineNumber,
+ llvm::StringRef File, bool PublicOnly) {
   auto I = llvm::make_unique();
   bool IsInAnonymousNamespace = false;
   populateSymbolInfo(*I, D, FC, LineNumber, File, IsInAnonymousNamespace);
   if (PublicOnly && ((IsInAnonymousNamespace ||
   !isPublic(D->getAccess(), D->getLinkageInternal()
-return nullptr;
+return {};
 
   I->TagType = D->getTagKind();
   parseFields(*I, D, PublicOnly);
@@ -369,18 +378,44 @@ std::unique_ptr emitInfo(const Rec
 }
 parseBases(*I, C);
   }
-  return std::unique_ptr{std::move(I)};
+
+  if (I->Namespace.empty()) {
+auto Parent = llvm::make_unique();
+Parent->USR = SymbolID();
+Parent->ChildRecords.emplace_back(I->USR, I->Name, InfoType::IT_record);
+return {std::unique_ptr{std::move(I)},
+std::unique_ptr{std::move(Parent)}};
+  }
+
+  switch (I->Namespace[0].RefType) {
+  case InfoType::IT_namespace: {
+auto Parent = llvm::make_unique();
+Parent->USR = I->Namespace[0].USR;
+Parent->ChildRecords.emplace_back(I->USR, I->Name, InfoType::IT_record);
+return {std::unique_ptr{std::move(I)}

[PATCH] D63911: [clang-doc] Serialize child namespaces and records

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 207605.

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

https://reviews.llvm.org/D63911

Files:
  clang-tools-extra/clang-doc/Mapper.cpp
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/Serialize.h
  clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp

Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -35,48 +35,30 @@
   ClangDocSerializeTestVisitor(EmittedInfoList &EmittedInfos, bool Public)
   : EmittedInfos(EmittedInfos), Public(Public) {}
 
-  bool VisitNamespaceDecl(const NamespaceDecl *D) {
+  template  bool mapDecl(const T *D) {
 auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
  /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
+if (I.first)
+  EmittedInfos.emplace_back(std::move(I.first));
+if (I.second)
+  EmittedInfos.emplace_back(std::move(I.second));
 return true;
   }
 
+  bool VisitNamespaceDecl(const NamespaceDecl *D) { return mapDecl(D); }
+
   bool VisitFunctionDecl(const FunctionDecl *D) {
 // Don't visit CXXMethodDecls twice
 if (dyn_cast(D))
   return true;
-auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
-return true;
+return mapDecl(D);
   }
 
-  bool VisitCXXMethodDecl(const CXXMethodDecl *D) {
-auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
-return true;
-  }
+  bool VisitCXXMethodDecl(const CXXMethodDecl *D) { return mapDecl(D); }
 
-  bool VisitRecordDecl(const RecordDecl *D) {
-auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
-return true;
-  }
+  bool VisitRecordDecl(const RecordDecl *D) { return mapDecl(D); }
 
-  bool VisitEnumDecl(const EnumDecl *D) {
-auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
-if (I)
-  EmittedInfos.emplace_back(std::move(I));
-return true;
-  }
+  bool VisitEnumDecl(const EnumDecl *D) { return mapDecl(D); }
 };
 
 void ExtractInfosFromCode(StringRef Code, size_t NumExpectedInfos, bool Public,
@@ -101,19 +83,19 @@
 // Test serialization of namespace declarations.
 TEST(SerializeTest, emitNamespaceInfo) {
   EmittedInfoList Infos;
-  ExtractInfosFromCode("namespace A { namespace B { void f() {} } }", 3,
+  ExtractInfosFromCode("namespace A { namespace B { void f() {} } }", 5,
/*Public=*/false, Infos);
 
   NamespaceInfo *A = InfoAsNamespace(Infos[0].get());
   NamespaceInfo ExpectedA(EmptySID, "A");
   CheckNamespaceInfo(&ExpectedA, A);
 
-  NamespaceInfo *B = InfoAsNamespace(Infos[1].get());
+  NamespaceInfo *B = InfoAsNamespace(Infos[2].get());
   NamespaceInfo ExpectedB(EmptySID, "B");
   ExpectedB.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
   CheckNamespaceInfo(&ExpectedB, B);
 
-  NamespaceInfo *BWithFunction = InfoAsNamespace(Infos[2].get());
+  NamespaceInfo *BWithFunction = InfoAsNamespace(Infos[4].get());
   NamespaceInfo ExpectedBWithFunction(EmptySID);
   FunctionInfo F;
   F.Name = "f";
@@ -127,7 +109,7 @@
 
 TEST(SerializeTest, emitAnonymousNamespaceInfo) {
   EmittedInfoList Infos;
-  ExtractInfosFromCode("namespace { }", 1, /*Public=*/false, Infos);
+  ExtractInfosFromCode("namespace { }", 2, /*Public=*/false, Infos);
 
   NamespaceInfo *A = InfoAsNamespace(Infos[0].get());
   NamespaceInfo ExpectedA(EmptySID);
@@ -151,7 +133,7 @@
 template <>
 void F::TemplateMethod();
 typedef struct {} G;)raw",
-   7, /*Public=*/false, Infos);
+   10, /*Public=*/false, Infos);
 
   RecordInfo *E = InfoAsRecord(Infos[0].get());
   RecordInfo ExpectedE(EmptySID, "E");
@@ -159,7 +141,7 @@
   ExpectedE.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
   CheckRecordInfo(&ExpectedE, E);
 
-  RecordInfo *RecordWithEConstructor = InfoAsRecord(Infos[1].get());
+  RecordInfo *RecordWithEConstructor = InfoAsRecord(Infos[2].get());
   RecordInfo ExpectedRecordWithEConstructor(EmptySID);
   FunctionInfo EConstructor;
   EConstructor.Name = "E";
@@ -173,7 +155,7 @@
   std::move(EConstructor));
   CheckRecordInfo(&ExpectedRecordWithEConstructor, RecordWithEConstructor);
 
-  RecordInfo *RecordWithMethod = InfoAsRecord(Infos[2].get());
+  RecordInfo *RecordWithMethod = InfoAsRecord(Infos[3].

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:71
+  SVal RetV = Call.getReturnValue();
+  Optional RetDV = RetV.getAs();
+  if (!RetDV.hasValue())

`auto` is encouraged here.


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

https://reviews.llvm.org/D63915



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

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

This is starting to look great, thanks!




Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:96
+
+  Out << '\'' << Name << "' always returns "
+  << (Value ? "true" : "false");

Let's omit the word "always" here, as we know that there are exceptions from 
this rule. This may look bad if both `Parser::Error() always returns true` and 
`Parser::Error() returns false` appear in the same report.



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:120
+<< (Value ? "true" : "false")
+<< " according to the LLVM coding standard, but it returns "
+<< (Value ? "false" : "true");

LLVM coding standard is a fairly specific document: 
https://llvm.org/docs/CodingStandards.html . It doesn't seem to say anything 
about parsers.

Let's make this much softer: `Parser::Error() returns false` and that's it.

Also given that this note always applies to inlined calls, let's move this 
logic to `checkEndFunction()`. I.e., we emit the "false" note in 
`checkEndFunction` but we emit the "true" note in `checkPostCall`.


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

https://reviews.llvm.org/D63915



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

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:86
+  std::string Name = "";
+  Name += dyn_cast(Call.getDecl())->getParent()->getName();
+  Name += "::";

Either `cast<>` or check for null.



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:116
+
+BR.markInteresting(SFC);
+

This isn't the stack frame i was talking about, but if you move this code to 
`checkEndFunction` it will be.


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

https://reviews.llvm.org/D63915



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

[PATCH] D64089: [Driver] Introduce -stdlib++-isystem

[Dan Albert via Phabricator via cfe-commits]

danalbert added a comment.

> For example, when we're building against the Android NDK, we might want to 
> use the NDK's C++ headers (which have a custom inline namespace) even if we 
> have C++ headers installed next to the driver.

Since NDK r19 the NDK libc++ headers are already installed alongside the 
driver, so this doesn't benefit that use case. The others sound useful, just 
FYI in case the NDK was the main motivating factor here :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64089



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

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

[Akira Hatanaka via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaExpr.cpp:16218
+checkNonTrivialCUnion(E->getType(), E->getExprLoc(),
+  Sema::NTCUC_LValueToRValueVolatile);
+

ahatanak wrote:
> rjmccall wrote:
> > ahatanak wrote:
> > > rjmccall wrote:
> > > > ahatanak wrote:
> > > > > rjmccall wrote:
> > > > > > ahatanak wrote:
> > > > > > > rjmccall wrote:
> > > > > > > > It looks like you're generally warning about this based on the 
> > > > > > > > specific context that forced an lvalue-to-rvalue conversion.  
> > > > > > > > I'm not sure `volatile` is special except that we actually 
> > > > > > > > perform the load even in unused-value contexts.  Is the 
> > > > > > > > assumption that you've exhaustively covered all the other 
> > > > > > > > contexts of lvalue-to-rvalue conversions whose values will 
> > > > > > > > actually be used?  That seems questionable to me.
> > > > > > > Yes, that was my assumption. All the other contexts where 
> > > > > > > lvalue-to-rvalue conversion is performed and the result is used 
> > > > > > > are already covered by other calls sites of 
> > > > > > > `checkNonTrivialCUnion`, which informs the users that the 
> > > > > > > struct/union is being used in an invalid context.
> > > > > > > 
> > > > > > > Do you have a case in mind that I didn't think of where a 
> > > > > > > lvalue-to-rvalue conversion requires a non-trivial 
> > > > > > > initialization/destruction/copying of a union but clang fails to 
> > > > > > > emit any diagnostics?
> > > > > > > 
> > > > > > > Also I realized that lvalue-to-rvalue conversion of volatile 
> > > > > > > types doesn't always require non-trivial destruction, so I think 
> > > > > > > `CheckDestruct` shouldn't be set in this case.
> > > > > > > 
> > > > > > > ```
> > > > > > > void foo(U0 *a, volatile U0 *b) {
> > > > > > >   // this doesn't require destruction.
> > > > > > >   // this is perfectly valid if U0 is non-trivial to destruct but 
> > > > > > > trivial to copy.
> > > > > > >   *a = *b;  
> > > > > > > }
> > > > > > > ```
> > > > > > > 
> > > > > > > For the same reason, I think `CheckDestruct` shouldn't be set for 
> > > > > > > function returns (but it should be set for function parameters 
> > > > > > > since they are destructed by the callee).
> > > > > > There are a *lot* of places that trigger lvalue-to-rvalue 
> > > > > > conversion.  Many of them aren't legal with structs (in C), but I'm 
> > > > > > worried about approving a pattern with the potential to be wrong by 
> > > > > > default just because we didn't think about some weird case.  As an 
> > > > > > example, casts can trigger lvalue-to-rvalue conversion; I think the 
> > > > > > only casts allowed with structs are the identity cast and the cast 
> > > > > > to `void`, but those are indeed allowed.  Now, a cast to `void` 
> > > > > > means the value is ignored, so we can elide a non-volatile load in 
> > > > > > the operand, and an identity cast isn't terminal; if the idea is 
> > > > > > that we're checking all the *terminal* uses of a struct r-value, 
> > > > > > then we're in much more restricted territory (and don't need to 
> > > > > > worry about things like commas and conditional operators that can 
> > > > > > propagate values out).  But this still worries me.
> > > > > > 
> > > > > > I'm not sure we need to be super-pedantic about destructibility vs. 
> > > > > > copyability in some of this checking.  It's certain possible in 
> > > > > > C++, but I can't imagine what sort of *primitive* type would be 
> > > > > > trivially copyable but not trivially destructible.  (The reverse 
> > > > > > isn't true: something like a relative pointer would be 
> > > > > > non-trivially copyable but still trivially destructible.)
> > > > > > 
> > > > > > Is there any way to make this check cheaper so that we can 
> > > > > > immediately avoid any further work for types that are obviously 
> > > > > > copyable/destructible?  All the restricted types are (possibly 
> > > > > > arrays of) record types, right?
> > > > > I'm not sure I fully understand what you are saying, but by 
> > > > > "cheaper", do you mean fewer and simpler rules for allowing or 
> > > > > disallowing non-trivial unions even when that might result in 
> > > > > rejecting unions used in contexts in which non-trivial 
> > > > > initialization/destruction/copying is not required? If so, we can 
> > > > > probably diagnose any lvalue-to-rvalue conversions regardless of 
> > > > > whether the source is volatile if the type is either non-trivial to 
> > > > > copy or destruct.
> > > > Sorry, that point was separate from the discussion of `volatile` and 
> > > > lvalue-to-rvalue conversions.  I mean that you're changing a lot of 
> > > > core paths in Sema, and it would be nice if we could very quickly 
> > > > decide based on the type that no restrictions apply instead of having 
> > > > to make a funct

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

[Akira Hatanaka via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaExpr.cpp:16218
+checkNonTrivialCUnion(E->getType(), E->getExprLoc(),
+  Sema::NTCUC_LValueToRValueVolatile);
+

rjmccall wrote:
> ahatanak wrote:
> > rjmccall wrote:
> > > ahatanak wrote:
> > > > rjmccall wrote:
> > > > > ahatanak wrote:
> > > > > > rjmccall wrote:
> > > > > > > It looks like you're generally warning about this based on the 
> > > > > > > specific context that forced an lvalue-to-rvalue conversion.  I'm 
> > > > > > > not sure `volatile` is special except that we actually perform 
> > > > > > > the load even in unused-value contexts.  Is the assumption that 
> > > > > > > you've exhaustively covered all the other contexts of 
> > > > > > > lvalue-to-rvalue conversions whose values will actually be used?  
> > > > > > > That seems questionable to me.
> > > > > > Yes, that was my assumption. All the other contexts where 
> > > > > > lvalue-to-rvalue conversion is performed and the result is used are 
> > > > > > already covered by other calls sites of `checkNonTrivialCUnion`, 
> > > > > > which informs the users that the struct/union is being used in an 
> > > > > > invalid context.
> > > > > > 
> > > > > > Do you have a case in mind that I didn't think of where a 
> > > > > > lvalue-to-rvalue conversion requires a non-trivial 
> > > > > > initialization/destruction/copying of a union but clang fails to 
> > > > > > emit any diagnostics?
> > > > > > 
> > > > > > Also I realized that lvalue-to-rvalue conversion of volatile types 
> > > > > > doesn't always require non-trivial destruction, so I think 
> > > > > > `CheckDestruct` shouldn't be set in this case.
> > > > > > 
> > > > > > ```
> > > > > > void foo(U0 *a, volatile U0 *b) {
> > > > > >   // this doesn't require destruction.
> > > > > >   // this is perfectly valid if U0 is non-trivial to destruct but 
> > > > > > trivial to copy.
> > > > > >   *a = *b;  
> > > > > > }
> > > > > > ```
> > > > > > 
> > > > > > For the same reason, I think `CheckDestruct` shouldn't be set for 
> > > > > > function returns (but it should be set for function parameters 
> > > > > > since they are destructed by the callee).
> > > > > There are a *lot* of places that trigger lvalue-to-rvalue conversion. 
> > > > >  Many of them aren't legal with structs (in C), but I'm worried about 
> > > > > approving a pattern with the potential to be wrong by default just 
> > > > > because we didn't think about some weird case.  As an example, casts 
> > > > > can trigger lvalue-to-rvalue conversion; I think the only casts 
> > > > > allowed with structs are the identity cast and the cast to `void`, 
> > > > > but those are indeed allowed.  Now, a cast to `void` means the value 
> > > > > is ignored, so we can elide a non-volatile load in the operand, and 
> > > > > an identity cast isn't terminal; if the idea is that we're checking 
> > > > > all the *terminal* uses of a struct r-value, then we're in much more 
> > > > > restricted territory (and don't need to worry about things like 
> > > > > commas and conditional operators that can propagate values out).  But 
> > > > > this still worries me.
> > > > > 
> > > > > I'm not sure we need to be super-pedantic about destructibility vs. 
> > > > > copyability in some of this checking.  It's certain possible in C++, 
> > > > > but I can't imagine what sort of *primitive* type would be trivially 
> > > > > copyable but not trivially destructible.  (The reverse isn't true: 
> > > > > something like a relative pointer would be non-trivially copyable but 
> > > > > still trivially destructible.)
> > > > > 
> > > > > Is there any way to make this check cheaper so that we can 
> > > > > immediately avoid any further work for types that are obviously 
> > > > > copyable/destructible?  All the restricted types are (possibly arrays 
> > > > > of) record types, right?
> > > > I'm not sure I fully understand what you are saying, but by "cheaper", 
> > > > do you mean fewer and simpler rules for allowing or disallowing 
> > > > non-trivial unions even when that might result in rejecting unions used 
> > > > in contexts in which non-trivial initialization/destruction/copying is 
> > > > not required? If so, we can probably diagnose any lvalue-to-rvalue 
> > > > conversions regardless of whether the source is volatile if the type is 
> > > > either non-trivial to copy or destruct.
> > > Sorry, that point was separate from the discussion of `volatile` and 
> > > lvalue-to-rvalue conversions.  I mean that you're changing a lot of core 
> > > paths in Sema, and it would be nice if we could very quickly decide based 
> > > on the type that no restrictions apply instead of having to make a 
> > > function call, a switch, and a bunch of other calls in order to realize 
> > > that e.g. `void*` never needs additional checking.  Currently you have a 
> > > `!CPlusPlus` check in front o

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 207600.
ahatanak added a comment.

Call `hasNonTrivialPrimitiveCStruct` to check whether the type is a non-trivial 
C struct/union before calling `checkNonTrivialCUnion`. Fix comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63753

Files:
  include/clang/AST/Type.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/AST/Type.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaType.cpp
  test/CodeGenObjC/Inputs/strong_in_union.h
  test/CodeGenObjC/strong-in-c-struct.m
  test/SemaObjC/arc-decls.m
  test/SemaObjC/non-trivial-c-union.m

Index: test/SemaObjC/non-trivial-c-union.m
===
--- /dev/null
+++ test/SemaObjC/non-trivial-c-union.m
@@ -0,0 +1,80 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -fobjc-arc -fobjc-runtime-has-weak -verify %s
+
+typedef union { // expected-note 8 {{'U0' has subobjects that are non-trivial to default-initialize}} expected-note 36 {{'U0' has subobjects that are non-trivial to destruct}} expected-note 26 {{'U0' has subobjects that are non-trivial to copy}}
+  id f0; // expected-note 8 {{f0 has type '__strong id' that is non-trivial to default-initialize}} expected-note 36 {{f0 has type '__strong id' that is non-trivial to destruct}} expected-note 26 {{f0 has type '__strong id' that is non-trivial to copy}}
+  __weak id f1; // expected-note 8 {{f1 has type '__weak id' that is non-trivial to default-initialize}} expected-note 36 {{f1 has type '__weak id' that is non-trivial to destruct}} expected-note 26 {{f1 has type '__weak id' that is non-trivial to copy}}
+} U0;
+
+typedef struct {
+  U0 f0;
+  id f1;
+} S0;
+
+id g0;
+U0 ug0;
+U0 ug1 = { .f0 = 0 };
+S0 sg0;
+S0 sg1 = { .f0 = {0}, .f1 = 0 };
+
+U0 foo0(U0); // expected-error {{cannot use type 'U0' for a function/method parameter since it is a union that is non-trivial to destruct}} expected-error {{cannot use type 'U0' for a function/method parameter since it is a union that is non-trivial to copy}} expected-error {{cannot use type 'U0' for function/method return since it is a union that is non-trivial to destruct}} expected-error {{cannot use type 'U0' for function/method return since it is a union that is non-trivial to copy}}
+S0 foo1(S0); // expected-error {{cannot use type 'S0' for a function/method parameter since it contains a union that is non-trivial to destruct}} expected-error {{cannot use type 'S0' for a function/method parameter since it contains a union that is non-trivial to copy}} expected-error {{cannot use type 'S0' for function/method return since it contains a union that is non-trivial to destruct}} expected-error {{cannot use type 'S0' for function/method return since it contains a union that is non-trivial to copy}}
+
+@interface C
+-(U0)m0:(U0)arg; // expected-error {{cannot use type 'U0' for a function/method parameter since it is a union that is non-trivial to destruct}} expected-error {{cannot use type 'U0' for a function/method parameter since it is a union that is non-trivial to copy}} expected-error {{cannot use type 'U0' for function/method return since it is a union that is non-trivial to destruct}} expected-error {{cannot use type 'U0' for function/method return since it is a union that is non-trivial to copy}}
+-(S0)m1:(S0)arg; // expected-error {{cannot use type 'S0' for a function/method parameter since it contains a union that is non-trivial to destruct}} expected-error {{cannot use type 'S0' for a function/method parameter since it contains a union that is non-trivial to copy}} expected-error {{cannot use type 'S0' for function/method return since it contains a union that is non-trivial to destruct}} expected-error {{cannot use type 'S0' for function/method return since it contains a union that is non-trivial to copy}}
+@end
+
+void testBlockFunction(void) {
+  (void)^(U0 a){ return ug0; }; // expected-error {{cannot use type 'U0' for a function/method parameter since it is a union that is non-trivial to destruct}} expected-error {{cannot use type 'U0' for a function/method parameter since it is a union that is non-trivial to copy}} expected-error {{cannot use type 'U0' for function/method return since it is a union that is non-trivial to destruct}} expected-error {{cannot use type 'U0' for function/method return since it is a union that is non-trivial to copy}}
+  (void)^(S0 a){ return sg0; }; // expected-error {{cannot use type 'S0' for a function/method parameter since it contains a union that is non-trivial to destruct}} expected-error {{cannot use type 'S0' for a function/method parameter since it contains a union that is non-trivial to copy}} expected-error {{cannot use type 'S0' for function/method return since it contains a union that is non-trivial to destruct}} expected-error {{cannot use type 'S0' for function/method return since it contains a u

[PATCH] D63845: [WIP] Create a clang attribute that lets users specify LLVM attributes

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D63845#1561983 , @lebedev.ri wrote:

> In D63845#1561793 , @jdoerfert wrote:
>
> > In D63845#1560605 , @aaron.ballman 
> > wrote:
> >
> > > In D63845#1559995 , @lebedev.ri 
> > > wrote:
> > >
> > > > What's the target use-case here? What can't be solved with normal 
> > > > attributes?
> > >
> >
> >
> > With "normal" you mean something like `__attribute__((noescape))`? We don't 
> > have them for all attributes, I doubt we want to but I might be wrong.
>
>
> That is precisely the question.
>  What is the motivation for exposing such LLVM-specific low-level unstable 
> implementation detail?


I would disagree to the unstable part. Keeping LLVM-IR backward compatibility 
is always a priority and changing the meaning of an existing LLVM-IR attribute 
will break various things already.
Why would you think it is unstable? Also, we basically do expose the low-level 
parts one by one through "normal" attributes as soon as someone has enough 
motivation to write all the necessary boilerplate (see more below). Why not 
merge all the logic/code-gen into a single uniform framework that deals with 
LLVM-IR attributes?

 I also wonder if all these should cause a clang diagnostic, at least under 
 `-Wall`.
>> 
>> What do you mean?
> 
> These are very much a generally-incompatible, very fragile, extensions.
>  They will not work on any other compiler, and will likely not work
>  on a different version of clang. It is not okay not to diagnose such cases.

It totally depends on your definition of "will not work". They might not be 
recognized and therefore be ignored, correct. They should not be misinterpreted 
as (1) the name should make them unambiguous and (2) LLVM-IR needs to be 
backward compatible anyway. Take different version of clang and 
`__attribute__((noescape))` for example, or take gcc. All the problems apply 
but it is even far less obvious that it is not a language feature and will not 
be portable (across compilers). (Given that various commercial compilers are 
build on top of LLVM/Clang there is an argument for forward compatibility.) I 
agree on the diagnose issue, we should, as we do, warn for unrecognized 
attributes. But again, both clang and gcc do that by default.

As a side node, I actually want to provide user with a 
"macro-portability-layer" that hides the spelling and even allows to remove the 
attributes for incompatible compilers.  Though, one step at a time.

 How is versioning expected to be handled? New attribute vs old clang, and 
 vice versa.
>> 
>> Unknown attributes are not necessarily a problem, they should compile fine 
>> ignoring the attribute, or am I wrong?
> 
> I don't know, that's why i'm asking.

(See above.)

In D63845#1564289 , @aaron.ballman 
wrote:

> > In D63845#1561983 , @lebedev.ri 
> > wrote:
> > 
> >> In D63845#1561793 , @jdoerfert 
> >> wrote:
> >>
> >> > In D63845#1560605 , 
> >> > @aaron.ballman wrote:
> >> >
> >> > > In D63845#1559995 , 
> >> > > @lebedev.ri wrote:
> >> > >
> >> > > > What's the target use-case here? What can't be solved with normal 
> >> > > > attributes?
> >> > >
> >> >
> >> >
> >> > With "normal" you mean something like `__attribute__((noescape))`? We 
> >> > don't have them for all attributes, I doubt we want to but I might be 
> >> > wrong.
> >>
> >>
> >> That is precisely the question.
> >>  What is the motivation for exposing such LLVM-specific low-level unstable 
> >> implementation detail?
> > 
> > 
> > There's a couple of potential use cases for this -- most of which are more 
> > for LLVM developers than end users.
>
> That's why I am hesitant to provide it as an end-user feature. I have not 
> seen a compelling case for why a generic set of user-facing attributes is a 
> good design as opposed to thinking about individual attributes LLVM exposes 
> and carefully thinking how we want to allow users access to it (if at all), 
> which is the status quo. Once we expose this sort of feature to users, they 
> *will* use it even if it isn't meant for them, and you can't unring a bell. 
> What stability guarantees do these attributes have?  How are you planning to 
> handle semantic checking of the attributes (so that attributes which only 
> appertain to some constructs are properly diagnosed when written on the wrong 
> construct)? How do you plan to handle attributes that require arguments (as 
> well as diagnosing improper argument types, etc)? How do you diagnose 
> mutually exclusive backend attributes? These sort of questions are ones that 
> all get answered when exposing individual attributes as needed, but they're 
> proble

[PATCH] D63503: cmake: Add CLANG_LINK_CLANG_DYLIB option

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

One comment inline. Otherwise LGTM.




Comment at: clang/CMakeLists.txt:327
+set(CLANG_LINK_CLANG_DYLIB ${LLVM_LINK_LLVM_DYLIB} CACHE BOOL
+"Link tools against libclang_shared.so")
+

We should generate a config error if `LLVM_LINK_LLVM_DYLIB=Off` and 
`CLANG_LINK_CLANG_DYLIB=On`, because that will cause some odd errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63503



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

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364954: [C++2a] Add __builtin_bit_cast, used to implement 
std::bit_cast (authored by epilk, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D62825?vs=206304&id=207596#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62825

Files:
  cfe/trunk/include/clang-c/Index.h
  cfe/trunk/include/clang/AST/ExprCXX.h
  cfe/trunk/include/clang/AST/OperationKinds.def
  cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
  cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/include/clang/Basic/StmtNodes.td
  cfe/trunk/include/clang/Basic/TokenKinds.def
  cfe/trunk/include/clang/Parse/Parser.h
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/AST/Expr.cpp
  cfe/trunk/lib/AST/ExprClassification.cpp
  cfe/trunk/lib/AST/ExprConstant.cpp
  cfe/trunk/lib/AST/ItaniumMangle.cpp
  cfe/trunk/lib/AST/StmtPrinter.cpp
  cfe/trunk/lib/AST/StmtProfile.cpp
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/CGExprAgg.cpp
  cfe/trunk/lib/CodeGen/CGExprComplex.cpp
  cfe/trunk/lib/CodeGen/CGExprConstant.cpp
  cfe/trunk/lib/CodeGen/CGExprScalar.cpp
  cfe/trunk/lib/Edit/RewriteObjCFoundationAPI.cpp
  cfe/trunk/lib/Lex/PPMacroExpansion.cpp
  cfe/trunk/lib/Parse/ParseExpr.cpp
  cfe/trunk/lib/Parse/ParseExprCXX.cpp
  cfe/trunk/lib/Sema/SemaCast.cpp
  cfe/trunk/lib/Sema/SemaExceptionSpec.cpp
  cfe/trunk/lib/Sema/TreeTransform.h
  cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
  cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  cfe/trunk/test/CodeGenCXX/builtin-bit-cast-no-tbaa.cpp
  cfe/trunk/test/CodeGenCXX/builtin-bit-cast.cpp
  cfe/trunk/test/SemaCXX/builtin-bit-cast.cpp
  cfe/trunk/test/SemaCXX/constexpr-builtin-bit-cast.cpp
  cfe/trunk/tools/libclang/CIndex.cpp
  cfe/trunk/tools/libclang/CXCursor.cpp
  llvm/trunk/include/llvm/ADT/APInt.h
  llvm/trunk/lib/ExecutionEngine/ExecutionEngine.cpp
  llvm/trunk/lib/Support/APInt.cpp

Index: llvm/trunk/lib/Support/APInt.cpp
===
--- llvm/trunk/lib/Support/APInt.cpp
+++ llvm/trunk/lib/Support/APInt.cpp
@@ -2934,3 +2934,56 @@
   LLVM_DEBUG(dbgs() << __func__ << ": solution (wrap): " << X << '\n');
   return X;
 }
+
+/// StoreIntToMemory - Fills the StoreBytes bytes of memory starting from Dst
+/// with the integer held in IntVal.
+void llvm::StoreIntToMemory(const APInt &IntVal, uint8_t *Dst,
+unsigned StoreBytes) {
+  assert((IntVal.getBitWidth()+7)/8 >= StoreBytes && "Integer too small!");
+  const uint8_t *Src = (const uint8_t *)IntVal.getRawData();
+
+  if (sys::IsLittleEndianHost) {
+// Little-endian host - the source is ordered from LSB to MSB.  Order the
+// destination from LSB to MSB: Do a straight copy.
+memcpy(Dst, Src, StoreBytes);
+  } else {
+// Big-endian host - the source is an array of 64 bit words ordered from
+// LSW to MSW.  Each word is ordered from MSB to LSB.  Order the destination
+// from MSB to LSB: Reverse the word order, but not the bytes in a word.
+while (StoreBytes > sizeof(uint64_t)) {
+  StoreBytes -= sizeof(uint64_t);
+  // May not be aligned so use memcpy.
+  memcpy(Dst + StoreBytes, Src, sizeof(uint64_t));
+  Src += sizeof(uint64_t);
+}
+
+memcpy(Dst, Src + sizeof(uint64_t) - StoreBytes, StoreBytes);
+  }
+}
+
+/// LoadIntFromMemory - Loads the integer stored in the LoadBytes bytes starting
+/// from Src into IntVal, which is assumed to be wide enough and to hold zero.
+void llvm::LoadIntFromMemory(APInt &IntVal, uint8_t *Src, unsigned LoadBytes) {
+  assert((IntVal.getBitWidth()+7)/8 >= LoadBytes && "Integer too small!");
+  uint8_t *Dst = reinterpret_cast(
+   const_cast(IntVal.getRawData()));
+
+  if (sys::IsLittleEndianHost)
+// Little-endian host - the destination must be ordered from LSB to MSB.
+// The source is ordered from LSB to MSB: Do a straight copy.
+memcpy(Dst, Src, LoadBytes);
+  else {
+// Big-endian - the destination is an array of 64 bit words ordered from
+// LSW to MSW.  Each word must be ordered from MSB to LSB.  The source is
+// ordered from MSB to LSB: Reverse the word order, but not the bytes in
+// a word.
+while (LoadBytes > sizeof(uint64_t)) {
+  LoadBytes -= sizeof(uint64_t);
+  // May not be aligned so use memcpy.
+  memcpy(Dst, Src + LoadBytes, sizeof(uint64_t));
+  Dst += sizeof(uint64_t);
+}
+
+memcpy(Dst + sizeof(uint64_t) - LoadBytes, Src, LoadBytes);
+  }
+}
Index: llvm/trunk/lib/ExecutionEngine/ExecutionEngine.cpp
===
--- llvm/trunk/lib/ExecutionEngine/ExecutionEngine.cpp
+++ llvm/trunk/lib/ExecutionEn

r364954 - [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Tue Jul  2 11:28:13 2019
New Revision: 364954

URL: http://llvm.org/viewvc/llvm-project?rev=364954&view=rev
Log:
[C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

This commit adds a new builtin, __builtin_bit_cast(T, v), which performs a
bit_cast from a value v to a type T. This expression can be evaluated at
compile time under specific circumstances.

The compile time evaluation currently doesn't support bit-fields, but I'm
planning on fixing this in a follow up (some of the logic for figuring this out
is in CodeGen). I'm also planning follow-ups for supporting some more esoteric
types that the constexpr evaluator supports, as well as extending
__builtin_memcpy constexpr evaluation to use the same infrastructure.

rdar://44987528

Differential revision: https://reviews.llvm.org/D62825

Added:
cfe/trunk/test/CodeGenCXX/builtin-bit-cast-no-tbaa.cpp
cfe/trunk/test/CodeGenCXX/builtin-bit-cast.cpp
cfe/trunk/test/SemaCXX/builtin-bit-cast.cpp
cfe/trunk/test/SemaCXX/constexpr-builtin-bit-cast.cpp
Modified:
cfe/trunk/include/clang-c/Index.h
cfe/trunk/include/clang/AST/ExprCXX.h
cfe/trunk/include/clang/AST/OperationKinds.def
cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/StmtNodes.td
cfe/trunk/include/clang/Basic/TokenKinds.def
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/AST/ExprClassification.cpp
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/AST/ItaniumMangle.cpp
cfe/trunk/lib/AST/StmtPrinter.cpp
cfe/trunk/lib/AST/StmtProfile.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CGExprAgg.cpp
cfe/trunk/lib/CodeGen/CGExprComplex.cpp
cfe/trunk/lib/CodeGen/CGExprConstant.cpp
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/Edit/RewriteObjCFoundationAPI.cpp
cfe/trunk/lib/Lex/PPMacroExpansion.cpp
cfe/trunk/lib/Parse/ParseExpr.cpp
cfe/trunk/lib/Parse/ParseExprCXX.cpp
cfe/trunk/lib/Sema/SemaCast.cpp
cfe/trunk/lib/Sema/SemaExceptionSpec.cpp
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
cfe/trunk/tools/libclang/CIndex.cpp
cfe/trunk/tools/libclang/CXCursor.cpp

Modified: cfe/trunk/include/clang-c/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=364954&r1=364953&r2=364954&view=diff
==
--- cfe/trunk/include/clang-c/Index.h (original)
+++ cfe/trunk/include/clang-c/Index.h Tue Jul  2 11:28:13 2019
@@ -2546,7 +2546,11 @@ enum CXCursorKind {
*/
   CXCursor_OMPTargetTeamsDistributeSimdDirective = 279,
 
-  CXCursor_LastStmt = CXCursor_OMPTargetTeamsDistributeSimdDirective,
+  /** C++2a std::bit_cast expression.
+   */
+  CXCursor_BuiltinBitCastExpr = 280,
+
+  CXCursor_LastStmt = CXCursor_BuiltinBitCastExpr,
 
   /**
* Cursor that represents the translation unit itself.

Modified: cfe/trunk/include/clang/AST/ExprCXX.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=364954&r1=364953&r2=364954&view=diff
==
--- cfe/trunk/include/clang/AST/ExprCXX.h (original)
+++ cfe/trunk/include/clang/AST/ExprCXX.h Tue Jul  2 11:28:13 2019
@@ -4716,6 +4716,35 @@ public:
   }
 };
 
+/// Represents a C++2a __builtin_bit_cast(T, v) expression. Used to implement
+/// std::bit_cast. These can sometimes be evaluated as part of a constant
+/// expression, but otherwise CodeGen to a simple memcpy in general.
+class BuiltinBitCastExpr final
+: public ExplicitCastExpr,
+  private llvm::TrailingObjects {
+  friend class ASTStmtReader;
+  friend class CastExpr;
+  friend class TrailingObjects;
+
+  SourceLocation KWLoc;
+  SourceLocation RParenLoc;
+
+public:
+  BuiltinBitCastExpr(QualType T, ExprValueKind VK, CastKind CK, Expr *SrcExpr,
+ TypeSourceInfo *DstType, SourceLocation KWLoc,
+ SourceLocation RParenLoc)
+  : ExplicitCastExpr(BuiltinBitCastExprClass, T, VK, CK, SrcExpr, 0,
+ DstType),
+KWLoc(KWLoc), RParenLoc(RParenLoc) {}
+
+  SourceLocation getBeginLoc() const LLVM_READONLY { return KWLoc; }
+  SourceLocation getEndLoc() const LLVM_READONLY { return RParenLoc; }
+
+  static bool classof(const Stmt *T) {
+return T->getStmtClass() == BuiltinBitCastExprClass;
+  }
+};
+
 } // namespace clang
 
 #endif // LLVM_CLANG_AST_EXPRCXX_H

Modified: cfe/trunk/include/clang/AST/OperationKinds.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Ope

[PATCH] D64089: [Driver] Introduce -stdlib++-isystem

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai created this revision.
smeenai added reviewers: compnerd, phosek, rsmith, zer0.
Herald added a subscriber: srhines.
Herald added a project: clang.

There are times when we wish to explicitly control the C++ standard
library search paths used by the driver. For example, when we're
building against the Android NDK, we might want to use the NDK's C++
headers (which have a custom inline namespace) even if we have C++
headers installed next to the driver. We might also be building against
a non-standard directory layout and wanting to specify the C++ standard
library include directories explicitly.

We could accomplish this by passing -nostdinc++ and adding an explicit
-isystem for our custom search directories. However, users of our
toolchain may themselves want to use -nostdinc++ and a custom C++ search
path (libc++'s build does this, for example), and our added -isystem
won't respect the -nostdinc++, leading to multiple C++ header
directories on the search path, which causes build failures.

Add a new driver option -stdlib++-isystem to support this use case.
Passing this option suppresses adding the default C++ library include
paths in the driver, and it also respects -nostdinc++ to allow users to
still override the C++ library paths themselves.

It's a bit unfortunate that we end up with both -stdlib++-isystem and
-cxx-isystem, but their semantics differ significantly. -cxx-isystem is
unaffected by -nostdinc++ and is added to the end of the search path
(which is not appropriate for C++ standard library headers, since they
often #include_next into other system headers), while -stdlib++-isystem
respects -nostdinc++, is added to the beginning of the search path, and
suppresses the default C++ library include paths.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64089

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stdlibxx-isystem.cpp

Index: clang/test/Driver/stdlibxx-isystem.cpp
===
--- /dev/null
+++ clang/test/Driver/stdlibxx-isystem.cpp
@@ -0,0 +1,48 @@
+// By default, we should search for libc++ next to the driver.
+// RUN: mkdir -p %t/bin
+// RUN: mkdir -p %t/include/c++/v1
+// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN:   -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=LIBCXX %s
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
+// RUN:   -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=LIBCXX %s
+// LIBCXX: InstalledDir: [[INSTALLDIR:.+$]]
+// LIBCXX: "-internal-isystem" "[[INSTALLDIR]]/../include/c++/v1"
+
+// Passing -stdlib++-isystem should suppress the default search.
+// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -stdlib=libc++ \
+// RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=NODEFAULT %s
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
+// RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -stdlib=libc++ \
+// RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=NODEFAULT %s
+// NODEFAULT: InstalledDir: [[INSTALLDIR:.+$]]
+// NODEFAULT-NOT: "-internal-isystem" "[[INSTALLDIR]]/../include/c++/v1"
+
+// And we should add it as an -internal-isystem.
+// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -stdlib=libc++ \
+// RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=INCPATH %s
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
+// RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -stdlib=libc++ \
+// RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=INCPATH %s
+// INCPATH: "-internal-isystem" "/tmp/foo" "-internal-isystem" "/tmp/bar"
+
+// We shouldn't pass the -stdlib++-isystem to cc1.
+// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN:   -stdlib++-isystem /tmp -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=NOCC1 %s
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
+// RUN:   -stdlib++-isystem /tmp -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=NOCC1 %s
+// NOCC1-NOT: "-stdlib++-isystem" "/tmp"
+
+// It should respect -nostdinc++.
+// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -nostdinc++ \
+// RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=NOSTDINCXX %s
+// RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
+// RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -nostdinc++ \
+// RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=NOST

[PATCH] D63962: [clang-doc] Fix segfault in comment sorting

[Julie Hockett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364949: [clang-doc] Fix segfault in comment sorting 
(authored by juliehockett, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63962?vs=207170&id=207591#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63962

Files:
  clang-tools-extra/trunk/clang-doc/Representation.h


Index: clang-tools-extra/trunk/clang-doc/Representation.h
===
--- clang-tools-extra/trunk/clang-doc/Representation.h
+++ clang-tools-extra/trunk/clang-doc/Representation.h
@@ -75,15 +75,16 @@
  Other.ParamName, Other.CloseName, Other.SelfClosing,
  Other.Explicit, Other.AttrKeys, Other.AttrValues, Other.Args);
 
-if (FirstCI < SecondCI ||
-(FirstCI == SecondCI && Children.size() < Other.Children.size()))
+if (FirstCI < SecondCI)
   return true;
 
-if (FirstCI > SecondCI || Children.size() > Other.Children.size())
-  return false;
+if (FirstCI == SecondCI) {
+  return std::lexicographical_compare(
+  Children.begin(), Children.end(), Other.Children.begin(),
+  Other.Children.end(), llvm::deref());
+}
 
-return std::equal(Children.begin(), Children.end(), Other.Children.begin(),
-  llvm::deref{});
+return false;
   }
 
   SmallString<16>


Index: clang-tools-extra/trunk/clang-doc/Representation.h
===
--- clang-tools-extra/trunk/clang-doc/Representation.h
+++ clang-tools-extra/trunk/clang-doc/Representation.h
@@ -75,15 +75,16 @@
  Other.ParamName, Other.CloseName, Other.SelfClosing,
  Other.Explicit, Other.AttrKeys, Other.AttrValues, Other.Args);
 
-if (FirstCI < SecondCI ||
-(FirstCI == SecondCI && Children.size() < Other.Children.size()))
+if (FirstCI < SecondCI)
   return true;
 
-if (FirstCI > SecondCI || Children.size() > Other.Children.size())
-  return false;
+if (FirstCI == SecondCI) {
+  return std::lexicographical_compare(
+  Children.begin(), Children.end(), Other.Children.begin(),
+  Other.Children.end(), llvm::deref());
+}
 
-return std::equal(Children.begin(), Children.end(), Other.Children.begin(),
-  llvm::deref{});
+return false;
   }
 
   SmallString<16>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r364949 - [clang-doc] Fix segfault in comment sorting

[Julie Hockett via cfe-commits]

Author: juliehockett
Date: Tue Jul  2 10:57:11 2019
New Revision: 364949

URL: http://llvm.org/viewvc/llvm-project?rev=364949&view=rev
Log:
[clang-doc] Fix segfault in comment sorting

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

Modified:
clang-tools-extra/trunk/clang-doc/Representation.h

Modified: clang-tools-extra/trunk/clang-doc/Representation.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/Representation.h?rev=364949&r1=364948&r2=364949&view=diff
==
--- clang-tools-extra/trunk/clang-doc/Representation.h (original)
+++ clang-tools-extra/trunk/clang-doc/Representation.h Tue Jul  2 10:57:11 2019
@@ -75,15 +75,16 @@ struct CommentInfo {
  Other.ParamName, Other.CloseName, Other.SelfClosing,
  Other.Explicit, Other.AttrKeys, Other.AttrValues, Other.Args);
 
-if (FirstCI < SecondCI ||
-(FirstCI == SecondCI && Children.size() < Other.Children.size()))
+if (FirstCI < SecondCI)
   return true;
 
-if (FirstCI > SecondCI || Children.size() > Other.Children.size())
-  return false;
+if (FirstCI == SecondCI) {
+  return std::lexicographical_compare(
+  Children.begin(), Children.end(), Other.Children.begin(),
+  Other.Children.end(), llvm::deref());
+}
 
-return std::equal(Children.begin(), Children.end(), Other.Children.begin(),
-  llvm::deref{});
+return false;
   }
 
   SmallString<16>


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

[PATCH] D62619: [analyzer][IDF] Add a control dependency calculator + a new debug checker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Static Analyzer bits look great to me as well!




Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:257
+  // Dumps immediate control dependencies for each block.
+  void dump() {
+CFG *cfg = PostDomTree.getCFG();

kuhar wrote:
> kuhar wrote:
> > Can `dump` be const? 
> In LLVM, `dump`s are usually annotated with the `LLVM_DUMP_METHOD` attribute 
> and not compiled in release builds. Is the convention different in the static 
> analyzer?
`LLVM_DUMP_METHOD` is great. Hiding dump methods under `#ifndef NDEBUG` is 
something i've seen very rarely. It's fairly annoying to me that exploded graph 
dumps are unavailable in release builds, but apart from that i don't have any 
immediate opinion, so this sounds like a global LLVM policy that we're 
historically not paying much attention to, but i don't mind complying.



Comment at: clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp:90
+if (AnalysisDeclContext *AC = mgr.getAnalysisDeclContext(D)) {
+  ControlDependencyCalculator dom(AC->getCFG());
+  dom.dump();

CaPiTaLiZe VaRiAbLeS.

(*doesn't really care*)


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

https://reviews.llvm.org/D62619



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

[PATCH] D59254: [RFC] Implementation of Clang randstruct

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D59254#1566895 , @shawnl wrote:

> My point is that most languages these days that intend to be compiled to 
> machine code want compatibility with the C ABI, and randstruct will be part 
> of that (and can be made compatible between languages by sharing the seed). 
> LLVM knows what a struct is.


Basic stuff, sure LLVM can know about and move fields around, especially if 
clang slaps an attribute on the struct saying it's fine.

However, struct layout is deeply ingrained in code generation. Too deeply to 
just move it to LLVM. Consider this simple example:

  template 
  struct Arr {
  int arr[Size];
  };
  
  struct A { int a, b, c; char d, e, f, g; };
  
  Arr<__builtin_offsetof(A, d)> arr;

There's way more complex stuff that falls out of how C and C++ specify the 
language. That complexity really shouldn't be in scope for this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59254



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

[PATCH] D59254: [RFC] Implementation of Clang randstruct

[Shawn Landden via Phabricator via cfe-commits]

shawnl added a comment.

My point is that most languages these days that intend to be compiled to 
machine code want compatibility with the C ABI, and randstruct will be part of 
that (and can be made compatible between languages by sharing the seed). LLVM 
knows what a struct is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59254



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

[PATCH] D64087: [clang] Correct source locations for instantiations of out-of-line defaulted special member functions. (PR25683)

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added a comment.

See PR25683 (https://bugs.llvm.org/show_bug.cgi?id=25683) for more details.  
The patch posted here differs slightly from what is posted in the PR; 
`getLocation()` is called instead of `getBeginLoc()` since the latter may 
return a customized begin location.

I believe the impact to the two tests is a desirable change; the instantiation 
note seems like it should be correlated with the definition, not the 
declaration.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64087



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

[PATCH] D64087: [clang] Correct source locations for instantiations of out-of-line defaulted special member functions. (PR25683)

[Tom Honermann via Phabricator via cfe-commits]

tahonermann created this revision.
tahonermann added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes PR25683 (https://bugs.llvm.org/show_bug.cgi?id=25683)

This change completes adjustments of source locations for FunctionDecl
definitions corresponding to function template instantiations.  Prior
changes made back in 2011 (

  "Fixed implicit instantiations source range."
  
https://github.com/llvm/llvm-project/commit/12dcbf3eaa6d2c8b9ee814ddb8bf23bef644bfaf

) updated the "inner start location" when instantiating a definition,
but for out-of-line definitions of defaulted special member functions
(functions without defined bodies), this resulted in the
"inner start location" matching the location of the out-of-line
defaulted definition, but left the end location matching the end of
the in-class declaration.  With this change, the declaration location,
"inner start location", and end location are now all adjusted to match
the FunctionDecl for the template pattern.


Repository:
  rC Clang

https://reviews.llvm.org/D64087

Files:
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/SemaCXX/member-init.cpp
  clang/test/SemaTemplate/virtual-member-functions.cpp


Index: clang/test/SemaTemplate/virtual-member-functions.cpp
===
--- clang/test/SemaTemplate/virtual-member-functions.cpp
+++ clang/test/SemaTemplate/virtual-member-functions.cpp
@@ -7,10 +7,10 @@
 
 namespace PR5557 {
 template  struct A {
-  A(); // expected-note{{instantiation}}
+  A();
   virtual int a(T x);
 };
-template A::A() {}
+template A::A() {} // expected-note{{instantiation}}
 
 template int A::a(T x) { 
   return *x; // expected-error{{requires pointer operand}}
@@ -33,10 +33,10 @@
 namespace PR5557_dtor {
 template  struct A {
   A(); // Don't have an implicit constructor.
-  ~A(); // expected-note{{instantiation}}
+  ~A();
   virtual int a(T x);
 };
-template A::~A() {}
+template A::~A() {} // expected-note{{instantiation}}
 
 template int A::a(T x) { 
   return *x; // expected-error{{requires pointer operand}}
Index: clang/test/SemaCXX/member-init.cpp
===
--- clang/test/SemaCXX/member-init.cpp
+++ clang/test/SemaCXX/member-init.cpp
@@ -164,11 +164,11 @@
 
 namespace explicit_instantiation {
 template struct X {
-  X(); // expected-note {{in instantiation of default member initializer 
'explicit_instantiation::X::n' requested here}}
+  X();
   int n = T::error; // expected-error {{type 'float' cannot be used prior to 
'::' because it has no members}}
 };
 template struct X; // ok
-template X::X() {}
+template X::X() {} // expected-note {{in instantiation of 
default member initializer 'explicit_instantiation::X::n' requested 
here}}
 template struct X; // expected-note {{in instantiation of member 
function 'explicit_instantiation::X::X' requested here}}
 }
 
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4234,8 +4234,10 @@
   // unimported module.
   Function->setVisibleDespiteOwningModule();
 
-  // Copy the inner loc start from the pattern.
+  // Copy source locations from the pattern.
+  Function->setLocation(PatternDecl->getLocation());
   Function->setInnerLocStart(PatternDecl->getInnerLocStart());
+  Function->setRangeEnd(PatternDecl->getEndLoc());
 
   EnterExpressionEvaluationContext EvalContext(
   *this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);


Index: clang/test/SemaTemplate/virtual-member-functions.cpp
===
--- clang/test/SemaTemplate/virtual-member-functions.cpp
+++ clang/test/SemaTemplate/virtual-member-functions.cpp
@@ -7,10 +7,10 @@
 
 namespace PR5557 {
 template  struct A {
-  A(); // expected-note{{instantiation}}
+  A();
   virtual int a(T x);
 };
-template A::A() {}
+template A::A() {} // expected-note{{instantiation}}
 
 template int A::a(T x) { 
   return *x; // expected-error{{requires pointer operand}}
@@ -33,10 +33,10 @@
 namespace PR5557_dtor {
 template  struct A {
   A(); // Don't have an implicit constructor.
-  ~A(); // expected-note{{instantiation}}
+  ~A();
   virtual int a(T x);
 };
-template A::~A() {}
+template A::~A() {} // expected-note{{instantiation}}
 
 template int A::a(T x) { 
   return *x; // expected-error{{requires pointer operand}}
Index: clang/test/SemaCXX/member-init.cpp
===
--- clang/test/SemaCXX/member-init.cpp
+++ clang/test/SemaCXX/member-init.cpp
@@ -164,11 +164,11 @@
 
 namespace explicit_instantiation {
 template struct X {
-  X(); // expected-note {{in instantiation of default member initializer 'explicit_instantiation::X::n' requested here}}
+  X();
   int n = T::error; //

[PATCH] D63976: Allow clang -Os and -Oz to work with -flto and lld

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

In D63976#1566236 , @ruiu wrote:

> I agree with Teresa. I don't think automatically setting O3 
>  for Os and Oz is a good idea 
> because these three options are different (that's why we have three different 
> options in the first place). Adding an Os and Oz to lld's LTO option seems 
> like a good idea, but they should be mapped to their corresponding features.


It shouldn't Matt




Comment at: llvm-9.0.0-20190629/clang/lib/Driver/ToolChains/CommonArgs.cpp:395
+  if(OOpt == "s" || OOpt == "z")
+OOpt = "3";
+}

tejohnson wrote:
> Os/Oz are closer to O2 than O3 (which allows much more aggressive code size 
> increasing optimizations).
> 
> Better though to add a size level to the LTO::Config, teach lld to pass it 
> through properly, then using the LTO Config to set the SizeLevel in the old 
> PM and the PassBuilder::OptimizationLevel in the new PM when setting up the 
> LTO backend pipelines, similar to how CodeGenLevel.OptimizeSize is handled in 
> clang (BackendUtil.cpp).
> 
> My concern is that silently mapping Os/Oz to do something different than in 
> the non-LTO pipeline is going to end up more confusing than the current error 
> (which isn't good either, but at least makes it clear that it isn't 
> supported).
Using O2 makes sense to me. 
Beyond this does it matter much? Isn't the important part of Os/Oz  carried 
through a function attribute?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63976



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

[PATCH] D43159: Modernize: Use nullptr more.

[Louis Dionne via Phabricator via cfe-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: dexonsmith.

I'm fine with this given what the author said about `nullptr` already being 
used in those headers.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D43159



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

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL364938: clang-format: Add new style option 
AlignConsecutiveMacros (authored by sammccall, committed by ).
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D28462?vs=192694&id=207572#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D28462

Files:
  cfe/trunk/docs/ClangFormatStyleOptions.rst
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/WhitespaceManager.cpp
  cfe/trunk/lib/Format/WhitespaceManager.h
  cfe/trunk/unittests/Format/FormatTest.cpp

Index: cfe/trunk/lib/Format/WhitespaceManager.h
===
--- cfe/trunk/lib/Format/WhitespaceManager.h
+++ cfe/trunk/lib/Format/WhitespaceManager.h
@@ -171,6 +171,9 @@
   /// \c EscapedNewlineColumn for the first tokens or token parts in a line.
   void calculateLineBreakInformation();
 
+  /// \brief Align consecutive C/C++ preprocessor macros over all \c Changes.
+  void alignConsecutiveMacros();
+
   /// Align consecutive assignments over all \c Changes.
   void alignConsecutiveAssignments();
 
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -341,6 +341,7 @@
 
 IO.mapOptional("AccessModifierOffset", Style.AccessModifierOffset);
 IO.mapOptional("AlignAfterOpenBracket", Style.AlignAfterOpenBracket);
+IO.mapOptional("AlignConsecutiveMacros", Style.AlignConsecutiveMacros);
 IO.mapOptional("AlignConsecutiveAssignments",
Style.AlignConsecutiveAssignments);
 IO.mapOptional("AlignConsecutiveDeclarations",
@@ -666,6 +667,7 @@
   LLVMStyle.AlignTrailingComments = true;
   LLVMStyle.AlignConsecutiveAssignments = false;
   LLVMStyle.AlignConsecutiveDeclarations = false;
+  LLVMStyle.AlignConsecutiveMacros = false;
   LLVMStyle.AllowAllArgumentsOnNextLine = true;
   LLVMStyle.AllowAllConstructorInitializersOnNextLine = true;
   LLVMStyle.AllowAllParametersOfDeclarationOnNextLine = true;
Index: cfe/trunk/lib/Format/WhitespaceManager.cpp
===
--- cfe/trunk/lib/Format/WhitespaceManager.cpp
+++ cfe/trunk/lib/Format/WhitespaceManager.cpp
@@ -91,6 +91,7 @@
 
   llvm::sort(Changes, Change::IsBeforeInFile(SourceMgr));
   calculateLineBreakInformation();
+  alignConsecutiveMacros();
   alignConsecutiveDeclarations();
   alignConsecutiveAssignments();
   alignTrailingComments();
@@ -428,6 +429,130 @@
   return i;
 }
 
+// Aligns a sequence of matching tokens, on the MinColumn column.
+//
+// Sequences start from the first matching token to align, and end at the
+// first token of the first line that doesn't need to be aligned.
+//
+// We need to adjust the StartOfTokenColumn of each Change that is on a line
+// containing any matching token to be aligned and located after such token.
+static void AlignMacroSequence(
+unsigned &StartOfSequence, unsigned &EndOfSequence, unsigned &MinColumn,
+unsigned &MaxColumn, bool &FoundMatchOnLine,
+std::function AlignMacrosMatches,
+SmallVector &Changes) {
+  if (StartOfSequence > 0 && StartOfSequence < EndOfSequence) {
+
+FoundMatchOnLine = false;
+int Shift = 0;
+
+for (unsigned I = StartOfSequence; I != EndOfSequence; ++I) {
+  if (Changes[I].NewlinesBefore > 0) {
+Shift = 0;
+FoundMatchOnLine = false;
+  }
+
+  // If this is the first matching token to be aligned, remember by how many
+  // spaces it has to be shifted, so the rest of the changes on the line are
+  // shifted by the same amount
+  if (!FoundMatchOnLine && AlignMacrosMatches(Changes[I])) {
+FoundMatchOnLine = true;
+Shift = MinColumn - Changes[I].StartOfTokenColumn;
+Changes[I].Spaces += Shift;
+  }
+
+  assert(Shift >= 0);
+  Changes[I].StartOfTokenColumn += Shift;
+  if (I + 1 != Changes.size())
+Changes[I + 1].PreviousEndOfTokenColumn += Shift;
+}
+  }
+
+  MinColumn = 0;
+  MaxColumn = UINT_MAX;
+  StartOfSequence = 0;
+  EndOfSequence = 0;
+}
+
+void WhitespaceManager::alignConsecutiveMacros() {
+  if (!Style.AlignConsecutiveMacros)
+return;
+
+  auto AlignMacrosMatches = [](const Change &C) {
+const FormatToken *Current = C.Tok;
+unsigned SpacesRequiredBefore = 1;
+
+if (Current->SpacesRequiredBefore == 0 || !Current->Previous)
+  return false;
+
+Current = Current->Previous;
+
+// If token is a ")", skip over the parameter list, to the
+// token that precedes the "("
+if (Current->is(tok::r_paren) && Current->MatchingParen) {
+  Current = Current->MatchingParen->Previous;
+  SpacesRequiredBefore = 0;
+}
+
+if (!Current || !Current->is(tok::identifier))
+  return false

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

It sounds like this is fairly low-risk (it's behind a flag that's off by 
default), has been approved, and we want it in clang-format 9, so I'm going to 
rebase and land it.

I don't understand the patch in detail so if there is significant fallout I'll 
have to revert and leave to @MyDeveloperDay and @VelocityRa.


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

https://reviews.llvm.org/D28462



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

[PATCH] D64018: [clangd] Store hash of command line in index shards.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 207578.
kadircet marked 3 inline comments as done.
kadircet added a comment.

- Use an InternedCompileCommand struct rather than a pack of StringRefs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64018

Files:
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/index/Serialization.h
  clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
  clang-tools-extra/clangd/unittests/SerializationTests.cpp

Index: clang-tools-extra/clangd/unittests/SerializationTests.cpp
===
--- clang-tools-extra/clangd/unittests/SerializationTests.cpp
+++ clang-tools-extra/clangd/unittests/SerializationTests.cpp
@@ -8,6 +8,7 @@
 
 #include "index/Index.h"
 #include "index/Serialization.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/SHA1.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "gmock/gmock.h"
@@ -240,6 +241,36 @@
   }
 }
 
+TEST(SerializationTest, CmdlTest) {
+  auto In = readIndexFile(YAML);
+  EXPECT_TRUE(bool(In)) << In.takeError();
+
+  tooling::CompileCommand Cmd;
+  Cmd.Directory = "testdir";
+  Cmd.CommandLine.push_back("cmd1");
+  Cmd.CommandLine.push_back("cmd2");
+  Cmd.Filename = "ignored";
+  Cmd.Heuristic = "ignored";
+  Cmd.Output = "ignored";
+
+  IndexFileOut Out(*In);
+  Out.Format = IndexFileFormat::RIFF;
+  Out.Cmd = &Cmd;
+  {
+std::string Serialized = llvm::to_string(Out);
+
+auto In = readIndexFile(Serialized);
+ASSERT_TRUE(bool(In)) << In.takeError();
+ASSERT_TRUE(In->Cmd);
+
+const tooling::CompileCommand &SerializedCmd = In->Cmd.getValue();
+EXPECT_EQ(SerializedCmd.CommandLine, Cmd.CommandLine);
+EXPECT_EQ(SerializedCmd.Directory, Cmd.Directory);
+EXPECT_NE(SerializedCmd.Filename, Cmd.Filename);
+EXPECT_NE(SerializedCmd.Heuristic, Cmd.Heuristic);
+EXPECT_NE(SerializedCmd.Output, Cmd.Output);
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -2,6 +2,7 @@
 #include "TestFS.h"
 #include "TestTU.h"
 #include "index/Background.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/Threading.h"
 #include "gmock/gmock.h"
@@ -526,5 +527,49 @@
"unittest:///B.h"));
 }
 
+TEST_F(BackgroundIndexTest, CmdLineHash) {
+  MockFSProvider FS;
+  llvm::StringMap Storage;
+  size_t CacheHits = 0;
+  MemoryShardStorage MSS(Storage, CacheHits);
+  OverlayCDB CDB(/*Base=*/nullptr, /*FallbackFlags=*/{},
+ /*ResourceDir=*/std::string(""));
+  BackgroundIndex Idx(Context::empty(), FS, CDB,
+  [&](llvm::StringRef) { return &MSS; });
+
+  tooling::CompileCommand Cmd;
+  FS.Files[testPath("A.cc")] = "#include \"A.h\"";
+  FS.Files[testPath("A.h")] = "";
+  Cmd.Filename = "../A.cc";
+  Cmd.Directory = testPath("build");
+  Cmd.CommandLine = {"clang++", "../A.cc", "-fsyntax-only"};
+  CDB.setCompileCommand(testPath("build/../A.cc"), Cmd);
+  ASSERT_TRUE(Idx.blockUntilIdleForTest());
+
+  EXPECT_THAT(Storage.keys(), ElementsAre(testPath("A.cc"), testPath("A.h")));
+  // Make sure we only store the Cmd for main file.
+  EXPECT_FALSE(MSS.loadShard(testPath("A.h"))->Cmd);
+
+  {
+tooling::CompileCommand CmdStored = *MSS.loadShard(testPath("A.cc"))->Cmd;
+EXPECT_EQ(CmdStored.CommandLine, Cmd.CommandLine);
+EXPECT_EQ(CmdStored.Directory, Cmd.Directory);
+  }
+
+  // FIXME: Changing compile commands should be enough to invalidate the cache.
+  FS.Files[testPath("A.cc")] = " ";
+  Cmd.CommandLine = {"clang++", "../A.cc", "-Dfoo", "-fsyntax-only"};
+  CDB.setCompileCommand(testPath("build/../A.cc"), Cmd);
+  ASSERT_TRUE(Idx.blockUntilIdleForTest());
+
+  EXPECT_FALSE(MSS.loadShard(testPath("A.h"))->Cmd);
+
+  {
+tooling::CompileCommand CmdStored = *MSS.loadShard(testPath("A.cc"))->Cmd;
+EXPECT_EQ(CmdStored.CommandLine, Cmd.CommandLine);
+EXPECT_EQ(CmdStored.Directory, Cmd.Directory);
+  }
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/index/Serialization.h
===
--- clang-tools-extra/clangd/index/Serialization.h
+++ clang-tools-extra/clangd/index/Serialization.h
@@ -27,6 +27,7 @@
 #include "Headers.h"
 #include "Index.h"
 #include "index/Symbol.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/Error.h"
 
 namespace clang {
@@ -44,6 +45,8 @@
   llvm::Optional Relations;
   // Keys are URIs of the source files.
   llvm::Optional Sources;
+  // This c

[PATCH] D59254: [RFC] Implementation of Clang randstruct

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D59254#1566711 , @shawnl wrote:

> I think the essential functionality of this patch should be in LLVM and not 
> Clang, so that all front-ends can benefit. Too many generally useful things 
> are in Clang when they should be in LLVM (e.g. C ABI for ARM and x86; ranged 
> switch statements). I opened an upstream bug to discuss this. 
> https://github.com/clang-randstruct/llvm-project/issues/52


C and C++ layout decisions definitely belong in clang, not in LLVM. There might 
be opportunity to opt certain structures into re-layout through attributes, but 
generally speaking most of it needs to stay in clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59254



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

[PATCH] D64083: [OpenCL][Sema] Improve address space support for blocks

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks! I would suggest to wait a day or two before committing just in 
case John has any feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64083



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

[PATCH] D62413: [OpenCL][PR41727] Prevent ICE on global dtors

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 207574.
Anastasia added a comment.

Added a target hook to query address spaces with the largest pointer width.


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

https://reviews.llvm.org/D62413

Files:
  lib/CodeGen/CGDeclCXX.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/CodeGen/TargetInfo.h
  test/CodeGenOpenCLCXX/atexit.cl

Index: test/CodeGenOpenCLCXX/atexit.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/atexit.cl
@@ -0,0 +1,11 @@
+//RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+
+struct S {
+  ~S(){};
+};
+S s;
+
+//CHECK-LABEL: define internal void @__cxx_global_var_init()
+//CHECK: call i32 @__cxa_atexit(void (i8*)* bitcast (void (%struct.S addrspace(4)*)* @_ZNU3AS41SD1Ev to void (i8*)*), i8* addrspacecast (i8 addrspace(1)* getelementptr inbounds (%struct.S, %struct.S addrspace(1)* @s, i32 0, i32 0) to i8*), i8 addrspace(1)* @__dso_handle)
+
+//CHECK: declare i32 @__cxa_atexit(void (i8*)*, i8*, i8 addrspace(1)*)
Index: lib/CodeGen/TargetInfo.h
===
--- lib/CodeGen/TargetInfo.h
+++ lib/CodeGen/TargetInfo.h
@@ -267,6 +267,12 @@
LangAS SrcAddr, LangAS DestAddr,
llvm::Type *DestTy) const;
 
+  /// Get address space with the largest size (pointer size in this address
+  /// space has the largest width for the target).
+  virtual LangAS getAddrSpaceWithLargestPtrSize() const {
+return LangAS::Default;
+  }
+
   /// Get the syncscope used in LLVM IR.
   virtual llvm::SyncScope::ID getLLVMSyncScopeID(const LangOptions &LangOpts,
  SyncScope Scope,
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -2284,8 +2284,19 @@
   llvm::Type *dtorTy =
 llvm::FunctionType::get(CGF.VoidTy, CGF.Int8PtrTy, false)->getPointerTo();
 
+  // Preserve address space of addr.
+  auto AddrAS = addr ? addr->getType()->getPointerAddressSpace() : 0;
+  auto AddrInt8PtrTy =
+  AddrAS ? CGF.Int8Ty->getPointerTo(AddrAS) : CGF.Int8PtrTy;
+
+  // Create a variable that binds the atexit to this shared object.
+  llvm::Constant *handle =
+  CGF.CGM.CreateRuntimeVariable(CGF.Int8Ty, "__dso_handle");
+  auto *GV = cast(handle->stripPointerCasts());
+  GV->setVisibility(llvm::GlobalValue::HiddenVisibility);
+
   // extern "C" int __cxa_atexit(void (*f)(void *), void *p, void *d);
-  llvm::Type *paramTys[] = { dtorTy, CGF.Int8PtrTy, CGF.Int8PtrTy };
+  llvm::Type *paramTys[] = {dtorTy, AddrInt8PtrTy, handle->getType()};
   llvm::FunctionType *atexitTy =
 llvm::FunctionType::get(CGF.IntTy, paramTys, false);
 
@@ -2294,12 +2305,6 @@
   if (llvm::Function *fn = dyn_cast(atexit.getCallee()))
 fn->setDoesNotThrow();
 
-  // Create a variable that binds the atexit to this shared object.
-  llvm::Constant *handle =
-  CGF.CGM.CreateRuntimeVariable(CGF.Int8Ty, "__dso_handle");
-  auto *GV = cast(handle->stripPointerCasts());
-  GV->setVisibility(llvm::GlobalValue::HiddenVisibility);
-
   if (!addr)
 // addr is null when we are trying to register a dtor annotated with
 // __attribute__((destructor)) in a constructor function. Using null here is
@@ -2309,7 +2314,7 @@
 
   llvm::Value *args[] = {llvm::ConstantExpr::getBitCast(
  cast(dtor.getCallee()), dtorTy),
- llvm::ConstantExpr::getBitCast(addr, CGF.Int8PtrTy),
+ llvm::ConstantExpr::getBitCast(addr, AddrInt8PtrTy),
  handle};
   CGF.EmitNounwindRuntimeCall(atexit, args);
 }
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -3577,8 +3577,12 @@
 llvm::Constant *
 CodeGenModule::CreateRuntimeVariable(llvm::Type *Ty,
  StringRef Name) {
-  auto *Ret =
-  GetOrCreateLLVMGlobal(Name, llvm::PointerType::getUnqual(Ty), nullptr);
+  auto PtrTy =
+  getContext().getLangOpts().OpenCL
+  ? llvm::PointerType::get(
+Ty, getContext().getTargetAddressSpace(LangAS::opencl_global))
+  : llvm::PointerType::getUnqual(Ty);
+  auto *Ret = GetOrCreateLLVMGlobal(Name, PtrTy, nullptr);
   setDSOLocal(cast(Ret->stripPointerCasts()));
   return Ret;
 }
Index: lib/CodeGen/CGDeclCXX.cpp
===
--- lib/CodeGen/CGDeclCXX.cpp
+++ lib/CodeGen/CGDeclCXX.cpp
@@ -14,6 +14,7 @@
 #include "CGCXXABI.h"
 #include "CGObjCRuntime.h"
 #include "CGOpenMPRuntime.h"
+#include "TargetInfo.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "llvm/AD

[PATCH] D63538: [analyzer][CFG] Return the correct terminator condition

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

Szelethus planned changes to this revision.
Szelethus added a comment.

Something's off. `test/Analysis/edges-new.mm` fails, but I don't remember any 
test failures at home -- I'll investigate.

Before this patch:
F9448950: image.png 
After this patch:
F9448940: image.png 


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

https://reviews.llvm.org/D63538



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

[PATCH] D64083: [OpenCL][Sema] Improve address space support for blocks

[Marco Antognini via Phabricator via cfe-commits]

mantognini created this revision.
mantognini added reviewers: rjmccall, Anastasia.
Herald added subscribers: cfe-commits, yaxunl.
Herald added a project: clang.

This patch ensures that the following code is compiled identically with
-cl-std=CL2.0 and -fblocks -cl-std=c++.

  kernel void test(void) {
void (^const block_A)(void) = ^{
  return;
};
  }

A new test is not added because cl20-device-side-enqueue.cl will cover
this once blocks are further improved for C++ for OpenCL.

The changes to Sema::PerformImplicitConversion are based on
the parts of Sema::CheckAssignmentConstraints on block pointer
conversions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64083

Files:
  clang/lib/Sema/SemaExprCXX.cpp


Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -4216,7 +4216,20 @@
 break;
 
   case ICK_Block_Pointer_Conversion: {
-From = ImpCastExprToType(From, ToType.getUnqualifiedType(), CK_BitCast,
+QualType LHSType = Context.getCanonicalType(ToType).getUnqualifiedType();
+QualType RHSType = Context.getCanonicalType(FromType).getUnqualifiedType();
+
+// Assumptions based on Sema::IsBlockPointerConversion.
+assert(isa(LHSType) && "BlockPointerType expected");
+assert(isa(RHSType) && "BlockPointerType expected");
+
+LangAS AddrSpaceL =
+LHSType->getAs()->getPointeeType().getAddressSpace();
+LangAS AddrSpaceR =
+RHSType->getAs()->getPointeeType().getAddressSpace();
+CastKind Kind =
+AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
+From = ImpCastExprToType(From, ToType.getUnqualifiedType(), Kind,
  VK_RValue, /*BasePath=*/nullptr, CCK).get();
 break;
   }


Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -4216,7 +4216,20 @@
 break;
 
   case ICK_Block_Pointer_Conversion: {
-From = ImpCastExprToType(From, ToType.getUnqualifiedType(), CK_BitCast,
+QualType LHSType = Context.getCanonicalType(ToType).getUnqualifiedType();
+QualType RHSType = Context.getCanonicalType(FromType).getUnqualifiedType();
+
+// Assumptions based on Sema::IsBlockPointerConversion.
+assert(isa(LHSType) && "BlockPointerType expected");
+assert(isa(RHSType) && "BlockPointerType expected");
+
+LangAS AddrSpaceL =
+LHSType->getAs()->getPointeeType().getAddressSpace();
+LangAS AddrSpaceR =
+RHSType->getAs()->getPointeeType().getAddressSpace();
+CastKind Kind =
+AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
+From = ImpCastExprToType(From, ToType.getUnqualifiedType(), Kind,
  VK_RValue, /*BasePath=*/nullptr, CCK).get();
 break;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64074: [OpenCL][Sema] Fix builtin rewriting

[Marco Antognini via Phabricator via cfe-commits]

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

I added the FIXME and reduced the amount of testing. Let me know if it looks 
alright.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64074



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

r364938 - clang-format: Add new style option AlignConsecutiveMacros

[Sam McCall via cfe-commits]

Author: sammccall
Date: Tue Jul  2 08:53:14 2019
New Revision: 364938

URL: http://llvm.org/viewvc/llvm-project?rev=364938&view=rev
Log:
clang-format: Add new style option AlignConsecutiveMacros

This option behaves similarly to AlignConsecutiveDeclarations and
AlignConsecutiveAssignments, aligning the assignment of C/C++
preprocessor macros on consecutive lines.

I've worked in many projects (embedded, mostly) where header files full
of large, well-aligned "#define" blocks are a common pattern. We
normally avoid using clang-format on these files, since it ruins any
existing alignment in said blocks. This style option will align "simple"
PP macros (no parameters) and PP macros with parameter lists on
consecutive lines.

Related Bugzilla entry (thanks mcuddie):
https://llvm.org/bugs/show_bug.cgi?id=20637

Patch by Nick Renieris (VelocityRa)!

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

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/WhitespaceManager.cpp
cfe/trunk/lib/Format/WhitespaceManager.h
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=364938&r1=364937&r2=364938&view=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Tue Jul  2 08:53:14 2019
@@ -192,6 +192,20 @@ the configuration (without a prefix: ``A
 
 
 
+**AlignConsecutiveMacros** (``bool``)
+  If ``true``, aligns consecutive C/C++ preprocessor macros.
+
+  This will align the C/C++ preprocessor macros of consecutive lines. This
+  will result in formattings like
+
+  .. code-block:: c++
+
+#define SHORT_NAME   42
+#define LONGER_NAME  0x007f
+#define EVEN_LONGER_NAME (2)
+#define foo(x)   (x * x)
+#define bar(y, z)(y + z)
+
 **AlignConsecutiveAssignments** (``bool``)
   If ``true``, aligns consecutive assignments.
 

Modified: cfe/trunk/include/clang/Format/Format.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=364938&r1=364937&r2=364938&view=diff
==
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Tue Jul  2 08:53:14 2019
@@ -79,6 +79,19 @@ struct FormatStyle {
   /// brackets.
   BracketAlignmentStyle AlignAfterOpenBracket;
 
+  /// \brief If ``true``, aligns consecutive C/C++ preprocessor macros.
+  ///
+  /// This will align C/C++ preprocessor macros of consecutive lines.
+  /// Will result in formattings like
+  /// \code
+  ///   #define SHORT_NAME   42
+  ///   #define LONGER_NAME  0x007f
+  ///   #define EVEN_LONGER_NAME (2)
+  ///   #define foo(x)   (x * x)
+  ///   #define bar(y, z)(y + z)
+  /// \endcode
+  bool AlignConsecutiveMacros;
+
   /// If ``true``, aligns consecutive assignments.
   ///
   /// This will align the assignment operators of consecutive lines. This

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=364938&r1=364937&r2=364938&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Tue Jul  2 08:53:14 2019
@@ -341,6 +341,7 @@ template <> struct MappingTraitshttp://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=364938&r1=364937&r2=364938&view=diff
==
--- cfe/trunk/lib/Format/WhitespaceManager.cpp (original)
+++ cfe/trunk/lib/Format/WhitespaceManager.cpp Tue Jul  2 08:53:14 2019
@@ -91,6 +91,7 @@ const tooling::Replacements &WhitespaceM
 
   llvm::sort(Changes, Change::IsBeforeInFile(SourceMgr));
   calculateLineBreakInformation();
+  alignConsecutiveMacros();
   alignConsecutiveDeclarations();
   alignConsecutiveAssignments();
   alignTrailingComments();
@@ -428,6 +429,130 @@ static unsigned AlignTokens(const Format
   return i;
 }
 
+// Aligns a sequence of matching tokens, on the MinColumn column.
+//
+// Sequences start from the first matching token to align, and end at the
+// first token of the first line that doesn't need to be aligned.
+//
+// We need to adjust the StartOfTokenColumn of each Change that is on a line
+// containing any matching token to be aligned and located after such token.
+static void AlignMacroSequence(
+unsigned &StartOfSequence, unsigned &EndOfSequence, unsigned &MinColumn,
+unsigned &MaxColumn, bool &FoundMatchOnLine,
+std::function AlignMacrosMatches,
+SmallVector &Changes) {
+  if (StartOfSequence > 0 && StartOfSequence < EndOfSequence) {
+
+FoundM

[PATCH] D64074: [OpenCL][Sema] Fix builtin rewriting

[Marco Antognini via Phabricator via cfe-commits]

mantognini updated this revision to Diff 207569.
mantognini added a comment.

Address comments: reduce testing & add FIXME.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64074

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CodeGenOpenCL/builtins.cl
  clang/test/CodeGenOpenCL/pipe_builtin.cl
  clang/test/CodeGenOpenCL/to_addr_builtin.cl


Index: clang/test/CodeGenOpenCL/to_addr_builtin.cl
===
--- clang/test/CodeGenOpenCL/to_addr_builtin.cl
+++ clang/test/CodeGenOpenCL/to_addr_builtin.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -O0 -cl-std=CL2.0 
-o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -O0 -cl-std=c++ -o 
- %s | FileCheck %s
 
 // CHECK: %[[A:.*]] = type { float, float, float }
 typedef struct {
Index: clang/test/CodeGenOpenCL/pipe_builtin.cl
===
--- clang/test/CodeGenOpenCL/pipe_builtin.cl
+++ clang/test/CodeGenOpenCL/pipe_builtin.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -emit-llvm -cl-ext=+cl_khr_subgroups -O0 -cl-std=CL2.0 -o - 
%s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -cl-ext=+cl_khr_subgroups -O0 -cl-std=c++ -o - 
%s | FileCheck %s
 
 // CHECK-DAG: %opencl.pipe_ro_t = type opaque
 // CHECK-DAG: %opencl.pipe_wo_t = type opaque
Index: clang/test/CodeGenOpenCL/builtins.cl
===
--- clang/test/CodeGenOpenCL/builtins.cl
+++ clang/test/CodeGenOpenCL/builtins.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -finclude-default-header -cl-std=CL2.0 -O0 -emit-llvm -o 
- -triple "spir-unknown-unknown" | FileCheck %s
+// RUN: %clang_cc1 %s -finclude-default-header -cl-std=c++ -fblocks -O0 
-emit-llvm -o - -triple "spir-unknown-unknown" | FileCheck %s
 
 void testBranchingOnEnqueueKernel(queue_t default_queue, unsigned flags, 
ndrange_t ndrange) {
 // Ensure `enqueue_kernel` can be branched upon.
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5360,7 +5360,7 @@
 ///  FunctionDecl is returned.
 /// TODO: Handle pointer return types.
 static FunctionDecl *rewriteBuiltinFunctionDecl(Sema *Sema, ASTContext 
&Context,
-const FunctionDecl *FDecl,
+FunctionDecl *FDecl,
 MultiExprArg ArgExprs) {
 
   QualType DeclType = FDecl->getType();
@@ -5408,7 +5408,7 @@
   FunctionProtoType::ExtProtoInfo EPI;
   QualType OverloadTy = Context.getFunctionType(FT->getReturnType(),
 OverloadParams, EPI);
-  DeclContext *Parent = Context.getTranslationUnitDecl();
+  DeclContext *Parent = FDecl->getParent();
   FunctionDecl *OverloadDecl = FunctionDecl::Create(Context, Parent,
 FDecl->getLocation(),
 FDecl->getLocation(),
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -1477,6 +1477,7 @@
 BUILTIN(__builtin_coro_end, "bv*Ib", "n")
 BUILTIN(__builtin_coro_suspend, "cIb", "n")
 BUILTIN(__builtin_coro_param, "bv*v*", "n")
+
 // OpenCL v2.0 s6.13.16, s9.17.3.5 - Pipe functions.
 // We need the generic prototype, since the packet type could be anything.
 LANGBUILTIN(read_pipe, "i.", "tn", OCLC20_LANG)
@@ -1512,6 +1513,8 @@
 LANGBUILTIN(get_kernel_sub_group_count_for_ndrange, "Ui.", "tn", OCLC20_LANG)
 
 // OpenCL v2.0 s6.13.9 - Address space qualifier functions.
+// FIXME Pointer parameters of OpenCL builtins should have their address space
+// requirement defined.
 LANGBUILTIN(to_global, "v*v*", "tn", OCLC20_LANG)
 LANGBUILTIN(to_local, "v*v*", "tn", OCLC20_LANG)
 LANGBUILTIN(to_private, "v*v*", "tn", OCLC20_LANG)


Index: clang/test/CodeGenOpenCL/to_addr_builtin.cl
===
--- clang/test/CodeGenOpenCL/to_addr_builtin.cl
+++ clang/test/CodeGenOpenCL/to_addr_builtin.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -O0 -cl-std=CL2.0 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple spir-unknown-unknown -emit-llvm -O0 -cl-std=c++ -o - %s | FileCheck %s
 
 // CHECK: %[[A:.*]] = type { float, float, float }
 typedef struct {
Index: clang/test/CodeGenOpenCL/pipe_builtin.cl
===
--- clang/test/CodeGenOpenCL/pipe_builtin.cl
+++ clang/test/CodeGenOpenCL/pipe_builtin.cl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -emit-llvm -cl-ext=+cl_khr