balazske added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp:114-117 + const auto MacroIt = llvm::find_if( + PP.macros(), [&](const auto &K) { return K.first->getName() == Macro; }); + if (MacroIt == PP.macro_end()) + return llvm::None; ---------------- martong wrote: > martong wrote: > > Szelethus wrote: > > > NoQ wrote: > > > > martong wrote: > > > > > Szelethus wrote: > > > > > > This seems a bit clunky even for the `Preprocessor` -- how about > > > > > > > > > > > > ```lang=c++ > > > > > > const auto *MacroII = PP.getIdentifierInfo(Macro); > > > > > > if (!MacroII) > > > > > > return; > > > > > > const MacroInfo *MI = PP.getMacroInfo(MacroII); > > > > > > assert(MI); > > > > > > ``` > > > > > Ok, but we cannot assert on `MI`, because there may be cases when the > > > > > macro is not defined in a TU. In that case we should just return with > > > > > None. > > > > What exactly happens when the macro is `#undef`-ined and redefined? We > > > > get the last redefinition that's valid at the end of the translation > > > > unit, right? Can we check whether there are multiple definitions and > > > > guard against that? > > > Ugh, now that you say it that is a valid concern. I had to deal with that > > > back in the day: https://reviews.llvm.org/D52794?id=171962#inline-476352 > > Solving this does not seem easy in my opinion. To handle `#undef`s we > > should build an infrastructure where summaries can reference callable > > objects. This is necessary, because in `evalCall` the value of `EOF` would > > depend on the souce location of the call expression of the function with > > the summary. Not impossible to solve, but certainly introduces complexity. > > Do you think that the redefinition of EOF is so common? I mean maybe it is > > too much hassle for a very rare edge case (?). > The standard library (libc or libc++) should define EOF consitently in > stdio.h. > Now, if the application redefines the value of EOF then the code could be > broken, or at least it would not be compatible with libc. Consider the > following code that is perfectly legit if we don't redefine EOF, but if we do: > ``` > #include <stdio.h> > #include <stdlib.h> > #define EOF -2 // Here be dragons !!! > int main(void) > { > FILE* fp = fopen("test.txt", "r"); > int c; > while ((c = fgetc(fp)) != EOF) { // BOOM: Infinite loop !!! > putchar(c); > } > fclose(fp); > } > ``` > So, redefinition of EOF (or any standard macro) results in broken code. > And this is also a warning: > ``` > ) clang eof-inf.c > eof-inf.c:3:9: warning: 'EOF' macro redefined [-Wmacro-redefined] > #define EOF -2 // Here be dragons !!! > ^ > /usr/include/x86_64-linux-gnu/bits/libio.h:66:10: note: previous definition > is here > # define EOF (-1) > ^ > 1 warning generated. > ``` It should be possible to get the list of all macro definitions (of a macro). We can select the first item, or one that is inside a system header (by the SourceLocation). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74473/new/ https://reviews.llvm.org/D74473 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits