Page MenuHomePhabricator

Mordante (Mark de Wever)
User

Projects

User does not belong to any projects.

User Details

User Since
May 10 2019, 9:08 AM (72 w, 2 d)

Recent Activity

Today

Mordante added inline comments to D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels.
Sun, Sep 27, 11:28 AM · Restricted Project
Mordante updated the diff for D87561: [Sema] List conversion validate character array.

Addresses the review comments.
Adds an extra test case to test whether the proper overload is called. The proper overload is a bit of a surprise so when the expected behaviour changes the overload test can be adjusted.

Sun, Sep 27, 10:26 AM · Restricted Project
Mordante added inline comments to D87561: [Sema] List conversion validate character array.
Sun, Sep 27, 9:47 AM · Restricted Project

Yesterday

Mordante requested review of D88363: [CodeGen] Improve likelihood attribute branch weights.
Sat, Sep 26, 10:23 AM · Restricted Project
Mordante added a comment to D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels.

Friendly ping.

Sat, Sep 26, 4:52 AM · Restricted Project

Sat, Sep 19

Mordante added inline comments to D87561: [Sema] List conversion validate character array.
Sat, Sep 19, 7:48 AM · Restricted Project
Mordante committed rGd4dd96130058: Fixes complexity of map insert_or_assign with a hint. (authored by Mordante).
Fixes complexity of map insert_or_assign with a hint.
Sat, Sep 19, 7:29 AM
Mordante closed D62779: [2/2] Fix complexity of map insert_or_assign with a hint..
Sat, Sep 19, 7:29 AM · Restricted Project
Mordante added a comment to D62779: [2/2] Fix complexity of map insert_or_assign with a hint..

Thanks for the review!

Sat, Sep 19, 7:29 AM · Restricted Project

Thu, Sep 17

Mordante added inline comments to D62779: [2/2] Fix complexity of map insert_or_assign with a hint..
Thu, Sep 17, 11:54 AM · Restricted Project
Mordante updated the diff for D62779: [2/2] Fix complexity of map insert_or_assign with a hint..

Improves the code based on @ldionne's suggestion

Thu, Sep 17, 10:47 AM · Restricted Project

Wed, Sep 16

Mordante added inline comments to D62779: [2/2] Fix complexity of map insert_or_assign with a hint..
Wed, Sep 16, 11:17 AM · Restricted Project

Tue, Sep 15

Mordante added inline comments to D87561: [Sema] List conversion validate character array.
Tue, Sep 15, 11:42 AM · Restricted Project
Mordante added a comment to D62779: [2/2] Fix complexity of map insert_or_assign with a hint..

I'll rebase the patch and then have a look at your suggestion.

Tue, Sep 15, 11:19 AM · Restricted Project
Mordante added a comment to D62778: [1/2] Add a benchmark for map operations..

Thanks for accepting and committing the patch.

Tue, Sep 15, 11:17 AM · Restricted Project

Mon, Sep 14

Mordante added a comment to D87561: [Sema] List conversion validate character array.

Thanks for the feedback!

Mon, Sep 14, 10:50 AM · Restricted Project

Sat, Sep 12

Mordante requested review of D87566: [Sema] Permit conversions to arrays of unknown bound.
Sat, Sep 12, 11:00 AM · Restricted Project
Mordante requested review of D87565: [Sema] Improve const_cast conformance to N4261.
Sat, Sep 12, 10:53 AM · Restricted Project
Mordante requested review of D87563: [Sema] Improve overload resolution.
Sat, Sep 12, 10:09 AM · Restricted Project
Mordante requested review of D87561: [Sema] List conversion validate character array.
Sat, Sep 12, 9:44 AM · Restricted Project

Wed, Sep 9

Mordante updated the diff for D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels.

Update the patch to match the behaviour where the attribute only affects the substatement of an if statement. This means the likelihood attributes on labels are accepted but are a no-op. Since they're a no-op the documentation doesn't mention them.

Wed, Sep 9, 1:00 PM · Restricted Project
Mordante committed rG08196e0b2e1f: Implements [[likely]] and [[unlikely]] in IfStmt. (authored by Mordante).
Implements [[likely]] and [[unlikely]] in IfStmt.
Wed, Sep 9, 11:50 AM
Mordante closed D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt.
Wed, Sep 9, 11:50 AM · Restricted Project, Restricted Project
Mordante added inline comments to D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt.
Wed, Sep 9, 11:48 AM · Restricted Project, Restricted Project
Mordante added a comment to D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt.

