[PATCH] D123235: [OpenMP] atomic compare fail : Parser & AST support

2023-11-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. FWIW, this appears to have broken some build bots: https://lab.llvm.org/buildbot/#/builders/121/builds/36635 -- can you revert and investigate (or fix-forward quickly)? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123235/new/ https://reviews.llvm.org/D1

[PATCH] D148474: [Clang] Fix ResolveConstructorOverload to not select a conversion function if we are going use copy elision

2023-11-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. The changes look reasonable to me (@rsmith had a lot of comments, but I think you addressed them; it would be nice if he could confirm), but should definitely come with a release note. So LGTM modulo those nits and any last-mi

[PATCH] D159309: [ASTMatchers] basic matcher support for ConceptReference

2023-11-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM as well btw -- @sammccall, it looks like this one may have fallen off your radar? (Please add a release note when landing though; we have a section for AST matchers specifically.) Repository: rG LLVM Github Monorepo C

[PATCH] D156910: [clang] Add pragma force_vectorize

2023-11-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D156910#4602946 , @kitaisreal wrote: > Hello @aaron.ballman, @erichkeane could you please review this revision ? I > wondering if this feature would be useful. My apologies, this fell off my radar by accident, sorry fo

[PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

2023-11-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D140996#4655288 , @bolshakov-a wrote: > Sorry, but I don't know what remains to be done here. It seems that the only > important question is about ABI, but I've already answered that the changes > under discussion seem

[PATCH] D86993: Document Clang's expectations of the C standard library.

2023-10-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D86993#4655487 , @rsmith wrote: > In D86993#4655474 , @RalfJung wrote: > >> With everything moving to github PRs, what are the next steps for this patch? > > I'm unlikely to have cy

[PATCH] D134475: [clang-cl] Add support for [[msvc::constexpr]] C++11 attribute

2023-10-27 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D134475#4655362 , @RIscRIpt wrote: > Should I re-submit it to GitHub? Youch... I'm sorry this hasn't managed to get up to the top of my queue yet! I agree with Erich, as unfortunate as it is not to complete the review i

[PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

2023-10-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D140996#4646880 , @cor3ntin wrote: > It would be nice to figure out a plan for this PR in the next few weeks, > before we get kicked out of Phab! Agreed -- @bolshakov-a are you still planning to actively work on this pa

[PATCH] D154581: [clang][Interp] Track existing InitMaps in InterpState

2023-10-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154581/new/ https://reviews.llvm.org/D154581 ___ cfe-commits mailing lis

[PATCH] D157252: [clang][ExprConst] Handle 0 type size in builtin_memcpy etc.

2023-10-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with a fixme comment added. Comment at: clang/test/Sema/builtin-memcpy.c:4-8 +int b() { + struct { } a[10]; + __builtin_memcpy(&a[2], a, 2); // expe

[PATCH] D156565: Diagnose use of VLAs in C++ by default

2023-10-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D156565#4654820 , @cor3ntin wrote: >> Any concerns with this approach? > > Sounds reasonable to me Thanks for the double-check! This should now be fixed in https://github.com/llvm/llvm-project/commit/f8d448d5e587a23886c

[PATCH] D156565: Diagnose use of VLAs in C++ by default

2023-10-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D156565#4654818 , @aaron.ballman wrote: > In D156565#4654773 , @nathanchance > wrote: > >> Is it expected that this introduces a warning for C code, as the commit >> message an

[PATCH] D156565: Diagnose use of VLAs in C++ by default

2023-10-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D156565#4654773 , @nathanchance wrote: > Is it expected that this introduces a warning for C code, as the commit > message and tests appear to only affect C++? A trivial example from the Linux > kernel: > > https://eli

[PATCH] D157252: [clang][ExprConst] Handle 0 type size in builtin_memcpy etc.

2023-10-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/test/Sema/builtin-memcpy.c:4-8 +int b() { + struct { } a[10]; + __builtin_memcpy(&a[2], a, 2); // expected-warning {{buffer has size 0, but size argument is 2}} + return 0; +} The only other test I'd

[PATCH] D156565: Diagnose use of VLAs in C++ by default

2023-10-20 Thread Aaron Ballman via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7339c0f782d5: Diagnose use of VLAs in C++ by default (authored by aaron.ballman). Changed prior to commit: https://reviews.llvm.org/D156565?vs=557

[PATCH] D157331: [clang] Implement C23

2023-10-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D157331#4654362 , @jrtc27 wrote: > In D157331#4654353 , @aaron.ballman > wrote: > >> In D157331#4654265 , @mstorsjo >> wrote: >> >>> Th

[PATCH] D154581: [clang][Interp] Track existing InitMaps in InterpState

2023-10-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/AST/Interp/Descriptor.cpp:328-330 +InitMap::T *InitMap::data() { return Data.get(); } +const InitMap::T *InitMap::data() const { return Data.get(); } Inline these into the header now? Even though these

[PATCH] D157331: [clang] Implement C23

2023-10-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D157331#4654265 , @mstorsjo wrote: > This change broke building a recent version of gettext. Gettext uses gnulib > for polyfilling various utility functions. Since not long ago, these > functions internally use ``, > h

[PATCH] D156453: [clang][Interp] Create only globals when initializing a global variable

2023-10-17 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1213 std::optional SubExprT = classify(SubExpr); - if (E->getStorageDuration() == SD_Static) { +

[PATCH] D156453: [clang][Interp] Create only globals when initializing a global variable

2023-10-16 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1213 std::optional SubExprT = classify(SubExpr); - if (E->getStorageDuration() == SD_Static) { + bool IsStatic = E->getStorageDuration() == SD_Static; + if (GlobalDecl || IsStatic) { -

[PATCH] D156453: [clang][Interp] Create only globals when initializing a global variable

2023-10-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D156453#4652939 , @tbaeder wrote: > It gets interpreted as a constant expression in > `Sema::CheckCompleteVariableInitialization()`: > > * #0: Context.cpp:73 > clang::interp::Context::evaluateAsInitializer(this=0x000

[PATCH] D156565: Diagnose use of VLAs in C++ by default

2023-10-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156565/new/ https://reviews.llvm.org/D156565 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158069: [clang][Interp] Fix getIndex() for composite array elements

2023-10-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158069/new/ https://reviews.llvm.org/D158069 ___ cfe-commits mailing lis

[PATCH] D154581: [clang][Interp] Track existing InitMaps in InterpState

2023-10-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/AST/Interp/Descriptor.cpp:42 const Descriptor *D) { + new (Ptr) InitMapPtr(std::nullopt); + tbaeder wrote: > aaron.ballman wrote: > > This worries me a little bit for a few reaso

[PATCH] D159351: [Sema] Change order of displayed overloads in diagnostics

2023-10-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I think the changes should come with a release note so users know about the improved user experience (it would be great to show the code example from this patch summary in the re

[PATCH] D145284: WIP [clang] adds capabilities for SARIF to be written to file

2023-10-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. @cjdb -- how would you like to proceed with this review? There's already quite a bit of discussion on the current Phab patch set, but it seems unlikely that we'd be able to wrap everything up by the Nov 15th deadline. But then again, GitHub has no ability to do st

[PATCH] D155548: [clang][ExprConst] Short-circuit ConstantExpr evaluation

2023-10-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. In D155548#4653363 , @cor3ntin wrote: > @aaron.ballman Given the recent discussion we had on both github > (https://github.com/llvm/llvm-project/pull/66203) and privately, I'm going to

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

2023-10-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D140828#4653389 , @nikic wrote: > FYI this causes some compile-time regression: > http://llvm-compile-time-tracker.com/compare.php?from=bc7d88faf1a595ab59952a2054418cdd0d98&to=af4751738db89a142a8880c782d12d4201b222a8

[PATCH] D158069: [clang][Interp] Fix getIndex() for composite array elements

2023-10-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/AST/Interp/Pointer.h:81-88 + /// Equality operators are just for tests. + bool operator==(const Pointer &P) const { +return Pointee == P.Pointee && Base == P.Base && Offset == P.Offset; + } + + bool operator!=(con

[PATCH] D154581: [clang][Interp] Track existing InitMaps in InterpState

2023-10-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added subscribers: hubert.reinterpretcast, rsmith. aaron.ballman added inline comments. Comment at: clang/lib/AST/Interp/Descriptor.cpp:42 const Descriptor *D) { + new (Ptr) InitMapPtr(std::nullopt); + This worries me a lit

[PATCH] D159393: [clang] Fix several issues in the generated AttrHasAttributeImpl.inc

2023-10-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM -- that release note is *awesome*, thank you for that! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159393/new/ https://reviews.llvm.org/D159393 ___

[PATCH] D157331: [clang] Implement C23

2023-10-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Please wait for @jrtc27 to also sign off before landing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157331/new/ https://r

[PATCH] D157331: [clang] Implement C23

2023-10-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D157331#4653222 , @jrtc27 wrote: > One more thought: we need to specify a triple for the tests as otherwise > it'll use whatever default you have configured in LLVM, which will affect the > exact code generated (especia

[PATCH] D157331: [clang] Implement C23

2023-10-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/test/C/C2x/n2683_2.c:7-15 +// CHECK:%result64 = alloca i64, align 8 +// CHECK:%flag_add = alloca i8, align 1 +// CHECK:store i64 0, ptr %result64, align 8 +// CHECK:%0 = call { i64, i1 } @llvm.sadd.with.overfl

[PATCH] D157331: [clang] Implement C23

2023-10-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/test/C/C2x/n2683_2.c:7-15 +// CHECK:%result64 = alloca i64, align 8 +// CHECK:%flag_add = alloca i8, align 1 +// CHECK:store i64 0, ptr %result64, align 8 +// CHECK:%0 = call { i64, i1 } @llvm.sadd.with.overfl

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

2023-10-05 Thread Aaron Ballman via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG5d78b78c8538: [C2X] N3007 Type inference for object definitions (authored by to268, committed by aaron.ballman). Changed prior to commit: https://reviews.llvm.org/D133289?vs=557115&id=557610#toc Reposi

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

2023-10-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I spotted some minor issues, but this LGTM for the initial implementation. Thank you for all your effort on this project!! There are some outstanding issues that should be handle

[PATCH] D154262: [clang][Interp] LambdaThisCaptures

2023-10-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a request for some comments. Comment at: clang/lib/AST/Interp/ByteCodeEmitter.h:76 llvm::DenseMap> LambdaCaptures; - unsigned LambdaThisCapt

[PATCH] D156453: [clang][Interp] Create only globals when initializing a global variable

2023-10-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. I guess I'm a bit lost as to what the code example has to do with the constant expression interpreter in the first place. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156453/new/ https://reviews.llvm.org/D156453 __

[PATCH] D158516: [clang][Interp] Only lazily visit constant globals

2023-10-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, I suspect this may have to change for `constexpr` support in C23, but we can burn that bridge when we get to it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D158069: [clang][Interp] Fix getIndex() for composite array elements

2023-10-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/AST/Interp/Pointer.h:81-88 + /// Equality operators are just for tests. + bool operator==(const Pointer &P) const { +return Pointee == P.Pointee && Base == P.Base && Offset == P.Offset; + } + + bool operator!=(con

[PATCH] D158413: [Lex] Introduce Preprocessor::LexTokensUntilEOF()

2023-10-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a minor naming nit. I think finding a way to unify with `SkipUntil` would be good follow-up work, though. Comment at: clang/lib/Lex/Preprocesso

[PATCH] D157331: [clang] Implement C23

2023-10-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Precommit CI is still finding some issues that boil down to: # .---command stderr # | C:\ws\src\clang\test\Modules/Inputs/System/usr/include\module.map:19:12: error: header 'stdckdint.h' not found # |19 | header "stdckdint.h" # | |

[PATCH] D148381: [Clang] Implement the 'counted_by' attribute

2023-10-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D148381#4652825 , @void wrote: > Yes, I mean to do it as a direct follow-up. 😊 Okay, let's go that route then. This gets a fairly sig

[PATCH] D156565: Diagnose use of VLAs in C++ by default

2023-10-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 557575. aaron.ballman added a comment. Updated based on review feedback. Specifically: - Added test cases, fixed handling of parenthesized expressions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156565/new/ https://reviews.llvm.org/D156565

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

2023-10-03 Thread Aaron Ballman via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGaf16a4e131c1: Improve error message for constexpr constructors of virtual base classes (authored by NoumanAmir657, committed by aaron.ballman). Cha

[PATCH] D156565: Diagnose use of VLAs in C++ by default

2023-10-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman marked an inline comment as not done. aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:2617 + } else if (getLangOpts().CPlusPlus) { +if (getLangOpts().CPlusPlus11 && IsStaticAssertLike(ArraySize, Context)) + VLADiag = getLangOpt

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

2023-10-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. The only thing left are the comments on the release note, but I can fix that up when landing if you need me to commit on your behalf. If so, what name and email address would you like me to use for patch attribution? Otherwise, if you have commit privileges, then

[PATCH] D156565: Diagnose use of VLAs in C++ by default

2023-10-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:147-149 +def ext_vla_cxx_static_assert : ExtWarn< + "variable length arrays in C++ are a Clang extension; did you mean to use " + "'

[PATCH] D148381: [Clang] Implement the 'counted_by' attribute

2023-10-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:8420-8428 +if (Result.getResultKind() == LookupResult::Found) { + SourceRange SR = CBA->getCountedByFieldLoc(); + Diag(SR.getBegin(), + diag::err_flexible_array_counted_by_att

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

2023-10-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/docs/ReleaseNotes.rst:211-212 with terminals with dark background colors. This is also more consistent with GCC. +- Clang now displays an improved diagnostic and a note when defaulted special + member is a contexpr in a

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

2023-10-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/docs/ReleaseNotes.rst:211-212 with terminals with dark background colors. This is also more consistent with GCC. +- Clang now displays an improved diagnostic and a note when defaulted special + member is a contexpr in a

[PATCH] D148381: [Clang] Implement the 'counted_by' attribute

2023-10-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D148381#4652056 , @void wrote: > FINALLY! found out how to do suggestions for typos. <3 Thank you for your perseverance! Comment at: clang/lib/Sema/SemaDeclAttr.cpp:8420-8428 +if (Result.getResult

[PATCH] D158069: [clang][Interp] Fix getIndex() for composite array elements

2023-10-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/AST/Interp/Pointer.h:81-88 + /// Equality operators are just for tests. + bool operator==(const Pointer &P) const { +return Pointee == P.Pointee && Base == P.Base && Offset == P.Offset; + } + + bool operator!=(con

[PATCH] D156565: Diagnose use of VLAs in C++ by default

2023-10-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 557531. aaron.ballman marked 4 inline comments as done. aaron.ballman added a comment. Updated based on review feedback. Specifically: - Updated a FIXME comment in a test to clarify what's happening. - Reworded diagnostic to add "in C++" CHANGES SINCE

[PATCH] D156565: Diagnose use of VLAs in C++ by default

2023-10-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman marked 4 inline comments as done. aaron.ballman added a subscriber: tbaeder. aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:143 +def ext_vla_cxx : ExtWarn< + "variable length arrays are a Clang extension">, InGroup

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

2023-10-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Accepting as-is; we'll keep our eyes peeled for any post-commit feedback. Thank you for all the hard work on this one, it was a big, involved patch! Repository: rG LLVM Github

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

2023-09-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D158540#4651859 , @NoumanAmir657 wrote: > @aaron.Ballman > Do I need to make changes other than this? The virtual base diagnostic > doesn't have a test case in files that would generate it. Should I add the > above ex

[PATCH] D156565: Diagnose use of VLAs in C++ by default

2023-09-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 557483. aaron.ballman added a comment. Updated based on feedback on the RFC. Specifically, this variant recognizes `int array[cond ? 1 : -1];` or similar constructs and recommends using a `static_assert` instead. This new diagnostic is under its own wa

[PATCH] D157331: [clang] Implement C23

2023-09-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Precommit CI seems to be finding issues with your changes: [3278/3832] Building CXX object tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaChecking.cpp.o FAILED: tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaChecking.cpp.o ccache /usr/bin/c++ -D

[PATCH] D133361: [BPF] Attribute preserve_static_offset for structs

2023-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a reviewer: erichkeane. aaron.ballman added a comment. I don't have a whole lot of opinions on the attribute itself (I'm not super familiar with BPF), but did spot some things to do on the Clang side. This will also need reviewers for the LLVM changes -- any ideas on who usua

[PATCH] D157331: [clang] Implement C23

2023-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:373-387 + bool CkdOperation = false; + SourceManager &SM = S.getSourceManager(); + if (BuiltinID == Builtin::BI__builtin_add_overflow && +TheCall->getExprLoc().isMacroID() && Lexer::getImm

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

2023-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Aside from some minor nits in the release notes, I think this LGTM. I'm going to hold off on accepting officially for a day or two just so other code reviews have a chance to weigh in (the codegen code owners were only added to the review today). ==

[PATCH] D155572: [clang][Interp] Start implementing binary operators for complex types

2023-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D155572#4646353 , @tbaeder wrote: > In D155572#4645997 , @aaron.ballman > wrote: > >> Hmmm,

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

2023-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D158540#4643563 , @NoumanAmir657 wrote: > **Code:** > > struct Base { >constexpr Base() = default; > }; > struct Derived : virtual Base { > constexpr Derived() = default; > }; > > struct NoCopyMove {

[PATCH] D158069: [clang][Interp] Fix getIndex() for composite array elements

2023-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/AST/Interp/Context.h:88 + /// Returns the program. This is only needed for unittests. + Program &getProgram() const { return *P.get(); } + tbaeder wrote: > cor3ntin wrote: > > This should return a const

[PATCH] D148381: [Clang] Implement the 'counted_by' attribute

2023-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:4246 +private: + SourceRange countedByFieldLoc; +public: void wrote: > aaron.ballman wrote: > > void wrote: > > > erichkeane wrote: > > > > aaron.ballman wrote: > > >

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

2023-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/test/SemaCXX/cxx2b-deducing-this.cpp:192 +f(); // expected-error{{call to non-static member function without an object argument}} +f(Over_Call_Func_Example{}); // expected-error{{call to non-static m

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

2023-09-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/Sema/Sema.h:3775 bool UseMemberUsingDeclRules, bool ConsiderCudaAttrs = true, - bool ConsiderRequiresClauses = true); + bool UseOverrideRules = false);

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

2023-09-27 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added reviewers: rjmccall, efriedma. aaron.ballman added a comment. I think we're getting pretty close! My goal is to get this landed ASAP; I do not think it needs to be hidden behind a feature flag (-std=c++2b is sufficient), and it's good that we're not defining the feature test

[PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

2023-09-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/docs/ReleaseNotes.rst:267-269 +- Fix segfault while running clang-rename on a non existing file. + (`#36471 `_) +- Fix crash when diagnosing incorrect usage of ``_Nullable``

[PATCH] D141192: [Clang] Fix warnings on bad shifts.

2023-09-26 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D141192#4647977 , @budimirarandjelovicsyrmia wrote: > Ping. Given the lack of response, I think it's safe to assume this patch has been abandoned by the author. I'd recommend starting a new patch review for it on GitH

[PATCH] D159522: [Clang][C] Fixed a bug where we reject an _Atomic qualified integer in a switch statment

2023-09-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM (with a tiny nit with the formatting in the release notes), thank you for the fix! Comment at: clang/docs/ReleaseNotes.rst:223 (`#64836

[PATCH] D159522: [Clang][C] Fixed a bug where we reject an _Atomic qualified integer in a switch statment

2023-09-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Thank you for working on this! You should also add a release note for the fix. Comment at: clang/lib/Sema/SemaOverload.cpp:6306-6310 + ExprResult Converted = DefaultLvalueConversion(From); + QualType T = Converted.isUsable() ? Converted.get()->g

[PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

2023-09-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. In D147844#4641735 , @ldionne wrote: > Ping. What's missing to land this? Folks have raised enough concerns about chattiness o

[PATCH] D158413: [Lex] Introduce Preprocessor::LexTokensUntilEOF()

2023-09-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Lex/Preprocessor.cpp:1000 + std::vector toks; + while (1) { +Token tok; v.g.vassilev wrote: > aaron.ballman wrote: > > Hahnfeld wrote: > > > aaron.ballman wrote: > > > > I'd prefer not to assume the

[PATCH] D158413: [Lex] Introduce Preprocessor::LexTokensUntilEOF()

2023-09-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Lex/Preprocessor.cpp:1000 + std::vector toks; + while (1) { +Token tok; Hahnfeld wrote: > aaron.ballman wrote: > > I'd prefer not to assume the token stream has an EOF token (perhaps the > > stream

[PATCH] D155572: [clang][Interp] Start implementing binary operators for complex types

2023-09-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D155572#4645995 , @tbaeder wrote: > In D155572#4542484 , @aaron.ballman > wrote: > >> In D155572#4541403 , @tbaeder >> wrote: >> >>> In

[PATCH] D148381: [Clang] Implement the 'counted_by' attribute

2023-09-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/include/clang/AST/Decl.h:4272-4275 +FieldDecl *FD = nullptr; +for (FieldDecl *Field : fields()) + FD = Field; +return FD; void wrote: > aaron.ballman wrote: > > Could this be implemented as:

[PATCH] D158415: [Lex] Handle repl_input_end in Preprocessor::LexAll()

2023-09-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. The idea LGTM when D158413 lands, even if that lands with a slightly different design. (IOW, once D158413 lands, feel free to push this change without furth

[PATCH] D158413: [Lex] Introduce Preprocessor::LexAll()

2023-09-14 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Lex/Preprocessor.cpp:998 +std::vector Preprocessor::LexAll() { + std::vector toks; v.g.vassilev wrote: > Shouldn't we take the results as a `LexAll(std::vector &result)`? > > Perhaps we should rename

[PATCH] D158502: [clang][Interp] Actually consider ConstantExpr result

2023-09-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158502/new/ https://reviews.llvm.org/D158502 ___ cfe-commits mailing list

[PATCH] D158472: [clang][Diagnostics] Emit fix-it hint separately on overload resolution failure

2023-09-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D158472#4605228 , @hazohelet wrote: > This breaks the one-note-for-one-overload-candidate rule of overload > resolution failure diagnostics > (https://github.com/llvm/llvm-project/blob/ff08c8e57e39d7970b65637595cdc22190

[PATCH] D148381: [Clang] Implement the 'counted_by' attribute

2023-09-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:857 + if (IsDynamic) { +const LangOptions::StrictFlexArraysLevelKind StrictFlexArraysLevel = +getLangOpts().getStrictFlexArraysLevel(); nickdesaulniers wrote: > aaron.ba

[PATCH] D148381: [Clang] Implement the 'counted_by' attribute

2023-09-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman requested changes to this revision. aaron.ballman added reviewers: efriedma, rjmccall, erichkeane. aaron.ballman added a comment. This revision now requires changes to proceed. Thank you for working on this! Please be sure to also add a release note to `clang/docs/ReleaseNotes.rst`

[PATCH] D159103: [Driver][HLSL] Improve diagnostics for invalid shader model and stage

2023-09-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159103/new/ https://reviews.llvm.org/D159103 ___ cfe-commits mailing list cfe-comm

[PATCH] D151834: Include math-errno with fast-math

2023-09-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D151834#4644391 , @zahiraam wrote: > In D151834#4644378 , @aaron.ballman > wrote: > >> In D151834#4644375 , @zahiraam >> wrote: >> >>>

[PATCH] D151834: Include math-errno with fast-math

2023-09-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D151834#4644375 , @zahiraam wrote: > In D151834#4644373 , @aaron.ballman > wrote: > >> In D151834#4643925 , @uabelho >> wrote: >> >>> H

[PATCH] D151834: Include math-errno with fast-math

2023-09-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D151834#4643925 , @uabelho wrote: > Hi @zahiraam , > > I have a couple of downstream testcases that fail with this patch. > Before > > > clang bbi-86364.c -lm -O3 > > ./a.out > > passed but with the patch the assert i

[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

2023-09-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added reviewers: efriedma, rjmccall. aaron.ballman added a comment. Adding codegen code owners as reviewers just so they're aware (I think you're fine to land these changes if you don't hear back in the next day or two). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

2023-09-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D158540#4638605 , @NoumanAmir657 wrote: > In D158540#4632457 , @NoumanAmir657 > wrote: > >> @aaron.ballman >> This error gets generated on test cases even when a struct/class a

[PATCH] D141714: Fix ast print of variables with attributes

2023-09-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Comment at: clang/lib/AST/DeclPrinter.cpp:269-276 +static bool mustPrintOnLeftSide(attr::Kind kind) { + switch (kind) { +#include "clang/Basic/AttrLeftSideMustPrintList.inc" +return true; + default: +return false; + }

[PATCH] D152752: [MS] Fix passing aligned records by value in some cases

2023-09-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D152752#4641396 , @rnk wrote: > Following up, the vararg fix is here: > https://github.com/llvm/llvm-project/pull/65692 Thank you for the fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D151834: Include math-errno with fast-math

2023-09-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D151834#4639600 , @zahiraam wrote: > In D151834#4639536 , @aaron.ballman > wrote: > >> LGTM, but please add a release note describing the changes when you land >> them. > > @aar

[PATCH] D151834: Include math-errno with fast-math

2023-09-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, but please add a release note describing the changes when you land them. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151834/new/ https://reviews.llvm.org/D151834

[PATCH] D159393: [clang] Fix several issues in the generated AttrHasAttributeImpl.inc

2023-09-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a subscriber: Endill. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, though please add a release note because there are user-facing changes regarding what the feature test macros return or what spelli

[PATCH] D159435: [NFC] remove unneded header includes

2023-09-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159435/new/ https://reviews.llvm.org/D159435 _

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

2023-09-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, I like this as a simplification, too! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159247/new/ https://reviews.llvm.org/D15

[PATCH] D159383: [Headers] Remove musl-related comment about NULL

2023-09-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. As far as the change goes, this is fine (it keeps the status quo we previously had), but I still wonder if we should be trying to detect that the user is working with musl and do an include_next so our stddef.h header never get

[PATCH] D159312: [Headers] Remove a space in NULL define

2023-09-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D159312#4633989 , @dalias wrote: > I don't understand the motivation of trying to match musl's definition here. > musl explicitly **does not support** using a compiler-provided `stddef.h` or > other standard headers. If

  1   2   3   4   5   6   7   8   9   10   >