aaron.ballman 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();
----------------
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.
================
Comment at: clang/lib/AST/MicrosoftMangle.cpp:2480-2481
#include "clang/Basic/RISCVVTypes.def"
+#define WASM_TYPE(Name, Id, SingletonId) case BuiltinType::Id:
+#include "clang/Basic/WebAssemblyReferenceTypes.def"
case BuiltinType::ShortAccum:
----------------
pmatos wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > Is it reasonable that this simply cannot mangle in Microsoft ABI mode?
> > Still wondering about this
> @aaron.ballman Why wouldn't externref_t be able to mangle in Microsoft ABI
> mode? Quite clueless when it comes to ABIs.
Including the wasm types here means that in Microsoft mode, use of those types
in a context which requires mangling will diagnose (see this snippet just
below):
```
unsigned DiagID = Diags.getCustomDiagID(
DiagnosticsEngine::Error, "cannot mangle this built-in %0 type yet");
Diags.Report(Range.getBegin(), DiagID)
<< T->getName(Context.getASTContext().getPrintingPolicy()) << Range;
```
So I'm wondering whether you consider that to be reasonable behavior or not. If
you don't want the diagnostic, then you should add a vendor extension mangling
for the 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
