[cfe-dev] How to propose patches ?

[Matthieu Monrocq]

Hello clang :)

I have been working recently on a change to the Diagnostics in order to
enrich the current system, the end goal being to offer additional
explanations and help to the user (on top of the already displayed error
message).

I have proposed some patches on cfe-commits some time ago, and then
re-proposed them (rebased on current ToT), but I haven't yet heard from
anyone, so I guess I am doing something wrong.

At the moment, I simply send my proposals on cfe-commits (which seemed
appropriate), but perhaps that I should send them on this list instead ?

- Is there someone that I should put in copy ? (and where to find this
person, as I suppose it would depend on the area of expertise)

- Would it be better to ask for commit access, and have my changes validated
after the fact ?

I tried to look about it on the clang website, but it does not seem to be
mentioned.


Here is an excerpt of the last mail I sent, with the appropriate patches in
attachment:

1. A simple patch in StringRef.h:
> put assertions in StringRef(char const*) because it should not be
constructed from a null pointer (strlen has undefined behavior)
> replace memcmp with a version with asserts to against guard against null
arguments (because the default constructor of StringRef creates a null
pointer)

No functional change expected, merely an easier diagnostic (it sure helped
me track down a bug I had). I didn't notice any slow-down on my Debug+Assert
build playing the tests.

This can be found in llvm_stringref_undefined_behavior.diff.


2. A StringRef-ication of the DiagnosticIDs API and internals.

Simple grunt work, no functional change expected. I took the opportunity to
make some cleanup in lib/Lex/Pragma.cpp and lib/Frontend/Warnings.cpp taking
advantages of StringRef.

This can be found in clang_stringrefize_diagnostics.diff


3. A simple new diagnostic for non-virtual destructor in polymorphic
classes.

The difference with the existing diagnostic is that instead of warning at
the definition of the class, it instead warns when invoking `delete` on it.
Hopefully reducing the number of false positives.

Unfortunately it is incomplete since a "final" class should not trigger this
warning but this information does not seem to be available in the AST. I've
put a FIXME near the code.

It is interestingly a very small patch, which can be found in
clang_non_virtual_destructor.diff


The tests passes for all 3 patches (whether individually or as a group), at
least as much as tests ever passed on my system (~80 unexpected failures
because of my msys environment).


-- Matthieu
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110518/ec235644/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm_stringref_undefined_behavior.diff
Type: application/octet-stream
Size: 1379 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110518/ec235644/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang_non_virtual_destructor.diff
Type: application/octet-stream
Size: 6009 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110518/ec235644/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang_stringrefize_diagnostics.diff
Type: application/octet-stream
Size: 22033 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110518/ec235644/attachment-0002.obj>