[clang] c9e403d - [clang][Interp] Fix zero-init of float and pointer arrays

2023-04-25 Thread Timm Bäder via cfe-commits

Author: Timm Bäder
Date: 2023-04-25T09:00:47+02:00
New Revision: c9e403d1992b064e9cd5b94749fb3f00fa0c0910

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

LOG: [clang][Interp] Fix zero-init of float and pointer arrays

Our Zero opcode only exists for integer types. Use
visitZeroInitializer() here as well so it works for floats and pointers.

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

Added: 


Modified: 
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/test/AST/Interp/arrays.cpp

Removed: 




diff  --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp 
b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index 536438b347a20..9f79a7bcf9d34 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1291,7 +1291,7 @@ bool 
ByteCodeExprGen::visitArrayInitializer(const Expr *Initializer) {
   //   since we memset our Block*s to 0 and so we have the desired value
   //   without this.
   for (size_t I = 0; I != NumElems; ++I) {
-if (!this->emitZero(*ElemT, Initializer))
+if (!this->visitZeroInitializer(CAT->getElementType(), Initializer))
   return false;
 if (!this->emitInitElem(*ElemT, I, Initializer))
   return false;

diff  --git a/clang/test/AST/Interp/arrays.cpp 
b/clang/test/AST/Interp/arrays.cpp
index 413ab2fa45d84..22ccafd579241 100644
--- a/clang/test/AST/Interp/arrays.cpp
+++ b/clang/test/AST/Interp/arrays.cpp
@@ -334,3 +334,19 @@ namespace IncDec {
// ref-error {{not an integral constant 
expression}} \
   // ref-note {{in call to}}
 };
+
+namespace ZeroInit {
+  struct A {
+int *p[2];
+  };
+  constexpr A a = {};
+  static_assert(a.p[0] == nullptr, "");
+  static_assert(a.p[1] == nullptr, "");
+
+  struct B {
+double f[2];
+  };
+  constexpr B b = {};
+  static_assert(b.f[0] == 0.0, "");
+  static_assert(b.f[1] == 0.0, "");
+}



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


[PATCH] D149059: [clang][Interp] Fix zero-init of float and pointer arrays

2023-04-25 Thread Timm Bäder 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 rGc9e403d1992b: [clang][Interp] Fix zero-init of float and 
pointer arrays (authored by tbaeder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149059

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


Index: clang/test/AST/Interp/arrays.cpp
===
--- clang/test/AST/Interp/arrays.cpp
+++ clang/test/AST/Interp/arrays.cpp
@@ -334,3 +334,19 @@
// ref-error {{not an integral constant 
expression}} \
   // ref-note {{in call to}}
 };
+
+namespace ZeroInit {
+  struct A {
+int *p[2];
+  };
+  constexpr A a = {};
+  static_assert(a.p[0] == nullptr, "");
+  static_assert(a.p[1] == nullptr, "");
+
+  struct B {
+double f[2];
+  };
+  constexpr B b = {};
+  static_assert(b.f[0] == 0.0, "");
+  static_assert(b.f[1] == 0.0, "");
+}
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1291,7 +1291,7 @@
   //   since we memset our Block*s to 0 and so we have the desired value
   //   without this.
   for (size_t I = 0; I != NumElems; ++I) {
-if (!this->emitZero(*ElemT, Initializer))
+if (!this->visitZeroInitializer(CAT->getElementType(), Initializer))
   return false;
 if (!this->emitInitElem(*ElemT, I, Initializer))
   return false;


Index: clang/test/AST/Interp/arrays.cpp
===
--- clang/test/AST/Interp/arrays.cpp
+++ clang/test/AST/Interp/arrays.cpp
@@ -334,3 +334,19 @@
// ref-error {{not an integral constant expression}} \
   // ref-note {{in call to}}
 };
+
+namespace ZeroInit {
+  struct A {
+int *p[2];
+  };
+  constexpr A a = {};
+  static_assert(a.p[0] == nullptr, "");
+  static_assert(a.p[1] == nullptr, "");
+
+  struct B {
+double f[2];
+  };
+  constexpr B b = {};
+  static_assert(b.f[0] == 0.0, "");
+  static_assert(b.f[1] == 0.0, "");
+}
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1291,7 +1291,7 @@
   //   since we memset our Block*s to 0 and so we have the desired value
   //   without this.
   for (size_t I = 0; I != NumElems; ++I) {
-if (!this->emitZero(*ElemT, Initializer))
+if (!this->visitZeroInitializer(CAT->getElementType(), Initializer))
   return false;
 if (!this->emitInitElem(*ElemT, I, Initializer))
   return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D149123: [AArch64][InlineAsm]Add Clang support for flag output constraints

2023-04-25 Thread Mingming Liu via Phabricator via cfe-commits
mingmingl updated this revision to Diff 516640.
mingmingl edited the summary of this revision.
mingmingl added reviewers: dmgreen, nickdesaulniers, efriedma.
mingmingl added a comment.

remove 'No newline at end of file'


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

https://reviews.llvm.org/D149123

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/test/CodeGen/inline-asm-aarch64-flag-output.c
  clang/test/Preprocessor/aarch64_asm_flag_output.c
  clang/test/Preprocessor/init-aarch64.c
  clang/test/Preprocessor/predefined-win-macros.c

Index: clang/test/Preprocessor/predefined-win-macros.c
===
--- clang/test/Preprocessor/predefined-win-macros.c
+++ clang/test/Preprocessor/predefined-win-macros.c
@@ -129,4 +129,5 @@
 // CHECK-ARM64-MINGW: #define WINNT 1
 // CHECK-ARM64-MINGW: #define _WIN32 1
 // CHECK-ARM64-MINGW: #define _WIN64 1
+// CHECK-ARM64-MINGW: #define __GCC_ASM_FLAG_OUTPUTS__ 1
 // CHECK-ARM64-MINGW: #define __aarch64__ 1
Index: clang/test/Preprocessor/init-aarch64.c
===
--- clang/test/Preprocessor/init-aarch64.c
+++ clang/test/Preprocessor/init-aarch64.c
@@ -106,6 +106,7 @@
 // AARCH64-NEXT: #define __FLT_RADIX__ 2
 // AARCH64-NEXT: #define __FP_FAST_FMA 1
 // AARCH64-NEXT: #define __FP_FAST_FMAF 1
+// AARCH64-NEXT: #define __GCC_ASM_FLAG_OUTPUTS__ 1
 // AARCH64-NEXT: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1
 // AARCH64-NEXT: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2 1
 // AARCH64-NEXT: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 1
Index: clang/test/Preprocessor/aarch64_asm_flag_output.c
===
--- /dev/null
+++ clang/test/Preprocessor/aarch64_asm_flag_output.c
@@ -0,0 +1,3 @@
+// RUN: %clang -target aarch64-unknown-unknown -x c -E -dM -o - %s | FileCheck -match-full-lines %s
+
+// CHECK: #define __GCC_ASM_FLAG_OUTPUTS__ 1
Index: clang/test/CodeGen/inline-asm-aarch64-flag-output.c
===
--- /dev/null
+++ clang/test/CodeGen/inline-asm-aarch64-flag-output.c
@@ -0,0 +1,130 @@
+// RUN: %clang_cc1 -O2 -emit-llvm %s -o - -triple aarch64 | FileCheck %s
+
+int test_cceq(int a, int* b) {
+// CHECK-LABEL: @test_cceq
+// CHECK: = tail call1 { i32, i32 } asm "ands ${0:w}, ${0:w}, #3", "=r,={@cceq},0"(i32 %a)
+  asm("ands %w[a], %w[a], #3"
+  : [a] "+r"(a), "=@cceq"(*b));
+  return a;
+}
+
+int test_ccne(int a, int* b) {
+// CHECK-LABEL: @test_ccne
+// CHECK: = tail call { i32, i32 } asm "ands ${0:w}, ${0:w}, #3", "=r,={@ccne},0"(i32 %a)
+  asm("ands %w[a], %w[a], #3"
+  : [a] "+r"(a), "=@ccne"(*b));
+  return a;
+}
+
+int test_cccs(int a, int* b) {
+// CHECK-LABEL: @test_cccs
+// CHECK: = tail call { i32, i32 } asm "ands ${0:w}, ${0:w}, #3", "=r,={@cccs},0"(i32 %a)
+  asm("ands %w[a], %w[a], #3"
+  : [a] "+r"(a), "=@cccs"(*b));
+  return a;
+}
+
+int test_cchs(int a, int* b) {
+// CHECK-LABEL: @test_cchs
+// CHECK: = tail call { i32, i32 } asm "ands ${0:w}, ${0:w}, #3", "=r,={@cchs},0"(i32 %a)
+  asm("ands %w[a], %w[a], #3"
+  : [a] "+r"(a), "=@cchs"(*b));
+  return a;
+}
+
+int test_(int a, int* b) {
+// CHECK-LABEL: @test_
+// CHECK: = tail call { i32, i32 } asm "ands ${0:w}, ${0:w}, #3", "=r,={@},0"(i32 %a)
+  asm("ands %w[a], %w[a], #3"
+  : [a] "+r"(a), "=@"(*b));
+  return a;
+}
+
+int test_cclo(int a, int* b) {
+// CHECK-LABEL: @test_cclo
+// CHECK: = tail call { i32, i32 } asm "ands ${0:w}, ${0:w}, #3", "=r,={@cclo},0"(i32 %a)
+  asm("ands %w[a], %w[a], #3"
+  : [a] "+r"(a), "=@cclo"(*b));
+  return a;
+}
+
+int test_ccmi(int a, int* b) {
+// CHECK-LABEL: @test_ccmi
+// CHECK: = tail call { i32, i32 } asm "ands ${0:w}, ${0:w}, #3", "=r,={@ccmi},0"(i32 %a)
+  asm("ands %w[a], %w[a], #3"
+  : [a] "+r"(a), "=@ccmi"(*b));
+  return a;
+}
+
+int test_ccpl(int a, int* b) {
+// CHECK-LABEL: @test_ccpl
+// CHECK: = tail call { i32, i32 } asm "ands ${0:w}, ${0:w}, #3", "=r,={@ccpl},0"(i32 %a)
+  asm("ands %w[a], %w[a], #3"
+  : [a] "+r"(a), "=@ccpl"(*b));
+  return a;
+}
+
+int test_ccvs(int a, int* b) {
+// CHECK-LABEL: @test_ccvs
+// CHECK: = tail call { i32, i32 } asm "ands ${0:w}, ${0:w}, #3", "=r,={@ccvs},0"(i32 %a)
+  asm("ands %w[a], %w[a], #3"
+  : [a] "+r"(a), "=@ccvs"(*b));
+  return a;
+}
+
+int test_ccvc(int a, int* b) {
+// CHECK-LABEL: @test_ccvc
+// CHECK: = tail call { i32, i32 } asm "ands ${0:w}, ${0:w}, #3", "=r,={@ccvc},0"(i32 %a)
+  asm("ands %w[a], %w[a], #3"
+  : [a] "+r"(a), "=@ccvc"(*b));
+  return a;
+}
+
+int test_cchi(int a, int* b) {
+// CHECK-LABEL: @test_cchi
+// CHECK: = tail call { i32, i32 } asm "ands ${0:w}, ${0:w}, #3", "=r,={@cchi},0"(i32 %a)
+  asm("ands %w[a], %w[a], #3"
+  : [a] "+r"(a), "=@cchi"(*b));
+  return a;
+}
+
+int test_ccls(int a, int* b) {
+// CHECK-LABEL: @test_ccls
+// CHECK: = tail cal

[PATCH] D149123: [AArch64][InlineAsm]Add Clang support for flag output constraints

2023-04-25 Thread Mingming Liu via Phabricator via cfe-commits
mingmingl added a comment.

While testing this patch with `./bin/clang -cc1 -S -triple=aarch64 
inline-asm-aarch64-flag-output.c` (which invokes global-isel for instruction 
selection according to `print-after-all` output), turns out GlobalISel doesn't 
support flag output yet (for x86 or aarch64).

`/bin/clang -cc1 -O1 -S -triple=aarch64 file.c` and `/bin/clang -cc1 -O2 -S 
-triple=aarch64 file.c` works.

I filed https://github.com/llvm/llvm-project/issues/62343 to track but but 
think the issue is a non-blocker of this patch and D149032 
 for now.


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

https://reviews.llvm.org/D149123

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


[PATCH] D147610: [RISCV][MC] Add support for experimental Zfbfmin extension

2023-04-25 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment.

In D147610#4294322 , @joshua-arch1 
wrote:

> In D147610#4294260 , @craig.topper 
> wrote:
>
>> In D147610#4294247 , @joshua-arch1 
>> wrote:
>>
>>> I'm wondering whether it is appropriate to just use FPR16 for the 
>>> destination of fcvt.bf16.s? The destination is in BF16 format instead of 
>>> simple FP16.  Your implemention looks like just replacing fcvt.h.s with 
>>> fcvt.bf16.s. Do we need to define a new register class?
>>
>> Registers classes only distinguished by the registers in them and what their 
>> alignment and spill size are. It doesn't define anything about the format of 
>> the register. A BF16 specific register class would be identical in those 
>> properties to the FPR16 register class.
>
> Do you have any plan for code generation of these instructions with 
> corresponding intrinsics?

The RVI toolchain SIG is supposed to be setting up a task group to define 
intrinsics for all extensions.


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

https://reviews.llvm.org/D147610

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


[PATCH] D143364: [RISCV] Support scalar/fix-length vector NTLH intrinsic with different domain

2023-04-25 Thread NAKAMURA Takumi via Phabricator via cfe-commits
chapuni added inline comments.



Comment at: clang/lib/Headers/CMakeLists.txt:360
 endif()
 if(RISCV IN_LIST LLVM_TARGETS_TO_BUILD)
   # Generate riscv_vector.h

Here



Comment at: clang/test/CodeGen/RISCV/ntlh-intrinsics/riscv32-zihintntl.c:5
+#include 
+#include 
+

This is generated only if riscv is configured.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143364

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


[PATCH] D147989: [clang] Fix Attribute Placement

2023-04-25 Thread Priyanshi Agarwal via Phabricator via cfe-commits
ipriyanshi1708 marked an inline comment as done.
ipriyanshi1708 added a comment.

In D147989#4293278 , @aaron.ballman 
wrote:

> Oops, I spoke too soon -- it looks like the precommit CI failure is related 
> to this patch. This is what I get when I tested locally:
>
>   Assertion failed: NextVal != ArgumentEnd && "Value for integer select 
> modifier was" " larger than the number of options in the diagnostic string!", 
> file F:\source\llvm-project\clang\lib\Basic\Diagnostic.cpp, line 623
>   PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ 
> and include the crash backtrace, preprocessed source, and associated run 
> script.
>   Stack dump:
>   0.  Program arguments: 
> f:\\source\\llvm-project\\llvm\\out\\build\\x64-debug\\bin\\clang.exe -cc1 
> -internal-isystem 
> f:\\source\\llvm-project\\llvm\\out\\build\\x64-debug\\lib\\clang\\17\\include
>  -nostdsysteminc -std=c++11 -triple x86_64-unknown-unknown 
> F:\\source\\llvm-project\\clang\\test\\CXX\\drs\\dr16xx.cpp -verify 
> -fexceptions -fcxx-exceptions -pedantic-errors
>   1.  F:\source\llvm-project\clang\test\CXX\drs\dr16xx.cpp:91:3: current 
> parser token 'template'
>   2.  F:\source\llvm-project\clang\test\CXX\drs\dr16xx.cpp:71:1: parsing 
> namespace 'dr1638'
>   Exception Code: 0x8003
>#0 0x7ff6625ea26c HandleAbort 
> F:\source\llvm-project\llvm\lib\Support\Windows\Signals.inc:419:0
>#1 0x7ffa9b23bc31 (C:\WINDOWS\SYSTEM32\ucrtbased.dll+0x6bc31)
>#2 0x7ffa9b23d889 (C:\WINDOWS\SYSTEM32\ucrtbased.dll+0x6d889)
>#3 0x7ffa9b2430bf (C:\WINDOWS\SYSTEM32\ucrtbased.dll+0x730bf)
>#4 0x7ffa9b241091 (C:\WINDOWS\SYSTEM32\ucrtbased.dll+0x71091)
>#5 0x7ffa9b243a1f (C:\WINDOWS\SYSTEM32\ucrtbased.dll+0x73a1f)
>#6 0x7ff662a19b8c HandleSelectModifier 
> F:\source\llvm-project\clang\lib\Basic\Diagnostic.cpp:622:0
>#7 0x7ff662a17137 clang::Diagnostic::FormatDiagnostic(char const *, 
> char const *, class llvm::SmallVectorImpl &) const 
> F:\source\llvm-project\clang\lib\Basic\Diagnostic.cpp:1005:0
>#8 0x7ff662a16518 clang::Diagnostic::FormatDiagnostic(class 
> llvm::SmallVectorImpl &) const 
> F:\source\llvm-project\clang\lib\Basic\Diagnostic.cpp:805:0
>#9 0x7ff663e5bcc9 clang::TextDiagnosticBuffer::HandleDiagnostic(enum 
> clang::DiagnosticsEngine::Level, class clang::Diagnostic const &) 
> F:\source\llvm-project\clang\lib\Frontend\TextDiagnosticBuffer.cpp:30:0
>   #10 0x7ff663ef2d71 
> clang::VerifyDiagnosticConsumer::HandleDiagnostic(enum 
> clang::DiagnosticsEngine::Level, class clang::Diagnostic const &) 
> F:\source\llvm-project\clang\lib\Frontend\VerifyDiagnosticConsumer.cpp:757:0
>   #11 0x7ff662a0b66f clang::DiagnosticIDs::EmitDiag(class 
> clang::DiagnosticsEngine &, enum clang::DiagnosticIDs::Level) const 
> F:\source\llvm-project\clang\lib\Basic\DiagnosticIDs.cpp:839:0
>   #12 0x7ff662a0b597 clang::DiagnosticIDs::ProcessDiag(class 
> clang::DiagnosticsEngine &) const 
> F:\source\llvm-project\clang\lib\Basic\DiagnosticIDs.cpp:831:0
>   #13 0x7ff662a267ef clang::DiagnosticsEngine::ProcessDiag(void) 
> F:\source\llvm-project\clang\include\clang\Basic\Diagnostic.h:1038:0
>   #14 0x7ff662a15f64 
> clang::DiagnosticsEngine::EmitCurrentDiagnostic(bool) 
> F:\source\llvm-project\clang\lib\Basic\Diagnostic.cpp:549:0
>   #15 0x7ff667b47cbc clang::Sema::EmitCurrentDiagnostic(unsigned int) 
> F:\source\llvm-project\clang\lib\Sema\Sema.cpp:1575:0
>   #16 0x7ff667b89d97 
> clang::Sema::ImmediateDiagBuilder::~ImmediateDiagBuilder(void) 
> F:\source\llvm-project\clang\include\clang\Sema\Sema.h:1726:0
>   #17 0x7ff667b8e9e8 clang::Sema::ImmediateDiagBuilder::`scalar deleting 
> dtor'(unsigned int) 
> (f:\source\llvm-project\llvm\out\build\x64-debug\bin\clang.exe+0x81fe9e8)
>   #18 0x7ff667b72b06 std::_Destroy_in_place clang::Sema::ImmediateDiagBuilder>(class clang::Sema::ImmediateDiagBuilder &) 
> C:\Program Files\Microsoft Visual 
> Studio\2022\Professional\VC\Tools\MSVC\14.35.32215\include\xmemory:300:0
>   #19 0x7ff667bac5b4 std::_Optional_destruct_base clang::Sema::ImmediateDiagBuilder, 0>::reset(void) C:\Program Files\Microsoft 
> Visual 
> Studio\2022\Professional\VC\Tools\MSVC\14.35.32215\include\optional:133:0
>   #20 0x7ff667b53424 
> clang::Sema::SemaDiagnosticBuilder::~SemaDiagnosticBuilder(void) 
> F:\source\llvm-project\clang\lib\Sema\Sema.cpp:1859:0
>   #21 0x7ff668618b4a clang::Sema::ParsedFreeStandingDeclSpec(class 
> clang::Scope *, enum clang::AccessSpecifier, class clang::DeclSpec &, class 
> clang::ParsedAttributesView const &, class llvm::MutableArrayRef clang::TemplateParameterList *>, bool, class clang::RecordDecl *&) 
> F:\source\llvm-project\clang\lib\Sema\SemaDecl.cpp:5151:0
>   #22 0x7ff668618545 clang::Sema::ParsedFreeStandingDeclSpec(class 
> clang::Scope *, enum clang::AccessSpecifier, class clang::DeclSpec &, class 
> clang::Par

[PATCH] D134821: [flang][driver] Allow main program to be in an archive

2023-04-25 Thread Shao-Ce SUN via Phabricator via cfe-commits
sunshaoce updated this revision to Diff 516651.
sunshaoce added a comment.

Address @awarzynski's comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134821

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  flang/docs/FlangDriver.md
  flang/test/CMakeLists.txt
  flang/test/Driver/link-c-main.c
  flang/test/Driver/link-f90-main.f90
  flang/test/Driver/linker-flags.f90

Index: flang/test/Driver/linker-flags.f90
===
--- flang/test/Driver/linker-flags.f90
+++ flang/test/Driver/linker-flags.f90
@@ -12,6 +12,14 @@
 !   Make sure they're not added.
 ! RUN: %flang -### -flang-experimental-exec -target aarch64-windows-msvc %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,MSVC --implicit-check-not libcmt --implicit-check-not oldnames
 
+! Check linker invocation to generate shared object (only GNU toolchain for now)
+! Output should not contain any undefined reference to _QQmain since it is not
+! considered a valid entry point for shared objects, which are usually specified
+! using the bind attribute.
+! RUN: %flang -### -flang-experimental-exec -shared -target x86_64-linux-gnu %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,GNU-SHARED --implicit-check-not _QQmain
+! RUN: %flang -### -flang-experimental-exec -shared -target aarch64-linux-gnu %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,GNU-SHARED --implicit-check-not _QQmain
+! RUN: %flang -### -flang-experimental-exec -shared -target riscv64-linux-gnu %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,GNU-SHARED --implicit-check-not _QQmain
+
 ! Compiler invocation to generate the object file
 ! CHECK-LABEL: {{.*}} "-emit-obj"
 ! CHECK-SAME:  "-o" "[[object_file:.*\.o]]" {{.*}}Inputs/hello.f90
@@ -23,6 +31,7 @@
 !   executable and may find the GNU linker from MinGW or Cygwin.
 ! GNU-LABEL:  "{{.*}}ld{{(\.exe)?}}"
 ! GNU-SAME: "[[object_file]]"
+! GNU-SAME: --undefined=_QQmain
 ! GNU-SAME: -lFortran_main
 ! GNU-SAME: -lFortranRuntime
 ! GNU-SAME: -lFortranDecimal
@@ -50,3 +59,9 @@
 ! MSVC-SAME: FortranDecimal.lib
 ! MSVC-SAME: /subsystem:console
 ! MSVC-SAME: "[[object_file]]"
+
+! Linker invocation to generate a shared object
+! GNU-SHARED-LABEL:  "{{.*}}ld"
+! GNU-SHARED-SAME: "[[object_file]]"
+! GNU-SHARED-SAME: -lFortranRuntime
+! GNU-SHARED-SAME: -lFortranDecimal
Index: flang/test/Driver/link-f90-main.f90
===
--- /dev/null
+++ flang/test/Driver/link-f90-main.f90
@@ -0,0 +1,23 @@
+! Test that a fortran main program can be linked to an executable
+! by flang.
+!
+! For now, this test only covers the Gnu toolchain on linux.
+
+!REQUIRES: x86-registered-target || aarch64-registered-target || riscv64-registered-target
+!REQUIRES: system-linux
+
+! RUN: %flang_fc1 -emit-obj %s -o %t.o
+! RUN: %flang -target x86_64-unknown-linux-gnu %t.o -o %t.out -flang-experimental-exec
+! RUN: llvm-objdump --syms %t.out | FileCheck %s
+
+! Test that it also works if the program is bundled in an archive.
+
+! RUN: llvm-ar -r %t.a %t.o
+! RUN: %flang -target x86_64-unknown-linux-gnu %t.a -o %ta.out -flang-experimental-exec
+! RUN: llvm-objdump --syms %ta.out | FileCheck %s
+
+end program
+
+! CHECK-DAG: F .text {{[a-f0-9]+}} main
+! CHECK-DAG: F .text {{[a-f0-9]+}} _QQmain
+! CHECK-DAG: _FortranAProgramStart
Index: flang/test/Driver/link-c-main.c
===
--- /dev/null
+++ flang/test/Driver/link-c-main.c
@@ -0,0 +1,28 @@
+/*
+Test that an object file with a C main function can be linked to an executable
+by Flang.
+
+For now, this test only covers the Gnu toolchain on Linux.
+
+REQUIRES: x86-registered-target || aarch64-registered-target || riscv64-registered-target
+REQUIRES: system-linux, c-compiler
+
+RUN: %cc -c %s -o %t.o
+RUN: %flang -target x86_64-unknown-linux-gnu %t.o -o %t.out -flang-experimental-exec
+RUN: llvm-objdump --syms %t.out | FileCheck %s --implicit-check-not Fortran
+
+Test that it also works if the c-main is bundled in an archive.
+
+RUN: llvm-ar -r %t.a %t.o
+RUN: %flang -target x86_64-unknown-linux-gnu %t.a -o %ta.out -flang-experimental-exec
+RUN: llvm-objdump --syms %ta.out | FileCheck %s --implicit-check-not Fortran
+*/
+
+int main(void) {
+return 0;
+}
+
+/*
+CHECK-DAG: F .text {{[a-f0-9]+}} main
+CHECK-DAG: *UND* {{[a-f0-9]+}} _QQmain
+*/
Index: flang/test/CMakeLists.txt
===
--- flang/test/CMakeLists.txt
+++ flang/test/CMakeLists.txt
@@ -57,6 +57,7 @@
   fir-opt
   tco
   bbc
+  llvm-ar
   llvm-dis
   llvm-objdump
   llvm-readobj
Index: flang/docs/FlangDriver.md
===
--- flang/docs/FlangDriver.md
+++ flang/docs/FlangDriver.md
@@ -149,13 +149,6 @@
 +- 3: backend, {2}, assembler
 4: assemb

[PATCH] D148723: [clang] Enforce external linkage for inline builtin original declaration

2023-04-25 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 516645.
serge-sans-paille retitled this revision from "[clang] Enforce internal linkage 
for inline builtin" to "[clang] Enforce external linkage for inline builtin 
original declaration".
serge-sans-paille edited the summary of this revision.
serge-sans-paille added a comment.
Herald added a subscriber: mstorsjo.

Change approach and update test case accordingly.


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

https://reviews.llvm.org/D148723

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/CodeGen/asm-label-inline-builtins.c
  clang/test/CodeGen/inline-builtin-asm-name.c
  clang/test/CodeGen/inline-builtin-comdat.c
  clang/test/CodeGen/pr9614.c

Index: clang/test/CodeGen/pr9614.c
===
--- clang/test/CodeGen/pr9614.c
+++ clang/test/CodeGen/pr9614.c
@@ -30,6 +30,8 @@
   memchr("", '.', 0);
 }
 
+// CHECK-LABEL: declare i32 @abs(i32
+//
 // CHECK-LABEL: define{{.*}} void @f()
 // CHECK: call void @foo()
 // CHECK: call i32 @abs(i32 noundef %0)
@@ -37,9 +39,8 @@
 // CHECK: call void @llvm.prefetch.p0(
 // CHECK: call ptr @memchr(
 // CHECK: ret void
