[PATCH] D55057: [Headers] Make max_align_t match GCC's implementation.

jyknight added inline comments. Comment at: lib/Headers/__stddef_max_align_t.h:40 __attribute__((__aligned__(__alignof__(long double; +#ifdef __i386__ + __float128 __clang_max_align_nonce3 EricWF wrote: > uweigand wrote: > > jyknight wrote: > > > uwei

[PATCH] D54547: PTH-- Remove feature entirely-

jyknight accepted this revision. jyknight added a comment. This revision is now accepted and ready to land. Since you've already submitted a fix to Boost, https://github.com/boostorg/build/commit/3385fe2aa699a45e722a1013658f824b6a7c761f, I think this is fine to remove. CHANGES SINCE LAST ACTIO

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

jyknight added a comment. In D55150#1318082 , @chandlerc wrote: > I think this should be `internal-driver-option` and the text updated? I don't > think these are necessarily experimental, they're internals of the compiler > implementation, and not a supp

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

jyknight added a comment. In D55150#1321046 , @kristina wrote: > Personally I'm against this type of warning as it's likely anyone using > `-mllvm` is actually intending to adjust certain behaviors across one or more > passes with a lot of switches suppo

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

jyknight added a comment. In D55150#1321705 , @kristina wrote: > > There is a cost to having people encode these flags into their build > > systems -- it can then cause issues if we ever change these internal flags. > > I do not think any Clang maintaine

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

jyknight added a comment. In D55150#1321759 , @george.karpenkov wrote: > Using `-Xclang` is the only way to pass options to the static analyzer, I > don't think we should warn on it. Well,, that seems unfortunate if we have the only supported interface

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

jyknight added a comment. In D55150#1321829 , @efriedma wrote: > I'm not sure that putting a warning that can be disabled really helps here; > anyone who needs the option will just disable the warning anyway, and then > users adding additional options so

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

jyknight added a comment. In D54355#1327237 , @craig.topper wrote: > Here's the test case that we have https://reviews.llvm.org/P8123 gcc seems > to accept it at O0 Smaller test case: extern unsigned long long __sdt_unsp; void foo() { __a

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

jyknight added a comment. The intricate initialization-order workarounds apparently don't work in all build modes, so I've updated this code to have constexpr functions and initializations in r355278. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57914/new/ http

[PATCH] D58154: Add support for -fpermissive.

jyknight added a comment. Hm -- in GCC, -fpermissive has nothing at all to do with -pedantic/-pedantic-errors, but I suppose it should be fine to do this way. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58154/new/ https://reviews.llvm.org/D58154 ___

[PATCH] D58154: Add support for -fpermissive.

jyknight added a comment. The errors disabled by this feature are default-error warnings -- you can already get the same effect by using -Wno-. Why is it bad to additionally allow -fpermissive to disable them? (If we have any diagnostics which are currently default-on-warnings which should not

[PATCH] D58154: Add support for -fpermissive.

jyknight added a comment. Ah -- I now understand your concern, and managing user expectations appropriately does seem like a potential concern. I did not look too closely before into what exactly GCC categorized as "permerror" (errors that can be disabled with -fpermissive) vs what Clang categ

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

jyknight added a comment. Sorry, forgot to re-ping this in a timely manner. :) The discussion on mailing list concluded positively, so waiting for an LG here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58548/new/ https://reviews.llvm.org/D58548

[PATCH] D59346: [X86] Add gcc rotate intrinsics to ia32intrin.h

jyknight accepted this revision. jyknight added a comment. Looks good. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59346/new/ https://reviews.llvm.org/D59346 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

This revision was automatically updated to reflect the committed changes. Closed by commit rC356789: IR: Support parsing numeric block ids, and emit them in textual output. (authored by jyknight, committed by ). Changed prior to commit: https://reviews.llvm.org/D58548?vs=187994&id=191913#toc R

[PATCH] D59711: PR41183: Don't emit Wstrict-prototypes warning for an implicit function declaration.

jyknight created this revision. jyknight added reviewers: rsmith, arphaman. Herald added a project: clang. Herald added a subscriber: cfe-commits. This case should emit an -Wimplicit-function-declaration warning. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D59711 Files: cl

[PATCH] D59509: Make static constructors + destructors minsize + cold (except for in -O0)

jyknight added a comment. Looks reasonable to me. Comment at: clang/test/CodeGen/static-attr.cpp:4 + +// WITHOUT-NOT: cold minsize noinline +// WITH: define internal void @__cxx_global_var_init() [[ATTR:#[0-9]]] lebedev.ri wrote: > This is fragile, it may have

[PATCH] D59711: PR41183: Don't emit Wstrict-prototypes warning for an implicit function declaration.

jyknight added a comment. Herald added a subscriber: dexonsmith. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59711/new/ https://reviews.llvm.org/D59711 ___ cfe-commits mailing list cfe-commits@

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

jyknight added inline comments. Comment at: lld/ELF/InputFiles.cpp:402 + if (Config->DepLibs) +if (fs::exists(Specifier)) + Driver->addFile(Specifier, /*WithLOption=*/false); bd1976llvm wrote: > jyknight wrote: > > bd1976llvm wrote: > > > jyknight wrote

[PATCH] D59711: PR41183: Don't emit Wstrict-prototypes warning for an implicit function declaration.

jyknight added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59711/new/ https://reviews.llvm.org/D59711 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

jyknight added a comment. There shouldn't be an empty string ("") in the asm output -- that should be a reference to the "l_yes" label, not the empty string. That seems very weird... Even odder: running clang -S on the above without -fno-integrated-as emits a ".quad .Ltmp00" (note the extra "0"

[PATCH] D59711: PR41183: Don't emit Wstrict-prototypes warning for an implicit function declaration.

This revision was automatically updated to reflect the committed changes. Closed by commit rC360084: PR41183: Don't emit strict-prototypes warning for an implicit function (authored by jyknight, committed by ). Changed prior to commit: https://reviews.llvm.org/D59711?vs=191924&id=198341#toc Re

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

jyknight created this revision. jyknight added a reviewer: rsmith. Previously, it had been using CK_BitCast even for casts that only change const/restrict/volatile. Now it will use CK_Noop where appropriate. This is an alternate solution to r336746. https://reviews.llvm.org/D52918 Files: cla

[PATCH] D52919: Emit diagnostic note when calling an invalid function declaration.

jyknight created this revision. jyknight added a reviewer: rsmith. The comment said it was intentionally not emitting any diagnostic because the declaration itself was already diagnosed. However, everywhere else that wants to not emit a diagnostic without an extra note emits note_invalid_subexpr_i

[PATCH] D52924: Make __builtin_object_size use the EM_IgnoreSideEffects evaluation mode.

jyknight created this revision. jyknight added a reviewer: rsmith. And, since EM_OffsetFold is now unused, remove it. While builtin_object_size intends to ignore the presence of side-effects in its argument, the EM_OffsetFold mode was NOT configured to ignore side-effects. Rather it was effective

[PATCH] D52926: Rename EM_ConstantExpressionUnevaluated to EM_ConstantFoldUnevaluated, which is actually what the code is currently doing.

jyknight created this revision. jyknight added a reviewer: rsmith. And, configure it to explicitly request equivalent behavior to ConstantFold, instead of getting that behavior accidentally. While it seems that diagnose_if and enable_if were intended to require a constexpr argument, the code was

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

jyknight added a comment. In https://reviews.llvm.org/D52918#1256420, @aaron.ballman wrote: > Patch is missing tests -- perhaps you could dump the AST and check the > casting notation from the output? It would appear that which casts get emitted in C mode is almost completely untested. I adde

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

jyknight updated this revision to Diff 168477. jyknight added a comment. Added test. https://reviews.llvm.org/D52918 Files: clang/lib/AST/ExprConstant.cpp clang/lib/Sema/SemaExpr.cpp clang/test/Sema/c-casts.c Index: clang/test/Sema/c-casts.c =

[PATCH] D52919: Emit diagnostic note when calling an invalid function declaration.

This revision was automatically updated to reflect the committed changes. Closed by commit rC343867: Emit diagnostic note when calling an invalid function declaration. (authored by jyknight, committed by ). Changed prior to commit: https://reviews.llvm.org/D52919?vs=168417&id=168490#toc Reposi

[PATCH] D52939: ExprConstant: Propagate correct return values from constant evaluation.

jyknight created this revision. jyknight added a reviewer: rsmith. The constant evaluation now returns false whenever indicating failure would be appropriate for the requested mode, instead of returning "true" for success, and depending on the caller examining the various status variables after th

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

This revision was automatically updated to reflect the committed changes. Closed by commit rC343892: Emit CK_NoOp casts in C mode, not just C++. (authored by jyknight, committed by ). Changed prior to commit: https://reviews.llvm.org/D52918?vs=168477&id=168541#toc Repository: rL LLVM https:

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

This revision was automatically updated to reflect the committed changes. Closed by commit rL343892: Emit CK_NoOp casts in C mode, not just C++. (authored by jyknight, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D52918?vs=168477&id=

[PATCH] D57664: [opaque pointer types] Fix the CallInfo passed to EmitCall in some edge cases.

jyknight created this revision. jyknight added reviewers: dblaikie, rsmith. Herald added a subscriber: Anastasia. Herald added a project: clang. Currently, EmitCall emits a call instruction with a function type derived from the pointee-type of the callee. This *should* be the same as the type crea

[PATCH] D57668: [opaque pointer types] Pass function types for runtime function calls.

jyknight marked an inline comment as done. jyknight added inline comments. Comment at: clang/lib/CodeGen/CGObjC.cpp:1028-1030 +llvm::Constant *getPropertyFn = cast( +CGM.getObjCRuntime().GetPropertyGetFunction().getCallee()); if (!getPropertyFn) { ---

[PATCH] D57664: [opaque pointer types] Fix the CallInfo passed to EmitCall in some edge cases.

jyknight marked an inline comment as done. jyknight added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:3837 +// having pointee types). +llvm::FunctionType *IRFuncTyFromInfo = getTypes().GetFunctionType(CallInfo); +assert(IRFuncTy == IRFuncTyFromInfo); --

[PATCH] D57664: [opaque pointer types] Fix the CallInfo passed to EmitCall in some edge cases.

This revision was automatically updated to reflect the committed changes. Closed by commit rC353181: [opaque pointer types] Fix the CallInfo passed to EmitCall in some (authored by jyknight, committed by ). Changed prior to commit: https://reviews.llvm.org/D57664?vs=184975&id=185313#toc Reposi

[PATCH] D57767: [opaque pointer types] Cleanup CGBuilder's Create*GEP.

jyknight created this revision. jyknight added a reviewer: dblaikie. Herald added subscribers: cfe-commits, jfb, jholewinski. Herald added a project: clang. The various EltSize, Offset, DataLayout, and StructLayout arguments are all computable from the Address's element type and the DataLayout whi

[PATCH] D57794: Fix MSVC constructor call extension after b92d290e48e9 (r353181).

jyknight created this revision. jyknight added reviewers: thakis, rsmith. Herald added subscribers: cfe-commits, mstorsjo. Herald added a project: clang. The assert added to EmitCall there was triggering in Windows Chromium builds, due to a mismatch of the return type. The MSVC constructor call e

[PATCH] D57794: Fix MSVC constructor call extension after b92d290e48e9 (r353181).

This revision was automatically updated to reflect the committed changes. Closed by commit rL353246: Fix MSVC constructor call extension after b92d290e48e9 (r353181). (authored by jyknight, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to com

[PATCH] D57801: [opaque pointer types] Pass through function types for TLS initialization and global destructor calls.

jyknight created this revision. jyknight added a reviewer: dblaikie. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D57801 Files: clang/lib/CodeGen/CGCXX.cpp clang/lib/CodeGen/CGCXXABI.h clang/lib/CodeGe

[PATCH] D57804: [opaque pointer types] Make EmitCall pass Function Types to CreateCall/Invoke.

jyknight created this revision. jyknight added a reviewer: dblaikie. Herald added a project: clang. Herald added a subscriber: cfe-commits. Also, remove the getFunctionType() function from CGCallee, since it accesses the pointee type of the value. The only use was in EmitCall, so just inline it in

[PATCH] D57801: [opaque pointer types] Pass through function types for TLS initialization and global destructor calls.

This revision was automatically updated to reflect the committed changes. Closed by commit rC353355: [opaque pointer types] Pass through function types for TLS (authored by jyknight, committed by ). Changed prior to commit: https://reviews.llvm.org/D57801?vs=185467&id=185677#toc Repository:

[PATCH] D57804: [opaque pointer types] Make EmitCall pass Function Types to CreateCall/Invoke.

This revision was automatically updated to reflect the committed changes. Closed by commit rC353356: [opaque pointer types] Make EmitCall pass Function Types to (authored by jyknight, committed by ). Changed prior to commit: https://reviews.llvm.org/D57804?vs=185476&id=185678#toc Repository:

[PATCH] D57767: [opaque pointer types] Cleanup CGBuilder's Create*GEP.

This revision was automatically updated to reflect the committed changes. Closed by commit rL353629: [opaque pointer types] Cleanup CGBuilder's Create*GEP. (authored by jyknight, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: http

[PATCH] D58120: [Builtins] Treat `bcmp` as a builtin.

jyknight accepted this revision. jyknight added a comment. This revision is now accepted and ready to land. Looks reasonable to me. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58120/new/ https://reviews.llvm.org/D58120 ___

[PATCH] D58091: Customize warnings for missing built-in type

jyknight added a comment. I think this warning (-Wbuiltin-requires-header) doesn't really make sense as its own warning. We already have two related (on-by-default) warnings. For declarations: test.c:1:6: warning: incompatible redeclaration of library function 'exit' [-Wincompatible-library

[PATCH] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

jyknight added inline comments. Comment at: lib/Basic/Targets/RISCV.h:94 +if (HasA) + MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 32; + } MaxAtomicPromoteWidth is an incompatible ABI-change kind of thing. We should set that to the maximum atomic siz

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

jyknight created this revision. jyknight added a reviewer: philip.pfaffe. Herald added subscribers: cfe-commits, jdoerfert, jfb, dexonsmith, steven_wu, hiraditya, mehdi_amini. Herald added projects: clang, LLVM. Just as as llvm IR supports explicitly specifying numeric value ids for instructions,

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

jyknight updated this revision to Diff 187963. jyknight added a comment. Add some wording to LangRef and clang-format. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58548/new/ https://reviews.llvm.org/D58548 Files: clang/test/CodeGenCXX/discard-

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

jyknight added a comment. In D58548#1407164 , @dexonsmith wrote: > I like this idea, and I don’t think the textual IR central is too important. > A few things: > > - Changes to the IR should always be discussed on llvm-dev. Did this already > happen?

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

jyknight updated this revision to Diff 187994. jyknight marked 4 inline comments as done. jyknight added a comment. Minor tweaks per comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58548/new/ https://reviews.llvm.org/D58548 Files: clang/

[PATCH] D62035: [AST] const-ify ObjC inherited class search

jyknight added a comment. I don't really have much to say about this, and the patch is probably fine, but I do note that most of the other accessors on this class also return mutable objects. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62035/new/ https://revi

[PATCH] D60748: Fix i386 struct and union parameter alignment

jyknight added a comment. I don't think this was correct (where by "correct", there, I mean "what GCC does", as this patch is intended to match GCC behavior). I think this change may well break more cases than it fixes, so IMO, this should be reverted, until it's implemented properly. Consider

[PATCH] D64487: [clang, test] Fix Clang :: Headers/max_align.c on 64-bit SPARC

jyknight accepted this revision. jyknight added a comment. This revision is now accepted and ready to land. LGTM. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64487/new/ https://reviews.llvm.org/D64487 ___ cfe-commi

[PATCH] D64793: [Driver] Properly use values-X[ca].o, values-xpg[46].o on Solaris

jyknight added a comment. Is this really necessary? Users don't typically pass -std= to the driver for linking anyways (what do you even pass if you've compiled both C and C++ code?) so this seems a rather odd way to control behavior. How about instead just adding "values-xpg6.o" unconditionall

[PATCH] D64793: [Driver] Properly use values-X[ca].o, values-xpg[46].o on Solaris

jyknight added a comment. > I fear it is necessary: at least it matches documented behaviour of both the > Sun/Oracle Studio compilers and gcc. I will defer to your opinion here. But -- one last attempt at dissuading you. :) Is this really doing something _important_, or is it just legacy cruft

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

jyknight added a comment. I'm not sure the history behind why these were added as default-on warningsthey don't really seem appropriate as default warnings to me, either. But I think someone else probably ought to approve the change. Repository: rC Clang CHANGES SINCE LAST ACTION htt

[PATCH] D65582: IR: accept and print numbered %N names for function args

jyknight added a comment. Herald added a subscriber: wuzish. +1 for doing this. I started looking at fixing this when I modified the printer to print proper labels for numbered basic-blocks (instead of comments), but I didn't do so because of the amount of test churn was off-putting. I think th

[PATCH] D62268: Add back --sysroot support for darwin header search.

jyknight created this revision. jyknight added reviewers: ldionne, jfb. Herald added subscribers: cfe-commits, dexonsmith. Herald added a project: clang. Before e97b5f5cf37e ([clang][Darwin] Refactor header search path logic i

[PATCH] D62268: Add back --sysroot support for darwin header search.

jyknight added a comment. In D62268#1512672 , @ldionne wrote: > This LGTM. > > When I did the refactor, all the code was only using `-isysroot` (and never > accessing `--sysroot`), so I thought only `-isysroot` was relevant on Darwin. > Seems like I was

[PATCH] D62268: Add back --sysroot support for darwin header search.

This revision was automatically updated to reflect the committed changes. Closed by commit rC361429: Add back --sysroot support for darwin header search. (authored by jyknight, committed by ). Changed prior to commit: https://reviews.llvm.org/D62268?vs=200813&id=200815#toc Repository: rC Cla

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

jyknight added a comment. In D54355#1328557 , @void wrote: > The issue is that "`n`" is expecting an immediate value, but the result of > the ternary operator isn't calculated by the front-end, because it doesn't > "know" that the evaluation of `__builti

[PATCH] D55616: Emit ASM input in a constant context

jyknight added a comment. This seems like a good start, but not complete. "n" and "i" both should require that their argument is a constant expression. For "n", it actually must be an immediate constant integer, so setRequiresImmediate() should be used there. For "i", you may use an lvalue con

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

jyknight added a comment. That example cannot be expected to ever evaluate the expression as "1" -- it doesn't in GCC, nor should it in Clang. An asm constraint of "n" or "i" (but not, e.g., "nr") must require a constant expression, and evaluating the argument as a constant expression necessari

[PATCH] D53199: Fix the behavior of clang's -w flag.

jyknight added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53199/new/ https://reviews.llvm.org/D53199 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67573: Fix __atomic_is_lock_free's return type.

jyknight created this revision. jyknight added a reviewer: rsmith. Herald added subscribers: cfe-commits, jfb, kristof.beyls. Herald added a project: clang. It is specified to return a bool in GCC's documentation and implementation, but the clang builtin says it returns an int. This would be prett

[PATCH] D67982: [ObjC] Add some additional test cases around pointer conversions.

jyknight created this revision. jyknight added a reviewer: rjmccall. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is especially important for Objective-C++, which is entirely missing this testing at the moment. This annotates with "FIXME" the cases which I change in

[PATCH] D67983: [ObjC] Diagnose implicit type coercion from ObjC 'Class' to object pointer types.

jyknight created this revision. jyknight added a reviewer: rjmccall. Herald added a project: clang. Herald added a subscriber: cfe-commits. For example, in Objective-C mode, the initialization of 'x' in: @implementation MyType + (void)someClassMethod { MyType *x = self; } @end is cor

[PATCH] D67983: [ObjC] Diagnose implicit type coercion from ObjC 'Class' to object pointer types.

jyknight added a comment. Note that the test-case diffs are on top of https://reviews.llvm.org/D67982, which I split out to make the actual change in behavior in this commit clearer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67983/new/ https:/

[PATCH] D67982: [ObjC] Add some additional test cases around pointer conversions.

jyknight added a comment. (See https://reviews.llvm.org/D67983 for the proposed behavior change.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67982/new/ https://reviews.llvm.org/D67982 ___ cfe-commit

[PATCH] D67573: Fix __atomic_is_lock_free's return type.

jyknight added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67573/new/ https://reviews.llvm.org/D67573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D68410: [AttrDocs] document always_inline

jyknight added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4399-4400 + let Content = [{ +Hint that inline substitution should be attempted when optimizations are +disabled. Does not guarantee that inline substitution actually occurs. +}];

[PATCH] D67847: [Support] make report_fatal_error `abort` instead of `exit`

jyknight added a comment. The `abort()` function raises SIGABRT, for which the default behavior is to trigger a coredump. Do we actually want that behavior? Either `_exit()` (long available extension, which lld already uses) or `quick_exit()` (the new C standard way) seem possibly preferable?

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

jyknight added a comment. In https://reviews.llvm.org/D47894#1158811, @manojgupta wrote: > @efriedma @jyknight Does the change match your expectations where warnings > are still generated but codeGen does not emit nonnull attribute? Yes, this seems sensible IMO. Comment at:

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

jyknight accepted this revision. jyknight added a comment. This revision is now accepted and ready to land. Please refer to the commit for the LLVM half of the change in the commit message for this. LGTM, other than some minor suggestions for the help texts. Comment at: docs/

[PATCH] D28213: [Frontend] Correct values of ATOMIC_*_LOCK_FREE to match builtin

jyknight reopened this revision. jyknight added a comment. This revision is now accepted and ready to land. The close was due to phabricator problem, reopening. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D28213/new/ https://reviews.llvm.org/D2821

[PATCH] D67982: [ObjC] Add some additional test cases around pointer conversions.

This revision was automatically updated to reflect the committed changes. Closed by commit rG1c982af05997: [ObjC] Add some additional test cases around pointer conversions. (authored by jyknight). Changed prior to commit: https://reviews.llvm.org/D67982?vs=221586&id=225439#toc Repository: rG

[PATCH] D67983: [ObjC] Diagnose implicit type coercion from ObjC 'Class' to object pointer types.

This revision was automatically updated to reflect the committed changes. Closed by commit rGccc4d83cda16: [ObjC] Diagnose implicit type coercion from ObjC 'Class' to object pointer… (authored by jyknight). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D67983: [ObjC] Diagnose implicit type coercion from ObjC 'Class' to object pointer types.

jyknight added a comment. In D67983#1723376 , @thakis wrote: > After this, Class can no longer be used as a key type in an Obj-C dictionary > literal. Is that intentional? Such code was already an on by default incompatible-pointer-types warning in Obj

[PATCH] D67983: [ObjC] Diagnose implicit type coercion from ObjC 'Class' to object pointer types.

jyknight added a comment. In D67983#1723681 , @rjmccall wrote: > We could probably do a quick check to see if the class informally conforms to > the protocol. `+copyWithZone` seems to be marked unavailable in ARC; not > sure if that would cause problems

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

jyknight added a comment. In the commit message, you refer to the preferred alignemtn as the "true" alignment, but that's misleading. As discussed on the mailing list thread, it's not true alignment at all. Comment at: clang/include/clang/AST/RecordLayout.h:74 + /// The maxi

[PATCH] D80225: [Driver] Recognize -fuse-ld={bfd, gold, lld} but don't prepend "ld." or "ld64." for other values

jyknight added a comment. It's worrying to me that there number of places in LLVM that at the exact argument value of "-fuse-ld=". E.g. in the windows and PS4 toolchains. We already claim to support arbitrary values and full paths, but if you specify "-fuse-ld=/path/to/lld-link" on Windows toda

[PATCH] D80417: Fix Darwin 'constinit thread_local' variables.

jyknight created this revision. jyknight added a reviewer: rjmccall. Herald added a project: clang. Herald added a subscriber: cfe-commits. Unlike other platforms using ItaniumCXXABI, Darwin does not allow the creation of a thread-wrapper function for a variable in the TU of users. Because of this

[PATCH] D80417: Fix Darwin 'constinit thread_local' variables.

This revision was automatically updated to reflect the committed changes. jyknight marked an inline comment as done. Closed by commit rGaca3d067efe1: Fix Darwin 'constinit thread_local' variables. (authored by jyknight). Changed prior to commit: https://reviews.llvm.org/D80417?vs=265642&id=2665

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

jyknight added a comment. Why not have clang always emit atomicrmw for floats, and let AtomicExpandPass handle legalizing that into integer atomics if necessary, rather than adding a target hook in clang? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71726/new/ https://reviews.llvm.or

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

jyknight added a comment. In D71726#2165445 , @yaxunl wrote: > In D71726#2165424 , @jyknight wrote: > > > Why not have clang always emit atomicrmw for floats, and let > > AtomicExpandPass handle legalizing that int

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

jyknight created this revision. jyknight added a reviewer: rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. ItaniumCXXABI::usesThreadWrapperFunction calls VarDecl::needsDestruction, which calls QualType::isDestructedType, which checks CXXRecordDecl::hasTrivialDestruct

[PATCH] D81432: Add #includes so that ROCm.h is compilable stand-alone.

jyknight added a comment. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81432/new/ https://reviews.llvm.org/D81432 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/c

[PATCH] D81772: Reduce -Wregister from DefaultError to a default-on warning.

jyknight created this revision. jyknight added a reviewer: rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. There is a lot of C++ code in the wild still using 'register', which will become broken if we switch the default compilation mode to C++17. A default-on warning

[PATCH] D83015: [Driver] Add -fld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

jyknight added a comment. BTW, I just noticed recently that we have a -mlinker-version= flag, too, which is only used on darwin at the moment. That's another instance of "we need to condition behavior based on what linker we're invoking", but in this case, between multiple versions of apple's l

[PATCH] D82777: Clang Driver: Use Apple ld64's new @response-file support.

jyknight created this revision. jyknight added reviewers: steven_wu, arphaman. Herald added subscribers: cfe-commits, dexonsmith. Herald added a project: clang. jyknight added a child revision: D82782: Clang Driver: refactor support for writing response files to be specified at Command creation, r

[PATCH] D82782: Clang Driver: refactor support for writing response files to be specified at Command creation, rather than as part of the Tool.

jyknight created this revision. jyknight added reviewers: rnk, arphaman, steven_wu. Herald added subscribers: cfe-commits, sstefan1, kerbowa, luismarques, apazos, sameer.abuasal, pzheng, s.egerton, lenary, Jim, mstorsjo, jocewei, PkmX, dexonsmith, the_o, brucehoult, MartinMosbeck, rogfer01, edwar

[PATCH] D82777: Clang Driver: Use Apple ld64's new @response-file support.

This revision was automatically updated to reflect the committed changes. Closed by commit rG381df1653c92: Clang Driver: Use Apple ld64's new @response-file support. (authored by jyknight). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82777/new/ ht

[PATCH] D82782: Clang Driver: refactor support for writing response files to be specified at Command creation, rather than as part of the Tool.

This revision was automatically updated to reflect the committed changes. Closed by commit rG4772b99dffec: Clang Driver: refactor support for writing response files to be specified at… (authored by jyknight). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

jyknight added a comment. Hi, it seems that this change is incompatible with the `[NSItemProvider loadItemForTypeIdentifier:options:completionHandler:]` method's expected (but extremely weird and unusual!) usage. Per https://developer.apple.com/documentation/foundation/nsitemprovider/1403900-l

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

jyknight added a comment. In D71726#2182667 , @tra wrote: >> If a target would like to treat single and double fp atomics as unsupported, >> it can override the default behavior in its own TargetInfo. I really don't think this should be a target option a

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

jyknight added a comment. Oh, one more note, C11 has -- and clang already supports -- `_Atomic long double x; x += 4;` via lowering to a cmpxchg loop. Now that we have an LLVM IR representation for atomicrmw fadd/fsub, clang should be lowering the _Atomic += to that, too. (Doesn't need to be in

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

jyknight added a comment. In D66831#2019125 , @vsapsai wrote: > Agree that is a mistake in `NSItemProvider` API. The solution I offer is not > to revert the change but to add cc1 flag > `-fcompatibility-qualified-id-block-type-checking` and to make the d

[PATCH] D79511: [ObjC] Add compatibility mode for type checking of qualified id block parameters.

jyknight added inline comments. Comment at: clang/test/SemaObjC/block-type-safety.m:170 +genericBlockWithParam = blockWithParam; +blockWithParam = genericBlockWithParam; // expected-error {{incompatible block pointer types assigning to 'void (^)(NSAllArray *)' from 'void

[PATCH] D79511: [ObjC] Add compatibility mode for type checking of qualified id block parameters.

jyknight accepted this revision. jyknight added a comment. This revision is now accepted and ready to land. It looks like you didn't squash your two commits before uploading, so the diff for review now only includes the changes for the comment, not the complete patch. Other than needing to squas

<    1   2   3   4   5   6   7   8   9   >