Szelethus marked 10 inline comments as done.
Szelethus added inline comments.
================
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:667
+
+//===----------------------------------------------------------------------===//
+// Declarations of helper functions and data structures for expanding macros.
----------------
xazax.hun wrote:
> Maybe it would be worth to move these helpers to a separate file and add some
> documentation why we actually simulate the preprocessor rather than doing
> something else.
I'm actually a little unsure about that -- I have had similar thought, but the
placement here can be justified with a simple "Where else?". But that's a
little vague, I'll admit.
================
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:724
+ TokenPrinter Printer(OS, PP);
+ return { getExpandedMacroImpl(Printer, MacroLoc, PP), OS.str() };
+}
----------------
xazax.hun wrote:
> Maybe could directly return ExpansionInfo?
I renamed the function to be less confusing. Did this address your concern?
================
Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:753
+ // macro.
+ if (const MacroInfo *MI = PP.getMacroInfo(II)) {
+ getExpandedMacroImpl(Printer, T.getLocation(), PP);
----------------
Szelethus wrote:
> xazax.hun wrote:
> > This might not be completely correct. We are using the preprocessor state
> > after processing the whole translation unit rather than the preprocessor
> > state at the expansion location. This might not cause any problems unless
> > there is a collision between macro names and non-macro names. Also, undefs,
> > redefs might cause troubles.
> Yup, you're right, I'm afraid :/
>
> I'll investigate whether I can gather the macro definition at the spelling
> location with `Lexer`.
I'm really-really happy it didn't have to come to that.
https://reviews.llvm.org/D52794
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
