[PATCH] D66361: Improve behavior in the case of stack exhaustion.

rsmith updated this revision to Diff 216520. rsmith marked 5 inline comments as done. rsmith added a comment. - Address review comments from Aaron. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66361/new/ https://reviews.llvm.org/D66361 Files:

[PATCH] D66361: Improve behavior in the case of stack exhaustion.

rsmith added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:14 let Component = "Sema" in { -let CategoryName = "Semantic Issue" in { +def warn_stack_exhausted : Warning< + "stack nearly exhausted; compilation time may suffer, and "

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

rsmith added a comment. The `ALPHA_OFFSET` code seems to be unnecessarily "clever" to me. I think the warning can be suppressed by reversing the operands: `ALPHA_OFFSET ^ 2` But if I were maintaining that code I would get rid of the xor hack entirely and use #define CHANNEL_OFFSET(i) (i)

[PATCH] D66364: Diagnose use of _Thread_local as an extension when appropriate

rsmith added a comment. In D66364#1633981 , @aaron.ballman wrote: > My motivation is for portability. _Thread_local (and all the rest) do not > exist in C99 or earlier (or C++), so having some way to warn users of that is > useful. I agree that we

[PATCH] D66364: Diagnose use of _Thread_local as an extension when appropriate

rsmith added a comment. `_Thread_local` is a reserved identifier; we generally don't produce extension warnings for uses of reserved identifiers. (Eg, there's no warning for `_Atomic` in C++ or `_Bool` in C89, and no warning for uses of `__type_traits` or `__builtins`.) But I note that we

[PATCH] D66361: Improve behavior in the case of stack exhaustion.

rsmith added inline comments. Comment at: lib/Basic/Stack.cpp:53-54 + // If the stack pointer has a surprising value, we do not understand this + // stack usage scheme. (Perhaps the target allocates new stack regions on + // demand for us.) Don't try to guess what's going on.

[PATCH] D66361: Improve behavior in the case of stack exhaustion.

rsmith updated this revision to Diff 215680. rsmith added a comment. - Disable stack exhaustion test if threads are disabled. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66361/new/ https://reviews.llvm.org/D66361 Files:

[PATCH] D66361: Improve behavior in the case of stack exhaustion.

rsmith updated this revision to Diff 215678. rsmith marked 2 inline comments as done. rsmith added a comment. - Review feedback: use _AddressOfReturnAddress with MSVC, and simplify Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66361/new/

[PATCH] D66361: Improve behavior in the case of stack exhaustion.

rsmith created this revision. rsmith added reviewers: rnk, aaron.ballman. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. Clang performs various recursive operations (such as template instantiation), and may use non-trivial amounts of stack space in each recursive

[PATCH] D63889: Check possible warnings on global initializers for reachability

rsmith added inline comments. Comment at: clang/include/clang/Sema/Sema.h:1684-1686 + void pushDeclForInitializer(Decl *D) { DeclForInitializer.push_back(D); } + + void popDeclForInitializer() { DeclForInitializer.pop_back(); } I don't think a simple list of

[PATCH] D65695: Implements CWG 1601 in [over.ics.rank/4.2]

rsmith added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:3758 +static QualType +getFixedEnumUnderlayingType(const StandardConversionSequence ) { + Underlaying -> Underlying (here and below) Comment at:

[PATCH] D66040: [Sema] Implement DR2386 for C++17 structured binding

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. LGTM with minor adjustments to the test. Comment at: clang/test/CXX/drs/dr23xx.cpp:43-57 +#if __cplusplus >= 201707L +// Otherwise, if the qualified-id std::tuple_size names

[PATCH] D66100: Add __has_builtin support for builtin function-like type traits.

This revision was automatically updated to reflect the committed changes. Closed by commit rL368785: Add __has_builtin support for builtin function-like type traits. (authored by rsmith, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

rsmith added a comment. I would like to better understand the big picture for descriptors, pointers, and so on. I'm not yet seeing how the pieces are going to fit together and not frequently require expensive operations. For example, pointer arithmetic requires determining the array bound of

[PATCH] D66100: Add __has_builtin support for builtin function-like type traits.

rsmith created this revision. rsmith added a reviewer: aaron.ballman. Herald added subscribers: cfe-commits, kristina, jfb. Herald added a project: clang. Previously __has_builtin(__builtin_*) would return false for __builtin_*s that we modeled as keywords rather than as functions (because they

[PATCH] D66040: [Sema] Implement DR2386 for C++17 structured binding

rsmith added a comment. Looks good. Can you add a test to `test/CXX/drs/dr23xx.cpp` with a suitable `dr2386: 10` comment to track that we've implemented DR2386 in Clang 10, please? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66040/new/

[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:103-105 + // A local variable cannot be odr-used (6.2) in a default argument. + if (DRE->isNonOdrUse() != NOUR_None) +return false; Please add tests for the distinction

[PATCH] D65192: [Sema] Disable some enabled-by-default -Wparentheses diagnostics

rsmith added a comment. Can you reduce this patch to only the `&&` within `||` and `&` within `|` changes? I think we have reasonable consensus that that should not be enabled by default, so let's land that part now. If you want to continue discussing `-Wshift-op-parentheses` after that, we

[PATCH] D62648: [Sema][Typo] Fix assertion failure for expressions with multiple typos

rsmith added a comment. I don't think you need to change the `TreeTranform` base class to support this; `TreeTransform::TransformExpr` is an extension point which you can override from `TransformTypos` to inject the custom logic you need. But I also don't think this

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

rsmith added a comment. In D64799#1605211 , @ilya-biryukov wrote: > @rsmith, emitting the typos on pop expression evaluation context resulted in > a bunch of failures when trying to transform the resulting errors, see a > typical stacktrace below. >

[PATCH] D65192: [Sema] Disable some enabled-by-default -Wparentheses diagnostics

rsmith added a comment. For the `&&` vs `||` and `&` vs `|` warnings, it seems extremely implausible to me that they meet the "few or no false-positives" criterion for an enabled-by-default warning. Even assuming that people don't know the relative precedences of those operators, we'd expect a

[PATCH] D64146: [ConstExprPreter][WIP] Initial patch for the constexpr interpreter

rsmith added a comment. In D64146#1604717 , @nand wrote: > > How do you intend to represent pointers cast to integer types? Allocating > > 64 bits of state for a 64-bit integer is insufficient to model that case. > > Is this ever going to be allowed in

[PATCH] D64146: [ConstExprPreter][WIP] Initial patch for the constexpr interpreter

rsmith added a comment. How do you intend to represent pointers cast to integer types? Allocating 64 bits of state for a 64-bit integer is insufficient to model that case. Comment at: clang/include/clang/AST/ASTContext.h:583-584 + /// Returns the constexpr VM context. +

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

rsmith added a comment. In D64838#1602840 , @Nathan-Huckleberry wrote: > I agree that parsing according to attribute name/type is not a good solution. > > It sounds like we have narrowed it down to two choices: > Do we want to follow the gcc method of

[PATCH] D62648: [Sema][Typo] Fix assertion failure for expressions with multiple typos

rsmith added a comment. Thanks, LGTM. Do you need someone to commit this for you? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62648/new/ https://reviews.llvm.org/D62648 ___ cfe-commits mailing list

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

rsmith added a comment. In D64799#1592263 , @rnk wrote: > In D64799#1591732 , @ilya-biryukov > wrote: > > > @rsmith, I'll look into emitting the typos when we pop expression > > evaluation context, but do we

[PATCH] D63889: Check possible warnings on global initializers for reachability

rsmith added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:352 SetParamDefaultArgument(Param, DefaultArg, EqualLoc); + CurContext->removeDecl(Param); + CurContext = Cur; We may need to delay the diagnostics here until the default argument is

[PATCH] D64914: Implement P1771

rsmith added a comment. Please update cxx_status.html to mark P1771R1 as implemented in SVN. Comment at: clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp:87-88 // expected-warning@28 {{use of the 'nodiscard' attribute is a C++17 extension}} +// expected-warning@66

[PATCH] D65022: [Sema] Always instantiate the initializer of a variable template with undeduced type (8.0 regression)

rsmith added a comment. Well, this restores an incorrect behaviour: we're not permitted to substitute into the initializer of the variable template here. Can you look into fixing this properly, by instantiating the type when forming the MemberExpr? If not, taking this to fix the regression

[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

rsmith added a comment. In D64799#1589557 , @ilya-biryukov wrote: > I tried to find a good place to emit unresolved typos earlier (to make sure > CodeGen does not ever get `TypoExpr`), but couldn't find one. > Please let me know if there's some obvious

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

rsmith added a comment. I don't think this problem really has anything to do with statement expressions; consider: struct Widget a = (init2(), a); ... which has the same behaviour and presumably produces the same warning. It's just a C / C++ difference. In C++, these examples are undefined

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

rsmith added inline comments. Comment at: clang/lib/Parse/ParseTentative.cpp:2130 + ParsedAttributesWithRange attrs(AttrFactory); + ParseGNUAttributes(attrs, nullptr, nullptr); + if (attrs.size() <= 0) { It's not correct in general to call arbitrary parsing

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

rsmith added inline comments. Comment at: clang/test/CodeGenCXX/pr40771-ctad-with-lambda-copy-capture.cpp:1 +// RUN: %clang_cc1 -emit-obj --std=c++17 -fcxx-exceptions -fexceptions %s +// PR40771: check that this input does not crash or assert This will run the

[PATCH] D64448: gsl::Owner/gsl::Pointer: Add implicit annotations for some std types

rsmith added subscribers: aaron.ballman, rsmith. rsmith added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2773 + let Args = [TypeArgument<"DerefType", /*opt=*/1>]; + let MeaningfulToClassTemplateDefinition = 1; let Documentation = [LifetimeOwnerDocs];

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

rsmith added inline comments. Comment at: clang/test/CodeGenCXX/pr40771-ctad-with-lambda-copy-capture.cpp:4-24 +struct outer +{ +struct inner +{ +~inner() {} +}; + Please use a more minimal testcase, such as: ``` struct P { ~P(); }; struct Q

[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

rsmith added inline comments. Comment at: clang/test/Frontend/rewrite-includes-conditions.c:17 +line4 +#elif value2 < value2 +line5 Did you mean for this to be `value1 < value2` rather than `value2 < value2`? Comment at:

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

rsmith added a comment. C++20 designated initializers don't permit mixing designated fields with non-designated ones, so some of the examples here are invalid. However, I think we should be looking for an attribute design that works well in both C and C++, and with the various Clang extensions

[PATCH] D50763: [Parser] Refactor and fix bugs in late-parsing

rsmith added inline comments. Herald added a project: clang. Comment at: lib/Parse/ParseCXXInlineMethods.cpp:266 +public: + UseCachedTokensRAII(Parser , CachedTokens , const void *Data) + : Self(Self), Data(Data) { Can you pass ownership of the cached

[PATCH] D64317: [Driver] Add float-divide-by-zero back to supported sanitizers after D63793/rC365272

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

[PATCH] D63894: [CXX] Exercise all paths through these tests

rsmith accepted this revision. rsmith added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/SemaCXX/linkage2.cpp:3 +// RUN: %clang_cc1 -fsyntax-only -verify -Wno-c++11-extensions -Wno-local-type-template-args %s -std=gnu++98 // RUN:

[PATCH] D64058: [cxx2a] P0624R2 fix: only lambdas with no lambda-capture are default-constructible and assignable.

rsmith accepted this revision. rsmith added a comment. Do you have commit access or do you need someone to commit this for you? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64058/new/ https://reviews.llvm.org/D64058

[PATCH] D64317: [Driver] Add float-divide-by-zero back to supported sanitizers after D63793/rC365272

rsmith added inline comments. Comment at: lib/Driver/SanitizerArgs.cpp:27-30 static const SanitizerMask NeedsUbsanRt = SanitizerKind::Undefined | SanitizerKind::Integer | SanitizerKind::ImplicitConversion | SanitizerKind::Nullability | +SanitizerKind::CFI |

[PATCH] D63793: Treat the range of representable values of floating-point types as [-inf, +inf] not as [-max, +max].

This revision was automatically updated to reflect the committed changes. Closed by commit rL365272: Treat the range of representable values of floating-point types as [-inf, +inf]… (authored by rsmith, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits.

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. LG with a few tweaks. Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:228-230 +def note_constexpr_bit_cast_indet_dest : Note< + "indeterminate value can only

[PATCH] D62648: [Sema][Typo] Fix assertion failure for expressions with multiple typos

rsmith accepted this revision. rsmith added inline comments. This revision is now accepted and ready to land. Comment at: lib/Sema/SemaExprCXX.cpp:7762-7764 + llvm::SmallDenseMap NewTransformCache; + auto SavedTransformCache = TransformCache; + TransformCache =

[PATCH] D62648: [Sema][Typo] Fix assertion failure for expressions with multiple typos

rsmith added a comment. Thanks, this looks good; just some nits. Comment at: lib/Sema/SemaExprCXX.cpp:7616 /// anything (having been passed an empty TypoCorrection). - void EmitAllDiagnostics() { + void EmitAllDiagnostics(bool isAmbiguous) { for (TypoExpr *TE :

[PATCH] D63508: make -frewrite-includes also rewrite conditions in #if/#elif

rsmith added a comment. Patch generally looks good; just a minor concern about the output format. Comment at: clang/lib/Frontend/Rewrite/InclusionRewriter.cpp:487-490 +OS << " - evaluated by -frewrite-includes */" << MainEOL; +OS << (elif ? "#elif" :

[PATCH] D63793: Treat the range of representable values of floating-point types as [-inf, +inf] not as [-max, +max].

rsmith created this revision. rsmith added reviewers: rnk, BillyONeal. Herald added a project: clang. Prior to r329065, we used [-max, max] as the range of representable values because LLVM's `fptrunc` did not guarantee defined behavior when truncating from a larger floating-point type to a

[PATCH] D62293: [modules] Add PP callbacks for entering and leaving a submodule.

rsmith added a comment. `PPCallbacks` seems to be missing the addition of `EnteredSubmodule` and `LeftSubmodule`; `PPChainedCallbacks` seems to be missing the addition of `LeftSubmodule`. Generally this seems reasonable. Comment at: lib/Lex/PPLexerChange.cpp:783 +if

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

rsmith added inline comments. Comment at: include/clang/AST/Decl.h:4303 + + StringLiteral *getSTLUuid() { return STLUuid; } +}; What does "STL" mean here? Comment at: include/clang/Sema/ParsedAttr.h:337-346 + /// Constructor for

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rsmith added inline comments. Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:223 +def note_constexpr_bit_cast_invalid_type : Note< + "cannot constexpr evaluate a bit_cast with a " + "%select{union|pointer|member pointer|volatile|struct with a reference member}0"

[PATCH] D63161: Devirtualize destructor of final class.

rsmith added inline comments. Comment at: test/CodeGenCXX/devirtualize-dtor-final.cpp:20 + void evil(D *p) { +// CHECK-NOT: call void@_ZN5Test11DD1Ev +delete p; Missing space after `void` here Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D63161: Devirtualize destructor of final class.

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Thanks! Minor comment then feel free to commit. Comment at: lib/CodeGen/CGExprCXX.cpp:1877 + DevirtualizedDtor->getParent(); + if (getCXXRecord(Base)

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

rsmith marked an inline comment as done. rsmith added inline comments. Comment at: lib/CodeGen/CGExprAgg.cpp:1850 +AggValueSlot::Overlap_t +CodeGenFunction::overlapForFieldInit(const FieldDecl *FD) { + if (!FD->hasAttr() || !FD->getType()->isRecordType())

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL363976: P0840R2: support for [[no_unique_address]] attribute (authored by rsmith, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES

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

rsmith added a comment. In D63451#1549563 , @rjmccall wrote: > Can this attribute not be applied to a base class, or to a type? The standard attribute forbids that right now. We could add a custom attribute that permits it, but we're required to

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

rsmith updated this revision to Diff 205892. rsmith marked 4 inline comments as done. rsmith added a comment. - Use custom code to specify CXXABI requirements on attributes. - Remove dead code that would have handled [[no_unique_address]] in C. - Extend documentation to include an example and to

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

rsmith added inline comments. Comment at: lib/CodeGen/CGExprAgg.cpp:1850 +AggValueSlot::Overlap_t +CodeGenFunction::overlapForFieldInit(const FieldDecl *FD) { + if (!FD->hasAttr() || !FD->getType()->isRecordType()) rsmith wrote: > rjmccall wrote: > >

[PATCH] D63376: [clang] Small improvments after Adding APValue to ConstantExpr

rsmith added inline comments. Comment at: clang/include/clang/Sema/Sema.h:10298 + bool AllowFold = true, + bool StoreResult = true); ExprResult VerifyIntegerConstantExpression(Expr *E,

[PATCH] D63508: make -frewrite-includes handle __has_include wrapped in a macro

rsmith added a comment. Perhaps we should rewrite all `#if`-like directives to `#if 0` or `#if 1`? Either that or we could emit pragmas indicating the values of later `__has_include`s. Special-casing macros of this specific form is at best a partial solution. Repository: rC Clang CHANGES

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

rsmith marked 2 inline comments as done. rsmith added inline comments. Comment at: lib/AST/Decl.cpp:3937 + // -- [has] virtual member functions or virtual base classes, or + // -- has subobjects of nonzero size or bit-fields of nonzero length + if (const auto *CXXRD =

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

rsmith updated this revision to Diff 205488. rsmith added a comment. - Remove accidentally-added file Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63451/new/ https://reviews.llvm.org/D63451 Files: include/clang/AST/Decl.h include/clang/AST/DeclCXX.h

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

rsmith added inline comments. Comment at: include/clang/AST/DeclCXX.h:337-341 /// true when this class is empty for traits purposes, -/// i.e. has no data members other than 0-width bit-fields, has no +/// i.e. has no data members other than 0-width bit-fields and

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

rsmith updated this revision to Diff 205487. rsmith marked 5 inline comments as done. rsmith added a comment. Herald added a reviewer: jfb. Herald added subscribers: jfb, arphaman. - Address review comments from @AaronBallman. Repository: rC Clang CHANGES SINCE LAST ACTION

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

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Thanks! I think we can maybe make this a little more convenient to use. For the non-JSON `-ast-dump` format, I added `utils/make-ast-dump-check.sh` which can be used as (for example): $

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

rsmith added a comment. In D57086#1546386 , @aaron.ballman wrote: > In D57086#1535873 , @domdom wrote: > > > Something I should ask, it seems like GCC only ignores the NullStmts at the > > end if it's in C mode.

[PATCH] D63161: Devirtualize destructor of final class.

rsmith added inline comments. Comment at: lib/CodeGen/CGExprCXX.cpp:1887 +Dtor = DevirtualizedDtor; +Ptr = CGF.EmitPointerWithAlignment(Inner); + } else { yamauchi wrote: > rsmith wrote: > > In this case we'll emit the inner

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL363620: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and… (authored by rsmith, committed by

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

rsmith marked an inline comment as done. rsmith added inline comments. Comment at: lib/CodeGen/CGExprConstant.cpp:162 + return replace(V, BeginOff, EndOff, Vals.begin(), Vals.end()); +} + rjmccall wrote: > Can these go to STLExtras or somewhere similar? Done.

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

rsmith updated this revision to Diff 205182. rsmith marked 2 inline comments as done. rsmith added a comment. - Address additional review comments from rjmccall. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63371/new/ https://reviews.llvm.org/D63371 Files:

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

rsmith marked an inline comment as done. rsmith added inline comments. Comment at: lib/CodeGen/CGExprConstant.cpp:967 // Constant folding is currently missing support for a few features supported // here: CK_ToUnion, CK_ReinterpretMemberPointer, and DesignatedInitUpdateExpr.

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

rsmith updated this revision to Diff 205162. rsmith added a comment. - Fix somewhat-incorrect comment on Size member. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63371/new/ https://reviews.llvm.org/D63371 Files: lib/CodeGen/CGExprConstant.cpp

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

rsmith added inline comments. Comment at: lib/CodeGen/CGExprConstant.cpp:73 +/// Incremental builder for an llvm::Constant* holding a structure constant. +class ConstantBuilder : private ConstantBuilderUtils { + llvm::SmallVector Elems; rjmccall wrote: > This

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

rsmith updated this revision to Diff 205161. rsmith marked 13 inline comments as done. rsmith added a comment. - Address review comments from rjmccall. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63371/new/ https://reviews.llvm.org/D63371 Files:

[PATCH] D63376: [clang] Small improvments after Adding APValue to ConstantExpr

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Nice cleanup! Comment at: clang/include/clang/Sema/Sema.h:10298 + bool AllowFold = true, +

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

rsmith created this revision. rsmith added a reviewer: rjmccall. Herald added subscribers: aheejin, dschuff. Herald added a project: clang. Add support for the C++2a [[no_unique_address]] attribute for targets using the Itanium C++ ABI. This depends on D63371 .

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

rsmith added a comment. In D63371#1546500 , @rjmccall wrote: > Isn't `[[no_unique_address]]` only significant for empty members? I'm not > sure why they need significant support from constant-building, since they > expand to no meaningful initializer.

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

rsmith created this revision. rsmith added a reviewer: rjmccall. Herald added a project: clang. This adds a ConstantBuilder class that deals with incrementally building an aggregate constant, including support for overwriting previously-emitted parts of the aggregate with new values. This fixes

[PATCH] D63369: [AST] Fixed extraneous warnings for binary conditional operator

rsmith accepted this revision. rsmith added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/AST/Expr.cpp:2351-2352 return false; if (!Exp->getLHS()) return true; return Exp->getLHS()->isUnusedResultAWarning(WarnE,

[PATCH] D63161: Devirtualize destructor of final class.

rsmith added inline comments. Comment at: lib/CodeGen/CGExprCXX.cpp:1867-1868 - if (Dtor->isVirtual()) { + if (Dtor->isVirtual() && + !(Dtor->hasAttr() || RD->hasAttr())) { CGF.CGM.getCXXABI().emitVirtualObjectDelete(CGF, DE, Ptr, ElementType,

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rsmith added a comment. In D62825#1542662 , @rjmccall wrote: > In D62825#1542639 , @rsmith wrote: > > > In my view, the mistake was specifying `nullptr_t` to have the same size > > and alignment as `void*`; it

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rsmith added a comment. In D62825#1542597 , @rjmccall wrote: > In D62825#1542374 , @rsmith wrote: > > > In D62825#1542309 , @rjmccall > > wrote: > > > > > In D62825#1542301

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rsmith added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:5360 + // FIXME: Its possible under the C++ standard for 'char' to not be 8 bits, but + // we don't support a host or target where that is the case. Still, we should + // use a more generic type in case

[PATCH] D63161: Devirtualize destructor of final class.

rsmith added inline comments. Comment at: lib/CodeGen/CGExprCXX.cpp:1871 + CGF.CGM.getLangOpts().AppleKext))) + Dtor = DevirtualizedDtor; +else { `Dtor` could be the destructor for a type derived from

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rsmith added a comment. (You might argue that it's ridiculous to require that `nullptr_t` have the same size and alignment as `void*` but not have the same storage representation as a null `void*`. I'd agree, and I've raised this in committee before, but without success) Repository: rC

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rsmith added a comment. In D62825#1542309 , @rjmccall wrote: > In D62825#1542301 , @rsmith wrote: > > > In D62825#1542247 , @rjmccall > > wrote: > > > > > In what sense is

[PATCH] D63157: C++ DR712 and others: handle non-odr-use resulting from an lvalue-to-rvalue conversion applied to a member access or similar not-quite-trivial lvalue expression.

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL363295: C++ DR712 and others: handle non-odr-use resulting from an lvalue-to-rvalue… (authored by rsmith, committed by ).

[PATCH] D63164: [HIP] Add option to force lambda nameing following ODR in HIP/CUDA.

rsmith added a comment. I think this is the wrong way to handle this issue. We need to give lambdas a mangling if they occur in functions for which there can be definitions in multiple translation units. In regular C++ code, that's inline functions and function template specializations, so

[PATCH] D62825: [C++2a] Add __builtin_bit_cast, used to implement std::bit_cast

rsmith added a comment. In D62825#1542247 , @rjmccall wrote: > In what sense is the bit-pattern of a null pointer indeterminate? The problem is not null pointers, it's `nullptr_t`, which is required to have the same size and alignment as `void*` but

[PATCH] D63157: C++ DR712 and others: handle non-odr-use resulting from an lvalue-to-rvalue conversion applied to a member access or similar not-quite-trivial lvalue expression.

rsmith added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:1429 +/// for instance if a block or lambda or a member of a local class uses a +/// const int variable or constexpr variable from an enclosing function. CodeGenFunction::ConstantEmission rjmccall

[PATCH] D63157: C++ DR712 and others: handle non-odr-use resulting from an lvalue-to-rvalue conversion applied to a member access or similar not-quite-trivial lvalue expression.

rsmith updated this revision to Diff 204589. rsmith marked 2 inline comments as done. rsmith added a comment. Updated comment, fixed typo, added use of `[[clang::lifetimebound]]`. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63157/new/

[PATCH] D62988: Add an attribute to allow fields of non-trivial types in C unions

rsmith added a comment. In D62988#1540359 , @rjmccall wrote: > For ARC, you could bzero the union member; this is what how we tell people to > initialize malloc'ed memory as well, since there's no default-constructor > concept that they can invoke for

[PATCH] D62988: Add an attribute to allow fields of non-trivial types in C unions

rsmith added a comment. In D62988#1539230 , @ahatanak wrote: > In D62988#1538577 , @rsmith wrote: > > > How do you write correct (non-leaking, non-double-freeing, > > non-releasing-invalid-pointers) code with this

[PATCH] D63175: [MS] Pretend constexpr variable template specializations are inline

rsmith accepted this revision. rsmith added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/AST/ASTContext.cpp:9809 + // variable template specializations inline. + if (isa(VD) && VD->isConstexpr()) +return

[PATCH] D63161: Devirtualize destructor of final class.

rsmith added inline comments. Comment at: lib/CodeGen/CGExprCXX.cpp:1867-1868 - if (Dtor->isVirtual()) { + if (Dtor->isVirtual() && + !(Dtor->hasAttr() || RD->hasAttr())) { CGF.CGM.getCXXABI().emitVirtualObjectDelete(CGF, DE, Ptr, ElementType,

[PATCH] D63157: C++ DR712 and others: handle non-odr-use resulting from an lvalue-to-rvalue conversion applied to a member access or similar not-quite-trivial lvalue expression.

rsmith created this revision. rsmith added a reviewer: rjmccall. Herald added a subscriber: jdoerfert. Herald added a project: clang. When a variable is named in a context where we can't directly emit a reference to it (because we don't know for sure that it's going to be defined, or it's from an

[PATCH] D62988: Add an attribute to allow fields of non-trivial types in C unions

rsmith added a comment. How do you write correct (non-leaking, non-double-freeing, non-releasing-invalid-pointers) code with this attribute? For example, suppose I have a `__strong` union member: does storing to it release the old value (which might be a different union member)? If so, how do

[PATCH] D63072: [clang] Fixing incorrect implicit deduction guides (PR41549)

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

[PATCH] D50360: [Concepts] Requires Expressions

rsmith added inline comments. Comment at: include/clang/AST/DeclCXX.h:2051 +/// \code +/// template requires requires (T t) { {t++} -> Regular }; +/// \endcode The semicolon here is in the wrong place (should be before the final `}`, not after).

[PATCH] D63072: [clang] Fixing incorrect implicit deduction guides (PR41549)

rsmith added inline comments. Comment at: clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp:503 + +umm<> m(1); + Should this be using CTAD? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63072/new/

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