Page MenuHomePhabricator

[clang-tidy] Implement readability-function-cognitive-complexity check
ClosedPublic

Authored by lebedev.ri on Aug 17 2017, 8:57 AM.

Details

Summary

Currently, there is basically just one clang-tidy check to impose some sanity limits on functions - clang-tidy-readability-function-size.
It is nice, allows to limit line count, total number of statements, number of branches, number of function parameters (not counting implicit this), nesting level.
However, those are simple generic metrics. It is still trivially possible to write a function, which does not violate any of these metrics, yet is still rather unreadable.

Thus, some additional, slightly more complicated metric is needed.
There is a well-known Cyclomatic complexity, but certainly has its downsides.
And there is a COGNITIVE COMPLEXITY by SonarSource, which is available for opensource on https://sonarcloud.io/.
I did ask them, and received an answer that it is it can be implemented in clang-tidy.

There is a number of official open-source LGPL-3 implementations already:

There are other open-source LGPL-3 implementations already:

There are other 3rd party implementations:

This check checks function Cognitive Complexity metric, and flags the functions with Cognitive Complexity exceeding the configured limit.
The default limit is 25, same as in 'upstream'.

The metric is implemented as per COGNITIVE COMPLEXITY by SonarSource specification version 1.2 (19 April 2017), with two notable exceptions:

  • preprocessor conditionals (#ifdef, #if, #elif, #else, #endif) are not accounted for. Could be done. Currently, upstream does not account for them either.
  • each method in a recursion cycle is not accounted for. It can't be fully implemented, because cross-translational-unit analysis would be needed, which is not possible in clang-tidy. Thus, at least right now, i completely avoided implementing it.

Diff Detail

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
  • Rebased
  • As advised by @aaron.ballman, moved into it's own directory/module. Please review that, i'm not entirely sure i have done that fully correctly.

@chandlerc Hi!
@aaron.ballman has suggested for me to try to talk to you about this.
Is there some precedent for the licensing 'issue' at hand? Do you have any opinion?
@dberlin did not react to the pings, so i'm not sure i personally can come up with anything better for LICENSE.txt

If there are no further ideas, i'll try to contact sonarsource.

lebedev.ri changed the repository for this revision from rL LLVM to rCTE Clang Tools Extra.

Rebased.

  • Rebased
  • As advised by @aaron.ballman, moved into it's own directory/module. Please review that, i'm not entirely sure i have done that fully correctly.

@chandlerc Hi!
@aaron.ballman has suggested for me to try to talk to you about this.
Is there some precedent for the licensing 'issue' at hand? Do you have any opinion?
@dberlin did not react to the pings, so i'm not sure i personally can come up with anything better for LICENSE.txt

Sadly, what you need here is legal advice for a good way to handle this, and I'm not a lawyer and so I can't really give you that advice.

To be clear, what you currently have isn't OK for several reasons, not least of which what Aaron brought up that this is not in fact a license.

If there are no further ideas, i'll try to contact sonarsource.

Unless Danny can volunteer his time, we finish with relicensing efforts and can devote the foundation's lawyer's (sadly precious) time to this, I think either you or sonarsource working to understand the best legal way to contribute this would be the best way forward. Sorry that this is a tricky situation. Happy to have a private discussion w/ your or sonarsources's lawyer via my @llvm.org email address if useful.

  • Rebased
  • As advised by @aaron.ballman, moved into it's own directory/module. Please review that, i'm not entirely sure i have done that fully correctly.

@chandlerc Hi!
@aaron.ballman has suggested for me to try to talk to you about this.
Is there some precedent for the licensing 'issue' at hand? Do you have any opinion?
@dberlin did not react to the pings, so i'm not sure i personally can come up with anything better for LICENSE.txt

Sadly, what you need here is legal advice for a good way to handle this, and I'm not a lawyer and so I can't really give you that advice.

To be clear, what you currently have isn't OK for several reasons, not least of which what Aaron brought up that this is not in fact a license.

If there are no further ideas, i'll try to contact sonarsource.

Unless Danny can volunteer his time, we finish with relicensing efforts and can devote the foundation's lawyer's (sadly precious) time to this, I think either you or sonarsource working to understand the best legal way to contribute this would be the best way forward. Sorry that this is a tricky situation. Happy to have a private discussion w/ your or sonarsources's lawyer via my @llvm.org email address if useful.

Mailed and heard back (yay!).
They don't want to take active stance/first steps in this,
but they also did *not* retract that initial 'agreement' i quoted earlier (i did ask about that explicitly.).
So nothing is changed so far, this will have to wait until someone can volunteer their precious lawyer time.
(Can show the mail exchange if needed.)

alexfh requested changes to this revision.Mar 14 2018, 8:31 AM

Removing from my dashboard until the licensing stuff gets sorted out.

This revision now requires changes to proceed.Mar 14 2018, 8:31 AM
lebedev.ri updated this revision to Diff 154491.Jul 7 2018, 6:32 AM

Rebased, just to control bitrot, no changes.

Anything new here? Is there a LLVM foundation lawyer or something like that we can ask?

alexfh requested changes to this revision.Oct 1 2018, 6:19 AM

Anything new here? Is there a LLVM foundation lawyer or something like that we can ask?

I didn't notice any substantial changes w.r.t. license since Chandler's comment on Feb. 28th.

This revision now requires changes to proceed.Oct 1 2018, 6:19 AM
  • Rebased
  • As advised by @aaron.ballman, moved into it's own directory/module. Please review that, i'm not entirely sure i have done that fully correctly.

@chandlerc Hi!
@aaron.ballman has suggested for me to try to talk to you about this.
Is there some precedent for the licensing 'issue' at hand? Do you have any opinion?
@dberlin did not react to the pings, so i'm not sure i personally can come up with anything better for LICENSE.txt

Sadly, what you need here is legal advice for a good way to handle this, and I'm not a lawyer and so I can't really give you that advice.

To be clear, what you currently have isn't OK for several reasons, not least of which what Aaron brought up that this is not in fact a license.

If there are no further ideas, i'll try to contact sonarsource.

Unless Danny can volunteer his time, we finish with relicensing efforts

Hi @chandlerc !
I was just wondering what percentage of "finish" was meant here, and where at it's now?

and can devote the foundation's lawyer's (sadly precious) time to this, I think either you or sonarsource working to understand the best legal way to contribute this would be the best way forward. Sorry that this is a tricky situation. Happy to have a private discussion w/ your or sonarsources's lawyer via my @ llvm.org email address if useful.

Herald added a project: Restricted Project. · View Herald TranscriptMar 12 2019, 2:58 PM
Herald added a subscriber: jdoerfert. · View Herald Transcript
JonasToth resigned from this revision.Jan 4 2020, 2:39 PM
mgehre removed a subscriber: mgehre.Jan 4 2020, 3:29 PM
  • Rebased
  • As advised by @aaron.ballman, moved into it's own directory/module. Please review that, i'm not entirely sure i have done that fully correctly.

@chandlerc Hi!
@aaron.ballman has suggested for me to try to talk to you about this.
Is there some precedent for the licensing 'issue' at hand? Do you have any opinion?
@dberlin did not react to the pings, so i'm not sure i personally can come up with anything better for LICENSE.txt

Sadly, what you need here is legal advice for a good way to handle this, and I'm not a lawyer and so I can't really give you that advice.

To be clear, what you currently have isn't OK for several reasons, not least of which what Aaron brought up that this is not in fact a license.

If there are no further ideas, i'll try to contact sonarsource.

Unless Danny can volunteer his time, we finish with relicensing efforts

Hi @chandlerc !
I was just wondering what percentage of "finish" was meant here, and where at it's now?

And how far along is LLVM now, one year later? :)

