[clang] a1a63d6 - [clang][dataflow] Add two repros for non-convergence involving pointers in loops.

[Martin Braenne via cfe-commits]

Author: Martin Braenne
Date: 2023-08-23T07:03:16Z
New Revision: a1a63d68a46882e051eedcb632723e15f2ee331b

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

LOG: [clang][dataflow] Add two repros for non-convergence involving pointers in 
loops.

These are broken out from https://reviews.llvm.org/D156658, which it now seems 
obvious isn't the right way to solve the non-convergence.

Instead, my plan is to address the non-convergence through pointer value 
widening, but the exact way this should be implemented is TBD. In the meantime, 
I think there's value in getting these repros submitted to record the current 
undesirable behavior.

Reviewed By: ymandel, xazax.hun

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

Added: 


Modified: 
clang/unittests/Analysis/FlowSensitive/TestingSupport.h
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 




diff  --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h 
b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
index 11dc4ed76aa4fc..44dbf27a745867 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -402,8 +402,8 @@ llvm::Error checkDataflowWithNoopAnalysis(
 std::function<
 void(const llvm::StringMap> &,
  ASTContext &)>
-VerifyResults,
-DataflowAnalysisOptions Options,
+VerifyResults = [](const auto &, auto &) {},
+DataflowAnalysisOptions Options = {BuiltinOptions()},
 LangStandard::Kind Std = LangStandard::lang_cxx17,
 llvm::StringRef TargetFun = "target");
 

diff  --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 117230e14aa9e9..4c31de3c8085bd 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2667,11 +2667,7 @@ TEST(TransferTest, CannotAnalyzeFunctionTemplate) {
 void target() {}
   )";
   ASSERT_THAT_ERROR(
-  checkDataflowWithNoopAnalysis(
-  Code,
-  [](const llvm::StringMap> 
&Results,
- ASTContext &ASTCtx) {},
-  {BuiltinOptions()}),
+  checkDataflowWithNoopAnalysis(Code),
   llvm::FailedWithMessage("Cannot analyze templated declarations"));
 }
 
@@ -2683,11 +2679,7 @@ TEST(TransferTest, CannotAnalyzeMethodOfClassTemplate) {
 };
   )";
   ASSERT_THAT_ERROR(
-  checkDataflowWithNoopAnalysis(
-  Code,
-  [](const llvm::StringMap> 
&Results,
- ASTContext &ASTCtx) {},
-  {BuiltinOptions()}),
+  checkDataflowWithNoopAnalysis(Code),
   llvm::FailedWithMessage("Cannot analyze templated declarations"));
 }
 
@@ -3836,6 +3828,52 @@ TEST(TransferTest, 
LoopWithStructReferenceAssignmentConverges) {
   });
 }
 
+TEST(TransferTest, LoopDereferencingChangingPointerConverges) {
+  std::string Code = R"cc(
+bool some_condition();
+
+void target(int i1, int i2) {
+  int *p = &i1;
+  while (true) {
+(void)*p;
+if (some_condition())
+  p = &i1;
+else
+  p = &i2;
+  }
+}
+  )cc";
+  // FIXME: Implement pointer value widening to make analysis converge.
+  ASSERT_THAT_ERROR(
+  checkDataflowWithNoopAnalysis(Code),
+  llvm::FailedWithMessage("maximum number of iterations reached"));
+}
+
+TEST(TransferTest, LoopDereferencingChangingRecordPointerConverges) {
+  std::string Code = R"cc(
+struct Lookup {
+  int x;
+};
+
+bool some_condition();
+
+void target(Lookup l1, Lookup l2) {
+  Lookup *l = &l1;
+  while (true) {
+(void)l->x;
+if (some_condition())
+  l = &l1;
+else
+  l = &l2;
+  }
+}
+  )cc";
+  // FIXME: Implement pointer value widening to make analysis converge.
+  ASSERT_THAT_ERROR(
+  checkDataflowWithNoopAnalysis(Code),
+  llvm::FailedWithMessage("maximum number of iterations reached"));
+}
+
 TEST(TransferTest, DoesNotCrashOnUnionThisExpr) {
   std::string Code = R"(
 union Union {



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

[PATCH] D158513: [clang][dataflow] Add two repros for non-convergence involving pointers in loops.

[Martin Böhme via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa1a63d68a468: [clang][dataflow] Add two repros for 
non-convergence involving pointers in… (authored by mboehme).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158513

Files:
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2667,11 +2667,7 @@
 void target() {}
   )";
   ASSERT_THAT_ERROR(
-  checkDataflowWithNoopAnalysis(
-  Code,
-  [](const llvm::StringMap> 
&Results,
- ASTContext &ASTCtx) {},
-  {BuiltinOptions()}),
+  checkDataflowWithNoopAnalysis(Code),
   llvm::FailedWithMessage("Cannot analyze templated declarations"));
 }
 
@@ -2683,11 +2679,7 @@
 };
   )";
   ASSERT_THAT_ERROR(
-  checkDataflowWithNoopAnalysis(
-  Code,
-  [](const llvm::StringMap> 
&Results,
- ASTContext &ASTCtx) {},
-  {BuiltinOptions()}),
+  checkDataflowWithNoopAnalysis(Code),
   llvm::FailedWithMessage("Cannot analyze templated declarations"));
 }
 
@@ -3836,6 +3828,52 @@
   });
 }
 
+TEST(TransferTest, LoopDereferencingChangingPointerConverges) {
+  std::string Code = R"cc(
+bool some_condition();
+
+void target(int i1, int i2) {
+  int *p = &i1;
+  while (true) {
+(void)*p;
+if (some_condition())
+  p = &i1;
+else
+  p = &i2;
+  }
+}
+  )cc";
+  // FIXME: Implement pointer value widening to make analysis converge.
+  ASSERT_THAT_ERROR(
+  checkDataflowWithNoopAnalysis(Code),
+  llvm::FailedWithMessage("maximum number of iterations reached"));
+}
+
+TEST(TransferTest, LoopDereferencingChangingRecordPointerConverges) {
+  std::string Code = R"cc(
+struct Lookup {
+  int x;
+};
+
+bool some_condition();
+
+void target(Lookup l1, Lookup l2) {
+  Lookup *l = &l1;
+  while (true) {
+(void)l->x;
+if (some_condition())
+  l = &l1;
+else
+  l = &l2;
+  }
+}
+  )cc";
+  // FIXME: Implement pointer value widening to make analysis converge.
+  ASSERT_THAT_ERROR(
+  checkDataflowWithNoopAnalysis(Code),
+  llvm::FailedWithMessage("maximum number of iterations reached"));
+}
+
 TEST(TransferTest, DoesNotCrashOnUnionThisExpr) {
   std::string Code = R"(
 union Union {
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -402,8 +402,8 @@
 std::function<
 void(const llvm::StringMap> &,
  ASTContext &)>
-VerifyResults,
-DataflowAnalysisOptions Options,
+VerifyResults = [](const auto &, auto &) {},
+DataflowAnalysisOptions Options = {BuiltinOptions()},
 LangStandard::Kind Std = LangStandard::lang_cxx17,
 llvm::StringRef TargetFun = "target");
 


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2667,11 +2667,7 @@
 void target() {}
   )";
   ASSERT_THAT_ERROR(
-  checkDataflowWithNoopAnalysis(
-  Code,
-  [](const llvm::StringMap> &Results,
- ASTContext &ASTCtx) {},
-  {BuiltinOptions()}),
+  checkDataflowWithNoopAnalysis(Code),
   llvm::FailedWithMessage("Cannot analyze templated declarations"));
 }
 
@@ -2683,11 +2679,7 @@
 };
   )";
   ASSERT_THAT_ERROR(
-  checkDataflowWithNoopAnalysis(
-  Code,
-  [](const llvm::StringMap> &Results,
- ASTContext &ASTCtx) {},
-  {BuiltinOptions()}),
+  checkDataflowWithNoopAnalysis(Code),
   llvm::FailedWithMessage("Cannot analyze templated declarations"));
 }
 
@@ -3836,6 +3828,52 @@
   });
 }
 
+TEST(TransferTest, LoopDereferencingChangingPointerConverges) {
+  std::string Code = R"cc(
+bool some_condition();
+
+void target(int i1, int i2) {
+  int *p = &i1;
+  while (true) {
+(void)*p;
+if (some_condition())
+  p = &i1;
+else
+  p = &i2;
+  }
+}
+  )cc";
+  // FIXME: Implement pointer value widening to make analysis converge.
+  ASSERT_THAT_ERROR(
+  checkDataflowWithNoopAnalysis(Code),
+  llvm::FailedWithMessage("maximum number of iterations reached"));
+}
+
+T

[PATCH] D155775: [HIP][Clang][Driver][RFC] Add driver support for C++ Parallel Algorithm Offload

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/hipstdpar.c:1
+// RUN: not %clang -### -hipstdpar --compile %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=HIPSTDPAR-MISSING-LIB %s

Better to use `--hipstdpar` instead of `-hipstdpar`.

In GCC, `-h` is a Joined Separate option that accepts a value.


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

https://reviews.llvm.org/D155775

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

[PATCH] D158252: Fix regression of D157680

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/x86-no-gather-no-scatter.cpp:2
 /// Tests -mno-gather and -mno-scatter
-// RUN: %clang -c -mno-gather -### %s 2>&1 | FileCheck --check-prefix=NOGATHER 
%s
-// RUN: %clang_cl -c /Qgather- -### %s 2>&1 | FileCheck 
--check-prefix=NOGATHER %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -c -mno-gather -### %s 2>&1 | 
FileCheck --check-prefix=NOGATHER %s
+// RUN: %clang_cl --target=x86_64-windows -c /Qgather- -### %s 2>&1 | 
FileCheck --check-prefix=NOGATHER %s

This may not be worth changing now because please don't use 3.4 deprecated 
`-target ` for newer tests. And I'd expect that you have checked `--target=` as 
`%clang_cl` command uses it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158252

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

[PATCH] D158505: [clang-format] Fix weird handling of AfterColon

[Owen Pan via Phabricator via cfe-commits]

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



Comment at: clang/unittests/Format/FormatTest.cpp:7954
+  verifyFormat(
+  "Class::Class(int some, int arguments, int loonoog,\n"
+  " int more) noexcept :\n"

`lng`?



Comment at: clang/unittests/Format/FormatTest.cpp:7994-7997
   verifyFormat("Constructor(aa ,\n"
-   "aa ) :\n"
-   "aa(aa) {}",
+   "aa ) : "
+   "aa(aa) {}",
Style);

Shorten the ctor name so that we don't have to split the line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158505

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

[PATCH] D158526: [clang] Properly print unnamed members in diagnostics

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/include/clang/AST/Decl.h:3186
+
+  void printName(raw_ostream &OS, const PrintingPolicy &Policy) const override;
 };

shafik wrote:
> So it looks like w/o this we would end up using `NamedDecl::printName(...)` 
> is that right?
Right.



Comment at: clang/test/AST/ast-dump-APValue-anon-union.cpp:43
   // CHECK:  | `-VarDecl {{.*}}  col:{{.*}} u0b 'const 
U0':'const U0' constexpr listinit
-  // CHECK-NEXT:  |   |-value: Union . Union .f Float 3.141500e+00
+  // CHECK-NEXT:  |   |-value: Union .U0::(anonymous union at {{.*}}) Union .f 
Float 3.141500e+00
 

shafik wrote:
> I am curious what the rest of the diagnostic message from `anonymous union 
> at...` says.
It points to the source location of anonymous member definition, I'm seeing 
"/absolute/path/to/file/ast-dump-APValue-anon-union.cpp::20:3"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158526

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

[PATCH] D158476: [driver] Search for compatible Android runtime directories

[Stephen Hines via Phabricator via cfe-commits]

srhines added a comment.

This approach LGTM, assuming that https://reviews.llvm.org/D140925 lands as 
well to ensure that the triple does use `androideabi`, since it would prevent 
custom build setups from needing additional help.

> (Aside: why do we need to produce runtime libraries for different versions in 
> the first place? We want one for the minimum OS version we support, of 
> course, but then there's important additions like version 29 adding ELF TLS 
> support and version 31 adding a thread properties API used by LSAN, so 
> building libraries targeting those important versions and having them be 
> dynamically selected based on your target OS version is super useful. We 
> could build the runtime libraries for every single OS version we wanted to 
> support, but that's a lot of versions (at least 21 through 31, times four 
> architectures), which is wasteful for both build times and toolchain 
> distribution size.)

I don't think we need all the versions, but today we really only use 21 (lowest 
level), and 30 (a conservative level for the platform that supports all our 
apexes). Ideally we'd also pick up the latest platform version, and we probably 
should explicitly call out 29 for ELF TLS, so that would be ~4 supported 
levels, but maybe we can use 29 instead of 30 to cut it to ~3 for most 
architectures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158476

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

[PATCH] D150253: [RISCV] Add Zvfhmin extension for clang.

[Jianjian Guan via Phabricator via cfe-commits]

