[PATCH] D37436: Initial implementation of C attributes (WG14 N2137)

aaron.ballman added a comment. In https://reviews.llvm.org/D37436#869851, @hfinkel wrote: > In https://reviews.llvm.org/D37436#869467, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D37436#869462, @hfinkel wrote: > > > > > I think that I misunderstood your concern. Let me see if I can su

[PATCH] D37436: Initial implementation of C attributes (WG14 N2137)

aaron.ballman added a comment. In https://reviews.llvm.org/D37436#870305, @rsmith wrote: > Also, your wording paper appears to allow things like > > struct [[foo]] S *p; // ok in c n2137, ill-formed in c++ > struct T {}; > int n = sizeof(struct [[foo]] T); // ok in c n2137, ill-formed in c+

[PATCH] D37436: Initial implementation of C attributes (WG14 N2137)

aaron.ballman updated this revision to Diff 115279. aaron.ballman added a comment. Updated based on review feedback. - Rename CAttributes to DoubleSquareBracketAttributes - Added Objective-C test cases to demonstrate no regressed behavior (based on a use-case pointed out during review) - Fixed r

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

aaron.ballman added reviewers: sbenza, JonasToth. aaron.ballman added a comment. Adding a few reviewers to hopefully help Roman get some feedback. Repository: rL LLVM https://reviews.llvm.org/D36836 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:8593 - bool Match = true; + // is this a tautological comparison? if yes, than contains the always-result + llvm::Optional Result; Comments should be complete sentences with capitaliz

