[PATCH] D125683: [runtimes] Replace LIBCXX_ENABLE_STATIC_ABI_LIBRARY & friends by a new LIBCXX_CXX_ABI choice

[Petr Hosek via Phabricator via cfe-commits]

phosek added inline comments.



Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:124
 set(RUNTIMES_${target}_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
-set(RUNTIMES_${target}_LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
 set(RUNTIMES_${target}_LIBCXX_ABI_VERSION 2 CACHE STRING "")

ldionne wrote:
> Note that I am removing these options here because I don't think they are 
> required -- since we specify `LIBCXXABI_ENABLE_SHARED=OFF`, there is no 
> shared libc++abi to link against, so we should already be linking against 
> `libc++abi.a` regardless of this change.
This option was set to merge `libc++abi.a` into `libc++.a` so to achieve the 
same effect, presumably we would need to set 
`-DLIBCXX_CXX_ABI=libcxxabi-objects`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125683

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

[PATCH] D125765: [RISCV] Add type aliases float16_t, float32_t and float64_t

[Wang Pengcheng via Phabricator via cfe-commits]

pcwang-thead updated this revision to Diff 430246.
pcwang-thead added a comment.
Herald added a subscriber: MaskRay.

Add a test to check RVV type aliases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125765

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-type-aliases.c

Index: clang/test/CodeGen/RISCV/rvv-type-aliases.c
===
--- /dev/null
+++ clang/test/CodeGen/RISCV/rvv-type-aliases.c
@@ -0,0 +1,155 @@
+// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d -target-feature +v \
+// RUN: -target-feature +zfh -target-feature +experimental-zvfh \
+// RUN: -fsyntax-only -verify -ast-dump %s | FileCheck %s
+
+#include 
+
+// expected-no-diagnostics
+void bar(void) {
+  // CHECK: f16 'float16_t':'_Float16'
+  float16_t f16;
+  // CHECK: f32 'float32_t':'float'
+  float32_t f32;
+  // CHECK: f64 'float64_t':'double'
+  float64_t f64;
+
+  // CHECK: b1 'vbool1_t':'__rvv_bool1_t'
+  vbool1_t b1;
+  // CHECK: b2 'vbool2_t':'__rvv_bool2_t'
+  vbool2_t b2;
+  // CHECK: b4 'vbool4_t':'__rvv_bool4_t'
+  vbool4_t b4;
+  // CHECK: b8 'vbool8_t':'__rvv_bool8_t'
+  vbool8_t b8;
+  // CHECK: b16 'vbool16_t':'__rvv_bool16_t'
+  vbool16_t b16;
+  // CHECK: b32 'vbool32_t':'__rvv_bool32_t'
+  vbool32_t b32;
+  // CHECK: b64 'vbool64_t':'__rvv_bool64_t'
+  vbool64_t b64;
+
+  // CHECK: i8mf8 'vint8mf8_t':'__rvv_int8mf8_t'
+  vint8mf8_t i8mf8;
+  // CHECK: u8mf8 'vuint8mf8_t':'__rvv_uint8mf8_t'
+  vuint8mf8_t u8mf8;
+  // CHECK: i8mf4 'vint8mf4_t':'__rvv_int8mf4_t'
+  vint8mf4_t i8mf4;
+  // CHECK: u8mf4 'vuint8mf4_t':'__rvv_uint8mf4_t'
+  vuint8mf4_t u8mf4;
+  // CHECK: i8mf2 'vint8mf2_t':'__rvv_int8mf2_t'
+  vint8mf2_t i8mf2;
+  // CHECK: u8mf2 'vuint8mf2_t':'__rvv_uint8mf2_t'
+  vuint8mf2_t u8mf2;
+  // CHECK: i8m1 'vint8m1_t':'__rvv_int8m1_t'
+  vint8m1_t i8m1;
+  // CHECK: u8m1 'vuint8m1_t':'__rvv_uint8m1_t'
+  vuint8m1_t u8m1;
+  // CHECK: i8m2 'vint8m2_t':'__rvv_int8m2_t'
+  vint8m2_t i8m2;
+  // CHECK: u8m2 'vuint8m2_t':'__rvv_uint8m2_t'
+  vuint8m2_t u8m2;
+  // CHECK: i8m4 'vint8m4_t':'__rvv_int8m4_t'
+  vint8m4_t i8m4;
+  // CHECK: u8m4 'vuint8m4_t':'__rvv_uint8m4_t'
+  vuint8m4_t u8m4;
+  // CHECK: i8m8 'vint8m8_t':'__rvv_int8m8_t'
+  vint8m8_t i8m8;
+  // CHECK: u8m8 'vuint8m8_t':'__rvv_uint8m8_t'
+  vuint8m8_t u8m8;
+
+  // CHECK: i16mf4 'vint16mf4_t':'__rvv_int16mf4_t'
+  vint16mf4_t i16mf4;
+  // CHECK: u16mf4 'vuint16mf4_t':'__rvv_uint16mf4_t'
+  vuint16mf4_t u16mf4;
+  // CHECK: i16mf2 'vint16mf2_t':'__rvv_int16mf2_t'
+  vint16mf2_t i16mf2;
+  // CHECK: u16mf2 'vuint16mf2_t':'__rvv_uint16mf2_t'
+  vuint16mf2_t u16mf2;
+  // CHECK: i16m1 'vint16m1_t':'__rvv_int16m1_t'
+  vint16m1_t i16m1;
+  // CHECK: u16m1 'vuint16m1_t':'__rvv_uint16m1_t'
+  vuint16m1_t u16m1;
+  // CHECK: i16m2 'vint16m2_t':'__rvv_int16m2_t'
+  vint16m2_t i16m2;
+  // CHECK: u16m2 'vuint16m2_t':'__rvv_uint16m2_t'
+  vuint16m2_t u16m2;
+  // CHECK: i16m4 'vint16m4_t':'__rvv_int16m4_t'
+  vint16m4_t i16m4;
+  // CHECK: u16m4 'vuint16m4_t':'__rvv_uint16m4_t'
+  vuint16m4_t u16m4;
+  // CHECK: i16m8 'vint16m8_t':'__rvv_int16m8_t'
+  vint16m8_t i16m8;
+  // CHECK: u16m8 'vuint16m8_t':'__rvv_uint16m8_t'
+  vuint16m8_t u16m8;
+
+  // CHECK: i32mf2 'vint32mf2_t':'__rvv_int32mf2_t'
+  vint32mf2_t i32mf2;
+  // CHECK: u32mf2 'vuint32mf2_t':'__rvv_uint32mf2_t'
+  vuint32mf2_t u32mf2;
+  // CHECK: i32m1 'vint32m1_t':'__rvv_int32m1_t'
+  vint32m1_t i32m1;
+  // CHECK: u32m1 'vuint32m1_t':'__rvv_uint32m1_t'
+  vuint32m1_t u32m1;
+  // CHECK: i32m2 'vint32m2_t':'__rvv_int32m2_t'
+  vint32m2_t i32m2;
+  // CHECK: u32m2 'vuint32m2_t':'__rvv_uint32m2_t'
+  vuint32m2_t u32m2;
+  // CHECK: i32m4 'vint32m4_t':'__rvv_int32m4_t'
+  vint32m4_t i32m4;
+  // CHECK: u32m4 'vuint32m4_t':'__rvv_uint32m4_t'
+  vuint32m4_t u32m4;
+  // CHECK: i32m8 'vint32m8_t':'__rvv_int32m8_t'
+  vint32m8_t i32m8;
+  // CHECK: u32m8 'vuint32m8_t':'__rvv_uint32m8_t'
+  vuint32m8_t u32m8;
+
+  // CHECK: i64m1 'vint64m1_t':'__rvv_int64m1_t'
+  vint64m1_t i64m1;
+  // CHECK: u64m1 'vuint64m1_t':'__rvv_uint64m1_t'
+  vuint64m1_t u64m1;
+  // CHECK: i64m2 'vint64m2_t':'__rvv_int64m2_t'
+  vint64m2_t i64m2;
+  // CHECK: u64m2 'vuint64m2_t':'__rvv_uint64m2_t'
+  vuint64m2_t u64m2;
+  // CHECK: i64m4 'vint64m4_t':'__rvv_int64m4_t'
+  vint64m4_t i64m4;
+  // CHECK: u64m4 'vuint64m4_t':'__rvv_uint64m4_t'
+  vuint64m4_t u64m4;
+  // CHECK: i64m8 'vint64m8_t':'__rvv_int64m8_t'
+  vint64m8_t i64m8;
+  // CHECK: u64m8 'vuint64m8_t':'__rvv_uint64m8_t'
+  vuint64m8_t u64m8;
+
+  // CHECK: f16mf4 'vfloat16mf4_t':'__rvv_float16mf4_t'
+  vfloat16mf4_t f16mf4;
+  // CHECK: f16mf2 'vfloat16mf2_t':'__rvv_float16mf2_t'
+  vfloat16mf2_t f16mf2;
+  // CHECK: f16m1 'vfloat16m1_t':'__rvv_float16m1_t'
+  vfloat16m1_t f16m1;
+  // CHECK: f16m2 'vfloat16m2_t':'__rvv_float16m2_t'
+  vfloat16m2_t f16m2;
+  // C

[PATCH] D123676: [clang-format] Fix WhitespaceSensitiveMacros not being honoured when macro closing parenthesis is followed by a newline.

[Marek Kurdej via Phabricator via cfe-commits]

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

Reverted for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123676

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

[clang] 573a5b5 - Revert "[clang-format] Fix WhitespaceSensitiveMacros not being honoured when macro closing parenthesis is followed by a newline."

[Marek Kurdej via cfe-commits]

Author: Marek Kurdej
Date: 2022-05-18T07:27:45+02:00
New Revision: 573a5b58001d6dd86d404832b7b1c45a1b4f4c55

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

LOG: Revert "[clang-format] Fix WhitespaceSensitiveMacros not being honoured 
when macro closing parenthesis is followed by a newline."

This reverts commit 50cd52d9357224cce66a9e00c9a0417c658a5655.

It provoked regressions in C++ and ObjectiveC as described in 
https://reviews.llvm.org/D123676#3515949.

Reproducers:
```
MACRO_BEGIN
#if A
int f();
#else
int f();
#endif
```

```
NS_SWIFT_NAME(A)
@interface B : C
@property(readonly) D value;
@end
```

Added: 


Modified: 
clang/lib/Format/FormatTokenLexer.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/FormatTokenLexer.cpp 
b/clang/lib/Format/FormatTokenLexer.cpp
index d1d236cf032b2..187b30fd55a7e 100644
--- a/clang/lib/Format/FormatTokenLexer.cpp
+++ b/clang/lib/Format/FormatTokenLexer.cpp
@@ -1027,10 +1027,7 @@ FormatToken *FormatTokenLexer::getNextToken() {
   Tokens.back()->Tok.getIdentifierInfo()->getPPKeywordID() ==
   tok::pp_define) &&
 it != Macros.end()) {
-  if (it->second == TT_UntouchableMacroFunc)
-FormatTok->setFinalizedType(TT_UntouchableMacroFunc);
-  else
-FormatTok->setType(it->second);
+  FormatTok->setType(it->second);
   if (it->second == TT_IfMacro) {
 // The lexer token currently has type tok::kw_unknown. However, for 
this
 // substitution to be treated correctly in the TokenAnnotator, faking

diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index bde543131931e..be081a9189600 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -1839,8 +1839,7 @@ void 
UnwrappedLineParser::parseStructuralElement(IfStmtKind *IfKind,
 : CommentsBeforeNextToken.front()->NewlinesBefore > 0;
 
 if (FollowedByNewline && (Text.size() >= 5 || FunctionLike) &&
-tokenCanStartNewLine(*FormatTok) && Text == Text.upper() &&
-!PreviousToken->isTypeFinalized()) {
+tokenCanStartNewLine(*FormatTok) && Text == Text.upper()) {
   PreviousToken->setFinalizedType(TT_FunctionLikeOrFreestandingMacro);
   addUnwrappedLine();
   return;

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 22c46a129403b..e54a6db2ca46b 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -23661,11 +23661,6 @@ TEST_F(FormatTest, WhitespaceSensitiveMacros) {
 
   // Don't use the helpers here, since 'mess up' will change the whitespace
   // and these are all whitespace sensitive by definition
-
-  // Newlines are important here.
-  EXPECT_EQ("FOO(1+2  );\n", format("FOO(1+2  );\n", Style));
-  EXPECT_EQ("FOO(1+2  )\n", format("FOO(1+2  )\n", Style));
-
   EXPECT_EQ("FOO(String-ized&Messy+But(: :Still)=Intentional);",
 format("FOO(String-ized&Messy+But(: :Still)=Intentional);", 
Style));
   EXPECT_EQ(



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

[PATCH] D125848: [clang-format] Handle attributes in enum declaration.

[Tyler Chatow via Phabricator via cfe-commits]

tchatow created this revision.
Herald added a project: All.
tchatow requested review of this revision.
Herald added a project: clang.

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

Ensures that attributes in the enum declaration are interpreted
correctly, for instance:

enum class [[nodiscard]] E {

  a,
  b

};


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125848

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3565,6 +3565,7 @@
   verifyFormat("enum X E {} d;");
   verifyFormat("enum __attribute__((...)) E {} d;");
   verifyFormat("enum __declspec__((...)) E {} d;");
+  verifyFormat("enum [[nodiscard]] E {} d;");
   verifyFormat("enum {\n"
"  Bar = Foo::value\n"
"};",
@@ -3658,6 +3659,7 @@
   verifyFormat("enum struct X E {} d;");
   verifyFormat("enum struct __attribute__((...)) E {} d;");
   verifyFormat("enum struct __declspec__((...)) E {} d;");
+  verifyFormat("enum struct [[nodiscard]] E {} d;");
   verifyFormat("enum struct X f() {\n  a();\n  return 42;\n}");
 }
 
@@ -3675,6 +3677,7 @@
   verifyFormat("enum class X E {} d;");
   verifyFormat("enum class __attribute__((...)) E {} d;");
   verifyFormat("enum class __declspec__((...)) E {} d;");
+  verifyFormat("enum class [[nodiscard]] E {} d;");
   verifyFormat("enum class X f() {\n  a();\n  return 42;\n}");
 }
 
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3360,11 +3360,18 @@
 
   while (FormatTok->Tok.getIdentifierInfo() ||
  FormatTok->isOneOf(tok::colon, tok::coloncolon, tok::less,
-tok::greater, tok::comma, tok::question)) {
+tok::greater, tok::comma, tok::question,
+tok::l_square, tok::r_square)) {
 nextToken();
 // We can have macros or attributes in between 'enum' and the enum name.
 if (FormatTok->is(tok::l_paren))
   parseParens();
+if (FormatTok->is(TT_AttributeSquare)) {
+  parseSquare();
+  // Consume the closing TT_AttributeSquare.
+  if (FormatTok->Next && FormatTok->is(TT_AttributeSquare))
+nextToken();
+}
 if (FormatTok->is(tok::identifier)) {
   nextToken();
   // If there are two identifiers in a row, this is likely an elaborate


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3565,6 +3565,7 @@
   verifyFormat("enum X E {} d;");
   verifyFormat("enum __attribute__((...)) E {} d;");
   verifyFormat("enum __declspec__((...)) E {} d;");
+  verifyFormat("enum [[nodiscard]] E {} d;");
   verifyFormat("enum {\n"
"  Bar = Foo::value\n"
"};",
@@ -3658,6 +3659,7 @@
   verifyFormat("enum struct X E {} d;");
   verifyFormat("enum struct __attribute__((...)) E {} d;");
   verifyFormat("enum struct __declspec__((...)) E {} d;");
+  verifyFormat("enum struct [[nodiscard]] E {} d;");
   verifyFormat("enum struct X f() {\n  a();\n  return 42;\n}");
 }
 
@@ -3675,6 +3677,7 @@
   verifyFormat("enum class X E {} d;");
   verifyFormat("enum class __attribute__((...)) E {} d;");
   verifyFormat("enum class __declspec__((...)) E {} d;");
+  verifyFormat("enum class [[nodiscard]] E {} d;");
   verifyFormat("enum class X f() {\n  a();\n  return 42;\n}");
 }
 
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3360,11 +3360,18 @@
 
   while (FormatTok->Tok.getIdentifierInfo() ||
  FormatTok->isOneOf(tok::colon, tok::coloncolon, tok::less,
-tok::greater, tok::comma, tok::question)) {
+tok::greater, tok::comma, tok::question,
+tok::l_square, tok::r_square)) {
 nextToken();
 // We can have macros or attributes in between 'enum' and the enum name.
 if (FormatTok->is(tok::l_paren))
   parseParens();
+if (FormatTok->is(TT_AttributeSquare)) {
+  parseSquare();
+  // Consume the closing TT_AttributeSquare.
+  if (FormatTok->Next && FormatTok->is(TT_AttributeSquare))
+nextToken();
+}
 if (FormatTok->is(tok::identifier)) {
   nextToken();
   // If there are two identifiers in a row, this is likely an elaborate
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124977: [NFC][Clang] Modify expect of fail test or XFAIL because CSKY align is different

[Zixuan Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdca37af061fb: [NFC][Clang] Modify expect of fail test or 
XFAIL because CSKY align is different (authored by zixuan-wu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124977

Files:
  clang/test/CodeGen/c-strings.c
  clang/test/Sema/builtin-alloca-with-align.c


Index: clang/test/Sema/builtin-alloca-with-align.c
===
--- clang/test/Sema/builtin-alloca-with-align.c
+++ clang/test/Sema/builtin-alloca-with-align.c
@@ -30,4 +30,8 @@
 
 void test8(void) {
   __builtin_alloca_with_align(sizeof(__INT64_TYPE__), 
__alignof__(__INT64_TYPE__)); // expected-warning {{second argument to 
__builtin_alloca_with_align is supposed to be in bits}}
+#if defined(__csky__)
+  // expected-error@-1 {{requested alignment must be 8 or greater}}
+  // Because the alignment of long long is 4 in CSKY target
+#endif
 }
Index: clang/test/CodeGen/c-strings.c
===
--- clang/test/CodeGen/c-strings.c
+++ clang/test/CodeGen/c-strings.c
@@ -20,6 +20,11 @@
 // fails the check for "@f3.x = ... align [ALIGN]", since ALIGN is derived
 // from the alignment of a single i8, which is still 1.
 
+// XFAIL: csky
+// CSKY aligns arrays of size 4+ bytes to a 32-bit boundary, which
+// fails the check for "@f2.x = ... align [ALIGN]", since ALIGN is derived
+// from the alignment of a single i8, which is still 1.
+
 #if defined(__s390x__)
 unsigned char align = 2;
 #else
@@ -69,4 +74,3 @@
 }
 
 char x[3] = "ola";
-


Index: clang/test/Sema/builtin-alloca-with-align.c
===
--- clang/test/Sema/builtin-alloca-with-align.c
+++ clang/test/Sema/builtin-alloca-with-align.c
@@ -30,4 +30,8 @@
 
 void test8(void) {
   __builtin_alloca_with_align(sizeof(__INT64_TYPE__), __alignof__(__INT64_TYPE__)); // expected-warning {{second argument to __builtin_alloca_with_align is supposed to be in bits}}
+#if defined(__csky__)
+  // expected-error@-1 {{requested alignment must be 8 or greater}}
+  // Because the alignment of long long is 4 in CSKY target
+#endif
 }
Index: clang/test/CodeGen/c-strings.c
===
--- clang/test/CodeGen/c-strings.c
+++ clang/test/CodeGen/c-strings.c
@@ -20,6 +20,11 @@
 // fails the check for "@f3.x = ... align [ALIGN]", since ALIGN is derived
 // from the alignment of a single i8, which is still 1.
 
+// XFAIL: csky
+// CSKY aligns arrays of size 4+ bytes to a 32-bit boundary, which
+// fails the check for "@f2.x = ... align [ALIGN]", since ALIGN is derived
+// from the alignment of a single i8, which is still 1.
+
 #if defined(__s390x__)
 unsigned char align = 2;
 #else
@@ -69,4 +74,3 @@
 }
 
 char x[3] = "ola";
-
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] dca37af - [NFC][Clang] Modify expect of fail test or XFAIL because CSKY align is different

[Zi Xuan Wu via cfe-commits]

Author: Zi Xuan Wu (Zeson)
Date: 2022-05-18T10:53:30+08:00
New Revision: dca37af061fbe6590399dfb4e3fd3dda16d63144

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

LOG: [NFC][Clang] Modify expect of fail test or XFAIL because CSKY align is 
different

CSKY is always in 4-byte align, no matter it's long long type.
For global aggregate variable, it's 4-byte align if its size is bigger than or 
equal to 4 bytes.

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

Added: 


Modified: 
clang/test/CodeGen/c-strings.c
clang/test/Sema/builtin-alloca-with-align.c

Removed: 




diff  --git a/clang/test/CodeGen/c-strings.c b/clang/test/CodeGen/c-strings.c
index 085380101554..e783b6cb6ad0 100644
--- a/clang/test/CodeGen/c-strings.c
+++ b/clang/test/CodeGen/c-strings.c
@@ -20,6 +20,11 @@
 // fails the check for "@f3.x = ... align [ALIGN]", since ALIGN is derived
 // from the alignment of a single i8, which is still 1.
 
+// XFAIL: csky
+// CSKY aligns arrays of size 4+ bytes to a 32-bit boundary, which
+// fails the check for "@f2.x = ... align [ALIGN]", since ALIGN is derived
+// from the alignment of a single i8, which is still 1.
+
 #if defined(__s390x__)
 unsigned char align = 2;
 #else
@@ -69,4 +74,3 @@ void f4(void) {
 }
 
 char x[3] = "ola";
-

diff  --git a/clang/test/Sema/builtin-alloca-with-align.c 
b/clang/test/Sema/builtin-alloca-with-align.c
index cac171f0e7ea..0a3e0f6a2022 100644
--- a/clang/test/Sema/builtin-alloca-with-align.c
+++ b/clang/test/Sema/builtin-alloca-with-align.c
@@ -30,4 +30,8 @@ void test7(int a) {
 
 void test8(void) {
   __builtin_alloca_with_align(sizeof(__INT64_TYPE__), 
__alignof__(__INT64_TYPE__)); // expected-warning {{second argument to 
__builtin_alloca_with_align is supposed to be in bits}}
+#if defined(__csky__)
+  // expected-error@-1 {{requested alignment must be 8 or greater}}
+  // Because the alignment of long long is 4 in CSKY target
+#endif
 }



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

[PATCH] D125585: [HLSL][clang][Driver] Parse target profile early to update Driver::TargetTriple.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 430211.
python3kgae added a comment.

Use llvm::Optional.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125585

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/HLSL.cpp
  clang/lib/Driver/ToolChains/HLSL.h
  clang/unittests/Driver/ToolChainTest.cpp

Index: clang/unittests/Driver/ToolChainTest.cpp
===
--- clang/unittests/Driver/ToolChainTest.cpp
+++ clang/unittests/Driver/ToolChainTest.cpp
@@ -387,6 +387,28 @@
   std::vector> Errors;
 };
 
+static void validateTargetProfile(StringRef TargetProfile,
+  StringRef ExpectTriple, Driver &TheDriver,
+  DiagnosticsEngine &Diags) {
+  EXPECT_TRUE(TheDriver.BuildCompilation(
+  {"clang", "--driver-mode=dxc", TargetProfile.data(), "foo.hlsl"}));
+  EXPECT_STREQ(TheDriver.getTargetTriple().c_str(), ExpectTriple.data());
+  EXPECT_EQ(Diags.getNumErrors(), 0u);
+}
+
+static void validateTargetProfile(StringRef TargetProfile,
+  StringRef ExpectError, Driver &TheDriver,
+  DiagnosticsEngine &Diags,
+  SimpleDiagnosticConsumer *DiagConsumer,
+  unsigned NumOfErrors) {
+  EXPECT_TRUE(TheDriver.BuildCompilation(
+  {"clang", "--driver-mode=dxc", TargetProfile.data(), "foo.hlsl"}));
+  EXPECT_EQ(Diags.getNumErrors(), NumOfErrors);
+  EXPECT_STREQ(DiagConsumer->Errors.back().c_str(), ExpectError.data());
+  Diags.Clear();
+  DiagConsumer->clear();
+}
+
 TEST(DxcModeTest, TargetProfileValidation) {
   IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
 
@@ -400,111 +422,38 @@
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
   DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagConsumer);
   Driver TheDriver("/bin/clang", "", Diags, "", InMemoryFileSystem);
-  std::unique_ptr C(
-  TheDriver.BuildCompilation({"clang", "--driver-mode=dxc", "foo.hlsl"}));
-  EXPECT_TRUE(C);
-  EXPECT_TRUE(!C->containsError());
-
-  auto &TC = C->getDefaultToolChain();
-  bool ContainsError = false;
-  auto Args = TheDriver.ParseArgStrings({"-Tvs_6_0"}, false, ContainsError);
-  EXPECT_FALSE(ContainsError);
-  auto Triple = TC.ComputeEffectiveClangTriple(Args);
-  EXPECT_STREQ(Triple.c_str(), "dxil--shadermodel6.0-vertex");
-  EXPECT_EQ(Diags.getNumErrors(), 0u);
 
-  Args = TheDriver.ParseArgStrings({"-Ths_6_1"}, false, ContainsError);
-  EXPECT_FALSE(ContainsError);
-  Triple = TC.ComputeEffectiveClangTriple(Args);
-  EXPECT_STREQ(Triple.c_str(), "dxil--shadermodel6.1-hull");
-  EXPECT_EQ(Diags.getNumErrors(), 0u);
-
-  Args = TheDriver.ParseArgStrings({"-Tds_6_2"}, false, ContainsError);
-  EXPECT_FALSE(ContainsError);
-  Triple = TC.ComputeEffectiveClangTriple(Args);
-  EXPECT_STREQ(Triple.c_str(), "dxil--shadermodel6.2-domain");
-  EXPECT_EQ(Diags.getNumErrors(), 0u);
-
-  Args = TheDriver.ParseArgStrings({"-Tds_6_2"}, false, ContainsError);
-  EXPECT_FALSE(ContainsError);
-  Triple = TC.ComputeEffectiveClangTriple(Args);
-  EXPECT_STREQ(Triple.c_str(), "dxil--shadermodel6.2-domain");
-  EXPECT_EQ(Diags.getNumErrors(), 0u);
-
-  Args = TheDriver.ParseArgStrings({"-Tgs_6_3"}, false, ContainsError);
-  EXPECT_FALSE(ContainsError);
-  Triple = TC.ComputeEffectiveClangTriple(Args);
-  EXPECT_STREQ(Triple.c_str(), "dxil--shadermodel6.3-geometry");
-  EXPECT_EQ(Diags.getNumErrors(), 0u);
-
-  Args = TheDriver.ParseArgStrings({"-Tps_6_4"}, false, ContainsError);
-  EXPECT_FALSE(ContainsError);
-  Triple = TC.ComputeEffectiveClangTriple(Args);
-  EXPECT_STREQ(Triple.c_str(), "dxil--shadermodel6.4-pixel");
-  EXPECT_EQ(Diags.getNumErrors(), 0u);
-
-  Args = TheDriver.ParseArgStrings({"-Tcs_6_5"}, false, ContainsError);
-  EXPECT_FALSE(ContainsError);
-  Triple = TC.ComputeEffectiveClangTriple(Args);
-  EXPECT_STREQ(Triple.c_str(), "dxil--shadermodel6.5-compute");
-  EXPECT_EQ(Diags.getNumErrors(), 0u);
-
-  Args = TheDriver.ParseArgStrings({"-Tms_6_6"}, false, ContainsError);
-  EXPECT_FALSE(ContainsError);
-  Triple = TC.ComputeEffectiveClangTriple(Args);
-  EXPECT_STREQ(Triple.c_str(), "dxil--shadermodel6.6-mesh");
-  EXPECT_EQ(Diags.getNumErrors(), 0u);
-
-  Args = TheDriver.ParseArgStrings({"-Tas_6_7"}, false, ContainsError);
-  EXPECT_FALSE(ContainsError);
-  Triple = TC.ComputeEffectiveClangTriple(Args);
-  EXPECT_STREQ(Triple.c_str(), "dxil--shadermodel6.7-amplification");
-  EXPECT_EQ(Diags.getNumErrors(), 0u);
-
-  Args = TheDriver.ParseArgStrings({"-Tlib_6_x"}, false, ContainsError);
-  EXPECT_FALSE(ContainsError);
-  Triple = TC.ComputeEffectiveClangTriple(Args);
-  EXPECT_STREQ(Triple.c_str(), "dxil--shadermodel6.15-library");
-  EXPECT_EQ(Diags.getNumErrors(), 0u);
+  validateTargetProfile("-Tvs_6_0", "dxil--shadermodel6.0-vertex", TheDriver,
+Diags);
+

[PATCH] D125557: [APInt] Remove all uses of zextOrSelf, sextOrSelf and truncOrSelf

[Chris Lattner via Phabricator via cfe-commits]

lattner added inline comments.



Comment at: llvm/lib/Analysis/ConstantFolding.cpp:2884
 if (IntrinsicID == Intrinsic::smul_fix_sat) {
-  APInt Max = APInt::getSignedMaxValue(Width).sextOrSelf(ExtendedWidth);
-  APInt Min = APInt::getSignedMinValue(Width).sextOrSelf(ExtendedWidth);
+  APInt Max = APInt::getSignedMaxValue(Width).sext(ExtendedWidth);
+  APInt Min = APInt::getSignedMinValue(Width).sext(ExtendedWidth);

foad wrote:
> lattner wrote:
> > I think this can be a zext given the top bit will be zero
> Sure the first one could be zext, but the second one can't be, so it feels 
> conceptually simpler (to me) to keep them both as sext.
likewise, I'm fine with this either way.  zext is slightly more "Strength 
reduced" than sext, but it doesn't matter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125557

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

[PATCH] D124244: [analyzer] add StoreToImmutable and ModelConstQualifiedReturn checkers

[Zurab Tsinadze via Phabricator via cfe-commits]

zukatsinadze added a comment.

In D124244#3493307 , @martong wrote:

>> The patch adds two new checkers.
>
> I don't see any technical dependencies between the two, so, please split it 
> into two independent patches.

StoreToImmutable does not depend on the modeling checker, but I am using the 
BugType pointer from StoreToImmutable in the modeling checker for Note 
diagnostics. 
Should it still be split and just not merge the second patch until the first 
one is committed?

In D124244#3493310 , @martong wrote:

> Also, could you please update the `Static Analyzer` section of 
> `clang/docs/ReleaseNotes.rst` with the new checkers and their description?

Sorry, I missed this comment. I will add release notes in the next round.


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

https://reviews.llvm.org/D124244

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

[PATCH] D125840: [Analyzer] Removed extra space from NSErrorChecker debug message and updated relevant tests

[Artem Dergachev via Phabricator via cfe-commits]

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

Thanks! I'll commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125840

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

[PATCH] D124244: [analyzer] add StoreToImmutable and ModelConstQualifiedReturn checkers

[Zurab Tsinadze via Phabricator via cfe-commits]

zukatsinadze added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:767-769
 assert(isa(sreg) || isa(sreg) ||
-   isa(sreg));
+   isa(sreg) ||
+   isa(sreg));

steakhal wrote:
> Please merge these into a single `isa()`.
error: macro "assert" passed 4 arguments, but takes just 1
`assert(isa(sreg));`

So I had to add extra ()-s around `isa`, I guess macro expands weirdly? 



Comment at: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt:68-70
   MIGChecker.cpp
+  cert/ModelConstQualifiedReturnChecker.cpp
   MoveChecker.cpp

steakhal wrote:
> Put this in the right alphabetical place.
I think it is in the right alphabetical place not considering cert/ (other 
certs are in similar order), but maybe I should remove the cert directory, what 
do you think? 
It was created by me a few years ago, but I don't see a point now anymore.



Comment at: clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp:27-29
+  BugType ImmutableMemoryBind{this,
+  "Immutable memory should not be overwritten",
+  categories::MemoryError};

steakhal wrote:
> You could expose a pointer to this by creating a `const BugType *getBugType() 
> const;` getter method, which could be used by other checkers.
I did something similar based on SmartPtrChecker.



Comment at: clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp:43
+
+  if (!isa(R->getMemorySpace()))
+return;

NoQ wrote:
> This check catches a lot more regions than the ones produced by the other 
> checker. In particular, it'll warn on all global constants. Did you intend 
> this to happen? You don't seem to have tests for that.
Could you give an example? I tried with the following snippet and it didn't 
give a warning

```
int a = 1, b = 1;
void foo(int *p) {
  a = 2;
  p[b++] = 3;
  int *c = &a;
  *c = 5;
}
```



Comment at: clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp:52
+  ImmutableMemoryBind, "modifying immutable memory", ErrorNode);
+  Report->markInteresting(R);
+  C.emitReport(std::move(Report));

NoQ wrote:
> I also recommend `trackExpressionValue` to make sure we have all 
> reassignments highlighted as the value gets bounced between pointer 
> variables. The user will need proof that the pointer actually points to const 
> memory.
What expression should I use for tracking? I tried to simply cast `S` to 
`Expr`, but it didn't really work. 
Then I came up with something ugly: cast `S` to `BinaryOperator` -> `getLHS` -> 
`getSubExpr` -> `ignoreImpCasts`.  


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

https://reviews.llvm.org/D124244

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

[PATCH] D124244: [analyzer] add StoreToImmutable and ModelConstQualifiedReturn checkers

[Zurab Tsinadze via Phabricator via cfe-commits]

zukatsinadze updated this revision to Diff 430204.
zukatsinadze marked 14 inline comments as done.

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

https://reviews.llvm.org/D124244

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/StoreToImmutable.h
  clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/cert/ModelConstQualifiedReturnChecker.cpp
  clang/test/Analysis/analyzer-enabled-checkers.c
  clang/test/Analysis/cert/env30-c.cpp
  clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c

Index: clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
===
--- clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
+++ clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
@@ -32,6 +32,7 @@
 // CHECK-NEXT: core.NullDereference
 // CHECK-NEXT: core.StackAddrEscapeBase
 // CHECK-NEXT: core.StackAddressEscape
+// CHECK-NEXT: core.StoreToImmutable
 // CHECK-NEXT: core.UndefinedBinaryOperatorResult
 // CHECK-NEXT: core.VLASize
 // CHECK-NEXT: core.builtin.BuiltinFunctions
Index: clang/test/Analysis/cert/env30-c.cpp
===
--- /dev/null
+++ clang/test/Analysis/cert/env30-c.cpp
@@ -0,0 +1,222 @@
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=core,alpha.security.cert.env.ModelConstQualifiedReturn \
+// RUN: -analyzer-output=text -verify -Wno-unused %s
+
+#include "../Inputs/system-header-simulator.h"
+char *getenv(const char *name);
+char *setlocale(int category, const char *locale);
+char *strerror(int errnum);
+int setenv(const char *name, const char *value, int overwrite);
+void free(void *memblock);
+void *malloc(size_t size);
+char *strdup(const char *s);
+
+typedef struct {
+} tm;
+char *asctime(const tm *timeptr);
+
+int strcmp(const char *, const char *);
+void const_parameter_function(const char *);
+
+
+
+void getenv_test1() {
+  char *a, *b, *c;
+  a = getenv("VAR");
+  // expected-note@-1{{Return value of 'getenv' should be treated as const-qualified}}
+  // expected-note@-2{{Value assigned to 'a'}}
+  b = a;
+  // expected-note@-1{{The value of 'a' is assigned to 'b'}}
+  c = b;
+  // expected-note@-1{{The value of 'b' is assigned to 'c'}}
+  *c = 'A';
+  // expected-warning@-1{{Modifying immutable memory [core.StoreToImmutable]}}
+  // expected-note@-2{{Modifying immutable memory}}
+}
+
+void getenv_test2() {
+  char *p;
+  p = getenv("VAR");
+  const_parameter_function(p); // no-warning
+}
+
+void modify(char *p) {
+  *p = 'A';
+  // expected-warning@-1{{Modifying immutable memory [core.StoreToImmutable]}}
+  // expected-note@-2{{Modifying immutable memory}}
+}
+
+void getenv_test3() {
+  char *p = getenv("VAR");
+  // expected-note@-1{{Return value of 'getenv' should be treated as const-qualified}}
+  // expected-note@-2{{'p' initialized here}}
+  char *pp = p;
+  // expected-note@-1{{'pp' initialized to the value of 'p'}}
+  modify(pp); // Writing to immutable region within the call.
+  // expected-note@-1{{Calling 'modify'}}
+  // expected-note@-2{{Passing value via 1st parameter 'p'}}
+}
+
+void does_not_modify(char *p) {
+  *p;
+}
+
+void getenv_test4() {
+  char *p = getenv("VAR");
+  does_not_modify(p); // no-warning
+}
+
+void getenv_test5() {
+  static char *array[] = {0};
+
+  if (!array[0]) {
+// expected-note@-1{{Taking true branch}}
+array[0] = getenv("TEMPDIR");
+// expected-note@-1{{Return value of 'getenv' should be treated as const-qualified}}
+// expected-note@-2{{Assigning value}}
+  }
+
+  *array[0] = 'A';
+  // expected-warning@-1{{Modifying immutable memory [core.StoreToImmutable]}}
+  // expected-note@-2{{Modifying immutable memory}}
+}
+
+void getenv_test6() {
+  char *p = getenv("VAR");
+  // expected-note@-1{{Return value of 'getenv' should be treated as const-qualified}}
+  if (!p)
+// expected-note@-1{{Assuming 'p' is non-null}}
+// expected-note@-2{{Taking false branch}}
+return;
+  p[0] = '\0';
+  // expected-warning@-1{{Modifying immutable memory [core.StoreToImmutable]}}
+  // expected-note@-2{{Modifying immutable memory}}
+}
+
+void setlocale_test() {
+  char *p = setlocale(0, "VAR");
+  // expected-note@-1{{Return value of 'setlocale' should be treated as const-qualified}}
+  // expected-note@-2{{'p' initialized here}}
+  *p = 'A';
+  // expected-warning@-1{{Modifying immutable memory [core.StoreToImmutable]}}
+  // expected-note@-2{{Modifying immutable memory}}
+}
+
+void strerror_test() {
+char *p = strerror(0);
+// expected-note@-1{{Return value of 'strerror' should be treated as const-qualified}}
+// expected-note@-2{{'p' initialized here}}
+*p = 'A';
+// expected-warning@-1

[PATCH] D125585: [HLSL][clang][Driver] Parse target profile early to update Driver::TargetTriple.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/Driver/ToolChains/HLSL.h:32
 Action::OffloadKind DeviceOffloadKind) const override;
-  std::string ComputeEffectiveClangTriple(const llvm::opt::ArgList &Args,
-  types::ID InputType) const override;
+  static std::string parseTargetProfile(StringRef TargetProfile);
 };

I don't like using an empty string as a sentinel for an error. Since this is no 
longer overriding an existing method, we should change it. Probably the easiest 
change is to llvm::Optional. Then you can return `None` in the failure cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125585

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

[PATCH] D125840: [Analyzer] Removed extra space from NSErrorChecker debug message and updated relevant tests

[Usama Hameed via Phabricator via cfe-commits]

usama54321 created this revision.
Herald added subscribers: manas, steakhal, ASDenysPetrov, martong, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, 
xazax.hun.
Herald added a project: All.
usama54321 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/D125840

Files:
  clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
  clang/test/Analysis/CheckNSError.m


Index: clang/test/Analysis/CheckNSError.m
===
--- clang/test/Analysis/CheckNSError.m
+++ clang/test/Analysis/CheckNSError.m
@@ -27,7 +27,7 @@
 
 @implementation A
 - (void)myMethodWhichMayFail:(NSError **)error {   // expected-warning 
{{Method accepting NSError** should have a non-void return value to indicate 
whether or not an error occurred}}
-  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // 
expected-warning {{Potential null dereference}}
+  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // 
expected-warning {{Potential null dereference. According to coding standards in 
'Creating and Returning NSError Objects' the parameter may be null 
[osx.cocoa.NSError]}}
 }
 
 - (BOOL)myMethodWhichMayFail2:(NSError **)error {  // no-warning
@@ -50,7 +50,7 @@
 typedef struct __CFError* CFErrorRef;
 
 void foo(CFErrorRef* error) { // expected-warning {{Function accepting 
CFErrorRef* should have a non-void return value to indicate whether or not an 
error occurred}}
-  *error = 0;  // expected-warning {{Potential null dereference}}
+  *error = 0;  // expected-warning {{Potential null dereference. According to 
coding standards documented in CoreFoundation/CFError.h the parameter may be 
null [osx.coreFoundation.CFError]}}
 }
 
 int f1(CFErrorRef* error) {
@@ -74,7 +74,7 @@
 }
 
 int __attribute__((nonnull(1))) f5(int *x, CFErrorRef *error) {
-  *error = 0; // expected-warning {{Potential null dereference}}
+  *error = 0; // expected-warning {{Potential null dereference. According to 
coding standards documented in CoreFoundation/CFError.h the parameter may be 
null [osx.coreFoundation.CFError]}}
   return 0;
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
@@ -266,7 +266,7 @@
   SmallString<128> Buf;
   llvm::raw_svector_ostream os(Buf);
 