jacquesguan updated this revision to Diff 552622.
jacquesguan added a comment.
Herald added subscribers: wangpc, sunshaoce.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150253

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/include/clang/Support/RISCVVIntrinsicUtils.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaRISCVVectorLookup.cpp
  clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/zvfhmin-error.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/zvfhmin.c
  clang/test/Sema/riscv-vector-float16-check.c
  clang/utils/TableGen/RISCVVEmitter.cpp

Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -655,6 +655,7 @@
 for (auto RequiredFeature : RequiredFeatures) {
   RVVRequire RequireExt = StringSwitch(RequiredFeature)
   .Case("RV64", RVV_REQ_RV64)
+  .Case("ZvfhminOrZvfh", RVV_REQ_ZvfhminOrZvfh)
   .Case("Xsfvcp", RVV_REQ_Xsfvcp)
   .Default(RVV_REQ_None);
   assert(RequireExt != RVV_REQ_None && "Unrecognized required feature?");
Index: clang/test/Sema/riscv-vector-float16-check.c
===
--- clang/test/Sema/riscv-vector-float16-check.c
+++ clang/test/Sema/riscv-vector-float16-check.c
@@ -4,18 +4,18 @@
 // REQUIRES: riscv-registered-target
 #include 
 
-vfloat16m1_t foo() { /* expected-error {{RISC-V type 'vfloat16m1_t' (aka '__rvv_float16m1_t') requires the 'zvfh' extension}} */
-  vfloat16m1_t f16m1; /* expected-error {{RISC-V type 'vfloat16m1_t' (aka '__rvv_float16m1_t') requires the 'zvfh' extension}} */
+vfloat16m1_t foo() { /* expected-error {{RISC-V type 'vfloat16m1_t' (aka '__rvv_float16m1_t') requires the 'zvfh or zvfhmin' extension}} */
+  vfloat16m1_t f16m1; /* expected-error {{RISC-V type 'vfloat16m1_t' (aka '__rvv_float16m1_t') requires the 'zvfh or zvfhmin' extension}} */
 
-  (void)f16m1; /* expected-error {{RISC-V type 'vfloat16m1_t' (aka '__rvv_float16m1_t') requires the 'zvfh' extension}} */
+  (void)f16m1; /* expected-error {{RISC-V type 'vfloat16m1_t' (aka '__rvv_float16m1_t') requires the 'zvfh or zvfhmin' extension}} */
 
-  return f16m1; /* expected-error {{RISC-V type 'vfloat16m1_t' (aka '__rvv_float16m1_t') requires the 'zvfh' extension}} */
+  return f16m1; /* expected-error {{RISC-V type 'vfloat16m1_t' (aka '__rvv_float16m1_t') requires the 'zvfh or zvfhmin' extension}} */
 }
 
-vfloat16m1x2_t bar() { /* expected-error {{RISC-V type 'vfloat16m1x2_t' (aka '__rvv_float16m1x2_t') requires the 'zvfh' extension}} */
-  vfloat16m1x2_t f16m1x2; /* expected-error {{RISC-V type 'vfloat16m1x2_t' (aka '__rvv_float16m1x2_t') requires the 'zvfh' extension}} */
+vfloat16m1x2_t bar() { /* expected-error {{RISC-V type 'vfloat16m1x2_t' (aka '__rvv_float16m1x2_t') requires the 'zvfh or zvfhmin' extension}} */
+  vfloat16m1x2_t f16m1x2; /* expected-error {{RISC-V type 'vfloat16m1x2_t' (aka '__rvv_float16m1x2_t') requires the 'zvfh or zvfhmin' extension}} */
 
-  (void)f16m1x2; /* expected-error {{RISC-V type 'vfloat16m1x2_t' (aka '__rvv_float16m1x2_t') requires the 'zvfh' extension}} */
+  (void)f16m1x2; /* expected-error {{RISC-V type 'vfloat16m1x2_t' (aka '__rvv_float16m1x2_t') requires the 'zvfh or zvfhmin' extension}} */
 
-  return f16m1x2; /* expected-error {{RISC-V type 'vfloat16m1x2_t' (aka '__rvv_float16m1x2_t') requires the 'zvfh' extension}} */
+  return f16m1x2; /* expected-error {{RISC-V type 'vfloat16m1x2_t' (aka '__rvv_float16m1x2_t') requires the 'zvfh or zvfhmin' extension}} */
 }
Index: clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/zvfhmin.c
===
--- /dev/null
+++ clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/zvfhmin.c
@@ -0,0 +1,27 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// REQUIRES: riscv-registered-target
+// RUN: %clang_cc1 -triple riscv64 -target-feature +v \
+// RUN:   -target-feature +zvfhmin -disable-O0-optnone  \
+// RUN:   -emit-llvm %s -o - | opt -S -passes=mem2reg | \
+// RUN:   FileCheck --check-prefix=CHECK-ZVFHMIN %s
+
+#include 
+
+// CHECK-ZVFHMIN-LABEL: @test_vfncvt_f_f_w_f16m1(
+// CHECK-ZVFHMIN-NEXT:  entry:
+// CHECK-ZVFHMIN-NEXT:[[TMP0:%.*]] = call  @llvm.riscv.vfncvt.f.f.w.nxv4f16.nxv4f32.i64( poison,  [[SRC:%.*]], i64 [[VL:%.*]])
+// CHECK-ZVFHMIN-NEXT:ret  [[TMP0]]
+//
+vfloat16m1_t test_vfncvt_f_f_w_f16m1(vfloat32m2_t src, size_t vl) {
+  return __riscv_vfncvt_f(src, vl);
+}
+
+
+// CHECK-ZVFHMIN-LABEL: @test_vfwcvt_f_f_v_f16m1(
+// CHECK-ZVFHMIN-NEXT:  entry:
+// CHECK-ZVFHMIN-NEXT:[[TMP0:%.*]] = call  @llvm.riscv.vfwcvt.f.f.v.nxv4f32.nxv4f16.i64( poison,  [[

[clang] 7037331 - [Coroutines] [CoroElide] Don't think exceptional terminator don't leak coro handle unconditionally any more

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2023-08-23T16:51:53+08:00
New Revision: 7037331a2f05990cd59f35a7c0f6ce87c0f3cb5f

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

LOG: [Coroutines] [CoroElide] Don't think exceptional terminator don't leak 
coro handle unconditionally any more

Close https://github.com/llvm/llvm-project/issues/59723.

The fundamental cause of the above issue is that we assumed the memory
of coroutine frame can be released by stack unwinding automatically
if the allocation of the coroutine frame is elided. But we missed one
point: the stack unwinding has different semantics with the explicit
coroutine_handle<>::destroy(). Since the latter is explicit so it shows
the intention of the user. So we can blame the user to destroy the
coroutine frame incorrectly in case of use-after-free happens. But we
can't do so with stack unwinding.

So after this patch, we won't think the exceptional terminator don't
leak the coroutine handle unconditionally. Instead, we think the
exceptional terminator will leak the coroutine handle too if the
coroutine is leaked somewhere along the search path.

Concretely for C++, we can think the exceptional terminator is not
special any more. Maybe this may cause some performance regressions.
But I've tested the motivating example (std::generator). And on the
other side, the coroutine elision is a middle end opitmization and not
a language feature. So we don't think we should blame such regressions
especially we are correcting the miscompilations.

Added: 
clang/test/CodeGenCoroutines/pr59723.cpp

Modified: 
clang/test/CodeGenCoroutines/coro-halo.cpp
llvm/lib/Transforms/Coroutines/CoroElide.cpp

Removed: 




diff  --git a/clang/test/CodeGenCoroutines/coro-halo.cpp 
b/clang/test/CodeGenCoroutines/coro-halo.cpp
index 6244f130b7be23..e75bedaf81fa22 100644
--- a/clang/test/CodeGenCoroutines/coro-halo.cpp
+++ b/clang/test/CodeGenCoroutines/coro-halo.cpp
@@ -1,5 +1,7 @@
 // This tests that the coroutine heap allocation elision optimization could 
happen succesfully.
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 -O2 -emit-llvm 
%s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 -O2 -emit-llvm 
%s \
+// RUN:   -fcxx-exceptions -fexceptions -o - | FileCheck %s
 
 #include "Inputs/coroutine.h"
 #include "Inputs/numeric.h"

diff  --git a/clang/test/CodeGenCoroutines/pr59723.cpp 
b/clang/test/CodeGenCoroutines/pr59723.cpp
new file mode 100644
index 00..7fc9995f417ac3
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/pr59723.cpp
@@ -0,0 +1,237 @@
+// This is reduced test case from 
https://github.com/llvm/llvm-project/issues/59723.
+// This is not a minimal reproducer intentionally to check the compiler's 
ability.
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 
-fcxx-exceptions\
+// RUN: -fexceptions -O2 -emit-llvm %s -o - | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+// executor and operation base
+
+class bug_any_executor;
+
+struct bug_async_op_base
+{
+   void invoke();
+
+protected:
+
+   ~bug_async_op_base() = default;
+};
+
+class bug_any_executor
+{
+   using op_type = bug_async_op_base;
+
+public:
+
+   virtual ~bug_any_executor() = default;
+
+   // removing noexcept enables clang to find that the pointer has escaped
+   virtual void post(op_type& op) noexcept = 0;
+
+   virtual void wait() noexcept = 0;
+};
+
+class bug_thread_executor : public bug_any_executor
+{
+
+public:
+
+   void start()
+   {
+   
+   }
+
+   ~bug_thread_executor()
+   {
+   }
+
+   // although this implementation is not realy noexcept due to allocation 
but I have a real one that is and required to be noexcept
+   virtual void post(bug_async_op_base& op) noexcept override;
+
+   virtual void wait() noexcept override
+   {
+   
+   }
+};
+
+// task and promise
+
+struct bug_final_suspend_notification
+{
+   virtual std::coroutine_handle<> get_waiter() = 0;
+};
+
+class bug_task;
+
+class bug_task_promise
+{
+   friend bug_task;
+public:
+
+   bug_task get_return_object() noexcept;
+
+   constexpr std::suspend_always initial_suspend() noexcept { return {}; }
+
+   std::suspend_always final_suspend() noexcept 
+   {
+   return {};
+   }
+
+   void unhandled_exception() noexcept;
+
+   constexpr void return_void() const noexcept {}
+
+   void get_result() const
+   {
+   
+   }
+};
+
+template 
+T exchange(T &&t, U &&u) {
+T ret = t;
+t = u;
+return ret;
+}
+
+class bug_task
+{
+   friend bug_task_promise;
+   using handle = std::coroutine_handle<>;
+   using promise_t = bug_task_promise;
+
+  

[clang] 654fa9a - [RISCV] Add Zvfhmin extension for clang

[Jianjian GUAN via cfe-commits]

Author: Jianjian GUAN
Date: 2023-08-23T17:08:39+08:00
New Revision: 654fa9a7e898ce25a6d18f3a7c7b747e1059395b

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

LOG: [RISCV] Add Zvfhmin extension for clang

This patch adds the Zvfhmin extension for clang.

Reviewed By: craig.topper, michaelmaitland

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

Added: 
clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/zvfhmin-error.c
clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/zvfhmin.c

Modified: 
clang/include/clang/Basic/riscv_vector.td
clang/include/clang/Support/RISCVVIntrinsicUtils.h
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaRISCVVectorLookup.cpp
clang/test/Sema/riscv-vector-float16-check.c
clang/utils/TableGen/RISCVVEmitter.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/riscv_vector.td 
b/clang/include/clang/Basic/riscv_vector.td
index cf4dd8af2242de..7f0df6b729e296 100644
--- a/clang/include/clang/Basic/riscv_vector.td
+++ b/clang/include/clang/Basic/riscv_vector.td
@@ -2270,7 +2270,13 @@ let Log2LMUL = [-3, -2, -1, 0, 1, 2] in {
   def vfwcvt_rtz_x_f_v : RVVConvToWidenSignedBuiltin<"vfwcvt_rtz_x">;
   def vfwcvt_f_xu_v : RVVConvBuiltin<"Fw", "FwUv", "csi", "vfwcvt_f">;
   def vfwcvt_f_x_v : RVVConvBuiltin<"Fw", "Fwv", "csi", "vfwcvt_f">;
-  def vfwcvt_f_f_v : RVVConvBuiltin<"w", "wv", "xf", "vfwcvt_f">;
+  def vfwcvt_f_f_v : RVVConvBuiltin<"w", "wv", "f", "vfwcvt_f">;
+  let RequiredFeatures = ["ZvfhminOrZvfh"] in
+def vfwcvt_f_f_v_fp16 : RVVConvBuiltin<"w", "wv", "x", "vfwcvt_f"> {
+  let Name = "vfwcvt_f_f_v";
+  let IRName = "vfwcvt_f_f_v";
+  let MaskedIRName = "vfwcvt_f_f_v_mask";
+}
 }
 
 // 14.19. Narrowing Floating-Point/Integer Type-Convert Instructions
@@ -2360,9 +2366,11 @@ let ManualCodegen = [{
 defm :
   RVVConvBuiltinSet<"vfncvt_f_xu_w", "csi", [["Fv", "FvUwu"]]>;
   }
-  let OverloadedName = "vfncvt_f" in
-defm :
-  RVVConvBuiltinSet<"vfncvt_f_f_w", "xf", [["v", "vwu"]]>;
+  let OverloadedName = "vfncvt_f" in {
+defm : RVVConvBuiltinSet<"vfncvt_f_f_w", "f", [["v", "vwu"]]>;
+let RequiredFeatures = ["ZvfhminOrZvfh"] in
+defm : RVVConvBuiltinSet<"vfncvt_f_f_w", "x", [["v", "vwu"]]>;
+  }
 }
   }
 
@@ -2403,9 +2411,11 @@ let ManualCodegen = [{
   defm :
 RVVConvBuiltinSet<"vfncvt_f_xu_w", "csi", [["Fv", "FvUw"]]>;
 }
-let OverloadedName = "vfncvt_f" in
-  defm :
-RVVConvBuiltinSet<"vfncvt_f_f_w", "xf", [["v", "vw"]]>;
+let OverloadedName = "vfncvt_f" in {
+  defm : RVVConvBuiltinSet<"vfncvt_f_f_w", "f", [["v", "vw"]]>;
+  let RequiredFeatures = ["ZvfhminOrZvfh"] in
+  defm : RVVConvBuiltinSet<"vfncvt_f_f_w", "x", [["v", "vw"]]>;
+}
   }
 }
 }

diff  --git a/clang/include/clang/Support/RISCVVIntrinsicUtils.h 
b/clang/include/clang/Support/RISCVVIntrinsicUtils.h
index f8a7e505a1e4e4..72878368ce1a33 100644
--- a/clang/include/clang/Support/RISCVVIntrinsicUtils.h
+++ b/clang/include/clang/Support/RISCVVIntrinsicUtils.h
@@ -483,7 +483,8 @@ class RVVIntrinsic {
 enum RVVRequire : uint8_t {
   RVV_REQ_None = 0,
   RVV_REQ_RV64 = 1 << 0,
-  RVV_REQ_Xsfvcp = 1 << 1,
+  RVV_REQ_ZvfhminOrZvfh = 1 << 1,
+  RVV_REQ_Xsfvcp = 1 << 2,
 
   LLVM_MARK_AS_BITMASK_ENUM(RVV_REQ_Xsfvcp)
 };

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 660c0b55df892d..e3b4d151536528 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5506,8 +5506,9 @@ void Sema::checkRVVTypeSupport(QualType Ty, 
SourceLocation Loc, ValueDecl *D) {
   !TI.hasFeature("zve64x"))
 Diag(Loc, diag::err_riscv_type_requires_extension, D) << Ty << "zve64x";
   if (Ty->isRVVType(/* Bitwidth */ 16, /* IsFloat */ true) &&
-  !TI.hasFeature("zvfh"))
-Diag(Loc, diag::err_riscv_type_requires_extension, D) << Ty << "zvfh";
+  !TI.hasFeature("zvfh") && !TI.hasFeature("zvfhmin"))
+Diag(Loc, diag::err_riscv_type_requires_extension, D)
+<< Ty << "zvfh or zvfhmin";
   if (Ty->isRVVType(/* Bitwidth */ 32, /* IsFloat */ true) &&
   !TI.hasFeature("zve32f"))
 Diag(Loc, diag::err_riscv_type_requires_extension, D) << Ty << "zve32f";

diff  --git a/clang/lib/Sema/SemaRISCVVectorLookup.cpp 
b/clang/lib/Sema/SemaRISCVVectorLookup.cpp
index c5e076ffc70e5b..ebdd498cc7644a 100644
--- a/clang/lib/Sema/SemaRISCVVectorLookup.cpp
+++ b/clang/lib/Sema/SemaRISCVVectorLookup.cpp
@@ -262,6 +262,16 @@ void RISCVIntrinsicManagerImpl::ConstructRVVIntrinsics(
   if ((BaseTypeI & Record.TypeRangeMask) != BaseTypeI)
 continue;
 
+  if (BaseType == BasicType::Float16) {
+if ((Record.RequiredExtensions & RVV_REQ_ZvfhminOrZvfh) ==

[PATCH] D150253: [RISCV] Add Zvfhmin extension for clang

[Jianjian Guan via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG654fa9a7e898: [RISCV] Add Zvfhmin extension for clang 
(authored by jacquesguan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150253

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/include/clang/Support/RISCVVIntrinsicUtils.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaRISCVVectorLookup.cpp
  clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/zvfhmin-error.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/zvfhmin.c
  clang/test/Sema/riscv-vector-float16-check.c
  clang/utils/TableGen/RISCVVEmitter.cpp

Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -653,6 +653,7 @@
 for (auto RequiredFeature : RequiredFeatures) {
   RVVRequire RequireExt = StringSwitch(RequiredFeature)
   .Case("RV64", RVV_REQ_RV64)
+  .Case("ZvfhminOrZvfh", RVV_REQ_ZvfhminOrZvfh)
   .Case("Xsfvcp", RVV_REQ_Xsfvcp)
   .Default(RVV_REQ_None);
   assert(RequireExt != RVV_REQ_None && "Unrecognized required feature?");
Index: clang/test/Sema/riscv-vector-float16-check.c
===
--- clang/test/Sema/riscv-vector-float16-check.c
+++ clang/test/Sema/riscv-vector-float16-check.c
@@ -4,18 +4,18 @@
 // REQUIRES: riscv-registered-target
 #include 
 
-vfloat16m1_t foo() { /* expected-error {{RISC-V type 'vfloat16m1_t' (aka '__rvv_float16m1_t') requires the 'zvfh' extension}} */
-  vfloat16m1_t f16m1; /* expected-error {{RISC-V type 'vfloat16m1_t' (aka '__rvv_float16m1_t') requires the 'zvfh' extension}} */
+vfloat16m1_t foo() { /* expected-error {{RISC-V type 'vfloat16m1_t' (aka '__rvv_float16m1_t') requires the 'zvfh or zvfhmin' extension}} */
+  vfloat16m1_t f16m1; /* expected-error {{RISC-V type 'vfloat16m1_t' (aka '__rvv_float16m1_t') requires the 'zvfh or zvfhmin' extension}} */
 
-  (void)f16m1; /* expected-error {{RISC-V type 'vfloat16m1_t' (aka '__rvv_float16m1_t') requires the 'zvfh' extension}} */
+  (void)f16m1; /* expected-error {{RISC-V type 'vfloat16m1_t' (aka '__rvv_float16m1_t') requires the 'zvfh or zvfhmin' extension}} */
 
-  return f16m1; /* expected-error {{RISC-V type 'vfloat16m1_t' (aka '__rvv_float16m1_t') requires the 'zvfh' extension}} */
+  return f16m1; /* expected-error {{RISC-V type 'vfloat16m1_t' (aka '__rvv_float16m1_t') requires the 'zvfh or zvfhmin' extension}} */
 }
 
-vfloat16m1x2_t bar() { /* expected-error {{RISC-V type 'vfloat16m1x2_t' (aka '__rvv_float16m1x2_t') requires the 'zvfh' extension}} */
-  vfloat16m1x2_t f16m1x2; /* expected-error {{RISC-V type 'vfloat16m1x2_t' (aka '__rvv_float16m1x2_t') requires the 'zvfh' extension}} */
+vfloat16m1x2_t bar() { /* expected-error {{RISC-V type 'vfloat16m1x2_t' (aka '__rvv_float16m1x2_t') requires the 'zvfh or zvfhmin' extension}} */
+  vfloat16m1x2_t f16m1x2; /* expected-error {{RISC-V type 'vfloat16m1x2_t' (aka '__rvv_float16m1x2_t') requires the 'zvfh or zvfhmin' extension}} */
 
-  (void)f16m1x2; /* expected-error {{RISC-V type 'vfloat16m1x2_t' (aka '__rvv_float16m1x2_t') requires the 'zvfh' extension}} */
+  (void)f16m1x2; /* expected-error {{RISC-V type 'vfloat16m1x2_t' (aka '__rvv_float16m1x2_t') requires the 'zvfh or zvfhmin' extension}} */
 
-  return f16m1x2; /* expected-error {{RISC-V type 'vfloat16m1x2_t' (aka '__rvv_float16m1x2_t') requires the 'zvfh' extension}} */
+  return f16m1x2; /* expected-error {{RISC-V type 'vfloat16m1x2_t' (aka '__rvv_float16m1x2_t') requires the 'zvfh or zvfhmin' extension}} */
 }
Index: clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/zvfhmin.c
===
--- /dev/null
+++ clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/zvfhmin.c
@@ -0,0 +1,27 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// REQUIRES: riscv-registered-target
+// RUN: %clang_cc1 -triple riscv64 -target-feature +v \
+// RUN:   -target-feature +zvfhmin -disable-O0-optnone  \
+// RUN:   -emit-llvm %s -o - | opt -S -passes=mem2reg | \
+// RUN:   FileCheck --check-prefix=CHECK-ZVFHMIN %s
+
+#include 
+
+// CHECK-ZVFHMIN-LABEL: @test_vfncvt_f_f_w_f16m1(
+// CHECK-ZVFHMIN-NEXT:  entry:
+// CHECK-ZVFHMIN-NEXT:[[TMP0:%.*]] = call  @llvm.riscv.vfncvt.f.f.w.nxv4f16.nxv4f32.i64( poison,  [[SRC:%.*]], i64 [[VL:%.*]])
+// CHECK-ZVFHMIN-NEXT:ret  [[TMP0]]
+//
+vfloat16m1_t test_vfncvt_f_f_w_f16m1(vfloat32m2_t src, size_t vl) {
+  return __riscv_vfncvt_f(src, vl);
+}
+
+
+// CHECK-ZVFHMIN-LABEL: @test_vfwcvt_f_f_v_f16m1(
+// CHECK-ZVFHMIN-NEXT:  entry:
+// CHE

[PATCH] D157684: [clang][ASTImporter] Repeated friend templates are partially imported

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

balazske added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:4087
+
+  if (TypeSourceInfo *TSI = FD1->getFriendType())
+return Importer.IsStructurallyEquivalent(

This can be `const`?



Comment at: clang/lib/AST/ASTImporter.cpp:4105
 
-  T TypeOrDecl = GetCanTypeOrDecl(FD);
-
-  for (const FriendDecl *FoundFriend : RD->friends()) {
+  for (FriendDecl *FoundFriend : RD->friends()) {
 if (FoundFriend == FD) {

It is better if this is `const`.



Comment at: clang/lib/AST/ASTImporter.cpp:4130
   SmallVector ImportedEquivalentFriends;
-
-  while (ImportedFriend) {
-bool Match = false;
-if (D->getFriendDecl() && ImportedFriend->getFriendDecl()) {
-  Match =
-  IsStructuralMatch(D->getFriendDecl(), 
ImportedFriend->getFriendDecl(),
-/*Complain=*/false);
-} else if (D->getFriendType() && ImportedFriend->getFriendType()) {
-  Match = Importer.IsStructurallyEquivalent(
-  D->getFriendType()->getType(),
-  ImportedFriend->getFriendType()->getType(), /*Complain=*/false);
-}
-if (Match)
+  for (auto *ImportedFriend : RD->friends())
+if (IsEquivalentFriend(Importer, D, ImportedFriend))

`auto` should be replaced here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157684

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

[PATCH] D158562: [clang][Sema] Add truncation warning on fortified snprintf

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

Can't say much about the rest of the patch so I'd defer to someone else. Since 
gcc has `-Wformat-truncation`, can we disable this warning in clang as well? 
Can you add a test for that?




Comment at: clang/lib/Sema/SemaChecking.cpp:1136-1137
+  [&](const Expr *FormatExpr, StringRef &FormatStrRef, size_t &StrLen) {
+if (auto *Format = dyn_cast(FormatExpr)) {
+  if (!Format->isOrdinary() && !Format->isUTF8())
+return false;





Comment at: clang/lib/Sema/SemaChecking.cpp:1325
 DestinationSize = ComputeSizeArgument(0);
+const auto *FormatExpr = TheCall->getArg(/*Arg=*/2)->IgnoreParenImpCasts();
+StringRef FormatStrRef;

(Really not a fan of this, it adds nothing).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158562

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

[PATCH] D156054: [Clang][Sema] DR722 (nullptr and varargs) and missing -Wvarargs diagnostics

[Mital Ashok via Phabricator via cfe-commits]

MitalAshok marked an inline comment as not done.
MitalAshok added inline comments.



Comment at: clang/test/SemaCXX/varargs.cpp:34
   enum Unscoped1 { One = 0x7FFF };
-  (void)__builtin_va_arg(ap, Unscoped1); // ok
+  (void)__builtin_va_arg(ap, Unscoped1); // expected-warning {{second argument 
to 'va_arg' is of promotable type 'Unscoped1'; this va_arg has undefined 
behavior because arguments will be promoted to 'int'}}
 

MitalAshok wrote:
> Unscoped1 is promoted to int when passed to a variadic function.
> 
> The underlying type for Unscoped1 is unsigned int, so only Unscoped1 and 
> unsigned int are compatible, not Unscoped1 and int. An Unscoped1 passed to a 
> variadic function must be retrieved via va_arg(ap, int).
> 
Although I guess the warning is now wrong because even though `void f(int x, 
...) { std::va_list ap; va_start(ap, x); va_arg(ap, Unscoped1); }` `f(0, 
Unscoped1{2})` would be UB, `f(0, 2u)` would not be UB.

The user still should be warned about it, so I could create a new warning 
"second argument to 'va_arg' is of promotable enumeration type 'Unscoped1'; 
this va_arg may have undefined behavior because arguments of this enumeration 
type will be promoted to 'int', not the underlying type 'unsigned int'", and 
maybe suggest a fix `Unscoped1{va_arg(ap, unsigned)}`.

Or we could ignore it and pretend that int and enums with underlying types 
unsigned are compatible for the purposes of va_arg


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156054

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

[PATCH] D158140: WIP: [clang-repl] Basic WebAssembly support for running inside a JS engine

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

This is quite exciting! Most of it looks already good enough to me. See inline 
comment.




Comment at: clang/lib/Interpreter/WASM.cpp:79
+  int Result =
+  lld::wasm::link(LinkerArgs, llvm::outs(), llvm::errs(), false, false);
+  if (!Result)

I am not sure how we can solve that dependency here. Worst case scenario, could 
we check if `lld` is installed and make a system call?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158140

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

[clang] 6c274ba - [clang-repl] Disambiguate declarations with private typedefs

[Jonas Hahnfeld via cfe-commits]

Author: Jonas Hahnfeld
Date: 2023-08-23T11:29:26+02:00
New Revision: 6c274ba4108b07358ebd4e8d607c72d6db8c8100

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

LOG: [clang-repl] Disambiguate declarations with private typedefs

Member functions and static variable definitions may use typedefs that
are private in the global context, but fine in the class context.

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

Added: 


Modified: 
clang/lib/Parse/ParseTentative.cpp
clang/test/Interpreter/disambiguate-decl-stmt.cpp

Removed: 




diff  --git a/clang/lib/Parse/ParseTentative.cpp 
b/clang/lib/Parse/ParseTentative.cpp
index 66433705250010..45e73226d706b8 100644
--- a/clang/lib/Parse/ParseTentative.cpp
+++ b/clang/lib/Parse/ParseTentative.cpp
@@ -83,6 +83,17 @@ bool Parser::isCXXDeclarationStatement(
   isDeductionGuide,
   DeclSpec::FriendSpecified::No))
 return true;
+} else if (SS.isNotEmpty()) {
+  // If the scope is not empty, it could alternatively be something 
like
+  // a typedef or using declaration. That declaration might be private
+  // in the global context, which would be diagnosed by calling into
+  // isCXXSimpleDeclaration, but may actually be fine in the context of
+  // member functions and static variable definitions. Check if the 
next
+  // token is also an identifier and assume a declaration.
+  // We cannot check if the scopes match because the declarations could
+  // involve namespaces and friend declarations.
+  if (NextToken().is(tok::identifier))
+return true;
 }
 break;
   }

diff  --git a/clang/test/Interpreter/disambiguate-decl-stmt.cpp 
b/clang/test/Interpreter/disambiguate-decl-stmt.cpp
index cbc456da6eb512..a49d7013c540ac 100644
--- a/clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ b/clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -fincremental-extensions -std=c++20 %s
 // RUN: %clang_cc1 -fsyntax-only -DMS -fms-extensions -verify 
-fincremental-extensions -std=c++20 %s
 
-// expected-no-diagnostics
-
 extern "C" int printf(const char*,...);
 
 // Decls which are hard to disambiguate
@@ -47,6 +45,37 @@ ANestedDtor::A1::A2::~A2() { printf("Dtor 
A::A1::A2::~A2\n"); }
 
 // Ctors
 
+// Private typedefs / using declarations
+class PrivateUsingMember { using T = int; T f(); };
+PrivateUsingMember::T PrivateUsingMember::f() { return 0; }
+
+class PrivateUsingVar { using T = int; static T i; };
+PrivateUsingVar::T PrivateUsingVar::i = 42;
+
+// The same with namespaces
+namespace PrivateUsingNamespace { class Member { using T = int; T f(); }; }
+PrivateUsingNamespace::Member::T PrivateUsingNamespace::Member::f() { return 
0; }
+
+namespace PrivateUsingNamespace { class Var { using T = int; static T i; }; }
+PrivateUsingNamespace::Var::T PrivateUsingNamespace::Var::i = 42;
+
+// The same with friend declarations
+class PrivateUsingFriendMember;
+class PrivateUsingFriendVar;
+class PrivateUsingFriend { friend class PrivateUsingFriendMember; friend class 
PrivateUsingFriendVar; using T = int; };
+class PrivateUsingFriendMember { PrivateUsingFriend::T f(); };
+PrivateUsingFriend::T PrivateUsingFriendMember::f() { return 0; }
+
+class PrivateUsingFriendVar { static PrivateUsingFriend::T i; };
+PrivateUsingFriend::T PrivateUsingFriendVar::i = 42;
+
+// The following should still diagnose (inspired by PR13642)
+// FIXME: Should not be diagnosed twice!
+class PR13642 { class Inner { public: static int i; }; };
+// expected-note@-1 2 {{implicitly declared private here}}
+PR13642::Inner::i = 5;
+// expected-error@-1 2 {{'Inner' is a private member of 'PR13642'}}
+
 // Deduction guide
 template struct A { A(); A(T); };
 A() -> A;



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

[PATCH] D157838: [clang-repl] Disambiguate declarations with private typedefs

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6c274ba4108b: [clang-repl] Disambiguate declarations with 
private typedefs (authored by Hahnfeld).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157838

Files:
  clang/lib/Parse/ParseTentative.cpp
  clang/test/Interpreter/disambiguate-decl-stmt.cpp


Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -fincremental-extensions -std=c++20 %s
 // RUN: %clang_cc1 -fsyntax-only -DMS -fms-extensions -verify 
-fincremental-extensions -std=c++20 %s
 
-// expected-no-diagnostics
-
 extern "C" int printf(const char*,...);
 
 // Decls which are hard to disambiguate
@@ -47,6 +45,37 @@
 
 // Ctors
 
+// Private typedefs / using declarations
+class PrivateUsingMember { using T = int; T f(); };
+PrivateUsingMember::T PrivateUsingMember::f() { return 0; }
+
+class PrivateUsingVar { using T = int; static T i; };
+PrivateUsingVar::T PrivateUsingVar::i = 42;
+
+// The same with namespaces
+namespace PrivateUsingNamespace { class Member { using T = int; T f(); }; }
+PrivateUsingNamespace::Member::T PrivateUsingNamespace::Member::f() { return 
0; }
+
+namespace PrivateUsingNamespace { class Var { using T = int; static T i; }; }
+PrivateUsingNamespace::Var::T PrivateUsingNamespace::Var::i = 42;
+
+// The same with friend declarations
+class PrivateUsingFriendMember;
+class PrivateUsingFriendVar;
+class PrivateUsingFriend { friend class PrivateUsingFriendMember; friend class 
PrivateUsingFriendVar; using T = int; };
+class PrivateUsingFriendMember { PrivateUsingFriend::T f(); };
+PrivateUsingFriend::T PrivateUsingFriendMember::f() { return 0; }
+
+class PrivateUsingFriendVar { static PrivateUsingFriend::T i; };
+PrivateUsingFriend::T PrivateUsingFriendVar::i = 42;
+
+// The following should still diagnose (inspired by PR13642)
+// FIXME: Should not be diagnosed twice!
+class PR13642 { class Inner { public: static int i; }; };
+// expected-note@-1 2 {{implicitly declared private here}}
+PR13642::Inner::i = 5;
+// expected-error@-1 2 {{'Inner' is a private member of 'PR13642'}}
+
 // Deduction guide
 template struct A { A(); A(T); };
 A() -> A;
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -83,6 +83,17 @@
   isDeductionGuide,
   DeclSpec::FriendSpecified::No))
 return true;
+} else if (SS.isNotEmpty()) {
+  // If the scope is not empty, it could alternatively be something 
like
+  // a typedef or using declaration. That declaration might be private
+  // in the global context, which would be diagnosed by calling into
+  // isCXXSimpleDeclaration, but may actually be fine in the context of
+  // member functions and static variable definitions. Check if the 
next
+  // token is also an identifier and assume a declaration.
+  // We cannot check if the scopes match because the declarations could
+  // involve namespaces and friend declarations.
+  if (NextToken().is(tok::identifier))
+return true;
 }
 break;
   }


Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- clang/test/Interpreter/disambiguate-decl-stmt.cpp
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -fincremental-extensions -std=c++20 %s
 // RUN: %clang_cc1 -fsyntax-only -DMS -fms-extensions -verify -fincremental-extensions -std=c++20 %s
 
-// expected-no-diagnostics
-
 extern "C" int printf(const char*,...);
 
 // Decls which are hard to disambiguate
@@ -47,6 +45,37 @@
 
 // Ctors
 
+// Private typedefs / using declarations
+class PrivateUsingMember { using T = int; T f(); };
+PrivateUsingMember::T PrivateUsingMember::f() { return 0; }
+
+class PrivateUsingVar { using T = int; static T i; };
+PrivateUsingVar::T PrivateUsingVar::i = 42;
+
+// The same with namespaces
+namespace PrivateUsingNamespace { class Member { using T = int; T f(); }; }
+PrivateUsingNamespace::Member::T PrivateUsingNamespace::Member::f() { return 0; }
+
+namespace PrivateUsingNamespace { class Var { using T = int; static T i; }; }
+PrivateUsingNamespace::Var::T PrivateUsingNamespace::Var::i = 42;
+
+// The same with friend declarations
+class PrivateUsingFriendMember;
+class PrivateUsingFriendVar;
+class PrivateUsingFriend { friend class PrivateUsingFriendMember; friend class PrivateUsingFriendVar; using T = int; 

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:389
   auto *ClosingLine = AnnotatedLines.begin() + ClosingLineIndex + 1;
   auto OutdentBy = I[J]->Level - TheLine->Level;
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;

See below.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:392-393
++CompactedLine) {
 if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+  (*CompactedLine)->Level -= std::min(OutdentBy, 
(*CompactedLine)->Level);
   }

Or similar.



Comment at: clang/unittests/Format/FormatTest.cpp:4230-4237
+  verifyNoCrash("namespace ns1 { namespace ns2 { namespace ns3 {\n"
+"void func_in_ns() {\n"
+"  int res{0};\n"
+"// block for debug mode\n"
+"#ifndef NDEBUG\n"
+"#endif\n"
+"}\n"

It seems that we can reduce the test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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

[PATCH] D158591: Add support of Windows Trace Logging macros

[Richard Dzenis via Phabricator via cfe-commits]

RIscRIpt created this revision.
Herald added a subscriber: pengfei.
Herald added a project: All.
RIscRIpt requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Consider the following code:

  #include 
  #include 
  #include 
  #include 
  
  TRACELOGGING_DEFINE_PROVIDER(
  g_hMyComponentProvider,
  "SimpleTraceLoggingProvider",
  // {0205c616-cf97-5c11-9756-56a2cee02ca7}
  (0x0205c616,0xcf97,0x5c11,0x97,0x56,0x56,0xa2,0xce,0xe0,0x2c,0xa7));
  
  void test()
  {
  TraceLoggingFunction(g_hMyComponentProvider);
  }
  
  int main()
  {
  TraceLoggingRegister(g_hMyComponentProvider);
  test();
  TraceLoggingUnregister(g_hMyComponentProvider);
  }

It compiles with MSVC, but clang-cl reports an error:

  C:\Program Files (x86)\Windows 
Kits\10\\include\10.0.22621.0\\shared/TraceLoggingActivity.h(377,30): note: 
expanded from macro '_tlgThisFunctionName'
  #define _tlgThisFunctionName __FUNCTION__
   ^
  .\tl.cpp(14,5): error: cannot initialize an array element of type 'char' with 
an lvalue of type 'const char[5]'
  TraceLoggingFunction(g_hMyComponentProvider);
  ^~~~

The second commit is not needed to support above code, however, during isolated 
tests in ms_predefined_expr.cpp
I found that MSVC accepts code with constexpr, whereas clang-cl does not.
I see that in most places PredefinedExpr is supported in constant evaluation, 
so I didn't wrap my code with ``if(MicrosoftExt)``.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158591

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/Sema.cpp
  clang/test/Sema/ms_predefined_expr.cpp


Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- clang/test/Sema/ms_predefined_expr.cpp
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -168,3 +168,40 @@
 void test_char_injection(decltype(sizeof('"')), decltype(sizeof("()"))) {
   unused("" __FUNCSIG__); // expected-warning{{expansion of predefined 
identifier '__FUNCSIG__' to a string literal is a Microsoft extension}}
 }
+
+void test_in_struct_init() {
+  struct {
+char F[sizeof(__FUNCTION__)];
+  } s1 = { __FUNCTION__ }; // expected-warning{{initializing an array from a 
'__FUNCTION__' predefined identifier is a Microsoft extension}}
+
+  struct {
+char F[sizeof("F:" __FUNCTION__)]; // expected-warning{{expansion of 
predefined identifier '__FUNCTION__' to a string literal is a Microsoft 
extension}}
+  } s2 = { "F:" __FUNCTION__ }; // expected-warning{{expansion of predefined 
identifier '__FUNCTION__' to a string literal is a Microsoft extension}}
+
+  class C {
+public:
+  struct {
+char F[sizeof(__FUNCTION__)];
+  } s;
+  } c1 = { { __FUNCTION__ } }; // expected-warning{{initializing an array from 
a '__FUNCTION__' predefined identifier is a Microsoft extension}}
+}
+
+void test_in_constexpr_struct_init() {
+  struct {
+char F[sizeof(__FUNCTION__)];
+  } constexpr s1 = { __FUNCTION__ }; // expected-warning{{initializing an 
array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
+  ASSERT_EQ(__FUNCTION__, s1.F);
+
+  struct {
+char F[sizeof("F:" __FUNCTION__)]; // expected-warning{{expansion of 
predefined identifier '__FUNCTION__' to a string literal is a Microsoft 
extension}}
+  } constexpr s2 = { "F:" __FUNCTION__ }; // expected-warning{{expansion of 
predefined identifier '__FUNCTION__' to a string literal is a Microsoft 
extension}}
+  ASSERT_EQ("F:" __FUNCTION__, s2.F); // expected-warning{{expansion of 
predefined identifier '__FUNCTION__' to a string literal is a Microsoft 
extension}}
+
+  class C {
+public:
+  struct {
+char F[sizeof("F:" __FUNCTION__)]; // expected-warning{{expansion of 
predefined identifier '__FUNCTION__' to a string literal is a Microsoft 
extension}}
+  } s;
+  } constexpr c1 = { { "F:" __FUNCTION__ } }; // expected-warning{{expansion 
of predefined identifier '__FUNCTION__' to a string literal is a Microsoft 
extension}}
+  ASSERT_EQ("F:" __FUNCTION__, c1.s.F); // expected-warning{{expansion of 
predefined identifier '__FUNCTION__' to a string literal is a Microsoft 
extension}}
+}
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1492,15 +1492,11 @@
 }
 
 Decl *Sema::getCurLocalScopeDecl() {
-  if (const BlockScopeInfo *BSI = getCurBlock())
-return BSI->TheDecl;
-  if (const LambdaScopeInfo *LSI = getCurLambda())
-return LSI->CallOperator;
-  if (const CapturedRegionScopeInfo *CSI = getCurCapturedRegion())
-return CSI->TheCapturedDecl;
-  if (NamedDecl *ND = getCurFunctionOrMethodDecl())
-return ND;
-  return nullptr;
+  DeclContext *DC = CurContext;
+  while (DC && !isa(DC) && !isa(DC) &&
+ !isa(DC) && !isa(DC))
+DC = DC->getPa

[PATCH] D157747: Support Unicode Microsoft predefined macros

[Richard Dzenis via Phabricator via cfe-commits]

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

While I work at `__LPREFIX`, I remove this revision from review queue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157747

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

[PATCH] D158505: [clang-format] Fix weird handling of AfterColon

[Björn Schäpers via Phabricator via cfe-commits]

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

Will be fixed on commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158505

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

[PATCH] D158592: [clang][dataflow] Produce pointer values for callees of member operator calls.

[Martin Böhme via Phabricator via cfe-commits]

mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Calls to member operators are a special case in that their callees have pointer
type. The callees of non-operator non-static member functions are not pointers.
See the comments in the code for details.

This issue came up in the Crubit nullability check; the fact that we weren't
modeling the `PointerValue` caused non-convergence.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158592

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -5550,6 +5550,59 @@
   });
 }
 
+// Check that a callee of a member operator call is modeled as a 
`PointerValue`.
+// Member operator calls are unusual in that their callee is a pointer that
+// stems from a `FunctionToPointerDecay`. In calls to non-operator non-static
+// member functions, the callee is a `MemberExpr` (which does not have pointer
+// type).
+// We want to make sure that we produce a pointer value for the callee in this
+// specific scenario and that its storage location is durable (for 
convergence).
+TEST(TransferTest, MemberOperatorCallModelsPointerForCallee) {
+  std::string Code = R"(
+struct S {
+  bool operator!=(S s);
+};
+void target() {
+  S s;
+  (void)(s != s);
+  (void)(s != s);
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+using ast_matchers::selectFirst;
+using ast_matchers::match;
+using ast_matchers::traverse;
+using ast_matchers::cxxOperatorCallExpr;
+
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+auto Matches = match(
+traverse(TK_AsIs, cxxOperatorCallExpr().bind("call")), ASTCtx);
+
+ASSERT_EQ(Matches.size(), 2);
+
+auto *Call1 = Matches[0].getNodeAs("call");
+auto *Call2 = Matches[1].getNodeAs("call");
+
+ASSERT_THAT(Call1, NotNull());
+ASSERT_THAT(Call2, NotNull());
+
+EXPECT_EQ(cast(Call1->getCallee())->getCastKind(),
+  CK_FunctionToPointerDecay);
+EXPECT_EQ(cast(Call2->getCallee())->getCastKind(),
+  CK_FunctionToPointerDecay);
+
+auto *Ptr1 = cast(Env.getValue(*Call1->getCallee()));
+auto *Ptr2 = cast(Env.getValue(*Call2->getCallee()));
+
+ASSERT_EQ(&Ptr1->getPointeeLoc(), &Ptr2->getPointeeLoc());
+  });
+}
+
 // Check that fields of anonymous records are modeled.
 TEST(TransferTest, AnonymousStruct) {
   std::string Code = R"(
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -175,12 +175,16 @@
 const ValueDecl *VD = S->getDecl();
 assert(VD != nullptr);
 
-// `DeclRefExpr`s to fields and non-static methods aren't glvalues, and
-// there's also no sensible `Value` we can assign to them, so skip them.
-if (isa(VD))
-  return;
-if (auto *Method = dyn_cast(VD);
-Method && !Method->isStatic())
+// Some `DeclRefExpr`s aren't glvalues, so we can't associate them with a
+// `StorageLocation`, and there's also no sensible `Value` that we can
+// assign to them. Examples:
+// - Non-static member variables
+// - Non static member functions
+//   Note: Member operators are an exception to this, but apparently only
+//   if the `DeclRefExpr` is used within the callee of a
+//   `CXXOperatorCallExpr`. In other cases, for example when applying the
+//   address-of operator, the `DeclRefExpr` is a prvalue.
+if (!S->isGLValue())
   return;
 
 auto *DeclLoc = Env.getStorageLocation(*VD);


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -5550,6 +5550,59 @@
   });
 }
 
+// Check that a callee of a member operator call is modeled as a `PointerValue`.
+// Member operator calls are unusual in that their callee is a pointer that
+// stems from a `FunctionToPointerDecay`. In calls to non-operator non-static
+// member functions, the callee is a `MemberExpr` (which does not have pointer
+// type).
+// We want to make sure that we produce a pointer value for the callee in this
+// specific scenario and th

[PATCH] D158591: Add support of Windows Trace Logging macros

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:1495-1499
+  DeclContext *DC = CurContext;
+  while (DC && !isa(DC) && !isa(DC) &&
+ !isa(DC) && !isa(DC))
+DC = DC->getParent();
+  return dyn_cast_or_null(DC);

I think this is reimplementing `getCurFunctionOrMethodDecl`
maybe we want to do 

```
if(Decl* DC = getCurFunctionOrMethodDecl())
return DC;
return dyn_cast_or_null(CurrentContext);
```

