[PATCH] D133066: fix a typo in comment of AddConversionCandidate

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou created this revision.
zhouyizhou added reviewers: MikeStump, riccibruno, rsmith.
Herald added a project: All.
zhouyizhou requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Hi 
I think the comment above the condition expression ICS.Standard.First == 
ICK_Lvalue_To_Rvalue should be:

//In the second case, if the reference is an rvalue reference and   
  
//the first standard conversion sequence of the user-defined
  
//conversion sequence includes an lvalue-to-rvalue conversion, the  
  
 //program is ill-formed.

Many thanks
Zhouyi Zhou 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133066

Files:
  clang/lib/Sema/SemaOverload.cpp


Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -7495,7 +7495,7 @@
 
 // C++0x [dcl.init.ref]p5:
 //In the second case, if the reference is an rvalue reference and
-//the second standard conversion sequence of the user-defined
+//the first standard conversion sequence of the user-defined
 //conversion sequence includes an lvalue-to-rvalue conversion, the
 //program is ill-formed.
 if (ToType->isRValueReferenceType() &&


Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -7495,7 +7495,7 @@
 
 // C++0x [dcl.init.ref]p5:
 //In the second case, if the reference is an rvalue reference and
-//the second standard conversion sequence of the user-defined
+//the first standard conversion sequence of the user-defined
 //conversion sequence includes an lvalue-to-rvalue conversion, the
 //program is ill-formed.
 if (ToType->isRValueReferenceType() &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133066: fix a typo in comment of AddConversionCandidate

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D133066#3764384 , @aaron.ballman 
wrote:

> The existing comment is correct according to my copy of the C++11 standard, 
> but the standard has changed since C++11 and those words no longer appear in 
> http://eel.is/c++draft/dcl.init.ref#5. Some more investigation is needed, but 
> I suspect the correct action to take here is to update the comment based on 
> the current standards wording or to see if there's a bug because the code 
> does not match the comment.

thank Aaron for review my patch, 
I am a passionate beginner, 
this is a very good learning experience for me ;-)  I am looking forward to 
seeing the final change.

Thanks again


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133066

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

[PATCH] D133066: fix a typo in comment of AddConversionCandidate

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D133066#3767146 , @aaron.ballman 
wrote:

> In D133066#3765503 , @zhouyizhou 
> wrote:
>
>> In D133066#3764384 , 
>> @aaron.ballman wrote:
>>
>>> The existing comment is correct according to my copy of the C++11 standard, 
>>> but the standard has changed since C++11 and those words no longer appear 
>>> in http://eel.is/c++draft/dcl.init.ref#5. Some more investigation is 
>>> needed, but I suspect the correct action to take here is to update the 
>>> comment based on the current standards wording or to see if there's a bug 
>>> because the code does not match the comment.
>>
>> thank Aaron for review my patch, 
>> I am a passionate beginner,
>
> Welcome to the community, we're glad to have you here!

Thank Aaron for your encouragement and guidance! Hope I can be some beneficial 
to the community.

>> this is a very good learning experience for me ;-)  I am looking forward to 
>> seeing the final change.
>
> Happy to help, but to be clear on the next steps: are you planning to do the 
> investigation work, or were you hoping someone else would do it?

As an amateur, this is a difficult job for me, but I can't help taking a try.

Following your guidance, I found the original C++11 document on the internet: 
https://raw.githubusercontent.com/yjlintw/book-Coding-Developer/master/%E6%A0%87%E5%87%86%E6%96%87%E6%A1%A3/ISO%20IEC%2014882%202011%20(C%2B%2B11).pdf
 
(the non ASCII code in URL means this document is maintained by a Chinese like 
me). 
And those words are there!

I am eager to do some investigation work in the elementary stage, but I believe 
the final work should be done by someone else.

Thanks again for your enthusiasm and your patience!

Cheers
Zhouyi


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133066

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

[PATCH] D133066: fix a typo in comment of AddConversionCandidate

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D133066#3780900 , @aaron.ballman 
wrote:

> In D133066#3768091 , @zhouyizhou 
> wrote:
>
>> In D133066#3767146 , 
>> @aaron.ballman wrote:
>>
>>> In D133066#3765503 , @zhouyizhou 
>>> wrote:
>>>
 In D133066#3764384 , 
 @aaron.ballman wrote:

> The existing comment is correct according to my copy of the C++11 
> standard, but the standard has changed since C++11 and those words no 
> longer appear in http://eel.is/c++draft/dcl.init.ref#5. Some more 
> investigation is needed, but I suspect the correct action to take here is 
> to update the comment based on the current standards wording or to see if 
> there's a bug because the code does not match the comment.

 thank Aaron for review my patch, 
 I am a passionate beginner,
>>>
>>> Welcome to the community, we're glad to have you here!
>>
>> Thank Aaron for your encouragement and guidance! Hope I can be some 
>> beneficial to the community.
>>
 this is a very good learning experience for me ;-)  I am looking forward 
 to seeing the final change.
>>>
>>> Happy to help, but to be clear on the next steps: are you planning to do 
>>> the investigation work, or were you hoping someone else would do it?
>>
>> As an amateur, this is a difficult job for me, but I can't help taking a try.
>
> For what it's worth, it's not easy for experts either. :-D

Thanks again for your patience, and sorry for the late feedback, I think I will 
spend 3 more weeks to work out and submit my tentative work.

>> Following your guidance, I found the original C++11 document on the 
>> internet: 
>> https://raw.githubusercontent.com/yjlintw/book-Coding-Developer/master/%E6%A0%87%E5%87%86%E6%96%87%E6%A1%A3/ISO%20IEC%2014882%202011%20(C%2B%2B11).pdf
>>  
>> (the non ASCII code in URL means this document is maintained by a Chinese 
>> like me). 
>> And those words are there!
>
> Yup, that matches my copy of C++11; btw, you can find a late-stage working 
> draft on the committee website at: 
> https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2011/n3242.pdf

Yes, and I also find those words in above document and the example that follows 
is almost the same except a minor difference.

>> I am eager to do some investigation work in the elementary stage, but I 
>> believe the final work should be done by someone else.
>>
>> Thanks again for your enthusiasm and your patience!
>
> My pleasure!
>
> For the moment, I don't think any changes are needed here regarding the 
> comment.

Thanks again for your guidance. OK. and I use "git log -p  cba72b1f620fd" the 
study the original submit which add those words.
This is a fruitful and happy journey for me ;-)

Cheers
Zhouyi


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133066

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

[PATCH] D133066: fix a typo in comment of AddConversionCandidate

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

my current progress - succesfully build  commit cba72b1f620fd on debian6:

1. git checkout cba72b1f620fd
2. download debian-6.0.10-amd64-DVD-1.iso and use it to create a debian6 
virtual machine
3. debian6/llvm-project>cp -fr clang llvm/tools
4. modifiy llvm/tools/CMakefileLists.txt:

diff --git a/llvm/tools/CMakeLists.txt b/llvm/tools/CMakeLists.txt
index 2f37911..71cdbcc 100644

- a/llvm/tools/CMakeLists.txt

+++ b/llvm/tools/CMakeLists.txt
@@ -46,8 +46,6 @@ add_subdirectory(llvm-stub)
 add_subdirectory(edis)
 add_subdirectory(llvmc)

-if( EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/clang/CMakeLists.txt )

- add_subdirectory( ${CMAKE_CURRENT_SOURCE_DIR}/clang )

-endif( EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/clang/CMakeLists.txt )
+add_subdirectory(clang)

5. debian6/llvm-project>mkdir build
6. debian6/llvm-project>cd build
7. cmake ../llvm -DLLVM_TARGETS_TO_BUILD="X86"  -DCMAKE_BUILD_TYPE="Debug" 
-DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++
8. make clang -j 8

I am now begin to debugging newly build clang-2.9

Thanks again for your guidance and encouragement ;-)
cheers
Zhouyi


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133066

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

[PATCH] D121498: let EST_Uninstantiated in FunctionProtoType::canThrow return CT_Dependent

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou created this revision.
zhouyizhou added reviewers: rsmith, erichkeane, CornedBee.
Herald added a project: All.
zhouyizhou requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When compile following code without -std=c++17, clang will abort by 
llvm_unreachable:

- code begin ---

template
void foo() noexcept(T::value);

struct S {

  static const bool value;

};

const bool S::value = 0;

template
constexpr bool is_noexcept_function(void(Args...) noexcept(is_noexcept)) {

  return is_noexcept;

}

void fn()
{

  is_noexcept_function(foo);

}

- code end ---

So, my solution is to let EST_Uninstantiated in FunctionProtoType::canThrow 
return CT_Dependent

Thanks
Zhouyi Zhou 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121498

