On Wed, Apr 25, 2012 at 8:42 AM, Dodji Seketeli <do...@redhat.com> wrote: > Gabriel Dos Reis <g...@integrable-solutions.net> writes: > >> On Wed, Apr 11, 2012 at 3:46 AM, Dodji Seketeli <do...@redhat.com> wrote: >>> There are various conversion related warnings that trigger on >>> potentially dangerous uses of NULL (or __null). NULL is defined as a >>> macro in a system header, so calling warning or warning_at on a virtual >>> location of NULL yields no diagnostic. So the test accompanying this >>> patch (as well as others), was failling when run with >>> -ftrack-macro-expansion. >>> >>> I think it's necessary to use the location of NULL that is in the main >>> source code (instead of, e.g, the spelling location that is in the >>> system header where the macro is defined) in those cases. Note that for >>> __null, we don't have the issue. >> >> I like the idea. However, you should write a separate function >> (get_null_ptr_cst_location?) that computes the location of the null >> pointer constant token and call it from where you need the location. > > OK. I have introduced such a new function and I gave it the slightly > more generic name expansion_point_location_if_in_system_header as I > suspect it can be useful for macros that are similar to NULL. I haven't > spotted such macros (from test regressions) yet, though.
Thanks. But a point of the suggestion was that we won't need the same comment/explanation duplicated over at least 3 places. Just one. All those three places deal exactly with one instance: null pointer constant. That deserves a function in and of itself, which is documented by the duplicated comments. Please make that change. Everything else is OK. thanks.
