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

[Anton Bikineev via Phabricator via cfe-commits]

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

Thanks everybody!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134456

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

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

[Anton Bikineev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7b85e765000d: [PGO] Consider parent context when weighing 
branches with likelyhood. (authored by AntonBikineev).

Changed prior to commit:
  https://reviews.llvm.org/D134456?vs=462250=466313#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134456

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
  clang/test/Profile/cxx-never-executed-branch.cpp


Index: clang/test/Profile/cxx-never-executed-branch.cpp
===
--- /dev/null
+++ clang/test/Profile/cxx-never-executed-branch.cpp
@@ -0,0 +1,32 @@
+// Test that clang doesn't emit llvm.expect when the counter is 0
+
+// RUN: llvm-profdata merge %S/Inputs/cxx-never-executed-branch.proftext -o 
%t.profdata
+// RUN: %clang_cc1 -std=c++20 %s -triple %itanium_abi_triple -O2 -o - 
-emit-llvm -fprofile-instrument-use-path=%t.profdata -disable-llvm-passes | 
FileCheck %s
+
+int rand();
+
+// CHECK: define {{.*}}@_Z13is_in_profilev
+// CHECK-NOT: call {{.*}}@llvm.expect
+
+int is_in_profile() {
+  int rando = rand();
+  int x = 0;
+  if (rando == 0) [[likely]]
+x = 2;
+  else
+x = 3;
+  return x;
+}
+
+// CHECK: define {{.*}}@_Z17is_not_in_profilev
+// CHECK: call {{.*}}@llvm.expect
+
+int is_not_in_profile() {
+  int rando = rand();
+  int x = 0;
+  if (rando == 0) [[likely]]
+x = 2;
+  else
+x = 3;
+  return x;
+}
Index: clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
===
--- /dev/null
+++ clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
@@ -0,0 +1,13 @@
+_Z13is_in_profilev
+0x00029f493458
+2
+2
+# The branch is never executed.
+0
+
+_Z17is_not_in_profilev
+0x00029f493458
+2
+# The function was never executed
+0
+0
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -815,11 +815,20 @@
   // Prefer the PGO based weights over the likelihood attribute.
   // When the build isn't optimized the metadata isn't used, so don't generate
   // it.
+  // Also, differentiate between disabled PGO and a never executed branch with
+  // PGO. Assuming PGO is in use:
+  // - we want to ignore the [[likely]] attribute if the branch is never
+  // executed,
+  // - assuming the profile is poor, preserving the attribute may still be
+  // beneficial.
+  // As an approximation, preserve the attribute only if both the branch and 
the
+  // parent context were not executed.
   Stmt::Likelihood LH = Stmt::LH_None;
-  uint64_t Count = getProfileCount(S.getThen());
-  if (!Count && CGM.getCodeGenOpts().OptimizationLevel)
+  uint64_t ThenCount = getProfileCount(S.getThen());
+  if (!ThenCount && !getCurrentProfileCount() &&
+  CGM.getCodeGenOpts().OptimizationLevel)
 LH = Stmt::getLikelihood(S.getThen(), S.getElse());
-  EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock, Count, LH);
+  EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock, ThenCount, LH);
 
   // Emit the 'then' code.
   EmitBlock(ThenBlock);


Index: clang/test/Profile/cxx-never-executed-branch.cpp
===
--- /dev/null
+++ clang/test/Profile/cxx-never-executed-branch.cpp
@@ -0,0 +1,32 @@
+// Test that clang doesn't emit llvm.expect when the counter is 0
+
+// RUN: llvm-profdata merge %S/Inputs/cxx-never-executed-branch.proftext -o %t.profdata
+// RUN: %clang_cc1 -std=c++20 %s -triple %itanium_abi_triple -O2 -o - -emit-llvm -fprofile-instrument-use-path=%t.profdata -disable-llvm-passes | FileCheck %s
+
+int rand();
+
+// CHECK: define {{.*}}@_Z13is_in_profilev
+// CHECK-NOT: call {{.*}}@llvm.expect
+
+int is_in_profile() {
+  int rando = rand();
+  int x = 0;
+  if (rando == 0) [[likely]]
+x = 2;
+  else
+x = 3;
+  return x;
+}
+
+// CHECK: define {{.*}}@_Z17is_not_in_profilev
+// CHECK: call {{.*}}@llvm.expect
+
+int is_not_in_profile() {
+  int rando = rand();
+  int x = 0;
+  if (rando == 0) [[likely]]
+x = 2;
+  else
+x = 3;
+  return x;
+}
Index: clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
===
--- /dev/null
+++ clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
@@ -0,0 +1,13 @@
+_Z13is_in_profilev
+0x00029f493458
+2
+2
+# The branch is never executed.
+0
+
+_Z17is_not_in_profilev
+0x00029f493458
+2
+# The function was never executed
+0
+0
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -815,11 +815,20 @@
   // Prefer the PGO based weights over the likelihood attribute.
   // When the 

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

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

Anyone wants to take a look/stamp? :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134456

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

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

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.



> No, it sounds like we need a proposal for 
> `[[likely_unless_the_optimizer_decides_otherwise]]` -- the `[[likely]]` 
> attribute was intended for always-likely optimization decisions.

Well, whatever the default behavior is chosen.

> I agree that we should be consistent, but there's different ways we can be 
> consistent. Consistent with what the feature paper intended? Consistent with 
> our past decisions? Consistent with how other implementations behave?

What I meant is that if the status quo is preserved, the never taken [[likely]] 
branch would be considered as "unlikely'' after PGO (the purpose of this patch).

> What do other implementations that support PGO do? If PGO implementations 
> consistently behave a certain way, that would be interesting to know.

I just checked GCC and it seems like it considers likely as a forcing 
optimization: https://godbolt.org/z/69E3383cq


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134456

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

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

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

> Do users have some other escape hatch to tell PGO "ignore what you think you 
> know about this branch" so that a user who *wants* PGO to lose has some 
> ability to do that other than "don't use PGO"?

Sounds like we need another proposal for [[always_likely]] :)
I understand both pros and cons of PGO having preference. However, whatever 
solution we stick to, I'd prefer it to be consistent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134456

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

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

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

In D134456#3809581 , @aaron.ballman 
wrote:

> I'm on the fence about this... the attributes were added as optimization 
> hints, but for somewhat interesting use cases. The obvious one of "I think 
> this path is likely/unlikely and so I'll help the compiler figure it out" was 
> one such use case, but honestly that's akin to `inline` and `register` in 
> terms of how likely it is the user will make a better decision than the 
> optimizer. But the more interesting use case was "The optimizer is going to 
> consider this to be the unlikely path, but I need it to be optimized because 
> I need my failure code to fail as fast as possible". Basically, some folks 
> wanted to use this to override the otherwise-reasonable decisions from the 
> optimizer. Based on that, I'm not certain we should let PGO win -- the user 
> is telling you "optimize based on something I know more about than you", and 
> having a profile that covers the code path doesn't necessarily change that. 
> WDYT?

I don't think that the second case is how things works today. PGO is still 
preferred over the attributes. However, the current behavior is somewhat 
strange:

- if the probability is infinitesimal (e.g. 0.1%), the compiler will ignore 
the attribute,
- if the probability is zero (the branch never hit in the profile), the 
compiler will consider the attribute.




Comment at: clang/lib/CodeGen/CGStmt.cpp:815
   // Prefer the PGO based weights over the likelihood attribute.
   // When the build isn't optimized the metadata isn't used, so don't generate

rnk wrote:
> I lean towards implementing the intended behavior reflected in this comment 
> here, which is that PGO data overrides programmer annotations in case of 
> conflict. We should also check the PGO docs to see if this is documented. If 
> we've already made promises about how this is supposed to work, I'd prefer to 
> trust our past decisions rather than revisiting them.
As Hans pointed out in a Chrome bug: the behaviour is documented here: 
https://clang.llvm.org/docs/AttributeReference.html#likely-and-unlikely:
"These attributes have no effect on the generated code when using PGO 
(Profile-Guided Optimization) or at optimization level 0."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134456

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

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

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev updated this revision to Diff 462250.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134456

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
  clang/test/Profile/cxx-never-executed-branch.cpp


Index: clang/test/Profile/cxx-never-executed-branch.cpp
===
--- /dev/null
+++ clang/test/Profile/cxx-never-executed-branch.cpp
@@ -0,0 +1,32 @@
+// Test that clang doesn't emit llvm.expect when the counter is 0
+
+// RUN: llvm-profdata merge %S/Inputs/cxx-never-executed-branch.proftext -o 
%t.profdata
+// RUN: %clang_cc1 -std=c++20 %s -triple %itanium_abi_triple -O2 -o - 
-emit-llvm -fprofile-instrument-use-path=%t.profdata -disable-llvm-passes | 
FileCheck %s
+
+int rand();
+
+// CHECK: define {{.*}}@_Z13is_in_profilev
+// CHECK-NOT: call {{.*}}@llvm.expect
+
+int is_in_profile() {
+  int rando = rand();
+  int x = 0;
+  if (rando == 0) [[likely]]
+x = 2;
+  else
+x = 3;
+  return x;
+}
+
+// CHECK: define {{.*}}@_Z17is_not_in_profilev
+// CHECK: call {{.*}}@llvm.expect
+
+int is_not_in_profile() {
+  int rando = rand();
+  int x = 0;
+  if (rando == 0) [[likely]]
+x = 2;
+  else
+x = 3;
+  return x;
+}
Index: clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
===
--- /dev/null
+++ clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
@@ -0,0 +1,13 @@
+_Z13is_in_profilev
+0x00029f493458
+2
+2
+# The branch is never executed.
+0
+
+_Z17is_not_in_profilev
+0x00029f493458
+2
+# The function was never executed
+0
+0
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -815,11 +815,21 @@
   // Prefer the PGO based weights over the likelihood attribute.
   // When the build isn't optimized the metadata isn't used, so don't generate
   // it.
+  // Also, differentiate between disabled PGO and a never executed branch with
+  // PGO. Assuming PGO is in use:
+  // - we want to ignore the [[likely]] attribute if the branch is never
+  // executed,
+  // - assuming the profile is poor, preserving the attribute may still be
+  // beneficial.
+  // As an approximation, preserve the attribute only if both the branch and 
the
+  // parent context were not executed.
   Stmt::Likelihood LH = Stmt::LH_None;
-  uint64_t Count = getProfileCount(S.getThen());
-  if (!Count && CGM.getCodeGenOpts().OptimizationLevel)
+  uint64_t ThenCount = getProfileCount(S.getThen());
+  if (!ThenCount && !getCurrentProfileCount() &&
+  CGM.getCodeGenOpts().OptimizationLevel) {
 LH = Stmt::getLikelihood(S.getThen(), S.getElse());
-  EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock, Count, LH);
+  }
+  EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock, ThenCount, LH);
 
   // Emit the 'then' code.
   EmitBlock(ThenBlock);


Index: clang/test/Profile/cxx-never-executed-branch.cpp
===
--- /dev/null
+++ clang/test/Profile/cxx-never-executed-branch.cpp
@@ -0,0 +1,32 @@
+// Test that clang doesn't emit llvm.expect when the counter is 0
+
+// RUN: llvm-profdata merge %S/Inputs/cxx-never-executed-branch.proftext -o %t.profdata
+// RUN: %clang_cc1 -std=c++20 %s -triple %itanium_abi_triple -O2 -o - -emit-llvm -fprofile-instrument-use-path=%t.profdata -disable-llvm-passes | FileCheck %s
+
+int rand();
+
+// CHECK: define {{.*}}@_Z13is_in_profilev
+// CHECK-NOT: call {{.*}}@llvm.expect
+
+int is_in_profile() {
+  int rando = rand();
+  int x = 0;
+  if (rando == 0) [[likely]]
+x = 2;
+  else
+x = 3;
+  return x;
+}
+
+// CHECK: define {{.*}}@_Z17is_not_in_profilev
+// CHECK: call {{.*}}@llvm.expect
+
+int is_not_in_profile() {
+  int rando = rand();
+  int x = 0;
+  if (rando == 0) [[likely]]
+x = 2;
+  else
+x = 3;
+  return x;
+}
Index: clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
===
--- /dev/null
+++ clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
@@ -0,0 +1,13 @@
+_Z13is_in_profilev
+0x00029f493458
+2
+2
+# The branch is never executed.
+0
+
+_Z17is_not_in_profilev
+0x00029f493458
+2
+# The function was never executed
+0
+0
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -815,11 +815,21 @@
   // Prefer the PGO based weights over the likelihood attribute.
   // When the build isn't optimized the metadata isn't used, so don't generate
   // it.
+  // Also, differentiate between disabled PGO and a never executed branch with
+  // PGO. Assuming PGO is in use:
+  // - we want to ignore the [[likely]] 

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

[Anton Bikineev via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134456

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

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

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev created this revision.
AntonBikineev added a reviewer: hans.
Herald added a subscriber: wenlei.
Herald added a project: All.
AntonBikineev requested review of this revision.
Herald added a project: clang.

Generally, with PGO enabled the C++20 likelyhood attributes shall be
dropped assuming the profile has a good coverage. However, currently
this is not the case for the following code:

  if (always_false()) [[likely]] {
...
  }


The patch fixes this and drops the attribute, if the parent context was
executed in the profile. The patch still preserves the attribute, if the
parent context was not executed, e.g. to support the cases when the
profile has insufficient coverage and the programmer's assumption is
correct.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134456

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
  clang/test/Profile/cxx-never-executed-branch.cpp


Index: clang/test/Profile/cxx-never-executed-branch.cpp
===
--- /dev/null
+++ clang/test/Profile/cxx-never-executed-branch.cpp
@@ -0,0 +1,32 @@
+// Test the clang doesn't emit llvm.expect when the counter is 0
+
+// RUN: llvm-profdata merge %S/Inputs/cxx-never-executed-branch.proftext -o 
%t.profdata
+// RUN: %clang_cc1 -std=c++20 %s -O2 -o - -emit-llvm 
-fprofile-instrument-use-path=%t.profdata -disable-llvm-passes | FileCheck %s
+
+int rand();
+
+// CHECK: define {{.*}}@_Z13is_in_profilev
+// CHECK-NOT: call {{.*}}@llvm.expect
+
+int is_in_profile() {
+  int rando = rand();
+  int x = 0;
+  if (rando == 0) [[likely]]
+x = 2;
+  else
+x = 3;
+  return x;
+}
+
+// CHECK: define {{.*}}@_Z17is_not_in_profilev
+// CHECK: call {{.*}}@llvm.expect
+
+int is_not_in_profile() {
+  int rando = rand();
+  int x = 0;
+  if (rando == 0) [[likely]]
+x = 2;
+  else
+x = 3;
+  return x;
+}
Index: clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
===
--- /dev/null
+++ clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
@@ -0,0 +1,13 @@
+_Z13is_in_profilev
+0x00029f493458
+2
+2
+# The branch is never executed.
+0
+
+_Z17is_not_in_profilev
+0x00029f493458
+2
+# The function was never executed
+0
+0
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -815,11 +815,21 @@
   // Prefer the PGO based weights over the likelihood attribute.
   // When the build isn't optimized the metadata isn't used, so don't generate
   // it.
+  // Also, differentiate between disabled PGO and a never executed branch with
+  // PGO. Assuming PGO is in use:
+  // - we want to ignore the [[likely]] attribute if the branch is never
+  // executed,
+  // - assuming the profile is poor, preserving the attribute may still be
+  // beneficial.
+  // As an approximation, preserve the attribute only if both the branch and 
the
+  // parent context were not executed.
   Stmt::Likelihood LH = Stmt::LH_None;
-  uint64_t Count = getProfileCount(S.getThen());
-  if (!Count && CGM.getCodeGenOpts().OptimizationLevel)
+  uint64_t ThenCount = getProfileCount(S.getThen());
+  if (!ThenCount && !getCurrentProfileCount() &&
+  CGM.getCodeGenOpts().OptimizationLevel) {
 LH = Stmt::getLikelihood(S.getThen(), S.getElse());
-  EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock, Count, LH);
+  }
+  EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock, ThenCount, LH);
 
   // Emit the 'then' code.
   EmitBlock(ThenBlock);


Index: clang/test/Profile/cxx-never-executed-branch.cpp
===
--- /dev/null
+++ clang/test/Profile/cxx-never-executed-branch.cpp
@@ -0,0 +1,32 @@
+// Test the clang doesn't emit llvm.expect when the counter is 0
+
+// RUN: llvm-profdata merge %S/Inputs/cxx-never-executed-branch.proftext -o %t.profdata
+// RUN: %clang_cc1 -std=c++20 %s -O2 -o - -emit-llvm -fprofile-instrument-use-path=%t.profdata -disable-llvm-passes | FileCheck %s
+
+int rand();
+
+// CHECK: define {{.*}}@_Z13is_in_profilev
+// CHECK-NOT: call {{.*}}@llvm.expect
+
+int is_in_profile() {
+  int rando = rand();
+  int x = 0;
+  if (rando == 0) [[likely]]
+x = 2;
+  else
+x = 3;
+  return x;
+}
+
+// CHECK: define {{.*}}@_Z17is_not_in_profilev
+// CHECK: call {{.*}}@llvm.expect
+
+int is_not_in_profile() {
+  int rando = rand();
+  int x = 0;
+  if (rando == 0) [[likely]]
+x = 2;
+  else
+x = 3;
+  return x;
+}
Index: clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
===
--- /dev/null
+++ clang/test/Profile/Inputs/cxx-never-executed-branch.proftext
@@ -0,0 +1,13 @@
+_Z13is_in_profilev
+0x00029f493458
+2
+2
+# The branch is never executed.
+0
+
+_Z17is_not_in_profilev

[PATCH] D99808: [Sema] Move 'char-expression-as-unsigned < 0' into a separate diagnostic

