jgorbe 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: > pmatos wrote: > > 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? > I'd recommend something like: > ``` > assert(Target->getTriple().isWasm() && Target->hasFeature("reference-types") > && "shouldn't try to generate type externref outsid of WebAssembly target"); > #define WASM_REF_TYPE(Name, MangledName, Id, SingletonId, AS) > \ > if (BuiltinType::Id == BuiltinType::WasmExternRef) > \ > return SingletonId; > #include "clang/Basic/WebAssemblyReferenceTypes.def" > } > llvm_unreachable("couldn't find the externref type?"); > ``` > so it's clear that you shouldn't call this outside of a wasm target and that > it's not an option to not find the type. In a non-assert build, doing just `assert(false)` causes the build to fail if you use `-Werror -Wreturn-type`. ``` clang/lib/AST/ASTContext.cpp:4097:1: error: non-void function does not return a value in all control paths [-Werror,-Wreturn-type] ``` 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