Or something like that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158591

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

[PATCH] D158140: WIP: [clang-repl] Basic WebAssembly support for running inside a JS engine

[Anubhab Ghosh via Phabricator via cfe-commits]

argentite added inline comments.



Comment at: clang/lib/Interpreter/WASM.cpp:79
+  int Result =
+  lld::wasm::link(LinkerArgs, llvm::outs(), llvm::errs(), false, false);
+  if (!Result)

v.g.vassilev wrote:
> I am not sure how we can solve that dependency here. Worst case scenario, 
> could we check if `lld` is installed and make a system call?
AFAIK we can't really `exec()` within Emscripten.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158140

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

[PATCH] D158595: [clang][Interp] Allow zero-ini of primitives with an empty init list

[Timm Bäder via Phabricator via cfe-commits]

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, shafik, cor3ntin.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158595

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/test/AST/Interp/literals.cpp


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -91,6 +91,15 @@
// ref-error {{not an integral constant 
expression}} \
// ref-note {{outside the range of 
representable values}} \
 
+namespace PrimitiveEmptyInitList {
+  constexpr int a = {};
+  static_assert(a == 0, "");
+  constexpr bool b = {};
+  static_assert(!b, "");
+  constexpr double d = {};
+  static_assert(d == 0.0, "");
+}
+
 
 enum E {};
 constexpr E e = static_cast(0);
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -675,8 +675,10 @@
 
   // Primitive values.
   if (std::optional T = classify(E->getType())) {
-assert(E->getNumInits() == 1);
 assert(!DiscardResult);
+if (E->getNumInits() == 0)
+  return this->visitZeroInitializer(E->getType(), E);
+assert(E->getNumInits() == 1);
 return this->delegate(E->inits()[0]);
   }
 


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -91,6 +91,15 @@
// ref-error {{not an integral constant expression}} \
// ref-note {{outside the range of representable values}} \
 
+namespace PrimitiveEmptyInitList {
+  constexpr int a = {};
+  static_assert(a == 0, "");
+  constexpr bool b = {};
+  static_assert(!b, "");
+  constexpr double d = {};
+  static_assert(d == 0.0, "");
+}
+
 
 enum E {};
 constexpr E e = static_cast(0);
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -675,8 +675,10 @@
 
   // Primitive values.
   if (std::optional T = classify(E->getType())) {
-assert(E->getNumInits() == 1);
 assert(!DiscardResult);
+if (E->getNumInits() == 0)
+  return this->visitZeroInitializer(E->getType(), E);
+assert(E->getNumInits() == 1);
 return this->delegate(E->inits()[0]);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158140: WIP: [clang-repl] Basic WebAssembly support for running inside a JS engine

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added reviewers: pmatos, tlively.
v.g.vassilev added a comment.

Let's add more reviewers to see if we can figure out how to move forward here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158140

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

[PATCH] D158591: Add support of Windows Trace Logging macros

[Richard Dzenis via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/Sema.cpp:1495-1499
+  DeclContext *DC = CurContext;
+  while (DC && !isa(DC) && !isa(DC) &&
+ !isa(DC) && !isa(DC))
+DC = DC->getParent();
+  return dyn_cast_or_null(DC);

cor3ntin wrote:
> I think this is reimplementing `getCurFunctionOrMethodDecl`
> maybe we want to do 
> 
> ```
> if(Decl* DC = getCurFunctionOrMethodDecl())
> return DC;
> return dyn_cast_or_null(CurrentContext);
> ```
> 
> Or something like that
Well, unfortunately, not really.

The previous implementation did call `getCurFunctionOrMethodDecl()`, but it 
returned nullptr when we were inside a RecordDecl.
If you examine the implementation of `getFunctionLevelDeclContext(bool 
AllowLambda)`, it halts if `isa(DC)`. I tried patching 
`getFunctionLevelDeclContext()` to skip RecordDecl, but this triggered around 
70 clang/tests. I didn't want to delve into understanding the failure of each 
test. If you believe there's an issue with our current code, I can allocate 
time to investigate each specific test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158591

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

[PATCH] D158140: WIP: [clang-repl] Basic WebAssembly support for running inside a JS engine

[Anubhab Ghosh via Phabricator via cfe-commits]

argentite added a comment.

Here's a demo of it in action with clang and lld linked statically out of tree. 
https://wasmdemo.argentite.me


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158140

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

[PATCH] D152752: [MS] Fix passing aligned records by value in some cases

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Intel is seeing some fallout from this change in our downstream (reverting this 
commit causes our test case to pass). Our test looks like this:

  #include 
  #include 
  #include 
  #include 
  
  typedef union {
char c[32];
short s[16];
int d[8];
long long l[4];
__m128 h[2];
__m256 m;
__m256i mi;
__m256d md;
  } M256;
  
  M256 make_m256_from_int(int d1, int d2, int d3, int d4, int d5, int d6, int 
d7, int d8) {
M256 ret;
ret.d[0] = d1;
ret.d[1] = d2;
ret.d[2] = d3;
ret.d[3] = d4;
ret.d[4] = d5;
ret.d[5] = d6;
ret.d[6] = d7;
ret.d[7] = d8;
return ret;
  }
  
  typedef union {
char c;
  } M8;
  
  M8 make_m8_from_char(char c) {
M8 ret;
ret.c = c;
return ret;
  }
  
  int test(char* format, ...) {
int i, j;
M256 m256;
M8 m8;
int retValue = 0;
va_list args;
va_start(args, format);
  
for (i = 0; format[i]; i++) {
switch (format[i]) {
case '1':
m8 = va_arg(args, M8);
retValue += m8.c;
break;
case '6':
m256 = va_arg(args, M256);
retValue += m256.d[0] + m256.d[1] + m256.d[2] + 
m256.d[3] + m256.d[4] + m256.d[5] + m256.d[6] + m256.d[7];
break;
}
}
  
va_end(args);
return retValue;
  }
  
  int main(void) {
int retValue = 0;
  
if (test("16", make_m8_from_char(-12), make_m256_from_int(1, 2, 1, 2, 
1, 2, 1, 2))) {
fprintf(stderr, "test failed for: M8(%d) 
M256(%d,%d,%d,%d,%d,%d,%d,%d)\n", 12, 1, 2, 1, 2, 1, 2, 1, 2);
retValue++;
}
  
if (retValue) {
printf("FAILED %d tests\n", retValue);
} else {
printf("PASSED\n");
}
return retValue;
  }

What we're seeing is the difference in IR emitted by FE is the byval attribute 
and alignment for 3rd argument.

Bad

  %call2 = call i32 (ptr, ...) @test(ptr noundef @"??_C@_02KMALDIDP@16?$AA@", 
ptr noundef byval(%union.M8) align 4 %agg.tmp1, ptr noundef %agg.tmp), !dbg !237

Good

  %call2 = call i32 (ptr, ...) @test(ptr noundef @"??_C@_02KMALDIDP@16?$AA@", 
ptr noundef byval(%union.M8) align 4 %agg.tmp1, ptr noundef byval(%union.M256) 
align 4 %agg.tmp), !dbg !237

(We're seeing it with this clang-cl-esque command line: `icx -c  -Zi -Od  
/arch:AVX check_types_reduced.c -Xclang -emit-llvm -Xclang 
-disable-llvm-passes`)

I didn't spot any UB in the test, so I think something may be incorrect with 
this patch (but codegen is not my area of specialty, so please correct me if 
I'm wrong).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152752

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

[PATCH] D154603: [analyzer][clangsa] Add new option to alpha.security.cert.InvalidPtrChecker

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

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

Add tests for checker option
Remove unnecessary const_cast
Only model a getenv call if there is a value to model
Use getPredecessor to better indicate what happens during EG building
Hoist GetEnvCall variable
Fix dangling strings in note generation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154603

Files:
  clang/test/Analysis/cert/env34-c.c
  clang/test/Analysis/invalid-ptr-checker.c


Index: clang/test/Analysis/invalid-ptr-checker.c
===
--- /dev/null
+++ clang/test/Analysis/invalid-ptr-checker.c
@@ -0,0 +1,56 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=alpha.security.cert.env.InvalidPtr \
+// RUN:  -analyzer-config 
alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=false \
+// RUN:  -analyzer-output=text -verify -Wno-unused %s
+//
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=alpha.security.cert.env.InvalidPtr \
+// RUN:  -analyzer-config \
+// RUN: alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
+// RUN: -analyzer-output=text -verify=pedantic -Wno-unused %s
+
+#include "Inputs/system-header-simulator.h"
+
+char *getenv(const char *name);
+int setenv(const char *name, const char *value, int overwrite);
+int strcmp(const char *, const char *);
+
+int custom_env_handler(const char **envp);
+
+void getenv_after_getenv(void) {
+  char *v1 = getenv("V1");
+  // pedantic-note@-1{{previous function call was here}}
+
+  char *v2 = getenv("V2");
+  // pedantic-note@-1{{'getenv' call may invalidate the result of the previous 
'getenv'}}
+
+  strcmp(v1, v2);
+  // pedantic-warning@-1{{use of invalidated pointer 'v1' in a function call}}
+  // pedantic-note@-2{{use of invalidated pointer 'v1' in a function call}}
+}
+
+void setenv_after_getenv(void) {
+  char *v1 = getenv("VAR1");
+
+  setenv("VAR2", "...", 1);
+  // expected-note@-1{{'setenv' call may invalidate the environment returned 
by getenv}}
+  // pedantic-note@-2{{'setenv' call may invalidate the environment returned 
by getenv}}
+
+  strcmp(v1, "");
+  // expected-warning@-1{{use of invalidated pointer 'v1' in a function call}}
+  // expected-note@-2{{use of invalidated pointer 'v1' in a function call}}
+  // pedantic-warning@-3{{use of invalidated pointer 'v1' in a function call}}
+  // pedantic-note@-4{{use of invalidated pointer 'v1' in a function call}}
+}
+
+int main(int argc, const char *argv[], const char *envp[]) {
+  setenv("VAR", "...", 0);
+  // expected-note@-1 2 {{'setenv' call may invalidate the environment 
parameter of 'main'}}
+  // pedantic-note@-2 2 {{'setenv' call may invalidate the environment 
parameter of 'main'}}
+
+  *envp;
+  // expected-warning@-1 2 {{dereferencing an invalid pointer}}
+  // expected-note@-2 2 {{dereferencing an invalid pointer}}
+  // pedantic-warning@-3 2 {{dereferencing an invalid pointer}}
+  // pedantic-note@-4 2 {{dereferencing an invalid pointer}}
+}
Index: clang/test/Analysis/cert/env34-c.c
===
--- clang/test/Analysis/cert/env34-c.c
+++ clang/test/Analysis/cert/env34-c.c
@@ -2,10 +2,6 @@
 // RUN:  -analyzer-checker=alpha.security.cert.env.InvalidPtr\
 // RUN:  -analyzer-config 
alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
 // RUN:  -analyzer-output=text -verify -Wno-unused %s
-//
-// TODO: write test cases that follow the pattern:
-//   "getenv -> store pointer -> setenv -> use stored pointer"
-//   and not rely solely on getenv as an invalidating function
 
 #include "../Inputs/system-header-simulator.h"
 char *getenv(const char *name);


Index: clang/test/Analysis/invalid-ptr-checker.c
===
--- /dev/null
+++ clang/test/Analysis/invalid-ptr-checker.c
@@ -0,0 +1,56 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=alpha.security.cert.env.InvalidPtr \
+// RUN:  -analyzer-config alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=false \
+// RUN:  -analyzer-output=text -verify -Wno-unused %s
+//
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=alpha.security.cert.env.InvalidPtr \
+// RUN:  -analyzer-config \
+// RUN: alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
+// RUN: -analyzer-output=text -verify=pedantic -Wno-unused %s
+
+#include "Inputs/system-header-simulator.h"
+
+char *getenv(const char *name);
+int setenv(const char *name, const char *value, int overwrite);
+int strcmp(const char *, const char *);
+
+int custom_env_handler(const char **envp);
+
+void getenv_after_getenv(void) {
+  char *v1 = getenv("V1");
+  // pedantic-note@-1{{previous function call was here}}
+
+  char *v2 = getenv("V2");
+  // pedantic-note@-1{{'getenv' call may invalidate the result of the previous 'getenv'}}
+
+  strcmp(v1, v2);
+  // pedantic-warning@-1{{use of inva

[PATCH] D158601: [Clang] Always constant-evaluate operands of comparisons to nullptr

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin created this revision.
Herald added a project: All.
cor3ntin requested review of this revision.
Herald added subscribers: cfe-commits, wangpc.
Herald added a project: clang.

even if we know what the result is going to be.
There may be side effects we ought not to ignore,

Fixes #64923


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158601

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/compare-cxx2a.cpp


Index: clang/test/SemaCXX/compare-cxx2a.cpp
===
--- clang/test/SemaCXX/compare-cxx2a.cpp
+++ clang/test/SemaCXX/compare-cxx2a.cpp
@@ -461,3 +461,21 @@
   template bool f6() { return 0 < y6; } // expected-note 
{{instantiation of}}
   void g6() { f6(); } // expected-note {{instantiation of}}
 }
+
+
+namespace GH64923 {
+using nullptr_t = decltype(nullptr);
+struct MyTask{};
+constexpr MyTask DoAnotherThing() {
+return {};
+}
+
+constexpr nullptr_t operator++(MyTask &&T); // expected-note 2{{declared here}}
+void DoSomething() {
+  if constexpr (++DoAnotherThing() != nullptr) {} // expected-error 
{{constexpr if condition is not a constant expression}} \
+  // expected-note  
{{undefined function 'operator++' cannot be used in a constant expression}}
+
+  if constexpr (nullptr == ++DoAnotherThing()) {} // expected-error 
{{constexpr if condition is not a constant expression}} \
+  // expected-note  
{{undefined function 'operator++' cannot be used in a constant expression}}
+}
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -13306,6 +13306,10 @@
 // C++11 [expr.rel]p4, [expr.eq]p3: If two operands of type std::nullptr_t
 // are compared, the result is true of the operator is <=, >= or ==, and
 // false otherwise.
+LValue Res;
+if (!EvaluatePointer(E->getLHS(), Res, Info) ||
+!EvaluatePointer(E->getRHS(), Res, Info))
+  return false;
 return Success(CmpResult::Equal, E);
   }
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -170,6 +170,11 @@
   requires-expression. This fixes:
   (`#64138 _`).
 
+- Expressions producing ``nullptr`` are correctly evaluated
+  by the constant interpreter when appearing as the operand
+  of a binary comparision.
+  (`#64923 _``)
+
 Bug Fixes to AST Handling
 ^
 - Fixed an import failure of recursive friend class template.


Index: clang/test/SemaCXX/compare-cxx2a.cpp
===
--- clang/test/SemaCXX/compare-cxx2a.cpp
+++ clang/test/SemaCXX/compare-cxx2a.cpp
@@ -461,3 +461,21 @@
   template bool f6() { return 0 < y6; } // expected-note {{instantiation of}}
   void g6() { f6(); } // expected-note {{instantiation of}}
 }
+
+
+namespace GH64923 {
+using nullptr_t = decltype(nullptr);
+struct MyTask{};
+constexpr MyTask DoAnotherThing() {
+return {};
+}
+
+constexpr nullptr_t operator++(MyTask &&T); // expected-note 2{{declared here}}
+void DoSomething() {
+  if constexpr (++DoAnotherThing() != nullptr) {} // expected-error {{constexpr if condition is not a constant expression}} \
+  // expected-note  {{undefined function 'operator++' cannot be used in a constant expression}}
+
+  if constexpr (nullptr == ++DoAnotherThing()) {} // expected-error {{constexpr if condition is not a constant expression}} \
+  // expected-note  {{undefined function 'operator++' cannot be used in a constant expression}}
+}
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -13306,6 +13306,10 @@
 // C++11 [expr.rel]p4, [expr.eq]p3: If two operands of type std::nullptr_t
 // are compared, the result is true of the operator is <=, >= or ==, and
 // false otherwise.
+LValue Res;
+if (!EvaluatePointer(E->getLHS(), Res, Info) ||
+!EvaluatePointer(E->getRHS(), Res, Info))
+  return false;
 return Success(CmpResult::Equal, E);
   }
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -170,6 +170,11 @@
   requires-expression. This fixes:
   (`#64138 _`).
 
+- Expressions producing ``nullptr`` are correctly evaluated
+  by the constant interpreter when appearing as the operand
+  of a b

[clang] f672094 - [Sema] Fix -Wparentheses warning seen with gcc

[Bjorn Pettersson via cfe-commits]

Author: Bjorn Pettersson
Date: 2023-08-23T14:27:08+02:00
New Revision: f6720947101f2866ca4812056842ada06cf59242

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

LOG: [Sema] Fix -Wparentheses warning seen with gcc

This patch simply replace
  assert(X || Y && "...")
by
  assert((X || Y) && "...")
to silence -Wparentheses warnings.

Added: 


Modified: 
clang/lib/Sema/SemaDecl.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 8e5920b8babeb2..3925e2a7f3382c 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -14379,10 +14379,9 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl 
*var) {
   if (Stack != &ConstSegStack && MSVCEnv &&
   ConstSegStack.CurrentValue != ConstSegStack.DefaultValue &&
   var->getType().isConstQualified()) {
-assert(!Reason ||
-   Reason != QualType::NonConstantStorageReason::
- NonConstNonReferenceType &&
-   "This case should've already been handled elsewhere");
+assert((!Reason || Reason != QualType::NonConstantStorageReason::
+ NonConstNonReferenceType) &&
+   "This case should've already been handled elsewhere");
 Diag(var->getLocation(), diag::warn_section_msvc_compat)
 << var << ConstSegStack.CurrentValue << (int)(!HasConstInit
 ? QualType::NonConstantStorageReason::NonTrivialCtor



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

[clang] 89bf39b - [clang][CGExprConstant] Resolve unused variable 'C' warning

[Bjorn Pettersson via cfe-commits]

Author: Bjorn Pettersson
Date: 2023-08-23T14:27:13+02:00
New Revision: 89bf39b068e8bc51f4178d1e794fd78e8e86b508

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

LOG: [clang][CGExprConstant] Resolve unused variable 'C' warning

Added: 


Modified: 
clang/lib/CodeGen/CGExprConstant.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGExprConstant.cpp 
b/clang/lib/CodeGen/CGExprConstant.cpp
index 52d61f1e298087..2c847f0bb13fd2 100644
--- a/clang/lib/CodeGen/CGExprConstant.cpp
+++ b/clang/lib/CodeGen/CGExprConstant.cpp
@@ -1132,7 +1132,7 @@ class ConstExprEmitter :
 return CGM.GetAddrOfConstantStringFromLiteral(S).getPointer();
   return nullptr;
 case CK_NullToPointer:
-  if (llvm::Constant *C = Visit(subExpr, destType))
+  if (Visit(subExpr, destType))
 return CGM.EmitNullConstant(destType);
   return nullptr;
 



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

[PATCH] D154603: [analyzer][clangsa] Add new option to alpha.security.cert.InvalidPtrChecker

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

gamesh411 updated this revision to Diff 552676.
gamesh411 marked an inline comment as done.
gamesh411 added a comment.

rebased and squashed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154603

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/cert/env34-c-cert-examples.c
  clang/test/Analysis/cert/env34-c.c
  clang/test/Analysis/invalid-ptr-checker.c

Index: clang/test/Analysis/invalid-ptr-checker.c
===
--- /dev/null
+++ clang/test/Analysis/invalid-ptr-checker.c
@@ -0,0 +1,56 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=alpha.security.cert.env.InvalidPtr \
+// RUN:  -analyzer-config alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=false \
+// RUN:  -analyzer-output=text -verify -Wno-unused %s
+//
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=alpha.security.cert.env.InvalidPtr \
+// RUN:  -analyzer-config \
+// RUN: alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
+// RUN: -analyzer-output=text -verify=pedantic -Wno-unused %s
+
+#include "Inputs/system-header-simulator.h"
+
+char *getenv(const char *name);
+int setenv(const char *name, const char *value, int overwrite);
+int strcmp(const char *, const char *);
+
+int custom_env_handler(const char **envp);
+
+void getenv_after_getenv(void) {
+  char *v1 = getenv("V1");
+  // pedantic-note@-1{{previous function call was here}}
+
+  char *v2 = getenv("V2");
+  // pedantic-note@-1{{'getenv' call may invalidate the result of the previous 'getenv'}}
+
+  strcmp(v1, v2);
+  // pedantic-warning@-1{{use of invalidated pointer 'v1' in a function call}}
+  // pedantic-note@-2{{use of invalidated pointer 'v1' in a function call}}
+}
+
+void setenv_after_getenv(void) {
+  char *v1 = getenv("VAR1");
+
+  setenv("VAR2", "...", 1);
+  // expected-note@-1{{'setenv' call may invalidate the environment returned by getenv}}
+  // pedantic-note@-2{{'setenv' call may invalidate the environment returned by getenv}}
+
+  strcmp(v1, "");
+  // expected-warning@-1{{use of invalidated pointer 'v1' in a function call}}
+  // expected-note@-2{{use of invalidated pointer 'v1' in a function call}}
+  // pedantic-warning@-3{{use of invalidated pointer 'v1' in a function call}}
+  // pedantic-note@-4{{use of invalidated pointer 'v1' in a function call}}
+}
+
+int main(int argc, const char *argv[], const char *envp[]) {
+  setenv("VAR", "...", 0);
+  // expected-note@-1 2 {{'setenv' call may invalidate the environment parameter of 'main'}}
+  // pedantic-note@-2 2 {{'setenv' call may invalidate the environment parameter of 'main'}}
+
+  *envp;
+  // expected-warning@-1 2 {{dereferencing an invalid pointer}}
+  // expected-note@-2 2 {{dereferencing an invalid pointer}}
+  // pedantic-warning@-3 2 {{dereferencing an invalid pointer}}
+  // pedantic-note@-4 2 {{dereferencing an invalid pointer}}
+}
Index: clang/test/Analysis/cert/env34-c.c
===
--- clang/test/Analysis/cert/env34-c.c
+++ clang/test/Analysis/cert/env34-c.c
@@ -1,5 +1,6 @@
 // RUN: %clang_analyze_cc1 \
 // RUN:  -analyzer-checker=alpha.security.cert.env.InvalidPtr\
+// RUN:  -analyzer-config alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
 // RUN:  -analyzer-output=text -verify -Wno-unused %s
 
 #include "../Inputs/system-header-simulator.h"
Index: clang/test/Analysis/cert/env34-c-cert-examples.c
===
--- clang/test/Analysis/cert/env34-c-cert-examples.c
+++ clang/test/Analysis/cert/env34-c-cert-examples.c
@@ -1,15 +1,49 @@
+// Default options.
 // RUN: %clang_analyze_cc1 \
 // RUN:  -analyzer-checker=core,alpha.security.cert.env.InvalidPtr \
 // RUN:  -verify -Wno-unused %s
+//
+// Test the laxer handling of getenv function (this is the default).
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,alpha.security.cert.env.InvalidPtr \
+// RUN:  -analyzer-config alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=false \
+// RUN:  -verify -Wno-unused %s
+//
+// Test the stricter handling of getenv function.
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,alpha.security.cert.env.InvalidPtr \
+// RUN:  -analyzer-config alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
+// RUN:  -verify=pedantic -Wno-unused %s
 
 #include "../Inputs/system-header-simulator.h"
 char *getenv(const char *name);
+int setenv(const char *name, const char *value, int overwrite);
 int strcmp(const char*, const char*);
 char *strdup(const char*);
 void free(void *memblock);
 void *malloc(size_t size);
 
-void incorrect_usage(void) {
+v

[PATCH] D157385: [clang][CFG] Cleanup functions

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

Looks good to me, but let's wait for the CFG maintainers to approve it.




Comment at: clang/lib/Analysis/ThreadSafety.cpp:2429
 }
+
 case CFGElement::TemporaryDtor: {

Could you remove the added line?


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

https://reviews.llvm.org/D157385

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

[PATCH] D158433: [Clang] Do not change the type of captured vars when checking lambda constraints

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

@aaron.ballman Should we land that? it already missed rc3...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158433

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

[PATCH] D157684: [clang][ASTImporter] Repeated friend templates are partially imported

[Ding Fei via Phabricator via cfe-commits]

danix800 updated this revision to Diff 552678.
danix800 added a comment.

Add `const` qualifier & replace `auto` with explicit type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157684

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

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4054,6 +4054,25 @@
  ->lookup(ToRecordOfFriend->getDeclName())
  .empty());
   }
+
+  void testRepeatedFriendImport(const char *Code) {
+Decl *ToTu = getToTuDecl(Code, Lang_CXX03);
+Decl *FromTu = getTuDecl(Code, Lang_CXX03, "from.cc");
+
+auto *ToFriend1 = FirstDeclMatcher().match(ToTu, friendDecl());
+auto *ToFriend2 = LastDeclMatcher().match(ToTu, friendDecl());
+auto *FromFriend1 =
+FirstDeclMatcher().match(FromTu, friendDecl());
+auto *FromFriend2 =
+LastDeclMatcher().match(FromTu, friendDecl());
+
+FriendDecl *ToImportedFriend1 = Import(FromFriend1, Lang_CXX03);
+FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX03);
+
+EXPECT_NE(ToImportedFriend1, ToImportedFriend2);
+EXPECT_EQ(ToFriend1, ToImportedFriend1);
+EXPECT_EQ(ToFriend2, ToImportedFriend2);
+  }
 };
 
 TEST_P(ImportFriendClasses, ImportOfFriendRecordDoesNotMergeDefinition) {
@@ -4395,21 +4414,7 @@
 friend class X;
   };
   )";
-  Decl *ToTu = getToTuDecl(Code, Lang_CXX03);
-  Decl *FromTu = getTuDecl(Code, Lang_CXX03, "from.cc");
-
-  auto *ToFriend1 = FirstDeclMatcher().match(ToTu, friendDecl());
-  auto *ToFriend2 = LastDeclMatcher().match(ToTu, friendDecl());
-  auto *FromFriend1 =
-  FirstDeclMatcher().match(FromTu, friendDecl());
-  auto *FromFriend2 = LastDeclMatcher().match(FromTu, friendDecl());
-
-  FriendDecl *ToImportedFriend1 = Import(FromFriend1, Lang_CXX03);
-  FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX03);
-
-  EXPECT_NE(ToImportedFriend1, ToImportedFriend2);
-  EXPECT_EQ(ToFriend1, ToImportedFriend1);
-  EXPECT_EQ(ToFriend2, ToImportedFriend2);
+  testRepeatedFriendImport(Code);
 }
 
 TEST_P(ImportFriendClasses, ImportOfRepeatedFriendDecl) {
@@ -4420,21 +4425,31 @@
 friend void f();
   };
   )";
-  Decl *ToTu = getToTuDecl(Code, Lang_CXX03);
-  Decl *FromTu = getTuDecl(Code, Lang_CXX03, "from.cc");
-
-  auto *ToFriend1 = FirstDeclMatcher().match(ToTu, friendDecl());
-  auto *ToFriend2 = LastDeclMatcher().match(ToTu, friendDecl());
-  auto *FromFriend1 =
-  FirstDeclMatcher().match(FromTu, friendDecl());
-  auto *FromFriend2 = LastDeclMatcher().match(FromTu, friendDecl());
+  testRepeatedFriendImport(Code);
+}
 
-  FriendDecl *ToImportedFriend1 = Import(FromFriend1, Lang_CXX03);
-  FriendDecl *ToImportedFriend2 = Import(FromFriend2, Lang_CXX03);
+TEST_P(ImportFriendClasses, ImportOfRepeatedFriendFunctionTemplateDecl) {
+  const char *Code =
+  R"(
+template 
+class Container {
+  template  friend void m();
+  template  friend void m();
+};
+  )";
+  testRepeatedFriendImport(Code);
+}
 
-  EXPECT_NE(ToImportedFriend1, ToImportedFriend2);
-  EXPECT_EQ(ToFriend1, ToImportedFriend1);
-  EXPECT_EQ(ToFriend2, ToImportedFriend2);
+TEST_P(ImportFriendClasses, ImportOfRepeatedFriendClassTemplateDecl) {
+  const char *Code =
+  R"(
+template 
+class Container {
+  template  friend class X;
+  template  friend class X;
+};
+  )";
+  testRepeatedFriendImport(Code);
 }
 
 TEST_P(ASTImporterOptionSpecificTestBase, FriendFunInClassTemplate) {
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -4079,22 +4079,34 @@
   unsigned int IndexOfDecl;
 };
 
