[PATCH] D146987: [Assignment Tracking] Enable by default

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

Hello, I also noticed this potentially causing problems for scalable vectors:
https://godbolt.org/z/qdr8P86aW
That probably counts as one of the "edge cases for things we hadn't accounted 
for".
Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146987

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

[PATCH] D148901: [clang][Interp] Fix post-inc/dec operator result

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

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

  We pushed the wrong value on the stack, always leaving a 0 behind.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148901

Files:
  clang/lib/AST/Interp/Interp.h
  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
@@ -706,6 +706,23 @@
  // ref-note {{cannot refer to element -1 of array of 3 elements}}
 return *p;
   }
+
+  /// This used to leave a 0 on the stack instead of the previous
+  /// value of a.
+  constexpr int bug1Inc() {
+int a = 3;
+int b = a++;
+return b;
+  }
+  static_assert(bug1Inc() == 3);
+
+  constexpr int bug1Dec() {
+int a = 3;
+int b = a--;
+return b;
+  }
+  static_assert(bug1Dec() == 3);
+
 };
 #endif
 
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -437,7 +437,7 @@
   T Result;
 
   if constexpr (DoPush == PushVal::Yes)
-S.Stk.push(Result);
+S.Stk.push(Value);
 
   if constexpr (Op == IncDecOp::Inc) {
 if (!T::increment(Value, &Result)) {


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -706,6 +706,23 @@
  // ref-note {{cannot refer to element -1 of array of 3 elements}}
 return *p;
   }
+
+  /// This used to leave a 0 on the stack instead of the previous
+  /// value of a.
+  constexpr int bug1Inc() {
+int a = 3;
+int b = a++;
+return b;
+  }
+  static_assert(bug1Inc() == 3);
+
+  constexpr int bug1Dec() {
+int a = 3;
+int b = a--;
+return b;
+  }
+  static_assert(bug1Dec() == 3);
+
 };
 #endif
 
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -437,7 +437,7 @@
   T Result;
 
   if constexpr (DoPush == PushVal::Yes)