and can devote the foundation's lawyer's (sadly precious) time to this, I think either you or sonarsource working to understand the best legal way to contribute this would be the best way forward. Sorry that this is a tricky situation. Happy to have a private discussion w/ your or sonarsources's lawyer via my @ llvm.org email address if useful.

lebedev.ri edited the summary of this revision. (Show Details)

Rebased.

There is a number of official open-source LGPL-3 implementations already:

There are other open-source LGPL-3 implementations already:

There are other 3rd party implementations:

Quite honestly, i do not understand how did the license question arose.
Would have it been fine if i based this on the open-source-licensed code?
Would have it not been? Would same license question be raised?
Somehow i don't think it would have been.

Is this really just about Copyright SonarSource S.A., 2018, Switzerland. All content is copyright protected. in https://www.sonarsource.com/docs/CognitiveComplexity.pdf ?
But that is only about the document, not the algorithm.
But even if we enternain the idea that all of the implementations must bow to that license,
then surely this is not the first code in LLVM that is implicitly/explicitly based on copyrighted doc.

This is rather frustrating.

Rebased.

There is a number of official open-source LGPL-3 implementations already:

There are other open-source LGPL-3 implementations already:

There are other 3rd party implementations:

Quite honestly, i do not understand how did the license question arose.
Would have it been fine if i based this on the open-source-licensed code?
Would have it not been? Would same license question be raised?
Somehow i don't think it would have been.

