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