[PATCH] D54900: [Sema] Avoid CallExpr::setNumArgs in Sema::BuildCallToObjectOfClassType

aaron.ballman added inline comments. Comment at: lib/Sema/SemaOverload.cpp:13260 + // The number of arguments slots to allocate in the call. + // If we have default arguments we need to allocate space for them arguments -> argument Also, I think this comment

[PATCH] D54900: [Sema] Avoid CallExpr::setNumArgs in Sema::BuildCallToObjectOfClassType

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

[PATCH] D54902: [AST][Sema] Remove CallExpr::setNumArgs

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from two small NFC nits. Comment at: include/clang/AST/ExprCXX.h:168 public: - CXXMemberCallExpr(ASTContext &C, Expr *fn, ArrayRef args, -

[PATCH] D54941: [clang-tidy] Ignore bool -> single bit bitfield conversion in readability-implicit-bool-conversion

aaron.ballman added inline comments. Comment at: docs/clang-tidy/checks/readability-implicit-bool-conversion.rst:77-78 -- boolean expression/literal to integer, +- boolean expression/literal to integer (conversion from boolean to a single + bit bitfield is explicitly allowed)

[PATCH] D54405: Record whether a AST Matcher constructs a Node

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Please commit with whatever patch makes use of `nodeConstructors()` (as this functionality doesn't stand on its own). Repository: rC Clang CHANGES SINCE LAST ACTION h

[PATCH] D54737: [clang-tidy] Add the abseil-duration-comparison check

aaron.ballman added inline comments. Comment at: clang-tidy/abseil/DurationComparisonCheck.cpp:25 +static llvm::Optional getScaleForInverse(llvm::StringRef Name) { + static const std::unordered_map ScaleMap( + {{"ToDoubleHours", DurationScale::Hours}, sammc

[PATCH] D54737: [clang-tidy] Add the abseil-duration-comparison check

aaron.ballman added inline comments. Comment at: clang-tidy/abseil/DurationComparisonCheck.cpp:25 +static llvm::Optional getScaleForInverse(llvm::StringRef Name) { + static const std::unordered_map ScaleMap( + {{"ToDoubleHours", DurationScale::Hours}, sammc

[PATCH] D54941: [clang-tidy] Ignore bool -> single bit bitfield conversion in readability-implicit-bool-conversion

aaron.ballman added inline comments. Comment at: docs/clang-tidy/checks/readability-implicit-bool-conversion.rst:77-78 -- boolean expression/literal to integer, +- boolean expression/literal to integer (conversion from boolean to a single + bit bitfield is explicitly allowed)

[PATCH] D54737: [clang-tidy] Add the abseil-duration-comparison check

aaron.ballman added inline comments. Comment at: clang-tidy/abseil/DurationComparisonCheck.cpp:25 +static llvm::Optional getScaleForInverse(llvm::StringRef Name) { + static const std::unordered_map ScaleMap( + {{"ToDoubleHours", DurationScale::Hours}, sammc

[PATCH] D53751: [ASTImporter] Added Import functions for transition to new API.

aaron.ballman added a comment. In D53751#1301551 , @shafik wrote: > I think these changes make sense at a high level but I am not sure about the > refactoring strategy. I am especially concerned we may end up in place where > all the effected users of th

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

aaron.ballman added inline comments. Comment at: lib/Sema/SemaTemplate.cpp:3070 +// and returns true. +static bool prettyPrintTypeTrait(const NestedNameSpecifier *const NNS, + llvm::raw_string_ostream &OS, No need for the pointer i

[PATCH] D53655: [ASTImporter] Fix redecl chain of classes and class templates

aaron.ballman accepted this revision. aaron.ballman added a comment. This seems mostly reasonable to me, but @rsmith is more well-versed in this area and may have further comments. You should give him a few days to chime in before committing. Comment at: include/clang/AST/Dec

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

aaron.ballman added inline comments. Comment at: lib/Sema/SemaTemplate.cpp:3076 +return false; + const RecordDecl * Record = NNS->getAsRecordDecl(); + // In namespace "::std". Formatting is incorrect here; you should run the patch through clang-format. Can

[PATCH] D53751: [ASTImporter] Added Import functions for transition to new API.

aaron.ballman added a comment. In D53751#1311037 , @martong wrote: > > I didn't actually see this comment get addressed other than to say it won't > > be a problem in practice, which I'm not certain I agree with. Was there a > > reason why this got commi

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

aaron.ballman added inline comments. Comment at: test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp:136 Agg f8 = {EnumVal}; // OK + // expected-warning@+1 {{implicit conversion from 'int' to 'float' changes value from 123456789 to 1.2345679E+8}} Agg f9 = {123456789}; //

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

aaron.ballman added inline comments. Comment at: clang-tidy/utils/FixItHintUtils.cpp:35 +static bool isValueType(QualType QT) { return isValueType(QT.getTypePtr()); } +static bool isArrayType(QualType QT) { return isa(QT.getTypePtr()); } +static bool isReferenceType(QualType QT)

[PATCH] D55068: NFC: Simplify dumpStmt child handling

aaron.ballman added inline comments. Comment at: lib/AST/ASTDumper.cpp:1987 +ConstStmtVisitor::Visit(S); + Was there something special about calling the Visit methods directly and bailing out? Your code certainly looks reasonable, but I wonder if the outpu

[PATCH] D55069: Extend the CommentVisitor with parameter types

aaron.ballman added a comment. Overall this seems reasonable, but this change is currently a no-op because nothing is using this -- what's the plan for using and testing these changes? Comment at: include/clang/AST/CommentVisitor.h:31 - RetTy Visit(PTR(Comment) C) { + RetT

[PATCH] D55070: Replace FullComment member being visited with parameter

aaron.ballman added a comment. This is a really nice cleanup, thank you! Comment at: lib/AST/ASTDumper.cpp:114-115 +public ConstStmtVisitor, +public CommentVisitorBase, +public TypeVisitor { Why not inherit from `ConstCommentVisitor` ins

[PATCH] D55069: Extend the CommentVisitor with parameter types

aaron.ballman added a comment. In D55069#1313276 , @aaron.ballman wrote: > Overall this seems reasonable, but this change is currently a no-op because > nothing is using this -- what's the plan for using and testing these changes? The answer is: D55070

[PATCH] D55070: Replace FullComment member being visited with parameter

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

[PATCH] D55069: Extend the CommentVisitor with parameter types

aaron.ballman added a comment. In D55069#1313591 , @steveire wrote: > The follow-up patch didn't build anymore with the `&&`. > > ../tools/clang/lib/AST/ASTDumper.cpp:2672:75: error: cannot bind rvalue > reference of type ‘const clang::comments::FullCom

[PATCH] D55083: Re-arrange content in FunctionDecl dump

aaron.ballman added a comment. The summary explains what it's doing, but not why it's doing it -- why is this change in behavior needed? Does this not break any tests? Btw, as we work on this refactoring, I think a good approach will be to build up the base of tests around AST dumping so that w

[PATCH] D55068: NFC: Simplify dumpStmt child handling

aaron.ballman added inline comments. Comment at: lib/AST/ASTDumper.cpp:1987 +ConstStmtVisitor::Visit(S); + steveire wrote: > aaron.ballman wrote: > > Was there something special about calling the Visit methods directly and > > bailing out? Your code certai

[PATCH] D55101: [clang-tidy] Ignore namespaced and C++ member functions in google-objc-function-naming check 🙈

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: test/clang-tidy/google-objc-function-naming.mm:24-25 + +class Baz { + public: +int value() { return 0; } Can you run thi

[PATCH] D55069: Extend the CommentVisitor with parameter types

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D55069#1314508 , @steveire wrote: > > Huh, that's surprising. It's a perfect forwarding reference > > It's not a forwarding reference b

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

aaron.ballman added inline comments. Comment at: lib/AST/NestedNameSpecifier.cpp:308-310 +if (ResolveTemplateArguments && getAsRecordDecl()) { + if (const auto *Record = + dyn_cast(getAsRecordDecl())) { I'd remove the `getAsRecordDecl()` fro

[PATCH] D51211: [Sema] Emit -Wformat properly for bitfield promotions.

aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:7598-7602 + if (ICE->getCastKind() == CK_IntegralCast && + From->isPromotableIntegerType() && + S.Context.getPromotedIntegerType(From) == To) +return true; + return false;

[PATCH] D55083: Re-arrange content in FunctionDecl dump

aaron.ballman added a comment. In D55083#1314546 , @steveire wrote: > Yes, please commit your new tests for FunctionDecl dumping before this patch > can go in. r347994 has those tests. Repository: rC Clang CHANGES SINCE LAST ACTION https://review

[PATCH] D51211: [Sema] Emit -Wformat properly for bitfield promotions.

aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:7711-7715 + // It's an integer promotion if the destination type is the promoted + // source type. + return ICE->getCastKind() == CK_IntegralCast && + From->isPromotableIntegerType() && +

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

aaron.ballman added inline comments. Comment at: include/clang/AST/NestedNameSpecifier.h:220 + void print(raw_ostream &OS, const PrintingPolicy &Policy, + bool ResolveTemplateArguments = false) const; Quuxplusone wrote: > Peanut gallery says: Shoul

[PATCH] D54450: Get the correct range of tokens for preprocessor conditions

aaron.ballman added a comment. P-p-power ping! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54450/new/ https://reviews.llvm.org/D54450 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D55101: [clang-tidy] Ignore namespaced and C++ member functions in google-objc-function-naming check 🙈

aaron.ballman added a comment. In D55101#1315294 , @benhamilton wrote: > > Would you be okay with landing this fix and if we get any further reports > > for Objective-C++ sources then we can suppress it in all C++/Objective-C++ > > sources? I think there

[PATCH] D55068: NFC: Simplify dumpStmt child handling

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D55068#1315924 , @steveire wrote: > Aaron, you added tests for the existing behavior which pass after this patch. > Is there anything

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

aaron.ballman added inline comments. Comment at: lib/Sema/SemaTemplate.cpp:3055 +// Print a diagnostic for the failing static_assert expression. Defaults to +// pretty-printing the expression. Comment is a bit out of date as this is no longer specific to `stati

[PATCH] D55188: Extract TextChildDumper class

aaron.ballman added inline comments. Comment at: include/clang/AST/ASTDumperUtils.h:22 + +namespace ast_dumper { +// Colors used for various parts of the AST dump I'm not certain this namespace is useful, especially when it gets imported at TU scope in ASTDumper

[PATCH] D55212: Handle alloc_size attribute on function pointers

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:1072 def AllocSize : InheritableAttr { let Spellings = [GCC<"alloc_size">]; + let Subjects = SubjectList<[HasFunctionProto]>; Does GCC support writing `alloc_size` on function point

[PATCH] D55188: Extract TextChildDumper class

aaron.ballman added inline comments. Comment at: include/clang/AST/ASTDumperUtils.h:22 + +namespace ast_dumper { +// Colors used for various parts of the AST dump steveire wrote: > aaron.ballman wrote: > > I'm not certain this namespace is useful, especially when

[PATCH] D51211: [Sema] Emit -Wformat properly for bitfield promotions.

aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:7711-7715 + // It's an integer promotion if the destination type is the promoted + // source type. + return ICE->getCastKind() == CK_IntegralCast && + From->isPromotableIntegerType() && +

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

aaron.ballman added inline comments. Comment at: lib/Sema/SemaTemplate.cpp:3055 +// Print a diagnostic for the failing static_assert expression. Defaults to +// pretty-printing the expression. courbet wrote: > aaron.ballman wrote: > > Comment is a bit out of da

[PATCH] D54903: [Sema] Improve static_assert diagnostics.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: lib/Sema/SemaTemplate.cpp:3055 +// Print a diagnostic for the failing static_assert expression. Defaults to +// pretty-printing the expressi

[PATCH] D51211: [Sema] Emit -Wformat properly for bitfield promotions.

aaron.ballman added inline comments. Comment at: test/Sema/format-strings-bitfield-promotion.c:1 +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s + ebevhan wrote: > ebevhan wrote: > > aaron.ballman wrote: > > > aaron.ballman wrote: > >

[PATCH] D55221: [AST] Make ArrayTypeTraitExpr non-polymorphic.

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

[PATCH] D55225: [AST] Assert that no type class is polymorphic

aaron.ballman added inline comments. Comment at: lib/AST/Type.cpp:299 + static_assert(!std::is_polymorphic::value, \ +#CLASS "Type should not be polymorphic!"); +#include "clang/AST/TypeNodes.def" This will squish your class name and error text t

[PATCH] D55225: [AST] Assert that no type class is polymorphic

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: lib/AST/Type.cpp:299 + static_assert(!std::is_polymorphic::value, \ +#CLASS "Type should not be polymorphic!"); +#include "cl

[PATCH] D55212: Handle alloc_size attribute on function pointers

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:1072 def AllocSize : InheritableAttr { let Spellings = [GCC<"alloc_size">]; + let Subjects = SubjectList<[HasFunctionProto]>; arichardson wrote: > aaron.ballman wrote: > > Does GCC

[PATCH] D55222: [AST] Assert that no statement/expression class is polymorphic

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

[PATCH] D55188: Extract TextChildDumper class

aaron.ballman added inline comments. Comment at: include/clang/AST/ASTDumperUtils.h:22 + +namespace ast_dumper { +// Colors used for various parts of the AST dump steveire wrote: > aaron.ballman wrote: > > steveire wrote: > > > aaron.ballman wrote: > > > > I'm no

[PATCH] D55189: Extract TextNodeDumper class

aaron.ballman added a comment. Basically looking good, modulo namespace questions from D55188 and a few other organizational questions. Comment at: include/clang/AST/ASTTextNodeDumper.h:1 +//===--- ASTTextNodeDumper.h - Printing of AST nodes

[PATCH] D55212: Handle alloc_size attribute on function pointers

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:1072 def AllocSize : InheritableAttr { let Spellings = [GCC<"alloc_size">]; + let Subjects = SubjectList<[HasFunctionProto]>; arichardson wrote: > aaron.ballman wrote: > > arichards

[PATCH] D55190: Move dump of individual comment nodes to NodeDumper

aaron.ballman added a comment. Looks reasonable, but similar to D55189 , can more of the implementation be pushed into a .cpp file rather than left in a header? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55190/new/ https://re

[PATCH] D55257: Inline handling of DependentSizedArrayType

aaron.ballman added a comment. > It is necessary to perform all printing before any traversal to child nodes. This piqued my interest -- is `VisitFunctionDecl()` then incorrect because it streams output, then dumps parameter children, then dumps more output, then dumps override children? Or do

[PATCH] D55257: Inline handling of DependentSizedArrayType

aaron.ballman added a comment. In D55257#1318376 , @steveire wrote: > In D55257#1318328 , @aaron.ballman > wrote: > > > > It is necessary to perform all printing before any traversal to child > > > nodes. > > > >

[PATCH] D55257: Inline handling of DependentSizedArrayType

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D55257#1319016 , @steveire wrote: > In D55257#1318769 , @aaron.ballman > wrote: > > > In D552

[PATCH] D55188: Extract TextChildDumper class

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a commenting nit. Comment at: include/clang/AST/ASTDumperUtils.h:112 +public: + /// Dump a child of the current node. + template void addChil

[PATCH] D55189: Extract TextNodeDumper class

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: lib/AST/ASTDumper.cpp:90 // Utilities -void dumpPointer(const void *Ptr); -void dumpSourceRange(SourceRange R); -void dumpLoc

[PATCH] D51211: [Sema] Emit -Wformat properly for bitfield promotions.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: test/Sema/format-strings-bitfield-promotion.c:1 +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s +

[PATCH] D55270: [Sema] Further improvements to to static_assert diagnostics.

aaron.ballman added inline comments. Comment at: lib/Sema/SemaTemplate.cpp:3065 + + ~FailedBooleanConditionPrinterHelper() override {} + Is this definition necessary? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55270/new/ htt

[PATCH] D55321: Do not check for parameters shadowing fields in function declarations

aaron.ballman created this revision. aaron.ballman added reviewers: rsmith, george.karpenkov. Herald added subscribers: kristof.beyls, javed.absar. This patch changes the way we handle `-Wshadow-field` so that we do not issue diagnostics for parameters in function declarations (as those are harml

[PATCH] D55321: Do not check for parameters shadowing fields in function declarations

aaron.ballman updated this revision to Diff 176807. aaron.ballman added a comment. Added a test with an out-of-line definition. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55321/new/ https://reviews.llvm.org/D55321 Files: lib/Sema/SemaChecking.cpp lib/Sema/SemaDecl.cpp test/Sem

[PATCH] D55321: Do not check for parameters shadowing fields in function declarations

aaron.ballman marked 2 inline comments as done. aaron.ballman added inline comments. Comment at: test/SemaCXX/warn-shadow.cpp:236 } + void F(int B); // Ok, declaration; not definition. }; lebedev.ri wrote: > Can you please also add one function with out-of-l

[PATCH] D55321: Do not check for parameters shadowing fields in function declarations

aaron.ballman marked 2 inline comments as done. aaron.ballman added inline comments. Comment at: test/SemaCXX/warn-shadow.cpp:236 } + void F(int B); // Ok, declaration; not definition. }; steveire wrote: > aaron.ballman wrote: > > lebedev.ri wrote: > > > Can

[PATCH] D55321: Do not check for parameters shadowing fields in function declarations

aaron.ballman closed this revision. aaron.ballman added a comment. Committed in r348400. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55321/new/ https://reviews.llvm.org/D55321 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D55337: NFC: Move dumpDeclRef to NodeDumper

aaron.ballman added inline comments. Comment at: include/clang/AST/TextNodeDumper.h:28 const comments::FullComment *> { + TextTreeStructure &TreeStructure; raw_ostream &OS; This makes me a bit wary because you creat

[PATCH] D55338: NFC: Move VisitStmt code to dumpStmt

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

[PATCH] D55190: Move dump of individual comment nodes to NodeDumper

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

[PATCH] D55339: NFC: Move VisitExpr code to dumpStmt

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a small nit. Comment at: lib/AST/ASTDumper.cpp:1698 +if (auto *E = dyn_cast(S)) { + NodeDumper.dumpType(E->getType()); --

[PATCH] D55340: NFC: Move dump of individual Stmts to TextNodeDumper

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some small nits. Comment at: lib/AST/TextNodeDumper.cpp:685 +void TextNodeDumper::VisitObjCSelectorExpr(const ObjCSelectorExpr *Node) { + + OS

[PATCH] D55212: Handle alloc_size attribute on function pointers

aaron.ballman added a comment. I think all that's missing are some C++ tests and some nits. Comment at: include/clang/Basic/Attr.td:1072 def AllocSize : InheritableAttr { let Spellings = [GCC<"alloc_size">]; + let Subjects = SubjectList<[HasFunctionProto]>; ---

[PATCH] D55270: [Sema] Further improvements to to static_assert diagnostics.

aaron.ballman added inline comments. Comment at: test/PCH/cxx-static_assert.cpp:17 -// expected-error@12 {{static_assert failed "N is not 2!"}} +// expected-error@12 {{static_assert failed due to requirement '1 == 2' "N is not 2!"}} T<1> t1; // expected-note {{in instantiatio

[PATCH] D42682: [clang-tidy] Add io-functions-misused checker

aaron.ballman added inline comments. Comment at: clang-tidy/bugprone/IoFunctionsCheck.cpp:32 +has(cxxMemberCallExpr( +on(hasType(namedDecl(hasAnyName("istream", +callee(cxxMethodDecl(hasName("get")).bind("DeclOfGet")

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

aaron.ballman added inline comments. Comment at: clang/docs/UsersManual.rst:3137 -W Enable the specified warning - -XclangPass to the clang compiler + -XclangPass to the clang cc1 frontend. For experimental use only. -

[PATCH] D55212: Handle alloc_size attribute on function pointers

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from two small style nits with the new test file. Comment at: test/SemaCXX/alloc-size.cpp:6 + int alloc(int) __attribute__((alloc_size(2))); // exp

[PATCH] D55393: Re-order content of template parameter dumps

aaron.ballman requested changes to this revision. aaron.ballman added inline comments. This revision now requires changes to proceed. Comment at: test/AST/ast-dump-decl.cpp:330-331 // CHECK-NEXT: TemplateTypeParmDecl -// CHECK-NEXT: TemplateArgument type 'int' // CHECK-NE

[PATCH] D55083: Re-arrange content in FunctionDecl dump

aaron.ballman added inline comments. Comment at: test/AST/ast-dump-decl.cpp:217 // CHECK-NEXT: ParmVarDecl +// CHECK-NEXT: TemplateArgument // CHECK-NEXT: CompoundStmt I find this ordering to be confusing, especially because the template argument i

[PATCH] D55394: Re-order type param children of ObjC nodes

aaron.ballman added reviewers: ABataev, rsmith. aaron.ballman added a comment. I don't know enough about ObjC to feel comfortable saying whether this is a reasonable reordering or not, so adding some reviewers. Comment at: test/AST/ast-dump-decl.m:90 // CHECK-NEXT: -ObjCPro

[PATCH] D55393: Re-order content of template parameter dumps

aaron.ballman added inline comments. Comment at: test/AST/ast-dump-decl.cpp:330-331 // CHECK-NEXT: TemplateTypeParmDecl -// CHECK-NEXT: TemplateArgument type 'int' // CHECK-NEXT: inherited from TemplateTypeParm 0x{{[^ ]*}} 'T' +// CHECK-NEXT: TemplateArgument type

[PATCH] D55270: [Sema] Further improvements to to static_assert diagnostics.

aaron.ballman added inline comments. Comment at: test/PCH/cxx-static_assert.cpp:17 -// expected-error@12 {{static_assert failed "N is not 2!"}} +// expected-error@12 {{static_assert failed due to requirement '1 == 2' "N is not 2!"}} T<1> t1; // expected-note {{in instantiatio

[PATCH] D55097: [constexpr][c++2a] Try-catch blocks in constexpr functions

aaron.ballman added a comment. Some minor nits from the peanut gallery. Comment at: lib/AST/ExprConstant.cpp:4278 + case Stmt::CXXTryStmtClass: +// Evaluate try blocks by evaluating all sub statements +return EvaluateStmt(Result, Info, cast(S)->getTryBlock(), Case); -

[PATCH] D55097: [constexpr][c++2a] Try-catch blocks in constexpr functions

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a few other small nits. Comment at: lib/Sema/SemaDeclCXX.cpp:1904 + case Stmt::CXXTryStmtClass: +if (!Cxx2aLoc.isValid()) + Cxx2aLoc =

[PATCH] D55395: Re-order content in OMPDeclareReductionDecl dump

aaron.ballman added a comment. This is a novel approach that's not used anywhere else in the AST dumper and there are several ways we could handle this, including: - What's proposed (adding a new node to the tree that's not directly an AST node) - Making use of the pointer information. e.g., ht

[PATCH] D55395: Re-order content in OMPDeclareReductionDecl dump

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM with a small nit. @ABataev, are you okay with this approach? Comment at: lib/AST/ASTDumper.cpp:1060 + dumpStmt(D->getCombiner()); + if (auto *Initializer = D->getInitializer()) { dumpStmt(Initializ

[PATCH] D55393: Re-order content of template parameter dumps

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some minor nits Comment at: lib/AST/ASTDumper.cpp:106 + const Decl *From = nullptr, +

[PATCH] D55489: Implement dumpFunctionDeclParameters in NodeDumper

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

[PATCH] D55490: Add dumpMethodDeclOverrides to NodeDumper

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, aside from a minor nit. Comment at: lib/AST/ASTDumper.cpp:921 if (const CXXMethodDecl *MD = dyn_cast(D)) { +dumpMethodDeclOverrides(MD);

[PATCH] D55495: Change InitListExpr dump to label and pointer

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a small nit. Comment at: lib/AST/ASTDumper.cpp:1956-1959 + OS << " array_filler"; + NodeDumper.dumpPointer(Filler); + dumpStmt(

[PATCH] D55492: Implement Attr dumping in terms of visitors

aaron.ballman added inline comments. Comment at: include/clang/AST/AttrVisitor.h:21 + +namespace attrvisitor { + I don't think the namespace adds value. Comment at: include/clang/AST/AttrVisitor.h:23-24 + +template struct make_ptr { using type

[PATCH] D55492: Implement Attr dumping in terms of visitors

aaron.ballman added inline comments. Comment at: include/clang/AST/AttrVisitor.h:21 + +namespace attrvisitor { + steveire wrote: > aaron.ballman wrote: > > I don't think the namespace adds value. > I think the point is to separate the implementation detail. I don

[PATCH] D55270: [Sema] Further improvements to to static_assert diagnostics.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: test/PCH/cxx-static_assert.cpp:17 -// expected-error@12 {{static_assert failed "N is not 2!"}} +// expected-error@12 {{static_assert failed

[PATCH] D55492: Implement Attr dumping in terms of visitors

aaron.ballman added inline comments. Comment at: include/clang/AST/AttrVisitor.h:23-24 + +template struct make_ptr { using type = T *; }; +template struct make_const_ptr { using type = const T *; }; + aaron.ballman wrote: > steveire wrote: > > aaron.ballman wro

[PATCH] D55346: [clang-tidy] check for using declaration qualification

aaron.ballman added inline comments. Comment at: clang-tidy/abseil/QualifiedAliasesCheck.cpp:29 + // Finds the nested-name-specifier location. + const NestedNameSpecifierLoc QualifiedLoc = MatchedDecl->getQualifierLoc(); + const SourceLocation FrontLoc = QualifiedLoc.getBeginL

[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace

aaron.ballman added inline comments. Comment at: clang-tidy/abseil/SafelyScopedCheck.cpp:22 + // The target using declaration is either: + // 1. not in any namespace declaration, or + // 2. in some namespace declaration but not in the innermost layer Why is th

[PATCH] D55409: [clang-tidy] check for using declarations not in an anonymous namespace when there exists one

aaron.ballman requested changes to this revision. aaron.ballman added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/abseil/AnonymousEnclosedAliasesCheck.cpp:32 +void AnonymousEnclosedAliasesCheck::check(const MatchFinder::MatchResult &Res

[PATCH] D55410: [clang-tidy] check for flagging using declarations in headers

aaron.ballman added a comment. Have you run this check over any large code bases to see what its false positive rate looks like? I have to imagine we're going to need some escape hatch for system headers (we shouldn't complain about using declarations outside of the user's control).

[PATCH] D55447: [Sema] Fix Modified Type in address_space AttributedType

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. Missing test cases. Comment at: clang/lib/Sema/SemaType.cpp:5747 +static bool BuildAddressSpaceIndex(LangAS &ASIdx, const Expr *AddrSpace, +

[PATCH] D54450: Get the correct range of tokens for preprocessor conditions

aaron.ballman added inline comments. Comment at: lib/Lex/PPExpressions.cpp:154-156 // Consume the ). -Result.setEnd(PeekTok.getLocation()); PP.LexNonComment(PeekTok); +Result.setEnd(PeekTok.getLocation()); lebedev.ri wrote: > I'm not sure this i

[PATCH] D54450: Get the correct range of tokens for preprocessor conditions

aaron.ballman updated this revision to Diff 177508. aaron.ballman marked 3 inline comments as done. aaron.ballman added a comment. Adding tests based on review feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54450/new/ https://reviews.llvm.org/D54450 Files: include/clang/Lex/

[PATCH] D55482: [clang-tidy] Improve google-objc-function-naming diagnostics 📙

aaron.ballman added inline comments. Comment at: clang-tidy/google/FunctionNamingCheck.cpp:114-125 + if (MatchedDecl->getStorageClass() == SC_Static) { +diag(MatchedDecl->getLocation(), + "static function name %0 must be in Pascal case as required by " + "Goo

[PATCH] D55510: [ExprConstant] Improve memchr/memcmp for type mismatch and multibyte element types

aaron.ballman added inline comments. Comment at: lib/AST/ExprConstant.cpp:6151 +Info.Ctx.getBaseElementType(getType(Result.getLValueBase())); +const bool IsRawByte = BuiltinOp == Builtin::BImemchr || + BuiltinOp == Builtin::BI__builtin_memchr

[PATCH] D55488: Add utility for dumping a label with child nodes

aaron.ballman added inline comments. Comment at: include/clang/AST/ASTDumperUtils.h:115 + template + void addChild(const std::string &label, Fn doAddChild) { +if (label.empty()) label -> Label doAddChild -> DoAddChild Comment at: include

<    4   5   6   7   8   9   10   11   12   13   >