[PATCH] D134705: [clang][DebugInfo] Emit debuginfo for non-constant case value

2022-09-28 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song added a comment.

In D134705#3818657 , @dblaikie wrote:

> SGTM - could you also add some CHECKs in the test that show the enumeration 
> was added to the enumerators list on the CU (& you can probably test with 
> just one enumerator - don't need two in each test for instance)

Okay, will do.

> & I'm not quite following - what's the difference between the two cases being 
> tested? Oh, I guess the second has implicit conversions in the case 
> parameters? Fair enough, maybe would benefit from some comments.

Yes, the first is without implicit conversion and the second is with implicit 
conversion. I will add some comments to clarify it in the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134705

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


[PATCH] D132017: [clang][analyzer] Add errno modeling to StreamChecker

2022-09-28 Thread Gabor Marton via Phabricator via cfe-commits
martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!




Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:465
+  /// post-call event.
+  class NoErrnoConstraint : public ErrnoConstraintBase {
+  public:

Makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132017

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


[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:236-239
+- Clang now supports loading multiple configuration files. The files from
+  default configuration paths are loaded first, unless ``--no-default-config``
+  option is used. All files explicitly specified using ``--config`` option
+  are loaded afterwards.

I would say this paragraph is a combination of two provided below. IMHO.



Comment at: clang/docs/ReleaseNotes.rst:240
+  are loaded afterwards.
+- Default configuration paths were partially changed. Clang now attempts to 
load
+  ``-.cfg`` first, and falls back to loading both

This paragraph is actually about two changes. One is about loading driver mode 
file, this is an extension. The other is about skipping files using original 
target prefix. It is potentially breaking change and is worth separate 
paragraph.



Comment at: clang/docs/UsersManual.rst:918
+First, the algorithm searches for a configuration file named
+``-.cfg`` where `target` is the triple for the target being
+built for, and `driver` is the name of the currently used driver. The algorithm

I would propose changing ``->`` and `target`->`triple` to 
reduce misunderstanding.



Comment at: clang/docs/UsersManual.rst:949-950
+If none of the aforementioned files are found, the driver will instead search
+for separate target and driver configuration files and attempt to load both.
+The former is named ``.cfg`` while the latter is named
+``.cfg``. Similarly to the previous variants, the canonical driver name

Maybe `target`->`triple`, ``->``?



Comment at: clang/docs/UsersManual.rst:964
+clang-g++.cfg
 
 The configuration file consists of command-line options specified on one or

I think we need to put something like:
It is not an error if any or both of these files are not found.



Comment at: clang/lib/Driver/Driver.cpp:1089
+
+  bool ModeSuffixUnique = !ClangNameParts.ModeSuffix.empty() &&
+  ClangNameParts.ModeSuffix != RealMode.str();

Can the variable name be better? It looks like it means that driver mode is 
replaced (overridden).



Comment at: clang/test/Driver/config-file3.c:33
+// RUN: ln -s %clang %t/testdmode/x86_64-unknown-linux-gnu-clang
+// RUN: echo > %t/testdmode/x86_64-unknown-linux-gnu-clang++.cfg
+// RUN: echo > %t/testdmode/x86_64-unknown-linux-gnu-clang-g++.cfg

mgorny wrote:
> pengfei wrote:
> > MaskRay wrote:
> > > Since %t is rebuilt. You can just use `touch`
> > `touch` may not work on Windows?
> Well, my experience tells me to avoid external commands when the goal can be 
> achieved via a builtin but `touch` is more popular in clang indeed.
IIRC, `shell` is not available on Windows.


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

https://reviews.llvm.org/D134337

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


[PATCH] D132249: [clang][analyzer] Add some functions to StreamChecker with errno modeling.

2022-09-28 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h:110
+/// \arg \c InvalE Expression that causes invalidation of \c errno.
+ProgramStateRef setErrnoStdMustBeChecked(ProgramStateRef State,
+ CheckerContext &C, const Expr 
*InvalE);

Why do you consider important the `Std` part in the naming?



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:880
+
+  // In the success case, do not change 'errno' and errno state.
+  StateFailed = errno_modeling::setErrnoForStdFailure(StateFailed, C,

Could you please elaborate in the comment why don't we have to use 
`errno_modeling::setErrnoForStdSuccess` here? 



Comment at: clang/test/Analysis/stream-error.c:246
+  int rc = fsetpos(F, &Pos);
+  clang_analyzer_eval(feof(F)); // expected-warning{{FALSE}}
+  if (rc) {

This might be `TRUE` as well isn't it?  Because only the "successful call to 
the fseek() function clears the end-of-file indicator". I'd expect this to be 
TRUE only in the block of `if (rc)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132249

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


[PATCH] D53847: [C++2a] P0634r3: Down with typename!

2022-09-28 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment.

In D53847#3819344 , @ayzhao wrote:

> add test and fix for P2092 

These changes look good to me, thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D53847

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


[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Michał Górny via Phabricator via cfe-commits
mgorny marked 4 inline comments as done.
mgorny added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:236-239
+- Clang now supports loading multiple configuration files. The files from
+  default configuration paths are loaded first, unless ``--no-default-config``
+  option is used. All files explicitly specified using ``--config`` option
+  are loaded afterwards.

sepavloff wrote:
> I would say this paragraph is a combination of two provided below. IMHO.
Yeah, it was intentional. I was thinking it would be cleaner to describe this 
in tandem but I won't insist. Just please confirm whether I should remove it or 
keep it.



Comment at: clang/docs/ReleaseNotes.rst:240
+  are loaded afterwards.
+- Default configuration paths were partially changed. Clang now attempts to 
load
+  ``-.cfg`` first, and falls back to loading both

sepavloff wrote:
> This paragraph is actually about two changes. One is about loading driver 
> mode file, this is an extension. The other is about skipping files using 
> original target prefix. It is potentially breaking change and is worth 
> separate paragraph.
Updated.



Comment at: clang/lib/Driver/Driver.cpp:1089
+
+  bool ModeSuffixUnique = !ClangNameParts.ModeSuffix.empty() &&
+  ClangNameParts.ModeSuffix != RealMode.str();

sepavloff wrote:
> Can the variable name be better? It looks like it means that driver mode is 
> replaced (overridden).
Can you suggest a better name? It's used to avoid searching for the same 
filename twice.



Comment at: clang/lib/Driver/Driver.cpp:6195
+
+  assert(false && "Unhandled Mode");
+  return "clang";

MaskRay wrote:
> MaskRay wrote:
> > If the switch covers all enum members, we typically use llvm_unreachable to 
> > work around a GCC -Wswitch warning.
> > Clang correctly knows all branches are covered and do not need 
> > `llvm_unreachable`
> `llvm_unreachable`
Right, sorry, forgot about it.


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

https://reviews.llvm.org/D134337

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


[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Michał Górny via Phabricator via cfe-commits
mgorny updated this revision to Diff 463455.
mgorny marked an inline comment as done.
mgorny added a comment.

Updated docs per @sepavloff 's suggestions.


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

https://reviews.llvm.org/D134337

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UsersManual.rst
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/config-file3.c

Index: clang/test/Driver/config-file3.c
===
--- clang/test/Driver/config-file3.c
+++ clang/test/Driver/config-file3.c
@@ -14,107 +14,181 @@
 // CHECK-REL: Configuration file: {{.*}}/workdir/cfg-1
 // CHECK-REL: -Wundefined-var-template
 
+//--- Config files are searched for in binary directory as well.
+//
+// RUN: mkdir %t/testbin
+// RUN: ln -s %clang %t/testbin/clang
+// RUN: echo "-Werror" > %t/testbin/aaa.cfg
+// RUN: %t/testbin/clang --config-system-dir= --config-user-dir= --config aaa.cfg -c -no-canonical-prefixes -### %s 2>&1 | FileCheck %s -check-prefix CHECK-BIN
+//
+// CHECK-BIN: Configuration file: {{.*}}/testbin/aaa.cfg
+// CHECK-BIN: -Werror
 
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg first.
+//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries x86_64-unknown-linux-gnu-clang++.cfg first.
 //
 // RUN: mkdir %t/testdmode
-// RUN: ln -s %clang %t/testdmode/qqq-clang-g++
-// RUN: echo "-Wundefined-func-template" > %t/testdmode/qqq-clang-g++.cfg
-// RUN: echo "-Werror" > %t/testdmode/qqq.cfg
-// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c -no-canonical-prefixes -### %s 2>&1 | FileCheck %s -check-prefix FULL-NAME
+// RUN: ln -s %clang %t/testdmode/i386-unknown-linux-gnu-clang-g++
+// RUN: ln -s %clang %t/testdmode/x86_64-unknown-linux-gnu-clang-g++
+// RUN: ln -s %clang %t/testdmode/x86_64-unknown-linux-gnu-clang
+// RUN: touch %t/testdmode/x86_64-unknown-linux-gnu-clang++.cfg
+// RUN: touch %t/testdmode/x86_64-unknown-linux-gnu-clang-g++.cfg
+// RUN: touch %t/testdmode/x86_64-unknown-linux-gnu-clang.cfg
+// RUN: touch %t/testdmode/x86_64-unknown-linux-gnu.cfg
+// RUN: touch %t/testdmode/i386-unknown-linux-gnu-clang++.cfg
+// RUN: touch %t/testdmode/i386-unknown-linux-gnu-clang-g++.cfg
+// RUN: touch %t/testdmode/i386-unknown-linux-gnu-clang.cfg
+// RUN: touch %t/testdmode/i386-unknown-linux-gnu.cfg
+// RUN: touch %t/testdmode/clang++.cfg
+// RUN: touch %t/testdmode/clang-g++.cfg
+// RUN: touch %t/testdmode/clang.cfg
+// RUN: %t/testdmode/x86_64-unknown-linux-gnu-clang-g++ --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1
 //
-// FULL-NAME: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
-// FULL-NAME: -Wundefined-func-template
-// FULL-NAME-NOT: -Werror
+// FULL1-NOT: Configuration file:
+// FULL1: Configuration file: {{.*}}/testdmode/x86_64-unknown-linux-gnu-clang++.cfg
+// FULL1-NOT: Configuration file:
+
+//--- -m32 overrides triple.
 //
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg even without -no-canonical-prefixes.
-// (As the clang executable and symlink are in different directories, this
-// requires specifying the path via --config-*-dir= though.)
+// RUN: %t/testdmode/x86_64-unknown-linux-gnu-clang-g++ -m32 --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1-I386
 //
-// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir=%t/testdmode -c -### %s 2>&1 | FileCheck %s -check-prefix SYMLINK
+// FULL1-I386-NOT: Configuration file:
+// FULL1-I386: Configuration file: {{.*}}/testdmode/i386-unknown-linux-gnu-clang++.cfg
+// FULL1-I386-NOT: Configuration file:
+
+//--- --target= also works for overriding triple.
+//
+// RUN: %t/testdmode/x86_64-unknown-linux-gnu-clang-g++ --target=i386-unknown-linux-gnu --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1-I386
+
+//--- With --target= + -m64, -m64 takes precedence.
+//
+// RUN: %t/testdmode/x86_64-unknown-linux-gnu-clang-g++ --target=i386-unknown-linux-gnu -m64 --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1
+
+//--- i386 prefix also works for 32-bit.
 //
-// SYMLINK: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
+// RUN: %t/testdmode/i386-unknown-linux-gnu-clang-g++ --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1-I386
+
+//--- i386 prefix + -m64 also works for 64-bit.
+//
+// RUN: %t/testdmode/i386-unknown-linux-gnu-clang-g++ -m64 --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1
 
 //--- File specified by --config is loaded after the one inferred from the executable.
 //
-// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir=%S/Inputs/config --config-user-dir

[PATCH] D134744: [clang][Interp] Implement rem opcode

2022-09-28 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 463456.
tbaeder marked an inline comment as done.

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

https://reviews.llvm.org/D134744

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Opcodes.td
  clang/test/AST/Interp/literals.cpp

Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -145,3 +145,25 @@
 
 #endif
 };
+
+namespace rem {
+  static_assert(2 % 2 == 0, "");
+  static_assert(2 % 1 == 0, "");
+  static_assert(-3 % 4 == -3, "");
+  static_assert(4 % -2 == 0, "");
+  static_assert(-3 % -4 == -3, "");
+
+  constexpr int zero() { return 0; }
+  static_assert(10 % zero() == 20, ""); // ref-error {{not an integral constant expression}} \
+// ref-note {{division by zero}} \
+// expected-error {{not an integral constant expression}} \
+// expected-note {{division by zero}}
+
+
+  static_assert(true % true == 0, "");
+  static_assert(false % true == 0, "");
+  static_assert(true % false == 10, ""); // ref-error {{not an integral constant expression}} \
+ // ref-note {{division by zero}} \
+ // expected-error {{not an integral constant expression}} \
+ // expected-note {{division by zero}}
+};
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -54,9 +54,13 @@
   list Types;
 }
 
-def AluTypeClass : TypeClass {
+def NumberTypeClass : TypeClass {
   let Types = [Sint8, Uint8, Sint16, Uint16, Sint32,
-   Uint32, Sint64, Uint64, Bool];
+   Uint32, Sint64, Uint64];
+}
+
+def AluTypeClass : TypeClass {
+  let Types = !listconcat(NumberTypeClass.Types, [Bool]);
 }
 
 def PtrTypeClass : TypeClass {
@@ -393,6 +397,10 @@
 def Sub : AluOpcode;
 def Add : AluOpcode;
 def Mul : AluOpcode;
+def Rem : Opcode {
+  let Types = [NumberTypeClass];
+  let HasGroup = 1;
+}
 
 
 //===--===//
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -153,6 +153,43 @@
   return AddSubMulHelper(S, OpPC, Bits, LHS, RHS);
 }
 
+/// 1) Pops the RHS from the stack.
+/// 2) Pops the LHS from the stack.
+/// 3) Pushes 'LHS % RHS' on the stack (the remainder of dividing LHS by RHS).
+template ::T>
+bool Rem(InterpState &S, CodePtr OpPC) {
+  const T &RHS = S.Stk.pop();
+  const T &LHS = S.Stk.pop();
+
+  if (RHS.isZero()) {
+const SourceInfo &Loc = S.Current->getSource(OpPC);
+S.FFDiag(Loc, diag::note_expr_divide_by_zero);
+return false;
+  }
+
+  if (LHS.isSigned() && LHS.isMin() && RHS.isNegative() && RHS.isMinusOne()) {
+APSInt LHSInt = LHS.toAPSInt();
+APSInt RHSInt = RHS.toAPSInt();
+APSInt Result = LHSInt % RHSInt;
+
+SmallString<32> Trunc;
+Result.toString(Trunc, 10);
+const SourceInfo &Loc = S.Current->getSource(OpPC);
+const Expr *E = S.Current->getExpr(OpPC);
+S.CCEDiag(Loc, diag::warn_integer_constant_overflow)
+<< Trunc << E->getType();
+return false;
+  }
+
+  const unsigned Bits = RHS.bitWidth() * 2;
+  T Result;
+  if (!T::rem(LHS, RHS, Bits, &Result)) {
+S.Stk.push(Result);
+return true;
+  }
+  return false;
+}
+
 //===--===//
 // Inv
 //===--===//
Index: clang/lib/AST/Interp/Integral.h
===
--- clang/lib/AST/Interp/Integral.h
+++ clang/lib/AST/Interp/Integral.h
@@ -210,6 +210,11 @@
 return CheckMulUB(A.V, B.V, R->V);
   }
 
+  static bool rem(Integral A, Integral B, unsigned OpBits, Integral *R) {
+*R = Integral(A.V % B.V);
+return false;
+  }
+
   static bool neg(Integral A, Integral *R) {
 *R = -A;
 return false;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -211,6 +211,8 @@
   return Discard(this->emitAdd(*T, BO));
 case BO_Mul:
   return Discard(this->emitMul(*T, BO));
+case BO_Rem:
+  return Discard(this->emitRem(*T, BO));
 case BO_Assign:
   if (!this->emitStore(*T, BO))
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lis

[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:236-239
+- Clang now supports loading multiple configuration files. The files from
+  default configuration paths are loaded first, unless ``--no-default-config``
+  option is used. All files explicitly specified using ``--config`` option
+  are loaded afterwards.

mgorny wrote:
> sepavloff wrote:
> > I would say this paragraph is a combination of two provided below. IMHO.
> Yeah, it was intentional. I was thinking it would be cleaner to describe this 
> in tandem but I won't insist. Just please confirm whether I should remove it 
> or keep it.
No, I don't insist. It is OK.



Comment at: clang/lib/Driver/Driver.cpp:1089
+
+  bool ModeSuffixUnique = !ClangNameParts.ModeSuffix.empty() &&
+  ClangNameParts.ModeSuffix != RealMode.str();

mgorny wrote:
> sepavloff wrote:
> > Can the variable name be better? It looks like it means that driver mode is 
> > replaced (overridden).
> Can you suggest a better name? It's used to avoid searching for the same 
> filename twice.
DriverModeReplaced? Or something like that.


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

https://reviews.llvm.org/D134337

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


[PATCH] D134797: [X86][vectorcall] Make floating-type passed by value to match with MSVC

2022-09-28 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei created this revision.
pengfei added a reviewer: rnk.
Herald added a project: All.
pengfei requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The passing format of floating-point types are different from vector when SSE 
registers exhausted.
They are passed indirectly by value rather than address. 
https://godbolt.org/z/Kbs89f36P


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134797

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/vectorcall.c


Index: clang/test/CodeGen/vectorcall.c
===
--- clang/test/CodeGen/vectorcall.c
+++ clang/test/CodeGen/vectorcall.c
@@ -140,4 +140,19 @@
 // X86-SAME: <4 x float>* inreg noundef %0,
 // X86-SAME: i32 inreg noundef %edx,
 // X86-SAME: <4 x float>* noundef %1)
+
+// The passing format of floating-point types are different from vector when 
SSE registers exhausted.
+// They are passed indirectly by value rather than address.
+void __vectorcall vectorcall_indirect_fp(
+double xmm0, double xmm1, double xmm2, double xmm3, double xmm4,
+v4f32 xmm5, double stack) {
+}
+// X86: define dso_local x86_vectorcallcc void 
@"\01vectorcall_indirect_fp@@{{[0-9]+}}"
+// X86-SAME: (double inreg noundef %xmm0,
+// X86-SAME: double inreg noundef %xmm1,
+// X86-SAME: double inreg noundef %xmm2,
+// X86-SAME: double inreg noundef %xmm3,
+// X86-SAME: double inreg noundef %xmm4,
+// X86-SAME: <4 x float> inreg noundef %xmm5,
+// X86-SAME: double* noundef byval(double) align 4 %0)
 #endif
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -1855,7 +1855,8 @@
 return ABIArgInfo::getDirect();
   return ABIArgInfo::getExpand();
 }
-return getIndirectResult(Ty, /*ByVal=*/false, State);
+bool ByVal = IsVectorCall && Ty->isFloatingType();
+return getIndirectResult(Ty, ByVal, State);
   }
 
   if (isAggregateTypeForABI(Ty)) {


Index: clang/test/CodeGen/vectorcall.c
===
--- clang/test/CodeGen/vectorcall.c
+++ clang/test/CodeGen/vectorcall.c
@@ -140,4 +140,19 @@
 // X86-SAME: <4 x float>* inreg noundef %0,
 // X86-SAME: i32 inreg noundef %edx,
 // X86-SAME: <4 x float>* noundef %1)
+
+// The passing format of floating-point types are different from vector when SSE registers exhausted.
+// They are passed indirectly by value rather than address.
+void __vectorcall vectorcall_indirect_fp(
+double xmm0, double xmm1, double xmm2, double xmm3, double xmm4,
+v4f32 xmm5, double stack) {
+}
+// X86: define dso_local x86_vectorcallcc void @"\01vectorcall_indirect_fp@@{{[0-9]+}}"
+// X86-SAME: (double inreg noundef %xmm0,
+// X86-SAME: double inreg noundef %xmm1,
+// X86-SAME: double inreg noundef %xmm2,
+// X86-SAME: double inreg noundef %xmm3,
+// X86-SAME: double inreg noundef %xmm4,
+// X86-SAME: <4 x float> inreg noundef %xmm5,
+// X86-SAME: double* noundef byval(double) align 4 %0)
 #endif
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -1855,7 +1855,8 @@
 return ABIArgInfo::getDirect();
   return ABIArgInfo::getExpand();
 }
-return getIndirectResult(Ty, /*ByVal=*/false, State);
+bool ByVal = IsVectorCall && Ty->isFloatingType();
+return getIndirectResult(Ty, ByVal, State);
   }
 
   if (isAggregateTypeForABI(Ty)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

2022-09-28 Thread David Spickett via Phabricator via cfe-commits
DavidSpickett added inline comments.



Comment at: clang/lib/Support/CMakeLists.txt:21
+  # libLLVM-*.so, to be used by clang-tblgen. This is so clang-tblgen doesn't
+  # accidentally link against libLLVMSupport twice (once statically and once 
via
+  # libLLVM-*.so).

Without this change, is it the case that it will always link against 
libLLVMSupport twice with the DYLIB conifg?

"accidentally" sounds like you could stumble into it but from what I see, it's 
always been doing this and once your other change lands, it would always result 
in an error.
```
This is so clang-tblgen doesn't link against libLLVMSupport twice (once 
statically and once via libLLVM-*.so).
```



Comment at: clang/lib/Support/CMakeLists.txt:26
+DISABLE_LLVM_LINK_LLVM_DYLIB
+${clangSupport_sources})
+endif()

Can you detail which targets link to/include what and how the problem happens? 
I'm trying to understand why we can't just use `DISABLE_LLVM_LINK_LLVM_DYLIB` 
on its own here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134637

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


[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Michał Górny via Phabricator via cfe-commits
mgorny marked 2 inline comments as done.
mgorny added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:1089
+
+  bool ModeSuffixUnique = !ClangNameParts.ModeSuffix.empty() &&
+  ClangNameParts.ModeSuffix != RealMode.str();

sepavloff wrote:
> mgorny wrote:
> > sepavloff wrote:
> > > Can the variable name be better? It looks like it means that driver mode 
> > > is replaced (overridden).
> > Can you suggest a better name? It's used to avoid searching for the same 
> > filename twice.
> DriverModeReplaced? Or something like that.
Ok, how about something more literal? `TryModeSuffix`.


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

https://reviews.llvm.org/D134337

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


[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Michał Górny via Phabricator via cfe-commits
mgorny updated this revision to Diff 463464.
mgorny added a comment.

Rename the bool to `TryModeSuffix`.


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

https://reviews.llvm.org/D134337

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UsersManual.rst
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/config-file3.c

Index: clang/test/Driver/config-file3.c
===
--- clang/test/Driver/config-file3.c
+++ clang/test/Driver/config-file3.c
@@ -14,107 +14,181 @@
 // CHECK-REL: Configuration file: {{.*}}/workdir/cfg-1
 // CHECK-REL: -Wundefined-var-template
 
+//--- Config files are searched for in binary directory as well.
+//
+// RUN: mkdir %t/testbin
+// RUN: ln -s %clang %t/testbin/clang
+// RUN: echo "-Werror" > %t/testbin/aaa.cfg
+// RUN: %t/testbin/clang --config-system-dir= --config-user-dir= --config aaa.cfg -c -no-canonical-prefixes -### %s 2>&1 | FileCheck %s -check-prefix CHECK-BIN
+//
+// CHECK-BIN: Configuration file: {{.*}}/testbin/aaa.cfg
+// CHECK-BIN: -Werror
 
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg first.
+//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries x86_64-unknown-linux-gnu-clang++.cfg first.
 //
 // RUN: mkdir %t/testdmode
-// RUN: ln -s %clang %t/testdmode/qqq-clang-g++
-// RUN: echo "-Wundefined-func-template" > %t/testdmode/qqq-clang-g++.cfg
-// RUN: echo "-Werror" > %t/testdmode/qqq.cfg
-// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c -no-canonical-prefixes -### %s 2>&1 | FileCheck %s -check-prefix FULL-NAME
+// RUN: ln -s %clang %t/testdmode/i386-unknown-linux-gnu-clang-g++
+// RUN: ln -s %clang %t/testdmode/x86_64-unknown-linux-gnu-clang-g++
+// RUN: ln -s %clang %t/testdmode/x86_64-unknown-linux-gnu-clang
+// RUN: touch %t/testdmode/x86_64-unknown-linux-gnu-clang++.cfg
+// RUN: touch %t/testdmode/x86_64-unknown-linux-gnu-clang-g++.cfg
+// RUN: touch %t/testdmode/x86_64-unknown-linux-gnu-clang.cfg
+// RUN: touch %t/testdmode/x86_64-unknown-linux-gnu.cfg
+// RUN: touch %t/testdmode/i386-unknown-linux-gnu-clang++.cfg
+// RUN: touch %t/testdmode/i386-unknown-linux-gnu-clang-g++.cfg
+// RUN: touch %t/testdmode/i386-unknown-linux-gnu-clang.cfg
+// RUN: touch %t/testdmode/i386-unknown-linux-gnu.cfg
+// RUN: touch %t/testdmode/clang++.cfg
+// RUN: touch %t/testdmode/clang-g++.cfg
+// RUN: touch %t/testdmode/clang.cfg
+// RUN: %t/testdmode/x86_64-unknown-linux-gnu-clang-g++ --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1
 //
-// FULL-NAME: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
-// FULL-NAME: -Wundefined-func-template
-// FULL-NAME-NOT: -Werror
+// FULL1-NOT: Configuration file:
+// FULL1: Configuration file: {{.*}}/testdmode/x86_64-unknown-linux-gnu-clang++.cfg
+// FULL1-NOT: Configuration file:
+
+//--- -m32 overrides triple.
 //
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg even without -no-canonical-prefixes.
-// (As the clang executable and symlink are in different directories, this
-// requires specifying the path via --config-*-dir= though.)
+// RUN: %t/testdmode/x86_64-unknown-linux-gnu-clang-g++ -m32 --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1-I386
 //
-// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir=%t/testdmode -c -### %s 2>&1 | FileCheck %s -check-prefix SYMLINK
+// FULL1-I386-NOT: Configuration file:
+// FULL1-I386: Configuration file: {{.*}}/testdmode/i386-unknown-linux-gnu-clang++.cfg
+// FULL1-I386-NOT: Configuration file:
+
+//--- --target= also works for overriding triple.
+//
+// RUN: %t/testdmode/x86_64-unknown-linux-gnu-clang-g++ --target=i386-unknown-linux-gnu --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1-I386
+
+//--- With --target= + -m64, -m64 takes precedence.
+//
+// RUN: %t/testdmode/x86_64-unknown-linux-gnu-clang-g++ --target=i386-unknown-linux-gnu -m64 --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1
+
+//--- i386 prefix also works for 32-bit.
 //
-// SYMLINK: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
+// RUN: %t/testdmode/i386-unknown-linux-gnu-clang-g++ --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1-I386
+
+//--- i386 prefix + -m64 also works for 64-bit.
+//
+// RUN: %t/testdmode/i386-unknown-linux-gnu-clang-g++ -m64 --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1
 
 //--- File specified by --config is loaded after the one inferred from the executable.
 //
-// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir=%S/Inputs/config --config-user-dir= --config i386-qqq.cfg -c -no-canonical-prefixes

[PATCH] D134454: [Driver][Distro] Fix ArchLinux sysroot detection

2022-09-28 Thread Adrian Ratiu via Phabricator via cfe-commits
10ne1 marked 5 inline comments as done.
10ne1 added a comment.

FYI: @MaskRay I think you will be very happy that after the simplifications 
Nick suggested the Distro::* additions are not necessary anymore.




Comment at: clang/lib/Driver/ToolChains/Linux.cpp:377-389
   const StringRef InstallDir = GCCInstallation.getInstallPath();
   const StringRef TripleStr = GCCInstallation.getTriple().str();
   const Multilib &Multilib = GCCInstallation.getMultilib();
+  std::string BasePath = (InstallDir + "/../../../../" + TripleStr).str();
 
-  std::string Path =
-  (InstallDir + "/../../../../" + TripleStr + "/libc" + 
Multilib.osSuffix())
-  .str();
-
-  if (getVFS().exists(Path))
-return Path;
+  if (Distro.IsArchLinux() && getVFS().exists(BasePath))
+return BasePath;

nickdesaulniers wrote:
> ...seems like some of this is duplicated with CSKY above. Can you please 
> refactor the MIPS and CSKY checks to reuse more code?
> 
> I doubt arch-linux has a CSKY port; I suspect you might be able to check for 
> arch-linux first, then do the CSKY and MIPS special casing after.
> 
> All this code is doing is figuring out a Path, then if it exists then return 
> it.  Feel like is should be:
> 
> ```
> if android:
>   path = ...
> elif arch:
>   path = ...
> elif csky:
>   path = ...
> elif mips:
>   path = ...
> else:
>   return ""
> 
> if path.exists():
>   return path
> return ""
> ```
Thanks for this suggestion. Indeed after doing all the simplifications I have 
some great news: we do not need to test if Distro == ArchLinux because the 
sysroot path it uses is the implicit base path one common to all architectures!

So just doing the following is enough to also fix ArchLinux:

```
  std::string Path = (InstallDir + "/../../../../" + TripleStr).str();
  
  if (getTriple().isCSKY())
Path = Path + "/libc";
  
  if (getTriple().isMIPS()) {
...
   }

  if (getVFS().exists(Path))
return Path;

  return std::string();
```


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

https://reviews.llvm.org/D134454

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


[PATCH] D134454: [Driver][Distro] Fix ArchLinux sysroot detection

2022-09-28 Thread Adrian Ratiu via Phabricator via cfe-commits
10ne1 updated this revision to Diff 463465.
10ne1 marked an inline comment as done.
10ne1 edited the summary of this revision.
Herald added subscribers: atanasyan, arichardson, sdardis.

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

https://reviews.llvm.org/D134454

Files:
  clang/lib/Driver/ToolChains/Linux.cpp


Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -355,40 +355,31 @@
   return AndroidSysRootPath;
   }
 
-  if (getTriple().isCSKY()) {
-// CSKY toolchains use different names for sysroot folder.
-if (!GCCInstallation.isValid())
-  return std::string();
-// GCCInstallation.getInstallPath() =
-//   $GCCToolchainPath/lib/gcc/csky-linux-gnuabiv2/6.3.0
-// Path = $GCCToolchainPath/csky-linux-gnuabiv2/libc
-std::string Path = (GCCInstallation.getInstallPath() + "/../../../../" +
-GCCInstallation.getTriple().str() + "/libc")
-   .str();
-if (getVFS().exists(Path))
-  return Path;
-return std::string();
-  }
-
-  if (!GCCInstallation.isValid() || !getTriple().isMIPS())
+  if (!GCCInstallation.isValid())
 return std::string();
 
-  // Standalone MIPS toolchains use different names for sysroot folder
-  // and put it into different places. Here we try to check some known
-  // variants.
-
   const StringRef InstallDir = GCCInstallation.getInstallPath();
   const StringRef TripleStr = GCCInstallation.getTriple().str();
-  const Multilib &Multilib = GCCInstallation.getMultilib();
+  std::string Path = (InstallDir + "/../../../../" + TripleStr).str();
 
-  std::string Path =
-  (InstallDir + "/../../../../" + TripleStr + "/libc" + 
Multilib.osSuffix())
-  .str();
+  // CSKY toolchains use different names for sysroot folder.
+  // GCCInstallation.getInstallPath() =
+  //   $GCCToolchainPath/lib/gcc/csky-linux-gnuabiv2/6.3.0
+  // Path = $GCCToolchainPath/csky-linux-gnuabiv2/libc
+  if (getTriple().isCSKY())
+Path = Path + "/libc";
 
-  if (getVFS().exists(Path))
-return Path;
+  // Standalone MIPS toolchains use different names for sysroot folder
+  // and put it into different places. Here we try to check some known
+  // variants.
+  if (getTriple().isMIPS()) {
+const Multilib &Multilib = GCCInstallation.getMultilib();
+Path = Path + "/libc" + Multilib.osSuffix();
+if (getVFS().exists(Path))
+  return Path;
 
-  Path = (InstallDir + "/../../../../sysroot" + Multilib.osSuffix()).str();
+Path = (InstallDir + "/../../../../sysroot" + Multilib.osSuffix()).str();
+  }
 
   if (getVFS().exists(Path))
 return Path;


Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -355,40 +355,31 @@
   return AndroidSysRootPath;
   }
 
-  if (getTriple().isCSKY()) {
-// CSKY toolchains use different names for sysroot folder.
-if (!GCCInstallation.isValid())
-  return std::string();
-// GCCInstallation.getInstallPath() =
-//   $GCCToolchainPath/lib/gcc/csky-linux-gnuabiv2/6.3.0
-// Path = $GCCToolchainPath/csky-linux-gnuabiv2/libc
-std::string Path = (GCCInstallation.getInstallPath() + "/../../../../" +
-GCCInstallation.getTriple().str() + "/libc")
-   .str();
-if (getVFS().exists(Path))
-  return Path;
-return std::string();
-  }
-
-  if (!GCCInstallation.isValid() || !getTriple().isMIPS())
+  if (!GCCInstallation.isValid())
 return std::string();
 
-  // Standalone MIPS toolchains use different names for sysroot folder
-  // and put it into different places. Here we try to check some known
-  // variants.
-
   const StringRef InstallDir = GCCInstallation.getInstallPath();
   const StringRef TripleStr = GCCInstallation.getTriple().str();
-  const Multilib &Multilib = GCCInstallation.getMultilib();
+  std::string Path = (InstallDir + "/../../../../" + TripleStr).str();
 
-  std::string Path =
-  (InstallDir + "/../../../../" + TripleStr + "/libc" + Multilib.osSuffix())
-  .str();
+  // CSKY toolchains use different names for sysroot folder.
+  // GCCInstallation.getInstallPath() =
+  //   $GCCToolchainPath/lib/gcc/csky-linux-gnuabiv2/6.3.0
+  // Path = $GCCToolchainPath/csky-linux-gnuabiv2/libc
+  if (getTriple().isCSKY())
+Path = Path + "/libc";
 
-  if (getVFS().exists(Path))
-return Path;
+  // Standalone MIPS toolchains use different names for sysroot folder
+  // and put it into different places. Here we try to check some known
+  // variants.
+  if (getTriple().isMIPS()) {
+const Multilib &Multilib = GCCInstallation.getMultilib();
+Path = Path + "/libc" + Multilib.osSuffix();
+if (getVFS().exists(Path))
+  return Path;
 
-  Path = (InstallDi

[PATCH] D134361: [clang][Interp] Fix copy constructors of structs with array members

2022-09-28 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134361

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


[PATCH] D134744: [clang][Interp] Implement rem opcode

2022-09-28 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 463467.

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

https://reviews.llvm.org/D134744

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Opcodes.td
  clang/test/AST/Interp/literals.cpp

Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -145,3 +145,25 @@
 
 #endif
 };
+
+namespace rem {
+  static_assert(2 % 2 == 0, "");
+  static_assert(2 % 1 == 0, "");
+  static_assert(-3 % 4 == -3, "");
+  static_assert(4 % -2 == 0, "");
+  static_assert(-3 % -4 == -3, "");
+
+  constexpr int zero() { return 0; }
+  static_assert(10 % zero() == 20, ""); // ref-error {{not an integral constant expression}} \
+// ref-note {{division by zero}} \
+// expected-error {{not an integral constant expression}} \
+// expected-note {{division by zero}}
+
+
+  static_assert(true % true == 0, "");
+  static_assert(false % true == 0, "");
+  static_assert(true % false == 10, ""); // ref-error {{not an integral constant expression}} \
+ // ref-note {{division by zero}} \
+ // expected-error {{not an integral constant expression}} \
+ // expected-note {{division by zero}}
+};
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -54,9 +54,13 @@
   list Types;
 }
 
-def AluTypeClass : TypeClass {
+def NumberTypeClass : TypeClass {
   let Types = [Sint8, Uint8, Sint16, Uint16, Sint32,
-   Uint32, Sint64, Uint64, Bool];
+   Uint32, Sint64, Uint64];
+}
+
+def AluTypeClass : TypeClass {
+  let Types = !listconcat(NumberTypeClass.Types, [Bool]);
 }
 
 def PtrTypeClass : TypeClass {
@@ -393,6 +397,10 @@
 def Sub : AluOpcode;
 def Add : AluOpcode;
 def Mul : AluOpcode;
+def Rem : Opcode {
+  let Types = [NumberTypeClass];
+  let HasGroup = 1;
+}
 
 
 //===--===//
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -153,6 +153,42 @@
   return AddSubMulHelper(S, OpPC, Bits, LHS, RHS);
 }
 
+/// 1) Pops the RHS from the stack.
+/// 2) Pops the LHS from the stack.
+/// 3) Pushes 'LHS % RHS' on the stack (the remainder of dividing LHS by RHS).
+template ::T>
+bool Rem(InterpState &S, CodePtr OpPC) {
+  const T &RHS = S.Stk.pop();
+  const T &LHS = S.Stk.pop();
+
+  if (RHS.isZero()) {
+const SourceInfo &Loc = S.Current->getSource(OpPC);
+S.FFDiag(Loc, diag::note_expr_divide_by_zero);
+return false;
+  }
+
+  if (LHS.isSigned() && LHS.isMin() && RHS.isNegative() && RHS.isMinusOne()) {
+APSInt LHSInt = LHS.toAPSInt();
+APSInt RHSInt = RHS.toAPSInt();
+APSInt Result = LHSInt % RHSInt;
+
+SmallString<32> Trunc;
+(-LHSInt.extend(LHSInt.getBitWidth() + 1)).toString(Trunc, 10);
+const SourceInfo &Loc = S.Current->getSource(OpPC);
+const Expr *E = S.Current->getExpr(OpPC);
+S.CCEDiag(Loc, diag::note_constexpr_overflow) << Trunc << E->getType();
+return false;
+  }
+
+  const unsigned Bits = RHS.bitWidth() * 2;
+  T Result;
+  if (!T::rem(LHS, RHS, Bits, &Result)) {
+S.Stk.push(Result);
+return true;
+  }
+  return false;
+}
+
 //===--===//
 // Inv
 //===--===//
Index: clang/lib/AST/Interp/Integral.h
===
--- clang/lib/AST/Interp/Integral.h
+++ clang/lib/AST/Interp/Integral.h
@@ -210,6 +210,11 @@
 return CheckMulUB(A.V, B.V, R->V);
   }
 
+  static bool rem(Integral A, Integral B, unsigned OpBits, Integral *R) {
+*R = Integral(A.V % B.V);
+return false;
+  }
+
   static bool neg(Integral A, Integral *R) {
 *R = -A;
 return false;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -211,6 +211,8 @@
   return Discard(this->emitAdd(*T, BO));
 case BO_Mul:
   return Discard(this->emitMul(*T, BO));
+case BO_Rem:
+  return Discard(this->emitRem(*T, BO));
 case BO_Assign:
   if (!this->emitStore(*T, BO))
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/ma

[PATCH] D134744: [clang][Interp] Implement rem opcode

2022-09-28 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/Interp.h:173
+APSInt RHSInt = RHS.toAPSInt();
+APSInt Result = LHSInt % RHSInt;
+

`Result`is unused in here now I guess. Also `RHSInt`


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

https://reviews.llvm.org/D134744

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


[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:1098
+  // Try loading separate config for the target (variants 3. and 4.)
+  CfgFileName = RealTriple.str() + ".cfg";
+  if (searchForFile(CfgFilePath, CfgFileSearchDirs, CfgFileName, getVFS()) &&

Loading driver-mode config should take place before loading target config, it 
allows to override default settings set by driver mode config in target config. 
Also a test is need for such case.



Comment at: clang/test/Driver/config-file3.c:27
 
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg 
first.
+//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries 
x86_64-unknown-linux-gnu-clang++.cfg first.
 //

Tests must check the case when target prefix is not a real triple as in the 
original test (qqq-clang).



Comment at: clang/test/Driver/config-file3.c:60
+//
+// RUN: %t/testdmode/x86_64-unknown-linux-gnu-clang-g++ 
--target=i386-unknown-linux-gnu --config-system-dir= --config-user-dir= 
-no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1-I386
+

We also need a test that checks that in the case of 
`--target=i386-unknown-linux-gnu` and absence of 
`i386-unknown-linux-gnu-clang++.cfg` clang does not load 
`x86_64-unknown-linux-gnu-clang-g++.cfg`.



Comment at: clang/test/Driver/config-file3.c:168-170
+// FULL4: Configuration file: {{.*}}/testdmode/x86_64-unknown-linux-gnu.cfg
+// FULL4-NOT: Configuration file:
+// FULL4: Configuration file: {{.*}}/testdmode/clang-g++.cfg

target and driver mode configs should be in reverse order.


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

https://reviews.llvm.org/D134337

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


[PATCH] D134744: [clang][Interp] Implement rem opcode

2022-09-28 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 463472.

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

https://reviews.llvm.org/D134744

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Opcodes.td
  clang/test/AST/Interp/literals.cpp

Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -145,3 +145,25 @@
 
 #endif
 };
+
+namespace rem {
+  static_assert(2 % 2 == 0, "");
+  static_assert(2 % 1 == 0, "");
+  static_assert(-3 % 4 == -3, "");
+  static_assert(4 % -2 == 0, "");
+  static_assert(-3 % -4 == -3, "");
+
+  constexpr int zero() { return 0; }
+  static_assert(10 % zero() == 20, ""); // ref-error {{not an integral constant expression}} \
+// ref-note {{division by zero}} \
+// expected-error {{not an integral constant expression}} \
+// expected-note {{division by zero}}
+
+
+  static_assert(true % true == 0, "");
+  static_assert(false % true == 0, "");
+  static_assert(true % false == 10, ""); // ref-error {{not an integral constant expression}} \
+ // ref-note {{division by zero}} \
+ // expected-error {{not an integral constant expression}} \
+ // expected-note {{division by zero}}
+};
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -54,9 +54,13 @@
   list Types;
 }
 
-def AluTypeClass : TypeClass {
+def NumberTypeClass : TypeClass {
   let Types = [Sint8, Uint8, Sint16, Uint16, Sint32,
-   Uint32, Sint64, Uint64, Bool];
+   Uint32, Sint64, Uint64];
+}
+
+def AluTypeClass : TypeClass {
+  let Types = !listconcat(NumberTypeClass.Types, [Bool]);
 }
 
 def PtrTypeClass : TypeClass {
@@ -393,6 +397,10 @@
 def Sub : AluOpcode;
 def Add : AluOpcode;
 def Mul : AluOpcode;
+def Rem : Opcode {
+  let Types = [NumberTypeClass];
+  let HasGroup = 1;
+}
 
 
 //===--===//
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -153,6 +153,39 @@
   return AddSubMulHelper(S, OpPC, Bits, LHS, RHS);
 }
 
+/// 1) Pops the RHS from the stack.
+/// 2) Pops the LHS from the stack.
+/// 3) Pushes 'LHS % RHS' on the stack (the remainder of dividing LHS by RHS).
+template ::T>
+bool Rem(InterpState &S, CodePtr OpPC) {
+  const T &RHS = S.Stk.pop();
+  const T &LHS = S.Stk.pop();
+
+  if (RHS.isZero()) {
+const SourceInfo &Loc = S.Current->getSource(OpPC);
+S.FFDiag(Loc, diag::note_expr_divide_by_zero);
+return false;
+  }
+
+  if (LHS.isSigned() && LHS.isMin() && RHS.isNegative() && RHS.isMinusOne()) {
+APSInt LHSInt = LHS.toAPSInt();
+SmallString<32> Trunc;
+(-LHSInt.extend(LHSInt.getBitWidth() + 1)).toString(Trunc, 10);
+const SourceInfo &Loc = S.Current->getSource(OpPC);
+const Expr *E = S.Current->getExpr(OpPC);
+S.CCEDiag(Loc, diag::note_constexpr_overflow) << Trunc << E->getType();
+return false;
+  }
+
+  const unsigned Bits = RHS.bitWidth() * 2;
+  T Result;
+  if (!T::rem(LHS, RHS, Bits, &Result)) {
+S.Stk.push(Result);
+return true;
+  }
+  return false;
+}
+
 //===--===//
 // Inv
 //===--===//
Index: clang/lib/AST/Interp/Integral.h
===
--- clang/lib/AST/Interp/Integral.h
+++ clang/lib/AST/Interp/Integral.h
@@ -210,6 +210,11 @@
 return CheckMulUB(A.V, B.V, R->V);
   }
 
+  static bool rem(Integral A, Integral B, unsigned OpBits, Integral *R) {
+*R = Integral(A.V % B.V);
+return false;
+  }
+
   static bool neg(Integral A, Integral *R) {
 *R = -A;
 return false;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -211,6 +211,8 @@
   return Discard(this->emitAdd(*T, BO));
 case BO_Mul:
   return Discard(this->emitMul(*T, BO));
+case BO_Rem:
+  return Discard(this->emitRem(*T, BO));
 case BO_Assign:
   if (!this->emitStore(*T, BO))
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson added inline comments.



Comment at: clang/test/Driver/config-file3.c:27
 
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg 
first.
+//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries 
x86_64-unknown-linux-gnu-clang++.cfg first.
 //

sepavloff wrote:
> Tests must check the case when target prefix is not a real triple as in the 
> original test (qqq-clang).
It would be quite important for me that this continues to work. I made use of 
that in the CheriBSD toolchain when creating [[ 
https://github.com/CTSRD-CHERI/cheribuild/blob/master/pycheribuild/projects/cross/llvm.py#L499
 | symlinked binaries to easily build for different ABIs]] such as 
`cheribsd-riscv64-hybrid-clang++` and `cheribsd-riscv64-purecap-clang-cpp`. It 
appears this previously only worked if the prefix did not start with a valid 
triple (which is why I put the OS before the architecture). I think it would 
also be nice if the whole prefix was checked even if it starts with a valid 
triple, but this does not need to be changed in this patch (haven't looked at 
it in detail so this might actually work).


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

https://reviews.llvm.org/D134337

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


[PATCH] D134453: Introduce the `AlwaysIncludeTypeForNonTypeTemplateArgument` into printing policy

2022-09-28 Thread Nenad Mikša via Phabricator via cfe-commits
DoDoENT added inline comments.



Comment at: clang/include/clang/AST/PrettyPrinter.h:307
+  /// decltype(s) will be printed as "S" if enabled and as 
"S<{1,2}>" if disabled,
+  /// regardless if PrintCanonicalTypes is enabled.
+  unsigned AlwaysIncludeTypeForNonTypeTemplateArgument : 1;

dblaikie wrote:
> DoDoENT wrote:
> > dblaikie wrote:
> > > DoDoENT wrote:
> > > > DoDoENT wrote:
> > > > > dblaikie wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > DoDoENT wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > dblaikie wrote:
> > > > > > > > > > DoDoENT wrote:
> > > > > > > > > > > dblaikie wrote:
> > > > > > > > > > > > What does `PrintCanonicalTypes` have to do with this? 
> > > > > > > > > > > > Does it overlap with this functionality in some way, 
> > > > > > > > > > > > but doesn't provide the functionality you want in 
> > > > > > > > > > > > particular?
> > > > > > > > > > > Thank you for the question. If you set the 
> > > > > > > > > > > `PrintCanonicalTypes` to `false`, the `S` 
> > > > > > > > > > > would be printed as `S` even without this 
> > > > > > > > > > > patch. However, if you set it to `true`, it will be 
> > > > > > > > > > > printed as `S<{1, 2}>`.
> > > > > > > > > > > 
> > > > > > > > > > > I don't fully understand why it does that, but it's quite 
> > > > > > > > > > > annoying.
> > > > > > > > > > > 
> > > > > > > > > > > For a better example, please take a look at the 
> > > > > > > > > > > `TemplateIdWithComplexFullTypeNTTP` unit tests that I've 
> > > > > > > > > > > added: if `PrintCanonicalTypes` is set to `true`, the 
> > > > > > > > > > > original print output of type is `NDArray > > > > > > > > > > {{{0}}}, {{{0}}}>`, and if set to `false` (which is 
> > > > > > > > > > > default), the output is `NDArray > > > > > > > > > > Width{{{0}}}, Channels{{{0}}}>` - so the NTTP type is 
> > > > > > > > > > > neither fully written nor fully omitted, which is weird.
> > > > > > > > > > > 
> > > > > > > > > > > As I said, I don't really understand the idea behind 
> > > > > > > > > > > `PrintCanonicalTypes`, but when my new 
> > > > > > > > > > > `AlwaysIncludeTypeForNonTypeTemplateArgument` is enabled, 
> > > > > > > > > > > you will get the full type printed, regardless of value 
> > > > > > > > > > > of `PrintCanonicalTypes` setting.
> > > > > > > > > > > 
> > > > > > > > > > Perhaps this might be more of a bug in PrintCanonicalTypes 
> > > > > > > > > > than something to add a separate flag for.
> > > > > > > > > > 
> > > > > > > > > > @aaron.ballman D2 for context here... 
> > > > > > > > > > 
> > > > > > > > > > Hmm, actually, just adding the top level `Height{{0}}, 
> > > > > > > > > > Width{{0}}, Channels{{0}}` is sufficient to make this code 
> > > > > > > > > > compile (whereas with the `{{{0}}}` it doesn't form a valid 
> > > > > > > > > > identifier.
> > > > > > > > > > 
> > > > > > > > > > So what's your use case for needing more explicitness than 
> > > > > > > > > > that top level? 
> > > > > > > > > > Perhaps this might be more of a bug in PrintCanonicalTypes 
> > > > > > > > > > than something to add a separate flag for.
> > > > > > > > > >
> > > > > > > > > > @aaron.ballman D2 for context here...
> > > > > > > > > 
> > > > > > > > > I looked over D2 again and haven't spotted anything with 
> > > > > > > > > it that seems amiss; the change there is to grab the 
> > > > > > > > > canonical type before trying to print it which is all the 
> > > > > > > > > more I'd expect `PrintCanonicalTypes` to impact.
> > > > > > > > > 
> > > > > > > > > This looks like the behavior you'd get when you desugar the 
> > > > > > > > > type. Check out the AST dump for `s`: 
> > > > > > > > > https://godbolt.org/z/vxh5j6qWr
> > > > > > > > > ```
> > > > > > > > > `-VarDecl  col:20 s 'S':'S<{1, 
> > > > > > > > > 2}>' callinit
> > > > > > > > > ```
> > > > > > > > > We generate that type information at 
> > > > > > > > > https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/TextNodeDumper.cpp#L663
> > > > > > > > >  for doing the AST dump, note how the second type printed is 
> > > > > > > > > the desugared type and that matches what we're seeing from 
> > > > > > > > > the pretty printer.
> > > > > > > > > So what's your use case for needing more explicitness than 
> > > > > > > > > that top level?
> > > > > > > > 
> > > > > > > > As I described in the [github 
> > > > > > > > issue](https://github.com/llvm/llvm-project/issues/57562), I'm 
> > > > > > > > trying to write a clang-based tool that will have different 
> > > > > > > > behavior if the printed `{{{0}}}` is actually `Width` than if 
> > > > > > > > its `Height` or anything else.
> > > > > > > > 
> > > > > > > > You can see the the issue in the AST dump for `bla`: 
> > > > > > > > https://godbolt.org/z/fMr4f13o3
> > > > > > > > 
> > > > > > > > The type is
> > > > > > > > ```
> > > > > > > > `-VarDecl  col:21 bla 'NDArray > > > > > > > W>':'NDArray' callinit
> > > 

[PATCH] D133092: [clang] fix generation of .debug_aranges with LTO

2022-09-28 Thread Azat Khuzhin via Phabricator via cfe-commits
azat added a comment.

Can someone make a final review please? Thanks in advance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133092

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


[PATCH] D134733: [clang-format][chore] transparent #include name regex

2022-09-28 Thread Konrad Wilhelm Kleine via Phabricator via cfe-commits
kwk added a comment.

@MyDeveloperDay please see my potentially uneducated comments.




Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:400
+llvm::Regex getCppIncludeRegex() {
+  static const char CppIncludeRegexPattern[] =
+  R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";

MyDeveloperDay wrote:
> What if this was overridable via the Style we could experiment with changing 
> the regex
Well, right now that was never a requirement and to be honest it isn't very 
worth exploring this because its not easy to determine which the correct 
matching group is for the include name.

For example, let's say you would allow this to be configurable from the config 
file and a user enters the regex from my previous patch (D121370):

```
^[\t\ ]*[@#][\t\ ]*(import|include)([^"/]*("[^"]+")|[^]+>)|[\t/\ 
]*([^;]+;))
```

How would you determine which is the correct matching group? I first thought 
about taking the last one matching but then I found that my regex was not able 
to cope with trailing comments. And even in my case there's more than one 
matching group for the include name. All in all I really don't see any value in 
outsourcing this to the config file is not something I see value in.

The only thing valuable would be to have a regex for multiple languages. Sorry 
if this is what you intended. I just wouldn't want to move this out to the 
config file for the end-user to play with. 



Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:407
+const llvm::SmallVectorImpl &Matches) {
+  if (Matches.size() >= 3) {
+return Matches[2];

MyDeveloperDay wrote:
> ‘>= 2’
@MyDeveloperDay correct me if I'm wrong but if an array has size `3` it has 
indexes `0`, `1` and `2`. And an array of size `2` only has `0` and `1`.  So 
the case `=2`, which is implied by your suggested`>=2`, is actually an out of 
bounds access when going `Matches[2]`.  Because that is effectively accessing 
the third element. The only valid change would be to check for `Matches.size() 
> 2` IMHO and that is the same as `Matches.size() >= 3`.

I must admit that I had to look at the regex a few times only to realize that 
it has two non-optional matching groups `(...)`. The third matching group at 
index `0` is the whole line. So in theory `>=3` isn't possible with the current 
regex. I wanted to give my best to have this logic "survive" a change to the 
regex in which for example something is added after the matching group of the 
include name; let's say a comment or something.

I hope I haven't made myself a complete fool.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134733

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


[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Michał Górny via Phabricator via cfe-commits
mgorny added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:1098
+  // Try loading separate config for the target (variants 3. and 4.)
+  CfgFileName = RealTriple.str() + ".cfg";
+  if (searchForFile(CfgFilePath, CfgFileSearchDirs, CfgFileName, getVFS()) &&

sepavloff wrote:
> Loading driver-mode config should take place before loading target config, it 
> allows to override default settings set by driver mode config in target 
> config. Also a test is need for such case.
Do you have any specific option in mind? The main idea was that you use full 
triple+driver config when you need overrides because — as we pretty much 
established before — the options accepted by different drivers are too 
different for them to be meaningfully used with a single config file shared 
between targets.



Comment at: clang/test/Driver/config-file3.c:27
 
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg 
first.
+//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries 
x86_64-unknown-linux-gnu-clang++.cfg first.
 //

arichardson wrote:
> sepavloff wrote:
> > Tests must check the case when target prefix is not a real triple as in the 
> > original test (qqq-clang).
> It would be quite important for me that this continues to work. I made use of 
> that in the CheriBSD toolchain when creating [[ 
> https://github.com/CTSRD-CHERI/cheribuild/blob/master/pycheribuild/projects/cross/llvm.py#L499
>  | symlinked binaries to easily build for different ABIs]] such as 
> `cheribsd-riscv64-hybrid-clang++` and `cheribsd-riscv64-purecap-clang-cpp`. 
> It appears this previously only worked if the prefix did not start with a 
> valid triple (which is why I put the OS before the architecture). I think it 
> would also be nice if the whole prefix was checked even if it starts with a 
> valid triple, but this does not need to be changed in this patch (haven't 
> looked at it in detail so this might actually work).
If the prefix is not a valid triple, then clang ignores it and uses host triple 
instead. And now we're back to square one. If I check both variants, it's too 
complex. If I don't, it's bad too.


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

https://reviews.llvm.org/D134337

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


[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

2022-09-28 Thread Nicolai Hähnle via Phabricator via cfe-commits
nhaehnle added inline comments.



Comment at: clang/lib/Support/CMakeLists.txt:21
+  # libLLVM-*.so, to be used by clang-tblgen. This is so clang-tblgen doesn't
+  # accidentally link against libLLVMSupport twice (once statically and once 
via
+  # libLLVM-*.so).

DavidSpickett wrote:
> Without this change, is it the case that it will always link against 
> libLLVMSupport twice with the DYLIB conifg?
> 
> "accidentally" sounds like you could stumble into it but from what I see, 
> it's always been doing this and once your other change lands, it would always 
> result in an error.
> ```
> This is so clang-tblgen doesn't link against libLLVMSupport twice (once 
> statically and once via libLLVM-*.so).
> ```
I meant "accidentally" in the sense that *-tblgen isn't supposed to link 
against libLLVM-*.so, but ended up doing so after clangSupport was added 
earlier this year. Perhaps I should just remover the adverb?



Comment at: clang/lib/Support/CMakeLists.txt:26
+DISABLE_LLVM_LINK_LLVM_DYLIB
+${clangSupport_sources})
+endif()

DavidSpickett wrote:
> Can you detail which targets link to/include what and how the problem 
> happens? I'm trying to understand why we can't just use 
> `DISABLE_LLVM_LINK_LLVM_DYLIB` on its own here.
clangSupport is included by clang-tblgen but also by libclangcpp. The 
underlying idea is that of all the users of clangSupport, clang-tblgen is 
special because it uses the DISABLE_LLVM_LINK_LLVM_DYLIB override.

clangSupport links against Support, which becomes a link against libLLVM-*.so 
with LLVM_LINK_LLVM_DYLIB=ON. So, in an LLVM_LINK_LLVM_DYLIB=ON build, we get 
with this change:

- clangSupport links against Support, which becomes dynamically linking against 
libLLVM-*.so (this is unchanged)
- clangSupport_tablegen links against Support statically
- clang-tblgen links against clangSupport_tablegen (and also directly against 
Support) statically
- other users of clangSupport link against clangSupport somehow, and then 
transitively dynamically against libLLVM-*.so

Does that answer your questions?

Specifically, if we were to just add DISABLE_LLVM_LINK_LLVM_DYLIB to 
clangSupport, then we risk a situation where some other user of clangSupport 
links against Support twice; once via the copy of Support that is statically 
linked to from clangSupport; and once via libLLVM-*.so that gets pulled in via 
other dependencies. To be honest, I don't know for certain whether that is a 
problem that would happen, but it seemed likely enough to me that I wouldn't 
want to risk it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134637

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


[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:1098
+  // Try loading separate config for the target (variants 3. and 4.)
+  CfgFileName = RealTriple.str() + ".cfg";
+  if (searchForFile(CfgFilePath, CfgFileSearchDirs, CfgFileName, getVFS()) &&

mgorny wrote:
> sepavloff wrote:
> > Loading driver-mode config should take place before loading target config, 
> > it allows to override default settings set by driver mode config in target 
> > config. Also a test is need for such case.
> Do you have any specific option in mind? The main idea was that you use full 
> triple+driver config when you need overrides because — as we pretty much 
> established before — the options accepted by different drivers are too 
> different for them to be meaningfully used with a single config file shared 
> between targets.
I mean we can have several driver-mode configs like `tool.cfg`, where some 
default settings are set (maybe by using @file constructs`) and several 
`target.cfg` which may override options defined in `tool.cfg`. Due to different 
target and driver configs loaded separately, we can avoid creating separate 
config for each target+driver combination. Not a killer feature, but it is 
convenient.

As I understand, it is enough to swap code that tries loading driver config 
below with this piece of code, no?



Comment at: clang/test/Driver/config-file3.c:27
 
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg 
first.
+//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries 
x86_64-unknown-linux-gnu-clang++.cfg first.
 //

mgorny wrote:
> arichardson wrote:
> > sepavloff wrote:
> > > Tests must check the case when target prefix is not a real triple as in 
> > > the original test (qqq-clang).
> > It would be quite important for me that this continues to work. I made use 
> > of that in the CheriBSD toolchain when creating [[ 
> > https://github.com/CTSRD-CHERI/cheribuild/blob/master/pycheribuild/projects/cross/llvm.py#L499
> >  | symlinked binaries to easily build for different ABIs]] such as 
> > `cheribsd-riscv64-hybrid-clang++` and `cheribsd-riscv64-purecap-clang-cpp`. 
> > It appears this previously only worked if the prefix did not start with a 
> > valid triple (which is why I put the OS before the architecture). I think 
> > it would also be nice if the whole prefix was checked even if it starts 
> > with a valid triple, but this does not need to be changed in this patch 
> > (haven't looked at it in detail so this might actually work).
> If the prefix is not a valid triple, then clang ignores it and uses host 
> triple instead. And now we're back to square one. If I check both variants, 
> it's too complex. If I don't, it's bad too.
Our customers use this feature. Target prefix may designate, for example, debug 
build or build with specific framework.


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

https://reviews.llvm.org/D134337

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


[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

2022-09-28 Thread David Spickett via Phabricator via cfe-commits
DavidSpickett added a comment.

+1 from me also, but someone else should check that this is a reasonable way to 
implement it cmake wise (not that this is a horrible hack but I never can tell 
with cmake).

One more question, does this same issue potentially apply to llvm-tblgen and 
has that got any special cmake changes to account for it? (if it doesn't, leave 
it as is, but as a comparison point)




Comment at: clang/lib/Support/CMakeLists.txt:21
+  # libLLVM-*.so, to be used by clang-tblgen. This is so clang-tblgen doesn't
+  # accidentally link against libLLVMSupport twice (once statically and once 
via
+  # libLLVM-*.so).

nhaehnle wrote:
> DavidSpickett wrote:
> > Without this change, is it the case that it will always link against 
> > libLLVMSupport twice with the DYLIB conifg?
> > 
> > "accidentally" sounds like you could stumble into it but from what I see, 
> > it's always been doing this and once your other change lands, it would 
> > always result in an error.
> > ```
> > This is so clang-tblgen doesn't link against libLLVMSupport twice (once 
> > statically and once via libLLVM-*.so).
> > ```
> I meant "accidentally" in the sense that *-tblgen isn't supposed to link 
> against libLLVM-*.so, but ended up doing so after clangSupport was added 
> earlier this year. Perhaps I should just remover the adverb?
That would work, otherwise it seems like a thing that sometimes happens under 
conditions that aren't explained.



Comment at: clang/lib/Support/CMakeLists.txt:26
+DISABLE_LLVM_LINK_LLVM_DYLIB
+${clangSupport_sources})
+endif()

nhaehnle wrote:
> DavidSpickett wrote:
> > Can you detail which targets link to/include what and how the problem 
> > happens? I'm trying to understand why we can't just use 
> > `DISABLE_LLVM_LINK_LLVM_DYLIB` on its own here.
> clangSupport is included by clang-tblgen but also by libclangcpp. The 
> underlying idea is that of all the users of clangSupport, clang-tblgen is 
> special because it uses the DISABLE_LLVM_LINK_LLVM_DYLIB override.
> 
> clangSupport links against Support, which becomes a link against libLLVM-*.so 
> with LLVM_LINK_LLVM_DYLIB=ON. So, in an LLVM_LINK_LLVM_DYLIB=ON build, we get 
> with this change:
> 
> - clangSupport links against Support, which becomes dynamically linking 
> against libLLVM-*.so (this is unchanged)
> - clangSupport_tablegen links against Support statically
> - clang-tblgen links against clangSupport_tablegen (and also directly against 
> Support) statically
> - other users of clangSupport link against clangSupport somehow, and then 
> transitively dynamically against libLLVM-*.so
> 
> Does that answer your questions?
> 
> Specifically, if we were to just add DISABLE_LLVM_LINK_LLVM_DYLIB to 
> clangSupport, then we risk a situation where some other user of clangSupport 
> links against Support twice; once via the copy of Support that is statically 
> linked to from clangSupport; and once via libLLVM-*.so that gets pulled in 
> via other dependencies. To be honest, I don't know for certain whether that 
> is a problem that would happen, but it seemed likely enough to me that I 
> wouldn't want to risk it.
> Does that answer your questions?

Yes but I don't think I have the expertise to say this is a good way to 
implement this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134637

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


[PATCH] D134801: [clang][Interp] Implement ConditionalOperators

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

Implement visiting `ConditionalOperator`s.

This also makes it possible to enable the first existing test case with the new 
constant interpreter. Although I'm not sure if I should actually do that, I 
quite enjoy just running `ninja check-clang-ast-interp` and run all the 
relevant tests in under 2 seconds.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134801

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/test/AST/Interp/literals.cpp
  clang/test/SemaCXX/constexpr-factorial.cpp

Index: clang/test/SemaCXX/constexpr-factorial.cpp
===
--- clang/test/SemaCXX/constexpr-factorial.cpp
+++ clang/test/SemaCXX/constexpr-factorial.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fexperimental-new-constant-interpreter %s
 
 constexpr unsigned oddfac(unsigned n) {
   return n == 1 ? 1 : n * oddfac(n-2);
Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -183,3 +183,51 @@
   constexpr long unsigned RHS = 3;
   static_assert(LHS / RHS == 4, "");
 };
+
+namespace cond {
+  constexpr bool isEven(int n) {
+return n % 2 == 0 ? true : false;
+  }
+  static_assert(isEven(2), "");
+  static_assert(!isEven(3), "");
+  static_assert(isEven(100), "");
+
+  constexpr int M = 5 ? 10 : 20;
+  static_assert(M == 10, "");
+
+  static_assert(5 ? 13 : 16 == 13, "");
+  static_assert(0 ? 13 : 16 == 16, "");
+
+#if __cplusplus >= 201402L
+  constexpr int N = 20;
+  constexpr int foo() {
+int m = N > 0 ? 5 : 10;
+
+return m == 5 ? isEven(m) : true;
+  }
+  static_assert(foo() == false, "");
+
+  constexpr int dontCallMe(unsigned m) {
+if (m == 0) return 0;
+return dontCallMe(m - 2);
+  }
+
+  // Can't call this because it will run into infinite recursion.
+  constexpr int assertNotReached() {
+return dontCallMe(3);
+  }
+
+  constexpr int testCond() {
+return true ? 5 : assertNotReached();
+  }
+
+  constexpr int testCond2() {
+return false ? assertNotReached() : 10;
+  }
+
+  static_assert(testCond() == 5, "");
+  static_assert(testCond2() == 10, "");
+
+#endif
+
+};
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -87,6 +87,7 @@
   bool VisitMemberExpr(const MemberExpr *E);
   bool VisitArrayInitIndexExpr(const ArrayInitIndexExpr *E);
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E);
+  bool VisitConditionalOperator(const ConditionalOperator *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -344,6 +344,37 @@
   return this->visit(E->getSourceExpr());
 }
 
+template 
+bool ByteCodeExprGen::VisitConditionalOperator(
+const ConditionalOperator *E) {
+  const Expr *Condition = E->getCond();
+  const Expr *TrueExpr = E->getTrueExpr();
+  const Expr *FalseExpr = E->getFalseExpr();
+
+  LabelTy LabelEnd = this->getLabel();   // Label after the operator.
+  LabelTy LabelFalse = this->getLabel(); // Label for the false expr.
+
+  if (!this->visit(Condition))
+return false;
+  if (!this->jumpFalse(LabelFalse))
+return false;
+
+  if (!this->visit(TrueExpr))
+return false;
+  if (!this->jump(LabelEnd))
+return false;
+
+  this->emitLabel(LabelFalse);
+
+  if (!this->visit(FalseExpr))
+return false;
+
+  this->fallthrough(LabelEnd);
+  this->emitLabel(LabelEnd);
+
+  return true;
+}
+
 template  bool ByteCodeExprGen::discard(const Expr *E) {
   OptionScope Scope(this, /*NewDiscardResult=*/true);
   return this->Visit(E);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Michał Górny via Phabricator via cfe-commits
mgorny added inline comments.



Comment at: clang/test/Driver/config-file3.c:27
 
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg 
first.
+//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries 
x86_64-unknown-linux-gnu-clang++.cfg first.
 //

sepavloff wrote:
> mgorny wrote:
> > arichardson wrote:
> > > sepavloff wrote:
> > > > Tests must check the case when target prefix is not a real triple as in 
> > > > the original test (qqq-clang).
> > > It would be quite important for me that this continues to work. I made 
> > > use of that in the CheriBSD toolchain when creating [[ 
> > > https://github.com/CTSRD-CHERI/cheribuild/blob/master/pycheribuild/projects/cross/llvm.py#L499
> > >  | symlinked binaries to easily build for different ABIs]] such as 
> > > `cheribsd-riscv64-hybrid-clang++` and 
> > > `cheribsd-riscv64-purecap-clang-cpp`. It appears this previously only 
> > > worked if the prefix did not start with a valid triple (which is why I 
> > > put the OS before the architecture). I think it would also be nice if the 
> > > whole prefix was checked even if it starts with a valid triple, but this 
> > > does not need to be changed in this patch (haven't looked at it in detail 
> > > so this might actually work).
> > If the prefix is not a valid triple, then clang ignores it and uses host 
> > triple instead. And now we're back to square one. If I check both variants, 
> > it's too complex. If I don't, it's bad too.
> Our customers use this feature. Target prefix may designate, for example, 
> debug build or build with specific framework.
How are we supposed to avoid the "absurd" case where x86_64 configs are loaded 
for `-m32` invocation then?


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

https://reviews.llvm.org/D134337

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


[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

2022-09-28 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a reviewer: beanz.
mstorsjo added a subscriber: beanz.
mstorsjo added a comment.

Adding @beanz who might have some more cmake knowledge on whether this is the 
best/least bad way of doing things.




Comment at: clang/lib/Support/CMakeLists.txt:26
+DISABLE_LLVM_LINK_LLVM_DYLIB
+${clangSupport_sources})
+endif()

DavidSpickett wrote:
> nhaehnle wrote:
> > DavidSpickett wrote:
> > > Can you detail which targets link to/include what and how the problem 
> > > happens? I'm trying to understand why we can't just use 
> > > `DISABLE_LLVM_LINK_LLVM_DYLIB` on its own here.
> > clangSupport is included by clang-tblgen but also by libclangcpp. The 
> > underlying idea is that of all the users of clangSupport, clang-tblgen is 
> > special because it uses the DISABLE_LLVM_LINK_LLVM_DYLIB override.
> > 
> > clangSupport links against Support, which becomes a link against 
> > libLLVM-*.so with LLVM_LINK_LLVM_DYLIB=ON. So, in an 
> > LLVM_LINK_LLVM_DYLIB=ON build, we get with this change:
> > 
> > - clangSupport links against Support, which becomes dynamically linking 
> > against libLLVM-*.so (this is unchanged)
> > - clangSupport_tablegen links against Support statically
> > - clang-tblgen links against clangSupport_tablegen (and also directly 
> > against Support) statically
> > - other users of clangSupport link against clangSupport somehow, and then 
> > transitively dynamically against libLLVM-*.so
> > 
> > Does that answer your questions?
> > 
> > Specifically, if we were to just add DISABLE_LLVM_LINK_LLVM_DYLIB to 
> > clangSupport, then we risk a situation where some other user of 
> > clangSupport links against Support twice; once via the copy of Support that 
> > is statically linked to from clangSupport; and once via libLLVM-*.so that 
> > gets pulled in via other dependencies. To be honest, I don't know for 
> > certain whether that is a problem that would happen, but it seemed likely 
> > enough to me that I wouldn't want to risk it.
> > Does that answer your questions?
> 
> Yes but I don't think I have the expertise to say this is a good way to 
> implement this change.
FWIW I did try to implement something similar for some of the MLIR tablegen 
tools too, and I ended up doing something very similar (splitting support 
libraries into a regular and static-only variant, but for a deeper hierarcy of 
libraries) - but I haven't taken time to try to upstream that yet. If we get 
this as generic pattern for how to do it, I could try to upstream those patches 
later too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134637

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


[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added inline comments.



Comment at: clang/test/Driver/config-file3.c:27
 
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg 
first.
+//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries 
x86_64-unknown-linux-gnu-clang++.cfg first.
 //

mgorny wrote:
> sepavloff wrote:
> > mgorny wrote:
> > > arichardson wrote:
> > > > sepavloff wrote:
> > > > > Tests must check the case when target prefix is not a real triple as 
> > > > > in the original test (qqq-clang).
> > > > It would be quite important for me that this continues to work. I made 
> > > > use of that in the CheriBSD toolchain when creating [[ 
> > > > https://github.com/CTSRD-CHERI/cheribuild/blob/master/pycheribuild/projects/cross/llvm.py#L499
> > > >  | symlinked binaries to easily build for different ABIs]] such as 
> > > > `cheribsd-riscv64-hybrid-clang++` and 
> > > > `cheribsd-riscv64-purecap-clang-cpp`. It appears this previously only 
> > > > worked if the prefix did not start with a valid triple (which is why I 
> > > > put the OS before the architecture). I think it would also be nice if 
> > > > the whole prefix was checked even if it starts with a valid triple, but 
> > > > this does not need to be changed in this patch (haven't looked at it in 
> > > > detail so this might actually work).
> > > If the prefix is not a valid triple, then clang ignores it and uses host 
> > > triple instead. And now we're back to square one. If I check both 
> > > variants, it's too complex. If I don't, it's bad too.
> > Our customers use this feature. Target prefix may designate, for example, 
> > debug build or build with specific framework.
> How are we supposed to avoid the "absurd" case where x86_64 configs are 
> loaded for `-m32` invocation then?
In this case overloading target does not makes sense. You need to analyze 
`RealTriple` in `Driver::loadDefaultConfigFiles` and if it is wrong, use target 
prefix as if it is real target.


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

https://reviews.llvm.org/D134337

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


[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Michał Górny via Phabricator via cfe-commits
mgorny added inline comments.



Comment at: clang/test/Driver/config-file3.c:27
 
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg 
first.
+//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries 
x86_64-unknown-linux-gnu-clang++.cfg first.
 //

sepavloff wrote:
> mgorny wrote:
> > sepavloff wrote:
> > > mgorny wrote:
> > > > arichardson wrote:
> > > > > sepavloff wrote:
> > > > > > Tests must check the case when target prefix is not a real triple 
> > > > > > as in the original test (qqq-clang).
> > > > > It would be quite important for me that this continues to work. I 
> > > > > made use of that in the CheriBSD toolchain when creating [[ 
> > > > > https://github.com/CTSRD-CHERI/cheribuild/blob/master/pycheribuild/projects/cross/llvm.py#L499
> > > > >  | symlinked binaries to easily build for different ABIs]] such as 
> > > > > `cheribsd-riscv64-hybrid-clang++` and 
> > > > > `cheribsd-riscv64-purecap-clang-cpp`. It appears this previously only 
> > > > > worked if the prefix did not start with a valid triple (which is why 
> > > > > I put the OS before the architecture). I think it would also be nice 
> > > > > if the whole prefix was checked even if it starts with a valid 
> > > > > triple, but this does not need to be changed in this patch (haven't 
> > > > > looked at it in detail so this might actually work).
> > > > If the prefix is not a valid triple, then clang ignores it and uses 
> > > > host triple instead. And now we're back to square one. If I check both 
> > > > variants, it's too complex. If I don't, it's bad too.
> > > Our customers use this feature. Target prefix may designate, for example, 
> > > debug build or build with specific framework.
> > How are we supposed to avoid the "absurd" case where x86_64 configs are 
> > loaded for `-m32` invocation then?
> In this case overloading target does not makes sense. You need to analyze 
> `RealTriple` in `Driver::loadDefaultConfigFiles` and if it is wrong, use 
> target prefix as if it is real target.
What do you mean by "wrong"? The target triple is always a correct triple.


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

https://reviews.llvm.org/D134337

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


[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

2022-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added reviewers: phosek, Ericson2314.
aaron.ballman added a comment.

Adding the CMake code owners as reviewers for input on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134637

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


[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added inline comments.



Comment at: clang/test/Driver/config-file3.c:27
 
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg 
first.
+//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries 
x86_64-unknown-linux-gnu-clang++.cfg first.
 //

mgorny wrote:
> sepavloff wrote:
> > mgorny wrote:
> > > sepavloff wrote:
> > > > mgorny wrote:
> > > > > arichardson wrote:
> > > > > > sepavloff wrote:
> > > > > > > Tests must check the case when target prefix is not a real triple 
> > > > > > > as in the original test (qqq-clang).
> > > > > > It would be quite important for me that this continues to work. I 
> > > > > > made use of that in the CheriBSD toolchain when creating [[ 
> > > > > > https://github.com/CTSRD-CHERI/cheribuild/blob/master/pycheribuild/projects/cross/llvm.py#L499
> > > > > >  | symlinked binaries to easily build for different ABIs]] such as 
> > > > > > `cheribsd-riscv64-hybrid-clang++` and 
> > > > > > `cheribsd-riscv64-purecap-clang-cpp`. It appears this previously 
> > > > > > only worked if the prefix did not start with a valid triple (which 
> > > > > > is why I put the OS before the architecture). I think it would also 
> > > > > > be nice if the whole prefix was checked even if it starts with a 
> > > > > > valid triple, but this does not need to be changed in this patch 
> > > > > > (haven't looked at it in detail so this might actually work).
> > > > > If the prefix is not a valid triple, then clang ignores it and uses 
> > > > > host triple instead. And now we're back to square one. If I check 
> > > > > both variants, it's too complex. If I don't, it's bad too.
> > > > Our customers use this feature. Target prefix may designate, for 
> > > > example, debug build or build with specific framework.
> > > How are we supposed to avoid the "absurd" case where x86_64 configs are 
> > > loaded for `-m32` invocation then?
> > In this case overloading target does not makes sense. You need to analyze 
> > `RealTriple` in `Driver::loadDefaultConfigFiles` and if it is wrong, use 
> > target prefix as if it is real target.
> What do you mean by "wrong"? The target triple is always a correct triple.
Not triple, sorry. Target prefix taken from `ClangNameParts.TargetPrefix`.


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

https://reviews.llvm.org/D134337

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


[PATCH] D134652: [clang-format] Add Basic Carbon Support/Infrastructure to clang-format

2022-09-28 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 463502.
MyDeveloperDay retitled this revision from "[clang-format] Add Really Basic 
Carbon Support/Infrastructure" to "[clang-format] Add Basic Carbon 
Support/Infrastructure to clang-format".
MyDeveloperDay added a comment.

Improve the handling of generics.
Ensure match is formatted like switch.
Move closer to Google style.
Add some pointer support for return types
Add additional integration


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

https://reviews.llvm.org/D134652

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/tools/clang-format/clang-format-diff.py
  clang/tools/clang-format/git-clang-format
  clang/unittests/Format/CMakeLists.txt
  clang/unittests/Format/FormatTestCarbon.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1045,6 +1045,97 @@
   EXPECT_TOKEN(Tokens[9], tok::colon, TT_GotoLabelColon);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsCarbonReturnType) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_Carbon);
+
+  auto Tokens = annotate("fn Sum(var a: i32, var b: i32) -> i32 {", Style);
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[13], tok::arrow, TT_TrailingReturnArrow);
+  EXPECT_TOKEN(Tokens[15], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("fn Add[me: Self](k: i32) -> Self {", Style);
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[12], tok::arrow, TT_TrailingReturnArrow);
+  EXPECT_TOKEN(Tokens[14], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("fn Add[me: Self](k: i32) -> Self;", Style);
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[12], tok::arrow, TT_TrailingReturnArrow);
+  EXPECT_TOKEN(Tokens[14], tok::semi, TT_Unknown);
+
+  Tokens = annotate("fn Add[me: Self](k: i32) -> i32*;", Style);
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[12], tok::arrow, TT_TrailingReturnArrow);
+  EXPECT_TOKEN(Tokens[15], tok::semi, TT_Unknown);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsCarbonTypeOnColon) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_Carbon);
+
+  auto Tokens = annotate("let id: i32 = 57;", Style);
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::colon, TT_JsTypeColon);
+
+  Tokens = annotate("var t2: {.x: i32, .y: i32} = {.x = 2, .y = 5};", Style);
+  ASSERT_EQ(Tokens.size(), 28u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::colon, TT_JsTypeColon);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsCarbonPointerTypeOnColon) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_Carbon);
+  auto Tokens = annotate("fn GetSetX[addr me: Point*](x: i32) -> i32 {", Style);
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::colon, TT_JsTypeColon);
+  EXPECT_TOKEN(Tokens[7], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate(
+  "fn GetSetX[addr me: Point*, addr me: Point*](x: i32) -> i32 {", Style);
+  ASSERT_EQ(Tokens.size(), 24u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::colon, TT_JsTypeColon);
+  EXPECT_TOKEN(Tokens[7], tok::star, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[11], tok::colon, TT_JsTypeColon);
+  EXPECT_TOKEN(Tokens[13], tok::star, TT_PointerOrReference);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsCarbonTypeOnColonForLoop) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_Carbon);
+
+  auto Tokens = annotate("for (var name: String in names) {", Style);
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::colon, TT_JsTypeColon);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsCarbonTypeExclaim) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_Carbon);
+
+  auto Tokens = annotate("(T:! Addable)", Style);
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::colon, TT_JsTypeColon);
+  EXPECT_TOKEN(Tokens[3], tok::exclaim, TT_UnaryOperator);
+  ASSERT_EQ(Tokens[3]->SpacesRequiredBefore, false);
+  EXPECT_TOKEN(Tokens[4], tok::identifier, TT_Unknown);
+  ASSERT_EQ(Tokens[4]->SpacesRequiredBefore, true);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsCarbonClassTemplate) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_Carbon);
+
+  auto Tokens = annotate("class GenericClass(T:! Type) {", Style);
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_Unknown);
+  ASSERT_EQ(Tokens[2]->SpacesRequiredBefore, false);
+  ASSERT_EQ(Tokens[2]->MustBreakBefore, false);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsCarbonMatch) {
+  FormatStyle Style = getGoogleStyle(Forma

[PATCH] D134749: [clang][Interp] Implement Div opcode

2022-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/test/AST/Interp/literals.cpp:181
+
+
+  constexpr int LHS = 12;

The only test coverage is for integer division; do you intend to also cover 
floating-point division?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134749

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


[PATCH] D134744: [clang][Interp] Implement rem opcode

2022-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/test/AST/Interp/literals.cpp:161
+// expected-note {{division by zero}}
+
+

Same question here as with div in regards to testing float behavior. (If you 
don't intend to support floats yet, make sure the commit message makes that 
clear.)


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

https://reviews.llvm.org/D134744

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


[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Michał Górny via Phabricator via cfe-commits
mgorny added inline comments.



Comment at: clang/test/Driver/config-file3.c:27
 
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg 
first.
+//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries 
x86_64-unknown-linux-gnu-clang++.cfg first.
 //

sepavloff wrote:
> mgorny wrote:
> > sepavloff wrote:
> > > mgorny wrote:
> > > > sepavloff wrote:
> > > > > mgorny wrote:
> > > > > > arichardson wrote:
> > > > > > > sepavloff wrote:
> > > > > > > > Tests must check the case when target prefix is not a real 
> > > > > > > > triple as in the original test (qqq-clang).
> > > > > > > It would be quite important for me that this continues to work. I 
> > > > > > > made use of that in the CheriBSD toolchain when creating [[ 
> > > > > > > https://github.com/CTSRD-CHERI/cheribuild/blob/master/pycheribuild/projects/cross/llvm.py#L499
> > > > > > >  | symlinked binaries to easily build for different ABIs]] such 
> > > > > > > as `cheribsd-riscv64-hybrid-clang++` and 
> > > > > > > `cheribsd-riscv64-purecap-clang-cpp`. It appears this previously 
> > > > > > > only worked if the prefix did not start with a valid triple 
> > > > > > > (which is why I put the OS before the architecture). I think it 
> > > > > > > would also be nice if the whole prefix was checked even if it 
> > > > > > > starts with a valid triple, but this does not need to be changed 
> > > > > > > in this patch (haven't looked at it in detail so this might 
> > > > > > > actually work).
> > > > > > If the prefix is not a valid triple, then clang ignores it and uses 
> > > > > > host triple instead. And now we're back to square one. If I check 
> > > > > > both variants, it's too complex. If I don't, it's bad too.
> > > > > Our customers use this feature. Target prefix may designate, for 
> > > > > example, debug build or build with specific framework.
> > > > How are we supposed to avoid the "absurd" case where x86_64 configs are 
> > > > loaded for `-m32` invocation then?
> > > In this case overloading target does not makes sense. You need to analyze 
> > > `RealTriple` in `Driver::loadDefaultConfigFiles` and if it is wrong, use 
> > > target prefix as if it is real target.
> > What do you mean by "wrong"? The target triple is always a correct triple.
> Not triple, sorry. Target prefix taken from `ClangNameParts.TargetPrefix`.
Now I'm confused. Are you suggesting that we use prefix instead of target if 
it's… incorrect?


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

https://reviews.llvm.org/D134337

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


[PATCH] D134744: [clang][Interp] Implement rem opcode

2022-09-28 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/test/AST/Interp/literals.cpp:161
+// expected-note {{division by zero}}
+
+

aaron.ballman wrote:
> Same question here as with div in regards to testing float behavior. (If you 
> don't intend to support floats yet, make sure the commit message makes that 
> clear.)
Ultimately sure, but as of right now, floats aren't supported at all, so nope.


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

https://reviews.llvm.org/D134744

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


[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added inline comments.



Comment at: clang/test/Driver/config-file3.c:27
 
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg 
first.
+//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries 
x86_64-unknown-linux-gnu-clang++.cfg first.
 //

mgorny wrote:
> sepavloff wrote:
> > mgorny wrote:
> > > sepavloff wrote:
> > > > mgorny wrote:
> > > > > sepavloff wrote:
> > > > > > mgorny wrote:
> > > > > > > arichardson wrote:
> > > > > > > > sepavloff wrote:
> > > > > > > > > Tests must check the case when target prefix is not a real 
> > > > > > > > > triple as in the original test (qqq-clang).
> > > > > > > > It would be quite important for me that this continues to work. 
> > > > > > > > I made use of that in the CheriBSD toolchain when creating [[ 
> > > > > > > > https://github.com/CTSRD-CHERI/cheribuild/blob/master/pycheribuild/projects/cross/llvm.py#L499
> > > > > > > >  | symlinked binaries to easily build for different ABIs]] such 
> > > > > > > > as `cheribsd-riscv64-hybrid-clang++` and 
> > > > > > > > `cheribsd-riscv64-purecap-clang-cpp`. It appears this 
> > > > > > > > previously only worked if the prefix did not start with a valid 
> > > > > > > > triple (which is why I put the OS before the architecture). I 
> > > > > > > > think it would also be nice if the whole prefix was checked 
> > > > > > > > even if it starts with a valid triple, but this does not need 
> > > > > > > > to be changed in this patch (haven't looked at it in detail so 
> > > > > > > > this might actually work).
> > > > > > > If the prefix is not a valid triple, then clang ignores it and 
> > > > > > > uses host triple instead. And now we're back to square one. If I 
> > > > > > > check both variants, it's too complex. If I don't, it's bad too.
> > > > > > Our customers use this feature. Target prefix may designate, for 
> > > > > > example, debug build or build with specific framework.
> > > > > How are we supposed to avoid the "absurd" case where x86_64 configs 
> > > > > are loaded for `-m32` invocation then?
> > > > In this case overloading target does not makes sense. You need to 
> > > > analyze `RealTriple` in `Driver::loadDefaultConfigFiles` and if it is 
> > > > wrong, use target prefix as if it is real target.
> > > What do you mean by "wrong"? The target triple is always a correct triple.
> > Not triple, sorry. Target prefix taken from `ClangNameParts.TargetPrefix`.
> Now I'm confused. Are you suggesting that we use prefix instead of target if 
> it's… incorrect?
Basically yes.

We need to support arbitrary target prefixes, because they are used now. If 
`ClangNameParts.TargetPrefix` is not a valid triple, use it to build config 
file names.

We also need to use config files like `x86_64.cfg` where middle components are 
dropped, they are also used. They also need to support target overloading.


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

https://reviews.llvm.org/D134337

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


[PATCH] D134744: [clang][Interp] Implement rem opcode

2022-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/test/AST/Interp/literals.cpp:161
+// expected-note {{division by zero}}
+
+

tbaeder wrote:
> aaron.ballman wrote:
> > Same question here as with div in regards to testing float behavior. (If 
> > you don't intend to support floats yet, make sure the commit message makes 
> > that clear.)
> Ultimately sure, but as of right now, floats aren't supported at all, so nope.
Okay, fine by me.

In terms of the request by @shafik for `INT_MIN % -1`, keep in mind there are 
arbitrary width integer types like `_BitInt`, so you should add test coverage 
for code like:
```
constexpr _BitInt(7) Val = -64;
static_assert(Val % (_BitInt(7)-1, "");
```
and similar for division.


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

https://reviews.llvm.org/D134744

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


[PATCH] D134361: [clang][Interp] Fix copy constructors of structs with array members

2022-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:641
 return this->visitInitializer(DIE->getExpr());
+  } else if (const auto AILE = dyn_cast(Initializer)) {
+// TODO: This compiles to quite a lot of bytecode if the array is larger.





Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:654-664
+  if (!this->emitDupPtr(SubExpr))
+return false;
+
+  if (!this->visit(SubExpr))
+return false;
+
+  if (!this->emitInitElem(*ElemT, I, Initializer))

In all of these cases we're leaving `ArrayIndex` set to `I` instead of `None`, 
is that intentional? (Might be worth an RAII object to handle this sort of 
thing.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134361

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


[PATCH] D134804: [clang][Interp] Implement bitwise Not operations

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134804

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Opcodes.td
  clang/test/AST/Interp/literals.cpp


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -66,6 +66,13 @@
 static_assert(-true, "");
 static_assert(-false, ""); //expected-error{{failed}} ref-error{{failed}}
 
+static_assert(~0 == -1, "");
+static_assert(~1 == -1, "");
+static_assert(~-1 == 0, "");
+static_assert(~255 == -256, "");
+
+
+
 constexpr int m = 10;
 constexpr const int *p = &m;
 static_assert(p != nullptr, "");
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -422,6 +422,12 @@
   let HasGroup = 1;
 }
 
+// [Real] -> [Real]
+def Not: Opcode {
+  let Types = [NumberTypeClass];
+  let HasGroup = 1;
+}
+
 
//===--===//
 // Cast.
 
//===--===//
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -238,6 +238,20 @@
   return true;
 }
 
+/// 1) Pops the value from the stack.
+/// 2) Pushes the bitwise negated value on the stack (~V).
+template ::T>
+bool Not(InterpState &S, CodePtr OpPC) {
+  const T &Val = S.Stk.pop();
+  T Result;
+  if (!T::Not(Val, &Result)) {
+S.Stk.push(Result);
+return true;
+  }
+
+  return false;
+}
+
 
//===--===//
 // EQ, NE, GT, GE, LT, LE
 
//===--===//
Index: clang/lib/AST/Interp/Integral.h
===
--- clang/lib/AST/Interp/Integral.h
+++ clang/lib/AST/Interp/Integral.h
@@ -225,6 +225,11 @@
 return false;
   }
 
+  static bool Not(Integral A, Integral *R) {
+*R = Integral(~A.V);
+return false;
+  }
+
 private:
   template  static bool CheckAddUB(T A, T B, T &R) {
 if constexpr (std::is_signed_v) {
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1049,6 +1049,11 @@
   return DiscardResult ? this->emitPop(T, E) : true;
 });
   case UO_Not:// ~x
+if (!this->Visit(SubExpr))
+  return false;
+if (Optional T = classify(E->getType()))
+  return this->emitNot(*T, E);
+return false;
   case UO_Real:   // __real x
   case UO_Imag:   // __imag x
   case UO_Extension:


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -66,6 +66,13 @@
 static_assert(-true, "");
 static_assert(-false, ""); //expected-error{{failed}} ref-error{{failed}}
 
+static_assert(~0 == -1, "");
+static_assert(~1 == -1, "");
+static_assert(~-1 == 0, "");
+static_assert(~255 == -256, "");
+
+
+
 constexpr int m = 10;
 constexpr const int *p = &m;
 static_assert(p != nullptr, "");
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -422,6 +422,12 @@
   let HasGroup = 1;
 }
 
+// [Real] -> [Real]
+def Not: Opcode {
+  let Types = [NumberTypeClass];
+  let HasGroup = 1;
+}
+
 //===--===//
 // Cast.
 //===--===//
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -238,6 +238,20 @@
   return true;
 }
 
+/// 1) Pops the value from the stack.
+/// 2) Pushes the bitwise negated value on the stack (~V).
+template ::T>
+bool Not(InterpState &S, CodePtr OpPC) {
+  const T &Val = S.Stk.pop();
+  T Result;
+  if (!T::Not(Val, &Result)) {
+S.Stk.push(Result);
+return true;
+  }
+
+  return false;
+}
+
 //===--===//
 // EQ, NE, GT, GE, LT, LE
 //===--===//
Index: clang/

[PATCH] D134804: [clang][Interp] Implement bitwise Not operations

2022-09-28 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 463515.

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

https://reviews.llvm.org/D134804

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Opcodes.td
  clang/test/AST/Interp/literals.cpp


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -66,6 +66,13 @@
 static_assert(-true, "");
 static_assert(-false, ""); //expected-error{{failed}} ref-error{{failed}}
 
+static_assert(~0 == -1, "");
+static_assert(~1 == -2, "");
+static_assert(~-1 == 0, "");
+static_assert(~255 == -256, "");
+
+
+
 constexpr int m = 10;
 constexpr const int *p = &m;
 static_assert(p != nullptr, "");
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -422,6 +422,12 @@
   let HasGroup = 1;
 }
 
+// [Real] -> [Real]
+def Not: Opcode {
+  let Types = [NumberTypeClass];
+  let HasGroup = 1;
+}
+
 
//===--===//
 // Cast.
 
//===--===//
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -238,6 +238,20 @@
   return true;
 }
 
+/// 1) Pops the value from the stack.
+/// 2) Pushes the bitwise negated value on the stack (~V).
+template ::T>
+bool Not(InterpState &S, CodePtr OpPC) {
+  const T &Val = S.Stk.pop();
+  T Result;
+  if (!T::Not(Val, &Result)) {
+S.Stk.push(Result);
+return true;
+  }
+
+  return false;
+}
+
 
//===--===//
 // EQ, NE, GT, GE, LT, LE
 
//===--===//
Index: clang/lib/AST/Interp/Integral.h
===
--- clang/lib/AST/Interp/Integral.h
+++ clang/lib/AST/Interp/Integral.h
@@ -225,6 +225,11 @@
 return false;
   }
 
+  static bool Not(Integral A, Integral *R) {
+*R = Integral(~A.V);
+return false;
+  }
+
 private:
   template  static bool CheckAddUB(T A, T B, T &R) {
 if constexpr (std::is_signed_v) {
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1049,6 +1049,11 @@
   return DiscardResult ? this->emitPop(T, E) : true;
 });
   case UO_Not:// ~x
+if (!this->Visit(SubExpr))
+  return false;
+if (Optional T = classify(E->getType()))
+  return this->emitNot(*T, E);
+return false;
   case UO_Real:   // __real x
   case UO_Imag:   // __imag x
   case UO_Extension:


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -66,6 +66,13 @@
 static_assert(-true, "");
 static_assert(-false, ""); //expected-error{{failed}} ref-error{{failed}}
 
+static_assert(~0 == -1, "");
+static_assert(~1 == -2, "");
+static_assert(~-1 == 0, "");
+static_assert(~255 == -256, "");
+
+
+
 constexpr int m = 10;
 constexpr const int *p = &m;
 static_assert(p != nullptr, "");
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -422,6 +422,12 @@
   let HasGroup = 1;
 }
 
+// [Real] -> [Real]
+def Not: Opcode {
+  let Types = [NumberTypeClass];
+  let HasGroup = 1;
+}
+
 //===--===//
 // Cast.
 //===--===//
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -238,6 +238,20 @@
   return true;
 }
 
+/// 1) Pops the value from the stack.
+/// 2) Pushes the bitwise negated value on the stack (~V).
+template ::T>
+bool Not(InterpState &S, CodePtr OpPC) {
+  const T &Val = S.Stk.pop();
+  T Result;
+  if (!T::Not(Val, &Result)) {
+S.Stk.push(Result);
+return true;
+  }
+
+  return false;
+}
+
 //===--===//
 // EQ, NE, GT, GE, LT, LE
 //===--===//
Index: clang/lib/AST/Interp/Integral.h
===
--- clang/lib/AST/Interp/Integral.h
+++ clang/lib/AST/Interp/Integral.h
@@ -225

[PATCH] D134361: [clang][Interp] Fix copy constructors of structs with array members

2022-09-28 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder marked an inline comment as done.
tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:654-664
+  if (!this->emitDupPtr(SubExpr))
+return false;
+
+  if (!this->visit(SubExpr))
+return false;
+
+  if (!this->emitInitElem(*ElemT, I, Initializer))

aaron.ballman wrote:
> In all of these cases we're leaving `ArrayIndex` set to `I` instead of 
> `None`, is that intentional? (Might be worth an RAII object to handle this 
> sort of thing.)
Heh :) Good you notice this as well. Yes, that's something I was worried about. 
It's not intentional at all. I'll try adding a RAII object.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134361

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


[PATCH] D134175: [clang][Interp] Implement record instance functions and returning struct types

2022-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Generally looks reasonable to me.




Comment at: clang/test/AST/Interp/records.cpp:52
 static_assert(!ints2.c, "");
 
 constexpr Ints ints3 = getInts();

Can you think of a reasonable way for us to test that you're handling the RVO 
pointer properly?


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

https://reviews.llvm.org/D134175

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


[PATCH] D133468: [clang] Implement divergence for TypedefType and UsingType

2022-09-28 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added a comment.

@sammccall ping, is the explanation above reasonable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133468

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


[PATCH] D134057: [clang][Interp] Start implementing record types

2022-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:689
+const Decl *Callee = CE->getCalleeDecl();
+const Function *Func = P.getFunction(dyn_cast(Callee));
+

tbaeder wrote:
> aaron.ballman wrote:
> > tbaeder wrote:
> > > aaron.ballman wrote:
> > > > What if this comes back as `nullptr` (does `getFunction()` handle that 
> > > > gracefully)?
> > > It does not :) I was relying on a crash somewhere when I have a test case 
> > > for it.
> > I'd rather use an explicit `assert` for that instead of relying on a crash; 
> > it's easier to spot the `assert` in that case.
> I forgot I hadn't posted this to phab yet, but 
> https://reviews.llvm.org/D134699 introduces a `getFunction` that asserts that 
> the given `FunctionDecl` is not null.
Ah, okay, that works fine for me then.


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

https://reviews.llvm.org/D134057

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


[PATCH] D134311: [clang] handle extended integer constant expressions in _Static_assert (PR #57687)

2022-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Thank you for working on this! It looks pretty close to good, with just a few 
comments. Can you also add a release note for the changes as well (to 
clang/docs/ReleaseNotes.rst)?




Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16733
+  while (auto *BaseCast = dyn_cast(BaseExpr))
+BaseExpr = BaseCast->getSubExpr();
+}

tbaeder wrote:
> There is `Expr::ignoreParenImpCasts()` or `Expr::ImpCasts()` that should do 
> this loop for you.
+1, I would use `Expr::IgnoreImpCasts()` instead of this manual loop.



Comment at: clang/test/Sema/static-assert.c:79
+static int static_var;
+_Static_assert(&static_var != 0, "");  // ext-warning {{'_Static_assert' is a 
C11 extension}} expected-warning {{comparison of address of 'static_var' not 
equal to a null pointer is always true}}
+_Static_assert("" != 0, "");   // ext-warning {{'_Static_assert' is a 
C11 extension}}

This makes it a bit easier to see there are two diagnostics expected on that 
line instead of just one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134311

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


[PATCH] D125944: Template instantiation error recovery

2022-09-28 Thread Vassil Vassilev via Phabricator via cfe-commits
v.g.vassilev added a reviewer: aaron.ballman.
v.g.vassilev added a comment.

This looks reasonable to me but let's have another pair of eyes.


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

https://reviews.llvm.org/D125944

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


[PATCH] D134361: [clang][Interp] Fix copy constructors of structs with array members

2022-09-28 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 463522.

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

https://reviews.llvm.org/D134361

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

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -37,6 +37,9 @@
 static_assert(ints.b == 30, "");
 static_assert(ints.c, "");
 static_assert(ints.getTen() == 10, "");
+static_assert(ints.numbers[0] == 1, "");
+static_assert(ints.numbers[1] == 2, "");
+static_assert(ints.numbers[2] == 3, "");
 
 constexpr const BoolPair &BP = ints.bp;
 static_assert(BP.first, "");
@@ -64,11 +67,17 @@
 static_assert(ints4.a == (40 * 50), "");
 static_assert(ints4.b == 0, "");
 static_assert(ints4.c, "");
-
-
-// FIXME: Implement initialization by DeclRefExpr.
-//constexpr Ints ints4 = ints3;  TODO
-
+static_assert(ints4.numbers[0] == 1, "");
+static_assert(ints4.numbers[1] == 2, "");
+static_assert(ints4.numbers[2] == 3, "");
+
+constexpr Ints ints5 = ints4;
+static_assert(ints5.a == (40 * 50), "");
+static_assert(ints5.b == 0, "");
+static_assert(ints5.c, "");
+static_assert(ints5.numbers[0] == 1, "");
+static_assert(ints5.numbers[1] == 2, "");
+static_assert(ints5.numbers[2] == 3, "");
 
 
 struct Ints2 {
Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -105,7 +105,7 @@
   const Record::Field *F = R->getField(Member);
 
   if (Optional T = this->classify(InitExpr->getType())) {
-if (!this->emitDupPtr(InitExpr))
+if (!this->emitThis(InitExpr))
   return false;
 
 if (!this->visit(InitExpr))
@@ -116,7 +116,7 @@
   } else {
 // Non-primitive case. Get a pointer to the field-to-initialize
 // on the stack and call visitInitialzer() for it.
-if (!this->emitDupPtr(InitExpr))
+if (!this->emitThis(InitExpr))
   return false;
 
 if (!this->emitGetPtrField(F->Offset, InitExpr))
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -34,6 +34,7 @@
 template  class VariableScope;
 template  class DeclScope;
 template  class OptionScope;
+template  class ArrayIndexScope;
 
 /// Compilation context for expressions.
 template 
@@ -85,6 +86,8 @@
   bool VisitConstantExpr(const ConstantExpr *E);
   bool VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *E);
   bool VisitMemberExpr(const MemberExpr *E);
+  bool VisitArrayInitIndexExpr(const ArrayInitIndexExpr *E);
+  bool VisitOpaqueValueExpr(const OpaqueValueExpr *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
@@ -199,6 +202,7 @@
   friend class RecordScope;
   friend class DeclScope;
   friend class OptionScope;
+  friend class ArrayIndexScope;
 
   /// Emits a zero initializer.
   bool visitZeroInitializer(PrimType T, const Expr *E);
@@ -260,7 +264,7 @@
   /// Current scope.
   VariableScope *VarScope = nullptr;
 
-  /// Current argument index.
+  /// Current argument index. Needed to emit ArrayInitIndexExpr.
   llvm::Optional ArrayIndex;
 
   /// Flag indicating if return value is to be discarded.
@@ -362,6 +366,22 @@
   }
 };
 
+template  class ArrayIndexScope final {
+public:
+  ArrayIndexScope(ByteCodeExprGen *Ctx, uint64_t Index) : Ctx(Ctx) {
+OldArrayIndex = Ctx->ArrayIndex;
+Ctx->ArrayIndex = Index;
+  }
+
+  ~ArrayIndexScope() {
+Ctx->ArrayIndex = OldArrayIndex;
+  }
+
+private:
+  ByteCodeExprGen *Ctx;
+  Optional OldArrayIndex;
+};
+
 } // namespace interp
 } // namespace clang
 
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -325,6 +325,18 @@
   return false;
 }
 
+template 
+bool ByteCodeExprGen::VisitArrayInitIndexExpr(
+const ArrayInitIndexExpr *E) {
+  assert(ArrayIndex);
+  return this->emitConstUint64(*ArrayIndex, E);
+}
+
+template 
+bool ByteCodeExprGen::VisitOpaqueValueExpr(const OpaqueValueExpr *E) {
+  return this->visit(E->getSourceExpr());
+}
+
 template  bool ByteCodeExprGen::discard(const Expr *E) {
   OptionScope Scope(this, /*NewDiscardResult=*/true);
   return this->Visit(E);
@@ -626,6 +638,32 @@
 return true;
   } else if (const auto *DIE = dyn_cast(Initializer)) {
 return this->visitInitializer(DIE->getExpr());
+  } else if (const auto *AILE = dyn_cast(Initializer)) {
+// TODO: This compiles to quite a lot of bytecode if the array is larger.
+//   Investigate compiling this to a loop, or at least try 

[PATCH] D134175: [clang][Interp] Implement record instance functions and returning struct types

2022-09-28 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/test/AST/Interp/records.cpp:52
 static_assert(!ints2.c, "");
 
 constexpr Ints ints3 = getInts();

aaron.ballman wrote:
> Can you think of a reasonable way for us to test that you're handling the RVO 
> pointer properly?
I think this is tested already by any function returning a non-primitive value, 
i.e. a struct or an array, e.g. `getInts()` above.


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

https://reviews.llvm.org/D134175

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


[PATCH] D134175: [clang][Interp] Implement record instance functions and returning struct types

2022-09-28 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!




Comment at: clang/test/AST/Interp/records.cpp:52
 static_assert(!ints2.c, "");
 
 constexpr Ints ints3 = getInts();

tbaeder wrote:
> aaron.ballman wrote:
> > Can you think of a reasonable way for us to test that you're handling the 
> > RVO pointer properly?
> I think this is tested already by any function returning a non-primitive 
> value, i.e. a struct or an array, e.g. `getInts()` above.
Oh derp, you're right!


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

https://reviews.llvm.org/D134175

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


[PATCH] D134361: [clang][Interp] Fix copy constructors of structs with array members

2022-09-28 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/D134361/new/

https://reviews.llvm.org/D134361

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


[PATCH] D134744: [clang][Interp] Implement rem opcode

2022-09-28 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/test/AST/Interp/literals.cpp:161
+// expected-note {{division by zero}}
+
+

aaron.ballman wrote:
> tbaeder wrote:
> > aaron.ballman wrote:
> > > Same question here as with div in regards to testing float behavior. (If 
> > > you don't intend to support floats yet, make sure the commit message 
> > > makes that clear.)
> > Ultimately sure, but as of right now, floats aren't supported at all, so 
> > nope.
> Okay, fine by me.
> 
> In terms of the request by @shafik for `INT_MIN % -1`, keep in mind there are 
> arbitrary width integer types like `_BitInt`, so you should add test coverage 
> for code like:
> ```
> constexpr _BitInt(7) Val = -64;
> static_assert(Val % (_BitInt(7)-1, "");
> ```
> and similar for division.
I just checked, `_BitInt` doesn't work at all; it bails out when creating the 
`Descriptor` for the variable.

I already added the suggested note for the `INT_MIN` case in this patch, but I 
wasn't sure how to test is properly. Should I just use a fixed-width integer 
type and copy its `_MIN` value in the test?


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

https://reviews.llvm.org/D134744

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


[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Michał Górny via Phabricator via cfe-commits
mgorny updated this revision to Diff 463531.
mgorny marked 2 inline comments as done.
mgorny added a comment.

Swap driver and target config loading order.


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

https://reviews.llvm.org/D134337

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UsersManual.rst
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/config-file3.c

Index: clang/test/Driver/config-file3.c
===
--- clang/test/Driver/config-file3.c
+++ clang/test/Driver/config-file3.c
@@ -14,107 +14,181 @@
 // CHECK-REL: Configuration file: {{.*}}/workdir/cfg-1
 // CHECK-REL: -Wundefined-var-template
 
+//--- Config files are searched for in binary directory as well.
+//
+// RUN: mkdir %t/testbin
+// RUN: ln -s %clang %t/testbin/clang
+// RUN: echo "-Werror" > %t/testbin/aaa.cfg
+// RUN: %t/testbin/clang --config-system-dir= --config-user-dir= --config aaa.cfg -c -no-canonical-prefixes -### %s 2>&1 | FileCheck %s -check-prefix CHECK-BIN
+//
+// CHECK-BIN: Configuration file: {{.*}}/testbin/aaa.cfg
+// CHECK-BIN: -Werror
 
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg first.
+//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries x86_64-unknown-linux-gnu-clang++.cfg first.
 //
 // RUN: mkdir %t/testdmode
-// RUN: ln -s %clang %t/testdmode/qqq-clang-g++
-// RUN: echo "-Wundefined-func-template" > %t/testdmode/qqq-clang-g++.cfg
-// RUN: echo "-Werror" > %t/testdmode/qqq.cfg
-// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c -no-canonical-prefixes -### %s 2>&1 | FileCheck %s -check-prefix FULL-NAME
+// RUN: ln -s %clang %t/testdmode/i386-unknown-linux-gnu-clang-g++
+// RUN: ln -s %clang %t/testdmode/x86_64-unknown-linux-gnu-clang-g++
+// RUN: ln -s %clang %t/testdmode/x86_64-unknown-linux-gnu-clang
+// RUN: touch %t/testdmode/x86_64-unknown-linux-gnu-clang++.cfg
+// RUN: touch %t/testdmode/x86_64-unknown-linux-gnu-clang-g++.cfg
+// RUN: touch %t/testdmode/x86_64-unknown-linux-gnu-clang.cfg
+// RUN: touch %t/testdmode/x86_64-unknown-linux-gnu.cfg
+// RUN: touch %t/testdmode/i386-unknown-linux-gnu-clang++.cfg
+// RUN: touch %t/testdmode/i386-unknown-linux-gnu-clang-g++.cfg
+// RUN: touch %t/testdmode/i386-unknown-linux-gnu-clang.cfg
+// RUN: touch %t/testdmode/i386-unknown-linux-gnu.cfg
+// RUN: touch %t/testdmode/clang++.cfg
+// RUN: touch %t/testdmode/clang-g++.cfg
+// RUN: touch %t/testdmode/clang.cfg
+// RUN: %t/testdmode/x86_64-unknown-linux-gnu-clang-g++ --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1
 //
-// FULL-NAME: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
-// FULL-NAME: -Wundefined-func-template
-// FULL-NAME-NOT: -Werror
+// FULL1-NOT: Configuration file:
+// FULL1: Configuration file: {{.*}}/testdmode/x86_64-unknown-linux-gnu-clang++.cfg
+// FULL1-NOT: Configuration file:
+
+//--- -m32 overrides triple.
 //
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg even without -no-canonical-prefixes.
-// (As the clang executable and symlink are in different directories, this
-// requires specifying the path via --config-*-dir= though.)
+// RUN: %t/testdmode/x86_64-unknown-linux-gnu-clang-g++ -m32 --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1-I386
 //
-// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir=%t/testdmode -c -### %s 2>&1 | FileCheck %s -check-prefix SYMLINK
+// FULL1-I386-NOT: Configuration file:
+// FULL1-I386: Configuration file: {{.*}}/testdmode/i386-unknown-linux-gnu-clang++.cfg
+// FULL1-I386-NOT: Configuration file:
+
+//--- --target= also works for overriding triple.
+//
+// RUN: %t/testdmode/x86_64-unknown-linux-gnu-clang-g++ --target=i386-unknown-linux-gnu --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1-I386
+
+//--- With --target= + -m64, -m64 takes precedence.
+//
+// RUN: %t/testdmode/x86_64-unknown-linux-gnu-clang-g++ --target=i386-unknown-linux-gnu -m64 --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1
+
+//--- i386 prefix also works for 32-bit.
 //
-// SYMLINK: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
+// RUN: %t/testdmode/i386-unknown-linux-gnu-clang-g++ --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1-I386
+
+//--- i386 prefix + -m64 also works for 64-bit.
+//
+// RUN: %t/testdmode/i386-unknown-linux-gnu-clang-g++ -m64 --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1
 
 //--- File specified by --config is loaded after the one inferred from the executable.
 //
-// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir=%S/Inputs/config --config-user-di

[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Alexander Richardson via Phabricator via cfe-commits
arichardson added inline comments.



Comment at: clang/test/Driver/config-file3.c:27
 
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg 
first.
+//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries 
x86_64-unknown-linux-gnu-clang++.cfg first.
 //

sepavloff wrote:
> mgorny wrote:
> > sepavloff wrote:
> > > mgorny wrote:
> > > > sepavloff wrote:
> > > > > mgorny wrote:
> > > > > > sepavloff wrote:
> > > > > > > mgorny wrote:
> > > > > > > > arichardson wrote:
> > > > > > > > > sepavloff wrote:
> > > > > > > > > > Tests must check the case when target prefix is not a real 
> > > > > > > > > > triple as in the original test (qqq-clang).
> > > > > > > > > It would be quite important for me that this continues to 
> > > > > > > > > work. I made use of that in the CheriBSD toolchain when 
> > > > > > > > > creating [[ 
> > > > > > > > > https://github.com/CTSRD-CHERI/cheribuild/blob/master/pycheribuild/projects/cross/llvm.py#L499
> > > > > > > > >  | symlinked binaries to easily build for different ABIs]] 
> > > > > > > > > such as `cheribsd-riscv64-hybrid-clang++` and 
> > > > > > > > > `cheribsd-riscv64-purecap-clang-cpp`. It appears this 
> > > > > > > > > previously only worked if the prefix did not start with a 
> > > > > > > > > valid triple (which is why I put the OS before the 
> > > > > > > > > architecture). I think it would also be nice if the whole 
> > > > > > > > > prefix was checked even if it starts with a valid triple, but 
> > > > > > > > > this does not need to be changed in this patch (haven't 
> > > > > > > > > looked at it in detail so this might actually work).
> > > > > > > > If the prefix is not a valid triple, then clang ignores it and 
> > > > > > > > uses host triple instead. And now we're back to square one. If 
> > > > > > > > I check both variants, it's too complex. If I don't, it's bad 
> > > > > > > > too.
> > > > > > > Our customers use this feature. Target prefix may designate, for 
> > > > > > > example, debug build or build with specific framework.
> > > > > > How are we supposed to avoid the "absurd" case where x86_64 configs 
> > > > > > are loaded for `-m32` invocation then?
> > > > > In this case overloading target does not makes sense. You need to 
> > > > > analyze `RealTriple` in `Driver::loadDefaultConfigFiles` and if it is 
> > > > > wrong, use target prefix as if it is real target.
> > > > What do you mean by "wrong"? The target triple is always a correct 
> > > > triple.
> > > Not triple, sorry. Target prefix taken from `ClangNameParts.TargetPrefix`.
> > Now I'm confused. Are you suggesting that we use prefix instead of target 
> > if it's… incorrect?
> Basically yes.
> 
> We need to support arbitrary target prefixes, because they are used now. If 
> `ClangNameParts.TargetPrefix` is not a valid triple, use it to build config 
> file names.
> 
> We also need to use config files like `x86_64.cfg` where middle components 
> are dropped, they are also used. They also need to support target overloading.
I can replace my approach with simple shell scripts that invoke clang 
--config=..., but IMO it would be nice if the logic was something like the 
first check being
`if "explicit --target empty" and "ClangNameParts.TargetPrefix non-empty" and 
"ClangNameParts.TargetPrefix not a valid triple" -> load 
ClangNameParts.TargetPrefix + ".cfg"` followed by the current logic.
I don't mind either way whether the first check should also load a 
driver-specific config file or not. I'd also be happy if the logic was "try 
loading ClangNameParts.TargetPrefix if explicit --target empty" regardless of 
whether it is a valid triple or not.
Does this approach sound reasonable?


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

https://reviews.llvm.org/D134337

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


[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Michał Górny via Phabricator via cfe-commits
mgorny marked an inline comment as done.
mgorny added inline comments.



Comment at: clang/test/Driver/config-file3.c:27
 
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg 
first.
+//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries 
x86_64-unknown-linux-gnu-clang++.cfg first.
 //

arichardson wrote:
> sepavloff wrote:
> > mgorny wrote:
> > > sepavloff wrote:
> > > > mgorny wrote:
> > > > > sepavloff wrote:
> > > > > > mgorny wrote:
> > > > > > > sepavloff wrote:
> > > > > > > > mgorny wrote:
> > > > > > > > > arichardson wrote:
> > > > > > > > > > sepavloff wrote:
> > > > > > > > > > > Tests must check the case when target prefix is not a 
> > > > > > > > > > > real triple as in the original test (qqq-clang).
> > > > > > > > > > It would be quite important for me that this continues to 
> > > > > > > > > > work. I made use of that in the CheriBSD toolchain when 
> > > > > > > > > > creating [[ 
> > > > > > > > > > https://github.com/CTSRD-CHERI/cheribuild/blob/master/pycheribuild/projects/cross/llvm.py#L499
> > > > > > > > > >  | symlinked binaries to easily build for different ABIs]] 
> > > > > > > > > > such as `cheribsd-riscv64-hybrid-clang++` and 
> > > > > > > > > > `cheribsd-riscv64-purecap-clang-cpp`. It appears this 
> > > > > > > > > > previously only worked if the prefix did not start with a 
> > > > > > > > > > valid triple (which is why I put the OS before the 
> > > > > > > > > > architecture). I think it would also be nice if the whole 
> > > > > > > > > > prefix was checked even if it starts with a valid triple, 
> > > > > > > > > > but this does not need to be changed in this patch (haven't 
> > > > > > > > > > looked at it in detail so this might actually work).
> > > > > > > > > If the prefix is not a valid triple, then clang ignores it 
> > > > > > > > > and uses host triple instead. And now we're back to square 
> > > > > > > > > one. If I check both variants, it's too complex. If I don't, 
> > > > > > > > > it's bad too.
> > > > > > > > Our customers use this feature. Target prefix may designate, 
> > > > > > > > for example, debug build or build with specific framework.
> > > > > > > How are we supposed to avoid the "absurd" case where x86_64 
> > > > > > > configs are loaded for `-m32` invocation then?
> > > > > > In this case overloading target does not makes sense. You need to 
> > > > > > analyze `RealTriple` in `Driver::loadDefaultConfigFiles` and if it 
> > > > > > is wrong, use target prefix as if it is real target.
> > > > > What do you mean by "wrong"? The target triple is always a correct 
> > > > > triple.
> > > > Not triple, sorry. Target prefix taken from 
> > > > `ClangNameParts.TargetPrefix`.
> > > Now I'm confused. Are you suggesting that we use prefix instead of target 
> > > if it's… incorrect?
> > Basically yes.
> > 
> > We need to support arbitrary target prefixes, because they are used now. If 
> > `ClangNameParts.TargetPrefix` is not a valid triple, use it to build config 
> > file names.
> > 
> > We also need to use config files like `x86_64.cfg` where middle components 
> > are dropped, they are also used. They also need to support target 
> > overloading.
> I can replace my approach with simple shell scripts that invoke clang 
> --config=..., but IMO it would be nice if the logic was something like the 
> first check being
> `if "explicit --target empty" and "ClangNameParts.TargetPrefix non-empty" and 
> "ClangNameParts.TargetPrefix not a valid triple" -> load 
> ClangNameParts.TargetPrefix + ".cfg"` followed by the current logic.
> I don't mind either way whether the first check should also load a 
> driver-specific config file or not. I'd also be happy if the logic was "try 
> loading ClangNameParts.TargetPrefix if explicit --target empty" regardless of 
> whether it is a valid triple or not.
> Does this approach sound reasonable?
Ok, so I guess that the logic would be: if name prefix's (non-normalized) 
triple does not have valid arch or valid os parts, we use that instead of real 
triple.



Comment at: clang/test/Driver/config-file3.c:60
+//
+// RUN: %t/testdmode/x86_64-unknown-linux-gnu-clang-g++ 
--target=i386-unknown-linux-gnu --config-system-dir= --config-user-dir= 
-no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1-I386
+

sepavloff wrote:
> We also need a test that checks that in the case of 
> `--target=i386-unknown-linux-gnu` and absence of 
> `i386-unknown-linux-gnu-clang++.cfg` clang does not load 
> `x86_64-unknown-linux-gnu-clang-g++.cfg`.
Isn't this what `FULL2 + -m32` tests for?


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

https://reviews.llvm.org/D134337

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


[PATCH] D134744: [clang][Interp] Implement rem opcode

2022-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/test/AST/Interp/literals.cpp:161
+// expected-note {{division by zero}}
+
+

tbaeder wrote:
> aaron.ballman wrote:
> > tbaeder wrote:
> > > aaron.ballman wrote:
> > > > Same question here as with div in regards to testing float behavior. 
> > > > (If you don't intend to support floats yet, make sure the commit 
> > > > message makes that clear.)
> > > Ultimately sure, but as of right now, floats aren't supported at all, so 
> > > nope.
> > Okay, fine by me.
> > 
> > In terms of the request by @shafik for `INT_MIN % -1`, keep in mind there 
> > are arbitrary width integer types like `_BitInt`, so you should add test 
> > coverage for code like:
> > ```
> > constexpr _BitInt(7) Val = -64;
> > static_assert(Val % (_BitInt(7)-1, "");
> > ```
> > and similar for division.
> I just checked, `_BitInt` doesn't work at all; it bails out when creating the 
> `Descriptor` for the variable.
> 
> I already added the suggested note for the `INT_MIN` case in this patch, but 
> I wasn't sure how to test is properly. Should I just use a fixed-width 
> integer type and copy its `_MIN` value in the test?
Ah, well that's something we should fix! :-D

> I already added the suggested note for the INT_MIN case in this patch, but I 
> wasn't sure how to test is properly. Should I just use a fixed-width integer 
> type and copy its _MIN value in the test?

You should be able to use:
```
#define INT_MIN (~__INT_MAX__)
```
(and similar for the other MIN macros) which relies on the predefined macro 
instead of something from a header.


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

https://reviews.llvm.org/D134744

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


[PATCH] D134744: [clang][Interp] Implement rem opcode

2022-09-28 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 463543.

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

https://reviews.llvm.org/D134744

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Opcodes.td
  clang/test/AST/Interp/literals.cpp

Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -145,3 +145,32 @@
 
 #endif
 };
+
+#define INT_MIN (~__INT_MAX__)
+
+namespace rem {
+  static_assert(2 % 2 == 0, "");
+  static_assert(2 % 1 == 0, "");
+  static_assert(-3 % 4 == -3, "");
+  static_assert(4 % -2 == 0, "");
+  static_assert(-3 % -4 == -3, "");
+
+  constexpr int zero() { return 0; }
+  static_assert(10 % zero() == 20, ""); // ref-error {{not an integral constant expression}} \
+// ref-note {{division by zero}} \
+// expected-error {{not an integral constant expression}} \
+// expected-note {{division by zero}}
+
+
+  static_assert(true % true == 0, "");
+  static_assert(false % true == 0, "");
+  static_assert(true % false == 10, ""); // ref-error {{not an integral constant expression}} \
+ // ref-note {{division by zero}} \
+ // expected-error {{not an integral constant expression}} \
+ // expected-note {{division by zero}}
+  constexpr int x = INT_MIN % - 1; // ref-error {{must be initialized by a constant expression}} \
+   // ref-note {{value 2147483648 is outside the range}} \
+   // expected-error {{must be initialized by a constant expression}} \
+   // expected-note {{value 2147483648 is outside the range}} \
+
+};
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -54,9 +54,13 @@
   list Types;
 }
 
-def AluTypeClass : TypeClass {
+def NumberTypeClass : TypeClass {
   let Types = [Sint8, Uint8, Sint16, Uint16, Sint32,
-   Uint32, Sint64, Uint64, Bool];
+   Uint32, Sint64, Uint64];
+}
+
+def AluTypeClass : TypeClass {
+  let Types = !listconcat(NumberTypeClass.Types, [Bool]);
 }
 
 def PtrTypeClass : TypeClass {
@@ -393,6 +397,10 @@
 def Sub : AluOpcode;
 def Add : AluOpcode;
 def Mul : AluOpcode;
+def Rem : Opcode {
+  let Types = [NumberTypeClass];
+  let HasGroup = 1;
+}
 
 
 //===--===//
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -153,6 +153,39 @@
   return AddSubMulHelper(S, OpPC, Bits, LHS, RHS);
 }
 
+/// 1) Pops the RHS from the stack.
+/// 2) Pops the LHS from the stack.
+/// 3) Pushes 'LHS % RHS' on the stack (the remainder of dividing LHS by RHS).
+template ::T>
+bool Rem(InterpState &S, CodePtr OpPC) {
+  const T &RHS = S.Stk.pop();
+  const T &LHS = S.Stk.pop();
+
+  if (RHS.isZero()) {
+const SourceInfo &Loc = S.Current->getSource(OpPC);
+S.FFDiag(Loc, diag::note_expr_divide_by_zero);
+return false;
+  }
+
+  if (LHS.isSigned() && LHS.isMin() && RHS.isNegative() && RHS.isMinusOne()) {
+APSInt LHSInt = LHS.toAPSInt();
+SmallString<32> Trunc;
+(-LHSInt.extend(LHSInt.getBitWidth() + 1)).toString(Trunc, 10);
+const SourceInfo &Loc = S.Current->getSource(OpPC);
+const Expr *E = S.Current->getExpr(OpPC);
+S.CCEDiag(Loc, diag::note_constexpr_overflow) << Trunc << E->getType();
+return false;
+  }
+
+  const unsigned Bits = RHS.bitWidth() * 2;
+  T Result;
+  if (!T::rem(LHS, RHS, Bits, &Result)) {
+S.Stk.push(Result);
+return true;
+  }
+  return false;
+}
+
 //===--===//
 // Inv
 //===--===//
Index: clang/lib/AST/Interp/Integral.h
===
--- clang/lib/AST/Interp/Integral.h
+++ clang/lib/AST/Interp/Integral.h
@@ -210,6 +210,11 @@
 return CheckMulUB(A.V, B.V, R->V);
   }
 
+  static bool rem(Integral A, Integral B, unsigned OpBits, Integral *R) {
+*R = Integral(A.V % B.V);
+return false;
+  }
+
   static bool neg(Integral A, Integral *R) {
 *R = -A;
 return false;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -211,6 +211,8 @@
   return Discard(this->emitAdd

[PATCH] D134744: [clang][Interp] Implement rem opcode

2022-09-28 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder marked an inline comment as done.
tbaeder added inline comments.



Comment at: clang/test/AST/Interp/literals.cpp:161
+// expected-note {{division by zero}}
+
+

aaron.ballman wrote:
> tbaeder wrote:
> > aaron.ballman wrote:
> > > tbaeder wrote:
> > > > aaron.ballman wrote:
> > > > > Same question here as with div in regards to testing float behavior. 
> > > > > (If you don't intend to support floats yet, make sure the commit 
> > > > > message makes that clear.)
> > > > Ultimately sure, but as of right now, floats aren't supported at all, 
> > > > so nope.
> > > Okay, fine by me.
> > > 
> > > In terms of the request by @shafik for `INT_MIN % -1`, keep in mind there 
> > > are arbitrary width integer types like `_BitInt`, so you should add test 
> > > coverage for code like:
> > > ```
> > > constexpr _BitInt(7) Val = -64;
> > > static_assert(Val % (_BitInt(7)-1, "");
> > > ```
> > > and similar for division.
> > I just checked, `_BitInt` doesn't work at all; it bails out when creating 
> > the `Descriptor` for the variable.
> > 
> > I already added the suggested note for the `INT_MIN` case in this patch, 
> > but I wasn't sure how to test is properly. Should I just use a fixed-width 
> > integer type and copy its `_MIN` value in the test?
> Ah, well that's something we should fix! :-D
> 
> > I already added the suggested note for the INT_MIN case in this patch, but 
> > I wasn't sure how to test is properly. Should I just use a fixed-width 
> > integer type and copy its _MIN value in the test?
> 
> You should be able to use:
> ```
> #define INT_MIN (~__INT_MAX__)
> ```
> (and similar for the other MIN macros) which relies on the predefined macro 
> instead of something from a header.
Done, thanks. But I guess this now depends on https://reviews.llvm.org/D134804 
since it uses `~` haha


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

https://reviews.llvm.org/D134744

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


[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Michał Górny via Phabricator via cfe-commits
mgorny updated this revision to Diff 463548.
mgorny edited the summary of this revision.
mgorny added a comment.
Herald added subscribers: pcwang-thead, s.egerton, simoncook.

Implement the backwards compatible logic for using name prefix if 1) no target 
override options are used, and 2) name prefix is not a valid triple.


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

https://reviews.llvm.org/D134337

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UsersManual.rst
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/config-file3.c

Index: clang/test/Driver/config-file3.c
===
--- clang/test/Driver/config-file3.c
+++ clang/test/Driver/config-file3.c
@@ -14,107 +14,251 @@
 // CHECK-REL: Configuration file: {{.*}}/workdir/cfg-1
 // CHECK-REL: -Wundefined-var-template
 
+//--- Config files are searched for in binary directory as well.
+//
+// RUN: mkdir %t/testbin
+// RUN: ln -s %clang %t/testbin/clang
+// RUN: echo "-Werror" > %t/testbin/aaa.cfg
+// RUN: %t/testbin/clang --config-system-dir= --config-user-dir= --config aaa.cfg -c -no-canonical-prefixes -### %s 2>&1 | FileCheck %s -check-prefix CHECK-BIN
+//
+// CHECK-BIN: Configuration file: {{.*}}/testbin/aaa.cfg
+// CHECK-BIN: -Werror
 
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg first.
+//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries x86_64-unknown-linux-gnu-clang++.cfg first.
 //
 // RUN: mkdir %t/testdmode
+// RUN: ln -s %clang %t/testdmode/cheribsd-riscv64-hybrid-clang++
 // RUN: ln -s %clang %t/testdmode/qqq-clang-g++
-// RUN: echo "-Wundefined-func-template" > %t/testdmode/qqq-clang-g++.cfg
-// RUN: echo "-Werror" > %t/testdmode/qqq.cfg
-// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir= -c -no-canonical-prefixes -### %s 2>&1 | FileCheck %s -check-prefix FULL-NAME
+// RUN: ln -s %clang %t/testdmode/x86_64-clang
+// RUN: ln -s %clang %t/testdmode/i386-unknown-linux-gnu-clang-g++
+// RUN: ln -s %clang %t/testdmode/x86_64-unknown-linux-gnu-clang-g++
+// RUN: ln -s %clang %t/testdmode/x86_64-unknown-linux-gnu-clang
+// RUN: touch %t/testdmode/cheribsd-riscv64-hybrid-clang++.cfg
+// RUN: touch %t/testdmode/cheribsd-riscv64-hybrid.cfg
+// RUN: touch %t/testdmode/qqq-clang-g++.cfg
+// RUN: touch %t/testdmode/qqq.cfg
+// RUN: touch %t/testdmode/x86_64-clang.cfg
+// RUN: touch %t/testdmode/x86_64.cfg
+// RUN: touch %t/testdmode/x86_64-unknown-linux-gnu-clang++.cfg
+// RUN: touch %t/testdmode/x86_64-unknown-linux-gnu-clang-g++.cfg
+// RUN: touch %t/testdmode/x86_64-unknown-linux-gnu-clang.cfg
+// RUN: touch %t/testdmode/x86_64-unknown-linux-gnu.cfg
+// RUN: touch %t/testdmode/i386-unknown-linux-gnu-clang++.cfg
+// RUN: touch %t/testdmode/i386-unknown-linux-gnu-clang-g++.cfg
+// RUN: touch %t/testdmode/i386-unknown-linux-gnu-clang.cfg
+// RUN: touch %t/testdmode/i386-unknown-linux-gnu.cfg
+// RUN: touch %t/testdmode/clang++.cfg
+// RUN: touch %t/testdmode/clang-g++.cfg
+// RUN: touch %t/testdmode/clang.cfg
+// RUN: %t/testdmode/x86_64-unknown-linux-gnu-clang-g++ --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1
+//
+// FULL1-NOT: Configuration file:
+// FULL1: Configuration file: {{.*}}/testdmode/x86_64-unknown-linux-gnu-clang++.cfg
+// FULL1-NOT: Configuration file:
+
+//--- -m32 overrides triple.
 //
-// FULL-NAME: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
-// FULL-NAME: -Wundefined-func-template
-// FULL-NAME-NOT: -Werror
+// RUN: %t/testdmode/x86_64-unknown-linux-gnu-clang-g++ -m32 --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1-I386
 //
-//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg even without -no-canonical-prefixes.
-// (As the clang executable and symlink are in different directories, this
-// requires specifying the path via --config-*-dir= though.)
+// FULL1-I386-NOT: Configuration file:
+// FULL1-I386: Configuration file: {{.*}}/testdmode/i386-unknown-linux-gnu-clang++.cfg
+// FULL1-I386-NOT: Configuration file:
+
+//--- --target= also works for overriding triple.
 //
-// RUN: %t/testdmode/qqq-clang-g++ --config-system-dir= --config-user-dir=%t/testdmode -c -### %s 2>&1 | FileCheck %s -check-prefix SYMLINK
+// RUN: %t/testdmode/x86_64-unknown-linux-gnu-clang-g++ --target=i386-unknown-linux-gnu --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1-I386
+
+//--- With --target= + -m64, -m64 takes precedence.
+//
+// RUN: %t/testdmode/x86_64-unknown-linux-gnu-clang-g++ --target=i386-unknown-linux-gnu -m64 --config-system-dir= --config-user-dir= -no-canonical-prefixes --version 2>&1 | FileCheck %s -check-prefix FULL1
+
+//--- i386 prefix also works for 32-bit.
 //
-// SYMLINK: Configuration file: {{.*}}/testdmode/qqq-clang-g++.cfg
+// RUN: %t/te

[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

2022-09-28 Thread Michał Górny via Phabricator via cfe-commits
mgorny added a comment.

@sepavloff, @arichardson, does this approach look OK? I've included the 
cheribsd name in tests.


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

https://reviews.llvm.org/D134337

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


[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

2022-09-28 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 463575.
balazske marked 6 inline comments as done.
balazske added a comment.

Implemented review comments, changed warning text, changed documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133119

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousReallocUsageCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousReallocUsageCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/suspicious-realloc-usage.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-realloc-usage.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-realloc-usage.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-realloc-usage.cpp
@@ -0,0 +1,79 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-realloc-usage %t
+
+void *realloc(void *, __SIZE_TYPE__);
+
+namespace std {
+  using ::realloc;
+}
+
+namespace n1 {
+  void *p;
+}
+
+namespace n2 {
+  void *p;
+}
+
+struct P {
+  void *p;
+  void *q;
+  P *pp;
+  void *&f();
+};
+
+struct P1 {
+  static void *p;
+  static void *q;
+};
+
+template
+struct P2 {
+  static void *p;
+  static void *q;
+};
+
+template
+void templ(void *p) {
+  A::p = realloc(A::p, 10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'A::p' may be set to null if 'realloc' fails, which may result in a leak of the original buffer [bugprone-suspicious-realloc-usage]
+  p = realloc(A::p, 10);
+  A::q = realloc(A::p, 10);
+  A::p = realloc(B::p, 10);
+  P2::p = realloc(P2::p, 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 'P2::p' may be set to null if 'realloc' fails, which may result in a leak of the original buffer [bugprone-suspicious-realloc-usage]
+  P2::p = realloc(P2::p, 1);
+}
+
+void *&getPtr();
+P &getP();
+
+void foo(void *p, P *p1, int *pi) {
+  p = realloc(p, 111);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'p' may be set to null if 'realloc' fails, which may result in a leak of the original buffer [bugprone-suspicious-realloc-usage]
+
+  p = std::realloc(p, sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'p' may be set to null if 'realloc' fails, which may result in a leak of the original buffer [bugprone-suspicious-realloc-usage]
+
+  p1->p = realloc(p1->p, 10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: 'p1->p' may be set to null if 'realloc' fails, which may result in a leak of the original buffer [bugprone-suspicious-realloc-usage]
+
+  p1->pp->p = realloc(p1->pp->p, 10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 'p1->pp->p' may be set to null if 'realloc' fails, which may result in a leak of the original buffer [bugprone-suspicious-realloc-usage]
+
+  pi = (int*)realloc(pi, 10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 'pi' may be set to null if 'realloc' fails, which may result in a leak of the original buffer [bugprone-suspicious-realloc-usage]
+
+  templ>(p);
+}
+
+void no_warn(void *p, P *p1, P *p2) {
+  void *q = realloc(p, 10);
+  q = realloc(p, 10);
+  p1->q = realloc(p1->p, 10);
+  p2->p = realloc(p1->p, 20);
+  n1::p = realloc(n2::p, 30);
+  p1->pp->p = realloc(p1->p, 10);
+  getPtr() = realloc(getPtr(), 30);
+  getP().p = realloc(getP().p, 20);
+  p1->f() = realloc(p1->f(), 30);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -124,6 +124,7 @@
`bugprone-suspicious-memory-comparison `_,
`bugprone-suspicious-memset-usage `_, "Yes"
`bugprone-suspicious-missing-comma `_,
+   `bugprone-suspicious-realloc-usage `_,
`bugprone-suspicious-semicolon `_, "Yes"
`bugprone-suspicious-string-compare `_, "Yes"
`bugprone-swapped-arguments `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/suspicious-realloc-usage.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/suspicious-realloc-usage.rst
@@ -0,0 +1,32 @@
+.. title:: clang-tidy - bugprone-suspicious-realloc-usage
+
+bugprone-suspicious-realloc-usage
+=
+
+This check finds usages of ``realloc`` where the return value is assigned to the
+same expression as passed to the first argument:
+``p = realloc(p, size);``
+The problem with this construct is that if ``realloc`` fails it returns a
+null pointer but does not deallocate the original memory. If no other variable
+is pointing to it, the original memory block is not available any more for the
+program to use or free. In eithe

[clang] 3d20806 - [clang][DR2621] using enum NAME lookup fix

2022-09-28 Thread Nathan Sidwell via cfe-commits

Author: Nathan Sidwell
Date: 2022-09-28T08:50:27-07:00
New Revision: 3d2080683f1dc37010fb56cf7d0e1632cda00f15

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

LOG: [clang][DR2621] using enum NAME lookup fix

Although using-enum's grammar is 'using elaborated-enum-specifier',
the lookup for the enum is ordinary lookup (and not the tagged-type
lookup that normally occurs wth an tagged-type specifier).  Thus (a)
we can find typedefs and (b) do not find enum tags hidden by a non-tag
name (the struct stat thing).

This reimplements that part of using-enum handling, to address DR2621,
where clang's behaviour does not match std intent (and other
compilers).

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/drs/dr26xx.cpp
clang/test/CodeCompletion/using-enum.cpp
clang/test/Parser/cxx20-using-enum.cpp
clang/test/SemaCXX/cxx20-using-enum.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0dd3b54aa4753..0bae33cdf4234 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -356,6 +356,8 @@ C++20 Feature Support
   the time of checking, which should now allow the libstdc++ ranges 
implementation
   to work for at least trivial examples.  This fixes
   `Issue 44178 `_.
+- Clang implements DR2621, correcting a defect in ``using enum`` handling.  The
+  name is found via ordinary lookup so typedefs are found.
 
 C++2b Feature Support
 ^

diff  --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 2abb5f09292ec..ee401200ddfa5 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -607,6 +607,9 @@ def warn_cxx17_compat_using_enum_declaration : Warning<
 def ext_using_enum_declaration : ExtWarn<
   "using enum declaration is a C++20 extension">,
   InGroup;
+def err_using_enum_expect_identifier : Error<
+  "using enum %select{requires an enum or typedef name|"
+  "does not permit an elaborated enum specifier}0">;
 def err_constructor_bad_name : Error<
   "missing return type for function %0; did you mean the constructor name 
%1?">;
 def err_destructor_tilde_identifier : Error<

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 93c598b2eba48..2b544928ab763 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -562,6 +562,8 @@ def warn_cxx17_compat_using_decl_class_member_enumerator : 
Warning<
   "with C++ standards before C++20">, InGroup, DefaultIgnore;
 def err_using_enum_is_dependent : Error<
   "using-enum cannot name a dependent type">;
+def err_using_enum_not_enum : Error<
+  "%0 is not an enumerated type">;
 def err_ambiguous_inherited_constructor : Error<
   "constructor of %0 inherited from multiple base class subobjects">;
 def note_ambiguous_inherited_constructor_using : Note<

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index b6fd8174e1d87..72995befeadf2 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -6125,7 +6125,9 @@ class Sema final {
   const ParsedAttributesView &AttrList);
   Decl *ActOnUsingEnumDeclaration(Scope *CurScope, AccessSpecifier AS,
   SourceLocation UsingLoc,
-  SourceLocation EnumLoc, const DeclSpec &);
+  SourceLocation EnumLoc,
+  SourceLocation IdentLoc, IdentifierInfo &II,
+  CXXScopeSpec *SS = nullptr);
   Decl *ActOnAliasDeclaration(Scope *CurScope, AccessSpecifier AS,
   MultiTemplateParamsArg TemplateParams,
   SourceLocation UsingLoc, UnqualifiedId &Name,

diff  --git a/clang/lib/Parse/ParseDeclCXX.cpp 
b/clang/lib/Parse/ParseDeclCXX.cpp
index d0bf89799c258..52e627a375442 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -678,6 +678,8 @@ bool Parser::ParseUsingDeclarator(DeclaratorContext Context,
 ///
 /// using-enum-declaration: [C++20, dcl.enum]
 ///   'using' elaborated-enum-specifier ;
+///   The terminal name of the elaborated-enum-specifier undergoes
+///   or

[PATCH] D134283: [clang][DR2621] using enum NAME lookup fix

2022-09-28 Thread Nathan Sidwell 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 rG3d2080683f1d: [clang][DR2621] using enum NAME lookup fix 
(authored by urnathan).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D134283?vs=462951&id=463578#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134283

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/drs/dr26xx.cpp
  clang/test/CodeCompletion/using-enum.cpp
  clang/test/Parser/cxx20-using-enum.cpp
  clang/test/SemaCXX/cxx20-using-enum.cpp

Index: clang/test/SemaCXX/cxx20-using-enum.cpp
===
--- clang/test/SemaCXX/cxx20-using-enum.cpp
+++ clang/test/SemaCXX/cxx20-using-enum.cpp
@@ -8,7 +8,7 @@
 enum A { a, // expected-note{{declared here}}
  b,
  c };
-class C; // expected-note{{previous use}}
+class C;
 enum class D : int;
 enum class D { d,
e,
@@ -20,11 +20,11 @@
 #if __cplusplus < 202002
 // expected-warning@-2{{is a C++20 extension}}
 #endif
-using enum Bob::B; // expected-error{{no enum named 'B'}}
+using enum Bob::B; // expected-error{{unknown type name B}}
 #if __cplusplus < 202002
 // expected-warning@-2{{is a C++20 extension}}
 #endif
-using enum Bob::C; // expected-error{{tag type that does not match}}
+using enum Bob::C; // expected-error{{'Bob::C' is not an enumerated type}}
 #if __cplusplus < 202002
 // expected-warning@-2{{is a C++20 extension}}
 #endif
@@ -38,6 +38,16 @@
 #if __cplusplus < 202002
 // expected-warning@-2{{is a C++20 extension}}
 #endif
+
+void DR2621() {
+  using A_t = Bob::A;
+  using enum A_t;
+#if __cplusplus < 202002
+// expected-warning@-2{{is a C++20 extension}}
+#endif
+  A_t x = a;
+}
+
 } // namespace One
 
 namespace Two {
Index: clang/test/Parser/cxx20-using-enum.cpp
===
--- clang/test/Parser/cxx20-using-enum.cpp
+++ clang/test/Parser/cxx20-using-enum.cpp
@@ -4,9 +4,9 @@
 namespace A {}
 
 void f() {
-  using enum A::+; // expected-error {{expected identifier}}
-  using enum; // expected-error {{expected identifier or '{'}}
-  using enum class; // expected-error {{expected identifier or '{'}}
-  using enum : blah; // expected-error {{unknown type name 'blah'}} expected-error {{unnamed enumeration must be a definition}}
+  using enum A::+; // expected-error {{using enum requires an enum or typedef name}}
+  using enum; // expected-error {{using enum requires an enum or typedef name}}
+  using enum class; // expected-error {{using enum requires an enum or typedef name}}
+  using enum enum q; // expected-error {{using enum does not permit an elaborated enum specifier}}
 }
 }
Index: clang/test/CodeCompletion/using-enum.cpp
===
--- clang/test/CodeCompletion/using-enum.cpp
+++ clang/test/CodeCompletion/using-enum.cpp
@@ -2,6 +2,6 @@
 
 namespace N2 {
   using enum AAA;
-  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:4:14 %s | FileCheck -check-prefix=CHECK-CC1 %s
+  // RUN: %clang_cc1 -std=c++20 -fsyntax-only -code-completion-at=%s:4:14 %s | FileCheck -check-prefix=CHECK-CC1 %s
   // CHECK-CC1: COMPLETION: AAA
 };
Index: clang/test/CXX/drs/dr26xx.cpp
===
--- clang/test/CXX/drs/dr26xx.cpp
+++ clang/test/CXX/drs/dr26xx.cpp
@@ -1,5 +1,19 @@
 // RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown %s -verify
 
+namespace dr2621 { // dr2621: yes
+enum class E { a };
+namespace One {
+using E_t = E;
+using enum E_t; // typedef ok
+auto v = a;
+}
+namespace Two {
+using dr2621::E;
+int E; // we see this
+using enum E; // expected-error {{unknown type name E}}
+}
+}
+
 namespace dr2628 { // dr2628: yes
 
 template 
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -11851,30 +11851,30 @@
 Decl *Sema::ActOnUsingEnumDeclaration(Scope *S, AccessSpecifier AS,
   SourceLocation UsingLoc,
   SourceLocation EnumLoc,
-  const DeclSpec &DS) {
-  switch (DS.getTypeSpecType()) {
-  case DeclSpec::TST_error:
-// This will already have been diagnosed
+  SourceLocation IdentLoc,
+  IdentifierInfo &II, CXXScopeSpec *SS) {
+  assert(!SS->isInvalid() && "ScopeSpec is invalid");
+  ParsedType TypeRep = getTypeName(II, I

[PATCH] D134578: Add missing `struct` keyword to the test p2-2.cpp

2022-09-28 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

Is this the only thing that blocks D53847 ?
I suggest to stamp this if so (happy to do it myself). In case @ChuanqiXu will 
have comments we can address them in post-commit review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134578

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


[PATCH] D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods.

2022-09-28 Thread Puyan Lotfi via Phabricator via cfe-commits
plotfi added a comment.

In D86049#3819109 , @mwyman wrote:

> In D86049#3818981 , @plotfi wrote:
>
>> @ahatanak I can revive some of what I was working on from 
>> https://reviews.llvm.org/D86049?id=285923 if we think we need a thunk for 
>> the checks as @rjmccall mentioned.
>
> I believe the generated direct methods already handle the null checks and 
> class init in `CGObjCCommonMac::GenerateDirectMethodPrologue`, meaning the 
> thunks aren't strictly necessary for the callee to handle them.
>
> Could the thunks instead allow us to have publicly-visible mangled names 
> (something akin to the new selector stubs `_objc_msgSend$selectorName` but 
> for `_objc_direct$Class_selectorName`) while leaving the actual impl name 
> alone, letting the stack traces see normal ObjC symbol names?

I think the square brackets are still problematic for lining, so is LLVM's 
handling of \01 (I believe).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86049

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


[PATCH] D134813: Properly print unnamed TagDecl objects in diagnostics

2022-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision.
aaron.ballman added reviewers: erichkeane, jyknight.
Herald added subscribers: arphaman, delcypher.
Herald added a reviewer: dang.
Herald added a reviewer: ributzka.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: llvm-commits.

The diagnostics engine is very smart about being passed a `NamedDecl` to print 
as part of a diagnostic; it gets the "right" form of the name, quotes it 
properly, etc. However, the result of using an unnamed tag declaration was to 
print `''` instead of anything useful.

This patch causes us to print the same information we'd have gotten if we had 
printed the type of the declaration rather than the name of it, as that's the 
most relevant information we can display.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134813

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/DeclTemplate.h
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/AST/NestedNameSpecifier.cpp
  clang/lib/AST/TemplateName.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/AST/ast-dump-record-definition-data-json.cpp
  clang/test/Analysis/func-mapping-test.cpp
  clang/test/ExtractAPI/enum.c
  clang/test/ExtractAPI/typedef_anonymous_record.c
  clang/test/ExtractAPI/underscored.c
  clang/test/Index/Core/index-source.m
  clang/test/Index/annotate-comments-typedef.m
  clang/test/Index/c-index-api-loadTU-test.m
  clang/test/Index/c-index-getCursor-test.m
  clang/test/Index/cxx11-lambdas.cpp
  clang/test/Index/cxx14-lambdas.cpp
  clang/test/Index/print-type.c
  clang/test/Index/print-type.cpp
  clang/test/Index/targeted-annotation.c
  clang/test/Index/targeted-cursor.c
  clang/test/Index/usrs.cpp
  clang/test/Index/usrs.m
  clang/test/Sema/address-packed.c
  clang/test/Sema/attr-flag-enum.c
  clang/test/SemaCXX/attr-unused.cpp
  clang/test/SemaCXX/lambda-expressions.cpp
  clang/test/SemaCXX/ms-interface.cpp
  clang/test/SemaObjCXX/arc-0x.mm
  clang/test/Templight/templight-empty-entries-fix.cpp
  llvm/utils/lit/lit/TestRunner.py

Index: llvm/utils/lit/lit/TestRunner.py
===
--- llvm/utils/lit/lit/TestRunner.py
+++ llvm/utils/lit/lit/TestRunner.py
@@ -1174,6 +1174,7 @@
 ('%/p', sourcedir.replace('\\', '/')),
 ('%/t', tmpBase.replace('\\', '/') + '.tmp'),
 ('%/T', tmpDir.replace('\\', '/')),
+('%/et',tmpName.replace('\\', '')),
 ])
 
 # "%{/[STpst]:regex_replacement}" should be normalized like "%/[STpst]" but we're
Index: clang/test/Templight/templight-empty-entries-fix.cpp
===
--- clang/test/Templight/templight-empty-entries-fix.cpp
+++ clang/test/Templight/templight-empty-entries-fix.cpp
@@ -5,13 +5,13 @@
 }
 
 // CHECK-LABEL: {{^---$}}
-// CHECK: {{^name:[ ]+'lambda at .*templight-empty-entries-fix.cpp:4:3'$}}
+// CHECK: {{^name:[ ]+'\(lambda at .*templight-empty-entries-fix.cpp:4:3\)'$}}
 // CHECK: {{^kind:[ ]+Memoization$}}
 // CHECK: {{^event:[ ]+Begin$}}
 // CHECK: {{^orig:[ ]+'.*templight-empty-entries-fix.cpp:4:3'$}}
 // CHECK: {{^poi:[ ]+'.*templight-empty-entries-fix.cpp:4:3'$}}
 // CHECK-LABEL: {{^---$}}
-// CHECK: {{^name:[ ]+'lambda at .*templight-empty-entries-fix.cpp:4:3'$}}
+// CHECK: {{^name:[ ]+'\(lambda at .*templight-empty-entries-fix.cpp:4:3\)'$}}
 // CHECK: {{^kind:[ ]+Memoization$}}
 // CHECK: {{^event:[ ]+End$}}
 // CHECK: {{^orig:[ ]+'.*templight-empty-entries-fix.cpp:4:3'$}}
@@ -225,37 +225,37 @@
 }
 
 // CHECK-LABEL: {{^---$}}
-// CHECK: {{^name:[ ]+unnamed struct$}}
+// CHECK: {{^name:[ ]+'\(unnamed struct at .*templight-empty-entries-fix.cpp:223:3\)'$}}
 // CHECK: {{^kind:[ ]+Memoization$}}
 // CHECK: {{^event:[ ]+Begin$}}
 // CHECK: {{^orig:[ ]+'.*templight-empty-entries-fix.cpp:223:3'$}}
 // CHECK: {{^poi:[ ]+'.*templight-empty-entries-fix.cpp:224:5'$}}
 // CHECK-LABEL: {{^---$}}
-// CHECK: {{^name:[ ]+unnamed struct$}}
+// CHECK: {{^name:[ ]+'\(unnamed struct at .*templight-empty-entries-fix.cpp:223:3\)'$}}
 // CHECK: {{^kind:[ ]+Memoization$}}
 // CHECK: {{^event:[ ]+End$}}
 // CHECK: {{^orig:[ ]+'.*templight-empty-entries-fix.cpp:223:3'$}}
 // CHECK: {{^poi:[ ]+'.*templight-empty-entries-fix.cpp:224:5'$}}
 // CHECK-LABEL: {{^---$}}
-// CHECK: {{^name:[ ]+unnamed struct$}}
+// CHECK: {{^name:[ ]+'\(unnamed struct at .*templight-empty-entries-fix.cpp:223:3\)'$}}
 // CHECK: {{^kind:[ ]+Memoization$}}
 // CHECK: {{^event:[ ]+Begin$}}
 // CHECK: {{^orig:[ ]+'.*templight-empty-entries-fix.cpp:223:3'$}}
 // CHECK: {{^poi:[ ]+'.*templight-empty-entries-fix.cpp:224:5'$}}
 // CHECK-LABEL: {{^---$}}
-// CHECK: {{^name:[ ]+unnamed struct$}

[PATCH] D126481: [analyzer] Handle SymbolCast in SValBuilder

2022-09-28 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment.

In D126481#3818769 , @martong wrote:

> Yeah okay. I get it now.  Thank you for your patience and your time on 
> elaborating the issue.
>
> First, I think we'd need to fabricate a test case that shows us the bug even 
> without applying your patch (D103096 ).
>
> Then we can iterate onto the solution. What we could do is to collect the 
> constants and types on the way of the cast visitation and then apply the same 
> logic that you have in D103096 .

Suppose we have found the way to handle it. But what if we find a contradiction 
while simplifying, like `(short)(int x) = 0` and `(int x) = 1`. So that we 
would already know that it is impossible. What `SVal` should we return in such 
case? Undef? Unknown? Original one?

> But then there is an open question: what should we do if there is another 
> kind of symbol in the chain of SymbolCasts? E.g SymbolCast, UnarySymExpr, 
> SymbolCast.

IMO, it doesn't matter, since we are just waiting for a constant. Doesn't 
matter to what `Sym` it is associated. The main question, as I said, what we 
shall do with two different constants.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126481

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


[PATCH] D126481: [analyzer] Handle SymbolCast in SValBuilder

2022-09-28 Thread Denys Petrov via Phabricator via cfe-commits
ASDenysPetrov added a comment.

@martong

> Then we can iterate onto the solution. What we could do is to collect the 
> constants and types on the way of the cast visitation and then apply the same 
> logic that you have in D103096 .

Suppose we have found the way to handle it. But what if we find a contradiction 
while simplifying, like `(short)(int x) = 0` and `(int x) = 1`. So that we 
would already know that it is impossible. What `SVal` should we return in such 
case? Undef? Unknown? Original one?

> But then there is an open question: what should we do if there is another 
> kind of symbol in the chain of SymbolCasts? E.g SymbolCast, UnarySymExpr, 
> SymbolCast.

IMO, it doesn't matter, since we are just waiting for a constant. Doesn't 
matter to what `Sym` it is associated. The main question, as I said, what we 
shall do with two different constants.

Do you have any ideas?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126481

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


[PATCH] D134813: Properly print unnamed TagDecl objects in diagnostics

2022-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/test/ExtractAPI/enum.c:3
 // RUN: split-file %s %t
-// RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
+// RUN: sed -e "s@INPUT_FILE@%/et/input.h@g" -e 
"s@INPUT_DIR@%{/t:regex_replacement}@g" \
 // RUN: %t/reference.output.json.in >> %t/reference.output.json

This test's use of `diff` makes the diagnostic changes rather challenging. We 
can't diff the file path that's included when printing an unnamed object, 
because that path may be different from machine to machine. `diff` doesn't have 
any wildcard matching functionality to help here either. So we use `sed` to 
mutate the test file.. but the output uses the path of `%t` with escapes and we 
don't have a sed-compatible way to do that on Windows.

e.g., the output we want to match will contain 
`F:\\users\\aballman\\desktop\\test.c` on Windows but `%t` gives 
`F:\users\aballman\desktop\test.c` which sed turns into `F:sers ballman esktop 
est.c` or some other garbled form.

I'm hoping someone has a better way to approach this. I added `%/et` (for 
escaped %t) to get the behavior I needed, but I'd prefer not to modify lit for 
this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134813

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


[clang] 44ad670 - [clang][msan] Turn on -fsanitize-memory-param-retval by default

2022-09-28 Thread Arthur Eubanks via cfe-commits

Author: Arthur Eubanks
Date: 2022-09-28T09:36:39-07:00
New Revision: 44ad67031cc1a10483337f8b1f728e2be237685e

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

LOG: [clang][msan] Turn on -fsanitize-memory-param-retval by default

This eagerly reports use of undef values when passed to noundef
parameters or returned from noundef functions.

This also decreases binary sizes under msan.

To go back to the previous behavior, pass `-fno-sanitize-memory-param-retval`.

Reviewed By: vitalybuka, MaskRay

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Driver/Options.td
clang/include/clang/Driver/SanitizerArgs.h
clang/lib/Driver/SanitizerArgs.cpp
clang/test/CodeGen/kmsan-param-retval.c
clang/test/CodeGen/msan-param-retval.c
clang/test/Driver/fsanitize-memory-param-retval.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0bae33cdf423..ee31da39d197 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -447,10 +447,15 @@ Static Analyzer
   ``scanbuild`` was also updated accordingly.
   Passing these flags will result in a hard error.
 
-.. _release-notes-ubsan:
-
-Undefined Behavior Sanitizer (UBSan)
-
+.. _release-notes-sanitizers:
+
+Sanitizers
+--
+- ``-fsanitize-memory-param-retval`` is turned on by default. With
+  ``-fsanitize=memory``, passing uninitialized variables to functions and
+  returning uninitialized variables from functions is more aggressively
+  reported. ``-fno-sanitize-memory-param-retval`` restores the previous
+  behavior.
 
 Core Analysis Improvements
 ==

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 429ab38fe4af..61ffaf9d04db 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1767,7 +1767,7 @@ def sanitize_address_destructor_EQ
 defm sanitize_memory_param_retval
 : BoolFOption<"sanitize-memory-param-retval",
 CodeGenOpts<"SanitizeMemoryParamRetval">,
-DefaultFalse,
+DefaultTrue,
 PosFlag, NegFlag,
 BothFlags<[], " detection of uninitialized parameters and return 
values">>;
  Note: This flag was introduced when it was necessary to distinguish 
between

diff  --git a/clang/include/clang/Driver/SanitizerArgs.h 
b/clang/include/clang/Driver/SanitizerArgs.h
index 65677f79742b..52889c3fe189 100644
--- a/clang/include/clang/Driver/SanitizerArgs.h
+++ b/clang/include/clang/Driver/SanitizerArgs.h
@@ -34,7 +34,7 @@ class SanitizerArgs {
   int BinaryMetadataFeatures = 0;
   int MsanTrackOrigins = 0;
   bool MsanUseAfterDtor = true;
-  bool MsanParamRetval = false;
+  bool MsanParamRetval = true;
   bool CfiCrossDso = false;
   bool CfiICallGeneralizePointers = false;
   bool CfiCanonicalJumpTables = false;

diff  --git a/clang/lib/Driver/SanitizerArgs.cpp 
b/clang/lib/Driver/SanitizerArgs.cpp
index 631cf007da5d..edbb7625aaf2 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -1184,8 +1184,8 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const 
llvm::opt::ArgList &Args,
   if (MsanUseAfterDtor)
 CmdArgs.push_back("-fsanitize-memory-use-after-dtor");
 
-  if (MsanParamRetval)
-CmdArgs.push_back("-fsanitize-memory-param-retval");
+  if (!MsanParamRetval)
+CmdArgs.push_back("-fno-sanitize-memory-param-retval");
 
   // FIXME: Pass these parameters as function attributes, not as -llvm flags.
   if (!TsanMemoryAccess) {

diff  --git a/clang/test/CodeGen/kmsan-param-retval.c 
b/clang/test/CodeGen/kmsan-param-retval.c
index 3d952c01c7f7..dd7e1f5786c1 100644
--- a/clang/test/CodeGen/kmsan-param-retval.c
+++ b/clang/test/CodeGen/kmsan-param-retval.c
@@ -1,12 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 
-fsanitize=kernel-memory -no-enable-noundef-analysis -o - %s | \
 // RUN: FileCheck %s --check-prefix=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 
-fsanitize=kernel-memory -o - %s | \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 
-fsanitize=kernel-memory -fno-sanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,NOUNDEF_ONLY
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 
-fsanitize=kernel-memory -mllvm -msan-eager-checks -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 
-fsanitize=kernel-memory -no-enable-noundef-analysis 
-fsanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=CLEAN
-// RUN: %clang_cc1 -tri

[PATCH] D134669: [clang][msan] Turn on -fsanitize-memory-param-retval by default

2022-09-28 Thread Arthur Eubanks via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG44ad67031cc1: [clang][msan] Turn on 
-fsanitize-memory-param-retval by default (authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134669

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/CodeGen/kmsan-param-retval.c
  clang/test/CodeGen/msan-param-retval.c
  clang/test/Driver/fsanitize-memory-param-retval.c

Index: clang/test/Driver/fsanitize-memory-param-retval.c
===
--- clang/test/Driver/fsanitize-memory-param-retval.c
+++ clang/test/Driver/fsanitize-memory-param-retval.c
@@ -1,14 +1,14 @@
-// RUN: %clang -target i386-gnu-linux %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target aarch64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=kernel-memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target i386-gnu-linux %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target aarch64-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=kernel-memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
 
-// CHECK: "-fsanitize-memory-param-retval"
+// CHECK: "-fno-sanitize-memory-param-retval"
 
-// RUN: %clang -target aarch64-linux-gnu -fsyntax-only %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck --check-prefix=11 %s
-// 11: "-fsanitize-memory-param-retval"
+// RUN: %clang -target aarch64-linux-gnu -fsyntax-only %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck --check-prefix=11 %s
+// 11: "-fno-sanitize-memory-param-retval"
 
-// RUN: not %clang -target x86_64-linux-gnu -fsyntax-only %s -fsanitize=memory -fsanitize-memory-param-retval=1 2>&1 | FileCheck --check-prefix=EXCESS %s
-// EXCESS: error: unknown argument: '-fsanitize-memory-param-retval=
+// RUN: not %clang -target x86_64-linux-gnu -fsyntax-only %s -fsanitize=memory -fno-sanitize-memory-param-retval=1 2>&1 | FileCheck --check-prefix=EXCESS %s
+// EXCESS: error: unknown argument: '-fno-sanitize-memory-param-retval=
Index: clang/test/CodeGen/msan-param-retval.c
===
--- clang/test/CodeGen/msan-param-retval.c
+++ clang/test/CodeGen/msan-param-retval.c
@@ -1,12 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -no-enable-noundef-analysis -o - %s | \
 // RUN: FileCheck %s --check-prefix=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -o - %s | \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -fno-sanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,NOUNDEF_ONLY
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -mllvm -msan-eager-checks -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -no-enable-noundef-analysis -fsanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -fsanitize-memory-param-retval -o - %s | \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
 
 void bar(int x) {
Index: clang/test/CodeGen/kmsan-param-retval.c
===
--- clang/test/CodeGen/kmsan-param-retval.c
+++ clang/test/CodeGen/kmsan-param-retval.c
@@ -1,12 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 -fsanitize=kernel-memory -no-enable-noundef-analysis -o - %s | \
 // RUN: FileC

[clang] 4824059 - Moving some C papers around on the status page; NFC

2022-09-28 Thread Aaron Ballman via cfe-commits

Author: Aaron Ballman
Date: 2022-09-28T12:37:52-04:00
New Revision: 4824059c1a7d1b03c0e01b4ba4b0c53305e22d0a

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

LOG: Moving some C papers around on the status page; NFC

These three are basically related to the TS 18661 integration, so now
they're grouped there.

Added: 


Modified: 
clang/www/c_status.html

Removed: 




diff  --git a/clang/www/c_status.html b/clang/www/c_status.html
index 6333a0f07191..b8cd3e3ff6c0 100644
--- a/clang/www/c_status.html
+++ b/clang/www/c_status.html
@@ -659,7 +659,7 @@ C2x implementation status
   Clang 9
 
 
-  TS 18661 Integration
+  TS 18661 Integration
 

 https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2314.pdf";>N2314
@@ -681,6 +681,18 @@ C2x implementation status
 https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2546.pdf";>N2546
 Unknown
   
+  
+https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2580.htm";>N2580
+Unknown
+  
+   
+https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2640.htm";>N2640
+Unknown
+  
+   
+https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2755.htm";>N2755
+Unknown
+  
 
   Preprocessor line numbers unspecified
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2322.htm";>N2322
@@ -796,11 +808,6 @@ C2x implementation status
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2572.pdf";>N2572
   Partial
 
-
-  Decimal floating-point triples
-  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2580.htm";>N2580
-  Unknown
-
 
   Remove mixed wide string literal concatenation
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2594.htm";>N2594
@@ -832,11 +839,6 @@ C2x implementation status
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2626.pdf";>N2626
   Clang 13
 
-
-  Missing DEC_EVAL_METHOD
-  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2640.htm";>N2640
-  Unknown
-
 
   Missing +(x) in table
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2641.htm";>N2641
@@ -918,11 +920,6 @@ C2x implementation status
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2749.pdf";>N2749
   Unknown
 
-
-  Static initialization of DFP zeros
-  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2755.htm";>N2755
-  Unknown
-
 
   __has_include for C
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2799.pdf";>N2799



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


[PATCH] D134286: [C2x] implement typeof and typeof_unqual

2022-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Ping


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

https://reviews.llvm.org/D134286

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


[PATCH] D134453: Introduce the `AlwaysIncludeTypeForNonTypeTemplateArgument` into printing policy

2022-09-28 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

Moving this to a top level discussion because it's easier to write replies here 
than in an inline comment.

>> If it's that the tool is trying to get the type information purely from the 
>> string representation, maybe the tool should be doing things differently - 
>> relying on Clang's AST/type APIs, rather than on inspecting the resulting 
>> string?
>
> I completely agree, however, the documentation on type API is not very good, 
> and couldn't find a better way to get a string representation of the type 
> using this API, besides manually traversing the AST and concatenating bits 
> and pieces into a string which is essentially the same thing that TypePrinter 
> does...

Sorry, I'm confused - it sounded like you didn't actually want a string, though 
- you want the type information to make various semantic-aware choices in your 
tool, yes?

> On the other hand, inventing new data structures to hold the type information 
> or directly saving pointers to parts of the AST does not sound appealing to 
> me, at least not for the current level of complexity I'm aiming for.

Generally the way to do it, if you want to introspect into the type, its 
template parameters, etc, then yes, keeping pointers to the AST or reparsing 
the name with Clang as-needed, would be the way to go - or walking the AST 
up-front and generating your own data structure (not necessarily a string) with 
the data you want in some structured format. Relying on the string printing of 
Clang to produce enough introspective information for a static analysis tool I 
think would be at odds with other tradeoffs that stringification is trying to 
achieve.

>> But if the type as a string is correct as far as C++ is concerned, is that 
>> not enough information (necessarily - the C++ compiler can determine all the 
>> type information from the type as written with only the top level types in 
>> the NTTP names)?
>
> I think that the essential problem here is the as far as C++ is concerned? 
> What about the developer? Developers may have more benefit from getting a 
> full type name in their diagnostic than just something that is correct as far 
> as C++ is concerned.
>
> Consider this error message, which is completely useless and compare it to 
> the much more informative errors by GCC and MSVC that immediately tell you 
> what's wrong with your code.
>
> So, why shouldn't Clang behave the same way as well?

Again, I'm not advocating for the printing as-is, I think adding the top level 
name that disambiguates would be a good thing - and I think the GCC and MSVC 
examples somewhat show why adding all the other layers would be harmful to 
readability - there's a lot of text in those messages that doesn't directly 
help the user and gets in the way of identifying the important differences 
between the type names.

For instance, let's take the GCC message:

  : In function 'void foo()':
  :25:9: error: no match for 'operator=' (operand types are 
'NDArray{Dimension{0}}}>' and 'NDArray{Dimension{0}}}>')
 25 | a = b;
| ^
  :2:8: note: candidate: 'constexpr NDArray{Dimension{0}}}>& NDArray{Dimension{0}}}>::operator=(const NDArray{Dimension{0}}}>&)'
  2 | struct NDArray {};
|^~~
  :2:8: note:   no known conversion for argument 1 from 'NDArray{Dimension{0}}}>' to 'const NDArray{Dimension{0}}}>&'
  :2:8: note: candidate: 'constexpr NDArray{Dimension{0}}}>& NDArray{Dimension{0}}}>::operator=(NDArray{Dimension{0}}}>&&)'
  :2:8: note:   no known conversion for argument 1 from 'NDArray{Dimension{0}}}>' to 'NDArray{Dimension{0}}}>&&'

And compare it to:

  : In function 'void foo()':
  :25:9: error: no match for 'operator=' (operand types are 
'NDArray' and 'NDArray')
 25 | a = b;
| ^
  :2:8: note: candidate: 'constexpr NDArray& 
NDArray::operator=(const NDArray&)'
  2 | struct NDArray {};
|^~~
  :2:8: note:   no known conversion for argument 1 from 'NDArray' to 'const NDArray&'
  :2:8: note: candidate: 'constexpr NDArray& 
NDArray::operator=(NDArray&&)'
  :2:8: note:   no known conversion for argument 1 from 'NDArray' to 'NDArray&&'

That seems significantly easier to read/easier to identify the relevant 
difference for the developer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134453

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


[PATCH] D134813: Properly print unnamed TagDecl objects in diagnostics

2022-09-28 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision.
erichkeane added a comment.
This revision is now accepted and ready to land.

LGTM!




Comment at: clang/lib/AST/Decl.cpp:4480
+  // the tag is anonymous and we should print it differently.
+  if (Name.isIdentifier() && !Name.getAsIdentifierInfo()) {
+// If the caller wanted to print a qualified name, they've already printed

Do we want to assert that this is either an EnumDecl, or a RecordDecl AND 
RD.isAnonymousStructOrUnion?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134813

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


[PATCH] D133289: [C2X] N3007 Type inference for object definitions

2022-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/Parse/ParseExpr.cpp:1515
+// This is a temporary fix while we don't support C2x 6.5.2.5p4
+if (getLangOpts().C2x && GetLookAheadToken(2).getKind() == tok::l_brace) {
+  Diag(Tok, diag::err_c2x_auto_compound_literal_not_allowed);

to268 wrote:
> to268 wrote:
> > aaron.ballman wrote:
> > > Why would this not be handled from `Sema::ActOnCompoundLiteral()`?
> > You're right, it's better to handle this in `Sema::ActOnCompoundLiteral()`
> > The problem is that right now, the usage of the `auto` keyword in a 
> > compound literal isn't parsed as a compound literal. 
> > This compound literal is unable to reach `Sema::ActOnCompoundLiteral()` 
> > because the parser emits that it's an invalid expression.
> > 
> > To summarize, i'm unable to handle handle a compound literal that uses the 
> > `auto` keyword inside `Sema::ActOnCompoundLiteral()`
> > if it's never going to be called.
> > ```
> > int test_ncl = (int){12};   // Parsed as a CL
> > auto test_cl = (auto){12};  // Not parsed as a CL and emits 
> > "expected expression"
> > /*
> >  * |-DeclStmt 0x562180ede8b0 
> >  * | `-VarDecl 0x562180ede730  col:9 test_ncl 'int' cinit
> >  * |   `-ImplicitCastExpr 0x562180ede898  'int' 
> > 
> >  * | `-CompoundLiteralExpr 0x562180ede870  'int' lvalue
> >  * |   `-InitListExpr 0x562180ede828  'int'
> >  * | `-IntegerLiteral 0x562180ede7b0  'int' 12
> >  * |-DeclStmt 0x562180ede970 
> >  * | `-VarDecl 0x562180ede908  col:10 invalid test_cl 'auto'
> >  * `-ReturnStmt 0x562180ede9a8 
> >  *   `-IntegerLiteral 0x562180ede988  'int' 0
> >  */
> > ```
> When we are parsing an expression between parentheses in 
> `Parser::ParseParenExpression()` line 2972,
> we are calling `Parser::isTypeIdInParens()` which when using C will return 
> `true` if it's a type specifier,
> which is not the case for the `auto` keyword.
> 
> Ultimately, we fall into the conditional block of 
> `Parser::ParseParenExpression()` line 3191,
> which will return an `ExprError()` (AKA: a parsing error).
> 
> This is why we are unable to forbid a compound literal using the `auto` 
> keyword at the 
> Semantic Analysis stage and why this feature is not working out of the box in 
> the first place.
> When we are parsing an expression between parentheses in 
> Parser::ParseParenExpression() line 2972,
we are calling Parser::isTypeIdInParens() which when using C will return true 
if it's a type specifier,
which is not the case for the auto keyword.

A, that makes a lot of sense!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133289

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


[PATCH] D134286: [C2x] implement typeof and typeof_unqual

2022-09-28 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision.
erichkeane added a comment.
This revision is now accepted and ready to land.

LGTM.


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

https://reviews.llvm.org/D134286

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


[PATCH] D134578: Add missing `struct` keyword to the test p2-2.cpp

2022-09-28 Thread Alan Zhao via Phabricator via cfe-commits
ayzhao added a comment.

In D134578#3821178 , @ilya-biryukov 
wrote:

> Is this the only thing that blocks D53847 ?
> I suggest to stamp this if so (happy to do it myself). In case @ChuanqiXu 
> will have comments we can address them in post-commit review.

SG, I'll go ahead and land this and D53847 .

TBH this patch was probably trivial enough to not require


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134578

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


[clang] 6f2b347 - Add missing `struct` keyword to the test p2-2.cpp

2022-09-28 Thread Alan Zhao via cfe-commits

Author: Alan Zhao
Date: 2022-09-28T09:48:00-07:00
New Revision: 6f2b34789541ff95d7f339eac5dc031d29655a58

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

LOG: Add missing `struct` keyword to the test p2-2.cpp

While working on D53847, I noticed that this test would fail once we
started recognizing the types in the modified `export` statement [0].
The tests would fail because Clang would emit a "declaration does not
declare anything" diagnostic instead of the expected namespace scope
diagnostic.

I believe that the test is currently incorrectly passing because Clang
doesn't parse the type and therefore doesn't treat the statement as a
declaration. My understanding is that the intention of this test case is
that it wants to export a `struct` type, which I believe requires a
`struct` keyword, even for types with template parameters. With this
change, the only error with these two statements should be the
namespace scope issue.

[0]: https://reviews.llvm.org/D53847?id=462032#inline-1297053

Reviewed By: erichkeane

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

Added: 


Modified: 
clang/test/CXX/module/module.interface/p2-2.cpp

Removed: 




diff  --git a/clang/test/CXX/module/module.interface/p2-2.cpp 
b/clang/test/CXX/module/module.interface/p2-2.cpp
index 359e068d230af..04904a8d86f27 100644
--- a/clang/test/CXX/module/module.interface/p2-2.cpp
+++ b/clang/test/CXX/module/module.interface/p2-2.cpp
@@ -14,7 +14,7 @@ struct X {
   U bar();
 };
 
-export template  X::iterator;  // 
expected-error {{cannot export 'iterator' as it is not at namespace scope}}
+export template  struct X::iterator;   // 
expected-error {{cannot export 'iterator' as it is not at namespace scope}}
 export template  void X::foo();// 
expected-error {{cannot export 'foo' as it is not at namespace scope}}
 export template  template  U X::bar(); // 
expected-error {{cannot export 'bar' as it is not at namespace scope}}
 
@@ -32,6 +32,6 @@ export void Y::foo();// expected-error 
{{cannot export 'foo'
 export template  U Y::bar(); // expected-error {{cannot export 
'bar' as it is not at namespace scope}}
 
 export {
-  template  X::iterator; // expected-error {{cannot export 
'iterator' as it is not at namespace scope}}
-  struct Y::iterator;   // expected-error {{cannot export 
'iterator' as it is not at namespace scope}}
+  template  struct X::iterator; // expected-error {{cannot 
export 'iterator' as it is not at namespace scope}}
+  struct Y::iterator;  // expected-error {{cannot 
export 'iterator' as it is not at namespace scope}}
 }



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


[PATCH] D134578: Add missing `struct` keyword to the test p2-2.cpp

2022-09-28 Thread Alan Zhao via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6f2b34789541: Add missing `struct` keyword to the test 
p2-2.cpp (authored by ayzhao).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134578

Files:
  clang/test/CXX/module/module.interface/p2-2.cpp


Index: clang/test/CXX/module/module.interface/p2-2.cpp
===
--- clang/test/CXX/module/module.interface/p2-2.cpp
+++ clang/test/CXX/module/module.interface/p2-2.cpp
@@ -14,7 +14,7 @@
   U bar();
 };
 
-export template  X::iterator;  // 
expected-error {{cannot export 'iterator' as it is not at namespace scope}}
+export template  struct X::iterator;   // 
expected-error {{cannot export 'iterator' as it is not at namespace scope}}
 export template  void X::foo();// 
expected-error {{cannot export 'foo' as it is not at namespace scope}}
 export template  template  U X::bar(); // 
expected-error {{cannot export 'bar' as it is not at namespace scope}}
 
@@ -32,6 +32,6 @@
 export template  U Y::bar(); // expected-error {{cannot export 
'bar' as it is not at namespace scope}}
 
 export {
-  template  X::iterator; // expected-error {{cannot export 
'iterator' as it is not at namespace scope}}
-  struct Y::iterator;   // expected-error {{cannot export 
'iterator' as it is not at namespace scope}}
+  template  struct X::iterator; // expected-error {{cannot 
export 'iterator' as it is not at namespace scope}}
+  struct Y::iterator;  // expected-error {{cannot 
export 'iterator' as it is not at namespace scope}}
 }


Index: clang/test/CXX/module/module.interface/p2-2.cpp
===
--- clang/test/CXX/module/module.interface/p2-2.cpp
+++ clang/test/CXX/module/module.interface/p2-2.cpp
@@ -14,7 +14,7 @@
   U bar();
 };
 
-export template  X::iterator;  // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
+export template  struct X::iterator;   // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
 export template  void X::foo();// expected-error {{cannot export 'foo' as it is not at namespace scope}}
 export template  template  U X::bar(); // expected-error {{cannot export 'bar' as it is not at namespace scope}}
 
@@ -32,6 +32,6 @@
 export template  U Y::bar(); // expected-error {{cannot export 'bar' as it is not at namespace scope}}
 
 export {
-  template  X::iterator; // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
-  struct Y::iterator;   // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
+  template  struct X::iterator; // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
+  struct Y::iterator;  // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods.

2022-09-28 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

In D86049#3819109 , @mwyman wrote:

> In D86049#3818981 , @plotfi wrote:
>
>> @ahatanak I can revive some of what I was working on from 
>> https://reviews.llvm.org/D86049?id=285923 if we think we need a thunk for 
>> the checks as @rjmccall mentioned.
>
> I believe the generated direct methods already handle the null checks and 
> class init in `CGObjCCommonMac::GenerateDirectMethodPrologue`, meaning the 
> thunks aren't strictly necessary for the callee to handle them.

I don't know exactly what the thunk is supposed to do, but it sounds like the 
thunk is going to do the initialization by calling `objc_opt_self` and then 
call the direct class method. So the initialization isn't done inside the 
direct method.

If the thunk is inlined, the call to `objc_opt_self` can be optimized away if 
it we can prove `self` is already initialized. But we'd have to teach llvm to 
remove the call to `objc_opt_self` or add a new optimization pass to do so (see 
https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGObjCMac.cpp#L4057).

John, is that what you had in mind?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86049

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


[PATCH] D134578: Add missing `struct` keyword to the test p2-2.cpp

2022-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D134578#3821301 , @ayzhao wrote:

> In D134578#3821178 , @ilya-biryukov 
> wrote:
>
>> Is this the only thing that blocks D53847 ?
>> I suggest to stamp this if so (happy to do it myself). In case @ChuanqiXu 
>> will have comments we can address them in post-commit review.
>
> SG, I'll go ahead and land this and D53847 .
>
> TBH this patch was probably trivial enough to not require

FWIW, the changes here LGTM and I agree that this probably didn't require a 
review given the trivial nature of the changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134578

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


[clang] 4848f3b - [C++2a] P0634r3: Down with typename!

2022-09-28 Thread Alan Zhao via cfe-commits

Author: Nicolas Lesser
Date: 2022-09-28T09:50:19-07:00
New Revision: 4848f3bf2ff5ec57a8e2b8d3676c947dcf0fd735

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

LOG: [C++2a] P0634r3: Down with typename!

This patch implements P0634r3 that removes the need for 'typename' in certain 
contexts.

For example,

```
template 
using foo = T::type; // ok
```

This is also allowed in previous language versions as an extension, because I 
think it's pretty useful. :)

Reviewed By: #clang-language-wg, erichkeane

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

Added: 
clang/test/CXX/temp/temp.res/p4.cpp

Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/DeclSpec.h
clang/include/clang/Sema/Sema.h
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Parse/ParseExpr.cpp
clang/lib/Parse/ParseExprCXX.cpp
clang/lib/Parse/ParseTemplate.cpp
clang/lib/Parse/ParseTentative.cpp
clang/lib/Parse/Parser.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/test/CXX/basic/basic.lookup/basic.lookup.qual/class.qual/p2.cpp
clang/test/CXX/drs/dr1xx.cpp
clang/test/CXX/drs/dr2xx.cpp
clang/test/CXX/drs/dr4xx.cpp
clang/test/CXX/drs/dr5xx.cpp
clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp
clang/test/CXX/temp/temp.res/p3.cpp
clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p1.cpp
clang/test/FixIt/fixit.cpp
clang/test/Parser/cxx-member-initializers.cpp
clang/test/SemaCXX/MicrosoftCompatibility.cpp
clang/test/SemaCXX/MicrosoftExtensions.cpp
clang/test/SemaCXX/MicrosoftSuper.cpp
clang/test/SemaCXX/rounding-math-crash.cpp
clang/test/SemaCXX/typo-correction.cpp
clang/test/SemaCXX/unknown-type-name.cpp
clang/test/SemaTemplate/alias-templates.cpp
clang/test/SemaTemplate/typename-specifier-3.cpp
clang/www/cxx_status.html

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ee31da39d1978..b82aec630287b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -358,6 +358,8 @@ C++20 Feature Support
   `Issue 44178 `_.
 - Clang implements DR2621, correcting a defect in ``using enum`` handling.  The
   name is found via ordinary lookup so typedefs are found.
+- Implemented `P0634r3 
`_,
+  which removes the requirement for the ``typename`` keyword in certain 
contexts.
 
 C++2b Feature Support
 ^

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 2b544928ab763..1484622d8f8ae 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5432,6 +5432,12 @@ def err_typename_refers_to_using_value_decl : Error<
   "%0 in %1">;
 def note_using_value_decl_missing_typename : Note<
   "add 'typename' to treat this using declaration as a type">;
+def warn_cxx17_compat_implicit_typename : Warning<"use of implicit 'typename' 
is "
+  "incompatible with C++ standards before C++20">, InGroup,
+  DefaultIgnore;
+def ext_implicit_typename : ExtWarn<"missing 'typename' prior to dependent "
+  "type name %0%1; implicit 'typename' is a C++20 extension">,
+  InGroup;
 
 def err_template_kw_refers_to_non_template : Error<
   "%0%select{| following the 'template' keyword}1 "

diff  --git a/clang/include/clang/Parse/Parser.h 
b/clang/include/clang/Parse/Parser.h
index e6144ae01c779..cb396b8b54122 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -857,9 +857,12 @@ class Parser : public CodeCompletionHandler {
 public:
   // If NeedType is true, then TryAnnotateTypeOrScopeToken will try harder to
   // find a type name by attempting typo correction.
-  bool TryAnnotateTypeOrScopeToken();
-  bool TryAnnotateTypeOrScopeTokenAfterScopeSpec(CXXScopeSpec &SS,
- bool IsNewScope);
+  bool
+  TryAnnotateTypeOrScopeToken(ImplicitTypenameContext AllowImplicitTypename =
+  ImplicitTypenameContext::No);
+  bool TryAnnotateTypeOrScopeTokenAfterScopeSpec(
+  CXXScopeSpec &SS, bool IsNewScope,
+  ImplicitTypenameContext AllowImplicitTypename);
   bool TryAnnotateCXXScopeToken(bool EnteringContext = false);
 
   bool MightBeCXXScopeToken() {
@@ -885,7 +888,11 @@ class Parser : public CodeCompletionHandler {
 /// Annotation was successful.
 ANK_Success
   };
-  AnnotatedNameKind TryAnnotateN

[PATCH] D53847: [C++2a] P0634r3: Down with typename!

2022-09-28 Thread Alan Zhao 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 rG4848f3bf2ff5: [C++2a] P0634r3: Down with typename! (authored 
by Rakete, committed by ayzhao).

Changed prior to commit:
  https://reviews.llvm.org/D53847?vs=463351&id=463596#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D53847

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/CXX/basic/basic.lookup/basic.lookup.qual/class.qual/p2.cpp
  clang/test/CXX/drs/dr1xx.cpp
  clang/test/CXX/drs/dr2xx.cpp
  clang/test/CXX/drs/dr4xx.cpp
  clang/test/CXX/drs/dr5xx.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp
  clang/test/CXX/temp/temp.res/p3.cpp
  clang/test/CXX/temp/temp.res/p4.cpp
  clang/test/CXX/temp/temp.res/temp.dep/temp.dep.type/p1.cpp
  clang/test/FixIt/fixit.cpp
  clang/test/Parser/cxx-member-initializers.cpp
  clang/test/SemaCXX/MicrosoftCompatibility.cpp
  clang/test/SemaCXX/MicrosoftExtensions.cpp
  clang/test/SemaCXX/MicrosoftSuper.cpp
  clang/test/SemaCXX/rounding-math-crash.cpp
  clang/test/SemaCXX/typo-correction.cpp
  clang/test/SemaCXX/unknown-type-name.cpp
  clang/test/SemaTemplate/alias-templates.cpp
  clang/test/SemaTemplate/typename-specifier-3.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -959,11 +959,7 @@
   
   
 https://wg21.link/p2092r0";>P2092R0
-
-  Partial
-typename not yet optional (depends on P0634R3).
-  
-
+Clang 16
   
   
 https://wg21.link/p2113r0";>P2113R0
@@ -1052,7 +1048,7 @@
 
   typename optional in more contexts
   https://wg21.link/p0634r3";>P0634R3
-  No
+  Clang 16
 
 
   Pack expansion in lambda init-capture
Index: clang/test/SemaTemplate/typename-specifier-3.cpp
===
--- clang/test/SemaTemplate/typename-specifier-3.cpp
+++ clang/test/SemaTemplate/typename-specifier-3.cpp
@@ -28,7 +28,7 @@
   typedef int arg;
 };
 struct C {
-  typedef B::X x; // expected-error {{missing 'typename'}}
+  typedef B::X x; // precxx17-warning{{missing 'typename' prior to dependent type name B::X; implicit 'typename' is a C++20 extension}}
 };
   };
 
Index: clang/test/SemaTemplate/alias-templates.cpp
===
--- clang/test/SemaTemplate/alias-templates.cpp
+++ clang/test/SemaTemplate/alias-templates.cpp
@@ -193,11 +193,10 @@
   struct base {
 template  struct derived;
   };
-  // FIXME: The diagnostics here are terrible.
   template 
-  using derived = base::template derived; // expected-error {{expected a type}} expected-error {{expected ';'}}
+  using derived = base::template derived; // expected-warning {{missing 'typename'}}
   template 
-  using derived2 = ::PR16904::base::template derived; // expected-error {{expected a type}} expected-error {{expected ';'}}
+  using derived2 = ::PR16904::base::template derived; // expected-warning {{missing 'typename'}}
 }
 
 namespace PR14858 {
Index: clang/test/SemaCXX/unknown-type-name.cpp
===
--- clang/test/SemaCXX/unknown-type-name.cpp
+++ clang/test/SemaCXX/unknown-type-name.cpp
@@ -36,15 +36,15 @@
 
   static int n;
   static type m;
-  static int h(T::type, int); // expected-error{{missing 'typename'}}
-  static int h(T::type x, char); // expected-error{{missing 'typename'}}
+  static int h(T::type, int); // expected-warning{{implicit 'typename' is a C++20 extension}}
+  static int h(T::type x, char); // expected-warning{{implicit 'typename' is a C++20 extension}}
 };
 
 template
-A::type g(T t) { return t; } // expected-error{{missing 'typename'}}
+A::type g(T t) { return t; } // expected-warning{{implicit 'typename' is a C++20 extension}}
 
 template
-A::type A::f() { return type(); } // expected-error{{missing 'typename'}}
+A::type A::f() { return type(); } // expected-warning{{implicit 'typename' is a C++20 extension}}
 
 template
 void f(T::type) { } // expected-error{{missing 'typename'}}
@@ -84,11 +84,11 @@
 
 template int A::n(T::value); // ok
 template
-A::type // expected-error{{missing 'typename'}}
+A::type // expected-

[PATCH] D134361: [clang][Interp] Fix copy constructors of structs with array members

2022-09-28 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:649
+
+if (!ElemT)
+  return false;

Curious what case requires this check?



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:382
+  ByteCodeExprGen *Ctx;
+  Optional OldArrayIndex;
+};

Why an `Optional`? Your not checking it and I don't see how it won't be set?


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

https://reviews.llvm.org/D134361

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


[PATCH] D134361: [clang][Interp] Fix copy constructors of structs with array members

2022-09-28 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:649
+
+if (!ElemT)
+  return false;

shafik wrote:
> Curious what case requires this check?
I don't think there's a real test case for this, we could as well change the 
initializer to `*classify(SubExpr->getType());`. It's just the pattern used 
everywhere else. We could also use `classifyPrim` instead.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:382
+  ByteCodeExprGen *Ctx;
+  Optional OldArrayIndex;
+};

shafik wrote:
> Why an `Optional`? Your not checking it and I don't see how it won't be set?
The first time we see an `ArrayInitLoopExpr`, the 
`ByteCodeExprGen::ArrayIndex` will be `None`, so we need to use this 
here (note: just writing this right now, it's pretty late for me, I didn't test 
//just// using  a `uint64_t`)


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

https://reviews.llvm.org/D134361

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


[PATCH] D134815: [Sema] print more readable identifier of anonymous struct of -Wconsumed

2022-09-28 Thread YingChi Long via Phabricator via cfe-commits
inclyc created this revision.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
inclyc published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Working in D133574  we discovered -Wconsumed 
print '' with anonymous
class/struct. After this patch we give a line number, file name of
anonymous struct/class declaration.

Example:

  struct S {
struct {
  __attribute__((callable_when(consumed))) void func();
} s;
  };



  local/anoy-consume.cpp:3:20: warning: consumed analysis attribute is attached 
to member of class 'S::(unnamed struct at local/anoy-consume.cpp:2:3)' which 
isn't marked as consumable [-Wconsumed]
  __attribute__((callable_when(consumed))) void func();
 ^
  1 warning generated.

Link: https://reviews.llvm.org/D133574#3817743
Link: https://godbolt.org/z/16vP3voTW


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134815

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/SemaCXX/warn-consumed-parsing.cpp


Index: clang/test/SemaCXX/warn-consumed-parsing.cpp
===
--- clang/test/SemaCXX/warn-consumed-parsing.cpp
+++ clang/test/SemaCXX/warn-consumed-parsing.cpp
@@ -62,5 +62,10 @@
   Status {
 };
 
-
-
+class Anonymous {
+  struct /* anonymous */ {
+void callableWhen()   CALLABLE_WHEN("unconsumed"); // expected-warning-re 
{{consumed analysis attribute is attached to member of class 
'Anonymous::(unnamed struct at {{.*}})' which isn't marked as consumable}}
+void consumes()   SET_TYPESTATE(consumed); // expected-warning-re 
{{consumed analysis attribute is attached to member of class 
'Anonymous::(unnamed struct at {{.*}})' which isn't marked as consumable}}
+bool testUnconsumed() TEST_TYPESTATE(consumed); // expected-warning-re 
{{consumed analysis attribute is attached to member of class 
'Anonymous::(unnamed struct at {{.*}})' which isn't marked as consumable}}
+  } anonymous;
+};
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -1223,7 +1223,8 @@
 
   if (const CXXRecordDecl *RD = ThisType->getAsCXXRecordDecl()) {
 if (!RD->hasAttr()) {
-  S.Diag(AL.getLoc(), diag::warn_attr_on_unconsumable_class) << RD;
+  S.Diag(AL.getLoc(), diag::warn_attr_on_unconsumable_class)
+  << S.Context.getTagDeclType(RD);
 
   return false;
 }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -212,6 +212,8 @@
   of FAM-like arrays.
 - Clang now correctly diagnoses a warning when defercencing a void pointer in 
C mode.
   This fixes `Issue 53631 `_
+- Clang now prints more readable identifier for anonymous class/struct of
+  ``-Wconsumed``.
 
 Non-comprehensive list of changes in this release
 -


Index: clang/test/SemaCXX/warn-consumed-parsing.cpp
===
--- clang/test/SemaCXX/warn-consumed-parsing.cpp
+++ clang/test/SemaCXX/warn-consumed-parsing.cpp
@@ -62,5 +62,10 @@
   Status {
 };
 
-
-
+class Anonymous {
+  struct /* anonymous */ {
+void callableWhen()   CALLABLE_WHEN("unconsumed"); // expected-warning-re {{consumed analysis attribute is attached to member of class 'Anonymous::(unnamed struct at {{.*}})' which isn't marked as consumable}}
+void consumes()   SET_TYPESTATE(consumed); // expected-warning-re {{consumed analysis attribute is attached to member of class 'Anonymous::(unnamed struct at {{.*}})' which isn't marked as consumable}}
+bool testUnconsumed() TEST_TYPESTATE(consumed); // expected-warning-re {{consumed analysis attribute is attached to member of class 'Anonymous::(unnamed struct at {{.*}})' which isn't marked as consumable}}
+  } anonymous;
+};
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -1223,7 +1223,8 @@
 
   if (const CXXRecordDecl *RD = ThisType->getAsCXXRecordDecl()) {
 if (!RD->hasAttr()) {
-  S.Diag(AL.getLoc(), diag::warn_attr_on_unconsumable_class) << RD;
+  S.Diag(AL.getLoc(), diag::warn_attr_on_unconsumable_class)
+  << S.Context.getTagDeclType(RD);
 
   return false;
 }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -212,6 +212,8 @@
   of FAM-like arrays.
 - Clang now correctly diagnoses a warning when defercencing a void pointer in C mode.
   This fixes `Issue 53631 

[PATCH] D134815: [Sema] print more readable identifier of anonymous struct of -Wconsumed

2022-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

FWIW, I approached this in a slightly different way here: 
https://reviews.llvm.org/D134813 -- I've not landed it yet because I'm still 
hoping I can find a way to not modify lit just to fix one fragile test, but I 
think we ultimately want to go this other route because that fixes all of the 
places we pass in a `TagDecl` instead of playing whack-a-mole as we find new 
bad diagnostic behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134815

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


[PATCH] D86049: RFC: Implement optional exportable wrapper function generation for objc_direct methods.

2022-09-28 Thread Puyan Lotfi via Phabricator via cfe-commits
plotfi added a comment.

In D86049#3821183 , @plotfi wrote:

> In D86049#3819109 , @mwyman wrote:
>
>> In D86049#3818981 , @plotfi wrote:
>>
>>> @ahatanak I can revive some of what I was working on from 
>>> https://reviews.llvm.org/D86049?id=285923 if we think we need a thunk for 
>>> the checks as @rjmccall mentioned.
>>
>> I believe the generated direct methods already handle the null checks and 
>> class init in `CGObjCCommonMac::GenerateDirectMethodPrologue`, meaning the 
>> thunks aren't strictly necessary for the callee to handle them.
>>
>> Could the thunks instead allow us to have publicly-visible mangled names 
>> (something akin to the new selector stubs `_objc_msgSend$selectorName` but 
>> for `_objc_direct$Class_selectorName`) while leaving the actual impl name 
>> alone, letting the stack traces see normal ObjC symbol names?
>
> I think the square brackets are still problematic for linking, so is LLVM's 
> handling of \01 (I believe).

@mwyman @ahatanak The mangling code change is to appease ld64 by the way:

https://github.com/apple-oss-distributions/ld64/blob/ld64-609/src/ld/Options.cpp#L1378-L1408

The wildCardMatch function does some symbol stripping off of '?' and '['. I 
alter ']' just for stylistic consistency however. And the dropping of prefix 
byte is so that you get the preceding underbar that all visible Darwin symbols 
seem to need to have (but I am not as certain on that one).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86049

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


[PATCH] D134804: [clang][Interp] Implement bitwise Not operations

2022-09-28 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments.



Comment at: clang/test/AST/Interp/literals.cpp:72
+static_assert(~-1 == 0, "");
+static_assert(~255 == -256, "");
+

Some more tests covering `~INT_MIN == INT_MAX` and vice versa and unscoped enum 
case as well e.g.

```
enum E {};
E e = static_cast(0);
static_assert(~e == -1, "");
```


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

https://reviews.llvm.org/D134804

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


[PATCH] D134804: [clang][Interp] Implement bitwise Not operations

2022-09-28 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik accepted this revision.
shafik added a comment.
This revision is now accepted and ready to land.

Otherwise LGTM


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

https://reviews.llvm.org/D134804

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


[PATCH] D134311: [clang] handle extended integer constant expressions in _Static_assert (PR #57687)

2022-09-28 Thread Martin Sebor via Phabricator via cfe-commits
msebor updated this revision to Diff 463607.
msebor marked an inline comment as done.
msebor added a comment.

Changes from previous version:

- Replace loop with `Expr::IgnoreImpCasts()`.
- Use a multiline comment in a test to improve readability.


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

https://reviews.llvm.org/D134311

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/Sema/static-assert.c


Index: clang/test/Sema/static-assert.c
===
--- clang/test/Sema/static-assert.c
+++ clang/test/Sema/static-assert.c
@@ -74,3 +74,23 @@
 
 _Static_assert(1 , "") // expected-error {{expected ';' after 
'_Static_assert'}} \
   // ext-warning {{'_Static_assert' is a C11 extension}}
+
+static int static_var;
+_Static_assert(&static_var != 0, "");  // ext-warning {{'_Static_assert' is a 
C11 extension}} \
+   // expected-warning {{comparison of 
address of 'static_var' not equal to a null pointer is always true}}
+_Static_assert("" != 0, "");   // ext-warning {{'_Static_assert' is a 
C11 extension}}
+_Static_assert(("" != 0), ""); // ext-warning {{'_Static_assert' is a 
C11 extension}}
+_Static_assert(*"1", "");  // ext-warning {{'_Static_assert' is a 
C11 extension}}
+_Static_assert("1"[0], "");// ext-warning {{'_Static_assert' is a 
C11 extension}}
+_Static_assert(1.0 != 0, "");  // ext-warning {{'_Static_assert' is a 
C11 extension}}
+_Static_assert(__builtin_strlen("1"), "");  // ext-warning {{'_Static_assert' 
is a C11 extension}}
+#ifndef __cplusplus
+// ext-warning@-9 {{expression is not an integer constant expression; folding 
it to a constant is a GNU extension}}
+// ext-warning@-8 {{expression is not an integer constant expression; folding 
it to a constant is a GNU extension}}
+// ext-warning@-8 {{expression is not an integer constant expression; folding 
it to a constant is a GNU extension}}
+// ext-warning@-8 {{expression is not an integer constant expression; folding 
it to a constant is a GNU extension}}
+// ext-warning@-8 {{expression is not an integer constant expression; folding 
it to a constant is a GNU extension}}
+// ext-warning@-8 {{expression is not an integer constant expression; folding 
it to a constant is a GNU extension}}
+// __builtin_strlen(literal) is considered an integer constant expression
+// and doesn't cause a pedantic warning
+#endif
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -16736,10 +16736,21 @@
   AssertExpr = FullAssertExpr.get();
 
 llvm::APSInt Cond;
