[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] 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] 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] 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] 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] 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] 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] 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] 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] 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.

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] 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. 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] 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 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] 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] 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] 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] 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. 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] 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] 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] 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 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 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] 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] 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] 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] 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] 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] 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.

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.

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] 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] 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] 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] 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] 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] 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 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] D57315: [opaque pointer types] Add a FunctionCallee wrapper type, and use it.

This revision was automatically updated to reflect the committed changes. Closed by commit rC352791: [opaque pointer types] Add a FunctionCallee wrapper type, and use it. (authored by jyknight, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL352535: Fix the behavior of clang's -w flag. (authored by jyknight, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D53199?vs=183966&id=184142#

[PATCH] D57330: Adjust documentation for git migration.

This revision was automatically updated to reflect the committed changes. jyknight marked 2 inline comments as done. Closed by commit rC352514: Adjust documentation for git migration. (authored by jyknight, committed by ). Changed prior to commit: https://reviews.llvm.org/D57330?vs=183891&id=18

[PATCH] D57330: Adjust documentation for git migration.

jyknight marked 8 inline comments as done. jyknight added a comment. In D57330#1375096 , @labath wrote: > I am not sure we should be recommending to people to place the build folder > under the llvm-project checkout. Is that how people use the monorepo bu

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

jyknight added inline comments. Comment at: clang/lib/Basic/DiagnosticIDs.cpp:460-463 + // Honor -w: this disables all messages mapped to Warning severity, and *also* + // any diagnostics which are not Error/Fatal by default (that is, they were + // upgraded by any of the mec

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

jyknight updated this revision to Diff 183966. jyknight marked 2 inline comments as done. jyknight added a comment. Fix to not disable remarks, reword comment, adjust implementation-of-module.m test-case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53199/new/ https://reviews.llvm.org

[PATCH] D57330: Adjust documentation for git migration.

jyknight updated this revision to Diff 183891. jyknight added a comment. Fix some warnings I added. Restore TestSuiteMakefileGuide.rst, which apparently isn't 100% obsolete. (But it is incorrect, and I'm not sure exactly how to fix it, so I just left a FIXME). CHANGES SINCE LAST ACTION https:

[PATCH] D57330: Adjust documentation for git migration.

jyknight created this revision. jyknight added reviewers: jlebar, rnk, mehdi_amini. Herald added subscribers: jsji, jfb, arphaman, christof, delcypher, hiraditya, nhaehnle, jvesely, nemanjai, kubamracek, arsenm. Herald added a reviewer: bollu. Herald added a reviewer: serge-sans-paille. This fixe

[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 https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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] 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] 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. 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] 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] 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] 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#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#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#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] 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] 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] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

jyknight created this revision. jyknight added reviewers: rsmith, chandlerc. Herald added a subscriber: arphaman. This warning, Wexperimental-driver-option, is on by default, exempt from -Werror, and may be disabled. Additionally, change the documentation to note that these options are not intend

