[PATCH] D119707: [C2x] Implement WG14 N2764 the [[noreturn]] attribute

aaron.ballman added a comment. I've made the requested changes from @jyknight in a766545402d8569532294d097d026cf2e837b5c2 -- please let me know if there are more issues you'd like me to address. Thank you for the post-commit

[PATCH] D84225: [CFE] Add nomerge function attribute to inline assembly.

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaStmtAttr.cpp:186 void VisitCallExpr(const CallExpr *E) { FoundCallExpr = true; } + void VisitAsmStmt(const AsmStmt *S) { FoundCallExpr = true; } xbolva00 wrote: > pengfei wrote: > > xbolva0

[PATCH] D119525: [clang] Fix crash when array size is missing in initializer

aaron.ballman added a comment. Continues to LGTM, though I had a recommendation for the release note. Feel free to land when you'd like. Comment at: clang/docs/ReleaseNotes.rst:54 +-- +- ``CXXNewExpr::getArraySize()`` previously returned a ``llvm::Optional`` +

[PATCH] D119926: [Clang][AArch64] Enable _Float16 _Complex type

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:1117 CanQualType BFloat16Ty; - CanQualType Float16Ty; // C11 extension ISO/IEC TS 18661-3 + CanQualType Float16Ty, Float16ComplexTy; // C11 extension ISO/IEC TS 18661-3 CanQualType VoidP

[PATCH] D111100: enable plugins for clang-tidy

