[PATCH] D114425: [clang] Add __builtin_bswap128

[Nikolas Klauser via Phabricator via cfe-commits]

philnik abandoned this revision.
philnik added a comment.

I don't plan to work on this any time soon, so I'll abandon it to clean up the 
review queue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114425

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

[PATCH] D157572: [clang] Add `[[clang::library_extension]]` attribute

[Nikolas Klauser via Phabricator via cfe-commits]

philnik abandoned this revision.
philnik added a comment.

Abandoning, since it seems like we want to extend `diagnose_if` instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157572

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

[PATCH] D155475: [Clang][Sema] Add -Wctad-selects-copy to diagnose copy deduction candidate

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:10932
+DiagnoseCTADCopy();
+  } else if (!PP.getSourceManager().isInSystemHeader(Template->getLocation())) 
{
+DiagnoseCTADCopy();

cor3ntin wrote:
> So the idea here is that the standard library would have protected against 
> that by providing deduction guides?
> Which is funny because the original issue was reported because they did not.
> Do you have an example of when we should not emit the diagnostic for a 
> library type?
> 
> @ldionne @philnik Opinions on this warning?
I think this definitely has to be suppressed in system headers somehow. e.g. 
`as_rvalue_view` uses the copy ctor CTAD to avoid double-wrapping iterators. 
(see 
https://github.com/llvm/llvm-project/blob/main/libcxx/include/__ranges/as_rvalue_view.h#L75).
 OTOH I'm not sure whether this should be suppressed if the type is from a 
system header, since `reverse_iterator{some-other-reverse-iterator}` might not 
be intended either, regardless of deduction guides.


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

https://reviews.llvm.org/D155475

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

[PATCH] D157572: [clang] Add `[[clang::library_extension]]` attribute

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D157572#4617504 , @aaron.ballman 
wrote:

> In D157572#4614561 , @cjdb wrote:
>
>> In D157572#4613622 , 
>> @aaron.ballman wrote:
>>
>>> In D157572#4612141 , @cjdb wrote:
>>>
 I don't dislike it, but I am a bit concerned about misuse being noisy.
>>>
>>> So you're concerned that a library author uses `diagnose_if` to add a 
>>> diagnostic to a warning group that makes the diagnostic seem too chatty, so 
>>> the user disables the group and loses the compiler's diagnostics? Or are 
>>> there other kinds of misuse you're worried about?
>>
>> More or less the former. I don't know if it'll actually manifest in practice 
>> though, I don't see many `diagnose_if` diagnostics to begin with.
>
> I think the former is sort of a "doctor, doctor, it hurts" situation; the 
> issue isn't really with `diagnose_if`, it's with the library author's use of 
> it. But at the same time, I can see why it would be beneficial to be able to 
> work around it if needed.
>
 As much as I hate suppressing diagnostics, I think there needs to be a way 
 to suppress the `diagnose_if` forms of warning without suppressing 
 something that the compiler would otherwise generate. Something like:

 - `-Wno-deprecated`: suppresses anything that `-Wdeprecated` would turn on.
 - `-Wno-deprecated=diagnose_if`: just the ones flagged by `diagnose_if`.
 - `-Wno-deprecated=non-diagnose_if`: complement to #2.

 (and similarly for `-Wno-error=`.)

 I'm not sure about the system header knob though: `[[deprecated]]` and 
 `[[nodiscard]]` still show up even when the declaration is in a system 
 header?
>>>
>>> Correct, those will still show up when the declaration is in a system 
>>> header but not when the use is in a system header: 
>>> https://godbolt.org/z/PjqKbGsrr
>>
>> Right, my question was more "what is this knob doing?"
>
> Ah! We have various knobs on our diagnostics, like:
> `ShowInSystemHeader` -- 
> https://github.com/llvm/llvm-project/blob/2dc6281b98d07f43a64d0ef34405d9a12d59e8b6/clang/include/clang/Basic/DiagnosticSemaKinds.td#L7818
>  (if used, the diagnostic will be shown in a system header)
> `SFINAEFailure` -- 
> https://github.com/llvm/llvm-project/blob/2dc6281b98d07f43a64d0ef34405d9a12d59e8b6/clang/include/clang/Basic/DiagnosticSemaKinds.td#L93
>  (if used, the diagnostic causes SFINAE to fail in a SFINAE context)
> `DefaultIgnore` -- 
> https://github.com/llvm/llvm-project/blob/2dc6281b98d07f43a64d0ef34405d9a12d59e8b6/clang/include/clang/Basic/DiagnosticSemaKinds.td#L141
>  (if used, the warning is off by default and must be explicitly enabled via 
> its group)
> `DefaultError` -- 
> https://github.com/llvm/llvm-project/blob/2dc6281b98d07f43a64d0ef34405d9a12d59e8b6/clang/include/clang/Basic/DiagnosticSemaKinds.td#L262
>  (if used, the warning defaults to being an error but users can disable the 
> error with `-Wno`)
> (etc)
>
> All of these have been of use to us as implementers, so it seems likely that 
> these same knobs would be of use to library authors adding their own compiler 
> diagnostics, so perhaps we should consider that as part of the design?
>
>>> We currently have `-Wuser-defined-warnings` as the warning group for 
>>> `diagnose_if` warning diagnostics, so I wonder if it would make sense to 
>>> allow `-Wno-deprecated` suppresses anything that `-Wdeprecated` would turn 
>>> on, while `-Wdeprecated -Wno-user-defined-warnings` would turn on only the 
>>> compiler-generated deprecation warnings and not the diagnose_if-generated 
>>> ones?
>>
>> Oh neat, this simplifies things a lot!
>
> I think it could make for a reasonable user experience.
>
> I'm curious what @erichkeane thinks as attributes code owner, but personally, 
> I like the idea of extending `diagnose_if` over the idea of adding 
> `clang::library_extension` because it's a more general solution that I think 
> will give more utility to our users. However, I also want to know if @philnik 
> @Mordante @ldionne (and others) share that preference because I think they're 
> going to be the guinea pigs^W^Wearly adopters of this functionality.

I think this approach makes a lot of sense. I'm not sure all the knobs make 
sense to expose (e.g. `SFINAEFailure` could just be implemented with a normal 
`enable_if` IIUC), but at least `ShowInSystemHeader` and `DefaultIgnore` are 
ones where I already have use-cases in mind. As said before, we could also move 
our `atomic` ordering warnings from `-Wuser-defined-warnings` to 
`-Watomic-memory-ordering`, which seems like a huge usability improvement. I'm 
sure there will be more use-cases in the future for extending warning flags and 
having the ability to tune how and when warnings are emitted.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://review

[PATCH] D157572: [clang] Add `[[clang::library_extension]]` attribute

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D157572#4606513 , @aaron.ballman 
wrote:

> In D157572#4604595 , @philnik wrote:
>
>> In D157572#4604482 , 
>> @aaron.ballman wrote:
>>
 This allows standard libraries to mark symbols as extensions, so the 
 compiler can generate extension warnings when they are used.
>>>
>>> Huh, so this is basically the opposite of the `__extension__` macro (which 
>>> is used to silence extension warnings)?
>>
>> I guess, kind-of. I never really understood the semantics of `__extension__` 
>> though, so I'm not 100% certain.
>
> It's used in system headers to say "I'm using an extension here, don't warn 
> about it in -pedantic mode".

Hm, OK. I thought I tried to use it that way at some point and it didn't work 
in the way I expected.

>>> I don't think we need to introduce a new attribute to do this, we already 
>>> have `diagnose_if`. e.g., https://godbolt.org/z/a5ae4T56o would that 
>>> suffice?
>>
>> Part of the idea here is that the diagnostics should be part of 
>> `-Wc++ab-extension`.
>
> Hmmm, okay. And I'm assuming `-Wsystem-headers -pedantic` is too chatty 
> because it's telling the user about all use of extensions, not extensions 
> being introduced by the library itself? (e.g., 
> https://godbolt.org/z/Gs3YGheMM is also not what you're after)

Yeah, for libc++ we don't support `-Wsystem-headers` in any meaningful way and 
this doesn't achieve the effect I want.

>> I guess we could allow warning flags instead of just `"warning"` and 
>> `"error"` in `diagnose_if` that specifies which warning group the diagnostic 
>> should be part of. Something like 
>> `__attribute__((__diagnose_if__(__cplusplus >= 201703L, "basic_string_view 
>> is a C++17 extension", "-Wc++17-extensions")))`. I'm not sure how one could 
>> implement that, but I guess there is some mechanism to translate 
>> "-Wwhatever" to a warning group, since you can push and pop warnings.  That 
>> would open people up to add a diagnostic to pretty much any warning group. I 
>> don't know if that's a good idea. I don't really see a problem with that 
>> other than people writing weird code, but people do that all the time 
>> anyways. Maybe I'm missing something really problematic though.
>
> That's actually a pretty interesting idea; `diagnose_if` could be given 
> another parameter to specify a diagnostic group to associate the diagnostic 
> with. This would let you do some really handy things like:
>
>   void func(int i) __attribute__((diagnose_if(i < 0, "passing a negative 
> value to 'func' is deprecated", "warning", "-Wdeprecated")));
>
> But if we went this route, would we want to expose other diagnostic-related 
> knobs like "show in system header" and "default to an error"? Also, the 
> attribute currently can only be associated with a function; we can use this 
> for classes by sticking it on a constructor but there's not much help for 
> putting it on say a namespace or an enumeration. So we may need to extend the 
> attribute in other ways. CC @cjdb as this seems of interest to you as well.

I looked a bit around the code yesterday and it looks like it should be 
relatively easy to implement. I think we would just have to extend 
`CustomDiagInfo` to save some more information and pass the warning group in 
there from the `diagnose_if` sema handler. There are already utilities to 
translate `-Wwhatever` into the corresponding warning group IDs, so that part 
is trivial. Given that, I think this sounds like a very interesting way to go. 
This would also allow us to move the libc++ `atomic` checks to 
`-Watomic-memory-ordering` and I'm sure there will be more use-cases. Regarding 
the knobs, I think that would be interesting for some things too. We could add 
optional string arguments at the end like 
`"show_in_system_header={false,true}"`, `"default_to_error={false,true}"`, 
`"enable_by_default={false,true}"`, etc. If they aren't set we probably want to 
default to `show_in_system_header=false`, `default_to_error=false` and 
`enable_by_default=true`. Are there any other interesting knobs?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157572

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

[PATCH] D157572: [clang] Add `[[clang::library_extension]]` attribute

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D157572#4604482 , @aaron.ballman 
wrote:

>> This allows standard libraries to mark symbols as extensions, so the 
>> compiler can generate extension warnings when they are used.
>
> Huh, so this is basically the opposite of the `__extension__` macro (which is 
> used to silence extension warnings)?

I guess, kind-of. I never really understood the semantics of `__extension__` 
though, so I'm not 100% certain.

> I don't think we need to introduce a new attribute to do this, we already 
> have `diagnose_if`. e.g., https://godbolt.org/z/a5ae4T56o would that suffice?

Part of the idea here is that the diagnostics should be part of 
`-Wc++ab-extension`. I guess we could allow warning flags instead of just 
`"warning"` and `"error"` in `diagnose_if` that specifies which warning group 
the diagnostic should be part of. Something like 
`__attribute__((__diagnose_if__(__cplusplus >= 201703L, "basic_string_view is a 
C++17 extension", "-Wc++17-extensions")))`. I'm not sure how one could 
implement that, but I guess there is some mechanism to translate "-Wwhatever" 
to a warning group, since you can push and pop warnings.  That would open 
people up to add a diagnostic to pretty much any warning group. I don't know if 
that's a good idea. I don't really see a problem with that other than people 
writing weird code, but people do that all the time anyways. Maybe I'm missing 
something really problematic though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157572

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

[PATCH] D155064: [clang][SemaCXX] Diagnose tautological uses of consteval if and is_constant_evaluated

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.



Comment at: libcxx/include/__type_traits/is_constant_evaluated.h:31
+  return false;
+#endif
 }

Mordante wrote:
> hazohelet wrote:
> > Mordante wrote:
> > > Why is this needed? Does this mean the builtin will fail in C++03 mode?
> > `__libcpp_is_constant_evaluated` always returns false under C++03 or 
> > earlier, where constexpr isn't available. This is a fix to run libc++ tests 
> > without seeing warnings from clang with this patch.
> > (Live demo: https://godbolt.org/z/oszebM5o5)
> I see can you add a comment in that regard? To me it looks like the builtin 
> fails in C++03, where returning false indeed is always the right answer.
https://godbolt.org/z/bafeeY7b7 shows that `__builtin_is_constant_evaluated()` 
doesn't always return false in C++03, so this change seems wrong to me.


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

https://reviews.llvm.org/D155064

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

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

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

I think these changes would allow us to drop the `stddef.h` compat header from 
libc++, which would be really nice.




Comment at: clang/lib/Headers/stddef.h:36-40
+#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L) ||  
\
+(defined(__cplusplus) && defined(_MSC_EXTENSIONS) &&   
\
+ defined(_NATIVE_NULLPTR_SUPPORTED))
+#define __need_nullptr_t
+#endif





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




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157757

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

[PATCH] D157572: [clang] Add `[[clang::library_extension]]` attribute

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5690-5691
+def warn_unknown_ext : Warning<"Unknown extension kind: %0">;
+def warn_cxx11_ext : Warning<"%0 is a C++11 extension">,
+InGroup;
+def warn_cxx14_ext : Warning<"%0 is a C++14 extension">,

cjdb wrote:
> We shouldn't need this one, since Clang (almost) doesn't distinguish between 
> C++03 and C++11. To my knowledge, C++03 doesn't even support attributes, so 
> this would be a moot addition.
It does enough to regularly cause headaches (no `>>`, `constexpr`, `alignas`, 
buggy `enum class`). C++03 has supported GNU attributes forever, and since LLVM 
17 also supports C++11 attributes. Also note that this isn't a new warning.



Comment at: clang/test/SemaCXX/attr-library-extension.cpp:9-13
+#ifdef GNUAttr
+#define EXTENSION(name) __attribute__((library_extension(name)))
+#else
+#define EXTENSION(name) [[clang::library_extension(name)]]
+#endif

cjdb wrote:
> I don't think we need to test for both of these: using 
> `[[clang::library_extension]]` directly should suffice.
I've been asked to test both before, and IMO it's a good idea, since there are 
attributes which only have one of the spellings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157572

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

[PATCH] D157572: [clang] Add `[[clang::library_extension]]` attribute

[Nikolas Klauser via Phabricator via cfe-commits]

philnik created this revision.
philnik added reviewers: aaron.ballman, erichkeane.
Herald added a subscriber: arphaman.
Herald added a project: All.
philnik requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This allows standard libraries to mark symbols as extensions, so the compiler 
can generate extension warnings when they are used.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157572

Files:
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/DeclBase.cpp
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/SemaAvailability.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaCXX/attr-library-extension.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -8280,6 +8280,7 @@
   switch (D->getAvailability()) {
   case AR_Available:
   case AR_NotYetIntroduced:
+  case AR_Extension:
 if (const EnumConstantDecl *EnumConst = dyn_cast(D))
   return getCursorAvailabilityForDecl(
   cast(EnumConst->getDeclContext()));
Index: clang/test/SemaCXX/attr-library-extension.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-library-extension.cpp
@@ -0,0 +1,130 @@
+// RUN: %clang_cc1 -Wno-unused-value -std=c++03 -verify=cxx03,cxx11,cxx14,cxx17,cxx20,cxx23,gnu -fsyntax-only %s -DGNUAttr
+// RUN: %clang_cc1 -Wno-unused-value -std=c++11 -verify=cxx11,cxx14,cxx17,cxx20,cxx23,gnu -fsyntax-only %s
+// RUN: %clang_cc1 -Wno-unused-value -std=c++14 -verify=cxx14,cxx17,cxx20,cxx23,gnu -fsyntax-only %s
+// RUN: %clang_cc1 -Wno-unused-value -std=c++17 -verify=cxx17,cxx20,cxx23,gnu -fsyntax-only %s
+// RUN: %clang_cc1 -Wno-unused-value -std=c++20 -verify=cxx20,cxx23,gnu -fsyntax-only %s
+// RUN: %clang_cc1 -Wno-unused-value -std=c++23 -verify=cxx23,gnu -fsyntax-only %s
+// RUN: %clang_cc1 -Wno-unused-value -std=c++26 -verify=gnu -fsyntax-only %s
+
+#ifdef GNUAttr
+#define EXTENSION(name) __attribute__((library_extension(name)))
+#else
+#define EXTENSION(name) [[clang::library_extension(name)]]
+#endif
+
+struct EXTENSION("C++11") StructCxx11Ext {}; // cxx03-note {{'StructCxx11Ext' has been explicitly marked as an extension here}}
+struct EXTENSION("C++14") StructCxx14Ext {}; // cxx11-note {{'StructCxx14Ext' has been explicitly marked as an extension here}}
+struct EXTENSION("C++17") StructCxx17Ext {}; // cxx14-note {{'StructCxx17Ext' has been explicitly marked as an extension here}}
+struct EXTENSION("C++20") StructCxx20Ext {}; // cxx17-note {{'StructCxx20Ext' has been explicitly marked as an extension here}}
+struct EXTENSION("C++23") StructCxx23Ext {}; // cxx20-note {{'StructCxx23Ext' has been explicitly marked as an extension here}}
+struct EXTENSION("C++26") StructCxx26Ext {}; // cxx23-note {{'StructCxx26Ext' has been explicitly marked as an extension here}}
+struct EXTENSION("GNU") GNUExt {}; // gnu-note {{'GNUExt' has been explicitly marked as an extension here}}
+
+void consume(StructCxx11Ext); // cxx03-warning {{'StructCxx11Ext' is a C++11 extension}}
+void consume(StructCxx14Ext); // cxx11-warning {{'StructCxx14Ext' is a C++14 extension}}
+void consume(StructCxx17Ext); // cxx14-warning {{'StructCxx17Ext' is a C++17 extension}}
+void consume(StructCxx20Ext); // cxx17-warning {{'StructCxx20Ext' is a C++20 extension}}
+void consume(StructCxx23Ext); // cxx20-warning {{'StructCxx23Ext' is a C++23 extension}}
+void consume(StructCxx26Ext); // cxx23-warning {{'StructCxx26Ext' is a C++2c extension}}
+void consume(GNUExt); // gnu-warning {{'GNUExt' is a GNU extension}}
+
+namespace EXTENSION("C++11") NSCxx11Ext { // cxx03-note {{'NSCxx11Ext' has been explicitly marked as an extension here}}
+  struct S {};
+}
+void consume(NSCxx11Ext::S); // cxx03-warning {{'NSCxx11Ext' is a C++11 extension}}
+
+namespace EXTENSION("C++14") NSCxx14Ext { // cxx11-note {{'NSCxx14Ext' has been explicitly marked as an extension here}}
+  struct S {};
+}
+void consume(NSCxx14Ext::S); // cxx11-warning {{'NSCxx14Ext' is a C++14 extension}}
+
+namespace EXTENSION("C++17") NSCxx17Ext { // cxx14-note {{'NSCxx17Ext' has been explicitly marked as an extension here}}
+  struct S {};
+}
+void consume(NSCxx17Ext::S); // cxx14-warning {{'NSCxx17Ext' is a C++17 extension}}
+
+namespace EXTENSION("C++20") NSCxx20Ext { // cxx17-note {{'NSCxx20Ext' has been explicitly marked as an extension here}}
+  struct S {};
+}
+void consume(NSCxx20Ext::S); // cxx17-warning {{'NSCxx20Ext' is a C++20 extension}}
+
+namespace EXTENSION("C++23") NSCxx23Ext { // cxx20-note {{'NSCxx23Ext' has been explicitly marked as an extension here}}
+  struct S {};
+}
+void consume(NSCxx23Ext::S); // cxx20-warning {

[PATCH] D157324: [clang] Move the Clang CI jobs off of the libc++ builders

[Nikolas Klauser via Phabricator via cfe-commits]

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

Thanks for working on this! I think it would make a lot of sense to unify this 
build and the Debian build,  since currently we are building Clang twice 
without a good reason.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157324

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

[PATCH] D155809: [NFC] [Clang] Fix strict weak ordering in ItaniumVTableBuilder

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

@aaron.ballman I think what @danlark means is that when building clang against 
a libc++ which has debug assertions enabled, the clang tests he mentioned 
result in an assertion firing within libc++. i.e. the libc++ debug mode catches 
the non-strict weak ordering that it gets from the clang code. That's why he 
can't just add a test that fires the assertion. Currently, there are no bots 
that build clang against a libc++ with debug assertions enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155809

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

[PATCH] D156247: [Clang] Add a warning on uses of coroutine keywords

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D156247#4538626 , @aaron.ballman 
wrote:

> I would like explicit buy-in from the libc++ folks on the idea of adding 
> `-fno-coroutines` as they're going to be impacted by Clang supporting such an 
> option (and they may have additional testing requirements to ensure that 
> libc++ works reasonably well when coroutines are disabled).

I don't think we want to support having another flag for a dialect, since that 
will result in more problems down the road. We already have a problem with the 
number of configurations we support and adding style-guide configurations won't 
help that. IMO this should just be a clang-tidy check. They can be enforced 
just as well as compiler warnings/errors, and don't cause problems for other 
people.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156247

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

[PATCH] D152003: [clang] Fix `static_cast` to array of unknown bound

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D152003#4531732 , @aaron.ballman 
wrote:

> In D152003#4531404 , @Fznamznon 
> wrote:
>
>>> should we try to land that today?
>>
>> I'm not sure. It causes failures in libc++ testing:
>>
>>   Failed Tests (3):
>> llvm-libc++-shared.cfg.in :: std/ranges/range.access/end.pass.cpp
>> llvm-libc++-shared.cfg.in :: std/ranges/range.access/rbegin.pass.cpp
>> llvm-libc++-shared.cfg.in :: std/ranges/range.access/rend.pass.cpp
>>
>> I haven't figured out why. Trying to compile:
>>
>>   #include 
>>   
>>   using RangeCREndT = decltype(std::ranges::crend);
>>   static_assert(!std::is_invocable_v);
>>
>> fails with this patch only using libc++, libstdc++ is fine. I'm not sure it 
>> is my misunderstanding, bug in the patch or bug in libc++.
>
> CC @ldionne @philnik @Mordante for libc++ opinions

Thanks for the heads-up! From what I can tell, this could be a problem in the 
compiler, in `is_invocable`, in `cend` or in `end`. Unfortunately, I don't have 
the time right now to make a smaller reproducer and Louis isn't available. 
@Mordante do you have time to take a look at this? I will probably only have 
time to get to it by the end of the week or early next week.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152003

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

[PATCH] D154893: [Clang] Fix some triviality computations

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.



Comment at: clang/include/clang/AST/DeclCXX.h:1269
   /// Determine whether this class has a non-trivial copy constructor
   /// (C++ [class.copy]p6, C++11 [class.copy]p12)
   bool hasNonTrivialCopyConstructor() const {

royjacobson wrote:
> shafik wrote:
> > These references looks like they need to be updated. Same below and it 
> > looks like `hasNonTrivialCopyConstructorForCall` is missing references all 
> > together.
> TBH I don't think those functions actually need references to the standard? 
> Whether the actual member functions are trivial or not is already calculated 
> before. Do you think I can just remove it? :)
I think it makes sense to call out that these functions represent something 
from the standard. There are other functions with similar names, which don't 
have an equivalent in the C++ standard, like `hasTrivialDestructorForCall`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154893

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

[PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

[Nikolas Klauser via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG874217f99b99: [clang] Enable C++11-style attributes in all 
language modes (authored by philnik).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151683

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/Features.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Parse/Parser.h
  clang/lib/Basic/Attributes.cpp
  clang/lib/Lex/Lexer.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/AST/ast-dump-attr.m
  clang/test/AST/ast-dump-c-attr.c
  clang/test/AST/attr-annotate-type.c
  clang/test/CodeGen/attr-btf_type_tag-func.c
  clang/test/CodeGen/attr-btf_type_tag-var.c
  clang/test/Frontend/noderef.c
  clang/test/OpenMP/assumes_messages_attr.c
  clang/test/OpenMP/openmp_attribute_compat.cpp
  clang/test/Parser/asm.c
  clang/test/Parser/c2x-attributes.c
  clang/test/Parser/c2x-attributes.m
  clang/test/Parser/cxx-decl.cpp
  clang/test/Parser/objc-attr.m
  clang/test/ParserHLSL/group_shared.hlsl
  clang/test/Preprocessor/has_c_attribute.c
  clang/test/Sema/annotate-type.c
  clang/test/Sema/annotate.c
  clang/test/Sema/attr-availability-square-brackets.c
  clang/test/Sema/attr-external-source-symbol-cxx.cpp
  clang/test/Sema/attr-external-source-symbol.c
  clang/test/Sema/attr-likelihood.c
  clang/test/Sema/attr-objc-bridge-related.m
  clang/test/Sema/attr-regparm.c
  clang/test/Sema/attr-type-safety.c
  clang/test/Sema/c2x-attr.c
  clang/test/Sema/c2x-noreturn.c
  clang/test/Sema/internal_linkage.c
  clang/test/Sema/matrix-type-builtins.c
  clang/test/Sema/neon-vector-types.c
  clang/test/Sema/overload-arm-mve.c
  clang/test/Sema/overloadable.c
  clang/test/Sema/vector-gcc-compat.c
  clang/test/SemaCXX/attr-cxx-disabled.cpp
  clang/test/SemaCXX/cxx98-compat.cpp
  clang/test/SemaCXX/warn-c++11-extensions.cpp
  clang/test/SemaObjC/attr-objc-gc.m
  clang/unittests/AST/AttrTest.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -575,7 +575,6 @@
 // FIXME: We should ask the driver for the appropriate default flags.
 lang_opts.GNUMode = true;
 lang_opts.GNUKeywords = true;
-lang_opts.DoubleSquareBracketAttributes = true;
 lang_opts.CPlusPlus11 = true;
 
 // The Darwin libc expects this macro to be set.
Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3394,14 +3394,10 @@
   // If this is the C++11 variety, also add in the LangOpts test.
   if (Variety == "CXX11")
 Test += " && LangOpts.CPlusPlus11";
-  else if (Variety == "C2x")
-Test += " && LangOpts.DoubleSquareBracketAttributes";
 } else if (Variety == "CXX11")
   // C++11 mode should be checked against LangOpts, which is presumed to be
   // present in the caller.
   Test = "LangOpts.CPlusPlus11";
-else if (Variety == "C2x")
-  Test = "LangOpts.DoubleSquareBracketAttributes";
 
 std::string TestStr = !Test.empty()
   ? Test + " ? " + llvm::itostr(Version) + " : 0"
Index: clang/unittests/AST/AttrTest.cpp
===
--- clang/unittests/AST/AttrTest.cpp
+++ clang/unittests/AST/AttrTest.cpp
@@ -157,7 +157,7 @@
   AST = buildASTFromCodeWithArgs(R"c(
 __auto_type [[clang::annotate_type("auto")]] auto_var = 1;
   )c",
- {"-fdouble-square-bracket-attributes"},
+ {},
  "input.c");
 
   {
Index: clang/test/SemaObjC/attr-objc-gc.m
===
--- clang/test/SemaObjC/attr-objc-gc.m
+++ clang/test/SemaObjC/attr-objc-gc.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -fdouble-square-bracket-attributes -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify %s
 static id __attribute((objc_gc(weak))) a;
 static id __attribute((objc_gc(strong))) b;
 
Index: clang/test/SemaCXX/warn-c++11-extensions.cpp
===
--- clang/test/SemaCXX/warn-c++11-extensions.cpp
+++ clang/test/SemaCXX/warn-c++11-extensions.cpp
@@ -7,3 +7,5 @@
 
 enum struct E1 { A, B }; // expected-warning {{scoped enumerations

[PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 542743.
philnik added a comment.

- Address comments
- Try to fix CI
- Rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151683

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/Features.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Parse/Parser.h
  clang/lib/Basic/Attributes.cpp
  clang/lib/Lex/Lexer.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/AST/ast-dump-attr.m
  clang/test/AST/ast-dump-c-attr.c
  clang/test/AST/attr-annotate-type.c
  clang/test/CodeGen/attr-btf_type_tag-func.c
  clang/test/CodeGen/attr-btf_type_tag-var.c
  clang/test/Frontend/noderef.c
  clang/test/OpenMP/assumes_messages_attr.c
  clang/test/OpenMP/openmp_attribute_compat.cpp
  clang/test/Parser/asm.c
  clang/test/Parser/c2x-attributes.c
  clang/test/Parser/c2x-attributes.m
  clang/test/Parser/cxx-decl.cpp
  clang/test/Parser/objc-attr.m
  clang/test/ParserHLSL/group_shared.hlsl
  clang/test/Preprocessor/has_c_attribute.c
  clang/test/Sema/annotate-type.c
  clang/test/Sema/annotate.c
  clang/test/Sema/attr-availability-square-brackets.c
  clang/test/Sema/attr-external-source-symbol-cxx.cpp
  clang/test/Sema/attr-external-source-symbol.c
  clang/test/Sema/attr-likelihood.c
  clang/test/Sema/attr-objc-bridge-related.m
  clang/test/Sema/attr-regparm.c
  clang/test/Sema/attr-type-safety.c
  clang/test/Sema/c2x-attr.c
  clang/test/Sema/c2x-noreturn.c
  clang/test/Sema/internal_linkage.c
  clang/test/Sema/matrix-type-builtins.c
  clang/test/Sema/neon-vector-types.c
  clang/test/Sema/overload-arm-mve.c
  clang/test/Sema/overloadable.c
  clang/test/Sema/vector-gcc-compat.c
  clang/test/SemaCXX/attr-cxx-disabled.cpp
  clang/test/SemaCXX/cxx98-compat.cpp
  clang/test/SemaCXX/warn-c++11-extensions.cpp
  clang/test/SemaObjC/attr-objc-gc.m
  clang/unittests/AST/AttrTest.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -575,7 +575,6 @@
 // FIXME: We should ask the driver for the appropriate default flags.
 lang_opts.GNUMode = true;
 lang_opts.GNUKeywords = true;
-lang_opts.DoubleSquareBracketAttributes = true;
 lang_opts.CPlusPlus11 = true;
 
 // The Darwin libc expects this macro to be set.
Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3394,14 +3394,10 @@
   // If this is the C++11 variety, also add in the LangOpts test.
   if (Variety == "CXX11")
 Test += " && LangOpts.CPlusPlus11";
-  else if (Variety == "C2x")
-Test += " && LangOpts.DoubleSquareBracketAttributes";
 } else if (Variety == "CXX11")
   // C++11 mode should be checked against LangOpts, which is presumed to be
   // present in the caller.
   Test = "LangOpts.CPlusPlus11";
-else if (Variety == "C2x")
-  Test = "LangOpts.DoubleSquareBracketAttributes";
 
 std::string TestStr = !Test.empty()
   ? Test + " ? " + llvm::itostr(Version) + " : 0"
Index: clang/unittests/AST/AttrTest.cpp
===
--- clang/unittests/AST/AttrTest.cpp
+++ clang/unittests/AST/AttrTest.cpp
@@ -157,7 +157,7 @@
   AST = buildASTFromCodeWithArgs(R"c(
 __auto_type [[clang::annotate_type("auto")]] auto_var = 1;
   )c",
- {"-fdouble-square-bracket-attributes"},
+ {},
  "input.c");
 
   {
Index: clang/test/SemaObjC/attr-objc-gc.m
===
--- clang/test/SemaObjC/attr-objc-gc.m
+++ clang/test/SemaObjC/attr-objc-gc.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -fdouble-square-bracket-attributes -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify %s
 static id __attribute((objc_gc(weak))) a;
 static id __attribute((objc_gc(strong))) b;
 
Index: clang/test/SemaCXX/warn-c++11-extensions.cpp
===
--- clang/test/SemaCXX/warn-c++11-extensions.cpp
+++ clang/test/SemaCXX/warn-c++11-extensions.cpp
@@ -7,3 +7,5 @@
 
 enum struct E1 { A, B }; // expected-warning {{scoped enumerations are a C++11 extension}}
 enum class E2 { C, D }; // expected-warning {{scoped enumerations are a C++11 extension}}
+
+[[]] void 

[PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D151683#4508209 , @MaskRay wrote:

> Reverse ping @philnik :)
>
> The `release/17.x` branch will be created soon 
> (https://discourse.llvm.org/t/llvm-17-0-0-release-planning-and-update/71762).

I know. I just didn't have the time yet. If you want to make sure it's in 17 
feel free to commandeer and fix the last bits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151683

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

[PATCH] D155064: [clang][SemaCXX] Diagnose tautological uses of consteval if and is_constant_evaluated

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.



Comment at: 
libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/ranges.copy.segmented.pass.cpp:96
 int main(int, char**) {
-  if (!std::is_constant_evaluated()) {
-test_containers, std::deque>();

cor3ntin wrote:
> this is a funny one, what's the history of that?
Probably some code moving around. I think this was originally in another 
function.



Comment at: 
libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.verify.cpp:27
   static_assert(!std::is_constant_evaluated(), "");
-  // expected-warning@-1 0-1 {{'std::is_constant_evaluated' will always 
evaluate to 'true' in a manifestly constant-evaluated expression}}
+  // expected-warning@-1 0-1 {{'std::is_constant_evaluated' will always 
evaluate to true in this context}}
 #endif

cor3ntin wrote:
> Mordante wrote:
> > hazohelet wrote:
> > > philnik wrote:
> > > > Mordante wrote:
> > > > > Since libc++ support the latest ToT Clang and the last two official 
> > > > > releases this wont work. The `expected-warning` needs to be a 
> > > > > `expected-warning-re` that works for both the new and old diagnostic
> > > > You can also just shorten it to `'std::is_constant_evaluated' will 
> > > > always evaluate to`. Seems good enough to me.
> > > Thanks!
> > I really would like a regex. To me the current message misses an important 
> > piece of information; the `true` part. I care less about the rest of the 
> > message, but stripping the `true` means a warning like 
> > `std::is_constant_evaluated' will always evaluate to FALSE` would be valid 
> > too.
> Agreed with Mordante
We're not in the business of testing the compiler though. Taking a closer look, 
I'm not actually sure why this test exists at all. It doesn't seem like it 
tests anything useful w.r.t. the library. This has been added in 2fc5a78, but 
there the warning isn't checked, so that was clearly not the original intention.


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

https://reviews.llvm.org/D155064

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

[PATCH] D155064: [clang][SemaCXX] Diagnose tautological uses of consteval if and is_constant_evaluated

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.



Comment at: 
libcxx/test/std/utilities/meta/meta.const.eval/is_constant_evaluated.verify.cpp:27
   static_assert(!std::is_constant_evaluated(), "");
-  // expected-warning@-1 0-1 {{'std::is_constant_evaluated' will always 
evaluate to 'true' in a manifestly constant-evaluated expression}}
+  // expected-warning@-1 0-1 {{'std::is_constant_evaluated' will always 
evaluate to true in this context}}
 #endif

Mordante wrote:
> Since libc++ support the latest ToT Clang and the last two official releases 
> this wont work. The `expected-warning` needs to be a `expected-warning-re` 
> that works for both the new and old diagnostic
You can also just shorten it to `'std::is_constant_evaluated' will always 
evaluate to`. Seems good enough to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155064

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

[PATCH] D155078: [ci] Make libc++ and Clang CI scripts independent

[Nikolas Klauser via Phabricator via cfe-commits]

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

I like this a lot! LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155078

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

[PATCH] D153920: [clang] Move the clang formatting job to run-buildbot to fix the CI

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 539183.
philnik added a comment.
Herald added a reviewer: NoQ.

Try to fix CI


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153920

Files:
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/utils/ci/buildkite-pipeline.yml
  clang/utils/ci/run-buildbot
  libcxx/utils/ci/buildkite-pipeline-clang.yml
  libcxx/utils/ci/generate-buildkite-pipeline

Index: libcxx/utils/ci/generate-buildkite-pipeline
===
--- libcxx/utils/ci/generate-buildkite-pipeline
+++ libcxx/utils/ci/generate-buildkite-pipeline
@@ -11,16 +11,4 @@
 # This script generates the appropriate libc++ CI pipeline based on which project(s) were changed.
 #
 
-if git diff --name-only HEAD~1 | grep -q -E "^libcxx/|^libcxxabi/|^libunwind/|^runtimes/|^cmake/"; then
-  LIBCXX_CHANGED=true
-fi
-
-if git diff --name-only HEAD~1 | grep -q -E "^clang/"; then
-  CLANG_CHANGED=true
-fi
-
-if [[ "${CLANG_CHANGED}" == "true" && "${LIBCXX_CHANGED}" != "true" ]]; then
-  cat libcxx/utils/ci/buildkite-pipeline-clang.yml
-else
-  cat libcxx/utils/ci/buildkite-pipeline.yml
-fi
+cat clang/utils/ci/buildkite-pipeline.yml
Index: clang/utils/ci/run-buildbot
===
--- /dev/null
+++ clang/utils/ci/run-buildbot
@@ -0,0 +1,128 @@
+#!/usr/bin/env bash
+#===--===##
+#
+# 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
+#
+#===--===##
+
+set -ex
+set -o pipefail
+unset LANG
+unset LC_ALL
+unset LC_COLLATE
+
+PROGNAME="$(basename "${0}")"
+
+function usage() {
+cat <
+
+[-h|--help] Display this help and exit.
+
+--llvm-rootPath to the root of the LLVM monorepo. By default, we try
+to figure it out based on the current working directory.
+
+--build-dirThe directory to use for building the library. By default,
+this is '/build/'.
+EOF
+}
+
+if [[ $# == 0 ]]; then
+   usage
+   exit 0
+fi
+
+while [[ $# -gt 0 ]]; do
+case ${1} in
+-h|--help)
+usage
+exit 0
+;;
+--llvm-root)
+MONOREPO_ROOT="${2}"
+shift; shift
+;;
+--build-dir)
+BUILD_DIR="${2}"
+shift; shift
+;;
+*)
+BUILDER="${1}"
+shift
+;;
+esac
+done
+
+MONOREPO_ROOT="${MONOREPO_ROOT:="$(git rev-parse --show-toplevel)"}"
+BUILD_DIR="${BUILD_DIR:=${MONOREPO_ROOT}/build/${BUILDER}}"
+INSTALL_DIR="${BUILD_DIR}/install"
+
+# Print the version of a few tools to aid diagnostics in some cases
+cmake --version
+ninja --version
+
+case "${BUILDER}" in
+check-format)
+! grep -rnI '[[:blank:]]$' clang/lib clang/include clang/docs
+;;
+build-clang)
+mkdir install
+# We use Release here to avoid including debug information. Otherwise, the
+# clang binary is very large, which is problematic because we need to upload
+# the artifacts for other jobs to use. This may seem like nothing, but with
+# the number of jobs we run daily, this can result in thousands of GB of
+# network I/O.
+cmake  \
+-S llvm\
+-B build   \
+-G Ninja   \
+-DCMAKE_CXX_COMPILER_LAUNCHER="ccache" \
+-DCMAKE_BUILD_TYPE=Release \
+-DCMAKE_INSTALL_PREFIX=install \
+-DLLVM_ENABLE_PROJECTS="clang;compiler-rt" \
+
+ninja -C build install-clang install-clang-resource-headers
+ccache -s
+tar -cJvf install.tar.xz install/
+buildkite-agent artifact upload --debug install.tar.xz
+;;
+generic-cxx03)
+buildkite-agent artifact download install.tar.xz .
+tar -xvf install.tar.xz
+export CC=$(pwd)/install/bin/clang
+export CXX=$(pwd)/install/bin/clang++
+chmod +x install/bin/clang install/bin/clang++
+libcxx/utils/ci/run-buildbot generic-cxx03
+;;
+generic-cxx26)
+buildkite-agent artifact download install.tar.xz .
+tar -xvf install.tar.xz
+export CC=$(pwd)/install/bin/clang
+export CXX=$(pwd)/install/bin/clang++
+chmod +x install/bin/clang install/bin/clang++
+libcxx/utils/ci/run-buildbot generic-cxx26
+;;
+generic-modules)
+buildkite-agent artifact download install.

[PATCH] D153920: [clang] Move the clang formatting job to run-buildbot to fix the CI

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 538798.
philnik added a comment.

Try to fix CI


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153920

Files:
  clang/utils/ci/buildkite-pipeline.yml
  clang/utils/ci/run-buildbot
  libcxx/utils/ci/buildkite-pipeline-clang.yml
  libcxx/utils/ci/generate-buildkite-pipeline

Index: libcxx/utils/ci/generate-buildkite-pipeline
===
--- libcxx/utils/ci/generate-buildkite-pipeline
+++ libcxx/utils/ci/generate-buildkite-pipeline
@@ -11,16 +11,4 @@
 # This script generates the appropriate libc++ CI pipeline based on which project(s) were changed.
 #
 
-if git diff --name-only HEAD~1 | grep -q -E "^libcxx/|^libcxxabi/|^libunwind/|^runtimes/|^cmake/"; then
-  LIBCXX_CHANGED=true
-fi
-
-if git diff --name-only HEAD~1 | grep -q -E "^clang/"; then
-  CLANG_CHANGED=true
-fi
-
-if [[ "${CLANG_CHANGED}" == "true" && "${LIBCXX_CHANGED}" != "true" ]]; then
-  cat libcxx/utils/ci/buildkite-pipeline-clang.yml
-else
-  cat libcxx/utils/ci/buildkite-pipeline.yml
-fi
+cat clang/utils/ci/buildkite-pipeline.yml
Index: clang/utils/ci/run-buildbot
===
--- /dev/null
+++ clang/utils/ci/run-buildbot
@@ -0,0 +1,128 @@
+#!/usr/bin/env bash
+#===--===##
+#
+# 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
+#
+#===--===##
+
+set -ex
+set -o pipefail
+unset LANG
+unset LC_ALL
+unset LC_COLLATE
+
+PROGNAME="$(basename "${0}")"
+
+function usage() {
+cat <
+
+[-h|--help] Display this help and exit.
+
+--llvm-rootPath to the root of the LLVM monorepo. By default, we try
+to figure it out based on the current working directory.
+
+--build-dirThe directory to use for building the library. By default,
+this is '/build/'.
+EOF
+}
+
+if [[ $# == 0 ]]; then
+   usage
+   exit 0
+fi
+
+while [[ $# -gt 0 ]]; do
+case ${1} in
+-h|--help)
+usage
+exit 0
+;;
+--llvm-root)
+MONOREPO_ROOT="${2}"
+shift; shift
+;;
+--build-dir)
+BUILD_DIR="${2}"
+shift; shift
+;;
+*)
+BUILDER="${1}"
+shift
+;;
+esac
+done
+
+MONOREPO_ROOT="${MONOREPO_ROOT:="$(git rev-parse --show-toplevel)"}"
+BUILD_DIR="${BUILD_DIR:=${MONOREPO_ROOT}/build/${BUILDER}}"
+INSTALL_DIR="${BUILD_DIR}/install"
+
+# Print the version of a few tools to aid diagnostics in some cases
+cmake --version
+ninja --version
+
+case "${BUILDER}" in
+check-format)
+! grep -rnI '[[:blank:]]$' clang/lib clang/include clang/docs
+;;
+build-clang)
+mkdir install
+# We use Release here to avoid including debug information. Otherwise, the
+# clang binary is very large, which is problematic because we need to upload
+# the artifacts for other jobs to use. This may seem like nothing, but with
+# the number of jobs we run daily, this can result in thousands of GB of
+# network I/O.
+cmake  \
+-S llvm\
+-B build   \
+-G Ninja   \
+-DCMAKE_CXX_COMPILER_LAUNCHER="ccache" \
+-DCMAKE_BUILD_TYPE=Release \
+-DCMAKE_INSTALL_PREFIX=install \
+-DLLVM_ENABLE_PROJECTS="clang;compiler-rt" \
+
+ninja -C build install-clang install-clang-resource-headers
+ccache -s
+tar -cJvf install.tar.xz install/
+buildkite-agent artifact upload --debug install.tar.xz
+;;
+generic-cxx03)
+buildkite-agent artifact download install.tar.xz .
+tar -xvf install.tar.xz
+export CC=$(pwd)/install/bin/clang
+export CXX=$(pwd)/install/bin/clang++
+chmod +x install/bin/clang install/bin/clang++
+libcxx/utils/ci/run-buildbot generic-cxx03
+;;
+generic-cxx26)
+buildkite-agent artifact download install.tar.xz .
+tar -xvf install.tar.xz
+export CC=$(pwd)/install/bin/clang
+export CXX=$(pwd)/install/bin/clang++
+chmod +x install/bin/clang install/bin/clang++
+libcxx/utils/ci/run-buildbot generic-cxx26
+;;
+generic-modules)
+buildkite-agent artifact download install.tar.xz .
+tar -xvf install.tar.xz
+export CC=$(pwd)/install/bin/clang
+export CXX

[PATCH] D154778: [clang] Fix __is_trivially_equality_comparable for classes which contain arrays of non-trivially equality comparable types

[Nikolas Klauser via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
philnik marked an inline comment as done.
Closed by commit rGc6b4433a0f20: [clang] Fix __is_trivially_equality_comparable 
for classes which contain arrays… (authored by philnik).

Changed prior to commit:
  https://reviews.llvm.org/D154778?vs=538393&id=538742#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154778

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaCXX/type-traits.cpp


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3141,6 +3141,13 @@
 };
 
static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableContainsArray));
 
+struct TriviallyEqualityComparableContainsMultiDimensionArray {
+  int a[4][4];
+
+  bool operator==(const 
TriviallyEqualityComparableContainsMultiDimensionArray&) const = default;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableContainsMultiDimensionArray));
+
 struct TriviallyEqualityComparableNonTriviallyCopyable {
   TriviallyEqualityComparableNonTriviallyCopyable(const 
TriviallyEqualityComparableNonTriviallyCopyable&);
   ~TriviallyEqualityComparableNonTriviallyCopyable();
@@ -3292,6 +3299,20 @@
 };
 
static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasEnum));
 
+struct NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs {
+  E e[1];
+
+  bool operator==(const 
NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs&) const = 
default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs));
+
+struct NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2 {
+  E e[1][1];
+
+  bool operator==(const 
NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2&) const = 
default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2));
+
 namespace hidden_friend {
 
 struct TriviallyEqualityComparable {
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2672,6 +2672,9 @@
   }) &&
  llvm::all_of(Decl->fields(), [](const FieldDecl *FD) {
auto Type = FD->getType();
+   if (Type->isArrayType())
+ Type = 
Type->getBaseElementTypeUnsafe()->getCanonicalTypeUnqualified();
+
if (Type->isReferenceType() || Type->isEnumeralType())
  return false;
if (const auto *RD = Type->getAsCXXRecordDecl())


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3141,6 +3141,13 @@
 };
 static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableContainsArray));
 
+struct TriviallyEqualityComparableContainsMultiDimensionArray {
+  int a[4][4];
+
+  bool operator==(const TriviallyEqualityComparableContainsMultiDimensionArray&) const = default;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableContainsMultiDimensionArray));
+
 struct TriviallyEqualityComparableNonTriviallyCopyable {
   TriviallyEqualityComparableNonTriviallyCopyable(const TriviallyEqualityComparableNonTriviallyCopyable&);
   ~TriviallyEqualityComparableNonTriviallyCopyable();
@@ -3292,6 +3299,20 @@
 };
 static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasEnum));
 
