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

Reply via email to