Files:
  clang/lib/AST/Type.cpp


Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -3320,8 +3320,9 @@
   switch (getExceptionSpecType()) {
   case EST_Unparsed:
   case EST_Unevaluated:
-  case EST_Uninstantiated:
 llvm_unreachable("should not call this with unresolved exception specs");
+  case EST_Uninstantiated:
+return CT_Dependent;
 
   case EST_DynamicNone:
   case EST_BasicNoexcept:


Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -3320,8 +3320,9 @@
   switch (getExceptionSpecType()) {
   case EST_Unparsed:
   case EST_Unevaluated:
-  case EST_Uninstantiated:
 llvm_unreachable("should not call this with unresolved exception specs");
+  case EST_Uninstantiated:
+return CT_Dependent;
 
   case EST_DynamicNone:
   case EST_BasicNoexcept:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121498: let EST_Uninstantiated in FunctionProtoType::canThrow return CT_Dependent

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D121498#3379222 , @erichkeane 
wrote:

> This seems reasonable to me, but can you please add a lit test and fix the 
> format of the code in your commit message?

Thank you for reviewing the patch for me ;-)  I am a beginner, please give me a 
couple of days to add the lit test and fix the format of the code in my commit 
message and make my patch better.

Thanks again
Zhouyi


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121498

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

[PATCH] D121498: let EST_Uninstantiated in FunctionProtoType::canThrow return CT_Dependent

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou updated this revision to Diff 415601.
zhouyizhou edited the summary of this revision.

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

https://reviews.llvm.org/D121498

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaTemplate/class-template-deduction.cpp


Index: clang/test/SemaTemplate/class-template-deduction.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/class-template-deduction.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++17 -verify %s
+// RUN: %clang_cc1 -std=c++1z -verify %s
+#if __cplusplus >= 201703
+// expected-no-diagnostics
+#endif
+template
+void foo() noexcept(T::value);
+struct S {
+  static const bool value;
+};
+
+const bool S::value = 0;
+
+template
+#if __cplusplus >= 201703
+constexpr bool is_noexcept_function(void(Args...) noexcept(is_noexcept)) {
+#else
+constexpr bool is_noexcept_function(void(Args...) noexcept(is_noexcept)) { // 
expected-note{{candidate template ignored}}
+#endif 
+  return is_noexcept; 
+}
+
+void fn()
+{
+#if __cplusplus >= 201703
+  is_noexcept_function(foo);
+#else
+  is_noexcept_function(foo);// expected-error{{no matching function 
for call}}
+#endif 
+
+
+}
+
+
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -3325,9 +3325,10 @@
   switch (getExceptionSpecType()) {
   case EST_Unparsed:
   case EST_Unevaluated:
-  case EST_Uninstantiated:
 llvm_unreachable("should not call this with unresolved exception specs");
 
+  case EST_Uninstantiated:
+return CT_Dependent;
   case EST_DynamicNone:
   case EST_BasicNoexcept:
   case EST_NoexceptTrue:


Index: clang/test/SemaTemplate/class-template-deduction.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/class-template-deduction.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++17 -verify %s
+// RUN: %clang_cc1 -std=c++1z -verify %s
+#if __cplusplus >= 201703
+// expected-no-diagnostics
+#endif
+template
+void foo() noexcept(T::value);
+struct S {
+  static const bool value;
+};
+
+const bool S::value = 0;
+
+template
+#if __cplusplus >= 201703
+constexpr bool is_noexcept_function(void(Args...) noexcept(is_noexcept)) {
+#else
+constexpr bool is_noexcept_function(void(Args...) noexcept(is_noexcept)) { // expected-note{{candidate template ignored}}
+#endif	
+  return is_noexcept; 
+}
+
+void fn()
+{
+#if __cplusplus >= 201703
+  is_noexcept_function(foo);
+#else
+  is_noexcept_function(foo);	// expected-error{{no matching function for call}}
+#endif	
+
+
+}
+
+
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -3325,9 +3325,10 @@
   switch (getExceptionSpecType()) {
   case EST_Unparsed:
   case EST_Unevaluated:
-  case EST_Uninstantiated:
 llvm_unreachable("should not call this with unresolved exception specs");
 
+  case EST_Uninstantiated:
+return CT_Dependent;
   case EST_DynamicNone:
   case EST_BasicNoexcept:
   case EST_NoexceptTrue:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121498: let EST_Uninstantiated in FunctionProtoType::canThrow return CT_Dependent

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou updated this revision to Diff 415655.
zhouyizhou edited the summary of this revision.

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

https://reviews.llvm.org/D121498

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaTemplate/class-template-deduction.cpp


Index: clang/test/SemaTemplate/class-template-deduction.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/class-template-deduction.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++17 -verify %s
+// RUN: %clang_cc1 -std=c++1z -verify %s
+#if __cplusplus >= 201703
+// expected-no-diagnostics
+#endif
+template
+void foo() noexcept(U::X);
+class A {
+public:
+  static const bool X;
+};
+
+const bool A::X = 0;
+
+template
+#if __cplusplus >= 201703
+bool bar(void(B...) noexcept(x)) 
+#else
+bool bar(void(B...) noexcept(x))  // expected-note{{candidate template 
ignored}}
+#endif
+{
+  return x; 
+}
+
+void func()
+{
+#if __cplusplus >= 201703
+  bar(foo);
+#else
+  bar(foo); // expected-error{{no matching function for call}}
+#endif 
+}
+
+
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -3325,9 +3325,10 @@
   switch (getExceptionSpecType()) {
   case EST_Unparsed:
   case EST_Unevaluated:
-  case EST_Uninstantiated:
 llvm_unreachable("should not call this with unresolved exception specs");
 
+  case EST_Uninstantiated:
+return CT_Dependent;
   case EST_DynamicNone:
   case EST_BasicNoexcept:
   case EST_NoexceptTrue:


Index: clang/test/SemaTemplate/class-template-deduction.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/class-template-deduction.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++17 -verify %s
+// RUN: %clang_cc1 -std=c++1z -verify %s
+#if __cplusplus >= 201703
+// expected-no-diagnostics
+#endif
+template
+void foo() noexcept(U::X);
+class A {
+public:
+  static const bool X;
+};
+
+const bool A::X = 0;
+
+template
+#if __cplusplus >= 201703
+bool bar(void(B...) noexcept(x)) 
+#else
+bool bar(void(B...) noexcept(x))  // expected-note{{candidate template ignored}}
+#endif
+{
+  return x; 
+}
+
+void func()
+{
+#if __cplusplus >= 201703
+  bar(foo);
+#else
+  bar(foo);	// expected-error{{no matching function for call}}
+#endif	
+}
+
+
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -3325,9 +3325,10 @@
   switch (getExceptionSpecType()) {
   case EST_Unparsed:
   case EST_Unevaluated:
-  case EST_Uninstantiated:
 llvm_unreachable("should not call this with unresolved exception specs");
 
+  case EST_Uninstantiated:
+return CT_Dependent;
   case EST_DynamicNone:
   case EST_BasicNoexcept:
   case EST_NoexceptTrue:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121498: let EST_Uninstantiated in FunctionProtoType::canThrow return CT_Dependent

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou updated this revision to Diff 415677.
zhouyizhou edited the summary of this revision.

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

https://reviews.llvm.org/D121498

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaTemplate/class-template-noexcept.cpp


Index: clang/test/SemaTemplate/class-template-noexcept.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/class-template-noexcept.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++17 -verify %s
+// RUN: %clang_cc1 -std=c++1z -verify %s
+#if __cplusplus >= 201703
+// expected-no-diagnostics
+#endif
+class A {
+public:
+  static const char X;
+};
+const char A::X = 0;
+
+template void func() noexcept(U::X);
+
+template
+#if __cplusplus >= 201703
+void foo(void(B...) noexcept(x)) {} 
+#else
+void foo(void(B...) noexcept(x)) {} // expected-note{{candidate template 
ignored}}
+#endif
+
+void bar()
+{
+#if __cplusplus >= 201703
+  foo(func);
+#else
+  foo(func);// expected-error{{no matching function for call}}
+#endif 
+}
+
+
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -3325,9 +3325,10 @@
   switch (getExceptionSpecType()) {
   case EST_Unparsed:
   case EST_Unevaluated:
-  case EST_Uninstantiated:
 llvm_unreachable("should not call this with unresolved exception specs");
 
+  case EST_Uninstantiated:
+return CT_Dependent;
   case EST_DynamicNone:
   case EST_BasicNoexcept:
   case EST_NoexceptTrue:


Index: clang/test/SemaTemplate/class-template-noexcept.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/class-template-noexcept.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++17 -verify %s
+// RUN: %clang_cc1 -std=c++1z -verify %s
+#if __cplusplus >= 201703
+// expected-no-diagnostics
+#endif
+class A {
+public:
+  static const char X;
+};
+const char A::X = 0;
+
+template void func() noexcept(U::X);
+
+template
+#if __cplusplus >= 201703
+void foo(void(B...) noexcept(x)) {} 
+#else
+void foo(void(B...) noexcept(x)) {} // expected-note{{candidate template ignored}}
+#endif
+
+void bar()
+{
+#if __cplusplus >= 201703
+  foo(func);
+#else
+  foo(func);	// expected-error{{no matching function for call}}
+#endif	
+}
+
+
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -3325,9 +3325,10 @@
   switch (getExceptionSpecType()) {
   case EST_Unparsed:
   case EST_Unevaluated:
-  case EST_Uninstantiated:
 llvm_unreachable("should not call this with unresolved exception specs");
 
+  case EST_Uninstantiated:
+return CT_Dependent;
   case EST_DynamicNone:
   case EST_BasicNoexcept:
   case EST_NoexceptTrue:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121498: let EST_Uninstantiated in FunctionProtoType::canThrow return CT_Dependent

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou updated this revision to Diff 415805.

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

https://reviews.llvm.org/D121498

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaTemplate/class-template-noexcept.cpp


Index: clang/test/SemaTemplate/class-template-noexcept.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/class-template-noexcept.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++17 -verify %s
+// RUN: %clang_cc1 -std=c++1z -verify %s
+#if __cplusplus >= 201703
+// expected-no-diagnostics
+#endif
+class A {
+public:
+  static const char X;
+};
+const char A::X = 0;
+
+template void func() noexcept(U::X);
+
+template
+#if __cplusplus >= 201703
+void foo(void(B...) noexcept(x)) {} 
+#else
+void foo(void(B...) noexcept(x)) {} // expected-note{{candidate template 
ignored}}
+#endif
+
+void bar()
+{
+#if __cplusplus >= 201703
+  foo(func);
+#else
+  foo(func);// expected-error{{no matching function for call}}
+#endif 
+}
+
+
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -3325,7 +3325,6 @@
   switch (getExceptionSpecType()) {
   case EST_Unparsed:
   case EST_Unevaluated:
-  case EST_Uninstantiated:
 llvm_unreachable("should not call this with unresolved exception specs");
 
   case EST_DynamicNone:
@@ -3347,6 +3346,7 @@
 return CT_Can;
 return CT_Dependent;
 
+  case EST_Uninstantiated:
   case EST_DependentNoexcept:
 return CT_Dependent;
   }


Index: clang/test/SemaTemplate/class-template-noexcept.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/class-template-noexcept.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++17 -verify %s
+// RUN: %clang_cc1 -std=c++1z -verify %s
+#if __cplusplus >= 201703
+// expected-no-diagnostics
+#endif
+class A {
+public:
+  static const char X;
+};
+const char A::X = 0;
+
+template void func() noexcept(U::X);
+
+template
+#if __cplusplus >= 201703
+void foo(void(B...) noexcept(x)) {} 
+#else
+void foo(void(B...) noexcept(x)) {} // expected-note{{candidate template ignored}}
+#endif
+
+void bar()
+{
+#if __cplusplus >= 201703
+  foo(func);
+#else
+  foo(func);	// expected-error{{no matching function for call}}
+#endif	
+}
+
+
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -3325,7 +3325,6 @@
   switch (getExceptionSpecType()) {
   case EST_Unparsed:
   case EST_Unevaluated:
-  case EST_Uninstantiated:
 llvm_unreachable("should not call this with unresolved exception specs");
 
   case EST_DynamicNone:
@@ -3347,6 +3346,7 @@
 return CT_Can;
 return CT_Dependent;
 
+  case EST_Uninstantiated:
   case EST_DependentNoexcept:
 return CT_Dependent;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121498: let EST_Uninstantiated in FunctionProtoType::canThrow return CT_Dependent

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D121498#3385896 , @erichkeane 
wrote:

> LGTM!  Let me know if you need me to commit this for you (and if so, what 
> name/email address you'd like it committed under).

Thank you for your guidance!
I do need you to commit this for me. I want my name and following email address 
be committed with the patch:
Zhouyi Zhou

Thanks again
Zhouyi




Comment at: clang/lib/AST/Type.cpp:3330
 
+  case EST_Uninstantiated:
+return CT_Dependent;

erichkeane wrote:
> Slight preference for moving just this line to 3351 (above or below) to get 
> the fallthrough behavior on the existing CT_Dependent without condition.
thank you for your guidance, thanks zhouyi


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

https://reviews.llvm.org/D121498

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

[PATCH] D119493: Fixing surplus assert condition in EvaluateTemporary

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou updated this revision to Diff 416636.
zhouyizhou edited the summary of this revision.
zhouyizhou added reviewers: mizvekov, eli.friedman, EricWF, faisalv.
Herald added a project: All.

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

https://reviews.llvm.org/D119493

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/AST/issue53741.cpp


Index: clang/test/AST/issue53741.cpp
===
--- /dev/null
+++ clang/test/AST/issue53741.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+// expected-no-diagnostics 
+typedef bool Var;
+bool foobool() {
+   return (bool().Var::~Var(), false); 
+}
+
+  
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10173,7 +10173,7 @@
 /// Evaluate an expression of record type as a temporary.
 static bool EvaluateTemporary(const Expr *E, LValue &Result, EvalInfo &Info) {
   assert(!E->isValueDependent());
-  assert(E->isPRValue() && E->getType()->isRecordType());
+  assert(E->isPRValue());
   return TemporaryExprEvaluator(Info, Result).Visit(E);
 }
 


Index: clang/test/AST/issue53741.cpp
===
--- /dev/null
+++ clang/test/AST/issue53741.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+// expected-no-diagnostics 
+typedef bool Var;
+bool foobool() {
+	return (bool().Var::~Var(), false); 
+}
+
+  
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10173,7 +10173,7 @@
 /// Evaluate an expression of record type as a temporary.
 static bool EvaluateTemporary(const Expr *E, LValue &Result, EvalInfo &Info) {
   assert(!E->isValueDependent());
-  assert(E->isPRValue() && E->getType()->isRecordType());
+  assert(E->isPRValue());
   return TemporaryExprEvaluator(Info, Result).Visit(E);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119493: Fixing surplus assert condition in EvaluateTemporary

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou created this revision.
zhouyizhou added reviewers: rsmith, hokein.
zhouyizhou requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In LLVM/Clang Debug build, following code will trigger the assert to fire:
typedef int I;
void foo() {

  int i = (int().I::~I(), 3);

}

I think the non record type temporary object should also be allowed here.
Thanks 
Zhouyi Zhou
zhouzho...@gmail.com


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119493

Files:
  clang/lib/AST/ExprConstant.cpp


Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10173,7 +10173,7 @@
 /// Evaluate an expression of record type as a temporary.
 static bool EvaluateTemporary(const Expr *E, LValue &Result, EvalInfo &Info) {
   assert(!E->isValueDependent());
-  assert(E->isPRValue() && E->getType()->isRecordType());
+  assert(E->isPRValue());
   return TemporaryExprEvaluator(Info, Result).Visit(E);
 }
 


Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -10173,7 +10173,7 @@
 /// Evaluate an expression of record type as a temporary.
 static bool EvaluateTemporary(const Expr *E, LValue &Result, EvalInfo &Info) {
   assert(!E->isValueDependent());
-  assert(E->isPRValue() && E->getType()->isRecordType());
+  assert(E->isPRValue());
   return TemporaryExprEvaluator(Info, Result).Visit(E);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129953: [PATCH] [clang-tidy] NFC: add preposition "of" to code annotation of ElseAfterReturnCheck

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou created this revision.
zhouyizhou added reviewers: njames93, nridge, alexfh_, stephenkelly.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
zhouyizhou requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Hi,
I think we should add add preposition "of" to code annotation of  
ElseAfterReturnCheck ;-)
Many Thanks Zhouyi

Zhouyi Zhou 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129953

Files:
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp


Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -263,7 +263,7 @@
 if (!WarnOnConditionVariables)
   return;
 if (IsLastInScope) {
-  // If the if statement is the last statement its enclosing statements
+  // If the if statement is the last statement of its enclosing statements
   // scope, we can pull the decl out of the if statement.
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
<< ControlFlowInterruptor
@@ -299,7 +299,7 @@
 if (!WarnOnConditionVariables)
   return;
 if (IsLastInScope) {
-  // If the if statement is the last statement its enclosing statements
+  // If the if statement is the last statement of its enclosing statements
   // scope, we can pull the decl out of the if statement.
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
<< ControlFlowInterruptor


Index: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -263,7 +263,7 @@
 if (!WarnOnConditionVariables)
   return;
 if (IsLastInScope) {
-  // If the if statement is the last statement its enclosing statements
+  // If the if statement is the last statement of its enclosing statements
   // scope, we can pull the decl out of the if statement.
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
<< ControlFlowInterruptor
@@ -299,7 +299,7 @@
 if (!WarnOnConditionVariables)
   return;
 if (IsLastInScope) {
-  // If the if statement is the last statement its enclosing statements
+  // If the if statement is the last statement of its enclosing statements
   // scope, we can pull the decl out of the if statement.
   DiagnosticBuilder Diag = diag(ElseLoc, WarningMessage)
<< ControlFlowInterruptor
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129953: [PATCH] [clang-tidy] NFC: add preposition "of" to code annotation of ElseAfterReturnCheck

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D129953#3657920 , @njames93 wrote:

> While you're here could you also put full stops( or periods 🙃) at the end of 
> the sentences.

Thank Nathan for reviewing my patch
I am here, and am very happy to make further enhancement ;-) But because my 
native language is not English, I don't know where should I put the full stops 
at . 
Isn't  "If the if statement is the last statement of its enclosing statements 
scope, we can pull the decl out of the if statement." constitute a complete 
sentence?

Thanks again
Zhouyi


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129953

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

[PATCH] D129953: [PATCH] [clang-tidy] NFC: add preposition "of" to code annotation of ElseAfterReturnCheck

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

Hi,

I have no write access to LLVM, could you commit for me at your convenience ;-)

Many thanks

Zhouyi Zhou 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129953

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

[PATCH] D102577: TreeTransform.h: make the switch case more beautiful

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou created this revision.
zhouyizhou added reviewers: sepavloff, craig.topper, rsmith, klimek, 
doug.gregor.
Herald added a subscriber: pengfei.
zhouyizhou requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Hi, 


I am new to LLVM, and I really want to get involved in LLVM community.


  

I guess if the following switch case of function


TransformNestedNameSpecifierLoc could possibly made more beautiful with 


'break' stmt moved inside of the right brace.


  

I think move of 'break' stmt will not change the invoking destructor of 


IdInfo.


  

Thanks for your effort.


  

I have done make check-all on x86_64-linux  


Signed-off-by: Zhouyi Zhou 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D102577

Files:
  clang/lib/Sema/TreeTransform.h


Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -3964,8 +3964,8 @@
   if (SemaRef.BuildCXXNestedNameSpecifier(/*Scope=*/nullptr, IdInfo, false,
   SS, FirstQualifierInScope, 
false))
 return NestedNameSpecifierLoc();
-}
   break;
+}
 
 case NestedNameSpecifier::Namespace: {
   NamespaceDecl *NS


Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -3964,8 +3964,8 @@
   if (SemaRef.BuildCXXNestedNameSpecifier(/*Scope=*/nullptr, IdInfo, false,
   SS, FirstQualifierInScope, false))
 return NestedNameSpecifierLoc();
-}
   break;
+}
 
 case NestedNameSpecifier::Namespace: {
   NamespaceDecl *NS
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102577: TreeTransform.h: make the switch case more beautiful

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou updated this revision to Diff 345710.
zhouyizhou added a comment.

I use git diff -U9 instead


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

https://reviews.llvm.org/D102577

Files:
  clang/lib/Sema/TreeTransform.h


Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -3964,8 +3964,8 @@
   if (SemaRef.BuildCXXNestedNameSpecifier(/*Scope=*/nullptr, IdInfo, false,
   SS, FirstQualifierInScope, 
false))
 return NestedNameSpecifierLoc();
-}
   break;
+}
 
 case NestedNameSpecifier::Namespace: {
   NamespaceDecl *NS


Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -3964,8 +3964,8 @@
   if (SemaRef.BuildCXXNestedNameSpecifier(/*Scope=*/nullptr, IdInfo, false,
   SS, FirstQualifierInScope, false))
 return NestedNameSpecifierLoc();
-}
   break;
+}
 
 case NestedNameSpecifier::Namespace: {
   NamespaceDecl *NS
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D102577: TreeTransform.h: make the switch case more beautiful

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

I do some "grep" in clang/lib directory:
llvm-project/clang/lib#find . -name "*.*"|xargs grep -n -B 2 "break;"|grep -A 2 
"}"

my patch also seems to match the coding style of other parts of clang.

Thanks again


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

https://reviews.llvm.org/D102577

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

[PATCH] D102577: NFC: TreeTransform.h: make the switch case more beautiful

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou updated this revision to Diff 345792.
zhouyizhou retitled this revision from "TreeTransform.h: make the switch case 
more beautiful " to "NFC: TreeTransform.h: make the switch case more beautiful 
".
zhouyizhou added a comment.

use clang-format to fix the function TransformNestedNameSpecifierLoc


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

https://reviews.llvm.org/D102577

Files:
  clang/lib/Sema/TreeTransform.h


Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -3941,12 +3941,10 @@
   return Sema::ConditionResult();
 }
 
-template
-NestedNameSpecifierLoc
-TreeTransform::TransformNestedNameSpecifierLoc(
-NestedNameSpecifierLoc NNS,
- QualType ObjectType,
- NamedDecl *FirstQualifierInScope) 
{
+template 
+NestedNameSpecifierLoc TreeTransform::TransformNestedNameSpecifierLoc(
+NestedNameSpecifierLoc NNS, QualType ObjectType,
+NamedDecl *FirstQualifierInScope) {
   SmallVector Qualifiers;
   for (NestedNameSpecifierLoc Qualifier = NNS; Qualifier;
Qualifier = Qualifier.getPrefix())
@@ -3960,28 +3958,26 @@
 switch (QNNS->getKind()) {
 case NestedNameSpecifier::Identifier: {
   Sema::NestedNameSpecInfo IdInfo(QNNS->getAsIdentifier(),
-  Q.getLocalBeginLoc(), Q.getLocalEndLoc(), 
ObjectType);
+  Q.getLocalBeginLoc(), Q.getLocalEndLoc(),
+  ObjectType);
   if (SemaRef.BuildCXXNestedNameSpecifier(/*Scope=*/nullptr, IdInfo, false,
   SS, FirstQualifierInScope, 
false))
 return NestedNameSpecifierLoc();
-}
   break;
+}
 
 case NestedNameSpecifier::Namespace: {
-  NamespaceDecl *NS
-= cast_or_null(
-getDerived().TransformDecl(
-  Q.getLocalBeginLoc(),
-   
QNNS->getAsNamespace()));
+  NamespaceDecl *NS =
+  cast_or_null(getDerived().TransformDecl(
+  Q.getLocalBeginLoc(), QNNS->getAsNamespace()));
   SS.Extend(SemaRef.Context, NS, Q.getLocalBeginLoc(), Q.getLocalEndLoc());
   break;
 }
 
 case NestedNameSpecifier::NamespaceAlias: {
-  NamespaceAliasDecl *Alias
-= cast_or_null(
-  getDerived().TransformDecl(Q.getLocalBeginLoc(),
- QNNS->getAsNamespaceAlias()));
+  NamespaceAliasDecl *Alias =
+  cast_or_null(getDerived().TransformDecl(
+  Q.getLocalBeginLoc(), QNNS->getAsNamespaceAlias()));
   SS.Extend(SemaRef.Context, Alias, Q.getLocalBeginLoc(),
 Q.getLocalEndLoc());
   break;
@@ -4017,7 +4013,7 @@
 if (TL.getType()->isEnumeralType())
   SemaRef.Diag(TL.getBeginLoc(),
diag::warn_cxx98_compat_enum_nested_name_spec);
-SS.Extend(SemaRef.Context, /*FIXME:*/SourceLocation(), TL,
+SS.Extend(SemaRef.Context, /*FIXME:*/ SourceLocation(), TL,
   Q.getLocalEndLoc());
 break;
   }
@@ -4026,7 +4022,7 @@
   TypedefTypeLoc TTL = TL.getAs();
   if (!TTL || !TTL.getTypedefNameDecl()->isInvalidDecl()) {
 SemaRef.Diag(TL.getBeginLoc(), diag::err_nested_name_spec_non_tag)
-  << TL.getType() << SS.getRange();
+<< TL.getType() << SS.getRange();
   }
   return NestedNameSpecifierLoc();
 }


Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -3941,12 +3941,10 @@
   return Sema::ConditionResult();
 }
 
-template
-NestedNameSpecifierLoc
-TreeTransform::TransformNestedNameSpecifierLoc(
-NestedNameSpecifierLoc NNS,
- QualType ObjectType,
- NamedDecl *FirstQualifierInScope) {
+template 
+NestedNameSpecifierLoc TreeTransform::TransformNestedNameSpecifierLoc(
+NestedNameSpecifierLoc NNS, QualType ObjectType,
+NamedDecl *FirstQualifierInScope) {
   SmallVector Qualifiers;
   for (NestedNameSpecifierLoc Qualifier = NNS; Qualifier;
Qualifier = Qualifier.getPrefix())
@@ -3960,28 +3958,26 @@
 switch (QNNS->getKind()) {
 case NestedNameSpecifier::Identifier: {
   Sema::NestedNameSpecInfo IdInfo(QNNS->getAsIdentifier(),
-  Q.getLocalBeginLoc(), Q.getLocalEndLoc(), ObjectType);
+  Q.getLocalBeginLoc(), Q.getLocalEndLoc(),
+   

[PATCH] D109408: [libcxxabi] NFC: fix incorrect indentation of braces

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D109408#3018830 , @ldionne wrote:

> In D109408#2996128 , @zhouyizhou 
> wrote:
>
>> In D109408#2996092 , @Quuxplusone 
>> wrote:
>>
>>> I was going to land this just now, but then realized that I've lost the 
>>> thread of why we're doing most of this. Why are we touching 
>>> clang/test/SemaCXX/? So I'm going to land just the fixing of the curly 
>>> braces in libcxxabi/src/.
>>
>> Hi, 
>> Very sorry to bring you so much trouble. 
>> I am new to LLVM, and I am only intend to fix the curly braces in 
>> libcxxabi/src/,
>> I followed https://www.llvm.org/docs/Phabricator.html#phabricator-reviews to 
>> make a patch using: 
>> git diff -U99 u 
>>  > 
>> /tmp/mypatch.patch
>> I am also very confused why clang/test/SemaCXX/ appears in mypatch.patch.
>> Sorry again.
>> And thanks for landing the fixing of  the curly braces in libcxxabi/src/ for 
>> me.
>>
>> Zhouyi
>
> When you submit patches, please follow 
> https://libcxx.llvm.org/Contributing.html. In particular, you should follow 
> these instructions for submitting using `arc diff` -- it works a lot better 
> than the web interface: 
> https://www.llvm.org/docs/Phabricator.html#requesting-a-review-via-the-command-line.
>
> Thanks for the patch!

Thanks for the instructions
I will follow https://libcxx.llvm.org/Contributing.html and will follow the 
instructions to requesting a review via the command line.
It's a very good opportunity to improve myself ;-)
Thanks again
Zhouyi


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109408

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

[PATCH] D104604: [clang] NFC: add line break at the end of if expressions

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou created this revision.
zhouyizhou added reviewers: pengfei, rsmith, rjmccall, doug.gregor, sepavloff, 
craig.topper, klimek.
zhouyizhou requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Hi,

In function TransformTemplateArgument,
would it be better to add line break at the end of "if" expressions?

I use clang-format to do the job for me.

Thanks a lot


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104604

Files:
  clang/lib/Sema/TreeTransform.h


Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -4322,10 +4322,10 @@
   Arg, QualType(), getDerived().getBaseLocation());
 }
 
-template
+template 
 bool TreeTransform::TransformTemplateArgument(
- const TemplateArgumentLoc &Input,
- TemplateArgumentLoc &Output, bool 
Uneval) {
+const TemplateArgumentLoc &Input, TemplateArgumentLoc &Output,
+bool Uneval) {
   const TemplateArgument &Arg = Input.getArgument();
   switch (Arg.getKind()) {
   case TemplateArgument::Null:
@@ -4374,7 +4374,8 @@
   DI = InventTypeSourceInfo(Input.getArgument().getAsType());
 
 DI = getDerived().TransformType(DI);
-if (!DI) return true;
+if (!DI)
+  return true;
 
 Output = TemplateArgumentLoc(TemplateArgument(DI->getType()), DI);
 return false;
@@ -4390,9 +4391,8 @@
 
 CXXScopeSpec SS;
 SS.Adopt(QualifierLoc);
-TemplateName Template
-  = getDerived().TransformTemplateName(SS, Arg.getAsTemplate(),
-   Input.getTemplateNameLoc());
+TemplateName Template = getDerived().TransformTemplateName(
+SS, Arg.getAsTemplate(), Input.getTemplateNameLoc());
 if (Template.isNull())
   return true;
 
@@ -4414,11 +4414,13 @@
 Sema::ExpressionEvaluationContextRecord::EK_TemplateArgument);
 
 Expr *InputExpr = Input.getSourceExpression();
-if (!InputExpr) InputExpr = Input.getArgument().getAsExpr();
+if (!InputExpr)
+  InputExpr = Input.getArgument().getAsExpr();
 
 ExprResult E = getDerived().TransformExpr(InputExpr);
 E = SemaRef.ActOnConstantExpression(E);
-if (E.isInvalid()) return true;
+if (E.isInvalid())
+  return true;
 Output = TemplateArgumentLoc(TemplateArgument(E.get()), E.get());
 return false;
   }


Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -4322,10 +4322,10 @@
   Arg, QualType(), getDerived().getBaseLocation());
 }
 
-template
+template 
 bool TreeTransform::TransformTemplateArgument(
- const TemplateArgumentLoc &Input,
- TemplateArgumentLoc &Output, bool Uneval) {
+const TemplateArgumentLoc &Input, TemplateArgumentLoc &Output,
+bool Uneval) {
   const TemplateArgument &Arg = Input.getArgument();
   switch (Arg.getKind()) {
   case TemplateArgument::Null:
@@ -4374,7 +4374,8 @@
   DI = InventTypeSourceInfo(Input.getArgument().getAsType());
 
 DI = getDerived().TransformType(DI);
-if (!DI) return true;
+if (!DI)
+  return true;
 
 Output = TemplateArgumentLoc(TemplateArgument(DI->getType()), DI);
 return false;
@@ -4390,9 +4391,8 @@
 
 CXXScopeSpec SS;
 SS.Adopt(QualifierLoc);
-TemplateName Template
-  = getDerived().TransformTemplateName(SS, Arg.getAsTemplate(),
-   Input.getTemplateNameLoc());
+TemplateName Template = getDerived().TransformTemplateName(
+SS, Arg.getAsTemplate(), Input.getTemplateNameLoc());
 if (Template.isNull())
   return true;
 
@@ -4414,11 +4414,13 @@
 Sema::ExpressionEvaluationContextRecord::EK_TemplateArgument);
 
 Expr *InputExpr = Input.getSourceExpression();
-if (!InputExpr) InputExpr = Input.getArgument().getAsExpr();
+if (!InputExpr)
+  InputExpr = Input.getArgument().getAsExpr();
 
 ExprResult E = getDerived().TransformExpr(InputExpr);
 E = SemaRef.ActOnConstantExpression(E);
-if (E.isInvalid()) return true;
+if (E.isInvalid())
+  return true;
 Output = TemplateArgumentLoc(TemplateArgument(E.get()), E.get());
 return false;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104604: [clang] NFC: add line break at the end of if expressions

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D104604#2849676 , @xgupta wrote:

> It seems you need someone to commit this patch? If so you need to share your 
> name and email address. So that someone with commit access can commit it for 
> you.

Thank you very much, I did want someone to commit this patch for me.

My name is Zhouyi Zhou
My email address is zhouzho...@gmail.com


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104604

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

[PATCH] D112050: [clang] NFC: return void in an alternative expression after call to void function.

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

ping 
Just a ping, to see if there are any comments :-P


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112050

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

[PATCH] D115014: [clang] RFC: NFC: simplify macro tokens assignment

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou created this revision.
zhouyizhou added reviewers: lattner, rnk, MaskRay, simon_tatham.
zhouyizhou requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I think we could simplify Tokens = &*Macro->tokens_begin() in 
clang::TokenLexer::Init.
Because the type of Tokens is const class clang::Token *, the return type of  
Macro->tokens_begin()  is also class clang::Token *, so I think the &* sequence 
can be removed
Thanks for you time

Thanks
Zhouyi

Signed-off-by: Zhouyi Zhou 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115014

Files:
  clang/lib/Lex/TokenLexer.cpp


Index: clang/lib/Lex/TokenLexer.cpp
===
--- clang/lib/Lex/TokenLexer.cpp
+++ clang/lib/Lex/TokenLexer.cpp
@@ -50,7 +50,7 @@
   AtStartOfLine = Tok.isAtStartOfLine();
   HasLeadingSpace = Tok.hasLeadingSpace();
   NextTokGetsSpace = false;
-  Tokens = &*Macro->tokens_begin();
+  Tokens = Macro->tokens_begin();
   OwnsTokens = false;
   DisableMacroExpansion = false;
   IsReinject = false;


Index: clang/lib/Lex/TokenLexer.cpp
===
--- clang/lib/Lex/TokenLexer.cpp
+++ clang/lib/Lex/TokenLexer.cpp
@@ -50,7 +50,7 @@
   AtStartOfLine = Tok.isAtStartOfLine();
   HasLeadingSpace = Tok.hasLeadingSpace();
   NextTokGetsSpace = false;
-  Tokens = &*Macro->tokens_begin();
+  Tokens = Macro->tokens_begin();
   OwnsTokens = false;
   DisableMacroExpansion = false;
   IsReinject = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115014: [clang] RFC: NFC: simplify macro tokens assignment

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D115014#3169400 , @simon_tatham 
wrote:

> I don't think so, I'm afraid. If you look at the definition of 
> `MacroInfo::tokens_begin()` in `clang/include/clang/Lex/MacroInfo.h`, you see 
> that it doesn't return a raw `Token *`: it returns a C++ iterator object.
>
>   using tokens_iterator = SmallVectorImpl::const_iterator;
>   
>   tokens_iterator tokens_begin() const { return ReplacementTokens.begin(); }
>
> So you do have to dereference the iterator to get a `Token &`, and then 
> address-take that to turn it into a `Token *`.

Thank Simon for reviewing my patch  ;-)

In llvm/include/llvm/ADT/SmallVector.h, I found  this: using const_iterator = 
const T *;

also I use gdb to print the return type of tokens_begin()
(gdb) ptype const_iterator
type = const class clang::Token {

...
} *

So, there still remains little puzzle in my head  ;-)

Thanks again
Zhouyi


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115014

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

[PATCH] D115014: [clang] RFC: NFC: simplify macro tokens assignment

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D115014#3169444 , @simon_tatham 
wrote:

> Ah, now I see what you mean – I didn't look far enough!
>
> I don't know this code well (in fact I'm not sure why you picked me as a 
> reviewer), but off the top of my head: the question is not just whether 
> `tokens_iterator` happens to be the same thing as `const Token *` //now//. 
> It's whether it's guaranteed to stay that way in the future.
>
> One of the purposes of these iterator types is that they form an opaque 
> abstraction layer: the client uses the type in only the ways that are 
> guaranteed to work by the iterator specification, and then the implementation 
> can completely change without breaking any client.
>
> If you make a change like this, and later `SmallVector::iterator` changes to 
> some other legal implementation, or `tokens_iterator` changes to be something 
> other than a `SmallVector::iterator`, then this change will have to be 
> reverted.

Thank Simon for resolve my confusion

I am crystal clear about the use of &* pair now. I did benefit a lot.

I pick you as a reviewer because you have committed to TokenLexer.cpp recently 
(I use git log along with git blame );-)

Thanks again
Zhouyi


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115014

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

[PATCH] D133066: fix a typo in comment of AddConversionCandidate

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

After 4 weeks' study, I think the comment didn't need to be changed, sorry to 
have bring your so much trouble.
During this valuable process of studying, I grow up a lot. I learned to read 
the C++ standard, and compare the standard to its implementation.
In my case, the "user-defined conversion" is the variable "Candidate", the 
"second standard conversion sequence" is the object member  
"Candidate.FinalConversion".
The only pity during my study is that I can't find a example code to let Clang 
(even with commit cba72b1f620fd) hit the code below above comment.

I am going to close this thread.

Thanks for reviewing my patch, and for your guidance.
Cheers
Zhouyi


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133066

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

[PATCH] D133066: fix a typo in comment of AddConversionCandidate

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D133066#3846907 , @aaron.ballman 
wrote:

> In D133066#3845136 , @zhouyizhou 
> wrote:
>
>> After 4 weeks' study, I think the comment didn't need to be changed, sorry 
>> to have bring your so much trouble.
>
> No worries at all, there was no trouble here!

Thank Aaron for your patience and for your encouragement!

>> During this valuable process of studying, I grow up a lot. I learned to read 
>> the C++ standard, and compare the standard to its implementation.
>> In my case, the "user-defined conversion" is the variable "Candidate", the 
>> "second standard conversion sequence" is the object member  
>> "Candidate.FinalConversion".
>> The only pity during my study is that I can't find a example code to let 
>> Clang (even with commit cba72b1f620fd) hit the code below above comment.
>
> I'm glad you found it valuable! As for a code example to hit the code below 
> that comment, the closest I could come is:
>
>   struct S {
> operator int&& () const { return 12; }
>   };
>   
>   void func(int &&i);
>   
>   int main() {
> S s;
> func(s);
>   }
>
> however, that still fails the lvalue-to-rvalue test. I poked at it for a 
> while and I'm not seeing a case where it's possible to hit that condition (it 
> doesn't mean one doesn't exist, just that I didn't have the time to chase it 
> down fully).

Yes! this is the closest example that I can try to hit the code below that 
comment. Yes, I can't hit that condition both with "commit cba72b1f620fd" (I 
debug her in old debian 6 virtual machine) and clang current. However this is 
still a very fruitful journal for me ;-)

I am going to close this thread after a couple of days.

And thanks again for your time ;-)

Cheers
Zhouyi


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133066

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

[PATCH] D137263: add boundary check for ASTUnresolvedSet::erase

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou created this revision.
zhouyizhou added reviewers: chandlerc, aprantl, rsmith, rjmccall.
Herald added a project: All.
zhouyizhou requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When compile following code with clang (Debug build), Assertion will be 
triggered.

struct A
{

  struct Nested {};
  operator Nested*() {return 0;};

};

struct B : A
{

  using A::operator typename A::Nested*;
  operator typename A::Nested *() {
  struct A * thi = this;
  return *thi;
  };

};

The assertion fail is caused by:
void erase(unsigned I) { Decls[I] = Decls.pop_back_val(); }
when size of Decls is 1 before erase.

clang-14 build on Ubuntu 22.04 don't trigger above assertion because clang-14 
using g++ -std=c++14 by default:

