martong added a comment. In D83174#2158275 <https://reviews.llvm.org/D83174#2158275>, @aaron.ballman wrote:
> In D83174#2156454 <https://reviews.llvm.org/D83174#2156454>, @v.g.vassilev > wrote: > > > >> Also, I would like to add that the current test present in this diff > > >> does not fail in the existing system but it was the best I and Vassil > > >> could come up with to replicate our problem. > > > > > > Is there a way to run something like creduce to come up with a test case > > > that does replicate the issue? I'd like to ensure we have a test case > > > that fails without the patch applied so that we can be sure we don't > > > regress the behavior. > > > > Not easily. That bug comes from internal infrastructure which uses clang's > > API and happens when calling directly the mangler. Running creduce would > > require failure to be reproducible with bare clang. I was hoping @rsmith > > had something up his sleeve. > > > Drat. While the code looks reasonable, this makes me less confident in the > fix. I'm adding some more reviewers who have touched the AST serialization > and modules before in case they also have ideas for test cases. I had a patch a few months ago for ASTImporter that tries to properly handle inherited attributes (D68634 <https://reviews.llvm.org/D68634>). That patch had not made it to the upstream, because that's not generic enough, it handles only 2 or 3 attributes. Nevertheless, some of the unit tests might be inspiring and we could reuse them here. E.g. in the below code replace "import" with "deserialization": TEST_P(ImportAttrs, ImportShouldInheritExistingInheritableAttr) { Decl *ToTU = getToTuDecl( R"( void f() __attribute__((analyzer_noreturn)); void f(); )", Lang_C); Decl *FromTU = getTuDecl( R"( void f(); )", Lang_C, "input0.c"); auto *From = FirstDeclMatcher<FunctionDecl>().match( FromTU, functionDecl(hasName("f"))); auto *To0 = FirstDeclMatcher<FunctionDecl>().match(ToTU, functionDecl(hasName("f"))); ASSERT_TRUE(To0->hasAttrs()); ASSERT_TRUE(To0->getAttr<AnalyzerNoReturnAttr>()); auto *To1 = LastDeclMatcher<FunctionDecl>().match(ToTU, functionDecl(hasName("f"))); ASSERT_TRUE(To1->hasAttrs()); ASSERT_TRUE(To1->getAttr<AnalyzerNoReturnAttr>()); ASSERT_TRUE(To1->getAttr<AnalyzerNoReturnAttr>()->isInherited()); // Should have an Inherited attribute. auto *Imported = Import(From, Lang_C); EXPECT_TRUE(Imported->hasAttrs()); ASSERT_TRUE(Imported->getAttr<AnalyzerNoReturnAttr>()); EXPECT_TRUE(Imported->getAttr<AnalyzerNoReturnAttr>()->isInherited()); } CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83174/new/ https://reviews.llvm.org/D83174 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