-
-// CHECK: declare void @foo()
-// CHECK: declare i32 @abs(i32
-// CHECK: declare ptr @strrchr(ptr noundef, i32 noundef)
-// CHECK: declare ptr @memchr(
-// CHECK: declare void @llvm.prefetch.p0(
+//
+// CHECK-LABEL: declare void @foo()
+// CHECK-LABEL: declare ptr @strrchr(ptr noundef, i32 noundef)
+// CHECK-LABEL: declare ptr @memchr(
+// CHECK-LABEL: declare void @llvm.prefetch.p0(
Index: clang/test/CodeGen/inline-builtin-comdat.c
===
--- /dev/null
+++ clang/test/CodeGen/inline-builtin-comdat.c
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -triple x86_64-windows -S -emit-llvm -disable-llvm-passes %s -o - | FileCheck %s
+// Make sure we don't generate definition for Inline Builtins, and that all
+// linkage are compatible with comdat rules.
+
+double __cdecl frexp( double _X, int* _Y);
+inline __attribute__((always_inline))  long double __cdecl frexpl( long double __x, int *__exp ) {
+  return (long double) frexp((double)__x, __exp );
+}
+
+long double pain(void)
+{
+long double f = 123.45;
+int i;
+long double f2 = frexpl(f, &i);
+return f2;
+}
+
+// CHECK-NOT: define {{.*}} @frexpl(
+//
+// CHECK-LABEL: declare dso_local double @frexpl(
+//
+// CHECK-LABEL: define internal double @frexpl.inline(
+// CHECK:   call double @frexp
+//
+// CHECK-LABEL: declare dso_local double @frexp(
+//
+// CHECK-LABEL: define dso_local double @pain(
+// CHECK:call double @frexpl.inline(
Index: clang/test/CodeGen/inline-builtin-asm-name.c
===
--- clang/test/CodeGen/inline-builtin-asm-name.c
+++ clang/test/CodeGen/inline-builtin-asm-name.c
@@ -1,14 +1,14 @@
 // RUN: %clang_cc1 -triple i686-windows-gnu -emit-llvm -o - %s -disable-llvm-optzns | FileCheck %s
 
-// CHECK: call i32 @"\01_asm_func_name.inline"
-
-// CHECK: declare dso_local i32 @"\01_asm_func_name"(ptr noundef, i32 noundef, ptr noundef, ptr noundef)
-
-// CHECK: define internal i32 @"\01_asm_func_name.inline"
-
+// CHECK-LABEL: declare dso_local i32 @"\01_asm_func_name"(ptr noundef, i32 noundef, ptr noundef, ptr noundef)
+//
+// CHECK-LABEL: define internal i32 @"\01_asm_func_name.inline"(
 // CHECK: call i32 @__mingw_vsnprintf
-
-// CHECK: declare dso_local i32 @__mingw_vsnprintf
+//
+// CHECK-LABEL: declare dso_local i32 @__mingw_vsnprintf
+//
+// CHECK-LABEL: define dso_local void @call(ptr noundef %fmt, ...)
+// CHECK: call i32 @"\01_asm_func_name.inline"
 
 typedef unsigned int size_t;
 
Index: clang/test/CodeGen/asm-label-inline-builtins.c
===
--- clang/test/CodeGen/asm-label-inline-builtins.c
+++ clang/test/CodeGen/asm-label-inline-builtins.c
@@ -25,8 +25,8 @@
   vprintf(fmt, ap);
 }
 
-// CHECK-LABEL: void @test(
-// CHECK: call i32 @__vprintfieee128.inline(
-//
 // CHECK-LABEL: internal i32 @__vprintfieee128.inline(
 // CHECK: call i32 @__vfprintf_chkieee128(
+//
+// CHECK-LABEL: void @test(
+// CHECK: call i32 @__vprintfieee128.inline(
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -11552,6 +11552,11 @@
   if (!FD->isExternallyVisible())
 return GVA_Internal;
 
+  // Inline Builtin declaration are forward declared as external and their
+  // actual implementation is provided under the ".inline" declaration.
+  if (FD->isInlineBuiltinDeclaration())
+return GVA_StrongExternal;
+
   // Non-user-provided functions get emitted as weak definitions with every
   // use, no matter whether they've been explicitly instantiated etc.
   if (!FD->isUserProvided())
___
cfe-commits mailing list
cfe-commits@lists

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

2023-04-25 Thread Donát Nagy via Phabricator via cfe-commits
donat.nagy marked 4 inline comments as done.
donat.nagy added a comment.

@steakhal I marked a few comments as Done (I accidentally missed some when I 
was creating the most recent patch) and now the only not-Done thing is the 
followup commit for refactoring the optionalness of RegionRawOffsetV2. Do you 
see anything else to do?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148355

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


[PATCH] D148949: [dataflow] HTMLLogger - show the value of the current expr

2023-04-25 Thread Martin Böhme via Phabricator via cfe-commits
mboehme added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp:96
+for (const auto& Prop : V.properties())
+  JOS.attributeObject(Prop.first(), [&] { dump(*Prop.second); });
+

sammccall wrote:
> mboehme wrote:
> > IIUC, this places properties on the current HTML element as attributes, 
> > just like the built-in attributes that we add for other purposes (e.g. 
> > "value_id", "kind").
> > 
> >   - What happens if we have a property whose name conflicts with one of the 
> > built-in attributes?
> >   - Even if we don't have a naming conflict, I think it could be 
> > potentially confusing to have user-defined properties appear in the same 
> > list and in the same way as built-in attributes.
> > 
> > Suggestion: Can we nest all properties inside of a "properties" attribute?
> > 
> > Edit: Having looked at the HTML template now, I see that we exclude certain 
> > attributes there ('kind', 'value_id', 'type', 'location') when listing 
> > properties. I still think naming conflicts are a potential problem though. 
> > I think it would also be clearer to explicitly pick the properties out of a 
> > `properties` attribute rather than excluding a blocklist of attributes.
> Right, the data model is: a value (really, a Value/StorageLocation mashed 
> together) is just a bag of attributes.
> 
> I don't think making it more complicated is an improvement: being built-in 
> isn't the same thing as being custom-rendered.
> e.g. "pointee" and "truth" want the default key-value rendering despite being 
> built-in.
> Having the exclude list in the template is ugly, but either you end up 
> encoding the rendering info twice in the template like that, or you encode it 
> once in the template and once in the JSON generation (by what goes in the 
> "properties" map vs the main map). I'd rather call this purely a template 
> concern.
> 
> Namespace conflict could be a problem: the current behavior is that the last 
> value wins (per JS rules).
> IMO the simplest fix is to prepend "p:" and "f:" to properties/struct fields. 
> These would be shown - otherwise the user can't distinguish between a 
> property & field with the same name.
> 
> I had this in the prototype, but dropped them because they seemed a bit ugly 
> and conflicts unlikely in practice. WDYT?
> Namespace conflict could be a problem: the current behavior is that the last 
> value wins (per JS rules).
> IMO the simplest fix is to prepend "p:" and "f:" to properties/struct fields. 
> These would be shown - otherwise the user can't distinguish between a 
> property & field with the same name.

Yes, this makes sense to me. I looked at your example screenshot and wasn't 
sure whether they were both fields or whether one of them was a property -- I 
think there's value in indicating explicitly what they are.

> I had this in the prototype, but dropped them because they seemed a bit ugly 
> and conflicts unlikely in practice. WDYT?

I do think there's a fair chance of conflicts -- many of the attribute names 
here are short and generic and look like likely field names (e.g. `kind`, 
`type`). Even if the chance of a conflict is relatively low, a conflict will be 
pretty confusing when it does happen -- and given that we'll be using this 
feature when we're debugging (i.e. already confused), I think this is worth 
avoiding.

One more question: How do the "p:" and "f:" items sort in the output? I think 
these should be sorted together and grouped -- e.g. builtins first, then 
fields, then properties. (Yes, I know this is more work... but I think it's 
worth it.)



Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp:121-123
+  for (const auto &Child : cast(V).children())
+JOS.attributeObject(Child.first->getNameAsString(),
+[&] { dump(*Child.second); });

sammccall wrote:
> mboehme wrote:
> > 
> this is neat but capturing the structured binding `Val` is a C++20 feature
Are you sure? I can see nothing here that would indicate this:

https://en.cppreference.com/w/cpp/language/structured_binding

And Clang doesn't complain in `-std=c++17`:

https://godbolt.org/z/jvYE3cTdq


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148949

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


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

2023-04-25 Thread Donát Nagy via Phabricator via cfe-commits
donat.nagy added inline comments.



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

steakhal wrote:
> steakhal wrote:
> > donat.nagy wrote:
> > > donat.nagy wrote:
> > > > steakhal wrote:
> > > > > donat.nagy wrote:
> > > > > > steakhal wrote:
> > > > > > > 
> > > > > > You're completely right, I just blindly copied this test from the 
> > > > > > needlessly overcomplicated `computeExtentBegin()`.
> > > > > Hold on. This would only skip the lower bounds check if it's an 
> > > > > `UnknownSpaceRegion`.
> > > > > Shouldn't we early return instead?
> > > > This behavior is inherited from the code before my commit: the old 
> > > > block `if ( /*... =*/ extentBegin.getAs() ) { /* ... */ }` is 
> > > > equivalent to `if (llvm::isa(SR)) { /*...*/ }` and 
> > > > there was no early return connected to //this// NonLocness check. (The 
> > > > old code skipped the upper bound check if the result of `evalBinOpNN()` 
> > > > is unknown, and that's what I changed because I saw no reason to do an 
> > > > early return there.)
> > > > 
> > > > After some research into the memory region model, I think that there is 
> > > > no reason to perform an early return -- in fact, the condition of this  
> > > > `if` seems to be too narrow because we would like to warn about code 
> > > > like
> > > >   struct foo {
> > > > int tag;
> > > > int array[5];
> > > >   };
> > > >   int f(struct foo *p) {
> > > > return p->arr[-1];
> > > >   }
> > > > despite the fact that it's indexing into a `FieldRegion` inside a 
> > > > `SymbolicRegion` in `UnknownSpaceRegion`. That is, instead of checking 
> > > > the top-level MemorySpace, the correct logic would be checking the kind 
> > > > of the memory region and/or perhaps its immediate super-region.
> > > > 
> > > > As this is a complex topic and completely unrelated to the main goal of 
> > > > this commit; I'd prefer to keep the old (not ideal, but working) logic 
> > > > in this patch, then revisit this question by creating a separate 
> > > > follow-up commit.
> > > Minor nitpick: your suggested change accidentally negated the conditional 
> > > :) ... and I said that it's "completely right". I'm glad that I noticed 
> > > this and inserted the "!" before the `isa` check because otherwise it 
> > > could've been annoying to debug this...
> > Agreed.
> Sorry about that. Happens
No problem :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148355

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


[PATCH] D148730: [C11] Allow initialization of an atomic-qualified pointer from a null pointer constant

2023-04-25 Thread Bogdan Graur via Phabricator via cfe-commits
bgraur added a comment.

We see the same compilation error as the one reported by @sbc100 in several 
other C libraries.

Here's a short reproducer: https://godbolt.org/z/Y8eds47na

The tests imply that this should work. Please revert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148730

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


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

2023-04-25 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

In D148355#4294738 , @donat.nagy 
wrote:

> @steakhal I marked a few comments as Done (I accidentally missed some when I 
> was creating the most recent patch) and now the only not-Done thing is the 
> followup commit for refactoring the optionalness of RegionRawOffsetV2. Do you 
> see anything else to do?

I think it's good. I've just scheduled the measurement now for 180+ OSS 
projects. Stay tuned!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148355

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


[PATCH] D148284: [clangd] Add "readonly" token to const member expressions

2023-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added a comment.

Thanks for the patch!

`addExtraModifier` is a kind of hacky mechanism to attach modifiers in 
`CollectExtraHighlightings` to tokens created during 
`findExplicitReferences()`; it would be nicer to add the modifier at the time 
of creating the token, but it doesn't look like we have enough information in 
`ReferenceLoc` to do that in this case. We could consider extending 
`ReferenceLoc` with more information in the future, but I think this is fine 
for now.

Do you think it's feasible to make a best-effort attempt to use similar logic 
for `CXXDependentScopeMemberExpr`? I don't think it returns a useful 
`getType()`, but its `getBaseType()` should tell us whether the object type is 
const or not. (It wouldn't handle `mutable` because we don't have a concrete 
member decl to work with, but that's a less common case compared to the base 
type being const or not.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148284

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


[PATCH] D148949: [dataflow] HTMLLogger - show the value of the current expr

2023-04-25 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 516689.
sammccall added a comment.

display properties at the end of the kv list


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148949

Files:
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
  clang/lib/Analysis/FlowSensitive/HTMLLogger.css
  clang/lib/Analysis/FlowSensitive/HTMLLogger.html
  clang/lib/Analysis/FlowSensitive/HTMLLogger.js
  clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
@@ -176,6 +176,7 @@
   << "has analysis point state";
   EXPECT_THAT(Logs[0], HasSubstr("transferBranch(0)")) << "has analysis logs";
   EXPECT_THAT(Logs[0], HasSubstr("LocToVal")) << "has built-in lattice dump";
+  EXPECT_THAT(Logs[0], HasSubstr("\"type\": \"int\"")) << "has value dump";
 }
 
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/HTMLLogger.js
===
--- clang/lib/Analysis/FlowSensitive/HTMLLogger.js
+++ clang/lib/Analysis/FlowSensitive/HTMLLogger.js
@@ -73,6 +73,9 @@
 }
 return parent.insertBefore(clone, next);
   }
+  // data-use="xyz": use  instead. (Allows recursion.)
+  if ('use' in tmpl.dataset)
+return inflate(document.getElementById(tmpl.dataset.use), data, parent, next);
   //  tag handling. Base case: recursively inflate.
   function handle(data) {
 for (c of tmpl.content.childNodes)
Index: clang/lib/Analysis/FlowSensitive/HTMLLogger.html
===
--- clang/lib/Analysis/FlowSensitive/HTMLLogger.html
+++ clang/lib/Analysis/FlowSensitive/HTMLLogger.html
@@ -10,6 +10,30 @@
 
 
 
+
+
+  
+
+  {{v.kind}}
+#{{v.value_id}}
+  
+  
+{{v.type}} @{{v.location}}
+  
+
+
+  {{kv[0]}}
+{{kv[1]}}
+
+  
+
+  
+
+  
+
+
 
 
 
@@ -50,6 +74,10 @@
   {{state.block}} (iteration {{state.iter}}) initial state
   Element {{selection.elt}} (iteration {{state.iter}})
 
+
+  Value
+  
+
 
   Logs
   {{state.logs}}
Index: clang/lib/Analysis/FlowSensitive/HTMLLogger.css
===
--- clang/lib/Analysis/FlowSensitive/HTMLLogger.css
+++ clang/lib/Analysis/FlowSensitive/HTMLLogger.css
@@ -116,3 +116,27 @@
   position: relative;
   margin-left: 0.5em;
 }
+
+.value {
+  border: 1px solid #888;
+  font-size: x-small;
+  flex-grow: 1;
+}
+.value summary {
+  background-color: #ace;
+  display: flex;
+  justify-content: space-between;
+}
+.value .address {
+  font-size: xx-small;
+  font-family: monospace;
+  color: #888;
+}
+.value .property {
+  display: flex;
+  margin-top: 0.5em;
+}
+.value .property .key {
+  font-weight: bold;
+  min-width: 5em;
+}
Index: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
===
--- clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
+++ clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
@@ -67,6 +67,7 @@
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/Program.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
 // Defines assets: HTMLLogger_{html_js,css}
 #include "HTMLLogger.inc"
@@ -79,6 +80,96 @@
 
 using StreamFactory = std::function()>;
 
+// Recursively dumps Values/StorageLocations as JSON
+class ModelDumper {
+public:
+  ModelDumper(llvm::json::OStream &JOS, const Environment &Env)
+  : JOS(JOS), Env(Env) {}
+
+  void dump(Value &V) {
+JOS.attribute("value_id", llvm::to_string(&V));
+if (!Visited.insert(&V).second)
+  return;
+
+JOS.attribute("kind", debugString(V.getKind()));
+
+switch (V.getKind()) {
+case Value::Kind::Integer:
+case Value::Kind::TopBool:
+case Value::Kind::AtomicBool:
+  break;
+case Value::Kind::Reference:
+  JOS.attributeObject(
+  "referent", [&] { dump(cast(V).getReferentLoc()); });
+  break;
+case Value::Kind::Pointer:
+  JOS.attributeObject(
+  "pointee", [&] { dump(cast(V).getPointeeLoc()); });
+  break;
+case Value::Kind::Struct:
+  for (const auto &Child : cast(V).children())
+JOS.attributeObject("f:" + Child.first->getNameAsString(),
+[&] { dump(*Child.second); });
+  break;
+case Value::Kind::Disjunction: {
+  auto &VV = cast(V);
+  JOS.attributeObject("lhs", [&] { dump(VV.getLeftSubValue()); });
+  JOS.attributeObject("rhs", [&] { dump(VV.getRightSubValue()); });
+  break;
+}
+case Value::Kind::Conjunction: {
+  auto &VV = cast(V);
+  JOS.attributeObject

[PATCH] D148949: [dataflow] HTMLLogger - show the value of the current expr

2023-04-25 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp:96
+for (const auto& Prop : V.properties())
+  JOS.attributeObject(Prop.first(), [&] { dump(*Prop.second); });
+

mboehme wrote:
> sammccall wrote:
> > mboehme wrote:
> > > IIUC, this places properties on the current HTML element as attributes, 
> > > just like the built-in attributes that we add for other purposes (e.g. 
> > > "value_id", "kind").
> > > 
> > >   - What happens if we have a property whose name conflicts with one of 
> > > the built-in attributes?
> > >   - Even if we don't have a naming conflict, I think it could be 
> > > potentially confusing to have user-defined properties appear in the same 
> > > list and in the same way as built-in attributes.
> > > 
> > > Suggestion: Can we nest all properties inside of a "properties" attribute?
> > > 
> > > Edit: Having looked at the HTML template now, I see that we exclude 
> > > certain attributes there ('kind', 'value_id', 'type', 'location') when 
> > > listing properties. I still think naming conflicts are a potential 
> > > problem though. I think it would also be clearer to explicitly pick the 
> > > properties out of a `properties` attribute rather than excluding a 
> > > blocklist of attributes.
> > Right, the data model is: a value (really, a Value/StorageLocation mashed 
> > together) is just a bag of attributes.
> > 
> > I don't think making it more complicated is an improvement: being built-in 
> > isn't the same thing as being custom-rendered.
> > e.g. "pointee" and "truth" want the default key-value rendering despite 
> > being built-in.
> > Having the exclude list in the template is ugly, but either you end up 
> > encoding the rendering info twice in the template like that, or you encode 
> > it once in the template and once in the JSON generation (by what goes in 
> > the "properties" map vs the main map). I'd rather call this purely a 
> > template concern.
> > 
> > Namespace conflict could be a problem: the current behavior is that the 
> > last value wins (per JS rules).
> > IMO the simplest fix is to prepend "p:" and "f:" to properties/struct 
> > fields. These would be shown - otherwise the user can't distinguish between 
> > a property & field with the same name.
> > 
> > I had this in the prototype, but dropped them because they seemed a bit 
> > ugly and conflicts unlikely in practice. WDYT?
> > Namespace conflict could be a problem: the current behavior is that the 
> > last value wins (per JS rules).
> > IMO the simplest fix is to prepend "p:" and "f:" to properties/struct 
> > fields. These would be shown - otherwise the user can't distinguish between 
> > a property & field with the same name.
> 
> Yes, this makes sense to me. I looked at your example screenshot and wasn't 
> sure whether they were both fields or whether one of them was a property -- I 
> think there's value in indicating explicitly what they are.
> 
> > I had this in the prototype, but dropped them because they seemed a bit 
> > ugly and conflicts unlikely in practice. WDYT?
> 
> I do think there's a fair chance of conflicts -- many of the attribute names 
> here are short and generic and look like likely field names (e.g. `kind`, 
> `type`). Even if the chance of a conflict is relatively low, a conflict will 
> be pretty confusing when it does happen -- and given that we'll be using this 
> feature when we're debugging (i.e. already confused), I think this is worth 
> avoiding.
> 
> One more question: How do the "p:" and "f:" items sort in the output? I think 
> these should be sorted together and grouped -- e.g. builtins first, then 
> fields, then properties. (Yes, I know this is more work... but I think it's 
> worth it.)
> One more question: How do the "p:" and "f:" items sort in the output? I think 
> these should be sorted together and grouped -- e.g. builtins first, then 
> fields, then properties. (Yes, I know this is more work... but I think it's 
> worth it.)

Javascript objects are ordered these days, so they'll display in the order we 
output them here.
So they're already grouped, I rearranged to put properties at the end.



Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp:121-123
+  for (const auto &Child : cast(V).children())
+JOS.attributeObject(Child.first->getNameAsString(),
+[&] { dump(*Child.second); });

mboehme wrote:
> sammccall wrote:
> > mboehme wrote:
> > > 
> > this is neat but capturing the structured binding `Val` is a C++20 feature
> Are you sure? I can see nothing here that would indicate this:
> 
> https://en.cppreference.com/w/cpp/language/structured_binding
> 
> And Clang doesn't complain in `-std=c++17`:
> 
> https://godbolt.org/z/jvYE3cTdq
Hmm, what about this :-)

> Structured bindings cannot be captured by lambda expressions: (until C++20)

> https://godbolt.org/z/jvYE3cTdq

The capture is the prob

[PATCH] D149133: [clang][Interp] BaseToDerived casts

2023-04-25 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

  We can implement these similarly to DerivedToBase casts. We just have to
  walk the class hierarchy, sum the base offsets and subtract it from the
  current base offset of the pointer.

As a side-effect, this also changes the `BaseToDerived` casts to only emit 
//one// opcode instead of one per base cast.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149133

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Opcodes.td
  clang/lib/AST/Interp/Pointer.h
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -586,6 +586,58 @@
   static_assert(test() == 1);
 }
 
+namespace BaseToDerived {
+namespace A {
+  struct A {};
+  struct B : A { int n; };
+  struct C : B {};
+  C c = {};
+  constexpr C *pb = (C*)((A*)&c + 1); // expected-error {{must be initialized by a constant expression}} \
+  // expected-note {{cannot access derived class of pointer past the end of object}} \
+  // ref-error {{must be initialized by a constant expression}} \
+  // ref-note {{cannot access derived class of pointer past the end of object}}
+}
+namespace B {
+  struct A {};
+  struct Z {};
+  struct B : Z, A {
+int n;
+   constexpr B() : n(10) {}
+  };
+  struct C : B {
+   constexpr C() : B() {}
+  };
+
+  constexpr C c = {};
+  constexpr const A *pa = &c;
+  constexpr const C *cp = (C*)pa;
+  constexpr const B *cb = (B*)cp;
+
+  static_assert(cb->n == 10);
+  static_assert(cp->n == 10);
+}
+
+namespace C {
+  struct Base { int *a; };
+  struct Base2 : Base { int f[12]; };
+
+  struct Middle1 { int b[3]; };
+  struct Middle2 : Base2 { char c; };
+  struct Middle3 : Middle2 { char g[3]; };
+  struct Middle4 { int f[3]; };
+  struct Middle5 : Middle4, Middle3 { char g2[3]; };
+
+  struct NotQuiteDerived : Middle1, Middle5 { bool d; };
+  struct Derived : NotQuiteDerived { int e; };
+
+  constexpr NotQuiteDerived NQD1 = {};
+
+  constexpr Middle5 *M4 = (Middle5*)((Base2*)&NQD1);
+  static_assert(M4->a == nullptr);
+  static_assert(M4->g2[0] == 0);
+}
+}
+
 
 namespace VirtualDtors {
   class A {
Index: clang/lib/AST/Interp/Pointer.h
===
--- clang/lib/AST/Interp/Pointer.h
+++ clang/lib/AST/Interp/Pointer.h
@@ -100,6 +100,14 @@
 return Pointer(Pointee, Field, Field);
   }
 
+  /// Subtract the given offset from the current Base and Offset
+  /// of the pointer.
+  Pointer atFieldSub(unsigned Off) const {
+assert(Offset >= Off);
+unsigned O = Offset - Off;
+return Pointer(Pointee, O, O);
+  }
+
   /// Restricts the scope of an array element pointer.
   Pointer narrow() const {
 // Null pointers cannot be narrowed.
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -291,6 +291,10 @@
   let Args = [ArgUint32];
 }
 
+def GetPtrDerivedPop : Opcode {
+  let Args = [ArgUint32];
+}
+
 // [Pointer] -> [Pointer]
 def GetPtrVirtBase : Opcode {
   // RecordDecl of base class.
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -67,8 +67,9 @@
 bool CheckRange(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
 CheckSubobjectKind CSK);
 
-/// Checks if accessing a base of the given pointer is valid.
-bool CheckBase(InterpState &S, CodePtr OpPC, const Pointer &Ptr);
+/// Checks if accessing a base or derived record of the given pointer is valid.
+bool CheckBaseDerived(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
+  CheckSubobjectKind CSK);
 
 /// Checks if a pointer points to const storage.
 bool CheckConst(InterpState &S, CodePtr OpPC, const Pointer &Ptr);
@@ -1078,11 +1079,25 @@
   return true;
 }
 
+inline bool GetPtrDerivedPop(InterpState &S, CodePtr OpPC, uint32_t Off) {
+  const Pointer &Ptr = S.Stk.pop();
+
+  if (!CheckNull(S, OpPC, Ptr, CSK_Derived))
+return false;
+  if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Derived))
+return false;
+
+  llvm::errs() << "Derived Offset: " << Off << ". Ptr: " << Ptr << "\n";
+
+  S.Stk.push(Ptr.atFieldSub(Off));
+  return true;
+}
+
 inline bool GetPtrBase(InterpState &S, CodePtr OpPC, uint32_t Off) {
   const Pointer &Ptr = S.Stk.peek();
   if (!CheckNull(S, OpPC,

[PATCH] D149133: [clang][Interp] BaseToDerived casts

2023-04-25 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 516701.

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

https://reviews.llvm.org/D149133

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Opcodes.td
  clang/lib/AST/Interp/Pointer.h
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -586,6 +586,58 @@
   static_assert(test() == 1);
 }
 
+namespace BaseToDerived {
+namespace A {
+  struct A {};
+  struct B : A { int n; };
+  struct C : B {};
+  C c = {};
+  constexpr C *pb = (C*)((A*)&c + 1); // expected-error {{must be initialized by a constant expression}} \
+  // expected-note {{cannot access derived class of pointer past the end of object}} \
+  // ref-error {{must be initialized by a constant expression}} \
+  // ref-note {{cannot access derived class of pointer past the end of object}}
+}
+namespace B {
+  struct A {};
+  struct Z {};
+  struct B : Z, A {
+int n;
+   constexpr B() : n(10) {}
+  };
+  struct C : B {
+   constexpr C() : B() {}
+  };
+
+  constexpr C c = {};
+  constexpr const A *pa = &c;
+  constexpr const C *cp = (C*)pa;
+  constexpr const B *cb = (B*)cp;
+
+  static_assert(cb->n == 10);
+  static_assert(cp->n == 10);
+}
+
+namespace C {
+  struct Base { int *a; };
+  struct Base2 : Base { int f[12]; };
+
+  struct Middle1 { int b[3]; };
+  struct Middle2 : Base2 { char c; };
+  struct Middle3 : Middle2 { char g[3]; };
+  struct Middle4 { int f[3]; };
+  struct Middle5 : Middle4, Middle3 { char g2[3]; };
+
+  struct NotQuiteDerived : Middle1, Middle5 { bool d; };
+  struct Derived : NotQuiteDerived { int e; };
+
+  constexpr NotQuiteDerived NQD1 = {};
+
+  constexpr Middle5 *M4 = (Middle5*)((Base2*)&NQD1);
+  static_assert(M4->a == nullptr);
+  static_assert(M4->g2[0] == 0);
+}
+}
+
 
 namespace VirtualDtors {
   class A {
Index: clang/lib/AST/Interp/Pointer.h
===
--- clang/lib/AST/Interp/Pointer.h
+++ clang/lib/AST/Interp/Pointer.h
@@ -100,6 +100,14 @@
 return Pointer(Pointee, Field, Field);
   }
 
+  /// Subtract the given offset from the current Base and Offset
+  /// of the pointer.
+  Pointer atFieldSub(unsigned Off) const {
+assert(Offset >= Off);
+unsigned O = Offset - Off;
+return Pointer(Pointee, O, O);
+  }
+
   /// Restricts the scope of an array element pointer.
   Pointer narrow() const {
 // Null pointers cannot be narrowed.
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -291,6 +291,10 @@
   let Args = [ArgUint32];
 }
 
+def GetPtrDerivedPop : Opcode {
+  let Args = [ArgUint32];
+}
+
 // [Pointer] -> [Pointer]
 def GetPtrVirtBase : Opcode {
   // RecordDecl of base class.
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -67,8 +67,9 @@
 bool CheckRange(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
 CheckSubobjectKind CSK);
 
-/// Checks if accessing a base of the given pointer is valid.
-bool CheckBase(InterpState &S, CodePtr OpPC, const Pointer &Ptr);
+/// Checks if accessing a base or derived record of the given pointer is valid.
+bool CheckBaseDerived(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
+  CheckSubobjectKind CSK);
 
 /// Checks if a pointer points to const storage.
 bool CheckConst(InterpState &S, CodePtr OpPC, const Pointer &Ptr);
@@ -1078,11 +1079,21 @@
   return true;
 }
 
+inline bool GetPtrDerivedPop(InterpState &S, CodePtr OpPC, uint32_t Off) {
+  const Pointer &Ptr = S.Stk.pop();
+  if (!CheckNull(S, OpPC, Ptr, CSK_Derived))
+return false;
+  if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Derived))
+return false;
+  S.Stk.push(Ptr.atFieldSub(Off));
+  return true;
+}
+
 inline bool GetPtrBase(InterpState &S, CodePtr OpPC, uint32_t Off) {
   const Pointer &Ptr = S.Stk.peek();
   if (!CheckNull(S, OpPC, Ptr, CSK_Base))
 return false;
-  if (!CheckBase(S, OpPC, Ptr))
+  if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Base))
 return false;
   S.Stk.push(Ptr.atField(Off));
   return true;
@@ -1092,7 +1103,7 @@
   const Pointer &Ptr = S.Stk.pop();
   if (!CheckNull(S, OpPC, Ptr, CSK_Base))
 return false;
-  if (!CheckBase(S, OpPC, Ptr))
+  if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Base))
 return false;
   S.Stk.push(Ptr.atField(Off));
   return true;