_ZN5clang16ASTUnresolvedSet5eraseEj:
.LFB3970:
.cfi_startproc
endbr64
pushq   %rbp
.cfi_def_cfa_offset 16
.cfi_offset 6, -16
movq%rsp, %rbp
.cfi_def_cfa_register 6
pushq   %r12
pushq   %rbx
subq$16, %rsp
.cfi_offset 12, -24
.cfi_offset 3, -32
movq%rdi, -24(%rbp)
movl%esi, -28(%rbp)
movq-24(%rbp), %r12
movq-24(%rbp), %rax
movl-28(%rbp), %edx
movl%edx, %esi
movq%rax, %rdi
call_ZN5clang9ASTVectorINS_14DeclAccessPairEEixEj
movq%rax, %rbx
movq%r12, %rdi
call_ZN5clang9ASTVectorINS_14DeclAccessPairEE12pop_back_valEv
movq%rax, (%rbx)

We can see when compile with -std=c++14 
_ZN5clang9ASTVectorINS_14DeclAccessPairEEixEj is called before 
_ZN5clang9ASTVectorINS_14DeclAccessPairEE12pop_back_valEv, so above assertion 
will not trigger

Thanks for review my patch
Zhouyi Zhou
zhouzho...@gmail.com


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137263

Files:
  clang/include/clang/AST/ASTUnresolvedSet.h


Index: clang/include/clang/AST/ASTUnresolvedSet.h
===
--- clang/include/clang/AST/ASTUnresolvedSet.h
+++ clang/include/clang/AST/ASTUnresolvedSet.h
@@ -69,7 +69,12 @@
 return false;
   }
 
-  void erase(unsigned I) { Decls[I] = Decls.pop_back_val(); }
+  void erase(unsigned I) {
+if (Decls.size() == 1) /// Let else branch complain when size < 1
+  Decls.pop_back_val();
+else
+  Decls[I] = Decls.pop_back_val();
+  }
 
   void clear() { Decls.clear(); }
 


Index: clang/include/clang/AST/ASTUnresolvedSet.h
===
--- clang/include/clang/AST/ASTUnresolvedSet.h
+++ clang/include/clang/AST/ASTUnresolvedSet.h
@@ -69,7 +69,12 @@
 return false;
   }
 
-  void erase(unsigned I) { Decls[I] = Decls.pop_back_val(); }
+  void erase(unsigned I) {
+if (Decls.size() == 1) /// Let else branch complain when size < 1
+  Decls.pop_back_val();
+else
+  Decls[I] = Decls.pop_back_val();
+  }
 
   void clear() { Decls.clear(); }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137263: add boundary check for ASTUnresolvedSet::erase

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou updated this revision to Diff 472642.
zhouyizhou added a comment.

We should also consider the situation of erasing the size - 1th element


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

https://reviews.llvm.org/D137263

Files:
  clang/include/clang/AST/ASTUnresolvedSet.h


Index: clang/include/clang/AST/ASTUnresolvedSet.h
===
--- clang/include/clang/AST/ASTUnresolvedSet.h
+++ clang/include/clang/AST/ASTUnresolvedSet.h
@@ -69,7 +69,12 @@
 return false;
   }
 
-  void erase(unsigned I) { Decls[I] = Decls.pop_back_val(); }
+  void erase(unsigned I) {
+if (I == (Decls.size() - 1))
+  Decls.pop_back_val();
+else
+  Decls[I] = Decls.pop_back_val();
+  }
 
   void clear() { Decls.clear(); }
 


Index: clang/include/clang/AST/ASTUnresolvedSet.h
===
--- clang/include/clang/AST/ASTUnresolvedSet.h
+++ clang/include/clang/AST/ASTUnresolvedSet.h
@@ -69,7 +69,12 @@
 return false;
   }
 
-  void erase(unsigned I) { Decls[I] = Decls.pop_back_val(); }
+  void erase(unsigned I) {
+if (I == (Decls.size() - 1))
+  Decls.pop_back_val();
+else
+  Decls[I] = Decls.pop_back_val();
+  }
 
   void clear() { Decls.clear(); }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137263: add boundary check for ASTUnresolvedSet::erase

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

Following is the back trace when the assertion fires before my fix:
#6  0x77960e96 in __GI___assert_fail (assertion=0x656ed285 "Begin + 
idx < End",

  file=0x656ed240 
"/tmp/llvm-test/llvm-project.save/clang/include/clang/AST/ASTVector.h", 
line=112, 
  function=0x656ed1b0 "T& clang::ASTVector::operator[](unsigned int) 
[with T = clang::DeclAccessPair; clang::ASTVector::reference = 
clang::DeclAccessPair&]") at ./assert/assert.c:101

#7  0x5f448c05 in clang::ASTVector::operator[] 
(this=0x691a19b8, idx=0)

  at /tmp/llvm-test/llvm-project.save/clang/include/clang/AST/ASTVector.h:112

#8  0x5f446e26 in clang::ASTUnresolvedSet::erase (this=0x691a19b8, 
I=0)

  at 
/tmp/llvm-test/llvm-project.save/clang/include/clang/AST/ASTUnresolvedSet.h:72

#9  0x5f43e7a3 in clang::CXXRecordDecl::removeConversion 
(this=0x691a18d8, ConvDecl=0x691a1c48)

  at /tmp/llvm-test/llvm-project.save/clang/lib/AST/DeclCXX.cpp:1797

#10 0x5e121349 in clang::Sema::HideUsingShadowDecl 
(this=0x6918f990, S=0x691acce0, Shadow=0x691a1c48)

  at /tmp/llvm-test/llvm-project.save/clang/lib/Sema/SemaDeclCXX.cpp:12158

#11 0x5e80bc89 in clang::Sema::CheckOverload (this=0x6918f990, 
S=0x691acce0, New=0x691a1da0, Old=...,

  Match=@0x7fff72b8: 0x0, NewIsUsingDecl=false) at 
/tmp/llvm-test/llvm-project.save/clang/lib/Sema/SemaOverload.cpp:1063

#12 0x5dfdb925 in clang::Sema::CheckFunctionDeclaration 
(this=0x6918f990, S=0x691acce0, NewFD=0x691a1da0,

  Previous=..., IsMemberSpecialization=false, DeclIsDefn=true)
  at /tmp/llvm-test/llvm-project.save/clang/lib/Sema/SemaDecl.cpp:11356

#13 0x5dfd64f3 in clang::Sema::ActOnFunctionDeclarator 
(this=0x6918f990, S=0x691acce0, D=...,

  DC=0x691a1918, TInfo=0x691a1d68, Previous=..., 
TemplateParamListsRef=..., AddToScope=@0x7fff7b30: true)
  at /tmp/llvm-test/llvm-project.save/clang/lib/Sema/SemaDecl.cpp:10205

#14 0x5dfc57cb in clang::Sema::HandleDeclarator (this=0x6918f990, 
S=0x691acce0, D=..., TemplateParamLists=...)

  at /tmp/llvm-test/llvm-project.save/clang/lib/Sema/SemaDecl.cpp:6348

--Type  for more, q to quit, c to continue without paging--
#15 0x5e0fa7bc in clang::Sema::ActOnCXXMemberDeclarator 
(this=0x6918f990, S=0x691acce0, AS=clang::AS_public,

  D=..., TemplateParameterLists=..., BW=0x0, VS=..., 
InitStyle=clang::ICIS_NoInit)
  at /tmp/llvm-test/llvm-project.save/clang/lib/Sema/SemaDeclCXX.cpp:3477

#16 0x5dc5b40a in clang::Parser::ParseCXXInlineMethodDef 
(this=0x6919b200, AS=clang::AS_public, AccessAttrs=...,

  D=..., TemplateInfo=..., VS=..., PureSpecLoc=...)
  at 
/tmp/llvm-test/llvm-project.save/clang/lib/Parse/ParseCXXInlineMethods.cpp:42

#17 0x5dc9b22b in clang::Parser::ParseCXXClassMemberDeclaration 
(this=0x6919b200, AS=clang::AS_public,

  AccessAttrs=..., TemplateInfo=..., TemplateDiags=0x0)
  at /tmp/llvm-test/llvm-project.save/clang/lib/Parse/ParseDeclCXX.cpp:2912

#18 0x5dc9d149 in 
clang::Parser::ParseCXXClassMemberDeclarationWithPragmas (this=0x6919b200,

  AS=@0x7fff94f0: clang::AS_public, AccessAttrs=..., 
TagType=clang::TST_struct, TagDecl=0x691a18d8)
  at /tmp/llvm-test/llvm-project.save/clang/lib/Parse/ParseDeclCXX.cpp:3343

#19 0x5dc9df12 in clang::Parser::ParseCXXMemberSpecification 
(this=0x6919b200, RecordLoc=..., AttrFixitLoc=...,

  Attrs=..., TagType=25, TagDecl=0x691a18d8) at 
/tmp/llvm-test/llvm-project.save/clang/lib/Parse/ParseDeclCXX.cpp:3547

#20 0x5dc97ce3 in clang::Parser::ParseClassSpecifier 
(this=0x6919b200, TagTokKind=clang::tok::kw_struct,

  StartLoc=..., DS=..., TemplateInfo=..., AS=clang::AS_none, 
EnteringContext=true, 
  DSC=clang::Parser::DeclSpecContext::DSC_top_level, Attributes=...)
  at /tmp/llvm-test/llvm-project.save/clang/lib/Parse/ParseDeclCXX.cpp:2063

#21 0x5dc7504a in clang::Parser::ParseDeclarationSpecifiers 
(this=0x6919b200, DS=..., TemplateInfo=...,

  AS=clang::AS_none, DSContext=clang::Parser::DeclSpecContext::DSC_top_level, 
LateAttrs=0x0)
  at /tmp/llvm-test/llvm-project.save/clang/lib/Parse/ParseDecl.cpp:4144

#22 0x5dc4a70e in clang::Parser::ParseDeclOrFunctionDefInternal 
(this=0x6919b200, Attrs=..., DS=...,

  AS=clang::AS_none) at 
/tmp/llvm-test/llvm-project.save/clang/lib/Parse/Parser.cpp:1087

#23 0x5dc4adb1 in clang::Parser::ParseDeclarationOrFunctionDefinition 
(this=0x6919b200, Attrs=..., DS=0x0,

  AS=clang::AS_none) at 
/tmp/llvm-test/llvm-project.save/clang/lib/Parse/Parser.cpp:1193

#24 0x5dc4a260 in clang::Parser::ParseExternalDeclaration 
(this=0x6919b200, Attrs=..., DS=0x0)

  at /tmp/llvm-test/llvm-project.save/clang/lib/Parse/Parser.cpp:1019

#25 0x5dc48fac in clang::Parser::ParseTopLevelDecl 
(this=0x6919b200, Result=...,

  ImportState=@0x7fffa97c: clang:

[PATCH] D137263: add boundary check for ASTUnresolvedSet::erase

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D137263#3903845 , @rjmccall wrote:

> Good catch!  Easy to see how this escaped notice for a decade, but still, 
> it'll be good to fix.
>
> LGTM with a minor improvement.

Thank John for your encouragement and guidance!
I have no write access to LLVM project, could you please commit it for me when 
you are free?

Cheers
Zhouyi Zhou
zhouzho...@gmail.com


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

https://reviews.llvm.org/D137263

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

[PATCH] D137263: add boundary check for ASTUnresolvedSet::erase

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D137263#3904184 , @MaskRay wrote:

> The summary and comments use Markdown. You comment is currently formatted 
> strangely because you did not use fenced code blocks :)
> (The commit message does not necessarily use Markdown. Since the summary 
> comes from the commit message, many just use Markdown unconditionally and 
> that is fine.)

Thank Ray for your precious advice!

Sorry for the trouble that I have made, I will learn to use fenced code blocks 
next time ( I am a beginner who eager to learn ;-))!

Sincerely yours
Zhouyi


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

https://reviews.llvm.org/D137263

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

[PATCH] D119493: Fixing surplus assert condition in EvaluateTemporary

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D119493#3403458 , @efriedma wrote:

> 

Thank Eli for your comment and guidance and sorry to have carelessly neglected 
your comment for 7 months!

> I'm not sure this is the right fix. If we were handling the pseudo-destructor 
> correctly, I would expect the following to compile successfully:
>
>   typedef bool Var;
>   constexpr bool foobool() {
>   return (bool().Var::~Var(), false); 
>   }

Yes, g++ can compile above code successfully, while we can't.
I am going to try to fix above (I am a beginner who eager to learn ;-)

Cheers
Zhouyi


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

https://reviews.llvm.org/D119493

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

[PATCH] D137263: add boundary check for ASTUnresolvedSet::erase

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

Thank Ray for editing the summary for me ;-) I learned to use fenced code 
blocks now from your example.
Thank you both!


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

https://reviews.llvm.org/D137263

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

[PATCH] D137263: add boundary check for ASTUnresolvedSet::erase

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D137263#3909722 , @MaskRay wrote:

> 

Thank Ray for your guidance!

> We need a regression test. See `clang/test/SemaCXX/using-decl*` for some 
> other using declaration tests. Study some tests, find a file which is 
> appropriate, and add a reduced case there.
> Use `ninja check-clang-semacxx` to run `clang/test/SemaCXX` tests. Use 
> `$build/bin/llvm-lit -v clang/test/SemaCXX/using-decl-1.cpp` to run one test.
>
> I find that if I comment out ` 
> cast(Shadow->getDeclContext())->removeConversion(Shadow);` in 
> `Sema::HideUsingShadowDecl`, no test fails... So we have a missing coverage 
> issue.

OK, I will try to accomplish all above. There are a lot to learn for me, so 
please wait for my completion which may take relatively a long time (maybe 
about 2 weeks).

>> clang-14 build on Ubuntu 22.04 don't trigger above assertion because 
>> clang-14 using g++ -std=c++14 by default:
>
> The instruction and the diassembly is irrelevant to this fix and can be 
> removed.

Done

Thanks again

Sincerely
Zhouyi


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

https://reviews.llvm.org/D137263

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

[PATCH] D137263: add boundary check for ASTUnresolvedSet::erase

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D137263#3909722 , @MaskRay wrote:

> I find that if I comment out ` 
> cast(Shadow->getDeclContext())->removeConversion(Shadow);` in 
> `Sema::HideUsingShadowDecl`, no test fails... So we have a missing coverage 
> issue.

Hi 
I found a counterexample for above case, excited! !  and I could not hesitate 
to report you that good news ;-)

  struct A
  {
  struct Nested {};
  operator Nested*() {return 0;};
  };
  
  struct B : A
  {
  using A::operator typename A::Nested*;
  operator typename A::Nested *() {
  struct A * thi = this;
  return *thi;
  };
  };
  
  int
  main()
  {
  
  struct B b;
  auto s = *b;
  
  }

After  comment out ` 
cast(Shadow->getDeclContext())->removeConversion(Shadow);`
clang++ report following:

  using.C:21:11: error: use of overloaded operator '*' is ambiguous (operand 
type 'struct B')
  auto s = *b;
   ^~
  using.C:21:11: note: because of ambiguity in conversion of 'struct B' to 
'A::Nested *'
  using.C:4:9: note: candidate function
  operator Nested*() {return 0;};
  ^
  using.C:10:9: note: candidate function
  operator typename A::Nested *() {
  ^
  using.C:21:11: note: built-in candidate operator*(struct A::Nested *)
  auto s = *b;
   ^
  using.C:21:11: note: built-in candidate operator*(const struct A::Nested *)
  1 error generated.

So I think we could add above to the test case.
What's your opinion?

I learned a lot under your guidance!

Thank you both
Sincerely 
Zhouyi


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

https://reviews.llvm.org/D137263

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

[PATCH] D137263: add boundary check for ASTUnresolvedSet::erase

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou updated this revision to Diff 473495.
zhouyizhou added a comment.

add a test case
thank you ;-)


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

https://reviews.llvm.org/D137263

Files:
  clang/include/clang/AST/ASTUnresolvedSet.h
  clang/test/SemaCXX/using-decl-templates.cpp


Index: clang/test/SemaCXX/using-decl-templates.cpp
===
--- clang/test/SemaCXX/using-decl-templates.cpp
+++ clang/test/SemaCXX/using-decl-templates.cpp
@@ -102,6 +102,32 @@
 };
 } // namespace DontDiagnoseInvalidTest
 