-  os << "Potential null dereference.  According to coding standards ";
+  os << "Potential null dereference. According to coding standards ";
   os << (isNSError
  ? "in 'Creating and Returning NSError Objects' the parameter"
  : "documented in CoreFoundation/CFError.h the parameter");


Index: clang/test/Analysis/CheckNSError.m
===
--- clang/test/Analysis/CheckNSError.m
+++ clang/test/Analysis/CheckNSError.m
@@ -27,7 +27,7 @@
 
 @implementation A
 - (void)myMethodWhichMayFail:(NSError **)error {   // expected-warning {{Method accepting NSError** should have a non-void return value to indicate whether or not an error occurred}}
-  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // expected-warning {{Potential null dereference}}
+  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // expected-warning {{Potential null dereference. According to coding standards in 'Creating and Returning NSError Objects' the parameter may be null [osx.cocoa.NSError]}}
 }
 
 - (BOOL)myMethodWhichMayFail2:(NSError **)error {  // no-warning
@@ -50,7 +50,7 @@
 typedef struct __CFError* CFErrorRef;
 
 void foo(CFErrorRef* error) { // expected-warning {{Function accepting CFErrorRef* should have a non-void return value to indicate whether or not an error occurred}}
-  *error = 0;  // expected-warning {{Potential null dereference}}
+  *error = 0;  // expected-warning {{Potential null dereference. According to coding standards documented in CoreFoundation/CFError.h the parameter may be null [osx.coreFoundation.CFError]}}
 }
 
 int f1(CFErrorRef* error) {
@@ -74,7 +74,7 @@
 }
 
 int __attribute__((nonnull(1))) f5(int *x, CFErrorRef *error) {
-  *error = 0; // expected-warning {{Potential null dereference}}
+  *error = 0; // expected-warning {{Potential null dereference. According to coding standards documented in CoreFoundation/CFError.h the parameter may be null [osx.coreFoundation.CFError]}}
   return 0;
 }
 
Index: clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
@@ -266,7 +266,7 @@
   SmallString<128> Buf;
   llvm::raw_svector_ostream os(Buf);
 
-  os << "Potential null dereference.  According to coding stan

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

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

dexonsmith added a comment.

In D122895#3511855 , @aaron.ballman 
wrote:

>> (It also seems unfortunate to regress the false positive rate of this 
>> diagnostic before `-fstrict-prototypes` is available.)
>
> `-fno-knr-functions` is available already today in trunk, so I'm not certain 
> I understand your concern there (or were you unaware we changed the flag name 
> perhaps?).

That's right, I missed it. Thanks for pointing it out!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122895

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

[PATCH] D113107: Support of expression granularity for _Float16.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Okay, well, first off, we definitely shouldn't be repeating conditions like 
`isX86()` all over the place.  What you want to do is to have a general 
predicate which answers whether we should emit this operation with excess 
precision; imagine an architecture that wanted to emit `float` operations with 
`double`.

It looks like you ought to be able to use the abstraction around `BinOpInfo` to 
good effect here.  You can add a promotion type on the `ScalarExprEmitter` 
which, if set, tells the emitter that it should actually produce a value of 
that type.  `EmitBinOps` can recognize that a promotion type is already set and 
use that type as the operation type instead of the formal type of the 
expression.  If a promotion type isn't set on the emitter, `EmitBinOps` can try 
to recognize that it's in the case where it needs a promotion type; it would 
then set a promotion type before evaluating the operands, but then truncate the 
result (and clear the promotion type) before returning.  The logic around 
compound assignment will take some care (and good testing), but it should work 
the same basic way, and similarly in the complex emitter.


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

https://reviews.llvm.org/D113107

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

[PATCH] D125839: [gmodules] Skip CXXDeductionGuideDecls when visiting FunctionDecls in DebugTypeVisitor

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

Without this fix, `CXXNameMangler::mangleUnresolvedName` crashes when it tries 
to mangle the name of a `CXXDeductionGuideDecl`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125839

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

[clang-tools-extra] 79ca4ed - [pseudo] Design notes from discussion today. NFC

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-05-18T00:08:47+02:00
New Revision: 79ca4ed3e782e505e8964a3c968de5fd4f09ca7a

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

LOG: [pseudo] Design notes from discussion today. NFC

Added: 
clang-tools-extra/pseudo/DesignNotes.md

Modified: 


Removed: 




diff  --git a/clang-tools-extra/pseudo/DesignNotes.md 
b/clang-tools-extra/pseudo/DesignNotes.md
new file mode 100644
index ..421cc02aef75
--- /dev/null
+++ b/clang-tools-extra/pseudo/DesignNotes.md
@@ -0,0 +1,123 @@
+# Error recovery (2022-05-07)
+
+Problem: we have two fairly generic cases of recovery bounded within a range:
+ - sequences: `int x; this is absolute garbage; x++;`
+ - brackets: `void foo() { this is garbage too }`
+
+So far, we've thought of these as 
diff erent, and had precise ideas about
+brackets ("lazy parsing") and vague ones about sequences.
+Con we unify them instead?
+
+In both cases we want to recognize the bounds of the garbage item based on
+basic token-level features of the surrounding code, and avoid any interference
+with the surrounding code.
+
+## Brackets
+
+Consider a rule like `compound-stmt := { stmt-seq }`.
+
+The desired recovery is:
+- if we fail at `{ . stmt-seq }`
+- ... and we can find for the matching `}`
+- then consume up to that token as an opaque broken `stmt-seq`
+- ... and advance state to `{ stmt-seq . }`
+
+We can annotate the rule to describe this: `{ stmt-seq [recovery] }`.
+We can generalize as `{ stmt-seq [recovery=rbrace] }`, allowing for 
diff erent
+**strategies** to find the resume point.
+
+(It's OK to assume we're skipping over one RHS nonterminal, we can always
+introduce a nonterminal for the bracket contents if necessary).
+
+## Sequences
+
+Can we apply the same technique to sequences?
+Simplest case: delimited right-recursive sequence.
+
+```
+param-list := param
+param-list := param , param-list
+```
+
+We need recovery in **both** rules.
+`param` in the first rule matches the *last* param in a list,
+in the second rule it matches all others. We want to recover in any position.
+
+If we want to be able to recovery `int x int y` as two parameters, then we
+should extract a `param-and-comma` rule that recovery could operate on.
+
+### Last vs not-last elements
+
+Sequences will usually have two rules with recovery, we discussed:
+ - how to pick the correct one to recover with
+ - in a left-recursive rule they correspond to last & not-last elements
+ - the recovery strategy knows whether we're recoverying last or not-last
+ - we could have the strategy return (pos, symbol parsed), and artificially
+   require distinct symbols (e.g. `stmt` vs `last_stmt`).
+ - we can rewrite left-recursion in the grammar as right-recursion
+
+However, on reflection I think we can simply allow recovery according to both
+rules. The "wrong" recovery will produce a parse head that dies.
+
+## How recovery fits into GLR
+
+Recovery should kick in at the point where we would otherwise abandon all
+variants of an high-level parse.
+
+e.g. Suppose we're parsing `static_cast(1)` and are up to `bar`.
+Our GSS looks something like:
+
+```
+ "the static cast's type starts at foo"
+---> {expr := static_cast < . type > ( expr )}
+ | "foo... is a class name"
+ + {type := identifier .}
+ | "foo... is a template ID"
+ + {type := identifier . < template-arg-list >}
+```
+
+Since `foo bar baz` isn't a valid class name or template ID, both active heads
+will soon die, as will the parent GSS Node - the latter should trigger 
recovery.
+
+- we need a refcount in GSS nodes so we can recognize never-reduced node death
+- when a node dies, we look up its recovery actions (symbol, strategy).
+  These are the union of the recovery actions for each item.
+  They can be stored in the action table.
+  Here: `actions[State, death] = Recovery(type, matching-angle-bracket)`
+- we try each strategy: feeding in the start position = token of the dying node
+  (`foo`) getting out the end position (`>`).
+- We form an opaque forest node with the correct symbol (`type`) spanning
+  [start, end)
+- We create a GSS node to represent the state after recovery.
+  The new state is found in the Goto table in the usual way.
+
+```
+ "the static cast's type starts at foo"
+---> {expr := static_cast < . type > ( expr )}
+ | "`foo bar baz` is an unparseable type"
+ + {expr := static_cast < type . > (expr)}
+```
+
+## Which recovery heads to activate
+
+We probably shouldn't *always* create active recovery heads when a recoverable
+node dies (and thus keep it alive).
+By design GLR creates multiple speculative parse heads and lets incorrect heads
+disappear.
+
+Concretely, the expression `(int *)(x)` is a v

[PATCH] D125839: [gmodules] Skip CXXDeductionGuideDecls when visiting FunctionDecls in DebugTypeVisitor

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added a reviewer: aprantl.
ahatanak added a project: clang.
Herald added a project: All.
ahatanak requested review of this revision.

rdar://92845517


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125839

Files:
  clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
  clang/test/Modules/Inputs/gmodules-deduction-guide.h
  clang/test/Modules/gmodules-deduction-guide.cpp


Index: clang/test/Modules/gmodules-deduction-guide.cpp
===
--- /dev/null
+++ clang/test/Modules/gmodules-deduction-guide.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -std=c++2b -x c++-header -emit-pch -fmodule-format=obj -I 
%S/Inputs \
+// RUN:   -o %t.pch %S/Inputs/gmodules-deduction-guide.h \
+// RUN:   -mllvm -debug-only=pchcontainer &>%t-pch.ll
+// RUN: cat %t-pch.ll | FileCheck %s
+
+// CHECK: ![[V0:.*]] = distinct !DICompositeType(tag: DW_TAG_structure_type, 
name: "S",
+// CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "Type0",{{.*}}, baseType: 
![[V0]])
Index: clang/test/Modules/Inputs/gmodules-deduction-guide.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/gmodules-deduction-guide.h
@@ -0,0 +1,11 @@
+struct A {
+};
+
+template 
+struct S{
+  S(const A &);
+};
+
+S(const A&) -> S;
+
+typedef decltype(S(A())) Type0;
Index: clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
===
--- clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
+++ clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
@@ -96,6 +96,10 @@
 }
 
 bool VisitFunctionDecl(FunctionDecl *D) {
+  // Skip deduction guides.
+  if (isa(D))
+return true;
+
   if (isa(D))
 // This is not yet supported. Constructing the `this' argument
 // mandates a CodeGenFunction.


Index: clang/test/Modules/gmodules-deduction-guide.cpp
===
--- /dev/null
+++ clang/test/Modules/gmodules-deduction-guide.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -std=c++2b -x c++-header -emit-pch -fmodule-format=obj -I %S/Inputs \
+// RUN:   -o %t.pch %S/Inputs/gmodules-deduction-guide.h \
+// RUN:   -mllvm -debug-only=pchcontainer &>%t-pch.ll
+// RUN: cat %t-pch.ll | FileCheck %s
+
+// CHECK: ![[V0:.*]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "S",
+// CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "Type0",{{.*}}, baseType: ![[V0]])
Index: clang/test/Modules/Inputs/gmodules-deduction-guide.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/gmodules-deduction-guide.h
@@ -0,0 +1,11 @@
+struct A {
+};
+
+template 
+struct S{
+  S(const A &);
+};
+
+S(const A&) -> S;
+
+typedef decltype(S(A())) Type0;
Index: clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
===
--- clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
+++ clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
@@ -96,6 +96,10 @@
 }
 
 bool VisitFunctionDecl(FunctionDecl *D) {
+  // Skip deduction guides.
+  if (isa(D))
+return true;
+
   if (isa(D))
 // This is not yet supported. Constructing the `this' argument
 // mandates a CodeGenFunction.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125802: Fix std::has_unique_object_representations for _BitInt types with padding bits

[Mital Ashok via Phabricator via cfe-commits]

MitalAshok updated this revision to Diff 430186.
MitalAshok edited the summary of this revision.
MitalAshok added a comment.

Added release note and missing test for struct with reference member


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125802

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/ASTContext.cpp
  clang/test/SemaCXX/has_unique_object_reps_bitint.cpp

Index: clang/test/SemaCXX/has_unique_object_reps_bitint.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/has_unique_object_reps_bitint.cpp
@@ -0,0 +1,88 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify -std=c++17 -Wno-bitfield-width %s
+//  expected-no-diagnostics
+
+static_assert(__has_unique_object_representations(_BitInt(8)));
+static_assert(__has_unique_object_representations(unsigned _BitInt(8)));
+static_assert(__has_unique_object_representations(_BitInt(sizeof(int) * 8u)));
+// sizeof(_BitInt(24)) may be 4 to align it to the next greater integer type, in which case it would have 8 padding bits
+static_assert(__has_unique_object_representations(_BitInt(24)) == (sizeof(_BitInt(24)) == 3));
+
+static_assert(!__has_unique_object_representations(_BitInt(7)));
+static_assert(!__has_unique_object_representations(unsigned _BitInt(7)));
+static_assert(!__has_unique_object_representations(_BitInt(2)));
+static_assert(!__has_unique_object_representations(unsigned _BitInt(1)));
+
+template 
+constexpr bool check() {
+  if constexpr (N <= __BITINT_MAXWIDTH__) {
+static_assert(__has_unique_object_representations(_BitInt(N)) == (sizeof(_BitInt(N)) * 8u == N));
+static_assert(__has_unique_object_representations(unsigned _BitInt(N)) == (sizeof(unsigned _BitInt(N)) * 8u == N));
+  }
+  return true;
+}
+
+template 
+constexpr bool do_check = (check() && ...);
+
+static_assert(do_check<2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18>);
+static_assert(do_check<15, 16, 17, 23, 24, 25, 31, 32, 33>);
+static_assert(do_check<39, 40, 41, 47, 48, 49>);
+static_assert(do_check<127, 128, 129, 255, 256, 257, 383, 384, 385>);
+
+template 
+struct in_struct {
+  _BitInt(N) x;
+  static constexpr bool check() {
+return __has_unique_object_representations(in_struct) == __has_unique_object_representations(_BitInt(N));
+  }
+};
+
+static_assert(in_struct<8>::check());
+static_assert(in_struct<7>::check());
+
+struct bit_fields_1 {
+  _BitInt(7) x : 7;
+  unsigned _BitInt(1) y : 1;
+};
+
+static_assert(__has_unique_object_representations(bit_fields_1) == (sizeof(bit_fields_1) == 1));
+
+struct bit_fields_2 {
+  _BitInt(8) x : 7;
+};
+
+static_assert(!__has_unique_object_representations(bit_fields_2));
+
+struct bit_fields_3 {
+  _BitInt(15) x : 8;
+};
+
+static_assert(__has_unique_object_representations(bit_fields_3) == (sizeof(bit_fields_3) == 1));
+
+#if __BITINT_MAXWIDTH__ >= 129
+struct bit_fields_4 {
+  _BitInt(129) x : 128;
+};
+
+static_assert(__has_unique_object_representations(bit_fields_4) == (sizeof(bit_fields_4) == 128 / 8));
+#endif
+
+struct bit_fields_5 {
+  _BitInt(2) x : 8;
+};
+
+static_assert(!__has_unique_object_representations(bit_fields_5));
+
+template 
+struct ref_member {
+  _BitInt(N) & x;
+};
+
+struct int_ref_member {
+  int &x;
+};
+
+static_assert(__has_unique_object_representations(ref_member<7>));
+static_assert(__has_unique_object_representations(ref_member<8>));
+static_assert(__has_unique_object_representations(ref_member<7>) == __has_unique_object_representations(ref_member<8>));
+static_assert(__has_unique_object_representations(ref_member<8>) == __has_unique_object_representations(int_ref_member));
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2684,7 +2684,12 @@
 if (!RD->isUnion())
   return structHasUniqueObjectRepresentations(Context, RD);
   }