+struct NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs {
+  E e[1];
+
+  bool operator==(const NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs));
+
+struct NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2 {
+  E e[1][1];
+
+  bool operator==(const NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2));
+
 namespace hidden_friend {
 
 struct TriviallyEqualityComparable {
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2672,6 +2672,9 @@
   }) &&
  llvm::all_of(Decl->fields(), [](const FieldDecl *FD) {
auto Type = FD->getType();
+   if (Type->isArrayType())
+ Type = Type->getBaseElementTypeUnsafe()->getCanonicalTypeUnqualified();
+
if (Type->isRefer

[PATCH] D154778: [clang] Fix __is_trivially_equality_comparable for classes which contain arrays of non-trivially equality comparable types

[Nikolas Klauser via Phabricator via cfe-commits]

philnik created this revision.
philnik added reviewers: aaron.ballman, cor3ntin, erichkeane.
Herald added a project: All.
philnik requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes #63656


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154778

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaCXX/type-traits.cpp


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3141,6 +3141,13 @@
 };
 
static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableContainsArray));
 
+struct TriviallyEqualityComparableContainsMultiDimensionArray {
+  int a[4][4];
+
+  bool operator==(const 
TriviallyEqualityComparableContainsMultiDimensionArray&) const = default;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableContainsMultiDimensionArray));
+
 struct TriviallyEqualityComparableNonTriviallyCopyable {
   TriviallyEqualityComparableNonTriviallyCopyable(const 
TriviallyEqualityComparableNonTriviallyCopyable&);
   ~TriviallyEqualityComparableNonTriviallyCopyable();
@@ -3277,6 +3284,20 @@
 };
 
static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasEnum));
 
+struct NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs {
+  E e[1];
+
+  bool operator==(const 
NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs&) const = 
default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs));
+
+struct NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2 {
+  E e[1][1];
+
+  bool operator==(const 
NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2&) const = 
default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2));
+
 namespace hidden_friend {
 
 struct TriviallyEqualityComparable {
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2672,6 +2672,9 @@
   }) &&
  llvm::all_of(Decl->fields(), [](const FieldDecl *FD) {
auto Type = FD->getType();
+   if (Type->isArrayType())
+Type = 
Type->getBaseElementTypeUnsafe()->getCanonicalTypeUnqualified();
+
if (Type->isReferenceType() || Type->isEnumeralType())
  return false;
if (const auto *RD = Type->getAsCXXRecordDecl())


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3141,6 +3141,13 @@
 };
 static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableContainsArray));
 
+struct TriviallyEqualityComparableContainsMultiDimensionArray {
+  int a[4][4];
+
+  bool operator==(const TriviallyEqualityComparableContainsMultiDimensionArray&) const = default;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableContainsMultiDimensionArray));
+
 struct TriviallyEqualityComparableNonTriviallyCopyable {
   TriviallyEqualityComparableNonTriviallyCopyable(const TriviallyEqualityComparableNonTriviallyCopyable&);
   ~TriviallyEqualityComparableNonTriviallyCopyable();
@@ -3277,6 +3284,20 @@
 };
 static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasEnum));
 
+struct NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs {
+  E e[1];
+
+  bool operator==(const NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs));
+
+struct NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2 {
+  E e[1][1];
+
+  bool operator==(const NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableNonTriviallyEqualityComparableArrs2));
+
 namespace hidden_friend {
 
 struct TriviallyEqualityComparable {
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2672,6 +2672,9 @@
   }) &&
  llvm::all_of(Decl->fields(), [](const FieldDecl *FD) {
auto Type = FD->getType();
+   if (Type->isArrayType())
+Type = Type->getBaseElementTypeUnsafe()->getCanonicalTypeUnqualified();
+
if (Type->isReferenceType() || Type->isEnumeralType())
  return false;
if (const auto *RD = Type->getAsCXXRecordDecl())
___
cfe-commits mailing list
cfe-commits@l

[PATCH] D122874: [clang] Add GNU spelling for no_unqiue_address attribute

[Nikolas Klauser via Phabricator via cfe-commits]

philnik abandoned this revision.
philnik added a comment.

I'm dropping this in favour of D151683 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122874

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

[PATCH] D153920: [clang] Move the clang formatting job to run-buildbot to fix the CI

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 538290.
philnik marked 7 inline comments as done.
philnik added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153920

Files:
  clang/utils/ci/buildkite-pipeline.yml
  clang/utils/ci/run-buildbot
  libcxx/utils/ci/buildkite-pipeline-clang.yml
  libcxx/utils/ci/generate-buildkite-pipeline

Index: libcxx/utils/ci/generate-buildkite-pipeline
===
--- libcxx/utils/ci/generate-buildkite-pipeline
+++ libcxx/utils/ci/generate-buildkite-pipeline
@@ -11,16 +11,4 @@
 # This script generates the appropriate libc++ CI pipeline based on which project(s) were changed.
 #
 
-if git diff --name-only HEAD~1 | grep -q -E "^libcxx/|^libcxxabi/|^libunwind/|^runtimes/|^cmake/"; then
-  LIBCXX_CHANGED=true
-fi
-
-if git diff --name-only HEAD~1 | grep -q -E "^clang/"; then
-  CLANG_CHANGED=true
-fi
-
-if [[ "${CLANG_CHANGED}" == "true" && "${LIBCXX_CHANGED}" != "true" ]]; then
-  cat libcxx/utils/ci/buildkite-pipeline-clang.yml
-else
-  cat libcxx/utils/ci/buildkite-pipeline.yml
-fi
+cat clang/utils/ci/buildkite-pipeline.yml
Index: clang/utils/ci/run-buildbot
===
--- /dev/null
+++ clang/utils/ci/run-buildbot
@@ -0,0 +1,129 @@
+#!/usr/bin/env bash
+#===--===##
+#
+# 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
+#
+#===--===##
+
+set -ex
+set -o pipefail
+unset LANG
+unset LC_ALL
+unset LC_COLLATE
+
+PROGNAME="$(basename "${0}")"
+
+function usage() {
+cat <
+
+[-h|--help] Display this help and exit.
+
+--llvm-rootPath to the root of the LLVM monorepo. By default, we try
+to figure it out based on the current working directory.
+
+--build-dirThe directory to use for building the library. By default,
+this is '/build/'.
+EOF
+}
+
+if [[ $# == 0 ]]; then
+   usage
+   exit 0
+fi
+
+while [[ $# -gt 0 ]]; do
+case ${1} in
+-h|--help)
+usage
+exit 0
+;;
+--llvm-root)
+MONOREPO_ROOT="${2}"
+shift; shift
+;;
+--build-dir)
+BUILD_DIR="${2}"
+shift; shift
+;;
+--osx-roots
+*)
+BUILDER="${1}"
+shift
+;;
+esac
+done
+
+MONOREPO_ROOT="${MONOREPO_ROOT:="$(git rev-parse --show-toplevel)"}"
+BUILD_DIR="${BUILD_DIR:=${MONOREPO_ROOT}/build/${BUILDER}}"
+INSTALL_DIR="${BUILD_DIR}/install"
+
+# Print the version of a few tools to aid diagnostics in some cases
+cmake --version
+ninja --version
+
+case "${BUILDER}" in
+check-format)
+! grep -rnI '[[:blank:]]$' clang/lib clang/include clang/docs
+;;
+build-clang)
+mkdir install
+# We use Release here to avoid including debug information. Otherwise, the
+# clang binary is very large, which is problematic because we need to upload
+# the artifacts for other jobs to use. This may seem like nothing, but with
+# the number of jobs we run daily, this can result in thousands of GB of
+# network I/O.
+cmake  \
+-S llvm\
+-B build   \
+-G Ninja   \
+-DCMAKE_CXX_COMPILER_LAUNCHER="ccache" \
+-DCMAKE_BUILD_TYPE=Release \
+-DCMAKE_INSTALL_PREFIX=install \
+-DLLVM_ENABLE_PROJECTS="clang;compiler-rt" \
+
+ninja -C build install-clang install-clang-resource-headers
+ccache -s
+tar -cJvf install.tar.xz install/
+buildkite-agent artifact upload --debug install.tar.xz
+;;
+generic-cxx03)
+buildkite-agent artifact download install.tar.xz .
+tar -xvf install.tar.xz
+export CC=$(pwd)/install/bin/clang
+export CXX=$(pwd)/install/bin/clang++
+chmod +x install/bin/clang install/bin/clang++
+libcxx/utils/ci/run-buildbot generic-cxx03
+;;
+generic-cxx26)
+buildkite-agent artifact download install.tar.xz .
+tar -xvf install.tar.xz
+export CC=$(pwd)/install/bin/clang
+export CXX=$(pwd)/install/bin/clang++
+chmod +x install/bin/clang install/bin/clang++
+libcxx/utils/ci/run-buildbot generic-cxx26
+;;
+generic-modules)
+buildkite-agent artifact download install.tar.xz .
+tar -xvf inst

[PATCH] D153920: [clang] Move the clang formatting job to run-buildbot to fix the CI

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added subscribers: cor3ntin, erichkeane, aaron.ballman.
philnik added a comment.

@aaron.ballman @erichkeane @cor3ntin (anybody else?) are you fine with moving 
the clang build kite-pipeline to `clang/utils/ci` and adding a `run-buildbot`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153920

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

[PATCH] D153920: [clang] Move the clang formatting job to run-buildbot to fix the CI

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 538208.
philnik marked 2 inline comments as done.
philnik added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153920

Files:
  clang/utils/ci/buildkite-pipeline.yml
  clang/utils/ci/run-buildbot
  libcxx/utils/ci/buildkite-pipeline-clang.yml
  libcxx/utils/ci/generate-buildkite-pipeline
  libcxx/utils/ci/run-buildbot

Index: libcxx/utils/ci/run-buildbot
===
--- libcxx/utils/ci/run-buildbot
+++ libcxx/utils/ci/run-buildbot
@@ -203,6 +203,9 @@
 # Check if the diff is empty, fail otherwise.
 ! grep -q '^--- a' ${BUILD_DIR}/clang-format.patch
 ;;
+check-format-clang)
+! grep -rnI '[[:blank:]]$' clang/lib clang/include clang/docs
+;;
 check-generated-output)
 # `! foo` doesn't work properly with `set -e`, use `! foo || false` instead.
 # https://stackoverflow.com/questions/57681955/set-e-does-not-respect-logical-not
Index: libcxx/utils/ci/generate-buildkite-pipeline
===
--- libcxx/utils/ci/generate-buildkite-pipeline
+++ libcxx/utils/ci/generate-buildkite-pipeline
@@ -11,16 +11,4 @@
 # This script generates the appropriate libc++ CI pipeline based on which project(s) were changed.
 #
 
-if git diff --name-only HEAD~1 | grep -q -E "^libcxx/|^libcxxabi/|^libunwind/|^runtimes/|^cmake/"; then
-  LIBCXX_CHANGED=true
-fi
-
-if git diff --name-only HEAD~1 | grep -q -E "^clang/"; then
-  CLANG_CHANGED=true
-fi
-
-if [[ "${CLANG_CHANGED}" == "true" && "${LIBCXX_CHANGED}" != "true" ]]; then
-  cat libcxx/utils/ci/buildkite-pipeline-clang.yml
-else
-  cat libcxx/utils/ci/buildkite-pipeline.yml
-fi
+cat libcxx/utils/ci/buildkite-pipeline-clang.yml
Index: clang/utils/ci/run-buildbot
===
--- /dev/null
+++ clang/utils/ci/run-buildbot
@@ -0,0 +1,170 @@
+#!/usr/bin/env bash
+#===--===##
+#
+# 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
+#
+#===--===##
+
+set -ex
+set -o pipefail
+unset LANG
+unset LC_ALL
+unset LC_COLLATE
+
+PROGNAME="$(basename "${0}")"
+
+function usage() {
+cat <
+
+[-h|--help] Display this help and exit.
+
+--llvm-rootPath to the root of the LLVM monorepo. By default, we try
+to figure it out based on the current working directory.
+
+--build-dirThe directory to use for building the library. By default,
+this is '/build/'.
+
+--osx-rootsPath to pre-downloaded macOS dylibs. By default, we download
+them from Green Dragon. This is only relevant at all when
+running back-deployment testing if one wants to override
+the old dylibs we use to run the tests with different ones.
+Environment variables
+CC  The C compiler to use, this value is used by CMake. This
+variable is optional.
+
+CXX The C++ compiler to use, this value is used by CMake. This
+variable is optional.
+
+CMAKE   The CMake binary to use. This variable is optional.
+EOF
+}
+
+if [[ $# == 0 ]]; then
+   usage
+   exit 0
+fi
+
+while [[ $# -gt 0 ]]; do
+case ${1} in
+-h|--help)
+usage
+exit 0
+;;
+--llvm-root)
+MONOREPO_ROOT="${2}"
+shift; shift
+;;
+--build-dir)
+BUILD_DIR="${2}"
+shift; shift
+;;
+--osx-roots)
+OSX_ROOTS="${2}"
+shift; shift
+;;
+*)
+BUILDER="${1}"
+shift
+;;
+esac
+done
+
+MONOREPO_ROOT="${MONOREPO_ROOT:="$(git rev-parse --show-toplevel)"}"
+BUILD_DIR="${BUILD_DIR:=${MONOREPO_ROOT}/build/${BUILDER}}"
+INSTALL_DIR="${BUILD_DIR}/install"
+
+# If we can find Ninja/CMake provided by Xcode, use those since we know their
+# version will generally work with the Clang shipped in Xcode (e.g. if Clang
+# knows about -std=c++20, the CMake bundled in Xcode will probably know about
+# that flag too).
+if xcrun --find ninja &>/dev/null; then
+NINJA="$(xcrun --find ninja)"
+elif which ninja &>/dev/null; then
+# The current implementation of modules needs the absolute path to the ninja
+# binary.
+# TODO MODULES Is this still needed when CMake has libc++ module support?
+NINJA="$(which ninja)"
+else
+NINJA="ninja"
+fi
+
+if [ -z "${CMAKE}" ]; then
+if xcrun --find cmake &>/dev/nu

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

Would it be possible to make `__` also a placeholder, so standard libraries can 
use this before C++26? Or is `_` already a reserved identifier? (I don't think 
so)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153536

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

[PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 536623.
philnik edited the summary of this revision.
philnik added a comment.

Try to fix CI


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151683

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/Features.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Parse/Parser.h
  clang/lib/Basic/Attributes.cpp
  clang/lib/Lex/Lexer.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/AST/ast-dump-attr.m
  clang/test/AST/ast-dump-c-attr.c
  clang/test/AST/attr-annotate-type.c
  clang/test/CodeGen/attr-btf_type_tag-func.c
  clang/test/CodeGen/attr-btf_type_tag-var.c
  clang/test/Frontend/noderef.c
  clang/test/OpenMP/assumes_messages_attr.c
  clang/test/OpenMP/openmp_attribute_compat.cpp
  clang/test/Parser/asm.c
  clang/test/Parser/c2x-attributes.c
  clang/test/Parser/c2x-attributes.m
  clang/test/Parser/cxx-decl.cpp
  clang/test/Parser/objc-attr.m
  clang/test/ParserHLSL/group_shared.hlsl
  clang/test/Preprocessor/has_c_attribute.c
  clang/test/Sema/annotate-type.c
  clang/test/Sema/annotate.c
  clang/test/Sema/attr-availability-square-brackets.c
  clang/test/Sema/attr-external-source-symbol-cxx.cpp
  clang/test/Sema/attr-external-source-symbol.c
  clang/test/Sema/attr-likelihood.c
  clang/test/Sema/attr-objc-bridge-related.m
  clang/test/Sema/attr-regparm.c
  clang/test/Sema/attr-type-safety.c
  clang/test/Sema/c2x-attr.c
  clang/test/Sema/c2x-noreturn.c
  clang/test/Sema/internal_linkage.c
  clang/test/Sema/matrix-type-builtins.c
  clang/test/Sema/neon-vector-types.c
  clang/test/Sema/overload-arm-mve.c
  clang/test/Sema/overloadable.c
  clang/test/Sema/vector-gcc-compat.c
  clang/test/SemaCXX/attr-cxx-disabled.cpp
  clang/test/SemaCXX/cxx98-compat.cpp
  clang/test/SemaCXX/warn-c++11-extensions.cpp
  clang/test/SemaObjC/attr-objc-gc.m
  clang/unittests/AST/AttrTest.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -575,7 +575,6 @@
 // FIXME: We should ask the driver for the appropriate default flags.
 lang_opts.GNUMode = true;
 lang_opts.GNUKeywords = true;
-lang_opts.DoubleSquareBracketAttributes = true;
 lang_opts.CPlusPlus11 = true;
 
 // The Darwin libc expects this macro to be set.
Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3394,14 +3394,10 @@
   // If this is the C++11 variety, also add in the LangOpts test.
   if (Variety == "CXX11")
 Test += " && LangOpts.CPlusPlus11";
-  else if (Variety == "C2x")
-Test += " && LangOpts.DoubleSquareBracketAttributes";
 } else if (Variety == "CXX11")
   // C++11 mode should be checked against LangOpts, which is presumed to be
   // present in the caller.
   Test = "LangOpts.CPlusPlus11";
-else if (Variety == "C2x")
-  Test = "LangOpts.DoubleSquareBracketAttributes";
 
 std::string TestStr = !Test.empty()
   ? Test + " ? " + llvm::itostr(Version) + " : 0"
Index: clang/unittests/AST/AttrTest.cpp
===
--- clang/unittests/AST/AttrTest.cpp
+++ clang/unittests/AST/AttrTest.cpp
@@ -157,7 +157,7 @@
   AST = buildASTFromCodeWithArgs(R"c(
 __auto_type [[clang::annotate_type("auto")]] auto_var = 1;
   )c",
- {"-fdouble-square-bracket-attributes"},
+ {},
  "input.c");
 
   {
Index: clang/test/SemaObjC/attr-objc-gc.m
===
--- clang/test/SemaObjC/attr-objc-gc.m
+++ clang/test/SemaObjC/attr-objc-gc.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -fdouble-square-bracket-attributes -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify %s
 static id __attribute((objc_gc(weak))) a;
 static id __attribute((objc_gc(strong))) b;
 
Index: clang/test/SemaCXX/warn-c++11-extensions.cpp
===
--- clang/test/SemaCXX/warn-c++11-extensions.cpp
+++ clang/test/SemaCXX/warn-c++11-extensions.cpp
@@ -7,3 +7,5 @@
 
 enum struct E1 { A, B }; // expected-warning {{scoped enumerations are a C++11 extension}}
 enum class E2 { C, D }; // expected-warning {{scoped enumerations are a C++11 extension}}

[PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 536620.
philnik marked 16 inline comments as done.
philnik added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151683

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/Features.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Parse/Parser.h
  clang/lib/Basic/Attributes.cpp
  clang/lib/Lex/Lexer.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/AST/ast-dump-attr.m
  clang/test/AST/ast-dump-c-attr.c
  clang/test/AST/attr-annotate-type.c
  clang/test/CodeGen/attr-btf_type_tag-func.c
  clang/test/CodeGen/attr-btf_type_tag-var.c
  clang/test/Frontend/noderef.c
  clang/test/OpenMP/assumes_messages_attr.c
  clang/test/OpenMP/openmp_attribute_compat.cpp
  clang/test/Parser/asm.c
  clang/test/Parser/c2x-attributes.c
  clang/test/Parser/c2x-attributes.m
  clang/test/Parser/cxx-decl.cpp
  clang/test/Parser/objc-attr.m
  clang/test/ParserHLSL/group_shared.hlsl
  clang/test/Preprocessor/has_c_attribute.c
  clang/test/Sema/annotate-type.c
  clang/test/Sema/annotate.c
  clang/test/Sema/attr-availability-square-brackets.c
  clang/test/Sema/attr-external-source-symbol-cxx.cpp
  clang/test/Sema/attr-external-source-symbol.c
  clang/test/Sema/attr-likelihood.c
  clang/test/Sema/attr-objc-bridge-related.m
  clang/test/Sema/attr-regparm.c
  clang/test/Sema/attr-type-safety.c
  clang/test/Sema/c2x-attr.c
  clang/test/Sema/c2x-noreturn.c
  clang/test/Sema/internal_linkage.c
  clang/test/Sema/matrix-type-builtins.c
  clang/test/Sema/neon-vector-types.c
  clang/test/Sema/overload-arm-mve.c
  clang/test/Sema/overloadable.c
  clang/test/Sema/vector-gcc-compat.c
  clang/test/SemaCXX/attr-cxx-disabled.cpp
  clang/test/SemaCXX/cxx98-compat.cpp
  clang/test/SemaCXX/warn-c++11-extensions.cpp
  clang/test/SemaObjC/attr-objc-gc.m
  clang/unittests/AST/AttrTest.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -575,7 +575,6 @@
 // FIXME: We should ask the driver for the appropriate default flags.
 lang_opts.GNUMode = true;
 lang_opts.GNUKeywords = true;
-lang_opts.DoubleSquareBracketAttributes = true;
 lang_opts.CPlusPlus11 = true;
 
 // The Darwin libc expects this macro to be set.
Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3394,14 +3394,10 @@
   // If this is the C++11 variety, also add in the LangOpts test.
   if (Variety == "CXX11")
 Test += " && LangOpts.CPlusPlus11";
-  else if (Variety == "C2x")
-Test += " && LangOpts.DoubleSquareBracketAttributes";
 } else if (Variety == "CXX11")
   // C++11 mode should be checked against LangOpts, which is presumed to be
   // present in the caller.
   Test = "LangOpts.CPlusPlus11";
-else if (Variety == "C2x")
-  Test = "LangOpts.DoubleSquareBracketAttributes";
 
 std::string TestStr = !Test.empty()
   ? Test + " ? " + llvm::itostr(Version) + " : 0"
Index: clang/unittests/AST/AttrTest.cpp
===
--- clang/unittests/AST/AttrTest.cpp
+++ clang/unittests/AST/AttrTest.cpp
@@ -157,7 +157,7 @@
   AST = buildASTFromCodeWithArgs(R"c(
 __auto_type [[clang::annotate_type("auto")]] auto_var = 1;
   )c",
- {"-fdouble-square-bracket-attributes"},
+ {},
  "input.c");
 
   {
Index: clang/test/SemaObjC/attr-objc-gc.m
===
--- clang/test/SemaObjC/attr-objc-gc.m
+++ clang/test/SemaObjC/attr-objc-gc.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -fdouble-square-bracket-attributes -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify %s
 static id __attribute((objc_gc(weak))) a;
 static id __attribute((objc_gc(strong))) b;
 
Index: clang/test/SemaCXX/warn-c++11-extensions.cpp
===
--- clang/test/SemaCXX/warn-c++11-extensions.cpp
+++ clang/test/SemaCXX/warn-c++11-extensions.cpp
@@ -7,3 +7,5 @@
 
 enum struct E1 { A, B }; // expected-warning {{scoped enumerations are a C++11 extension}}
 enum class E2 { C, D }; // expected-warning {{scoped enumerations are a C++11 extension}

[PATCH] D153890: [clang] Fix checking the equality comparator of base classes in __is_trivially_equality_comparable

[Nikolas Klauser via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3cf8b982042e: [clang] Fix checking the equality comparator 
of base classes in… (authored by philnik).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153890

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaCXX/type-traits.cpp


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3264,6 +3264,21 @@
 };
 
static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasReferenceMember));
 
+struct NotTriviallyEqualityComparableNonTriviallyComparableBaseBase {
+  int i;
+
+  bool operator==(const 
NotTriviallyEqualityComparableNonTriviallyComparableBaseBase&) const {
+return true;
+  }
+};
+
+struct NotTriviallyEqualityComparableNonTriviallyComparableBase : 
NotTriviallyEqualityComparableNonTriviallyComparableBaseBase {
+  int i;
+
+  bool operator==(const 
NotTriviallyEqualityComparableNonTriviallyComparableBase&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableNonTriviallyComparableBase));
+
 enum E {
   a,
   b
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2667,7 +2667,7 @@
   return llvm::all_of(Decl->bases(),
   [](const CXXBaseSpecifier &BS) {
 if (const auto *RD = 
BS.getType()->getAsCXXRecordDecl())
-  HasNonDeletedDefaultedEqualityComparison(RD);
+  return HasNonDeletedDefaultedEqualityComparison(RD);
 return true;
   }) &&
  llvm::all_of(Decl->fields(), [](const FieldDecl *FD) {


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3264,6 +3264,21 @@
 };
 static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasReferenceMember));
 
+struct NotTriviallyEqualityComparableNonTriviallyComparableBaseBase {
+  int i;
+
+  bool operator==(const NotTriviallyEqualityComparableNonTriviallyComparableBaseBase&) const {
+return true;
+  }
+};
+
+struct NotTriviallyEqualityComparableNonTriviallyComparableBase : NotTriviallyEqualityComparableNonTriviallyComparableBaseBase {
+  int i;
+
+  bool operator==(const NotTriviallyEqualityComparableNonTriviallyComparableBase&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableNonTriviallyComparableBase));
+
 enum E {
   a,
   b
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2667,7 +2667,7 @@
   return llvm::all_of(Decl->bases(),
   [](const CXXBaseSpecifier &BS) {
 if (const auto *RD = BS.getType()->getAsCXXRecordDecl())
-  HasNonDeletedDefaultedEqualityComparison(RD);
+  return HasNonDeletedDefaultedEqualityComparison(RD);
 return true;
   }) &&
  llvm::all_of(Decl->fields(), [](const FieldDecl *FD) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153890: [clang] Fix checking the equality comparator of base classes in __is_trivially_equality_comparable

[Nikolas Klauser via Phabricator via cfe-commits]

philnik created this revision.
philnik added reviewers: aaron.ballman, cor3ntin.
Herald added a project: All.
philnik requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes #63192


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153890

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaCXX/type-traits.cpp


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3264,6 +3264,21 @@
 };
 
static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasReferenceMember));
 
+struct NotTriviallyEqualityComparableNonTriviallyComparableBaseBase {
+  int i;
+
+  bool operator==(const 
NotTriviallyEqualityComparableNonTriviallyComparableBaseBase&) const {
+return true;
+  }
+};
+
+struct NotTriviallyEqualityComparableNonTriviallyComparableBase : 
NotTriviallyEqualityComparableNonTriviallyComparableBaseBase {
+  int i;
+
+  bool operator==(const 
NotTriviallyEqualityComparableNonTriviallyComparableBase&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableNonTriviallyComparableBase));
+
 enum E {
   a,
   b
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2667,7 +2667,7 @@
   return llvm::all_of(Decl->bases(),
   [](const CXXBaseSpecifier &BS) {
 if (const auto *RD = 
BS.getType()->getAsCXXRecordDecl())
-  HasNonDeletedDefaultedEqualityComparison(RD);
+  return HasNonDeletedDefaultedEqualityComparison(RD);
 return true;
   }) &&
  llvm::all_of(Decl->fields(), [](const FieldDecl *FD) {


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3264,6 +3264,21 @@
 };
 static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasReferenceMember));
 
+struct NotTriviallyEqualityComparableNonTriviallyComparableBaseBase {
+  int i;
+
+  bool operator==(const NotTriviallyEqualityComparableNonTriviallyComparableBaseBase&) const {
+return true;
+  }
+};
+
+struct NotTriviallyEqualityComparableNonTriviallyComparableBase : NotTriviallyEqualityComparableNonTriviallyComparableBaseBase {
+  int i;
+
+  bool operator==(const NotTriviallyEqualityComparableNonTriviallyComparableBase&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableNonTriviallyComparableBase));
+
 enum E {
   a,
   b
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2667,7 +2667,7 @@
   return llvm::all_of(Decl->bases(),
   [](const CXXBaseSpecifier &BS) {
 if (const auto *RD = BS.getType()->getAsCXXRecordDecl())
-  HasNonDeletedDefaultedEqualityComparison(RD);
+  return HasNonDeletedDefaultedEqualityComparison(RD);
 return true;
   }) &&
  llvm::all_of(Decl->fields(), [](const FieldDecl *FD) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153737: [clang] __is_trivially_equality_comparable should return false for arrays

[Nikolas Klauser via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4de66e8c4434: [clang] __is_trivially_equality_comparable 
should return false for arrays (authored by philnik).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153737

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaCXX/type-traits.cpp


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3110,7 +3110,7 @@
 static_assert(!__is_trivially_equality_comparable(void), "");
 static_assert(__is_trivially_equality_comparable(int), "");
 static_assert(!__is_trivially_equality_comparable(int[]), "");
-static_assert(__is_trivially_equality_comparable(int[3]), "");
+static_assert(!__is_trivially_equality_comparable(int[3]), "");
 static_assert(!__is_trivially_equality_comparable(float), "");
 static_assert(!__is_trivially_equality_comparable(double), "");
 static_assert(!__is_trivially_equality_comparable(long double), "");
@@ -3134,6 +3134,13 @@
 };
 static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable), 
"");
 
+struct TriviallyEqualityComparableContainsArray {
+  int a[4];
+
+  bool operator==(const TriviallyEqualityComparableContainsArray&) const = 
default;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableContainsArray));
+
 struct TriviallyEqualityComparableNonTriviallyCopyable {
   TriviallyEqualityComparableNonTriviallyCopyable(const 
TriviallyEqualityComparableNonTriviallyCopyable&);
   ~TriviallyEqualityComparableNonTriviallyCopyable();
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2684,7 +2684,7 @@
 const ASTContext &Context) const {
   QualType CanonicalType = getCanonicalType();
   if (CanonicalType->isIncompleteType() || CanonicalType->isDependentType() ||
-  CanonicalType->isEnumeralType())
+  CanonicalType->isEnumeralType() || CanonicalType->isArrayType())
 return false;
 
   if (const auto *RD = CanonicalType->getAsCXXRecordDecl()) {


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3110,7 +3110,7 @@
 static_assert(!__is_trivially_equality_comparable(void), "");
 static_assert(__is_trivially_equality_comparable(int), "");
 static_assert(!__is_trivially_equality_comparable(int[]), "");
-static_assert(__is_trivially_equality_comparable(int[3]), "");
+static_assert(!__is_trivially_equality_comparable(int[3]), "");
 static_assert(!__is_trivially_equality_comparable(float), "");
 static_assert(!__is_trivially_equality_comparable(double), "");
 static_assert(!__is_trivially_equality_comparable(long double), "");
@@ -3134,6 +3134,13 @@
 };
 static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable), "");
 
+struct TriviallyEqualityComparableContainsArray {
+  int a[4];
+
+  bool operator==(const TriviallyEqualityComparableContainsArray&) const = default;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableContainsArray));
+
 struct TriviallyEqualityComparableNonTriviallyCopyable {
   TriviallyEqualityComparableNonTriviallyCopyable(const TriviallyEqualityComparableNonTriviallyCopyable&);
   ~TriviallyEqualityComparableNonTriviallyCopyable();
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2684,7 +2684,7 @@
 const ASTContext &Context) const {
   QualType CanonicalType = getCanonicalType();
   if (CanonicalType->isIncompleteType() || CanonicalType->isDependentType() ||
-  CanonicalType->isEnumeralType())
+  CanonicalType->isEnumeralType() || CanonicalType->isArrayType())
 return false;
 
   if (const auto *RD = CanonicalType->getAsCXXRecordDecl()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151963: [clang][NFC] Remove trailing whitespaces and enforce it in lib, include and docs

[Nikolas Klauser via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf6d557ee34b6: [clang][NFC] Remove trailing whitespaces and 
enforce it in lib, include and docs (authored by philnik).

Changed prior to commit:
  https://reviews.llvm.org/D151963?vs=527706&id=534608#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151963

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ClangLinkerWrapper.rst
  clang/docs/ClangOffloadPackager.rst
  clang/docs/ClangTransformerTutorial.rst
  clang/docs/DebuggingCoroutines.rst
  clang/docs/ExternalClangExamples.rst
  clang/docs/JSONCompilationDatabase.rst
  clang/docs/LanguageExtensions.rst
  clang/docs/MisExpect.rst
  clang/docs/SourceBasedCodeCoverage.rst
  clang/docs/StandardCPlusPlusModules.rst
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/docs/analyzer/checkers.rst
  clang/docs/analyzer/developer-docs/DebugChecks.rst
  clang/docs/tools/dump_ast_matchers.py
  clang/docs/tools/dump_format_style.py
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/Analysis/AnalysisDeclContext.h
  clang/include/clang/Analysis/CallGraph.h
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/OpenCLExtensions.def
  clang/include/clang/Basic/TypeNodes.td
  clang/include/clang/Basic/riscv_vector.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/CMakeLists.txt
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/Analysis/FlowSensitive/CMakeLists.txt
  clang/lib/Analysis/UninitializedValues.cpp
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  libcxx/utils/ci/buildkite-pipeline-clang.yml

Index: libcxx/utils/ci/buildkite-pipeline-clang.yml
===
--- libcxx/utils/ci/buildkite-pipeline-clang.yml
+++ libcxx/utils/ci/buildkite-pipeline-clang.yml
@@ -17,6 +17,21 @@
 # LLVM RELEASE bump version
 LLVM_HEAD_VERSION: "17"
 steps:
+  - label: "Format"
+commands:
+  - "! grep -rnI '[[:blank:]]$' clang/lib clang/include clang/docs || false"
+
+agents:
+  queue: "libcxx-builders"
+  os: "linux"
+retry:
+  automatic:
+- exit_status: -1  # Agent was lost
+  limit: 2
+timeout_in_minutes: 120
+
+  - wait
+
   - label: "Building clang"
 commands:
   - "mkdir install"
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1242,7 +1242,7 @@
 const QualType &ElementTy,
 const LocationContext *LCtx,
 SVal *ElementCountVal) {
-  assert(Region != nullptr && "Not-null region expected");	
+  assert(Region != nullptr && "Not-null region expected");
 
   QualType Ty = ElementTy.getDesugaredType(getContext());
   while (const auto *NTy = dyn_cast(Ty))
Index: clang/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp
@@ -80,7 +80,7 @@
   mgr.registerChecker();
 }
 
-bool ento::shouldRegisterNSAutoreleasePoolChecker(const CheckerManager &mgr) { 
+bool ento::shouldRegisterNSAutoreleasePoolChecker(const CheckerManager &mgr) {
   const LangOptions &LO = mgr.getLangOpts();
   return LO.getGC() != LangOptions::NonGC;
 }
Index: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
@@ -227,7 +227,7 @@
  CheckerContext &C) const {
   // Cleanup
   auto State = C.getState();
-  
+
   auto ContMap = State->get();
   for (const auto &Cont : ContMap) {
 if (!SR.isLiveRegion(Cont.first)) {
@@ -1021,7 +1021,7 @@
SymbolRef NewSym) {
   auto &SymMgr = SVB.

[PATCH] D153737: [clang] __is_trivially_equality_comparable should return false for arrays

[Nikolas Klauser via Phabricator via cfe-commits]

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

When comparing two arrays, their pointers are compared instead of their 
elements, which menas that they are not trivially equality comparable


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153737

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaCXX/type-traits.cpp


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3110,7 +3110,7 @@
 static_assert(!__is_trivially_equality_comparable(void), "");
 static_assert(__is_trivially_equality_comparable(int), "");
 static_assert(!__is_trivially_equality_comparable(int[]), "");
-static_assert(__is_trivially_equality_comparable(int[3]), "");
+static_assert(!__is_trivially_equality_comparable(int[3]), "");
 static_assert(!__is_trivially_equality_comparable(float), "");
 static_assert(!__is_trivially_equality_comparable(double), "");
 static_assert(!__is_trivially_equality_comparable(long double), "");
@@ -3134,6 +3134,13 @@
 };
 static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable), 
"");
 
+struct TriviallyEqualityComparableContainsArray {
+  int a[4];
+
+  bool operator==(const TriviallyEqualityComparableContainsArray&) const = 
default;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableContainsArray));
+
 struct TriviallyEqualityComparableNonTriviallyCopyable {
   TriviallyEqualityComparableNonTriviallyCopyable(const 
TriviallyEqualityComparableNonTriviallyCopyable&);
   ~TriviallyEqualityComparableNonTriviallyCopyable();
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2684,7 +2684,7 @@
 const ASTContext &Context) const {
   QualType CanonicalType = getCanonicalType();
   if (CanonicalType->isIncompleteType() || CanonicalType->isDependentType() ||
-  CanonicalType->isEnumeralType())
+  CanonicalType->isEnumeralType() || CanonicalType->isArrayType())
 return false;
 
   if (const auto *RD = CanonicalType->getAsCXXRecordDecl()) {


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3110,7 +3110,7 @@
 static_assert(!__is_trivially_equality_comparable(void), "");
 static_assert(__is_trivially_equality_comparable(int), "");
 static_assert(!__is_trivially_equality_comparable(int[]), "");
-static_assert(__is_trivially_equality_comparable(int[3]), "");
+static_assert(!__is_trivially_equality_comparable(int[3]), "");
 static_assert(!__is_trivially_equality_comparable(float), "");
 static_assert(!__is_trivially_equality_comparable(double), "");
 static_assert(!__is_trivially_equality_comparable(long double), "");
@@ -3134,6 +3134,13 @@
 };
 static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable), "");
 
