[PATCH] D17348: Add -Wcast-calling-convention to warn when casting away calling conventions

[Nico Weber via cfe-commits]

thakis added a comment.

Nice!


================
Comment at: include/clang/Basic/DiagnosticGroups.td:294
@@ -293,2 +293,3 @@
 def SelTypeCast : DiagGroup<"cast-of-sel-type">;
+def CastCallingConvention : DiagGroup<"cast-calling-convention">;
 def FunctionDefInObjCContainer : DiagGroup<"function-def-in-objc-container">;
----------------
nit: since this isn't referenced in this file, consider using an unnamed inline `InGroup<DiagGroup<"cast-calling-convention">` in the other file instead

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6515
@@ +6514,3 @@
+def warn_cast_calling_conv : Warning<
+  "cast between incompatible calling conventions '%0' and '%1'">,
+  InGroup<CastCallingConvention>, DefaultIgnore;
----------------
Maybe add ", likely to abort at runtime" to communicate the gravity of the situation better.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6516
@@ +6515,3 @@
+  "cast between incompatible calling conventions '%0' and '%1'">,
+  InGroup<CastCallingConvention>, DefaultIgnore;
+def note_change_calling_conv_fixit : Note<
----------------
I'd build a large codebase of your choice with this enabled and see how it does. If it does well I'd land it default-on. (I spent some time enabling DefaultIgnore warnings that nobody turned on after they landed recently, and I'd prefer to not having to do this for this warning too :-) )

================
Comment at: lib/Sema/SemaCast.cpp:1762
@@ +1761,3 @@
+
+  // The source expression is a pointer to a known function defined in this TU.
+  // Diagnose this cast, as it is probably bad.
----------------
If it's an inline function in a header, would we want to warn? Should this check if the body's SourceLocation is from the main file?

================
Comment at: lib/Sema/SemaCast.cpp:1775
@@ +1774,3 @@
+  Self.Diag(NameLoc, diag::note_change_calling_conv_fixit)
+      << FD << DstCCName << FixItHint::CreateInsertion(NameLoc, CCAttrText);
+}
----------------
Consider doing something like r164858 did to see if there's a define for the calling convention (WINAPI or what have you) and suggest that instead


http://reviews.llvm.org/D17348