On May 29, 2012, at 6:24 PM, Chandler Carruth wrote: > On Tue, May 29, 2012 at 5:34 PM, Anna Zaks <[email protected]> wrote: > + // If the function is defined as a builtin macro, do not show macro > + // expansion. > + SourceLocation SL = SizeOfArg->getExprLoc(); > + SourceRange DSR = Dest->getSourceRange(); > + SourceRange SSR = SizeOfArg->getSourceRange(); > + SourceManager &SM = PP.getSourceManager(); > + > + if (SM.isMacroArgExpansion(SL)) { > + ReadableName = Lexer::getImmediateMacroName(SL, SM, LangOpts); > + SL = SM.getSpellingLoc(SL); > + DSR = SourceRange(SM.getSpellingLoc(DSR.getBegin()), > + SM.getSpellingLoc(DSR.getEnd())); > + SSR = SourceRange(SM.getSpellingLoc(SSR.getBegin()), > + SM.getSpellingLoc(SSR.getEnd())); > + } > > I'm not terribly fond of this kind of highly customized printing logic when > we may not even emit the diagnostic. >
Are you concerned about performance? I doubt that this warning is triggered much and I only added retrieval of more suitable source location information in case the faulty call is a builtin macro. As is the warning quality is unacceptable when memcpy(and others) are defined as builtins: -/Users/anya/tmp/ex.m:3:27: warning: argument to 'sizeof' in '__builtin___memcpy_chk' call is the same expression as the - destination; did you mean to dereference it? [-Wsizeof-pointer-memaccess] - memcpy(to, from, sizeof(to)); - ~~~~~~~~~~~~~~~~~~~~~~~~^~~~ -/usr/include/secure/_string.h:55:41: note: expanded from macro 'memcpy' - ? __builtin___memcpy_chk (dest, src, len, __darwin_obsz0 (dest)) \ +/Users/anya/tmp/ex.m:3:27: warning: argument to 'sizeof' in memcpy' call is the same expression as the + destination; did you mean to dereference it? [-Wsizeof-pointer-memaccess] + memcpy(to, from, sizeof(to)); ~~~~~~~~~~~~~~~~~~~~~~~~^~~~ > I wonder -- can we construct this information from within the diagnostic > printer, and get it to fire in more circumstances? If not, is there a way to > simply tell the diagnostic engine the intent -- to print the builtin name -- > rather than hard coding the logic to get from here to there? I agree that we should come up with a better solution for handling builtin printing, which would apply in all cases. Since diagnostic engine works with SourceLocations, I am not sure it can detect if a location is a builtin or not. Teaching it to print builtins sounds like a plausible idea. However, we might be breaking the abstraction by doing that. (We might as well teach it about printing functions.) I am not very well familiar with that code. This band-aid fix can be removed after the proper solution is in place. Anna.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
