[PATCH] D93938: [clang-format] Fixed AfterEnum handling

[Ally Tiritoglu via Phabricator via cfe-commits]

atirit updated this revision to Diff 330495.
atirit added a comment.

Fixed `AfterEnum`'s compatibility with `AllowShortEnumsOnASingleLine`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93938

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13316,6 +13316,7 @@
 TEST_F(FormatTest, AllmanBraceBreaking) {
   FormatStyle AllmanBraceStyle = getLLVMStyle();
   AllmanBraceStyle.BreakBeforeBraces = FormatStyle::BS_Allman;
+  AllmanBraceStyle.AllowShortEnumsOnASingleLine = false;
 
   EXPECT_EQ("namespace a\n"
 "{\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3662,12 +3662,31 @@
 return true;
   }
 
-  if (isAllmanBrace(Left) || isAllmanBrace(Right))
-return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
-   (Line.startsWith(tok::kw_typedef, tok::kw_enum) &&
-Style.BraceWrapping.AfterEnum) ||
+  if (isAllmanBrace(Left) || isAllmanBrace(Right)) {
+bool lineContainsBreakingTokens = false;
+FormatToken *breakingSearchToken = Right.Previous;
+while ((breakingSearchToken = breakingSearchToken->Next)) {
+  bool hasBreakingComma = breakingSearchToken->is(tok::comma) &&
+  breakingSearchToken->Next->is(tok::r_brace);
+  if (breakingSearchToken->isTrailingComment() || hasBreakingComma) {
+lineContainsBreakingTokens = true;
+break;
+  }
+}
+bool isAllowedByAfterEnum =
+(Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
+(Line.startsWith(tok::kw_typedef, tok::kw_enum) &&
+ Style.BraceWrapping.AfterEnum);
+bool isLineTooBig = (strlen(Right.TokenText.data()) +
+ Right.OriginalColumn) > Style.ColumnLimit;
+bool isAllowedByShortEnums = !Style.AllowShortEnumsOnASingleLine ||
+ isLineTooBig || lineContainsBreakingTokens;
+return (isAllowedByAfterEnum &&
+(isAllowedByShortEnums || lineContainsBreakingTokens)) ||
(Line.startsWith(tok::kw_class) && Style.BraceWrapping.AfterClass) 
||
(Line.startsWith(tok::kw_struct) && 
Style.BraceWrapping.AfterStruct);
+  }
+
   if (Left.is(TT_ObjCBlockLBrace) &&
   Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Never)
 return true;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13316,6 +13316,7 @@
 TEST_F(FormatTest, AllmanBraceBreaking) {
   FormatStyle AllmanBraceStyle = getLLVMStyle();
   AllmanBraceStyle.BreakBeforeBraces = FormatStyle::BS_Allman;
+  AllmanBraceStyle.AllowShortEnumsOnASingleLine = false;
 
   EXPECT_EQ("namespace a\n"
 "{\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3662,12 +3662,31 @@
 return true;
   }
 
-  if (isAllmanBrace(Left) || isAllmanBrace(Right))
-return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
-   (Line.startsWith(tok::kw_typedef, tok::kw_enum) &&
-Style.BraceWrapping.AfterEnum) ||
+  if (isAllmanBrace(Left) || isAllmanBrace(Right)) {
+bool lineContainsBreakingTokens = false;
+FormatToken *breakingSearchToken = Right.Previous;
+while ((breakingSearchToken = breakingSearchToken->Next)) {
+  bool hasBreakingComma = breakingSearchToken->is(tok::comma) &&
+  breakingSearchToken->Next->is(tok::r_brace);
+  if (breakingSearchToken->isTrailingComment() || hasBreakingComma) {
+lineContainsBreakingTokens = true;
+break;
+  }
+}
+bool isAllowedByAfterEnum =
+(Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
+(Line.startsWith(tok::kw_typedef, tok::kw_enum) &&
+ Style.BraceWrapping.AfterEnum);
+bool isLineTooBig = (strlen(Right.TokenText.data()) +
+ Right.OriginalColumn) > Style.ColumnLimit;
+bool isAllowedByShortEnums = !Style.AllowShortEnumsOnASingleLine ||
+ isLineTooBig || lineContainsBreakingTokens;
+return (isAllowedByAfterEnum &&
+(isAllowedByShortEnums || lineContainsBreakingTokens)) ||
(Line.startsWith(tok::kw_class) && Style.BraceWrapping.AfterClass) ||
(Line.startsWith(tok::kw_struct) && 

[PATCH] D98160: [clang] Use decltype((E)) for compound requirement type constraint

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 330493.
mizvekov added a comment.

lint


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98160

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.req/compound-requirement.cpp

Index: clang/test/CXX/expr/expr.prim/expr.prim.req/compound-requirement.cpp
===
--- clang/test/CXX/expr/expr.prim/expr.prim.req/compound-requirement.cpp
+++ clang/test/CXX/expr/expr.prim/expr.prim.req/compound-requirement.cpp
@@ -79,19 +79,23 @@
 template
 constexpr bool is_same_v = true;
 
+template struct remove_reference { using type = T; };
+template struct remove_reference { using type = T; };
+
 template
 concept Same = is_same_v;
 
 template
-concept Large = sizeof(T) >= 4; // expected-note{{because 'sizeof(short) >= 4' (2 >= 4) evaluated to false}}
+concept Large = sizeof(typename remove_reference::type) >= 4;
+// expected-note@-1{{because 'sizeof(typename remove_reference::type) >= 4' (2 >= 4) evaluated to false}}
 
-template requires requires (T t) { { t } -> Large; } // expected-note{{because 'decltype(t)' (aka 'short') does not satisfy 'Large':}}
+template requires requires (T t) { { t } -> Large; } // expected-note{{because 'decltype((t))' (aka 'short &') does not satisfy 'Large':}}
 struct r7 {};
 
 using r7i1 = r7;
 using r7i2 = r7; // expected-error{{constraints not satisfied for class template 'r7' [with T = short]}}
 
-template requires requires (T t) { { t } -> Same; }
+template requires requires (T t) { { t } -> Same; }
 struct r8 {};
 
 using r8i1 = r8;
@@ -99,7 +103,8 @@
 
 // Substitution failure in type constraint
 
-template requires requires (T t) { { t } -> Same; } // expected-note{{because 'Same' would be invalid: type 'int' cannot be used prior to '::' because it has no members}}
+template requires requires (T t) { { t } -> Same; }
+// expected-note@-1{{because 'Same' would be invalid: type 'int' cannot be used prior to '::' because it has no members}}
 struct r9 {};
 
 struct M { using type = M; };
@@ -122,6 +127,17 @@
 template requires requires (T t) { { t } -> IsEven; } // expected-error{{concept named in type constraint is not a type concept}}
 struct r11 {};
 
+// Value categories
+
+template
+requires requires (int b) {
+  { a } -> Same;
+  { b } -> Same;
+  { 0 } -> Same;
+  { static_cast(a) } -> Same;
+} void f1() {}
+template void f1<>();
+
 // C++ [expr.prim.req.compound] Example
 namespace std_example {
   template concept C1 =
@@ -172,4 +188,4 @@
   static_assert(C5);
   template struct C5_check {}; // expected-note{{because 'short' does not satisfy 'C5'}}
   using c5 = C5_check; // expected-error{{constraints not satisfied for class template 'C5_check' [with T = short]}}
-}
\ No newline at end of file
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -8832,6 +8832,28 @@
   return Context.getTypeOfExprType(E);
 }
 
+/// getDecltypeForParenthesizedExpr - Given an expr, will return the type for
+/// that expression, as in [dcl.type.simple]p4 but without taking id-expressions
+/// and class member access into account.
+static QualType getDecltypeForParenthesizedExpr(Sema , Expr *E) {
+  // C++11 [dcl.type.simple]p4:
+  //   [...]
+  QualType T = E->getType();
+  switch (E->getValueKind()) {
+  // - otherwise, if e is an xvalue, decltype(e) is T&&, where T is the
+  //   type of e;
+  case VK_XValue:
+return S.Context.getRValueReferenceType(T);
+  // - otherwise, if e is an lvalue, decltype(e) is T&, where T is the
+  //   type of e;
+  case VK_LValue:
+return S.Context.getLValueReferenceType(T);
+  //  - otherwise, decltype(e) is the type of e.
+  case VK_RValue:
+return T;
+  }
+}
+
 /// getDecltypeForExpr - Given an expr, will return the decltype for
 /// that expression, according to the rules in C++11
 /// [dcl.type.simple]p4 and C++11 [expr.lambda.prim]p18.
@@ -8896,22 +8918,7 @@
 }
   }
 
-
-  // C++11 [dcl.type.simple]p4:
-  //   [...]
-  QualType T = E->getType();
-  switch (E->getValueKind()) {
-  // - otherwise, if e is an xvalue, decltype(e) is T&&, where T is the
-  //   type of e;
-  case VK_XValue: T = S.Context.getRValueReferenceType(T); break;
-  // - otherwise, if e is an lvalue, decltype(e) is T&, where T is the
-  //   type of e;
-  case VK_LValue: T = S.Context.getLValueReferenceType(T); break;
-  //  - otherwise, decltype(e) is the type of e.
-  case VK_RValue: break;
-  }
-
-  return T;
+  return getDecltypeForParenthesizedExpr(S, E);
 }
 
 QualType Sema::BuildDecltypeType(Expr *E, SourceLocation Loc,
@@ -8930,6 +8937,11 @@
   return 

Re: [PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Siva Chandra via cfe-commits]

On Sat, Mar 13, 2021 at 9:36 AM Eric Christopher  wrote:

>
>
> On Fri, Mar 5, 2021 at 1:15 AM Siva Chandra via Phabricator <
> revi...@reviews.llvm.org> wrote:
>
>> sivachandra added a comment.
>>
>> In D97736#2605535 , @phosek
>> wrote:
>>
>> > Have you considered using an input linker script? We could generate
>> `libc.so` that could look something like:
>> >
>> >   INPUT(libllvmlibc.a /lib/libc.so)
>> >
>> > We would need to pass `--sysroot` to the linker for this to work. The
>> driver could remain completely agnostic of whether you're using LLVM libc
>> or not.
>>
>> Yes, that was also considered. Those downstream users who have the
>> flexibility to do it that way should be able to do it that way. However,
>> not all downstream users or normal clang users will have that liberty [1].
>> Another point to note is that we will have to do this with all libc
>> components like `libc.so`, `libm.so` etc.
>>
>> [1] I think all of this can be done. For example, we can set all this up
>> when building a distribution. However, I am not sure this is worth it when
>> we know this is a transient phase. Soon, when LLVM libc is complete enough,
>> a more appropriate option would be the one which allows choosing a libc as
>> Eric pointed out.
>>
>
> To be clear I'm not a fan of a "pick your libc" option as opposed to just
> naming the compiled llvm libc as perhaps libc.[a,so,etc] similar to other
> platforms. I think we'd need a good reason to diverge here.
>

The reason is that LLVM libc is not a libc until it can be one. This option
is being added so that one can use LLVM libc as a source of alternate
implementations. Once LLVM libc can actually be a full libc, this option is
not required. Also, from that point in time, the LLVM libc binary should be
given the conventional libc. name as you suggest.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98594: [clang/docs/LibASTMatchersTutorial.rst,clang/docs/HowToSetupToolingForLLVM.rst] ninja now uses `configure.py` rather than `bootstrap.py`

[Samuel Marks via Phabricator via cfe-commits]

SamuelMarks updated this revision to Diff 330487.

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

https://reviews.llvm.org/D98594

Files:
  clang/docs/HowToSetupToolingForLLVM.rst
  clang/docs/LibASTMatchersTutorial.rst


Index: clang/docs/LibASTMatchersTutorial.rst
===
--- clang/docs/LibASTMatchersTutorial.rst
+++ clang/docs/LibASTMatchersTutorial.rst
@@ -33,7 +33,7 @@
   git clone https://github.com/martine/ninja.git
   cd ninja
   git checkout release
-  ./bootstrap.py
+  ./configure.py --bootstrap
   sudo cp ninja /usr/bin/
 
   cd ~/clang-llvm
Index: clang/docs/HowToSetupToolingForLLVM.rst
===
--- clang/docs/HowToSetupToolingForLLVM.rst
+++ clang/docs/HowToSetupToolingForLLVM.rst
@@ -156,7 +156,7 @@
 
   $ git clone git://github.com/martine/ninja.git
   $ cd ninja/
-  $ ./bootstrap.py
+  $ ./configure.py --bootstrap
 
 This will result in a single binary ``ninja`` in the current directory.
 It doesn't require installation and can just be copied to any location


Index: clang/docs/LibASTMatchersTutorial.rst
===
--- clang/docs/LibASTMatchersTutorial.rst
+++ clang/docs/LibASTMatchersTutorial.rst
@@ -33,7 +33,7 @@
   git clone https://github.com/martine/ninja.git
   cd ninja
   git checkout release
-  ./bootstrap.py
+  ./configure.py --bootstrap
   sudo cp ninja /usr/bin/
 
   cd ~/clang-llvm
Index: clang/docs/HowToSetupToolingForLLVM.rst
===
--- clang/docs/HowToSetupToolingForLLVM.rst
+++ clang/docs/HowToSetupToolingForLLVM.rst
@@ -156,7 +156,7 @@
 
   $ git clone git://github.com/martine/ninja.git
   $ cd ninja/
-  $ ./bootstrap.py
+  $ ./configure.py --bootstrap
 
 This will result in a single binary ``ninja`` in the current directory.
 It doesn't require installation and can just be copied to any location
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D98594: [clang/docs/LibASTMatchersTutorial.rst,clang/docs/HowToSetupToolingForLLVM.rst] ninja now uses `configure.py` rather than `bootstrap.py`

[Samuel Marks via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98594

Files:
  clang/docs/HowToSetupToolingForLLVM.rst
  clang/docs/LibASTMatchersTutorial.rst


Index: clang/docs/LibASTMatchersTutorial.rst
===
--- clang/docs/LibASTMatchersTutorial.rst
+++ clang/docs/LibASTMatchersTutorial.rst
@@ -33,7 +33,7 @@
   git clone https://github.com/martine/ninja.git
   cd ninja
   git checkout release
-  ./bootstrap.py
+  ./configure.py
   sudo cp ninja /usr/bin/
 
   cd ~/clang-llvm
Index: clang/docs/HowToSetupToolingForLLVM.rst
===
--- clang/docs/HowToSetupToolingForLLVM.rst
+++ clang/docs/HowToSetupToolingForLLVM.rst
@@ -156,7 +156,7 @@
 
   $ git clone git://github.com/martine/ninja.git
   $ cd ninja/
-  $ ./bootstrap.py
+  $ ./configure.py
 
 This will result in a single binary ``ninja`` in the current directory.
 It doesn't require installation and can just be copied to any location


Index: clang/docs/LibASTMatchersTutorial.rst
===
--- clang/docs/LibASTMatchersTutorial.rst
+++ clang/docs/LibASTMatchersTutorial.rst
@@ -33,7 +33,7 @@
   git clone https://github.com/martine/ninja.git
   cd ninja
   git checkout release
-  ./bootstrap.py
+  ./configure.py
   sudo cp ninja /usr/bin/
 
   cd ~/clang-llvm
Index: clang/docs/HowToSetupToolingForLLVM.rst
===
--- clang/docs/HowToSetupToolingForLLVM.rst
+++ clang/docs/HowToSetupToolingForLLVM.rst
@@ -156,7 +156,7 @@
 
   $ git clone git://github.com/martine/ninja.git
   $ cd ninja/
-  $ ./bootstrap.py
+  $ ./configure.py
 
 This will result in a single binary ``ninja`` in the current directory.
 It doesn't require installation and can just be copied to any location
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97990: [clang] Improve diagnostics on implicitly deleted defaulted comparisons

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8968-8970
 def note_defaulted_comparison_calls_deleted : Note<
   "defaulted %0 is implicitly deleted because it would invoke a deleted "
   "comparison function%select{| for member %2| for base class %2}1">;

mizvekov wrote:
> rsmith wrote:
> > Would it be useful to apply the same diagnostic improvement to this 
> > diagnostic too? (Genuine question: I *think* we'll attach a note pointing 
> > to the deleted function in this case, which would probably make the "for T" 
> > part just be noise, but I've not checked.)
> Yeah I thought about checking other errors for similar improvements. It's on 
> my list to check that.
For this one in particular, we really do attach another note that makes it 
obvious, pointing to the explicitly marked deleted function.
`note_defaulted_comparison_not_constexpr` at first glance looks like might 
suffer from the same problem, but in this case I don't think the error ever 
could apply to the complete object.
And then nothing else that I could find.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97990

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

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

> If what you're suggesting is that Clang have a SYCL-specific page that serves 
> a similar purpose to https://clang.llvm.org/docs/OpenCLSupport.html, then I 
> agree we should have that (and it should be linked to from 
> https://clang.llvm.org/docs/index.html the same as OpenCL, OpenMP, etc). 
> However, I think that's orthogonal to this patch and should be done 
> separately. (If the documentation already existed, then I'd request that this 
> patch update it, but because those docs don't exist yet, I think it's 
> unrelated enough to warrant being done separately.)
>
> If you're requesting something different, I'd appreciate a bit more specific 
> details.

I don't really mind the format. It doesn't have to be in Clang documentation 
although perhaps it makes the most sense logically. What I would like to see is 
a transparancy within the commnity that is I believe a common practice as per 
clang contribution policy:

  A specification: The specification must be sufficient to understand the 
design of the feature as well as interpret the meaning of specific examples. 
The specification should be detailed enough that another compiler vendor could 
implement the feature.

I don't find it great to continue reviewing the work where I don't have a clear 
picture. I also would like to avoid the situation the community has to work 
with code that we don't understand. We should not be in need to reverse 
engineer what is being implemented.  It is not a comfortable position to be nor 
it is productive. Once we know the high-level language semantic it facilitates 
us to reason about implementation should we need to redesign anything or fix a 
bug. If we only have fragements of code it doesn't tell us a lot and there is 
very little we can do with it.  Perhaps this is why good practices have been 
added by the community? There are also other areas whether it demonstrated to 
work really well - for very good reasons we no longer add undocumented 
attributes to clang.

> Given that we lack the dedicated documentation page for SYCL within Clang, I 
> guess I view that as the status quo (we've already started some of the 
> upstreaming work and none of it is documented in a convenient place for Clang 
> users). Based on that, as someone who doesn't really do much with SYCL to 
> date, I think it's reasonable to accept this patch because it extends 
> existing functionality in a straight-forward manner with a reasonable 
> explanation as to why.

I am not aware of all work on SYCL, but I believe some documentation was 
provided, for example, for the new kernel attribute?

Personally I don't find the patch straight-forward mainly because it deviates 
from the original design wrt target address space map. But there are other 
concerns - for example the need for `InferAddressSpace` that is not required 
for embedded C and C++ functionality that to my understanding SYCL aims to 
implement. However, I don't know whether my understanding of the aim is correct 
as I don't have any reference to verify my understanding.

> If there are technical concerns to be addressed, I'm not certain it's clear 
> (to me, at least) what they are.

Well the issue I got stuck at was highlighted earlier - the need to add new 
address spaces map for SYCL. So far the design we were striking at is that 
every language would have a new entry in the map but adding multiple maps per 
language seems to defeat the purpose. Considering that OpenCL and SYCL would 
likely be supported by similar targets we should try to avoid this or at least 
try to understand why we ended up with such situation and what we might be able 
to do to improve. I would accept if we can't do anything with this immediately 
in case it requires big restructuring but perhaps at least this deserves a 
wider community communication.

> That's reasonable, but I think we also have to trust the domain experts who 
> are submitting patches to have done their design homework and to be willing 
> to address concerns as they come up when there's concretely a problem. This 
> is not an extremely experimental invention from an unknown lone contributor; 
> this is a long-time maintainer upstreaming work from downstream repo with 
> users who use this functionality.

I don't think this can be called a trust issue. For me personally, it is about 
making sure to align with the requirements from various domains that none of us 
has complete knowledge of. There is certainly no invention but however a case 
of substantially new functionality that is using the feature scattered all over 
the frontend source code. I am sorry that I just don't find it trivial. While I 
have worked on this topic for many years I wouldn't trust myself to make a new 
design decision alone because it is important to get things right in general 
not just for my product. As a matter of fact me and some other developers from 
this area were working quite a lot in the last years on the 

[PATCH] D98160: [clang] Use decltype((E)) for compound requirement type constraint

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 330484.
mizvekov added a comment.

- Uses yet another simpler approach: Since dependence should not matter (we 
were using the type of the expression after template instantiation anyway), we 
don't even need to build a `decltype` type here, and just get the canonical 
type from the `(expr)` instead. Kudos to rsmith for this suggestion in private.
- Adds some more tests, testing more kinds of values.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98160

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.req/compound-requirement.cpp

Index: clang/test/CXX/expr/expr.prim/expr.prim.req/compound-requirement.cpp
===
--- clang/test/CXX/expr/expr.prim/expr.prim.req/compound-requirement.cpp
+++ clang/test/CXX/expr/expr.prim/expr.prim.req/compound-requirement.cpp
@@ -79,19 +79,23 @@
 template
 constexpr bool is_same_v = true;
 
+template struct remove_reference { using type = T; };
+template struct remove_reference { using type = T; };
+
 template
 concept Same = is_same_v;
 
 template
-concept Large = sizeof(T) >= 4; // expected-note{{because 'sizeof(short) >= 4' (2 >= 4) evaluated to false}}
+concept Large = sizeof(typename remove_reference::type) >= 4;
+// expected-note@-1{{because 'sizeof(typename remove_reference::type) >= 4' (2 >= 4) evaluated to false}}
 
-template requires requires (T t) { { t } -> Large; } // expected-note{{because 'decltype(t)' (aka 'short') does not satisfy 'Large':}}
+template requires requires (T t) { { t } -> Large; } // expected-note{{because 'decltype((t))' (aka 'short &') does not satisfy 'Large':}}
 struct r7 {};
 
 using r7i1 = r7;
 using r7i2 = r7; // expected-error{{constraints not satisfied for class template 'r7' [with T = short]}}
 
-template requires requires (T t) { { t } -> Same; }
+template requires requires (T t) { { t } -> Same; }
 struct r8 {};
 
 using r8i1 = r8;
@@ -99,7 +103,8 @@
 
 // Substitution failure in type constraint
 
-template requires requires (T t) { { t } -> Same; } // expected-note{{because 'Same' would be invalid: type 'int' cannot be used prior to '::' because it has no members}}
+template requires requires (T t) { { t } -> Same; }
+// expected-note@-1{{because 'Same' would be invalid: type 'int' cannot be used prior to '::' because it has no members}}
 struct r9 {};
 
 struct M { using type = M; };
@@ -122,6 +127,17 @@
 template requires requires (T t) { { t } -> IsEven; } // expected-error{{concept named in type constraint is not a type concept}}
 struct r11 {};
 
+// Value categories
+
+template
+requires requires (int b) {
+  { a } -> Same;
+  { b } -> Same;
+  { 0 } -> Same;
+  { static_cast(a) } -> Same;
+} void f1() {}
+template void f1<>();
+
 // C++ [expr.prim.req.compound] Example
 namespace std_example {
   template concept C1 =
@@ -172,4 +188,4 @@
   static_assert(C5);
   template struct C5_check {}; // expected-note{{because 'short' does not satisfy 'C5'}}
   using c5 = C5_check; // expected-error{{constraints not satisfied for class template 'C5_check' [with T = short]}}
-}
\ No newline at end of file
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -8832,6 +8832,30 @@
   return Context.getTypeOfExprType(E);
 }
 
+/// getDecltypeForParenthesizedExpr - Given an expr, will return the type for
+/// that expression, as in [dcl.type.simple]p4 but without taking id-expressions
+/// and class member access into account.
+static QualType getDecltypeForParenthesizedExpr(Sema , Expr *E) {
+  // C++11 [dcl.type.simple]p4:
+  //   [...]
+  QualType T = E->getType();
+  switch (E->getValueKind()) {
+  // - otherwise, if e is an xvalue, decltype(e) is T&&, where T is the
+  //   type of e;
+  case VK_XValue:
+return S.Context.getRValueReferenceType(T);
+  // - otherwise, if e is an lvalue, decltype(e) is T&, where T is the
+  //   type of e;
+  case VK_LValue:
+return S.Context.getLValueReferenceType(T);
+  //  - otherwise, decltype(e) is the type of e.
+  case VK_RValue:
+return T;
+  default:
+llvm_unreachable("bad kind");
+  }
+}
+
 /// getDecltypeForExpr - Given an expr, will return the decltype for
 /// that expression, according to the rules in C++11
 /// [dcl.type.simple]p4 and C++11 [expr.lambda.prim]p18.
@@ -8896,22 +8920,7 @@
 }
   }
 
-
-  // C++11 [dcl.type.simple]p4:
-  //   [...]
-  QualType T = E->getType();
-  switch (E->getValueKind()) {
-  // - otherwise, if e is an xvalue, decltype(e) is T&&, where T is the
-  //   type of e;
-  case VK_XValue: T = S.Context.getRValueReferenceType(T); break;
-  // - 

[PATCH] D98574: [Sparc] Define the same macros for -mcpu=v9 as GCC on Linux

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 requested changes to this revision.
jrtc27 added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Basic/Targets/Sparc.cpp:160-164
+  // Linux doesn't need these variants, but the BSDs do.
+  if (getTriple().getOS() != llvm::Triple::Linux) {
+Builder.defineMacro("__sparcv9");
+Builder.defineMacro("__sparcv9__");
+  }

This is wrong, they should never be defined on any OS for 32-bit SPARC; see 
https://github.com/gcc-mirror/gcc/blob/master/gcc/config/sparc/netbsd-elf.h#L22 
for example (FreeBSD only ever had a 64-bit port so isn't interesting to look 
at). That is, the contents of this `if` should just be deleted, leaving just 
`__sparc_v9__`.



Comment at: clang/lib/Basic/Targets/Sparc.cpp:246-256
+  if (getTriple().getOS() == llvm::Triple::Linux) {
 Builder.defineMacro("__sparc_v9__");
-Builder.defineMacro("__sparcv9__");
+  } else {
+Builder.defineMacro("__sparcv9");
+// Solaris doesn't need these variants, but the BSDs do.
+if (getTriple().getOS() != llvm::Triple::Solaris) {
+  Builder.defineMacro("__sparc64__");

This doesn't need changing, we can define more things than GCC to keep it 
simple.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98574

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

[PATCH] D98574: [Sparc] Define the same macros for -mcpu=v9 as GCC on Linux

[Brad Smith via Phabricator via cfe-commits]

brad added a comment.

I do not immediately see why the other tests are failing, but at a bare minimum 
the following test from clang/test/Preprocessor/predefined-arch-macros.c will 
have to be updated..

  // RUN: %clang -mcpu=v9 -E -dM %s -o - 2>&1 \
  // RUN: -target sparc-unknown-linux \
  // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_SPARC-V9
  // CHECK_SPARC-V9-NOT: #define __sparcv8 1
  // CHECK_SPARC-V9-NOT: #define __sparcv8__ 1
  // CHECK_SPARC-V9: #define __sparc_v9__ 1
  // CHECK_SPARC-V9: #define __sparcv9 1
  // CHECK_SPARC-V9: #define __sparcv9__ 1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98574

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

[PATCH] D94741: [Utils] Check for more global information in update_test_checks

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

It looks like this has broken handling of `getelementptr %T`, and generates 
`getelementptr [[T]]` for it now, see e.g. the first change in 
https://github.com/llvm/llvm-project/commit/7ee96429a0b057bcc97331a6a762fc3cd00aed61.

In 
https://github.com/llvm/llvm-project/commit/6491e0165e96a93960e2dfb338c52c7eb155f408#diff-85c14e813467fc768fb641be9567780053ef1162da8cc12dd6bcb29d5e14384eR575
 I was forced to rename the function argument to avoid a clash between the type 
`%S2` and the value `%s2`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94741

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

[PATCH] D95745: Support unwinding from inline assembly

[Paul via Phabricator via cfe-commits]

cynecx added a comment.

Weekly ping!


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

https://reviews.llvm.org/D95745

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

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:2245
+  case tok::kw___ibm128:
+DS.SetTypeSpecType(DeclSpec::TST_ibm128, Loc, PrevSpec, DiagID, Policy);
+break;

jwakely wrote:
> hubert.reinterpretcast wrote:
> > jwakely wrote:
> > > hubert.reinterpretcast wrote:
> > > > qiucf wrote:
> > > > > hubert.reinterpretcast wrote:
> > > > > > qiucf wrote:
> > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > Not sure what the best method is to implement this, but `long 
> > > > > > > > double` and `__ibm128` are the same type for GCC when 
> > > > > > > > `-mabi=ibmlongdouble` is in effect.
> > > > > > > Seems clang is also different from GCC under 
> > > > > > > `-mabi=ieeelongdouble`? I saw `__float128` and `long double` are 
> > > > > > > the same for GCC but not for clang.
> > > > > > Have you checked whether the new libstdc++ for which this support 
> > > > > > is being added needs the GCC behaviour to work properly?
> > > > > > 
> > > > > > The GCC behaviour allows the following to be compiled without 
> > > > > > introducing novel overload resolution tiebreakers:
> > > > > > ```
> > > > > > void f(__float128);
> > > > > > void f(__ibm128);
> > > > > > void f(int);
> > > > > > 
> > > > > > long double ld;
> > > > > > 
> > > > > > int main() { f(ld); }
> > > > > > ```
> > > > > As I saw both GCC and clang have error for ambiguous `operator<<` for:
> > > > > 
> > > > > ```
> > > > > std::cout << "long double:\n";
> > > > > std::cout << std::numeric_limits::max() << std::endl;
> > > > > std::cout << std::numeric_limits::min() << std::endl;
> > > > > 
> > > > > std::cout << "__float128:\n";
> > > > > std::cout << std::numeric_limits<__float128>::max() << std::endl;
> > > > > std::cout << std::numeric_limits<__float128>::min() << std::endl;
> > > > > 
> > > > > std::cout << "__ibm128:\n";
> > > > > std::cout << std::numeric_limits<__ibm128>::max() << std::endl;
> > > > > std::cout << std::numeric_limits<__ibm128>::min() << std::endl;
> > > > > ```
> > > > @jwakely, are the overload resolution errors expected? @qiucf, are you 
> > > > sure you have a sufficiently new libstdc++?
> > > > @jwakely, are the overload resolution errors expected? 
> > > 
> > > Yes. Iostreams support `long double` but not `__float128`, unless that 
> > > happens to be the same type as `long double` (due to a 
> > > `-mabi=ieeelongdouble` option).
> > Meaning that Clang's `__float128` iosteams support (with libstdc++) would 
> > diverge from GCC.
> > 
> > For example, Clang reports the call below as ambiguous even with 
> > `-mabi=ieeelongdouble`:
> > ```
> > void f(double);
> > void f(long double);
> > 
> > void g(__float128 f128) { f(f128); }
> > ```
> > 
> > https://godbolt.org/z/dhYEKa
> > 
> > Insofar as the user experience goes, is this lack of iostreams support for 
> > `__float128` (even with `-mabi=ieeelongdouble`) within the realm of the 
> > intended design of libstdc++?
> The lack of iostreams support for `__float128` is the intended design.
> 
> On power we support `float`, `double` and three types of `long double`. If 
> `__float128` is a distinct type from all those `long double` types it won't 
> work.
> 
> GCC on power defines `__float128` as a macro expanding to `__ieee128`, so it 
> is the same as one of the `long double` types.
Okay, it sounds like the different treatment Clang has does not really 
interfere with recommended usage insofar as libstdc++ iostreams (and hopefully 
this extends to the rest of the library).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93377

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

[PATCH] D98134: [RFC][POC] Introduce callback argument encoding mode into callback metadata

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 330464.
tianshilei1992 added a comment.

remove the requirement of `size_t size`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98134

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/attr-callback.c
  clang/test/CodeGen/callback_annotated.c
  clang/test/CodeGen/callback_openmp.c
  clang/test/CodeGenCXX/attr-callback.cpp
  clang/test/OpenMP/parallel_codegen.cpp
  clang/test/Sema/attr-callback-broken.c
  clang/test/Sema/attr-callback.c
  clang/test/SemaCXX/attr-callback.cpp
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/AbstractCallSite.h
  llvm/include/llvm/IR/MDBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/IR/AbstractCallSite.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/test/Analysis/CallGraph/callback-calls.ll
  llvm/test/Analysis/CallGraph/ignore-callback-uses.ll
  llvm/test/Transforms/Attributor/IPConstantProp/multiple_callbacks.ll
  llvm/test/Transforms/Attributor/IPConstantProp/openmp_parallel_for.ll
  llvm/test/Transforms/Attributor/IPConstantProp/pthreads.ll
  llvm/test/Transforms/Attributor/IPConstantProp/thread_local_acs.ll
  llvm/test/Transforms/Attributor/callbacks.ll
  llvm/test/Transforms/Attributor/noundef.ll
  llvm/test/Transforms/OpenMP/parallel_deletion.ll
  llvm/test/Transforms/OpenMP/parallel_deletion_cg_update.ll
  llvm/test/Transforms/OpenMP/parallel_deletion_remarks.ll
  llvm/test/Transforms/OpenMP/parallel_region_merging.ll
  llvm/unittests/IR/AbstractCallSiteTest.cpp
  llvm/unittests/IR/LegacyPassManagerTest.cpp

Index: llvm/unittests/IR/LegacyPassManagerTest.cpp
===
--- llvm/unittests/IR/LegacyPassManagerTest.cpp
+++ llvm/unittests/IR/LegacyPassManagerTest.cpp
@@ -768,7 +768,7 @@
"}\n"
"\n"
"!0 = !{!1}\n"
-   "!1 = !{i64 0, i64 1, i1 false}";
+   "!1 = !{i64 0, i64 0, i64 1, i1 false}";
 
   SMDiagnostic Err;
   std::unique_ptr M = parseAssemblyString(IR, Err, Context);
Index: llvm/unittests/IR/AbstractCallSiteTest.cpp
===
--- llvm/unittests/IR/AbstractCallSiteTest.cpp
+++ llvm/unittests/IR/AbstractCallSiteTest.cpp
@@ -23,7 +23,7 @@
   return Mod;
 }
 
-TEST(AbstractCallSite, CallbackCall) {
+TEST(AbstractCallSite, FlatCallbackCall) {
   LLVMContext C;
 
   const char *IR =
@@ -36,7 +36,7 @@
   "}\n"
   "declare !callback !0 void @broker(i32, void (i8*, ...)*, ...)\n"
   "!0 = !{!1}\n"
-  "!1 = !{i64 1, i64 -1, i1 true}";
+  "!1 = !{i64 0, i64 1, i64 -1, i1 true}";
 
   std::unique_ptr M = parseIR(C, IR);
   ASSERT_TRUE(M);
@@ -53,3 +53,105 @@
   EXPECT_TRUE(ACS.isCallee(CallbackUse));
   EXPECT_EQ(ACS.getCalledFunction(), Callback);
 }
+
+TEST(AbstractCallSite, StackedCallbackCallOpenMP) {
+  LLVMContext C;
+
+  const char *IR =
+"define dso_local void @callback(i64, i64) {\n"
+"  ret void\n"
+"}\n"
+"define dso_local void @broker() {\n"
+"entry:\n"
+"  %a = alloca i32, align 4\n"
+"  %b = alloca float, align 4\n"
+"  %c = alloca double, align 8\n"
+"  %a.casted = alloca i64, align 8\n"
+"  %b.casted = alloca i64, align 8\n"
+"  %c.casted = alloca i64, align 8\n"
+"  %args = alloca [3 x i8*], align 8\n"
+"  %0 = load i32, i32* %a, align 4\n"
+"  %conv = bitcast i64* %a.casted to i32*\n"
+"  store i32 %0, i32* %conv, align 4\n"
+"  %1 = load i64, i64* %a.casted, align 8\n"
+"  %2 = load float, float* %b, align 4\n"
+"  %conv1 = bitcast i64* %b.casted to float*\n"
+"  store float %2, float* %conv1, align 4\n"
+"  %b.addr = load i64, i64* %b.casted, align 8\n"
+"  %3 = getelementptr inbounds [3 x i8*], [3 x i8*]* %args, i32 0, i32 0\n"
+"  %4 = bitcast i8** %3 to i64*\n"
+"  store i64 %1, i64* %4, align 8\n"
+"  %5 = getelementptr inbounds [3 x i8*], [3 x i8*]* %args, i32 0, i32 1\n"
+"  %6 = bitcast i8** %5 to i64*\n"
+"  store i64 %b.addr, i64* %6, align 8\n"
+"  %7 = getelementptr inbounds [3 x i8*], [3 x i8*]* %args, i32 0, i32 0\n"
+"  call i32 @__tgt_target(i64 -1, i8* bitcast (void (i64, i64)* @callback to i8*), i32 2, i8** null, i8** %7, i64* null, i64* null)\n"
+"  ret void\n"
+"}\n"
+"declare !callback !1 i32 @__tgt_target(i64, i8*, i32, i8**, i8**, i64*, i64*)\n"
+"!1 = !{!2}\n"
+"!2 = !{i64 1, i64 1, i64 4, i1 false}\n";
+
+  std::unique_ptr M = parseIR(C, IR);
+  ASSERT_TRUE(M);
+
+  Function *Callback = M->getFunction("callback");
+  ASSERT_NE(Callback, nullptr);
+
+  const Use *CallbackUse = Callback->getSingleUndroppableUse();
+  ASSERT_NE(CallbackUse, nullptr);
+
+  AbstractCallSite 

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added inline comments.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:2245
+  case tok::kw___ibm128:
+DS.SetTypeSpecType(DeclSpec::TST_ibm128, Loc, PrevSpec, DiagID, Policy);
+break;

hubert.reinterpretcast wrote:
> jwakely wrote:
> > hubert.reinterpretcast wrote:
> > > qiucf wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > qiucf wrote:
> > > > > > hubert.reinterpretcast wrote:
> > > > > > > Not sure what the best method is to implement this, but `long 
> > > > > > > double` and `__ibm128` are the same type for GCC when 
> > > > > > > `-mabi=ibmlongdouble` is in effect.
> > > > > > Seems clang is also different from GCC under 
> > > > > > `-mabi=ieeelongdouble`? I saw `__float128` and `long double` are 
> > > > > > the same for GCC but not for clang.
> > > > > Have you checked whether the new libstdc++ for which this support is 
> > > > > being added needs the GCC behaviour to work properly?
> > > > > 
> > > > > The GCC behaviour allows the following to be compiled without 
> > > > > introducing novel overload resolution tiebreakers:
> > > > > ```
> > > > > void f(__float128);
> > > > > void f(__ibm128);
> > > > > void f(int);
> > > > > 
> > > > > long double ld;
> > > > > 
> > > > > int main() { f(ld); }
> > > > > ```
> > > > As I saw both GCC and clang have error for ambiguous `operator<<` for:
> > > > 
> > > > ```
> > > > std::cout << "long double:\n";
> > > > std::cout << std::numeric_limits::max() << std::endl;
> > > > std::cout << std::numeric_limits::min() << std::endl;
> > > > 
> > > > std::cout << "__float128:\n";
> > > > std::cout << std::numeric_limits<__float128>::max() << std::endl;
> > > > std::cout << std::numeric_limits<__float128>::min() << std::endl;
> > > > 
> > > > std::cout << "__ibm128:\n";
> > > > std::cout << std::numeric_limits<__ibm128>::max() << std::endl;
> > > > std::cout << std::numeric_limits<__ibm128>::min() << std::endl;
> > > > ```
> > > @jwakely, are the overload resolution errors expected? @qiucf, are you 
> > > sure you have a sufficiently new libstdc++?
> > > @jwakely, are the overload resolution errors expected? 
> > 
> > Yes. Iostreams support `long double` but not `__float128`, unless that 
> > happens to be the same type as `long double` (due to a 
> > `-mabi=ieeelongdouble` option).
> Meaning that Clang's `__float128` iosteams support (with libstdc++) would 
> diverge from GCC.
> 
> For example, Clang reports the call below as ambiguous even with 
> `-mabi=ieeelongdouble`:
> ```
> void f(double);
> void f(long double);
> 
> void g(__float128 f128) { f(f128); }
> ```
> 
> https://godbolt.org/z/dhYEKa
> 
> Insofar as the user experience goes, is this lack of iostreams support for 
> `__float128` (even with `-mabi=ieeelongdouble`) within the realm of the 
> intended design of libstdc++?
The lack of iostreams support for `__float128` is the intended design.

On power we support `float`, `double` and three types of `long double`. If 
`__float128` is a distinct type from all those `long double` types it won't 
work.

GCC on power defines `__float128` as a macro expanding to `__ieee128`, so it is 
the same as one of the `long double` types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93377

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

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:2245
+  case tok::kw___ibm128:
+DS.SetTypeSpecType(DeclSpec::TST_ibm128, Loc, PrevSpec, DiagID, Policy);
+break;

jwakely wrote:
> hubert.reinterpretcast wrote:
> > qiucf wrote:
> > > hubert.reinterpretcast wrote:
> > > > qiucf wrote:
> > > > > hubert.reinterpretcast wrote:
> > > > > > Not sure what the best method is to implement this, but `long 
> > > > > > double` and `__ibm128` are the same type for GCC when 
> > > > > > `-mabi=ibmlongdouble` is in effect.
> > > > > Seems clang is also different from GCC under `-mabi=ieeelongdouble`? 
> > > > > I saw `__float128` and `long double` are the same for GCC but not for 
> > > > > clang.
> > > > Have you checked whether the new libstdc++ for which this support is 
> > > > being added needs the GCC behaviour to work properly?
> > > > 
> > > > The GCC behaviour allows the following to be compiled without 
> > > > introducing novel overload resolution tiebreakers:
> > > > ```
> > > > void f(__float128);
> > > > void f(__ibm128);
> > > > void f(int);
> > > > 
> > > > long double ld;
> > > > 
> > > > int main() { f(ld); }
> > > > ```
> > > As I saw both GCC and clang have error for ambiguous `operator<<` for:
> > > 
> > > ```
> > > std::cout << "long double:\n";
> > > std::cout << std::numeric_limits::max() << std::endl;
> > > std::cout << std::numeric_limits::min() << std::endl;
> > > 
> > > std::cout << "__float128:\n";
> > > std::cout << std::numeric_limits<__float128>::max() << std::endl;
> > > std::cout << std::numeric_limits<__float128>::min() << std::endl;
> > > 
> > > std::cout << "__ibm128:\n";
> > > std::cout << std::numeric_limits<__ibm128>::max() << std::endl;
> > > std::cout << std::numeric_limits<__ibm128>::min() << std::endl;
> > > ```
> > @jwakely, are the overload resolution errors expected? @qiucf, are you sure 
> > you have a sufficiently new libstdc++?
> > @jwakely, are the overload resolution errors expected? 
> 
> Yes. Iostreams support `long double` but not `__float128`, unless that 
> happens to be the same type as `long double` (due to a `-mabi=ieeelongdouble` 
> option).
Meaning that Clang's `__float128` iosteams support (with libstdc++) would 
diverge from GCC.

For example, Clang reports the call below as ambiguous even with 
`-mabi=ieeelongdouble`:
```
void f(double);
void f(long double);

void g(__float128 f128) { f(f128); }
```

https://godbolt.org/z/dhYEKa

Insofar as the user experience goes, is this lack of iostreams support for 
`__float128` (even with `-mabi=ieeelongdouble`) within the realm of the 
intended design of libstdc++?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93377

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

[PATCH] D97080: [flang][driver] Add -fintrinsic-modules-path option

[Bryan Chan via Phabricator via cfe-commits]

bryanpkc added inline comments.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:292
+  std::string driverPath = llvm::sys::fs::getMainExecutable(nullptr, nullptr);
+  driverPath = driverPath.substr(0, driverPath.size() - 9);
+  return driverPath.append("/../include/flang/");

Can you use `llvm::sys::path::remove_filename` here?


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

https://reviews.llvm.org/D97080

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

Re: [PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

[Eric Christopher via cfe-commits]

On Fri, Mar 5, 2021 at 1:15 AM Siva Chandra via Phabricator <
revi...@reviews.llvm.org> wrote:

> sivachandra added a comment.
>
> In D97736#2605535 , @phosek
> wrote:
>
> > Have you considered using an input linker script? We could generate
> `libc.so` that could look something like:
> >
> >   INPUT(libllvmlibc.a /lib/libc.so)
> >
> > We would need to pass `--sysroot` to the linker for this to work. The
> driver could remain completely agnostic of whether you're using LLVM libc
> or not.
>
> Yes, that was also considered. Those downstream users who have the
> flexibility to do it that way should be able to do it that way. However,
> not all downstream users or normal clang users will have that liberty [1].
> Another point to note is that we will have to do this with all libc
> components like `libc.so`, `libm.so` etc.
>
> [1] I think all of this can be done. For example, we can set all this up
> when building a distribution. However, I am not sure this is worth it when
> we know this is a transient phase. Soon, when LLVM libc is complete enough,
> a more appropriate option would be the one which allows choosing a libc as
> Eric pointed out.
>

To be clear I'm not a fan of a "pick your libc" option as opposed to just
naming the compiled llvm libc as perhaps libc.[a,so,etc] similar to other
platforms. I think we'd need a good reason to diverge here.

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

[PATCH] D98583: [ASTMatchers] Fix documentation for hasAnyBody matcher

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: baloghadamsoftware, aaron.ballman.
Herald added a subscriber: rnkovacs.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Looks like a oversight when the matcher was added.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98583

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h


Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -5217,7 +5217,7 @@
 ///   void f() {}
 ///   void g();
 /// \endcode
-/// hasAnyBody(functionDecl())
+/// functionDecl(hasAnyBody(compoundStmt()))
 ///   matches both 'void f();'
 ///   and 'void f() {}'
 /// with compoundStmt()
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -7509,7 +7509,7 @@
   void f();
   void f() {}
   void g();
-hasAnyBody(functionDecl())
+functionDecl(hasAnyBody(compoundStmt()))
   matches both 'void f();'
   and 'void f() {}'
 with compoundStmt()


Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -5217,7 +5217,7 @@
 ///   void f() {}
 ///   void g();
 /// \endcode
-/// hasAnyBody(functionDecl())
+/// functionDecl(hasAnyBody(compoundStmt()))
 ///   matches both 'void f();'
 ///   and 'void f() {}'
 /// with compoundStmt()
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -7509,7 +7509,7 @@
   void f();
   void f() {}
   void g();
-hasAnyBody(functionDecl())
+functionDecl(hasAnyBody(compoundStmt()))
   matches both 'void f();'
   and 'void f() {}'
 with compoundStmt()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89909: [SYCL] Implement SYCL address space attributes handling

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D89909#2623887 , @Anastasia wrote:

> In D89909#2623250 , @aaron.ballman 
> wrote:
>
>> In D89909#2623211 , @Anastasia 
>> wrote:
>>
>>> In D89909#2617194 , @bader wrote:
>>>
 @Anastasia, do you suggest we copy 
 https://github.com/intel/llvm/blob/sycl/sycl/doc/CompilerAndRuntimeDesign.md
  document to clang/docs within this patch?
>>>
>>> For the purpose of this specific topic, you could indeed move "Address 
>>> spaces handling" section into the clang documentation. I would suggest 
>>> creating a dedicated page where you could gather SYCL specific internals 
>>> for the clang community. You could also link the github page to it for more 
>>> comprehensive details.
>>>
>>> I would recommend extending the documentation slightly to focus on what 
>>> behavior is expected to be implemented rather than how you propose to 
>>> implement it in clang too. This is a general guideline for the clang 
>>> contributions that suggest that the documentation should be detailed such 
>>> that it would be possible to implement it in other frontend/compiler. You 
>>> could for example include some more information on expected language 
>>> semantic i.e. what you inherit from embedded C and what you inherent from 
>>> OpenCL or any other available language features with relevant spec/doc 
>>> references including SYCL spec. This should facilitate anyone who needs to 
>>> understand the implementation to find further details.
>>>
>>> It would probably make sense to create a separate review to avoid adding 
>>> too much noise here. Once you create a review we can link it here and also 
>>> refine any necessary details as we go along.
>>
>> These all seem like good suggestions, from my limited perspective, so thank 
>> you for them! I'm not opposed to the documentation requests, however, I 
>> think that is work that is separable from the proposed changes in this 
>> review and could be done with post-commit follow-ups, unless you think the 
>> current documentation is wrong as it relates to this patch. I worry that 
>> back-and-forth on documentation clarity (while certainly important) is going 
>> to hold this patch up for several more months, which is a burden.
>
> Just to be clear I am not suggesting a nice-to-have documentation. I would 
> like to see the documentation that explains what behavior is expected from 
> the compiler. I think this is normally a good starting point and it has been 
> a common practice for many contributions I believe too.

If what you're suggesting is that Clang have a SYCL-specific page that serves a 
similar purpose to https://clang.llvm.org/docs/OpenCLSupport.html, then I agree 
we should have that (and it should be linked to from 
https://clang.llvm.org/docs/index.html the same as OpenCL, OpenMP, etc). 
However, I think that's orthogonal to this patch and should be done separately. 
(If the documentation already existed, then I'd request that this patch update 
it, but because those docs don't exist yet, I think it's unrelated enough to 
warrant being done separately.)

If you're requesting something different, I'd appreciate a bit more specific 
details.

> I guess we could first commit the implementation and then work out what we 
> want it to be doing but that implies a bigger risks to the community.

Given that we lack the dedicated documentation page for SYCL within Clang, I 
guess I view that as the status quo (we've already started some of the 
upstreaming work and none of it is documented in a convenient place for Clang 
users). Based on that, as someone who doesn't really do much with SYCL to date, 
I think it's reasonable to accept this patch because it extends existing 
functionality in a straight-forward manner with a reasonable explanation as to 
why.

> Also I am also not entirely happy with some directions taken deviating the 
> original design in this patch.

If there are technical concerns to be addressed, I'm not certain it's clear (to 
me, at least) what they are.

> But it is hard to assess whether there are better practical alternatives and 
> suggest anything without understanding what is being implemented.

That's reasonable, but I think we also have to trust the domain experts who are 
submitting patches to have done their design homework and to be willing to 
address concerns as they come up when there's concretely a problem. This is not 
an extremely experimental invention from an unknown lone contributor; this is a 
long-time maintainer upstreaming work from downstream repo with users who use 
this functionality.

> Address spaces are used by many stakeholders in clang so having a clear 
> understanding of various use cases publicly accessible would benefit the 
> community and reduce the risks of disruptions and negative impact of each 
> other's work.

[PATCH] D98574: [Sparc] Define the same macros for -mcpu=v9 as GCC on Linux

[John Paul Adrian Glaubitz via Phabricator via cfe-commits]

glaubitz created this revision.
glaubitz added reviewers: jrtc27, ro, efriedma, brad, jfb, venkatra, jyknight.
Herald added a subscriber: fedor.sergeev.
glaubitz requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When targeting SPARC V8+ or V9 on Linux, GCC only defines the macro
__sparc_v9__ while clang also defines additional macros such as
__sparcv9 that are used on Solaris and the BSDs only. Make sure,
clang behaves as GCC on Linux and defines __sparc_v9__ only to avoid
compatibility problems.

  

Fixes PR49562


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98574

Files:
  clang/lib/Basic/Targets/Sparc.cpp


Index: clang/lib/Basic/Targets/Sparc.cpp
===
--- clang/lib/Basic/Targets/Sparc.cpp
+++ clang/lib/Basic/Targets/Sparc.cpp
@@ -156,9 +156,12 @@
   Builder.defineMacro("__sparcv8__");
   break;
 case CG_V9:
-  Builder.defineMacro("__sparcv9");
-  Builder.defineMacro("__sparcv9__");
   Builder.defineMacro("__sparc_v9__");
+  // Linux doesn't need these variants, but the BSDs do.
+  if (getTriple().getOS() != llvm::Triple::Linux) {
+Builder.defineMacro("__sparcv9");
+Builder.defineMacro("__sparcv9__");
+  }
   break;
 }
   }
@@ -239,13 +242,17 @@
 void SparcV9TargetInfo::getTargetDefines(const LangOptions ,
  MacroBuilder ) const {
   SparcTargetInfo::getTargetDefines(Opts, Builder);
-  Builder.defineMacro("__sparcv9");
   Builder.defineMacro("__arch64__");
-  // Solaris doesn't need these variants, but the BSDs do.
-  if (getTriple().getOS() != llvm::Triple::Solaris) {
-Builder.defineMacro("__sparc64__");
+  if (getTriple().getOS() == llvm::Triple::Linux) {
 Builder.defineMacro("__sparc_v9__");
-Builder.defineMacro("__sparcv9__");
+  } else {
+Builder.defineMacro("__sparcv9");
+// Solaris doesn't need these variants, but the BSDs do.
+if (getTriple().getOS() != llvm::Triple::Solaris) {
+  Builder.defineMacro("__sparc64__");
+  Builder.defineMacro("__sparc_v9__");
+  Builder.defineMacro("__sparcv9__");
+}
   }
 
   Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_1");


Index: clang/lib/Basic/Targets/Sparc.cpp
===
--- clang/lib/Basic/Targets/Sparc.cpp
+++ clang/lib/Basic/Targets/Sparc.cpp
@@ -156,9 +156,12 @@
   Builder.defineMacro("__sparcv8__");
   break;
 case CG_V9:
-  Builder.defineMacro("__sparcv9");
-  Builder.defineMacro("__sparcv9__");
   Builder.defineMacro("__sparc_v9__");
+  // Linux doesn't need these variants, but the BSDs do.
+  if (getTriple().getOS() != llvm::Triple::Linux) {
+Builder.defineMacro("__sparcv9");
+Builder.defineMacro("__sparcv9__");
+  }
   break;
 }
   }
@@ -239,13 +242,17 @@
 void SparcV9TargetInfo::getTargetDefines(const LangOptions ,
  MacroBuilder ) const {
   SparcTargetInfo::getTargetDefines(Opts, Builder);
-  Builder.defineMacro("__sparcv9");
   Builder.defineMacro("__arch64__");
-  // Solaris doesn't need these variants, but the BSDs do.
-  if (getTriple().getOS() != llvm::Triple::Solaris) {
-Builder.defineMacro("__sparc64__");
+  if (getTriple().getOS() == llvm::Triple::Linux) {
 Builder.defineMacro("__sparc_v9__");
-Builder.defineMacro("__sparcv9__");
+  } else {
+Builder.defineMacro("__sparcv9");
+// Solaris doesn't need these variants, but the BSDs do.
+if (getTriple().getOS() != llvm::Triple::Solaris) {
+  Builder.defineMacro("__sparc64__");
+  Builder.defineMacro("__sparc_v9__");
+  Builder.defineMacro("__sparcv9__");
+}
   }
 
   Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_1");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev updated this revision to Diff 330428.
v.g.vassilev added a comment.

Formatting.


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

https://reviews.llvm.org/D96033

Files:
  clang/include/clang/CodeGen/CodeGenAction.h
  clang/include/clang/Frontend/FrontendAction.h
  clang/include/clang/Interpreter/Interpreter.h
  clang/include/clang/Interpreter/Transaction.h
  clang/lib/CMakeLists.txt
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/IncrementalExecutor.cpp
  clang/lib/Interpreter/IncrementalExecutor.h
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/test/CMakeLists.txt
  clang/test/Interpreter/execute.cpp
  clang/test/Interpreter/sanity.c
  clang/test/lit.cfg.py
  clang/tools/CMakeLists.txt
  clang/tools/clang-repl/CMakeLists.txt
  clang/tools/clang-repl/ClangRepl.cpp
  clang/unittests/CMakeLists.txt
  clang/unittests/CodeGen/CMakeLists.txt
  clang/unittests/CodeGen/IncrementalProcessingTest.cpp
  clang/unittests/Interpreter/CMakeLists.txt
  clang/unittests/Interpreter/IncrementalProcessingTest.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp

Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- /dev/null
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -0,0 +1,120 @@
+//===- unittests/Interpreter/InterpreterTest.cpp --- Interpreter tests ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// Unit tests for Clang's Interpreter library.
+//
+//===--===//
+
+#include "clang/Interpreter/Interpreter.h"
+
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclGroup.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
+
+#include "llvm/ADT/ArrayRef.h"
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+
+namespace {
+using Args = std::vector;
+static std::unique_ptr
+createInterpreter(const Args  = {},
+  DiagnosticConsumer *Client = nullptr) {
+  Args ClangArgs = {"-Xclang", "-emit-llvm-only"};
+  ClangArgs.insert(ClangArgs.end(), ExtraArgs.begin(), ExtraArgs.end());
+  auto CI = cantFail(clang::IncrementalCompilerBuilder::create(ClangArgs));
+  if (Client)
+CI->getDiagnostics().setClient(Client, /*ShouldOwnClient=*/false);
+  return cantFail(clang::Interpreter::create(std::move(CI)));
+}
+
+TEST(InterpreterTest, Sanity) {
+  std::unique_ptr Interp = createInterpreter();
+  Transaction (cantFail(Interp->Parse("void g(); void g() {}")));
+  EXPECT_EQ(2U, R1.Decls.size());
+
+  Transaction (cantFail(Interp->Parse("int i;")));
+  EXPECT_EQ(1U, R2.Decls.size());
+}
+
+static std::string DeclToString(DeclGroupRef DGR) {
+  return llvm::cast(DGR.getSingleDecl())->getQualifiedNameAsString();
+}
+
+TEST(InterpreterTest, IncrementalInputTopLevelDecls) {
+  std::unique_ptr Interp = createInterpreter();
+  auto R1OrErr = Interp->Parse("int var1 = 42; int f() { return var1; }");
+  // gtest doesn't expand into explicit bool conversions.
+  EXPECT_TRUE(!!R1OrErr);
+  auto R1 = R1OrErr->Decls;
+  EXPECT_EQ(2U, R1.size());
+  EXPECT_EQ("var1", DeclToString(R1[0]));
+  EXPECT_EQ("f", DeclToString(R1[1]));
+
+  auto R2OrErr = Interp->Parse("int var2 = f();");
+  EXPECT_TRUE(!!R2OrErr);
+  auto R2 = R2OrErr->Decls;
+  EXPECT_EQ(1U, R2.size());
+  EXPECT_EQ("var2", DeclToString(R2[0]));
+}
+
+TEST(InterpreterTest, Errors) {
+  Args ExtraArgs = {"-Xclang", "-diagnostic-log-file", "-Xclang", "-"};
+
+  // Create the diagnostic engine with unowned consumer.
+  std::string DiagnosticOutput;
+  llvm::raw_string_ostream DiagnosticsOS(DiagnosticOutput);
+  auto DiagPrinter = std::make_unique(
+  DiagnosticsOS, new DiagnosticOptions());
+
+  auto Interp = createInterpreter(ExtraArgs, DiagPrinter.get());
+  auto Err = Interp->Parse("intentional_error v1 = 42; ").takeError();
+  using ::testing::HasSubstr;
+  EXPECT_THAT(DiagnosticsOS.str(),
+  HasSubstr("error: unknown type name 'intentional_error'"));
+  EXPECT_EQ("Parsing failed.", llvm::toString(std::move(Err)));
+
+  EXPECT_DEATH((void)Interp->Parse("int var1 = 42;"), "");
+}
+
+// Here we test whether the user can mix declarations and statements. The
+// interpreter should be smart enough to recognize the declarations from the
+// statements and wrap the latter into a declaration, producing valid code.
+TEST(InterpreterTest, DeclsAndStatements) {
+  Args ExtraArgs = {"-Xclang", "-diagnostic-log-file", "-Xclang", "-"};
+
+  // Create the diagnostic engine with