[PATCH] D38320: [clang] Fix serializers for `TypeTemplateParmDecl` + related types

elsteveogrande abandoned this revision. elsteveogrande added a comment. Dropped in favor of other fix as mentioned above Repository: rC Clang https://reviews.llvm.org/D38320 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D42043: c-index: CXString: fix MSAN read-past-end bug

elsteveogrande updated this revision to Diff 161370. elsteveogrande added a comment. Rebase + fix conflicts for very old diff. Works again. `ninja check-clang` with MSAN-enabled build: Before: Failing Tests (2): Clang :: CodeGen/signed_metadata.cpp Clang :: Index/comment-to-html-

[PATCH] D42043: c-index: CXString: fix MSAN read-past-end bug

elsteveogrande added a comment. hi @vsk + @arphaman , it's been a while -- I brought this old diff up to date, wondering if you could take a look again. Thanks! Repository: rC Clang https://reviews.llvm.org/D42043 ___ cfe-commits mailing list c

[PATCH] D50942: SemaExceptionSpec: ensure old/new specs are both parsed and evaluated when comparing

elsteveogrande created this revision. elsteveogrande added a reviewer: rsmith. Herald added a subscriber: cfe-commits. Before checking that the overriding definition of a method is not more lax than the overridden one, ensure the exception spec is parsed, and also evaluated. Test case by

[PATCH] D50943: Decl: allow "setInvalidDecl" for TagDecls

elsteveogrande created this revision. elsteveogrande added a reviewer: rsmith. Herald added a subscriber: cfe-commits. For `TagDecl`s, complete or incomplete, allow `setInvalidDecl` calls. Other `assert`s are interfering but we'll deal with them separately. https://bugs.llvm.org/show_bug.cgi?id=

[PATCH] D50942: SemaExceptionSpec: ensure old/new specs are both parsed and evaluated when comparing

elsteveogrande marked an inline comment as done. elsteveogrande added inline comments. Comment at: lib/Sema/SemaExceptionSpec.cpp:915 + // lexically-surrounding class. + switch (New->getType()->castAs()->getExceptionSpecType()) { + case EST_Unparsed: vitaut w

[PATCH] D50942: SemaExceptionSpec: ensure old/new specs are both parsed and evaluated when comparing

elsteveogrande updated this revision to Diff 161399. elsteveogrande marked an inline comment as done. elsteveogrande added a comment. fix dopey copy-paste error. Tested again with `ninja check-clang-modules` Repository: rC Clang https://reviews.llvm.org/D50942 Files: lib/Sema/SemaExceptio

[PATCH] D50947: make PendingFakeDefinitionData a set, not map of decl data -> enum

elsteveogrande created this revision. Herald added a subscriber: cfe-commits. Of the three enums, only two are used (Fake and FakeLoaded), and even when switching the mapped value to FakeLoaded, it's leading to an unexpected state anyway (i.e. there's no check for only Fake's in the map). Disca

[PATCH] D50948: Change how LambdaDefinitionData is stored

elsteveogrande created this revision. Herald added a subscriber: cfe-commits. Previously `LambdaDefinitionData` extended `DefinitionData` and either might have been used interchangably in a `CXXRecordDecl`'s `DefinitionData` field. However there are cases where `LambdaDefinitionData` ("LDD") are

[PATCH] D50949: Handle lambdas in .pcm [de]serialization

elsteveogrande created this revision. elsteveogrande added a reviewer: rsmith. Herald added a subscriber: cfe-commits. - Drop a couple of asserts preventing this (in the merge function) - Merge all fields as usual - Ensure select fields match, for ODR checking - add previously-failing test to merg

[PATCH] D50942: SemaExceptionSpec: ensure old/new specs are both parsed and evaluated when comparing

elsteveogrande planned changes to this revision. elsteveogrande marked an inline comment as done. elsteveogrande added a comment. This broke some tests. It fixes the test case added here under `test/Modules` but causes errors in `test/CodeGenCXX` and other dirs. Repository: rC Clang https:/

[PATCH] D38320: [clang] Fix serializers for `TypeTemplateParmDecl` + related types

elsteveogrande added a comment. Hi @bruno, @rsmith, does this approach look ok? I couldn't easily figure out a better way to store inherited-default-info; doing it alongside the default value seemed the cleanest. Note: I saw there are three ways to store these data inside the `DefaultArgStorag

[PATCH] D51580: (WIP) fix spurious exception spec error, PR38627

elsteveogrande created this revision. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D51580 Files: test/Modules/Inputs/lax-base-except/a.h test/Modules/Inputs/lax-base-except/module.modulemap test/Modules/lax-base-except.cpp Index: test/Modules/la

[PATCH] D51608: [modules] when deserializing method, ensure it has correct exception spec

elsteveogrande created this revision. elsteveogrande added reviewers: rsmith, dblaikie. Herald added a subscriber: cfe-commits. (PR38627) After deserializing, the `PendingExceptionSpecUpdates` table is iterated over to apply exception specs to functions. There may be `EST_None`-type in this ta

[PATCH] D50942: SemaExceptionSpec: ensure old/new specs are both parsed and evaluated when comparing