-template 
-static FriendCountAndPosition getFriendCountAndPosition(
-const FriendDecl *FD,
-llvm::function_ref GetCanTypeOrDecl) {
+static bool IsEquivalentFriend(ASTImporter &Importer, FriendDecl *FD1,
+   FriendDecl *FD2) {
+  if ((!FD1->getFriendType()) != (!FD2->getFriendType()))
+return false;
+
+  if (const TypeSourceInfo *TSI = FD1->getFriendType())
+return Importer.IsStructurallyEquivalent(
+TSI->getType(), FD2->getFriendType()->getType(), /*Complain=*/false);
+
+  ASTImporter::NonEquivalentDeclSet NonEquivalentDecls;
+  StructuralEquivalenceContext Ctx(
+  FD1->getASTContext(), FD2->getASTContext(), NonEquivalentDecls,
+  StructuralEquivalenceKind::Default,
+  /* StrictTypeSpelling = */ false, /* Complain = */ false);
+  return Ctx.IsEquivalent(FD1, FD2);
+}
+
+static FriendCountAndPosition getFriendCountAndPosition(ASTImporter &Importer,
+   

[PATCH] D157684: [clang][ASTImporter] Repeated friend templates are partially imported

[Ding Fei via Phabricator via cfe-commits]

danix800 added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:4105
 
-  T TypeOrDecl = GetCanTypeOrDecl(FD);
-
-  for (const FriendDecl *FoundFriend : RD->friends()) {
+  for (FriendDecl *FoundFriend : RD->friends()) {
 if (FoundFriend == FD) {

balazske wrote:
> It is better if this is `const`.
As API requires this decl cannot be const.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157684

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

[PATCH] D154603: [analyzer][clangsa] Add new option to alpha.security.cert.InvalidPtrChecker

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

gamesh411 marked 8 inline comments as done.
gamesh411 added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:116-117
+const NoteTag *Note =
+C.getNoteTag([Region, FunctionName, Message](PathSensitiveBugReport 
&BR,
+ llvm::raw_ostream &Out) {
+  if (!BR.isInteresting(Region))

steakhal wrote:
> `FunctionName` and `Message` will dangle inside the NoteTag.
Good catch, thanks! Fixed this with a lambda capture initializer.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:125
 
-  const NoteTag *Note =
-  C.getNoteTag([SymbolicEnvPtrRegion, FunctionName](
-   PathSensitiveBugReport &BR, llvm::raw_ostream &Out) {
-if (!BR.isInteresting(SymbolicEnvPtrRegion))
-  return;
-Out << '\'' << FunctionName
-<< "' call may invalidate the environment parameter of 'main'";
-  });
+  ExplodedNode *CurrentChainEnd = nullptr;
+

steakhal wrote:
> donat.nagy wrote:
> > Perhaps add a comment that clarifies that passing a `nullptr` as the 
> > ExplodedNode to `addTransition` is equivalent to specifying the current 
> > node. I remember this because I was studying its implementation recently, 
> > but I would've been surprised and suspicious otherwise.
> If `nullptr` is equivalent with `C.getPredecessor()` inside 
> `addTransition()`, why not simply initialize it to that value instead of to 
> `nullptr`?
I ended up using C.getPredecessor() instead of explaining; this seems a bit 
more intuitive (if such a thing even exists in CSA).



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:218
+State->add(Call.getReturnValue().getAsRegion());
+   C.addTransition(State);
+  }

steakhal wrote:
> donat.nagy wrote:
> > I fear that this state transition will go "sideways" and the later state 
> > transitions (which add the note tags) will branch off instead of building 
> > onto this. IIUC calling `CheckerContext::addTransition` registers the 
> > transition without updating the "current ExplodedNode" field of 
> > `CheckerContext`, so you need to explicitly store and pass around the 
> > ExplodedNode returned by it if you want to build on it.
> > 
> > This is an ugly and counter-intuitive API, and I also ran into a very 
> > similar issue a few weeks ago (@Szelethus helped me).
> I think the usage here is correct.
(the line number of this comment desync-ed)
I agree, that the addTransition API is easy to misuse, and I would welcome a 
more streamlined approach.
I tried to pay attention to "build" the state and the Exploded Graph by always 
providing the Exploded Node (second parameter), and this seems fine.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:44
+  // environment variable buffer. This is implied in the standard, but in
+  // practice does not cause problems (in the commonly used environments).
+  bool InvalidatingGetEnv = false;

Reworded the message here



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:48
+  // GetEnv can be treated invalidating and non-invalidating as well.
+  const CallDescription GetEnvCall{{"getenv"}, 1};
+

Hoisted here



Comment at: clang/test/Analysis/invalid-ptr-checker.c:51
+
+  *envp;
+  // expected-warning@-1 2 {{dereferencing an invalid pointer}}

This gives 2 warnings. One for subexpression `envp`, and one for the whole 
statement `*envp`. This is the current behaviour ( check 
clang/test/Analysis/cert/env31-c.c ), and this patch does not change it.
However, I would like to devise a solution for this in a different patch.
One option would be to make the error of this checker Fatal, so only one would 
appear, or refine the checkLocation callback to only consider one of these 2 
cases for reporting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154603

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

[PATCH] D158601: [Clang] Always constant-evaluate operands of comparisons to nullptr

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:13312
+!EvaluatePointer(E->getRHS(), Res, Info))
+  return false;
 return Success(CmpResult::Equal, E);

I kinda expected the fix for this bug to simply //remove// a special case... 
does just falling through to the `DoAfter()` call not work?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158601

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

[PATCH] D158582: [AMDGPU] Respect unresolved symbol option if forwarded to linker

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

Remember to clang format.




Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:564-575
+  // If the user has manually passed -Wl,--unresolved-symbols=* as a linker
+  // option, we should not add --no-undefined
+  bool UnresolvedOpt = false;
+  for (auto A : Args)
+if (A->getOption().matches(options::OPT_Wl_COMMA) ||
+ A->getOption().matches(options::OPT_Xlinker))
+  for (StringRef V : A->getValues())

A little more concisely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158582

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

[PATCH] D154603: [analyzer][clangsa] Add new option to alpha.security.cert.InvalidPtrChecker

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

gamesh411 added a comment.

In D154603#4580609 , @steakhal wrote:

> I'm sorry starting the review of this one only now, but I'm quite booked.
> Is it still relevant? If so, I'll continue.

Yes thanks for the effort!
I would like to go through with this option, and then I would like to fix the 
following issues with this checker as well:

- the previous function call notes could be more streamlined
- the interesting notes are also shown, when another checker hits those nodes 
with its report (for example taint checker giving a warning for getenv would 
also trigger the display of the 'previous function call was here' note here), 
this I would like to filter with bug category filters
- try to consolidate the multiple warnings coming from this checkers 
checkLocation callback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154603

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

[PATCH] D158591: Add support of Windows Trace Logging macros

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:1495-1499
+  DeclContext *DC = CurContext;
+  while (DC && !isa(DC) && !isa(DC) &&
+ !isa(DC) && !isa(DC))
+DC = DC->getParent();
+  return dyn_cast_or_null(DC);

RIscRIpt wrote:
> cor3ntin wrote:
> > I think this is reimplementing `getCurFunctionOrMethodDecl`
> > maybe we want to do 
> > 
> > ```
> > if(Decl* DC = getCurFunctionOrMethodDecl())
> > return DC;
> > return dyn_cast_or_null(CurrentContext);
> > ```
> > 
> > Or something like that
> Well, unfortunately, not really.
> 
> The previous implementation did call `getCurFunctionOrMethodDecl()`, but it 
> returned nullptr when we were inside a RecordDecl.
> If you examine the implementation of `getFunctionLevelDeclContext(bool 
> AllowLambda)`, it halts if `isa(DC)`. I tried patching 
> `getFunctionLevelDeclContext()` to skip RecordDecl, but this triggered around 
> 70 clang/tests. I didn't want to delve into understanding the failure of each 
> test. If you believe there's an issue with our current code, I can allocate 
> time to investigate each specific test case.
You are right, i missed that.
I wish we had a better name for this function but I can't think of anything



Comment at: clang/lib/Sema/Sema.cpp:1496-1497
+  DeclContext *DC = CurContext;
+  while (DC && !isa(DC) && !isa(DC) &&
+ !isa(DC) && !isa(DC))
+DC = DC->getParent();




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158591

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

[PATCH] D158301: Add back overriding-t-options for -m-version-min diagnostic

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: clang/test/Driver/darwin-version.c:217
 // RUN:   FileCheck --check-prefix=CHECK-VERSION-TNO-OSV1 %s
-// CHECK-VERSION-TNO-OSV1: overriding '-mmacos-version-min=10.6' option with 
'-target x86_64-apple-macos10.11.2'
+// CHECK-VERSION-TNO-OSV1: overriding '-mmacos-version-min=10.6' option with 
'-target x86_64-apple-macos10.11.2' [-Woverriding-t-option]
 

MaskRay wrote:
> dblaikie wrote:
> > MaskRay wrote:
> > > dexonsmith wrote:
> > > > MaskRay wrote:
> > > > > dexonsmith wrote:
> > > > > > MaskRay wrote:
> > > > > > > hans wrote:
> > > > > > > > MaskRay wrote:
> > > > > > > > > hans wrote:
> > > > > > > > > > dexonsmith wrote:
> > > > > > > > > > > MaskRay wrote:
> > > > > > > > > > > > dexonsmith wrote:
> > > > > > > > > > > > > Why would we want to use the old name here? An alias 
> > > > > > > > > > > > > seems strictly better to me. 
> > > > > > > > > > > > Making `overriding-t-option` an alias for 
> > > > > > > > > > > > `overriding-option` would make 
> > > > > > > > > > > > `-Wno-overriding-t-option` applies to future overriding 
> > > > > > > > > > > > option diagnostics, which is exactly what I want to 
> > > > > > > > > > > > avoid.
> > > > > > > > > > > > 
> > > > > > > > > > > I understand that you don't want `-t-` to apply to work 
> > > > > > > > > > > on future overriding option diagnostics, but I think the 
> > > > > > > > > > > platform divergence you're adding here is worse.
> > > > > > > > > > > 
> > > > > > > > > > > Having a few Darwin-specific options use 
> > > > > > > > > > > `-Woverriding-t-option` (and everything else use 
> > > > > > > > > > > `-Woverriding-option`) as the canonical spelling is hard 
> > > > > > > > > > > to reason about for maintainers, and for users.
> > > > > > > > > > > 
> > > > > > > > > > > And might not users on other platforms have 
> > > > > > > > > > > `-Woverriding-t-option` hardcoded in  build settings? (So 
> > > > > > > > > > > @dblaikie's argument that we shouldn't arbitrarily make 
> > > > > > > > > > > things hard for users would apply to all options?)
> > > > > > > > > > > 
> > > > > > > > > > > IMO, if we're not comfortable removing 
> > > > > > > > > > > `-Woverriding-t-option` entirely, then it should live on 
> > > > > > > > > > > as an alias (easy to reason about), not as 
> > > > > > > > > > > canonical-in-special-cases (hard to reason about).
> > > > > > > > > > > IMO, if we're not comfortable removing 
> > > > > > > > > > > -Woverriding-t-option entirely, then it should live on as 
> > > > > > > > > > > an alias (easy to reason about), not as 
> > > > > > > > > > > canonical-in-special-cases (hard to reason about).
> > > > > > > > > > 
> > > > > > > > > > +1 if we can't drop the old spelling, an alias seems like 
> > > > > > > > > > the best option.
> > > > > > > > > Making `overriding-t-option` an alias for 
> > > > > > > > > `overriding-option`, as I mentioned, will make 
> > > > > > > > > `-Wno-overriding-t-option` affect new overriding-options 
> > > > > > > > > uses. This is exactly what I want to avoid.
> > > > > > > > > 
> > > > > > > > > I know there are some `-Wno-overriding-t-option` uses. 
> > > > > > > > > Honestly, they are far fewer than other diagnostics we are 
> > > > > > > > > introducing or changing in Clang. I can understand the 
> > > > > > > > > argument "make -Werror users easier for this specific 
> > > > > > > > > diagnostic" (but `-Werror` will complain about other new 
> > > > > > > > > diagnostics), but do we really want to in the Darwin case? I 
> > > > > > > > > think no. They can remove the version from the target triple 
> > > > > > > > > like 
> > > > > > > > > https://github.com/facebook/ocamlrep/blame/abc14b8aafcc6746ec37bf7bf0de24bfc58d63a0/prelude/apple/apple_target_sdk_version.bzl#L50
> > > > > > > > > 　or make the version part consistent with 
> > > > > > > > > `-m.*os-version-min`.
> > > > > > > > > 
> > > > > > > > > This change may force these users to re-think how they should 
> > > > > > > > > fix  their build. I apology to these users, but I don't feel 
> > > > > > > > > that adding an alias is really necessary.
> > > > > > > > > Making overriding-t-option an alias for overriding-option, as 
> > > > > > > > > I mentioned, will make -Wno-overriding-t-option affect new 
> > > > > > > > > overriding-options uses. This is exactly what I want to avoid.
> > > > > > > > 
> > > > > > > > Is keeping them separate actually important, though? 
> > > > > > > > -Wno-overriding-option has the same issue in that case, that 
> > > > > > > > using the flag will also affect any new overriding-options 
> > > > > > > > uses, and I don't think that's a problem.
> > > > > > > `-Wno-overriding-option` is properly named, so affecting new 
> > > > > > > overriding-options uses looks fine to me.
> > > > > > > `-Wno-overriding-t-option` is awkward, and making it affect new 
> > > > > > > uses makes me nervous.
> > > > > > > 
> > > > > > > The 

[PATCH] D158318: [Sema] tolerate more promotion matches in format string checking

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I just realized that we may need some additional test coverage for `scanf`, as 
that interface also uses `ArgType::matchesType()`: 
https://github.com/llvm/llvm-project/blob/5686f06d7fc02b7e2ab1eceb56f3830b6fdf7301/clang/lib/AST/ScanfFormatString.cpp#L511
 but I think that's the last thing remaining on the patch.




Comment at: clang/lib/AST/FormatString.cpp:480-481
+  break;
+case BuiltinType::Half:
+case BuiltinType::Float16:
+case BuiltinType::Float:

fcloutier wrote:
> aaron.ballman wrote:
> > Should these be checking for `T == C.FloatTy` to return 
> > `NoMatchPromotionTypeConfusion`?
> I don't think it's necessary. `T` is the format specifier's expected type, 
> and no format specifier expects a `float` (due to floating-point types being 
> promoted to `double` by default argument promotion), so there's never a case 
> where `T` is `FloatTy`.
Ah good point!


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

https://reviews.llvm.org/D158318

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

[PATCH] D158601: [Clang] Always constant-evaluate operands of comparisons to nullptr

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:13312
+!EvaluatePointer(E->getRHS(), Res, Info))
+  return false;
 return Success(CmpResult::Equal, E);

tbaeder wrote:
> I kinda expected the fix for this bug to simply //remove// a special case... 
> does just falling through to the `DoAfter()` call not work?
Nope, EvaluateComparisonBinaryOperator does compute the result of the 
comparison. `DoAfter` only adjust the result.
Removing that block of codes lead to a whole bunch of test failures


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158601

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

[PATCH] D154603: [analyzer][clangsa] Add new option to alpha.security.cert.InvalidPtrChecker

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

gamesh411 added a comment.

In D154603#4609809 , @gamesh411 wrote:

> In D154603#4580609 , @steakhal 
> wrote:
>
>> I'm sorry starting the review of this one only now, but I'm quite booked.
>> Is it still relevant? If so, I'll continue.
>
> Yes, thanks for the effort!

I would like to go through with this option, and then I would like to fix the 
following issues with this checker as well:

- the previous function call notes could be more streamlined
- the notes of this checker are also shown when another checker hits those 
nodes with its report
  - for example taint checker giving a warning to `getenv` usage would also 
trigger the display of the 'previous function call was here' note here), this I 
would like to filter with bug category filters
  - code examples for this filtering are below
- try to consolidate the multiple warnings coming from this checker's 
`checkLocation` callback

category based filtering ( example from 
lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:167 ):

  If (!BR.isInteresting(CallLocation) ||
BR.getBugType().getCategory() != categories::TaintedData) { //but this 
would be InvalidPtr BugType's category, namely memory_error
return "";
  }

or checker based filtering ( example from 
lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:397 )

  if (&BR.getBugType() != smartptr::getNullDereferenceBugType() || // this is a 
comparison of the address of a static bugtype
  !BR.isInteresting(ThisRegion))

This second one gives a more precise filtering, but the implementation-specific 
detail of storing the bugtype by reference is what seems to make this work, 
which I find hacky.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154603

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

[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

[Louis Dionne via Phabricator via cfe-commits]

ldionne added inline comments.



Comment at: clang/lib/Headers/stddef.h:118-122
+#ifdef __cplusplus
+namespace std {
+typedef decltype(nullptr) nullptr_t;
+}
+using ::std::nullptr_t;

iana wrote:
> aaron.ballman wrote:
> > ldionne wrote:
> > > iana wrote:
> > > > aaron.ballman wrote:
> > > > > iana wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > iana wrote:
> > > > > > > > ldionne wrote:
> > > > > > > > > iana wrote:
> > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > Related:
> > > > > > > > > > > 
> > > > > > > > > > > https://github.com/llvm/llvm-project/issues/37564
> > > > > > > > > > > https://cplusplus.github.io/LWG/issue3484
> > > > > > > > > > > 
> > > > > > > > > > > CC @ldionne
> > > > > > > > > > I don't _think_ this change actually changes the way 
> > > > > > > > > > nullptr_t gets defined in C++, does it?
> > > > > > > > > I think we absolutely don't want to touch `std::nullptr_t` 
> > > > > > > > > from this header. It's libc++'s responsibility to define 
> > > > > > > > > that, and in fact we define it in `std::__1`, so this is even 
> > > > > > > > > an ABI break (or I guess it would be a compiler error, not 
> > > > > > > > > sure).
> > > > > > > > I'm really not touching it though. All I did is move it from 
> > > > > > > > `__need_NULL` to `__need_nullptr_t`.
> > > > > > > > 
> > > > > > > > The old behavior is that `std::nullptr_t` would only be touched 
> > > > > > > > if (no `__need_` macros were set or if `__need_NULL` was set), 
> > > > > > > > and (_MSC_EXTENSIONS and _NATIVE_NULLPTR_SUPPORTED are defined).
> > > > > > > > 
> > > > > > > > The new behavior is that `std::nullptr_t` will only be touched 
> > > > > > > > if ((no `__need_` macros are set) and (_MSC_EXTENSIONS and 
> > > > > > > > _NATIVE_NULLPTR_SUPPORTED are defined)) or (the new 
> > > > > > > > `__need_nullptr_t` macro is set)
> > > > > > > > 
> > > > > > > > So the only change is that C++ code that previously set 
> > > > > > > > `__need_NULL` will no longer get `std::nullptr_t`. @efriedma 
> > > > > > > > felt like that was a fine change.
> > > > > > > Does libc++ provide the symbols for a freestanding compilation?
> > > > > > > 
> > > > > > > I was pointing out those links specifically because the C++ 
> > > > > > > standard currently says that stddef.h (the C standard library 
> > > > > > > header) needs to provide a definition of `std::nullptr_t`, but 
> > > > > > > that LWG thinks that's perhaps not the right way to do that and 
> > > > > > > may be removing that requirement.
> > > > > > It is weird the standard puts that in stddef.h and not cstddef. I 
> > > > > > think libc++ could provide that in their stddef.h anyway, but the 
> > > > > > intent in this review is to not rock the boat and only do the 
> > > > > > minimal change discussed above.
> > > > > Yeah, this discussion is to figure out whether we have an existing 
> > > > > bug we need to address and if so, where to address it (libc++, clang, 
> > > > > or the C++ standard). I don't think your changes are exacerbating 
> > > > > anything, more just that they've potentially pointed out something 
> > > > > related.
> > > > 👍 
> > > > Does libc++ provide the symbols for a freestanding compilation?
> > > 
> > > I don't think we do. We basically don't support `-ffreestanding` right 
> > > now (we support embedded and funky platforms via other mechanisms).
> > > 
> > > But regardless, `` should never define something in namespace 
> > > `std`, that should be libc++'s responsibility IMO. What we could do here 
> > > instead is just
> > > 
> > > ```
> > > #ifdef __cplusplus
> > > typedef decltype(nullptr) nullptr_t;
> > > #else
> > > typedef typeof(nullptr) nullptr_t;
> > > #endif
> > > ```
> > > 
> > > and then let libc++'s `` do
> > > 
> > > ```
> > > _LIBCPP_BEGIN_NAMESPACE_STD
> > > using ::nullptr_t;
> > > _LIBCPP_END_NAMESPACE_STD
> > > ```
> > > 
> > > If Clang's `` did define `::nullptr_t`, we could likely remove 
> > > libc++'s `` and that might simplify things.
> > >> Does libc++ provide the symbols for a freestanding compilation?
> > > I don't think we do. We basically don't support -ffreestanding right now 
> > > (we support embedded and funky platforms via other mechanisms).
> > 
> > Okay, that's what I thought as well. Thanks!
> > 
> > > But regardless,  should never define something in namespace 
> > > std, that should be libc++'s responsibility IMO. What we could do here 
> > > instead is just
> > 
> > Ah, so you're thinking stddef.h should provide the global nullptr_t and 
> > cstddef should provide the std::nullptr_t. I was thinking stddef.h should 
> > not define nullptr_t in C++ mode at all; it's a C header, not a C++ header. 
> > That led me to thinking about what the behavior should be in C23 given that 
> > it supports nullptr_t.
> > 
> > Were it not for the current requirement that stddef.h provide nullptr_t, I 
> > think stddef.h should do:
> > ```
> > typedef typeof(nullptr) nullp

[PATCH] D158591: Add support of Windows Trace Logging macros

[Richard Dzenis via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/Sema.cpp:1495-1499
+  DeclContext *DC = CurContext;
+  while (DC && !isa(DC) && !isa(DC) &&
+ !isa(DC) && !isa(DC))
+DC = DC->getParent();
+  return dyn_cast_or_null(DC);

cor3ntin wrote:
> RIscRIpt wrote:
> > cor3ntin wrote:
> > > I think this is reimplementing `getCurFunctionOrMethodDecl`
> > > maybe we want to do 
> > > 
> > > ```
> > > if(Decl* DC = getCurFunctionOrMethodDecl())
> > > return DC;
> > > return dyn_cast_or_null(CurrentContext);
> > > ```
> > > 
> > > Or something like that
> > Well, unfortunately, not really.
> > 
> > The previous implementation did call `getCurFunctionOrMethodDecl()`, but it 
> > returned nullptr when we were inside a RecordDecl.
> > If you examine the implementation of `getFunctionLevelDeclContext(bool 
> > AllowLambda)`, it halts if `isa(DC)`. I tried patching 
> > `getFunctionLevelDeclContext()` to skip RecordDecl, but this triggered 
> > around 70 clang/tests. I didn't want to delve into understanding the 
> > failure of each test. If you believe there's an issue with our current 
> > code, I can allocate time to investigate each specific test case.
> You are right, i missed that.
> I wish we had a better name for this function but I can't think of anything
A perfect name would be `getFunctionLevelDeclContext`, but it's taken. Welp... 
I'll try to dig-into related functions, and update when I find something. An 
alternative solution would be to parameterize (in some way) 
`getFunctionLevelDeclContext` (e.g. add bool flags, or list of skippable types, 
etc.).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158591

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

[PATCH] D158612: [flang][driver] Ensure negative flags have the same visibility as positive

[Tom Eccles via Phabricator via cfe-commits]

tblah created this revision.
tblah added reviewers: awarzynski, bogner, MaskRay.
Herald added a reviewer: sscalpone.
Herald added projects: Flang, All.
tblah requested review of this revision.
Herald added subscribers: cfe-commits, jdoerfert.
Herald added a project: clang.

https://reviews.llvm.org/D157151 and https://reviews.llvm.org/D157837
added visibility flags to flang options, hiding options which are
supported only in Clang and not in Flang.

After this change, some negative flags e.g. `-fno-reciprocal-math` no
longer work with flang. These flags are supported in flang (as can be
seen from the support for the positive flags).

I also opted to make sure the clang visibility is the same on these
flags, although I did not look at changing the visibility of non-flang
flags.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158612

Files:
  clang/include/clang/Driver/Options.td
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90


Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -39,6 +39,7 @@
 ! HELP-NEXT: -fhonor-nansSpecify that floating-point optimizations 
are not allowed that assume arguments and results are not NANs.
 ! HELP-NEXT: -fimplicit-none No implicit typing allowed unless 
overridden by IMPLICIT statements
 ! HELP-NEXT: -finput-charset= Specify the default character set for 
source files
+! HELP-NEXT: -fintegrated-as Enable the integrated assembler
 ! HELP-NEXT: -fintrinsic-modules-path 
 ! HELP-NEXT: Specify where to find the compiled 
intrinsic modules
 ! HELP-NEXT: -flarge-sizes   Use INTEGER(KIND=8) for the result type 
in size-related intrinsics
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -43,6 +43,7 @@
 ! CHECK-NEXT: -fhonor-nansSpecify that floating-point 
optimizations are not allowed that assume arguments and results are not NANs.
 ! CHECK-NEXT: -fimplicit-none No implicit typing allowed unless 
overridden by IMPLICIT statements
 ! CHECK-NEXT: -finput-charset= Specify the default character set for 
source files
+! CHECK-NEXT: -fintegrated-as Enable the integrated assembler
 ! CHECK-NEXT: -fintrinsic-modules-path 
 ! CHECK-NEXT: Specify where to find the compiled 
intrinsic modules
 ! CHECK-NEXT: -flang-experimental-hlfir
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2366,13 +2366,13 @@
   PosFlag,
-  NegFlag>;
+  NegFlag>;
 defm approx_func : BoolFOption<"approx-func", LangOpts<"ApproxFunc">, 
DefaultFalse,
PosFlag,
-   NegFlag>;
+   NegFlag>;
 defm finite_math_only : BoolFOption<"finite-math-only",
   LangOpts<"FiniteMathOnly">, DefaultFalse,
   PosFlag,
-  PosFlag>;
+  PosFlag>;
 def fhonor_nans : Flag<["-"], "fhonor-nans">, Group,
   Visibility<[ClangOption, FlangOption]>,
   HelpText<"Specify that floating-point optimizations are not allowed that "
@@ -3383,12 +3383,12 @@
   LangOpts<"ROPI">, DefaultFalse,
   PosFlag,
-  NegFlag>;
+  NegFlag>;
 defm rwpi : BoolFOption<"rwpi",
   LangOpts<"RWPI">, DefaultFalse,
   PosFlag,
-  NegFlag>;
+  NegFlag>;
 def fplugin_EQ : Joined<["-"], "fplugin=">, Group,
   Flags<[NoXarchOption]>, MetaVarName<"">,
   HelpText<"Load the named plugin (dynamic shared object)">;
@@ -5311,7 +5311,7 @@
 defm integrated_as : BoolFOption<"integrated-as",
   CodeGenOpts<"DisableIntegratedAS">, DefaultFalse,
   NegFlag,
-  PosFlag,
+  PosFlag,
   BothFlags<[], [ClangOption], " the integrated assembler">>;
 
 def fintegrated_cc1 : Flag<["-"], "fintegrated-cc1">,


Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -39,6 +39,7 @@
 ! HELP-NEXT: -fhonor-nansSpecify that floating-point optimizations are not allowed that assume arguments and results are not NANs.
 ! HELP-NEXT: -fimplicit-none No implicit typing allowed unless overridden by IMPLICIT statements
 ! HELP-NEXT: -finput-charset= Specify the default character set for source files
+! HELP-NEXT: -fintegrated-as Enable the integrated assembler
 ! HELP-NEXT: -fintrinsic-modules-path 
 ! HELP-NEXT: Specify where to find the compiled intrinsic modules
 ! HELP-NEXT: -flarge-sizes   Use INTEGER(KIND=8) for the result type in size-related intrinsics
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ fl

[PATCH] D154603: [analyzer][clangsa] Add new option to alpha.security.cert.InvalidPtrChecker

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

steakhal added a comment.

In D154603#4609872 , @gamesh411 wrote:

> - try to consolidate the multiple warnings coming from this checker's 
> `checkLocation` callback
>
> category based filtering ( example from 
> lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:167 ):
>
>   If (!BR.isInteresting(CallLocation) ||
> BR.getBugType().getCategory() != categories::TaintedData) { //but this 
> would be InvalidPtr BugType's category, namely memory_error
> return "";
>   }
>
> or checker based filtering ( example from 
> lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:397 )
>
>   if (&BR.getBugType() != smartptr::getNullDereferenceBugType() || // this is 
> a comparison of the address of a static bugtype
>   !BR.isInteresting(ThisRegion))
>
> This second one gives a more precise filtering, but the 
> implementation-specific detail of storing the bugtype by reference is what 
> seems to make this work, which I find hacky.

If the checker issues a NoteTag, it makes sense in certain situations to make 
sure that it acts on only reports issued by that checker. The standard way of 
achieving that is by comparing the tags, as you do in the second example.
There is nothing wrong with this, if that particular checker issued that 
NoteTag in the first place. It's marginally debatable, if it was not issued by 
the given checker but rather something else. That would suggest to me some 
logic flaw, or coupling issue. For cross-checker cases, I think the bug 
category would be the better option, but I would still need to think about 
that, so not set in stone :D

FYI I haven't looked at the patch yet, but I wanted to answer your question.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154603

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

[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.

[Ying Yi via Phabricator via cfe-commits]

MaggieYi created this revision.
MaggieYi added reviewers: MaskRay, peter.smith, vitalybuka, probinson, 
pgousseau, glandium, uabelho.
Herald added a project: All.
MaggieYi requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

PR for https://github.com/llvm/llvm-project/issues/64931.

An execute-only target disallows data access to code sections. When enabling 
the function sanitizer (-fsanitize=function), UBSan function signatures and 
type hashes are emitted within the function's prologue data to enable checking 
of the function type. This results in a non-execute access to the code section 
and a runtime error.

To solve the issue, -fsanitize=function should not be included in any check 
group (e.g. undefined) on an execute-only target. If a user passes 
-fsanitize=undefined, there is no error and no warning. However, if the user 
explicitly passes -fsanitize=function on an execute-only target, an error will 
be emitted.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158614

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Basic/Sanitizers.h
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/Sanitizers.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/ubsan-function.c
  clang/test/CodeGenObjCXX/crash-function-type.mm
  clang/test/Driver/fsanitize.c

Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -971,3 +971,14 @@
 
 // RUN: not %clang --target=x86_64-linux-gnu -fsanitize=undefined,function -mcmodel=large %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION-CODE-MODEL
 // CHECK-UBSAN-FUNCTION-CODE-MODEL: error: invalid argument '-fsanitize=function' only allowed with '-mcmodel=small'
+
+// RUN: not %clang --target=x86_64-sie-ps5 -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: not %clang --target=x86_64-sie-ps5 -fsanitize=undefined,function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: %clang --target=x86_64-sie-ps5 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-UNDEFINED
+
+// RUN: not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: not %clang --target=armv6t2-eabi -mexecute-only -fsanitize=undefined,function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-FUNCTION
+// RUN: %clang --target=armv6t2-eabi -mexecute-only -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UBSAN-UNDEFINED
+
+// CHECK-UBSAN-FUNCTION: error: unsupported option '-fsanitize=function' for the execute only target {{('x86_64-sie-ps5'|'armv6t2-unknown-unknown-eabi')}}
+// CHECK-UBSAN-UNDEFINED-NOT: error: unsupported option '-fsanitize=function' for the execute only target {{('x86_64-sie-ps5'|'armv6t2-unknown-unknown-eabi')}}
Index: clang/test/CodeGenObjCXX/crash-function-type.mm
===
--- clang/test/CodeGenObjCXX/crash-function-type.mm
+++ clang/test/CodeGenObjCXX/crash-function-type.mm
@@ -1,3 +1,6 @@
+// Mark test as unsupported on PS5 due to PS5 doesn't support function sanitizer.
+// UNSUPPORTED: target=x86_64-sie-ps5
+
 // RUN: %clang_cc1 -fblocks -fsanitize=function -emit-llvm %s -o %t
 
 void g(void (^)());
Index: clang/test/CodeGen/ubsan-function.c
===
--- clang/test/CodeGen/ubsan-function.c
+++ clang/test/CodeGen/ubsan-function.c
@@ -1,4 +1,6 @@
 // RUN: %clang_cc1 -emit-llvm -triple x86_64 -std=c17 -fsanitize=function %s -o - | FileCheck %s
+// RUN: not %clang_cc1 -emit-llvm -triple x86_64-sie-ps5 -fsanitize=function %s -o 2>&1 | FileCheck %s --check-prefix=UBSAN-FUNCTION-ERR
+// RUN: not %clang_cc1 -emit-llvm -triple armv6t2-unknown-unknown-eabi -target-feature +execute-only -fsanitize=function %s -o 2>&1 | FileCheck %s --check-prefix=UBSAN-FUNCTION-ERR
 
 // CHECK-LABEL: define{{.*}} @call_no_prototype(
 // CHECK-NOT: __ubsan_handle_function_type_mismatch
@@ -7,3 +9,5 @@
 // CHECK-LABEL: define{{.*}} @call_prototype(
 // CHECK: __ubsan_handle_function_type_mismatch
 void call_prototype(void (*f)(void)) { f(); }
+
+// UBSAN-FUNCTION-ERR: error: unsupported option '-fsanitize=function' for the execute only target {{('x86_64-sie-ps5'|'armv6t2-unknown-unknown-eabi')}}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4398,6 +4398,16 @@
 
   ParseLangArgs(LangOpts, Args, DashX, T, Res.getPreprocessorOpts().Includes,
 Diags);
+
+  // An execute-only target doesn't support the function sanitizer. Since `clang
+

[clang] eb0e6c3 - [clang-repl] support code completion at a REPL.

[Vassil Vassilev via cfe-commits]

Author: Fred Fu
Date: 2023-08-23T14:00:59Z
New Revision: eb0e6c3134ef6deafe0a4958e9e1a1214b3c2f14

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

LOG: [clang-repl] support code completion at a REPL.

This patch enabled code completion for ClangREPL. The feature was built upon
three existing Clang components: a list completer for LineEditor, a
CompletionConsumer from SemaCodeCompletion, and the ASTUnit::codeComplete 
method.
The first component serves as the main entry point of handling interactive 
inputs.

Because a completion point for a compiler instance has to be unchanged once it
is set, an incremental compiler instance is created for each code
completion. Such a compiler instance carries over AST context source from the
main interpreter compiler in order to obtain declarations or bindings from
previous input in the same REPL session.

The most important API codeComplete in Interpreter/CodeCompletion is a thin
wrapper that calls with ASTUnit::codeComplete with necessary arguments, such as
a code completion point and a ReplCompletionConsumer, which communicates
completion results from SemaCodeCompletion back to the list completer for the
REPL.

In addition, PCC_TopLevelOrExpression and CCC_TopLevelOrExpression` top levels
were added so that SemaCodeCompletion can treat top level statements like
expression statements at the REPL. For example,

clang-repl> int foo = 42;
clang-repl> f

>From a parser's persective, the cursor is at a top level. If we used code
completion without any changes, PCC_Namespace would be supplied to
Sema::CodeCompleteOrdinaryName, and thus the completion results would not
include foo.

Currently, the way we use PCC_TopLevelOrExpression and
CCC_TopLevelOrExpression is no different from the way we use PCC_Statement
and CCC_Statement respectively.

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

Added: 
clang/include/clang/Interpreter/CodeCompletion.h
clang/lib/Interpreter/CodeCompletion.cpp
clang/test/CodeCompletion/incrememal-mode-completion-no-error.cpp
clang/test/CodeCompletion/incremental-top-level.cpp
clang/unittests/Interpreter/CodeCompletionTest.cpp

Modified: 
clang/include/clang/Frontend/ASTUnit.h
clang/include/clang/Sema/CodeCompleteConsumer.h
clang/include/clang/Sema/Sema.h
clang/lib/Frontend/ASTUnit.cpp
clang/lib/Interpreter/CMakeLists.txt
clang/lib/Interpreter/IncrementalParser.cpp
clang/lib/Interpreter/IncrementalParser.h
clang/lib/Interpreter/Interpreter.cpp
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/Parser.cpp
clang/lib/Sema/CodeCompleteConsumer.cpp
clang/lib/Sema/SemaCodeComplete.cpp
clang/tools/clang-repl/ClangRepl.cpp
clang/tools/libclang/CIndexCodeCompletion.cpp
clang/unittests/Interpreter/CMakeLists.txt

Removed: 




diff  --git a/clang/include/clang/Frontend/ASTUnit.h 
b/clang/include/clang/Frontend/ASTUnit.h
index b762be1c9b1d6d..c6d0d4d7e90233 100644
--- a/clang/include/clang/Frontend/ASTUnit.h
+++ b/clang/include/clang/Frontend/ASTUnit.h
@@ -77,6 +77,7 @@ class Preprocessor;
 class PreprocessorOptions;
 class Sema;
 class TargetInfo;
+class SyntaxOnlyAction;
 
 /// \brief Enumerates the available scopes for skipping function bodies.
 enum class SkipFunctionBodiesScope { None, Preamble, PreambleAndMainFile };
@@ -887,6 +888,10 @@ class ASTUnit {
   /// \param IncludeBriefComments Whether to include brief documentation within
   /// the set of code completions returned.
   ///
+  /// \param Act If supplied, this argument is used to parse the input file,
+  /// allowing customized parsing by overriding SyntaxOnlyAction lifecycle
+  /// methods.
+  ///
   /// FIXME: The Diag, LangOpts, SourceMgr, FileMgr, StoredDiagnostics, and
   /// OwnedBuffers parameters are all disgusting hacks. They will go away.
   void CodeComplete(StringRef File, unsigned Line, unsigned Column,
@@ -897,7 +902,8 @@ class ASTUnit {
 DiagnosticsEngine &Diag, LangOptions &LangOpts,
 SourceManager &SourceMgr, FileManager &FileMgr,
 SmallVectorImpl &StoredDiagnostics,
-SmallVectorImpl &OwnedBuffers);
+SmallVectorImpl &OwnedBuffers,
+std::unique_ptr Act = nullptr);
 
   /// Save this translation unit to a file with the given name.
   ///

diff  --git a/clang/include/clang/Interpreter/CodeCompletion.h 
b/clang/include/clang/Interpreter/CodeCompletion.h
new file mode 100644
index 00..6631d33358600f
--- /dev/null
+++ b/clang/include/clang/Interpreter/CodeCompletion.h
@@ -0,0 +1,33 @@
+//===- CodeCompletion.h - Code Completion for ClangRepl ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https:

[PATCH] D154382: [ClangRepl] support code completion at a REPL

[Vassil Vassilev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGeb0e6c3134ef: [clang-repl] support code completion at a 
REPL. (authored by capfredf, committed by v.g.vassilev).

Changed prior to commit:
  https://reviews.llvm.org/D154382?vs=552418&id=552696#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154382

Files:
  clang/include/clang/Frontend/ASTUnit.h
  clang/include/clang/Interpreter/CodeCompletion.h
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/CodeCompletion.cpp
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/incrememal-mode-completion-no-error.cpp
  clang/test/CodeCompletion/incremental-top-level.cpp
  clang/tools/clang-repl/ClangRepl.cpp
  clang/tools/libclang/CIndexCodeCompletion.cpp
  clang/unittests/Interpreter/CMakeLists.txt
  clang/unittests/Interpreter/CodeCompletionTest.cpp

Index: clang/unittests/Interpreter/CodeCompletionTest.cpp
===
--- /dev/null
+++ clang/unittests/Interpreter/CodeCompletionTest.cpp
@@ -0,0 +1,101 @@
+#include "clang/Interpreter/CodeCompletion.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Interpreter/Interpreter.h"
+#include "clang/Sema/CodeCompleteConsumer.h"
+#include "llvm/LineEditor/LineEditor.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+namespace {
+auto CB = clang::IncrementalCompilerBuilder();
+
+static std::unique_ptr createInterpreter() {
+  auto CI = cantFail(CB.CreateCpp());
+  return cantFail(clang::Interpreter::create(std::move(CI)));
+}
+
+static std::vector runComp(clang::Interpreter &MainInterp,
+llvm::StringRef Prefix,
+llvm::Error &ErrR) {
+  auto CI = CB.CreateCpp();
+  if (auto Err = CI.takeError()) {
+ErrR = std::move(Err);
+return {};
+  }
+
+  auto Interp = clang::Interpreter::create(std::move(*CI));
+  if (auto Err = Interp.takeError()) {
+// log the error and returns an empty vector;
+ErrR = std::move(Err);
+
+return {};
+  }
+
+  std::vector Results;
+
+  codeComplete(
+  const_cast((*Interp)->getCompilerInstance()),
+  Prefix, 1, Prefix.size(), MainInterp.getCompilerInstance(), Results);
+
+  std::vector Comps;
+  for (auto c : convertToCodeCompleteStrings(Results)) {
+if (c.find(Prefix) == 0)
+  Comps.push_back(c.substr(Prefix.size()));
+  }
+
+  return Comps;
+}
+
+TEST(CodeCompletionTest, Sanity) {
+  auto Interp = createInterpreter();
+  if (auto R = Interp->ParseAndExecute("int foo = 12;")) {
+consumeError(std::move(R));
+return;
+  }
+  auto Err = llvm::Error::success();
+  auto comps = runComp(*Interp, "f", Err);
+  EXPECT_EQ((size_t)2, comps.size()); // foo and float
+  EXPECT_EQ(comps[0], std::string("oo"));
+  EXPECT_EQ((bool)Err, false);
+}
+
+TEST(CodeCompletionTest, SanityNoneValid) {
+  auto Interp = createInterpreter();
+  if (auto R = Interp->ParseAndExecute("int foo = 12;")) {
+consumeError(std::move(R));
+return;
+  }
+  auto Err = llvm::Error::success();
+  auto comps = runComp(*Interp, "babanana", Err);
+  EXPECT_EQ((size_t)0, comps.size()); // foo and float
+  EXPECT_EQ((bool)Err, false);
+}
+
+TEST(CodeCompletionTest, TwoDecls) {
+  auto Interp = createInterpreter();
+  if (auto R = Interp->ParseAndExecute("int application = 12;")) {
+consumeError(std::move(R));
+return;
+  }
+  if (auto R = Interp->ParseAndExecute("int apple = 12;")) {
+consumeError(std::move(R));
+return;
+  }
+  auto Err = llvm::Error::success();
+  auto comps = runComp(*Interp, "app", Err);
+  EXPECT_EQ((size_t)2, comps.size());
+  EXPECT_EQ((bool)Err, false);
+}
+
+TEST(CodeCompletionTest, CompFunDeclsNoError) {
+  auto Interp = createInterpreter();
+  auto Err = llvm::Error::success();
+  auto comps = runComp(*Interp, "void app(", Err);
+  EXPECT_EQ((bool)Err, false);
+}
+
+} // anonymous namespace
Index: clang/unittests/Interpreter/CMakeLists.txt
===
--- clang/unittests/Interpreter/CMakeLists.txt
+++ clang/unittests/Interpreter/CMakeLists.txt
@@ -9,6 +9,7 @@
 add_clang_unittest(ClangReplInterpreterTests
   IncrementalProcessingTest.cpp
   InterpreterTest.cpp
+  CodeCompletionTest.cpp
   )
 target_link_libraries(ClangReplInterpreterTests PUBLIC
   clangAST
Index: clang/tools/libclang/CIndexCodeCompletion.cpp

[PATCH] D148997: [clang] Add a new annotation token: annot_repl_input_end

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D148997#4607833 , @bnbarham wrote:

> In D148997#4561620 , @v.g.vassilev 
> wrote:
>
>> So, in that case we should bring back the boolean flag for incremental 
>> processing and keep the `IncrementalExtensions` LanguageOption separate. In 
>> that case `IncrementalExtensions` would mean that we must turn on 
>> incremental processing for managing lifetime and only use the language 
>> option when extending the parsing logic. However, I think the problem would 
>> be what to do with the `tok::eof` and `tok::annot_repl_input_end`? I'd 
>> probably need @aaron.ballman or @rsmith here...
>
> Would you be happy to make that change, or should I put it up? Separating the 
> options and what to do about the token in general could be separate PRs.

If you could do it would be better as it would capture better your concrete use 
case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148997

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

[PATCH] D158601: [Clang] Always constant-evaluate operands of comparisons to nullptr

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:13312
+!EvaluatePointer(E->getRHS(), Res, Info))
+  return false;
 return Success(CmpResult::Equal, E);

cor3ntin wrote:
> tbaeder wrote:
> > I kinda expected the fix for this bug to simply //remove// a special 
> > case... does just falling through to the `DoAfter()` call not work?
> Nope, EvaluateComparisonBinaryOperator does compute the result of the 
> comparison. `DoAfter` only adjust the result.
> Removing that block of codes lead to a whole bunch of test failures
Shame, but ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158601

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

[PATCH] D158615: [clang] Emit an error if variable ends up with incomplete array type

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon created this revision.
Herald added a project: All.
Fznamznon requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This adds an error if variable with incomplete type has initializer with
incomplete type, so it is not possible to deduce array size from
initializer.

Fixes https://github.com/llvm/llvm-project/issues/37257


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158615

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/gh37257.cpp


Index: clang/test/SemaCXX/gh37257.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/gh37257.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template
+T&& create();
+
+template
+void test() {
+  T t(create()...); // expected-error{{variable has incomplete type 
'int[]'}}
+  (void) t;
+}
+
+struct A;
+
+int main() {
+  test(); // expected-note {{in instantiation of function template 
specialization 'test' requested here}}
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -13449,6 +13449,21 @@
 IsParenListInit = !InitSeq.steps().empty() &&
   InitSeq.step_begin()->Kind ==
   InitializationSequence::SK_ParenthesizedListInit;
+QualType VDeclType = VDecl->getType();
+if (!VDeclType->isDependentType() &&
+Context.getAsIncompleteArrayType(VDeclType)) {
+  if (Init && !Init->getType().isNull() &&
+  !Init->getType()->isDependentType()) {
+if (Context.getAsIncompleteArrayType(Init->getType())) {
+  // Bail out if it is not possible to deduce array size from the
+  // initializer.
+  Diag(VDecl->getLocation(), diag::err_typecheck_decl_incomplete_type)
+  << VDeclType;
+  VDecl->setInvalidDecl();
+  return;
+}
+  }
+}
   }
 
   // Check for self-references within variable initializers.
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -163,6 +163,9 @@
   ``await_suspend`` could be misoptimized, including accesses to the awaiter
   object itself.
   (`#56301 `_)
+- Clang now emits an error if it is not possible to deduce array size for a
+  variable with incomplete array type.
+  (`#37257 `_)
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/SemaCXX/gh37257.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/gh37257.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template
+T&& create();
+
+template
+void test() {
+  T t(create()...); // expected-error{{variable has incomplete type 'int[]'}}
+  (void) t;
+}
+
+struct A;
+
+int main() {
+  test(); // expected-note {{in instantiation of function template specialization 'test' requested here}}
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -13449,6 +13449,21 @@
 IsParenListInit = !InitSeq.steps().empty() &&
   InitSeq.step_begin()->Kind ==
   InitializationSequence::SK_ParenthesizedListInit;
+QualType VDeclType = VDecl->getType();
+if (!VDeclType->isDependentType() &&
+Context.getAsIncompleteArrayType(VDeclType)) {
+  if (Init && !Init->getType().isNull() &&
+  !Init->getType()->isDependentType()) {
+if (Context.getAsIncompleteArrayType(Init->getType())) {
+  // Bail out if it is not possible to deduce array size from the
+  // initializer.
+  Diag(VDecl->getLocation(), diag::err_typecheck_decl_incomplete_type)
+  << VDeclType;
+  VDecl->setInvalidDecl();
+  return;
+}
+  }
+}
   }
 
   // Check for self-references within variable initializers.
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -163,6 +163,9 @@
   ``await_suspend`` could be misoptimized, including accesses to the awaiter
   object itself.
   (`#56301 `_)
+- Clang now emits an error if it is not possible to deduce array size for a
+  variable with incomplete array type.
+  (`#37257 `_)
 
 Bug Fixes to Compiler Builtins
 ^^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/

[PATCH] D154603: [analyzer][clangsa] Add new option to alpha.security.cert.InvalidPtrChecker

[Donát Nagy via Phabricator via cfe-commits]

donat.nagy added a comment.

The change looks promising, I only have minor remarks.




Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:125
 
-  const NoteTag *Note =
-  C.getNoteTag([SymbolicEnvPtrRegion, FunctionName](
-   PathSensitiveBugReport &BR, llvm::raw_ostream &Out) {
-if (!BR.isInteresting(SymbolicEnvPtrRegion))
-  return;
-Out << '\'' << FunctionName
-<< "' call may invalidate the environment parameter of 'main'";
-  });
+  ExplodedNode *CurrentChainEnd = nullptr;
+

gamesh411 wrote:
> steakhal wrote:
> > donat.nagy wrote:
> > > Perhaps add a comment that clarifies that passing a `nullptr` as the 
> > > ExplodedNode to `addTransition` is equivalent to specifying the current 
> > > node. I remember this because I was studying its implementation recently, 
> > > but I would've been surprised and suspicious otherwise.
> > If `nullptr` is equivalent with `C.getPredecessor()` inside 
> > `addTransition()`, why not simply initialize it to that value instead of to 
> > `nullptr`?
> I ended up using C.getPredecessor() instead of explaining; this seems a bit 
> more intuitive (if such a thing even exists in CSA).
> if such a thing even exists in CSA 
😆 




Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:121-122
+const NoteTag *Note =
+C.getNoteTag([Region, FunctionName = SmallString<64>{FunctionName},
+  Message = SmallString<256>{Message}](
+ PathSensitiveBugReport &BR, llvm::raw_ostream &Out) {

Minor nitpick: in situations like this, when we want to save an already 
composed string, `std::string` is better than `SmallString` because it doesn't 
occupy more memory than the actual length of the string. (OTOH `SmallString` is 
better for buffer variables, I've seen code that creates a SmallString, 
composes the message in it, then converts it to a `std::string` for longer-term 
storage.)

Of course these are all just inconsequential micro-optimization details...



Comment at: clang/test/Analysis/invalid-ptr-checker.c:10
+// RUN: alpha.security.cert.env.InvalidPtr:InvalidatingGetEnv=true \
+// RUN: -analyzer-output=text -verify=pedantic -Wno-unused %s
+

Use `-verify=expected,pedantic` here and then you can eliminate the 
`pedantic-warning` lines that duplicate the messages of `expected-warning`s.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154603

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

[PATCH] D156858: Add Documentation for Execution Results Handling in Clang-REPL

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev accepted this revision.
v.g.vassilev added a comment.

The pre-merge check seems to fail at clang-format for some reason. This patch 
does not change anything that should be formatted by clang-format.

We will try to add the images as editable graphviz content. We need to see if 
the infrastructure allows for this as suggested here: 
https://discourse.llvm.org/t/any-tool-for-creating-editable-diagrams-in-llvm-clang-repl-documentation-similar-to-mermaid-ingithub/72729/2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156858

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

[PATCH] D158615: [clang] Emit an error if variable ends up with incomplete array type

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:13460
+  // initializer.
+  Diag(VDecl->getLocation(), diag::err_typecheck_decl_incomplete_type)
+  << VDeclType;

Not sure about the error message though, maybe it makes sense to add a message 
similar to `err_new_array_size_unknown_from_init` .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158615

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

[clang] 3edd338 - [clang-repl] Add Documentation for Execution Results Handling.

[Vassil Vassilev via cfe-commits]

Author: Krishna-13-cyber
Date: 2023-08-23T14:12:12Z
New Revision: 3edd338a6407d9410f6a283c5dc32ba676ac0b8f

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

LOG: [clang-repl] Add Documentation for Execution Results Handling.

This patch adds documentation for execution results handling in Clang-REPL with
the below features:

  * Automatic Printf feature
  * Value Synthesis feature
  * Pretty Printing feature

I am issuing this patch on behalf of @QuillPusher

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

Added: 


Modified: 
clang/docs/ClangRepl.rst
clang/docs/conf.py

Removed: 




diff  --git a/clang/docs/ClangRepl.rst b/clang/docs/ClangRepl.rst
index bd99bc82f1..e0050f1a3b56ee 100644
--- a/clang/docs/ClangRepl.rst
+++ b/clang/docs/ClangRepl.rst
@@ -213,6 +213,411 @@ concept helps support advanced use cases such as template 
instantiations on dema
 automatic language interoperability. It also helps static languages such as 
C/C++ become
 apt for data science.
 
+Execution Results Handling in Clang-Repl
+
+
+Execution Results Handling features discussed below help extend the Clang-Repl
+functionality by creating an interface between the execution results of a
+program and the compiled program.
+
+1. **Capture Execution Results**: This feature helps capture the execution 
results
+of a program and bring them back to the compiled program.
+
+2. **Dump Captured Execution Results**: This feature helps create a temporary 
dump
+for Value Printing/Automatic Printf, that is, to display the value and type of
+the captured data.
+
+
+1. Capture Execution Results
+
+
+In many cases, it is useful to bring back the program execution result to the
+compiled program. This result can be stored in an object of type **Value**.
+
+How Execution Results are captured (Value Synthesis):
+-
+
+The synthesizer chooses which expression to synthesize, and then it replaces
+the original expression with the synthesized expression. Depending on the
+expression type, it may choose to save an object (``LastValue``) of type 
'value'
+while allocating memory to it (``SetValueWithAlloc()``), or not (
+``SetValueNoAlloc()``).
+
+.. graphviz::
+:name: valuesynthesis
+:caption: Value Synthesis
+:alt: Shows how an object of type 'Value' is synthesized
+:align: center
+
+ digraph "valuesynthesis" {
+ rankdir="LR";
+ graph [fontname="Verdana", fontsize="12"];
+ node [fontname="Verdana", fontsize="12"];
+ edge [fontname="Sans", fontsize="9"];
+
+ start [label=" Create an Object \n 'Last Value' \n of type 'Value' ", 
shape="note", fontcolor=white, fillcolor="#ff", style=filled];
+ assign [label=" Assign the result \n to the 'LastValue' \n (based on 
respective \n Memory Allocation \n scenario) ", shape="box"]
+ print [label=" Pretty Print \n the Value Object ", shape="Msquare", 
fillcolor="yellow", style=filled];
+ start -> assign;
+ assign -> print;
+
+   subgraph SynthesizeExpression {
+ synth [label=" SynthesizeExpr() ", shape="note", fontcolor=white, 
fillcolor="#ff", style=filled];
+ mem [label=" New Memory \n Allocation? ", shape="diamond"];
+ withaloc [label=" SetValueWithAlloc() ", shape="box"];
+ noaloc [label=" SetValueNoAlloc() ", shape="box"];
+ right [label=" 1. RValue Structure \n (a temporary value)", 
shape="box"];
+ left2 [label=" 2. LValue Structure \n (a variable with \n an 
address)", shape="box"];
+ left3 [label=" 3. Built-In Type \n (int, float, etc.)", 
shape="box"];
+ output [label=" move to 'Assign' step ", shape="box"];
+
+ synth -> mem;
+ mem -> withaloc [label="Yes"];
+ mem -> noaloc [label="No"];
+ withaloc -> right;
+ noaloc -> left2;
+ noaloc -> left3;
+ right -> output;
+ left2 -> output;
+ left3 -> output;
+  }
+output -> assign
+  }
+
+Where is the captured result stored?
+
+
+``LastValue`` holds the last result of the value printing. It is a class member
+because it can be accessed even after subsequent inputs.
+
+**Note:** If no value printing happens, then it is in an invalid state.
+
+Improving Efficiency and User Experience
+
+
+The Value object is essentially used to create a mapping between an expression
+'type' and the allocated 'memory'. Built-in types (bool, char, int,
+float, double, etc.) are copyable. Their their memory allocation siz

[PATCH] D156858: Add Documentation for Execution Results Handling in Clang-REPL

[Vassil Vassilev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3edd338a6407: [clang-repl] Add Documentation for Execution 
Results Handling. (authored by Krishna-13-cyber, committed by v.g.vassilev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156858

Files:
  clang/docs/ClangRepl.rst
  clang/docs/conf.py

Index: clang/docs/conf.py
===
--- clang/docs/conf.py
+++ clang/docs/conf.py
@@ -49,6 +49,7 @@
 if sphinx.version_info >= (3, 0):
 # This requires 0.5 or later.
 extensions.append("recommonmark")
+extensions.append('sphinx.ext.graphviz')
 else:
 source_parsers = {".md": "recommonmark.parser.CommonMarkParser"}
 source_suffix[".md"] = "markdown"
Index: clang/docs/ClangRepl.rst
===
--- clang/docs/ClangRepl.rst
+++ clang/docs/ClangRepl.rst
@@ -213,6 +213,411 @@
 automatic language interoperability. It also helps static languages such as C/C++ become
 apt for data science.
 
+Execution Results Handling in Clang-Repl
+
+
+Execution Results Handling features discussed below help extend the Clang-Repl
+functionality by creating an interface between the execution results of a
+program and the compiled program.
+
+1. **Capture Execution Results**: This feature helps capture the execution results
+of a program and bring them back to the compiled program.
+
+2. **Dump Captured Execution Results**: This feature helps create a temporary dump
+for Value Printing/Automatic Printf, that is, to display the value and type of
+the captured data.
+
+
+1. Capture Execution Results
+
+
+In many cases, it is useful to bring back the program execution result to the
+compiled program. This result can be stored in an object of type **Value**.
+
+How Execution Results are captured (Value Synthesis):
+-
+
+The synthesizer chooses which expression to synthesize, and then it replaces
+the original expression with the synthesized expression. Depending on the
+expression type, it may choose to save an object (``LastValue``) of type 'value'
+while allocating memory to it (``SetValueWithAlloc()``), or not (
+``SetValueNoAlloc()``).
+
+.. graphviz::
+:name: valuesynthesis
+:caption: Value Synthesis
+:alt: Shows how an object of type 'Value' is synthesized
+:align: center
+
+ digraph "valuesynthesis" {
+ rankdir="LR";
+ graph [fontname="Verdana", fontsize="12"];
+ node [fontname="Verdana", fontsize="12"];
+ edge [fontname="Sans", fontsize="9"];
+
+ start [label=" Create an Object \n 'Last Value' \n of type 'Value' ", shape="note", fontcolor=white, fillcolor="#ff", style=filled];
+ assign [label=" Assign the result \n to the 'LastValue' \n (based on respective \n Memory Allocation \n scenario) ", shape="box"]
+ print [label=" Pretty Print \n the Value Object ", shape="Msquare", fillcolor="yellow", style=filled];
+ start -> assign;
+ assign -> print;
+
+   subgraph SynthesizeExpression {
+ synth [label=" SynthesizeExpr() ", shape="note", fontcolor=white, fillcolor="#ff", style=filled];
+ mem [label=" New Memory \n Allocation? ", shape="diamond"];
+ withaloc [label=" SetValueWithAlloc() ", shape="box"];
+ noaloc [label=" SetValueNoAlloc() ", shape="box"];
+ right [label=" 1. RValue Structure \n (a temporary value)", shape="box"];
+ left2 [label=" 2. LValue Structure \n (a variable with \n an address)", shape="box"];
+ left3 [label=" 3. Built-In Type \n (int, float, etc.)", shape="box"];
+ output [label=" move to 'Assign' step ", shape="box"];
+
+ synth -> mem;
+ mem -> withaloc [label="Yes"];
+ mem -> noaloc [label="No"];
+ withaloc -> right;
+ noaloc -> left2;
+ noaloc -> left3;
+ right -> output;
+ left2 -> output;
+ left3 -> output;
+  }
+output -> assign
+  }
+
+Where is the captured result stored?
+
+
+``LastValue`` holds the last result of the value printing. It is a class member
+because it can be accessed even after subsequent inputs.
+
+**Note:** If no value printing happens, then it is in an invalid state.
+
+Improving Efficiency and User Experience
+
+
+The Value object is essentially used to create a mapping between an expression
+'type' and the allocated 'memory'. Built-in types (bool, char, int,
+float, double, etc.) are copyable. Their their memory allocation size is know

[PATCH] D156259: [clang-format] Fix a bug that erroneously placed function arguments on a new line despite all arguments being able to fit on the same line.

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 updated this revision to Diff 552701.
jp4a50 added a comment.

Minor refactoring.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156259

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -22177,8 +22177,25 @@
"  }\n"
"};");
 
-  // Multiple lambdas in the same parentheses change indentation rules. These
-  // lambdas are forced to start on new lines.
+  // Lambdas that fit on a single line within an argument list are not forced
+  // onto new lines.
+  verifyFormat("SomeFunction([] {});");
+  verifyFormat("SomeFunction(0, [] {});");
+  verifyFormat("SomeFunction([] {}, 0);");
+  verifyFormat("SomeFunction(0, [] {}, 0);");
+  verifyFormat("SomeFunction([] { return 0; }, 0);");
+  verifyFormat("SomeFunction(a, [] { return 0; }, b);");
+  verifyFormat("SomeFunction([] { return 0; }, [] { return 0; });");
+  verifyFormat("SomeFunction([] { return 0; }, [] { return 0; }, b);");
+  verifyFormat("auto lng =\n"
+   "SomeFunction([] { return 0; }, [] { return 0; }, b);");
+  // Exceeded column limit. We need to break.
+  verifyFormat("auto lngName = SomeFunction(\n"
+   "[] { return anotherLooonngName; }, [] { "
+   "return 0; }, b);");
+
+  // Multiple multi-line lambdas in the same parentheses change indentation
+  // rules. These lambdas are always forced to start on new lines.
   verifyFormat("SomeFunction(\n"
"[]() {\n"
"  //\n"
@@ -22187,7 +22204,7 @@
"  //\n"
"});");
 
-  // A lambda passed as arg0 is always pushed to the next line.
+  // A multi-line lambda passed as arg0 is always pushed to the next line.
   verifyFormat("SomeFunction(\n"
"[this] {\n"
"  //\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -5203,30 +5203,6 @@
   return true;
   }
 
-  // Deal with lambda arguments in C++ - we want consistent line breaks whether
-  // they happen to be at arg0, arg1 or argN. The selection is a bit nuanced
-  // as aggressive line breaks are placed when the lambda is not the last arg.
-  if ((Style.Language == FormatStyle::LK_Cpp ||
-   Style.Language == FormatStyle::LK_ObjC) &&
-  Left.is(tok::l_paren) && Left.BlockParameterCount > 0 &&
-  !Right.isOneOf(tok::l_paren, TT_LambdaLSquare)) {
-// Multiple lambdas in the same function call force line breaks.
-if (Left.BlockParameterCount > 1)
-  return true;
-
-// A lambda followed by another arg forces a line break.
-if (!Left.Role)
-  return false;
-auto Comma = Left.Role->lastComma();
-if (!Comma)
-  return false;
-auto Next = Comma->getNextNonComment();
-if (!Next)
-  return false;
-if (!Next->isOneOf(TT_LambdaLSquare, tok::l_brace, tok::caret))
-  return true;
-  }
-
   return false;
 }
 
Index: clang/lib/Format/ContinuationIndenter.h
===
--- clang/lib/Format/ContinuationIndenter.h
+++ clang/lib/Format/ContinuationIndenter.h
@@ -433,6 +433,9 @@
   /// literal sequence, 0 otherwise.
   unsigned StartOfStringLiteral;
 
+  /// Disallow line breaks for this line.
+  bool NoLineBreak;
+
   /// A stack keeping track of properties applying to parenthesis
   /// levels.
   SmallVector Stack;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -260,6 +260,7 @@
/*NoLineBreak=*/false));
   State.NoContinuation = false;
   State.StartOfStringLiteral = 0;
+  State.NoLineBreak = false;
   State.StartOfLineLevel = 0;
   State.LowestLevelOnLine = 0;
   State.IgnoreStackForComparison = false;
@@ -342,7 +343,7 @@
 return true;
   }
 
-  return !CurrentState.NoLineBreak;
+  return !State.NoLineBreak && !CurrentState.NoLineBreak;
 }
 
 bool ContinuationIndenter::mustBreak(const LineState &State) {
@@ -653,6 +654,38 @@
   const FormatToken &Previous = *State.NextToken->Previous;
   auto &CurrentState = State.Stack.back();
 
+  bool DisallowLineBreaksOnThisLine = Style.isCpp() && [&Current] {
+// Deal with lambda arguments in C++. The aim here is to ensure that we
+// don't over-indent lambda function bodies when lambdas are p

[PATCH] D158616: To allow RAV to visit initializer when bitfield and initializer both given

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 created this revision.
NoumanAmir657 added reviewers: nicholas, Eugene.Zelenko.
Herald added a project: All.
NoumanAmir657 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The RAV does not visit the initializer when the bitfield is given because it 
uses if-else statement. Code was changed to two seperate ifs so that it can 
visit both of them if both are present.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158616

Files:
  clang/include/clang/AST/RecursiveASTVisitor.h


Index: clang/include/clang/AST/RecursiveASTVisitor.h
===
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2124,7 +2124,7 @@
   TRY_TO(TraverseDeclaratorHelper(D));
   if (D->isBitField())
 TRY_TO(TraverseStmt(D->getBitWidth()));
-  else if (D->hasInClassInitializer())
+  if (D->hasInClassInitializer())
 TRY_TO(TraverseStmt(D->getInClassInitializer()));
 })
 


Index: clang/include/clang/AST/RecursiveASTVisitor.h
===
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2124,7 +2124,7 @@
   TRY_TO(TraverseDeclaratorHelper(D));
   if (D->isBitField())
 TRY_TO(TraverseStmt(D->getBitWidth()));
-  else if (D->hasInClassInitializer())
+  if (D->hasInClassInitializer())
 TRY_TO(TraverseStmt(D->getInClassInitializer()));
 })
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156259: [clang-format] Fix a bug that erroneously placed function arguments on a new line despite all arguments being able to fit on the same line.

