[PATCH] D132136: [clang] Perform implicit lvalue-to-rvalue cast with new interpreter

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM, this wasn't quite what I had in mind, but I can see how my suggestion was going to be far too tricky to try to implement as a lit test. Thanks for adding the unit test coverage! CHANGES SINCE LAST ACTION https://revie

[PATCH] D133425: Silence -Wctad-maybe-unsupported stemming from system headers

aaron.ballman created this revision. aaron.ballman added reviewers: dblaikie, ldionne, clang-language-wg, libc++. Herald added a project: All. aaron.ballman requested review of this revision. Herald added a project: clang. Currently, this diagnostic fires on system header code that the user has no

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

aaron.ballman added a reviewer: jcranmer-intel. aaron.ballman added a comment. I made some suggested documentation changes, but would love to hear more from folks heavily into floating-point options. Comment at: clang/docs/UsersManual.rst:1409-1410 + Disable fast-math mode.

[PATCH] D133202: [Clang] Avoid __builtin_assume_aligned crash when the 1st arg is array type

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6ed21fc51523: Avoid __builtin_assume_aligned crash when the 1st arg is array type (authored by yronglin, committed by aaron.ballman). Repository:

[PATCH] D133202: [Clang] Avoid __builtin_assume_aligned crash when the 1st arg is array type

aaron.ballman added a comment. I've pushed the changes up for you, thanks! In D133202#3774801 , @yronglin wrote: > ping~ Something to keep in mind for the future: we typically only "ping" a review after about a week of no activity on it (just because f

[PATCH] D132918: [clang] Fix a crash in constant evaluation

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132918/new/ https://reviews.llvm.org/D132918

[PATCH] D132997: [clang][Interp] Handle DeclRefExpr of reference types

aaron.ballman added a comment. In D132997#3763367 , @tbaeder wrote: > In D132997#3763094 , @shafik wrote: > >> I think like @aaron.ballman was saying in another PR you should aim for as >> complete set of tests a

[PATCH] D133325: [Driver] Allow search of included response files as configuration files

aaron.ballman added inline comments. Comment at: clang/docs/UsersManual.rst:947-949 +Option `--search-config-dirs` changes the search algorith if the included +file is specified without a directory separator. In this case the file is looked +for in the same directories as config

[PATCH] D132727: [clang][Interp] Implement array initializers and subscript expressions

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/D132727/new/ https://reviews.llvm.org/D132727 ___ cfe-commits mailing lis

[PATCH] D127284: [clang-repl] Support statements on global scope in incremental mode.

aaron.ballman added a comment. In D127284#3776036 , @rsmith wrote: > In terms of the high-level direction here, I think it would make sense to > approach this by adding a full-fledged language extension to Clang to allow > statements at the top level (w

[PATCH] D133425: Silence -Wctad-maybe-unsupported stemming from system headers

aaron.ballman added a comment. In D133425#3775353 , @ldionne wrote: > Wouldn't re-applying > https://github.com/llvm/llvm-project/commit/fcd549a7d8284a8e7c763fee3da2206acd8cdc4f > (which had been reverted IIUC) be a more precise fix for this problem? We

[PATCH] D133425: Silence -Wctad-maybe-unsupported stemming from system headers

aaron.ballman added a comment. In D133425#3777598 , @aaron.ballman wrote: > In D133425#3775353 , @ldionne wrote: > >> Re-applying `fcd549a7d8284a8e7c763fee3da2206acd8cdc4f` would not require any >> Clang changes

[PATCH] D127284: [clang-repl] Support statements on global scope in incremental mode.

aaron.ballman added a comment. In D127284#3776805 , @aaron.ballman wrote: > In D127284#3776036 , @rsmith wrote: > >> In terms of the high-level direction here, I think it would make sense to >> approach this by

[PATCH] D132977: [HLSL] Call global constructors inside entry

aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:167-170 +assert(cast(CS->getOperand(0))->getValue() == 65535 && + "HLSL doesn't support setting priority for global ctors."); +assert(isa(CS->getOperand(2)) && + "HLSL

[PATCH] D132997: [clang][Interp] Handle DeclRefExpr of reference types

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 a comment in the test, thanks! Comment at: clang/test/AST/Interp/references.cpp:91 + +static_assert(RefToMemberExpr() == 11, "");

[PATCH] D133499: [clang]: Add DeclContext::dumpDecl() in order to conveniently dump an AST from a DeclContext.

aaron.ballman added inline comments. Comment at: clang/lib/AST/ASTDumper.cpp:204 +LLVM_DUMP_METHOD void DeclContext::dumpDecl() const { + if (const Decl *D = dyn_cast(this)) +D->dump(); Comment at: clang/lib/AST/ASTDumper.cpp:205 + if (c

[PATCH] D132977: [HLSL] Call global constructors inside entry

aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:167-170 +assert(cast(CS->getOperand(0))->getValue() == 65535 && + "HLSL doesn't support setting priority for global ctors."); +assert(isa(CS->getOperand(2)) && + "HLSL

[PATCH] D129883: [HLSL] Support cbuffer/tbuffer for hlsl.

aaron.ballman added a comment. (I didn't have the chance to do a complete review, but I did a partial one and spotted some things) Comment at: clang/lib/Parse/ParseHLSL.cpp:79-80 +ParsedAttributes Attrs(AttrFactory); +MaybeParseCXX11Attributes(Attrs); +MaybeParseMi

[PATCH] D133499: [clang]: Add DeclContext::dumpDecl() in order to conveniently dump an AST from a DeclContext.

aaron.ballman added inline comments. Comment at: clang/lib/AST/ASTDumper.cpp:205 + if (const Decl *D = dyn_cast(this)) +D->dump(); +} erichkeane wrote: > aaron.ballman wrote: > > erichkeane wrote: > > > One thing to note is that the 'else' case here is a lit

[PATCH] D129883: [HLSL] Support cbuffer/tbuffer for hlsl.

aaron.ballman added inline comments. Comment at: clang/lib/Parse/ParseHLSL.cpp:79-80 +ParsedAttributes Attrs(AttrFactory); +MaybeParseCXX11Attributes(Attrs); +MaybeParseMicrosoftAttributes(Attrs); + beanz wrote: > python3kgae wrote: > > aaron.ballman

[PATCH] D132977: [HLSL] Call global constructors inside entry

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132977/new/ https://reviews.llvm.org/D132977

[PATCH] D133499: [clang]: Add DeclContext::dumpDecl() in order to conveniently dump an AST from a DeclContext.

aaron.ballman added inline comments. Comment at: clang/lib/AST/ASTDumper.cpp:205 + if (const Decl *D = dyn_cast(this)) +D->dump(); +} shafik wrote: > aaron.ballman wrote: > > erichkeane wrote: > > > aaron.ballman wrote: > > > > erichkeane wrote: > > > > > On

[PATCH] D133029: [Sema] Allow to diagnose the references to std::vector with incomplete T

aaron.ballman added subscribers: erichkeane, ldionne. aaron.ballman added a comment. CC @erichkeane for attribute questions, and CC @ldionne for STL questions and design/usability of the proposed attribute In D133029#3779663 , @ilya-biryukov wrote: > I

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D129755#3777038 , @aaronpuchert wrote: > I was under the impression that we've already switched to C++17, but the > Windows pre-subm

[PATCH] D133570: #57353 on github

aaron.ballman added reviewers: rjmccall, efriedma. aaron.ballman added a comment. In D133570#3779856 , @OfekShochat wrote: > its my first time contributing. this is probably very bad and goes against a > lot of guidelines, but I wouldnt resist to fix th

[PATCH] D133570: Clang codegen, fixes issue with emitting partially initialized constant arrays larger than 2^32

aaron.ballman added a comment. In D133570#3780071 , @OfekShochat wrote: > hello! thank you so much, Ill do that now, thanks! just a question, what are > your guidelines when it comes to commit naming/number etc? Thanks, the new title is great! I'm not

[PATCH] D133029: [Sema] Allow to diagnose the references to std::vector with incomplete T

aaron.ballman added a comment. In D133029#3779970 , @ldionne wrote: > Just my .02, but I am conflicted between: > > 1. Simply not doing anything -- the diagnostic users get when they violate > the requirement currently is probably not that bad? I did see

[PATCH] D133570: Clang codegen, fixes issue with emitting partially initialized constant arrays larger than 2^32

aaron.ballman added a comment. In D133570#3780171 , @OfekShochat wrote: > hello again, I added two tests, one for llvm and one for clang. how do I run > those specifically so I see I didnt do something wrong? The short answer is: you can run the `check

[PATCH] D129883: [HLSL] Support cbuffer/tbuffer for hlsl.

aaron.ballman added inline comments. Comment at: clang/lib/Parse/ParseHLSL.cpp:79-80 +ParsedAttributes Attrs(AttrFactory); +MaybeParseCXX11Attributes(Attrs); +MaybeParseMicrosoftAttributes(Attrs); + beanz wrote: > aaron.ballman wrote: > > beanz wrote:

[PATCH] D133088: [Clang] Fix wrong diagnostic for scope identifier with internal linkage

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5904-5905 def err_block_extern_cant_init : Error< - "'extern' variable cannot have an initializer">; + "declaration of block scope identifier with %select{external|internal}0 " +

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default standard

aaron.ballman added a comment. In D131465#3780717 , @AaronLiu wrote: > This cause a large amount failures in our testing with errors such as: > > error: ISO C++17 does not allow 'register' storage class specifier > [-Wregister] > > error: ISO C++17 d

[PATCH] D133499: [clang]: Add DeclContext::dumpDecl() in order to conveniently dump an AST from a DeclContext.

aaron.ballman added inline comments. Comment at: clang/lib/AST/ASTDumper.cpp:205 + if (const Decl *D = dyn_cast(this)) +D->dump(); +} shafik wrote: > aaron.ballman wrote: > > shafik wrote: > > > aaron.ballman wrote: > > > > erichkeane wrote: > > > > > aaron.

[PATCH] D133194: rewording note note_constexpr_invalid_cast

aaron.ballman added inline comments. Comment at: clang/test/Sema/cast.c:2 // RUN: %clang_cc1 -fsyntax-only -triple x86_64-unknown-unknown %s -verify +// RUN: %clang_cc1 -std=c99 -fsyntax-only -DConstantCast -Wvla -verify %s + Instead of adding this RUN line, I

[PATCH] D129883: [HLSL] Support cbuffer/tbuffer for hlsl.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129883/new/ https://reviews.llvm.org/D129883

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

aaron.ballman added a comment. In D132952#3773469 , @inclyc wrote: >> Yeah, that's a different way of delineating than I was thinking originally >> and it's worth more thought. I was thinking the separation would be "this is >> a VLA" (for people who wa

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

aaron.ballman added a comment. In D133066#3768091 , @zhouyizhou wrote: > In D133066#3767146 , @aaron.ballman > wrote: > >> In D133066#3765503 , @zhouyizhou >> wrote: >>

[PATCH] D133194: rewording note note_constexpr_invalid_cast

aaron.ballman added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:7476-7483 +CCEDiag(E, diag::note_constexpr_invalid_cast) +<< 0 << Info.Ctx.getLangOpts().CPlusPlus; return static_cast(this)->VisitCastExpr(E); } bool VisitCXXDynamicCastExpr(co

[PATCH] D133518: [HLSL] Call global destructors from entries

aaron.ballman added a comment. This generally LGTM, but to double-check: the behavior if the user does call `atexit()` or or `at_quick_exit()` is that we simply don't call those handlers? As opposed to something worse like we dispatch to the handler and then crash on exit or something like that

[PATCH] D132916: [clang] Explicitly set the EmulatedTLS codegen option

aaron.ballman added a comment. In D132916#3773791 , @mstorsjo wrote: > @efriedma Does this look like what you had in mind; does it seem reasonable > to @aaron.ballman? This seems reasonable to me, but I leave it to @efriedma and @rjmccall for the final

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

aaron.ballman added a comment. Thank you for working on this! This approach can work (though I would recommend using an RAII object akin to `InMessageExpressionRAIIObject`), but have you explored making a new `DeclSpecContext` and modifying `isDefiningTypeSpecifierContext()`? I think that would

[PATCH] D133194: rewording note note_constexpr_invalid_cast

aaron.ballman added inline comments. Comment at: clang/test/Sema/cast.c:1-5 +// RUN: %clang_cc1 -fsyntax-only -triple x86_64-unknown-unknown %s -verify -Wvla + +int array[(long)(char *)0]; // expected-warning {{variable length array used}} \ +// expe

[PATCH] D133194: rewording note note_constexpr_invalid_cast

aaron.ballman added inline comments. Comment at: clang/test/SemaCXX/constant-expression-cxx11.cpp:3 // RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify=expected,cxx11_20,cxx20_2b -triple x86_64-linux -Wno-string-plus-int -Wno-pointer-arith -Wno-zero-length-array -Wno-c99-desig

[PATCH] D133088: [Clang] Fix wrong diagnostic for scope identifier with internal linkage

aaron.ballman added a comment. In D133088#3782126 , @pmor13 wrote: > @aaron.ballman > >> block scope variable to have *internal* linkage instead of *no* linkage > > static int x; > > void f(void) > { > extern int x; // block scope, interna

[PATCH] D133518: [HLSL] Call global destructors from entries

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D133518#3781308 , @beanz wrote: > In D133518#3781101 , @aaron.ballman > wrote: > >> This ge

[PATCH] D133194: rewording note note_constexpr_invalid_cast

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D133194#3781271 , @Codesbyusman wrote: > updated, I am not getting why this is happening but in the .cpp test file > without Wvla al

[PATCH] D133668: [HLSL] Use _BitInt(16) for int16_t to avoid promote to int.

aaron.ballman added a comment. Drive-by comment before I get into the review: does HLSL intend to follow the standard in terms of behavior of intN_t? If yes, then this doesn't follow the behavior allowed by the standard or the direction WG14 chose. We discussed https://www.open-std.org/jtc1/sc2

[PATCH] D133194: rewording note note_constexpr_invalid_cast

This revision was automatically updated to reflect the committed changes. Closed by commit rGea26ed1f9c37: Rewording note note_constexpr_invalid_cast (authored by Codesbyusman, committed by aaron.ballman). Changed prior to commit: https://reviews.llvm.org/D133194?vs=459296&id=459422#toc Reposi

[PATCH] D133668: [HLSL] Use _BitInt(16) for int16_t to avoid promote to int.

aaron.ballman added a comment. In D133668#3783090 , @beanz wrote: > HLSL deviates from C here. HLSL doesn't actually have `short` (although I'm > actually not sure we should disable it in Clang). We do have `int16_t`, but > we don't promote `int16_t` to

[PATCH] D133088: [Clang] Reword diagnostic for scope identifier with linkage

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/D133088/new/ https://reviews.llvm.org/D133088 _

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

aaron.ballman added a comment. In D133574#3781907 , @inclyc wrote: > Use RAII object to maintain the Parser state > >> have you explored making a new DeclSpecContext and modifying >> isDefiningTypeSpecifierContext()? I think that would likely be a cleane

[PATCH] D133634: [clang] Allow vector of BitInt

aaron.ballman added a reviewer: erichkeane. aaron.ballman added a subscriber: erichkeane. aaron.ballman added a comment. In D133634#3783160 , @python3kgae wrote: > In D133634#3782999 , @xbolva00 > wrote: > >> Mi

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

aaron.ballman added a comment. This is awesome, thank you for working on it! @to268 and I had a really good discussion during my office hours and what we decided for next steps were: 0) Next time you upload a patch, please use `-U` to give the patch more context for reviewers to see. 1. Ne

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

aaron.ballman added a comment. FYI: regarding `auto auto`, I've heard back from the author of N3007 and the intent was to disallow this construct ("others" was meant to exclude `auto` and "except typedef" was meant to add onto the list of exclusions). Repository: rG LLVM Github Monorepo CHA

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

aaron.ballman added a comment. One thought that occurred to me is that the way C has this specified is awfully effectively the same way implicit int worked. It may be worth exploring a change to our implicit int functionality to instead generate an implicit `auto` type when in C2x mode. Repos

[PATCH] D133668: [HLSL] Use _BitInt(16) for int16_t to avoid promote to int.

aaron.ballman added a comment. In D133668#3783975 , @beanz wrote: > In D133668#3783489 , @aaron.ballman > wrote: > >> Okay, that's good to know! If you don't intend to *ever* conform to the >> standard in this a

[PATCH] D133771: Add a "Potentially Breaking Changes" section to the Clang release notes

aaron.ballman created this revision. aaron.ballman added reviewers: tstellar, mgorny, jyknight. Herald added a project: All. aaron.ballman requested review of this revision. Herald added a project: clang. Sometimes we make changes to the compiler that we expect may cause disruption for users. For

[PATCH] D133771: Add a "Potentially Breaking Changes" section to the Clang release notes

aaron.ballman added a subscriber: thieta. aaron.ballman added a comment. @tstellar or @thieta -- will this cause any problems for your release scripts? (Note I used `Clang |release|` in the new wording; I can pull the `|release|` bit out if that's a problem.) Also, do you suppose there's a way w

[PATCH] D133800: [Clang 15.0.1] Downgrade implicit int and implicit function declaration to warning only

aaron.ballman created this revision. aaron.ballman added reviewers: mgorny, thieta, tstellar. Herald added a subscriber: jdoerfert. Herald added a project: All. aaron.ballman requested review of this revision. Herald added a project: clang. The changes in Clang 15.0.0 which enabled these diagnosti

[PATCH] D133800: [Clang 15.0.1] Downgrade implicit int and implicit function declaration to warning only

aaron.ballman added a comment. @thieta and @tstellar -- because these changes are expected to go into the 15.0.1 branch, not the current main branch, it wasn't clear to me what the best way to go about the review (whether I should submit a Phab request or make a fork of Clang and submit a GitH

[PATCH] D133800: [Clang 15.0.1] Downgrade implicit int and implicit function declaration to warning only

aaron.ballman added reviewers: rjmccall, jyknight, MaskRay. aaron.ballman added a comment. Herald added a subscriber: StephenFan. In D133800#3787378 , @thieta wrote: > I think the easiest way to handle this is that when you have it approved here > - push

[PATCH] D133800: [Clang 15.0.1] Downgrade implicit int and implicit function declaration to warning only

aaron.ballman updated this revision to Diff 460035. aaron.ballman added a comment. Herald added subscribers: kbarton, nemanjai. Updated the failing tests caught during review (thank you!) and moved the release notes around and reworded them somewhat for clarity. CHANGES SINCE LAST ACTION http

[PATCH] D133800: [Clang 15.0.1] Downgrade implicit int and implicit function declaration to warning only

aaron.ballman added a comment. In D133800#3788950 , @nikic wrote: > I tested this patch as well and saw the same failures as @mgorny. Thank you, I think I've got those patched up, but with one difference: > > diff --git a/clang/test/CodeGen/neon-cr

[PATCH] D129926: [clang-format] Handle constructor invocations after new operator in C# correct

aaron.ballman added a comment. In D129926#3788866 , @MyDeveloperDay wrote: > @eoanermine ping... we need your name/email before we can commit. > > (@curdeius, @owenpan, @HazardyKnusperkeks ) we need to have a policy for > this, that if we don't get the

[PATCH] D133800: [Clang 15.0.1] Downgrade implicit int and implicit function declaration to warning only

aaron.ballman added a comment. In D133800#3789188 , @mgorny wrote: > In D133800#3789100 , @aaron.ballman > wrote: > >> In D133800#3788950 , @nikic wrote: >> >>> I tested

[PATCH] D133800: [Clang 15.0.1] Downgrade implicit int and implicit function declaration to warning only

aaron.ballman updated this revision to Diff 460051. aaron.ballman added a comment. Added the `not` back into the neon-crypto.c test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133800/new/ https://reviews.llvm.org/D133800 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Bas

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

aaron.ballman added a comment. In D133289#3788655 , @to268 wrote: > In D133289#3784042 , @aaron.ballman > wrote: > >> auto auto k = 12; // 99% this is intended to be accepted; first `auto` is >> the storage cl

[PATCH] D133807: Update Unicode to 15.0

aaron.ballman added a comment. The changes LGTM, but I'd like to wait for @tahonermann to weigh in with the final acceptance. Comment at: llvm/lib/Support/UnicodeCaseFold.cpp:713 + // 8 characters + if (C <= 0xa7c2) return C | 1; cor3ntin wrote: > shafi

[PATCH] D133800: [Clang 15.0.1] Downgrade implicit int and implicit function declaration to warning only

aaron.ballman added a comment. In D133800#3787378 , @thieta wrote: > I think the easiest way to handle this is that when you have it approved here > - push it to fork on GitHub based on the release branch and create a GitHub > issue and write /branch ab

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

aaron.ballman added inline comments. Comment at: clang/include/clang/Parse/Parser.h:2311 case DeclSpecContext::DSC_association: +case DeclSpecContext::DSC_offsetof: return true; Is this correct? I don't think we can deduce the type from `offsetof`

[PATCH] D132421: [HLSL] Support PCH for cc1 mode

aaron.ballman added inline comments. Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:50 +if (S.LookupQualifiedName(Result, HLSLNamespace)) { + NamedDecl *Found = Result.getFoundDecl(); + if (auto *TD = dyn_cast(Found)) { 99.9% sure this is saf

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

aaron.ballman added inline comments. Comment at: clang/include/clang/Parse/Parser.h:2311 case DeclSpecContext::DSC_association: +case DeclSpecContext::DSC_offsetof: return true; inclyc wrote: > aaron.ballman wrote: > > Is this correct? I don't thi

[PATCH] D133800: [Clang 15.0.1] Downgrade implicit int and implicit function declaration to warning only

aaron.ballman closed this revision. aaron.ballman added a comment. In D133800#3789377 , @aaron.ballman wrote: > In D133800#3787378 , @thieta wrote: > >> I think the easiest way to handle this is that when you hav

[PATCH] D133771: Add a "Potentially Breaking Changes" section to the Clang release notes

aaron.ballman marked 3 inline comments as done. aaron.ballman added inline comments. Comment at: clang/docs/ReleaseNotes.rst:52 + into an error-only diagnostic in the next Clang release. Fixes + `Issue 50055: `_. +- ``-Wincompa

[PATCH] D133771: Add a "Potentially Breaking Changes" section to the Clang release notes

This revision was automatically updated to reflect the committed changes. aaron.ballman marked an inline comment as done. Closed by commit rGe076680bd59c: Add a "Potentially Breaking Changes" section to the Clang release notes (authored by aaron.ballman). Changed prior to commit: https://review

[PATCH] D133711: [Sema] Reject array element types whose sizes aren't a multiple of their alignments

aaron.ballman added a comment. In D133711#3790933 , @rnk wrote: > This seems like a new error that will be pretty easy to run into, especially > in the funky MSVC virtual base case. @aaron.ballman proposed some kind of new > breaking change announcement

[PATCH] D125142: [clang][auto-init] Remove -enable flag for "zero" mode

aaron.ballman added a comment. In D125142#3782296 , @xbolva00 wrote: > In D125142#3782244 , > @nickdesaulniers wrote: > >> @rsmith can we get some guidance here? Has your opinion changed in the time >> since GC

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from a few nits. Comment at: clang/docs/UsersManual.rst:1411 + Note: ``-ffast-math`` causes ``crtfastmath.o`` to be linked with code. + Comment at: clang/doc

[PATCH] D133853: [AST] Add msvc-specific C++11 attributes

aaron.ballman added a comment. Thank you for the patch! I'm wondering what the goal is for these changes. We don't typically add attributes to Clang that don't have any effect unless there's a very compelling reason to do so. Are you intending to add semantics for these attributes in follow-up

[PATCH] D133249: [libc++] Documents details of the pre-commit CI.

aaron.ballman added inline comments. Comment at: clang/www/hacking.html:279 + + Some changes in Clang affect https://libcxx.llvm.org";>libc++, + for example: ldionne wrote: > This follows my previous comment, but we really don't want to encourage Clang > folks

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

aaron.ballman added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:290 +return this->emitConst( +E, Ctx.getASTContext().getTypeSizeInChars(ArgType).getQuantity()); + } erichkeane wrote: > shafik wrote: > > I notice that `Handle

[PATCH] D133583: [clang][ubsan] Fix __builtin_assume_aligned incorrect type descriptor and C++ object polymorphic address

aaron.ballman added a comment. In D133583#3780460 , @yronglin wrote: > Hi, follow D133202 , should I both fix > alignment in this patch or in another separate patch? (this seems have > different behavior with gcc https

[PATCH] D128440: [WebAssembly] Initial support for reference type funcref in clang

aaron.ballman added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:954 10, // wasm_externref, +20, // wasm_funcref }; pmatos wrote: > aaron.ballman wrote: > > Where did this value come from? > Unsure what you mean here. This is th

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/Type.h:1972-1973 + /// Check if this is a WebAssembly Reference Type. + bool isWebAssemblyReferenceType() const; + bool isWebAssemblyExternrefType() const; /// Determines if this is a sizeless type sup

[PATCH] D133108: [clang] Rework IsTailPaddedMemberArray into isFlexibleArrayMemberExpr

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:15881 +/// Check whether this array fits the idiom of a flexible array member, +/// depending on -fstrict-flex-array value /// depending on the value of -fstrict-flex-array value.

[PATCH] D133725: Searching for tokens including comments

aaron.ballman added a comment. Can you help me understand the expected use for these change? Comment at: clang/lib/Lex/Lexer.cpp:1278 - const char *TokenBegin = File.data() + LocInfo.second; + const char *const TokenBegin = File.data() + LocInfo.second; D

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

aaron.ballman added inline comments. Comment at: clang/test/Sema/offsetof.c:79 +int a; +struct B // no-error, struct B is not defined within __builtin_offsetof directly +{ inclyc wrote: > inclyc wrote: > > aaron.ballman wrote: > > > inclyc wrote: > >

[PATCH] D133583: [clang][ubsan] Fix __builtin_assume_aligned incorrect type descriptor and C++ object polymorphic address

aaron.ballman added inline comments. Comment at: clang/test/Sema/builtin-redecl.cpp:5-6 +#include + // Redeclaring library builtins is OK. yronglin wrote: > aaron.ballman wrote: > > We can be tricky instead of including a header file. > Use `decltype` not wor

[PATCH] D133924: add clang_CXXMethod_isDeleted function

aaron.ballman added a comment. Thanks for this! Can you add a release note to clang/docs/ReleaseNotes.rst and some test coverage for the change to clang/test/Index for the change? Also, it looks like precommit CI found a relevant failure that needs to be fixed: TEST 'Clan

[PATCH] D133887: [Clang] Support label at end of compound statement

aaron.ballman added a comment. In D133887#3790414 , @cor3ntin wrote: > Thanks for doing that work. > > Given that this paper was touted as a C compatibility feature, I think we > should implement the C feature at the same time > https://www.open-std.org/

[PATCH] D133948: [clang][C++20] Fix clang/clangd assert/crash after compilation errors

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Please add a release note for the fix when you land the changes. Comment at: clang/test/SemaCXX/remove-nested-immediate-invocation-crash.cpp:4 + +a, class

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

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] D123235: [OpenMP] atomic compare fail : Parser & AST support

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] D86993: Document Clang's expectations of the C standard library.

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] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

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] D156910: [clang] Add pragma force_vectorize

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] D159309: [ASTMatchers] basic matcher support for ConceptReference

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] D128499: [Clang] Fix: Restore warning inadvertently removed by D126061.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a minor nit in the testing. Comment at: clang/lib/Sema/SemaDecl.cpp:4633-4637 Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS, De

[PATCH] D128319: Survive #pragma once from virtual file.

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. +1, this needs some test coverage for the changes. It should also have a release note to notify users of the fix. Comment at: clang/lib/Lex/Pragma.cpp:415-419 + if (getCurrentFileLexer()->getFileEnt

[PATCH] D128645: Update developer policy.

aaron.ballman added subscribers: cfe-commits, aaron.ballman. aaron.ballman added inline comments. Comment at: llvm/docs/DeveloperPolicy.rst:87-91 -#. Patches should be made with ``git format-patch``, or similar (see special - commands for `Requesting Phabricator review via the

[PATCH] D128182: [NFC] Switch FloatModeKind enum class to use bitmask enums

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128182/new/ https://reviews.llvm.org/D128182 ___ cfe-commits mailing list

<    20   21   22   23   24   25   26   27   28   29   >