elsteveogrande abandoned this revision. elsteveogrande added a comment. Abandoning in favor of https://reviews.llvm.org/D51608 which works better. Repository: rC Clang https://reviews.llvm.org/D50942 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D51608: [modules] when deserializing method, ensure it has correct exception spec

elsteveogrande added a comment. Hi @rsmith ! Thanks for taking a look at this. I'd much prefer to fix the underlying problem and not swat at these symptoms, so thanks for the analysis of what's actually going on here. :) Let me take a stab at fixing the real problem in the manner you descri

[PATCH] D50947: lambdas in modules: make PendingFakeDefinitionData a set

elsteveogrande added inline comments. Comment at: lib/Serialization/ASTReaderDecl.cpp:1766 + +// FIXME: handle serialized lambdas assert(!DD.IsLambda && !MergeDD.IsLambda && "faked up lambda definition?"); rsmith wrote: > Did you mean to add this FIXME?

[PATCH] D50947: lambdas in modules: make PendingFakeDefinitionData a set

elsteveogrande abandoned this revision. elsteveogrande added inline comments. Comment at: lib/Serialization/ASTReaderDecl.cpp:1760-1765 - if (PFDI != Reader.PendingFakeDefinitionData.end() && - PFDI->second == ASTReader::PendingFakeDefinitionKind::Fake) { + if (PFDI != Rea

[PATCH] D50947: lambdas in modules: make PendingFakeDefinitionData a set

elsteveogrande abandoned this revision. elsteveogrande added a comment. I'll drop this one, since this is not the logic change we want. Repository: rC Clang https://reviews.llvm.org/D50947 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D51608: [modules] when deserializing method, ensure it has correct exception spec

elsteveogrande abandoned this revision. elsteveogrande added a comment. that's great news, thanks! Repository: rC Clang https://reviews.llvm.org/D51608 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D38320: [clang] Fix serializers for `TypeTemplateParmDecl` + related types

elsteveogrande updated this revision to Diff 157682. elsteveogrande added a comment. Herald added a subscriber: chrib. Rebase very old diff Repository: rC Clang https://reviews.llvm.org/D38320 Files: lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriterDecl.cpp test/PCH/cxx-t

[PATCH] D38320: [clang] Fix serializers for `TypeTemplateParmDecl` + related types

elsteveogrande added a comment. cc @rsmith as this has to do with modules. Verified that (1) existing tests pass on trunk; (2) the new test included here fails when applied to trunk; (3) all tests (including new test) pass when this patch is applied. Repository: rC Clang https://reviews.ll

[PATCH] D38320: [clang] Fix serializers for `TypeTemplateParmDecl` + related types

elsteveogrande added a comment. hi @rsmith have a look and let me know if this change looks sensible :) Please make sure I have the right mental model for inheritance in the module case Repository: rC Clang https://reviews.llvm.org/D38320 ___ c

[PATCH] D38320: [clang] Fix serializers for `TypeTemplateParmDecl` + related types

elsteveogrande added inline comments. Comment at: lib/Serialization/ASTReaderDecl.cpp:3448-3456 if (!inheritDefaultTemplateArgument(Context, FTTP, ToParam)) break; } else if (auto *FNTTP = dyn_cast(FromParam)) { if (!inheritDefaultTemplateArgument(Cont

[PATCH] D38320: [clang] Fix serializers for `TypeTemplateParmDecl` + related types

elsteveogrande added a comment. Hi @arphaman, thanks very much! Sorry, I didn't notice there was activity on this old patch until now. I'll make these changes as suggested and re-send it with full context. Repository: rL LLVM https://reviews.llvm.org/D38320 _

[PATCH] D38320: [clang] Fix serializers for `TypeTemplateParmDecl` + related types

elsteveogrande updated this revision to Diff 128466. elsteveogrande edited the summary of this revision. elsteveogrande added a comment. Fixed local var names. Using `arc diff` which hopefully submits latest changes w/ full context Repository: rC Clang https://reviews.llvm.org/D38320 Files

[PATCH] D38320: [clang] Fix serializers for `TypeTemplateParmDecl` + related types

elsteveogrande updated this revision to Diff 128593. elsteveogrande added a comment. Update: now works properly with modules as well. This needed to handle the case where a `ParmDecl` is already inheriting, but it's innocuously inheriting from the same decl. Now `inheritDefaultTemplateArgument

[PATCH] D50948: lambdas in modules: change storage for LambdaDefinitionData

elsteveogrande updated this revision to Diff 165947. elsteveogrande added a comment. Rebase past dependency commit C341499, fix a conflict Repository: rC Clang https://reviews.llvm.org/D50948 Files: include/clang/AST/DeclCXX.h lib/AST/DeclCXX.cpp lib/Serialization/ASTReaderDecl.cpp l

[PATCH] D50949: lambdas in modules: handle lambdas in .pcm [de]serialization

elsteveogrande updated this revision to Diff 165948. elsteveogrande added a comment. Rebase atop updated dependency https://reviews.llvm.org/D50948. Small cleanups to this diff. Repository: rC Clang https://reviews.llvm.org/D50949 Files: include/clang/AST/DeclCXX.h include/clang/AST/La

[PATCH] D50948: lambdas in modules: change storage for LambdaDefinitionData

elsteveogrande added inline comments. Comment at: lib/Serialization/ASTReaderDecl.cpp:1771 auto *Def = DD.Definition; DD = std::move(MergeDD); DD.Definition = Def; Hi @rsmith, thanks again for looking! This is the part that I was concerned about:

[PATCH] D50949: lambdas in modules: handle lambdas in .pcm [de]serialization

elsteveogrande updated this revision to Diff 166130. elsteveogrande added a comment. rebase atop https://reviews.llvm.org/D50948 (correctly this time) to exclude those changes Repository: rC Clang https://reviews.llvm.org/D50949 Files: include/clang/AST/LambdaCapture.h lib/Serialization

[PATCH] D50948: lambdas in modules: change storage for LambdaDefinitionData

elsteveogrande updated this revision to Diff 166148. elsteveogrande added a comment. Change `Optional` to `unique_ptr` instead, to at least save an allocation and keep the same semantics. Reverified with `ninja check-clang`. Repository: rC Clang https://reviews.llvm.org/D50948 Files: inc

[PATCH] D50948: lambdas in modules: change storage for LambdaDefinitionData

elsteveogrande added a comment. I did try a copy of https://reviews.llvm.org/D50949 again, locally and without this diff, to see if I can avoid messing with this structure altogether. But then I get a `SEGV`. The reason behind this diff is to try to make the logic a little safer, though it's

[PATCH] D52956: Support `-fno-visibility-inlines-hidden`

elsteveogrande accepted this revision. elsteveogrande added a comment. This revision is now accepted and ready to land. lgtm! Repository: rC Clang https://reviews.llvm.org/D52956 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://list

[PATCH] D42043: c-index: CXString: fix MSAN read-past-end bug

elsteveogrande created this revision. Herald added a subscriber: cfe-commits. Previous impl would read the byte past the end of a string (a `llvm::StringRef`), possibly exceeding the allocation for that memory and raising an MSAN issue. This will instead save the character range specified by th

[PATCH] D42043: c-index: CXString: fix MSAN read-past-end bug

elsteveogrande updated this revision to Diff 129959. elsteveogrande added a comment. Change string-copy-on-demand logic; do only if `not IsNullTerminated`. Repository: rC Clang https://reviews.llvm.org/D42043 Files: include/clang-c/CXString.h tools/c-index-test/c-index-test.c tools/lib

[PATCH] D42043: c-index: CXString: fix MSAN read-past-end bug

elsteveogrande updated this revision to Diff 130225. elsteveogrande added a comment. Add a needed null-check on input string's data ptr. Repository: rC Clang https://reviews.llvm.org/D42043 Files: include/clang-c/CXString.h tools/c-index-test/c-index-test.c tools/libclang/CXString.cpp

[PATCH] D42043: c-index: CXString: fix MSAN read-past-end bug

elsteveogrande added a comment. Thanks very much for looking at this @vsk ! I actually found an ASAN bug in my new code, mixing and matching `malloc/free` and `operator`s `new/delete`. Comment at: tools/c-index-test/c-index-test.c:3268 - filename = clang_getFileName(file);

[PATCH] D42259: c-index-test: small fix to CXString handling and disposal

elsteveogrande created this revision. elsteveogrande added reviewers: vsk, benlangmuir, akyrtzi. Herald added a subscriber: cfe-commits. (Separating some unrelated changes out of https://reviews.llvm.org/D42043) Repository: rC Clang https://reviews.llvm.org/D42259 Files: tools/c-index-test

[PATCH] D42043: c-index: CXString: fix MSAN read-past-end bug

elsteveogrande marked 2 inline comments as done. elsteveogrande added inline comments. Comment at: tools/c-index-test/c-index-test.c:3268 - filename = clang_getFileName(file); - index_data->main_filename = clang_getCString(filename); - clang_disposeString(filename); + index

[PATCH] D42043: c-index: CXString: fix MSAN read-past-end bug

elsteveogrande updated this revision to Diff 130556. elsteveogrande marked an inline comment as done. elsteveogrande added a comment. Fixes, but first, a question for reviewers: Looking at the description of `clang_disposeString`: /** * \brief Free the given string. */ CINDEX_LINKAGE v

[PATCH] D42043: c-index: CXString: fix MSAN read-past-end bug

elsteveogrande updated this revision to Diff 130557. elsteveogrande added a comment. remove unneeded changes Repository: rC Clang https://reviews.llvm.org/D42043 Files: include/clang-c/CXString.h tools/c-index-test/c-index-test.c tools/libclang/CXString.cpp Index: tools/libclang/CXStr

[PATCH] D42043: c-index: CXString: fix MSAN read-past-end bug

elsteveogrande added a comment. Hi @vsk + @arphaman : I did find such problems with either MSAN (with poison-in-dtor) and ASAN, so we should be able to discover these instances pretty easily :) Repository: rC Clang https://reviews.llvm.org/D42043 _