Is this really just about Copyright SonarSource S.A., 2018, Switzerland. All content is copyright protected. in https://www.sonarsource.com/docs/CognitiveComplexity.pdf ?
But that is only about the document, not the algorithm.
But even if we enternain the idea that all of the implementations must bow to that license,
then surely this is not the first code in LLVM that is implicitly/explicitly based on copyrighted doc.

This is rather frustrating.

Given there are open source implementations, this is in my eyes completly acceptable to implement this in llvm.
The document is copyrighted, true. The algorithm itself is not, in european law it is even very hard to patent algorithms, software in general is not patentable.
The code is not copy&pasted, so there are no infringements on anything.

Rebased.

There is a number of official open-source LGPL-3 implementations already:

There are other open-source LGPL-3 implementations already:

There are other 3rd party implementations:

Quite honestly, i do not understand how did the license question arose.

It arose in a comment that I can't seem to get phab to show me the context for (which is a bit strange, I don't think I've run into that before): https://reviews.llvm.org/D36836#877636 Perhaps part of this was carrying discussion over from the IRC channel?

Would have it been fine if i based this on the open-source-licensed code?

I believe that would require legal analysis to answer.

Would have it not been? Would same license question be raised?

Likewise here (I suspect the answer would depend on what the license of the open source code is).

Somehow i don't think it would have been.

I don't wish to speculate about legal licensing issues on the mailing lists.

Is this really just about Copyright SonarSource S.A., 2018, Switzerland. All content is copyright protected. in https://www.sonarsource.com/docs/CognitiveComplexity.pdf ?
But that is only about the document, not the algorithm.
But even if we enternain the idea that all of the implementations must bow to that license,
then surely this is not the first code in LLVM that is implicitly/explicitly based on copyrighted doc.

This is rather frustrating.

I am sorry and I agree that it's frustrating. As far as I know, this captures the current state of affairs: https://reviews.llvm.org/D36836#1031600 and basically we're waiting for help from the foundation to clear the last hurdle.

Rebased.

There is a number of official open-source LGPL-3 implementations already:

There are other open-source LGPL-3 implementations already:

There are other 3rd party implementations:

Quite honestly, i do not understand how did the license question arose.

It arose in a comment that I can't seem to get phab to show me the context for (which is a bit strange, I don't think I've run into that before): https://reviews.llvm.org/D36836#877636 Perhaps part of this was carrying discussion over from the IRC channel?

Would have it been fine if i based this on the open-source-licensed code?

I believe that would require legal analysis to answer.

Would have it not been? Would same license question be raised?

Likewise here (I suspect the answer would depend on what the license of the open source code is).

Somehow i don't think it would have been.

I don't wish to speculate about legal licensing issues on the mailing lists.

Is this really just about Copyright SonarSource S.A., 2018, Switzerland. All content is copyright protected. in https://www.sonarsource.com/docs/CognitiveComplexity.pdf ?
But that is only about the document, not the algorithm.
But even if we enternain the idea that all of the implementations must bow to that license,
then surely this is not the first code in LLVM that is implicitly/explicitly based on copyrighted doc.

This is rather frustrating.

I am sorry and I agree that it's frustrating.

As far as I know, this captures the current state of affairs: https://reviews.llvm.org/D36836#1031600

As far as I know, yes. Some further back&forth reiterated:

@Roman so you know, none of the non-SonarSource implementations have an official license from us.
We put the spec out in the world and we're happy when someone uses it. And that's all.
I appreciate how frustrated you must be with your implementation caught between a proverbial rock and a hard place.
Unfortunately, we (the company) just aren't willing to do the paperwork.
(G. Ann Campbell)

