[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

rsmith added a comment. I worry that we're chasing after the implementation details of GCC here. It seems self-contradictory to say all three of these things at once: 1. The frontend function `foo` has known, builtin semantics X. (Eg, we'll use those semantics in the frontend.) 2. The symbol

[PATCH] D87565: [Sema] Improve const_cast conformance to N4261

rsmith added inline comments. Comment at: clang/lib/Sema/SemaCast.cpp:1817-1819 + NeedToMaterializeTemporary = + SrcType->isRecordType() || SrcType->isArrayType() || + SrcType->isFunctionPointerType() || SrcType->isMemberPointerType(); It

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

rsmith added a comment. Reverse ping: I have a patch implementing class type non-type template parameters that's blocked on this landing. If you won't have time to address @martong's comments soon, do you mind if I take this over and land it? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

rsmith updated this revision to Diff 298268. rsmith marked 3 inline comments as done. rsmith added a comment. - Addressing review feedback from @aaron.ballman. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89212/new/

[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

rsmith marked an inline comment as done. rsmith added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:6435-6436 +Attr *WeakA = nullptr; +for (Attr *A : VD->getAttrs()) { + if (!isa(A)) +continue; aaron.ballman wrote: > Ah, it's too

[PATCH] D88498: [FPEnv] Evaluate initializers in constant rounding mode

rsmith requested changes to this revision. rsmith added a comment. This revision now requires changes to proceed. In D88498#2329845 , @sepavloff wrote: > - Reverted check to the previous version, in which it applied to C++ file > level variables also.

[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

rsmith added a comment. In D89212#2326771 , @rjmccall wrote: > Patch looks good to me. I think we're running some internal tests; do you > want to wait for those? More testing and validation would be nice; I don't think this patch is urgent so I'm

[PATCH] D89360: Treat constant contexts as being in the default rounding mode.

rsmith added inline comments. Comment at: clang/test/SemaCXX/rounding-math.cpp:9 + +constexpr int f(int n) { return int(n * (1.0 / 3.0)); } + rsmith wrote: > sepavloff wrote: > > This code requires additional solution. The function body is built using > >

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

rsmith added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2165 +assert(getContext().getTargetAddressSpace(FI.arg_begin()->type) == 0 && + "Expected `this` pointer without address space attribute."); + jdoerfert wrote: > I'm unsure why

[PATCH] D89360: Treat constant contexts as being in the default rounding mode.

rsmith added a comment. In D89360#2329856 , @sepavloff wrote: > I would propose to consider solution in D88498 > . It tries to fix the real reason of the > malfunction - using dynamic rounding mode for evaluation of

[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

rsmith added a comment. FYI: I posted a patch to implement the "manifestly constant evaluated" rule as https://reviews.llvm.org/D89360. It looks like the stricter rule doesn't work in practice; far too much C++ code uses `-frounding-math` and expects to be able to still include things like

[PATCH] D89360: Treat constant contexts as being in the default rounding mode.

rsmith created this revision. rsmith added reviewers: rjmccall, mibintc, sepavloff. Herald added a project: clang. rsmith requested review of this revision. This addresses a regression where pretty much all C++ compilations using -frounding-math now fail, due to rounding being performed in

[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

rsmith updated this revision to Diff 297767. rsmith added a comment. - Don't warn if we see a weak definition for a declaration that's already Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89212/new/ https://reviews.llvm.org/D89212 Files:

[PATCH] D76620: [SYCL] Implement __builtin_unique_stable_name.

rsmith added a comment. In D76620#2324858 , @erichkeane wrote: > The feature that this supports is a part of the SYCL 2020 Provisional Spec, I > thought that was sufficient. We'll look into an RFC to re-submit in the > future. Does that cover only

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

rsmith added a comment. In D88712#2325823 , @MaskRay wrote: > In D88712#2324105 , @rsmith wrote: > >> What are the expected semantics in this case? Is this: >> >> - the function is still the builtin, but if it ends

[PATCH] D17993: [CodeGen] Apply 'nonnull' to 'this' pointer arguments.

rsmith requested changes to this revision. rsmith added a comment. This revision now requires changes to proceed. A lot of the test changes here seem to be over-constraining the existing tests. Tests that don't care about `nonnull` / `dereferenceable` `this` pointers should generally not be

[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

rsmith added a comment. In D89212#2324127 , @rjmccall wrote: > This seems like a good idea in the abstract; we'll need to figure out what > the practical consequences are. Looks like it triggers warnings in Abseil at least; there, the code looks like

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

rsmith added a comment. What are the expected semantics in this case? Is this: - the function is still the builtin, but if it ends up being a libcall, call a function with a different asm name, or - the function is not considered to be the builtin any more, or - something else? I think this

[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

rsmith created this revision. rsmith added reviewers: rjmccall, aaron.ballman. Herald added a project: clang. rsmith requested review of this revision. Such code will not work in general; we might have already used the non-weakness, for example when constant-evaluating an address comparison

[PATCH] D88498: [FPEnv] Evaluate initializers in constant rounding mode

rsmith added inline comments. Comment at: clang/lib/Parse/ParseDecl.cpp:2290 + // rounding mode. + if (VD->isFileVarDecl() || VD->isConstexpr() || + (!getLangOpts().CPlusPlus && VD->isStaticLocal())) { mibintc wrote: > sepavloff

[PATCH] D88984: Prefer libc++ headers in the -isysroot over the toolchain

rsmith added a comment. This seems like it would break the opposite situation: a user places a custom clang and the corresponding libc++ somewhere (perhaps in `$HOME`, or perhaps they run Clang from the build tree). Right now we use the custom clang and its corresponding libc++, but with this

[PATCH] D85144: [clang] Improve Dumping of APValues

rsmith added a comment. Ideally we should be tree-dumping the `LValue` representation rather than pretty-printing it (and similarly for member pointers and label address differences). The problem with doing so is that we can't interpret the `LValuePathEntry`s without knowing the type of the

[PATCH] D88643: [NFC] Correct name of profile function to Profile in APValue

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Good idea. (My use for this in class type non-type template parameters doesn't directly use `APValue`s as values in a folding set, but that seems like a reasonable use case for this

[PATCH] D87853: [SemaTemplate] Stop passing insertion position around during VarTemplate instantiation

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

[PATCH] D54222: [clang-tidy] Add a check to detect returning static locals in public headers

rsmith added a comment. In D54222#2313686 , @JonasToth wrote: > I am confused now, but richard knows this stuff very well! The patches you > referenced showed only cases where the static was defined in an inline-header > definition (and templates). Does

[PATCH] D87962: [clang] Change the multi-character character constants from extension to implementation-defined.

rsmith added inline comments. Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:109 "multi-character character constant">, InGroup; -def ext_four_char_character_literal : Extension< +def ext_four_char_character_literal : Warning< "multi-character character

[PATCH] D88446: docs: add documentation describing API Notes

rsmith accepted this revision. rsmith added a comment. Thanks, looks good to me. Comment at: clang/docs/APINotes.rst:250-252 + Note that the type is *not* parsed in the context where it will be used, + which means that macros are not available and nullability must be applied

[PATCH] D88518: Recognize setjmp and friends as builtins even if jmp_buf is not declared yet.

rsmith added a comment. In D88518#2308085 , @mstorsjo wrote: > This broke use of setjmp for mingw on x86_64. Thanks, should be fixed in rG8fb2a235b0f22dedba72b8b559ba33171a8dcd09 . Now

[PATCH] D88446: docs: add documentation describing API Notes

rsmith added inline comments. Comment at: clang/docs/APINotes.rst:233-235 + Note that the type is *not* parsed in the context where it will be used, + which means that macros are not available and nullability must be applied + explicitly (even in an

[PATCH] D54222: [clang-tidy] Add a check to detect returning static locals in public headers

rsmith added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-problematic-statics.rst:7-8 +Detects functions defined in headers that return the address of a static +local variable. These are not guaranteed to have the same addresss across +shared

[PATCH] D87962: [clang] Change the multi-character character constants from extension to implementation-defined.

rsmith added a comment. In D87962#2306043 , @aaron.ballman wrote: > That doesn't sound like the right approach to me -- Remarks are usually for > reporting backend decision-making to the frontend for things like > optimization passes. To be clear:

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

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Looks great, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63640/new/ https://reviews.llvm.org/D63640

[PATCH] D88338: [clangd] clangd --check: standalone diagnosis of common problems

rsmith added inline comments. Comment at: clang-tools-extra/clangd/test/check.test:1 +# RUN: clangd -log=verbose -check 2>&1 | FileCheck -strict-whitespace %s + This test implicitly parses a source file that `#include`s standard library headers, and fails if

[PATCH] D88518: Recognize setjmp and friends as builtins even if jmp_buf is not declared yet.

This revision was automatically updated to reflect the committed changes. Closed by commit rG1c604a9f5fd6: Recognize setjmp and friends as builtins even if jmp_buf is not declared yet. (authored by rsmith). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D67678: PR17164: Change clang's default behavior from -flax-vector-conversions=all to -flax-vector-conversions=integer.

rsmith added a comment. In D67678#1856229 , @rsmith wrote: > In D67678#1836953 , @dexonsmith > wrote: > >> In D67678#1836922 , @rsmith wrote: >> >>> If there's no timeline

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

rsmith added a comment. In D77491#2300013 , @rjmccall wrote: > Being permissive about recognizing builtins when the expected signature > requires a type that lookup can't find seems completely reasonable. We don't > really want to force library

[PATCH] D88518: Recognize setjmp and friends as builtins even if jmp_buf is not declared yet.

rsmith created this revision. rsmith added reviewers: tambre, rjmccall. Herald added a project: clang. rsmith requested review of this revision. This happens in glibc's headers. It's important that we recognize these functions so that we can mark them as returns_twice. Repository: rG LLVM

[PATCH] D88446: docs: add documentation describing API Notes

rsmith added a comment. Broadly, it seems reasonable to me for Clang to support this. I have no major concerns with the overall approach here, and it seems like you already have sufficient implementation experience with this approach to know that it's going to work out well in practice. I'm

[PATCH] D88445: Use "default member initializer" instead of "in-class initializer" for diagnostics

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

[PATCH] D88498: [FPEnv] Evaluate initializers in constant rounding mode

rsmith requested changes to this revision. rsmith added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Parse/ParseDecl.cpp:2290 + // rounding mode. + if (VD->isFileVarDecl() || VD->isConstexpr() || +

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

rsmith added a comment. We've hit a fairly subtle miscompile caused by this patch. glibc's setjmp.h looks like this (irrelevant parts removed): struct __jmp_buf_tag { /*...*/ }; extern int __sigsetjmp(struct __jmp_buf_tag __env[1], int); typedef struct __jmp_buf_tag sigjmp_buf[1];

[PATCH] D68364: Implement C++20's P0784 (More constexpr containers)

rsmith added a comment. In D68364#2299336 , @ldionne wrote: > In D68364#2294569 , @jwakely wrote: > >> I don't think your implementation is valid. I think P0784 only allows >> new-expressions and calls to

[PATCH] D88333: Better diagnostics for anonymous bit-fields with attributes or an initializer

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Thanks! Comment at: clang/lib/Parse/ParseDecl.cpp:4126 /// struct-declarator: declarator[opt] ':' constant-expression -if (Tok.isNot(tok::colon)) { +if

[PATCH] D88445: Use "default member initializer" instead of "in-class initializer" for diagnostics

rsmith added a comment. Thanks! Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:864-867 + "default member initialization of non-static data member is a C++11 " + "extension">, InGroup; def warn_cxx98_compat_nonstatic_member_init : Warning< + "default member

[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

rsmith added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:2446 + fpOptions.isFPConstrained()) { +Info.CCEDiag(E, diag::note_constexpr_float_arithmetic_strict); +return false; FFDiag Comment at:

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

rsmith added inline comments. Comment at: clang/include/clang/AST/APValue.h:651-653 + /// The following functions are used as part of initialization. during + /// Deserialization and Importing. Reserve the space then write element + /// directly in place as after

[PATCH] D86649: Fix for assertion failure on PR46865

rsmith requested changes to this revision. rsmith added a comment. This revision now requires changes to proceed. I think the assertion is correct: if we reach this point, then we have a `DeclRefExpr` that is not value-dependent and refers to a declaration whose initializer is value-dependent.

[PATCH] D88333: Better diagnostics for anonymous bit-fields with attributes or an initializer

rsmith added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:875-876 "C++ standards before C++20">, InGroup, DefaultIgnore; +def err_anon_bitfield_member_init : Error< + "anonymous bit-field cannot have an in-class initializer">; def

[PATCH] D87561: [Sema] List conversion validate character array

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Thanks, only nits here. Please feel free to submit after addressing them, or request another round of review if you prefer. Comment at:

[PATCH] D88333: Correctly parse attributes on the declaration of an anonymous bit-field

rsmith added a comment. I think we established rough consensus in CWG (at least, no one objected) that this is a grammar bug -- there's nothing for these attributes to appertain to, so they shouldn't be permitted. Unless CWG indicates it wants to change direction here I think we should

[PATCH] D88236: [PR47636] Fix tryEmitPrivate to handle non-constantarraytypes

rsmith added inline comments. Comment at: clang/lib/CodeGen/CGExprConstant.cpp:2148 +CommonElementType == nullptr && !NumInitElts) { const ArrayType *AT = CGM.getContext().getAsArrayType(DestType); CommonElementType =

[PATCH] D88236: [PR47636] Fix tryEmitPrivate to handle non-constantarraytypes

rsmith added inline comments. Comment at: clang/test/CodeGenCXX/pr47636.cpp:2 +// RUN: %clang_cc1 -o - -emit-llvm -triple x86_64-linux-pc %s | FileCheck %s +int(&_rvref)[] {1,2,3,4}; +// CHECK: @_ZGR10intu_rvref_ = internal global [4 x i32] [i32 1, i32 2, i32 3, i32 4]

[PATCH] D87808: [DebugInfo] Fix bug in constructor homing where it would use ctor homing when a class only has copy/move constructors

rsmith added a comment. In D87808#2288186 , @dblaikie wrote: > In D87808#2286664 , @rsmith wrote: > >> But I think this code should give the same outcome either way, because a >> class with any constructor other

[PATCH] D87808: [DebugInfo] Fix bug in constructor homing where it would use ctor homing when a class only has copy/move constructors

rsmith added a comment. In D87808#2280197 , @dblaikie wrote: > @rsmith What's the deal with these anonymous structs/unions? Why do they have > copy/move constructors (are those technically called from the enclosing > class's copy/move constructors?) but

[PATCH] D87080: [AST] Reduce the size of TemplateArgumentLocInfo.

rsmith added inline comments. Comment at: clang/include/clang/AST/TemplateBase.h:421 -public: - constexpr TemplateArgumentLocInfo() : Template({nullptr, nullptr, 0, 0}) {} - - TemplateArgumentLocInfo(TypeSourceInfo *TInfo) : Declarator(TInfo) {} + T *getTemplate() const { +

[PATCH] D87561: [Sema] List conversion validate character array

rsmith added inline comments. Comment at: clang/test/CXX/drs/dr14xx.cpp:411-414 + void f(const char[4]); + void f(const wchar_t[4]); + void f(const char16_t[4]); + void f(const char32_t[4]); rsmith wrote: > Mordante wrote: > > rsmith wrote: > > > These

[PATCH] D87561: [Sema] List conversion validate character array

rsmith added inline comments. Comment at: clang/test/CXX/drs/dr14xx.cpp:411-414 + void f(const char[4]); + void f(const wchar_t[4]); + void f(const char16_t[4]); + void f(const char32_t[4]); Mordante wrote: > rsmith wrote: > > These should presumably be

[PATCH] D87349: [clang] adapt c++17 behavior for dependent decltype-specifiers

rsmith added inline comments. Comment at: clang/include/clang/AST/Type.h:4478 - DecltypeType(Expr *E, QualType underlyingType, QualType can = QualType()); + const ASTContext + DecltypeType(Expr *E, QualType underlyingType, const ASTContext , We shouldn't

[PATCH] D68364: Implement C++20's P0784 (More constexpr containers)

rsmith added inline comments. Comment at: libcxx/include/version:254 // # define __cpp_lib_concepts 201806L +# define __cpp_lib_constexpr_dynamic_alloc 201907L // # define __cpp_lib_constexpr_misc 201811L

[PATCH] D87853: [Sema] Update specialization iterator after template argument deduction.

rsmith added a comment. Thanks, this is pretty subtle. Here's a small C++20 testcase that invalidates `InsertPos` while matching partial specializations: template constexpr bool v = true; template requires v constexpr bool v = true; template<> constexpr bool v = true; int k = v;

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

rsmith added inline comments. Comment at: clang/lib/AST/TemplateBase.cpp:71-72 if (T->isBooleanType() && !Policy.MSVCFormatting) { Out << (Val.getBoolValue() ? "true" : "false"); } else if (T->isCharType()) { reikdas wrote: > rsmith wrote: > >

[PATCH] D68364: Implement C++20's P0784 (More constexpr containers)

rsmith added inline comments. Comment at: libcxx/include/version:254 // # define __cpp_lib_concepts 201806L +# define __cpp_lib_constexpr_dynamic_alloc 201907L // # define __cpp_lib_constexpr_misc 201811L

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

rsmith added a comment. In D77491#2280096 , @jrtc27 wrote: > If someone cares about pthread_create they might wish to follow up on my > https://reviews.llvm.org/D58531, which I filed early last year to permit > pthread_create to have a proper type in

[PATCH] D58531: [clang] Specify type of pthread_create builtin

rsmith added inline comments. Comment at: clang/include/clang/Serialization/ASTBitCodes.h:1223 /// The number of special type IDs. const unsigned NumSpecialTypeIDs = 8; This presumably needs to be increased. Are we missing test coverage that would

[PATCH] D68364: Implement C++20's P0784 (More constexpr containers)

rsmith added a comment. The mechanism by which this interacts with Clang looks good to me. I've not done any detailed analysis to check all the functions made `constexpr` by P0784 are handled by this patch, though. Comment at: libcxx/include/version:254 // # define

[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

rsmith added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:2683 } + if (const BinaryOperator *BE = dyn_cast(E)) { +if (BE->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).isFPConstrained()) { `E` here is only intended for use in determining

[PATCH] D87561: [Sema] List conversion validate character array

rsmith added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:4989 +if (ToType->isArrayType() && ToType->isCharType() && +isa(From->getInit(0))) { InitializedEntity Entity = Mordante wrote: > rsmith wrote: > > This is too narrow a

[PATCH] D87349: [clang] adapt c++17 behavior for dependent decltype-specifiers

rsmith added inline comments. Comment at: clang/test/CodeGenCXX/mangle.cpp:805 - // CHECK-LABEL: define weak_odr void @_ZN6test341fIiEEvDTstDTplcvT__EcvS1__EEE + // CHECK-LABEL: define weak_odr void @_ZN6test341fIiEEvm template void f(decltype(sizeof(1)));

[PATCH] D87349: [clang] adapt c++17 behavior for dependent decltype-specifiers

rsmith added inline comments. Comment at: clang/test/CodeGenCXX/mangle-exprs.cpp:218 template void a(decltype(noexcept(int(; - // CHECK: void @_ZN5test51aIiEEvDTnxcvT__EE( + // CHECK: void @_ZN5test51aIiEEvb } As with the other case, the expression

[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

rsmith requested changes to this revision. rsmith added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/AST/ExprConstant.cpp:775-778 +/// Strict floating point is enabled, this inhibits +/// floating ponit constant folding. +bool

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

rsmith accepted this revision. rsmith added a comment. Thanks! This looks good to me (subject to Aaron's comment being addressed). Please wait a couple of days for any more comments from the other reviewers. Comment at: clang/lib/Sema/SemaDecl.cpp:9689 +

[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

rsmith added a comment. In D87528#2270561 , @rjmccall wrote: > Richard, what do you think the right rule is for when to use the special > constant-evaluation rules for this feature in C++? The C standard says that > constant expressions should use the

[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

rsmith added a comment. > @rsmith asked "I don't see changes to the constant evaluator". The semantic > rules for enabling this pragma require that strict or precise semantics be in > effect., see SemaAttr.cpp And the floating point constant evaluation doesn't > occur when strict or precise

[PATCH] D86048: [AST][RecoveryExpr] Popagate the error-bit from a VarDecl's initializer to DeclRefExpr.

rsmith added a comment. In D86048#2254607 , @sammccall wrote: > The crux is we're forcing `decltype(N)` to be a (unique) dependent type, > which feels wrong. > [...] > There are three behaviors: > > - standard up to C++14: traversing the expr looking for

[PATCH] D87565: [Sema] Improve const_cast conformance to N4261

rsmith added a comment. For what it's worth, the most recent thoughts from CWG on this question are from 2015 (prior to guaranteed copy elision, which probably changes the answer): http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1965. That direction would permit `const_cast` to

[PATCH] D87565: [Sema] Improve const_cast conformance to N4261

rsmith requested changes to this revision. rsmith added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Sema/SemaCast.cpp:1781-1782 // type T2 using the cast const_cast; and //-- if T1 is a class type, a prvalue of type

[PATCH] D87566: [Sema] Permit conversions to arrays of unknown bound

rsmith added a comment. I made a start on this a while back and found a bunch more places that needed updates: https://github.com/zygoloid/llvm-project/commit/c23440979ac4f07ce38657ce0e69d010d2afa83e Feel free to either take that patch as a basis or as inspiration for the remaining changes.

[PATCH] D87561: [Sema] List conversion validate character array

rsmith added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:4988 + +if (ToType->isArrayType() && ToType->isCharType() && +isa(From->getInit(0))) { `isCharType` is too narrow a check here. We also need to check for all the other types

[PATCH] D87563: [Sema] Improve overload resolution

rsmith added a comment. Looks like this is an implementation of DR1307. Please also add a test to `test/CXX/drs/dr13xx.cpp`. Comment at: clang/include/clang/Sema/Overload.h:548-549 + +/// The std::initializer_list expression to convert from. +const InitListExpr

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

rsmith added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:4583 if (!InitE) return getDefaultInitValue(VD->getType(), Val); The initializer might also be null because the variable is type-dependent (eg, `X x;`), in which case assuming

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

rsmith added inline comments. Comment at: clang/lib/Headers/intrin.h:435 #if defined(__i386__) || defined(__x86_64__) -static __inline__ void __DEFAULT_FN_ATTRS -__movsb(unsigned char *__dst, unsigned char const *__src, size_t __n) { +void __DEFAULT_FN_ATTRS __movsb(unsigned

[PATCH] D86508: [clang] improve GCC-compat for C90 __builtin_ functions

rsmith added a comment. It seems to me that adding new `__builtin_*` functions for GCC compatibility and adding new `LIBBUILTIN`s are unrelated changes, and it might be clearer to split them up into two commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D86508: [clang] improve GCC-compat for C90 __builtin_ functions

rsmith added inline comments. Comment at: clang/docs/LanguageExtensions.rst:2370-2398 +* ``bcmp`` * ``memchr`` -* ``memcmp`` (and its deprecated BSD / POSIX alias ``bcmp``) +* ``memcmp`` +* ``memcpy`` +* ``memmove`` +* ``memset`` +* ``strcat`` aaron.ballman

[PATCH] D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt

rsmith added inline comments. Comment at: clang/include/clang/AST/Stmt.h:1105- + /// The likelihood of a branch being taken. + enum Likelihood { +LH_None, ///< No attribute set. +LH_Likely, ///< Branch has the [[likely]] attribute. +LH_Unlikely, ///<

[PATCH] D86207: (De-)serialize BindingDecls before DecompositionDecl

rsmith added inline comments. Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:585 D->setLocation(ThisDeclLoc); D->setInvalidDecl(Record.readInt()); if (Record.readInt()) { // hasAttrs The bug is here: we should not be calling

[PATCH] D86993: Document Clang's expectations of the C standard library.

rsmith added a comment. In D86993#2251198 , @rjmccall wrote: > Wording looks good. Should we alsso document our assumptions about what > functions exist in the standard library — the functions that we'll always use > even in freestanding builds?

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

rsmith added a comment. In D85778#2251160 , @zequanwu wrote: > Hi, this change seems like hits a false positive case in chromium build: > https://bugs.chromium.org/p/chromium/issues/detail?id=1124085 That's not a false positive. The code is

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

rsmith added a comment. In D85778#2248714 , @nikic wrote: > Just for the record, the additional analysis has a measurable compile-time > impact (0.3% at O0): >

[PATCH] D86993: Document Clang's expectations of the C standard library.

rsmith updated this revision to Diff 289329. rsmith added a comment. Add missing word. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86993/new/ https://reviews.llvm.org/D86993 Files: clang/docs/Toolchain.rst Index: clang/docs/Toolchain.rst

[PATCH] D86993: Document Clang's expectations of the C standard library.

rsmith created this revision. rsmith added reviewers: fhahn, rjmccall. Herald added a project: clang. Herald added a subscriber: cfe-commits. rsmith requested review of this revision. As suggested in https://reviews.llvm.org/D86815 Repository: rG LLVM Github Monorepo

[PATCH] D86508: [clang] implement+test remaining C90 __builtin_ functions

rsmith added inline comments. Comment at: clang/include/clang/Basic/Builtins.def:488 +BUILTIN(__builtin_calloc, "v*zz", "F") +BUILTIN(__builtin_exit, "vi", "Fr") BUILTIN(__builtin_fprintf, "iP*cC*.", "Fp:1:") aaron.ballman wrote: > Should we be adding

[PATCH] D60939: [Concepts] Delayed Constraint Substitution

rsmith added a comment. I wonder if there's a cleaner way to model this: Suppose we add a new `Expr` subclass for an expression with delayed template argument substitution, which would capture a list of `TemplateArgument`s and an inner `Expr*` into which those template arguments have not yet

[PATCH] D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt

rsmith added a comment. Looking specifically for attributes in the 'then' and 'else' cases of an `if` seems like a fine first pass at this, but I think this is the wrong way to lower these attributes in the longer term: we should have a uniform treatment of them that looks for the most recent

[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled

rsmith requested changes to this revision. rsmith added a comment. This revision now requires changes to proceed. Sorry for not noticing this review earlier. We can't make this change. Module map files are still useful and still used even when modules are disabled -- they power the undeclared

[PATCH] D86508: [clang] implement+test remaining C90 __builtin_ functions

rsmith added a comment. LGTM I've not checked all the types are correct (someone should double-check that prior to commit), but it looks like GCC provides these `__builtin_*` functions, so we should too. The addition of the non-`__builtin_` versions should improve our diagnostics but

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

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Thanks! Comment at: clang/lib/Serialization/ASTReader.cpp:8395 +for (unsigned Idx = 0, N = LateParsed.size(); Idx < N; + /* In loop */) { + FunctionDecl *FD

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcff6dda604cb: More accurately compute the ranges of possible values for +, -, *, , %. (authored by rsmith). Changed prior to commit:

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

rsmith added a comment. In D77491#2246948 , @rjmccall wrote: > I'd still like @rsmith to sign off here as code owner. Generally, I'm happy with this direction. What happens for builtins with the "t" (custom typechecking) flag, for which the signature

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. rsmith marked an inline comment as done. Closed by commit rG0e00a95b4fad: Add new warning for compound punctuation tokens that are split across macro… (authored by

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

rsmith marked an inline comment as done. rsmith added inline comments. Herald added a subscriber: danielkiss. Comment at: clang/test/Parser/compound-token-split.cpp:30 + +[ // expected-warning-re ^}}'[' tokens introducing attribute appear in different source files}}

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