Details

doc/developer-notes.md
392 ↗	(On Diff #16686)	The suppression list probably deserves an update, since it has been extracted from core some time ago. But there are cases that will only trigger with some compilers and compiler versions (e.g. there is a `vptr` reported in `avalanche_tests` with clang-7), so I'd better leave it to prevent people from facing issues.

doc/developer-notes.md
364 ↗	(On Diff #16686)	Why is debug necessary here?

doc/developer-notes.md
364 ↗	(On Diff #16686)	It is not necessary, but can help in having a better trace if an failure is triggered.

doc/developer-notes.md
361 ↗	(On Diff #16686)	IMO these could just be added by the sanitizer module to some place and the test module can query these. There is no reason for people to have to deal with this.
392 ↗	(On Diff #16686)	We already require clang format 8, and this is packaged in most distros. We don't support 7 and less. Backing old stuff in there is just asking for regressions.

doc/developer-notes.md
244 ↗	(On Diff #16686)	There is one target per test now and it can be used instead of this.

Use the dedicated target for building and running script_tests.

jasonbcox added inline comments.

doc/developer-notes.md
311 ↗	(On Diff #16721)	Doesn't clang need to be used as well? My understanding is sanitizer support exists for gcc but we do not support it. Same comment/question for the examples below.

Fabien planned changes to this revision.Mar 3 2020, 20:32

Fabien added inline comments.

doc/developer-notes.md
311 ↗	(On Diff #16721)	AFAIK both can work, it's more a matter of version. But since we are using clang, I can add a note to state that it should preferably run with clang >8, so it will address a previous comment from @deadalnix at the same time.

Add a recommendation for using clang over gcc with sanitizers.
No clang version but "recent" in order to reduce maintenance of the doc.

doc/developer-notes.md
332 ↗	(On Diff #16747)	Why would we limit the sanitizer uses to clang?
342 ↗	(On Diff #16747)	Write paragraph as paragraphs.
381 ↗	(On Diff #16747)	I will reiterate that it would be very beneficial that this is done automatically when the sanitizer is enabled.

Fix the suppression files path, actually make the paragraph look like a paragraph.

doc/developer-notes.md
332 ↗	(On Diff #16747)	That's just what we are using on CI, as we had various issues with GCC in the past (I don't remember the details, I think there was some difference in the suppression file with some sanitizers, and some false positive on system libraries). I can certainly give it another try, the suppression list deserves an update anyway.
381 ↗	(On Diff #16747)	I agree it would be a good improvement, but it's non trivial and out of scope for this diff.

deadalnix requested changes to this revision.Mar 6 2020, 16:18

deadalnix added inline comments.

doc/developer-notes.md
334 ↗	(On Diff #16802)	I think this still need to be changed. This convey the idea that our goal is to be green in clang, whereas it is to be clean with gcc too.