[cfe-commits] [PATCH] Properly check for unused comparisons in arguments to function-like macros

[Matt Beaumont-Gay]

  I sort of don't care about the macro body case right now, both since I don't want to enshrine the current (sort of dumb) behavior in a test case, and since I'm going to fix it shortly with test cases for the correct behavior.

  Getting clever with looking at the sources of the start and end locations sounds like... more code. I recall you saying something recently about the YAGNI philosophy? :)

  Richard, I don't quite understand your test case, and besides I think it's a test for -Wunused-value rather than -Wunused-comparison?


================
Comment at: include/clang/Basic/SourceManager.h:1135-1136
@@ +1134,4 @@
+  /// \brief Tests whether the given source location represents the expansion of
+  /// a macro body (as opposed to an expansion of the argument to a
+  /// function-like macro).
+  bool isMacroBodyExpansion(SourceLocation Loc) const;
----------------
Chandler Carruth wrote:
> I'd put the parenthetical in its own sentence and not in the brief.
Done.

================
Comment at: lib/Basic/SourceManager.cpp:986-987
@@ +985,4 @@
+  FileID FID = getFileID(Loc);
+  const SrcMgr::SLocEntry *E = &getSLocEntry(FID);
+  const SrcMgr::ExpansionInfo &Expansion = E->getExpansion();
+  return Expansion.isMacroBodyExpansion();
----------------
Chandler Carruth wrote:
> Why make E a pointer?
> 
> Actually, why make a temporary? getSLocEntry(FID).getExpansion() not work?
> 
> I see its because that's what is done in the prior function. Perhaps both should be fixed, but whatev...
Done for both methods.


http://llvm-reviews.chandlerc.com/D259