+struct TriviallyEqualityComparableContainsArray {
+  int a[4];
+
+  bool operator==(const TriviallyEqualityComparableContainsArray&) const = default;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableContainsArray));
+
 struct TriviallyEqualityComparableNonTriviallyCopyable {
   TriviallyEqualityComparableNonTriviallyCopyable(const TriviallyEqualityComparableNonTriviallyCopyable&);
   ~TriviallyEqualityComparableNonTriviallyCopyable();
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2684,7 +2684,7 @@
 const ASTContext &Context) const {
   QualType CanonicalType = getCanonicalType();
   if (CanonicalType->isIncompleteType() || CanonicalType->isDependentType() ||
-  CanonicalType->isEnumeralType())
+  CanonicalType->isEnumeralType() || CanonicalType->isArrayType())
 return false;
 
   if (const auto *RD = CanonicalType->getAsCXXRecordDecl()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151625: [clang] Add `clang::equality_operator_compares_members_lexicographically`

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

@aaron.ballman Do you have any opinion here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151625

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

[PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.



Comment at: clang/test/Parser/cxx-decl.cpp:316
 #if __cplusplus >= 201103L
-// expected-error@+3 {{expected}}
+// expected-error@+2 {{expected}}
 // expected-error@-3 {{expected ';' after top level declarator}}

aaron.ballman wrote:
> Huh... I wasn't expecting to see a change here because there's no attribute 
> nearby. Probably fine, but still a bit curious.
This is probably because of the whitespace trim below.



Comment at: clang/test/ParserHLSL/group_shared.hlsl:14
-// expected-error@+1 {{expected expression}}
 float groupshared [[]] i = 12;
 

beanz wrote:
> aaron.ballman wrote:
> > philnik wrote:
> > > Should this also get an extension warning/should attributes be disabled 
> > > for HLSL?
> > CC @beanz 
> > 
> > I was wondering the same thing. :-)
> By bug rather than design DXC allows C++ attribute syntax in some places for 
> HLSL.
> 
> I'm totally fine with (and prefer) following the rest of the languages here 
> and having HLSL in Clang always allow C++ attributes regardless of language 
> version.
Would you like some sort of warning?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151683

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

[PATCH] D152083: [clang] Warning for uninitialized elements in fixed-size arrays

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticGroups.td:732
 def UninitializedConstReference : DiagGroup<"uninitialized-const-reference">;
+def UninitializedArrayElements : DiagGroup<"uninitialized-array-elements">;
 def Uninitialized  : DiagGroup<"uninitialized", [UninitializedSometimes,

Why do you make this a new warning group instead of just adding it to 
`-Wuninitialized`?



Comment at: clang/lib/Sema/SemaInit.cpp:983
  RequiresSecondPass, nullptr, 0);
+
+if (const ConstantArrayType *CAType = dyn_cast(T)) {

You'll want to add tests for this in `clang/test/Sema`


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

https://reviews.llvm.org/D152083

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

[PATCH] D147717: [C++20][NFC] Claim full support for consteval again

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

https://github.com/llvm/llvm-project/issues/60709 seems also pretty scary to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147717

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

[PATCH] D151963: [clang][NFC] Remove trailing whitespaces and enforce it in lib, include and docs

[Nikolas Klauser via Phabricator via cfe-commits]

philnik created this revision.
philnik added reviewers: erichkeane, aaron.ballman.
Herald added subscribers: luke, steakhal, frasercrmck, martong, luismarques, 
apazos, sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, 
MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, niosHD, sabuasal, 
simoncook, johnrusso, rbar, asb, kbarton, arichardson, nemanjai, dschuff.
Herald added a reviewer: paulkirth.
Herald added a reviewer: NoQ.
Herald added a project: All.
Herald added a comment.
philnik requested review of this revision.
Herald added subscribers: libcxx-commits, cfe-commits, pcwang-thead, MaskRay, 
aheejin.
Herald added projects: clang, libc++.
Herald added a reviewer: libc++.

NOTE: Clang-Format Team Automated Review Comment

Your review contains a change to ClangFormatStyleOptions.rst but not a change 
to clang/include/clang/Format/Format.h

ClangFormatStyleOptions.rst is auto generated from Format.h via 
clang/docs/tools/dump_format_style.py,  please run this to regenerate the .rst

You can validate that the rst is valid by running.

  ./docs/tools/dump_format_style.py
  mkdir -p html
  /usr/bin/sphinx-build -n ./docs ./html


A lot of editors remove trailing whitespaces. This patch removes any trailing 
whitespaces and makes sure that no new ones are added.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151963

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ClangLinkerWrapper.rst
  clang/docs/ClangOffloadPackager.rst
  clang/docs/ClangTransformerTutorial.rst
  clang/docs/DebuggingCoroutines.rst
  clang/docs/ExternalClangExamples.rst
  clang/docs/JSONCompilationDatabase.rst
  clang/docs/MisExpect.rst
  clang/docs/ReleaseNotes.rst
  clang/docs/SourceBasedCodeCoverage.rst
  clang/docs/StandardCPlusPlusModules.rst
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/docs/analyzer/checkers.rst
  clang/docs/analyzer/developer-docs/DebugChecks.rst
  clang/docs/tools/dump_ast_matchers.py
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/Analysis/AnalysisDeclContext.h
  clang/include/clang/Analysis/CallGraph.h
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/OpenCLExtensions.def
  clang/include/clang/Basic/TypeNodes.td
  clang/include/clang/Basic/riscv_vector.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/CMakeLists.txt
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/Analysis/FlowSensitive/CMakeLists.txt
  clang/lib/Analysis/UninitializedValues.cpp
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  libcxx/utils/ci/buildkite-pipeline-clang.yml

Index: libcxx/utils/ci/buildkite-pipeline-clang.yml
===
--- libcxx/utils/ci/buildkite-pipeline-clang.yml
+++ libcxx/utils/ci/buildkite-pipeline-clang.yml
@@ -17,6 +17,21 @@
 # LLVM RELEASE bump version
 LLVM_HEAD_VERSION: "17"
 steps:
+  - label: "Format"
+commands:
+  - "! grep -rn '[[:blank:]]$' clang/lib clang/include clang/docs || false"
+
+agents:
+  queue: "libcxx-builders"
+  os: "linux"
+retry:
+  automatic:
+- exit_status: -1  # Agent was lost
+  limit: 2
+timeout_in_minutes: 120
+
+  - wait
+
   - label: "Building clang"
 commands:
   - "mkdir install"
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1242,7 +1242,7 @@
 const QualType &ElementTy,
 const LocationContext *LCtx,
 SVal *ElementCountVal) {
-  assert(Region != nullptr && "Not-null region expected");	
+  assert(Region != nullptr && "Not-null region expected");
 
   QualType Ty = ElementTy.getDesugaredType(getContext());
   while (const auto *NTy = dyn_cast(Ty))
Index: clang/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp
@@ -80,7 +80,7 @@
   mgr.registerChecker();
 }
 
-bool en

[PATCH] D151625: [clang] Add `clang::equality_operator_compares_members_lexicographically`

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 527704.
philnik marked 2 inline comments as done.
philnik added a comment.
Herald added a subscriber: jdoerfert.

- Address comments
- Try to fix CI


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151625

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/AST/Type.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  
clang/test/SemaCXX/attr-equality-operator-compares-members-lexicographically.cpp

Index: clang/test/SemaCXX/attr-equality-operator-compares-members-lexicographically.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-equality-operator-compares-members-lexicographically.cpp
@@ -0,0 +1,139 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify %s -std=c++20
+
+void __attribute__((equality_operator_compares_members_lexicographically)) foo(); // expected-warning {{'equality_operator_compares_members_lexicographically' attribute only applies to classes and enums}}
+
+struct [[clang::equality_operator_compares_members_lexicographically]] TriviallyEqualityComparabe1 {
+  int i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparabe1));
+
+struct __attribute__((equality_operator_compares_members_lexicographically)) TriviallyEqualityComparabe2 {
+  int i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparabe2));
+
+struct [[clang::equality_operator_compares_members_lexicographically(1)]] TestMessage {}; // expected-error {{'equality_operator_compares_members_lexicographically' attribute takes no arguments}}
+
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableHasPadding {
+  short i;
+  int j;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasPadding), "");
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableHasFloat {
+  float i;
+  int j;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasFloat), "");
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableHasTailPadding {
+  int i;
+  char j;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasTailPadding), "");
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableBase : NotTriviallyEqualityComparableHasTailPadding {
+  char j;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableBase), "");
+
+class [[clang::equality_operator_compares_members_lexicographically]] TriviallyEqualityComparablePaddedOutBase {
+  int i;
+  char c;
+};
+static_assert(!__is_trivially_equality_comparable(TriviallyEqualityComparablePaddedOutBase), "");
+
+struct [[clang::equality_operator_compares_members_lexicographically]] TriviallyEqualityComparablePaddedOut : TriviallyEqualityComparablePaddedOutBase {
+  char j[3];
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparablePaddedOut), "");
+
+struct [[clang::equality_operator_compares_members_lexicographically]] TriviallyEqualityComparable1 {
+  char i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable1));
+
+struct [[clang::equality_operator_compares_members_lexicographically]] TriviallyEqualityComparable2 {
+  int i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable2));
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableTriviallyEqualityComparableBases
+: TriviallyEqualityComparable1, TriviallyEqualityComparable2 {
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableTriviallyEqualityComparableBases));
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableBitfield {
+  int i : 1;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableBitfield));
+
+// TODO: This is trivially equality comparable
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableBitfieldFilled {
+  char i : __CHAR_BIT__;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableBitfield));
+
+union [[clang::equality_operator_compares_members_lexicographically]] U {
+  int i;
+};
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableImplicitlyDeletedOperatorByUnion {
+  U u;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableImplicitlyDeletedOperatorByUnion));
+
+struct NotTriviallyEqualityComparableExplicitlyDeleted {
+  int i;
+
+  friend bool operator==(const NotTriviallyEqualityCompar

[PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D151683#4380877 , @erichkeane 
wrote:

> What is the justification for this?

What exactly are you asking for? Why I'd like to back port it? This would make 
quite a bit of code in libc++ simpler and avoids pit-falls where an attribute 
works in some place in some version but not in another.

> Do other compilers do this?

ICC and NVC++ support this: https://godbolt.org/z/TeMnGdGsY

> Was there an RFC?

No. I guess, since you are asking I should write one for this? Only for the 
removal of `-fdouble-square-bracket-attributes`, or also for back porting this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151683

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

[PATCH] D151625: [clang] Add `clang::equality_operator_compares_members_lexicographically`

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D151625#4380735 , @erichkeane 
wrote:

> I'm a little dense today perhaps, but I don't get the use case for this?  Can 
> you ELI-EWG?  Is this an attribute we expect our users to use?  The name is 
> horrifyingly long for users to use, but perhaps that is a good thing.

This is useful for libraries which have ADL requirements. Something like

  template 
  struct Holder {
T v;
  };
  
  struct Incomplete;
  
  template 
  class TupleImpl {
T v;
  
bool operator==(const TupleImpl&) const = default;
  };
  
  template 
  class Tuple : public TupleImpl {
bool operator==(const Tuple&) const = default; // ADL call here
  };
  
  void func() {
Tuple*> f; // Instantiation fails because Incomplete 
isn't defined
  }

has to work for `std::tuple` etc. to make them 
`__is_trivially_equality_comparable`, but the defaulted `operator==` will 
always try ADL (at least that's my understanding). To not make ADL calls, this 
attribute can be used instead, which basically say "trust me, my operator== 
does what the defaulted one would do".

Not sure what you mean by "Can you ELI-EWG?".
I agree that the name isn't great, but I couldn't come up with anything better 
that is still descriptive, and this attribute will probably only be used by 
very few libraries anyways (maybe just libc++, since this is pure QoI). As I 
said in the attribute description, the defaulted operator should be used if 
possible (which is probably the case in 99.9% of use cases).

(Note to self: maybe add the example as a test)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151625

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

[PATCH] D151623: [clang] Extend __is_trivially_equality_comparable to check for hidden friends

[Nikolas Klauser via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0e4c4c777308: [clang] Extend 
__is_trivially_equality_comparable to check for hidden friends (authored by 
philnik).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151623

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaCXX/type-traits.cpp

Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3270,6 +3270,172 @@
 };
 static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasEnum));
 
+namespace hidden_friend {
+
+struct TriviallyEqualityComparable {
+  int i;
+  int j;
+
+  void func();
+  bool operator==(int) const { return false; }
+
+  friend bool operator==(const TriviallyEqualityComparable&, const TriviallyEqualityComparable&) = default;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable), "");
+
+struct TriviallyEqualityComparableNonTriviallyCopyable {
+  TriviallyEqualityComparableNonTriviallyCopyable(const TriviallyEqualityComparableNonTriviallyCopyable&);
+  ~TriviallyEqualityComparableNonTriviallyCopyable();
+  friend bool operator==(const TriviallyEqualityComparableNonTriviallyCopyable&, const TriviallyEqualityComparableNonTriviallyCopyable&) = default;
+  int i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableNonTriviallyCopyable));
+
+struct NotTriviallyEqualityComparableHasPadding {
+  short i;
+  int j;
+
+  friend bool operator==(const NotTriviallyEqualityComparableHasPadding&, const NotTriviallyEqualityComparableHasPadding&) = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasPadding), "");
+
+struct NotTriviallyEqualityComparableHasFloat {
+  float i;
+  int j;
+
+  friend bool operator==(const NotTriviallyEqualityComparableHasFloat&, const NotTriviallyEqualityComparableHasFloat&) = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasFloat), "");
+
+struct NotTriviallyEqualityComparableHasTailPadding {
+  int i;
+  char j;
+
+  friend bool operator==(const NotTriviallyEqualityComparableHasTailPadding&, const NotTriviallyEqualityComparableHasTailPadding&) = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasTailPadding), "");
+
+struct NotTriviallyEqualityComparableBase : NotTriviallyEqualityComparableHasTailPadding {
+  char j;
+
+  friend bool operator==(const NotTriviallyEqualityComparableBase&, const NotTriviallyEqualityComparableBase&) = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableBase), "");
+
+class TriviallyEqualityComparablePaddedOutBase {
+  int i;
+  char c;
+
+public:
+  friend bool operator==(const TriviallyEqualityComparablePaddedOutBase&, const TriviallyEqualityComparablePaddedOutBase&) = default;
+};
+static_assert(!__is_trivially_equality_comparable(TriviallyEqualityComparablePaddedOutBase), "");
+
+struct TriviallyEqualityComparablePaddedOut : TriviallyEqualityComparablePaddedOutBase {
+  char j[3];
+
+  friend bool operator==(const TriviallyEqualityComparablePaddedOut&, const TriviallyEqualityComparablePaddedOut&) = default;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparablePaddedOut), "");
+
+struct TriviallyEqualityComparable1 {
+  char i;
+
+  friend bool operator==(const TriviallyEqualityComparable1&, const TriviallyEqualityComparable1&) = default;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable1));
+
+struct TriviallyEqualityComparable2 {
+  int i;
+
+  friend bool operator==(const TriviallyEqualityComparable2&, const TriviallyEqualityComparable2&) = default;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable2));
+
+struct NotTriviallyEqualityComparableTriviallyEqualityComparableBases
+: TriviallyEqualityComparable1, TriviallyEqualityComparable2 {
+  friend bool operator==(const NotTriviallyEqualityComparableTriviallyEqualityComparableBases&, const NotTriviallyEqualityComparableTriviallyEqualityComparableBases&) = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableTriviallyEqualityComparableBases));
+
+struct NotTriviallyEqualityComparableBitfield {
+  int i : 1;
+
+  friend bool operator==(const NotTriviallyEqualityComparableBitfield&, const NotTriviallyEqualityComparableBitfield&) = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableBitfield));
+
+// TODO: This is trivially equality comparable
+struct NotTriviallyEqualityComparableBitfieldFilled {
+  char i : __CHAR_BIT__;
+
+  friend bool operator==(const NotTriviallyEqualityComparableBitfieldFilled&, const NotTriviallyEqualityComparableBitfieldF

[PATCH] D151623: [clang] Extend __is_trivially_equality_comparable to check for hidden friends

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D151623#4380760 , @erichkeane 
wrote:

> Is this a builtin that is supposed to model something in the standard?  It 
> isn't clear to me how this one is supposed to work.

No, this is for optimization purposes. It is true when it is know that 
comparing two instances of T is equivalent to a `memcmp`. Or in other words 
every bit in the type is part of its value representation. Does that help?




Comment at: clang/test/SemaCXX/type-traits.cpp:3439
+
 #endif // __cplusplus >= 202002L
 };

erichkeane wrote:
> Is there a reason these tests need to be c++20 only?
Defaulted equality comparison is a C++20 feature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151623

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

[PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.
Herald added subscribers: Michael137, JDevlieghere.



Comment at: clang/test/ParserHLSL/group_shared.hlsl:14
-// expected-error@+1 {{expected expression}}
 float groupshared [[]] i = 12;
 

Should this also get an extension warning/should attributes be disabled for 
HLSL?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151683

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

[PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

[Nikolas Klauser via Phabricator via cfe-commits]

philnik created this revision.
philnik added reviewers: aaron.ballman, erichkeane.
Herald added subscribers: wlei, wenlei, jdoerfert, dmgreen.
Herald added a project: All.
philnik requested review of this revision.
Herald added subscribers: lldb-commits, cfe-commits, jplehr, sstefan1, MaskRay.
Herald added a reviewer: jdoerfert.
Herald added projects: clang, LLDB.

This also removes the `-fdouble-square-bracket-attributes` command line flag, 
which seems to not be used anywhere. At least a code search exclusively found 
mentions of it in documentation: 
https://sourcegraph.com/search?q=context:global+-fdouble-square-bracket-attributes+-file:clang/*+-file:test/Sema/*+-file:test/Parser/*+-file:test/AST/*+-file:test/Preprocessor/*+-file:test/Misc/*+archived:yes&patternType=standard&sm=0&groupBy=repo


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151683

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Parse/Parser.h
  clang/lib/Basic/Attributes.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Lex/Lexer.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/AST/ast-dump-attr.m
  clang/test/AST/ast-dump-c-attr.c
  clang/test/AST/attr-annotate-type.c
  clang/test/CodeGen/attr-btf_type_tag-func.c
  clang/test/CodeGen/attr-btf_type_tag-var.c
  clang/test/Frontend/noderef.c
  clang/test/OpenMP/assumes_messages_attr.c
  clang/test/OpenMP/openmp_attribute_compat.cpp
  clang/test/Parser/asm.c
  clang/test/Parser/c2x-attributes.c
  clang/test/Parser/c2x-attributes.m
  clang/test/Parser/cxx-decl.cpp
  clang/test/Parser/objc-attr.m
  clang/test/ParserHLSL/group_shared.hlsl
  clang/test/Preprocessor/has_c_attribute.c
  clang/test/Sema/annotate-type.c
  clang/test/Sema/annotate.c
  clang/test/Sema/attr-availability-square-brackets.c
  clang/test/Sema/attr-external-source-symbol-cxx.cpp
  clang/test/Sema/attr-external-source-symbol.c
  clang/test/Sema/attr-likelihood.c
  clang/test/Sema/attr-objc-bridge-related.m
  clang/test/Sema/attr-regparm.c
  clang/test/Sema/attr-type-safety.c
  clang/test/Sema/c2x-noreturn.c
  clang/test/Sema/internal_linkage.c
  clang/test/Sema/matrix-type-builtins.c
  clang/test/Sema/neon-vector-types.c
  clang/test/Sema/overload-arm-mve.c
  clang/test/Sema/overloadable.c
  clang/test/Sema/vector-gcc-compat.c
  clang/test/SemaCXX/attr-cxx-disabled.cpp
  clang/test/SemaObjC/attr-objc-gc.m
  clang/unittests/AST/AttrTest.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -575,7 +575,6 @@
 // FIXME: We should ask the driver for the appropriate default flags.
 lang_opts.GNUMode = true;
 lang_opts.GNUKeywords = true;
-lang_opts.DoubleSquareBracketAttributes = true;
 lang_opts.CPlusPlus11 = true;
 
 // The Darwin libc expects this macro to be set.
Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3387,14 +3387,10 @@
   // If this is the C++11 variety, also add in the LangOpts test.
   if (Variety == "CXX11")
 Test += " && LangOpts.CPlusPlus11";
-  else if (Variety == "C2x")
-Test += " && LangOpts.DoubleSquareBracketAttributes";
 } else if (Variety == "CXX11")
   // C++11 mode should be checked against LangOpts, which is presumed to be
   // present in the caller.
   Test = "LangOpts.CPlusPlus11";
-else if (Variety == "C2x")
-  Test = "LangOpts.DoubleSquareBracketAttributes";
 
 std::string TestStr = !Test.empty()
   ? Test + " ? " + llvm::itostr(Version) + " : 0"
Index: clang/unittests/AST/AttrTest.cpp
===
--- clang/unittests/AST/AttrTest.cpp
+++ clang/unittests/AST/AttrTest.cpp
@@ -157,7 +157,7 @@
   AST = buildASTFromCodeWithArgs(R"c(
 __auto_type [[clang::annotate_type("auto")]] auto_var = 1;
   )c",
- {"-fdouble-square-bracket-attributes"},
+ {},
  "input.c");
 
   {
Index: clang/test/SemaObjC/attr-objc-gc.m
===
--- clang/test/SemaObjC/attr-objc-gc.m
+++ clang/test/SemaObjC/attr-objc-gc.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -fdouble-square-bracket-attributes -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-d

[PATCH] D151625: [clang] Add `clang::equality_operator_compares_members_lexicographically`

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

If anybody has a better name for this I'm all ears.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151625

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

[PATCH] D151625: [clang] Add `clang::equality_operator_compares_members_lexicographically`

[Nikolas Klauser via Phabricator via cfe-commits]

philnik created this revision.
philnik added reviewers: aaron.ballman, erichkeane.
Herald added a subscriber: dschuff.
Herald added a project: All.
philnik requested review of this revision.
Herald added subscribers: cfe-commits, aheejin.
Herald added a project: clang.

This attribute is equivalent to a defaulted member equality comparison for the 
purposes of __is_trivially_equality_comparable. The difference is that it 
doesn't try to do ADL calls when instantiating a class template. We encountered 
this problem while trying to make classes in libc++ trivially equality 
comparable. It can be used to guarantee that enums are trivially equality 
comparable (which is more of a bonus than a good reason to add this attribute).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151625

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/AST/Type.cpp
  
clang/test/SemaCXX/attr-equality-operator-compares-members-lexicographically.cpp

Index: clang/test/SemaCXX/attr-equality-operator-compares-members-lexicographically.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-equality-operator-compares-members-lexicographically.cpp
@@ -0,0 +1,139 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++20
+
+void __attribute__((equality_operator_compares_members_lexicographically)) foo(); // expected-warning {{'equality_operator_compares_members_lexicographically' attribute only applies to classes and enums}}
+
+struct [[clang::equality_operator_compares_members_lexicographically]] TriviallyEqualityComparabe1 {
+  int i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparabe1));
+
+struct __attribute__((equality_operator_compares_members_lexicographically)) TriviallyEqualityComparabe2 {
+  int i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparabe2));
+
+struct [[clang::equality_operator_compares_members_lexicographically(1)]] TestMessage {}; // expected-error {{'equality_operator_compares_members_lexicographically' attribute takes no arguments}}
+
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableHasPadding {
+  short i;
+  int j;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasPadding), "");
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableHasFloat {
+  float i;
+  int j;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasFloat), "");
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableHasTailPadding {
+  int i;
+  char j;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasTailPadding), "");
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableBase : NotTriviallyEqualityComparableHasTailPadding {
+  char j;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableBase), "");
+
+class [[clang::equality_operator_compares_members_lexicographically]] TriviallyEqualityComparablePaddedOutBase {
+  int i;
+  char c;
+};
+static_assert(!__is_trivially_equality_comparable(TriviallyEqualityComparablePaddedOutBase), "");
+
+struct [[clang::equality_operator_compares_members_lexicographically]] TriviallyEqualityComparablePaddedOut : TriviallyEqualityComparablePaddedOutBase {
+  char j[3];
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparablePaddedOut), "");
+
+struct [[clang::equality_operator_compares_members_lexicographically]] TriviallyEqualityComparable1 {
+  char i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable1));
+
+struct [[clang::equality_operator_compares_members_lexicographically]] TriviallyEqualityComparable2 {
+  int i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable2));
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableTriviallyEqualityComparableBases
+: TriviallyEqualityComparable1, TriviallyEqualityComparable2 {
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableTriviallyEqualityComparableBases));
+
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableBitfield {
+  int i : 1;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableBitfield));
+
+// TODO: This is trivially equality comparable
+struct [[clang::equality_operator_compares_members_lexicographically]] NotTriviallyEqualityComparableBitfieldFilled {
+  char i : __CHAR_BIT__;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableBitfield));
+
+union [[clang::equality_operator_compares_members_lexicographically]] U {
+  int i;
+};
+
+struct [[cl

[PATCH] D151623: [clang] Extend __is_trivially_equality_comparable to check for hidden friends

[Nikolas Klauser via Phabricator via cfe-commits]

philnik created this revision.
philnik added reviewers: aaron.ballman, erichkeane.
Herald added a project: All.
philnik requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This allows types to be considered trivially equality comparable if a defaulted 
hidden friend is used.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151623

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaCXX/type-traits.cpp

Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3270,6 +3270,172 @@
 };
 static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasEnum));
 
+namespace hidden_friend {
+
+struct TriviallyEqualityComparable {
+  int i;
+  int j;
+
+  void func();
+  bool operator==(int) const { return false; }
+
+  friend bool operator==(const TriviallyEqualityComparable&, const TriviallyEqualityComparable&) = default;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable), "");
+
+struct TriviallyEqualityComparableNonTriviallyCopyable {
+  TriviallyEqualityComparableNonTriviallyCopyable(const TriviallyEqualityComparableNonTriviallyCopyable&);
+  ~TriviallyEqualityComparableNonTriviallyCopyable();
+  friend bool operator==(const TriviallyEqualityComparableNonTriviallyCopyable&, const TriviallyEqualityComparableNonTriviallyCopyable&) = default;
+  int i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableNonTriviallyCopyable));
+
+struct NotTriviallyEqualityComparableHasPadding {
+  short i;
+  int j;
+
+  friend bool operator==(const NotTriviallyEqualityComparableHasPadding&, const NotTriviallyEqualityComparableHasPadding&) = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasPadding), "");
+
+struct NotTriviallyEqualityComparableHasFloat {
+  float i;
+  int j;
+
+  friend bool operator==(const NotTriviallyEqualityComparableHasFloat&, const NotTriviallyEqualityComparableHasFloat&) = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasFloat), "");
+
+struct NotTriviallyEqualityComparableHasTailPadding {
+  int i;
+  char j;
+
+  friend bool operator==(const NotTriviallyEqualityComparableHasTailPadding&, const NotTriviallyEqualityComparableHasTailPadding&) = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasTailPadding), "");
+
+struct NotTriviallyEqualityComparableBase : NotTriviallyEqualityComparableHasTailPadding {
+  char j;
+
+  friend bool operator==(const NotTriviallyEqualityComparableBase&, const NotTriviallyEqualityComparableBase&) = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableBase), "");
+
+class TriviallyEqualityComparablePaddedOutBase {
+  int i;
+  char c;
+
+public:
+  friend bool operator==(const TriviallyEqualityComparablePaddedOutBase&, const TriviallyEqualityComparablePaddedOutBase&) = default;
+};
+static_assert(!__is_trivially_equality_comparable(TriviallyEqualityComparablePaddedOutBase), "");
+
+struct TriviallyEqualityComparablePaddedOut : TriviallyEqualityComparablePaddedOutBase {
+  char j[3];
+
+  friend bool operator==(const TriviallyEqualityComparablePaddedOut&, const TriviallyEqualityComparablePaddedOut&) = default;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparablePaddedOut), "");
+
+struct TriviallyEqualityComparable1 {
+  char i;
+
+  friend bool operator==(const TriviallyEqualityComparable1&, const TriviallyEqualityComparable1&) = default;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable1));
+
+struct TriviallyEqualityComparable2 {
+  int i;
+
+  friend bool operator==(const TriviallyEqualityComparable2&, const TriviallyEqualityComparable2&) = default;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable2));
+
+struct NotTriviallyEqualityComparableTriviallyEqualityComparableBases
+: TriviallyEqualityComparable1, TriviallyEqualityComparable2 {
+  friend bool operator==(const NotTriviallyEqualityComparableTriviallyEqualityComparableBases&, const NotTriviallyEqualityComparableTriviallyEqualityComparableBases&) = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableTriviallyEqualityComparableBases));
+
+struct NotTriviallyEqualityComparableBitfield {
+  int i : 1;
+
+  friend bool operator==(const NotTriviallyEqualityComparableBitfield&, const NotTriviallyEqualityComparableBitfield&) = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableBitfield));
+
+// TODO: This is trivially equality comparable
+struct NotTriviallyEqualityComparableBitfieldFilled {
+  char i : __CHAR_BIT__;
+
+  friend bool operator==(const NotTriviallyEqualityComparableBitfiel

[PATCH] D150321: [clang] Document extensions from later standards

[Nikolas Klauser via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
philnik marked an inline comment as done.
Closed by commit rGb09fad7f8e9c: [clang] Document extensions from later 
standards (authored by philnik).

Changed prior to commit:
  https://reviews.llvm.org/D150321?vs=521375&id=521402#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150321

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1370,6 +1370,41 @@
 
 More information could be found `here 
`_.
 
+Language Extensions Back-ported to Previous Standards
+=
+
+===  
= = ==
+Feature Feature Test Macro   
Introduced In Backported To Required Flags
+===  
= = ==
+variadic templates  __cpp_variadic_templates C++11 
C++03
+Alias templates __cpp_alias_templatesC++11 
C++03
+Non-static data member initializers __cpp_nsdmi  C++11 
C++03
+Range-based ``for`` loop__cpp_range_based_forC++11 
C++03
+RValue references   __cpp_rvalue_references  C++11 
C++03
+Attributes  __cpp_attributes C++11 
C++03 -fdouble-square-bracket-attributes
+variable templates  __cpp_variable_templates C++14 
C++03
+Binary literals __cpp_binary_literalsC++14 
C++03
+Relaxed constexpr   __cpp_constexpr  C++14 
C++11
+``if constexpr``__cpp_if_constexpr   C++17 
C++11
+fold expressions__cpp_fold_expressions   C++17 
C++03
+Lambda capture of \*this by value   __cpp_capture_star_this  C++17 
C++11
+Attributes on enums __cpp_enumerator_attributes  C++17 
C++11
+Guaranteed copy elision __cpp_guaranteed_copy_elisionC++17 
C++03
+Hexadecimal floating literals   __cpp_hex_float  C++17 
C++03
+``inline`` variables__cpp_inline_variables   C++17 
C++03
+Attributes on namespaces__cpp_namespace_attributes   C++17 
C++11
+Structured bindings __cpp_structured_bindingsC++17 
C++03
+template template arguments __cpp_template_template_args C++17 
C++03
+``static operator[]``   __cpp_multidimensional_subscript C++20 
C++03
+Designated initializers __cpp_designated_initializersC++20 
C++03
+Conditional ``explicit``__cpp_conditional_explicit   C++20 
C++03
+``using enum``  __cpp_using_enum C++20 
C++03
+``if consteval``__cpp_if_consteval   C++23 
C++20
+``static operator()``   __cpp_static_call_operator   C++23 
C++03
+---  
- - --
+Designated initializers  C99   
C89
+===  
= = ==
+
 Type Trait Primitives
 =
 


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1370,6 +1370,41 @@
 
 More information could be found `here `_.
 
+Language Extensions Back-ported to Previous Standards
+=
+
+===  = = ==
+Feature Feature Test Macro   Introduced In Backported To Required Flags
+===  = = ==
+variadic templates  __cpp_variadic_templates C++11 C++03
+Alias templates __cpp_alias_temp

[PATCH] D150321: [clang] Document extensions from later standards

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 521375.
philnik marked 5 inline comments as done.
philnik added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150321

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1370,6 +1370,41 @@
 
 More information could be found `here 
`_.
 
+Language extensions back-ported to previous standards
+=
+
+===  
= = ==
+Feature Feature Test Macro   
Introduced In Backported To Required Flags
+===  
= = ==
+variadic templates  __cpp_variadic_templates C++11 
C++03
+Alias templates __cpp_alias_templatesC++11 
C++03
+Non-static data member initializers __cpp_nsdmi  C++11 
C++03
+Range-based ``for`` loop__cpp_range_based_forC++11 
C++03
+RValue references   __cpp_rvalue_references  C++11 
C++03
+Attributes  __cpp_attributes C++11 
C++03 -fdouble-square-bracket-attributes
+variable templates  __cpp_variable_templates C++14 
C++03
+Binary literals __cpp_binary_literalsC++14 
C++03
+Relaxed constexpr   __cpp_constexpr  C++14 
C++11
+``if constexpr``__cpp_if_constexpr   C++17 
C++11
+fold expressions__cpp_fold_expressions   C++17 
C++03
+Lambda capture of \*this by value   __cpp_capture_star_this  C++17 
C++11
+Attributes on enums __cpp_enumerator_attributes  C++17 
C++11
+Guaranteed copy elision __cpp_guaranteed_copy_elisionC++17 
C++03
+Hexadecimal floating literals   __cpp_hex_float  C++17 
C++03
+``inline`` variables__cpp_inline_variables   C++17 
C++03
+Attributes on namespaces__cpp_namespace_attributes   C++17 
C++11
+Structured bindings __cpp_structured_bindingsC++17 
C++03
+template template arguments __cpp_template_template_args C++17 
C++03
+``static operator[]``   __cpp_multidimensional_subscript C++20 
C++03
+Designated initializers __cpp_designated_initializersC++20 
C++03
+Conditional ``explicit``__cpp_conditional_explicit   C++20 
C++03
+``using enum``  __cpp_using_enum C++20 
C++03
+``if consteval``__cpp_if_consteval   C++23 
C++20
+``static operator()``   __cpp_static_call_operator   C++23 
C++03
+---  
- - --
+Designated initializers  C99   
C89
+===  
= = ==
+
 Type Trait Primitives
 =
 


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1370,6 +1370,41 @@
 
 More information could be found `here `_.
 
+Language extensions back-ported to previous standards
+=
+
+===  = = ==
+Feature Feature Test Macro   Introduced In Backported To Required Flags
+===  = = ==
+variadic templates  __cpp_variadic_templates C++11 C++03
+Alias templates __cpp_alias_templatesC++11 C++03
+Non-static data member initializers __cpp_nsdmi  C++11 C++03
+Range-based ``for`` loop__cpp_range_based_forC++11 C++03
+RValue refere

[PATCH] D150321: [clang] Document extensions from later standards

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:1386
+Relaxed constexpr   __cpp_constexpr  C++14 
C++11
+Designated initializers __cpp_designated_initializersC++20 
C++03
+Attributes on enums __cpp_enumerator_attributes  C++17 
C++11

aaron.ballman wrote:
> This brings up a few questions for me: how should we handle C? For example, 
> designated initializers also exist in C99 and are backported to C89? And how 
> should we handle C features extended into C++ (or vice versa)?
> 
> Should we have multiple tables? Should we try to put both languages into one 
> table with different columns?
> 
> (I'm not asking you to sign up to figure out the C extensions, just trying to 
> get an idea for whether we want to change the layout of this table to account 
> for that sort of thing.)
I think it would make sense to put C features that are back-ported into 
previous standards also into this table (are there FTMs for C?). C into C++ or 
vice versa should probably live somewhere else, since in these cases it would 
also be interesting how things interact with other language features. Maybe 
this is not as much of a problem for C++->C(?), but definitely for C->C++. e.g. 
`__restrict` on member functions or VLAs with non-trivial classes. Essentially, 
there is a lot more to say than "this also exists in previous standards".



Comment at: clang/docs/LanguageExtensions.rst:1387
+Designated initializers __cpp_designated_initializersC++20 
C++03
+Attributes on enums __cpp_enumerator_attributes  C++17 
C++11
+Guaranteed copy elision __cpp_guaranteed_copy_elisionC++17 
C++03

aaron.ballman wrote:
> What are your thoughts on extensions enabled by a feature flag? e.g., 
> https://godbolt.org/z/ex9K5dzv6
I actually had no idea they existed (might be worth documenting :P)! 
Specifically for this case: Is there any reason this is behind a feature flag? 
I guess it's a non-conforming extension? If not, I'd say just make it an 
enabled-by-default extension (it would be a really nice cleanup opportunity for 
libc++). Anyways, we could add another column "flags required" for these cases. 
Do you know how many of these kinds of flags there are?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150321

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

[PATCH] D150321: [clang] Document extensions from later standards

[Nikolas Klauser via Phabricator via cfe-commits]

philnik created this revision.
philnik added a reviewer: aaron.ballman.
Herald added a project: All.
philnik 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/D150321

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1370,6 +1370,38 @@
 
 More information could be found `here 
`_.
 
+Language extensions back-ported to previous standards
+=
+
+===  
= =
+Feature Feature Test Macro   
Introduced In Backported To
+===  
= =
+Conditional ``explicit``__cpp_conditional_explicit   C++20 
C++03
+``if constexpr``__cpp_if_constexpr   C++17 
C++11
+fold expressions__cpp_fold_expressions   C++17 
C++03
+Alias templates __cpp_alias_templatesC++11 
C++03
+Binary literals __cpp_binary_literalsC++14 
C++03
+Lambda capture of \*this by value   __cpp_capture_star_this  C++17 
C++11
+Relaxed constexpr   __cpp_constexpr  C++14 
C++11
+Designated initializers __cpp_designated_initializersC++20 
C++03
+Attributes on enums __cpp_enumerator_attributes  C++17 
C++11
+Guaranteed copy elision __cpp_guaranteed_copy_elisionC++17 
C++03
+``if consteval``__cpp_if_consteval   C++23 
C++20
+Hexadecimal floating literals   __cpp_hex_float  C++17 
C++03
+``inline`` variables__cpp_inline_variables   C++17 
C++03
+``static operator[]``   __cpp_multidimensional_subscript C++20 
C++03
+Attributes on namespaces__cpp_namespace_attributes   C++17 
C++11
+Non-static data member initializers __cpp_nsdmi  C++11 
C++03
+Range-based ``for`` loop__cpp_range_based_forC++11 
C++03
+RValue references   __cpp_rvalue_references  C++11 
C++03
+``static operator()``   __cpp_static_call_operator   C++23 
C++03
+Strucuted bindings  __cpp_structured_bindingsC++17 
C++03
+template template arguments __cpp_template_template_args C++17 
C++03
+``using enum``  __cpp_using_enum C++20 
C++03
+variable templates  __cpp_variable_templates C++14 
C++03
+variadic templates  __cpp_variadic_templates C++11 
C++03
+===  
= ==
+
 Type Trait Primitives
 =
 


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1370,6 +1370,38 @@
 
 More information could be found `here `_.
 
+Language extensions back-ported to previous standards
+=
+
+===  = =
+Feature Feature Test Macro   Introduced In Backported To
+===  = =
+Conditional ``explicit``__cpp_conditional_explicit   C++20 C++03
+``if constexpr``__cpp_if_constexpr   C++17 C++11
+fold expressions__cpp_fold_expressions   C++17 C++03
+Alias templates __cpp_alias_templatesC++11 C++03
+Binary literals __cpp_binary_literalsC++14 C++03
+Lambda capture of \*this by value   __cpp_capture_star_this  C++17 C++11
+Relaxed constexpr   __cpp_constexpr  C++14 C++11
+Designated initializers __cpp_designated_initializersC++20 C++03
+Attributes on enums __cpp_enumerator_attributes  C++17 C++11
+Guaranteed copy elision __cpp_guaranteed_copy_elisionC++17 C++03
+``if consteval``  

[PATCH] D150072: [clang] Fix __is_trivially_equality_comparable for non-trivially-copyable types

[Nikolas Klauser via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG465d48748131: [clang] Fix __is_trivially_equality_comparable 
for non-trivially-copyable types (authored by philnik).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150072

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaCXX/type-traits.cpp


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3134,6 +3134,14 @@
 };
 static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable), 
"");
 
+struct TriviallyEqualityComparableNonTriviallyCopyable {
+  TriviallyEqualityComparableNonTriviallyCopyable(const 
TriviallyEqualityComparableNonTriviallyCopyable&);
+  ~TriviallyEqualityComparableNonTriviallyCopyable();
+  bool operator==(const TriviallyEqualityComparableNonTriviallyCopyable&) 
const = default;
+  int i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableNonTriviallyCopyable));
+
 struct NotTriviallyEqualityComparableHasPadding {
   short i;
   int j;
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2663,7 +2663,8 @@
   return false;
   }
 