Thanks for your feedback during the review.

Wed, Sep 9, 11:08 AM · Restricted Project, Restricted Project

Fri, Sep 4

Mordante updated the diff for D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt.

Addresses review comments, mainly:

  • Improving the documentation
  • Removing the AST bits since they're no longer required
Fri, Sep 4, 9:10 AM · Restricted Project, Restricted Project
Mordante added inline comments to D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt.
Fri, Sep 4, 9:08 AM · Restricted Project, Restricted Project

Wed, Sep 2

Mordante added inline comments to D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt.
Wed, Sep 2, 1:02 PM · Restricted Project, Restricted Project
Mordante added inline comments to D82994: [RFC] Instrumenting Clang/LLVM with Perfetto.
Wed, Sep 2, 10:21 AM · Restricted Project, Restricted Project
Mordante added a comment to D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt.

Looking specifically for attributes in the 'then' and 'else' cases of an if seems like a fine first pass at this, but I think this is the wrong way to lower these attributes in the longer term: we should have a uniform treatment of them that looks for the most recent prior branch and annotates it with weights. We could do that either by generating LLVM intrinsic calls that an LLVM pass lowers, or by having the frontend look backwards for the most recent branch and annotate it. I suppose it's instructive to consider a case such as:

void mainloop() noexcept; // probably terminates by calling `exit`
void f() {
  mainloop();
  [[unlikely]];
  somethingelse();
}

... where the [[unlikely]]; should probably cause us to split the somethingelse() out into a separate basic block that we mark as cold, but should not cause f() itself or its call to mainloop() to be considered unlikely -- except in the case where mainloop() can be proven to always terminate, in which case the implication is that it's unlikely that f() is invoked. Cases like this probably need the LLVM intrinsic approach to model them properly.

We indeed considered similar cases and wondered whether it was really intended to work this way. Since this approach seems a bit hard to explain to users, I changed the code back to only allow the attribute on the substatement of the if and else. For now I first want to implement the simple approach, which I expect will be the most common use case. Once finished we can see what the next steps will be.

Wed, Sep 2, 10:12 AM · Restricted Project, Restricted Project

Tue, Sep 1

Mordante added a comment to D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels.

This is not a "teach everyone about it, it's safe" feature. It's there to produce a very fine-grained control in those cases where it really matters, and where profiling-guided optimizations would produce exactly the wrong result. Using this feature should be an automatic "is this needed" question in a code review. It is needed sometimes, just rarely.

+1 but I would point out that when PGO is enabled, this attribute is ignored, so it's an either/or feature. Either you get tuned optimizations, or you get to guess at them, but the current design doesn't let you mix and match. Do see that as being a major issue with the design?

We did have discussions in SG14 about the possible need for "inverse PGO" optimization passes, but we didn't have the compiler dev expertise to know if we were on a good track or not. From the 8 years I've spent working on this kind of code, I've never mixed the two, since the interactions are just too unpredictable. What I will frequently do however, is compare benchmarks between PGO generated code, and our hand-specified optimizations. Sometimes the PGO detects interesting program flow that a mere human brain could not have envisioned, which we can then take advantage of (or de-prioritize) as needed.

Tue, Sep 1, 12:11 PM · Restricted Project
Mordante added a comment to D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt.

Thanks again for your feedback. Once we agree on what to do with the Likelihood state in the output I'll finish the changes and submit a new patch.

Tue, Sep 1, 12:05 PM · Restricted Project, Restricted Project

Sat, Aug 29

Mordante updated the diff for D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt.

Reworked the patch to match the behaviour as discussed in D86559.

  • The likelihood attributes only have an effect when used directly on the ThenStmt and ElseStmt.
  • Conflicting attributes on the ThenStmt and ElseStmt generate a diagnostic.
  • Moved the likelihood determination from the CodeGen to the Sema. This requires the state to be stored in the AST.
  • Updated the AST functions for the new likelihood bits.
  • Updated the documentation.
Sat, Aug 29, 9:06 AM · Restricted Project, Restricted Project
Mordante added a comment to D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels.

