dexonsmith (Duncan P. N. Exon Smith)
User

Projects

User does not belong to any projects.

User Details

User Since
Mar 18 2014, 10:33 AM (213 w, 6 d)

Recent Activity

Yesterday

dexonsmith added a comment to D45772: [Minor patch] Fix IR Module Printing.

Another high-level note (besides adding tests): please resubmit the patch with full context (e.g., git diff -U9999999 HEAD^..).

Mon, Apr 23, 11:47 AM
dexonsmith resigned from D45975: [DebugInfo] Invalidate debug info in ReassociatePass::RewriteExprTree.
Mon, Apr 23, 11:15 AM · debug-info
dexonsmith added a comment to D30882: Add a callback for __has_include and use it for dependency scanning.

Would it be ok to turn this on by default, without a flag, only in the case of the path actually existing, and only the found path being the one we add to the .d?

I think that pessimizes some incremental builds:

  • You have a __has_include("missing.h"), but don't include missing.h.
  • Change "missing.h" (but don't delete it).
  • An incremental build now has to rebuild the object file, even though nothing will have changed.

    However, it's fixing an actual bug, so it makes sense to me to be more conservative.
Mon, Apr 23, 11:11 AM
dexonsmith added a comment to D30882: Add a callback for __has_include and use it for dependency scanning.

Oh, that actually wasn't my intention when I wrote it.

Honestly I didn't expect it to log anything for missing paths at all, as we don't currently log all the missing (but attempted) paths for regular #include's.

Mon, Apr 23, 9:55 AM
dexonsmith added a comment to D30882: Add a callback for __has_include and use it for dependency scanning.

I don't think this is quite right. I know at least make-based incremental builds wouldn't deal well with this.

This is actually not a novel problem w.r.t. this patch. The exact same situation comes up with Makefile-included .d files and when one of the referenced headers is removed.

This is typically solved somewhere in the build system, for example Make has -include, and Ninja and llbuild have explicit support for this situation.

Mon, Apr 23, 8:27 AM
dexonsmith requested changes to D45772: [Minor patch] Fix IR Module Printing.

That doesn’t seem like the right place for the newline; instead, it should be wherever the asterisks were printed.

Mon, Apr 23, 6:27 AM

Sun, Apr 22

dexonsmith requested changes to D30882: Add a callback for __has_include and use it for dependency scanning.

I don't think this is quite right. I know at least make-based incremental builds wouldn't deal well with this.

Sun, Apr 22, 6:54 PM
dexonsmith requested changes to D45930: [Support] Upstream anonymization and manipulating of BCSymbolMaps.

Thanks for working on this! I've just done a pass through for style, and I have a few nits to pick.

Sun, Apr 22, 4:39 PM

Mon, Apr 16

dexonsmith committed rC330177: Fix malformed table introduced by r330174.
Fix malformed table introduced by r330174
Mon, Apr 16, 10:51 PM
dexonsmith committed rL330177: Fix malformed table introduced by r330174.
Fix malformed table introduced by r330174
Mon, Apr 16, 10:51 PM
dexonsmith committed rL330174: Remove GC-related warning terminology.
Remove GC-related warning terminology
Mon, Apr 16, 9:28 PM
dexonsmith committed rC330174: Remove GC-related warning terminology.
Remove GC-related warning terminology
Mon, Apr 16, 9:28 PM

Fri, Apr 13

dexonsmith requested changes to D17741: adds __FILE_BASENAME__ builtin macro.

Hal requested splitting the patch in two, since the two features are separable, but they both still seem to be here. Perhaps start with the prefix patch?

Fri, Apr 13, 6:47 AM

Wed, Apr 11

dexonsmith accepted D44668: [demangler] Add a "partial" demangling API for LLDB.

I'd prefer to have this be as close as possible to the libcxxabi demangler (and use the same data structures) so that it's easy to detect when they diverge. If we ever move to monorepo hopefully there will be a way to factor them out into a single implementation.

Wed, Apr 11, 2:50 PM

Mon, Apr 9

dexonsmith added a reviewer for D45472: [MachO] Emit Weak ReadOnlyWithRel to ConstDataSection: kledzik.

Have you confirmed this doesn't need to depend on the deployment target? I.e., do we need the old logic sometimes when back-deploying?

Mon, Apr 9, 7:26 PM

Mon, Apr 2

dexonsmith accepted D45132: [Bitcode] Change std::sort to llvm::sort in response to r327219.

LGTM.

Mon, Apr 2, 2:53 PM
dexonsmith accepted D45143: [unittests] Change std::sort to llvm::sort in response to r327219.

LGTM.

Mon, Apr 2, 2:53 PM

Tue, Mar 27

dexonsmith accepted D44944: [Analysis] Change std::sort to llvm::sort in response to r327219.

LGTM.

Tue, Mar 27, 11:01 AM

Mar 22 2018

dexonsmith requested changes to D44753: [Preprocessor] Rename __is_{target -> host}_* function-like builtin macros.

I agree with Saleem and Bob: __is_target_* is not confusing here and seems to be a straightforward spelling. It has also already shipped in LLVM 6.0.0: it would be awkward to stop supporting this syntax.

Mar 22 2018, 8:47 PM

Mar 8 2018

dexonsmith accepted D44263: Implement LWG 2221 - No formatted output operator for nullptr.

LGTM.

Mar 8 2018, 5:57 PM

Feb 16 2018

dexonsmith added a reviewer for D43253: bitcode support change for fast flags compatibility: steven_wu.
Feb 16 2018, 9:01 AM

Feb 4 2018

dexonsmith accepted D41889: [libcxxabi][demangler] Clean up and llvm-ify the type parser.

LGTM!

Feb 4 2018, 7:46 AM

Jan 31 2018

dexonsmith requested changes to D41889: [libcxxabi][demangler] Clean up and llvm-ify the type parser.

If it's possible to separate the bugfix into a separate commit (either before or after), I think you should.

Jan 31 2018, 5:07 PM
dexonsmith accepted D41887: [libcxxabi][demangler] Clean up and llvm-ify the expression parser.

LGTM.

Jan 31 2018, 2:45 PM

Jan 29 2018

dexonsmith accepted D41885: [libcxxabi][demangler] Improve handling of variadic templates.

LGTM once you clean up the #if 0.

Jan 29 2018, 2:20 PM

Jan 22 2018

dexonsmith requested changes to D41885: [libcxxabi][demangler] Improve handling of variadic templates.

Thanks for working on this!

Jan 22 2018, 11:08 AM

Jan 11 2018

dexonsmith accepted D41423: [Lex] Avoid out-of-bounds dereference in LexAngledStringLiteral..
Jan 11 2018, 4:05 PM

Jan 10 2018

dexonsmith added inline comments to D41423: [Lex] Avoid out-of-bounds dereference in LexAngledStringLiteral..
Jan 10 2018, 3:28 PM
dexonsmith requested changes to D41423: [Lex] Avoid out-of-bounds dereference in LexAngledStringLiteral..
Jan 10 2018, 10:38 AM

Jan 4 2018

dexonsmith added inline comments to D41723: Introduce the "retpoline" x86 mitigation technique for variant #2 of the speculative execution vulnerabilities disclosed today, specifically identified by CVE-2017-5715, "Branch Target Injection", and is one of the two halves to Spectre...
Jan 4 2018, 3:33 PM

Jan 3 2018

dexonsmith added a comment to D41695: [Metadata] Extend 'count' field of DISubrange to take a metadata node.

Not sure if this requires an RFC on llvm-dev, but please let me know if it does.

Jan 3 2018, 12:34 PM

Dec 13 2017

dexonsmith added a comment to D41050: Fix over-release of return value of lambda implicitly converted to block/function pointer.

Heh, alright, that works. It's unfortunate that -disable-llvm-passes doesn't suppress running the pass under -O0; that seems like an oversight.

Anyway, LGTM.

Dec 13 2017, 9:47 PM

Dec 10 2017

dexonsmith resigned from D41050: Fix over-release of return value of lambda implicitly converted to block/function pointer.

Akira and/or John should take a look instead of me.

Dec 10 2017, 10:34 AM
dexonsmith added reviewers for D41050: Fix over-release of return value of lambda implicitly converted to block/function pointer: ahatanak, rjmccall.
Dec 10 2017, 10:29 AM

Dec 9 2017

dexonsmith committed rC320271: Update Clang CMake cache to use cxx-headers, NFC.
Update Clang CMake cache to use cxx-headers, NFC
Dec 9 2017, 3:45 PM
dexonsmith committed rL320271: Update Clang CMake cache to use cxx-headers, NFC.
Update Clang CMake cache to use cxx-headers, NFC
Dec 9 2017, 3:45 PM

Dec 8 2017

dexonsmith added a comment to D41035: [driver][darwin] Refactor the target selection code, NFC.

This seems correct to me. I wouldn't mind having someone else back me up though.

Dec 8 2017, 3:01 PM
dexonsmith closed D40891: Revert a change in propagateMassToSuccessors that summed redundant edges n^2 times.

Committed in r320208.

Dec 8 2017, 2:52 PM
dexonsmith committed rL320208: Revert part of "Cleanup some GraphTraits iteration code".
Revert part of "Cleanup some GraphTraits iteration code"
Dec 8 2017, 2:43 PM

Dec 7 2017

dexonsmith requested changes to D40998: [driver][darwin] Take the OS version specified in "-target" as the target OS instead of inferring it from SDK / environment.
Dec 7 2017, 7:31 PM
dexonsmith accepted D40891: Revert a change in propagateMassToSuccessors that summed redundant edges n^2 times.

LGTM, after you try to shrink the testcase. Thanks for tracking this down!

Dec 7 2017, 6:38 PM

Dec 6 2017

dexonsmith added a comment to D40891: Revert a change in propagateMassToSuccessors that summed redundant edges n^2 times.

I ran the patch through clang-format. But looking at the code again, I see that calling the getEdgeProbability overload with a successor index or iterator instead of the destination basic block actually does something different from the original code: passing a successor index/iterator gets the probability for a single edge, while passing a basic block sums the probability for all edges between the two blocks.

However, and I have to admit that I'm in over my head here, what the original code was doing doesn't seem correct. For each edge from A to B, it was summing the probability of all edges from A to B, then adding the sum to the mass for B. If there are N edges from A to B, and they have probabilities that sum to M, then this will distribute N*M mass to B instead of M. In my contrived case, N=196928.

I still think this change is good, but I didn't mean for it to change the results. What are your thoughts?

Dec 6 2017, 4:16 PM
dexonsmith requested changes to D40891: Revert a change in propagateMassToSuccessors that summed redundant edges n^2 times.
Dec 6 2017, 11:30 AM

Nov 6 2017

dexonsmith added inline comments to D39356: [ThinLTO] Fix missing call graph edges for calls with bitcasts..
Nov 6 2017, 6:26 PM

Nov 2 2017

dexonsmith added inline comments to D39356: [ThinLTO] Fix missing call graph edges for calls with bitcasts..
Nov 2 2017, 4:51 PM
dexonsmith added a comment to D39502: [Driver] Make clang/cc conforms to UNIX standard.

Also split out the testcase for UNIX conformance. That test is not related
to output file cleanup. Split it out to make it clear that is for UNIX
conformance.

Nov 2 2017, 4:04 PM
dexonsmith added inline comments to D39356: [ThinLTO] Fix missing call graph edges for calls with bitcasts..
Nov 2 2017, 3:50 PM

Nov 1 2017

dexonsmith added a comment to D39502: [Driver] Make clang/cc conforms to UNIX standard.

I have a few nitpicks, but otherwise this LGTM. I'd like to wait for someone on the CUDA side to confirm though.

Nov 1 2017, 2:03 PM

Oct 26 2017

dexonsmith added a comment to D39245: [ADT] Shuffle containers before sorting to uncover non-deterministic behavior.

That said, my point that we shouldn't have yet another macro stands, I guess, independently from the bucket we decide to throw these checks in.

Oct 26 2017, 10:59 AM
dexonsmith requested changes to D39245: [ADT] Shuffle containers before sorting to uncover non-deterministic behavior.
Oct 26 2017, 8:48 AM
dexonsmith added a comment to D39245: [ADT] Shuffle containers before sorting to uncover non-deterministic behavior.

I don't really like the ever growing set of configuration macros here.

I would much prefer we have one ABI-breaking check macro and just use that for everything here...

Thanks @chandlerc. I am fine with enabling this based on LLVM_ABI_BREAKING_CHECKS. However, this macro is ON by default for +asserts builds. This means all existing +asserts bots would start failing as soon as this patch gets merged since there are about 15 unit test failures. Do we want to gate patches if they show up sort order failures uncovered by shuffling?
My idea was to control this via a separate macro so that we can isolate shuffle+sort failures into a separate bot which may make it easier to fix without gating the regular flow. Thoughts?

I agree with Chandler that we should avoid the proliferation of macros and use ABI_BREAKING_CHECKS instead.

Oct 26 2017, 8:46 AM

Oct 25 2017

dexonsmith added inline comments to D39245: [ADT] Shuffle containers before sorting to uncover non-deterministic behavior.
Oct 25 2017, 9:12 AM

Oct 24 2017

dexonsmith added a comment to D39245: [ADT] Shuffle containers before sorting to uncover non-deterministic behavior.

std::sort and array_pod_sort both use quicksort

FWIW, most standards-compliant std::sort implementations use introsort... although it's also unstable.

NOTE: To randomly shuffle before std::sort we may have to change all calls to std::sort with llvm::sort.

It looks to me like the current overloads of llvm::sort take a parallel execution policy, which would be unfortunate boilerplate to add. Perhaps you should create an overload in ADT/STLExtras.h with the same signature as std::sort (that wraps std::sort with the initial call to std::random_shuffle).

Sure, I can add overload for llvm::sort which takes ExecutionPolicy and invoke std::sort.

Oct 24 2017, 4:32 PM
dexonsmith added a comment to D39245: [ADT] Shuffle containers before sorting to uncover non-deterministic behavior.

std::sort and array_pod_sort both use quicksort

Oct 24 2017, 2:17 PM

Oct 10 2017

dexonsmith added a dependent revision for D38773: [Sema] Add support for flexible array members in Obj-C.: D38774: [CodeGen] Add support for IncompleteArrayType in Obj-C ivars..
Oct 10 2017, 8:22 PM
dexonsmith edited dependencies for D38774: [CodeGen] Add support for IncompleteArrayType in Obj-C ivars., added: 1; removed: 1.
Oct 10 2017, 8:22 PM
dexonsmith removed a dependent revision for D38114: [Sema] Emit an error for using tags with flexible array member as ObjC class ivar.: D38774: [CodeGen] Add support for IncompleteArrayType in Obj-C ivars..
Oct 10 2017, 8:22 PM
dexonsmith added a dependent revision for D38114: [Sema] Emit an error for using tags with flexible array member as ObjC class ivar.: D38774: [CodeGen] Add support for IncompleteArrayType in Obj-C ivars..
Oct 10 2017, 8:20 PM
dexonsmith added a dependency for D38774: [CodeGen] Add support for IncompleteArrayType in Obj-C ivars.: D38114: [Sema] Emit an error for using tags with flexible array member as ObjC class ivar..
Oct 10 2017, 8:20 PM

Aug 29 2017

dexonsmith added a comment to D37281: Make DIExpression plain metadata and not an MDNode.

Yes, abandoning seems reasonable. I think the main idea of step 4 when I filed PR22780 was to reduce memory usage, but this didn't turn out to be a bottleneck.

Aug 29 2017, 5:40 PM

Aug 25 2017

dexonsmith accepted D36669: Build LLVM with -Wstrict-prototypes enabled.

LGTM.

Aug 25 2017, 8:38 AM

Aug 23 2017

dexonsmith added a comment to D37075: Parse and print DIExpressions inline to ease IR and MIR testing.

Nice!

Aug 23 2017, 1:29 PM

Aug 21 2017

dexonsmith accepted D36556: [IR] AutoUpgrade ModuleFlagBehavior for PIC and PIE level.

LGTM! Thanks for reworking this.

Aug 21 2017, 2:20 PM

Aug 18 2017

dexonsmith requested changes to D36556: [IR] AutoUpgrade ModuleFlagBehavior for PIC and PIE level.
Aug 18 2017, 6:02 PM

Aug 17 2017

dexonsmith added a comment to D36556: [IR] AutoUpgrade ModuleFlagBehavior for PIC and PIE level.

Who changed the behaviour? Can we get them to review the behaviour of the upgrade?

Aug 17 2017, 11:50 AM

Aug 14 2017

dexonsmith accepted D36713: [libc++] Add a persistent way to disable availability.

LGTM.

Aug 14 2017, 3:29 PM

Aug 9 2017

dexonsmith accepted D36427: [libcxxabi][demangler] Improve representation of substitutions/templates.

I have a few more nitpicks; LGTM after you fix those.

Aug 9 2017, 10:43 AM

Aug 8 2017

dexonsmith added a comment to D34331: func.wrap.func.con: Unset function before destroying anything.

Don't you need // UNSUPPORTED: c++98, c++03 since std::function is supported in C++11 only?

Aug 8 2017, 10:44 AM

Aug 7 2017

dexonsmith added a comment to D36427: [libcxxabi][demangler] Improve representation of substitutions/templates.

Oh, also found a couple of things you should likely split into prep commits to simplify this patch.

Aug 7 2017, 5:13 PM
dexonsmith requested changes to D36427: [libcxxabi][demangler] Improve representation of substitutions/templates.

This looks like a great improvement. I've littered the patch with nit-picks, but my main concern is that there aren't any unit tests for the new data structures. I wonder if there's a hacky way to set that up...

Aug 7 2017, 5:11 PM

Jul 28 2017

dexonsmith accepted D35951: Remove the obsolete "offset" parameter from @llvm.dbg.value.

The code changes and test/Bitcode/upgrade-dbg-value.ll LGTM.

I spot-checked the other testcases, and couldn't find any that had anything other than i64 0. Is that expected? Or can you point me at a non-trivial one?

We have been phasing out any remaining use of the offset field and replaced it with DIExpressions. This is also why the upgrade merely strips any dbg.value intrinsics with nonzero offsets. It's effectively dead code.

Jul 28 2017, 7:39 AM

Jul 27 2017

dexonsmith added a comment to D35951: Remove the obsolete "offset" parameter from @llvm.dbg.value.

The code changes and test/Bitcode/upgrade-dbg-value.ll LGTM.

Jul 27 2017, 7:42 PM
dexonsmith added a comment to D35951: Remove the obsolete "offset" parameter from @llvm.dbg.value.

Should there also be an update to LangRef?

For those living downstream I should also note that the IR testcase updates are not strictly necessary, since the auto upgrade code also works for assembler input, but I think it is less confusing to have the testcases use the up-to-date version of the intrinsics.

I feel like it would be better to error, and require the upgrade. Is that not practical for intrinsics?

LLParser::ValidateEndOfModule() always calls UpgradeCallsToIntrinsic(), therefore special-casing just the debug info intrinsics would be inconsistent.

Jul 27 2017, 7:38 PM
dexonsmith accepted D35159: [libcxxabi][demangler] Use an AST to represent the demangled name.

Use LLVM naming conventions for all the new stuff in this patch.

Jul 27 2017, 2:17 PM
dexonsmith added a comment to D35951: Remove the obsolete "offset" parameter from @llvm.dbg.value.

Should there also be an update to LangRef?

Jul 27 2017, 11:16 AM

Jul 18 2017

dexonsmith accepted D34439: Add GCC's noexcept-type alias for c++1z-compat-mangling.

LGTM.

Jul 18 2017, 5:23 AM

Jul 16 2017

dexonsmith added a comment to D35159: [libcxxabi][demangler] Use an AST to represent the demangled name.

Rebase. I don't think the issue of purging underscores from this file/libcxx should block this, if we want to discuss that cfe-dev would probably be the place. I agree that it would be nice to clang-format this file, would anyone have any problems with me committing that after this? I found the git-blame of this file to be pretty useless, almost every line just points to r184097.
Are there any other thoughts on this change?

Jul 16 2017, 8:05 PM

Jul 13 2017

dexonsmith added a comment to D34331: func.wrap.func.con: Unset function before destroying anything.

@dexonsmith Mind if I hijack this and check in your changes to <functional> with my tests?

Not at all.

@dexonsmith I'm not sure it's sane to allow reentrant behavior. Could you explain why you think it is?

It didn't seem sane to me at first either, despite this being supported by std::unique_ptr and std::shared_ptr. I found our user fairly convincing, though:

  • They had an underlying reference counted object, so only the destruction of the last copy of A nulled out the pointer (mimicked that with the bool cancel).
  • They had a callback function intended to be called once, and dropping that function on cancellation (mimicked that with a global variable). The cancel operation nulled out a std::function, but destroying the objects captured in the lambda in that std::function also separately decided to perform a cancel, which should have been OK. The cancel function just contained callback = nullptr.

According to [reentrancy] it is implementation defined what STL functions are allowed to be recursively reentered. I'm not opposed to providing reentrancy where it's useful, but we would have to provide it well.
This is where I was running into problems while I was writing tests. There are so many different ways you can reenter std::function's special members that it makes it impossible for an implementation
to truly support reentrancy as the standard would require.

If you consider that every possible copy construction or destruction of a user type is a potential reentrancy point, the complexity of having well-defined reentrant behavior starts to become clear.
Any time a copy constructor or destructor is invoked you have a potential reentrancy point which, in order to provide well defined behavior, must also act as a sort of _sequence point_ where the side effects of the current call are visible to the reentrant callers (For example your patches use of operator=(nullptr_t)).

The methods fixed in this patch are seemingly improvements; It's clear that operator=(nullptr) can safely be made reentrant. On the other hand consider operator=(function const& rhs), which is specified as equivalent to function(rhs).swap(*this).
I posit swap is not the kind of thing that can easily be made reentrant, but that making std::function assignment reentrant in general clearly requires it.

If fixing this bug is sufficiently important I'm willing to accept that, but I don't think it improves the status quo; We may have fixed a specific users bug, but we haven't made these functions safely reentrant (in fact most of the special members are likely still full of reentrancy bugs).

Jul 13 2017, 8:50 AM

Jul 12 2017

dexonsmith added a reviewer for D35159: [libcxxabi][demangler] Use an AST to represent the demangled name: mclow.lists.

+mclow.lists

Jul 12 2017, 2:03 PM
dexonsmith added a reviewer for D34781: Introduce a MCReloc class: enderby.
Jul 12 2017, 1:39 PM
dexonsmith added a comment to D35159: [libcxxabi][demangler] Use an AST to represent the demangled name.

This LGTM. Mehdi, do you have any other concerns?

Jul 12 2017, 1:34 PM

Jul 9 2017

dexonsmith added a comment to D34331: func.wrap.func.con: Unset function before destroying anything.

@dexonsmith Mind if I hijack this and check in your changes to <functional> with my tests?

Jul 9 2017, 6:41 AM
dexonsmith added a dependency for D35159: [libcxxabi][demangler] Use an AST to represent the demangled name: D35158: [libcxxabi][demangler] NFC: Don't make everything a template.
Jul 9 2017, 6:13 AM
dexonsmith added a dependent revision for D35158: [libcxxabi][demangler] NFC: Don't make everything a template: D35159: [libcxxabi][demangler] Use an AST to represent the demangled name.
Jul 9 2017, 6:13 AM
dexonsmith accepted D35158: [libcxxabi][demangler] NFC: Don't make everything a template.

Assuming nothing in arena<>...Db actually changed (just moved), this LGTM if you split the move into a separate prep commit.

Jul 9 2017, 6:13 AM

Jul 6 2017

dexonsmith closed D34578: cmath: Support clang's -fdelayed-template-parsing.

Ping! Hal, you committed r283051. Do you have a problem with this?

It looks like you're just renaming __libcpp_isnan and friends to __libcpp_isnan_or_builtin` and similar. LGTM

Jul 6 2017, 10:20 PM
dexonsmith committed rL307357: cmath: Support clang's -fdelayed-template-parsing.
cmath: Support clang's -fdelayed-template-parsing
Jul 6 2017, 10:14 PM
dexonsmith added a comment to D34578: cmath: Support clang's -fdelayed-template-parsing.

Ping! Hal, you committed r283051. Do you have a problem with this?

Jul 6 2017, 1:58 PM
dexonsmith accepted D35069: [Frontend] Verify that the bitstream is not empty before reading the serialised diagnostics.

LGTM.

Jul 6 2017, 1:56 PM
dexonsmith added a comment to D34331: func.wrap.func.con: Unset function before destroying anything.

Ping!

Jul 6 2017, 1:49 PM

Jun 26 2017

dexonsmith added a comment to D34574: [Sema] Disable c++17 aligned new and delete operators if not implemented in the deployment target's c++ standard library.

Should we also remove the recently-added availability attributes in libc++ too? If I'm using a recent libc++ and providing my own aligned allocation functions, we shouldn't reject attempts to call those aligned allocation functions.

Jun 26 2017, 2:19 PM
dexonsmith added a comment to D34574: [Sema] Disable c++17 aligned new and delete operators if not implemented in the deployment target's c++ standard library.

Duncan and I had a discussion on this.

We are thinking about adding a warning that tells users that aligned allocation /deallocation operators are being called but they are not defined in the library.

Jun 26 2017, 2:08 PM
dexonsmith accepted D34556: [libcxx] Annotate c++17 aligned new/delete operators with availability attribute.

LGTM.

Jun 26 2017, 9:57 AM

Jun 23 2017

dexonsmith created D34578: cmath: Support clang's -fdelayed-template-parsing.
Jun 23 2017, 3:56 PM
dexonsmith added inline comments to D34556: [libcxx] Annotate c++17 aligned new/delete operators with availability attribute.
Jun 23 2017, 9:03 AM
dexonsmith added a comment to D34556: [libcxx] Annotate c++17 aligned new/delete operators with availability attribute.

I followed the other availability macros in using "strict", but I'm not sure this was the right decision. The documentation seems to recommend not using "strict" (it says that weakly-linking is almost always a better API choice).

libc++ is one of the exceptions. Use strict.

Jun 23 2017, 8:35 AM

Jun 22 2017

dexonsmith added a reviewer for D34446: [Support] sys::getProcessTriple should return a macOS triple using the system's version of macOS: lhames.

Right now, sys::getProcessTriple returns the LLVM_HOST_TRIPLE, whose system version might not be the actual version of the system on which the compiler running.

Are we sure no one depends on that behaviour?

The primary users of this function are the JIT libraries, but so far I haven't found any checks for the OS version in the JIT code. And it looks like most of the other users of this function don't check the OS version.

Jun 22 2017, 11:32 AM

Jun 21 2017

dexonsmith added inline comments to D34264: Introduce -Wunguarded-availability-new, which is like -Wunguarded-availability, except that it's enabled by default for new deployment targets.
Jun 21 2017, 5:42 PM
dexonsmith added a comment to D34446: [Support] sys::getProcessTriple should return a macOS triple using the system's version of macOS.

Right now, sys::getProcessTriple returns the LLVM_HOST_TRIPLE, whose system version might not be the actual version of the system on which the compiler running.

Jun 21 2017, 3:04 PM

Jun 19 2017

dexonsmith added a comment to D34251: Add a new driver option to disable warning about c++17's non-throwing exception specification in function signature.

We use the -Wc++NN-compat- prefix on all the other subwarnings of -Wc++NN-compat warnings

Jun 19 2017, 4:25 PM