-  return Context.hasUniqueObjectRepresentations(CanonicalType);
+  return Context.hasUniqueObjectRepresentations(
+  CanonicalType, /*CheckIfTriviallyCopyable=*/false);
 }
 
 bool QualType::isNonWeakInMRRWithObjCWeak(const ASTContext &Context) const {


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3134,6 +3134,14 @@
 };
 static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable), "");
 
+struct TriviallyEqualityComparableNonTriviallyCopyable {
+  TriviallyEqualityComparableNonTriviallyCopyable(const TriviallyEqualityComparableNonTriviallyCopyable&);
+  ~TriviallyEqualityComparableNonTriviallyCopyable();
+  bool operator==(const TriviallyEqualityComparableNonTriviallyCopyable&) const = default;
+  int i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableNonTriviallyCopyable));
+
 struct NotTriviallyEqualityComparableHasPadding {
   short i;
   int j;
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2663,7 +2663,8 @@
   return false;
   }
 
-  return Context.hasUniqueObjectRepresentations(CanonicalType);
+  return Context.hasUniqueObjectRepresentations(
+  CanonicalType, /*CheckIfTriviallyCopyable=*/false);
 }
 
 bool QualType::isNonWeakInMRRWithObjCWeak(const ASTContext &Context) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150072: [clang] Fix __is_trivially_equality_comparable for non-trivially-copyable types

[Nikolas Klauser via Phabricator via cfe-commits]

philnik created this revision.
philnik added a reviewer: aaron.ballman.
Herald added a project: All.
philnik 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/D150072

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaCXX/type-traits.cpp


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3134,6 +3134,14 @@
 };
 static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable), 
"");
 
+struct TriviallyEqualityComparableNonTriviallyCopyable {
+  TriviallyEqualityComparableNonTriviallyCopyable(const 
TriviallyEqualityComparableNonTriviallyCopyable&);
+  ~TriviallyEqualityComparableNonTriviallyCopyable();
+  bool operator==(const TriviallyEqualityComparableNonTriviallyCopyable&) 
const = default;
+  int i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableNonTriviallyCopyable));
+
 struct NotTriviallyEqualityComparableHasPadding {
   short i;
   int j;
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2663,7 +2663,8 @@
   return false;
   }
 
-  return Context.hasUniqueObjectRepresentations(CanonicalType);
+  return Context.hasUniqueObjectRepresentations(
+  CanonicalType, /*CheckIfTriviallyCopyable=*/false);
 }
 
 bool QualType::isNonWeakInMRRWithObjCWeak(const ASTContext &Context) const {


Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -3134,6 +3134,14 @@
 };
 static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable), "");
 
+struct TriviallyEqualityComparableNonTriviallyCopyable {
+  TriviallyEqualityComparableNonTriviallyCopyable(const TriviallyEqualityComparableNonTriviallyCopyable&);
+  ~TriviallyEqualityComparableNonTriviallyCopyable();
+  bool operator==(const TriviallyEqualityComparableNonTriviallyCopyable&) const = default;
+  int i;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparableNonTriviallyCopyable));
+
 struct NotTriviallyEqualityComparableHasPadding {
   short i;
   int j;
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2663,7 +2663,8 @@
   return false;
   }
 
-  return Context.hasUniqueObjectRepresentations(CanonicalType);
+  return Context.hasUniqueObjectRepresentations(
+  CanonicalType, /*CheckIfTriviallyCopyable=*/false);
 }
 
 bool QualType::isNonWeakInMRRWithObjCWeak(const ASTContext &Context) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149573: [Clang][C++23] Implement core language changes from P1467R9 extended floating-point types and standard names and introduce Bfloat16 arithmetic type.

[Nikolas Klauser via Phabricator via cfe-commits]

philnik removed a reviewer: libc++abi.
philnik added a comment.

In D149573#433 , @codemzs wrote:

> In D149573#4310601 , @philnik wrote:
>
>> In D149573#4310009 , @codemzs 
>> wrote:
>>
>>> My change to libcxxabi/test/test_demangle.pass.cpp (last file in the 
>>> change) is only one line i.e 
>>> {"_ZNK5clang4Type16isArithmeticTypeERNS_10ASTContextE", 
>>> "clang::Type::isArithmeticType(clang::ASTContext&) const"}, but running 
>>> clang-format on it changes bunch of lines.
>>
>> Please don't clang-format the test then. There is no need to do it, and the 
>> changes look rather confusing. I don't really understand why you add it at 
>> all TBH. This doesn't look like it tests anything from this patch.
>
> @philnik Thank you for your feedback on the changes to 
> `libcxxabi/test/test_demangle.pass.cpp`. I apologize for any confusion my 
> initial approach may have caused. I intended to keep the test file consistent 
> with the updated function prototype, but I understand your concern about 
> clang-format's impact on readability.
>
> I will revert the changes to the test file as you suggested, ensuring it does 
> not affect the current patch's functionality. I appreciate your guidance and 
> will consider these aspects in future updates.

No worries :D. The clang-format guidelines are a bit confusing, and there is 
nothing wrong with not knowing all the conventions of the different subprojects.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149573

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

[PATCH] D149573: [Clang][C++23] Implement core language changes from P1467R9 extended floating-point types and standard names and introduce Bfloat16 arithmetic type.

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D149573#4310009 , @codemzs wrote:

> My change to libcxxabi/test/test_demangle.pass.cpp (last file in the change) 
> is only one line i.e {"_ZNK5clang4Type16isArithmeticTypeERNS_10ASTContextE", 
> "clang::Type::isArithmeticType(clang::ASTContext&) const"}, but running 
> clang-format on it changes bunch of lines.

Please don't clang-format the test then. There is no need to do it, and the 
changes look rather confusing. I don't really understand why you add it at all 
TBH. This doesn't look like it tests anything from this patch.


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

https://reviews.llvm.org/D149573

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

[PATCH] D149553: [clang] Use -std=c++23 instead of -std=c++2b

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.



Comment at: libcxx/test/tools/clang_tidy_checks/uglify_attributes.cpp:46
 
-  if (lang_opts.CPlusPlus2b) {
 attributes.emplace_back("assume");

You probably want to `#ifdef` this based on Clang 16/17.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149553

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

[PATCH] D147844: Emit warning when implicit cast from int to bool happens in an conditional operator expression

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

I have to say I'm not really convinced this change is a good idea. The cases it 
flags don't really seem in any way ambiguous/erroneous.

In D147844#4286364 , @chaitanyav 
wrote:

> @aaron.ballman please take a look.
>
> I filed a issue that premerge checks are not working on AIX
> https://github.com/google/llvm-premerge-checks/issues/441
>
> Not sure why the TSAN, MSAN and ASAN checks are failing on libcxx
> https://buildkite.com/llvm-project/libcxx-ci/builds/22664

The AIX builders are currently broken. That has to do with an update. We don't 
really know what's going on with the sanitizer builds, but these are unrelated 
too (probably also a problem with the builders).




Comment at: libcxx/include/strstream:303
 : ostream(&__sb_),
-  __sb_(__s, __n, __s + (__mode & ios::app ? _VSTD::strlen(__s) : 0))
 {}

Please don't change the indentation. It was correct before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

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

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Nikolas Klauser via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe98776a180a7: [clang] Add __is_trivially_equality_comparable 
(authored by philnik).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147175

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp

Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -1,6 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++11 -fblocks -Wno-deprecated-builtins %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++14 -fblocks -Wno-deprecated-builtins %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++1z -fblocks -Wno-deprecated-builtins %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++11 -fblocks -Wno-deprecated-builtins -Wno-defaulted-function-deleted %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++14 -fblocks -Wno-deprecated-builtins -Wno-defaulted-function-deleted %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++17 -fblocks -Wno-deprecated-builtins -Wno-defaulted-function-deleted %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++20 -fblocks -Wno-deprecated-builtins -Wno-defaulted-function-deleted %s
 
 #define T(b) (b) ? 1 : -1
 #define F(b) (b) ? -1 : 1
@@ -570,7 +571,11 @@
   static_assert(__is_aggregate(DerivesAr), "");
   static_assert(__is_aggregate(DerivesArNB), "");
   static_assert(!__is_aggregate(HasCons), "");
+#if __cplusplus >= 202002L
+  static_assert(!__is_aggregate(HasDefaultCons), "");
+#else
   static_assert(__is_aggregate(HasDefaultCons), "");
+#endif
   static_assert(!__is_aggregate(HasExplicitDefaultCons), "");
   static_assert(!__is_aggregate(HasInheritedCons), "");
   static_assert(__is_aggregate(HasNoInheritedCons) == TrueAfterCpp14, "");
@@ -3098,6 +3103,168 @@
 
 } // namespace is_trivially_relocatable
 
+namespace is_trivially_equality_comparable {
+struct ForwardDeclared; // expected-note {{forward declaration of 'is_trivially_equality_comparable::ForwardDeclared'}}
+static_assert(!__is_trivially_equality_comparable(ForwardDeclared), ""); // expected-error {{incomplete type 'ForwardDeclared' used in type trait expression}}
+
+static_assert(!__is_trivially_equality_comparable(void), "");
+static_assert(__is_trivially_equality_comparable(int), "");
+static_assert(!__is_trivially_equality_comparable(int[]), "");
+static_assert(__is_trivially_equality_comparable(int[3]), "");
+static_assert(!__is_trivially_equality_comparable(float), "");
+static_assert(!__is_trivially_equality_comparable(double), "");
+static_assert(!__is_trivially_equality_comparable(long double), "");
+
+struct TriviallyEqualityComparableNoDefaultedComparator {
+  int i;
+  int j;
+};
+static_assert(!__is_trivially_equality_comparable(TriviallyEqualityComparableNoDefaultedComparator), "");
+
+#if __cplusplus >= 202002L
+
+struct TriviallyEqualityComparable {
+  int i;
+  int j;
+
+  void func();
+  bool operator==(int) const { return false; }
+
+  bool operator==(const TriviallyEqualityComparable&) const = default;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable), "");
+
+struct NotTriviallyEqualityComparableHasPadding {
+  short i;
+  int j;
+
+  bool operator==(const NotTriviallyEqualityComparableHasPadding&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasPadding), "");
+
+struct NotTriviallyEqualityComparableHasFloat {
+  float i;
+  int j;
+
+  bool operator==(const NotTriviallyEqualityComparableHasFloat&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasFloat), "");
+
+struct NotTriviallyEqualityComparableHasTailPadding {
+  int i;
+  char j;
+
+  bool operator==(const NotTriviallyEqualityComparableHasTailPadding&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasTailPadding), "");
+
+struct NotTriviallyEqualityComparableBase : NotTriviallyEqualityComparableHasTailPadding {
+  char j;
+
+  bool operator==(const NotTriviallyEqualityComparableBase&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableBase), "");
+
+class TriviallyEqualityComparablePaddedOutBase {
+  int i;
+  char c;
+
+public:
+  bool operator==(const TriviallyEqualityComparablePaddedOutBase&) const = def

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

ping @aaron.ballman


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147175

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

[PATCH] D141775: [Clang] Export CanPassInRegisters as a type trait

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D141775#4256381 , @royjacobson 
wrote:

> In D141775#4255923 , @philnik wrote:
>
>> Is this essentially "is_trivial_for_the_purposes_of_abi"?
>
> I'd say so. Are you asking for a potential libc++ use case?

Kinda. I first thought it would say whether the type is actually passed in 
registers. That would have been very useful for things like 
`__is_cheap_to_copy` and `move_only_function`. Unfortunately, trivial for the 
purposes of ABI isn't enough to know in these cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141775

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

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.



Comment at: libcxx/include/__type_traits/is_equality_comparable.h:46
 template 
-struct __is_trivially_equality_comparable
+struct __libcpp_is_trivially_equality_comparable
 : integral_constant philnik wrote:
> > Mordante wrote:
> > > This does not magically use the builtin right? Does the patch miss that 
> > > parts that use the builtin or is that a followup? If it's a followup I 
> > > would prefer move the libc++ code changes of this patch to a separate 
> > > review.
> > No, it doesn't magically use the builtin. I plan to do that in a follow-up. 
> > These changes are required though, since clang grabs the name with this 
> > patch.
> But it would still be possible to have these libc++ changes in a separate 
> review and commit them separately. I would say the libc++ changes are quite 
> trivial and can be landed quite quickly.
See D147953


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147175

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

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 512201.
philnik marked 3 inline comments as done.
philnik added a comment.

- Moved libc++ changes into it's own PR
- Rebased
- Fixed `enum` false-positive


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147175

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp

Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -1,6 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++11 -fblocks -Wno-deprecated-builtins %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++14 -fblocks -Wno-deprecated-builtins %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++1z -fblocks -Wno-deprecated-builtins %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++11 -fblocks -Wno-deprecated-builtins -Wno-defaulted-function-deleted %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++14 -fblocks -Wno-deprecated-builtins -Wno-defaulted-function-deleted %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++17 -fblocks -Wno-deprecated-builtins -Wno-defaulted-function-deleted %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++20 -fblocks -Wno-deprecated-builtins -Wno-defaulted-function-deleted %s
 
 #define T(b) (b) ? 1 : -1
 #define F(b) (b) ? -1 : 1
@@ -570,7 +571,11 @@
   static_assert(__is_aggregate(DerivesAr), "");
   static_assert(__is_aggregate(DerivesArNB), "");
   static_assert(!__is_aggregate(HasCons), "");
+#if __cplusplus >= 202002L
+  static_assert(!__is_aggregate(HasDefaultCons), "");
+#else
   static_assert(__is_aggregate(HasDefaultCons), "");
+#endif
   static_assert(!__is_aggregate(HasExplicitDefaultCons), "");
   static_assert(!__is_aggregate(HasInheritedCons), "");
   static_assert(__is_aggregate(HasNoInheritedCons) == TrueAfterCpp14, "");
@@ -3092,6 +3097,168 @@
 
 } // namespace is_trivially_relocatable
 
