[PATCH] D86751: Add new warning for compound punctuation tokens that are split across macro expansions or split by whitespace.

rsmith created this revision. rsmith added a reviewer: sbenza. Herald added a project: clang. Herald added a subscriber: cfe-commits. rsmith requested review of this revision. For example: FOO({}) ... forms a statement-expression after macro expansion. This warning applies to '({' and '})'

[PATCH] D86514: Correctly parse LateParsedTemplates in case of multiple dependent modules

rsmith added inline comments. Comment at: clang/include/clang/Serialization/ASTReader.h:903-905 // A list of late parsed template function data. SmallVector LateParsedTemplates; Given the changes below, we shouldn't need this any more, and can instead

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

rsmith added a comment. Have you considered using the `ext_vector_type` attribute instead of `vector_size` for this? That would have a couple of advantages: - It's not a GCC attribute, so there's no risk that GCC would give different meaning to the same syntax (such as using one byte per

[PATCH] D79279: Add overloaded versions of builtin mem* functions

rsmith added a comment. Thanks, I'm happy with this approach. If I understand correctly, the primary (perhaps sole) purpose of `__builtin_memcpy_sized` is to provide a primitive from which an atomic operation can be produced. That being the case, I wonder if the name is emphasizing the wrong

[PATCH] D86494: Work around GCC 10 not emitting base variants of structors for final classes, by avoiding referencing the symbols in question.

rsmith created this revision. rsmith added a reviewer: rjmccall. Herald added a project: clang. Herald added a subscriber: cfe-commits. rsmith requested review of this revision. As far as I can determine, the only case we need to be careful about is when we would generate a reference to the

[PATCH] D86398: [X86] Enable constexpr on _cast fp<-> uint intrinsics (PR31446)

rsmith accepted this revision. rsmith added a comment. It would be useful to include in our documentation a brief list of which of these intrinsics can be used in constant expressions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86398/new/

[PATCH] D86342: Enable constexpr on ROTATELEFT/ROTATERIGHT builtin intrinsics (PR47249)

rsmith added a comment. (Same comments as for the bitreverse patch.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86342/new/ https://reviews.llvm.org/D86342 ___ cfe-commits mailing list

[PATCH] D86339: Enable constexpr on BITREVERSE builtin intrinsics (PR47249)

rsmith added a comment. Can you also add documentation to LanguageExtensions.rst? Thanks! Comment at: clang/docs/ReleaseNotes.rst:61 + ``__builtin_bitreverse32`` and ``__builtin_bitreverse64`` may now be used + within constexpr expressions. There's no such

[PATCH] D83929: [clang]: Remove assertion which checks explicit declaration

This revision was automatically updated to reflect the committed changes. Closed by commit rGfe86dbb32da2: [clang]: Remove assertion which checks explicit declaration (authored by gousemoodhin, committed by rsmith). Herald added a project: clang. Herald added a subscriber: cfe-commits.

[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

rsmith accepted this revision. rsmith added a comment. This looks OK to me. I think we'll also need updates for `_ExtInt` now too -- we have more fundamental types with padding than only `long double` and `bool` now. (I continue to be displeased that we assume 8-bit `char` in

[PATCH] D86130: [AST] Fix a crash on mangling a binding decl from a DeclRefExpr.

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Here's a testcase that produces a name with external linkage: struct X { int i, j; }; auto [a,b] = X{1,2}; template void f(decltype(a + T())) {} template void f(int); We should make

[PATCH] D85601: Fixes an assertion when IntRange processes a throw expression

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85601/new/ https://reviews.llvm.org/D85601 ___ cfe-commits mailing list

[PATCH] D85710: Code generation support for lvalue bit-field conditionals.

rsmith added a comment. In D85710#2212174 , @rjmccall wrote: > I always imagined that we'd implement this by just inverting the emission: > emit the RHS of the assignment and then pass a callback down to a function > that descended into conditional

[PATCH] D85256: Add -Wtautological-value-range-compare warning.

rsmith added a comment. In D85256#2211129 , @rsmith wrote: > This appears to be a general problem: the `GetExprRange` mechanism in > SemaChecking miscomputes the ranges for `+`, `*`, and `-` expressions, and > we'll get them wrong for all warnings that

[PATCH] D85778: More accurately compute the ranges of possible values for +, -, *, &, %.

rsmith created this revision. rsmith added reviewers: sberg, rtrieu. Herald added a project: clang. Herald added a subscriber: cfe-commits. rsmith requested review of this revision. Continue to heuristically pick the wider of the two operands for narrowing conversion warnings so that some_char +

[PATCH] D85256: Add -Wtautological-value-range-compare warning.

rsmith added a comment. In D85256#2209211 , @sberg wrote: > I think this generates a false positive with `test.cc` > > enum E { E1 = 1, E2 = 2 }; > bool f(E e) { return ((e & E1) ? 1 : 0) + ((e & E2) ? 1 : 0) > 1; } > > and `clang++ -fsyntax-only

[PATCH] D85710: Code generation support for lvalue bit-field conditionals.

rsmith updated this revision to Diff 284594. rsmith added a comment. - Remove development aid from test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85710/new/ https://reviews.llvm.org/D85710 Files: clang/lib/CodeGen/CGAtomic.cpp

[PATCH] D85710: Code generation support for lvalue bit-field conditionals.

rsmith created this revision. rsmith added a reviewer: rjmccall. Herald added subscribers: cfe-commits, jfb. Herald added a project: clang. rsmith requested review of this revision. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. C++ permits use of (cond ? x.a : y.b) as

[PATCH] D85696: [AST] add parenthesis locations for IfStmt and SwitchStmt

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Only relatively trivial comments; please feel free to commit once they're addressed. Comment at: clang/include/clang/Sema/Sema.h:4382-4393 + StmtResult

[PATCH] D85601: Fixes an assertion when IntRange processes a throw expression

rsmith added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:10164 const BuiltinType *BT = cast(T); -assert(BT->isInteger()); +if (!BT->isInteger()) { + // This can happen in a conditional expression with a throw statement Mordante

[PATCH] D85699: PR47099: Split out of .

rsmith created this revision. rsmith added reviewers: hans, BillyONeal. Herald added subscribers: cfe-commits, jfb, mgorny. Herald added a project: clang. rsmith requested review of this revision. This aims to be compatible with Visual C++'s , but not exactly match it. The contents of are a

[PATCH] D85612: [Sema] Use proper integral cast for an enumerate with a fixed bool type

rsmith added inline comments. Comment at: clang/test/AST/ast-dump-enum-bool.cpp:1 +// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown -fsyntax-only -ast-dump %s | FileCheck %s + I don't think we really need a dedicated AST test for this. Such tests

[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.

rsmith added inline comments. Comment at: clang/include/clang/AST/DeclCXX.h:3843 /// The declaration that this binding binds to part of. + // FIXME: Currently not set during deserialization of the BindingDecl; + // only set when the corresponding DecompositionDecl is

[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.

rsmith added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:101-105 +if (const VarDecl *HoldingVar = Binding->getHoldingVar()) { + // C++20 [dcl.struct.bind]p4: + // Each vi is the name [...] that refers to the object bound to ri [...] + Decl =

[PATCH] D85612: [Sema] Use proper integral cast for an enumerate with a fixed bool type

rsmith added inline comments. Comment at: clang/test/AST/ast-dump-enum-bool.cpp:1 +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -ast-dump=json %s | FileCheck %s + aaron.ballman wrote: > riccibruno wrote: > > Why a (pretty unreadable) json

[PATCH] D85601: Fixes an assertion when IntRange processes a throw expression

rsmith added a comment. Did this only crash during error recovery before, or also for your `void g()` example? If we were only crashing in error recovery, that would suggest that error recovery was producing a bad AST, and perhaps we should be fixing this elsewhere. Comment

[PATCH] D85473: [Clang] Add option to allow marking pass-by-value args as noalias.

rsmith added inline comments. Comment at: clang/include/clang/Basic/LangOptions.def:371-374 +LANGOPT(PassByValueNoAlias, 1, 0, "Allows assuming no references to passed by " + "value escape before transferring execution " +

[PATCH] D79279: Add overloaded versions of builtin mem* functions

rsmith added a comment. In D79279#2201136 , @rjmccall wrote: > In D79279#2200916 , @rsmith wrote: > >> In D79279#2197176 , @rjmccall wrote: >> >>> I thought part of the

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

rsmith added a comment. In D85191#2199574 , @ebevhan wrote: > In D85191#2197663 , @efriedma wrote: > >>> If the intent is for getTypeInfo to always return sizes that are multiples >>> of the char size, then the

[PATCH] D85025: [RecoveryExpr]WIP: Support dependence in C-only codepath

rsmith added a comment. Looks reasonable to me. I expect you'll find more places that need to learn how to handle dependent types in C, but this looks like a solid start. Comment at: clang/lib/AST/Expr.cpp:3757-3758 case NPC_ValueDependentIsNull: - if

[PATCH] D85256: Add -Wtautological-value-range-compare warning.

This revision was automatically updated to reflect the committed changes. Closed by commit rGd6492d874478: Add -Wtautological-value-range-compare warning. (authored by rsmith). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85256/new/

[PATCH] D79279: Add overloaded versions of builtin mem* functions

rsmith added a comment. In D79279#2197176 , @rjmccall wrote: > I thought part of the point of `__builtin_memcpy` was so that C library > headers could do `#define memcpy(x, y, z) __builtin_memcpy(x, y, z)`. If so, > the conformance issue touches

[PATCH] D85097: [Sema] add warning for comparisons like 'x<=y<=z'

rsmith added a comment. The diagnostic we get for the case of three or more comparison operators here doesn't seem ideal. Perhaps we could do this check as part of the `SemaChecking` pass over the completed expression rather than doing it as we form the individual comparisons? That way we'll

[PATCH] D79279: Add overloaded versions of builtin mem* functions

rsmith added a comment. Two observations that are new to me in this review: 1. We already treat all builtins as being overloaded on address space. 2. The revised patch treats `__builtin_mem*_overloaded` as being overloaded *only* on address space, volatility, and atomicity. (We've tuned the

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

rsmith added a comment. In D85191#2195923 , @ebevhan wrote: > In D85191#2193645 , @rsmith wrote: > >>> This is not ideal, since it makes the calculations char size dependent, and >>> breaks for sizes that are not

[PATCH] D79279: Add overloaded versions of builtin mem* functions

rsmith added a comment. Thanks. I'd like @rjmccall to approve too, but the design of these intrinsics and the Sema and constant evaluation parts seem fine. (We don't strictly need constant evaluation to abort on library UB, so I think not catching the misalignment case is OK.)

[PATCH] D85256: Add -Wtautological-value-range-compare warning.

rsmith added a comment. In D85256#2194892 , @xbolva00 wrote: > Could this solve https://bugs.llvm.org/show_bug.cgi?id=40921 ? Not directly; `GetExprRange` in `SemaChecking` apparently intentionally treats casts as producing an arbitrary value of the

[PATCH] D85256: Add -Wtautological-value-range-compare warning.

rsmith updated this revision to Diff 283072. rsmith added a comment. N/A Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85256/new/ https://reviews.llvm.org/D85256 Files: clang/include/clang/Basic/DiagnosticGroups.td

[PATCH] D85256: Add -Wtautological-value-range-compare warning.

rsmith updated this revision to Diff 283070. rsmith added a comment. Unbreak test that was autobroken by lint. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85256/new/ https://reviews.llvm.org/D85256 Files:

[PATCH] D85256: Add -Wtautological-value-range-compare warning.

rsmith created this revision. rsmith added a reviewer: rtrieu. Herald added a project: clang. Herald added a subscriber: cfe-commits. rsmith requested review of this revision. This warning diagnoses cases where an expression is compared to a constant, and the comparison is tautological due to the

[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

rsmith added a comment. > This is not ideal, since it makes the calculations char size dependent, and breaks for sizes that are not a multiple of the char size. How can we have a non-bitfield member whose size is not a multiple of the size of a char? Repository: rG LLVM Github Monorepo

[PATCH] D79279: Add overloaded versions of builtin mem* functions

rsmith added inline comments. Comment at: clang/docs/LanguageExtensions.rst:2430 + +These overloads support destinations and sources which are a mix of the +following qualifiers: Comment at: clang/docs/LanguageExtensions.rst:2454 +and might

[PATCH] D84637: [AST] Enhance the const expression evaluator to support error-dependent exprs.

rsmith added a comment. In D84637#2175681 , @erichkeane wrote: > Based on richard's suggestions, that seems about right. However I believe > all the asserts should have a message to go along with them as we typicaly do. In this case, the asserts are

[PATCH] D84637: [AST] Enhance the const expression evaluator to support error-dependent exprs.

rsmith added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:4876 +assert(E->containsErrors()); +return ESR_Failed; + } This should not result in failure when checking for a potential constant expression; if an expression

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Thanks, looks nice =) Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:1205 +/// Attempt to deduce the template arguments by checking the base types +/// according to

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Looks good with suggestions applied, and with the portability problems in the test fixed. (Maybe just add a `-triple`? Though it would be good to also test `inalloca` and ARM constructor

[PATCH] D79279: Add overloaded versions of builtin mem* functions

rsmith added inline comments. Comment at: clang/docs/LanguageExtensions.rst:2435-2437 +* ``__builtin_memcpy_overloaded(QUAL void *dst, QUAL const void *src, size_t byte_size, size_t byte_element_size = )`` +* ``__builtin_memmove_overloaded(QUAL void *dst, QUAL const void *src,

[PATCH] D80925: Fix compiler crash when an expression parsed in the tentative parsing and must be claimed in the another evaluation context.

rsmith added a comment. In D80925#2177446 , @rsmith wrote: > I'm going to try to fix this a different way, by fixing the bad case in > `Sema::ClassifyName` instead. Done in llvmorg-12-init-1234-g23d6525cbdc. Repository: rG LLVM Github Monorepo

[PATCH] D80925: Fix compiler crash when an expression parsed in the tentative parsing and must be claimed in the another evaluation context.

rsmith added a comment. This is at best only a partial fix. `Sema::NC_ContextIndependentExpr` is supposed to be used (unsurprisingly) only if we form a context-independent annotation, but here we're forming a context-dependent expression that depends on whether it appears in an unevaluated

[PATCH] D79279: Add overloaded versions of builtin mem* functions

rsmith added a comment. These new builtins should ideally support constant evaluation if possible. Comment at: clang/docs/LanguageExtensions.rst:2439-2440 +Clang provides versions of the following functions which are overloaded based on +the pointer parameter types: +

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

rsmith added inline comments. Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:1281 + "Base class that isn't a record?"); + ToVisit.push_back(Base.getType()->getAs()); +} erichkeane wrote: > rsmith wrote: > > It would be better to add

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

rsmith added inline comments. Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:1204-1205 +// Attempt to deduce the template arguments by checking the base types according +// to (C++ [temp.deduct.call] p4b3. +/// Missing `///` Comment

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

rsmith added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2110 + DetermineNoUndef(RetTy, getTypes(), DL, RetAI)) { +RetAttrs.addAttribute(llvm::Attribute::NoUndef); + } rsmith wrote: > This only applies in C++. In C, it's valid for a

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

rsmith requested changes to this revision. rsmith added a comment. This revision now requires changes to proceed. I'd like to see some testcases for the C++ side of this. The following things all seem like they might be interesting: passing and returning classes and unions, especially ones that

[PATCH] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

rsmith added a comment. In D84048#2164950 , @erichkeane wrote: > Additionally, I sent out a CWG message on the reflector about this: > https://godbolt.org/z/bEr61T > > My implementation has this as ambiguous, but the wording makes it well-formed > I

[PATCH] D63640: [clang] Improve Serialization/Imporing/Dumping of APValues

rsmith added a comment. Are we at a point where we can test this now? Perhaps by adding an assert in codegen that we always have an evaluated value for any `constexpr` variable that we emit? Comment at: clang/lib/Sema/SemaDecl.cpp:11883-11885 ExprResult Result =

[PATCH] D83647: Don't allow mangling substitutions to refer to unrelated entities from different s.

rsmith added a comment. Hm, I think this is not quite right. For example, given: template struct X {}; template auto f(T a, decltype(a)) { struct A {}; struct B {}; return X(); } decltype(f(0, 0)) g() {} ... I think we won't use a substitution from the first parameter of `f`

[PATCH] D83647: Don't allow mangling substitutions to refer to unrelated entities from different s.

rsmith created this revision. rsmith added a reviewer: rjmccall. Herald added a project: clang. Herald added a subscriber: cfe-commits. Per the ABI rules, substitutable components are symbolic constructs, not the associated mangling character strings, so references to function parameters or

[PATCH] D83174: Teach AttachPreviousImpl to inherit MSInheritanceAttr attribute

rsmith added a comment. @aaron.ballman We will need to do something like this in general, but I'm not sure it's going to be as easy as just inheriting the attribute in the general case. What do you think? Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:3540 +Attr

[PATCH] D83263: [WIP] Clang crashed while checking for deletion of copy and move ctors

rsmith added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:9733 return true; return false; }; This is not the right fallback answer if the class is a dependent type -- we don't yet know if there's a non-deleted copy or move

[PATCH] D82921: Removed a RecursiveASTVisitor feature to visit operator kinds with different methods

rsmith accepted this revision. rsmith added a comment. This introduces a non-uniformity between `RecursiveASTVisitor` and `StmtVisitor`. `StmtVisitor` contains matching logic to treat the various kinds of unary and binary operators as if they were distinct AST node kinds, and that

[PATCH] D81678: Introduce frozen attribute at call sites for stricter poison analysis

rsmith added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2082 + const Type *RetTyPtr = RetTy.getTypePtr(); + if (!RetTy->isVoidType() && !RetTyPtr->isRecordType() && + RetAI.getKind() != ABIArgInfo::Indirect) { Other types that we should

[PATCH] D80981: [AST] Fix a crash on accessing a class without definition in constexpr function context.

rsmith accepted this revision. rsmith marked an inline comment as done. rsmith added a comment. This revision is now accepted and ready to land. Minor suggestions but this LGTM. Comment at: clang/lib/AST/ExprConstant.cpp:4320 +if (!RD->hasDefinition()) + return

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

rsmith added a comment. In D77598#2064193 , @rsmith wrote: > In D77598#1990197 , @rsmith wrote: > > > Is is feasible to check whether the corresponding template parameter either > > has a deduced type or is a

[PATCH] D82314: [RFC][Coroutines] Optimize the lifespan of temporary co_await object

rsmith added a comment. In D82314#2109728 , @lxfind wrote: > @rsmith Thanks. That's a good point. Do you know if there already exists > optimization passes in LLVM that attempts to shrink the range of lifetime > intrinsics? If so, I am curious why that

[PATCH] D82314: [RFC][Coroutines] Optimize the lifespan of temporary co_await object

rsmith added a comment. In D82314#2109437 , @lxfind wrote: > In D82314#2107910 , @junparser wrote: > > > Rather than doing it here, can we build await_resume call expression with > > MaterializedTemporaryExpr when

[PATCH] D80981: [AST] Fix a crash on accessing a class without definition in constexpr function context.

rsmith added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:4320 +if (!RD->hasDefinition()) + return APValue(); APValue Struct(APValue::UninitStruct(), RD->getNumBases(), hokein wrote: > rsmith wrote: > > hokein wrote: > > > sammccall

[PATCH] D81003: [clang] SequenceChecker: Also visit default arguments.

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Seems reasonable. I think we need similar handling for `CXXDefaultInitExpr`, for cases like this: int a; struct X { int b = ++a; }; int c = X{} + a; Comment at:

[PATCH] D81330: [clang] SequenceChecker: C++17 sequencing rule for overloaded operators.

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. It would be nice to avoid the duplication between the builtin and overloaded cases here, but I don't see a good way to do that. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D80981: [AST] Fix a crash on accessing a class without definition in constexpr function context.

rsmith added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:4320 +if (!RD->hasDefinition()) + return APValue(); APValue Struct(APValue::UninitStruct(), RD->getNumBases(), hokein wrote: > sammccall wrote: > > This doesn't look all that

[PATCH] D81392: [clang] Rename Decl::isHidden() to isUnconditionallyVisible()

rsmith added a comment. In D81392#2089251 , @mboehme wrote: > In D81392#2088131 , @rsmith wrote: > > > Maybe we should dump the ModuleOwnershipKind in general, not only an > > indicator of whether it's Visible or

[PATCH] D81678: Introduce partialinit attribute at call sites for stricter poison analysis

rsmith added a comment. It's not entirely clear to me what `partialinit` means. Does it mean that some bytes of the parameter might be undef / poison? Or does it mean that some bytes of the parameter that (for a struct parameter or array thereof) correspond to a struct member might be undef /

[PATCH] D80743: (PR46111) Properly handle elaborated types in an implicit deduction guide

rsmith accepted this revision. rsmith added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Sema/SemaTemplate.cpp:1967 +TransformType(InnerTLB, OrigDecl->getTypeSourceInfo()->getTypeLoc()); +TypeSourceInfo *TSI =

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

rsmith added a comment. I'm not entirely happy with this approach; there are lots of places where we perform lookups that are incidental and shouldn't result in an error if we end up looking outside the class, and I'm not sure I've marked them all as "synthetic". That said, I've tested this

[PATCH] D81392: [clang] Rename Decl::isHidden() to isUnconditionallyVisible()

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. I think renaming the flag in the AST dump output would be a good idea, though it'll be a lot of churn in the tests. I would prefer that we continue to dump a marker only if the declaration is

[PATCH] D81615: [clang] CWG 2082 and 2346: loosen the restrictions on parameters and local variables in default arguments.

rsmith added inline comments. Comment at: clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p9.cpp:8 + // expected-error@-1 {{default argument references parameter 'x'}} + void f4(int x, int y = x + 0); + // expected-error@-1 {{default argument references parameter 'x'}}

[PATCH] D77194: [clang] Persist Attr::IsPackExpansion into the PCH

rsmith added a comment. In D77194#2086245 , @sammccall wrote: > @rsmith are you OK with merging this into the release branch for 10.0.1? > cc @tstellar Yes, please do. (I would like for us to eventually maintain AST file format compatibility between

[PATCH] D81615: [clang] CWG 2082 and 2346: loosen the restrictions on parameters and local variables in default arguments.

rsmith accepted this revision. rsmith marked 2 inline comments as done. rsmith added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:98 +// +if (DRE->isNonOdrUse() != NOUR_Unevaluated) + return

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

rsmith added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6451 + "%1 is %select{|in}3complete">, + InGroup; def ext_typecheck_ordered_comparison_of_function_pointers : ExtWarn< pestctrl wrote: > efriedma wrote: > > `InGroup` >

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

rsmith added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:11571 + Diag(Loc, + getLangOpts().C11 + ? diag::ext_typecheck_compare_complete_incomplete_pointers efriedma wrote: > pestctrl wrote: > > efriedma

[PATCH] D79945: [Sema] Comparison of pointers to complete and incomplete types

rsmith added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:11571 + Diag(Loc, + getLangOpts().C11 + ? diag::ext_typecheck_compare_complete_incomplete_pointers pestctrl wrote: > efriedma wrote: > > I think this

[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

rsmith added a comment. Herald added a subscriber: ributzka. Are you still interested in pursuing this? This change looks good to me if so. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57626/new/ https://reviews.llvm.org/D57626

[PATCH] D79378: PR34581: Don't remove an 'if (p)' guarding a call to 'operator delete(p)' under -Oz.

This revision was automatically updated to reflect the committed changes. Closed by commit rGf39e12a06b60: PR34581: Dont remove an if (p) guarding a call to operator delete(p) under… (authored by rsmith). Changed prior to commit: https://reviews.llvm.org/D79378?vs=261963=268971#toc

[PATCH] D81313: Fix handling of constinit thread_locals with a forward-declared type.

rsmith added a comment. Your FIXME is a concern. I think it would be preferable for this function to require as a precondition that the type is complete. If the only call where that isn't true is the call from `ItaniumCXXABI::usesThreadWrapperFunction`, it seems reasonable to perform a

[PATCH] D79378: PR34581: Don't remove an 'if (p)' guarding a call to 'operator delete(p)' under -Oz.

rsmith added a comment. In D79378#2058753 , @dnsampaio wrote: > Hi @rsmith, > are you still looking into this? > cheers Sorry for the delay, I'll be getting back to this soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D79830: Add support of __builtin_expect_with_probability

rsmith added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10804-10807 + "probability of __builtin_expect_with_probability must be constant " + "floating-point expression">; +def err_probability_out_of_range : Error< + "probability of

[PATCH] D80743: (PR46111) Desugar Elaborated types in Deduction Guides.

rsmith added a comment. In D80743#2074121 , @erichkeane wrote: > @rsmith I think this implements what you've suggested? Yes, thanks. > This seems to 'work' for a small subset of works, but it doesn't properly > register the typedef to the

[PATCH] D80743: (PR46111) Desugar Elaborated types in Deduction Guides.

rsmith added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:1962-1964 + QualType TransformElaboratedType(TypeLocBuilder , ElaboratedTypeLoc TL) { +return TransformType(TLB, TL.getNamedTypeLoc()); + } Removing the qualifier seems like it might

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

rsmith added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2031-2033 + "%select{|member |static member }0%2" + "%select{| of %3| of struct %4| of union %4| of class %4" + "| of structured binding %3}1">, InGroup; I wonder if we

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

rsmith created this revision. rsmith added a reviewer: rnk. Herald added a project: clang. This is ill-formed per [basic.scope.class]p2. This is done by tracking all unqualified lookups performed within the scope of a class definition that find nothing within the class, and checking for

[PATCH] D68049: Propeller: Clang options for basic block sections

rsmith added a comment. Clang side of these changes LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68049/new/ https://reviews.llvm.org/D68049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D80836: Support GCC [[gnu::attributes]] in C2x mode

rsmith added inline comments. Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:87 Ret.emplace_back("CXX11", std::string(Name), "gnu", true); + if (Spelling->getValueAsBit("AllowInC")) +Ret.emplace_back("C2x", std::string(Name), "gnu", true);

[PATCH] D77598: Integral template argument suffix printing

rsmith added a comment. In D77598#1990197 , @rsmith wrote: > Is is feasible to check whether the corresponding template parameter either > has a deduced type or is a parameter of a function template? If not, we don't > need to clarify the type of the

[PATCH] D79800: [Sema] Implement DR2233

rsmith added a comment. Unfortunately, I think this approach basically can't work, because we need to consider inheriting default arguments from a previous (or subsequent!) declaration before dropping default arguments preceding a pack. I think we will instead need to detect this situation

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

rsmith added a comment. In D69764#2058334 , @MyDeveloperDay wrote: > @rsmith, Thank you for your comments, I don't agree, but thank you anyway. > > I will continue to feel there is some value here, My hope is that others will > feel the same. To be

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

rsmith added a comment. In D69764#2057945 , @aaron.ballman wrote: > In D69764#2056104 , @rsmith wrote: > > > I also don't think this is something that a user would *want* in > > `clang-format`: changing the

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

rsmith added a comment. I'm uncomfortable about `clang-format` performing this transformation at all. Generally, clang-format only makes changes that are guaranteed to preserve the meaning of the source program, and does not make changes that are only heuristically likely to be

[PATCH] D80055: Diagnose union tail padding

rsmith added a comment. In D80055#2055151 , @jfb wrote: > I was wondering if any of the tests were surprising to you, or if the > behavior described was as expected? I've highlighted one case where the test expectation doesn't match the standard

[PATCH] D76420: Prevent IR-gen from emitting consteval declarations

rsmith accepted this revision. rsmith marked an inline comment as done. rsmith added a comment. This revision is now accepted and ready to land. Thanks! Looks good. I'd like to eventually get to a point where every `ConstantExpr` that reaches code generation has `hasAPValueResult()` return

[PATCH] D80288: [AST] default implementation is possible for non-member functions in C++2a.

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Looks good subject to some comment tweaks. Comment at: clang/include/clang/AST/Decl.h:2128 - /// Whether this function is defaulted per C++0x. Only valid for - ///

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