[PATCH] D130033: [HLSL] Add resource binding attribute for HLSL.

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:6557-6558 +Here're resource binding examples with and without space: +``RWBuffer Uav : register(u3, space1)`` +``Buffer Buf : register(t1)`` +The full documentation is available here: https://

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

aaron.ballman added a comment. > In D119051#3715939 , @aaron.ballman > wrote: > >> In D119051#3714645 , @dblaikie >> wrote: >> >>> >> >> I would have thought use of `__is_pod` would tell us, but I'm not seeing t

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM, though I did have a nit about diagnostic wording that you can feel free to take or leave at your discretion. Thank you for this work, this was a very large patch with a whole lot of review comments, but I think what we en

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13069 - // therefore don't strictly fit into a signed bitfield of width 1. - if (FieldWidth == 1 && Value == 1) -return false; aaron.ballman wrote: > ShawnZhong wrote: > > Shaw

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

aaron.ballman added a comment. We leave formatting decisions in clang-tidy to clang-format and I don't think we should deviate from that policy here without a very clear understanding of when we should relax that restriction. That said, I'm personally not certain we should have such an option (

[PATCH] D3976: -Wcomma, a new warning for questionable uses of the comma operator

aaron.ballman added a comment. In D3976#3721421 , @gpakosz wrote: > Hello, > > As of today, `-Wcomma` is not really documented: > https://clang.llvm.org/docs/DiagnosticsReference.html#wcomma > >> The current whitelisted expressions are increments, decremen

[PATCH] D130033: [HLSL] Add resource binding attribute for HLSL.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, only nits left that can be fixed when landing (no need for additional review). Comment at: clang/include/clang/Basic/AttrDocs.td:6557-6560 +.. code-block

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13069 - // therefore don't strictly fit into a signed bitfield of width 1. - if (FieldWidth == 1 && Value == 1) -return false; ShawnZhong wrote: > aaron.ballman wrote: > > aaro

[PATCH] D131802: [clang] fix missing initialization of original number of expansions

aaron.ballman added a comment. This looks reasonable to me, but I leave it to others to do the final sign-off. Please be sure to add a release note for the fix though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131802/new/ https://reviews.llvm.

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

aaron.ballman added a comment. In D131386#3722822 , @njames93 wrote: > In D131386#3722749 , @aaron.ballman > wrote: > >> We leave formatting decisions in clang-tidy to clang-format and I don't >> think we should

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

aaron.ballman added a comment. In D131386#3722849 , @aaron.ballman wrote: > In D131386#3722822 , @njames93 > wrote: > >> In D131386#3722749 , >> @aaron.ballman wrote: >

[PATCH] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

aaron.ballman added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:13547-13551 + if (auto ValD = Info.EvaluatingDecl.dyn_cast()) { +const VarDecl *VD = dyn_cast_or_null(ValD); +if (VD && !VD->isConstexpr()) + NotConstexprVar = true; +

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

aaron.ballman added a comment. Do we need to gate this on use of `-fms-compatibility` as well? I'm not certain how cygwin factors in where it's sort of gcc and sort of msvc (perhaps the triple is sufficient for that?). Comment at: clang/lib/AST/DeclCXX.cpp:892 +if ((!

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

aaron.ballman added a comment. In D131386#3723144 , @carlosgalvezp wrote: >> but removing those options can break people's .clang-tidy config files > > Aren't there deprecation mechanisms? I think trying to be backwards > compatible across all possible

[PATCH] D131892: [Sema] fix false -Wcomma being emitted from void returning functions

aaron.ballman added a comment. Thank you for working on this fix! I think it's quite close to finished, but it needs some additional test coverage. Also, please add a release note about the fix so users know what's going on. Comment at: clang/lib/Sema/SemaExpr.cpp:13978-13980

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

aaron.ballman added subscribers: owenpan, MyDeveloperDay. aaron.ballman added a comment. In D131386#3723490 , @Mordante wrote: > In D131386#3722749 , @aaron.ballman > wrote: > >> We leave formatting decisions in

[PATCH] D131892: [Sema] fix false -Wcomma being emitted from void returning functions

aaron.ballman accepted this revision. aaron.ballman added a subscriber: tstellar. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/docs/ReleaseNotes.rst:74 number of arguments cause an assertion fault. +- Fix `#57151

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

aaron.ballman added a comment. This looks correct per my reading of [basic.start.dynamic], but is this an ABI breaking change that we may want to use ABI versioning for in case someone is relying on the old order for some reason? Also, the change should have a release note for the fix. Reposi

[PATCH] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

aaron.ballman added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:13547-13551 + if (auto ValD = Info.EvaluatingDecl.dyn_cast()) { +const VarDecl *VD = dyn_cast_or_null(ValD); +if (VD && !VD->isConstexpr()) + NotConstexprVar = true; +

[PATCH] D130510: Missing tautological compare warnings due to unary operators

aaron.ballman added a comment. This is looking much closer to what I think we had in mind, so I mostly have some cleanup suggestions. Comment at: clang/lib/Analysis/CFG.cpp:979 -const BinaryOperator *BitOp = dyn_cast(BoolExpr); +const BinaryOperator *BitOp = dyn_cas

[PATCH] D130033: [HLSL] Add resource binding attribute for HLSL.

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

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

aaron.ballman added a comment. In D127233#3726613 , @rjmccall wrote: > There's no compiler interoperation problem here; it's just a semantic concern > that someone could've been relying on the old behavior. The new behavior is > (AIUI) clearly required

[PATCH] D130510: Missing tautological compare warnings due to unary operators

aaron.ballman added a comment. Nice! Just a few more small nits to fix that I can see. Comment at: clang/lib/Analysis/CFG.cpp:1016-1021 + Optional getIntegerLiteralSubexpressionValue(const Expr *E) { + +const auto *UnOp = dyn_cast(E->IgnoreParens()); + +// If unary. +

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

aaron.ballman added a comment. In D131307#3726631 , @smeenai wrote: > Was it intended that the warning generated here isn't silenced by `-w`, only > by an explicit `-Wno-enum-constexpr-conversion` (or `-Wno-everythning`), and > that `-Wno-error` doesn't

[PATCH] D131464: [test] Make tests pass regardless of gnu++14/gnu++17 default

aaron.ballman added a comment. In D131464#3716905 , @MaskRay wrote: > Sorry, my previous main comment had been written before I introduced > `LIT_CLANG_STD_GROUP` in `llvm/utils/lit/lit/llvm/config.py`. The multiple > `%clang_cc1` approach actually look

[PATCH] D130510: Missing tautological compare warnings due to unary operators

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. This looks correct to me! Any further concerns @erichkeane or @rtrieu? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130510/new/ h

[PATCH] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

aaron.ballman added a comment. Precommit CI failures are unrelated to this patch. Comment at: clang/lib/AST/ExprConstant.cpp:13547-13550 + if (const auto *VD = dyn_cast_or_null(Info.EvaluatingDecl.dyn_cast())) { +if (VD && !VD->isConstexpr()) + ConstexprV

[PATCH] D128083: [C++20] Correctly handle constexpr/consteval explicitly defaulted special member instantiation behavior

aaron.ballman abandoned this revision. aaron.ballman added a comment. This was partially covered by https://reviews.llvm.org/D131479 and the rest should be handled in a different patch, so abandoning. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D131990: [DRAFT][WebAssembly] Do not support `[[clang::musttail]]` by default

aaron.ballman added a comment. > Marked draft for further discussion because I'm not sure getting this: > > test.cpp:10:7: warning: unknown attribute 'musttail' ignored > [-Wunknown-attributes] > [[clang::musttail]] return bar(x * 10); > > is actually better developer experience than gett

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

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. Adding new types to the type system is quite invasive; was there an RFC for this you can point me to along with a design document? I have no idea how to review this bec

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

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. Same question here about an RFC for adding this new type to the type system and a design document for how the type behaves as in https://reviews.llvm.org/D122215. ==

[PATCH] D130510: Missing tautological compare warnings due to unary operators

aaron.ballman added a comment. In D130510#3727096 , @rtrieu wrote: > This patch has been moving back and forth between > `IsIntegerLiteralConstantExpr` and `getIntegerLiteralSubexpressionValue`. > The first function is preexisting and the second one is

[PATCH] D131926: [clang-tidy] Fix for bugprone-sizeof-expression PR57167

aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-2.c:42 + sum += sizeof(__typeof(&TS)); + sum += sizeof(STRKWD MyStruct*); + sum += sizeof(__typeof(STRKWD MyStruct*)); Based on the document

[PATCH] D131942: [clang][Interp] Implement bool and nullptr literal expressions

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM as well; should we add a release note for this? I suspect not because it's improving an experimental feature that's not really ready for prime time yet, but I don't know how others feel. CHANGES SINCE LAST ACTION https

[PATCH] D131683: Diagnosing the Future Keywords

aaron.ballman added a comment. Precommit CI is failing with a failure that looks like it's relevant: TEST 'Clang :: Parser/static_assert.c' FAILED Script: -- : 'RUN: at line 1'; c:\ws\w9\llvm-project\premerge-checks\build\bin\clang.exe -cc1 -

[PATCH] D131942: [clang][Interp] Implement bool and nullptr literal expressions

aaron.ballman added a comment. In D131942#3728994 , @erichkeane wrote: > In D131942#3728974 , @aaron.ballman > wrote: > >> LGTM as well; should we add a release note for this? I suspect not because >> it's impr

[PATCH] D132031: Do not evaluate dependent immediate invocations

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Thank you, this LGTM, though I did have a testing question. Also, please be sure to add a release note for the fix. Comment at: clang/test/SemaCXX/cxx2a-conste

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

aaron.ballman added a comment. After some more thought and some offline discussions, I think I have a reasonable way forward: let's add `-Wsingle-bit-bitfield-constant-conversion` as a new warning group under `-Wbitfield-constant-conversion` that controls the diagnostic for one-bit bitfields. T

[PATCH] D131926: [clang-tidy] Fix for bugprone-sizeof-expression PR57167

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from removing some comments now that we figured out what's going on. Please hold off on landing for a day or two in case @njames93 has other opinions though. Comment at: clang-tools-extra/test/cl

[PATCH] D131942: [clang][Interp] Implement bool and nullptr literal expressions

aaron.ballman added a comment. In D131942#3729379 , @tbaeder wrote: > In D131942#3729057 , @aaron.ballman > wrote: > >> In D131942#3728994 , @erichkeane >> wrote: >> >>>

[PATCH] D131662: [clang] Try to improve diagnostics about uninitialized constexpr variables

aaron.ballman added a comment. Thanks for working on this, I like the direction it's heading! One question I'm kicking around my head is, should we do something similar for `const` variables? const int i; // error: default initialization of an object of const type 'const int' it sure seems l

[PATCH] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

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

[PATCH] D131662: [clang] Try to improve diagnostics about uninitialized constexpr variables

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaFixItUtils.cpp:208-210 + if (T->isArrayType()) { +return " = {}"; + } tbaeder wrote: > aaron.ballman wrote: > > I don't think this is a good change, consider: > > ``` > > int array[] = {};

[PATCH] D132056: [HLSL] Restrict to supported targets

aaron.ballman added reviewers: ABataev, Anastasia. aaron.ballman added a comment. I don't have particular concerns FWIW, but I'm not certain if others have opinions so I'm holding off on accepting for a bit. Comment at: clang/test/Driver/hlsl-lang-targets.hlsl:14 + +// Maybe s

[PATCH] D131084: Add support for specifying the severity of a SARIF Result.

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

[PATCH] D130510: Missing tautological compare warnings due to unary operators

aaron.ballman added a comment. In D130510#3729864 , @rtrieu wrote: > In D130510#3728719 , @aaron.ballman > wrote: > >> In D130510#3727096 , @rtrieu wrote: >> >>> This pat

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

aaron.ballman added a comment. Thank you for your patience on the review! I've taken a cursory look through and I'm still thinking about the patch. I've not seen anything that's really worrying. But this is pretty dense stuff and @delesley has way more experience with TIL, so I'm hoping he migh

[PATCH] D131662: [clang] Try to improve diagnostics about uninitialized constexpr variables

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, though you should add a release note. Comment at: clang/lib/Sema/SemaInit.cpp:8057 +const Decl *D = Entity.getDecl(); +const VarDecl *VD = dyn_cas

[PATCH] D131203: [HLSL] Initial codegen for SV_GroupIndex

aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:94 llvm::Function *F, const FunctionDecl *FD) { - if (HLSLShaderAttr *ShaderAttr = FD->getAttr()) { -const StringRef ShaderAttrKindStr = "dx.shader"; -F->addFnAttr(ShaderAttrKindStr

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

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. There's a whole ton of test coverage missing from this (no actual sema tests, no AST dumping tests for text or JSON) not to mention a ton of situational tests that are

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

aaron.ballman added a comment. Precommit CI found a relevant failure. Comment at: clang/lib/Frontend/SARIFDiagnosticPrinter.cpp:71 + // other infrastructure necessary when emitting more rich diagnostics. + if (!Info.getLocation().isValid()) { // TODO: What is this case? +

[PATCH] D132098: [clang][Interp] Implement inv and neg unary operations

aaron.ballman added a comment. I'd appreciate if we handled unary `~` and `+` soon after this Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:620 + case UO_Deref: // *x + case UO_Not:// ~x + case UO_Real: // __real x This is reachable and will al

[PATCH] D131084: Add support for specifying the severity of a SARIF Result.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1bd2b2dce2a9: Add support for specifying the severity of a SARIF Result (authored by vaibhav.y, committed by aaron.ballman). Repository: rG LLVM G

[PATCH] D132098: [clang][Interp] Implement inv and neg unary operations

aaron.ballman added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:620 + case UO_Deref: // *x + case UO_Not:// ~x + case UO_Real: // __real x tbaeder wrote: > aaron.ballman wrote: > > This is reachable and will always return true

[PATCH] D132098: [clang][Interp] Implement inv and neg unary operations

aaron.ballman added inline comments. Comment at: clang/lib/AST/Interp/Integral.h:173 + } + template static Integral from(Integral<0, SrcSign> Value) { tbaeder wrote: > aaron.ballman wrote: > > tbaeder wrote: > > > I'm a bit out of my element with the templat

[PATCH] D132098: [clang][Interp] Implement inv and neg unary operations

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132098/new/ https://reviews.llvm.org/D132098 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

[PATCH] D132232: Update coding standards for constexpr if statements; NFC

aaron.ballman created this revision. aaron.ballman added reviewers: dblaikie, rjmccall, echristo, erichkeane. Herald added a project: All. aaron.ballman requested review of this revision. Herald added a project: LLVM. We currently suggest that users not use an `else` clause after a `return` state

[PATCH] D130510: Missing tautological compare warnings due to unary operators

This revision was automatically updated to reflect the committed changes. Closed by commit rGfd874e5fb119: Missing tautological compare warnings due to unary operators (authored by Codesbyusman, committed by aaron.ballman). Changed prior to commit: https://reviews.llvm.org/D130510?vs=453976&id=

[PATCH] D131625: [HLSL] Entry functions require param annotation

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/Attr.h:193 +class HLSLAnnotationAttr : public InheritableAttr { +protected: beanz wrote: > aaron.ballman wrote: > > Is this intended to be used only for parameters (that's how I read the

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

aaron.ballman added inline comments. Comment at: clang/lib/AST/DeclCXX.cpp:1443-1447 +if (D->isTrivial()) { + data().HasTrivialSpecialMembers |= SMKind; +} +else + data().DeclaredNonTrivialSpecialMembers |= SMKind; Commen

[PATCH] D132232: Update coding standards for constexpr if statements; NFC

This revision was automatically updated to reflect the committed changes. Closed by commit rGcda093681bad: Update coding standards for constexpr if statements; NFC (authored by aaron.ballman). Changed prior to commit: https://reviews.llvm.org/D132232?vs=453993&id=454085#toc Repository: rG LL

[PATCH] D132232: Update coding standards for constexpr if statements; NFC

aaron.ballman added a comment. In D132232#3735911 , @dblaikie wrote: > Yeah, I'm OK with this, though yeah, having an example where the `else` is > necessary, but I don't mind too much. I went ahead and switched the example to Erich's because it wasn't

[PATCH] D132324: [RFC] Remove support for building libc++ with `LLVM_ENABLE_PROJECTS`

aaron.ballman added a comment. In D132324#3737976 , @Ericson2314 wrote: > Ah I see email about sphinx jobs defined out of tree :/ I will take a look at > that, see if it is easy to fix. This still hasn't been fixed, so none of the documentation is bein

[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

aaron.ballman added a comment. Thanks for working on this @nickdesaulniers! I think we actually want to go a slightly different direction than this and disable the diagnostics entirely. Basically, we should be make sure the format specifier diagnostics are accounting for the clarifications in

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

aaron.ballman added inline comments. Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L"); -Builder.defineMacro("__cpp_concepts", "201907L"); +Builder.defineMacro("__cpp_concepts", "202002L"); Bui

[PATCH] D132302: [clang] Add support for __attribute__((guard(nocf)))

aaron.ballman added a comment. There should be some explicit test coverage that show the new attribute spelling(s) work when spelled directly rather than when relying on predefined macros. Comment at: clang/include/clang/Basic/Attr.td:3496 // we might also want to support

[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

aaron.ballman added a comment. In D132266#3739542 , @inclyc wrote: > In D132266#3739513 , @aaron.ballman > wrote: > >> Thanks for working on this @nickdesaulniers! I think we actually want to go >> a slightly di

[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

aaron.ballman added a comment. In D132266#3739567 , @inclyc wrote: > N2562.pdf: > >> Modify 7.21.6.2p12: >> ... >> Unless a length modifier is specified, t~~T~~he corresponding argument >> shall be a pointer to int ~~signed integer~~. > > Does this clar

[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

aaron.ballman added a comment. In D132266#3739600 , @inclyc wrote: > What I'm confusing is > Which of the following two explanations is the exact meaning of `hhd`? > > 1. consumes a 32-bit signed integer, then truncates it *inside* printf > 2. consumes an

[PATCH] D132324: [RFC] Remove support for building libc++ with `LLVM_ENABLE_PROJECTS`

aaron.ballman added a comment. In D132324#3739646 , @Ericson2314 wrote: > I did a "soft revert" in rG952f90b72b3546d6b6b038d410f07ce520c59b48 > which > makes it a non-fatal error so

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

aaron.ballman added a comment. In D126907#3739750 , @Mordante wrote: > In D126907#3738383 , @erichkeane > wrote: > >> There was a test I dealt with previously where a ton of the header files >> were run with mod

[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

aaron.ballman added a comment. In D132266#3740013 , @inclyc wrote: > If some one use `%hhd` with an unmatched type argument. Should we emit > diagnose like > > format specifies type 'int' but the argument has type 'WhateverType' > > instead of > > fo

[PATCH] D132286: [clang][Interp] Implement function calls

aaron.ballman added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:603 + if (const auto *FuncDecl = dyn_cast_or_null(Callee)) { +Function *Func = P.getFunction(FuncDecl); + Comment at: clang/lib/AST/Interp/ByteCodeE

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

aaron.ballman added inline comments. Comment at: clang/include/clang/Frontend/SARIFDiagnosticPrinter.h:42 + + unsigned OwnsOutputStream : 1; + There's not a lot of benefit to using bit-fields here yet, so I'd make this field a `bool` instead for the time being.

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

aaron.ballman added inline comments. Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L"); -Builder.defineMacro("__cpp_concepts", "201907L"); +Builder.defineMacro("__cpp_concepts", "202002L"); Bui

[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

aaron.ballman added a comment. In D132266#3740168 , @inclyc wrote: >> format specifies type 'short' but the argument has type 'double' (promoted >> from 'float') > > I'm not sure about this. I'm curious about we just consider any integer with > width le

[PATCH] D132302: [clang] Add support for __attribute__((guard(nocf)))

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Please be sure to add a release note when you land (we have a section about changes to attributes). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

aaron.ballman added inline comments. Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:78 +emitFilename(FE->getName(), Loc.getManager()); +// FIXME: No current way to add file-only location to SARIF object + } cjdb wrote: > I think it wo

[PATCH] D132324: [RFC] Remove support for building libc++ with `LLVM_ENABLE_PROJECTS`

aaron.ballman added a comment. In D132324#3742394 , @ldionne wrote: > Please don't change libc++ things like that without my approval first. This > is a transition I've been working on for 2+ years, I had a local patch for it > waiting to be published,

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

aaron.ballman added inline comments. Comment at: clang/include/clang/Frontend/SARIFDiagnosticPrinter.h:63-65 + void setSarifWriter(SarifDocumentWriter *SarifWriter) { +Writer = std::unique_ptr(SarifWriter); + } abrahamcd wrote: > aaron.ballman wrote: > > Th

[PATCH] D131625: [HLSL] Entry functions require param annotation

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/Attr.h:193 +class HLSLAnnotationAttr : public InheritableAttr { +protected: beanz wrote: > aaron.ballman wrote: > > beanz wrote: > > > aaron.ballman wrote: > > > > Is this intended to be u

[PATCH] D131203: [HLSL] Initial codegen for SV_GroupIndex

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

[PATCH] D131683: Diagnosing the Future Keywords

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:85 + InGroup, DefaultIgnore; +def warn_c23_keyword : Warning<"'%0' is a keyword in C23">, + InGroup, DefaultIgnore; Comment at: clang/include/cl

[PATCH] D132324: [RFC] Remove support for building libc++ with `LLVM_ENABLE_PROJECTS`

aaron.ballman added a comment. In D132324#3742605 , @ldionne wrote: > This sucks, but I'm going to revert this series of patches. This is getting > somewhat out of hands and I feel that we're rushing to fix all kinds of > issues in various directions. I

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

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

[PATCH] D132414: [Clang] follow-up D128745, use ClangABICompat15 instead of ClangABICompat14

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Good catch @royjacobson on the ABI version number needing to be bumped! LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132414/n

[PATCH] D124033: [NFC] Adding a note about the macro __FLT_EVAL_METHOD__

This revision was automatically updated to reflect the committed changes. Closed by commit rGba8b5ff874ad: Adding a note about the macro __FLT_EVAL_METHOD__; NFC (authored by zahiraam, committed by aaron.ballman). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D131625: [HLSL] Entry functions require param annotation

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM though I had a question about the test's RUN line. Comment at: clang/test/SemaHLSL/Semantics/missing_entry_annotation.hlsl:1 +// RUN: %clang_cc1 -triple dx

[PATCH] D132550: Changes to code ownership in clang and clang-tidy

aaron.ballman created this revision. Herald added a subscriber: yaxunl. Herald added a project: All. aaron.ballman requested review of this revision. Herald added projects: clang, clang-tools-extra. This corresponds to the RFC posted to Discourse proposing changes for code ownership in both Clang

[PATCH] D132550: Changes to code ownership in clang and clang-tidy

aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: clang-tools-extra/CODE_OWNERS.TXT:8 +beautification by scripts. The fields are: name (N), email (E), Phabricator +handle (H), and description (D). xazax.hun wrote:

[PATCH] D131683: Diagnosing the Future Keywords

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/TokenKinds.def:384 +C2X_KEYWORD(true, BOOLSUPPORT) +C2X_KEYWORD(remove_quals, KEYC2X) + Codesbyusman wrote: > aaron.ballman wrote: > > This is techn

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

aaron.ballman added a subscriber: nickdesaulniers. aaron.ballman added a comment. Thank you for the patch, but it'd be really helpful to me as a reviewer if you and @nickdesaulniers could coordinate so there's only one patch trying to address #57102 instead of two competing patches (I'm happy to

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

aaron.ballman added inline comments. Comment at: clang/include/clang/Frontend/SARIFDiagnosticPrinter.h:31 +class SARIFDiagnosticPrinter : public DiagnosticConsumer { + raw_ostream &OS; + IntrusiveRefCntPtr DiagOpts; cjdb wrote: > cjdb wrote: > > Please make OS

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

aaron.ballman added inline comments. Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:86-88 +if (Range.isInvalid()) { + continue; +} cjdb wrote: > It seems @aaron.ballman has finally trained me :( {meme, src=ohno} My work here is done. ;-) Repos

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

aaron.ballman added inline comments. Comment at: clang/include/clang/Frontend/SARIFDiagnostic.h:30-33 + SARIFDiagnostic & operator= ( const SARIFDiagnostic && ) = delete; + SARIFDiagnostic ( SARIFDiagnostic && ) = delete; + SARIFDiagnostic & operator= ( const SARIFDiagnostic

[PATCH] D108211: Emit sizeof/alignof values as notes when a static_assert fails

aaron.ballman added a comment. In D108211#3748595 , @tbaeder wrote: > From looking at the output in the test cases, the additional diagnostics > seem unnecessary to me in almost all cases...? +1; my observation is that the extra note repeats informati

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

aaron.ballman added subscribers: dexonsmith, rjmccall. aaron.ballman added a comment. In D132568#3747598 , @nickdesaulniers wrote: > In D132568#3746551 , @aaron.ballman > wrote: > >> Thank you for the patch, but

[PATCH] D132661: [clang] Make guard(nocf) attribute available only for Windows

aaron.ballman added a comment. Thanks for this! Can you also add test coverage for the change? Comment at: clang/include/clang/Basic/Attr.td:402 def TargetWebAssembly : TargetArch<["wasm32", "wasm64"]>; +def TargetHasCFGuard : TargetSpec { + let CustomCode = [{ Target.getTrip

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

aaron.ballman added inline comments. Comment at: clang/test/FixIt/format.m:40 -void test_object_correction (id x) { +void test_object_correction (id x) { NSLog(@"%d", x); // expected-warning{{format specifies type 'int' but the argument has type 'id'}} in

[PATCH] D132503: Add cxx scope if needed for requires clause.

aaron.ballman added reviewers: erichkeane, clang-language-wg. aaron.ballman added a comment. FWIW, I think this looks correct as well. I've added some reviewers just in case there's something I've missed regarding concepts. Comment at: clang/lib/Parse/ParseTemplate.cpp:293-301

<    49   50   51   52   53   54   55   56   57   58   >