Index: clang/lib/AST/Interp/Interp.cpp
==

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

2023-04-25 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 516702.
hokein added a comment.

address review comments, added one more test for command code-actions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148783

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

Index: clang-tools-extra/clangd/test/fixits-embed-in-diagnostic.test
===
--- clang-tools-extra/clangd/test/fixits-embed-in-diagnostic.test
+++ clang-tools-extra/clangd/test/fixits-embed-in-diagnostic.test
@@ -48,6 +48,7 @@
 # CHECK-NEXT:"source": "clang"
 # CHECK-NEXT:  },
 # CHECK-NEXT:  {
+# CHECK-NEXT:"codeActions": [],
 # CHECK-NEXT:"message": "Previous use is here\n\nfoo.c:1:18: error: use of 'Point' with tag type that does not match previous declaration",
 # CHECK-NEXT:"range": {
 # CHECK-NEXT:  "end": {
Index: clang-tools-extra/clangd/test/fixits-command-documentchanges.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/fixits-command-documentchanges.test
@@ -0,0 +1,194 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"workspace":{"workspaceEdit":{"documentChanges":true}}},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","text":"int main(int i, char **a) { if (i = 2) {}}"}}}
+#  CHECK:"method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:"diagnostics": [
+# CHECK-NEXT:  {
+# CHECK-NEXT:"code": "-Wparentheses",
+# CHECK-NEXT:"message": "Using the result of an assignment as a condition without parentheses (fixes available)",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 37,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 32,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"severity": 2,
+# CHECK-NEXT:"source": "clang"
+# CHECK-NEXT:  }
+# CHECK-NEXT:],
+# CHECK-NEXT:"uri": "file://{{.*}}/foo.c",
+# CHECK-NEXT:"version": 0
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":0,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"severity":2,"message":"Using the result of an assignment as a condition without parentheses (fixes available)"}]}}}
+#  CHECK:  "id": 2,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "arguments": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "documentChanges": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "edits": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "newText": "(",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 32,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 32,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:{
+# CHECK-NEXT:  "newText": ")",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 37,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 37,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  "textDocument": {
+# CHECK-NEXT:"uri": "file:///clangd-test/foo.c",
+# CHECK-NEXT:"version": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  ]
+# CHECK-NEXT:}
+

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

2023-04-25 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1746
+
+ if (DiagOpts.EmbedFixesInDiagnostics && !CodeActions.empty()) 
{
+   Diag.codeActions.emplace(CodeActions);

kadircet wrote:
> we didn't have the empty check before, let's not introduce it now (i.e. we'll 
> still reply with an empty set of code actions rather than "none" when there 
> are no fixes known to clangd)
yeah, I added this empty check to keep `fixits-embed-in-diagnostic.test` 
unchanged (we don't emit code-action for diagnostic notes), but I think it is 
fair enough to change it.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1751-1753
  auto &FixItsForDiagnostic = LocalFixIts[Diag];
- llvm::copy(Fixes, std::back_inserter(FixItsForDiagnostic));
+ llvm::move(std::move(CodeActions),
+std::back_inserter(FixItsForDiagnostic));

kadircet wrote:
> i am not sure why this logic was appending to the vector rather than just 
> overwriting. but we shouldn't receive the same diagnostics from the callback 
> here, am i missing something? so what about just:
> ```
> LocalFixIts[Diag] = std::move(CodeActions);
> ```
Agreed, changed to overwriting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148783

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


[PATCH] D146269: MIPS: allow o32 abi with 64bit CPU and 64 abi with 32bit triple

2023-04-25 Thread YunQiang Su via Phabricator via cfe-commits
wzssyqa added a comment.

@MaskRay can you help to submit this patch?


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

https://reviews.llvm.org/D146269

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


[PATCH] D148066: [RISCV] Add Smaia and Ssaia extensions support

2023-04-25 Thread Kito Cheng via Phabricator via cfe-commits
kito-cheng added a comment.

> My concern would be that as we don't gate CSR names on enabling the relevant 
> extension, people could start using CSR names and encodings that could 
> change, without opting in via -menable-experimental-extensions, perhaps not 
> realising that they're using the unratified version. OTOH, you could argue it 
> was user error from the start by not trying to specify all the needed 
> extensions in the ISA naming string.

We decide don't gate CSR before, but I am wondering maybe we should gate those 
CSR if they are defined by a unratified/experimental ext., and remove the 
checking once it ratified, since it might change the name or CSR number before 
ratified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148066

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


[PATCH] D146054: [RISCV] Add --print-supported-extensions and -march=help support

2023-04-25 Thread Brandon Wu via Phabricator via cfe-commits
4vtomat updated this revision to Diff 516708.
4vtomat added a comment.

Rename clang/test/Driver/print-supported-marchs.c to 
clang/test/Driver/print-supported-extensions.c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/print-supported-extensions.c
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/RISCVISAInfo.h
  llvm/lib/Support/RISCVISAInfo.cpp

Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -139,6 +139,29 @@
 {"ztso", RISCVExtensionVersion{0, 1}},
 };
 
+void llvm::riscvMarchHelp() {
+  errs() << "All available -march extensions for RISC-V\n\n";
+  errs() << '\t' << left_justify("Name", 20) << "Version\n";
+
+  RISCVISAInfo::OrderedExtensionMap ExtMap;
+  for (const auto &E : SupportedExtensions)
+ExtMap[E.Name] = { E.Name, E.Version.Major, E.Version.Minor };
+  for (const auto &E : ExtMap)
+errs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+ E.second.MinorVersion);
+
+  errs() << "\nExperimental extensions\n";
+  ExtMap.clear();
+  for (const auto &E : SupportedExperimentalExtensions)
+ExtMap[E.Name] = { E.Name, E.Version.Major, E.Version.Minor };
+  for (const auto &E : ExtMap)
+errs() << format("\t%-20s%d.%d\n", E.first.c_str(), E.second.MajorVersion,
+ E.second.MinorVersion);
+
+  errs() << "\nUse -march to specify the target's extension.\n"
+"For example, clang -march=rv32i_v1p0\n";
+}
+
 static bool stripExperimentalPrefix(StringRef &Ext) {
   return Ext.consume_front("experimental-");
 }
Index: llvm/include/llvm/Support/RISCVISAInfo.h
===
--- llvm/include/llvm/Support/RISCVISAInfo.h
+++ llvm/include/llvm/Support/RISCVISAInfo.h
@@ -23,6 +23,8 @@
   unsigned MinorVersion;
 };
 
+void riscvMarchHelp();
+
 class RISCVISAInfo {
 public:
   RISCVISAInfo(const RISCVISAInfo &) = delete;
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -38,6 +38,7 @@
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
+#include "llvm/Support/RISCVISAInfo.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/TimeProfiler.h"
@@ -182,6 +183,14 @@
   return 0;
 }
 