[Anton Bikineev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG69545154cc28: [Sema] Move char-expression-as-unsigned 
 0 into a separate diagnostic (authored by AntonBikineev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99808

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/tautological-unsigned-char-zero-compare.cc

Index: clang/test/Sema/tautological-unsigned-char-zero-compare.cc
===
--- /dev/null
+++ clang/test/Sema/tautological-unsigned-char-zero-compare.cc
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -fsyntax-only \
+// RUN:-fno-signed-char \
+// RUN:-Wtautological-unsigned-zero-compare \
+// RUN:-Wtautological-unsigned-char-zero-compare \
+// RUN:-verify=unsigned %s
+// RUN: %clang_cc1 -fsyntax-only \
+// RUN:-Wtautological-unsigned-zero-compare \
+// RUN:-Wtautological-unsigned-char-zero-compare \
+// RUN:-verify=signed %s
+
+void f(char c, unsigned char uc, signed char cc) {
+  if (c < 0)
+return;
+  // unsigned-warning@-2 {{comparison of char expression < 0 is always false, since char is interpreted as unsigned}}
+  if (uc < 0)
+return;
+  // unsigned-warning@-2 {{comparison of unsigned expression < 0 is always false}}
+  // signed-warning@-3 {{comparison of unsigned expression < 0 is always false}}
+  if (cc < 0)
+return;
+  // Promoted to integer expressions should not warn.
+  if (c - 4 < 0)
+return;
+}
+
+void ref(char , unsigned char , signed char ) {
+  if (c < 0)
+return;
+  // unsigned-warning@-2 {{comparison of char expression < 0 is always false, since char is interpreted as unsigned}}
+  if (uc < 0)
+return;
+  // unsigned-warning@-2 {{comparison of unsigned expression < 0 is always false}}
+  // signed-warning@-3 {{comparison of unsigned expression < 0 is always false}}
+  if (cc < 0)
+return;
+  // Promoted to integer expressions should not warn.
+  if (c - 4 < 0)
+return;
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -11446,11 +11446,14 @@
 << OtherIsBooleanDespiteType << *Result
 << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange());
   } else {
-unsigned Diag = (isKnownToHaveUnsignedValue(OriginalOther) && Value == 0)
-? (HasEnumType(OriginalOther)
-   ? diag::warn_unsigned_enum_always_true_comparison
-   : diag::warn_unsigned_always_true_comparison)
-: diag::warn_tautological_constant_compare;
+bool IsCharTy = OtherT.withoutLocalFastQualifiers() == S.Context.CharTy;
+unsigned Diag =
+(isKnownToHaveUnsignedValue(OriginalOther) && Value == 0)
+? (HasEnumType(OriginalOther)
+   ? diag::warn_unsigned_enum_always_true_comparison
+   : IsCharTy ? diag::warn_unsigned_char_always_true_comparison
+  : diag::warn_unsigned_always_true_comparison)
+: diag::warn_tautological_constant_compare;
 
 S.Diag(E->getOperatorLoc(), Diag)
 << RhsConstant << OtherT << E->getOpcodeStr() << OS.str() << *Result
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6784,6 +6784,10 @@
   "result of comparison of %select{%3|unsigned expression}0 %2 "
   "%select{unsigned expression|%3}0 is always %4">,
   InGroup, DefaultIgnore;
+def warn_unsigned_char_always_true_comparison : Warning<
+  "result of comparison of %select{%3|char expression}0 %2 "
+  "%select{char expression|%3}0 is always %4, since char is interpreted as "
+  "unsigned">, InGroup, DefaultIgnore;
 def warn_unsigned_enum_always_true_comparison : Warning<
   "result of comparison of %select{%3|unsigned enum expression}0 %2 "
   "%select{unsigned enum expression|%3}0 is always %4">,
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -591,11 +591,13 @@
 def IntInBoolContext : DiagGroup<"int-in-bool-context">;
 def TautologicalTypeLimitCompare : DiagGroup<"tautological-type-limit-compare">;
 def TautologicalUnsignedZeroCompare : DiagGroup<"tautological-unsigned-zero-compare">;
+def TautologicalUnsignedCharZeroCompare : DiagGroup<"tautological-unsigned-char-zero-compare">;
 def TautologicalUnsignedEnumZeroCompare : 

[PATCH] D99808: [Sema] Move 'char-expression-as-unsigned < 0' into a separate diagnostic

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev updated this revision to Diff 336612.
AntonBikineev added a comment.

Fix formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99808

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/tautological-unsigned-char-zero-compare.cc

Index: clang/test/Sema/tautological-unsigned-char-zero-compare.cc
===
--- /dev/null
+++ clang/test/Sema/tautological-unsigned-char-zero-compare.cc
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -fsyntax-only \
+// RUN:-fno-signed-char \
+// RUN:-Wtautological-unsigned-zero-compare \
+// RUN:-Wtautological-unsigned-char-zero-compare \
+// RUN:-verify=unsigned %s
+// RUN: %clang_cc1 -fsyntax-only \
+// RUN:-Wtautological-unsigned-zero-compare \
+// RUN:-Wtautological-unsigned-char-zero-compare \
+// RUN:-verify=signed %s
+
+void f(char c, unsigned char uc, signed char cc) {
+  if (c < 0)
+return;
+  // unsigned-warning@-2 {{comparison of char expression < 0 is always false, since char is interpreted as unsigned}}
+  if (uc < 0)
+return;
+  // unsigned-warning@-2 {{comparison of unsigned expression < 0 is always false}}
+  // signed-warning@-3 {{comparison of unsigned expression < 0 is always false}}
+  if (cc < 0)
+return;
+  // Promoted to integer expressions should not warn.
+  if (c - 4 < 0)
+return;
+}
+
+void ref(char , unsigned char , signed char ) {
+  if (c < 0)
+return;
+  // unsigned-warning@-2 {{comparison of char expression < 0 is always false, since char is interpreted as unsigned}}
+  if (uc < 0)
+return;
+  // unsigned-warning@-2 {{comparison of unsigned expression < 0 is always false}}
+  // signed-warning@-3 {{comparison of unsigned expression < 0 is always false}}
+  if (cc < 0)
+return;
+  // Promoted to integer expressions should not warn.
+  if (c - 4 < 0)
+return;
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -11415,11 +11415,14 @@
 << OtherIsBooleanDespiteType << *Result
 << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange());
   } else {
-unsigned Diag = (isKnownToHaveUnsignedValue(OriginalOther) && Value == 0)
-? (HasEnumType(OriginalOther)
-   ? diag::warn_unsigned_enum_always_true_comparison
-   : diag::warn_unsigned_always_true_comparison)
-: diag::warn_tautological_constant_compare;
+bool IsCharTy = OtherT.withoutLocalFastQualifiers() == S.Context.CharTy;
+unsigned Diag =
+(isKnownToHaveUnsignedValue(OriginalOther) && Value == 0)
+? (HasEnumType(OriginalOther)
+   ? diag::warn_unsigned_enum_always_true_comparison
+   : IsCharTy ? diag::warn_unsigned_char_always_true_comparison
+  : diag::warn_unsigned_always_true_comparison)
+: diag::warn_tautological_constant_compare;
 
 S.Diag(E->getOperatorLoc(), Diag)
 << RhsConstant << OtherT << E->getOpcodeStr() << OS.str() << *Result
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6784,6 +6784,10 @@
   "result of comparison of %select{%3|unsigned expression}0 %2 "
   "%select{unsigned expression|%3}0 is always %4">,
   InGroup, DefaultIgnore;
+def warn_unsigned_char_always_true_comparison : Warning<
+  "result of comparison of %select{%3|char expression}0 %2 "
+  "%select{char expression|%3}0 is always %4, since char is interpreted as "
+  "unsigned">, InGroup, DefaultIgnore;
 def warn_unsigned_enum_always_true_comparison : Warning<
   "result of comparison of %select{%3|unsigned enum expression}0 %2 "
   "%select{unsigned enum expression|%3}0 is always %4">,
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -591,11 +591,13 @@
 def IntInBoolContext : DiagGroup<"int-in-bool-context">;
 def TautologicalTypeLimitCompare : DiagGroup<"tautological-type-limit-compare">;
 def TautologicalUnsignedZeroCompare : DiagGroup<"tautological-unsigned-zero-compare">;
+def TautologicalUnsignedCharZeroCompare : DiagGroup<"tautological-unsigned-char-zero-compare">;
 def TautologicalUnsignedEnumZeroCompare : DiagGroup<"tautological-unsigned-enum-zero-compare">;
 // For compatibility with GCC. Tautological comparison 

[PATCH] D99808: [Sema] Move 'char-expression-as-unsigned < 0' into a separate diagnostic

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

friendly ping..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99808

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

[PATCH] D99808: [Sema] Move 'char-expression-as-unsigned < 0' into a separate diagnostic

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev created this revision.
AntonBikineev added reviewers: aaron.ballman, thakis.
AntonBikineev requested review of this revision.
Herald added a project: clang.

This change splits '-Wtautological-unsigned-zero-compare' by reporting
char-expressions-interpreted-as-unsigned under a separate diagnostic
'-Wtautological-unsigned-char-zero-compare'. This is beneficial for
projects that want to enable '-Wtautological-unsigned-zero-compare' but at
the same time want to keep code portable for platforms with char being
signed or unsigned, such as Chromium.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99808

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/tautological-unsigned-char-zero-compare.cc

Index: clang/test/Sema/tautological-unsigned-char-zero-compare.cc
===
--- /dev/null
+++ clang/test/Sema/tautological-unsigned-char-zero-compare.cc
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -fsyntax-only \
+// RUN:-fno-signed-char \
+// RUN:-Wtautological-unsigned-zero-compare \
+// RUN:-Wtautological-unsigned-char-zero-compare \
+// RUN:-verify=unsigned %s
+// RUN: %clang_cc1 -fsyntax-only \
+// RUN:-Wtautological-unsigned-zero-compare \
+// RUN:-Wtautological-unsigned-char-zero-compare \
+// RUN:-verify=signed %s
+
+void f(char c, unsigned char uc, signed char cc) {
+  if (c < 0)
+return;
+  // unsigned-warning@-2 {{comparison of char expression < 0 is always false, since char is interpreted as unsigned}}
+  if (uc < 0)
+return;
+  // unsigned-warning@-2 {{comparison of unsigned expression < 0 is always false}}
+  // signed-warning@-3 {{comparison of unsigned expression < 0 is always false}}
+  if (cc < 0)
+return;
+  // Promoted to integer expressions should not warn.
+  if (c - 4 < 0)
+return;
+}
+
+void ref(char , unsigned char , signed char ) {
+  if (c < 0)
+return;
+  // unsigned-warning@-2 {{comparison of char expression < 0 is always false, since char is interpreted as unsigned}}
+  if (uc < 0)
+return;
+  // unsigned-warning@-2 {{comparison of unsigned expression < 0 is always false}}
+  // signed-warning@-3 {{comparison of unsigned expression < 0 is always false}}
+  if (cc < 0)
+return;
+  // Promoted to integer expressions should not warn.
+  if (c - 4 < 0)
+return;
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -11415,11 +11415,14 @@
 << OtherIsBooleanDespiteType << *Result
 << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange());
   } else {
-unsigned Diag = (isKnownToHaveUnsignedValue(OriginalOther) && Value == 0)
-? (HasEnumType(OriginalOther)
-   ? diag::warn_unsigned_enum_always_true_comparison
-   : diag::warn_unsigned_always_true_comparison)
-: diag::warn_tautological_constant_compare;
+bool IsCharTy = OtherT.withoutLocalFastQualifiers() == S.Context.CharTy;
+unsigned Diag =
+(isKnownToHaveUnsignedValue(OriginalOther) && Value == 0)
+? (HasEnumType(OriginalOther)
+   ? diag::warn_unsigned_enum_always_true_comparison
+   : IsCharTy ? diag::warn_unsigned_char_always_true_comparison
+  : diag::warn_unsigned_always_true_comparison)
+: diag::warn_tautological_constant_compare;
 
 S.Diag(E->getOperatorLoc(), Diag)
 << RhsConstant << OtherT << E->getOpcodeStr() << OS.str() << *Result
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6784,6 +6784,10 @@
   "result of comparison of %select{%3|unsigned expression}0 %2 "
   "%select{unsigned expression|%3}0 is always %4">,
   InGroup, DefaultIgnore;
+def warn_unsigned_char_always_true_comparison : Warning<
+  "result of comparison of %select{%3|char expression}0 %2 "
+  "%select{char expression|%3}0 is always %4, since char is interpreted as "
+  "unsigned">, InGroup, DefaultIgnore;
 def warn_unsigned_enum_always_true_comparison : Warning<
   "result of comparison of %select{%3|unsigned enum expression}0 %2 "
   "%select{unsigned enum expression|%3}0 is always %4">,
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -591,11 +591,13 @@
 def IntInBoolContext : DiagGroup<"int-in-bool-context">;
 def 

[PATCH] D99456: [C++2b] Support size_t literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added inline comments.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:593-594
+  // C++2b features.
+  if (LangOpts.CPlusPlus2b)
+Builder.defineMacro("__cpp_size_t_suffix", "202011L");
   if (LangOpts.Char8)

Quuxplusone wrote:
> aaron.ballman wrote:
> > AntonBikineev wrote:
> > > aaron.ballman wrote:
> > > > Because we allow this as an extension in all C++ modes, should this be 
> > > > enabled always rather than gated on C++2b?
> > > I was also wondering about this. I've checked that we also do the same 
> > > for other feature macros, such as __cpp_binary_literals, which is defined 
> > > for -std>=c++14 while at the same time is allowed as an extension before 
> > > C++14. Therefore I decided to mimic the behaviour.
> > Thanks for checking on that! I think that seems defensible enough. :-)
> Btw, thus far libc++ has tended to make the opposite choice: for example, 
> libc++ defines `__cpp_lib_variant == 202102` in all modes, because the 
> programmer conceivably might be depending on that macro to make some 
> decision, so we want to make sure it reflects the specific semantics that we 
> implement.  (For `__cpp_binary_literals` specifically, I agree it doesn't 
> really matter because nobody's going to be making decisions based on the 
> value of this macro.)
> 
> See https://reviews.llvm.org/D99290#inline-934563 (D96385, D97394) for 
> previous discussions on the libc++ side.
Thanks for pointing this out, Arthur.

I wish there was some consistency, however, I'm not sure if this is easily 
feasible. I guess the strategy of defining `__cpp_size_t_literals` on all modes 
would be problematic, since if the user code depends on 
`__cpp_size_t_literals`, it could suddenly receive the extension warning (when 
compiled with -std<2++2b), which is enabled by default.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:593-594
+  // C++2b features.
+  if (LangOpts.CPlusPlus2b)
+Builder.defineMacro("__cpp_size_t_suffix", "202011L");
   if (LangOpts.Char8)

AntonBikineev wrote:
> Quuxplusone wrote:
> > aaron.ballman wrote:
> > > AntonBikineev wrote:
> > > > aaron.ballman wrote:
> > > > > Because we allow this as an extension in all C++ modes, should this 
> > > > > be enabled always rather than gated on C++2b?
> > > > I was also wondering about this. I've checked that we also do the same 
> > > > for other feature macros, such as __cpp_binary_literals, which is 
> > > > defined for -std>=c++14 while at the same time is allowed as an 
> > > > extension before C++14. Therefore I decided to mimic the behaviour.
> > > Thanks for checking on that! I think that seems defensible enough. :-)
> > Btw, thus far libc++ has tended to make the opposite choice: for example, 
> > libc++ defines `__cpp_lib_variant == 202102` in all modes, because the 
> > programmer conceivably might be depending on that macro to make some 
> > decision, so we want to make sure it reflects the specific semantics that 
> > we implement.  (For `__cpp_binary_literals` specifically, I agree it 
> > doesn't really matter because nobody's going to be making decisions based 
> > on the value of this macro.)
> > 
> > See https://reviews.llvm.org/D99290#inline-934563 (D96385, D97394) for 
> > previous discussions on the libc++ side.
> Thanks for pointing this out, Arthur.
> 
> I wish there was some consistency, however, I'm not sure if this is easily 
> feasible. I guess the strategy of defining `__cpp_size_t_literals` on all 
> modes would be problematic, since if the user code depends on 
> `__cpp_size_t_literals`, it could suddenly receive the extension warning 
> (when compiled with -std<2++2b), which is enabled by default.
> Btw, thus far libc++ has tended to make the opposite choice: for example, 
> libc++ defines `__cpp_lib_variant == 202102` in all modes, because the 
> programmer conceivably might be depending on that macro to make some 
> decision, so we want to make sure it reflects the specific semantics that we 
> implement.  (For `__cpp_binary_literals` specifically, I agree it doesn't 
> really matter because nobody's going to be making decisions based on the 
> value of this macro.)
> 
> See https://reviews.llvm.org/D99290#inline-934563 (D96385, D97394) for 
> previous discussions on the libc++ side.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99456

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

[PATCH] D99456: [C++2b] Support size_t literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev closed this revision.
AntonBikineev added a comment.

Aaron, Richard, thanks a lot for reviewing this!

Sorry, forgot to add amend the commit with 'Differential revision', closing 
this manually..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99456

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

[PATCH] D99456: [C++2b] Support size_t literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev updated this revision to Diff 334422.
AntonBikineev added a comment.

Fix comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99456

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Lex/PPExpressions.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/size_t-literal.cpp
  clang/test/SemaCXX/cxx1y-user-defined-literals.cpp
  clang/test/SemaCXX/size_t-literal.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1270,7 +1270,7 @@
 
   Literal suffix uz, z for size_t, ssize_t
   https://wg21.link/p0330r8;>P0330R8
-  No
+  Clang 13
 
 
 
