On Tue, Mar 3, 2015 at 4:51 PM, Seth Cantrell <[email protected]> wrote:
> Does this patch look okay for me to go ahead and commit? > Yes, LGTM, thanks. > Thanks, > Seth > > > On Feb 27, 2015, at 8:30 PM, Seth Cantrell <[email protected]> > wrote: > > > On Feb 26, 2015, at 8:28 PM, Seth Cantrell <[email protected]> > wrote: > > > On Feb 26, 2015, at 7:24 PM, Richard Smith <[email protected]> wrote: > > On Thu, Feb 26, 2015 at 4:01 PM, Seth Cantrell <[email protected]> > wrote: > >> This enables a warning corresponding to gcc's warning of using %p >> modifiers with non-void* pointers, and to the printf/scanf specs which >> require void*. >> >> The warning is on by default for -Weverything users, off by default for >> others. It can be enabled with -pedantic or -Wformat-pedantic. It's not >> enabled by -Wformat. >> > > From a high level, this seems like a good approach to the problem, thanks. > > >> The patch includes tests and the clang-test target passes with this patch >> applied to a recent clang revision. >> >> I used clang-format to format my changes and the patch includes cleanup >> of a few trailing whitespace issues that were near my changes. >> >> See also the email thread at: >> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2015-February/041714.html > > > It doesn't seem ideal to call matchesType twice to find out which > diagnostic to issue; have you considered returning an enum { Match, > NoMatchPedantic, NoMatch }; or similar instead? > > > That did initially occur to me but I saw the matchesType usages in a few > other locations and wanted to keep a compatible API. It wasn't clear, for > example, whether the NoMatchPedantic option should convert to true or false > of if I should even support the current usage rather than make every use > check for each condition explicitly. > > I will go back and more carefully review the other usages to see what > refactoring I need to do. > > Thanks, > Seth > > > Here's an updated patch which takes the suggested approach. I did keep the > API compatible, so the matchesType's return value for mismatches converts > to false and the value for matches and pedantic mismatches converts to > true. This way most uses are unchanged and the code only distinguishes > between matches and pedantic mismatches in four checks. > > <0001-Add-a-format-warning-for-p-with-non-void-args.patch> > > > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