and basically we're waiting for help from the foundation to clear the last hurdle.

Is foundation even aware of this controversy/situation?
https://reviews.llvm.org/D36836#1021863, which is the last response i got, was 2.5 years ago.
For all we/i know this has gone off their radar.
I understand that it is fully possible that they simply haven't gotten around to it,
but i think it would be important to check that it isn't the case of lost mail.

aaron.ballman edited reviewers, added: tonic, asl, hfinkel; removed: dberlin.Sep 24 2020, 8:29 AM

Rebased.

There is a number of official open-source LGPL-3 implementations already:

There are other open-source LGPL-3 implementations already:

There are other 3rd party implementations:

Quite honestly, i do not understand how did the license question arose.

It arose in a comment that I can't seem to get phab to show me the context for (which is a bit strange, I don't think I've run into that before): https://reviews.llvm.org/D36836#877636 Perhaps part of this was carrying discussion over from the IRC channel?

Would have it been fine if i based this on the open-source-licensed code?

I believe that would require legal analysis to answer.

Would have it not been? Would same license question be raised?

Likewise here (I suspect the answer would depend on what the license of the open source code is).

Somehow i don't think it would have been.

I don't wish to speculate about legal licensing issues on the mailing lists.

Is this really just about Copyright SonarSource S.A., 2018, Switzerland. All content is copyright protected. in https://www.sonarsource.com/docs/CognitiveComplexity.pdf ?
But that is only about the document, not the algorithm.
But even if we enternain the idea that all of the implementations must bow to that license,
then surely this is not the first code in LLVM that is implicitly/explicitly based on copyrighted doc.

This is rather frustrating.

I am sorry and I agree that it's frustrating.

As far as I know, this captures the current state of affairs: https://reviews.llvm.org/D36836#1031600

As far as I know, yes. Some further back&forth reiterated:

@Roman so you know, none of the non-SonarSource implementations have an official license from us.
We put the spec out in the world and we're happy when someone uses it. And that's all.
I appreciate how frustrated you must be with your implementation caught between a proverbial rock and a hard place.
Unfortunately, we (the company) just aren't willing to do the paperwork.
(G. Ann Campbell)

Personally, I'm hopeful that this is sufficient to get us unstuck, but I'm not willing to make the determination simply because of the typical community view on making licensing decisions.

and basically we're waiting for help from the foundation to clear the last hurdle.

Is foundation even aware of this controversy/situation?
https://reviews.llvm.org/D36836#1021863, which is the last response i got, was 2.5 years ago.
For all we/i know this has gone off their radar.
I understand that it is fully possible that they simply haven't gotten around to it,
but i think it would be important to check that it isn't the case of lost mail.

I totally agree that we should double-check as it feels like this fell off the radar and I appreciate you pinging the review. @chandlerc is no longer on the foundation board and it looks like Danny is no longer on phabricator, so I've added some current LLVM foundation board members as reviewers to see if any of them can help us make progress.

tonic added a comment.Sep 24 2020, 8:41 AM

Licensing questions are best sent to the board@llvm.org mailing address. Can you reach out there?

Hi all,

The LLVM foundation board discussed this offline -- it looks like the contributor is a bit confused about the LLVM project policies towards copyright, license, patent stuff etc. I really appreciate the dilligence here to capture the ideas that went into this code.

My understanding is that the code and documentation are not derived from anyone else's code or documentation -- they are fresh implementations based on a paper written in english prose. If that is the case, please remove the LICENSE.TXT file -- it isn't necessary and it is confusing for people. I would also recommend replacing all the references to sonar source with a functional description of what this patch is doing - we aren't talking about adding a feature because of sonar source, my understanding is that this is adding a cyclomatic complexity checker. If and when the feature evolves over time, it would be misleading for it to be called sonar source.

Please note that I didn't do a full code review here, others should do that.

-Chris

lebedev.ri retitled this revision from [clang-tidy] Implement sonarsource-function-cognitive-complexity check to [clang-tidy] Implement readability-function-cognitive-complexity check.

@aaron.ballman @alexfh it is my understanding that i'm to proceed with commit. No explicit acknowledgement from your side is needed.

Hi all,

The LLVM foundation board discussed this offline