+namespace shadow_nested_operator {
+  template 
+  struct A
+  {
+struct Nested {};
+operator Nested*() {return 0;};
+  };
+
+  template 
+  struct B : A
+  {
+using A::operator typename A::Nested*;
+operator typename A::Nested *() {
+  struct A * thi = this;
+  return *thi;
+   };
+  };
+
+  int
+  foo ()
+  {
+struct B b;
+auto s = *b;
+  }
+} // namespace shadow_nested_operator
+
 namespace func_templ {
 namespace sss {
 double foo(int, double);
Index: clang/include/clang/AST/ASTUnresolvedSet.h
===
--- clang/include/clang/AST/ASTUnresolvedSet.h
+++ clang/include/clang/AST/ASTUnresolvedSet.h
@@ -69,7 +69,12 @@
 return false;
   }
 
-  void erase(unsigned I) { Decls[I] = Decls.pop_back_val(); }
+  void erase(unsigned I) {
+if (I == (Decls.size() - 1))
+  Decls.pop_back();
+else
+  Decls[I] = Decls.pop_back_val();
+  }
 
   void clear() { Decls.clear(); }
 


Index: clang/test/SemaCXX/using-decl-templates.cpp
===
--- clang/test/SemaCXX/using-decl-templates.cpp
+++ clang/test/SemaCXX/using-decl-templates.cpp
@@ -102,6 +102,32 @@
 };
 } // namespace DontDiagnoseInvalidTest
 
+namespace shadow_nested_operator {
+  template 
+  struct A
+  {
+struct Nested {};
+operator Nested*() {return 0;};
+  };
+
+  template 
+  struct B : A
+  {
+using A::operator typename A::Nested*;
+operator typename A::Nested *() {
+  struct A * thi = this;
+  return *thi;
+   };
+  };
+
+  int
+  foo ()
+  {
+struct B b;
+auto s = *b;
+  }
+} // namespace shadow_nested_operator
+
 namespace func_templ {
 namespace sss {
 double foo(int, double);
Index: clang/include/clang/AST/ASTUnresolvedSet.h
===
--- clang/include/clang/AST/ASTUnresolvedSet.h
+++ clang/include/clang/AST/ASTUnresolvedSet.h
@@ -69,7 +69,12 @@
 return false;
   }
 
-  void erase(unsigned I) { Decls[I] = Decls.pop_back_val(); }
+  void erase(unsigned I) {
+if (I == (Decls.size() - 1))
+  Decls.pop_back();
+else
+  Decls[I] = Decls.pop_back_val();
+  }
 
   void clear() { Decls.clear(); }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137263: add boundary check for ASTUnresolvedSet::erase

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou updated this revision to Diff 473509.
zhouyizhou added a comment.

Thank Ray for your guidance!
I achieved a lot in the process ;-)

Thanks
Zhouyi


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

https://reviews.llvm.org/D137263

Files:
  clang/include/clang/AST/ASTUnresolvedSet.h
  clang/test/SemaCXX/using-decl-templates.cpp


Index: clang/test/SemaCXX/using-decl-templates.cpp
===
--- clang/test/SemaCXX/using-decl-templates.cpp
+++ clang/test/SemaCXX/using-decl-templates.cpp
@@ -102,6 +102,28 @@
 };
 } // namespace DontDiagnoseInvalidTest
 
+namespace shadow_nested_operator {
+  template 
+  struct A {
+struct Nested {};
+operator Nested*() {return 0;};
+  };
+
+  template 
+  struct B : A {
+using A::operator typename A::Nested*;
+operator typename A::Nested *() {
+  struct A * thi = this;
+  return *thi;
+   };
+  };
+
+  int foo () {
+struct B b;
+auto s = *b;
+  }
+} // namespace shadow_nested_operator
+
 namespace func_templ {
 namespace sss {
 double foo(int, double);
Index: clang/include/clang/AST/ASTUnresolvedSet.h
===
--- clang/include/clang/AST/ASTUnresolvedSet.h
+++ clang/include/clang/AST/ASTUnresolvedSet.h
@@ -69,7 +69,12 @@
 return false;
   }
 
-  void erase(unsigned I) { Decls[I] = Decls.pop_back_val(); }
+  void erase(unsigned I) {
+if (I == Decls.size() - 1)
+  Decls.pop_back();
+else
+  Decls[I] = Decls.pop_back_val();
+  }
 
   void clear() { Decls.clear(); }
 


Index: clang/test/SemaCXX/using-decl-templates.cpp
===
--- clang/test/SemaCXX/using-decl-templates.cpp
+++ clang/test/SemaCXX/using-decl-templates.cpp
@@ -102,6 +102,28 @@
 };
 } // namespace DontDiagnoseInvalidTest
 
+namespace shadow_nested_operator {
+  template 
+  struct A {
+struct Nested {};
+operator Nested*() {return 0;};
+  };
+
+  template 
+  struct B : A {
+using A::operator typename A::Nested*;
+operator typename A::Nested *() {
+  struct A * thi = this;
+  return *thi;
+   };
+  };
+
+  int foo () {
+struct B b;
+auto s = *b;
+  }
+} // namespace shadow_nested_operator
+
 namespace func_templ {
 namespace sss {
 double foo(int, double);
Index: clang/include/clang/AST/ASTUnresolvedSet.h
===
--- clang/include/clang/AST/ASTUnresolvedSet.h
+++ clang/include/clang/AST/ASTUnresolvedSet.h
@@ -69,7 +69,12 @@
 return false;
   }
 
-  void erase(unsigned I) { Decls[I] = Decls.pop_back_val(); }
+  void erase(unsigned I) {
+if (I == Decls.size() - 1)
+  Decls.pop_back();
+else
+  Decls[I] = Decls.pop_back_val();
+  }
 
   void clear() { Decls.clear(); }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144157: Add 128-bit integer support to enum element

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou updated this revision to Diff 508350.
zhouyizhou added a comment.

Add 128-bit integer support to enum element like GCC extension do.
Also test coverage around _BitInt which can be arbitrarily large depending on 
the target.
Also leave room for improvements to C2X because C2x made changes in this area 
to how we calculate what type to represent the enumerations in.

Signed-off-by: Zhouyi Zhou 


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

https://reviews.llvm.org/D144157

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/AST/Interp/enums-targets.cpp
  clang/test/CXX/drs/dr3xx.cpp
  clang/test/CodeGen/enum2.c
  clang/test/Sema/enum.c

Index: clang/test/Sema/enum.c
===
--- clang/test/Sema/enum.c
+++ clang/test/Sema/enum.c
@@ -14,7 +14,7 @@
 };
 
 // minll maxull
-enum x  // expected-warning {{enumeration values exceed range of largest integer}}
+enum x  
 { y = -9223372036854775807LL-1,  // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
 z = 9223372036854775808ULL };// expected-warning {{ISO C restricts enumerator values to range of 'int'}}
 
Index: clang/test/CodeGen/enum2.c
===
--- clang/test/CodeGen/enum2.c
+++ clang/test/CodeGen/enum2.c
@@ -1,15 +1,43 @@
-// RUN: %clang_cc1 -triple i386-unknown-unknown %s -debug-info-kind=limited -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -debug-info-kind=limited -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -std=c2x -triple x86_64-linux-gnu %s -debug-info-kind=limited -emit-llvm -o - | FileCheck %s
 
 int v;
 enum e { MAX };
 
+__uint128_t v1;
+enum b {
+   b0 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL,
+};
+
+#if __STDC_VERSION__ >= 202000L
+_BitInt(256) v2;
+enum c {
+  c0 = 0x''''''''1wb
+  // TODO: check c0's debug inform under c2x
+};
+#endif
+
 void foo (void)
 {
   v = MAX;
+  // CHECK: store i32 0, ptr @v, align 4
+  v1 = b0;
+  // CHECK: store i128 24197857203266734864629346612071973665, ptr @v1, align 16
+#if __STDC_VERSION__ >= 202000L  
+  v2 = c0;
+  // TODO: check how v2's value is changed by the _BitInt enum under c2x
+#endif  
 }
+
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
 // CHECK-SAME: baseType: ![[LONG:[0-9]+]]
 // CHECK-SAME: elements: ![[ELTS:[0-9]+]]
 // CHECK: ![[LONG]] = !DIBasicType(name: "unsigned int", size: 32, encoding: DW_ATE_unsigned)
 // CHECK: ![[ELTS]] = !{![[MAX:[0-9]+]]}
 // CHECK: ![[MAX]] = !DIEnumerator(name: "MAX", value: 0)
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
+// CHECK-SAME: baseType: ![[BTYPE:[0-9]+]]
+// CHECK-SAME: elements: ![[ELTS:[0-9]+]]
+// CHECK: ![[BTYPE]] = !DIBasicType(name: "unsigned __int128", size: 128, encoding: DW_ATE_unsigned)
+// CHECK: ![[ELTS]] = !{![[E:[0-9]+]]}
+// CHECK: ![[E]] = !DIEnumerator(name: "b0", value: 24197857203266734864629346612071973665, isUnsigned: true)
Index: clang/test/CXX/drs/dr3xx.cpp
===
--- clang/test/CXX/drs/dr3xx.cpp
+++ clang/test/CXX/drs/dr3xx.cpp
@@ -1079,8 +1079,8 @@
 
 namespace dr377 { // dr377: yes
   enum E { // expected-error {{enumeration values exceed range of largest integer}}
-a = -__LONG_LONG_MAX__ - 1, // expected-error 0-1{{extension}}
-b = 2 * (unsigned long long)__LONG_LONG_MAX__ // expected-error 0-2{{extension}}
+a = -((__int128_t)__LONG_LONG_MAX__ << 64|__LONG_LONG_MAX__) - 1, // expected-error 0-2{{extension}}
+b = 2 * ((__uint128_t)__LONG_LONG_MAX__ << 64|__LONG_LONG_MAX__) // expected-error 0-2{{extension}}
   };
 }
 
Index: clang/test/AST/Interp/enums-targets.cpp
===
--- clang/test/AST/Interp/enums-targets.cpp
+++ clang/test/AST/Interp/enums-targets.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -triple i686-pc-linux -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -triple i686-pc-linux -verify %s
-// RUN: %clang_cc1 -triple x86_64-pc-linux -fexperimental-new-constant-interpreter -verify=warn %s
-// RUN: %clang_cc1 -triple x86_64-pc-linux -verify=warn %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux -fexperimental-new-constant-interpreter -verify %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux -verify %s
 // RUN: %clang_cc1 -triple x86_64-windows-msvc -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -triple x86_64-windows-msvc -verify %s
 // RUN: %clang_cc1 -triple hexagon -fexperimental-new-constant-interpreter -verify %s
@@ -11,7 +11,7 @@
 
 /// This test is split out from the rest since the output is target dependent.
 
-enum E { // warn-warning {{enumeration values exceed range of largest integer}}
+enum E {
   E1 = -__LONG_MAX__ -1L,
   E2 = __LONG_MAX__ *2UL+1UL
 };
Index: clang/lib/Sema/SemaDecl.cpp
==

[PATCH] D144157: Add 128-bit integer support to enum element

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou created this revision.
zhouyizhou added reviewers: aaron.ballman, doug.gregor, eddy-geek, rjmccall, 
MaskRay, ChuanqiXu, pengfei.
Herald added a project: All.
zhouyizhou requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add 128-bit integer support to enum element like GCC extension do.

Also remove comment: "TODO" from the code.  This comment is added by 
4ef40013d75ca ("Implement capturing of enum values and chaining of enums 
together"), while 6791a0d43b681 ("Improve handling of enumerator values for C 
and C++") has accomplished that "TODO".

I don't have write access to LLVM

Signed-off-by: Zhouyi Zhou 

Thanks a lot
Zhouyi


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144157

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/enum.c


Index: clang/test/Sema/enum.c
===
--- clang/test/Sema/enum.c
+++ clang/test/Sema/enum.c
@@ -1,7 +1,12 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple %s -fsyntax-only -verify 
-pedantic
+enum b {
+   b0 = (__uint128_t)-1, // expected-warning {{ISO C restricts enumerator 
values to range of 'int'}}
+   b1 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL, // 
expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+};
+
 enum e {A,
-B = 42LL << 32,// expected-warning {{ISO C restricts 
enumerator values to range of 'int'}}
-  C = -4, D = 12456 };
+   B = 42LL << 32,// expected-warning {{ISO C restricts enumerator 
values to range of 'int'}}
+   C = -4, D = 12456 };
 
 enum f { a = -2147483648, b = 2147483647 }; // ok.
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -19568,9 +19568,6 @@
 return;
   }
 
-  // TODO: If the result value doesn't fit in an int, it must be a long or long
-  // long value.  ISO C does not support this, but GCC does as an extension,
-  // emit a warning.
   unsigned IntWidth = Context.getTargetInfo().getIntWidth();
   unsigned CharWidth = Context.getTargetInfo().getCharWidth();
   unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
@@ -19690,14 +19687,20 @@
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
? Context.UnsignedLongTy : Context.LongTy;
-} else {
+} else if (NumPositiveBits <= Context.getTargetInfo().getLongLongWidth()) {
   BestWidth = Context.getTargetInfo().getLongLongWidth();
-  assert(NumPositiveBits <= BestWidth &&
- "How could an initializer get larger than ULL?");
   BestType = Context.UnsignedLongLongTy;
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
? Context.UnsignedLongLongTy : Context.LongLongTy;
+} else {
+  BestWidth = 128;
+  assert(NumPositiveBits <= BestWidth &&
+ "How could an initializer get larger than unsigned 128-bit 
integer type?");
+  BestType = Context.UnsignedInt128Ty;
+  BestPromotionType
+= (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
+   ? Context.UnsignedInt128Ty : Context.Int128Ty;
 }
   }
 


Index: clang/test/Sema/enum.c
===
--- clang/test/Sema/enum.c
+++ clang/test/Sema/enum.c
@@ -1,7 +1,12 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple %s -fsyntax-only -verify -pedantic
+enum b {
+   b0 = (__uint128_t)-1, // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+   b1 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL, // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+};
+
 enum e {A,
-B = 42LL << 32,// expected-warning {{ISO C restricts enumerator values to range of 'int'}}
-  C = -4, D = 12456 };
+   B = 42LL << 32,// expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+   C = -4, D = 12456 };
 
 enum f { a = -2147483648, b = 2147483647 }; // ok.
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -19568,9 +19568,6 @@
 return;
   }
 
-  // TODO: If the result value doesn't fit in an int, it must be a long or long
-  // long value.  ISO C does not support this, but GCC does as an extension,
-  // emit a warning.
   unsigned IntWidth = Context.getTargetInfo().getIntWidth();
   unsigned CharWidth = Context.getTargetInfo().getCharWidth();
   unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
@@ -19690,14 +19687,20 @@
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
? Context.UnsignedLongTy : Context.LongTy;
-   

[PATCH] D144157: Add 128-bit integer support to enum element

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou updated this revision to Diff 497874.
zhouyizhou added a comment.

add assertion for int128 support


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

https://reviews.llvm.org/D144157

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/enum.c


Index: clang/test/Sema/enum.c
===
--- clang/test/Sema/enum.c
+++ clang/test/Sema/enum.c
@@ -1,7 +1,12 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple %s -fsyntax-only -verify 
-pedantic
+enum b {
+   b0 = (__uint128_t)-1, // expected-warning {{ISO C restricts enumerator 
values to range of 'int'}}
+   b1 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL, // 
expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+};
+
 enum e {A,
-B = 42LL << 32,// expected-warning {{ISO C restricts 
enumerator values to range of 'int'}}
-  C = -4, D = 12456 };
+   B = 42LL << 32,// expected-warning {{ISO C restricts enumerator 
values to range of 'int'}}
+   C = -4, D = 12456 };
 
 enum f { a = -2147483648, b = 2147483647 }; // ok.
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -19568,9 +19568,6 @@
 return;
   }
 
-  // TODO: If the result value doesn't fit in an int, it must be a long or long
-  // long value.  ISO C does not support this, but GCC does as an extension,
-  // emit a warning.
   unsigned IntWidth = Context.getTargetInfo().getIntWidth();
   unsigned CharWidth = Context.getTargetInfo().getCharWidth();
   unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
@@ -19690,14 +19687,23 @@
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
? Context.UnsignedLongTy : Context.LongTy;
-} else {
+} else if (NumPositiveBits <= Context.getTargetInfo().getLongLongWidth()) {
   BestWidth = Context.getTargetInfo().getLongLongWidth();
-  assert(NumPositiveBits <= BestWidth &&
- "How could an initializer get larger than ULL?");
   BestType = Context.UnsignedLongLongTy;
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
? Context.UnsignedLongLongTy : Context.LongLongTy;
+} else {
+  BestWidth = 128;
+  assert(NumPositiveBits <= BestWidth &&
+"How could an initializer get larger than 128-bit?");
+  assert(Context.getTargetInfo().hasInt128Type() &&
+"How could an initializer get larger than ULL "
+"in target without 128-bit interger type support?");
+  BestType = Context.UnsignedInt128Ty;
+  BestPromotionType 
+= (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
+   ? Context.UnsignedInt128Ty : Context.Int128Ty;
 }
   }
 


Index: clang/test/Sema/enum.c
===
--- clang/test/Sema/enum.c
+++ clang/test/Sema/enum.c
@@ -1,7 +1,12 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple %s -fsyntax-only -verify -pedantic
+enum b {
+   b0 = (__uint128_t)-1, // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+   b1 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL, // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+};
+
 enum e {A,
-B = 42LL << 32,// expected-warning {{ISO C restricts enumerator values to range of 'int'}}
-  C = -4, D = 12456 };
+   B = 42LL << 32,// expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+   C = -4, D = 12456 };
 
 enum f { a = -2147483648, b = 2147483647 }; // ok.
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -19568,9 +19568,6 @@
 return;
   }
 
-  // TODO: If the result value doesn't fit in an int, it must be a long or long
-  // long value.  ISO C does not support this, but GCC does as an extension,
-  // emit a warning.
   unsigned IntWidth = Context.getTargetInfo().getIntWidth();
   unsigned CharWidth = Context.getTargetInfo().getCharWidth();
   unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
@@ -19690,14 +19687,23 @@
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
? Context.UnsignedLongTy : Context.LongTy;
-} else {
+} else if (NumPositiveBits <= Context.getTargetInfo().getLongLongWidth()) {
   BestWidth = Context.getTargetInfo().getLongLongWidth();
-  assert(NumPositiveBits <= BestWidth &&
- "How could an initializer get larger than ULL?");
   BestType = Context.UnsignedLongLongTy;
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)

[PATCH] D144157: Add 128-bit integer support to enum element

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou updated this revision to Diff 497884.
zhouyizhou added a comment.

debian build bot won't me pass,  so I clang-formatted the function ActOnEnumBody


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

https://reviews.llvm.org/D144157

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/enum.c

Index: clang/test/Sema/enum.c
===
--- clang/test/Sema/enum.c
+++ clang/test/Sema/enum.c
@@ -1,4 +1,9 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple %s -fsyntax-only -verify -pedantic
+enum b {
+   b0 = (__uint128_t)-1, // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+   b1 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL, // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+};
+
 enum e {A,
 B = 42LL << 32,// expected-warning {{ISO C restricts enumerator values to range of 'int'}}
   C = -4, D = 12456 };
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -19557,9 +19557,9 @@
 
   if (Enum->isDependentType()) {
 for (unsigned i = 0, e = Elements.size(); i != e; ++i) {
-  EnumConstantDecl *ECD =
-cast_or_null(Elements[i]);
-  if (!ECD) continue;
+  EnumConstantDecl *ECD = cast_or_null(Elements[i]);
+  if (!ECD)
+continue;
 
   ECD->setType(EnumType);
 }
@@ -19568,9 +19568,6 @@
 return;
   }
 
-  // TODO: If the result value doesn't fit in an int, it must be a long or long
-  // long value.  ISO C does not support this, but GCC does as an extension,
-  // emit a warning.
   unsigned IntWidth = Context.getTargetInfo().getIntWidth();
   unsigned CharWidth = Context.getTargetInfo().getCharWidth();
   unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
@@ -19581,9 +19578,9 @@
   unsigned NumPositiveBits = 0;
 
   for (unsigned i = 0, e = Elements.size(); i != e; ++i) {
-EnumConstantDecl *ECD =
-  cast_or_null(Elements[i]);
-if (!ECD) continue;  // Already issued a diagnostic.
+EnumConstantDecl *ECD = cast_or_null(Elements[i]);
+if (!ECD)
+  continue; // Already issued a diagnostic.
 
 const llvm::APSInt &InitVal = ECD->getInitVal();
 
@@ -19594,8 +19591,8 @@
   unsigned ActiveBits = InitVal.getActiveBits();
   NumPositiveBits = std::max({NumPositiveBits, ActiveBits, 1u});
 } else {
-  NumNegativeBits = std::max(NumNegativeBits,
- (unsigned)InitVal.getMinSignedBits());
+  NumNegativeBits =
+  std::max(NumNegativeBits, (unsigned)InitVal.getMinSignedBits());
 }
   }
 
@@ -19637,8 +19634,7 @@
   BestPromotionType = BestType;
 
 BestWidth = Context.getIntWidth(BestType);
-  }
-  else if (NumNegativeBits) {
+  } else if (NumNegativeBits) {
 // If there is a negative value, figure out the smallest integer type (of
 // int/long/longlong) that fits.
 // If it's packed, check also if it fits a char or a short.
@@ -19681,23 +19677,36 @@
 } else if (NumPositiveBits <= IntWidth) {
   BestType = Context.UnsignedIntTy;
   BestWidth = IntWidth;
-  BestPromotionType
-= (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
-   ? Context.UnsignedIntTy : Context.IntTy;
+  BestPromotionType =
+  (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
+  ? Context.UnsignedIntTy
+  : Context.IntTy;
 } else if (NumPositiveBits <=
(BestWidth = Context.getTargetInfo().getLongWidth())) {
   BestType = Context.UnsignedLongTy;
-  BestPromotionType
-= (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
-   ? Context.UnsignedLongTy : Context.LongTy;
-} else {
+  BestPromotionType =
+  (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
+  ? Context.UnsignedLongTy
+  : Context.LongTy;
+} else if (NumPositiveBits <= Context.getTargetInfo().getLongLongWidth()) {
   BestWidth = Context.getTargetInfo().getLongLongWidth();
-  assert(NumPositiveBits <= BestWidth &&
- "How could an initializer get larger than ULL?");
   BestType = Context.UnsignedLongLongTy;
-  BestPromotionType
-= (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
-   ? Context.UnsignedLongLongTy : Context.LongLongTy;
+  BestPromotionType =
+  (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
+  ? Context.UnsignedLongLongTy
+  : Context.LongLongTy;
+} else {
+  BestWidth = 128;
+  assert(NumPositiveBits <= BestWidth &&
+ "How could an initializer get larger than 128-bit?");
+  assert(Context.getTargetInfo().hasInt128Type() &&
+ "How could an initializer get larger than UL

[PATCH] D144157: Add 128-bit integer support to enum element

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D144157#4131103 , @ChuanqiXu wrote:

>> debian build bot won't let me pass, so I clang-formatted the function 
>> ActOnEnumBody
>
> You can try `git diff -U0 --no-color --relative HEAD^ | 
> clang/tools/clang-format/clang-format-diff.py -p1 -i` to format the changed 
> part only. Currently there are many untouched part changed due to the format. 
> This is not good.

Thank you for your valuable advice, the command is fantastic indeed. I will 
resend the patch immediately.  
Thanks again
Cheers Zhouyi


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

https://reviews.llvm.org/D144157

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

[PATCH] D144157: Add 128-bit integer support to enum element

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou updated this revision to Diff 497889.
zhouyizhou added a comment.

reformat the patch according to Chuanqi's Advice


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

https://reviews.llvm.org/D144157

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/enum.c


Index: clang/test/Sema/enum.c
===
--- clang/test/Sema/enum.c
+++ clang/test/Sema/enum.c
@@ -1,4 +1,9 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple %s -fsyntax-only -verify 
-pedantic
+enum b {
+   b0 = (__uint128_t)-1, // expected-warning {{ISO C restricts enumerator 
values to range of 'int'}}
+   b1 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL, // 
expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+};
+
 enum e {A,
 B = 42LL << 32,// expected-warning {{ISO C restricts 
enumerator values to range of 'int'}}
   C = -4, D = 12456 };
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -19690,14 +19690,24 @@
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
? Context.UnsignedLongTy : Context.LongTy;
-} else {
+} else if (NumPositiveBits <= Context.getTargetInfo().getLongLongWidth()) {
   BestWidth = Context.getTargetInfo().getLongLongWidth();
-  assert(NumPositiveBits <= BestWidth &&
- "How could an initializer get larger than ULL?");
   BestType = Context.UnsignedLongLongTy;
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
? Context.UnsignedLongLongTy : Context.LongLongTy;
+} else {
+  BestWidth = 128;
+  assert(NumPositiveBits <= BestWidth &&
+ "How could an initializer get larger than 128-bit?");
+  assert(Context.getTargetInfo().hasInt128Type() &&
+ "How could an initializer get larger than ULL in target without "
+ "128-bit interger type support?");
+  BestType = Context.UnsignedInt128Ty;
+  BestPromotionType =
+  (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
+  ? Context.UnsignedInt128Ty
+  : Context.Int128Ty;
 }
   }
 


Index: clang/test/Sema/enum.c
===
--- clang/test/Sema/enum.c
+++ clang/test/Sema/enum.c
@@ -1,4 +1,9 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple %s -fsyntax-only -verify -pedantic
+enum b {
+   b0 = (__uint128_t)-1, // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+   b1 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL, // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+};
+
 enum e {A,
 B = 42LL << 32,// expected-warning {{ISO C restricts enumerator values to range of 'int'}}
   C = -4, D = 12456 };
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -19690,14 +19690,24 @@
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
? Context.UnsignedLongTy : Context.LongTy;
-} else {
+} else if (NumPositiveBits <= Context.getTargetInfo().getLongLongWidth()) {
   BestWidth = Context.getTargetInfo().getLongLongWidth();
-  assert(NumPositiveBits <= BestWidth &&
- "How could an initializer get larger than ULL?");
   BestType = Context.UnsignedLongLongTy;
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
? Context.UnsignedLongLongTy : Context.LongLongTy;
+} else {
+  BestWidth = 128;
+  assert(NumPositiveBits <= BestWidth &&
+ "How could an initializer get larger than 128-bit?");
+  assert(Context.getTargetInfo().hasInt128Type() &&
+ "How could an initializer get larger than ULL in target without "
+ "128-bit interger type support?");
+  BestType = Context.UnsignedInt128Ty;
+  BestPromotionType =
+  (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
+  ? Context.UnsignedInt128Ty
+  : Context.Int128Ty;
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144157: Add 128-bit integer support to enum element

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou updated this revision to Diff 497893.

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

https://reviews.llvm.org/D144157

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/enum.c


Index: clang/test/Sema/enum.c
===
--- clang/test/Sema/enum.c
+++ clang/test/Sema/enum.c
@@ -1,4 +1,9 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple %s -fsyntax-only -verify 
-pedantic
+enum b {
+   b0 = (__uint128_t)-1, // expected-warning {{ISO C restricts enumerator 
values to range of 'int'}}
+   b1 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL, // 
expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+};
+
 enum e {A,
 B = 42LL << 32,// expected-warning {{ISO C restricts 
enumerator values to range of 'int'}}
   C = -4, D = 12456 };
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -19568,9 +19568,6 @@
 return;
   }
 
-  // TODO: If the result value doesn't fit in an int, it must be a long or long
-  // long value.  ISO C does not support this, but GCC does as an extension,
-  // emit a warning.
   unsigned IntWidth = Context.getTargetInfo().getIntWidth();
   unsigned CharWidth = Context.getTargetInfo().getCharWidth();
   unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
@@ -19690,14 +19687,24 @@
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
? Context.UnsignedLongTy : Context.LongTy;
-} else {
+} else if (NumPositiveBits <= Context.getTargetInfo().getLongLongWidth()) {
   BestWidth = Context.getTargetInfo().getLongLongWidth();
-  assert(NumPositiveBits <= BestWidth &&
- "How could an initializer get larger than ULL?");
   BestType = Context.UnsignedLongLongTy;
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
? Context.UnsignedLongLongTy : Context.LongLongTy;
+} else {
+  BestWidth = 128;
+  assert(NumPositiveBits <= BestWidth &&
+ "How could an initializer get larger than 128-bit?");
+  assert(Context.getTargetInfo().hasInt128Type() &&
+ "How could an initializer get larger than ULL in target without "
+ "128-bit interger type support?");
+  BestType = Context.UnsignedInt128Ty;
+  BestPromotionType =
+  (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
+  ? Context.UnsignedInt128Ty
+  : Context.Int128Ty;
 }
   }
 


Index: clang/test/Sema/enum.c
===
--- clang/test/Sema/enum.c
+++ clang/test/Sema/enum.c
@@ -1,4 +1,9 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple %s -fsyntax-only -verify -pedantic
+enum b {
+   b0 = (__uint128_t)-1, // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+   b1 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL, // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+};
+
 enum e {A,
 B = 42LL << 32,// expected-warning {{ISO C restricts enumerator values to range of 'int'}}
   C = -4, D = 12456 };
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -19568,9 +19568,6 @@
 return;
   }
 
-  // TODO: If the result value doesn't fit in an int, it must be a long or long
-  // long value.  ISO C does not support this, but GCC does as an extension,
-  // emit a warning.
   unsigned IntWidth = Context.getTargetInfo().getIntWidth();
   unsigned CharWidth = Context.getTargetInfo().getCharWidth();
   unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
@@ -19690,14 +19687,24 @@
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
? Context.UnsignedLongTy : Context.LongTy;
-} else {
+} else if (NumPositiveBits <= Context.getTargetInfo().getLongLongWidth()) {
   BestWidth = Context.getTargetInfo().getLongLongWidth();
-  assert(NumPositiveBits <= BestWidth &&
- "How could an initializer get larger than ULL?");
   BestType = Context.UnsignedLongLongTy;
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
? Context.UnsignedLongLongTy : Context.LongLongTy;
+} else {
+  BestWidth = 128;
+  assert(NumPositiveBits <= BestWidth &&
+ "How could an initializer get larger than 128-bit?");
+  assert(Context.getTargetInfo().hasInt128Type() &&
+ "How could an initializer get larger than ULL in target without "
+ "128-bit interger type support?");
+  Bes

[PATCH] D144157: Add 128-bit integer support to enum element

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou updated this revision to Diff 498520.
zhouyizhou added a comment.

Add CodeGen Test following MaskRay(Fangrui Song)'s Advice, I learned a lot 
during this process, to be honest ;-)


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

https://reviews.llvm.org/D144157

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGen/enum3.c
  clang/test/Sema/enum.c


Index: clang/test/Sema/enum.c
===
--- clang/test/Sema/enum.c
+++ clang/test/Sema/enum.c
@@ -1,4 +1,9 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple %s -fsyntax-only -verify 
-pedantic
+enum b {
+   b0 = (__uint128_t)-1, // expected-warning {{ISO C restricts enumerator 
values to range of 'int'}}
+   b1 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL, // 
expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+};
+
 enum e {A,
 B = 42LL << 32,// expected-warning {{ISO C restricts 
enumerator values to range of 'int'}}
   C = -4, D = 12456 };
Index: clang/test/CodeGen/enum3.c
===
--- /dev/null
+++ clang/test/CodeGen/enum3.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -debug-info-kind=limited 
-emit-llvm -o - | FileCheck %s
+
+enum b {
+   b0 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL,
+};
+__uint128_t v;
+
+void foo (void)
+{
+  v = b0;
+}
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
+// CHECK-SAME: baseType: ![[BTYPE:[0-9]+]]
+// CHECK-SAME: elements: ![[ELTS:[0-9]+]]
+// CHECK: ![[BTYPE]] = !DIBasicType(name: "unsigned __int128", size: 128, 
encoding: DW_ATE_unsigned)
+// CHECK: ![[ELTS]] = !{![[E:[0-9]+]]}
+// CHECK: ![[E]] = !DIEnumerator(name: "b0", value: 
24197857203266734864629346612071973665, isUnsigned: true)
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -19568,9 +19568,6 @@
 return;
   }
 
-  // TODO: If the result value doesn't fit in an int, it must be a long or long
-  // long value.  ISO C does not support this, but GCC does as an extension,
-  // emit a warning.
   unsigned IntWidth = Context.getTargetInfo().getIntWidth();
   unsigned CharWidth = Context.getTargetInfo().getCharWidth();
   unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