The use case for this becomes clearer when considering that the attribute that will be used 95% of the time is [[unlikely]]. You may have a constructor that you wish to ask the compiler to please, please, do not inline this, in a particular function, even if the rest of that function is either likely or neutral.

Sat, Aug 29, 8:55 AM · Restricted Project
Mordante added a comment to D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels.

As one of the SG14 industry members driving this, I'm firmly in the camp that this is what we're expecting. In the first case the 1/2 case are "neutral". This is a very explicit, and local, marker. Anything else makes it so vague as to be unusable for fine tuned code.

Sat, Aug 29, 8:42 AM · Restricted Project
Mordante added a comment to D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels.

That is exactly the behavior I am coming to believe we should follow. You can either write it on a compound statement that is guarded by a flow control decision (if/else/while) or you can write it on a label, otherwise the attribute is parsed and ignored (with a diagnostic). This feels like the right mixture of useful with understandable semantics so far, but perhaps we'll find other examples that change our minds.

The fallthrough behavior question has one more case we may want to think about:

switch (x) {
case 0:
case 1:
[[likely]] case 2: break;
[[unlikely]] default:
}

Does this mark cases 0 and 1 as being likely or not? I could see users wanting to use shorthand rather than repeat themselves on all the cases. However, I'm also not certain whether there would be any performance impact if we marked only case 2 as likely and left cases 0 and 1 with default likelihood. My gut feeling is that this should only mark case 2, but others may have different views.

Not according to the standard: "A path of execution includes a label if and only if it contains a jump to that label."

A switch statement contains a jump to all of its labels, though. So all of those labels are included in the path of execution, which is not likely what's intended by the standard.

Sat, Aug 29, 8:32 AM · Restricted Project
Mordante added inline comments to D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt.
Sat, Aug 29, 8:08 AM · Restricted Project, Restricted Project

Aug 27 2020

Mordante planned changes to D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels.

That is exactly the behavior I am coming to believe we should follow. You can either write it on a compound statement that is guarded by a flow control decision (if/else/while) or you can write it on a label, otherwise the attribute is parsed and ignored (with a diagnostic). This feels like the right mixture of useful with understandable semantics so far, but perhaps we'll find other examples that change our minds.

The fallthrough behavior question has one more case we may want to think about:

switch (x) {
case 0:
case 1:
[[likely]] case 2: break;
[[unlikely]] default:
}

Does this mark cases 0 and 1 as being likely or not? I could see users wanting to use shorthand rather than repeat themselves on all the cases. However, I'm also not certain whether there would be any performance impact if we marked only case 2 as likely and left cases 0 and 1 with default likelihood. My gut feeling is that this should only mark case 2, but others may have different views.

Aug 27 2020, 10:21 AM · Restricted Project
Mordante planned changes to D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt.

I fixed the issues locally. Before I resubmit them I'd rather discuss in D86559 the direction we want to take, so we can avoid an additional review cycle.

Aug 27 2020, 9:43 AM · Restricted Project, Restricted Project

Aug 26 2020

Mordante added a comment to D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels.

I'd like to understand your reasoning for ignore + diagnose as opposed to count attrs (+ optionally diagnose) or other strategies. I can see pros and cons to basically anything we do here.

If we decide to ignore conflicting likelihoods, then I agree we should issue a diagnostic. However, I suspect that's going to come up frequently in practice because people are going to write macros that include these attributes. For instance, it's very sensible for a developer to put [[unlikely]] in front of an assert under the assumption this is telling the compiler "this assertion isn't likely to happen" when in fact it's saying "this branch containing the assertion is unlikely to be taken".

Aug 26 2020, 12:03 PM · Restricted Project
Mordante added inline comments to D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt.
Aug 26 2020, 11:18 AM · Restricted Project, Restricted Project

Aug 25 2020

Mordante added a comment to D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels.

It seems like this patch would basically "copy" the [[likely]] attribute from line C up to lines A and B, where it would reinforce the likelihood of path A and (maybe?) "cancel out" the unlikelihood of path B, without actually saying anything specifically about the likelihood of label C (which is surely what the programmer intended by applying the attribute, right?).

