[PATCH] [-cxx-abi microsoft] Mangle declarations inside extern "C"

[Charles Davis]

  Other than these minor points, LGTM.


================
Comment at: lib/AST/MicrosoftMangle.cpp:227-228
@@ -203,8 +226,4 @@
 
-  // Clang's "overloadable" attribute extension to C/C++ implies name mangling
-  // (always) as does passing a C++ member function and a function
-  // whose name is not a simple identifier.
   const FunctionDecl *FD = dyn_cast<FunctionDecl>(D);
-  if (FD && (FD->hasAttr<OverloadableAttr>() || isa<CXXMethodDecl>(FD) ||
-             !FD->getDeclName().isIdentifier()))
-    return true;
+  if (FD) {
+    LanguageLinkage L = FD->getLanguageLinkage();
----------------
You can combine these two lines like so:

  lang=c++
  if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {

This is an extremely common pattern in the LLVM codebase. In fact, the only time you shouldn't do this is when you want to use the variable outside the `if` block.

================
Comment at: lib/AST/MicrosoftMangle.cpp:252-253
@@ -215,5 +251,4 @@
 
-  // Variables at global scope with internal linkage are not mangled.
-  if (!FD) {
-    const DeclContext *DC = D->getDeclContext();
-    if (DC->isTranslationUnit() && D->getFormalLinkage() == InternalLinkage)
+  const VarDecl *VD = dyn_cast<VarDecl>(D);
+  if (VD) {
+    // C variables are not mangled.
----------------
These two can be combined, too.

================
Comment at: lib/AST/MicrosoftMangle.cpp:258
@@ -222,4 +257,3 @@
 
-  // C functions and "main" are not mangled.
-  if ((FD && FD->isMain()) || isInCLinkageSpecification(D))
-    return false;
+    // Variables at global scope with non-internal linkage are not mangled
+    const DeclContext *DC = getEffectiveDeclContext(D);
----------------
You're missing a period at the end of this sentence.


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