[Jon Phillips via Phabricator via cfe-commits]

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

@owenpan right you are! Missed those somehow. Made a further two changes. Hope 
I haven't missed anything else!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156259

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

[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

The PS5 bits LGTM, but as I'm not familiar with the ARM aspects I won't give 
final approval.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158614

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

[PATCH] D158582: [AMDGPU] Respect unresolved symbol option if forwarded to linker

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

The `-Wl` and `-Xlinker` options are intended for the host linker and we 
intentionally do not pass them to the device linker.

If users want to pass options to the device linker, they need to use 
-Xoffload-linker.

There are multiple options affecting the handling of unresolved symbols. I 
think the proper way is to use `--no-undefined` as the default, which is always 
passed before those options from `-Xoffload-linker` so that the latter can 
override the former.

I believe the driver already passes `-Xoffload-linker`  options to 
amdgpu::Linker::ConstructJob by Args. I think we probably only need to move all 
the default options (line 563-566) to 554 so that they can be overridden.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158582

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

[PATCH] D158293: [NFC][Clang] Fix static code analyzer concern about null value dereference

[Soumi Manna via Phabricator via cfe-commits]

Manna updated this revision to Diff 552706.
Manna edited the summary of this revision.
Manna removed reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.

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

https://reviews.llvm.org/D158293

Files:
  clang/lib/Lex/PPDirectives.cpp


Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -491,7 +491,8 @@
   llvm::SaveAndRestore SARSkipping(SkippingExcludedConditionalBlock, true);
 
   ++NumSkipped;
-  assert(!CurTokenLexer && CurPPLexer && "Lexing a macro, not a file?");
+  assert(!CurTokenLexer && CurPPLexer && CurLexer &&
+ "Lexing a macro, not a file?");
 
   if (PreambleConditionalStack.reachedEOFWhileSkipping())
 PreambleConditionalStack.clearSkipInfo();


Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -491,7 +491,8 @@
   llvm::SaveAndRestore SARSkipping(SkippingExcludedConditionalBlock, true);
 
   ++NumSkipped;
-  assert(!CurTokenLexer && CurPPLexer && "Lexing a macro, not a file?");
+  assert(!CurTokenLexer && CurPPLexer && CurLexer &&
+ "Lexing a macro, not a file?");
 
   if (PreambleConditionalStack.reachedEOFWhileSkipping())
 PreambleConditionalStack.clearSkipInfo();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158582: [AMDGPU] Respect unresolved symbol option if forwarded to linker

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D158582#4610023 , @yaxunl wrote:

> The `-Wl` and `-Xlinker` options are intended for the host linker and we 
> intentionally do not pass them to the device linker.
>
> If users want to pass options to the device linker, they need to use 
> -Xoffload-linker.
>
> There are multiple options affecting the handling of unresolved symbols. I 
> think the proper way is to use `--no-undefined` as the default, which is 
> always passed before those options from `-Xoffload-linker` so that the latter 
> can override the former.
>
> I believe the driver already passes `-Xoffload-linker`  options to 
> amdgpu::Linker::ConstructJob by Args. I think we probably only need to move 
> all the default options (line 563-566) to 554 so that they can be overridden.

Yeah, the original problem was not being able to overload the defaults. This 
fundamentally is just because the `-Wl` options are being added in 
`AddLinkerInputs` before the defaults. Surprised I didn't catch that, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158582

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

[PATCH] D158293: [NFC][Clang] Fix static code analyzer concern about null value dereference

[Soumi Manna via Phabricator via cfe-commits]

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



Comment at: clang/lib/Lex/PPDirectives.cpp:555
   while (true) {
 CurLexer->Lex(Tok);
 

tahonermann wrote:
> I don't think this change is sufficient. If `CurLexer` is null, then the 
> `else` branch will be entered and an unconditional dereference occurs there 
> as well. It looks like more analysis is needed, but perhaps the correct fix 
> is to add a non-null assertion somewhere above.
Thank you @tahonermann for review and feedback. Yes, my analysis was wrong. 

>>perhaps the correct fix is to add a non-null assertion somewhere above.

It sounds reasonable to me. I have added an assert above.


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

https://reviews.llvm.org/D158293

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

[PATCH] D158499: [analyzer] Compute FAM dynamic size

[Donát Nagy via Phabricator via cfe-commits]

donat.nagy added a comment.

In D158499#4608974 , @danix800 wrote:

> One of the observable issue with inconsistent size type is
>
>   void clang_analyzer_eval(int);
>   
>   typedef unsigned long long size_t;
>   void *malloc(unsigned long size);
>   void free(void *);
>   
>   void symbolic_longlong_and_int0(long long len) {
> char *a = malloc(5);
> (void)a[len + 1]; // no-warning
> // len: [-1,3]
> clang_analyzer_eval(-1 <= len && len <= 3); // expected-warning {{TRUE}}
> clang_analyzer_eval(0 <= len);  // expected-warning 
> {{UNKNOWN}}
> clang_analyzer_eval(len <= 2);  // expected-warning 
> {{UNKNOWN}}
> free(a);
>   }
>
> which is extracted from 
> `clang/test/Analysis/array-bound-v2-constraint-check.c`,
> with `DynamicMemoryModeling` turned on,
> the second warning does not hold anymore: `clang_analyzer_eval(0 <= len);`
> will be reported as `TRUE` which is not expected.
>
> `DynamicMemoryModeling` will record the extent of allocated memory as `5ULL`,
> `ArrayBoundV2` will do `len + 1 < 5ULL` assumption, simplified to `len < 
> 4ULL`,
> which casts `len` to unsigned, dropping `-1`, similar to
>
>   void clang_analyzer_eval(int);
>   
>   void test(int len) {
> if (len >= -1 && len <= 4U) { // len is promoted into unsigned, thus can 
> never be negative
> clang_analyzer_eval(0 <= len);  // TRUE
> }
>   }

This issue is very interesting and scary -- I wouldn't have guessed that the 
core constraint handling algorithms are so buggy that things like this can 
happen 😦. I reproduced the issue and double-checked that the logic error is 
//not// in ArrayBoundV2 (despite that its simplification algorithm is messy and 
suspicious); I'd be very happy if this issue could be resolved (perhaps along 
with other inaccuracies of APSInt math...).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158499

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

