[clang] 64788d7 - [clang] Include missing LangOpts in `getModuleHash`.

[Michael Spencer via cfe-commits]

Author: Michael Spencer
Date: 2020-07-07T17:13:23-06:00
New Revision: 64788d7d5377345af5e3080d26cb6a76c324ab5b

URL: https://github.com/llvm/llvm-project/commit/64788d7d5377345af5e3080d26cb6a76c324ab5b
DIFF: https://github.com/llvm/llvm-project/commit/64788d7d5377345af5e3080d26cb6a76c324ab5b.diff

LOG: [clang] Include missing LangOpts in `getModuleHash`.

`ObjCRuntime` and `CommentOpts.BlockCommandNames` are checked by
`ASTReader::checkLanguageOptions`, but are not part of the module
context hash. This can lead to errors when using implicit modules if
different TUs have different values for these options when using the
same module cache.

This was not hit very often due to the rare usage of
`-fblock-command-names=` and that `ObjCRuntime` is by default set by
the target triple, which is part of the existing context hash.

Added: 
    

Modified: 
    clang/include/clang/Basic/ObjCRuntime.h
    clang/lib/Frontend/CompilerInvocation.cpp
    clang/test/Modules/context-hash.c
    llvm/include/llvm/Support/VersionTuple.h

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/ObjCRuntime.h b/clang/include/clang/Basic/ObjCRuntime.h
index 1c4a69269dee..26403bfa98c9 100644
--- a/clang/include/clang/Basic/ObjCRuntime.h
+++ b/clang/include/clang/Basic/ObjCRuntime.h
@@ -476,6 +476,10 @@ class ObjCRuntime {
   friend bool operator!=(const ObjCRuntime &left, const ObjCRuntime &right) {
     return !(left == right);
   }
+
+  friend llvm::hash_code hash_value(const ObjCRuntime &OCR) {
+    return llvm::hash_combine(OCR.getKind(), OCR.getVersion());
+  }
 };
 
 raw_ostream &operator<<(raw_ostream &out, const ObjCRuntime &value);

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index f58854cd9e08..6f6af917e3a3 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3852,6 +3852,10 @@ std::string CompilerInvocation::getModuleHash() const {
   for (StringRef Feature : LangOpts->ModuleFeatures)
     code = hash_combine(code, Feature);
 
+  code = hash_combine(code, LangOpts->ObjCRuntime);
+  const auto &BCN = LangOpts->CommentOpts.BlockCommandNames;
+  code = hash_combine(code, hash_combine_range(BCN.begin(), BCN.end()));
+
   // Extend the signature with the target options.
   code = hash_combine(code, TargetOpts->Triple, TargetOpts->CPU,
                       TargetOpts->ABI);

diff  --git a/clang/test/Modules/context-hash.c b/clang/test/Modules/context-hash.c
index 33dfb2f15a2c..8bb7422f6a54 100644
--- a/clang/test/Modules/context-hash.c
+++ b/clang/test/Modules/context-hash.c
@@ -1,3 +1,6 @@
+// This test verifies that only strict hashing includes search paths and
+// diagnostics in the module context hash.
+
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fsyntax-only -internal-isystem \
 // RUN:   %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps \
@@ -20,8 +23,25 @@
 // RUN: echo %t > %t.path
 // RUN: cat %t.path %t1 %t2 %t3 %t4 | FileCheck %s
 
-// This test verifies that only strict hashing includes search paths and
-// diagnostics in the module context hash.
+// This tests things verified by ASTReader::checkLanguageOptions that are not
+// part of LangOpts.def.
+
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fsyntax-only -internal-isystem \
+// RUN:   %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps \
+// RUN:   -fmodules-cache-path=%t -x objective-c %s -Rmodule-build 2> %t1
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fsyntax-only -internal-isystem \
+// RUN:   %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps \
+// RUN:   -fobjc-runtime=macosx-1.0.0.0 \
+// RUN:   -fmodules-cache-path=%t -x objective-c %s -Rmodule-build 2> %t2
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fsyntax-only -internal-isystem \
+// RUN:   %S/Inputs/System/usr/include -fmodules -fimplicit-module-maps \
+// RUN:   -fcomment-block-commands=lp,bj \
+// RUN:   -fmodules-cache-path=%t -x objective-c %s -Rmodule-build 2> %t3
+// RUN: echo %t > %t.path
+// RUN: cat %t.path %t1 %t2 %t3 | FileCheck --check-prefix=LANGOPTS %s
 
 #include <stdio.h>
 
@@ -32,3 +52,10 @@
 // CHECK: cstd-[[AST_HASH]].pcm'
 // CHECK-NOT: building module 'cstd' as '{{.*[/\\]}}[[CONTEXT_HASH]]{{[/\\]}}
 // CHECK: cstd-[[AST_HASH]].pcm'
+
+// LANGOPTS: [[PREFIX:(.*[/\\])+[a-zA-Z0-9.-]+]]
+// LANGOPTS: building module 'cstd' as '[[PREFIX]]{{[/\\]}}[[CONTEXT_HASH:[A-Z0-9]+]]{{[/\\]}}cstd-[[AST_HASH:[A-Z0-9]+]].pcm'
+// LANGOPTS-NOT: building module 'cstd' as '{{.*[/\\]}}[[CONTEXT_HASH]]{{[/\\]}}
+// LANGOPTS: cstd-[[AST_HASH]].pcm'
+// LANGOPTS-NOT: building module 'cstd' as '{{.*[/\\]}}[[CONTEXT_HASH]]{{[/\\]}}
+// LANGOPTS: cstd-[[AST_HASH]].pcm'

diff  --git a/llvm/include/llvm/Support/VersionTuple.h b/llvm/include/llvm/Support/VersionTuple.h
index ad89e40f0f14..6f3711f06f1a 100644
--- a/llvm/include/llvm/Support/VersionTuple.h
+++ b/llvm/include/llvm/Support/VersionTuple.h
@@ -14,6 +14,7 @@
 #ifndef LLVM_SUPPORT_VERSIONTUPLE_H
 #define LLVM_SUPPORT_VERSIONTUPLE_H
 
+#include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/Optional.h"
 #include <string>
 #include <tuple>
@@ -144,6 +145,10 @@ class VersionTuple {
     return !(X < Y);
   }
 
+  friend llvm::hash_code hash_value(const VersionTuple &VT) {
+    return llvm::hash_combine(VT.Major, VT.Minor, VT.Subminor, VT.Build);
+  }
+
   /// Retrieve a string representation of the version number.
   std::string getAsString() const;