This is an archive of the discontinued LLVM Phabricator instance.

[clang][ItaniumMangle] Check SizeExpr for DependentSizedArrayType (PR49478)
ClosedPublic

Authored by oToToT on Mar 26 2021, 3:31 AM.

Details

Summary

As ArrayType::ArrayType mentioned in clang/lib/AST/Type.cpp, a DependentSizedArrayType might not have size expression because it it used as the type of a dependent array of unknown bound with a dependent braced initializer.

Thus, I add a check when mangling array of that type.

This should fix https://bugs.llvm.org/show_bug.cgi?id=49478

Diff Detail

Event Timeline

oToToT requested review of this revision.Mar 26 2021, 3:31 AM
oToToT created this revision.
oToToT updated this revision to Diff 333526.Mar 26 2021, 3:58 AM

Remove redundant whitespace in comment.

yaxunl added inline comments.Mar 26 2021, 7:41 AM
clang/test/AST/ast-dump-array-json.cpp
7

I am not sure if this test tests the code you change since the mangled variable name does not encode type.

oToToT added inline comments.Mar 26 2021, 11:52 AM
clang/test/AST/ast-dump-array-json.cpp
7

I think this test my code. Or, at least, this test will trigger a runtime error in the latest clang without this patch when mangling A::Test.

Also, maybe I could try construct a test to mangle variable with element type if needed.

WDYT?

rsmith added inline comments.Mar 26 2021, 2:28 PM
clang/test/AST/ast-dump-array-json.cpp
7

I would like to see a test that verifies the A<type>_ mangling is produced.

oToToT added inline comments.Mar 27 2021, 3:46 AM
clang/test/AST/ast-dump-array-json.cpp
7

Ah, you're right! After some investigation, I found that this CHECK in the test doesn't really check my patch.
This test only ensure that clang won't get runtime error when handling this file.

However, I can't find any clever way to construct a test like this to ensure the output of CXXNameMangler::mangleType(const DependentSizedArrayType *). (It is called here only for checking ABI tags, so I guess no output will be displayed).

I will try replace this test with a unittest to check the output.

oToToT updated this revision to Diff 334840.Apr 1 2021, 3:49 PM

Updated tests to check mangled type name.

However, I not sure whether it is proper to put the tests here.

rsmith accepted this revision.Apr 1 2021, 4:04 PM
This revision is now accepted and ready to land.Apr 1 2021, 4:04 PM
oToToT updated this revision to Diff 334848.Apr 1 2021, 4:18 PM

Remove redundant trailing spaces.

Sorry for not checking this at the first time I submit.