aaron.ballman added inline comments. Comment at: clang-tools-extra/test/CMakeLists.txt:91 + if(TARGET CTTestTidyModule) + list(APPEND CLANG_TOOLS_TEST_DEPS CTTestTidyModule LLVMHello) + target_include_directories(CTTestTidyModule PUBLIC BEFORE "${CLANG_TOOLS_SOURCE_DI

[PATCH] D119926: [Clang][AArch64] Enable _Float16 _Complex type

aaron.ballman added inline comments. Comment at: clang/test/CodeGen/aarch64-complex-half-math.c:3 +// RUN: %clang_cc1 %s -emit-llvm -triple aarch64-unknown-unknown -o - | FileCheck %s --check-prefix=AARCH64 +// REQUIRES: aarch64-registered-target + There's nothi

[PATCH] D119525: [clang] Fix crash when array size is missing in initializer

aaron.ballman added inline comments. Comment at: clang/docs/ReleaseNotes.rst:54 +-- +- ``CXXNewExpr::getArraySize()`` previously returned a ``llvm::Optional`` + wrapping a ``nullptr`` when the ``CXXNewExpr`` did not have an array tbaeder wrote: >

[PATCH] D116774: AST: Move __va_list tag back to std conditionally on AArch64.

aaron.ballman added a comment. Only a small nit from me. Comment at: clang/lib/AST/ASTContext.cpp:8555-8556 +// namespace std { struct __va_list { +NamespaceDecl *NS; +NS = NamespaceDecl::Create(const_cast(*Context), + Context->getTrans

[PATCH] D116774: AST: Move __va_list tag back to std conditionally on AArch64.

aaron.ballman added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:8555-8556 +// namespace std { struct __va_list { +NamespaceDecl *NS; +NS = NamespaceDecl::Create(const_cast(*Context), + Context->getTranslationUnitDecl(),

[PATCH] D124127: Thread safety analysis: Pack CapabilityExpr using PointerIntPair (NFC)

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

[PATCH] D124128: Thread safety analysis: Store CapabilityExprs in ScopedLockableFactEntry (NFC)

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a question (if we can't use `emplace_back()` the code is still fine). Comment at: clang/lib/Analysis/ThreadSafety.cpp:911-919 +UnderlyingMu

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thanks for the cleanup here! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124033/new/ https://reviews.llvm.org/D124033 __

[PATCH] D124132: Thread safety analysis: Don't pass capability kind where not needed (NFC)

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

[PATCH] D124258: [C89/C2x] Change the behavior of implicit int diagnostics

aaron.ballman marked 2 inline comments as done. aaron.ballman added inline comments. Comment at: clang/docs/ReleaseNotes.rst:169 + As of C2x, support for implicit int has been removed, and the warning options + will have no effect. Specifying ``-Wimplicit-int`` in C89 mode will

[PATCH] D123667: [clang][AST] Fix crash getting name of a template decl

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, with a commenting nit. The Precommit CI test failures look unrelated to me as well. Do you need me to commit on your behalf? If so, what name and email address would you l

[PATCH] D124258: [C89/C2x] Change the behavior of implicit int diagnostics

aaron.ballman marked 7 inline comments as done. aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/LangOptions.h:534 + /// Returns true if implicit int is supported at all. + bool implicitIntEnabled() const { return !CPlusPlus && !C2x; } + --

[PATCH] D123667: [clang][AST] Fix crash getting name of a template decl

aaron.ballman closed this revision. aaron.ballman added a comment. Thank you for the fix! I've commit on your behalf in 225b91e6cbba31ff1ce787a152a67977d08fdcab . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D124066: [clang-tidy] Ignore macros defined within declarations in modernize-macro-to-enum

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D124066#3463109 , @LegalizeAdulthood wrote: > In D124066#3463008 , @aaron.ballman > wrote:

[PATCH] D124258: [C89/C2x] Change the behavior of implicit int diagnostics

aaron.ballman added inline comments. Comment at: clang/docs/ReleaseNotes.rst:168-169 + ``-Wno-error=implicit-int``, or disabled entirely with ``-Wno-implicit-int``. + As of C2x, support for implicit int has been removed, and the warning options + will have no effect. Specifyin

[PATCH] D124131: Thread safety analysis: Store capability kind in CapabilityExpr

aaron.ballman added inline comments. Comment at: clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h:277 + /// The kind of capability as specified by @ref CapabilityAttr::getName. + StringRef CapKind; + Hr, I think this may actually be safe, but it d

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

aaron.ballman added a comment. Thank you for looking into this! I've also thought that the `__builtin_dump_struct` implementation hasn't been quite as robust as it could be. However, the primary use case for that builtin has been for kernel folks to debug when they have no access to an actual d

[PATCH] D124258: [C89/C2x] Change the behavior of implicit int diagnostics

aaron.ballman added a comment. Precommit CI failures are down to just clang-format related ones. However, as with the implicit function declaration diagnostic, I expect there to be a long tail of failing tests which aren't caught by me locally or by the precommit CI bots. My plan is to track th

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

aaron.ballman added a comment. In D124221#3469251 , @rsmith wrote: > In D124221#3468706 , @aaron.ballman > wrote: > >> Thank you for looking into this! I've also thought that the >> `__builtin_dump_struct` imple

[PATCH] D124131: Thread safety analysis: Store capability kind in CapabilityExpr

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with the extra safety measure added. I'm happy to review again if you'd like, but don't require it. Comment at: clang/include/clang/Analysis/Analyses/Thre

[PATCH] D124389: [clang][NFC] Inclusive language: remove use of Whitelist in clang/lib/Analysis/

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

[PATCH] D123783: [clang] Eliminate TypeProcessingState::trivial.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you for the cleanup! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123783/new/ https://reviews.llvm.org/D123783 _

[PATCH] D124128: Thread safety analysis: Store CapabilityExprs in ScopedLockableFactEntry (NFC)

aaron.ballman added a comment. Continues to LGTM Comment at: clang/lib/Analysis/ThreadSafety.cpp:911-919 +UnderlyingMutexes.push_back(UnderlyingCapability{M, UCK_Acquired}); } void addExclusiveUnlock(const CapabilityExpr &M) { -UnderlyingMutexes.emplace_back(M.s

[PATCH] D124316: [clang-tidy] Modernize-macro-to-enum should skip macros used in other macros

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with a few small nits. Comment at: clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp:230 + void rememberExpressionName(const Token &Tok); + voi

[PATCH] D124341: [clang-tidy][NFC] Replace many instances of std::string where a StringRef would suffice.

aaron.ballman added a comment. I went through the changes and they look correct to me... yet I'm still mildly terrified. :-D Have you tried running clang-tidy through its paces with ASan/MSan enabled to see if the change introduces any use-after-free issues in practice? Repository: rG LLVM

[PATCH] D124434: [Clang][Test] Run tests in C++14 mode explicitly.

aaron.ballman added a comment. In D124434#3475006 , @dblaikie wrote: > @rsmith @aaron.ballman - might be especially interesting to know your > thoughts on the C++ chapter-based testing and what the intent there is as > clang changes default versions/new

[PATCH] D123958: [randstruct] Randomize all elements of a record

aaron.ballman added a comment. Can you also add a test specifically for an enumeration declaration, as that was something we found bugs with? Comment at: clang/lib/AST/Randstruct.cpp:201 +dyn_cast(RandomizedFields.back()->getType())) + if (CA->getSize().sle(2)

[PATCH] D124487: [HLSL] Adjust access specifier behavior

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1603 +def ext_hlsl_access_specifiers : ExtWarn< + "access specifiers are a clang HLSL extension">; This also needs to go into a warning group.

[PATCH] D124373: [clang] add parameter pack/pack expansion matchers

aaron.ballman added a comment. Due to how expensive it is to instantiate all the templates from ASTMatchers.h, we typically don't add new matchers unless there's a need for them in-tree or they're expected to be generally useful to a number of out-of-tree matchers. Are you planning to make use

[PATCH] D122920: [Clang][CodeGen]Fix __builtin_dump_struct missing record type field name

aaron.ballman added a comment. In D122920#3476388 , @yihanaa wrote: > In D122920#3472379 , @asoffer wrote: > >> In D122920#3471865 , @yihanaa >> wrote: >> >>> In D122920#

[PATCH] D124534: Add a warning diagnostic for line directive of a gnu extension

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. Thank you for working on this new diagnostic! Comment at: clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp:1 - -// This file intentionally uses a CRLF newlines! - -void foo() { -

[PATCH] D124221: Add new builtin __builtin_reflect_struct.

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:420 +auto *FD = IFD ? IFD->getAnonField() : dyn_cast(D); +if (!FD || (FD->isUnnamedBitfield() || FD->isAnonymousStructOrUnion())) + continue; rsmith wrote: > yihanaa wro

[PATCH] D124373: [clang] add parameter pack/pack expansion matchers

aaron.ballman added a comment. In D124373#3477996 , @upsj wrote: > This is part of my long-term goal to add support for forwarding parameters > and documentation for make_unique-like functions to clangd. To be fair, the > details are not entirely fleshe

[PATCH] D124258: [C89/C2x] Change the behavior of implicit int diagnostics

aaron.ballman added a comment. Ping for the other reviewers in case they have thoughts. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124258/new/ https://reviews.llvm.org/D124258 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:/

[PATCH] D111548: [Clang] Add the `annotate_type` attribute

aaron.ballman added inline comments. Comment at: clang/lib/Parse/ParseDecl.cpp:3184 +continue; + // We reject AT_LifetimeBound and AT_AnyX86NoCfCheck, even though they + // are type attributes, because we historically haven't allowed these -

[PATCH] D124434: [Clang][Test] Run tests in C++14 mode explicitly.

aaron.ballman added a comment. In D124434#3479051 , @junaire wrote: >> In general, my concern with the this patch is that it loses test coverage by >> specifying an explicit language mode. We typically prefer to fix the tests >> so that they can work in

[PATCH] D124487: [HLSL] Adjust access specifier behavior

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from whitespace changes. Comment at: clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp:1 - -// This file intentionally uses a CRL

[PATCH] D123958: [randstruct] Randomize all elements of a record

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/unittests/AST/RandstructTest.cpp:368 +int h; +char name[0]; +} __attribute__((randomize_layout)); v

[PATCH] D122920: [Clang][CodeGen]Fix __builtin_dump_struct missing record type field name

aaron.ballman added a comment. In D122920#3479884 , @erichkeane wrote: > In D122920#3479192 , @yihanaa wrote: > >>> While we'd usually be happy to take the fix-in-hand and apply it, part of >>> the discussion on

[PATCH] D111548: [Clang] Add the `annotate_type` attribute

aaron.ballman added inline comments. Comment at: clang/test/SemaCXX/annotate-type.cpp:2 +// RUN: %clang_cc1 %s -std=c++17 -fsyntax-only -verify + +struct S1 { mboehme wrote: > aaron.ballman wrote: > > mboehme wrote: > > > aaron.ballman wrote: > > > > mboehme wrot

[PATCH] D124462: [Analyzer] Fix clang::ento::taint::dumpTaint definition

aaron.ballman added a comment. In D124462#3480219 , @steakhal wrote: > Thanks for the stats. > @aaron.ballman WDYT, where should we put the `LLVM_DUMP_METHOD` ? The documentation for the attribute says function definitions, but I don't think it matters

[PATCH] D124434: [Clang][Test] Run tests in C++14 mode explicitly.

aaron.ballman added a comment. In D124434#3480481 , @dblaikie wrote: > Yeah, I have mixed feelings - I think at least in theory, C++ tries to be > mostly backwards compatible and so old tests should run successfully in new > language modes - and I guess

[PATCH] D124258: [C89/C2x] Change the behavior of implicit int diagnostics

aaron.ballman added inline comments. Comment at: clang/unittests/AST/SourceLocationTest.cpp:135-145 TEST(ParmVarDecl, KNRLocation) { LocationVerifier Verifier; - Verifier.expectLocation(1, 8); - EXPECT_TRUE(Verifier.match("void f(i) {}", varDecl(), Lang_C99)); + Verifier.e

[PATCH] D124534: Add a diagnostic for line directive of a gnu extension

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:431 +def ext_pp_gnu_line_directive : Extension< + "this style of line directive is a GNU extension">; + Oops, I missed this last time around, but we need this to be

[PATCH] D124258: [C89/C2x] Change the behavior of implicit int diagnostics

aaron.ballman marked 2 inline comments as done. aaron.ballman added a comment. @rsmith -- do you have any lingering concerns about the severity of the diagnostic? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124258/new/ https://reviews.llvm.org/D124258 ___

[PATCH] D123544: [randstruct] Automatically randomize a structure of function pointers

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

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

aaron.ballman added a comment. In D122895#3481564 , @manojgupta wrote: > Some of our users are not very happy with the churn probably caused by this > change where the declaration has the "void" argument but the later definition > does not have explici

[PATCH] D123924: [clang-apply-replacements] Added an option to ignore insert conflict.

aaron.ballman added a comment. I don't know enough about this code base to feel comfortable signing off on it, but the changes look reasonable to me FWIW. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123924/new/ https://reviews.llvm.org/D123924 ___

[PATCH] D124434: [Clang][Test] Run tests in C++14 mode explicitly.

aaron.ballman added a comment. In D124434#3480915 , @dblaikie wrote: > I guess coming back to your other point about restructing the way all this > testing works (be a pity to gate the default change on this work as it sounds > like a big project) - yea

[PATCH] D124534: Add a diagnostic for line directive of a gnu extension

aaron.ballman added a comment. In D124534#3481264 , @ken-matsui wrote: > Hi, @aaron.ballman > > Thanks for your kind reviews! > I’ve updated the code, but is this what you’d expected? It looks like the precommit CI pipeline can't test your patch -- I th

[PATCH] D116280: [clang] adds unary type trait checks as compiler built-ins

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM modulo whatever changes are needed based on feedback from the original review (I noticed Richard has some suggestions there which may impact this review). Repository: rG

[PATCH] D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/IntegralLiteralExpressionMatcher.cpp:25 + + // not a hexadecimal floating-point literal + if (Token.getLength() > 2 && Begin[0] == '0' && std::toupper(Begin[1]) == 'X') (S

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

aaron.ballman added a comment. In D122895#3482555 , @tahonermann wrote: >> I think it's debatable whether this is a bug or not > > For C99 through C17, I kind of agree, but for C2x (where the warning is still > issued with `-Wstrict-prototypes`), my und

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

aaron.ballman added a comment. In D122895#3482707 , @manojgupta wrote: > Is disabling the pedantic warning an option for your users? > > Disabling it wholesale is not an option since they actually want this warning > (the older version). But we agree

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

aaron.ballman added a comment. In D122895#3482575 , @aaron.ballman wrote: > In D122895#3482555 , @tahonermann > wrote: > >>> I think it's debatable whether this is a bug or not >> >> For C99 through C17, I kind

[PATCH] D124534: Add a diagnostic for line directive of a gnu extension

aaron.ballman added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:1356 + +PP.Diag(FlagTok, diag::ext_pp_gnu_line_directive); } else if (FlagVal == 2) { ken-matsui wrote: > aaron.ballman wrote: > > I speculate that this change is wrong. > > >

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

aaron.ballman added a comment. In D122895#3483163 , @manojgupta wrote: > Basically, I'm wondering if you'd be able to enable -fno-knr-function? > > Thanks. this looks promising. Any ideas when -fno-knr-function support was > added? Oops, I had a sli

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

aaron.ballman added a comment. In D122895#3483175 , @manojgupta wrote: > Unless I probably mis-interpreted something, -fno-knr-functions does not > suppress the warning: https://godbolt.org/z/rbEfbbb33 Godbolt hasn't had the chance to catch up to ht

[PATCH] D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/IntegralLiteralExpressionMatcher.cpp:99 + + if (!Current->isLiteral() || isStringLiteral(Current->getKind()) || + !isIntegralConstant(*Current)) { LegalizeAdulthood wro

[PATCH] D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum

aaron.ballman added a comment. To clarify my previous comments, I'm fine punting on the edge cases until user reports come in, so don't let them block this review if you feel strongly about not supporting them. But when user reports start coming in, at some point I might start asking to replace

[PATCH] D111548: [Clang] Add the `annotate_type` attribute

aaron.ballman added a reviewer: erichkeane. aaron.ballman added subscribers: rsmith, erichkeane. aaron.ballman added a comment. Adding in @erichkeane just in case he spots anything I've missed. Comment at: clang/test/SemaCXX/annotate-type.cpp:2 +// RUN: %clang_cc1 %s -std=c++17

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:15319-15321 // OpenCL v2.0 s6.9.u - Implicit function declaration is not supported. else if (getLangOpts().OpenCL) diag_id = diag::err_opencl_im

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

aaron.ballman added a comment. In D122895#3484076 , @manojgupta wrote: > Tried locally but I still see the warning with -fno-knr-functions. It also > says that the argument is unused. > > bin/clang --version > clang version 15.0.0 (https://github.com/ll

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

aaron.ballman added a comment. In D122983#3484064 , @manojgupta wrote: > We are finding a lot of failures in our ToT builds with this change. here is > an example for a configure script: > > $ cat tent.c > int main () > { > tgetent(0,0); > return 0;

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

aaron.ballman added a comment. In D122895#3484097 , @aaron.ballman wrote: > In D122895#3484076 , @manojgupta > wrote: > >> Tried locally but I still see the warning with -fno-knr-functions. It also >> says that

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

aaron.ballman added a comment. In D122983#3484189 , @rjmccall wrote: > IIRC, the reason it works it that way is that "warnings which default to an > error" are really "errors which you can explicitly downgrade to a warning". > Maybe those ought to be d

[PATCH] D124701: [clang] Honor __attribute__((no_builtin("foo"))) on functions

aaron.ballman added a comment. FWIW, it looks like precommit CI found issues in the newly added test which should be addressed. Comment at: clang/test/CodeGen/no-builtin-2.c:3 + +#include + You shouldn't include system headers -- that will pull from whatever

[PATCH] D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum

aaron.ballman added a comment. In D124500#3483328 , @LegalizeAdulthood wrote: > In D124500#3483224 , @aaron.ballman > wrote: > >> To clarify my previous comments, I'm fine punting on the edge cases until >> use

[PATCH] D124719: [docs] PCH usage documentation update

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thanks for improving the docs! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124719/new/ https://reviews.llvm.org/D124719 _

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

aaron.ballman added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:1356 + +PP.Diag(FlagTok, diag::ext_pp_gnu_line_directive); } else if (FlagVal == 2) { ken-matsui wrote: > aaron.ballman wrote: > > ken-matsui wrote: > > > aaron.ballman wrote:

[PATCH] D124694: [randstruct] Move initializer check to be more effective

aaron.ballman added a comment. struct t { int a, b, c, d, e; } x = { .a = 2, 4, 5, 6 }; This situation seems like it should be an error, shouldn't it? The user specified one designated initializer (yay, that one is correct), but the rest are positional initializers and so there's no te

[PATCH] D124320: [clang-tidy] Add createChecks method that also checks for LangaugeOptions

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Just to double-check, this is an NFC change, isn't it (thus there's no tests)? Assuming that's true, this LGTM (though please add NFC to the commit message title). Repository:

[PATCH] D124221: Reimplement `__builtin_dump_struct` in Sema.

aaron.ballman added a comment. Thanks for working on this, I like the direction it's going! Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8470-8473 +def err_expected_struct_pointer_argument : Error< + "expected pointer to struct as %ordinal0 argument to %1, foun

[PATCH] D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum

aaron.ballman added a comment. In D124500#3485924 , @LegalizeAdulthood wrote: > In D124500#3485462 , @aaron.ballman > wrote: > >> I largely agree, but I've found cases where we'll convert correct code to >> inc

[PATCH] D104975: Implement P1949

aaron.ballman added a comment. In D104975#3486313 , @intractabilis wrote: > Can you roll this back and don't support P1949 > ? No; P1949 was adopted for C++23, so this is effectively a n

[PATCH] D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/IntegralLiteralExpressionMatcher.cpp:99 + + if (!Current->isLiteral() || isStringLiteral(Current->getKind()) || + !isIntegralConstant(*Current)) { LegalizeAdulthood wro

[PATCH] D124694: [randstruct] Move initializer check to be more effective

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D124694#3486547 , @void wrote: > In D124694#3485585 , @aaron.ballman > wrote: > >> struct

[PATCH] D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/IntegralLiteralExpressionMatcher.cpp:99 + + if (!Current->isLiteral() || isStringLiteral(Current->getKind()) || + !isIntegralConstant(*Current)) { aaron.ballman wrote:

[PATCH] D124790: [HLSL] Enable half type for hlsl.

aaron.ballman added inline comments. Comment at: clang/include/clang/Driver/Options.td:6766-6767 +def enable_16bit_types : DXCFlag<"enable-16bit-types">, Alias, + HelpText<"Enable 16bit types and disable min precision types." + "Available in HLSL 2018 and shader model

[PATCH] D124258: [C89/C2x] Change the behavior of implicit int diagnostics

aaron.ballman added a comment. Assuming no further comments from reviewers, I plan to land this sometime tomorrow. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124258/new/ https://reviews.llvm.org/D124258 ___ cfe-commits mailing list cfe-co

[PATCH] D124221: Reimplement `__builtin_dump_struct` in Sema.

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8470-8473 +def err_expected_struct_pointer_argument : Error< + "expected pointer to struct as %ordinal0 argument to %1, found %2">; +def err_expected_callable_argument : Error< + "

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

aaron.ballman added a subscriber: rsmith. aaron.ballman added a comment. Thank you for working on this! Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1023-1024 +// Typoed directive warnings +def TypoedDirective : DiagGroup<"typoed-directive">; + W

[PATCH] D123009: [Sema] Enum conversion warning when one signed and other unsigned.

aaron.ballman added a comment. Thanks for the updates! I think this is getting somewhat close, but I'd like to see some additional test cases to ensure we're not regressing behavior we care about (I think we may be losing warnings about sign conversion). // Signed enums enum SE1 { N1 = -1 }

[PATCH] D123952: [FPEnv] Allow CompoundStmt to keep FP options

aaron.ballman added a comment. Thanks for working on this! One thing that's not clear to me is what bugs this is solving -- the test coverage only shows a change to textual AST dumping behavior. Is this otherwise expected to be an NFC change, or should there be some codegen tests that show a di

[PATCH] D123009: [Sema] Enum conversion warning when one signed and other unsigned.

aaron.ballman added a comment. Also, before we land this, you should add a release note to clang/docs/ReleaseNotes.rst about the bugfix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123009/new/ https://reviews.llvm.org/D123009 __

[PATCH] D124790: [HLSL] Enable half type for hlsl.

aaron.ballman added inline comments. Comment at: clang/lib/Basic/LangOptions.cpp:197 - // OpenCL has half keyword - Opts.Half = Opts.OpenCL; + // OpenCL and HLSL have half keyword + Opts.Half = Opts.OpenCL || Opts.HLSL; python3kgae wrote: > aaron.ballman wr

[PATCH] D123009: [Sema] Enum conversion warning when one signed and other unsigned.

aaron.ballman added inline comments. Comment at: clang/docs/ReleaseNotes.rst:113-114 by unary operators. +- ``-Wenum-conversion`` now warns on conversion of signed enum to unsigned enum + and unsigned enum to signed enum rather than ``-Wsign-conversion``. M

[PATCH] D124341: [clang-tidy][NFC] Replace many instances of std::string where a StringRef would suffice.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D124341#3476533 , @njames93 wrote: > In D124341#3475083 , @aaron.ballman > wrote: > >> I we

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

aaron.ballman added a comment. In D124534#3490098 , @ken-matsui wrote: > @aaron.ballman > >> If so, I think putting Diag after the call of this function would be better. > > With the above change, I tried to add comments to failed tests, but there > wer

[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

aaron.ballman created this revision. aaron.ballman added reviewers: erichkeane, rsmith, rjmccall. Herald added a project: All. aaron.ballman requested review of this revision. Herald added a project: clang. With sufficiently tortured code, it's possible to cause a stack overflow when parsing decl

[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

aaron.ballman added a comment. Two things to note: 1. I'm not convinced that adding a test case is the right thing to do here. On the one hand, we want to verify that we now emit the diagnostic before crashing. But on the other hand, this test is extremely expensive to run (because it exhausts

[PATCH] D124790: [HLSL] Enable half type for hlsl.

aaron.ballman added a comment. In D124790#3489690 , @python3kgae wrote: > Add option -fcgl which output clang codeGen result to avoid test dependent on > build DirectX backend. Thanks -- I think this should actually be a separate patch though, because

[PATCH] D124258: [C89/C2x] Change the behavior of implicit int diagnostics

aaron.ballman closed this revision. aaron.ballman added a comment. I've landed in 2cb2cd242ca08d0bbd2a51a41f1317442e5414fc and will keep my eyes on the bots for long tail issues from the diagnostic change. CHANGES SINCE LAST

[PATCH] D111548: [Clang] Add the `annotate_type` attribute

aaron.ballman added a subscriber: rjmccall. aaron.ballman added inline comments. Comment at: clang/test/SemaCXX/annotate-type.cpp:2 +// RUN: %clang_cc1 %s -std=c++17 -fsyntax-only -verify + +struct S1 { mboehme wrote: > aaron.ballman wrote: > > mboehme wrote: > >

[PATCH] D124447: [clang-tidy] Add infrastructure support for running on project-level information

aaron.ballman added a reviewer: alexfh. aaron.ballman added a comment. Thanks, I think this is a neat idea! I haven't had the chance to review thoroughly yet, but wanted to point out that precommit CI is showing a fair amount of failures that I believe are related to your patch. Repository:

[PATCH] D124996: [clang][preprocessor] Fix unsigned-ness of utf8 char literals

aaron.ballman added inline comments. Comment at: clang/test/Lexer/utf8-char-literal.cpp:16 char f = u8'ab'; // expected-error {{Unicode character literals may not contain multiple characters}} #elif __STDC_VERSION__ > 202000L char a = u8'ñ'; // expected-error {{character

<    29   30   31   32   33   34   35   36   37   38   >