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

2020-11-25 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D91311#2416940 , @ldionne wrote: > LGTM from the libc++ point of view. The CI is passing -- those failures are > flaky modules tests that we need to fix. Perhaps we need to specify `-fmodules-validate-system-headers` in the

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

2020-11-24 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 307496. rsmith added a comment. - Remove _LIBCPP_HAS_NO_PREFERRED_NAME Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91311/new/ https://reviews.llvm.org/D91311 Files: clang/include/clang/Basic/Attr.td

[PATCH] D91895: [Clang] improve -Wimplicit-fallthrough GCC compat

2020-11-23 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. I'd suggest instead of removing these warnings, we split the warning flag in two: 1. A warning flag that warns that there is implicit fallthrough that's not equivalent to `break;` (which sounds like it's what the kernel folks want in the 99% case) -- that is, warn if

[PATCH] D91821: Fix PR42049 - Crash when parsing bad decltype use within template argument list after name assumed to be a function template

2020-11-20 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1055 + // semi-colon. + EndLoc = PP.getLastCachedTokenLocation(); +} It seems to me that either `EndLoc` was miscomputed and should already point to the last token that we

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

2020-11-18 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D91311#2403805 , @ldionne wrote: > We can stick with this design, but I'd like to understand why `#if > _LIBCPP_HAS_NO_PREFERRED_NAME` is necessary in ``, and also the CI is > failing on MacOS. You mean the HWAddressSanitizer

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

2020-11-18 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/include/clang/AST/StmtDataCollectors.td:67-75 +if (auto *ParamTypeDecl = dyn_cast(TemplParam)) { + const clang::Type *ParamType = ParamTypeDecl->getTypeForDecl(); + if (auto *autoT =

[PATCH] D67112: [Sema] Introduce function reference conversion, NFC

2020-11-17 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D67112#2401110 , @aaronpuchert wrote: > In D67112#2398577 , @rsmith wrote: > >> Looks fine as far as it goes, but it looks like we're also missing a cast in >> function pointer

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

2020-11-17 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:15545-15546 +// is only used for C code. +if (getLangOpts().MSVCCompat && (Kind == TTK_Enum) && Previous.empty() && +(TUK == TUK_Reference)) { + LookupResult TypedefEnumLookup(*this, Name,

[PATCH] D17993: [CodeGen] Apply 'nonnull' and 'dereferenceable(N)' to 'this' pointer arguments.

2020-11-17 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2169 +if (!CodeGenOpts.NullPointerIsValid && +getContext().getTargetAddressSpace(FI.arg_begin()->type) == 0) { + Attrs.addAttribute(llvm::Attribute::NonNull); jdoerfert

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

2020-11-17 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D91311#2400998 , @dblaikie wrote: > In D91311#2400926 , @ldionne wrote: > >> For instance, I can easily imagine a library that provides an API where some >> types shouldn't be named (for

[PATCH] D17993: [CodeGen] Apply 'nonnull' and 'dereferenceable(N)' to 'this' pointer arguments.

2020-11-17 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2169 +if (!CodeGenOpts.NullPointerIsValid && +getContext().getTargetAddressSpace(FI.arg_begin()->type) == 0) { + Attrs.addAttribute(llvm::Attribute::NonNull); jdoerfert

[PATCH] D91651: [clang] Add a warning (à la gcc) for too small enum bitfields

2020-11-17 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:16443-16446 + const unsigned BitsNeeded = + IsSignedEnum + ? std::max(ED->getNumPositiveBits() + 1, ED->getNumNegativeBits()) + : ED->getNumPositiveBits();

[PATCH] D91630: [Parse] Add parsing support for C++ attributes on using-declarations

2020-11-17 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. Do `[[deprecated]]` and `[[maybe_unused]]` now work for //using-declarator//s? If so, a test for that would be useful. Comment at: clang/lib/Parse/ParseDeclCXX.cpp:714 - // C++11 attributes are not allowed on a using-declaration, but GNU ones - //

[PATCH] D67112: [Sema] Introduce function reference conversion, NFC

2020-11-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Looks fine as far as it goes, but it looks like we're also missing a cast in function pointer initialization via function conversion: void f() noexcept; void (*p)() = f; results in

[PATCH] D17993: [CodeGen] Apply 'nonnull' and 'dereferenceable(N)' to 'this' pointer arguments.

2020-11-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG69cd776e1ee7: [CodeGen] Apply nonnull and dereferenceable(N) to this pointer (authored by CJ-Johnson, committed by rsmith). Herald added a project: clang. Changed prior to commit:

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

2020-11-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith marked an inline comment as done. rsmith added a comment. In D91311#2398144 , @ldionne wrote: > What the attribute achieves is great, however I must say I'm really with > Arthur's original comment regarding the ergonomics of it. I do agree, the

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

2020-11-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:4861 +// We need correct types when the template-name is unresolved or when it +// might be overloaded. +if (!ResolvedTemplate) Quuxplusone wrote: > And from the PR summary: >

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

2020-11-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 305393. rsmith added a comment. Tweak wording of comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91488/new/ https://reviews.llvm.org/D91488 Files: clang/include/clang/Basic/LangOptions.h

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

2020-11-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 305392. rsmith added a comment. Improve commit message. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91488/new/ https://reviews.llvm.org/D91488 Files: clang/include/clang/Basic/LangOptions.h

[PATCH] D90871: [Sema] Fold VLAs to constant arrays in a few more contexts

2020-11-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. The direction here seems fine to me. Comment at: clang/include/clang/Sema/Sema.h:2478 - Decl *ActOnDeclarator(Scope *S, Declarator ); + Decl *ActOnDeclarator(Scope *S, Declarator , bool NextTokIsEqual = false); Can we set a

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

2020-11-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith created this revision. rsmith added a reviewer: rjmccall. Herald added a subscriber: dexonsmith. Herald added a project: clang. rsmith requested review of this revision. For the Itanium ABI, this implements the mangling rule suggested in

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

2020-11-13 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith marked 2 inline comments as done. rsmith added inline comments. Comment at: libcxx/include/iosfwd:188 +#ifdef _LIBCPP_PREFERRED_NAME +template aaron.ballman wrote: > We always define `_LIBCPP_PREFERRED_NAME` so is this actually needed? Thanks, I was

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

2020-11-13 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 305222. rsmith added a comment. - Properly disable redundant redeclarations if preferred_name attribute is Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91311/new/ https://reviews.llvm.org/D91311 Files:

[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

2020-11-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/include/clang/Sema/DeclSpec.h:1762 }; +using FDK = FunctionDefinitionKind; I don't think it's OK to have an initialism like this in the `clang` namespace scope -- generally-speaking, the larger the scope of a

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

2020-11-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2367 +def PreferredName : InheritableAttr { + let Spellings = [Clang<"preferred_name">]; + let Subjects = SubjectList<[ClassTmpl]>; aaron.ballman wrote: > This seems like one we should

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

2020-11-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 304985. rsmith added a comment. - Remove debugging code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91311/new/ https://reviews.llvm.org/D91311 Files: clang/include/clang/Basic/Attr.td

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

2020-11-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 304984. rsmith marked 5 inline comments as done. rsmith added a comment. - Address review comments. - Extend libc++ changes to cover . - Bugfixes from further testing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

2020-11-11 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith created this revision. rsmith added reviewers: aaron.ballman, EricWF. Herald added a subscriber: jdoerfert. Herald added projects: clang, libc++. Herald added a reviewer: libc++. rsmith requested review of this revision. This attribute permits a typedef to be associated with a class

[PATCH] D90188: Add support for attribute 'using_if_exists'

2020-11-10 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:5273 + namespace empty_namespace {}; + using empty_namespace::does_not_exist __attribute__((using_if_exists)); // no error! + aaron.ballman wrote: > Mordante wrote: > >

[PATCH] D89998: [c++20] For P0732R2 / P1907R1: Basic code generation and name mangling supportfor non-type template parameters of class type and template parameter objects.

2020-11-09 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb637148ecb62: [c++20] For P0732R2 / P1907R1: Basic code generation and name (authored by rsmith). Changed prior to commit:

[PATCH] D72281: [Matrix] Add matrix type to Clang.

2020-11-09 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Herald added a subscriber: dang. Comment at: clang/lib/AST/ItaniumMangle.cpp:3353-3371 + auto = getASTContext(); + unsigned BitWidth = ASTCtx.getTypeSize(ASTCtx.getSizeType()); + llvm::APSInt Rows(BitWidth); + Rows = T->getNumRows(); +

[PATCH] D89998: [c++20] For P0732R2 / P1907R1: Basic code generation and name mangling supportfor non-type template parameters of class type and template parameter objects.

2020-11-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:5181 + mangleType(T); + Out << "0E"; + return; rsmith wrote: > rjmccall wrote: > > This could also be extracted and shared with the template-argument mangler. > > In fact,

[PATCH] D89998: [c++20] For P0732R2 / P1907R1: Basic code generation and name mangling supportfor non-type template parameters of class type and template parameter objects.

2020-11-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 303611. rsmith marked an inline comment as done. rsmith added a comment. - Factor out duplicated mangling of null pointers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89998/new/

[PATCH] D89998: [c++20] For P0732R2 / P1907R1: Basic code generation and name mangling supportfor non-type template parameters of class type and template parameter objects.

2020-11-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith marked an inline comment as done. rsmith added inline comments. Comment at: clang/lib/AST/APValue.cpp:1052 + // FIXME: These should be modeled as having the + // LifetimeExtendedTemporaryDecl itself as the base. + auto *MTE = dyn_cast(E);

[PATCH] D89998: [c++20] For P0732R2 / P1907R1: Basic code generation and name mangling supportfor non-type template parameters of class type and template parameter objects.

2020-11-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 303609. rsmith marked 10 inline comments as done. rsmith added a comment. - Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89998/new/ https://reviews.llvm.org/D89998 Files:

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

2020-11-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision. rsmith added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/CodeGenCXX/this-nonnull.cpp:1-2 +// RUN: %clang_cc1 -S -emit-llvm -o - -triple %itanium_abi_triple %s | FileCheck %s -check-prefix=CHECK-YES +//

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-11-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. In D88295#2365560 , @Quuxplusone wrote: >> So, how about we add another `CES` flag > > As the original author of `CES_AsIfByStdMove`, I am opposed to

[PATCH] D90719: [DebugInfo] Modify ctor homing as workaround for unconstructed libcxx types

2020-11-05 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. Perhaps we could address both the UB and the debug info homing issue by adding a new attribute on the libc++ types that says it's valid to create an instance of the type without calling a constructor? (If we want to support old libc++ with trunk clang, we could detect

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

2020-11-05 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. Having looked a bit more at the callers here, I think the pattern we're going to want is: - everywhere that prints a template argument list also asks for a list of corresponding template parameters - if the list of template parameters is unknown (perhaps because the

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

2020-11-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith requested changes to this revision. rsmith added a comment. This revision now requires changes to proceed. I think the documentation isn't quite right yet, but otherwise I think I'm happy. (With a couple of code change suggestions.) In D79279#2240487

[PATCH] D90634: Implement Lambda Conversion Operators for All CCs for MSVC.

2020-11-03 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/Sema/SemaLambda.cpp:1285 + /// that someone who intentionally places 'thiscall' on the lambda call + /// operator will still get that overload, since we don't have the a way of + /// detecting the attribute by the time we

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

2020-11-01 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. Thanks! We should also have a test for the behavior when targeting the MS ABI, where we sometimes don't emit the `nonnull dereferenceable` because the "`this`" pointer might actually point outside the object, but otherwise I think this is ready to go. Please can you

[PATCH] D88295: [Sema] Fix volatile check when test if a return object can be implicitly move

2020-10-30 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D88295#2365312 , @Quuxplusone wrote: > I believe the organization of that directory implies that the path should be > `class/class.init/class.copy.elision/`, not `special/class.copy/`. > Otherwise LGTM, and I like this new test

[PATCH] D88905: [Clang] Allow "ext_vector_type" applied to Booleans

2020-10-30 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/docs/LanguageExtensions.rst:466 + typedef bool bool4 __attribute__((ext_vector_type(4))); + // Objects of bool8 type hold 8 bits, sizeof(bool8) == 1 + Comment talks about `bool8` but we defined the type `bool4`.

[PATCH] D90123: [Sema] Improve notes for value category mismatch in overloading

2020-10-29 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Thanks! Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4280 "candidate %sub{select_ovl_candidate_kind}0,1,2 not viable: " -"expects an l-value for " +

[PATCH] D90188: Add support for attribute 'using_if_exists'

2020-10-27 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In addition to moving the diagnostic to `DiagnoseUseOfDecl`, you may well get better error recovery if you teach `ClassifyName` about the new kind of declaration, and have it `DiagnoseUseOfDecl` it immediately and return `NameClassification::Error()` -- that way, the

[PATCH] D89998: [c++20] For P0732R2 / P1907R1: Basic code generation and name mangling supportfor non-type template parameters of class type and template parameter objects.

2020-10-27 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:657-660 // ::= $1? // ::= $H? // ::= $I? // ::= $J? For what it's worth, I'm fairly

[PATCH] D89998: [c++20] For P0732R2 / P1907R1: Basic code generation and name mangling support for non-type template parameters of class type and template parameter objects.

2020-10-27 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 301070. rsmith added a comment. - Ignore cv-qualifiers in member types for Itanium mangling. - Add MSVC-compatible mangling support. - Fix bug where template argument value mangling would be used when passing a template parameter object as a pointer or

[PATCH] D89998: [c++20] For P0732R2 / P1907R1: Basic code generation and name mangling support for non-type template parameters of class type and template parameter objects.

2020-10-24 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 300524. rsmith added a comment. - Fix incorrect mangling for zero-but-not-null template arguments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89998/new/ https://reviews.llvm.org/D89998 Files:

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

2020-10-22 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/test/Parser/pragma-fenv_access.c:28 +#if defined(CPP) & defined(STRICT) +//not-expected-error@+3 {{constexpr variable 'frac' must be initialized by a constant expression}} +//not-expected-note@+2 {{compile time floating point

[PATCH] D89998: [c++20] For P0732R2 / P1907R1: Basic code generation and name mangling support for non-type template parameters of class type and template parameter objects.

2020-10-22 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith created this revision. rsmith added a reviewer: rjmccall. Herald added a project: clang. rsmith requested review of this revision. This follows the approach I proposed in https://github.com/itanium-cxx-abi/cxx-abi/issues/47 on 2020-09-06. Repository: rG LLVM Github Monorepo

[PATCH] D89986: [AIX]ignore the option -fvisibility-inlines-hidden when there is no option -fvisibility=*

2020-10-22 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp:30-34 +// COMMON-ASM: mflr 0 +// COMMON-ASM-NEXT:stw 0, 8(1) +// COMMON-ASM-NEXT:stwu 1, -64(1) +// COMMON-ASM-NEXT:bl ._Z1fv +// NOP-ASM-NEXT: nop

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

2020-10-21 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. (Accepted subject to previous comment.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84637/new/ https://reviews.llvm.org/D84637

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

2020-10-21 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. There are a couple of cases where you're returning `EvalStmtResult` from a function with a `bool` return type, that I'd like to see fixed before this lands. All the other comments are directed towards producing more precise behavior when evaluating a function

[PATCH] D89520: Don't permit array bound constant folding in OpenCL.

2020-10-20 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6781fee08505: Dont permit array bound constant folding in OpenCL. (authored by rsmith). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D89520: Don't permit array bound constant folding in OpenCL.

2020-10-20 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 299515. rsmith added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89520/new/ https://reviews.llvm.org/D89520 Files: clang/lib/AST/Decl.cpp clang/lib/AST/ExprConstant.cpp

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

2020-10-20 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Thanks, I'm happy with this, though please wait a day or two for comments from the other reviewers. We should also add some documentation covering the rules we're using for the

[PATCH] D89747: Add option to use older clang ABI behavior when passing certain union types as function arguments

2020-10-19 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/test/CodeGen/X86/avx-union.c:7 +// RUN: %clang_cc1 -w -ffreestanding -triple x86_64-linux-gnu -target-feature +avx512f -fclang-abi-compat=11.0 -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-LEGACY

[PATCH] D89747: Add option to use older clang ABI behavior when passing certain union types as function arguments

2020-10-19 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. This seems fine to me; thanks for the fix. For future reference, please upload diffs with full context (http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface).

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

2020-10-19 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D89559#2339206 , @erichkeane wrote: > Since the work is 99% the same, I think I should start doing that (emitting > BOTH in the case where it matches the default member but NOT the default free > function CC) in this review,

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

2020-10-19 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D63640#2331779 , @Tyker wrote: > but the "real" blocker is that the testing depends on D85144 > for testing. > we could land it marking the tests XFAIL and correct it when dumping >

[PATCH] D88498: [FPEnv] Apply dynamic rounding to function body only

2020-10-19 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. Herald added a subscriber: dexonsmith. The tests in this patch exhibit the same behavior with and without the patch applied; I think almost all the functionality changes from here are superseded by the change to consider whether we're in a manifestly constant evaluated

[PATCH] D89649: Fix __has_unique_object_representations with no_unique_address

2020-10-18 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. This looks fine as far as it goes, but it doesn't fix all cases of incorrect behavior of `__has_unique_object_representations` due to `[[no_unique_address]]`. Feel free to either to land this

[PATCH] D74130: [clang] fix consteval call in default arguments

2020-10-17 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/Sema/Sema.cpp:1026 + HandleImmediateInvocations(ExprEvalContexts.back()); + Tyker wrote: > rsmith wrote: > > What do we need this for? If I'm understanding the patch correctly, I think > > the only way we

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

2020-10-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:2445 + FPO.getFPExceptionMode() != LangOptions::FPE_Ignore || + FPO.getAllowFEnvAccess()) && Info.Ctx.getLangOpts().CPlusPlus) { +Info.FFDiag(E,

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

2020-10-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/Sema/SemaLambda.cpp:1278 + if (CallOpCC == DefaultMember) +return DefaultFree; + return CallOpCC; rsmith wrote: > erichkeane wrote: > > rjmccall wrote: > > > ...I made this comment in my first review, but

[PATCH] D89559: PR47372: Fix Lambda invoker calling conventions

2020-10-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/Sema/SemaLambda.cpp:1278 + if (CallOpCC == DefaultMember) +return DefaultFree; + return CallOpCC; erichkeane wrote: > rjmccall wrote: > > ...I made this comment in my first review, but Phabricator threw

[PATCH] D89520: Don't permit array bound constant folding in OpenCL.

2020-10-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 298764. rsmith added a comment. Rebase on D89523 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89520/new/ https://reviews.llvm.org/D89520 Files:

[PATCH] D89523: PR44406: Follow behavior of array bound constant folding in more recent versions of GCC.

2020-10-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG552c6c232872: PR44406: Follow behavior of array bound constant folding in more recent… (authored by rsmith). Changed prior to commit:

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

2020-10-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7e801ca0efa9: Treat constant contexts as being in the default rounding mode. (authored by rsmith). Repository: rG LLVM Github Monorepo CHANGES

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

2020-10-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D89360#2329167 , @rjmccall wrote: > Okay. I can accept the fairly unfortunate thing with `const int` > initializers. When is `IsConstantContext` set in C, just static initializers? This affects static initializers,

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

2020-10-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 298724. rsmith added a comment. - Add more testcases to demonstrate behavior in C. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89360/new/ https://reviews.llvm.org/D89360 Files:

[PATCH] D89520: Don't permit array bound constant folding in OpenCL.

2020-10-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D89520#2334477 , @Anastasia wrote: >> I couldn't find any spec justification for accepting the kinds of cases >> that D20090 accepts, so a reference to >> where in the OpenCL specification >>

[PATCH] D89523: PR44406: Follow behavior of array bound constant folding in more recent versions of GCC.

2020-10-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D89523#2333939 , @efriedma wrote: > Also update the documentation? See > https://clang.llvm.org/docs/UsersManual.html#differences-between-various-standard-modes > . Done. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D89523: PR44406: Follow behavior of array bound constant folding in more recent versions of GCC.

2020-10-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 298705. rsmith added a comment. - Update User's Manual to more accurately describe GNU mode and Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89523/new/ https://reviews.llvm.org/D89523 Files:

[PATCH] D89523: PR44406: Follow behavior of array bound constant folding in more recent versions of GCC.

2020-10-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 298702. rsmith marked 2 inline comments as done. rsmith added a comment. - Update User's Manual to more accurately describe GNU mode and Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89523/new/

[PATCH] D89523: PR44406: Follow behavior of array bound constant folding in more recent versions of GCC.

2020-10-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith marked 3 inline comments as done. rsmith added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:5939 +ElemTy = TryToFixInvalidVariablyModifiedType(ElemTy, Context, + SizeIsNegative, Oversized); +if

[PATCH] D89523: PR44406: Follow behavior of array bound constant folding in more recent versions of GCC.

2020-10-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith created this revision. rsmith added a reviewer: eli.friedman. Herald added a project: clang. rsmith requested review of this revision. Old GCC used to aggressively fold VLAs to constant-bound arrays at block scope in GNU mode. That's non-conforming, and more modern versions of GCC only do

[PATCH] D89520: Don't permit array bound constant folding in OpenCL.

2020-10-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith created this revision. rsmith added reviewers: Anastasia, bader, yaxunl. Herald added subscribers: kerbowa, nhaehnle, jvesely. Herald added a project: clang. rsmith requested review of this revision. Permitting non-standards-driven "do the best you can" constant-folding of array bounds is

[PATCH] D74130: [clang] fix consteval call in default arguments

2020-10-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/include/clang/Parse/Parser.h:2928 + SmallVectorImpl , + SourceLocation , bool InConstantConstext = false); void ParseBracketDeclarator(Declarator ); Comment at:

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

2020-10-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. If the answer is that we consider (2) to be incorrect, but that this is only a partial step in that direction, I think that's fine -- that's enough that we can know where the bug is if / when

[PATCH] D85802: [clang] Add -fc++-abi= flag for specifying which C++ ABI to use

2020-10-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D85802#2333105 , @leonardchan wrote: > In D85802#2332895 , @phosek wrote: > >> In D85802#2332808 , @rnk wrote: >> >>> Would

[PATCH] D20090: [OPENCL] Fix wrongly vla error for OpenCL array.

2020-10-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: cfe/trunk/lib/Sema/SemaType.cpp:2067 + S.LangOpts.GNUMode || + S.LangOpts.OpenCL).isInvalid(); } This looks wrong to me. The OpenCL

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

2020-10-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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

2020-10-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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

2020-10-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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.

2020-10-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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.

2020-10-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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

2020-10-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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.

2020-10-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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.

2020-10-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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.

2020-10-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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.

2020-10-14 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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

2020-10-13 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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.

2020-10-13 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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.

2020-10-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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.

2020-10-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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

2020-10-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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.

2020-10-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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.

2020-10-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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

  1   2   3   4   5   6   7   8   9   10   >