[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:8605 - return Match; + const char *Cmp; // Simplified, pretty-printed comparison string + // Similar to E->getOpcodeStr(), but with extra 0 on either LHS or RHS I'd drop this comment

[PATCH] D37629: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rL LLVM https://reviews.llvm.org/D37629 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D33722: [clang-tidy] Add checker for undelegated copy of base classes

aaron.ballman added inline comments. Comment at: clang-tidy/misc/CopyConstructorInitCheck.cpp:24 +withInitializer(cxxConstructExpr(unless(hasDescendant(implicitCastExpr( +.bind("cruct-expr"))); + You pick a more readable name than

[PATCH] D38171: Implement clang-tidy check aliases.

aaron.ballman added a comment. Thank you for working on this -- it's very nice functionality! Comment at: clang-tidy/cert/CERTTidyModule.cpp:79 + void addWarningCheckAliases( + llvm::DenseMap &WarningCheckAliases) { +WarningCheckAliases.insert( You sh

[PATCH] D38159: [clang] Fix printf fixit for objc specific types

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from a few minor nits, LGTM! Comment at: test/FixIt/fixit-format-ios.m:2 +// RUN: cp %s %t +// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-onl

[PATCH] D38179: [clang-tidy] Handle unions in modernize-use-equals-default check

aaron.ballman added inline comments. Comment at: clang-tidy/modernize/UseEqualsDefaultCheck.cpp:117 + const CXXMethodDecl *Operator) { + const auto *Record = Operator->getParent(); + Please don't use `auto` here or els

[PATCH] D38203: [Sema] Corrected the warn-on-throw-from-noexcept behavior to include nothrow

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from a minor testing nit, LGTM! Thanks! Comment at: test/SemaCXX/warn-throw-out-noexcept-func.cpp:20 + } + void __attribute__((nothrow)) SomeThrow() {//

[PATCH] D38202: Add Documentation to attribute-nothrow. Additionally, limit to functions.

aaron.ballman added inline comments. Comment at: include/clang/Basic/AttrDocs.td:2692 + let Content = [{ +Clang supports the GNU style ``__attribute__((nothrow))`` attribute as an +equivilent of `noexcept` on function declarations. This attribute informs the Sho

[PATCH] D38205: [Sema] Warn on attribute nothrow conflicting with language specifiers

aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1411 +def warn_nothrow_attr_disagrees_with_exception_specification +: ExtWarn<"Attribute nothrow ignored, it disagrees with language specified " + "exception specificatio

[PATCH] D62435: Add Attribute NoThrow as an Exception Specifier Type

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from some minor nits. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2789 InGroup; +def warn_nothrow_attribute_ignored : Warning<"nothrow attribute conflicts with" + " exception sp

[PATCH] D54258: [Clang] Fix pretty printing of CUDA address spaces

aaron.ballman added a comment. In D54258#1524643 , @richardmembarth wrote: > Do you know when this will be merged? I apologize, I wasn't aware you needed this merged on your behalf. I normally would be happy to do so, but I'm currently traveling. Maybe

[PATCH] D59402: Suggestions to fix -Wmissing-{prototypes,variable-declarations}

aaron.ballman added inline comments. Comment at: lib/Sema/SemaDecl.cpp:13340-13342 } + else +Diag(FD->getTypeSpecStartLoc(), diag::note_static_for_internal_linkage) Formatting here is a bit off -- you should run through clang-format. Also, I'd

[PATCH] D61552: [clang] Adapt ASTMatcher to explicit(bool) specifier

aaron.ballman added a comment. In D61552#1530730 , @Tyker wrote: > i didn't manage to get "clang/docs/tools/dump_ast_matchers.py" to work. it > keep failing to parse and generating empty files as a result. Are you getting errors from running it, or just

[PATCH] D62571: Implement codegen for MSVC unions with reference members

aaron.ballman added reviewers: rnk, majnemer. aaron.ballman added subscribers: majnemer, rnk. aaron.ballman added a comment. This looks reasonable to me, but @rnk and @majnemer may have opinions as well. Comment at: clang/test/CodeGenCXX/ms-union-member-ref.cpp:3-6 +union A { +

[PATCH] D63039: Various improvements to Clang MSVC Visualizers

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, Mike! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63039/new/ https://reviews.llvm.org/D63039

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D57086#1535873 , @domdom wrote: > Sorry I it's taken me a while to get back to this work. I've rebased the > changes and taken advanta

[PATCH] D62750: Show note for -Wmissing-prototypes for functions with parameters

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: lib/Sema/SemaDecl.cpp:13335 +? FixItHint::CreateInsertion(FTL.getRParenLoc(), "void") +: FixI

[PATCH] D63139: [Diagnostics] Implement -Wswitch-unreachable

aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticGroups.td:541 def SwitchEnum : DiagGroup<"switch-enum">; +def SwitchUnreachable : DiagGroup<"switch-unreachable">; def Switch : DiagGroup<"switch">; I don't think you

[PATCH] D62750: Show note for -Wmissing-prototypes for functions with parameters

aaron.ballman added inline comments. Comment at: lib/Sema/SemaDecl.cpp:13335 +? FixItHint::CreateInsertion(FTL.getRParenLoc(), "void") +: FixItHint{}); } aaronpuchert wrote: > aaron.ballman wrote: > > In t

[PATCH] D63260: [Attr] Support _attribute__ ((fallthrough))

aaron.ballman added inline comments. Comment at: lib/Sema/AnalysisBasedWarnings.cpp:1279 continue; - if (S.getLangOpts().CPlusPlus11) { + if (S.getLangOpts().CPlusPlus11 || S.getLangOpts().C99) { const Stmt *Term = B->getTerminatorStmt(); -

[PATCH] D59402: Suggestions to fix -Wmissing-{prototypes,variable-declarations}

aaron.ballman added inline comments. Comment at: lib/Sema/SemaDecl.cpp:13345-13346 +<< (FD->getStorageClass() == SC_None +? FixItHint::CreateInsertion(FD->getTypeSpecStartLoc(), + "static ") +

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5609 +def warn_mul_in_bool_context : Warning< + "'*' in bool context, maybe you mean '&&'?">, + InGroup; I would appreciate seeing some motivating examples for this case

[PATCH] D63299: [Clang] Parse GNU fallthrough attributes

aaron.ballman added inline comments. Comment at: lib/Parse/ParseStmt.cpp:102 ParsedAttributesWithRange Attrs(AttrFactory); + MaybeParseGNUAttributes(Attrs); MaybeParseCXX11Attributes(Attrs, nullptr, /*MightBeObjCMessageSend*/ true); xbolva00 wrote: > xbolv

[PATCH] D63490: Script for generating AST JSON dump test cases

aaron.ballman created this revision. aaron.ballman added a reviewer: rsmith. Trying to write or maintain test code for AST dumping of JSON information can be onerous due to small changes in the test requiring a large amount of line numbers or other information to be updated in the expected test

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

aaron.ballman added a comment. Most of the comments are about minor nits like grammar and coding conventions, but I did have some questions regarding what kinds of functions the sycl_kernel attribute gets applied to. Also, I'd like to see some additional tests that demonstrate the sycl device a

[PATCH] D62952: [analyzer][tests] Use normalize_sarif in place of diff_sarif

aaron.ballman added a comment. In D62952#1535088 , @hubert.reinterpretcast wrote: > @aaron.ballman, for similar cases in the plist output, it has been proposed > > - that the reference expected file be committed into the tree pre-normalized, > and > - th

[PATCH] D59402: Suggestions to fix -Wmissing-{prototypes,variable-declarations}

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Comment at: lib/Sema/SemaDecl.cpp:13345-13346 +<< (FD->getStorageClass() == SC_None +? FixItHint::CreateInsertion(FD->getTypeSpecStartLoc(), +

[PATCH] D63299: [Clang] Parse GNU fallthrough attributes

aaron.ballman added a comment. In D63299#1548306 , @xbolva00 wrote: > Ok, If we peek a few tokens ahead and check attribute if it is stmt attribute > and then we call MaybeParseGNUAttr - this probably would work but are you > fine with it as a workaround

[PATCH] D63088: [clang-tidy] misc-unused-parameters: don't comment out parameter name for C code

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Funny enough, I've got a paper out for WG14 to try to relax this for C2x: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2381.pdf Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D63451: P0840R2: support for [[no_unique_address]] attribute

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. Can you also add some SemaCXX tests that ensure the attribute is properly diagnosed when written on a bit-field, a static data member, a function, is given an argument, etc? Comment at: include/clan

[PATCH] D62952: [analyzer][tests] Use normalize_sarif in place of diff_sarif

aaron.ballman added a comment. In D62952#1548580 , @hubert.reinterpretcast wrote: > In D62952#1548377 , @aaron.ballman > wrote: > > > In general, that seems reasonable, but I would prefer to take care of more > >

[PATCH] D62952: [analyzer][tests] Use normalize_sarif in place of diff_sarif

aaron.ballman added a comment. In D62952#1548620 , @hubert.reinterpretcast wrote: > In D62952#1548593 , @aaron.ballman > wrote: > > > But is there a reason to not keep `%diff_sarif` and define it in terms of > >

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

aaron.ballman added inline comments. Comment at: clang/test/SemaSYCL/device-attributes.cpp:3 + +[[clang::sycl_kernel]] int gv2 = 0; // expected-warning {{'sycl_kernel' attribute only applies to functions}} +__attribute((sycl_kernel)) int gv3 = 0; // expected-warning {{'sycl_kern

[PATCH] D61552: [clang] Adapt ASTMatcher to explicit(bool) specifier

aaron.ballman added a comment. In D61552#1550080 , @Tyker wrote: > fixed requested changes. > > > Are you getting errors from running it, or just incorrect output? > > the issue happens to me even on master so i suppose the input is correct. > here is the

[PATCH] D62952: [analyzer] SARIF: Add EOF newline; replace diff_sarif

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62952/new/ https://reviews.llvm.org/D62952 ___

[PATCH] D63490: Script for generating AST JSON dump test cases

aaron.ballman closed this revision. aaron.ballman added a comment. Committed in r363820; we'll look into integrating the script into lit as you suggested for a possible follow-up. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63490/new/ https://reviews.llvm.org/D63490 __

[PATCH] D63451: P0840R2: support for [[no_unique_address]] attribute

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. The attribute parts LGTM! You can change the `TargetItaniumCXXABI` part in a follow-up commit if you'd prefer. Comment at: lib/AST/Decl.cpp:3945 +retur

[PATCH] D61552: [clang] Adapt ASTMatcher to explicit(bool) specifier

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a documentation nit. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6190 +/// cxxDeductionGuideDecl(isExplicit()) will match #6, but

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

aaron.ballman added a comment. In D57086#1549632 , @domdom wrote: > clang-format the patch Thanks! Do you need someone to commit on your behalf? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57086/new/ https://reviews.llvm.org/D57086 __

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3306 +def warn_xor_used_as_pow_base_two : Warning< + "result of '%0' is %1, maybe you mean '%2' (%3)?">, + InGroup; We usually use "did you mean" in these kinds of diagn

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

aaron.ballman added a comment. In D63423#1547244 , @regehr wrote: > In D63423#1547216 , @xbolva00 wrote: > > > The only remaining question is, as your said, whether or not to diagnose > > xors in macros. @regerh @j

[PATCH] D61552: [clang] Adapt ASTMatcher to explicit(bool) specifier

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6190 +/// cxxDeductionGuideDecl(isExplicit()) will match #6, but not #5. +/// cxxConstructorDecl(isExplicit()) will match #8, but not #7 or #9. +AST_POLYMORPHIC_MATCHER(isExplicit, AST_

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

aaron.ballman added a comment. In D63423#1550563 , @xbolva00 wrote: > We will be noisy in this case > > #define IRQ_CHINT4_LEVEL(11 ^ 7) > #define IRQ_CHINT3_LEVEL(10 ^ 7) > #define IRQ_CHINT2_LEVEL(9 ^ 7) > #define IRQ_CHIN

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

aaron.ballman added a comment. In D63423#1550586 , @xbolva00 wrote: > ... But I go thru codesearch and this is just one case I think. > > I think we should warn in macros... In codesearch case ,they are many more > true positives, then false negatives. >

[PATCH] D63535: [clang][AST] ASTNameGenerator: A refactoring of CodegenNameGeneratorImpl (NFC).

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/Mangle.h:249-250 +struct ASTNameGenerator { + std::unique_ptr MC; + llvm::DataLayout DL; + Do these still need to be public members? Back when this class was an implementation detail, tha

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

aaron.ballman added a comment. In D63423#1550697 , @jfb wrote: > What I meant with macros was that I don't think we should warn on: > > #define LEGIT(a, b) ({ work work work; a ^ b; work work work; }) > > LEGIT(10, 5); > > > If the constants are inli

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

aaron.ballman added a comment. In D63423#1550713 , @jfb wrote: > In D63423#1550705 , @aaron.ballman > wrote: > > > In D63423#1550697 , @jfb wrote: > > > > > What I meant wit

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

aaron.ballman added a comment. In D63423#1550768 , @xbolva00 wrote: > >> Perhaps the author can run the check over a large corpus of code to see > >> whether fps come up in practice? (The presence of fps will suggest to not > >> warn in macros, but the a

[PATCH] D63535: [clang][AST] ASTNameGenerator: A refactoring of CodegenNameGeneratorImpl (NFC).

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM with a few nits. Comment at: clang/include/clang/AST/Mangle.h:253 +public: + ASTNameGenerator(ASTContext &Ctx); + bool writeName(const Decl *D, raw_ostream &OS); Slight preference to ma

[PATCH] D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks

aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/gen-static-analyzer-docs.py:67 + with open(os.path.join(__location__, checker["FullPackageName"]+".rst"),"w") as f: +f.write(".. title:: clang-tidy - %s\n" % checker["FullPackageName"

[PATCH] D63954: Add lifetime categories attributes

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2770 +def Owner : InheritableAttr { + let Spellings = [CXX11<"gsl", "Owner">]; + let Subjects = SubjectList<[CXXRecord]>; xazax.hun wrote: > aaron.ballman wrote: > > xazax.hun wr

[PATCH] D64380: Add 'require_designated_init' and 'required' attribute to clang

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1948 +def RequiresDesignator : InheritableAttr { + let Spellings = [Clang<"requires_designator">]; + let Subjects = SubjectList<[Record]>; emmettneyman wrote: > compnerd wrote: >

[PATCH] D64632: [clang-format] Don't detect call to ObjC class method as C++11 attribute specifier

aaron.ballman added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:389 bool isCpp11AttributeSpecifier(const FormatToken &Tok) { if (!Style.isCpp() || !Tok.startsSequence(tok::l_square, tok::l_square)) return false; Clang has a fea

[PATCH] D64600: [ObjC] Add an attribute that "clamps" signed char BOOLs to {0,1}

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3279 +def ObjCClampingBool : TypeAttr { + let Spellings = [Clang<"objc_clamping_bool">]; + let Subjects = SubjectList<[TypedefName]>; Is there a desire for similar functionality

[PATCH] D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks

aaron.ballman added a comment. I think this looks reasonable to me, though I am still not certain if the relative path in the python script will work with both the svn in-tree directory layout as well as the git monorepo layout (which I'm far less familiar with). Repository: rG LLVM Github

[PATCH] D61749: [clang-tidy] initial version of readability-convert-member-functions-to-static

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a tiny nit. Comment at: clang-tools-extra/clang-tidy/readability/ConvertMemberFunctionsToStatic.cpp:110 + assert(TSI); + auto FTL = TSI->getT

[PATCH] D64780: Disallow most calling convention attributes on PS4.

aaron.ballman added inline comments. Comment at: test/Sema/no_callconv.cpp:1 +// RUN: %clang_cc1 %s -triple x86_64-scei-ps4 -DPS4 -fsyntax-only -verify +// RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -fsyntax-only -verify Does this really need an svn:exec

[PATCH] D64098: [NFC][clang] Refactor getCompilationPhases step 1: Move list of phases into Types.def table.

aaron.ballman added inline comments. Comment at: clang/include/clang/Driver/Types.h:100 /// done for type 'Id'. - void getCompilationPhases( -ID Id, -llvm::SmallVectorImpl &Phases); + const std::vector getCompilationPhases(ID Id); Please drop the t

[PATCH] D64098: [NFC][clang] Refactor getCompilationPhases step 1: Move list of phases into Types.def table.

aaron.ballman added inline comments. Comment at: clang/lib/Driver/Types.cpp:305 + P.clear(); + for (auto Phase : getInfo(Id).Phases) +P.push_back(Phase); Can't you use the local `Phases` object instead of calling `getInfo()` again? This seems like it wants

[PATCH] D61989: [clang-tidy] enable modernize-concat-nested-namespaces on header files

aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/modernize-concat-nested-namespaces.h:1 +#pragma once + shixiao wrote: > alexfh wrote: > > alexfh wrote: > > > Please put the header under Inputs// directory to keep the > > > tes

[PATCH] D64914: Implement P1771

aaron.ballman added a comment. There seems to be some separable concerns in this patch. I'd rather real with `warn_unused_result`, `const`, and `pure` attributes separately only because those are under the `gnu` attribute namespace and I'd prefer to match GNU's semantics for those attributes (o

[PATCH] D64838: [Attr] Support _attribute__ ((fallthrough))

aaron.ballman added a comment. In D64838#1589770 , @Nathan-Huckleberry wrote: > The main problem that we have is that the `__attribute__` token always causes > the parser to read the line as a declaration. Then the declaration parser > handles reading t

[PATCH] D64098: [NFC][clang] Refactor getCompilationPhases step 1: Move list of phases into Types.def table.

aaron.ballman added inline comments. Comment at: clang/lib/Driver/Types.cpp:305 + P.clear(); + for (auto Phase : getInfo(Id).Phases) +P.push_back(Phase); plotfi wrote: > aaron.ballman wrote: > > Can't you use the local `Phases` object instead of calling `ge

[PATCH] D64600: [ObjC] Add an attribute that "clamps" signed char BOOLs to {0,1}

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3279 +def ObjCClampingBool : TypeAttr { + let Spellings = [Clang<"objc_clamping_bool">]; + let Subjects = SubjectList<[TypedefName]>; dexonsmith wrote: > erik.pilkington wrote: >

[PATCH] D64838: [Attr] Support _attribute__ ((fallthrough))

aaron.ballman added a comment. In D64838#1592118 , @Nathan-Huckleberry wrote: > In D64838#1592111 , @aaron.ballman > wrote: > > > In D64838#1589770 , > > @Nathan-Huckleber

[PATCH] D64098: [NFC][clang] Refactor getCompilationPhases step 1: Move list of phases into Types.def table.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/lib/Driver/Types.cpp:303 + // Types.def is correct. Everything above this comment will be removed + // in a subsequent NFC

[PATCH] D64914: Implement P1771

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2346 +// spellings. +bool IsCXX11NoDiscard() const { + return this->getSemanticSpelling() == CXX11_nodiscard; I don't think this is strictly required, but perhaps it's

[PATCH] D64914: Implement P1771

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:2833-2835 if (D->getFunctionType() && - D->getFunctionType()->getReturnType()->isVoidType()) { + D->getFunctionType()->getReturnType()->isVoidType() && + !isa(D)) {

[PATCH] D64838: [Attr] Support _attribute__ ((fallthrough))

aaron.ballman added a comment. In D64838#1592520 , @Nathan-Huckleberry wrote: > void foo() { > __attribute__((address_space(0))) *x; > *y; > } > > > If the attributes are parsed then function body looks like this to the parser: > > { > *

[PATCH] D64780: Disallow most calling convention attributes on PS4.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a nit. You should give other reviewers a chance to sign off in case they have additional comments. Comment at: lib/Basic/Targets/OSTargets.h:56

[PATCH] D64883: Add new warning -Walloca for use of builtin alloca function

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2776 +def warn_alloca : Warning< + "use of builtin function %0">, + InGroup>, DefaultIgnore; george.burgess.iv wrote: > nit: I'd just say "use of function '%0'" he

[PATCH] D64744: #pragma clang loop vectorize_predicate(enable|disable)

aaron.ballman added a comment. Is it intentional that this review has no reviewers listed (like, is this a work in progress you don't expect review on yet)? Comment at: clang/include/clang/Basic/Attr.td:2985 + "pipeline", "pipeline_initiation_interval

[PATCH] D65104: [clang-tidy] Add FixItHint for performance-noexcept-move-constructor

aaron.ballman added inline comments. Comment at: clang-tidy/performance/NoexceptMoveConstructorCheck.cpp:57 + assert(Decl->getNumParams() > 0); + SourceLocation NoexceptLoc = Decl->getParamDecl(Decl->getNumParams() - 1) + ->getSourc

[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

aaron.ballman accepted this revision. aaron.ballman added a comment. In D64666#1583629 , @jfb wrote: > I think you want to default-ignore the "may lose precision" warnings, but not > the ones that you know always lose precision. We don't often add off-b

[PATCH] D64883: Add new warning -Walloca for use of builtin alloca function

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2776 +def warn_alloca : Warning< + "use of builtin function %0">, + InGroup>, DefaultIgnore; george.burgess.iv wrote: > aaron.ballman wrote: > > george.burgess.iv

[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

aaron.ballman added a comment. In D64666#1596619 , @xbolva00 wrote: > I think @jfb wanted to say that if we know we always lose the precision, we > should warn even without -Wxyz, no? Oh, good catch! I hadn't noticed that *both* diagnostics were `Defaul

[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. In D64666#1596660 , @xbolva00 wrote: > I think we should warn in that case even if GCC does not warn. Strong +1. In D64666#15

[PATCH] D64883: Add new warning -Walloca for use of builtin alloca function

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2776 +def warn_alloca : Warning< + "use of builtin function %0">, + InGroup>, DefaultIgnore; george.burgess.iv wrote: > aaron.ballman wrote: > > george.burgess.iv

[PATCH] D63954: Add lifetime categories attributes

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4167 + +The attribute ``[[gsl::Owner(T)]]`` applies to structs and classes that own an +object of type ``T``: Do either of these attributes make sense on a union type? If so, m

[PATCH] D62829: [clang-tidy] Check for dynamically initialized statics in headers.

aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-dynamic-static-initializers.rst:9 + +This can pose problems in certain multithreaded contexts. Eugene.Zelenko wrote: > Will be good idea to provide example. Agreed

[PATCH] D64914: Implement P1771

aaron.ballman added a comment. In D64914#1595660 , @erichkeane wrote: > Rebased and did all the comments (including the www_status). @aaron.ballman > : Wasn't positive what you meant about the conversion functions, but I think > I got one? I was talki

[PATCH] D64914: Implement P1771

aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:2835 + D->getFunctionType()->getReturnType()->isVoidType() && + !isa(D)) { S.Diag(AL.getLoc(), diag::warn_attribute_void_function_method

[PATCH] D64744: #pragma clang loop vectorize_predicate(enable|disable)

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

[PATCH] D65104: [clang-tidy] Add FixItHint for performance-noexcept-move-constructor

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

[PATCH] D65176: [NFC][clang] Refactor getCompilationPhases()+Types.def step 2.

aaron.ballman added inline comments. Comment at: clang/include/clang/Driver/Types.def:39-45 +// Some of the options in Flags have been removed, so far those are: +// a - The type should only be assembled: Now, check that Phases contains +// phases::Assemble but not phases::

[PATCH] D63954: Add lifetime categories attributes

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2771 + let Spellings = [CXX11<"gsl", "Owner">]; + let Subjects = SubjectList<[CXXRecord]>; + let Args = [TypeArgument<"DerefType", /*opt=*/1>]; This subject should be `Struct` a

[PATCH] D64678: [Sema] Fix -Wuninitialized for struct assignment from GNU C statement expression

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from a comment request. Comment at: clang/lib/Sema/SemaDecl.cpp:11258 // are handled by a dataflow analysis. - if (!VDecl->hasLocalStorage() || VDecl->getType()->isRecordType() || - VDecl->

[PATCH] D65092: [clang] Add isDirectlyDerivedFrom AST Matcher.

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2672 + BaseName, 1) { + assert(!BaseName.empty()); + return isDirectlyDerivedFrom(hasName(BaseName)) I don't think this assertion is reasonable

[PATCH] D65092: [clang] Add isDirectlyDerivedFrom AST Matcher.

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2672 + BaseName, 1) { + assert(!BaseName.empty()); + return isDirectlyDerivedFrom(hasName(BaseName)) AntonBikineev wrote: > aaron.ballman wrote:

[PATCH] D64666: [Sema] Enable -Wimplicit-float-conversion for integral to floating point precision loss

aaron.ballman added a comment. In D64666#1599544 , @jfb wrote: > In D64666#1599524 , @ziangwan wrote: > > > In D64666#1598194 , @jfb wrote: > > > > > Thanks, the update looks

[PATCH] D65092: [clang] Add isDirectlyDerivedFrom AST Matcher.

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

[PATCH] D65176: [NFC][clang] Refactor getCompilationPhases()+Types.def step 2.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/include/clang/Driver/Types.def:39-45 +// Some of the options in Flags have been removed, so far those are: +// a - The type should only

[PATCH] D65212: [analyzer] Fix exporting SARIF files from scan-build on Windows

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Do you need someone to commit on your behalf? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65212/new/ https://reviews.llvm.org/D65212 ___

[PATCH] D65212: [analyzer] Fix exporting SARIF files from scan-build on Windows

aaron.ballman closed this revision. aaron.ballman added a comment. I've commit on your behalf in r366941, thank you for the patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65212/new/ https://reviews.llvm.org/D65212 _

[PATCH] D64914: Implement P1771

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:296-301 + if (A) { +StringRef Msg = A->getMessage(); +if (!Msg.empty()) + Diag(Loc, diag::warn_unused_constructor_msg) << A << Msg << R1 << R2; +else + Diag

<    9   10   11   12   13   14   15   16   17   18   >