@@ -19690,14 +19687,24 @@
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
? Context.UnsignedLongTy : Context.LongTy;
-} else {
+} else if (NumPositiveBits <= Context.getTargetInfo().getLongLongWidth()) {
   BestWidth = Context.getTargetInfo().getLongLongWidth();
-  assert(NumPositiveBits <= BestWidth &&
- "How could an initializer get larger than ULL?");
   BestType = Context.UnsignedLongLongTy;
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
? Context.UnsignedLongLongTy : Context.LongLongTy;
+} else {
+  BestWidth = 128;
+  assert(NumPositiveBits <= BestWidth &&
+ "How could an initializer get larger than 128-bit?");
+  assert(Context.getTargetInfo().hasInt128Type() &&
+ "How could an initializer get larger than ULL in target without "
+ "128-bit interger type support?");
+  BestType = Context.UnsignedInt128Ty;
+  BestPromotionType =
+  (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
+  ? Context.UnsignedInt128Ty
+  : Context.Int128Ty;
 }
   }
 


Index: clang/test/Sema/enum.c
===
--- clang/test/Sema/enum.c
+++ clang/test/Sema/enum.c
@@ -1,4 +1,9 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple %s -fsyntax-only -verify -pedantic
+enum b {
+   b0 = (__uint128_t)-1, // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+   b1 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL, // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+};
+
 enum e {A,
 B = 42LL << 32,// expected-warning {{ISO C restricts enumerator values to range of 'int'}}
   C = -4, D = 12456 };
Index: clang/test/CodeGen/enum3.c
===
--- /dev/null
+++ clang/test/CodeGen/enum3.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -debug-info-kind=limited -emit-llvm -o - | FileCheck %s
+
+enum b {
+   b0 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL,
+};
+__uint128_t v;
+
+void foo (void)
+{
+  v = b0;
+}
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
+// CHECK-SAME: baseType: ![[BTYPE:[0-9]+]]
+// CHECK-SAME: elements: ![[ELTS:[0-9]+]]
+// CHECK: ![[BTYPE]] = !DIBasicType(name: "unsigned __int128", size: 128, 

[PATCH] D144157: Add 128-bit integer support to enum element

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou updated this revision to Diff 498557.
zhouyizhou added a comment.

make test case even better according to MaskRay's guidance


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

https://reviews.llvm.org/D144157

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGen/enum2.c
  clang/test/Sema/enum.c


Index: clang/test/Sema/enum.c
===
--- clang/test/Sema/enum.c
+++ clang/test/Sema/enum.c
@@ -1,4 +1,9 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple %s -fsyntax-only -verify 
-pedantic
+enum b {
+   b0 = (__uint128_t)-1, // expected-warning {{ISO C restricts enumerator 
values to range of 'int'}}
+   b1 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL, // 
expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+};
+
 enum e {A,
 B = 42LL << 32,// expected-warning {{ISO C restricts 
enumerator values to range of 'int'}}
   C = -4, D = 12456 };
Index: clang/test/CodeGen/enum2.c
===
--- clang/test/CodeGen/enum2.c
+++ clang/test/CodeGen/enum2.c
@@ -1,15 +1,30 @@
-// RUN: %clang_cc1 -triple i386-unknown-unknown %s -debug-info-kind=limited 
-emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -debug-info-kind=limited 
-emit-llvm -o - | FileCheck %s
 
 int v;
 enum e { MAX };
+enum b {
+   b0 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL,
+};
+__uint128_t v1;
 
 void foo (void)
 {
   v = MAX;
+  // CHECK: store i32 0, ptr @v, align 4
+  v1 = b0;
+  // CHECK: store i128 24197857203266734864629346612071973665, ptr @v1, align 
16
 }
+
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
 // CHECK-SAME: baseType: ![[LONG:[0-9]+]]
 // CHECK-SAME: elements: ![[ELTS:[0-9]+]]
 // CHECK: ![[LONG]] = !DIBasicType(name: "unsigned int", size: 32, encoding: 
DW_ATE_unsigned)
 // CHECK: ![[ELTS]] = !{![[MAX:[0-9]+]]}
 // CHECK: ![[MAX]] = !DIEnumerator(name: "MAX", value: 0)
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
+// CHECK-SAME: baseType: ![[BTYPE:[0-9]+]]
+// CHECK-SAME: elements: ![[ELTS:[0-9]+]]
+// CHECK: ![[BTYPE]] = !DIBasicType(name: "unsigned __int128", size: 128, 
encoding: DW_ATE_unsigned)
+// CHECK: ![[ELTS]] = !{![[E:[0-9]+]]}
+// CHECK: ![[E]] = !DIEnumerator(name: "b0", value: 
24197857203266734864629346612071973665, isUnsigned: true)
+
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -19568,9 +19568,6 @@
 return;
   }
 
-  // TODO: If the result value doesn't fit in an int, it must be a long or long
-  // long value.  ISO C does not support this, but GCC does as an extension,
-  // emit a warning.
   unsigned IntWidth = Context.getTargetInfo().getIntWidth();
   unsigned CharWidth = Context.getTargetInfo().getCharWidth();
   unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
@@ -19690,14 +19687,24 @@
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
? Context.UnsignedLongTy : Context.LongTy;
-} else {
+} else if (NumPositiveBits <= Context.getTargetInfo().getLongLongWidth()) {
   BestWidth = Context.getTargetInfo().getLongLongWidth();
-  assert(NumPositiveBits <= BestWidth &&
- "How could an initializer get larger than ULL?");
   BestType = Context.UnsignedLongLongTy;
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
? Context.UnsignedLongLongTy : Context.LongLongTy;
+} else {
+  BestWidth = 128;
+  assert(NumPositiveBits <= BestWidth &&
+ "How could an initializer get larger than 128-bit?");
+  assert(Context.getTargetInfo().hasInt128Type() &&
+ "How could an initializer get larger than ULL in target without "
+ "128-bit interger type support?");
+  BestType = Context.UnsignedInt128Ty;
+  BestPromotionType =
+  (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
+  ? Context.UnsignedInt128Ty
+  : Context.Int128Ty;
 }
   }
 


Index: clang/test/Sema/enum.c
===
--- clang/test/Sema/enum.c
+++ clang/test/Sema/enum.c
@@ -1,4 +1,9 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple %s -fsyntax-only -verify -pedantic
+enum b {
+   b0 = (__uint128_t)-1, // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+   b1 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL, // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+};
+
 enum e {A,
 B = 42LL << 32,// expected-warning {{ISO C restricts enumerator values to range of 'int'}}
   C = -4, D = 12456 };
Index: clang/test/CodeGen/enum2.c
==

[PATCH] D144157: Add 128-bit integer support to enum element

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou updated this revision to Diff 498571.
zhouyizhou added a comment.

delete the trailing blank line

Thank you all ;-)

I have no access write to LLVM, could you commit it for me when you are not 
busy?

Thanks a lot
Cheers
Zhouyi

Zhouyi Zhou 


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

https://reviews.llvm.org/D144157

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGen/enum2.c
  clang/test/Sema/enum.c


Index: clang/test/Sema/enum.c
===
--- clang/test/Sema/enum.c
+++ clang/test/Sema/enum.c
@@ -1,4 +1,9 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple %s -fsyntax-only -verify 
-pedantic
+enum b {
+   b0 = (__uint128_t)-1, // expected-warning {{ISO C restricts enumerator 
values to range of 'int'}}
+   b1 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL, // 
expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+};
+
 enum e {A,
 B = 42LL << 32,// expected-warning {{ISO C restricts 
enumerator values to range of 'int'}}
   C = -4, D = 12456 };
Index: clang/test/CodeGen/enum2.c
===
--- clang/test/CodeGen/enum2.c
+++ clang/test/CodeGen/enum2.c
@@ -1,15 +1,29 @@
-// RUN: %clang_cc1 -triple i386-unknown-unknown %s -debug-info-kind=limited 
-emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -debug-info-kind=limited 
-emit-llvm -o - | FileCheck %s
 
 int v;
 enum e { MAX };
+enum b {
+   b0 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL,
+};
+__uint128_t v1;
 
 void foo (void)
 {
   v = MAX;
+  // CHECK: store i32 0, ptr @v, align 4
+  v1 = b0;
+  // CHECK: store i128 24197857203266734864629346612071973665, ptr @v1, align 
16
 }
+
 // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
 // CHECK-SAME: baseType: ![[LONG:[0-9]+]]
 // CHECK-SAME: elements: ![[ELTS:[0-9]+]]
 // CHECK: ![[LONG]] = !DIBasicType(name: "unsigned int", size: 32, encoding: 
DW_ATE_unsigned)
 // CHECK: ![[ELTS]] = !{![[MAX:[0-9]+]]}
 // CHECK: ![[MAX]] = !DIEnumerator(name: "MAX", value: 0)
+// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
+// CHECK-SAME: baseType: ![[BTYPE:[0-9]+]]
+// CHECK-SAME: elements: ![[ELTS:[0-9]+]]
+// CHECK: ![[BTYPE]] = !DIBasicType(name: "unsigned __int128", size: 128, 
encoding: DW_ATE_unsigned)
+// CHECK: ![[ELTS]] = !{![[E:[0-9]+]]}
+// CHECK: ![[E]] = !DIEnumerator(name: "b0", value: 
24197857203266734864629346612071973665, isUnsigned: true)
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -19568,9 +19568,6 @@
 return;
   }
 
-  // TODO: If the result value doesn't fit in an int, it must be a long or long
-  // long value.  ISO C does not support this, but GCC does as an extension,
-  // emit a warning.
   unsigned IntWidth = Context.getTargetInfo().getIntWidth();
   unsigned CharWidth = Context.getTargetInfo().getCharWidth();
   unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
@@ -19690,14 +19687,24 @@
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
? Context.UnsignedLongTy : Context.LongTy;
-} else {
+} else if (NumPositiveBits <= Context.getTargetInfo().getLongLongWidth()) {
   BestWidth = Context.getTargetInfo().getLongLongWidth();
-  assert(NumPositiveBits <= BestWidth &&
- "How could an initializer get larger than ULL?");
   BestType = Context.UnsignedLongLongTy;
   BestPromotionType
 = (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
? Context.UnsignedLongLongTy : Context.LongLongTy;
+} else {
+  BestWidth = 128;
+  assert(NumPositiveBits <= BestWidth &&
+ "How could an initializer get larger than 128-bit?");
+  assert(Context.getTargetInfo().hasInt128Type() &&
+ "How could an initializer get larger than ULL in target without "
+ "128-bit interger type support?");
+  BestType = Context.UnsignedInt128Ty;
+  BestPromotionType =
+  (NumPositiveBits == BestWidth || !getLangOpts().CPlusPlus)
+  ? Context.UnsignedInt128Ty
+  : Context.Int128Ty;
 }
   }
 


Index: clang/test/Sema/enum.c
===
--- clang/test/Sema/enum.c
+++ clang/test/Sema/enum.c
@@ -1,4 +1,9 @@
 // RUN: %clang_cc1 -triple %itanium_abi_triple %s -fsyntax-only -verify -pedantic
+enum b {
+   b0 = (__uint128_t)-1, // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+   b1 = (__uint128_t)0x123456789abcdef0ULL << 64|0x0fedcba987654321ULL, // expected-warning {{ISO C restricts enumerator values to range of 'int'}}
+};
+
 enum e {A,
 B = 42LL << 32,// expected-warning {{ISO C restricts

[PATCH] D144157: Add 128-bit integer support to enum element

[zhouyizhou via Phabricator via cfe-commits]

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

Thank you all for your guidance!

Now I am starting to learn C2X N2997, this is a fruitful process of learning 
for me ;-)

Cheers
Zhouyi




Comment at: clang/lib/Sema/SemaDecl.cpp:19571-19573
-  // TODO: If the result value doesn't fit in an int, it must be a long or long
-  // long value.  ISO C does not support this, but GCC does as an extension,
-  // emit a warning.

h-vetinari wrote:
> That comment is still relevant AFAICT, at least partially (the ISO C 
> restrictions are still there, GCC still extends it)
> That comment is still relevant AFAICT, at least partially (the ISO C 
> restrictions are still there, GCC still extends it)

Thank for reviewing the patch ;-)

I think clang (with -pedantic) has emit the warning in function 
Sema::CheckEnumConstant, which behave very similar to what GCC (with -pedantic) 
do.

Cheers
Zhouyi


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

https://reviews.llvm.org/D144157

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

[PATCH] D149709: Fix a typo in head comment of CurPPLexer

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou created this revision.
zhouyizhou added reviewers: vsapsai, zequanwu, ChuanqiXu, MaskRay, pengfei.
Herald added a project: All.
zhouyizhou requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In head comment of CurPPLexer field of class Preprocessor, 
'The current top of the stack what we're lexing from' should be
'The current top of the stack that we're lexing from'

Signed-off-by: Zhouyi Zhou 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149709

Files:
  clang/include/clang/Lex/Preprocessor.h


Index: clang/include/clang/Lex/Preprocessor.h
===
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -729,7 +729,7 @@
   /// Only one of CurLexer, or CurTokenLexer will be non-null.
   std::unique_ptr CurLexer;
 
-  /// The current top of the stack what we're lexing from
+  /// The current top of the stack that we're lexing from
   /// if not expanding a macro.
   ///
   /// This is an alias for CurLexer.


Index: clang/include/clang/Lex/Preprocessor.h
===
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -729,7 +729,7 @@
   /// Only one of CurLexer, or CurTokenLexer will be non-null.
   std::unique_ptr CurLexer;
 
-  /// The current top of the stack what we're lexing from
+  /// The current top of the stack that we're lexing from
   /// if not expanding a macro.
   ///
   /// This is an alias for CurLexer.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149709: Fix a typo in head comment of CurPPLexer

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D149709#4314390 , @vsapsai wrote:

> Thanks for the fix! Don't see any other places in the file to fix.

Thanks for review my patch ;-)
I have no write access to LLVM project,  can you commit it for me ?

Thanks in Advance ;-)
Zhouyi Zhou  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149709

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

[PATCH] D112050: [clang] NFC: return void in an alternative expression after call to void function.

[zhouyizhou via Phabricator via cfe-commits]

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

It is better to return void in an alternative expression after call to void 
function. Also, I processed emitDestroy's function body using clang-format.

I don't have write access to LLVM repository,  if the patch got approved, can 
someone commit for me ?

Thanks 
Zhouyi

Signed-off-by: Zhouyi Zhou 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112050

Files:
  clang/lib/CodeGen/CGDecl.cpp


Index: clang/lib/CodeGen/CGDecl.cpp
===
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -2181,14 +2181,15 @@
   Destroyer *destroyer,
   bool useEHCleanupForArray) {
   const ArrayType *arrayType = getContext().getAsArrayType(type);
-  if (!arrayType)
-return destroyer(*this, addr, type);
+  if (!arrayType) {
+destroyer(*this, addr, type);
+return;
+  }
 
   llvm::Value *length = emitArrayLength(arrayType, type, addr);
 
-  CharUnits elementAlign =
-addr.getAlignment()
-.alignmentOfArrayElement(getContext().getTypeSizeInChars(type));
+  CharUnits elementAlign = addr.getAlignment().alignmentOfArrayElement(
+  getContext().getTypeSizeInChars(type));
 
   // Normally we have to check whether the array is zero-length.
   bool checkZeroLength = true;
@@ -2196,15 +2197,16 @@
   // But if the array length is constant, we can suppress that.
   if (llvm::ConstantInt *constLength = dyn_cast(length)) {
 // ...and if it's constant zero, we can just skip the entire thing.
-if (constLength->isZero()) return;
+if (constLength->isZero())
+  return;
 checkZeroLength = false;
   }
 
   llvm::Value *begin = addr.getPointer();
   llvm::Value *end =
   Builder.CreateInBoundsGEP(addr.getElementType(), begin, length);
-  emitArrayDestroy(begin, end, type, elementAlign, destroyer,
-   checkZeroLength, useEHCleanupForArray);
+  emitArrayDestroy(begin, end, type, elementAlign, destroyer, checkZeroLength,
+   useEHCleanupForArray);
 }
 
 /// emitArrayDestroy - Destroys all the elements of the given array,


