[PATCH] Recover from missing typenames on template args for MSVC compatibility

Reid Kleckner rnk at google.com
Tue Jun 10 15:00:52 PDT 2014


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3115
@@ +3114,3 @@
+def ext_ms_template_type_arg_missing_typename : ExtWarn<
+  "template type parameter requires a type; assuming 'typename' was intended as a Microsoft extension">,
+  InGroup<Microsoft>;
----------------
Richard Smith wrote:
> This diagnostic text isn't going to work well with `-fms-extensions -pedantic-errors`. Maybe phrase this more similarly to the existing cases:
> 
>   "template argument for template type parameter must be a type; omitted 'typename' is a Microsoft extension"
> 
> (I'm also not convinced that we should call this a "Microsoft extension", but I can live with it.)
I was trying to make the diagnostic more concise, but I can put it back.  I think I understand the issue with -pedantic-errors.  Is it that we shouldn't make the diagnostic sound like we recovered if it gets upgraded from an ExtWarn to an error?

I agree that "Microsoft extension" is maybe not the best way to describe these types of warnings, but it's consistent with every other -Wmicrosoft diagnostic.

================
Comment at: lib/Sema/SemaExpr.cpp:2196
@@ +2195,3 @@
+        << SourceRange(SS.getBeginLoc(), NameInfo.getEndLoc())
+        << FixItHint::CreateInsertion(SS.getBeginLoc(), "typename ");
+    return ExprError();
----------------
Richard Smith wrote:
> Our policy for fixits is to only put them on errors/warnings if we recover as if the fixit were applied (this is important for -fixit behavior). Either remove the fixit for now or move it to a note.
OK, I'll remove the fixit.  The recovery was unnecessary because now we recover prior to instantiation.

================
Comment at: lib/Sema/SemaTemplate.cpp:5921-5923
@@ -5896,2 +5920,5 @@
 
+// FIXME: We probably come here when the user implicitly instantiates a
+// template.  We're dropping Converted here on the floor instead of using our
+// corrected AST to continue parsing.
 DeclResult
----------------
Richard Smith wrote:
> This comment doesn't look quite right: we don't get here for references to a template-id.  We might have an explicit instantiation or an explicit or partial specialization, but the one thing we *don't* have is an implicit instantiation.
> 
> This would also make more sense down next to `Converted`.
Woops, this should be reverted.  I wrote this when I was half-way through understanding the problem.

http://reviews.llvm.org/D4049






More information about the cfe-commits mailing list