+Expr *BaseExpr = AssertExpr;
+AllowFoldKind FoldKind = NoFold;
+
+if (!getLangOpts().CPlusPlus) {
+  // In C mode only allow folding and strip the implicit conversion
+  // to the type of the first _Static_assert argument that would
+  // otherwise suppress diagnostics for arguments that convert to int.
+  FoldKind = AllowFold;
+  BaseExpr = BaseExpr->IgnoreImpCasts();
+}
+
 if (!Failed && VerifyIntegerConstantExpression(
-   AssertExpr, &Cond,
-   diag::err_static_assert_expression_is_not_constant)
-   .isInvalid())
+   BaseExpr, &Cond,
+   diag::err_static_assert_expression_is_not_constant,
+   FoldKind).isInvalid())
   Failed = true;
 
 if (!Failed && !Cond) {


Index: clang/test/Sema/static-assert.c
===
--- clang/test/Sema/static-assert.c
+++ clang/test/Sema/static-assert.c
@@ -74,3 +74,23 @@
 
 _Static_assert(1 , "") // expected-error {{expected ';' after '_Static_assert'}} \
   // ext-warning {{'_Static_assert' is a C11 extension}}
+
+static int static_var;
+_Static_assert(&static_var != 0, "");  // ext-warning {{'_Static_assert' is a C11 extension}} \
+   // expected-warning {{comparison of address of 'static_var' not equal to a null pointer is always true}}
+_Static_assert("" != 0, "");   // ext-warning {{'_Static_assert' is a C11 extension}}
+_Static_assert(("" != 0), ""); // ext-warning {{'_Static_assert' is a C11 extension}}
+_Static_assert(*"1", "");  // ext-warning {{'_Static_assert' is a C11 extension}}
+_Static_assert("1"[0], "");// ext-warning {{'_Static_assert' is a C11 extension}}
+_Static_assert(1.0 != 0, "");  // ext-warning {{'_Static_assert' is a C11 extension}}
+_Static_assert(__builtin_strlen("1"), "");  // ext-warning {{'_Static_assert' is a C11 extension}}
+#ifndef __cplusplus
+// ext-warning@-9 {{expression is not an integer constant expression; folding it to a constant is a GNU extension}}
+// ext-warning@-8 {{expression is not an intege

[PATCH] D134311: [clang] handle extended integer constant expressions in _Static_assert (PR #57687)

2022-09-28 Thread Martin Sebor via Phabricator via cfe-commits
msebor added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16733
+  while (auto *BaseCast = dyn_cast(BaseExpr))
+BaseExpr = BaseCast->getSubExpr();
+}

aaron.ballman wrote:
> tbaeder wrote:
> > There is `Expr::ignoreParenImpCasts()` or `Expr::ImpCasts()` that should do 
> > this loop for you.
> +1, I would use `Expr::IgnoreImpCasts()` instead of this manual loop.
Sure.  Thank you both for the suggestion!


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

https://reviews.llvm.org/D134311

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


[PATCH] D134454: [Driver][Distro] Fix ArchLinux sysroot detection

2022-09-28 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments.



Comment at: clang/lib/Driver/ToolChains/Linux.cpp:363
   const StringRef TripleStr = GCCInstallation.getTriple().str();
-  const Multilib &Multilib = GCCInstallation.getMultilib();
+  std::string Path = (InstallDir + "/../../../../" + TripleStr).str();
 

With all of the string concatenation going on, I wonder if `Path` should be a 
`Twine`?  `llvm::vfs::Filesystem::exists` accepts a `const Twine&`.  That 
avoids multiple reallocations and copies, and does one lazily.

(Every time I've tried to use `Twine`, I wind up with either `-Wdangling-gsl` 
or segfaults though! Still, please give it a shot.)



Comment at: clang/lib/Driver/ToolChains/Linux.cpp:369-375
+  if (getTriple().isCSKY())
+Path = Path + "/libc";
 
-  if (getVFS().exists(Path))
-return Path;
+  // Standalone MIPS toolchains use different names for sysroot folder
+  // and put it into different places. Here we try to check some known
+  // variants.
+  if (getTriple().isMIPS()) {

CSKY and MIPS should be mutually exclusive. Please make this second `if` an 
`else if`.

```
if csky:
  ...
elif mips:
  ...
```



Comment at: clang/lib/Driver/ToolChains/Linux.cpp:370
+  if (getTriple().isCSKY())
+Path = Path + "/libc";
 

+=



Comment at: clang/lib/Driver/ToolChains/Linux.cpp:373
+  // Standalone MIPS toolchains use different names for sysroot folder
+  // and put it into different places. Here we try to check some known
+  // variants.

Specifically there's 2 known variants for mips, it looks like. Please update 
this comment.



Comment at: clang/lib/Driver/ToolChains/Linux.cpp:376
+  if (getTriple().isMIPS()) {
+const Multilib &Multilib = GCCInstallation.getMultilib();
+Path = Path + "/libc" + Multilib.osSuffix();

It might be worthwhile to have the variable be
```
std::string OSSuffix = GCCInstallation.getMultilib().osSuffix();
```
rather than the `Multilib` object.



Comment at: clang/lib/Driver/ToolChains/Linux.cpp:377
+const Multilib &Multilib = GCCInstallation.getMultilib();
+Path = Path + "/libc" + Multilib.osSuffix();
+if (getVFS().exists(Path))

+=



Comment at: clang/lib/Driver/ToolChains/Linux.cpp:377-389
   const StringRef InstallDir = GCCInstallation.getInstallPath();
   const StringRef TripleStr = GCCInstallation.getTriple().str();
   const Multilib &Multilib = GCCInstallation.getMultilib();
+  std::string BasePath = (InstallDir + "/../../../../" + TripleStr).str();
 
-  std::string Path =
-  (InstallDir + "/../../../../" + TripleStr + "/libc" + 
Multilib.osSuffix())
-  .str();
-
-  if (getVFS().exists(Path))
-return Path;
+  if (Distro.IsArchLinux() && getVFS().exists(BasePath))
+return BasePath;

10ne1 wrote:
> nickdesaulniers wrote:
> > ...seems like some of this is duplicated with CSKY above. Can you please 
> > refactor the MIPS and CSKY checks to reuse more code?
> > 
> > I doubt arch-linux has a CSKY port; I suspect you might be able to check 
> > for arch-linux first, then do the CSKY and MIPS special casing after.
> > 
> > All this code is doing is figuring out a Path, then if it exists then 
> > return it.  Feel like is should be:
> > 
> > ```
> > if android:
> >   path = ...
> > elif arch:
> >   path = ...
> > elif csky:
> >   path = ...
> > elif mips:
> >   path = ...
> > else:
> >   return ""
> > 
> > if path.exists():
> >   return path
> > return ""
> > ```
> Thanks for this suggestion. Indeed after doing all the simplifications I have 
> some great news: we do not need to test if Distro == ArchLinux because the 
> sysroot path it uses is the implicit base path one common to all 
> architectures!
> 
> So just doing the following is enough to also fix ArchLinux:
> 
> ```
>   std::string Path = (InstallDir + "/../../../../" + TripleStr).str();
>   
>   if (getTriple().isCSKY())
> Path = Path + "/libc";
>   
>   if (getTriple().isMIPS()) {
> ...
>}
> 
>   if (getVFS().exists(Path))
> return Path;
> 
>   return std::string();
> ```
Yes, this is much nicer in that we don't have to special case arch-linux.  
Sometimes refactorings beget more refactorings; it's nice when that happens.


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

https://reviews.llvm.org/D134454

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


[clang] 60727d8 - [C2x] implement typeof and typeof_unqual

2022-09-28 Thread Aaron Ballman via cfe-commits

Author: Aaron Ballman
Date: 2022-09-28T13:27:52-04:00
New Revision: 60727d856927383daf304fcf8f19fcc8ade828ad

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

LOG: [C2x] implement typeof and typeof_unqual

This implements WG14 N2927 and WG14 N2930, which together define the
feature for typeof and typeof_unqual, which get the type of their
argument as either fully qualified or fully unqualified. The argument
to either operator is either a type name or an expression. If given a
type name, the type information is pulled directly from the given name.
If given an expression, the type information is pulled from the
expression. Recursive use of these operators is allowed and has the
expected behavior (the innermost operator is resolved to a type, and
that's used to resolve the next layer of typeof specifier, until a
fully resolved type is determined.

Note, we already supported typeof in GNU mode as a non-conforming
extension and we are *not* exposing typeof_unqual as a non-conforming
extension in that mode, nor are we exposing typeof or typeof_unqual as
a nonconforming extension in other language modes. The GNU variant of
typeof supports a form where the parentheses are elided from the
operator when given an expression (e.g., typeof 0 i = 12;). When in C2x
mode, we do not support this extension.

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

Added: 
clang/test/C/C2x/n2927.c
clang/test/C/C2x/n2927_2.c
clang/test/C/C2x/n2930.c
clang/test/Parser/c2x-typeof-ext-warns.c
clang/test/Parser/c2x-typeof.c
clang/test/Sema/c2x-typeof.c

Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/CanonicalType.h
clang/include/clang/AST/PropertiesBase.td
clang/include/clang/AST/RecursiveASTVisitor.h
clang/include/clang/AST/Type.h
clang/include/clang/AST/TypeLoc.h
clang/include/clang/AST/TypeProperties.td
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/Specifiers.h
clang/include/clang/Basic/TokenKinds.def
clang/include/clang/Sema/DeclSpec.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/ASTImporter.cpp
clang/lib/AST/ASTStructuralEquivalence.cpp
clang/lib/AST/ODRHash.cpp
clang/lib/AST/Type.cpp
clang/lib/AST/TypeLoc.cpp
clang/lib/AST/TypePrinter.cpp
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/ParseExpr.cpp
clang/lib/Sema/DeclSpec.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/lib/Sema/SemaTemplateDeduction.cpp
clang/lib/Sema/SemaTemplateVariadic.cpp
clang/lib/Sema/SemaType.cpp
clang/lib/Sema/TreeTransform.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/Lexer/keywords_test.c
clang/tools/libclang/CIndex.cpp
clang/www/c_status.html

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b82aec630287b..8c0c85bab2704 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -311,6 +311,22 @@ C2x Feature Support
   ``-Wunused-label`` warning.
 - Implemented `WG14 N2508 
`_,
   so labels can placed everywhere inside a compound statement.
+- Implemented `WG14 N2927 
`_,
+  the Not-so-magic ``typeof`` operator. Also implemented
+  `WG14 N2930 `_,
+  renaming ``remove_quals``, so the ``typeof_unqual`` operator is also
+  supported. Both of these operators are supported only in C2x mode. The
+  ``typeof`` operator specifies the type of the given parenthesized expression
+  operand or type name, including all qualifiers. The ``typeof_unqual``
+  operator is similar to ``typeof`` except that all qualifiers are removed,
+  including atomic type qualification and type attributes which behave like a
+  qualifier, such as an address space attribute.
+
+  .. code-block:: c
+
+__attribute__((address_space(1))) const _Atomic int Val;
+typeof(Val) OtherVal; // type is '__attribute__((address_space(1))) const 
_Atomic int'
+typeof_unqual(Val) OtherValUnqual; // type is 'int'
 
 C++ Language Changes in Clang
 -

diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index fb68842a34be4..787e4234d4ea9 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1714,9 +1714,9 @@ class ASTContext : public RefCountedBase {
   /// Return a ObjCObjectPointerType type for the gi

[PATCH] D134286: [C2x] implement typeof and typeof_unqual

2022-09-28 Thread Aaron Ballman 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 rG60727d856927: [C2x] implement typeof and typeof_unqual 
(authored by aaron.ballman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134286

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/CanonicalType.h
  clang/include/clang/AST/PropertiesBase.td
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/ODRHash.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/C/C2x/n2927.c
  clang/test/C/C2x/n2927_2.c
  clang/test/C/C2x/n2930.c
  clang/test/Lexer/keywords_test.c
  clang/test/Parser/c2x-typeof-ext-warns.c
  clang/test/Parser/c2x-typeof.c
  clang/test/Sema/c2x-typeof.c
  clang/tools/libclang/CIndex.cpp
  clang/www/c_status.html

Index: clang/www/c_status.html
===
--- clang/www/c_status.html
+++ clang/www/c_status.html
@@ -1083,17 +1083,11 @@
 

 https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2927.htm";>N2927
-
-  Partial
-Clang supports typeof in GNU standards mode, but its
-compatibility with this proposal is unknown. Also, Clang does not yet
-support remove_quals.
-  
-
+Clang 16
   

 https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2930.pdf";>N2930
-No
+Clang 16
   
 
   Type annex tgmath narrowing macros with integer args v2
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1817,7 +1817,7 @@
 }
 
 bool CursorVisitor::VisitTypeOfTypeLoc(TypeOfTypeLoc TL) {
-  if (TypeSourceInfo *TSInfo = TL.getUnderlyingTInfo())
+  if (TypeSourceInfo *TSInfo = TL.getUnmodifiedTInfo())
 return Visit(TSInfo->getTypeLoc());
 
   return false;
Index: clang/test/Sema/c2x-typeof.c
===
--- /dev/null
+++ clang/test/Sema/c2x-typeof.c
@@ -0,0 +1,94 @@
+// RUN: %clang_cc1 -verify -std=c2x %s
+
+// Demonstrate that we get the correct type information. Do this by leaning
+// heavily on redeclarations needing to use the same type for both decls.
+extern int i;
+extern typeof(i) i;
+extern typeof_unqual(i) i;
+
+extern const int j;
+extern typeof(j) j;
+
+extern const int n; // expected-note 2 {{previous declaration is here}}
+extern typeof(i) n; // expected-error {{redeclaration of 'n' with a different type: 'typeof (i)' (aka 'int') vs 'const int'}}
+extern typeof_unqual(n) n;  // expected-error {{redeclaration of 'n' with a different type: 'typeof_unqual (n)' (aka 'int') vs 'const int'}}
+
+// Ensure we get a redeclaration error here for the types not matching.
+extern typeof(j) k;// expected-note {{previous declaration is here}}
+extern typeof_unqual(j) k; // expected-error {{redeclaration of 'k' with a different type: 'typeof_unqual (j)' (aka 'int') vs 'typeof (j)' (aka 'const int')}}
+
+// Make sure the type-form of the operator also works.
+extern typeof(int) l;
+extern typeof_unqual(const int) l;
+
+extern typeof(const int) m;// expected-note {{previous declaration is here}}
+extern typeof_unqual(const int) m; // expected-error {{redeclaration of 'm' with a different type: 'typeof_unqual(const int)' (aka 'int') vs 'typeof(const int)' (aka 'const int')}}
+
+// Show that we can use an incomplete type which is then completed later.
+extern typeof(struct T) *o;
+struct T { int a; } t;
+extern typeof(struct T) *o;
+extern typeof(t) *o;
+extern typeof(&t) o;
+extern typeof_unqual(volatile struct T) *o;
+extern typeof_unqual(t) *o;
+extern typeof_unqual(&t) o;
+
+// Show that we properly strip the _Atomic qualifier.
+extern _Atomic int i2;
+extern _Atomic(int) i2;
+extern 

  1   2   >