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

2018-05-08 Thread James Y Knight via Phabricator via cfe-commits
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] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

2018-05-08 Thread James Y Knight via Phabricator via cfe-commits
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] D47137: [Sparc] Add floating-point register names

2018-05-21 Thread James Y Knight via Phabricator via cfe-commits
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] D47138: [Sparc] Use the leon arch for Leon3's when using an external assembler

2018-05-21 Thread James Y Knight via Phabricator via cfe-commits
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] D47138: [Sparc] Use the leon arch for Leon3's when using an external assembler

2018-05-22 Thread James Y Knight via Phabricator via cfe-commits
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] D47137: [Sparc] Add floating-point register names

2018-05-23 Thread James Y Knight via Phabricator via cfe-commits
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] D37954: Try to shorten system header paths when using -MD depfiles

2017-10-21 Thread James Y Knight via Phabricator via cfe-commits
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

2017-10-24 Thread James Y Knight via Phabricator via cfe-commits
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] D49927: [libc++] Exclude posix_l/strtonum fallback inclusion for newlib > 2.4

2018-07-27 Thread James Y Knight via Phabricator via cfe-commits
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] D32146: PR32476: __nop_locale_mgmt.h not needed with newlib 2.5+

2017-06-14 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL305394: PR32476: __nop_locale_mgmt.h not needed with newlib 2.5+ (authored by jyknight). Changed prior to commit: https://reviews.llvm.org/D32146?vs=102265&id=102562#toc Repository: rL LLVM https://

[PATCH] D34105: Define _GNU_SOURCE for rtems c++

2017-06-14 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL305399: Define _GNU_SOURCE for rtems c++ (authored by jyknight). Changed prior to commit: https://reviews.llvm.org/D34105?vs=102188&id=102565#toc Repository: rL LLVM https://reviews.llvm.org/D34105

[PATCH] D34294: Rework libcxx strerror_r handling.

2017-06-16 Thread James Y Knight via Phabricator via cfe-commits
jyknight created this revision. Herald added subscribers: krytarowski, srhines. The set of #ifdefs used to handle the two incompatible variants of strerror_r were not complete (they didn't handle newlib appropriately). Rather than attempting to make the ifdefs more complex, make them unnecessary

[PATCH] D34294: Rework libcxx strerror_r handling.

2017-06-16 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In https://reviews.llvm.org/D34294#782806, @krytarowski wrote: > New one is harder to comprehend and less portable (usage of `__atribute__`). I can't disagree more strongly. This is fundamentally portable C++ code -- __attribute__((unused)) is simply warning suppress

[PATCH] D34294: Rework libcxx strerror_r handling.

2017-06-20 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. EricWF, wdyt? https://reviews.llvm.org/D34294 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-12-19 Thread James Y Knight via Phabricator via cfe-commits
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] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

2018-06-07 Thread James Y Knight via Phabricator via cfe-commits
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] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

2018-06-08 Thread James Y Knight via Phabricator via cfe-commits
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] D52703: Allow ifunc resolvers to accept arguments

2018-10-09 Thread James Y Knight via Phabricator via cfe-commits
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] D52924: Make __builtin_object_size use the EM_IgnoreSideEffects evaluation mode.

2018-10-09 Thread James Y Knight via Phabricator via cfe-commits
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] D53199: Fix the behavior of clang's -w flag.

2018-10-12 Thread James Y Knight via Phabricator via cfe-commits
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] D52939: ExprConstant: Propagate correct return values from constant evaluation.

2018-10-14 Thread James Y Knight via Phabricator via cfe-commits
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.

2018-10-14 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-01-24 Thread James Y Knight via Phabricator via cfe-commits
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] D57330: Adjust documentation for git migration.

2019-01-28 Thread James Y Knight via Phabricator via cfe-commits
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] D57330: Adjust documentation for git migration.

2019-01-28 Thread James Y Knight via Phabricator via cfe-commits
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] D53199: Fix the behavior of clang's -w flag.

2019-01-28 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-01-28 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-01-29 Thread James Y Knight via Phabricator via cfe-commits
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] D57330: Adjust documentation for git migration.

2019-01-29 Thread James Y Knight via Phabricator via cfe-commits
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] D53199: Fix the behavior of clang's -w flag.

