[PATCH] D98160: [clang] Use decltype((E)) for compound requirement type constraint

rsmith added a comment. Thanks, I like this approach. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2761 def note_expr_requirement_constraints_not_satisfied_simple : Note< - "%select{and|because}0 %1 does not satisfy %2:">; + "%select{and|because}0

[PATCH] D98193: [CUDA][HIP] Allow non-ODR use of host var in device

rsmith added a comment. In principle this seems reasonable to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98193/new/ https://reviews.llvm.org/D98193 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D98193: [CUDA][HIP] Allow non-ODR use of host var in device

rsmith added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:17050 +auto Target = SemaRef.IdentifyCUDATarget(FD); +if (Var && Var->isFileVarDecl() && !Var->hasAttr() && +!Var->hasAttr() && !Var->hasAttr() && I suspect you want

[PATCH] D99466: Fix PR48889: assertion failure for void() in flattened ternary expression

rsmith accepted this revision. rsmith added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:466-469 +// [expr.type.conv]: if the type is cv void and the initializer is () or {}, +// the expression is a

[PATCH] D99407: [clang][ItaniumMangle] Check SizeExpr for DependentSizedArrayType (PR49478)

rsmith added inline comments. Comment at: clang/test/AST/ast-dump-array-json.cpp:6 + +// CHECK: "mangledName": "_ZN1A4TestE", +// CHECK: "mangledName": "_ZN1A4TestE", oToToT wrote: > yaxunl wrote: > > I am not sure if this test tests the code you change since

[PATCH] D97831: [Clang][Sema] Implement GCC -Wcast-function-type

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Looks good. Some possible improvements to make the diagnostic a bit more precise, but I'd be happy if you want to address some of those as follow-ups after this change.

[PATCH] D97831: [Clang][Sema] Implement GCC -Wcast-function-type

rsmith added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8368 +def warn_cast_function_type : Warning< + "cast between incompatible function types from %0 to %1">, + InGroup, DefaultIgnore; I'd find something like "cast from

[PATCH] D99134: Lambdas are not necessarily locals. This resolves DR48250.

rsmith added a comment. Ah, I think I see how we get here. I think this fix might possibly break a case such as: template int x = [](auto){ return T(); }.operator()(T()); int y = x; ... where transforming the call to `operator()` might end up looking for the transformed version of the

[PATCH] D99134: Lambdas are not necessarily locals. This resolves DR48250.

rsmith added a comment. Hm, do we ever call `FindInstantiatedDecl` on a lambda class in the first place? It seems plausible to me that this condition is unreachable. But to the extent that it's reachable, it seems mostly correct: a lambda expression appearing in a non-dependent context might

[PATCH] D99117: Attempt to further improve the documentation for the [[clang::lifetimebound]] attribute.

rsmith added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3038-3039 +is considered to refer to its underlying array, and aggregates (arrays and +simple ``struct``s) are considered to refer to all objects that their +subobjects refer to. +

[PATCH] D99117: Attempt to further improve the documentation for the [[clang::lifetimebound]] attribute.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5fab60377c1a: Attempt to further improve the documentation for the [[clang::lifetimebound]]… (authored by rsmith). Changed prior to commit:

[PATCH] D99117: Attempt to further improve the documentation for the [[clang::lifetimebound]] attribute.

rsmith created this revision. rsmith added a reviewer: dblaikie. Herald added a reviewer: aaron.ballman. rsmith requested review of this revision. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D99117 Files: clang/include/clang/Basic/AttrDocs.td

[PATCH] D98889: [clang] Replaced some manual pointer tagging with llvm::PointerIntPair.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc3134d7c44f1: [clang] Replaced some manual pointer tagging with llvm::PointerIntPair. (authored by haberman, committed by rsmith). Repository: rG

[PATCH] D98912: Fix -Winteger-overflow to diagnose regardless of the top-level syntactic form.

rsmith created this revision. rsmith added a reviewer: vsapsai. rsmith requested review of this revision. Herald added a project: clang. Previously -Winteger-overflow did not do any checking of expressions whose top-level syntactic form wasn't one of a small list of special cases. This meant that

[PATCH] D98904: Instantiate static constexpr data members on MS ABI.

rsmith added a comment. In D98904#2636109 , @zoecarver wrote: > I think we'd basically need a condition that says: `is-microsoft && > less-than-cxx17 && is-constexpr && is-static-data-member`. Yes. Looking back over the history a bit here,

[PATCH] D98904: Instantiate static constexpr data members on MS ABI.

rsmith added a comment. In D98904#2636021 , @zoecarver wrote: > So, as I mentioned in the description of this patch, the reason that these > members are treated as definitions is that Clang implicitly marks them as > inline when targeting MS. This is

[PATCH] D98889: [clang] Replaced some manual pointer tagging with llvm::PointerIntPair.

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Thanks! Do we need `InitializedEntity::getDecl` to return a pointer to non-const? Looks good assuming we do. Comment at: clang/include/clang/Sema/Initialization.h:190

[PATCH] D98816: PR49585: Emit the jump destination for a for loop 'continue' from within the scope of the condition variable.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa875721d8a2d: PR49585: Emit the jump destination for a for loop continue from within the… (authored by rsmith). Repository: rG LLVM Github

[PATCH] D98816: PR49585: Emit the jump destination for a for loop 'continue' from within the scope of the condition variable.

rsmith added a comment. Note that this fixes a miscompile where we'd destroy a for loop condition variable twice when encountering a `continue` (once before the increment and again after). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98816/new/

[PATCH] D98816: PR49585: Emit the jump destination for a for loop 'continue' from within the scope of the condition variable.

rsmith created this revision. rsmith added a reviewer: rjmccall. rsmith requested review of this revision. Herald added a project: clang. The condition variable is in scope in the loop increment, so we need to emit the jump destination from wthin the scope of the condition variable. For GCC

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

rsmith added a comment. @rjmccall I'd like your input on this patch in particular, if you have time. I'm nervous in general about the looming idea of declaration unloading, but the fact that it's been working in Cling for a long time gives me some confidence that we can do that in a way that's

[PATCH] D98363: [Sema] Fold VLA types in compound literals to constant arrays.

rsmith added inline comments. Comment at: clang/test/Sema/vla.c:134 + // expected-warning@+1{{variable length array folded to constant array as an extension}} + char (*a9)[] = (char[2][ksize]) {{1,2,3,4},{4,3,2,1}}; + aaron.ballman wrote: > efriedma wrote: >

[PATCH] D97990: [clang] Improve diagnostics on implicitly deleted defaulted comparisons

rsmith accepted this revision. rsmith added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8968-8970 def note_defaulted_comparison_calls_deleted : Note< "defaulted %0 is implicitly deleted

[PATCH] D98104: Update __is_unsigned builtin to match the Standard.

rsmith added a comment. LGTM too, once the remaining pending comments are addressed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98104/new/ https://reviews.llvm.org/D98104 ___ cfe-commits mailing list

[PATCH] D98104: Update __is_unsigned builtin to match the Standard.

rsmith added inline comments. Comment at: clang/docs/LanguageExtensions.rst:1207 * ``__is_union`` (C++, GNU, Microsoft, Embarcadero) * ``__is_unsigned`` (C++, Embarcadero) + Returns false for enumeration types. Note, before Clang 13, returned true for Please

[PATCH] D98160: [clang] Use decltype((E)) for compound requirement type constraint

rsmith added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:5446 // Build a new, canonical decltype(expr) type. Canon = new (*this, TypeAlignment) DependentDecltypeType(*this, e); DependentDecltypeTypes.InsertNode(Canon, InsertPos);

[PATCH] D98095: [clang] Fix ICE on invalid type parameters for concepts

rsmith added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:1644-1661 +TemplateIdAnnotation *TemplateId = DS.getRepAsTemplateId(); +ConceptDecl *TypeConstraintConcept = nullptr; +llvm::SmallVector TemplateArgs; if (DS.isConstrainedAuto()) { -

[PATCH] D97990: [clang] Always provide relevant subobject for 'no viable comparison'

rsmith added a comment. I don't think this change is right: for struct A { int bool operator==(const A&) const = default; }; we should warn about the reference member (as we do), but for struct A { int bool operator<(const A&) const = default; }; ... we shouldn't

[PATCH] D98087: [clang] Fix constrained decltype(auto) deduction

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG71e6e82746ca: [clang] Fix constrained decltype(auto) deduction (authored by mizvekov, committed by rsmith). Repository: rG LLVM Github Monorepo

[PATCH] D98087: [clang] Fix constrained decltype(auto) deduction

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Looks good, thanks! Comment at: clang/test/CXX/dcl/dcl.spec/dcl.type/dcl.spec.auto/p6.cpp:59-63 + True decltype(auto) e = a; + static_assert(is_same_v); + + True

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

rsmith added inline comments. Comment at: clang/test/Interpreter/execute.c:1 +// RUN: cat %s | clang-repl | FileCheck %s + Presumably here (and in all the interpreter tests) we will need to check that we configured Clang and LLVM so that they can actually JIT

[PATCH] D95396: Improve static_assert/_Static_assert diagnostics

rsmith added a comment. Looks like `test/FixIt/fixit-static-assert.cpp` is failing in Phabricator's pre-merge checks: B91556 . Please take a look at that before landing this; I think there's a decent chance that it's indicative of a real problem. CHANGES

[PATCH] D95691: Implement P2173 for attributes on lambdas

rsmith accepted this revision. rsmith added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Basic/DiagnosticGroups.td:269 +def CXXPre2BCompatPedantic : + DiagGroup<"c++98-c++11-c++14-c++17-c++20-compat-pedantic",

[PATCH] D88220: [C++20] P1825R0: More implicit moves

rsmith added inline comments. Comment at: clang/www/cxx_status.html:1206-1210 + + More implicit moves + https://wg21.link/p1825r0;>P1825R0 (DR) + Clang 13 + We have, so far, listed DR papers only in the section of `cxx_status` for the

[PATCH] D93031: Enable fexec-charset option

rsmith added inline comments. Comment at: clang/include/clang/Lex/Preprocessor.h:145 ModuleLoader + LiteralConverter LT; Please give this a longer name. Abbreviation names should only be used in fairly small scopes where it's easy to look up what

[PATCH] D97733: Fix infinite recursion during IR emission if a constant-initialized lifetime-extended temporary object's initializer refers back to the same object.

This revision was automatically updated to reflect the committed changes. Closed by commit rG9e2579dbf434: Fix infinite recursion during IR emission if a constant-initialized lifetime… (authored by rsmith). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D97733: Fix infinite recursion during IR emission if a constant-initialized lifetime-extended temporary object's initializer refers back to the same object.

rsmith added a comment. @rjmccall We could alternatively address this by creating and registering the global first, and only then building and adding its initializer, but either way we're going to need some special-case code to detect and handle this situation (because we might need to change

[PATCH] D97733: Fix infinite recursion during IR emission if a constant-initialized lifetime-extended temporary object's initializer refers back to the same object.

rsmith created this revision. rsmith added a reviewer: rjmccall. rsmith requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. `GetAddrOfGlobalTemporary` previously tried to emit the initializer of a global temporary before updating the global

[PATCH] D95409: [clang] implicitly delete space ship operator with function pointers

This revision was automatically updated to reflect the committed changes. Closed by commit rG4a8530fc3039: [clang] implicitly delete space ship operator with function pointers (authored by mizvekov, committed by rsmith). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D97204: [RFC] Clang 64-bit source locations

rsmith added a comment. Thanks for doing this! The 8-9% memory hit is better than I'd feared, but still seems uncomfortably large. I've left comments on a couple of places where I think we could substantially reduce this. The performance regression seems large enough that people will notice,

[PATCH] D65696: Implements CWG 2082 Referring to parameters in unevaluated operands of default arguments

rsmith added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:143-148 + /// CheckDefaultArgumentVisitorODR - C++ [dcl.fct.default] Traverses + /// the default argument of a parameter to determine whether it + /// contains ODR violations. These violations cannot be

[PATCH] D96147: [SEMA] Added warn_decl_shadow support for structured bindings

This revision was automatically updated to reflect the committed changes. Closed by commit rG039f79c78cfa: [SEMA] Added warn_decl_shadow support for structured bindings (authored by Vexthil, committed by rsmith). Changed prior to commit: https://reviews.llvm.org/D96147?vs=325578=325892#toc

[PATCH] D95409: [clang] implicitly delete space ship operator with function pointers

rsmith added a comment. Thanks, this looks fine. I wish we had a better way to determine whether a builtin comparison is usable without actually building a use of it. :-( In D95409#2565227 , @mizvekov wrote: > By the way I realize I may not have put the

[PATCH] D96147: [SEMA] Added warn_decl_shadow support for structured bindings

rsmith added a comment. Hm, before we go forward with this: can we get a test for the "shadows a structured binding" case, please? You might also need to add a new case to the "declaration shadows ..." warning text. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D96147: [SEMA] Added warn_decl_shadow support for structured bindings

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Looks good, thanks! Do you need someone to commit this for you? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96147/new/

[PATCH] D77598: Integral template argument suffix and cast printing

rsmith added inline comments. Comment at: clang/lib/AST/TemplateBase.cpp:90 + } else if (T->isWideCharType()) +Out << "L'" << reinterpret_cast(Val.getLimitedValue(WCHAR_MAX)) +<< "'"; `getLimitedValue` doesn't return a pointer, and the host

[PATCH] D97000: [clang] Increase the bitness of data length in ASTDeclContextNameLookup

rsmith added a comment. Thanks for the diagnosis and the patch! We spend a significant amount of our AST file size on these lookup tables, so I think the extra two bytes per lookup entry is best avoided if possible. We also have the same problem for other on-disk hash tables. I went ahead and

[PATCH] D95396: Improve static_assert/_Static_assert diagnostics

rsmith added a comment. @rnk / @thakis Can you take a look at this and see if you're happy with this "defining `assert` implicitly defines `static_assert`" approach? Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:427-429 +def warn_cxx_static_assert_in_c :

[PATCH] D95396: Improve static_assert/_Static_assert diagnostics

rsmith added inline comments. Comment at: clang/lib/Parse/ParseDeclCXX.cpp:874-876 +if (!getLangOpts().CPlusPlus) + Diag(Tok, diag::warn_cxx_static_assert_in_c) + << FixItHint::CreateReplacement(Tok.getLocation(), "_Static_assert");

[PATCH] D96147: [SEMA] Added warn_decl_shadow support for structured bindings

rsmith added a comment. Thanks! Comment at: clang/lib/Sema/SemaDecl.cpp:7571 + NamedDecl *ShadowedDecl = R.getFoundDecl(); + return isa(ShadowedDecl) || isa(ShadowedDecl) + ? ShadowedDecl rsmith wrote: > I think we should also warn if a

[PATCH] D95396: Improve static_assert/_Static_assert diagnostics

rsmith added a comment. FYI, https://reviews.llvm.org/D17444 captures some of the history here. Comment at: clang/lib/Parse/ParseDeclCXX.cpp:874-876 +if (!getLangOpts().CPlusPlus) + Diag(Tok, diag::warn_cxx_static_assert_in_c) + <<

[PATCH] D77598: Integral template argument suffix and cast printing

rsmith added inline comments. Comment at: clang/lib/AST/TemplateBase.cpp:110 +break; + default: +if (T->isUnsignedIntegerType() && T->isWideCharType()) I think we should use the prettier printing for `wchar_t` / `char8_t` / `char16_t` /

[PATCH] D95691: Implement P2173 for attributes on lambdas

rsmith added inline comments. Comment at: clang/include/clang/Basic/DiagnosticGroups.td:269 +def CXXPre2BCompatPedantic : + DiagGroup<"c++98-c++11-c++14-c++17-c++20-compat-pedantic", [CXXPre2BCompat]>; rjmccall wrote: > rsmith wrote: > > rjmccall wrote: > > >

[PATCH] D96092: [AST] Update LVal before evaluating lambda decl fields.

rsmith accepted this revision. rsmith added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/AST/ExprConstant.cpp:10032-10033 + +// FIXME: Diagnostics here should point to the end of the initializer +// list, not the start. +if

[PATCH] D95691: Implement P2173 for attributes on lambdas

rsmith added inline comments. Comment at: clang/include/clang/Basic/DiagnosticGroups.td:269 +def CXXPre2BCompatPedantic : + DiagGroup<"c++98-c++11-c++14-c++17-c++20-compat-pedantic", [CXXPre2BCompat]>; rjmccall wrote: > Quuxplusone wrote: > > aaron.ballman

[PATCH] D95691: Implement P2173 for attributes on lambdas

rsmith added a comment. In D95691#2540450 , @rjmccall wrote: > The patch seems technically okay to me. Do we need to recognize lambdas in > any tentative-parse situations, or is it always the reverse (e.g. recognizing > ObjC message sends as

[PATCH] D93095: Introduce -Wreserved-identifier

rsmith added inline comments. Comment at: clang/lib/AST/Decl.cpp:1087 + StringRef Name = II->getName(); + // '_' is a reserved identifier, but it's use is so common (e.g. to store + // ignored values) that we don't warn on it. Comment at:

[PATCH] D95396: Improve static_assert/_Static_assert diagnostics

rsmith added inline comments. Comment at: clang/lib/Parse/ParseDeclCXX.cpp:874-876 +if (!getLangOpts().CPlusPlus) + Diag(Tok, diag::warn_cxx_static_assert_in_c) + << FixItHint::CreateReplacement(Tok.getLocation(), "_Static_assert"); I don't

[PATCH] D95644: Ensure that we traverse non-op() method bodys of lambdas

rsmith added inline comments. Comment at: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:485 + A a; + auto l = [a] { }; + auto lCopy = l; steveire wrote: > rsmith wrote: > > steveire wrote: > > > I don't know how to create a lambda with a default

[PATCH] D95644: Ensure that we traverse non-op() method bodys of lambdas

rsmith added inline comments. Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:2065 if (const CXXRecordDecl *RD = MD->getParent()) { - if (RD->isLambda()) { + if (RD->isLambda() && RD->getLambdaCallOperator() == MD) { VisitBody = VisitBody &&

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

rsmith accepted this revision. rsmith added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Sema/SemaChecking.cpp:13384 case Stmt::MemberExprClass: { expr = cast(expr)->getBase(); break; ilya

[PATCH] D95573: [ASTMatchers] Avoid pathological traversal over nested lambdas

rsmith added inline comments. Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:2063-2065 + if (const auto *MD = dyn_cast(D)) { +if (const CXXRecordDecl *RD = MD->getParent()) { + if (RD->isLambda()) { This is incorrectly skipping the bodies of

[PATCH] D93095: Introduce -Wreserved-identifier

rsmith added inline comments. Comment at: clang/lib/Sema/Sema.cpp:2421-2428 +// Perform a lookup at TUScope. If it succeeds, we're at global scope and a +// single '_' is enough to be reserved. +LookupResult IdentifierLookup(*this, II, SourceLocation(), +

[PATCH] D95559: clang: Fix static_assert in a few contexts in microsoft mode

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Looks good, and I think we should also backport this to Clang 12. Comment at: clang/lib/Parse/ParseDecl.cpp:4219 // Parse _Static_assert declaration. -if

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

rsmith added a comment. In D91913#2526317 , @hvdijk wrote: > In D91913#2526117 , @rsmith wrote: > >> I think @rnk's observation that `__VA_OPT__` isn't actually available in any >> compilation mode other than

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

rsmith added a comment. In D91913#2526255 , @rnk wrote: > In D91913#2525993 , @hvdijk wrote: > >> @rnk Taking it upon yourself to revert this without approval and without >> communication on all branches,

[PATCH] D95487: Itanium Mangling: Fix handling of in .

rsmith accepted this revision. rsmith added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:549 void mangleInitListElements(const InitListExpr *InitList); - void mangleDeclRefExpr(const NamedDecl *D); - void mangleExpression(const Expr *E, unsigned Arity =

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

rsmith added a comment. rG0436ec2128c9775ba13b0308937238fc79673fdd enables `__VA_OPT__` across language modes and allows support for it to be detected by `#ifdef`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D94987: DR39: Perform ambiguous subobject checks for class member access as part of object argument conversion, not as part of name lookup.

rsmith added inline comments. Comment at: clang/include/clang/AST/CXXInheritance.h:77 - CXXBasePath() = default; + /// Additional data stashed on the base path by its consumers. + union { rjmccall wrote: > rsmith wrote: > > rjmccall wrote: > > > rsmith

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

rsmith added a comment. In D91913#2525993 , @hvdijk wrote: > @rnk Taking it upon yourself to revert this without approval and without > communication on all branches, especially given the earlier suggestion by > @rsmith to only revert this on the LLVM

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

rsmith accepted this revision. rsmith added a comment. Clang side looks good to me. On the LLVM side, is it intended that invalid `-binutils-version` values are silently accepted? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85474/new/

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

rsmith added a comment. In D91913#2521031 , @thakis wrote: > In D91913#2520503 , @hvdijk wrote: > >> Chrome `-std=c++*` when building with clang, but `-std=gnu++*` when building >> with GCC for exactly this

[PATCH] D94987: DR39: Perform ambiguous subobject checks for class member access as part of object argument conversion, not as part of name lookup.

rsmith added a comment. In D94987#2521774 , @rjmccall wrote: >> You can. The rule is that you resolve using-declarations to the declarations >> they name, and you resolve typedef declarations to the types they name, and >> the resulting set of

[PATCH] D94987: DR39: Perform ambiguous subobject checks for class member access as part of object argument conversion, not as part of name lookup.

rsmith added a comment. In D94987#2507481 , @rjmccall wrote: > How does this new rule work if we find overlapping but non-equal sets of > declarations in different subobjects? I'm sure you can get that with `using` > declarations. You can. The rule is

[PATCH] D85474: Add -fbinutils-version= to gate ELF features on the specified binutils version

rsmith added inline comments. Comment at: clang/docs/ReleaseNotes.rst:103-104 + For example, ``-fbinutils-version=2.35`` means compatibility with GNU as/ld + before 2.35 is not needed: new features can be used and don't work around old + GNU as/ld bugs.

[PATCH] D92808: [ObjC][ARC] Annotate calls with attributes instead of emitting retainRV or claimRV calls in the IR

rsmith added a comment. Reverted in rG925ae8c790c7e354f12ec14a6cac6aa49fc75b29 for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92808/new/ https://reviews.llvm.org/D92808

[PATCH] D92808: [ObjC][ARC] Annotate calls with attributes instead of emitting retainRV or claimRV calls in the IR

rsmith added a comment. This change violates layering by including an Analysis header from IR; I'll be landing a revert as soon as I've finished testing it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92808/new/ https://reviews.llvm.org/D92808

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Looks like this change matches the behavior of GCC all the way back to at least GCC 4.1. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D94732: [CUDA] Normalize handling of defauled dtor.

rsmith added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:15162-15170 + bool SkipDtorChecks = VD->getType()->isArrayType(); + + // CUDA: Skip destructor checks for host-only variables during device-side + // compilation + SkipDtorChecks |= +

[PATCH] D93922: Mangle `__alignof__` differently than `alignof`.

rsmith added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:4010 +if (Context.getASTContext().getLangOpts().getClangABICompat() >= +LangOptions::ClangABI::Ver11) { + Out << "u8__uuidof"; Should this be `>= Ver12` / `> Ver11` (given

[PATCH] D94732: [CUDA] Normalize handling of defauled dtor.

rsmith added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:15162-15170 + bool SkipDtorChecks = VD->getType()->isArrayType(); + + // CUDA: Skip destructor checks for host-only variables during device-side + // compilation + SkipDtorChecks |= +

[PATCH] D93095: Introduce -Wreserved-identifier

rsmith added inline comments. Comment at: clang/lib/Sema/Sema.cpp:2421-2428 +// Perform a lookup at TUScope. If it succeeds, we're at global scope and a +// single '_' is enough to be reserved. +LookupResult IdentifierLookup(*this, II, SourceLocation(), +

[PATCH] D94987: DR39: Perform ambiguous subobject checks for class member access as part of object argument conversion, not as part of name lookup.

rsmith created this revision. rsmith added a reviewer: rjmccall. rsmith requested review of this revision. Herald added a project: clang. Under C++ core issue 39, the rules for class-scope name lookup were reworked so that name lookup no longer concerns itself with whether the names were found in

[PATCH] D90448: [clang] Add type check for explicit instantiation of static data members

rsmith added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:10111-10124 +// Check the static member's type given in the explicit instantiation +// definition against the one in the class template. This won't happen in +// explicit instantiation

[PATCH] D92037: clang: Pass -platform-version to new MachO LLD

rsmith added inline comments. Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:341 // Add the deployment target. - if (Version[0] >= 520) + if (Version[0] >= 520 || LinkerIsLLDDarwinNew) MachOTC.addPlatformVersionArgs(Args, CmdArgs); Not a regression

[PATCH] D93101: [Clang][Codegen] Truncate initializers of union bitfield members

rsmith added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:9801-9804 +return EvaluateInPlace(Result.getUnionValue(), Info, Subobject, InitExpr) || + (Field->isBitField() && +truncateBitfieldValue(Info, InitExpr, Result.getUnionValue(), +

[PATCH] D77056: [Sema] Allow non-member operators for sizeless SVE types

rsmith added a comment. In D77056#2465936 , @rsandifo-arm wrote: > Either way, I realise this isn't great style. It just seems like a practical > compromise between the rock of classes having a constant size and the hard > place of vectors having a

[PATCH] D77598: Integral template argument suffix and cast printing

rsmith added inline comments. Comment at: clang/lib/AST/TemplateBase.cpp:111-115 + Out << "u8'" << Val << "'"; +else if (T->isUnsignedIntegerType() && T->isChar16Type()) + Out << "u16'" << Val << "'"; +else if (T->isUnsignedIntegerType() &&

[PATCH] D77056: [Sema] Allow non-member operators for sizeless SVE types

rsmith added a comment. The implementation looks fine to me. That said, I'm concerned that the design of this feature will severely limit its utility. For classes and enums, operators are typically defined in an associated namespace so that they can be found by ADL. For these types, there are

[PATCH] D91659: Allow enum typedefs to be referenced with the 'enum' specifier under MSVC compat mode

rsmith added a comment. In D91659#2458872 , @rnk wrote: > In D91659#2458639 , @rsmith wrote: > >> > > What is the issue with the current structure, ActOnTag produces the wrong AST > node, an ElaboratedType, but

[PATCH] D91129: Print source location in the error message when parens are missing around sizeof typename and the expression is inside macro expansion

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe53b9f733a7c: Print source location in the error message when parens are missing around… (authored by shivanshu3, committed by rsmith). Repository:

[PATCH] D91659: Allow enum typedefs to be referenced with the 'enum' specifier under MSVC compat mode

rsmith added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:15752-15754 + bool AnonymousEnumEligible = getLangOpts().MSVCCompat && + (Kind == TTK_Enum) && +

[PATCH] D91659: Allow enum typedefs to be referenced with the 'enum' specifier under MSVC compat mode

rsmith added a comment. @rnk Your thoughts on this would be appreciated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91659/new/ https://reviews.llvm.org/D91659 ___ cfe-commits mailing list

[PATCH] D91129: Print source location in the error message when parens are missing around sizeof typename and the expression is inside macro expansion

rsmith added inline comments. Comment at: clang/lib/Parse/ParseExpr.cpp:2267-2280 +SourceLocation OpTokLoc = OpTok.getLocation(); +if (OpTokLoc.isMacroID()) { + SourceLocation OpTokExpansionLoc = + PP.getSourceManager().getFileLoc(OpTokLoc);

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

rsmith added a comment. In D91488#2451755 , @teemperor wrote: > I believe rsmith is in GMT-8, so I assume this won't get a fix soon and I > went ahead and reverted in 22ccdb787024e954318e35fcf904fd4fa36f5679 >

[PATCH] D92409: [AST][NFC] Silence GCC warning about multiline comments

rsmith added a comment. In D92409#2450550 , @thopre wrote: > In D92409#2426196 , @thopre wrote: > >> Is there a way to disable it from the header? I've noticed this warning when >> compiling an application using

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7b3470baf8ba: Consider reference, pointer, and pointer-to-member TemplateArguments to be… (authored by rsmith). Repository: rG LLVM Github

[PATCH] D80977: Diagnose cases where the name of a class member is used within a class definition before the member name is declared.

rsmith updated this revision to Diff 311085. rsmith added a comment. Rebase and fix test failure. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80977/new/ https://reviews.llvm.org/D80977 Files: clang/include/clang/Basic/DiagnosticGroups.td

[PATCH] D91488: Consider reference, pointer, and pointer-to-membber TemplateArguments to be different if they have different types.

rsmith added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:4847 + /// Do we need to mangle template arguments with exactly correct types? + bool needExactType(unsigned I) const { +// We need correct types when the template-name is unresolved or when it

[PATCH] D91311: Add new 'preferred_name' attribute.

rsmith added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2384 + let Spellings = [Clang<"preferred_name", /*AllowInC*/0>]; + let Subjects = SubjectList<[ClassTmpl]>; + let Args = [TypeArgument<"TypedefType">]; jyknight wrote: > I wonder if

<    1   2   3   4   5   6   7   8   9   10   >