Skip to content

C#: Fix a bug in ThrowingCallable #637

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 12, 2018

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Dec 7, 2018

A method such as

void M()
{
    throw new Exception();
}

was incorrectly not categorized as a ThrowingCallable, that is, a callable that always throws an exception upon invocation.

A method such as

```
void M()
{
    throw new Exception();
}
```

was incorrectly not categorized as a `ThrowingCallable`, that is, a callable
that always throws an exception upon invocation.
@hvitved hvitved added the C# label Dec 7, 2018
@hvitved hvitved requested a review from calumgrant December 7, 2018 09:58
@hvitved hvitved requested a review from a team as a code owner December 7, 2018 09:58
c.getExceptionClass() = te.getThrownExceptionType() and
// For stub implementations, there may exist proper implementations that are not seen
// during compilation, so we conservatively rule those out
not isStub(te)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realise this is the previous logic, but doesn't it make sense to have isStub at a higher level, for example the callable or the class?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps at the callable level, but usually that would be one-to-one with the throw statement. I don't think the whole class needs to be a stub for one of the callables to be a stub.

Copy link
Contributor

@calumgrant calumgrant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@calumgrant calumgrant merged commit 8e546a3 into github:master Dec 12, 2018
@hvitved hvitved deleted the csharp/cfg/throwing-callable branch December 12, 2018 19:28
cklin pushed a commit that referenced this pull request May 23, 2022
…zers

Fix sanitization by strings.Replace[All] in go/unsafe-quoting and go/log-injection
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants