date:20230814

[clang] c4ada13 - [NFC][Clang] Fix static analyzer concern about null value dereference

2023-08-14 Thread Elizabeth Andrews via cfe-commits


Author: Elizabeth Andrews
Date: 2023-08-14T12:19:45-07:00
New Revision: c4ada13e4b3e72543e454a12a6db085812114c0c

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

LOG: [NFC][Clang] Fix static analyzer concern about null value dereference

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

Added: 


Modified: 
clang/lib/Sema/SemaExprCXX.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 82afb084efd0b1..e6eb9508f8e6ff 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -9072,8 +9072,10 @@ Sema::BuildExprRequirement(
 MultiLevelTemplateArgumentList MLTAL(Param, TAL.asArray(),
  /*Final=*/false);
 MLTAL.addOuterRetainedLevels(TPL->getDepth());
-Expr *IDC = Param->getTypeConstraint()->getImmediatelyDeclaredConstraint();
-ExprResult Constraint = SubstExpr(IDC, MLTAL);
+const TypeConstraint *TC = Param->getTypeConstraint();
+assert(TC && "Type Constraint cannot be null here");
+ExprResult Constraint =
+SubstExpr(TC->getImmediatelyDeclaredConstraint(), MLTAL);
 if (Constraint.isInvalid()) {
   Status = concepts::ExprRequirement::SS_ExprSubstitutionFailure;
 } else {



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

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

2023-08-14 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

If `getStmtForDiagnostics()` in general, never returns null, then shouldn't we 
mark the API with an appropriate attribute?


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

https://reviews.llvm.org/D157888

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

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

2023-08-14 Thread Balázs Benics via Phabricator via cfe-commits

steakhal added a comment.

Hmm. I guess the assertion is to silence some tool. And I think actually that 
function might very well also return null in some cases.
Why do you think it cannot or at least should not return null in your context? 
I couldn't infer this from the context, neither from the description of this 
patch.

Without that, I would prefer an if to guard the code, instead of asserting this.




Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:657
+  const Stmt *S = AcquireNode->getStmtForDiagnostics();
+  assert(S && "Statmement cannot be null.");
   PathDiagnosticLocation LocUsedForUniqueing =

I think there is a typo in the word statement.


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

https://reviews.llvm.org/D157888

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

[PATCH] D157280: [PGO] Enable `-fprofile-update` for `-fprofile-generate`

2023-08-14 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added inline comments.



Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1797
   addPGOInstrPasses(MPM, Level, /* RunProfileGen */ true,
-/* IsCS */ true, PGOOpt->CSProfileGenFile,
-PGOOpt->ProfileRemappingFile,
+/* IsCS */ true, PGOOpt->AtomicCounterUpdate,
+PGOOpt->CSProfileGenFile, PGOOpt->ProfileRemappingFile,

qiongsiwu1 wrote:
> MaskRay wrote:
> > The canonical way to specify the parameter name is `/*IsCS=*/true`
> Ah thanks for pointing it out! Let me fix it in a later NFC patch if that is 
> OK. There are quite a few changes required and they would make the actual 
> feature harder to see. 
I think it's better to fix the argument style while you are modifying it. 
Otherwise, there may not be not much value to do a batch change for this file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157280

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

[clang] cf2cf19 - [Profile] Allow online merging with debug info correlation.

2023-08-14 Thread Zequan Wu via cfe-commits


Author: Zequan Wu
Date: 2023-08-14T15:15:01-04:00
New Revision: cf2cf195d5fb0a07c122c5c274bd6deb0790e015

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

LOG: [Profile] Allow online merging with debug info correlation.

When using debug info correlation, value profiling needs to be switched off.
So, we are only merging counter sections. In that case the existance of data
section is just used to provide an extra check in case of corrupted profile.

This patch performs counter merging by iterating the counter section by counter
size and add them together.

Reviewed By: ellis, MaskRay

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

Added: 


Modified: 
clang/lib/CodeGen/BackendUtil.cpp
compiler-rt/lib/profile/InstrProfilingMerge.c
compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
compiler-rt/test/profile/instrprof-merge-error.c

Removed: 




diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index e1aa697a7747ff..e902f7372f3ab5 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -102,7 +102,7 @@ namespace {
 
 // Default filename used for profile generation.
 std::string getDefaultProfileGenName() {
-  return DebugInfoCorrelate ? "default_%p.proflite" : "default_%m.profraw";
+  return DebugInfoCorrelate ? "default_%m.proflite" : "default_%m.profraw";
 }
 
 class EmitAssemblyHelper {

diff  --git a/compiler-rt/lib/profile/InstrProfilingMerge.c 
b/compiler-rt/lib/profile/InstrProfilingMerge.c
index 9e72b9e4f88537..c4ae067c4dc902 100644
--- a/compiler-rt/lib/profile/InstrProfilingMerge.c
+++ b/compiler-rt/lib/profile/InstrProfilingMerge.c
@@ -47,13 +47,16 @@ uint64_t lprofGetLoadModuleSignature(void) {
 COMPILER_RT_VISIBILITY
 int __llvm_profile_check_compatibility(const char *ProfileData,
uint64_t ProfileSize) {
-  /* Check profile header only for now  */
   __llvm_profile_header *Header = (__llvm_profile_header *)ProfileData;
   __llvm_profile_data *SrcDataStart, *SrcDataEnd, *SrcData, *DstData;
+  char *SrcCountersStart, *SrcCountersEnd;
   SrcDataStart =
   (__llvm_profile_data *)(ProfileData + sizeof(__llvm_profile_header) +
   Header->BinaryIdsSize);
   SrcDataEnd = SrcDataStart + Header->DataSize;
+  SrcCountersStart = (char *)SrcDataEnd;
+  SrcCountersEnd = SrcCountersStart +
+   Header->CountersSize * __llvm_profile_counter_entry_size();
 
   if (ProfileSize < sizeof(__llvm_profile_header))
 return 1;
@@ -72,6 +75,11 @@ int __llvm_profile_check_compatibility(const char 
*ProfileData,
   Header->ValueKindLast != IPVK_Last)
 return 1;
 
+  if (SrcCountersEnd - SrcCountersStart !=
+  __llvm_profile_end_counters() - __llvm_profile_begin_counters()) {
+return 1;
+  }
+
   if (ProfileSize <
   sizeof(__llvm_profile_header) + Header->BinaryIdsSize +
   Header->DataSize * sizeof(__llvm_profile_data) + Header->NamesSize +
@@ -102,13 +110,6 @@ static uintptr_t signextIfWin64(void *V) {
 COMPILER_RT_VISIBILITY
 int __llvm_profile_merge_from_buffer(const char *ProfileData,
  uint64_t ProfileSize) {
-  if (__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE) {
-PROF_ERR(
-"%s\n",
-"Debug info correlation does not support profile merging at runtime. "
-"Instead, merge raw profiles using the llvm-profdata tool.");
-return 1;
-  }
   if (__llvm_profile_get_version() & VARIANT_MASK_TEMPORAL_PROF) {
 PROF_ERR("%s\n",
  "Temporal profiles do not support profile merging at runtime. "
@@ -118,7 +119,8 @@ int __llvm_profile_merge_from_buffer(const char 
*ProfileData,
 
   __llvm_profile_data *SrcDataStart, *SrcDataEnd, *SrcData, *DstData;
   __llvm_profile_header *Header = (__llvm_profile_header *)ProfileData;
-  char *SrcCountersStart;
+  char *SrcCountersStart, *DstCounter;
+  const char *SrcCountersEnd, *SrcCounter;
   const char *SrcNameStart;
   const char *SrcValueProfDataStart, *SrcValueProfData;
   uintptr_t CountersDelta = Header->CountersDelta;
@@ -128,14 +130,36 @@ int __llvm_profile_merge_from_buffer(const char 
*ProfileData,
   Header->BinaryIdsSize);
   SrcDataEnd = SrcDataStart + Header->DataSize;
   SrcCountersStart = (char *)SrcDataEnd;
-  SrcNameStart = SrcCountersStart +
- Header->CountersSize * __llvm_profile_counter_entry_size();
+  SrcCountersEnd = SrcCountersStart +
+ Header->CountersSize * __llvm_profile_counter_entry_size(); 
+  SrcNameStart = SrcCountersEnd;
   SrcValueProfDataStart =
   SrcNameStart + Header->NamesSize +

[PATCH] D157632: [Profile] Allow online merging with debug info correlation.

2023-08-14 Thread Zequan 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 rGcf2cf195d5fb: [Profile] Allow online merging with debug info 
correlation. (authored by zequanwu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157632

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  compiler-rt/lib/profile/InstrProfilingMerge.c
  compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
  compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
  compiler-rt/test/profile/instrprof-merge-error.c

Index: compiler-rt/test/profile/instrprof-merge-error.c
===
--- compiler-rt/test/profile/instrprof-merge-error.c
+++ compiler-rt/test/profile/instrprof-merge-error.c
@@ -1,11 +1,5 @@
 // RUN: rm -rf %t; mkdir %t
 
-// RUN: %clang_pgogen -o %t/dbg -g -mllvm --debug-info-correlate -mllvm --disable-vp=true %s
-// RUN: env LLVM_PROFILE_FILE=%t/dbg_%m.profdata %run %t/dbg 2>&1 | count 0
-// RUN: env LLVM_PROFILE_FILE=%t/dbg_%m.profdata %run %t/dbg 2>&1 | FileCheck %s --check-prefix=DBG
-
-// DBG: Debug info correlation does not support profile merging at runtime.
-
 // RUN: %clang_pgogen -o %t/timeprof -mllvm -pgo-temporal-instrumentation %s
 // RUN: env LLVM_PROFILE_FILE=%t/timeprof_%m.profdata %run %t/timeprof 2>&1 | count 0
 // RUN: env LLVM_PROFILE_FILE=%t/timeprof_%m.profdata %run %t/timeprof 2>&1 | FileCheck %s --check-prefix=TIMEPROF
Index: compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
===
--- compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
+++ compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
@@ -24,3 +24,23 @@
 // RUN: llvm-profdata merge -o %t.cov.normal.profdata %t.cov.profraw
 
 // RUN: diff <(llvm-profdata show --all-functions --counts %t.cov.normal.profdata) <(llvm-profdata show --all-functions --counts %t.cov.profdata)
+
+// Test debug info correlate with online merging.
+
+// RUN: env LLVM_PROFILE_FILE=%t-1.profraw %run %t.normal
+// RUN: env LLVM_PROFILE_FILE=%t-2.profraw %run %t.normal
+// RUN: llvm-profdata merge -o %t.normal.profdata %t-1.profraw %t-2.profraw
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t %t.profdir/
+
+// RUN: diff <(llvm-profdata show --all-functions --counts %t.normal.profdata) <(llvm-profdata show --all-functions --counts %t.profdata)
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.cov.proflite %run %t.cov
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.cov.proflite %run %t.cov
+// RUN: llvm-profdata merge -o %t.cov.profdata --debug-info=%t.cov %t.profdir/
+
+// RUN: diff <(llvm-profdata show --all-functions --counts %t.cov.normal.profdata) <(llvm-profdata show --all-functions --counts %t.cov.profdata)
Index: compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
===
--- compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
+++ compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
@@ -18,3 +18,23 @@
 // RUN: llvm-profdata merge -o %t.cov.normal.profdata %t.cov.profraw
 
 // RUN: diff <(llvm-profdata show --all-functions --counts %t.cov.normal.profdata) <(llvm-profdata show --all-functions --counts %t.cov.profdata)
+
+// Test debug info correlate with online merging.
+
+// RUN: env LLVM_PROFILE_FILE=%t-1.profraw %run %t.normal
+// RUN: env LLVM_PROFILE_FILE=%t-2.profraw %run %t.normal
+// RUN: llvm-profdata merge -o %t.normal.profdata %t-1.profraw %t-2.profraw
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t.dSYM %t.profdir/
+
+// RUN: diff <(llvm-profdata show --all-functions --counts %t.normal.profdata) <(llvm-profdata show --all-functions --counts %t.profdata)
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.cov.proflite %run %t.cov
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.cov.proflite %run %t.cov
+// RUN: llvm-profdata merge -o %t.cov.profdata --debug-info=%t.cov.dSYM %t.profdir/
+
+// RUN: diff <(llvm-profdata show --all-functions --counts %t.cov.normal.profdata) <(llvm-profdata show --all-functions --counts %t.cov.profdata)
Index: compiler-rt/lib/profile/InstrProfilingMerge.c
===
--- compiler-rt/lib/profile/InstrProfilingMerge.c
+++ compiler-rt/lib/profile/InstrProfilingMerge.c
@@ -47,13 +47,16 @@
 COMPILER_RT_VISIBILITY
 int

[PATCH] D157762: [WIP] Implement [[msvc::no_unique_address]]

2023-08-14 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added inline comments.



Comment at: clang/lib/AST/Decl.cpp:4510-4512
+for (const FieldDecl *Field : CXXRD->fields())
+  if (Field->getType()->getAsCXXRecordDecl())
+return false;

maybe `llvm::any_of`? Not especially shorter, but maybe makes it a smidge 
easier to read?



Comment at: clang/lib/AST/Decl.cpp:4523-4524
 bool FieldDecl::isPotentiallyOverlapping() const {
-  return hasAttr() && getType()->getAsCXXRecordDecl();
+  return (hasAttr() ||
+  hasAttr()) &&
+ getType()->getAsCXXRecordDecl();

Having to check both of these in several places seems problematic - can we wrap 
that up somewhere? (or, maybe ideally, is there a way for 
`msvc::no_unique_address` to map to the actual NoUniqueAddressAttr as a 
different spelling of the same thing?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157762

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

[PATCH] D157280: [PGO] Enable `-fprofile-update` for `-fprofile-generate`

2023-08-14 Thread Qiongsi Wu via Phabricator via cfe-commits

qiongsiwu1 added a comment.

In D157280#4586078 , @MaskRay wrote:

> Please add a link to https://reviews.llvm.org/D87737 in the summary/commit 
> message.

Will do!




Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1797
   addPGOInstrPasses(MPM, Level, /* RunProfileGen */ true,
-/* IsCS */ true, PGOOpt->CSProfileGenFile,
-PGOOpt->ProfileRemappingFile,
+/* IsCS */ true, PGOOpt->AtomicCounterUpdate,
+PGOOpt->CSProfileGenFile, PGOOpt->ProfileRemappingFile,

MaskRay wrote:
> The canonical way to specify the parameter name is `/*IsCS=*/true`
Ah thanks for pointing it out! Let me fix it in a later NFC patch if that is 
OK. There are quite a few changes required and they would make the actual 
feature harder to see. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157280

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

[PATCH] D157837: [flang][driver] Update the visibility of Clang options in Flang

2023-08-14 Thread Andrzej Warzynski via Phabricator via cfe-commits

awarzynski updated this revision to Diff 550047.
awarzynski added a comment.

Add missing `TargetSpecific` flag to the definition of `mcpu_EQ`, remove 
redundant `TODO`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157837

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

Index: flang/test/Driver/fast_math.f90
===
--- flang/test/Driver/fast_math.f90
+++ flang/test/Driver/fast_math.f90
@@ -70,7 +70,7 @@
 ! UNSUPPORTED: system-windows
 ! UNSUPPORTED: target=powerpc{{.*}}
 ! RUN: %flang -ffast-math -### %s -o %t 2>&1 \
-! RUN:   --target=x86_64-unknown-linux -no-pie --gcc-toolchain="" \
+! RUN:   --target=x86_64-unknown-linux -no-pie \
 ! RUN:   --sysroot=%S/../../../clang/test/Driver/Inputs/basic_linux_tree \
 ! RUN: | FileCheck --check-prefix=CHECK-CRT %s
 ! CHECK-CRT: {{crtbegin.?\.o}}
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -14,224 +14,242 @@
 ! HELP:USAGE: flang
 ! HELP-EMPTY:
 ! HELP-NEXT:OPTIONS:
-! HELP-NEXT: -###   Print (but do not run) the commands to run for this compilation
-! HELP-NEXT: -cpp   Enable predefined and command line preprocessor macros
-! HELP-NEXT: -c Only run preprocess, compile, and assemble steps
-! HELP-NEXT: -D = Define  to  (or 1 if  omitted)
-! HELP-NEXT: -emit-llvm Use the LLVM representation for assembler and object files
-! HELP-NEXT: -E Only run the preprocessor
+! HELP-NEXT: -###Print (but do not run) the commands to run for this compilation
+! HELP-NEXT: -cppEnable predefined and command line preprocessor macros
+! HELP-NEXT: -c  Only run preprocess, compile, and assemble steps
+! HELP-NEXT: -D =  Define  to  (or 1 if  omitted)
+! HELP-NEXT: -emit-llvm  Use the LLVM representation for assembler and object files
+! HELP-NEXT: -E  Only run the preprocessor
 ! HELP-NEXT: -falternative-parameter-statement
-! HELP-NEXT: Enable the old style PARAMETER statement
-! HELP-NEXT: -fapprox-func  Allow certain math function calls to be replaced with an approximately equivalent calculation
-! HELP-NEXT: -fbackslashSpecify that backslash in string introduces an escape character
-! HELP-NEXT: -fcolor-diagnosticsEnable colors in diagnostics
-! HELP-NEXT: -fconvert=  Set endian conversion of data for unformatted files
-! HELP-NEXT: -fdefault-double-8 Set the default double precision kind to an 8 byte wide type
-! HELP-NEXT: -fdefault-integer-8Set the default integer and logical kind to an 8 byte wide type
-! HELP-NEXT: -fdefault-real-8   Set the default real kind to an 8 byte wide type
-! HELP-NEXT: -ffast-mathAllow aggressive, lossy floating-point optimizations
-! HELP-NEXT: -ffixed-form   Process source files in fixed form
+! HELP-NEXT: Enable the old style PARAMETER statement
+! HELP-NEXT: -fapprox-func   Allow certain math function calls to be replaced with an approximately equivalent calculation
+! HELP-NEXT: -fbackslash Specify that backslash in string introduces an escape character
+! HELP-NEXT: -fcolor-diagnostics Enable colors in diagnostics
+! HELP-NEXT: -fconvert=   Set endian conversion of data for unformatted files
+! HELP-NEXT: -fdefault-double-8  Set the default double precision kind to an 8 byte wide type
+! HELP-NEXT: -fdefault-integer-8 Set the default integer and logical kind to an 8 byte wide type
+! HELP-NEXT: -fdefault-real-8Set the default real kind to an 8 byte wide type
+! HELP-NEXT: -ffast-math Allow aggressive, lossy floating-point optimizations
+! HELP-NEXT: -ffixed-formProcess source files in fixed form
 ! HELP-NEXT: -ffixed-line-length=
-! HELP-NEXT: Use  as character line width in fixed mode
-! HELP-NEXT: -ffp-contract= Form fused FP ops (e.g. FMAs)
-! HELP-NEXT: -ffree-formProcess source files in free form
-! HELP-NEXT: -fimplicit-noneNo implicit typing allowed unless overridden by IMPLICIT statements
+! HELP-NEXT: Use  as character line width in fixed mode
+! HELP-NEXT: -ffp-contract=   Form fused FP ops (e.g. FMAs)
+! HELP-NEXT: -ffree-form Process source files in free form
+! HELP-NEXT: -fhonor-infinities  Specify that floating-point optimizations are not allowed that assume arguments and results are not +-inf.
+! HELP-NEXT: -fhonor-nansSpecify that floating-point optimizations are not

[PATCH] D157837: [flang][driver] Update the visibility of Clang options in Flang

2023-08-14 Thread Andrzej Warzynski via Phabricator via cfe-commits

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

Thank you all for reviewing!

In D157837#4584387 , @bogner wrote:

> I can't speak to which flags should be present in flang-new or not

That's determined by what's tested/used in tests.

> You'll need to update the patch to use the "ClangOption" spelling rather than 
> "Default" of course, as discussed in https://reviews.llvm.org/D157151

Yup, no problem. I may wait for your changes to land first.




Comment at: flang/test/Driver/target-cpu-features.f90:14
 ! Negative test. ARM cpu with x86 target.
-! RUN: not %flang --target=x86_64-linux-gnu -mcpu=cortex-a57 -c %s -### 2>&1 \
+! RUN: %flang --target=x86_64-linux-gnu -mcpu=cortex-a57 -c %s -### 2>&1 \
 ! RUN: | FileCheck %s -check-prefix=CHECK-NO-A57

tblah wrote:
> Why doesn't this fail now?
Missing `TargetSpecific` flag in the definition of the `mcpu_EQ` option. Sorry 
i didn't have time to debug earlier. To be fixed in the next revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157837

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

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-14 Thread Matt Arsenault via Phabricator via cfe-commits

arsenm added inline comments.



Comment at: llvm/lib/CodeGen/TargetPassConfig.cpp:1281-1282
+else
+  WithColor::warning()
+  << "-fsplit-machine-functions is only valid for X86.\n";
   }

shenhan wrote:
> arsenm wrote:
> > You cannot spam warnings here. The other instance of printing here looks 
> > like a new addition and should be removed
> Thanks. Do you suggest moving the warnings to the underlying pass? (Although 
> that means we create passes that only issue warnings.)
Move it to the pass, and use a backend remark, not directly print to the 
console (e.g. DiagnosticInfoUnsupported)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157750

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

[PATCH] D157280: [PGO] Enable `-fprofile-update` for `-fprofile-generate`

2023-08-14 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

Please add a link to https://reviews.llvm.org/D87737 in the summary/commit 
message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157280

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

[PATCH] D157750: Properly handle -fsplit-machine-functions for fatbinary compilation

2023-08-14 Thread Han Shen via Phabricator via cfe-commits

shenhan added inline comments.



Comment at: clang/test/Driver/fsplit-machine-functions-with-cuda-nvptx.c:9
+
+// Check that -fsplit-machine-functions is passed to both x86 and cuda 
compilation and does not cause driver error.
+// MFS2: -fsplit-machine-functions

tra wrote:
> shenhan wrote:
> > tra wrote:
> > > We will still see a warning, right? So, for someone compiling with 
> > > `-Werror` that's going to be a problem.
> > > 
> > > Also, if the warning is issued from the top-level driver, we may not even 
> > > be able to suppress it when we disable splitting on GPU side with 
> > > `-Xarch_device -fno-split-machine-functions`.
> > > 
> > > 
> > > We will still see a warning, right?
> > Yes, there still will be a warning. We've discussed it and we think that 
> > pass -fsplit-machine-functions in this case is not a proper usage and a 
> > warning is warranted, and it is not good that skip doing split silently 
> > while uses explicitly ask for it.
> > 
> > > Also, if the warning is issued from the top-level driver
> > The warning will not be issued from the top-level driver, it will be issued 
> > when configuring optimization passes.
> > So:
> > 
> > 
> >   - -fsplit-machine-functions -Xarch_device -fno-split-machine-functions
> > Will enable MFS for host, disable MFS for gpus and without any warnings.
> > 
> >   - -Xarch_host -fsplit-machine-functions
> > The same as the above
> > 
> >   - -Xarch_host -fsplit-machine-functions -Xarch_device 
> > -fno-split-machine-functions
> > The same as the above
> > 
> > We've discussed it and we think that pass -fsplit-machine-functions in this 
> > case is not a proper usage and a warning is warranted, and it is not good 
> > that skip doing split silently while uses explicitly ask for it.
> 
> I would agree with that assertion if we were talking exclusively about CUDA 
> compilation.
> However, a common real world use pattern is that the flags are set globally 
> for all C++ compilations, and then CUDA compilations within the project need 
> to do whatever they need to to keep things working. The original user intent 
> was for the option to affect the host compilation. There's no inherent 
> assumption that it will do anything useful for the GPU.
> 
> In number of similar cases in the past we did settle on silently ignoring 
> some top-level flags that we do expect to encounter in real projects, but 
> which made no sense for the GPU. E.g. sanitizers. If the project is built w/ 
> sanitizer enabled, the idea is to sanitize the host code, The GPU code 
> continues to be built w/o sanitizer enabled. 
> 
> Anyways, as long as we have a way to deal with it it's not a big deal one way 
> or another.
> 
> > -fsplit-machine-functions -Xarch_device -fno-split-machine-functions
> > Will enable MFS for host, disable MFS for gpus and without any warnings.
> 
> OK. This will work.
> 
> 
> In number of similar cases in the past we did settle on silently ignoring 
> some top-level flags that we do expect to encounter in real projects, but 
> which made no sense for the GPU. E.g. sanitizers. If the project is built w/ 
> sanitizer enabled, the idea is to sanitize the host code, The GPU code 
> continues to be built w/o sanitizer enabled.

Can I understand it this way - if the compiler is **only** building for CPUs, 
then silently ignore any optimization flags is not a good behavior. If the 
compiler is building CPUs and GPUs, it is still not a good behavior to silently 
ignore optimization flags for CPUs, but it is probably ok to silently ignore 
optimization flags for GPUs.

> OK. This will work.
Thanks for confirming.



Comment at: llvm/lib/CodeGen/TargetPassConfig.cpp:1281-1282
+else
+  WithColor::warning()
+  << "-fsplit-machine-functions is only valid for X86.\n";
   }

arsenm wrote:
> You cannot spam warnings here. The other instance of printing here looks like 
> a new addition and should be removed
Thanks. Do you suggest moving the warnings to the underlying pass? (Although 
that means we create passes that only issue warnings.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157750

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

[PATCH] D157911: clang: Add __builtin_exp10* and use new llvm.exp10 intrinsic

2023-08-14 Thread Joshua Batista via Phabricator via cfe-commits

bob80905 added a comment.

Release note for addition of exp10 builtin?


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

https://reviews.llvm.org/D157911

___
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 support

2023-08-14 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

I think the best place to test `RISCVISAInfo.cpp` is 
`llvm/unittests/Support/RISCVISAInfoTest.cpp`.

`clang/test/Driver/print-supported-extensions.c` can test just a few lines, so 
that changes to RISC-V extensions will generally not require updates to 
`clang/test/Driver/print-supported-extensions.c`




Comment at: clang/test/Driver/print-supported-extensions.c:6
+// RUN: %clang --target=riscv64 --print-supported-extensions 2>&1 | \
+// RUN:   FileCheck --implicit-check-not=warning: --strict-whitespace 
--match-full-lines %s
+

Now that D156363 is stable, we can replace `FileCheck 
--implicit-check-not=warning:` with `%clang -Werror`.



Comment at: llvm/lib/Support/RISCVISAInfo.cpp:217
+  for (const auto  : SupportedExtensions)
+ExtMap[E.Name] = { E.Version.Major, E.Version.Minor };
+  for (const auto  : ExtMap)

MaskRay wrote:
> clang-format will remove the space after `{`
Not done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

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

[PATCH] D150262: Disable sanitizer's on ifunc resolvers.

2023-08-14 Thread Daniel Kiss 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 rG1ef3de6b09f6: Disable sanitizers on ifunc resolvers. 
(authored by danielkiss).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150262

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/ifunc.c


Index: clang/test/CodeGen/ifunc.c
===
--- clang/test/CodeGen/ifunc.c
+++ clang/test/CodeGen/ifunc.c
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 -triple i386-unknown-linux-gnu -emit-llvm -o - %s | 
FileCheck %s
 // RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -emit-llvm -o - %s | 
FileCheck %s
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -fsanitize=thread -O2 
-emit-llvm -o - %s | FileCheck %s --check-prefix=SAN
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -fsanitize=address -O2 
-emit-llvm -o - %s | FileCheck %s --check-prefix=SAN
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -fsanitize=memory -O2 
-emit-llvm -o - %s | FileCheck %s --check-prefix=SAN
 
 int foo(int) __attribute__ ((ifunc("foo_ifunc")));
 
@@ -39,3 +42,11 @@
 
 // CHECK: call i32 @foo(i32
 // CHECK: call void @goo()
+
+// SAN: define internal nonnull ptr @foo_ifunc() #[[#FOO_IFUNC:]] {
+
+// SAN: define dso_local noalias ptr @goo_ifunc() #[[#GOO_IFUNC:]] {
+
+// SAN-DAG: attributes #[[#FOO_IFUNC]] = {{{.*}} 
disable_sanitizer_instrumentation {{.*}}
+
+// SAN-DAG: attributes #[[#GOO_IFUNC]] = {{{.*}} 
disable_sanitizer_instrumentation {{.*}}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5832,7 +5832,9 @@
 Entry->eraseFromParent();
   } else
 GIF->setName(MangledName);
-
+  if (auto *F = dyn_cast(Resolver)) {
+F->addFnAttr(llvm::Attribute::DisableSanitizerInstrumentation);
+  }
   SetCommonAttributes(GD, GIF);
 }
 


Index: clang/test/CodeGen/ifunc.c
===
--- clang/test/CodeGen/ifunc.c
+++ clang/test/CodeGen/ifunc.c
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 -triple i386-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s
 // RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -fsanitize=thread -O2 -emit-llvm -o - %s | FileCheck %s --check-prefix=SAN
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -fsanitize=address -O2 -emit-llvm -o - %s | FileCheck %s --check-prefix=SAN
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -fsanitize=memory -O2 -emit-llvm -o - %s | FileCheck %s --check-prefix=SAN
 
 int foo(int) __attribute__ ((ifunc("foo_ifunc")));
 
@@ -39,3 +42,11 @@
 
 // CHECK: call i32 @foo(i32
 // CHECK: call void @goo()
+
+// SAN: define internal nonnull ptr @foo_ifunc() #[[#FOO_IFUNC:]] {
+
+// SAN: define dso_local noalias ptr @goo_ifunc() #[[#GOO_IFUNC:]] {
+
+// SAN-DAG: attributes #[[#FOO_IFUNC]] = {{{.*}} disable_sanitizer_instrumentation {{.*}}
+
+// SAN-DAG: attributes #[[#GOO_IFUNC]] = {{{.*}} disable_sanitizer_instrumentation {{.*}}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5832,7 +5832,9 @@
 Entry->eraseFromParent();
   } else
 GIF->setName(MangledName);
-
+  if (auto *F = dyn_cast(Resolver)) {
+F->addFnAttr(llvm::Attribute::DisableSanitizerInstrumentation);
+  }
   SetCommonAttributes(GD, GIF);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 1ef3de6 - Disable sanitizer's on ifunc resolvers.

2023-08-14 Thread Daniel Kiss via cfe-commits


Author: Daniel Kiss
Date: 2023-08-14T20:56:55+02:00
New Revision: 1ef3de6b09f6b21a383fc7cf1ce1283df738015a

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

LOG: Disable sanitizer's on ifunc resolvers.

Resolvers are running before the module is initialised which leads to
crashes due to the santizer is not yet initialised.

Fixes #40287

Reviewed By: hctim

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

Added: 


Modified: 
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGen/ifunc.c

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 1c48d3b2ace93b..3a79dec5359260 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -5832,7 +5832,9 @@ void CodeGenModule::emitIFuncDefinition(GlobalDecl GD) {
 Entry->eraseFromParent();
   } else
 GIF->setName(MangledName);
-
+  if (auto *F = dyn_cast(Resolver)) {
+F->addFnAttr(llvm::Attribute::DisableSanitizerInstrumentation);
+  }
   SetCommonAttributes(GD, GIF);
 }
 

diff  --git a/clang/test/CodeGen/ifunc.c b/clang/test/CodeGen/ifunc.c
index 64f7f3d4ec65ce..0b0a0549620f8b 100644
--- a/clang/test/CodeGen/ifunc.c
+++ b/clang/test/CodeGen/ifunc.c
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 -triple i386-unknown-linux-gnu -emit-llvm -o - %s | 
FileCheck %s
 // RUN: %clang_cc1 -triple i386-unknown-linux-gnu -O2 -emit-llvm -o - %s | 
FileCheck %s
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -fsanitize=thread -O2 
-emit-llvm -o - %s | FileCheck %s --check-prefix=SAN
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -fsanitize=address -O2 
-emit-llvm -o - %s | FileCheck %s --check-prefix=SAN
+// RUN: %clang_cc1 -triple i386-unknown-linux-gnu -fsanitize=memory -O2 
-emit-llvm -o - %s | FileCheck %s --check-prefix=SAN
 
 int foo(int) __attribute__ ((ifunc("foo_ifunc")));
 
@@ -39,3 +42,11 @@ void* goo_ifunc(void) {
 
 // CHECK: call i32 @foo(i32
 // CHECK: call void @goo()
+
+// SAN: define internal nonnull ptr @foo_ifunc() #[[#FOO_IFUNC:]] {
+
+// SAN: define dso_local noalias ptr @goo_ifunc() #[[#GOO_IFUNC:]] {
+
+// SAN-DAG: attributes #[[#FOO_IFUNC]] = {{{.*}} 
disable_sanitizer_instrumentation {{.*}}
+
+// SAN-DAG: attributes #[[#GOO_IFUNC]] = {{{.*}} 
disable_sanitizer_instrumentation {{.*}}



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

[PATCH] D157911: clang: Add __builtin_exp10* and use new llvm.exp10 intrinsic

2023-08-14 Thread Matt Arsenault via Phabricator via cfe-commits

arsenm created this revision.
arsenm added reviewers: jcranmer-intel, kpn, sepavloff, andrew.w.kaylor, foad, 
bob80905.
Herald added a subscriber: StephenFan.
Herald added a project: All.
arsenm requested review of this revision.
Herald added a subscriber: wdng.

https://reviews.llvm.org/D157911

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/constrained-math-builtins.c
  clang/test/CodeGen/math-builtins.c
  clang/test/CodeGenOpenCL/builtins-f16.cl

Index: clang/test/CodeGenOpenCL/builtins-f16.cl
===
--- clang/test/CodeGenOpenCL/builtins-f16.cl
+++ clang/test/CodeGenOpenCL/builtins-f16.cl
@@ -24,6 +24,9 @@
   // CHECK: call half @llvm.exp2.f16(half %h0)
   res = __builtin_exp2f16(h0);
 
+  // CHECK: call half @llvm.exp10.f16(half %h0)
+  res = __builtin_exp10f16(h0);
+
   // CHECK: call half @llvm.floor.f16(half %h0)
   res = __builtin_floorf16(h0);
 
Index: clang/test/CodeGen/math-builtins.c
===
--- clang/test/CodeGen/math-builtins.c
+++ clang/test/CodeGen/math-builtins.c
@@ -318,6 +318,17 @@
 // HAS_ERRNO: declare x86_fp80 @exp2l(x86_fp80 noundef) [[NOT_READNONE]]
 // HAS_ERRNO: declare fp128 @exp2f128(fp128 noundef) [[NOT_READNONE]]
 
+__builtin_exp10(f);   __builtin_exp10f(f);  __builtin_exp10l(f); __builtin_exp10f128(f);
+
+// NO__ERRNO: declare double @llvm.exp10.f64(double) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare float @llvm.exp10.f32(float) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare x86_fp80 @llvm.exp10.f80(x86_fp80) [[READNONE_INTRINSIC]]
+// NO__ERRNO: declare fp128 @llvm.exp10.f128(fp128) [[READNONE_INTRINSIC]]
+// HAS_ERRNO: declare double @exp10(double noundef) [[NOT_READNONE]]
+// HAS_ERRNO: declare float @exp10f(float noundef) [[NOT_READNONE]]
+// HAS_ERRNO: declare x86_fp80 @exp10l(x86_fp80 noundef) [[NOT_READNONE]]
+// HAS_ERRNO: declare fp128 @exp10f128(fp128 noundef) [[NOT_READNONE]]
+
 __builtin_expm1(f);  __builtin_expm1f(f); __builtin_expm1l(f); __builtin_expm1f128(f);
 
 // NO__ERRNO: declare double @expm1(double noundef) [[READNONE]]
Index: clang/test/CodeGen/constrained-math-builtins.c
===
--- clang/test/CodeGen/constrained-math-builtins.c
+++ clang/test/CodeGen/constrained-math-builtins.c
@@ -64,6 +64,13 @@
 // CHECK: call x86_fp80 @llvm.experimental.constrained.exp2.f80(x86_fp80 %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
 // CHECK: call fp128 @llvm.experimental.constrained.exp2.f128(fp128 %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
 
+  __builtin_exp10(f);   __builtin_exp10f(f);  __builtin_exp10l(f); __builtin_exp10f128(f);
+
+// CHECK: call double @exp10(double noundef %{{.*}})
+// CHECK: call float @exp10f(float noundef %{{.*}})
+// CHECK: call x86_fp80 @exp10l(x86_fp80 noundef %{{.*}})
+// CHECK: call fp128 @exp10f128(fp128 noundef %{{.*}})
+
   __builtin_floor(f);  __builtin_floorf(f); __builtin_floorl(f); __builtin_floorf128(f);
 
 // CHECK: call double @llvm.experimental.constrained.floor.f64(double %{{.*}}, metadata !"fpexcept.strict")
@@ -223,6 +230,11 @@
 // CHECK: declare x86_fp80 @llvm.experimental.constrained.exp2.f80(x86_fp80, metadata, metadata)
 // CHECK: declare fp128 @llvm.experimental.constrained.exp2.f128(fp128, metadata, metadata)
 
+// CHECK: declare double @exp10(double noundef)
+// CHECK: declare float @exp10f(float noundef)
+// CHECK: declare x86_fp80 @exp10l(x86_fp80 noundef)
+// CHECK: declare fp128 @exp10f128(fp128 noundef)
+
 // CHECK: declare double @llvm.experimental.constrained.floor.f64(double, metadata)
 // CHECK: declare float @llvm.experimental.constrained.floor.f32(float, metadata)
 // CHECK: declare x86_fp80 @llvm.experimental.constrained.floor.f80(x86_fp80, metadata)
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -2341,7 +2341,16 @@
   return RValue::get(emitUnaryMaybeConstrainedFPBuiltin(*this, E,
Intrinsic::exp2,
Intrinsic::experimental_constrained_exp2));
-
+case Builtin::BI__builtin_exp10:
+case Builtin::BI__builtin_exp10f:
+case Builtin::BI__builtin_exp10f16:
+case Builtin::BI__builtin_exp10l:
+case Builtin::BI__builtin_exp10f128: {
+  // TODO: strictfp support
+  if (Builder.getIsFPConstrained())
+break;
+  return RValue::get(emitUnaryBuiltin(*this, E, Intrinsic::exp10));
+}
 case Builtin::BIfabs:
 case Builtin::BIfabsf:
 case Builtin::BIfabsl:
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@

[PATCH] D157280: [PGO] Enable `-fprofile-update` for `-fprofile-generate`

2023-08-14 Thread Fangrui Song via Phabricator via cfe-commits

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

Thanks!




Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1797
   addPGOInstrPasses(MPM, Level, /* RunProfileGen */ true,
-/* IsCS */ true, PGOOpt->CSProfileGenFile,
-PGOOpt->ProfileRemappingFile,
+/* IsCS */ true, PGOOpt->AtomicCounterUpdate,
+PGOOpt->CSProfileGenFile, PGOOpt->ProfileRemappingFile,

The canonical way to specify the parameter name is `/*IsCS=*/true`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157280

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

[PATCH] D157632: [Profile] Allow online merging with debug info correlation.

2023-08-14 Thread Zequan Wu via Phabricator via cfe-commits

zequanwu updated this revision to Diff 550039.
zequanwu marked 3 inline comments as done.
zequanwu added a comment.

Rebase and address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157632

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  compiler-rt/lib/profile/InstrProfilingMerge.c
  compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
  compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
  compiler-rt/test/profile/instrprof-merge-error.c

Index: compiler-rt/test/profile/instrprof-merge-error.c
===
--- compiler-rt/test/profile/instrprof-merge-error.c
+++ compiler-rt/test/profile/instrprof-merge-error.c
@@ -1,11 +1,5 @@
 // RUN: rm -rf %t; mkdir %t
 
-// RUN: %clang_pgogen -o %t/dbg -g -mllvm --debug-info-correlate -mllvm --disable-vp=true %s
-// RUN: env LLVM_PROFILE_FILE=%t/dbg_%m.profdata %run %t/dbg 2>&1 | count 0
-// RUN: env LLVM_PROFILE_FILE=%t/dbg_%m.profdata %run %t/dbg 2>&1 | FileCheck %s --check-prefix=DBG
-
-// DBG: Debug info correlation does not support profile merging at runtime.
-
 // RUN: %clang_pgogen -o %t/timeprof -mllvm -pgo-temporal-instrumentation %s
 // RUN: env LLVM_PROFILE_FILE=%t/timeprof_%m.profdata %run %t/timeprof 2>&1 | count 0
 // RUN: env LLVM_PROFILE_FILE=%t/timeprof_%m.profdata %run %t/timeprof 2>&1 | FileCheck %s --check-prefix=TIMEPROF
Index: compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
===
--- compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
+++ compiler-rt/test/profile/Linux/instrprof-debug-info-correlate.c
@@ -24,3 +24,23 @@
 // RUN: llvm-profdata merge -o %t.cov.normal.profdata %t.cov.profraw
 
 // RUN: diff <(llvm-profdata show --all-functions --counts %t.cov.normal.profdata) <(llvm-profdata show --all-functions --counts %t.cov.profdata)
+
+// Test debug info correlate with online merging.
+
+// RUN: env LLVM_PROFILE_FILE=%t-1.profraw %run %t.normal
+// RUN: env LLVM_PROFILE_FILE=%t-2.profraw %run %t.normal
+// RUN: llvm-profdata merge -o %t.normal.profdata %t-1.profraw %t-2.profraw
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t %t.profdir/
+
+// RUN: diff <(llvm-profdata show --all-functions --counts %t.normal.profdata) <(llvm-profdata show --all-functions --counts %t.profdata)
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.cov.proflite %run %t.cov
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.cov.proflite %run %t.cov
+// RUN: llvm-profdata merge -o %t.cov.profdata --debug-info=%t.cov %t.profdir/
+
+// RUN: diff <(llvm-profdata show --all-functions --counts %t.cov.normal.profdata) <(llvm-profdata show --all-functions --counts %t.cov.profdata)
Index: compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
===
--- compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
+++ compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c
@@ -18,3 +18,23 @@
 // RUN: llvm-profdata merge -o %t.cov.normal.profdata %t.cov.profraw
 
 // RUN: diff <(llvm-profdata show --all-functions --counts %t.cov.normal.profdata) <(llvm-profdata show --all-functions --counts %t.cov.profdata)
+
+// Test debug info correlate with online merging.
+
+// RUN: env LLVM_PROFILE_FILE=%t-1.profraw %run %t.normal
+// RUN: env LLVM_PROFILE_FILE=%t-2.profraw %run %t.normal
+// RUN: llvm-profdata merge -o %t.normal.profdata %t-1.profraw %t-2.profraw
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t
+// RUN: llvm-profdata merge -o %t.profdata --debug-info=%t.dSYM %t.profdir/
+
+// RUN: diff <(llvm-profdata show --all-functions --counts %t.normal.profdata) <(llvm-profdata show --all-functions --counts %t.profdata)
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.cov.proflite %run %t.cov
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.cov.proflite %run %t.cov
+// RUN: llvm-profdata merge -o %t.cov.profdata --debug-info=%t.cov.dSYM %t.profdir/
+
+// RUN: diff <(llvm-profdata show --all-functions --counts %t.cov.normal.profdata) <(llvm-profdata show --all-functions --counts %t.cov.profdata)
Index: compiler-rt/lib/profile/InstrProfilingMerge.c
===
--- compiler-rt/lib/profile/InstrProfilingMerge.c
+++ compiler-rt/lib/profile/InstrProfilingMerge.c
@@ -47,13 +47,16 @@
 COMPILER_RT_VISIBILITY
 int __llvm_profile_check_compatibility(const char *ProfileData,

[PATCH] D157794: [Driver] Make /Zi and /Z7 aliases of -g rather than handling them specially

2023-08-14 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay accepted this revision.
MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4522
   RenderDebugInfoCompressionArgs(Args, CmdArgs, D, TC);
+
+

Keep just one blank line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157794

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

[PATCH] D86154: AMDGPU: Add llvm.amdgcn.{read,readfirst,write}lane2 intrinsics with type overloads

2023-08-14 Thread Nicolai Hähnle via Phabricator via cfe-commits

nhaehnle abandoned this revision.
nhaehnle added a comment.

Indeed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86154

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

[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

2023-08-14 Thread Qizhi Hu via Phabricator via cfe-commits

jcsxky added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:5831
+FoundTemplate->getFriendObjectKind() != Decl::FOK_None &&
+!D->specializations().empty();
+if (IsStructuralMatch(D, FoundTemplate, true, IgnoreDepth)) {

balazske wrote:
> Probably add `IsFriendTemplate`?
I think this is not needed because D is not always a friend template class 
according to the testcase (IsFriendTemplate is false in testcase).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156693

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

[PATCH] D157207: [clangd] Fix typo in comment

2023-08-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG09a622baa2d8: [clangd] Fix typo in comment (authored by 
Eymay, committed by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157207

Files:
  clang-tools-extra/clangd/Protocol.h


Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -592,7 +592,7 @@
 /// Clangd extension: parameters configurable at `initialize` time.
 /// LSP defines this type as `any`.
 struct InitializationOptions {
-  // What we can change throught the didChangeConfiguration request, we can
+  // What we can change through the didChangeConfiguration request, we can
   // also set through the initialize request (initializationOptions field).
   ConfigurationSettings ConfigSettings;
 


Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -592,7 +592,7 @@
 /// Clangd extension: parameters configurable at `initialize` time.
 /// LSP defines this type as `any`.
 struct InitializationOptions {
-  // What we can change throught the didChangeConfiguration request, we can
+  // What we can change through the didChangeConfiguration request, we can
   // also set through the initialize request (initializationOptions field).
   ConfigurationSettings ConfigSettings;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 09a622b - [clangd] Fix typo in comment

2023-08-14 Thread Kadir Cetinkaya via cfe-commits


Author: Eymen Ünay
Date: 2023-08-14T20:07:58+02:00
New Revision: 09a622baa2d80ccc38eb42b6b58b39519704399f

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

LOG: [clangd] Fix typo in comment

Reviewed By: kadircet

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

Added: 


Modified: 
clang-tools-extra/clangd/Protocol.h

Removed: 




diff  --git a/clang-tools-extra/clangd/Protocol.h 
b/clang-tools-extra/clangd/Protocol.h
index 23a48e0a8e5f61..e88c804692568f 100644
--- a/clang-tools-extra/clangd/Protocol.h
+++ b/clang-tools-extra/clangd/Protocol.h
@@ -592,7 +592,7 @@ bool fromJSON(const llvm::json::Value &, 
ConfigurationSettings &,
 /// Clangd extension: parameters configurable at `initialize` time.
 /// LSP defines this type as `any`.
 struct InitializationOptions {
-  // What we can change throught the didChangeConfiguration request, we can
+  // What we can change through the didChangeConfiguration request, we can
   // also set through the initialize request (initializationOptions field).
   ConfigurationSettings ConfigSettings;
 



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

[PATCH] D157794: [Driver] Make /Zi and /Z7 aliases of -g rather than handling them specially

2023-08-14 Thread Reid Kleckner via Phabricator via cfe-commits

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

So, as I understand the discussion, after this patch, it will still be possible 
to get dwarf and codeview in the same compile, but users will have to resort to 
cc1 flags.

I think that's a good end state. I would like to retain the ability to generate 
both, but the driver should make it very hard to enable this mode. Burying it 
in cc1 flags seems like a good end state. Comments about lack of testing are 
valid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157794

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

[PATCH] D156205: [clang][WebAssembly] Link crt1 even in case of -shared

2023-08-14 Thread Sam Clegg via Phabricator via cfe-commits

sbc100 added a comment.

(update change title to use the `[clang][WebAssembly]`convention)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156205

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

[PATCH] D157833: [C++20] [Coroutines] Mark await_suspend as noinline if the awaiter is not empty

2023-08-14 Thread John McCall via Phabricator via cfe-commits

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:5496
+  // execution of the await_suspend. To achieve this, we need to prevent the
+  // await_suspend get inlined before CoroSplit pass.
+  //

Suggestion:

> The `await_suspend` call performed by `co_await` is essentially asynchronous
> to the execution of the coroutine.  Inlining it normally into an unsplit 
> coroutine
> can cause miscompilation because the coroutine CFG misrepresents the true
> control flow of the program: things that happen in the `await_suspend` are not
> guaranteed to happen prior to the resumption of the coroutine, and things that
> happen after the resumption of the coroutine (including its exit and the
> potential deallocation of the coroutine frame) are not guaranteed to happen
> only after the end of `await_suspend`.
>
> The short-term solution to this problem is to mark the call as uninlinable.
> But we don't want to do this if the call is known to be trivial, which is very
> common.

We don't need to get into the long-term solution here, but I'd suggest a pattern
like:

```
  call @llvm.coro.await_suspend(token %suspend, ptr %awaiter, ptr 
@awaitSuspendFn)
```

and then we just replace that with the normal call during function splitting.  
I guess we'd have to make sure we did everything right with the return values 
and exceptions, which might be a pain.  But it does have the really nice 
property that we could move all of this "is it trivial" analysis into the LLVM 
passes: if we decide that it's safe to inline the function before splitting 
(e.g. `awaitSuspendFn` doesn't contain any uses of `this`), then the 
optimization is as simple as doing the replacement in CoroEarly instead of 
after splitting.



Comment at: clang/lib/CodeGen/CGCoroutine.cpp:146
+// We can't use (CoroutineSuspendExpr).getCommonExpr()->getType() directly
+// since its type may be AutoType, ElaboratedType, ...
+class AwaiterTypeFinder : public TypeVisitor {

Please just do `getNonReferenceType()->getAsCXXRecordDecl()` and conservatively 
say it might escape if that's null.  And you really don't need to repeat the 
well-formedness checks about the awaiter type that Sema is presumably already 
going to have done; that's just asking for extra work if this code ever 
diverges from Sema, e.g. if the language standard changes.  So you should be 
able to do all of this without a visitor.



Comment at: clang/lib/CodeGen/CGCoroutine.cpp:165
+// to give users better user experience. It doesn't matter with the
+// correctness but 1 byte memory overhead.
+#ifdef NDEBUG

I'm sorry, but I'm really confused about what you're doing here.  In general, 
we really don't want compiler behavior to change based on whether we're running 
a release version of the compiler.  And this would be disabling the 
optimization completely in release builds?

Are you maybe trying to check whether we're doing an unoptimized build of the 
*program*?  That's `CodeGenOpts.OptimizationLevel == 0`.



Comment at: clang/lib/CodeGen/CodeGenFunction.h:351
 
+  bool maySuspendLeakCoroutineHandle() const {
+return isCoroutine() && CurCoro.MaySuspendLeak;

(1) I'd prefer that we use the term "escape" over "leak" here.
(2) None of these bugs require us to escape the coroutine handle.

Maybe `mustPreventInliningOfAwaitSuspend`?  It's not really a general property.


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

https://reviews.llvm.org/D157833

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

[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

2023-08-14 Thread Qizhi Hu via Phabricator via cfe-commits

jcsxky updated this revision to Diff 550030.
jcsxky added a comment.

fixed according to the review and add testcase to StructuralEquivalenceTest.cpp.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156693

Files:
  clang/include/clang/AST/ASTStructuralEquivalence.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  clang/unittests/AST/StructuralEquivalenceTest.cpp

Index: clang/unittests/AST/StructuralEquivalenceTest.cpp
===
--- clang/unittests/AST/StructuralEquivalenceTest.cpp
+++ clang/unittests/AST/StructuralEquivalenceTest.cpp
@@ -1,5 +1,6 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTStructuralEquivalence.h"
+#include "clang/AST/DeclTemplate.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Testing/CommandLineArgs.h"
@@ -130,15 +131,18 @@
 return makeStmts(Wrap(SrcCode0), Wrap(SrcCode1), Lang, AMatcher);
   }
 
-  bool testStructuralMatch(Decl *D0, Decl *D1) {
+  bool testStructuralMatch(Decl *D0, Decl *D1,
+   bool IgnoreTemplateParmDepth = false) {
 llvm::DenseSet> NonEquivalentDecls01;
 llvm::DenseSet> NonEquivalentDecls10;
 StructuralEquivalenceContext Ctx01(
-D0->getASTContext(), D1->getASTContext(),
-NonEquivalentDecls01, StructuralEquivalenceKind::Default, false, false);
+D0->getASTContext(), D1->getASTContext(), NonEquivalentDecls01,
+StructuralEquivalenceKind::Default, false, false, false,
+IgnoreTemplateParmDepth);
 StructuralEquivalenceContext Ctx10(
-D1->getASTContext(), D0->getASTContext(),
-NonEquivalentDecls10, StructuralEquivalenceKind::Default, false, false);
+D1->getASTContext(), D0->getASTContext(), NonEquivalentDecls10,
+StructuralEquivalenceKind::Default, false, false, false,
+IgnoreTemplateParmDepth);
 bool Eq01 = Ctx01.IsEquivalent(D0, D1);
 bool Eq10 = Ctx10.IsEquivalent(D1, D0);
 EXPECT_EQ(Eq01, Eq10);
@@ -1689,6 +1693,45 @@
   EXPECT_FALSE(testStructuralMatch(t));
 }
 
+TEST_F(StructuralEquivalenceTemplateTest,
+   ClassTemplateEquivalentFriendClassTemplate) {
+  auto t = makeDecls(
+  R"(
+  template 
+  class A {
+  public:
+template 
+friend class A;
+
+A(T x)  :x(x) {}
+
+  private:
+T x;
+  };
+  )",
+
+  R"(
+  template 
+  class A {
+  public:
+template 
+friend class A;
+
+A(T x) : x(x) {}
+
+  private:
+T x;
+  };
+
+  A a1(0);
+  )",
+  Lang_CXX03, friendDecl(),
+  classTemplateDecl(has(cxxRecordDecl(hasDefinition(), hasName("A");
+  auto *Friend = cast(get<0>(t));
+  EXPECT_FALSE(testStructuralMatch(Friend->getFriendDecl(), get<1>(t)));
+  EXPECT_TRUE(testStructuralMatch(Friend->getFriendDecl(), get<1>(t), true));
+}
+
 TEST_F(
 StructuralEquivalenceTemplateTest,
 ClassTemplSpecWithInequivalentShadowedTemplArg) {
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4246,6 +4246,58 @@
   EXPECT_TRUE(Imported->getPreviousDecl());
 }
 
+TEST_P(ImportFriendClasses, SkipComparingFriendTemplateDepth) {
+  Decl *ToTU = getToTuDecl(
+  R"(
+  template 
+  class A;
+
+  template 
+  class A {
+  public:
+template 
+friend class A;
+
+A(T x)  :x(x) {}
+
+  private:
+T x;
+  };
+  )",
+  Lang_CXX11);
+
+  auto *Fwd = FirstDeclMatcher().match(
+  ToTU,
+  classTemplateDecl(has(cxxRecordDecl(hasDefinition(), hasName("A");
+  Decl *FromTU = getTuDecl(
+  R"(
+  template 
+  class A;
+
+  template 
+  class A {
+  public:
+template 
+friend class A;
+
+A(T x) : x(x) {}
+
+  private:
+T x;
+  };
+
+  A a1(0);
+  )",
+  Lang_CXX11, "input1.cc");
+  auto *FromA = FirstDeclMatcher().match(
+  FromTU,
+  classTemplateDecl(has(cxxRecordDecl(hasDefinition(), hasName("A");
+  auto *ToA = Import(FromA, Lang_CXX11);
+  EXPECT_TRUE(ToA);
+  EXPECT_EQ(Fwd->getTemplatedDecl()->getTypeForDecl(),
+ToA->getTemplatedDecl()->getTypeForDecl());
+}
+
 TEST_P(ImportFriendClasses,
ImportOfClassTemplateDefinitionShouldConnectToFwdFriend) {
   Decl *ToTU = getToTuDecl(
Index: clang/lib/AST/ASTStructuralEquivalence.cpp
===
--- clang/lib/AST/ASTStructuralEquivalence.cpp
+++ clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -1092,7 +1092,8 @@
   case Type::TemplateTypeParm: {
 const auto *Parm1 = cast(T1);
 const auto *Parm2 = cast(T2);
-

[PATCH] D157362: [RISCV] Add MC layer support for Zicfilp.

2023-08-14 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:665
 
+let Predicates = [HasStdExtZicfilp] in {
+def : InstAlias<"lpad $imm20", (AUIPC X0, uimm20:$imm20)>;

There is a designated spot in this file for InstAliases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157362

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

[PATCH] D157632: [Profile] Allow online merging with debug info correlation.

2023-08-14 Thread Ellis Hoag via Phabricator via cfe-commits

ellis added inline comments.



Comment at: compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c:24
+
+// RUN: rm -rf %t.profdir && mkdir %t.profdir
+

Let's move this line down since we don't use it until line 31.



Comment at: compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c:30
+
+// RUN: %clang_pgogen -o %t -g -mllvm --debug-info-correlate -mllvm 
--disable-vp=true %S/../Inputs/instrprof-debug-info-correlate-main.cpp 
%S/../Inputs/instrprof-debug-info-correlate-foo.cpp
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.proflite %run %t

This looks to be the same as line 2. Lets just reuse that executable.



Comment at: compiler-rt/test/profile/Darwin/instrprof-debug-info-correlate.c:39
+
+// RUN: %clang_pgogen -o %t.cov -g -mllvm --debug-info-correlate -mllvm 
-pgo-function-entry-coverage -mllvm --disable-vp=true 
%S/../Inputs/instrprof-debug-info-correlate-main.cpp 
%S/../Inputs/instrprof-debug-info-correlate-foo.cpp
+// RUN: env LLVM_PROFILE_FILE=%t.profdir/%m.cov.proflite %run %t.cov

Same here. This is the same as line 12. (And I believe these comments also 
apply to the Linux test)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157632

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

[PATCH] D157777: [ASTMatcher] Add matcher for 'MacroQualifiedType'

2023-08-14 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7261
 
+/// Matches macro qualified types.
+///

How about: Matches qualified types when the qualifier is applied via a macro.

and then a second example like:
```
int * const qual_ptr;

#define nonnull _Nonnull
int * const nonnull macro_qual_ptr;
```
where we match `macro_qual_ptr` but not `qual_ptr`.



Comment at: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp:1842-1847
+  EXPECT_TRUE(matches(R"(
+#define CDECL __attribute__((cdecl))
+typedef void (CDECL *X)();
+  )",
+  typedefDecl(hasName("X"), hasType(pointerType(pointee(
+macroQualifiedType()));

I'd appreciate extra test coverage from the example I posted above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D15

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

[PATCH] D157905: [include-cleaner] Filter references to identity macros

2023-08-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added a project: All.
kadircet requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Despite being true positives, these results just confuse users. So
filter them out.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157905

Files:
  clang-tools-extra/include-cleaner/lib/Analysis.cpp
  clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp


Index: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -483,5 +483,29 @@
 Pair(Code.point("partial"), UnorderedElementsAre(Partial);
 }
 
+TEST_F(WalkUsedTest, IgnoresIdentityMacros) {
+  llvm::Annotations Code(R"cpp(
+  #include "header.h"
+  void $bar^bar() {
+$stdin^stdin();
+  }
+  )cpp");
+  Inputs.Code = Code.code();
+  Inputs.ExtraFiles["header.h"] = guard(R"cpp(
+  #include "inner.h"
+  void stdin();
+  )cpp");
+  Inputs.ExtraFiles["inner.h"] = guard(R"cpp(
+  #define stdin stdin
+  )cpp");
+
+  TestAST AST(Inputs);
+  auto  = AST.sourceManager();
+  auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID()));
+  EXPECT_THAT(offsetToProviders(AST, SM),
+  UnorderedElementsAre(
+  // FIXME: we should have a reference from stdin to header.h
+  Pair(Code.point("bar"), UnorderedElementsAre(MainFile;
+}
 } // namespace
 } // namespace clang::include_cleaner
Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -28,10 +28,29 @@
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
+#include 
 #include 
 
 namespace clang::include_cleaner {
 
+namespace {
+bool shouldSkipMacro(const Macro ) {
+  static const auto *MacroNamesToIgnore = new llvm::StringSet<>{
+  // C standard says these are implementation defined macros, hence most of
+  // the standard library providers implement it by defining these as 
macros
+  // that resolve to themselves.
+  // This results in surprising behavior from users point of view (we
+  // generate a usage of stdio.h, in places unrelated to standard library).
+  // FIXME: Also eliminate the false positives by treating declarations
+  // resulting from these expansions as used.
+  "stdin",
+  "stdout",
+  "stderr",
+  };
+  return MacroNamesToIgnore->contains(M.Name->getName());
+}
+} // namespace
+
 void walkUsed(llvm::ArrayRef ASTRoots,
   llvm::ArrayRef MacroRefs,
   const PragmaIncludes *PI, const SourceManager ,
@@ -51,7 +70,8 @@
   }
   for (const SymbolReference  : MacroRefs) {
 assert(MacroRef.Target.kind() == Symbol::Macro);
-if (!SM.isWrittenInMainFile(SM.getSpellingLoc(MacroRef.RefLocation)))
+if (!SM.isWrittenInMainFile(SM.getSpellingLoc(MacroRef.RefLocation)) ||
+shouldSkipMacro(MacroRef.Target.macro()))
   continue;
 CB(MacroRef, headersForSymbol(MacroRef.Target, SM, PI));
   }


Index: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -483,5 +483,29 @@
 Pair(Code.point("partial"), UnorderedElementsAre(Partial);
 }
 
+TEST_F(WalkUsedTest, IgnoresIdentityMacros) {
+  llvm::Annotations Code(R"cpp(
+  #include "header.h"
+  void $bar^bar() {
+$stdin^stdin();
+  }
+  )cpp");
+  Inputs.Code = Code.code();
+  Inputs.ExtraFiles["header.h"] = guard(R"cpp(
+  #include "inner.h"
+  void stdin();
+  )cpp");
+  Inputs.ExtraFiles["inner.h"] = guard(R"cpp(
+  #define stdin stdin
+  )cpp");
+
+  TestAST AST(Inputs);
+  auto  = AST.sourceManager();
+  auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID()));
+  EXPECT_THAT(offsetToProviders(AST, SM),
+  UnorderedElementsAre(
+  // FIXME: we should have a reference from stdin to header.h
+  Pair(Code.point("bar"), UnorderedElementsAre(MainFile;
+}
 } // namespace
 } // namespace clang::include_cleaner
Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -28,10 +28,29 @@
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
+#include 
 #include 
 
 namespace clang::include_cleaner {
 
+namespace {

[PATCH] D157793: [Headers] Add missing __need_ macros to stdarg.h

2023-08-14 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D157793#4585685 , @iana wrote:

> In D157793#4585268 , @aaron.ballman 
> wrote:
>
>> In D157793#4583698 , @iana wrote:
>>
>>> In D157793#4583663 , @MaskRay 
>>> wrote:
>>>
> Apple needs a __need_ macro for va_list. Add one, and also ones for 
> va_start/va_arg/va_end, __va_copy, va_copy.

 Do you have a link to the specification or the source that this is needed?
>>>
>>> We need is so that our  doesn't have to redeclare 
>>> va_list and doesn't pull in all of stdarg.h either. As far as I know 
>>> there's no specification for this (or stddef.h) being include-able in 
>>> pieces.
>>
>> I'm not opposed, but this adds significant complexity to support a 
>> questionable use case -- the C standard library headers are not intended to 
>> be partially included, so if the plan is to use these `__need` macros in all 
>> of the headers we provide, I think that should go through an RFC process to 
>> be sure we want to maintain the added complexity. (Also, who is "we" in this 
>> case?)
>
> We is Apple.

Thank you!

> We have a  header that is generating redeclaration 
> warnings in modules mode. I can't just include  instead since it 
> would change the semantics of . stdarg.h and stddef.h 
> appear to be the only clang headers that Apple needs to use in pieces though, 
> I don't think we should add this complexity to any other headers for 
> consistency or "just because".

Thank you for the explanation; if it's just these two files, I think that's 
pretty reasonable. I was worried it was also going to spill into limits.h, 
stdint.h, and other freestanding headers.




Comment at: clang/test/Headers/stdarg.c:2
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=c89 
-Werror=implicit-function-declaration -std=c89 %t/stdarg0.c
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=c99 
-Werror=implicit-function-declaration -std=c99 %t/stdarg0.c

I think the triples can be removed.



Comment at: clang/test/Headers/stdarg.c:11
+static void f(int p, ...) {
+__gnuc_va_list g; // c89-error{{undeclared}} c99-error{{undeclared}}
+va_list v; // c89-error{{undeclared}} c99-error{{undeclared}}

You should spell out the first instance of the diagnostic



Comment at: clang/test/Headers/stdarg.c:34
+__va_copy(g, v);
+va_copy(g, v); // c89-error{{implicit}} c89-note{{va_copy}} 
c99-no-diagnostics
+}

You should spell out these diagnostics, and I think `c99-no-diagnostics` should 
be placed up by the RUN lines so it's more obvious that we expect no 
diagnostics in C99 mode.

Actually, this file should perhaps be split into two files as they're testing 
different things. (I was tripped up seeing no-diagnostics but we have 
`c99-error` entries above, that's when I realized the split file was being used 
differently in the RUN lines which is a bit novel.) But I'm not certain I fully 
understand what your comment means about why we're using split file in the 
first place, so I might be missing something.



Comment at: clang/test/Headers/stdargneeds.c:2
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify 
-Werror=implicit-function-declaration -std=c89 %t/stdargneeds0.c
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify 
-Werror=implicit-function-declaration -std=c89 %t/stdargneeds1.c

Same suggestions in this file regarding triples and spelling out diagnostics, 
but I think a single file makes sense as this is testing each of the 
`__needs_whatever` macros the same way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157793

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

[PATCH] D157794: [Driver] Make /Zi and /Z7 aliases of -g rather than handling them specially

2023-08-14 Thread Shoaib Meenai via Phabricator via cfe-commits

smeenai added a subscriber: mstorsjo.
smeenai added inline comments.



Comment at: clang/test/Driver/cl-options.c:567
 
-// This test was super sneaky: "/Z7" means "line-tables", but "-gdwarf" occurs
-// later on the command line, so it should win. Interestingly the cc1 arguments
-// came out right, but had wrong semantics, because an invariant assumed by
-// CompilerInvocation was violated: it expects that at most one of {gdwarfN,
-// line-tables-only} appear. If you assume that, then you can safely use
-// Args.hasArg to test whether a boolean flag is present without caring
-// where it appeared. And for this test, it appeared to the left of -gdwarf
-// which made it "win". This test could not detect that bug.
+// If We specify both /Z7 and -gdwarf we should get dwarf, not codeview.
 // RUN: %clang_cl /Z7 -gdwarf /c -### -- %s 2>&1 | FileCheck 
-check-prefix=Z7_gdwarf %s

bogner wrote:
> smeenai wrote:
> > bogner wrote:
> > > rnk wrote:
> > > > I think Meta folks depend on a mode that emits both codeview and DWARF. 
> > > > +@smeenai
> > > Hopefully if that's the case there's a better test somewhere than this 
> > > one that discusses in detail how `/Z7` is an alias for 
> > > `-gline-tables-only`, which hasn't been true since 2017.
> > Thanks for the headers up :) We don't depend on that mode any more though, 
> > so no problems on our end.
> Also note that this change does not affect what happens if we specify both 
> `-gcodeview` and `-gdwarf` together. That continues to pass both `-gcodeview` 
> and `-dwarf-version=...` to `clang -cc1`. However, I don't see any tests that 
> actually check that behaviour, so if that is a mode that's useful to keep 
> working we definitely have some gaps there.
> 
> This change means that if you want to ask `-cc1` for dwarf *and* codeview 
> with `/Z7` or `/Zi`, you'll need to specify `-gdwarf` and `-gcodeview` now. 
> This matches how you would get both sets of options to render with `-g`, 
> which I think makes sense.
Yeah, that sounds good. IIRC when we were using the joint codeview + DWARF mode 
it was with the gcc-style Clang driver and not clang-cl, and we passed `-gdwarf 
-gcodeview` or similar to enable it anyway. We don't need that anymore though; 
@mstorsjo would know if there's anything on the MinGW side that cares for it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157794

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

[PATCH] D157904: Improve reliability of CompilationDatabaseTest

2023-08-14 Thread Ellis Hoag via Phabricator via cfe-commits

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157904

Files:
  clang/unittests/Tooling/CompilationDatabaseTest.cpp


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -6,9 +6,6 @@
 //
 
//===--===//
 
-#include "clang/AST/DeclCXX.h"
-#include "clang/AST/DeclGroup.h"
-#include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/FileMatchTrie.h"
 #include "clang/Tooling/JSONCompilationDatabase.h"
@@ -24,6 +21,8 @@
 
 using testing::ElementsAre;
 using testing::EndsWith;
+using testing::IsEmpty;
+using testing::UnorderedElementsAreArray;
 
 static void expectFailure(StringRef JSONDatabase, StringRef Explanation) {
   std::string ErrorMessage;
@@ -83,8 +82,8 @@
 
 TEST(JSONCompilationDatabase, GetAllFiles) {
   std::string ErrorMessage;
-  EXPECT_EQ(std::vector(),
-getAllFiles("[]", ErrorMessage, JSONCommandLineSyntax::Gnu))
+  EXPECT_THAT(getAllFiles("[]", ErrorMessage, JSONCommandLineSyntax::Gnu),
+  IsEmpty())
   << ErrorMessage;
 
   std::vector expected_files;
@@ -97,8 +96,7 @@
   expected_files.push_back(std::string(PathStorage.str()));
   llvm::sys::path::native("//net/file1", PathStorage);
   expected_files.push_back(std::string(PathStorage.str()));
-  EXPECT_EQ(expected_files,
-getAllFiles(R"json(
+  EXPECT_THAT(getAllFiles(R"json(
 [
   {
 "directory": "//net/dir",
@@ -121,7 +119,8 @@
 "file": "//net/dir/foo/../file3"
   }
 ])json",
-ErrorMessage, JSONCommandLineSyntax::Gnu))
+  ErrorMessage, JSONCommandLineSyntax::Gnu),
+  UnorderedElementsAreArray(expected_files))
   << ErrorMessage;
 }
 
@@ -550,7 +549,7 @@
   CommandLine.push_back("two");
   FixedCompilationDatabase Database(".", CommandLine);
 
-  EXPECT_EQ(0ul, Database.getAllFiles().size());
+  EXPECT_THAT(Database.getAllFiles(), IsEmpty());
 }
 
 TEST(FixedCompilationDatabase, GetAllCompileCommands) {


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -6,9 +6,6 @@
 //
 //===--===//
 
-#include "clang/AST/DeclCXX.h"
-#include "clang/AST/DeclGroup.h"
-#include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/FileMatchTrie.h"
 #include "clang/Tooling/JSONCompilationDatabase.h"
@@ -24,6 +21,8 @@
 
 using testing::ElementsAre;
 using testing::EndsWith;
+using testing::IsEmpty;
+using testing::UnorderedElementsAreArray;
 
 static void expectFailure(StringRef JSONDatabase, StringRef Explanation) {
   std::string ErrorMessage;
@@ -83,8 +82,8 @@
 
 TEST(JSONCompilationDatabase, GetAllFiles) {
   std::string ErrorMessage;
-  EXPECT_EQ(std::vector(),
-getAllFiles("[]", ErrorMessage, JSONCommandLineSyntax::Gnu))
+  EXPECT_THAT(getAllFiles("[]", ErrorMessage, JSONCommandLineSyntax::Gnu),
+  IsEmpty())
   << ErrorMessage;
 
   std::vector expected_files;
@@ -97,8 +96,7 @@
   expected_files.push_back(std::string(PathStorage.str()));
   llvm::sys::path::native("//net/file1", PathStorage);
   expected_files.push_back(std::string(PathStorage.str()));
-  EXPECT_EQ(expected_files,
-getAllFiles(R"json(
+  EXPECT_THAT(getAllFiles(R"json(
 [
   {
 "directory": "//net/dir",
@@ -121,7 +119,8 @@
 "file": "//net/dir/foo/../file3"
   }
 ])json",
-ErrorMessage, JSONCommandLineSyntax::Gnu))
+  ErrorMessage, JSONCommandLineSyntax::Gnu),
+  UnorderedElementsAreArray(expected_files))
   << ErrorMessage;
 }
 
@@ -550,7 +549,7 @@
   CommandLine.push_back("two");
   FixedCompilationDatabase Database(".", CommandLine);
 
-  EXPECT_EQ(0ul, Database.getAllFiles().size());
+  EXPECT_THAT(Database.getAllFiles(), IsEmpty());
 }
 
 TEST(FixedCompilationDatabase, GetAllCompileCommands) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157794: [Driver] Make /Zi and /Z7 aliases of -g rather than handling them specially

2023-08-14 Thread Justin Bogner via Phabricator via cfe-commits

bogner added inline comments.



Comment at: clang/test/Driver/cl-options.c:567
 
-// This test was super sneaky: "/Z7" means "line-tables", but "-gdwarf" occurs
-// later on the command line, so it should win. Interestingly the cc1 arguments
-// came out right, but had wrong semantics, because an invariant assumed by
-// CompilerInvocation was violated: it expects that at most one of {gdwarfN,
-// line-tables-only} appear. If you assume that, then you can safely use
-// Args.hasArg to test whether a boolean flag is present without caring
-// where it appeared. And for this test, it appeared to the left of -gdwarf
-// which made it "win". This test could not detect that bug.
+// If We specify both /Z7 and -gdwarf we should get dwarf, not codeview.
 // RUN: %clang_cl /Z7 -gdwarf /c -### -- %s 2>&1 | FileCheck 
-check-prefix=Z7_gdwarf %s

smeenai wrote:
> bogner wrote:
> > rnk wrote:
> > > I think Meta folks depend on a mode that emits both codeview and DWARF. 
> > > +@smeenai
> > Hopefully if that's the case there's a better test somewhere than this one 
> > that discusses in detail how `/Z7` is an alias for `-gline-tables-only`, 
> > which hasn't been true since 2017.
> Thanks for the headers up :) We don't depend on that mode any more though, so 
> no problems on our end.
Also note that this change does not affect what happens if we specify both 
`-gcodeview` and `-gdwarf` together. That continues to pass both `-gcodeview` 
and `-dwarf-version=...` to `clang -cc1`. However, I don't see any tests that 
actually check that behaviour, so if that is a mode that's useful to keep 
working we definitely have some gaps there.

This change means that if you want to ask `-cc1` for dwarf *and* codeview with 
`/Z7` or `/Zi`, you'll need to specify `-gdwarf` and `-gcodeview` now. This 
matches how you would get both sets of options to render with `-g`, which I 
think makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157794

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

[PATCH] D157479: [Clang][DebugInfo] Emit narrower base types for structured binding declarations that bind to struct bitfields

2023-08-14 Thread Adrian Prantl via Phabricator via cfe-commits

aprantl added a comment.

> If no suitable integer type is found in the target, no debug information is 
> emitted anymore in order to prevent wrong debug output.

Why is emitting a bitfield type not an option?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157479

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

[PATCH] D157479: [Clang][DebugInfo] Emit narrower base types for structured binding declarations that bind to struct bitfields

2023-08-14 Thread Adrian Prantl via Phabricator via cfe-commits

aprantl added inline comments.



Comment at: clang/test/CodeGenCXX/debug-info-structured-binding-bitfield.cpp:525
+// CHECK: !202 = !DILocation(line: 279, column: 8, scope: !194)
+// CHECK: !203 = !DILocation(line: 279, column: 17, scope: !194)
+// CHECK: !204 = !DILocation(line: 280, column: 1, scope: !194)

This test is going to to be a nightmare to maintain since it's hardcoding all 
the metadata numbering. Please use FileCheck variables `![[VAR:[0-9]+]]` to 
refer to other fields. Also, this test is probably checking too much.
The primary thing this patch changes is the data types of the fields, so the 
CHECK lines should focus on that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157479

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

[PATCH] D156821: [CodeGen] [ubsan] Respect integer overflow handling in abs builtin

2023-08-14 Thread Artem Labazov via Phabricator via cfe-commits

artem updated this revision to Diff 550013.
artem marked 5 inline comments as done.
artem edited the summary of this revision.
artem added a comment.

Fixed review comments


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

https://reviews.llvm.org/D156821

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/abs-overflow.c
  compiler-rt/test/ubsan/TestCases/Misc/abs.cpp

Index: compiler-rt/test/ubsan/TestCases/Misc/abs.cpp
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/Misc/abs.cpp
@@ -0,0 +1,30 @@
+// REQUIRES: arch=x86_64
+//
+// RUN: %clangxx -fsanitize=signed-integer-overflow -w %s -O3 -o %t
+// RUN: %run %t 2>&1 | FileCheck %s --check-prefix=RECOVER
+//
+// RUN: %clangxx -fsanitize=signed-integer-overflow -fno-sanitize-recover=signed-integer-overflow -w %s -O3 -o %t.abort
+// RUN: not %run %t.abort 2>&1 | FileCheck %s --check-prefix=ABORT
+
+#include 
+#include 
+
+int main() {
+  // ABORT: abs.cpp:[[@LINE+3]]:17: runtime error: negation of -{{[0-9]+}} cannot be represented in type 'int'; cast to an unsigned type to negate this value to itself
+  // RECOVER: abs.cpp:[[@LINE+2]]:17: runtime error: negation of -{{[0-9]+}} cannot be represented in type 'int'; cast to an unsigned type to negate this value to itself
+  // RECOVER: abs.cpp:[[@LINE+2]]:7: runtime error: negation of -{{[0-9]+}} cannot be represented in type 'int'; cast to an unsigned type to negate this value to itself
+  __builtin_abs(INT_MIN);
+  abs(INT_MIN);
+
+  // RECOVER: abs.cpp:[[@LINE+2]]:18: runtime error: negation of -{{[0-9]+}} cannot be represented in type 'long'; cast to an unsigned type to negate this value to itself
+  // RECOVER: abs.cpp:[[@LINE+2]]:8: runtime error: negation of -{{[0-9]+}} cannot be represented in type 'long'; cast to an unsigned type to negate this value to itself
+  __builtin_labs(LONG_MIN);
+  labs(LONG_MIN);
+
+  // RECOVER: abs.cpp:[[@LINE+2]]:19: runtime error: negation of -{{[0-9]+}} cannot be represented in type 'long long'; cast to an unsigned type to negate this value to itself
+  // RECOVER: abs.cpp:[[@LINE+2]]:9: runtime error: negation of -{{[0-9]+}} cannot be represented in type 'long long'; cast to an unsigned type to negate this value to itself
+  __builtin_llabs(LLONG_MIN);
+  llabs(LLONG_MIN);
+
+  return 0;
+}
Index: clang/test/CodeGen/abs-overflow.c
===
--- /dev/null
+++ clang/test/CodeGen/abs-overflow.c
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm %s -o - -fwrapv -fsanitize=signed-integer-overflow | FileCheck %s --check-prefix=WRAPV
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm %s -o - -ftrapv | FileCheck %s --check-prefixes=BOTH-TRAP,TRAPV
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm %s -o - -fsanitize=signed-integer-overflow | FileCheck %s --check-prefixes=BOTH-TRAP,CATCH_UB
+// COM: TODO: Support -ftrapv-handler.
+
+extern int abs(int x);
+
+int absi(int x) {
+// WRAPV:   [[NEG:%.*]] = sub i32 0, [[X:%.*]]
+// WRAPV:   [[CMP:%.*]] = icmp slt i32 [[X]], 0
+// WRAPV:   [[SEL:%.*]] = select i1 [[CMP]], i32 [[NEG]], i32 [[X]]
+//
+// BOTH-TRAP:   [[NEG:%.*]] = call { i32, i1 } @llvm.ssub.with.overflow.i32(i32 0, i32 [[X:%.*]])
+// BOTH-TRAP:   [[NEGV:%.*]] = extractvalue { i32, i1 } [[NEG]], 0
+// BOTH-TRAP:   [[OFL:%.*]] = extractvalue { i32, i1 } [[NEG]], 1
+// BOTH-TRAP:   [[NOFL:%.*]] = xor i1 [[OFL]], true
+// BOTH-TRAP:   br i1 [[NOFL]], label %[[CONT:.*]], label %[[TRAP:[a-zA-Z_.]*]]
+// BOTH-TRAP:   [[TRAP]]:
+// TRAPV-NEXT:llvm.ubsantrap
+// CATCH_UB:  @__ubsan_handle_negate_overflow
+// BOTH-TRAP-NEXT:unreachable
+// BOTH-TRAP:   [[CONT]]:
+// BOTH-TRAP-NEXT:[[ABSCOND:%.*]] = icmp slt i32 [[X]], 0
+// BOTH-TRAP-NEXT:select i1 [[ABSCOND]], i32 [[NEGV]], i32 [[X]]
+  return abs(x);
+}
+
+int babsi(int x) {
+// WRAPV:   [[NEG:%.*]] = sub i32 0, [[X:%.*]]
+// WRAPV:   [[CMP:%.*]] = icmp slt i32 [[X]], 0
+// WRAPV:   [[SEL:%.*]] = select i1 [[CMP]], i32 [[NEG]], i32 [[X]]
+//
+// BOTH-TRAP:   [[NEG:%.*]] = call { i32, i1 } @llvm.ssub.with.overflow.i32(i32 0, i32 [[X:%.*]])
+// BOTH-TRAP:   [[NEGV:%.*]] = extractvalue { i32, i1 } [[NEG]], 0
+// BOTH-TRAP:   [[OFL:%.*]] = extractvalue { i32, i1 } [[NEG]], 1
+// BOTH-TRAP:   [[NOFL:%.*]] = xor i1 [[OFL]], true
+// BOTH-TRAP:   br i1 [[NOFL]], label %[[CONT:.*]], label %[[TRAP:[a-zA-Z_.]*]]
+// BOTH-TRAP:   [[TRAP]]:
+// TRAPV-NEXT:llvm.ubsantrap
+// CATCH_UB:  @__ubsan_handle_negate_overflow
+// BOTH-TRAP-NEXT:unreachable
+// BOTH-TRAP:   [[CONT]]:
+// BOTH-TRAP-NEXT:[[ABSCOND:%.*]] = icmp slt i32 [[X]], 0
+// BOTH-TRAP-NEXT:select i1 [[ABSCOND]], i32 [[NEGV]], i32 [[X]]
+  return __builtin_abs(x);
+}
Index: clang/lib/CodeGen/CGBuiltin.cpp

[PATCH] D157610: [include-cleaner][clangd][clang-tidy] Ignore resource dir during include-cleaner analysis.

2023-08-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp:122
   llvm::DenseSet SeenSymbols;
+  std::string ResourceDir = HS->getHeaderSearchOpts().ResourceDir;
   // FIXME: Find a way to have less code duplication between include-cleaner

VitaNuo wrote:
> kadircet wrote:
> > let's use `HS->getModuleMap().getBuiltinDir()` then we can get away with 
> > just comparing that pointer to `H.physical()->getLastRef().getDir()` (same 
> > applies to all the other places as well)
> This only works in `clangd` code for me. I get `nullptr` for 
> `HS->getModuleMap().getBuiltinDir()` in other places (Clang Tidy check and 
> the library).
hmm that's probably because you're not setting up the include structure 
correctly. resource-dir isn't the `builtindir`, it's `resource-dir + 
"/include"` (the comments i had in the clangd unittests applies in a similar 
manner to other tests as well).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157610

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

[PATCH] D157793: [Headers] Add missing __need_ macros to stdarg.h

2023-08-14 Thread Ian Anderson via Phabricator via cfe-commits

iana added a comment.

In D157793#4585268 , @aaron.ballman 
wrote:

> In D157793#4583698 , @iana wrote:
>
>> In D157793#4583663 , @MaskRay 
>> wrote:
>>
 Apple needs a __need_ macro for va_list. Add one, and also ones for 
 va_start/va_arg/va_end, __va_copy, va_copy.
>>>
>>> Do you have a link to the specification or the source that this is needed?
>>
>> We need is so that our  doesn't have to redeclare 
>> va_list and doesn't pull in all of stdarg.h either. As far as I know there's 
>> no specification for this (or stddef.h) being include-able in pieces.
>
> I'm not opposed, but this adds significant complexity to support a 
> questionable use case -- the C standard library headers are not intended to 
> be partially included, so if the plan is to use these `__need` macros in all 
> of the headers we provide, I think that should go through an RFC process to 
> be sure we want to maintain the added complexity. (Also, who is "we" in this 
> case?)

We is Apple. We have a  header that is generating 
redeclaration warnings in modules mode. I can't just include  instead 
since it would change the semantics of . stdarg.h and 
stddef.h appear to be the only clang headers that Apple needs to use in pieces 
though, I don't think we should add this complexity to any other headers for 
consistency or "just because".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157793

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

[clang] 700ca0e - [NFC] Red Hat is two words

2023-08-14 Thread Josh Stone via cfe-commits


Author: Josh Stone
Date: 2023-08-14T10:10:10-07:00
New Revision: 700ca0e7432d4ef485900f16d9f051e2bbc8523c

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

LOG: [NFC] Red Hat is two words

Added: 


Modified: 
clang/lib/Driver/Distro.cpp
llvm/docs/Security.rst

Removed: 




diff  --git a/clang/lib/Driver/Distro.cpp b/clang/lib/Driver/Distro.cpp
index 6e0087565941ea..005c31bd38893c 100644
--- a/clang/lib/Driver/Distro.cpp
+++ b/clang/lib/Driver/Distro.cpp
@@ -112,7 +112,7 @@ static Distro::DistroType 
DetectDistro(llvm::vfs::FileSystem ) {
   if (Version != Distro::UnknownDistro)
 return Version;
 
-  // Otherwise try some distro-specific quirks for RedHat...
+  // Otherwise try some distro-specific quirks for Red Hat...
   llvm::ErrorOr> File =
   VFS.getBufferForFile("/etc/redhat-release");
 

diff  --git a/llvm/docs/Security.rst b/llvm/docs/Security.rst
index 11acac33f0a19f..2729541facbfe6 100644
--- a/llvm/docs/Security.rst
+++ b/llvm/docs/Security.rst
@@ -40,7 +40,7 @@ The members of the group represent a wide cross-section of 
the community, and me
 * Dimitry Andric (individual; FreeBSD) [dim]
 * Ed Maste (individual; FreeBSD) [emaste]
 * George Burgess IV (Google) [george.burgess.iv]
-* Josh Stone (RedHat; Rust) [cuviper]
+* Josh Stone (Red Hat; Rust) [cuviper]
 * Kate McInnes (Apple) []
 * Kristof Beyls (ARM) [kristof.beyls]
 * Matthew Riley (Google) [mattdr]



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

[PATCH] D141757: [clangd] allow extracting to variable for lambda expressions

2023-08-14 Thread Julian Schmidt via Phabricator via cfe-commits

5chmidti marked an inline comment as done.
5chmidti added inline comments.



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:91
+
+// Local variables declared inside of the selected lambda cannot go out of
+// scope. The DeclRefExprs that are important are the variables captured 
and

nridge wrote:
> Looking at 
> [RecursiveASTVisitor::TraverseLambdaExpr](https://searchfox.org/llvm/rev/094539cfcb46f55824402565e5c18580df689a67/clang/include/clang/AST/RecursiveASTVisitor.h#2652-2691),
>  there are a few things it traverses in addition to the lambda's parameters 
> and body (which we are saying are ok to skip) and the lambda's captures 
> (which we are traversing).
> 
> For example, the lambda may have an explicit result type which references a 
> variable in scope:
> 
> ```
> int main() {
>   if (int a = 1)
> if ([[ []() -> decltype(a){ return 1; } ]] () == 4)
>   a = a + 1;
> }
> 
> ```
> 
> Here, extracting the lambda succeeds, and the reference to `a` in 
> `decltype(a)` becomes dangling.
I'll update the diff when I've implemented this. A requires clause may 
reference a variable like `a` as well.



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:182-184
+if (InsertionPoint->Parent->ASTNode.get() != nullptr) {
+  return false;
+}

nridge wrote:
> 5chmidti wrote:
> > This is supposed to stop the following invalid code from happening:
> > ```
> > void foo() {
> >   int placeholder = 42;
> >   [](int x = placeholder {};
> >   extern void bar(int x = placeholder);
> > }
> > ```
> > 
> > clangd does not seem to support extracting from the initializers of 
> > defaulted parameters, should I keep the condition as is, or should I do 
> > something different here? It is legal to have default arguments in global 
> > scope (examples below).
> > 
> > The following insertions could exist (out of scope for this patch):
> > ```
> > int placeholder = 42;
> > void foo() {
> >   [](int x = placeholder {};
> >   extern void bar(int x = placeholder);
> > }
> > ```
> > ```
> > struct X {
> >   static inline int placeholder = 42;
> >   void foo(int x = placeholder) {}
> > };
> > ```
> > 
> > Either way, I'll have to adjust the comment because this is not just to 
> > stop default parameter initializers in lambdas from being extracted to a 
> > local scope.
> I'm having trouble following this comment. Can you give the testcase that 
> would produce this invalid code?
I provided test cases in lines 147-150 in ExtractVariableTests.cpp. For the 
example given in my comment, the test case would be:
Pre:
```
void foo() {
  [](int x = [[42]]) {};
}
```
(erroneous) Post:
```
void foo() {
  int placeholder = 42;
  [](int x = placeholder) {};
}
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141757

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

[PATCH] D157794: [Driver] Make /Zi and /Z7 aliases of -g rather than handling them specially

2023-08-14 Thread Shoaib Meenai via Phabricator via cfe-commits

smeenai added inline comments.



Comment at: clang/test/Driver/cl-options.c:567
 
-// This test was super sneaky: "/Z7" means "line-tables", but "-gdwarf" occurs
-// later on the command line, so it should win. Interestingly the cc1 arguments
-// came out right, but had wrong semantics, because an invariant assumed by
-// CompilerInvocation was violated: it expects that at most one of {gdwarfN,
-// line-tables-only} appear. If you assume that, then you can safely use
-// Args.hasArg to test whether a boolean flag is present without caring
-// where it appeared. And for this test, it appeared to the left of -gdwarf
-// which made it "win". This test could not detect that bug.
+// If We specify both /Z7 and -gdwarf we should get dwarf, not codeview.
 // RUN: %clang_cl /Z7 -gdwarf /c -### -- %s 2>&1 | FileCheck 
-check-prefix=Z7_gdwarf %s

bogner wrote:
> rnk wrote:
> > I think Meta folks depend on a mode that emits both codeview and DWARF. 
> > +@smeenai
> Hopefully if that's the case there's a better test somewhere than this one 
> that discusses in detail how `/Z7` is an alias for `-gline-tables-only`, 
> which hasn't been true since 2017.
Thanks for the headers up :) We don't depend on that mode any more though, so 
no problems on our end.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157794

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

[PATCH] D156784: [AArch64][PAC] Declare FPAC subtarget feature

2023-08-14 Thread Anatoly Trosinenko via Phabricator via cfe-commits

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

Looks like adding FeatureFPAC may require changes in more places - need to 
investigate whether I have to add something like AEK_FPAC constant, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156784

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

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

2023-08-14 Thread Qiongsi Wu via Phabricator via cfe-commits

qiongsiwu1 added a comment.

In D155290#4582825 , @MaskRay wrote:

> Using `%p` should not magically change the behavior and I am unsure the 
> result is better for PGO. 
> If we decide to provide such a mode, it needs to be opt-in by adding a new 
> format specifier or API.
> Can you elaborate separate profile files are going to be useful?

Thanks again for taking a look at this patch!

Sure! We are generating separate profiles through `%p` for two reasons. First, 
we think that `%p` implies a profile per process, even if `exec` is not called 
after `fork`. It is unexpected that with `%p`, a program that forks child 
processes only create one profile file from the parent. This is why we did not 
create a new API or create a new pattern. Second, we are trying to catch 
non-gracefully terminated processes during profile generate. We observed a 
scenario where a hot function having all 0 counters. The reasons was the child 
process was created by a fork (but no `exec` followed), and then was terminated 
probably by a signal, instead of going through `exit()`. Such a process did not 
have a chance to write back the profile for merging. The parent process took a 
different call path and never called the hot function. We would like to detect 
such a situation for a particular process. Hence this patch creates an empty 
profile file if a process is not terminated gracefully. Using `%p` was probably 
the most natural choice when debugging such issues.

One can argue that we may look into the continuous mode to manage abnormal 
terminations during profile generate. I have not looked too closely at that, 
but we think it is still useful for a user to tell that some processes did not 
write any profile back, for debugging, or for diagnostics.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155290

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

[PATCH] D157362: [RISCV] Add MC layer support for Zicfilp.

2023-08-14 Thread Philip Reames via Phabricator via cfe-commits

reames added inline comments.



Comment at: llvm/docs/RISCVUsage.rst:120
  ``Zicboz``   Assembly Support
+ ``Zicfilp``  Assembly Support
  ``Zicntr``   (`See Note <#riscv-i2p1-note>`__)

This in the wrong place; this is not a ratified extension.  

You need to link to the right spec version as well.  This is particularly 
important here as this spec has gone through a ton of churn, and may still do 
so.  



Comment at: llvm/test/MC/RISCV/zicfilp-valid.s:17
+
+# CHECK-ASM-AND-OBJ: auipc zero, 22
+# CHECK-ASM: encoding: [0x17,0x60,0x01,0x00]

If the proper extension is provided to llvm-objdump, we really shouldn't be 
disassembling this as the auipc name, it should be the landing pad mnemonic.  

I think this is just the use of -m no-aliases above.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157362

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

[PATCH] D155081: Specify the developer policy around links to external resources

2023-08-14 Thread Tanya Lattner via Phabricator via cfe-commits

tonic added a comment.

@lattner Please review.


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

https://reviews.llvm.org/D155081

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

[PATCH] D86993: Document Clang's expectations of the C standard library.

2023-08-14 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D86993#4584619 , @nikic wrote:

> @aaron.ballman I wanted to check back whether the linked document is what you 
> had in mind, or whether some more/else would be needed.

Sorry about the delayed review; this fell off my radar! This is exactly along 
the lines of what I was thinking of -- I left some comments on the document. In 
terms of next steps, I think we can do a few things:

1. You and I can iterate on the document until we're ready to submit it to WG14
2. Simultaneously, I think we can move forward with this review -- we can add a 
statement along the lines of "C standard library implementations that do not 
guarantee these properties are incompatible with Clang and LLVM (and with 
several other major compilers); please see WG14 N, which proposes changes 
to the C standard to make this behavior conforming." (I'm not strongly tied to 
these words, just so long as there's some link back to the WG14 document 
justifying the deviation from the C standard.)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86993

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

[PATCH] D156205: wasm: link crt1 even in case of -shared

2023-08-14 Thread Sam Clegg via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG989ce069a463: [clang][WebAssembly] Link crt1 even in case of 
-shared (authored by yamt, committed by sbc100).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156205

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/test/Driver/wasm-toolchain.c


Index: clang/test/Driver/wasm-toolchain.c
===
--- clang/test/Driver/wasm-toolchain.c
+++ clang/test/Driver/wasm-toolchain.c
@@ -33,19 +33,19 @@
 // LINK_KNOWN: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
-// -shared should be passed through to `wasm-ld` and not include crt1.o with a 
known OS.
+// -shared should be passed through to `wasm-ld` and include crt1-reactor.o 
with a known OS.
 
-// RUN: %clang -### -shared --target=wasm32-wasi --sysroot=/foo %s 2>&1 \
+// RUN: %clang -### -shared -mexec-model=reactor --target=wasm32-wasi 
--sysroot=/foo %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=LINK_KNOWN_SHARED %s
 // LINK_KNOWN_SHARED: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_KNOWN_SHARED: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "-shared" 
"[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+// LINK_KNOWN_SHARED: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1-reactor.o" 
"--entry" "_initialize" "-shared" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
-// -shared should be passed through to `wasm-ld` and not include crt1.o with 
an unknown OS.
+// -shared should be passed through to `wasm-ld` and include crt1-reactor.o 
with an unknown OS.
 
-// RUN: %clang -### -shared --target=wasm32-unknown-unknown --sysroot=/foo %s 
2>&1 \
+// RUN: %clang -### -shared -mexec-model=reactor 
--target=wasm32-unknown-unknown --sysroot=/foo %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=LINK_UNKNOWN_SHARED %s
 // LINK_UNKNOWN_SHARED: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_UNKNOWN_SHARED: wasm-ld{{.*}}" "-shared" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+// LINK_UNKNOWN_SHARED: wasm-ld{{.*}}" "crt1-reactor.o" "--entry" 
"_initialize" "-shared" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C link command-line with optimization with known OS.
 
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -77,31 +77,43 @@
   Args.AddAllArgs(CmdArgs, options::OPT_u);
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
 
-  const char *Crt1 = "crt1.o";
+  bool IsCommand = true;
+  const char *Crt1;
   const char *Entry = nullptr;
 
-  // If crt1-command.o exists, it supports new-style commands, so use it.
-  // Otherwise, use the old crt1.o. This is a temporary transition measure.
-  // Once WASI libc no longer needs to support LLVM versions which lack
-  // support for new-style command, it can make crt1.o the same as
-  // crt1-command.o. And once LLVM no longer needs to support WASI libc
-  // versions before that, it can switch to using crt1-command.o.
-  if (ToolChain.GetFilePath("crt1-command.o") != "crt1-command.o")
-Crt1 = "crt1-command.o";
+  // When -shared is specified, use the reactor exec model unless
+  // specified otherwise.
+  if (Args.hasArg(options::OPT_shared))
+IsCommand = false;
 
   if (const Arg *A = Args.getLastArg(options::OPT_mexec_model_EQ)) {
 StringRef CM = A->getValue();
 if (CM == "command") {
-  // Use default values.
+  IsCommand = true;
 } else if (CM == "reactor") {
-  Crt1 = "crt1-reactor.o";
-  Entry = "_initialize";
+  IsCommand = false;
 } else {
   ToolChain.getDriver().Diag(diag::err_drv_invalid_argument_to_option)
   << CM << A->getOption().getName();
 }
   }
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles, 
options::OPT_shared))
+
+  if (IsCommand) {
+// If crt1-command.o exists, it supports new-style commands, so use it.
+// Otherwise, use the old crt1.o. This is a temporary transition measure.
+// Once WASI libc no longer needs to support LLVM versions which lack
+// support for new-style command, it can make crt1.o the same as
+// crt1-command.o. And once LLVM no longer needs to support WASI libc
+// versions before that, it can switch to using crt1-command.o.
+Crt1 = "crt1.o";
+if (ToolChain.GetFilePath("crt1-command.o") != "crt1-command.o")
+  Crt1 = "crt1-command.o";
+  } else {
+Crt1 = "crt1-reactor.o";
+Entry = "_initialize";
+  }
+
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles))

[clang] 989ce06 - [clang][WebAssembly] Link crt1 even in case of -shared

2023-08-14 Thread Sam Clegg via cfe-commits


Author: YAMAMOTO Takashi
Date: 2023-08-14T09:36:44-07:00
New Revision: 989ce069a4638fd561bebc95f478ca9e45154fec

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

LOG: [clang][WebAssembly] Link crt1 even in case of -shared

This allows -mexec-model=reactor -shared produces a library module
with _initialize entrypoint, which is preferrable over __wasm_call_ctors.

This partially reverts https://reviews.llvm.org/D153293

Discussion: https://github.com/dicej/component-linking-demo/issues/3

Reviewed By: sbc100

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/WebAssembly.cpp
clang/test/Driver/wasm-toolchain.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/WebAssembly.cpp 
b/clang/lib/Driver/ToolChains/WebAssembly.cpp
index 36bed3166ff3c6..2098a68cbc6e56 100644
--- a/clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ b/clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -77,31 +77,43 @@ void wasm::Linker::ConstructJob(Compilation , const 
JobAction ,
   Args.AddAllArgs(CmdArgs, options::OPT_u);
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
 
-  const char *Crt1 = "crt1.o";
+  bool IsCommand = true;
+  const char *Crt1;
   const char *Entry = nullptr;
 
-  // If crt1-command.o exists, it supports new-style commands, so use it.
-  // Otherwise, use the old crt1.o. This is a temporary transition measure.
-  // Once WASI libc no longer needs to support LLVM versions which lack
-  // support for new-style command, it can make crt1.o the same as
-  // crt1-command.o. And once LLVM no longer needs to support WASI libc
-  // versions before that, it can switch to using crt1-command.o.
-  if (ToolChain.GetFilePath("crt1-command.o") != "crt1-command.o")
-Crt1 = "crt1-command.o";
+  // When -shared is specified, use the reactor exec model unless
+  // specified otherwise.
+  if (Args.hasArg(options::OPT_shared))
+IsCommand = false;
 
   if (const Arg *A = Args.getLastArg(options::OPT_mexec_model_EQ)) {
 StringRef CM = A->getValue();
 if (CM == "command") {
-  // Use default values.
+  IsCommand = true;
 } else if (CM == "reactor") {
-  Crt1 = "crt1-reactor.o";
-  Entry = "_initialize";
+  IsCommand = false;
 } else {
   ToolChain.getDriver().Diag(diag::err_drv_invalid_argument_to_option)
   << CM << A->getOption().getName();
 }
   }
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles, 
options::OPT_shared))
+
+  if (IsCommand) {
+// If crt1-command.o exists, it supports new-style commands, so use it.
+// Otherwise, use the old crt1.o. This is a temporary transition measure.
+// Once WASI libc no longer needs to support LLVM versions which lack
+// support for new-style command, it can make crt1.o the same as
+// crt1-command.o. And once LLVM no longer needs to support WASI libc
+// versions before that, it can switch to using crt1-command.o.
+Crt1 = "crt1.o";
+if (ToolChain.GetFilePath("crt1-command.o") != "crt1-command.o")
+  Crt1 = "crt1-command.o";
+  } else {
+Crt1 = "crt1-reactor.o";
+Entry = "_initialize";
+  }
+
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles))
 CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(Crt1)));
   if (Entry) {
 CmdArgs.push_back(Args.MakeArgString("--entry"));

diff  --git a/clang/test/Driver/wasm-toolchain.c 
b/clang/test/Driver/wasm-toolchain.c
index 8e4e749df01440..f950283ec42aa0 100644
--- a/clang/test/Driver/wasm-toolchain.c
+++ b/clang/test/Driver/wasm-toolchain.c
@@ -33,19 +33,19 @@
 // LINK_KNOWN: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" 
"-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
-// -shared should be passed through to `wasm-ld` and not include crt1.o with a 
known OS.
+// -shared should be passed through to `wasm-ld` and include crt1-reactor.o 
with a known OS.
 
-// RUN: %clang -### -shared --target=wasm32-wasi --sysroot=/foo %s 2>&1 \
+// RUN: %clang -### -shared -mexec-model=reactor --target=wasm32-wasi 
--sysroot=/foo %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=LINK_KNOWN_SHARED %s
 // LINK_KNOWN_SHARED: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_KNOWN_SHARED: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "-shared" 
"[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+// LINK_KNOWN_SHARED: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1-reactor.o" 
"--entry" "_initialize" "-shared" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
-// -shared should be passed through to `wasm-ld` and not include crt1.o with 
an unknown OS.
+// -shared should be passed through to `wasm-ld` and include

[PATCH] D157080: [clangd] Symbol search includes parameter types for (member) functions

2023-08-14 Thread Nathan Ridge via Phabricator via cfe-commits

nridge added a comment.

I understood https://github.com/clangd/clangd/issues/1344 as being about 
`workspace/symbol` rather than `textDocument/documentSymbol`, though I see now 
that both are affected.

The analysis is a bit different for the two, though: unlike `DocumentSymbol`, 
the result types for `workspace/symbol` (LSP specifies two options, 
`SymbolInformation` or `WorkspaceSymbol`) do not have a `detail` field. So, for 
`workspace/symbol`, the issue would require a fix on the clangd side (or a 
change to LSP, I guess).

@sammccall, would you support adding the signature to the symbol name for 
`workspace/symbol`?


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

https://reviews.llvm.org/D157080

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

[clang] 0c3a02b - Function multi-versioning: disable ifunc for ELF targets other than glibc/Android/FreeBSD

2023-08-14 Thread Fangrui Song via cfe-commits


Author: Fangrui Song
Date: 2023-08-14T08:59:59-07:00
New Revision: 0c3a02b8c09bb408a74a638a263e51d67c92ca74

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

LOG: Function multi-versioning: disable ifunc for ELF targets other than 
glibc/Android/FreeBSD

Generalize D127933 (Fuchsia special case) to other ELF targets. Ensure
that musl, NetBSD, OpenBSD, etc do not get ifunc codegen which is
unsupported in their rtld.

Link: 
https://discourse.llvm.org/t/does-ifunc-use-from-llvm-require-os-support/67628
Close: https://github.com/llvm/llvm-project/issues/64631

Added: 


Modified: 
clang/include/clang/Basic/TargetInfo.h
clang/test/CodeGen/attr-target-mv-va-args.c
clang/test/CodeGen/unique-internal-linkage-names.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/TargetInfo.h 
b/clang/include/clang/Basic/TargetInfo.h
index 41ef47eb565b1c..61be52149341f0 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -1414,7 +1414,9 @@ class TargetInfo : public TransferrableTargetInfo,
 
   /// Identify whether this target supports IFuncs.
   bool supportsIFunc() const {
-return getTriple().isOSBinFormatELF() && !getTriple().isOSFuchsia();
+return getTriple().isOSBinFormatELF() &&
+   ((getTriple().isOSLinux() && !getTriple().isMusl()) ||
+getTriple().isOSFreeBSD());
   }
 
   // Validate the contents of the __builtin_cpu_supports(const char*)

diff  --git a/clang/test/CodeGen/attr-target-mv-va-args.c 
b/clang/test/CodeGen/attr-target-mv-va-args.c
index e75796d7ee0383..96821c610235bd 100644
--- a/clang/test/CodeGen/attr-target-mv-va-args.c
+++ b/clang/test/CodeGen/attr-target-mv-va-args.c
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck %s 
--check-prefix=LINUX
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck %s 
--check-prefix=IFUNC-ELF
+// RUN: %clang_cc1 -triple x86_64-pc-freebsd -emit-llvm %s -o - | FileCheck %s 
--check-prefix=IFUNC-ELF
 // RUN: %clang_cc1 -triple x86_64-windows-pc -emit-llvm %s -o - | FileCheck %s 
--check-prefixes=NO-IFUNC,WINDOWS
-// RUN: %clang_cc1 -triple x86_64-fuchsia -emit-llvm %s -o - | FileCheck %s 
--check-prefixes=NO-IFUNC,FUCHSIA
+// RUN: %clang_cc1 -triple x86_64-linux-musl -emit-llvm %s -o - | FileCheck %s 
--check-prefixes=NO-IFUNC,NO-IFUNC-ELF
+// RUN: %clang_cc1 -triple x86_64-fuchsia -emit-llvm %s -o - | FileCheck %s 
--check-prefixes=NO-IFUNC,NO-IFUNC-ELF
 int __attribute__((target("sse4.2"))) foo(int i, ...) { return 0; }
 int __attribute__((target("arch=sandybridge"))) foo(int i, ...);
 int __attribute__((target("arch=ivybridge"))) foo(int i, ...) {return 1;}
@@ -10,23 +12,23 @@ int bar(void) {
   return foo(1, 'a', 1.1) + foo(2, 2.2, "asdf");
 }
 
-// LINUX: @foo.ifunc = weak_odr ifunc i32 (i32, ...), ptr @foo.resolver
-// LINUX: define{{.*}} i32 @foo.sse4.2(i32 noundef %i, ...)
-// LINUX: ret i32 0
-// LINUX: define{{.*}} i32 @foo.arch_ivybridge(i32 noundef %i, ...)
-// LINUX: ret i32 1
-// LINUX: define{{.*}} i32 @foo(i32 noundef %i, ...)
-// LINUX: ret i32 2
-// LINUX: define{{.*}} i32 @bar()
-// LINUX: call i32 (i32, ...) @foo.ifunc(i32 noundef 1, i32 noundef 97, double
-// LINUX: call i32 (i32, ...) @foo.ifunc(i32 noundef 2, double noundef 
2.2{{[0-9Ee+]+}}, ptr noundef
+// IFUNC-ELF: @foo.ifunc = weak_odr ifunc i32 (i32, ...), ptr @foo.resolver
+// IFUNC-ELF: define{{.*}} i32 @foo.sse4.2(i32 noundef %i, ...)
+// IFUNC-ELF: ret i32 0
+// IFUNC-ELF: define{{.*}} i32 @foo.arch_ivybridge(i32 noundef %i, ...)
+// IFUNC-ELF: ret i32 1
+// IFUNC-ELF: define{{.*}} i32 @foo(i32 noundef %i, ...)
+// IFUNC-ELF: ret i32 2
+// IFUNC-ELF: define{{.*}} i32 @bar()
+// IFUNC-ELF: call i32 (i32, ...) @foo.ifunc(i32 noundef 1, i32 noundef 97, 
double
+// IFUNC-ELF: call i32 (i32, ...) @foo.ifunc(i32 noundef 2, double noundef 
2.2{{[0-9Ee+]+}}, ptr noundef
 
-// LINUX: define weak_odr ptr @foo.resolver() comdat
-// LINUX: ret ptr @foo.arch_sandybridge
-// LINUX: ret ptr @foo.arch_ivybridge
-// LINUX: ret ptr @foo.sse4.2
-// LINUX: ret ptr @foo
-// LINUX: declare i32 @foo.arch_sandybridge(i32 noundef, ...)
+// IFUNC-ELF: define weak_odr ptr @foo.resolver() comdat
+// IFUNC-ELF: ret ptr @foo.arch_sandybridge
+// IFUNC-ELF: ret ptr @foo.arch_ivybridge
+// IFUNC-ELF: ret ptr @foo.sse4.2
+// IFUNC-ELF: ret ptr @foo
+// IFUNC-ELF: declare i32 @foo.arch_sandybridge(i32 noundef, ...)
 
 // NO-IFUNC: define dso_local i32 @foo.sse4.2(i32 noundef %i, ...)
 // NO-IFUNC: ret i32 0
@@ -39,10 +41,10 @@ int bar(void) {
 // NO-IFUNC: call i32 (i32, ...) @foo.resolver(i32 noundef 2, double noundef 
2.2{{[0-9Ee+]+}}, ptr noundef
 
 // WINDOWS: define weak_odr dso_local i32 @foo.resolver(i32 %0, ...) comdat
-// FUCHSIA:

[PATCH] D155081: Specify the developer policy around links to external resources

2023-08-14 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman updated this revision to Diff 549979.
aaron.ballman added a comment.

Tweaked wording to link to the policy about adding links to github issues in 
commit messages instead.


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

https://reviews.llvm.org/D155081

Files:
  llvm/docs/DeveloperPolicy.rst


Index: llvm/docs/DeveloperPolicy.rst
===
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -201,6 +201,11 @@
   entire failing program into ``llvm/test`` as this creates a *time-to-test*
   burden on all developers. Please keep them short.
 
+* Avoid adding links to resources that are not available to the entire
+  community, such as links to private bug trackers, internal corporate
+  documentation, etc. Instead, add sufficient comments to the test to provide
+  the context behind such links.
+
 Note that llvm/test and clang/test are designed for regression and small 
feature
 tests only. More extensive test cases (e.g., entire applications, benchmarks,
 etc) should be added to the ``llvm-test`` test suite.  The llvm-test suite is
@@ -256,6 +261,11 @@
the change (more invasive changes require more testing). A reasonable subset
might be something like "``llvm-test/MultiSource/Benchmarks``".
 
+#. Ensure that links in source code and test files point to publicly available
+   resources and are used primarily to add additional information rather than
+   to supply critical context. The surrounding comments should be sufficient
+   to provide the context behind such links.
+
 Additionally, the committer is responsible for addressing any problems found in
 the future that the change is responsible for.  For example:
 
@@ -336,8 +346,6 @@
   code snippets and gory details should be left to bug comments, web
   review or the mailing list.
 
-* If the patch fixes a bug in GitHub Issues, please include the PR# in the 
message.
-
 * Text formatting and spelling should follow the same rules as documentation
   and in-code comments, ex. capitalization, full stop, etc.
 
@@ -346,8 +354,13 @@
   related commit. This could be as simple as "Revert commit  because it
   caused PR#".
 
-* If the patch has been reviewed, add a link to its review page, as shown
-  `here `_.
+* It is acceptable to add metadata to the commit message to automate processes,
+  including for downstream consumers. If the patch fixes a bug in GitHub 
Issues,
+  we encourage adding a reference to the issue being closed, as described
+  `here `_.
+  Other kinds of metadata are also acceptable, including links to resources
+  that are not available to the entire community. However, such links should
+  not be used in place of making the commit message self-explanatory.
 
 For minor violations of these recommendations, the community normally favors
 reminding the contributor of this policy over reverting. Minor corrections and


Index: llvm/docs/DeveloperPolicy.rst
===
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -201,6 +201,11 @@
   entire failing program into ``llvm/test`` as this creates a *time-to-test*
   burden on all developers. Please keep them short.
 
+* Avoid adding links to resources that are not available to the entire
+  community, such as links to private bug trackers, internal corporate
+  documentation, etc. Instead, add sufficient comments to the test to provide
+  the context behind such links.
+
 Note that llvm/test and clang/test are designed for regression and small feature
 tests only. More extensive test cases (e.g., entire applications, benchmarks,
 etc) should be added to the ``llvm-test`` test suite.  The llvm-test suite is
@@ -256,6 +261,11 @@
the change (more invasive changes require more testing). A reasonable subset
might be something like "``llvm-test/MultiSource/Benchmarks``".
 
+#. Ensure that links in source code and test files point to publicly available
+   resources and are used primarily to add additional information rather than
+   to supply critical context. The surrounding comments should be sufficient
+   to provide the context behind such links.
+
 Additionally, the committer is responsible for addressing any problems found in
 the future that the change is responsible for.  For example:
 
@@ -336,8 +346,6 @@
   code snippets and gory details should be left to bug comments, web
   review or the mailing list.
 
-* If the patch fixes a bug in GitHub Issues, please include the PR# in the message.
-
 * Text formatting and spelling should follow the same rules as documentation
   and in-code comments, ex. capitalization, full stop, etc.
 
@@ -346,8 +354,13 @@
   related commit. This could be as simple as "Revert commit  because it
   caused PR#".
 
-* If the patch has

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

2023-08-14 Thread Donát Nagy via Phabricator via cfe-commits

donat.nagy updated this revision to Diff 549978.
donat.nagy marked 7 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156312

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/analyzer-enabled-checkers.c
  clang/test/Analysis/bitwise-ops-nocrash.c
  clang/test/Analysis/bitwise-ops.c
  clang/test/Analysis/bitwise-shift-common.c
  clang/test/Analysis/bitwise-shift-pedantic.c
  clang/test/Analysis/bitwise-shift-sanity-checks.c
  clang/test/Analysis/bitwise-shift-state-update.c
  clang/test/Analysis/casts.c
  clang/test/Analysis/diagnostics/track_subexpressions.cpp
  clang/test/Analysis/left-shift-cxx2a.cpp
  clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
  clang/test/Analysis/symbol-simplification-nonloc-loc.cpp

Index: clang/test/Analysis/symbol-simplification-nonloc-loc.cpp
===
--- clang/test/Analysis/symbol-simplification-nonloc-loc.cpp
+++ clang/test/Analysis/symbol-simplification-nonloc-loc.cpp
@@ -14,7 +14,7 @@
   if (p) {
 // no-crash
   }
-  if (p == (int *)0x404) {
+  if (p == (int *)0x1b) {
 // no-crash
   }
 }
@@ -29,7 +29,7 @@
   if (p) {
 // no-crash
   }
-  if (p == (int *)0x404) {
+  if (p == (int *)0x1b) {
 // no-crash
   }
 }
@@ -43,8 +43,6 @@
   nonloc_OP_loc(p, BINOP(-)); // no-crash
 
   // Bitwise operators:
-  // expected-warning@+2 {{The result of the left shift is undefined due to shifting by '1028', which is greater or equal to the width of type 'int' [core.UndefinedBinaryOperatorResult]}}
-  // expected-warning@+2 {{The result of the right shift is undefined due to shifting by '1028', which is greater or equal to the width of type 'int' [core.UndefinedBinaryOperatorResult]}}
   nonloc_OP_loc(p, BINOP(<<)); // no-crash
   nonloc_OP_loc(p, BINOP(>>)); // no-crash
   nonloc_OP_loc(p, BINOP(&));
Index: clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
===
--- clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
+++ clang/test/Analysis/std-c-library-functions-arg-enabled-checkers.c
@@ -24,6 +24,7 @@
 // CHECK-NEXT: apiModeling.TrustReturnsNonnull
 // CHECK-NEXT: apiModeling.llvm.CastValue
 // CHECK-NEXT: apiModeling.llvm.ReturnValue
+// CHECK-NEXT: core.BitwiseShift
 // CHECK-NEXT: core.DivideZero
 // CHECK-NEXT: core.DynamicTypePropagation
 // CHECK-NEXT: core.NonnilStringConstants
Index: clang/test/Analysis/left-shift-cxx2a.cpp
===
--- clang/test/Analysis/left-shift-cxx2a.cpp
+++ clang/test/Analysis/left-shift-cxx2a.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple x86_64-apple-darwin13 -Wno-shift-count-overflow -verify=expected,cxx17 -std=c++17 %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -triple x86_64-apple-darwin13 -Wno-shift-count-overflow -verify=expected,cxx2a -std=c++2a %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-disable-checker core.BitwiseShift -triple x86_64-apple-darwin13 -Wno-shift-count-overflow -verify=expected,cxx17 -std=c++17 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-disable-checker core.BitwiseShift -triple x86_64-apple-darwin13 -Wno-shift-count-overflow -verify=expected,cxx2a -std=c++2a %s
 
 int testNegativeShift(int a) {
   if (a == -5) {
Index: clang/test/Analysis/diagnostics/track_subexpressions.cpp
===
--- clang/test/Analysis/diagnostics/track_subexpressions.cpp
+++ clang/test/Analysis/diagnostics/track_subexpressions.cpp
@@ -12,10 +12,10 @@
 
 void shift_by_undefined_value() {
   uint8_t shift_amount = get_uint8_max(); // expected-note{{'shift_amount' initialized to 255}}
-// expected-note@-1{{Calling 'get_uint8_max'}}
-// expected-note@-2{{Returning from 'get_uint8_max'}}
-  (void)(TCP_MAXWIN << shift_amount); // expected-warning{{The result of the left shift is undefined due to shifting by '255', which is greater or equal to the width of type 'int'}}
-  // expected-note@-1{{The result of the left shift is undefined due to shifting by '255', which is greater or equal to the width of type 'int'}}
+  // expected-note@-1{{Calling 'get_uint8_max'}}
+  // expected-note@-2{{Returning from 'get_uint8_max'}}
+  (void)(TCP_MAXWIN << shift_amount); // expected-warning{{Left shift by '255' overflows the capacity of 'int'}}
+  // expected-note@-1{{The result of left

[PATCH] D156312: [analyzer] Upstream BitwiseShiftChecker

2023-08-14 Thread Donát Nagy via Phabricator via cfe-commits

donat.nagy marked 13 inline comments as done.
donat.nagy added a comment.

I handled the inline comments, I'll check the example code and edit the release 
notes tomorrow.




Comment at: clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp:272
+  if (const auto ConcreteRight = Right.getAs()) {
+const std::int64_t RHS = ConcreteRight->getValue().getExtValue();
+assert(RHS > MaximalAllowedShift);

steakhal wrote:
> `getExtValue()`, I believe, asserts that it "fits", thus the concrete int is 
> at most 64 bits wide. Does it work for `_BigInt`s? (or whatever we have like 
> that :D)
This final check is executed when we already know that 0 <= ConcreteRight < 128 
//(or the bit width of the largest integer type)//. I added a comment that 
mentions this and replaced the type `std::int64_t` with a plain `unsigned`.

I also updated the testcase `large_right` to ensure that later changes don't 
introduce crashes related to absurdly oversized right arguments.



Comment at: clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp:335-339
+auto R =
+std::make_unique(BT, ShortMsg, Msg, ErrNode);
+bugreporter::trackExpressionValue(ErrNode, Op->getLHS(), *R);
+bugreporter::trackExpressionValue(ErrNode, Op->getRHS(), *R);
+return R;

steakhal wrote:
> AHA! Now I got you. You also call this `R`.
> Just use `R` at the callsite as well. Job done.
This is a code fragment that survived my refactoring efforts without 
significant changes... until now :)

Now that you highlighted it, I'm renaming `R` to `BR` just to be a contrarian :)



Comment at: clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp:355-357
+if (!BTPtr)
+  BTPtr = std::make_unique(this, "Bitwise shift",
+"Suspicious operation");

steakhal wrote:
> How about eagerly inline initializing this field? And avoiding `mutable` as 
> well.
This pattern systematically appears in all checkers that I've seen because the 
constructor of `BugType` queries and stores the name of the checker, which is 
only initialized when the checker (`*this`) is registered. This dependency 
means that the `BugType` cannot be initialized in the constructor of the 
`Checker`.



Comment at: clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp:53
+class BitwiseShiftValidator {
+  // Primary mutable state:
+  CheckerContext 

steakhal wrote:
> donat.nagy wrote:
> > steakhal wrote:
> > > Where does this comment corresponds to?
> > The two data members (`Ctx` and `State`) that are directly below the 
> > comment.
> > 
> > I tried to split the list of data members into three groups (primary 
> > mutable state, secondary mutable state, `const`  members), but now I 
> > reconsider this I feel that these header comments are actually superfluous.
> > 
> > I'll return to a variant of the earlier layout where I put 
> > `NonNegFlags`/`UpperBound` to the end and comment that they are only used 
> > for note tag creation; but don't emphasize the (immediately visible) 
> > `const`ness / non-`const`-ness of other members with comments.
> One way to emphasize this is by choosing names like `FoldedState`, or 
> anything else that has something to do with "motion" or "change".
> Given that if we have `ProgramStateRefs` in helper classes they usually 
> remain still. Think of BugReportVisitors for example. Consequently,it's 
> important to raise awareness when it's not like that. A different name would 
> achieve this.
My intuition is that the variable name "State" already implies that it will be 
regularly changed and updated as we are going forward; but you're right that in 
this project we've a tradition of storing snapshots of "old" states in various 
helper classes.

(By the way, I'm a bit worried when I read code that stores and passes around 
random stale snapshots of the state. Of course we cannot avoid the branching 
nature of the exploded graph, but other than that it'd be good to store the 
State in language constructs where you cannot accidentally lose information by 
building onto an older state variable.) 



Comment at: clang/lib/StaticAnalyzer/Checkers/BitwiseShiftChecker.cpp:60-63
+  // If there is an upper bound, it is <= the size of a primitive type
+  // (expressed in bits), so this is a very safe sentinel value:
+  enum { NoUpperBound = 999 };
+  unsigned UpperBound = NoUpperBound;

steakhal wrote:
> donat.nagy wrote:
> > steakhal wrote:
> > > Have you considered using `std::optional`?
> > > Given that the dimension of this variable is "bits", I think it would be 
> > > great to have that in its name.
> > I considered using `std::optional`, but using this "old-school" solution 
> > ensures that `NoUpperBound` is comparable to and larger than the "real" 
> > upper bound values, which lets me avoid some ugly boolean logic. I'll 
> >

[PATCH] D157691: [ASTImporter] Remove extranous FunctionTemplateDecl introduced by templated friend

2023-08-14 Thread Balázs Kéri via Phabricator via cfe-commits

balazske added a comment.

It is better to add a test to ASTImporterTests.cpp with the same code, and 
check the properties of DeclContext and LexicalDeclContext in the From and To 
AST (should be the same in "From" and "To" side).




Comment at: clang/lib/AST/ASTImporter.cpp:6451
+  if (D->getFriendObjectKind() == Decl::FOK_None)
+LexicalDC->addDeclInternal(ToFunc);
 

`addDeclToContexts(D, ToFunction)` should be better


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157691

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

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

2023-08-14 Thread Tom Honermann via Phabricator via cfe-commits

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

Looks good to me!


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

https://reviews.llvm.org/D157888

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

[PATCH] D157554: [NFC][Clang] Fix static analyzer concern about null pointer dereference

2023-08-14 Thread Tom Honermann via Phabricator via cfe-commits

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

Looks good to me!


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

https://reviews.llvm.org/D157554

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

[PATCH] D157885: [NFC][Clang] Fix static analyzer concern about null value derefence

2023-08-14 Thread Tom Honermann via Phabricator via cfe-commits

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

Looks fine to me!


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

https://reviews.llvm.org/D157885

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

[PATCH] D157280: [PGO] Enable `-fprofile-update` for `-fprofile-generate`

2023-08-14 Thread Qiongsi Wu via Phabricator via cfe-commits

qiongsiwu1 added a comment.

Ping for review. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157280

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

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

2023-08-14 Thread Elizabeth Andrews via Phabricator via cfe-commits

eandrews created this revision.
eandrews added reviewers: aaron.ballman, tahonermann.
Herald added subscribers: steakhal, abrachet, manas, ASDenysPetrov, martong, 
dkrupp, donat.nagy, Szelethus, a.sidorin, baloghadamsoftware.
Herald added a reviewer: NoQ.
Herald added a project: All.
eandrews requested review of this revision.

https://reviews.llvm.org/D157888

Files:
  clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -653,10 +653,11 @@
   if (Type.isSuppressOnSink()) {
 const ExplodedNode *AcquireNode = getAcquireSite(ErrorNode, Sym, C);
 if (AcquireNode) {
+  const Stmt *S = AcquireNode->getStmtForDiagnostics();
+  assert(S && "Statmement cannot be null.");
   PathDiagnosticLocation LocUsedForUniqueing =
   PathDiagnosticLocation::createBegin(
-  AcquireNode->getStmtForDiagnostics(), C.getSourceManager(),
-  AcquireNode->getLocationContext());
+  S, C.getSourceManager(), AcquireNode->getLocationContext());
 
   R = std::make_unique(
   Type, Msg, ErrorNode, LocUsedForUniqueing,


Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -653,10 +653,11 @@
   if (Type.isSuppressOnSink()) {
 const ExplodedNode *AcquireNode = getAcquireSite(ErrorNode, Sym, C);
 if (AcquireNode) {
+  const Stmt *S = AcquireNode->getStmtForDiagnostics();
+  assert(S && "Statmement cannot be null.");
   PathDiagnosticLocation LocUsedForUniqueing =
   PathDiagnosticLocation::createBegin(
-  AcquireNode->getStmtForDiagnostics(), C.getSourceManager(),
-  AcquireNode->getLocationContext());
+  S, C.getSourceManager(), AcquireNode->getLocationContext());
 
   R = std::make_unique(
   Type, Msg, ErrorNode, LocUsedForUniqueing,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157855: [clang][ExprConstant] Improve error message of compound assignment against uninitialized object

2023-08-14 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder added a comment.

Generally LGTM, but I feel like the changes aren't c++2a specific (except for 
uninitialized variables in constexpr functions).




Comment at: clang/lib/AST/ExprConstant.cpp:4447
+  Info.FFDiag(E, diag::note_constexpr_access_uninit)
+  << /*read of*/ 0 << /*uninitialized object*/ 1;
+  return false;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157855

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

[PATCH] D157793: [Headers] Add missing __need_ macros to stdarg.h

2023-08-14 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D157793#4583698 , @iana wrote:

> In D157793#4583663 , @MaskRay wrote:
>
>>> Apple needs a __need_ macro for va_list. Add one, and also ones for 
>>> va_start/va_arg/va_end, __va_copy, va_copy.
>>
>> Do you have a link to the specification or the source that this is needed?
>
> We need is so that our  doesn't have to redeclare 
> va_list and doesn't pull in all of stdarg.h either. As far as I know there's 
> no specification for this (or stddef.h) being include-able in pieces.

I'm not opposed, but this adds significant complexity to support a questionable 
use case -- the C standard library headers are not intended to be partially 
included, so if the plan is to use these `__need` macros in all of the headers 
we provide, I think that should go through an RFC process to be sure we want to 
maintain the added complexity. (Also, who is "we" in this case?)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157793

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

[PATCH] D157554: [NFC][Clang] Fix static analyzer concern about null pointer dereference

2023-08-14 Thread Elizabeth Andrews via Phabricator via cfe-commits

eandrews updated this revision to Diff 549960.
eandrews added a comment.

Applied review comments


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

https://reviews.llvm.org/D157554

Files:
  clang/lib/Sema/SemaExprCXX.cpp


Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -9072,8 +9072,10 @@
 MultiLevelTemplateArgumentList MLTAL(Param, TAL.asArray(),
  /*Final=*/false);
 MLTAL.addOuterRetainedLevels(TPL->getDepth());
-Expr *IDC = Param->getTypeConstraint()->getImmediatelyDeclaredConstraint();
-ExprResult Constraint = SubstExpr(IDC, MLTAL);
+const TypeConstraint *TC = Param->getTypeConstraint();
+assert(TC && "Type Constraint cannot be null here");
+ExprResult Constraint =
+SubstExpr(TC->getImmediatelyDeclaredConstraint(), MLTAL);
 if (Constraint.isInvalid()) {
   Status = concepts::ExprRequirement::SS_ExprSubstitutionFailure;
 } else {


Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -9072,8 +9072,10 @@
 MultiLevelTemplateArgumentList MLTAL(Param, TAL.asArray(),
  /*Final=*/false);
 MLTAL.addOuterRetainedLevels(TPL->getDepth());
-Expr *IDC = Param->getTypeConstraint()->getImmediatelyDeclaredConstraint();
-ExprResult Constraint = SubstExpr(IDC, MLTAL);
+const TypeConstraint *TC = Param->getTypeConstraint();
+assert(TC && "Type Constraint cannot be null here");
+ExprResult Constraint =
+SubstExpr(TC->getImmediatelyDeclaredConstraint(), MLTAL);
 if (Constraint.isInvalid()) {
   Status = concepts::ExprRequirement::SS_ExprSubstitutionFailure;
 } else {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157632: [Profile] Allow online merging with debug info correlation.

2023-08-14 Thread Zequan Wu via Phabricator via cfe-commits

zequanwu added a comment.

In D157632#4581219 , @ellis wrote:

> In D157632#4580576 , @zequanwu 
> wrote:
>
>> BTW, I noticed something strange with `-pgo-function-entry-coverage` when 
>> merging via llvm-profdata.
>
> This is intentional. The two raw profiles individually store blocks as either 
> covered or not, but when we merge them they get converted to counters and 
> accumulated.
> https://github.com/llvm/llvm-project/blob/6a0feb1503e21432e63d93b44357bad43f8026d1/llvm/lib/ProfileData/InstrProf.cpp#L726
> Then in `PGOUseFunc::populateCoverage()` we annotate blocks as alive if their 
> counter value is non-zero.
> https://github.com/llvm/llvm-project/blob/1aee2434ce4c9b5785cbc8f72cbbbd64f9e85297/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp#L1399
> My logic was that the indexed profile represents these counters as ints, so 
> we might as well accumulate them. Also, this makes the implementation simpler.

Thanks for explanation. That makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157632

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

[PATCH] D157869: [clang-tidy] Avoid overflow when dumping unsigned integer values

2023-08-14 Thread Daniel Alcaide Nombela via Phabricator via cfe-commits

ealcdan added a comment.

Thanks for the review. How should I procceed? Should I remove the "Fixes:" 
entry from the commit message?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157869

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

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

2023-08-14 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a subscriber: ldionne.
aaron.ballman added inline comments.



Comment at: clang/lib/Headers/stddef.h:16
+defined(__need_wint_t) ||  
\
+(defined(__STDC_WANT_LIB_EXT1__) && __STDC_WANT_LIB_EXT1__ >= 1)
 

C23 K.3.1.1p2: The functions, macros, and types declared or defined in K.3 and 
its subclauses are declared and defined by their respective headers if 
__STDC_WANT_LIB_EXT1__ is defined as a macro which expands to the integer 
constant 1 at the point in the source file where the appropriate header is 
first included.

So I don't think this should be `>= 1`. Same below. (Though I notice we seem to 
do this in other places, so perhaps that's more of a general cleanup we should 
make?)



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

Related:

https://github.com/llvm/llvm-project/issues/37564
https://cplusplus.github.io/LWG/issue3484

CC @ldionne



Comment at: clang/test/Headers/stddef.c:1-3
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=c99 
-std=c99 %s
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=c11 
-std=c11 %s
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=c23 
-std=c23 %s

Are the triples necessary? Better if we can remove them.



Comment at: clang/test/Headers/stddef.c:7
+
+ptrdiff_t p0; // c99-error{{unknown}} c11-error{{unknown}} c23-error{{unknown}}
+size_t s0; // c99-error{{unknown}} c11-error{{unknown}} c23-error{{unknown}}

At least one of these `unknown` errors should be fully spelled out (the rest 
can then be assumed to be the same wording as the long-form).



Comment at: clang/test/Headers/stddefneeds.c:1-2
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=c99 
-std=c99 %s
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=c23 
-std=c23 %s
+

Do we need the triples?



Comment at: clang/test/Headers/stddefneeds.c:8
+
+ptrdiff_t p0; // c99-error{{unknown}} c23-error{{unknown}}
+size_t s0; // c99-error{{unknown}} c23-error{{unknown}}

Should spell out one of these.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157757

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

[clang] d7fcb5b - clang: Don't use grep in a test

2023-08-14 Thread Matt Arsenault via cfe-commits


Author: Matt Arsenault
Date: 2023-08-14T11:28:41-04:00
New Revision: d7fcb5b6b5280dc61d5afe0920bb78a82cfe2f27

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

LOG: clang: Don't use grep in a test

Issue #10894 seems to claim this wasn't working. The test does seem to
work as intended, except the CHECKs added in
3ac4299d3798eb7078905d5fc8f23781556c90a1 aren't doing anything since
it wasn't really using FileCheck.

Added: 


Modified: 
clang/test/CodeGen/2007-06-15-AnnotateAttribute.c

Removed: 




diff  --git a/clang/test/CodeGen/2007-06-15-AnnotateAttribute.c 
b/clang/test/CodeGen/2007-06-15-AnnotateAttribute.c
index 05a9e6d7dc3941..7482fad190a4c3 100644
--- a/clang/test/CodeGen/2007-06-15-AnnotateAttribute.c
+++ b/clang/test/CodeGen/2007-06-15-AnnotateAttribute.c
@@ -1,5 +1,13 @@
-// RUN: %clang_cc1 -emit-llvm %s -o - | grep llvm.global.annotations
-// RUN: %clang_cc1 -emit-llvm %s -o - | grep llvm.var.annotation | count 3
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: @.str.3 = private unnamed_addr constant [28 x i8] 
c"GlobalValAnnotationWithArgs\00", section "llvm.metadata"
+// CHECK-NEXT: @.args = private unnamed_addr constant { i32, 
%struct.TestStruct } { i32 42, %struct.TestStruct { i32 1, i32 2 } }, section 
"llvm.metadata"
+
+// CHECK: llvm.global.annotations
+
+// CHECK: llvm.var.annotation
+// CHECK: llvm.var.annotation
+// CHECK: llvm.var.annotation
 
 /* Global variable with attribute */
 int X __attribute__((annotate("GlobalValAnnotation")));
@@ -20,13 +28,11 @@ struct TestStruct {
   int b;
 };
 int Y __attribute__((annotate(
-  "GlobalValAnnotationWithArgs", 
+  "GlobalValAnnotationWithArgs",
   42,
   (struct TestStruct) { .a = 1, .b = 2 }
 )));
 
-// CHECK: @.str.3 = private unnamed_addr constant [28 x i8] 
c"GlobalValAnnotationWithArgs\00", section "llvm.metadata"
-// CHECK-NEXT: @.args = private unnamed_addr constant { i32, 
%struct.TestStruct } { i32 42, %struct.TestStruct { i32 1, i32 2 } }, section 
"llvm.metadata"
 
 int main(void) {
   static int a __attribute__((annotate("GlobalValAnnotation")));



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

[PATCH] D157663: [Driver] Default riscv*- triples to -fdebug-default-version=4

2023-08-14 Thread Fangrui Song via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbbc0f99f3bc9: [Driver] Default riscv*- triples to 
-fdebug-default-version=4 (authored by MaskRay).

Changed prior to commit:
  https://reviews.llvm.org/D157663?vs=549233=549956#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157663

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/test/Driver/clang-g-opts.c


Index: clang/test/Driver/clang-g-opts.c
===
--- clang/test/Driver/clang-g-opts.c
+++ clang/test/Driver/clang-g-opts.c
@@ -37,3 +37,8 @@
 
 // CHECK-WITH-G-STANDALONE: "-debug-info-kind=standalone"
 // CHECK-WITH-G-STANDALONE: "-dwarf-version=2"
+
+/// TODO: Special case before R_RISCV_SET_ULEB128 linker support becomes more 
widely available.
+// RUN: %clang -### -S %s -g --target=riscv64-linux-gnu 2>&1 | FileCheck 
--check-prefix=VERSION4 %s
+// RUN: %clang -### -S %s -g --target=riscv64-unknown-elf 2>&1 | FileCheck 
--check-prefix=VERSION4 %s
+// VERSION4: "-dwarf-version=4"
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -427,6 +427,12 @@
   return UnwindTableLevel::None;
 }
 
+unsigned ToolChain::GetDefaultDwarfVersion() const {
+  // TODO: Remove the RISC-V special case when R_RISCV_SET_ULEB128 linker
+  // support becomes more widely available.
+  return getTriple().isRISCV() ? 4 : 5;
+}
+
 Tool *ToolChain::getClang() const {
   if (!Clang)
 Clang.reset(new tools::Clang(*this, useIntegratedBackend()));
Index: clang/include/clang/Driver/ToolChain.h
===
--- clang/include/clang/Driver/ToolChain.h
+++ clang/include/clang/Driver/ToolChain.h
@@ -561,7 +561,7 @@
 
   // Return the DWARF version to emit, in the absence of arguments
   // to the contrary.
-  virtual unsigned GetDefaultDwarfVersion() const { return 5; }
+  virtual unsigned GetDefaultDwarfVersion() const;
 
   // Some toolchains may have different restrictions on the DWARF version and
   // may need to adjust it. E.g. NVPTX may need to enforce DWARF2 even when 
host


Index: clang/test/Driver/clang-g-opts.c
===
--- clang/test/Driver/clang-g-opts.c
+++ clang/test/Driver/clang-g-opts.c
@@ -37,3 +37,8 @@
 
 // CHECK-WITH-G-STANDALONE: "-debug-info-kind=standalone"
 // CHECK-WITH-G-STANDALONE: "-dwarf-version=2"
+
+/// TODO: Special case before R_RISCV_SET_ULEB128 linker support becomes more widely available.
+// RUN: %clang -### -S %s -g --target=riscv64-linux-gnu 2>&1 | FileCheck --check-prefix=VERSION4 %s
+// RUN: %clang -### -S %s -g --target=riscv64-unknown-elf 2>&1 | FileCheck --check-prefix=VERSION4 %s
+// VERSION4: "-dwarf-version=4"
Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -427,6 +427,12 @@
   return UnwindTableLevel::None;
 }
 
+unsigned ToolChain::GetDefaultDwarfVersion() const {
+  // TODO: Remove the RISC-V special case when R_RISCV_SET_ULEB128 linker
+  // support becomes more widely available.
+  return getTriple().isRISCV() ? 4 : 5;
+}
+
 Tool *ToolChain::getClang() const {
   if (!Clang)
 Clang.reset(new tools::Clang(*this, useIntegratedBackend()));
Index: clang/include/clang/Driver/ToolChain.h
===
--- clang/include/clang/Driver/ToolChain.h
+++ clang/include/clang/Driver/ToolChain.h
@@ -561,7 +561,7 @@
 
   // Return the DWARF version to emit, in the absence of arguments
   // to the contrary.
-  virtual unsigned GetDefaultDwarfVersion() const { return 5; }
+  virtual unsigned GetDefaultDwarfVersion() const;
 
   // Some toolchains may have different restrictions on the DWARF version and
   // may need to adjust it. E.g. NVPTX may need to enforce DWARF2 even when host
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] bbc0f99 - [Driver] Default riscv*- triples to -fdebug-default-version=4

2023-08-14 Thread Fangrui Song via cfe-commits


Author: Fangrui Song
Date: 2023-08-14T08:26:52-07:00
New Revision: bbc0f99f3bc96f1db16f649fc21dd18e5b0918f6

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

LOG: [Driver] Default riscv*- triples to -fdebug-default-version=4

This adds a RISC-V special case to ToolChain::GetDefaultDwarfVersion,
affecting Linux/Haiku/RISCVToolChain.

DWARF v5 .debug_loclists/.debug_rnglists's
DW_LLE_offset_pair/DW_RLE_offset_pair entry kinds utilitize `.uleb128 A-B`
directives where A and B reference local labels in code sections.
When A and B are separated by a RISC-V linker-relaxable instruction,
A-B is incorrectly folded without a relocation, causing incorrect debug
information.

```
void ext(void);
int foo(int x) {ext(); return 0;}
// DW_AT_location [DW_FORM_loclistx] of a DW_TAG_formal_parameter references a 
DW_LLE_offset_pair that can be incorrect after linker relaxation.

int ext(void);
void foo() { {
  int ret = ext();
  if (__builtin_expect(ret, 0))
ext();
} }
// DW_AT_ranges [DW_FORM_rnglistx] of a DW_TAG_lexical_block references a 
DW_RLE_offset_pair that can be incorrect after linker relaxation.
```

D157657 will implement R_RISCV_SET_ULEB128/R_RISCV_SUB_ULEB128
relocations, fixing the issue, but the relocation is only supported by
bleeding-edge binutils 2.41 and not by lld/ELF yet.

The goal is to make the emitted DWARF correct after linking.
Many users don't care about the default DWARF version, but a linker
error will be unacceptable. Let's just downgrade the default DWARF
version, before binutils>=2.41 is more widely available.

An alternative compatibility option is to add a toggle to DwarfDebug.cpp,
but that doesn't seem like a good idea.

Reviewed By: asb, kito-cheng

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

Added: 


Modified: 
clang/include/clang/Driver/ToolChain.h
clang/lib/Driver/ToolChain.cpp
clang/test/Driver/clang-g-opts.c

Removed: 




diff  --git a/clang/include/clang/Driver/ToolChain.h 
b/clang/include/clang/Driver/ToolChain.h
index e3fcbd9322b0e4..2e74507f71267c 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -561,7 +561,7 @@ class ToolChain {
 
   // Return the DWARF version to emit, in the absence of arguments
   // to the contrary.
-  virtual unsigned GetDefaultDwarfVersion() const { return 5; }
+  virtual unsigned GetDefaultDwarfVersion() const;
 
   // Some toolchains may have 
diff erent restrictions on the DWARF version and
   // may need to adjust it. E.g. NVPTX may need to enforce DWARF2 even when 
host

diff  --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index d60fdbc179683b..8dafc3d481c2e0 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -427,6 +427,12 @@ ToolChain::getDefaultUnwindTableLevel(const ArgList ) 
const {
   return UnwindTableLevel::None;
 }
 
+unsigned ToolChain::GetDefaultDwarfVersion() const {
+  // TODO: Remove the RISC-V special case when R_RISCV_SET_ULEB128 linker
+  // support becomes more widely available.
+  return getTriple().isRISCV() ? 4 : 5;
+}
+
 Tool *ToolChain::getClang() const {
   if (!Clang)
 Clang.reset(new tools::Clang(*this, useIntegratedBackend()));

diff  --git a/clang/test/Driver/clang-g-opts.c 
b/clang/test/Driver/clang-g-opts.c
index d982b1070cae11..5ee0fe64fe484f 100644
--- a/clang/test/Driver/clang-g-opts.c
+++ b/clang/test/Driver/clang-g-opts.c
@@ -37,3 +37,8 @@
 
 // CHECK-WITH-G-STANDALONE: "-debug-info-kind=standalone"
 // CHECK-WITH-G-STANDALONE: "-dwarf-version=2"
+
+/// TODO: Special case before R_RISCV_SET_ULEB128 linker support becomes more 
widely available.
+// RUN: %clang -### -S %s -g --target=riscv64-linux-gnu 2>&1 | FileCheck 
--check-prefix=VERSION4 %s
+// RUN: %clang -### -S %s -g --target=riscv64-unknown-elf 2>&1 | FileCheck 
--check-prefix=VERSION4 %s
+// VERSION4: "-dwarf-version=4"



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

[PATCH] D157808: [clang] Add missing field to VisibilityAttr json AST dump

2023-08-14 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM!


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

https://reviews.llvm.org/D157808

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

[PATCH] D157275: [Driver] Select newest GCC installation on Solaris

2023-08-14 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

Thanks for the update. Regarding testing, it seems that unittests will be more 
convenience than creating so many placeholder files in `Inputs/`. 
`clang/unittests/Driver/ToolChainTest.cpp` `TEST(ToolChainTest, 
VFSGCCInstallation)` has an example for Linux. You can add another for Solaris.




Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2258
 
-  Prefixes.push_back(CandidatePrefix);
+  SolarisPrefixes.push_back(CandidatePrefix);
 }

It's better to use https://en.wikipedia.org/wiki/Schwartzian_transform here to 
avoid calling `llvm::sys::path::filename` and `Generic_GCC::GCCVersion::Parse` 
in the comparator. Then the comparator is just `A < B` and can just be removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157275

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

[PATCH] D157885: [NFC][Clang] Fix static analyzer concern about null value derefence

2023-08-14 Thread Elizabeth Andrews via Phabricator via cfe-commits

eandrews created this revision.
eandrews added reviewers: aaron.ballman, tahonermann.
Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, 
a.sidorin, baloghadamsoftware.
Herald added a project: All.
eandrews requested review of this revision.

https://reviews.llvm.org/D157885

Files:
  clang/lib/Serialization/ASTReaderDecl.cpp


Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -4459,7 +4459,9 @@
   if (auto *VTSD = dyn_cast(D)) {
 VTSD->setPointOfInstantiation(POI);
   } else if (auto *VD = dyn_cast(D)) {
-VD->getMemberSpecializationInfo()->setPointOfInstantiation(POI);
+MemberSpecializationInfo *MSInfo = VD->getMemberSpecializationInfo();
+assert(MSInfo && "No member specialization information");
+MSInfo->setPointOfInstantiation(POI);
   } else {
 auto *FD = cast(D);
 if (auto *FTSInfo = FD->TemplateOrSpecialization


Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -4459,7 +4459,9 @@
   if (auto *VTSD = dyn_cast(D)) {
 VTSD->setPointOfInstantiation(POI);
   } else if (auto *VD = dyn_cast(D)) {
-VD->getMemberSpecializationInfo()->setPointOfInstantiation(POI);
+MemberSpecializationInfo *MSInfo = VD->getMemberSpecializationInfo();
+assert(MSInfo && "No member specialization information");
+MSInfo->setPointOfInstantiation(POI);
   } else {
 auto *FD = cast(D);
 if (auto *FTSInfo = FD->TemplateOrSpecialization
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157783: [clang] Add rmissing fields to DeprecatedAttr and UnavailableAttr json AST dump

2023-08-14 Thread Aaron Ballman via Phabricator via cfe-commits

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

LGTM!


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

https://reviews.llvm.org/D157783

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

[PATCH] D142660: [AIX] supporting -X options for llvm-ranlib in AIX OS

2023-08-14 Thread Digger Lin via Phabricator via cfe-commits

DiggerLin added inline comments.



Comment at: clang/test/lit.cfg.py:391
 if "system-aix" in config.available_features:
-config.substitutions.append(("llvm-nm", "env OBJECT_MODE=any llvm-nm"))
-config.substitutions.append(("llvm-ar", "env OBJECT_MODE=any llvm-ar"))
+   config.environment["OBJECT_MODE"] ="any"
 

jhenderson wrote:
> It might be a good idea for you to run the `black` tool on python changes, to 
> ensure they conform to the coding standard, much like you do with 
> clang-format for C++ changes.
thanks for let me know.



Comment at: llvm/tools/llvm-ar/llvm-ar.cpp:74
+ << "  -X{32|64|32_64|any}   - Specify which archive symbol tables "
+"should be generated if they do not already exist (AIX OS only)\n";
 }

jhenderson wrote:
> Strictly speaking, this should be "Big Archive formats only" not "AIX OS 
> only" since theoretically you could have Big Archive archives on other 
> platforms. However, I'm not bothered if you don't want to change this. The 
> same probably goes for other tools for that matter, but don't change them now.
Strictly speaking, this should be "Big Archive format on AIX OS only" ,
in AIX OS , you can still input the -X option , but the X option not work for 
gnu archive format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142660

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

[PATCH] D157868: [clang] Use RecoveryExprs for broken defaultargs, instead of OpaqueValueExprs

2023-08-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:3026
+  void ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc,
+  ExprResult DefaultArg);
   ExprResult ConvertParamDefaultArgument(ParmVarDecl *Param, Expr *DefaultArg,

sammccall wrote:
> nit: Nullable `ExprResult*` since there are only two states?
> Extra get() in some callers, but less mysterious
i guess you meant `Expr*` ?



Comment at: clang/lib/Parse/ParseCXXInlineMethods.cpp:398
 DefArgResult = ParseAssignmentExpression();
-  DefArgResult = Actions.CorrectDelayedTyposInExpr(DefArgResult);
+  DefArgResult = Actions.CorrectDelayedTyposInExpr(DefArgResult, Param);
   if (DefArgResult.isInvalid()) {

sammccall wrote:
> If this fixes the recursive copy-constructor case you mentioned, can you add 
> a test?
> 
> (Else does it do anything? Or just cleanup)
no it doesn't. unfortunately that requires change in a separate code path to 
mark any method decls with invalid parmvardecls as invalid, which i'll try to 
put together. as that's the behavior for regular functiondecls, i don't see why 
it should be different for methods (if so we should probably change the 
behavior for functions instead).



Comment at: clang/lib/Parse/ParseCXXInlineMethods.cpp:400
   if (DefArgResult.isInvalid()) {
-Actions.ActOnParamDefaultArgumentError(Param, EqualLoc);
+Actions.ActOnParamDefaultArgumentError(Param, EqualLoc, DefArgResult);
   } else {

sammccall wrote:
> DefArgResult is never anything here. I'd probably find 
> `/*DefaultArg=*/nullptr` more obvious
maybe i am missing something, but "invalid" doesn't mean "unusable".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157868

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

[PATCH] D157868: [clang] Use RecoveryExprs for broken defaultargs, instead of OpaqueValueExprs

2023-08-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet updated this revision to Diff 549939.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Use Expr* instead of ExprResult
- Add dump test to demonstrate new RecoveryExpr


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157868

Files:
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/AST/ast-dump-default-arg-recovery.cpp
  clang/test/Index/complete-optional-params.cpp

Index: clang/test/Index/complete-optional-params.cpp
===
--- clang/test/Index/complete-optional-params.cpp
+++ clang/test/Index/complete-optional-params.cpp
@@ -79,7 +79,7 @@
 // CHECK-CC5-NEXT: Objective-C interface
 
 // RUN: c-index-test -code-completion-at=%s:17:11 %s | FileCheck -check-prefix=CHECK-CC6 %s
-// CHECK-CC6: FunctionDecl:{ResultType void}{TypedText foo_2}{LeftParen (}{Optional {Placeholder Bar1 b1 = Bar1()}{Optional {Comma , }{Placeholder Bar2 b2}}}{RightParen )} (50)
+// CHECK-CC6: FunctionDecl:{ResultType void}{TypedText foo_2}{LeftParen (}{Optional {Placeholder Bar1 b1 = Bar1()}{Optional {Comma , }{Placeholder Bar2 b2 = Bar2()}}}{RightParen )} (50)
 // CHECK-CC6: Completion contexts:
 // CHECK-CC6-NEXT: Any type
 // CHECK-CC6-NEXT: Any value
Index: clang/test/AST/ast-dump-default-arg-recovery.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-default-arg-recovery.cpp
@@ -0,0 +1,6 @@
+// RUN: not %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -ast-dump -frecovery-ast %s | FileCheck %s
+
+void foo();
+void fun(int arg = foo());
+//  CHECK: -ParmVarDecl {{.*}}  col:14 invalid arg 'int' cinit
+// CHECK-NEXT:   -RecoveryExpr {{.*}}  'int' contains-errors
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -20,6 +20,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
+#include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/RecursiveASTVisitor.h"
@@ -37,11 +38,13 @@
 #include "clang/Sema/EnterExpressionEvaluationContext.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
+#include "clang/Sema/Ownership.h"
 #include "clang/Sema/ParsedTemplate.h"
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/ScopeInfo.h"
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Template.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
@@ -329,23 +332,16 @@
   ParmVarDecl *Param = cast(param);
   UnparsedDefaultArgLocs.erase(Param);
 
-  auto Fail = [&] {
-Param->setInvalidDecl();
-Param->setDefaultArg(new (Context) OpaqueValueExpr(
-EqualLoc, Param->getType().getNonReferenceType(), VK_PRValue));
-  };
-
   // Default arguments are only permitted in C++
   if (!getLangOpts().CPlusPlus) {
 Diag(EqualLoc, diag::err_param_default_argument)
   << DefaultArg->getSourceRange();
-return Fail();
+return ActOnParamDefaultArgumentError(param, EqualLoc, DefaultArg);
   }
 
   // Check for unexpanded parameter packs.
-  if (DiagnoseUnexpandedParameterPack(DefaultArg, UPPC_DefaultArgument)) {
-return Fail();
-  }
+  if (DiagnoseUnexpandedParameterPack(DefaultArg, UPPC_DefaultArgument))
+return ActOnParamDefaultArgumentError(param, EqualLoc, DefaultArg);
 
   // C++11 [dcl.fct.default]p3
   //   A default argument expression [...] shall not be specified for a
@@ -360,14 +356,14 @@
 
   ExprResult Result = ConvertParamDefaultArgument(Param, DefaultArg, EqualLoc);
   if (Result.isInvalid())
-return Fail();
+return ActOnParamDefaultArgumentError(param, EqualLoc, DefaultArg);
 
   DefaultArg = Result.getAs();
 
   // Check that the default argument is well-formed
   CheckDefaultArgumentVisitor DefaultArgChecker(*this, DefaultArg);
   if (DefaultArgChecker.Visit(DefaultArg))
-return Fail();
+return ActOnParamDefaultArgumentError(param, EqualLoc, DefaultArg);
 
   SetParamDefaultArgument(Param, DefaultArg, EqualLoc);
 }
@@ -389,16 +385,23 @@
 
 /// ActOnParamDefaultArgumentError - Parsing or semantic analysis of
 /// the default argument for the parameter param failed.
-void Sema::ActOnParamDefaultArgumentError(Decl *param,
-  SourceLocation EqualLoc) {
+void Sema::ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc,
+  Expr *DefaultArg) {
   if (!param)
 return;
 
   ParmVarDecl *Param = cast(param);
   Param->setInvalidDecl();

[PATCH] D142660: [AIX] supporting -X options for llvm-ranlib in AIX OS

2023-08-14 Thread Digger Lin via Phabricator via cfe-commits

DiggerLin updated this revision to Diff 549938.
DiggerLin marked 4 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142660

Files:
  clang/lib/Driver/OffloadBundler.cpp
  clang/test/lit.cfg.py
  clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
  llvm/include/llvm/Object/Archive.h
  llvm/include/llvm/Object/ArchiveWriter.h
  llvm/lib/ObjCopy/Archive.cpp
  llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp
  llvm/lib/Object/Archive.cpp
  llvm/lib/Object/ArchiveWriter.cpp
  llvm/lib/Object/COFFImportFile.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/test/tools/llvm-ranlib/AIX-X-option-non-AIX.test
  llvm/test/tools/llvm-ranlib/aix-X-option.test
  llvm/tools/llvm-ar/llvm-ar.cpp
  llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp

Index: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp
===
--- llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp
+++ llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp
@@ -594,18 +594,17 @@
 
   if (NewMembers.size() == 1)
 return writeArchive(OutputFile, NewMembers.begin()->second.getMembers(),
-/*WriteSymtab=*/true,
+SymtabWritingMode::NormalSymtab,
 /*Kind=*/object::Archive::K_DARWIN, C.Deterministic,
 /*Thin=*/false);
 
   SmallVector, 2> OutputBinaries;
   for (const std::pair  : NewMembers) {
 Expected> OutputBufferOrErr =
-writeArchiveToBuffer(M.second.getMembers(),
- /*WriteSymtab=*/true,
- /*Kind=*/object::Archive::K_DARWIN,
- C.Deterministic,
- /*Thin=*/false);
+writeArchiveToBuffer(
+M.second.getMembers(), SymtabWritingMode::NormalSymtab,
+/*Kind=*/object::Archive::K_DARWIN, C.Deterministic,
+/*Thin=*/false);
 if (!OutputBufferOrErr)
   return OutputBufferOrErr.takeError();
 std::unique_ptr  = OutputBufferOrErr.get();
Index: llvm/tools/llvm-ar/llvm-ar.cpp
===
--- llvm/tools/llvm-ar/llvm-ar.cpp
+++ llvm/tools/llvm-ar/llvm-ar.cpp
@@ -69,7 +69,9 @@
  << "  -v --version  - Display the version of this program\n"
  << "  -D- Use zero for timestamps and uids/gids "
 "(default)\n"
- << "  -U- Use actual timestamps and uids/gids\n";
+ << "  -U- Use actual timestamps and uids/gids\n"
+ << "  -X{32|64|32_64|any}   - Specify which archive symbol tables "
+"should be generated if they do not already exist (AIX OS only)\n";
 }
 
 static void printArHelp(StringRef ToolName) {
@@ -225,7 +227,8 @@
 static bool CompareFullPath = false;  ///< 'P' modifier
 static bool OnlyUpdate = false;   ///< 'u' modifier
 static bool Verbose = false;  ///< 'v' modifier
-static bool Symtab = true;///< 's' modifier
+static SymtabWritingMode Symtab =
+SymtabWritingMode::NormalSymtab;  ///< 's' modifier
 static bool Deterministic = true; ///< 'D' and 'U' modifiers
 static bool Thin = false; ///< 'T' modifier
 static bool AddLibrary = false;   ///< 'L' modifier
@@ -371,11 +374,11 @@
   CompareFullPath = true;
   break;
 case 's':
-  Symtab = true;
+  Symtab = SymtabWritingMode::NormalSymtab;
   MaybeJustCreateSymTab = true;
   break;
 case 'S':
-  Symtab = false;
+  Symtab = SymtabWritingMode::NoSymtab;
   break;
 case 'u':
   OnlyUpdate = true;
@@ -1074,9 +1077,31 @@
   // In summary, we only need to update the symbol table if we have none.
   // This is actually very common because of broken build systems that think
   // they have to run ranlib.
-  if (OldArchive->hasSymbolTable())
-return;
+  if (OldArchive->hasSymbolTable()) {
+if (OldArchive->kind() != object::Archive::K_AIXBIG)
+  return;
 
+// For archives in the Big Archive format, the bit mode option specifies
+// which symbol table to generate. The presence of a symbol table that does
+// not match the specified bit mode does not prevent creation of the symbol
+// table that has been requested.
+if (OldArchive->kind() == object::Archive::K_AIXBIG) {
+  BigArchive *BigArc = dyn_cast(OldArchive);
+  if (BigArc->has32BitGlobalSymtab() &&
+  Symtab == SymtabWritingMode::BigArchive32)
+return;
+
+  if (BigArc->has64BitGlobalSymtab() &&
+  Symtab == SymtabWritingMode::BigArchive64)
+return;
+
+  if (BigArc->has32BitGlobalSymtab() && BigArc->has64BitGlobalSymtab() &&
+  Symtab == SymtabWritingMode::NormalSymtab)
+return;
+
+  Symtab =

[PATCH] D157869: [clang-tidy] Avoid overflow when dumping unsigned integer values

2023-08-14 Thread Piotr Zegar via Phabricator via cfe-commits

PiotrZSL added a comment.

I agree that this change is an improvement that is welcome, but that's not a 
fix for a #60217.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157869

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

[PATCH] D157869: [clang-tidy] Avoid overflow when dumping unsigned integer values

2023-08-14 Thread Piotr Zegar via Phabricator via cfe-commits

PiotrZSL added a comment.

This wont fix issue, simply because problem happen during reading -1, not 
during dumping, and per documentation -1 was a valid value. Same issue happen 
in other checks that use -1. Most probably best way would be to change those 
variables into std::int64_t, in C++ creating any structure of size above max 
int will fail with error anyway, so using signed i64 should be sufficient, and 
would also accept -1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157869

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

[PATCH] D157879: [clang] Report missing designated initializers in C++

2023-08-14 Thread Mariya Podchishchaeva via Phabricator via cfe-commits

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

Prior to this change clang didn't emit missing-field-initializers
warning for designated initializers. The comments say that it is done to
match gcc behavior. However, gcc behaves so only for C. For C++ warnings
are emitted.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157879

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp

Index: clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
===
--- clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
+++ clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp
@@ -4,6 +4,7 @@
 // RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,reorder -Wno-c99-designator -Werror=reorder-init-list -Wno-initializer-overrides
 // RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,override -Wno-c99-designator -Wno-reorder-init-list -Werror=initializer-overrides
 // RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected -Wno-c99-designator -Wno-reorder-init-list -Wno-initializer-overrides
+// RUN: %clang_cc1 -std=c++20 %s -verify=cxx20,expected,wmissing -Wmissing-field-initializers -Wno-c99-designator -Wno-reorder-init-list -Wno-initializer-overrides
 
 
 namespace class_with_ctor {
@@ -49,15 +50,18 @@
 A a4 = {
   .x = 1, // override-note {{previous}}
   .x = 1 // override-error {{overrides prior initialization}}
-};
+}; // wmissing-warning {{missing field 'y' initializer}}
 A a5 = {
   .y = 1, // override-note {{previous}}
   .y = 1 // override-error {{overrides prior initialization}}
-};
+}; // wmissing-warning {{missing field 'x' initializer}}
 B b2 = {.a = 1}; // pedantic-error {{brace elision for designated initializer is a C99 extension}}
+ // wmissing-warning@-1 {{missing field 'y' initializer}}
 B b3 = {.a = 1, 2}; // pedantic-error {{mixture of designated and non-designated}} pedantic-note {{first non-designated}} pedantic-error {{brace elision}}
 B b4 = {.a = 1, 2, 3}; // pedantic-error {{mixture of designated and non-designated}} pedantic-note {{first non-designated}} pedantic-error {{brace elision}} expected-error {{excess elements}}
 B b5 = {.a = nullptr}; // expected-error {{cannot initialize}}
+   // wmissing-warning@-1 {{missing field 'y' initializer}}
+   // wmissing-warning@-2 {{missing field 'a' initializer}}
 struct C { int :0, x, :0, y, :0; };
 C c = {
   .x = 1, // override-note {{previous}}
@@ -66,7 +70,14 @@
   .y = 1, // override-error {{overrides prior initialization}} // reorder-note {{previous initialization for field 'y' is here}}
   .x = 1, // reorder-error {{declaration order}} override-error {{overrides prior initialization}} override-note {{previous}}
   .x = 1, // override-error {{overrides prior initialization}}
-};
+}; //wmissing-warning {{missing field 'x' initializer}}
+
+struct Foo { int a, b; };
+
+struct Foo foo0 = { 1 }; // wmissing-warning {{missing field 'b' initializer}}
+struct Foo foo1 = { .a = 1 }; // wmissing-warning {{missing field 'b' initializer}}
+struct Foo foo2 = { .b = 1 }; // wmissing-warning {{missing field 'a' initializer}}
+
 }
 
 namespace base_class {
@@ -215,5 +226,5 @@
 .c = 1, // reorder-error {{field 'd' will be initialized after field 'c'}} // reorder-note {{previous initialization for field 'c' is here}}
 .b = 1, // reorder-error {{field 'c' will be initialized after field 'b'}} // reorder-note {{previous initialization for field 'b' is here}}
 .a = 1, // reorder-error {{field 'b' will be initialized after field 'a'}}
-};
+}; // wmissing-warning {{missing field 'a' initializer}}
 }
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -2252,6 +2252,8 @@
 !IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts());
   bool HasDesignatedInit = false;
 
+  llvm::SmallPtrSet InitializedFields;
+
   while (Index < IList->getNumInits()) {
 Expr *Init = IList->getInit(Index);
 SourceLocation InitLoc = Init->getBeginLoc();
@@ -2277,6 +2279,7 @@
 RecordDecl::field_iterator F = RD->field_begin();
 while (std::next(F) != Field)
   ++F;
+InitializedFields.insert(*F);
 QualType ET = SemaRef.Context.getBaseElementType(F->getType());
 if (checkDestructorReference(ET, InitLoc, SemaRef)) {
   hadError = true;
@@ -2288,7 +2291,8 @@
 
   // Disable check for missing fields when designators are used.
   // This matches gcc behaviour.
-  CheckForMissingFields = false;
+  if (!SemaRef.getLangOpts().CPlusPlus)
+CheckForMissingFields = false;
   continue;
 }
 
@@ -2367,6 +2371,7 @@

[PATCH] D157783: [clang] Add rmissing fields to DeprecatedAttr and UnavailableAttr json AST dump

2023-08-14 Thread serge via Phabricator via cfe-commits

serge-sans-paille marked an inline comment as done.
serge-sans-paille added a comment.

Review taken into account. Thanks for handling this batch of patches o/


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

https://reviews.llvm.org/D157783

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

[PATCH] D157783: [clang] Add rmissing fields to DeprecatedAttr and UnavailableAttr json AST dump

2023-08-14 Thread serge via Phabricator via cfe-commits

serge-sans-paille updated this revision to Diff 549929.
serge-sans-paille added a comment.

Only output extra fields if they are not empty


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

https://reviews.llvm.org/D157783

Files:
  clang/include/clang/AST/JSONNodeDumper.h
  clang/lib/AST/JSONNodeDumper.cpp
  clang/test/AST/ast-dump-attr-json.cpp

Index: clang/test/AST/ast-dump-attr-json.cpp
===
--- clang/test/AST/ast-dump-attr-json.cpp
+++ clang/test/AST/ast-dump-attr-json.cpp
@@ -8,6 +8,13 @@
   __attribute__((cleanup(cleanup_function))) int var;
 }
 
+__attribute__((deprecated)) int deprecated_var0;
+__attribute__((deprecated("reason"))) int deprecated_var1;
+__attribute__((deprecated("reason", "replacement"))) int deprecated_var2;
+
+__attribute__((unavailable)) int unavailable_var0;
+__attribute__((unavailable("reason"))) int unavailable_var1;
+
 // NOTE: CHECK lines have been autogenerated by gen_ast_dump_json_test.py
 // using --filters=VarDecl
 
@@ -139,3 +146,237 @@
 // CHECK-NEXT:   }
 // CHECK-NEXT:  ]
 // CHECK-NEXT: }
+
+
+// CHECK-NOT: {{^}}Dumping
+// CHECK:  "kind": "VarDecl",
+// CHECK-NEXT:  "loc": {
+// CHECK-NEXT:   "offset": 282,
+// CHECK-NEXT:   "line": 11,
+// CHECK-NEXT:   "col": 33,
+// CHECK-NEXT:   "tokLen": 15
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:"offset": 250,
+// CHECK-NEXT:"col": 1,
+// CHECK-NEXT:"tokLen": 13
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:"offset": 282,
+// CHECK-NEXT:"col": 33,
+// CHECK-NEXT:"tokLen": 15
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "name": "deprecated_var0",
+// CHECK-NEXT:  "mangledName": "deprecated_var0",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "int"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "DeprecatedAttr",
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT:  "offset": 265,
+// CHECK-NEXT:  "col": 16,
+// CHECK-NEXT:  "tokLen": 10
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {
+// CHECK-NEXT:  "offset": 265,
+// CHECK-NEXT:  "col": 16,
+// CHECK-NEXT:  "tokLen": 10
+// CHECK-NEXT: }
+// CHECK-NEXT:}
+// CHECK-NEXT:   }
+// CHECK-NEXT:  ]
+// CHECK-NEXT: }
+
+
+// CHECK-NOT: {{^}}Dumping
+// CHECK:  "kind": "VarDecl",
+// CHECK-NEXT:  "loc": {
+// CHECK-NEXT:   "offset": 341,
+// CHECK-NEXT:   "line": 12,
+// CHECK-NEXT:   "col": 43,
+// CHECK-NEXT:   "tokLen": 15
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:"offset": 299,
+// CHECK-NEXT:"col": 1,
+// CHECK-NEXT:"tokLen": 13
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:"offset": 341,
+// CHECK-NEXT:"col": 43,
+// CHECK-NEXT:"tokLen": 15
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "name": "deprecated_var1",
+// CHECK-NEXT:  "mangledName": "deprecated_var1",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "int"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "DeprecatedAttr",
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT:  "offset": 314,
+// CHECK-NEXT:  "col": 16,
+// CHECK-NEXT:  "tokLen": 10
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {
+// CHECK-NEXT:  "offset": 333,
+// CHECK-NEXT:  "col": 35,
+// CHECK-NEXT:  "tokLen": 1
+// CHECK-NEXT: }
+// CHECK-NEXT:},
+// CHECK-NEXT:"message": "reason"
+// CHECK-NEXT:   }
+// CHECK-NEXT:  ]
+// CHECK-NEXT: }
+
+
+// CHECK-NOT: {{^}}Dumping
+// CHECK:  "kind": "VarDecl",
+// CHECK-NEXT:  "loc": {
+// CHECK-NEXT:   "offset": 415,
+// CHECK-NEXT:   "line": 13,
+// CHECK-NEXT:   "col": 58,
+// CHECK-NEXT:   "tokLen": 15
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {
+// CHECK-NEXT:"offset": 358,
+// CHECK-NEXT:"col": 1,
+// CHECK-NEXT:"tokLen": 13
+// CHECK-NEXT:   },
+// CHECK-NEXT:   "end": {
+// CHECK-NEXT:"offset": 415,
+// CHECK-NEXT:"col": 58,
+// CHECK-NEXT:"tokLen": 15
+// CHECK-NEXT:   }
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "name": "deprecated_var2",
+// CHECK-NEXT:  "mangledName": "deprecated_var2",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "int"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "DeprecatedAttr",
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {
+// CHECK-NEXT:  "offset": 373,
+// CHECK-NEXT:  "col": 16,
+// CHECK-NEXT:  "tokLen": 10
+// CHECK-NEXT: },
+// CHECK-NEXT: "end": {
+// CHECK-NEXT:  "offset": 407,
+// CHECK-NEXT:  "col": 50,
+// CHECK-NEXT:  "tokLen": 1
+// CHECK-NEXT: }
+// CHECK-NEXT:},
+//

[PATCH] D155978: [SPIRV] Add SPIR-V logical triple.

2023-08-14 Thread Nathan Gauër via Phabricator via cfe-commits

Keenuts added inline comments.



Comment at: llvm/unittests/TargetParser/TripleTest.cpp:1307
+  EXPECT_TRUE(T.isSPIRV());
+
   T.setArch(Triple::spirv32);

Anastasia wrote:
> pmatos wrote:
> > Keenuts wrote:
> > > pmatos wrote:
> > > > I am not well-versed in SPIRV for gfx but if we are using 32bits in 
> > > > SPIRV logical, can't we reuse spirv32? Is there some sort of strong 
> > > > reason not to or adding spirv for logical spirv as an alternative to 
> > > > spirv32 and spirv64 is just easier?
> > > This is a very valid question! And I'm not sure of the best option.
> > > My understanding is: in logical SPIR-V, there are no notion of pointer 
> > > size, or architecture size. We cannot offset pointers by a sizeof(), or 
> > > do such things.
> > > 
> > > But the options I had didn't seem to allow this kind of "undefined 
> > > architecture".
> > > I chose 32bits here because the IDs are 32bit. But pointer addresses are 
> > > not, so returning 32bit is not quite right either.
> > > 
> > > 
> > This is a tricky one tbh - I would have to do a bit more research to have a 
> > more insighful reply here. Maybe @mpaszkowski or @Anastasia can add more.
> I think to do this properly in LLVM would require an IR extension or 
> something, it is maybe worth starting RFC to discuss this.
> 
> Out of curiosity do you have any code that can be compiled from any GFX 
> source language that would need a pointer size to be emitted in IR? If there 
> is no code that can be written in such a way then using one fixed pointer 
> width could be ok. Then the SPIR-V backend should just recognise it and lower 
> as required. Otherwise target configurable types might be useful: 
> https://llvm.org/docs/LangRef.html#target-extension-type
> 
> In general we tried to avoid adding bitwidth neutral SPIR-V triple originally 
> just because LLVM always requires a pointer width. We were thinking of adding 
> vulkan as a component to the existing SPIR-V 34|64 targets 
> https://discourse.llvm.org/t/setting-version-environment-and-extensions-for-spir-v-target.
Hello!

Thanks for the feedback!
I asked around, and it seems that no, we cannot write code that pointer 
arithmetic or requires the pointer size that I know of.
The code that could require is changes the memory modal to something else, like 
this proposal: 
https://github.com/microsoft/hlsl-specs/blob/main/proposals/0010-vk-buffer-ref.md
But here, the memory model is PhysicalStorageBuffer64, which tells us pointers 
are 64bit.

We also have the SPV_KHR_variable_pointers extension 
(https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/KHR/SPV_KHR_variable_pointers.asciidoc)
which specifically says pointers have no size or bit pattern...


> Otherwise target configurable types might be useful

Not familiar enough with LLVM/Clang to fully understand, so reformulating we'd 
have:
- the target would return 32-bit, even if we have no pointer size
- the IR would not use classic pointers at all, since their type would be 
misleading.
- the IR would use this newly created type (let's call is LogicalPointer) 
instead of real pointers.
- the backend would then convert this new type to something else when lowering.

If that's the case, seems fair, I'd follow this advice.

> We were thinking of adding vulkan as a component to the existing SPIR-V 34|64 
> targets
Same idea here, add VulkanX to the OSType part of the triple (replacing the 
ShaderModel used for DXIL).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155978

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

[PATCH] D157275: [Driver] Select newest GCC installation on Solaris

2023-08-14 Thread Rainer Orth via Phabricator via cfe-commits

ro updated this revision to Diff 549925.
ro added a comment.
Herald added a subscriber: mgrang.

Updated based on discussions in Issue #53709:

- Sort Solaris GCC prefixes in reverse version order so the latest version is 
picked.
- Update testcase to match.

Tested on `amd64-pc-solaris2.11` and `x86_64-pc-linux-gnu`.

I'm seeing weird testsuite failures in 2-stage builds on Solaris/amd64 with gcc 
12, but not with clang 16, which I don't yet understand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157275

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0/sparcv8plus/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0/32/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/11/lib/gcc/x86_64-pc-solaris2.11/11.4.0/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/4.7/lib/gcc/i386-pc-solaris2.11/4.7.3/amd64/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/4.7/lib/gcc/i386-pc-solaris2.11/4.7.3/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/4.7/lib/gcc/sparc-sun-solaris2.11/4.7.3/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/4.7/lib/gcc/sparc-sun-solaris2.11/4.7.3/sparcv9/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/7/lib/gcc/sparcv9-sun-solaris2.11/7.5.0/32/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/7/lib/gcc/sparcv9-sun-solaris2.11/7.5.0/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/7/lib/gcc/x86_64-pc-solaris2.11/7.5.0/32/crtbegin.o
  
clang/test/Driver/Inputs/solaris_multi_gcc_tree/usr/gcc/7/lib/gcc/x86_64-pc-solaris2.11/7.5.0/crtbegin.o
  clang/test/Driver/solaris-multi-gcc-search.test


Index: clang/test/Driver/solaris-multi-gcc-search.test
===
--- /dev/null
+++ clang/test/Driver/solaris-multi-gcc-search.test
@@ -0,0 +1,51 @@
+/// Check that clang uses the latest available Solaris GCC installation.
+
+/// Check sparc-sun-solaris2.11, 32-bit
+// RUN: %clang -v 2>&1 --target=sparc-sun-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_multi_gcc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SPARC %s
+// CHECK-SPARC: Found candidate GCC installation: 
{{.*}}11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0
+// CHECK-SPARC-NEXT: Selected GCC installation: 
{{.*}}11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0
+// CHECK-SPARC-NEXT: Candidate multilib: .;@m64
+// CHECK-SPARC-NEXT: Candidate multilib: sparcv8plus;@m32
+// CHECK-SPARC-NEXT: Selected multilib: sparcv8plus;@m32
+
+/// Check i386-pc-solaris2.11, 32-bit
+// RUN: %clang -v 2>&1 --target=i386-pc-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_multi_gcc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-I386 %s
+// CHECK-I386: Found candidate GCC installation: 
{{.*}}11/lib/gcc/x86_64-pc-solaris2.11/11.4.0
+// CHECK-I386-NEXT: Selected GCC installation: 
{{.*}}11/lib/gcc/x86_64-pc-solaris2.11/11.4.0
+// CHECK-I386-NEXT: Candidate multilib: .;@m64
+// CHECK-I386-NEXT: Candidate multilib: 32;@m32
+// CHECK-I386-NEXT: Selected multilib: 32;@m32
+
+/// Check sparcv9-sun-solaris2.11, 64-bit
+// RUN: %clang -v 2>&1 --target=sparcv9-sun-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_multi_gcc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-SPARCV9 %s
+// CHECK-SPARCV9: Found candidate GCC installation: 
{{.*}}11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0
+// CHECK-SPARCV9-NEXT: Selected GCC installation: 
{{.*}}11/lib/gcc/sparcv9-sun-solaris2.11/11.4.0
+// CHECK-SPARCV9-NEXT: Candidate multilib: .;@m64
+// CHECK-SPARCV9-NEXT: Candidate multilib: sparcv8plus;@m32
+// CHECK-SPARCV9-NEXT: Selected multilib: .;@m64
+
+/// Check x86_64-pc-solaris2.11, 64-bit
+// RUN: %clang -v 2>&1 --target=x86_64-pc-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_multi_gcc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-X86_64 %s
+
+/// Check amd64-pc-solaris2.11, 64-bit
+// RUN: %clang -v 2>&1 --target=amd64-pc-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_multi_gcc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-X86_64 %s
+// CHECK-X86_64: Found candidate GCC installation: 
{{.*}}11/lib/gcc/x86_64-pc-solaris2.11/11.4.0
+// CHECK-X86_64-NEXT: Selected GCC installation: 
{{.*}}11/lib/gcc/x86_64-pc-solaris2.11/11.4.0
+// CHECK-X86_64-NEXT: Candidate multilib: .;@m64
+// CHECK-X86_64-NEXT: Candidate multilib: 32;@m32
+// CHECK-X86_64-NEXT: Selected multilib: .;@m64
Index:

[PATCH] D153701: [Clang] Implement P2718R0 "Lifetime extension in range-based for loops"

2023-08-14 Thread Yurong via Phabricator via cfe-commits

yronglin added a comment.

In D153701#4563919 , @yronglin wrote:

> Sorry for the late reply.  I tried to investigate the memory impact of 
> creating these additional materializations by build the whole llvm-project 
> and compare the number of `MaterializedTemporaryExpr` created during parsing.
>
> Steps:
>
> 1. Add a public member `uint64_t NumMetarilizedTemporaryExpr = 0;` in 
> `ASTContext` .
> 2. Increment the value of `NumMetarilizedTemporaryExpr ` in 
> `Sema::CreateMaterializeTemporaryExpr`.
> 3. Write the `NumMetarilizedTemporaryExpr ` into a text file when 
> `ASTContext` destruction, each translation unit will append a line to the 
> file to record the value of `NumMetarilizedTemporaryExpr `.
> 4. Build the entire llvm-project separately using the compiler that creates 
> addational materializations and the compiler that doesn't.
> 5. Sum the numbers produced by each translation unit.
>
> The result is:
>
> | Item | Count |
> | Addational Materialized Temporarys Total | 50655585 |
> | Clang Trunk Total| 18346347 |
> | Diff | 32309238 |
> |
>
> The more detail result in https://yronglin.github.io
>
> The gap between these two numbers is very large. So I'think we can create 
> additional materializations only within for-range initializers. I'm not sure 
> if I can find a way to only create materializes for temporaries that need to 
> have an extended lifetime, WDYT?

I have updated the table, and it was sorted by `Count Inc` col.

**Num files: 7445**

| Item | Count | Mem  |
| Addational Materialized Temporarys Total | 50655585 | 1187240.2734 
(KB)  |
| Clang Trunk Total| 18346347 | 429992.5078 
(KB) |
| Diff | 32309238 | 757247.7656 
(KB) |
| Avg  | 4339.7230 | 4.2380 (KB/file) |


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153701

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

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

2023-08-14 Thread Elizabeth Andrews 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 rG421c9bbf65b7: [NFC][Clang] Fix static analyzer concern 
(authored by eandrews).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157454

Files:
  clang/lib/CodeGen/CGObjC.cpp


Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -222,6 +222,7 @@
   QualType ResultType = E->getType();
   const ObjCObjectPointerType *InterfacePointerType
 = ResultType->getAsObjCInterfacePointerType();
+  assert(InterfacePointerType && "Unexpected InterfacePointerType - null");
   ObjCInterfaceDecl *Class
 = InterfacePointerType->getObjectType()->getInterface();
   CGObjCRuntime  = CGM.getObjCRuntime();


Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -222,6 +222,7 @@
   QualType ResultType = E->getType();
   const ObjCObjectPointerType *InterfacePointerType
 = ResultType->getAsObjCInterfacePointerType();
+  assert(InterfacePointerType && "Unexpected InterfacePointerType - null");
   ObjCInterfaceDecl *Class
 = InterfacePointerType->getObjectType()->getInterface();
   CGObjCRuntime  = CGM.getObjCRuntime();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 421c9bb - [NFC][Clang] Fix static analyzer concern

2023-08-14 Thread Elizabeth Andrews via cfe-commits


Author: Elizabeth Andrews
Date: 2023-08-14T07:14:32-07:00
New Revision: 421c9bbf65b78e3410415cac2edf4e00bd4d38ca

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

LOG: [NFC][Clang] Fix static analyzer concern

Fix static analyzer concern about null value
dereference. InterfacePointerType is dereferenced
and should not be null.

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

Added: 


Modified: 
clang/lib/CodeGen/CGObjC.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGObjC.cpp b/clang/lib/CodeGen/CGObjC.cpp
index 46c37eaea82b1a..6c594b5db4bca1 100644
--- a/clang/lib/CodeGen/CGObjC.cpp
+++ b/clang/lib/CodeGen/CGObjC.cpp
@@ -222,6 +222,7 @@ llvm::Value 
*CodeGenFunction::EmitObjCCollectionLiteral(const Expr *E,
   QualType ResultType = E->getType();
   const ObjCObjectPointerType *InterfacePointerType
 = ResultType->getAsObjCInterfacePointerType();
+  assert(InterfacePointerType && "Unexpected InterfacePointerType - null");
   ObjCInterfaceDecl *Class
 = InterfacePointerType->getObjectType()->getInterface();
   CGObjCRuntime  = CGM.getObjCRuntime();



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

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

2023-08-14 Thread Elizabeth Andrews via Phabricator via cfe-commits

eandrews added a comment.

Pre-merge check fails are unrelated - fatal error C1060: compiler is out of 
heap space


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

https://reviews.llvm.org/D157454

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

[PATCH] D157868: [clang] Use RecoveryExprs for broken defaultargs, instead of OpaqueValueExprs

2023-08-14 Thread Sam McCall via Phabricator via cfe-commits

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

Nice, recoveryexpr is a better fit here. These changes tend to cause occasional 
new error-handling crashes on under-tested paths, but I guess it's a good time 
in the release cycle for that!

You might want a clangd or ast-dump test showing that we now preserve the 
internal structure of (some) invalid default initializes.




Comment at: clang/include/clang/Sema/Sema.h:3026
+  void ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc,
+  ExprResult DefaultArg);
   ExprResult ConvertParamDefaultArgument(ParmVarDecl *Param, Expr *DefaultArg,

nit: Nullable `ExprResult*` since there are only two states?
Extra get() in some callers, but less mysterious



Comment at: clang/lib/Parse/ParseCXXInlineMethods.cpp:398
 DefArgResult = ParseAssignmentExpression();
-  DefArgResult = Actions.CorrectDelayedTyposInExpr(DefArgResult);
+  DefArgResult = Actions.CorrectDelayedTyposInExpr(DefArgResult, Param);
   if (DefArgResult.isInvalid()) {

If this fixes the recursive copy-constructor case you mentioned, can you add a 
test?

(Else does it do anything? Or just cleanup)



Comment at: clang/lib/Parse/ParseCXXInlineMethods.cpp:400
   if (DefArgResult.isInvalid()) {
-Actions.ActOnParamDefaultArgumentError(Param, EqualLoc);
+Actions.ActOnParamDefaultArgumentError(Param, EqualLoc, DefArgResult);
   } else {

DefArgResult is never anything here. I'd probably find `/*DefaultArg=*/nullptr` 
more obvious



Comment at: clang/lib/Parse/ParseDecl.cpp:7478
+Actions.ActOnParamDefaultArgumentError(Param, EqualLoc,
+   DefArgResult);
 SkipUntil(tok::comma, tok::r_paren, StopAtSemi | StopBeforeMatch);

Ditto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157868

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

[PATCH] D157663: [Driver] Default riscv*- triples to -fdebug-default-version=4

2023-08-14 Thread Alex Bradbury via Phabricator via cfe-commits

asb accepted this revision.
asb added a comment.

LGTM, this seems like a good workaround.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157663

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

[PATCH] D157869: [clang-tidy] Avoid overflow when dumping unsigned integer values

2023-08-14 Thread Daniel Alcaide Nombela via Phabricator via cfe-commits

ealcdan created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
ealcdan requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Some options take the maximum unsigned integer value as default, but
they are being dumped to a string as integers. This makes -dump-config
write invalid '-1' values for these options. This change fixes this
issue by using utostr if the option is unsigned.

Change-Id: I22d481047330a89984d6eb27bb02f85c71d42bdb

Fixes: #60217


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157869

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h


Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -350,12 +350,21 @@
 /// Stores an option with the check-local name \p LocalName with
 /// integer value \p Value to \p Options.
 template 
-std::enable_if_t::value>
+std::enable_if_t::value && std::is_signed::value>
 store(ClangTidyOptions::OptionMap , StringRef LocalName,
   T Value) const {
   storeInt(Options, LocalName, Value);
 }
 
+/// Stores an option with the check-local name \p LocalName with
+/// unsigned integer value \p Value to \p Options.
+template 
+std::enable_if_t::value>
+store(ClangTidyOptions::OptionMap , StringRef LocalName,
+  T Value) const {
+  storeUnsigned(Options, LocalName, Value);
+}
+
 /// Stores an option with the check-local name \p LocalName as the string
 /// representation of the Enum \p Value to \p Options.
 ///
@@ -398,7 +407,8 @@
 
 void storeInt(ClangTidyOptions::OptionMap , StringRef LocalName,
   int64_t Value) const;
-
+void storeUnsigned(ClangTidyOptions::OptionMap ,
+   StringRef LocalName, uint64_t Value) const;
 
 std::string NamePrefix;
 const ClangTidyOptions::OptionMap 
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -138,6 +138,12 @@
   store(Options, LocalName, llvm::itostr(Value));
 }
 
+void ClangTidyCheck::OptionsView::storeUnsigned(
+ClangTidyOptions::OptionMap , StringRef LocalName,
+uint64_t Value) const {
+  store(Options, LocalName, llvm::utostr(Value));
+}
+
 template <>
 void ClangTidyCheck::OptionsView::store(
 ClangTidyOptions::OptionMap , StringRef LocalName,


Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -350,12 +350,21 @@
 /// Stores an option with the check-local name \p LocalName with
 /// integer value \p Value to \p Options.
 template 
-std::enable_if_t::value>
+std::enable_if_t::value && std::is_signed::value>
 store(ClangTidyOptions::OptionMap , StringRef LocalName,
   T Value) const {
   storeInt(Options, LocalName, Value);
 }
 
+/// Stores an option with the check-local name \p LocalName with
+/// unsigned integer value \p Value to \p Options.
+template 
+std::enable_if_t::value>
+store(ClangTidyOptions::OptionMap , StringRef LocalName,
+  T Value) const {
+  storeUnsigned(Options, LocalName, Value);
+}
+
 /// Stores an option with the check-local name \p LocalName as the string
 /// representation of the Enum \p Value to \p Options.
 ///
@@ -398,7 +407,8 @@
 
 void storeInt(ClangTidyOptions::OptionMap , StringRef LocalName,
   int64_t Value) const;
-
+void storeUnsigned(ClangTidyOptions::OptionMap ,
+   StringRef LocalName, uint64_t Value) const;
 
 std::string NamePrefix;
 const ClangTidyOptions::OptionMap 
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -138,6 +138,12 @@
   store(Options, LocalName, llvm::itostr(Value));
 }
 
+void ClangTidyCheck::OptionsView::storeUnsigned(
+ClangTidyOptions::OptionMap , StringRef LocalName,
+uint64_t Value) const {
+  store(Options, LocalName, llvm::utostr(Value));
+}
+
 template <>
 void ClangTidyCheck::OptionsView::store(
 ClangTidyOptions::OptionMap , StringRef LocalName,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157868: [clang] Use RecoveryExprs for broken defaultargs, instead of OpaqueValueExprs

2023-08-14 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet created this revision.
kadircet added reviewers: sammccall, aaron.ballman.
Herald added a subscriber: arphaman.
Herald added a project: All.
kadircet requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.

This makes sure we can preserve invalid-ness for consumers of this
node, it prevents crashes. It also aligns better with rest of the places that
store invalid expressions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157868

Files:
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/Index/complete-optional-params.cpp

Index: clang/test/Index/complete-optional-params.cpp
===
--- clang/test/Index/complete-optional-params.cpp
+++ clang/test/Index/complete-optional-params.cpp
@@ -79,7 +79,7 @@
 // CHECK-CC5-NEXT: Objective-C interface
 
 // RUN: c-index-test -code-completion-at=%s:17:11 %s | FileCheck -check-prefix=CHECK-CC6 %s
-// CHECK-CC6: FunctionDecl:{ResultType void}{TypedText foo_2}{LeftParen (}{Optional {Placeholder Bar1 b1 = Bar1()}{Optional {Comma , }{Placeholder Bar2 b2}}}{RightParen )} (50)
+// CHECK-CC6: FunctionDecl:{ResultType void}{TypedText foo_2}{LeftParen (}{Optional {Placeholder Bar1 b1 = Bar1()}{Optional {Comma , }{Placeholder Bar2 b2 = Bar2()}}}{RightParen )} (50)
 // CHECK-CC6: Completion contexts:
 // CHECK-CC6-NEXT: Any type
 // CHECK-CC6-NEXT: Any value
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -20,6 +20,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
+#include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/RecursiveASTVisitor.h"
@@ -37,11 +38,13 @@
 #include "clang/Sema/EnterExpressionEvaluationContext.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
+#include "clang/Sema/Ownership.h"
 #include "clang/Sema/ParsedTemplate.h"
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/ScopeInfo.h"
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Template.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
@@ -329,23 +332,16 @@
   ParmVarDecl *Param = cast(param);
   UnparsedDefaultArgLocs.erase(Param);
 
-  auto Fail = [&] {
-Param->setInvalidDecl();
-Param->setDefaultArg(new (Context) OpaqueValueExpr(
-EqualLoc, Param->getType().getNonReferenceType(), VK_PRValue));
-  };
-
   // Default arguments are only permitted in C++
   if (!getLangOpts().CPlusPlus) {
 Diag(EqualLoc, diag::err_param_default_argument)
   << DefaultArg->getSourceRange();
-return Fail();
+return ActOnParamDefaultArgumentError(param, EqualLoc, DefaultArg);
   }
 
   // Check for unexpanded parameter packs.
-  if (DiagnoseUnexpandedParameterPack(DefaultArg, UPPC_DefaultArgument)) {
-return Fail();
-  }
+  if (DiagnoseUnexpandedParameterPack(DefaultArg, UPPC_DefaultArgument))
+return ActOnParamDefaultArgumentError(param, EqualLoc, DefaultArg);
 
   // C++11 [dcl.fct.default]p3
   //   A default argument expression [...] shall not be specified for a
@@ -360,14 +356,14 @@
 
   ExprResult Result = ConvertParamDefaultArgument(Param, DefaultArg, EqualLoc);
   if (Result.isInvalid())
-return Fail();
+return ActOnParamDefaultArgumentError(param, EqualLoc, DefaultArg);
 
   DefaultArg = Result.getAs();
 
   // Check that the default argument is well-formed
   CheckDefaultArgumentVisitor DefaultArgChecker(*this, DefaultArg);
   if (DefaultArgChecker.Visit(DefaultArg))
-return Fail();
+return ActOnParamDefaultArgumentError(param, EqualLoc, DefaultArg);
 
   SetParamDefaultArgument(Param, DefaultArg, EqualLoc);
 }
@@ -389,16 +385,24 @@
 
 /// ActOnParamDefaultArgumentError - Parsing or semantic analysis of
 /// the default argument for the parameter param failed.
-void Sema::ActOnParamDefaultArgumentError(Decl *param,
-  SourceLocation EqualLoc) {
+void Sema::ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc,
+  ExprResult DefaultArg) {
   if (!param)
 return;
 
   ParmVarDecl *Param = cast(param);
   Param->setInvalidDecl();
   UnparsedDefaultArgLocs.erase(Param);
-  Param->setDefaultArg(new (Context) OpaqueValueExpr(
-  EqualLoc, Param->getType().getNonReferenceType(), VK_PRValue));
+  ExprResult RE;
+  if (DefaultArg.isUsable()) {
+RE = CreateRecoveryExpr(EqualLoc, DefaultArg.get()->getEndLoc(),
+{DefaultArg.get()},
+

[PATCH] D157080: [clangd] Symbol search includes parameter types for (member) functions

2023-08-14 Thread Florian Humblot via Phabricator via cfe-commits

florianhumblot abandoned this revision.
florianhumblot added a comment.

@sammccall I guess you're right, I'll look into getting a fix into vscode 
instead.


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

https://reviews.llvm.org/D157080

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

[PATCH] D157781: [clang] Add cleanup_function field to CleanupAttr json AST dump

2023-08-14 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman accepted this revision.
aaron.ballman 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/D157781/new/

https://reviews.llvm.org/D157781

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

[PATCH] D157775: [clang] Add aliasee field to AliasAttr json AST dump

2023-08-14 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman accepted this revision.
aaron.ballman 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/D157775/new/

https://reviews.llvm.org/D157775

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

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

2023-08-14 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D112921#4581641 , @rnk wrote:

> What are the next steps here? Have concerns been addressed? The CI failures 
> seem unrelated (libcxx tests is an AIX bot out of disk, clang CI is an 
> interpreter test that seems unrelated, but please confirm).

We definitely need the libc++ reviewers to approve before we move forward. IMO. 
the clang bits look to be in reasonable shape in terms of code quality.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112921

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

[PATCH] D157783: [clang] Add rmissing fields to DeprecatedAttr and UnavailableAttr json AST dump

2023-08-14 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/lib/AST/JSONNodeDumper.cpp:541-548
+void JSONNodeDumper::VisitDeprecatedAttr(const DeprecatedAttr *DA) {
+  JOS.attribute("message", DA->getMessage());
+  JOS.attribute("replacement", DA->getReplacement());
+}
+
+void JSONNodeDumper::VisitUnavailableAttr(const UnavailableAttr *UA) {
+  JOS.attribute("message", UA->getMessage());

I think we should probably skip emitting the field if the field is empty. e.g., 
`[[deprecated("")]]` and `[[deprecated]]` should have the same output that 
doesn't have a `message` field; this keeps the JSON dumps somewhat more 
succinct for those cases. WDYT?


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

https://reviews.llvm.org/D157783

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

< 1 2 3 >

101 - 200 of 251 matches

Mail list logo