+/// Print supported extensions of the given target.
+static int PrintSupportedExtensions(std::string TargetStr) {
+  llvm::riscvMarchHelp();
+
+  return 0;
+}
+
+
 int cc1_main(ArrayRef Argv, const char *Argv0, void *MainAddr) {
   ensureSufficientStack();
 
@@ -223,6 +232,10 @@
   if (Clang->getFrontendOpts().PrintSupportedCPUs)
 return PrintSupportedCPUs(Clang->getTargetOpts().Triple);
 
+  // --print-supported-extensions takes priority over the actual compilation.
+  if (Clang->getFrontendOpts().PrintSupportedExtensions)
+return PrintSupportedExtensions(Clang->getTargetOpts().Triple);
+
   // Infer the builtin include path if unspecified.
   if (Clang->getHeaderSearchOpts().UseBuiltinIncludes &&
   Clang->getHeaderSearchOpts().ResourceDir.empty())
Index: clang/test/Driver/print-supported-extensions.c
===
--- /dev/null
+++ clang/test/Driver/print-supported-extensions.c
@@ -0,0 +1,98 @@
+// Test that --print-supported-extensions lists supported extensions.
+
+// REQUIRES: riscv-registered-target
+
+// RUN: %clang --target=riscv64 --print-supported-extensions 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-RISCV
+
+// Test -march=help alias.
+// RUN: %clang --target=riscv64 -march=help 2>&1 | \
+// RUN:   FileCheck %s --check-prefix=CHECK-RISCV
+
+// CHECK-NOT: warning: argument unused during compilation
+// CHECK-RISCV: Target: riscv64
+// CHECK-NEXT: All available -march extensions for RISC-V
+// CHECK-NEXT: 	NameVersion
+// CHECK-NEXT: 	i   2.0
+// CHECK-NEXT: 	e   1.9
+// CHECK-NEXT: 	m   2.0
+// CHECK-NEXT: 	a   2.0
+// CHECK-NEXT: 	f   2.0
+// CHECK-NEXT: 	d   2.0
+// CHECK-NEXT: 	c   2.0
+// CHECK-NEXT: 	v   1.0
+// CHECK-NEXT: 	h   1.0
+// CHECK-NEXT: 	svinval 1.0
+// CHECK-NEXT: 	svnapot 1.0
+// CHECK-NEXT: 	svpbmt  1.0
+// CHECK-NEXT: 	zicbom  1.0
+// CHECK-NEXT: 	zicbop  1.0
+// CHECK-NEXT: 	zicboz  1.0
+// CHECK-NEXT: 	zicsr   2.0
+// CHECK-NEXT: 	zifence

[PATCH] D148992: [clang-repl] Fix dynamic library test to avoid cstdio and linker

2023-04-25 Thread Stefan Gränitz via Phabricator via cfe-commits
sgraenitz added inline comments.



Comment at: clang/test/Interpreter/dynamic-library.cpp:12
+//   return 5;
+// }
 

Should we wrap this in a `extern "C"` block? Otherwise, the shared library has 
a C++ interface, which is not stable. It won't matter in this simple case, but 
the test might become a pattern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148992

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


[PATCH] D148992: [clang-repl] Fix dynamic library test to avoid cstdio and linker

2023-04-25 Thread Anubhab Ghosh via Phabricator via cfe-commits
argentite updated this revision to Diff 516715.
argentite marked an inline comment as done.
argentite added a comment.

extern "C" and enable PS4/5


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148992

Files:
  clang/test/Interpreter/Inputs/dynamic-library-test.cpp
  clang/test/Interpreter/Inputs/libdynamic-library-test.so
  clang/test/Interpreter/dynamic-library.cpp


Index: clang/test/Interpreter/dynamic-library.cpp
===
--- clang/test/Interpreter/dynamic-library.cpp
+++ clang/test/Interpreter/dynamic-library.cpp
@@ -1,13 +1,25 @@
 // REQUIRES: host-supports-jit, system-linux
-// UNSUPPORTED: target={{.*-(ps4|ps5)}}
 
-// RUN: %clang -xc++ -o %T/libdynamic-library-test.so -fPIC -shared -DLIBRARY 
%S/Inputs/dynamic-library-test.cpp
-// RUN: cat %s | env LD_LIBRARY_PATH=%T:$LD_LIBRARY_PATH clang-repl | 
FileCheck %s
+// To generate libdynamic-library-test.so :
+// clang -xc++ -o libdynamic-library-test.so -fPIC -shared
+//
+// extern "C" {
+//
+// int ultimate_answer = 0;
+// 
+// int calculate_answer() {
+//   ultimate_answer = 42;
+//   return 5;
+// }
+//
+// }
 
-#include 
+// RUN: cat %s | env LD_LIBRARY_PATH=%S/Inputs:$LD_LIBRARY_PATH clang-repl | 
FileCheck %s
 
-extern int ultimate_answer;
-int calculate_answer();
+extern "C" int printf(const char* format, ...);
+
+extern "C" int ultimate_answer;
+extern "C" int calculate_answer();
 
 %lib libdynamic-library-test.so
 
Index: clang/test/Interpreter/Inputs/dynamic-library-test.cpp
===
--- clang/test/Interpreter/Inputs/dynamic-library-test.cpp
+++ /dev/null
@@ -1,6 +0,0 @@
-int ultimate_answer = 0;
-
-int calculate_answer() {
-  ultimate_answer = 42;
-  return 5;
-}


Index: clang/test/Interpreter/dynamic-library.cpp
===
--- clang/test/Interpreter/dynamic-library.cpp
+++ clang/test/Interpreter/dynamic-library.cpp
@@ -1,13 +1,25 @@
 // REQUIRES: host-supports-jit, system-linux
-// UNSUPPORTED: target={{.*-(ps4|ps5)}}
 
-// RUN: %clang -xc++ -o %T/libdynamic-library-test.so -fPIC -shared -DLIBRARY %S/Inputs/dynamic-library-test.cpp
-// RUN: cat %s | env LD_LIBRARY_PATH=%T:$LD_LIBRARY_PATH clang-repl | FileCheck %s
+// To generate libdynamic-library-test.so :
+// clang -xc++ -o libdynamic-library-test.so -fPIC -shared
+//
+// extern "C" {
+//
+// int ultimate_answer = 0;
+// 
+// int calculate_answer() {
+//   ultimate_answer = 42;
+//   return 5;
+// }
+//
+// }
 
-#include 
+// RUN: cat %s | env LD_LIBRARY_PATH=%S/Inputs:$LD_LIBRARY_PATH clang-repl | FileCheck %s
 
-extern int ultimate_answer;
-int calculate_answer();
+extern "C" int printf(const char* format, ...);
+
+extern "C" int ultimate_answer;
+extern "C" int calculate_answer();
 
 %lib libdynamic-library-test.so
 
Index: clang/test/Interpreter/Inputs/dynamic-library-test.cpp
===
--- clang/test/Interpreter/Inputs/dynamic-library-test.cpp
+++ /dev/null
@@ -1,6 +0,0 @@
-int ultimate_answer = 0;
-
-int calculate_answer() {
-  ultimate_answer = 42;
-  return 5;
-}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2023-04-25 Thread Orlando Cazalet-Hyams via Phabricator via cfe-commits
Orlando added a comment.

Thanks everyone for your reports and help, it's really appreciated.

In D146987#4286318 , @dmgreen wrote:

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

You're right there. Fixed the assertion firing in D149137 
, but the behaviour isn't quite right yet so 
I've opened a bug for it here llvm.org/PR62346 .

---

@mstorsjo and @srj, the issue with the faulty assertion in a std::sort 
predicate is fixed with D149045 .

---

I found an additional assertion failure on the chromium build page (for 
pigweed) which I've fixed in D149135 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146987

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


[PATCH] D133863: [RISCV] Add MC support of RISCV zcmt Extension

2023-04-25 Thread Xinlong Wu via Phabricator via cfe-commits
VincentWu updated this revision to Diff 516718.
VincentWu marked 2 inline comments as done.
VincentWu added a comment.

rebase & address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133863

Files:
  clang/test/Preprocessor/riscv-target-features.c
  llvm/docs/RISCVUsage.rst
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/RISCVFeatures.td
  llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
  llvm/lib/Target/RISCV/RISCVSchedRocket.td
  llvm/lib/Target/RISCV/RISCVSchedSiFive7.td
  llvm/lib/Target/RISCV/RISCVSystemOperands.td
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/rv32zcmt-invalid.s
  llvm/test/MC/RISCV/rv32zcmt-valid.s
  llvm/test/MC/RISCV/rvzcmt-user-csr-name.s

Index: llvm/test/MC/RISCV/rvzcmt-user-csr-name.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rvzcmt-user-csr-name.s
@@ -0,0 +1,29 @@
+# RUN: llvm-mc %s -triple=riscv32 -riscv-no-aliases -mattr=+experimental-zcmt -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-INST,CHECK-ENC %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+experimental-zcmt < %s \
+# RUN: | llvm-objdump -d --mattr=+experimental-zcmt - \
+# RUN: | FileCheck -check-prefix=CHECK-INST-ALIAS %s
+#
+# RUN: llvm-mc %s -triple=riscv64 -riscv-no-aliases -mattr=+experimental-zcmt -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-INST,CHECK-ENC %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+experimental-zcmt < %s \
+# RUN: | llvm-objdump -d --mattr=+experimental-zcmt - \
+# RUN: | FileCheck -check-prefix=CHECK-INST-ALIAS %s
+
+##
+# Jump Vector Table CSR
+##
+
+# jvt
+# name
+# CHECK-INST: csrrs t1, jvt, zero
+# CHECK-ENC:  encoding: [0x73,0x23,0x70,0x01]
+# CHECK-INST-ALIAS: csrr t1, jvt
+# uimm12
+# CHECK-INST: csrrs t2, jvt, zero
+# CHECK-ENC:  encoding: [0xf3,0x23,0x70,0x01]
+# CHECK-INST-ALIAS: csrr t2, jvt
+# name
+csrrs t1, jvt, zero
+# uimm12
+csrrs t2, 0x017, zero
Index: llvm/test/MC/RISCV/rv32zcmt-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rv32zcmt-valid.s
@@ -0,0 +1,39 @@
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+experimental-zcmt\
+# RUN:  -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=+experimental-zcmt\
+# RUN:  -mattr=m < %s \
+# RUN: | llvm-objdump --mattr=+experimental-zcmt\
+# RUN:  -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefixes=CHECK-OBJ,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+experimental-zcmt\
+# RUN:  -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+experimental-zcmt\
+# RUN:  -mattr=m < %s \
+# RUN: | llvm-objdump --mattr=+experimental-zcmt\
+# RUN:  -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefixes=CHECK-OBJ,CHECK-ASM-AND-OBJ %s
+#
+# RUN: not llvm-mc -triple riscv32 \
+# RUN: -riscv-no-aliases -show-encoding < %s 2>&1 \
+# RUN: | FileCheck -check-prefixes=CHECK-NO-EXT %s
+# RUN: not llvm-mc -triple riscv64 \
+# RUN: -riscv-no-aliases -show-encoding < %s 2>&1 \
+# RUN: | FileCheck -check-prefixes=CHECK-NO-EXT %s
+
+# CHECK-ASM-AND-OBJ: cm.jt 1
+# CHECK-ASM: encoding: [0x06,0xa0]
+# CHECK-NO-EXT: error: instruction requires the following: 'Zcmt' (table jump instuctions for code-size reduction){{$}}
+cm.jt 1
+
+# CHECK-ASM: cm.jalt 1
+# CHECK-OBJ: cm.jt 1
+# CHECK-ASM: encoding: [0x06,0xa0]
+# CHECK-NO-EXT: error: instruction requires the following: 'Zcmt' (table jump instuctions for code-size reduction){{$}}
+cm.jalt 1
+
+# CHECK-ASM-AND-OBJ: cm.jalt 32
+# CHECK-ASM: encoding: [0x82,0xa0]
+# CHECK-NO-EXT: error: instruction requires the following: 'Zcmt' (table jump instuctions for code-size reduction){{$}}
+cm.jalt 32
Index: llvm/test/MC/RISCV/rv32zcmt-invalid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rv32zcmt-invalid.s
@@ -0,0 +1,10 @@
+# RUN: not llvm-mc -triple=riscv32 -mattr=+experimental-zcmt -riscv-no-aliases -show-encoding < %s 2>&1 \
+# RUN: | FileCheck -check-prefixes=CHECK-ERROR %s
+# RUN: not llvm-mc -triple=riscv64 -mattr=+experimental-zcmt -riscv-no-aliases -show-encoding < %s 2>&1 \
+# RUN: | FileCheck -check-prefixes=CHECK-ERROR %s
+
+# CHECK-ERROR: error: immediate must be an integer in the range [0, 31]
+cm.jt 64
+
+# CHECK-ERROR: error: immediate must be an integer in the range [0, 255]
+cm.jalt 256
Index: llvm/test/MC/RISCV/attribute-arch.s
===
--- llvm/test/MC/RISCV/attribute-arch.s
+++ llvm/test/MC/RISCV/att

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

2023-04-25 Thread Jeremy Morse via Phabricator via cfe-commits
jmorse added a comment.

Given our repeated back-and-forth, it's probably better to do some pre-testing: 
@MaskRay @paulkirth we've got a command line switch for enabling this, are 
there any fuchsia / chromium facilities for pull-request-builds that we'd be 
able to use to pre-check this; or if those aren't available would you have any 
time to fire off a build with this flag enabled to see if there's further 
trouble? Apologies for the burden, the end reward is sounder and more complete 
variable locations when debugging though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146987

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


[PATCH] D133863: [RISCV] Add MC support of RISCV zcmt Extension

2023-04-25 Thread Xinlong Wu via Phabricator via cfe-commits
VincentWu updated this revision to Diff 516722.
VincentWu added a comment.

paraphrase grammar of error msg


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133863

Files:
  clang/test/Preprocessor/riscv-target-features.c
  llvm/docs/RISCVUsage.rst
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/RISCVFeatures.td
  llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
  llvm/lib/Target/RISCV/RISCVSchedRocket.td
  llvm/lib/Target/RISCV/RISCVSchedSiFive7.td
  llvm/lib/Target/RISCV/RISCVSystemOperands.td
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/rv32zcmt-invalid.s
  llvm/test/MC/RISCV/rv32zcmt-valid.s
  llvm/test/MC/RISCV/rvzcmt-user-csr-name.s

Index: llvm/test/MC/RISCV/rvzcmt-user-csr-name.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rvzcmt-user-csr-name.s
@@ -0,0 +1,29 @@
+# RUN: llvm-mc %s -triple=riscv32 -riscv-no-aliases -mattr=+experimental-zcmt -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-INST,CHECK-ENC %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+experimental-zcmt < %s \
+# RUN: | llvm-objdump -d --mattr=+experimental-zcmt - \
+# RUN: | FileCheck -check-prefix=CHECK-INST-ALIAS %s
+#
+# RUN: llvm-mc %s -triple=riscv64 -riscv-no-aliases -mattr=+experimental-zcmt -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-INST,CHECK-ENC %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+experimental-zcmt < %s \
+# RUN: | llvm-objdump -d --mattr=+experimental-zcmt - \
+# RUN: | FileCheck -check-prefix=CHECK-INST-ALIAS %s
+
+##
+# Jump Vector Table CSR
+##
+
+# jvt
+# name
+# CHECK-INST: csrrs t1, jvt, zero
+# CHECK-ENC:  encoding: [0x73,0x23,0x70,0x01]
+# CHECK-INST-ALIAS: csrr t1, jvt
+# uimm12
+# CHECK-INST: csrrs t2, jvt, zero
+# CHECK-ENC:  encoding: [0xf3,0x23,0x70,0x01]
+# CHECK-INST-ALIAS: csrr t2, jvt
+# name
+csrrs t1, jvt, zero
+# uimm12
+csrrs t2, 0x017, zero
Index: llvm/test/MC/RISCV/rv32zcmt-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rv32zcmt-valid.s
@@ -0,0 +1,39 @@
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+experimental-zcmt\
+# RUN:  -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=+experimental-zcmt\
+# RUN:  -mattr=m < %s \
+# RUN: | llvm-objdump --mattr=+experimental-zcmt\
+# RUN:  -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefixes=CHECK-OBJ,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+experimental-zcmt\
+# RUN:  -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+experimental-zcmt\
+# RUN:  -mattr=m < %s \
+# RUN: | llvm-objdump --mattr=+experimental-zcmt\
+# RUN:  -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefixes=CHECK-OBJ,CHECK-ASM-AND-OBJ %s
+#
+# RUN: not llvm-mc -triple riscv32 \
+# RUN: -riscv-no-aliases -show-encoding < %s 2>&1 \
+# RUN: | FileCheck -check-prefixes=CHECK-NO-EXT %s
+# RUN: not llvm-mc -triple riscv64 \
+# RUN: -riscv-no-aliases -show-encoding < %s 2>&1 \
+# RUN: | FileCheck -check-prefixes=CHECK-NO-EXT %s
+
+# CHECK-ASM-AND-OBJ: cm.jt 1
+# CHECK-ASM: encoding: [0x06,0xa0]
+# CHECK-NO-EXT: error: instruction requires the following: 'Zcmt' (table jump instuctions for code-size reduction){{$}}
+cm.jt 1
+
+# CHECK-ASM: cm.jalt 1
+# CHECK-OBJ: cm.jt 1
+# CHECK-ASM: encoding: [0x06,0xa0]
+# CHECK-NO-EXT: error: instruction requires the following: 'Zcmt' (table jump instuctions for code-size reduction){{$}}
+cm.jalt 1
+
+# CHECK-ASM-AND-OBJ: cm.jalt 32
+# CHECK-ASM: encoding: [0x82,0xa0]
+# CHECK-NO-EXT: error: instruction requires the following: 'Zcmt' (table jump instuctions for code-size reduction){{$}}
+cm.jalt 32
Index: llvm/test/MC/RISCV/rv32zcmt-invalid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rv32zcmt-invalid.s
@@ -0,0 +1,10 @@
+# RUN: not llvm-mc -triple=riscv32 -mattr=+experimental-zcmt -riscv-no-aliases -show-encoding < %s 2>&1 \
+# RUN: | FileCheck -check-prefixes=CHECK-ERROR %s
+# RUN: not llvm-mc -triple=riscv64 -mattr=+experimental-zcmt -riscv-no-aliases -show-encoding < %s 2>&1 \
+# RUN: | FileCheck -check-prefixes=CHECK-ERROR %s
+
+# CHECK-ERROR: error: immediate must be an integer in the range [0, 31]
+cm.jt 64
+
+# CHECK-ERROR: error: immediate must be an integer in the range [0, 255]
+cm.jalt 256
Index: llvm/test/MC/RISCV/attribute-arch.s
===
--- llvm/test/MC/RISCV/attribute-arch.s
+++ llvm/test/MC/RISCV/attribute-arch.s
@@ -231,6 +231,9 @@
 .at

[PATCH] D149119: [CMake] Use llvm-nm to extract symbols for staged LTO builds on Windows

2023-04-25 Thread Tomas Matheson via Phabricator via cfe-commits
tmatheson added a comment.

This looks like a nice addition. Would it make sense to use llvm-nm always, not 
restricted to bootstrap builds? And would that work on Windows and allow us to 
simplify this script substantially by using one tool for all platforms?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149119

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


[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

2023-04-25 Thread Ivan Murashko via Phabricator via cfe-commits
ivanmurashko added a comment.

@arphaman, @andrewjcg, what's the status of the diff?
The functionality is important for me and I want to get it landed.

BTW: @andrewjcg I can commander the diff if you don't mind.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

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


[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

2023-04-25 Thread Sergio Afonso via Phabricator via cfe-commits
skatrak updated this revision to Diff 516729.
skatrak added a comment.
Herald added subscribers: llvm-commits, cfe-commits, bviyer, Moerafaat, 
zero9178, awarzynski, sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, 
tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, 
mgester, arpith-jacob, csigg, nicolasvasilache, antiagainst, shauheen, thopre, 
hiraditya.
Herald added a reviewer: nicolasvasilache.
Herald added a reviewer: ftynse.
Herald added projects: clang, MLIR, LLVM.

Try to fix issue with applying patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/lib/Semantics/CMakeLists.txt
  flang/lib/Semantics/finalize-omp.cpp
  flang/lib/Semantics/finalize-omp.h
  flang/lib/Semantics/semantics.cpp
  flang/test/Lower/OpenMP/Todo/omp-declare-target.f90
  flang/test/Lower/OpenMP/omp-declare-target.f90
  flang/test/Lower/OpenMP/requires-error.f90
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90
  flang/test/Semantics/OpenMP/declare-target-implicit-capture-rewrite.f90
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
  mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.h
  mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
  mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td
  mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
  mlir/lib/Target/LLVMIR/CMakeLists.txt
  mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
  mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
  mlir/test/Dialect/OpenMP/attr.mlir
  mlir/test/Target/LLVMIR/openmp-llvm.mlir

Index: mlir/test/Target/LLVMIR/openmp-llvm.mlir
===
--- mlir/test/Target/LLVMIR/openmp-llvm.mlir
+++ mlir/test/Target/LLVMIR/openmp-llvm.mlir
@@ -2537,3 +2537,51 @@
 // CHECK: @__omp_rtl_assume_no_nested_parallelism = weak_odr hidden constant i32 0
 module attributes {omp.flags = #omp.flags, 
omp.is_device = #omp.isdevice} {}
+
+// -
+
+// Check that the atomic default memory order is picked up by atomic operations.
+module attributes {
+  omp.atomic_default_mem_order = #omp
+} {
+  // CHECK-LABEL: @omp_atomic_default_mem_order
+  // CHECK-SAME: (ptr %[[ARG0:.*]], ptr %[[ARG1:.*]], i32 %[[EXPR:.*]])
+  llvm.func @omp_atomic_default_mem_order(%arg0 : !llvm.ptr,
+  %arg1 : !llvm.ptr,
+  %expr : i32) -> () {
+
+// CHECK: %[[X1:.*]] = load atomic i32, ptr %[[ARG0]] seq_cst, align 4
+// CHECK: store i32 %[[X1]], ptr %[[ARG1]], align 4
+omp.atomic.read %arg1 = %arg0 : !llvm.ptr, i32
+
+// CHECK: store atomic i32 %[[EXPR]], ptr %[[ARG1]] seq_cst, align 4
+// CHECK: call void @__kmpc_flush(ptr @{{.*}})
+omp.atomic.write %arg1 = %expr : !llvm.ptr, i32
+
+// CHECK: atomicrmw add ptr %[[ARG1]], i32 %[[EXPR]] seq_cst
+omp.atomic.update %arg1 : !llvm.ptr {
+^bb0(%xval: i32):
+  %newval = llvm.add %xval, %expr : i32
+  omp.yield(%newval : i32)
+}
+
+// CHECK: %[[xval:.*]] = atomicrmw xchg ptr %[[ARG0]], i32 %[[EXPR]] seq_cst
+// CHECK: store i32 %[[xval]], ptr %[[ARG1]]
+omp.atomic.capture {
+  omp.atomic.read %arg1 = %arg0 : !llvm.ptr, i32
+  omp.atomic.write %arg0 = %expr : !llvm.ptr, i32
+}
+
+llvm.return
+  }
+}
+
+// -
+
+// Check that OpenMP requires flags are registered by a global constructor.
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, ptr, ptr }]
+// CHECK-SAME: [{ i32, ptr, ptr } { i32 0, ptr @[[REG_FN:.*]], ptr null }]
+// CHECK: define {{.*}} @[[REG_FN]]({{.*}})
+// CHECK-NOT: }
+// CHECK:   call void @__tgt_register_requires(i64 10)
+module attributes {omp.requires = #omp} {}
Index: mlir/test/Dialect/OpenMP/attr.mlir
===
--- mlir/test/Dialect/OpenMP/attr.mlir
+++ mlir/test/Dialect/OpenMP/attr.mlir
@@ -29,3 +29,23 @@
 
 // CHECK: module attributes {omp.flags = #omp.flags} {
 module attributes {omp.flags = #omp.flags} {}
+
+// 
+
+// CHECK-LABEL: func @omp_decl_tar_host
+// CHECK-SAME: {{.*}} attributes {omp.declare_target = #omp} {
+func.func @omp_decl_tar_host() -> () attributes {omp.declare_target = #omp} {
+  return
+}
+
+// CHECK-LABEL: func @omp_decl_tar_nohost
+// CHECK-SAME: {{.*}} attributes {omp.declare_target = #omp} {
+func.func @omp_decl_tar_nohost() -> () attributes {omp.declare_target = #omp} {
+  return
+}
+
+// CHECK-LABEL: func @omp_decl_tar_any
+// CHECK-SAME: {{.*}} attributes {omp.declare_target = #omp} {
+func.func @omp_decl_tar_any() -> () attributes {omp.declare_target = #

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

2023-04-25 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks!




Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1694
 
-std::vector ClangdLSPServer::getFixes(llvm::StringRef File,
-   const clangd::Diagnostic &D) {
-  std::lock_guard Lock(FixItsMutex);
-  auto DiagToFixItsIter = FixItsMap.find(File);
-  if (DiagToFixItsIter == FixItsMap.end())
-return {};
+std::vector ClangdLSPServer::getFixes(StringRef File,
+  const clangd::Diagnostic &D) 
{

nit: let's keep `llvm::StringRef`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148783

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


[PATCH] D147218: [OpenMP][Flang][MLIR] Lowering of OpenMP requires directive from parse tree to MLIR

2023-04-25 Thread Sergio Afonso via Phabricator via cfe-commits
skatrak updated this revision to Diff 516730.
skatrak added a comment.

Try undoing last change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147218

Files:
  flang/include/flang/Lower/OpenMP.h
  flang/lib/Lower/Bridge.cpp
  flang/lib/Lower/OpenMP.cpp
  flang/test/Lower/OpenMP/requires-error.f90
  flang/test/Lower/OpenMP/requires-notarget.f90
  flang/test/Lower/OpenMP/requires.f90

Index: flang/test/Lower/OpenMP/requires.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires.f90
@@ -0,0 +1,14 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks the lowering of requires into MLIR
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.atomic_default_mem_order = #omp
+!CHECK-SAME: omp.requires = #omp
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
+
+subroutine f
+  !$omp declare target
+end subroutine f
Index: flang/test/Lower/OpenMP/requires-notarget.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-notarget.f90
@@ -0,0 +1,12 @@
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  module attributes {
+!CHECK-SAME: omp.atomic_default_mem_order = #omp
+!CHECK-NOT:  omp.requires
+program requires
+  !$omp requires unified_shared_memory reverse_offload atomic_default_mem_order(seq_cst)
+end program requires
Index: flang/test/Lower/OpenMP/requires-error.f90
===
--- /dev/null
+++ flang/test/Lower/OpenMP/requires-error.f90
@@ -0,0 +1,15 @@
+! RUN: not %flang_fc1 -emit-fir -fopenmp %s -o - 2>&1 | FileCheck %s
+
+! This test checks that requires lowering into MLIR skips creating the
+! omp.requires attribute with target-related clauses if there are no device
+! functions in the compilation unit
+
+!CHECK:  error: {{.*}} conflicting atomic_default_mem_order clause found:
+!CHECK-SAME: acq_rel != seq_cst
+program requires
+  !$omp requires atomic_default_mem_order(seq_cst)
+end program requires
+
+subroutine f()
+  !$omp requires atomic_default_mem_order(acq_rel)
+end subroutine f
Index: flang/lib/Lower/OpenMP.cpp
===
--- flang/lib/Lower/OpenMP.cpp
+++ flang/lib/Lower/OpenMP.cpp
@@ -24,7 +24,9 @@
 #include "flang/Semantics/tools.h"
 #include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "mlir/Dialect/SCF/IR/SCF.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
+#include 
 
 using namespace mlir;
 
@@ -2239,11 +2241,13 @@
   converter.bindSymbol(sym, symThreadprivateExv);
 }
 
-void handleDeclareTarget(Fortran::lower::AbstractConverter &converter,
- Fortran::lower::pft::Evaluation &eval,
- const Fortran::parser::OpenMPDeclareTargetConstruct
- &declareTargetConstruct) {
-  llvm::SmallVector symbols;
+/// Extract the list of function and variable symbols affected by the given
+/// 'declare target' directive and return the intended device type for them.
+static mlir::omp::DeclareTargetDeviceType getDeclareTargetInfo(
+Fortran::lower::pft::Evaluation &eval,
+const Fortran::parser::OpenMPDeclareTargetConstruct &declareTargetConstruct,
+SmallVectorImpl &symbols) {
+  // Gather the symbols
   auto findFuncAndVarSyms = [&](const Fortran::parser::OmpObjectList &objList) {
 for (const auto &ompObject : objList.v) {
   Fortran::common::visit(
@@ -2261,12 +2265,11 @@
 }
   };
 
+  // The default capture type
+  auto deviceType = Fortran::parser::OmpDeviceTypeClause::Type::Any;
   const auto &spec{std::get(
   declareTargetConstruct.t)};
-  auto mod = converter.getFirOpBuilder().getModule();
 
-  // The default capture type
-  auto deviceType = Fortran::parser::OmpDeviceTypeClause::Type::Any;
   if (const auto *objectList{
   Fortran::parser::Unwrap(spec.u)}) {
 // Case: declare target(func, var1, var2)
@@ -2288,8 +2291,7 @@
  std::get_if(
  &clause.u)}) {
 // Case: declare target link(var1, var2)...
-TODO(converter.getCurrentLocation(),
- "the link clause is currently unsupported");
+TODO_NOLOC("the link clause is currently unsupported");
   } else if (const auto *deviceClause{
  std::get_if(
  &clause.u)}) {
@@ -2299,6 +2301,24 @@
 }
   }
 
+  switch (deviceType) {
+  case Fortran::parser::OmpDeviceTypeClause::Type::Any:
+return mlir::omp::DeclareTargetDeviceType::any;
+

[PATCH] D149144: [clang][dataflow] Eliminate intermediate `ReferenceValue`s from `Environment::DeclToLoc`.

2023-04-25 Thread Martin Böhme via Phabricator via cfe-commits
mboehme created this revision.
Herald added subscribers: martong, xazax.hun, inglorion.
Herald added a reviewer: NoQ.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

For the wider context of this change, see the RFC at
https://discourse.llvm.org/t/70086.

After this change, global and local variables of reference type are associated
directly with the `StorageLocation` of the referenced object instead of the
`StorageLocation` of a `ReferenceValue`.

Some tests that explicitly check for an existence of `ReferenceValue` for a
variable of reference type have been modified accordingly.

As discussed in the RFC, I have added an assertion to `Environment::join()` to
check that if both environments contain an entry for the same declaration in
`DeclToLoc`, they both map to the same `StorageLocation`. As discussed in
https://discourse.llvm.org/t/70086/5, this also necessitates removing
declarations from `DeclToLoc` when they go out of scope.

In the RFC, I proposed a gradual migration for this change, but it appears
that all of the callers of `Environment::setStorageLocation(const ValueDecl &,
SkipPast` are in the dataflow framework itself, and that there are only a few of
them.

As this is the function whose semantics are changing in a way that callers
potentially need to adapt to, I've decided to change the semantics of the
function directly.

The semantics of `getStorageLocation(const ValueDecl &, SkipPast SP` now no
longer depend on the behavior of the `SP` parameter. (There don't appear to be
any callers that use `SkipPast::ReferenceThenPointer`, so I've added an
assertion that forbids this usage.)

This patch adds a default argument for the `SP` parameter and removes the
explicit `SP` argument at the callsites that are touched by this change. A
followup patch will remove the argument from the remaining callsites,
allowing the `SkipPast` parameter to be removed entirely. (I don't want to do
that in this patch so that semantics-changing changes can be reviewed separately
from semantics-neutral changes.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149144

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.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
@@ -403,14 +403,9 @@
 
 const StorageLocation *FooLoc =
 Env.getStorageLocation(*FooDecl, SkipPast::None);
-ASSERT_TRUE(isa_and_nonnull(FooLoc));
-
-const ReferenceValue *FooVal =
-cast(Env.getValue(*FooLoc));
-const StorageLocation &FooReferentLoc = FooVal->getReferentLoc();
-EXPECT_TRUE(isa(&FooReferentLoc));
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
 
-const Value *FooReferentVal = Env.getValue(FooReferentLoc);
+const Value *FooReferentVal = Env.getValue(*FooLoc);
 EXPECT_TRUE(isa_and_nonnull(FooReferentVal));
   });
 }
@@ -494,11 +489,9 @@
 ASSERT_THAT(BazRefDecl, NotNull());
 ASSERT_THAT(BazPtrDecl, NotNull());
 
-const auto *FooLoc = cast(
+const auto *FooLoc = cast(
 Env.getStorageLocation(*FooDecl, SkipPast::None));
-const auto *FooVal = cast(Env.getValue(*FooLoc));
-const auto *FooReferentVal =
-cast(Env.getValue(FooVal->getReferentLoc()));
+const auto *FooReferentVal = cast(Env.getValue(*FooLoc));
 
 const auto *BarVal =
 cast(FooReferentVal->getChild(*BarDecl));
@@ -507,13 +500,17 @@
 
 const auto *FooRefVal =
 cast(BarReferentVal->getChild(*FooRefDecl));
-const StorageLocation &FooReferentLoc = FooRefVal->getReferentLoc();
-EXPECT_THAT(Env.getValue(FooReferentLoc), IsNull());
+const auto &FooReferentLoc =
+cast(FooRefVal->getReferentLoc());
+EXPECT_THAT(Env.getValue(FooReferentLoc), NotNull());
+EXPECT_THAT(Env.getValue(FooReferentLoc.getChild(*BarDecl)), IsNull());
 
 const auto *FooPtrVal =
 cast(BarReferentVal->getChild(*FooPtrDecl));
-const StorageLocation &FooPtrPointeeLoc = FooPtrVal->getPointeeLoc();
-EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), IsNull());
+const auto &FooPtrPointeeLoc =
+cast(FooPtrVal->getPointeeLoc());
+EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), NotNull());
+EXPECT_THAT(Env.getValue(FooPtrPointeeLoc.getChild(*BarDecl)), IsNull());
 
 const auto *BazRefVal =
 cast(BarReferentVal->getChild(*BazRefDecl));
@@ -1058,16 +1055,9 @@
 
 const StorageLocation *FooLo

[PATCH] D149144: [clang][dataflow] Eliminate intermediate `ReferenceValue`s from `Environment::DeclToLoc`.

2023-04-25 Thread Martin Böhme via Phabricator via cfe-commits
mboehme added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:43
 
+namespace {
+

There were a number of pre-existing declarations that were ODR violation risks. 
I'm adding a new file-local function below, so instead of making it static, I 
decided to put everything that's supposed to be file-local in an anonymous 
namespace.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:505
+cast(FooRefVal->getReferentLoc());
+EXPECT_THAT(Env.getValue(FooReferentLoc), NotNull());
+EXPECT_THAT(Env.getValue(FooReferentLoc.getChild(*BarDecl)), IsNull());

We're creating a slightly deeper object hierarchy here because we now call 
`createValue()` with the non-reference type in a number of places. I believe 
this slightly deeper hierarchy shouldn't be a problem in practice.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:2662
+
+ASSERT_THAT(EnvAfterLoop.getValue(*BarDecl), IsNull());
   });

The changes in this test are a result of the fact that we now remove 
declarations from `Environment::DeclToLoc` when they go out of scope.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149144

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


[PATCH] D146418: Support for OpenMP 5.0 sec 2.12.7 - Declare Target initializer expressions

2023-04-25 Thread Ritanya via Phabricator via cfe-commits
RitanyaB updated this revision to Diff 516733.
RitanyaB added a comment.

Adding the support in SemaOpenMP.cpp.


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

https://reviews.llvm.org/D146418

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/declare_target_messages.cpp
  clang/test/OpenMP/declare_target_variables_ast_print.cpp
  clang/test/OpenMP/nvptx_target_exceptions_messages.cpp

Index: clang/test/OpenMP/nvptx_target_exceptions_messages.cpp
===
--- clang/test/OpenMP/nvptx_target_exceptions_messages.cpp
+++ clang/test/OpenMP/nvptx_target_exceptions_messages.cpp
@@ -95,7 +95,7 @@
 int (*D)() = C; // expected-note {{used here}}
 // host-note@-1 {{used here}}
 #pragma omp end declare target
-int foobar3() { throw 1; }
+int foobar3() { throw 1; } // expected-error {{cannot use 'throw' with exceptions disabled}}
 
 // Check no infinite recursion in deferred diagnostic emitter.
 long E = (long)&E;
Index: clang/test/OpenMP/declare_target_variables_ast_print.cpp
===
--- /dev/null
+++ clang/test/OpenMP/declare_target_variables_ast_print.cpp
@@ -0,0 +1,111 @@
+// RUN: %clang_cc1 -w -verify -fopenmp -I %S/Inputs -ast-print %s | FileCheck %s --check-prefix=CHECK
+// expected-no-diagnostics
+
+static int variable = 100; 
+static float variable1 = 200;
+static float variable2 = variable1; 
+
+static int var = 1;
+
+static int var1 = 10;
+static int *var2 = &var1;
+static int **ptr1 = &var2;
+
+int arr[2] = {1,2};
+int (*arrptr)[2] = &arr;
+
+class declare{
+  public: int x;
+  void print();
+};
+declare obj1;
+declare *obj2 = &obj1;
+
+struct target{
+  int x;
+  void print();
+};
+static target S;
+
+#pragma omp declare target
+int target_var = variable;
+float target_var1 = variable2;
+int *ptr = &var;
+int ***ptr2 = &ptr1;
+int (**ptr3)[2] = &arrptr;
+declare **obj3 = &obj2;
+target *S1 = &S;
+#pragma omp end declare target
+// CHECK: #pragma omp declare target
+// CHECK-NEXT: static int variable = 100;
+// CHECK-NEXT: #pragma omp end declare target
+
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: static float variable1 = 200;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: static float variable2 = variable1;
+// CHECK-NEXT: #pragma omp end declare target
+
+// CHECK: #pragma omp declare target
+// CHECK-NEXT: static int var = 1;
+// CHECK-NEXT: #pragma omp end declare target
+
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: static int var1 = 10;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: static int *var2 = &var1;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: static int **ptr1 = &var2;
+// CHECK-NEXT: #pragma omp end declare target
+
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: int arr[2] = {1, 2};
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: int (*arrptr)[2] = &arr;
+// CHECK-NEXT: #pragma omp end declare target
+
+// CHECK-NEXT: class declare {
+// CHECK-NEXT: public: 
+// CHECK-NEXT:  int x;
+// CHECK-NEXT:  void print();
+// CHECK-NEXT: };
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: declare obj1;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: declare *obj2 = &obj1;
+// CHECK-NEXT: #pragma omp end declare target
+
+// CHECK-NEXT: struct target {
+// CHECK-NEXT:  int x;
+// CHECK-NEXT:  void print();
+// CHECK-NEXT: };
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: static target S;
+// CHECK-NEXT: #pragma omp end declare target
+
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: int target_var = variable;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: float target_var1 = variable2;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: int *ptr = &var;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: int ***ptr2 = &ptr1;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: int (**ptr3)[2] = &arrptr;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: declare **obj3 = &obj2; 
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: target *S1 = &S;
+// CHECK-NEXT: #pragma omp end declare target
Index: clang/test/OpenMP/declare_target_messages.cpp
===
--- clang/test/OpenMP/declare_target_messages.cpp
+++ clang/test/OpenMP/de

[PATCH] D148992: [clang-repl] Fix dynamic library test to avoid cstdio and linker

2023-04-25 Thread Stefan Gränitz via Phabricator via cfe-commits
sgraenitz accepted this revision.
sgraenitz added a comment.

Thanks. LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148992

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


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

2023-04-25 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 516734.
hokein marked an inline comment as done.
hokein added a comment.

update and rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148783

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

Index: clang-tools-extra/clangd/test/fixits-embed-in-diagnostic.test
===
--- clang-tools-extra/clangd/test/fixits-embed-in-diagnostic.test
+++ clang-tools-extra/clangd/test/fixits-embed-in-diagnostic.test
@@ -48,6 +48,7 @@
 # CHECK-NEXT:"source": "clang"
 # CHECK-NEXT:  },
 # CHECK-NEXT:  {
+# CHECK-NEXT:"codeActions": [],
 # CHECK-NEXT:"message": "Previous use is here\n\nfoo.c:1:18: error: use of 'Point' with tag type that does not match previous declaration",
 # CHECK-NEXT:"range": {
 # CHECK-NEXT:  "end": {
Index: clang-tools-extra/clangd/test/fixits-command-documentchanges.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/fixits-command-documentchanges.test
@@ -0,0 +1,194 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"workspace":{"workspaceEdit":{"documentChanges":true}}},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","text":"int main(int i, char **a) { if (i = 2) {}}"}}}
+#  CHECK:"method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:"diagnostics": [
+# CHECK-NEXT:  {
+# CHECK-NEXT:"code": "-Wparentheses",
+# CHECK-NEXT:"message": "Using the result of an assignment as a condition without parentheses (fixes available)",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 37,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 32,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"severity": 2,
+# CHECK-NEXT:"source": "clang"
+# CHECK-NEXT:  }
+# CHECK-NEXT:],
+# CHECK-NEXT:"uri": "file://{{.*}}/foo.c",
+# CHECK-NEXT:"version": 0
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":0,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"severity":2,"message":"Using the result of an assignment as a condition without parentheses (fixes available)"}]}}}
+#  CHECK:  "id": 2,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "arguments": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "documentChanges": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "edits": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "newText": "(",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 32,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 32,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:{
+# CHECK-NEXT:  "newText": ")",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 37,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 37,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  "textDocument": {
+# CHECK-NEXT:"uri": "file:///clangd-test/foo.c",
+# CHECK-NEXT:"version": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  ]
+# CHECK-NEXT:}
+# CHECK-NEX

[clang-tools-extra] 67e02b2 - [clangd] Add support TextDocumentEdit.

2023-04-25 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2023-04-25T13:07:42+02:00
New Revision: 67e02b282b70c05b10e4862fdeae4de45de39844

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

LOG: [clangd] Add support TextDocumentEdit.

This is an initial patch to add TextDocumentEdit (versioned edits) support in
clangd, the scope is minimal:

- add and extend the corresponding protocol structures
- propagate the documentChanges for diagnostic codeactions (our motivated case)

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

Added: 
clang-tools-extra/clangd/test/fixits-codeaction-documentchanges.test
clang-tools-extra/clangd/test/fixits-command-documentchanges.test

Modified: 
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/Diagnostics.h
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Protocol.h
clang-tools-extra/clangd/test/fixits-embed-in-diagnostic.test

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 6ead59a7ec90e..bef6c4de131ed 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -45,6 +45,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 namespace clang {
@@ -95,6 +96,26 @@ CodeAction toCodeAction(const ClangdServer::TweakRef &T, 
const URIForFile &File,
   return CA;
 }
 
+/// Convert from Fix to LSP CodeAction.
+CodeAction toCodeAction(const Fix &F, const URIForFile &File,
+const std::optional &Version,
+bool SupportsDocumentChanges) {
+  CodeAction Action;
+  Action.title = F.Message;
+  Action.kind = std::string(CodeAction::QUICKFIX_KIND);
+  Action.edit.emplace();
+  if (!SupportsDocumentChanges) {
+Action.edit->changes.emplace();
+(*Action.edit->changes)[File.uri()] = {F.Edits.begin(), F.Edits.end()};
+  } else {
+Action.edit->documentChanges.emplace();
+TextDocumentEdit& Edit = Action.edit->documentChanges->emplace_back();
+Edit.textDocument = VersionedTextDocumentIdentifier{{File}, Version};
+Edit.edits = {F.Edits.begin(), F.Edits.end()};
+  }
+  return Action;
+}
+
 void adjustSymbolKinds(llvm::MutableArrayRef Syms,
SymbolKindBitset Kinds) {
   for (auto &S : Syms) {
@@ -487,6 +508,7 @@ void ClangdLSPServer::onInitialize(const InitializeParams 
&Params,
   Params.capabilities.HierarchicalDocumentSymbol;
   SupportsReferenceContainer = Params.capabilities.ReferenceContainer;
   SupportFileStatus = Params.initializationOptions.FileStatus;
+  SupportsDocumentChanges = Params.capabilities.DocumentChanges;
   HoverContentFormat = Params.capabilities.HoverContentFormat;
   Opts.LineFoldingOnly = Params.capabilities.LineFoldingOnly;
   SupportsOffsetsInSignatureHelp = Params.capabilities.OffsetsInSignatureHelp;
@@ -761,8 +783,10 @@ void ClangdLSPServer::onCommandApplyTweak(const TweakArgs 
&Args,
   return Reply(std::move(Err));
 
 WorkspaceEdit WE;
+// FIXME: use documentChanges when SupportDocumentChanges is true.
+WE.changes.emplace();
 for (const auto &It : R->ApplyEdits) {
-  WE.changes[URI::createFile(It.first()).toString()] =
+  (*WE.changes)[URI::createFile(It.first()).toString()] =
   It.second.asTextEdits();
 }
 // ApplyEdit will take care of calling Reply().
@@ -833,8 +857,12 @@ void ClangdLSPServer::onRename(const RenameParams &Params,
if (auto Err = validateEdits(*Server, R->GlobalChanges))
  return Reply(std::move(Err));
WorkspaceEdit Result;
+   // FIXME: use documentChanges if SupportDocumentChanges is
+   // true.
+   Result.changes.emplace();
for (const auto &Rep : R->GlobalChanges) {
- Result.changes[URI::createFile(Rep.first()).toString()] =
+ (*Result
+   .changes)[URI::createFile(Rep.first()).toString()] =
  Rep.second.asTextEdits();
}
Reply(Result);
@@ -984,8 +1012,8 @@ void ClangdLSPServer::onCodeAction(const CodeActionParams 
&Params,
   std::vector FixIts;
   if (KindAllowed(CodeAction::QUICKFIX_KIND)) {
 for (const Diagnostic &D : Params.context.diagnostics) {
-  for (auto &F : getFixes(File.file(), D)) {
-FixIts.push_back(toCodeAction(F, Params.textDocument.uri));
+  for (auto &CA : getFixes(File.file(), D)) {
+FixIts.push_back(CA);
 FixIts.back().diagnostics = {D};
   }
 }
@@ -1663,8 +1691,8 @@ void ClangdLSPServer::profile(MemoryTree &MT)

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

2023-04-25 Thread Haojian Wu 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 rG67e02b282b70: [clangd] Add support TextDocumentEdit. 
(authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148783

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

Index: clang-tools-extra/clangd/test/fixits-embed-in-diagnostic.test
===
--- clang-tools-extra/clangd/test/fixits-embed-in-diagnostic.test
+++ clang-tools-extra/clangd/test/fixits-embed-in-diagnostic.test
@@ -48,6 +48,7 @@
 # CHECK-NEXT:"source": "clang"
 # CHECK-NEXT:  },
 # CHECK-NEXT:  {
+# CHECK-NEXT:"codeActions": [],
 # CHECK-NEXT:"message": "Previous use is here\n\nfoo.c:1:18: error: use of 'Point' with tag type that does not match previous declaration",
 # CHECK-NEXT:"range": {
 # CHECK-NEXT:  "end": {
Index: clang-tools-extra/clangd/test/fixits-command-documentchanges.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/fixits-command-documentchanges.test
@@ -0,0 +1,194 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"workspace":{"workspaceEdit":{"documentChanges":true}}},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","text":"int main(int i, char **a) { if (i = 2) {}}"}}}
+#  CHECK:"method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:"diagnostics": [
+# CHECK-NEXT:  {
+# CHECK-NEXT:"code": "-Wparentheses",
+# CHECK-NEXT:"message": "Using the result of an assignment as a condition without parentheses (fixes available)",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 37,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 32,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"severity": 2,
+# CHECK-NEXT:"source": "clang"
+# CHECK-NEXT:  }
+# CHECK-NEXT:],
+# CHECK-NEXT:"uri": "file://{{.*}}/foo.c",
+# CHECK-NEXT:"version": 0
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":0,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"severity":2,"message":"Using the result of an assignment as a condition without parentheses (fixes available)"}]}}}
+#  CHECK:  "id": 2,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "arguments": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "documentChanges": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "edits": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "newText": "(",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 32,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 32,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:{
+# CHECK-NEXT:  "newText": ")",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 37,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 37,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  "textDocument": {
+# CHECK-NEXT:"uri": "file:///clangd-test/foo.c",
+# CHECK-NEXT:"version": 0
+# CHECK-NEXT:   

[PATCH] D149144: [clang][dataflow] Eliminate intermediate `ReferenceValue`s from `Environment::DeclToLoc`.

2023-04-25 Thread Martin Böhme via Phabricator via cfe-commits
mboehme updated this revision to Diff 516745.
mboehme added a comment.

Add two assertions that I forgot to add in the first upload.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149144

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.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
@@ -403,14 +403,9 @@
 
 const StorageLocation *FooLoc =
 Env.getStorageLocation(*FooDecl, SkipPast::None);
-ASSERT_TRUE(isa_and_nonnull(FooLoc));
-
-const ReferenceValue *FooVal =
-cast(Env.getValue(*FooLoc));
-const StorageLocation &FooReferentLoc = FooVal->getReferentLoc();
-EXPECT_TRUE(isa(&FooReferentLoc));
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
 
-const Value *FooReferentVal = Env.getValue(FooReferentLoc);
+const Value *FooReferentVal = Env.getValue(*FooLoc);
 EXPECT_TRUE(isa_and_nonnull(FooReferentVal));
   });
 }
@@ -494,11 +489,9 @@
 ASSERT_THAT(BazRefDecl, NotNull());
 ASSERT_THAT(BazPtrDecl, NotNull());
 
-const auto *FooLoc = cast(
+const auto *FooLoc = cast(
 Env.getStorageLocation(*FooDecl, SkipPast::None));
-const auto *FooVal = cast(Env.getValue(*FooLoc));
-const auto *FooReferentVal =
-cast(Env.getValue(FooVal->getReferentLoc()));
+const auto *FooReferentVal = cast(Env.getValue(*FooLoc));
 
 const auto *BarVal =
 cast(FooReferentVal->getChild(*BarDecl));
@@ -507,13 +500,17 @@
 
 const auto *FooRefVal =
 cast(BarReferentVal->getChild(*FooRefDecl));
-const StorageLocation &FooReferentLoc = FooRefVal->getReferentLoc();
-EXPECT_THAT(Env.getValue(FooReferentLoc), IsNull());
+const auto &FooReferentLoc =
+cast(FooRefVal->getReferentLoc());
+EXPECT_THAT(Env.getValue(FooReferentLoc), NotNull());
+EXPECT_THAT(Env.getValue(FooReferentLoc.getChild(*BarDecl)), IsNull());
 
 const auto *FooPtrVal =
 cast(BarReferentVal->getChild(*FooPtrDecl));
-const StorageLocation &FooPtrPointeeLoc = FooPtrVal->getPointeeLoc();
-EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), IsNull());
+const auto &FooPtrPointeeLoc =
+cast(FooPtrVal->getPointeeLoc());
+EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), NotNull());
+EXPECT_THAT(Env.getValue(FooPtrPointeeLoc.getChild(*BarDecl)), IsNull());
 
 const auto *BazRefVal =
 cast(BarReferentVal->getChild(*BazRefDecl));
@@ -1058,16 +1055,9 @@
 
 const StorageLocation *FooLoc =
 Env.getStorageLocation(*FooDecl, SkipPast::None);
-ASSERT_TRUE(isa_and_nonnull(FooLoc));
-
-const ReferenceValue *FooVal =
-dyn_cast(Env.getValue(*FooLoc));
-ASSERT_THAT(FooVal, NotNull());
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
 
-const StorageLocation &FooReferentLoc = FooVal->getReferentLoc();
-EXPECT_TRUE(isa(&FooReferentLoc));
-
-const Value *FooReferentVal = Env.getValue(FooReferentLoc);
+const Value *FooReferentVal = Env.getValue(*FooLoc);
 EXPECT_TRUE(isa_and_nonnull(FooReferentVal));
   });
 }
@@ -1905,15 +1895,13 @@
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
 
-const auto *FooVal =
-cast(Env.getValue(*FooDecl, SkipPast::None));
+const auto *FooLoc = Env.getStorageLocation(*FooDecl);
 
 const ValueDecl *QuxDecl = findValueDecl(ASTCtx, "Qux");
 ASSERT_THAT(QuxDecl, NotNull());
 
-const auto *QuxVal =
-cast(Env.getValue(*QuxDecl, SkipPast::None));
-EXPECT_EQ(&QuxVal->getReferentLoc(), &FooVal->getReferentLoc());
+const auto *QuxLoc = Env.getStorageLocation(*QuxDecl);
+EXPECT_EQ(QuxLoc, FooLoc);
   });
 }
 