Index: clang/lib/CodeGen/CGDecl.cpp
===
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -2181,14 +2181,15 @@
   Destroyer *destroyer,
   bool useEHCleanupForArray) {
   const ArrayType *arrayType = getContext().getAsArrayType(type);
-  if (!arrayType)
-return destroyer(*this, addr, type);
+  if (!arrayType) {
+destroyer(*this, addr, type);
+return;
+  }
 
   llvm::Value *length = emitArrayLength(arrayType, type, addr);
 
-  CharUnits elementAlign =
-addr.getAlignment()
-.alignmentOfArrayElement(getContext().getTypeSizeInChars(type));
+  CharUnits elementAlign = addr.getAlignment().alignmentOfArrayElement(
+  getContext().getTypeSizeInChars(type));
 
   // Normally we have to check whether the array is zero-length.
   bool checkZeroLength = true;
@@ -2196,15 +2197,16 @@
   // But if the array length is constant, we can suppress that.
   if (llvm::ConstantInt *constLength = dyn_cast(length)) {
 // ...and if it's constant zero, we can just skip the entire thing.
-if (constLength->isZero()) return;
+if (constLength->isZero())
+  return;
 checkZeroLength = false;
   }
 
   llvm::Value *begin = addr.getPointer();
   llvm::Value *end =
   Builder.CreateInBoundsGEP(addr.getElementType(), begin, length);
-  emitArrayDestroy(begin, end, type, elementAlign, destroyer,
-   checkZeroLength, useEHCleanupForArray);
+  emitArrayDestroy(begin, end, type, elementAlign, destroyer, checkZeroLength,
+   useEHCleanupForArray);
 }
 
 /// emitArrayDestroy - Destroys all the elements of the given array,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112050: [clang] NFC: return void in an alternative expression after call to void function.

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

I create this patch because I am also a Linux hobbyist,  in Linux kernel, 
return statement in a void function is always standalone. For example in 
kernel/event/core.c:

static void
perf_event_exit_event(struct perf_event *child_event,

  struct perf_event_context *child_ctx,
  struct task_struct *child)

{

  ...
  if (!parent_event) {
  perf_event_wakeup(child_event);
  return;
  }
  ...

}

In above code, return statement is standalone. It was rarely seen statement 
like "return perf_event_wakeup(child_event);" in Linux kernel.

Thanks
Zhouyi


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112050

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

[PATCH] D108609: [clang] NFC: remove superfluous braces

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou created this revision.
zhouyizhou added reviewers: pengfei, rsmith, rjmccall, doug.gregor, sepavloff, 
craig.topper, klimek.
zhouyizhou requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

In commit 9bb33f572f7609d469d3a505c9987b83eac5b78c, a pair of superfluous 
braces are introduced to the function Sema::BuildDeclarationNameExpr.
This patch tries to remove the superfluous braces. Also use clang-format to 
further beautify the above function.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108609

Files:
  clang/lib/Sema/SemaExpr.cpp

Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -3250,8 +3250,7 @@
 
   // Make sure that we're referring to a value.
   if (!isa(D)) {
-Diag(Loc, diag::err_ref_non_value)
-  << D << SS.getRange();
+Diag(Loc, diag::err_ref_non_value) << D << SS.getRange();
 Diag(D->getLocation(), diag::note_declared_at);
 return ExprError();
   }
@@ -3277,210 +3276,204 @@
   return BuildAnonymousStructUnionMemberReference(SS, NameInfo.getLoc(),
   indirectField);
 
-  {
-QualType type = VD->getType();
-if (type.isNull())
-  return ExprError();
-ExprValueKind valueKind = VK_PRValue;
+  QualType type = VD->getType();
+  if (type.isNull())
+return ExprError();
+  ExprValueKind valueKind = VK_PRValue;
 
-// In 'T ...V;', the type of the declaration 'V' is 'T...', but the type of
-// a reference to 'V' is simply (unexpanded) 'T'. The type, like the value,
-// is expanded by some outer '...' in the context of the use.
-type = type.getNonPackExpansionType();
+  // In 'T ...V;', the type of the declaration 'V' is 'T...', but the type of
+  // a reference to 'V' is simply (unexpanded) 'T'. The type, like the value,
+  // is expanded by some outer '...' in the context of the use.
+  type = type.getNonPackExpansionType();
 
-switch (D->getKind()) {
+  switch (D->getKind()) {
 // Ignore all the non-ValueDecl kinds.
 #define ABSTRACT_DECL(kind)
 #define VALUE(type, base)
-#define DECL(type, base) \
-case Decl::type:
+#define DECL(type, base) case Decl::type:
 #include "clang/AST/DeclNodes.inc"
-  llvm_unreachable("invalid value decl kind");
-
-// These shouldn't make it here.
-case Decl::ObjCAtDefsField:
-  llvm_unreachable("forming non-member reference to ivar?");
-
-// Enum constants are always r-values and never references.
-// Unresolved using declarations are dependent.
-case Decl::EnumConstant:
-case Decl::UnresolvedUsingValue:
-case Decl::OMPDeclareReduction:
-case Decl::OMPDeclareMapper:
-  valueKind = VK_PRValue;
+llvm_unreachable("invalid value decl kind");
+
+  // These shouldn't make it here.
+  case Decl::ObjCAtDefsField:
+llvm_unreachable("forming non-member reference to ivar?");
+
+  // Enum constants are always r-values and never references.
+  // Unresolved using declarations are dependent.
+  case Decl::EnumConstant:
+  case Decl::UnresolvedUsingValue:
+  case Decl::OMPDeclareReduction:
+  case Decl::OMPDeclareMapper:
+valueKind = VK_PRValue;
+break;
+
+  // Fields and indirect fields that got here must be for
+  // pointer-to-member expressions; we just call them l-values for
+  // internal consistency, because this subexpression doesn't really
+  // exist in the high-level semantics.
+  case Decl::Field:
+  case Decl::IndirectField:
+  case Decl::ObjCIvar:
+assert(getLangOpts().CPlusPlus && "building reference to field in C?");
+
+// These can't have reference type in well-formed programs, but
+// for internal consistency we do this anyway.
+type = type.getNonReferenceType();
+valueKind = VK_LValue;
+break;
+
+  // Non-type template parameters are either l-values or r-values
+  // depending on the type.
+  case Decl::NonTypeTemplateParm: {
+if (const ReferenceType *reftype = type->getAs()) {
+  type = reftype->getPointeeType();
+  valueKind = VK_LValue; // even if the parameter is an r-value reference
   break;
+}
 
-// Fields and indirect fields that got here must be for
-// pointer-to-member expressions; we just call them l-values for
-// internal consistency, because this subexpression doesn't really
-// exist in the high-level semantics.
-case Decl::Field:
-case Decl::IndirectField:
-case Decl::ObjCIvar:
-  assert(getLangOpts().CPlusPlus &&
- "building reference to field in C?");
-
-  // These can't have reference type in well-formed programs, but
-  // for internal consistency we do this anyway.
-  type = type.getNonReferenceType();
+// [expr.prim.id.unqual]p2:
+//   If the entity is a template parameter object for a template
+//   parameter o

[PATCH] D108609: [clang] NFC: remove superfluous braces

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

Thanks for reviewing
I don't have the write access to LLVM, could you please commit this patch for 
me?
Signed-off-by: Zhouyi Zhou 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108609

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

[PATCH] D109408: [libcxxabi] NFC: fix incorrect indentation of braces

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou created this revision.
zhouyizhou added a reviewer: libcxx-commits.
zhouyizhou requested review of this revision.
Herald added projects: clang, libc++abi.
Herald added a subscriber: cfe-commits.
Herald added a reviewer: libc++abi.

Some functions in cxa_exception_storage.cpp have incorrect indentation of 
braces,
use clang-format to fix them

Signed-off-by: Zhouyi Zhou 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109408

Files:
  clang/test/SemaCXX/cxx1z-lambda-star-this.cpp
  clang/test/SemaCXX/cxx20-lambda-decltype-this.cpp
  libcxxabi/src/cxa_exception_storage.cpp

Index: libcxxabi/src/cxa_exception_storage.cpp
===
--- libcxxabi/src/cxa_exception_storage.cpp
+++ libcxxabi/src/cxa_exception_storage.cpp
@@ -18,28 +18,28 @@
 
 namespace __cxxabiv1 {
 extern "C" {
-static __cxa_eh_globals eh_globals;
-__cxa_eh_globals *__cxa_get_globals() { return &eh_globals; }
-__cxa_eh_globals *__cxa_get_globals_fast() { return &eh_globals; }
-}
+static __cxa_eh_globals eh_globals;
+__cxa_eh_globals *__cxa_get_globals() { return &eh_globals; }
+__cxa_eh_globals *__cxa_get_globals_fast() { return &eh_globals; }
 }
+} // namespace __cxxabiv1
 
 #elif defined(HAS_THREAD_LOCAL)
 
 namespace __cxxabiv1 {
 
 namespace {
-__cxa_eh_globals * __globals () {
-static thread_local __cxa_eh_globals eh_globals;
-return &eh_globals;
-}
-}
+__cxa_eh_globals *__globals() {
+  static thread_local __cxa_eh_globals eh_globals;
+  return &eh_globals;
+}
+} // namespace
 
 extern "C" {
-__cxa_eh_globals * __cxa_get_globals  () { return __globals (); }
-__cxa_eh_globals * __cxa_get_globals_fast () { return __globals (); }
-}
+__cxa_eh_globals *__cxa_get_globals() { return __globals(); }
+__cxa_eh_globals *__cxa_get_globals_fast() { return __globals(); }
 }
+} // namespace __cxxabiv1
 
 #else
 
@@ -56,50 +56,49 @@
 
 namespace __cxxabiv1 {
 namespace {
-std::__libcpp_tls_key key_;
-std::__libcpp_exec_once_flag flag_ = _LIBCPP_EXEC_ONCE_INITIALIZER;
-
-void _LIBCPP_TLS_DESTRUCTOR_CC destruct_ (void *p) {
-__free_with_fallback ( p );
-if ( 0 != std::__libcpp_tls_set ( key_, NULL ) )
-abort_message("cannot zero out thread value for __cxa_get_globals()");
-}
-
-void construct_ () {
-if ( 0 != std::__libcpp_tls_create ( &key_, destruct_ ) )
-abort_message("cannot create thread specific key for __cxa_get_globals()");
-}
+std::__libcpp_tls_key key_;
+std::__libcpp_exec_once_flag flag_ = _LIBCPP_EXEC_ONCE_INITIALIZER;
+
+void _LIBCPP_TLS_DESTRUCTOR_CC destruct_(void *p) {
+  __free_with_fallback(p);
+  if (0 != std::__libcpp_tls_set(key_, NULL))
+abort_message("cannot zero out thread value for __cxa_get_globals()");
 }
 
+void construct_() {
+  if (0 != std::__libcpp_tls_create(&key_, destruct_))
+abort_message("cannot create thread specific key for __cxa_get_globals()");
+}
+} // namespace
+
 extern "C" {
-__cxa_eh_globals * __cxa_get_globals () {
-//  Try to get the globals for this thread
-__cxa_eh_globals* retVal = __cxa_get_globals_fast ();
-
-//  If this is the first time we've been asked for these globals, create them
-if ( NULL == retVal ) {
-retVal = static_cast<__cxa_eh_globals*>
-(__calloc_with_fallback (1, sizeof (__cxa_eh_globals)));
-if ( NULL == retVal )
-abort_message("cannot allocate __cxa_eh_globals");
-if ( 0 != std::__libcpp_tls_set ( key_, retVal ) )
-   abort_message("std::__libcpp_tls_set failure in __cxa_get_globals()");
-   }
-return retVal;
-}
-
-// Note that this implementation will reliably return NULL if not
-// preceded by a call to __cxa_get_globals().  This is an extension
-// to the Itanium ABI and is taken advantage of in several places in
-// libc++abi.
-__cxa_eh_globals * __cxa_get_globals_fast () {
-//  First time through, create the key.
-if (0 != std::__libcpp_execute_once(&flag_, construct_))
-abort_message("execute once failure in __cxa_get_globals_fast()");
-//static int init = construct_();
-return static_cast<__cxa_eh_globals*>(std::__libcpp_tls_get(key_));
-}
+__cxa_eh_globals *__cxa_get_globals() {
+  //  Try to get the globals for this thread
+  __cxa_eh_globals *retVal = __cxa_get_globals_fast();
+
+  //  If this is the first time we've been asked for these globals, create them
+  if (NULL == retVal) {
+retVal = static_cast<__cxa_eh_globals *>(
+__calloc_with_fallback(1, sizeof(__cxa_eh_globals)));
+if (NULL == retVal)
+  abort_message("cannot allocate __cxa_eh_globals");
+if (0 != std::__libcpp_tls_set(key_, retVal))
+  abort_message("std::__libcpp_tls_set failure in __cxa_get_globals()");
+  }
+  return retVal;
+}
 
+// Note t

[PATCH] D109408: [libcxxabi] NFC: fix incorrect indentation of braces

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D109408#2988417 , @ldionne wrote:

> FWIW, I dislike that we don't indent stuff at all inside namespaces -- often 
> I find it useful to indent things inside short-lived namespaces. But let's go 
> for simplicity and consistency.
>
> Did you look into clang-formatting other files in libc++abi? What would the 
> diff look like?

Thanks for reviewing,
I will look into other files in libc++abi one by one, and report what I have 
found ;-)
Thanks again
Zhouyi




Comment at: libcxxabi/src/cxa_exception_storage.cpp:102
 }
 }
+} // namespace __cxxabiv1

Quuxplusone wrote:
> LGTM, FWIW.
> 
> It's surprising that clang-format adds `} // namespace foo` closing comments, 
> but doesn't add `} // extern "C"` closing comments: this lonely brace is a 
> bit confusing and I think it //would// benefit from an `// extern "C"` 
> comment.
Thanks for reviewing, 
clang-format do behave a little strange to me. The result of invoking 
clang-format in git cloned directory is diffent from invoking clang-format in 
other directory ( for example /tmp).
By the way, I have not write access to LLVM, so I need someone to commit for me.

Thanks again ;-)
Zhouyi


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109408

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

[PATCH] D109408: [libcxxabi] NFC: fix incorrect indentation of braces

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D109408#2988417 , @ldionne wrote:

> FWIW, I dislike that we don't indent stuff at all inside namespaces -- often 
> I find it useful to indent things inside short-lived namespaces. But let's go 
> for simplicity and consistency.
>
> Did you look into clang-formatting other files in libc++abi? What would the 
> diff look like?

I take a general view of .cpp(.h) files in libc++abi, they are distributed in 
several directories. I do an experiment with
fuzz/cxa_demangle_fuzzer.cpp using clang-format, the result is as follows:

- cxa_demangle_fuzzer.cpp.orig2021-09-09 02:19:17.117872938 +0800

+++ cxa_demangle_fuzzer.cpp2021-09-09 02:19:34.845879852 +0800
@@ -1,15 +1,15 @@
-#include 
 #include 
-#include 
+#include 
 #include 
-extern "C" char *
-__cxa_demangle(const char *mangled_name, char *buf, size_t *n, int *status);
+#include 
+extern "C" char *__cxa_demangle(const char *mangled_name, char *buf, size_t *n,
+int *status);

extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {

- char *str = new char[size+1];

+  char *str = new char[size + 1];

  memcpy(str, data, size);
  str[size] = 0;
  free(__cxa_demangle(str, 0, 0, 0));

- delete [] str;

+  delete[] str;

  return 0;

}

As a beginner of LLVM and C++, I guess clang-formatting is meaningful ;-)

I guess we can clang-format a set of files at a time, and step-by-step 
clang-format all the .cpp(.h) files in libc++abi and fine-tune the results as 
how Quuxplusone did it ,
what's your opinion? If it is OK, I am very honored and pleased to do this work 
;-)

Zhouyi


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109408

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

[PATCH] D109408: [libcxxabi] NFC: fix incorrect indentation of braces

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

Hi,
I have no write access to LLVM, can you commit D109408 
 for me?
Thanks a lot
Zhouyi Zhou 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109408

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

[PATCH] D109408: [libcxxabi] NFC: fix incorrect indentation of braces

[zhouyizhou via Phabricator via cfe-commits]

zhouyizhou added a comment.

In D109408#2996092 , @Quuxplusone 
wrote:

> I was going to land this just now, but then realized that I've lost the 
> thread of why we're doing most of this. Why are we touching 
> clang/test/SemaCXX/? So I'm going to land just the fixing of the curly braces 
> in libcxxabi/src/.

Hi, 
Very sorry to bring you so much trouble. 
I am new to LLVM, and I am only intend to fix the curly braces in 
libcxxabi/src/,
I followed https://www.llvm.org/docs/Phabricator.html#phabricator-reviews to 
make a patch using: 
git diff -U99 u 
 > 
/tmp/mypatch.patch
I am also very confused why clang/test/SemaCXX/ appears in mypatch.patch.
Sorry again.
And thanks for landing the fixing of  the curly braces in libcxxabi/src/ for me.

Zhouyi


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109408

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