[PATCH] D16770: [MS] PR26234: Allow typedef redefinition of equally qualified, sized and aligned types in C

[Reid Kleckner via cfe-commits]

rnk added inline comments.

================
Comment at: include/clang/AST/ASTContext.h:1771-1782
@@ -1770,1 +1770,14 @@
 
+  /// Return true is the given types are compatible in C from MSVC's point of
+  /// view.
+  //
+  // Conditions:
+  //   1. Both types are equally-qualified, sized and aligned;
+  //   2. Both types are either integer, floating-point, enumeral or pointers;
+  //   3. If pointers:
+  //     3.1. Levels of pointers are equal;
+  //     3.2. Pointee types are MSVC-compatible OR
+  //     3.3. One type points to void and another points to char.
+  //   4. Enumeral types are NOT compatible with floating-point types.
+  bool areMSCompatibleTypesInC(QualType OldType, QualType NewType);
+
----------------
Please update the comments to reflect that this only applies to typedef types.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:4265-4267
@@ -4264,2 +4264,5 @@
   InGroup<DiagGroup<"objc-forward-class-redefinition">>;
+def warn_benign_redefinition_different_typedef : ExtWarn<
+  "benign typedef redefinition with different types%diff{ ($ vs $)|}0,1 "
+  "is a Microsoft extension">, InGroup<Microsoft>;
 def err_redefinition_different_typedef : Error<
----------------
majnemer wrote:
> I think giving a warning that 'benign' sends the message that the code is OK and that the warning should be disabled.  I think we should say something like "typedef redefinition is ignored due to conflicting underlying type".
We aren't ignoring it because the underyling types differ, we're ignoring it because they are the same size and signedness and we're in MSVC quirks mode. Maybe "ignoring conflicting integer typedef redefinition%diff{...} as a Microsoft extension; types have the same width and signedness"? Is that too much text?

================
Comment at: lib/AST/ASTContext.cpp:6727-6728
@@ +6726,4 @@
+bool ASTContext::areMSCompatibleTypesInC(QualType OldType, QualType NewType) {
+  assert(getLangOpts().MicrosoftExt &&
+         "This routine must be called in Microsoft mode only!");
+  assert(!getLangOpts().CPlusPlus && !getLangOpts().ObjC1 &&
----------------
majnemer wrote:
> Shouldn't this be `MSVCCompat`, not `MicrosoftExt`?
+1


http://reviews.llvm.org/D16770