[PATCH] D48524: [ODRHash] Do not put elaborated types in the TypeMap

[Richard Trieu via Phabricator via cfe-commits]

rtrieu added a comment.

Hey Vassil,

I have been giving this a little more thought and I have a solution I'd like 
you to try out.  If the type pointers aren't unique enough, then the indexing 
scheme will not work for hashing.  Can you try this idea in your build 
environment and see if works for you?  That is,  use this version AddType:

  void ODRHash::AddType(const Type *T) {
assert(T && "Expecting non-null pointer.");
  
ODRTypeVisitor(ID, *this).Visit(T);
  }

If the recursion issue that originally made the indexing scheme necessary is no 
longer present, then this would be path forward for ODR Hashing.


Repository:
  rC Clang

https://reviews.llvm.org/D48524



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48524: [ODRHash] Do not put elaborated types in the TypeMap

[Richard Trieu via Phabricator via cfe-commits]

rtrieu added a comment.

Running this, I see that it causes a regression in one of the other ODR Hash 
tests.  Specifically, the diagnostics on line 1150 & 1151 of 
test/Modules/odr_hash.cpp are not seen.  Was that an expected side-effect of 
this patch?


Repository:
  rC Clang

https://reviews.llvm.org/D48524



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48524: [ODRHash] Do not put elaborated types in the TypeMap

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev created this revision.
v.g.vassilev added reviewers: rsmith, rtrieu, bruno.
Herald added a subscriber: cfe-commits.

r332281 (https://reviews.llvm.org/D45463) teaches the ASTContext to generate 
different elaborated types if there is an owning tag. This exposed a bug with 
our odr hasher: we add both types in the `TypeMap` which makes the hashes 
different.

This patch is a quick fix for the problem. However, it looks like the `TypeMap` 
in a way disables the nice cross-tu hashing logic.


Repository:
  rC Clang

https://reviews.llvm.org/D48524

Files:
  lib/AST/ODRHash.cpp
  test/Modules/Inputs/odr_hash-elaborated-types/first.h
  test/Modules/Inputs/odr_hash-elaborated-types/module.modulemap
  test/Modules/Inputs/odr_hash-elaborated-types/second.h
  test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h
  test/Modules/Inputs/odr_hash-elaborated-types/textual_time.h
  test/Modules/odr_hash-elaborated-types.cpp


Index: test/Modules/Inputs/odr_hash-elaborated-types/textual_time.h
===
--- test/Modules/Inputs/odr_hash-elaborated-types/textual_time.h
+++ test/Modules/Inputs/odr_hash-elaborated-types/textual_time.h
@@ -0,0 +1,6 @@
+#ifndef _TIME_H
+#define _TIME_H
+
+struct timespec { };
+
+#endif
Index: test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h
===
--- test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h
+++ test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h
@@ -0,0 +1,11 @@
+#ifndef _SYS_STAT_H
+#define _SYS_STAT_H
+
+#include "textual_time.h"
+
+struct stat {
+  struct timespec st_atim;
+  struct timespec st_mtim;
+};
+
+#endif
Index: test/Modules/Inputs/odr_hash-elaborated-types/second.h
===
--- test/Modules/Inputs/odr_hash-elaborated-types/second.h
+++ test/Modules/Inputs/odr_hash-elaborated-types/second.h
@@ -0,0 +1,6 @@
+#ifndef SECOND
+#define SECOND
+
+#include "textual_stat.h"
+
+#endif
Index: test/Modules/Inputs/odr_hash-elaborated-types/module.modulemap
===
--- test/Modules/Inputs/odr_hash-elaborated-types/module.modulemap
+++ test/Modules/Inputs/odr_hash-elaborated-types/module.modulemap
@@ -0,0 +1,5 @@
+module M {
+  module first { header "first.h" export *}
+  module second { header "second.h" export *}
+  export *
+}
Index: test/Modules/Inputs/odr_hash-elaborated-types/first.h
===
--- test/Modules/Inputs/odr_hash-elaborated-types/first.h
+++ test/Modules/Inputs/odr_hash-elaborated-types/first.h
@@ -0,0 +1,6 @@
+#ifndef FIRST
+#define FIRST
+
+#include "textual_time.h"
+
+#endif
Index: test/Modules/odr_hash-elaborated-types.cpp
===
--- test/Modules/odr_hash-elaborated-types.cpp
+++ test/Modules/odr_hash-elaborated-types.cpp
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -std=c++1z -I%S/Inputs/odr_hash-elaborated-types -verify %s
+// RUN: %clang_cc1 -std=c++1z -fmodules -fmodules-local-submodule-visibility 
-fmodule-map-file=%S/Inputs/odr_hash-elaborated-types/module.modulemap 
-fmodules-cache-path=%t -x c++ -I%S/Inputs/odr_hash-elaborated-types -verify %s
+
+#include "textual_stat.h"
+
+#include "first.h"
+#include "second.h"
+
+void use() { struct stat value; }
+
+// expected-no-diagnostics
Index: lib/AST/ODRHash.cpp
===
--- lib/AST/ODRHash.cpp
+++ lib/AST/ODRHash.cpp
@@ -749,7 +749,10 @@
 
 void ODRHash::AddType(const Type *T) {
   assert(T && "Expecting non-null pointer.");
-  auto Result = TypeMap.insert(std::make_pair(T, TypeMap.size()));
+  const Type *TypeInTypeMap = T;
+  if (const ElaboratedType *ElTy = dyn_cast(T))
+TypeInTypeMap = ElTy->getNamedType().getTypePtr();
+  auto Result = TypeMap.insert(std::make_pair(TypeInTypeMap, TypeMap.size()));
   ID.AddInteger(Result.first->second);
   // On first encounter of a Type pointer, process it.  Every time afterwards,
   // only the index value is needed.


Index: test/Modules/Inputs/odr_hash-elaborated-types/textual_time.h
===
--- test/Modules/Inputs/odr_hash-elaborated-types/textual_time.h
+++ test/Modules/Inputs/odr_hash-elaborated-types/textual_time.h
@@ -0,0 +1,6 @@
+#ifndef _TIME_H
+#define _TIME_H
+
+struct timespec { };
+
+#endif
Index: test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h
===
--- test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h
+++ test/Modules/Inputs/odr_hash-elaborated-types/textual_stat.h
@@ -0,0 +1,11 @@
+#ifndef _SYS_STAT_H
+#define _SYS_STAT_H
+
+#include "textual_time.h"
+
+struct stat {
+  struct timespec st_atim;
+  struct timespec st_mtim;
+};
+
+#endif