@@ -2593,9 +2581,8 @@
 
 const auto *FooVal =
 cast(Env.getValue(*FooDecl, SkipPast::None));
-const auto *BarVal =
-cast(Env.getValue(*BarDecl, SkipPast::None));
-EXPECT_EQ(&BarVal->getReferentLoc(), &FooVal->getPointeeLoc());
+const auto *BarLoc = Env.getStorageLocation(*BarDecl);
+EXPECT_EQ(BarLoc, &FooVal->getPointeeLoc());
   });
 }
 
@@ -2643,17 +2630,20 @@
 void target(int *Foo) {
   do {
 int Bar = *Foo;
+// [[in_loop]]
   } while (true);
   (void)0;
-  /*[[p]]*/
+  // [[after_loop]]
 }
   )";
   runDataflow(

[PATCH] D142907: LangRef: Add "dynamic" option to "denormal-fp-math"

2023-04-25 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added a comment.

In D142907#4288555 , @efriedma wrote:

> If you have a library function that's built with 
> "denormal-fp-math"="dynamic,dynamic", you can link it into code built in any 
> mode, and LTO should be able to propagate that mode from the caller to the 
> callee.  That doesn't require clang to do anything special; you can just 
> specify -fdenormal-fp-math=dynamic while building the library, and the user 
> specifies -fdenormal-fp-math=ieee while building their code.

That's essentially what this does. I think the part you are missing is the 
existing special treatment of the builtin device library functions. The default 
set of attributes for the current translation unit is forcibly set on functions 
in bitcode libraries linked in and internalized with -mlink-builtin-bitcode. We 
need to logically merge with the current translation unit's mode, or else we're 
potentially breaking the linked in function. The main reason I'm doing this in 
the first place is to move towards a model with less special treatment of these 
libraries.

> I guess you're worried specifically about ODR inline functions, defined in 
> headers?  The user specifies a specific mode because they know their code 
> honors it... but the user might not be aware of the effect on functions 
> defined in library headers.  Other libraries in the same binary might use the 
> same header, but specify a different mode.  So if the user specifies a 
> denormal mode, we should ignore it for ODR inline functions, because they 
> didn't actually mean to apply the denormal mode to those definitions?

No, the clang changes are to handle the headerless bitcode-only device 
libraries only. The user is supposed to be unaware the builtin libraries exist. 
They're an implementation detail managed by the clang driver 
(-mlink-builtin-bitcode is a cc1 only flag) and have a special contract with 
the compiler.

> I'm not sure about applying those semantics automatically; I don't think 
> there's any precedent in clang for anything like this.  The closest thing I 
> can think of is -fvisibility-inlines-hidden.  I'd prefer to RFC it separately 
> from the rest of the patch, and loop in clang frontend owners, since the 
> precedent we set here will apply to other sorts of attributes.

This isn't new, isn't end user facing, and isn't general purpose. This is the 
minimum required update to the existing -mlink-builtin-bitcode handling.


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

https://reviews.llvm.org/D142907

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


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

2023-04-25 Thread Balázs Benics via Phabricator via cfe-commits
steakhal accepted this revision.
steakhal added a comment.
This revision is now accepted and ready to land.

In D148355#4294798 , @steakhal wrote:

> In D148355#4294738 , @donat.nagy 
> wrote:
>
>> @steakhal I marked a few comments as Done (I accidentally missed some when I 
>> was creating the most recent patch) and now the only not-Done thing is the 
>> followup commit for refactoring the optionalness of RegionRawOffsetV2. Do 
>> you see anything else to do?
>
> I think it's good. I've just scheduled the measurement now for 180+ OSS 
> projects. Stay tuned!

The results are all good. The effect is pretty much equal to the effect of our 
implementation except for a handful of absent reports - and we are talking 
about at most 20 issues in total.
I must say, the report diff looks much better than I anticipated.
Thank you for working on this. Feel free to merge this change whenever you 
want. Thanks.

BTW the issues themselves look terrible. One cannot understand and track the 
value of the index and the assumed buffer size etc.
So the checker will still remain in alpha.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148355

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


[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

2023-04-25 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.
Herald added subscribers: hoy, Enna1, StephenFan.

We gave this a try in Chromium: 
https://bugs.chromium.org/p/chromium/issues/detail?id=1434989

While the diagnostics are interesting, two things make this less useful for our 
developers:

1. It also fires in situations where the developer didn't set any expectations, 
for example for static variable initialization guards where clang implicitly 
adds branch weights (https://crbug.com/1434989#c7)
2. Due to inlining etc., it often gets the source locations wrong, which means 
it points at code where again there were no expectations -- but perhaps that 
code got inlined into an expectations somewhere else. (e.g. 
https://crbug.com/1434989#c9)

Especially 2) is probably not easy to fix. Do you have any tips on how 
developers can use this more effectively?




Comment at: clang/include/clang/Driver/Options.td:1434
+def fdiagnostics_misexpect_tolerance_EQ : Joined<["-"], 
"fdiagnostics-misexpect-tolerance=">,
+Group, Flags<[CC1Option]>, MetaVarName<"">,
+HelpText<"Prevent misexpect diagnostics from being output if the profile 
counts are within N% of the expected. ">;

Should this be a driver mode option and not just cc1? The doc suggests using 
it, which currently won't work without an `-Xclang` prefix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115907

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


[PATCH] D149088: [clang-format] Add run-clang-format.py script.

2023-04-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

is it possible to pass other arguments like `-n`  if not running in place is 
there any protection from prevent stdout/stderr from overlapping..?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149088

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


[PATCH] D149088: [clang-format] Add run-clang-format.py script.

2023-04-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments.



Comment at: clang/tools/run-clang-format.py:63
+default="c,cc,cpp,cxx,c++,h,hh,hpp,hxx,h++")
+parser.add_argument('-style',
+help='formatting style',

what about 

```
run-clang-format ` --  
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149088

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


[PATCH] D149148: [Sema] Fix _Alignas/isCXX11Attribute() FIXME

2023-04-25 Thread Richard Sandiford via Phabricator via cfe-commits
rsandifo-arm created this revision.
rsandifo-arm added a reviewer: erichkeane.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
rsandifo-arm requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When doing https://reviews.llvm.org/D148105 , I hadn't noticed that
there was also a FIXME about the misclassification of _Alignas in
ProcessDeclAttribute.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149148

Files:
  clang/lib/Sema/SemaDeclAttr.cpp


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -8591,13 +8591,7 @@
 
   // Ignore C++11 attributes on declarator chunks: they appertain to the type
   // instead.
-  // FIXME: We currently check the attribute syntax directly instead of using
-  // isCXX11Attribute(), which currently erroneously classifies the C11
-  // `_Alignas` attribute as a C++11 attribute. `_Alignas` can appear on the
-  // `DeclSpec`, so we need to let it through here to make sure it is processed
-  // appropriately. Once the behavior of isCXX11Attribute() is fixed, we can
-  // go back to using that here.
-  if (AL.getSyntax() == ParsedAttr::AS_CXX11 && 
!Options.IncludeCXX11Attributes)
+  if (AL.isCXX11Attribute() && !Options.IncludeCXX11Attributes)
 return;
 
   // Unknown attributes are automatically warned on. Target-specific attributes


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -8591,13 +8591,7 @@
 
   // Ignore C++11 attributes on declarator chunks: they appertain to the type
   // instead.
-  // FIXME: We currently check the attribute syntax directly instead of using
-  // isCXX11Attribute(), which currently erroneously classifies the C11
-  // `_Alignas` attribute as a C++11 attribute. `_Alignas` can appear on the
-  // `DeclSpec`, so we need to let it through here to make sure it is processed
-  // appropriately. Once the behavior of isCXX11Attribute() is fixed, we can
-  // go back to using that here.
-  if (AL.getSyntax() == ParsedAttr::AS_CXX11 && !Options.IncludeCXX11Attributes)
+  if (AL.isCXX11Attribute() && !Options.IncludeCXX11Attributes)
 return;
 
   // Unknown attributes are automatically warned on. Target-specific attributes
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D148601: [Clang] Handle Error message to output proper Prefix

2023-04-25 Thread Usman Akinyemi via Phabricator via cfe-commits
Unique_Usman updated this revision to Diff 516762.

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

https://reviews.llvm.org/D148601

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/lib/Frontend/VerifyDiagnosticConsumer.cpp


Index: clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
===
--- clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
+++ clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
@@ -1095,7 +1095,8 @@
 // Produce an error if no expected-* directives could be found in the
 // source file(s) processed.
 if (Status == HasNoDirectives) {
-  Diags.Report(diag::err_verify_no_directives).setForceEmit();
+  const auto &Prefixes = Diags.getDiagnosticOptions().VerifyPrefixes;
+  Diags.Report(diag::err_verify_no_directives).setForceEmit() << 
*Prefixes.begin();
   ++NumErrors;
   Status = HasNoDirectivesReported;
 }
Index: clang/include/clang/Basic/DiagnosticFrontendKinds.td
===
--- clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -175,7 +175,7 @@
 "%select{expected|'expected-no-diagnostics'}0 directive cannot follow "
 "%select{'expected-no-diagnostics' directive|other expected directives}0">;
 def err_verify_no_directives : Error<
-"no expected directives found: consider use of 'expected-no-diagnostics'">;
+"no expected directives found: consider use of '%0-no-diagnostics'">;
 def err_verify_nonconst_addrspace : Error<
   "qualifier 'const' is needed for variables in address space '%0'">;
 


Index: clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
===
--- clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
+++ clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
@@ -1095,7 +1095,8 @@
 // Produce an error if no expected-* directives could be found in the
 // source file(s) processed.
 if (Status == HasNoDirectives) {
-  Diags.Report(diag::err_verify_no_directives).setForceEmit();
+  const auto &Prefixes = Diags.getDiagnosticOptions().VerifyPrefixes;
+  Diags.Report(diag::err_verify_no_directives).setForceEmit() << *Prefixes.begin();
   ++NumErrors;
   Status = HasNoDirectivesReported;
 }
Index: clang/include/clang/Basic/DiagnosticFrontendKinds.td
===
--- clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -175,7 +175,7 @@
 "%select{expected|'expected-no-diagnostics'}0 directive cannot follow "
 "%select{'expected-no-diagnostics' directive|other expected directives}0">;
 def err_verify_no_directives : Error<
-"no expected directives found: consider use of 'expected-no-diagnostics'">;
+"no expected directives found: consider use of '%0-no-diagnostics'">;
 def err_verify_nonconst_addrspace : Error<
   "qualifier 'const' is needed for variables in address space '%0'">;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D149149: [clang][Interp] Check one-past-the-end pointers in GetPtrField

2023-04-25 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

  Rename CheckBaseDerived to something more general and call it in
  GetPtrField() as well, so we don't crash later in Pointer::toAPValue().


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149149

Files:
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/Interp.h
  clang/test/AST/Interp/records.cpp


Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -510,6 +510,12 @@
 // expected-note {{cannot access base class of 
pointer past the end of object}} \
 // ref-error {{must be initialized by a constant 
expression}} \
 // ref-note {{cannot access base class of pointer 
past the end of object}}
+
+  constexpr const int *pn = &(&b + 1)->n; // expected-error {{must be 
initialized by a constant expression}} \
+  // expected-note {{cannot access 
field of pointer past the end of object}} \
+  // ref-error {{must be initialized 
by a constant expression}} \
+  // ref-note {{cannot access field of 
pointer past the end of object}}
+
 }
 
 #if __cplusplus >= 202002L
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -67,9 +67,9 @@
 bool CheckRange(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
 CheckSubobjectKind CSK);
 
-/// Checks if accessing a base or derived record of the given pointer is valid.
-bool CheckBaseDerived(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
-  CheckSubobjectKind CSK);
+/// Checks if Ptr is a one-past-the-end pointer.
+bool CheckSubobject(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
+CheckSubobjectKind CSK);
 
 /// Checks if a pointer points to const storage.
 bool CheckConst(InterpState &S, CodePtr OpPC, const Pointer &Ptr);
@@ -1039,6 +1039,8 @@
 return false;
   if (!CheckRange(S, OpPC, Ptr, CSK_Field))
 return false;
+  if (!CheckSubobject(S, OpPC, Ptr, CSK_Field))
+return false;
   S.Stk.push(Ptr.atField(Off));
   return true;
 }
@@ -1083,7 +1085,7 @@
   const Pointer &Ptr = S.Stk.pop();
   if (!CheckNull(S, OpPC, Ptr, CSK_Derived))
 return false;
-  if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Derived))
+  if (!CheckSubobject(S, OpPC, Ptr, CSK_Derived))
 return false;
   S.Stk.push(Ptr.atFieldSub(Off));
   return true;
@@ -1093,7 +1095,7 @@
   const Pointer &Ptr = S.Stk.peek();
   if (!CheckNull(S, OpPC, Ptr, CSK_Base))
 return false;
-  if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Base))
+  if (!CheckSubobject(S, OpPC, Ptr, CSK_Base))
 return false;
   S.Stk.push(Ptr.atField(Off));
   return true;
@@ -1103,7 +1105,7 @@
   const Pointer &Ptr = S.Stk.pop();
   if (!CheckNull(S, OpPC, Ptr, CSK_Base))
 return false;
-  if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Base))
+  if (!CheckSubobject(S, OpPC, Ptr, CSK_Base))
 return false;
   S.Stk.push(Ptr.atField(Off));
   return true;
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -211,8 +211,8 @@
   return false;
 }
 
-bool CheckBaseDerived(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
-  CheckSubobjectKind CSK) {
+bool CheckSubobject(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
+CheckSubobjectKind CSK) {
   if (!Ptr.isOnePastEnd())
 return true;
 


Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -510,6 +510,12 @@
 // expected-note {{cannot access base class of pointer past the end of object}} \
 // ref-error {{must be initialized by a constant expression}} \
 // ref-note {{cannot access base class of pointer past the end of object}}
+
+  constexpr const int *pn = &(&b + 1)->n; // expected-error {{must be initialized by a constant expression}} \
+  // expected-note {{cannot access field of pointer past the end of object}} \
+  // ref-error {{must be initialized by a constant expression}} \
+  // ref-note {{cannot access field of pointer past the end of object}}
+
 }
 
 #if __cplusplus >= 202002L
Index: cla

cfe-commits@lists.llvm.org

2023-04-25 Thread Martin Böhme via Phabricator via cfe-commits
mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This parameter was already a no-op, so removing it doesn't change behavior.

Depends On D149144 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149151

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
  clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -723,8 +723,8 @@
 const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
 ASSERT_THAT(BarDecl, NotNull());
 
-const auto *FooLoc = cast(
-Env.getStorageLocation(*FooDecl, SkipPast::None));
+const auto *FooLoc =
+cast(Env.getStorageLocation(*FooDecl));
 const auto *BarVal =
 cast(Env.getValue(*BarDecl, SkipPast::None));
 EXPECT_EQ(&BarVal->getPointeeLoc(), FooLoc);
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -98,7 +98,7 @@
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
 
-EXPECT_EQ(Env.getStorageLocation(*FooDecl, SkipPast::None), nullptr);
+EXPECT_EQ(Env.getStorageLocation(*FooDecl), nullptr);
   },
   LangStandard::lang_cxx17,
   /*ApplyBuiltinTransfer=*/false);
@@ -121,8 +121,7 @@
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
 
-const StorageLocation *FooLoc =
-Env.getStorageLocation(*FooDecl, SkipPast::None);
+const StorageLocation *FooLoc = Env.getStorageLocation(*FooDecl);
 ASSERT_TRUE(isa_and_nonnull(FooLoc));
 
 const Value *FooVal = Env.getValue(*FooLoc);
@@ -147,8 +146,7 @@
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
 
-const StorageLocation *FooLoc =
-Env.getStorageLocation(*FooDecl, SkipPast::None);
+const StorageLocation *FooLoc = Env.getStorageLocation(*FooDecl);
 ASSERT_TRUE(isa_and_nonnull(FooLoc));
 
 const Value *FooVal = Env.getValue(*FooLoc);
@@ -227,8 +225,8 @@
 }
 ASSERT_THAT(UnmodeledDecl, NotNull());
 
-const auto *FooLoc = cast(
-Env.getStorageLocation(*FooDecl, SkipPast::None));
+const auto *FooLoc =
+cast(Env.getStorageLocation(*FooDecl));
 const auto *UnmodeledLoc = &FooLoc->getChild(*UnmodeledDecl);
 ASSERT_TRUE(isa(UnmodeledLoc));
 ASSERT_THAT(Env.getValue(*UnmodeledLoc), IsNull());
@@ -274,8 +272,8 @@
 }
 ASSERT_THAT(BarDecl, NotNull());
 
-const auto *FooLoc = cast(
-Env.getStorageLocation(*FooDecl, SkipPast::None));
+const auto *FooLoc =
+cast(Env.getStorageLocation(*FooDecl));
 const auto *BarLoc =
 cast(&FooLoc->getChild(*BarDecl));
 
@@ -322,8 +320,8 @@
 }
 ASSERT_THAT(BarDecl, NotNull());
 
-const auto *FooLoc = cast(
-Env.getStorageLocation(*FooDecl, SkipPast::None));
+const auto *FooLoc =
+cast(Env.getStorageLocation(*FooDecl));
 const auto *BarLoc =
 cast(&FooLoc->getChild(*BarDecl));
 
@@ -369,8 +367,8 @@
 }
 ASSERT_THAT(BarDecl, NotNull());
 
-const auto *FooLoc = cast(
-Env.getStorageLocation(*FooDecl, SkipPast::None));
+const auto *FooLoc =
+cast(Env.getStorageLocation(*FooDecl));
 const auto *BarLoc =
 cast(&FooLoc->getChild(*BarDecl));
 
@@ -401,8 +399,7 @@
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
 
-const StorageLocation *FooLoc =
-Env.getStorageLocation(*FooDecl, SkipPast::None);
+const StorageLocation *FooLoc = Env.getStorageLocation(*FooDecl);
 ASSERT_TRUE(isa_and_nonnull(FooLoc));
 
 const Value *FooReferentVal = Env.getValue(*FooLoc);
@@ -489,8 +486,8 @@
 ASSERT_THA

[PATCH] D149009: [Sema]Select correct lexical context during template instantiate

2023-04-25 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:3598
+  FD->isDefined(FDFriend, true) &&
+  FDFriend->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None) {
+// if Function defined by inline friend, use inline fried as DeclContext

HerrCai0907 wrote:
> erichkeane wrote:
> > So in what case would the currently-instantiated definition NOT also be a 
> > friend?  I would think this last condition should be able to be an assert 
> > instead.
> Last condition cannot be an assert, define and declare in difference place is 
> common case, what we need to identifier in here is inlined friend define.
Can you be more clear here?  WHEN can a definition and declaration NOT have the 
same friend object kind?  THAT is probably incorrect a bug.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:3601
+FD = const_cast(FDFriend);
+Owner = FD->getDescribedFunctionTemplate()->getLexicalDeclContext();
+  }

HerrCai0907 wrote:
> erichkeane wrote:
> > I THINK this should just be:
> > `Owner = FD->getLexicalDeclContext()` here.  
> The old `Owner` is `FunctionTemplate->getLexicalDeclContext()`. So I think we 
> should use same level owner.
Its not a question of "SameLevel", its that it was the accessible one based on 
what was around.  IN this case, we have a 'better' alternative, which is the 
FunctionDecl.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149009

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


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

2023-04-25 Thread Kiran Chandramohan via Phabricator via cfe-commits
kiranchandramohan added a comment.

In D146557#4292223 , @TIFitis wrote:

> Cleaned up how IsBegin argument is passed for createTargetData call.

Please submit this directly as an NFC patch.

We should always work towards simple patches that are easy to review. Sometimes 
a big patch is required to give the full context. But it can then be broken up 
into small patches for faster review. I have created D149153 
 for the `inlineConvertOmpRegions` change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146557

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


[PATCH] D146418: Support for OpenMP 5.0 sec 2.12.7 - Declare Target initializer expressions