Index: clang/test/SemaCXX/size_t-literal.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/size_t-literal.cpp
@@ -0,0 +1,99 @@
+// RUN: %clang_cc1 -std=c++2b -triple x86_64-linux -Wpre-c++2b-compat -fsyntax-only -verify=cxx2b %s
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-linux -fsyntax-only -verify=cxx20 %s
+// RUN: %clang_cc1 -std=c++2b -triple i686-linux -fsyntax-only -verify=cxx2b-32 %s
+// RUN: %clang_cc1 -x c -std=c11 -fsyntax-only -verify=c11 %s
+
+#ifdef __cplusplus
+
+typedef __SIZE_TYPE__ size_t;
+// Assume ptrdiff_t is the signed integer type corresponding to size_t.
+typedef __PTRDIFF_TYPE__ ssize_t;
+
+template 
+struct is_same { static constexpr bool value = false; };
+
+template 
+struct is_same { static constexpr bool value = true; };
+
+void SSizeT() {
+  auto a1 = 1z;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a2 = 1Z;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+}
+
+void SizeT() {
+  auto a1 = 1uz;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a2 = 1uZ;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a3 = 1Uz;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a4 = 1UZ;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a5 = 1zu;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a6 = 1Zu;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a7 = 1zU;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a8 = 1ZU;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+}
+
+void oor() {
+#if __i386__
+  (void)3'000'000'000z; // cxx2b-32-error {{signed 'size_t' literal is out of range of possible signed 'size_t' values}}
+  (void)3'000'000'000uz;
+  (void)5'000'000'000uz; // cxx2b-32-error {{'size_t' literal is out of range of possible 'size_t' values}}
+
+  (void)0x8000z;
+  (void)0x8000uz;
+  (void)0x18000uz; //cxx2b-32-error {{'size_t' literal is out of range of possible 'size_t' values}}
+#endif
+}
+
+#else
+
+void f() {
+  (void)1z;  // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1Z;  // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1uz; // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1uZ; // c11-error {{'size_t' suffix for literals is a C++2b feature}}

[PATCH] D99456: [C++2b] Support size_t literals

[Anton Bikineev via Phabricator via cfe-commits]

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



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:593-594
+  // C++2b features.
+  if (LangOpts.CPlusPlus2b)
+Builder.defineMacro("__cpp_size_t_suffix", "202011L");
   if (LangOpts.Char8)

aaron.ballman wrote:
> Because we allow this as an extension in all C++ modes, should this be 
> enabled always rather than gated on C++2b?
I was also wondering about this. I've checked that we also do the same for 
other feature macros, such as __cpp_binary_literals, which is defined for 
-std>=c++14 while at the same time is allowed as an extension before C++14. 
Therefore I decided to mimic the behaviour.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99456

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

[PATCH] D99456: [C++2b] Support size_t literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev updated this revision to Diff 334288.
AntonBikineev marked 2 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99456

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Lex/PPExpressions.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/size_t-literal.cpp
  clang/test/SemaCXX/cxx1y-user-defined-literals.cpp
  clang/test/SemaCXX/size_t-literal.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1270,7 +1270,7 @@
 
   Literal suffix uz, z for size_t, ssize_t
   https://wg21.link/p0330r8;>P0330R8
-  No
+  Clang 13
 
 
 
Index: clang/test/SemaCXX/size_t-literal.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/size_t-literal.cpp
@@ -0,0 +1,99 @@
+// RUN: %clang_cc1 -std=c++2b -triple x86_64-linux -Wpre-c++2b-compat -fsyntax-only -verify=cxx2b %s
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-linux -fsyntax-only -verify=cxx20 %s
+// RUN: %clang_cc1 -std=c++2b -triple i686-linux -fsyntax-only -verify=cxx2b-32 %s
+// RUN: %clang_cc1 -x c -std=c11 -fsyntax-only -verify=c11 %s
+
+#ifdef __cplusplus
+
+typedef __SIZE_TYPE__ size_t;
+// Assume ptrdiff_t is the signed integer type corresponding to size_t.
+typedef __PTRDIFF_TYPE__ ssize_t;
+
+template 
+struct is_same { static constexpr bool value = false; };
+
+template 
+struct is_same { static constexpr bool value = true; };
+
+void SSizeT() {
+  auto a1 = 1z;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a2 = 1Z;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+}
+
+void SizeT() {
+  auto a1 = 1uz;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a2 = 1uZ;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a3 = 1Uz;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a4 = 1UZ;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a5 = 1zu;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a6 = 1Zu;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a7 = 1zU;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a8 = 1ZU;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+}
+
+void oor() {
+#if __i386__
+  (void)3'000'000'000z; // cxx2b-32-error {{signed 'size_t' literal is out of range of possible signed 'size_t' values}}
+  (void)3'000'000'000uz;
+  (void)5'000'000'000uz; // cxx2b-32-error {{'size_t' literal is out of range of possible 'size_t' values}}
+
+  (void)0x8000z;
+  (void)0x8000uz;
+  (void)0x18000uz; //cxx2b-32-error {{'size_t' literal is out of range of possible 'size_t' values}}
+#endif
+}
+
+#else
+
+void f() {
+  (void)1z;  // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1Z;  // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1uz; // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1uZ; // c11-error {{'size_t' suffix for literals is a C++2b 

[PATCH] D99456: [C++2b] Support size_t literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev updated this revision to Diff 334263.
AntonBikineev added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99456

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Lex/PPExpressions.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/size_t-literal.cpp
  clang/test/SemaCXX/cxx1y-user-defined-literals.cpp
  clang/test/SemaCXX/size_t-literal.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1270,7 +1270,7 @@
 
   Literal suffix uz, z for size_t, ssize_t
   https://wg21.link/p0330r8;>P0330R8
-  No
+  Clang 13
 
 
 
Index: clang/test/SemaCXX/size_t-literal.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/size_t-literal.cpp
@@ -0,0 +1,99 @@
+// RUN: %clang_cc1 -std=c++2b -triple x86_64-linux -Wpre-c++2b-compat -fsyntax-only -verify=cxx2b %s
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-linux -fsyntax-only -verify=cxx20 %s
+// RUN: %clang_cc1 -std=c++2b -triple i686-linux -fsyntax-only -verify=cxx2b-32 %s
+// RUN: %clang_cc1 -x c -std=c11 -fsyntax-only -verify=c11 %s
+
+#ifdef __cplusplus
+
+typedef __SIZE_TYPE__ size_t;
+// Assume ptrdiff_t is the signed integer type corresponding to size_t.
+typedef __PTRDIFF_TYPE__ ssize_t;
+
+template 
+struct is_same { static constexpr bool value = false; };
+
+template 
+struct is_same { static constexpr bool value = true; };
+
+void SSizeT() {
+  auto a1 = 1z;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a2 = 1Z;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+}
+
+void SizeT() {
+  auto a1 = 1uz;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a2 = 1uZ;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a3 = 1Uz;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a4 = 1UZ;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a5 = 1zu;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a6 = 1Zu;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a7 = 1zU;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a8 = 1ZU;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+}
+
+void oor() {
+#if __i386__
+  (void)3'000'000'000z; // cxx2b-32-error {{signed 'size_t' literal is out of range of possible signed 'size_t' values}}
+  (void)3'000'000'000uz;
+  (void)5'000'000'000uz; // cxx2b-32-error {{'size_t' literal is out of range of possible 'size_t' values}}
+
+  (void)0x8000z;
+  (void)0x8000uz;
+  (void)0x18000uz; //cxx2b-32-error {{'size_t' literal is out of range of possible 'size_t' values}}
+#endif
+}
+
+#else
+
+void f() {
+  (void)1z;  // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1Z;  // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1uz; // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1uZ; // c11-error {{'size_t' suffix for literals is a C++2b 

[PATCH] D99456: [C++2b] Support size_t literals

[Anton Bikineev via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:198-200
+def err_size_t_literal_too_large: Error<
+  "%select{signed |}0'size_t' literal is out of range of possible "
+  "%select{signed |}0'size_t' values">;

rsmith wrote:
> I wonder if it'd be better to say `'ssize_t'` instead of `signed 'size_t'` 
> here? The latter sounds self-contradictory since `size_t` is an unsigned type.
I thought about it (and actually had it first as ssize_t). The problem with 
ssize_t is that it's not defined by C or C++ Standards, AFAIK, but by POSIX. 
The proposal calls it "the signed integer type corresponding to std::size_t", 
so I decided to shorten it to "signed 'size_t'". However, I don't have strong 
opinion on this.



Comment at: clang/lib/Sema/SemaExpr.cpp:3895
   // be an unsigned int.
   bool AllowUnsigned = Literal.isUnsigned || Literal.getRadix() != 10;
 

rsmith wrote:
> AntonBikineev wrote:
> > I now begin to think that we should probably also prohibit things like 
> > 0x1234z to be implicitly interpreted as unsigned. Wdyt?
> Table 8 says that a `z`-suffixed literal has type `ssize_t` (if the value 
> fits) or `size_t` (otherwise), so I think taking `AllowUnsigned` into account 
> is correct (although perhaps surprising).
Thanks for pointing this out! Changed it back and fixed the tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99456

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

[PATCH] D99456: [C++2b] Support size_t literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev updated this revision to Diff 334102.
AntonBikineev marked an inline comment as done.
AntonBikineev added a comment.

Address comments. Also:

- always issue a new diagnostic if size_t/ssize_t is out of range;
- prohibit numbers with 'z' suffix and non-10-radix to be interpreted as 
unsigned.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99456

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Lex/PPExpressions.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/size_t-literal.cpp
  clang/test/SemaCXX/cxx1y-user-defined-literals.cpp
  clang/test/SemaCXX/size_t-literal.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1270,7 +1270,7 @@
 
   Literal suffix uz, z for size_t, ssize_t
   https://wg21.link/p0330r8;>P0330R8
-  No
+  Clang 13
 
 
 
Index: clang/test/SemaCXX/size_t-literal.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/size_t-literal.cpp
@@ -0,0 +1,99 @@
+// RUN: %clang_cc1 -std=c++2b -triple x86_64-linux -Wpre-c++2b-compat -fsyntax-only -verify=cxx2b %s
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-linux -fsyntax-only -verify=cxx20 %s
+// RUN: %clang_cc1 -std=c++2b -triple i686-linux -fsyntax-only -verify=cxx2b-32 %s
+// RUN: %clang_cc1 -x c -std=c11 -fsyntax-only -verify=c11 %s
+
+#ifdef __cplusplus
+
+typedef __SIZE_TYPE__ size_t;
+// Assume ptrdiff_t is the signed integer type corresponding to size_t.
+typedef __PTRDIFF_TYPE__ ssize_t;
+
+template 
+struct is_same { static constexpr bool value = false; };
+
+template 
+struct is_same { static constexpr bool value = true; };
+
+void SSizeT() {
+  auto a1 = 1z;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a2 = 1Z;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+}
+
+void SizeT() {
+  auto a1 = 1uz;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a2 = 1uZ;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a3 = 1Uz;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a4 = 1UZ;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a5 = 1zu;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a6 = 1Zu;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a7 = 1zU;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a8 = 1ZU;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+}
+
+void oor() {
+#if __i386__
+  (void)3'000'000'000z; // cxx2b-32-error {{signed 'size_t' literal is out of range of possible signed 'size_t' values}}
+  (void)3'000'000'000uz;
+  (void)5'000'000'000uz; // cxx2b-32-error {{'size_t' literal is out of range of possible 'size_t' values}}
+
+  (void)0x8000z; //cxx2b-32-error {{signed 'size_t' literal is out of range of possible signed 'size_t' values}}
+  (void)0x8000uz;
+  (void)0x18000uz; //cxx2b-32-error {{'size_t' literal is out of range of possible 'size_t' values}}
+#endif
+}
+
+#else
+
+void f() {
+  

[PATCH] D99456: [C++2b] Support size_t literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev marked 3 inline comments as done.
AntonBikineev added inline comments.



Comment at: clang/lib/Lex/LiteralSupport.cpp:640
 isFloat16 = true;
 continue;
   }

rsmith wrote:
> Looks like this might fix [[ https://godbolt.org/z/9Pe4qr1c7 | incorrect 
> acceptance ]] of `1.0f16f` and `1.0f16L` in CUDA mode too :)
Nice!



Comment at: clang/lib/Sema/SemaExpr.cpp:3895
   // be an unsigned int.
   bool AllowUnsigned = Literal.isUnsigned || Literal.getRadix() != 10;
 

I now begin to think that we should probably also prohibit things like 0x1234z 
to be implicitly interpreted as unsigned. Wdyt?



Comment at: clang/lib/Sema/SemaExpr.cpp:3997
   // does not fit in a signed long long, but has no U suffix.
   if (Ty.isNull()) {
 Diag(Tok.getLocation(), 
diag::ext_integer_literal_too_large_for_signed);

I think this branch should also be covered, since we probably don't want 
promotion from size_t to ULL but instead have a separate diagnostic that size_t 
is out-of-range. I've added another diagnostic and branch here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99456

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

[PATCH] D99456: [C++2b] Support size_t literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev updated this revision to Diff 334037.
AntonBikineev marked 2 inline comments as done.
AntonBikineev added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99456

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Lex/PPExpressions.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/size_t-literal.cpp
  clang/test/SemaCXX/cxx1y-user-defined-literals.cpp
  clang/test/SemaCXX/size_t-literal.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1270,7 +1270,7 @@
 
   Literal suffix uz, z for size_t, ssize_t
   https://wg21.link/p0330r8;>P0330R8
-  No
+  Clang 13
 
 
 
Index: clang/test/SemaCXX/size_t-literal.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/size_t-literal.cpp
@@ -0,0 +1,86 @@
+// RUN: %clang_cc1 -std=c++2b -Wpre-c++2b-compat -fsyntax-only -verify=cxx2b %s
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=cxx20 %s
+// RUN: %clang_cc1 -x c -std=c11 -fsyntax-only -verify=c11 %s
+
+#ifdef __cplusplus
+
+typedef __SIZE_TYPE__ size_t;
+// Assume ptrdiff_t is the signed integer type corresponding to size_t.
+typedef __PTRDIFF_TYPE__ ssize_t;
+
+template 
+struct is_same { static constexpr bool value = false; };
+
+template 
+struct is_same { static constexpr bool value = true; };
+
+void SSizeT() {
+  auto a1 = 1z;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a2 = 1Z;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+}
+
+void SizeT() {
+  auto a1 = 1uz;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a2 = 1uZ;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a3 = 1Uz;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a4 = 1UZ;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a5 = 1zu;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a6 = 1Zu;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a7 = 1zU;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a8 = 1ZU;
+  // cxx2b-warning@-1 {{'size_t' suffix for literals is incompatible with C++ standards before C++2b}}
+  // cxx20-warning@-2 {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+}
+
+#else
+
+void f() {
+  (void)1z;  // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1Z;  // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1uz; // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1uZ; // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1Uz; // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1UZ; // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1zu; // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1Zu; // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1zU; // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1ZU; // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+}
+
+#endif
Index: 

[PATCH] D99456: [C++2b] Support size_t literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev marked 7 inline comments as done.
AntonBikineev added a comment.

Thanks Richard for taking a look!




Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:190
   InGroup, DefaultIgnore;
+def ext_cxx2b_size_t_suffix : Extension<
+  "'size_t' suffix for literals is a C++2b extension">,

rsmith wrote:
> Should this be an `ExtWarn`? I think we should warn on this by default.
I tried to follow the strategy used for 'long long' (only warn with 
-Wlong-long), but I agree that warn-by-default would probably be better.



Comment at: clang/lib/Lex/LiteralSupport.cpp:594-595
 
   // Loop over all of the characters of the suffix.  If we see something bad,
   // we break out of the loop.
   for (; s != ThisTokEnd; ++s) {

rsmith wrote:
> General comment: I think the checks here have become too complex and 
> error-prone. I suggest we add another flag, `hasSize`, that's set when we 
> parse any size modifier (`F`, `L`, `LL`, `H`, `F128`, `I64`, and now `Z`), 
> and when parsing any size modifier, replace the existing ad-hoc set of checks 
> with a check for `hasSize`.
Good idea! It actually helped to flush out the issue when Q and H are mixed 
together: https://godbolt.org/z/8hj39P93a


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99456

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

[PATCH] D99456: [C++2b] Support size_t literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev updated this revision to Diff 333972.
AntonBikineev marked an inline comment as done.
AntonBikineev added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99456

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Lex/PPExpressions.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/size_t-literal.cpp
  clang/test/SemaCXX/cxx1y-user-defined-literals.cpp
  clang/test/SemaCXX/size_t-literal.cpp

Index: clang/test/SemaCXX/size_t-literal.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/size_t-literal.cpp
@@ -0,0 +1,68 @@
+// RUN: %clang_cc1 -std=c++2b -Wc++2b-extensions -fsyntax-only -verify=cxx2b %s
+// RUN: %clang_cc1 -std=c++20 -Wc++2b-extensions -fsyntax-only -verify=cxx20 %s
+// RUN: %clang_cc1 -x c -std=c11 -fsyntax-only -verify=c11 %s
+
+#ifdef __cplusplus
+
+typedef __SIZE_TYPE__ size_t;
+// Assume ptrdiff_t is the signed integer type corresponding to size_t.
+typedef __PTRDIFF_TYPE__ ssize_t;
+
+template 
+struct is_same { static constexpr bool value = false; };
+
+template 
+struct is_same { static constexpr bool value = true; };
+
+// cxx2b-no-diagnostics
+
+void SSizeT() {
+  auto a1 = 1z; // cxx20-warning {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a2 = 1Z; // cxx20-warning {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+}
+
+void SizeT() {
+  auto a1 = 1uz; // cxx20-warning {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a2 = 1uZ; // cxx20-warning {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a3 = 1Uz; // cxx20-warning {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a4 = 1UZ; // cxx20-warning {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a5 = 1zu; // cxx20-warning {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a6 = 1Zu; // cxx20-warning {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a7 = 1zU; // cxx20-warning {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+
+  auto a8 = 1ZU; // cxx20-warning {{'size_t' suffix for literals is a C++2b extension}}
+  static_assert(is_same::value);
+}
+
+#else
+
+void f() {
+  (void)1z;  // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1Z;  // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1uz; // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1uZ; // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1Uz; // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1UZ; // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1zu; // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1Zu; // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1zU; // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+  (void)1ZU; // c11-error {{'size_t' suffix for literals is a C++2b feature}}
+}
+
+#endif
Index: clang/test/SemaCXX/cxx1y-user-defined-literals.cpp
===
--- clang/test/SemaCXX/cxx1y-user-defined-literals.cpp
+++ clang/test/SemaCXX/cxx1y-user-defined-literals.cpp
@@ -34,7 +34,7 @@
 string s = "foo"s;
 char error = 'x's; // expected-error {{invalid suffix}} expected-error {{expected ';'}}
 
-int _1z = 1z; // expected-error {{invalid suffix}}
+int _1y = 1y; // expected-error {{invalid suffix}}
 int _1b = 1b; // expected-error {{invalid digit}}
 
 complex cf1 = 1if, cf2 = 2.if, cf3 = 0x3if;
Index: clang/test/Lexer/size_t-literal.cpp
===
--- /dev/null
+++ clang/test/Lexer/size_t-literal.cpp
@@ -0,0 +1,161 @@
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify %s
+
+#if 1z != 1
+#error "size_t suffix must be recognized by preprocessor"
+#endif
+#if 1uz != 1
+#error "size_t suffix must be recognized by preprocessor"
+#endif
+
+void ValidSuffix() {
+  // Decimal literals.
+  {
+auto a1 = 1z;
+auto a2 = 1Z;
+
+auto a3 = 1uz;
+auto a4 = 1uZ;
+auto a5 = 1Uz;
+auto a6 = 1UZ;
+
+auto a7 = 1zu;
+auto a8 = 1Zu;
+auto a9 = 1zU;
+auto a10 = 1ZU;
+
+auto a11 = 1'2z;
+auto a12 = 1'2Z;
+  }
+  // Hexadecimal literals.
+  {
+auto a1 = 0x1z;
+auto a2 = 0x1Z;
+
+auto a3 = 0x1uz;
+auto a4 = 0x1uZ;
+auto a5 = 0x1Uz;
+auto a6 = 0x1UZ;
+

[PATCH] D99456: [C++2b] Support size_t literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev marked 9 inline comments as done.
AntonBikineev added a comment.

Thanks Aaron for taking a look! Addressed the comments. I also hope it's okay 
to test preprocessor in the same test (test/Lexer/size_t-literal.cpp).




Comment at: clang/lib/Sema/SemaExpr.cpp:3911
+  if (Literal.isSizeT) {
+assert(Ty.isNull() && "size_t literals can't be Microsoft literals");
+unsigned SizeTSize = Context.getTargetInfo().getTypeWidth(

aaron.ballman wrote:
> The assert message doesn't seem to match the predicate -- why does a null 
> qualtype imply it's a microsoft literal?
Hm, agree, that's probably misleading. I've tried to check that the previous 
branch couldn't be taken if this one has been. I think 
"assert(!Literal.MicrosoftInteger && ...);" would make more sense.



Comment at: clang/test/SemaCXX/size_t-literal.cpp:14-16
+#if __cplusplus >= 202101L
+//  expected-no-diagnostics
+#endif

aaron.ballman wrote:
> Rather than check `__cplusplus` like this, I think the RUN lines should 
> specify a verify prefix. e.g., `-verify=cxx2b` and then use 
> `cxx2b-no-diagnostics` and `-verify=cxx20` and then use `cxx20-warning {{}}`.
Good idea, thanks! That has made the test much cleaner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99456

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

[PATCH] D99456: [C++2b] Support size_t literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev updated this revision to Diff 333811.
AntonBikineev added a comment.

Fix formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99456

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Lex/PPExpressions.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/size_t-literal.cpp
  clang/test/SemaCXX/cxx1y-user-defined-literals.cpp
  clang/test/SemaCXX/size_t-literal.cpp

Index: clang/test/SemaCXX/size_t-literal.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/size_t-literal.cpp
@@ -0,0 +1,80 @@
+// RUN: %clang_cc1 -std=c++2b -Werror=c++2b-extensions -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++20 -Werror=c++2b-extensions -fsyntax-only -verify %s
+
+typedef __SIZE_TYPE__ size_t;
+// Assume ptrdiff_t is the signed integer type corresponding to size_t.
+typedef __PTRDIFF_TYPE__ ssize_t;
+
+template 
+struct is_same { static constexpr bool value = false; };
+
+template 
+struct is_same { static constexpr bool value = true; };
+
+#if __cplusplus >= 202101L
+//  expected-no-diagnostics
+#endif
+
+void SSizeT() {
+  auto a1 = 1z;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a2 = 1Z;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+}
+
+void SizeT() {
+  auto a1 = 1uz;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a2 = 1uZ;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a3 = 1Uz;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a4 = 1UZ;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a5 = 1zu;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a6 = 1Zu;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a7 = 1zU;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a8 = 1ZU;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+}
Index: clang/test/SemaCXX/cxx1y-user-defined-literals.cpp
===
--- clang/test/SemaCXX/cxx1y-user-defined-literals.cpp
+++ clang/test/SemaCXX/cxx1y-user-defined-literals.cpp
@@ -34,7 +34,7 @@
 string s = "foo"s;
 char error = 'x's; // expected-error {{invalid suffix}} expected-error {{expected ';'}}
 
-int _1z = 1z; // expected-error {{invalid suffix}}
+int _1y = 1y; // expected-error {{invalid suffix}}
 int _1b = 1b; // expected-error {{invalid digit}}
 
 complex cf1 = 1if, cf2 = 2.if, cf3 = 0x3if;
Index: clang/test/Lexer/size_t-literal.cpp
===
--- /dev/null
+++ clang/test/Lexer/size_t-literal.cpp
@@ -0,0 +1,154 @@
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify %s
+
+void ValidSuffix() {
+  // Decimal literals.
+  {
+auto a1 = 1z;
+auto a2 = 1Z;
+
+auto a3 = 1uz;
+auto a4 = 1uZ;
+auto a5 = 1Uz;
+auto a6 = 1UZ;
+
+auto a7 = 1zu;
+auto a8 = 1Zu;
+auto a9 = 1zU;
+auto a10 = 1ZU;
+
+auto a11 = 1'2z;
+auto a12 = 1'2Z;
+  }
+  // Hexadecimal literals.
+  {
+auto a1 = 0x1z;
+auto a2 = 0x1Z;
+
+auto a3 = 0x1uz;
+auto a4 = 0x1uZ;
+auto a5 = 0x1Uz;
+auto a6 = 0x1UZ;
+
+auto a7 = 0x1zu;
+auto a8 = 0x1Zu;
+auto a9 = 0x1zU;
+auto a10 = 0x1ZU;
+
+auto a11 = 0x1'2z;
+auto a12 = 0x1'2Z;
+  }
+  // Binary literals.
+  {
+auto a1 = 0b1z;
+auto a2 = 0b1Z;
+
+auto a3 = 0b1uz;
+auto a4 = 0b1uZ;
+auto a5 = 0b1Uz;
+auto a6 = 0b1UZ;
+
+auto a7 = 0b1zu;
+auto a8 = 0b1Zu;
+auto a9 = 0b1zU;
+auto a10 = 0b1ZU;
+
+auto a11 = 0b1'1z;
+auto a12 = 0b1'1Z;
+  }
+  // Octal literals.
+  {
+auto a1 = 01z;
+auto a2 = 01Z;
+
+auto a3 = 01uz;
+auto a4 = 01uZ;
+auto a5 = 01Uz;
+auto a6 = 01UZ;
+
+auto a7 = 01zu;
+auto a8 = 01Zu;
+auto a9 = 01zU;
+auto a10 = 

[PATCH] D99456: [C++2b] Support size_t literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev updated this revision to Diff 333681.
AntonBikineev added a comment.

Fix failing tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99456

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Lex/PPExpressions.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/size_t-literal.cpp
  clang/test/SemaCXX/cxx1y-user-defined-literals.cpp
  clang/test/SemaCXX/size_t-literal.cpp

Index: clang/test/SemaCXX/size_t-literal.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/size_t-literal.cpp
@@ -0,0 +1,80 @@
+// RUN: %clang_cc1 -std=c++2b -Werror=c++2b-extensions -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++20 -Werror=c++2b-extensions -fsyntax-only -verify %s
+
+typedef __SIZE_TYPE__ size_t;
+// Assume ptrdiff_t is the signed integer type corresponding to size_t.
+typedef __PTRDIFF_TYPE__ ssize_t;
+
+template 
+struct is_same { static constexpr bool value = false; };
+
+template 
+struct is_same { static constexpr bool value = true; };
+
+#if __cplusplus >= 202101L
+//  expected-no-diagnostics
+#endif
+
+void SSizeT() {
+  auto a1 = 1z;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a2 = 1Z;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+}
+
+void SizeT() {
+  auto a1 = 1uz;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a2 = 1uZ;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a3 = 1Uz;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a4 = 1UZ;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a5 = 1zu;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a6 = 1Zu;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a7 = 1zU;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a8 = 1ZU;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+}
Index: clang/test/SemaCXX/cxx1y-user-defined-literals.cpp
===
--- clang/test/SemaCXX/cxx1y-user-defined-literals.cpp
+++ clang/test/SemaCXX/cxx1y-user-defined-literals.cpp
@@ -34,7 +34,7 @@
 string s = "foo"s;
 char error = 'x's; // expected-error {{invalid suffix}} expected-error {{expected ';'}}
 
-int _1z = 1z; // expected-error {{invalid suffix}}
+int _1y = 1y; // expected-error {{invalid suffix}}
 int _1b = 1b; // expected-error {{invalid digit}}
 
 complex cf1 = 1if, cf2 = 2.if, cf3 = 0x3if;
Index: clang/test/Lexer/size_t-literal.cpp
===
--- /dev/null
+++ clang/test/Lexer/size_t-literal.cpp
@@ -0,0 +1,154 @@
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify %s
+
+void ValidSuffix() {
+  // Decimal literals.
+  {
+auto a1 = 1z;
+auto a2 = 1Z;
+
+auto a3 = 1uz;
+auto a4 = 1uZ;
+auto a5 = 1Uz;
+auto a6 = 1UZ;
+
+auto a7 = 1zu;
+auto a8 = 1Zu;
+auto a9 = 1zU;
+auto a10 = 1ZU;
+
+auto a11 = 1'2z;
+auto a12 = 1'2Z;
+  }
+  // Hexadecimal literals.
+  {
+auto a1 = 0x1z;
+auto a2 = 0x1Z;
+
+auto a3 = 0x1uz;
+auto a4 = 0x1uZ;
+auto a5 = 0x1Uz;
+auto a6 = 0x1UZ;
+
+auto a7 = 0x1zu;
+auto a8 = 0x1Zu;
+auto a9 = 0x1zU;
+auto a10 = 0x1ZU;
+
+auto a11 = 0x1'2z;
+auto a12 = 0x1'2Z;
+  }
+  // Binary literals.
+  {
+auto a1 = 0b1z;
+auto a2 = 0b1Z;
+
+auto a3 = 0b1uz;
+auto a4 = 0b1uZ;
+auto a5 = 0b1Uz;
+auto a6 = 0b1UZ;
+
+auto a7 = 0b1zu;
+auto a8 = 0b1Zu;
+auto a9 = 0b1zU;
+auto a10 = 0b1ZU;
+
+auto a11 = 0b1'1z;
+auto a12 = 0b1'1Z;
+  }
+  // Octal literals.
+  {
+auto a1 = 01z;
+auto a2 = 01Z;
+
+auto a3 = 01uz;
+auto a4 = 01uZ;
+auto a5 = 01Uz;
+auto a6 = 01UZ;
+
+auto a7 = 01zu;
+auto a8 = 01Zu;
+auto a9 = 01zU;
+auto 

[PATCH] D99456: [C++2b] Support size_t literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev created this revision.
AntonBikineev added a reviewer: rsmith.
AntonBikineev requested review of this revision.
Herald added a project: clang.

This adds support for C++2b z/uz suffixes for size_t literals (P0330).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99456

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Lex/LiteralSupport.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Lex/PPExpressions.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/Lexer/size_t-literal.cpp
  clang/test/SemaCXX/size_t-literal.cpp

Index: clang/test/SemaCXX/size_t-literal.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/size_t-literal.cpp
@@ -0,0 +1,80 @@
+// RUN: %clang_cc1 -std=c++2b -Werror=c++2b-extensions -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++20 -Werror=c++2b-extensions -fsyntax-only -verify %s
+
+typedef __SIZE_TYPE__ size_t;
+// Assume ptrdiff_t is the signed integer type corresponding to size_t.
+typedef __PTRDIFF_TYPE__ ssize_t;
+
+template 
+struct is_same { static constexpr bool value = false; };
+
+template 
+struct is_same { static constexpr bool value = true; };
+
+#if __cplusplus >= 202101L
+//  expected-no-diagnostics
+#endif
+
+void SSizeT() {
+  auto a1 = 1z;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a2 = 1Z;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+}
+
+void SizeT() {
+  auto a1 = 1uz;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a2 = 1uZ;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a3 = 1Uz;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a4 = 1UZ;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a5 = 1zu;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a6 = 1Zu;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a7 = 1zU;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+
+  auto a8 = 1ZU;
+#if __cplusplus < 202101L
+  // expected-error@-2 {{size_t suffix for literals is a C++2b extension}}
+#endif
+  static_assert(is_same::value);
+}
Index: clang/test/Lexer/size_t-literal.cpp
===
--- /dev/null
+++ clang/test/Lexer/size_t-literal.cpp
@@ -0,0 +1,154 @@
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify -ferror-limit=0 %s
+
+void ValidSuffix() {
+  // Decimal literals.
+  {
+auto a1 = 1z;
+auto a2 = 1Z;
+
+auto a3 = 1uz;
+auto a4 = 1uZ;
+auto a5 = 1Uz;
+auto a6 = 1UZ;
+
+auto a7 = 1zu;
+auto a8 = 1Zu;
+auto a9 = 1zU;
+auto a10 = 1ZU;
+
+auto a11 = 1'2z;
+auto a12 = 1'2Z;
+  }
+  // Hexadecimal literals.
+  {
+auto a1 = 0x1z;
+auto a2 = 0x1Z;
+
+auto a3 = 0x1uz;
+auto a4 = 0x1uZ;
+auto a5 = 0x1Uz;
+auto a6 = 0x1UZ;
+
+auto a7 = 0x1zu;
+auto a8 = 0x1Zu;
+auto a9 = 0x1zU;
+auto a10 = 0x1ZU;
+
+auto a11 = 0x1'2z;
+auto a12 = 0x1'2Z;
+  }
+  // Binary literals.
+  {
+auto a1 = 0b1z;
+auto a2 = 0b1Z;
+
+auto a3 = 0b1uz;
+auto a4 = 0b1uZ;
+auto a5 = 0b1Uz;
+auto a6 = 0b1UZ;
+
+auto a7 = 0b1zu;
+auto a8 = 0b1Zu;
+auto a9 = 0b1zU;
+auto a10 = 0b1ZU;
+
+auto a11 = 0b1'1z;
+auto a12 = 0b1'1Z;
+  }
+  // Octal literals.
+  {
+auto a1 = 01z;
+auto a2 = 01Z;
+
+auto a3 = 01uz;
+auto a4 = 01uZ;
+auto a5 = 01Uz;
+auto a6 = 01UZ;
+
+auto a7 = 01zu;
+auto a8 = 01Zu;
+auto a9 = 01zU;
+auto a10 = 01ZU;
+
+auto a11 = 0'1z;
+auto a12 = 0'1Z;
+  }
+}
+
+void InvalidSuffix() {
+  // Long.
+  {
+auto a1 = 1lz; // expected-error {{invalid suffix}}
+auto a2 = 1lZ; // expected-error {{invalid suffix}}
+auto a3 = 1Lz; // expected-error {{invalid suffix}}
+auto a4 = 1LZ; // expected-error {{invalid suffix}}
+
+auto a5 = 1zl; // expected-error {{invalid suffix}}
+auto a6 = 1Zl; // expected-error {{invalid suffix}}
+auto a7 = 1zL; // expected-error {{invalid suffix}}
+auto a8 = 1ZL; // 

[PATCH] D99396: [clang] Support C++20's using ENUM_ID::IDENTIFIER

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev created this revision.
AntonBikineev added a reviewer: rsmith.
AntonBikineev added a project: clang.
AntonBikineev requested review of this revision.

This change merely allows nested-name-specifiers for enums in using
declarations but doesn't add support for the "using-enum-declaration"
syntax proposed in P1099 .


Repository:
  rC Clang

https://reviews.llvm.org/D99396

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/cxx20-using-enum.cpp
  clang/test/SemaCXX/enum-scoped.cpp

Index: clang/test/SemaCXX/enum-scoped.cpp
===
--- clang/test/SemaCXX/enum-scoped.cpp
+++ clang/test/SemaCXX/enum-scoped.cpp
@@ -301,7 +301,7 @@
   int E::*p; // expected-error {{does not point into a class}}
   using E::f; // expected-error {{no member named 'f'}}
 
-  using E::a; // expected-error {{using declaration cannot refer to a scoped enumerator}}
+  using E::a; // expected-error {{using declaration cannot refer to a scoped enumerator before C++20}}
   E b = a; // expected-error {{undeclared}}
 }
 
Index: clang/test/SemaCXX/cxx20-using-enum.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx20-using-enum.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
+
+enum class E {
+  e1,
+  e2
+};
+
+void foo() {
+  using E::e1;
+  auto a = e1;
+}
+
+namespace N {
+using E::e2;
+constexpr auto a = e2;
+} // namespace N
+
+struct C {
+  using E::e1;
+  E e = e1;
+};
+
+namespace N1 {
+enum class E {
+  e1,
+};
+using E::e1;
+auto a = e1;
+} // namespace N1
+
+enum class E1 {
+  e1 // expected-note{{conflicting declaration}}
+};
+enum class E2 {
+  e1 // expected-note{{target of using declaration}}
+};
+using E1::e1;
+using E2::e1; // expected-error{{target of using declaration conflicts with declaration already in scope}}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -12231,7 +12231,8 @@
   // C++14 [namespace.udecl]p7:
   // A using-declaration shall not name a scoped enumerator.
   if (auto *ED = R.getAsSingle()) {
-if (cast(ED->getDeclContext())->isScoped()) {
+if (!LangOpts.CPlusPlus20 &&
+cast(ED->getDeclContext())->isScoped()) {
   Diag(IdentLoc, diag::err_using_decl_can_not_refer_to_scoped_enum)
 << SS.getRange();
   return BuildInvalid();
@@ -12392,10 +12393,13 @@
 // C++03 [namespace.udecl]p3:
 // C++0x [namespace.udecl]p8:
 //   A using-declaration for a class member shall be a member-declaration.
+// C++20 [namespace.udecl]p7:
+//   A using-declaration that names a class member other than an enumerator
+//   shall be a member-declaration.
 
 // If we weren't able to compute a valid scope, it might validly be a
-// dependent class scope or a dependent enumeration unscoped scope. If
-// we have a 'typename' keyword, the scope must resolve to a class type.
+// dependent class scope or a dependent scoped/unscoped enumeration scope.
+// If we have a 'typename' keyword, the scope must resolve to a class type.
 if ((HasTypename && !NamedContext) ||
 (NamedContext && NamedContext->getRedeclContext()->isRecord())) {
   auto *RD = NamedContext
@@ -12484,7 +12488,11 @@
 return false;
   }
 
-  if (!NamedContext->isRecord()) {
+  bool IsCXX20ScopedEnum = LangOpts.CPlusPlus20 &&
+   NamedContext->getDeclKind() == Decl::Enum &&
+   cast(NamedContext)->isScoped();
+
+  if (!NamedContext->isRecord() && !IsCXX20ScopedEnum) {
 // Ideally this would point at the last name in the specifier,
 // but we don't have that level of source info.
 Diag(SS.getRange().getBegin(),
@@ -12503,6 +12511,13 @@
 //   nested-name-specifier shall name a base class of the class
 //   being defined.
 
+// C++20 [namespace.udecl]p3:
+//   In a using-declaration used as a member-declaration, each
+//   using-declarator shall either name an enumerator or have a
+//   nested-name-specifier naming a base class of the class being defined.
+if (IsCXX20ScopedEnum)
+  return false;
+
 if (cast(CurContext)->isProvablyNotDerivedFrom(
  cast(NamedContext))) {
   if (CurContext == NamedContext) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -539,7 +539,7 @@
 def err_using_decl_can_not_refer_to_namespace : Error<
   "using declaration cannot refer to a namespace">;
 def err_using_decl_can_not_refer_to_scoped_enum : Error<
-  "using declaration cannot refer to a scoped enumerator">;
+ 

[PATCH] D97951: [Sema] Fix diagnostics for one-byte length modifier

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

Thanks Aaron for taking a look!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97951

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

[PATCH] D97951: [Sema] Fix diagnostics for one-byte length modifier

[Anton Bikineev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4f8e299785e8: [Sema] Fix diagnostics for one-byte length 
modifier (authored by AntonBikineev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97951

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/FixIt/format.m


Index: clang/test/FixIt/format.m
===
--- clang/test/FixIt/format.m
+++ clang/test/FixIt/format.m
@@ -169,6 +169,12 @@
 
   NSLog(@"%@", 'abcd'); // expected-warning{{format specifies type 'id' but 
the argument has type 'int'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
+
+  NSLog(@"%hhd", 'a'); // expected-warning{{format specifies type 'char' but 
the argument has type 'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:15}:"%d"
+
+  NSLog(@"%hhu", 'a'); // expected-warning{{format specifies type 'unsigned 
char' but the argument has type 'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:15}:"%d"
 }
 
 void multichar_constants_false_negative() {
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -8730,8 +8730,11 @@
   } else if (const CharacterLiteral *CL = dyn_cast(E)) {
 // Special case for 'a', which has type 'int' in C.
 // Note, however, that we do /not/ want to treat multibyte constants like
-// 'MooV' as characters! This form is deprecated but still exists.
-if (ExprTy == S.Context.IntTy)
+// 'MooV' as characters! This form is deprecated but still exists. In
+// addition, don't treat expressions as of type 'char' if one byte length
+// modifier is provided.
+if (ExprTy == S.Context.IntTy &&
+FS.getLengthModifier().getKind() != LengthModifier::AsChar)
   if (llvm::isUIntN(S.Context.getCharWidth(), CL->getValue()))
 ExprTy = S.Context.CharTy;
   }


Index: clang/test/FixIt/format.m
===
--- clang/test/FixIt/format.m
+++ clang/test/FixIt/format.m
@@ -169,6 +169,12 @@
 
   NSLog(@"%@", 'abcd'); // expected-warning{{format specifies type 'id' but the argument has type 'int'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
+
+  NSLog(@"%hhd", 'a'); // expected-warning{{format specifies type 'char' but the argument has type 'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:15}:"%d"
+
+  NSLog(@"%hhu", 'a'); // expected-warning{{format specifies type 'unsigned char' but the argument has type 'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:15}:"%d"
 }
 
 void multichar_constants_false_negative() {
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -8730,8 +8730,11 @@
   } else if (const CharacterLiteral *CL = dyn_cast(E)) {
 // Special case for 'a', which has type 'int' in C.
 // Note, however, that we do /not/ want to treat multibyte constants like
-// 'MooV' as characters! This form is deprecated but still exists.
-if (ExprTy == S.Context.IntTy)
+// 'MooV' as characters! This form is deprecated but still exists. In
+// addition, don't treat expressions as of type 'char' if one byte length
+// modifier is provided.
+if (ExprTy == S.Context.IntTy &&
+FS.getLengthModifier().getKind() != LengthModifier::AsChar)
   if (llvm::isUIntN(S.Context.getCharWidth(), CL->getValue()))
 ExprTy = S.Context.CharTy;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97951: [Sema] Fix diagnostics for one-byte length modifier

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev created this revision.
AntonBikineev added a reviewer: jordan_rose.
AntonBikineev added a project: clang.
AntonBikineev requested review of this revision.

In case a char-literal of type int (C/ObjectiveC) corresponds to a
format specifier with the %hh length modifier, don't treat the literal
as of type char for issuing diagnostics, as otherwise this results in:

printf("%hhd", 'e');
warning: format specifies type 'char' but the argument has type 'char'.


Repository:
  rC Clang

https://reviews.llvm.org/D97951

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/FixIt/format.m


Index: clang/test/FixIt/format.m
===
--- clang/test/FixIt/format.m
+++ clang/test/FixIt/format.m
@@ -169,6 +169,12 @@
 
   NSLog(@"%@", 'abcd'); // expected-warning{{format specifies type 'id' but 
the argument has type 'int'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
+
+  NSLog(@"%hhd", 'a'); // expected-warning{{format specifies type 'char' but 
the argument has type 'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:15}:"%d"
+
+  NSLog(@"%hhu", 'a'); // expected-warning{{format specifies type 'unsigned 
char' but the argument has type 'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:15}:"%d"
 }
 
 void multichar_constants_false_negative() {
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -8650,8 +8650,11 @@
   } else if (const CharacterLiteral *CL = dyn_cast(E)) {
 // Special case for 'a', which has type 'int' in C.
 // Note, however, that we do /not/ want to treat multibyte constants like
-// 'MooV' as characters! This form is deprecated but still exists.
-if (ExprTy == S.Context.IntTy)
+// 'MooV' as characters! This form is deprecated but still exists. In
+// addition, don't treat expressions as of type 'char' if one byte length
+// modifier is provided.
+if (ExprTy == S.Context.IntTy &&
+FS.getLengthModifier().getKind() != LengthModifier::AsChar)
   if (llvm::isUIntN(S.Context.getCharWidth(), CL->getValue()))
 ExprTy = S.Context.CharTy;
   }


Index: clang/test/FixIt/format.m
===
--- clang/test/FixIt/format.m
+++ clang/test/FixIt/format.m
@@ -169,6 +169,12 @@
 
   NSLog(@"%@", 'abcd'); // expected-warning{{format specifies type 'id' but the argument has type 'int'}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
+
+  NSLog(@"%hhd", 'a'); // expected-warning{{format specifies type 'char' but the argument has type 'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:15}:"%d"
+
+  NSLog(@"%hhu", 'a'); // expected-warning{{format specifies type 'unsigned char' but the argument has type 'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:15}:"%d"
 }
 
 void multichar_constants_false_negative() {
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -8650,8 +8650,11 @@
   } else if (const CharacterLiteral *CL = dyn_cast(E)) {
 // Special case for 'a', which has type 'int' in C.
 // Note, however, that we do /not/ want to treat multibyte constants like
-// 'MooV' as characters! This form is deprecated but still exists.
-if (ExprTy == S.Context.IntTy)
+// 'MooV' as characters! This form is deprecated but still exists. In
+// addition, don't treat expressions as of type 'char' if one byte length
+// modifier is provided.
+if (ExprTy == S.Context.IntTy &&
+FS.getLengthModifier().getKind() != LengthModifier::AsChar)
   if (llvm::isUIntN(S.Context.getCharWidth(), CL->getValue()))
 ExprTy = S.Context.CharTy;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77477: tsan: don't instrument __attribute__((naked)) functions

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

Thanks for taking a look!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77477



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

[PATCH] D77477: tsan: don't instrument __attribute__((naked)) functions

[Anton Bikineev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9e1ccec8d529: tsan: dont instrument 
__attribute__((naked)) functions (authored by AntonBikineev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77477

Files:
  llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
  llvm/test/Instrumentation/ThreadSanitizer/tsan_basic.ll


Index: llvm/test/Instrumentation/ThreadSanitizer/tsan_basic.ll
===
--- llvm/test/Instrumentation/ThreadSanitizer/tsan_basic.ll
+++ llvm/test/Instrumentation/ThreadSanitizer/tsan_basic.ll
@@ -78,5 +78,18 @@
   call void @SwiftError(i8** %0)
   ret void
 }
+
+; CHECK-LABEL: @NakedTest(i32* %a)
+; CHECK-NEXT:   call void @foo()
+; CHECK-NEXT:   %tmp1 = load i32, i32* %a, align 4
+; CHECK-NEXT:   ret i32 %tmp1
+define i32 @NakedTest(i32* %a) naked sanitize_thread {
+  call void @foo()
+  %tmp1 = load i32, i32* %a, align 4
+  ret i32 %tmp1
+}
+
+declare void @foo() nounwind
+
 ; CHECK: define internal void @tsan.module_ctor()
 ; CHECK: call void @__tsan_init()
Index: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
@@ -441,6 +441,11 @@
   // the module constructor.
   if (F.getName() == kTsanModuleCtorName)
 return false;
+  // Naked functions can not have prologue/epilogue
+  // (__tsan_func_entry/__tsan_func_exit) generated, so don't instrument them 
at
+  // all.
+  if (F.hasFnAttribute(Attribute::Naked))
+return false;
   initialize(*F.getParent());
   SmallVector AllLoadsAndStores;
   SmallVector LocalLoadsAndStores;


Index: llvm/test/Instrumentation/ThreadSanitizer/tsan_basic.ll
===
--- llvm/test/Instrumentation/ThreadSanitizer/tsan_basic.ll
+++ llvm/test/Instrumentation/ThreadSanitizer/tsan_basic.ll
@@ -78,5 +78,18 @@
   call void @SwiftError(i8** %0)
   ret void
 }
+
+; CHECK-LABEL: @NakedTest(i32* %a)
+; CHECK-NEXT:   call void @foo()
+; CHECK-NEXT:   %tmp1 = load i32, i32* %a, align 4
+; CHECK-NEXT:   ret i32 %tmp1
+define i32 @NakedTest(i32* %a) naked sanitize_thread {
+  call void @foo()
+  %tmp1 = load i32, i32* %a, align 4
+  ret i32 %tmp1
+}
+
+declare void @foo() nounwind
+
 ; CHECK: define internal void @tsan.module_ctor()
 ; CHECK: call void @__tsan_init()
Index: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
@@ -441,6 +441,11 @@
   // the module constructor.
   if (F.getName() == kTsanModuleCtorName)
 return false;
+  // Naked functions can not have prologue/epilogue
+  // (__tsan_func_entry/__tsan_func_exit) generated, so don't instrument them at
+  // all.
+  if (F.hasFnAttribute(Attribute::Naked))
+return false;
   initialize(*F.getParent());
   SmallVector AllLoadsAndStores;
   SmallVector LocalLoadsAndStores;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd36a03331026: [clang-tidy] New checker 
performance-trivially-destructible-check (authored by AntonBikineev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69435

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp
  clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.h
  clang-tools-extra/clang-tidy/utils/Matchers.h
  clang-tools-extra/clang-tidy/utils/TypeTraits.cpp
  clang-tools-extra/clang-tidy/utils/TypeTraits.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp
@@ -0,0 +1,84 @@
+// RUN: %check_clang_tidy %s performance-trivially-destructible %t
+// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,performance-trivially-destructible' -fix
+// RUN: clang-tidy %t.cpp -checks='-*,performance-trivially-destructible' -warnings-as-errors='-*,performance-trivially-destructible'
+
+struct TriviallyDestructible1 {
+  int a;
+};
+
+struct TriviallyDestructible2 : TriviallyDestructible1 {
+  ~TriviallyDestructible2() = default;
+  TriviallyDestructible1 b;
+};
+
+struct NotTriviallyDestructible1 : TriviallyDestructible2 {
+  ~NotTriviallyDestructible1();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'NotTriviallyDestructible1' can be made trivially destructible by defaulting the destructor on its first declaration [performance-trivially-destructible]
+  // CHECK-FIXES: ~NotTriviallyDestructible1() = default;
+  TriviallyDestructible2 b;
+};
+
+NotTriviallyDestructible1::~NotTriviallyDestructible1() = default; // to-be-removed
+// CHECK-MESSAGES: :[[@LINE-1]]:28: note: destructor definition is here
+// CHECK-FIXES: {{^}}// to-be-removed
+
+// Don't emit for class template with type-dependent fields.
+template 
+struct MaybeTriviallyDestructible1 {
+  ~MaybeTriviallyDestructible1() noexcept;
+  T t;
+};
+
+template 
+MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default;
+
+// Don't emit for specializations.
+template struct MaybeTriviallyDestructible1;
+
+// Don't emit for class template with type-dependent bases.
+template 
+struct MaybeTriviallyDestructible2 : T {
+  ~MaybeTriviallyDestructible2() noexcept;
+};
+
+template 
+MaybeTriviallyDestructible2::~MaybeTriviallyDestructible2() noexcept = default;
+
+// Emit for templates without dependent bases and fields.
+template 
+struct MaybeTriviallyDestructible1 {
+  ~MaybeTriviallyDestructible1() noexcept;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'MaybeTriviallyDestructible1' can be made trivially destructible by defaulting the destructor on its first declaration [performance-trivially-destructible]
+  // CHECK-FIXES: ~MaybeTriviallyDestructible1() noexcept = default;
+  TriviallyDestructible1 t;
+};
+
+template 
+MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default; // to-be-removed
+// CHECK-MESSAGES: :[[@LINE-1]]:35: note: destructor definition is here
+// CHECK-FIXES: {{^}}// to-be-removed
+
+// Emit for explicit specializations.
+template <>
+struct MaybeTriviallyDestructible1: TriviallyDestructible1 {
+  ~MaybeTriviallyDestructible1() noexcept;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'MaybeTriviallyDestructible1' can be made trivially destructible by defaulting the destructor on its first declaration [performance-trivially-destructible]
+  // CHECK-FIXES: ~MaybeTriviallyDestructible1() noexcept = default;
+};
+
+MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default; // to-be-removed
+// CHECK-MESSAGES: :[[@LINE-1]]:38: note: destructor definition is here
+// CHECK-FIXES: {{^}}// to-be-removed
+
+struct NotTriviallyDestructible2 {
+  virtual ~NotTriviallyDestructible2();
+};
+
+NotTriviallyDestructible2::~NotTriviallyDestructible2() = default;
+
+struct NotTriviallyDestructible3: NotTriviallyDestructible2 {
+  ~NotTriviallyDestructible3();
+};
+
+NotTriviallyDestructible3::~NotTriviallyDestructible3() = default;
Index: clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst
@@ -0,0 +1,15 @@
+.. title:: clang-tidy - 

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

In D69435#1729580 , @gribozavr2 wrote:

> Alright then. (Although I don't know whether the redeclaration chain always 
> models that wording in the standard, and what that wording means when we 
> don't have a linear ordering of text -- that is, in modules.)
>
> Do you have commit access or do you need me to commit for you?


Thanks for taking the look, I'll commit.


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

https://reviews.llvm.org/D69435



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:45
+  isDefaulted(),
+  unless(anyOf(isFirstDecl(), isVirtual(),
+   ofClass(cxxRecordDecl(

gribozavr2 wrote:
> The "isFirstDecl" part probably will always work, however, it is semantically 
> not really the right predicate to check. What we want to check is where the 
> constructor is declared -- in the class or outside the class, correct? If so, 
> then the matcher should be looking at the DeclContext of the constructor decl.
Thanks for the comment.
> What we want to check is where the constructor is declared -- in the class or 
> outside the class, correct? 
I was following the Standard's wording, which says that trivial destructor is 
not user-provided, where [[ 
http://eel.is/c++draft/dcl.fct.def.default#def:user-provided | user-provided ]] 
means
> user-declared and not explicitly defaulted or deleted on its **first 
> declaration**.
Technically, I think both ways should work as C++ doesn't allow more than 2 
special function declarations.


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

https://reviews.llvm.org/D69435



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

> There is no difference in perf for GCC.

Yes, but that's because gcc still optimizes the call to noinlined function. In 
the common scenario which this check addresses (destructor defaulted in .cpp 
file) gcc is not able to optimize the call.


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

https://reviews.llvm.org/D69435



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

BTW, this very contrived benchmark shows 5x: 
http://quick-bench.com/sczBi_lVndKut9jOj4UofC0HYew


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

https://reviews.llvm.org/D69435



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

> Did you see some significant perf improvements for Chromium?

I don't see major improvements in browser benchmarks like Speedometer, but in 
the Blink garbage collector, which queues destructors to be executed later, 
this change proved to reduce the number of them by 5%. That, in turn, reduces 
GC time on the main thread which is significant for latency.


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

https://reviews.llvm.org/D69435



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

Roman, thanks for reviewing and the ideas.

> Can you be more specific what the question is?

I meant to ping if anybody has more comments on this patch. Otherwise I would 
go ahead and submit it.


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

https://reviews.llvm.org/D69435



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

Does anybody have suggestions on this?


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

https://reviews.llvm.org/D69435



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev updated this revision to Diff 226773.
AntonBikineev added a comment.

Thanks for the suggestions!

Added isTriviallyDestructible matcher to utils/Matchers.h.


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

https://reviews.llvm.org/D69435

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp
  clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.h
  clang-tools-extra/clang-tidy/utils/Matchers.h
  clang-tools-extra/clang-tidy/utils/TypeTraits.cpp
  clang-tools-extra/clang-tidy/utils/TypeTraits.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp
@@ -0,0 +1,84 @@
+// RUN: %check_clang_tidy %s performance-trivially-destructible %t
+// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,performance-trivially-destructible' -fix
+// RUN: clang-tidy %t.cpp -checks='-*,performance-trivially-destructible' -warnings-as-errors='-*,performance-trivially-destructible'
+
+struct TriviallyDestructible1 {
+  int a;
+};
+
+struct TriviallyDestructible2 : TriviallyDestructible1 {
+  ~TriviallyDestructible2() = default;
+  TriviallyDestructible1 b;
+};
+
+struct NotTriviallyDestructible1 : TriviallyDestructible2 {
+  ~NotTriviallyDestructible1();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'NotTriviallyDestructible1' can be made trivially destructible by defaulting the destructor on its first declaration [performance-trivially-destructible]
+  // CHECK-FIXES: ~NotTriviallyDestructible1() = default;
+  TriviallyDestructible2 b;
+};
+
+NotTriviallyDestructible1::~NotTriviallyDestructible1() = default; // to-be-removed
+// CHECK-MESSAGES: :[[@LINE-1]]:28: note: destructor definition is here
+// CHECK-FIXES: {{^}}// to-be-removed
+
+// Don't emit for class template with type-dependent fields.
+template 
+struct MaybeTriviallyDestructible1 {
+  ~MaybeTriviallyDestructible1() noexcept;
+  T t;
+};
+
+template 
+MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default;
+
+// Don't emit for specializations.
+template struct MaybeTriviallyDestructible1;
+
+// Don't emit for class template with type-dependent bases.
+template 
+struct MaybeTriviallyDestructible2 : T {
+  ~MaybeTriviallyDestructible2() noexcept;
+};
+
+template 
+MaybeTriviallyDestructible2::~MaybeTriviallyDestructible2() noexcept = default;
+
+// Emit for templates without dependent bases and fields.
+template 
+struct MaybeTriviallyDestructible1 {
+  ~MaybeTriviallyDestructible1() noexcept;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'MaybeTriviallyDestructible1' can be made trivially destructible by defaulting the destructor on its first declaration [performance-trivially-destructible]
+  // CHECK-FIXES: ~MaybeTriviallyDestructible1() noexcept = default;
+  TriviallyDestructible1 t;
+};
+
+template 
+MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default; // to-be-removed
+// CHECK-MESSAGES: :[[@LINE-1]]:35: note: destructor definition is here
+// CHECK-FIXES: {{^}}// to-be-removed
+
+// Emit for explicit specializations.
+template <>
+struct MaybeTriviallyDestructible1: TriviallyDestructible1 {
+  ~MaybeTriviallyDestructible1() noexcept;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'MaybeTriviallyDestructible1' can be made trivially destructible by defaulting the destructor on its first declaration [performance-trivially-destructible]
+  // CHECK-FIXES: ~MaybeTriviallyDestructible1() noexcept = default;
+};
+
+MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default; // to-be-removed
+// CHECK-MESSAGES: :[[@LINE-1]]:38: note: destructor definition is here
+// CHECK-FIXES: {{^}}// to-be-removed
+
+struct NotTriviallyDestructible2 {
+  virtual ~NotTriviallyDestructible2();
+};
+
+NotTriviallyDestructible2::~NotTriviallyDestructible2() = default;
+
+struct NotTriviallyDestructible3: NotTriviallyDestructible2 {
+  ~NotTriviallyDestructible3();
+};
+
+NotTriviallyDestructible3::~NotTriviallyDestructible3() = default;
Index: clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst
@@ -0,0 +1,15 @@
+.. title:: clang-tidy - performance-trivially-destructible
+
+performance-trivially-destructible

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

Roman, could you please take another look at it?


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

https://reviews.llvm.org/D69435



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev updated this revision to Diff 226475.
AntonBikineev marked 2 inline comments as done.
AntonBikineev added a comment.

Thanks for the suggestions! Addressed some of them.


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

https://reviews.llvm.org/D69435

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp
  clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp
@@ -0,0 +1,80 @@
+// RUN: %check_clang_tidy %s performance-trivially-destructible %t
+
+struct TriviallyDestructible1 {
+  int a;
+};
+
+struct TriviallyDestructible2 : TriviallyDestructible1 {
+  TriviallyDestructible1 b;
+};
+
+struct NotTriviallyDestructible1 : TriviallyDestructible2 {
+  ~NotTriviallyDestructible1();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'NotTriviallyDestructible1' can be made trivially destructible by defaulting the destructor on it first declaration [performance-trivially-destructible]
+  // CHECK-FIXES: ~NotTriviallyDestructible1() = default;
+  TriviallyDestructible2 b;
+};
+
+NotTriviallyDestructible1::~NotTriviallyDestructible1() = default; // to-be-removed
+// CHECK-MESSAGES: :[[@LINE-1]]:28: note: destructor definition is here
+// CHECK-FIXES: {{^}}// to-be-removed
+
+// Don't emit for class template with type-dependent fields.
+template 
+struct MaybeTriviallyDestructible1 {
+  ~MaybeTriviallyDestructible1() noexcept;
+  T t;
+};
+
+template 
+MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default;
+
+// Don't emit for specializations.
+template struct MaybeTriviallyDestructible1;
+
+// Don't emit for class template with type-dependent bases.
+template 
+struct MaybeTriviallyDestructible2 : T {
+  ~MaybeTriviallyDestructible2() noexcept;
+};
+
+template 
+MaybeTriviallyDestructible2::~MaybeTriviallyDestructible2() noexcept = default;
+
+// Emit for templates without dependent bases and fields.
+template 
+struct MaybeTriviallyDestructible1 {
+  ~MaybeTriviallyDestructible1() noexcept;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'MaybeTriviallyDestructible1' can be made trivially destructible by defaulting the destructor on it first declaration [performance-trivially-destructible]
+  // CHECK-FIXES: ~MaybeTriviallyDestructible1() noexcept = default;
+  TriviallyDestructible1 t;
+};
+
+template 
+MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default; // to-be-removed
+// CHECK-MESSAGES: :[[@LINE-1]]:35: note: destructor definition is here
+// CHECK-FIXES: {{^}}// to-be-removed
+
+// Emit for explicit specializations.
+template <>
+struct MaybeTriviallyDestructible1: TriviallyDestructible1 {
+  ~MaybeTriviallyDestructible1() noexcept;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: class 'MaybeTriviallyDestructible1' can be made trivially destructible by defaulting the destructor on it first declaration [performance-trivially-destructible]
+  // CHECK-FIXES: ~MaybeTriviallyDestructible1() noexcept = default;
+};
+
+MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default; // to-be-removed
+// CHECK-MESSAGES: :[[@LINE-1]]:38: note: destructor definition is here
+// CHECK-FIXES: {{^}}// to-be-removed
+
+struct NotTriviallyDestructible2 {
+  virtual ~NotTriviallyDestructible2();
+};
+
+NotTriviallyDestructible2::~NotTriviallyDestructible2() = default;
+
+struct NotTriviallyDestructible3: NotTriviallyDestructible2 {
+  ~NotTriviallyDestructible3();
+};
+
+NotTriviallyDestructible3::~NotTriviallyDestructible3() = default;
Index: clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst
@@ -0,0 +1,15 @@
+.. title:: clang-tidy - performance-trivially-destructible
+
+performance-trivially-destructible
+==
+
+Finds types that could be made trivially-destructible by removing out-of-line
+defaulted destructor declarations.
+
+.. code-block:: c++
+
+   struct A: TrivialType {
+ ~A(); // Makes A non-trivially-destructible.
+ TrivialType trivial_fields;
+   };
+   A::~A() = default;
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- 

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev marked 9 inline comments as done.
AntonBikineev added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:22
+
+bool CheckPotentiallyTriviallyDestructible(const CXXDestructorDecl *Dtor) {
+  if (Dtor->isFirstDecl() || !Dtor->isExplicitlyDefaulted())

lebedev.ri wrote:
> I believe static functions are preferred to anon namespaces;
> most (if not all) of this checking should/could be done in the matcher itself,
> i.e. the check() could be called to unconditionally issue diag.
I couldn't find matchers to check triviallity of special functions or something 
like isFirstDecl(). Perhaps, would make sense to add some.



Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:50
+void TriviallyDestructibleCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(cxxDestructorDecl().bind("decl"), this);
+}

lebedev.ri wrote:
> https://en.cppreference.com/w/cpp/language/destructor says defaulted 
> destructor is C++11 feature,
> so not register if this isn't C++11?
Right. However, I think it will also make sense to extend the check for 
destructors with empty bodies.



Comment at: 
clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:77
+  << FirstDecl << FixItHint::CreateRemoval(FirstDeclRange)
+  << FixItHint::CreateRemoval(SecondDeclRange);
+  diag(MatchedDecl->getLocation(), "destructor definition is here",

lebedev.ri wrote:
> This always drops out-of-line defaulting.
> Is there/can there be any case where such defaultig should just be moved 
> in-line into the class?
> 
I've thought about this. I think it mostly comes down to code-style preference 
(rule-of-5 vs rule-of-0). But after giving it another thought, I think, the 
current implementation may lead to more surprises for the user, so probably 
moving //= default// to the first declaration would be a better idea :)



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp:1
+// RUN: %check_clang_tidy %s performance-trivially-destructible %t
+

lebedev.ri wrote:
> Do you also want to check that fix-it applies and result passes sema?
Aren't fix-its already checked by CHECK-FIXES?
It would be great to check if the result is compilable. Should I run another 
clang_tidy instance to check that or is there another way to do so?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69435



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

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev created this revision.
AntonBikineev added a reviewer: klimek.
AntonBikineev added projects: clang, clang-tools-extra.
Herald added subscribers: mgehre, xazax.hun, mgorny.

Checks for types which can be made trivially-destructible by removing
out-of-line defaulted destructor declarations.

The check is motivated by the work on C++ garbage collector in Blink
(rendering engine in Chrome), which strives to minimize destructor usage
and improve runtime of sweeping phase.

In the entire chromium codebase the check hits over ~2500 times.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D69435

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp
  clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-trivially-destructible.cpp
@@ -0,0 +1,68 @@
+// RUN: %check_clang_tidy %s performance-trivially-destructible %t
+
+struct TriviallyDestructible1 {
+  int a;
+};
+
+struct TriviallyDestructible2 : TriviallyDestructible1 {
+  TriviallyDestructible1 b;
+};
+
+struct NonTriviallyDestructible1 : TriviallyDestructible2 {
+  ~NonTriviallyDestructible1(); // to-be-removed
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: destructor '~NonTriviallyDestructible1' can be removed to make the class trivially destructible [performance-trivially-destructible]
+  // CHECK-FIXES: {{^}}  // to-be-removed
+  TriviallyDestructible2 b;
+};
+
+NonTriviallyDestructible1::~NonTriviallyDestructible1() = default; // to-be-removed
+// CHECK-MESSAGES: :[[@LINE-1]]:28: note: destructor definition is here
+// CHECK-FIXES: {{^}}// to-be-removed
+
+// Don't emit for class template with type-dependent fields.
+template 
+struct MaybeTriviallyDestructible1 {
+  ~MaybeTriviallyDestructible1() noexcept;
+  T t;
+};
+
+template 
+MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default;
+
+// Don't emit for specializations.
+template struct MaybeTriviallyDestructible1;
+
+// Don't emit for class template with type-dependent bases.
+template 
+struct MaybeTriviallyDestructible2 : T {
+  ~MaybeTriviallyDestructible2() noexcept;
+};
+
+template 
+MaybeTriviallyDestructible2::~MaybeTriviallyDestructible2() noexcept = default;
+
+// Emit for templates without dependent bases and fields.
+template 
+struct MaybeTriviallyDestructible1 {
+  ~MaybeTriviallyDestructible1() noexcept; // to-be-removed
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: destructor '~MaybeTriviallyDestructible1' can be removed to make the class trivially destructible [performance-trivially-destructible]
+  // CHECK-FIXES: {{^}}  // to-be-removed
+  TriviallyDestructible1 t;
+};
+
+template 
+MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default; // to-be-removed
+// CHECK-MESSAGES: :[[@LINE-1]]:35: note: destructor definition is here
+// CHECK-FIXES: {{^}}// to-be-removed
+
+// Emit for explicit specializations.
+template <>
+struct MaybeTriviallyDestructible1: TriviallyDestructible1 {
+  ~MaybeTriviallyDestructible1() noexcept; // to-be-removed
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: destructor '~MaybeTriviallyDestructible1' can be removed to make the class trivially destructible [performance-trivially-destructible]
+  // CHECK-FIXES: {{^}}  // to-be-removed
+};
+
+MaybeTriviallyDestructible1::~MaybeTriviallyDestructible1() noexcept = default; // to-be-removed
+// CHECK-MESSAGES: :[[@LINE-1]]:38: note: destructor definition is here
+// CHECK-FIXES: {{^}}// to-be-removed
Index: clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-trivially-destructible.rst
@@ -0,0 +1,15 @@
+.. title:: clang-tidy - performance-trivially-destructible
+
+performance-trivially-destructible
+==
+
+Finds types that could be made trivially-destrictuble by removing out-of-line
+defaulted destructor declarations.
+
+.. code-block:: c++
+
+   struct A: TrivialType {
+ ~A(); // Makes A non-trivially-destructible.
+ TrivialType trivial_fields;
+   };
+   A::~A() = default;
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ 

[PATCH] D65589: [clang] Fix mismatched args constructing AddressSpaceAttr.

[Anton Bikineev via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL368152: [clang] Fix mismatched args constructing 
AddressSpaceAttr. (authored by AntonBikineev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65589?vs=212882=213840#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65589

Files:
  cfe/trunk/lib/Sema/SemaType.cpp
  cfe/trunk/unittests/AST/ASTTraverserTest.cpp


Index: cfe/trunk/lib/Sema/SemaType.cpp
===
--- cfe/trunk/lib/Sema/SemaType.cpp
+++ cfe/trunk/lib/Sema/SemaType.cpp
@@ -5978,9 +5978,9 @@
 }
 
 ASTContext  = S.Context;
-auto *ASAttr = ::new (Ctx) AddressSpaceAttr(
-Attr.getRange(), Ctx, Attr.getAttributeSpellingListIndex(),
-static_cast(ASIdx));
+auto *ASAttr = ::new (Ctx)
+AddressSpaceAttr(Attr.getRange(), Ctx, static_cast(ASIdx),
+ Attr.getAttributeSpellingListIndex());
 
 // If the expression is not value dependent (not templated), then we can
 // apply the address space qualifiers just to the equivalent type.
Index: cfe/trunk/unittests/AST/ASTTraverserTest.cpp
===
--- cfe/trunk/unittests/AST/ASTTraverserTest.cpp
+++ cfe/trunk/unittests/AST/ASTTraverserTest.cpp
@@ -139,6 +139,8 @@
 { 
 };
 
+void parmvardecl_attr(struct A __attribute__((address_space(19)))*);
+
 )cpp");
 
   const FunctionDecl *Func = getFunctionNode(AST.get(), "func");
@@ -220,5 +222,16 @@
 R"cpp(
 TemplateArgument
 )cpp");
+
+  Func = getFunctionNode(AST.get(), "parmvardecl_attr");
+
+  const auto *Parm = Func->getParamDecl(0);
+  const auto TL = Parm->getTypeSourceInfo()->getTypeLoc();
+  ASSERT_TRUE(TL.getType()->isPointerType());
+
+  const auto ATL = TL.getNextTypeLoc().getAs();
+  const auto *AS = cast(ATL.getAttr());
+  EXPECT_EQ(toTargetAddressSpace(static_cast(AS->getAddressSpace())),
+19u);
 }
 } // namespace clang


Index: cfe/trunk/lib/Sema/SemaType.cpp
===
--- cfe/trunk/lib/Sema/SemaType.cpp
+++ cfe/trunk/lib/Sema/SemaType.cpp
@@ -5978,9 +5978,9 @@
 }
 
 ASTContext  = S.Context;
-auto *ASAttr = ::new (Ctx) AddressSpaceAttr(
-Attr.getRange(), Ctx, Attr.getAttributeSpellingListIndex(),
-static_cast(ASIdx));
+auto *ASAttr = ::new (Ctx)
+AddressSpaceAttr(Attr.getRange(), Ctx, static_cast(ASIdx),
+ Attr.getAttributeSpellingListIndex());
 
 // If the expression is not value dependent (not templated), then we can
 // apply the address space qualifiers just to the equivalent type.
Index: cfe/trunk/unittests/AST/ASTTraverserTest.cpp
===
--- cfe/trunk/unittests/AST/ASTTraverserTest.cpp
+++ cfe/trunk/unittests/AST/ASTTraverserTest.cpp
@@ -139,6 +139,8 @@
 { 
 };
 
+void parmvardecl_attr(struct A __attribute__((address_space(19)))*);
+
 )cpp");
 
   const FunctionDecl *Func = getFunctionNode(AST.get(), "func");
@@ -220,5 +222,16 @@
 R"cpp(
 TemplateArgument
 )cpp");
+
+  Func = getFunctionNode(AST.get(), "parmvardecl_attr");
+
+  const auto *Parm = Func->getParamDecl(0);
+  const auto TL = Parm->getTypeSourceInfo()->getTypeLoc();
+  ASSERT_TRUE(TL.getType()->isPointerType());
+
+  const auto ATL = TL.getNextTypeLoc().getAs();
+  const auto *AS = cast(ATL.getAttr());
+  EXPECT_EQ(toTargetAddressSpace(static_cast(AS->getAddressSpace())),
+19u);
 }
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65589: [clang] Fix mismatched args constructing AddressSpaceAttr.

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

Ping.


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

https://reviews.llvm.org/D65589



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

[PATCH] D65589: [clang] Fix mismatched args constructing AddressSpaceAttr.

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

Pinging for review..


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

https://reviews.llvm.org/D65589



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

[PATCH] D65589: [clang] Fix mismatched args constructing AddressSpaceAttr.

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev updated this revision to Diff 212882.
AntonBikineev added a comment.

Roman, -ast-dump shows the correct result, because the correct address space is 
encoded in qualifiers and is passed to Context.getAddrSpaceQualType(...) later 
in this function. This is why this bug is not observable from the AST. But if 
you try accessing real Attr* from TypeSourceInfo, you'll see this mismatch.


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

https://reviews.llvm.org/D65589

Files:
  clang/lib/Sema/SemaType.cpp
  clang/unittests/AST/ASTTraverserTest.cpp


Index: clang/unittests/AST/ASTTraverserTest.cpp
===
--- clang/unittests/AST/ASTTraverserTest.cpp
+++ clang/unittests/AST/ASTTraverserTest.cpp
@@ -139,6 +139,8 @@
 { 
 };
 
+void parmvardecl_attr(struct A __attribute__((address_space(19)))*);
+
 )cpp");
 
   const FunctionDecl *Func = getFunctionNode(AST.get(), "func");
@@ -220,5 +222,16 @@
 R"cpp(
 TemplateArgument
 )cpp");
+
+  Func = getFunctionNode(AST.get(), "parmvardecl_attr");
+
+  const auto *Parm = Func->getParamDecl(0);
+  const auto TL = Parm->getTypeSourceInfo()->getTypeLoc();
+  ASSERT_TRUE(TL.getType()->isPointerType());
+
+  const auto ATL = TL.getNextTypeLoc().getAs();
+  const auto *AS = cast(ATL.getAttr());
+  EXPECT_EQ(toTargetAddressSpace(static_cast(AS->getAddressSpace())),
+19u);
 }
 } // namespace clang
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5978,9 +5978,9 @@
 }
 
 ASTContext  = S.Context;
-auto *ASAttr = ::new (Ctx) AddressSpaceAttr(
-Attr.getRange(), Ctx, Attr.getAttributeSpellingListIndex(),
-static_cast(ASIdx));
+auto *ASAttr = ::new (Ctx)
+AddressSpaceAttr(Attr.getRange(), Ctx, static_cast(ASIdx),
+ Attr.getAttributeSpellingListIndex());
 
 // If the expression is not value dependent (not templated), then we can
 // apply the address space qualifiers just to the equivalent type.


Index: clang/unittests/AST/ASTTraverserTest.cpp
===
--- clang/unittests/AST/ASTTraverserTest.cpp
+++ clang/unittests/AST/ASTTraverserTest.cpp
@@ -139,6 +139,8 @@
 { 
 };
 
+void parmvardecl_attr(struct A __attribute__((address_space(19)))*);
+
 )cpp");
 
   const FunctionDecl *Func = getFunctionNode(AST.get(), "func");
@@ -220,5 +222,16 @@
 R"cpp(
 TemplateArgument
 )cpp");
+
+  Func = getFunctionNode(AST.get(), "parmvardecl_attr");
+
+  const auto *Parm = Func->getParamDecl(0);
+  const auto TL = Parm->getTypeSourceInfo()->getTypeLoc();
+  ASSERT_TRUE(TL.getType()->isPointerType());
+
+  const auto ATL = TL.getNextTypeLoc().getAs();
+  const auto *AS = cast(ATL.getAttr());
+  EXPECT_EQ(toTargetAddressSpace(static_cast(AS->getAddressSpace())),
+19u);
 }
 } // namespace clang
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5978,9 +5978,9 @@
 }
 
 ASTContext  = S.Context;
-auto *ASAttr = ::new (Ctx) AddressSpaceAttr(
-Attr.getRange(), Ctx, Attr.getAttributeSpellingListIndex(),
-static_cast(ASIdx));
+auto *ASAttr = ::new (Ctx)
+AddressSpaceAttr(Attr.getRange(), Ctx, static_cast(ASIdx),
+ Attr.getAttributeSpellingListIndex());
 
 // If the expression is not value dependent (not templated), then we can
 // apply the address space qualifiers just to the equivalent type.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65589: [clang] Fix mismatched args constructing AddressSpaceAttr.

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev created this revision.
AntonBikineev added reviewers: rsmith, klimek.
AntonBikineev added a project: clang.

Interestingly enough, but this wasn't caught in tests. I wonder if it's 
possible to write a test case for it.


Repository:
  rC Clang

https://reviews.llvm.org/D65589

Files:
  clang/lib/Sema/SemaType.cpp


Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5978,9 +5978,9 @@
 }
 
 ASTContext  = S.Context;
-auto *ASAttr = ::new (Ctx) AddressSpaceAttr(
-Attr.getRange(), Ctx, Attr.getAttributeSpellingListIndex(),
-static_cast(ASIdx));
+auto *ASAttr = ::new (Ctx)
+AddressSpaceAttr(Attr.getRange(), Ctx, static_cast(ASIdx),
+ Attr.getAttributeSpellingListIndex());
 
 // If the expression is not value dependent (not templated), then we can
 // apply the address space qualifiers just to the equivalent type.


Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5978,9 +5978,9 @@
 }
 
 ASTContext  = S.Context;
-auto *ASAttr = ::new (Ctx) AddressSpaceAttr(
-Attr.getRange(), Ctx, Attr.getAttributeSpellingListIndex(),
-static_cast(ASIdx));
+auto *ASAttr = ::new (Ctx)
+AddressSpaceAttr(Attr.getRange(), Ctx, static_cast(ASIdx),
+ Attr.getAttributeSpellingListIndex());
 
 // If the expression is not value dependent (not templated), then we can
 // apply the address space qualifiers just to the equivalent type.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65279: [clang] Fail for empty names in is*DerivedFrom matchers.

[Anton Bikineev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367022: [clang] Fail for empty names in is*DerivedFrom 
matchers. (authored by AntonBikineev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65279?vs=211734=211753#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65279

Files:
  cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
  cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp


Index: cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -566,6 +566,13 @@
 cxxRecordDecl(isDerivedFrom(namedDecl(hasName("X"));
 }
 
+TEST(DeclarationMatcher, IsDerivedFromEmptyName) {
+  const char *const Code = "class X {}; class Y : public X {};";
+  EXPECT_TRUE(notMatches(Code, cxxRecordDecl(isDerivedFrom("";
+  EXPECT_TRUE(notMatches(Code, cxxRecordDecl(isDirectlyDerivedFrom("";
+  EXPECT_TRUE(notMatches(Code, cxxRecordDecl(isSameOrDerivedFrom("";
+}
+
 TEST(DeclarationMatcher, IsLambda) {
   const auto IsLambda = cxxMethodDecl(ofClass(cxxRecordDecl(isLambda(;
   EXPECT_TRUE(matches("auto x = []{};", IsLambda));
Index: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
===
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
@@ -2639,7 +2639,8 @@
 
 /// Overloaded method as shortcut for \c isDerivedFrom(hasName(...)).
 AST_MATCHER_P_OVERLOAD(CXXRecordDecl, isDerivedFrom, std::string, BaseName, 1) 
{
-  assert(!BaseName.empty());
+  if (BaseName.empty())
+return false;
   return isDerivedFrom(hasName(BaseName)).matches(Node, Finder, Builder);
 }
 
@@ -2655,7 +2656,8 @@
 /// \c isSameOrDerivedFrom(hasName(...)).
 AST_MATCHER_P_OVERLOAD(CXXRecordDecl, isSameOrDerivedFrom, std::string,
BaseName, 1) {
-  assert(!BaseName.empty());
+  if (BaseName.empty())
+return false;
   return isSameOrDerivedFrom(hasName(BaseName)).matches(Node, Finder, Builder);
 }
 
@@ -2687,7 +2689,8 @@
 /// Overloaded method as shortcut for \c isDirectlyDerivedFrom(hasName(...)).
 AST_MATCHER_P_OVERLOAD(CXXRecordDecl, isDirectlyDerivedFrom, std::string,
BaseName, 1) {
-  assert(!BaseName.empty());
+  if (BaseName.empty())
+return false;
   return isDirectlyDerivedFrom(hasName(BaseName))
   .matches(Node, Finder, Builder);
 }


Index: cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -566,6 +566,13 @@
 cxxRecordDecl(isDerivedFrom(namedDecl(hasName("X"));
 }
 
+TEST(DeclarationMatcher, IsDerivedFromEmptyName) {
+  const char *const Code = "class X {}; class Y : public X {};";
+  EXPECT_TRUE(notMatches(Code, cxxRecordDecl(isDerivedFrom("";
+  EXPECT_TRUE(notMatches(Code, cxxRecordDecl(isDirectlyDerivedFrom("";
+  EXPECT_TRUE(notMatches(Code, cxxRecordDecl(isSameOrDerivedFrom("";
+}
+
 TEST(DeclarationMatcher, IsLambda) {
   const auto IsLambda = cxxMethodDecl(ofClass(cxxRecordDecl(isLambda(;
   EXPECT_TRUE(matches("auto x = []{};", IsLambda));
Index: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
===
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
@@ -2639,7 +2639,8 @@
 
 /// Overloaded method as shortcut for \c isDerivedFrom(hasName(...)).
 AST_MATCHER_P_OVERLOAD(CXXRecordDecl, isDerivedFrom, std::string, BaseName, 1) {
-  assert(!BaseName.empty());
+  if (BaseName.empty())
+return false;
   return isDerivedFrom(hasName(BaseName)).matches(Node, Finder, Builder);
 }
 
@@ -2655,7 +2656,8 @@
 /// \c isSameOrDerivedFrom(hasName(...)).
 AST_MATCHER_P_OVERLOAD(CXXRecordDecl, isSameOrDerivedFrom, std::string,
BaseName, 1) {
-  assert(!BaseName.empty());
+  if (BaseName.empty())
+return false;
   return isSameOrDerivedFrom(hasName(BaseName)).matches(Node, Finder, Builder);
 }
 
@@ -2687,7 +2689,8 @@
 /// Overloaded method as shortcut for \c isDirectlyDerivedFrom(hasName(...)).
 AST_MATCHER_P_OVERLOAD(CXXRecordDecl, isDirectlyDerivedFrom, std::string,
BaseName, 1) {
-  assert(!BaseName.empty());
+  if (BaseName.empty())
+return false;
   return isDirectlyDerivedFrom(hasName(BaseName))
   .matches(Node, Finder, Builder);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D65279: [clang] Fail for empty names in is*DerivedFrom matchers.

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev created this revision.
AntonBikineev added a reviewer: aaron.ballman.
Herald added a project: clang.

Repository:
  rC Clang

https://reviews.llvm.org/D65279

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp


Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -566,6 +566,13 @@
 cxxRecordDecl(isDerivedFrom(namedDecl(hasName("X"));
 }
 
+TEST(DeclarationMatcher, IsDerivedFromEmptyName) {
+  const char *const Code = "class X {}; class Y : public X {};";
+  EXPECT_TRUE(notMatches(Code, cxxRecordDecl(isDerivedFrom("";
+  EXPECT_TRUE(notMatches(Code, cxxRecordDecl(isDirectlyDerivedFrom("";
+  EXPECT_TRUE(notMatches(Code, cxxRecordDecl(isSameOrDerivedFrom("";
+}
+
 TEST(DeclarationMatcher, IsLambda) {
   const auto IsLambda = cxxMethodDecl(ofClass(cxxRecordDecl(isLambda(;
   EXPECT_TRUE(matches("auto x = []{};", IsLambda));
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2639,7 +2639,8 @@
 
 /// Overloaded method as shortcut for \c isDerivedFrom(hasName(...)).
 AST_MATCHER_P_OVERLOAD(CXXRecordDecl, isDerivedFrom, std::string, BaseName, 1) 
{
-  assert(!BaseName.empty());
+  if (BaseName.empty())
+return false;
   return isDerivedFrom(hasName(BaseName)).matches(Node, Finder, Builder);
 }
 
@@ -2655,7 +2656,8 @@
 /// \c isSameOrDerivedFrom(hasName(...)).
 AST_MATCHER_P_OVERLOAD(CXXRecordDecl, isSameOrDerivedFrom, std::string,
BaseName, 1) {
-  assert(!BaseName.empty());
+  if (BaseName.empty())
+return false;
   return isSameOrDerivedFrom(hasName(BaseName)).matches(Node, Finder, Builder);
 }
 
@@ -2687,7 +2689,8 @@
 /// Overloaded method as shortcut for \c isDirectlyDerivedFrom(hasName(...)).
 AST_MATCHER_P_OVERLOAD(CXXRecordDecl, isDirectlyDerivedFrom, std::string,
BaseName, 1) {
-  assert(!BaseName.empty());
+  if (BaseName.empty())
+return false;
   return isDirectlyDerivedFrom(hasName(BaseName))
   .matches(Node, Finder, Builder);
 }


Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -566,6 +566,13 @@
 cxxRecordDecl(isDerivedFrom(namedDecl(hasName("X"));
 }
 
+TEST(DeclarationMatcher, IsDerivedFromEmptyName) {
+  const char *const Code = "class X {}; class Y : public X {};";
+  EXPECT_TRUE(notMatches(Code, cxxRecordDecl(isDerivedFrom("";
+  EXPECT_TRUE(notMatches(Code, cxxRecordDecl(isDirectlyDerivedFrom("";
+  EXPECT_TRUE(notMatches(Code, cxxRecordDecl(isSameOrDerivedFrom("";
+}
+
 TEST(DeclarationMatcher, IsLambda) {
   const auto IsLambda = cxxMethodDecl(ofClass(cxxRecordDecl(isLambda(;
   EXPECT_TRUE(matches("auto x = []{};", IsLambda));
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2639,7 +2639,8 @@
 
 /// Overloaded method as shortcut for \c isDerivedFrom(hasName(...)).
 AST_MATCHER_P_OVERLOAD(CXXRecordDecl, isDerivedFrom, std::string, BaseName, 1) {
-  assert(!BaseName.empty());
+  if (BaseName.empty())
+return false;
   return isDerivedFrom(hasName(BaseName)).matches(Node, Finder, Builder);
 }
 
@@ -2655,7 +2656,8 @@
 /// \c isSameOrDerivedFrom(hasName(...)).
 AST_MATCHER_P_OVERLOAD(CXXRecordDecl, isSameOrDerivedFrom, std::string,
BaseName, 1) {
-  assert(!BaseName.empty());
+  if (BaseName.empty())
+return false;
   return isSameOrDerivedFrom(hasName(BaseName)).matches(Node, Finder, Builder);
 }
 
@@ -2687,7 +2689,8 @@
 /// Overloaded method as shortcut for \c isDirectlyDerivedFrom(hasName(...)).
 AST_MATCHER_P_OVERLOAD(CXXRecordDecl, isDirectlyDerivedFrom, std::string,
BaseName, 1) {
-  assert(!BaseName.empty());
+  if (BaseName.empty())
+return false;
   return isDirectlyDerivedFrom(hasName(BaseName))
   .matches(Node, Finder, Builder);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65092: [clang] Add isDirectlyDerivedFrom AST Matcher.

[Anton Bikineev via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL367010: [clang] Add isDirectlyDerivedFrom AST matcher. 
(authored by AntonBikineev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65092?vs=211513=211715#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65092

Files:
  cfe/trunk/docs/LibASTMatchersReference.html
  cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
  cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
  cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp
  cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
  cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: cfe/trunk/docs/LibASTMatchersReference.html
===
--- cfe/trunk/docs/LibASTMatchersReference.html
+++ cfe/trunk/docs/LibASTMatchersReference.html
@@ -2581,6 +2581,11 @@
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1CXXRecordDecl.html;>CXXRecordDeclisDirectlyDerivedFromstd::string BaseName
+Overloaded method as shortcut for isDirectlyDerivedFrom(hasName(...)).
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1CXXRecordDecl.html;>CXXRecordDeclisExplicitTemplateSpecialization
 Matches explicit template specializations of function, class, or
 static member variable template instantiations.
@@ -5263,6 +5268,26 @@
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1CXXRecordDecl.html;>CXXRecordDeclisDirectlyDerivedFromMatcherhttps://clang.llvm.org/doxygen/classclang_1_1NamedDecl.html;>NamedDecl Base
+Matches C++ classes that are directly derived from a class matching Base.
+
+Note that a class is not considered to be derived from itself.
+
+Example matches Y, C (Base == hasName("X"))
+  class X;
+  class Y : public X {};  // directly derived
+  class Z : public Y {};  // indirectly derived
+  typedef X A;
+  typedef A B;
+  class C : public B {};  // derived from a typedef of X
+
+In the following example, Bar matches isDerivedFrom(hasName("X")):
+  class Foo;
+  typedef Foo X;
+  class Bar : public Foo {};  // derived from a type that X is a typedef of
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1CXXRecordDecl.html;>CXXRecordDeclisSameOrDerivedFromMatcherhttps://clang.llvm.org/doxygen/classclang_1_1NamedDecl.html;>NamedDecl Base
 Similar to isDerivedFrom(), but also matches classes that directly
 match Base.
Index: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
===
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
@@ -2634,7 +2634,7 @@
 /// \endcode
 AST_MATCHER_P(CXXRecordDecl, isDerivedFrom,
   internal::Matcher, Base) {
-  return Finder->classIsDerivedFrom(, Base, Builder);
+  return Finder->classIsDerivedFrom(, Base, Builder, /*Directly=*/false);
 }
 
 /// Overloaded method as shortcut for \c isDerivedFrom(hasName(...)).
@@ -2659,6 +2659,38 @@
   return isSameOrDerivedFrom(hasName(BaseName)).matches(Node, Finder, Builder);
 }
 
+/// Matches C++ classes that are directly derived from a class matching \c Base.
+///
+/// Note that a class is not considered to be derived from itself.
+///
+/// Example matches Y, C (Base == hasName("X"))
+/// \code
+///   class X;
+///   class Y : public X {};  // directly derived
+///   class Z : public Y {};  // indirectly derived
+///   typedef X A;
+///   typedef A B;
+///   class C : public B {};  // derived from a typedef of X
+/// \endcode
+///
+/// In the following example, Bar matches isDerivedFrom(hasName("X")):
+/// \code
+///   class Foo;
+///   typedef Foo X;
+///   class Bar : public Foo {};  // derived from a type that X is a typedef of
+/// \endcode
+AST_MATCHER_P_OVERLOAD(CXXRecordDecl, isDirectlyDerivedFrom,
+   internal::Matcher, Base, 0) {
+  return Finder->classIsDerivedFrom(, Base, Builder, /*Directly=*/true);
+}
+
+/// Overloaded method as shortcut for \c isDirectlyDerivedFrom(hasName(...)).
+AST_MATCHER_P_OVERLOAD(CXXRecordDecl, isDirectlyDerivedFrom, std::string,
+   BaseName, 1) {
+  assert(!BaseName.empty());
+  return isDirectlyDerivedFrom(hasName(BaseName))
+  .matches(Node, Finder, Builder);
+}
 /// Matches the first method of a class or struct that satisfies \c
 /// InnerMatcher.
 ///
Index: cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -974,10 +974,11 @@
   /// Returns true if the given class is directly or indirectly derived
   /// from a base type matching \c base.
   ///
-  /// A class is 

[PATCH] D65092: [clang] Add isDirectlyDerivedFrom AST Matcher.

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev updated this revision to Diff 211513.
AntonBikineev marked an inline comment as not done.
AntonBikineev added a comment.

Missed that, thanks for the point!


Repository:
  rC Clang

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

https://reviews.llvm.org/D65092

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -331,6 +331,16 @@
   EXPECT_TRUE(notMatches("class Y;", IsDerivedFromX));
   EXPECT_TRUE(notMatches("", IsDerivedFromX));
 
+  DeclarationMatcher IsDirectlyDerivedFromX =
+  cxxRecordDecl(isDirectlyDerivedFrom("X"));
+
+  EXPECT_TRUE(
+  matches("class X {}; class Y : public X {};", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatches("class X {};", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatches("class X;", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatches("class Y;", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatches("", IsDirectlyDerivedFromX));
+
   DeclarationMatcher IsAX = cxxRecordDecl(isSameOrDerivedFrom("X"));
 
   EXPECT_TRUE(matches("class X {}; class Y : public X {};", IsAX));
@@ -341,13 +351,22 @@
 
   DeclarationMatcher ZIsDerivedFromX =
 cxxRecordDecl(hasName("Z"), isDerivedFrom("X"));
+  DeclarationMatcher ZIsDirectlyDerivedFromX =
+  cxxRecordDecl(hasName("Z"), isDirectlyDerivedFrom("X"));
   EXPECT_TRUE(
 matches("class X {}; class Y : public X {}; class Z : public Y {};",
 ZIsDerivedFromX));
+  EXPECT_TRUE(
+  notMatches("class X {}; class Y : public X {}; class Z : public Y {};",
+ ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(
 matches("class X {};"
   "template class Y : public X {};"
   "class Z : public Y {};", ZIsDerivedFromX));
+  EXPECT_TRUE(notMatches("class X {};"
+ "template class Y : public X {};"
+ "class Z : public Y {};",
+ ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(matches("class X {}; template class Z : public X {};",
   ZIsDerivedFromX));
   EXPECT_TRUE(
@@ -411,6 +430,9 @@
 matches("class X {}; class Y : public X {}; "
   "typedef Y V; typedef V W; class Z : public W {};",
 ZIsDerivedFromX));
+  EXPECT_TRUE(notMatches("class X {}; class Y : public X {}; "
+ "typedef Y V; typedef V W; class Z : public W {};",
+ ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(
 matches("template class X {}; "
   "template class A { class Z : public X {}; };",
@@ -467,6 +489,14 @@
   "template<> struct X<0> : public A {};"
   "struct B : public X<42> {};",
 cxxRecordDecl(hasName("B"), isDerivedFrom(recordDecl(hasName("A"));
+  EXPECT_TRUE(notMatches(
+  "struct A {};"
+  "template struct X;"
+  "template struct X : public X {};"
+  "template<> struct X<0> : public A {};"
+  "struct B : public X<42> {};",
+  cxxRecordDecl(hasName("B"),
+isDirectlyDerivedFrom(recordDecl(hasName("A"));
 
   // FIXME: Once we have better matchers for template type matching,
   // get rid of the Variable(...) matching and match the right template
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -108,6 +108,7 @@
   REGISTER_OVERLOADED_2(hasType);
   REGISTER_OVERLOADED_2(ignoringParens);
   REGISTER_OVERLOADED_2(isDerivedFrom);
+  REGISTER_OVERLOADED_2(isDirectlyDerivedFrom);
   REGISTER_OVERLOADED_2(isSameOrDerivedFrom);
   REGISTER_OVERLOADED_2(loc);
   REGISTER_OVERLOADED_2(pointsTo);
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -430,7 +430,8 @@
 
   bool classIsDerivedFrom(const CXXRecordDecl *Declaration,
   const Matcher ,
-  BoundNodesTreeBuilder *Builder) override;
+  BoundNodesTreeBuilder *Builder,
+  bool Directly) override;
 
   // Implements ASTMatchFinder::matchesChildOf.
   bool matchesChildOf(const ast_type_traits::DynTypedNode ,
@@ -817,7 +818,8 @@
 // derived from itself.
 bool MatchASTVisitor::classIsDerivedFrom(const CXXRecordDecl *Declaration,
  const Matcher ,
-  

[PATCH] D65092: [clang] Add isDirectlyDerivedFrom AST Matcher.

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev updated this revision to Diff 211508.
AntonBikineev marked an inline comment as done.
AntonBikineev added a comment.

Thanks for the comments!


Repository:
  rC Clang

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

https://reviews.llvm.org/D65092

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -331,6 +331,16 @@
   EXPECT_TRUE(notMatches("class Y;", IsDerivedFromX));
   EXPECT_TRUE(notMatches("", IsDerivedFromX));
 
+  DeclarationMatcher IsDirectlyDerivedFromX =
+  cxxRecordDecl(isDirectlyDerivedFrom("X"));
+
+  EXPECT_TRUE(
+  matches("class X {}; class Y : public X {};", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatches("class X {};", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatches("class X;", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatches("class Y;", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatches("", IsDirectlyDerivedFromX));
+
   DeclarationMatcher IsAX = cxxRecordDecl(isSameOrDerivedFrom("X"));
 
   EXPECT_TRUE(matches("class X {}; class Y : public X {};", IsAX));
@@ -341,13 +351,22 @@
 
   DeclarationMatcher ZIsDerivedFromX =
 cxxRecordDecl(hasName("Z"), isDerivedFrom("X"));
+  DeclarationMatcher ZIsDirectlyDerivedFromX =
+  cxxRecordDecl(hasName("Z"), isDirectlyDerivedFrom("X"));
   EXPECT_TRUE(
 matches("class X {}; class Y : public X {}; class Z : public Y {};",
 ZIsDerivedFromX));
+  EXPECT_TRUE(
+  notMatches("class X {}; class Y : public X {}; class Z : public Y {};",
+ ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(
 matches("class X {};"
   "template class Y : public X {};"
   "class Z : public Y {};", ZIsDerivedFromX));
+  EXPECT_TRUE(notMatches("class X {};"
+ "template class Y : public X {};"
+ "class Z : public Y {};",
+ ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(matches("class X {}; template class Z : public X {};",
   ZIsDerivedFromX));
   EXPECT_TRUE(
@@ -411,6 +430,9 @@
 matches("class X {}; class Y : public X {}; "
   "typedef Y V; typedef V W; class Z : public W {};",
 ZIsDerivedFromX));
+  EXPECT_TRUE(notMatches("class X {}; class Y : public X {}; "
+ "typedef Y V; typedef V W; class Z : public W {};",
+ ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(
 matches("template class X {}; "
   "template class A { class Z : public X {}; };",
@@ -467,6 +489,14 @@
   "template<> struct X<0> : public A {};"
   "struct B : public X<42> {};",
 cxxRecordDecl(hasName("B"), isDerivedFrom(recordDecl(hasName("A"));
+  EXPECT_TRUE(notMatches(
+  "struct A {};"
+  "template struct X;"
+  "template struct X : public X {};"
+  "template<> struct X<0> : public A {};"
+  "struct B : public X<42> {};",
+  cxxRecordDecl(hasName("B"),
+isDirectlyDerivedFrom(recordDecl(hasName("A"));
 
   // FIXME: Once we have better matchers for template type matching,
   // get rid of the Variable(...) matching and match the right template
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -430,7 +430,8 @@
 
   bool classIsDerivedFrom(const CXXRecordDecl *Declaration,
   const Matcher ,
-  BoundNodesTreeBuilder *Builder) override;
+  BoundNodesTreeBuilder *Builder,
+  bool Directly) override;
 
   // Implements ASTMatchFinder::matchesChildOf.
   bool matchesChildOf(const ast_type_traits::DynTypedNode ,
@@ -817,7 +818,8 @@
 // derived from itself.
 bool MatchASTVisitor::classIsDerivedFrom(const CXXRecordDecl *Declaration,
  const Matcher ,
- BoundNodesTreeBuilder *Builder) {
+ BoundNodesTreeBuilder *Builder,
+ bool Directly) {
   if (!Declaration->hasDefinition())
 return false;
   for (const auto  : Declaration->bases()) {
@@ -842,7 +844,7 @@
   *Builder = std::move(Result);
   return true;
 }
-if (classIsDerivedFrom(ClassDecl, Base, Builder))
+if (!Directly && classIsDerivedFrom(ClassDecl, Base, Builder, Directly))
   return true;
   }
   return false;
Index: 

[PATCH] D65092: [clang] Add isDirectlyDerivedFrom AST Matcher.

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2672
+   BaseName, 1) {
+  assert(!BaseName.empty());
+  return isDirectlyDerivedFrom(hasName(BaseName))

aaron.ballman wrote:
> I don't think this assertion is reasonable -- we should instead test this as 
> a predicate and return false if the base name is empty.
It's done in the same way as for existing isDerivedFrom and isSameOrDerivedFrom 
matchers. Maybe it would make sense to change all of them, but I guess it 
should rather be a separate commit.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65092



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

[PATCH] D65092: [clang] Add isDirectlyDerivedFrom AST Matcher.

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

Manuel, would you mind taking another look at the change please?


Repository:
  rC Clang

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

https://reviews.llvm.org/D65092



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

[PATCH] D65092: [clang] Add isDirectlyDerivedFrom AST Matcher.

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev updated this revision to Diff 211261.
AntonBikineev added a comment.

Thanks for the note, updated the diff with autogenerated 
LibASTMatcherReference.html.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65092

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -331,6 +331,16 @@
   EXPECT_TRUE(notMatches("class Y;", IsDerivedFromX));
   EXPECT_TRUE(notMatches("", IsDerivedFromX));
 
+  DeclarationMatcher IsDirectlyDerivedFromX =
+  cxxRecordDecl(isDirectlyDerivedFrom("X"));
+
+  EXPECT_TRUE(
+  matches("class X {}; class Y : public X {};", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatches("class X {};", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatches("class X;", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatches("class Y;", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatches("", IsDirectlyDerivedFromX));
+
   DeclarationMatcher IsAX = cxxRecordDecl(isSameOrDerivedFrom("X"));
 
   EXPECT_TRUE(matches("class X {}; class Y : public X {};", IsAX));
@@ -341,13 +351,22 @@
 
   DeclarationMatcher ZIsDerivedFromX =
 cxxRecordDecl(hasName("Z"), isDerivedFrom("X"));
+  DeclarationMatcher ZIsDirectlyDerivedFromX =
+  cxxRecordDecl(hasName("Z"), isDirectlyDerivedFrom("X"));
   EXPECT_TRUE(
 matches("class X {}; class Y : public X {}; class Z : public Y {};",
 ZIsDerivedFromX));
+  EXPECT_TRUE(
+  notMatches("class X {}; class Y : public X {}; class Z : public Y {};",
+ ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(
 matches("class X {};"
   "template class Y : public X {};"
   "class Z : public Y {};", ZIsDerivedFromX));
+  EXPECT_TRUE(notMatches("class X {};"
+ "template class Y : public X {};"
+ "class Z : public Y {};",
+ ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(matches("class X {}; template class Z : public X {};",
   ZIsDerivedFromX));
   EXPECT_TRUE(
@@ -411,6 +430,9 @@
 matches("class X {}; class Y : public X {}; "
   "typedef Y V; typedef V W; class Z : public W {};",
 ZIsDerivedFromX));
+  EXPECT_TRUE(notMatches("class X {}; class Y : public X {}; "
+ "typedef Y V; typedef V W; class Z : public W {};",
+ ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(
 matches("template class X {}; "
   "template class A { class Z : public X {}; };",
@@ -467,6 +489,14 @@
   "template<> struct X<0> : public A {};"
   "struct B : public X<42> {};",
 cxxRecordDecl(hasName("B"), isDerivedFrom(recordDecl(hasName("A"));
+  EXPECT_TRUE(notMatches(
+  "struct A {};"
+  "template struct X;"
+  "template struct X : public X {};"
+  "template<> struct X<0> : public A {};"
+  "struct B : public X<42> {};",
+  cxxRecordDecl(hasName("B"),
+isDirectlyDerivedFrom(recordDecl(hasName("A"));
 
   // FIXME: Once we have better matchers for template type matching,
   // get rid of the Variable(...) matching and match the right template
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -430,7 +430,8 @@
 
   bool classIsDerivedFrom(const CXXRecordDecl *Declaration,
   const Matcher ,
-  BoundNodesTreeBuilder *Builder) override;
+  BoundNodesTreeBuilder *Builder,
+  bool Directly) override;
 
   // Implements ASTMatchFinder::matchesChildOf.
   bool matchesChildOf(const ast_type_traits::DynTypedNode ,
@@ -817,7 +818,8 @@
 // derived from itself.
 bool MatchASTVisitor::classIsDerivedFrom(const CXXRecordDecl *Declaration,
  const Matcher ,
- BoundNodesTreeBuilder *Builder) {
+ BoundNodesTreeBuilder *Builder,
+ bool Directly) {
   if (!Declaration->hasDefinition())
 return false;
   for (const auto  : Declaration->bases()) {
@@ -842,7 +844,7 @@
   *Builder = std::move(Result);
   return true;
 }
-if (classIsDerivedFrom(ClassDecl, Base, Builder))
+if (!Directly && classIsDerivedFrom(ClassDecl, Base, Builder, Directly))
   return true;
   }
   return false;

[PATCH] D65092: [clang] Add isDirectlyDerivedFrom AST Matcher.

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev updated this revision to Diff 211148.
AntonBikineev added a comment.

Manuel, I left some cases which deviated from the isDerivedFrom matcher.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65092

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -331,6 +331,16 @@
   EXPECT_TRUE(notMatches("class Y;", IsDerivedFromX));
   EXPECT_TRUE(notMatches("", IsDerivedFromX));
 
+  DeclarationMatcher IsDirectlyDerivedFromX =
+  cxxRecordDecl(isDirectlyDerivedFrom("X"));
+
+  EXPECT_TRUE(
+  matches("class X {}; class Y : public X {};", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatches("class X {};", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatches("class X;", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatches("class Y;", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatches("", IsDirectlyDerivedFromX));
+
   DeclarationMatcher IsAX = cxxRecordDecl(isSameOrDerivedFrom("X"));
 
   EXPECT_TRUE(matches("class X {}; class Y : public X {};", IsAX));
@@ -341,13 +351,22 @@
 
   DeclarationMatcher ZIsDerivedFromX =
 cxxRecordDecl(hasName("Z"), isDerivedFrom("X"));
+  DeclarationMatcher ZIsDirectlyDerivedFromX =
+  cxxRecordDecl(hasName("Z"), isDirectlyDerivedFrom("X"));
   EXPECT_TRUE(
 matches("class X {}; class Y : public X {}; class Z : public Y {};",
 ZIsDerivedFromX));
+  EXPECT_TRUE(
+  notMatches("class X {}; class Y : public X {}; class Z : public Y {};",
+ ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(
 matches("class X {};"
   "template class Y : public X {};"
   "class Z : public Y {};", ZIsDerivedFromX));
+  EXPECT_TRUE(notMatches("class X {};"
+ "template class Y : public X {};"
+ "class Z : public Y {};",
+ ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(matches("class X {}; template class Z : public X {};",
   ZIsDerivedFromX));
   EXPECT_TRUE(
@@ -411,6 +430,9 @@
 matches("class X {}; class Y : public X {}; "
   "typedef Y V; typedef V W; class Z : public W {};",
 ZIsDerivedFromX));
+  EXPECT_TRUE(notMatches("class X {}; class Y : public X {}; "
+ "typedef Y V; typedef V W; class Z : public W {};",
+ ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(
 matches("template class X {}; "
   "template class A { class Z : public X {}; };",
@@ -467,6 +489,14 @@
   "template<> struct X<0> : public A {};"
   "struct B : public X<42> {};",
 cxxRecordDecl(hasName("B"), isDerivedFrom(recordDecl(hasName("A"));
+  EXPECT_TRUE(notMatches(
+  "struct A {};"
+  "template struct X;"
+  "template struct X : public X {};"
+  "template<> struct X<0> : public A {};"
+  "struct B : public X<42> {};",
+  cxxRecordDecl(hasName("B"),
+isDirectlyDerivedFrom(recordDecl(hasName("A"));
 
   // FIXME: Once we have better matchers for template type matching,
   // get rid of the Variable(...) matching and match the right template
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -430,7 +430,8 @@
 
   bool classIsDerivedFrom(const CXXRecordDecl *Declaration,
   const Matcher ,
-  BoundNodesTreeBuilder *Builder) override;
+  BoundNodesTreeBuilder *Builder,
+  bool Directly) override;
 
   // Implements ASTMatchFinder::matchesChildOf.
   bool matchesChildOf(const ast_type_traits::DynTypedNode ,
@@ -817,7 +818,8 @@
 // derived from itself.
 bool MatchASTVisitor::classIsDerivedFrom(const CXXRecordDecl *Declaration,
  const Matcher ,
- BoundNodesTreeBuilder *Builder) {
+ BoundNodesTreeBuilder *Builder,
+ bool Directly) {
   if (!Declaration->hasDefinition())
 return false;
   for (const auto  : Declaration->bases()) {
@@ -842,7 +844,7 @@
   *Builder = std::move(Result);
   return true;
 }
-if (classIsDerivedFrom(ClassDecl, Base, Builder))
+if (!Directly && classIsDerivedFrom(ClassDecl, Base, Builder, Directly))
   return true;
   }
   return false;
Index: 

[PATCH] D65092: [clang] Add isDirectlyDerivedFrom AST Matcher.

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev created this revision.
AntonBikineev added a reviewer: rsmith.
AntonBikineev added a project: clang.

This patch adds isDirectlyDerivedFrom AST-matcher which is similar to 
isDerivedFrom but only matches against direct base classes.


Repository:
  rC Clang

https://reviews.llvm.org/D65092

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -331,6 +331,16 @@
   EXPECT_TRUE(notMatches("class Y;", IsDerivedFromX));
   EXPECT_TRUE(notMatches("", IsDerivedFromX));
 
+  DeclarationMatcher IsDirectlyDerivedFromX =
+  cxxRecordDecl(isDirectlyDerivedFrom("X"));
+
+  EXPECT_TRUE(
+  matches("class X {}; class Y : public X {};", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatches("class X {};", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatches("class X;", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatches("class Y;", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatches("", IsDirectlyDerivedFromX));
+
   DeclarationMatcher IsAX = cxxRecordDecl(isSameOrDerivedFrom("X"));
 
   EXPECT_TRUE(matches("class X {}; class Y : public X {};", IsAX));
@@ -341,125 +351,240 @@
 
   DeclarationMatcher ZIsDerivedFromX =
 cxxRecordDecl(hasName("Z"), isDerivedFrom("X"));
+  DeclarationMatcher ZIsDirectlyDerivedFromX =
+  cxxRecordDecl(hasName("Z"), isDirectlyDerivedFrom("X"));
   EXPECT_TRUE(
 matches("class X {}; class Y : public X {}; class Z : public Y {};",
 ZIsDerivedFromX));
+  EXPECT_TRUE(
+  notMatches("class X {}; class Y : public X {}; class Z : public Y {};",
+ ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(
 matches("class X {};"
   "template class Y : public X {};"
   "class Z : public Y {};", ZIsDerivedFromX));
+  EXPECT_TRUE(notMatches("class X {};"
+ "template class Y : public X {};"
+ "class Z : public Y {};",
+ ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(matches("class X {}; template class Z : public X {};",
   ZIsDerivedFromX));
+  EXPECT_TRUE(matches("class X {}; template class Z : public X {};",
+  ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(
 matches("template class X {}; "
   "template class Z : public X {};",
 ZIsDerivedFromX));
+  EXPECT_TRUE(matches("template class X {}; "
+  "template class Z : public X {};",
+  ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(
 matches("template class X {}; "
   "template class Z : public X {};",
 ZIsDerivedFromX));
+  EXPECT_TRUE(matches("template class X {}; "
+  "template class Z : public X {};",
+  ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(
 notMatches("template class A { class Z : public X {}; };",
ZIsDerivedFromX));
+  EXPECT_TRUE(
+  notMatches("template class A { class Z : public X {}; };",
+ ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(
 matches("template class A { public: class Z : public X {}; }; "
   "class X{}; void y() { A::Z z; }", ZIsDerivedFromX));
+  EXPECT_TRUE(
+  matches("template class A { public: class Z : public X {}; }; "
+  "class X{}; void y() { A::Z z; }",
+  ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(
 matches("template  class X {}; "
   "template class A { class Z : public X {}; };",
 ZIsDerivedFromX));
+  EXPECT_TRUE(
+  matches("template  class X {}; "
+  "template class A { class Z : public X {}; };",
+  ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(
 notMatches("template class X> class A { "
  "  class Z : public X {}; };", ZIsDerivedFromX));
+  EXPECT_TRUE(notMatches("template class X> class A { "
+ "  class Z : public X {}; };",
+ ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(
 matches("template class X> class A { "
   "  public: class Z : public X {}; }; "
   "template class X {}; void y() { A::Z z; }",
 ZIsDerivedFromX));
+  EXPECT_TRUE(matches("template class X> class A { "
+  "  public: class Z : public X {}; }; "
+  "template class X {}; void y() { A::Z z; }",
+  ZIsDirectlyDerivedFromX));
   EXPECT_TRUE(
 notMatches("template class A { class Z : public X::D {}; };",
ZIsDerivedFromX));
+  EXPECT_TRUE(
+  notMatches("template class A { class Z : 

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for 'this' parameter

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

@rsmith, thanks. Le'ts see if there is a need for the command-line option down 
the road.


Repository:
  rL LLVM

https://reviews.llvm.org/D46441



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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for 'this' parameter

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

@rsmith do you have any comments or suggestions?


https://reviews.llvm.org/D46441



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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

[Anton Bikineev via Phabricator via cfe-commits]

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

I've submitted an issue to the Core about the case. Presumably, it will be 
included in the next revision (mailing deadline of which is tomorrow).




Comment at: lib/CodeGen/CGCall.cpp:1893
+
+IsCtor = isa(TargetDecl);
   }

rjmccall wrote:
> I feel like you should just use `TargetDecl && 
> isa(TargetDecl)` below; it's more obvious.
> 
> Is there not an analogous rule for destructors?
There appears to be no similar rule for destructors, maybe because at the point 
of calling a destructor the value of the object/subobjects is well-determined.


Repository:
  rC Clang

https://reviews.llvm.org/D46441



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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

@efriedma copy and move constructors are particular cases where the value of a 
constructed object may be accessed through a glvalue not obtained from 'this' 
pointer (but from the first arg of a ctor).


Repository:
  rC Clang

https://reviews.llvm.org/D46441



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

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev updated this revision to Diff 145332.
AntonBikineev added a comment.
Herald added subscribers: aheejin, sbc100.

I've moved setting noalias-attribute down to IR-function creation. This is 
needed in the context of emitting a constructor call when the definition of the 
constructor is not available (and clang emits an IR-constructor-declaration). 
@lebedev.ri @aaron.ballman I've also adjusted existing tests. There a quite a 
few of them, so it seems there is no reason of having a specific test case.


Repository:
  rC Clang

https://reviews.llvm.org/D46441

Files:
  lib/CodeGen/CodeGenModule.cpp
  test/CXX/except/except.spec/p14-ir.cpp
  test/CodeGenCXX/constructor-init.cpp
  test/CodeGenCXX/copy-constructor-synthesis-2.cpp
  test/CodeGenCXX/copy-constructor-synthesis.cpp
  test/CodeGenCXX/dllexport-members.cpp
  test/CodeGenCXX/dllimport-members.cpp
  test/CodeGenCXX/implicit-copy-constructor.cpp
  test/CodeGenCXX/pod-member-memcpys.cpp
  test/CodeGenCXX/wasm-args-returns.cpp
  test/CodeGenObjCXX/implicit-copy-constructor.mm

Index: test/CodeGenObjCXX/implicit-copy-constructor.mm
===
--- test/CodeGenObjCXX/implicit-copy-constructor.mm
+++ test/CodeGenObjCXX/implicit-copy-constructor.mm
@@ -41,7 +41,7 @@
   D d2(d);
 }
 
-// CHECK-LABEL: define linkonce_odr void @_ZN1DC1ERS_(%struct.D* %this, %struct.D* dereferenceable({{[0-9]+}})) unnamed_addr
+// CHECK-LABEL: define linkonce_odr void @_ZN1DC1ERS_(%struct.D* %this, %struct.D* noalias dereferenceable({{[0-9]+}})) unnamed_addr
 // CHECK: call void @_ZN1AC1Ev
 // CHECK: call void @_ZN1CC2ERS_1A
 // CHECK: call void @_ZN1AD1Ev
Index: test/CodeGenCXX/wasm-args-returns.cpp
===
--- test/CodeGenCXX/wasm-args-returns.cpp
+++ test/CodeGenCXX/wasm-args-returns.cpp
@@ -44,7 +44,7 @@
 test(copy_ctor);
 // CHECK: define void @_Z7forward9copy_ctor(%struct.copy_ctor* noalias sret %{{.*}}, %struct.copy_ctor* %{{.*}})
 //
-// CHECK: declare %struct.copy_ctor* @_ZN9copy_ctorC1ERKS_(%struct.copy_ctor* returned, %struct.copy_ctor* dereferenceable(8))
+// CHECK: declare %struct.copy_ctor* @_ZN9copy_ctorC1ERKS_(%struct.copy_ctor* returned, %struct.copy_ctor* noalias dereferenceable(8))
 //
 // CHECK: define void @_Z14test_copy_ctorv()
 // CHECK: %[[tmp:.*]] = alloca %struct.copy_ctor, align 8
@@ -62,7 +62,7 @@
 test(aligned_copy_ctor);
 // CHECK: define void @_Z7forward17aligned_copy_ctor(%struct.aligned_copy_ctor* noalias sret %{{.*}}, %struct.aligned_copy_ctor* %{{.*}})
 //
-// CHECK: declare %struct.aligned_copy_ctor* @_ZN17aligned_copy_ctorC1ERKS_(%struct.aligned_copy_ctor* returned, %struct.aligned_copy_ctor* dereferenceable(16))
+// CHECK: declare %struct.aligned_copy_ctor* @_ZN17aligned_copy_ctorC1ERKS_(%struct.aligned_copy_ctor* returned, %struct.aligned_copy_ctor* noalias dereferenceable(16))
 //
 // CHECK: define void @_Z22test_aligned_copy_ctorv()
 // CHECK: %[[tmp:.*]] = alloca %struct.aligned_copy_ctor, align 16
Index: test/CodeGenCXX/pod-member-memcpys.cpp
===
--- test/CodeGenCXX/pod-member-memcpys.cpp
+++ test/CodeGenCXX/pod-member-memcpys.cpp
@@ -181,51 +181,51 @@
 
 CALL_CC(PackedMembers)
 // PackedMembers copy-assignment:
-// CHECK-LABEL: define linkonce_odr void @_ZN13PackedMembersC2ERKS_(%struct.PackedMembers* %this, %struct.PackedMembers* dereferenceable({{[0-9]+}}))
+// CHECK-LABEL: define linkonce_odr void @_ZN13PackedMembersC2ERKS_(%struct.PackedMembers* %this, %struct.PackedMembers* noalias dereferenceable({{[0-9]+}}))
 // CHECK: call void @_ZN6NonPODC1ERKS_
 // CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}} align 1 {{.*}} align 1 {{.*}}i64 16, i1 {{.*}})
 // CHECK: ret void
 
 CALL_CC(BitfieldMember2)
 // BitfieldMember2 copy-constructor:
-// CHECK-2-LABEL: define linkonce_odr void @_ZN15BitfieldMember2C2ERKS_(%struct.BitfieldMember2* %this, %struct.BitfieldMember2* dereferenceable({{[0-9]+}}))
+// CHECK-2-LABEL: define linkonce_odr void @_ZN15BitfieldMember2C2ERKS_(%struct.BitfieldMember2* %this, %struct.BitfieldMember2* noalias dereferenceable({{[0-9]+}}))
 // CHECK-2: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}} align 4 {{.*}} align 4 {{.*}}i64 16, i1 false)
 // CHECK-2: call void @_ZN6NonPODC1ERKS_
 // CHECK-2: ret void
 
 CALL_CC(BitfieldMember3)
 // BitfieldMember3 copy-constructor:
-// CHECK-LABEL: define linkonce_odr void @_ZN15BitfieldMember3C2ERKS_(%struct.BitfieldMember3* %this, %struct.BitfieldMember3* dereferenceable({{[0-9]+}}))
+// CHECK-LABEL: define linkonce_odr void @_ZN15BitfieldMember3C2ERKS_(%struct.BitfieldMember3* %this, %struct.BitfieldMember3* noalias dereferenceable({{[0-9]+}}))
 // CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}} align 8 {{.*}} align 8 {{.*}}i64 8, i1 false)
 // CHECK: ret void
 
 CALL_CC(ReferenceMember)
 // ReferenceMember copy-constructor:
-// CHECK-LABEL: define linkonce_odr void 

[PATCH] D46441: [clang][CodeGenCXX] Noalias attr for copy/move constructor arguments

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev created this revision.
AntonBikineev added reviewers: rsmith, aaron.ballman.
AntonBikineev added a project: clang.

The patch addresses this bug .

According to the Standard (taken from the bug description):
[class.ctor] paragraph 14:

"During the construction of an object, if the value of the object or any of its 
subobjects is accessed through a glvalue that is not obtained, directly or 
indirectly, from the constructor’s this pointer, the value of the object or 
subobject thus obtained is unspecified."


Repository:
  rC Clang

https://reviews.llvm.org/D46441

Files:
  lib/CodeGen/CGCXX.cpp


Index: lib/CodeGen/CGCXX.cpp
===
--- lib/CodeGen/CGCXX.cpp
+++ lib/CodeGen/CGCXX.cpp
@@ -224,6 +224,11 @@
   } else {
 const auto *CD = cast(MD);
 GD = GlobalDecl(CD, toCXXCtorType(Type));
+
+// As long as `this` argument always comes first
+// we hardcode the position of the `src` argument
+if (CD->isCopyOrMoveConstructor())
+  Fn->addParamAttr(1, llvm::Attribute::NoAlias);
   }
 
   setFunctionLinkage(GD, Fn);


Index: lib/CodeGen/CGCXX.cpp
===
--- lib/CodeGen/CGCXX.cpp
+++ lib/CodeGen/CGCXX.cpp
@@ -224,6 +224,11 @@
   } else {
 const auto *CD = cast(MD);
 GD = GlobalDecl(CD, toCXXCtorType(Type));
+
+// As long as `this` argument always comes first
+// we hardcode the position of the `src` argument
+if (CD->isCopyOrMoveConstructor())
+  Fn->addParamAttr(1, llvm::Attribute::NoAlias);
   }
 
   setFunctionLinkage(GD, Fn);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43153: [clang] Implement P0692 "Access Checking on Specializations"

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

@rsmith Thanks for pointing out this example. Now I see that I misunderstood 
the wording.
Another question is that in the provided example you say that the following line

  template A::B x; // ok!

should suppress the error of accessing private A::B. But in the wording it's 
said that

> The usual access checking rules do not apply to non-dependent names used to 
> specify **template arguments of the simple-template-id** of the partial 
> specialization.

So it gave me the impression that only names used as template arguments should 
be exempted from checks. But here `A::B` is a non-dependent name that's part of 
the declaration-specifier. So is my impression wrong and names in 
declaration-specifiers/declarators of partial-specialiations of 
variable-templates should be exempted from checks as well?

Thanks.


Repository:
  rC Clang

https://reviews.llvm.org/D43153



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

[PATCH] D43153: [clang] Implement P0692 "Access Checking on Specializations"

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev created this revision.
AntonBikineev added reviewers: rsmith, lebedev.ri.

This is an attempt to implement P0692 

Please note a couple of things:

1. given that clang already suppresses access checks on explicit 
specializations of classes as an extension, I decided to not make the feature 
C++17-specific and backport it to previous standards. Not sure that this is 
right though;
2. I'm also not sure that ParseOptionalCXXScopeSpecifer in a declarator is the 
correct place for suppressing checks.


Repository:
  rC Clang

https://reviews.llvm.org/D43153

Files:
  include/clang/Parse/RAIIObjectsForParser.h
  include/clang/Sema/DeclSpec.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/ParseTemplate.cpp
  test/CXX/drs/dr1xx.cpp
  test/SemaCXX/access.cpp

Index: test/SemaCXX/access.cpp
===
--- test/SemaCXX/access.cpp
+++ test/SemaCXX/access.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify %s
 
 class C {
 struct S; // expected-note {{previously declared 'private' here}}
@@ -169,3 +169,103 @@
   }
   void bar() { foo(); }
 }
+
+namespace P0692 {
+  namespace class_ {
+template 
+class trait_type {};
+   
+template 
+class trait_nontype;
+
+template  class>
+class trait_template;
+
+class A {
+  class B1 {}; // expected-note 2{{here}}
+
+  template 
+  class B2 {}; // expected-note 2{{here}}
+
+  class B3 {};
+   
+  static const int I = 0; // expected-note 4{{here}}
+};
+   
+template 
+class trait_type {
+  void use_private(int = A::I) { // expected-error {{private}}
+A::B2 error; // expected-error {{private}}
+  }
+};
+
+template  class T>
+class trait_type {
+  static const int I = A::I; // expected-error {{private}}
+};
+   
+template <>
+class trait_type
+  : A::B1 { // expected-error {{private}}
+  void use_private(int = A::I) { // expected-error {{private}}
+A::B1 error; // expected-error {{private}}
+  }
+};
+
+template class trait_type;
+
+template <>
+class trait_nontype {
+  static const int I = A::I; // expected-error {{private}}
+};
+
+template <>
+class trait_template {
+  A::B2 error; // expected-error {{private}}
+};
+  }
+
+  namespace function {
+class A {
+  class B1 {}; // expected-note 1{{here}}
+
+  class B2 {};
+   
+  using Int = int; // expected-note 1{{here}}
+};
+   
+template 
+void foo1() {}
+
+template <> void foo1() {
+A::B1 error; // expected-error {{private}}
+}
+
+template void foo1();
+
+template 
+void foo2(A::Int) {} // expected-error {{private}}
+  }
+
+  namespace variable {
+class A {
+  class B1 {};
+
+  template 
+  class B2 {};
+   
+  static const int I = 0; // expected-note 2{{here}}
+};
+
+template 
+int var = 0;
+
+template <>
+int var =
+A::I; // expected-error {{private}}
+
+template 
+int var =
+A::I; // expected-error {{private}}
+  }
+}
Index: test/CXX/drs/dr1xx.cpp
===
--- test/CXX/drs/dr1xx.cpp
+++ test/CXX/drs/dr1xx.cpp
@@ -917,12 +917,12 @@
   template  void C::g() {}
 
   class A {
-class B {}; // expected-note {{here}}
+class B {};
 void f();
   };
 
   template void C::f();
-  template <> void C::g(); // expected-error {{private}}
+  template <> void C::g();
 
   void A::f() {
 C cb;
Index: lib/Parse/ParseTemplate.cpp
===
--- lib/Parse/ParseTemplate.cpp
+++ lib/Parse/ParseTemplate.cpp
@@ -235,7 +235,10 @@
 
   // Parse the declarator.
   ParsingDeclarator DeclaratorInfo(*this, DS, (DeclaratorContext)Context);
+  DeclaratorInfo.setTemplateDeclOrSpec(TemplateInfo.Kind !=
+   ParsedTemplateInfo::NonTemplate);
   ParseDeclarator(DeclaratorInfo);
+
   // Error parsing the declarator?
   if (!DeclaratorInfo.hasName()) {
 // If so, skip until the semi-colon or a }.
Index: lib/Parse/ParseDeclCXX.cpp
===
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -1375,15 +1375,22 @@
   //   The usual access checking rules do not apply to names used to specify
   //   explicit instantiations.
   //
-  // As an extension we do not perform access checking on the names used to
-  // specify explicit specializations either. This is important to allow
-  // specializing traits classes for private types.
+  // C++2a [temp.spec] 17.8/6:
+  //   The usual access checking rules do not apply to names in a declaration
+  //   of an explicit instantiation or explicit specialization, 

[PATCH] D35661: [clang] Implement P0704: "Fixing const-qualified pointers to members"

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

ping..


https://reviews.llvm.org/D35661



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

[PATCH] D35661: [clang] Implement P0704: "Fixing const-qualified pointers to members"

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev created this revision.

https://reviews.llvm.org/D35661

Files:
  lib/Sema/SemaExprCXX.cpp
  test/CXX/expr/expr.mptr.oper/p7-1z.cpp


Index: test/CXX/expr/expr.mptr.oper/p7-1z.cpp
===
--- test/CXX/expr/expr.mptr.oper/p7-1z.cpp
+++ test/CXX/expr/expr.mptr.oper/p7-1z.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -std=c++1z -fsyntax-only -verify %s
+
+struct X { };
+
+template T& lvalue();
+template T&& xvalue();
+template T prvalue();
+
+// In a .* expression whose object expression is an rvalue, the
+// program is ill-formed if the second operand is a pointer to member
+// function with (non-const, C++17) ref-qualifier &.
+// In a ->* expression or in a .* expression whose object
+// expression is an lvalue, the program is ill-formed if the second
+// operand is a pointer to member function with ref-qualifier &&.
+void test(X *xp, int (X::*pmf)(int), int (X::*l_pmf)(int) &, 
+  int (X::*r_pmf)(int) &&, int (X::*cl_pmf)(int) const &,
+  int (X::*cr_pmf)(int) const &&) {
+  // No ref-qualifier.
+  (lvalue().*pmf)(17);
+  (xvalue().*pmf)(17);
+  (prvalue().*pmf)(17);
+  (xp->*pmf)(17);
+
+  // Lvalue ref-qualifier.
+  (lvalue().*l_pmf)(17);
+  (xvalue().*l_pmf)(17); // expected-error-re{{pointer-to-member function 
type 'int (X::*)(int){{( __attribute__\(\(thiscall\)\))?}} &' can only be 
called on an lvalue}}
+  (prvalue().*l_pmf)(17); // expected-error-re{{pointer-to-member function 
type 'int (X::*)(int){{( __attribute__\(\(thiscall\)\))?}} &' can only be 
called on an lvalue}}
+  (xp->*l_pmf)(17);
+
+  // Rvalue ref-qualifier.
+  (lvalue().*r_pmf)(17); // expected-error-re{{pointer-to-member function 
type 'int (X::*)(int){{( __attribute__\(\(thiscall\)\))?}} &&' can only be 
called on an rvalue}}
+  (xvalue().*r_pmf)(17);
+  (prvalue().*r_pmf)(17);
+  (xp->*r_pmf)(17);  // expected-error-re{{pointer-to-member function type 
'int (X::*)(int){{( __attribute__\(\(thiscall\)\))?}} &&' can only be called on 
an rvalue}}
+
+  // Lvalue const ref-qualifier.
+  (lvalue().*cl_pmf)(17);
+  (xvalue().*cl_pmf)(17);
+  (prvalue().*cl_pmf)(17);
+  (xp->*cl_pmf)(17);
+
+  // Rvalue const ref-qualifier.
+  (lvalue().*cr_pmf)(17); // expected-error-re{{pointer-to-member function 
type 'int (X::*)(int){{( __attribute__\(\(thiscall\)\))?}} const &&' can only 
be called on an rvalue}}
+  (xvalue().*cr_pmf)(17);
+  (prvalue().*cr_pmf)(17);
+  (xp->*cr_pmf)(17);  // expected-error-re{{pointer-to-member function type 
'int (X::*)(int){{( __attribute__\(\(thiscall\)\))?}} const &&' can only be 
called on an rvalue}}
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -5178,9 +5178,11 @@
   break;
 
 case RQ_LValue:
-  if (!isIndirect && !LHS.get()->Classify(Context).isLValue())
-Diag(Loc, diag::err_pointer_to_member_oper_value_classify)
-  << RHSType << 1 << LHS.get()->getSourceRange();
+  if (!isIndirect && !LHS.get()->Classify(Context).isLValue()) {
+if (!getLangOpts().CPlusPlus1z || !Proto->isConst())
+  Diag(Loc, diag::err_pointer_to_member_oper_value_classify)
+<< RHSType << 1 << LHS.get()->getSourceRange();
+  }
   break;
 
 case RQ_RValue:


Index: test/CXX/expr/expr.mptr.oper/p7-1z.cpp
===
--- test/CXX/expr/expr.mptr.oper/p7-1z.cpp
+++ test/CXX/expr/expr.mptr.oper/p7-1z.cpp
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -std=c++1z -fsyntax-only -verify %s
+
+struct X { };
+
+template T& lvalue();
+template T&& xvalue();
+template T prvalue();
+
+// In a .* expression whose object expression is an rvalue, the
+// program is ill-formed if the second operand is a pointer to member
+// function with (non-const, C++17) ref-qualifier &.
+// In a ->* expression or in a .* expression whose object
+// expression is an lvalue, the program is ill-formed if the second
+// operand is a pointer to member function with ref-qualifier &&.
+void test(X *xp, int (X::*pmf)(int), int (X::*l_pmf)(int) &, 
+  int (X::*r_pmf)(int) &&, int (X::*cl_pmf)(int) const &,
+  int (X::*cr_pmf)(int) const &&) {
+  // No ref-qualifier.
+  (lvalue().*pmf)(17);
+  (xvalue().*pmf)(17);
+  (prvalue().*pmf)(17);
+  (xp->*pmf)(17);
+
+  // Lvalue ref-qualifier.
+  (lvalue().*l_pmf)(17);
+  (xvalue().*l_pmf)(17); // expected-error-re{{pointer-to-member function type 'int (X::*)(int){{( __attribute__\(\(thiscall\)\))?}} &' can only be called on an lvalue}}
+  (prvalue().*l_pmf)(17); // expected-error-re{{pointer-to-member function type 'int (X::*)(int){{( __attribute__\(\(thiscall\)\))?}} &' can only be called on an lvalue}}
+  (xp->*l_pmf)(17);
+
+  // Rvalue ref-qualifier.
+  (lvalue().*r_pmf)(17); // expected-error-re{{pointer-to-member function type 'int (X::*)(int){{( __attribute__\(\(thiscall\)\))?}} &&' can only be 

[PATCH] D26830: [libcxx] Add string_view literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

In https://reviews.llvm.org/D26830#711870, @EricWF wrote:

> @AntonBikineev when will you be able to make he requested changes? I would 
> like to land this ASAP.


Eric, have you looked at commit 7d32d2f5a1f39d4cae9469645faa74b647698001? This 
functionality has already been implemented by Marshall, we should abandon this 
patch.


https://reviews.llvm.org/D26830



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

[PATCH] D26830: [libcxx] Add string_view literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

In https://reviews.llvm.org/D26830#711870, @EricWF wrote:

> @AntonBikineev when will you be able to make he requested changes? I would 
> like to land this ASAP.


Will do that today


https://reviews.llvm.org/D26830



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

[PATCH] D26830: [libcxx] Add string_view literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

In https://reviews.llvm.org/D26830#619340, @EricWF wrote:

> Please ping this once the Clang changes have been accepted. I'm just waiting 
> on those to give this the final OK.


I don't have commit privileges to Clang either, so that patch is stuck. Could 
you commit there please?


https://reviews.llvm.org/D26830



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

[PATCH] D27199: [libcxx] Make std::ignore constexpr

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

In https://reviews.llvm.org/D27199#619278, @EricWF wrote:

> @AntonBikineev Do you need somebody to commit this for you?


oh, yeah, it would be great. I don't have commit privileges.


https://reviews.llvm.org/D27199



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

[PATCH] D26829: [clang] Allow lexer to handle string_view literals

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a comment.

@rsmith Richard, any plans to merge this or is there anything left?


https://reviews.llvm.org/D26829



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

[PATCH] D27199: [libcxx] Make std::ignore constexpr

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev created this revision.
AntonBikineev added reviewers: mclow.lists, EricWF.
AntonBikineev added a subscriber: cfe-commits.

This addresses DR 2773 .

  // 20.5.2.4, tuple creation functions:
  constexpr unspecified ignore;
  }


https://reviews.llvm.org/D27199

Files:
  include/tuple
  test/std/utilities/tuple/tuple.tuple/tuple.creation/tie.pass.cpp
  www/cxx1z_status.html


Index: www/cxx1z_status.html
===
--- www/cxx1z_status.html
+++ www/cxx1z_status.html
@@ -407,7 +407,7 @@
http://wg21.link/LWG2767;>2767not_fn 
call_wrapper can form invalid typesIssaquahComplete
http://wg21.link/LWG2769;>2769Redundant 
const in the return type of any_cast(const 
any&)IssaquahComplete
http://wg21.link/LWG2771;>2771Broken 
Effects of some basic_string::compare functions in terms of 
basic_string_viewIssaquahComplete
-   http://wg21.link/LWG2773;>2773Making 
std::ignore constexprIssaquah
+   http://wg21.link/LWG2773;>2773Making 
std::ignore constexprIssaquahComplete
http://wg21.link/LWG2777;>2777basic_string_view::copy should 
use char_traits::copyIssaquahComplete
http://wg21.link/LWG2778;>2778basic_string_view is missing 
constexprIssaquah
 
Index: test/std/utilities/tuple/tuple.tuple/tuple.creation/tie.pass.cpp
===
--- test/std/utilities/tuple/tuple.tuple/tuple.creation/tie.pass.cpp
+++ test/std/utilities/tuple/tuple.tuple/tuple.creation/tie.pass.cpp
@@ -22,6 +22,15 @@
 
 #include "test_macros.h"
 
+#if TEST_STD_VER > 14
+template 
+constexpr auto check_ce_ignore(T ignore)
+{
+ignore = 4;
+return ignore;
+}
+#endif
+
 int main()
 {
 {
@@ -40,4 +49,9 @@
 static_assert ( std::get<1>(t) == 1.1, "" );
 }
 #endif
+#if TEST_STD_VER > 14
+{
+constexpr auto t = check_ce_ignore(std::ignore);
+}
+#endif
 }
Index: include/tuple
===
--- include/tuple
+++ include/tuple
@@ -70,7 +70,7 @@
 void swap(tuple&) noexcept(AND(swap(declval(), declval())...));
 };
 
-const unspecified ignore;
+constexpr unspecified ignore;
 
 template  tuple  make_tuple(T&&...); // constexpr in C++14
 template  tuple forward_as_tuple(T&&...) noexcept; // 
constexpr in C++14
@@ -1047,11 +1047,11 @@
 struct __ignore_t
 {
 template 
-_LIBCPP_INLINE_VISIBILITY
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
 const __ignore_t& operator=(_Tp&&) const {return *this;}
 };
 
-namespace { const __ignore_t ignore = __ignore_t(); }
+namespace { _LIBCPP_CONSTEXPR_AFTER_CXX14 const __ignore_t 
ignore = __ignore_t(); }
 
 template 
 struct __make_tuple_return_impl


Index: www/cxx1z_status.html
===
--- www/cxx1z_status.html
+++ www/cxx1z_status.html
@@ -407,7 +407,7 @@
 	http://wg21.link/LWG2767;>2767not_fn call_wrapper can form invalid typesIssaquahComplete
 	http://wg21.link/LWG2769;>2769Redundant const in the return type of any_cast(const any&)IssaquahComplete
 	http://wg21.link/LWG2771;>2771Broken Effects of some basic_string::compare functions in terms of basic_string_viewIssaquahComplete
-	http://wg21.link/LWG2773;>2773Making std::ignore constexprIssaquah
+	http://wg21.link/LWG2773;>2773Making std::ignore constexprIssaquahComplete
 	http://wg21.link/LWG2777;>2777basic_string_view::copy should use char_traits::copyIssaquahComplete
 	http://wg21.link/LWG2778;>2778basic_string_view is missing constexprIssaquah
 
Index: test/std/utilities/tuple/tuple.tuple/tuple.creation/tie.pass.cpp
===
--- test/std/utilities/tuple/tuple.tuple/tuple.creation/tie.pass.cpp
+++ test/std/utilities/tuple/tuple.tuple/tuple.creation/tie.pass.cpp
@@ -22,6 +22,15 @@
 
 #include "test_macros.h"
 
+#if TEST_STD_VER > 14
+template 
+constexpr auto check_ce_ignore(T ignore)
+{
+ignore = 4;
+return ignore;
+}
+#endif
+
 int main()
 {
 {
@@ -40,4 +49,9 @@
 static_assert ( std::get<1>(t) == 1.1, "" );
 }
 #endif
+#if TEST_STD_VER > 14
+{
+constexpr auto t = check_ce_ignore(std::ignore);
+}
+#endif
 }
Index: include/tuple
===
--- include/tuple
+++ include/tuple
@@ -70,7 +70,7 @@
 void swap(tuple&) noexcept(AND(swap(declval(), declval())...));
 };
 
-const unspecified ignore;
+constexpr unspecified ignore;
 
 template  tuple  make_tuple(T&&...); // constexpr in C++14
 template  tuple forward_as_tuple(T&&...) noexcept; // constexpr in C++14
@@ -1047,11 +1047,11 @@
 struct __ignore_t
 {
 template 
-_LIBCPP_INLINE_VISIBILITY
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
 const __ignore_t& operator=(_Tp&&) const {return *this;}
 };
 

[PATCH] D27162: Support relaxed constexpr on chrono::duration operations

[Anton Bikineev via Phabricator via cfe-commits]

AntonBikineev added a subscriber: cfe-commits.
AntonBikineev updated this revision to Diff 79491.

https://reviews.llvm.org/D27162

Files:
  include/chrono
  test/std/utilities/time/time.duration/time.duration.arithmetic/op_++.pass.cpp
  
test/std/utilities/time/time.duration/time.duration.arithmetic/op_++int.pass.cpp
  test/std/utilities/time/time.duration/time.duration.arithmetic/op_+=.pass.cpp
  test/std/utilities/time/time.duration/time.duration.arithmetic/op_--.pass.cpp
  
test/std/utilities/time/time.duration/time.duration.arithmetic/op_--int.pass.cpp
  test/std/utilities/time/time.duration/time.duration.arithmetic/op_-=.pass.cpp
  
test/std/utilities/time/time.duration/time.duration.arithmetic/op_divide=.pass.cpp
  
test/std/utilities/time/time.duration/time.duration.arithmetic/op_mod=duration.pass.cpp
  
test/std/utilities/time/time.duration/time.duration.arithmetic/op_mod=rep.pass.cpp
  
test/std/utilities/time/time.duration/time.duration.arithmetic/op_times=.pass.cpp
  www/cxx1z_status.html

Index: www/cxx1z_status.html
===
--- www/cxx1z_status.html
+++ www/cxx1z_status.html
@@ -132,7 +132,7 @@
 	http://wg21.link/P0502R0;>P0502R0LWGThrowing out of a parallel algorithm terminates - but how?Issaquah
 	http://wg21.link/P0503R0;>P0503R0LWGCorrecting library usage of "literal type"Issaquah
 	http://wg21.link/P0504R0;>P0504R0LWGRevisiting in-place tag types for any/optional/variantIssaquah
-	http://wg21.link/P0505R0;>P0505R0LWGWording for GB 50 - constexpr for chronoIssaquah
+	http://wg21.link/P0505R0;>P0505R0LWGWording for GB 50 - constexpr for chronoIssaquahComplete4.0
 	http://wg21.link/P0508R0;>P0508R0LWGWording for GB 58 - structured bindings for node_handlesIssaquah
 	http://wg21.link/P0509R1;>P0509R1LWGUpdating “Restrictions on exception handling”Issaquah
 	http://wg21.link/P0510R0;>P0510R0LWGDisallowing references, incomplete types, arrays, and empty variantsIssaquah
Index: test/std/utilities/time/time.duration/time.duration.arithmetic/op_times=.pass.cpp
===
--- test/std/utilities/time/time.duration/time.duration.arithmetic/op_times=.pass.cpp
+++ test/std/utilities/time/time.duration/time.duration.arithmetic/op_times=.pass.cpp
@@ -11,14 +11,33 @@
 
 // duration
 
-// duration& operator*=(const rep& rhs);
+// constexpr duration& operator*=(const rep& rhs);
 
 #include 
 #include 
 
+#include "test_macros.h"
+
+#if TEST_STD_VER > 14
+constexpr std::chrono::hours test_ce(int n, int k) {
+std::chrono::hours h(n);
+h *= k;
+return h;
+}
+#endif
+
 int main()
 {
 std::chrono::nanoseconds ns(3);
 ns *= 5;
 assert(ns.count() == 15);
+
+#if TEST_STD_VER > 14
+{
+static_assert ( test_ce (7, 2) == std::chrono::hours(14), "" );
+static_assert ( test_ce (9, 5) == std::chrono::hours(45), "" );
+static_assert ( test_ce (5, 0) == std::chrono::hours(0), "" );
+static_assert ( test_ce (0, 0) == std::chrono::hours(0), "" );
+}
+#endif
 }
Index: test/std/utilities/time/time.duration/time.duration.arithmetic/op_mod=rep.pass.cpp
===
--- test/std/utilities/time/time.duration/time.duration.arithmetic/op_mod=rep.pass.cpp
+++ test/std/utilities/time/time.duration/time.duration.arithmetic/op_mod=rep.pass.cpp
@@ -11,14 +11,32 @@
 
 // duration
 
-// duration& operator%=(const rep& rhs)
+// constexpr duration& operator%=(const rep& rhs)
 
 #include 
 #include 
 
+#include "test_macros.h"
+
+#if TEST_STD_VER > 14
+constexpr std::chrono::hours test_ce(int n, int k) {
+std::chrono::hours h(n);
+h %= k;
+return h;
+}
+#endif
+
 int main()
 {
 std::chrono::microseconds us(11);
 us %= 3;
 assert(us.count() == 2);
+
+#if TEST_STD_VER > 14
+{
+static_assert ( test_ce (7, 2) == std::chrono::hours(1), "" );
+static_assert ( test_ce (9, 5) == std::chrono::hours(4), "" );
+static_assert ( test_ce (0, 4) == std::chrono::hours(0), "" );
+}
+#endif
 }
Index: test/std/utilities/time/time.duration/time.duration.arithmetic/op_mod=duration.pass.cpp
===
--- test/std/utilities/time/time.duration/time.duration.arithmetic/op_mod=duration.pass.cpp
+++ test/std/utilities/time/time.duration/time.duration.arithmetic/op_mod=duration.pass.cpp
@@ -11,17 +11,36 @@
 
 // duration
 
-// duration& operator%=(const duration& rhs)
+// constexpr duration& operator%=(const duration& rhs)
 
 #include 
 #include 
 
+#include "test_macros.h"
+
+#if TEST_STD_VER > 14
+constexpr std::chrono::hours test_ce(int n, int k) {
+typedef std::chrono::hours H;
+H h(n);
+h %= H(k);
+return h;
+}
+#endif
+
 int main()
 {
 std::chrono::microseconds us(11);
 std::chrono::microseconds us2(3);
 us %= us2;
 assert(us.count() == 2);
 us %= std::chrono::milliseconds(3);
 assert(us.count() == 2);
+