[PATCH] D108469: Improve handling of static assert messages.

aaron.ballman added reviewers: tahonermann, clang-language-wg. aaron.ballman added a comment. Thanks for picking this back up! I have mostly nits, a few questions, and am adding some extra reviewers who may have an opinion. Also, I think the summary needs to be updated (this is no longer waiting

[PATCH] D127762: [Clang][AArch64] Add ACLE attributes for SME.

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/Type.h:4064 bool HasTrailingReturn : 1; +unsigned AArch64SMEAttributes : 8; Qualifiers TypeQuals; If we're taking up space in the extra bitfields, why do we need to also take u

[PATCH] D128645: Update developer policy.

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 web interface - `_ ). If you use a - different tool, make

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

aaron.ballman requested changes to this revision. aaron.ballman added a subscriber: shafik. aaron.ballman added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Basic/TargetInfo.h:225-226 unsigned HasAlignMac68kSupport : 1; - uns

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Sarif.h:296-297 +/// The SourceLocation provided must point at the beginning of the token. +typedef std::function +TokenLenMetric; + I worry about the performance aspects of using a ca

[PATCH] D128256: [Clang][AArch64] Limit arm_locally_streaming to function definitions only.

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:9835-9839 + if (D.isFunctionDefinition()) { +NewFD->setWillHaveBody(); +ProcessDeclAttributes(S, NewFD, D); +NewFD->setWillHaveBody(false); + } else sdesmalen wrote: > aaro

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

aaron.ballman added inline comments. Comment at: clang/include/clang/Sema/ScopeInfo.h:77-78 - CompoundScopeInfo(bool IsStmtExpr) : IsStmtExpr(IsStmtExpr) {} + /// FP options at the beginning of the compound statement, prior to + /// any pragma. + FPOptions FPFeatures; -

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

aaron.ballman added inline comments. Comment at: clang/lib/Basic/Sarif.cpp:161 +Region["endColumn"] = adjustColumnPos( +R.getEnd(), Lexer::MeasureTokenLength(R.getEnd().getLocWithOffset(0), + R.getEnd().getManager(), LO));

[PATCH] D128706: [Clang] Disable clang-format entirely for clang/test tree.

aaron.ballman added a comment. The changes here LGTM and I think this is the correct way to go. However, since you discovered that it was perhaps intentional that we did some style checking on tests, do you think we should have a brief community RFC just to make sure everyone's on the same page

[PATCH] D128103: Adds AST Matcher for ObjCStringLiteral

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. We typically don't add AST matchers until we have a need for them to be used in-tree (ASTMatchers.h is already really expensive to parse; adding matchers for everything possible with AST nodes would be prohibitively ex

[PATCH] D128645: Update developer policy.

aaron.ballman added inline comments. Comment at: llvm/docs/DeveloperPolicy.rst:87 -#. Patches should be made with ``git format-patch``, or similar (see special - commands for `Requesting Phabricator review via the web interface - `_ ). If you use a - different tool, make

[PATCH] D108469: Improve handling of static assert messages.

aaron.ballman added a comment. Patch application is still failing the precommit CI; it'd be nice to get that corrected for some early test feedback. Comment at: clang/docs/ReleaseNotes.rst:275 This fixes `Issue 55962 `_. +

[PATCH] D108469: Improve handling of static assert messages.

aaron.ballman added a comment. I've removed the parent revision as this review no longer requires it to proceed. That should get precommit CI working again. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108469/new/ https://reviews.llvm.org/D108469

[PATCH] D128715: [clang-tidy] Fix confusable identifiers interaction with DeclContext

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, but I have some additional testing requests which might spawn follow-up patches. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc/confusable-

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

aaron.ballman added a comment. In D128319#3615779 , @v.g.vassilev wrote: > Historically, the patch was developed before the extended support of virtual > files in Clang. Given that this is very hard to test and it looks like this > is not needed on our

[PATCH] D128645: Update developer policy.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. The changes LGTM, but you should wait a bit before landing in case other reviewers/subscribers want to chime in if they still have opinions. Comment at: llvm/d

[PATCH] D108469: Improve handling of static assert messages.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! The precommit CI failures appear to be unrelated as best I can tell. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108469/new

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM again, this time for real. Comment at: clang/include/clang/Basic/TargetInfo.h:225-226 unsigned HasAlignMac68kSupport : 1; - unsigned RealTypeUsesObjCFP

[PATCH] D128766: Update references to Discourse instead of the mailing lists.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with some minor corrections. Thank you for this! Comment at: clang/docs/ExternalClangExamples.rst:21 -If you know of (or wrote!) a tool or project using

[PATCH] D128706: [Clang] Disable clang-format entirely for clang/test tree.

aaron.ballman added a comment. Should we also do `llvm/test` and `clang-tools-extra/test` at the same time? The LLVM tests will show up in precommit CI, but I'm not certain that precommit CI actually tests clang-tools-extra (it also notably lacks coverage for libc++ and lldb as well). Reposit

[PATCH] D128747: ISSUE - incorrect -Winfinite-recursion warning on potentially-unevaluated operand #21668

aaron.ballman added a comment. Thank you for this fix, it's coming along well! Can you also add a release note to `clang/docs/ReleaseNotes.rst` mentioning the fix? Comment at: clang/lib/Analysis/CFG.cpp:4062 + // operand. [...] + // We add only potentially evaluated statem

[PATCH] D128762: [Clang] Rename StringLiteral::isAscii() => isOrdinary() [NFC]

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/Expr.h:1789-1886 + enum StringKind { Ordinary, Wide, UTF8, UTF16, UTF32 }; private: unsigned numTrailingObjects(OverloadToken) const { return 1; } unsigned numTrailingObjects(OverloadToken) const {

[PATCH] D128762: [Clang] Rename StringLiteral::isAscii() => isOrdinary() [NFC]

aaron.ballman added a comment. Also, precommit CI pointed out that you need to make some additional changes in clang-tools-extra to avoid build errors. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128762/new/ https://reviews.llvm.org/D128762 ___

[PATCH] D128103: Adds AST Matcher for ObjCStringLiteral

aaron.ballman added a comment. The only things left to do are: regenerate the documentation by running clang/docs/tools/dump_ast_matchers.py and add a release note for the new matcher, but otherwise this looks good to me. In D128103#3617156 , @NoQ wrote

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM modulo the rename request. Comment at: clang/lib/AST/Stmt.cpp:370-371 setStmts(Stmts); + if (hasStoredFPFeatures()) +setStoredFPFeatures(FPFeatures

[PATCH] D127762: [Clang][AArch64] Add ACLE attributes for SME.

aaron.ballman added a comment. In D127762#3615702 , @sdesmalen wrote: > Thanks for your patience reviewing this patch @aaron.ballman! Happy to help, thank you for your patience with my constant stream of questions about the design. :-) ==

[PATCH] D128762: [Clang] Rename StringLiteral::isAscii() => isOrdinary() [NFC]

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Assuming precommit CI comes back clean, this LGTM! Comment at: clang/include/clang/AST/Expr.h:1789-1886 + enum StringKind { Ordinary, Wide, UTF8, UTF16, UTF32

[PATCH] D128814: [Clang][Preprocessor] Fix inconsistent `FLT_EVAL_METHOD` when compiling vs preprocessing

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

[PATCH] D128747: ISSUE - incorrect -Winfinite-recursion warning on potentially-unevaluated operand #21668

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Thank you for the fix, the changes LGTM! From our off-review discussion, I understand you need me to commit this on your behalf. What name and email address would you like me to

[PATCH] D128715: [clang-tidy] Fix confusable identifiers interaction with DeclContext

aaron.ballman added a comment. In D128715#3619650 , @serge-sans-paille wrote: > @aaron.ballman : I agree with most of your suggestion except the one below > that I annotated accordingly > > struct Base { > virtual void O0(); > > private: >

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

aaron.ballman added a comment. Thanks, I think this is heading in the right direction! Comment at: clang/include/clang/Basic/Sarif.h:167 + + ThreadFlow setRange(const CharSourceRange &ItemRange) { +Range = ItemRange; Should we assert this source range is n

[PATCH] D128248: [clang] Avoid an assertion in APValue::hasArrayFiller()

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Basically LGTM -- can you also add a release note about the assertion fix? Comment at: clang/test/SemaCXX/constexpr-array-init.cpp:1 + +// RUN: %clang_cc1 -std=

[PATCH] D127807: [clang-tidy] Properly forward clang-tidy output when running tests

aaron.ballman added a comment. FWIW, I'm also a tentative LG on these changes. I'm not certain if there's a reason why we did things this way in the first place, but I think the new output is more readable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D128747: ISSUE - incorrect -Winfinite-recursion warning on potentially-unevaluated operand #21668

This revision was automatically updated to reflect the committed changes. Closed by commit rGcde1df4ca4f2: Correct -Winfinite-recursion warning on potentially-unevaluated operand (authored by appmonster007, committed by aaron.ballman). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D128844: Improve the formatting of static_assert messages

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I agree with the changes in principle, but it looks like the libc++ precommit CI builder is failing with a bunch of failures... but those failures look like the precommit CI isn'

[PATCH] D128103: Adds AST Matcher for ObjCStringLiteral

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Do you need me to commit on your behalf? If so, what name and email address would you like me to use for patch attribution? CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Thank you for the fixes to this! I'll land this again on your behalf with the same information as before. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG329fae7103d3: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface (authored by vaibhav.y, committed by aaron.ballman). Re

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

aaron.ballman added a comment. I had to roll it back because of failures with test bots: https://lab.llvm.org/buildbot/#/builders/91/builds/11328 So this was reverted in b46ad1b5be694feefabd4c6cd112cbbd04a7b3a7 , can you take

[PATCH] D128715: [clang-tidy] Fix confusable identifiers interaction with DeclContext

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from some minor style nits. Comment at: clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp:130-132 +if (ND1->getAccess() != AS_private && isMemberOf(ND1, RD0)) { + return true

[PATCH] D111617: [RISCV] Lazily add RVV C intrinsics.

aaron.ballman added a comment. Some drive-by comments from the peanut gallery. Comment at: clang/lib/Parse/ParsePragma.cpp:3963 +<< PP.getSpelling(Tok) << "riscv" << /*Expected=*/true << "'intrinsic'"; +return; + } It's fine to warn on this, but t

[PATCH] D128119: [clang] enforce instantiation of constexpr template functions

aaron.ballman added reviewers: rsmith, clang-language-wg. aaron.ballman added a comment. It's a bummer that patch application failed on precommit CI for reasons unrelated to your patch (as best I can tell, anyway)... Also, please update the patch summary to have some more details about what your

[PATCH] D128449: [clang] Introduce -Warray-parameter

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:3216 + (Ty->isVariableArrayType() && +cast(Ty)->getSizeModifier() == +ArrayType::ArraySizeModifier::Star); I forgot about this gotcha -- arrays are

[PATCH] D111617: [RISCV] Lazily add RVV C intrinsics.

aaron.ballman added inline comments. Comment at: clang/lib/Parse/ParsePragma.cpp:3963 +<< PP.getSpelling(Tok) << "riscv" << /*Expected=*/true << "'intrinsic'"; +return; + } kito-cheng wrote: > aaron.ballman wrote: > > It's fine to warn on this, but

[PATCH] D128706: [Clang] Disable clang-format entirely for clang/test tree.

aaron.ballman commandeered this revision. aaron.ballman edited reviewers, added: mboehme; removed: aaron.ballman. aaron.ballman added a comment. At the off-list request of @mboehme, I'm commandeering this review to take it over the finish line whenever the RFC finishes. Repository: rG LLVM Gi

[PATCH] D129048: Rewording the "static_assert" to static assertion

aaron.ballman added a comment. Thank you for getting a great start on this diagnostic rewording! I think we should also update DiagnosticParseKinds.td at the same time, but because that's about parsing rather than semantics, not all of the diagnostics should be reworded to use "static assertion

[PATCH] D128449: [clang] Introduce -Warray-parameter

aaron.ballman added inline comments. Comment at: clang/docs/ReleaseNotes.rst:324-326 +- Added the ``-Warray-parameter`` warning. It detects function redefinition, + where different definition involve argument type that decay to the same + pointer type from different array types

[PATCH] D129061: [Lex] Diagnose macro in command lines

aaron.ballman added a reviewer: serge-sans-paille. aaron.ballman added a comment. Thank you for looking into this! I think we may need to figure out something better for handling macros defined by the driver, because otherwise we're going to get a bunch of false positives from this. For example,

[PATCH] D128706: [Clang] Disable clang-format entirely for clang/test tree.

aaron.ballman updated this revision to Diff 442298. aaron.ballman added a comment. Herald added projects: LLVM, clang-tools-extra. Herald added a subscriber: llvm-commits. Rebased and made the same changes for the LLVM and clang-tools-extra tests. CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D128012: [HLSL] Add ExternalSemaSource & vector alias

aaron.ballman added inline comments. Comment at: clang/include/clang/Sema/HLSLExternalSemaSource.h:22 +class HLSLExternalSemaSource : public ExternalSemaSource { + static char ID; + It looks like you're missing the `classof` and `isA` support that actually uses

[PATCH] D112579: Allow non-variadic functions to be attributed with `__attribute__((format))`

aaron.ballman added a comment. In D112579#3625195 , @fcloutier wrote: > Thanks, Aaron. I wasn't sure how to follow up given how long it had been > since the review started. I understand that we're all busy (which explains > the week delay on my part her

[PATCH] D129048: Rewording the "static_assert" to static assertion

aaron.ballman added a comment. Precommit CI failures currently look to be unrelated. I think the only thing remaining is this bit: > I think we should also update DiagnosticParseKinds.td at the same time... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

aaron.ballman added reviewers: tahonermann, clang-language-wg. aaron.ballman added inline comments. Comment at: clang/docs/ReleaseNotes.rst:271 +- Added ``-Winvalid-utf8`` which diagnoses invalid UTF-8 code unit sequences in + comments. Should we mention `P229

[PATCH] D129048: Rewording the "static_assert" to static assertion

aaron.ballman added a comment. In D129048#3630584 , @Codesbyusman wrote: > Yes I am looking into the DiagnosticParseKinds.td Excellent, thank you! In D129048#3630589 , @Codesbyusman wrote: > Yes I am looking

[PATCH] D128012: [HLSL] Add ExternalSemaSource & vector alias

aaron.ballman added a comment. In D128012#3630680 , @beanz wrote: > @aaron.ballman, I just had a total brain-asleep moment, and pushed this > without realizing that I had feedback from you. Are you okay with me > addressing that most-commit or would you

[PATCH] D112579: Allow non-variadic functions to be attributed with `__attribute__((format))`

aaron.ballman added a comment. In D112579#3630647 , @fcloutier wrote: > I'm afraid that's also not possible: `D` is a `Decl`, so it doesn't have > `getType()`. `Decl` is the tightest-fitting superclass of `BlockDecl`, > `FunctionDecl` and `ObjCMethodDec

[PATCH] D128012: [HLSL] Add ExternalSemaSource & vector alias

aaron.ballman added a comment. Giving the posthumous review my LGTM so it's clear. :-) Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:41-44 + auto *UsingDecl = UsingDirectiveDecl::Create( + AST, AST.getTranslationUnitDecl(), SourceLocation(), SourceLocation(), +

[PATCH] D112579: Allow non-variadic functions to be attributed with `__attribute__((format))`

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/test/SemaCXX/attr-format.cpp:76 + format("bare string"); + format("%s", 123); // expected-warning{{format specifies type 'char *' but

[PATCH] D129170: [Sema] Add deprecation warnings for some compiler provided __has_* type traits

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticGroups.td:190 : DiagGroup<"deprecated-dynamic-exception-spec">; +def DeprecatedHasBuiltins : DiagGroup<"deprecated-has-builtins">; def DeprecatedImplementations :DiagGroup<"deprecated-imple

[PATCH] D128745: [Sema] fix trailing parameter pack handling for function template partial ordering

aaron.ballman added reviewers: erichkeane, clang-language-wg. aaron.ballman added a comment. Thank you for working on this! Can you please add more details to the patch summary about the changes? Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5184 - // FIXME: This mim

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

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

[PATCH] D129202: [clang] Add a fixit for warn-self-assign if LHS is a field with the same name as parameter on RHS

aaron.ballman added a comment. Thank you for working on this, I think it's a nice new diagnostic. You should add a release note to describe it. One question I have is about how chatty this is over real world code, especially older code bases. There used to be two prevailing ways to silence the

[PATCH] D129277: [clang] [Serialization] Fix swapped PPOpts/ExistingPPOpts parameters. NFC.

aaron.ballman added a comment. Thanks for catching this! Is it really an NFC change though (it seems like it would change some of the diagnostic behavior and the list of suggested predefines)? Can you add test coverage for the change? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D129277: [clang] [Serialization] Fix swapped PPOpts/ExistingPPOpts parameters. NFC.

aaron.ballman added a comment. In D129277#3636596 , @mstorsjo wrote: > In D129277#3636567 , @aaron.ballman > wrote: > >> Thanks for catching this! Is it really an NFC change though (it seems like >> it would cha

[PATCH] D129170: [Sema] Add deprecation warnings for some compiler provided __has_* type traits

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:5400-5401 +SourceLocation KWLoc) { + if (!S.getLangOpts().CPlusPlus11) +return; + erichkeane wrote: > royjacobson wrote: > > erichkean

[PATCH] D129202: [clang] Add a fixit for warn-self-assign if LHS is a field with the same name as parameter on RHS

aaron.ballman added a comment. In D129202#3634646 , @njames93 wrote: > In D129202#3633618 , @aaron.ballman > wrote: > >> Thank you for working on this, I think it's a nice new diagnostic. You >> should add a rel

[PATCH] D128745: [Sema] fix trailing parameter pack handling for function template partial ordering

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5184 - // FIXME: This mimics what GCC implements, but doesn't match up with the - // proposed resolution for core issue 692. This area needs to be sorted out, ychen wrot

[PATCH] D128119: [clang] enforce instantiation of constexpr template functions

aaron.ballman added a comment. Pinging about updates to this, because if this really does fix https://github.com/llvm/llvm-project/issues/55560, I think we want to get that fix into Clang 15. The release branch point is coming up pretty soon (https://discourse.llvm.org/t/llvm-15-0-0-release-sch

[PATCH] D129362: Undeprecate ATOMIC_FLAG_INIT in C++

aaron.ballman created this revision. aaron.ballman added reviewers: clang-language-wg, erichkeane, cor3ntin, tambre, ldionne. Herald added a project: All. aaron.ballman requested review of this revision. Herald added a project: clang. C++20 deprecated ATOMIC_FLAG_INIT thinking it was deprecated i

[PATCH] D128706: [Clang] Disable clang-format entirely for clang/test tree.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfee77a20732c: Disable clang-format entirely for test directories (authored by aaron.ballman). Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D129202: [clang] Add a fixit for warn-self-assign if LHS is a field with the same name as parameter on RHS

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with some minor stuff you can fix when landing. Thank you for this! Comment at: clang/include/clang/Sema/Sema.h:5170-5171 + FieldDecl * + getSelfAssign

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

aaron.ballman accepted this revision. aaron.ballman added a comment. I only spotted one thing I think is actually an issue, the rest is style related. LGTM with the one issue fixed. Comment at: clang/lib/Lex/Lexer.cpp:2707-2709 +if (!isASCII(C)) { + goto Multi

[PATCH] D124446: [clang-tidy] Add the misc-discarded-return-value check

aaron.ballman added reviewers: alexfh, LegalizeAdulthood. aaron.ballman added a subscriber: alexfh. aaron.ballman added a comment. Precommit CI has found some build errors with the changes that should be addressed. Thank you for all the timing measurement information, that's really helpful! It

[PATCH] D129362: Undeprecate ATOMIC_FLAG_INIT in C++

aaron.ballman added a comment. In D129362#3638485 , @tambre wrote: > Makes sense to me, thanks! Thanks! Would you mind handling the libc++ side like last time, or do you prefer I handle it as part of the changes in this patch? Repository: rG LLVM Gi

[PATCH] D128119: [clang] Enforce instantiation of constexpr template functions during non-constexpr evaluation

aaron.ballman added a comment. Can you also add a release note for the fix? Comment at: clang/test/SemaCXX/constexpr-late-instantiation.cpp:2 +// Make sure foo is instantiated and we don't get a link error +// RUN: not %clang_cc1 -S -emit-llvm %s -fsyntax-only 2>&1 | FileCheck

[PATCH] D129353: [clang-tidy] Improve check cert-dcl58-cpp.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/DontModifyStdNamespaceCheck.cpp:29 + continue; +ArrayRef List1 = Arg.getPackAsArray(); +if (matchesFirstInRange(InnerMatcher, List1.begin(), List1.end(), Finder, Might

[PATCH] D129277: [clang] [Serialization] Fix swapped PPOpts/ExistingPPOpts parameters. NFC.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D129277#3637317 , @mstorsjo wrote: > In D129277#3636795 , @aaron.ballman > wrote: > >> In D

[PATCH] D129170: [Sema] Add deprecation warnings for some compiler provided __has_* type traits

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:5400-5401 +SourceLocation KWLoc) { + if (!S.getLangOpts().CPlusPlus11) +return; + royjacobson wrote: > aaron.ballman wrote: > > erichk

[PATCH] D128449: [clang] Introduce -Warray-parameter

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you for the new diagnostic! Comment at: clang/lib/Sema/SemaDecl.cpp:3213 +static bool EquivalentArrayTypes(QualType Old, QualType New, +

[PATCH] D129362: Undeprecate ATOMIC_FLAG_INIT in C++

aaron.ballman added a comment. In D129362#3639193 , @tambre wrote: > In D129362#3638896 , @aaron.ballman > wrote: > >> In D129362#3638485 , @tambre wrote: >> >>> Makes se

[PATCH] D128119: [clang] Enforce instantiation of constexpr template functions during non-constexpr evaluation

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some test changes, assuming that precommit CI comes back green. Comment at: clang/docs/ReleaseNotes.rst:184 initializer is not allowed this i

[PATCH] D122248: [clang][CodeGen]Fix clang crash and add bitfield support in __builtin_dump_struct

aaron.ballman added a comment. Thanks for the fix! Can you please add a release note as well? Comment at: clang/test/CodeGen/dump-struct-builtin.c:653 + // CHECK: call i32 (i8*, ...) @printf( + __builtin_dump_struct(&a, &printf); +} Can you add some test cov

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

aaron.ballman added a comment. In D109701#3376127 , @vaibhav.y wrote: > Hi, > > Apologies for the long delay! I was on a break to focus to other projects. Not a problem at all! > I have some changes in mind such as: > > - Creating the `SarifLog` object

[PATCH] D121927: [Clang] Work with multiple pragmas weak before definition

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM, thank you for the fix! Comment at: clang/include/clang/Sema/Weak.h:62 +return false; + return LHS.getAlias()->getName() == RHS.getAlias()->getName(); +} hubert.reinterpr

[PATCH] D122029: Change __auto_type behavior with qualifiers to match GCC behavior

aaron.ballman closed this revision. aaron.ballman added a comment. Thanks for the reviews, I've committed in e4a42c5b64d044ae28d9483b0ebd12038d5b5917 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122029/new/ https:

[PATCH] D120185: [ASTMatchers] Output currently processing match and nodes on crash

aaron.ballman added a comment. In D120185#3403230 , @njames93 wrote: > In D120185#3397508 , @thakis wrote: > >> Looks like this breaks tests on Windows: >> http://45.33.8.238/win/54785/step_7.txt >> >> Please tak

[PATCH] D122335: [clang] Emit crash reproduction as a single tar file

aaron.ballman added a comment. In D122335#3403283 , @abrachet wrote: > In D122335#3403281 , > @hubert.reinterpretcast wrote: > >> For users on Windows, would this cause extra trouble if they wanted to see >> wha

[PATCH] D121927: [Clang] Work with multiple pragmas weak before definition

aaron.ballman added inline comments. Comment at: clang/include/clang/Sema/Weak.h:47 +static unsigned getHashValue(const WeakInfo &W) { + return W.getAlias() ? DenseMapInfo::getHashValue(W.getAlias()->getName()) + : DenseMapInfo::getHashValue("");

[PATCH] D122248: [clang][CodeGen]Fix clang crash and add bitfield support in __builtin_dump_struct

aaron.ballman added a comment. In D122248#3403143 , @yihanaa wrote: > 1. Support zero-width bitfield, named bitfield and unnamed bitfield. > 2. Add a release notes. > > The builtin function __builtin_dump_struct behaves for zero-width bitfield > and unna

[PATCH] D122335: [clang] Emit crash reproduction as a single tar file

aaron.ballman added a comment. In D122335#3403357 , @erichkeane wrote: > In D122335#3403305 , @aaron.ballman > wrote: > >> In D122335#3403283 , @abrachet >> wrote: >> >

[PATCH] D122248: [clang][CodeGen]Fix clang crash and add bitfield support in __builtin_dump_struct

aaron.ballman added a comment. In D122248#3403349 , @erichkeane wrote: > In D122248#3403343 , @yihanaa wrote: > >> In D122248#3403315 , >> @aaron.ballman wrote: >> >>> I

[PATCH] D122335: [clang] Emit crash reproduction as a single tar file

aaron.ballman added a comment. In D122335#3403459 , @erichkeane wrote: > In D122335#3403453 , @aaron.ballman > wrote: > >> In D122335#3403357 , @erichkeane >> wrote: >>

[PATCH] D122341: Fix a crash with variably-modified parameter types in a naked function

aaron.ballman created this revision. aaron.ballman added reviewers: erichkeane, efriedma, jyknight, rjmccall. Herald added a project: All. aaron.ballman requested review of this revision. Herald added a project: clang. Naked functions have no prolog, so it's not valid to emit prolog code to evalu

[PATCH] D122248: [clang][CodeGen]Fix clang crash and add bitfield support in __builtin_dump_struct

aaron.ballman added a comment. In D122248#3403478 , @erichkeane wrote: > If it is ok, I think we should probably change the format of the 'dump' for > fields. Using the colon to split up the field from the value is unfortunate, > may I suggest replaci

[PATCH] D122341: Fix a crash with variably-modified parameter types in a naked function

aaron.ballman updated this revision to Diff 417884. aaron.ballman added a comment. Updating based on review feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122341/new/ https://reviews.llvm.org/D122341 Files: clang/docs/ReleaseNotes.rst clang/lib/CodeGen/CodeGenFunction.cpp

[PATCH] D122341: Fix a crash with variably-modified parameter types in a naked function

aaron.ballman added a comment. In D122341#3403587 , @erichkeane wrote: > Other than this 1 thing, LGTM. I DO find myself wondering how much of the > CXXMethodDecl bit in the branch above is valid for 'naked' as well. If it > ISN'T we might consider

[PATCH] D122335: [clang] Emit crash reproduction as a single tar file

aaron.ballman added a comment. In D122335#3404754 , @arichardson wrote: > In D122335#3404358 , @jrtc27 wrote: > >> As a developer who often deals with crashes locally this is more annoying; >> currently I can ju

[PATCH] D122248: [clang][CodeGen]Fix clang crash and add bitfield support in __builtin_dump_struct

aaron.ballman added a comment. In D122248#3403734 , @yihanaa wrote: > What if we don't emit '=' for zero-width bitfield, like this: > > struct Bar { > unsigned c : 1; > unsigned : 3; > unsigned : 0; > unsigned b; > }; > > struct Bar { >

[PATCH] D122155: Add warning when eval-method is set in the presence of value unsafe floating-point calculations.

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticFrontendKinds.td:50-53 +def warn_eval_method_setting_via_option_in_value_unsafe_context : Warning< +"setting the eval method via '-ffp-eval-method' has not effect when numeric " +"result

[PATCH] D120185: [ASTMatchers] Output currently processing match and nodes on crash

aaron.ballman added a comment. In D120185#3403676 , @njames93 wrote: > In D120185#3403270 , @aaron.ballman > wrote: > >> I'd like to understand what's happening better rather than land with a >> disabled test. N

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