Yes A is double [[likely]] and B is both [[likely]] and [[unlikely]]. Based on http://eel.is/c++draft/dcl.attr.likelihood#:attribute,likely
"The use of the likely attribute is intended to allow implementations to optimize for the case where paths of execution including it are arbitrarily more likely than any alternative path of execution that does not include such an attribute on a statement or label."
So it seem it's intended to see a goto as a part of a path of execution, since it's an unconditional jump.

Aug 25 2020, 12:23 PM · Restricted Project
Mordante requested review of D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels.
Aug 25 2020, 11:08 AM · Restricted Project

Aug 23 2020

Mordante updated the diff for D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt.

Addresses review comments:

  • Adds support for c2x using [[clang::likely]] and [[clang::unlikely]]
  • Remove warnings about conflicting likelihoods, except those required by the Standard
  • The attributes can now be placed inside the compound statement in of the ThenStmt and ElseStmt
Aug 23 2020, 11:29 AM · Restricted Project, Restricted Project
Mordante added inline comments to D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt.
Aug 23 2020, 11:08 AM · Restricted Project, Restricted Project

Aug 16 2020

Mordante added inline comments to D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt.
Aug 16 2020, 11:47 AM · Restricted Project, Restricted Project
Mordante committed rGfef260712407: [Sema] Use the proper cast for a fixed bool enum. (authored by Mordante).
[Sema] Use the proper cast for a fixed bool enum.
Aug 16 2020, 9:45 AM
Mordante closed D85612: [Sema] Use proper integral cast for an enumerate with a fixed bool type.
Aug 16 2020, 9:44 AM · Restricted Project
Mordante committed rG827ba67e3833: [Sema] Validate calls to GetExprRange. (authored by Mordante).
[Sema] Validate calls to GetExprRange.
Aug 16 2020, 9:36 AM
Mordante closed D85601: Fixes an assertion when IntRange processes a throw expression.
Aug 16 2020, 9:36 AM · Restricted Project

Aug 14 2020

Mordante planned changes to D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt.
Aug 14 2020, 12:52 PM · Restricted Project, Restricted Project
Mordante updated the diff for D85601: Fixes an assertion when IntRange processes a throw expression.

Addresses review comments.

Aug 14 2020, 11:32 AM · Restricted Project
Mordante added inline comments to D85601: Fixes an assertion when IntRange processes a throw expression.
Aug 14 2020, 10:28 AM · Restricted Project
Mordante updated the diff for D85612: [Sema] Use proper integral cast for an enumerate with a fixed bool type.

Addresses review comments.

Aug 14 2020, 10:25 AM · Restricted Project
Mordante added inline comments to D85612: [Sema] Use proper integral cast for an enumerate with a fixed bool type.
Aug 14 2020, 9:46 AM · Restricted Project

Aug 10 2020

Mordante added a comment to D85601: Fixes an assertion when IntRange processes a throw expression.

I added void g() since that's valid code which also caused an assertion failure. So the issue isn't in the error recovery but in determining the required IntRange. It seems the code doesn't take http://eel.is/c++draft/expr.cond#2.1 into account.

Aug 10 2020, 12:54 PM · Restricted Project
Mordante updated the diff for D85612: [Sema] Use proper integral cast for an enumerate with a fixed bool type.

Addresses the review comments.

Aug 10 2020, 12:39 PM · Restricted Project
Mordante added inline comments to D85612: [Sema] Use proper integral cast for an enumerate with a fixed bool type.
Aug 10 2020, 12:37 PM · Restricted Project

Aug 9 2020

Mordante requested review of D85612: [Sema] Use proper integral cast for an enumerate with a fixed bool type.
Aug 9 2020, 9:19 AM · Restricted Project
Mordante requested review of D85601: Fixes an assertion when IntRange processes a throw expression.
Aug 9 2020, 4:58 AM · Restricted Project

Aug 2 2020

Mordante updated the diff for D72103: [Sema] Avoid using an invalid InsertPos.

Rebased the patch and updated the unit tests.

Aug 2 2020, 10:21 AM · Restricted Project
Mordante requested review of D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt.
Aug 2 2020, 4:42 AM · Restricted Project, Restricted Project

Apr 7 2020

Mordante planned changes to D71142: [Sema] Validate large bitfields.

Any comments?

@rsmith @aaron.ballman

There are outstanding review comments, so I'm waiting for the patch author to address those, unless I've missed a question somewhere?

