[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

Zahira Ammarguellat via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon May 20 05:55:57 PDT 2019


zahiraam marked 12 inline comments as done.
zahiraam added a comment.

@rsmith I think I have made all the changes you have pointed out to. But please note that this new patch only impements an explicit AST representation of uuid  in template arguments. It **does not** fix the bug for which this review was opened for. 
I will take care of the bug in time. But before doing that I want to make sure that the changes required to give an explicit AST for uuid is correct.

Thanks for taking the time to review. And sorry my response is slow to come. This work is done in my "spare" time.



================
Comment at: lib/Sema/SemaExprCXX.cpp:675-678
+  if (expr.isUnset()) {
+    uuid_expr =
+        new (Context) CXXUuidofExpr(TypeInfoType.withConst(), Operand, UuidStr,
+                                    SourceRange(TypeidLoc, RParenLoc));
----------------
rsmith wrote:
> We should store a pointer to the UUID declaration on a non-dependent `CXXUuidofExpr`.
Not sure what that means.


================
Comment at: lib/Sema/SemaExprCXX.cpp:675-678
+  if (expr.isUnset()) {
+    uuid_expr =
+        new (Context) CXXUuidofExpr(TypeInfoType.withConst(), Operand, UuidStr,
+                                    SourceRange(TypeidLoc, RParenLoc));
----------------
zahiraam wrote:
> rsmith wrote:
> > We should store a pointer to the UUID declaration on a non-dependent `CXXUuidofExpr`.
> Not sure what that means.
However the test case is still failing. Still need to find a solution for the fail.


================
Comment at: lib/Sema/SemaExprCXX.cpp:619
+ /// Finds the __declSpec uuid Decl off a type.
+ static void FindADeclOffAType(Sema &SemaRef,
+                               QualType QT,
----------------
rsmith wrote:
> Do we need both this and getUuidAttrOfType?
No.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D43576/new/

https://reviews.llvm.org/D43576





More information about the cfe-commits mailing list