+namespace is_trivially_equality_comparable {
+struct ForwardDeclared; // expected-note {{forward declaration of 'is_trivially_equality_comparable::ForwardDeclared'}}
+static_assert(!__is_trivially_equality_comparable(ForwardDeclared), ""); // expected-error {{incomplete type 'ForwardDeclared' used in type trait expression}}
+
+static_assert(!__is_trivially_equality_comparable(void), "");
+static_assert(__is_trivially_equality_comparable(int), "");
+static_assert(!__is_trivially_equality_comparable(int[]), "");
+static_assert(__is_trivially_equality_comparable(int[3]), "");
+static_assert(!__is_trivially_equality_comparable(float), "");
+static_assert(!__is_trivially_equality_comparable(double), "");
+static_assert(!__is_trivially_equality_comparable(long double), "");
+
+struct TriviallyEqualityComparableNoDefaultedComparator {
+  int i;
+  int j;
+};
+static_assert(!__is_trivially_equality_comparable(TriviallyEqualityComparableNoDefaultedComparator), "");
+
+#if __cplusplus >= 202002L
+
+struct TriviallyEqualityComparable {
+  int i;
+  int j;
+
+  void func();
+  bool operator==(int) const { return false; }
+
+  bool operator==(const TriviallyEqualityComparable&) const = default;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable), "");
+
+struct NotTriviallyEqualityComparableHasPadding {
+  short i;
+  int j;
+
+  bool operator==(const NotTriviallyEqualityComparableHasPadding&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasPadding), "");
+
+struct NotTriviallyEqualityComparableHasFloat {
+  float i;
+  int j;
+
+  bool operator==(const NotTriviallyEqualityComparableHasFloat&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasFloat), "");
+
+struct NotTriviallyEqualityComparableHasTailPadding {
+  int i;
+  char j;
+
+  bool operator==(const NotTriviallyEqualityComparableHasTailPadding&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasTailPadding), "");
+
+struct NotTriviallyEqualityComparableBase : NotTriviallyEqualityComparableHasTailPadding {
+  char j;
+
+  bool operator==(const NotTriviallyEqualityComparableBase&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableBase), "");
+
+class TriviallyEqualityComparablePaddedOutBase {
+  int i;
+  char c;
+
+public:
+  bool operator==(const TriviallyEqualityComparablePaddedOutB

[PATCH] D141775: [Clang] Export CanPassInRegisters as a type trait

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

Is this essentially "is_trivial_for_the_purposes_of_abi"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141775

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

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Nikolas Klauser via Phabricator via cfe-commits]

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



Comment at: libcxx/include/__type_traits/is_equality_comparable.h:46
 template 
-struct __is_trivially_equality_comparable
+struct __libcpp_is_trivially_equality_comparable
 : integral_constant This does not magically use the builtin right? Does the patch miss that parts 
> that use the builtin or is that a followup? If it's a followup I would prefer 
> move the libc++ code changes of this patch to a separate review.
No, it doesn't magically use the builtin. I plan to do that in a follow-up. 
These changes are required though, since clang grabs the name with this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147175

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

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D147175#4251076 , @aaron.ballman 
wrote:

> One thing I can't quite determine is whether we expect to call this type 
> trait often or not. Are either of the uses in libc++ going to be instantiated 
> frequently? I'm trying to figure out whether we want the current 
> implementation of `HasNonDeletedDefaultedEqualityComparison()` or whether we 
> should bite the bullet up front and add another bit to 
> `CXXRecordDecl::DefinitionData` so we compute this property once as the class 
> is being formed and don't have to loop over all of the methods and bases each 
> time. We do this for other special member functions, as with: 
> https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/DeclCXX.h#L695

We currently use `__is_trivially_equality_comparable` only for `std::equal` 
(and I'm working on using it for `std::find`), and I don't think there are a 
lot more places where we can use it. It's also only relevant for cases where we 
have raw pointers. Would the `enable_if` be evaluated, even if the arguments 
aren't raw pointers in the example below?

  template , 
remove_cvref_t> && __is_trivially_equality_comparable(T)>>
  bool equal(T* first1, T* last1, U* first2);
  
  

If not, then I don't think it makes sense to save the information, since it 
will most likely not be evaluated more than once or twice per type.




Comment at: clang/lib/AST/Type.cpp:2618-2619
+ llvm::all_of(Decl->fields(), [](const FieldDecl *FD) {
+   if (!FD->getType()->isRecordType())
+ return !FD->getType()->isReferenceType();
+   return HasNonDeletedDefaultedEqualityComparison(

aaron.ballman wrote:
> Why the check for !RecordType? If any field is a reference, we can bail out 
> early, so I think this is better as:
> ```
> if (FD->getType()->isReferenceType())
>   return false;
> if (const auto *RD = FD->getType()->getAsCXXRecordDecl())
>   return HasNonDeletedDefaultedEqualityComparison(RD);
> return true;
> ```
> WDYT?
I don't really have an opinion here. Your version seems reasonable, so I'm 
going to take it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147175

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

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 511762.
philnik marked 2 inline comments as done.
philnik added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147175

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp
  libcxx/include/__algorithm/equal.h
  libcxx/include/__string/constexpr_c_functions.h
  libcxx/include/__type_traits/is_equality_comparable.h
  libcxx/test/libcxx/type_traits/is_trivially_comparable.compile.pass.cpp

Index: libcxx/test/libcxx/type_traits/is_trivially_comparable.compile.pass.cpp
===
--- libcxx/test/libcxx/type_traits/is_trivially_comparable.compile.pass.cpp
+++ libcxx/test/libcxx/type_traits/is_trivially_comparable.compile.pass.cpp
@@ -13,32 +13,32 @@
 enum Enum : int {};
 enum class EnumClass : int {};
 
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(std::__is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
 struct S {
   char c;
@@ -51,8 +51,8 @@
 struct VirtualBase : virtual S {};
 struct NonVirtualBase : S, S2 {};
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
 // This is trivially_equality_comparable, but we can't detect it currently
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
Index: libcxx/include/__type_traits/is_equality_comparable.h
===
--- libcxx/include/__type_traits/is_equality_comparable.h
+++ libcxx/include/__type_traits/is_equality_comparable.h
@@ -43,14 +43,14 @@
 //   always compared.
 
 template 
-struct __is_trivially_equality_comparable
+struct __libcpp_is_trivially_equality_comparable
 : integral_constant::value && is_inte

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D147175#4246513 , @aaron.ballman 
wrote:

> Ah, I like this approach -- it keeps things roughly in sync with checking for 
> unique object representations, which is great. I spotted a few questions, but 
> in general this is heading in the right direction. You should also add a 
> release note to clang/docs/ReleaseNotes.rst so that users know there's a new 
> builtin coming.
>
> I suppose one thing to double-check: do you know of any other STL 
> implementations that might be using this identifier as part of their 
> implementation details? (We've had issues in the past where we accidentally 
> stole a reserved identifier that was being used by libstdc++ and it caused 
> issues.)

Neither libstdc++ nor the MSVC STL use the `__is_trivially_equality_comparable` 
AFAICT. A sourcegraph search also exclusively finds libc++: 
https://sourcegraph.com/search?q=context%3Aglobal+__is_trivially_equality_comparable&patternType=standard&sm=1&groupBy=repo
 (which only started using it in this release cycle, so it shouldn't be a 
problem).




Comment at: clang/lib/AST/Type.cpp:2598-2599
+static bool HasDefaultedEqualityComparison(const CXXRecordDecl *Decl) {
+  if (Decl->isUnion())
+return false;
+

aaron.ballman wrote:
> Hmmm, is this correct? I thought there was a defaulted equality comparison 
> operator in this case, but it's defined as deleted.
> 
> http://eel.is/c++draft/class.compare.default#2
> 
> Perhaps this function is looking for a usable defaulted equality comparison 
> operator and not just "does it have one at all"?
Yes, this is looking for a usable one. I've renamed it. (Maybe someone has an 
idea for a better name?)



Comment at: clang/lib/AST/Type.cpp:2616-2617
+  }) &&
+ llvm::all_of(Decl->fields(), [](const FieldDecl *FD) {
+   if (!FD->getType()->isRecordType())
+ return true;

aaron.ballman wrote:
> Do we have to look for fields with references per 
> http://eel.is/c++draft/class.compare.default#2 ?
Good catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147175

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

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 511460.
philnik marked 4 inline comments as done.
philnik added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147175

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp
  libcxx/include/__algorithm/equal.h
  libcxx/include/__string/constexpr_c_functions.h
  libcxx/include/__type_traits/is_equality_comparable.h
  libcxx/test/libcxx/type_traits/is_trivially_comparable.compile.pass.cpp

Index: libcxx/test/libcxx/type_traits/is_trivially_comparable.compile.pass.cpp
===
--- libcxx/test/libcxx/type_traits/is_trivially_comparable.compile.pass.cpp
+++ libcxx/test/libcxx/type_traits/is_trivially_comparable.compile.pass.cpp
@@ -13,32 +13,32 @@
 enum Enum : int {};
 enum class EnumClass : int {};
 
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(std::__is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
 struct S {
   char c;
@@ -51,8 +51,8 @@
 struct VirtualBase : virtual S {};
 struct NonVirtualBase : S, S2 {};
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
 // This is trivially_equality_comparable, but we can't detect it currently
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
Index: libcxx/include/__type_traits/is_equality_comparable.h
===
--- libcxx/include/__type_traits/is_equality_comparable.h
+++ libcxx/include/__type_traits/is_equality_comparable.h
@@ -43,14 +43,14 @@
 //   always compared.
 
 template 
-struct __is_trivially_equality_comparable
+struct __libcpp_is_trivially_equality_comparable
 : integral_constant::value && is_inte

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 510887.
philnik added a comment.

Remove formatting changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147175

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp
  libcxx/include/__algorithm/equal.h
  libcxx/include/__string/constexpr_c_functions.h
  libcxx/include/__type_traits/is_equality_comparable.h
  libcxx/test/libcxx/type_traits/is_trivially_comparable.compile.pass.cpp

Index: libcxx/test/libcxx/type_traits/is_trivially_comparable.compile.pass.cpp
===
--- libcxx/test/libcxx/type_traits/is_trivially_comparable.compile.pass.cpp
+++ libcxx/test/libcxx/type_traits/is_trivially_comparable.compile.pass.cpp
@@ -13,32 +13,32 @@
 enum Enum : int {};
 enum class EnumClass : int {};
 
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(std::__is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
 struct S {
   char c;
@@ -51,8 +51,8 @@
 struct VirtualBase : virtual S {};
 struct NonVirtualBase : S, S2 {};
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
 // This is trivially_equality_comparable, but we can't detect it currently
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
Index: libcxx/include/__type_traits/is_equality_comparable.h
===
--- libcxx/include/__type_traits/is_equality_comparable.h
+++ libcxx/include/__type_traits/is_equality_comparable.h
@@ -43,14 +43,14 @@
 //   always compared.
 
 template 
-struct __is_trivially_equality_comparable
+struct __libcpp_is_trivially_equality_comparable
 : integral_constant::value && is_integral<_Tp>::value &&
   

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 510865.
philnik marked 5 inline comments as done.
philnik added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147175

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp
  libcxx/include/__algorithm/equal.h
  libcxx/include/__string/constexpr_c_functions.h
  libcxx/include/__type_traits/is_equality_comparable.h
  libcxx/test/libcxx/type_traits/is_trivially_comparable.compile.pass.cpp

Index: libcxx/test/libcxx/type_traits/is_trivially_comparable.compile.pass.cpp
===
--- libcxx/test/libcxx/type_traits/is_trivially_comparable.compile.pass.cpp
+++ libcxx/test/libcxx/type_traits/is_trivially_comparable.compile.pass.cpp
@@ -13,32 +13,32 @@
 enum Enum : int {};
 enum class EnumClass : int {};
 
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(std::__is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
 struct S {
   char c;
@@ -51,8 +51,8 @@
 struct VirtualBase : virtual S {};
 struct NonVirtualBase : S, S2 {};
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
 // This is trivially_equality_comparable, but we can't detect it currently
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
Index: libcxx/include/__type_traits/is_equality_comparable.h
===
--- libcxx/include/__type_traits/is_equality_comparable.h
+++ libcxx/include/__type_traits/is_equality_comparable.h
@@ -43,14 +43,14 @@
 //   always compared.
 
 template 
-struct __is_trivially_equality_comparable
+struct __libcpp_is_trivially_equality_comparable
 : integral_constant::value && i

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D147175#4240572 , @aaron.ballman 
wrote:

> Hmmm, is this effectively `std::has_unique_object_representations` (ensuring 
> that all bits of the object representation contribute to the value)?

These two traits are indeed very similar. The main difference is that 
`has_unique_object_representations` requires that the type is trivially 
copyable, and `__is_trivially_equality_comparable` requires a defaulted 
equality comparison operator.




Comment at: clang/lib/AST/Type.cpp:2640
+
+  return false;
+}

aaron.ballman wrote:
> I think this might be missing cases, like complex integers, scoped 
> enumerations, vector types?, pointer types, atomic versions of these types...
I think this should be covered by the switch to using 
`hasUniqueObjectRepresentation`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147175

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

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Nikolas Klauser via Phabricator via cfe-commits]

philnik updated this revision to Diff 509598.
philnik added a comment.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.

Try to fix CI


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147175

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/RecordLayout.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/RecordLayout.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp
  libcxx/include/__algorithm/equal.h
  libcxx/include/__string/constexpr_c_functions.h
  libcxx/include/__type_traits/is_equality_comparable.h
  libcxx/test/libcxx/type_traits/is_trivially_comparable.compile.pass.cpp

Index: libcxx/test/libcxx/type_traits/is_trivially_comparable.compile.pass.cpp
===
--- libcxx/test/libcxx/type_traits/is_trivially_comparable.compile.pass.cpp
+++ libcxx/test/libcxx/type_traits/is_trivially_comparable.compile.pass.cpp
@@ -13,32 +13,32 @@
 enum Enum : int {};
 enum class EnumClass : int {};
 
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(std::__is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
 struct S {
   char c;
@@ -51,8 +51,8 @@
 struct VirtualBase : virtual S {};
 struct NonVirtualBase : S, S2 {};
 
-static_assert(!std::__is_trivially_equality_comparable::value, "");
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
 
 // This is trivially_equality_comparable, but we can't detect it currently
-static_assert(!std::__is_trivially_equality_comparable::value, "");
+static_assert(!std::__libcpp_is_trivially_equality_comparable::value, "");
Index: libcxx/include/__type_traits/is_equality_comparable.h
===
--- libcxx/include/__type_traits/is_equality_comparable.h
+++ libcxx/include/__type_traits/is_equality_comparable.h
@@ -43,14 +43,14 @@
 //   always compared.
 
 template 
-struct __is_trivially

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

[Nikolas Klauser via Phabricator via cfe-commits]

philnik created this revision.
philnik added a reviewer: aaron.ballman.
Herald added a project: All.
philnik requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch adds a new trait to allow standard libraries to forward `std::equal` 
calls to `memcmp` in more cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147175

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/RecordLayout.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/RecordLayout.cpp
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-traits.cpp

Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -1,6 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++11 -fblocks -Wno-deprecated-builtins %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++14 -fblocks -Wno-deprecated-builtins %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++1z -fblocks -Wno-deprecated-builtins %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++11 -fblocks -Wno-deprecated-builtins -Wno-defaulted-function-deleted %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++14 -fblocks -Wno-deprecated-builtins -Wno-defaulted-function-deleted %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++17 -fblocks -Wno-deprecated-builtins -Wno-defaulted-function-deleted %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify -std=gnu++20 -fblocks -Wno-deprecated-builtins -Wno-defaulted-function-deleted %s
 
 #define T(b) (b) ? 1 : -1
 #define F(b) (b) ? -1 : 1
@@ -570,7 +571,11 @@
   static_assert(__is_aggregate(DerivesAr), "");
   static_assert(__is_aggregate(DerivesArNB), "");
   static_assert(!__is_aggregate(HasCons), "");
+#if __cplusplus >= 202002L
+  static_assert(!__is_aggregate(HasDefaultCons), "");
+#else
   static_assert(__is_aggregate(HasDefaultCons), "");
+#endif
   static_assert(!__is_aggregate(HasExplicitDefaultCons), "");
   static_assert(!__is_aggregate(HasInheritedCons), "");
   static_assert(__is_aggregate(HasNoInheritedCons) == TrueAfterCpp14, "");
@@ -3055,6 +3060,139 @@
 
 } // namespace is_trivially_relocatable
 
+namespace is_trivially_equality_comparable {
+struct ForwardDeclared; // expected-note {{forward declaration of 'is_trivially_equality_comparable::ForwardDeclared'}}
+static_assert(!__is_trivially_equality_comparable(ForwardDeclared), ""); // expected-error {{incomplete type 'ForwardDeclared' used in type trait expression}}
+
+static_assert(!__is_trivially_equality_comparable(void), "");
+static_assert(__is_trivially_equality_comparable(int), "");
+static_assert(!__is_trivially_equality_comparable(int[]), "");
+static_assert(!__is_trivially_equality_comparable(int[3]), "");
+static_assert(!__is_trivially_equality_comparable(float), "");
+static_assert(!__is_trivially_equality_comparable(double), "");
+static_assert(!__is_trivially_equality_comparable(long double), "");
+
+struct TriviallyEqualityComparableNoDefaultedComparator {
+  int i;
+  int j;
+};
+static_assert(!__is_trivially_equality_comparable(TriviallyEqualityComparableNoDefaultedComparator), "");
+
+#if __cplusplus >= 202002L
+
+struct TriviallyEqualityComparable {
+  int i;
+  int j;
+
+  void func();
+  bool operator==(int) const { return false; }
+
+  bool operator==(const TriviallyEqualityComparable&) const = default;
+};
+static_assert(__is_trivially_equality_comparable(TriviallyEqualityComparable), "");
+
+struct NotTriviallyEqualityComparableHasPadding {
+  short i;
+  int j;
+
+  bool operator==(const NotTriviallyEqualityComparableHasPadding&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasPadding), "");
+
+struct NotTriviallyEqualityComparableHasFloat {
+  float i;
+  int j;
+
+  bool operator==(const NotTriviallyEqualityComparableHasFloat&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasFloat), "");
+
+struct NotTriviallyEqualityComparableHasTailPadding {
+  int i;
+  char j;
+
+  bool operator==(const NotTriviallyEqualityComparableHasTailPadding&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableHasTailPadding), "");
+
+struct NotTriviallyEqualityComparableBase : NotTriviallyEqualityComparableHasTailPadding {
+  char j;
+
+  bool operator==(const NotTriviallyEqualityComparableBase&) const = default;
+};
+static_assert(!__is_trivially_equality_comparable(NotTriviallyEqualityComparableBase), "");
+
+struct TriviallyEqualit

[PATCH] D129951: adds `__disable_adl` attribute

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D129951#4178844 , @cjdb wrote:

> In D129951#4178154 , @philnik wrote:
>
>> I don't think libc++ can adopt this without having to essentially duplicate 
>> our code, since GCC doesn't support `__disable_adl` (and AFAICT there is no 
>> coordination between GCC and Clang to add it to both).
>
> I haven't had a lot of time to drive this in Clang, let alone GCC. Even if 
> libc++ can't ultimately use it (which would be sad), there are other 
> libraries that can. For example, Abseil has a similar attitude towards 
> functions as Niebloids, and could wrap it behind a macro.

Abseil has the same support problem though AFAICT. In fact, most open source 
libraries don't //just// support clang.

>> Have you tested what impact making the members `static` has? Both clang and 
>> GCC already support this as an extension back to C++11: 
>> https://godbolt.org/z/drE5v8nYo.
>
> A quick change to the original benchmark  
> shows the AST for `static operator()` being substantially larger than a 
> function template with ADL being disabled. I haven't properly benchmarked 
> build time impact, but here's a quick one 
> . The averages 
> are below:
>
> **`__disable_adl`**
>
>   real  0.1164
>   user  0.0706
>   sys   0.0488
>
> **`static operator()`**
>
>   real  0.1272
>   user  0.081
>   sys   0.0488
>
> It is worth acknowledging that the assembly output is now much closer with 
> optimised flags (1.63x larger as opposed to 7.56x larger), but 1.26x larger 
> with `-g` (this is down from 1.66x as non-static).

Couldn't that be overcome with some optimizations for Niebloids?

>> Maybe it would make more sense to add an attribute `[[clang::cpo]]` instead 
>> to tell clang that the class should just be treated as an overload set? Make 
>> it requirements that the class is empty, there are no non-static member 
>> functions and the class is declared `final` and you should get any benefits 
>> without the major drawback of basically no portability. It's of course 
>> possible that I'm missing something major, but I think that general way 
>> would be a lot more acceptable. Any thoughts?
>
> CPOs and Niebloids are different things (and `__disable_adl` is for 
> Niebloids, not CPOs), so any such attribute would need a different name.

Yes. Sorry for the conflation.

> Having said that, a struct that hasn't has no base and is final only slightly 
> improves the AST size  with respect to the 
> improvement by using an actual overload set. Finally, there would still be a 
> portability issue because even if `[[clang::niebloid]]` works on Clang, there 
> would still need to be coordination for it to work on GCC; otherwise GCC w/ 
> libc++ mode would have copyable Niebloids; something that the original libc++ 
> design worked hard to ensure wasn't possible so that a feature like this 
> could exist.

I don't know about the original design, but at least the algorithms are 
copyable. I wouldn't be too concerned if that was different between clang and 
GCC, it's at least conforming in both cases. Regarding AST size, I don't know 
how representative LoC in the dump are, but shouldn't it be possible to 
overcome memory usage by modeling Niebloids in a different way than normal 
classes?

> It is again worth acknowledging that the assembly output in an optimised 
> build would have parity, but a build using `-O0 -g` will still be ~1.26x 
> larger.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129951

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

[PATCH] D129951: adds `__disable_adl` attribute

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

I don't think libc++ can adopt this without having to essentially duplicate our 
code, since GCC doesn't support `__disable_adl` (and AFAICT there is no 
coordination between GCC and Clang to add it to both). Have you tested what 
impact making the members `static` has? Both clang and GCC already support this 
as an extension back to C++11: https://godbolt.org/z/drE5v8nYo. Maybe it would 
make more sense to add an attribute `[[clang::cpo]]` instead to tell clang that 
the class should just be treated as an overload set? Make it requirements that 
the class is empty, there are no non-static member functions and the class is 
declared `final` and you should get any benefits without the major drawback of 
basically no portability. It's of course possible that I'm missing something 
major, but I think that general way would be a lot more acceptable. Any 
thoughts?




Comment at: clang/lib/Sema/SemaOverload.cpp:9455-9459
+// Functions in 'std::ranges' are hidden from ADL per [range.iter.ops]/2 
and
+// [algorithms.requirements]/2.
+if ((*I)->isInStdRangesNamespace() &&
+Name.getNameKind() == DeclarationName::NameKind::Identifier)
+  continue;

cjdb wrote:
> rsmith wrote:
> > I worry that this might break some stdlib implementation that finds helper 
> > functions via ADL in `std::ranges` somehow. Also, it seems desirable to 
> > make this opt-in for the stdlib implementation and indeed for end-user 
> > functions not in `std::ranges`.
> > 
> > Have you considered enabling this behavior with an attribute instead of by 
> > detecting whether the function is in `std::ranges`?
> You're the second person to have suggested this, and I daresay Aaron will 
> too, based on our prior discussion about this. We'd chatted about 
> `__disable_adl` specifically, so that anyone who uses it won't be affected by 
> a silent change from Clang to GCC: they'd instead get a break. I would prefer 
> an attribute too.
> 
> My main concern is that other stdlib implementers would object to adding yet 
> another annotation to their function calls (based on my failure to get libc++ 
> to be as aggressive as Microsoft/STL is with `[[nodiscard]]`).
FWIW we are getting more aggressive with adding `[[nodiscard]]`. We have 
extensions enabled by default and are (slowly) adding it to more places.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129951

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

[PATCH] D143524: Make the -Wunused-template default.

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D143524#4148344 , @v.g.vassilev 
wrote:

> In D143524#4148271 , @philnik wrote:
>
>> It looks like this warning is incompatible with `-Wctad-maybe-unsupported`. 
>> It warns that the deduction guide is unused, but the deduction guide is 
>> required suppress `-Wctad-maybe-unsupported`. https://godbolt.org/z/G8bMjYsbn
>
> That should not be too hard to fix. It seems that just need to ignore 
> `CXXDeductionGuideDecl` in `Sema::ShouldRemoveFromUnused`. Is that blocking 
> you?

No, I just added a `[[maybe_unused]]`. D144667 
 should fix any libc++ problems.


Repository:
  rC Clang

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

https://reviews.llvm.org/D143524

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

[PATCH] D143524: Make the -Wunused-template default.

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

It looks like this warning is incompatible with `-Wctad-maybe-unsupported`. It 
warns that the deduction guide is unused, but the deduction guide is required 
suppress `-Wctad-maybe-unsupported`. https://godbolt.org/z/G8bMjYsbn


Repository:
  rC Clang

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

https://reviews.llvm.org/D143524

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

[PATCH] D143524: Make the -Wunused-template default.

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D143524#4148024 , @v.g.vassilev 
wrote:

> In D143524#4148006 , @philnik wrote:
>
>> The emitted warnings from the libc++ CI look like a false-positive to me. 
>> While the functions are never called, they are used in an unevaluated 
>> context. I would expect `-Wunused` warnings to only be emitted when I can 
>> just remove the code without problems, which doesn't seem to be the case 
>> here. It would probably just get turned off again by lots of people if there 
>> are too many false-positives, which I don't think is the goal here.
>
> From what I see is that most of the templates are marked with static which 
> means internal linkage. Entities with internal linkage in header files are 
> essentially different across translation units which is an ODR violation. I 
> believe the discussion here gives more insights of how this works: 
> https://reviews.llvm.org/D29877
>
> Indeed the warning is noisy but it will potentially fix broken code which 
> were unable to diagnose before. Especially that in some cases where static 
> templates in header files are used as an idiom. In theory this approach can 
> extend to cases described in https://wg21.link/p2691 where our inability to 
> diagnose/fix these cases leads to some design decisions which may not be 
> optimal.

I missed the `static` at the beginning. That explains the warning, thanks! I 
agree this should be fixed. I'll look into making a patch to enable 
`-Wunused-template` and fix any problems. Hopefully there aren't too many.


Repository:
  rC Clang

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

https://reviews.llvm.org/D143524

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

[PATCH] D143524: Make the -Wunused-template default.

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

The emitted warnings from the libc++ CI look like a false-positive to me. While 
the functions are never called, they are used in an unevaluated context. I 
would expect `-Wunused` warnings to only be emitted when I can just remove the 
code without problems, which doesn't seem to be the case here. It would 
probably just get turned off again by lots of people if there are too many 
false-positives, which I don't think is the goal here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D143524

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

[PATCH] D144334: [Clang] Add C++2b attribute [[assume(expression)]]

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1448
+def Assume : StmtAttr {
+  let Spellings = [CXX11<"", "assume", 202302>];
+  let Documentation = [AssumeDocs];

tahonermann wrote:
> philnik wrote:
> > Izaron wrote:
> > > Honestly I don't have a clue what these numbers `202302` (apparently the 
> > > `mm` format) would mean :( I'd be grateful if you could help me with 
> > > the right value for this.
> > https://isocpp.org/std/standing-documents/sd-6-sg10-feature-test-recommendations
> >  has generally the correct values for this. In this case it's `202207`.
> The draft standard is a better reference (http://eel.is/c++draft/cpp.cond#6). 
> Fortunately, it agrees with SD6.
The problem with the standard is that it doesn't list the old values, so it's 
useless if you want to implement a paper for which the FTM has already been 
bumped by another paper (or you don't know whether there is another paper).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144334

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

[PATCH] D144334: [Clang] Add C++2b attribute [[assume(expression)]]

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1448
+def Assume : StmtAttr {
+  let Spellings = [CXX11<"", "assume", 202302>];
+  let Documentation = [AssumeDocs];

Izaron wrote:
> Honestly I don't have a clue what these numbers `202302` (apparently the 
> `mm` format) would mean :( I'd be grateful if you could help me with the 
> right value for this.
https://isocpp.org/std/standing-documents/sd-6-sg10-feature-test-recommendations
 has generally the correct values for this. In this case it's `202207`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144334

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

[PATCH] D144192: GH60642: Fix ICE when checking a lambda defined in a concept definition

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D144192#4134902 , @erichkeane 
wrote:

> In D144192#4134900 , @philnik wrote:
>
>> In D144192#4134894 , @erichkeane 
>> wrote:
>>
>>> In D144192#4134893 , @philnik 
>>> wrote:
>>>
 In D144192#4134887 , @erichkeane 
 wrote:

> Looks like I have ot abandon this now?  Anyway, fix was committed, but I 
> forgot the differential-revision line.

 You can also Close the revision instead of abandoning it to make it 
 obvious that it has been landed.
>>>
>>> I didn't actually have that option in the "Add Action" section?  I was 
>>> looking for it, but it seems to have disappeared?
>>
>> I have it on my open revisions, maybe you just missed it?
>
> This is what I have for options on my open revisions (I went and grabbed a 
> snap from a different review): https://imgur.com/a/WYD3uAV
>
> So I don't see 'close' as an option?

Weird. I see a few more options: F26527345: Bildschirmfoto vom 2023-02-17 
15-30-33.png 


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

https://reviews.llvm.org/D144192

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

[PATCH] D144192: GH60642: Fix ICE when checking a lambda defined in a concept definition

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D144192#4134894 , @erichkeane 
wrote:

> In D144192#4134893 , @philnik wrote:
>
>> In D144192#4134887 , @erichkeane 
>> wrote:
>>
>>> Looks like I have ot abandon this now?  Anyway, fix was committed, but I 
>>> forgot the differential-revision line.
>>
>> You can also Close the revision instead of abandoning it to make it obvious 
>> that it has been landed.
>
> I didn't actually have that option in the "Add Action" section?  I was 
> looking for it, but it seems to have disappeared?

I have it on my open revisions, maybe you just missed it?


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

https://reviews.llvm.org/D144192

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

[PATCH] D144192: GH60642: Fix ICE when checking a lambda defined in a concept definition

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D144192#4134887 , @erichkeane 
wrote:

> Looks like I have ot abandon this now?  Anyway, fix was committed, but I 
> forgot the differential-revision line.

You can also Close the revision instead of abandoning it to make it obvious 
that it has been landed.


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

https://reviews.llvm.org/D144192

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

[PATCH] D143670: Stop claiming we support [[carries_dependency]]

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D143670#4116118 , @cor3ntin wrote:

> Thanks for doing this Aaron.
> Did you look in libc++ if they used it anywhere? (I assume they don't)

I'm not aware of any uses, and I would be quite surprised if it was used 
anywhere given that apparently nobody properly implemented it. IIUC this 
attribute is supposed to improve performance in some way, and we normally 
require a benchmark to show that some change actually does that if it claims to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143670

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

[PATCH] D141954: Forbid implicit conversion of constraint expression to bool

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D141954#4061962 , @erichkeane 
wrote:

> Speaking of: I guess the libcxx failure is NOT caused by me?  
> https://reviews.llvm.org/D141992
>
> I see this on a different review, despite this not being committed.

That's not great. Seems like some recently landed patch broke this. Do you mind 
committing the libc++-part of this patch to get the Clang CI green again? 
(Assuming the libc++ CI is green)


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

https://reviews.llvm.org/D141954

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

[PATCH] D141954: Forbid implicit conversion of constraint expression to bool

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D141954#4060246 , @erichkeane 
wrote:

> In D141954#4060244 , @philnik wrote:
>
>> In D141954#4060212 , @erichkeane 
>> wrote:
>>
>>> @ldionne @Mordante  The libcxx failure isn't clear what it is complaining 
>>> about, but I thought this fix might end up breaking a libcxx test, since it 
>>> would be easy to fall into the trap of having an implicit conversion here.
>>>
>>> Any chance you could prod at that test a little and see whose fault it is?
>>>
>>> ALSO: NOTE TO SELF: need release note!
>>
>> I think the problem is that `__type_traits/common_reference.h` doesn't 
>> re-export `__type_traits/common_type.h` in the modulemap. It //should// be 
>> enough to add `export common_type` to `module common_reference` 
>> (`libcxx/include/module.modulemap.in:1399`). See `module is_arithmetic` for 
>> an example. This is just a guess though, this stuff can be very weird.
>
> Oh man, I hope the others know what you're talking about :)  I don't really 
> know how I caused this though :/

I've applied your patch locally and this diff fixes the issue:

  diff --git a/libcxx/include/module.modulemap.in 
b/libcxx/include/module.modulemap.in
  index 5d4cf53aa334..30209b353d6b 100644
  --- a/libcxx/include/module.modulemap.in
  +++ b/libcxx/include/module.modulemap.in
  @@ -747,7 +747,10 @@ module std [system] {
 module assignable { private header 
"__concepts/assignable.h" }
 module boolean_testable   { private header 
"__concepts/boolean_testable.h" }
 module class_or_enum  { private header 
"__concepts/class_or_enum.h" }
  -  module common_reference_with  { private header 
"__concepts/common_reference_with.h" }
  +  module common_reference_with  {
  +private header "__concepts/common_reference_with.h"
  +export type_traits.common_reference
  +  }
 module common_with{ private header 
"__concepts/common_with.h" }
 module constructible  { private header 
"__concepts/constructible.h" }
 module convertible_to { private header 
"__concepts/convertible_to.h" }

I don't know why your patch changes the behaviour of clang here, but I wouldn't 
worry too much about it. There is a lot of weirdness around modules and 
dependent type names resulting in pretty much useless error messages. I hope 
the work on C++ modules reduces this weirdness, but I don't really know how 
that stuff works.


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

https://reviews.llvm.org/D141954

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

[PATCH] D141954: Forbid implicit conversion of constraint expression to bool

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D141954#4060212 , @erichkeane 
wrote:

> @ldionne @Mordante  The libcxx failure isn't clear what it is complaining 
> about, but I thought this fix might end up breaking a libcxx test, since it 
> would be easy to fall into the trap of having an implicit conversion here.
>
> Any chance you could prod at that test a little and see whose fault it is?
>
> ALSO: NOTE TO SELF: need release note!

I think the problem is that `__type_traits/common_reference.h` doesn't 
re-export `__type_traits/common_type.h` in the modulemap. It //should// be 
enough to add `export common_type` to `module common_reference` 
(`libcxx/include/module.modulemap.in:1399`). See `module is_arithmetic` for an 
example. This is just a guess though, this stuff can be very weird.


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

https://reviews.llvm.org/D141954

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

[PATCH] D141572: [C++] [Coroutines] Deprecates the '-fcoroutines-ts' flag

[Nikolas Klauser via Phabricator via cfe-commits]

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

LGTM from the libc++ side of things % nit. I think @aaron.ballman should take 
another look at the Clang changes.




Comment at: libcxx/utils/generate_header_tests.py:22
 
+"coroutine": "(defined(__cpp_impl_coroutine) && __cpp_impl_coroutine >= 
201902L) || (defined(__cpp_coroutines) && __cpp_coroutines >= 201703L)",
+

A `TODO LLVM17: simplify this to __cplusplus >= 202002L` would be nice. 


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

https://reviews.llvm.org/D141572

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

[PATCH] D133574: [C2x] reject type definitions in offsetof

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D133574#4051899 , @aaron.ballman 
wrote:

> In D133574#4051782 , @bkramer wrote:
>
>> GitHub finds around 1.9k instances of this pattern to compute `alignof`. 
>> There's a lot of duplicates and `#ifdefs` so the real number is going to be 
>> smaller, but it seems to be quite common. The annoying part is that there is 
>> no `_Alignof` in C99 so for many of those projects there is no easy fix.
>>
>> Can this be demoted to a warning?
>>
>> https://github.com/search?type=code&q=%2Foffsetof%5Cs*%5C%28%5Cs*struct%5Cs*%5C%7B%2F+lang%3Ac
>
> Adding clang-vendors/others with opinions on diagnostic severity because this 
> could be disruptive.
>
> 1. Yes, we can.
> 2. Blarg. I have sympathy because the standard was unclear on this point for 
> so long. I don't have as much sympathy for "there's no easy fix" -- the code 
> is not portable as-is (it's UB), and Clang supports `_Alignof` in C99 and 
> earlier, so for Clang there is an easy fix.
> 3. Instead, should we downgrade to a warning-defaults-to-error instead of 
> just a warning? That gives these folks a path forward while still making it 
> clear to users "don't do this." But if we think we won't reasonably be able 
> to turn this into an error, I think a warning is better.
> 4. Alternatively, should we document that we support this as an extension and 
> only give an extension warning for it?
>
> Regardless, we need to either make a decision shortly or we should revert 
> these changes until after the Clang 16 branch and re-land after that to give 
> ourselves more time to make a decision. If we don't have an obvious "this is 
> the right approach" answer and an implementation by Jan 19 (next Thursday), I 
> think we should revert to give ourselves breathing room.

I'm not a vendor, but another option would be to make it a 
warning-defaults-to-error and indicate that it will be promoted to an error in 
a few (2?) releases. That makes it less painful, but still make it an error 
eventually. I don't know if clang has done anything like this before though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133574

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

[PATCH] D141572: [C++] [Coroutines] Deprecates the '-fcoroutines-ts' flag

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D141572#4050492 , @ChuanqiXu wrote:

> @philnik when I run `ninja libcxx-generate-files` locally, it says `ninja: 
> error: unknown target 'libcxx-generate-files'`. The following off is my 
> configuring command:
>
>   cmake -G Ninja -S llvm -B build_libcxx_modules 
> -DLLVM_ENABLE_PROJECTS="clang"  -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi" 
> -DCMAKE_BUILD_TYPE=Release
>
> Do you know if anything goes wrong?

It's probably because you're using the bootstrapping build. Since this invokes 
another CMake command internally to build libc++, the targets aren't available. 
It should work with `cmake -G Ninja -S runtimes -B <...> 
-DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi"`.


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

https://reviews.llvm.org/D141572

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

[PATCH] D141572: [C++] [Coroutines] Deprecates the '-fcoroutines-ts' flag

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D141572#4047428 , @aaron.ballman 
wrote:

> Thank you for working on this! I think the Debian CI failure is unrelated, 
> but the libc++ one looks plausibly related:
>
>   
> /home/libcxx-builder/.buildkite-agent/builds/e6f11fd202bc-1/llvm-project/libcxx-ci/build/generic-cxx03/include/c++/v1/module.modulemap:773:10:
>  error: module 'std.coroutine' requires feature 'coroutines'
> module coroutine {
>^
>   
> /home/libcxx-builder/.buildkite-agent/builds/e6f11fd202bc-1/llvm-project/libcxx-ci/libcxx/test/libcxx/modules_include.sh.cpp:158:10:
>  note: submodule of top-level module 'std' implicitly imported here
>   #include 
>^
>   1 error generated.
>
> adding the libc++ reviewers for awareness.

Yes, this is related. It should be enough to add `"coroutine": 
"(defined(__cpp_impl_coroutine) && __cpp_impl_coroutine >= 201902L) || 
(defined(__cpp_coroutines) && __cpp_coroutines >= 201703L)"` to 
`libcxx/utils/generate_header_tests.py` and run `ninja libcxx-generate-files`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141572

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

[PATCH] D136953: [C++20] Diagnose invalid and reserved module names

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:282
+  StringRef FirstComponentName = Path[0].first->getName();
+  if (!getSourceManager().isInSystemHeader(Path[0].second) &&
+  (FirstComponentName == "std" ||

aaron.ballman wrote:
> philnik wrote:
> > aaron.ballman wrote:
> > > philnik wrote:
> > > > aaron.ballman wrote:
> > > > > philnik wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > philnik wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > philnik wrote:
> > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > erichkeane wrote:
> > > > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > > > > > cor3ntin wrote:
> > > > > > > > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > > > > > > > std modules should be irreverent with 
> > > > > > > > > > > > > > > > > > > > system headers; The intuition of the 
> > > > > > > > > > > > > > > > > > > > wording should be that the users can't 
> > > > > > > > > > > > > > > > > > > > declare modules like `std` or 
> > > > > > > > > > > > > > > > > > > > `std.compat` to avoid possible 
> > > > > > > > > > > > > > > > > > > > conflicting. The approach I imaged may 
> > > > > > > > > > > > > > > > > > > > be add a new compilation flags (call it 
> > > > > > > > > > > > > > > > > > > > `-fstd-modules`) now. And if the 
> > > > > > > > > > > > > > > > > > > > compiler found a `std` module 
> > > > > > > > > > > > > > > > > > > > declaration without `-fstd-modules`, 
> > > > > > > > > > > > > > > > > > > > emit an error.  
> > > > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > > > For now, I think we can skip the check 
> > > > > > > > > > > > > > > > > > > > for `-fstd-modules` and add it back 
> > > > > > > > > > > > > > > > > > > > when we starts to support std modules 
> > > > > > > > > > > > > > > > > > > > actually.
> > > > > > > > > > > > > > > > > > > The idea is that standard modules are 
> > > > > > > > > > > > > > > > > > > built from system directories... it seems 
> > > > > > > > > > > > > > > > > > > a better heuristic than adding a flag for 
> > > > > > > > > > > > > > > > > > > the purpose of 1 diagnostics ( maybe some 
> > > > > > > > > > > > > > > > > > > other system library could in theory 
> > > > > > > > > > > > > > > > > > > export std with no warning, but I'm not 
> > > > > > > > > > > > > > > > > > > super worried about that being a concern 
> > > > > > > > > > > > > > > > > > > in practice)
> > > > > > > > > > > > > > > > > > > The idea is that standard modules are 
> > > > > > > > > > > > > > > > > > > built from system directories...
> > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > This is not true. For example, if someday 
> > > > > > > > > > > > > > > > > > libc++ supports std modules, then we need 
> > > > > > > > > > > > > > > > > > to build the std modules in our working 
> > > > > > > > > > > > > > > > > > directory, which is not system directories. 
> > > > > > > > > > > > > > > > > > And **ideally**, we would distribute and 
> > > > > > > > > > > > > > > > > > install module file in the system 
> > > > > > > > > > > > > > > > > > directories. But it is irreverent with the 
> > > > > > > > > > > > > > > > > > path of the source file.
> > > > > > > > > > > > > > > > > > then we need to build the std modules in 
> > > > > > > > > > > > > > > > > > our working directory, which is not system 
> > > > > > > > > > > > > > > > > > directories.
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > `-isystem`, pragmas, and linemarkers are all 
> > > > > > > > > > > > > > > > > ways around that -- I don't think we need a 
> > > > > > > > > > > > > > > > > feature flag for this, unless I'm 
> > > > > > > > > > > > > > > > > misunderstanding something.
> > > > > > > > > > > > > > > > Although it may be a little bit nit picker, the 
> > > > > > > > > > > > > > > > module unit which declares the std modules 
> > > > > > > > > > > > > > > > won't be header. It is a module unit. So it 
> > > > > > > > > > > > > > > > requires we implement std modules by wrapping 
> > > > > > > > > > > > > > > > linemarkers around the std modules declaration, 
> > > > > > > > > > > > > > > > which looks a little bit overkill.
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > And another point is that maybe we need to 
> > > > > > > > > > > > > > > > introduce another feature flags to implement 
> > > > > > > > > > > > > > > > std modules. Although I tried to implement std 
> > > > > > > > > > > > > > > > modules within the current features, I can't 
> > > > > > > > > > > > > > > > prove we can implement std modules in that way 
> > > > > > > > > >

[PATCH] D136953: [C++20] Diagnose invalid and reserved module names

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:282
+  StringRef FirstComponentName = Path[0].first->getName();
+  if (!getSourceManager().isInSystemHeader(Path[0].second) &&
+  (FirstComponentName == "std" ||

aaron.ballman wrote:
> philnik wrote:
> > aaron.ballman wrote:
> > > philnik wrote:
> > > > aaron.ballman wrote:
> > > > > philnik wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > philnik wrote:
> > > > > > > > ChuanqiXu wrote:
> > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > erichkeane wrote:
> > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > > > cor3ntin wrote:
> > > > > > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > > > > > std modules should be irreverent with 
> > > > > > > > > > > > > > > > > > system headers; The intuition of the 
> > > > > > > > > > > > > > > > > > wording should be that the users can't 
> > > > > > > > > > > > > > > > > > declare modules like `std` or `std.compat` 
> > > > > > > > > > > > > > > > > > to avoid possible conflicting. The approach 
> > > > > > > > > > > > > > > > > > I imaged may be add a new compilation flags 
> > > > > > > > > > > > > > > > > > (call it `-fstd-modules`) now. And if the 
> > > > > > > > > > > > > > > > > > compiler found a `std` module declaration 
> > > > > > > > > > > > > > > > > > without `-fstd-modules`, emit an error.  
> > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > For now, I think we can skip the check for 
> > > > > > > > > > > > > > > > > > `-fstd-modules` and add it back when we 
> > > > > > > > > > > > > > > > > > starts to support std modules actually.
> > > > > > > > > > > > > > > > > The idea is that standard modules are built 
> > > > > > > > > > > > > > > > > from system directories... it seems a better 
> > > > > > > > > > > > > > > > > heuristic than adding a flag for the purpose 
> > > > > > > > > > > > > > > > > of 1 diagnostics ( maybe some other system 
> > > > > > > > > > > > > > > > > library could in theory export std with no 
> > > > > > > > > > > > > > > > > warning, but I'm not super worried about that 
> > > > > > > > > > > > > > > > > being a concern in practice)
> > > > > > > > > > > > > > > > > The idea is that standard modules are built 
> > > > > > > > > > > > > > > > > from system directories...
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > This is not true. For example, if someday 
> > > > > > > > > > > > > > > > libc++ supports std modules, then we need to 
> > > > > > > > > > > > > > > > build the std modules in our working directory, 
> > > > > > > > > > > > > > > > which is not system directories. And 
> > > > > > > > > > > > > > > > **ideally**, we would distribute and install 
> > > > > > > > > > > > > > > > module file in the system directories. But it 
> > > > > > > > > > > > > > > > is irreverent with the path of the source file.
> > > > > > > > > > > > > > > > then we need to build the std modules in our 
> > > > > > > > > > > > > > > > working directory, which is not system 
> > > > > > > > > > > > > > > > directories.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > `-isystem`, pragmas, and linemarkers are all ways 
> > > > > > > > > > > > > > > around that -- I don't think we need a feature 
> > > > > > > > > > > > > > > flag for this, unless I'm misunderstanding 
> > > > > > > > > > > > > > > something.
> > > > > > > > > > > > > > Although it may be a little bit nit picker, the 
> > > > > > > > > > > > > > module unit which declares the std modules won't be 
> > > > > > > > > > > > > > header. It is a module unit. So it requires we 
> > > > > > > > > > > > > > implement std modules by wrapping linemarkers 
> > > > > > > > > > > > > > around the std modules declaration, which looks a 
> > > > > > > > > > > > > > little bit overkill.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > And another point is that maybe we need to 
> > > > > > > > > > > > > > introduce another feature flags to implement std 
> > > > > > > > > > > > > > modules. Although I tried to implement std modules 
> > > > > > > > > > > > > > within the current features, I can't prove we can 
> > > > > > > > > > > > > > implement std modules in that way in the end of the 
> > > > > > > > > > > > > > day.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > Let me add some more words. The standards require 
> > > > > > > > > > > > > > us to implement std modules without deprecating the 
> > > > > > > > > > > > > > system headers. This strategy leads the direction 
> > > > > > > > > > > > > > to "implement the components in the original 
> > > > > > > > > > > > > > headers and control their visibility in the std 
> > > > > > > > > > > > > > module unit".
> > > > > > > > 

[PATCH] D136953: [C++20] Diagnose invalid and reserved module names

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:282
+  StringRef FirstComponentName = Path[0].first->getName();
+  if (!getSourceManager().isInSystemHeader(Path[0].second) &&
+  (FirstComponentName == "std" ||

aaron.ballman wrote:
> philnik wrote:
> > aaron.ballman wrote:
> > > philnik wrote:
> > > > aaron.ballman wrote:
> > > > > philnik wrote:
> > > > > > ChuanqiXu wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > erichkeane wrote:
> > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > cor3ntin wrote:
> > > > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > > > std modules should be irreverent with system 
> > > > > > > > > > > > > > > > headers; The intuition of the wording should be 
> > > > > > > > > > > > > > > > that the users can't declare modules like `std` 
> > > > > > > > > > > > > > > > or `std.compat` to avoid possible conflicting. 
> > > > > > > > > > > > > > > > The approach I imaged may be add a new 
> > > > > > > > > > > > > > > > compilation flags (call it `-fstd-modules`) 
> > > > > > > > > > > > > > > > now. And if the compiler found a `std` module 
> > > > > > > > > > > > > > > > declaration without `-fstd-modules`, emit an 
> > > > > > > > > > > > > > > > error.  
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > For now, I think we can skip the check for 
> > > > > > > > > > > > > > > > `-fstd-modules` and add it back when we starts 
> > > > > > > > > > > > > > > > to support std modules actually.
> > > > > > > > > > > > > > > The idea is that standard modules are built from 
> > > > > > > > > > > > > > > system directories... it seems a better heuristic 
> > > > > > > > > > > > > > > than adding a flag for the purpose of 1 
> > > > > > > > > > > > > > > diagnostics ( maybe some other system library 
> > > > > > > > > > > > > > > could in theory export std with no warning, but 
> > > > > > > > > > > > > > > I'm not super worried about that being a concern 
> > > > > > > > > > > > > > > in practice)
> > > > > > > > > > > > > > > The idea is that standard modules are built from 
> > > > > > > > > > > > > > > system directories...
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > This is not true. For example, if someday libc++ 
> > > > > > > > > > > > > > supports std modules, then we need to build the std 
> > > > > > > > > > > > > > modules in our working directory, which is not 
> > > > > > > > > > > > > > system directories. And **ideally**, we would 
> > > > > > > > > > > > > > distribute and install module file in the system 
> > > > > > > > > > > > > > directories. But it is irreverent with the path of 
> > > > > > > > > > > > > > the source file.
> > > > > > > > > > > > > > then we need to build the std modules in our 
> > > > > > > > > > > > > > working directory, which is not system directories.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > `-isystem`, pragmas, and linemarkers are all ways 
> > > > > > > > > > > > > around that -- I don't think we need a feature flag 
> > > > > > > > > > > > > for this, unless I'm misunderstanding something.
> > > > > > > > > > > > Although it may be a little bit nit picker, the module 
> > > > > > > > > > > > unit which declares the std modules won't be header. It 
> > > > > > > > > > > > is a module unit. So it requires we implement std 
> > > > > > > > > > > > modules by wrapping linemarkers around the std modules 
> > > > > > > > > > > > declaration, which looks a little bit overkill.
> > > > > > > > > > > > 
> > > > > > > > > > > > And another point is that maybe we need to introduce 
> > > > > > > > > > > > another feature flags to implement std modules. 
> > > > > > > > > > > > Although I tried to implement std modules within the 
> > > > > > > > > > > > current features, I can't prove we can implement std 
> > > > > > > > > > > > modules in that way in the end of the day.
> > > > > > > > > > > > 
> > > > > > > > > > > > Let me add some more words. The standards require us to 
> > > > > > > > > > > > implement std modules without deprecating the system 
> > > > > > > > > > > > headers. This strategy leads the direction to 
> > > > > > > > > > > > "implement the components in the original headers and 
> > > > > > > > > > > > control their visibility in the std module unit".
> > > > > > > > > > > > It may look like:
> > > > > > > > > > > > 
> > > > > > > > > > > > ```
> > > > > > > > > > > > //--- std.cppm
> > > > > > > > > > > > module;
> > > > > > > > > > > > #include 
> > > > > > > > > > > > ...
> > > > > > > > > > > > export module std;
> > > > > > > > > > > > ```
> > > > > > > > > > > > 
> > > > > > > > > > > > Then how can control the visibility?  In my original 
> > > > > > > > > > > > experiments, I use the style:
> > > > > > > > > > > >