Enable all DCHECKs

Question

Enable all DCHECKs

Closed this issue 5 years ago · 6 comments

An umbrella issue for discussions and tracking the progress.

@deepak1556, if I remember it right you were the one who really wanted it to happen ) May I suggest an approach how we can do it.

Fixing all DCHECKs at once seems to be quite difficult, so maybe we could enable DCHECKs and comment out all that fail and then fix them one by one?
It would allow us to make sure we won't break anything while we work on other stuff
and also we would be able to split the work by area of expertise.

Kudos to @Tyriar for the idea.
/cc @alespergl, @groundwater

Answer 1 · 2017-09-25T02:21:58.000Z

Thanks for creating the issue. It sounds good, but most of the DCHECK failures we hit are in the content layer not sure if we can comment them out and work our way up. I started with fixing the basic failure of app being initialized on the wrong thread and thereafter started fixing tests in each module. Although I am not sure if we have tests to cover all scenarios, but it seems to be a good start. Will have branch set up in Electron soon to track #327. Here is a list of failures I encountered so far https://gist.github.com/deepak1556/cb4d531e5dc0219e87f90a668ae90c5f

Answer 2 · 2017-09-25T10:01:14.000Z

If there are tons of DHECKS being hit when probably it won't make sense to create a patch to comment them all out. I was just thinking that it would be great to have DCHECKs enabled for the upgrades
when we can (unintentionally) break even more stuff )
It's up to you to decide though.

Answer 3 · 2017-09-25T14:30:39.000Z

I agree it's almost impossible to comment out all the DCHECKs that are being hit. There's just too many, that's why I disabled them universally.
What we can do though is to use a build flag to disable DCHECKs selectively per some larger components. That would at least prevent further regressions in the rest of the code base.

Answer 4 · 2017-09-30T12:56:51.000Z

@deepak1556 @alespergl
If it's not possible to disable all individual failing DCHECKs right now,
maybe we can do it when most of them are fixed? Just to enable them globally earlier.
Does it make sense?

Answer 5 · 2017-10-01T14:29:33.000Z

If a huge amount of DCHECKs are being hit there isn't much point in disabling selectively. The effort would be better spent identifying the patches that are causing the issues and fix or upstream to get them passing again.

Answer 6 · 2019-07-31T22:50:22.000Z

Hey there,

It's been a long time coming but the libchromiumcontent repository is being archived as it is no longer used in the Electron build system (🎉 this is a good thing).

If your issue is still relevant to Electron in it's current build system (please verify first) then you should probably raise a similar issue on electron/electron.

Thanks 😄