Apr 7 2020, 11:55 AM · Restricted Project
Mordante added a comment to D74877: [clang] fix incorrect Wdocumentation fix-its.

Take all \params and function parameters in account when looking for a solution and find the best total solution.

Currently all unmapped parameters are taken into account.
I cannot imagine a use case where taking the mapped ones into account could influence the outcome.
Also I personally don't like breaking any existing correct documentation.
Do you have some example?

Apr 7 2020, 11:55 AM · Restricted Project

Mar 21 2020

Mordante added a comment to D74877: [clang] fix incorrect Wdocumentation fix-its.

Sorry it took a while.

Mar 21 2020, 11:13 AM · Restricted Project
Mordante added inline comments to D63059: Implements multiline regex support.
Mar 21 2020, 9:37 AM · Restricted Project

Mar 17 2020

Mordante updated the diff for D63059: Implements multiline regex support.

Implements the multiline using a derived class.

Mar 17 2020, 12:23 PM · Restricted Project
Mordante added a comment to D63059: Implements multiline regex support.

Thanks for the confirmation it's an ABI break. I like your suggestion to use a derived class, so I'll try that solution first.

I'm not 100% sure that solution addresses the ABI break problem, though. I'd have to think about it harder and run it through at least another person to be confident.

Mar 17 2020, 12:23 PM · Restricted Project
Mordante added a comment to D72103: [Sema] Avoid using an invalid InsertPos.

Friendly ping.

Mar 17 2020, 11:49 AM · Restricted Project

Mar 12 2020

Mordante committed rG56926a9146fc: Revert "[libcxx] Enable C++17 for the benchmarks." (authored by Mordante).
Revert "[libcxx] Enable C++17 for the benchmarks."
Mar 12 2020, 2:39 PM
Mordante added a reverting change for rGd184d0226301: [libcxx] Enable C++17 for the benchmarks.: rG56926a9146fc: Revert "[libcxx] Enable C++17 for the benchmarks.".
Mar 12 2020, 2:39 PM
Herald added a reviewer for D63059: Implements multiline regex support: Restricted Project.

Thanks for the confirmation it's an ABI break. I like your suggestion to use a derived class, so I'll try that solution first.

Mar 12 2020, 2:39 PM · Restricted Project
Mordante added a comment to D75955: [libcxx] Enable C++17 for the benchmarks.

Seems it requires CMake 3.8 to use CXX_STANDARD 17, so reverted the commit for now.

Mar 12 2020, 2:39 PM · Restricted Project
Mordante added a comment to D75955: [libcxx] Enable C++17 for the benchmarks.

CXX_STANDARD_REQUIRED is already set by the main LLVM CMake script.

Mar 12 2020, 2:39 PM · Restricted Project
Mordante added a comment to D62778: [1/2] Add a benchmark for map operations..

Good catch, I'll update the patch.

Mar 12 2020, 2:07 PM · Restricted Project
Mordante updated the diff for D62778: [1/2] Add a benchmark for map operations..

Addresses @EricWF's comment.

Mar 12 2020, 2:07 PM · Restricted Project
Mordante committed rGd184d0226301: [libcxx] Enable C++17 for the benchmarks. (authored by Mordante).
[libcxx] Enable C++17 for the benchmarks.
Mar 12 2020, 1:37 PM
Mordante closed D75955: [libcxx] Enable C++17 for the benchmarks.
Mar 12 2020, 1:36 PM · Restricted Project

Mar 10 2020

Mordante updated subscribers of D63059: Implements multiline regex support.

@mclow.lists, @ldionne is the change of the size of __l_anchor an ABI break?

Mar 10 2020, 1:39 PM · Restricted Project
Mordante updated the diff for D75955: [libcxx] Enable C++17 for the benchmarks.

Noticed I didn't stage my last hunk. Don't use BENCHMARK_DIALECT_FLAG which no longer is set.

Mar 10 2020, 1:39 PM · Restricted Project
Mordante requested review of D62778: [1/2] Add a benchmark for map operations..

It builds for me, but I had CMAKE_CXX_STANDARD set to 17 for this build. It seems the method used to use C++17 for the benchmarks didn't work. I feel fixing this issue should not be part of this commit. So I created D75955 to fix the issue. With that patch applied it also works for me when CMAKE_CXX_STANDARD set to 14. This is the default value.

Mar 10 2020, 1:06 PM · Restricted Project
Mordante created D75955: [libcxx] Enable C++17 for the benchmarks.
Mar 10 2020, 1:06 PM · Restricted Project

Mar 8 2020

Mordante updated the diff for D62779: [2/2] Fix complexity of map insert_or_assign with a hint..

Added unit tests as requested and rebased against master.

Mar 8 2020, 11:44 AM · Restricted Project
Mordante added a comment to D62779: [2/2] Fix complexity of map insert_or_assign with a hint..

Thanks for looking into these patches!

Mar 8 2020, 11:44 AM · Restricted Project
Mordante updated the diff for D62778: [1/2] Add a benchmark for map operations..

Rebased against master.

Mar 8 2020, 11:44 AM · Restricted Project

Mar 4 2020

Mordante added a comment to D75613: [Sema] Reword -Wrange-loop-analysis warning messages.

Thanks for the patch! I've some minor nits, but other then that it looks fine.

Mar 4 2020, 12:21 PM · Restricted Project
Mordante added a comment to D74877: [clang] fix incorrect Wdocumentation fix-its.

@gribozavr2 what's your opinion of this patch?

Mar 4 2020, 11:47 AM · Restricted Project

Feb 22 2020

Mordante added a comment to D72103: [Sema] Avoid using an invalid InsertPos.

Friendly ping.

Feb 22 2020, 10:58 AM · Restricted Project
Mordante committed rG56eb15a1c710: [Sema] Fix pointer-to-int-cast diagnostic for _Bool (authored by Mordante).
[Sema] Fix pointer-to-int-cast diagnostic for _Bool
Feb 22 2020, 10:41 AM
Mordante closed D74860: [Sema] Fix pointer-to-int-cast diagnostic for _Bool.
Feb 22 2020, 10:40 AM · Restricted Project
Mordante added inline comments to D74877: [clang] fix incorrect Wdocumentation fix-its.
Feb 22 2020, 10:40 AM · Restricted Project
Mordante updated the diff for D74860: [Sema] Fix pointer-to-int-cast diagnostic for _Bool.

Added some C++ tests for similar c-style casts.

Feb 22 2020, 10:05 AM · Restricted Project
Mordante added a comment to D74860: [Sema] Fix pointer-to-int-cast diagnostic for _Bool.

Can we test the same thing in C++?

Feb 22 2020, 10:05 AM · Restricted Project

Feb 20 2020

Mordante added a reviewer for D74877: [clang] fix incorrect Wdocumentation fix-its: gribozavr2.

Thanks for your patch. I also would like to get better fixits for the arguments.

Feb 20 2020, 1:25 PM · Restricted Project
Mordante updated the diff for D62453: Regex backreference [3/3] Validate backreferences in the constructor..

Rebased as requested.

Feb 20 2020, 12:19 PM · Restricted Project
Mordante added a comment to D62451: Regex backreference [1/3] Fixes backreferences for extended grammar..

Thanks for the review and committing! (BTW I have commit access.)

Feb 20 2020, 12:10 PM · Restricted Project

Feb 19 2020

Mordante created D74860: [Sema] Fix pointer-to-int-cast diagnostic for _Bool.
Feb 19 2020, 12:52 PM · Restricted Project
Mordante added a comment to D62451: Regex backreference [1/3] Fixes backreferences for extended grammar..

Friendly ping.

Feb 19 2020, 12:24 PM · Restricted Project
Mordante added a comment to D72231: [Sema] Adds the pointer-to-int-cast diagnostic.

Okay. Can we raise this with the kernel folks instead of just assuming they'll be opposed? An obvious patch to fix a few dozen places where they're hit by a warning they intentionally enabled really doesn't seem like a burden. If they push back, fine, we can enable the warning without covering enums.

Feb 19 2020, 11:55 AM · Restricted Project

Feb 18 2020

Mordante added a comment to D72231: [Sema] Adds the pointer-to-int-cast diagnostic.

There appear to a be semantic difference between GCC and clang with the current version of this patch which results in a lot of additional warnings in the Linux kernel: https://godbolt.org/z/eHFJd8

Warning about casting to an enum seems clearly correct and in scope for this warning. Warning about casting to _Bool seems clearly incorrect and should not be warned about at all.

Feb 18 2020, 1:03 PM · Restricted Project