2019-01-29 Thread James Y Knight via Phabricator via cfe-commits
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] D57315: [opaque pointer types] Add a FunctionCallee wrapper type, and use it.

2019-01-31 Thread James Y Knight via Phabricator via cfe-commits
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] D55057: [Headers] Make max_align_t match GCC's implementation.

2018-11-29 Thread James Y Knight via Phabricator via cfe-commits
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] D55057: [Headers] Make max_align_t match GCC's implementation.

2018-11-29 Thread James Y Knight via Phabricator via cfe-commits
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] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

2018-11-30 Thread James Y Knight via Phabricator via cfe-commits
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.

2018-12-03 Thread James Y Knight via Phabricator via cfe-commits
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-

2018-12-03 Thread James Y Knight via Phabricator via cfe-commits
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.

2018-12-05 Thread James Y Knight via Phabricator via cfe-commits
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.

2018-12-06 Thread James Y Knight via Phabricator via cfe-commits
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.

2018-12-06 Thread James Y Knight via Phabricator via cfe-commits
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.

2018-12-06 Thread James Y Knight via Phabricator via cfe-commits
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.

2018-12-06 Thread James Y Knight via Phabricator via cfe-commits
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

2018-12-11 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-03-02 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-03-04 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-03-05 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-03-06 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-03-18 Thread James Y Knight via Phabricator via cfe-commits
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

2019-03-18 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-03-22 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-03-22 Thread James Y Knight via Phabricator via cfe-commits
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)

2019-04-08 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-04-09 Thread James Y Knight via Phabricator via cfe-commits
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

2019-04-09 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-04-19 Thread James Y Knight via Phabricator via cfe-commits
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

2019-04-19 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-05-06 Thread James Y Knight via Phabricator via cfe-commits
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++.

2018-10-04 Thread James Y Knight via Phabricator via cfe-commits
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.

2018-10-04 Thread James Y Knight via Phabricator via cfe-commits
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.

2018-10-04 Thread James Y Knight via Phabricator via cfe-commits
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.

2018-10-04 Thread James Y Knight via Phabricator via cfe-commits
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++.

2018-10-05 Thread James Y Knight via Phabricator via cfe-commits
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++.

2018-10-05 Thread James Y Knight via Phabricator via cfe-commits
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.

2018-10-05 Thread James Y Knight via Phabricator via cfe-commits
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.

2018-10-05 Thread James Y Knight via Phabricator via cfe-commits
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++.

2018-10-05 Thread James Y Knight via Phabricator via cfe-commits
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++.

2018-10-05 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-02-03 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-02-04 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-02-04 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-02-05 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-02-05 Thread James Y Knight via Phabricator via cfe-commits
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).

2019-02-05 Thread James Y Knight via Phabricator via cfe-commits
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).

2019-02-05 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-02-05 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-02-05 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-02-06 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-02-06 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-02-09 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-02-13 Thread James Y Knight via Phabricator via cfe-commits
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

2019-02-13 Thread James Y Knight via Phabricator via cfe-commits
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

2019-02-19 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-02-22 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-02-22 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-02-22 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-02-22 Thread James Y Knight via Phabricator via cfe-commits
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

2019-05-30 Thread James Y Knight via Phabricator via cfe-commits
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

2019-05-30 Thread James Y Knight via Phabricator via cfe-commits
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

2019-07-23 Thread James Y Knight via Phabricator via cfe-commits
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

2019-07-23 Thread James Y Knight via Phabricator via cfe-commits
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

2019-07-29 Thread James Y Knight via Phabricator via cfe-commits
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

2019-07-29 Thread James Y Knight via Phabricator via cfe-commits
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

2019-08-01 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-05-22 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-05-22 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-05-22 Thread James Y Knight via Phabricator via cfe-commits
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

2018-12-12 Thread James Y Knight via Phabricator via cfe-commits
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

2018-12-12 Thread James Y Knight via Phabricator via cfe-commits
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

2018-12-12 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-01-10 Thread James Y Knight via Phabricator via cfe-commits
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.

2019-09-13 Thread James Y Knight via Phabricator via cfe-commits
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

  1   2   3   4   5   >