[PATCH] D57075: [ObjC] For type substitution in generics use a regular recursive type visitor.

vsapsai updated this revision to Diff 183668. vsapsai added a comment. Rebase on top of https://reviews.llvm.org/D57270 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57075/new/ https://reviews.llvm.org/D57075 Files: clang/lib/AST/Type.cpp Index: clang/lib/AST/Type.cpp ==

[PATCH] D56561: [Preprocessor] For missing file in framework add note about framework location.

vsapsai updated this revision to Diff 183678. vsapsai added a comment. - Tweak doxygen comment to express IsFrameworkFound in terms of IsFrameworkFound returned by DirectoryLookup. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56561/new/ https://reviews.llvm.org/D56561 Files: clang/

[PATCH] D56561: [Preprocessor] For missing file in framework add note about framework location.

vsapsai marked 2 inline comments as done. vsapsai added inline comments. Comment at: clang/include/clang/Lex/DirectoryLookup.h:175 + /// \param [out] IsFrameworkFound For a framework directory set to true if + /// specified '.framework' directory is found. + /// --

[PATCH] D57427: [CodeGen][ObjC] Fix assert on calling `__builtin_constant_p` with ObjC objects.

vsapsai created this revision. vsapsai added reviewers: rjmccall, ahatanak, void. Herald added subscribers: dexonsmith, jkorous. When we are calling `__builtin_constant_p` with ObjC objects of different classes, we hit the assertion > Assertion failed: (isa(Val) && "cast() argument of incompatibl

[PATCH] D57427: [CodeGen][ObjC] Fix assert on calling `__builtin_constant_p` with ObjC objects.

vsapsai added a comment. In D57427#1376493 , @void wrote: > LGTM, but may want to wait for other reviewers. Thanks for the prompt review, Bill. I'll wait for other reviewers to confirm the change makes sense for Objective-C. CHANGES SINCE LAST ACTION

[PATCH] D57427: [CodeGen][ObjC] Fix assert on calling `__builtin_constant_p` with ObjC objects.

vsapsai updated this revision to Diff 184334. vsapsai added a comment. - Add in the test `-disable-llvm-passes` and relax FileCheck expectations to accept alloca-store-load. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57427/new/ https://reviews.llvm.org/D57427 Files: clang/lib/Cod

[PATCH] D53725: [CodeGen] Move `emitConstant` from ScalarExprEmitter to CodeGenFunction. NFC.

vsapsai updated this revision to Diff 172046. vsapsai added a comment. - Switch to camelCase. https://reviews.llvm.org/D53725 Files: clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CGExprScalar.cpp clang/lib/CodeGen/CodeGenFunction.h Index: clang/lib/CodeGen/CodeGenFunction.h ===

[PATCH] D53674: [CodeGen] Fix assertion on referencing constexpr Obj-C object with ARC.

vsapsai updated this revision to Diff 172049. vsapsai added a comment. - Rename `EmitConstant` to `emitScalarConstant`. - Tweak comment to be explicitly about intended IR code, not about Obj-C++ code. https://reviews.llvm.org/D53674 Files: clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/CGEx

[PATCH] D53674: [CodeGen] Fix assertion on referencing constexpr Obj-C object with ARC.

vsapsai updated this revision to Diff 172051. vsapsai added a comment. Exclude commits tracked in a different review. https://reviews.llvm.org/D53674 Files: clang/lib/CodeGen/CGObjC.cpp clang/test/CodeGenObjCXX/arc-constexpr.mm Index: clang/test/CodeGenObjCXX/arc-constexpr.mm =

[PATCH] D53725: [CodeGen] Move `emitConstant` from ScalarExprEmitter to CodeGenFunction. NFC.

vsapsai added a comment. Can you please check https://reviews.llvm.org/D53674 ? It builds on top of this change and I plan to commit them together. https://reviews.llvm.org/D53725 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists

[PATCH] D53725: [CodeGen] Move `emitConstant` from ScalarExprEmitter to CodeGenFunction. NFC.

This revision was automatically updated to reflect the committed changes. Closed by commit rL345897: [CodeGen] Move `emitConstant` from ScalarExprEmitter to CodeGenFunction. NFC. (authored by vsapsai, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://revi

[PATCH] D53674: [CodeGen] Fix assertion on referencing constexpr Obj-C object with ARC.

vsapsai added a comment. Thanks for the review, John. I'll update the comment with TODO and commit. https://reviews.llvm.org/D53674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53674: [CodeGen] Fix assertion on referencing constexpr Obj-C object with ARC.

This revision was automatically updated to reflect the committed changes. Closed by commit rL345903: [CodeGen] Fix assertion on referencing constexpr Obj-C object with ARC. (authored by vsapsai, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.ll

[PATCH] D54132: [CodeGenCXX] XFAIL test for ASAN on Darwin.

vsapsai created this revision. vsapsai added reviewers: george.karpenkov, lebedev.ri. Herald added a subscriber: dexonsmith. The test hits stack overflow trying to instantiate recursive templates. It is observed with ASAN and not with a regular build because ASAN increases stack frame size. rdar:

[PATCH] D50050: [AST] CastExpr: BasePathSize is not large enough.

vsapsai added a comment. Disabling test in https://reviews.llvm.org/D54132. Repository: rC Clang https://reviews.llvm.org/D50050 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54132: [CodeGenCXX] XFAIL test for ASAN on Darwin.

vsapsai added inline comments. Comment at: clang/test/CodeGenCXX/castexpr-basepathsize-threshold.cpp:8 +// recursive template instantiation limit. +// XFAIL: darwin && asan + george.karpenkov wrote: > Do we actually want UNSUPPORTED here? We don't want to fail if

[PATCH] D54132: [CodeGenCXX] XFAIL test for ASAN on Darwin.

This revision was automatically updated to reflect the committed changes. Closed by commit rC346200: [CodeGenCXX] XFAIL test for ASAN on Darwin. (authored by vsapsai, committed by ). Changed prior to commit: https://reviews.llvm.org/D54132?vs=172692&id=172699#toc Repository: rC Clang https:

[PATCH] D47687: [Sema] Missing -Wlogical-op-parentheses warnings in macros (PR18971)

vsapsai added a comment. Sorry, you've decided to abandon the patch, it took a lot of good work. Xing, are you sure you don't want to see this change finished? I agree that delays in code review can be frustrating and I think it is something we can improve. https://reviews.llvm.org/D47687 _

[PATCH] D47687: [Sema] Missing -Wlogical-op-parentheses warnings in macros (PR18971)

vsapsai added a comment. OK. Good to know you are still working on it. In https://reviews.llvm.org/D47687#1271880, @Higuoxing wrote: > In https://reviews.llvm.org/D47687#1266893, @vsapsai wrote: > > > Sorry about the delay. The change seems to be correct but `ninja > > check-clang` reveals the

[PATCH] D54245: [VFS] Implement `RedirectingFileSystem::getRealPath`.

vsapsai created this revision. vsapsai added reviewers: bruno, benlangmuir. Herald added subscribers: dexonsmith, hiraditya. It fixes the case when Objective-C framework is added as a subframework through a symlink. When parent framework infers a module map and fails to detect a symlink, it would

[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

vsapsai added inline comments. Comment at: clang/lib/Frontend/DependencyFile.cpp:100 + +DepCollector.maybeAddDependency(HeaderPath, /*FromModule=*/false, IsSystem, +/*IsModuleFile*/ false, This `/*FromModule=*/false` looks

[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

vsapsai added inline comments. Comment at: clang/lib/Lex/ModuleMap.cpp:1198-1203 +// If the header in the module map refers to a symlink, Header.Entry +// refers to the actual file. The callback should be notified of both. +if (!FullPathAsWritten.empty() && +!

[PATCH] D54245: [VFS] Implement `RedirectingFileSystem::getRealPath`.

vsapsai added a comment. In https://reviews.llvm.org/D54245#1292008, @bruno wrote: > Hi Volodymyr. Does this behavior changed after your VFS fallback change? https://reviews.llvm.org/D50539 didn't change `RedirectingFileSystem` behavior regarding real paths. But for `-ivfsoverlay` it started u

[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

vsapsai added inline comments. Comment at: clang/lib/Lex/ModuleMap.cpp:1198-1203 +// If the header in the module map refers to a symlink, Header.Entry +// refers to the actual file. The callback should be notified of both. +if (!FullPathAsWritten.empty() && +!

[PATCH] D54245: [VFS] Implement `RedirectingFileSystem::getRealPath`.

vsapsai added a comment. Ping. https://reviews.llvm.org/D54245 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

vsapsai added a comment. The test case I've promised is touch a.h touch b.h touch c.h ln -s b.h d.h echo '#include "a.h"' > umbrella.h echo '#include "b.h"' >> umbrella.h echo '#include "b.h"' > test_case.c cat < module.modulemap module my_module { umbrella header "

[PATCH] D54245: [VFS] Implement `RedirectingFileSystem::getRealPath`.

This revision was automatically updated to reflect the committed changes. Closed by commit rL347009: [VFS] Implement `RedirectingFileSystem::getRealPath`. (authored by vsapsai, committed by ). Herald added a subscriber: kristina. Changed prior to commit: https://reviews.llvm.org/D54245?vs=17309

[PATCH] D49736: [Basic] Emit warning flag suggestion only in case there's existing flag *similar* to the unknown one

vsapsai added a comment. Have you considered the same approach as typo correction? I.e. for the max allowed edit distance use percentage of the input size. For example, something similar to `TypoCorrectionConsumer::addName`

[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.

vsapsai added a comment. Thanks for the feedback. Answered one simple question, the rest needs more time. Comment at: libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:186 + std::vector v(array, array + 3); + assert(std::fabs(v[0] - 0.0f) <

[PATCH] D55543: [CodeGen] Fix assertion on throwing object with inlined inherited constructor and non-trivial destructor.

vsapsai created this revision. vsapsai added reviewers: rsmith, rjmccall. Herald added subscribers: dexonsmith, jkorous. Fixes assertion > Assertion failed: (isa(Val) && "cast() argument of incompatible > type!"), function cast, file llvm/Support/Casting.h, line 255. It was triggered by trying

[PATCH] D58659: [Sema] Fix assertion when `auto` parameter in lambda has an attribute.

vsapsai created this revision. vsapsai added reviewers: rsmith, arphaman. Herald added subscribers: jdoerfert, dexonsmith, jkorous. Fixes the assertion > no Attr* for AttributedType* > UNREACHABLE executed at llvm-project/clang/lib/Sema/SemaType.cpp:298! In `TypeProcessingState::getAttributedTy

[PATCH] D58659: [Sema] Fix assertion when `auto` parameter in lambda has an attribute.

vsapsai added a comment. Have doubts that checking only the outermost type for being `AttributedType` after `Sema::ReplaceAutoType` is sufficient but haven't found a counterexample yet. Decided to post the proposed fix to see if others have any ideas. CHANGES SINCE LAST ACTION https://review

[PATCH] D57628: [index] Improve indexing support for MSPropertyDecl.

vsapsai added a comment. Herald added a subscriber: jdoerfert. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57628/new/ https://reviews.llvm.org/D57628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[PATCH] D58147: [CodeGen] Fix calling llvm.var.annotation outside of a basic block.

vsapsai added a comment. In D58147#1398094 , @rjmccall wrote: > Okay. Is it acceptable for the annotation to simply disappear in this case? > I don't really understand the purposes of annotations well enough to judge. Looks like it is acceptable. The

[PATCH] D57628: [index] Improve indexing support for MSPropertyDecl.

This revision was automatically updated to reflect the committed changes. Closed by commit rL354942: [index] Improve indexing support for MSPropertyDecl. (authored by vsapsai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https:/

[PATCH] D57628: [index] Improve indexing support for MSPropertyDecl.

vsapsai added a comment. Thanks for reviewing the change! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57628/new/ https://reviews.llvm.org/D57628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailma

[PATCH] D58147: [CodeGen] Fix calling llvm.var.annotation outside of a basic block.

vsapsai added a comment. Thanks for the review. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58147/new/ https://reviews.llvm.org/D58147 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo

[PATCH] D58147: [CodeGen] Fix calling llvm.var.annotation outside of a basic block.

This revision was automatically updated to reflect the committed changes. Closed by commit rL355166: [CodeGen] Fix calling llvm.var.annotation outside of a basic block. (authored by vsapsai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to co

[PATCH] D58094: Fix -Wnonportable-include-path suppression for header maps with absolute paths.

vsapsai added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58094/new/ https://reviews.llvm.org/D58094 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58659: [Sema] Fix assertion when `auto` parameter in lambda has an attribute.

vsapsai added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:266 +auto *NewAttrTy = cast(T.getTypePtr()); +for (TypeAttrPair &A : AttrsForTypes) { + if (A.first == AttrTy) jkorous wrote: > Do you think it would make sense to ter

[PATCH] D58659: [Sema] Fix assertion when `auto` parameter in lambda has an attribute.

vsapsai added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:261 +/// necessary. +QualType ReplaceAutoType(QualType TypeWithAuto, QualType Replacement) { + QualType T = sema.ReplaceAutoType(TypeWithAuto, Replacement); aaron.ballman wrote: >

[PATCH] D58659: [Sema] Fix assertion when `auto` parameter in lambda has an attribute.

vsapsai added a comment. Thanks for the review, Aaron. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58659/new/ https://reviews.llvm.org/D58659 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/l

[PATCH] D58659: [Sema] Fix assertion when `auto` parameter in lambda has an attribute.

This revision was automatically updated to reflect the committed changes. Closed by commit rL357298: [Sema] Fix assertion when `auto` parameter in lambda has an attribute. (authored by vsapsai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D58094: Fix -Wnonportable-include-path suppression for header maps with absolute paths.

vsapsai added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58094/new/ https://reviews.llvm.org/D58094 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58094: Fix -Wnonportable-include-path suppression for header maps with absolute paths.

vsapsai added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58094/new/ https://reviews.llvm.org/D58094 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60848: [Parser] Avoid correcting delayed typos in array subscript multiple times.

vsapsai created this revision. vsapsai added reviewers: rsmith, erik.pilkington, majnemer. Herald added subscribers: dexonsmith, jkorous. We correct some typos in `ActOnArraySubscriptExpr` and `ActOnOMPArraySectionExpr`, so when their result is `ExprError`, we can end up correcting delayed typos i

[PATCH] D60848: [Parser] Avoid correcting delayed typos in array subscript multiple times.

vsapsai added a comment. In D60848#1472799 , @erik.pilkington wrote: > Wait, why is NumTypos incorrect here? I think its because we don't handle the > typo on the line: `[self undeclaredMethod:undeclaredArg];`, even the > following asserts now. Seems li

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

vsapsai added a comment. I see you've ended up implementing the intrinsics with inline assembly but are there any reasons not to do it with builtins like in this patch? The problem with inline assembly is that for some Apple platforms we require developers to emit bitcode. And inline assembly i

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

vsapsai added a comment. Thanks for the insightful explanation, Craig. As far as I understand, implementing intrinsics with builtins is possible but it is more complex and wasn't providing enough value to prefer it over inline assembly. If that is correct, I'd like to revive the abandoned imple

[PATCH] D60848: [Parser] Avoid correcting delayed typos in array subscript multiple times.

vsapsai updated this revision to Diff 197421. vsapsai added a comment. - Add a test case with `-disable-free`. Thanks for the suggestion, Erik. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60848/new/ https://reviews.llvm.org/D60848 Files: clang/lib/Parse/ParseExpr.cpp clang/test/

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

vsapsai added a comment. Having in bitcode something like `@llvm.x86.encls.64` is better than inline assembly because we understand the meaning of the bitcode while we don't parse assembly and have a very limited understanding of what it is doing. The use case we need to support is basically

[PATCH] D60848: [Parser] Avoid correcting delayed typos in array subscript multiple times.

This revision was automatically updated to reflect the committed changes. Closed by commit rL359713: [Parser] Avoid correcting delayed typos in array subscript multiple times. (authored by vsapsai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prio

[PATCH] D61619: Make language option `GNUAsm` discoverable with `__has_extension` macro.

vsapsai created this revision. vsapsai added reviewers: aaron.ballman, rsmith. Herald added subscribers: dexonsmith, jkorous, eraman. This can be used for better support of `-fno-gnu-inline-asm` builds. rdar://problem/49540880 https://reviews.llvm.org/D61619 Files: clang/include/clang/Basic/

[PATCH] D61621: [X86] Make `x86intrin.h`, `immintrin.h` includable with `-fno-gnu-inline-asm`.

vsapsai created this revision. vsapsai added reviewers: craig.topper, GBuella. Herald added subscribers: dexonsmith, jkorous. vsapsai added a parent revision: D61619: Make language option `GNUAsm` discoverable with `__has_extension` macro.. Currently `immintrin.h` includes `pconfigintrin.h` and `

[PATCH] D44387: [x86] Introduce the pconfig/encl[u|s|v] intrinsics

vsapsai added a comment. Discussed with the team different approaches and suggesting https://reviews.llvm.org/D61621 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D44387/new/ https://reviews.llvm.org/D44387 ___ cfe-commits mailing list cfe-

[PATCH] D61707: [Preprocessor] Fix crash emitting note with framework location for "file not found" error.

vsapsai created this revision. vsapsai added reviewers: arphaman, erik.pilkington, jkorous. Herald added a subscriber: dexonsmith. A filename can be remapped with a header map to point to a framework header and we can find the corresponding framework without the header. But if the original filenam

[PATCH] D61621: [X86] Make `x86intrin.h`, `immintrin.h` includable with `-fno-gnu-inline-asm`.

vsapsai updated this revision to Diff 198875. vsapsai added a comment. - Improve `-fno-gnu-inline-asm` support for MS-compatibility too. Didn't use `-fms-compatibility` in the test and it seems to be working fine. Don't know if it is accidental and if I should add the flag. CHANGES SINCE LAST A

[PATCH] D30882: Add a callback for __has_include and use it for dependency scanning

vsapsai added inline comments. Comment at: clang/lib/Frontend/DependencyFile.cpp:300-304 StringRef Filename = FE->getName(); if (!FileMatchesDepCriteria(Filename.data(), FileType)) return; AddFilename(llvm::sys::path::remove_leading_dotslash(Filename)); --

[PATCH] D30882: Add a callback for __has_include and use it for dependency scanning

vsapsai added a comment. @dexonsmith, does my change address your concerns? https://reviews.llvm.org/D30882 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

vsapsai added a comment. Seems like this change causes 2 test failures: Clang :: Driver/clang_f_opts.c Clang :: Frontend/gnu-mcount.c Some of the failing bots are http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/15363/ http://green.lab.llvm.org/green/job/clang-stage1-cmake

[PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

vsapsai added a comment. Confirm that reverting the change locally fixes the tests. If nobody beats me to it, I plan to revert the change in 30-60 minutes. @srhines, if you want to fix it in another way and need more time, please let me know. Repository: rL LLVM https://reviews.llvm.org/D52

[PATCH] D52191: Fix logic around determining use of frame pointer with -pg.

vsapsai added a comment. Thanks for the fix. Repository: rL LLVM https://reviews.llvm.org/D52191 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30882: Add a callback for __has_include and use it for dependency scanning

This revision was automatically updated to reflect the committed changes. Closed by commit rL342517: Add a callback for `__has_include` and use it for dependency scanning. (authored by vsapsai, committed by ). Herald added subscribers: llvm-commits, jsji. Changed prior to commit: https://review

[PATCH] D44263: Implement LWG 2221 - No formatted output operator for nullptr

vsapsai added a comment. Seems like revert didn't go through. I suspect it is causing build failure in http://green.lab.llvm.org/green/job/clang-stage1-configure-RA/49593/consoleText SANITIZER_TEST_OBJECTS.sanitizer_vector_test.cc.i386.o SANITIZER_TEST_OBJECTS.gtest-all.cc.i386.o /Users/buil

[PATCH] D44263: Implement LWG 2221 - No formatted output operator for nullptr

vsapsai added a comment. Reverted in r342599 . https://reviews.llvm.org/D44263 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50539: [VFS] Add property 'fallthrough' that controls fallback to real file system.

vsapsai added a comment. Ping. https://reviews.llvm.org/D50539 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51867: [Diagnostics] Add error handling to FormatDiagnostic()

vsapsai added a comment. It seems like there are too many asserts and they are too specific, they seem to be aimed at specific potential bugs. What about asserts that make sure we maintain some invariants? For example, check `DiagStr < DiagEnd` once in a loop instead of every place we increment

[PATCH] D57628: [index] Improve indexing support for MSPropertyDecl.

vsapsai created this revision. vsapsai added reviewers: akyrtzi, benlangmuir. Herald added subscribers: arphaman, dexonsmith, jkorous. Currently the symbol for MSPropertyDecl has kind `SymbolKind::Unknown` which can trip up various indexing tools. rdar://problem/46764224 https://reviews.llvm.or

[PATCH] D56561: [Preprocessor] For missing file in framework add note about framework location.

vsapsai updated this revision to Diff 184866. vsapsai added a comment. - Improve the diagnostic message wording. Rebased the patch, so the diff between 2 revisions can be noisy. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56561/new/ https://reviews.llvm.org/D56561 Files: clang/inc

[PATCH] D56561: [Preprocessor] For missing file in framework add note about framework location.

This revision was automatically updated to reflect the committed changes. Closed by commit rC353231: [Preprocessor] Add a note with framework location for "file not found" error. (authored by vsapsai, committed by ). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.o

[PATCH] D57270: [ObjC] Fix non-canonical types preventing type arguments substitution.

vsapsai added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57270/new/ https://reviews.llvm.org/D57270 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57628: [index] Improve indexing support for MSPropertyDecl.

vsapsai added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57628/new/ https://reviews.llvm.org/D57628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57427: [CodeGen][ObjC] Fix assert on calling `__builtin_constant_p` with ObjC objects.

vsapsai added a comment. @ahatanak, can you please take a look again? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57427/new/ https://reviews.llvm.org/D57427 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D57427: [CodeGen][ObjC] Fix assert on calling `__builtin_constant_p` with ObjC objects.

vsapsai added a comment. In D57427#1386781 , @hans wrote: > What's the status of this patch? Is it something we should merge onto the 8.0 > release branch? Waiting on a one more review iteration from @ahatanak and hope that afterwards the patch should

[PATCH] D57427: [CodeGen][ObjC] Fix assert on calling `__builtin_constant_p` with ObjC objects.

This revision was automatically updated to reflect the committed changes. Closed by commit rL353577: [CodeGen][ObjC] Fix assert on calling `__builtin_constant_p` with ObjC objects. (authored by vsapsai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed

[PATCH] D57991: [Driver][Darwin] Emit an error when using -pg on OS without support for it.

vsapsai created this revision. vsapsai added reviewers: dexonsmith, bob.wilson, steven_wu. Herald added a subscriber: jkorous. Instead of letting a program fail at runtime, emit an error during compilation. rdar://problem/12206955 https://reviews.llvm.org/D57991 Files: clang/include/clang/Ba

[PATCH] D58094: Fix -Wnonportable-include-path suppression for header maps with absolute paths.

vsapsai created this revision. vsapsai added reviewers: dexonsmith, bruno. Herald added a subscriber: jkorous. In `DirectoryLookup::LookupFile` parameter `HasBeenMapped` doesn't cover the case when clang finds a file through a header map but doesn't remap the lookup filename because the target pat

[PATCH] D58094: Fix -Wnonportable-include-path suppression for header maps with absolute paths.

vsapsai marked an inline comment as done. vsapsai added inline comments. Comment at: clang/lib/Lex/HeaderSearch.cpp:388 } else { Result = HS.getFileMgr().getFile(Dest); } I have considered changing this to `.getFile(Dest, /*OpenFile=*/true)` so that t

[PATCH] D57991: [Driver][Darwin] Emit an error when using -pg on OS without support for it.

vsapsai updated this revision to Diff 186522. vsapsai added a comment. Herald added a subscriber: jdoerfert. - Use `%select` per Steven's recommendation. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57991/new/ https://reviews.llvm.org/D57991 Files: clang/include/clang/Basic/Diagnost

[PATCH] D58147: [CodeGen] Fix calling llvm.var.annotation outside of a basic block.

vsapsai created this revision. vsapsai added a reviewer: rjmccall. Herald added subscribers: dexonsmith, jkorous, aprantl. When we have an annotated local variable after a function returns, we generate IR that fails verification with the error > Instruction referencing instruction not embedded in

[PATCH] D58147: [CodeGen] Fix calling llvm.var.annotation outside of a basic block.

vsapsai added a comment. I've checked and seems like other places where we `EmitAnnotationCall` should be safe. We do that for globals, function parameters, fields (`FieldDecl`) and early return isn't applicable in those cases. We can do that for any expressions with `__builtin_annotation` but

[PATCH] D57991: [Driver][Darwin] Emit an error when using -pg on OS without support for it.

This revision was automatically updated to reflect the committed changes. Closed by commit rL354084: [Driver][Darwin] Emit an error when using -pg on OS without support for it. (authored by vsapsai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed pri

[PATCH] D57270: [ObjC] Fix non-canonical types preventing type arguments substitution.

This revision was automatically updated to reflect the committed changes. Closed by commit rL354164: [ObjC] Fix non-canonical types preventing type arguments substitution. (authored by vsapsai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D57075: [ObjC] For type substitution in generics use a regular recursive type visitor.

This revision was automatically updated to reflect the committed changes. Closed by commit rL354180: [ObjC] For type substitution in generics use a regular recursive type visitor. (authored by vsapsai, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed

[PATCH] D57076: [ObjC generics] Fix applying `__kindof` to the type parameter.

This revision was automatically updated to reflect the committed changes. vsapsai marked an inline comment as done. Closed by commit rC354189: [ObjC generics] Fix applying `__kindof` to the type parameter. (authored by vsapsai, committed by ). Herald added a subscriber: jdoerfert. Herald added a p

[PATCH] D57075: [ObjC] For type substitution in generics use a regular recursive type visitor.

vsapsai added a comment. Thanks for the review, Erik. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57075/new/ https://reviews.llvm.org/D57075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm

[PATCH] D37089: [Sema] Error out early for tags defined inside an enumeration.

vsapsai added a comment. It occurred to me that another approach for fixing the crash can be setting `IsTypeSpecifier` to `true` while we ParseCastExpression. According to cast-expression: unary-expression ( type-id ) cast-expression type-id: type-specifier-seq abstract-declarato

[PATCH] D37322: [Sema] Correct typos in LHS, RHS before building a binop expression.

vsapsai created this revision. Specifically, typo correction should be done before dispatching between different kinds of binary operations like pseudo-object assignment, overloaded binary operation, etc. Without this change we hit an assertion Assertion failed: (!LHSExpr->hasPlaceholderType(B

[PATCH] D37322: [Sema] Correct typos in LHS, RHS before building a binop expression.

vsapsai added a comment. In https://reviews.llvm.org/D37322#859065, @ahatanak wrote: > Is it possible to avoid creating CXXDependentScopeMemberExpr in the first > place? It seems to me that we shouldn't be creating a > CXXDependentScopeMemberExpr for an ObjC property access. > > Or perhaps ther

[PATCH] D37089: [Sema] Error out early for tags defined inside an enumeration.

vsapsai added a comment. Ping. If you don't have time to review the change in depth, please say if the approach is wrong on high level. https://reviews.llvm.org/D37089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D37341: [Sema] Fix an assert-on-invalid by avoiding function template specialisation deduction for invalid functions with fabricated template arguments

vsapsai added a comment. Does your fix work for deeper nesting too (e.g. template in template in template)? Looks like it should, just want to confirm. Are there other places where you need to avoid calling `DeduceTemplateArguments` due to templates depth mismatch? `CheckDependentFunctionTempl

[PATCH] D37322: [Sema] Correct typos in LHS, RHS before building a binop expression.

vsapsai added a comment. Correcting typo in Sema::ActOnMemberAccessExpr is an interesting idea and it almost works. After such change there is a single failure in clang/test/SemaCXX/typo-correction-cxx11.cpp void run(A *annotations) { map new_annotations; auto &annotation = *annota

[PATCH] D37322: [Sema] Correct typos in LHS, RHS before building a binop expression.

This revision was automatically updated to reflect the committed changes. Closed by commit rL313323: [Sema] Correct typos in LHS, RHS before building a binop expression. (authored by vsapsai). Changed prior to commit: https://reviews.llvm.org/D37322?vs=113348&id=115323#toc Repository: rL LLV

[PATCH] D37322: [Sema] Correct typos in LHS, RHS before building a binop expression.

vsapsai added a comment. Thanks for the review. Repository: rL LLVM https://reviews.llvm.org/D37322 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37089: [Sema] Error out early for tags defined inside an enumeration.

vsapsai updated this revision to Diff 115333. vsapsai added a comment. - Remove extraneous diagnostics in test case per Reid Kleckner feedback. https://reviews.llvm.org/D37089 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/SemaDecl.cpp clang/test/Sema/enum.c clan

[PATCH] D37089: [Sema] Error out early for tags defined inside an enumeration.

vsapsai marked an inline comment as done. vsapsai added inline comments. Comment at: clang/test/Sema/enum.c:128 +// PR28903 +struct PR28903 { // expected-warning {{empty struct is a GNU extension}} + enum { rnk wrote: > This is a simpler test case that fixes the

[PATCH] D37881: [Sema] Prevent InstantiateClass from checking unrelated exception specs.

vsapsai created this revision. Sema::InstantiateClass should check only exception specs added during class instantiation and ignore already present delayed specs. This fixes a case where we instantiate a class before parsing member initializers, check exceptions for a different class and fail to f

[PATCH] D37881: [Sema] Prevent InstantiateClass from checking unrelated exception specs.

vsapsai added a comment. I don't like name `SavePendingDelayedStateRAII` as too vague. Hope you'll be able to suggest something better or I'll have better ideas later. https://reviews.llvm.org/D37881 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D37089: [Sema] Error out early for tags defined inside an enumeration.

This revision was automatically updated to reflect the committed changes. vsapsai marked an inline comment as done. Closed by commit rL313386: [Sema] Error out early for tags defined inside an enumeration. (authored by vsapsai). Changed prior to commit: https://reviews.llvm.org/D37089?vs=115333

[PATCH] D38009: [Sema] Fix using old initializer during switch statement transformation.

vsapsai created this revision. Herald added subscribers: kristof.beyls, aemerson. It fixes a crash in CodeGen when we are trying to generate code for initializer expression created before template instantiation, like CallExpr '' |-UnresolvedLookupExpr '' lvalue (ADL) = 'parse' `-DeclRefExpr

<    1   2   3   4   5   6   7   8   9   10   >