[cfe-dev] How to propose patches ?

[Argyrios Kyrtzidis]

Hi Matthieu,

Posting to cfe-commits is the way to go; most clang-devs are pretty busy at the moment so it's a bit of bad timing, but I "pledge" to review your patches :-)

-Argyrios 

On May 18, 2011, at 10:07 AM, Matthieu Monrocq wrote:

> 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
> <llvm_stringref_undefined_behavior.diff><clang_non_virtual_destructor.diff><clang_stringrefize_diagnostics.diff>_______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev