[CTU] Do not allow different CPP dialects in CTU

[martong]

Summary

If CPP dialects are different then return with error.

Consider this STL code:

template<typename _Alloc>
  struct __alloc_traits
#if __cplusplus >= 201103L
  : std::allocator_traits<_Alloc>
#endif
  { // ...
  };

This class template would create ODR errors during merging the two units,
since in one translation unit the class template has a base class, however
in the other unit it has none.

[a_sidorin]

Hi Gabor,
Please find my comments inline.

lib/CrossTU/CrossTranslationUnit.cpp

237

The comment change looks strange.

259

Should we create another counter, like NumLangDialectMismatch?

[martong]

• Revert comment change
• Add lang_dialect_mismatch and stats for that

[martong]

lib/CrossTU/CrossTranslationUnit.cpp

237

Ok, I reverted this hunk.

259

Okay, I added the new error code lang_dialect_mismatch too.

[• Quuxplusone]

Consider this STL code:

template<typename _Alloc>
  struct __alloc_traits
#if __cplusplus >= 201103L
  : std::allocator_traits<_Alloc>
#endif
  { // ...
  };

This class template would create ODR errors during merging the two units,
since in one translation unit the class template has a base class, however
in the other unit it has none.

How is #if __cplusplus >= 201103L qualitatively different from #ifndef NDEBUG or #if MYLIB_ABI_VERSION==2 or #if __DATE__ == "2018-04-01"?

[r.stahl]

lib/CrossTU/CrossTranslationUnit.cpp

255

This is likely to become a bug in the future, but I didn't see a better way to compare dialects.

Is there a way to add a test that lights up once there is a new dialect?

Would it be too pessimistic to compare the entire LangOpts? Some stuff in there should even still produce errors, right? For example "GnuMode". I skimmed over them and didn't find an obvious one that would differ between translation units of the same project. Maybe the template depth could be set selectively, but to fix that we could mask "COMPATIBLE_" and "BENIGN_" opts.

[martong]

How is #if cplusplus >= 201103L qualitatively different from #ifndef NDEBUG or #if MYLIB_ABI_VERSION==2 or #if DATE__ == "2018-04-01"?

Ideally, all macros should be the same in the two TUs... If we were very strict then we could check for that, but that might be just too strict.

If there is an ODR error (either because of a macro mismatch or for other reasons) then structural equivalency will diagnose that, but may be nontrivial for the user to identify what causes the ODR error.
Checking the language and the language dialect is easy and it prevents such disturbing errors early.

[martong]

lib/CrossTU/CrossTranslationUnit.cpp

255

This is likely to become a bug in the future, but I didn't see a better way to compare dialects.
Is there a way to add a test that lights up once there is a new dialect?

LANGOPTS are defined as bitfields: #define LANGOPT(Name, Bits, Default, Description) unsigned Name : Bits;
I can't think of any solution to avoid the future bug, because there is no way to enumerate all the C++ dialects.
I am going to ask around about this on cfe-dev, maybe somebody will have an idea.

Would it be too pessimistic to compare the entire LangOpts?

I think that would be too pessimistic.

[xazax.hun]

LGTM! I think we should commit this as is for now but maybe adding a TODO comment to summarize the problem would be nice. Maybe we could have an isSameDialect or similar method within LangOpts, so it is harder to break this code.

include/clang/CrossTU/CrossTranslationUnit.h

enum class index_error_code {
   missing_index_file,
   invalid_index_format,
   multiple_definitions,
   missing_definition,
   failed_import,
   failed_to_get_external_ast,
   failed_to_generate_usr,
   triple_mismatch,
-  lang_mismatch
+  lang_mismatch,
+  lang_dialect_mismatch
 };
 
 class IndexError : public llvm::ErrorInfo<IndexError> {
 public:
   static char ID;
   IndexError(index_error_code C) : Code(C), LineNo(0) {}
   IndexError(index_error_code C, std::string FileName, int LineNo = 0)
       : Code(C), FileName(std::move(FileName)), LineNo(LineNo) {}

lib/CrossTU/CrossTranslationUnit.cpp

 STATISTIC(
     NumNotInOtherTU,
     "The # of getCTUDefinition called but the function is not in any other TU");
 STATISTIC(NumGetCTUSuccess,
           "The # of getCTUDefinition successfully returned the "
           "requested function's body");
 STATISTIC(NumTripleMismatch, "The # of triple mismatches");
 STATISTIC(NumLangMismatch, "The # of language mismatches");
+STATISTIC(NumLangDialectMismatch, "The # of language dialect mismatches");
 
 // Same as Triple's equality operator, but we check a field only if that is
 // known in both instances.
 bool hasEqualKnownFields(const llvm::Triple &Lhs, const llvm::Triple &Rhs) {
   using llvm::Triple;
   if (Lhs.getArch() != Triple::UnknownArch &&
       Rhs.getArch() != Triple::UnknownArch && Lhs.getArch() != Rhs.getArch())
     return false;
std::string message(int Condition) const override {
     case index_error_code::failed_to_get_external_ast:
       return "Failed to load external AST source.";
     case index_error_code::failed_to_generate_usr:
       return "Failed to generate USR.";
     case index_error_code::triple_mismatch:
       return "Triple mismatch";
     case index_error_code::lang_mismatch:
       return "Language mismatch";
+    case index_error_code::lang_dialect_mismatch:
+      return "Language dialect mismatch";
     }
     llvm_unreachable("Unrecognized index_error_code.");
   }
 };
 
 static llvm::ManagedStatic<IndexErrorCategory> Category;
 } // end anonymous namespace
 
if (!hasEqualKnownFields(TripleTo, TripleFrom)) {
     ++NumTripleMismatch;
     return llvm::make_error<IndexError>(index_error_code::triple_mismatch,
                                         Unit->getMainFileName(), TripleTo.str(),
                                         TripleFrom.str());
   }
 
   const auto &LangTo = Context.getLangOpts();
   const auto &LangFrom = Unit->getASTContext().getLangOpts();
+
   // FIXME: Currenty we do not support CTU across C++ and C and across
   // different dialects of C++.
   if (LangTo.CPlusPlus != LangFrom.CPlusPlus) {
     ++NumLangMismatch;
     return llvm::make_error<IndexError>(index_error_code::lang_mismatch);
   }
 
+  // If CPP dialects are different then return with error.
+  //
+  // Consider this STL code:
+  //   template<typename _Alloc>
+  //     struct __alloc_traits
+  //   #if __cplusplus >= 201103L
+  //     : std::allocator_traits<_Alloc>
+  //   #endif
+  //     { // ...
+  //     };
+  // This class template would create ODR errors during merging the two units,
+  // since in one translation unit the class template has a base class, however
+  // in the other unit it has none.
+  if (LangTo.CPlusPlus11 != LangFrom.CPlusPlus11 ||
+      LangTo.CPlusPlus14 != LangFrom.CPlusPlus14 ||
+      LangTo.CPlusPlus17 != LangFrom.CPlusPlus17 ||
+      LangTo.CPlusPlus2a != LangFrom.CPlusPlus2a) {
+    ++NumLangDialectMismatch;
+    return llvm::make_error<IndexError>(
+        index_error_code::lang_dialect_mismatch);
+  }
+
   TranslationUnitDecl *TU = Unit->getASTContext().getTranslationUnitDecl();
   if (const FunctionDecl *ResultDecl =
           findFunctionInDeclContext(TU, LookupFnName))
     return importDefinition(ResultDecl);
   return llvm::make_error<IndexError>(index_error_code::failed_import);
 }
 
 void CrossTranslationUnitContext::emitCrossTUDiagnostics(const IndexError &IE) {