-  if (!Field->getType()->isReferenceType() &&
+
+  // A _BitInt type may not be unique if it has padding bits
+  // but if it is a bitfield the padding bits are not used
+  bool IsBitIntType =
+  !Field->getType()->isReferenceType() && Field->getType()->isBitIntType();
+  if (!Field->getType()->isReferenceType() && !IsBitIntType &&
   !Context.hasUniqueObjectRepresentations(Field->getType()))
 return llvm::None;
 
@@ -2692,9 +2697,17 @@
   Context.toBits(Context.getTypeSizeInChars(Field->getType()));
   if (Field->isBitField()) {
 int64_t BitfieldSize = Field->getBitWidthValue(Context);
-if (BitfieldSize > FieldSizeInBits)
+if (IsBitIntType) {
+  if ((unsigned)BitfieldSize >
+  cast(Field->getType())->getNumBits())
+return llvm::None;
+} else if (BitfieldSize > FieldSizeInBits) {
   return llvm::None;
+}
 FieldSizeInBits = BitfieldSize;
+  } else if (IsBitIntType &&
+ !Context.hasUnique

[PATCH] D125829: [clang] Fix __has_builtin

[Artem Belevich via Phabricator via cfe-commits]

tra added a subscriber: echristo.
tra added a comment.

@echristo - FYI, in case you have an opinion on target builtin feature checks.




Comment at: clang/include/clang/Basic/Builtins.h:263
+  ///false if it is disabled.
+  bool isRequiredTargetFeaturesEnabled(
+  unsigned BuiltinID, const llvm::StringMap &TargetFetureMap) const;

I think we should boil it down to just 
`evaluateRequiredTargetFeatures(StringRef RequiredFeatures, ... 
TargetFeatureMap)`.




Comment at: clang/lib/Basic/BuiltinTargetFeatures.h:32
+/// pairs.
+class TargetFeatures {
+  struct FeatureListStatus {

I don't think it needs to be part of the public API. Feature check function 
operates on `llvm::StringMap` and does not need to know about this class, 
and the implementation of this class can be moved into `Builtins.cpp` where 
it's actually used.


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

https://reviews.llvm.org/D125829

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

[PATCH] D125821: [clang][dataflow] Fix double visitation of nested logical operators

[Eric Li via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
li.zhe.hua marked an inline comment as done.
Closed by commit rG5bbef2e3fff1: [clang][dataflow] Fix double visitation of 
nested logical operators (authored by li.zhe.hua).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125821

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


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2467,6 +2467,67 @@
   EXPECT_EQ(&BazVal->getRightSubValue(), BarVal);
 });
   }
+
+  {
+std::string Code = R"(
+  void target(bool A, bool B, bool C, bool D) {
+bool Foo = ((A && B) && C) && D;
+// [[p]]
+  }
+)";
+runDataflow(
+Code, [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+  ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+  const Environment &Env = Results[0].second.Env;
+
+  const ValueDecl *ADecl = findValueDecl(ASTCtx, "A");
+  ASSERT_THAT(ADecl, NotNull());
+
+  const auto *AVal =
+  dyn_cast_or_null(Env.getValue(*ADecl, 
SkipPast::None));
+  ASSERT_THAT(AVal, NotNull());
+
+  const ValueDecl *BDecl = findValueDecl(ASTCtx, "B");
+  ASSERT_THAT(BDecl, NotNull());
+
+  const auto *BVal =
+  dyn_cast_or_null(Env.getValue(*BDecl, 
SkipPast::None));
+  ASSERT_THAT(BVal, NotNull());
+
+  const ValueDecl *CDecl = findValueDecl(ASTCtx, "C");
+  ASSERT_THAT(CDecl, NotNull());
+
+  const auto *CVal =
+  dyn_cast_or_null(Env.getValue(*CDecl, 
SkipPast::None));
+  ASSERT_THAT(CVal, NotNull());
+
+  const ValueDecl *DDecl = findValueDecl(ASTCtx, "D");
+  ASSERT_THAT(DDecl, NotNull());
+
+  const auto *DVal =
+  dyn_cast_or_null(Env.getValue(*DDecl, 
SkipPast::None));
+  ASSERT_THAT(DVal, NotNull());
+
+  const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+  ASSERT_THAT(FooDecl, NotNull());
+
+  const auto *FooVal = dyn_cast_or_null(
+  Env.getValue(*FooDecl, SkipPast::None));
+  ASSERT_THAT(FooVal, NotNull());
+
+  const auto &FooLeftSubVal =
+  cast(FooVal->getLeftSubValue());
+  const auto &FooLeftLeftSubVal =
+  cast(FooLeftSubVal.getLeftSubValue());
+  EXPECT_EQ(&FooLeftLeftSubVal.getLeftSubValue(), AVal);
+  EXPECT_EQ(&FooLeftLeftSubVal.getRightSubValue(), BVal);
+  EXPECT_EQ(&FooLeftSubVal.getRightSubValue(), CVal);
+  EXPECT_EQ(&FooVal->getRightSubValue(), DVal);
+});
+  }
 }
 
 TEST_F(TransferTest, AssignFromBoolNegation) {
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -571,12 +571,15 @@
 return *Val;
 }
 
-// Sub-expressions that are logic operators are not added in basic blocks
-// (e.g. see CFG for `bool d = a && (b || c);`). If `SubExpr` is a logic
-// operator, it isn't evaluated and assigned a value yet. In that case, we
-// need to first visit `SubExpr` and then try to get the value that gets
-// assigned to it.
-Visit(&SubExpr);
+if (Env.getStorageLocation(SubExpr, SkipPast::None) == nullptr) {
+  // Sub-expressions that are logic operators are not added in basic blocks
+  // (e.g. see CFG for `bool d = a && (b || c);`). If `SubExpr` is a logic
+  // operator, it may not have been evaluated and assigned a value yet. In
+  // that case, we need to first visit `SubExpr` and then try to get the
+  // value that gets assigned to it.
+  Visit(&SubExpr);
+}
+
 if (auto *Val = dyn_cast_or_null(
 Env.getValue(SubExpr, SkipPast::Reference)))
   return *Val;


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2467,6 +2467,67 @@
   EXPECT_EQ(&BazVal->getRightSubValue(), BarVal);
 });
   }
+
+  {
+std::string Code = R"(
+  void target(bool A, bool B, bool C, bool D) {
+bool Foo = ((A && B) && C) && D;
+// [[p]]
+  }
+)";
+runDataflow(
+Code, [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+  ASSERT_THAT(

[clang] 5bbef2e - [clang][dataflow] Fix double visitation of nested logical operators

[Eric Li via cfe-commits]

Author: Eric Li
Date: 2022-05-17T20:28:48Z
New Revision: 5bbef2e3fff123293ed9c2037e2662e352bf37af

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

LOG: [clang][dataflow] Fix double visitation of nested logical operators

Sub-expressions that are logical operators are not spelled out
separately in basic blocks, so we need to manually visit them when we
encounter them. We do this in both the `TerminatorVisitor`
(conditionally) and the `TransferVisitor` (unconditionally), which can
cause cause an expression to be visited twice when the binary
operators are nested 2+ times.

This changes the visit in `TransferVisitor` to check if it has been
evaluated before trying to visit the sub-expression.

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp 
b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 3a94434b7aeb7..f88406595d728 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -571,12 +571,15 @@ class TransferVisitor : public 
ConstStmtVisitor {
 return *Val;
 }
 
-// Sub-expressions that are logic operators are not added in basic blocks
-// (e.g. see CFG for `bool d = a && (b || c);`). If `SubExpr` is a logic
-// operator, it isn't evaluated and assigned a value yet. In that case, we
-// need to first visit `SubExpr` and then try to get the value that gets
-// assigned to it.
-Visit(&SubExpr);
+if (Env.getStorageLocation(SubExpr, SkipPast::None) == nullptr) {
+  // Sub-expressions that are logic operators are not added in basic blocks
+  // (e.g. see CFG for `bool d = a && (b || c);`). If `SubExpr` is a logic
+  // operator, it may not have been evaluated and assigned a value yet. In
+  // that case, we need to first visit `SubExpr` and then try to get the
+  // value that gets assigned to it.
+  Visit(&SubExpr);
+}
+
 if (auto *Val = dyn_cast_or_null(
 Env.getValue(SubExpr, SkipPast::Reference)))
   return *Val;

diff  --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index db1f1cff311c7..9819409291040 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2467,6 +2467,67 @@ TEST_F(TransferTest, AssignFromCompositeBoolExpression) {
   EXPECT_EQ(&BazVal->getRightSubValue(), BarVal);
 });
   }
+
+  {
+std::string Code = R"(
+  void target(bool A, bool B, bool C, bool D) {
+bool Foo = ((A && B) && C) && D;
+// [[p]]
+  }
+)";
+runDataflow(
+Code, [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+  ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+  const Environment &Env = Results[0].second.Env;
+
+  const ValueDecl *ADecl = findValueDecl(ASTCtx, "A");
+  ASSERT_THAT(ADecl, NotNull());
+
+  const auto *AVal =
+  dyn_cast_or_null(Env.getValue(*ADecl, 
SkipPast::None));
+  ASSERT_THAT(AVal, NotNull());
+
+  const ValueDecl *BDecl = findValueDecl(ASTCtx, "B");
+  ASSERT_THAT(BDecl, NotNull());
+
+  const auto *BVal =
+  dyn_cast_or_null(Env.getValue(*BDecl, 
SkipPast::None));
+  ASSERT_THAT(BVal, NotNull());
+
+  const ValueDecl *CDecl = findValueDecl(ASTCtx, "C");
+  ASSERT_THAT(CDecl, NotNull());
+
+  const auto *CVal =
+  dyn_cast_or_null(Env.getValue(*CDecl, 
SkipPast::None));
+  ASSERT_THAT(CVal, NotNull());
+
+  const ValueDecl *DDecl = findValueDecl(ASTCtx, "D");
+  ASSERT_THAT(DDecl, NotNull());
+
+  const auto *DVal =
+  dyn_cast_or_null(Env.getValue(*DDecl, 
SkipPast::None));
+  ASSERT_THAT(DVal, NotNull());
+
+  const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+  ASSERT_THAT(FooDecl, NotNull());
+
+  const auto *FooVal = dyn_cast_or_null(
+  Env.getValue(*FooDecl, SkipPast::None));
+  ASSERT_THAT(FooVal, NotNull());
+
+  const auto &FooLeftSubVal =
+  cast(FooVal->getLeftSubValue());
+  const auto &FooLeftLeftSubVal =
+  cast(FooLeftSubVal.getLeftSubValue());
+  EXPECT_EQ(&FooLeftLeftSubVal.getLeftSubValue(), AVal);
+  EXPECT_EQ(&FooLeftLeftSubVal.getRightSubValue(), BVal);
+  EXPECT_EQ(&FooLeftSubVal.getRightSubValue(), CVal);
+

[PATCH] D125821: [clang][dataflow] Fix double visitation of nested logical operators

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125821

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

[PATCH] D125259: [C11] Diagnose unreachable generic selection associations

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added a comment.

> I'm questioning the benefit to supporting _Generic in C++ and starting to 
> wonder if perhaps we should pull this extension back.

I imagine it may be used in shared system headers. E.g., implementations of 
`tgmath.h` that don't use the builtins.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125259

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

[PATCH] D115232: [clangd] Indexing of standard library

[Sam McCall via Phabricator via cfe-commits]

sammccall added a subscriber: thakis.
sammccall added a comment.

Hmm, the test keeps crashing on the GN bot: 
http://45.33.8.238/win/58316/step_9.txt
Unfortunately the stacktrace is not symbolized, and I'm not seeing this 
elsewhere (e.g. premerge bot).

@thakis, any idea why unittests no longer manage to symbolize stack traces on 
crash on the windows bot? I believe this used to work...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115232

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

[PATCH] D125814: Fix strict prototype diagnostic wording for definitions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D125814#3520137 , @jyknight wrote:

> This confuses me.
>
> Looking at behavior with default flags:
>
> We won't emit a -Wdeprecated-non-prototype warning for `int foo();`, until we 
> subsequently find `int foo(int arg) { return 5; }`.

Close, but not quite. With default flags, we won't warn for `int foo();` until 
we see a subsequent `int foo(int arg)` (regardless of whether it's a 
declaration or a definition).

> Since we definitely have the context of what's going on at that point, in 
> order to have determined that there's a conflict, what prevents doing the 
> "right thing": emitting only 1 warning (at the definition site) and 1 note 
> (at the declaration site)?

I'm going to continue to investigate; I recall arriving at this complex logic 
because I was trying to balance the deprecation warning diagnostics against the 
changes behavior diagnostics to keep the chattiness down. However, maybe this 
logic can now be simplified if I reword the diagnostic messages somewhat and 
rearrange some of the logic. I'll see what I can come up with.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125814

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

[clang-tools-extra] 6aabf60 - Revert "Reland "[clangd] Indexing of standard library""

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-05-17T21:33:48+02:00
New Revision: 6aabf60f2fb7589430c0ecc8fe95913c973fa248

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

LOG: Revert "Reland "[clangd] Indexing of standard library""

This reverts commit ccdb56ac10eef3048135169a67d239328c2b1de6.

Still seeing windows failures on GN bots: 
http://45.33.8.238/win/58316/step_9.txt

Unfortunately I can't debug these at all - it's a bare unsymbolized
stacktrace, and I can't reproduce the failure.

Added: 


Modified: 
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/TUScheduler.h
clang-tools-extra/clangd/index/FileIndex.cpp
clang-tools-extra/clangd/index/FileIndex.h
clang-tools-extra/clangd/index/SymbolOrigin.cpp
clang-tools-extra/clangd/index/SymbolOrigin.h
clang-tools-extra/clangd/unittests/CMakeLists.txt
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Removed: 
clang-tools-extra/clangd/index/StdLib.cpp
clang-tools-extra/clangd/index/StdLib.h
clang-tools-extra/clangd/unittests/StdLibTests.cpp



diff  --git a/clang-tools-extra/clangd/CMakeLists.txt 
b/clang-tools-extra/clangd/CMakeLists.txt
index 7cfbd6f95750e..9c37cfe7b7001 100644
--- a/clang-tools-extra/clangd/CMakeLists.txt
+++ b/clang-tools-extra/clangd/CMakeLists.txt
@@ -119,7 +119,6 @@ add_clang_library(clangDaemon
   index/Ref.cpp
   index/Relation.cpp
   index/Serialization.cpp
-  index/StdLib.cpp
   index/Symbol.cpp
   index/SymbolCollector.cpp
   index/SymbolID.cpp

diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 69a0f63972aae..80d7d5c5ece19 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -26,7 +26,6 @@
 #include "index/CanonicalIncludes.h"
 #include "index/FileIndex.h"
 #include "index/Merge.h"
-#include "index/StdLib.h"
 #include "refactor/Rename.h"
 #include "refactor/Tweak.h"
 #include "support/Cancellation.h"
@@ -60,39 +59,16 @@ namespace {
 // Update the FileIndex with new ASTs and plumb the diagnostics responses.
 struct UpdateIndexCallbacks : public ParsingCallbacks {
   UpdateIndexCallbacks(FileIndex *FIndex,
-   ClangdServer::Callbacks *ServerCallbacks,
-   const ThreadsafeFS &TFS, AsyncTaskRunner *Tasks)
-  : FIndex(FIndex), ServerCallbacks(ServerCallbacks), TFS(TFS),
-Tasks(Tasks) {}
+   ClangdServer::Callbacks *ServerCallbacks)
+  : FIndex(FIndex), ServerCallbacks(ServerCallbacks) {}
 
-  void onPreambleAST(PathRef Path, llvm::StringRef Version,
- const CompilerInvocation &CI, ASTContext &Ctx,
+  void onPreambleAST(PathRef Path, llvm::StringRef Version, ASTContext &Ctx,
  Preprocessor &PP,
  const CanonicalIncludes &CanonIncludes) override {
-// If this preamble uses a standard library we haven't seen yet, index it.
-if (FIndex)
-  if (auto Loc = Stdlib.add(*CI.getLangOpts(), PP.getHeaderSearchInfo()))
-indexStdlib(CI, std::move(*Loc));
-
 if (FIndex)
   FIndex->updatePreamble(Path, Version, Ctx, PP, CanonIncludes);
   }
 
-  void indexStdlib(const CompilerInvocation &CI, StdLibLocation Loc) {
-auto Task = [this, LO(*CI.getLangOpts()), Loc(std::move(Loc)),
- CI(std::make_unique(CI))]() mutable {
-  IndexFileIn IF;
-  IF.Symbols = indexStandardLibrary(std::move(CI), Loc, TFS);
-  if (Stdlib.isBest(LO))
-FIndex->updatePreamble(std::move(IF));
-};
-if (Tasks)
-  // This doesn't have a semaphore to enforce -j, but it's rare.
-  Tasks->runAsync("IndexStdlib", std::move(Task));
-else
-  Task();
-  }
-
   void onMainAST(PathRef Path, ParsedAST &AST, PublishFn Publish) override {
 if (FIndex)
   FIndex->updateMain(Path, AST);
@@ -127,9 +103,6 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
 private:
   FileIndex *FIndex;
   ClangdServer::Callbacks *ServerCallbacks;
-  const ThreadsafeFS &TFS;
-  StdLibSet Stdlib;
-  AsyncTaskRunner *Tasks;
 };
 
 class DraftStoreFS : public ThreadsafeFS {
@@ -181,15 +154,12 @@ ClangdServer::ClangdServer(const 
GlobalCompilationDatabase &CDB,
   Transient(Opts.ImplicitCancellation ? TUScheduler::InvalidateOnUpdate
   : TUScheduler::NoInvalidation),
   DirtyFS(std::make_unique(TFS, DraftMgr)) {
-  if (Opts.AsyncThreadsCount != 0)
-  

[PATCH] D125557: [APInt] Remove all uses of zextOrSelf, sextOrSelf and truncOrSelf

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: llvm/lib/IR/ConstantRange.cpp:724
 auto BW = getBitWidth();
-APInt Min = APInt::getMinValue(BW).zextOrSelf(ResultBitWidth);
-APInt Max = APInt::getMaxValue(BW).zextOrSelf(ResultBitWidth);
+APInt Min = APInt::getMinValue(BW);
+APInt Max = APInt::getMaxValue(BW);

foad wrote:
> foad wrote:
> > efriedma wrote:
> > > efriedma wrote:
> > > > Making the bitwidth of the result here not equal to ResultBitWidth 
> > > > seems suspect.
> > > > 
> > > > I think there should just be an `if (ResultBitWidth < BW) return 
> > > > getFull(ResultBitWidth);` here.  Then a simple conversion just works.
> > > Actually, looking at D27294 again, maybe it is actually making the result 
> > > bitwidth intentionally inflate like this.
> > > 
> > > This could use a comment explaining what it's doing, in any case.
> > I agree it could use a comment but I don't feel qualified to write it - I 
> > am just trying to preserve the current behaviour.
> @efriedma do you have any objection to the patch as-is?
No objection.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125557

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

[PATCH] D123649: Allow flexible array initialization in C++.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4639
+  getDataLayout().getTypeAllocSize(Init->getType()));
+  assert(VarSize == CstSize && "Emitted constant has unexpected size");
+#endif

ahatanak wrote:
> This assertion fails when the following code is compiled:
> 
> 
> ```
> typedef unsigned char uint8_t;
> typedef uint8_t uint8_a16 __attribute__((aligned(16)));
> 
> void foo1() {
>   static const uint8_a16 array1[] = { 1 };
> }
> ```
`sizeof(uint8_a16[1])` is 16, but we currently emit a one-byte global.  So it 
seems like there's an underlying bug exposed by the assertion.

gcc thinks this is nonsense, and just prints an error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123649

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

[PATCH] D91157: [AArch64] Out-of-line atomics (-moutline-atomics) implementation.

[Pavel Iliin via Phabricator via cfe-commits]

ilinpv added a comment.

I think it looks reasonable to define 5th memory model, add barriers __sync_* 
builtins and to outline-atomics calls as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91157

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

[PATCH] D125555: [clang] Add __has_target_feature

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/docs/LanguageExtensions.rst:275
+  // On amdgcn target
+  #if __has_target_feature("s-memtime-inst")
+x = __builtin_amdgcn_s_memtime();

yaxunl wrote:
> yaxunl wrote:
> > aaron.ballman wrote:
> > > tra wrote:
> > > > yaxunl wrote:
> > > > > tra wrote:
> > > > > > Do you have a better example? This particular case could've been 
> > > > > > handled by existing `__has_builtin()`.
> > > > > > 
> > > > > > While I could see usefulness of checking features (e.g. for 
> > > > > > CUDA/NVPTX it could be used to chose inline assembly supported only 
> > > > > > by newer PTX versions, but even then that choice could be made 
> > > > > > using existing methods, even if they are not always direct (e.g. by 
> > > > > > using CUDA_VERSION as a proxy for "new enough PTX version").
> > > > > > 
> > > > > `__has_builtin` returns 1 as long as the builtin is known to clang, 
> > > > > even if the builtin is not supported by the target CPU. This is 
> > > > > because the required target feature for a builtin is in ASTContext, 
> > > > > whereas `__has_builtin` is evaluated in preprocessor, where the 
> > > > > information is not known.
> > > > I'm missing something.  `__has_target_feature("s-memtime-inst")` is 
> > > > also evaluated by preprocessor, right next to where 
> > > > `__has_target_builtin` is processed.
> > > > I guess what you're saying is that preprocessor does not pay attention 
> > > > to the target feature conditions attached to those builtins.
> > > > Those are evaluted in `CodeGenFunction::checkTargetFeatures`.
> > > > 
> > > > This means that in order to use `__has_feature()` to detect 
> > > > availability of target builtins would effectively require the user to 
> > > > specify exactly the same set of conditions, as applied to the builtin. 
> > > > That will work for builtins that map to features 1:1, but it will be 
> > > > prone to breaking for NVPTX builtins that carry fairly large set of 
> > > > feature requirements and that set changes every time we add a new PTX 
> > > > or GPU variant, which happens fairly often.
> > > > 
> > > > I wonder if it may be better to generalize target builtin feature 
> > > > evaluation and use it from both preprocessor and the codegen and just 
> > > > stick with `__has_builtin`, only now working correctly to indicate 
> > > > availability of the target builtins.
> > > > 
> > > > 
> > > > 
> > > > 
> > > > 
> > > > `__has_builtin` returns 1 as long as the builtin is known to clang, 
> > > > even if the builtin is not supported by the target CPU. This is because 
> > > > the required target feature for a builtin is in ASTContext, whereas 
> > > > `__has_builtin` is evaluated in preprocessor, where the information is 
> > > > not known.
> > > 
> > > Why is that not a deficiency with `__has_builtin` that we should fix?
> > It is arguable whether this is a bug for `__has_builtin`. I tend to treat 
> > it as a bug and think it should be fixed. However, fixing it may cause 
> > regressions since there may be existing code expecting `__has_builtin` not 
> > depending on availability of required target feature. This will limit the 
> > usability of a fix for this issue.
> > 
> > Even if we agree that this is a bug for `__has_builtin` which should be 
> > fixed, this does conflict with adding `__has_target_feature`, as 
> > `__has_target_feature` has more uses than determining whether a target 
> > builtin is available, e.g. choosing different algorithms based on 
> > availability of certain target feature, or using certain inline assembly 
> > code.
> > It is arguable whether this is a bug for `__has_builtin`. I tend to treat 
> > it as a bug and think it should be fixed. However, fixing it may cause 
> > regressions since there may be existing code expecting `__has_builtin` not 
> > depending on availability of required target feature. This will limit the 
> > usability of a fix for this issue.
> > 
> > Even if we agree that this is a bug for `__has_builtin` which should be 
> > fixed, this does conflict with adding `__has_target_feature`, as 
> > `__has_target_feature` has more uses than determining whether a target 
> > builtin is available, e.g. choosing different algorithms based on 
> > availability of certain target feature, or using certain inline assembly 
> > code.
> 
> sorry typo. this does not conflict
Opened https://reviews.llvm.org/D125829 to fix `__has_builtin`


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

https://reviews.llvm.org/D12

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

[PATCH] D125829: [clang] Fix __has_builtin

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: tra, aaron.ballman, rsmith.
Herald added a project: All.
yaxunl requested review of this revision.

Fix `__has_builtin` to return 1 only if the requested target features
of a builtin are enabled by refactoring the code for checking
required target features of a builtin and use it in evaluation
of `__has_builtin`.


https://reviews.llvm.org/D125829

Files:
  clang/include/clang/Basic/Builtins.h
  clang/lib/Basic/BuiltinTargetFeatures.h
  clang/lib/Basic/Builtins.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/Preprocessor/hash_builtin.cpp
  clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp

Index: clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp
===
--- clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp
+++ clang/unittests/CodeGen/CheckTargetFeaturesTest.cpp
@@ -1,4 +1,4 @@
-#include "../lib/CodeGen/CodeGenFunction.h"
+#include "../lib/Basic/BuiltinTargetFeatures.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
@@ -11,7 +11,7 @@
 StringMap SM;
 for (StringRef F : Features)
   SM.insert(std::make_pair(F, true));
-clang::CodeGen::TargetFeatures TF(SM);
+clang::Builtin::TargetFeatures TF(SM);
 return TF.hasRequiredFeatures(BuiltinFeatures);
   };
   // Make sure the basic function ',' and '|' works correctly
Index: clang/test/Preprocessor/hash_builtin.cpp
===
--- /dev/null
+++ clang/test/Preprocessor/hash_builtin.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple amdgcn -target-cpu gfx906 -E %s -o - | FileCheck %s
+
+// CHECK: has_s_memtime_inst
+#if __has_builtin(__builtin_amdgcn_s_memtime)
+  int has_s_memtime_inst;
+#endif
+
+// CHECK-NOT: has_gfx10_inst
+#if __has_builtin(__builtin_amdgcn_mov_dpp8)
+  int has_gfx10_inst;
+#endif
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -1640,7 +1640,8 @@
 // usual allocation and deallocation functions. Required by libc++
 return 201802;
   default:
-return true;
+return getBuiltinInfo().isRequiredTargetFeaturesEnabled(
+II->getBuiltinID(), getTargetInfo().getTargetOpts().FeatureMap);
   }
   return true;
 } else if (II->getTokenID() != tok::identifier ||
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -4840,76 +4840,6 @@
   llvm::Value *FormResolverCondition(const MultiVersionResolverOption &RO);
 };
 
-/// TargetFeatures - This class is used to check whether the builtin function
-/// has the required tagert specific features. It is able to support the
-/// combination of ','(and), '|'(or), and '()'. By default, the priority of
-/// ',' is higher than that of '|' .
-/// E.g:
-/// A,B|C means the builtin function requires both A and B, or C.
-/// If we want the builtin function requires both A and B, or both A and C,
-/// there are two ways: A,B|A,C or A,(B|C).
-/// The FeaturesList should not contain spaces, and brackets must appear in
-/// pairs.
-class TargetFeatures {
-  struct FeatureListStatus {
-bool HasFeatures;
-StringRef CurFeaturesList;
-  };
-
-  const llvm::StringMap &CallerFeatureMap;
-
-  FeatureListStatus getAndFeatures(StringRef FeatureList) {
-int InParentheses = 0;
-bool HasFeatures = true;
-size_t SubexpressionStart = 0;
-for (size_t i = 0, e = FeatureList.size(); i < e; ++i) {
-  char CurrentToken = FeatureList[i];
-  switch (CurrentToken) {
-  default:
-break;
-  case '(':
-if (InParentheses == 0)
-  SubexpressionStart = i + 1;
-++InParentheses;
-break;
-  case ')':
---InParentheses;
-assert(InParentheses >= 0 && "Parentheses are not in pair");
-LLVM_FALLTHROUGH;
-  case '|':
-  case ',':
-if (InParentheses == 0) {
-  if (HasFeatures && i != SubexpressionStart) {
-StringRef F = FeatureList.slice(SubexpressionStart, i);
-HasFeatures = CurrentToken == ')' ? hasRequiredFeatures(F)
-  : CallerFeatureMap.lookup(F);
-  }
-  SubexpressionStart = i + 1;
-  if (CurrentToken == '|') {
-return {HasFeatures, FeatureList.substr(SubexpressionStart)};
-  }
-}
-break;
-  }
-}
-assert(InParentheses == 0 && "Parentheses are not in pair");
-if (HasFeatures && SubexpressionStart != FeatureList.size())
-  HasFeatures =
-  CallerFeatureMap.lookup(FeatureList.substr(SubexpressionStart));
-return {HasFeatures, S

[PATCH] D125821: [clang][dataflow] Fix double visitation of nested logical operators

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua marked an inline comment as done.
li.zhe.hua added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:574
 
-// Sub-expressions that are logic operators are not added in basic blocks
-// (e.g. see CFG for `bool d = a && (b || c);`). If `SubExpr` is a logic
-// operator, it isn't evaluated and assigned a value yet. In that case, we
-// need to first visit `SubExpr` and then try to get the value that gets
-// assigned to it.
-Visit(&SubExpr);
-if (auto *Val = dyn_cast_or_null(
-Env.getValue(SubExpr, SkipPast::Reference)))
+auto *SubExprLoc = Env.getStorageLocation(SubExpr, SkipPast::Reference);
+if (SubExprLoc == nullptr) {

ymandel wrote:
> I'm afraid this may allow us to hide a bug.  Specifically: consider if 
> `SubExpr` was already evaluated to a RefenceValue and that value's 
> StorageLocation does *not* map to a value. Then, this line will be true, but 
> we won't want to revisit the `SubExrp`. Now, that may be impossible in 
> practice, so I'm not sure how big a problem this is, but it seems safer to 
> just use `SkipPast::None` and not rely on any guarantees.
> 
> That said, i see why  you want the uniformity of `Reference` because it is 
> used below. That said, any idea if we actually need to use `Reference` 
> skipping at all? I think so -- the case of a variable or field access as 
> sub-expression, but those probably have a cast around them anyhow, so i'm not 
> sure those will require reference skipping.
> I'm afraid this may allow us to hide a bug.

Ah, yes, that is definitely true.

> That said, any idea if we actually need to use `Reference` skipping at all?

Whatever we choose, I imagine it should be kept the same as the analogous line 
in [[ 
https://github.com/llvm/llvm-project/blob/118c5d1c97b4191678663bf2a938eee7dec6f0b1/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp#L112-L113
 | extendFlowCondition ]]? I've opted to not change it for now, given my 
uncertainty.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125821

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

[PATCH] D125821: [clang][dataflow] Fix double visitation of nested logical operators

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua updated this revision to Diff 430152.
li.zhe.hua added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125821

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


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2467,6 +2467,67 @@
   EXPECT_EQ(&BazVal->getRightSubValue(), BarVal);
 });
   }
+
+  {
+std::string Code = R"(
+  void target(bool A, bool B, bool C, bool D) {
+bool Foo = ((A && B) && C) && D;
+// [[p]]
+  }
+)";
+runDataflow(
+Code, [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+  ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+  const Environment &Env = Results[0].second.Env;
+
+  const ValueDecl *ADecl = findValueDecl(ASTCtx, "A");
+  ASSERT_THAT(ADecl, NotNull());
+
+  const auto *AVal =
+  dyn_cast_or_null(Env.getValue(*ADecl, 
SkipPast::None));
+  ASSERT_THAT(AVal, NotNull());
+
+  const ValueDecl *BDecl = findValueDecl(ASTCtx, "B");
+  ASSERT_THAT(BDecl, NotNull());
+
+  const auto *BVal =
+  dyn_cast_or_null(Env.getValue(*BDecl, 
SkipPast::None));
+  ASSERT_THAT(BVal, NotNull());
+
+  const ValueDecl *CDecl = findValueDecl(ASTCtx, "C");
+  ASSERT_THAT(CDecl, NotNull());
+
+  const auto *CVal =
+  dyn_cast_or_null(Env.getValue(*CDecl, 
SkipPast::None));
+  ASSERT_THAT(CVal, NotNull());
+
+  const ValueDecl *DDecl = findValueDecl(ASTCtx, "D");
+  ASSERT_THAT(DDecl, NotNull());
+
+  const auto *DVal =
+  dyn_cast_or_null(Env.getValue(*DDecl, 
SkipPast::None));
+  ASSERT_THAT(DVal, NotNull());
+
+  const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+  ASSERT_THAT(FooDecl, NotNull());
+
+  const auto *FooVal = dyn_cast_or_null(
+  Env.getValue(*FooDecl, SkipPast::None));
+  ASSERT_THAT(FooVal, NotNull());
+
+  const auto &FooLeftSubVal =
+  cast(FooVal->getLeftSubValue());
+  const auto &FooLeftLeftSubVal =
+  cast(FooLeftSubVal.getLeftSubValue());
+  EXPECT_EQ(&FooLeftLeftSubVal.getLeftSubValue(), AVal);
+  EXPECT_EQ(&FooLeftLeftSubVal.getRightSubValue(), BVal);
+  EXPECT_EQ(&FooLeftSubVal.getRightSubValue(), CVal);
+  EXPECT_EQ(&FooVal->getRightSubValue(), DVal);
+});
+  }
 }
 
 TEST_F(TransferTest, AssignFromBoolNegation) {
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -571,12 +571,15 @@
 return *Val;
 }
 
-// Sub-expressions that are logic operators are not added in basic blocks
-// (e.g. see CFG for `bool d = a && (b || c);`). If `SubExpr` is a logic
-// operator, it isn't evaluated and assigned a value yet. In that case, we
-// need to first visit `SubExpr` and then try to get the value that gets
-// assigned to it.
-Visit(&SubExpr);
+if (Env.getStorageLocation(SubExpr, SkipPast::None) == nullptr) {
+  // Sub-expressions that are logic operators are not added in basic blocks
+  // (e.g. see CFG for `bool d = a && (b || c);`). If `SubExpr` is a logic
+  // operator, it may not have been evaluated and assigned a value yet. In
+  // that case, we need to first visit `SubExpr` and then try to get the
+  // value that gets assigned to it.
+  Visit(&SubExpr);
+}
+
 if (auto *Val = dyn_cast_or_null(
 Env.getValue(SubExpr, SkipPast::Reference)))
   return *Val;


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2467,6 +2467,67 @@
   EXPECT_EQ(&BazVal->getRightSubValue(), BarVal);
 });
   }
+
+  {
+std::string Code = R"(
+  void target(bool A, bool B, bool C, bool D) {
+bool Foo = ((A && B) && C) && D;
+// [[p]]
+  }
+)";
+runDataflow(
+Code, [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+  ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+  const Environment &Env = Results[0].second.Env;
+
+  const ValueDecl *ADecl = findValueDecl(AST

[clang-tools-extra] ccdb56a - Reland "[clangd] Indexing of standard library"

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-05-17T21:02:23+02:00
New Revision: ccdb56ac10eef3048135169a67d239328c2b1de6

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

LOG: Reland "[clangd] Indexing of standard library"

This reverts commit 76ddbb1ca747366417be64fdf79218df099a5973.

Added: 
clang-tools-extra/clangd/index/StdLib.cpp
clang-tools-extra/clangd/index/StdLib.h
clang-tools-extra/clangd/unittests/StdLibTests.cpp

Modified: 
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/TUScheduler.h
clang-tools-extra/clangd/index/FileIndex.cpp
clang-tools-extra/clangd/index/FileIndex.h
clang-tools-extra/clangd/index/SymbolOrigin.cpp
clang-tools-extra/clangd/index/SymbolOrigin.h
clang-tools-extra/clangd/unittests/CMakeLists.txt
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CMakeLists.txt 
b/clang-tools-extra/clangd/CMakeLists.txt
index 9c37cfe7b700..7cfbd6f95750 100644
--- a/clang-tools-extra/clangd/CMakeLists.txt
+++ b/clang-tools-extra/clangd/CMakeLists.txt
@@ -119,6 +119,7 @@ add_clang_library(clangDaemon
   index/Ref.cpp
   index/Relation.cpp
   index/Serialization.cpp
+  index/StdLib.cpp
   index/Symbol.cpp
   index/SymbolCollector.cpp
   index/SymbolID.cpp

diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 80d7d5c5ece1..69a0f63972aa 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -26,6 +26,7 @@
 #include "index/CanonicalIncludes.h"
 #include "index/FileIndex.h"
 #include "index/Merge.h"
+#include "index/StdLib.h"
 #include "refactor/Rename.h"
 #include "refactor/Tweak.h"
 #include "support/Cancellation.h"
@@ -59,16 +60,39 @@ namespace {
 // Update the FileIndex with new ASTs and plumb the diagnostics responses.
 struct UpdateIndexCallbacks : public ParsingCallbacks {
   UpdateIndexCallbacks(FileIndex *FIndex,
-   ClangdServer::Callbacks *ServerCallbacks)
-  : FIndex(FIndex), ServerCallbacks(ServerCallbacks) {}
+   ClangdServer::Callbacks *ServerCallbacks,
+   const ThreadsafeFS &TFS, AsyncTaskRunner *Tasks)
+  : FIndex(FIndex), ServerCallbacks(ServerCallbacks), TFS(TFS),
+Tasks(Tasks) {}
 
-  void onPreambleAST(PathRef Path, llvm::StringRef Version, ASTContext &Ctx,
+  void onPreambleAST(PathRef Path, llvm::StringRef Version,
+ const CompilerInvocation &CI, ASTContext &Ctx,
  Preprocessor &PP,
  const CanonicalIncludes &CanonIncludes) override {
+// If this preamble uses a standard library we haven't seen yet, index it.
+if (FIndex)
+  if (auto Loc = Stdlib.add(*CI.getLangOpts(), PP.getHeaderSearchInfo()))
+indexStdlib(CI, std::move(*Loc));
+
 if (FIndex)
   FIndex->updatePreamble(Path, Version, Ctx, PP, CanonIncludes);
   }
 
+  void indexStdlib(const CompilerInvocation &CI, StdLibLocation Loc) {
+auto Task = [this, LO(*CI.getLangOpts()), Loc(std::move(Loc)),
+ CI(std::make_unique(CI))]() mutable {
+  IndexFileIn IF;
+  IF.Symbols = indexStandardLibrary(std::move(CI), Loc, TFS);
+  if (Stdlib.isBest(LO))
+FIndex->updatePreamble(std::move(IF));
+};
+if (Tasks)
+  // This doesn't have a semaphore to enforce -j, but it's rare.
+  Tasks->runAsync("IndexStdlib", std::move(Task));
+else
+  Task();
+  }
+
   void onMainAST(PathRef Path, ParsedAST &AST, PublishFn Publish) override {
 if (FIndex)
   FIndex->updateMain(Path, AST);
@@ -103,6 +127,9 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
 private:
   FileIndex *FIndex;
   ClangdServer::Callbacks *ServerCallbacks;
+  const ThreadsafeFS &TFS;
+  StdLibSet Stdlib;
+  AsyncTaskRunner *Tasks;
 };
 
 class DraftStoreFS : public ThreadsafeFS {
@@ -154,12 +181,15 @@ ClangdServer::ClangdServer(const 
GlobalCompilationDatabase &CDB,
   Transient(Opts.ImplicitCancellation ? TUScheduler::InvalidateOnUpdate
   : TUScheduler::NoInvalidation),
   DirtyFS(std::make_unique(TFS, DraftMgr)) {
+  if (Opts.AsyncThreadsCount != 0)
+IndexTasks.emplace();
   // Pass a callback into `WorkScheduler` to extract symbols from a newly
   // parsed file and rebuild the file index synchronously each time an AST
   // is parsed.
-  WorkScheduler.empl

[PATCH] D125823: [clang][dataflow] Weaken guard to only check for storage location

[Eric Li 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 rG854c273cbb7e: [clang][dataflow] Weaken guard to only check 
for storage location (authored by li.zhe.hua).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125823

Files:
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp


Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -104,7 +104,7 @@
 private:
   void extendFlowCondition(const Expr &Cond) {
 // The terminator sub-expression might not be evaluated.
-if (Env.getValue(Cond, SkipPast::None) == nullptr)
+if (Env.getStorageLocation(Cond, SkipPast::None) == nullptr)
   transfer(StmtToEnv, Cond, Env);
 
 // FIXME: The flow condition must be an r-value, so `SkipPast::None` should


Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -104,7 +104,7 @@
 private:
   void extendFlowCondition(const Expr &Cond) {
 // The terminator sub-expression might not be evaluated.
-if (Env.getValue(Cond, SkipPast::None) == nullptr)
+if (Env.getStorageLocation(Cond, SkipPast::None) == nullptr)
   transfer(StmtToEnv, Cond, Env);
 
 // FIXME: The flow condition must be an r-value, so `SkipPast::None` should
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 854c273 - [clang][dataflow] Weaken guard to only check for storage location

[Eric Li via cfe-commits]

Author: Eric Li
Date: 2022-05-17T18:58:07Z
New Revision: 854c273cbb7ec6745dc63cfe1951b51e9092e8ee

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

LOG: [clang][dataflow] Weaken guard to only check for storage location

Weaken the guard for whether a sub-expression has been evaluated to
only check for the storage location, instead of checking for the
value. It should be sufficient to check for the storage location, as
we don't necessarily guarantee that a value will be set for the
location (although this is currently true right now).

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

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp 
b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index a8d739c155ab..aebc9dd6f6fa 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -104,7 +104,7 @@ class TerminatorVisitor : public 
ConstStmtVisitor {
 private:
   void extendFlowCondition(const Expr &Cond) {
 // The terminator sub-expression might not be evaluated.
-if (Env.getValue(Cond, SkipPast::None) == nullptr)
+if (Env.getStorageLocation(Cond, SkipPast::None) == nullptr)
   transfer(StmtToEnv, Cond, Env);
 
 // FIXME: The flow condition must be an r-value, so `SkipPast::None` should



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

[PATCH] D125506: [PowerPC] Implement XL compat __fnabs and __fnabss builtins.

[Lei Huang via Phabricator via cfe-commits]

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

LGTM with minor updates before commit.




Comment at: clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-fnabs.c:25
+// RUN: %clang_cc1 -no-opaque-pointers -triple powerpc-unknown-aix \
+// RUN:   -emit-llvm %s -target-cpu pwr6 -o - | FileCheck %s
+

Since we emit `xsnabsdp` for pwr7 and above I don't think it's necessary to 
have all combination tested on both pwr7 and pwr8.




Comment at: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-fnabs.ll:24
+
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   -mattr=-vsx < %s | FileCheck %s --check-prefix=CHECK-NOVSX

I think the default pwr level is lower then pwr6 which means it's no vsx by 
default.  Maybe add `pwr[7|8]` to these test lines to test the `-vsx` attr?



Comment at: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-fnabs.ll:45
+; CHECK-PWR6-NEXT:fnabs 1, 1
+; CHECK-PWR6-NEXT:blr
+;

probably don't need both NOVSX and PWR6 checks since they are the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125506

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

[PATCH] D125821: [clang][dataflow] Fix double visitation of nested logical operators

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:574
 
-// Sub-expressions that are logic operators are not added in basic blocks
-// (e.g. see CFG for `bool d = a && (b || c);`). If `SubExpr` is a logic
-// operator, it isn't evaluated and assigned a value yet. In that case, we
-// need to first visit `SubExpr` and then try to get the value that gets
-// assigned to it.
-Visit(&SubExpr);
-if (auto *Val = dyn_cast_or_null(
-Env.getValue(SubExpr, SkipPast::Reference)))
+auto *SubExprLoc = Env.getStorageLocation(SubExpr, SkipPast::Reference);
+if (SubExprLoc == nullptr) {

I'm afraid this may allow us to hide a bug.  Specifically: consider if 
`SubExpr` was already evaluated to a RefenceValue and that value's 
StorageLocation does *not* map to a value. Then, this line will be true, but we 
won't want to revisit the `SubExrp`. Now, that may be impossible in practice, 
so I'm not sure how big a problem this is, but it seems safer to just use 
`SkipPast::None` and not rely on any guarantees.

That said, i see why  you want the uniformity of `Reference` because it is used 
below. That said, any idea if we actually need to use `Reference` skipping at 
all? I think so -- the case of a variable or field access as sub-expression, 
but those probably have a cast around them anyhow, so i'm not sure those will 
require reference skipping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125821

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

[PATCH] D125823: [clang][dataflow] Weaken guard to only check for storage location

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua created this revision.
li.zhe.hua added a reviewer: ymandel.
Herald added subscribers: tschuett, steakhal.
Herald added a project: All.
li.zhe.hua requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Weaken the guard for whether a sub-expression has been evaluated to
only check for the storage location, instead of checking for the
value. It should be sufficient to check for the storage location, as
we don't necessarily guarantee that a value will be set for the
location (although this is currently true right now).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125823

Files:
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp


Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -104,7 +104,7 @@
 private:
   void extendFlowCondition(const Expr &Cond) {
 // The terminator sub-expression might not be evaluated.
-if (Env.getValue(Cond, SkipPast::None) == nullptr)
+if (Env.getStorageLocation(Cond, SkipPast::None) == nullptr)
   transfer(StmtToEnv, Cond, Env);
 
 // FIXME: The flow condition must be an r-value, so `SkipPast::None` should


Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -104,7 +104,7 @@
 private:
   void extendFlowCondition(const Expr &Cond) {
 // The terminator sub-expression might not be evaluated.
-if (Env.getValue(Cond, SkipPast::None) == nullptr)
+if (Env.getStorageLocation(Cond, SkipPast::None) == nullptr)
   transfer(StmtToEnv, Cond, Env);
 
 // FIXME: The flow condition must be an r-value, so `SkipPast::None` should
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123649: Allow flexible array initialization in C++.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4639
+  getDataLayout().getTypeAllocSize(Init->getType()));
+  assert(VarSize == CstSize && "Emitted constant has unexpected size");
+#endif

This assertion fails when the following code is compiled:


```
typedef unsigned char uint8_t;
typedef uint8_t uint8_a16 __attribute__((aligned(16)));

void foo1() {
  static const uint8_a16 array1[] = { 1 };
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123649

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

[PATCH] D125821: [clang][dataflow] Fix double visitation of nested logical operators

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua created this revision.
li.zhe.hua added a reviewer: ymandel.
Herald added subscribers: tschuett, steakhal.
Herald added a project: All.
li.zhe.hua requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Sub-expressions that are logical operators are not spelled out
separately in basic blocks, so we need to manually visit them when we
encounter them. We do this in both the `TerminatorVisitor`
(conditionally) and the `TransferVisitor` (unconditionally), which can
cause cause an expression to be visited twice when the binary
operators are nested 2+ times.

This changes the visit in `TransferVisitor` to check if it has been
evaluated before trying to visit the sub-expression.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125821

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

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2467,6 +2467,67 @@
   EXPECT_EQ(&BazVal->getRightSubValue(), BarVal);
 });
   }
+
+  {
+std::string Code = R"(
+  void target(bool A, bool B, bool C, bool D) {
+bool Foo = ((A && B) && C) && D;
+// [[p]]
+  }
+)";
+runDataflow(
+Code, [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+  ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+  const Environment &Env = Results[0].second.Env;
+
+  const ValueDecl *ADecl = findValueDecl(ASTCtx, "A");
+  ASSERT_THAT(ADecl, NotNull());
+
+  const auto *AVal =
+  dyn_cast_or_null(Env.getValue(*ADecl, SkipPast::None));
+  ASSERT_THAT(AVal, NotNull());
+
+  const ValueDecl *BDecl = findValueDecl(ASTCtx, "B");
+  ASSERT_THAT(BDecl, NotNull());
+
+  const auto *BVal =
+  dyn_cast_or_null(Env.getValue(*BDecl, SkipPast::None));
+  ASSERT_THAT(BVal, NotNull());
+
+  const ValueDecl *CDecl = findValueDecl(ASTCtx, "C");
+  ASSERT_THAT(CDecl, NotNull());
+
+  const auto *CVal =
+  dyn_cast_or_null(Env.getValue(*CDecl, SkipPast::None));
+  ASSERT_THAT(CVal, NotNull());
+
+  const ValueDecl *DDecl = findValueDecl(ASTCtx, "D");
+  ASSERT_THAT(DDecl, NotNull());
+
+  const auto *DVal =
+  dyn_cast_or_null(Env.getValue(*DDecl, SkipPast::None));
+  ASSERT_THAT(DVal, NotNull());
+
+  const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+  ASSERT_THAT(FooDecl, NotNull());
+
+  const auto *FooVal = dyn_cast_or_null(
+  Env.getValue(*FooDecl, SkipPast::None));
+  ASSERT_THAT(FooVal, NotNull());
+
+  const auto &FooLeftSubVal =
+  cast(FooVal->getLeftSubValue());
+  const auto &FooLeftLeftSubVal =
+  cast(FooLeftSubVal.getLeftSubValue());
+  EXPECT_EQ(&FooLeftLeftSubVal.getLeftSubValue(), AVal);
+  EXPECT_EQ(&FooLeftLeftSubVal.getRightSubValue(), BVal);
+  EXPECT_EQ(&FooLeftSubVal.getRightSubValue(), CVal);
+  EXPECT_EQ(&FooVal->getRightSubValue(), DVal);
+});
+  }
 }
 
 TEST_F(TransferTest, AssignFromBoolNegation) {
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -571,14 +571,18 @@
 return *Val;
 }
 
-// Sub-expressions that are logic operators are not added in basic blocks
-// (e.g. see CFG for `bool d = a && (b || c);`). If `SubExpr` is a logic
-// operator, it isn't evaluated and assigned a value yet. In that case, we
-// need to first visit `SubExpr` and then try to get the value that gets
-// assigned to it.
-Visit(&SubExpr);
-if (auto *Val = dyn_cast_or_null(
-Env.getValue(SubExpr, SkipPast::Reference)))
+auto *SubExprLoc = Env.getStorageLocation(SubExpr, SkipPast::Reference);
+if (SubExprLoc == nullptr) {
+  // Sub-expressions that are logic operators are not added in basic blocks
+  // (e.g. see CFG for `bool d = a && (b || c);`). If `SubExpr` is a logic
+  // operator, it may not have been evaluated and assigned a value yet. In
+  // that case, we need to first visit `SubExpr` and then try to get the
+  // value that gets assigned to it.
+  Visit(&SubExpr);
+  SubExprLoc = Env.getStorageLocation(SubExpr, SkipPast::Reference);
+  assert(SubExprLoc != nullptr);
+}
+if (auto *Val = dyn_cast_or_null(Env.getValue(*SubExprLoc)))
   return *Val;
 
 // If the value of `SubExpr` is still unknown, we 

[PATCH] D125312: [pseudo] benchmark cleanups. NFC

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe8e00e342c4f: [pseudo] benchmark cleanups. NFC (authored by 
sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D125312?vs=428357&id=430135#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125312

Files:
  clang-tools-extra/pseudo/benchmarks/Benchmark.cpp

Index: clang-tools-extra/pseudo/benchmarks/Benchmark.cpp
===
--- clang-tools-extra/pseudo/benchmarks/Benchmark.cpp
+++ clang-tools-extra/pseudo/benchmarks/Benchmark.cpp
@@ -10,12 +10,12 @@
 // important pieces of the pseudoparser (grammar compliation, LR table build
 // etc).
 //
-// Note: make sure we build it in Relase mode.
+// Note: make sure to build the benchmark in Release mode.
 //
 // Usage:
 //   tools/clang/tools/extra/pseudo/benchmarks/ClangPseudoBenchmark \
-//  --grammar=/path/to/cxx.bnf --source=/patch/to/source-to-parse.cpp \
-//  --benchmark_filter=runParseOverall
+//  --grammar=../clang-tools-extra/pseudo/lib/cxx.bnf \
+//  --source=../clang/lib/Sema/SemaDecl.cpp
 //
 //===--===//
 
@@ -35,16 +35,17 @@
 #include 
 
 using llvm::cl::desc;
-using llvm::cl::init;
 using llvm::cl::opt;
+using llvm::cl::Required;
 
 static opt GrammarFile("grammar",
 desc("Parse and check a BNF grammar file."),
-init(""));
-static opt Source("source", desc("Source file"));
+Required);
+static opt Source("source", desc("Source file"), Required);
 
 namespace clang {
 namespace pseudo {
+namespace bench {
 namespace {
 
 const std::string *GrammarText = nullptr;
@@ -68,75 +69,90 @@
   G = Grammar::parseBNF(*GrammarText, Diags).release();
 }
 
-static void runParseBNFGrammar(benchmark::State &State) {
+static void parseBNF(benchmark::State &State) {
   std::vector Diags;
   for (auto _ : State)
 Grammar::parseBNF(*GrammarText, Diags);
 }
-BENCHMARK(runParseBNFGrammar);
+BENCHMARK(parseBNF);
 
-static void runBuildLR(benchmark::State &State) {
+static void buildSLR(benchmark::State &State) {
   for (auto _ : State)
-clang::pseudo::LRTable::buildSLR(*G);
+LRTable::buildSLR(*G);
 }
-BENCHMARK(runBuildLR);
+BENCHMARK(buildSLR);
 
-TokenStream parseableTokenStream() {
+TokenStream lexAndPreprocess() {
   clang::LangOptions LangOpts = genericLangOpts();
-  TokenStream RawStream = clang::pseudo::lex(*SourceText, LangOpts);
+  TokenStream RawStream = pseudo::lex(*SourceText, LangOpts);
   auto DirectiveStructure = DirectiveTree::parse(RawStream);
-  clang::pseudo::chooseConditionalBranches(DirectiveStructure, RawStream);
+  chooseConditionalBranches(DirectiveStructure, RawStream);
   TokenStream Cook =
   cook(DirectiveStructure.stripDirectives(RawStream), LangOpts);
-  return clang::pseudo::stripComments(Cook);
+  return stripComments(Cook);
 }
 
-static void runPreprocessTokens(benchmark::State &State) {
+static void lex(benchmark::State &State) {
+  clang::LangOptions LangOpts = genericLangOpts();
   for (auto _ : State)
-parseableTokenStream();
+clang::pseudo::lex(*SourceText, LangOpts);
   State.SetBytesProcessed(static_cast(State.iterations()) *
   SourceText->size());
 }
-BENCHMARK(runPreprocessTokens);
+BENCHMARK(lex);
 
-static void runGLRParse(benchmark::State &State) {
+static void preprocess(benchmark::State &State) {
   clang::LangOptions LangOpts = genericLangOpts();
+  TokenStream RawStream = clang::pseudo::lex(*SourceText, LangOpts);
+  for (auto _ : State) {
+auto DirectiveStructure = DirectiveTree::parse(RawStream);
+chooseConditionalBranches(DirectiveStructure, RawStream);
+stripComments(
+cook(DirectiveStructure.stripDirectives(RawStream), LangOpts));
+  }
+  State.SetBytesProcessed(static_cast(State.iterations()) *
+  SourceText->size());
+}
+BENCHMARK(preprocess);
+
+static void glrParse(benchmark::State &State) {
   LRTable Table = clang::pseudo::LRTable::buildSLR(*G);
-  TokenStream ParseableStream = parseableTokenStream();
   SymbolID StartSymbol = *G->findNonterminal("translation-unit");
+  TokenStream Stream = lexAndPreprocess();
   for (auto _ : State) {
 pseudo::ForestArena Forest;
 pseudo::GSS GSS;
-glrParse(ParseableStream, ParseParams{*G, Table, Forest, GSS}, StartSymbol);
+pseudo::glrParse(Stream, ParseParams{*G, Table, Forest, GSS}, StartSymbol);
   }
   State.SetBytesProcessed(static_cast(State.iterations()) *
   SourceText->size());
 }
-BENCHMARK(runGLRParse);
+BENCHMARK(glrParse);
 
-static void runParseOverall(benchmark::State &State) {
-  clang::LangOptions LangOpts = genericLangOpts();
+static void full(benchmark::State &State) {
   LRTable Table = clang::ps

[clang-tools-extra] e8e00e3 - [pseudo] benchmark cleanups. NFC

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-05-17T20:22:42+02:00
New Revision: e8e00e342c4fadc8355d1dfb8de86fb0a3dcd5f7

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

LOG: [pseudo] benchmark cleanups. NFC

- add missing benchmark for lex/preprocess steps
- name benchmarks after the function they're benchmarking, when appropriate
- remove unergonomic "run" prefixes from benchmark names
- give a useful error message if --grammar or --source are missing
- Use realistic example of how to run, run all benchmarks by default.
  (for someone who doesn't know the commands, this is the most useful action)
- Improve typos/wording in comment
- clean up unused vars
- avoid "parseable stream" name, which isn't a great name & not one I expected
  to escape from ClangPseudoMain

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

Added: 


Modified: 
clang-tools-extra/pseudo/benchmarks/Benchmark.cpp

Removed: 




diff  --git a/clang-tools-extra/pseudo/benchmarks/Benchmark.cpp 
b/clang-tools-extra/pseudo/benchmarks/Benchmark.cpp
index 5518a51166593..fb028be0c7abd 100644
--- a/clang-tools-extra/pseudo/benchmarks/Benchmark.cpp
+++ b/clang-tools-extra/pseudo/benchmarks/Benchmark.cpp
@@ -10,12 +10,12 @@
 // important pieces of the pseudoparser (grammar compliation, LR table build
 // etc).
 //
-// Note: make sure we build it in Relase mode.
+// Note: make sure to build the benchmark in Release mode.
 //
 // Usage:
 //   tools/clang/tools/extra/pseudo/benchmarks/ClangPseudoBenchmark \
-//  --grammar=/path/to/cxx.bnf --source=/patch/to/source-to-parse.cpp \
-//  --benchmark_filter=runParseOverall
+//  --grammar=../clang-tools-extra/pseudo/lib/cxx.bnf \
+//  --source=../clang/lib/Sema/SemaDecl.cpp
 //
 
//===--===//
 
@@ -35,16 +35,17 @@
 #include 
 
 using llvm::cl::desc;
-using llvm::cl::init;
 using llvm::cl::opt;
+using llvm::cl::Required;
 
 static opt GrammarFile("grammar",
 desc("Parse and check a BNF grammar 
file."),
-init(""));
-static opt Source("source", desc("Source file"));
+Required);
+static opt Source("source", desc("Source file"), Required);
 
 namespace clang {
 namespace pseudo {
+namespace bench {
 namespace {
 
 const std::string *GrammarText = nullptr;
@@ -68,75 +69,90 @@ void setupGrammarAndSource() {
   G = Grammar::parseBNF(*GrammarText, Diags).release();
 }
 
-static void runParseBNFGrammar(benchmark::State &State) {
+static void parseBNF(benchmark::State &State) {
   std::vector Diags;
   for (auto _ : State)
 Grammar::parseBNF(*GrammarText, Diags);
 }
-BENCHMARK(runParseBNFGrammar);
+BENCHMARK(parseBNF);
 
-static void runBuildLR(benchmark::State &State) {
+static void buildSLR(benchmark::State &State) {
   for (auto _ : State)
-clang::pseudo::LRTable::buildSLR(*G);
+LRTable::buildSLR(*G);
 }
-BENCHMARK(runBuildLR);
+BENCHMARK(buildSLR);
 
-TokenStream parseableTokenStream() {
+TokenStream lexAndPreprocess() {
   clang::LangOptions LangOpts = genericLangOpts();
-  TokenStream RawStream = clang::pseudo::lex(*SourceText, LangOpts);
+  TokenStream RawStream = pseudo::lex(*SourceText, LangOpts);
   auto DirectiveStructure = DirectiveTree::parse(RawStream);
-  clang::pseudo::chooseConditionalBranches(DirectiveStructure, RawStream);
+  chooseConditionalBranches(DirectiveStructure, RawStream);
   TokenStream Cook =
   cook(DirectiveStructure.stripDirectives(RawStream), LangOpts);
-  return clang::pseudo::stripComments(Cook);
+  return stripComments(Cook);
 }
 
-static void runPreprocessTokens(benchmark::State &State) {
+static void lex(benchmark::State &State) {
+  clang::LangOptions LangOpts = genericLangOpts();
   for (auto _ : State)
-parseableTokenStream();
+clang::pseudo::lex(*SourceText, LangOpts);
   State.SetBytesProcessed(static_cast(State.iterations()) *
   SourceText->size());
 }
-BENCHMARK(runPreprocessTokens);
+BENCHMARK(lex);
 
-static void runGLRParse(benchmark::State &State) {
+static void preprocess(benchmark::State &State) {
   clang::LangOptions LangOpts = genericLangOpts();
+  TokenStream RawStream = clang::pseudo::lex(*SourceText, LangOpts);
+  for (auto _ : State) {
+auto DirectiveStructure = DirectiveTree::parse(RawStream);
+chooseConditionalBranches(DirectiveStructure, RawStream);
+stripComments(
+cook(DirectiveStructure.stripDirectives(RawStream), LangOpts));
+  }
+  State.SetBytesProcessed(static_cast(State.iterations()) *
+  SourceText->size());
+}
+BENCHMARK(preprocess);
+
+static void glrParse(benchmark::State &State) {
   LRTable Table = clang::pseudo::LRTable::buildSLR(*G);
-

[clang-tools-extra] 127a149 - [clangd] Add command-line flag to set background indexing thread priority.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-05-17T20:17:07+02:00
New Revision: 127a1492d72902e2b2cd8905c1198743761f52fb

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

LOG: [clangd] Add command-line flag to set background indexing thread priority.

This is a followup to D124715, which changed the default, and it anticipates
future patches raising the priority of Low (which is currently equal to
Background on Windows & Linux).
The main point is to allow users to restore the old behavior, which e.g.
allows efficiency cores to remain idle.

I did consider making this a config setting, this is a more complicated change:
 - needs to touch queue priorities as well as thread priorities
 - we don't know the priority until evaluating the config inside the task
 - users would want the ability to prioritize background indexing tasks relative
   to each other without necessarily affecting thread priority, so using one
   option for both may be confusing
I don't really have a use case, so I prefer the simpler thing.

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/index/Background.cpp
clang-tools-extra/clangd/index/Background.h
clang-tools-extra/clangd/tool/ClangdMain.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdServer.h 
b/clang-tools-extra/clangd/ClangdServer.h
index d1854c5f903ef..6d999722805ed 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -110,6 +110,7 @@ class ClangdServer {
 /// If true, ClangdServer automatically indexes files in the current 
project
 /// on background threads. The index is stored in the project root.
 bool BackgroundIndex = false;
+llvm::ThreadPriority BackgroundIndexPriority = llvm::ThreadPriority::Low;
 
 /// If set, use this index to augment code completion results.
 SymbolIndex *StaticIndex = nullptr;

diff  --git a/clang-tools-extra/clangd/index/Background.cpp 
b/clang-tools-extra/clangd/index/Background.cpp
index 71860a3a4bcd2..b5349468eb24f 100644
--- a/clang-tools-extra/clangd/index/Background.cpp
+++ b/clang-tools-extra/clangd/index/Background.cpp
@@ -92,6 +92,7 @@ BackgroundIndex::BackgroundIndex(
 const ThreadsafeFS &TFS, const GlobalCompilationDatabase &CDB,
 BackgroundIndexStorage::Factory IndexStorageFactory, Options Opts)
 : SwapIndex(std::make_unique()), TFS(TFS), CDB(CDB),
+  IndexingPriority(Opts.IndexingPriority),
   ContextProvider(std::move(Opts.ContextProvider)),
   IndexedSymbols(IndexContents::All),
   Rebuilder(this, &IndexedSymbols, Opts.ThreadPoolSize),
@@ -165,6 +166,7 @@ BackgroundQueue::Task 
BackgroundIndex::indexFileTask(std::string Path) {
   elog("Indexing {0} failed: {1}", Path, std::move(Error));
   });
   T.QueuePri = IndexFile;
+  T.ThreadPri = IndexingPriority;
   T.Tag = std::move(Tag);
   T.Key = Key;
   return T;

diff  --git a/clang-tools-extra/clangd/index/Background.h 
b/clang-tools-extra/clangd/index/Background.h
index 97f9095ed2c23..688c09814cd25 100644
--- a/clang-tools-extra/clangd/index/Background.h
+++ b/clang-tools-extra/clangd/index/Background.h
@@ -136,6 +136,8 @@ class BackgroundIndex : public SwapIndex {
 // Arbitrary value to ensure some concurrency in tests.
 // In production an explicit value is specified.
 size_t ThreadPoolSize = 4;
+// Thread priority when indexing files.
+llvm::ThreadPriority IndexingPriority = llvm::ThreadPriority::Low;
 // Callback that provides notifications as indexing makes progress.
 std::function OnProgress = nullptr;
 // Function called to obtain the Context to use while indexing the 
specified
@@ -194,6 +196,7 @@ class BackgroundIndex : public SwapIndex {
   // configuration
   const ThreadsafeFS &TFS;
   const GlobalCompilationDatabase &CDB;
+  llvm::ThreadPriority IndexingPriority;
   std::function ContextProvider;
 
   llvm::Error index(tooling::CompileCommand);

diff  --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp 
b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index d86b5dab51e99..76cfaf7a35dfe 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -169,6 +169,21 @@ opt EnableBackgroundIndex{
 init(true),
 };
 
+opt BackgroundIndexPriority{
+"background-index-priority",
+cat(Features),
+desc("Thread priority for building the background index. "
+ "The effect of this flag is OS-specific."),
+values(clEnumValN(llvm::ThreadPriority::Background, "background",
+  "Minimum priority, runs on idle CPUs. "
+  "May leave 'performance' cores unused."),
+   clEnumValN(llvm::ThreadPriority::Low, "l

[PATCH] D125673: [clangd] Add command-line flag to set background indexing thread priority.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG127a1492d729: [clangd] Add command-line flag to set 
background indexing thread priority. (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125673

Files:
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/Background.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -169,6 +169,21 @@
 init(true),
 };
 
+opt BackgroundIndexPriority{
+"background-index-priority",
+cat(Features),
+desc("Thread priority for building the background index. "
+ "The effect of this flag is OS-specific."),
+values(clEnumValN(llvm::ThreadPriority::Background, "background",
+  "Minimum priority, runs on idle CPUs. "
+  "May leave 'performance' cores unused."),
+   clEnumValN(llvm::ThreadPriority::Low, "low",
+  "Reduced priority compared to interactive work."),
+   clEnumValN(llvm::ThreadPriority::Default, "normal",
+  "Same priority as other clangd work.")),
+init(llvm::ThreadPriority::Low),
+};
+
 opt EnableClangTidy{
 "clang-tidy",
 cat(Features),
@@ -876,6 +891,7 @@
   }
 #endif
   Opts.BackgroundIndex = EnableBackgroundIndex;
+  Opts.BackgroundIndexPriority = BackgroundIndexPriority;
   Opts.ReferencesLimit = ReferencesLimit;
   auto PAI = createProjectAwareIndex(loadExternalIndex, Sync);
   if (StaticIdx) {
Index: clang-tools-extra/clangd/index/Background.h
===
--- clang-tools-extra/clangd/index/Background.h
+++ clang-tools-extra/clangd/index/Background.h
@@ -136,6 +136,8 @@
 // Arbitrary value to ensure some concurrency in tests.
 // In production an explicit value is specified.
 size_t ThreadPoolSize = 4;
+// Thread priority when indexing files.
+llvm::ThreadPriority IndexingPriority = llvm::ThreadPriority::Low;
 // Callback that provides notifications as indexing makes progress.
 std::function OnProgress = nullptr;
 // Function called to obtain the Context to use while indexing the 
specified
@@ -194,6 +196,7 @@
   // configuration
   const ThreadsafeFS &TFS;
   const GlobalCompilationDatabase &CDB;
+  llvm::ThreadPriority IndexingPriority;
   std::function ContextProvider;
 
   llvm::Error index(tooling::CompileCommand);
Index: clang-tools-extra/clangd/index/Background.cpp
===
--- clang-tools-extra/clangd/index/Background.cpp
+++ clang-tools-extra/clangd/index/Background.cpp
@@ -92,6 +92,7 @@
 const ThreadsafeFS &TFS, const GlobalCompilationDatabase &CDB,
 BackgroundIndexStorage::Factory IndexStorageFactory, Options Opts)
 : SwapIndex(std::make_unique()), TFS(TFS), CDB(CDB),
+  IndexingPriority(Opts.IndexingPriority),
   ContextProvider(std::move(Opts.ContextProvider)),
   IndexedSymbols(IndexContents::All),
   Rebuilder(this, &IndexedSymbols, Opts.ThreadPoolSize),
@@ -165,6 +166,7 @@
   elog("Indexing {0} failed: {1}", Path, std::move(Error));
   });
   T.QueuePri = IndexFile;
+  T.ThreadPri = IndexingPriority;
   T.Tag = std::move(Tag);
   T.Key = Key;
   return T;
Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -110,6 +110,7 @@
 /// If true, ClangdServer automatically indexes files in the current 
project
 /// on background threads. The index is stored in the project root.
 bool BackgroundIndex = false;
+llvm::ThreadPriority BackgroundIndexPriority = llvm::ThreadPriority::Low;
 
 /// If set, use this index to augment code completion results.
 SymbolIndex *StaticIndex = nullptr;


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -169,6 +169,21 @@
 init(true),
 };
 
+opt BackgroundIndexPriority{
+"background-index-priority",
+cat(Features),
+desc("Thread priority for building the background index. "
+ "The effect of this flag is OS-specific."),
+values(clEnumValN(llvm::ThreadPriority::Background, "background",
+  "Minimum priority, runs on idle CPUs. "
+  "May leave 'performance' cores unused."),
+   clEnumValN(llvm::ThreadPriority::Low, "low",
+  "Reduced priority compared 

[PATCH] D124382: [Clang] Recognize target address space in superset calculation

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

My concerns have been addressed. I'll defer the final LGTM to @Anastasia.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124382

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

[PATCH] D125814: Fix strict prototype diagnostic wording for definitions

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

This confuses me.

Looking at behavior with default flags:

We won't emit a -Wdeprecated-non-prototype warning for `int foo();`, until we 
subsequently find `int foo(int arg) { return 5; }`. Since we definitely have 
the context of what's going on at that point, in order to have determined that 
there's a conflict, what prevents doing the "right thing": emitting only 1 
warning (at the definition site) and 1 note (at the declaration site)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125814

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

[PATCH] D115232: [clangd] Indexing of standard library

[Sam McCall via Phabricator via cfe-commits]

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

fix HasSubstr matcher type issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115232

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/index/FileIndex.h
  clang-tools-extra/clangd/index/StdLib.cpp
  clang-tools-extra/clangd/index/StdLib.h
  clang-tools-extra/clangd/index/SymbolOrigin.cpp
  clang-tools-extra/clangd/index/SymbolOrigin.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/StdLibTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -1123,7 +1123,8 @@
   public:
 BlockPreambleThread(llvm::StringRef BlockVersion, Notification &N)
 : BlockVersion(BlockVersion), N(N) {}
-void onPreambleAST(PathRef Path, llvm::StringRef Version, ASTContext &Ctx,
+void onPreambleAST(PathRef Path, llvm::StringRef Version,
+   const CompilerInvocation &, ASTContext &Ctx,
Preprocessor &, const CanonicalIncludes &) override {
   if (Version == BlockVersion)
 N.wait();
Index: clang-tools-extra/clangd/unittests/StdLibTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/StdLibTests.cpp
@@ -0,0 +1,162 @@
+//===-- StdLibTests.cpp -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Annotations.h"
+#include "ClangdServer.h"
+#include "CodeComplete.h"
+#include "Compiler.h"
+#include "Config.h"
+#include "SyncAPI.h"
+#include "TestFS.h"
+#include "index/StdLib.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceManager.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace testing;
+
+namespace clang {
+namespace clangd {
+namespace {
+
+// Check the generated header sources contains usual standard library headers.
+TEST(StdLibTests, getStdlibUmbrellaHeader) {
+  LangOptions LO;
+  LO.CPlusPlus = true;
+
+  auto CXX = getStdlibUmbrellaHeader(LO).str();
+  EXPECT_THAT(CXX, HasSubstr("#include "));
+  EXPECT_THAT(CXX, HasSubstr("#include "));
+  EXPECT_THAT(CXX, Not(HasSubstr("#include ")));
+
+  LO.CPlusPlus = false;
+  auto C = getStdlibUmbrellaHeader(LO).str();
+  EXPECT_THAT(C, Not(HasSubstr("#include ")));
+  EXPECT_THAT(C, Not(HasSubstr("#include ")));
+  EXPECT_THAT(C, HasSubstr("#include "));
+}
+
+MATCHER_P(Named, Name, "") { return arg.Name == Name; }
+
+// Build an index, and check if it contains the right symbols.
+TEST(StdLibTests, indexStandardLibrary) {
+  MockFS FS;
+  FS.Files["std/foo.h"] = R"cpp(
+  #include 
+  #if __cplusplus >= 201703L
+int foo17();
+  #elif __cplusplus >= 201402L
+int foo14();
+  #else
+bool foo98();
+  #endif
+  )cpp";
+  FS.Files["nonstd/platform_stuff.h"] = "int magic = 42;";
+
+  ParseInputs OriginalInputs;
+  OriginalInputs.TFS = &FS;
+  OriginalInputs.CompileCommand.Filename = testPath("main.cc");
+  OriginalInputs.CompileCommand.CommandLine = {"clang++", testPath("main.cc"),
+   "-isystemstd/",
+   "-isystemnonstd/", "-std=c++14"};
+  OriginalInputs.CompileCommand.Directory = testRoot();
+  IgnoreDiagnostics Diags;
+  auto CI = buildCompilerInvocation(OriginalInputs, Diags);
+  ASSERT_TRUE(CI);
+
+  StdLibLocation Loc;
+  Loc.Paths.push_back(testPath("std/"));
+
+  auto Symbols =
+  indexStandardLibrary("#include ", std::move(CI), Loc, FS);
+  EXPECT_THAT(Symbols, ElementsAre(Named("foo14")));
+}
+
+TEST(StdLibTests, StdLibSet) {
+  StdLibSet Set;
+  MockFS FS;
+  FS.Files["std/_"] = "";
+  FS.Files["libc/_"] = "";
+
+  auto Add = [&](const LangOptions &LO,
+ std::vector SearchPath) {
+SourceManagerForFile SM("scratch", "");
+SM.get().getFileManager().setVirtualFileSystem(FS.view(llvm::None));
+HeaderSearch HS(/*HSOpts=*/nullptr, SM.get(), SM.get().getDiagnostics(), LO,
+  

[PATCH] D125259: [C11] Diagnose unreachable generic selection associations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a subscriber: rsmith.
aaron.ballman added a comment.

In D125259#3519947 , @aaron.ballman 
wrote:

> Oh wow, good catch! I'll correct this.

Oof, the plot thickens... the diagnostic is correct for some types, but is 
incorrect for others: https://godbolt.org/z/fahzx53W6. I also discovered a 
parsing issue in C++ where we get confused by elaborated type specifiers: 
https://godbolt.org/z/1ejxqd9ss.

I'm questioning the benefit to supporting `_Generic` in C++ and starting to 
wonder if perhaps we should pull this extension back. C++ already has a rich 
type system that can accomplish the same thing and we're not following the C 
semantics so there's not really an argument to be made for the extension 
letting people write code in both languages. @rsmith, do you have thoughts 
about that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125259

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

[PATCH] D125026: [clang-tidy][NFC] Reimplement SimplifyBooleanExpr with RecursiveASTVisitors

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D125026#3519831 , 
@LegalizeAdulthood wrote:

> Just for my own edification, how did you know/suspect that a pure visitor 
> would be faster than matchers?

Mainly cause the fact we are creating 2 matcher expressions that differ by a 
bool value for each pattern, a visitor can easily handle both cases in one go.
There's also overhead with ASTMatchers that for simple cases sometimes may not 
be worth it.
It's not all good news though, there's a cost associated with running another 
ASTTraversal, however as we already have a visitor in this check we don't pay 
for that cost again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125026

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

[PATCH] D113107: Support of expression granularity for _Float16.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

Finally got around to work on this.
@rjmccall, @andrew.w.kaylor  at your convenience please let me know your 
thoughts. There are a couple of things I'm still not sure about! Thanks.


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

https://reviews.llvm.org/D113107

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

[PATCH] D113107: Support of expression granularity for _Float16.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 430115.
zahiraam marked an inline comment as done.
Herald added subscribers: llvm-commits, hiraditya.
Herald added projects: LLVM, All.

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

https://reviews.llvm.org/D113107

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/X86/Float16-arithmetic.c
  clang/test/CodeGen/X86/Float16-complex.c
  clang/test/CodeGen/X86/avx512fp16-complex.c
  clang/test/Sema/Float16.c
  clang/test/Sema/conversion-target-dep.c
  clang/test/SemaCXX/Float16.cpp
  llvm/lib/CodeGen/TargetLoweringBase.cpp

Index: llvm/lib/CodeGen/TargetLoweringBase.cpp
===
--- llvm/lib/CodeGen/TargetLoweringBase.cpp
+++ llvm/lib/CodeGen/TargetLoweringBase.cpp
@@ -188,8 +188,8 @@
   }
 }
   } else {
-setLibcallName(RTLIB::FPEXT_F16_F32, "__gnu_h2f_ieee");
-setLibcallName(RTLIB::FPROUND_F32_F16, "__gnu_f2h_ieee");
+  setLibcallName(RTLIB::FPEXT_F16_F32, "__gnu_h2f_ieee");
+  setLibcallName(RTLIB::FPROUND_F32_F16, "__gnu_f2h_ieee");
   }
 
   if (TT.isGNUEnvironment() || TT.isOSFuchsia() ||
Index: clang/test/SemaCXX/Float16.cpp
===
--- clang/test/SemaCXX/Float16.cpp
+++ clang/test/SemaCXX/Float16.cpp
@@ -1,18 +1,10 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc %s
-// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s
 
-#ifdef HAVE
 // expected-no-diagnostics
-#endif // HAVE
 
-#ifndef HAVE
-// expected-error@+2{{_Float16 is not supported on this target}}
-#endif // !HAVE
 _Float16 f;
 
-#ifndef HAVE
-// expected-error@+2{{invalid suffix 'F16' on floating constant}}
-#endif // !HAVE
 const auto g = 1.1F16;
Index: clang/test/Sema/conversion-target-dep.c
===
--- clang/test/Sema/conversion-target-dep.c
+++ clang/test/Sema/conversion-target-dep.c
@@ -6,7 +6,7 @@
 
 long double ld;
 double d;
-_Float16 f16; // x86-error {{_Float16 is not supported on this target}}
+_Float16 f16;
 
 int main(void) {
   ld = d; // x86-warning {{implicit conversion increases floating-point precision: 'double' to 'long double'}}
Index: clang/test/Sema/Float16.c
===
--- clang/test/Sema/Float16.c
+++ clang/test/Sema/Float16.c
@@ -1,18 +1,12 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc %s
-// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc -target-feature +avx512fp16 %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc -target-feature +avx512fp16 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s
 
-#ifndef HAVE
-// expected-error@+2{{_Float16 is not supported on this target}}
-#endif // HAVE
-_Float16 f;
-
-#ifdef HAVE
 _Complex _Float16 a;
 void builtin_complex(void) {
   _Float16 a = 0;
   (void)__builtin_complex(a, a); // expected-error {{'_Complex _Float16' is invalid}}
 }
-#endif
+
Index: clang/test/CodeGen/X86/Float16-complex.c
===
--- clang/test/CodeGen/X86/Float16-complex.c
+++ clang/test/CodeGen/X86/Float16-complex.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -target-feature +avx512fp16 -o - | FileCheck %s --check-prefix=X86
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -o - | FileCheck %s --check-prefixes=X86,X86-FP16
 
 _Float16 _Complex add_half_rr(_Float16 a, _Float16 b) {
   // X86-LABEL: @add_half_rr(
@@ -119,8 +120,8 @@
 }
 _Float16 _Complex div_half_rc(_Float16 a, _Float16 _Complex b) {
   // X86-LABEL: @div_half_rc(
-  // X86-NOT: fdiv
-  // X86: call {{.*}} @__divhc3(
+  // X86-FP16: fdiv
+  // X86-FP16: fdiv
   // X86: ret
   return a / b;
 }
Index: clang/test/CodeGen/X86/Float16-arithmetic.c
===
--- /dev/null
++

[PATCH] D125259: [C11] Diagnose unreachable generic selection associations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D125259#3519822 , @aeubanks wrote:

>   $ cat /tmp/a.cc
>   typedef struct Test {
>   } Test;
>   
>   void f() {
> const Test constVal;
> _Generic(constVal, const Test : 0);
>   }
>   $ ./build/rel/bin/clang -fsyntax-only  -x c++ /tmp/a.cc -Wall
>   /tmp/a.cc:6:28: warning: due to lvalue conversion of the controlling 
> expression, association of type 'const Test' will never be selected because 
> it is qualified [-Wunreachable-code-generic-assoc]
> _Generic(constVal, const Test : 0);
>  ^
>   /tmp/a.cc:6:35: warning: expression result unused [-Wunused-value]
> _Generic(constVal, const Test : 0);
> ^
>   2 warnings generated.
>   $ ./build/rel/bin/clang -fsyntax-only  -x c /tmp/a.cc -Wall
>   /tmp/a.cc:6:28: warning: due to lvalue conversion of the controlling 
> expression, association of type 'const Test' (aka 'const struct Test') will 
> never be selected because it is qualified [-Wunreachable-code-generic-assoc]
> _Generic(constVal, const Test : 0);
>  ^
>   /tmp/a.cc:6:12: error: controlling expression type 'Test' (aka 'struct 
> Test') not compatible with any generic association type
> _Generic(constVal, const Test : 0);
>  ^~~~
>   1 warning and 1 error generated.
>
> the C++ case looks wrong, it's warning that `const Test` can't be selected 
> then selects it

Oh wow, good catch! I'll correct this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125259

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

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D122895#3512314 , @aaron.ballman 
wrote:

> In D122895#3511682 , @aaron.ballman 
> wrote:
>
>> In D122895#3511632 , @jyknight 
>> wrote:
>>
>>> The warnings for this case aren't great:
>>>
>>>   int foo();
>>>   
>>>   int
>>>   foo(int arg) {
>>> return 5;
>>>   }
>>
>> Yeah, that's not ideal, I'm looking into it to see if I can improve that 
>> scenario or not.
>
> There's not much to be done about the strict prototype warning for the 
> declaration; the issue is that we need to give the strict prototypes warning 
> when forming the function *type* for the declaration, which is long before we 
> have any idea there's a subsequent definition (also, because this is for 
> forming the type, we don't know what declarations, if any, may be related, so 
> there's no real way to improve the fix-it behavior). However, I think the 
> second warning is just bad wording -- we know we have a function definition 
> with a prototype for this particular diagnostic. However, there's the 
> redeclaration case to consider at the same time. So here's the full test and 
> the best behavior that I can come up with so far:
>
>   void foo();
>   void foo(int arg);
>   
>   void bar();
>   void bar(int arg) {}
>
> With `-Wstrict-prototypes` enabled:
>
>   F:\source\llvm-project>llvm\out\build\x64-Debug\bin\clang.exe -fsyntax-only 
> -Wstrict-prototypes "C:\Users\aballman\OneDrive - Intel 
> Corporation\Desktop\test.c"
>   C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:1:9: warning: 
> a function declaration without a prototype
> is deprecated in all versions of C [-Wstrict-prototypes]
>   void foo();
>   ^
>void
>   C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:2:6: warning: 
> this function declaration with a prototype
> will change behavior in C2x because of a previous declaration with no 
> prototype [-Wdeprecated-non-prototype]
>   void foo(int arg);
>^
>   C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:4:9: warning: 
> a function declaration without a prototype
> is deprecated in all versions of C [-Wstrict-prototypes]
>   void bar();
>   ^
>void
>   C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:5:6: warning: 
> this function definition with a prototype
> will change behavior in C2x because of a previous declaration with no 
> prototype [-Wdeprecated-non-prototype]
>   void bar(int arg) {}
>^
>   4 warnings generated.
>
> With `-Wstrict-prototypes` disabled:
>
>   F:\source\llvm-project>llvm\out\build\x64-Debug\bin\clang.exe -fsyntax-only 
> "C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c"
>   C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:1:6: warning: 
> a function declaration without a prototype
> is deprecated in all versions of C and is not supported in C2x 
> [-Wdeprecated-non-prototype]
>   void foo();
>^
>void
>   C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:2:6: warning: 
> this function declaration with a prototype
> will change behavior in C2x because of a previous declaration with no 
> prototype [-Wdeprecated-non-prototype]
>   void foo(int arg);
>^
>   C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:4:6: warning: 
> a function declaration without a prototype
> is deprecated in all versions of C and is not supported in C2x 
> [-Wdeprecated-non-prototype]
>   void bar();
>^
>void
>   C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:5:6: warning: 
> this function definition with a prototype
> will change behavior in C2x because of a previous declaration with no 
> prototype [-Wdeprecated-non-prototype]
>   void bar(int arg) {}
>^
>   4 warnings generated.
>
> It's not ideal, but it's about the most workable solution I can come up with 
> that doesn't regress far more important cases. It'd be nice if we had a note 
> instead of leaning on the previous warning like we're doing, but I still 
> claim this is defensible given how rare the situation is that you'd declare 
> without a prototype and later redeclare (perhaps through a definition) with a 
> non-`void` prototype. Note (it's easy to miss with the wall of text), the 
> difference between the two runs is that when strict prototypes are enabled, 
> we issue the strict prototype warning on the first declaration and when 
> strict prototypes are disabled, we issue the "is not supported in C2x" 
> diagnostic on the first declaration -- but in either case the intention is to 
> alert the user to which previous declaration has no prototype for the 
> subsequent declaration/definition that caused the issue.
>
> `this function declaration|definition with a prototype will change behavior 
> in C2x be

[PATCH] D125814: Fix strict prototype diagnostic wording for definitions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: jyknight, dexonsmith.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a project: clang.

Post-commit feedback on https://reviews.llvm.org/D122895 pointed out that the 
diagnostic wording for some code was using "declaration" in a confusing way, 
such as:

  int foo(); // warning: a function declaration without a prototype is 
deprecated in all versions of C and is not supported in C2x
  
  int foo(int arg) { // warning: a function declaration without a prototype is 
deprecated in all versions of C and is not supported in C2x
return 5;
  }

This patch addresses the confusion in two ways. First, it updates the warning 
text to be specific about a function *with* a prototype. But that pointed out 
that we had a note which was perhaps not carrying its weight in terms of 
conveying useful information to the user. In all of the situations we'd issue 
the note, we already issue a warning with basically the same text as the note, 
so the note comes across as overly chatty. The note has been removed and we're 
leaning a bit more heavily on the prior diagnostic pointing out the previous 
problematic declaration in that case.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125814

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/warn-deprecated-non-prototype.c
  clang/test/Sema/warn-strict-prototypes.c

Index: clang/test/Sema/warn-strict-prototypes.c
===
--- clang/test/Sema/warn-strict-prototypes.c
+++ clang/test/Sema/warn-strict-prototypes.c
@@ -55,9 +55,8 @@
 }
 
 // Function declaration with no types
-void foo10(); // expected-warning {{a function declaration without a prototype is deprecated in all versions of C}} \
- expected-note {{a function declaration without a prototype is not supported in C2x}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"void"
+void foo10(); // expected-warning {{a function declaration without a prototype is deprecated in all versions of C}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:12-[[@LINE-1]]:12}:"void"
 // K&R function definition with incomplete param list declared
 void foo10(p, p2) void *p; {} // expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}} \
  expected-warning {{parameter 'p2' was not declared, defaults to 'int'; ISO C99 and later do not support implicit int}}
Index: clang/test/Sema/warn-deprecated-non-prototype.c
===
--- clang/test/Sema/warn-deprecated-non-prototype.c
+++ clang/test/Sema/warn-deprecated-non-prototype.c
@@ -24,8 +24,7 @@
 
 // On by default warnings
 void func(); // expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}} \
-strict-warning {{a function declaration without a prototype is deprecated in all versions of C}} \
-strict-note {{a function declaration without a prototype is not supported in C2x}}
+strict-warning {{a function declaration without a prototype is deprecated in all versions of C}}
 void func(a, b) int a, b; {} // both-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}}
 
 void one_more(a, b) int a, b; {} // both-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}}
@@ -40,18 +39,16 @@
 }
 
 void order1();// expected-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}} \
- strict-warning {{a function declaration without a prototype is deprecated in all versions of C}} \
- strict-note {{a function declaration without a prototype is not supported in C2x}}
-void order1(int i);   // both-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}}
+ strict-warning {{a function declaration without a prototype is deprecated in all versions of C}}
+void order1(int i);   // both-warning {{this function declaration with a prototype will change behavior in C2x because of a previous declaration with no prototype}}
 
 void order2(int i);
 void order2();// both-warning {{a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x}} \
  strict-warning {{a function declaration without a prototype is deprecated in all versions of C}}
 
 void order3();// expected-warning {{a function declarat

[PATCH] D125749: [analyzer][NFC] Introduce SVal::isa

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

steakhal added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp:44
   // Uninitialized value used for the mutex?
-  if (V.getAs()) {
+  if (V.isa()) {
 if (ExplodedNode *N = C.generateErrorNode()) {

xazax.hun wrote:
> Is there a difference between `isa` vs `IsUndef`? I think we 
> should standardize on one of them (or potentially remove `IsUndef` and the 
> like?
Yea, they are the same. I'm not sure which to pick though.
I still like the terser member function style. I'll replace this occurrence and 
similars.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125749

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

[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`

[H. Vetinari via Phabricator via cfe-commits]

h-vetinari added a comment.

Very happy to see this finally happening! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125788

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

[PATCH] D125709: [analyzer][Casting] Support isa, cast, dyn_cast of SVals

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

xazax.hun added a comment.

While not having tests might be OK, but I'd prefer to introduce at least a 
couple uses of the new facilities so existing tests cover them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125709

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

[PATCH] D125026: [clang-tidy][NFC] Reimplement SimplifyBooleanExpr with RecursiveASTVisitors

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added a comment.

Just for my own edification, how did you know/suspect that a pure visitor would 
be faster than matchers?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125026

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

[PATCH] D125749: [analyzer][NFC] Introduce SVal::isa

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

xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp:44
   // Uninitialized value used for the mutex?
-  if (V.getAs()) {
+  if (V.isa()) {
 if (ExplodedNode *N = C.generateErrorNode()) {

Is there a difference between `isa` vs `IsUndef`? I think we 
should standardize on one of them (or potentially remove `IsUndef` and the like?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125749

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

[PATCH] D124918: [clang-tidy] Add a new check for non-trivial unused variables.

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst:172-173
+List of functions that are considered not to read some of their arguments
+unless their return value is read. Arguments are identified by a bitmask, where
+the i-th LSB being set indicates that the i-th argument is unused.
+

veluca93 wrote:
> LegalizeAdulthood wrote:
> > veluca93 wrote:
> > > LegalizeAdulthood wrote:
> > > > This isn't user-friendly at all.  Why not an array of argument indices 
> > > > starting at zero?
> > > Done. I'm using an empty array to indicate "everything", which is perhaps 
> > > a bit weird.
> > I think `[-1]` might be a better sentinel.  What do you do for functions 
> > that take zero arguments?
> It doesn't matter, right? For the purpose of tracking whether variables are 
> used, 0-argument free/static functions are meaningless anyway.
OK, yes, I was thinking member functions not free/static functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124918

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

[PATCH] D125259: [C11] Diagnose unreachable generic selection associations

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

  $ cat /tmp/a.cc
  typedef struct Test {
  } Test;
  
  void f() {
const Test constVal;
_Generic(constVal, const Test : 0);
  }
  $ ./build/rel/bin/clang -fsyntax-only  -x c++ /tmp/a.cc -Wall
  /tmp/a.cc:6:28: warning: due to lvalue conversion of the controlling 
expression, association of type 'const Test' will never be selected because it 
is qualified [-Wunreachable-code-generic-assoc]
_Generic(constVal, const Test : 0);
 ^
  /tmp/a.cc:6:35: warning: expression result unused [-Wunused-value]
_Generic(constVal, const Test : 0);
^
  2 warnings generated.
  $ ./build/rel/bin/clang -fsyntax-only  -x c /tmp/a.cc -Wall
  /tmp/a.cc:6:28: warning: due to lvalue conversion of the controlling 
expression, association of type 'const Test' (aka 'const struct Test') will 
never be selected because it is qualified [-Wunreachable-code-generic-assoc]
_Generic(constVal, const Test : 0);
 ^
  /tmp/a.cc:6:12: error: controlling expression type 'Test' (aka 'struct Test') 
not compatible with any generic association type
_Generic(constVal, const Test : 0);
 ^~~~
  1 warning and 1 error generated.

the C++ case looks wrong, it's warning that `const Test` can't be selected then 
selects it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125259

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

[PATCH] D125770: [clang-tidy] modernize-deprecated-headers should ignore system headers

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp:37
 
-#include  // FIXME: We should have no warning into system 
headers.
-// CHECK-MESSAGES-WARN-INTO-HEADERS: mysystemlib.h:1:10: warning: inclusion of 
deprecated C++ header 'assert.h'; consider using 'cassert' instead 
[modernize-deprecated-headers]
+#include  // no-warning: Don't warn into system headers.
 

Where is this file?  I can't find it in my tree.

```
D:\legalize\llvm\llvm-project\clang-tools-extra
> dir/s/b mysystemlib.h
File Not Found
```

Does this patch depend on some other unsubmitted patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125770

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

[PATCH] D125769: [clang-tidy] Introduce the WarnIntoHeaders option to modernize-deprecated-headers

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h:63
   std::vector IncludesToBeProcessed;
+  bool WarnIntoHeaders;
 };

1) How is this different from the clang-tidy option that specifies whether or 
not fixits are applied to header files?

  As an owner of a code base, I would know which header files are included from 
C source files and I would set my header-file regex (honestly, not a fan of a 
regex for that option; I'd prefer white/black lists, but that's another 
discussion) to exclude header files that are known to be included in C source 
files.

2) Assuming that the header-file regex is somehow insufficient to cover this 
scenario, I like the functionality but the name of this option feels "off".  
(Naming things is hard.)  Elsewhere we have options that say `HeaderFile` not 
`Headers` and `Into` just doesn't sound like the way normal conversation would 
state the situation.  Something like `CheckHeaderFile` would be more consistent 
with existing options.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-headers-extern-c.cpp:2-4
 // Copy the 'mylib.h' to a directory under the build directory. This is
 // required, since the relative order of the emitted diagnostics depends on the
 // absolute file paths which is sorted by clang-tidy prior emitting.

IMO, all of this hackery is simply ducking the issue, which is that 
`check_clang_tidy.py` doesn't have proper support for validating fixits applied 
to header files.

See https://reviews.llvm.org/D17482


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125769

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

[PATCH] D125802: Fix std::has_unique_object_representations for _BitInt types with padding bits

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

This likely also needs a Release Note.




Comment at: clang/lib/AST/ASTContext.cpp:2697
   int64_t FieldSizeInBits =
   Context.toBits(Context.getTypeSizeInChars(Field->getType()));
   if (Field->isBitField()) {

MitalAshok wrote:
> erichkeane wrote:
> > This answer ends up being wrong in the case of _BitInt, consider 
> > _BitInt(7).  Its field-size would be 7, yet this would result in 8.  I'm 
> > not sure of the fallout of this though.
> The field size should be 8 for _BitInt(7), since it takes 8 bits in the 
> struct (including the padding bit). It is filtered out later with 
> "hasUniqueObjectRepresentations" on line 2709 if it wasn't in a bit field
I see, thanks for the explanation.



Comment at: clang/test/SemaCXX/has_unique_object_reps_bitint.cpp:7
+static_assert(__has_unique_object_representations(_BitInt(sizeof(int) * 8u)));
+static_assert(sizeof(_BitInt(24)) != 4 || 
!__has_unique_object_representations(_BitInt(24)));
+

MitalAshok wrote:
> erichkeane wrote:
> > Whats going on here?  I don't particularly get the condition.
> sizeof(_BitInt(24)) == sizeof(int32_t) (4) since we align to the next larger 
> builtin integer type. But if that weren't the case for some reason (i.e., 
> that changes in the future, I don't know if _BitInt is ABI stable), this 
> wouldn't start failing for unrelated reasons
I see!  Can you clarify that with a comment here?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125802

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

[PATCH] D125802: Fix std::has_unique_object_representations for _BitInt types with padding bits

[Mital Ashok via Phabricator via cfe-commits]

MitalAshok added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:2697
   int64_t FieldSizeInBits =
   Context.toBits(Context.getTypeSizeInChars(Field->getType()));
   if (Field->isBitField()) {

erichkeane wrote:
> This answer ends up being wrong in the case of _BitInt, consider _BitInt(7).  
> Its field-size would be 7, yet this would result in 8.  I'm not sure of the 
> fallout of this though.
The field size should be 8 for _BitInt(7), since it takes 8 bits in the struct 
(including the padding bit). It is filtered out later with 
"hasUniqueObjectRepresentations" on line 2709 if it wasn't in a bit field



Comment at: clang/test/SemaCXX/has_unique_object_reps_bitint.cpp:7
+static_assert(__has_unique_object_representations(_BitInt(sizeof(int) * 8u)));
+static_assert(sizeof(_BitInt(24)) != 4 || 
!__has_unique_object_representations(_BitInt(24)));
+

erichkeane wrote:
> Whats going on here?  I don't particularly get the condition.
sizeof(_BitInt(24)) == sizeof(int32_t) (4) since we align to the next larger 
builtin integer type. But if that weren't the case for some reason (i.e., that 
changes in the future, I don't know if _BitInt is ABI stable), this wouldn't 
start failing for unrelated reasons


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125802

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

[PATCH] D125513: [clang-cl] Add /Zc:wchar_t- option

[Pengxuan Zheng 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 rG366e57de23ed: [clang-cl] Add /Zc:wchar_t- option (authored 
by pzheng).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125513

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/cl-options.c
  clang/test/Driver/cl-zc.cpp


Index: clang/test/Driver/cl-zc.cpp
===
--- clang/test/Driver/cl-zc.cpp
+++ clang/test/Driver/cl-zc.cpp
@@ -47,7 +47,7 @@
 // RUN: %clang_cl /c -### /Zc:wchar_t -- %s 2>&1 | FileCheck 
-check-prefix=WCHAR_T-ON %s
 // WCHAR_T-ON-NOT: argument unused during compilation
 // RUN: %clang_cl /c -### /Zc:wchar_t- -- %s 2>&1 | FileCheck 
-check-prefix=WCHAR_T-OFF %s
-// WCHAR_T-OFF: argument unused during compilation
+// WCHAR_T-OFF: "-fno-wchar"
 
 // RUN: %clang_cl /c -### /Zc:auto -- %s 2>&1 | FileCheck 
-check-prefix=AUTO-ON %s
 // AUTO-ON-NOT: argument unused during compilation
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -404,7 +404,6 @@
 // RUN:/Zc:inline \
 // RUN:/Zc:rvalueCast \
 // RUN:/Zc:ternary \
-// RUN:/Zc:wchar_t \
 // RUN:/ZH:MD5 \
 // RUN:/ZH:SHA1 \
 // RUN:/ZH:SHA_256 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7589,6 +7589,11 @@
   CmdArgs.push_back("-fno-dllexport-inlines");
  }
 
+ if (Args.hasFlag(options::OPT__SLASH_Zc_wchar_t_,
+  options::OPT__SLASH_Zc_wchar_t, false)) {
+   CmdArgs.push_back("-fno-wchar");
+ }
+
   Arg *MostGeneralArg = Args.getLastArg(options::OPT__SLASH_vmg);
   Arg *BestCaseArg = Args.getLastArg(options::OPT__SLASH_vmb);
   if (MostGeneralArg && BestCaseArg)
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6487,6 +6487,10 @@
 def _SLASH_Zc_twoPhase_ : CLFlag<"Zc:twoPhase-">,
   HelpText<"Disable two-phase name lookup in templates (default)">,
   Alias;
+def _SLASH_Zc_wchar_t : CLFlag<"Zc:wchar_t">,
+  HelpText<"Enable C++ builtin type wchar_t (default)">;
+def _SLASH_Zc_wchar_t_ : CLFlag<"Zc:wchar_t-">,
+  HelpText<"Disable C++ builtin type wchar_t">;
 def _SLASH_Z7 : CLFlag<"Z7">,
   HelpText<"Enable CodeView debug information in object files">;
 def _SLASH_Zi : CLFlag<"Zi">, Alias<_SLASH_Z7>,
@@ -6674,7 +6678,6 @@
 def _SLASH_Zc_inline : CLIgnoredFlag<"Zc:inline">;
 def _SLASH_Zc_rvalueCast : CLIgnoredFlag<"Zc:rvalueCast">;
 def _SLASH_Zc_ternary : CLIgnoredFlag<"Zc:ternary">;
-def _SLASH_Zc_wchar_t : CLIgnoredFlag<"Zc:wchar_t">;
 def _SLASH_ZH_MD5 : CLIgnoredFlag<"ZH:MD5">;
 def _SLASH_ZH_SHA1 : CLIgnoredFlag<"ZH:SHA1">;
 def _SLASH_ZH_SHA_256 : CLIgnoredFlag<"ZH:SHA_256">;


Index: clang/test/Driver/cl-zc.cpp
===
--- clang/test/Driver/cl-zc.cpp
+++ clang/test/Driver/cl-zc.cpp
@@ -47,7 +47,7 @@
 // RUN: %clang_cl /c -### /Zc:wchar_t -- %s 2>&1 | FileCheck -check-prefix=WCHAR_T-ON %s
 // WCHAR_T-ON-NOT: argument unused during compilation
 // RUN: %clang_cl /c -### /Zc:wchar_t- -- %s 2>&1 | FileCheck -check-prefix=WCHAR_T-OFF %s
-// WCHAR_T-OFF: argument unused during compilation
+// WCHAR_T-OFF: "-fno-wchar"
 
 // RUN: %clang_cl /c -### /Zc:auto -- %s 2>&1 | FileCheck -check-prefix=AUTO-ON %s
 // AUTO-ON-NOT: argument unused during compilation
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -404,7 +404,6 @@
 // RUN:/Zc:inline \
 // RUN:/Zc:rvalueCast \
 // RUN:/Zc:ternary \
-// RUN:/Zc:wchar_t \
 // RUN:/ZH:MD5 \
 // RUN:/ZH:SHA1 \
 // RUN:/ZH:SHA_256 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7589,6 +7589,11 @@
   CmdArgs.push_back("-fno-dllexport-inlines");
  }
 
+ if (Args.hasFlag(options::OPT__SLASH_Zc_wchar_t_,
+  options::OPT__SLASH_Zc_wchar_t, false)) {
+   CmdArgs.push_back("-fno-wchar");
+ }
+
   Arg *MostGeneralArg = Args.getLastArg(options::OPT__SLASH_vmg);
   Arg *BestCaseArg = Args.getLastArg(options::OPT__SLASH_vmb);
   if (MostGeneralArg && BestCaseArg)
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clan

[clang] 366e57d - [clang-cl] Add /Zc:wchar_t- option

[Pengxuan Zheng via cfe-commits]

Author: Pengxuan Zheng
Date: 2022-05-17T09:40:30-07:00
New Revision: 366e57de23ed20ac95201e1623dfffed215e98f8

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

LOG: [clang-cl] Add /Zc:wchar_t- option

Map /Zc:wchar_t- to the cc1 flag -fno-wchar which is already supported.

Reviewed By: thakis

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

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/cl-options.c
clang/test/Driver/cl-zc.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 4be0b574fa444..e18b71ba064df 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6487,6 +6487,10 @@ def _SLASH_Zc_twoPhase : CLFlag<"Zc:twoPhase">,
 def _SLASH_Zc_twoPhase_ : CLFlag<"Zc:twoPhase-">,
   HelpText<"Disable two-phase name lookup in templates (default)">,
   Alias;
+def _SLASH_Zc_wchar_t : CLFlag<"Zc:wchar_t">,
+  HelpText<"Enable C++ builtin type wchar_t (default)">;
+def _SLASH_Zc_wchar_t_ : CLFlag<"Zc:wchar_t-">,
+  HelpText<"Disable C++ builtin type wchar_t">;
 def _SLASH_Z7 : CLFlag<"Z7">,
   HelpText<"Enable CodeView debug information in object files">;
 def _SLASH_Zi : CLFlag<"Zi">, Alias<_SLASH_Z7>,
@@ -6674,7 +6678,6 @@ def _SLASH_Zc_forScope : CLIgnoredFlag<"Zc:forScope">;
 def _SLASH_Zc_inline : CLIgnoredFlag<"Zc:inline">;
 def _SLASH_Zc_rvalueCast : CLIgnoredFlag<"Zc:rvalueCast">;
 def _SLASH_Zc_ternary : CLIgnoredFlag<"Zc:ternary">;
-def _SLASH_Zc_wchar_t : CLIgnoredFlag<"Zc:wchar_t">;
 def _SLASH_ZH_MD5 : CLIgnoredFlag<"ZH:MD5">;
 def _SLASH_ZH_SHA1 : CLIgnoredFlag<"ZH:SHA1">;
 def _SLASH_ZH_SHA_256 : CLIgnoredFlag<"ZH:SHA_256">;

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 75515469421b0..5dce5b81fa8a6 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -7589,6 +7589,11 @@ void Clang::AddClangCLArgs(const ArgList &Args, 
types::ID InputType,
   CmdArgs.push_back("-fno-dllexport-inlines");
  }
 
+ if (Args.hasFlag(options::OPT__SLASH_Zc_wchar_t_,
+  options::OPT__SLASH_Zc_wchar_t, false)) {
+   CmdArgs.push_back("-fno-wchar");
+ }
+
   Arg *MostGeneralArg = Args.getLastArg(options::OPT__SLASH_vmg);
   Arg *BestCaseArg = Args.getLastArg(options::OPT__SLASH_vmb);
   if (MostGeneralArg && BestCaseArg)

diff  --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c
index 244b4430dd0dc..4eae94b1bff20 100644
--- a/clang/test/Driver/cl-options.c
+++ b/clang/test/Driver/cl-options.c
@@ -404,7 +404,6 @@
 // RUN:/Zc:inline \
 // RUN:/Zc:rvalueCast \
 // RUN:/Zc:ternary \
-// RUN:/Zc:wchar_t \
 // RUN:/ZH:MD5 \
 // RUN:/ZH:SHA1 \
 // RUN:/ZH:SHA_256 \

diff  --git a/clang/test/Driver/cl-zc.cpp b/clang/test/Driver/cl-zc.cpp
index cf7734875e1f1..53d28dddefa73 100644
--- a/clang/test/Driver/cl-zc.cpp
+++ b/clang/test/Driver/cl-zc.cpp
@@ -47,7 +47,7 @@
 // RUN: %clang_cl /c -### /Zc:wchar_t -- %s 2>&1 | FileCheck 
-check-prefix=WCHAR_T-ON %s
 // WCHAR_T-ON-NOT: argument unused during compilation
 // RUN: %clang_cl /c -### /Zc:wchar_t- -- %s 2>&1 | FileCheck 
-check-prefix=WCHAR_T-OFF %s
-// WCHAR_T-OFF: argument unused during compilation
+// WCHAR_T-OFF: "-fno-wchar"
 
 // RUN: %clang_cl /c -### /Zc:auto -- %s 2>&1 | FileCheck 
-check-prefix=AUTO-ON %s
 // AUTO-ON-NOT: argument unused during compilation



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

[PATCH] D124918: [clang-tidy] Add a new check for non-trivial unused variables.

[Luca Versari via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst:172-173
+List of functions that are considered not to read some of their arguments
+unless their return value is read. Arguments are identified by a bitmask, where
+the i-th LSB being set indicates that the i-th argument is unused.
+

LegalizeAdulthood wrote:
> veluca93 wrote:
> > LegalizeAdulthood wrote:
> > > This isn't user-friendly at all.  Why not an array of argument indices 
> > > starting at zero?
> > Done. I'm using an empty array to indicate "everything", which is perhaps a 
> > bit weird.
> I think `[-1]` might be a better sentinel.  What do you do for functions that 
> take zero arguments?
It doesn't matter, right? For the purpose of tracking whether variables are 
used, 0-argument free/static functions are meaningless anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124918

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

[PATCH] D125513: [clang-cl] Add /Zc:wchar_t- option

[Pengxuan Zheng via Phabricator via cfe-commits]

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

In D125513#3519200 , @thakis wrote:

> Looks good to me after addressing Hans's comment.
>
> Do you have commit access?

Thanks for asking, @thakis. I do have comment access.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125513

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

[PATCH] D125209: [clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:90
+  // to the `ASTContext`.
+  Finder->addMatcher(ast_matchers::translationUnitDecl().bind("TU"), this);
+}

In modernize-macro-to-enum I had to similarly discard macros defined inside a 
top-level decl.  I did `decl(hasParent(translationUnitDecl()))` to match the 
top-level decls.  Would that simplify your check as well?



Comment at: 
clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:123
 
-IncludeModernizePPCallbacks::IncludeModernizePPCallbacks(ClangTidyCheck &Check,
- LangOptions LangOpts)
+detail::IncludeModernizePPCallbacks::IncludeModernizePPCallbacks(
+DeprecatedHeadersCheck &Check, LangOptions LangOpts)

IMO it's poor style to define entities that are declared within a namespace 
outside the namespace within which they were declared.

Is there some reason this isn't defined with the rest of the methods on the 
callback class?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125209

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

[PATCH] D125513: [clang-cl] Add /Zc:wchar_t- option

[Pengxuan Zheng via Phabricator via cfe-commits]

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



Comment at: clang/test/Driver/cl-zc.cpp:50
 // RUN: %clang_cl /c -### /Zc:wchar_t- -- %s 2>&1 | FileCheck 
-check-prefix=WCHAR_T-OFF %s
-// WCHAR_T-OFF: argument unused during compilation
+// WCHAR_T-OFF: "-fno-wchar"
 

hans wrote:
> Should probably remove `/Zc:wchar_t` from the list of ignored options in 
> clang/test/Driver/cl-options.c too.
Good catch. I have updated clang/test/Driver/cl-options.c in the latest change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125513

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

[PATCH] D125513: [clang-cl] Add /Zc:wchar_t- option

[Pengxuan Zheng via Phabricator via cfe-commits]

pzheng updated this revision to Diff 430096.
pzheng marked an inline comment as done.
pzheng added a comment.

Address @hans's comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125513

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/cl-options.c
  clang/test/Driver/cl-zc.cpp


Index: clang/test/Driver/cl-zc.cpp
===
--- clang/test/Driver/cl-zc.cpp
+++ clang/test/Driver/cl-zc.cpp
@@ -47,7 +47,7 @@
 // RUN: %clang_cl /c -### /Zc:wchar_t -- %s 2>&1 | FileCheck 
-check-prefix=WCHAR_T-ON %s
 // WCHAR_T-ON-NOT: argument unused during compilation
 // RUN: %clang_cl /c -### /Zc:wchar_t- -- %s 2>&1 | FileCheck 
-check-prefix=WCHAR_T-OFF %s
-// WCHAR_T-OFF: argument unused during compilation
+// WCHAR_T-OFF: "-fno-wchar"
 
 // RUN: %clang_cl /c -### /Zc:auto -- %s 2>&1 | FileCheck 
-check-prefix=AUTO-ON %s
 // AUTO-ON-NOT: argument unused during compilation
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -404,7 +404,6 @@
 // RUN:/Zc:inline \
 // RUN:/Zc:rvalueCast \
 // RUN:/Zc:ternary \
-// RUN:/Zc:wchar_t \
 // RUN:/ZH:MD5 \
 // RUN:/ZH:SHA1 \
 // RUN:/ZH:SHA_256 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7589,6 +7589,11 @@
   CmdArgs.push_back("-fno-dllexport-inlines");
  }
 
+ if (Args.hasFlag(options::OPT__SLASH_Zc_wchar_t_,
+  options::OPT__SLASH_Zc_wchar_t, false)) {
+   CmdArgs.push_back("-fno-wchar");
+ }
+
   Arg *MostGeneralArg = Args.getLastArg(options::OPT__SLASH_vmg);
   Arg *BestCaseArg = Args.getLastArg(options::OPT__SLASH_vmb);
   if (MostGeneralArg && BestCaseArg)
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6487,6 +6487,10 @@
 def _SLASH_Zc_twoPhase_ : CLFlag<"Zc:twoPhase-">,
   HelpText<"Disable two-phase name lookup in templates (default)">,
   Alias;
+def _SLASH_Zc_wchar_t : CLFlag<"Zc:wchar_t">,
+  HelpText<"Enable C++ builtin type wchar_t (default)">;
+def _SLASH_Zc_wchar_t_ : CLFlag<"Zc:wchar_t-">,
+  HelpText<"Disable C++ builtin type wchar_t">;
 def _SLASH_Z7 : CLFlag<"Z7">,
   HelpText<"Enable CodeView debug information in object files">;
 def _SLASH_Zi : CLFlag<"Zi">, Alias<_SLASH_Z7>,
@@ -6674,7 +6678,6 @@
 def _SLASH_Zc_inline : CLIgnoredFlag<"Zc:inline">;
 def _SLASH_Zc_rvalueCast : CLIgnoredFlag<"Zc:rvalueCast">;
 def _SLASH_Zc_ternary : CLIgnoredFlag<"Zc:ternary">;
-def _SLASH_Zc_wchar_t : CLIgnoredFlag<"Zc:wchar_t">;
 def _SLASH_ZH_MD5 : CLIgnoredFlag<"ZH:MD5">;
 def _SLASH_ZH_SHA1 : CLIgnoredFlag<"ZH:SHA1">;
 def _SLASH_ZH_SHA_256 : CLIgnoredFlag<"ZH:SHA_256">;


Index: clang/test/Driver/cl-zc.cpp
===
--- clang/test/Driver/cl-zc.cpp
+++ clang/test/Driver/cl-zc.cpp
@@ -47,7 +47,7 @@
 // RUN: %clang_cl /c -### /Zc:wchar_t -- %s 2>&1 | FileCheck -check-prefix=WCHAR_T-ON %s
 // WCHAR_T-ON-NOT: argument unused during compilation
 // RUN: %clang_cl /c -### /Zc:wchar_t- -- %s 2>&1 | FileCheck -check-prefix=WCHAR_T-OFF %s
-// WCHAR_T-OFF: argument unused during compilation
+// WCHAR_T-OFF: "-fno-wchar"
 
 // RUN: %clang_cl /c -### /Zc:auto -- %s 2>&1 | FileCheck -check-prefix=AUTO-ON %s
 // AUTO-ON-NOT: argument unused during compilation
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -404,7 +404,6 @@
 // RUN:/Zc:inline \
 // RUN:/Zc:rvalueCast \
 // RUN:/Zc:ternary \
-// RUN:/Zc:wchar_t \
 // RUN:/ZH:MD5 \
 // RUN:/ZH:SHA1 \
 // RUN:/ZH:SHA_256 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7589,6 +7589,11 @@
   CmdArgs.push_back("-fno-dllexport-inlines");
  }
 
+ if (Args.hasFlag(options::OPT__SLASH_Zc_wchar_t_,
+  options::OPT__SLASH_Zc_wchar_t, false)) {
+   CmdArgs.push_back("-fno-wchar");
+ }
+
   Arg *MostGeneralArg = Args.getLastArg(options::OPT__SLASH_vmg);
   Arg *BestCaseArg = Args.getLastArg(options::OPT__SLASH_vmb);
   if (MostGeneralArg && BestCaseArg)
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6487,6 +6487,10 @@
 def _SLASH_Zc_twoPhase_ : C

[PATCH] D124918: [clang-tidy] Add a new check for non-trivial unused variables.

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst:172-173
+List of functions that are considered not to read some of their arguments
+unless their return value is read. Arguments are identified by a bitmask, where
+the i-th LSB being set indicates that the i-th argument is unused.
+

veluca93 wrote:
> LegalizeAdulthood wrote:
> > This isn't user-friendly at all.  Why not an array of argument indices 
> > starting at zero?
> Done. I'm using an empty array to indicate "everything", which is perhaps a 
> bit weird.
I think `[-1]` might be a better sentinel.  What do you do for functions that 
take zero arguments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124918

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

[PATCH] D125802: Fix std::has_unique_object_representations for _BitInt types with padding bits

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I think I agree with the justification here, though am a touch confused in the 
test.  I'm also a touch concerned that we have getSubobjectSizeInBits returning 
a 'rounded up to power of 2' bit  happening here.  The bitfield case returns 
non-powers-of-two, but the _BitInt case does not.




Comment at: clang/lib/AST/ASTContext.cpp:2697
   int64_t FieldSizeInBits =
   Context.toBits(Context.getTypeSizeInChars(Field->getType()));
   if (Field->isBitField()) {

This answer ends up being wrong in the case of _BitInt, consider _BitInt(7).  
Its field-size would be 7, yet this would result in 8.  I'm not sure of the 
fallout of this though.



Comment at: clang/test/SemaCXX/has_unique_object_reps_bitint.cpp:7
+static_assert(__has_unique_object_representations(_BitInt(sizeof(int) * 8u)));
+static_assert(sizeof(_BitInt(24)) != 4 || 
!__has_unique_object_representations(_BitInt(24)));
+

Whats going on here?  I don't particularly get the condition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125802

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

[PATCH] D125802: Fix std::has_unique_object_representations for _BitInt types with padding bits

[Mital Ashok via Phabricator via cfe-commits]

MitalAshok created this revision.
Herald added a project: All.
MitalAshok retitled this revision from "Fix 
std::has_unique_object_representations for _BitInt types with padding" to "Fix 
std::has_unique_object_representations for _BitInt types with padding bits".
MitalAshok edited the summary of this revision.
MitalAshok added reviewers: erichkeane, aaron.ballman.
MitalAshok added a reviewer: rsmith.
MitalAshok edited projects, added clang; removed All.
Herald added a project: All.
MitalAshok published this revision for review.
Herald added a subscriber: cfe-commits.

"std::has_unique_object_representations<_BitInt(N)>" was always true, even if 
the type has padding bits (since the trait assumes all integer types have no 
padding bits). The standard has an explicit note that this should not hold for 
types with padding bits.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125802

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/SemaCXX/has_unique_object_reps_bitint.cpp

Index: clang/test/SemaCXX/has_unique_object_reps_bitint.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/has_unique_object_reps_bitint.cpp
@@ -0,0 +1,76 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify -std=c++17 -Wno-bitfield-width %s
+//  expected-no-diagnostics
+
+static_assert(__has_unique_object_representations(_BitInt(8)));
+static_assert(__has_unique_object_representations(unsigned _BitInt(8)));
+static_assert(__has_unique_object_representations(_BitInt(sizeof(int) * 8u)));
+static_assert(sizeof(_BitInt(24)) != 4 || !__has_unique_object_representations(_BitInt(24)));
+
+static_assert(!__has_unique_object_representations(_BitInt(7)));
+static_assert(!__has_unique_object_representations(unsigned _BitInt(7)));
+static_assert(!__has_unique_object_representations(_BitInt(2)));
+static_assert(!__has_unique_object_representations(unsigned _BitInt(1)));
+
+template 
+constexpr bool check() {
+  if constexpr (N <= __BITINT_MAXWIDTH__) {
+static_assert(__has_unique_object_representations(_BitInt(N)) == (sizeof(_BitInt(N)) * 8u == N));
+static_assert(__has_unique_object_representations(unsigned _BitInt(N)) == (sizeof(unsigned _BitInt(N)) * 8u == N));
+  }
+  return true;
+}
+
+template 
+constexpr bool do_check = (check() && ...);
+
+static_assert(do_check<2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18>);
+static_assert(do_check<15, 16, 17, 23, 24, 25, 31, 32, 33>);
+static_assert(do_check<39, 40, 41, 47, 48, 49>);
+static_assert(do_check<127, 128, 129, 255, 256, 257, 383, 384, 385>);
+
+template 
+struct in_struct {
+  _BitInt(N) x;
+  static constexpr bool check() {
+return __has_unique_object_representations(in_struct) == __has_unique_object_representations(_BitInt(N));
+  }
+};
+
+static_assert(in_struct<8>::check());
+static_assert(in_struct<7>::check());
+
+struct bit_fields_1 {
+  _BitInt(7) x : 7;
+  unsigned _BitInt(1) y : 1;
+};
+
+static_assert(__has_unique_object_representations(bit_fields_1) == (sizeof(bit_fields_1) == 1));
+
+struct bit_fields_2 {
+  _BitInt(8) x : 7;
+};
+
+static_assert(!__has_unique_object_representations(bit_fields_2));
+
+struct bit_fields_3 {
+  _BitInt(15) x : 8;
+};
+
+static_assert(__has_unique_object_representations(bit_fields_3) == (sizeof(bit_fields_3) == 1));
+
+#if __BITINT_MAXWIDTH__ >= 129
+struct bit_fields_4 {
+  _BitInt(129) x : 128;
+};
+
+static_assert(__has_unique_object_representations(bit_fields_4) == (sizeof(bit_fields_4) == 128 / 8));
+#endif
+
+struct bit_fields_5 {
+  _BitInt(2) x : 8;
+};
+
+static_assert(!__has_unique_object_representations(bit_fields_5));
+
+static_assert(__has_unique_object_representations(_BitInt(7)&) == __has_unique_object_representations(_BitInt(8)&));
+static_assert(__has_unique_object_representations(_BitInt(8)&) == __has_unique_object_representations(int&));
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2684,7 +2684,12 @@
 if (!RD->isUnion())
   return structHasUniqueObjectRepresentations(Context, RD);
   }
-  if (!Field->getType()->isReferenceType() &&
+
+  // A _BitInt type may not be unique if it has padding bits
+  // but if it is a bitfield the padding bits are not used
+  bool IsBitIntType =
+  !Field->getType()->isReferenceType() && Field->getType()->isBitIntType();
+  if (!Field->getType()->isReferenceType() && !IsBitIntType &&
   !Context.hasUniqueObjectRepresentations(Field->getType()))
 return llvm::None;
 
@@ -2692,9 +2697,17 @@
   Context.toBits(Context.getTypeSizeInChars(Field->getType()));
   if (Field->isBitField()) {
 int64_t BitfieldSize = Field->getBitWidthValue(Context);
-if (BitfieldSize > FieldSizeInBits)
+if (IsBitIntType) {
+  if ((unsigned)BitfieldSize >
+  cast(Field->getType())->getNumBits())
+return llvm::None;
+} els

[PATCH] D123534: [dwarf] Emit a DIGlobalVariable for constant strings.

[Mitch Phillips via Phabricator via cfe-commits]

hctim added a comment.

Why, on Fuchsia, on WIndows, the DIs are in the order `q -> p -> r` is beyond 
me (instead of the file order and my order `p -> q -> r`. Looking into it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123534

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

[PATCH] D125705: [OpenMP] Don't build the offloading driver without a source input

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D125705#3519605 , @jhuber6 wrote:

> In D125705#3519570 , @yaxunl wrote:
>
>> HIP toolchain allows clang driver to compile bundled bitcode or assembly for 
>> mixed host/device compilation or device-only multi-GPU compilation.
>>
>> e.g.
>>
>> clang --offload-arch=gfx906 --offload-arch=gfx908 a.bc b.s
>>
>> Can you add a test to make sure this does not break HIP toolchain? Thanks.
>
> This only changes the new driver, which doesn't support HIP right now anyway. 
> This should be captured by some existing tests but I could try to dig them up

If it is only for the new driver that is fine. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125705

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

[PATCH] D125749: [analyzer][NFC] Introduce SVal::isa

[Aman LaChapelle via Phabricator via cfe-commits]

bzcheeseman added a comment.

Not a qualified reviewer for anything other than the `llvm::isa` usage and that 
line looks good to me!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125749

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

[PATCH] D125771: [clang-tidy] Add a useful note about -std=c++11-or-later

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added a comment.

I thought there wasn't any support for validating fixits applied to header 
files?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125771

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

[PATCH] D125705: [OpenMP] Don't build the offloading driver without a source input

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D125705#3519570 , @yaxunl wrote:

> HIP toolchain allows clang driver to compile bundled bitcode or assembly for 
> mixed host/device compilation or device-only multi-GPU compilation.
>
> e.g.
>
> clang --offload-arch=gfx906 --offload-arch=gfx908 a.bc b.s
>
> Can you add a test to make sure this does not break HIP toolchain? Thanks.

This only changes the new driver, which doesn't support HIP right now anyway. 
This should be captured by some existing tests but I could try to dig them up


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125705

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

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

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

xazax.hun added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:352
+  reports are lost compared to single-TU analysis, the lost reports are highly
+  likely to be false positives.
 

I wonder if you also want to mention that this still finds most of the CTU 
findings in most cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123773

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

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

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

xazax.hun accepted this revision.
xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:446
+}
+const bool BState = State->contains(D);
+if (!BState) { // This is the first time we see this foreign function.

martong wrote:
> xazax.hun wrote:
> > martong wrote:
> > > xazax.hun wrote:
> > > > xazax.hun wrote:
> > > > > martong wrote:
> > > > > > xazax.hun wrote:
> > > > > > > So if we see the same foreign function called in multiple 
> > > > > > > contexts, we will only queue one of the contexts for the CTU. Is 
> > > > > > > this the intended design? 
> > > > > > > 
> > > > > > > So if I see:
> > > > > > > ```
> > > > > > > foreign(true);
> > > > > > > foreign(false);
> > > > > > > ```
> > > > > > > 
> > > > > > > The new CTU will only evaluate `foreign(true)` but not 
> > > > > > > `foreign(false)`. 
> > > > > > This is intentional.
> > > > > > ```
> > > > > > foreign(true);
> > > > > > foreign(false);
> > > > > > ```
> > > > > > The new CTU will evaluate the following paths in the exploded graph:
> > > > > > ```
> > > > > > foreign(true); // conservative evaluated
> > > > > > foreign(false); // conservative evaluated
> > > > > > foreign(true); // inlined
> > > > > > foreign(false); // inlined
> > > > > > ```
> > > > > > The point is to keep bifurcation to a minimum and avoid the 
> > > > > > exponential blow up.
> > > > > > So, we will never have a path like this:
> > > > > > ```
> > > > > > foreign(true); // conservative evaluated
> > > > > > foreign(false); // inlined
> > > > > > ```
> > > > > > 
> > > > > > Actually, this is the same strategy that we use during the dynamic 
> > > > > > dispatch of C++ virtual calls. See `DynamicDispatchBifurcationMap`.
> > > > > > 
> > > > > > The conservative evaluation happens in the first phase, the 
> > > > > > inlining in the second phase (assuming the phase1 inlining option 
> > > > > > is set to none).
> > > > > > The new CTU will evaluate the following paths in the exploded graph:
> > > > > > ```
> > > > > > foreign(true); // conservative evaluated
> > > > > > foreign(false); // conservative evaluated
> > > > > > foreign(true); // inlined
> > > > > > foreign(false); // inlined
> > > > > > ```
> > > > > 
> > > > > When we encounter `foreign(true)`, we would add the decl to 
> > > > > `CTUDispatchBifurcationSet`. So the second time we see a call to the 
> > > > > function `foreign(false);`, we will just do the conservative 
> > > > > evaluation and will not add the call to the CTU worklist. So how will 
> > > > > `foreign(false);` be inlined in the second pass? Do I miss something? 
> > > > > 
> > > > Oh, I think I now understand. Do you expect `foreign(false);` to be 
> > > > inlined after we return from `foreign(true);` in the second pass? Sorry 
> > > > for the confusion, in that case it looks good to me.
> > > > Oh, I think I now understand. Do you expect `foreign(false);` to be 
> > > > inlined after we return from `foreign(true);` in the second pass?
> > > 
> > > Yes, that's right.
> > Actually, in this case I wonder whether we really need a set of 
> > declarations. Wouldn't a boolean be sufficient for each path?
> > 
> > So in case of:
> > ```
> > foreign1(true);
> > foreign2(false);
> > ```
> > 
> > Why would we want to add `foreign2` to the CTU worklist? More specifically, 
> > why is it important whether a foreign call is actually the same declaration 
> > that we saw earlier on the path? Isn't being foreign the only important 
> > information in this case?
> Good point. 
> 
> By using the set of declarations we might have a path where `foreign1` is 
> conservatively evaluated and then `foreign2` is inlined. I was having a hard 
> time to find any use-cases where this would be useful, but could not find 
> one. 
> 
> On the other hand, by using a `bool`, as you suggest, no such superfluous 
> path would exist. Plus, the dependent child patch (D123784) is becoming 
> unnecessary because the CTUWorklist will have at most one element during the 
> first phase.
> 
> Besides, I've made measurements comparing the `bool` and the `set` 
> implementation. The results are quite similar. Both have lost the same amount 
> of bugreports compared to the single-tu analysis and the average length of 
> the bugpaths are also similar. {F23090628}
Great news! :) Looks like this was a nice simplification, thanks for looking 
into it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123773

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

[PATCH] D125789: FIX the assembly format of the x86 backend to make both clang and gcc happy

[Changwei Zou via Phabricator via cfe-commits]

sheisc added a comment.

In D125789#3519538 , @craig.topper 
wrote:

> Is it gcc that needs a fix or binutils?

Yes. Exactly speaking, it is the GNU assembler in binutils.
https://www.gnu.org/software/binutils/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125789

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

[PATCH] D125705: [OpenMP] Don't build the offloading driver without a source input

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

HIP toolchain allows clang driver to compile bundled bitcode or assembly for 
mixed host/device compilation or device-only multi-GPU compilation.

e.g.

clang --offload-arch=gfx906 --offload-arch=gfx908 a.bc b.s

Can you add a test to make sure this does not break HIP toolchain? Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125705

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

[PATCH] D125789: FIX the assembly format of the x86 backend to make both clang and gcc happy

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

Is it gcc that needs a fix or binutils?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125789

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

[PATCH] D125557: [APInt] Remove all uses of zextOrSelf, sextOrSelf and truncOrSelf

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

Coming into this late, but I'd have preferred to see this separated into at 
least two pieces.  One for each "non-obvious" adjustment, and one final one 
which just did the replace on the renaming sites.  This differs from feedback 
from other reviewers above, so don't feel bound by this in any way.  Just 
expressing the general preference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125557

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

[PATCH] D125789: FIX the assembly format of the x86 backend to make both clang and gcc happy

[Changwei Zou via Phabricator via cfe-commits]

sheisc added a comment.

In D125789#3519493 , @pengfei wrote:

> In D125789#3519433 , @sheisc wrote:
>
>> In D125789#3519411 , @pengfei 
>> wrote:
>>
>>> I think another way is to report the issue to GCC. From the perspective of 
>>> the user, GCC should support both `{%k1} {z}` and `{%k1}{z}`. Then we don't 
>>> need the clange on LLVM.
>>
>> Yes. It is a good idea. 
>> However, it appears that there is no such white space in the instructions as 
>> described in Intel's manuals.
>> So I don't know which one should be the correct format.
>> Anyway, not a big issue.
>> I found this problem when using the fuzzer (i.e. AFL) to build Firefox.
>
> Yeah. This is an interesting question. I didn't notice the difference between 
> LLVM and GCC. I think either way changing here or GCC is OK :)

Hi Pengfei,

Thanks a lot.  The guys in the GCC community can make our life easier by 
supporting the white space.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125789

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

[PATCH] D125789: FIX the assembly format of the x86 backend to make both clang and gcc happy

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

In D125789#3519433 , @sheisc wrote:

> In D125789#3519411 , @pengfei wrote:
>
>> I think another way is to report the issue to GCC. From the perspective of 
>> the user, GCC should support both `{%k1} {z}` and `{%k1}{z}`. Then we don't 
>> need the clange on LLVM.
>
> Yes. It is a good idea. 
> However, it appears that there is no such white space in the instructions as 
> described in Intel's manuals.
> So I don't know which one should be the correct format.
> Anyway, not a big issue.
> I found this problem when using the fuzzer (i.e. AFL) to build Firefox.

Yeah. This is an interesting question. I didn't notice the difference between 
LLVM and GCC. I think either way changing here or GCC is OK :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125789

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

[PATCH] D125728: [WebAssembly] Update supported features in -mcpu=generic

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

Oh, and per recent updates 
 to the LLVM 
Developer policy I think it would be worth updating the Clang ReleaseNotes.rst 
to mention this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125728

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

[PATCH] D125728: [WebAssembly] Update supported features in -mcpu=generic

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

Based on the discussion we had, I think this makes sense. It's a bit 
repetitive, but could you please add a test to clang/test/Driver that checks 
the list of enabled features for generic (and for completeness, probably 
bleeding-edge as well). Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125728

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

[PATCH] D125789: FIX the assembly format of the x86 backend to make both clang and gcc happy

[Changwei Zou via Phabricator via cfe-commits]

sheisc added a comment.

In D125789#3519411 , @pengfei wrote:

> I think another way is to report the issue to GCC. From the perspective of 
> the user, GCC should support both `{%k1} {z}` and `{%k1}{z}`. Then we don't 
> need the clange on LLVM.

Yes. It is a good idea. 
However, it appears that there is no such white space in the instructions as 
described in Intel's manuals.
So I don't know which one should be the correct format.
Anyway, not a big issue.
I found this problem when using the fuzzer (i.e. AFL) to build Firefox.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125789

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

[PATCH] D121733: Clean pathnames in FileManager.

[Paul Pluzhnikov via Phabricator via cfe-commits]

ppluzhnikov added inline comments.



Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:546
+  {"include/wordexp.h", ""},
+  {"include/x86intrin.h", ""},
+  {"include/xlocale.h", ""},

ilya-biryukov wrote:
> ppluzhnikov wrote:
> > ilya-biryukov wrote:
> > > Why do we change the order of elements here?
> > > Please revert, this increases the diff and is not relevant to the actual 
> > > change.
> > Note that the elements are inserted into a map
> > (after commit b3a991df3cd6a; used to be a vector before).
> > 
> > Also note that there are duplicates, e.g.
> > 
> > {"bits/typesizes.h", ""},
> > {"bits/typesizes.h", ""},
> > 
> > which can't work as intended / is already broken.
> > 
> > Sorting helps to find these duplicates.
> > 
> This refactoring makes sense, but please split this into a separate change.
https://reviews.llvm.org/D125742


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121733

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

[PATCH] D125789: FIX the assembly format of the x86 backend to make both clang and gcc happy

[Changwei Zou via Phabricator via cfe-commits]

sheisc abandoned this revision.
sheisc added a comment.

In D125789#3519361 , @pengfei wrote:

> I guess a lot of lines of tests need to update
>
>   $ grep -rn " {z}" llvm/test/CodeGen/X86/ | wc -l
>   7797

Thanks.
Yes. It seems to be a big revision, caused by only one white space :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125789

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

[PATCH] D125789: FIX the assembly format of the x86 backend to make both clang and gcc happy

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

I think another way is to report the issue to GCC. From the perspective of the 
user, GCC should support both `{%k1} {z}` and `{%k1}{z}`. Then we don't need 
the clange on LLVM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125789

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

[PATCH] D125788: [flang][driver] Rename `flang-new` as `flang`

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

Sounds good to me. https://reviews.llvm.org/D125796 for the bot side, let me 
know if/when the first half of the change goes in.

I'll leave it to others to approve the change overall. I do think being able to 
do `./bin/flang` and get the right flang will save a lot of confusion but I'm 
not a user of either of them myself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125788

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

[clang-tools-extra] 76ddbb1 - Revert "[clangd] Indexing of standard library"

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-05-17T17:17:27+02:00
New Revision: 76ddbb1ca747366417be64fdf79218df099a5973

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

LOG: Revert "[clangd] Indexing of standard library"

This reverts commit ecaa4d9662c9a6ac013ac40a8ad72a2c75e3fd3b.

Added: 


Modified: 
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/TUScheduler.h
clang-tools-extra/clangd/index/FileIndex.cpp
clang-tools-extra/clangd/index/FileIndex.h
clang-tools-extra/clangd/index/SymbolOrigin.cpp
clang-tools-extra/clangd/index/SymbolOrigin.h
clang-tools-extra/clangd/unittests/CMakeLists.txt
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Removed: 
clang-tools-extra/clangd/index/StdLib.cpp
clang-tools-extra/clangd/index/StdLib.h
clang-tools-extra/clangd/unittests/StdLibTests.cpp



diff  --git a/clang-tools-extra/clangd/CMakeLists.txt 
b/clang-tools-extra/clangd/CMakeLists.txt
index 7cfbd6f95750e..9c37cfe7b7001 100644
--- a/clang-tools-extra/clangd/CMakeLists.txt
+++ b/clang-tools-extra/clangd/CMakeLists.txt
@@ -119,7 +119,6 @@ add_clang_library(clangDaemon
   index/Ref.cpp
   index/Relation.cpp
   index/Serialization.cpp
-  index/StdLib.cpp
   index/Symbol.cpp
   index/SymbolCollector.cpp
   index/SymbolID.cpp

diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 69a0f63972aae..80d7d5c5ece19 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -26,7 +26,6 @@
 #include "index/CanonicalIncludes.h"
 #include "index/FileIndex.h"
 #include "index/Merge.h"
-#include "index/StdLib.h"
 #include "refactor/Rename.h"
 #include "refactor/Tweak.h"
 #include "support/Cancellation.h"
@@ -60,39 +59,16 @@ namespace {
 // Update the FileIndex with new ASTs and plumb the diagnostics responses.
 struct UpdateIndexCallbacks : public ParsingCallbacks {
   UpdateIndexCallbacks(FileIndex *FIndex,
-   ClangdServer::Callbacks *ServerCallbacks,
-   const ThreadsafeFS &TFS, AsyncTaskRunner *Tasks)
-  : FIndex(FIndex), ServerCallbacks(ServerCallbacks), TFS(TFS),
-Tasks(Tasks) {}
+   ClangdServer::Callbacks *ServerCallbacks)
+  : FIndex(FIndex), ServerCallbacks(ServerCallbacks) {}
 
-  void onPreambleAST(PathRef Path, llvm::StringRef Version,
- const CompilerInvocation &CI, ASTContext &Ctx,
+  void onPreambleAST(PathRef Path, llvm::StringRef Version, ASTContext &Ctx,
  Preprocessor &PP,
  const CanonicalIncludes &CanonIncludes) override {
-// If this preamble uses a standard library we haven't seen yet, index it.
-if (FIndex)
-  if (auto Loc = Stdlib.add(*CI.getLangOpts(), PP.getHeaderSearchInfo()))
-indexStdlib(CI, std::move(*Loc));
-
 if (FIndex)
   FIndex->updatePreamble(Path, Version, Ctx, PP, CanonIncludes);
   }
 
-  void indexStdlib(const CompilerInvocation &CI, StdLibLocation Loc) {
-auto Task = [this, LO(*CI.getLangOpts()), Loc(std::move(Loc)),
- CI(std::make_unique(CI))]() mutable {
-  IndexFileIn IF;
-  IF.Symbols = indexStandardLibrary(std::move(CI), Loc, TFS);
-  if (Stdlib.isBest(LO))
-FIndex->updatePreamble(std::move(IF));
-};
-if (Tasks)
-  // This doesn't have a semaphore to enforce -j, but it's rare.
-  Tasks->runAsync("IndexStdlib", std::move(Task));
-else
-  Task();
-  }
-
   void onMainAST(PathRef Path, ParsedAST &AST, PublishFn Publish) override {
 if (FIndex)
   FIndex->updateMain(Path, AST);
@@ -127,9 +103,6 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
 private:
   FileIndex *FIndex;
   ClangdServer::Callbacks *ServerCallbacks;
-  const ThreadsafeFS &TFS;
-  StdLibSet Stdlib;
-  AsyncTaskRunner *Tasks;
 };
 
 class DraftStoreFS : public ThreadsafeFS {
@@ -181,15 +154,12 @@ ClangdServer::ClangdServer(const 
GlobalCompilationDatabase &CDB,
   Transient(Opts.ImplicitCancellation ? TUScheduler::InvalidateOnUpdate
   : TUScheduler::NoInvalidation),
   DirtyFS(std::make_unique(TFS, DraftMgr)) {
-  if (Opts.AsyncThreadsCount != 0)
-IndexTasks.emplace();
   // Pass a callback into `WorkScheduler` to extract symbols from a newly
   // parsed file and rebuild the file index synchronously each time an AST
   // is parsed.
-  WorkScheduler.