2023-04-25 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:14467-14468
+  // directive and has static storage duration.
+  if (D && D->hasAttr() && isa(D)) {
+if ((cast(D))->hasGlobalStorage())
+  ActOnOpenMPImplicitDeclareTarget(D);

if (auto *VD = dyn_cast_or_null(D); LangOpts.OpenMP && VD && 
VD->hasAttr() && VD->hasGlobalStorage())



Comment at: clang/lib/Sema/SemaOpenMP.cpp:23091
+/// Adds OMPDeclareTargetDeclAttr to the referenced variables.
+void *AddOMPDeclareTargetDeclAttr(const DeclRefExpr *DeclRef, Decl 
*TargetDecl, 
+  SmallVector &DeclVector) {

static void addOMP...



Comment at: clang/lib/Sema/SemaOpenMP.cpp:23092
+void *AddOMPDeclareTargetDeclAttr(const DeclRefExpr *DeclRef, Decl 
*TargetDecl, 
+  SmallVector &DeclVector) {
+  Decl *DeclVar = nullptr;

SmallVectorImpl &DeclVector



Comment at: clang/lib/Sema/SemaOpenMP.cpp:23093-23100
+  Decl *DeclVar = nullptr;
+  if (DeclRef) {
+DeclVar = (Decl *)DeclRef->getDecl();
+if (isa(DeclVar)) { 
+  DeclVar->addAttr(TargetDecl->getAttr());
+  DeclVector.push_back(DeclVar);
+}

if (!DeclRef)
  return;
if (auto *VD = dyn_cast_or_null(DeclRef->getDecl()) {
  DeclVar->addAttr(TargetDecl->getAttr());
  DeclVector.push_back(DeclVar);
}



Comment at: clang/lib/Sema/SemaOpenMP.cpp:23103-23114
+void RecursiveExprReader(Expr *Ex, Decl *TargetDecl, 
+ SmallVector &DeclVector) {
+  for (Expr::child_iterator it = Ex->child_begin(); it != Ex->child_end(); 
+   ++it) {
+if (const DeclRefExpr *refExpr = dyn_cast(*it)) {
+  AddOMPDeclareTargetDeclAttr(refExpr, TargetDecl, DeclVector);
+} else if (isa(*it))

Use StmtVisitor to implement this correctly.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:23120
+void Sema::ActOnOpenMPImplicitDeclareTarget(Decl *TargetDecl) {
+  SmallVector DeclVector;
+  DeclVector.push_back(TargetDecl);

100 is to much, use either default value `SmallVector DeclVector;` or 
smaller value (4 should be enough)



Comment at: clang/lib/Sema/SemaOpenMP.cpp:23122
+  DeclVector.push_back(TargetDecl);
+  while (DeclVector.size() != 0) {
+Decl *D = DeclVector[DeclVector.size() - 1];

while (!DeclVector.empty())



Comment at: clang/lib/Sema/SemaOpenMP.cpp:23123-23124
+  while (DeclVector.size() != 0) {
+Decl *D = DeclVector[DeclVector.size() - 1];
+DeclVector.pop_back();
+if (isa(D) && D->hasAttr()) {

Decl *D = DeclVector.pop_back_val();



Comment at: clang/lib/Sema/SemaOpenMP.cpp:23125-23126
+DeclVector.pop_back();
+if (isa(D) && D->hasAttr()) {
+  VarDecl *TargetVarDecl = cast(D);
+  if (TargetVarDecl->hasInit() && TargetVarDecl->hasGlobalStorage()) {

Use dyn_cast instead of isa/cast


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

https://reviews.llvm.org/D146418

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


[PATCH] D149154: [clang][Interp] Emit diagnostic when comparing function pointers

2023-04-25 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

  Function pointers can be compared for (in)equality but, but LE, GE, LT,
  and GT opcodes should emit an error and abort.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149154

Files:
  clang/lib/AST/Interp/FunctionPointer.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Opcodes.td
  clang/test/AST/Interp/functions.cpp

Index: clang/test/AST/Interp/functions.cpp
===
--- clang/test/AST/Interp/functions.cpp
+++ clang/test/AST/Interp/functions.cpp
@@ -178,6 +178,31 @@
   static_assert(s.fp == nullptr, ""); // zero-initialized function pointer.
 }
 
+namespace Comparison {
+  void f(), g();
+  constexpr void (*pf)() = &f, (*pg)() = &g;
+
+  constexpr bool u13 = pf < pg; // ref-warning {{ordered comparison of function pointers}} \
+// ref-error {{must be initialized by a constant expression}} \
+// ref-note {{comparison between '&f' and '&g' has unspecified value}} \
+// expected-warning {{ordered comparison of function pointers}} \
+// expected-error {{must be initialized by a constant expression}} \
+// expected-note {{comparison between '&f' and '&g' has unspecified value}}
+
+  constexpr bool u14 = pf < (void(*)())nullptr; // ref-warning {{ordered comparison of function pointers}} \
+// ref-error {{must be initialized by a constant expression}} \
+// ref-note {{comparison between '&f' and 'nullptr' has unspecified value}} \
+// expected-warning {{ordered comparison of function pointers}} \
+// expected-error {{must be initialized by a constant expression}} \
+// expected-note {{comparison between '&f' and 'nullptr' has unspecified value}}
+
+
+
+  static_assert(pf != pg, "");
+  static_assert(pf == &f, "");
+  static_assert(pg == &g, "");
+}
+
 }
 
 struct F {
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -94,7 +94,7 @@
 }
 
 def ComparableTypeClass : TypeClass {
-  let Types = !listconcat(AluTypeClass.Types, [Ptr], [Float]);
+  let Types = !listconcat(AluTypeClass.Types, [Ptr], [Float], [FnPtr]);
 }
 
 class SingletonTypeClass : TypeClass {
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -637,6 +637,32 @@
   return CmpHelper(S, OpPC, Fn);
 }
 
+/// Function pointers cannot be compared in an ordered way.
+template <>
+inline bool CmpHelper(InterpState &S, CodePtr OpPC,
+   CompareFn Fn) {
+  const auto &RHS = S.Stk.pop();
+  const auto &LHS = S.Stk.pop();
+
+  std::string LS =
+  LHS.toAPValue().getAsString(S.getCtx(), LHS.getType(S.getCtx()));
+  std::string RS =
+  RHS.toAPValue().getAsString(S.getCtx(), RHS.getType(S.getCtx()));
+  const SourceInfo &Loc = S.Current->getSource(OpPC);
+  S.FFDiag(Loc, diag::note_constexpr_pointer_comparison_unspecified)
+  << LS << RS;
+  return false;
+}
+
+template <>
+inline bool CmpHelperEQ(InterpState &S, CodePtr OpPC,
+ CompareFn Fn) {
+  const auto &RHS = S.Stk.pop();
+  const auto &LHS = S.Stk.pop();
+  S.Stk.push(Boolean::from(Fn(LHS.compare(RHS;
+  return true;
+}
+
 template <>
 inline bool CmpHelper(InterpState &S, CodePtr OpPC, CompareFn Fn) {
   using BoolT = PrimConv::T;
Index: clang/lib/AST/Interp/FunctionPointer.h
===
--- clang/lib/AST/Interp/FunctionPointer.h
+++ clang/lib/AST/Interp/FunctionPointer.h
@@ -6,6 +6,7 @@
 #include "Function.h"
 #include "Primitives.h"
 #include "clang/AST/APValue.h"
+#include "clang/AST/ASTContext.h"
 
 namespace clang {
 namespace interp {
@@ -38,6 +39,13 @@
 OS << ")";
   }
 
+  QualType getType(const ASTContext &Ctx) const {
+if (!Func)
+  return Ctx.NullPtrTy;
+
+return Func->getDecl()->getType();
+  }
+
   ComparisonCategoryResult compare(const FunctionPointer &RHS) const {
 if (Func == RHS.Func)
   return ComparisonCategoryResult::Equal;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D148601: [Clang] Handle Error message to output proper Prefix

2023-04-25 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment.

In D148601#4279604 , @Unique_Usman 
wrote:

> In D148601#4279334 , @tbaeder wrote:
>
>> I am not 100% sure about the semantics of passing multiple prefixes, i.e. if 
>> the error is emitted for all prefixes individually or if it's only emitted 
>> if no `expected` line for any of the prefixes is found. In the latter case 
>> we should probably add all the prefixes to the error message.
>
> I tested different scenerios e.g added more than one RUN lines with different 
> value of -verify, what I concluded on is that if we have multiple  RUN lines 
> with each of them having no directive, the prefixes generated is always of 
> the first occurence  with no  expected directive so, the error is always 
> generated for the first occurence with no expected directive.

Yeah but I think you can do `-verify=foo,bar`(?) in which case the list f 
prefixes would actually have more than one item.


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

https://reviews.llvm.org/D148601

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


[PATCH] D149158: [clang][analyzer] Cleanup tests of StdCLibraryFunctionsChecker (NFC)

2023-04-25 Thread Balázs Kéri via Phabricator via cfe-commits
balazske created this revision.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, gamesh411, 
dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, 
baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: All.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Function declarations are moved into common header that can be reused
to avoid repetitions in different test files.
Some small problems in the tests were found and fixed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149158

Files:
  clang/test/Analysis/Inputs/std-c-library-functions-POSIX.h
  clang/test/Analysis/Inputs/std-c-library-functions.h
  clang/test/Analysis/std-c-library-functions-POSIX.c
  clang/test/Analysis/std-c-library-functions-arg-constraints.c
  clang/test/Analysis/std-c-library-functions.c

Index: clang/test/Analysis/std-c-library-functions.c
===
--- clang/test/Analysis/std-c-library-functions.c
+++ clang/test/Analysis/std-c-library-functions.c
@@ -50,6 +50,9 @@
 // CHECK-NEXT: Loaded summary for: int isspace(int)
 // CHECK-NEXT: Loaded summary for: int isupper(int)
 // CHECK-NEXT: Loaded summary for: int isxdigit(int)
+// CHECK-NEXT: Loaded summary for: int toupper(int)
+// CHECK-NEXT: Loaded summary for: int tolower(int)
+// CHECK-NEXT: Loaded summary for: int toascii(int)
 // CHECK-NEXT: Loaded summary for: int getc(FILE *)
 // CHECK-NEXT: Loaded summary for: int fgetc(FILE *)
 // CHECK-NEXT: Loaded summary for: int getchar(void)
@@ -59,16 +62,14 @@
 // CHECK-NEXT: Loaded summary for: ssize_t write(int, const void *, size_t)
 // CHECK-NEXT: Loaded summary for: ssize_t getline(char **restrict, size_t *restrict, FILE *restrict)
 // CHECK-NEXT: Loaded summary for: ssize_t getdelim(char **restrict, size_t *restrict, int, FILE *restrict)
+// CHECK-NEXT: Loaded summary for: char *getenv(const char *)
 
+#include "Inputs/std-c-library-functions.h"
 
 void clang_analyzer_eval(int);
 
 int glob;
 
-typedef struct FILE FILE;
-#define EOF -1
-
-int getc(FILE *);
 void test_getc(FILE *fp) {
   int x;
   while ((x = getc(fp)) != EOF) {
@@ -77,17 +78,11 @@
   }
 }
 
-int fgetc(FILE *);
 void test_fgets(FILE *fp) {
   clang_analyzer_eval(fgetc(fp) < 256); // expected-warning{{TRUE}}
   clang_analyzer_eval(fgetc(fp) >= 0); // expected-warning{{UNKNOWN}}
 }
 
-
-typedef typeof(sizeof(int)) size_t;
-typedef signed long ssize_t;
-ssize_t read(int, void *, size_t);
-ssize_t write(int, const void *, size_t);
 void test_read_write(int fd, char *buf) {
   glob = 1;
   ssize_t x = write(fd, buf, 10);
@@ -106,8 +101,6 @@
   }
 }
 
-size_t fread(void *restrict, size_t, size_t, FILE *restrict);
-size_t fwrite(const void *restrict, size_t, size_t, FILE *restrict);
 void test_fread_fwrite(FILE *fp, int *buf) {
 
   size_t x = fwrite(buf, sizeof(int), 10, fp);
@@ -128,8 +121,6 @@
   (void)fread(ptr, sz, nmem, fp); // expected-warning {{1st function call argument is an uninitialized value}}
 }
 
-ssize_t getline(char **restrict, size_t *restrict, FILE *restrict);
-ssize_t getdelim(char **restrict, size_t *restrict, int, FILE *restrict);
 void test_getline(FILE *fp) {
   char *line = 0;
   size_t n = 0;
@@ -139,7 +130,6 @@
   }
 }
 
-int isascii(int);
 void test_isascii(int x) {
   clang_analyzer_eval(isascii(123)); // expected-warning{{TRUE}}
   clang_analyzer_eval(isascii(-1)); // expected-warning{{FALSE}}
@@ -157,7 +147,6 @@
   clang_analyzer_eval(glob); // expected-warning{{TRUE}}
 }
 
-int islower(int);
 void test_islower(int x) {
   clang_analyzer_eval(islower('x')); // expected-warning{{TRUE}}
   clang_analyzer_eval(islower('X')); // expected-warning{{FALSE}}
@@ -165,7 +154,6 @@
 clang_analyzer_eval(x < 'a'); // expected-warning{{FALSE}}
 }
 
-int getchar(void);
 void test_getchar(void) {
   int x = getchar();
   if (x == EOF)
@@ -174,27 +162,23 @@
   clang_analyzer_eval(x < 256); // expected-warning{{TRUE}}
 }
 
-int isalpha(int);
 void test_isalpha(void) {
   clang_analyzer_eval(isalpha(']')); // expected-warning{{FALSE}}
   clang_analyzer_eval(isalpha('Q')); // expected-warning{{TRUE}}
   clang_analyzer_eval(isalpha(128)); // expected-warning{{UNKNOWN}}
 }
 
-int isalnum(int);
 void test_alnum(void) {
   clang_analyzer_eval(isalnum('1')); // expected-warning{{TRUE}}
   clang_analyzer_eval(isalnum(')')); // expected-warning{{FALSE}}
 }
 
-int isblank(int);
 void test_isblank(void) {
   clang_analyzer_eval(isblank('\t')); // expected-warning{{TRUE}}
   clang_analyzer_eval(isblank(' ')); // expected-warning{{TRUE}}
   clang_analyzer_eval(isblank('\n')); // expected-warning{{FALSE}}
 }
 
-int ispunct(int);
 void test_ispunct(int x) {
   clang_analyzer_eval(ispunct(' ')); // expected-warning{{FALSE}}
   clang_analyzer_eval(ispunct(-1)); // expected-warning{{FALSE}}
@@ -204,21 +188,17 @@
 clang_analyzer_eval(x < 127); // expected-warning

cfe-commits@lists.llvm.org

2023-04-25 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel accepted this revision.
ymandel added a comment.

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149151

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


[PATCH] D125272: [clang] Add -fcheck-new support

2023-04-25 Thread Pedro Falcato via Phabricator via cfe-commits
heatd added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4117
+  if (const Arg *A = Args.getLastArg(OPT_fcheck_new))
+Opts.CheckNew = true;
+

MaskRay wrote:
> Use `CodeGenOpts<"CheckNew">` and avoid change to this file. CodeGenOpts 
> seems more suitable than LangOpts.
Sorry, I haven't looked at this rev in a good while due to $REASONS. Could you 
elaborate on this? I'm not opposed to moving it to CodeGenOpts per se, but I 
would like to understand your reasoning. It seems to me that fcheck-new is more 
of a language change (since it removes the implicit [[nonnull]] on the result 
of operator new) than a codegen change, although it does have its effects on 
codegen, naturally. A quick look at CodeGenOpts in Options.td would hint to me 
that these mostly/only affect codegen itself. What do you think? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125272

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


[PATCH] D149160: [clang][analyzer] Handle special value AT_FDCWD in affected standard functions

2023-04-25 Thread Balázs Kéri via Phabricator via cfe-commits
balazske created this revision.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, gamesh411, 
dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, 
baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a reviewer: NoQ.
Herald added a project: All.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Some file and directory related functions have an integer file descriptor 
argument
that can be a valid file descriptor or a special value AT_FDCWD. This value is
relatively often used in open source projects and is usually defined as a 
negative
number, and the checker reports false warnings (a valid file descriptor is not
negative) if this fix is not included.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149160

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints.c

Index: clang/test/Analysis/std-c-library-functions-arg-constraints.c
===
--- clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -3,6 +3,7 @@
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
 // RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
 // RUN:   -analyzer-checker=debug.StdCLibraryFunctionsTester \
 // RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -triple x86_64-unknown-linux-gnu \
@@ -13,6 +14,7 @@
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
 // RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
 // RUN:   -analyzer-checker=debug.StdCLibraryFunctionsTester \
 // RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -triple x86_64-unknown-linux-gnu \
@@ -309,3 +311,26 @@
   // bugpath-warning{{The 1st argument to '__buf_size_arg_constraint_concrete' is out of the accepted range; It should be a buffer with size equal to or greater than 10}} \
   // bugpath-note{{The 1st argument to '__buf_size_arg_constraint_concrete' is out of the accepted range; It should be a buffer with size equal to or greater than 10}}
 }
+
+void test_file_fd_at_functions() {
+  (void)linkat(-22, "from", AT_FDCWD, "to", 0); // \
+  // report-warning{{The 1st argument to 'linkat' is -22 but should be -101 or >= 0}} \
+  // bugpath-warning{{The 1st argument to 'linkat' is -22 but should be -101 or >= 0}} \
+  // bugpath-note{{The 1st argument to 'linkat' is -22 but should be -101 or >= 0}}
+
+  // no warning for these functions if the AT_FDCWD value is used
+  (void)linkat(AT_FDCWD, "from", AT_FDCWD, "to", 0);
+  (void)faccessat(AT_FDCWD, "path", 0, 0);
+  (void)symlinkat("oldpath", AT_FDCWD, "newpath");
+  (void)mkdirat(AT_FDCWD, "path", 0);
+  (void)mknodat(AT_FDCWD, "path", 0, 0);
+  (void)fchmodat(AT_FDCWD, "path", 0, 0);
+  (void)fchownat(AT_FDCWD, "path", 0, 0, 0);
+  (void)linkat(AT_FDCWD, "oldpath", AT_FDCWD, "newpath", 0);
+  (void)unlinkat(AT_FDCWD, "newpath", 0);
+  struct stat St;
+  (void)fstatat(AT_FDCWD, "newpath", &St, 0);
+  char Buf[10];
+  (void)readlinkat(AT_FDCWD, "newpath", Buf, 10);
+  (void)renameat(AT_FDCWD, "oldpath", AT_FDCWD, "newpath");
+}
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1527,14 +1527,18 @@
   const RangeInt UCharRangeMax =
   std::min(BVF.getMaxValue(ACtx.UnsignedCharTy).getLimitedValue(), IntMax);
 
-  // The platform dependent value of EOF.
-  // Try our best to parse this from the Preprocessor, otherwise fallback to -1.
-  const auto EOFv = [&C]() -> RangeInt {
+  // Get platform dependent values of some macros.
+  // Try our best to parse this from the Preprocessor, otherwise fallback to a
+  // default value (what is found in a library header).
+  auto GetMacroValue = [&C](const char *Name, int Default) -> RangeInt {
 if (const std::optional OptInt =
-tryExpandAsInteger("EOF", C.getPreprocessor()))
+tryExpandAsInteger(Name, C.getPreprocessor()))
   return *OptInt;
-return -1;
-  }();
+return Default;
+  };
+
+  const auto EOFv = GetMacroValue("EOF", -1);
+  const auto AT_FDCWDv = GetMacroValue("AT_FDCWD", -100);
 
   // Auxiliary class to aid adding summaries to the summary map.
   struct AddToFunctionSummaryMap {
@@ -2130,6 +2134,7 @@
 Summary(NoEvalCall)
 .Case(ReturnsZero, ErrnoMustNotBeChecked)
 .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
+   

[PATCH] D149162: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & getAddrOfDeclareTargetVar into the OMPIRBuilder

2023-04-25 Thread Andrew Gozillon via Phabricator via cfe-commits
agozillon created this revision.
Herald added subscribers: sunshaoce, bzcheeseman, rriddle, rogfer01, guansong, 
hiraditya, yaxunl.
Herald added a reviewer: sscalpone.
Herald added a project: All.
agozillon requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: llvm-commits, cfe-commits, jplehr, sstefan1, 
stephenneuendorffer.
Herald added projects: clang, LLVM.

This change tries to move registerTargetglobalVariable and
getAddrOfDeclareTargetVar out of Clang's CGOpenMPRuntime
and into the OMPIRBuilder for shared use with MLIR's OpenMPDialect
and Flang (or other languages that may want to utilise it).

This primarily does this by trying to hoist the Clang specific
types into arguments or callback functions in the form of
lambdas, replacing it with LLVM equivelants and
tilising shared OMPIRBuilder enumerators for
the clauses, rather than Clang's own variation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149162

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

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -32,6 +32,7 @@
 #include "llvm/IR/Value.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
@@ -4999,7 +5000,8 @@
   static_cast(
   CE->getFlags());
   switch (Flags) {
-  case OffloadEntriesInfoManager::OMPTargetGlobalVarEntryTo: {
+  case OffloadEntriesInfoManager::OMPTargetGlobalVarEntryEnter:
+  case OffloadEntriesInfoManager::OMPTargetGlobalVarEntryTo:
 if (Config.isEmbedded() && Config.hasRequiresUnifiedSharedMemory())
   continue;
 if (!CE->getAddress()) {
@@ -5010,7 +5012,6 @@
 if (CE->getVarSize() == 0)
   continue;
 break;
-  }
   case OffloadEntriesInfoManager::OMPTargetGlobalVarEntryLink:
 assert(((Config.isEmbedded() && !CE->getAddress()) ||
 (!Config.isEmbedded() && CE->getAddress())) &&
@@ -5022,6 +5023,8 @@
   continue;
 }
 break;
+  default:
+  break;
   }
 
   // Hidden or internal symbols on the device are not externally visible.
@@ -5058,6 +5061,164 @@
   EntryInfo.Line, NewCount);
 }
 
+llvm::TargetRegionEntryInfo
+OpenMPIRBuilder::getTargetEntryUniqueInfo(StringRef FileName, uint64_t Line,
+  llvm::StringRef ParentName) {
+  llvm::sys::fs::UniqueID ID;
+  if (auto EC = llvm::sys::fs::getUniqueID(FileName, ID)) {
+assert(EC &&
+   "Unable to get unique ID for file, during getTargetEntryUniqueInfo");
+  }
+  return llvm::TargetRegionEntryInfo(ParentName, ID.getDevice(), ID.getFile(),
+ Line);
+}
+
+
+llvm::Constant *OpenMPIRBuilder::getAddrOfDeclareTargetVar(
+OffloadEntriesInfoManager::OMPTargetGlobalVarEntryKind CaptureClause,
+OffloadEntriesInfoManager::OMPTargetDeviceClauseKind DeviceClause,
+bool IsDeclaration, bool IsExternallyVisible, llvm::StringRef Filename,
+uint64_t Line, llvm::StringRef MangledName, llvm::Module *LlvmModule,
+std::vector &GeneratedRefs, bool OpenMPSIMD,
+bool OpenMPIsDevice, std::vector TargetTriple,
+llvm::Type *LlvmPtrTy, std::function GlobalInitializer,
+std::function VariableLinkage) {
+  // TODO: convert this to utilise the IRBuilder Config rather than
+  // a passed down argument.
+  if (OpenMPSIMD)
+return nullptr;
+
+  if (CaptureClause ==
+  OffloadEntriesInfoManager::OMPTargetGlobalVarEntryLink ||
+  ((CaptureClause ==
+OffloadEntriesInfoManager::OMPTargetGlobalVarEntryTo ||
+   CaptureClause ==
+OffloadEntriesInfoManager::OMPTargetGlobalVarEntryEnter) &&
+   Config.hasRequiresUnifiedSharedMemory())) {
+SmallString<64> PtrName;
+{
+  llvm::raw_svector_ostream OS(PtrName);
+  OS << MangledName;
+  if (!IsExternallyVisible) {
+auto EntryInfo = getTargetEntryUniqueInfo(Filename, Line);
+OS << llvm::format("_%x", EntryInfo.FileID);
+  }
+  OS << "_decl_tgt_ref_ptr";
+}
+
+llvm::Value *Ptr = LlvmModule->getNamedValue(PtrName);
+
+if (!Ptr) {
+  llvm::GlobalValue *GlobalValue = LlvmModule->getNamedValue(MangledName);
+  Ptr = getOrCreateInternalVariable(LlvmPtrTy, PtrName);
+
+  auto *GV = cast(Ptr);
+  GV->setLinkage(llvm::GlobalValue::WeakAnyLinkage);
+
+  if (!OpenMPIsDevice) {
+if (GlobalInitializer)
+  GV->setInitializer(GlobalInitializer());
+else
+  GV->setInitializer(GlobalValue);
+  }
+
+  register

[PATCH] D149162: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & getAddrOfDeclareTargetVar into the OMPIRBuilder

2023-04-25 Thread Andrew Gozillon via Phabricator via cfe-commits
agozillon added a comment.

I've tested this with the two following combinations, x86/AMDGPU and x86/NVPTX 
(newer plugin I think, not old, a little unsure on the runtime segment) and it 
passes the Clang test suites, if there is anything else I can do to make sure 
this is tested nicely and due diligence has been done please do mention it!

I admit the lengthy argument list is probably not the prettiest in the world, 
but the other option I could think of was an interface class of some kind that 
Clang+Flang could fill, or implement but that feels more like additional 
boilerplate than a fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149162

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


[PATCH] D149160: [clang][analyzer] Handle special value AT_FDCWD in affected standard functions

2023-04-25 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 516818.
balazske added a comment.

Fixed formatting problems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149160

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints.c

Index: clang/test/Analysis/std-c-library-functions-arg-constraints.c
===
--- clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -3,6 +3,7 @@
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
 // RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
 // RUN:   -analyzer-checker=debug.StdCLibraryFunctionsTester \
 // RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -triple x86_64-unknown-linux-gnu \
@@ -13,6 +14,7 @@
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
 // RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
 // RUN:   -analyzer-checker=debug.StdCLibraryFunctionsTester \
 // RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -triple x86_64-unknown-linux-gnu \
@@ -309,3 +311,26 @@
   // bugpath-warning{{The 1st argument to '__buf_size_arg_constraint_concrete' is out of the accepted range; It should be a buffer with size equal to or greater than 10}} \
   // bugpath-note{{The 1st argument to '__buf_size_arg_constraint_concrete' is out of the accepted range; It should be a buffer with size equal to or greater than 10}}
 }
+
+void test_file_fd_at_functions() {
+  (void)linkat(-22, "from", AT_FDCWD, "to", 0); // \
+  // report-warning{{The 1st argument to 'linkat' is -22 but should be -101 or >= 0}} \
+  // bugpath-warning{{The 1st argument to 'linkat' is -22 but should be -101 or >= 0}} \
+  // bugpath-note{{The 1st argument to 'linkat' is -22 but should be -101 or >= 0}}
+
+  // no warning for these functions if the AT_FDCWD value is used
+  (void)linkat(AT_FDCWD, "from", AT_FDCWD, "to", 0);
+  (void)faccessat(AT_FDCWD, "path", 0, 0);
+  (void)symlinkat("oldpath", AT_FDCWD, "newpath");
+  (void)mkdirat(AT_FDCWD, "path", 0);
+  (void)mknodat(AT_FDCWD, "path", 0, 0);
+  (void)fchmodat(AT_FDCWD, "path", 0, 0);
+  (void)fchownat(AT_FDCWD, "path", 0, 0, 0);
+  (void)linkat(AT_FDCWD, "oldpath", AT_FDCWD, "newpath", 0);
+  (void)unlinkat(AT_FDCWD, "newpath", 0);
+  struct stat St;
+  (void)fstatat(AT_FDCWD, "newpath", &St, 0);
+  char Buf[10];
+  (void)readlinkat(AT_FDCWD, "newpath", Buf, 10);
+  (void)renameat(AT_FDCWD, "oldpath", AT_FDCWD, "newpath");
+}
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1527,14 +1527,18 @@
   const RangeInt UCharRangeMax =
   std::min(BVF.getMaxValue(ACtx.UnsignedCharTy).getLimitedValue(), IntMax);
 
-  // The platform dependent value of EOF.
-  // Try our best to parse this from the Preprocessor, otherwise fallback to -1.
-  const auto EOFv = [&C]() -> RangeInt {
+  // Get platform dependent values of some macros.
+  // Try our best to parse this from the Preprocessor, otherwise fallback to a
+  // default value (what is found in a library header).
+  auto GetMacroValue = [&C](const char *Name, int Default) -> RangeInt {
 if (const std::optional OptInt =
-tryExpandAsInteger("EOF", C.getPreprocessor()))
+tryExpandAsInteger(Name, C.getPreprocessor()))
   return *OptInt;
-return -1;
-  }();
+return Default;
+  };
+
+  const auto EOFv = GetMacroValue("EOF", -1);
+  const auto AT_FDCWDv = GetMacroValue("AT_FDCWD", -100);
 
   // Auxiliary class to aid adding summaries to the summary map.
   struct AddToFunctionSummaryMap {
@@ -2130,6 +2134,8 @@
 Summary(NoEvalCall)
 .Case(ReturnsZero, ErrnoMustNotBeChecked)
 .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
+.ArgConstraint(ArgumentCondition(
+0, WithinRange, Range({AT_FDCWDv, AT_FDCWDv}, {0, IntMax})))
 .ArgConstraint(NotNull(ArgNo(1;
 
 // int dup(int fildes);
@@ -2207,7 +2213,8 @@
 .Case(ReturnsZero, ErrnoMustNotBeChecked)
 .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
 .ArgConstraint(NotNull(ArgNo(0)))
-.ArgConstraint(ArgumentCondition(1, WithinRange, Range(0, IntMax)))
+.ArgConstraint(ArgumentCondition(
+1, WithinRange, Range({AT_FDCWDv, AT_FDCWDv}, {0, IntMax})))
 .ArgConstraint(NotN

[clang] 1395cde - Fix codegen for initialization of global atomics

2023-04-25 Thread Aaron Ballman via cfe-commits

Author: Aaron Ballman
Date: 2023-04-25T11:37:06-04:00
New Revision: 1395cde24b3641e284bb1daae7d56c189a2635e3

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

LOG: Fix codegen for initialization of global atomics

This amends 2e275e24355cb224981f9beb2b026a3169fc7232. That commit added
a null to pointer cast kind when determining whether the expression can
be a valid constant initializer, but failed to update the constant
expression evaluator to perform the evaluation. This commit updates the
constant expression evaluator to handle that cast kind.

Added: 


Modified: 
clang/lib/AST/ExprConstant.cpp
clang/test/CodeGen/atomic.c

Removed: 




diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 6bfb3a37b243..49572b0f5869 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -14839,6 +14839,7 @@ class AtomicExprEvaluator :
 switch (E->getCastKind()) {
 default:
   return ExprEvaluatorBaseTy::VisitCastExpr(E);
+case CK_NullToPointer:
 case CK_NonAtomicToAtomic:
   return This ? EvaluateInPlace(Result, Info, *This, E->getSubExpr())
   : Evaluate(Result, Info, E->getSubExpr());

diff  --git a/clang/test/CodeGen/atomic.c b/clang/test/CodeGen/atomic.c
index f232d5b63ac7..69f06eedb7ca 100644
--- a/clang/test/CodeGen/atomic.c
+++ b/clang/test/CodeGen/atomic.c
@@ -1,5 +1,9 @@
 // RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686-apple-darwin9 | FileCheck %s
 
+// CHECK: @[[GLOB_POINTER:.+]] = internal global ptr null
+// CHECK: @[[GLOB_INT:.+]] = internal global i32 0
+// CHECK: @[[GLOB_FLT:.+]] = internal global float {{[0e\+-\.]+}}, align
+
 int atomic(void) {
   // non-sensical test for sync functions
   int old;
@@ -118,3 +122,19 @@ void addrspace(int  __attribute__((address_space(256))) * 
P) {
   __sync_xor_and_fetch(P, 123);
   // CHECK: atomicrmw xor ptr addrspace(256){{.*}}, i32 123 seq_cst, align 4
 }
+
+// Ensure that global initialization of atomics is correct.
+static _Atomic(int *) glob_pointer = (void *)0;
+static _Atomic int glob_int = 0;
+static _Atomic float glob_flt = 0.0f;
+
+void force_global_uses(void) {
+  (void)glob_pointer;
+  // CHECK: %[[LOCAL_INT:.+]] = load atomic i32, ptr @[[GLOB_POINTER]] seq_cst
+  // CHECK-NEXT: inttoptr i32 %[[LOCAL_INT]] to ptr
+  (void)glob_int;
+  // CHECK: load atomic i32, ptr @[[GLOB_INT]] seq_cst
+  (void)glob_flt;
+  // CHECK: %[[LOCAL_FLT:.+]] = load atomic i32, ptr @[[GLOB_FLT]] seq_cst
+  // CHECK-NEXT: bitcast i32 %[[LOCAL_FLT]] to float
+}



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


[PATCH] D148730: [C11] Allow initialization of an atomic-qualified pointer from a null pointer constant

2023-04-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D148730#4294781 , @bgraur wrote:

> We see the same compilation error as the one reported by @sbc100 in several 
> other C libraries.
>
> Here's a short reproducer: https://godbolt.org/z/Y8eds47na
>
> The tests imply that this should work. Please revert.

Thank you (both) for reporting this issue so quickly! I believe I've addressed 
it with 1395cde24b3641e284bb1daae7d56c189a2635e3 
, but if 
you're still having problems after that commit, let me know and I'll revert 
both changes to investigate further.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148730

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


[PATCH] D149154: [clang][Interp] Emit diagnostic when comparing function pointers

2023-04-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/Interp.h:652-653
+  const SourceInfo &Loc = S.Current->getSource(OpPC);
+  S.FFDiag(Loc, diag::note_constexpr_pointer_comparison_unspecified)
+  << LS << RS;
+  return false;

Can we pass in the result of `getType()` instead of doing this string 
conversion dance?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149154

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


[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

2023-04-25 Thread Paul Kirth via Phabricator via cfe-commits
paulkirth added a comment.

Thanks for the report Hans.

In D115907#4295363 , @hans wrote:

> We gave this a try in Chromium: 
> https://bugs.chromium.org/p/chromium/issues/detail?id=1434989
>
> While the diagnostics are interesting, two things make this less useful for 
> our developers:
>
> 1. It also fires in situations where the developer didn't set any 
> expectations, for example for static variable initialization guards where 
> clang implicitly adds branch weights (https://crbug.com/1434989#c7)

Hmm, I wasn't even aware that clang would insert `llvm.expect` without a user 
annotation. I'm also a bit surprised to see this crop up, since we haven't run 
into it on Fuchsia.

The problem is that we can't really distinguish those cases in IR... and even 
when we could, for IR based instrumentation we'd need to embed that into the 
branch weights somehow. D131306  explores a 
way to track the provenance of branch weights, so we could probably use that to 
distinguish them if we could also distinguish the `llvm.expect` usages the 
compiler inserts from user inserted ones. I put that work on hold, since it 
needs an RFC and I've been busy w/ other things, but I'll take some time in the 
next day or two to write that out and post the RFC to discord, since D131306 
 should solve a big part of that issue.

> 2. Due to inlining etc., it often gets the source locations wrong, which 
> means it points at code where again there were no expectations -- but perhaps 
> that code got inlined into an expectations somewhere else. (e.g. 
> https://crbug.com/1434989#c9)

The checking depends somewhat on the instrumentation type (frontend vs. backend 
instrumentation) In the case of frontend instrumentation, when the expect 
intrinsic is lowered (very early in the pipeline) we can report the diagnostic 
right away, since branch weights from profiling have already been attached. The 
//should// mean that you should get fairly accurate source information, since 
this happens before any optimizations run.

Backend instrumentation is more complicated, since we can't report the 
discrepancy until we're adding the branch weights to the IR. Whether inlining 
plays a role there or not is highly dependent on how you're compiling (i.e., no 
LTO, LTO, ThinLTO). The ordering is also somewhat dependent on if you're using 
Sampling or Instrumentation based profiles, but I believe both of those will 
always add weights before inlining. ThinLTO may be an outlier here, since I 
think weights from sampling based profiles can run multiple times in the 
ThinLTO pipeline.

> Especially 2) is probably not easy to fix. Do you have any tips on how 
> developers can use this more effectively?

When we report the diagnostic, we do our best to provide the source location, 
but that is only as good as what is available in the IR. I can certainly take a 
look at how we could improve reporting, but my recollection is that source 
information isn't always maintained well as the various passes run.If we're 
lucky in this case, we can maybe just tweak how its reporting the source 
location to include the inlining context. There's probably a bigger discussion 
that needs to be had about how to preserve debug and source location info 
through the backend passes.

The log also reported several places where expected hot code was never 
executed. If that isn't desirable, I'd also suggest that you could use the 
`-fdiagnostic-hotness-threshold=` to ignore reporting about branches that are 
not executed some minimum number of times. MisExpect is remarks based, so I 
beleive that is currently working. If not I'm happy to add that functionality.




Comment at: clang/include/clang/Driver/Options.td:1434
+def fdiagnostics_misexpect_tolerance_EQ : Joined<["-"], 
"fdiagnostics-misexpect-tolerance=">,
+Group, Flags<[CC1Option]>, MetaVarName<"">,
+HelpText<"Prevent misexpect diagnostics from being output if the profile 
counts are within N% of the expected. ">;

hans wrote:
> Should this be a driver mode option and not just cc1? The doc suggests using 
> it, which currently won't work without an `-Xclang` prefix.
That should also work from clang w/o `-Xclang`, shouldn't it? This is used 
exactly like `-fdiagnostics-hotness-threshold=`, so IIRC that should still work 
from clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115907

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


[PATCH] D147989: [clang] Fix Attribute Placement

2023-04-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

The issue is that `GetDiagnosticTypeSpecifierID()` is called for more 
diagnostics than just `warn_declspec_attribute_ignored`. You need to also 
update the `err_constexpr_tag` and `err_standalone_class_nested_name_specifier` 
to add `enum class` and `enum struct` as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147989

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


[PATCH] D149098: [Clang] Add tests and mark as implemented WG14 N2728 (char16_t & char32_t string literals shall be UTF-16 & UTF-32)

2023-04-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!




Comment at: clang/test/Lexer/char-literal.cpp:49
+#ifndef __cplusplus
+// expected-error@-2 {{universal character name refers to a control character}}
+#endif

cor3ntin wrote:
> I think these tests would be clearer with a different verify tag rather than 
> an ifdef, but it's kinda preexisting so feel free to ignore.
That'd be a nice cleanup for post-commit though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149098

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


[PATCH] D148372: [clang] add diagnose when member function contains invalid default argument

2023-04-25 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Are there any calls to `Sema::ActOnParamDefaultArgumentError` left? If not, can 
you also remove the function?

Otherwise this LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148372

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


[PATCH] D148987: [clang][Interp] Check Neg ops for errors

2023-04-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/Interp.h:438-441
+  // FIXME: This code Just Works[tm] for floats, but it's probably not doing
+  //   the right thing. At least the diagnostic could be better without
+  //   the conversion to an APInt.
+

I'm confused -- why would negation be UB for a floating-point type? For integer 
types, it's a matter of not being able to represent the value when the source 
is `INT_MIN`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148987

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


[PATCH] D148869: [Driver][BareMetal] Support --emit-static-lib in BareMetal driver

2023-04-25 Thread Alex Brachet via Phabricator via cfe-commits
abrachet accepted this revision.
abrachet added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Driver/ToolChains/BareMetal.cpp:170
 bool BareMetal::handlesTarget(const llvm::Triple &Triple) {
   return isARMBareMetal(Triple) || isAArch64BareMetal(Triple) ||
  isRISCVBareMetal(Triple);

Looks like clang-format is complaining about this file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148869

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


[PATCH] D149163: [NFC][CLANG] Fix coverity remarks about large copy by values

2023-04-25 Thread Soumi Manna via Phabricator via cfe-commits
Manna created this revision.
Manna added a reviewer: tahonermann.
Herald added a subscriber: arphaman.
Herald added a project: All.
Manna requested review of this revision.
Herald added a project: clang.

Reported By Static Analyzer Tool, Coverity:

Big parameter passed by value
Copying large values is inefficient, consider passing by reference; Low, 
medium, and high size thresholds for detection can be adjusted.

1. Inside "CodeGenModule.cpp" file, in 
clang::​CodeGen::​CodeGenModule::​EmitBackendOptionsMetadata(clang::​CodeGenOptions):
 A very large function call parameter exceeding the high threshold is passed by 
value.

pass_by_value: Passing parameter CodeGenOpts of type clang::CodeGenOptions 
const (size 2168 bytes) by value, which exceeds the high threshold of 512 bytes.

2. Inside "CodeCompleteConsumer.cpp" file, in 
clang::​PrintingCodeCompleteConsumer::​ProcessCodeCompleteResults(clang::​Sema 
&, clang::​CodeCompletionContext, clang::​CodeCompletionResult *, unsigned 
int): A large function call parameter exceeding the low threshold is passed by 
value.

pass_by_value: Passing parameter Context of type clang::CodeCompletionContext 
(size 200 bytes) by value, which exceeds the low threshold of 128 bytes.

3. Inside "CodeCompleteConsumer.h" file, in 
clang::​CodeCompleteConsumer::​ProcessCodeCompleteResults(clang::​Sema &, 
clang::​CodeCompletionContext, clang::​CodeCompletionResult *, unsigned int): A 
large function call parameter exceeding the low threshold is passed by value.

pass_by_value: Passing parameter Context of type clang::CodeCompletionContext 
(size 200 bytes) by value, which exceeds the low threshold of 128 bytes.

4. Inside "ASTUnit.cpp" file, in 
::​AugmentedCodeCompleteConsumer::​ProcessCodeCompleteResults(clang::​Sema
 &, clang::​CodeCompletionContext, clang::​CodeCompletionResult *, unsigned 
int): A large function call parameter exceeding the low threshold is passed by 
value.

pass_by_value: Passing parameter Context of type clang::CodeCompletionContext 
(size 200 bytes) by value, which exceeds the low threshold of 128 bytes.

5. Inside "SemaType.cpp" file, in IsNoDerefableChunk(clang::​DeclaratorChunk): 
A large function call parameter exceeding the low threshold is passed by value.

pass_by_value: Passing parameter Chunk of type clang::DeclaratorChunk (size 176 
bytes) by value, which exceeds the low threshold of 128 bytes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149163

Files:
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/SemaType.cpp
  clang/tools/libclang/CIndexCodeCompletion.cpp
  clang/unittests/Sema/CodeCompleteTest.cpp

Index: clang/unittests/Sema/CodeCompleteTest.cpp
===
--- clang/unittests/Sema/CodeCompleteTest.cpp
+++ clang/unittests/Sema/CodeCompleteTest.cpp
@@ -54,7 +54,7 @@
 CompletedFuncDecls(CompletedFuncDecls),
 CCTUInfo(std::make_shared()) {}
 
-  void ProcessCodeCompleteResults(Sema &S, CodeCompletionContext Context,
+  void ProcessCodeCompleteResults(Sema &S, const CodeCompletionContext &Context,
   CodeCompletionResult *Results,
   unsigned NumResults) override {
 for (unsigned I = 0; I < NumResults; ++I) {
@@ -89,7 +89,7 @@
   : CodeCompleteConsumer(/*CodeCompleteOpts=*/{}), ResultCtx(ResultCtx),
 CCTUInfo(std::make_shared()) {}
 
-  void ProcessCodeCompleteResults(Sema &S, CodeCompletionContext Context,
+  void ProcessCodeCompleteResults(Sema &S, const CodeCompletionContext &Context,
   CodeCompletionResult *Results,
   unsigned NumResults) override {
 ResultCtx.VisitedNamespaces =
Index: clang/tools/libclang/CIndexCodeCompletion.cpp
===
--- clang/tools/libclang/CIndexCodeCompletion.cpp
+++ clang/tools/libclang/CIndexCodeCompletion.cpp
@@ -572,8 +572,8 @@
   CCTUInfo(Results.CodeCompletionAllocator), TU(TranslationUnit) {}
 ~CaptureCompletionResults() override { Finish(); }
 
-void ProcessCodeCompleteResults(Sema &S, 
-CodeCompletionContext Context,
+void ProcessCodeCompleteResults(Sema &S,
+const CodeCompletionContext &Context,
 CodeCompletionResult *Results,
 unsigned NumResults) override {
   StoredResults.reserve(StoredResults.size() + NumResults);
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4553,7 +4553,7 @@
   return false;
 }
 
-static bool IsNoDerefableChunk(DeclaratorChunk

[PATCH] D149165: [clangd] Deduplicate missing-include findings

2023-04-25 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision.
kadircet added a reviewer: VitaNuo.
Herald added a subscriber: arphaman.
Herald added a project: All.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149165

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -403,15 +403,34 @@
   ParsedAST AST = TU.build();
 
   auto Findings = computeIncludeCleanerFindings(AST).MissingIncludes;
-  // FIXME: Deduplicate references resulting from expansion of the same macro 
in
-  // multiple places.
-  EXPECT_THAT(Findings, testing::SizeIs(2));
+  EXPECT_THAT(Findings, testing::SizeIs(1));
   auto RefRange = Findings.front().SymRefRange;
   auto &SM = AST.getSourceManager();
   EXPECT_EQ(RefRange.file(), SM.getMainFileID());
   // FIXME: Point at the spelling location, rather than the include.
   EXPECT_EQ(halfOpenToRange(SM, RefRange.toCharRange(SM)), MainFile.range());
-  EXPECT_EQ(RefRange, Findings[1].SymRefRange);
+}
+
+TEST(IncludeCleaner, MissingIncludesAreUnique) {
+  Annotations MainFile(R"cpp(
+#include "all.h"
+FOO([[Foo]]);)cpp");
+
+  TestTU TU;
+  TU.AdditionalFiles["foo.h"] = guard("struct Foo {};");
+  TU.AdditionalFiles["all.h"] = guard(R"(
+#include "foo.h"
+#define FOO(X) X y; X z)");
+
+  TU.Code = MainFile.code();
+  ParsedAST AST = TU.build();
+
+  auto Findings = computeIncludeCleanerFindings(AST).MissingIncludes;
+  EXPECT_THAT(Findings, testing::SizeIs(1));
+  auto RefRange = Findings.front().SymRefRange;
+  auto &SM = AST.getSourceManager();
+  EXPECT_EQ(RefRange.file(), SM.getMainFileID());
+  EXPECT_EQ(halfOpenToRange(SM, RefRange.toCharRange(SM)), MainFile.range());
 }
 
 TEST(IncludeCleaner, NoCrash) {
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -40,6 +40,7 @@
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/GenericUniformityImpl.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include "llvm/ADT/SmallString.h"
@@ -415,6 +416,20 @@
 Ref.Target, TouchingTokens.back().range(SM), Providers};
 MissingIncludes.push_back(std::move(DiagInfo));
   });
+  // Put possibly equal diagnostics together for deduplication.
+  // The duplicates might be from macro arguments that get expanded multiple
+  // times.
+  llvm::stable_sort(MissingIncludes, [](const MissingIncludeDiagInfo &LHS,
+const MissingIncludeDiagInfo &RHS) {
+if (LHS.Symbol == RHS.Symbol) {
+  // We can get away just by comparing the offsets as all the ranges are in
+  // main file.
+  return LHS.SymRefRange.beginOffset() < RHS.SymRefRange.beginOffset();
+}
+// If symbols are different we don't care about the ordering.
+return true;
+  });
+  MissingIncludes.erase(llvm::unique(MissingIncludes), MissingIncludes.end());
   std::vector UnusedIncludes =
   getUnused(AST, Used, /*ReferencedPublicHeaders*/ {});
   return {std::move(UnusedIncludes), std::move(MissingIncludes)};


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -403,15 +403,34 @@
   ParsedAST AST = TU.build();
 
   auto Findings = computeIncludeCleanerFindings(AST).MissingIncludes;
-  // FIXME: Deduplicate references resulting from expansion of the same macro in
-  // multiple places.
-  EXPECT_THAT(Findings, testing::SizeIs(2));
+  EXPECT_THAT(Findings, testing::SizeIs(1));
   auto RefRange = Findings.front().SymRefRange;
   auto &SM = AST.getSourceManager();
   EXPECT_EQ(RefRange.file(), SM.getMainFileID());
   // FIXME: Point at the spelling location, rather than the include.
   EXPECT_EQ(halfOpenToRange(SM, RefRange.toCharRange(SM)), MainFile.range());
-  EXPECT_EQ(RefRange, Findings[1].SymRefRange);
+}
+
+TEST(IncludeCleaner, MissingIncludesAreUnique) {
+  Annotations MainFile(R"cpp(
+#include "all.h"
+FOO([[Foo]]);)cpp");
+
+  TestTU TU;
+  TU.AdditionalFiles["foo.h"] = guard("struct Foo {};");
+  TU.AdditionalFiles["all.h"] = guard(R"(
+#include "foo.h"
+#define FOO(X) X y; X z)");
+
+  TU.Code = MainFile.code();
+  ParsedAST AST = TU.build();
+
+  auto Findings = computeIncludeCleanerFindings(AS

[PATCH] D149163: [NFC][CLANG] Fix coverity remarks about large copy by values

2023-04-25 Thread Soumi Manna via Phabricator via cfe-commits
Manna updated this revision to Diff 516834.

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

https://reviews.llvm.org/D149163

Files:
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/lib/CodeGen/CGGPUBuiltin.cpp
  clang/lib/CodeGen/CGNonTrivialStruct.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/SemaType.cpp
  clang/tools/libclang/CIndexCodeCompletion.cpp
  clang/unittests/Sema/CodeCompleteTest.cpp

Index: clang/unittests/Sema/CodeCompleteTest.cpp
===
--- clang/unittests/Sema/CodeCompleteTest.cpp
+++ clang/unittests/Sema/CodeCompleteTest.cpp
@@ -54,7 +54,7 @@
 CompletedFuncDecls(CompletedFuncDecls),
 CCTUInfo(std::make_shared()) {}
 
-  void ProcessCodeCompleteResults(Sema &S, CodeCompletionContext Context,
+  void ProcessCodeCompleteResults(Sema &S, const CodeCompletionContext &Context,
   CodeCompletionResult *Results,
   unsigned NumResults) override {
 for (unsigned I = 0; I < NumResults; ++I) {
@@ -89,7 +89,7 @@
   : CodeCompleteConsumer(/*CodeCompleteOpts=*/{}), ResultCtx(ResultCtx),
 CCTUInfo(std::make_shared()) {}
 
-  void ProcessCodeCompleteResults(Sema &S, CodeCompletionContext Context,
+  void ProcessCodeCompleteResults(Sema &S, const CodeCompletionContext &Context,
   CodeCompletionResult *Results,
   unsigned NumResults) override {
 ResultCtx.VisitedNamespaces =
Index: clang/tools/libclang/CIndexCodeCompletion.cpp
===
--- clang/tools/libclang/CIndexCodeCompletion.cpp
+++ clang/tools/libclang/CIndexCodeCompletion.cpp
@@ -572,8 +572,8 @@
   CCTUInfo(Results.CodeCompletionAllocator), TU(TranslationUnit) {}
 ~CaptureCompletionResults() override { Finish(); }
 
-void ProcessCodeCompleteResults(Sema &S, 
-CodeCompletionContext Context,
+void ProcessCodeCompleteResults(Sema &S,
+const CodeCompletionContext &Context,
 CodeCompletionResult *Results,
 unsigned NumResults) override {
   StoredResults.reserve(StoredResults.size() + NumResults);
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4553,7 +4553,7 @@
   return false;
 }
 
-static bool IsNoDerefableChunk(DeclaratorChunk Chunk) {
+static bool IsNoDerefableChunk(const DeclaratorChunk &Chunk) {
   return (Chunk.Kind == DeclaratorChunk::Pointer ||
   Chunk.Kind == DeclaratorChunk::Array);
 }
Index: clang/lib/Sema/CodeCompleteConsumer.cpp
===
--- clang/lib/Sema/CodeCompleteConsumer.cpp
+++ clang/lib/Sema/CodeCompleteConsumer.cpp
@@ -638,8 +638,8 @@
 }
 
 void PrintingCodeCompleteConsumer::ProcessCodeCompleteResults(
-Sema &SemaRef, CodeCompletionContext Context, CodeCompletionResult *Results,
-unsigned NumResults) {
+Sema &SemaRef, const CodeCompletionContext &Context,
+CodeCompletionResult *Results, unsigned NumResults) {
   std::stable_sort(Results, Results + NumResults);
 
   if (!Context.getPreferredType().isNull())
Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -1952,7 +1952,8 @@
|  (1LL << CodeCompletionContext::CCC_ClassOrStructTag);
 }
 
-void ProcessCodeCompleteResults(Sema &S, CodeCompletionContext Context,
+void ProcessCodeCompleteResults(Sema &S,
+const CodeCompletionContext &Context,
 CodeCompletionResult *Results,
 unsigned NumResults) override;
 
@@ -2063,10 +2064,9 @@
   }
 }
 
-void AugmentedCodeCompleteConsumer::ProcessCodeCompleteResults(Sema &S,
-CodeCompletionContext Context,
-CodeCompletionResult *Results,
-unsigned NumResults) {
+void AugmentedCodeCompleteConsumer::ProcessCodeCompleteResults(
+Sema &S, const CodeCompletionContext &Context,
+CodeCompletionResult *Results, unsigned NumResults) {
   // Merge the results we were given with the results we cached.
   bool AddedResult = false;
   uint64_t InContexts =
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1711,7 +1711,7 @@

[PATCH] D149098: [Clang] Add tests and mark as implemented WG14 N2728 (char16_t & char32_t string literals shall be UTF-16 & UTF-32)

2023-04-25 Thread Tom Honermann via Phabricator via cfe-commits
tahonermann updated this revision to Diff 516835.
tahonermann added a comment.

Thank you @cor3ntin, that was an excellent suggestion; this is much more 
readable now! I updated the new code to use custom verify tags (I left the 
existing code alone).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149098

Files:
  clang/test/Lexer/char-literal.cpp
  clang/www/c_status.html

Index: clang/www/c_status.html
===
--- clang/www/c_status.html
+++ clang/www/c_status.html
@@ -929,7 +929,7 @@
 
   char16_t & char32_t string literals shall be UTF-16 & UTF-32
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2728.htm";>N2728
-  Unknown
+  Yes
 
 
   IEC 60559 binding
Index: clang/test/Lexer/char-literal.cpp
===
--- clang/test/Lexer/char-literal.cpp
+++ clang/test/Lexer/char-literal.cpp
@@ -1,5 +1,8 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -Wfour-char-constants -fsyntax-only -verify %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c11 -x c -Wfour-char-constants -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -Wfour-char-constants -fsyntax-only -verify=cxx,expected %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++17 -Wfour-char-constants -fsyntax-only -verify=cxx,expected %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++20 -Wfour-char-constants -fsyntax-only -verify=cxx,expected %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c11 -x c -Wfour-char-constants -fsyntax-only -verify=c,expected %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c2x -x c -Wfour-char-constants -fsyntax-only -verify=c,expected %s
 
 #ifndef __cplusplus
 typedef __WCHAR_TYPE__ wchar_t;
@@ -38,3 +41,81 @@
 #ifdef __cplusplus
 // expected-error@-2 {{too long}}
 #endif
+
+// UTF-8 character literal code point ranges.
+#if __cplusplus >= 201703L || __STDC_VERSION__ >= 201710L
+_Static_assert(u8'\U' == 0x00, ""); // c-error {{universal character name refers to a control character}}
+_Static_assert(u8'\U007F' == 0x7F, ""); // c-error {{universal character name refers to a control character}}
+_Static_assert(u8'\U0080', ""); // c-error {{universal character name refers to a control character}}
+// cxx-error@-1 {{character too large for enclosing character literal type}}
+_Static_assert((unsigned char)u8'\xFF' == (unsigned char)0xFF, "");
+#endif
+
+// UTF-8 string literal code point ranges.
+_Static_assert(u8"\U"[0] == 0x00, ""); // c-error {{universal character name refers to a control character}}
+_Static_assert(u8"\U007F"[0] == 0x7F, ""); // c-error {{universal character name refers to a control character}}
+_Static_assert((unsigned char)u8"\U0080"[0] == (unsigned char)0xC2, ""); // c-error {{universal character name refers to a control character}}
+_Static_assert((unsigned char)u8"\U0080"[1] == (unsigned char)0x80, ""); // c-error {{universal character name refers to a control character}}
+_Static_assert((unsigned char)u8"\U07FF"[0] == (unsigned char)0xDF, "");
+_Static_assert((unsigned char)u8"\U07FF"[1] == (unsigned char)0xBF, "");
+_Static_assert((unsigned char)u8"\U0800"[0] == (unsigned char)0xE0, "");
+_Static_assert((unsigned char)u8"\U0800"[1] == (unsigned char)0xA0, "");
+_Static_assert((unsigned char)u8"\U0800"[2] == (unsigned char)0x80, "");
+_Static_assert(u8"\UD800"[0], ""); // expected-error {{invalid universal character}}
+_Static_assert(u8"\UDFFF"[0], ""); // expected-error {{invalid universal character}}
+_Static_assert((unsigned char)u8"\U"[0] == (unsigned char)0xEF, "");
+_Static_assert((unsigned char)u8"\U"[1] == (unsigned char)0xBF, "");
+_Static_assert((unsigned char)u8"\U"[2] == (unsigned char)0xBF, "");
+_Static_assert((unsigned char)u8"\U0001"[0] == (unsigned char)0xF0, "");
+_Static_assert((unsigned char)u8"\U0001"[1] == (unsigned char)0x90, "");
+_Static_assert((unsigned char)u8"\U0001"[2] == (unsigned char)0x80, "");
+_Static_assert((unsigned char)u8"\U0001"[3] == (unsigned char)0x80, "");
+_Static_assert((unsigned char)u8"\U0010"[0] == (unsigned char)0xF4, "");
+_Static_assert((unsigned char)u8"\U0010"[1] == (unsigned char)0x8F, "");
+_Static_assert((unsigned char)u8"\U0010"[2] == (unsigned char)0xBF, "");
+_Static_assert((unsigned char)u8"\U0010"[3] == (unsigned char)0xBF, "");
+_Static_assert(u8"\U0011"[0], ""); // expected-error {{invalid universal character}}
+
+#if !defined(__STDC_UTF_16__)
+#error __STDC_UTF_16__ is not defined.
+#endif
+#if __STDC_UTF_16__ != 1
+#error __STDC_UTF_16__ has the wrong value.
+#endif
+
+// UTF-16 character literal code point ranges.
+_Static_assert(u'\U' == 0x, ""); // c-error

[PATCH] D142907: LangRef: Add "dynamic" option to "denormal-fp-math"

2023-04-25 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

Oh, sorry, I missed that the new code specifically runs on functions imported 
using -mlink-builtin-bitcode.  I somehow thought it was running on all 
functions.

LGTM


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

https://reviews.llvm.org/D142907

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


[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

2023-04-25 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added a comment.
Herald added a subscriber: hoy.

Ping for review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451

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


[PATCH] D141451: [clang] report inlining decisions with -Wattribute-{warning|error}

2023-04-25 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision.
erichkeane added a comment.
This revision is now accepted and ready to land.

Sorry, I didn't see your updates I guess!  Thanks for the yak-shave btw, I 
realize that has turned into a heck of a mess :/

CFE LGTM.  I don't believe I can approve the LLVM components however.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141451

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


[PATCH] D148987: [clang][Interp] Check Neg ops for errors

2023-04-25 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/Interp.h:438-441
+  // FIXME: This code Just Works[tm] for floats, but it's probably not doing
+  //   the right thing. At least the diagnostic could be better without
+  //   the conversion to an APInt.
+

aaron.ballman wrote:
> I'm confused -- why would negation be UB for a floating-point type? For 
> integer types, it's a matter of not being able to represent the value when 
> the source is `INT_MIN`.
No idea, this was just something that came to mind when I wrote the code. If 
it's not an issue, that's even better can I can just remove the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148987

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


[PATCH] D148723: [clang] Enforce external linkage for inline builtin original declaration

2023-04-25 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

I'm a little concerned that this will explode in unexpected ways... in 
particular, it'll fail to link if the function doesn't actually exist 
externally.  Which it probably doesn't if it would otherwise be linkonce_odr.


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

https://reviews.llvm.org/D148723

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


[PATCH] D148723: [clang] Enforce external linkage for inline builtin original declaration

2023-04-25 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

Really, I'd prefer to keep isInlineBuiltinDeclaration() targeted as narrowly as 
possible; part of that is making it not trigger for C++ inline functions (which 
it never did in the past).


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

https://reviews.llvm.org/D148723

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


[PATCH] D142401: [Clang] Fix a crash when recursively callig a default member initializer.

2023-04-25 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik accepted this revision.
shafik added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142401

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


[PATCH] D149154: [clang][Interp] Emit diagnostic when comparing function pointers

2023-04-25 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/Interp.h:652-653
+  const SourceInfo &Loc = S.Current->getSource(OpPC);
+  S.FFDiag(Loc, diag::note_constexpr_pointer_comparison_unspecified)
+  << LS << RS;
+  return false;

aaron.ballman wrote:
> Can we pass in the result of `getType()` instead of doing this string 
> conversion dance?
Well the diagnostic doesn't print the result of the LHS/RHS:
```
./array.cpp:202:18: error: constexpr variable 'u13' must be initialized by a 
constant expression
  202 |   constexpr bool u13 = pf < pg; // ref-warning {{ordered comparison of 
function pointers}}
  |  ^ ~~~
./array.cpp:202:27: note: comparison between '&f' and '&g' has unspecified value
  202 |   constexpr bool u13 = pf < pg; // ref-warning {{ordered comparison of 
function pointers}}
  |   ^
```

I'm not exactly a fan of how the code looks though. I might add a helper 
function for this later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149154

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


[PATCH] D72538: [ThinLTO] Add additional ThinLTO pipeline testing with new PM

2023-04-25 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson added a comment.

In D72538#4291552 , @nikic wrote:

> Would it be possible to cut down the Clang side tests to only check parts 
> that Clang controls in some way, e.g. that SLPVectorizer is enabled? This 
> test is something of a PITA because it tests LLVM implementation details but 
> is not part of the LLVM tests, so it usually gets missed when doing pipeline 
> changes.

I think we can cut this down significantly, since the llvm tests added here 
check the full ThinLTO default pipeline setup at different opt levels. However, 
the point of the clang test is to make sure that for a distributed ThinLTO 
backend we correctly invoke thinBackend() which should set up the ThinLTO 
default pipeline. So how about I cut this down to check a single pass that is 
only invoked in the LTO backends? Specifically, I'm thinking of 
WholeProgramDevirtPass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72538

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


[PATCH] D149123: [AArch64][InlineAsm]Add Clang support for flag output constraints

2023-04-25 Thread David Li via Phabricator via cfe-commits
davidxl added inline comments.



Comment at: clang/lib/Basic/Targets/AArch64.cpp:1216
+// Returns the length of cc constraint.
+static unsigned matchAsmCCConstraint(const char *&Name) {
+  constexpr unsigned len = 5;

Name is not modified in this method, so perhaps dropping '&'?


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

https://reviews.llvm.org/D149123

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


[PATCH] D149172: [clang][Interp] Emit proper diagnostic when comparing unrelated pointers

2023-04-25 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Also adds `toDiagnosticString()` to `Pointer` so we can use that when emitting 
the diagnostic.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149172

Files:
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Pointer.cpp
  clang/lib/AST/Interp/Pointer.h
  clang/test/AST/Interp/literals.cpp


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -125,6 +125,35 @@
   static_assert(!!FP, "");
 }
 
+namespace PointerComparison {
+
+  struct S { int a, b; } s;
+  constexpr void *null = 0;
+  constexpr void *pv = (void*)&s.a;
+  constexpr void *qv = (void*)&s.b;
+  constexpr bool v1 = null < (int*)0;
+  constexpr bool v2 = null < pv; // expected-error {{must be initialized by a 
constant expression}} \
+ // expected-note {{comparison between 
'nullptr' and '&s.a' has unspecified value}} \
+ // ref-error {{must be initialized by a 
constant expression}} \
+ // ref-note {{comparison between 'nullptr' 
and '&s.a' has unspecified value}} \
+
+  constexpr bool v3 = null == pv; // ok
+  constexpr bool v4 = qv == pv; // ok
+
+  /// FIXME: These two are rejected by the current interpreter, but
+  ///   accepted by GCC.
+  constexpr bool v5 = qv >= pv; // ref-error {{constant expression}} \
+// ref-note {{unequal pointers to void}}
+  constexpr bool v8 = qv > (void*)&s.a; // ref-error {{constant expression}} \
+// ref-note {{unequal pointers to 
void}}
+  constexpr bool v6 = qv > null; // expected-error {{must be initialized by a 
constant expression}} \
+ // expected-note {{comparison between '&s.b' 
and 'nullptr' has unspecified value}} \
+ // ref-error {{must be initialized by a 
constant expression}} \
+ // ref-note {{comparison between '&s.b' and 
'nullptr' has unspecified value}}
+
+  constexpr bool v7 = qv <= (void*)&s.b; // ok
+}
+
 namespace SizeOf {
   constexpr int soint = sizeof(int);
   constexpr int souint = sizeof(unsigned int);
Index: clang/lib/AST/Interp/Pointer.h
===
--- clang/lib/AST/Interp/Pointer.h
+++ clang/lib/AST/Interp/Pointer.h
@@ -79,6 +79,9 @@
   /// Converts the pointer to an APValue.
   APValue toAPValue() const;
 
+  /// Converts the pointer to a string usable in diagnostics.
+  std::string toDiagnosticString(const ASTContext &Ctx) const;
+
   /// Converts the pointer to an APValue that is an rvalue.
   APValue toRValue(const Context &Ctx) const;
 
Index: clang/lib/AST/Interp/Pointer.cpp
===
--- clang/lib/AST/Interp/Pointer.cpp
+++ clang/lib/AST/Interp/Pointer.cpp
@@ -150,6 +150,13 @@
   return APValue(Base, Offset, Path, IsOnePastEnd, IsNullPtr);
 }
 
+std::string Pointer::toDiagnosticString(const ASTContext &Ctx) const {
+  if (!Pointee)
+return "nullptr";
+
+  return toAPValue().getAsString(Ctx, getType());
+}
+
 bool Pointer::isInitialized() const {
   assert(Pointee && "Cannot check if null pointer was initialized");
   Descriptor *Desc = getFieldDesc();
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -671,7 +671,9 @@
 
   if (!Pointer::hasSameBase(LHS, RHS)) {
 const SourceInfo &Loc = S.Current->getSource(OpPC);
-S.FFDiag(Loc, diag::note_invalid_subexpr_in_const_expr);
+S.FFDiag(Loc, diag::note_constexpr_pointer_comparison_unspecified)
+<< LHS.toDiagnosticString(S.getCtx())
+<< RHS.toDiagnosticString(S.getCtx());
 return false;
   } else {
 unsigned VL = LHS.getByteOffset();


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -125,6 +125,35 @@
   static_assert(!!FP, "");
 }
 
+namespace PointerComparison {
+
+  struct S { int a, b; } s;
+  constexpr void *null = 0;
+  constexpr void *pv = (void*)&s.a;
+  constexpr void *qv = (void*)&s.b;
+  constexpr bool v1 = null < (int*)0;
+  constexpr bool v2 = null < pv; // expected-error {{must be initialized by a constant expression}} \
+ // expected-note {{comparison between 'nullptr' and '&s.a' has unspecified value}} \
+ // ref-error {{must be initialized by a constant expression}} \
+ 

[PATCH] D148975: -fdebug-prefix-map=: make the last win when multiple prefixes match

2023-04-25 Thread Scott Linder via Phabricator via cfe-commits
scott.linder added inline comments.



Comment at: clang/include/clang/Basic/CodeGenOptions.h:209
 
-  std::map DebugPrefixMap;
+  llvm::SmallVector, 0> DebugPrefixMap;
   std::map CoveragePrefixMap;

MaskRay wrote:
> scott.linder wrote:
> > What benefit does forcing allocation have? Why not use the default, or 
> > switch to `std::vector`?
> This doesn't force allocation. I use inline element size 0 to make the member 
> variable smaller than a `std::vector`.
> `std::vector` likely compiles to larger code.
Sorry, I just didn't realize this was idiomatic (i.e. I hadn't read 
https://llvm.org/docs/ProgrammersManual.html#vector), and I didn't see other 
similar uses in the file.

There are 15 `std::vector` members of `CodeGenOptions`, but no `SmallVector<_, 
0>` members; maybe the rest can be converted in an NFC patch, so things are 
consistent?



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:72-79
 CGDebugInfo::CGDebugInfo(CodeGenModule &CGM)
 : CGM(CGM), DebugKind(CGM.getCodeGenOpts().getDebugInfo()),
   DebugTypeExtRefs(CGM.getCodeGenOpts().DebugTypeExtRefs),
   DBuilder(CGM.getModule()) {
-  for (const auto &KV : CGM.getCodeGenOpts().DebugPrefixMap)
-DebugPrefixMap[KV.first] = KV.second;
+  for (const auto &[From, To] : CGM.getCodeGenOpts().DebugPrefixMap)
+DebugPrefixMap.emplace_back(From, To);
   CreateCompileUnit();

MaskRay wrote:
> scott.linder wrote:
> > Can you use the member-initializer-list here?
> No, this doesn't work. We convert a vector of `std::string` to a vector of 
> `StringRef`.
If all we are doing is creating another vector which shares the underlying 
strings with the original, why not just save a reference to the original 
vector? Is the cost of the extra dereference when accessing it greater than the 
cost of traversing it plus the extra storage for the `StringRef`s?

It seems like the original utility was just to get the `std::map` sorting 
behavior, which we no longer need.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148975

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


[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

2023-04-25 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

In D115907#4295923 , @paulkirth wrote:

>> 2. Due to inlining etc., it often gets the source locations wrong, which 
>> means it points at code where again there were no expectations -- but 
>> perhaps that code got inlined into an expectations somewhere else. (e.g. 
>> https://crbug.com/1434989#c9)
>
> The checking depends somewhat on the instrumentation type (frontend vs. 
> backend instrumentation) In the case of frontend instrumentation, when the 
> expect intrinsic is lowered (very early in the pipeline) we can report the 
> diagnostic right away, since branch weights from profiling have already been 
> attached. The //should// mean that you should get fairly accurate source 
> information, since this happens before any optimizations run.

We use the backend instrumentation since that gives the best performance, but 
it's good to hear that the locations may be more accurate with frontend 
instrumentation. Maybe we should add that to the docs?

> The log also reported several places where expected hot code was never 
> executed. If that isn't desirable, I'd also suggest that you could use the 
> `-fdiagnostic-hotness-threshold=` to ignore reporting about branches that are 
> not executed some minimum number of times. MisExpect is remarks based, so I 
> beleive that is currently working. If not I'm happy to add that functionality.

Oh nice, I will try that flag.




Comment at: clang/include/clang/Driver/Options.td:1434
+def fdiagnostics_misexpect_tolerance_EQ : Joined<["-"], 
"fdiagnostics-misexpect-tolerance=">,
+Group, Flags<[CC1Option]>, MetaVarName<"">,
+HelpText<"Prevent misexpect diagnostics from being output if the profile 
counts are within N% of the expected. ">;

paulkirth wrote:
> hans wrote:
> > Should this be a driver mode option and not just cc1? The doc suggests 
> > using it, which currently won't work without an `-Xclang` prefix.
> That should also work from clang w/o `-Xclang`, shouldn't it? This is used 
> exactly like `-fdiagnostics-hotness-threshold=`, so IIRC that should still 
> work from clang.
Hmm, if I add `-fdiagnostics-misexpect-tolerance=10` I get errors about the 
flag being unused. I had to use `-Xclang` for it to work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115907

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


[PATCH] D148757: [clang] Apply last -fcoverage-prefix-map option

2023-04-25 Thread Gulfem Savrun Yeniceri via Phabricator via cfe-commits
gulfem updated this revision to Diff 516853.
gulfem added a comment.

Updated the text.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148757

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/lib/CodeGen/CoverageMappingGen.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/coverage-prefix-map.c


Index: clang/test/Profile/coverage-prefix-map.c
===
--- clang/test/Profile/coverage-prefix-map.c
+++ clang/test/Profile/coverage-prefix-map.c
@@ -19,3 +19,13 @@
 
 // RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm 
-mllvm -enable-name-compression=false -main-file-name coverage-prefix-map.c 
%t/root/nested/coverage-prefix-map.c -fcoverage-compilation-dir=/custom 
-fcoverage-prefix-map=/custom=/nonsense -o - | FileCheck 
--check-prefix=COVERAGE-COMPILATION-DIR %s
 // COVERAGE-COMPILATION-DIR: @__llvm_coverage_mapping = {{.*"\\02.*}}nonsense
+
+// Test that last -fcoverage-prefix-map option 
(-fcoverage-prefix-map==newpath) is applied.
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm 
-mllvm -enable-name-compression=false -main-file-name coverage-prefix-map.c 
%t/root/nested/coverage-prefix-map.c -fcoverage-prefix-map=%/t/root=. 
-fcoverage-prefix-map==newpath -o - | FileCheck 
--check-prefix=COVERAGE-PREFIX-MAP-ORDER %s
+// COVERAGE-PREFIX-MAP-ORDER: @__llvm_coverage_mapping = 
{{.*"\\02.*newpath.*root.*nested.*coverage-prefix-map\.c}}
+
+// Test that last -fcoverage-prefix-map option 
(-fcoverage-prefix-map=%/t/root=.) is applied.
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm 
-mllvm -enable-name-compression=false -main-file-name coverage-prefix-map.c 
%t/root/nested/coverage-prefix-map.c -fcoverage-prefix-map==newpath 
-fcoverage-prefix-map=%/t/root=. -o - | FileCheck 
--check-prefix=COVERAGE-PREFIX-MAP-REORDER %s
+// COVERAGE-PREFIX-MAP-REORDER: @__llvm_coverage_mapping =
+// COVERAGE-PREFIX-MAP-REORDER-NOT: newpath
+// COVERAGE-PREFIX-MAP-REORDER-SAME: nested{{.*coverage-prefix-map\.c}}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1702,8 +1702,7 @@
 
   for (const auto &Arg : Args.getAllArgValues(OPT_fcoverage_prefix_map_EQ)) {
 auto Split = StringRef(Arg).split('=');
-Opts.CoveragePrefixMap.insert(
-{std::string(Split.first), std::string(Split.second)});
+Opts.CoveragePrefixMap.emplace_back(Split.first, Split.second);
   }
 
   const llvm::Triple::ArchType DebugEntryValueArchs[] = {
Index: clang/lib/CodeGen/CoverageMappingGen.h
===
--- clang/lib/CodeGen/CoverageMappingGen.h
+++ clang/lib/CodeGen/CoverageMappingGen.h
@@ -107,7 +107,10 @@
   llvm::SmallDenseMap FileEntries;
   std::vector FunctionNames;
   std::vector FunctionRecords;
-  std::map CoveragePrefixMap;
+
+  /// Prefix replacement map for coverage.
+  llvm::SmallVector, 0>
+  CoveragePrefixMap;
 
   std::string getCurrentDirname();
   std::string normalizeFilename(StringRef Filename);
Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1635,7 +1635,8 @@
 CoverageMappingModuleGen::CoverageMappingModuleGen(
 CodeGenModule &CGM, CoverageSourceInfo &SourceInfo)
 : CGM(CGM), SourceInfo(SourceInfo) {
-  CoveragePrefixMap = CGM.getCodeGenOpts().CoveragePrefixMap;
+  for (const auto &[From, To] : CGM.getCodeGenOpts().CoveragePrefixMap)
+CoveragePrefixMap.emplace_back(From, To);
 }
 
 std::string CoverageMappingModuleGen::getCurrentDirname() {
@@ -1650,10 +1651,13 @@
 std::string CoverageMappingModuleGen::normalizeFilename(StringRef Filename) {
   llvm::SmallString<256> Path(Filename);
   llvm::sys::path::remove_dots(Path, /*remove_dot_dot=*/true);
-  for (const auto &Entry : CoveragePrefixMap) {
-if (llvm::sys::path::replace_path_prefix(Path, Entry.first, Entry.second))
+
+  // Traverse coverage prefix mapping options that are provided in reverse
+  // order.
+  for (const auto &[From, To] : llvm::reverse(CoveragePrefixMap))
+if (llvm::sys::path::replace_path_prefix(Path, From, To))
   break;
-  }
+
   return Path.str().str();
 }
 
Index: clang/include/clang/Basic/CodeGenOptions.h
===
--- clang/include/clang/Basic/CodeGenOptions.h
+++ clang/include/clang/Basic/CodeGenOptions.h
@@ -207,7 +207,9 @@
   std::string RecordCommandLine;
 
   std::map DebugPrefixMap;
-  std::map CoveragePrefixMap;
+
+  /// Prefix replacement map for coverage.
+  llvm::SmallVector, 0> Coverag

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

2023-04-25 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno added a comment.



> Looks like this patch causes a number of issues for us, I will work with 
> @jansvoboda11 to see if there's some way to resolve them.

If you can share a reproducer I'm pretty sure @ivanmurashko can help make it 
work for y'all too. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

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


[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

2023-04-25 Thread Andrey Ali Khan Bolshakov via Phabricator via cfe-commits
bolshakov-a added a comment.

Ping.


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

https://reviews.llvm.org/D146386

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


[PATCH] D148967: Disable atexit()-based lowering when LTO'ing kernel/kext code

2023-04-25 Thread Ahmed Bougacha via Phabricator via cfe-commits
ab accepted this revision.
ab added a comment.
This revision is now accepted and ready to land.

A comment inline, but LGTM otherwise, thanks!




Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1208
+  // -disable-atexit-based-global-dtor-lowering CodeGen flag.
+  // report_fatal_error("@llvm.global_dtors should have been lowered already");
 }

stray fatal_error?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148967

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


[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

2023-04-25 Thread Andrew Gallagher via Phabricator via cfe-commits
andrewjcg added a comment.

@ivanmurashko: Sorry for the delay getting back to you here.  Feel free to 
commandeer, as I don't have plans to get to this soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103930

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


[PATCH] D147281: Stop modifying trailing return types.

2023-04-25 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

Looks like this caused a change in behavior: 
https://github.com/llvm/llvm-project/issues/62361


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147281

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


[PATCH] D149000: Update with warning message for comparison to NULL pointer

2023-04-25 Thread Krishna Narayanan via Phabricator via cfe-commits
Krishna-13-cyber updated this revision to Diff 516857.
Krishna-13-cyber added a comment.

- Updated with reviewed changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149000

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/conditional-expr.c
  clang/test/Sema/warn-tautological-compare.c


Index: clang/test/Sema/warn-tautological-compare.c
===
--- clang/test/Sema/warn-tautological-compare.c
+++ clang/test/Sema/warn-tautological-compare.c
@@ -93,3 +93,9 @@
   x = array ? 1 : 0; // expected-warning {{address of array}}
   x = &x ? 1 : 0;// expected-warning {{address of 'x'}}
 }
+
+void test4(void)
+{
+int *a = (void *) 0;
+int b = (&a) == ((void *) 0); // expected-warning {{comparison of address of 
'a' equal to a null pointer is always false}}
+}
Index: clang/test/Sema/conditional-expr.c
===
--- clang/test/Sema/conditional-expr.c
+++ clang/test/Sema/conditional-expr.c
@@ -86,7 +86,7 @@
 
 int Postgresql(void) {
   char x;
-  return &x) != ((void *) 0)) ? (*(&x) = ((char) 1)) : (void) ((void *) 
0)), (unsigned long) ((void *) 0)); // expected-warning {{C99 forbids 
conditional expressions with only one void side}}
+  return &x) != ((void *) 0)) ? (*(&x) = ((char) 1)) : (void) ((void *) 
0)), (unsigned long) ((void *) 0)); // expected-warning {{comparison of address 
of 'x' not equal to a null pointer is always true}}
 }
 
 #define nil ((void*) 0)
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -14723,7 +14723,7 @@
 
   bool IsAddressOf = false;
 
-  if (UnaryOperator *UO = dyn_cast(E)) {
+  if (UnaryOperator *UO = dyn_cast(E->IgnoreParenCasts())) {
 if (UO->getOpcode() != UO_AddrOf)
   return;
 IsAddressOf = true;


Index: clang/test/Sema/warn-tautological-compare.c
===
--- clang/test/Sema/warn-tautological-compare.c
+++ clang/test/Sema/warn-tautological-compare.c
@@ -93,3 +93,9 @@
   x = array ? 1 : 0; // expected-warning {{address of array}}
   x = &x ? 1 : 0;// expected-warning {{address of 'x'}}
 }
+
+void test4(void)
+{
+int *a = (void *) 0;
+int b = (&a) == ((void *) 0); // expected-warning {{comparison of address of 'a' equal to a null pointer is always false}}
+}
Index: clang/test/Sema/conditional-expr.c
===
--- clang/test/Sema/conditional-expr.c
+++ clang/test/Sema/conditional-expr.c
@@ -86,7 +86,7 @@
 
 int Postgresql(void) {
   char x;
-  return &x) != ((void *) 0)) ? (*(&x) = ((char) 1)) : (void) ((void *) 0)), (unsigned long) ((void *) 0)); // expected-warning {{C99 forbids conditional expressions with only one void side}}
+  return &x) != ((void *) 0)) ? (*(&x) = ((char) 1)) : (void) ((void *) 0)), (unsigned long) ((void *) 0)); // expected-warning {{comparison of address of 'x' not equal to a null pointer is always true}}
 }
 
 #define nil ((void*) 0)
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -14723,7 +14723,7 @@
 
   bool IsAddressOf = false;
 
-  if (UnaryOperator *UO = dyn_cast(E)) {
+  if (UnaryOperator *UO = dyn_cast(E->IgnoreParenCasts())) {
 if (UO->getOpcode() != UO_AddrOf)
   return;
 IsAddressOf = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D115907: [misexpect] Re-implement MisExpect Diagnostics

2023-04-25 Thread Paul Kirth via Phabricator via cfe-commits
paulkirth added a comment.

In D115907#4296154 , @hans wrote:

> In D115907#4295923 , @paulkirth 
> wrote:
>
>>> 2. Due to inlining etc., it often gets the source locations wrong, which 
>>> means it points at code where again there were no expectations -- but 
>>> perhaps that code got inlined into an expectations somewhere else. (e.g. 
>>> https://crbug.com/1434989#c9)
>>
>> The checking depends somewhat on the instrumentation type (frontend vs. 
>> backend instrumentation) In the case of frontend instrumentation, when the 
>> expect intrinsic is lowered (very early in the pipeline) we can report the 
>> diagnostic right away, since branch weights from profiling have already been 
>> attached. The //should// mean that you should get fairly accurate source 
>> information, since this happens before any optimizations run.
>
> We use the backend instrumentation since that gives the best performance, but 
> it's good to hear that the locations may be more accurate with frontend 
> instrumentation. Maybe we should add that to the docs?

Let me look into this a bit. I'm not sure if this is just we're not doing the 
best practice when printing the diagnostic, or if the resolution is being 
degraded due to optimizations. We may want to update the documentation anyway, 
but I'd like to be certain.




Comment at: clang/include/clang/Driver/Options.td:1434
+def fdiagnostics_misexpect_tolerance_EQ : Joined<["-"], 
"fdiagnostics-misexpect-tolerance=">,
+Group, Flags<[CC1Option]>, MetaVarName<"">,
+HelpText<"Prevent misexpect diagnostics from being output if the profile 
counts are within N% of the expected. ">;

hans wrote:
> paulkirth wrote:
> > hans wrote:
> > > Should this be a driver mode option and not just cc1? The doc suggests 
> > > using it, which currently won't work without an `-Xclang` prefix.
> > That should also work from clang w/o `-Xclang`, shouldn't it? This is used 
> > exactly like `-fdiagnostics-hotness-threshold=`, so IIRC that should still 
> > work from clang.
> Hmm, if I add `-fdiagnostics-misexpect-tolerance=10` I get errors about the 
> flag being unused. I had to use `-Xclang` for it to work.
That isn't WAI then. I'll try to address that today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115907

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


[PATCH] D149123: [AArch64][InlineAsm]Add Clang support for flag output constraints

2023-04-25 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments.



Comment at: clang/lib/Basic/Targets/AArch64.cpp:1216
+// Returns the length of cc constraint.
+static unsigned matchAsmCCConstraint(const char *&Name) {
+  constexpr unsigned len = 5;

davidxl wrote:
> Name is not modified in this method, so perhaps dropping '&'?
Yeah, looks like this was copied from D57394. Probably both places should be 
fixed.

A `char *&` is a code smell.



Comment at: clang/lib/Basic/Targets/AArch64.cpp:1310
+// CC condition
+if (auto Len = matchAsmCCConstraint(Name)) {
+  Name += Len - 1;

please don't use `auto` here.


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

https://reviews.llvm.org/D149123

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


[PATCH] D72538: [ThinLTO] Add additional ThinLTO pipeline testing with new PM

2023-04-25 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment.

In D72538#4296119 , @tejohnson wrote:

> In D72538#4291552 , @nikic wrote:
>
>> Would it be possible to cut down the Clang side tests to only check parts 
>> that Clang controls in some way, e.g. that SLPVectorizer is enabled? This 
>> test is something of a PITA because it tests LLVM implementation details but 
>> is not part of the LLVM tests, so it usually gets missed when doing pipeline 
>> changes.
>
> I think we can cut this down significantly, since the llvm tests added here 
> check the full ThinLTO default pipeline setup at different opt levels. 
> However, the point of the clang test is to make sure that for a distributed 
> ThinLTO backend we correctly invoke thinBackend() which should set up the 
> ThinLTO default pipeline. So how about I cut this down to check a single pass 
> that is only invoked in the LTO backends? Specifically, I'm thinking of 
> WholeProgramDevirtPass.

That sounds reasonable to me!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72538

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


  1   2   >