[clang] 9e150ad - [NFC][clang] EditedSource::applyRewrites - useless call

[via cfe-commits]

Author: Manna, Soumi
Date: 2023-08-23T07:39:20-07:00
New Revision: 9e150adaea7be2d87fb13255671584719a560939

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

LOG: [NFC][clang] EditedSource::applyRewrites - useless call

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/lib/Edit/EditedSource.cpp

Removed: 




diff  --git a/clang/lib/Edit/EditedSource.cpp b/clang/lib/Edit/EditedSource.cpp
index a3386b2489b07c..5e77b0141e8a0e 100644
--- a/clang/lib/Edit/EditedSource.cpp
+++ b/clang/lib/Edit/EditedSource.cpp
@@ -430,7 +430,7 @@ void EditedSource::applyRewrites(EditsReceiver &receiver,
 if (offs == CurEnd) {
   StrVec += act.Text;
   CurLen += act.RemoveLen;
-  CurEnd.getWithOffset(act.RemoveLen);
+  CurEnd = CurEnd.getWithOffset(act.RemoveLen);
   continue;
 }
 



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

[PATCH] D158227: [clang] EditedSource::applyRewrites - useless call

[Soumi Manna via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9e150adaea7b: [NFC][clang] EditedSource::applyRewrites - 
useless call (authored by Manna).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158227

Files:
  clang/lib/Edit/EditedSource.cpp


Index: clang/lib/Edit/EditedSource.cpp
===
--- clang/lib/Edit/EditedSource.cpp
+++ clang/lib/Edit/EditedSource.cpp
@@ -430,7 +430,7 @@
 if (offs == CurEnd) {
   StrVec += act.Text;
   CurLen += act.RemoveLen;
-  CurEnd.getWithOffset(act.RemoveLen);
+  CurEnd = CurEnd.getWithOffset(act.RemoveLen);
   continue;
 }
 


Index: clang/lib/Edit/EditedSource.cpp
===
--- clang/lib/Edit/EditedSource.cpp
+++ clang/lib/Edit/EditedSource.cpp
@@ -430,7 +430,7 @@
 if (offs == CurEnd) {
   StrVec += act.Text;
   CurLen += act.RemoveLen;
-  CurEnd.getWithOffset(act.RemoveLen);
+  CurEnd = CurEnd.getWithOffset(act.RemoveLen);
   continue;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158227: [clang] EditedSource::applyRewrites - useless call

[Soumi Manna via Phabricator via cfe-commits]

Manna added a comment.

The failure seems unrelated to my change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158227

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

[PATCH] D155290: [PGO] Use Unique Profile Files when New Processes are Forked

[Qiongsi Wu via Phabricator via cfe-commits]

qiongsiwu1 added a comment.

Thanks for chiming in @phosek !

In D155290#4587438 , @phosek wrote:

> Can we avoid the use of `pthread_atfork` and the dependency on pthreads? 
> Using pthreads inside the profile runtime implementation is problematic in 
> the case where you want to instrument the libc itself which is used on some 
> platforms and it's one of the reasons why we would like to eliminate the 
> dependency on libc altogether and use `sanitizer_common` instead, see 
> https://discourse.llvm.org/t/using-c-and-sanitizer-common-in-profile-runtime/58629.

Yes that is something I will try. The case illustrated in 
`https://discourse.llvm.org/t/using-c-and-sanitizer-common-in-profile-runtime/58629`
 makes sense. Is there something similar to `pthread_atfork` in 
`sanitizer_common` you would recommend? This functionality needs a hook at 
fork, and it does not really rely on pthreads.

> In D155290#4585607 , @qiongsiwu1 
> wrote:
>
>> In D155290#4582825 , @MaskRay 
>> wrote:
>>
>>> Using `%p` should not magically change the behavior and I am unsure the 
>>> result is better for PGO. 
>>> If we decide to provide such a mode, it needs to be opt-in by adding a new 
>>> format specifier or API.
>>> Can you elaborate separate profile files are going to be useful?
>>
>> Thanks again for taking a look at this patch!
>>
>> Sure! We are generating separate profiles through `%p` for two reasons. 
>> First, we think that `%p` implies a profile per process, even if `exec` is 
>> not called after `fork`. It is unexpected that with `%p`, a program that 
>> forks child processes only create one profile file from the parent. This is 
>> why we did not create a new API or create a new pattern. Second, we are 
>> trying to catch non-gracefully terminated processes during profile generate. 
>> We observed a scenario where a hot function having all 0 counters. The 
>> reasons was the child process was created by a fork (but no `exec` 
>> followed), and then was terminated probably by a signal, instead of going 
>> through `exit()`. Such a process did not have a chance to write back the 
>> profile for merging. The parent process took a different call path and never 
>> called the hot function. We would like to detect such a situation for a 
>> particular process. Hence this patch creates an empty profile file if a 
>> process is not terminated gracefully. Using `%p` is probably the most 
>> natural choice when debugging such issues.
>>
>> One can argue that we may look into the continuous mode to manage abnormal 
>> terminations during profile generate. I have not looked too closely at that, 
>> but we think it is still useful for a user to tell that some processes did 
>> not write any profile back, for debugging, or for diagnostics.
>
> Handling cases like these is exactly why the continous mode was created. Your 
> solution only covers a single case, but there's many scenarios where process 
> can terminate abnormally and fail to invoke `atexit` hooks which would 
> results in profile not being written out.

Yes I agree there is overlap with continuous mode. That said, there is still 
the first reason that we need to implement this feature, i.e. no new profile 
file is created at fork when `%p` is present in the file name pattern. My 
understanding is that `%p` implies a profile file per process created, see the 
new test case `compiler-rt/test/profile/Posix/instrprof-fork.c`. My expectation 
when running the test case is that we should have two different profile files, 
while currently we only have one profile file created. As @MaskRay pointed out 
in his comment for this test case, the profile files generated for the two 
processes can be very different. In the context of fork without exec, the 
current implementation will only generate one profile file. Effectively the 
result is identical with the profile generated without `%p`, if I understand 
the current implementation correctly. Additionally, continuous mode is not 
available on all platforms (for example AIX which is our use case). We are 
working on getting the continuous mode online, but `%p` as a debugging feature 
in the context of fork without exec seems to be useful.

With this said, if it is indeed our //intention// that new profile files should 
not be created upon fork when `%p` is present in the file pattern, I am happy 
to choose alternative routes. I would love to know why we have such an 
intention before moving on. I understand that this is an //existing// feature 
so we may be hesitant to change its behavior, and we would rather not have 
dependency on `pthread`, which is an implementation issue. Are there additional 
reasons that we prefer the current behavior?

Thanks again for the input!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D1

[clang] f94a937 - Revert "[clang-repl] support code completion at a REPL."

[Vassil Vassilev via cfe-commits]

Author: Vassil Vassilev
Date: 2023-08-23T14:46:15Z
New Revision: f94a937cb37a949cca7a084669c634a87b1d2bb1

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

LOG: Revert "[clang-repl] support code completion at a REPL."

This reverts commit eb0e6c3134ef6deafe0a4958e9e1a1214b3c2f14 due to failures in
clangd such as https://lab.llvm.org/buildbot/#/builders/57/builds/29377

Added: 


Modified: 
clang/include/clang/Frontend/ASTUnit.h
clang/include/clang/Sema/CodeCompleteConsumer.h
clang/include/clang/Sema/Sema.h
clang/lib/Frontend/ASTUnit.cpp
clang/lib/Interpreter/CMakeLists.txt
clang/lib/Interpreter/IncrementalParser.cpp
clang/lib/Interpreter/IncrementalParser.h
clang/lib/Interpreter/Interpreter.cpp
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/Parser.cpp
clang/lib/Sema/CodeCompleteConsumer.cpp
clang/lib/Sema/SemaCodeComplete.cpp
clang/tools/clang-repl/ClangRepl.cpp
clang/tools/libclang/CIndexCodeCompletion.cpp
clang/unittests/Interpreter/CMakeLists.txt

Removed: 
clang/include/clang/Interpreter/CodeCompletion.h
clang/lib/Interpreter/CodeCompletion.cpp
clang/test/CodeCompletion/incrememal-mode-completion-no-error.cpp
clang/test/CodeCompletion/incremental-top-level.cpp
clang/unittests/Interpreter/CodeCompletionTest.cpp



diff  --git a/clang/include/clang/Frontend/ASTUnit.h 
b/clang/include/clang/Frontend/ASTUnit.h
index c6d0d4d7e90233..b762be1c9b1d6d 100644
--- a/clang/include/clang/Frontend/ASTUnit.h
+++ b/clang/include/clang/Frontend/ASTUnit.h
@@ -77,7 +77,6 @@ class Preprocessor;
 class PreprocessorOptions;
 class Sema;
 class TargetInfo;
-class SyntaxOnlyAction;
 
 /// \brief Enumerates the available scopes for skipping function bodies.
 enum class SkipFunctionBodiesScope { None, Preamble, PreambleAndMainFile };
@@ -888,10 +887,6 @@ class ASTUnit {
   /// \param IncludeBriefComments Whether to include brief documentation within
   /// the set of code completions returned.
   ///
-  /// \param Act If supplied, this argument is used to parse the input file,
-  /// allowing customized parsing by overriding SyntaxOnlyAction lifecycle
-  /// methods.
-  ///
   /// FIXME: The Diag, LangOpts, SourceMgr, FileMgr, StoredDiagnostics, and
   /// OwnedBuffers parameters are all disgusting hacks. They will go away.
   void CodeComplete(StringRef File, unsigned Line, unsigned Column,
@@ -902,8 +897,7 @@ class ASTUnit {
 DiagnosticsEngine &Diag, LangOptions &LangOpts,
 SourceManager &SourceMgr, FileManager &FileMgr,
 SmallVectorImpl &StoredDiagnostics,
-SmallVectorImpl &OwnedBuffers,
-std::unique_ptr Act = nullptr);
+SmallVectorImpl &OwnedBuffers);
 
   /// Save this translation unit to a file with the given name.
   ///

diff  --git a/clang/include/clang/Interpreter/CodeCompletion.h 
b/clang/include/clang/Interpreter/CodeCompletion.h
deleted file mode 100644
index 6631d33358600f..00
--- a/clang/include/clang/Interpreter/CodeCompletion.h
+++ /dev/null
@@ -1,33 +0,0 @@
-//===- CodeCompletion.h - Code Completion for ClangRepl ---===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-//
-// This file defines the classes which performs code completion at the REPL.
-//
-//===--===//
-
-#ifndef LLVM_CLANG_INTERPRETER_CODE_COMPLETION_H
-#define LLVM_CLANG_INTERPRETER_CODE_COMPLETION_H
-#include 
-#include 
-
-namespace llvm {
-class StringRef;
-} // namespace llvm
-
-namespace clang {
-class CodeCompletionResult;
-class CompilerInstance;
-
-void codeComplete(CompilerInstance *InterpCI, llvm::StringRef Content,
-  unsigned Line, unsigned Col, const CompilerInstance 
*ParentCI,
-  std::vector &CCResults);
-
-std::vector
-convertToCodeCompleteStrings(const std::vector &Results);
-} // namespace clang
-#endif

diff  --git a/clang/include/clang/Sema/CodeCompleteConsumer.h 
b/clang/include/clang/Sema/CodeCompleteConsumer.h
index c9e866676e884c..bb4b638050383c 100644
--- a/clang/include/clang/Sema/CodeCompleteConsumer.h
+++ b/clang/include/clang/Sema/CodeCompleteConsumer.h
@@ -336,10 +336,7 @@ class CodeCompletionContext {
 CCC_Recovery,
 
 /// Code completion in a @class forward declaration.
-CCC_ObjCClassForwardDecl,
-
-/// Code completion at a top level in a REPL session.
-CCC_TopLevelOrExpression,
+CCC_ObjCClassForwardDecl
   };
 
   u

[PATCH] D155826: [HIP][Clang][Preprocessor][RFC] Add preprocessor support for C++ Parallel Algorithm Offload

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:590
+  Builder.defineMacro("__HIPSTDPAR__");
+  if (!LangOpts.CUDAIsDevice)
+Builder.defineMacro("__HIPSTDPAR_INTERPOSE_ALLOC__");

We usually prefer defining the macro consistently in host and device 
compilation to avoid the chance of violation of ODR.


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

https://reviews.llvm.org/D155826

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

[clang] 6f9e53a - Revert "[clang-repl] Add Documentation for Execution Results Handling."

[Vassil Vassilev via cfe-commits]

Author: Vassil Vassilev
Date: 2023-08-23T14:46:57Z
New Revision: 6f9e53a4c3344ceedac29b179be1956e9489c684

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

LOG: Revert "[clang-repl] Add Documentation for Execution Results Handling."

This reverts commit 3edd338a6407d9410f6a283c5dc32ba676ac0b8f due to missing
graphviz https://lab.llvm.org/buildbot/#/builders/92/builds/49520

Added: 


Modified: 
clang/docs/ClangRepl.rst
clang/docs/conf.py

Removed: 




diff  --git a/clang/docs/ClangRepl.rst b/clang/docs/ClangRepl.rst
index e0050f1a3b56ee..bd99bc82f1 100644
--- a/clang/docs/ClangRepl.rst
+++ b/clang/docs/ClangRepl.rst
@@ -213,411 +213,6 @@ concept helps support advanced use cases such as template 
instantiations on dema
 automatic language interoperability. It also helps static languages such as 
C/C++ become
 apt for data science.
 
-Execution Results Handling in Clang-Repl
-
-
-Execution Results Handling features discussed below help extend the Clang-Repl
-functionality by creating an interface between the execution results of a
-program and the compiled program.
-
-1. **Capture Execution Results**: This feature helps capture the execution 
results
-of a program and bring them back to the compiled program.
-
-2. **Dump Captured Execution Results**: This feature helps create a temporary 
dump
-for Value Printing/Automatic Printf, that is, to display the value and type of
-the captured data.
-
-
-1. Capture Execution Results
-
-
-In many cases, it is useful to bring back the program execution result to the
-compiled program. This result can be stored in an object of type **Value**.
-
-How Execution Results are captured (Value Synthesis):
--
-
-The synthesizer chooses which expression to synthesize, and then it replaces
-the original expression with the synthesized expression. Depending on the
-expression type, it may choose to save an object (``LastValue``) of type 
'value'
-while allocating memory to it (``SetValueWithAlloc()``), or not (
-``SetValueNoAlloc()``).
-
-.. graphviz::
-:name: valuesynthesis
-:caption: Value Synthesis
-:alt: Shows how an object of type 'Value' is synthesized
-:align: center
-
- digraph "valuesynthesis" {
- rankdir="LR";
- graph [fontname="Verdana", fontsize="12"];
- node [fontname="Verdana", fontsize="12"];
- edge [fontname="Sans", fontsize="9"];
-
- start [label=" Create an Object \n 'Last Value' \n of type 'Value' ", 
shape="note", fontcolor=white, fillcolor="#ff", style=filled];
- assign [label=" Assign the result \n to the 'LastValue' \n (based on 
respective \n Memory Allocation \n scenario) ", shape="box"]
- print [label=" Pretty Print \n the Value Object ", shape="Msquare", 
fillcolor="yellow", style=filled];
- start -> assign;
- assign -> print;
-
-   subgraph SynthesizeExpression {
- synth [label=" SynthesizeExpr() ", shape="note", fontcolor=white, 
fillcolor="#ff", style=filled];
- mem [label=" New Memory \n Allocation? ", shape="diamond"];
- withaloc [label=" SetValueWithAlloc() ", shape="box"];
- noaloc [label=" SetValueNoAlloc() ", shape="box"];
- right [label=" 1. RValue Structure \n (a temporary value)", 
shape="box"];
- left2 [label=" 2. LValue Structure \n (a variable with \n an 
address)", shape="box"];
- left3 [label=" 3. Built-In Type \n (int, float, etc.)", 
shape="box"];
- output [label=" move to 'Assign' step ", shape="box"];
-
- synth -> mem;
- mem -> withaloc [label="Yes"];
- mem -> noaloc [label="No"];
- withaloc -> right;
- noaloc -> left2;
- noaloc -> left3;
- right -> output;
- left2 -> output;
- left3 -> output;
-  }
-output -> assign
-  }
-
-Where is the captured result stored?
-
-
-``LastValue`` holds the last result of the value printing. It is a class member
-because it can be accessed even after subsequent inputs.
-
-**Note:** If no value printing happens, then it is in an invalid state.
-
-Improving Efficiency and User Experience
-
-
-The Value object is essentially used to create a mapping between an expression
-'type' and the allocated 'memory'. Built-in types (bool, char, int,
-float, double, etc.) are copyable. Their their memory allocation size is known
-and the Value object can introduce a small-buffer optimization.
-In case of objects, the ``Value`` class provides reference-counted 

[PATCH] D157385: [clang][CFG] Cleanup functions

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

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

This is a great improvement. When I saw that clang now supports it and e.g. the 
CSA operates on the CFG, I also considered adding this.
Now I don't need to do it, so many thanks!

The code looks correct to me, except that the cleanup should be before the dtor 
if it has any. Other than that, the code coverage also looks great, thus I 
would not oppose.




Comment at: clang/lib/Analysis/CFG.cpp:1901-1902
   appendLifetimeEnds(Block, VD, S);
-if (BuildOpts.AddImplicitDtors)
+if (BuildOpts.AddImplicitDtors && !hasTrivialDestructor(VD))
   appendAutomaticObjDtor(Block, VD, S);
+if (HasCleanupAttr)

This condition looks new. Is it an orthogonal improvement?



Comment at: clang/lib/Analysis/CFG.cpp:1901-1904
+if (BuildOpts.AddImplicitDtors && !hasTrivialDestructor(VD))
   appendAutomaticObjDtor(Block, VD, S);
+if (HasCleanupAttr)
+  appendCleanupFunction(Block, VD);

steakhal wrote:
> This condition looks new. Is it an orthogonal improvement?
Shouldn't the cleanup function run first, and then the dtor of the variable?
This way the object is already "dead" even before reaching the cleanup handler.
https://godbolt.org/z/sT65boooW



Comment at: clang/lib/Analysis/CFG.cpp:2113
+
+bool CFGBuilder::hasTrivialDestructor(VarDecl *VD) const {
   // Check for const references bound to temporary. Set type to pointee.

Can't this accept `const VarDecl*` instead? That way we could get rid of that 
`const_cast` above.



Comment at: clang/test/Analysis/scopes-cfg-output.cpp:1473-1474
+// CHECK-NEXT:3: F f __attribute__((cleanup(cleanup_F)));
+// CHECK-NEXT:4: [B1.3].~F() (Implicit destructor)
+// CHECK-NEXT:5: CleanupFunction (cleanup_F)
+// CHECK-NEXT:6: CFGScopeEnd(f)

tbaeder wrote:
> aaronpuchert wrote:
> > Interesting test! But it seems CodeGen has them swapped: compiling this 
> > snippet with `clang -c -S -emit-llvm` I get
> > ```lang=LLVM
> > define dso_local void @_Z4testv() #0 personality ptr @__gxx_personality_v0 {
> >   %1 = alloca %class.F, align 1
> >   %2 = alloca ptr, align 8
> >   %3 = alloca i32, align 4
> >   invoke void @_Z9cleanup_FP1F(ptr noundef %1)
> >   to label %4 unwind label %5
> > 
> > 4:; preds = %0
> >   call void @_ZN1FD2Ev(ptr noundef nonnull align 1 dereferenceable(1) %1) #3
> >   ret void
> > 
> > ; ...
> > }
> > ```
> > So first cleanup, then destructor. This is with 17.0.0-rc2.
> Interesting, I thought I checked this and used the correct order. Will 
> re-check, thanks.
I believe this demonstrates the wrong order I also spotted earlier.



Comment at: clang/test/Analysis/scopes-cfg-output.cpp:1480
+public:
+  ~F() {}
+};

aaronpuchert wrote:
> tbaeder wrote:
> > aaronpuchert wrote:
> > > As with the cleanup function, a definition shouldn't be necessary.
> > Is there a way to test whether the contents of the cleanup function are 
> > being checked as well? From these tests, I only know we consider them 
> > called, but not whether we (properly) analyze their bodies in the context 
> > as well. Or is that separate from this patch?
> For now we're just adding a new element to the CFG and adapting the 
> respective tests. The CFG is generated on a per-function basis, and the 
> generation of one function's CFG will never look into another function's 
> body. It might use some (declaration) properties of course, like whether it 
> has `[[noreturn]]` or `noexcept`. Of course we should also generate a CFG for 
> the cleanup function, but that's independent of this change.
> 
> Users of the CFG will naturally need to be taught about this new element type 
> to “understand” it. Otherwise they should simply skip it. Since the CFG 
> previously did not contain such elements, there should be no change for them. 
> So we can also initially just add the element and not tell anybody about it.
> 
> We could also add understanding of the new element type to other CFG users, 
> but you don't have to do this. If you only care about Thread Safety Analysis, 
> then it's totally fine to handle it only there.
> 
> But let's move all changes to Thread Safety Analysis into a follow-up, so 
> that we don't have to bother the CFG maintainers with that.
Speaking of `noreturn`, I think it is worth demonstrating that if the cleanup 
function has `noreturn` attribute, then the dtor is not called.


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

https://reviews.llvm.org/D157385

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

[PATCH] D158624: [RISCV] Implement multiVersionSortPriority

[Piyou Chen via Phabricator via cfe-commits]

BeMg created this revision.
Herald added subscribers: jobnoorman, luke, sunshaoce, VincentWu, vkmr, 
frasercrmck, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, 
psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, 
edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, 
simoncook, johnrusso, rbar, asb, arichardson.
Herald added a project: All.
BeMg requested review of this revision.
Herald added subscribers: cfe-commits, wangpc, eopXD, MaskRay.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158624

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h


Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -114,6 +114,7 @@
   void fillValidCPUList(SmallVectorImpl &Values) const override;
   bool isValidTuneCPUName(StringRef Name) const override;
   void fillValidTuneCPUList(SmallVectorImpl &Values) const override;
+  unsigned multiVersionSortPriority(StringRef Name) const override;
 };
 class LLVM_LIBRARY_VISIBILITY RISCV32TargetInfo : public RISCVTargetInfo {
 public:
Index: clang/lib/Basic/Targets/RISCV.cpp
===
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -346,3 +346,6 @@
   bool Is64Bit = getTriple().isArch64Bit();
   llvm::RISCV::fillValidTuneCPUArchList(Values, Is64Bit);
 }
+unsigned RISCVTargetInfo::multiVersionSortPriority(StringRef Name) const {
+  return llvm::RISCVISAInfo::getExtensionSerial(Name);
+}


Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -114,6 +114,7 @@
   void fillValidCPUList(SmallVectorImpl &Values) const override;
   bool isValidTuneCPUName(StringRef Name) const override;
   void fillValidTuneCPUList(SmallVectorImpl &Values) const override;
+  unsigned multiVersionSortPriority(StringRef Name) const override;
 };
 class LLVM_LIBRARY_VISIBILITY RISCV32TargetInfo : public RISCVTargetInfo {
 public:
Index: clang/lib/Basic/Targets/RISCV.cpp
===
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -346,3 +346,6 @@
   bool Is64Bit = getTriple().isArch64Bit();
   llvm::RISCV::fillValidTuneCPUArchList(Values, Is64Bit);
 }
+unsigned RISCVTargetInfo::multiVersionSortPriority(StringRef Name) const {
+  return llvm::RISCVISAInfo::getExtensionSerial(Name);
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158522: [NFC][CLANG] Fix static analyzer bugs about large copy by values

[Soumi Manna via Phabricator via cfe-commits]

Manna added a comment.

Failure is unrelated to my patch.


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

https://reviews.llvm.org/D158522

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

[clang] 30c60ec - [NFC][CLANG] Fix static analyzer bugs about large copy by values

[via cfe-commits]

Author: Manna, Soumi
Date: 2023-08-23T07:57:04-07:00
New Revision: 30c60ec52f1551c07a3dbab0316ddccf356c6663

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

LOG: [NFC][CLANG] Fix static analyzer bugs about large copy by values

Static Analyzer Tool complains about a large function call parameter which is 
is passed by value in CGBuiltin.cpp file.

1. In CodeGenFunction::EmitSMELdrStr(clang::SVETypeFlags, 
llvm::SmallVectorImpl &, unsigned int): We are passing parameter 
TypeFlags of type clang::SVETypeFlags by value.

2. In CodeGenFunction::EmitSMEZero(clang::SVETypeFlags, 
llvm::SmallVectorImpl &, unsigned int): We are passing parameter 
TypeFlags of type clang::SVETypeFlags by value.

3. In CodeGenFunction::EmitSMEReadWrite(clang::SVETypeFlags, 
llvm::SmallVectorImpl &, unsigned int): We are passing parameter 
TypeFlags of type clang::SVETypeFlags by value.

4. In CodeGenFunction::EmitSMELd1St1(clang::SVETypeFlags, 
llvm::SmallVectorImpl &, unsigned int): We are passing parameter 
TypeFlags of type clang::SVETypeFlags by value.

I see many places in CGBuiltin.cpp file, we are passing parameter TypeFlags of 
type clang::SVETypeFlags by reference.

clang::SVETypeFlags inherits several other types.

This patch passes parameter TypeFlags by reference instead of by value in the 
function.

Reviewed By: tahonermann, sdesmalen

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

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/CodeGenFunction.h

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index d35c4766243caa..a99cf3d82aaed5 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -9515,7 +9515,7 @@ Value *CodeGenFunction::EmitTileslice(Value *Offset, 
Value *Base) {
   return Builder.CreateAdd(Base, CastOffset, "tileslice");
 }
 
-Value *CodeGenFunction::EmitSMELd1St1(SVETypeFlags TypeFlags,
+Value *CodeGenFunction::EmitSMELd1St1(const SVETypeFlags &TypeFlags,
   SmallVectorImpl &Ops,
   unsigned IntID) {
   Ops[3] = EmitSVEPredicateCast(
@@ -9545,7 +9545,7 @@ Value *CodeGenFunction::EmitSMELd1St1(SVETypeFlags 
TypeFlags,
   return Builder.CreateCall(F, NewOps);
 }
 
-Value *CodeGenFunction::EmitSMEReadWrite(SVETypeFlags TypeFlags,
+Value *CodeGenFunction::EmitSMEReadWrite(const SVETypeFlags &TypeFlags,
  SmallVectorImpl &Ops,
  unsigned IntID) {
   auto *VecTy = getSVEType(TypeFlags);
@@ -9562,7 +9562,7 @@ Value *CodeGenFunction::EmitSMEReadWrite(SVETypeFlags 
TypeFlags,
   return Builder.CreateCall(F, Ops);
 }
 
-Value *CodeGenFunction::EmitSMEZero(SVETypeFlags TypeFlags,
+Value *CodeGenFunction::EmitSMEZero(const SVETypeFlags &TypeFlags,
 SmallVectorImpl &Ops,
 unsigned IntID) {
   // svzero_za() intrinsic zeros the entire za tile and has no paramters.
@@ -9572,7 +9572,7 @@ Value *CodeGenFunction::EmitSMEZero(SVETypeFlags 
TypeFlags,
   return Builder.CreateCall(F, Ops);
 }
 
-Value *CodeGenFunction::EmitSMELdrStr(SVETypeFlags TypeFlags,
+Value *CodeGenFunction::EmitSMELdrStr(const SVETypeFlags &TypeFlags,
   SmallVectorImpl &Ops,
   unsigned IntID) {
   Function *Cntsb = CGM.getIntrinsic(Intrinsic::aarch64_sme_cntsb);

diff  --git a/clang/lib/CodeGen/CodeGenFunction.h 
b/clang/lib/CodeGen/CodeGenFunction.h
index d7e759824c2819..af9ab55cbabd92 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -4292,16 +4292,16 @@ class CodeGenFunction : public CodeGenTypeCache {
   unsigned IntID);
   llvm::Value *EmitAArch64SVEBuiltinExpr(unsigned BuiltinID, const CallExpr 
*E);
 
-  llvm::Value *EmitSMELd1St1(SVETypeFlags TypeFlags,
+  llvm::Value *EmitSMELd1St1(const SVETypeFlags &TypeFlags,
  llvm::SmallVectorImpl &Ops,
  unsigned IntID);
-  llvm::Value *EmitSMEReadWrite(SVETypeFlags TypeFlags,
+  llvm::Value *EmitSMEReadWrite(const SVETypeFlags &TypeFlags,
 llvm::SmallVectorImpl &Ops,
 unsigned IntID);
-  llvm::Value *EmitSMEZero(SVETypeFlags TypeFlags,
+  llvm::Value *EmitSMEZero(const SVETypeFlags &TypeFlags,
llvm::SmallVectorImpl &Ops,
unsigned IntID);
-  llvm::Value *EmitSMELdrStr(SVETypeFlags TypeFlags,
+  llvm::Value *EmitSMELdrStr(const SVETypeFlags &TypeFlags,
  llvm::SmallVectorImpl &Ops,
   

[PATCH] D158522: [NFC][CLANG] Fix static analyzer bugs about large copy by values

[Soumi Manna via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG30c60ec52f15: [NFC][CLANG] Fix static analyzer bugs about 
large copy by values (authored by Manna).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158522

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.h


Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -4292,16 +4292,16 @@
   unsigned IntID);
   llvm::Value *EmitAArch64SVEBuiltinExpr(unsigned BuiltinID, const CallExpr 
*E);
 
-  llvm::Value *EmitSMELd1St1(SVETypeFlags TypeFlags,
+  llvm::Value *EmitSMELd1St1(const SVETypeFlags &TypeFlags,
  llvm::SmallVectorImpl &Ops,
  unsigned IntID);
-  llvm::Value *EmitSMEReadWrite(SVETypeFlags TypeFlags,
+  llvm::Value *EmitSMEReadWrite(const SVETypeFlags &TypeFlags,
 llvm::SmallVectorImpl &Ops,
 unsigned IntID);
-  llvm::Value *EmitSMEZero(SVETypeFlags TypeFlags,
+  llvm::Value *EmitSMEZero(const SVETypeFlags &TypeFlags,
llvm::SmallVectorImpl &Ops,
unsigned IntID);
-  llvm::Value *EmitSMELdrStr(SVETypeFlags TypeFlags,
+  llvm::Value *EmitSMELdrStr(const SVETypeFlags &TypeFlags,
  llvm::SmallVectorImpl &Ops,
  unsigned IntID);
   llvm::Value *EmitAArch64SMEBuiltinExpr(unsigned BuiltinID, const CallExpr 
*E);
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -9515,7 +9515,7 @@
   return Builder.CreateAdd(Base, CastOffset, "tileslice");
 }
 
-Value *CodeGenFunction::EmitSMELd1St1(SVETypeFlags TypeFlags,
+Value *CodeGenFunction::EmitSMELd1St1(const SVETypeFlags &TypeFlags,
   SmallVectorImpl &Ops,
   unsigned IntID) {
   Ops[3] = EmitSVEPredicateCast(
@@ -9545,7 +9545,7 @@
   return Builder.CreateCall(F, NewOps);
 }
 
-Value *CodeGenFunction::EmitSMEReadWrite(SVETypeFlags TypeFlags,
+Value *CodeGenFunction::EmitSMEReadWrite(const SVETypeFlags &TypeFlags,
  SmallVectorImpl &Ops,
  unsigned IntID) {
   auto *VecTy = getSVEType(TypeFlags);
@@ -9562,7 +9562,7 @@
   return Builder.CreateCall(F, Ops);
 }
 
-Value *CodeGenFunction::EmitSMEZero(SVETypeFlags TypeFlags,
+Value *CodeGenFunction::EmitSMEZero(const SVETypeFlags &TypeFlags,
 SmallVectorImpl &Ops,
 unsigned IntID) {
   // svzero_za() intrinsic zeros the entire za tile and has no paramters.
@@ -9572,7 +9572,7 @@
   return Builder.CreateCall(F, Ops);
 }
 
-Value *CodeGenFunction::EmitSMELdrStr(SVETypeFlags TypeFlags,
+Value *CodeGenFunction::EmitSMELdrStr(const SVETypeFlags &TypeFlags,
   SmallVectorImpl &Ops,
   unsigned IntID) {
   Function *Cntsb = CGM.getIntrinsic(Intrinsic::aarch64_sme_cntsb);


Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -4292,16 +4292,16 @@
   unsigned IntID);
   llvm::Value *EmitAArch64SVEBuiltinExpr(unsigned BuiltinID, const CallExpr *E);
 
-  llvm::Value *EmitSMELd1St1(SVETypeFlags TypeFlags,
+  llvm::Value *EmitSMELd1St1(const SVETypeFlags &TypeFlags,
  llvm::SmallVectorImpl &Ops,
  unsigned IntID);
-  llvm::Value *EmitSMEReadWrite(SVETypeFlags TypeFlags,
+  llvm::Value *EmitSMEReadWrite(const SVETypeFlags &TypeFlags,
 llvm::SmallVectorImpl &Ops,
 unsigned IntID);
-  llvm::Value *EmitSMEZero(SVETypeFlags TypeFlags,
+  llvm::Value *EmitSMEZero(const SVETypeFlags &TypeFlags,
llvm::SmallVectorImpl &Ops,
unsigned IntID);
-  llvm::Value *EmitSMELdrStr(SVETypeFlags TypeFlags,
+  llvm::Value *EmitSMELdrStr(const SVETypeFlags &TypeFlags,
  llvm::SmallVectorImpl &Ops,
  unsigned IntID);
   llvm::Value *EmitAArch64SMEBuiltinExpr(unsigned BuiltinID, const CallExpr *E);
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -9515,7 +9515,7 @

[PATCH] D158612: [flang][driver] Ensure negative flags have the same visibility as positive

[Andrzej Warzynski via Phabricator via cfe-commits]

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

LGTM, ta!




Comment at: clang/include/clang/Driver/Options.td:5314
   NegFlag,
-  PosFlag,
+  PosFlag,
   BothFlags<[], [ClangOption], " the integrated assembler">>;

Could you add a test? 
https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/fno-integrated-as.f90

I would probably rename that file as `integrated-as.f90` and test 3 variants:
```
! RUN: 
! RUN: -fintegrated-as
! RUN: -fno-integrated-as
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158612

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

[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added a comment.

I think this is also true of `-fsanitize=kcfi`, isn't it? That also works by 
writing a cookie just before the function entry point. If we're diagnosing 
incompatibilities with execute-only, then perhaps we should do it for both.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158614

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

[PATCH] D155935: [RISCV] Enable multiversion function

[Piyou Chen via Phabricator via cfe-commits]

BeMg updated this revision to Diff 552721.
BeMg added a comment.
Herald added a subscriber: sunshaoce.

Rebase and update testcase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155935

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/RISCV/riscv-fmv.c

Index: clang/test/CodeGen/RISCV/riscv-fmv.c
===
--- /dev/null
+++ clang/test/CodeGen/RISCV/riscv-fmv.c
@@ -0,0 +1,106 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --include-generated-funcs --version 2
+// REQUIRES: riscv-registered-target
+// RUN: %clang_cc1 -triple riscv64 -target-feature +zifencei \
+// RUN:  -target-feature +m -target-feature +a   \
+// RUN:  -target-feature +f -target-feature +d   \
+// RUN:  -emit-llvm %s -o - | FileCheck %s   \
+// RUN:  --check-prefix=CHECK-IR
+
+__attribute__((target("arch=+v"))) void test1(void) {}
+__attribute__((target("arch=+v,+zbb"))) void test1(void) {}
+__attribute__((target("arch=+v,+zbb,+zicond1p0"))) void test1(void) {}
+__attribute__((target("arch=rv64gc"))) void test1(void) {}
+__attribute__((target("default"))) void test1(void) {}
+
+__attribute__((target("default"))) void test2(void) {}
+
+void test3() { test1(); test2(); }
+
+// CHECK-IR: $test1.resolver = comdat any
+// CHECK-IR: $test2.resolver = comdat any
+
+// CHECK-IR: @0 = private unnamed_addr constant [2 x i8] c"v\00", align 1
+// CHECK-IR: @1 = private unnamed_addr constant [6 x i8] c"v;zbb\00", align 1
+// CHECK-IR: @2 = private unnamed_addr constant [26 x i8] c"v;zbb;experimental-zicond\00", align 1
+// CHECK-IR: @3 = private unnamed_addr constant [25 x i8] c"m;a;f;d;c;zicsr;zifencei\00", align 1
+
+
+// CHECK-IR-LABEL: define dso_local void @test1.v
+// CHECK-IR-SAME: () #[[ATTR0:[0-9]+]] {
+// CHECK-IR-NEXT:  entry:
+// CHECK-IR-NEXT:ret void
+//
+//
+// CHECK-IR-LABEL: define dso_local void @test1.zbb_v
+// CHECK-IR-SAME: () #[[ATTR1:[0-9]+]] {
+// CHECK-IR-NEXT:  entry:
+// CHECK-IR-NEXT:ret void
+//
+//
+// CHECK-IR-LABEL: define dso_local void @test1.experimental-zicond_zbb_v
+// CHECK-IR-SAME: () #[[ATTR2:[0-9]+]] {
+// CHECK-IR-NEXT:  entry:
+// CHECK-IR-NEXT:ret void
+//
+//
+// CHECK-IR-LABEL: define dso_local void @test1.zifencei_zicsr_m_f_d_c_a
+// CHECK-IR-SAME: () #[[ATTR3:[0-9]+]] {
+// CHECK-IR-NEXT:  entry:
+// CHECK-IR-NEXT:ret void
+//
+//
+// CHECK-IR-LABEL: define dso_local void @test1
+// CHECK-IR-SAME: () #[[ATTR4:[0-9]+]] {
+// CHECK-IR-NEXT:  entry:
+// CHECK-IR-NEXT:ret void
+//
+//
+// CHECK-IR-LABEL: define dso_local void @test2
+// CHECK-IR-SAME: () #[[ATTR4]] {
+// CHECK-IR-NEXT:  entry:
+// CHECK-IR-NEXT:ret void
+//
+//
+// CHECK-IR-LABEL: define dso_local void @test3
+// CHECK-IR-SAME: () #[[ATTR4]] {
+// CHECK-IR-NEXT:  entry:
+// CHECK-IR-NEXT:call void @test1.resolver()
+// CHECK-IR-NEXT:call void @test2.resolver()
+// CHECK-IR-NEXT:ret void
+//
+//
+// CHECK-IR-LABEL: define weak_odr void @test1.resolver() comdat {
+// CHECK-IR-NEXT:  resolver_entry:
+// CHECK-IR-NEXT:[[TMP0:%.*]] = call i1 @__riscv_ifunc_select(ptr @[[GLOB0:[0-9]+]])
+// CHECK-IR-NEXT:br i1 [[TMP0]], label [[RESOLVER_RETURN:%.*]], label [[RESOLVER_ELSE:%.*]]
+// CHECK-IR:   resolver_return:
+// CHECK-IR-NEXT:musttail call void @test1.v()
+// CHECK-IR-NEXT:ret void
+// CHECK-IR:   resolver_else:
+// CHECK-IR-NEXT:[[TMP1:%.*]] = call i1 @__riscv_ifunc_select(ptr @[[GLOB1:[0-9]+]])
+// CHECK-IR-NEXT:br i1 [[TMP1]], label [[RESOLVER_RETURN1:%.*]], label [[RESOLVER_ELSE2:%.*]]
+// CHECK-IR:   resolver_return1:
+// CHECK-IR-NEXT:musttail call void @test1.zbb_v()
+// CHECK-IR-NEXT:ret void
+// CHECK-IR:   resolver_else2:
+// CHECK-IR-NEXT:[[TMP2:%.*]] = call i1 @__riscv_ifunc_select(ptr @[[GLOB2:[0-9]+]])
+// CHECK-IR-NEXT:br i1 [[TMP2]], label [[RESOLVER_RETURN3:%.*]], label [[RESOLVER_ELSE4:%.*]]
+// CHECK-IR:   resolver_return3:
+// CHECK-IR-NEXT:musttail call void @test1.experimental-zicond_zbb_v()
+// CHECK-IR-NEXT:ret void
+// CHECK-IR:   resolver_else4:
+// CHECK-IR-NEXT:[[TMP3:%.*]] = call i1 @__riscv_ifunc_select(ptr @[[GLOB3:[0-9]+]])
+// CHECK-IR-NEXT:br i1 [[TMP3]], label [[RESOLVER_RETURN5:%.*]], label [[RESOLVER_ELSE6:%.*]]
+// CHECK-IR:   resolver_return5:
+// CHECK-IR-NEXT:musttail call void @test1.zifencei_zicsr_m_f_d_c_a()
+// CHECK-IR-NEXT:ret void
+// CHECK-IR:   resolver_else6:
+// CHECK-IR-NEXT:musttail call void @test1()
+// CHECK-IR-NEXT:ret void
+//
+//
+// CHECK-IR-LABEL: define weak_odr void @test2.resolver() comdat {
+// CHECK-IR-NEXT:  resolver_entry:
+// CHECK-IR-NEXT:musttail call void @test2()
+// CHECK-

[PATCH] D158624: [RISCV] Implement multiVersionSortPriority

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

I don't think you can just use any arbitrary function?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158624

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

[PATCH] D158614: [UBSan] Disable the function sanitizer on an execute-only target.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham added inline comments.



Comment at: clang/lib/Basic/Sanitizers.cpp:134-143
+  if ((A &&
+   A->getOption().matches(clang::driver::options::OPT_mexecute_only)) ||
+  (std::find(Features.begin(), Features.end(), "+execute-only") !=
+   Features.end())) {
+// The execute-only output is supported only on ARMv6T2 and ARMv7 and 
above.
+if (llvm::ARM::parseArchVersion(Triple.getArchName()) > 7 ||
+llvm::ARM::parseArch(Triple.getArchName()) ==

Why do we need to check //both// of `-mexecute-only` and the `+execute-only` 
target feature? It looks to me as if the code in 
`Driver/ToolChains/Arch/ARM.cpp` that handles `-mexecute-only` ends up setting 
the same target feature anyway. And it checks the supported architecture 
versions first.

Would it not be better to //just// check the target feature here, and avoid 
having a duplicated copy of the rest of this logic which needs to be kept in 
sync with the ARM driver?

Does something go wrong if you do it that way?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158614

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

[PATCH] D155769: [HIP][Clang][docs][RFC] Add documentation for C++ Parallel Algorithm Offload

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/docs/HIPSupport.rst:232
+execution seamlessly falls back to the host CPU. It is legal to specify 
multiple
+``--offload-arcj``s. All the flags we introduce, as well as a thorough view of
+various restrictions and their implications will be provided below.

typo j -> h



Comment at: clang/docs/HIPSupport.rst:549
+   Standard.
+
+Open Questions / Future Developments

can we add an item explaining whether relocatable object files compiled from 
`--hipstdpar` and HIP be linked together? 


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

https://reviews.llvm.org/D155769

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

[PATCH] D158612: [flang][driver] Ensure negative flags have the same visibility as positive

[Tom Eccles via Phabricator via cfe-commits]

tblah updated this revision to Diff 552723.
tblah added a comment.

Thanks for taking a look.

Changes: add test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158612

Files:
  clang/include/clang/Driver/Options.td
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/fintegrated-as.f90
  flang/test/Driver/fno-integrated-as.f90

Index: flang/test/Driver/fintegrated-as.f90
===
--- flang/test/Driver/fintegrated-as.f90
+++ flang/test/Driver/fintegrated-as.f90
@@ -1,4 +1,4 @@
-! Tests for the `-fno-integrated-as` flag.
+! Tests for the `-f(no-)integrated-as` flag.
 
 ! UNSUPPORTED: system-windows
 
@@ -11,10 +11,11 @@
 ! CHECK-SAME: "-o" "[[assembly_file:.*]].s"
 ! CHECK-NEXT: "-o" "{{.*}}.o" "[[assembly_file:.*]].s"
 
-!-
-! Without `-fno-integrated-as`
-!-
+!
+! Without `-fno-integrated-as` (default) / With `-fintegrated-as`
+!
 ! Verify that there _is no_ separate line with an assembler invocation
+! RUN: %flang -c -fintegrated-as %s -### 2>&1 | FileCheck %s -check-prefix=DEFAULT
 ! RUN: %flang -c %s -### 2>&1 | FileCheck %s -check-prefix=DEFAULT
 ! DEFAULT-LABEL: "-fc1"
-! DEFAULT-SAME: "-o" "{{.*}}.o" "{{.*}}fno-integrated-as.f90"
+! DEFAULT-SAME: "-o" "{{.*}}.o" "{{.*}}fintegrated-as.f90"
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -39,6 +39,7 @@
 ! HELP-NEXT: -fhonor-nansSpecify that floating-point optimizations are not allowed that assume arguments and results are not NANs.
 ! HELP-NEXT: -fimplicit-none No implicit typing allowed unless overridden by IMPLICIT statements
 ! HELP-NEXT: -finput-charset= Specify the default character set for source files
+! HELP-NEXT: -fintegrated-as Enable the integrated assembler
 ! HELP-NEXT: -fintrinsic-modules-path 
 ! HELP-NEXT: Specify where to find the compiled intrinsic modules
 ! HELP-NEXT: -flarge-sizes   Use INTEGER(KIND=8) for the result type in size-related intrinsics
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -43,6 +43,7 @@
 ! CHECK-NEXT: -fhonor-nansSpecify that floating-point optimizations are not allowed that assume arguments and results are not NANs.
 ! CHECK-NEXT: -fimplicit-none No implicit typing allowed unless overridden by IMPLICIT statements
 ! CHECK-NEXT: -finput-charset= Specify the default character set for source files
+! CHECK-NEXT: -fintegrated-as Enable the integrated assembler
 ! CHECK-NEXT: -fintrinsic-modules-path 
 ! CHECK-NEXT: Specify where to find the compiled intrinsic modules
 ! CHECK-NEXT: -flang-experimental-hlfir
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2366,13 +2366,13 @@
   PosFlag,
-  NegFlag>;
+  NegFlag>;
 defm approx_func : BoolFOption<"approx-func", LangOpts<"ApproxFunc">, DefaultFalse,
PosFlag,
-   NegFlag>;
+   NegFlag>;
 defm finite_math_only : BoolFOption<"finite-math-only",
   LangOpts<"FiniteMathOnly">, DefaultFalse,
   PosFlag,
-  PosFlag>;
+  PosFlag>;
 def fhonor_nans : Flag<["-"], "fhonor-nans">, Group,
   Visibility<[ClangOption, FlangOption]>,
   HelpText<"Specify that floating-point optimizations are not allowed that "
@@ -3383,12 +3383,12 @@
   LangOpts<"ROPI">, DefaultFalse,
   PosFlag,
-  NegFlag>;
+  NegFlag>;
 defm rwpi : BoolFOption<"rwpi",
   LangOpts<"RWPI">, DefaultFalse,
   PosFlag,
-  NegFlag>;
+  NegFlag>;
 def fplugin_EQ : Joined<["-"], "fplugin=">, Group,
   Flags<[NoXarchOption]>, MetaVarName<"">,
   HelpText<"Load the named plugin (dynamic shared object)">;
@@ -5311,7 +5311,7 @@
 defm integrated_as : BoolFOption<"integrated-as",
   CodeGenOpts<"DisableIntegratedAS">, DefaultFalse,
   NegFlag,
-  PosFlag,
+  PosFlag,
   BothFlags<[], [ClangOption], " the integrated assembler">>;
 
 def fintegrated_cc1 : Flag<["-"], "fintegrated-cc1">,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158227: [clang] EditedSource::applyRewrites - useless call

[Douglas Yung via Phabricator via cfe-commits]

dyung added a comment.

In D158227#4610097 , @Manna wrote:

> The failure seems unrelated to my change.

Hi @Manna, I'm not sure which failure you are referring to, but there is a test 
failure on at least 2 build bots that seems to have been caused by your change. 
Can you take a look and revert if you need time to investigate?

https://lab.llvm.org/buildbot/#/builders/139/builds/48141
https://lab.llvm.org/buildbot/#/builders/216/builds/26116


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158227

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

[PATCH] D158626: [AArch64] Add missing vrnd intrinsics

[Max Iyengar via Phabricator via cfe-commits]

miyengar created this revision.
miyengar added a reviewer: vhscampos.
Herald added subscribers: hiraditya, kristof.beyls.
Herald added a project: All.
miyengar requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This patch adds 8 missing intrinsics as specified in the Arm ACLE document 
section 2.12.1.1 : 
https://arm-software.github.io/acle/neon_intrinsics/advsimd.html#rounding-3 


The intrinsics implemented are:

- vrnd32z_f64
- vrnd32zq_f64
- vrnd64z_f64
- vrnd64zq_f64
- vrnd32x_f64
- vrnd32xq_f64
- vrnd64x_f64
- vrnd64xq_f64


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158626

Files:
  clang/include/clang/Basic/arm_neon.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/aarch64-v8.5a-neon-frint3264-intrinsic.c
  llvm/lib/Target/AArch64/AArch64InstrFormats.td
  llvm/test/CodeGen/AArch64/v8.5a-neon-frint3264-intrinsic.ll

Index: llvm/test/CodeGen/AArch64/v8.5a-neon-frint3264-intrinsic.ll
===
--- llvm/test/CodeGen/AArch64/v8.5a-neon-frint3264-intrinsic.ll
+++ llvm/test/CodeGen/AArch64/v8.5a-neon-frint3264-intrinsic.ll
@@ -81,3 +81,85 @@
   %val = tail call <4 x float> @llvm.aarch64.neon.frint64z.v4f32(<4 x float> %a)
   ret <4 x float> %val
 }
+
+declare <1 x double> @llvm.aarch64.neon.frint32x.v1f64(<1 x double>)
+declare <2 x double> @llvm.aarch64.neon.frint32x.v2f64(<2 x double>)
+declare <1 x double> @llvm.aarch64.neon.frint32z.v1f64(<1 x double>)
+declare <2 x double> @llvm.aarch64.neon.frint32z.v2f64(<2 x double>)
+
+define dso_local <1 x double> @t_vrnd32x_f64(<1 x double> %a) {
+; CHECK-LABEL: t_vrnd32x_f64:
+; CHECK: frint32x d0, d0
+; CHECK-NEXT:ret
+entry:
+  %val = tail call <1 x double> @llvm.aarch64.neon.frint32x.v1f64(<1 x double> %a)
+  ret <1 x double> %val
+}
+
+define dso_local <2 x double> @t_vrnd32xq_f64(<2 x double> %a) {
+; CHECK-LABEL: t_vrnd32xq_f64:
+; CHECK: frint32x v0.2d, v0.2d
+; CHECK-NEXT:ret
+entry:
+  %val = tail call <2 x double> @llvm.aarch64.neon.frint32x.v2f64(<2 x double> %a)
+  ret <2 x double> %val
+}
+
+define dso_local <1 x double> @t_vrnd32z_f64(<1 x double> %a) {
+; CHECK-LABEL: t_vrnd32z_f64:
+; CHECK: frint32z d0, d0
+; CHECK-NEXT:ret
+entry:
+  %val = tail call <1 x double> @llvm.aarch64.neon.frint32z.v1f64(<1 x double> %a)
+  ret <1 x double> %val
+}
+
+define dso_local <2 x double> @t_vrnd32zq_f64(<2 x double> %a) {
+; CHECK-LABEL: t_vrnd32zq_f64:
+; CHECK: frint32z v0.2d, v0.2d
+; CHECK-NEXT:ret
+entry:
+  %val = tail call <2 x double> @llvm.aarch64.neon.frint32z.v2f64(<2 x double> %a)
+  ret <2 x double> %val
+}
+
+declare <1 x double> @llvm.aarch64.neon.frint64x.v1f64(<1 x double>)
+declare <2 x double> @llvm.aarch64.neon.frint64x.v2f64(<2 x double>)
+declare <1 x double> @llvm.aarch64.neon.frint64z.v1f64(<1 x double>)
+declare <2 x double> @llvm.aarch64.neon.frint64z.v2f64(<2 x double>)
+
+define dso_local <1 x double> @t_vrnd64x_f64(<1 x double> %a) {
+; CHECK-LABEL: t_vrnd64x_f64:
+; CHECK: frint64x d0, d0
+; CHECK-NEXT:ret
+entry:
+  %val = tail call <1 x double> @llvm.aarch64.neon.frint64x.v1f64(<1 x double> %a)
+  ret <1 x double> %val
+}
+
+define dso_local <2 x double> @t_vrnd64xq_f64(<2 x double> %a) {
+; CHECK-LABEL: t_vrnd64xq_f64:
+; CHECK: frint64x v0.2d, v0.2d
+; CHECK-NEXT:ret
+entry:
+  %val = tail call <2 x double> @llvm.aarch64.neon.frint64x.v2f64(<2 x double> %a)
+  ret <2 x double> %val
+}
+
+define dso_local <1 x double> @t_vrnd64z_f64(<1 x double> %a) {
+; CHECK-LABEL: t_vrnd64z_f64:
+; CHECK: frint64z d0, d0
+; CHECK-NEXT:ret
+entry:
+  %val = tail call <1 x double> @llvm.aarch64.neon.frint64z.v1f64(<1 x double> %a)
+  ret <1 x double> %val
+}
+
+define dso_local <2 x double> @t_vrnd64zq_f64(<2 x double> %a) {
+; CHECK-LABEL: t_vrnd64zq_f64:
+; CHECK: frint64z v0.2d, v0.2d
+; CHECK-NEXT:ret
+entry:
+  %val = tail call <2 x double> @llvm.aarch64.neon.frint64z.v2f64(<2 x double> %a)
+  ret <2 x double> %val
+}
Index: llvm/lib/Target/AArch64/AArch64InstrFormats.td
===
--- llvm/lib/Target/AArch64/AArch64InstrFormats.td
+++ llvm/lib/Target/AArch64/AArch64InstrFormats.td
@@ -6282,24 +6282,30 @@
 : SIMDTwoVectorFP;
 
 // Supports only S and D element sizes
-let mayRaiseFPException = 1, Uses = [FPCR] in
-multiclass SIMDTwoVectorSD opc, string asm,
+multiclass SIMDTwoVectorSD {
-
-  def v2f32 : BaseSIMDTwoSameVector<0, U, 00, opc, 0b00, V64,
+  let mayRaiseFPException = 1, Uses = [FPCR] in {
+def v2f32 : BaseSIMDTwoSameVector<0, U, 00, {0b, opc}, 0b00, V64,
 asm, ".2s", ".2s",
   [(set (v2f32 V64:$Rd), (OpNode (v2f32 V64:$Rn)))]>;
-  def v4f32 : BaseSIMDTwoSameVector<1, U, 00,

[PATCH] D152054: [OpenMP] Codegen support for thread_limit on target directive

[Sandeep via Phabricator via cfe-commits]

sandeepkosuri updated this revision to Diff 552725.
sandeepkosuri added a comment.

Added PCH options to the RUN lines in LIT tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152054

Files:
  clang/include/clang/Basic/OpenMPKinds.h
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_codegen.cpp
  clang/test/OpenMP/target_parallel_for_simd_tl_codegen.cpp
  clang/test/OpenMP/target_parallel_for_tl_codegen.cpp
  clang/test/OpenMP/target_parallel_generic_loop_tl_codegen.cpp
  clang/test/OpenMP/target_parallel_tl_codegen.cpp
  clang/test/OpenMP/target_simd_tl_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  openmp/runtime/src/kmp.h
  openmp/runtime/src/kmp_csupport.cpp
  openmp/runtime/src/kmp_ftn_entry.h
  openmp/runtime/src/kmp_global.cpp
  openmp/runtime/src/kmp_runtime.cpp
  openmp/runtime/test/target/target_thread_limit.cpp

Index: openmp/runtime/test/target/target_thread_limit.cpp
===
--- /dev/null
+++ openmp/runtime/test/target/target_thread_limit.cpp
@@ -0,0 +1,168 @@
+// RUN: %libomp-cxx-compile -fopenmp-version=51
+// RUN: %libomp-run | FileCheck %s --check-prefix OMP51
+
+#include 
+#include 
+
+void foo() {
+#pragma omp parallel num_threads(10)
+  { printf("\ntarget: foo(): parallel num_threads(10)"); }
+}
+
+int main(void) {
+
+  int tl = 4;
+  printf("\nmain: thread_limit = %d", omp_get_thread_limit());
+  // OMP51: main: thread_limit = {{[0-9]+}}
+
+#pragma omp target thread_limit(tl)
+  {
+printf("\ntarget: thread_limit = %d", omp_get_thread_limit());
+// OMP51: target: thread_limit = 4
+// check whether thread_limit is honoured
+#pragma omp parallel
+{ printf("\ntarget: parallel"); }
+// OMP51: target: parallel
+// OMP51: target: parallel
+// OMP51: target: parallel
+// OMP51: target: parallel
+// OMP51-NOT: target: parallel
+
+// check whether num_threads is honoured
+#pragma omp parallel num_threads(2)
+{ printf("\ntarget: parallel num_threads(2)"); }
+// OMP51: target: parallel num_threads(2)
+// OMP51: target: parallel num_threads(2)
+// OMP51-NOT: target: parallel num_threads(2)
+
+// check whether thread_limit is honoured when there is a conflicting
+// num_threads
+#pragma omp parallel num_threads(10)
+{ printf("\ntarget: parallel num_threads(10)"); }
+// OMP51: target: parallel num_threads(10)
+// OMP51: target: parallel num_threads(10)
+// OMP51: target: parallel num_threads(10)
+// OMP51: target: parallel num_threads(10)
+// OMP51-NOT: target: parallel num_threads(10)
+
+// check whether threads are limited across functions
+foo();
+// OMP51: target: foo(): parallel num_threads(10)
+// OMP51: target: foo(): parallel num_threads(10)
+// OMP51: target: foo(): parallel num_threads(10)
+// OMP51: target: foo(): parallel num_threads(10)
+// OMP51-NOT: target: foo(): parallel num_threads(10)
+
+// check if user can set num_threads at runtime
+omp_set_num_threads(2);
+#pragma omp parallel
+{ printf("\ntarget: parallel with omp_set_num_thread(2)"); }
+// OMP51: target: parallel with omp_set_num_thread(2)
+// OMP51: target: parallel with omp_set_num_thread(2)
+// OMP51-NOT: target: parallel with omp_set_num_thread(2)
+
+// make sure thread_limit is unaffected by omp_set_num_threads
+printf("\ntarget: thread_limit = %d", omp_get_thread_limit());
+// OMP51: target: thread_limit = 4
+  }
+
+// checking consecutive target regions with different thread_limits
+#pragma omp target thread_limit(3)
+  {
+printf("\nsecond target: thread_limit = %d", omp_get_thread_limit());
+// OMP51: second target: thread_limit = 3
+#pragma omp parallel
+{ printf("\nsecond target: parallel"); }
+// OMP51: second target: parallel
+// OMP51: second target: parallel
+// OMP51: second target: parallel
+// OMP51-NOT: second target: parallel
+  }
+
+  // confirm that thread_limit's effects are limited to target region
+  printf("\nmain: thread_limit = %d", omp_get_thread_limit());
+  // OMP51: main: thread_limit = {{[0-9]+}}
+#pragma omp parallel num_threads(10)
+  { printf("\nmain: parallel num_threads(10)"); }
+  // OMP51: main: parallel num_threads(10)
+  // OMP51: main: parallel num_threads(10)
+  // OMP51: main: parallel num_threads(10)
+  // OMP51: main: parallel num_threads(10)
+  // OMP51: main: parallel num_threads(10)
+  // OMP51: main: parallel num_threads(10)
+  // OMP51: main: parallel num_threads(10)
+  // OMP51: main: parallel num_threads(10)
+  // OMP51: main: parallel num_threads(10)
+  // OMP51: main: parallel num_threads(10)
+  // OMP51-NOT: main: parallel num_threads(10)
+
+// check combined target directives which support

[clang] 969df3b - Revert "[NFC][clang] EditedSource::applyRewrites - useless call"

[via cfe-commits]

Author: Manna, Soumi
Date: 2023-08-23T08:32:30-07:00
New Revision: 969df3ba1076cd7d443e420be8d4fc8d62a13e51

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

LOG: Revert "[NFC][clang] EditedSource::applyRewrites - useless call"

This reverts commit 9e150adaea7be2d87fb13255671584719a560939.

Added: 


Modified: 
clang/lib/Edit/EditedSource.cpp

Removed: 




diff  --git a/clang/lib/Edit/EditedSource.cpp b/clang/lib/Edit/EditedSource.cpp
index 5e77b0141e8a0e..a3386b2489b07c 100644
--- a/clang/lib/Edit/EditedSource.cpp
+++ b/clang/lib/Edit/EditedSource.cpp
@@ -430,7 +430,7 @@ void EditedSource::applyRewrites(EditsReceiver &receiver,
 if (offs == CurEnd) {
   StrVec += act.Text;
   CurLen += act.RemoveLen;
-  CurEnd = CurEnd.getWithOffset(act.RemoveLen);
+  CurEnd.getWithOffset(act.RemoveLen);
   continue;
 }
 



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

[PATCH] D158140: WIP: [clang-repl] Basic WebAssembly support for running inside a JS engine

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

Very nice idea!




Comment at: clang/lib/Interpreter/CMakeLists.txt:21
   Value.cpp
+  WASM.cpp
 

Elsewhere in LLVM either use WebAssembly (spelled out).   If you really prefer 
to keep it short you should use `Wasm` (not all caps).

Also, I wonder if we should call this `Emscripten` given that it seems 
emscripten specific.. then you could have another one called WASI or 
WebAssembly for the less specific one?



Comment at: clang/lib/Interpreter/WASM.cpp:79
+  int Result =
+  lld::wasm::link(LinkerArgs, llvm::outs(), llvm::errs(), false, false);
+  if (!Result)

argentite wrote:
> v.g.vassilev wrote:
> > I am not sure how we can solve that dependency here. Worst case scenario, 
> > could we check if `lld` is installed and make a system call?
> AFAIK we can't really `exec()` within Emscripten.
This looks its using the in-process call to the linker library, rather then an 
exec of the linker process.. which could work.   But is it OK to have the 
compiler depend on the linker like this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158140

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

[PATCH] D158628: [clang][dataflow][WIP] Prototype for pointer value widening (nullable `getPointeeLoc()` flavor)

[Martin Böhme via Phabricator via cfe-commits]

mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

DO NOT REVIEW

This is a WIP for discussion only.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158628

Files:
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/Value.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
  clang/unittests/Analysis/FlowSensitive/ValueTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/ValueTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/ValueTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/ValueTest.cpp
@@ -31,8 +31,8 @@
 
 TEST(ValueTest, AliasedPointersEquivalent) {
   auto L = ScalarStorageLocation(QualType());
-  PointerValue V1(L);
-  PointerValue V2(L);
+  PointerValue V1(&L);
+  PointerValue V2(&L);
   EXPECT_TRUE(areEquivalentValues(V1, V2));
   EXPECT_TRUE(areEquivalentValues(V2, V1));
 }
@@ -60,7 +60,7 @@
 TEST(ValueTest, DifferentKindsNotEquivalent) {
   Arena A;
   auto L = ScalarStorageLocation(QualType());
-  PointerValue V1(L);
+  PointerValue V1(&L);
   TopBoolValue V2(A.makeAtomRef(Atom(0)));
   EXPECT_FALSE(areEquivalentValues(V1, V2));
   EXPECT_FALSE(areEquivalentValues(V2, V1));
@@ -68,9 +68,9 @@
 
 TEST(ValueTest, NotAliasedPointersNotEquivalent) {
   auto L1 = ScalarStorageLocation(QualType());
-  PointerValue V1(L1);
+  PointerValue V1(&L1);
   auto L2 = ScalarStorageLocation(QualType());
-  PointerValue V2(L2);
+  PointerValue V2(&L2);
   EXPECT_FALSE(areEquivalentValues(V1, V2));
   EXPECT_FALSE(areEquivalentValues(V2, V1));
 }
Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -768,7 +768,7 @@
 const auto *FooLoc =
 cast(Env.getStorageLocation(*FooDecl));
 const auto *BarVal = cast(Env.getValue(*BarDecl));
-EXPECT_EQ(&BarVal->getPointeeLoc(), FooLoc);
+EXPECT_EQ(BarVal->getPointeeLoc(), FooLoc);
   });
 }
 
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -165,7 +165,7 @@
 ASSERT_THAT(FooValue, NotNull());
 
 EXPECT_TRUE(isa(FooValue->getPointeeLoc()));
-auto *FooPointeeValue = Env.getValue(FooValue->getPointeeLoc());
+auto *FooPointeeValue = Env.getValue(*FooValue->getPointeeLoc());
 ASSERT_THAT(FooPointeeValue, NotNull());
 EXPECT_TRUE(isa(FooPointeeValue));
   });
@@ -496,7 +496,7 @@
 const auto &FooPtrVal =
 *cast(getFieldValue(&BarLoc, *FooPtrDecl, Env));
 const auto &FooPtrPointeeLoc =
-cast(FooPtrVal.getPointeeLoc());
+*cast(FooPtrVal.getPointeeLoc());
 EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), NotNull());
 EXPECT_THAT(getFieldValue(&FooPtrPointeeLoc, *BarDecl, Env), IsNull());
 
@@ -504,8 +504,9 @@
 
 const auto &BazPtrVal =
 *cast(getFieldValue(&BarLoc, *BazPtrDecl, Env));
-const StorageLocation &BazPtrPointeeLoc = BazPtrVal.getPointeeLoc();
-EXPECT_THAT(Env.getValue(BazPtrPointeeLoc), NotNull());
+const StorageLocation *BazPtrPointeeLoc = BazPtrVal.getPointeeLoc();
+ASSERT_THAT(BazPtrPointeeLoc, NotNull());
+EXPECT_THAT(Env.getValue(*BazPtrPointeeLoc), NotNull());
   });
 }
 
@@ -534,8 +535,8 @@
 ASSERT_TRUE(isa_and_nonnull(FooLoc));
 
 const PointerValue *FooVal = cast(Env.getValue(*FooLoc));
-const StorageLocation &FooPointeeLoc = FooVal->getPointeeLoc();
-EXPECT_TRUE(isa(&FooPointeeLoc));
+const auto &FooPointeeLoc =
+*cast(FooVal->getPointeeLoc());
 
 const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
 EXPECT_TRUE(isa_and_nonnull(FooPointeeVal));
@@ -637,28 +638,32 @@
 const auto &FooLoc =
 *cast(Env.getStorageLocation(*FooDecl));
 const auto &FooVal = *cast(Env.getValue(FooLoc));
+ASSERT_THAT(FooVal.getPointeeLoc(), NotNull());
 const auto &FooPointeeVal =
-*cast(Env.getValue(FooVal.getPointeeLoc()));
+*cast(Env.get

[PATCH] D158629: [ClangRepl] support code completion at the repl

[Fred Fu via Phabricator via cfe-commits]

capfredf created this revision.
Herald added subscribers: ChuanqiXu, kadircet, arphaman.
Herald added a project: All.
capfredf requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.

This patch enabled code completion for ClangREPL. The feature was built upon 
three
existing Clang components: a list completer for `LineEditor`,
a CompletionConsumer from `SemaCodeCompletion`, and the ASTUnit::codeComplete 
method.
with the first component serving as the main entry point of handling 
interactive inputs.

Because a completion point for a compiler instance has to be unchanged once it
is set, an incremental compiler instance is created for each code
completion. Such a compiler instance carries over AST context source from the
main interpreter compiler in order to obtain declarations or bindings from
previous input in the same REPL session.

The most important API `codeComplete` in `Interpreter/CodeCompletion` is a thin
wrapper that calls with ASTUnit::codeComplete with necessary arguments, such as
a code completion point and a `ReplCompletionConsumer`, which communicates
completion results from `SemaCodeCompletion` back to the list completer for the
REPL.

In addition, `PCC_TopLevelOrExpression and `CCC_TopLevelOrExpression` top levels
were added so that `SemaCodeCompletion` can treat top level statements like
expression statements at the REPL. For example,

  clang-repl> int foo = 42;
  clang-repl> f

>From a parser's persective, the cursor is at a top level. If we used code
completion without any changes, `PCC_Namespace` would be supplied to
`Sema::CodeCompleteOrdinaryName`, and thus the completion results would not
include `foo`.

Currently, the way we use `PCC_TopLevelOrExpression` and
`CCC_TopLevelOrExpression` is no different from the way we use `PCC_Statement`
and `CCC_Statement` respectively.

Previous Differential Revision: https://reviews.llvm.org/D154382


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158629

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang/include/clang/Frontend/ASTUnit.h
  clang/include/clang/Interpreter/CodeCompletion.h
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/CodeCompletion.cpp
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/incrememal-mode-completion-no-error.cpp
  clang/test/CodeCompletion/incremental-top-level.cpp
  clang/tools/clang-repl/ClangRepl.cpp
  clang/tools/libclang/CIndexCodeCompletion.cpp
  clang/unittests/Interpreter/CMakeLists.txt
  clang/unittests/Interpreter/CodeCompletionTest.cpp

Index: clang/unittests/Interpreter/CodeCompletionTest.cpp
===
--- /dev/null
+++ clang/unittests/Interpreter/CodeCompletionTest.cpp
@@ -0,0 +1,101 @@
+#include "clang/Interpreter/CodeCompletion.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Interpreter/Interpreter.h"
+#include "clang/Sema/CodeCompleteConsumer.h"
+#include "llvm/LineEditor/LineEditor.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+namespace {
+auto CB = clang::IncrementalCompilerBuilder();
+
+static std::unique_ptr createInterpreter() {
+  auto CI = cantFail(CB.CreateCpp());
+  return cantFail(clang::Interpreter::create(std::move(CI)));
+}
+
+static std::vector runComp(clang::Interpreter &MainInterp,
+llvm::StringRef Prefix,
+llvm::Error &ErrR) {
+  auto CI = CB.CreateCpp();
+  if (auto Err = CI.takeError()) {
+ErrR = std::move(Err);
+return {};
+  }
+
+  auto Interp = clang::Interpreter::create(std::move(*CI));
+  if (auto Err = Interp.takeError()) {
+// log the error and returns an empty vector;
+ErrR = std::move(Err);
+
+return {};
+  }
+
+  std::vector Results;
+
+  codeComplete(
+  const_cast((*Interp)->getCompilerInstance()),
+  Prefix, 1, Prefix.size(), MainInterp.getCompilerInstance(), Results);
+
+  std::vector Comps;
+  for (auto c : convertToCodeCompleteStrings(Results)) {
+if (c.find(Prefix) == 0)
+  Comps.push_back(c.substr(Prefix.size()));
+  }
+
+  return Comps;
+}
+
+TEST(CodeCompletionTest, Sanity) {
+  auto Interp = createInterpreter();
+  if (auto R = Interp->ParseAndExecute("int foo = 12;")) {
+consumeError(std::move(R));
+return;
+  }
+  auto Err = llvm::Error::success();
+  auto comps = runComp(*Interp, "f", Err);
+  EXPECT_EQ((size_t)2, comps.size()); // foo and float
+  EXPECT_EQ(comps[0], std::string("oo"));

[PATCH] D158140: WIP: [clang-repl] Basic WebAssembly support for running inside a JS engine

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

I suppose you could build a similar thing that runs under node and uses that 
`wasi-unknown-unknown` target?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158140

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

[PATCH] D158227: [clang] EditedSource::applyRewrites - useless call

[Soumi Manna via Phabricator via cfe-commits]

Manna added a comment.

In D158227#4610219 , @dyung wrote:

> In D158227#4610097 , @Manna wrote:
>
>> The failure seems unrelated to my change.
>
> Hi @Manna, I'm not sure which failure you are referring to, but there is a 
> test failure on at least 2 build bots that seems to have been caused by your 
> change. Can you take a look and revert if you need time to investigate?
>
> https://lab.llvm.org/buildbot/#/builders/139/builds/48141
> https://lab.llvm.org/buildbot/#/builders/216/builds/26116

Thank you @dyung for reporting the buildbot failure. I have reverted my patch. 
I will investigate it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158227

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

[PATCH] D158630: [clang][dataflow][WIP] Prototype for pointer value widening ("unknown storage location" flavor)

[Martin Böhme via Phabricator via cfe-commits]

mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

DO NOT REVIEW

This is a WIP for discussion only.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158630

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -3843,10 +3843,7 @@
   }
 }
   )cc";
-  // FIXME: Implement pointer value widening to make analysis converge.
-  ASSERT_THAT_ERROR(
-  checkDataflowWithNoopAnalysis(Code),
-  llvm::FailedWithMessage("maximum number of iterations reached"));
+  ASSERT_THAT_ERROR(checkDataflowWithNoopAnalysis(Code), llvm::Succeeded());
 }
 
 TEST(TransferTest, LoopDereferencingChangingRecordPointerConverges) {
@@ -3868,10 +3865,7 @@
   }
 }
   )cc";
-  // FIXME: Implement pointer value widening to make analysis converge.
-  ASSERT_THAT_ERROR(
-  checkDataflowWithNoopAnalysis(Code),
-  llvm::FailedWithMessage("maximum number of iterations reached"));
+  ASSERT_THAT_ERROR(checkDataflowWithNoopAnalysis(Code), llvm::Succeeded());
 }
 
 TEST(TransferTest, DoesNotCrashOnUnionThisExpr) {
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -161,6 +161,16 @@
 return CurrentEnv.makeTopBoolValue();
   }
 
+  if (auto *PrevPtr = dyn_cast(&Prev)) {
+auto &CurPtr = cast(Current);
+
+if (&PrevPtr->getPointeeLoc() != &CurPtr.getPointeeLoc())
+  // TODO: Widen properties
+  return CurrentEnv.create(
+  CurrentEnv.getDataflowAnalysisContext().getUnknownStorageLocation(
+  CurPtr.getPointeeLoc().getType()));
+  }
+
   // FIXME: Add other built-in model widening.
 
   // Custom-model widening.
@@ -658,6 +668,20 @@
 void Environment::setValue(const StorageLocation &Loc, Value &Val) {
   assert(!isa(&Val) || &cast(&Val)->getLoc() == &Loc);
 
+  // TODO: Currently, if `Loc` is an unknown storage location, just bail out.
+  // But is this the right thing to do?
+  // Alternatives:
+  // - Assert that `Loc` is not an unknown storage location. But then that
+  //   potentially puts the burden of testing on the caller. And what else would
+  //   they do except not set the value?
+  // - Because the unknown storage location is conceptually a "top", it
+  //   effectively refers to the set of _all_ possible storage locations. So,
+  //   because we could be potentially setting the value of any storage,
+  //   maybe we should blow away _all_ of the entries in `LocToVal`? Or at least
+  //   those whose type is one that is compatible with `Loc` in the sense that
+  //   it can refer to a value of type `Loc.getType()`?
+  if (getDataflowAnalysisContext().isUnknownStorageLocation(Loc))
+return;
   LocToVal[&Loc] = &Val;
 }
 
@@ -686,6 +710,8 @@
 }
 
 Value *Environment::getValue(const StorageLocation &Loc) const {
+  if (getDataflowAnalysisContext().isUnknownStorageLocation(Loc))
+return nullptr;
   return LocToVal.lookup(&Loc);
 }
 
Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -90,6 +90,32 @@
   return Loc;
 }
 
+StorageLocation &
+DataflowAnalysisContext::getUnknownStorageLocation(QualType Ty) {
+  auto Res = UnknownStorageLocations.try_emplace(Ty, nullptr);
+  if (Res.second) {
+if (!Ty.isNull() && Ty->isRecordType()) {
+  llvm::DenseMap FieldLocs;
+  for (const FieldDecl *Field : getModeledFields(Ty))
+FieldLocs.insert({Field, &getUnknownStorageLocation(
+ Field->getType().getNonReferenceType())});
+  Res.first->second =
+  &arena().create(Ty, std::move(FieldLocs));
+} else {
+  Res.first->second = &arena().create(Ty);
+}
+  }
+  return *Res.first->second;
+}
+
+bool DataflowAnalysisContext::isUnknownStorageLocation(
+const StorageLocation &Loc) const {
+  auto Iter = UnknownStorageLocations.find(Loc.getType());
+  if (Iter == UnknownStorageLocations.end())
+return false;
+  return Iter->second == &Loc;
+}
+
 PointerValue &
 DataflowAnalysisContext::getOrCreateNullPointerValue(QualType PointeeT