-S.Stk.push(Result);
+S.Stk.push(Value);
 
   if constexpr (Op == IncDecOp::Inc) {
 if (!T::increment(Value, &Result)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147844: Emit warning when implicit cast from int to bool happens in an conditional operator expression

[NagaChaitanya Vellanki via Phabricator via cfe-commits]

chaitanyav added a comment.

@aaron.ballman please take a look.

I filed a issue that premerge checks are not working on AIX
https://github.com/google/llvm-premerge-checks/issues/441

Not sure why the TSAN, MSAN and ASAN checks are failing on libcxx


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

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

[clang] 0ba922f - Revert "Reapply D146987 "[Assignment Tracking] Enable by default""

[Carlos Alberto Enciso via cfe-commits]

Author: Carlos Alberto Enciso
Date: 2023-04-21T09:11:40+01:00
New Revision: 0ba922f600469df273c753f873668e41025487c0

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

LOG: Revert "Reapply D146987 "[Assignment Tracking] Enable by default""

This reverts commit b74aeaccbae876ca348aa87a3db05d444052ae65.
Note: The author (Orlando) asked to revert this commit.

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/test/CodeGen/assignment-tracking/flag.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 604b4a45fffc1..eed0d517a1ad7 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5817,7 +5817,7 @@ def fexperimental_assignment_tracking_EQ : Joined<["-"], 
"fexperimental-assignme
   Group, CodeGenOpts<"EnableAssignmentTracking">,
   NormalizedValuesScope<"CodeGenOptions::AssignmentTrackingOpts">,
   Values<"disabled,enabled,forced">, 
NormalizedValues<["Disabled","Enabled","Forced"]>,
-  MarshallingInfoEnum, "Enabled">;
+  MarshallingInfoEnum, "Disabled">;
 
 } // let Flags = [CC1Option, NoDriverOption]
 

diff  --git a/clang/test/CodeGen/assignment-tracking/flag.cpp 
b/clang/test/CodeGen/assignment-tracking/flag.cpp
index 3bd974fe07c6c..aa1f054dae4d7 100644
--- a/clang/test/CodeGen/assignment-tracking/flag.cpp
+++ b/clang/test/CodeGen/assignment-tracking/flag.cpp
@@ -8,10 +8,10 @@
 // RUN: -emit-llvm  %s -o - -fexperimental-assignment-tracking=disabled 
-O1\
 // RUN: | FileCheck %s --check-prefixes=DISABLE
 
- Enabled by default:
+ Disabled by default:
 // RUN: %clang_cc1 -triple x86_64-none-linux-gnu -debug-info-kind=standalone   
\
 // RUN: -emit-llvm  %s -o - -O1
\
-// RUN: | FileCheck %s --check-prefixes=ENABLE
+// RUN: | FileCheck %s --check-prefixes=DISABLE
 
  Disabled at O0 unless forced.
 // RUN: %clang_cc1 -triple x86_64-none-linux-gnu -debug-info-kind=standalone   
\



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

[PATCH] D146987: [Assignment Tracking] Enable by default

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

I'm also running into issues due to this commit. Unfortunately, the issues 
don't seem to be entirely deterministic. They can be reproduced with 
https://martin.st/temp/python-preproc.c:

  $ clang -target i686-w64-mingw32 -c -g -O3 python-preproc.c
  Assertion failed: (!(Elmt.getFragmentOrDefault() == 
Next.getFragmentOrDefault())), function operator(), file 
AssignmentTrackingAnalysis.cpp, line 2020.

I'm seeing this on macOS with LLVM built with Xcode's clang. On Linux, built 
with either GCC or Clang, I don't run into this issue.

When running the seemingly working compiler build on Linux in valgrind, I'm 
hitting use of uninitialized data:

  ==3054473== Conditional jump or move depends on uninitialised value(s)
  ==3054473==at 0x4900F0A: 
llvm::ConstantExpr::getGetElementPtr(llvm::Type*, llvm::Constant*, 
llvm::ArrayRef, bool, std::optional, llvm::Type*) 
(in /home/martin/code/llvm-project-bisect/llvm/build/bin/clang-17)
  ==3054473==by 0x48C865F: foldGEPOfGEP(llvm::GEPOperator*, llvm::Type*, 
bool, llvm::ArrayRef) (in 
/home/martin/code/llvm-project-bisect/llvm/build/bin/clang-17)
  ==3054473==by 0x48D1285: llvm::ConstantFoldGetElementPtr(llvm::Type*, 
llvm::Constant*, bool, std::optional, 
llvm::ArrayRef) (in 
/home/martin/code/llvm-project-bisect/llvm/build/bin/clang-17)
  ==3054473==by 0x4900CEF: 
llvm::ConstantExpr::getGetElementPtr(llvm::Type*, llvm::Constant*, 
llvm::ArrayRef, bool, std::optional, llvm::Type*) 
(in /home/martin/code/llvm-project-bisect/llvm/build/bin/clang-17)
  ==3054473==by 0x4025C21: simplifyGEPInst(llvm::Type*, llvm::Value*, 
llvm::ArrayRef, bool, llvm::SimplifyQuery const&, unsigned int) 
[clone .constprop.0] (in 
/home/martin/code/llvm-project-bisect/llvm/build/bin/clang-17)
  ==3054473==by 0x4036E5F: 
simplifyInstructionWithOperands(llvm::Instruction*, 
llvm::ArrayRef, llvm::SimplifyQuery const&, unsigned int) [clone 
.constprop.0] (in /home/martin/code/llvm-project-bisect/llvm/build/bin/clang-17)
  ==3054473==by 0x4037423: llvm::simplifyInstruction(llvm::Instruction*, 
llvm::SimplifyQuery const&) (in 
/home/martin/code/llvm-project-bisect/llvm/build/bin/clang-17)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146987

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

[PATCH] D144269: [Analyzer] Show "taint originated here" note of alpha.security.taint.TaintPropagation checker at the correct place

[Daniel Krupp via Phabricator via cfe-commits]

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

@steakhal is there anything else to do before we merge this? Thanks.


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

https://reviews.llvm.org/D144269

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

[PATCH] D147591: [clang][Interp] Handle CXXTemporaryObjectExprs

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

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147591

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

[PATCH] D146030: [clang][Interp] Handle LambdaExprs

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

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146030

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

[PATCH] D147844: Emit warning when implicit cast from int to bool happens in an conditional operator expression

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

I have to say I'm not really convinced this change is a good idea. The cases it 
flags don't really seem in any way ambiguous/erroneous.

In D147844#4286364 , @chaitanyav 
wrote:

> @aaron.ballman please take a look.
>
> I filed a issue that premerge checks are not working on AIX
> https://github.com/google/llvm-premerge-checks/issues/441
>
> Not sure why the TSAN, MSAN and ASAN checks are failing on libcxx
> https://buildkite.com/llvm-project/libcxx-ci/builds/22664

The AIX builders are currently broken. That has to do with an update. We don't 
really know what's going on with the sanitizer builds, but these are unrelated 
too (probably also a problem with the builders).




Comment at: libcxx/include/strstream:303
 : ostream(&__sb_),
-  __sb_(__s, __n, __s + (__mode & ios::app ? _VSTD::strlen(__s) : 0))
 {}

Please don't change the indentation. It was correct before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

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

[PATCH] D148908: [Driver][NFC] Simplify code.

[Jianjian Guan via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148908

Files:
  clang/lib/Driver/Driver.cpp


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4095,16 +4095,15 @@
 
   Current = NewCurrent;
 
-  // Use the current host action in any of the offloading actions, if
-  // required.
-  if (!UseNewOffloadingDriver)
-if (OffloadBuilder->addHostDependenceToDeviceActions(Current, 
InputArg))
-  break;
-
   // Try to build the offloading actions and add the result as a dependency
   // to the host.
   if (UseNewOffloadingDriver)
 Current = BuildOffloadingActions(C, Args, I, Current);
+  // Use the current host action in any of the offloading actions, if
+  // required.
+  else if (OffloadBuilder->addHostDependenceToDeviceActions(Current,
+InputArg))
+break;
 
   if (Current->getType() == types::TY_Nothing)
 break;


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4095,16 +4095,15 @@
 
   Current = NewCurrent;
 
-  // Use the current host action in any of the offloading actions, if
-  // required.
-  if (!UseNewOffloadingDriver)
-if (OffloadBuilder->addHostDependenceToDeviceActions(Current, InputArg))
-  break;
-
   // Try to build the offloading actions and add the result as a dependency
   // to the host.
   if (UseNewOffloadingDriver)
 Current = BuildOffloadingActions(C, Args, I, Current);
+  // Use the current host action in any of the offloading actions, if
+  // required.
+  else if (OffloadBuilder->addHostDependenceToDeviceActions(Current,
+InputArg))
+break;
 
   if (Current->getType() == types::TY_Nothing)
 break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144943: [clang][Interp] Implement bitcasts (WIP)

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

tbaeder updated this revision to Diff 515659.

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

https://reviews.llvm.org/D144943

Files:
  clang/lib/AST/Interp/Boolean.h
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Descriptor.h
  clang/lib/AST/Interp/Floating.h
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/lib/AST/Interp/Opcodes.td
  clang/lib/AST/Interp/Pointer.cpp
  clang/lib/AST/Interp/Pointer.h
  clang/lib/AST/Interp/PrimType.h
  clang/test/AST/Interp/builtin-bit-cast.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
@@ -90,6 +90,11 @@
 static_assert(p != nullptr, "");
 static_assert(*p == 10, "");
 
+constexpr const void *cp = (void *)p;
+// FIXME: This should be an error in the new interpreter.
+constexpr const int *pm = (int*)cp; // ref-error {{ must be initialized by a constant expression}} \
+// ref-note {{cast from 'const void *' is not allowed}}
+
 constexpr const int* getIntPointer() {
   return &m;
 }
Index: clang/test/AST/Interp/builtin-bit-cast.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/builtin-bit-cast.cpp
@@ -0,0 +1,228 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify -std=c++2a -fsyntax-only %s
+// RUN: %clang_cc1 -verify=ref -std=c++2a -fsyntax-only %s
+// RUN: %clang_cc1 -verify -std=c++2a -fsyntax-only -triple aarch64_be-linux-gnu -fexperimental-new-constant-interpreter %s
+// RUN: %clang_cc1 -verify -std=c++2a -fsyntax-only -triple aarch64_be-linux-gnu -fexperimental-new-constant-interpreter %s
+
+
+/// ref-no-diagnostics
+
+
+/// FIXME: This is a version of clang/test/SemaCXX/builtin-bit-cast.cpp with
+///   the currently supported subset of operations. They should *all* be
+///   supported though.
+
+
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+#  define LITTLE_END 1
+#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+#  define LITTLE_END 0
+#else
+#  error "huh?"
+#endif
+
+template  struct is_same {
+  static constexpr bool value = false;
+};
+template  struct is_same {
+  static constexpr bool value = true;
+};
+
+static_assert(sizeof(int) == 4);
+static_assert(sizeof(long long) == 8);
+
+template 
+constexpr To bit_cast(const From &from) {
+  static_assert(sizeof(To) == sizeof(From));
+  return __builtin_bit_cast(To, from);
+}
+
+template 
+constexpr bool round_trip(const Init &init) {
+  return bit_cast(bit_cast(init)) == init;
+}
+
+static_assert(round_trip((int)-1));
+static_assert(round_trip((int)0x12345678));
+static_assert(round_trip((int)0x87654321));
+static_assert(round_trip((int)0x0C05FEFE));
+static_assert(round_trip((int)0x0C05FEFE));
+
+constexpr unsigned char input[] = {0xCA, 0xFE, 0xBA, 0xBE};
+constexpr unsigned expected = LITTLE_END ? 0xBEBAFECA : 0xCAFEBABE;
+static_assert(bit_cast(input) == expected);
+
+constexpr short S[] = {10, 20};
+constexpr int I = __builtin_bit_cast(int, S);
+static_assert(I == (LITTLE_END ? 1310730 : 655380));
+
+
+struct int_splicer {
+  unsigned x;
+  unsigned y;
+
+  constexpr int_splicer() : x(1), y(2) {}
+  constexpr int_splicer(unsigned x, unsigned y) : x(x), y(y) {}
+
+  constexpr bool operator==(const int_splicer &other) const {
+return other.x == x && other.y == y;
+  }
+};
+
+constexpr int_splicer splice(0x0C05FEFE, 0xCAFEBABE);
+
+static_assert(bit_cast(splice) == (LITTLE_END
+   ? 0xCAFEBABE0C05FEFE
+   : 0x0C05FEFECAFEBABE));
+
+constexpr int_splicer IS = bit_cast(0xCAFEBABE0C05FEFE);
+static_assert(bit_cast(0xCAFEBABE0C05FEFE).x == (LITTLE_END
+  ? 0x0C05FEFE
+  : 0xCAFEBABE));
+
+static_assert(round_trip(splice));
+static_assert(round_trip(splice));
+
+struct base2 {
+};
+
+struct base3 {
+  unsigned z;
+  constexpr base3() : z(3) {}
+};
+
+struct bases : int_splicer, base2, base3 {
+  unsigned doublez;
+  constexpr bases() : doublez(4) {}
+};
+
+struct tuple4 {
+  unsigned x, y, z, doublez;
+
+  constexpr bool operator==(tuple4 const &other) const {
+return x == other.x && y == other.y &&
+   z == other.z && doublez == other.doublez;
+  }
+};
+constexpr bases b;// = {{1, 2}, {}, {3}, 4};
+constexpr tuple4 t4 = bit_cast(b);
+
+// Regardless of endianness, this should hold:
+static_assert(t4.x == 1);
+static_assert(t4.y == 2);
+static_assert(t4.z == 3);
+static_assert(t4.doublez == 4);
+//static_assert(t4 == tuple4{1, 2, 3, 4});
+static_assert(round_trip(b));
+
+
+namespace WithBases {
+  struct Base {
+char A[3] = {1,2,

[PATCH] D147840: [clang][Interp] Handle DiscardResult for DeclRef- and ParenExprs

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

tbaeder updated this revision to Diff 515660.

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

https://reviews.llvm.org/D147840

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
@@ -780,4 +780,18 @@
   return F{12}.a;
 }
 static_assert(ignoredDecls() == 12, "");
+
+struct A{};
+constexpr int ignoredExprs() {
+  (void)(1 / 2);
+  A a;
+  a; // expected-warning {{unused}} \
+ // ref-warning {{unused}}
+  (void)a;
+  (a); // expected-warning {{unused}} \
+   // ref-warning {{unused}}
+
+  return 0;
+}
+
 #endif
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -231,7 +231,12 @@
 
 template 
 bool ByteCodeExprGen::VisitParenExpr(const ParenExpr *PE) {
-  return this->visit(PE->getSubExpr());
+  const Expr *SubExpr = PE->getSubExpr();
+
+  if (DiscardResult)
+return this->discard(SubExpr);
+
+  return this->visit(SubExpr);
 }
 
 template 
@@ -1996,6 +2001,9 @@
 
 template 
 bool ByteCodeExprGen::VisitDeclRefExpr(const DeclRefExpr *E) {
+  if (DiscardResult)
+return true;
+
   const auto *D = E->getDecl();
 
   if (const auto *ECD = dyn_cast(D)) {


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -780,4 +780,18 @@
   return F{12}.a;
 }
 static_assert(ignoredDecls() == 12, "");
+
+struct A{};
+constexpr int ignoredExprs() {
+  (void)(1 / 2);
+  A a;
+  a; // expected-warning {{unused}} \
+ // ref-warning {{unused}}
+  (void)a;
+  (a); // expected-warning {{unused}} \
+   // ref-warning {{unused}}
+
+  return 0;
+}
+
 #endif
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -231,7 +231,12 @@
 
 template 
 bool ByteCodeExprGen::VisitParenExpr(const ParenExpr *PE) {
-  return this->visit(PE->getSubExpr());
+  const Expr *SubExpr = PE->getSubExpr();
+
+  if (DiscardResult)
+return this->discard(SubExpr);
+
+  return this->visit(SubExpr);
 }
 
 template 
@@ -1996,6 +2001,9 @@
 
 template 
 bool ByteCodeExprGen::VisitDeclRefExpr(const DeclRefExpr *E) {
+  if (DiscardResult)
+return true;
+
   const auto *D = E->getDecl();
 
   if (const auto *ECD = dyn_cast(D)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144164: [clang][Interp] Handle PtrMemOps

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

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144164

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

[PATCH] D143334: [clang][Interp] Fix diagnosing uninitialized ctor record arrays

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

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143334

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

[PATCH] D144457: [clang][Interp] Handle global composite temporaries

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

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144457

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

[PATCH] D142630: [clang][Interp] Implement virtual function calls

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

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142630

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

[PATCH] D148596: [KMSAN] Enable on SystemZ

[Ilya Leoshkevich via Phabricator via cfe-commits]

iii updated this revision to Diff 515661.
iii added a comment.

- (unsigned)0 -> 0u (0 doesn't work, because the overload becomes ambiguous).
- Improve matching in the testcase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148596

Files:
  clang/lib/Driver/ToolChains/Linux.cpp
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
  llvm/test/Instrumentation/MemorySanitizer/SystemZ/basic-kernel.ll

Index: llvm/test/Instrumentation/MemorySanitizer/SystemZ/basic-kernel.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/MemorySanitizer/SystemZ/basic-kernel.ll
@@ -0,0 +1,169 @@
+; RUN: opt < %s -S -mcpu=z13 -msan-kernel=1 -float-abi=soft -passes=msan 2>&1 | FileCheck %s
+
+target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-a:8:16-n32:64"
+target triple = "s390x-unknown-linux-gnu"
+
+define void @Store1(ptr %p, i8 %x) sanitize_memory {
+entry:
+  store i8 %x, ptr %p
+  ret void
+}
+
+; CHECK-LABEL: define {{[^@]+}}@Store1(
+; CHECK: [[META_PTR:%[a-z0-9_]+]] = alloca { ptr, ptr }, align 8
+; CHECK: call void @__msan_metadata_ptr_for_store_1(ptr [[META_PTR]], ptr %p)
+; CHECK: [[META:%[a-z0-9_]+]] = load { ptr, ptr }, ptr [[META_PTR]], align 8
+; CHECK: [[SHADOW:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 0
+; CHECK: [[ORIGIN:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 1
+; CHECK: store i8 {{.+}}, ptr [[SHADOW]], align 1
+; CHECK: ret void
+
+define void @Store2(ptr %p, i16 %x) sanitize_memory {
+entry:
+  store i16 %x, ptr %p
+  ret void
+}
+
+; CHECK-LABEL: define {{[^@]+}}@Store2(
+; CHECK: [[META_PTR:%[a-z0-9_]+]] = alloca { ptr, ptr }, align 8
+; CHECK: call void @__msan_metadata_ptr_for_store_2(ptr [[META_PTR]], ptr %p)
+; CHECK: [[META:%[a-z0-9_]+]] = load { ptr, ptr }, ptr [[META_PTR]], align 8
+; CHECK: [[SHADOW:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 0
+; CHECK: [[ORIGIN:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 1
+; CHECK: store i16 {{.+}}, ptr [[SHADOW]], align 2
+; CHECK: ret void
+
+define void @Store4(ptr %p, i32 %x) sanitize_memory {
+entry:
+  store i32 %x, ptr %p
+  ret void
+}
+
+; CHECK-LABEL: define {{[^@]+}}@Store4(
+; CHECK: [[META_PTR:%[a-z0-9_]+]] = alloca { ptr, ptr }, align 8
+; CHECK: call void @__msan_metadata_ptr_for_store_4(ptr [[META_PTR]], ptr %p)
+; CHECK: [[META:%[a-z0-9_]+]] = load { ptr, ptr }, ptr [[META_PTR]], align 8
+; CHECK: [[SHADOW:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 0
+; CHECK: [[ORIGIN:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 1
+; CHECK: store i32 {{.+}}, ptr [[SHADOW]], align 4
+; CHECK: ret void
+
+define void @Store8(ptr %p, i64 %x) sanitize_memory {
+entry:
+  store i64 %x, ptr %p
+  ret void
+}
+
+; CHECK-LABEL: define {{[^@]+}}@Store8(
+; CHECK: [[META_PTR:%[a-z0-9_]+]] = alloca { ptr, ptr }, align 8
+; CHECK: call void @__msan_metadata_ptr_for_store_8(ptr [[META_PTR]], ptr %p)
+; CHECK: [[META:%[a-z0-9_]+]] = load { ptr, ptr }, ptr [[META_PTR]], align 8
+; CHECK: [[SHADOW:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 0
+; CHECK: [[ORIGIN:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 1
+; CHECK: store i64 {{.+}}, ptr [[SHADOW]], align 8
+; CHECK: ret void
+
+define void @Store16(ptr %p, i128 %x) sanitize_memory {
+entry:
+  store i128 %x, ptr %p
+  ret void
+}
+
+; CHECK-LABEL: define {{[^@]+}}@Store16(
+; CHECK: [[META_PTR:%[a-z0-9_]+]] = alloca { ptr, ptr }, align 8
+; CHECK: call void @__msan_metadata_ptr_for_store_n(ptr [[META_PTR]], ptr %p, i64 16)
+; CHECK: [[META:%[a-z0-9_]+]] = load { ptr, ptr }, ptr [[META_PTR]], align 8
+; CHECK: [[SHADOW:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 0
+; CHECK: [[ORIGIN:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 1
+; CHECK: store i128 {{.+}}, ptr [[SHADOW]], align 8
+; CHECK: ret void
+
+define i8 @Load1(ptr %p) sanitize_memory {
+entry:
+  %0 = load i8, ptr %p
+  ret i8 %0
+}
+
+; CHECK-LABEL: define {{[^@]+}}@Load1(
+; CHECK: [[META_PTR:%[a-z0-9_]+]] = alloca { ptr, ptr }, align 8
+; CHECK: call void @__msan_metadata_ptr_for_load_1(ptr [[META_PTR]], ptr %p)
+; CHECK: [[META:%[a-z0-9_]+]] = load { ptr, ptr }, ptr [[META_PTR]], align 8
+; CHECK: [[SHADOW:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 0
+; CHECK: [[ORIGIN:%[a-z0-9_]+]] = extractvalue { ptr, ptr } [[META]], 1
+; CHECK: [[SHADOW_VAL:%[a-z0-9_]+]] = load i8, ptr [[SHADOW]], align 1
+; CHECK: [[ORIGIN_VAL:%[a-z0-9_]+]] = load i32, ptr [[ORIGIN]], align 4
+; CHECK: store i8 [[SHADOW_VAL]], ptr %retval_shadow, align 8
+; CHECK: store i32 [[ORIGIN_VAL]], ptr %retval_origin, align 4
+; CHECK: ret i8 {{.+}}
+
+define i16 @Load2(ptr %p) sanitize_memory {
+entry:
+  %0 = load i16, ptr %p
+  ret i16 %0
+}
+
+; CHECK-LABEL: define {{[^@]+}}@Load2(
+; CHECK: [[META_PTR:%[a-z0-9_]+]] = alloca { ptr, ptr }, align 8
+; CHECK: call void @__msan_metadata_ptr_for_load_2(ptr [[META_PTR]], ptr %p)
+; CHECK: [[META:%[a-

[PATCH] D148908: [Driver][NFC] Simplify code.

[Ben Shi via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148908

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

[PATCH] D148914: [Sema][NFC] add check before using `BuildExpressionFromIntegralTemplateArgument`

[Congcong Cai via Phabricator via cfe-commits]

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

This patch wants to avoid Sema crash for inline friend decl like

  template  int foo(F1 X);
  template  struct A {
template  friend int foo(F1 X) { return A1; }
  };
  
  template struct A<1>;
  int a = foo(1.0);

But this case is still not fixed


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148914

Files:
  clang/lib/Sema/SemaTemplateInstantiate.cpp


Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1849,7 +1849,7 @@
 assert(!paramType->isDependentType() && "param type still dependent");
 result = SemaRef.BuildExpressionFromDeclTemplateArgument(arg, paramType, 
loc);
 refParam = paramType->isReferenceType();
-  } else {
+  } else if (arg.getKind() == TemplateArgument::Integral) {
 result = SemaRef.BuildExpressionFromIntegralTemplateArgument(arg, loc);
 assert(result.isInvalid() ||
SemaRef.Context.hasSameType(result.get()->getType(),


Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1849,7 +1849,7 @@
 assert(!paramType->isDependentType() && "param type still dependent");
 result = SemaRef.BuildExpressionFromDeclTemplateArgument(arg, paramType, loc);
 refParam = paramType->isReferenceType();
-  } else {
+  } else if (arg.getKind() == TemplateArgument::Integral) {
 result = SemaRef.BuildExpressionFromIntegralTemplateArgument(arg, loc);
 assert(result.isInvalid() ||
SemaRef.Context.hasSameType(result.get()->getType(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148783: [clangd] Add support TextDocumentEdit.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 515671.
hokein marked 2 inline comments as done.
hokein added a comment.

address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148783

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/Diagnostics.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/test/fixits-codeaction-documentchanges.test

Index: clang-tools-extra/clangd/test/fixits-codeaction-documentchanges.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/fixits-codeaction-documentchanges.test
@@ -0,0 +1,145 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"codeAction":{"codeActionLiteralSupport":{}}},"workspace":{"workspaceEdit":{"documentChanges":true}}},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"int main(int i, char **a) { if (i = 2) {}}"}}}
+#  CHECK:"method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:"diagnostics": [
+# CHECK-NEXT:  {
+# CHECK-NEXT:"code": "-Wparentheses",
+# CHECK-NEXT:"message": "Using the result of an assignment as a condition without parentheses (fixes available)",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 37,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 32,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"severity": 2,
+# CHECK-NEXT:"source": "clang"
+# CHECK-NEXT:  }
+# CHECK-NEXT:],
+# CHECK-NEXT:"uri": "file://{{.*}}/foo.c",
+# CHECK-NEXT:"version": 1
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":0,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"severity":2,"message":"Using the result of an assignment as a condition without parentheses (fixes available)", "code": "-Wparentheses", "source": "clang"}]}}}
+#  CHECK:  "id": 2,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "diagnostics": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "code": "-Wparentheses",
+# CHECK-NEXT:  "message": "Using the result of an assignment as a condition without parentheses (fixes available)",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 37,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 32,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "severity": 2,
+# CHECK-NEXT:  "source": "clang"
+# CHECK-NEXT:}
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  "edit": {
+# CHECK-NEXT:"documentChanges": [
+# CHECK-NEXT:  {
+# CHECK-NEXT:"edits": [
+# CHECK-NEXT:  {
+# CHECK-NEXT:"newText": "(",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 32,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 32,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  {
+# CHECK-NEXT:"newText": ")",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 37,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 37,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+# CHECK-NEXT:],
+# CHECK-NEXT:"textDocument": {
+# CHECK-NEXT:  "uri": "file:///clangd-test/foo.c",
+# CHECK-NEXT:  "version": 1
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+# CHECK-NEXT:]
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "kin

[PATCH] D148783: [clangd] Add support TextDocumentEdit.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

> can you also have a version of the 
> clang-tools-extra/clangd/test/fixits-command.test with documentChanges 
> support? it's unlikely to have clients in that configuration but i believe 
> the deserialization issue i mentioned above would be discoverable by such a 
> test.

I'm happy to add a test for that, but I'm not sure the deserialization issue 
you mentioned in the comment, is the one to use `mapOptional`?




Comment at: clang-tools-extra/clangd/ClangdLSPServer.h:201
   void bindMethods(LSPBinder &, const ClientCapabilities &Caps);
-  std::vector getFixes(StringRef File, const clangd::Diagnostic &D);
+  std::pair, std::vector>
+  getFixes(StringRef File, const clangd::Diagnostic &D);

kadircet wrote:
> instead of a pair maybe a:
> ```
> struct VersionedFixes {
>   std::optional DocumentVersion;
>   std::vector Fixes;
> };
> ```
we don't need this struct now, as we switch to store the `CodeAction`.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.h:236
   std::mutex FixItsMutex;
   typedef std::map, LSPDiagnosticCompare>
   DiagnosticToReplacementMap;

kadircet wrote:
> can we instead have a:
> ```
> std::map, LSPDiagnosticCompare> 
> Fixes;
> ```
> 
> We'll make sure we store code actions with necessary version information.
> That way `FixItsMap` can stay the same, and rest of the code will look more 
> uniform; we'll do the conversion from Fixes to CodeActions during 
> `onDiagnosticsReady`
good idea! 



Comment at: clang-tools-extra/clangd/Protocol.cpp:735
   llvm::json::ObjectMapper O(Params, P);
-  return O && O.map("changes", R.changes);
+  return O && O.map("changes", R.changes) && O.map("documentChanges", 
R.documentChanges);
 }

kadircet wrote:
> we actually want `O.mapOptional` for both "changes" and "documentChanges".
I think `map` is a better fit here, it has a specific version of 
`std::optional`, see 
https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Support/JSON.h#L852.

`mapOptional`  doesn't set the field when missing the key in json object,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148783

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

[clang] 22a408a - [Pipelines] Don't explicitly require ORE

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2023-04-21T13:22:04+02:00
New Revision: 22a408ae513c753f0b6a7a9b55fedf8fd735f1b1

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

LOG: [Pipelines] Don't explicitly require ORE

LICM does not use ORE from the pass manager, it constructs its
own instance. As such, explicitly requiring the analysis in the
pipeline is unnecessary.

Added: 


Modified: 
clang/test/CodeGen/thinlto-distributed-newpm.ll
llvm/lib/Passes/PassBuilderPipelines.cpp
llvm/test/Other/new-pm-defaults.ll
llvm/test/Other/new-pm-thinlto-postlink-defaults.ll
llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
llvm/test/Other/new-pm-thinlto-prelink-defaults.ll
llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll

Removed: 




diff  --git a/clang/test/CodeGen/thinlto-distributed-newpm.ll 
b/clang/test/CodeGen/thinlto-distributed-newpm.ll
index 071e076a6a0d3..57e2011f55aed 100644
--- a/clang/test/CodeGen/thinlto-distributed-newpm.ll
+++ b/clang/test/CodeGen/thinlto-distributed-newpm.ll
@@ -51,7 +51,6 @@
 ; CHECK-O: Running pass: TailCallElimPass on main
 ; CHECK-O: Running pass: SimplifyCFGPass on main
 ; CHECK-O: Running pass: ReassociatePass on main
-; CHECK-O: Running pass: 
RequireAnalysisPass<{{.*}}OptimizationRemarkEmitterAnalysis
 ; CHECK-O: Running pass: LoopSimplifyPass on main
 ; CHECK-O: Running pass: LCSSAPass on main
 ; CHECK-O: Running pass: SimplifyCFGPass on main
@@ -96,7 +95,6 @@
 ; CHECK-O: Running pass: LoopUnrollPass on main
 ; CHECK-O: Running pass: WarnMissedTransformationsPass on main
 ; CHECK-O: Running pass: InstCombinePass on main
-; CHECK-O: Running pass: 
RequireAnalysisPass<{{.*}}OptimizationRemarkEmitterAnalysis
 ; CHECK-O: Running pass: LoopSimplifyPass on main
 ; CHECK-O: Running pass: LCSSAPass on main
 ; CHECK-O: Running pass: LICMPass on b

diff  --git a/llvm/lib/Passes/PassBuilderPipelines.cpp 
b/llvm/lib/Passes/PassBuilderPipelines.cpp
index da72bedf70582..4fa7326b24719 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -20,7 +20,6 @@
 #include "llvm/Analysis/CGSCCPassManager.h"
 #include "llvm/Analysis/GlobalsModRef.h"
 #include "llvm/Analysis/InlineAdvisor.h"
-#include "llvm/Analysis/OptimizationRemarkEmitter.h"
 #include "llvm/Analysis/ProfileSummaryInfo.h"
 #include "llvm/Analysis/ScopedNoAliasAA.h"
 #include "llvm/Analysis/TypeBasedAliasAnalysis.h"
@@ -459,10 +458,6 @@ 
PassBuilder::buildO1FunctionSimplificationPipeline(OptimizationLevel Level,
 
   invokeLoopOptimizerEndEPCallbacks(LPM2, Level);
 
-  // We provide the opt remark emitter pass for LICM to use. We only need to do
-  // this once as it is immutable.
-  FPM.addPass(
-  RequireAnalysisPass());
   FPM.addPass(createFunctionToLoopPassAdaptor(std::move(LPM1),
   /*UseMemorySSA=*/true,
   /*UseBlockFrequencyInfo=*/true));
@@ -642,10 +637,6 @@ 
PassBuilder::buildFunctionSimplificationPipeline(OptimizationLevel Level,
 
   invokeLoopOptimizerEndEPCallbacks(LPM2, Level);
 
-  // We provide the opt remark emitter pass for LICM to use. We only need to do
-  // this once as it is immutable.
-  FPM.addPass(
-  RequireAnalysisPass());
   FPM.addPass(createFunctionToLoopPassAdaptor(std::move(LPM1),
   /*UseMemorySSA=*/true,
   /*UseBlockFrequencyInfo=*/true));
@@ -1186,8 +1177,6 @@ void PassBuilder::addVectorPasses(OptimizationLevel Level,
  /*AllowSpeculation=*/true));
 LPM.addPass(SimpleLoopUnswitchPass(/* NonTrivial */ Level ==
OptimizationLevel::O3));
-ExtraPasses.addPass(
-RequireAnalysisPass());
 ExtraPasses.addPass(
 createFunctionToLoopPassAdaptor(std::move(LPM), /*UseMemorySSA=*/true,
 /*UseBlockFrequencyInfo=*/true));
@@ -1255,8 +1244,6 @@ void PassBuilder::addVectorPasses(OptimizationLevel Level,
 // the CFG mess this may created if allowed to modify CFG, so forbid that.
 FPM.addPass(SROAPass(SROAOptions::PreserveCFG));
 FPM.addPass(InstCombinePass());
-FPM.addPass(
-RequireAnalysisPass());
 FPM.addPass(createFunctionToLoopPassAdaptor(
 LICMPass(PTO.LicmMssaOptCap, PTO.LicmMssaNoAccForPromotionCap,
  /*AllowSpeculation=*/true),

diff  --git a/llvm/test/Other/new-pm-defaults.ll 
b/llvm/test/Other/new-pm-defaults.ll
index 0cf856852f20b..c4e32e6878d26 100644
--- a/llvm/test/Other/new-pm-defaults.ll
+++ b/llvm/test/Other/

[clang] 7c00219 - [clang] trigger -Wcast-qual on functional casts

[Aaron Ballman via cfe-commits]

Author: Jorge Pinto Sousa
Date: 2023-04-21T07:45:05-04:00
New Revision: 7c0021923503a9a5fe1ba1f0b778b5b83c42aa43

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

LOG: [clang] trigger -Wcast-qual on functional casts

-Wcast-qual does not trigger on the following code in Clang, but does
in GCC.

const auto i = 42;
using T = int*;
auto p = T(&i);

The expected behavior is that a functional cast should trigger
the warning the same as the equivalent C cast because
the meaning is the same, and nothing about the functional cast
makes it easier to recognize that a const_cast is occurring.

Fixes https://github.com/llvm/llvm-project/issues/62083
Differential Revision: https://reviews.llvm.org/D148276

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaCast.cpp
clang/test/SemaCXX/warn-cast-qual.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7beae03247796..9cc3936745ac6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -231,6 +231,8 @@ Improvements to Clang's diagnostics
 - ``-Wformat`` now recognizes ``%lb`` for the ``printf``/``scanf`` family of
   functions.
   (`#62247: `_).
+- ``-Wcast-qual`` now triggers on function-style casts.
+  (`#62083 `_)
 
 Bug Fixes in This Version
 -

diff  --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index cd71288476345..be30d36a4f9a4 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -3332,6 +3332,9 @@ ExprResult 
Sema::BuildCXXFunctionalCastExpr(TypeSourceInfo *CastTypeInfo,
   if (auto *ConstructExpr = dyn_cast(SubExpr))
 ConstructExpr->setParenOrBraceRange(SourceRange(LPLoc, RPLoc));
 
+  // -Wcast-qual
+  DiagnoseCastQual(Op.Self, Op.SrcExpr, Op.DestType);
+
   return Op.complete(CXXFunctionalCastExpr::Create(
   Context, Op.ResultType, Op.ValueKind, CastTypeInfo, Op.Kind,
   Op.SrcExpr.get(), &Op.BasePath, CurFPFeatureOverrides(), LPLoc, RPLoc));

diff  --git a/clang/test/SemaCXX/warn-cast-qual.cpp 
b/clang/test/SemaCXX/warn-cast-qual.cpp
index d1f0cc73a63d4..4fe8de9ae59c8 100644
--- a/clang/test/SemaCXX/warn-cast-qual.cpp
+++ b/clang/test/SemaCXX/warn-cast-qual.cpp
@@ -34,6 +34,17 @@ void foo_0() {
   const int &a6 = (int &)((int &)a);   // expected-warning {{cast from 
'const int' to 'int &' drops const qualifier}}
   const int &a7 = (int &)((const int &)a); // expected-warning {{cast from 
'const int' to 'int &' drops const qualifier}}
   const int &a8 = (const int &)((int &)a); // expected-warning {{cast from 
'const int' to 'int &' drops const qualifier}}
+
+  using T = int&;
+  using T2 = const int&;
+  const int &a11 =T2(a);  // no warning
+  int a22 = T(a); // expected-warning {{cast from 'const int' to 
'int &' drops const qualifier}}
+  const int &a33 = T(a);  // expected-warning {{cast from 'const int' to 
'int &' drops const qualifier}}
+  int &a44 = T(T2(a));// expected-warning {{cast from 'const int' to 
'int &' drops const qualifier}}
+  int &a55 = T(T(a)); // expected-warning {{cast from 'const int' to 
'int &' drops const qualifier}}
+  const int &a66 = T(T(a));   // expected-warning {{cast from 'const int' to 
'int &' drops const qualifier}}
+  int &a77 = T(T2(a));// expected-warning {{cast from 'const int' to 
'int &' drops const qualifier}}
+  const int &a88 = T2(T(a));  // expected-warning {{cast from 'const int' to 
'int &' drops const qualifier}}
 }
 
 void foo_1() {
@@ -49,6 +60,17 @@ void foo_1() {
   volatile int &a6 = (int &)((int &)a);  // expected-warning {{cast 
from 'volatile int' to 'int &' drops volatile qualifier}}
   volatile int &a7 = (int &)((volatile int &)a); // expected-warning {{cast 
from 'volatile int' to 'int &' drops volatile qualifier}}
   volatile int &a8 = (volatile int &)((int &)a); // expected-warning {{cast 
from 'volatile int' to 'int &' drops volatile qualifier}}
+
+  using T = int&;
+  using T2 = volatile int&;
+  volatile int &a11 =T2(a); // no warning
+  int a22 = T(a);   // expected-warning {{cast from 'volatile int' 
to 'int &' drops volatile qualifier}}
+  volatile int &a33 = T(a); // expected-warning {{cast from 'volatile int' 
to 'int &' drops volatile qualifier}}
+  int &a44 = T(T2(a));  // expected-warning {{cast from 'volatile int' 
to 'int &' drops volatile qualifier}}
+  int &a55 = T(T(a));   // expected-warning {{cast from 'volatile int' 
to 'int &' drops volatile qualifier}}
+  volatile int &a66 = T(T(a));  // expected-warning {{cast from 'volatile int' 
to 'int &' drops volatile qualifier}}
+  int &a77 = T(T

[PATCH] D148276: [clang] trigger -Wcast-qual on functional casts

[Aaron Ballman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7c0021923503: [clang] trigger -Wcast-qual on functional 
casts (authored by sousajo, committed by aaron.ballman).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D148276?vs=513583&id=515680#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148276

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaCast.cpp
  clang/test/SemaCXX/warn-cast-qual.cpp

Index: clang/test/SemaCXX/warn-cast-qual.cpp
===
--- clang/test/SemaCXX/warn-cast-qual.cpp
+++ clang/test/SemaCXX/warn-cast-qual.cpp
@@ -34,6 +34,17 @@
   const int &a6 = (int &)((int &)a);   // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
   const int &a7 = (int &)((const int &)a); // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
   const int &a8 = (const int &)((int &)a); // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+
+  using T = int&;
+  using T2 = const int&;
+  const int &a11 =T2(a);  // no warning
+  int a22 = T(a); // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int &a33 = T(a);  // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  int &a44 = T(T2(a));// expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  int &a55 = T(T(a)); // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int &a66 = T(T(a));   // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  int &a77 = T(T2(a));// expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
+  const int &a88 = T2(T(a));  // expected-warning {{cast from 'const int' to 'int &' drops const qualifier}}
 }
 
 void foo_1() {
@@ -49,6 +60,17 @@
   volatile int &a6 = (int &)((int &)a);  // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
   volatile int &a7 = (int &)((volatile int &)a); // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
   volatile int &a8 = (volatile int &)((int &)a); // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+
+  using T = int&;
+  using T2 = volatile int&;
+  volatile int &a11 =T2(a); // no warning
+  int a22 = T(a);   // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int &a33 = T(a); // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  int &a44 = T(T2(a));  // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  int &a55 = T(T(a));   // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int &a66 = T(T(a));  // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  int &a77 = T(T2(a));  // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
+  volatile int &a88 = T2(T(a)); // expected-warning {{cast from 'volatile int' to 'int &' drops volatile qualifier}}
 }
 
 void foo_2() {
@@ -64,6 +86,17 @@
   const volatile int &a6 = (int &)((int &)a);// expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
   const volatile int &a7 = (int &)((const volatile int &)a); // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
   const volatile int &a8 = (const volatile int &)((int &)a); // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+
+  using T = int&;
+  using T2 = const volatile int&;
+  const volatile int &a11 =T2(a);   // no warning
+  int a22 = T(a); // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int &a33 = T(a); // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  int &a44 = T(T2(a));// expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  int &a55 = T(T(a)); // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int &a66 = T(T(a));  // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int &a77 = T(T2(a)); // expected-warning {{cast from 'const volatile int' to 'int &' drops const and volatile qualifiers}}
+  const volatile int &a88 = T2(T(a)); // expected-warning {{cast from 'const v

[clang] 9e9e096 - [clang-format] Fix dropped 'else'.

[Manuel Klimek via cfe-commits]

Author: Manuel Klimek
Date: 2023-04-21T11:59:45Z
New Revision: 9e9e096ae95b9a41bf855cda01963a65313e9560

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

LOG: [clang-format] Fix dropped 'else'.

'else' was dropped accidentally in 398cddf6acec.

Patch by Jared Grubb.

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

Added: 


Modified: 
clang/lib/Format/UnwrappedLineFormatter.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineFormatter.cpp 
b/clang/lib/Format/UnwrappedLineFormatter.cpp
index b545fa02cefb..ee0de2c8e20d 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -511,10 +511,9 @@ class LineJoiner {
 ShouldMerge = !Style.BraceWrapping.AfterClass ||
   (NextLine.First->is(tok::r_brace) &&
!Style.BraceWrapping.SplitEmptyRecord);
-  }
-  if (TheLine->InPPDirective ||
-  !TheLine->First->isOneOf(tok::kw_class, tok::kw_enum,
-   tok::kw_struct)) {
+  } else if (TheLine->InPPDirective ||
+ !TheLine->First->isOneOf(tok::kw_class, tok::kw_enum,
+  tok::kw_struct)) {
 // Try to merge a block with left brace unwrapped that wasn't yet
 // covered.
 ShouldMerge = !Style.BraceWrapping.AfterFunction ||



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

[PATCH] D146310: [clang-format] Fix dropped 'else' in 398cddf6acec

[Manuel Klimek 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 rG9e9e096ae95b: [clang-format] Fix dropped 'else'. 
(authored by klimek).
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.
Herald added a comment.

NOTE: Clang-Format Team Automated Review Comment

It looks like your clang-format review does not contain any unit tests, please 
try to ensure all code changes have a unit test (unless this is an `NFC` or 
refactoring, adding documentation etc..)

Add your unit tests in `clang/unittests/Format` and you can build with `ninja 
FormatTests`.  We recommend using the `verifyFormat(xxx)` format of unit tests 
rather than `EXPECT_EQ` as this will ensure you change is tolerant to random 
whitespace changes (see FormatTest.cpp as an example)

For situations where your change is altering the TokenAnnotator.cpp which can 
happen if you are trying to improve the annotation phase to ensure we are 
correctly identifying the type of a token, please add a token annotator test in 
`TokenAnnotatorTest.cpp`


Changed prior to commit:
  https://reviews.llvm.org/D146310?vs=506133&id=515683#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146310

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -511,10 +511,9 @@
 ShouldMerge = !Style.BraceWrapping.AfterClass ||
   (NextLine.First->is(tok::r_brace) &&
!Style.BraceWrapping.SplitEmptyRecord);
-  }
-  if (TheLine->InPPDirective ||
-  !TheLine->First->isOneOf(tok::kw_class, tok::kw_enum,
-   tok::kw_struct)) {
+  } else if (TheLine->InPPDirective ||
+ !TheLine->First->isOneOf(tok::kw_class, tok::kw_enum,
+  tok::kw_struct)) {
 // Try to merge a block with left brace unwrapped that wasn't yet
 // covered.
 ShouldMerge = !Style.BraceWrapping.AfterFunction ||


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -511,10 +511,9 @@
 ShouldMerge = !Style.BraceWrapping.AfterClass ||
   (NextLine.First->is(tok::r_brace) &&
!Style.BraceWrapping.SplitEmptyRecord);
-  }
-  if (TheLine->InPPDirective ||
-  !TheLine->First->isOneOf(tok::kw_class, tok::kw_enum,
-   tok::kw_struct)) {
+  } else if (TheLine->InPPDirective ||
+ !TheLine->First->isOneOf(tok::kw_class, tok::kw_enum,
+  tok::kw_struct)) {
 // Try to merge a block with left brace unwrapped that wasn't yet
 // covered.
 ShouldMerge = !Style.BraceWrapping.AfterFunction ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146310: [clang-format] Fix dropped 'else' in 398cddf6acec

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

Submitted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146310

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

[PATCH] D147217: [OpenMP][OMPIRBuilder] OpenMPIRBuilder support for requires directive

[Sergio Afonso via Phabricator via cfe-commits]

skatrak updated this revision to Diff 515684.
skatrak added a comment.

Avoid creating registration function for the device.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147217

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -9,12 +9,14 @@
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
 #include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
 #include "llvm/IR/BasicBlock.h"
+#include "llvm/IR/Constants.h"
 #include "llvm/IR/DIBuilder.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/InstIterator.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
+#include "llvm/IR/Type.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/Passes/PassBuilder.h"
 #include "llvm/Support/Casting.h"
@@ -5731,7 +5733,8 @@
 
 TEST_F(OpenMPIRBuilderTest, OffloadEntriesInfoManager) {
   OpenMPIRBuilder OMPBuilder(*M);
-  OMPBuilder.setConfig(OpenMPIRBuilderConfig(true, false, false, false));
+  OMPBuilder.setConfig(
+  OpenMPIRBuilderConfig(true, false, false, false, false, false, false));
   OffloadEntriesInfoManager &InfoManager = OMPBuilder.OffloadInfoManager;
   TargetRegionEntryInfo EntryInfo("parent", 1, 2, 4, 0);
   InfoManager.initializeTargetRegionEntryInfo(EntryInfo, 0);
@@ -5746,4 +5749,44 @@
   GlobalValue::WeakAnyLinkage);
   EXPECT_TRUE(InfoManager.hasDeviceGlobalVarEntryInfo("gvar"));
 }
+
+TEST_F(OpenMPIRBuilderTest, CreateRegisterRequires) {
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+
+  OMPBuilder.setConfig(
+  OpenMPIRBuilderConfig(/*IsEmbedded=*/false,
+/*IsTargetCodegen=*/false,
+/*OpenMPOffloadMandatory=*/false,
+/*HasRequiresReverseOffload=*/true,
+/*HasRequiresUnifiedAddress=*/false,
+/*HasRequiresUnifiedSharedMemory=*/true,
+/*HasRequiresDynamicAllocators=*/false));
+
+  auto FName =
+  OMPBuilder.createPlatformSpecificName({"omp_offloading", "requires_reg"});
+  EXPECT_EQ(FName, ".omp_offloading.requires_reg");
+
+  Function *Fn = OMPBuilder.createRegisterRequires(FName);
+  EXPECT_NE(Fn, nullptr);
+  EXPECT_EQ(FName, Fn->getName());
+
+  EXPECT_EQ(Fn->getSection(), ".text.startup");
+  EXPECT_TRUE(Fn->hasInternalLinkage());
+  EXPECT_TRUE(Fn->hasFnAttribute(Attribute::NoInline));
+  EXPECT_TRUE(Fn->hasFnAttribute(Attribute::NoUnwind));
+  EXPECT_EQ(Fn->size(), 1u);
+
+  BasicBlock *Entry = &Fn->getEntryBlock();
+  EXPECT_FALSE(Entry->empty());
+  EXPECT_EQ(Fn->getReturnType()->getTypeID(), Type::VoidTyID);
+
+  CallInst *Call = &cast(*Entry->begin());
+  EXPECT_EQ(Call->getCalledFunction()->getName(), "__tgt_register_requires");
+  EXPECT_EQ(Call->getNumOperands(), 2u);
+
+  Value *Flags = Call->getArgOperand(0);
+  EXPECT_EQ(cast(Flags)->getSExtValue(),
+OMPBuilder.Config.getRequiresFlags());
+}
 } // namespace
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -21,10 +21,12 @@
 #include "llvm/Analysis/OptimizationRemarkEmitter.h"
 #include "llvm/Analysis/ScalarEvolution.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
+#include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DerivedTypes.h"
+#include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/MDBuilder.h"
@@ -328,6 +330,104 @@
   return splitBB(Builder, CreateBranch, Old->getName() + Suffix);
 }
 
+//===--===//
+// OpenMPIRBuilderConfig
+//===--===//
+
+namespace {
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+/// Values for bit flags for marking which requires clauses have been used.
+enum OpenMPOffloadingRequiresDirFlags {
+  /// flag undefined.
+  OMP_REQ_UNDEFINED = 0x000,
+  /// no requires directive present.
+  OMP_REQ_NONE = 0x001,
+  /// reverse_offload clause.
+  OMP_REQ_REVERSE_OFFLOAD = 0x002,
+  /// unified_address clause.
+  OMP_REQ_UNIFIED_ADDRESS = 0x004,
+  /// unified_shared_memory clause.
+  OMP_REQ_UNIFIED_SHARED_MEMORY = 0x008,
+  /// dynamic_allocators clause.
+  OMP_REQ_DYNAMIC_ALLOCATORS = 0x010,
+  LLVM_MARK_AS_BITMASK_ENUM(/*LargestV

[clang] 5ea1580 - Revert "Reland [Modules] Remove unnecessary check when generating name lookup table in ASTWriter"

[Ilya Biryukov via cfe-commits]

Author: Ilya Biryukov
Date: 2023-04-21T14:08:18+02:00
New Revision: 5ea158077ec9ca50857ede5cbb0b27c61663fd55

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

LOG: Revert "Reland [Modules] Remove unnecessary check when generating name 
lookup table in ASTWriter"

This reverts commit 67b298f6d82e0b4bb648ac0dabe895e816a77ef1.

We got linker errors with undefined symbols during a compiler release
and tracked it down to this change. I am in the process of understanding
what is happening and getting a reproducer.

Sorry for reverting this again.

I will reopen #61065 until we fix this.

Added: 


Modified: 
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Serialization/ASTWriter.cpp
clang/test/Modules/pr61065.cppm

Removed: 




diff  --git a/clang/include/clang/Serialization/ASTWriter.h 
b/clang/include/clang/Serialization/ASTWriter.h
index d31fa38b9382..09ee1744e894 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -514,6 +514,7 @@ class ASTWriter : public ASTDeserializationListener,
   void WriteTypeAbbrevs();
   void WriteType(QualType T);
 
+  bool isLookupResultExternal(StoredDeclsList &Result, DeclContext *DC);
   bool isLookupResultEntirelyExternal(StoredDeclsList &Result, DeclContext 
*DC);
 
   void GenerateNameLookupTable(const DeclContext *DC,

diff  --git a/clang/lib/Serialization/ASTWriter.cpp 
b/clang/lib/Serialization/ASTWriter.cpp
index 029f30416d61..245304254811 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -3849,6 +3849,12 @@ class ASTDeclContextNameLookupTrait {
 
 } // namespace
 
+bool ASTWriter::isLookupResultExternal(StoredDeclsList &Result,
+   DeclContext *DC) {
+  return Result.hasExternalDecls() &&
+ DC->hasNeedToReconcileExternalVisibleStorage();
+}
+
 bool ASTWriter::isLookupResultEntirelyExternal(StoredDeclsList &Result,
DeclContext *DC) {
   for (auto *D : Result.getLookupResult())
@@ -3879,17 +3885,20 @@ ASTWriter::GenerateNameLookupTable(const DeclContext 
*ConstDC,
   // order.
   SmallVector Names;
 
-  // We also track whether we're writing out the DeclarationNameKey for
-  // constructors or conversion functions.
-  bool IncludeConstructorNames = false;
-  bool IncludeConversionNames = false;
+  // We also build up small sets of the constructor and conversion function
+  // names which are visible.
+  llvm::SmallPtrSet ConstructorNameSet, ConversionNameSet;
+
+  for (auto &Lookup : *DC->buildLookup()) {
+auto &Name = Lookup.first;
+auto &Result = Lookup.second;
 
-  for (auto &[Name, Result] : *DC->buildLookup()) {
 // If there are no local declarations in our lookup result, we
 // don't need to write an entry for the name at all. If we can't
 // write out a lookup set without performing more deserialization,
 // just skip this entry.
-if (isLookupResultEntirelyExternal(Result, DC))
+if (isLookupResultExternal(Result, DC) &&
+isLookupResultEntirelyExternal(Result, DC))
   continue;
 
 // We also skip empty results. If any of the results could be external and
@@ -3906,20 +3915,24 @@ ASTWriter::GenerateNameLookupTable(const DeclContext 
*ConstDC,
 // results for them. This in almost certainly a bug in Clang's name lookup,
 // but that is likely to be hard or impossible to fix and so we tolerate it
 // here by omitting lookups with empty results.
-if (Result.getLookupResult().empty())
+if (Lookup.second.getLookupResult().empty())
   continue;
 
-switch (Name.getNameKind()) {
+switch (Lookup.first.getNameKind()) {
 default:
-  Names.push_back(Name);
+  Names.push_back(Lookup.first);
   break;
 
 case DeclarationName::CXXConstructorName:
-  IncludeConstructorNames = true;
+  assert(isa(DC) &&
+ "Cannot have a constructor name outside of a class!");
+  ConstructorNameSet.insert(Name);
   break;
 
 case DeclarationName::CXXConversionFunctionName:
-  IncludeConversionNames = true;
+  assert(isa(DC) &&
+ "Cannot have a conversion function name outside of a class!");
+  ConversionNameSet.insert(Name);
   break;
 }
   }
@@ -3927,34 +3940,55 @@ ASTWriter::GenerateNameLookupTable(const DeclContext 
*ConstDC,
   // Sort the names into a stable order.
   llvm::sort(Names);
 
-  if (IncludeConstructorNames || IncludeConversionNames) {
+  if (auto *D = dyn_cast(DC)) {
 // We need to establish an ordering of constructor and conversion function
-// names, and they don't have an intrinsic ordering. We also need to write
-// out all constructor and conversion function result

[PATCH] D148799: [clang] Return `StringRef` from `TargetInfo::getClobbers()`

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

> Context not available.

Please update the diff with context, 
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface.

I'm not sure what the benefit of this change is. While I did say less raw 
pointers = better, perhaps I'd make an exception for `const char *` for 
constant strings. In this case, no one is manipulating the string until (I 
assume) after it's converted to std::string, so we're not removing risky 
accesses in that way. If the API were returning a `const char *` that we then 
did a bunch of find first, split at, strlen, etc on, then this would make more 
sense.

https://llvm.org/docs/ProgrammersManual.html#passing-strings-the-stringref-and-twine-classes

Also this says you can implicitly construct from a string literal, so I am 
surprised you need the `llvm::StringLiteral`. Unless this is just a thing that 
asserts that it is in fact, a literal (I've not used this before myself).

Overall I applaud the effort but in this particular case it may not be needed.


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

https://reviews.llvm.org/D148799

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

[PATCH] D147844: Emit warning when implicit cast from int to bool happens in an conditional operator expression

[NagaChaitanya Vellanki via Phabricator via cfe-commits]

chaitanyav updated this revision to Diff 515687.
chaitanyav added a comment.

Fix indentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Analysis/uninit-vals.c
  clang/test/Sema/integer-overflow.c
  clang/test/Sema/parentheses.c
  clang/test/Sema/parentheses.cpp
  clang/test/SemaCXX/array-bounds.cpp
  clang/test/SemaCXX/integer-overflow.cpp
  libcxx/include/__chrono/duration.h
  libcxx/include/strstream
  libcxx/test/libcxx/algorithms/nth_element_stability.pass.cpp
  libcxx/test/libcxx/algorithms/partial_sort_stability.pass.cpp
  libcxx/test/libcxx/algorithms/sort_stability.pass.cpp
  libcxxabi/src/cxa_personality.cpp

Index: libcxxabi/src/cxa_personality.cpp
===
--- libcxxabi/src/cxa_personality.cpp
+++ libcxxabi/src/cxa_personality.cpp
@@ -718,9 +718,7 @@
 if (actionEntry == 0)
 {
 // Found a cleanup
-results.reason = actions & _UA_SEARCH_PHASE
- ? _URC_CONTINUE_UNWIND
- : _URC_HANDLER_FOUND;
+results.reason = (actions & _UA_SEARCH_PHASE) ? _URC_CONTINUE_UNWIND : _URC_HANDLER_FOUND;
 return;
 }
 // Convert 1-based byte offset into
@@ -832,9 +830,8 @@
 // End of action list. If this is phase 2 and we have found
 // a cleanup (ttypeIndex=0), return _URC_HANDLER_FOUND;
 // otherwise return _URC_CONTINUE_UNWIND.
-results.reason = hasCleanup && actions & _UA_CLEANUP_PHASE
- ? _URC_HANDLER_FOUND
- : _URC_CONTINUE_UNWIND;
+results.reason =
+(hasCleanup && (actions & _UA_CLEANUP_PHASE)) ? _URC_HANDLER_FOUND : _URC_CONTINUE_UNWIND;
 return;
 }
 // Go to next action
@@ -1243,10 +1240,9 @@
 {
 const __shim_type_info* excpType =
 static_cast(new_exception_header->exceptionType);
-adjustedPtr =
-__getExceptionClass(&new_exception_header->unwindHeader) == kOurDependentExceptionClass ?
-((__cxa_dependent_exception*)new_exception_header)->primaryException :
-new_exception_header + 1;
+adjustedPtr = (__getExceptionClass(&new_exception_header->unwindHeader) == kOurDependentExceptionClass)
+  ? ((__cxa_dependent_exception*)new_exception_header)->primaryException
+  : new_exception_header + 1;
 if (!exception_spec_can_catch(ttypeIndex, classInfo, ttypeEncoding,
   excpType, adjustedPtr,
   unwind_exception, base))
Index: libcxx/test/libcxx/algorithms/sort_stability.pass.cpp
===
--- libcxx/test/libcxx/algorithms/sort_stability.pass.cpp
+++ libcxx/test/libcxx/algorithms/sort_stability.pass.cpp
@@ -32,7 +32,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = kSize / 2 - i * (i % 2 ? -1 : 1);
+v[i].value = kSize / 2 - i * ((i % 2) ? -1 : 1);
   }
   std::less comp;
   std::__sort_dispatch(v.begin(), v.end(), comp);
@@ -44,7 +44,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = kSize / 2 - i * (i % 2 ? -1 : 1);
+v[i].value = kSize / 2 - i * ((i % 2) ? -1 : 1);
   }
   auto deterministic_v = deterministic();
   std::sort(v.begin(), v.end());
@@ -62,7 +62,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = kSize / 2 - i * (i % 2 ? -1 : 1);
+v[i].value = kSize / 2 - i * ((i % 2) ? -1 : 1);
   }
   auto snapshot_v = v;
   auto snapshot_custom_v = v;
Index: libcxx/test/libcxx/algorithms/partial_sort_stability.pass.cpp
===
--- libcxx/test/libcxx/algorithms/partial_sort_stability.pass.cpp
+++ libcxx/test/libcxx/algorithms/partial_sort_stability.pass.cpp
@@ -32,7 +32,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = (i % 2 ? 1 : kSize / 2 + i);
+v[i].value = ((i % 2) ? 1 : kSize / 2 + i);
   }
   auto comp = std::less();
   std::__partial_sort_impl(v.begin(), v.begin() + kSize / 2, v.end(), comp);
@@ -44,7 +44,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = (i % 2 ? 1 : kSize / 2 + i);
+v[i].value = ((i % 2) ? 1 : kSize / 2 + i);
   }
   auto deterministic_v = deterministic();
 

[PATCH] D139010: [clang][WebAssembly] Implement support for table types and builtins

[Paulo Matos via Phabricator via cfe-commits]

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



Comment at: clang/test/Sema/builtins-wasm.c:12-13
+  __builtin_wasm_table_size(table, table);// expected-error {{too 
many arguments to function call, expected 1, have 2}}
+  void *a = __builtin_wasm_table_size(table); // expected-error 
{{incompatible integer to pointer conversion initializing 'void *' with an 
expression of type 'int'}}
+  __externref_t b = __builtin_wasm_table_size(table); // expected-error 
{{initializing '__externref_t' with an expression of incompatible type 'int'}}
+  int c = __builtin_wasm_table_size(table);

aaron.ballman wrote:
> Instead of relying on assignment diagnostics to test the return type of the 
> call, why not use the language? (Same can be used for the other, similar 
> tests.)
> ```
> #define EXPR_HAS_TYPE(expr, type) _Generic((expr), type : 1, default : 0)
> 
> _Static_assert(EXPR_HAS_TYPE(__builtin_wasm_table_size(table), int), "");
> ```
OK - I will apply those changes. I hadn't seen this way of doing this before. 
Thanks.



Comment at: clang/test/Sema/builtins-wasm.c:21
+  __builtin_wasm_table_grow(ref, ref, size); // expected-error 
{{1st argument must be a WebAssembly table}}
+  __builtin_wasm_table_grow(table, table, size); // expected-error 
{{2nd argument must match the element type of the WebAssembly table in the 1st 
argument}}
+  __builtin_wasm_table_grow(table, ref, table);  // expected-error 
{{3rd argument must be an integer}}

aaron.ballman wrote:
> I can't make sense of this diagnostic -- what is the element type of the web 
> assembly table in the 1st argument? Why is the second argument needed at all 
> if it needs to be derived from the type of the first argument and these 
> things don't represent values (due to being zero-sized)?
The element type needs to be a reference type. Either an externref or a 
funcref. So, if we have a table of externref, the second element needs to have 
that type because it's the element we are growing the table with. The element 
is needed because it's what we'll use to fill the table with.

Well... they do represent values, just not values that exist on webassembly 
module. They are host values. They are for example javascript strings, or 
numbers, or objects of any kind. They are just opaque in the webassembly module 
but if retrieved by the javascript side they are just normal sized objects.



Comment at: clang/test/Sema/builtins-wasm.c:32
+  __builtin_wasm_table_fill(table, table, ref, nelem);   // 
expected-error {{2nd argument must be an integer}}
+  __builtin_wasm_table_fill(table, index, index, ref);   // 
expected-error {{3rd argument must match the element type of the WebAssembly 
table in the 1st argument}}
+  __builtin_wasm_table_fill(table, index, ref, table);   // 
expected-error {{4th argument must be an integer}}

aaron.ballman wrote:
> Similar question here about the third argument.
Exactly the same story as above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139010

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

[PATCH] D146987: [Assignment Tracking] Enable by default

[Jeremy Morse via Phabricator via cfe-commits]

jmorse added a comment.

/me grumbles about all the world being a VAX,

@mstorsjo I can't replicate the crash, but can replicate the valgrind 
jump-on-uninitialized-value with a small reproducer [0] that doesn't feature 
any debug-info, using `opt --passes=early-cse reduced.ll`. The trace I've 
reduced around:

  ==609193== Conditional jump or move depends on uninitialised value(s)
  ==609193==at 0x5653F8B: simplifyICmpInst(unsigned int, llvm::Value*, 
llvm::Value*, llvm::SimplifyQuery const&, unsigned int) (in 
/fast/fs/llvm-main/build/bin/opt)
  ==609193==by 0x5654F22: simplifyICmpInst(unsigned int, llvm::Value*, 
llvm::Value*, llvm::SimplifyQuery const&, unsigned int) (in 
/fast/fs/llvm-main/build/bin/opt)
  ==609193==by 0x566107B: 
simplifyInstructionWithOperands(llvm::Instruction*, 
llvm::ArrayRef, llvm::SimplifyQuery const&, unsigned int) (in 
/fast/fs/llvm-main/build/bin/opt)
  ==609193==by 0x566181E: llvm::simplifyInstruction(llvm::Instruction*, 
llvm::SimplifyQuery const&) (in /fast/fs/llvm-main/build/bin/opt)

I've been building rG61967bbc7d4e9 
 using 
ubuntu's packaged clang-9. I can't be completely certain, but seemingly a user 
of PatternMatch::BinaryOp_match doesn't check that the pattern matched before 
examining the pattern? It doesn't reproduce in a stage2 RelWithDebInfo build, 
which is highly suspicious and feels like it's my environment. Could you 
confirm it's definitely assignment-tracking at fault by using `-Xclang 
-fexperimental-assignment-tracking=forced` to enable and `-Xclang 
-fexperimental-assignment-tracking=disabled` to disable, which should control 
the behaviour if it's AT at fault.

@dmgreen Aaahh, yes, we hadn't considered SVE at all. I think there's a natural 
solution, which is to treat such stores as indicating a variable is 
memory-homed at that point, although determining the size could be troublesome. 
I'll leave that to @Orlando .

@srj I'm unable to reproduce that assertion on linux with rG61967bbc7d4e9 
, however 
it looks like that assertion doesn't expect to have two identical objects 
compared with each other. Some light googling suggests that it's permissible 
for std::sort to do that, and some people complain about it, so I suppose in 
some environments that assertion is ill formed. We should be able to fix that 
with a small refinement.

Many thanks for reporting the problems all.

[0] https://gist.github.com/jmorse/55da51f56d9c756fa77b42e705bdc674


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146987

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

[PATCH] D148918: [Test] Regenerate checks using update_test_checks.py

[Philipp via Phabricator via cfe-commits]

phst updated this revision to Diff 515695.
phst added a comment.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

- Check for a ‘buffer’ type instead of ‘buffer-live’.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148918

Files:
  clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el
  llvm/test/Transforms/Util/flattencfg.ll

Index: llvm/test/Transforms/Util/flattencfg.ll
===
--- llvm/test/Transforms/Util/flattencfg.ll
+++ llvm/test/Transforms/Util/flattencfg.ll
@@ -1,11 +1,27 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
 ; RUN: opt -passes=flattencfg -S < %s | FileCheck %s
 
 
 ; This test checks whether the pass completes without a crash.
 ; The code is not transformed in any way
-;
-; CHECK-LABEL: @test_not_crash
 define void @test_not_crash(i32 %in_a) #0 {
+; CHECK-LABEL: define void @test_not_crash
+; CHECK-SAME: (i32 [[IN_A:%.*]]) {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:[[CMP0:%.*]] = icmp eq i32 [[IN_A]], -1
+; CHECK-NEXT:[[CMP1:%.*]] = icmp ne i32 [[IN_A]], 0
+; CHECK-NEXT:[[COND0:%.*]] = and i1 [[CMP0]], [[CMP1]]
+; CHECK-NEXT:br i1 [[COND0]], label [[B0:%.*]], label [[B1:%.*]]
+; CHECK:   b0:
+; CHECK-NEXT:[[CMP2:%.*]] = icmp eq i32 [[IN_A]], 0
+; CHECK-NEXT:[[CMP3:%.*]] = icmp ne i32 [[IN_A]], 1
+; CHECK-NEXT:[[COND1:%.*]] = or i1 [[CMP2]], [[CMP3]]
+; CHECK-NEXT:br i1 [[COND1]], label [[EXIT:%.*]], label [[B1]]
+; CHECK:   b1:
+; CHECK-NEXT:br label [[EXIT]]
+; CHECK:   exit:
+; CHECK-NEXT:ret void
+;
 entry:
   %cmp0 = icmp eq i32 %in_a, -1
   %cmp1 = icmp ne i32 %in_a, 0
@@ -25,17 +41,19 @@
   ret void
 }
 
-; CHECK-LABEL: @test_not_crash2
+define void @test_not_crash2(float %a, float %b) #0 {
+; CHECK-LABEL: define void @test_not_crash2
+; CHECK-SAME: (float [[A:%.*]], float [[B:%.*]]) {
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:%0 = fcmp ult float %a
-; CHECK-NEXT:%1 = fcmp ult float %b
-; CHECK-NEXT:[[COND:%[a-z0-9]+]] = and i1 %0, %1
-; CHECK-NEXT:br i1 [[COND]], label %bb4, label %bb3
+; CHECK-NEXT:[[TMP0:%.*]] = fcmp ult float [[A]], 1.00e+00
+; CHECK-NEXT:[[TMP1:%.*]] = fcmp ult float [[B]], 1.00e+00
+; CHECK-NEXT:[[TMP2:%.*]] = and i1 [[TMP0]], [[TMP1]]
+; CHECK-NEXT:br i1 [[TMP2]], label [[BB4:%.*]], label [[BB3:%.*]]
 ; CHECK:   bb3:
-; CHECK-NEXT:br label %bb4
+; CHECK-NEXT:br label [[BB4]]
 ; CHECK:   bb4:
 ; CHECK-NEXT:ret void
-define void @test_not_crash2(float %a, float %b) #0 {
+;
 entry:
   %0 = fcmp ult float %a, 1.00e+00
   br i1 %0, label %bb0, label %bb1
@@ -54,18 +72,20 @@
   br i1 %1, label %bb4, label %bb3
 }
 
-; CHECK-LABEL: @test_not_crash3
+define void @test_not_crash3(i32 %a) #0 {
+; CHECK-LABEL: define void @test_not_crash3
+; CHECK-SAME: (i32 [[A:%.*]]) {
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:%a_eq_0 = icmp eq i32 %a, 0
-; CHECK-NEXT:%a_eq_1 = icmp eq i32 %a, 1
-; CHECK-NEXT:[[COND:%[a-z0-9]+]] = or i1 %a_eq_0, %a_eq_1
-; CHECK-NEXT:br i1 [[COND]], label %bb2, label %bb3
+; CHECK-NEXT:[[A_EQ_0:%.*]] = icmp eq i32 [[A]], 0
+; CHECK-NEXT:[[A_EQ_1:%.*]] = icmp eq i32 [[A]], 1
+; CHECK-NEXT:[[TMP0:%.*]] = or i1 [[A_EQ_0]], [[A_EQ_1]]
+; CHECK-NEXT:br i1 [[TMP0]], label [[BB2:%.*]], label [[BB3:%.*]]
 ; CHECK:   bb2:
-; CHECK-NEXT:br label %bb3
+; CHECK-NEXT:br label [[BB3]]
 ; CHECK:   bb3:
-; CHECK-NEXT:%check_badref = phi i32 [ 17, %entry ], [ 11, %bb2 ]
+; CHECK-NEXT:[[CHECK_BADREF:%.*]] = phi i32 [ 17, [[ENTRY:%.*]] ], [ 11, [[BB2]] ]
 ; CHECK-NEXT:ret void
-define void @test_not_crash3(i32 %a) #0 {
+;
 entry:
   %a_eq_0 = icmp eq i32 %a, 0
   br i1 %a_eq_0, label %bb0, label %bb1
@@ -88,18 +108,20 @@
 
 @g = global i32 0, align 4
 
-; CHECK-LABEL: @test_then
+define void @test_then(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: define void @test_then
+; CHECK-SAME: (i32 [[X:%.*]], i32 [[Y:%.*]], i32 [[Z:%.*]]) {
 ; CHECK-NEXT:  entry.x:
-; CHECK-NEXT:%cmp.x = icmp ne i32 %x, 0
-; CHECK-NEXT:%cmp.y = icmp ne i32 %y, 0
-; CHECK-NEXT:[[COND:%[a-z0-9]+]] = or i1 %cmp.x, %cmp.y
-; CHECK-NEXT:br i1 [[COND]], label %if.then.y, label %exit
+; CHECK-NEXT:[[CMP_X:%.*]] = icmp ne i32 [[X]], 0
+; CHECK-NEXT:[[CMP_Y:%.*]] = icmp ne i32 [[Y]], 0
+; CHECK-NEXT:[[TMP0:%.*]] = or i1 [[CMP_X]], [[CMP_Y]]
+; CHECK-NEXT:br i1 [[TMP0]], label [[IF_THEN_Y:%.*]], label [[EXIT:%.*]]
 ; CHECK:   if.then.y:
-; CHECK-NEXT:store i32 %z, ptr @g, align 4
-; CHECK-NEXT:br label %exit
+; CHECK-NEXT:store i32 [[Z]], ptr @g, align 4
+; CHECK-NEXT:br label [[EXIT]]
 ; CHECK:   exit:
 ; CHECK-NEXT:ret void
-define void @test_then(i32 %x, i32 %y, i32 %z) {
+;
 entry.x:
   %cmp.x = icmp ne i32 %x, 0
   br i1 %cmp.x, label %if.th

[PATCH] D148919: [Clang][Sema] Fix invalid cast when validating SVE types within CheckVariableDeclarationType.

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm created this revision.
Herald added a subscriber: tschuett.
Herald added a project: All.
paulwalker-arm requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, jplehr, sstefan1.
Herald added a project: clang.

Fixes #62087


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148919

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaOpenMP/arm-sve-acle-types.cpp


Index: clang/test/SemaOpenMP/arm-sve-acle-types.cpp
===
--- /dev/null
+++ clang/test/SemaOpenMP/arm-sve-acle-types.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fopenmp -fsyntax-only -triple aarch64-arm-none-eabi 
-target-feature +sve -verify %s
+// expected-no-diagnostics
+
+__SVBool_t foo(int);
+
+void test() {
+#pragma omp parallel
+  {
+__SVBool_t pg = foo(1);
+  }
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -8705,7 +8705,7 @@
   }
 
   // Check that SVE types are only used in functions with SVE available.
-  if (T->isSVESizelessBuiltinType() && CurContext->isFunctionOrMethod()) {
+  if (T->isSVESizelessBuiltinType() && isa(CurContext)) {
 const FunctionDecl *FD = cast(CurContext);
 llvm::StringMap CallerFeatureMap;
 Context.getFunctionFeatureMap(CallerFeatureMap, FD);


Index: clang/test/SemaOpenMP/arm-sve-acle-types.cpp
===
--- /dev/null
+++ clang/test/SemaOpenMP/arm-sve-acle-types.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fopenmp -fsyntax-only -triple aarch64-arm-none-eabi -target-feature +sve -verify %s
+// expected-no-diagnostics
+
+__SVBool_t foo(int);
+
+void test() {
+#pragma omp parallel
+  {
+__SVBool_t pg = foo(1);
+  }
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -8705,7 +8705,7 @@
   }
 
   // Check that SVE types are only used in functions with SVE available.
-  if (T->isSVESizelessBuiltinType() && CurContext->isFunctionOrMethod()) {
+  if (T->isSVESizelessBuiltinType() && isa(CurContext)) {
 const FunctionDecl *FD = cast(CurContext);
 llvm::StringMap CallerFeatureMap;
 Context.getFunctionFeatureMap(CallerFeatureMap, FD);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148918: [Test] Regenerate checks using update_test_checks.py

[Philipp via Phabricator via cfe-commits]

phst updated this revision to Diff 515697.
phst added a comment.

Check for a ‘buffer’ type instead of ‘buffer-live’.

In Emacs 29, ‘buffer-live’ is no longer recognized as a type and generates a
compilation warning.  Every function that requires a live buffer already checks
whether the buffer is live, so we don’t need to check ourselves.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148918

Files:
  clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el


Index: clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el
===
--- clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el
+++ clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el
@@ -168,9 +168,9 @@
 only STDERR may be nil.  CALLBACK is called in the case of
 success; it is called with a single argument, STDOUT.  On
 failure, a buffer containing the error output is displayed."
-  (cl-check-type stdin buffer-live)
-  (cl-check-type stdout buffer-live)
-  (cl-check-type stderr (or null buffer-live))
+  (cl-check-type stdin buffer)
+  (cl-check-type stdout buffer)
+  (cl-check-type stderr (or null buffer))
   (cl-check-type callback function)
   (lambda (process event)
 (cl-check-type process process)
@@ -192,7 +192,7 @@
 
 (defun clang-include-fixer--replace-buffer (stdout)
   "Replace current buffer by content of STDOUT."
-  (cl-check-type stdout buffer-live)
+  (cl-check-type stdout buffer)
   (barf-if-buffer-read-only)
   (cond ((fboundp 'replace-buffer-contents) (replace-buffer-contents stdout))
 ((clang-include-fixer--insert-line stdout (current-buffer)))
@@ -207,8 +207,8 @@
 line missing from TO, insert that line into TO so that the buffer
 contents are equal and return non-nil.  Otherwise, do nothing and
 return nil.  Buffer restrictions are ignored."
-  (cl-check-type from buffer-live)
-  (cl-check-type to buffer-live)
+  (cl-check-type from buffer)
+  (cl-check-type to buffer)
   (with-current-buffer from
 (save-excursion
   (save-restriction


Index: clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el
===
--- clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el
+++ clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el
@@ -168,9 +168,9 @@
 only STDERR may be nil.  CALLBACK is called in the case of
 success; it is called with a single argument, STDOUT.  On
 failure, a buffer containing the error output is displayed."
-  (cl-check-type stdin buffer-live)
-  (cl-check-type stdout buffer-live)
-  (cl-check-type stderr (or null buffer-live))
+  (cl-check-type stdin buffer)
+  (cl-check-type stdout buffer)
+  (cl-check-type stderr (or null buffer))
   (cl-check-type callback function)
   (lambda (process event)
 (cl-check-type process process)
@@ -192,7 +192,7 @@
 
 (defun clang-include-fixer--replace-buffer (stdout)
   "Replace current buffer by content of STDOUT."
-  (cl-check-type stdout buffer-live)
+  (cl-check-type stdout buffer)
   (barf-if-buffer-read-only)
   (cond ((fboundp 'replace-buffer-contents) (replace-buffer-contents stdout))
 ((clang-include-fixer--insert-line stdout (current-buffer)))
@@ -207,8 +207,8 @@
 line missing from TO, insert that line into TO so that the buffer
 contents are equal and return non-nil.  Otherwise, do nothing and
 return nil.  Buffer restrictions are ignored."
-  (cl-check-type from buffer-live)
-  (cl-check-type to buffer-live)
+  (cl-check-type from buffer)
+  (cl-check-type to buffer)
   (with-current-buffer from
 (save-excursion
   (save-restriction
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148919: [Clang][Sema] Fix invalid cast when validating SVE types within CheckVariableDeclarationType.

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added reviewers: dmgreen, sdesmalen.
paulwalker-arm added a comment.

This is not my area so I don't know if replacing `isFunctionOrMethod()` rather 
than just extending the if clause is a bad idea, but all the tests still pass 
including the new one that would trigger an assert prior to this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148919

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

[PATCH] D148799: [clang] Return `StringRef` from `TargetInfo::getClobbers()`

[Sviatoslav Osipov via Phabricator via cfe-commits]

Stoorx added a comment.

> In this case, no one is manipulating the string until (I assume) after it's 
> converted to std::string, so we're not removing risky accesses in that way.

Maybe the solution is to return some kind of string (std::string or 
llvm::SmallString) by value or by const reference?

> If the API were returning a const char * that we then did a bunch of find 
> first, split at, strlen, etc on, then this would make more sense.

At least StringRef has the `Length` inside (we do not need to call strlen or 
someting) and according to documentation it can handle non-null terminated 
strings.

  /// StringRef - Represent a constant reference to a string, i.e. a character
  /// array and a length, which need not be null terminated.

Offtop: Is there any discussion platform where I can share some ideas? I have 
some of them how to refactor TargetInfo classes, but it's a major (and maybe 
breaking) change I have to discuss prior to implement.


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

https://reviews.llvm.org/D148799

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

[PATCH] D148639: [NFC][clang] Fix static analyzer concerns about AUTO_CAUSES_COPY

[Soumi Manna via Phabricator via cfe-commits]

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

@aaron.ballman, do you have any more concerns or comments? Thank you


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

https://reviews.llvm.org/D148639

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

[PATCH] D148835: [clang] removes trailing whitespace

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D148835#4284871 , @cjdb wrote:

> I've had some very good input about why this probably shouldn't go ahead: git 
> history erasure :')

While that is a common criticism, if we're going to do this at all, we should 
do it in a single patch, as we can use --ignore-rev: 
https://akrabat.com/ignoring-revisions-with-git-blame/

This is a pretty common thing to do, and I don't think it should prevent us 
from doing this, which I think is the 'greater good' here.  The alternative is 
to continue having a bunch of unrelated patches piece-meal 'erase' git history 
as people accidentally fix these.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148835

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

[PATCH] D148799: [clang] Return `StringRef` from `TargetInfo::getClobbers()`

[Sviatoslav Osipov via Phabricator via cfe-commits]

Stoorx updated this revision to Diff 515707.
Stoorx added a comment.

Rebase & upload with context


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

https://reviews.llvm.org/D148799

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/ARC.h
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Basic/Targets/AVR.h
  clang/lib/Basic/Targets/BPF.h
  clang/lib/Basic/Targets/CSKY.h
  clang/lib/Basic/Targets/DirectX.h
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/Basic/Targets/Lanai.h
  clang/lib/Basic/Targets/Le64.h
  clang/lib/Basic/Targets/LoongArch.h
  clang/lib/Basic/Targets/M68k.cpp
  clang/lib/Basic/Targets/M68k.h
  clang/lib/Basic/Targets/MSP430.h
  clang/lib/Basic/Targets/Mips.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/PNaCl.h
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/Sparc.h
  clang/lib/Basic/Targets/SystemZ.h
  clang/lib/Basic/Targets/TCE.h
  clang/lib/Basic/Targets/VE.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/Basic/Targets/XCore.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGStmt.cpp

Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2779,7 +2779,8 @@
  "unwind clobber can't be used with asm goto");
 
   // Add machine specific clobbers
-  std::string MachineClobbers = getTarget().getClobbers();
+  std::string MachineClobbers =
+  static_cast(getTarget().getClobbers());
   if (!MachineClobbers.empty()) {
 if (!Constraints.empty())
   Constraints += ',';
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -939,7 +939,8 @@
 
   // Build the constraints. FIXME: We should support immediates when possible.
   std::string Constraints = "={@ccc},r,r,~{cc},~{memory}";
-  std::string MachineClobbers = CGF.getTarget().getClobbers();
+  std::string MachineClobbers =
+  static_cast(CGF.getTarget().getClobbers());
   if (!MachineClobbers.empty()) {
 Constraints += ',';
 Constraints += MachineClobbers;
@@ -1082,7 +1083,8 @@
   AsmOS << "$0, ${1:y}";
 
   std::string Constraints = "=r,*Z,~{memory}";
-  std::string MachineClobbers = CGF.getTarget().getClobbers();
+  std::string MachineClobbers =
+  static_cast(CGF.getTarget().getClobbers());
   if (!MachineClobbers.empty()) {
 Constraints += ',';
 Constraints += MachineClobbers;
Index: clang/lib/Basic/Targets/XCore.h
===
--- clang/lib/Basic/Targets/XCore.h
+++ clang/lib/Basic/Targets/XCore.h
@@ -49,7 +49,9 @@
 return TargetInfo::VoidPtrBuiltinVaList;
   }
 
-  const char *getClobbers() const override { return ""; }
+  const StringRef getClobbers() const override {
+return llvm::StringLiteral("");
+  }
 
   ArrayRef getGCCRegNames() const override {
 static const char *const GCCRegNames[] = {
Index: clang/lib/Basic/Targets/X86.h
===
--- clang/lib/Basic/Targets/X86.h
+++ clang/lib/Basic/Targets/X86.h
@@ -262,8 +262,8 @@
StringRef Constraint, unsigned Size) const;
 
   std::string convertConstraint(const char *&Constraint) const override;
-  const char *getClobbers() const override {
-return "~{dirflag},~{fpsr},~{flags}";
+  const StringRef getClobbers() const override {
+return llvm::StringLiteral("~{dirflag},~{fpsr},~{flags}");
   }
 
   StringRef getConstraintRegister(StringRef Constraint,
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -130,7 +130,7 @@
 return false;
   }
 
-  const char *getClobbers() const final { return ""; }
+  const StringRef getClobbers() const final { return llvm::StringLiteral(""); }
 
   bool isCLZForZeroUndef() const final { return false; }
 
Index: clang/lib/Basic/Targets/VE.h
===
--- clang/lib/Basic/Targets/VE.h
+++ clang/lib/Basic/Targets/VE.h
@@ -69,7 +69,9 @@
 }
   }
 
-  const char *getClobbers() const override { return ""; }
+  const StringRef getClobbers() const override {
+return llvm::StringLiteral("");
+  }
 
   ArrayRef getGCCRegNames() const override {
 static const char *const GCCRegNames[] = {
Index: clang/lib/Basic/Targets/TCE.h
===
--- clang/lib/Basic/Targets/TCE.h
+++ clang/lib/Basic/Targets/TCE.h
@@ -99,7 +99,9 @@
 return std:

[PATCH] D148658: [clang] Make access to submodules via `iterator_range`

[Sviatoslav Osipov via Phabricator via cfe-commits]

Stoorx updated this revision to Diff 515708.
Stoorx added a comment.

Rebase & upload with context


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

https://reviews.llvm.org/D148658

Files:
  clang/include/clang/Basic/Module.h
  clang/lib/Basic/Module.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1903,18 +1903,16 @@
 SavedStrings.push_back(FilenameDup.data());
 
 HeaderFileInfoTrait::key_type Key = {
-  FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0
-};
+FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0};
 HeaderFileInfoTrait::data_type Data = {
-  Empty, {}, {M, ModuleMap::headerKindToRole(U.Kind)}
-};
+Empty, {}, {M, ModuleMap::headerKindToRole(U.Kind)}};
 // FIXME: Deal with cases where there are multiple unresolved header
 // directives in different submodules for the same header.
 Generator.insert(Key, Data, GeneratorTrait);
 ++NumHeaderSearchEntries;
   }
-
-  Worklist.append(M->submodule_begin(), M->submodule_end());
+  auto SubmodulesRange = M->submodules();
+  Worklist.append(SubmodulesRange.begin(), SubmodulesRange.end());
 }
   }
 
@@ -2701,9 +2699,8 @@
 /// given module).
 static unsigned getNumberOfModules(Module *Mod) {
   unsigned ChildModules = 0;
-  for (auto Sub = Mod->submodule_begin(), SubEnd = Mod->submodule_end();
-   Sub != SubEnd; ++Sub)
-ChildModules += getNumberOfModules(*Sub);
+  for (auto *Submodule : Mod->submodules())
+ChildModules += getNumberOfModules(Submodule);
 
   return ChildModules + 1;
 }
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -4309,15 +4309,12 @@
 /*IsInclusionDirective=*/false);
 // Enumerate submodules.
 if (Mod) {
-  for (Module::submodule_iterator Sub = Mod->submodule_begin(),
-  SubEnd = Mod->submodule_end();
-   Sub != SubEnd; ++Sub) {
-
+  for (auto *Submodule : Mod->submodules()) {
 Builder.AddTypedTextChunk(
-Builder.getAllocator().CopyString((*Sub)->Name));
+Builder.getAllocator().CopyString(Submodule->Name));
 Results.AddResult(Result(
 Builder.TakeString(), CCP_Declaration, CXCursor_ModuleImportDecl,
-(*Sub)->isAvailable() ? CXAvailability_Available
+Submodule->isAvailable() ? CXAvailability_Available
   : CXAvailability_NotAvailable));
   }
 }
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1240,7 +1240,8 @@
 ModMap.resolveConflicts(Mod, /*Complain=*/false);
 
 // Queue the submodules, so their exports will also be resolved.
-Stack.append(Mod->submodule_begin(), Mod->submodule_end());
+auto SubmodulesRange = Mod->submodules();
+Stack.append(SubmodulesRange.begin(), SubmodulesRange.end());
   }
 }
 
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -429,11 +429,9 @@
   }
 
   // Recurse into submodules.
-  for (clang::Module::submodule_iterator Sub = Module->submodule_begin(),
-  SubEnd = Module->submodule_end();
-   Sub != SubEnd; ++Sub)
+  for (auto *Submodule : Module->submodules())
 if (std::error_code Err = collectModuleHeaderIncludes(
-LangOpts, FileMgr, Diag, ModMap, *Sub, Includes))
+LangOpts, FileMgr, Diag, ModMap, Submodule, Includes))
   return Err;
 
   return std::error_code();
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -605,8 +605,9 @@
   Module *Current = Stack.pop_back_val();
   if (Current->IsUnimportable) continue;
   Current->IsAvailable = true;
-  Stack.insert(Stack.end(),
-   Current->submodule_begin(), Current->submodule_end());
+  auto SubmodulesRange = Current->submodules();
+  Stack.insert(Stack.end(), SubmodulesRange.begin(),
+   SubmodulesRange.end());
 }
   }
 }
Index: clang/

[PATCH] D148835: [clang] removes trailing whitespace

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D148835#4286905 , @erichkeane 
wrote:

> In D148835#4284871 , @cjdb wrote:
>
>> I've had some very good input about why this probably shouldn't go ahead: 
>> git history erasure :')
>
> While that is a common criticism, if we're going to do this at all, we should 
> do it in a single patch, as we can use --ignore-rev: 
> https://akrabat.com/ignoring-revisions-with-git-blame/
>
> This is a pretty common thing to do, and I don't think it should prevent us 
> from doing this, which I think is the 'greater good' here.  The alternative 
> is to continue having a bunch of unrelated patches piece-meal 'erase' git 
> history as people accidentally fix these.

We already ignore blame revisions like this today: 
https://github.com/llvm/llvm-project/blob/main/.git-blame-ignore-revs

However, I'd ask that we hold off on this change unless we have some tooling in 
place that rejects new additions of trailing whitespace, otherwise we're going 
to end up in this exact same situation again in another week or two. When CI 
starts failing on patches that insert *new* trailing whitespace, then I'm all 
for this change because we can fix it once and hopefully keep it fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148835

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

[PATCH] D148799: [clang] Return `StringRef` from `TargetInfo::getClobbers()`

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

You need to clearly state the "why" of the change in the commit message. 
Sometimes it seems obvious to the author but a reviewer has to assume and may 
assume incorrectly.

In this case, I guess that since the result of the function only has its length 
checked then is added to another string, you would avoid a heap allocation for 
the new std::string temporary by using StringRef.

You could also do this by making a StringRef from the const char* and checking 
its length. That does leave a small ambiguity as to whether returning nullptr 
and empty string is the same thing. Which it is, but it's not clear just from 
the prototype of the function. So changing getClobbers to return StringRef 
resolves both issues.

But equally, you might have had a completely different goal. Tell me if I am 
right :)

> Offtop: Is there any discussion platform where I can share some ideas? I have 
> some of them how to refactor TargetInfo classes, but it's a major (and maybe 
> breaking) change I have to discuss prior to implement.

An RFC on discourse is the usual way. If you can supply some work in progress 
patches or a tree on github then even better (and prevent you from proposing 
things you later find to be impossible, not that I have ever done that of 
course :) ).


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

https://reviews.llvm.org/D148799

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

[PATCH] D144651: [Serialization] Place command line defines in the correct file

[John Brawn via Phabricator via cfe-commits]

john.brawn added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144651

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

[PATCH] D134456: [PGO] Consider parent context when weighing branches with likelyhood.

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.
Herald added a subscriber: wlei.

>> Another point: having a diagnostic fire (failing a `-Werror` build) 
>> depending on the content of the profile passed to `-fprofile-use` seems 
>> pretty hostile to build workflows.
>
> Okay, this last point is especially compelling to me. I think opt remarks are 
> probably the correct way to go.

Zombie comment, but I think optimization remarks and warnings for when the 
[[likely]] annotation doesn't match profile data already exist, see here: 
https://clang.llvm.org/docs/MisExpect.html (it predates this review by a few 
months, but I just learned about it recently).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134456

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

[PATCH] D148211: [clang][tests] Fix Flang driver tests for Windows

[Ádám Kallai via Phabricator via cfe-commits]

kaadam added a comment.

Yes, it could be. I will update them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148211

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

[PATCH] D148849: [OpenMP-OPT] Remove limit for heap to stack conversions of __kmpc_alloc_shared allocations

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 closed this revision.
doru1004 added a comment.

Commit: 1a58c3d601b4c982afeb714c3a6c4be4d787cbf1 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148849

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

[PATCH] D148914: [Sema][NFC] add check before using `BuildExpressionFromIntegralTemplateArgument`

[Erich Keane via Phabricator via cfe-commits]

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

This is the wrong fix here, the crash is appropriate here, else we're not 
getting the type right.  For your example, we need to be setting up the 
template arguments correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148914

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

[PATCH] D148211: [clang][tests] Fix Flang driver tests for Windows

[Ádám Kallai via Phabricator via cfe-commits]

kaadam updated this revision to Diff 515717.
kaadam added a comment.

Updated


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

https://reviews.llvm.org/D148211

Files:
  clang/test/Driver/flang/flang.f90
  clang/test/Driver/flang/flang_ucase.F90
  clang/test/Driver/flang/multiple-inputs.f90


Index: clang/test/Driver/flang/multiple-inputs.f90
===
--- clang/test/Driver/flang/multiple-inputs.f90
+++ clang/test/Driver/flang/multiple-inputs.f90
@@ -1,7 +1,7 @@
 ! Check that flang driver can handle multiple inputs at once.
 
 ! RUN: %clang --driver-mode=flang -### -fsyntax-only %S/Inputs/one.f90 
%S/Inputs/two.f90 2>&1 | FileCheck --check-prefixes=CHECK-SYNTAX-ONLY %s
-! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang-new" "-fc1"
+! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang-new{{[^"/]*}}" "-fc1"
 ! CHECK-SYNTAX-ONLY: "{{[^"]*}}/Inputs/one.f90"
-! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang-new" "-fc1"
+! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang-new{{[^"/]*}}" "-fc1"
 ! CHECK-SYNTAX-ONLY: "{{[^"]*}}/Inputs/two.f90"
Index: clang/test/Driver/flang/flang_ucase.F90
===
--- clang/test/Driver/flang/flang_ucase.F90
+++ clang/test/Driver/flang/flang_ucase.F90
@@ -13,7 +13,7 @@
 ! * (no type specified, resulting in an object file)
 
 ! All invocations should begin with flang -fc1, consume up to here.
-! ALL-LABEL: "{{[^"]*}}flang-new" "-fc1"
+! ALL-LABEL: "{{[^"]*}}flang-new{{[^"/]*}}" "-fc1"
 
 ! Check that f90 files are not treated as "previously preprocessed"
 ! ... in --driver-mode=flang.
Index: clang/test/Driver/flang/flang.f90
===
--- clang/test/Driver/flang/flang.f90
+++ clang/test/Driver/flang/flang.f90
@@ -13,7 +13,7 @@
 ! * (no type specified, resulting in an object file)
 
 ! All invocations should begin with flang -fc1, consume up to here.
-! ALL-LABEL: "{{[^"]*}}flang-new" "-fc1"
+! ALL-LABEL: "{{[^"]*}}flang-new{{[^"/]*}}" "-fc1"
 
 ! Check that f90 files are not treated as "previously preprocessed"
 ! ... in --driver-mode=flang.


Index: clang/test/Driver/flang/multiple-inputs.f90
===
--- clang/test/Driver/flang/multiple-inputs.f90
+++ clang/test/Driver/flang/multiple-inputs.f90
@@ -1,7 +1,7 @@
 ! Check that flang driver can handle multiple inputs at once.
 
 ! RUN: %clang --driver-mode=flang -### -fsyntax-only %S/Inputs/one.f90 %S/Inputs/two.f90 2>&1 | FileCheck --check-prefixes=CHECK-SYNTAX-ONLY %s
-! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang-new" "-fc1"
+! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang-new{{[^"/]*}}" "-fc1"
 ! CHECK-SYNTAX-ONLY: "{{[^"]*}}/Inputs/one.f90"
-! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang-new" "-fc1"
+! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang-new{{[^"/]*}}" "-fc1"
 ! CHECK-SYNTAX-ONLY: "{{[^"]*}}/Inputs/two.f90"
Index: clang/test/Driver/flang/flang_ucase.F90
===
--- clang/test/Driver/flang/flang_ucase.F90
+++ clang/test/Driver/flang/flang_ucase.F90
@@ -13,7 +13,7 @@
 ! * (no type specified, resulting in an object file)
 
 ! All invocations should begin with flang -fc1, consume up to here.
-! ALL-LABEL: "{{[^"]*}}flang-new" "-fc1"
+! ALL-LABEL: "{{[^"]*}}flang-new{{[^"/]*}}" "-fc1"
 
 ! Check that f90 files are not treated as "previously preprocessed"
 ! ... in --driver-mode=flang.
Index: clang/test/Driver/flang/flang.f90
===
--- clang/test/Driver/flang/flang.f90
+++ clang/test/Driver/flang/flang.f90
@@ -13,7 +13,7 @@
 ! * (no type specified, resulting in an object file)
 
 ! All invocations should begin with flang -fc1, consume up to here.
-! ALL-LABEL: "{{[^"]*}}flang-new" "-fc1"
+! ALL-LABEL: "{{[^"]*}}flang-new{{[^"/]*}}" "-fc1"
 
 ! Check that f90 files are not treated as "previously preprocessed"
 ! ... in --driver-mode=flang.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148918: Check for a ‘buffer’ type instead of ‘buffer-live’.

[Philipp via Phabricator via cfe-commits]

phst added a comment.

Thanks for the review! I think you also need to land it (I don't seem to have 
the necessary permissions).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148918

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

[PATCH] D148211: [clang][tests] Fix Flang driver tests for Windows

[Bryan Chan via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D148211

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

[PATCH] D144269: [Analyzer] Show "taint originated here" note of alpha.security.taint.TaintPropagation checker at the correct place

[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.

I didn't go through the whole revision this time, but I think the next steps 
are already clear for the next round.
My impression was that I might not expressed my intent and expectations about 
the directions of the next step.
I hope I managed this time. Let me know if you have questions.




Comment at: clang/include/clang/StaticAnalyzer/Checkers/Taint.h:82-103
+/// Returns the tainted Symbols for a given Statement and state.
+std::vector getTaintedSymbols(ProgramStateRef State, const Stmt *S,
+ const LocationContext *LCtx,
+ TaintTagType Kind = TaintTagGeneric,
+ bool returnFirstOnly = false);
+
+/// Returns the tainted Symbols for a given SVal and state.

The overloads having the extra `ReturnFirstOnly` parameter shouldn't be visible 
here in the header.
That is an implementation detail that no users should know about.
Note that having a single default argument overload potentially doubles the 
variations the user might need to keep in mind when choosing the right one. So 
there is value in simplicity.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:248-254
+  std::vector TaintedSyms =
+  getTaintedSymbols(errorState, TaintedSVal);
+  // Mark all tainted symbols interesting
+  // to track back the propagation of taintedness.
+  for (auto Sym : TaintedSyms) {
+BR->markInteresting(Sym);
+  }





Comment at: clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp:62
+CheckerContext &C,
+std::vector TaintedSyms) const {
+  if (ExplodedNode *N = C.generateErrorNode(StateZero)) {





Comment at: clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp:108-109
+  if ((stateNotZero && stateZero)) {
+std::vector taintedSyms = getTaintedSymbols(C.getState(), *DV);
+if (!taintedSyms.empty()) {
+  reportTaintBug("Division by a tainted value, possibly zero", stateZero, 
C,





Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:863-868
+  std::vector TaintedSyms =
+  getTaintedSymbols(State, Call.getReturnValue());
+  if (!TaintedSyms.empty()) {
+TaintedSymbols.push_back(TaintedSyms[0]);
+TaintedIndexes.push_back(ArgNum);
+  }





Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:875-879
+  std::vector TaintedSyms = getTaintedSymbols(State, *V);
+  if (!TaintedSyms.empty()) {
+TaintedSymbols.push_back(TaintedSyms[0]);
+TaintedIndexes.push_back(ArgNum);
+  }

In these cases, the code would acquire all the tainted subsymbols, which then 
we throw away and keep only the first one.
This is why I suggested the approach I did I'm my last review.



Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:936-940
+IsMatching =
+IsMatching || (PropSrcArgs.contains(I) &&
+   isTaintedOrPointsToTainted(State, C.getSVal(E)));
+std::optional TaintedSVal =
+getTaintedPointeeOrPointer(State, C.getSVal(E));

Here `getTaintedPointeeOrPointer` would be called two times, unnecessarily.



Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:947-948
+  if (!TaintedArgSyms.empty()) {
+TaintedSymbols.insert(TaintedSymbols.begin(), TaintedArgSyms.begin(),
+  TaintedArgSyms.end());
+TaintedIndexes.push_back(I);





Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:1009-1010
   assert(E);
-  std::optional TaintedSVal{getTaintedPointeeOrPointer(C, C.getSVal(E))};
+  std::optional TaintedSVal{
+  getTaintedPointeeOrPointer(C.getState(), C.getSVal(E))};
 





Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:1019-1023
+std::vector TaintedSyms =
+getTaintedSymbols(C.getState(), *TaintedSVal);
+for (auto TaintedSym : TaintedSyms) {
+  report->markInteresting(TaintedSym);
+}





Comment at: clang/lib/StaticAnalyzer/Checkers/Taint.cpp:149
   const LocationContext *LCtx, TaintTagType Kind) {
-  SVal val = State->getSVal(S, LCtx);
-  return isTainted(State, val, Kind);
+  return !getTaintedSymbols(State, S, LCtx, Kind, true).empty();
 }

We usually pass booleans by "name".



Comment at: clang/lib/StaticAnalyzer/Checkers/Taint.cpp:294
+  std::vector TaintedCasts =
+  getTaintedSymbols(State, SC->getOperand(), Kind);
+  

[PATCH] D148355: [analyzer] Fix comparison logic in ArrayBoundCheckerV2

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

donat.nagy added a comment.

I tested this patch on a few open source projects and it significantly reduced 
the number of alpha.security.ArrayBoundV2 reports:

| Project name | memcached | tmux | curl | twin | vim | openssl |
| Baseline report #| 0 | 2| 13   | 6| 12  | 7   |
| Report # with this patch | 0 | 2| 0| 3| 3   | 0   |
|

(The patched version did not produce any additional reports, it seems these 
projects does not contain any bugs where the enhancements of the upper bound 
check could become significant. As a minor side effect, applying the patch also 
eliminated an alpha.deadcode.UnreachableCode report -- presumably because the 
analysis wasn't stopped at some false positive.)

I briefly looked at the eliminated reports, but they are very convoluted and I 
didn't analyze them to understand their (presumably buggy) logic. It seems that 
the remaining reports are also false positive (which is not surprising, as we 
are analyzing released, stable versions), but they are mostly reasonable:

- Several reports complain about expressions like `c = fgetc(f), isdigit(c)` 
because `fgetc(f)` returns tainted output and (at least in glibc) character 
type macros like `isdigit(c)` are equivalent to `((*__ctype_b_loc())[(int)(c)] 
& `. It would be relatively straightforward to fix these reports, 
perhaps by modelling the behavior of the internal function `__ctype_b_loc()`.
- Several reports are produced by code like `s = get_tainted_string(); int n = 
strlen(s); if (n > 0) return s[n-1];` where the analyzer says that `n-1` is a 
tainted index so this may be an overflow error.  To model this correctly we'd 
need to maintain a connection between the memory region of the string and the 
symbol conjured as the return value of strlen; my first guess is that this is 
doable but not too easy.
- There are also some sporadic false positives where the analyzer is confused 
by code that relies on tricky ad-hoc invariants.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148355

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

[PATCH] D148799: [clang] Return `StringRef` from `TargetInfo::getClobbers()`

[Sviatoslav Osipov via Phabricator via cfe-commits]

Stoorx added a comment.

I've looked through usages of `getClobbers` again, and found that in each case 
the result if the function is casted to `std::string`.

  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGStmt.cpp
  
  std::string Constraints = "=r,*Z,~{memory}";
std::string MachineClobbers =
static_cast(CGF.getTarget().getClobbers());
if (!MachineClobbers.empty()) {
  Constraints += ',';
  Constraints += MachineClobbers;
}

I think we can 'make two deals with one shot' if we would:

- Make `std::string_view` as a return type of `getClobbers` (by value, since 
the `std::string_view` is 'TriviallyCopyable')
- (Maybe) Make private static field of `std::string` type for actual clobber 
value (or `std::string_view` type since it can be constexpr-constructed)
- Return the value of this field from `getClobbers`
- Change type of MachineClobbers (in usages, see example above) variable to 
`std::string_view` since it actually used for read-only access. (We do not need 
to construct `std::string` and destruct it almost immediately)

As a result we get:

- More robust usage of `getClobbers` (a bit ridiculous but...)
- Zero unnecessary allocations

(Also: I've discovered that `llvm::StringRef` is exact the same thing as 
`std::string_view`. I think of it so much :^) )


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

https://reviews.llvm.org/D148799

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

[PATCH] D148812: [NFC][clang] Fix static analyzer concerns

[Soumi Manna via Phabricator via cfe-commits]

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

@erichkeane do you have any more concerns?


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

https://reviews.llvm.org/D148812

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

[PATCH] D148799: [clang] Return `StringRef` from `TargetInfo::getClobbers()`

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

string_view is a good call, especially as this is not doing anything fancy with 
it, and we have recently been allowed to use it.

> (Maybe) Make private static field of std::string type for actual clobber 
> value (or std::string_view type since it can be constexpr-constructed)

This I don't see the need for. If the string_views reference constant strings 
doesn't that amount to the same thing?

  const std::string_view getClobbers() const override {
  return "";
}

This makes a string view out of a pointer to a const string, but I guess it's 
up to the compiler how far it can optimise that.

You may be correct but it seems like a small optimisation.


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

https://reviews.llvm.org/D148799

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

[PATCH] D146987: [Assignment Tracking] Enable by default

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D146987#4286797 , @jmorse wrote:

> /me grumbles about all the world being a VAX,
>
> @mstorsjo I can't replicate the crash, but can replicate the valgrind 
> jump-on-uninitialized-value with a small reproducer [0] that doesn't feature 
> any debug-info

Ok, it's possible that bit was a red herring here. It didn't show up in a build 
of Clang instrumented with asan either.

> Could you confirm it's definitely assignment-tracking at fault by using 
> `-Xclang -fexperimental-assignment-tracking=forced` to enable and `-Xclang 
> -fexperimental-assignment-tracking=disabled` to disable, which should control 
> the behaviour if it's AT at fault.

The `-Xclang -fexperimental-assignment-tracking=disabled` flag does make the 
assert that shows up when built with Xcode's clang go away at least. It doesn't 
affect the valgrind failure, so that's indeed unrelated.

So there's something in Clang/LLVM which behaves differently, to the point of 
triggering a failed assert, when built with Xcode's Clang (reproed with two 
different Xcode versions) but not on Linux (with GCC or Clang, at least with a 
possibly older Clang).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146987

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

[PATCH] D146987: [Assignment Tracking] Enable by default

[Jeremy Morse via Phabricator via cfe-commits]

jmorse added a comment.

Ah, you're right, it's actually the same assertion message that @srj reported 
which I totally skipped over. Currently I suspect that's due to different 
std::sort implementations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146987

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

[PATCH] D148812: [NFC][clang] Fix static analyzer concerns

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Still had this convo which wasn't resolved.




Comment at: clang/lib/Lex/Pragma.cpp:1110
   Module *M = nullptr;
-  for (auto IIAndLoc : ModuleName) {
+  for (const auto &IIAndLoc : ModuleName) {
 M = MM.lookupModuleQualified(IIAndLoc.first->getName(), M);

erichkeane wrote:
> Manna wrote:
> > This returns a std::pair which is not 
> > particularly expensive to copy
> In that case, I'd skip the change here and leave it as a copy.
This seems like it should be a copy.  About a pointer-and-a-half with a cheap 
ctor.


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

https://reviews.llvm.org/D148812

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

[PATCH] D148799: [clang] Return `std::string_view` from `TargetInfo::getClobbers()`

[Sviatoslav Osipov via Phabricator via cfe-commits]

Stoorx updated this revision to Diff 515728.
Stoorx retitled this revision from "[clang] Return `StringRef` from 
`TargetInfo::getClobbers()`" to "[clang] Return `std::string_view` from 
`TargetInfo::getClobbers()`".
Stoorx edited the summary of this revision.
Stoorx added a comment.

Refactoring according the discussion


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

https://reviews.llvm.org/D148799

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/ARC.h
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Basic/Targets/AVR.h
  clang/lib/Basic/Targets/BPF.h
  clang/lib/Basic/Targets/CSKY.h
  clang/lib/Basic/Targets/DirectX.h
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/Basic/Targets/Lanai.h
  clang/lib/Basic/Targets/Le64.h
  clang/lib/Basic/Targets/LoongArch.h
  clang/lib/Basic/Targets/M68k.cpp
  clang/lib/Basic/Targets/M68k.h
  clang/lib/Basic/Targets/MSP430.h
  clang/lib/Basic/Targets/Mips.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/PNaCl.h
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/Sparc.h
  clang/lib/Basic/Targets/SystemZ.h
  clang/lib/Basic/Targets/TCE.h
  clang/lib/Basic/Targets/VE.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/Basic/Targets/XCore.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGStmt.cpp

Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2779,7 +2779,7 @@
  "unwind clobber can't be used with asm goto");
 
   // Add machine specific clobbers
-  std::string MachineClobbers = getTarget().getClobbers();
+  std::string_view MachineClobbers = getTarget().getClobbers();
   if (!MachineClobbers.empty()) {
 if (!Constraints.empty())
   Constraints += ',';
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -939,7 +939,7 @@
 
   // Build the constraints. FIXME: We should support immediates when possible.
   std::string Constraints = "={@ccc},r,r,~{cc},~{memory}";
-  std::string MachineClobbers = CGF.getTarget().getClobbers();
+  std::string_view MachineClobbers CGF.getTarget().getClobbers();
   if (!MachineClobbers.empty()) {
 Constraints += ',';
 Constraints += MachineClobbers;
@@ -1082,7 +1082,7 @@
   AsmOS << "$0, ${1:y}";
 
   std::string Constraints = "=r,*Z,~{memory}";
-  std::string MachineClobbers = CGF.getTarget().getClobbers();
+  std::string_view MachineClobbers = CGF.getTarget().getClobbers();
   if (!MachineClobbers.empty()) {
 Constraints += ',';
 Constraints += MachineClobbers;
Index: clang/lib/Basic/Targets/XCore.h
===
--- clang/lib/Basic/Targets/XCore.h
+++ clang/lib/Basic/Targets/XCore.h
@@ -49,7 +49,7 @@
 return TargetInfo::VoidPtrBuiltinVaList;
   }
 
-  const char *getClobbers() const override { return ""; }
+  std::string_view getClobbers() const override { return ""; }
 
   ArrayRef getGCCRegNames() const override {
 static const char *const GCCRegNames[] = {
Index: clang/lib/Basic/Targets/X86.h
===
--- clang/lib/Basic/Targets/X86.h
+++ clang/lib/Basic/Targets/X86.h
@@ -262,7 +262,7 @@
StringRef Constraint, unsigned Size) const;
 
   std::string convertConstraint(const char *&Constraint) const override;
-  const char *getClobbers() const override {
+  std::string_view getClobbers() const override {
 return "~{dirflag},~{fpsr},~{flags}";
   }
 
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -130,7 +130,7 @@
 return false;
   }
 
-  const char *getClobbers() const final { return ""; }
+  std::string_view getClobbers() const final { return ""; }
 
   bool isCLZForZeroUndef() const final { return false; }
 
Index: clang/lib/Basic/Targets/VE.h
===
--- clang/lib/Basic/Targets/VE.h
+++ clang/lib/Basic/Targets/VE.h
@@ -69,7 +69,7 @@
 }
   }
 
-  const char *getClobbers() const override { return ""; }
+  std::string_view getClobbers() const override { return ""; }
 
   ArrayRef getGCCRegNames() const override {
 static const char *const GCCRegNames[] = {
Index: clang/lib/Basic/Targets/TCE.h
===
--- clang/lib/Basic/Targets/TCE.h
+++ clang/lib/Basic/Targets/TCE.h
@@ -99,7 +99,7 @@
 return std::nullopt;
   }
 
- 

[PATCH] D148355: [analyzer] Fix comparison logic in ArrayBoundCheckerV2

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

steakhal added a comment.

I like this. Looks really solid.
I only had style nitpicks.

Have you run measurements?
After that we could land it, I think.

FYI: I'll also schedule a test run with this patch. You should expect the 
results in a week - not because it takes that long, but because I need to find 
time for evaluating it xD.




Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:57-61
+  RegionRawOffsetV2(const SubRegion *base, SVal offset)
+  : baseRegion(base), byteOffset(offset) {
+if (baseRegion)
+  assert(llvm::isa(byteOffset));
+  }





Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:121-135
+  if (auto ConcreteTh = Threshold.getAs()) {
+std::tie(Value, Threshold) = getSimplifiedOffsets(Value, *ConcreteTh, SVB);
+  }
+  if (auto ConcreteTh = Threshold.getAs()) {
+QualType T = Value.getType(SVB.getContext());
+if (T->isUnsignedIntegerType() && ConcreteTh->getValue().isNegative()) {
+  // In this case we reduced the bound check to a comparison of the form





Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:136-137
+  }
+  SVal belowThreshold =
+  SVB.evalBinOpNN(State, BO_LT, Value, Threshold, SVB.getConditionType());
+

steakhal wrote:
> 




Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:136-140
+  SVal belowThreshold =
+  SVB.evalBinOpNN(State, BO_LT, Value, Threshold, SVB.getConditionType());
+
+  if (std::optional belowThresholdNL = belowThreshold.getAs())
+return State->assume(*belowThresholdNL);





Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:167-168
 
-  NonLoc rawOffsetVal = rawOffset.getByteOffset();
-
-  // CHECK LOWER BOUND: Is byteOffset < extent begin?
-  //  If so, we are doing a load/store
-  //  before the first valid offset in the memory region.
+  // At this point we know that rawOffset is not default-constructed so we may
+  // call this:
+  NonLoc ByteOffset = rawOffset.getByteOffset();

I don't think we need an explanation here.
BTW This "optional-like" `RegionRawOffsetV2` makes me angry.  It should be an 
`std::optional` in the first place.
You don't need to take actions, I'm just ranting...



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:173
+  const MemSpaceRegion *SR = rawOffset.getRegion()->getMemorySpace();
+  if (SR->getKind() != MemRegion::UnknownSpaceRegionKind) {
+// a pointer to UnknownSpaceRegionKind may point to the middle of





Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:174-175
+  if (SR->getKind() != MemRegion::UnknownSpaceRegionKind) {
+// a pointer to UnknownSpaceRegionKind may point to the middle of
+// an allocated region
 

Good point.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:183
 if (state_precedesLowerBound && !state_withinLowerBound) {
+  // We know that the index definitely precedes the lower bound
   reportOOB(checkerContext, state_precedesLowerBound, OOB_Precedes);





Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:195
+  getDynamicExtent(state, rawOffset.getRegion(), svalBuilder);
+  if (std::optional KnownSize = Size.getAs()) {
+ProgramStateRef state_withinUpperBound, state_exceedsUpperBound;





Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:196-198
+ProgramStateRef state_withinUpperBound, state_exceedsUpperBound;
+std::tie(state_withinUpperBound, state_exceedsUpperBound) =
+compareValueToThreshold(state, ByteOffset, *KnownSize, svalBuilder);

I think as we don't plan to overwrite/assign to these states, we could just use 
structured bindings.
I think that should be preferred over `std::tie()`-ing think. That is only not 
widespread because we just recently moved to C++17.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:212-215
 }
 
-assert(state_withinUpperBound);
-state = state_withinUpperBound;
+if (state_withinUpperBound)
+  state = state_withinUpperBound;

You just left the guarded block in the previous line.
By moving this statement there you could spare the `if`.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:310-311
   offset = getValue(offset, svalBuilder);
   if (!offset.isUnknownOrUndef())
 return RegionRawOffsetV2(subReg, offset);
 }




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148355

_

[PATCH] D148799: [clang] Return `std::string_view` from `TargetInfo::getClobbers()`

[Sviatoslav Osipov via Phabricator via cfe-commits]

Stoorx added a comment.

I did not set `const` qualifier for return type because `std::string_view` is 
constant by design. Or should I mark it `const` explicitly?


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

https://reviews.llvm.org/D148799

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

cfe-commits@lists.llvm.org

[Jens Massberg via Phabricator via cfe-commits]

massberg created this revision.
massberg added a reviewer: ilya-biryukov.
Herald added a project: All.
massberg requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch implemed the change proposed in [P2002R1] to 11.11.1 
[class.compare.default] paragraph 1.

A defaulted compariosn operator function must be non-volatile and must either 
have no ref-qualifier or the ref-qualifier &.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148924

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/class/class.compare/class.compare.default/p1.cpp


Index: clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
===
--- clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
+++ clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
@@ -15,7 +15,8 @@
 
   bool operator<(const A&) const;
   bool operator<=(const A&) const = default;
-  bool operator==(const A&) const volatile && = default; // surprisingly, OK
+  bool operator==(const A&) const && = default; // expected-error 
{{ref-qualifier '&&' is not allowed on defaulted comparison operators}}
+  bool operator>=(const A&) const volatile = default; // expected-error 
{{defaulted comparison operator function must not be volatile}}
   bool operator<=>(const A&) = default; // expected-error {{defaulted member 
three-way comparison operator must be const-qualified}}
   bool operator>=(const B&) const = default; // expected-error-re {{invalid 
parameter type for defaulted relational comparison operator; found 'const B &', 
expected 'const A &'{{$
   static bool operator>(const B&) = default; // expected-error {{overloaded 
'operator>' cannot be a static member function}}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -8579,8 +8579,8 @@
   // C++2a [class.compare.default]p1:
   //   A defaulted comparison operator function for some class C shall be a
   //   non-template function declared in the member-specification of C that is
-  //-- a non-static const member of C having one parameter of type
-  //   const C&, or
+  //-- a non-static const non-volatile member of C having one parameter of 
type
+  //   const C& and either no ref-qualifier or the ref-qualifier &, or
   //-- a friend of C having two parameters of type const C& or two
   //   parameters of type C.
 
@@ -8590,6 +8590,16 @@
 auto *MD = cast(FD);
 assert(!MD->isStatic() && "comparison function cannot be a static member");
 
+const FunctionProtoType *FnType = 
FD->getType()->castAs();
+if (FnType->isVolatile()) {
+  Diag(FD->getLocation(), diag::err_volatile_comparison_operator);
+  return true;
+}
+if (FnType->getRefQualifier() == RQ_RValue) {
+  Diag(FD->getLocation(), diag::err_ref_qualifier_comparison_operator);
+  return true;
+}
+
 // If we're out-of-class, this is the class we're comparing.
 if (!RD)
   RD = MD->getParent();
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9430,6 +9430,10 @@
 def note_in_declaration_of_implicit_equality_comparison : Note<
   "while declaring the corresponding implicit 'operator==' "
   "for this defaulted 'operator<=>'">;
+def err_volatile_comparison_operator : Error<
+  "defaulted comparison operator function must not be volatile">;
+def err_ref_qualifier_comparison_operator : Error<
+  "ref-qualifier '&&' is not allowed on defaulted comparison operators">;
 
 def ext_implicit_exception_spec_mismatch : ExtWarn<
   "function previously declared with an %select{explicit|implicit}0 exception "


Index: clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
===
--- clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
+++ clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
@@ -15,7 +15,8 @@
 
   bool operator<(const A&) const;
   bool operator<=(const A&) const = default;
-  bool operator==(const A&) const volatile && = default; // surprisingly, OK
+  bool operator==(const A&) const && = default; // expected-error {{ref-qualifier '&&' is not allowed on defaulted comparison operators}}
+  bool operator>=(const A&) const volatile = default; // expected-error {{defaulted comparison operator function must not be volatile}}
   bool operator<=>(const A&) = default; // expected-error {{defaulted member three-way comparison operator must be const-qualified}}
   bool operator>=(const B&) const = default; // expected-error-re {{invalid parameter type for defaulted relational c

[PATCH] D148355: [analyzer] Fix comparison logic in ArrayBoundCheckerV2

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

steakhal added a comment.

Okay, so you just added some numbers :D
Now it's my turn I guess. I'll compare this version against the version I 
shared, and use in our product.
Stay tuned.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148355

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

[PATCH] D148812: [NFC][clang] Fix static analyzer concerns

[Soumi Manna via Phabricator via cfe-commits]

Manna updated this revision to Diff 515729.
Manna edited the summary of this revision.
Manna added a comment.

I have addressed @erichkeane's review comment.


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

https://reviews.llvm.org/D148812

Files:
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ComputeDependence.cpp
  clang/lib/ASTMatchers/Dynamic/Marshallers.h
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -4259,7 +4259,7 @@
 
   // Generate all of the custom appertainsTo functions that the attributes
   // will be using.
-  for (auto I : Attrs) {
+  for (const auto &I : Attrs) {
 const Record &Attr = *I.second;
 if (Attr.isValueUnset("Subjects"))
   continue;
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -3971,7 +3971,7 @@
 }
 
 llvm::SmallPtrSet UninitializedBaseClasses;
-for (auto I : RD->bases())
+for (const auto &I : RD->bases())
   UninitializedBaseClasses.insert(I.getType().getCanonicalType());
 
 if (UninitializedFields.empty() && UninitializedBaseClasses.empty())
Index: clang/lib/CodeGen/CGVTables.cpp
===
--- clang/lib/CodeGen/CGVTables.cpp
+++ clang/lib/CodeGen/CGVTables.cpp
@@ -1269,13 +1269,13 @@
   else
 TypeVis = llvm::GlobalObject::VCallVisibilityPublic;
 
-  for (auto B : RD->bases())
+  for (const auto &B : RD->bases())
 if (B.getType()->getAsCXXRecordDecl()->isDynamicClass())
   TypeVis = std::min(
   TypeVis,
   GetVCallVisibilityLevel(B.getType()->getAsCXXRecordDecl(), Visited));
 
-  for (auto B : RD->vbases())
+  for (const auto &B : RD->vbases())
 if (B.getType()->getAsCXXRecordDecl()->isDynamicClass())
   TypeVis = std::min(
   TypeVis,
Index: clang/lib/ASTMatchers/Dynamic/Marshallers.h
===
--- clang/lib/ASTMatchers/Dynamic/Marshallers.h
+++ clang/lib/ASTMatchers/Dynamic/Marshallers.h
@@ -1008,7 +1008,7 @@
Diagnostics *) const override {
 
 std::vector NodeKinds;
-for (auto Arg : Args) {
+for (const auto &Arg : Args) {
   if (!Arg.Value.isNodeKind())
 return {};
   NodeKinds.push_back(Arg.Value.getNodeKind());
Index: clang/lib/AST/ComputeDependence.cpp
===
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -760,7 +760,7 @@
   D |= getDependenceInExpr(E->getNameInfo());
   if (auto *Q = E->getQualifier())
 D |= toExprDependence(Q->getDependence());
-  for (auto A : E->template_arguments())
+  for (const auto &A : E->template_arguments())
 D |= toExprDependence(A.getArgument().getDependence());
   return D;
 }
@@ -813,7 +813,7 @@
   if (auto *Q = E->getQualifier())
 D |= toExprDependence(Q->getDependence());
   D |= getDependenceInExpr(E->getMemberNameInfo());
-  for (auto A : E->template_arguments())
+  for (const auto &A : E->template_arguments())
 D |= toExprDependence(A.getArgument().getDependence());
   return D;
 }
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -8203,7 +8203,7 @@
   if (!CXXRD->hasDefinition() || !VisitBasesAndFields)
 return false;
 
-  for (auto B : CXXRD->bases())
+  for (const auto &B : CXXRD->bases())
 if (hasTemplateSpecializationInEncodedString(B.getType().getTypePtr(),
  true))
   return true;


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -4259,7 +4259,7 @@
 
   // Generate all of the custom appertainsTo functions that the attributes
   // will be using.
-  for (auto I : Attrs) {
+  for (const auto &I : Attrs) {
 const Record &Attr = *I.second;
 if (Attr.isValueUnset("Subjects"))
   continue;
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -3971,7 +3971,7 @@
 }
 
 llvm::SmallPtrSet UninitializedBaseClasses;
-for (auto I : RD->bases())
+for (const auto &I : RD->bases())
   UninitializedBaseClasses.insert(I.getType().getCanonicalType());
 
 if (UninitializedFields.empty() && UninitializedBaseClasses.empty())
Index: clang/lib/CodeGen/CGVTables.cpp
===

[PATCH] D148812: [NFC][clang] Fix static analyzer concerns

[Soumi Manna via Phabricator via cfe-commits]

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



Comment at: clang/lib/Lex/Pragma.cpp:1110
   Module *M = nullptr;
-  for (auto IIAndLoc : ModuleName) {
+  for (const auto &IIAndLoc : ModuleName) {
 M = MM.lookupModuleQualified(IIAndLoc.first->getName(), M);

erichkeane wrote:
> erichkeane wrote:
> > Manna wrote:
> > > This returns a std::pair which is not 
> > > particularly expensive to copy
> > In that case, I'd skip the change here and leave it as a copy.
> This seems like it should be a copy.  About a pointer-and-a-half with a cheap 
> ctor.
I missed that. Thanks @erichkeane.  Done


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

https://reviews.llvm.org/D148812

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

[PATCH] D148925: Fix discarding void-typed comma expressions

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

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

First, we need to handle void types in `visitExpr`, so we don't run into an 
assertion there when we try to pop a return value from the stack that isn't 
there.

Secondly, we need to handle it when visiting comma expressions so we don't do 
the same thing there.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148925

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
@@ -791,6 +791,8 @@
   (a); // expected-warning {{unused}} \
// ref-warning {{unused}}
 
+  (void)5, (void)6;
+
   return 0;
 }
 
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -264,8 +264,12 @@
 
   // Deal with operations which have composite or void types.
   if (BO->isCommaOp()) {
-if (!discard(LHS))
+if (this->discard(LHS))
   return false;
+if (RHS->getType()->isVoidType())
+  return this->discard(RHS);
+
+// Otherwise, visit RHS and optionally discard its value.
 return Discard(this->visit(RHS));
   }
 
@@ -1650,10 +1654,12 @@
   if (!visit(Exp))
 return false;
 
+  if (Exp->getType()->isVoidType())
+return this->emitRetVoid(Exp);
+
   if (std::optional T = classify(Exp))
 return this->emitRet(*T, Exp);
-  else
-return this->emitRetValue(Exp);
+  return this->emitRetValue(Exp);
 }
 
 /// Toplevel visitDecl().


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -791,6 +791,8 @@
   (a); // expected-warning {{unused}} \
// ref-warning {{unused}}
 
+  (void)5, (void)6;
+
   return 0;
 }
 
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -264,8 +264,12 @@
 
   // Deal with operations which have composite or void types.
   if (BO->isCommaOp()) {
-if (!discard(LHS))
+if (this->discard(LHS))
   return false;
+if (RHS->getType()->isVoidType())
+  return this->discard(RHS);
+
+// Otherwise, visit RHS and optionally discard its value.
 return Discard(this->visit(RHS));
   }
 
@@ -1650,10 +1654,12 @@
   if (!visit(Exp))
 return false;
 
+  if (Exp->getType()->isVoidType())
+return this->emitRetVoid(Exp);
+
   if (std::optional T = classify(Exp))
 return this->emitRet(*T, Exp);
-  else
-return this->emitRetValue(Exp);
+  return this->emitRetValue(Exp);
 }
 
 /// Toplevel visitDecl().
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148658: [clang] Make access to submodules via `iterator_range`

[Sviatoslav Osipov via Phabricator via cfe-commits]

Stoorx updated this revision to Diff 515734.

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

https://reviews.llvm.org/D148658

Files:
  clang/include/clang/Basic/Module.h
  clang/lib/Basic/Module.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -500,8 +500,7 @@
   // Submodule order depends on order of header includes for inferred submodules
   // we don't care about the exact order, so sort so that it's consistent across
   // TUs to improve sharing.
-  SmallVector Submodules(M->submodule_begin(),
- M->submodule_end());
+  SmallVector Submodules(M->submodules());
   llvm::stable_sort(Submodules, [](const Module *A, const Module *B) {
 return A->Name < B->Name;
   });
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1903,18 +1903,16 @@
 SavedStrings.push_back(FilenameDup.data());
 
 HeaderFileInfoTrait::key_type Key = {
-  FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0
-};
+FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0};
 HeaderFileInfoTrait::data_type Data = {
-  Empty, {}, {M, ModuleMap::headerKindToRole(U.Kind)}
-};
+Empty, {}, {M, ModuleMap::headerKindToRole(U.Kind)}};
 // FIXME: Deal with cases where there are multiple unresolved header
 // directives in different submodules for the same header.
 Generator.insert(Key, Data, GeneratorTrait);
 ++NumHeaderSearchEntries;
   }
-
-  Worklist.append(M->submodule_begin(), M->submodule_end());
+  auto SubmodulesRange = M->submodules();
+  Worklist.append(SubmodulesRange.begin(), SubmodulesRange.end());
 }
   }
 
@@ -2701,9 +2699,8 @@
 /// given module).
 static unsigned getNumberOfModules(Module *Mod) {
   unsigned ChildModules = 0;
-  for (auto Sub = Mod->submodule_begin(), SubEnd = Mod->submodule_end();
-   Sub != SubEnd; ++Sub)
-ChildModules += getNumberOfModules(*Sub);
+  for (auto *Submodule : Mod->submodules())
+ChildModules += getNumberOfModules(Submodule);
 
   return ChildModules + 1;
 }
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -4309,15 +4309,12 @@
 /*IsInclusionDirective=*/false);
 // Enumerate submodules.
 if (Mod) {
-  for (Module::submodule_iterator Sub = Mod->submodule_begin(),
-  SubEnd = Mod->submodule_end();
-   Sub != SubEnd; ++Sub) {
-
+  for (auto *Submodule : Mod->submodules()) {
 Builder.AddTypedTextChunk(
-Builder.getAllocator().CopyString((*Sub)->Name));
+Builder.getAllocator().CopyString(Submodule->Name));
 Results.AddResult(Result(
 Builder.TakeString(), CCP_Declaration, CXCursor_ModuleImportDecl,
-(*Sub)->isAvailable() ? CXAvailability_Available
+Submodule->isAvailable() ? CXAvailability_Available
   : CXAvailability_NotAvailable));
   }
 }
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1240,7 +1240,8 @@
 ModMap.resolveConflicts(Mod, /*Complain=*/false);
 
 // Queue the submodules, so their exports will also be resolved.
-Stack.append(Mod->submodule_begin(), Mod->submodule_end());
+auto SubmodulesRange = Mod->submodules();
+Stack.append(SubmodulesRange.begin(), SubmodulesRange.end());
   }
 }
 
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -429,11 +429,9 @@
   }
 
   // Recurse into submodules.
-  for (clang::Module::submodule_iterator Sub = Module->submodule_begin(),
-  SubEnd = Module->submodule_end();
-   Sub != SubEnd; ++Sub)
+  for (auto *Submodule : Module->submodules())
 if (std::error_code Err = collectModuleHeaderIncludes(
-LangOpts, FileMgr, Diag, ModMap, *Sub, Includes))
+LangOpts, FileMgr, Diag, ModMap, Submodule,

[PATCH] D148926: [Clang] Add support for [[msvc::constexpr]] C++11 attribute

[Richard Dzenis via Phabricator via cfe-commits]

RIscRIpt created this revision.
Herald added a reviewer: aaron.ballman.
Herald added a subscriber: jdoerfert.
Herald added a project: All.
RIscRIpt 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/D148926

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/AST/msvc-attrs-invalid.cpp
  clang/test/AST/msvc-attrs.cpp
  clang/test/AST/msvc-constexpr-new.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -83,6 +83,7 @@
 // CHECK-NEXT: LoaderUninitialized (SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: Lockable (SubjectMatchRule_record)
 // CHECK-NEXT: MIGServerRoutine (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_block)
+// CHECK-NEXT: MSConstexpr (SubjectMatchRule_function)
 // CHECK-NEXT: MSStruct (SubjectMatchRule_record)
 // CHECK-NEXT: MaybeUndef (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: MicroMips (SubjectMatchRule_function)
Index: clang/test/AST/msvc-constexpr-new.cpp
===
--- /dev/null
+++ clang/test/AST/msvc-constexpr-new.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fms-extensions -std=c++20 -ast-dump %s | FileCheck %s
+
+// CHECK: used operator new
+// CHECK: MSConstexprAttr 0x{{[0-9a-f]+}} 
+[[nodiscard]] [[msvc::constexpr]] inline void* __cdecl operator new(decltype(sizeof(void*)), void* p) noexcept { return p; }
+
+// CHECK: used constexpr construct_at
+// CHECK: AttributedStmt 0x{{[0-9a-f]+}} 
+// CHECK-NEXT: MSConstexprAttr 0x{{[0-9a-f]+}} 
+// CHECK-NEXT: ReturnStmt 0x{{[0-9a-f]+}} 
+constexpr int* construct_at(int* p, int v) { [[msvc::constexpr]] return ::new (p) int(v); }
+
+constexpr bool check_construct_at() { int x; return *construct_at(&x, 42) == 42; }
+
+static_assert(check_construct_at());
Index: clang/test/AST/msvc-attrs.cpp
===
--- /dev/null
+++ clang/test/AST/msvc-attrs.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -fms-extensions -std=c++20 -ast-dump %s | FileCheck %s
+
+// [[msvc::constexpr]] tests
+
+// MSConstexprDocs (1)
+// CHECK: used f1 'bool ()'
+// CHECK: MSConstexprAttr 0x{{[0-9a-f]+}} 
+[[msvc::constexpr]] bool f1() { return true; }
+
+// MSConstexprDocs (2)
+// CHECK: used constexpr f2 'bool ()'
+// CHECK: AttributedStmt 0x{{[0-9a-f]+}} 
+// CHECK-NEXT: MSConstexprAttr 0x{{[0-9a-f]+}} 
+// CHECK-NEXT: ReturnStmt 0x{{[0-9a-f]+}} 
+constexpr bool f2() { [[msvc::constexpr]] return f1(); }
+
+static_assert(f2());
+
+constexpr bool f3() { [[msvc::constexpr]] return f1() || f1() && f1(); }
+static_assert(f3());
+
+[[msvc::constexpr]] int f4(int x) { [[msvc::constexpr]] return x > 1 ? 1 + f4(x / 2) : 0; }
+constexpr bool f5() { [[msvc::constexpr]] return f4(32) == 5; }
+static_assert(f5());
+
+[[msvc::constexpr]] int f6(int x)
+{
+switch (x)
+{
+case 42: return 1337;
+default:
+ if (x < 0) [[msvc::constexpr]] return f4(-x);
+ else return x;
+}
+}
+
+constexpr bool f7() { [[msvc::constexpr]] return f6(f6(42) - 1337 + f6(-32) - 5 + (f6(1) ? f6(0) : f6(2))) == f6(0); }
+static_assert(f7());
Index: clang/test/AST/msvc-attrs-invalid.cpp
===
--- /dev/null
+++ clang/test/AST/msvc-attrs-invalid.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -fms-extensions -std=c++20 -verify %s
+
+// Check explicitly invalid code
+
+void runtime() {} // expected-note {{declared here}}
+
+[[msvc::constexpr]] void f0() { runtime(); } // expected-error {{[[msvc::constexpr]] function never produces a constant expression}} \
+ // expected-note {{non-constexpr function 'runtime' cannot be used in a constant expression}}
+[[msvc::constexpr]] constexpr void f1() {} // expected-error {{[[msvc::constexpr]] cannot be applied to a constexpr function 'f1'}}
+[[msvc::constexpr]] consteval void f2() {} // expected-error {{[[msvc::constexpr]] cannot be applied to a consteval function 'f2'}}
+
+// Check invalid code mixed with valid code
+
+[[msvc::constexpr]] bool undefined();
+
+[[msvc::constexpr]] int f4(int x) { return x > 1 ? 1 + f4(x / 2) : 0; } // expected-note {{[[msvc::constexpr]] function 'f4' is used not in '[[msvc::constexpr]] return' statement}}
+constexpr bool f5() {

[PATCH] D148799: [clang] Return `std::string_view` from `TargetInfo::getClobbers()`

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

> I did not set const qualifier for return type because std::string_view is 
> constant by design. Or should I mark it const explicitly?

I'm new to string_view but everything I see backs up that it is a constant view 
on the data as you say. I'm not sure what making it itself const would do, not 
worth looking into here.

Please update the commit message (here and locally, in case you happen to be 
using `arc`, which prefers one and I forget which one). It should include the 
reasoning for the change, as we've discussed here.




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:942
   std::string Constraints = "={@ccc},r,r,~{cc},~{memory}";
-  std::string MachineClobbers = CGF.getTarget().getClobbers();
+  std::string_view MachineClobbers CGF.getTarget().getClobbers();
   if (!MachineClobbers.empty()) {

You dropped a `=`, please make sure it compiles before updating the review.


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

https://reviews.llvm.org/D148799

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

[PATCH] D148925: Fix discarding void-typed comma expressions

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

So things cast to 'void' can have side effects, right?  Does the call to 
discard still evaluate them?

https://godbolt.org/z/a3ej9bxKe

  constexpr int foo(int start) {
  int i = start;
  
  (void)i++;
  (void)i++,(void)i++;
  return i;
  }
  constexpr int Value = foo(5);
  static_assert(Value == 8);


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148925

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

[PATCH] D148799: [clang] Return `std::string_view` from `TargetInfo::getClobbers()`

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

Build issues will also show up in the "Build Status" here.


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

https://reviews.llvm.org/D148799

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

[PATCH] D148355: [analyzer] Fix comparison logic in ArrayBoundCheckerV2

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

donat.nagy added a comment.

@steakhal Thanks for the review, it was really instructive :). I'll probably 
upload the updated commit on Monday.

What kind of measurements are you suggesting? I analyzed a few open source 
projects with the patched Clang SA (see results in my previous commit) and a 
few additional analysis runs are still ongoing. (I didn't measure 
performance/runtime, because I don't think that this change significantly 
affects those.)




Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:167-168
 
-  NonLoc rawOffsetVal = rawOffset.getByteOffset();
-
-  // CHECK LOWER BOUND: Is byteOffset < extent begin?
-  //  If so, we are doing a load/store
-  //  before the first valid offset in the memory region.
+  // At this point we know that rawOffset is not default-constructed so we may
+  // call this:
+  NonLoc ByteOffset = rawOffset.getByteOffset();

steakhal wrote:
> I don't think we need an explanation here.
> BTW This "optional-like" `RegionRawOffsetV2` makes me angry.  It should be an 
> `std::optional` in the first place.
> You don't need to take actions, I'm just ranting...
Yes, I was also very annoyed by this "intrustive optional" behavior and I 
seriously considered refactoring the code to use a real std::optional, but I 
didn't want to snowball this change into a full-scale rewrite so I ended up 
with the assert in the constructor and this comment. Perhaps I'll refactor it 
in a separate NFC...



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:173
+  const MemSpaceRegion *SR = rawOffset.getRegion()->getMemorySpace();
+  if (SR->getKind() != MemRegion::UnknownSpaceRegionKind) {
+// a pointer to UnknownSpaceRegionKind may point to the middle of

steakhal wrote:
> 
You're completely right, I just blindly copied this test from the needlessly 
overcomplicated `computeExtentBegin()`.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:212-215
 }
 
-assert(state_withinUpperBound);
-state = state_withinUpperBound;
+if (state_withinUpperBound)
+  state = state_withinUpperBound;

steakhal wrote:
> You just left the guarded block in the previous line.
> By moving this statement there you could spare the `if`.
Nice catch :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148355

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

[PATCH] D148926: [Clang] Add support for [[msvc::constexpr]] C++11 attribute

[Richard Dzenis via Phabricator via cfe-commits]

RIscRIpt abandoned this revision.
RIscRIpt added a comment.

This had to be submitted to https://reviews.llvm.org/D134475


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148926

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

[PATCH] D148355: [analyzer] Fix comparison logic in ArrayBoundCheckerV2

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

steakhal added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:167-168
 
-  NonLoc rawOffsetVal = rawOffset.getByteOffset();
-
-  // CHECK LOWER BOUND: Is byteOffset < extent begin?
-  //  If so, we are doing a load/store
-  //  before the first valid offset in the memory region.
+  // At this point we know that rawOffset is not default-constructed so we may
+  // call this:
+  NonLoc ByteOffset = rawOffset.getByteOffset();

donat.nagy wrote:
> steakhal wrote:
> > I don't think we need an explanation here.
> > BTW This "optional-like" `RegionRawOffsetV2` makes me angry.  It should be 
> > an `std::optional` in the first place.
> > You don't need to take actions, I'm just ranting...
> Yes, I was also very annoyed by this "intrustive optional" behavior and I 
> seriously considered refactoring the code to use a real std::optional, but I 
> didn't want to snowball this change into a full-scale rewrite so I ended up 
> with the assert in the constructor and this comment. Perhaps I'll refactor it 
> in a separate NFC...
Please :D



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:173
+  const MemSpaceRegion *SR = rawOffset.getRegion()->getMemorySpace();
+  if (SR->getKind() != MemRegion::UnknownSpaceRegionKind) {
+// a pointer to UnknownSpaceRegionKind may point to the middle of

donat.nagy wrote:
> steakhal wrote:
> > 
> You're completely right, I just blindly copied this test from the needlessly 
> overcomplicated `computeExtentBegin()`.
Hold on. This would only skip the lower bounds check if it's an 
`UnknownSpaceRegion`.
Shouldn't we early return instead?



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:212-215
 }
 
-assert(state_withinUpperBound);
-state = state_withinUpperBound;
+if (state_withinUpperBound)
+  state = state_withinUpperBound;

donat.nagy wrote:
> steakhal wrote:
> > You just left the guarded block in the previous line.
> > By moving this statement there you could spare the `if`.
> Nice catch :)
On second though no. The outer if guards `state_exceedsUpperBound`.
So this check seems necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148355

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

[PATCH] D148914: [Sema][NFC] add check before using `BuildExpressionFromIntegralTemplateArgument`

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

See my comment on https://github.com/llvm/llvm-project/issues/62265 for results 
of my debugging.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148914

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

[PATCH] D134475: Add C++11 attribute msvc::constexpr

[Richard Dzenis via Phabricator via cfe-commits]

RIscRIpt updated this revision to Diff 515740.
RIscRIpt added a comment.

A proper implementation

In D134475#4224082 , @erichkeane 
wrote:

> I don't have a good idea, I don't think any sort of RAII object is the right 
> way (since it will be wrong on child calls/etc), and the ParentMap is 
> definitely not the right way.  Perhaps just a flag for 
> CheckConstexprFunction?  I've not spent too much time in this section of the 
> compiler, so hopefully someone else can come along and help. I suspect it is 
> a property of `CallStackFrame`?  But that already contains a link to the 
> FucntionDecl, right?  As far as the 'on return statement', I think the bit on 
> CallStackFrame/EvalInfo is probably what is necessary.



> But that already contains a link to the FucntionDecl, right?

Yes, but we need to know current context (whether we're in `[[msvc::constexpr]] 
return` statement).

> As far as the 'on return statement', I think the bit on 
> CallStackFrame/EvalInfo is probably what is necessary.

That's what I ended-up doing. This was the most reasonable approach I could 
come-up with.

I added a property to `CallStackFrame`, because it's a property of current 
function: whether we are in valid context which permits evaluation of 
`[[msvc::constexpr]]`. Additionally I added `MSConstexprContextRAII` which is 
used in `case Stmt::AttributedStmtClass:` to alter the property.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134475

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/AST/msvc-attrs-invalid.cpp
  clang/test/AST/msvc-attrs.cpp
  clang/test/AST/msvc-constexpr-new.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -83,6 +83,7 @@
 // CHECK-NEXT: LoaderUninitialized (SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: Lockable (SubjectMatchRule_record)
 // CHECK-NEXT: MIGServerRoutine (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_block)
+// CHECK-NEXT: MSConstexpr (SubjectMatchRule_function)
 // CHECK-NEXT: MSStruct (SubjectMatchRule_record)
 // CHECK-NEXT: MaybeUndef (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: MicroMips (SubjectMatchRule_function)
Index: clang/test/AST/msvc-constexpr-new.cpp
===
--- /dev/null
+++ clang/test/AST/msvc-constexpr-new.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fms-extensions -std=c++20 -ast-dump %s | FileCheck %s
+
+// CHECK: used operator new
+// CHECK: MSConstexprAttr 0x{{[0-9a-f]+}} 
+[[nodiscard]] [[msvc::constexpr]] inline void* __cdecl operator new(decltype(sizeof(void*)), void* p) noexcept { return p; }
+
+// CHECK: used constexpr construct_at
+// CHECK: AttributedStmt 0x{{[0-9a-f]+}} 
+// CHECK-NEXT: MSConstexprAttr 0x{{[0-9a-f]+}} 
+// CHECK-NEXT: ReturnStmt 0x{{[0-9a-f]+}} 
+constexpr int* construct_at(int* p, int v) { [[msvc::constexpr]] return ::new (p) int(v); }
+
+constexpr bool check_construct_at() { int x; return *construct_at(&x, 42) == 42; }
+
+static_assert(check_construct_at());
Index: clang/test/AST/msvc-attrs.cpp
===
--- /dev/null
+++ clang/test/AST/msvc-attrs.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -fms-extensions -std=c++20 -ast-dump %s | FileCheck %s
+
+// [[msvc::constexpr]] tests
+
+// MSConstexprDocs (1)
+// CHECK: used f1 'bool ()'
+// CHECK: MSConstexprAttr 0x{{[0-9a-f]+}} 
+[[msvc::constexpr]] bool f1() { return true; }
+
+// MSConstexprDocs (2)
+// CHECK: used constexpr f2 'bool ()'
+// CHECK: AttributedStmt 0x{{[0-9a-f]+}} 
+// CHECK-NEXT: MSConstexprAttr 0x{{[0-9a-f]+}} 
+// CHECK-NEXT: ReturnStmt 0x{{[0-9a-f]+}} 
+constexpr bool f2() { [[msvc::constexpr]] return f1(); }
+
+static_assert(f2());
+
+constexpr bool f3() { [[msvc::constexpr]] return f1() || f1() && f1(); }
+static_assert(f3());
+
+[[msvc::constexpr]] int f4(int x) { [[msvc::constexpr]] return x > 1 ? 1 + f4(x / 2) : 0; }
+constexpr bool f5() { [[msvc::constexpr]] return f4(32) == 5; }
+static_assert(f5());
+
+[[msvc::constexpr]] int f6(int x)
+{
+switch (x)
+{
+case 42: return 1337;
+default:
+ if (x < 0) [[msvc::constexpr]] return f4(-x);
+ else return x;
+}
+}
+
+constexpr bool f7() { [[msvc::constexpr]] return f6(f6(42) - 133

[PATCH] D148799: [clang] Return `std::string_view` from `TargetInfo::getClobbers()`

[Sviatoslav Osipov via Phabricator via cfe-commits]

Stoorx updated this revision to Diff 515752.
Stoorx edited the summary of this revision.
Stoorx added a comment.

Fix, Rebase & retitle


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

https://reviews.llvm.org/D148799

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/ARC.h
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Basic/Targets/AVR.h
  clang/lib/Basic/Targets/BPF.h
  clang/lib/Basic/Targets/CSKY.h
  clang/lib/Basic/Targets/DirectX.h
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/Basic/Targets/Lanai.h
  clang/lib/Basic/Targets/Le64.h
  clang/lib/Basic/Targets/LoongArch.h
  clang/lib/Basic/Targets/M68k.cpp
  clang/lib/Basic/Targets/M68k.h
  clang/lib/Basic/Targets/MSP430.h
  clang/lib/Basic/Targets/Mips.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/PNaCl.h
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/Sparc.h
  clang/lib/Basic/Targets/SystemZ.h
  clang/lib/Basic/Targets/TCE.h
  clang/lib/Basic/Targets/VE.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/Basic/Targets/XCore.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGStmt.cpp

Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2779,7 +2779,7 @@
  "unwind clobber can't be used with asm goto");
 
   // Add machine specific clobbers
-  std::string MachineClobbers = getTarget().getClobbers();
+  std::string_view MachineClobbers = getTarget().getClobbers();
   if (!MachineClobbers.empty()) {
 if (!Constraints.empty())
   Constraints += ',';
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -939,7 +939,7 @@
 
   // Build the constraints. FIXME: We should support immediates when possible.
   std::string Constraints = "={@ccc},r,r,~{cc},~{memory}";
-  std::string MachineClobbers = CGF.getTarget().getClobbers();
+  std::string_view MachineClobbers = CGF.getTarget().getClobbers();
   if (!MachineClobbers.empty()) {
 Constraints += ',';
 Constraints += MachineClobbers;
@@ -1082,7 +1082,7 @@
   AsmOS << "$0, ${1:y}";
 
   std::string Constraints = "=r,*Z,~{memory}";
-  std::string MachineClobbers = CGF.getTarget().getClobbers();
+  std::string_view MachineClobbers = CGF.getTarget().getClobbers();
   if (!MachineClobbers.empty()) {
 Constraints += ',';
 Constraints += MachineClobbers;
Index: clang/lib/Basic/Targets/XCore.h
===
--- clang/lib/Basic/Targets/XCore.h
+++ clang/lib/Basic/Targets/XCore.h
@@ -49,7 +49,7 @@
 return TargetInfo::VoidPtrBuiltinVaList;
   }
 
-  const char *getClobbers() const override { return ""; }
+  std::string_view getClobbers() const override { return ""; }
 
   ArrayRef getGCCRegNames() const override {
 static const char *const GCCRegNames[] = {
Index: clang/lib/Basic/Targets/X86.h
===
--- clang/lib/Basic/Targets/X86.h
+++ clang/lib/Basic/Targets/X86.h
@@ -262,7 +262,7 @@
StringRef Constraint, unsigned Size) const;
 
   std::string convertConstraint(const char *&Constraint) const override;
-  const char *getClobbers() const override {
+  std::string_view getClobbers() const override {
 return "~{dirflag},~{fpsr},~{flags}";
   }
 
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -130,7 +130,7 @@
 return false;
   }
 
-  const char *getClobbers() const final { return ""; }
+  std::string_view getClobbers() const final { return ""; }
 
   bool isCLZForZeroUndef() const final { return false; }
 
Index: clang/lib/Basic/Targets/VE.h
===
--- clang/lib/Basic/Targets/VE.h
+++ clang/lib/Basic/Targets/VE.h
@@ -69,7 +69,7 @@
 }
   }
 
-  const char *getClobbers() const override { return ""; }
+  std::string_view getClobbers() const override { return ""; }
 
   ArrayRef getGCCRegNames() const override {
 static const char *const GCCRegNames[] = {
Index: clang/lib/Basic/Targets/TCE.h
===
--- clang/lib/Basic/Targets/TCE.h
+++ clang/lib/Basic/Targets/TCE.h
@@ -99,7 +99,7 @@
 return std::nullopt;
   }
 
-  const char *getClobbers() const override { return ""; }
+  std::string_view getClobbers() const override { return ""; }
 
   BuiltinVaListKind getBuiltinVaListKind() const override {
 

[PATCH] D148799: [clang] Return `std::string_view` from `TargetInfo::getClobbers()`

[David Spickett via Phabricator via cfe-commits]

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

If you have commit access already, wait until the pre-commit builds finish and 
check there isn't anything failing there.

If you don't, I can commit this for you as I did before. I'll do that on Monday.


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

https://reviews.llvm.org/D148799

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

[PATCH] D125171: [clang-format] Add a new clang-format option AlwaysBreakBeforeFunctionParameters

[jonathan molinatto via Phabricator via cfe-commits]

jrmolin updated this revision to Diff 515756.
jrmolin added a comment.

a unit test was failing, so I fixed it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125171

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25429,6 +25429,68 @@
   verifyFormat("auto x = 5s .count() == 5;");
 }
 
+TEST_F(FormatTest, BreakBeforeParameterList) {
+  FormatStyle Style = getLLVMStyle();
+  EXPECT_EQ(Style.BreakBeforeFunctionParameters, FormatStyle::FPBS_Leave);
+  Style.BinPackParameters = false;
+
+  // test Leave (basically transparent mode)
+
+  // verify that there is no break by default
+  verifyFormat("int function1();\n" // formatted
+   "int function2(int param1, int param2, int param3);\n"
+   "void function3(int param1, int param2, int param3) {}\n"
+   "int function4(int param1, int param2, int param3);\n",
+   "int function1();\n" // original
+   "int function2(\n"
+   "int param1, int param2, int param3);\n"
+   "void function3(int param1, int param2, int param3) {}\n"
+   "int function4(int param1, int param2, int param3);\n",
+   Style);
+
+  // test Always
+  // verify that there is a break when told to break
+  Style.BreakBeforeFunctionParameters = FormatStyle::FPBS_Always;
+  verifyFormat("int function1(\n"
+   "int param1,\n"
+   "int param2,\n"
+   "int param3);\n"
+   "int function2();\n"
+   "void function3(\n"
+   "int param1,\n"
+   "int param2,\n"
+   "int param3) {}\n"
+   "int function4(\n"
+   "int param1,\n"
+   "int param2,\n"
+   "int param3);\n"
+   "int function5(\n"
+   "int param1,\n"
+   "int param2,\n"
+   "int param3);\n",
+   Style);
+
+  // verify that having no parameters doesn't affect the parentheses
+  verifyFormat("void function1() {}\n", // the formatted part
+   "void function1() {}\n", // the original
+   Style);
+
+  verifyFormat("void function1();\n", // the formatted part
+   "void function1();\n", // the original
+   Style);
+
+  // test Never
+  Style.BreakBeforeFunctionParameters = FormatStyle::FPBS_Never;
+  verifyFormat("int function1();\n" // the formatted part
+   "int function2(int param1, int param2, int param3);\n",
+   "int function1();\n" // the original
+   "int function2(\n"
+   "int param1,\n"
+   "int param2,\n"
+   "int param3);\n",
+   Style);
+}
+
 TEST_F(FormatTest, InterfaceAsClassMemberName) {
   verifyFormat("class Foo {\n"
"  int interface;\n"
Index: clang/unittests/Format/ConfigParseTest.cpp
===
--- clang/unittests/Format/ConfigParseTest.cpp
+++ clang/unittests/Format/ConfigParseTest.cpp
@@ -607,6 +607,17 @@
   CHECK_PARSE("BreakBeforeBraces: Custom", BreakBeforeBraces,
   FormatStyle::BS_Custom);
 
+  CHECK_PARSE("BreakBeforeFunctionParameters: Always",
+  BreakBeforeFunctionParameters, FormatStyle::FPBS_Always);
+  CHECK_PARSE("BreakBeforeFunctionParameters: Leave",
+  BreakBeforeFunctionParameters, FormatStyle::FPBS_Leave);
+  CHECK_PARSE("BreakBeforeFunctionParameters: true",
+  BreakBeforeFunctionParameters, FormatStyle::FPBS_Always);
+  CHECK_PARSE("BreakBeforeFunctionParameters: false",
+  BreakBeforeFunctionParameters, FormatStyle::FPBS_Leave);
+  CHECK_PARSE("BreakBeforeFunctionParameters: Never",
+  BreakBeforeFunctionParameters, FormatStyle::FPBS_Never);
+
   Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Never;
   CHECK_PARSE("BraceWrapping:\n"
   "  AfterControlStatement: MultiLine",
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4867,6 +4867,21 @@
 return true;
   }
 
+  // If BreakBeforeFunctionParameters is Always, we want to break before
+  // the next parameter, if there is one. If it is Leave and a newline exists,
+  // make sure we insert one. Otherwise, no newline.
+  if (Left.is(tok::l_p

[PATCH] D125171: [clang-format] Add a new clang-format option AlwaysBreakBeforeFunctionParameters

[jonathan molinatto via Phabricator via cfe-commits]

jrmolin added inline comments.



Comment at: clang/include/clang/Format/Format.h:851
+  /// The function parameter breaking style to use.
+  /// \version 16.0
+  FunctionParameterBreakingStyle BreakBeforeFunctionParameters;

so should this be 17.0 now? This has gone on quite long.



Comment at: clang/lib/Format/Format.cpp:345
+
+// For backward compatibility.
+IO.enumCase(Value, "false", FormatStyle::FPBS_Leave);

I should probably delete these, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125171

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

cfe-commits@lists.llvm.org

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a reviewer: clang-language-wg.
ilya-biryukov added a comment.

Thanks! I believe we should also recover in the similar manner we do for 
missing `const`, see corresponding comment.

Extra NITs:

- there is a typo in description: *compariosn* should be comparison,
- we probably want to add this change to release notes.

Also adding @clang-language-wg in case someone else wants to chime in.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9434
+def err_volatile_comparison_operator : Error<
+  "defaulted comparison operator function must not be volatile">;
+def err_ref_qualifier_comparison_operator : Error<

NIT: for consistency with wording of notes above.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9436
+def err_ref_qualifier_comparison_operator : Error<
+  "ref-qualifier '&&' is not allowed on defaulted comparison operators">;
 

NIT: for consistency with the wording of `err_ref_qualifier_constructor`



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:8593-8601
+const FunctionProtoType *FnType = 
FD->getType()->castAs();
+if (FnType->isVolatile()) {
+  Diag(FD->getLocation(), diag::err_volatile_comparison_operator);
+  return true;
+}
+if (FnType->getRefQualifier() == RQ_RValue) {
+  Diag(FD->getLocation(), diag::err_ref_qualifier_comparison_operator);

NIT: this version is simpler and more aligned with the code below.
Also, could you move the `volatile` handling after the check for `const`?


Additionally, we seem to recover in case of `const` by adding it to the type 
and suggesting a fix-it to add it in the code.
Could we do the same for `volatile` and `ref-qualifier`, i.e. suggest a fix-it 
to remove the `ref-qualifier` and `volatile` and change the corresponding type 
to make AST pretend they were never there?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148924

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

[PATCH] D148925: Fix discarding void-typed comma expressions

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

tbaeder updated this revision to Diff 515762.

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

https://reviews.llvm.org/D148925

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
@@ -808,6 +808,8 @@
   (a); // expected-warning {{unused}} \
// ref-warning {{unused}}
 
+  (void)5, (void)6;
+
   return 0;
 }
 
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -264,8 +264,12 @@
 
   // Deal with operations which have composite or void types.
   if (BO->isCommaOp()) {
-if (!discard(LHS))
+if (!this->discard(LHS))
   return false;
+if (RHS->getType()->isVoidType())
+  return this->discard(RHS);
+
+// Otherwise, visit RHS and optionally discard its value.
 return Discard(this->visit(RHS));
   }
 
@@ -1650,10 +1654,12 @@
   if (!visit(Exp))
 return false;
 
+  if (Exp->getType()->isVoidType())
+return this->emitRetVoid(Exp);
+
   if (std::optional T = classify(Exp))
 return this->emitRet(*T, Exp);
-  else
-return this->emitRetValue(Exp);
+  return this->emitRetValue(Exp);
 }
 
 /// Toplevel visitDecl().


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -808,6 +808,8 @@
   (a); // expected-warning {{unused}} \
// ref-warning {{unused}}
 
+  (void)5, (void)6;
+
   return 0;
 }
 
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -264,8 +264,12 @@
 
   // Deal with operations which have composite or void types.
   if (BO->isCommaOp()) {
-if (!discard(LHS))
+if (!this->discard(LHS))
   return false;
+if (RHS->getType()->isVoidType())
+  return this->discard(RHS);
+
+// Otherwise, visit RHS and optionally discard its value.
 return Discard(this->visit(RHS));
   }
 
@@ -1650,10 +1654,12 @@
   if (!visit(Exp))
 return false;
 
+  if (Exp->getType()->isVoidType())
+return this->emitRetVoid(Exp);
+
   if (std::optional T = classify(Exp))
 return this->emitRet(*T, Exp);
-  else
-return this->emitRetValue(Exp);
+  return this->emitRetValue(Exp);
 }
 
 /// Toplevel visitDecl().
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148925: Fix discarding void-typed comma expressions

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

tbaeder added a comment.

In D148925#4287218 , @erichkeane 
wrote:

> So things cast to 'void' can have side effects, right?  Does the call to 
> discard still evaluate them?
>
> https://godbolt.org/z/a3ej9bxKe

Yes. `discard(E)` is just `visit(E)`, except that it sets `DiscardResult` to 
`true` for that one level of visiting. Following `visit()` calls will have 
`DiscardResult = false` again.


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

https://reviews.llvm.org/D148925

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

[PATCH] D148925: Fix discarding void-typed comma expressions

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D148925#4287497 , @tbaeder wrote:

> In D148925#4287218 , @erichkeane 
> wrote:
>
>> So things cast to 'void' can have side effects, right?  Does the call to 
>> discard still evaluate them?
>>
>> https://godbolt.org/z/a3ej9bxKe
>
> Yes. `discard(E)` is just `visit(E)`, except that it sets `DiscardResult` to 
> `true` for that one level of visiting. Following `visit()` calls will have 
> `DiscardResult = false` again.

Cool!  Mind adding the tests I proposed?  it would be nice to ensure/show 
side-effects are guaranteed.


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

https://reviews.llvm.org/D148925

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

[PATCH] D134475: [Clang] Add support for [[msvc::constexpr]] C++11 attribute

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Still suspicious about this/whether it models the MSVC implementation 
correctly, but no real implementation concerns.  I REALLY want us to document 
the attribute extensively however.




Comment at: clang/include/clang/Basic/AttrDocs.td:3543
+definition or a return statement. It has no effect on function declarations.
+This attribute permits constant evaluation of ``[[msvc::constexpr]]`` functions
+in ``[[msvc::constexpr]] return`` statements inside ``constexpr`` or

Does it prohibit the inverse?  I think this documentation overall needs a much 
better description of what the semantics are here, particularly anything that 
you found in experimentation on the MS implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134475

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

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov added a comment.

reduced test case (that is currently blocking this diff) :

  namespace outer::internal {
  
  template 
  concept myconcept = true;
  
  }
  
  namespace outer {
  
  template  class Foo;
  
  template  struct Bar {
template  friend class Foo;
  };
  
  Bar S;
  
  namespace internal {
  int f(Foo);
  }
  
  template  struct Foo {
friend int internal::f(Foo);
  };
  
  } // namespace outer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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

[PATCH] D148925: [clang][Interp] Fix discarding void-typed comma expressions

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

tbaeder updated this revision to Diff 515800.

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

https://reviews.llvm.org/D148925

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
@@ -808,9 +808,23 @@
   (a); // expected-warning {{unused}} \
// ref-warning {{unused}}
 
+  (void)5, (void)6;
+
   return 0;
 }
 
+/// Ignored comma expressions still have their
+/// expressions evaluated.
+constexpr int Comma(int start) {
+int i = start;
+
+(void)i++;
+(void)i++,(void)i++;
+return i;
+}
+constexpr int Value = Comma(5);
+static_assert(Value == 8);
+
 #endif
 
 namespace PredefinedExprs {
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -264,8 +264,12 @@
 
   // Deal with operations which have composite or void types.
   if (BO->isCommaOp()) {
-if (!discard(LHS))
+if (!this->discard(LHS))
   return false;
+if (RHS->getType()->isVoidType())
+  return this->discard(RHS);
+
+// Otherwise, visit RHS and optionally discard its value.
 return Discard(this->visit(RHS));
   }
 
@@ -1650,10 +1654,12 @@
   if (!visit(Exp))
 return false;
 
+  if (Exp->getType()->isVoidType())
+return this->emitRetVoid(Exp);
+
   if (std::optional T = classify(Exp))
 return this->emitRet(*T, Exp);
-  else
-return this->emitRetValue(Exp);
+  return this->emitRetValue(Exp);
 }
 
 /// Toplevel visitDecl().


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -808,9 +808,23 @@
   (a); // expected-warning {{unused}} \
// ref-warning {{unused}}
 
+  (void)5, (void)6;
+
   return 0;
 }
 
+/// Ignored comma expressions still have their
+/// expressions evaluated.
+constexpr int Comma(int start) {
+int i = start;
+
+(void)i++;
+(void)i++,(void)i++;
+return i;
+}
+constexpr int Value = Comma(5);
+static_assert(Value == 8);
+
 #endif
 
 namespace PredefinedExprs {
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -264,8 +264,12 @@
 
   // Deal with operations which have composite or void types.
   if (BO->isCommaOp()) {
-if (!discard(LHS))
+if (!this->discard(LHS))
   return false;
+if (RHS->getType()->isVoidType())
+  return this->discard(RHS);
+
+// Otherwise, visit RHS and optionally discard its value.
 return Discard(this->visit(RHS));
   }
 
@@ -1650,10 +1654,12 @@
   if (!visit(Exp))
 return false;
 
+  if (Exp->getType()->isVoidType())
+return this->emitRetVoid(Exp);
+
   if (std::optional T = classify(Exp))
 return this->emitRet(*T, Exp);
-  else
-return this->emitRetValue(Exp);
+  return this->emitRetValue(Exp);
 }
 
 /// Toplevel visitDecl().
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148925: [clang][Interp] Fix discarding void-typed comma expressions

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

tbaeder added a comment.

In D148925#4287502 , @erichkeane 
wrote:

> Cool!  Mind adding the tests I proposed?  it would be nice to ensure/show 
> side-effects are guaranteed.

Sure, thanks for the test case!


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

https://reviews.llvm.org/D148925

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

[PATCH] D146557: [MLIR][OpenMP] Refactoring how map clause is processed

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis updated this revision to Diff 515805.
TIFitis marked an inline comment as done.
TIFitis edited the summary of this revision.
TIFitis added a comment.

Updated tests. Addressed reviewer comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146557

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
  mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp
  mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
  mlir/test/Target/LLVMIR/omptarget-llvm.mlir

Index: mlir/test/Target/LLVMIR/omptarget-llvm.mlir
===
--- mlir/test/Target/LLVMIR/omptarget-llvm.mlir
+++ mlir/test/Target/LLVMIR/omptarget-llvm.mlir
@@ -12,32 +12,24 @@
 }
 
 // CHECK: @.offload_maptypes = private unnamed_addr constant [1 x i64] [i64 3]
+// CHECK: @.offload_sizes = private unnamed_addr constant [1 x i64] [i64 4]
 // CHECK-LABEL: define void @_QPopenmp_target_data() {
 // CHECK: %[[VAL_0:.*]] = alloca [1 x ptr], align 8
 // CHECK: %[[VAL_1:.*]] = alloca [1 x ptr], align 8
-// CHECK: %[[VAL_2:.*]] = alloca [1 x i64], align 8
-// CHECK: %[[VAL_3:.*]] = alloca i32, i64 1, align 4
-// CHECK: br label %[[VAL_4:.*]]
-// CHECK:   entry:; preds = %[[VAL_5:.*]]
-// CHECK: %[[VAL_6:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
-// CHECK: store ptr %[[VAL_3]], ptr %[[VAL_6]], align 8
-// CHECK: %[[VAL_7:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
-// CHECK: store ptr %[[VAL_3]], ptr %[[VAL_7]], align 8
-// CHECK: %[[VAL_8:.*]] = getelementptr inbounds [1 x i64], ptr %[[VAL_2]], i32 0, i32 0
-// CHECK: store i64 ptrtoint (ptr getelementptr (ptr, ptr null, i32 1) to i64), ptr %[[VAL_8]], align 4
+// CHECK: %[[VAL_2:.*]] = alloca i32, i64 1, align 4
+// CHECK: br label %[[VAL_3:.*]]
+// CHECK:   entry:; preds = %[[VAL_4:.*]]
+// CHECK: %[[VAL_5:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
+// CHECK: store ptr %[[VAL_2]], ptr %[[VAL_5]], align 8
+// CHECK: %[[VAL_6:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
+// CHECK: store ptr %[[VAL_2]], ptr %[[VAL_6]], align 8
+// CHECK: %[[VAL_7:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
+// CHECK: %[[VAL_8:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
+// CHECK: call void @__tgt_target_data_begin_mapper(ptr @2, i64 -1, i32 1, ptr %[[VAL_7]], ptr %[[VAL_8]], ptr @.offload_sizes, ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
+// CHECK: store i32 99, ptr %[[VAL_2]], align 4
 // CHECK: %[[VAL_9:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
 // CHECK: %[[VAL_10:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
-// CHECK: %[[VAL_11:.*]] = getelementptr inbounds [1 x i64], ptr %[[VAL_2]], i32 0, i32 0
-// CHECK: call void @__tgt_target_data_begin_mapper(ptr @2, i64 -1, i32 1, ptr %[[VAL_9]], ptr %[[VAL_10]], ptr %[[VAL_11]], ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
-// CHECK: br label %[[VAL_12:.*]]
-// CHECK:   omp.data.region:  ; preds = %[[VAL_4]]
-// CHECK: store i32 99, ptr %[[VAL_3]], align 4
-// CHECK: br label %[[VAL_13:.*]]
-// CHECK:   omp.region.cont:  ; preds = %[[VAL_12]]
-// CHECK: %[[VAL_14:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
-// CHECK: %[[VAL_15:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
-// CHECK: %[[VAL_16:.*]] = getelementptr inbounds [1 x i64], ptr %[[VAL_2]], i32 0, i32 0
-// CHECK: call void @__tgt_target_data_end_mapper(ptr @2, i64 -1, i32 1, ptr %[[VAL_14]], ptr %[[VAL_15]], ptr %[[VAL_16]], ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
+// CHECK: call void @__tgt_target_data_end_mapper(ptr @2, i64 -1, i32 1, ptr %[[VAL_9]], ptr %[[VAL_10]], ptr @.offload_sizes, ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
 // CHECK: ret void
 
 // -
@@ -56,38 +48,30 @@
 }
 
 // CHECK: @.offload_maptypes = private unnamed_addr constant [1 x i64] [i64 2]
+// CHECK: @.offload_sizes = private unnamed_addr constant [1 x i64] [i64 4096]
 // CHECK-LABEL: define void @_QPopenmp_target_data_region
 // CHECK: (ptr %[[ARG_0:.*]]) {
 // CHECK: %[[VAL_0:.*]] = alloca [1 x ptr], align 8
 // CHECK: %[[VAL_1:

[PATCH] D146557: [MLIR][OpenMP] Refactoring how map clause is processed

[Akash Banerjee via Phabricator via cfe-commits]

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



Comment at: mlir/test/Target/LLVMIR/omptarget-llvm.mlir:4-5
-llvm.func @_QPopenmp_target_data() {
-  %0 = llvm.mlir.constant(1 : i64) : i64
-  %1 = llvm.alloca %0 x i32 {bindc_name = "i", in_type = i32, 
operand_segment_sizes = array, uniq_name = 
"_QFopenmp_target_dataEi"} : (i64) -> !llvm.ptr
   omp.target_data   map((tofrom -> %1 : !llvm.ptr)) {

kiranchandramohan wrote:
> TIFitis wrote:
> > kiranchandramohan wrote:
> > > Why is the test changed?
> > Moved the map_operand to function parameter to be consistent with other 
> > tests. I think it can remain unchanged if you prefer.
> Consistency can be achieved in a follow-up NFC patch that you can submit 
> without review. If the test can only show the differences of this patch it is 
> easier to review.
Thanks for letting me know. I've updated the tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146557

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

[PATCH] D146987: [Assignment Tracking] Enable by default

[Steven Johnson via Phabricator via cfe-commits]

srj added a comment.

In D146987#4287137 , @jmorse wrote:

> Ah, you're right, it's actually the same assertion message that @srj reported 
> which I totally skipped over. Currently I suspect that's due to different 
> std::sort implementations.

...`std::stable_sort()`?

(But more seriously, could we please revert all of this unless/until a fix is 
imminent? Our testing is dead in the water at the moment.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146987

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

[PATCH] D146987: [Assignment Tracking] Enable by default

[Jeremy Morse via Phabricator via cfe-commits]

jmorse added a comment.

In D146987#4287794 , @srj wrote:

> (But more seriously, could we please revert all of this unless/until a fix is 
> imminent? Our testing is dead in the water at the moment.)

Should have been reverted in rG0ba922f60046 
 earlier 
today, are you still experiencing the same behaviour with that patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146987

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

[PATCH] D146987: [Assignment Tracking] Enable by default

[Steven Johnson via Phabricator via cfe-commits]

srj added a comment.

In D146987#4287819 , @jmorse wrote:

> Should have been reverted in rG0ba922f60046 
>  earlier 
> today, are you still experiencing the same behaviour with that patch?

Ah, sorry, I didn't see that -- I'll pull and rebuild this morning to verify!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146987

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

[PATCH] D148944: [clang][Driver] Fix crash with unsupported architectures in MinGW and CrossWindows.

[KOMATA Manabu via Phabricator via cfe-commits]

k-mana created this revision.
k-mana added a reviewer: fhahn.
Herald added subscribers: StephenFan, mstorsjo.
Herald added a project: All.
k-mana requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148944

Files:
  clang/lib/Driver/ToolChains/CrossWindows.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/test/Driver/unsupported-target-arch.c


Index: clang/test/Driver/unsupported-target-arch.c
===
--- clang/test/Driver/unsupported-target-arch.c
+++ clang/test/Driver/unsupported-target-arch.c
@@ -23,3 +23,11 @@
 // RUN: not %clang --target=noarch-unknown-nacl -o %t.o %s 2> %t.err
 // RUN: FileCheck --input-file=%t.err --check-prefix=CHECK-NOARCH-NACL %s
 // CHECK-NOARCH-NACL:  error: the target architecture 'noarch' is not 
supported by the target 'Native Client'
+//
+// RUN: not %clang --target=noarch-unknown-windows-gnu -o %t.o %s 2> %t.err
+// RUN: FileCheck --input-file=%t.err --check-prefix=CHECK-NOARCH-MINGW %s
+// CHECK-NOARCH-MINGW: error: unknown target triple 
'noarch-unknown-windows-gnu', please use -triple or -arch
+//
+// RUN: not %clang --target=noarch-unknown-windows-itanium -o %t.o %s 2> %t.err
+// RUN: FileCheck --input-file=%t.err --check-prefix=CHECK-NOARCH-CROSSWINDOWS 
%s
+// CHECK-NOARCH-CROSSWINDOWS: error: unknown target triple 
'noarch-unknown-windows-itanium', please use -triple or -arch
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -135,7 +135,7 @@
 CmdArgs.push_back("arm64pe");
 break;
   default:
-llvm_unreachable("Unsupported target architecture.");
+D.Diag(diag::err_target_unknown_triple) << TC.getEffectiveTriple().str();
   }
 
   Arg *SubsysArg =
Index: clang/lib/Driver/ToolChains/CrossWindows.cpp
===
--- clang/lib/Driver/ToolChains/CrossWindows.cpp
+++ clang/lib/Driver/ToolChains/CrossWindows.cpp
@@ -94,7 +94,8 @@
   CmdArgs.push_back("-m");
   switch (TC.getArch()) {
   default:
-llvm_unreachable("unsupported architecture");
+D.Diag(diag::err_target_unknown_triple) << TC.getEffectiveTriple().str();
+break;
   case llvm::Triple::arm:
   case llvm::Triple::thumb:
 // FIXME: this is incorrect for WinCE


Index: clang/test/Driver/unsupported-target-arch.c
===
--- clang/test/Driver/unsupported-target-arch.c
+++ clang/test/Driver/unsupported-target-arch.c
@@ -23,3 +23,11 @@
 // RUN: not %clang --target=noarch-unknown-nacl -o %t.o %s 2> %t.err
 // RUN: FileCheck --input-file=%t.err --check-prefix=CHECK-NOARCH-NACL %s
 // CHECK-NOARCH-NACL:  error: the target architecture 'noarch' is not supported by the target 'Native Client'
+//
+// RUN: not %clang --target=noarch-unknown-windows-gnu -o %t.o %s 2> %t.err
+// RUN: FileCheck --input-file=%t.err --check-prefix=CHECK-NOARCH-MINGW %s
+// CHECK-NOARCH-MINGW: error: unknown target triple 'noarch-unknown-windows-gnu', please use -triple or -arch
+//
+// RUN: not %clang --target=noarch-unknown-windows-itanium -o %t.o %s 2> %t.err
+// RUN: FileCheck --input-file=%t.err --check-prefix=CHECK-NOARCH-CROSSWINDOWS %s
+// CHECK-NOARCH-CROSSWINDOWS: error: unknown target triple 'noarch-unknown-windows-itanium', please use -triple or -arch
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -135,7 +135,7 @@
 CmdArgs.push_back("arm64pe");
 break;
   default:
-llvm_unreachable("Unsupported target architecture.");
+D.Diag(diag::err_target_unknown_triple) << TC.getEffectiveTriple().str();
   }
 
   Arg *SubsysArg =
Index: clang/lib/Driver/ToolChains/CrossWindows.cpp
===
--- clang/lib/Driver/ToolChains/CrossWindows.cpp
+++ clang/lib/Driver/ToolChains/CrossWindows.cpp
@@ -94,7 +94,8 @@
   CmdArgs.push_back("-m");
   switch (TC.getArch()) {
   default:
-llvm_unreachable("unsupported architecture");
+D.Diag(diag::err_target_unknown_triple) << TC.getEffectiveTriple().str();
+break;
   case llvm::Triple::arm:
   case llvm::Triple::thumb:
 // FIXME: this is incorrect for WinCE
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129835: [clang] adds a discardable attribute

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

>> With [[discardable]] one just needs to push/pop at the extremes of a file 
>> (and if a future version of module maps supports global pragmas for a 
>> module, there too, but that's a discussion that requires a design doc).
>
> I understood that, I just don't think that's a good thing. This is basically 
> an attribute that says "I know we said we wanted everything here to be 
> nodiscard, but JUST KIDDING not this one!" which is not a very clean approach 
> to writing headers.

@aaron.ballman Ugh, I've finally come up with a good use-case for 
`[[discardable]]`.

  class [[nodiscard]] iterator {
  public:
// usual iterator stuff
  
[[clang::discardable]] iterator operator++(int);
[[clang::discardable]] iterator operator--(int);
  };

I hate it, but the alternative is to decorate everything else with 
`[[nodiscard]]` just to facilitate these two operators. It's a compelling 
use-case, but I'm not sure if it's compelling enough on its own. WDYT?
(I personally think that those two should be nodiscard, but `i++` is pervasive 
enough that it might never be practical to correct.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129835

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

[PATCH] D148276: [clang] trigger -Wcast-qual on functional casts

[Thurston Dang via Phabricator via cfe-commits]

thurston added a comment.

I think this change is triggering an error on a buildbot 
(https://lab.llvm.org/buildbot/#/builders/37/builds/21631/steps/20/logs/stdio):

  
/b/sanitizer-x86_64-linux/build/llvm-project/clang/lib/Analysis/UnsafeBufferUsage.cpp:1348:45:
 error: cast from 'const clang::ASTContext' to 'clang::ASTContext &' drops 
const qualifier [-Werror,-Wcast-qual]
std::remove_const_t(Ctx),

even though the code is deliberately trying to remove the const qualifier the 
right (?) way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148276

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

[PATCH] D147844: Emit warning when implicit cast from int to bool happens in an conditional operator expression

[Mark de Wever via Phabricator via cfe-commits]

Mordante accepted this revision.
Mordante added a comment.

I too am not really convinced by all changes in libc++. On the other hand I 
don't think the change make the libc++ code looking bad. So I don't feel a 
reason to reject the libc++ changes.

I don't like the commit message, there's only a title and a bug report. Please 
add a bit more information in the message, with an example of code that now 
gives a diagnostic.

Note I only reviewed the libc++ changes, I leave the clang part to 
@aaron.ballman.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

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

[PATCH] D148817: [RISCV] Add Tag_RISCV_arch attribute by default when using clang as an assembler.

[Alex Bradbury via Phabricator via cfe-commits]

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

I'd appreciate some riscv32 RUN lines for completeness, but otherwise LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148817

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