Yay, thank you!

it looks like the contributor is a bit confused about the LLVM project policies towards copyright, license, patent stuff etc.

...

I really appreciate the dilligence here to capture the ideas that went into this code.

My understanding is that the code and documentation are not derived from anyone else's code or documentation -- they are fresh implementations based on a paper written in english prose. If that is the case,

That is the case, yes. This is based on the spec.

please remove the LICENSE.TXT file -- it isn't necessary and it is confusing for people. I would also recommend replacing all the references to sonar source with a functional description of what this patch is doing - we aren't talking about adding a feature because of sonar source,

Done, as much as possible. I'd still prefer to maintain a reference to the spec on which it is based.
Note that it was done because that is what was requested previously by reviewers.

my understanding is that this is adding a cyclomatic complexity checker.

Well, yes and no. Note that this is *NOT* a well-known cyclomatic complexity metric, but a novel cognitive complexity metric.

If and when the feature evolves over time, it would be misleading for it to be called sonar source.

Yes.

Please note that I didn't do a full code review here, others should do that.

All good on that front, this has been quite extensively reviewed years ago :)

-Chris

Roman

aaron.ballman added inline comments.Oct 2 2020, 12:11 PM
clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
94–97

I'm not certain this comment adds a whole lot.

155

outputed -> output

156

Criteria's -> Criteria

223

of binary -> of the binary

368–369

If there's not a node, do we really need to traverse it?

382

BinaryConditionalOperatorClass as well (for all the places we're dealing with ConditionalOperatorClass)?

389

Should IndirectGotoStmtClass be handled the same way?

411

BlockExprClass as well?

Should GNU statement expressions also be treated like a lambda or block?

429

SEHExceptStmtClass as well?

474

What about other special member functions like overloaded operators and whatnot? How about block declarations?

Namespaces is another one I was curious about but suspect may not want to be handled here.

clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h
17–21

This shouldn't be necessary, right?

Hi all,

The LLVM foundation board discussed this offline -- it looks like the contributor is a bit confused about the LLVM project policies towards copyright, license, patent stuff etc. I really appreciate the dilligence here to capture the ideas that went into this code.

My understanding is that the code and documentation are not derived from anyone else's code or documentation -- they are fresh implementations based on a paper written in english prose. If that is the case, please remove the LICENSE.TXT file -- it isn't necessary and it is confusing for people. I would also recommend replacing all the references to sonar source with a functional description of what this patch is doing - we aren't talking about adding a feature because of sonar source, my understanding is that this is adding a cyclomatic complexity checker. If and when the feature evolves over time, it would be misleading for it to be called sonar source.

Please note that I didn't do a full code review here, others should do that.

Thank you to you and the board for looking into this and helping us reach the right decision!

please remove the LICENSE.TXT file -- it isn't necessary and it is confusing for people. I would also recommend replacing all the references to sonar source with a functional description of what this patch is doing - we aren't talking about adding a feature because of sonar source,

Done, as much as possible. I'd still prefer to maintain a reference to the spec on which it is based.
Note that it was done because that is what was requested previously by reviewers.

I agree that we should still document that this is based off the Sonar Source paper because it's not a generic implementation but a specific one. I think you've struck a good balance with the current patch.

lebedev.ri updated this revision to Diff 295902.Oct 2 2020, 1:28 PM
lebedev.ri marked 10 inline comments as done.

Hi all,

The LLVM foundation board discussed this offline -- it looks like the contributor is a bit confused about the LLVM project policies towards copyright, license, patent stuff etc. I really appreciate the dilligence here to capture the ideas that went into this code.

My understanding is that the code and documentation are not derived from anyone else's code or documentation -- they are fresh implementations based on a paper written in english prose. If that is the case, please remove the LICENSE.TXT file -- it isn't necessary and it is confusing for people. I would also recommend replacing all the references to sonar source with a functional description of what this patch is doing - we aren't talking about adding a feature because of sonar source, my understanding is that this is adding a cyclomatic complexity checker. If and when the feature evolves over time, it would be misleading for it to be called sonar source.

Please note that I didn't do a full code review here, others should do that.

Thank you to you and the board for looking into this and helping us reach the right decision!

+1.

please remove the LICENSE.TXT file -- it isn't necessary and it is confusing for people. I would also recommend replacing all the references to sonar source with a functional description of what this patch is doing - we aren't talking about adding a feature because of sonar source,

Done, as much as possible. I'd still prefer to maintain a reference to the spec on which it is based.
Note that it was done because that is what was requested previously by reviewers.

I agree that we should still document that this is based off the Sonar Source paper because it's not a generic implementation but a specific one. I think you've struck a good balance with the current patch.

Good!

I've addressed most of the review comments, excluding the one about Microsoft SEH support - i'd prefer to leave that for a follow-up :)
There may be other straddlers anyway.

clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
368–369

This is consistent across this whole FunctionASTVisitor, and is consistent with other RecursiveASTVisitor<>s,
so i'll leave this as-is. If this is wrong, other places should be changed too.

382

No, BinaryConditionalOperator is explicitly exempt.

389

Right, it should be.

411

BlockExprClass as well?

Thank you for the example, that seems obvious & straight-forward to support.
As discussed, we should handle decls thought, not exprs.

Should GNU statement expressions also be treated like a lambda or block?

Hm, i guess it should be.

429

I'm very much unfamiliar with that extension.
I would prefer to leave it as-is for now.

474

What about other special member functions like overloaded operators and whatnot?

Those are CXXMethodDecl, but i'll add a test.

How about block declarations?

Right, we should handle them, not blockstatements.

Namespaces is another one I was curious about but suspect may not want to be handled here.

To the best of my knowledge, those can only appear at the global scope.

lebedev.ri updated this revision to Diff 295904.Oct 2 2020, 1:35 PM

Maybe fine tune comments - we count block decls, not block statements/expressions.

aaron.ballman added inline comments.Oct 2 2020, 1:38 PM
clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
368–369

Fine by me, thanks.

382

I didn't see anything in the paper that talked about this (it's a GNU extension). I feel like anywhere we handle a ternary conditional we should also handle a binary conditional, e.g.,

foo ? foo : bar; // ternary form
foo ? : bar; // binary form of the same thing

but maybe I'm misunderstanding something.

429

I'm fine handling it in a follow-up. I suspect there may be other esoteric AST nodes we want to add, I just tried to hit the ones that were most obvious to me.

474

Namespaces is another one I was curious about but suspect may not want to be handled here.

To the best of my knowledge, those can only appear at the global scope.

They can, but I wasn't certain if they'd add to complexity or not given that you can nest them deeply. But now I see that the paper doesn't mention them in terms of C++ but does talk about something similar with JavaScript that suggests they should not contribute to complexity.

lebedev.ri marked 4 inline comments as done.Oct 2 2020, 1:54 PM
lebedev.ri added inline comments.
clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
382

As discussed, it does seem like they should at least cause increase in nesting level.
I suspect, they should also cause increase in cognitive complexity IFF they're nested somehow,
because foo ?: bar?: baz doesn't really seem straight-forward.
But likewise, they're an extension, so i think we could fine-tune that later.

429

Okay, thanks.

474

What i mean is, can you define a new namespace within the function's body?
I don't believe that to be possible, therefore we shouldn't/couldn't care about that,
because we start at function scope.

aaron.ballman accepted this revision.Oct 2 2020, 2:11 PM

LGTM, thank you for your patience with the process.

clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
382

Agreed, this can be done in a follow-up.

474

What i mean is, can you define a new namespace within the function's body?

You cannot.

I don't believe that to be possible, therefore we shouldn't/couldn't care about that, because we start at function scope.

Agreed.

lebedev.ri marked 3 inline comments as done.Oct 2 2020, 2:14 PM

@aaron.ballman @JonasToth thank you for the review!
@tonic @lattner thank you for helping resolve the misunderstading :)

! In D36836#2309432, @aaron.ballman wrote:
LGTM, thank you for your patience with the process.

HURRAY!

lattner removed a subscriber: lattner.Oct 2 2020, 2:28 PM
This revision was not accepted when it landed; it landed in state Needs Review.Oct 2 2020, 2:31 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.

Hm, let me take a look..

Will push likely fix once tests pass locally.

Hm, let me take a look..

Will push likely fix once tests pass locally.

rG1596cc83509342eb37dbfe6b95e906759afc6741, let's see if that does it.