pmatos marked 5 inline comments as done.
pmatos added inline comments.

================
Comment at: clang/lib/AST/ASTContext.cpp:4012-4018
+  if (Target->getTriple().isWasm() && Target->hasFeature("reference-types")) {
+#define WASM_REF_TYPE(Name, MangledName, Id, SingletonId, AS)                  
\
+  if (BuiltinType::Id == BuiltinType::WasmExternRef)                           
\
+    return SingletonId;
+#include "clang/Basic/WebAssemblyReferenceTypes.def"
+  }
+  return QualType();
----------------
aaron.ballman wrote:
> Rather than returning a null type, should we assert we're in a wasm triple? 
> We shouldn't be trying to form the type outside of a wasm target, right?
asserting false with message rather than llvm_unreachable is the right way, 
correct?


================
Comment at: clang/lib/AST/ItaniumMangle.cpp:3147
+    type_name = MangledName;                                                   
\
+    Out << (type_name == InternalName ? "u" : "") << type_name.size()          
\
+        << type_name;                                                          
\
----------------
aaron.ballman wrote:
> aaron.ballman wrote:
> > tlively wrote:
> > > Our `MangledName` is not the same as our `InternalName`, so it looks like 
> > > this condition will never be true. Should be follow the simpler pattern 
> > > from the previous two targets instead?
> > I'm not an Itanium mangling expert, but doesn't this *have* to use the `u` 
> > marking per 
> > https://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangle.builtin-type 
> > because this is definitely a vendor extension type.
> Still wondering about this, pinging @rjmccall for Itanium ABI expertise
I think what you both are saying makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122215

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

Reply via email to