[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 uweigand wrote: > jyknight wrote: > > Can you fix clang to con

[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 Can you fix clang to consistently define `__SIZEOF_FLOAT128__`

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

jyknight updated this revision to Diff 169641. jyknight marked 11 inline comments as done. jyknight added a comment. Address most comments. https://reviews.llvm.org/D52939 Files: clang/include/clang/AST/Expr.h clang/lib/AST/ExprConstant.cpp Index: clang/lib/AST/ExprConstant.cpp ===

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

jyknight added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:12-32 // Constant expression evaluation produces four main results: // // * A success/failure flag indicating whether constant folding was successful. //This is the 'bool' return value used by mo

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

jyknight created this revision. jyknight added a reviewer: rsmith. It is intended to disable _all_ warnings, even those upgraded to errors via `-Werror=warningname` or `#pragma clang diagnostic error' https://reviews.llvm.org/D53199 Files: clang/lib/Basic/DiagnosticIDs.cpp clang/test/Fronte

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

This revision was automatically updated to reflect the committed changes. Closed by commit rC344110: ExprConstant: Make __builtin_object_size use EM_IgnoreSideEffects. (authored by jyknight, committed by ). Herald added a subscriber: kristina. Changed prior to commit: https://reviews.llvm.org/D

[PATCH] D52703: Allow ifunc resolvers to accept arguments

jyknight accepted this revision. jyknight added a comment. This revision is now accepted and ready to land. Given that there's no technical reason for the compiler to prohibit this (it was just clang trying to diagnose a probable user-error, which turns out to not be as probable as ), this seems

[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] 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] 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] 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] 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] 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] 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] 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] 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] 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] D49927: [libc++] Exclude posix_l/strtonum fallback inclusion for newlib > 2.4

jyknight accepted this revision. jyknight added a comment. This revision is now accepted and ready to land. Typo in commit message? They were added to 2.5, not 2.4 (the code is right, just the comment is wrong). Repository: rCXX libc++ https://reviews.llvm.org/D49927 _

[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] 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 added a comment. In https://reviews.llvm.org/D47894#1125961, @srhines wrote: > In https://reviews.llvm.org/D47894#1125728, @jyknight wrote: > > > In https://reviews.llvm.org/D47894#1125653, @efriedma wrote: > > > > > The problem would come from propagating nonnull-ness from something whi

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

jyknight added a comment. In https://reviews.llvm.org/D47894#1125653, @efriedma wrote: > The problem would come from propagating nonnull-ness from something which > isn't inherently nonnull. For example, strlen has a nonnull argument, so > `strlen(NULL)` is UB, therefore given `int z = strlen(

[PATCH] D47137: [Sparc] Add floating-point register names

jyknight added inline comments. Comment at: lib/Basic/Targets/Sparc.cpp:32 +"f22", "f23", "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31", "f32", +"f33", "f34", "f35", "f36", "f37", "f38", "f39", "f40", "f41", "f42", "f43", +"f44", "f45", "f46", "f47", "f48",

[PATCH] D47138: [Sparc] Use the leon arch for Leon3's when using an external assembler

jyknight accepted this revision. jyknight added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D47138#1107637, @dcederman wrote: > I did not find a good way to access the SparcCPUInfo struct from here. No > other arch under Toolchains seems to access Targ

[PATCH] D47138: [Sparc] Use the leon arch for Leon3's when using an external assembler

jyknight added a comment. There are more leon-based v8 CPUs listed in clang/lib/Basic/Targets/Sparc.cpp, which need to be listed here, also. Separately, I think it'd be a good idea to refactor to put this info into the SparcCPUInfo struct, so that it's harder for them to get out of sync. Repo

[PATCH] D47137: [Sparc] Add floating-point register names

jyknight added a comment. Can you add a test that we support this? clang/test/CodeGen/sparcv8-inline-asm.c would be a good place to add a test case similar to that in clang/test/CodeGen/aarch64-inline-asm.c : test_gcc_registers. Repository: rC Clang https://reviews.llvm.org/D47137 _

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

jyknight added a comment. In https://reviews.llvm.org/D42933#1091817, @jfb wrote: > In https://reviews.llvm.org/D42933#1091809, @jyknight wrote: > > > I also think that special casing these two specifiers doesn't make sense. > > The problem is a general issue -- and one I've often found irritati

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

jyknight added a comment. In https://reviews.llvm.org/D42933#1090384, @jfb wrote: > In https://reviews.llvm.org/D42933#1090286, @smeenai wrote: > > > I'd be fine with adding an option to relax the printf checking if the size > > and alignment of the specifier and the actual type match, even if t

[PATCH] D42742: [clangd] Use pthread instead of thread_local to support more runtimes.

jyknight added a comment. Is there some way to figure out what's going on in clang-x86_64-linux-selfhost-modules? I believe there should be no linux builders which are missing this function -- it was added to libgcc in 4.8, and we don't support older versions, so I think missing this function

[PATCH] D35755: [Solaris] gcc toolchain handling revamp

jyknight accepted this revision. jyknight added a comment. This revision is now accepted and ready to land. This looks reasonable on the face of it. I'm assuming you know the layout for Solaris, and it doesn't seem to change the behavior of non-Solaris, so LGTM. https://reviews.llvm.org/D35755

[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

jyknight added a comment. I'd still _very_ much like a solution that is acceptable for all libc to use, and have that on by default. However, I guess this is fine. Only concern I have is that it seems odd that so many test-cases needed to be changed. What fails without those test changes? ht

[PATCH] D37954: Try to shorten system header paths when using -MD depfiles

jyknight added a comment. I think the diagnosis on the original issue was incorrect. It seems to me that it was caused by the prefix being set as "/bin" instead of "/usr/bin", because clang _doesn't_ actually canonicalize its prefix, even when -no-canonical-prefixes isn't specified! All it does

[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

jyknight added a comment. In https://reviews.llvm.org/D34158#838454, @mibintc wrote: > This patch is responding to @jyknight 's concern about preprocessed input. > The patch as it stands doesn't have this issue. I added 2 test cases, one > using option -x cpp-output, and another for a source fi

[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

jyknight added a comment. Just to restate: the ideal outcome of this discussion for me would be to resolve things such that _ALL_ libc implementations will feel comfortable using this technique to provide the C11-required predefined macros. I'd love for linux, freebsd, macos, solaris, etc etc l

[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

jyknight added a comment. In https://reviews.llvm.org/D34158#827178, @joerg wrote: > I had a long discussion with James about this on IRC without reaching a clear > consensus. I consider forcing this behavior on all targets to be a major bug. > It should be opt-in and opt-in only: > > (1) The h

[PATCH] D34158: For standards compatibility, preinclude if the file is available

jyknight added a comment. In https://reviews.llvm.org/D34158#823316, @fedor.sergeev wrote: > Hmm... I tried this patch and now the following worries me: > > - it passes -finclude-if-exists stdc-predef.h on all platforms (say, > including my Solaris platform that has no system stdc-predef.h) Ri

<    1   2   3   4   5   >